diff --git a/docs/architecture/08-database-design.md b/docs/architecture/08-database-design.md index 9de4982ca..d6ea53775 100644 --- a/docs/architecture/08-database-design.md +++ b/docs/architecture/08-database-design.md @@ -243,9 +243,14 @@ CREATE TABLE items ( last_sync DATETIME, UNIQUE(jellyfin_id, server_id) + + -- Logical container (migration 027): episode → season/series, season → + -- series, track → album, else parent. See "Listing query shape". + -- container_id TEXT GENERATED ALWAYS AS (CASE item_type … END) VIRTUAL ); -- Performance indexes +CREATE INDEX idx_items_container ON items(container_id, sort_name, name); CREATE INDEX idx_items_server ON items(server_id); CREATE INDEX idx_items_library ON items(library_id); CREATE INDEX idx_items_parent ON items(parent_id); @@ -613,7 +618,8 @@ life of the app; nothing else opens the file. Everything goes through ``` Pragmas: `journal_mode = WAL`, `synchronous = NORMAL`, `busy_timeout = 5 s` on -every connection. +every connection. `PRAGMA optimize` runs once at open, after migrations, so the planner has +statistics (see "Listing query shape"). **Why.** It used to be one connection behind one `std::sync::Mutex`. WAL was on, but with a single connection its one benefit — readers running beside a writer — @@ -665,27 +671,52 @@ for the jobs grouped with it. `OfflineRepository::get_items` (`items_listing_sql`) is the hot read: every library, series, season and album page goes through it, and it must fit the -100 ms cache fast path on a phone. +100 ms cache fast path on a phone. Every other cached read follows the same +rules. -- **Availability is checked per row, with `EXISTS`** (cached for browsing, - downloaded, or a container with a downloaded child). It used to be a CTE - that built the id of *every* available item in the database before filtering - to the parent: ~80 ms on a desktop for a 100k-item cache, whatever the parent. -- **A parent that is not a library is matched on the hierarchy columns alone** - (`parent_id`/`album_id`/`season_id`/`series_id`), which SQLite answers with a - multi-index `OR`. Whether the parent is a library is looked up first; the - library clause can only match for a library, and inside the same `OR` it - forced a full scan. -- **`+i.server_id`**: the app never runs `ANALYZE`, and without statistics the - planner prefers the `server_id` index — which every row shares — over the - hierarchy indexes. The unary `+` takes it out of consideration. +- **Children are matched on `items.container_id`** (migration 027), a VIRTUAL + generated column holding the item's *logical* container: an episode's season + (else series, else parent), a season's series, a track's album, otherwise the + parent. Jellyfin's `ParentId` is the storage parent, not the logical one — in + a series without season folders an episode's `ParentId` is the series while + its `SeasonId` names a virtual season — so listings used to match on four + columns at once. That `OR` defeated the planner into walking the whole table, + and it was wrong: every episode carries its series id, so a series listed all + its episodes beside its seasons. Being generated, the column covers every + write path (cache, downloads, catalog crawl) without any of them knowing, and + cannot drift from the columns it is computed from. +- **`idx_items_container (container_id, sort_name, name)`** serves a listing as + one index range already in display order — no sort step. `sort_name` is + usually NULL in the cache (the cache never writes it), hence `name` in the + index too. +- **Containers exist even when never browsed.** A series lists its seasons, so + an episode whose season row was never cached (it arrived through Next Up or + Latest) would be unreachable from its series offline. `save_to_cache` — and + migration 027 for rows already on disk — inserts placeholders named from the + child's own fields (`season_name`, `series_name`, `album_name`) with + `synced_at` NULL, so they show only when a download makes them available; the + server's real row replaces them wholesale on the next browse. +- **Availability is a per-row `EXISTS`** (`downloaded_sql` / `available_sql`): + cached for browsing (only with the catalog-browse flag), downloaded, or a + container with a downloaded *descendant* — which is why that one check still + looks at all four link columns (a series is available through an episode two + levels down). It used to be a CTE that built the id of every available item + in the database before filtering, paying for the whole table on every call. +- **The library clause is added only for a library parent** (`is_library`), + never `OR`ed into an ordinary listing, where it forces a full scan. +- **`+i.server_id`** keeps the planner off the server index, which every row + shares. `PRAGMA optimize` at open (`analysis_limit = 400`) gives the planner + statistics, but even with them it chose that index for the old `OR`; the `+` + is the guarantee. - **User data is fetched in batches** (`with_user_data`, one `IN (…)` query per 500 rows), not once per row. -Result: 80 ms → 1.5 ms on the desktop benchmark; -`listing_a_non_library_parent_uses_the_hierarchy_indexes` asserts the plan. -`storage::tests::write_bench_database` (ignored) writes a phone-sized -catalogue for timing queries with the `sqlite3` CLI. +Measured on a ~110k-item benchmark catalogue (desktop): a series listing went +from ~80 ms to under 1 ms; migration 027 upgrades an existing database of that +size in ~0.1 s. `listing_a_non_library_parent_uses_the_container_index` and +migration 027's tests assert the plans; `storage::tests::write_bench_database` +(ignored; set `JELLYTAU_BENCH_DB`) writes the benchmark catalogue for the +`sqlite3` CLI. ## Rust Module Structure diff --git a/src-tauri/src/repository/offline.rs b/src-tauri/src/repository/offline.rs index aa0f771e9..5f6580602 100644 --- a/src-tauri/src/repository/offline.rs +++ b/src-tauri/src/repository/offline.rs @@ -290,6 +290,74 @@ impl OfflineRepository { } } +/// A container row to create if it was never cached. See `save_to_cache`. +struct ContainerPlaceholder { + id: String, + name: String, + item_type: &'static str, + series_id: Option, + series_name: Option, + album_artist: Option, +} + +/// The logical containers `item` lists under, as far as its own fields name +/// them — the same rule as `container_id` in migration 027: an episode's +/// season and series, a season's series, a track's album. +/// +/// TRACES: UR-002, UR-007 | DR-013 +fn container_placeholders(item: &MediaItem) -> Vec { + let mut out = Vec::new(); + let series = |item: &MediaItem| { + item.series_id.clone().map(|id| ContainerPlaceholder { + id, + name: item + .series_name + .clone() + .unwrap_or_else(|| "Series".to_string()), + item_type: "Series", + series_id: None, + series_name: None, + album_artist: None, + }) + }; + match item.item_type.as_str() { + "Episode" => { + if let Some(id) = item.season_id.clone() { + out.push(ContainerPlaceholder { + id, + name: item + .season_name + .clone() + .unwrap_or_else(|| "Season".to_string()), + item_type: "Season", + series_id: item.series_id.clone(), + series_name: item.series_name.clone(), + album_artist: None, + }); + } + out.extend(series(item)); + } + "Season" => out.extend(series(item)), + "Audio" => { + if let Some(id) = item.album_id.clone() { + out.push(ContainerPlaceholder { + id, + name: item + .album_name + .clone() + .unwrap_or_else(|| "Album".to_string()), + item_type: "MusicAlbum", + series_id: None, + series_name: None, + album_artist: item.album_artist.clone(), + }); + } + } + _ => {} + } + out +} + /// A `user_data` row's columns, starting at `offset`, in the order /// `playback_position_ticks, is_played, is_favorite, play_count, /// last_played_at, playback_context_type, playback_context_id`. @@ -315,6 +383,51 @@ fn row_to_user_data(row: &rusqlite::Row, offset: usize) -> UserData { } } +/// Item types that can be downloaded themselves. +const PLAYABLE_TYPES: &str = "'Audio', 'Movie', 'Episode'"; +/// Item types that are available when something below them is downloaded. +const CONTAINER_TYPES: &str = + "'MusicAlbum', 'Series', 'Season', 'BoxSet', 'Folder', 'CollectionFolder'"; + +/// SQL predicate: the row `alias` is on the device — a `playable` item with a +/// completed download, or a `container` with a downloaded descendant. +/// +/// Evaluated per row, with `EXISTS`, on whatever rows the query has already +/// narrowed to. Every query used to build the set of *all* downloaded items +/// in a CTE first and join against it, paying for the whole table on every +/// call however few rows it wanted (see 08-database-design.md → "Listing +/// query shape"). Descendants are found through all four link columns on +/// purpose: a series is available through an episode two levels down. +/// +/// TRACES: UR-002, UR-055 | DR-013, DR-082 +fn downloaded_sql(alias: &str, playable: &str, containers: &str) -> String { + format!( + "(({a}.item_type IN ({playable}) + AND EXISTS (SELECT 1 FROM downloads dl + WHERE dl.item_id = {a}.id AND dl.status = 'completed')) + OR ({a}.item_type IN ({containers}) + AND EXISTS (SELECT 1 FROM items dc + INNER JOIN downloads dl ON dl.item_id = dc.id AND dl.status = 'completed' + WHERE dc.parent_id = {a}.id OR dc.album_id = {a}.id + OR dc.season_id = {a}.id OR dc.series_id = {a}.id)))", + a = alias + ) +} + +/// SQL predicate: the row `alias` can be shown — downloaded (see +/// [`downloaded_sql`]) or, with `include_catalog`, cached for browsing. +/// `include_catalog` is the catalog-browse flag; offline with "Show all server +/// media" off it is false and only downloaded media shows +/// (`set_include_catalog_browse`). +fn available_sql(alias: &str, include_catalog: bool) -> String { + let downloaded = downloaded_sql(alias, PLAYABLE_TYPES, CONTAINER_TYPES); + if include_catalog { + format!("({alias}.synced_at IS NOT NULL OR {downloaded})") + } else { + downloaded + } +} + /// The listing query behind `get_items`: the cached children of one parent /// that are available to show. /// @@ -328,16 +441,17 @@ fn row_to_user_data(row: &rusqlite::Row, offset: usize) -> UserData { /// filtering to the parent, which cost ~80 ms on a desktop for a 100k-item /// cache whatever the parent — several times that on a phone. /// -/// A parent that is not a library is matched on the hierarchy columns alone, -/// which SQLite serves with one index lookup per column. The library clause is -/// dropped there rather than evaluated: it can only match when the parent *is* -/// a library, and inside the same `OR` it forced a scan of every row. The -/// `+` on `server_id` stops the planner — which has no statistics, the app -/// never runs `ANALYZE` — from choosing the server index, which every row -/// shares. +/// Children are matched on `container_id`, the logical container resolved by +/// migration 027 (an episode's season, a season's series, a track's album, +/// otherwise the parent) — one index range, already in display order. It +/// replaced a four-column `OR` that was both slow and wrong: every episode +/// carries its series id, so a series listed its episodes beside its seasons. +/// The library clause is added only when the parent *is* a library; inside the +/// same `OR` it forces a scan of every row. The `+` on `server_id` keeps the +/// planner off the server index, which every row shares. /// -/// Bind order: server id; the parent id four times (plus a fifth for a library -/// parent); the type-filter values; with the favourites filter, the user id. +/// Bind order: server id; the parent id (twice for a library parent); the +/// type-filter values; with the favourites filter, the user id. /// /// TRACES: UR-002, UR-007 | DR-013, DR-277 #[allow(clippy::too_many_arguments)] @@ -350,16 +464,12 @@ fn items_listing_sql( limit: usize, start_index: usize, ) -> String { - let catalog_available = if include_catalog { - "i.synced_at IS NOT NULL OR " - } else { - "" - }; + let available = available_sql("i", include_catalog); let parent_match = if parent_is_library { format!( "i.server_id = ? AND ( - i.parent_id = ? OR i.album_id = ? OR i.season_id = ? OR i.series_id = ? + i.container_id = ? -- When the requested parent is a LIBRARY, there is no per-item -- link back to it (library_id/parent_id are NULL in the cache), -- so match every item on the server and let the type filter @@ -397,9 +507,7 @@ fn items_listing_sql( library_type_matches_item!() ) } else { - "+i.server_id = ? - AND (i.parent_id = ? OR i.album_id = ? OR i.season_id = ? OR i.series_id = ?)" - .to_string() + "+i.server_id = ? AND i.container_id = ?".to_string() }; format!( "SELECT i.id, i.name, i.item_type, i.server_id, i.parent_id, i.library_id, i.overview, i.genres, @@ -409,24 +517,7 @@ fn items_listing_sql( i.parent_index_number, i.is_folder, i.premiere_date FROM items i WHERE {parent_match} - AND ( - {catalog_available}( - i.item_type IN ('Audio', 'Movie', 'Episode') - AND EXISTS ( - SELECT 1 FROM downloads d - WHERE d.item_id = i.id AND d.status = 'completed' - ) - ) - OR ( - i.item_type IN ('MusicAlbum', 'Series', 'Season', 'BoxSet', 'Folder', 'CollectionFolder') - AND EXISTS ( - SELECT 1 FROM items children - INNER JOIN downloads d ON d.item_id = children.id AND d.status = 'completed' - WHERE children.parent_id = i.id OR children.album_id = i.id - OR children.season_id = i.id OR children.series_id = i.id - ) - ) - ){type_filter}{favorites_filter} + AND {available}{type_filter}{favorites_filter} ORDER BY {order_by} LIMIT {limit} OFFSET {start_index}" ) @@ -679,6 +770,50 @@ impl OfflineRepository { // TRACES: UR-007 | DR-278 let owning_library = self.resolve_owning_library(parent_id).await; + // Placeholders for the logical containers these items list under + // (`container_id`, migration 027) when those were never cached: an + // episode fetched through Next Up or Latest has a season and series + // this device may never have browsed, and without their rows it would + // be unreachable from its series page offline. Named from the fields + // the child carries; `synced_at` stays NULL so they show only when a + // download makes them available, and the server's real row replaces + // them. Inserted before the parent stubs below, so a season that is + // also a parent gets a Season row rather than a nameless folder. + let mut placeholder_ids = std::collections::HashSet::new(); + let mut placeholders: Vec = Vec::new(); + for item in items { + for p in container_placeholders(item) { + if !placeholder_ids.insert(p.id.clone()) { + continue; + } + placeholders.push(Query::with_params( + "INSERT OR IGNORE INTO items + (id, server_id, library_id, name, item_type, is_folder, + series_id, series_name, album_artist) + VALUES (?, ?, ?, ?, ?, 1, ?, ?, ?)", + vec![ + QueryParam::String(p.id), + QueryParam::String(self.server_id.clone()), + owning_library + .clone() + .map(QueryParam::String) + .unwrap_or(QueryParam::Null), + QueryParam::String(p.name), + QueryParam::String(p.item_type.to_string()), + p.series_id + .map(QueryParam::String) + .unwrap_or(QueryParam::Null), + p.series_name + .map(QueryParam::String) + .unwrap_or(QueryParam::Null), + p.album_artist + .map(QueryParam::String) + .unwrap_or(QueryParam::Null), + ], + )); + } + } + // Stub rows for every parent referenced, so `items.parent_id` resolves // whatever order the server returned children and parents in. let mut parent_ids = std::collections::HashSet::new(); @@ -728,6 +863,9 @@ impl OfflineRepository { // TRACES: UR-002, UR-007 | DR-012 self.db_service .transaction_without_foreign_keys(move |tx| { + for placeholder in placeholders { + tx.execute(placeholder)?; + } for stub in stubs { tx.execute(stub)?; } @@ -748,6 +886,26 @@ impl OfflineRepository { .map_err(|e| RepoError::Database { message: e }) } + /// Whether `id` is one of this server's libraries. Listings decide this + /// before building their query: the library clause is only correct — and + /// only affordable — when it is. + async fn is_library(&self, id: &str) -> Result { + self.db_service + .query_optional( + Query::with_params( + "SELECT 1 FROM libraries WHERE id = ? AND server_id = ?", + vec![ + QueryParam::String(id.to_string()), + QueryParam::String(self.server_id.clone()), + ], + ), + |row| row.get::<_, i64>(0), + ) + .await + .map(|row| row.is_some()) + .map_err(|e| RepoError::Database { message: e }) + } + /// Which library the children of `parent_id` belong to. /// /// `Some(parent_id)` when the parent is itself a library, otherwise the @@ -1170,25 +1328,6 @@ impl OfflineRepository { )" ); - /// TRACES: UR-055 | DR-082, DR-083 - const DOWNLOADED_ITEMS_CTE: &'static str = " - WITH downloaded_items AS ( - SELECT DISTINCT i.id - FROM items i - INNER JOIN downloads d ON i.id = d.item_id - WHERE d.status = 'completed' - AND i.item_type IN ('Audio', 'Movie', 'Episode') - - UNION - - SELECT DISTINCT i.id - FROM items i - INNER JOIN items children ON (children.parent_id = i.id OR children.album_id = i.id OR children.season_id = i.id OR children.series_id = i.id) - INNER JOIN downloads d ON children.id = d.item_id - WHERE d.status = 'completed' - AND i.item_type IN ('MusicAlbum', 'Series', 'Season', 'BoxSet', 'Folder', 'CollectionFolder') - )"; - /// Downloaded-only browse: items under `parent_id` that are on the device. /// /// Unlike [`MediaRepository::get_items`], this never includes the @@ -1234,18 +1373,17 @@ impl OfflineRepository { // container (e.g. a downloaded Movie, or a stray track whose album isn't // cached) still surface. This mirrors the online music library, which // routes to a dedicated albums view. See [[offline-libraries-never-cached]]. - let sql = format!( - "{cte} - SELECT i.id, i.name, i.item_type, i.server_id, i.parent_id, i.library_id, i.overview, i.genres, - i.runtime_ticks, i.production_year, i.community_rating, i.official_rating, - i.primary_image_tag, i.album_id, i.album_name, i.album_artist, i.artists, - i.index_number, i.series_id, i.series_name, i.season_id, i.season_name, - i.parent_index_number, i.is_folder, i.premiere_date - FROM items i - INNER JOIN downloaded_items di ON i.id = di.id - WHERE i.server_id = ? - AND ( - i.parent_id = ? OR i.album_id = ? OR i.season_id = ? OR i.series_id = ? + let parent_is_library = self.is_library(parent_id).await?; + let downloaded = downloaded_sql("i", PLAYABLE_TYPES, CONTAINER_TYPES); + let downloaded_parent = downloaded_sql("parent", PLAYABLE_TYPES, CONTAINER_TYPES); + // A non-library parent lists its children by `container_id` (migration + // 027) — series → seasons → episodes, one index range. The library + // clause is only added for a library, where it is the whole point. + let parent_match = if parent_is_library { + format!( + "i.server_id = ? + AND ( + i.container_id = ? OR ( EXISTS ( SELECT 1 FROM libraries l @@ -1254,31 +1392,38 @@ impl OfflineRepository { ) -- Top-level only: hide leaves whose container is downloaded. AND NOT EXISTS ( - SELECT 1 FROM downloaded_items parent - WHERE parent.id = i.album_id - OR parent.id = i.season_id - OR parent.id = i.series_id - OR parent.id = i.parent_id + SELECT 1 FROM items parent + WHERE parent.id IN (i.album_id, i.season_id, i.series_id, i.parent_id) + AND {downloaded_parent} ) ) - ){type_filter} + )", + membership = Self::LIBRARY_HOLDS_ITEM, + ) + } else { + "+i.server_id = ? AND i.container_id = ?".to_string() + }; + let sql = format!( + "SELECT i.id, i.name, i.item_type, i.server_id, i.parent_id, i.library_id, i.overview, i.genres, + i.runtime_ticks, i.production_year, i.community_rating, i.official_rating, + i.primary_image_tag, i.album_id, i.album_name, i.album_artist, i.artists, + i.index_number, i.series_id, i.series_name, i.season_id, i.season_name, + i.parent_index_number, i.is_folder, i.premiere_date + FROM items i + WHERE {parent_match} + AND {downloaded}{type_filter} ORDER BY i.sort_name ASC, i.name ASC LIMIT {limit} OFFSET {start_index}", - cte = Self::DOWNLOADED_ITEMS_CTE, - membership = Self::LIBRARY_HOLDS_ITEM, ); - let query = Query::with_params( - sql, - vec![ - QueryParam::String(self.server_id.clone()), - QueryParam::String(parent_id.to_string()), - QueryParam::String(parent_id.to_string()), - QueryParam::String(parent_id.to_string()), - QueryParam::String(parent_id.to_string()), - QueryParam::String(parent_id.to_string()), - ], - ); + let mut params = vec![ + QueryParam::String(self.server_id.clone()), + QueryParam::String(parent_id.to_string()), + ]; + if parent_is_library { + params.push(QueryParam::String(parent_id.to_string())); + } + let query = Query::with_params(sql, params); let cached_items: Vec = self .db_service @@ -1307,19 +1452,18 @@ impl OfflineRepository { // completed download of a given media kind qualifies that library. let query = Query::with_params( format!( - "{cte} - SELECT l.id, l.name, l.collection_type, l.image_tag + "SELECT l.id, l.name, l.collection_type, l.image_tag FROM libraries l WHERE l.server_id = ? AND EXISTS ( SELECT 1 FROM items i - INNER JOIN downloaded_items di ON i.id = di.id WHERE i.server_id = l.server_id AND {membership} + AND {downloaded} ) ORDER BY l.sort_order ASC, l.name ASC", - cte = Self::DOWNLOADED_ITEMS_CTE, membership = Self::LIBRARY_HOLDS_ITEM, + downloaded = downloaded_sql("i", PLAYABLE_TYPES, CONTAINER_TYPES), ), vec![QueryParam::String(self.server_id.clone())], ); @@ -1607,23 +1751,9 @@ impl MediaRepository for OfflineRepository { "" }; - // Decided up front so the listing can use the hierarchy indexes; see + // Decided up front so the listing can use the container index; see // `items_listing_sql`. - let parent_is_library = self - .db_service - .query_optional( - Query::with_params( - "SELECT 1 FROM libraries WHERE id = ? AND server_id = ?", - vec![ - QueryParam::String(parent_id.to_string()), - QueryParam::String(self.server_id.clone()), - ], - ), - |row| row.get::<_, i64>(0), - ) - .await - .map_err(|e| RepoError::Database { message: e })? - .is_some(); + let parent_is_library = self.is_library(parent_id).await?; let sql = items_listing_sql( parent_is_library, @@ -1635,17 +1765,11 @@ impl MediaRepository for OfflineRepository { start_index, ); - // The requested id is compared against every hierarchy-linkage column - // because `parent_id` is not populated for cached items — music tracks - // link to their album via `album_id`, episodes to their season/series - // via `season_id`/`series_id`, and a library parent matches via the - // `libraries` EXISTS clause. See [[offline-libraries-never-cached]]. + // Children are matched on `container_id` (see `items_listing_sql`); a + // library parent also matches through the `libraries` EXISTS clause. let mut params = vec![ QueryParam::String(self.server_id.clone()), - QueryParam::String(parent_id.to_string()), // i.parent_id = ? - QueryParam::String(parent_id.to_string()), // i.album_id = ? - QueryParam::String(parent_id.to_string()), // i.season_id = ? - QueryParam::String(parent_id.to_string()), // i.series_id = ? + QueryParam::String(parent_id.to_string()), // i.container_id = ? ]; if parent_is_library { params.push(QueryParam::String(parent_id.to_string())); // libraries.id = ? @@ -1691,32 +1815,16 @@ impl MediaRepository for OfflineRepository { async fn get_item(&self, item_id: &str) -> Result { // Check if item is available offline (either downloaded itself or has downloaded children) let query = Query::with_params( - "WITH downloaded_items AS ( - -- Playable items with completed downloads - SELECT DISTINCT i.id + format!( + "SELECT i.id, i.name, i.item_type, i.server_id, i.parent_id, i.library_id, i.overview, i.genres, + i.runtime_ticks, i.production_year, i.community_rating, i.official_rating, + i.primary_image_tag, i.album_id, i.album_name, i.album_artist, i.artists, + i.index_number, i.series_id, i.series_name, i.season_id, i.season_name, + i.parent_index_number, i.is_folder, i.premiere_date FROM items i - INNER JOIN downloads d ON i.id = d.item_id - WHERE d.status = 'completed' - AND i.item_type IN ('Audio', 'Movie', 'Episode') - - UNION - - -- Containers with downloaded children - SELECT DISTINCT i.id - FROM items i - INNER JOIN items children ON (children.parent_id = i.id OR children.album_id = i.id OR children.season_id = i.id OR children.series_id = i.id) - INNER JOIN downloads d ON children.id = d.item_id - WHERE d.status = 'completed' - AND i.item_type IN ('MusicAlbum', 'Series', 'Season', 'BoxSet', 'Folder', 'CollectionFolder') - ) - SELECT i.id, i.name, i.item_type, i.server_id, i.parent_id, i.library_id, i.overview, i.genres, - i.runtime_ticks, i.production_year, i.community_rating, i.official_rating, - i.primary_image_tag, i.album_id, i.album_name, i.album_artist, i.artists, - i.index_number, i.series_id, i.series_name, i.season_id, i.season_name, - i.parent_index_number, i.is_folder, i.premiere_date - FROM items i - INNER JOIN downloaded_items di ON i.id = di.id - WHERE i.id = ?", + WHERE i.id = ? AND {}", + downloaded_sql("i", PLAYABLE_TYPES, CONTAINER_TYPES) + ), vec![QueryParam::String(item_id.to_string())], ); @@ -1745,42 +1853,27 @@ impl MediaRepository for OfflineRepository { let query = Query::with_params( format!( - "WITH downloaded_items AS ( - -- Playable items with completed downloads - SELECT DISTINCT i.id - FROM items i - INNER JOIN downloads d ON i.id = d.item_id - WHERE d.status = 'completed' - AND i.item_type IN ('Audio', 'Movie', 'Episode') - - UNION - - -- Containers with downloaded children - SELECT DISTINCT i.id - FROM items i - INNER JOIN items children ON (children.parent_id = i.id OR children.album_id = i.id OR children.season_id = i.id OR children.series_id = i.id) - INNER JOIN downloads d ON children.id = d.item_id - WHERE d.status = 'completed' - AND i.item_type IN ('MusicAlbum', 'Series', 'Season', 'BoxSet', 'Folder', 'CollectionFolder') - ) - SELECT i.id, i.name, i.item_type, i.server_id, i.parent_id, i.library_id, i.overview, i.genres, + "SELECT i.id, i.name, i.item_type, i.server_id, i.parent_id, i.library_id, i.overview, i.genres, i.runtime_ticks, i.production_year, i.community_rating, i.official_rating, i.primary_image_tag, i.album_id, i.album_name, i.album_artist, i.artists, i.index_number, i.series_id, i.series_name, i.season_id, i.season_name, i.parent_index_number, i.is_folder, i.premiere_date FROM items i - INNER JOIN downloaded_items di ON i.id = di.id WHERE i.server_id = ? AND i.library_id = ? + AND {downloaded} -- Collapse leaves into the container that was added: a new -- 14-track album should read as one album, not 14 songs. Only -- drops a leaf when its own container is present in the same -- result, so a standalone track or movie still appears. AND NOT EXISTS ( - SELECT 1 FROM downloaded_items parent + SELECT 1 FROM items parent WHERE parent.id IN (i.album_id, i.season_id, i.series_id, i.parent_id) + AND {downloaded_parent} ) ORDER BY i.synced_at DESC - LIMIT {}", limit_val + LIMIT {limit_val}", + downloaded = downloaded_sql("i", PLAYABLE_TYPES, CONTAINER_TYPES), + downloaded_parent = downloaded_sql("parent", PLAYABLE_TYPES, CONTAINER_TYPES), ), vec![ QueryParam::String(self.server_id.clone()), @@ -1895,25 +1988,7 @@ impl MediaRepository for OfflineRepository { // Only shows items that are downloaded or have downloaded children let query = Query::with_params( format!( - "WITH downloaded_items AS ( - -- Playable items with completed downloads (Audio tracks) - SELECT DISTINCT i.id - FROM items i - INNER JOIN downloads d ON i.id = d.item_id - WHERE d.status = 'completed' - AND i.item_type = 'Audio' - - UNION - - -- Containers with downloaded children (Albums) - SELECT DISTINCT i.id - FROM items i - INNER JOIN items children ON (children.parent_id = i.id OR children.album_id = i.id OR children.season_id = i.id OR children.series_id = i.id) - INNER JOIN downloads d ON children.id = d.item_id - WHERE d.status = 'completed' - AND i.item_type = 'MusicAlbum' - ), - ranked_plays AS ( + "WITH ranked_plays AS ( SELECT CASE WHEN ud.playback_context_type = 'container' THEN ud.playback_context_id @@ -1938,9 +2013,10 @@ impl MediaRepository for OfflineRepository { i.season_name, i.parent_index_number, i.is_folder, i.premiere_date FROM ranked_plays rp JOIN items i ON rp.display_id = i.id - INNER JOIN downloaded_items di ON i.id = di.id + WHERE {downloaded} ORDER BY rp.most_recent_play DESC", - limit_val + limit_val, + downloaded = downloaded_sql("i", "'Audio'", "'MusicAlbum'") ), vec![ QueryParam::String(self.user_id.clone()), @@ -2076,40 +2152,10 @@ impl MediaRepository for OfflineRepository { // never disagree about what is visible. Before DR-108 this leg was // downloads-only, which meant a user with no downloads got nothing from // the local index and every keystroke fell through to the server. - let catalog_branch = if include_catalog_browse() { - "UNION - - -- Synced catalog: fast online search, or the offline - -- 'Show all server media' view. See set_include_catalog_browse. - SELECT DISTINCT i.id - FROM items i - WHERE i.synced_at IS NOT NULL" - } else { - "" - }; + let available = available_sql("i", include_catalog_browse()); let sql = format!( - "WITH available_items AS ( - -- Playable items with completed downloads - SELECT DISTINCT i.id - FROM items i - INNER JOIN downloads d ON i.id = d.item_id - WHERE d.status = 'completed' - AND i.item_type IN ('Audio', 'Movie', 'Episode') - - UNION - - -- Containers with downloaded children - SELECT DISTINCT i.id - FROM items i - INNER JOIN items children ON (children.parent_id = i.id OR children.album_id = i.id OR children.season_id = i.id OR children.series_id = i.id) - INNER JOIN downloads d ON children.id = d.item_id - WHERE d.status = 'completed' - AND i.item_type IN ('MusicAlbum', 'Series', 'Season', 'BoxSet', 'Folder', 'CollectionFolder') - - {} - ) - SELECT i.id, i.name, i.item_type, i.server_id, i.parent_id, i.library_id, + "SELECT i.id, i.name, i.item_type, i.server_id, i.parent_id, i.library_id, i.overview, i.genres, i.runtime_ticks, i.production_year, i.community_rating, i.official_rating, i.primary_image_tag, i.album_id, i.album_name, i.album_artist, i.artists, @@ -2117,11 +2163,10 @@ impl MediaRepository for OfflineRepository { i.season_name, i.parent_index_number, i.is_folder, i.premiere_date FROM items i JOIN items_fts fts ON fts.rowid = i.rowid - INNER JOIN available_items ai ON i.id = ai.id - WHERE i.server_id = ? AND items_fts MATCH ?{} + WHERE i.server_id = ? AND items_fts MATCH ? AND {}{} ORDER BY rank LIMIT {}", - catalog_branch, type_filter, limit + available, type_filter, limit ); let mut params = vec![ @@ -2304,46 +2349,20 @@ impl MediaRepository for OfflineRepository { _ => String::new(), }; - let catalog_branch = if include_catalog_browse() { - "UNION - - SELECT DISTINCT i.id - FROM items i - WHERE i.synced_at IS NOT NULL" - } else { - "" - }; + let available = available_sql("i", include_catalog_browse()); let sql = format!( - "WITH available_items AS ( - SELECT DISTINCT i.id - FROM items i - INNER JOIN downloads d ON i.id = d.item_id - WHERE d.status = 'completed' - AND i.item_type IN ('Audio', 'Movie', 'Episode') - - UNION - - SELECT DISTINCT i.id - FROM items i - INNER JOIN items children ON (children.parent_id = i.id OR children.album_id = i.id OR children.season_id = i.id OR children.series_id = i.id) - INNER JOIN downloads d ON children.id = d.item_id - WHERE d.status = 'completed' - AND i.item_type IN ('MusicAlbum', 'Series', 'Season', 'BoxSet', 'Folder', 'CollectionFolder') - - {catalog_branch} - ) - SELECT i.id, i.name, i.item_type, i.server_id, i.parent_id, i.library_id, i.overview, i.genres, + "SELECT i.id, i.name, i.item_type, i.server_id, i.parent_id, i.library_id, i.overview, i.genres, i.runtime_ticks, i.production_year, i.community_rating, i.official_rating, i.primary_image_tag, i.album_id, i.album_name, i.album_artist, i.artists, i.index_number, i.series_id, i.series_name, i.season_id, i.season_name, i.parent_index_number, i.is_folder, i.premiere_date FROM items i - INNER JOIN available_items ai ON i.id = ai.id INNER JOIN user_data ud ON ud.item_id = i.id WHERE i.server_id = ? AND ud.user_id = ? - AND ud.is_favorite = 1{} + AND ud.is_favorite = 1 + AND {available}{} ORDER BY i.sort_name ASC, i.name ASC LIMIT {} OFFSET {}", type_filter, limit, start_index @@ -2459,25 +2478,7 @@ impl MediaRepository for OfflineRepository { // Filter by downloads using CTE let query = Query::with_params( format!( - "WITH downloaded_items AS ( - -- Playable items with completed downloads - SELECT DISTINCT i.id - FROM items i - INNER JOIN downloads d ON i.id = d.item_id - WHERE d.status = 'completed' - AND i.item_type IN ('Audio', 'Movie', 'Episode') - - UNION - - -- Containers with downloaded children - SELECT DISTINCT i.id - FROM items i - INNER JOIN items children ON (children.parent_id = i.id OR children.album_id = i.id OR children.season_id = i.id OR children.series_id = i.id) - INNER JOIN downloads d ON children.id = d.item_id - WHERE d.status = 'completed' - AND i.item_type IN ('MusicAlbum', 'Series', 'Season', 'BoxSet', 'Folder', 'CollectionFolder') - ) - SELECT i.id, i.name, i.item_type, i.server_id, i.parent_id, i.library_id, + "SELECT i.id, i.name, i.item_type, i.server_id, i.parent_id, i.library_id, i.overview, i.genres, i.runtime_ticks, i.production_year, i.community_rating, i.official_rating, i.primary_image_tag, i.album_id, i.album_name, i.album_artist, i.artists, @@ -2485,10 +2486,11 @@ impl MediaRepository for OfflineRepository { i.season_name, i.parent_index_number, i.is_folder, i.premiere_date FROM items i JOIN item_people ip ON i.id = ip.item_id - INNER JOIN downloaded_items di ON i.id = di.id - WHERE i.server_id = ? AND ip.person_id = ? + WHERE i.server_id = ? AND ip.person_id = ? AND {downloaded} ORDER BY i.production_year DESC, i.sort_name ASC - LIMIT {}", limit + LIMIT {}", + limit, + downloaded = downloaded_sql("i", PLAYABLE_TYPES, CONTAINER_TYPES) ), vec![ QueryParam::String(self.server_id.clone()), @@ -2918,8 +2920,18 @@ mod tests { season_name TEXT, parent_index_number INTEGER, synced_at TEXT, - sort_name TEXT + sort_name TEXT, + -- Same rule as schema.rs migration 027. + container_id TEXT GENERATED ALWAYS AS ( + CASE item_type + WHEN 'Episode' THEN COALESCE(season_id, series_id, parent_id) + WHEN 'Season' THEN COALESCE(series_id, parent_id) + WHEN 'Audio' THEN COALESCE(album_id, parent_id) + ELSE parent_id + END + ) VIRTUAL ); + CREATE INDEX idx_items_container ON items(container_id, sort_name, name); -- Mirrors the real FTS5 index and its triggers (schema.rs migration -- 001) so search can be exercised in tests at all. @@ -3983,11 +3995,16 @@ mod tests { "get_items(season_id) should return the episode" ); - // Browsing the series returns the episode (via series_id link). + // Browsing the series returns the season that holds the download — + // series → season → episode, the same hierarchy the server serves. + // (It used to return the episode itself, via its `series_id`, beside + // the seasons; see `a_series_lists_its_seasons_not_their_episodes`.) let series_items = repo.get_items("series-1", None).await.unwrap(); - assert!( - series_items.items.iter().any(|i| i.id == "ep-1"), - "get_items(series_id) should surface the downloaded episode" + let ids: Vec<&str> = series_items.items.iter().map(|i| i.id.as_str()).collect(); + assert_eq!( + ids, + vec!["season-1"], + "get_items(series_id) should list the season holding the download" ); } @@ -4396,9 +4413,11 @@ mod tests { // Drilling into the series returns its season; into the season, the episode. let in_series = repo.get_downloaded_items("series-1", None).await.unwrap(); - assert!( - in_series.items.iter().any(|i| i.id == "season-1"), - "series drill returns the season" + let in_series_ids: Vec<&str> = in_series.items.iter().map(|i| i.id.as_str()).collect(); + assert_eq!( + in_series_ids, + vec!["season-1"], + "series drill returns the season — not the season's episodes" ); let in_season = repo.get_downloaded_items("season-1", None).await.unwrap(); assert!( @@ -5800,7 +5819,7 @@ mod tests { /// /// TRACES: UR-002 | DR-013 #[test] - fn listing_a_non_library_parent_uses_the_hierarchy_indexes() { + fn listing_a_non_library_parent_uses_the_container_index() { use crate::utils::lock::MutexSafe; let db = crate::storage::Database::open_in_memory().unwrap(); let conn = db.connection(); @@ -5817,16 +5836,14 @@ mod tests { ); let mut stmt = conn.prepare(&format!("EXPLAIN QUERY PLAN {sql}")).unwrap(); let plan: Vec = stmt - .query_map(rusqlite::params!["srv", "p", "p", "p", "p"], |row| { - row.get::<_, String>(3) - }) + .query_map(rusqlite::params!["srv", "p"], |row| row.get::<_, String>(3)) .unwrap() .map(Result::unwrap) .collect(); let plan = plan.join("\n"); assert!( - plan.contains("MULTI-INDEX OR"), - "expected an index lookup per hierarchy column, got:\n{plan}" + plan.contains("idx_items_container"), + "expected a container index lookup, got:\n{plan}" ); assert!( !plan.contains("SCAN i") && !plan.contains("idx_items_server"), @@ -5834,4 +5851,108 @@ mod tests { ); } } + + /// A series lists its seasons — not every episode of every season. + /// + /// Children were matched on four columns at once (`parent_id`, `album_id`, + /// `season_id`, `series_id`), and every episode carries its series id, so + /// a cached series answered with its eleven seasons *and* all ~275 + /// episodes. With the series page's `limit: 100`, episodes whose names sort + /// before "Season …" could push seasons out of the answer altogether. + /// + /// TRACES: UR-002, UR-007 | DR-013 + #[tokio::test] + async fn a_series_lists_its_seasons_not_their_episodes() { + let _guard = lock_catalog_browse(); + set_include_catalog_browse(true); + let db_service = create_test_db(); + for sql in [ + "INSERT INTO items (id, server_id, name, item_type, synced_at) \ + VALUES ('show', 'test-server', 'Show', 'Series', '2026-01-01')", + "INSERT INTO items (id, server_id, parent_id, series_id, name, item_type, synced_at) \ + VALUES ('s1', 'test-server', 'show', 'show', 'Season 1', 'Season', '2026-01-01')", + "INSERT INTO items (id, server_id, parent_id, season_id, series_id, name, item_type, synced_at) \ + VALUES ('e1', 'test-server', 's1', 's1', 'show', 'A Pilot', 'Episode', '2026-01-01')", + ] { + db_service.execute(Query::new(sql)).await.unwrap(); + } + let repo = OfflineRepository::new( + db_service.clone(), + "test-server".to_string(), + "test-user".to_string(), + ); + + let ids: Vec = repo + .get_items("show", None) + .await + .unwrap() + .items + .into_iter() + .map(|i| i.id) + .collect(); + assert_eq!(ids, vec!["s1".to_string()]); + } + + /// Caching an episode (or a track) whose season, series (or album) was + /// never cached must leave that container reachable: a series lists its + /// seasons, so without a season row a downloaded episode would be + /// unreachable from its series page offline. + /// + /// TRACES: UR-002, UR-007 | DR-013 + #[tokio::test] + async fn caching_a_child_creates_its_missing_containers() { + let db_service = create_test_db(); + let repo = OfflineRepository::new( + db_service.clone(), + "test-server".to_string(), + "test-user".to_string(), + ); + let mut episode = create_test_item("ep", "Pilot", Some("next-up")); + episode.item_type = "Episode".to_string(); + episode.season_id = Some("season-9".to_string()); + episode.season_name = Some("Season 9".to_string()); + episode.series_id = Some("show-9".to_string()); + episode.series_name = Some("Show 9".to_string()); + let mut track = create_test_item("trk", "Song", Some("next-up")); + track.item_type = "Audio".to_string(); + track.album_id = Some("alb-9".to_string()); + track.album_name = Some("Record".to_string()); + repo.save_to_cache("next-up", &[episode, track]) + .await + .unwrap(); + + let row = |id: &'static str| { + let db_service = db_service.clone(); + async move { + db_service + .query_optional( + Query::with_params( + "SELECT name, item_type, container_id FROM items WHERE id = ?", + vec![QueryParam::String(id.to_string())], + ), + |r| { + Ok(( + r.get::<_, String>(0)?, + r.get::<_, String>(1)?, + r.get::<_, Option>(2)?, + )) + }, + ) + .await + .unwrap() + } + }; + assert_eq!( + row("season-9").await, + Some(("Season 9".into(), "Season".into(), Some("show-9".into()))) + ); + assert_eq!( + row("show-9").await, + Some(("Show 9".into(), "Series".into(), None)) + ); + assert_eq!( + row("alb-9").await, + Some(("Record".into(), "MusicAlbum".into(), None)) + ); + } } diff --git a/src-tauri/src/storage/mod.rs b/src-tauri/src/storage/mod.rs index 3dab59eee..7502315fb 100644 --- a/src-tauri/src/storage/mod.rs +++ b/src-tauri/src/storage/mod.rs @@ -65,6 +65,18 @@ impl Database { let conn = Arc::new(Mutex::new(conn)); Self::migrate_connection(&conn, MIGRATIONS)?; + // Planner statistics. Without them SQLite guesses between indexes, and + // guessed badly for the listing query (see 08-database-design.md → + // "Listing query shape"). `optimize` only analyses what is missing or + // stale; `analysis_limit` bounds each table's scan so this stays in the + // milliseconds on a large catalogue. Failure is not fatal. + if let Err(e) = conn + .lock_safe() + .execute_batch("PRAGMA analysis_limit = 400; PRAGMA optimize = 0x10002;") + { + error!("PRAGMA optimize failed: {}", e); + } + // Readers open after migrations, so they only ever see the final schema. let readers = (0..READER_CONNECTIONS) .map(|_| Self::open_reader(path)) @@ -1000,6 +1012,55 @@ mod tests { assert!(busy_timeout > 0, "expected a busy timeout"); } + /// The planner gets statistics: the app used to never run `ANALYZE`, so + /// SQLite guessed between indexes — and for the listing query guessed the + /// `server_id` index, which every row shares, turning an index lookup into + /// a walk of the whole catalogue. `PRAGMA optimize` at open refreshes + /// whatever statistics are missing or stale, bounded by `analysis_limit`. + /// + /// TRACES: UR-002 | DR-012 | UT-014 + #[test] + fn open_gives_the_planner_statistics() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("jellytau.db"); + { + let db = Database::open(&path).unwrap(); + let conn = db.connection(); + let conn = conn.lock_safe(); + conn.execute_batch( + "INSERT INTO servers (id, name, url) VALUES ('s', 'S', 'http://s');", + ) + .unwrap(); + for i in 0..2000 { + conn.execute( + "INSERT INTO items (id, server_id, name, item_type) VALUES (?1, 's', 'n', 'Audio')", + [format!("i{i}")], + ) + .unwrap(); + } + } + + let db = Database::open(&path).unwrap(); + let conn = db.connection(); + let conn = conn.lock_safe(); + let analysed: i64 = conn + .query_row( + "SELECT COUNT(*) FROM sqlite_master WHERE name = 'sqlite_stat1'", + [], + |r| r.get(0), + ) + .unwrap(); + assert_eq!(analysed, 1, "the planner has no statistics"); + let items_stats: i64 = conn + .query_row( + "SELECT COUNT(*) FROM sqlite_stat1 WHERE tbl = 'items'", + [], + |r| r.get(0), + ) + .unwrap(); + assert!(items_stats > 0, "no statistics for items"); + } + /// Writes a phone-sized catalogue to `$JELLYTAU_BENCH_DB` for timing /// queries with the `sqlite3` CLI. Not a test; run explicitly with /// `--ignored`. diff --git a/src-tauri/src/storage/schema.rs b/src-tauri/src/storage/schema.rs index f47f61d55..faf3024ad 100644 --- a/src-tauri/src/storage/schema.rs +++ b/src-tauri/src/storage/schema.rs @@ -31,6 +31,7 @@ pub const MIGRATIONS: &[(&str, &str)] = &[ ("024_multi_user_profiles", MIGRATION_024), ("025_backfill_item_library_id", MIGRATION_025), ("026_server_catalog_generation", MIGRATION_026), + ("027_items_container_id", MIGRATION_027), ]; /// Initial schema migration @@ -941,6 +942,169 @@ const MIGRATION_026: &str = r#" ALTER TABLE servers ADD COLUMN catalog_generation TEXT; "#; +/// One canonical "which container lists this item" link, plus an index that +/// serves a listing in display order. +/// +/// Jellyfin's `ParentId` is the *storage* parent, not the logical one: in a +/// series without season folders an episode's `ParentId` is the series while +/// its `SeasonId` names a virtual season, and a cached episode may arrive +/// without its season row at all. So listings matched children on four +/// columns at once (`parent_id`, `album_id`, `season_id`, `series_id`). That +/// was slow — the `OR` defeated the planner into walking the whole table — and +/// wrong: every episode carries its series id, so a series answered with its +/// seasons *and* all their episodes. +/// +/// `container_id` resolves the logical container once, by rule: an episode +/// belongs to its season (else its series, else its parent), a season to its +/// series, a track to its album, anything else to its parent. It is a VIRTUAL +/// generated column, so every write path — cache, downloads, catalog crawl — +/// is covered without touching any of them, and it cannot drift from the +/// columns it is computed from. The index covers the listing's +/// `ORDER BY sort_name, name` (`sort_name` is usually NULL in the cache). +/// +/// The placeholders keep offline navigation intact: an episode whose season +/// or series row was never cached used to surface directly under the series +/// through the `series_id` match. Now it lists under its season, so the season +/// (and series, and a track's album) must exist. They are built from the +/// names the child rows already carry, with `synced_at` NULL — they only show +/// when a download makes them available, and a real row from the server +/// replaces them wholesale (`save_to_cache` upserts every field). +/// +/// TRACES: UR-002, UR-007 | DR-013 +const MIGRATION_027: &str = r#" +ALTER TABLE items ADD COLUMN container_id TEXT GENERATED ALWAYS AS ( + CASE item_type + WHEN 'Episode' THEN COALESCE(season_id, series_id, parent_id) + WHEN 'Season' THEN COALESCE(series_id, parent_id) + WHEN 'Audio' THEN COALESCE(album_id, parent_id) + ELSE parent_id + END +) VIRTUAL; + +CREATE INDEX IF NOT EXISTS idx_items_container ON items(container_id, sort_name, name); + +INSERT OR IGNORE INTO items (id, server_id, library_id, name, item_type, is_folder, series_id, series_name) +SELECT season_id, server_id, MAX(library_id), COALESCE(MAX(season_name), 'Season'), 'Season', 1, + MAX(series_id), MAX(series_name) +FROM items +WHERE item_type = 'Episode' AND season_id IS NOT NULL +GROUP BY season_id; + +INSERT OR IGNORE INTO items (id, server_id, library_id, name, item_type, is_folder) +SELECT series_id, server_id, MAX(library_id), COALESCE(MAX(series_name), 'Series'), 'Series', 1 +FROM items +WHERE item_type IN ('Episode', 'Season') AND series_id IS NOT NULL +GROUP BY series_id; + +INSERT OR IGNORE INTO items (id, server_id, library_id, name, item_type, is_folder, album_artist) +SELECT album_id, server_id, MAX(library_id), COALESCE(MAX(album_name), 'Album'), 'MusicAlbum', 1, + MAX(album_artist) +FROM items +WHERE item_type = 'Audio' AND album_id IS NOT NULL +GROUP BY album_id; +"#; + +#[cfg(test)] +mod migration_027_tests { + use super::*; + use rusqlite::Connection; + + fn pre_027_db() -> Connection { + let conn = Connection::open_in_memory().unwrap(); + let upto = MIGRATIONS + .iter() + .position(|(name, _)| *name == "027_items_container_id") + .expect("migration 027 must be registered"); + for (_, sql) in &MIGRATIONS[..upto] { + conn.execute_batch(sql).unwrap(); + } + conn.execute_batch( + "INSERT INTO servers (id, name, url) VALUES ('s', 'S', 'http://s'); + -- An episode cached without its season or series rows. + INSERT INTO items (id, server_id, name, item_type, parent_id, season_id, season_name, + series_id, series_name, library_id) + VALUES ('ep', 's', 'Pilot', 'Episode', NULL, 'season', 'Season 1', + 'show', 'Show', NULL); + -- A track cached without its album. + INSERT INTO items (id, server_id, name, item_type, album_id, album_name, album_artist) + VALUES ('trk', 's', 'Song', 'Audio', 'alb', 'Record', 'Band'); + -- A folder child: its container is just its parent. + INSERT INTO items (id, server_id, name, item_type) VALUES ('box', 's', 'Box', 'BoxSet'); + INSERT INTO items (id, server_id, name, item_type, parent_id) + VALUES ('film', 's', 'Film', 'Movie', 'box');", + ) + .unwrap(); + conn + } + + fn container(conn: &Connection, id: &str) -> Option { + conn.query_row("SELECT container_id FROM items WHERE id = ?1", [id], |r| { + r.get(0) + }) + .unwrap() + } + + /// TRACES: UR-002, UR-007 | DR-013 + #[test] + fn every_item_resolves_to_its_logical_container() { + let conn = pre_027_db(); + conn.execute_batch(MIGRATION_027).unwrap(); + + assert_eq!(container(&conn, "ep").as_deref(), Some("season")); + assert_eq!(container(&conn, "season").as_deref(), Some("show")); + assert_eq!(container(&conn, "trk").as_deref(), Some("alb")); + assert_eq!(container(&conn, "film").as_deref(), Some("box")); + assert_eq!(container(&conn, "show"), None); + } + + /// Containers that were never cached get placeholders named from their + /// children, so an offline episode is still reachable series → season. + /// + /// TRACES: UR-002, UR-007 | DR-013 + #[test] + fn missing_containers_get_named_placeholders() { + let conn = pre_027_db(); + conn.execute_batch(MIGRATION_027).unwrap(); + + let row = |id: &str| -> (String, String, Option) { + conn.query_row( + "SELECT name, item_type, synced_at FROM items WHERE id = ?1", + [id], + |r| Ok((r.get(0)?, r.get(1)?, r.get(2)?)), + ) + .unwrap() + }; + assert_eq!(row("season"), ("Season 1".into(), "Season".into(), None)); + assert_eq!(row("show"), ("Show".into(), "Series".into(), None)); + assert_eq!(row("alb"), ("Record".into(), "MusicAlbum".into(), None)); + } + + /// Listing a container is one index range, already in display order. + /// + /// TRACES: UR-002, UR-007 | DR-013 + #[test] + fn a_container_listing_is_an_ordered_index_range() { + let conn = pre_027_db(); + conn.execute_batch(MIGRATION_027).unwrap(); + let plan: Vec = conn + .prepare( + "EXPLAIN QUERY PLAN SELECT id FROM items + WHERE container_id = ?1 ORDER BY sort_name, name", + ) + .unwrap() + .query_map(["season"], |r| r.get::<_, String>(3)) + .unwrap() + .map(Result::unwrap) + .collect(); + let plan = plan.join("\n"); + assert!(plan.contains("idx_items_container"), "{plan}"); + assert!( + !plan.contains("TEMP B-TREE"), + "listing needs a sort step:\n{plan}" + ); + } +} + #[cfg(test)] mod migration_024_tests { use super::*;