From 40f499eb862a87d96f768a012da84d8b4491f7e5 Mon Sep 17 00:00:00 2001 From: Dan Burkert Date: Fri, 19 Dec 2014 02:20:40 -0800 Subject: [PATCH] make ReadTransaction::get return option; cleanup tests --- src/cursor.rs | 109 +++++++++------------------------------------ src/environment.rs | 52 +++++++++++---------- src/lib.rs | 1 - src/transaction.rs | 102 ++++++++++++++++++++---------------------- 4 files changed, 97 insertions(+), 167 deletions(-) diff --git a/src/cursor.rs b/src/cursor.rs index e5bb29c..20715c2 100644 --- a/src/cursor.rs +++ b/src/cursor.rs @@ -40,7 +40,7 @@ pub trait ReadCursor<'txn> : Cursor<'txn> { } /// Open a new read-only cursor on the given database. - fn iter<'t>(&'t mut self) -> Items<'t> { + fn iter(&mut self) -> Items<'txn> { Items::new(self) } } @@ -221,12 +221,10 @@ impl <'txn> Iterator<(&'txn [u8], &'txn [u8])> for Items<'txn> { #[cfg(test)] mod test { - use libc::{c_void, size_t, c_uint}; use std::{io, ptr}; use test::{Bencher, black_box}; use environment::*; - use error::{LmdbResult, lmdb_result}; use ffi::*; use super::*; use transaction::*; @@ -237,23 +235,21 @@ mod test { let env = Environment::new().open(dir.path(), io::USER_RWX).unwrap(); let db = env.open_db(None).unwrap(); + let items = vec!((b"key1", b"val1"), + (b"key2", b"val2"), + (b"key3", b"val3")); + { let mut txn = env.begin_write_txn().unwrap(); - txn.put(db, b"key1", b"val1", WriteFlags::empty()).unwrap(); - txn.put(db, b"key2", b"val2", WriteFlags::empty()).unwrap(); - txn.put(db, b"key3", b"val3", WriteFlags::empty()).unwrap(); + for &(key, data) in items.iter() { + txn.put(db, key, data, WriteFlags::empty()).unwrap(); + } txn.commit().unwrap(); } let txn = env.begin_read_txn().unwrap(); let mut cursor = txn.open_read_cursor(db).unwrap(); - let iter = cursor.iter(); - - let items: Vec<(&[u8], &[u8])> = iter.collect(); - assert_eq!(vec!((b"key1", b"val1"), - (b"key2", b"val2"), - (b"key3", b"val3")), - items); + assert_eq!(items, cursor.iter().collect::>()); } #[test] @@ -354,77 +350,6 @@ mod test { assert!(cursor.get(None, None, MDB_NEXT_MULTIPLE).is_err()); } - /// Checks assumptions about which get operations return keys. - #[test] - fn test_get_keys() { - let dir = io::TempDir::new("test").unwrap(); - let env = Environment::new().open(dir.path(), io::USER_RWX).unwrap(); - - unsafe fn slice_to_val(slice: Option<&[u8]>) -> MDB_val { - match slice { - Some(slice) => - MDB_val { mv_size: slice.len() as size_t, - mv_data: slice.as_ptr() as *mut c_void }, - None => - MDB_val { mv_size: 0, - mv_data: ptr::null_mut() }, - } - } - - /// Returns true if the cursor get sets the key. - fn sets_key(cursor: &Cursor, - key: Option<&[u8]>, - data: Option<&[u8]>, - op: c_uint) - -> LmdbResult { - unsafe { - let mut key_val = slice_to_val(key); - let mut data_val = slice_to_val(data); - let key_ptr = key_val.mv_data; - try!(lmdb_result(mdb_cursor_get(cursor.cursor(), - &mut key_val, - &mut data_val, - op))); - Ok(key_ptr != key_val.mv_data) - } - } - let db = env.create_db(None, MDB_DUPSORT | MDB_DUPFIXED).unwrap(); - - let mut txn = env.begin_write_txn().unwrap(); - txn.put(db, b"key1", b"val1", WriteFlags::empty()).unwrap(); - txn.put(db, b"key1", b"val2", WriteFlags::empty()).unwrap(); - txn.put(db, b"key1", b"val3", WriteFlags::empty()).unwrap(); - txn.put(db, b"key2", b"val4", WriteFlags::empty()).unwrap(); - txn.put(db, b"key2", b"val5", WriteFlags::empty()).unwrap(); - txn.put(db, b"key2", b"val6", WriteFlags::empty()).unwrap(); - txn.put(db, b"key3", b"val7", WriteFlags::empty()).unwrap(); - txn.put(db, b"key3", b"val8", WriteFlags::empty()).unwrap(); - txn.put(db, b"key3", b"val9", WriteFlags::empty()).unwrap(); - - let cursor = txn.open_read_cursor(db).unwrap(); - assert!(sets_key(&cursor, None, None, MDB_FIRST).unwrap()); - assert!(!sets_key(&cursor, None, None, MDB_FIRST_DUP).unwrap()); - assert!(!sets_key(&cursor, Some(b"key2"), Some(b"val5"), MDB_GET_BOTH).unwrap()); - assert!(!sets_key(&cursor, Some(b"key2"), Some(b"val"), MDB_GET_BOTH_RANGE).unwrap()); - assert!(sets_key(&cursor, None, None, MDB_GET_CURRENT).unwrap()); - assert!(!sets_key(&cursor, None, None, MDB_GET_MULTIPLE).unwrap()); - assert!(sets_key(&cursor, None, None, MDB_LAST).unwrap()); - assert!(!sets_key(&cursor, None, None, MDB_LAST_DUP).unwrap()); - sets_key(&cursor, None, None, MDB_FIRST).unwrap(); - assert!(sets_key(&cursor, None, None, MDB_NEXT).unwrap()); - assert!(sets_key(&cursor, None, None, MDB_NEXT_DUP).unwrap()); - sets_key(&cursor, None, None, MDB_FIRST).unwrap(); - assert!(sets_key(&cursor, None, None, MDB_NEXT_MULTIPLE).unwrap()); - assert!(sets_key(&cursor, None, None, MDB_NEXT_NODUP).unwrap()); - assert!(sets_key(&cursor, None, None, MDB_PREV).unwrap()); - sets_key(&cursor, None, None, MDB_LAST).unwrap(); - assert!(sets_key(&cursor, None, None, MDB_PREV_DUP).unwrap()); - assert!(sets_key(&cursor, None, None, MDB_PREV_NODUP).unwrap()); - assert!(!sets_key(&cursor, Some(b"key2"), None, MDB_SET).unwrap()); - assert!(sets_key(&cursor, Some(b"key2"), None, MDB_SET_KEY).unwrap()); - assert!(sets_key(&cursor, Some(b"key2"), None, MDB_SET_RANGE).unwrap()); - } - #[test] fn test_put_del() { let dir = io::TempDir::new("test").unwrap(); @@ -446,7 +371,7 @@ mod test { cursor.get(None, None, MDB_LAST).unwrap()); } - fn get_bench_ctx<'a>(num_rows: u32) -> (io::TempDir, Environment) { + fn setup_bench_db<'a>(num_rows: u32) -> (io::TempDir, Environment) { let dir = io::TempDir::new("test").unwrap(); let env = Environment::new().open(dir.path(), io::USER_RWX).unwrap(); @@ -454,7 +379,11 @@ mod test { let db = env.open_db(None).unwrap(); let mut txn = env.begin_write_txn().unwrap(); for i in range(0, num_rows) { - txn.put(db, format!("key{}", i).as_bytes(), format!("val{}", i).as_bytes(), WriteFlags::empty()).unwrap(); + txn.put(db, + format!("key{}", i).as_bytes(), + format!("val{}", i).as_bytes(), + WriteFlags::empty()) + .unwrap(); } txn.commit().unwrap(); } @@ -465,7 +394,7 @@ mod test { #[bench] fn bench_seq_iter(b: &mut Bencher) { let n = 100; - let (_dir, env) = get_bench_ctx(n); + let (_dir, env) = setup_bench_db(n); let db = env.open_db(None).unwrap(); let txn = env.begin_read_txn().unwrap(); @@ -473,10 +402,12 @@ mod test { let mut cursor = txn.open_read_cursor(db).unwrap(); let mut i = 0; let mut count = 0u32; + for (key, data) in cursor.iter() { i = i + key.len() + data.len(); count = count + 1; } + black_box(i); assert_eq!(count, n); }); @@ -486,7 +417,7 @@ mod test { #[bench] fn bench_seq_cursor(b: &mut Bencher) { let n = 100; - let (_dir, env) = get_bench_ctx(n); + let (_dir, env) = setup_bench_db(n); let db = env.open_db(None).unwrap(); let txn = env.begin_read_txn().unwrap(); @@ -509,7 +440,7 @@ mod test { #[bench] fn bench_seq_raw(b: &mut Bencher) { let n = 100; - let (_dir, env) = get_bench_ctx(n); + let (_dir, env) = setup_bench_db(n); let db = env.open_db(None).unwrap(); let dbi: MDB_dbi = db.dbi(); diff --git a/src/environment.rs b/src/environment.rs index 285cc73..8037059 100644 --- a/src/environment.rs +++ b/src/environment.rs @@ -110,17 +110,25 @@ impl Environment { /// Close a database handle. Normally unnecessary. /// - /// This call is not mutex protected. Handles should only be closed by a single thread, and only - /// if no other threads are going to reference the database handle or one of its cursors any - /// further. Do not close a handle if an existing transaction has modified its database. Doing - /// so can cause misbehavior from database corruption to errors like `MDB_BAD_VALSIZE` (since the - /// DB name is gone). - /// /// Closing a database handle is not necessary, but lets `Transaction::open_database` reuse the /// handle value. Usually it's better to set a bigger `EnvironmentBuilder::set_max_dbs`, unless /// that value would be large. - pub unsafe fn close_db(&self, db: Database) { - mdb_dbi_close(self.env, db.dbi()) + pub fn close_db(&mut self, name: Option<&str>) -> LmdbResult<()> { + let db = try!(self.open_db(name)); + unsafe { mdb_dbi_close(self.env, db.dbi()) }; + Ok(()) + } + + pub fn clear_db(&mut self, name: Option<&str>) -> LmdbResult<()> { + let db = try!(self.open_db(name)); + let txn = try!(self.begin_write_txn()); + unsafe { lmdb_result(mdb_drop(txn.txn(), db.dbi(), 0)) } + } + + pub fn drop_db(&mut self, name: Option<&str>) -> LmdbResult<()> { + let db = try!(self.open_db(name)); + let txn = try!(self.begin_write_txn()); + unsafe { lmdb_result(mdb_drop(txn.txn(), db.dbi(), 1)) } } } @@ -236,10 +244,10 @@ mod test { .open(dir.path(), io::USER_RWX) .is_err()); - // opening non-existent env should not fail + // opening non-existent env should succeed assert!(Environment::new().open(dir.path(), io::USER_RWX).is_ok()); - // opening env with read-only should not fail + // opening env with read-only should succeed assert!(Environment::new().set_flags(MDB_RDONLY) .open(dir.path(), io::USER_RWX) .is_ok()); @@ -248,25 +256,20 @@ mod test { #[test] fn test_begin_txn() { let dir = io::TempDir::new("test").unwrap(); - let env = Environment::new().open(dir.path(), io::USER_RWX).unwrap(); - { - // Mutable env, mutable txn + { // writable environment + let env = Environment::new().open(dir.path(), io::USER_RWX).unwrap(); + assert!(env.begin_write_txn().is_ok()); - } { - // Mutable env, read-only txn assert!(env.begin_read_txn().is_ok()); - } { - // Read-only env, mutable txn + } + + { // read-only environment let env = Environment::new().set_flags(MDB_RDONLY) .open(dir.path(), io::USER_RWX) .unwrap(); + assert!(env.begin_write_txn().is_err()); - } { - // Read-only env, read-only txn - let env = Environment::new().set_flags(MDB_RDONLY) - .open(dir.path(), io::USER_RWX) - .unwrap(); assert!(env.begin_read_txn().is_ok()); } } @@ -274,9 +277,10 @@ mod test { #[test] fn test_open_db() { let dir = io::TempDir::new("test").unwrap(); - let env = Environment::new().set_max_dbs(10) + let env = Environment::new().set_max_dbs(1) .open(dir.path(), io::USER_RWX) .unwrap(); + assert!(env.open_db(None).is_ok()); assert!(env.open_db(Some("testdb")).is_err()); } @@ -284,7 +288,7 @@ mod test { #[test] fn test_create_db() { let dir = io::TempDir::new("test").unwrap(); - let env = Environment::new().set_max_dbs(10) + let env = Environment::new().set_max_dbs(11) .open(dir.path(), io::USER_RWX) .unwrap(); assert!(env.open_db(Some("testdb")).is_err()); diff --git a/src/lib.rs b/src/lib.rs index bc461e4..d4aea2d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -13,7 +13,6 @@ extern crate "lmdb-sys" as ffi; extern crate test; extern crate collections; - pub use cursor::Cursor; pub use database::Database; pub use environment::{Environment, EnvironmentBuilder}; diff --git a/src/transaction.rs b/src/transaction.rs index ef9d5b1..ad996b0 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -6,7 +6,7 @@ use std::io::BufWriter; use cursor::{RoCursor, RwCursor}; use environment::Environment; use database::Database; -use error::{LmdbResult, lmdb_result}; +use error::{LmdbError, LmdbResult, lmdb_result}; use ffi; use ffi::MDB_txn; use ffi::{DatabaseFlags, EnvironmentFlags, WriteFlags, MDB_RDONLY, MDB_RESERVE}; @@ -31,7 +31,7 @@ pub trait TransactionExt<'env> : Transaction<'env> { /// Any pending operations will be saved. fn commit(self) -> LmdbResult<()> { unsafe { - let result = lmdb_result(::ffi::mdb_txn_commit(self.txn())); + let result = lmdb_result(ffi::mdb_txn_commit(self.txn())); mem::forget(self); result } @@ -57,22 +57,26 @@ pub trait ReadTransaction<'env> : Transaction<'env> { /// This function retrieves the data associated with the given key in the database. If the /// database supports duplicate keys (`MDB_DUPSORT`) then the first data item for the key will /// be returned. Retrieval of other items requires the use of `Transaction::cursor_get`. - fn get<'txn>(&'txn self, database: Database, key: &[u8]) -> LmdbResult<&'txn [u8]> { - let mut key_val: ::ffi::MDB_val = ::ffi::MDB_val { mv_size: key.len() as size_t, + fn get<'txn>(&'txn self, database: Database, key: &[u8]) -> LmdbResult> { + let mut key_val: ffi::MDB_val = ffi::MDB_val { mv_size: key.len() as size_t, mv_data: key.as_ptr() as *mut c_void }; - let mut data_val: ::ffi::MDB_val = ::ffi::MDB_val { mv_size: 0, + let mut data_val: ffi::MDB_val = ffi::MDB_val { mv_size: 0, mv_data: ptr::null_mut() }; unsafe { - try!(lmdb_result(::ffi::mdb_get(self.txn(), - database.dbi(), - &mut key_val, - &mut data_val))); - let slice: &'txn [u8] = - mem::transmute(raw::Slice { - data: data_val.mv_data as *const u8, - len: data_val.mv_size as uint - }); - Ok(slice) + let err_code = ffi::mdb_get(self.txn(), database.dbi(), &mut key_val, &mut data_val); + if err_code == 0 { + let slice: &'txn [u8] = + mem::transmute(raw::Slice { + data: data_val.mv_data as *const u8, + len: data_val.mv_size as uint + }); + Ok(Some(slice)) + + } else if err_code == ffi::MDB_NOTFOUND { + Ok(None) + } else { + Err(LmdbError::from_err_code(err_code)) + } } } @@ -282,8 +286,8 @@ mod test { use std::sync::{Arc, Barrier, Future}; use environment::*; - use super::*; use ffi::*; + use super::*; #[test] @@ -299,13 +303,13 @@ mod test { txn.commit().unwrap(); let mut txn = env.begin_write_txn().unwrap(); - assert_eq!(b"val1", txn.get(db, b"key1").unwrap()); - assert_eq!(b"val2", txn.get(db, b"key2").unwrap()); - assert_eq!(b"val3", txn.get(db, b"key3").unwrap()); - assert!(txn.get(db, b"key").is_err()); + assert_eq!(b"val1", txn.get(db, b"key1").unwrap().unwrap()); + assert_eq!(b"val2", txn.get(db, b"key2").unwrap().unwrap()); + assert_eq!(b"val3", txn.get(db, b"key3").unwrap().unwrap()); + assert!(txn.get(db, b"key").unwrap().is_none()); txn.del(db, b"key1", None).unwrap(); - assert!(txn.get(db, b"key1").is_err()); + assert!(txn.get(db, b"key1").unwrap().is_none()); } #[test] @@ -322,50 +326,42 @@ mod test { txn.commit().unwrap(); let mut txn = env.begin_write_txn().unwrap(); - assert_eq!(b"val1", txn.get(db, b"key1").unwrap()); - assert!(txn.get(db, b"key").is_err()); + assert_eq!(b"val1", txn.get(db, b"key1").unwrap().unwrap()); + assert!(txn.get(db, b"key").unwrap().is_none()); txn.del(db, b"key1", None).unwrap(); - assert!(txn.get(db, b"key1").is_err()); + assert!(txn.get(db, b"key1").unwrap().is_none()); } #[test] fn test_close_database() { let dir = io::TempDir::new("test").unwrap(); - let env = Arc::new(Environment::new() - .set_max_dbs(10) - .open(dir.path(), io::USER_RWX) - .unwrap()); - - let db1 = { - let db = env.create_db(Some("db"), DatabaseFlags::empty()).unwrap(); - let txn = env.begin_write_txn().unwrap(); - txn.commit().unwrap(); - db - }; + let mut env = Environment::new().set_max_dbs(10) + .open(dir.path(), io::USER_RWX) + .unwrap(); - let db2 = { - let db = env.open_db(Some("db")).unwrap(); - let txn = env.begin_read_txn().unwrap(); - txn.commit().unwrap(); - db - }; + env.create_db(Some("db"), DatabaseFlags::empty()).unwrap(); + env.close_db(Some("db")).unwrap(); + assert!(env.open_db(Some("db")).is_ok()); + } - // Check that database handles are reused properly - assert!(db1.dbi() == db2.dbi()); + #[test] + fn test_clear_db() { + let dir = io::TempDir::new("test").unwrap(); + let mut env = Environment::new().open(dir.path(), io::USER_RWX).unwrap(); { + let db = env.open_db(None).unwrap(); let mut txn = env.begin_write_txn().unwrap(); - txn.put(db1, b"key1", b"val1", WriteFlags::empty()).unwrap(); - assert!(txn.commit().is_ok()); + txn.put(db, b"key", b"val", WriteFlags::empty()).unwrap(); + txn.commit().unwrap(); } - unsafe { env.close_db(db1) }; + env.clear_db(None).unwrap(); - { - let mut txn = env.begin_write_txn().unwrap(); - assert!(txn.put(db1, b"key2", b"val2", WriteFlags::empty()).is_err()); - } + let db = env.open_db(None).unwrap(); + let txn = env.begin_read_txn().unwrap(); + txn.get(db, b"key").is_err(); } #[test] @@ -388,14 +384,14 @@ mod test { let db = reader_env.open_db(None).unwrap(); { let txn = reader_env.begin_read_txn().unwrap(); - assert!(txn.get(db, key).is_err()); + assert!(txn.get(db, key).unwrap().is_none()); txn.abort(); } reader_barrier.wait(); reader_barrier.wait(); { let txn = reader_env.begin_read_txn().unwrap(); - txn.get(db, key).unwrap() == val + txn.get(db, key).unwrap().unwrap() == val } })); } @@ -443,7 +439,7 @@ mod test { for i in range(0, n) { assert_eq!( format!("{}{}", val, i).as_bytes(), - txn.get(db, format!("{}{}", key, i).as_bytes()).unwrap()); + txn.get(db, format!("{}{}", key, i).as_bytes()).unwrap().unwrap()); } } }