diff --git a/src-tauri/src/commands/library.rs b/src-tauri/src/commands/library.rs new file mode 100644 index 00000000..80337c22 --- /dev/null +++ b/src-tauri/src/commands/library.rs @@ -0,0 +1,323 @@ +//! Library browsing preferences — currently, which folders are hidden. +//! +//! The setting replaces a hardcoded frontend filter that dropped any item +//! literally named "Podcasts", which was one user's folder layout keyed on an +//! English string and shipped to everyone. What is hidden is now a user choice +//! made of stable ids, applied in the repository layer +//! (`repository::exclusions`) so every query path agrees; the frontend only +//! renders a picker over the candidates this module serves. +//! +//! TRACES: UR-076 | DR-209 + +use std::sync::Arc; + +use log::{debug, info, warn}; +use tauri::{Manager, State}; + +use crate::commands::repository::RepositoryManagerWrapper; +use crate::commands::storage::DatabaseWrapper; +use crate::repository::exclusions; +use crate::repository::types::{GetItemsOptions, SearchScope}; +use crate::repository::MediaRepository; +use crate::settings::LibrarySettings; +use crate::storage::db_service::{DatabaseService, Query, QueryParam}; +use crate::utils::lock::MutexSafe; + +/// `app_settings` key holding the persisted library preferences (JSON). +/// +/// Persisted for the same reason the streaming cap is: a hidden folder that +/// silently comes back on the next launch is a setting the user has to keep +/// re-applying, and they would have no way to tell it had been forgotten. +const LIBRARY_SETTINGS_KEY: &str = "library_settings"; + +/// How many immediate children of a library the picker will consider. +/// +/// A music library's root listing is folders and (on some layouts) artists, not +/// the whole catalog, so this is generous. It exists to stop a pathological +/// library from turning the settings page into an unbounded fetch. +const CANDIDATE_SCAN_LIMIT: usize = 500; + +/// Something the user may choose to hide: a library, or a folder directly +/// inside one. +/// +/// Which containers are *offerable* is a domain question (it depends on the +/// library's Jellyfin collection type and on what counts as a folder), so the +/// list is assembled here and the frontend renders it verbatim. +/// +/// TRACES: UR-076 | DR-209 +#[derive(specta::Type, Debug, Clone, serde::Serialize, serde::Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct ExclusionCandidate { + /// Stable Jellyfin item id — what gets stored when the user picks it. + pub id: String, + /// Display name of the folder (or of the library, for a whole-library entry). + pub name: String, + /// Library this candidate lives in, so the picker can group and disambiguate + /// two folders that share a name. + pub library_name: String, + /// True when the candidate *is* a library rather than a folder inside one. + pub is_library: bool, +} + +/// The library preferences currently in force. +/// +/// Read from the in-memory exclusion set rather than the database: that set is +/// what queries actually consult, so reading it is the only answer that cannot +/// disagree with what the user is seeing. +/// +/// TRACES: UR-076 | DR-209 +#[tauri::command] +#[specta::specta] +pub async fn library_get_settings() -> Result { + Ok(LibrarySettings { + excluded_item_ids: exclusions::excluded_item_ids(), + }) +} + +/// Replace the library preferences: apply them to every subsequent query and +/// persist them. +/// +/// Returns the sanitised value actually applied, so the picker shows what was +/// stored rather than what it sent. +/// +/// TRACES: UR-076 | DR-209 +#[tauri::command] +#[specta::specta] +pub async fn library_set_settings( + db: State<'_, DatabaseWrapper>, + settings: LibrarySettings, +) -> Result { + let sanitised = settings.sanitised(); + exclusions::set_excluded_item_ids(&sanitised.excluded_item_ids); + persist_library_settings(&db, &sanitised).await; + info!( + "[Library] {} folder(s) hidden from browsing", + sanitised.excluded_item_ids.len() + ); + Ok(sanitised) +} + +/// The folders the user may choose to hide. +/// +/// Offers each music library and the folders directly inside it. Music is the +/// only scope offered because it is the one where a foreign folder — podcasts, +/// audiobooks, sound effects — routinely shares a library with the media the +/// user actually browses; the scope is decided here rather than in the UI so the +/// collection-type table stays out of the frontend +/// (see `SearchScope::for_collection_type`). +/// +/// Reads through `HybridRepository::get_items_unfiltered` so folders that are +/// *already* hidden still appear — otherwise the setting could never be undone. +/// +/// TRACES: UR-076 | DR-209 +#[tauri::command] +#[specta::specta] +pub async fn library_get_exclusion_candidates( + manager: State<'_, RepositoryManagerWrapper>, + handle: String, +) -> Result, String> { + let repo = manager.0.get(&handle).ok_or("Repository not found")?; + + let libraries = repo + .as_ref() + .get_libraries() + .await + .map_err(|e| format!("{:?}", e))?; + + let mut candidates: Vec = Vec::new(); + + for library in libraries { + if SearchScope::for_collection_type(&library.collection_type) != Some(SearchScope::Music) { + continue; + } + + candidates.push(ExclusionCandidate { + id: library.id.clone(), + name: library.name.clone(), + library_name: library.name.clone(), + is_library: true, + }); + + let options = GetItemsOptions { + recursive: Some(false), + sort_by: Some("SortName".to_string()), + sort_order: Some("Ascending".to_string()), + limit: Some(CANDIDATE_SCAN_LIMIT), + ..Default::default() + }; + + match repo.get_items_unfiltered(&library.id, Some(options)).await { + Ok(result) => { + for item in result.items { + if !item.is_folder { + continue; + } + candidates.push(ExclusionCandidate { + id: item.id, + name: item.name, + library_name: library.name.clone(), + is_library: false, + }); + } + } + Err(e) => { + // One unreachable library must not cost the user the picker for + // the others — an empty section is recoverable, an error is not. + warn!( + "[Library] Could not list folders in {}: {:?}", + library.name, e + ); + } + } + } + + debug!("[Library] {} exclusion candidate(s)", candidates.len()); + Ok(candidates) +} + +/// Write the preferences to `app_settings`. +/// +/// Failure is logged, not returned: the setting has already been applied in +/// memory, and failing the whole call because the write failed would leave the +/// picker showing a state that *is* in force. +/// +/// TRACES: UR-076 | DR-209 +async fn persist_library_settings(db: &State<'_, DatabaseWrapper>, settings: &LibrarySettings) { + let db_service = { + let database = db.0.lock_safe(); + Arc::new(database.service()) + }; + + let encoded = match serde_json::to_string(settings) { + Ok(value) => value, + Err(e) => { + warn!("[Library] Failed to encode library settings: {}", e); + return; + } + }; + + let query = Query::with_params( + "INSERT OR REPLACE INTO app_settings (key, value, updated_at) + VALUES (?, ?, CURRENT_TIMESTAMP)", + vec![ + QueryParam::String(LIBRARY_SETTINGS_KEY.to_string()), + QueryParam::String(encoded), + ], + ); + + if let Err(e) = db_service.execute(query).await { + warn!("[Library] Failed to persist library settings: {}", e); + } +} + +/// Restore the persisted preferences at startup, into the exclusion set the +/// repository consults. +/// +/// Called from the Tauri `setup` hook. A missing or unreadable row leaves the +/// default — nothing hidden — in place, so a database problem shows the user +/// more than they asked for rather than less. +/// +/// TRACES: UR-076 | DR-209 +pub async fn restore_library_settings(app: &tauri::AppHandle) { + let db_service = { + let Some(db) = app.try_state::() else { + warn!("[Library] No database available; nothing hidden from browsing"); + return; + }; + let database = db.0.lock_safe(); + Arc::new(database.service()) + }; + + let query = Query::with_params( + "SELECT value FROM app_settings WHERE key = ?", + vec![QueryParam::String(LIBRARY_SETTINGS_KEY.to_string())], + ); + + let stored: Option = match db_service.query_optional(query, |row| row.get(0)).await { + Ok(value) => value, + Err(e) => { + warn!("[Library] Failed to read library settings: {}", e); + return; + } + }; + + let Some(stored) = stored else { return }; + let settings: LibrarySettings = match serde_json::from_str(&stored) { + Ok(settings) => settings, + Err(e) => { + warn!( + "[Library] Ignoring unreadable persisted library settings {:?}: {}", + stored, e + ); + return; + } + }; + + let settings = settings.sanitised(); + exclusions::set_excluded_item_ids(&settings.excluded_item_ids); + if !settings.excluded_item_ids.is_empty() { + info!( + "[Library] Restored {} hidden folder(s)", + settings.excluded_item_ids.len() + ); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// The persisted form must round-trip through the same camelCase JSON the + /// IPC boundary uses — a rename here silently un-hides every folder the user + /// chose, with no setting having been changed. + /// + /// TRACES: UR-076 | DR-209 | UT-203 + #[test] + fn test_library_settings_round_trip_through_json() { + let settings = LibrarySettings { + excluded_item_ids: vec!["folder-1".to_string(), "folder-2".to_string()], + }; + + let json = serde_json::to_string(&settings).expect("serialises"); + assert!(json.contains("\"excludedItemIds\""), "camelCase on the wire"); + + let parsed: LibrarySettings = serde_json::from_str(&json).expect("parses back"); + assert_eq!(parsed, settings); + } + + /// Settings persisted before this feature existed — and a row with the key + /// missing entirely — must load as "nothing hidden", never as an error the + /// caller has to handle or a default that hides something. + /// + /// TRACES: UR-076 | DR-209 | UT-203 + #[test] + fn test_library_settings_default_hides_nothing() { + let parsed: LibrarySettings = serde_json::from_str("{}").expect("parses"); + assert!(parsed.excluded_item_ids.is_empty()); + assert!(LibrarySettings::default().excluded_item_ids.is_empty()); + } + + /// Blank and duplicate ids are dropped on the way in, so a half-written or + /// hand-edited value cannot grow the list without bound or store an id that + /// matches nothing yet still shows as a selection. + /// + /// TRACES: UR-076 | DR-209 | UT-203 + #[test] + fn test_library_settings_sanitised() { + let settings = LibrarySettings { + excluded_item_ids: vec![ + " folder-1 ".to_string(), + "".to_string(), + " ".to_string(), + "folder-1".to_string(), + "folder-2".to_string(), + ], + } + .sanitised(); + + assert_eq!( + settings.excluded_item_ids, + vec!["folder-1".to_string(), "folder-2".to_string()] + ); + } +} diff --git a/src-tauri/src/commands/mod.rs b/src-tauri/src/commands/mod.rs index 3a3bbb89..a66b709b 100644 --- a/src-tauri/src/commands/mod.rs +++ b/src-tauri/src/commands/mod.rs @@ -8,6 +8,7 @@ pub mod conversions; pub mod device; pub mod download; pub mod favorites; +pub mod library; pub mod offline; pub mod playback_mode; pub mod playback_reporting; @@ -25,6 +26,7 @@ pub use connectivity::*; pub use conversions::*; pub use device::*; pub use download::*; +pub use library::*; pub use offline::*; pub use playback_mode::*; #[allow(unused_imports)] // Used when playback_reporting is fully integrated diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index d22251f3..ad1bac86 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -79,6 +79,10 @@ use commands::{ get_smart_cache_stats, image_get_url, is_item_pinned, + // Library browsing preferences (hidden folders) + library_get_exclusion_candidates, + library_get_settings, + library_set_settings, lms_create_sync_group, lms_dissolve_sync_group, // LMS multi-room sync group commands @@ -884,6 +888,10 @@ fn specta_builder() -> Builder { sync_full_catalog, catalog_sync_status, set_show_server_catalog, + // Library browsing preferences (UR-076 / DR-209) + library_get_settings, + library_set_settings, + library_get_exclusion_candidates, resume_queued_downloads, get_download_manager_stats, set_max_concurrent_downloads, @@ -1312,6 +1320,18 @@ pub fn run() { }); } + // Restore the folders the user hid from browsing, for the same + // reason and in the same way. Until it lands nothing is hidden — + // the pre-existing behaviour — and no query can have run this early. + // + // TRACES: UR-076 | DR-209 + { + let handle = app.handle().clone(); + tauri::async_runtime::spawn(async move { + crate::commands::restore_library_settings(&handle).await; + }); + } + // Initialize thumbnail cache info!("[INIT] Initializing thumbnail cache..."); let app_data_dir = if let Ok(test_data_dir) = std::env::var("JELLYTAU_DATA_DIR") { diff --git a/src-tauri/src/repository/exclusions.rs b/src-tauri/src/repository/exclusions.rs new file mode 100644 index 00000000..cb9e46a6 --- /dev/null +++ b/src-tauri/src/repository/exclusions.rs @@ -0,0 +1,359 @@ +//! Library folders the user has chosen to keep out of browsing. +//! +//! Some people file things inside a library that they never want to see while +//! browsing it — a "Podcasts" folder sitting in the music library is the +//! canonical case: its albums and tracks leak into album, artist, track and +//! playlist listings even though the user thinks of them as a different medium. +//! +//! This is a *domain* rule, not a presentation one: what an item belongs to, and +//! therefore whether a query should return it, is decided here in the repository +//! layer so every query path agrees. The predecessor of this module was a +//! frontend filter that dropped anything literally named "Podcasts" — one user's +//! folder layout, keyed on an English string, shipped to everyone. Excluding by +//! **id** instead of name is what makes the setting survive a rename, a +//! translation, or two folders sharing a name. +//! +//! The excluded set is process-wide rather than a field on a repository for the +//! same reason as `online::STREAMING_QUALITY`: it is a preference about *this +//! user's browsing*, not about a server session, so it must survive a repository +//! being rebuilt on re-login. It is written by the settings command and restored +//! from the database at startup. +//! +//! TRACES: UR-076 | DR-209 + +use std::collections::HashSet; +use std::sync::RwLock; + +use super::types::{MediaItem, SearchResult}; +use crate::utils::lock::RwLockSafe; + +/// Ids (normalised — see [`normalise_id`]) of items the user has hidden. +/// +/// Empty by default: nobody inherits somebody else's folder layout. +/// +/// TRACES: UR-076 | DR-209 +static EXCLUDED_IDS: RwLock> = RwLock::new(Vec::new()); + +/// Jellyfin writes the same GUID both dashed and undashed depending on the +/// endpoint, and ids arriving over IPC may carry stray whitespace. Comparing a +/// canonical form means a stored id keeps matching whichever spelling a query +/// happens to return. +fn normalise_id(id: &str) -> String { + id.trim().replace('-', "").to_ascii_lowercase() +} + +/// Replace the excluded set. Ids are normalised, de-duplicated and blanks +/// dropped, so a malformed value can never hide more than it names. +/// +/// TRACES: UR-076 | DR-209 +pub fn set_excluded_item_ids(ids: &[String]) { + let mut normalised: Vec = Vec::with_capacity(ids.len()); + for id in ids { + let id = normalise_id(id); + if id.is_empty() || normalised.contains(&id) { + continue; + } + normalised.push(id); + } + *EXCLUDED_IDS.write_safe() = normalised; +} + +/// The excluded set as currently applied, normalised. +/// +/// TRACES: UR-076 | DR-209 +pub fn excluded_item_ids() -> Vec { + EXCLUDED_IDS.read_safe().clone() +} + +/// Snapshot of the excluded set, taken once per list so a long listing does not +/// re-lock per item. +fn excluded_snapshot() -> HashSet { + EXCLUDED_IDS.read_safe().iter().cloned().collect() +} + +/// Whether `item` falls under one of `excluded`. +/// +/// The set is passed in rather than read from the global so the rule itself is a +/// pure function and can be tested without touching process state. +/// +/// An item matches on its own id or on any of the *links* it carries back to a +/// container: parent, album, library, series or season, and its artist entries. +/// That covers the shapes a hidden folder actually reaches a listing in — the +/// folder itself in a container listing, its albums (whose `parent_id` is the +/// folder), and their tracks (whose `album_id` is the album). It is deliberately +/// link-based rather than a full ancestry walk: the repository has no ancestor +/// index, and walking one would cost a round trip per row. +/// +/// TRACES: UR-076 | DR-209 +pub fn is_excluded_by(excluded: &HashSet, item: &MediaItem) -> bool { + if excluded.is_empty() { + return false; + } + + fn hidden(excluded: &HashSet, id: &str) -> bool { + excluded.contains(&normalise_id(id)) + } + + fn hidden_opt(excluded: &HashSet, id: &Option) -> bool { + match id { + Some(id) => hidden(excluded, id), + None => false, + } + } + + hidden(excluded, &item.id) + || hidden_opt(excluded, &item.parent_id) + || hidden_opt(excluded, &item.album_id) + || hidden_opt(excluded, &item.library_id) + || hidden_opt(excluded, &item.series_id) + || hidden_opt(excluded, &item.season_id) + || match &item.artist_items { + Some(artists) => artists.iter().any(|a| hidden(excluded, &a.id)), + None => false, + } +} + +/// Whether the currently-configured exclusions hide `item`. +/// +/// TRACES: UR-076 | DR-209 +pub fn is_excluded(item: &MediaItem) -> bool { + is_excluded_by(&excluded_snapshot(), item) +} + +/// Drop the user's hidden items from a repository result. +/// +/// Implemented as a trait so the hybrid repository's generic result helpers — +/// where the cache and server legs of every cache-first race converge — can +/// apply it to whatever they are carrying, instead of each query having to +/// remember to. +/// +/// TRACES: UR-076 | DR-209 +pub trait ExcludeHidden: Sized { + fn without_excluded(self) -> Self; +} + +impl ExcludeHidden for Vec { + fn without_excluded(mut self) -> Self { + let excluded = excluded_snapshot(); + if excluded.is_empty() { + return self; + } + self.retain(|item| !is_excluded_by(&excluded, item)); + self + } +} + +impl ExcludeHidden for SearchResult { + fn without_excluded(mut self) -> Self { + let before = self.items.len(); + self.items = self.items.without_excluded(); + // `total_record_count` is what the UI shows as "N results" and what + // paging is built against; leaving the server's count would advertise + // rows that were just removed. + let removed = before.saturating_sub(self.items.len()); + self.total_record_count = self.total_record_count.saturating_sub(removed); + self + } +} + +impl ExcludeHidden for MediaItem { + /// A single item fetched by id is never hidden. + /// + /// Exclusion hides things from *browsing*. An item asked for by id was + /// navigated to deliberately, or is being resolved by the player or a + /// download — answering "not found" there would break playback of anything + /// inside a hidden folder rather than merely tidying a listing. + fn without_excluded(self) -> Self { + self + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::repository::types::ArtistItem; + use crate::utils::lock::MutexSafe; + use std::sync::Mutex; + + /// Serialises the tests that write the process-global excluded set. Cargo + /// runs a crate's tests in one process, so without this two of them racing + /// would see each other's ids. + static EXCLUSION_TEST_LOCK: Mutex<()> = Mutex::new(()); + + fn item(id: &str) -> MediaItem { + MediaItem { + id: id.to_string(), + name: format!("item {id}"), + ..MediaItem::default() + } + } + + fn excluded(ids: &[&str]) -> HashSet { + ids.iter().map(|id| normalise_id(id)).collect() + } + + /// The default is empty: nobody inherits another user's folder layout, which + /// is exactly what the hardcoded "Podcasts" name filter did. + /// + /// TRACES: UR-076 | DR-209 | UT-203 + #[test] + fn test_no_exclusions_by_default_keeps_everything() { + let empty = HashSet::new(); + assert!(!is_excluded_by(&empty, &item("anything"))); + + let items = vec![item("a"), item("b")]; + assert_eq!(items.without_excluded().len(), 2); + } + + /// The folder itself, and anything linking back to it, is hidden. + /// + /// TRACES: UR-076 | DR-209 | UT-203 + #[test] + fn test_excludes_the_folder_and_what_points_at_it() { + let set = excluded(&["folder-1"]); + + assert!(is_excluded_by(&set, &item("folder-1")), "the folder itself"); + + let album = MediaItem { + parent_id: Some("folder-1".to_string()), + ..item("album-1") + }; + assert!(is_excluded_by(&set, &album), "an album inside the folder"); + + let track = MediaItem { + album_id: Some("folder-1".to_string()), + ..item("track-1") + }; + assert!(is_excluded_by(&set, &track), "a track of the folder"); + + let elsewhere = MediaItem { + parent_id: Some("folder-2".to_string()), + ..item("album-2") + }; + assert!(!is_excluded_by(&set, &elsewhere), "an unrelated album"); + } + + /// A whole library, a series/season and an artist are all excludable by the + /// same check — the setting is "hide this container", not "hide albums". + /// + /// TRACES: UR-076 | DR-209 | UT-203 + #[test] + fn test_excludes_via_every_container_link() { + let set = excluded(&["container"]); + + let by_library = MediaItem { + library_id: Some("container".to_string()), + ..item("x") + }; + assert!(is_excluded_by(&set, &by_library)); + + let by_series = MediaItem { + series_id: Some("container".to_string()), + ..item("x") + }; + assert!(is_excluded_by(&set, &by_series)); + + let by_season = MediaItem { + season_id: Some("container".to_string()), + ..item("x") + }; + assert!(is_excluded_by(&set, &by_season)); + + let by_artist = MediaItem { + artist_items: Some(vec![ + ArtistItem { + id: "other".to_string(), + name: "Other".to_string(), + }, + ArtistItem { + id: "container".to_string(), + name: "Hidden".to_string(), + }, + ]), + ..item("x") + }; + assert!(is_excluded_by(&set, &by_artist)); + } + + /// Ids are matched by identity, not spelling: Jellyfin serves the same GUID + /// dashed on one endpoint and undashed on another, and a stored id that + /// stopped matching would silently un-hide the folder. + /// + /// TRACES: UR-076 | DR-209 | UT-203 + #[test] + fn test_id_matching_ignores_dashes_case_and_padding() { + let set = excluded(&[" A1B2C3D4-0000-0000-0000-000000000000 "]); + assert!(is_excluded_by( + &set, + &item("a1b2c3d4-0000-0000-0000-000000000000") + )); + assert!(is_excluded_by( + &set, + &item("A1B2C3D4000000000000000000000000") + )); + assert!(!is_excluded_by(&set, &item("a1b2c3d4-0000-0000-0000-1"))); + } + + /// Filtering a `SearchResult` must also correct its count — the listing + /// header reads it, and a stale total advertises rows that are not there. + /// + /// TRACES: UR-076 | DR-209 | UT-203 + #[test] + fn test_search_result_count_follows_the_filter() { + let _guard = EXCLUSION_TEST_LOCK.lock_safe(); + set_excluded_item_ids(&["hidden".to_string()]); + + let result = SearchResult { + items: vec![item("keep"), item("hidden"), item("keep-2")], + total_record_count: 3, + } + .without_excluded(); + + set_excluded_item_ids(&[]); + + assert_eq!(result.items.len(), 2); + assert_eq!(result.total_record_count, 2); + assert!(result.items.iter().all(|i| i.id != "hidden")); + } + + /// A single item asked for by id is never withheld: exclusion hides things + /// from browsing, and refusing it here would break playback and downloads of + /// anything inside a hidden folder. + /// + /// TRACES: UR-076 | DR-209 | UT-203 + #[test] + fn test_direct_item_lookup_is_never_hidden() { + let _guard = EXCLUSION_TEST_LOCK.lock_safe(); + set_excluded_item_ids(&["hidden".to_string()]); + + let still_matches = is_excluded(&item("hidden")); + let survives = item("hidden").without_excluded(); + + set_excluded_item_ids(&[]); + + assert!(still_matches, "the predicate still matches the item"); + assert_eq!(survives.id, "hidden", "but a direct lookup keeps it"); + } + + /// The stored set is sanitised on the way in: blanks dropped, duplicates + /// collapsed, spellings normalised. + /// + /// TRACES: UR-076 | DR-209 | UT-203 + #[test] + fn test_set_excluded_item_ids_sanitises() { + let _guard = EXCLUSION_TEST_LOCK.lock_safe(); + set_excluded_item_ids(&[ + " ".to_string(), + "AB-CD".to_string(), + "abcd".to_string(), + "ef".to_string(), + ]); + let stored = excluded_item_ids(); + + set_excluded_item_ids(&[]); + let cleared = excluded_item_ids(); + + assert_eq!(stored, vec!["abcd".to_string(), "ef".to_string()]); + assert!(cleared.is_empty()); + } +} diff --git a/src-tauri/src/repository/hybrid.rs b/src-tauri/src/repository/hybrid.rs index 240aeab1..f7e27b6b 100644 --- a/src-tauri/src/repository/hybrid.rs +++ b/src-tauri/src/repository/hybrid.rs @@ -15,6 +15,7 @@ use async_trait::async_trait; use log::{debug, warn}; use tokio::time::{timeout, Duration}; +use super::exclusions::ExcludeHidden; use super::{types::*, MediaRepository, OfflineRepository, OnlineRepository}; /// Hybrid repository combining online and offline data sources @@ -151,6 +152,27 @@ impl HybridRepository { Ok(result.items) } + /// Immediate children of a container with the user's browsing exclusions + /// **not** applied. + /// + /// Exists for the exclusion picker in settings. Everything else in this + /// repository hides what the user has hidden, which would make the setting + /// one-way: a folder already excluded would vanish from the list of folders + /// to exclude and could never be un-hidden. Server-first so the picker sees + /// the real library, falling back to the cache when unreachable. + /// + /// TRACES: UR-076 | DR-209 + pub async fn get_items_unfiltered( + &self, + parent_id: &str, + options: Option, + ) -> Result { + match self.online.get_items(parent_id, options.clone()).await { + Ok(result) => Ok(result), + Err(e) => self.offline.get_items(parent_id, options).await.or(Err(e)), + } + } + /// Search only the local SQLite cache (downloaded content). /// /// Fast (100ms timeout) — used to render instant results before the server @@ -165,6 +187,7 @@ impl HybridRepository { let query = query.to_string(); self.cache_with_timeout(async move { offline.search(&query, options).await }) .await + .map(ExcludeHidden::without_excluded) } /// Favourites held locally, without touching the server. Backs the instant @@ -179,6 +202,7 @@ impl HybridRepository { let offline = Arc::clone(&self.offline); self.cache_with_timeout(async move { offline.get_favorites(scope, options).await }) .await + .map(ExcludeHidden::without_excluded) } /// Favourites straight from the server, persisted to the cache on the way @@ -199,7 +223,7 @@ impl HybridRepository { debug!("[HybridRepo] Failed to cache favourites: {:?}", e); } } - Ok(result) + Ok(result.without_excluded()) } /// Fetch a folder's items from the live server and persist them to the @@ -235,6 +259,10 @@ impl HybridRepository { parent_id: &str, options: Option, ) -> Result { + // Deliberately *not* filtered by the user's hidden folders: this surface + // manages what is on the device, and hiding a download would leave the + // user unable to delete a file they can still see the disk usage of. + // TRACES: UR-076 | DR-209 self.offline.get_downloaded_items(parent_id, options).await } @@ -258,7 +286,10 @@ impl HybridRepository { query: &str, options: Option, ) -> Result { - self.online.search(query, options).await + self.online + .search(query, options) + .await + .map(ExcludeHidden::without_excluded) } /// Merge cache and server search results into a single de-duplicated list. @@ -318,20 +349,30 @@ impl HybridRepository { /// 3. If cache is empty/stale → query server (fresh data) /// 4. If server fails → return cache even if empty (offline fallback) /// + /// Both legs are passed through [`ExcludeHidden`] before the "does the cache + /// have content?" question is asked. This is the single place the cache and + /// server results of a cache-first query converge, so applying the user's + /// browsing exclusions here covers every query built on it at once — and + /// filtering *before* the content check is what makes a cache page holding + /// nothing but hidden items fall through to the server instead of being + /// served as an empty listing. + /// /// @req: UR-002 - Access media when online or offline /// @req: DR-013 - Repository pattern for online/offline data access + /// + /// TRACES: UR-002, UR-076 | DR-013, DR-209 async fn parallel_race( &self, cache_future: F1, server_future: F2, ) -> Result where - T: MeaningfulContent + Clone + Send + 'static, + T: MeaningfulContent + ExcludeHidden + Clone + Send + 'static, F1: std::future::Future> + Send, F2: std::future::Future> + Send, { // Try cache first (100ms timeout already applied by callers) - let cache_result = cache_future.await; + let cache_result = cache_future.await.map(ExcludeHidden::without_excluded); if let Ok(data) = &cache_result { if data.has_content() { @@ -343,7 +384,7 @@ impl HybridRepository { // Cache miss — fall back to server debug!("[HybridRepo] Cache miss, querying server"); match server_future.await { - Ok(data) => Ok(data), + Ok(data) => Ok(data.without_excluded()), Err(e) => { // Server failed, try to return cache even if empty cache_result.or(Err(e)) @@ -364,7 +405,7 @@ impl HybridRepository { /// The callback runs only on a cache hit — on a miss the server result is /// already being fetched and cached by the normal path. /// - /// TRACES: UR-002, UR-025 | DR-155 + /// TRACES: UR-002, UR-025, UR-076 | DR-155, DR-209 async fn race_with_refresh( &self, cache_future: F1, @@ -372,12 +413,12 @@ impl HybridRepository { on_cache_hit: R, ) -> Result where - T: MeaningfulContent + Clone + Send + 'static, + T: MeaningfulContent + ExcludeHidden + Clone + Send + 'static, F1: std::future::Future> + Send, F2: std::future::Future> + Send, R: FnOnce(), { - let cache_result = cache_future.await; + let cache_result = cache_future.await.map(ExcludeHidden::without_excluded); if let Ok(data) = &cache_result { if data.has_content() { @@ -389,7 +430,7 @@ impl HybridRepository { debug!("[HybridRepo] Cache miss, querying server"); match server_future.await { - Ok(data) => Ok(data), + Ok(data) => Ok(data.without_excluded()), Err(e) => cache_result.or(Err(e)), } } @@ -475,10 +516,18 @@ impl MediaRepository for HybridRepository { let server_handle = tokio::spawn(async move { online.get_items(&parent_id_clone, options).await }); - // Check cache first (fast, 100ms timeout) + // Check cache first (fast, 100ms timeout). + // + // Exclusions are applied here rather than at each return below so the + // "has content" decisions further down are made about what the user will + // actually see. `get_items` is the one query that does not go through + // `parallel_race` — it interleaves the downloads-only gate and a + // background cache write — so it applies the filter itself. + // TRACES: UR-076 | DR-209 let cache_result = self .cache_with_timeout(async move { offline.get_items(&parent_id, opts_clone).await }) - .await; + .await + .map(ExcludeHidden::without_excluded); // Downloads-only gate: when the "Show all server media" toggle is off // (offline), an empty offline result is authoritative — the user asked @@ -555,7 +604,12 @@ impl MediaRepository for HybridRepository { } }); } - Ok(server_data) + // The cache keeps the server's full page (above) — an exclusion + // is a view preference and can be undone, so hiding items from + // the *cache* would make un-hiding them require a re-crawl. Only + // what is handed back is filtered. + // TRACES: UR-076 | DR-209 + Ok(server_data.without_excluded()) } Ok(Err(e)) => cache_result.or(Err(e)), Err(join_err) => cache_result.or(Err(RepoError::Network { @@ -667,7 +721,10 @@ impl MediaRepository for HybridRepository { limit: Option, ) -> Result, RepoError> { // Next up is dynamic, always fetch from server - self.online.get_next_up_episodes(series_id, limit).await + self.online + .get_next_up_episodes(series_id, limit) + .await + .map(ExcludeHidden::without_excluded) } async fn get_recently_played_audio( @@ -831,12 +888,18 @@ impl MediaRepository for HybridRepository { async fn get_live_tv_channels(&self) -> Result, RepoError> { // Live TV requires server communication - delegate to online repository - self.online.get_live_tv_channels().await + self.online + .get_live_tv_channels() + .await + .map(ExcludeHidden::without_excluded) } async fn get_channels(&self) -> Result { // Plugin channels require server communication - delegate to online repository - self.online.get_channels().await + self.online + .get_channels() + .await + .map(ExcludeHidden::without_excluded) } async fn open_live_stream(&self, item_id: &str) -> Result { diff --git a/src-tauri/src/repository/mod.rs b/src-tauri/src/repository/mod.rs index c9577c30..91124983 100644 --- a/src-tauri/src/repository/mod.rs +++ b/src-tauri/src/repository/mod.rs @@ -1,4 +1,6 @@ pub mod device_profile; +/// User-chosen browsing exclusions (UR-076 / DR-209). +pub mod exclusions; pub mod hybrid; pub mod offline; pub mod online; diff --git a/src-tauri/src/settings.rs b/src-tauri/src/settings.rs index 3d087937..05bb7ab1 100644 --- a/src-tauri/src/settings.rs +++ b/src-tauri/src/settings.rs @@ -320,6 +320,52 @@ impl VideoSettings { } } +/// Library browsing preferences. +/// +/// Currently a single list: the folders (or whole libraries) the user has asked +/// to keep out of browsing. It is a *list of ids*, never names — names are +/// unstable, locale-dependent and non-unique, and the hardcoded name filter this +/// setting replaced broke on exactly that. What the ids then hide is decided in +/// `repository::exclusions`; this struct is only how the choice is carried and +/// persisted. +/// +/// The default is an empty list: nobody inherits another user's folder layout. +/// +/// TRACES: UR-076 | DR-209 +#[derive(specta::Type, Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct LibrarySettings { + /// Stable item ids of the folders/libraries hidden from browsing. + /// + /// `#[serde(default)]` so settings JSON persisted before this field existed + /// loads as the previous behaviour (nothing hidden). + #[serde(default)] + pub excluded_item_ids: Vec, +} + +impl LibrarySettings { + /// Drop blanks and duplicates from the id list. + /// + /// Applied on the way in from IPC and on the way out of the database, so a + /// hand-edited or half-written value cannot make the list grow without bound + /// or carry an empty id (which would match nothing but still be shown as a + /// selection in the picker). + /// + /// TRACES: UR-076 | DR-209 + pub fn sanitised(mut self) -> Self { + let mut seen: Vec = Vec::with_capacity(self.excluded_item_ids.len()); + for id in self.excluded_item_ids.drain(..) { + let id = id.trim().to_string(); + if id.is_empty() || seen.contains(&id) { + continue; + } + seen.push(id); + } + self.excluded_item_ids = seen; + self + } +} + /// Serialise `AudioSettings` into the JSON payload handed to the Android player /// over JNI. /// diff --git a/src/lib/components/library/GenericMediaListPage.svelte b/src/lib/components/library/GenericMediaListPage.svelte index 624b585b..80c1e12d 100644 --- a/src/lib/components/library/GenericMediaListPage.svelte +++ b/src/lib/components/library/GenericMediaListPage.svelte @@ -19,7 +19,6 @@ import LibraryGrid from "./LibraryGrid.svelte"; import TrackList from "./TrackList.svelte"; import AlphabetScrollBar from "./AlphabetScrollBar.svelte"; - import { excludePodcasts } from "$lib/utils/podcastFilter"; import { createLogger } from "$lib/utils/logger"; const log = createLogger("GenericMediaListPage"); @@ -87,7 +86,7 @@ unlistenSearch = await listen("search-event", (event) => { const { requestId, result } = event.payload; if (requestId !== searchRequestId) return; - items = excludePodcasts(result.items); + items = result.items; }); } @@ -121,8 +120,9 @@ if (items.length === 0) loading = true; const repo = auth.getRepository(); - // Use backend search if search query is provided, otherwise use getItems with sort - // HACK: excludePodcasts drops the "Podcasts" folder stored in the music library. + // Use backend search if search query is provided, otherwise use getItems + // with sort. Neither result is filtered here: folders the user chose to + // hide are dropped by the repository layer. TRACES: UR-076 | DR-209 if (debouncedSearchQuery.trim()) { // Phase 1: instant cache-only (downloaded) results. The merged // cache+server union arrives later via the `search-event` listener, @@ -139,7 +139,7 @@ ); // Only apply if this is still the active query. if (requestId === searchRequestId) { - items = excludePodcasts(result.items); + items = result.items; } } else { // Leaving search — invalidate any in-flight server results. @@ -154,7 +154,7 @@ // resolves to online vs offline. TRACES: UR-067 | DR-116 favoritesOnly: favoritesOnly ? true : undefined, }); - items = excludePodcasts(result.items); + items = result.items; } } catch (e) { log.error(`Failed to load ${config.itemType}:`, e); diff --git a/src/lib/stores/music.ts b/src/lib/stores/music.ts index 6e2c2fea..d077e817 100644 --- a/src/lib/stores/music.ts +++ b/src/lib/stores/music.ts @@ -4,7 +4,6 @@ import { writable, derived } from "svelte/store"; import type { MediaItem, Genre } from "$lib/api/types"; import { auth } from "./auth"; -import { excludePodcasts } from "$lib/utils/podcastFilter"; import { selectDiverseGenres, sampleAcross } from "$lib/utils/genreDiversity"; import { buildHeroMix } from "$lib/utils/heroMix"; import { createLogger } from "$lib/utils/logger"; @@ -100,23 +99,20 @@ function createMusicStore() { .catch(() => [] as MediaItem[]), ]); - // HACK: drop the "Podcasts" folder that lives inside the music library. - const recentlyPlayedAlbums = excludePodcasts(recentlyPlayed); - const newlyAddedAlbums = excludePodcasts(newlyAdded.items); - const playlistItems = excludePodcasts(playlistsResult.items); - const rediscoverAlbums = excludePodcasts(rediscover); - const surpriseAlbums = excludePodcasts(surprise); + // Nothing is filtered here: folders the user chose to hide are already + // gone, dropped by the repository layer that answered these queries. + // TRACES: UR-076 | DR-209 // Mix the hero: fresh-in-your-ears first, then "remember this?", then // random albums from across the library. - const heroItems = buildHeroMix([recentlyPlayedAlbums, rediscoverAlbums, surpriseAlbums], hasArt); + const heroItems = buildHeroMix([recentlyPlayed, rediscover, surprise], hasArt); update(s => ({ ...s, - recentlyPlayed: recentlyPlayedAlbums, - newlyAdded: newlyAddedAlbums, - playlists: playlistItems, - rediscover: rediscoverAlbums, + recentlyPlayed, + newlyAdded: newlyAdded.items, + playlists: playlistsResult.items, + rediscover, heroItems, isLoading: false, })); @@ -143,8 +139,7 @@ function createMusicStore() { recursive: true, limit: SECTION_LIMIT, }); - // HACK: drop the "Podcasts" folder that lives in the music library. - return { id: genre.id, name: genre.name, items: excludePodcasts(result.items) }; + return { id: genre.id, name: genre.name, items: result.items }; } catch (e) { log.warn(`Failed to load genre row "${genre.name}":`, e); return { id: genre.id, name: genre.name, items: [] }; diff --git a/src/lib/utils/podcastFilter.ts b/src/lib/utils/podcastFilter.ts deleted file mode 100644 index 9fb7ff2a..00000000 --- a/src/lib/utils/podcastFilter.ts +++ /dev/null @@ -1,30 +0,0 @@ -// HACK: hide "Podcasts" from the music library. -// -// The user stores podcasts inside the music library under a folder/album named -// "Podcasts", so they leak into album/artist/track/playlist queries. Jellyfin's -// item queries here don't give us a clean server-side exclusion for that folder, -// so we filter client-side by name. This is intentionally a blunt instrument: -// anything whose own name, album, or (album) artist is literally "Podcasts" is -// dropped. If the folder is ever renamed, update PODCAST_FOLDER_NAME. -import type { MediaItem } from "$lib/api/types"; - -const PODCAST_FOLDER_NAME = "podcasts"; - -function isPodcastName(value: string | null | undefined): boolean { - return value?.trim().toLowerCase() === PODCAST_FOLDER_NAME; -} - -/** True when an item belongs to the "Podcasts" folder/album and should be hidden. */ -export function isPodcastItem(item: MediaItem): boolean { - return ( - isPodcastName(item.name) || - isPodcastName(item.albumName) || - isPodcastName(item.albumArtist) || - (item.artists?.some(isPodcastName) ?? false) - ); -} - -/** Remove "Podcasts" entries from a list of music items. */ -export function excludePodcasts(items: MediaItem[]): MediaItem[] { - return items.filter((item) => !isPodcastItem(item)); -} diff --git a/src/lib/utils/validation.test.ts b/src/lib/utils/validation.test.ts deleted file mode 100644 index cd44bdfa..00000000 --- a/src/lib/utils/validation.test.ts +++ /dev/null @@ -1,118 +0,0 @@ -/** - * Input validation utility tests - * - * TRACES: UR-009, UR-025 | DR-015 - */ - -import { describe, it, expect } from "vitest"; -import { - validateItemId, - validateImageType, - validateMediaSourceId, - validateNumericParam, - validateQueryParamValue, -} from "./validation"; - -describe("validateItemId", () => { - it("should accept valid item IDs", () => { - expect(() => validateItemId("123abc")).not.toThrow(); - expect(() => validateItemId("abc-123_def")).not.toThrow(); - expect(() => validateItemId("12345")).not.toThrow(); - }); - - it("should reject empty or non-string IDs", () => { - expect(() => validateItemId("")).toThrow("must be a non-empty string"); - expect(() => validateItemId(null as any)).toThrow("must be a non-empty string"); - expect(() => validateItemId(undefined as any)).toThrow("must be a non-empty string"); - }); - - it("should reject IDs exceeding max length", () => { - expect(() => validateItemId("a".repeat(51))).toThrow("exceeds maximum length"); - }); - - it("should reject IDs with invalid characters", () => { - expect(() => validateItemId("abc/def")).toThrow("contains invalid characters"); - expect(() => validateItemId("abc..def")).toThrow("contains invalid characters"); - expect(() => validateItemId("abc def")).toThrow("contains invalid characters"); - }); -}); - -describe("validateImageType", () => { - it("should accept valid image types", () => { - expect(() => validateImageType("Primary")).not.toThrow(); - expect(() => validateImageType("Backdrop")).not.toThrow(); - expect(() => validateImageType("Banner")).not.toThrow(); - expect(() => validateImageType("Logo")).not.toThrow(); - }); - - it("should reject invalid image types", () => { - expect(() => validateImageType("InvalidType")).toThrow("not a valid image type"); - expect(() => validateImageType("..")).toThrow("not a valid image type"); - expect(() => validateImageType("Primary/Avatar")).toThrow("not a valid image type"); - }); - - it("should reject empty or non-string types", () => { - expect(() => validateImageType("")).toThrow("must be a non-empty string"); - }); -}); - -describe("validateMediaSourceId", () => { - it("should accept valid media source IDs", () => { - expect(() => validateMediaSourceId("source-123")).not.toThrow(); - expect(() => validateMediaSourceId("video_stream_1")).not.toThrow(); - }); - - it("should reject IDs with invalid characters", () => { - expect(() => validateMediaSourceId("source/path")).toThrow("contains invalid characters"); - expect(() => validateMediaSourceId("source..path")).toThrow("contains invalid characters"); - }); - - it("should reject IDs exceeding max length", () => { - expect(() => validateMediaSourceId("a".repeat(51))).toThrow("exceeds maximum length"); - }); -}); - -describe("validateNumericParam", () => { - it("should accept valid numbers", () => { - expect(validateNumericParam(100)).toBe(100); - expect(validateNumericParam(0)).toBe(0); - expect(validateNumericParam(9999)).toBe(9999); - }); - - it("should reject non-integers", () => { - expect(() => validateNumericParam(10.5)).toThrow("must be an integer"); - expect(() => validateNumericParam("100")).toThrow("must be an integer"); - }); - - it("should respect min and max bounds", () => { - expect(() => validateNumericParam(-1, 0, 100)).toThrow("must be between 0 and 100"); - expect(() => validateNumericParam(101, 0, 100)).toThrow("must be between 0 and 100"); - }); - - it("should allow custom bounds", () => { - expect(validateNumericParam(50, 10, 100)).toBe(50); - expect(() => validateNumericParam(5, 10, 100)).toThrow("must be between 10 and 100"); - }); -}); - -describe("validateQueryParamValue", () => { - it("should accept valid query param values", () => { - expect(() => validateQueryParamValue("abc123")).not.toThrow(); - expect(() => validateQueryParamValue("value-with-dash")).not.toThrow(); - expect(() => validateQueryParamValue("value_with_underscore")).not.toThrow(); - }); - - it("should reject values with invalid characters", () => { - expect(() => validateQueryParamValue("value with spaces")).toThrow("contains invalid characters"); - expect(() => validateQueryParamValue("value/path")).toThrow("contains invalid characters"); - expect(() => validateQueryParamValue("value?query")).toThrow("contains invalid characters"); - }); - - it("should reject values exceeding max length", () => { - expect(() => validateQueryParamValue("a".repeat(101))).toThrow("exceeds maximum length"); - }); - - it("should respect custom max length", () => { - expect(() => validateQueryParamValue("a".repeat(50), 40)).toThrow("exceeds maximum length"); - }); -}); diff --git a/src/routes/settings/+page.svelte b/src/routes/settings/+page.svelte index e0e49d75..d43d5d1d 100644 --- a/src/routes/settings/+page.svelte +++ b/src/routes/settings/+page.svelte @@ -1,4 +1,4 @@ - +