Improve collision handling and add collision test.

src/lib.rs

@@ -139,9 +139,9 @@ pub enum ErrorKind {
     /// This kind of error happens when a trash item's original parent already contains an item with
     /// the same name and type (file or folder). In this case an error is produced and the
     /// restoration of the files is halted meaning that there may be files that could be restored
-    /// but left in the trash due to the error.
+    /// but were left in the trash due to the error.
     ///
-    /// `path`: The path of the file's blocking the trash item from being restored.
+    /// `path`: The path of the file that's blocking the trash item from being restored.
     /// `remaining_items`: All items that were not restored in the order they were provided,
     /// starting with the item that triggered the error.
     RestoreCollision { path: PathBuf, remaining_items: Vec<TrashItem> },
@@ -203,6 +203,8 @@ impl Hash for TrashItem {
 }
 
 /// Returns all `TrashItem`s that are currently in the trash.
+///
+/// The items are in no particular order and must be sorted when any kind of ordering is required.
 pub fn list() -> Result<Vec<TrashItem>, Error> {
     platform::list()
 }
@@ -223,7 +225,7 @@ where
 ///
 /// It may be the case that when restoring a file or a folder, the `original_path` already has
 /// a new item with the same name. When such a collision happens this function returns a
-///
+/// `RestoreCollision` kind of Error.
 pub fn restore_all<I>(items: I) -> Result<(), Error>
 where
     I: IntoIterator<Item = TrashItem>,

src/linux.rs

@@ -264,7 +264,38 @@ where
         // TODO add option to forcefully replace any target at the restore location
         // if it already exists.
         let original_path = item.original_path();
-        if original_path.exists() {
+        // Make sure the parent exists so that `create_dir` doesn't faile due to that.
+        std::fs::create_dir_all(&item.original_parent).map_err(|e| {
+            Error::new(ErrorKind::Filesystem { path: item.original_parent.clone() }, Box::new(e))
+        })?;
+        let mut collision = false;
+        if file.is_dir() {
+            // NOTE create_dir_all succeeds when the path already exist but create_dir
+            // fails with `std::io::ErrorKind::AlreadyExists`.
+            if let Err(e) = std::fs::create_dir(&original_path) {
+                if e.kind() == std::io::ErrorKind::AlreadyExists {
+                    collision = true;
+                } else {
+                    return Err(Error::new(
+                        ErrorKind::Filesystem { path: original_path.clone() },
+                        Box::new(e),
+                    ));
+                }
+            }
+        } else {
+            // File or symlink
+            if let Err(e) = OpenOptions::new().create_new(true).write(true).open(&original_path) {
+                if e.kind() == std::io::ErrorKind::AlreadyExists {
+                    collision = true;
+                } else {
+                    return Err(Error::new(
+                        ErrorKind::Filesystem { path: original_path.clone() },
+                        Box::new(e),
+                    ));
+                }
+            }
+        }
+        if collision {
             let remaining: Vec<_> = std::iter::once(item).chain(iter).collect();
             return Err(Error::kind_only(ErrorKind::RestoreCollision {
                 path: original_path,
@@ -386,7 +417,6 @@ fn move_to_trash(
                 let now = chrono::Local::now();
                 writeln!(file, "[Trash Info]")
                     .and_then(|_| {
-                        // TODO The path has to be relative to the topdir.
                         let absolute_uri = encode_uri_path(src);
                         let topdir_uri = encode_uri_path(topdir);
                         let relative_untrimmed = absolute_uri
@@ -554,7 +584,7 @@ fn home_trash() -> Result<PathBuf, Error> {
         }
     }
 
-    panic!("TODO add error kind for when the home trash is not found.");
+    panic!("Neither the XDG_DATA_HOME nor the HOME environment variable was found");
 }
 
 struct MountPoint {

src/tests.rs

@@ -169,3 +169,55 @@ fn restore() {
         std::fs::remove_file(path).unwrap();
     }
 }
+
+#[test]
+fn restore_collision() {
+    let file_name_prefix = get_unique_name();
+    let file_count: usize = 3;
+    let collision_remaining = file_count - 1;
+    let names: Vec<_> = (0..file_count).map(|i| format!("{}#{}", file_name_prefix, i)).collect();
+    for path in names.iter() {
+        File::create(path).unwrap();
+    }
+    trash::remove_all(&names).unwrap();
+    for path in names.iter().skip(file_count - collision_remaining) {
+        File::create(path).unwrap();
+    }
+    let mut targets: Vec<_> = trash::list()
+        .unwrap()
+        .into_iter()
+        .filter(|x| x.name.starts_with(&file_name_prefix))
+        .collect();
+    targets.sort_by(|a, b| a.name.cmp(&b.name));
+    assert_eq!(targets.len(), file_count);
+    let mut remaining_count = 0;
+    match trash::restore_all(targets) {
+        Err(e) => match e.kind() {
+            trash::ErrorKind::RestoreCollision { remaining_items, .. } => {
+                let iter = names.iter().skip(file_count - collision_remaining).zip(remaining_items);
+                for (original, remaining) in iter {
+                    assert_eq!(original, &remaining.name);
+                    remaining_count += 1;
+                }
+            }
+            _ => panic!("{:?}", e),
+        },
+        Ok(()) => panic!(
+            "restore_all was expected to return `trash::ErrorKind::RestoreCollision` but did not."
+        ),
+    }
+    let remaining = trash::list()
+        .unwrap()
+        .into_iter()
+        .filter(|x| x.name.starts_with(&file_name_prefix))
+        .collect::<Vec<_>>();
+    assert_eq!(remaining.len(), remaining_count);
+    // They are not in the trash anymore but they should be at their original location
+    for path in names.iter().take(file_count - collision_remaining) {
+        assert!(File::open(path).is_ok());
+    }
+    trash::purge_all(remaining).unwrap();
+    for path in names.iter() {
+        std::fs::remove_file(path).unwrap();
+    }
+}