fix(VFS): prevent Path::create_dir_all() failures to when executing in parallel (fixes #47)

Author: manuel-woelker

src/error.rs

@@ -115,6 +115,12 @@ pub enum VfsErrorKind {
     /// Generic error variant
     Other(String),
 
+    /// There is already a directory at the given path
+    DirectoryExists,
+
+    /// There is already a file at the given path
+    FileExists,
+
     /// Functionality not supported by this filesystem
     NotSupported,
 }
@@ -137,6 +143,12 @@ impl fmt::Display for VfsErrorKind {
             VfsErrorKind::NotSupported => {
                 write!(f, "Functionality not supported by this filesystem")
             }
+            VfsErrorKind::DirectoryExists => {
+                write!(f, "Directory already exists")
+            }
+            VfsErrorKind::FileExists => {
+                write!(f, "File already exists")
+            }
         }
     }
 }

src/impls/memory.rs

@@ -5,6 +5,7 @@ use crate::VfsResult;
 use crate::{FileSystem, VfsFileType};
 use crate::{SeekAndRead, VfsMetadata};
 use core::cmp;
+use std::collections::hash_map::Entry;
 use std::collections::HashMap;
 use std::fmt;
 use std::fmt::{Debug, Formatter};
@@ -149,13 +150,25 @@ impl FileSystem for MemoryFS {
 
     fn create_dir(&self, path: &str) -> VfsResult<()> {
         self.ensure_has_parent(path)?;
-        self.handle.write().unwrap().files.insert(
-            path.to_string(),
-            MemoryFile {
-                file_type: VfsFileType::Directory,
-                content: Default::default(),
-            },
-        );
+        let map = &mut self.handle.write().unwrap().files;
+        let entry = map.entry(path.to_string());
+        match entry {
+            Entry::Occupied(file) => {
+                return match file.get().file_type {
+                    VfsFileType::File => Err(VfsErrorKind::FileExists.into()),
+                    VfsFileType::Directory => Err(VfsErrorKind::DirectoryExists.into()),
+                }
+            }
+            Entry::Vacant(_) => {
+                map.insert(
+                    path.to_string(),
+                    MemoryFile {
+                        file_type: VfsFileType::Directory,
+                        content: Default::default(),
+                    },
+                );
+            }
+        }
         Ok(())
     }
 

src/impls/physical.rs

@@ -1,11 +1,11 @@
 //! A "physical" file system implementation using the underlying OS file system
 
 use crate::error::VfsErrorKind;
-use crate::VfsResult;
 use crate::{FileSystem, VfsMetadata};
 use crate::{SeekAndRead, VfsFileType};
+use crate::{VfsError, VfsResult};
 use std::fs::{File, OpenOptions};
-use std::io::Write;
+use std::io::{ErrorKind, Write};
 use std::path::{Path, PathBuf};
 
 /// A physical filesystem implementation using the underlying OS file system
@@ -41,7 +41,17 @@ impl FileSystem for PhysicalFS {
     }
 
     fn create_dir(&self, path: &str) -> VfsResult<()> {
-        std::fs::create_dir(self.get_path(path))?;
+        let fs_path = self.get_path(path);
+        std::fs::create_dir(&fs_path).map_err(|err| match err.kind() {
+            ErrorKind::AlreadyExists => {
+                let metadata = std::fs::metadata(&fs_path).unwrap();
+                if metadata.is_dir() {
+                    return VfsError::from(VfsErrorKind::DirectoryExists);
+                }
+                VfsError::from(VfsErrorKind::FileExists)
+            }
+            _ => err.into(),
+        })?;
         Ok(())
     }
 

src/path.rs

@@ -174,7 +174,10 @@ impl VfsPath {
 
     /// Creates the directory at this path
     ///
-    /// Note that the parent directory must exist.
+    /// Note that the parent directory must exist, while the given path must not exist.
+    ///
+    /// Returns VfsErrorKind::FileExists if a file already exists at the given path
+    /// Returns VfsErrorKind::DirectoryExists if a directory already exists at the given path
     ///
     /// ```
     /// # use vfs::{MemoryFS, VfsError, VfsFileType, VfsPath};
@@ -225,8 +228,15 @@ impl VfsPath {
                 .map(|it| it + pos)
                 .unwrap_or_else(|| path.len());
             let directory = &path[..end];
-            if !self.fs.fs.exists(directory)? {
-                self.fs.fs.create_dir(directory)?;
+            if let Err(error) = self.fs.fs.create_dir(directory) {
+                match error.kind() {
+                    VfsErrorKind::DirectoryExists => {}
+                    _ => {
+                        return Err(error.with_path(directory).with_context(|| {
+                            format!("Could not create directories at '{}'", path)
+                        }))
+                    }
+                }
             }
             if end == path.len() {
                 break;

src/test_macros.rs

@@ -109,6 +109,34 @@ macro_rules! test_vfs {
                 let metadata = path.metadata().unwrap();
                 assert_eq!(metadata.file_type, VfsFileType::Directory);
                 assert_eq!(metadata.len, 0);
+                path.create_dir_all().unwrap();
+                root.create_dir_all().unwrap();
+                Ok(())
+            }
+
+            #[test]
+            fn create_dir_all_should_fail_for_existing_file() -> VfsResult<()>{
+                let root = create_root();
+                let _string = String::new();
+                let path = root.join("foo").unwrap();
+                let path2 = root.join("foo/bar").unwrap();
+                path.create_file().unwrap();
+                let result = path2.create_dir_all();
+                match result {
+                    Ok(_) => {panic!("Expected error");}
+                    Err(err) => {
+                        let error_message = format!("{}", err);
+                        if let VfsErrorKind::FileExists = err.kind() {
+
+                        } else {
+                            panic!("Expected file exists error")
+                        }
+                        assert!(
+                            error_message.eq("Could not create directories at '/foo/bar' for '/foo': File already exists"),
+                            "Actual message: {}",
+                            error_message);
+                    }
+                }
                 Ok(())
             }