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
+3 -1
View File
@@ -475,6 +475,7 @@ Internal architecture, components, and application logic.
| DR-275 | Idle re-lock separates "the UI is locked" from "who is the active profile", so audio keeps playing and keeps reporting as the account that started it while the screen is locked. Lockscreen transport controls keep working untouched, because nothing on a lockscreen browses or starts new content — the locked UI refuses only what reaches past the current queue. The timer starts when playback stops rather than when the UI goes quiet, and unlocking to a *different* profile stops playback. It lives in Rust beside the player state machine: it needs authoritative playback state, and a frontend timer dies with the WebView on Android | Player | UR-083 | Proposed |
| DR-276 | The picker and PIN pad render an opaque `unlock_method` and an `UnlockOutcome` union the backend returns; the frontend never compares a PIN, counts an attempt, or infers that an account without a PIN is a child's. "Child account" is not modelled at all — a child profile is simply one with no PIN — so no role taxonomy is invented on either side of a boundary that has leaked taxonomy before | Frontend | UR-082, UR-083 | Proposed |
| DR-277 | A library listing is scoped to that library. The cached-browse query matched a library parent with an `EXISTS` that never referenced the item — it asked only whether a library with the requested id existed — so the clause was true for every cached row on the server. Music, Movies and TV concealed it because their landing pages pass `include_item_types`, which narrowed the result; the generic library page passes none, so opening Books, Photos, Collections or a mixed library served whatever happened to be cached. The stored `library_id` now decides wherever the cache kept one, because that is the server's own answer and the only thing able to scope a library whose type has no mapping or none at all; the `collection_type``item_type` taxonomy is the fallback for rows written before it was stored, and a library with neither matches nothing and falls through to the server. The taxonomy itself is now a single macro shared with the downloaded listing, which had the identical defect fixed in isolation (DR-167) while this path kept it | Repository | UR-007 | Done |
| DR-278 | Cached items record the library they came from. `save_to_cache` bound `library_id` NULL on every row it wrote, so the only association available was the `collection_type``item_type` taxonomy — which cannot distinguish two libraries of the *same* type (a server with "TV" and "Shows" served both the same contents) and says nothing about a library whose type it does not map. The write path is the single choke point every cached row passes through and it already knows the parent being browsed, so it resolves the owning library once per call: the parent itself when it is a library, otherwise the library its parent item was already filed under, which propagates the association down a hierarchy as it is browsed. Synthetic parents such as `favorites` match neither and stay NULL, since they are not a library and span several. Existing rows cannot be repaired locally — the association was never stored — so migration 025 clears `synced_at` to force a re-fetch, the same move MIGRATION_018 made for `is_folder`; the taxonomy fallback stays for one release while caches refill | Repository | UR-007 | Done |
| DR-198 | The webview runs under a real Content-Security-Policy, and the asset protocol is scoped to the one directory it still serves. `csp` was `null`, which disables CSP entirely: any script that reached the web layer — through a future `{@html}`, a dependency, or a devtools paste — would have inherited the whole IPC surface, and with it the user's session. `script-src 'self'` (Tauri injects a nonce for SvelteKit's inline bootstrap script at build time, so no `'unsafe-inline'` is needed) plus `object-src`/`frame-src 'none'` and `base-uri 'self'` is the part that is genuinely restrictive. `img-src`/`media-src`/`connect-src` cannot be: the Jellyfin origin is typed in by the user at run time and is commonly plain `http` on a LAN, so they allow `http:`/`https:` — a wide grant for *data*, but one that still bars `file:`, `filesystem:` and scripting schemes, and leaves `script-src` untouched. `style-src` keeps `'unsafe-inline'` because Svelte compiles `style="…"` attributes (including `app.html`'s `display: contents` wrapper) into markup; this is safe only while no `<style>` element survives into `index.html`, since a nonce there would make Tauri's injection outrank — and therefore void — `'unsafe-inline'`. `worker-src blob:` and `media-src blob:` are hls.js: it demuxes in a worker built from a blob and attaches MSE through `URL.createObjectURL`. `asset:` and `http://asset.localhost` are the same protocol under the two naming schemes `convertFileSrc` emits (custom scheme on Linux/macOS, `http` host on Windows/Android); `ipc:`/`http://ipc.localhost` is the invoke transport, which would otherwise be blocked by `connect-src`. A run-time CSP naming the server origin exactly was rejected: Tauri computes the header from immutable config when it serves the HTML, so it would mean rebuilding config and reloading the webview on every server change, for a policy the user can already point anywhere. The asset-protocol scope narrows from `$APPDATA/**` to `$APPDATA/thumbnails/**` — since DR-137 moved downloaded media to the loopback server, `imageCache` is the only `convertFileSrc` caller left, so the database and the encrypted-token fallback file no longer sit inside the grant | Security | UR-012, UR-071 | Done |
---
@@ -491,7 +492,7 @@ Internal architecture, components, and application logic.
| UR-004 | IR-003, IR-004, IR-008, IR-011 | DR-002, DR-004, DR-006, DR-129, DR-171, DR-176, DR-177, DR-181, DR-182, DR-183, DR-185, DR-188, DR-203, DR-265 |
| UR-005 | - | DR-001, DR-005, DR-009, DR-178, DR-179, DR-186, DR-193, DR-195 |
| UR-006 | IR-005, IR-006, IR-007, IR-008 | DR-200, DR-201 |
| UR-007 | IR-010 | DR-007, DR-008, DR-016, DR-257, DR-262, DR-277 |
| UR-007 | IR-010 | DR-007, DR-008, DR-016, DR-257, DR-262, DR-277, DR-278 |
| UR-008 | IR-010 | DR-007, DR-011 |
| UR-009 | IR-009, IR-010, IR-011 | - |
| UR-010 | IR-012, IR-021 | DR-037, DR-059 |
@@ -820,6 +821,7 @@ Internal architecture, components, and application logic.
| UT-247 | A library whose `collection_type` has no mapping — Books, Photos, a mixed library — does not return the server's films, albums and shows from cache | DR-277 | Done |
| UT-248 | Narrowing the library clause does not starve the libraries that do have landing pages: music, movies and TV each still list their own media and none of the others | DR-277 | Done |
| UT-249 | Opening an individual collection still lists its own children: a BoxSet's members are matched by the stored `parent_id`, not by the library clause, so narrowing that clause did not empty collections | DR-277 | Done |
| UT-250 | Two libraries of the same collection type are not interchangeable: seeded through the real cache write path, a "TV" and a "Shows" library each list their own series and not the other's | DR-278 | Done |
### Integration Tests
| Test ID | Test Description | Traces To | Status |
+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;
+25
View File
@@ -29,6 +29,7 @@ pub const MIGRATIONS: &[(&str, &str)] = &[
("022_people_fts", MIGRATION_022),
("023_downloads_expiry", MIGRATION_023),
("024_multi_user_profiles", MIGRATION_024),
("025_backfill_item_library_id", MIGRATION_025),
];
/// Initial schema migration
@@ -1067,3 +1068,27 @@ mod migration_024_tests {
assert_eq!(count(&conn, "SELECT COUNT(*) FROM download_grants"), 0);
}
}
/// Force cached items to be re-fetched so `library_id` is populated.
///
/// `save_to_cache` bound `library_id` NULL for every row it wrote, so nothing in
/// the cache knew which library it came from. The only available association was
/// the `collection_type` ↔ `item_type` taxonomy, which cannot tell two libraries
/// of the same type apart — a server with "TV" and "Shows" served both the same
/// contents — and says nothing at all about a library whose type it does not map
/// (Books, Photos, Collections, or a mixed library where Jellyfin sends no
/// collection type).
///
/// The write path now records the library. Existing rows cannot be repaired
/// locally — the association was never stored — so they are marked stale and
/// re-fetched on next browse, exactly as MIGRATION_018 did for `is_folder`.
///
/// Deliberately does not delete anything: downloads, favourites and playback
/// positions live in other tables and are untouched, and a cleared `synced_at`
/// only means "ask the server again", so an offline user keeps browsing what
/// they already had until the next successful fetch.
///
/// TRACES: UR-007 | DR-278
const MIGRATION_025: &str = r#"
UPDATE items SET synced_at = NULL;
"#;