aleleba/lightcraft
1
0
Fork 0
mirror of https://github.com/storytold/lightcraft.git synced 2026-11-01 05:57:09 -06:00
Code Issues Packages Projects Releases Wiki Activity

fix(lightroom): a content duplicate takes its file's capture time (#2589)

Browse Source 
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 <noreply@anthropic.com>
This commit is contained in:
Chad Elliott
2026-10-29 23:57:55 -05:00
co-authored by Claude Opus 5.5
parent 5b8b9b521d
commit d9476bc16d
6 changed files with 63 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/engine/src/import.rs
+2 -2
View File
@@ -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)]
crates/engine/src/import/commit.rs
+4 -8
View File
@@ -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)),
crates/engine/src/import/job.rs
+20 -19
View File
@@ -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<PhotoId>,
reason: &'static str,
hash: Option<String>,
/// The metadata the file itself carries, when it was read ([`Duplicate::file_meta`]).
file_meta: Option<Box<lightcraft_catalog::Meta>>,
/// What the file itself says, when it was read ([`Duplicate::file`]).
file: Option<Box<FileFacts>>,
},
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)
crates/engine/src/import/types.rs
+14 -5
View File
@@ -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<Box<lightcraft_catalog::Meta>>,
pub file: Option<Box<FileFacts>>,
}
/// 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<String>,
}
#[derive(Clone, Debug, Default, Serialize)]
crates/engine/src/lightroom_catalog.rs
+10 -8
View File
@@ -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();
crates/engine/src/lightroom_catalog/tests.rs
+13 -6
View File
@@ -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