diff --git a/Cargo.lock b/Cargo.lock index 7451994..3b6f90b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5222,10 +5222,22 @@ dependencies = [ "pin-project-lite", "signal-hook-registry", "socket2 0.5.7", + "tokio-macros", "tracing", "windows-sys 0.48.0", ] +[[package]] +name = "tokio-macros" +version = "2.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5b8a1e28f2deaa14e508979454cb3a223b10b938b45af148bc0986de36f1923b" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.60", +] + [[package]] name = "toml" version = "0.5.11" diff --git a/Cargo.toml b/Cargo.toml index b076230..fbc6a5e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -73,6 +73,7 @@ fork = "0.1" fastrand = "2" tempfile = "3" test-log = "0.2" +tokio = { version = "1", features = ["rt", "macros"] } # [patch.'https://github.com/pop-os/libcosmic'] # libcosmic = { path = "../libcosmic" } diff --git a/i18n/en/cosmic_files.ftl b/i18n/en/cosmic_files.ftl index c0c5e26..568344e 100644 --- a/i18n/en/cosmic_files.ftl +++ b/i18n/en/cosmic_files.ftl @@ -58,6 +58,7 @@ operations = Operations pending = Pending failed = Failed complete = Complete +copy_noun = Copy ## Open with open-with = Open with diff --git a/src/app.rs b/src/app.rs index 058358c..f11f94f 100644 --- a/src/app.rs +++ b/src/app.rs @@ -2383,6 +2383,16 @@ pub(crate) mod test_utils { })) } + // Filter `path` for files + pub fn filter_files(path: &Path) -> io::Result> { + Ok(path.read_dir()?.filter_map(|entry| { + entry.ok().and_then(|entry| { + let path = entry.path(); + path.is_file().then_some(path) + }) + })) + } + /// Boiler plate for Tab tests pub fn tab_click_new( files: usize, diff --git a/src/operation.rs b/src/operation.rs index 9b02c9e..166b003 100644 --- a/src/operation.rs +++ b/src/operation.rs @@ -8,7 +8,7 @@ use std::{ }, }; -use crate::app::Message; +use crate::{app::Message, fl}; fn err_str(err: T) -> String { err.to_string() @@ -71,31 +71,22 @@ impl Operation { .into_iter() .zip(std::iter::repeat(to.as_path())) .map(|(from, to)| { - log::info!("{:?}", from.parent()); if matches!(from.parent(), Some(parent) if parent == to) { // `from`'s parent is equal to `to` which means we're copying to the same // directory (duplicating files) let mut to = to.to_owned(); - let to = if let Some(full_name) = - from.file_name().and_then(|name| name.to_str()) - { - // Separate the full file name into its file name plus extension. - let (base_name, ext, needs_dot) = if full_name.starts_with('.') - { - // `[Path::file_name]` returns the full name for dotfiles (e.g. - // .someconf is the file_name) - (full_name, "", false) - } else { - // Consider everything beyond the first '.' to be a file - // extension. - full_name - .split_once('.') - .map(|(full_name, extension)| { - (full_name, extension, !extension.is_empty()) - }) - // File without an extension - .unwrap_or((full_name, "", false)) - }; + // Separate the full file name into its file name plus extension. + // `[Path::file_stem]` returns the full name for dotfiles (e.g. + // .someconf is the file name) + let to = if let (Some(stem), ext) = ( + // FIXME: Replace `[Path::file_stem]` with `[Path::file_prefix]` when stablized to handle .tar.gz et al. better + from.file_stem().and_then(|name| name.to_str()), + from.extension() + .and_then(|ext| ext.to_str()) + .unwrap_or_default(), + ) { + // '.' needs to be re-added for paths with extensions. + let dot = ext.is_empty().then_some("").unwrap_or("."); let mut n = 0u32; // Loop until a valid `copy n` variant is found loop { @@ -107,8 +98,8 @@ impl Operation { }; // Rebuild file name - let dot = if needs_dot { "." } else { "" }; - let new_name = format!("{base_name} (Copy {n}){dot}{ext}"); + let new_name = + format!("{stem} ({} {n}){dot}{ext}", fl!("copy_noun")); to = to.join(new_name); if !matches!(to.try_exists(), Ok(true)) { @@ -134,6 +125,7 @@ impl Operation { let msg_tx = msg_tx.clone(); tokio::task::spawn_blocking(move || -> fs_extra::error::Result<()> { log::info!("Copy {:?} to {:?}", paths, to); + //TODO: set options as desired let dir_options = fs_extra::dir::CopyOptions::default().copy_inside(true); let file_options = fs_extra::file::CopyOptions::default(); let copied_bytes = AtomicU64::default(); @@ -154,6 +146,7 @@ impl Operation { .await; }) }; + // Files and directory progress are handled separately let file_handler = |progress: fs_extra::file::TransitProcess| { copied_bytes.fetch_add(progress.copied_bytes, atomic::Ordering::Relaxed); handler(); @@ -164,11 +157,7 @@ impl Operation { //TODO: handle exceptions fs_extra::dir::TransitProcessResult::ContinueOrAbort }; - //TODO: set options as desired for (from, to) in paths.into_iter().zip(to.into_iter()) { - // This is essentially what `[fs_extra::copy_items_with_progress]` does - // except without handling options (e.g. overwrite). We're currently using - // the defaults anyway. if from.is_dir() { fs_extra::copy_items_with_progress( &[from], @@ -324,3 +313,200 @@ impl Operation { Ok(()) } } + +#[cfg(test)] +mod tests { + use std::{fs::File, io, path::PathBuf}; + + use cosmic::iced::futures::channel::mpsc; + use log::{debug, trace}; + use test_log::test; + use tokio::sync; + + use super::Operation; + use crate::{ + app::{ + test_utils::{ + empty_fs, filter_dirs, filter_files, read_dir_sorted, simple_fs, NAME_LEN, + NUM_DIRS, NUM_FILES, NUM_HIDDEN, NUM_NESTED, + }, + Message, + }, + fl, + }; + + // Tests hang with lower values + const BUF_SIZE: usize = 8; + + /// Simple wrapper around `[Operation::Copy]` + pub async fn operation_copy(paths: Vec, to: PathBuf) -> Result<(), String> { + let id = fastrand::u64(0..u64::MAX); + let (tx, mut rx) = mpsc::channel(BUF_SIZE); + Operation::Copy { + paths: paths.clone(), + to: to.clone(), + } + .perform(id, &sync::Mutex::new(tx).into()) + .await?; + + loop { + match rx.try_next() { + Ok(Some(Message::PendingProgress(id, progress))) => { + trace!("({id}) [ {paths:?} => {to:?} ] {progress}% complete)") + } + Ok(None) => break, + Err(e) => panic!("Receiving message from operation should succeed: {e:?}"), + _ => unreachable!("Only `Message::PendingProgress` is sent from operation"), + } + } + + Ok(()) + } + + #[test(tokio::test)] + async fn copy_file_to_same_location() -> io::Result<()> { + let fs = simple_fs(NUM_FILES, 0, 1, 0, NAME_LEN)?; + let path = fs.path(); + + // Get the first file from the first directory + let first_dir = filter_dirs(path)? + .next() + .expect("Should have at least one directory"); + let first_file = filter_files(&first_dir)? + .next() + .expect("Should have at least one file"); + + // Duplicate that file + let base_name = first_file + .file_name() + .and_then(|name| name.to_str()) + .expect("File name exists and is valid"); + debug!( + "Duplicating {} in {}", + first_file.display(), + first_dir.display() + ); + operation_copy(vec![first_file.clone()], first_dir.clone()) + .await + .expect("Copy operation should have succeeded"); + + assert!(first_file.exists(), "Original file should still exist"); + let expected = first_dir.join(format!("{base_name} ({} 1)", fl!("copy_noun"))); + assert!(expected.exists(), "File should have been duplicated"); + + Ok(()) + } + + #[test(tokio::test)] + async fn copy_file_with_extension_to_same_loc() -> io::Result<()> { + let fs = empty_fs()?; + let path = fs.path(); + + let base_name = "foo.txt"; + let base_path = path.join(base_name); + File::create(&base_path)?; + debug!("Duplicating {}", base_path.display()); + operation_copy(vec![base_path.clone()], path.to_owned()) + .await + .expect("Copy operation should have succeeded"); + + assert!(base_path.exists(), "Original file should still exist"); + let expected = path.join(format!("foo ({} 1).txt", fl!("copy_noun"))); + assert!(expected.exists(), "File should have been duplicated"); + + Ok(()) + } + + #[test(tokio::test)] + async fn copy_dir_to_same_location() -> io::Result<()> { + let fs = simple_fs(NUM_FILES, 0, NUM_DIRS, NUM_NESTED, NAME_LEN)?; + let path = fs.path(); + + // First directory path + let first_dir = filter_dirs(path)? + .next() + .expect("Should have at least one directory"); + let base_name = first_dir + .file_name() + .and_then(|name| name.to_str()) + .expect("First directory exists and has a valid name"); + debug!("Duplicating directory {}", first_dir.display()); + operation_copy(vec![first_dir.clone()], path.to_owned()) + .await + .expect("Copy operation should have succeeded"); + + assert!(first_dir.exists(), "Original directory should still exist"); + let expected = path.join(format!("{base_name} ({} 1)", fl!("copy_noun"))); + assert!(expected.exists(), "Directory should have been duplicated"); + + Ok(()) + } + + #[test(tokio::test)] + async fn copying_file_multiple_times_to_same_location() -> io::Result<()> { + let fs = empty_fs()?; + let path = fs.path(); + + let base_name = "cosmic"; + let base_path = path.join(base_name); + File::create(&base_path)?; + + for i in 1..5 { + debug!("Duplicating {}", base_path.display()); + operation_copy(vec![base_path.clone()], path.to_owned()) + .await + .expect("Copy operation should have succeeded"); + assert!(base_path.exists(), "Original file should still exist"); + assert!( + path.join(format!("{base_name} ({} {i})", fl!("copy_noun"))) + .exists(), + "File should have been duplicated (copy #{i})" + ); + } + + Ok(()) + } + + #[test(tokio::test)] + async fn copy_to_diff_dir_doesnt_dupe_files() -> io::Result<()> { + let fs = simple_fs(NUM_FILES, NUM_HIDDEN, NUM_DIRS, NUM_NESTED, NAME_LEN)?; + let path = fs.path(); + + let (first_dir, second_dir) = { + let mut dirs = filter_dirs(path)?; + ( + dirs.next().expect("Should have at least two dirs"), + dirs.next().expect("Should have at least two dirs"), + ) + }; + let first_file = filter_files(&first_dir)? + .next() + .expect("Should have at least one file"); + // Both directories have a file with the same name. + let base_name = first_file + .file_name() + .and_then(|name| name.to_str()) + .expect("File name exists and is valid"); + + debug!( + "Copying {} to {}", + first_file.display(), + second_dir.display() + ); + operation_copy(vec![first_file.clone()], second_dir.clone()) + .await + .expect_err( + "Copy operation should have failed because we're copying to different directories", + ); + assert!( + first_dir.join(base_name).exists(), + "First file should still exist" + ); + assert!( + second_dir.join(base_name).exists(), + "Second file should still exist" + ); + + Ok(()) + } +}