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

@@ -277,10 +277,21 @@ fn template_name(p: &Value, id: &str) -> Result<String> {
     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<PrintTemplate>) -> 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<Value> {
     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<Value> {
             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<Value> {
 fn template_delete(s: &mut Session, p: &Value) -> Result<Value> {
     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<Value> {
     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);

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<Option<Vec<u8>>> {
+        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::<Vec<_>>();
+    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();