From 61d9a1c3e4ebfa93788a33436f1f67e27e019077 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 3 Nov 2026 11:53:51 +0000 Subject: [PATCH] fix(engine): recovery writes and reads an image entry shared by several keys once --- crates/engine/src/cmd/recovery.rs | 18 ++++++++++++------ crates/engine/src/tests/recovery.rs | 25 +++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/crates/engine/src/cmd/recovery.rs b/crates/engine/src/cmd/recovery.rs index e36bb56ff..1ad456b94 100644 --- a/crates/engine/src/cmd/recovery.rs +++ b/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 = if images.is_empty() { BTreeSet::new() } else { store.list()?.into_iter().collect() }; + let mut have: BTreeSet = 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, 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>> = 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}")), diff --git a/crates/engine/src/tests/recovery.rs b/crates/engine/src/tests/recovery.rs index 38fbd8674..56c8d899d 100644 --- a/crates/engine/src/tests/recovery.rs +++ b/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());