fix(engine): recovery writes and reads an image entry shared by several keys once

crates/engine/src/cmd/recovery.rs

@@ -701,11 +701,12 @@ impl RecoveryJob {
         let meta_bytes = serde_json::to_vec_pretty(&meta).map_err(|e| e.to_string())?;
         let area = self.file.split_once('/').map_or("", |(a, _)| a);
         let _turn = WRITES.lock().unwrap_or_else(|e| e.into_inner());
-        let have: BTreeSet<String> = if images.is_empty() { BTreeSet::new() } else { store.list()?.into_iter().collect() };
+        let mut have: BTreeSet<String> = if images.is_empty() { BTreeSet::new() } else { store.list()?.into_iter().collect() };
         let mut image_bytes = 0;
         for (blob, data) in images {
             let name = format!("{area}/{blob}");
-            if !have.contains(&name) {
+            // Images of several keys can share one entry: it is written once.
+            if have.insert(name.clone()) {
                 store.write(&name, data)?;
                 image_bytes += data.len();
             }
@@ -1009,16 +1010,21 @@ fn restore_one(s: &mut Session, store: &Arc<dyn RecoveryStore>, file: &str) -> R
     super::threads::mark_continued(&mut doc);
     // The images, from the entries of the copy's area.
     let from = file.split_once('/').map_or("", |(a, _)| a);
-    let mut blobs = vec![];
+    let mut blobs: BTreeMap<String, Arc<Vec<u8>>> = BTreeMap::new();
     for (key, r) in &meta.images {
-        match store.read(&format!("{from}/{}", r.blob)) {
+        // Keys sharing an entry share its bytes: each entry is read (and written again) once.
+        let read = match blobs.get(&r.blob) {
+            Some(data) => Ok(data.clone()),
+            None => store.read(&format!("{from}/{}", r.blob)).map(Arc::new),
+        };
+        match read {
             Ok(data) => {
-                let mut blob = ImageBlob::new(r.mime.clone(), data);
+                let mut blob = ImageBlob { mime: r.mime.clone(), bytes: data, proxy: None };
                 // Until the linked file is read, the preview stands in for it.
                 if r.proxy {
                     blob.proxy = Some(blob.bytes.clone());
                 }
-                blobs.push((r.blob.clone(), blob.bytes.clone()));
+                blobs.insert(r.blob.clone(), blob.bytes.clone());
                 doc.images.insert(key.clone(), blob);
             }
             Err(e) => warnings.push(format!("the image “{key}” of the recovered document couldn't be read: {e}")),

crates/engine/src/tests/recovery.rs

@@ -643,6 +643,31 @@ fn images_are_kept_apart_and_written_again_only_when_they_change() {
     assert!(store.images().is_empty(), "{:?}", store.list());
 }
 
+/// Image keys sharing their bytes share one entry: it is written once when saved, and read and
+/// written once when restored.
+#[test]
+fn images_sharing_an_entry_are_written_once() {
+    let store = Arc::new(Counting::default());
+    let mut s = Session::new();
+    s.recovery.set_store(store.clone());
+    s.execute("file.new", &json!({"width": 200, "height": 100})).unwrap();
+    place_png(&mut s, "red.png", crate::tests_links::RED);
+    let doc = Arc::make_mut(&mut s.doc_mut().unwrap().doc);
+    let blob = doc.images.values().next().unwrap().clone();
+    for i in 0..50 {
+        doc.images.insert(format!("copy-{i}"), blob.clone());
+    }
+    s.execute("file.recovery.save", &json!({})).unwrap();
+    assert_eq!(store.images_written().len(), 1);
+    let images = images_of(&s);
+    drop(s);
+    let mut s = Session::new();
+    s.recovery.set_store(store.clone());
+    assert_eq!(restored(s.execute("file.recovery.restore", &json!({})).unwrap()), 1);
+    assert_eq!(images_of(&s), images);
+    assert_eq!(store.images_written().len(), 1, "restored into this session's area once");
+}
+
 #[test]
 fn copies_from_before_images_were_kept_apart_still_restore() {
     let store = Arc::new(MemoryStore::default());