Clean up code a bit, make clippy happier, use proper types.

Cargo.toml

@@ -2,7 +2,7 @@
 
 name = "rocksdb"
 description = "A Rust wrapper for Facebook's RocksDB embeddable database."
-version = "0.3.2"
+version = "0.4.0"
 authors = ["Tyler Neely <t@jujit.su>", "David Greenberg <dsg123456789@gmail.com>"]
 license = "Apache-2.0"
 keywords = ["database", "embedded", "LSM-tree", "persistence"]
@@ -24,4 +24,4 @@ name = "test"
 path = "test/test.rs"
 
 [dependencies]
-libc = "0.1.8"
+libc = "0.2.13"

README.md

@@ -3,7 +3,7 @@ rust-rocksdb
 [![Build Status](https://travis-ci.org/spacejam/rust-rocksdb.svg?branch=master)](https://travis-ci.org/spacejam/rust-rocksdb)
 [![crates.io](http://meritbadge.herokuapp.com/rocksdb)](https://crates.io/crates/rocksdb)
 
-This library has been tested against RocksDB 3.13.1 on linux and OSX.  The 0.3.2 crate should work with the Rust 1.5 stable and nightly releases as of 5/1/16.
+This library has been tested against RocksDB 3.13.1 on linux and OSX.  The 0.4.0 crate should work with the Rust 1.9 stable and nightly releases as of 7/1/16.
 
 ### status
   - [x] basic open/put/get/delete/close
@@ -36,7 +36,7 @@ sudo make install
 ###### Cargo.toml
 ```rust
 [dependencies]
-rocksdb = "0.3.2"
+rocksdb = "0.4.0"
 ```
 ###### Code
 ```rust
@@ -65,7 +65,7 @@ fn main() {
     // NB: db is automatically freed at end of lifetime
     let mut db = DB::open_default("/path/for/rocksdb/storage").unwrap();
     {
-        let mut batch = WriteBatch::new(); // WriteBatch and db both have trait Writable
+        let mut batch = WriteBatch::default(); // WriteBatch and db both have trait Writable
         batch.put(b"my key", b"my value");
         batch.put(b"key2", b"value2");
         batch.put(b"key3", b"value3");
@@ -139,7 +139,7 @@ fn concat_merge(new_key: &[u8], existing_val: Option<&[u8]>,
 
 fn main() {
     let path = "/path/to/rocksdb";
-    let mut opts = Options::new();
+    let mut opts = Options::default();
     opts.create_if_missing(true);
     opts.add_merge_operator("test operator", concat_merge);
     let mut db = DB::open(&opts, path).unwrap();
@@ -161,7 +161,7 @@ use rocksdb::DBCompactionStyle::DBUniversalCompaction;
 
 fn badly_tuned_for_somebody_elses_disk() -> DB {
     let path = "_rust_rocksdb_optimizetest";
-    let mut opts = Options::new();
+    let mut opts = Options::default();
     opts.create_if_missing(true);
     opts.set_max_open_files(10000);
     opts.set_use_fsync(false);

src/ffi.rs

@@ -157,7 +157,7 @@ extern "C" {
     pub fn rocksdb_options_set_level0_stop_writes_trigger(options: DBOptions,
                                                           no: c_int);
     pub fn rocksdb_options_set_write_buffer_size(options: DBOptions,
-                                                 bytes: u64);
+                                                 bytes: size_t);
     pub fn rocksdb_options_set_target_file_size_base(options: DBOptions,
                                                      bytes: u64);
     pub fn rocksdb_options_set_target_file_size_multiplier(options: DBOptions,

src/main.rs

@@ -41,7 +41,7 @@ use rocksdb::{DB, MergeOperands, Options, Writable};
 // std::str::from_utf8(v).unwrap());
 //        };
 //    }
-//    let opts = Options::new();
+//    let opts = Options::default();
 //    assert!(DB::destroy(&opts, path).is_ok());
 // }
 
@@ -86,7 +86,7 @@ fn concat_merge(_: &[u8],
 
 fn custom_merge() {
     let path = "_rust_rocksdb_mergetest";
-    let mut opts = Options::new();
+    let mut opts = Options::default();
     opts.create_if_missing(true);
     opts.add_merge_operator("test operator", concat_merge);
     {
@@ -114,7 +114,7 @@ fn custom_merge() {
 #[cfg(feature = "valgrind")]
 fn main() {
     let path = "_rust_rocksdb_valgrind";
-    let mut opts = Options::new();
+    let mut opts = Options::default();
     opts.create_if_missing(true);
     opts.add_merge_operator("test operator", concat_merge);
     let db = DB::open(&opts, path).unwrap();
@@ -180,8 +180,8 @@ mod tests {
     // dirty hack due to parallel tests causing contention.
     // sleep_ms(1000);
     // let path = "_rust_rocksdb_optimizetest";
-    // let mut opts = Options::new();
-    // let mut blockopts = BlockBasedOptions::new();
+    // let mut opts = Options::default();
+    // let mut blockopts = BlockBasedOptions::default();
     // let mut db = tuned_for_somebody_elses_disk(path, &mut opts, &mut blockopts);
     // let mut i = 0 as u64;
     // b.iter(|| {
@@ -193,8 +193,8 @@ mod tests {
     // #[bench]
     // fn b_reads(b: &mut Bencher) {
     // let path = "_rust_rocksdb_optimizetest";
-    // let mut opts = Options::new();
-    // let mut blockopts = BlockBasedOptions::new();
+    // let mut opts = Options::default();
+    // let mut blockopts = BlockBasedOptions::default();
     // {
     // let db = tuned_for_somebody_elses_disk(path, &mut opts, &mut blockopts);
     // let mut i = 0 as u64;

src/merge_operator.rs

@@ -19,10 +19,11 @@ use std::mem;
 use std::ptr;
 use std::slice;
 
+pub type MergeFn = fn(&[u8], Option<&[u8]>, &mut MergeOperands) -> Vec<u8>;
 
 pub struct MergeOperatorCallback {
     pub name: CString,
-    pub merge_fn: fn(&[u8], Option<&[u8]>, &mut MergeOperands) -> Vec<u8>,
+    pub merge_fn: MergeFn,
 }
 
 pub extern "C" fn destructor_callback(raw_cb: *mut c_void) {
@@ -182,7 +183,7 @@ fn mergetest() {
     use rocksdb::{DB, DBVector, Writable};
 
     let path = "_rust_rocksdb_mergetest";
-    let mut opts = Options::new();
+    let mut opts = Options::default();
     opts.create_if_missing(true);
     opts.add_merge_operator("test operator", test_provided_merge);
     {

src/rocksdb.rs

@@ -60,10 +60,12 @@ pub enum Direction {
     Reverse,
 }
 
+pub type KVBytes = (Box<[u8]>, Box<[u8]>);
+
 impl Iterator for DBIterator {
-    type Item = (Box<[u8]>, Box<[u8]>);
+    type Item = KVBytes;
 
-    fn next(&mut self) -> Option<(Box<[u8]>, Box<[u8]>)> {
+    fn next(&mut self) -> Option<KVBytes> {
         let native_iter = self.inner;
         if !self.just_seeked {
             match self.direction {
@@ -201,13 +203,13 @@ impl<'a> Snapshot<'a> {
     }
 
     pub fn iterator(&self, mode: IteratorMode) -> DBIterator {
-        let mut readopts = ReadOptions::new();
+        let mut readopts = ReadOptions::default();
         readopts.set_snapshot(self);
         DBIterator::new(self.db, &readopts, mode)
     }
 
     pub fn get(&self, key: &[u8]) -> Result<Option<DBVector>, String> {
-        let mut readopts = ReadOptions::new();
+        let mut readopts = ReadOptions::default();
         readopts.set_snapshot(self);
         self.db.get_opt(key, &readopts)
     }
@@ -216,7 +218,7 @@ impl<'a> Snapshot<'a> {
                   cf: DBCFHandle,
                   key: &[u8])
                   -> Result<Option<DBVector>, String> {
-        let mut readopts = ReadOptions::new();
+        let mut readopts = ReadOptions::default();
         readopts.set_snapshot(self);
         self.db.get_cf_opt(cf, key, &readopts)
     }
@@ -250,7 +252,7 @@ pub trait Writable {
 
 impl DB {
     pub fn open_default(path: &str) -> Result<DB, String> {
-        let mut opts = Options::new();
+        let mut opts = Options::default();
         opts.create_if_missing(true);
         DB::open(&opts, path)
     }
@@ -410,7 +412,7 @@ impl DB {
     }
 
     pub fn write(&self, batch: WriteBatch) -> Result<(), String> {
-        self.write_opt(batch, &WriteOptions::new())
+        self.write_opt(batch, &WriteOptions::default())
     }
 
     pub fn get_opt(&self,
@@ -449,7 +451,7 @@ impl DB {
     }
 
     pub fn get(&self, key: &[u8]) -> Result<Option<DBVector>, String> {
-        self.get_opt(key, &ReadOptions::new())
+        self.get_opt(key, &ReadOptions::default())
     }
 
     pub fn get_cf_opt(&self,
@@ -493,7 +495,7 @@ impl DB {
                   cf: DBCFHandle,
                   key: &[u8])
                   -> Result<Option<DBVector>, String> {
-        self.get_cf_opt(cf, key, &ReadOptions::new())
+        self.get_cf_opt(cf, key, &ReadOptions::default())
     }
 
     pub fn create_cf(&mut self,
@@ -551,16 +553,16 @@ impl DB {
     }
 
     pub fn iterator(&self, mode: IteratorMode) -> DBIterator {
-        let opts = ReadOptions::new();
-        DBIterator::new(&self, &opts, mode)
+        let opts = ReadOptions::default();
+        DBIterator::new(self, &opts, mode)
     }
 
     pub fn iterator_cf(&self,
                        cf_handle: DBCFHandle,
                        mode: IteratorMode)
                        -> Result<DBIterator, String> {
-        let opts = ReadOptions::new();
-        DBIterator::new_cf(&self, cf_handle, &opts, mode)
+        let opts = ReadOptions::default();
+        DBIterator::new_cf(self, cf_handle, &opts, mode)
     }
 
     pub fn snapshot(&self) -> Snapshot {
@@ -698,7 +700,7 @@ impl DB {
 
 impl Writable for DB {
     fn put(&self, key: &[u8], value: &[u8]) -> Result<(), String> {
-        self.put_opt(key, value, &WriteOptions::new())
+        self.put_opt(key, value, &WriteOptions::default())
     }
 
     fn put_cf(&self,
@@ -706,11 +708,11 @@ impl Writable for DB {
               key: &[u8],
               value: &[u8])
               -> Result<(), String> {
-        self.put_cf_opt(cf, key, value, &WriteOptions::new())
+        self.put_cf_opt(cf, key, value, &WriteOptions::default())
     }
 
     fn merge(&self, key: &[u8], value: &[u8]) -> Result<(), String> {
-        self.merge_opt(key, value, &WriteOptions::new())
+        self.merge_opt(key, value, &WriteOptions::default())
     }
 
     fn merge_cf(&self,
@@ -718,20 +720,20 @@ impl Writable for DB {
                 key: &[u8],
                 value: &[u8])
                 -> Result<(), String> {
-        self.merge_cf_opt(cf, key, value, &WriteOptions::new())
+        self.merge_cf_opt(cf, key, value, &WriteOptions::default())
     }
 
     fn delete(&self, key: &[u8]) -> Result<(), String> {
-        self.delete_opt(key, &WriteOptions::new())
+        self.delete_opt(key, &WriteOptions::default())
     }
 
     fn delete_cf(&self, cf: DBCFHandle, key: &[u8]) -> Result<(), String> {
-        self.delete_cf_opt(cf, key, &WriteOptions::new())
+        self.delete_cf_opt(cf, key, &WriteOptions::default())
     }
 }
 
-impl WriteBatch {
-    pub fn new() -> WriteBatch {
+impl Default for WriteBatch {
+    fn default() -> WriteBatch {
         WriteBatch {
             inner: unsafe { rocksdb_ffi::rocksdb_writebatch_create() },
         }
@@ -837,11 +839,6 @@ impl Drop for ReadOptions {
 }
 
 impl ReadOptions {
-    fn new() -> ReadOptions {
-        unsafe {
-            ReadOptions { inner: rocksdb_ffi::rocksdb_readoptions_create() }
-        }
-    }
     // TODO add snapshot setting here
     // TODO add snapshot wrapper structs with proper destructors;
     // that struct needs an "iterator" impl too.
@@ -860,6 +857,14 @@ impl ReadOptions {
     }
 }
 
+impl Default for ReadOptions {
+    fn default() -> ReadOptions {
+        unsafe {
+            ReadOptions { inner: rocksdb_ffi::rocksdb_readoptions_create() }
+        }
+    }
+}
+
 pub struct DBVector {
     base: *mut u8,
     len: usize,
@@ -905,7 +910,7 @@ fn external() {
         assert!(db.delete(b"k1").is_ok());
         assert!(db.get(b"k1").unwrap().is_none());
     }
-    let opts = Options::new();
+    let opts = Options::default();
     let result = DB::destroy(&opts, path);
     assert!(result.is_ok());
 }
@@ -914,7 +919,7 @@ fn external() {
 fn errors_do_stuff() {
     let path = "_rust_rocksdb_error";
     let db = DB::open_default(path).unwrap();
-    let opts = Options::new();
+    let opts = Options::default();
     // The DB will still be open when we try to destroy and the lock should fail
     match DB::destroy(&opts, path) {
         Err(ref s) => {
@@ -933,7 +938,7 @@ fn writebatch_works() {
         let db = DB::open_default(path).unwrap();
         {
             // test put
-            let batch = WriteBatch::new();
+            let batch = WriteBatch::default();
             assert!(db.get(b"k1").unwrap().is_none());
             let _ = batch.put(b"k1", b"v1111");
             assert!(db.get(b"k1").unwrap().is_none());
@@ -944,14 +949,14 @@ fn writebatch_works() {
         }
         {
             // test delete
-            let batch = WriteBatch::new();
+            let batch = WriteBatch::default();
             let _ = batch.delete(b"k1");
             let p = db.write(batch);
             assert!(p.is_ok());
             assert!(db.get(b"k1").unwrap().is_none());
         }
     }
-    let opts = Options::new();
+    let opts = Options::default();
     assert!(DB::destroy(&opts, path).is_ok());
 }
 
@@ -973,7 +978,7 @@ fn iterator_test() {
                      from_utf8(&*v).unwrap());
         }
     }
-    let opts = Options::new();
+    let opts = Options::default();
     assert!(DB::destroy(&opts, path).is_ok());
 }
 
@@ -995,6 +1000,6 @@ fn snapshot_test() {
         assert!(db.get(b"k2").unwrap().is_some());
         assert!(snap.get(b"k2").unwrap().is_none());
     }
-    let opts = Options::new();
+    let opts = Options::default();
     assert!(DB::destroy(&opts, path).is_ok());
 }

src/rocksdb_options.rs

@@ -13,12 +13,12 @@
 // limitations under the License.
 //
 extern crate libc;
-use self::libc::{c_int, size_t};
+use self::libc::c_int;
 use std::ffi::CString;
 use std::mem;
 
 use rocksdb_ffi;
-use merge_operator::{self, MergeOperands, MergeOperatorCallback,
+use merge_operator::{self, MergeFn, MergeOperatorCallback,
                      full_merge_callback, partial_merge_callback};
 use comparator::{self, ComparatorCallback, compare_callback};
 
@@ -60,17 +60,10 @@ impl Drop for WriteOptions {
 
 impl BlockBasedOptions {
     pub fn new() -> BlockBasedOptions {
-        let block_opts = unsafe {
-            rocksdb_ffi::rocksdb_block_based_options_create()
-        };
-        let rocksdb_ffi::DBBlockBasedTableOptions(opt_ptr) = block_opts;
-        if opt_ptr.is_null() {
-            panic!("Could not create rocksdb block based options".to_string());
-        }
-        BlockBasedOptions { inner: block_opts }
+        BlockBasedOptions::default()
     }
 
-    pub fn set_block_size(&mut self, size: u64) {
+    pub fn set_block_size(&mut self, size: usize) {
         unsafe {
             rocksdb_ffi::rocksdb_block_based_options_set_block_size(self.inner,
                                                                     size);
@@ -78,42 +71,20 @@ impl BlockBasedOptions {
     }
 }
 
-// TODO figure out how to create these in a Rusty way
-// /pub fn set_filter(&mut self, filter: rocksdb_ffi::DBFilterPolicy) {
-// /    unsafe {
-// /        rocksdb_ffi::rocksdb_block_based_options_set_filter_policy(
-// /            self.inner, filter);
-// /    }
-// /}
-
-/// /pub fn set_cache(&mut self, cache: rocksdb_ffi::DBCache) {
-/// /    unsafe {
-/// /        rocksdb_ffi::rocksdb_block_based_options_set_block_cache(
-/// /            self.inner, cache);
-/// /    }
-/// /}
-
-/// /pub fn set_cache_compressed(&mut self, cache: rocksdb_ffi::DBCache) {
-/// /    unsafe {
-/// /        rocksdb_ffi::
-/// rocksdb_block_based_options_set_block_cache_compressed(
-/// /            self.inner, cache);
-/// /    }
-/// /}
-
-
-impl Options {
-    pub fn new() -> Options {
-        unsafe {
-            let opts = rocksdb_ffi::rocksdb_options_create();
-            let rocksdb_ffi::DBOptions(opt_ptr) = opts;
+impl Default for BlockBasedOptions {
+    fn default() -> BlockBasedOptions {
+        let block_opts = unsafe {
+            rocksdb_ffi::rocksdb_block_based_options_create()
+        };
+        let rocksdb_ffi::DBBlockBasedTableOptions(opt_ptr) = block_opts;
         if opt_ptr.is_null() {
-                panic!("Could not create rocksdb options".to_string());
-            }
-            Options { inner: opts }
+            panic!("Could not create rocksdb block based options".to_string());
         }
+        BlockBasedOptions { inner: block_opts }
     }
+}
 
+impl Options {
     pub fn increase_parallelism(&mut self, parallelism: i32) {
         unsafe {
             rocksdb_ffi::rocksdb_options_increase_parallelism(self.inner,
@@ -138,10 +109,7 @@ impl Options {
 
     pub fn add_merge_operator(&mut self,
                               name: &str,
-                              merge_fn: fn(&[u8],
-                                           Option<&[u8]>,
-                                           &mut MergeOperands)
-                              -> Vec<u8>) {
+                              merge_fn: MergeFn) {
         let cb = Box::new(MergeOperatorCallback {
             name: CString::new(name.as_bytes()).unwrap(),
             merge_fn: merge_fn,
@@ -238,7 +206,7 @@ impl Options {
         }
     }
 
-    pub fn set_write_buffer_size(&mut self, size: size_t) {
+    pub fn set_write_buffer_size(&mut self, size: usize) {
         unsafe {
             rocksdb_ffi::rocksdb_options_set_write_buffer_size(self.inner,
                                                                size);
@@ -320,8 +288,33 @@ impl Options {
     }
 }
 
+impl Default for Options {
+    fn default() -> Options {
+        unsafe {
+            let opts = rocksdb_ffi::rocksdb_options_create();
+            let rocksdb_ffi::DBOptions(opt_ptr) = opts;
+            if opt_ptr.is_null() {
+                panic!("Could not create rocksdb options".to_string());
+            }
+            Options { inner: opts }
+        }
+    }
+}
+
+
 impl WriteOptions {
     pub fn new() -> WriteOptions {
+        WriteOptions::default()
+    }
+    pub fn set_sync(&mut self, sync: bool) {
+        unsafe {
+            rocksdb_ffi::rocksdb_writeoptions_set_sync(self.inner, sync);
+        }
+    }
+}
+
+impl Default for WriteOptions {
+    fn default() -> WriteOptions {
         let write_opts = unsafe { rocksdb_ffi::rocksdb_writeoptions_create() };
         let rocksdb_ffi::DBWriteOptions(opt_ptr) = write_opts;
         if opt_ptr.is_null() {
@@ -329,9 +322,4 @@ impl WriteOptions {
         }
         WriteOptions { inner: write_opts }
     }
-    pub fn set_sync(&mut self, sync: bool) {
-        unsafe {
-            rocksdb_ffi::rocksdb_writeoptions_set_sync(self.inner, sync);
-        }
-    }
 }

test/test_column_family.rs

@@ -20,11 +20,11 @@ pub fn test_column_family() {
 
     // should be able to create column families
     {
-        let mut opts = Options::new();
+        let mut opts = Options::default();
         opts.create_if_missing(true);
         opts.add_merge_operator("test operator", test_provided_merge);
         let mut db = DB::open(&opts, path).unwrap();
-        let opts = Options::new();
+        let opts = Options::default();
         match db.create_cf("cf1", &opts) {
             Ok(_) => println!("cf1 created successfully"),
             Err(e) => {
@@ -35,7 +35,7 @@ pub fn test_column_family() {
 
     // should fail to open db without specifying same column families
     {
-        let mut opts = Options::new();
+        let mut opts = Options::default();
         opts.add_merge_operator("test operator", test_provided_merge);
         match DB::open(&opts, path) {
             Ok(_) => {
@@ -52,7 +52,7 @@ pub fn test_column_family() {
 
     // should properly open db when specyfing all column families
     {
-        let mut opts = Options::new();
+        let mut opts = Options::default();
         opts.add_merge_operator("test operator", test_provided_merge);
         match DB::open_cf(&opts, path, &["cf1"]) {
             Ok(_) => println!("successfully opened db with column family"),
@@ -61,7 +61,7 @@ pub fn test_column_family() {
     }
     // TODO should be able to write, read, merge, batch, and iterate over a cf
     {
-        let mut opts = Options::new();
+        let mut opts = Options::default();
         opts.add_merge_operator("test operator", test_provided_merge);
         let db = match DB::open_cf(&opts, path, &["cf1"]) {
             Ok(db) => {
@@ -107,14 +107,14 @@ pub fn test_column_family() {
     }
     // should b able to drop a cf
     {
-        let mut db = DB::open_cf(&Options::new(), path, &["cf1"]).unwrap();
+        let mut db = DB::open_cf(&Options::default(), path, &["cf1"]).unwrap();
         match db.drop_cf("cf1") {
             Ok(_) => println!("cf1 successfully dropped."),
             Err(e) => panic!("failed to drop column family: {}", e),
         }
     }
 
-    assert!(DB::destroy(&Options::new(), path).is_ok());
+    assert!(DB::destroy(&Options::default(), path).is_ok());
 }
 
 fn test_provided_merge(_: &[u8],

test/test_iterator.rs

@@ -138,6 +138,6 @@ pub fn test_iterator() {
             assert!(!iterator1.valid());
         }
     }
-    let opts = Options::new();
+    let opts = Options::default();
     assert!(DB::destroy(&opts, path).is_ok());
 }

test/test_multithreaded.rs

@@ -47,5 +47,5 @@ pub fn test_multithreaded() {
         j2.join().unwrap();
         j3.join().unwrap();
     }
-    assert!(DB::destroy(&Options::new(), path).is_ok());
+    assert!(DB::destroy(&Options::default(), path).is_ok());
 }