fix(library): record which library a cached item came from
🏗️ Build and Test JellyTau / Run Tests (push) Failing after 13m40s
🏗️ Build and Test JellyTau / Android Compile Check (push) Skipped
🏗️ Build and Test JellyTau / Supply Chain (push) Successful in 43s
📱 Test APK / Build test APK (push) Successful in 49m21s
Publish Documentation / Build & publish docs to gitea-pages (push) Successful in 8m39s
Traceability Validation / Check Requirement Traces (push) Successful in 24s

"TV" and "Shows" showed identical contents, and so would any two
libraries of the same type.

save_to_cache bound library_id NULL on every row it wrote, so nothing in
the cache knew where an item came from. The only association available
was the collection_type/item_type taxonomy, and that is unable in
principle to tell two libraries of one type apart -- both are 'tvshows',
so every Series on the server satisfies either. DR-277 narrowed the
library clause, which stopped Books and Photos serving the whole server,
but no clause over that taxonomy could have fixed this.

The write path is the single choke point every cached row passes through
and it already knows the parent being browsed, so it now resolves the
owning library once per call: the parent itself when it is a library,
otherwise the library its parent item was filed under, which carries the
association down a hierarchy as it is browsed. Synthetic parents like
"favorites" match neither and stay NULL -- they are not a library and
span several.

This is what makes the taxonomy stop being load-bearing. Library types
nobody enumerated -- Books, Photos, Collections, mixed libraries with no
collection type at all -- are now scoped by the same link as everything
else rather than by whether someone remembered to add an arm for them.

Existing rows cannot be repaired locally, because the association was
never stored: migration 025 clears synced_at to force a re-fetch, the
same move MIGRATION_018 made for is_folder. Nothing is deleted --
downloads, favourites and playback positions live in other tables, and a
cleared synced_at only means "ask the server again".

