feat(library): exclude chosen folders from music browsing
Replaces a hardcoded filter that dropped anything named "Podcasts" from music results — one user's library layout compiled into the shipped product, keyed on an English literal, applied only at the six call sites someone had remembered. Exclusion is now a user setting stored in Rust and applied at the repository layer's convergence points, so scope is decided once and is the same on every screen. It matches on folder id rather than name: a title is not what an item is, which is why an album legitimately called "Podcasts" used to vanish. Deliberately not filtered: get_item (an id asked for by name was navigated to on purpose, and refusing it would break playback of anything inside a hidden folder), get_downloaded_items (hiding a download would leave the user unable to delete a file whose disk usage they can still see), and the offline cache (an exclusion is a view preference and must be reversible without a re-crawl). Also removes src/lib/utils/validation.ts — six exported validators with no caller outside their own test file, which made the module read as covered input validation while guarding nothing. TRACES: UR-076 | DR-209 | UT-203
This commit is contained in:
@@ -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<LibrarySettings, String> {
|
||||
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<LibrarySettings, String> {
|
||||
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<Vec<ExclusionCandidate>, 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<ExclusionCandidate> = 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::<DatabaseWrapper>() 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<String> = 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()]
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -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
|
||||
|
||||
@@ -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<tauri::Wry> {
|
||||
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") {
|
||||
|
||||
@@ -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<Vec<String>> = 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<String> = 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<String> {
|
||||
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<String> {
|
||||
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<String>, item: &MediaItem) -> bool {
|
||||
if excluded.is_empty() {
|
||||
return false;
|
||||
}
|
||||
|
||||
fn hidden(excluded: &HashSet<String>, id: &str) -> bool {
|
||||
excluded.contains(&normalise_id(id))
|
||||
}
|
||||
|
||||
fn hidden_opt(excluded: &HashSet<String>, id: &Option<String>) -> 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<MediaItem> {
|
||||
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<String> {
|
||||
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());
|
||||
}
|
||||
}
|
||||
@@ -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<GetItemsOptions>,
|
||||
) -> Result<SearchResult, RepoError> {
|
||||
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<GetItemsOptions>,
|
||||
) -> Result<SearchResult, RepoError> {
|
||||
// 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<SearchOptions>,
|
||||
) -> Result<SearchResult, RepoError> {
|
||||
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<T, F1, F2>(
|
||||
&self,
|
||||
cache_future: F1,
|
||||
server_future: F2,
|
||||
) -> Result<T, RepoError>
|
||||
where
|
||||
T: MeaningfulContent + Clone + Send + 'static,
|
||||
T: MeaningfulContent + ExcludeHidden + Clone + Send + 'static,
|
||||
F1: std::future::Future<Output = Result<T, RepoError>> + Send,
|
||||
F2: std::future::Future<Output = Result<T, RepoError>> + 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<T, F1, F2, R>(
|
||||
&self,
|
||||
cache_future: F1,
|
||||
@@ -372,12 +413,12 @@ impl HybridRepository {
|
||||
on_cache_hit: R,
|
||||
) -> Result<T, RepoError>
|
||||
where
|
||||
T: MeaningfulContent + Clone + Send + 'static,
|
||||
T: MeaningfulContent + ExcludeHidden + Clone + Send + 'static,
|
||||
F1: std::future::Future<Output = Result<T, RepoError>> + Send,
|
||||
F2: std::future::Future<Output = Result<T, RepoError>> + 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<usize>,
|
||||
) -> Result<Vec<MediaItem>, 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<Vec<MediaItem>, 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<SearchResult, RepoError> {
|
||||
// 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<LiveStreamInfo, RepoError> {
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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<String>,
|
||||
}
|
||||
|
||||
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<String> = 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.
|
||||
///
|
||||
|
||||
Reference in New Issue
Block a user