refactor options (#38)

- restruct content layout
- use enum to represent opaque structs
- cleanup code

src/lib.rs

@@ -24,9 +24,9 @@ pub mod rocksdb_options;
 pub mod merge_operator;
 pub mod comparator;
 
-pub use librocksdb_sys::{DBCompactionStyle, DBComparator, DBCompressionType, DBRecoveryMode,
-                         new_bloom_filter, self as rocksdb_ffi};
-pub use rocksdb::{DB, DBIterator, DBVector, Kv, ReadOptions, SeekKey,
-                  Writable, WriteBatch};
-pub use rocksdb_options::{BlockBasedOptions, Options, WriteOptions};
+pub use librocksdb_sys::{DBCompactionStyle, DBComparator, DBCompressionType,
+                         DBRecoveryMode, new_bloom_filter, self as rocksdb_ffi};
 pub use merge_operator::MergeOperands;
+pub use rocksdb::{DB, DBIterator, DBVector, Kv, SeekKey, Writable, WriteBatch};
+pub use rocksdb_options::{BlockBasedOptions, Options, ReadOptions,
+                          WriteOptions};

src/rocksdb.rs

@@ -12,6 +12,12 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 //
+
+
+use libc::{self, c_int, c_void, size_t};
+
+use rocksdb_ffi::{self, DBCFHandle, error_message};
+use rocksdb_options::{Options, ReadOptions, UnsafeSnap, WriteOptions};
 use std::collections::BTreeMap;
 use std::ffi::{CStr, CString};
 use std::fs;
@@ -20,11 +26,6 @@ use std::path::Path;
 use std::slice;
 use std::str::from_utf8;
 
-use libc::{self, c_int, c_void, size_t};
-
-use rocksdb_ffi::{self, DBCFHandle, error_message};
-use rocksdb_options::{Options, WriteOptions};
-
 const DEFAULT_COLUMN_FAMILY: &'static str = "default";
 
 pub struct DB {
@@ -40,20 +41,6 @@ pub struct WriteBatch {
     inner: rocksdb_ffi::DBWriteBatch,
 }
 
-pub struct ReadOptions {
-    inner: rocksdb_ffi::DBReadOptions,
-    upper_bound: Vec<u8>,
-}
-
-/// The UnsafeSnap must be destroyed by db, it maybe be leaked
-/// if not using it properly, hence named as unsafe.
-///
-/// This object is convenient for wrapping snapshot by yourself. In most
-/// cases, using `Snapshot` is enough.
-pub struct UnsafeSnap {
-    inner: rocksdb_ffi::DBSnapshot,
-}
-
 pub struct Snapshot<'a> {
     db: &'a DB,
     snap: UnsafeSnap,
@@ -82,8 +69,9 @@ impl<'a> From<&'a [u8]> for SeekKey<'a> {
 impl<'a> DBIterator<'a> {
     pub fn new(db: &'a DB, readopts: ReadOptions) -> DBIterator<'a> {
         unsafe {
-            let iterator = rocksdb_ffi::rocksdb_create_iterator(db.inner,
-                                                                readopts.inner);
+            let iterator =
+                rocksdb_ffi::rocksdb_create_iterator(db.inner,
+                                                     readopts.get_inner());
 
             DBIterator {
                 db: db,
@@ -167,7 +155,7 @@ impl<'a> DBIterator<'a> {
         unsafe {
             let iterator =
                 rocksdb_ffi::rocksdb_create_iterator_cf(db.inner,
-                                                        readopts.inner,
+                                                        readopts.get_inner(),
                                                         cf_handle);
             DBIterator {
                 db: db,
@@ -342,8 +330,9 @@ impl DB {
             .map(|_| rocksdb_ffi::DBCFHandle(0 as *mut c_void))
             .collect();
 
-        let cfopts: Vec<rocksdb_ffi::DBOptions> =
-            cf_opts_v.iter().map(|x| x.inner).collect();
+        let cfopts: Vec<_> = cf_opts_v.iter()
+            .map(|x| x.inner as *const rocksdb_ffi::DBOptions)
+            .collect();
 
         let db: rocksdb_ffi::DBInstance;
         let mut err: *const i8 = 0 as *const i8;
@@ -454,14 +443,6 @@ impl DB {
                    key: &[u8],
                    readopts: &ReadOptions)
                    -> Result<Option<DBVector>, String> {
-        if readopts.inner.0.is_null() {
-            return Err("Unable to create rocksdb read options.  This is a \
-                        fairly trivial call, and its failure may be \
-                        indicative of a mis-compiled or mis-loaded rocksdb \
-                        library."
-                .to_owned());
-        }
-
         unsafe {
             let val_len: size_t = 0;
             let val_len_ptr = &val_len as *const size_t;
@@ -469,7 +450,7 @@ impl DB {
             let err_ptr: *mut *const i8 = &mut err;
             let val =
                 rocksdb_ffi::rocksdb_get(self.inner,
-                                         readopts.inner,
+                                         readopts.get_inner(),
                                          key.as_ptr(),
                                          key.len() as size_t,
                                          val_len_ptr,
@@ -494,14 +475,6 @@ impl DB {
                       key: &[u8],
                       readopts: &ReadOptions)
                       -> Result<Option<DBVector>, String> {
-        if readopts.inner.0.is_null() {
-            return Err("Unable to create rocksdb read options.  This is a \
-                        fairly trivial call, and its failure may be \
-                        indicative of a mis-compiled or mis-loaded rocksdb \
-                        library."
-                .to_owned());
-        }
-
         unsafe {
             let val_len: size_t = 0;
             let val_len_ptr = &val_len as *const size_t;
@@ -509,7 +482,7 @@ impl DB {
             let err_ptr: *mut *const i8 = &mut err;
             let val =
                 rocksdb_ffi::rocksdb_get_cf(self.inner,
-                                            readopts.inner,
+                                            readopts.get_inner(),
                                             cf,
                                             key.as_ptr(),
                                             key.len() as size_t,
@@ -611,11 +584,11 @@ impl DB {
     }
 
     pub unsafe fn unsafe_snap(&self) -> UnsafeSnap {
-        UnsafeSnap { inner: rocksdb_ffi::rocksdb_create_snapshot(self.inner) }
+        UnsafeSnap::new(self.inner)
     }
 
     pub unsafe fn release_snap(&self, snap: &UnsafeSnap) {
-        rocksdb_ffi::rocksdb_release_snapshot(self.inner, snap.inner)
+        rocksdb_ffi::rocksdb_release_snapshot(self.inner, snap.get_inner())
     }
 
     pub fn put_opt(&self,
@@ -1095,49 +1068,6 @@ impl Writable for WriteBatch {
     }
 }
 
-impl Drop for ReadOptions {
-    fn drop(&mut self) {
-        unsafe { rocksdb_ffi::rocksdb_readoptions_destroy(self.inner) }
-    }
-}
-
-impl Default for ReadOptions {
-    fn default() -> ReadOptions {
-        unsafe {
-            ReadOptions { inner: rocksdb_ffi::rocksdb_readoptions_create(), upper_bound: vec![] }
-        }
-    }
-}
-
-impl ReadOptions {
-    pub fn new() -> ReadOptions {
-        ReadOptions::default()
-    }
-    // TODO add snapshot setting here
-    // TODO add snapshot wrapper structs with proper destructors;
-    // that struct needs an "iterator" impl too.
-    #[allow(dead_code)]
-    pub fn fill_cache(&mut self, v: bool) {
-        unsafe {
-            rocksdb_ffi::rocksdb_readoptions_set_fill_cache(self.inner, v);
-        }
-    }
-
-    pub unsafe fn set_snapshot(&mut self, snapshot: &UnsafeSnap) {
-        rocksdb_ffi::rocksdb_readoptions_set_snapshot(self.inner,
-                                                      snapshot.inner);
-    }
-
-    pub fn set_iterate_upper_bound(&mut self, key: &[u8]) {
-        self.upper_bound = Vec::from(key);
-        unsafe {
-            rocksdb_ffi::rocksdb_readoptions_set_iterate_upper_bound(self.inner,
-                                                                     self.upper_bound.as_ptr(),
-                                                                     self.upper_bound.len() as size_t);
-        }
-    }
-}
-
 pub struct DBVector {
     base: *mut u8,
     len: usize,
@@ -1173,9 +1103,9 @@ impl DBVector {
 
 #[cfg(test)]
 mod test {
-    use super::*;
     use rocksdb_options::*;
     use std::str;
+    use super::*;
     use tempdir::TempDir;
 
     #[test]

src/rocksdb_options.rs

@@ -12,34 +12,20 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 //
-use libc::{c_int, size_t};
-use std::ffi::CString;
-use std::mem;
 
-use rocksdb_ffi::{self, DBCompressionType, DBRecoveryMode};
+use comparator::{self, ComparatorCallback, compare_callback};
+use libc::{c_int, size_t};
 use merge_operator::{self, MergeOperatorCallback, full_merge_callback,
                      partial_merge_callback};
-use comparator::{self, ComparatorCallback, compare_callback};
 use merge_operator::MergeFn;
 
-pub struct BlockBasedOptions {
-    inner: rocksdb_ffi::DBBlockBasedTableOptions,
-}
-
-pub struct Options {
-    pub inner: rocksdb_ffi::DBOptions,
-}
-
-pub struct WriteOptions {
-    pub inner: rocksdb_ffi::DBWriteOptions,
-}
+use rocksdb_ffi::{self, DBOptions, DBWriteOptions, DBBlockBasedTableOptions,
+                  DBReadOptions, DBCompressionType, DBRecoveryMode};
+use std::ffi::CString;
+use std::mem;
 
-impl Drop for Options {
-    fn drop(&mut self) {
-        unsafe {
-            rocksdb_ffi::rocksdb_options_destroy(self.inner);
-        }
-    }
+pub struct BlockBasedOptions {
+    inner: *mut DBBlockBasedTableOptions,
 }
 
 impl Drop for BlockBasedOptions {
@@ -50,23 +36,14 @@ impl Drop for BlockBasedOptions {
     }
 }
 
-impl Drop for WriteOptions {
-    fn drop(&mut self) {
-        unsafe {
-            rocksdb_ffi::rocksdb_writeoptions_destroy(self.inner);
-        }
-    }
-}
-
 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 block based options".to_string());
+        unsafe {
+            let block_opts = rocksdb_ffi::rocksdb_block_based_options_create();
+            assert!(!block_opts.is_null(),
+                    "Could not create rocksdb block based options");
+            BlockBasedOptions { inner: block_opts }
         }
-        BlockBasedOptions { inner: block_opts }
     }
 }
 
@@ -114,38 +91,143 @@ 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);
-// /    }
-// /}
+/// The UnsafeSnap must be destroyed by db, it maybe be leaked
+/// if not using it properly, hence named as unsafe.
+///
+/// This object is convenient for wrapping snapshot by yourself. In most
+/// cases, using `Snapshot` is enough.
+pub struct UnsafeSnap {
+    inner: rocksdb_ffi::DBSnapshot,
+}
+
+impl UnsafeSnap {
+    pub unsafe fn new(db: rocksdb_ffi::DBInstance) -> UnsafeSnap {
+        UnsafeSnap { inner: rocksdb_ffi::rocksdb_create_snapshot(db) }
+    }
+
+    pub unsafe fn get_inner(&self) -> rocksdb_ffi::DBSnapshot {
+        self.inner
+    }
+}
+
+pub struct ReadOptions {
+    inner: *mut DBReadOptions,
+    upper_bound: Vec<u8>,
+}
+
+impl Drop for ReadOptions {
+    fn drop(&mut self) {
+        unsafe { rocksdb_ffi::rocksdb_readoptions_destroy(self.inner) }
+    }
+}
+
+impl Default for ReadOptions {
+    fn default() -> ReadOptions {
+        unsafe {
+            let opts = rocksdb_ffi::rocksdb_readoptions_create();
+            assert!(!opts.is_null(), "Unable to create rocksdb read options");
+            ReadOptions {
+                inner: opts,
+                upper_bound: vec![],
+            }
+        }
+    }
+}
+
+impl ReadOptions {
+    pub fn new() -> ReadOptions {
+        ReadOptions::default()
+    }
 
+    // TODO add snapshot setting here
+    // TODO add snapshot wrapper structs with proper destructors;
+    // that struct needs an "iterator" impl too.
+    #[allow(dead_code)]
+    pub fn fill_cache(&mut self, v: bool) {
+        unsafe {
+            rocksdb_ffi::rocksdb_readoptions_set_fill_cache(self.inner, v);
+        }
+    }
+
+    pub unsafe fn set_snapshot(&mut self, snapshot: &UnsafeSnap) {
+        rocksdb_ffi::rocksdb_readoptions_set_snapshot(self.inner,
+                                                      snapshot.inner);
+    }
+
+    pub fn set_iterate_upper_bound(&mut self, key: &[u8]) {
+        self.upper_bound = Vec::from(key);
+        unsafe {
+            rocksdb_ffi::rocksdb_readoptions_set_iterate_upper_bound(self.inner,
+                                                                     self.upper_bound.as_ptr(),
+                                                                     self.upper_bound.len() as size_t);
+        }
+    }
+
+    pub unsafe fn get_inner(&self) -> *const DBReadOptions {
+        self.inner
+    }
+}
+
+pub struct WriteOptions {
+    pub inner: *mut DBWriteOptions,
+}
+
+impl Drop for WriteOptions {
+    fn drop(&mut self) {
+        unsafe {
+            rocksdb_ffi::rocksdb_writeoptions_destroy(self.inner);
+        }
+    }
+}
+
+impl Default for WriteOptions {
+    fn default() -> WriteOptions {
+        let write_opts = unsafe { rocksdb_ffi::rocksdb_writeoptions_create() };
+        assert!(!write_opts.is_null(),
+                "Could not create rocksdb write options");
+        WriteOptions { inner: write_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);
+        }
+    }
+
+    pub fn disable_wal(&mut self, disable: bool) {
+        unsafe {
+            if disable {
+                rocksdb_ffi::rocksdb_writeoptions_disable_WAL(self.inner, 1);
+            } else {
+                rocksdb_ffi::rocksdb_writeoptions_disable_WAL(self.inner, 0);
+            }
+        }
+    }
+}
+
+pub struct Options {
+    pub inner: *mut DBOptions,
+}
+
+impl Drop for Options {
+    fn drop(&mut self) {
+        unsafe {
+            rocksdb_ffi::rocksdb_options_destroy(self.inner);
+        }
+    }
+}
 
 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());
-            }
+            assert!(!opts.is_null(), "Could not create rocksdb options");
             Options { inner: opts }
         }
     }
@@ -320,7 +402,8 @@ impl Options {
 
     pub fn set_max_manifest_file_size(&mut self, size: u64) {
         unsafe {
-            rocksdb_ffi::rocksdb_options_set_max_manifest_file_size(self.inner, size);
+            rocksdb_ffi::rocksdb_options_set_max_manifest_file_size(self.inner,
+                                                                    size);
         }
     }
 
@@ -418,40 +501,8 @@ impl Options {
 
     pub fn set_wal_recovery_mode(&mut self, mode: DBRecoveryMode) {
         unsafe {
-            rocksdb_ffi::rocksdb_options_set_wal_recovery_mode(self.inner, mode);
-        }
-    }
-}
-
-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() {
-            panic!("Could not create rocksdb write options".to_string());
-        }
-        WriteOptions { inner: write_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);
-        }
-    }
-
-    pub fn disable_wal(&mut self, disable: bool) {
-        unsafe {
-            if disable {
-                rocksdb_ffi::rocksdb_writeoptions_disable_WAL(self.inner, 1);
-            } else {
-                rocksdb_ffi::rocksdb_writeoptions_disable_WAL(self.inner, 0);
-            }
+            rocksdb_ffi::rocksdb_options_set_wal_recovery_mode(self.inner,
+                                                               mode);
         }
     }
 }