From 309d659b996b920b788cd409ff935c31bcb9bbf7 Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Fri, 9 Oct 2026 17:24:22 -0400 Subject: [PATCH] Catalog: folder records with colour labels (format version 4) Library folders can now carry what the library knows about them beyond their photos: FolderRecord (for now a colour label), kept in Catalog.folder_records under the folder's identity (folder_key) and changed through Op::SetFolderRecord, one undo step. An empty record is never kept; a path that names no folder is refused. folder_records_follow builds the ops that carry a folder's records and those of its subfolders to its new place on rename or move (dropping stale records left at the destination), so they can be committed with the photos' relinks. FolderNode reports the label. Co-Authored-By: Claude Opus 5.5 --- crates/catalog/src/folders.rs | 103 ++++++++++++- crates/catalog/src/journal.rs | 3 +- crates/catalog/src/lib.rs | 21 ++- crates/catalog/src/local.rs | 3 +- crates/catalog/src/tests_folder_records.rs | 167 +++++++++++++++++++++ crates/catalog/src/tests_format_version.rs | 18 ++- crates/catalog/src/tests_journal.rs | 1 + 7 files changed, 304 insertions(+), 12 deletions(-) create mode 100644 crates/catalog/src/tests_folder_records.rs diff --git a/crates/catalog/src/folders.rs b/crates/catalog/src/folders.rs index 61a8f8c4a..da21328ad 100644 --- a/crates/catalog/src/folders.rs +++ b/crates/catalog/src/folders.rs @@ -23,15 +23,21 @@ //! (`/Volumes/Macintosh HD/Users/me` and `/Users/me`, `/media/me/usb` and `/run/media/me/usb`) is //! two rows. Each row is still consistent: choosing it shows exactly the photos it counts. //! +//! A folder is otherwise only where photos are, but the library can also keep a +//! [`FolderRecord`] about it (its colour label), stored under the folder's identity +//! ([`crate::query::folder_key`]). A record follows its folder when the folder is renamed or moved +//! here ([`Catalog::folder_records_follow`]); one about a folder that no longer holds a photo is +//! kept (undo can bring the photos back) and shows again when the folder does. +//! //! This is the library's own view of its photos; browsing any folder on disk without importing //! is Local's job (see [`crate::local`]). use std::collections::{BTreeMap, HashMap}; -use serde::Serialize; +use serde::{Deserialize, Serialize}; -use crate::query::{folder_key, folder_within, split_path}; -use crate::{Catalog, Source}; +use crate::query::{folder_key, folder_rest, folder_within, key_within, split_path}; +use crate::{Catalog, ColorLabel, Op, Source}; /// Folder levels kept per photo. No real path comes near it; a deeper one is counted at its /// ancestor this many levels down, which bounds the tree's depth (and so every walk of it) @@ -58,9 +64,35 @@ pub struct FolderNode { /// so for the startup disk and for folders that hold other disks (`/Volumes`, `/mnt`…): their /// path covers those disks' photos too, so choosing them is not offered. pub selectable: bool, + /// The folder's colour label (see [`FolderRecord`]). + #[serde(skip_serializing_if = "Option::is_none")] + pub label: Option, pub children: Vec, } +/// What the library keeps about one of its folders beyond the photos in it. A record with +/// nothing in it is not kept. +#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct FolderRecord { + /// A colour label, as a photo has one. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub label: Option, +} + +impl FolderRecord { + /// Whether the record says nothing (and so is not kept). + pub fn is_empty(&self) -> bool { + self.label.is_none() + } +} + +/// The identity a folder's record is kept under: its [`folder_key`]; `None` for a path that +/// names no folder (empty, relative, `.`). +pub(crate) fn record_key(path: &str) -> Option { + let key = folder_key(path); + (split_path(path, true).absolute && !key.is_empty()).then_some(key) +} + /// A folder while the tree is being built (children by identity key). struct Level { name: String, @@ -151,6 +183,54 @@ pub fn folder_label(path: &str) -> String { } impl Catalog { + /// What the library keeps about folder `path`, however it is spelled. + pub fn folder_record(&self, path: &str) -> Option<&FolderRecord> { + record_key(path).and_then(|k| self.folder_records.get(&k)) + } + + /// The colour label of folder `path`, however it is spelled. + pub fn folder_color_label(&self, path: &str) -> Option { + self.folder_record(path).and_then(|r| r.label) + } + + /// The op that gives folder `path` colour label `label` (`None`: takes it off), keeping the + /// rest of its record. Applying it checks the path. + pub fn folder_label_op(&self, path: &str, label: Option) -> Op { + let mut record = self.folder_record(path).cloned().unwrap_or_default(); + record.label = label; + Op::SetFolderRecord { folder: path.to_string(), record: Some(record) } + } + + /// The ops that carry the records of folder `from` and the folders inside it over to `to` + /// (the same place below it), for a folder renamed or moved on disk: commit them with the + /// photos' relinks so undo puts both back. Records already at or below `to` are left from a + /// folder that is no longer there and are dropped (undo restores them). Empty when there is + /// nothing to carry. + pub fn folder_records_follow(&self, from: &str, to: &str) -> Vec { + let (Some(root), Some(dest)) = (record_key(from), record_key(to)) else { return Vec::new() }; + if root == dest { + return Vec::new(); + } + let moved: Vec<(String, FolderRecord)> = self + .folder_records + .iter() + .filter(|(k, _)| key_within(k, &root)) + .filter_map(|(k, r)| { + let rest = folder_rest(k, &root)?; + let base = dest.trim_end_matches('/'); + let path = if rest.is_empty() { dest.clone() } else { format!("{base}/{}", rest.join("/")) }; + Some((folder_key(&path), r.clone())) + }) + .collect(); + if moved.is_empty() { + return Vec::new(); + } + let stale = self.folder_records.keys().filter(|k| key_within(k, &root) || 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 + } + /// The library's volumes with their folders and photo counts: volumes by name /// (case-insensitive), each folder with its subfolders the same way. pub fn folder_tree(&self) -> Vec { @@ -196,7 +276,7 @@ impl Catalog { l.own += n; } } - let mut tree: Vec = roots.iter().filter_map(|k| build(k, &levels)).collect(); + let mut tree: Vec = roots.iter().filter_map(|k| build(k, &levels, &self.folder_records)).collect(); sort(&mut tree); // a folder that holds another disk's mount covers that disk's photos too let mounts: Vec = tree.iter().filter(|v| v.path != "/").map(|v| v.path.clone()).collect(); @@ -206,12 +286,21 @@ impl Catalog { } /// The node for `key` with everything below it (at most [`MAX_DEPTH`] deep). -fn build(key: &str, levels: &HashMap) -> Option { +fn build(key: &str, levels: &HashMap, records: &BTreeMap) -> Option { let l = levels.get(key)?; - let mut children: Vec = l.children.iter().filter_map(|k| build(k, levels)).collect(); + let mut children: Vec = l.children.iter().filter_map(|k| build(k, levels, records)).collect(); sort(&mut children); let count = l.own + children.iter().map(|c| c.count).sum::(); - Some(FolderNode { name: l.name.clone(), path: l.path.clone(), count, own: l.own, volume: l.volume, selectable: true, children }) + Some(FolderNode { + name: l.name.clone(), + path: l.path.clone(), + count, + own: l.own, + volume: l.volume, + selectable: true, + label: records.get(key).and_then(|r| r.label), + children, + }) } fn mark_selectable(nodes: &mut [FolderNode], mounts: &[String]) { diff --git a/crates/catalog/src/journal.rs b/crates/catalog/src/journal.rs index 5973975d6..008d034a2 100644 --- a/crates/catalog/src/journal.rs +++ b/crates/catalog/src/journal.rs @@ -46,6 +46,7 @@ //! | 1 | up to v0.2.0 | | //! | 2 | after v0.2.0 | `Op::SetBrowsed`, `Catalog.browsed`, `Photo.local_baseline` | //! | 3 | albums by hand | `Op::SetAlbumOrder`, `Album.order` | +//! | 4 | folder labels | `Op::SetFolderRecord`, `Catalog.folder_records` | //! //! Rules: //! - **Bump [`VERSION`]** (and add a row above) in the change that adds an [`Op`] variant or a @@ -70,7 +71,7 @@ pub const LOG: &str = "catalog.log"; const FORMAT: &str = "lightcraft-catalog"; /// The catalog format this build writes (and the newest it reads). See the module docs → /// *Format versions*; bump it whenever an [`Op`] variant or a serialized field is added. -pub const VERSION: u32 = 3; +pub const VERSION: u32 = 4; /// When [`Journal::wants_snapshot`] says it's time to compact the log. #[derive(Clone, Copy, Debug)] diff --git a/crates/catalog/src/lib.rs b/crates/catalog/src/lib.rs index 884ec6583..c8e2d7fae 100644 --- a/crates/catalog/src/lib.rs +++ b/crates/catalog/src/lib.rs @@ -30,7 +30,7 @@ use std::collections::BTreeMap; use std::sync::Arc; pub use dates::{DateRun, GroupBy}; -pub use folders::FolderNode; +pub use folders::{FolderNode, FolderRecord}; pub use journal::{Journal, LoadReport, PersistStats, SnapshotPolicy, SnapshotTiming}; pub use keywords::KeywordNode; use lightcraft_develop::DevelopSettings; @@ -217,6 +217,12 @@ pub enum Op { folder: String, at: Option, }, + /// What the library keeps about one of its folders (see [`folders::FolderRecord`]), under + /// its identity ([`query::folder_key`]); `None` or an empty record = keep nothing. + SetFolderRecord { + folder: String, + record: Option, + }, /// Several ops as one step (undo applies the inverses in reverse). Batch { ops: Vec, @@ -239,6 +245,9 @@ pub struct Catalog { /// When each Local folder was last browsed (folder path → ISO 8601), see [`local`]. #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] browsed: BTreeMap, + /// What the library keeps about its folders (folder identity → record), see [`folders`]. + #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] + folder_records: BTreeMap, /// Increments on every applied op. #[serde(skip)] pub revision: u64, @@ -620,6 +629,14 @@ impl Catalog { }; Op::SetBrowsed { folder, at: old } } + Op::SetFolderRecord { folder, record } => { + let key = folders::record_key(&folder).ok_or_else(|| CatalogError::Invalid(format!("not a folder: {folder}")))?; + let old = match record.filter(|r| !r.is_empty()) { + Some(r) => self.folder_records.insert(key.clone(), r), + None => self.folder_records.remove(&key), + }; + Op::SetFolderRecord { folder: key, record: old } + } Op::Batch { ops } => { let mut inverses = Vec::with_capacity(ops.len()); for op in ops { @@ -721,6 +738,8 @@ mod tests_album_order; #[cfg(test)] mod tests_background; #[cfg(test)] +mod tests_folder_records; +#[cfg(test)] mod tests_folders; #[cfg(test)] mod tests_format_version; diff --git a/crates/catalog/src/local.rs b/crates/catalog/src/local.rs index 4291f45fa..516a3322f 100644 --- a/crates/catalog/src/local.rs +++ b/crates/catalog/src/local.rs @@ -299,7 +299,8 @@ impl Op { | Op::SetAlbumRules { .. } | Op::RemoveStack { .. } | Op::SetLabelName { .. } - | Op::SetBrowsed { .. } => {} + | Op::SetBrowsed { .. } + | Op::SetFolderRecord { .. } => {} } } } diff --git a/crates/catalog/src/tests_folder_records.rs b/crates/catalog/src/tests_folder_records.rs new file mode 100644 index 000000000..4e172f47f --- /dev/null +++ b/crates/catalog/src/tests_folder_records.rs @@ -0,0 +1,167 @@ +//! What the library keeps about its folders: a colour label, like a photo's (see +//! [`crate::folders::FolderRecord`]). +//! +//! Scenarios, in the words of someone organising their library by folder: +//! +//! * Given a folder of the library, when I give it a colour label, its row in the folder tree +//! shows that label; the folders around it show none. +//! * However the folder is spelled, it is one folder with one label. +//! * When I take the label off, the library keeps nothing about the folder. +//! * Every change is one undo step: undo puts back the label the folder had before. +//! * A path that names no folder is refused and nothing changes. +//! * When the folder is renamed or moved, its label and those of the folders inside it go along; +//! a folder beside it whose name merely starts the same way keeps its own. Undo brings them +//! back, also a label that was left at the destination. +//! * Labels survive closing and reopening the library. + +use crate::*; + +fn add(c: &mut Catalog, path: &str) { + let id = c.alloc_photo_id(); + let name = path.rsplit('/').next().unwrap_or(path); + let p = Photo::new(id, Source::File { path: path.into() }, name, "JPEG", 60, 40, "2026-01-01T10:00:00"); + c.apply(Op::AddPhoto { photo: Box::new(p) }).unwrap(); +} + +fn library() -> Catalog { + let mut c = Catalog::new(); + for p in ["/pics/trip/a.jpg", "/pics/trip/day1/b.jpg", "/pics/trip2/c.jpg", "/pics/home/d.jpg"] { + add(&mut c, p); + } + c +} + +fn label(c: &mut Catalog, path: &str, l: Option) -> Op { + let op = c.folder_label_op(path, l); + c.apply(op).unwrap() +} + +/// The label of each row of the tree, by path, depth first (rows without one left out). +fn tree_labels(c: &Catalog) -> Vec<(String, ColorLabel)> { + fn walk(n: &FolderNode, out: &mut Vec<(String, ColorLabel)>) { + if let Some(l) = n.label { + out.push((n.path.clone(), l)); + } + n.children.iter().for_each(|c| walk(c, out)); + } + let mut out = Vec::new(); + c.folder_tree().iter().for_each(|n| walk(n, &mut out)); + out +} + +#[test] +fn a_new_library_knows_nothing_about_its_folders() { + let c = library(); + assert_eq!(c.folder_record("/pics/trip"), None); + assert_eq!(c.folder_color_label("/pics/trip"), None); + assert!(tree_labels(&c).is_empty()); +} + +#[test] +fn a_labelled_folder_shows_its_label_in_the_tree_and_only_there() { + let mut c = library(); + label(&mut c, "/pics/trip", Some(ColorLabel::Red)); + assert_eq!(c.folder_color_label("/pics/trip"), Some(ColorLabel::Red)); + assert_eq!(tree_labels(&c), vec![("/pics/trip".to_string(), ColorLabel::Red)]); +} + +#[test] +fn however_a_folder_is_spelled_it_has_one_label() { + let mut c = library(); + label(&mut c, "/pics//trip/", Some(ColorLabel::Green)); + assert_eq!(c.folder_color_label("/pics/./trip"), Some(ColorLabel::Green)); + label(&mut c, "/pics/home/../trip", Some(ColorLabel::Blue)); + assert_eq!(c.folder_color_label("/pics/trip"), Some(ColorLabel::Blue)); + assert_eq!(tree_labels(&c).len(), 1, "one folder, one record"); +} + +#[test] +fn taking_the_label_off_leaves_no_record() { + let mut c = library(); + label(&mut c, "/pics/trip", Some(ColorLabel::Purple)); + label(&mut c, "/pics/trip", None); + assert_eq!(c.folder_record("/pics/trip"), None); + assert!(!c.to_snapshot().contains("folder_records"), "nothing empty is saved"); +} + +#[test] +fn an_empty_record_is_no_record() { + let mut c = library(); + c.apply(Op::SetFolderRecord { folder: "/pics/trip".into(), record: Some(FolderRecord::default()) }).unwrap(); + assert_eq!(c.folder_record("/pics/trip"), None); +} + +#[test] +fn undo_puts_back_the_label_the_folder_had() { + let mut c = library(); + let undo_first = label(&mut c, "/pics/trip", Some(ColorLabel::Yellow)); + let undo_second = label(&mut c, "/pics/trip", Some(ColorLabel::Red)); + c.apply(undo_second).unwrap(); + assert_eq!(c.folder_color_label("/pics/trip"), Some(ColorLabel::Yellow)); + c.apply(undo_first).unwrap(); + assert_eq!(c.folder_record("/pics/trip"), None); +} + +#[test] +fn a_path_that_names_no_folder_is_refused() { + let mut c = library(); + let before = c.to_snapshot(); + for bad in ["", "relative/trip", ".", "a/.."] { + let op = Op::SetFolderRecord { folder: bad.into(), record: Some(FolderRecord { label: Some(ColorLabel::Red) }) }; + assert!(c.apply(op).is_err(), "{bad:?} names no folder"); + } + assert_eq!(c.to_snapshot(), before); +} + +#[test] +fn a_moved_folder_takes_its_labels_and_its_subfolders_labels_along() { + let mut c = library(); + label(&mut c, "/pics/trip", Some(ColorLabel::Red)); + label(&mut c, "/pics/trip/day1", Some(ColorLabel::Green)); + label(&mut c, "/pics/trip2", Some(ColorLabel::Blue)); + let ops = c.folder_records_follow("/pics/trip", "/archive/2026/trip"); + c.apply(Op::Batch { ops }).unwrap(); + assert_eq!(c.folder_record("/pics/trip"), None); + assert_eq!(c.folder_color_label("/archive/2026/trip"), Some(ColorLabel::Red)); + assert_eq!(c.folder_color_label("/archive/2026/trip/day1"), Some(ColorLabel::Green)); + assert_eq!(c.folder_color_label("/pics/trip2"), Some(ColorLabel::Blue), "a neighbour that only starts the same stays"); +} + +#[test] +fn undoing_a_move_brings_back_every_label_also_one_left_at_the_destination() { + let mut c = library(); + label(&mut c, "/pics/trip", Some(ColorLabel::Red)); + // a folder that once was at the destination left its labels behind + label(&mut c, "/pics/holiday", Some(ColorLabel::Purple)); + label(&mut c, "/pics/holiday/day9", Some(ColorLabel::Yellow)); + let before = c.to_snapshot(); + let ops = c.folder_records_follow("/pics/trip", "/pics/holiday"); + let undo = c.apply(Op::Batch { ops }).unwrap(); + assert_eq!(c.folder_color_label("/pics/holiday"), Some(ColorLabel::Red), "the moved folder's label wins"); + assert_eq!(c.folder_record("/pics/holiday/day9"), None, "what was known about the old folder goes"); + c.apply(undo).unwrap(); + assert_eq!(c.to_snapshot(), before); +} + +#[test] +fn a_folder_without_labels_moves_without_ops() { + let c = library(); + assert!(c.folder_records_follow("/pics/trip", "/elsewhere/trip").is_empty()); +} + +#[test] +fn labels_survive_closing_and_reopening_the_library() { + let mut c = library(); + label(&mut c, "/pics/home", Some(ColorLabel::Green)); + let reopened = Catalog::from_snapshot(&c.to_snapshot()).unwrap(); + assert_eq!(reopened.folder_color_label("/pics/home"), Some(ColorLabel::Green)); +} + +#[test] +fn a_library_saved_before_folder_records_opens_without_any() { + let c = library(); + let old = c.to_snapshot(); + assert!(!old.contains("folder_records")); + let reopened = Catalog::from_snapshot(&old).unwrap(); + assert_eq!(reopened.folder_record("/pics/trip"), None); +} diff --git a/crates/catalog/src/tests_format_version.rs b/crates/catalog/src/tests_format_version.rs index cb80ce67e..5b7732bf5 100644 --- a/crates/catalog/src/tests_format_version.rs +++ b/crates/catalog/src/tests_format_version.rs @@ -57,7 +57,20 @@ fn v2_library_loads_and_is_upgraded() { let (_, c, r) = Journal::open(Box::new(m.clone())).unwrap(); assert_eq!(c.to_snapshot(), full.to_snapshot()); assert_eq!((r.replayed, r.upgraded_from), (1, Some(2))); - assert_eq!(snapshot_version(&m), 3); + assert_eq!(snapshot_version(&m), u64::from(VERSION)); +} + +/// A library saved before folders could be labelled opens as it was and is rewritten as v4. +#[test] +fn v3_library_loads_and_is_upgraded() { + let (base, log, full) = legacy_parts(); + let m = MemStore::new(); + m.set(SNAPSHOT, format!("{{\"format\":\"lightcraft-catalog\",\"version\":3,\"seq\":2,\"catalog\":{}}}\n", base.to_snapshot()).into_bytes()); + m.set(LOG, log.into_bytes()); + let (_, c, r) = Journal::open(Box::new(m.clone())).unwrap(); + assert_eq!(c.to_snapshot(), full.to_snapshot()); + assert_eq!((r.replayed, r.upgraded_from), (1, Some(3))); + assert_eq!(snapshot_version(&m), 4); } #[test] @@ -193,8 +206,9 @@ fn op_variants_are_versioned() { | Op::Batch { .. } => 1, Op::SetBrowsed { .. } => 2, Op::SetAlbumOrder { .. } => 3, + Op::SetFolderRecord { .. } => 4, } } - let newest = since(&Op::SetAlbumOrder { id: AlbumId(0), order: None }); + let newest = since(&Op::SetFolderRecord { folder: "/".into(), record: None }); assert_eq!(newest, VERSION, "the newest op's version must be the current format version"); } diff --git a/crates/catalog/src/tests_journal.rs b/crates/catalog/src/tests_journal.rs index 0594b6d83..69cee6b0a 100644 --- a/crates/catalog/src/tests_journal.rs +++ b/crates/catalog/src/tests_journal.rs @@ -78,6 +78,7 @@ fn op_for(c: &mut Catalog, kind: u8, a: u8, b: u8) -> Op { 13 => Op::Batch { ops: c.remove_from_stacks_ops(&[pid]) }, 14 => c.set_top_ops(pid).unwrap_or(Op::Batch { ops: vec![] }), 9 if b.is_multiple_of(5) => Op::SetLabelName { label: ColorLabel::ALL[a as usize % 5], name: (a % 3 != 1).then(|| format!("Label {b}")) }, + 9 if b % 11 == 4 => c.folder_label_op(&format!("/p/f{}", a % 4), ColorLabel::ALL.get(usize::from(b % 6)).copied()), 9 if b % 7 == 3 => Op::SetAlbumOrder { id: aid, order: (!b.is_multiple_of(3)).then_some(u32::from(b % 5)) }, 9 if b.is_multiple_of(2) => { Op::SetFile { id: pid, file_name: format!("Renamed_{b}.jpg"), source: Source::File { path: format!("/p/Renamed_{b}.jpg") } }