diff --git a/crates/engine/src/library.rs b/crates/engine/src/library.rs index c98ef4617..6233737f2 100644 --- a/crates/engine/src/library.rs +++ b/crates/engine/src/library.rs @@ -272,6 +272,32 @@ impl SettingsLoad { } impl Session { + /// Check, without closing the library open now, that [`Session::open_library`] can be expected + /// to open `dir`: not a Lightroom catalog, not some other file, a readable folder (or one not + /// made yet), not locked by another program. Returns the library folder ([`resolve_dir`]). + /// Nothing is created; a library folder that exists gets its lock taken and released again. + /// Opening can still fail afterwards (a damaged catalog, a disk going away). + pub fn check_library(&self, dir: impl AsRef) -> Result { + let dir = dir.as_ref().to_path_buf(); + if let Some(place) = crate::lightroom_folder::lightroom_catalog_at(&dir) { + return Err(EngineError::LightroomCatalog { path: dir, catalog: place.catalog }); + } + let dir = resolve_dir(&dir)?; + if !dir.exists() { + return Ok(dir); + } + if !dir.is_dir() { + return Err(EngineError::Other(format!("{} is not a folder", dir.display()))); + } + std::fs::read_dir(&dir).map_err(|e| EngineError::Other(format!("can't open library {}: {e}", dir.display())))?; + // the library open in this session keeps its lock when reopened; any other must be free + let current = self.library.as_ref().is_some_and(|l| l.on_disk && same_dir(&l.dir, &dir)); + if !current && is_library(&dir) { + drop(LibraryLock::acquire(&dir, &program_name()).map_err(|e| EngineError::LibraryInUse(e.to_string()))?); + } + Ok(dir) + } + /// Open (or create) the library at `dir` into this session, replacing its catalog. With /// `seed_demo`, a newly created library starts with the procedural demo photos. /// @@ -721,4 +747,32 @@ mod resolve_tests { assert!(!e.contains("File exists"), "{e}"); let _ = std::fs::remove_dir_all(&dir); } + + /// `check_library` refuses what `open_library` would, without touching the open library. + #[test] + fn check_library_refuses_locked_libraries_files_and_lightroom_catalogs() { + let dir = std::env::temp_dir().join(format!("lc-check-lib-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + let (a, b) = (dir.join("a"), dir.join("b")); + std::fs::create_dir_all(&b).unwrap(); + let mut s = Session::new().with_fs(); + s.open_library(&b, false).unwrap(); + s.close_library().unwrap(); + s.open_library(&a, false).unwrap(); + assert_eq!(s.check_library(&a).unwrap(), a, "the open library keeps its lock"); + assert_eq!(s.check_library(&b).unwrap(), b); + assert_eq!(s.check_library(dir.join("new")).unwrap(), dir.join("new")); + assert!(!dir.join("new").exists(), "nothing created"); + let held = LibraryLock::acquire(&b, "other").unwrap(); + assert!(matches!(s.check_library(&b), Err(EngineError::LibraryInUse(_)))); + drop(held); + assert!(s.check_library(&b).is_ok(), "the probe released its lock"); + std::fs::write(dir.join("notes.txt"), b"x").unwrap(); + assert!(s.check_library(dir.join("notes.txt")).is_err()); + std::fs::write(dir.join("Cat.lrcat"), b"x").unwrap(); + assert!(matches!(s.check_library(dir.join("Cat.lrcat")), Err(EngineError::LightroomCatalog { .. }))); + assert!(s.library.as_ref().is_some_and(|l| l.dir == a)); + drop(s); + let _ = std::fs::remove_dir_all(&dir); + } } diff --git a/crates/ui-egui/src/panels/settings.rs b/crates/ui-egui/src/panels/settings.rs index a668439c2..fefe8ef55 100644 --- a/crates/ui-egui/src/panels/settings.rs +++ b/crates/ui-egui/src/panels/settings.rs @@ -673,17 +673,27 @@ pub fn open_library(app: &mut LightcraftApp, p: &Value) -> Result } /// Close the current library and open the one at `path`. If it can't be opened (locked by -/// another program, unreadable, a file), the error is returned and the current library stays open. +/// another program, unreadable, a file), the error is returned and the current library stays open: +/// the target is checked before anything is closed, and if opening it still fails the previous +/// library is opened again (both errors are reported if that fails too). fn switch_library(app: &mut LightcraftApp, path: &str) -> Result { - let dir = lightcraft_engine::library::resolve_dir(std::path::Path::new(path)).map_err(|e| e.to_string())?; + let dir = app.session.check_library(std::path::Path::new(path)).map_err(|e| format!("{path}: {e}"))?; let path = dir.to_string_lossy().into_owned(); // the library being left goes in the list too (also when it was opened at launch) - let previous = app.session.library.as_ref().filter(|l| l.on_disk).map(|l| l.dir.to_string_lossy().into_owned()); + let previous_dir = app.session.library.as_ref().filter(|l| l.on_disk).map(|l| l.dir.clone()); + let previous = previous_dir.as_ref().map(|d| d.to_string_lossy().into_owned()); app.session.close_library().map_err(|e| e.to_string())?; - app.session.open_library(&dir, false).map_err(|e| format!("{path}: {e}"))?; + let opened = app.session.open_library(&dir, false).map(|_| ()).map_err(|e| format!("{path}: {e}")); app.lightroom_last = None; app.renderer.forget_all(); app.ui.compare = None; + if let Err(err) = opened { + let Some(prev) = previous_dir else { return Err(err) }; + return match app.session.open_library(&prev, false) { + Ok(_) => Err(err), + Err(e) => Err(format!("{err}; reopening the previous library {} failed too: {e}", prev.display())), + }; + } app.ui.settings.library_path = path.clone(); if let Some(prev) = previous.filter(|p| p != &path && !app.ui.settings.recent_libraries.contains(p)) { remember_library(&mut app.ui.settings.recent_libraries, &prev); @@ -768,10 +778,19 @@ pub fn new_library(app: &mut LightcraftApp, p: &Value) -> Result switch_library(app, &dir.to_string_lossy()) } +/// Characters a folder name can't hold on Windows (and `/`, which no system allows). +const RESERVED_NAME_CHARS: [char; 9] = ['<', '>', ':', '"', '/', '\\', '|', '?', '*']; + fn valid_library_name(s: &str) -> Result<&str, String> { let n = s.trim(); - if n.is_empty() || n == "." || n == ".." || n.contains(['/', '\\']) || n.chars().any(char::is_control) { - return Err(format!("app.newLibrary: {s:?} can't be a library name (a folder name, without / or \\)")); + // Windows drops trailing dots and spaces from a folder name: refuse them rather than + // creating a folder whose name differs from the one asked for (spaces around the name + // are trimmed first) + let bad = n.is_empty() || n == "." || n == ".." || n.ends_with(['.', ' ']) || n.contains(RESERVED_NAME_CHARS) || n.chars().any(char::is_control); + if bad { + return Err(format!( + "app.newLibrary: {s:?} can't be a library name (a folder name without < > : \" / \\ | ? * and not ending in a dot or space)" + )); } Ok(n) } diff --git a/crates/ui-egui/src/tests_switch_library.rs b/crates/ui-egui/src/tests_switch_library.rs index d45b4ea14..8c5208e3d 100644 --- a/crates/ui-egui/src/tests_switch_library.rs +++ b/crates/ui-egui/src/tests_switch_library.rs @@ -162,3 +162,41 @@ fn missing_recent_libraries_are_greyed_out_and_pruned_and_the_list_is_capped() { assert!(app.ui.settings.recent_libraries.is_empty()); let _ = std::fs::remove_dir_all(&dir); } + +/// A library another program has open is refused before the current one is closed: the current +/// library stays open with its photos, and the window title keeps its name. +#[test] +fn switching_to_a_locked_library_keeps_the_current_one_open() { + let dir = temp("lockedlib"); + let mut app = app_in(&dir); + app.run("app.newLibrary", json!({"path": dir.to_string_lossy(), "name": "Other"})).unwrap(); + app.run("app.openLibrary", json!({"path": dir.join("first").to_string_lossy()})).unwrap(); + // "another program" holds Other's lock + let other = dir.join("Other"); + let held = lightcraft_catalog::LibraryLock::acquire(&other, "someone else").unwrap(); + let e = app.run("app.openLibrary", json!({"path": other.to_string_lossy()})).unwrap_err(); + assert!(e.contains("already open"), "{e}"); + assert!(app.session.library.as_ref().unwrap().dir.ends_with("first"), "the current library stays"); + assert!(app.session.library.as_ref().unwrap().on_disk); + assert!(app.ui.settings.library_path.ends_with("first")); + drop(held); + app.run("app.openLibrary", json!({"path": other.to_string_lossy()})).unwrap(); + assert!(app.session.library.as_ref().unwrap().dir.ends_with("Other")); + let _ = std::fs::remove_dir_all(&dir); +} + +/// Library names that can't be folder names everywhere (Windows' reserved characters, a +/// trailing dot) are refused; spaces around a name are trimmed. +#[test] +fn library_names_must_work_as_folder_names_on_every_system() { + let dir = temp("libnames"); + let mut app = app_in(&dir); + for bad in ["ab", "a:b", "a\"b", "a/b", "a\\b", "a|b", "a?b", "a*b", "name.", "..", " ", "a\u{7}"] { + let e = app.run("app.newLibrary", json!({"path": dir.to_string_lossy(), "name": bad})).unwrap_err(); + assert!(e.contains("library name"), "{bad:?}: {e}"); + } + assert!(app.session.library.as_ref().unwrap().dir.ends_with("first")); + app.run("app.newLibrary", json!({"path": dir.to_string_lossy(), "name": " Trip 2026 "})).unwrap(); + assert_eq!(app.session.library.as_ref().unwrap().dir, dir.join("Trip 2026")); + let _ = std::fs::remove_dir_all(&dir); +}