From de9b0a8951f0c5a758b1cf5267f7d0cc12f6f49e Mon Sep 17 00:00:00 2001 From: Chad Elliott Date: Tue, 27 Oct 2026 02:12:28 -0500 Subject: [PATCH 1/6] Issue #2589: a Lightroom import never creates a photo already in Recently Deleted A catalog photo whose file matched a photo in Recently Deleted by content was cloned with the trashed record's `deleted` flag (and its virtual-copy fields), so it landed in Recently Deleted while the report counted it as imported. A new virtual copy of a trashed master did the same. Both are now live records, as Create Virtual Copy already does. Catalog photos matched by path to a trashed photo stay there (the Lightroom import's OnDeleted::Skip, #706); the report counts them as `inRecentlyDeleted` and the result window says so in one line. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/lightroom_catalog.rs | 17 ++++- crates/engine/src/lightroom_catalog/tests.rs | 76 ++++++++++++++++++++ crates/ui-egui/locales/cs-formats.json | 1 + crates/ui-egui/locales/de-formats.json | 1 + crates/ui-egui/locales/es-formats.json | 1 + crates/ui-egui/locales/fi-formats.json | 1 + crates/ui-egui/locales/fr-formats.json | 1 + crates/ui-egui/locales/id-formats.json | 1 + crates/ui-egui/locales/it-formats.json | 1 + crates/ui-egui/locales/ja-formats.json | 1 + crates/ui-egui/locales/ka-formats.json | 1 + crates/ui-egui/locales/pl-formats.json | 1 + crates/ui-egui/locales/pt-br-formats.json | 1 + crates/ui-egui/locales/ru-formats.json | 1 + crates/ui-egui/locales/sv-formats.json | 1 + crates/ui-egui/locales/uk-formats.json | 1 + crates/ui-egui/locales/vi-formats.json | 1 + crates/ui-egui/locales/zh-hans-formats.json | 1 + crates/ui-egui/locales/zh-hant-formats.json | 1 + crates/ui-egui/src/lightroom_import.rs | 19 +++++ 20 files changed, 128 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/lightroom_catalog.rs b/crates/engine/src/lightroom_catalog.rs index 3066187ce..e29a9e71a 100644 --- a/crates/engine/src/lightroom_catalog.rs +++ b/crates/engine/src/lightroom_catalog.rs @@ -956,6 +956,9 @@ pub(crate) fn apply_prepared(s: &mut crate::Session, mut data: CatalogImport, co let mut promoted = HashSet::new(); let mut unmapped = BTreeMap::new(); let mut preserved = 0; + // Library photos in Recently Deleted that a catalog photo matched: they stay there (only an + // import of the files themselves brings a photo back, see `OnDeleted`), and the report says so. + let mut in_trash = HashSet::new(); // Masters precede their virtual copies. let mut photos: Vec<_> = data.photos.iter().collect(); photos.sort_by_key(|p| number(&p.image, "masterImage") != 0); @@ -980,6 +983,9 @@ pub(crate) fn apply_prepared(s: &mut crate::Session, mut data: CatalogImport, co .filter(|id| s.catalog.photo(*id).is_some_and(|p| source_path(p).is_some_and(|path| key(path) == key(&src.path)))); let existing_id = indexed.or_else(|| if master == 0 { by_path.get(&key(&src.path)).copied() } else { None }); let mut p = if let Some(id) = existing_id.and_then(|id| s.catalog.photo(id)) { + if id.deleted { + in_trash.insert(id.id); + } (**id).clone() } else if master != 0 { let master_id = ids.get(&master).copied().ok_or_else(|| error(format!("virtual copy {}: missing master", src.source_id)))?; @@ -991,12 +997,21 @@ pub(crate) fn apply_prepared(s: &mut crate::Session, mut data: CatalogImport, co p.id = s.catalog.alloc_photo_id(); p.copy_of = Some(master_id); p.copy_name = Some(text(&src.image, "copyName").to_string()); + // 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)) { // 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()); p } else { @@ -1105,7 +1120,7 @@ pub(crate) fn apply_prepared(s: &mut crate::Session, mut data: CatalogImport, co } s.merge_undo(s.undo.len().saturating_sub(undo0), "Import Lightroom Catalog"); Ok(AppliedImport { - report: json!({"source":data.source,"photos":ids.len(),"imported":imported,"library":s.library.as_ref().filter(|l| l.on_disk).map(|l| l.dir.to_string_lossy()),"collections":album_count,"preservedExistingEdits":preserved,"missing":missing,"failed":files.failed,"warnings":warnings,"skipped":data.skipped,"unmapped":unmapped,"archive":archive_path,"mapping":ids}), + report: json!({"source":data.source,"photos":ids.len(),"imported":imported,"library":s.library.as_ref().filter(|l| l.on_disk).map(|l| l.dir.to_string_lossy()),"collections":album_count,"preservedExistingEdits":preserved,"inRecentlyDeleted":in_trash.len(),"missing":missing,"failed":files.failed,"warnings":warnings,"skipped":data.skipped,"unmapped":unmapped,"archive":archive_path,"mapping":ids}), index: index.clone(), }) } diff --git a/crates/engine/src/lightroom_catalog/tests.rs b/crates/engine/src/lightroom_catalog/tests.rs index 654b40458..96a6fca4e 100644 --- a/crates/engine/src/lightroom_catalog/tests.rs +++ b/crates/engine/src/lightroom_catalog/tests.rs @@ -157,6 +157,82 @@ fn imports_copies_collections_ratings_and_mapped_edits_in_one_undo_step() { assert!(s.catalog.albums().next().is_none()); assert!(data.photos[1].settings.contains("CameraProfile")); // unmapped source stays intact } +/// A library photo at `path`, already committed. +fn library_photo(s: &mut crate::Session, path: &str, edit: impl FnOnce(&mut Photo)) -> PhotoId { + let id = s.catalog.alloc_photo_id(); + let mut p = Photo::new(id, Source::File { path: path.into() }, "a.ARW", "ARW", 20, 10, "before"); + p.content_hash = Some("same-bytes".into()); + edit(&mut p); + s.commit("Add", Op::AddPhoto { photo: Box::new(p) }).unwrap(); + id +} +/// A catalog photo whose file has the same bytes as a library photo in Recently Deleted (or a +/// virtual copy) is a new record of its own: it was counted as imported but landed in Recently +/// Deleted (or became a "copy" of nothing it was copied from), so a large import into a library +/// with a trash went missing photo by photo (#2589). +#[test] +fn a_content_duplicate_of_a_trashed_photo_or_virtual_copy_is_a_new_live_photo() { + let trashed: fn(&mut Photo) = |p| p.deleted = true; + let virtual_copy: fn(&mut Photo) = |p| { + p.copy_of = Some(PhotoId(999)); + p.copy_name = Some("Copy 1".into()); + }; + 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 mut data = sample(); + data.photos.retain(|p| number(&p.image, "masterImage") == 0); + let files = crate::import::ImportReport { + duplicates: vec![crate::import::Duplicate { + path: "/missing/test.ARW".into(), + existing: Some(other.0), + reason: "content", + existing_deleted: case == "trashed", + }], + ..Default::default() + }; + let undo0 = s.undo.len(); + let r = apply_prepared( + &mut s, + data, + ApplyPrepared { + update_existing: false, + files, + index: &mut ImportIndex::default(), + archive_path: None, + missing_sources: &HashSet::new(), + undo0, + }, + ) + .unwrap() + .report; + assert_eq!(r["imported"], 1, "{case}: {r}"); + assert_eq!(r["inRecentlyDeleted"], 0, "{case}: {r}"); + let new = s.catalog.photo(PhotoId(r["mapping"]["1"].as_u64().unwrap())).unwrap(); + assert_ne!(new.id, other, "{case}"); + assert!(!new.deleted, "{case}: the new record must not be in Recently Deleted"); + 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_eq!(s.catalog.photo(other).unwrap().deleted, case == "trashed", "{case}: the other photo is left as it was"); + } +} +/// A catalog photo matched (by path) to a library photo in Recently Deleted leaves it there, as +/// the import was set up to do (a Lightroom import doesn't restore, see `OnDeleted`), and the report +/// counts it so the photos don't go missing unexplained (#2589). A new virtual copy of it is +/// imported, not trashed with its master. +#[test] +fn a_catalog_photo_matching_a_trashed_record_stays_there_and_is_counted() { + let mut s = crate::Session::new(); + let trashed = library_photo(&mut s, "/missing/test.ARW", |p| p.deleted = true); + let r = apply(&mut s, sample(), false).unwrap(); + assert_eq!(r["mapping"]["1"], trashed.0, "{r}"); + assert!(s.catalog.photo(trashed).unwrap().deleted, "the photo stays in Recently Deleted"); + assert_eq!(r["inRecentlyDeleted"], 1, "{r}"); + let copy = s.catalog.photos().find(|p| p.copy_of == Some(trashed)).unwrap(); + assert!(!copy.deleted, "the new virtual copy is imported, not trashed"); + assert_eq!(r["imported"], 1, "only the copy was added: {r}"); +} #[test] fn preserves_existing_edits_and_rejects_invalid_structure_before_mutation() { let mut s = crate::Session::new(); diff --git a/crates/ui-egui/locales/cs-formats.json b/crates/ui-egui/locales/cs-formats.json index 1eb882f09..c60430c12 100644 --- a/crates/ui-egui/locales/cs-formats.json +++ b/crates/ui-egui/locales/cs-formats.json @@ -267,6 +267,7 @@ "Update “{name}” with Current Settings": "Aktualizovat „{name}“ podle aktuálního nastavení", "{n} preset{}": "Přednastavení: {n}{:.0}", "{count} more warnings.": "Další varování: {count}.", + "{in_trash} catalog photos are in this library's Recently Deleted and stay there; restore them from Recently Deleted to see them.": "Fotografie katalogu v nedávno odstraněných této knihovny: {in_trash}. Zůstávají tam; chcete-li je vidět, obnovte je z nedávno odstraněných.", "{skipped} catalog images were not imported because their records in the catalog are incomplete.": "Neimportované obrázky katalogu: {skipped}. Jejich záznamy v katalogu jsou neúplné.", "{photos} catalog records processed; {collections} collections.": "Zpracováno záznamů katalogu: {photos}; kolekcí: {collections}.", "{missing} originals are missing. Their catalog records are available; use Find Missing Photos to relink them.": "Chybějící originály: {missing}. Jejich záznamy jsou v katalogu dostupné; použijte Najít chybějící fotografie a znovu je propojte.", diff --git a/crates/ui-egui/locales/de-formats.json b/crates/ui-egui/locales/de-formats.json index 79390f136..9aa45bfb6 100644 --- a/crates/ui-egui/locales/de-formats.json +++ b/crates/ui-egui/locales/de-formats.json @@ -269,6 +269,7 @@ "{n} originals opened in the other editor": "{n} Originale im anderen Editor geöffnet", "· {in_source} identical copies": "· {in_source} identische Kopien", "{count} more warnings.": "{count} weitere Warnungen.", + "{in_trash} catalog photos are in this library's Recently Deleted and stay there; restore them from Recently Deleted to see them.": "{in_trash} Katalogfotos liegen in dieser Bibliothek unter „Zuletzt gelöscht“ und bleiben dort; stelle sie dort wieder her, um sie zu sehen.", "{skipped} catalog images were not imported because their records in the catalog are incomplete.": "{skipped} Katalogbilder wurden nicht importiert, weil ihre Einträge im Katalog unvollständig sind.", "{photos} catalog records processed; {collections} collections.": "{photos} Katalogeinträge verarbeitet; {collections} Sammlungen.", "{missing} originals are missing. Their catalog records are available; use Find Missing Photos to relink them.": "{missing} Originale fehlen. Ihre Katalogeinträge sind verfügbar; verwende „Fehlende Fotos suchen“, um sie neu zu verknüpfen.", diff --git a/crates/ui-egui/locales/es-formats.json b/crates/ui-egui/locales/es-formats.json index 33e5f52fc..a81dddb28 100644 --- a/crates/ui-egui/locales/es-formats.json +++ b/crates/ui-egui/locales/es-formats.json @@ -269,6 +269,7 @@ "{n} originals opened in the other editor": "{n} originales abiertos en el otro editor", "· {in_source} identical copies": "· {in_source} copias idénticas", "{count} more warnings.": "{count} advertencias más.", + "{in_trash} catalog photos are in this library's Recently Deleted and stay there; restore them from Recently Deleted to see them.": "{in_trash} fotos del catálogo están en Eliminadas recientemente de esta biblioteca y permanecen allí; restáuralas desde Eliminadas recientemente para verlas.", "{skipped} catalog images were not imported because their records in the catalog are incomplete.": "No se importaron {skipped} imágenes del catálogo porque sus registros en el catálogo están incompletos.", "{photos} catalog records processed; {collections} collections.": "{photos} registros del catálogo procesados; {collections} colecciones.", "{missing} originals are missing. Their catalog records are available; use Find Missing Photos to relink them.": "Faltan {missing} originales. Sus registros del catálogo están disponibles; usa Buscar fotos que faltan para volver a vincularlos.", diff --git a/crates/ui-egui/locales/fi-formats.json b/crates/ui-egui/locales/fi-formats.json index 1daab3dab..3babe2c75 100644 --- a/crates/ui-egui/locales/fi-formats.json +++ b/crates/ui-egui/locales/fi-formats.json @@ -257,6 +257,7 @@ "{pid} (missing)": "{pid} (puuttuu)", "{sel_n} selected · {counted}": "Valittu: {sel_n} · {counted}", "{size} GB": "{size} Gt", + "{in_trash} catalog photos are in this library's Recently Deleted and stay there; restore them from Recently Deleted to see them.": "Katalogin kuvia tämän kirjaston Äskettäin poistetuissa: {in_trash}. Ne jäävät sinne; palauta ne Äskettäin poistetuista nähdäksesi ne.", "{skipped} catalog images were not imported because their records in the catalog are incomplete.": "{skipped} catalog images were not imported because their records in the catalog are incomplete.", "{stage}… {:.0}%": "{stage}… {:.0}%", "{s} s of AV1 video": "{s} s AV1-videota", diff --git a/crates/ui-egui/locales/fr-formats.json b/crates/ui-egui/locales/fr-formats.json index 294273781..537dd7ed1 100644 --- a/crates/ui-egui/locales/fr-formats.json +++ b/crates/ui-egui/locales/fr-formats.json @@ -269,6 +269,7 @@ "{n} originals opened in the other editor": "{n} originaux ouverts dans l’autre éditeur", "· {in_source} identical copies": "· {in_source} copies identiques", "{count} more warnings.": "{count} avertissements supplémentaires.", + "{in_trash} catalog photos are in this library's Recently Deleted and stay there; restore them from Recently Deleted to see them.": "{in_trash} photos du catalogue sont dans « Supprimés récemment » de cette photothèque et y restent ; restaurez-les depuis « Supprimés récemment » pour les voir.", "{skipped} catalog images were not imported because their records in the catalog are incomplete.": "{skipped} images du catalogue n’ont pas été importées, car leurs entrées dans le catalogue sont incomplètes.", "{photos} catalog records processed; {collections} collections.": "{photos} entrées du catalogue traitées ; {collections} collections.", "{missing} originals are missing. Their catalog records are available; use Find Missing Photos to relink them.": "{missing} originaux sont manquants. Leurs entrées de catalogue sont disponibles ; utilisez Rechercher les photos manquantes pour rétablir les liens.", diff --git a/crates/ui-egui/locales/id-formats.json b/crates/ui-egui/locales/id-formats.json index 7fc325522..c596fa594 100644 --- a/crates/ui-egui/locales/id-formats.json +++ b/crates/ui-egui/locales/id-formats.json @@ -296,6 +296,7 @@ "“{to}” is already a person. Combining them makes one person; Undo separates them again.": "“{to}” sudah terdaftar sebagai orang. Jika digabungkan, keduanya menjadi satu orang; Urungkan akan memisahkannya lagi.", "{what} merge added at {mp}: its full size, {full}, needs more memory than this computer has": "Hasil penggabungan {what} ditambahkan dengan resolusi {mp}: resolusi penuhnya ({full}) memerlukan memori melebihi kapasitas komputer ini", "{what} merge added at {mp}, the largest a panorama can be (its full size is {full})": "Hasil penggabungan {what} ditambahkan dengan resolusi {mp}, resolusi maksimum untuk panorama (resolusi penuhnya {full})", + "{in_trash} catalog photos are in this library's Recently Deleted and stay there; restore them from Recently Deleted to see them.": "{in_trash} foto katalog ada di Baru dihapus pada pustaka ini dan tetap di sana; pulihkan dari Baru dihapus untuk melihatnya.", "{skipped} catalog images were not imported because their records in the catalog are incomplete.": "{skipped} catalog images were not imported because their records in the catalog are incomplete.", "Listing folders — folders: {folders}, files: {files}": "Listing folders — folders: {folders}, files: {files}", "Added {n} photo{}": "{n} foto ditambahkan{:.0}", diff --git a/crates/ui-egui/locales/it-formats.json b/crates/ui-egui/locales/it-formats.json index 7efae90e9..d9c6a5efe 100644 --- a/crates/ui-egui/locales/it-formats.json +++ b/crates/ui-egui/locales/it-formats.json @@ -250,6 +250,7 @@ "{s} s of AV1 video": "{s} s di video AV1", "Delete {n} effect preset{}? This can't be undone.": "Eliminare i preset effetto ({n})?{:.0} L'operazione non si può annullare.", "{count} more warnings.": "Altri avvisi: {count}.", + "{in_trash} catalog photos are in this library's Recently Deleted and stay there; restore them from Recently Deleted to see them.": "{in_trash} foto del catalogo sono in Eliminate di recente di questa libreria e vi restano; ripristinale da Eliminate di recente per vederle.", "{skipped} catalog images were not imported because their records in the catalog are incomplete.": "{skipped} immagini del catalogo non sono state importate perché i loro record nel catalogo sono incompleti.", "· {in_source} identical copies": "· copie identiche: {in_source}", "{what} merge added at {mp}, the largest a panorama can be (its full size is {full})": "Unione {what} aggiunta a {mp}, la dimensione massima di un panorama (la dimensione piena è {full})", diff --git a/crates/ui-egui/locales/ja-formats.json b/crates/ui-egui/locales/ja-formats.json index 99c14bf16..91cbbc995 100644 --- a/crates/ui-egui/locales/ja-formats.json +++ b/crates/ui-egui/locales/ja-formats.json @@ -273,6 +273,7 @@ "{n} originals opened in the other editor": "{n}個の元ファイルを別のエディタで開きました", "· {in_source} identical copies": "・同一のコピー {in_source} 件", "{count} more warnings.": "ほかに{count}件の警告があります。", + "{in_trash} catalog photos are in this library's Recently Deleted and stay there; restore them from Recently Deleted to see them.": "カタログの写真 {in_trash} 枚は、このライブラリの「最近削除した写真」にあり、そのまま残ります。表示するには「最近削除した写真」から復元してください。", "{skipped} catalog images were not imported because their records in the catalog are incomplete.": "カタログ内のレコードが不完全なため、{skipped}件の画像を読み込めませんでした。", "{photos} catalog records processed; {collections} collections.": "カタログの{photos}件のレコードを処理しました。コレクションは{collections}件です。", "{missing} originals are missing. Their catalog records are available; use Find Missing Photos to relink them.": "{missing}件のオリジナルが見つかりません。カタログのレコードは利用できます。「見つからない写真を検索」で再リンクしてください。", diff --git a/crates/ui-egui/locales/ka-formats.json b/crates/ui-egui/locales/ka-formats.json index 460368261..c77de0407 100644 --- a/crates/ui-egui/locales/ka-formats.json +++ b/crates/ui-egui/locales/ka-formats.json @@ -279,6 +279,7 @@ "Updated {name}": "Updated {name}", "new, inside {parent}": "new, inside {parent}", "{count} more warnings.": "{count} more warnings.", + "{in_trash} catalog photos are in this library's Recently Deleted and stay there; restore them from Recently Deleted to see them.": "კატალოგის ფოტოები ამ ბიბლიოთეკის ახლახან წაშლილებში: {in_trash}. ისინი იქ რჩება; სანახავად აღადგინეთ ისინი ახლახან წაშლილებიდან.", "{skipped} catalog images were not imported because their records in the catalog are incomplete.": "კატალოგის {skipped} სურათი არ დაიმპორტდა, რადგან მათი ჩანაწერები კატალოგში არასრულია.", "{missing} originals are missing. Their catalog records are available; use Find Missing Photos to relink them.": "{missing} originals are missing. Their catalog records are available; use Find Missing Photos to relink them.", "{n} JPEG files": "{n} JPEG files", diff --git a/crates/ui-egui/locales/pl-formats.json b/crates/ui-egui/locales/pl-formats.json index 20c7f840d..19fb9cdfe 100644 --- a/crates/ui-egui/locales/pl-formats.json +++ b/crates/ui-egui/locales/pl-formats.json @@ -261,6 +261,7 @@ "Update “{name}” with Current Settings": "Zaktualizuj „{name}” bieżącymi ustawieniami", "{n} preset{}": "Ustawienia: {n}{:.0}", "{count} more warnings.": "Pozostałe ostrzeżenia: {count}.", + "{in_trash} catalog photos are in this library's Recently Deleted and stay there; restore them from Recently Deleted to see them.": "Zdjęcia katalogu w Ostatnio usuniętych tej biblioteki: {in_trash}. Pozostają tam; przywróć je z Ostatnio usuniętych, aby je zobaczyć.", "{skipped} catalog images were not imported because their records in the catalog are incomplete.": "Nie zaimportowano obrazów z katalogu: {skipped}. Ich rekordy w katalogu są niekompletne.", "{photos} catalog records processed; {collections} collections.": "Przetworzone rekordy katalogu: {photos}; kolekcje: {collections}.", "{missing} originals are missing. Their catalog records are available; use Find Missing Photos to relink them.": "Brakujące oryginały: {missing}. Ich rekordy w katalogu są dostępne; użyj polecenia Znajdź brakujące zdjęcia, aby je ponownie powiązać.", diff --git a/crates/ui-egui/locales/pt-br-formats.json b/crates/ui-egui/locales/pt-br-formats.json index f17f2c0fd..7da70ee39 100644 --- a/crates/ui-egui/locales/pt-br-formats.json +++ b/crates/ui-egui/locales/pt-br-formats.json @@ -273,6 +273,7 @@ "{n} originals opened in the other editor": "{n} originais abertos no outro editor", "· {in_source} identical copies": "· {in_source} cópias idênticas", "{count} more warnings.": "Mais {count} avisos.", + "{in_trash} catalog photos are in this library's Recently Deleted and stay there; restore them from Recently Deleted to see them.": "{in_trash} fotos do catálogo estão em Excluídos recentemente desta biblioteca e continuam lá; restaure-as de Excluídos recentemente para vê-las.", "{skipped} catalog images were not imported because their records in the catalog are incomplete.": "{skipped} imagens do catálogo não foram importadas porque seus registros no catálogo estão incompletos.", "{photos} catalog records processed; {collections} collections.": "{photos} registros do catálogo processados; {collections} coleções.", "{missing} originals are missing. Their catalog records are available; use Find Missing Photos to relink them.": "{missing} originais estão ausentes. Seus registros no catálogo estão disponíveis; use Encontrar fotos ausentes para vinculá-los novamente.", diff --git a/crates/ui-egui/locales/ru-formats.json b/crates/ui-egui/locales/ru-formats.json index 76b7af427..f84b1f23c 100644 --- a/crates/ui-egui/locales/ru-formats.json +++ b/crates/ui-egui/locales/ru-formats.json @@ -269,6 +269,7 @@ "{n} originals opened in the other editor": "Открыто оригиналов в другом редакторе: {n}", "· {in_source} identical copies": "· идентичных копий: {in_source}", "{count} more warnings.": "Других предупреждений: {count}.", + "{in_trash} catalog photos are in this library's Recently Deleted and stay there; restore them from Recently Deleted to see them.": "Фото каталога в «Недавно удалённых» этой библиотеки: {in_trash}. Они остаются там; чтобы увидеть их, восстановите их из «Недавно удалённых».", "{skipped} catalog images were not imported because their records in the catalog are incomplete.": "Не импортированы изображения каталога: {skipped}. Их записи в каталоге неполные.", "{photos} catalog records processed; {collections} collections.": "Обработано записей каталога: {photos}; коллекций: {collections}.", "{missing} originals are missing. Their catalog records are available; use Find Missing Photos to relink them.": "Не найдены оригиналы: {missing}. Их записи каталога доступны; используйте поиск отсутствующих фотографий, чтобы восстановить ссылки.", diff --git a/crates/ui-egui/locales/sv-formats.json b/crates/ui-egui/locales/sv-formats.json index 9a71a0288..6cd199d12 100644 --- a/crates/ui-egui/locales/sv-formats.json +++ b/crates/ui-egui/locales/sv-formats.json @@ -296,6 +296,7 @@ "Move {n} photo{} to “{}”?": "Move {n} photo{} to “{}”?", "Moved {n} photo{} to “{}”. Edit → Undo moves them back.": "Moved {n} photo{} to “{}”. Edit → Undo moves them back.", "{n} selected photos · {pages} pages": "{n} selected photos · {pages} pages", + "{in_trash} catalog photos are in this library's Recently Deleted and stay there; restore them from Recently Deleted to see them.": "{in_trash} katalogfoton finns i Nyligen raderade i det här biblioteket och ligger kvar där; återställ dem från Nyligen raderade för att se dem.", "{skipped} catalog images were not imported because their records in the catalog are incomplete.": "{skipped} catalog images were not imported because their records in the catalog are incomplete.", "Listing folders — folders: {folders}, files: {files}": "Listing folders — folders: {folders}, files: {files}", "Added {n} photo{}": "{n} foto tillagda{:.0}", diff --git a/crates/ui-egui/locales/uk-formats.json b/crates/ui-egui/locales/uk-formats.json index 36022ac1a..5f08d7a7d 100644 --- a/crates/ui-egui/locales/uk-formats.json +++ b/crates/ui-egui/locales/uk-formats.json @@ -269,6 +269,7 @@ "{n} originals opened in the other editor": "Відкрито оригіналів в іншому редакторі: {n}", "· {in_source} identical copies": "· ідентичних копій: {in_source}", "{count} more warnings.": "Інших попереджень: {count}.", + "{in_trash} catalog photos are in this library's Recently Deleted and stay there; restore them from Recently Deleted to see them.": "Фото каталогу в нещодавно видалених цієї бібліотеки: {in_trash}. Вони залишаються там; щоб побачити їх, відновіть їх із нещодавно видалених.", "{skipped} catalog images were not imported because their records in the catalog are incomplete.": "Не імпортовано зображення каталогу: {skipped}. Їхні записи в каталозі неповні.", "{photos} catalog records processed; {collections} collections.": "Оброблено записів каталогу: {photos}; колекцій: {collections}.", "{missing} originals are missing. Their catalog records are available; use Find Missing Photos to relink them.": "Не знайдено оригінали: {missing}. Їхні записи каталогу доступні; скористайтеся пошуком відсутніх фотографій, щоб відновити посилання.", diff --git a/crates/ui-egui/locales/vi-formats.json b/crates/ui-egui/locales/vi-formats.json index d5e28bff0..f4d482868 100644 --- a/crates/ui-egui/locales/vi-formats.json +++ b/crates/ui-egui/locales/vi-formats.json @@ -257,6 +257,7 @@ "{pid} (missing)": "{pid} (thiếu)", "{sel_n} selected · {counted}": "Đã chọn {sel_n} · {counted}", "{size} GB": "{size} GB", + "{in_trash} catalog photos are in this library's Recently Deleted and stay there; restore them from Recently Deleted to see them.": "{in_trash} ảnh của danh mục nằm trong Đã xóa gần đây của thư viện này và vẫn ở đó; hãy khôi phục chúng từ Đã xóa gần đây để xem.", "{skipped} catalog images were not imported because their records in the catalog are incomplete.": "{skipped} catalog images were not imported because their records in the catalog are incomplete.", "{stage}… {:.0}%": "{stage}… {:.0}%", "{s} s of AV1 video": "{s} s of AV1 video", diff --git a/crates/ui-egui/locales/zh-hans-formats.json b/crates/ui-egui/locales/zh-hans-formats.json index 7406c39b8..9a06a534d 100644 --- a/crates/ui-egui/locales/zh-hans-formats.json +++ b/crates/ui-egui/locales/zh-hans-formats.json @@ -273,6 +273,7 @@ "{n} originals opened in the other editor": "已在其他编辑器中打开 {n} 个原始文件", "· {in_source} identical copies": "· {in_source} 个相同副本", "{count} more warnings.": "另有 {count} 条警告。", + "{in_trash} catalog photos are in this library's Recently Deleted and stay there; restore them from Recently Deleted to see them.": "目录中有 {in_trash} 张照片位于此图库的“最近删除”中,并将保留在那里;如需查看,请从“最近删除”中恢复。", "{skipped} catalog images were not imported because their records in the catalog are incomplete.": "有 {skipped} 张图像未导入,因为它们在目录中的记录不完整。", "{photos} catalog records processed; {collections} collections.": "已处理 {photos} 条目录记录;{collections} 个收藏集。", "{missing} originals are missing. Their catalog records are available; use Find Missing Photos to relink them.": "有 {missing} 个原始文件缺失。其目录记录可用;请使用“查找缺失照片”重新链接文件。", diff --git a/crates/ui-egui/locales/zh-hant-formats.json b/crates/ui-egui/locales/zh-hant-formats.json index d5c4eb279..81b5d3c82 100644 --- a/crates/ui-egui/locales/zh-hant-formats.json +++ b/crates/ui-egui/locales/zh-hant-formats.json @@ -273,6 +273,7 @@ "{n} originals opened in the other editor": "已在其他編輯器中開啟 {n} 個原始檔案", "· {in_source} identical copies": "· {in_source} 個相同副本", "{count} more warnings.": "另有 {count} 則警告。", + "{in_trash} catalog photos are in this library's Recently Deleted and stay there; restore them from Recently Deleted to see them.": "目錄中有 {in_trash} 張照片位於此圖庫的「最近刪除」中,並會保留在那裡;如需檢視,請從「最近刪除」中復原。", "{skipped} catalog images were not imported because their records in the catalog are incomplete.": "有 {skipped} 張影像未匯入,因為它們在目錄中的記錄不完整。", "{photos} catalog records processed; {collections} collections.": "已處理 {photos} 筆目錄記錄;{collections} 個收藏集。", "{missing} originals are missing. Their catalog records are available; use Find Missing Photos to relink them.": "有 {missing} 個原始檔案遺失。其目錄記錄可用;請使用「尋找遺失相片」重新連結檔案。", diff --git a/crates/ui-egui/src/lightroom_import.rs b/crates/ui-egui/src/lightroom_import.rs index 098c989d5..83be31b7f 100644 --- a/crates/ui-egui/src/lightroom_import.rs +++ b/crates/ui-egui/src/lightroom_import.rs @@ -340,6 +340,13 @@ fn result_dialog(value: &Value, summary: &str) -> crate::state::Dialog { missing = missing )); } + let in_trash = report["inRecentlyDeleted"].as_u64().unwrap_or(0); + if in_trash > 0 { + details.push(crate::i18n::tr_format!( + "{in_trash} catalog photos are in this library's Recently Deleted and stay there; restore them from Recently Deleted to see them.", + in_trash = in_trash + )); + } // images whose catalog records are incomplete come as data (`skipped`), said here in the UI // language; their ids are in the report for the CLI and agents let skipped = report["skipped"].as_array().map_or(0, Vec::len); @@ -713,6 +720,18 @@ mod tests { assert_eq!(&en[2..], ["w1", "w2"]); } + #[test] + fn photos_left_in_recently_deleted_are_one_line() { + let value = json!({"report": {"photos": 3, "collections": 0, "inRecentlyDeleted": 2}}); + crate::i18n::set_language(crate::i18n::Locale::En); + let crate::state::Dialog::LightroomResult { details, .. } = result_dialog(&value, "done") else { panic!("not a Lightroom result") }; + assert_eq!(details.len(), 2, "{details:?}"); + assert!(details[1].starts_with("2 catalog photos are in this library's Recently Deleted"), "{details:?}"); + let value = json!({"report": {"photos": 3, "collections": 0, "inRecentlyDeleted": 0}}); + let crate::state::Dialog::LightroomResult { details, .. } = result_dialog(&value, "done") else { panic!("not a Lightroom result") }; + assert_eq!(details.len(), 1, "no line when none are: {details:?}"); + } + #[test] fn skipped_images_do_not_take_the_warning_slots() { let warnings: Vec<_> = (0..10).map(|i| format!("w{i}")).collect(); From 7c74ae0ac3423c158e409fa400ae5386ac08ae1c Mon Sep 17 00:00:00 2001 From: Chad Elliott Date: Tue, 27 Oct 2026 03:34:08 -0500 Subject: [PATCH 2/6] 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 --- crates/engine/src/lightroom_catalog.rs | 44 +++++++++---- crates/engine/src/lightroom_catalog/tests.rs | 65 +++++++++++++++++++- 2 files changed, 96 insertions(+), 13 deletions(-) diff --git a/crates/engine/src/lightroom_catalog.rs b/crates/engine/src/lightroom_catalog.rs index e29a9e71a..e0dee272a 100644 --- a/crates/engine/src/lightroom_catalog.rs +++ b/crates/engine/src/lightroom_catalog.rs @@ -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(); diff --git a/crates/engine/src/lightroom_catalog/tests.rs b/crates/engine/src/lightroom_catalog/tests.rs index 96a6fca4e..725350a3e 100644 --- a/crates/engine/src/lightroom_catalog/tests.rs +++ b/crates/engine/src/lightroom_catalog/tests.rs @@ -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 = ["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(); From a31a43670bb3a12acb6da0ecc4c19429eaefef1a Mon Sep 17 00:00:00 2001 From: Chad Elliott Date: Tue, 27 Oct 2026 10:20:19 -0500 Subject: [PATCH 3/6] fix(lightroom): content duplicates start from import defaults; Show in Library skips Recently Deleted (#2589) A catalog photo whose bytes match another library photo got the generic develop defaults, so a raw with no white balance or lens setting in the catalog lost its as-shot white balance and embedded lens corrections and rendered unlike a file import. It now gets the import defaults (camera defaults plus the default preset), as a file import does, before the catalog's settings go on top. Show in Library selected the lowest mapped photo id, which can be a master left in Recently Deleted whose new virtual copy the import added; All Photos doesn't show it. It now selects the lowest mapped photo not in Recently Deleted, and the button is left out when there is none. Tests: a content-duplicate raw with no catalog settings equals its camera defaults (as-shot WB, lens profile on); Show in Library skips a trashed mapped photo and offers nothing when only trashed or vanished ids are mapped. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/lightroom_catalog.rs | 3 ++ crates/engine/src/lightroom_catalog/tests.rs | 46 ++++++++++++++++ crates/ui-egui/src/lightroom_import.rs | 55 +++++++++++++++++--- docs/lightroom-catalog-import.md | 3 +- 4 files changed, 98 insertions(+), 9 deletions(-) diff --git a/crates/engine/src/lightroom_catalog.rs b/crates/engine/src/lightroom_catalog.rs index e0dee272a..30691009f 100644 --- a/crates/engine/src/lightroom_catalog.rs +++ b/crates/engine/src/lightroom_catalog.rs @@ -1033,6 +1033,9 @@ pub(crate) fn apply_prepared(s: &mut crate::Session, mut data: CatalogImport, co p.meta.iso = m.iso; p.meta.gps = m.gps; p.meta.focus = m.focus.clone(); + // the settings a file import gives it (as-shot white balance, embedded lens + // corrections, the default preset); the catalog's settings apply on top + crate::import::apply_import_defaults(s, &mut p); p } else { let id = s.catalog.alloc_photo_id(); diff --git a/crates/engine/src/lightroom_catalog/tests.rs b/crates/engine/src/lightroom_catalog/tests.rs index 725350a3e..28d9c615f 100644 --- a/crates/engine/src/lightroom_catalog/tests.rs +++ b/crates/engine/src/lightroom_catalog/tests.rs @@ -245,6 +245,52 @@ fn a_content_duplicate_of_a_trashed_photo_or_virtual_copy_is_a_new_live_photo() assert_eq!(s.catalog.photo(other).unwrap().deleted, case == "trashed", "{case}: the other photo is left as it was"); } } +/// A content duplicate starts from the settings a file import gives a raw (its as-shot white +/// balance and embedded lens corrections), as its file would on its own, before the catalog's +/// settings go on top; not from the generic defaults (#2589). +#[test] +fn a_content_duplicate_raw_starts_from_its_camera_defaults() { + let mut s = crate::Session::new(); + let other = library_photo(&mut s, "/elsewhere/a.DNG", |p| { + p.deleted = true; + p.format = "DNG".into(); + p.kind = lightcraft_catalog::MediaKind::Raw; + p.as_shot_wb = Some((5200.0, 4.0)); + p.embedded_lens = Some(lightcraft_develop::EmbeddedLens::default()); + }); + let mut data = sample(); + data.photos.retain(|p| number(&p.image, "masterImage") == 0); + // a catalog row with no develop settings of its own + data.photos[0].settings.clear(); + let files = crate::import::ImportReport { + duplicates: vec![crate::import::Duplicate { + path: "/missing/test.ARW".into(), + existing: Some(other.0), + reason: "content", + existing_deleted: true, + }], + ..Default::default() + }; + let undo0 = s.undo.len(); + let r = apply_prepared( + &mut s, + data, + ApplyPrepared { + update_existing: false, + files, + index: &mut ImportIndex::default(), + archive_path: None, + missing_sources: &HashSet::new(), + undo0, + }, + ) + .unwrap() + .report; + let new = s.catalog.photo(PhotoId(r["mapping"]["1"].as_u64().unwrap())).unwrap(); + assert_eq!(*new.develop, new.camera_defaults(), "{r}"); + assert_eq!((new.develop.wb.temp, new.develop.wb.tint), (5200.0, 4.0), "the file's as-shot white balance"); + assert!(new.develop.optics.lens_profile, "the file's embedded lens corrections are on"); +} /// A catalog photo matched (by path) to a library photo in Recently Deleted leaves it there, as /// the import was set up to do (a Lightroom import doesn't restore, see `OnDeleted`), and the report /// counts it so the photos don't go missing unexplained (#2589). A new virtual copy of it is diff --git a/crates/ui-egui/src/lightroom_import.rs b/crates/ui-egui/src/lightroom_import.rs index 83be31b7f..1b67f19ca 100644 --- a/crates/ui-egui/src/lightroom_import.rs +++ b/crates/ui-egui/src/lightroom_import.rs @@ -303,7 +303,7 @@ fn terminal(app: &mut LightcraftApp, ctx: &egui::Context, task: &LightroomTask, } }; if matches!(kind, Kind::Import) && value.get("cancelled") != Some(&Value::Bool(true)) { - app.lightroom_result = Some(result_dialog(&value, &message)); + app.lightroom_result = Some(result_dialog(&value, &message, &app.session.catalog)); show_result(app); } app.lightroom_last = Some(value); @@ -318,8 +318,9 @@ fn show_result(app: &mut LightcraftApp) { } /// The lasting result window for an inspect or import: counts, failures, missing originals, the -/// skipped images (one line, in the UI language) and at most eight warnings. -fn result_dialog(value: &Value, summary: &str) -> crate::state::Dialog { +/// skipped images (one line, in the UI language) and at most eight warnings. Show in Library goes +/// to the lowest mapped photo that is not in Recently Deleted, where All Photos can show it. +fn result_dialog(value: &Value, summary: &str, catalog: &lightcraft_catalog::Catalog) -> crate::state::Dialog { let report = value.get("report").unwrap_or(value); let mut details = Vec::new(); if let Some(photos) = report["photos"].as_u64() { @@ -366,7 +367,10 @@ fn result_dialog(value: &Value, summary: &str) -> crate::state::Dialog { details.push(crate::i18n::tr_format!("{count} more warnings.", count = warnings.len() - 8)); } } - let first = value.get("error").is_none().then(|| report["mapping"].as_object()?.values().filter_map(Value::as_u64).min()).flatten(); + // a catalog photo can map to a library photo left in Recently Deleted (#2589), which All + // Photos doesn't show + let live = |id: &u64| catalog.photo(lightcraft_catalog::PhotoId(*id)).is_some_and(|p| !p.deleted); + let first = value.get("error").is_none().then(|| report["mapping"].as_object()?.values().filter_map(Value::as_u64).filter(live).min()).flatten(); crate::state::Dialog::LightroomResult { summary: summary.to_string(), library: report["library"].as_str().or_else(|| value["library"].as_str()).map(str::to_string), @@ -680,6 +684,33 @@ mod tests { assert!(!h.app.session.visible().is_empty()); } + /// Show in Library goes to a photo All Photos shows: not a master left in Recently Deleted + /// whose new virtual copy the import added (#2589), and nowhere when no mapped photo is shown. + #[test] + fn show_in_library_skips_photos_in_recently_deleted() { + use lightcraft_catalog::{Op, Photo, Source}; + let mut s = lightcraft_engine::Session::new(); + let mut add = |deleted: bool| { + let id = s.catalog.alloc_photo_id(); + let mut p = Photo::new(id, Source::File { path: format!("/p/{}.jpg", id.0) }, "a.jpg", "JPEG", 2, 2, "now"); + p.deleted = deleted; + s.commit("Add", Op::AddPhoto { photo: Box::new(p) }).unwrap(); + id.0 + }; + let (trashed, live) = (add(true), add(false)); + assert!(trashed < live); + let first = |mapping: Value| { + let crate::state::Dialog::LightroomResult { first, .. } = result_dialog(&json!({"report": {"mapping": mapping}}), "done", &s.catalog) + else { + panic!("not a Lightroom result") + }; + first + }; + assert_eq!(first(json!({"1": trashed, "2": live})), Some(live)); + assert_eq!(first(json!({"1": trashed})), None, "only a photo in Recently Deleted: no Show in Library"); + assert_eq!(first(json!({"1": live + 100})), None, "no longer in the library"); + } + #[test] fn lightroom_failure_stays_readable_without_offering_a_success_action() { let mut h = demo(); @@ -714,7 +745,9 @@ mod tests { {"image": 9, "reason": "masterSkipped"}, ]}}); crate::i18n::set_language(crate::i18n::Locale::En); - let crate::state::Dialog::LightroomResult { details: en, .. } = result_dialog(&value, "done") else { panic!("not a Lightroom result") }; + let crate::state::Dialog::LightroomResult { details: en, .. } = result_dialog(&value, "done", &Default::default()) else { + panic!("not a Lightroom result") + }; assert_eq!(en.len(), 4, "{en:?}"); assert_eq!(en[1], "3 catalog images were not imported because their records in the catalog are incomplete."); assert_eq!(&en[2..], ["w1", "w2"]); @@ -724,11 +757,15 @@ mod tests { fn photos_left_in_recently_deleted_are_one_line() { let value = json!({"report": {"photos": 3, "collections": 0, "inRecentlyDeleted": 2}}); crate::i18n::set_language(crate::i18n::Locale::En); - let crate::state::Dialog::LightroomResult { details, .. } = result_dialog(&value, "done") else { panic!("not a Lightroom result") }; + let crate::state::Dialog::LightroomResult { details, .. } = result_dialog(&value, "done", &Default::default()) else { + panic!("not a Lightroom result") + }; assert_eq!(details.len(), 2, "{details:?}"); assert!(details[1].starts_with("2 catalog photos are in this library's Recently Deleted"), "{details:?}"); let value = json!({"report": {"photos": 3, "collections": 0, "inRecentlyDeleted": 0}}); - let crate::state::Dialog::LightroomResult { details, .. } = result_dialog(&value, "done") else { panic!("not a Lightroom result") }; + let crate::state::Dialog::LightroomResult { details, .. } = result_dialog(&value, "done", &Default::default()) else { + panic!("not a Lightroom result") + }; assert_eq!(details.len(), 1, "no line when none are: {details:?}"); } @@ -737,7 +774,9 @@ mod tests { let warnings: Vec<_> = (0..10).map(|i| format!("w{i}")).collect(); let skipped: Vec<_> = (0..500).map(|i| json!({"image": i, "reason": "missingFile"})).collect(); let value = json!({"report": {"warnings": warnings, "skipped": skipped}}); - let crate::state::Dialog::LightroomResult { details, .. } = result_dialog(&value, "done") else { panic!("not a Lightroom result") }; + let crate::state::Dialog::LightroomResult { details, .. } = result_dialog(&value, "done", &Default::default()) else { + panic!("not a Lightroom result") + }; // one line for the skipped images, then the usual eight warnings and the rest counted assert_eq!(details.len(), 10, "{details:?}"); assert!(details[0].starts_with("500 catalog images were not imported")); diff --git a/docs/lightroom-catalog-import.md b/docs/lightroom-catalog-import.md index cd5df6b85..04cbfc415 100644 --- a/docs/lightroom-catalog-import.md +++ b/docs/lightroom-catalog-import.md @@ -52,7 +52,8 @@ lightcraft-cli run --library "LightCraft Library" library.importLightroom path=" The import itself (`library.importLightroom`, `lightcraft-cli run`) adds records to the currently open LightCraft library; only the desktop window above can make a new library for it first. The desktop result stays open until dismissed and shows the destination library, the number of records added, missing originals and warnings. -**Show in Library** opens All Photos, clears filters and selects an imported record. Errors stay +**Show in Library** opens All Photos, clears filters and selects an imported record (not one left in Recently +Deleted; the button is left out when every record is there). Errors stay readable in the same result window. If another dialog is open, the result waits until that dialog closes. The result is also available through `library.importLightroom {"status":true}` during that session. From 5b8b9b521d692304f40c7e309b561ae848e5fe07 Mon Sep 17 00:00:00 2001 From: Chad Elliott Date: Tue, 27 Oct 2026 15:18:58 -0500 Subject: [PATCH 4/6] fix(lightroom): a content duplicate keeps the metadata its file carries (#2589) A catalog photo whose bytes match another library photo became a new record that took only the camera description from that photo's record, so the copyright, rights, creator and other metadata its file carries (EXIF, IPTC, embedded XMP), which a file import keeps, was lost whenever the catalog's XMP didn't restate it. Copying them from the other record would have carried that photo's edits instead. The file import already reads each content duplicate before it skips it; the probed metadata now rides on the duplicate (`Duplicate::file_meta`, not serialised) and the new record starts from it, before the import defaults and the catalog's metadata go on top, as for a file import. Without a probe (a path match, which this branch never sees) it keeps the exposure description only, no longer the user-editable GPS. Test: a content duplicate of a trashed photo, imported through the Lightroom job with a file carrying copyright, status, usage terms and creator, keeps them and takes nothing from the other record's edited notice, creator or GPS. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/import/commit.rs | 10 +++- crates/engine/src/import/job.rs | 13 ++++- crates/engine/src/import/types.rs | 6 +++ crates/engine/src/lightroom_catalog.rs | 33 +++++++----- crates/engine/src/lightroom_catalog/tests.rs | 56 ++++++++++++++++++++ 5 files changed, 102 insertions(+), 16 deletions(-) diff --git a/crates/engine/src/import/commit.rs b/crates/engine/src/import/commit.rs index 7d1a69fdd..02f587a43 100644 --- a/crates/engine/src/import/commit.rs +++ b/crates/engine/src/import/commit.rs @@ -213,7 +213,7 @@ pub fn commit_prepared(s: &mut Session, opts: &ImportOptions, now: &str, prepare } report.imported.push(id.0); } - PreparedItem::Duplicate { path, existing, reason, hash } => { + PreparedItem::Duplicate { path, existing, reason, hash, file_meta } => { let existing = existing.or_else(|| { let h = hash?; new_hash.get(&h).copied().or_else(|| s.catalog.photos().find(|p| p.content_hash.as_deref() == Some(h.as_str())).map(|p| p.id)) @@ -232,7 +232,13 @@ pub fn commit_prepared(s: &mut Session, opts: &ImportOptions, now: &str, prepare report.restored.push(id.0); } } - _ => report.duplicates.push(Duplicate { path, existing: existing.map(|i| i.0), reason, existing_deleted: trashed.is_some() }), + _ => report.duplicates.push(Duplicate { + path, + existing: existing.map(|i| i.0), + reason, + existing_deleted: trashed.is_some(), + file_meta, + }), } } PreparedItem::Failed(path, e) => report.failed.push((path, e)), diff --git a/crates/engine/src/import/job.rs b/crates/engine/src/import/job.rs index 1730518d5..d4468d128 100644 --- a/crates/engine/src/import/job.rs +++ b/crates/engine/src/import/job.rs @@ -73,6 +73,8 @@ pub(super) enum PreparedItem { existing: Option, reason: &'static str, hash: Option, + /// The metadata the file itself carries, when it was read ([`Duplicate::file_meta`]). + file_meta: Option>, }, Failed(String, String), Kept(Kept), @@ -141,6 +143,7 @@ impl Prepared { existing: Some(existing), reason: if existing_path.is_some() { "path" } else { "content" }, hash: info.content_hash, + file_meta: Some(Box::new(info.meta)), }); } else { items.push(PreparedItem::Ready(Box::new(ReadyFile { path, stored, info, sidecar, placed }))); @@ -261,7 +264,7 @@ impl ImportJob { self.done.fetch_add(1, Relaxed); } Some((id, _)) => { - out.items.push(PreparedItem::Duplicate { path: f, existing: Some(id), reason: "path", hash: None }); + out.items.push(PreparedItem::Duplicate { path: f, existing: Some(id), reason: "path", hash: None, file_meta: None }); self.done.fetch_add(1, Relaxed); } None => todo.push(f), @@ -326,7 +329,13 @@ impl ImportJob { && let Some(existing) = self.by_hash.get(h) { self.done.fetch_add(1, Relaxed); - plan.slots.push(Slot::Item(PreparedItem::Duplicate { path, existing: *existing, reason: "content", hash: Some(h.clone()) })); + plan.slots.push(Slot::Item(PreparedItem::Duplicate { + path, + existing: *existing, + reason: "content", + hash: Some(h.clone()), + file_meta: Some(Box::new(info.meta)), + })); continue; } // the XMP sidecar (or a raw's embedded XMP), read before copying: its capture time files diff --git a/crates/engine/src/import/types.rs b/crates/engine/src/import/types.rs index 6ad3e2153..779c5f27c 100644 --- a/crates/engine/src/import/types.rs +++ b/crates/engine/src/import/types.rs @@ -146,6 +146,12 @@ pub struct Duplicate { /// That photo is in Recently Deleted (see [`OnDeleted`]). #[serde(rename = "existingDeleted", skip_serializing_if = "std::ops::Not::not")] pub existing_deleted: bool, + /// The metadata the file itself carries (EXIF, IPTC, embedded XMP), as a file import reads it; + /// `None` when the file wasn't read (a path match). A Lightroom import makes a record of its + /// own for a duplicate by content and starts it from these, not from the other photo's record, + /// whose metadata may have been edited (#2589). + #[serde(skip)] + pub file_meta: Option>, } #[derive(Clone, Debug, Default, Serialize)] diff --git a/crates/engine/src/lightroom_catalog.rs b/crates/engine/src/lightroom_catalog.rs index 30691009f..e77192b78 100644 --- a/crates/engine/src/lightroom_catalog.rs +++ b/crates/engine/src/lightroom_catalog.rs @@ -943,7 +943,8 @@ pub(crate) fn apply_prepared(s: &mut crate::Session, mut data: CatalogImport, co refuse_non_media(&mut data); validate(&data).map_err(error)?; let mut warnings = data.warnings.clone(); - let duplicates: HashMap<_, _> = files.duplicates.iter().filter_map(|d| d.existing.map(|id| (key(&d.path), PhotoId(id)))).collect(); + let duplicates: HashMap<_, _> = + files.duplicates.iter().filter_map(|d| d.existing.map(|id| (key(&d.path), (PhotoId(id), d.file_meta.as_deref())))).collect(); let mut by_path: HashMap<_, _> = s.catalog.photos().filter(|p| p.copy_of.is_none()).filter_map(|p| source_path(p).map(|path| (key(path), p.id))).collect(); let mut ids = HashMap::new(); @@ -1000,11 +1001,14 @@ 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)).cloned() { + } else if let Some((duplicate, file_meta)) = + duplicates.get(&key(&src.path)).and_then(|(id, meta)| s.catalog.photo(*id).cloned().map(|p| (p, *meta))) + { // Catalogs can contain distinct originals with identical bytes and independent edits. // 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. + // edits, history, versions, ratings or the metadata a person writes. Its metadata is + // what its own file carries, as a file import reads it (copyright, creator…). 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(), @@ -1024,15 +1028,20 @@ pub(crate) fn apply_prepared(s: &mut crate::Session, mut data: CatalogImport, co 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(); + if let Some(meta) = file_meta { + p.meta = meta.clone(); + } else { + // (the file wasn't read: only the exposure description, which no command edits; + // not GPS, which placing a photo on the map does) + 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.focus = m.focus.clone(); + } // the settings a file import gives it (as-shot white balance, embedded lens // corrections, the default preset); the catalog's settings apply on top crate::import::apply_import_defaults(s, &mut p); diff --git a/crates/engine/src/lightroom_catalog/tests.rs b/crates/engine/src/lightroom_catalog/tests.rs index 28d9c615f..34270621c 100644 --- a/crates/engine/src/lightroom_catalog/tests.rs +++ b/crates/engine/src/lightroom_catalog/tests.rs @@ -208,6 +208,8 @@ fn a_content_duplicate_of_a_trashed_photo_or_virtual_copy_is_a_new_live_photo() existing: Some(other.0), reason: "content", existing_deleted: case == "trashed", + // what the file itself carries + file_meta: Some(Box::new(lightcraft_catalog::Meta { camera: "Same Camera".into(), ..Default::default() })), }], ..Default::default() }; @@ -268,6 +270,7 @@ fn a_content_duplicate_raw_starts_from_its_camera_defaults() { existing: Some(other.0), reason: "content", existing_deleted: true, + file_meta: None, }], ..Default::default() }; @@ -291,6 +294,59 @@ fn a_content_duplicate_raw_starts_from_its_camera_defaults() { assert_eq!((new.develop.wb.temp, new.develop.wb.tint), (5200.0, 4.0), "the file's as-shot white balance"); assert!(new.develop.optics.lens_profile, "the file's embedded lens corrections are on"); } +/// A content duplicate keeps the metadata its file carries (copyright, rights, creator), as a file +/// import of it would, when the catalog's XMP doesn't restate it; it reads the file's, not the +/// other photo's, which a person may have edited (#2589). +#[test] +fn a_content_duplicate_keeps_the_rights_metadata_in_its_file() { + let dir = std::env::temp_dir().join(format!("lightcraft-lrcat-duplicate-rights-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(dir.join("Photos")).unwrap(); + let source = dir.join("source.lrcat"); + std::fs::write(dir.join("Photos/test.ARW"), b"same bytes").unwrap(); + std::fs::write(&source, native_catalog(&photo_tables((dir.to_str().unwrap(), "")))).unwrap(); + let mut s = crate::Session::new(); + let other = library_photo(&mut s, "/elsewhere/a.ARW", |p| { + p.deleted = true; + // edited on the other record after its import + p.meta.copyright = "Other photo's notice".into(); + p.meta.creator = "Other photo's creator".into(); + p.meta.gps = Some((1.0, 2.0)); + }); + let file_meta = lightcraft_catalog::Meta { + camera: "Same Camera".into(), + copyright: "© Jo Example".into(), + copyright_status: lightcraft_catalog::CopyrightStatus::Copyrighted, + usage_terms: "No reuse".into(), + creator: "Jo Example".into(), + ..Default::default() + }; + let probed = file_meta.clone(); + s.media.file_probe = Some(Arc::new(move |_| { + Ok(crate::media::ProbeInfo { + format: "ARW".into(), + width: 20, + height: 10, + content_hash: Some("same-bytes".into()), + meta: probed.clone(), + ..Default::default() + }) + })); + let mut job = crate::lightroom_job::LightroomJob::new(&mut s, source, false).unwrap(); + let prepared = job.prepare(&AtomicBool::new(false)).unwrap(); + let r = crate::lightroom_job::commit_prepared(&mut s, prepared).unwrap().report; + let new = s.catalog.photos().find(|p| p.id != other).unwrap_or_else(|| panic!("no new record: {r}")); + assert_eq!(new.content_hash.as_deref(), Some("same-bytes"), "{r}"); + assert_eq!(new.rating, 5, "the catalog's own metadata applies: {r}"); + let m = &new.meta; + assert_eq!( + (m.copyright.as_str(), m.copyright_status, m.usage_terms.as_str(), m.creator.as_str(), m.camera.as_str()), + ("© Jo Example", file_meta.copyright_status, "No reuse", "Jo Example", "Same Camera"), + "the file's own metadata: {r}" + ); + assert_eq!(m.gps, None, "nothing from the other photo's record"); + std::fs::remove_dir_all(dir).unwrap(); +} /// A catalog photo matched (by path) to a library photo in Recently Deleted leaves it there, as /// the import was set up to do (a Lightroom import doesn't restore, see `OnDeleted`), and the report /// counts it so the photos don't go missing unexplained (#2589). A new virtual copy of it is From d9476bc16dd0c301da05a0defced9701059d1426 Mon Sep 17 00:00:00 2001 From: Chad Elliott Date: Thu, 29 Oct 2026 23:57:55 -0500 Subject: [PATCH 5/6] fix(lightroom): a content duplicate takes its file's capture time (#2589) A content duplicate whose catalog row has no capture time took the capture time of the library photo with the same bytes, which Edit Capture Time may have changed. The import job now resolves the file's capture time (the file's, else its XMP sidecar's, else its modification time) before it sets the file aside as a duplicate, and hands it over with the file's metadata (`Duplicate::file`, a `FileFacts`). The Lightroom import uses it; the catalog's capture time still applies on top when the row has one. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/import.rs | 4 +- crates/engine/src/import/commit.rs | 12 ++---- crates/engine/src/import/job.rs | 39 ++++++++++---------- crates/engine/src/import/types.rs | 19 +++++++--- crates/engine/src/lightroom_catalog.rs | 18 +++++---- crates/engine/src/lightroom_catalog/tests.rs | 19 +++++++--- 6 files changed, 63 insertions(+), 48 deletions(-) diff --git a/crates/engine/src/import.rs b/crates/engine/src/import.rs index 6fe665896..e42057c2b 100644 --- a/crates/engine/src/import.rs +++ b/crates/engine/src/import.rs @@ -53,8 +53,8 @@ pub use scan::{ScanInput, ScanOutput, ScanProgress, scan, scan_with}; #[cfg(test)] use scan::{library_files, library_hashes}; pub use types::{ - Duplicate, ImportCandidate, ImportMode, ImportOptions, ImportReport, ImportUprightMode, ImportWarning, ImportWarningCode, Kept, Moved, OnDeleted, - Organize, + Duplicate, FileFacts, ImportCandidate, ImportMode, ImportOptions, ImportReport, ImportUprightMode, ImportWarning, ImportWarningCode, Kept, Moved, + OnDeleted, Organize, }; #[cfg(test)] diff --git a/crates/engine/src/import/commit.rs b/crates/engine/src/import/commit.rs index 02f587a43..a731488a0 100644 --- a/crates/engine/src/import/commit.rs +++ b/crates/engine/src/import/commit.rs @@ -213,7 +213,7 @@ pub fn commit_prepared(s: &mut Session, opts: &ImportOptions, now: &str, prepare } report.imported.push(id.0); } - PreparedItem::Duplicate { path, existing, reason, hash, file_meta } => { + PreparedItem::Duplicate { path, existing, reason, hash, file } => { let existing = existing.or_else(|| { let h = hash?; new_hash.get(&h).copied().or_else(|| s.catalog.photos().find(|p| p.content_hash.as_deref() == Some(h.as_str())).map(|p| p.id)) @@ -232,13 +232,9 @@ pub fn commit_prepared(s: &mut Session, opts: &ImportOptions, now: &str, prepare report.restored.push(id.0); } } - _ => report.duplicates.push(Duplicate { - path, - existing: existing.map(|i| i.0), - reason, - existing_deleted: trashed.is_some(), - file_meta, - }), + _ => { + report.duplicates.push(Duplicate { path, existing: existing.map(|i| i.0), reason, existing_deleted: trashed.is_some(), file }) + } } } PreparedItem::Failed(path, e) => report.failed.push((path, e)), diff --git a/crates/engine/src/import/job.rs b/crates/engine/src/import/job.rs index d4468d128..ffc098c50 100644 --- a/crates/engine/src/import/job.rs +++ b/crates/engine/src/import/job.rs @@ -1,6 +1,6 @@ use super::commit::{How, Plan, Slot, Transfer, commit_prepared, name_family, run_transfers, system_clock}; use super::files::{expand, probe_all}; -use super::types::{ImportCandidate, ImportMode, ImportOptions, ImportReport, Kept, OnDeleted}; +use super::types::{FileFacts, ImportCandidate, ImportMode, ImportOptions, ImportReport, Kept, OnDeleted}; use super::{HashMap, MediaKind, Path, PathBuf, Photo, PhotoId, ProbeInfo, Session, Source}; /// Import files/folders. See the module docs. @@ -73,8 +73,8 @@ pub(super) enum PreparedItem { existing: Option, reason: &'static str, hash: Option, - /// The metadata the file itself carries, when it was read ([`Duplicate::file_meta`]). - file_meta: Option>, + /// What the file itself says, when it was read ([`Duplicate::file`]). + file: Option>, }, Failed(String, String), Kept(Kept), @@ -143,7 +143,7 @@ impl Prepared { existing: Some(existing), reason: if existing_path.is_some() { "path" } else { "content" }, hash: info.content_hash, - file_meta: Some(Box::new(info.meta)), + file: Some(Box::new(FileFacts { meta: info.meta, captured: info.captured })), }); } else { items.push(PreparedItem::Ready(Box::new(ReadyFile { path, stored, info, sidecar, placed }))); @@ -264,7 +264,7 @@ impl ImportJob { self.done.fetch_add(1, Relaxed); } Some((id, _)) => { - out.items.push(PreparedItem::Duplicate { path: f, existing: Some(id), reason: "path", hash: None, file_meta: None }); + out.items.push(PreparedItem::Duplicate { path: f, existing: Some(id), reason: "path", hash: None, file: None }); self.done.fetch_add(1, Relaxed); } None => todo.push(f), @@ -324,20 +324,6 @@ impl ImportJob { break; } } - if let Some(h) = &info.content_hash - && !opts.local - && let Some(existing) = self.by_hash.get(h) - { - self.done.fetch_add(1, Relaxed); - plan.slots.push(Slot::Item(PreparedItem::Duplicate { - path, - existing: *existing, - reason: "content", - hash: Some(h.clone()), - file_meta: Some(Box::new(info.meta)), - })); - continue; - } // the XMP sidecar (or a raw's embedded XMP), read before copying: its capture time files // and names the copy when the file itself has none let raw = info.kind == MediaKind::Raw; @@ -359,6 +345,21 @@ impl ImportJob { if info.captured.is_none() { info.captured = info.file_modified.clone(); } + // same bytes as a library photo: skipped, with what its file says (its capture time too) + if let Some(h) = &info.content_hash + && !opts.local + && let Some(existing) = self.by_hash.get(h) + { + self.done.fetch_add(1, Relaxed); + plan.slots.push(Slot::Item(PreparedItem::Duplicate { + path, + existing: *existing, + reason: "content", + hash: Some(h.clone()), + file: Some(Box::new(FileFacts { meta: info.meta, captured: info.captured })), + })); + continue; + } let stored = match (mode, &self.copy_root) { (ImportMode::Copy | ImportMode::Move, Some(root)) if !crate::import_move::inside(Path::new(&path), root) diff --git a/crates/engine/src/import/types.rs b/crates/engine/src/import/types.rs index 779c5f27c..f342bf4c9 100644 --- a/crates/engine/src/import/types.rs +++ b/crates/engine/src/import/types.rs @@ -146,12 +146,21 @@ pub struct Duplicate { /// That photo is in Recently Deleted (see [`OnDeleted`]). #[serde(rename = "existingDeleted", skip_serializing_if = "std::ops::Not::not")] pub existing_deleted: bool, - /// The metadata the file itself carries (EXIF, IPTC, embedded XMP), as a file import reads it; - /// `None` when the file wasn't read (a path match). A Lightroom import makes a record of its - /// own for a duplicate by content and starts it from these, not from the other photo's record, - /// whose metadata may have been edited (#2589). + /// What the file itself says, as a file import reads it; `None` when the file wasn't read (a + /// path match). A Lightroom import makes a record of its own for a duplicate by content and + /// starts it from these, not from the other photo's record, which may have been edited (#2589). #[serde(skip)] - pub file_meta: Option>, + pub file: Option>, +} + +/// What a file import reads from a file itself, before anything in the library is edited. +#[derive(Clone, Debug, Default)] +pub struct FileFacts { + /// The metadata the file carries (EXIF, IPTC, embedded XMP). + pub meta: lightcraft_catalog::Meta, + /// The capture time a file import gives it: the file's, else its XMP sidecar's, else the + /// file's modification time. + pub captured: Option, } #[derive(Clone, Debug, Default, Serialize)] diff --git a/crates/engine/src/lightroom_catalog.rs b/crates/engine/src/lightroom_catalog.rs index e77192b78..0dfef543b 100644 --- a/crates/engine/src/lightroom_catalog.rs +++ b/crates/engine/src/lightroom_catalog.rs @@ -944,7 +944,7 @@ pub(crate) fn apply_prepared(s: &mut crate::Session, mut data: CatalogImport, co validate(&data).map_err(error)?; let mut warnings = data.warnings.clone(); let duplicates: HashMap<_, _> = - files.duplicates.iter().filter_map(|d| d.existing.map(|id| (key(&d.path), (PhotoId(id), d.file_meta.as_deref())))).collect(); + files.duplicates.iter().filter_map(|d| d.existing.map(|id| (key(&d.path), (PhotoId(id), d.file.as_deref())))).collect(); let mut by_path: HashMap<_, _> = s.catalog.photos().filter(|p| p.copy_of.is_none()).filter_map(|p| source_path(p).map(|path| (key(path), p.id))).collect(); let mut ids = HashMap::new(); @@ -1001,14 +1001,15 @@ 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, file_meta)) = + } else if let Some((duplicate, file)) = duplicates.get(&key(&src.path)).and_then(|(id, meta)| s.catalog.photo(*id).cloned().map(|p| (p, *meta))) { // Catalogs can contain distinct originals with identical bytes and independent edits. // 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. Its metadata is - // what its own file carries, as a file import reads it (copyright, creator…). + // edits, history, versions, ratings, capture time or the metadata a person writes. Its + // metadata and capture time are what its own file gives it, as a file import reads them + // (copyright, creator…); the catalog's capture time, when it has one, applies on top. 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(), @@ -1021,18 +1022,19 @@ pub(crate) fn apply_prepared(s: &mut crate::Session, mut data: CatalogImport, co ); 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(); - if let Some(meta) = file_meta { - p.meta = meta.clone(); + if let Some(file) = file { + p.meta = file.meta.clone(); + p.captured = file.captured.clone(); } else { // (the file wasn't read: only the exposure description, which no command edits; - // not GPS, which placing a photo on the map does) + // not GPS, which placing a photo on the map does, nor the capture time, which + // Edit Capture Time does) let m = &duplicate.meta; p.meta.camera = m.camera.clone(); p.meta.lens = m.lens.clone(); diff --git a/crates/engine/src/lightroom_catalog/tests.rs b/crates/engine/src/lightroom_catalog/tests.rs index 34270621c..aa380774c 100644 --- a/crates/engine/src/lightroom_catalog/tests.rs +++ b/crates/engine/src/lightroom_catalog/tests.rs @@ -209,7 +209,10 @@ fn a_content_duplicate_of_a_trashed_photo_or_virtual_copy_is_a_new_live_photo() reason: "content", existing_deleted: case == "trashed", // what the file itself carries - file_meta: Some(Box::new(lightcraft_catalog::Meta { camera: "Same Camera".into(), ..Default::default() })), + file: Some(Box::new(crate::import::FileFacts { + meta: lightcraft_catalog::Meta { camera: "Same Camera".into(), ..Default::default() }, + captured: None, + })), }], ..Default::default() }; @@ -270,7 +273,7 @@ fn a_content_duplicate_raw_starts_from_its_camera_defaults() { existing: Some(other.0), reason: "content", existing_deleted: true, - file_meta: None, + file: None, }], ..Default::default() }; @@ -294,11 +297,11 @@ fn a_content_duplicate_raw_starts_from_its_camera_defaults() { assert_eq!((new.develop.wb.temp, new.develop.wb.tint), (5200.0, 4.0), "the file's as-shot white balance"); assert!(new.develop.optics.lens_profile, "the file's embedded lens corrections are on"); } -/// A content duplicate keeps the metadata its file carries (copyright, rights, creator), as a file -/// import of it would, when the catalog's XMP doesn't restate it; it reads the file's, not the -/// other photo's, which a person may have edited (#2589). +/// A content duplicate keeps the metadata its file carries (copyright, rights, creator) and the +/// capture time its file gives it, as a file import of it would, when the catalog doesn't restate +/// them; it reads the file's, not the other photo's, which a person may have edited (#2589). #[test] -fn a_content_duplicate_keeps_the_rights_metadata_in_its_file() { +fn a_content_duplicate_keeps_the_rights_metadata_and_capture_time_in_its_file() { let dir = std::env::temp_dir().join(format!("lightcraft-lrcat-duplicate-rights-{}", std::process::id())); let _ = std::fs::remove_dir_all(&dir); std::fs::create_dir_all(dir.join("Photos")).unwrap(); @@ -312,6 +315,7 @@ fn a_content_duplicate_keeps_the_rights_metadata_in_its_file() { p.meta.copyright = "Other photo's notice".into(); p.meta.creator = "Other photo's creator".into(); p.meta.gps = Some((1.0, 2.0)); + p.captured = Some("1999-12-31T23:00:00".into()); }); let file_meta = lightcraft_catalog::Meta { camera: "Same Camera".into(), @@ -329,6 +333,7 @@ fn a_content_duplicate_keeps_the_rights_metadata_in_its_file() { height: 10, content_hash: Some("same-bytes".into()), meta: probed.clone(), + captured: Some("2024-05-06T07:08:09".into()), ..Default::default() }) })); @@ -345,6 +350,8 @@ fn a_content_duplicate_keeps_the_rights_metadata_in_its_file() { "the file's own metadata: {r}" ); assert_eq!(m.gps, None, "nothing from the other photo's record"); + // the catalog row has no capture time: the file's, not the other photo's edited one + assert_eq!(new.captured.as_deref(), Some("2024-05-06T07:08:09"), "{r}"); std::fs::remove_dir_all(dir).unwrap(); } /// A catalog photo matched (by path) to a library photo in Recently Deleted leaves it there, as From a57bd69d98e2231c19c516787ad1c3339dcec06f Mon Sep 17 00:00:00 2001 From: Chad Elliott Date: Fri, 30 Oct 2026 02:25:53 -0500 Subject: [PATCH 6/6] fix(import): a duplicate with a capture time skips its sidecar (#2589) Since a content duplicate carries its file's capture time, the duplicate check ran after the sidecar read, so a duplicate whose file already has a capture time read and parsed a sidecar it doesn't use (more I/O on a network share). The check now happens first and the sidecar is read for a duplicate only when the file has no capture time. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/import/job.rs | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/crates/engine/src/import/job.rs b/crates/engine/src/import/job.rs index ffc098c50..baf4e2e70 100644 --- a/crates/engine/src/import/job.rs +++ b/crates/engine/src/import/job.rs @@ -324,12 +324,19 @@ impl ImportJob { break; } } + // same bytes as a library photo: skipped below, once its capture time is known + let duplicate_of = + info.content_hash.as_ref().filter(|_| !opts.local).and_then(|h| self.by_hash.get(h).map(|existing| (h.clone(), *existing))); // the XMP sidecar (or a raw's embedded XMP), read before copying: its capture time files - // and names the copy when the file itself has none + // and names the copy when the file itself has none (a duplicate needs only that time) let raw = info.kind == MediaKind::Raw; - let packet = crate::sidecar::find_sidecar_for_import(&stems, self.naming, &path) - .and_then(|f| std::fs::read_to_string(f).ok()) - .or_else(|| info.xmp.clone().filter(|_| raw)); + let packet = if duplicate_of.is_some() && info.captured.is_some() { + None + } else { + crate::sidecar::find_sidecar_for_import(&stems, self.naming, &path) + .and_then(|f| std::fs::read_to_string(f).ok()) + .or_else(|| info.xmp.clone().filter(|_| raw)) + }; let target = crate::crs::Target::for_file(info.kind, &info.format, info.preview_only.is_some(), &info.meta.camera); let sidecar = packet.and_then(|x| match crate::sidecar::parse_sidecar(&x, target) { Ok(sc) => Some(sc), @@ -345,17 +352,13 @@ impl ImportJob { if info.captured.is_none() { info.captured = info.file_modified.clone(); } - // same bytes as a library photo: skipped, with what its file says (its capture time too) - if let Some(h) = &info.content_hash - && !opts.local - && let Some(existing) = self.by_hash.get(h) - { + if let Some((hash, existing)) = duplicate_of { self.done.fetch_add(1, Relaxed); plan.slots.push(Slot::Item(PreparedItem::Duplicate { path, - existing: *existing, + existing, reason: "content", - hash: Some(h.clone()), + hash: Some(hash), file: Some(Box::new(FileFacts { meta: info.meta, captured: info.captured })), })); continue;