From ce9fa94cb4d29b33e8af60976c19a100ac309c43 Mon Sep 17 00:00:00 2001 From: Cameron Cordes Date: Wed, 13 May 2026 08:58:04 -0400 Subject: [PATCH] libraries: surface globals, normalise excluded_dirs on write MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two follow-ups to the PATCH endpoint: 1. GET /libraries now returns ``global_excluded_dirs`` alongside the library list — the union-with-globals semantics is invisible from the per-library row alone, and the admin UI needs to show what's already being skipped before the operator adds entries that would duplicate. 2. PATCH /libraries/{id} canonicalises the excluded_dirs string on write via the new ``normalize_excluded_dirs_input``: trims per entry, drops empties, dedupes preserving first-occurrence order, comma-joins without inner whitespace. Empty / whitespace-only → NULL. Round-trip stable so re-saving an entry produces an identical row. Five new tests cover the empty / whitespace, trim, dedup, round-trip, and overlap-with-globals cases. effective_excluded_dirs continues to keep overlapping entries between globals and per-library on purpose — PathExcluder accepts repeats and there's no behavioural reason to dedupe at merge time. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/libraries.rs | 129 +++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 118 insertions(+), 11 deletions(-) diff --git a/src/libraries.rs b/src/libraries.rs index 67c908b..094c5ee 100644 --- a/src/libraries.rs +++ b/src/libraries.rs @@ -82,7 +82,9 @@ impl Library { /// Parse a comma-separated excluded_dirs column into a Vec, dropping /// empty entries (mirrors `AppState::parse_excluded_dirs` for the env -/// var). NULL → empty Vec. +/// var). NULL → empty Vec. Duplicates are preserved — `PathExcluder` +/// accepts repeats, and the storage-side normaliser is where dedup +/// happens. pub fn parse_excluded_dirs_column(raw: Option<&str>) -> Vec { match raw { None => Vec::new(), @@ -95,6 +97,32 @@ pub fn parse_excluded_dirs_column(raw: Option<&str>) -> Vec { } } +/// Canonicalise an excluded_dirs string for storage: parse → trim → +/// dedupe (preserving insertion order) → comma-join with no inner +/// whitespace. Empty / whitespace-only input → `None` (writes NULL). +/// +/// Used by `PATCH /libraries/{id}` so two users typing the same entries +/// in different orders / casings / whitespace land on the same stored +/// form, and a typo'd duplicate (`@eaDir, @eaDir`) collapses on save. +/// Round-trip stable: writing the output back through this function +/// yields the same string. +pub fn normalize_excluded_dirs_input(raw: &str) -> Option { + let parsed = parse_excluded_dirs_column(Some(raw)); + if parsed.is_empty() { + return None; + } + let mut seen = std::collections::HashSet::new(); + let deduped: Vec = parsed + .into_iter() + .filter(|s| seen.insert(s.clone())) + .collect(); + if deduped.is_empty() { + None + } else { + Some(deduped.join(",")) + } +} + impl From for Library { fn from(row: LibraryRow) -> Self { Library { @@ -335,6 +363,13 @@ pub struct LibraryStatus { #[derive(serde::Serialize)] pub struct LibrariesResponse { pub libraries: Vec, + /// Globally-excluded paths/patterns from the `EXCLUDED_DIRS` env var. + /// Applied **in union** with each library's own `excluded_dirs`. Surfaced + /// here so an admin UI can show the operator "you already skip these + /// everywhere" before they add per-library entries that would duplicate + /// the global list. Read-only — globals live in `.env` and aren't + /// mutable via the API today. + pub global_excluded_dirs: Vec, } #[get("/libraries")] @@ -361,7 +396,10 @@ pub async fn list_libraries(_claims: Claims, app_state: Data) -> impl .unwrap_or(LibraryHealth::Online), }) .collect(); - HttpResponse::Ok().json(LibrariesResponse { libraries }) + HttpResponse::Ok().json(LibrariesResponse { + libraries, + global_excluded_dirs: app_state.excluded_dirs.clone(), + }) } /// Body for PATCH /libraries/{id}. Both fields are optional — omitting @@ -416,15 +454,12 @@ pub async fn patch_library( } } if let Some(raw) = body.excluded_dirs.as_deref() { - let trimmed = raw.trim(); - // Empty / whitespace-only → NULL so the column reads back the - // same way a never-set library does (parse_excluded_dirs_column - // returns Vec::new() for NULL). - let stored: Option<&str> = if trimmed.is_empty() { - None - } else { - Some(trimmed) - }; + // Canonicalise on write — trim, dedupe, drop empties — so the DB + // stores a round-trip-stable form regardless of how messy the + // user typed it. Empty / whitespace-only → NULL (matches a + // never-set library, parse_excluded_dirs_column returns []). + let normalised = normalize_excluded_dirs_input(raw); + let stored: Option<&str> = normalised.as_deref(); match diesel::update(libraries::table.filter(libraries::id.eq(lib_id))) .set(libraries::excluded_dirs.eq(stored)) .execute(&mut conn) @@ -637,6 +672,78 @@ mod tests { assert_eq!(combined.len(), 3); } + #[test] + fn effective_excluded_dirs_keeps_overlap_between_global_and_per_library() { + // Two sources both excluding `@eaDir` is legal — `PathExcluder` + // accepts repeats, and there's no behavioral reason to dedupe + // here. Documents the design choice so a future refactor that + // tightens this is forced to update both code and tests. + let globals = vec!["@eaDir".to_string()]; + let lib = Library { + id: 1, + name: "main".into(), + root_path: "/x".into(), + enabled: true, + excluded_dirs: vec!["@eaDir".to_string(), "/private".to_string()], + }; + let combined = lib.effective_excluded_dirs(&globals); + // 2 occurrences of @eaDir + /private = 3 entries total. + assert_eq!(combined, vec!["@eaDir", "@eaDir", "/private"]); + } + + #[test] + fn normalize_excluded_dirs_input_handles_empty_and_whitespace() { + assert_eq!(normalize_excluded_dirs_input(""), None); + assert_eq!(normalize_excluded_dirs_input(" "), None); + assert_eq!(normalize_excluded_dirs_input(",,,"), None); + assert_eq!(normalize_excluded_dirs_input(" , , "), None); + } + + #[test] + fn normalize_excluded_dirs_input_trims_per_entry() { + // Inner whitespace stripped on each item, comma-joined without + // spaces. Mirrors how parse_excluded_dirs_column reads it back. + assert_eq!( + normalize_excluded_dirs_input(" @eaDir , /private , .thumbnails "), + Some("@eaDir,/private,.thumbnails".to_string()) + ); + } + + #[test] + fn normalize_excluded_dirs_input_dedupes_preserving_first_occurrence() { + // Exact-string duplicates collapse; the first occurrence wins + // (preserves the operator's typed order so they recognise their + // intent on round-trip). + assert_eq!( + normalize_excluded_dirs_input("@eaDir, /private, @eaDir, /private"), + Some("@eaDir,/private".to_string()) + ); + // Whitespace-distinct entries collapse to the same canonical + // form. Case is preserved — `Foo` and `foo` are different keys + // (filesystem case-sensitivity is platform-dependent; we don't + // make that call here). + assert_eq!( + normalize_excluded_dirs_input(" Foo,foo, Foo "), + Some("Foo,foo".to_string()) + ); + } + + #[test] + fn normalize_excluded_dirs_input_is_round_trip_stable() { + // Writing the normaliser's output back through it yields the + // same string. PATCH-clearing edits round-trip cleanly through + // parse_excluded_dirs_column too. + let raw = " /a/b ,, /a/b , c "; + let once = normalize_excluded_dirs_input(raw).expect("not empty"); + let twice = normalize_excluded_dirs_input(&once).expect("not empty"); + assert_eq!(once, twice); + // Parsing the stored form back gives the deduped Vec. + assert_eq!( + parse_excluded_dirs_column(Some(&once)), + vec!["/a/b".to_string(), "c".to_string()] + ); + } + fn probe_lib(id: i32, root: String) -> Library { Library { id,