Issue #2589: a content duplicate starts from the file, not from the other photo's record

The new record for a catalog photo whose bytes match another library photo copied that whole
record and reset a few fields, so the other photo's edits, history, versions, ratings and written
metadata (title, keywords, face regions) carried over, from a trashed photo too. It now starts as a
new photo and takes only what the bytes determine: format, size, kind, capture time, camera EXIF,
GPS, focus point, content hash, embedded lens data and preview-only state. The catalog's own
settings and metadata apply on top as before.

Tests: the duplicate test gives the other photo edits, history, a version, a title, keywords and a
Local baseline and checks none reach the new record; a reimport after a master and its virtual copy
were trashed counts each once.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Chad Elliott
2026-10-29 23:56:50 -05:00
co-authored by Claude Opus 5.5
parent de9b0a8951
commit 7c74ae0ac3
2 changed files with 96 additions and 13 deletions
+32 -12
View File
@@ -1000,19 +1000,39 @@ pub(crate) fn apply_prepared(s: &mut crate::Session, mut data: CatalogImport, co
// a new copy of a master in Recently Deleted is imported, not trashed with it
p.deleted = false;
p
} else if let Some(duplicate) = duplicates.get(&key(&src.path)).and_then(|id| s.catalog.photo(*id)) {
} else if let Some(duplicate) = duplicates.get(&key(&src.path)).and_then(|id| s.catalog.photo(*id)).cloned() {
// Catalogs can contain distinct originals with identical bytes and independent edits.
// The clone is a new record of its own: never in Recently Deleted because the photo with
// the same bytes is (#2589), nor a virtual copy because that photo is one.
let mut p = (**duplicate).clone();
p.id = s.catalog.alloc_photo_id();
p.source = Source::File { path: src.path.clone() };
p.deleted = false;
p.copy_of = None;
p.copy_name = None;
p.local_baseline = None;
p.imported = now.clone();
p.file_name = Path::new(&src.path).file_name().map(|n| n.to_string_lossy().into()).unwrap_or_else(|| src.path.clone());
// The new record takes only what the bytes determine from the photo with the same
// bytes, never that photo's own state: not its Recently Deleted (#2589), virtual copy,
// edits, history, versions, ratings or the metadata a person writes.
let name = Path::new(&src.path).file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_else(|| src.path.clone());
let mut p = Photo::new(
s.catalog.alloc_photo_id(),
Source::File { path: src.path.clone() },
&name,
&duplicate.format,
duplicate.width,
duplicate.height,
&now,
);
p.kind = duplicate.kind;
p.file_size = duplicate.file_size;
p.captured = duplicate.captured.clone();
p.duration = duplicate.duration;
p.as_shot_wb = duplicate.as_shot_wb;
p.content_hash = duplicate.content_hash.clone();
p.embedded_lens = duplicate.embedded_lens;
p.preview_only = duplicate.preview_only.clone();
p.preview_checked = duplicate.preview_checked.clone();
let m = &duplicate.meta;
p.meta.camera = m.camera.clone();
p.meta.lens = m.lens.clone();
p.meta.focal_mm = m.focal_mm;
p.meta.aperture = m.aperture;
p.meta.shutter = m.shutter.clone();
p.meta.iso = m.iso;
p.meta.gps = m.gps;
p.meta.focus = m.focus.clone();
p
} else {
let id = s.catalog.alloc_photo_id();
+64 -1
View File
@@ -179,7 +179,27 @@ fn a_content_duplicate_of_a_trashed_photo_or_virtual_copy_is_a_new_live_photo()
};
for (case, edit) in [("trashed", trashed), ("virtual copy", virtual_copy)] {
let mut s = crate::Session::new();
let other = library_photo(&mut s, "/elsewhere/a.ARW", edit);
let other = library_photo(&mut s, "/elsewhere/a.ARW", |p| {
edit(p);
// the other photo's own state, which the new record must not inherit
let mut d = (*p.develop).clone();
d.light.shadows = 77.0;
p.develop = std::sync::Arc::new(d);
p.edited = Some("earlier".into());
p.history.push(lightcraft_catalog::HistoryStep { label: "Other photo's edit".into(), settings: p.develop.clone() });
p.versions.push(lightcraft_catalog::Version {
name: "Other photo's version".into(),
created: "before".into(),
settings: p.develop.clone(),
auto: false,
});
p.meta.title = "Other photo's title".into();
p.meta.keywords = vec!["other-photo".into()];
p.local_baseline = Some(7);
// what the bytes determine is shared
p.meta.camera = "Same Camera".into();
p.file_size = 4096;
});
let mut data = sample();
data.photos.retain(|p| number(&p.image, "masterImage") == 0);
let files = crate::import::ImportReport {
@@ -214,6 +234,14 @@ fn a_content_duplicate_of_a_trashed_photo_or_virtual_copy_is_a_new_live_photo()
assert_eq!((new.copy_of, new.copy_name.as_deref()), (None, None), "{case}: the new record is no virtual copy");
assert_ne!(new.imported, "before", "{case}: imported now, not when the other photo was");
assert_eq!(new.rating, 5, "{case}: the catalog's own metadata applies");
assert_ne!(new.develop.light.shadows, 77.0, "{case}: no edits from the other photo");
assert!(new.history.iter().all(|h| h.label != "Other photo's edit"), "{case}: no history from the other photo");
assert!(new.versions.is_empty(), "{case}: no versions from the other photo");
assert_ne!(new.meta.title, "Other photo's title", "{case}");
assert!(!new.meta.keywords.iter().any(|k| k == "other-photo"), "{case}");
assert_eq!(new.local_baseline, None, "{case}");
assert_eq!((new.meta.camera.as_str(), new.file_size), ("Same Camera", 4096), "{case}: what the bytes determine is kept");
assert_eq!(new.content_hash.as_deref(), Some("same-bytes"), "{case}");
assert_eq!(s.catalog.photo(other).unwrap().deleted, case == "trashed", "{case}: the other photo is left as it was");
}
}
@@ -233,6 +261,41 @@ fn a_catalog_photo_matching_a_trashed_record_stays_there_and_is_counted() {
assert!(!copy.deleted, "the new virtual copy is imported, not trashed");
assert_eq!(r["imported"], 1, "only the copy was added: {r}");
}
/// Importing the catalog again after its master and virtual copy were both moved to Recently
/// Deleted matches each through the import index: each is counted once, and neither is restored.
#[test]
fn a_reimport_counts_each_trashed_match_once() {
let mut s = crate::Session::new();
// one import index across both imports, as a library on disk keeps in Interop/
let mut index = ImportIndex::default();
let mut import = |s: &mut crate::Session| {
let undo0 = s.undo.len();
apply_prepared(
s,
sample(),
ApplyPrepared {
update_existing: false,
files: Default::default(),
index: &mut index,
archive_path: None,
missing_sources: &HashSet::new(),
undo0,
},
)
.unwrap()
.report
};
let first = import(&mut s);
let ids: Vec<PhotoId> = ["1", "2"].iter().map(|k| PhotoId(first["mapping"][k].as_u64().unwrap())).collect();
for id in &ids {
s.commit("Delete", Op::SetDeleted { id: *id, deleted: true }).unwrap();
}
let r = import(&mut s);
assert_eq!(r["mapping"], first["mapping"], "{r}");
assert_eq!(r["inRecentlyDeleted"], 2, "{r}");
assert_eq!(r["imported"], 0, "{r}");
assert!(ids.iter().all(|id| s.catalog.photo(*id).unwrap().deleted));
}
#[test]
fn preserves_existing_edits_and_rejects_invalid_structure_before_mutation() {
let mut s = crate::Session::new();