Handle NO_LOCK when opening environments

Signed-off-by: Victor Porof <victor.porof@gmail.com>

src/backend/impl_lmdb/environment.rs

@@ -101,12 +101,22 @@ impl<'b> BackendEnvironmentBuilder<'b> for EnvironmentBuilderImpl {
     }
 
     fn open(&self, path: &Path) -> Result<Self::Environment, Self::Error> {
-        if !path.is_dir() {
-            if !self.make_dir {
-                return Err(ErrorImpl::DirectoryDoesNotExistError(path.into()));
-            }
-            fs::create_dir_all(path).map_err(ErrorImpl::IoError)?;
+        match self.env_path_type {
+            EnvironmentPathType::NoSubDir => {
+                if !path.is_file() {
+                    return Err(ErrorImpl::UnsuitableEnvironmentPath(path.into()));
+                }
+            },
+            EnvironmentPathType::SubDir => {
+                if !path.is_dir() {
+                    if !self.make_dir {
+                        return Err(ErrorImpl::UnsuitableEnvironmentPath(path.into()));
+                    }
+                    fs::create_dir_all(path)?;
+                }
+            },
         }
+
         self.builder.open(path).map_err(ErrorImpl::LmdbError).and_then(|lmdbenv| {
             EnvironmentImpl::new(path, self.env_path_type, self.env_lock_type, self.env_db_type, lmdbenv)
         })

src/backend/impl_lmdb/error.rs

@@ -22,7 +22,7 @@ use crate::{
 #[derive(Debug)]
 pub enum ErrorImpl {
     LmdbError(lmdb::Error),
-    DirectoryDoesNotExistError(PathBuf),
+    UnsuitableEnvironmentPath(PathBuf),
     IoError(io::Error),
 }
 
@@ -32,7 +32,7 @@ impl fmt::Display for ErrorImpl {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
         match self {
             ErrorImpl::LmdbError(e) => e.fmt(fmt),
-            ErrorImpl::DirectoryDoesNotExistError(_) => write!(fmt, "DirectoryDoesNotExistError"),
+            ErrorImpl::UnsuitableEnvironmentPath(_) => write!(fmt, "UnsuitableEnvironmentPath"),
             ErrorImpl::IoError(e) => e.fmt(fmt),
         }
     }
@@ -49,8 +49,14 @@ impl Into<StoreError> for ErrorImpl {
             ErrorImpl::LmdbError(lmdb::Error::DbsFull) => StoreError::DbsFull,
             ErrorImpl::LmdbError(lmdb::Error::ReadersFull) => StoreError::ReadersFull,
             ErrorImpl::LmdbError(error) => StoreError::LmdbError(error),
-            ErrorImpl::DirectoryDoesNotExistError(path) => StoreError::DirectoryDoesNotExistError(path),
+            ErrorImpl::UnsuitableEnvironmentPath(path) => StoreError::UnsuitableEnvironmentPath(path),
             ErrorImpl::IoError(error) => StoreError::IoError(error),
         }
     }
 }
+
+impl From<io::Error> for ErrorImpl {
+    fn from(e: io::Error) -> ErrorImpl {
+        ErrorImpl::IoError(e)
+    }
+}

src/backend/impl_safe/environment.rs

@@ -101,9 +101,11 @@ impl<'b> BackendEnvironmentBuilder<'b> for EnvironmentBuilderImpl {
     }
 
     fn open(&self, path: &Path) -> Result<Self::Environment, Self::Error> {
+        // Technically NO_SUB_DIR should change these checks here, but they're both currently
+        // unimplemented with this storage backend.
         if !path.is_dir() {
             if !self.make_dir {
-                return Err(ErrorImpl::DirectoryDoesNotExistError(path.into()));
+                return Err(ErrorImpl::UnsuitableEnvironmentPath(path.into()));
             }
             fs::create_dir_all(path)?;
         }

src/backend/impl_safe/error.rs

@@ -29,7 +29,7 @@ pub enum ErrorImpl {
     DbsIllegalOpen,
     DbNotFoundError,
     DbIsForeignError,
-    DirectoryDoesNotExistError(PathBuf),
+    UnsuitableEnvironmentPath(PathBuf),
     IoError(io::Error),
     BincodeError(BincodeError),
 }
@@ -45,7 +45,7 @@ impl fmt::Display for ErrorImpl {
             ErrorImpl::DbsIllegalOpen => write!(fmt, "DbIllegalOpen (safe mode)"),
             ErrorImpl::DbNotFoundError => write!(fmt, "DbNotFoundError (safe mode)"),
             ErrorImpl::DbIsForeignError => write!(fmt, "DbIsForeignError (safe mode)"),
-            ErrorImpl::DirectoryDoesNotExistError(_) => write!(fmt, "DirectoryDoesNotExistError (safe mode)"),
+            ErrorImpl::UnsuitableEnvironmentPath(_) => write!(fmt, "UnsuitableEnvironmentPath (safe mode)"),
             ErrorImpl::IoError(e) => e.fmt(fmt),
             ErrorImpl::BincodeError(e) => e.fmt(fmt),
         }
@@ -62,7 +62,7 @@ impl Into<StoreError> for ErrorImpl {
             ErrorImpl::KeyValuePairNotFound => StoreError::KeyValuePairNotFound,
             ErrorImpl::BincodeError(_) => StoreError::FileInvalid,
             ErrorImpl::DbsFull => StoreError::DbsFull,
-            ErrorImpl::DirectoryDoesNotExistError(path) => StoreError::DirectoryDoesNotExistError(path),
+            ErrorImpl::UnsuitableEnvironmentPath(path) => StoreError::UnsuitableEnvironmentPath(path),
             ErrorImpl::IoError(error) => StoreError::IoError(error),
             _ => StoreError::SafeModeError(self),
         }

src/error.rs

@@ -84,8 +84,8 @@ pub enum StoreError {
     #[fail(display = "I/O error: {:?}", _0)]
     IoError(io::Error),
 
-    #[fail(display = "directory does not exist or not a directory: {:?}", _0)]
-    DirectoryDoesNotExistError(PathBuf),
+    #[fail(display = "environment path does not exist or not the right type: {:?}", _0)]
+    UnsuitableEnvironmentPath(PathBuf),
 
     #[fail(display = "data error: {:?}", _0)]
     DataError(DataError),

src/helpers.rs

@@ -39,8 +39,8 @@ where
     let canonical = path.into().canonicalize()?;
 
     Ok(if cfg!(target_os = "windows") {
-        let url = Url::from_file_path(&canonical).map_err(|_| io::Error::new(io::ErrorKind::Other, "passing error"))?;
-        url.to_file_path().map_err(|_| io::Error::new(io::ErrorKind::Other, "path canonicalization error"))?
+        let map_err = |_| io::Error::new(io::ErrorKind::Other, "path canonicalization error");
+        Url::from_file_path(&canonical).and_then(|url| url.to_file_path()).map_err(map_err)?
     } else {
         canonical
     })

tests/env-lmdb.rs

@@ -72,7 +72,7 @@ fn test_open_fails() {
 
     let pb = nope.to_path_buf();
     match Rkv::new::<Lmdb>(nope.as_path()).err() {
-        Some(StoreError::DirectoryDoesNotExistError(p)) => {
+        Some(StoreError::UnsuitableEnvironmentPath(p)) => {
             assert_eq!(pb, p);
         },
         _ => panic!("expected error"),
@@ -104,6 +104,61 @@ fn test_open_from_builder() {
     check_rkv(&k);
 }
 
+#[test]
+fn test_open_from_builder_with_no_subdir_1() {
+    let root = Builder::new().prefix("test_open_from_builder").tempdir().expect("tempdir");
+    println!("Root path: {:?}", root.path());
+    fs::create_dir_all(root.path()).expect("dir created");
+    assert!(root.path().is_dir());
+
+    {
+        let mut builder = Rkv::environment_builder::<Lmdb>();
+        builder.set_max_dbs(2);
+
+        let k = Rkv::from_builder(root.path(), builder).expect("rkv");
+        check_rkv(&k);
+    }
+    {
+        let mut builder = Rkv::environment_builder::<Lmdb>();
+        builder.set_flags(EnvironmentFlags::NO_SUB_DIR);
+        builder.set_max_dbs(2);
+
+        let mut datamdb = root.path().to_path_buf();
+        datamdb.push("data.mdb");
+
+        let k = Rkv::from_builder(&datamdb, builder).expect("rkv");
+        check_rkv(&k);
+    }
+}
+
+#[test]
+#[should_panic(expected = "rkv: UnsuitableEnvironmentPath")]
+fn test_open_from_builder_with_no_subdir_2() {
+    let root = Builder::new().prefix("test_open_from_builder").tempdir().expect("tempdir");
+    println!("Root path: {:?}", root.path());
+    fs::create_dir_all(root.path()).expect("dir created");
+    assert!(root.path().is_dir());
+
+    {
+        let mut builder = Rkv::environment_builder::<Lmdb>();
+        builder.set_max_dbs(2);
+
+        let k = Rkv::from_builder(root.path(), builder).expect("rkv");
+        check_rkv(&k);
+    }
+    {
+        let mut builder = Rkv::environment_builder::<Lmdb>();
+        builder.set_flags(EnvironmentFlags::NO_SUB_DIR);
+        builder.set_max_dbs(2);
+
+        let mut datamdb = root.path().to_path_buf();
+        datamdb.push("bogus.mdb");
+
+        let k = Rkv::from_builder(&datamdb, builder).expect("rkv");
+        check_rkv(&k);
+    }
+}
+
 #[test]
 fn test_open_from_builder_with_dir_1() {
     let root = Builder::new().prefix("test_open_from_builder").tempdir().expect("tempdir");
@@ -118,7 +173,7 @@ fn test_open_from_builder_with_dir_1() {
 }
 
 #[test]
-#[should_panic(expected = "rkv: DirectoryDoesNotExistError(\"bogus\")")]
+#[should_panic(expected = "rkv: UnsuitableEnvironmentPath(\"bogus\")")]
 fn test_open_from_builder_with_dir_2() {
     let root = Path::new("bogus");
     println!("Root path: {:?}", root);

tests/env-safe.rs

@@ -66,7 +66,7 @@ fn test_open_fails_safe() {
 
     let pb = nope.to_path_buf();
     match Rkv::new::<SafeMode>(nope.as_path()).err() {
-        Some(StoreError::DirectoryDoesNotExistError(p)) => {
+        Some(StoreError::UnsuitableEnvironmentPath(p)) => {
             assert_eq!(pb, p);
         },
         _ => panic!("expected error"),
@@ -112,7 +112,7 @@ fn test_open_from_builder_with_dir_safe_1() {
 }
 
 #[test]
-#[should_panic(expected = "rkv: DirectoryDoesNotExistError(\"bogus\")")]
+#[should_panic(expected = "rkv: UnsuitableEnvironmentPath(\"bogus\")")]
 fn test_open_from_builder_with_dir_safe_2() {
     let root = Path::new("bogus");
     println!("Root path: {:?}", root);