From 1c60abb5860e6ba7cee95f324940d7d59aec3e40 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sat, 24 Oct 2026 02:42:21 -0700 Subject: [PATCH] LRC-PRINT-TEMPLATES: template changes that can't be saved are undone print.template.save, delete and import changed the template list before saving the preferences, and left the change in place when the save failed. A retried import then saw its own unsaved copy and stored another one as "Name (2)". The list now goes back to what it was when the save fails, as library.holdView and the slide templates do. --- crates/engine/src/cmd/print.rs | 22 +++++++++++--- crates/engine/src/tests_print.rs | 52 ++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 5 deletions(-) diff --git a/crates/engine/src/cmd/print.rs b/crates/engine/src/cmd/print.rs index 541773273..1390ca167 100644 --- a/crates/engine/src/cmd/print.rs +++ b/crates/engine/src/cmd/print.rs @@ -277,10 +277,21 @@ fn template_name(p: &Value, id: &str) -> Result { clean_name(p.get("name").and_then(Value::as_str).ok_or_else(|| bad(id, "missing `name`"))?).map_err(|e| bad(id, e)) } +/// Save the preferences after a change to the templates; if that fails, the templates go back to +/// `before`, so the list matches what is saved and a retry does the same thing. +fn save_templates(s: &mut Session, before: Vec) -> Result<()> { + if let Err(e) = s.save_prefs() { + s.print_templates = before; + return Err(e); + } + Ok(()) +} + fn template_save(s: &mut Session, p: &Value) -> Result { const ID: &str = "print.template.save"; let name = template_name(p, ID)?; let t = PrintTemplate::new(&name, &s.print).map_err(|e| bad(ID, e))?; + let before = s.print_templates.clone(); match s.print_templates.iter_mut().find(|x| x.name.eq_ignore_ascii_case(&name)) { Some(x) => *x = t, None => { @@ -290,7 +301,7 @@ fn template_save(s: &mut Session, p: &Value) -> Result { s.print_templates.push(t); } } - s.save_prefs()?; + save_templates(s, before)?; Ok(template_list(s)) } @@ -309,12 +320,12 @@ fn template_apply(s: &mut Session, p: &Value) -> Result { fn template_delete(s: &mut Session, p: &Value) -> Result { const ID: &str = "print.template.delete"; let name = template_name(p, ID)?; - let before = s.print_templates.len(); + let before = s.print_templates.clone(); s.print_templates.retain(|x| !x.name.eq_ignore_ascii_case(&name)); - if s.print_templates.len() == before { + if s.print_templates.len() == before.len() { return Err(bad(ID, format!("no print template `{name}`"))); } - s.save_prefs()?; + save_templates(s, before)?; Ok(template_list(s)) } @@ -360,8 +371,9 @@ fn template_import(s: &mut Session, p: &Value) -> Result { let name = unique_name(&t.name, &s.print_templates); let renamed = name != t.name; t.name = name.clone(); + let before = s.print_templates.clone(); s.print_templates.push(t); - s.save_prefs()?; + save_templates(s, before)?; let mut out = template_list(s); out["name"] = json!(name); out["renamed"] = json!(renamed); diff --git a/crates/engine/src/tests_print.rs b/crates/engine/src/tests_print.rs index ca6ea486c..3e9aae671 100644 --- a/crates/engine/src/tests_print.rs +++ b/crates/engine/src/tests_print.rs @@ -365,6 +365,58 @@ fn templates_export_to_a_file_and_import_from_one() { let _ = std::fs::remove_dir_all(dir); } +/// A library whose `prefs.json` can't be read, so saving the preferences fails. +#[derive(Clone, Default)] +struct PrefsUnreadable(MemStore); + +impl lightcraft_catalog::Store for PrefsUnreadable { + fn read(&mut self, name: &str) -> std::io::Result>> { + if name == "prefs.json" { + return Err(std::io::Error::other("locked by another process")); + } + self.0.read(name) + } + fn write_atomic(&mut self, name: &str, data: &[u8]) -> std::io::Result<()> { + self.0.write_atomic(name, data) + } + fn append(&mut self, name: &str, data: &[u8]) -> std::io::Result<()> { + self.0.append(name, data) + } + fn truncate(&mut self, name: &str, len: u64) -> std::io::Result<()> { + self.0.truncate(name, len) + } + fn describe(&self) -> String { + "prefs unreadable".into() + } +} + +#[test] +fn template_changes_that_cant_be_saved_are_undone() { + let mut s = Session::with_demo(); + let stores = + LibraryStores { dir: "print".into(), catalog: Box::new(MemStore::new()), files: Box::new(PrefsUnreadable::default()), on_disk: false }; + s.open_library_in(stores, false).unwrap(); + s.print_templates.push(crate::print::template::PrintTemplate::new("Proofs", &s.print).unwrap()); + let names = |s: &Session| s.print_templates.iter().map(|t| (t.name.clone(), t.settings.clone())).collect::>(); + let kept = names(&s); + let dir = std::env::temp_dir().join(format!("lightcraft-print-unsaved-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let file = dir.join("proofs.lcprint"); + run(&mut s, "print.template.export", json!({"name": "Proofs", "path": file.to_string_lossy()})); + + run(&mut s, "print.set", json!({"layout": {"style": "contactSheet"}})); + for (id, p) in [ + ("print.template.save", json!({"name": "Other"})), + ("print.template.save", json!({"name": "proofs"})), + ("print.template.delete", json!({"name": "Proofs"})), + ("print.template.import", json!({"path": file.to_string_lossy()})), + ] { + assert!(fails(&mut s, id, p.clone()).contains("prefs.json"), "{id} {p}"); + assert_eq!(names(&s), kept, "{id} {p}"); + } + let _ = std::fs::remove_dir_all(dir); +} + #[test] fn a_hostile_template_in_the_preferences_does_not_break_the_library() { let files = MemStore::new();