#1244: a crash between a recovery copy's document and its details keeps every image

The details name the copy's images, so the new document written before them paired with the old
details lost the images placed since the last copy (and a first copy lost all of them). Until the
new document is in, the details name the old and the new images.

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 }))

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<MemoryStore>,
+    dead: std::sync::atomic::AtomicBool,
+}
+
+impl RecoveryStore for DiesAfterDocument {
+    fn list(&self) -> std::result::Result<Vec<String>, String> {
+        self.inner.list()
+    }
+    fn read(&self, name: &str) -> std::result::Result<Vec<u8>, 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<crate::cmd::recovery::Lock, String> {
+        self.inner.lock(area)
+    }
+    fn now(&self) -> Option<i64> {
+        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}");
+    }
+}