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