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();