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 <noreply@anthropic.com>
This commit is contained in:
@@ -192,6 +192,10 @@ pub struct ThumbnailRequest {
|
|||||||
pub(crate) format: Option<ThumbnailFormat>,
|
pub(crate) format: Option<ThumbnailFormat>,
|
||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
pub(crate) shape: Option<ThumbnailShape>,
|
pub(crate) shape: Option<ThumbnailShape>,
|
||||||
|
/// 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<String>,
|
||||||
}
|
}
|
||||||
|
|
||||||
#[derive(Debug, Deserialize, PartialEq)]
|
#[derive(Debug, Deserialize, PartialEq)]
|
||||||
|
|||||||
40
src/files.rs
40
src/files.rs
@@ -422,7 +422,7 @@ pub async fn list_photos<TagD: TagDao, FS: FileSystemAccess>(
|
|||||||
sort_type,
|
sort_type,
|
||||||
&mut exif_dao_guard,
|
&mut exif_dao_guard,
|
||||||
&span_context,
|
&span_context,
|
||||||
app_state.base_path.as_ref(),
|
scoped_library.root_path.as_ref(),
|
||||||
limit,
|
limit,
|
||||||
offset,
|
offset,
|
||||||
);
|
);
|
||||||
@@ -473,11 +473,14 @@ pub async fn list_photos<TagD: TagDao, FS: FileSystemAccess>(
|
|||||||
.unwrap_or_else(|e| e.error_response());
|
.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 {
|
let files_result = if search_recursively {
|
||||||
// For recursive search without tags, manually list files recursively
|
|
||||||
is_valid_full_path(
|
is_valid_full_path(
|
||||||
&PathBuf::from(&app_state.base_path),
|
&PathBuf::from(&scoped_library.root_path),
|
||||||
&PathBuf::from(search_path),
|
&PathBuf::from(search_path),
|
||||||
false,
|
false,
|
||||||
)
|
)
|
||||||
@@ -486,8 +489,21 @@ pub async fn list_photos<TagD: TagDao, FS: FileSystemAccess>(
|
|||||||
list_files_recursive(&path).unwrap_or_default()
|
list_files_recursive(&path).unwrap_or_default()
|
||||||
})
|
})
|
||||||
.context("Invalid path")
|
.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)
|
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 {
|
match files_result {
|
||||||
@@ -510,10 +526,10 @@ pub async fn list_photos<TagD: TagDao, FS: FileSystemAccess>(
|
|||||||
match path.metadata() {
|
match path.metadata() {
|
||||||
Ok(md) => {
|
Ok(md) => {
|
||||||
let relative =
|
let relative =
|
||||||
path.strip_prefix(&app_state.base_path).unwrap_or_else(|_| {
|
path.strip_prefix(&scoped_library.root_path).unwrap_or_else(|_| {
|
||||||
panic!(
|
panic!(
|
||||||
"Unable to strip base path {} from file path {}",
|
"Unable to strip library root {} from file path {}",
|
||||||
&app_state.base_path.path(),
|
&scoped_library.root_path,
|
||||||
path.display()
|
path.display()
|
||||||
)
|
)
|
||||||
});
|
});
|
||||||
@@ -530,11 +546,11 @@ pub async fn list_photos<TagD: TagDao, FS: FileSystemAccess>(
|
|||||||
// Include files without metadata if they have extensions
|
// Include files without metadata if they have extensions
|
||||||
if path.extension().is_some() {
|
if path.extension().is_some() {
|
||||||
let relative = path
|
let relative = path
|
||||||
.strip_prefix(&app_state.base_path)
|
.strip_prefix(&scoped_library.root_path)
|
||||||
.unwrap_or_else(|_| {
|
.unwrap_or_else(|_| {
|
||||||
panic!(
|
panic!(
|
||||||
"Unable to strip base path {} from file path {}",
|
"Unable to strip library root {} from file path {}",
|
||||||
&app_state.base_path.path(),
|
&scoped_library.root_path,
|
||||||
path.display()
|
path.display()
|
||||||
)
|
)
|
||||||
});
|
});
|
||||||
@@ -668,7 +684,7 @@ pub async fn list_photos<TagD: TagDao, FS: FileSystemAccess>(
|
|||||||
sort_type,
|
sort_type,
|
||||||
&mut exif_dao_guard,
|
&mut exif_dao_guard,
|
||||||
&span_context,
|
&span_context,
|
||||||
app_state.base_path.as_ref(),
|
scoped_library.root_path.as_ref(),
|
||||||
limit,
|
limit,
|
||||||
offset,
|
offset,
|
||||||
);
|
);
|
||||||
|
|||||||
20
src/main.rs
20
src/main.rs
@@ -104,12 +104,26 @@ async fn get_image(
|
|||||||
|
|
||||||
let mut span = tracer.start_with_context("get_image", &context);
|
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);
|
let image_size = req.size.unwrap_or(PhotoSize::Full);
|
||||||
if image_size == PhotoSize::Thumb {
|
if image_size == PhotoSize::Thumb {
|
||||||
let relative_path = path
|
let relative_path = path
|
||||||
.strip_prefix(&app_state.base_path)
|
.strip_prefix(&library.root_path)
|
||||||
.expect("Error stripping base path prefix from thumbnail");
|
.expect("Error stripping library root prefix from thumbnail");
|
||||||
let relative_path_str = relative_path.to_string_lossy().replace('\\', "/");
|
let relative_path_str = relative_path.to_string_lossy().replace('\\', "/");
|
||||||
|
|
||||||
let thumbs = &app_state.thumbnail_path;
|
let thumbs = &app_state.thumbnail_path;
|
||||||
|
|||||||
@@ -369,6 +369,7 @@ fn collect_exif_memories(
|
|||||||
exif_dao: &Data<Mutex<Box<dyn ExifDao>>>,
|
exif_dao: &Data<Mutex<Box<dyn ExifDao>>>,
|
||||||
context: &opentelemetry::Context,
|
context: &opentelemetry::Context,
|
||||||
base_path: &str,
|
base_path: &str,
|
||||||
|
library_id: i32,
|
||||||
now: NaiveDate,
|
now: NaiveDate,
|
||||||
span_mode: MemoriesSpan,
|
span_mode: MemoriesSpan,
|
||||||
years_back: u32,
|
years_back: u32,
|
||||||
@@ -423,7 +424,7 @@ fn collect_exif_memories(
|
|||||||
path: file_path.clone(),
|
path: file_path.clone(),
|
||||||
created,
|
created,
|
||||||
modified,
|
modified,
|
||||||
library_id: crate::libraries::PRIMARY_LIBRARY_ID,
|
library_id,
|
||||||
},
|
},
|
||||||
file_date,
|
file_date,
|
||||||
))
|
))
|
||||||
@@ -434,6 +435,7 @@ fn collect_exif_memories(
|
|||||||
/// Collect memories from file system scan (for files not in EXIF DB)
|
/// Collect memories from file system scan (for files not in EXIF DB)
|
||||||
fn collect_filesystem_memories(
|
fn collect_filesystem_memories(
|
||||||
base_path: &str,
|
base_path: &str,
|
||||||
|
library_id: i32,
|
||||||
path_excluder: &PathExcluder,
|
path_excluder: &PathExcluder,
|
||||||
skip_paths: &HashSet<PathBuf>,
|
skip_paths: &HashSet<PathBuf>,
|
||||||
now: NaiveDate,
|
now: NaiveDate,
|
||||||
@@ -485,7 +487,7 @@ fn collect_filesystem_memories(
|
|||||||
path: path_relative,
|
path: path_relative,
|
||||||
created,
|
created,
|
||||||
modified,
|
modified,
|
||||||
library_id: crate::libraries::PRIMARY_LIBRARY_ID,
|
library_id,
|
||||||
},
|
},
|
||||||
file_date,
|
file_date,
|
||||||
))
|
))
|
||||||
@@ -560,6 +562,7 @@ pub async fn list_memories(
|
|||||||
&exif_dao,
|
&exif_dao,
|
||||||
&span_context,
|
&span_context,
|
||||||
&scoped_library.root_path,
|
&scoped_library.root_path,
|
||||||
|
scoped_library.id,
|
||||||
now,
|
now,
|
||||||
span_mode,
|
span_mode,
|
||||||
years_back,
|
years_back,
|
||||||
@@ -576,6 +579,7 @@ pub async fn list_memories(
|
|||||||
// Phase 2: File system scan (skip EXIF files)
|
// Phase 2: File system scan (skip EXIF files)
|
||||||
let fs_memories = collect_filesystem_memories(
|
let fs_memories = collect_filesystem_memories(
|
||||||
&scoped_library.root_path,
|
&scoped_library.root_path,
|
||||||
|
scoped_library.id,
|
||||||
&path_excluder,
|
&path_excluder,
|
||||||
&exif_paths,
|
&exif_paths,
|
||||||
now,
|
now,
|
||||||
@@ -1122,7 +1126,7 @@ mod tests {
|
|||||||
path: "photo1.jpg".to_string(),
|
path: "photo1.jpg".to_string(),
|
||||||
created: Some(jan_15_2024_9am),
|
created: Some(jan_15_2024_9am),
|
||||||
modified: 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(),
|
NaiveDate::from_ymd_opt(2024, 1, 15).unwrap(),
|
||||||
),
|
),
|
||||||
@@ -1131,7 +1135,7 @@ mod tests {
|
|||||||
path: "photo2.jpg".to_string(),
|
path: "photo2.jpg".to_string(),
|
||||||
created: Some(jan_15_2020_10am),
|
created: Some(jan_15_2020_10am),
|
||||||
modified: 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(),
|
NaiveDate::from_ymd_opt(2020, 1, 15).unwrap(),
|
||||||
),
|
),
|
||||||
@@ -1140,7 +1144,7 @@ mod tests {
|
|||||||
path: "photo3.jpg".to_string(),
|
path: "photo3.jpg".to_string(),
|
||||||
created: Some(jan_16_2021_8am),
|
created: Some(jan_16_2021_8am),
|
||||||
modified: 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(),
|
NaiveDate::from_ymd_opt(2021, 1, 16).unwrap(),
|
||||||
),
|
),
|
||||||
|
|||||||
Reference in New Issue
Block a user