From c01a0479b7b44f7c220c61b5d6afa9c2cc59f02b Mon Sep 17 00:00:00 2001 From: Cameron Date: Fri, 17 Apr 2026 17:16:11 -0400 Subject: [PATCH] fix: honor library param in /image, /photos, /memories The Phase 3 plumbing accepted `library=` but didn't actually route requests through the scoped library once it was resolved. Three concrete bugs surfaced when testing against a second mounted library: - `/image` always resolved paths against AppState.base_path (primary), so thumbnails for non-primary libraries 400'd when their rel_paths didn't exist under primary. Now resolves against the scoped library and defaults to primary when the param is omitted. - `/memories` walked the scoped library correctly but its helper functions hardcoded `library_id: PRIMARY_LIBRARY_ID` on every MemoryItem, causing clients to route thumbnails back to primary regardless of which library the memory actually came from. - `/photos` non-recursive listing delegated to a `RealFileSystem` constructed from AppState.base_path at startup, so walks always hit primary even when `library=2` was passed. The non-primary path now uses list_files against the scoped library's root; primary still goes through FileSystemAccess to preserve the existing test mock plumbing. Also adds `library` to ThumbnailRequest so the /image query param is actually parsed. Co-Authored-By: Claude Opus 4.7 --- src/data/mod.rs | 4 ++++ src/files.rs | 40 ++++++++++++++++++++++++++++------------ src/main.rs | 20 +++++++++++++++++--- src/memories.rs | 14 +++++++++----- 4 files changed, 58 insertions(+), 20 deletions(-) diff --git a/src/data/mod.rs b/src/data/mod.rs index 2aedbae..ff9ac25 100644 --- a/src/data/mod.rs +++ b/src/data/mod.rs @@ -192,6 +192,10 @@ pub struct ThumbnailRequest { pub(crate) format: Option, #[serde(default)] pub(crate) shape: Option, + /// Optional library filter. Accepts a library id (e.g. "1") or name + /// (e.g. "main"). When omitted, defaults to the primary library. + #[serde(default)] + pub(crate) library: Option, } #[derive(Debug, Deserialize, PartialEq)] diff --git a/src/files.rs b/src/files.rs index acb8fc5..552fb58 100644 --- a/src/files.rs +++ b/src/files.rs @@ -422,7 +422,7 @@ pub async fn list_photos( sort_type, &mut exif_dao_guard, &span_context, - app_state.base_path.as_ref(), + scoped_library.root_path.as_ref(), limit, offset, ); @@ -473,11 +473,14 @@ pub async fn list_photos( .unwrap_or_else(|e| e.error_response()); } - // Use recursive or non-recursive file listing based on flag + // Use recursive or non-recursive file listing based on flag. Both + // paths must walk the *scoped* library's root; the generic + // FileSystemAccess trait (file_system.get_files_for_path) is pinned + // to AppState's base_path at construction time and doesn't know + // which library the request targets. let files_result = if search_recursively { - // For recursive search without tags, manually list files recursively is_valid_full_path( - &PathBuf::from(&app_state.base_path), + &PathBuf::from(&scoped_library.root_path), &PathBuf::from(search_path), false, ) @@ -486,8 +489,21 @@ pub async fn list_photos( list_files_recursive(&path).unwrap_or_default() }) .context("Invalid path") - } else { + } else if scoped_library.id == app_state.primary_library().id { + // Primary library: preserve the original FileSystemAccess path so + // the test-mock path (MockFileSystem) continues to work. file_system.get_files_for_path(search_path) + } else { + is_valid_full_path( + &PathBuf::from(&scoped_library.root_path), + &PathBuf::from(search_path), + false, + ) + .map(|path| { + debug!("Valid path for non-recursive search: {:?}", path); + list_files(&path).unwrap_or_default() + }) + .context("Invalid path") }; match files_result { @@ -510,10 +526,10 @@ pub async fn list_photos( match path.metadata() { Ok(md) => { let relative = - path.strip_prefix(&app_state.base_path).unwrap_or_else(|_| { + path.strip_prefix(&scoped_library.root_path).unwrap_or_else(|_| { panic!( - "Unable to strip base path {} from file path {}", - &app_state.base_path.path(), + "Unable to strip library root {} from file path {}", + &scoped_library.root_path, path.display() ) }); @@ -530,11 +546,11 @@ pub async fn list_photos( // Include files without metadata if they have extensions if path.extension().is_some() { let relative = path - .strip_prefix(&app_state.base_path) + .strip_prefix(&scoped_library.root_path) .unwrap_or_else(|_| { panic!( - "Unable to strip base path {} from file path {}", - &app_state.base_path.path(), + "Unable to strip library root {} from file path {}", + &scoped_library.root_path, path.display() ) }); @@ -668,7 +684,7 @@ pub async fn list_photos( sort_type, &mut exif_dao_guard, &span_context, - app_state.base_path.as_ref(), + scoped_library.root_path.as_ref(), limit, offset, ); diff --git a/src/main.rs b/src/main.rs index 7305bae..0f3f6c8 100644 --- a/src/main.rs +++ b/src/main.rs @@ -104,12 +104,26 @@ async fn get_image( let mut span = tracer.start_with_context("get_image", &context); - if let Some(path) = is_valid_full_path(&app_state.base_path, &req.path, false) { + // Resolve library from query param; default to primary so clients that + // don't yet send `library=` continue to work. + let library = match libraries::resolve_library_param( + &app_state, + req.library.as_deref(), + ) { + Ok(Some(lib)) => lib, + Ok(None) => app_state.primary_library(), + Err(msg) => { + span.set_status(Status::error(msg.clone())); + return HttpResponse::BadRequest().body(msg); + } + }; + + if let Some(path) = is_valid_full_path(&library.root_path, &req.path, false) { let image_size = req.size.unwrap_or(PhotoSize::Full); if image_size == PhotoSize::Thumb { let relative_path = path - .strip_prefix(&app_state.base_path) - .expect("Error stripping base path prefix from thumbnail"); + .strip_prefix(&library.root_path) + .expect("Error stripping library root prefix from thumbnail"); let relative_path_str = relative_path.to_string_lossy().replace('\\', "/"); let thumbs = &app_state.thumbnail_path; diff --git a/src/memories.rs b/src/memories.rs index 89a7028..23086f6 100644 --- a/src/memories.rs +++ b/src/memories.rs @@ -369,6 +369,7 @@ fn collect_exif_memories( exif_dao: &Data>>, context: &opentelemetry::Context, base_path: &str, + library_id: i32, now: NaiveDate, span_mode: MemoriesSpan, years_back: u32, @@ -423,7 +424,7 @@ fn collect_exif_memories( path: file_path.clone(), created, modified, - library_id: crate::libraries::PRIMARY_LIBRARY_ID, + library_id, }, file_date, )) @@ -434,6 +435,7 @@ fn collect_exif_memories( /// Collect memories from file system scan (for files not in EXIF DB) fn collect_filesystem_memories( base_path: &str, + library_id: i32, path_excluder: &PathExcluder, skip_paths: &HashSet, now: NaiveDate, @@ -485,7 +487,7 @@ fn collect_filesystem_memories( path: path_relative, created, modified, - library_id: crate::libraries::PRIMARY_LIBRARY_ID, + library_id, }, file_date, )) @@ -560,6 +562,7 @@ pub async fn list_memories( &exif_dao, &span_context, &scoped_library.root_path, + scoped_library.id, now, span_mode, years_back, @@ -576,6 +579,7 @@ pub async fn list_memories( // Phase 2: File system scan (skip EXIF files) let fs_memories = collect_filesystem_memories( &scoped_library.root_path, + scoped_library.id, &path_excluder, &exif_paths, now, @@ -1122,7 +1126,7 @@ mod tests { path: "photo1.jpg".to_string(), created: Some(jan_15_2024_9am), modified: Some(jan_15_2024_9am), - library_id: crate::libraries::PRIMARY_LIBRARY_ID, + library_id: 1, }, NaiveDate::from_ymd_opt(2024, 1, 15).unwrap(), ), @@ -1131,7 +1135,7 @@ mod tests { path: "photo2.jpg".to_string(), created: Some(jan_15_2020_10am), modified: Some(jan_15_2020_10am), - library_id: crate::libraries::PRIMARY_LIBRARY_ID, + library_id: 1, }, NaiveDate::from_ymd_opt(2020, 1, 15).unwrap(), ), @@ -1140,7 +1144,7 @@ mod tests { path: "photo3.jpg".to_string(), created: Some(jan_16_2021_8am), modified: Some(jan_16_2021_8am), - library_id: crate::libraries::PRIMARY_LIBRARY_ID, + library_id: 1, }, NaiveDate::from_ymd_opt(2021, 1, 16).unwrap(), ),