From a9be2e14f31b4346837a74c07ec2b93cccbdfe26 Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Thu, 22 Oct 2026 14:13:25 -0400 Subject: [PATCH 1/2] LRC-LIB-LIBFOLDERS: catalog queries and streamed file hashing for locating a folder (#1112) Two catalog additions and one internal change, with nothing calling the new ones yet: - Catalog::folder_records_located: folder_records_follow for a folder located where it already is on disk (records of library folders at the destination stay). - Catalog::local_records_of: the Local records of some files, split into those that can give way and those that hold something of the person's. - Find Missing Photos hashes a file in 64 KiB pieces instead of reading it whole (the same hash). --- crates/catalog/src/folders.rs | 14 ++++- crates/catalog/src/lib.rs | 2 +- crates/catalog/src/local.rs | 37 +++++++++++ crates/catalog/src/tests_folder_records.rs | 38 ++++++++++++ crates/catalog/src/tests_local.rs | 35 +++++++++++ crates/engine/src/cmd/missing.rs | 72 +++++++++++++++++++++- 6 files changed, 194 insertions(+), 4 deletions(-) diff --git a/crates/catalog/src/folders.rs b/crates/catalog/src/folders.rs index aab3b867b..e8637ceca 100644 --- a/crates/catalog/src/folders.rs +++ b/crates/catalog/src/folders.rs @@ -207,6 +207,18 @@ impl Catalog { /// folder that is no longer there and are dropped (undo restores them). Empty when there is /// nothing to carry or drop. pub fn folder_records_follow(&self, from: &str, to: &str) -> Vec { + self.folder_records_carried(from, to, true) + } + + /// [`Catalog::folder_records_follow`] for a folder located where it already is on disk + /// (`folder.updateLocation`): nothing arrived at `to`, the library only learned that the + /// folder is there. So what it knows about folders at or below `to` (other folders of the + /// library there) stays; a record carried over replaces the one at the same place. + pub fn folder_records_located(&self, from: &str, to: &str) -> Vec { + self.folder_records_carried(from, to, false) + } + + fn folder_records_carried(&self, from: &str, to: &str, clear_destination: bool) -> Vec { let (Some(root), Some(dest)) = (record_key(from), record_key(to)) else { return Vec::new() }; if root == dest { return Vec::new(); @@ -224,7 +236,7 @@ impl Catalog { .collect(); // what was known at the old place goes, and so does what is left at the new one (also // when the moved folder carries nothing: the folder now there is not the one it was about) - let stale = self.folder_records.keys().filter(|k| key_within(k, &root) || key_within(k, &dest)); + let stale = self.folder_records.keys().filter(|k| key_within(k, &root) || (clear_destination && key_within(k, &dest))); let mut ops: Vec = stale.map(|k| Op::SetFolderRecord { folder: k.clone(), record: None }).collect(); ops.extend(moved.into_iter().map(|(k, r)| Op::SetFolderRecord { folder: k, record: Some(r) })); ops diff --git a/crates/catalog/src/lib.rs b/crates/catalog/src/lib.rs index 1776c895c..09bba131f 100644 --- a/crates/catalog/src/lib.rs +++ b/crates/catalog/src/lib.rs @@ -36,7 +36,7 @@ pub use folders::{FolderNode, FolderRecord}; pub use journal::{Journal, LoadReport, PersistStats, SnapshotPolicy, SnapshotTiming}; pub use keywords::KeywordNode; use lightcraft_develop::DevelopSettings; -pub use local::{DEFAULT_FORGET_DAYS, ForgetPlan, folder_of}; +pub use local::{DEFAULT_FORGET_DAYS, ForgetPlan, LocalRecords, folder_of}; pub use lock::{LibraryLock, LockError, LockOwner}; pub use model::*; pub use query::{DateGroup, Filter, Person, RatingOp, Sort, SortKey, mix64}; diff --git a/crates/catalog/src/local.rs b/crates/catalog/src/local.rs index 093c22427..70d8ea44a 100644 --- a/crates/catalog/src/local.rs +++ b/crates/catalog/src/local.rs @@ -34,6 +34,15 @@ use serde::Serialize; use crate::{Catalog, Op, Photo, PhotoId, Source}; +/// The Local records of some files ([`Catalog::local_records_of`]). +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct LocalRecords { + /// Only a record of having looked: nothing was changed on them and nothing refers to them. + pub untouched: Vec, + /// Holding something of the person's, with their files. + pub kept: Vec<(PhotoId, String)>, +} + /// Default for how long a Local folder may go unbrowsed before its untouched records are /// forgotten (days). pub const DEFAULT_FORGET_DAYS: u32 = 30; @@ -176,6 +185,34 @@ impl Catalog { &self.browsed } + /// The Local records of `files` (paths as stored), for library photos about to point at + /// those files (a folder located where it was browsed, `folder.updateLocation`): a file has one + /// record, so the Local one gives way. Told apart: those that are only a record of having + /// looked (untouched, see the module docs, and nothing refers to them), which can go, and + /// those that hold something of the person's (a rating, an edit, a place in an album), with + /// their files. Records in Recently Deleted are not Local records to this. + pub fn local_records_of(&self, files: &HashSet) -> LocalRecords { + let mut out = LocalRecords::default(); + if files.is_empty() { + return out; + } + let referenced = self.referenced(); + for p in self.photos.values().filter(|p| p.local && !p.deleted) { + let Source::File { path } = &p.source else { continue }; + if !files.contains(path) { + continue; + } + if p.untouched_local() && !referenced.contains(&p.id) { + out.untouched.push(p.id); + } else { + out.kept.push((p.id, path.clone())); + } + } + out.untouched.sort_unstable(); + out.kept.sort_unstable(); + out + } + /// Plan forgetting untouched Local records whose folder wasn't browsed for `days` days (as of /// `now`, ISO 8601). `in_use` keeps records the caller still needs. `days == 0` (never) or an /// unreadable `now` forgets nothing. See the module docs. diff --git a/crates/catalog/src/tests_folder_records.rs b/crates/catalog/src/tests_folder_records.rs index 3ae19c33a..eb2215ba6 100644 --- a/crates/catalog/src/tests_folder_records.rs +++ b/crates/catalog/src/tests_folder_records.rs @@ -161,6 +161,44 @@ fn an_unlabelled_folder_moved_onto_a_stale_label_does_not_inherit_it() { assert_eq!(c.folder_color_label("/pics/holiday"), Some(ColorLabel::Purple)); } +#[test] +fn a_located_folder_keeps_what_the_library_knows_of_the_folders_already_at_the_destination() { + let mut c = library(); + label(&mut c, "/pics/trip", Some(ColorLabel::Red)); + label(&mut c, "/pics/trip/day1", Some(ColorLabel::Green)); + // other folders of the library at the place it is said to be at: nothing arrived there + label(&mut c, "/pics/home", Some(ColorLabel::Blue)); + label(&mut c, "/pics/home/old", Some(ColorLabel::Yellow)); + let before = c.to_snapshot(); + let ops = c.folder_records_located("/pics/trip", "/pics/home"); + let undo = c.apply(Op::Batch { ops }).unwrap(); + assert_eq!(c.folder_record("/pics/trip"), None, "what was known at the old place goes"); + assert_eq!(c.folder_record("/pics/trip/day1"), None); + assert_eq!(c.folder_color_label("/pics/home"), Some(ColorLabel::Red), "a record carried over replaces the one at the same place"); + assert_eq!(c.folder_color_label("/pics/home/old"), Some(ColorLabel::Yellow), "a folder of the library there keeps its own"); + c.apply(undo).unwrap(); + assert_eq!(c.to_snapshot(), before); +} + +#[test] +fn a_located_folder_with_labels_inside_lands_them_at_the_same_places_below() { + let mut c = library(); + label(&mut c, "/pics/trip", Some(ColorLabel::Red)); + label(&mut c, "/pics/trip/day1", Some(ColorLabel::Green)); + let ops = c.folder_records_located("/pics/trip", "/mnt/nas/trip"); + c.apply(Op::Batch { ops }).unwrap(); + assert_eq!(c.folder_color_label("/mnt/nas/trip"), Some(ColorLabel::Red)); + assert_eq!(c.folder_color_label("/mnt/nas/trip/day1"), Some(ColorLabel::Green)); +} + +#[test] +fn a_folder_located_where_it_is_or_without_labels_gives_no_ops() { + let mut c = library(); + assert!(c.folder_records_located("/pics/trip", "/elsewhere/trip").is_empty()); + label(&mut c, "/pics/trip", Some(ColorLabel::Red)); + assert!(c.folder_records_located("/pics/trip", "/pics/trip").is_empty()); +} + #[test] fn labels_survive_the_journal_without_a_snapshot() { let m = MemStore::new(); diff --git a/crates/catalog/src/tests_local.rs b/crates/catalog/src/tests_local.rs index 4b76dace6..f89d326d0 100644 --- a/crates/catalog/src/tests_local.rs +++ b/crates/catalog/src/tests_local.rs @@ -1,5 +1,6 @@ //! Forgetting untouched Local browse records (see [`crate::local`]). +use std::collections::HashSet; use std::sync::Arc; use lightcraft_develop::DevelopSettings; @@ -265,3 +266,37 @@ fn op_photo_ids_cover_nested_ops() { op.photo_ids(&mut |id| ids.push(id.0)); assert_eq!(ids, vec![1, 2, 3, 4]); } + +fn paths(folder: &str, ns: &[u32]) -> HashSet { + ns.iter().map(|i| format!("/pics/{folder}/IMG_{i}.JPG")).collect() +} + +#[test] +fn local_records_of_files_tell_what_can_give_way_from_what_holds_something() { + let mut c = Catalog::new(); + let untouched = local(&mut c, "trip", 1); + let rated = local(&mut c, "trip", 2); + let in_album = local(&mut c, "trip", 3); + let _elsewhere = local(&mut c, "trip", 4); + c.apply(Op::SetRating { id: rated, rating: 3 }).unwrap(); + let a = c.alloc_album_id(); + let mut album = Album::new(a, "A"); + album.photos.push(in_album); + c.apply(Op::AddAlbum { album }).unwrap(); + + let found = c.local_records_of(&paths("trip", &[1, 2, 3])); + assert_eq!(found.untouched, vec![untouched]); + assert_eq!(found.kept, vec![(rated, "/pics/trip/IMG_2.JPG".to_string()), (in_album, "/pics/trip/IMG_3.JPG".to_string())]); +} + +#[test] +fn local_records_of_files_leave_out_library_photos_and_recently_deleted_ones() { + let mut c = Catalog::new(); + let gone = local(&mut c, "trip", 1); + c.apply(Op::SetDeleted { id: gone, deleted: true }).unwrap(); + let id = c.alloc_photo_id(); + let p = Photo::new(id, Source::File { path: "/pics/trip/IMG_2.JPG".into() }, "IMG_2.JPG", "JPEG", 60, 40, OLD); + c.apply(Op::AddPhoto { photo: Box::new(p) }).unwrap(); + assert_eq!(c.local_records_of(&paths("trip", &[1, 2])), LocalRecords::default()); + assert_eq!(c.local_records_of(&HashSet::new()), LocalRecords::default()); +} diff --git a/crates/engine/src/cmd/missing.rs b/crates/engine/src/cmd/missing.rs index 1bff30d2b..29decc4f3 100644 --- a/crates/engine/src/cmd/missing.rs +++ b/crates/engine/src/cmd/missing.rs @@ -90,9 +90,77 @@ fn stored_hash(p: &Photo) -> Option<&str> { p.content_hash.as_deref().map(|h| h.split(':').next().unwrap_or(h)).filter(|h| !h.is_empty()) } -/// The content hash of the file at `path` (as import computes it), if it can be read. +/// The content hash of the file at `path` (as import computes it), if it can be read. Read in +/// pieces: the file is whatever is on the disk, of any size. fn file_hash(path: &str) -> Option { - std::fs::read(path).ok().map(|b| lightcraft_preview::hash_bytes(&b).to_string()) + hash_reader(std::fs::File::open(path).ok()?) +} + +/// The content hash of what `r` gives, read in pieces (the hash import computes of the whole). +fn hash_reader(mut r: impl std::io::Read) -> Option { + let mut hasher = lightcraft_preview::Hasher128::new(); + let mut piece = vec![0u8; 1 << 16]; + loop { + match r.read(&mut piece) { + Ok(0) => return Some(hasher.finish().to_string()), + Ok(n) => { + hasher.update(piece.get(..n)?); + } + Err(e) if e.kind() == std::io::ErrorKind::Interrupted => {} + Err(_) => return None, + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// A reader that gives its bytes a few at a time, and an interruption in between. + struct Trickle<'a>(&'a [u8], bool); + + impl std::io::Read for Trickle<'_> { + fn read(&mut self, buf: &mut [u8]) -> std::io::Result { + self.1 = !self.1; + if self.1 { + return Err(std::io::ErrorKind::Interrupted.into()); + } + let n = self.0.len().min(buf.len()).min(7); + buf[..n].copy_from_slice(&self.0[..n]); + self.0 = &self.0[n..]; + Ok(n) + } + } + + #[test] + fn a_hash_read_in_pieces_is_the_hash_of_the_whole() { + // longer than one piece, so the read loop runs more than once + let bytes: Vec = (0..200_000u32).map(|i| (i % 251) as u8).collect(); + let whole = lightcraft_preview::hash_bytes(&bytes).to_string(); + assert_eq!(hash_reader(bytes.as_slice()).as_deref(), Some(whole.as_str())); + assert_eq!(hash_reader(Trickle(&bytes[..1000], false)).as_deref(), Some(lightcraft_preview::hash_bytes(&bytes[..1000]).to_string().as_str())); + assert_eq!(hash_reader(std::io::empty()).as_deref(), Some(lightcraft_preview::hash_bytes(&[]).to_string().as_str())); + } + + /// A scratch folder that goes away with the test, however it ends. + struct Scratch(std::path::PathBuf); + + impl Drop for Scratch { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } + } + + #[test] + fn a_file_hash_is_the_import_hash_and_a_missing_file_has_none() { + let dir = Scratch(std::env::temp_dir().join(format!("lc-file-hash-{}", std::process::id()))); + let _ = std::fs::remove_dir_all(&dir.0); + std::fs::create_dir_all(&dir.0).unwrap(); + let f = dir.0.join("a.bin"); + std::fs::write(&f, b"pixels").unwrap(); + assert_eq!(file_hash(f.to_str().unwrap()), Some(lightcraft_preview::hash_bytes(b"pixels").to_string())); + assert_eq!(file_hash(dir.0.join("nope").to_str().unwrap()), None); + } } /// An in-scope photo ([`checked_path`]) as Find Missing Photos sees it: its file, the size and From 5d24e34ed58f01369896c8dbd68d69394804d5dc Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Thu, 22 Oct 2026 15:32:44 -0400 Subject: [PATCH 2/2] LRC-LIB-LIBFOLDERS: a folder located below itself leaves the records already at its destination in place --- crates/catalog/src/folders.rs | 9 +++++++-- crates/catalog/src/tests_folder_records.rs | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/crates/catalog/src/folders.rs b/crates/catalog/src/folders.rs index e8637ceca..e2d3b312d 100644 --- a/crates/catalog/src/folders.rs +++ b/crates/catalog/src/folders.rs @@ -223,10 +223,15 @@ impl Catalog { if root == dest { return Vec::new(); } + // a folder located below itself: what is already at or below `to` belongs to the folders + // of the library there, not to the folder located, so it is neither carried nor cleared + // (the record of `from` itself still lands at `to`, replacing the one there) + let nested = !clear_destination && key_within(&dest, &root); + let carried = |k: &str| key_within(k, &root) && !(nested && key_within(k, &dest)); let moved: Vec<(String, FolderRecord)> = self .folder_records .iter() - .filter(|(k, _)| key_within(k, &root)) + .filter(|(k, _)| carried(k)) .filter_map(|(k, r)| { let rest = folder_rest(k, &root)?; let base = dest.trim_end_matches('/'); @@ -236,7 +241,7 @@ impl Catalog { .collect(); // what was known at the old place goes, and so does what is left at the new one (also // when the moved folder carries nothing: the folder now there is not the one it was about) - let stale = self.folder_records.keys().filter(|k| key_within(k, &root) || (clear_destination && key_within(k, &dest))); + let stale = self.folder_records.keys().filter(|k| carried(k) || (clear_destination && key_within(k, &dest))); let mut ops: Vec = stale.map(|k| Op::SetFolderRecord { folder: k.clone(), record: None }).collect(); ops.extend(moved.into_iter().map(|(k, r)| Op::SetFolderRecord { folder: k, record: Some(r) })); ops diff --git a/crates/catalog/src/tests_folder_records.rs b/crates/catalog/src/tests_folder_records.rs index eb2215ba6..a572db995 100644 --- a/crates/catalog/src/tests_folder_records.rs +++ b/crates/catalog/src/tests_folder_records.rs @@ -191,6 +191,25 @@ fn a_located_folder_with_labels_inside_lands_them_at_the_same_places_below() { assert_eq!(c.folder_color_label("/mnt/nas/trip/day1"), Some(ColorLabel::Green)); } +#[test] +fn a_folder_located_below_itself_leaves_the_records_of_the_folders_already_there_in_place() { + let mut c = library(); + label(&mut c, "/pics/trip", Some(ColorLabel::Red)); + label(&mut c, "/pics/trip/day1", Some(ColorLabel::Green)); + // another folder of the library already sits at the place the folder is said to be at + label(&mut c, "/pics/trip/day1/raw", Some(ColorLabel::Blue)); + let before = c.to_snapshot(); + let ops = c.folder_records_located("/pics/trip", "/pics/trip/day1"); + let undo = c.apply(Op::Batch { ops }).unwrap(); + assert_eq!(c.folder_record("/pics/trip"), None, "what was known at the old place goes"); + assert_eq!(c.folder_color_label("/pics/trip/day1"), Some(ColorLabel::Red), "a record carried over replaces the one at the same place"); + assert_eq!(c.folder_color_label("/pics/trip/day1/raw"), Some(ColorLabel::Blue), "a folder below the destination keeps its own"); + assert_eq!(c.folder_record("/pics/trip/day1/day1"), None, "the record at the destination is not carried below itself"); + assert_eq!(c.folder_record("/pics/trip/day1/day1/raw"), None); + c.apply(undo).unwrap(); + assert_eq!(c.to_snapshot(), before); +} + #[test] fn a_folder_located_where_it_is_or_without_labels_gives_no_ops() { let mut c = library();