From 0e83157e09d52e6152df91208e41693b62da2d46 Mon Sep 17 00:00:00 2001 From: "sorokin.oleg.edu" Date: Thu, 1 Oct 2026 18:13:23 -0700 Subject: [PATCH] Explain protected source destination collisions during Save As --- crates/oa-desktop/src/app.rs | 24 +++- crates/oa-desktop/src/app/save_as_tests.rs | 143 +++++++++++++++++++++ 2 files changed, 165 insertions(+), 2 deletions(-) diff --git a/crates/oa-desktop/src/app.rs b/crates/oa-desktop/src/app.rs index 7da73652..f8a0d6a6 100644 --- a/crates/oa-desktop/src/app.rs +++ b/crates/oa-desktop/src/app.rs @@ -5605,6 +5605,7 @@ fn write_snapshot_inner( .map_err(|errors| crate::document::DocumentError::Source(errors).to_string())?, ); } + let has_original_destination = previous.is_some(); let previous = previous.filter(|disk| { path.parent() .filter(|p| !p.as_os_str().is_empty()) @@ -5615,14 +5616,33 @@ fn write_snapshot_inner( == Some(&disk.root) && entry == disk.bundle.entry }); + // A missing-file expectation colliding with an existing destination is + // not evidence that an observed dependency changed outside the editor. + // Keep save_project's conflict reporting unchanged for that latter case. + let saving_existing = previous.is_some(); + let destination_error = |error: source_files::SourceFileError| match error { + source_files::SourceFileError::Conflict(paths) => { + let guidance = if saving_existing { + "Review the new included file's destination before saving." + } else if has_original_destination { + "Choose a fresh directory for Save As, or use Save All to update the open project's current entry." + } else { + "Choose a fresh directory for Save As." + }; + format!( + "Source destination files already exist: {paths:?}. Existing files are protected. {guidance}" + ) + } + other => other.to_string(), + }; let mut disk = match previous { Some(disk) => disk, - None => source_files::new_project(&path, LIMITS).map_err(|e| e.to_string())?, + None => source_files::new_project(&path, LIMITS).map_err(&destination_error)?, }; for path in snapshot.bundle.files.keys() { if !disk.expectations.contains_key(path) { source_files::expect_new_file(&mut disk, path, LIMITS) - .map_err(|e| e.to_string())?; + .map_err(&destination_error)?; } } Some(source_files::save_project(&disk, &snapshot, LIMITS).map_err(|e| e.to_string())?) diff --git a/crates/oa-desktop/src/app/save_as_tests.rs b/crates/oa-desktop/src/app/save_as_tests.rs index c6ddab7d..e61c3416 100644 --- a/crates/oa-desktop/src/app/save_as_tests.rs +++ b/crates/oa-desktop/src/app/save_as_tests.rs @@ -1,5 +1,148 @@ use super::*; +fn saved_multifile_app(directory: &std::path::Path) -> (App, Id, PathBuf) { + let mut project = Project::empty(); + let activity = project + .add_element(Kind::Activity, Layer::Oa, "Assess incident", None) + .unwrap(); + let mut app = App::with_project(project); + let path = directory.join("project.arcadia"); + complete_save(&mut app, path.clone()); + assert!(app.document.bundle().files.len() > 1); + (app, activity, path) +} + +fn rejected_save_preserves_editor(app: &mut App, path: PathBuf) -> String { + let snapshot = app.document.snapshot(); + let revision = app.document.revision(); + let undo = app.document.can_undo(); + let redo = app.document.can_redo(); + let draft = app.source_workspace.text().to_owned(); + let original_path = app.path.clone(); + let dirty = app.dirty(); + let result = write_snapshot( + app.canvas_document, + revision, + snapshot.clone(), + path, + app.disk_project.clone(), + ); + let error = match result { + Err(error) => error, + Ok(_) => panic!("Expected collision refusal"), + }; + let _ = app.update(Message::Saved(Err(error.clone()))); + assert!(Arc::ptr_eq(&snapshot, &app.document.snapshot())); + assert_eq!(app.document.revision(), revision); + assert_eq!(app.document.can_undo(), undo); + assert_eq!(app.document.can_redo(), redo); + assert_eq!(app.source_workspace.text(), draft); + assert_eq!(app.path, original_path); + assert_eq!(app.dirty(), dirty); + error.message +} + +#[test] +fn same_root_new_entry_collision_preserves_files_draft_and_history() { + let directory = tempfile::tempdir().unwrap(); + let (mut app, activity, original) = saved_multifile_app(directory.path()); + let bytes: Vec<_> = app + .document + .bundle() + .files + .keys() + .map(|p| (p.clone(), std::fs::read(directory.path().join(p)).unwrap())) + .collect(); + app.execute(Command::Rename(activity, "Review incident".into())); + app.execute(Command::Rename(activity, "Evaluate incident".into())); + let _ = app.update(Message::Undo); + assert!(app.document.can_undo() && app.document.can_redo()); + let cursor = app.source_workspace.files[&app.source_workspace.entry].cursor; + app.source_workspace.edit( + format!("{}\n// retained draft", app.source_workspace.text()), + cursor, + ); + let destination = directory.path().join("renamed.arcadia"); + let error = rejected_save_preserves_editor(&mut app, destination.clone()); + assert!(error.contains("already exist") && error.contains("Save All")); + assert!(!error.contains("changed outside")); + assert!(!destination.exists()); + assert_eq!(app.path, Some(original)); + for (path, expected) in bytes { + assert_eq!( + std::fs::read(directory.path().join(path)).unwrap(), + expected + ); + } +} + +#[test] +fn fresh_root_existing_include_is_protected_before_any_bundle_write() { + let original = tempfile::tempdir().unwrap(); + let destination = tempfile::tempdir().unwrap(); + let (mut app, activity, _) = saved_multifile_app(original.path()); + app.execute(Command::Rename(activity, "Review incident".into())); + let fragment = app.document.compiled().index.declarations[&activity] + .path + .clone(); + let collision = destination.path().join(&fragment); + std::fs::create_dir_all(collision.parent().unwrap()).unwrap(); + std::fs::write(&collision, b"// independent destination work").unwrap(); + let target = destination.path().join("copy.arcadia"); + let error = rejected_save_preserves_editor(&mut app, target.clone()); + assert!(error.contains(&fragment) && error.contains("fresh directory")); + assert!(!error.contains("changed outside")); + assert!(!target.exists()); + for path in app.document.bundle().files.keys() { + let file = destination.path().join(path); + if path == &fragment { + assert_eq!( + std::fs::read(file).unwrap(), + b"// independent destination work" + ); + } else { + assert!(!file.exists(), "Unexpected write: {path}"); + } + } +} + +#[test] +fn observed_dependency_change_remains_external_conflict_and_normal_saves_work() { + let original = tempfile::tempdir().unwrap(); + let fresh = tempfile::tempdir().unwrap(); + let (mut app, activity, path) = saved_multifile_app(original.path()); + app.execute(Command::Rename(activity, "Review incident".into())); + complete_save(&mut app, path.clone()); + assert!(!app.dirty()); + let fragment = app.document.compiled().index.declarations[&activity] + .path + .clone(); + let entry_before = std::fs::read(&path).unwrap(); + let original_fragment = std::fs::read(original.path().join(&fragment)).unwrap(); + std::fs::write(original.path().join(&fragment), b"// external edit").unwrap(); + app.execute(Command::Rename(activity, "Evaluate incident".into())); + let error = rejected_save_preserves_editor(&mut app, path.clone()); + assert!(error.contains("changed outside") && error.contains(&fragment)); + assert!(!error.contains("fresh directory")); + assert_eq!(std::fs::read(&path).unwrap(), entry_before); + assert_eq!( + std::fs::read(original.path().join(&fragment)).unwrap(), + b"// external edit" + ); + // Restore the observed bytes, then exercise the legitimate existing Save All. + std::fs::write(original.path().join(&fragment), original_fragment).unwrap(); + complete_save(&mut app, path); + assert!(!app.dirty()); + let model = app.project().clone(); + let target = fresh.path().join("copy.arcadia"); + complete_save(&mut app, target.clone()); + let saved = source_files::load_project(&target, LIMITS).unwrap(); + let compiled = source_project::compile_bundle(&saved.bundle, LIMITS).unwrap(); + assert_eq!(compiled.decoded.project, model); + assert_eq!(&saved.bundle, app.document.bundle()); + assert!(!app.dirty()); +} + fn complete_save(app: &mut App, path: PathBuf) { let receipt = write_snapshot( app.canvas_document,