diff --git a/crates/engine/src/cmd/recovery.rs b/crates/engine/src/cmd/recovery.rs index f466355c..ce4ce7a5 100644 --- a/crates/engine/src/cmd/recovery.rs +++ b/crates/engine/src/cmd/recovery.rs @@ -654,8 +654,20 @@ impl RecoveryJob { image_bytes += data.len(); } } + let meta_name = format!("{}{META_EXT}", self.file); + // The details name the images, so a crash between writing the document and its details + // would pair the new document with the old copy's images: when the new document has + // images the old details don't name, they name both until the new document is in. + let old = read_meta(store, &self.file).images; + if meta.images.iter().any(|(k, r)| old.get(k).is_none_or(|o| o.blob != r.blob)) { + let mut both = meta.clone(); + for (k, r) in old { + both.images.entry(k).or_insert(r); + } + store.write(&meta_name, &serde_json::to_vec_pretty(&both).map_err(|e| e.to_string())?)?; + } store.write(&format!("{}{DOC_EXT}", self.file), bytes)?; - store.write(&format!("{}{META_EXT}", self.file), &meta_bytes)?; + store.write(&meta_name, &meta_bytes)?; // Images this copy no longer shows, nor any other. sweep(store, area); Ok(json!({ "file": self.file, "title": self.title, "bytes": bytes.len(), "imageBytes": image_bytes })) diff --git a/crates/engine/src/tests_recovery.rs b/crates/engine/src/tests_recovery.rs index cf46b91d..37f5ae81 100644 --- a/crates/engine/src/tests_recovery.rs +++ b/crates/engine/src/tests_recovery.rs @@ -574,3 +574,63 @@ fn copies_from_before_images_were_kept_apart_still_restore() { assert_eq!(images_of(&s), images); assert_eq!(s.doc().unwrap().title(), "Untitled-1 [Recovered]"); } + +/// A store whose app dies right after writing a copy's document entry: later writes fail, so the +/// copy's details stay as the last interval left them. +struct DiesAfterDocument { + inner: Arc, + dead: std::sync::atomic::AtomicBool, +} + +impl RecoveryStore for DiesAfterDocument { + fn list(&self) -> std::result::Result, String> { + self.inner.list() + } + fn read(&self, name: &str) -> std::result::Result, String> { + self.inner.read(name) + } + fn write(&self, name: &str, bytes: &[u8]) -> std::result::Result<(), String> { + if self.dead.load(std::sync::atomic::Ordering::Relaxed) { + return Err("the app is gone".into()); + } + self.inner.write(name, bytes)?; + if name.ends_with(".vectorcraft") { + self.dead.store(true, std::sync::atomic::Ordering::Relaxed); + } + Ok(()) + } + fn remove(&self, name: &str) -> std::result::Result<(), String> { + self.inner.remove(name) + } + fn location(&self) -> String { + self.inner.location() + } + fn lock(&self, area: &str) -> std::result::Result { + self.inner.lock(area) + } + fn now(&self) -> Option { + self.inner.now() + } +} + +/// A crash between writing a copy's document and its details restores the new document with +/// every image, the one placed since the last copy too (and the first copy likewise). +#[test] +fn a_crash_between_a_copys_document_and_its_details_keeps_every_image() { + for first in [true, false] { + let store = Arc::new(MemoryStore::default()); + let mut s = session_in(&store); + if !first { + place_png(&mut s, "red.png", crate::tests_links::RED); + s.execute("file.recovery.save", &json!({})).unwrap(); + } + place_png(&mut s, "blue.png", crate::tests_links::BLUE); + let images = images_of(&s); + s.recovery.set_store(Arc::new(DiesAfterDocument { inner: store.clone(), dead: Default::default() })); + assert!(s.execute("file.recovery.save", &json!({})).is_err(), "the details weren't written"); + drop(s); + let mut s = relaunch(&store); + assert_eq!(restored(s.execute("file.recovery.restore", &json!({})).unwrap()), 1, "first copy: {first}"); + assert_eq!(images_of(&s), images, "first copy: {first}"); + } +}