Make DBPath use tempfile. (#394)

Cargo.toml

@@ -29,3 +29,4 @@ librocksdb-sys = { path = "librocksdb-sys", version = "6.4.6" }
 
 [dev-dependencies]
 trybuild = "1.0.21"
+tempfile = "3.1.0"

tests/test_backup.rs

@@ -12,6 +12,9 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+mod util;
+use crate::util::DBPath;
+
 use rocksdb::{
     backup::{BackupEngine, BackupEngineOptions, RestoreOptions},
     Options, DB,
@@ -20,12 +23,12 @@ use rocksdb::{
 #[test]
 fn backup_restore() {
     // create backup
-    let path = "_rust_rocksdb_backup_test";
-    let restore_path = "_rust_rocksdb_restore_from_backup_path";
+    let path = DBPath::new("backup_test");
+    let restore_path = DBPath::new("restore_from_backup_path");
     let mut opts = Options::default();
     opts.create_if_missing(true);
     {
-        let db = DB::open(&opts, path).unwrap();
+        let db = DB::open(&opts, &path).unwrap();
         assert!(db.put(b"k1", b"v1111").is_ok());
         let value = db.get(b"k1");
         assert_eq!(value.unwrap().unwrap(), b"v1111");
@@ -44,11 +47,9 @@ fn backup_restore() {
             );
             assert!(restore_status.is_ok());
 
-            let db_restore = DB::open_default(restore_path).unwrap();
+            let db_restore = DB::open_default(&restore_path).unwrap();
             let value = db_restore.get(b"k1");
             assert_eq!(value.unwrap().unwrap(), b"v1111");
         }
     }
-    assert!(DB::destroy(&opts, restore_path).is_ok());
-    assert!(DB::destroy(&opts, path).is_ok());
 }

tests/test_iterator.rs

@@ -253,7 +253,7 @@ fn test_prefix_iterator_uses_full_prefix() {
 
 #[test]
 fn test_full_iterator() {
-    let path = "_rust_rocksdb_fulliteratortest";
+    let path = DBPath::new("fulliteratortest");
     {
         let a1: Box<[u8]> = key(b"aaa1");
         let a2: Box<[u8]> = key(b"aaa2");
@@ -273,7 +273,7 @@ fn test_full_iterator() {
         opts.set_allow_concurrent_memtable_write(false);
         opts.set_memtable_factory(factory);
 
-        let db = DB::open(&opts, path).unwrap();
+        let db = DB::open(&opts, &path).unwrap();
 
         assert!(db.put(&*a1, &*a1).is_ok());
         assert!(db.put(&*a2, &*a2).is_ok());
@@ -295,8 +295,6 @@ fn test_full_iterator() {
         let a_iterator = db.full_iterator(IteratorMode::Start);
         assert_eq!(a_iterator.collect::<Vec<_>>(), expected)
     }
-    let opts = Options::default();
-    assert!(DB::destroy(&opts, path).is_ok());
 }
 
 fn custom_iter<'a>(db: &'a DB) -> impl Iterator<Item = usize> + 'a {
@@ -313,8 +311,6 @@ fn test_custom_iterator() {
         let db = DB::open(&opts, &path).unwrap();
         let _data = custom_iter(&db).collect::<Vec<usize>>();
     }
-    let opts = Options::default();
-    assert!(DB::destroy(&opts, path).is_ok());
 }
 
 #[test]

tests/test_rocksdb_options.rs

@@ -69,7 +69,7 @@ fn test_block_based_options() {
         let _db = DB::open(&opts, &n).unwrap();
 
         // read the setting from the LOG file
-        let mut rocksdb_log = fs::File::open(format!("{}/LOG", n.as_ref().to_str().unwrap()))
+        let mut rocksdb_log = fs::File::open(format!("{}/LOG", (&n).as_ref().to_str().unwrap()))
             .expect("rocksdb creates a LOG file");
         let mut settings = String::new();
         rocksdb_log.read_to_string(&mut settings).unwrap();

tests/util/mod.rs

@@ -1,39 +1,40 @@
 use std::path::{Path, PathBuf};
-use std::time::{SystemTime, UNIX_EPOCH};
+use tempfile;
 
 use rocksdb::{Options, DB};
 
-/// Ensures that DB::Destroy is called for this database when DBPath is dropped.
+/// Temporary database path which calls DB::Destroy when DBPath is dropped.
 pub struct DBPath {
+    #[allow(dead_code)]
+    dir: tempfile::TempDir, // kept for cleaning up during drop
     path: PathBuf,
 }
 
 impl DBPath {
-    /// Suffixes the given `prefix` with a timestamp to ensure that subsequent test runs don't reuse
-    /// an old database in case of panics prior to Drop being called.
+    /// Produces a fresh (non-existent) temporary path which will be DB::destroy'ed automatically.
     pub fn new(prefix: &str) -> DBPath {
-        let current_time = SystemTime::now().duration_since(UNIX_EPOCH).unwrap();
-        let path = format!(
-            "{}.{}.{}",
-            prefix,
-            current_time.as_secs(),
-            current_time.subsec_nanos()
-        );
+        let dir = tempfile::Builder::new()
+            .prefix(prefix)
+            .tempdir()
+            .expect("Failed to create temporary path for db.");
+        let path = dir.path().join("db");
 
-        DBPath {
-            path: PathBuf::from(path),
-        }
+        DBPath { dir, path }
     }
 }
 
 impl Drop for DBPath {
     fn drop(&mut self) {
         let opts = Options::default();
-        DB::destroy(&opts, &self.path).unwrap();
+        DB::destroy(&opts, &self.path).expect("Failed to destroy temporary DB");
     }
 }
 
-impl AsRef<Path> for DBPath {
+/// Convert a DBPath ref to a Path ref.
+/// We don't implement this for DBPath values because we want them to
+/// exist until the end of their scope, not get passed in to functions and
+/// dropped early.
+impl AsRef<Path> for &DBPath {
     fn as_ref(&self) -> &Path {
         &self.path
     }