From 2928880e8120e91465a7dd4ed2c18bfbdecc6e11 Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Thu, 8 Oct 2026 11:16:42 -0400 Subject: [PATCH] Import: onDeleted policy (skip|restore|fresh) for files in Recently Deleted; library.emptyRecentlyDeleted A file that is in Recently Deleted was always skipped as a duplicate with no way forward (#298). The report now flags such duplicates (existingDeleted); onDeleted restores the trashed photo with its edits or replaces it with a fresh import; and Empty Recently Deleted removes every trashed photo in one undoable step. Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/library.rs | 27 +++++++++++- crates/engine/src/import.rs | 64 ++++++++++++++++++++++++++- crates/engine/src/tests_import.rs | 73 +++++++++++++++++++++++++++++++ 3 files changed, 162 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/library.rs b/crates/engine/src/cmd/library.rs index 57632ea82..c3e6b8248 100644 --- a/crates/engine/src/cmd/library.rs +++ b/crates/engine/src/cmd/library.rs @@ -66,7 +66,14 @@ pub fn import_params(s: &Session, p: &Value) -> Result { { return Err(bad("library.import", format!("unknown metadata preset `{n}`"))); } + let on_deleted = match str_param(p, "onDeleted") { + Some(v) => { + crate::import::OnDeleted::parse(v).ok_or_else(|| bad("library.import", format!("unknown onDeleted `{v}` (skip|restore|fresh)")))? + } + None => Default::default(), + }; let opts = crate::import::ImportOptions { + on_deleted, mode, preset, keywords: strs(p, "keywords"), @@ -560,6 +567,24 @@ pub fn specs() -> Vec { id, deleted: false }))), + cmd!( + "library.emptyRecentlyDeleted", + "Empty Recently Deleted", + ["Photo"], + None, + "{} → {deleted} — removes every photo in Recently Deleted from the library (files on disk stay)", + always, + |s, _| { + let ids: Vec = s.catalog.photos().filter(|p| p.deleted).map(|p| p.id).collect(); + if ids.is_empty() { + return Ok(json!({"deleted": 0})); + } + let ops = ids.iter().map(|id| s.catalog.delete_permanently_ops(*id)).collect::>(); + s.commit("Empty Recently Deleted", Op::Batch { ops })?; + s.selection = Selection::default(); + Ok(json!({"deleted": ids.len()})) + } + ), cmd!("photo.deletePermanently", "Delete Permanently", ["Photo"], None, "{ids?}", has_selection, |s, p| { let t = s.targets(p); let ops = t.iter().map(|id| s.catalog.delete_permanently_ops(*id)).collect::>(); @@ -956,7 +981,7 @@ pub fn specs() -> Vec { "Import Photos", ["File"], Some("Cmd+Shift+I"), - "{paths: [file or folder (recursive)], mode?: add|copy|move (add = reference the files in place; copy = into the library's Originals/YYYY/YYYY-MM-DD/; move = as copy, then each original and its XMP sidecars are removed from the source — only after the copy is verified (a hard link on the same volume, else copied, synced and compared byte for byte) and its catalog record is saved; failed, duplicate and unchecked files keep their sources; a taken name gets -1, -2…; undo removes the photos from the library but leaves the files at the destination), destination?: folder for copies / moves, organize?: date (YYYY/YYYY-MM-DD) | month (YYYY/YYYY-MM) | flat | a folder template, e.g. `{date:%Y}/{date:%Y%m%d}` → 2026/20260114 (the template's `/` make the folders, each level expanded with the rename tokens and made a safe folder name: never outside the destination; must be relative, no `..`; a level with missing metadata is `unknown`) — dated by capture time, else the import time, rename?: file-name template for copies, original extension added (tokens: {name} {num} {seq} {seq:N} {date} {date:%Y%m%d} {folder} {camera} {lens} {iso} {rating} {title} {creator} {ext}; photo.renameTokens explains each; blank = keep names), renameStart?: 1, metadataPreset?: name, dng?: bool (copy raws as DNG; copy only), local?: bool (browsing: the photos stay out of the library, like library.browse; not with move), album?: albumId, albumName?: new album, preset?: presetId, keywords?: [..]} → {imported, duplicates, failed, moved?: [{from, to, sidecars?}], kept?: [{path, reason}] (move: sources left in place and why), album?}", + "{paths: [file or folder (recursive)], mode?: add|copy|move (add = reference the files in place; copy = into the library's Originals/YYYY/YYYY-MM-DD/; move = as copy, then each original and its XMP sidecars are removed from the source — only after the copy is verified (a hard link on the same volume, else copied, synced and compared byte for byte) and its catalog record is saved; failed, duplicate and unchecked files keep their sources; a taken name gets -1, -2…; undo removes the photos from the library but leaves the files at the destination), destination?: folder for copies / moves, organize?: date (YYYY/YYYY-MM-DD) | month (YYYY/YYYY-MM) | flat | a folder template, e.g. `{date:%Y}/{date:%Y%m%d}` → 2026/20260114 (the template's `/` make the folders, each level expanded with the rename tokens and made a safe folder name: never outside the destination; must be relative, no `..`; a level with missing metadata is `unknown`) — dated by capture time, else the import time, rename?: file-name template for copies, original extension added (tokens: {name} {num} {seq} {seq:N} {date} {date:%Y%m%d} {folder} {camera} {lens} {iso} {rating} {title} {creator} {ext}; photo.renameTokens explains each; blank = keep names), renameStart?: 1, metadataPreset?: name, dng?: bool (copy raws as DNG; copy only), local?: bool (browsing: the photos stay out of the library, like library.browse; not with move), album?: albumId, albumName?: new album, preset?: presetId, keywords?: [..], onDeleted?: skip|restore|fresh (a file that is in Recently Deleted: skip = leave it there and say so, restore = bring the photo back with its edits, fresh = delete the trashed record and import the file as new)} → {imported, duplicates: [{path, existing, reason, existingDeleted?}], restored?: [photoId], failed, moved?: [{from, to, sidecars?}], kept?: [{path, reason}] (move: sources left in place and why), album?}", always, |s, p| { let req = import_params(s, p)?; diff --git a/crates/engine/src/import.rs b/crates/engine/src/import.rs index 4895eee73..1bbbd64fc 100644 --- a/crates/engine/src/import.rs +++ b/crates/engine/src/import.rs @@ -48,9 +48,34 @@ pub enum ImportMode { Move, } +/// What an import does with a file the library already has in Recently Deleted. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub enum OnDeleted { + /// Leave it there; the report says so ([`Duplicate::existing_deleted`]). + #[default] + Skip, + /// Bring the trashed photo back, with its edits ([`ImportReport::restored`]). + Restore, + /// Delete the trashed record permanently and import the file as a new photo. + Fresh, +} + +impl OnDeleted { + pub fn parse(s: &str) -> Option { + match s { + "skip" => Some(Self::Skip), + "restore" => Some(Self::Restore), + "fresh" => Some(Self::Fresh), + _ => None, + } + } +} + /// What an import does besides adding the photos. #[derive(Clone, Debug, Default)] pub struct ImportOptions { + /// A file that is in Recently Deleted: skip (default), restore, or import afresh. + pub on_deleted: OnDeleted, pub mode: ImportMode, /// Applied to every imported photo (one History entry). pub preset: Option, @@ -147,12 +172,18 @@ pub struct Duplicate { pub existing: Option, /// `"path"` (already imported from there) or `"content"` (same bytes elsewhere). pub reason: &'static str, + /// That photo is in Recently Deleted (see [`OnDeleted`]). + #[serde(rename = "existingDeleted", skip_serializing_if = "std::ops::Not::not")] + pub existing_deleted: bool, } #[derive(Clone, Debug, Default, Serialize)] pub struct ImportReport { pub imported: Vec, pub duplicates: Vec, + /// [`OnDeleted::Restore`]: photos brought back from Recently Deleted instead of imported. + #[serde(skip_serializing_if = "Vec::is_empty")] + pub restored: Vec, /// Move: the originals that were moved (and removed from the source). #[serde(skip_serializing_if = "Vec::is_empty")] pub moved: Vec, @@ -638,6 +669,10 @@ impl ImportJob { let mut by_path = HashMap::new(); let mut by_hash = HashMap::new(); for p in s.catalog.photos() { + // a trashed photo is replaced by a fresh import, so it doesn't count as a duplicate + if p.deleted && opts.on_deleted == OnDeleted::Fresh { + continue; + } if let Source::File { path } = &p.source { by_path.insert(path.clone(), (p.id, p.local)); } @@ -840,6 +875,19 @@ pub fn commit_prepared(s: &mut Session, opts: &ImportOptions, now: &str, prepare let mut placed: Vec = Vec::new(); // photos added by this batch, by content (a duplicate of a file earlier in the import names it) let mut new_hash: HashMap = HashMap::new(); + // Fresh: trashed photos a new import replaces (by content or by file), each removed once + let (mut trash_hash, mut trash_path): (HashMap, HashMap) = Default::default(); + if opts.on_deleted == OnDeleted::Fresh { + for p in s.catalog.photos().filter(|p| p.deleted) { + if let Some(h) = &p.content_hash { + trash_hash.insert(h.clone(), p.id); + } + if let Source::File { path } = &p.source { + trash_path.insert(path.clone(), p.id); + } + } + } + let mut purged: std::collections::HashSet = Default::default(); for it in prepared.items { match it { PreparedItem::Promote(id) => { @@ -853,7 +901,16 @@ pub fn commit_prepared(s: &mut Session, opts: &ImportOptions, now: &str, prepare 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)) }); - report.duplicates.push(Duplicate { path, existing: existing.map(|i| i.0), reason }); + let trashed = existing.filter(|id| s.catalog.photo(*id).is_some_and(|p| p.deleted)); + match trashed { + Some(id) if opts.on_deleted == OnDeleted::Restore => { + if !report.restored.contains(&id.0) { + ops.push(Op::SetDeleted { id, deleted: false }); + report.restored.push(id.0); + } + } + _ => report.duplicates.push(Duplicate { path, existing: existing.map(|i| i.0), reason, existing_deleted: trashed.is_some() }), + } } PreparedItem::Failed(path, e) => report.failed.push((path, e)), PreparedItem::Kept(k) => report.kept.push(k), @@ -864,6 +921,11 @@ pub fn commit_prepared(s: &mut Session, opts: &ImportOptions, now: &str, prepare if let Some(h) = &info.content_hash { new_hash.insert(h.clone(), id); } + for old in info.content_hash.as_ref().and_then(|h| trash_hash.get(h)).into_iter().chain(trash_path.get(&stored)) { + if purged.insert(*old) { + ops.push(s.catalog.delete_permanently_ops(*old)); + } + } let sidecar = sidecar.map(|sc| sc.resolve_label(&s.catalog)).filter(|sc| *sc != crate::sidecar::SidecarData::default()); // a copy is catalogued under its new name let name = Path::new(&stored).file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_else(|| path.clone()); diff --git a/crates/engine/src/tests_import.rs b/crates/engine/src/tests_import.rs index af106765e..0853a7e9e 100644 --- a/crates/engine/src/tests_import.rs +++ b/crates/engine/src/tests_import.rs @@ -858,3 +858,76 @@ fn missing_photos_skip_local_browse_records() { s.execute("library.source", &json!({"kind": "missing"})).unwrap(); assert_eq!(s.visible_cloned().len(), 4, "the view lists what the count counts"); } + +/// Import a file that is in Recently Deleted (issue #298): by default it is skipped but the +/// report says it is in the trash; `onDeleted: restore` brings it back with its edits; +/// `onDeleted: fresh` replaces the trashed record with a new one. +fn trashed_photo(tag: &str) -> (Session, std::path::PathBuf, u64) { + let src = temp_dir(tag); + write_png(&src.join("a.png"), 1); + let mut s = Session::new().with_fs(); + let r = s.execute("library.import", &json!({"paths": [src.to_string_lossy()]})).unwrap(); + let id = r["imported"][0].as_u64().unwrap(); + s.execute("photo.rate", &json!({"ids": [id], "rating": 4})).unwrap(); + s.execute("photo.delete", &json!({"ids": [id]})).unwrap(); + (s, src, id) +} + +#[test] +fn duplicate_in_recently_deleted_is_reported_as_such() { + let (mut s, src, id) = trashed_photo("trash-report"); + let r = s.execute("library.import", &json!({"paths": [src.to_string_lossy()]})).unwrap(); + assert_eq!(ids(&r, "imported"), 0, "{r}"); + assert_eq!(r["duplicates"][0]["existing"], id, "{r}"); + assert_eq!(r["duplicates"][0]["existingDeleted"], true, "{r}"); + assert!(s.catalog.photo(lightcraft_catalog::PhotoId(id)).unwrap().deleted, "default: left in the trash"); + let _ = std::fs::remove_dir_all(&src); +} + +#[test] +fn import_can_restore_a_duplicate_from_recently_deleted() { + let (mut s, src, id) = trashed_photo("trash-restore"); + let r = s.execute("library.import", &json!({"paths": [src.to_string_lossy()], "onDeleted": "restore"})).unwrap(); + assert_eq!(r["restored"], json!([id]), "{r}"); + assert_eq!(ids(&r, "duplicates"), 0, "{r}"); + let p = s.catalog.photo(lightcraft_catalog::PhotoId(id)).unwrap(); + assert!(!p.deleted && p.rating == 4, "restored with its edits"); + let _ = std::fs::remove_dir_all(&src); +} + +#[test] +fn import_can_replace_a_duplicate_from_recently_deleted() { + let (mut s, src, id) = trashed_photo("trash-fresh"); + let r = s.execute("library.import", &json!({"paths": [src.to_string_lossy()], "onDeleted": "fresh"})).unwrap(); + assert_eq!(ids(&r, "imported"), 1, "{r}"); + let new = r["imported"][0].as_u64().unwrap(); + assert_ne!(new, id); + assert!(s.catalog.photo(lightcraft_catalog::PhotoId(id)).is_none(), "the trashed record is gone"); + let p = s.catalog.photo(lightcraft_catalog::PhotoId(new)).unwrap(); + assert!(!p.deleted && p.rating == 0, "a new photo without the old edits"); + assert_eq!(s.catalog.len(), 1); + let _ = std::fs::remove_dir_all(&src); +} + +#[test] +fn unknown_on_deleted_is_an_error() { + let mut s = Session::new().with_fs(); + let e = s.execute("library.import", &json!({"paths": [], "onDeleted": "bogus"})).unwrap_err().to_string(); + assert!(e.contains("onDeleted"), "{e}"); +} + +#[test] +fn empty_recently_deleted_removes_only_trashed_photos() { + let (mut s, src, id) = trashed_photo("trash-empty"); + write_png(&src.join("b.png"), 2); + let r = s.execute("library.import", &json!({"paths": [src.join("b.png").to_string_lossy()]})).unwrap(); + let keep = r["imported"][0].as_u64().unwrap(); + let r = s.execute("library.emptyRecentlyDeleted", &json!({})).unwrap(); + assert_eq!(r["deleted"], 1, "{r}"); + assert!(s.catalog.photo(lightcraft_catalog::PhotoId(id)).is_none()); + assert!(s.catalog.photo(lightcraft_catalog::PhotoId(keep)).is_some()); + // nothing to empty is not an error + assert_eq!(s.execute("library.emptyRecentlyDeleted", &json!({})).unwrap()["deleted"], 0); + s.execute("edit.undo", &json!({})).unwrap(); + let _ = std::fs::remove_dir_all(&src); +}