The new tests seed through save_to_cache rather than inserting rows
directly, so they exercise the path that was actually broken.
This commit is contained in:
2026-09-07 19:41:40 +02:00
parent dea78b89b9
commit c41b8ec896
3 changed files with 174 additions and 3 deletions
+146 -2
View File
@@ -446,12 +446,81 @@ impl OfflineRepository {
result
}
/// Which library the children of `parent_id` belong to.
///
/// `Some(parent_id)` when the parent is itself a library, otherwise the
/// library the parent item was already filed under — so the association
/// propagates down a hierarchy as it is browsed, without needing the server
/// to repeat it on every item. `None` for a parent that is neither, which
/// is how synthetic parents like "favorites" avoid being filed anywhere.
///
/// TRACES: UR-007 | DR-278
async fn resolve_owning_library(&self, parent_id: &str) -> Option<String> {
let is_library: Option<String> = self
.db_service
.query_optional(
Query::with_params(
"SELECT id FROM libraries WHERE id = ? AND server_id = ?",
vec![
QueryParam::String(parent_id.to_string()),
QueryParam::String(self.server_id.clone()),
],
),
|row| row.get(0),
)
.await
.ok()
.flatten();
if is_library.is_some() {
return is_library;
}
self.db_service
.query_optional(
Query::with_params(
"SELECT library_id FROM items WHERE id = ? AND library_id IS NOT NULL",
vec![QueryParam::String(parent_id.to_string())],
),
|row| row.get(0),
)
.await
.ok()
.flatten()
}
async fn save_to_cache_impl(
&self,
parent_id: &str,
items: &[MediaItem],
now: &str,
) -> Result<usize, RepoError> {
// Which library do these items belong to?
//
// Resolved once per call, from the parent being browsed. Two cases and
// nothing else:
//
// * the parent IS a library -> these are its direct children
// * the parent is an item -> inherit whatever library that item is
// already known to belong to, so tracks
// under an album and episodes under a
// season land in the same library as
// their container
//
// Synthetic parents ("favorites" and friends) match neither and stay
// NULL, which is correct: they are not a library and their contents
// span several.
//
// Until this existed, `library_id` was bound NULL for every cached row
// and the only way to associate an item with a library was the
// `collection_type` ↔ `item_type` taxonomy. That cannot tell two
// libraries of the *same* type apart — a server with "TV" and "Shows"
// served both the same contents — and has nothing to say about a
// library whose type it does not map (DR-278).
//
// TRACES: UR-007 | DR-278
let owning_library = self.resolve_owning_library(parent_id).await;
// Collect all unique parent IDs referenced by items being saved
let mut parent_ids = std::collections::HashSet::new();
parent_ids.insert(parent_id.to_string());
@@ -578,8 +647,12 @@ impl OfflineRepository {
vec![
QueryParam::String(item.id.clone()),
QueryParam::String(self.server_id.clone()),
// Library is NULL for cached items (may not be synced yet)
QueryParam::Null, // library_id
// The library this browse belongs to; NULL only for
// synthetic parents. See `resolve_owning_library`.
match &owning_library {
Some(lib) => QueryParam::String(lib.clone()),
None => QueryParam::Null,
}, // library_id
// Use the item's actual parent_id, not the function parameter
match &item.parent_id {
Some(pid) => QueryParam::String(pid.clone()),
@@ -5036,6 +5109,11 @@ mod tests {
/// TRACES: UR-007, UR-055 | DR-277 | UT-247
#[tokio::test]
async fn test_get_items_unknown_library_type_does_not_return_whole_server() {
// Shared global: other tests flip it, so hold the lock and
// state it explicitly rather than inheriting whatever ran last.
let _guard = lock_catalog_browse();
set_include_catalog_browse(true);
let db = create_test_db();
insert_item(&db, "movie-1", "Movie", None, None, None).await;
@@ -5067,6 +5145,62 @@ mod tests {
}
}
/// Two libraries of the *same* type are still two libraries. A server with
/// "TV" and "Shows" — or "Films" and "Kids Films" — must not serve both the
/// same contents.
///
/// The taxonomy fallback cannot tell them apart: it matches on
/// `collection_type`, which is identical for both, so every Series on the
/// server satisfies either one. Only the stored `library_id` can separate
/// them, which is why populating it is the real fix rather than a nicety.
///
/// TRACES: UR-007 | DR-277 | UT-250
#[tokio::test]
async fn test_get_items_two_libraries_of_one_type_are_not_interchangeable() {
// Shared global: other tests flip it, so hold the lock and
// state it explicitly rather than inheriting whatever ran last.
let _guard = lock_catalog_browse();
set_include_catalog_browse(true);
let db = create_test_db();
seed_library(&db, "tv-lib", "tvshows").await;
seed_library(&db, "shows-lib", "tvshows").await;
let repo = make_repo(&db);
// Seeded through the real write path, because that is what the fix
// changes: browsing a library is what files its contents under it.
for (id, lib) in [("series-a", "tv-lib"), ("series-b", "shows-lib")] {
let mut item = create_test_item(id, id, None);
item.item_type = "Series".to_string();
item.kind = crate::domain::MediaKind::Series;
repo.save_to_cache(lib, &[item]).await.unwrap();
}
for (lib, own, other) in [
("tv-lib", "series-a", "series-b"),
("shows-lib", "series-b", "series-a"),
] {
let ids: Vec<String> = repo
.get_items(lib, None)
.await
.unwrap()
.items
.iter()
.map(|i| i.id.clone())
.collect();
assert!(
ids.contains(&own.to_string()),
"{lib} should list {own}; got {:?}",
ids
);
assert!(
!ids.contains(&other.to_string()),
"{lib} must not list {other}, which lives in the other library; got {:?}",
ids
);
}
}
/// Opening an individual collection is a different path and must keep
/// working: a BoxSet's children carry `parent_id`, which the cache does
/// store, so they are matched by the ordinary parent link rather than by
@@ -5079,6 +5213,11 @@ mod tests {
/// TRACES: UR-007 | DR-277 | UT-249
#[tokio::test]
async fn test_get_items_collection_lists_its_own_children() {
// Shared global: other tests flip it, so hold the lock and
// state it explicitly rather than inheriting whatever ran last.
let _guard = lock_catalog_browse();
set_include_catalog_browse(true);
let db = create_test_db();
seed_library(&db, "boxset-lib", "boxsets").await;
@@ -5119,6 +5258,11 @@ mod tests {
/// TRACES: UR-007 | DR-277 | UT-248
#[tokio::test]
async fn test_get_items_typed_libraries_still_return_their_own_media() {
// Shared global: other tests flip it, so hold the lock and
// state it explicitly rather than inheriting whatever ran last.
let _guard = lock_catalog_browse();
set_include_catalog_browse(true);
let db = create_test_db();
seed_library(&db, "music-lib", "music").await;
seed_library(&db, "movie-lib", "movies").await;