From d9969ed76fac5a5108cb94880c9a8eb00b2c845b Mon Sep 17 00:00:00 2001 From: cucadmuh <49571317+cucadmuh@users.noreply.github.com> Date: Sun, 21 Jun 2026 20:55:35 +0300 Subject: [PATCH] fix(library): Navidrome artist letter buckets + safer index open/swap (#1144) (#1145) --- CHANGELOG.md | 13 + .../migrations/014_artist_name_sort.sql | 6 + .../psysonic-library/src/advanced_search.rs | 31 +- .../psysonic-library/src/artist_sort.rs | 63 +++ .../crates/psysonic-library/src/commands.rs | 13 +- src-tauri/crates/psysonic-library/src/dto.rs | 5 + src-tauri/crates/psysonic-library/src/lib.rs | 1 + .../psysonic-library/src/live_search.rs | 1 + .../psysonic-library/src/repos/artist.rs | 198 +++++++ .../crates/psysonic-library/src/repos/mod.rs | 2 + .../psysonic-library/src/repos/sync_state.rs | 38 ++ .../crates/psysonic-library/src/store.rs | 535 ++++++++++++++++-- .../psysonic-library/src/sync/artist_index.rs | 32 ++ .../crates/psysonic-library/src/sync/delta.rs | 22 +- .../psysonic-library/src/sync/initial.rs | 21 +- .../crates/psysonic-library/src/sync/mod.rs | 10 + src/api/library.ts | 2 + src/api/subsonicTypes.ts | 2 + src/config/settingsCredits.ts | 1 + src/hooks/useArtistsFiltering.ts | 13 +- src/hooks/useLibraryIgnoredArticles.ts | 38 ++ src/pages/Artists.tsx | 5 +- src/pages/Composers.tsx | 18 +- .../componentHelpers/artistsHelpers.test.ts | 71 ++- src/utils/componentHelpers/artistsHelpers.ts | 53 +- src/utils/library/advancedSearchLocal.ts | 9 +- 26 files changed, 1088 insertions(+), 115 deletions(-) create mode 100644 src-tauri/crates/psysonic-library/migrations/014_artist_name_sort.sql create mode 100644 src-tauri/crates/psysonic-library/src/artist_sort.rs create mode 100644 src-tauri/crates/psysonic-library/src/repos/artist.rs create mode 100644 src-tauri/crates/psysonic-library/src/sync/artist_index.rs create mode 100644 src/hooks/useLibraryIgnoredArticles.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 5dbbd4dc..d0b708f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -160,6 +160,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * With rows selected, **Play all** / **Add all to queue** become **Play selected** / **Add selected to queue** and act on the checked tracks only. * Bulk add now snapshots every checked row when the picker opens so all selected tracks land in the playlist, not just the last one. +### Artists letter index — Navidrome ignored articles + +**By [@cucadmuh](https://github.com/cucadmuh), PR [#1145](https://github.com/Psychotoxical/psysonic/pull/1145)**, closes [#1144](https://github.com/Psychotoxical/psysonic/issues/1144) + +* On the **Artists** page (and **Composers**), the A–Z filter now groups names like Navidrome: leading articles such as **The** are skipped before picking the letter — **The Beatles** lands under **B**, not **T**. The bucket follows the server's own `ignoredArticles` list when the local index knows it. +* The local library index stores `name_sort` and the server's `ignoredArticles` from `getArtists`, sorts browse SQL by the sort key (now indexed), and repairs stale keys once on upgrade. + +### Library index — safer open, swap and recovery + +**By [@cucadmuh](https://github.com/cucadmuh), PR [#1145](https://github.com/Psychotoxical/psysonic/pull/1145)** + +* The local library database now opens, swaps and restores through one pipeline, so a swapped or restored file always picks up pending migrations and one-time repairs instead of serving a stale schema. +* A panic or a poisoned lock in one query no longer wedges the whole library index — connections recover and report the error instead, and the new sort-key migration applies idempotently so a half-applied upgrade self-heals on the next launch. ## [1.48.1] - 2026-06-15 diff --git a/src-tauri/crates/psysonic-library/migrations/014_artist_name_sort.sql b/src-tauri/crates/psysonic-library/migrations/014_artist_name_sort.sql new file mode 100644 index 00000000..d2599acc --- /dev/null +++ b/src-tauri/crates/psysonic-library/migrations/014_artist_name_sort.sql @@ -0,0 +1,6 @@ +-- Artist browse sort key (Navidrome OrderArtistName parity) + server ignoredArticles watermark. +-- Applied idempotently from store.rs `apply_migration_14` (per-column guard) so a +-- partial apply recovers; keep this file in sync as the canonical DDL. +ALTER TABLE artist ADD COLUMN name_sort TEXT; +ALTER TABLE sync_state ADD COLUMN ignored_articles TEXT; +CREATE INDEX IF NOT EXISTS idx_artist_name_sort ON artist(server_id, name_sort); diff --git a/src-tauri/crates/psysonic-library/src/advanced_search.rs b/src-tauri/crates/psysonic-library/src/advanced_search.rs index 9edb54b4..d430623e 100644 --- a/src-tauri/crates/psysonic-library/src/advanced_search.rs +++ b/src-tauri/crates/psysonic-library/src/advanced_search.rs @@ -39,7 +39,7 @@ const ALBUM_COLUMNS: &str = "a.server_id, a.id, a.name, a.artist, a.artist_id, \ WHERE t.server_id = a.server_id AND t.album_id = a.id AND t.deleted = 0)), \ a.genre, a.cover_art_id, a.starred_at, a.synced_at, a.raw_json"; -const ARTIST_COLUMNS: &str = "ar.server_id, ar.id, ar.name, ar.album_count, \ +const ARTIST_COLUMNS: &str = "ar.server_id, ar.id, ar.name, ar.name_sort, ar.album_count, \ ar.synced_at, ar.raw_json"; /// Flat track projection used when browsing albums in advanced search. @@ -527,7 +527,9 @@ fn build_artist_from_table( } } let order = order_clause(&req.sort, EntityKind::Artist) - .unwrap_or_else(|| "ORDER BY ar.name COLLATE NOCASE ASC, ar.id ASC".to_string()); + .unwrap_or_else(|| { + "ORDER BY COALESCE(ar.name_sort, ar.name) COLLATE NOCASE ASC, ar.id ASC".to_string() + }); query_rows( store, ARTIST_COLUMNS, @@ -794,10 +796,16 @@ fn build_artist_from_fts( if !seen.insert(artist_id.clone()) { continue; } + let name = artist.unwrap_or_default(); + let name_sort = crate::artist_sort::sort_key_for_display_name( + &name, + crate::artist_sort::DEFAULT_IGNORED_ARTICLES, + ); deduped.push(LibraryArtistDto { server_id, id: artist_id, - name: artist.unwrap_or_default(), + name, + name_sort: Some(name_sort), album_count: None, synced_at, raw_json: Value::Null, @@ -1179,13 +1187,14 @@ fn map_album(r: &rusqlite::Row<'_>) -> rusqlite::Result { } fn map_artist(r: &rusqlite::Row<'_>) -> rusqlite::Result { - let raw: Option = r.get(5)?; + let raw: Option = r.get(6)?; Ok(LibraryArtistDto { server_id: r.get(0)?, id: r.get(1)?, name: r.get(2)?, - album_count: r.get(3)?, - synced_at: r.get(4)?, + name_sort: r.get(3)?, + album_count: r.get(4)?, + synced_at: r.get(5)?, raw_json: parse_raw_json(raw), }) } @@ -1211,10 +1220,16 @@ fn map_album_from_tracks(r: &rusqlite::Row<'_>) -> rusqlite::Result) -> rusqlite::Result { + let name: String = r.get(2)?; + let name_sort = crate::artist_sort::sort_key_for_display_name( + &name, + crate::artist_sort::DEFAULT_IGNORED_ARTICLES, + ); Ok(LibraryArtistDto { server_id: r.get(0)?, id: r.get(1)?, - name: r.get(2)?, + name, + name_sort: Some(name_sort), album_count: Some(r.get(3)?), synced_at: r.get(4)?, raw_json: Value::Null, @@ -1289,7 +1304,7 @@ fn sort_column(field: &str, entity: EntityKind) -> Option<&'static str> { ("name", EntityKind::Album) => Some("a.name COLLATE NOCASE"), ("year", EntityKind::Album) => Some("a.year"), ("artist", EntityKind::Album) => Some("a.artist COLLATE NOCASE"), - ("name", EntityKind::Artist) => Some("ar.name COLLATE NOCASE"), + ("name", EntityKind::Artist) => Some("COALESCE(ar.name_sort, ar.name) COLLATE NOCASE"), // SQLite built-in: ORDER BY RANDOM() LIMIT N — fast pseudo-random sample, // no index scan needed beyond the row-id range. Direction is ignored. ("random", _) => Some("RANDOM()"), diff --git a/src-tauri/crates/psysonic-library/src/artist_sort.rs b/src-tauri/crates/psysonic-library/src/artist_sort.rs new file mode 100644 index 00000000..df39eab1 --- /dev/null +++ b/src-tauri/crates/psysonic-library/src/artist_sort.rs @@ -0,0 +1,63 @@ +//! Display-name → sort/bucket key (Navidrome `SanitizeFieldForSortingNoArticle` subset). + +/// Navidrome default (`utils/str/sanitize_strings_test.go`). +pub const DEFAULT_IGNORED_ARTICLES: &str = "The El La Los Las Le Les Os As O A"; + +/// Strip leading articles from a display name (case-insensitive article match). +pub fn strip_leading_articles(name: &str, ignored_articles: &str) -> String { + let trimmed = name.trim(); + for article in ignored_articles.split(' ').filter(|s| !s.is_empty()) { + let prefix = format!("{} ", article); + if trimmed.len() >= prefix.len() + && trimmed[..prefix.len()].eq_ignore_ascii_case(&prefix) + { + return trimmed[prefix.len()..].trim_start().to_string(); + } + } + trimmed.to_string() +} + +/// Lowercase sort key used for SQL `ORDER BY` and UI letter buckets. +pub fn sort_key_for_display_name(name: &str, ignored_articles: &str) -> String { + strip_leading_articles(name, ignored_articles).to_lowercase() +} + +pub fn ignored_articles_or_default(ignored_articles: Option<&str>) -> &str { + match ignored_articles.map(str::trim).filter(|s| !s.is_empty()) { + Some(s) => s, + None => DEFAULT_IGNORED_ARTICLES, + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn strips_the_from_beatles() { + let key = sort_key_for_display_name("The Beatles", DEFAULT_IGNORED_ARTICLES); + assert_eq!(key, "beatles"); + } + + #[test] + fn strips_the_from_kinks() { + let key = sort_key_for_display_name("The Kinks", DEFAULT_IGNORED_ARTICLES); + assert_eq!(key, "kinks"); + } + + #[test] + fn leaves_non_article_names() { + assert_eq!( + sort_key_for_display_name("Adele", DEFAULT_IGNORED_ARTICLES), + "adele" + ); + } + + #[test] + fn custom_ignored_articles() { + assert_eq!( + sort_key_for_display_name("The Beatles", "The"), + "beatles" + ); + } +} diff --git a/src-tauri/crates/psysonic-library/src/commands.rs b/src-tauri/crates/psysonic-library/src/commands.rs index 07e77ab2..cc62d656 100644 --- a/src-tauri/crates/psysonic-library/src/commands.rs +++ b/src-tauri/crates/psysonic-library/src/commands.rs @@ -180,8 +180,8 @@ pub async fn library_get_status( conn.query_row( "SELECT sync_phase, capability_flags, library_tier, last_full_sync_at, \ last_delta_sync_at, next_poll_at, server_last_scan_iso, \ - indexes_last_modified_ms, artists_last_modified_ms, local_track_count, \ - server_track_count, last_error \ + indexes_last_modified_ms, artists_last_modified_ms, ignored_articles, \ + local_track_count, server_track_count, last_error \ FROM sync_state WHERE server_id = ?1 AND library_scope = ?2", params![server_id, scope], |r| { @@ -195,9 +195,10 @@ pub async fn library_get_status( server_last_scan_iso: r.get(6)?, indexes_last_modified_ms: r.get(7)?, artists_last_modified_ms: r.get(8)?, - local_track_count: r.get(9)?, - server_track_count: r.get(10)?, - last_error: r.get(11)?, + ignored_articles: r.get(9)?, + local_track_count: r.get(10)?, + server_track_count: r.get(11)?, + last_error: r.get(12)?, }) }, ) @@ -246,6 +247,7 @@ pub async fn library_get_status( server_last_scan_iso: row.server_last_scan_iso, indexes_last_modified_ms: row.indexes_last_modified_ms, artists_last_modified_ms: row.artists_last_modified_ms, + ignored_articles: row.ignored_articles, local_track_count, server_track_count: row.server_track_count, last_error: row.last_error, @@ -622,6 +624,7 @@ struct SyncStateRow { server_last_scan_iso: Option, indexes_last_modified_ms: Option, artists_last_modified_ms: Option, + ignored_articles: Option, local_track_count: Option, server_track_count: Option, last_error: Option, diff --git a/src-tauri/crates/psysonic-library/src/dto.rs b/src-tauri/crates/psysonic-library/src/dto.rs index ad304d2b..89170a30 100644 --- a/src-tauri/crates/psysonic-library/src/dto.rs +++ b/src-tauri/crates/psysonic-library/src/dto.rs @@ -28,6 +28,8 @@ pub struct SyncStateDto { pub server_last_scan_iso: Option, pub indexes_last_modified_ms: Option, pub artists_last_modified_ms: Option, + /// Space-separated leading articles from the server's `getArtists` response. + pub ignored_articles: Option, pub local_track_count: Option, pub server_track_count: Option, pub last_error: Option, @@ -476,6 +478,8 @@ pub struct LibraryArtistDto { pub server_id: String, pub id: String, pub name: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub name_sort: Option, pub album_count: Option, pub synced_at: i64, pub raw_json: Value, @@ -861,6 +865,7 @@ mod tests { server_last_scan_iso: None, indexes_last_modified_ms: None, artists_last_modified_ms: None, + ignored_articles: None, local_track_count: None, server_track_count: None, last_error: None, diff --git a/src-tauri/crates/psysonic-library/src/lib.rs b/src-tauri/crates/psysonic-library/src/lib.rs index 228d9a19..4d878a3c 100644 --- a/src-tauri/crates/psysonic-library/src/lib.rs +++ b/src-tauri/crates/psysonic-library/src/lib.rs @@ -17,6 +17,7 @@ pub mod analysis_backfill_policy; pub mod library_readiness; pub mod artist_artwork; pub mod artist_lossless_browse; +pub mod artist_sort; pub mod cover_backfill; pub mod cover_resolve; pub mod canonical; diff --git a/src-tauri/crates/psysonic-library/src/live_search.rs b/src-tauri/crates/psysonic-library/src/live_search.rs index b7364eed..0cb1a692 100644 --- a/src-tauri/crates/psysonic-library/src/live_search.rs +++ b/src-tauri/crates/psysonic-library/src/live_search.rs @@ -193,6 +193,7 @@ fn query_artists( server_id: r.get(0)?, id: r.get(1)?, name: r.get::<_, Option>(2)?.unwrap_or_default(), + name_sort: None, album_count: None, synced_at: r.get(3)?, raw_json: serde_json::Value::Null, diff --git a/src-tauri/crates/psysonic-library/src/repos/artist.rs b/src-tauri/crates/psysonic-library/src/repos/artist.rs new file mode 100644 index 00000000..80794876 --- /dev/null +++ b/src-tauri/crates/psysonic-library/src/repos/artist.rs @@ -0,0 +1,198 @@ +//! `artist` table — browse index rows from `getArtists` and track-derived backfill. + +use rusqlite::{params, Transaction}; + +use crate::artist_sort::{ignored_articles_or_default, sort_key_for_display_name}; +use crate::store::LibraryStore; +use psysonic_integration::subsonic::ArtistIndex; + +pub struct ArtistRepository<'a> { + store: &'a LibraryStore, +} + +impl<'a> ArtistRepository<'a> { + pub fn new(store: &'a LibraryStore) -> Self { + Self { store } + } + + /// Upsert artists from a Subsonic `getArtists` / `getIndexes` body. + pub fn upsert_index( + &self, + server_id: &str, + index: &ArtistIndex, + synced_at: i64, + ) -> Result { + let ignored = ignored_articles_or_default(index.ignored_articles.as_deref()); + let mut count = 0u32; + self.store.with_conn_mut("artist.upsert_index", |conn| { + let tx = conn.transaction()?; + for bucket in &index.index { + for artist in &bucket.artist { + let name_sort = sort_key_for_display_name(&artist.name, ignored); + upsert_artist_row( + &tx, + server_id, + &artist.id, + &artist.name, + &name_sort, + artist.album_count, + synced_at, + )?; + count += 1; + } + } + tx.commit()?; + Ok(()) + })?; + Ok(count) + } + + /// Materialize missing `artist` rows from synced tracks (pre-pass backfill). + pub fn backfill_from_tracks( + &self, + server_id: &str, + ignored_articles: &str, + synced_at: i64, + ) -> Result { + let rows: Vec<(String, String)> = self + .store + .with_read_conn(|conn| { + let mut stmt = conn.prepare( + "SELECT artist_id, MAX(artist) \ + FROM track \ + WHERE server_id = ?1 AND deleted = 0 \ + AND artist_id IS NOT NULL AND artist_id != '' \ + AND artist IS NOT NULL AND artist != '' \ + AND NOT EXISTS ( \ + SELECT 1 FROM artist ar \ + WHERE ar.server_id = track.server_id AND ar.id = track.artist_id \ + ) \ + GROUP BY artist_id", + )?; + let collected = stmt + .query_map(params![server_id], |row| { + Ok((row.get::<_, String>(0)?, row.get::<_, String>(1)?)) + })? + .collect::>>()?; + Ok(collected) + }) + .map_err(|e| e.to_string())?; + + if rows.is_empty() { + return Ok(0); + } + + let mut count = 0u32; + self.store.with_conn_mut("artist.backfill_from_tracks", |conn| { + let tx = conn.transaction()?; + for (id, name) in &rows { + let name_sort = sort_key_for_display_name(name, ignored_articles); + upsert_artist_row(&tx, server_id, id, name, &name_sort, None, synced_at)?; + count += 1; + } + tx.commit()?; + Ok(()) + })?; + Ok(count) + } + + /// One-time repair: fill `name_sort` where null (upgrade path). + pub fn backfill_null_name_sort(&self, ignored_articles: &str) -> Result { + let rows: Vec<(String, String, String)> = self + .store + .with_read_conn(|conn| { + let mut stmt = + conn.prepare("SELECT server_id, id, name FROM artist WHERE name_sort IS NULL")?; + let collected = stmt + .query_map([], |row| { + Ok(( + row.get::<_, String>(0)?, + row.get::<_, String>(1)?, + row.get::<_, String>(2)?, + )) + })? + .collect::>>()?; + Ok(collected) + }) + .map_err(|e| e.to_string())?; + + if rows.is_empty() { + return Ok(0); + } + + let mut count = 0u32; + self.store.with_conn_mut("artist.backfill_null_name_sort", |conn| { + let tx = conn.transaction()?; + for (server_id, id, name) in &rows { + let name_sort = sort_key_for_display_name(name, ignored_articles); + tx.execute( + "UPDATE artist SET name_sort = ?1 WHERE server_id = ?2 AND id = ?3", + params![name_sort, server_id, id], + )?; + count += 1; + } + tx.commit()?; + Ok(()) + })?; + Ok(count) + } +} + +fn upsert_artist_row( + tx: &Transaction<'_>, + server_id: &str, + id: &str, + name: &str, + name_sort: &str, + album_count: Option, + synced_at: i64, +) -> rusqlite::Result<()> { + tx.execute( + "INSERT INTO artist (server_id, id, name, name_sort, album_count, synced_at) \ + VALUES (?1, ?2, ?3, ?4, ?5, ?6) \ + ON CONFLICT(server_id, id) DO UPDATE SET \ + name = excluded.name, \ + name_sort = excluded.name_sort, \ + album_count = COALESCE(excluded.album_count, artist.album_count), \ + synced_at = excluded.synced_at", + params![server_id, id, name, name_sort, album_count, synced_at], + )?; + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::store::LibraryStore; + use psysonic_integration::subsonic::{ArtistIndex, ArtistRef, IndexBucket}; + + #[test] + fn upsert_index_stores_name_sort_for_the_beatles() { + let store = LibraryStore::open_in_memory(); + let repo = ArtistRepository::new(&store); + let index = ArtistIndex { + last_modified_ms: Some(1), + ignored_articles: Some("The".into()), + index: vec![IndexBucket { + name: "B".into(), + artist: vec![ArtistRef { + id: "ar_1".into(), + name: "The Beatles".into(), + album_count: Some(3), + cover_art: None, + }], + }], + }; + repo.upsert_index("s1", &index, 1000).unwrap(); + let name_sort: String = store + .with_conn("misc", |c| { + c.query_row( + "SELECT name_sort FROM artist WHERE server_id = 's1' AND id = 'ar_1'", + [], + |r| r.get(0), + ) + }) + .unwrap(); + assert_eq!(name_sort, "beatles"); + } +} diff --git a/src-tauri/crates/psysonic-library/src/repos/mod.rs b/src-tauri/crates/psysonic-library/src/repos/mod.rs index 6da4ab7f..5bc8ece0 100644 --- a/src-tauri/crates/psysonic-library/src/repos/mod.rs +++ b/src-tauri/crates/psysonic-library/src/repos/mod.rs @@ -1,3 +1,4 @@ +pub mod artist; pub mod artifact; pub mod fact; pub mod play_session; @@ -5,6 +6,7 @@ pub mod sync_state; pub mod track; pub mod track_id_history; +pub use artist::ArtistRepository; pub use artifact::ArtifactRepository; pub use fact::FactRepository; pub use play_session::PlaySessionRepository; diff --git a/src-tauri/crates/psysonic-library/src/repos/sync_state.rs b/src-tauri/crates/psysonic-library/src/repos/sync_state.rs index f5297f37..7f3beba6 100644 --- a/src-tauri/crates/psysonic-library/src/repos/sync_state.rs +++ b/src-tauri/crates/psysonic-library/src/repos/sync_state.rs @@ -561,6 +561,44 @@ impl<'a> SyncStateRepository<'a> { }) } + /// Read `ignored_articles` from the last `getArtists` pass (Navidrome + /// `IgnoredArticles` string — space-separated article tokens). + pub fn get_ignored_articles( + &self, + server_id: &str, + library_scope: &str, + ) -> Result, String> { + self.read(|conn| { + conn.query_row( + "SELECT ignored_articles FROM sync_state \ + WHERE server_id = ?1 AND library_scope = ?2", + params![server_id, library_scope], + |row| row.get::<_, Option>(0), + ) + .optional() + }) + .map(|opt| opt.flatten()) + } + + /// Persist server `ignoredArticles` for local artist sort-key computation. + pub fn set_ignored_articles( + &self, + server_id: &str, + library_scope: &str, + ignored_articles: &str, + ) -> Result<(), String> { + self.store.with_conn("sync_state.set_ignored_articles", |conn| { + conn.execute( + "INSERT INTO sync_state (server_id, library_scope, ignored_articles) \ + VALUES (?1, ?2, ?3) \ + ON CONFLICT(server_id, library_scope) DO UPDATE SET \ + ignored_articles = excluded.ignored_articles", + params![server_id, library_scope, ignored_articles], + )?; + Ok(()) + }) + } + /// Write `library_tier` (spec §6.2.2 — `small` / `medium` / `huge` /// / `unknown`). Drives the adaptive poll interval; PR-3d wires /// the EWMA loop that picks this. diff --git a/src-tauri/crates/psysonic-library/src/store.rs b/src-tauri/crates/psysonic-library/src/store.rs index 62ffaf7f..58ccd0ff 100644 --- a/src-tauri/crates/psysonic-library/src/store.rs +++ b/src-tauri/crates/psysonic-library/src/store.rs @@ -4,12 +4,18 @@ use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; use std::sync::Mutex; use std::time::Duration; -use rusqlite::{params, Connection, OpenFlags}; +use rusqlite::{params, Connection, OpenFlags, OptionalExtension}; use tauri::Manager; /// Current head of the embedded migrations. Bump each time a new /// `migrations/NNN_*.sql` is added. -pub const LIBRARY_DB_SCHEMA_VERSION: i64 = 13; +/// +/// Migration checklist (wiring, data backfill, open/swap path): +/// psysonic-workdocs `ai/agent-rules/08-library-db-migrations.md`. +pub const LIBRARY_DB_SCHEMA_VERSION: i64 = 14; + +/// One-time data repair after migration 014 (`artist.name_sort`). +pub(crate) const ARTIST_NAME_SORT_RECONCILE_ID: &str = "artist_name_sort_reconcile_v1"; /// Lowest applied schema version the current code can advance from purely /// additively. If a DB carries a version below this, the breaking-bump hook @@ -30,6 +36,8 @@ pub(crate) const MIGRATION_012_TRACK_GENRE_LEGACY: &str = /// artwork (fanart.tv) — image-scraper §12. Pure CREATE TABLE IF NOT EXISTS. pub(crate) const MIGRATION_013_ARTIST_ARTWORK_LOOKUP: &str = include_str!("../migrations/013_artist_artwork_lookup.sql"); +pub(crate) const MIGRATION_014_ARTIST_NAME_SORT: &str = + include_str!("../migrations/014_artist_name_sort.sql"); /// Embedded migrations. Ordered ascending by `version`; the runner sorts /// defensively before applying so the source order can stay readable. @@ -37,6 +45,7 @@ const MIGRATIONS: &[(i64, &str)] = &[ (1, INITIAL_SQL), (12, MIGRATION_012_TRACK_GENRE_LEGACY), (13, MIGRATION_013_ARTIST_ARTWORK_LOOKUP), + (14, MIGRATION_014_ARTIST_NAME_SORT), ]; /// Idempotent repair — also runs after the migration runner on every open so @@ -70,6 +79,9 @@ pub struct LibraryStore { read_conn: Mutex, /// IS-3 bulk ingest in progress — read paths skip write-lock work. bulk_ingest_active: AtomicBool, + /// `swap_database_file` / `restore_database_backup` — fail fast instead of + /// touching in-memory placeholder connections while the file is offline. + swap_in_progress: AtomicBool, } impl LibraryStore { @@ -83,10 +95,7 @@ impl LibraryStore { fn open_file(db_path: &Path) -> Result { let write_conn = Connection::open(db_path).map_err(|e| e.to_string())?; - configure_write_connection(&write_conn).map_err(|e| e.to_string())?; - run_migrations(&write_conn).map_err(|e| e.to_string())?; - ensure_genre_tags_schema(&write_conn).map_err(|e| e.to_string())?; - checkpoint_wal_conn(&write_conn, "open").map_err(|e| e.to_string())?; + prepare_write_connection_for_open(&write_conn).map_err(|e| e.to_string())?; let read_conn = Connection::open_with_flags(db_path, OpenFlags::SQLITE_OPEN_READ_ONLY) .map_err(|e| e.to_string())?; configure_read_connection(&read_conn).map_err(|e| e.to_string())?; @@ -94,6 +103,7 @@ impl LibraryStore { write_conn: Mutex::new(write_conn), read_conn: Mutex::new(read_conn), bulk_ingest_active: AtomicBool::new(false), + swap_in_progress: AtomicBool::new(false), }) } @@ -102,14 +112,14 @@ impl LibraryStore { let uri = in_memory_uri(); let write_conn = Connection::open(&uri).expect("in-memory write connection"); configure_write_connection(&write_conn).expect("write pragmas"); - run_migrations(&write_conn).expect("schema migration"); - ensure_genre_tags_schema(&write_conn).expect("genre tags schema"); + prepare_write_connection_for_open(&write_conn).expect("schema migration"); let read_conn = Connection::open(&uri).expect("in-memory read connection"); configure_read_connection(&read_conn).expect("read pragmas"); Self { write_conn: Mutex::new(write_conn), read_conn: Mutex::new(read_conn), bulk_ingest_active: AtomicBool::new(false), + swap_in_progress: AtomicBool::new(false), } } @@ -122,6 +132,36 @@ impl LibraryStore { self.bulk_ingest_active.load(Ordering::Acquire) } + fn swap_in_progress(&self) -> bool { + self.swap_in_progress.load(Ordering::Acquire) + } + + fn lock_write_conn(&self) -> Result, String> { + if self.swap_in_progress() { + return Err("library database swap in progress".to_string()); + } + match self.write_conn.lock() { + Ok(guard) => Ok(guard), + Err(poisoned) => { + crate::app_eprintln!("[library-db] write lock was poisoned — recovering"); + Ok(poisoned.into_inner()) + } + } + } + + fn lock_read_conn(&self) -> Result, String> { + if self.swap_in_progress() { + return Err("library database swap in progress".to_string()); + } + match self.read_conn.lock() { + Ok(guard) => Ok(guard), + Err(poisoned) => { + crate::app_eprintln!("[library-db] read lock was poisoned — recovering"); + Ok(poisoned.into_inner()) + } + } + } + /// Writer connection — sync ingest, migrations, mutations. /// /// `op` is logged on slow writes (`[library-db] SLOW write op=…`) — use a @@ -134,13 +174,10 @@ impl LibraryStore { f: impl FnOnce(&Connection) -> rusqlite::Result, ) -> Result { let lock_start = std::time::Instant::now(); - let conn = self - .write_conn - .lock() - .map_err(|_| "library store write lock poisoned".to_string())?; + let conn = self.lock_write_conn()?; let lock_wait_ms = lock_start.elapsed().as_millis(); let exec_start = std::time::Instant::now(); - let out = f(&conn).map_err(|e| e.to_string()); + let out = run_conn_closure(&conn, f); let exec_ms = exec_start.elapsed().as_millis(); log_write_op(op, lock_wait_ms, exec_ms); out @@ -151,11 +188,8 @@ impl LibraryStore { &self, f: impl FnOnce(&Connection) -> rusqlite::Result, ) -> Result { - let conn = self - .read_conn - .lock() - .map_err(|_| "library store read lock poisoned".to_string())?; - f(&conn).map_err(|e| e.to_string()) + let conn = self.lock_read_conn()?; + run_conn_closure(&conn, f) } pub(crate) fn with_conn_mut( @@ -172,13 +206,10 @@ impl LibraryStore { f: impl FnOnce(&mut Connection) -> rusqlite::Result, ) -> Result<(R, WriteOpTiming), String> { let lock_start = std::time::Instant::now(); - let mut conn = self - .write_conn - .lock() - .map_err(|_| "library store write lock poisoned".to_string())?; + let mut conn = self.lock_write_conn()?; let lock_wait_ms = lock_start.elapsed().as_millis() as u64; let exec_start = std::time::Instant::now(); - let out = f(&mut conn).map_err(|e| e.to_string())?; + let out = run_conn_mut_closure(&mut conn, f)?; let exec_ms = exec_start.elapsed().as_millis() as u64; log_write_op(op, lock_wait_ms as u128, exec_ms as u128); Ok((out, WriteOpTiming { lock_wait_ms, exec_ms })) @@ -192,8 +223,8 @@ impl LibraryStore { } /// Atomically switch the active sqlite file while replacing long-lived - /// write/read connections under the same locks so no command can keep - /// writing to the old inode after the swap. + /// write/read connections. Other threads see `library database swap in + /// progress` while the file is offline instead of touching placeholder DBs. pub fn swap_database_file( &self, active_path: &Path, @@ -202,14 +233,14 @@ impl LibraryStore { if !destination_path.exists() { return Ok(None); } - let mut write_conn = self - .write_conn - .lock() - .map_err(|_| "library store write lock poisoned".to_string())?; - let mut read_conn = self - .read_conn - .lock() - .map_err(|_| "library store read lock poisoned".to_string())?; + + let mut swap_guard = SwapInProgressGuard::new(self); + let mut write_conn = self.write_conn.lock().map_err(|_| { + "library store write lock poisoned during database swap".to_string() + })?; + let mut read_conn = self.read_conn.lock().map_err(|_| { + "library store read lock poisoned during database swap".to_string() + })?; let write_tmp = Connection::open_in_memory().map_err(|e| e.to_string())?; let read_tmp = Connection::open_in_memory().map_err(|e| e.to_string())?; @@ -237,28 +268,68 @@ impl LibraryStore { let _ = move_sidecar(&backup, active_path, "-wal"); let _ = move_sidecar(&backup, active_path, "-shm"); } + drop(read_conn); + drop(write_conn); + let (reopened_write, reopened_read) = open_database_connections(active_path) + .map_err(|e| format!("library swap reopen failed after rename error: {e}"))?; + let mut write_conn = self.write_conn.lock().map_err(|_| { + "library store write lock poisoned during database swap".to_string() + })?; + let mut read_conn = self.read_conn.lock().map_err(|_| { + "library store read lock poisoned during database swap".to_string() + })?; + *write_conn = reopened_write; + *read_conn = reopened_read; + swap_guard.release(); return Err(err.to_string()); } - let reopened_write = Connection::open(active_path).map_err(|e| e.to_string())?; - configure_write_connection(&reopened_write).map_err(|e| e.to_string())?; - let reopened_read = Connection::open_with_flags(active_path, OpenFlags::SQLITE_OPEN_READ_ONLY) - .map_err(|e| e.to_string())?; - configure_read_connection(&reopened_read).map_err(|e| e.to_string())?; - *write_conn = reopened_write; - *read_conn = reopened_read; - Ok(Some(backup)) + drop(read_conn); + drop(write_conn); + + let reopen = open_database_connections(active_path); + + let mut write_conn = self.write_conn.lock().map_err(|_| { + "library store write lock poisoned during database swap".to_string() + })?; + let mut read_conn = self.read_conn.lock().map_err(|_| { + "library store read lock poisoned during database swap".to_string() + })?; + + match reopen { + Ok((reopened_write, reopened_read)) => { + *write_conn = reopened_write; + *read_conn = reopened_read; + swap_guard.release(); + Ok(Some(backup)) + } + Err(open_err) => { + if backup.exists() { + if active_path.exists() { + remove_db_with_sidecars(active_path).ok(); + } + let _ = fs::rename(&backup, active_path); + let _ = move_sidecar(&backup, active_path, "-wal"); + let _ = move_sidecar(&backup, active_path, "-shm"); + } + let (reopened_write, reopened_read) = open_database_connections(active_path) + .map_err(|e| format!("library swap reopen failed after revert: {e}"))?; + *write_conn = reopened_write; + *read_conn = reopened_read; + swap_guard.release(); + Err(format!("library swap failed: {open_err}")) + } + } } pub fn restore_database_backup(&self, backup_path: &Path, active_path: &Path) -> Result<(), String> { - let mut write_conn = self - .write_conn - .lock() - .map_err(|_| "library store write lock poisoned".to_string())?; - let mut read_conn = self - .read_conn - .lock() - .map_err(|_| "library store read lock poisoned".to_string())?; + let mut swap_guard = SwapInProgressGuard::new(self); + let mut write_conn = self.write_conn.lock().map_err(|_| { + "library store write lock poisoned during database restore".to_string() + })?; + let mut read_conn = self.read_conn.lock().map_err(|_| { + "library store read lock poisoned during database restore".to_string() + })?; let write_tmp = Connection::open_in_memory().map_err(|e| e.to_string())?; let read_tmp = Connection::open_in_memory().map_err(|e| e.to_string())?; @@ -276,13 +347,21 @@ impl LibraryStore { move_sidecar(backup_path, active_path, "-shm")?; } - let reopened_write = Connection::open(active_path).map_err(|e| e.to_string())?; - configure_write_connection(&reopened_write).map_err(|e| e.to_string())?; - let reopened_read = Connection::open_with_flags(active_path, OpenFlags::SQLITE_OPEN_READ_ONLY) - .map_err(|e| e.to_string())?; - configure_read_connection(&reopened_read).map_err(|e| e.to_string())?; + drop(read_conn); + drop(write_conn); + + let (reopened_write, reopened_read) = + open_database_connections(active_path).map_err(|e| e.to_string())?; + + let mut write_conn = self.write_conn.lock().map_err(|_| { + "library store write lock poisoned during database restore".to_string() + })?; + let mut read_conn = self.read_conn.lock().map_err(|_| { + "library store read lock poisoned during database restore".to_string() + })?; *write_conn = reopened_write; *read_conn = reopened_read; + swap_guard.release(); Ok(()) } } @@ -310,6 +389,74 @@ fn log_write_op(op: &str, lock_wait_ms: u128, exec_ms: u128) { } } +struct SwapInProgressGuard<'a> { + store: &'a LibraryStore, + released: bool, +} + +impl<'a> SwapInProgressGuard<'a> { + fn new(store: &'a LibraryStore) -> Self { + store.swap_in_progress.store(true, Ordering::Release); + Self { + store, + released: false, + } + } + + fn release(&mut self) { + if !self.released { + self.store.swap_in_progress.store(false, Ordering::Release); + self.released = true; + } + } +} + +impl Drop for SwapInProgressGuard<'_> { + fn drop(&mut self) { + self.release(); + } +} + +fn run_conn_closure( + conn: &Connection, + f: impl FnOnce(&Connection) -> rusqlite::Result, +) -> Result { + let out = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| f(conn))); + match out { + Ok(result) => result.map_err(|e| e.to_string()), + Err(payload) => { + let detail = panic_payload_to_string(payload); + crate::app_eprintln!("[library-db] connection query panicked: {detail}"); + Err(format!("library connection query panicked: {detail}")) + } + } +} + +fn run_conn_mut_closure( + conn: &mut Connection, + f: impl FnOnce(&mut Connection) -> rusqlite::Result, +) -> Result { + let out = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| f(conn))); + match out { + Ok(result) => result.map_err(|e| e.to_string()), + Err(payload) => { + let detail = panic_payload_to_string(payload); + crate::app_eprintln!("[library-db] connection mutation panicked: {detail}"); + Err(format!("library connection mutation panicked: {detail}")) + } + } +} + +fn panic_payload_to_string(payload: Box) -> String { + if let Some(msg) = payload.downcast_ref::<&str>() { + msg.to_string() + } else if let Some(msg) = payload.downcast_ref::() { + msg.clone() + } else { + "unknown panic payload".to_string() + } +} + fn library_db_path(app: &tauri::AppHandle) -> Result { let base = app.path().app_data_dir().map_err(|e| e.to_string())?; let db_dir = base.join("databases").join("library"); @@ -429,6 +576,151 @@ fn checkpoint_wal_conn(conn: &Connection, op: &str) -> rusqlite::Result<()> { Ok(()) } +/// Open write + read handles after migrations, one-time repairs, and WAL checkpoint. +fn open_database_connections(db_path: &Path) -> rusqlite::Result<(Connection, Connection)> { + let write_conn = Connection::open(db_path)?; + configure_write_connection(&write_conn)?; + prepare_write_connection_for_open(&write_conn)?; + let read_conn = Connection::open_with_flags(db_path, OpenFlags::SQLITE_OPEN_READ_ONLY)?; + configure_read_connection(&read_conn)?; + Ok((write_conn, read_conn)) +} + +fn prepare_write_connection_for_open(conn: &Connection) -> rusqlite::Result<()> { + run_migrations(conn)?; + maybe_reconcile_artist_name_sort(conn)?; + ensure_genre_tags_schema(conn)?; + checkpoint_wal_conn(conn, "open")?; + Ok(()) +} + +fn artist_name_sort_column_exists(conn: &Connection) -> rusqlite::Result { + let column_exists: i64 = conn + .query_row( + "SELECT COUNT(*) FROM pragma_table_info('artist') WHERE name = 'name_sort'", + [], + |row| row.get(0), + ) + .unwrap_or(0); + Ok(column_exists > 0) +} + +fn sync_state_ignored_articles_column_exists(conn: &Connection) -> rusqlite::Result { + let column_exists: i64 = conn + .query_row( + "SELECT COUNT(*) FROM pragma_table_info('sync_state') WHERE name = 'ignored_articles'", + [], + |row| row.get(0), + ) + .unwrap_or(0); + Ok(column_exists > 0) +} + +/// Apply schema 014 idempotently — mirrors `migrations/014_artist_name_sort.sql` +/// but tolerates a partial prior apply (missing one column / re-run). +fn apply_migration_14(conn: &Connection) -> rusqlite::Result<()> { + if !artist_name_sort_column_exists(conn)? { + conn.execute_batch("ALTER TABLE artist ADD COLUMN name_sort TEXT;")?; + } + if !sync_state_ignored_articles_column_exists(conn)? { + conn.execute_batch("ALTER TABLE sync_state ADD COLUMN ignored_articles TEXT;")?; + } + conn.execute_batch( + "CREATE INDEX IF NOT EXISTS idx_artist_name_sort ON artist(server_id, name_sort);", + )?; + finish_migration_14_reconcile(conn)?; + Ok(()) +} + +fn record_schema_migration(conn: &Connection, version: i64) -> rusqlite::Result<()> { + conn.execute( + "INSERT INTO schema_migrations (version, applied_at) VALUES (?1, strftime('%s','now'))", + params![version], + )?; + Ok(()) +} + +fn finish_migration_14_reconcile(conn: &Connection) -> rusqlite::Result<()> { + if !artist_name_sort_reconcile_completed(conn)? { + repair_artist_name_sort_keys(conn)?; + mark_artist_name_sort_reconcile_completed(conn)?; + } + Ok(()) +} + +fn artist_name_sort_reconcile_completed(conn: &Connection) -> rusqlite::Result { + let completed: Option> = conn + .query_row( + "SELECT completed_at FROM library_data_migration WHERE id = ?1", + params![ARTIST_NAME_SORT_RECONCILE_ID], + |row| row.get(0), + ) + .optional()?; + Ok(completed.flatten().is_some()) +} + +fn mark_artist_name_sort_reconcile_completed(conn: &Connection) -> rusqlite::Result<()> { + conn.execute( + "INSERT INTO library_data_migration (id, cursor_rowid, started_at, completed_at) \ + VALUES (?1, 0, strftime('%s','now'), strftime('%s','now')) \ + ON CONFLICT(id) DO UPDATE SET completed_at = excluded.completed_at", + params![ARTIST_NAME_SORT_RECONCILE_ID], + )?; + Ok(()) +} + +/// One-time reconcile after schema 014 — not on every open (avoids long write locks at startup). +fn maybe_reconcile_artist_name_sort(conn: &Connection) -> rusqlite::Result<()> { + if !artist_name_sort_column_exists(conn)? { + return Ok(()); + } + if artist_name_sort_reconcile_completed(conn)? { + return Ok(()); + } + repair_artist_name_sort_keys(conn)?; + mark_artist_name_sort_reconcile_completed(conn)?; + Ok(()) +} + +/// Reconcile `artist.name_sort` with display `name` (upgrade / stale rows). +fn repair_artist_name_sort_keys(conn: &Connection) -> rusqlite::Result<()> { + let table_exists: i64 = conn + .query_row( + "SELECT COUNT(*) FROM sqlite_master WHERE type = 'table' AND name = 'artist'", + [], + |row| row.get(0), + ) + .unwrap_or(0); + if table_exists == 0 { + return Ok(()); + } + if !artist_name_sort_column_exists(conn)? { + return Ok(()); + } + let ignored = crate::artist_sort::DEFAULT_IGNORED_ARTICLES; + let tx = conn.unchecked_transaction()?; + { + let mut stmt = tx.prepare("SELECT server_id, id, name, name_sort FROM artist")?; + let mut rows = stmt.query([])?; + while let Some(row) = rows.next()? { + let server_id: String = row.get(0)?; + let id: String = row.get(1)?; + let name: String = row.get(2)?; + let current: Option = row.get(3)?; + let expected = crate::artist_sort::sort_key_for_display_name(&name, ignored); + if current.as_deref() == Some(&expected) { + continue; + } + tx.execute( + "UPDATE artist SET name_sort = ?1 WHERE server_id = ?2 AND id = ?3", + rusqlite::params![expected, server_id, id], + )?; + } + } + tx.commit()?; + Ok(()) +} + fn run_migrations(conn: &Connection) -> rusqlite::Result { run_migrations_with( conn, @@ -477,11 +769,17 @@ pub(crate) fn run_migrations_with( if already > 0 { continue; } + if version == 14 { + // Applied idempotently (per-column ADD + IF NOT EXISTS index) so a + // partial DDL apply — one ALTER landed before a crash, no + // schema_migrations row — recovers instead of failing on a + // duplicate-column re-run of the batch. + apply_migration_14(conn)?; + record_schema_migration(conn, version)?; + continue; + } conn.execute_batch(sql)?; - conn.execute( - "INSERT INTO schema_migrations (version, applied_at) VALUES (?1, strftime('%s','now'))", - params![version], - )?; + record_schema_migration(conn, version)?; } Ok(MigrationOutcome::Applied) } @@ -782,4 +1080,125 @@ mod tests { .unwrap(); assert_eq!(outcome, MigrationOutcome::Applied); } + + #[test] + fn artist_name_sort_reconcile_runs_once_and_sets_name_sort() { + let store = LibraryStore::open_in_memory(); + store + .with_conn_mut("test.seed_artist", |conn| { + conn.execute( + "INSERT INTO artist (server_id, id, name, name_sort, synced_at) \ + VALUES ('s1', 'ar1', 'The Beatles', 'the beatles', 1)", + [], + )?; + conn.execute( + "DELETE FROM library_data_migration WHERE id = ?1", + params![ARTIST_NAME_SORT_RECONCILE_ID], + )?; + Ok(()) + }) + .expect("seed artist"); + + store + .with_conn("test.reconcile", maybe_reconcile_artist_name_sort) + .expect("reconcile"); + + let name_sort: String = store + .with_read_conn(|conn| { + conn.query_row( + "SELECT name_sort FROM artist WHERE server_id = 's1' AND id = 'ar1'", + [], + |r| r.get(0), + ) + }) + .expect("read name_sort"); + assert_eq!(name_sort, "beatles"); + + let completed_before: i64 = store + .with_read_conn(|conn| { + conn.query_row( + "SELECT completed_at FROM library_data_migration WHERE id = ?1", + params![ARTIST_NAME_SORT_RECONCILE_ID], + |r| r.get(0), + ) + }) + .expect("reconcile marker"); + assert!(completed_before > 0); + + store + .with_conn("test.reconcile_again", maybe_reconcile_artist_name_sort) + .expect("reconcile again"); + + let name_sort_after: String = store + .with_read_conn(|conn| { + conn.query_row( + "SELECT name_sort FROM artist WHERE server_id = 's1' AND id = 'ar1'", + [], + |r| r.get(0), + ) + }) + .expect("read name_sort again"); + assert_eq!(name_sort_after, "beatles"); + } + + #[test] + fn migration_14_recovers_partial_schema_without_schema_migrations_row() { + let uri = in_memory_uri(); + let conn = Connection::open(&uri).expect("connection"); + configure_write_connection(&conn).expect("pragmas"); + let migrations_through_13: &[(i64, &str)] = &[ + (1, INITIAL_SQL), + (12, MIGRATION_012_TRACK_GENRE_LEGACY), + (13, MIGRATION_013_ARTIST_ARTWORK_LOOKUP), + ]; + run_migrations_with( + &conn, + migrations_through_13, + LIBRARY_DB_MIN_COMPATIBLE_VERSION, + no_op_hook, + ) + .expect("migrate through v13"); + conn.execute_batch(MIGRATION_014_ARTIST_NAME_SORT) + .expect("apply ddl only"); + + let recorded: i64 = conn + .query_row( + "SELECT COUNT(*) FROM schema_migrations WHERE version = 14", + [], + |r| r.get(0), + ) + .expect("count migration"); + assert_eq!(recorded, 0); + + run_migrations_with( + &conn, + MIGRATIONS, + LIBRARY_DB_MIN_COMPATIBLE_VERSION, + no_op_hook, + ) + .expect("recover partial migration"); + + let recorded_after: i64 = conn + .query_row( + "SELECT COUNT(*) FROM schema_migrations WHERE version = 14", + [], + |r| r.get(0), + ) + .expect("count migration after"); + assert_eq!(recorded_after, 1); + } + + #[test] + fn read_conn_recovers_after_closure_panic() { + let store = LibraryStore::open_in_memory(); + let first: Result = store.with_read_conn(|_conn| { + panic!("simulated read panic"); + }); + assert!(first.is_err()); + + let ok: i64 = store + .with_read_conn(|conn| conn.query_row("SELECT 1", [], |r| r.get(0))) + .expect("read after panic recovery"); + assert_eq!(ok, 1); + } } diff --git a/src-tauri/crates/psysonic-library/src/sync/artist_index.rs b/src-tauri/crates/psysonic-library/src/sync/artist_index.rs new file mode 100644 index 00000000..79dfb5f9 --- /dev/null +++ b/src-tauri/crates/psysonic-library/src/sync/artist_index.rs @@ -0,0 +1,32 @@ +//! Persist Subsonic `getArtists` / `getIndexes` bodies into the local `artist` table. + +use crate::repos::{ArtistRepository, SyncStateRepository}; +use crate::store::LibraryStore; +use psysonic_integration::subsonic::ArtistIndex; + +use super::error::SyncError; + +pub fn apply_artist_index( + store: &LibraryStore, + server_id: &str, + library_scope: &str, + index: &ArtistIndex, +) -> Result<(), SyncError> { + let synced_at = super::now_unix_ms(); + let ignored = crate::artist_sort::ignored_articles_or_default( + index.ignored_articles.as_deref(), + ); + let sync_state = SyncStateRepository::new(store); + sync_state + .set_ignored_articles(server_id, library_scope, ignored) + .map_err(SyncError::Storage)?; + let repo = ArtistRepository::new(store); + repo.upsert_index(server_id, index, synced_at).map_err(SyncError::Storage)?; + repo.backfill_from_tracks(server_id, ignored, synced_at).map_err(SyncError::Storage)?; + if let Some(ms) = index.last_modified_ms { + sync_state + .set_artists_last_modified_ms(server_id, library_scope, ms) + .map_err(SyncError::Storage)?; + } + Ok(()) +} diff --git a/src-tauri/crates/psysonic-library/src/sync/delta.rs b/src-tauri/crates/psysonic-library/src/sync/delta.rs index 3ba36605..09df2b5b 100644 --- a/src-tauri/crates/psysonic-library/src/sync/delta.rs +++ b/src-tauri/crates/psysonic-library/src/sync/delta.rs @@ -204,8 +204,20 @@ impl<'a> DeltaSyncRunner<'a> { } } - // DS-9 — stamp watermarks. + // DS-9 — stamp watermarks + refresh artist browse index when applicable. if let Some(ms) = probe.next_artists_watermark { + let scope = self.library_scope_opt(); + if let Ok(index) = self.subsonic.get_artists(scope).await { + super::artist_index::apply_artist_index( + self.store, + &self.server_id, + &self.library_scope, + &index, + )?; + } + // Advance the watermark to the probed value regardless of the index + // refresh result — a failed/empty `getArtists` must not force a full + // refetch on every delta. Wins over the index's own last-modified. sync_state .set_artists_last_modified_ms(&self.server_id, &self.library_scope, ms) .map_err(SyncError::Storage)?; @@ -529,13 +541,7 @@ struct DeltaPollOutcome { next_artists_watermark: Option, } -fn now_unix_ms() -> i64 { - use std::time::{SystemTime, UNIX_EPOCH}; - SystemTime::now() - .duration_since(UNIX_EPOCH) - .map(|d| d.as_millis().min(i64::MAX as u128) as i64) - .unwrap_or(0) -} +use super::now_unix_ms; async fn retry_with_backoff<'a, F, FFut, T, E>( runner: &DeltaSyncRunner<'a>, diff --git a/src-tauri/crates/psysonic-library/src/sync/initial.rs b/src-tauri/crates/psysonic-library/src/sync/initial.rs index 1852e893..701be5ec 100644 --- a/src-tauri/crates/psysonic-library/src/sync/initial.rs +++ b/src-tauri/crates/psysonic-library/src/sync/initial.rs @@ -1179,7 +1179,7 @@ impl<'a> InitialSyncRunner<'a> { async fn run_artist_pass( &self, - sync_state: &SyncStateRepository<'_>, + _sync_state: &SyncStateRepository<'_>, ) -> Result<(), SyncError> { let scope = self.library_scope_opt(); let artists = retry_with_backoff( @@ -1190,11 +1190,12 @@ impl<'a> InitialSyncRunner<'a> { .await .ok(); if let Some(index) = artists { - if let Some(ms) = index.last_modified_ms { - sync_state - .set_artists_last_modified_ms(&self.server_id, &self.library_scope, ms) - .map_err(SyncError::Storage)?; - } + super::artist_index::apply_artist_index( + self.store, + &self.server_id, + &self.library_scope, + &index, + )?; } Ok(()) } @@ -1227,13 +1228,7 @@ fn is_empty_cursor(v: &Value) -> bool { matches!(v, Value::Object(o) if o.is_empty()) } -fn now_unix_ms() -> i64 { - use std::time::{SystemTime, UNIX_EPOCH}; - SystemTime::now() - .duration_since(UNIX_EPOCH) - .map(|d| d.as_millis().min(i64::MAX as u128) as i64) - .unwrap_or(0) -} +use super::now_unix_ms; /// Wrap an async closure in §6.8 backoff. Retries on `SyncError::Transport` /// up to `MAX_ATTEMPTS_PER_BATCH`, sleeping per the backoff schedule diff --git a/src-tauri/crates/psysonic-library/src/sync/mod.rs b/src-tauri/crates/psysonic-library/src/sync/mod.rs index a3521378..ef8f0232 100644 --- a/src-tauri/crates/psysonic-library/src/sync/mod.rs +++ b/src-tauri/crates/psysonic-library/src/sync/mod.rs @@ -6,6 +6,7 @@ //! `DeltaSyncRunner` / background scheduler / Tauri surface follow in //! PR-3c / PR-3d / PR-5. +pub mod artist_index; pub mod backoff; pub mod bandwidth; pub mod budget; @@ -38,3 +39,12 @@ pub use scheduler::{BackgroundScheduler, SchedulerTickReport, DEFAULT_TOMBSTONE_ pub use strategy::IngestStrategy; pub use supervisor::SyncSupervisor; pub use tombstone::{should_auto_reconcile, TombstoneReconciler, TombstoneReport}; + +/// Wall-clock milliseconds since the Unix epoch, saturating to `i64::MAX`. +pub(crate) fn now_unix_ms() -> i64 { + use std::time::{SystemTime, UNIX_EPOCH}; + SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|d| d.as_millis().min(i64::MAX as u128) as i64) + .unwrap_or(0) +} diff --git a/src/api/library.ts b/src/api/library.ts index 0be172d2..ba3a7d1a 100644 --- a/src/api/library.ts +++ b/src/api/library.ts @@ -80,6 +80,7 @@ export interface SyncStateDto { serverLastScanIso?: string | null; indexesLastModifiedMs?: number | null; artistsLastModifiedMs?: number | null; + ignoredArticles?: string | null; localTrackCount?: number | null; serverTrackCount?: number | null; lastError?: string | null; @@ -237,6 +238,7 @@ export interface LibraryArtistDto { serverId: string; id: string; name: string; + nameSort?: string | null; albumCount?: number | null; syncedAt: number; rawJson: unknown; diff --git a/src/api/subsonicTypes.ts b/src/api/subsonicTypes.ts index 7222f410..3671644d 100644 --- a/src/api/subsonicTypes.ts +++ b/src/api/subsonicTypes.ts @@ -161,6 +161,8 @@ export interface SubsonicNowPlaying extends SubsonicSong { export interface SubsonicArtist { id: string; name: string; + /** Article-stripped lowercase sort key (local index / OpenSubsonic). */ + nameSort?: string; albumCount?: number; coverArt?: string; starred?: string; diff --git a/src/config/settingsCredits.ts b/src/config/settingsCredits.ts index 060d0d54..43c69ed4 100644 --- a/src/config/settingsCredits.ts +++ b/src/config/settingsCredits.ts @@ -170,6 +170,7 @@ const CONTRIBUTOR_ENTRIES = [ 'AutoDJ — smooth skip and interrupt blend: manual/out-of-queue crossfade, loud→loud ~2s advance, cold-target prep duck + deferred player-bar handoff (PR #1128)', 'Play queue sync — manual pull via connection indicator, idle auto-pull, multi-server push filter, flush-on-server-switch (PR #1131)', 'Niri compositor tiling WM detection (PR #1127)', + 'Local library index: Navidrome ignored-articles artist/composer letter buckets (name_sort + server ignoredArticles), idempotent migration with safe open/swap and poisoned-lock recovery (PR #1145)', ], }, { diff --git a/src/hooks/useArtistsFiltering.ts b/src/hooks/useArtistsFiltering.ts index 64a5c174..17fb40e7 100644 --- a/src/hooks/useArtistsFiltering.ts +++ b/src/hooks/useArtistsFiltering.ts @@ -1,7 +1,7 @@ import { useMemo } from 'react'; import type { SubsonicArtist } from '../api/subsonicTypes'; import { usePlayerStore } from '../store/playerStore'; -import { ALL_SENTINEL, artistBucketKey, compareBuckets, type ArtistListFlatRow } from '../utils/componentHelpers/artistsHelpers'; +import { ALL_SENTINEL, artistLetterBucket, compareBuckets, type ArtistListFlatRow } from '../utils/componentHelpers/artistsHelpers'; interface UseArtistsFilteringArgs { artists: SubsonicArtist[]; @@ -10,6 +10,8 @@ interface UseArtistsFilteringArgs { starredOnly: boolean; visibleCount: number; viewMode: 'grid' | 'list'; + /** Server `ignoredArticles` when known (local index); omit for Navidrome default. */ + ignoredArticles?: string | null; } interface UseArtistsFilteringResult { @@ -42,13 +44,14 @@ export function useArtistsFiltering({ starredOnly, visibleCount, viewMode, + ignoredArticles, }: UseArtistsFilteringArgs): UseArtistsFilteringResult { const starredOverrides = usePlayerStore(s => s.starredOverrides); const filtered = useMemo(() => { let out = artists; if (letterFilter !== ALL_SENTINEL) { - out = out.filter(a => artistBucketKey(a.name) === letterFilter); + out = out.filter(a => artistLetterBucket(a, ignoredArticles) === letterFilter); } if (filter) { const needle = filter.toLowerCase(); @@ -58,7 +61,7 @@ export function useArtistsFiltering({ out = out.filter(a => a.id in starredOverrides ? starredOverrides[a.id] : !!a.starred); } return out; - }, [artists, letterFilter, filter, starredOnly, starredOverrides]); + }, [artists, letterFilter, filter, starredOnly, starredOverrides, ignoredArticles]); const visible = useMemo(() => filtered.slice(0, visibleCount), [filtered, visibleCount]); const hasMore = visibleCount < filtered.length; @@ -67,12 +70,12 @@ export function useArtistsFiltering({ if (viewMode !== 'list') return { groups: {} as Record, letters: [] as string[] }; const g: Record = {}; for (const a of visible) { - const key = artistBucketKey(a.name); + const key = artistLetterBucket(a, ignoredArticles); if (!g[key]) g[key] = []; g[key].push(a); } return { groups: g, letters: Object.keys(g).sort(compareBuckets) }; - }, [visible, viewMode]); + }, [visible, viewMode, ignoredArticles]); const artistListFlatRows = useMemo((): ArtistListFlatRow[] => { if (viewMode !== 'list') return []; diff --git a/src/hooks/useLibraryIgnoredArticles.ts b/src/hooks/useLibraryIgnoredArticles.ts new file mode 100644 index 00000000..8a47fcfe --- /dev/null +++ b/src/hooks/useLibraryIgnoredArticles.ts @@ -0,0 +1,38 @@ +import { useEffect, useState } from 'react'; +import { libraryGetStatus } from '../api/library'; + +/** + * Server `ignoredArticles` (Navidrome `getArtists` watermark) for the active + * server's local index, used to bucket artist/composer letters the same way the + * index sorts `name_sort`. Falls back to `null` (Navidrome default list) when + * the index is disabled, the server omits the field, or the lookup fails. + * + * One lightweight read per server change — `ignoredArticles` rarely changes, so + * we deliberately avoid the polling that `useLibraryIndexSync` does. + */ +export function useLibraryIgnoredArticles( + serverId: string | null | undefined, + enabled = true, +): string | null { + const [ignoredArticles, setIgnoredArticles] = useState(null); + + useEffect(() => { + if (!enabled || !serverId) { + setIgnoredArticles(null); + return; + } + let cancelled = false; + void libraryGetStatus(serverId) + .then(status => { + if (!cancelled) setIgnoredArticles(status.ignoredArticles ?? null); + }) + .catch(() => { + if (!cancelled) setIgnoredArticles(null); + }); + return () => { + cancelled = true; + }; + }, [serverId, enabled]); + + return ignoredArticles; +} diff --git a/src/pages/Artists.tsx b/src/pages/Artists.tsx index 995e1285..ddee9556 100644 --- a/src/pages/Artists.tsx +++ b/src/pages/Artists.tsx @@ -20,6 +20,7 @@ import { ARTIST_LIST_ROW_EST, } from '../utils/componentHelpers/artistsHelpers'; import { useArtistsFiltering } from '../hooks/useArtistsFiltering'; +import { useLibraryIgnoredArticles } from '../hooks/useLibraryIgnoredArticles'; import { useArtistsBrowseCatalog } from '../hooks/useArtistsBrowseCatalog'; import { useBrowseArtistTextSearch } from '../hooks/useBrowseArtistTextSearch'; import { useMainstageInpageHeaderTight } from '../hooks/useMainstageInpageHeaderTight'; @@ -136,9 +137,11 @@ export default function Artists() { const selectedArtists = artists.filter(a => selectedIds.has(a.id)); + const ignoredArticles = useLibraryIgnoredArticles(serverId, indexEnabled); + const { filtered, visible, hasMore, groups, letters, artistListFlatRows, - } = useArtistsFiltering({ artists, filter: effectiveFilter, letterFilter, starredOnly, visibleCount, viewMode }); + } = useArtistsFiltering({ artists, filter: effectiveFilter, letterFilter, starredOnly, visibleCount, viewMode, ignoredArticles }); const pendingLetterMatch = browseMode === 'slice' diff --git a/src/pages/Composers.tsx b/src/pages/Composers.tsx index f63432e7..639c6318 100644 --- a/src/pages/Composers.tsx +++ b/src/pages/Composers.tsx @@ -20,6 +20,8 @@ import { peekComposerBrowseScrollRestore } from '../store/composerBrowseSessionS import { useScopedBrowseSearchQuery } from '../store/liveSearchScopeStore'; import { readComposerBrowseRestore } from '../utils/navigation/albumDetailNavigation'; import { filterArtistsWithRoleAlbumCredits } from '../utils/library/composerBrowse'; +import { ALL_SENTINEL, artistLetterBucket } from '../utils/componentHelpers/artistsHelpers'; +import { useLibraryIgnoredArticles } from '../hooks/useLibraryIgnoredArticles'; import { usePerfProbeFlags } from '../utils/perf/perfFlags'; import { VirtualCardGrid } from '../components/VirtualCardGrid'; import OverlayScrollArea from '../components/OverlayScrollArea'; @@ -28,7 +30,6 @@ import { useClientSliceInfiniteScroll } from '../hooks/useClientSliceInfiniteScr import { useInpageScrollViewport } from '../hooks/useInpageScrollViewport'; import InpageScrollSentinel from '../components/InpageScrollSentinel'; -const ALL_SENTINEL = 'ALL'; const ALPHABET = [ALL_SENTINEL, '#', ...'ABCDEFGHIJKLMNOPQRSTUVWXYZ'.split('')]; const COMPOSER_LIST_LETTER_ROW_EST = 48; @@ -172,15 +173,11 @@ export default function Composers() { }, [musicLibraryFilterVersion, reloadTick]); const starredOverrides = usePlayerStore(s => s.starredOverrides); + const ignoredArticles = useLibraryIgnoredArticles(serverId); const filtered = useMemo(() => { let out = composerSource; if (letterFilter !== ALL_SENTINEL) { - out = out.filter(a => { - const first = a.name[0]?.toUpperCase() ?? '#'; - const isAlpha = /^[A-Z]$/.test(first); - if (letterFilter === '#') return !isAlpha; - return first === letterFilter; - }); + out = out.filter(a => artistLetterBucket(a, ignoredArticles) === letterFilter); } if (effectiveFilter) { const needle = effectiveFilter.toLowerCase(); @@ -190,7 +187,7 @@ export default function Composers() { out = out.filter(a => a.id in starredOverrides ? starredOverrides[a.id] : !!a.starred); } return out; - }, [composerSource, letterFilter, effectiveFilter, starredOnly, starredOverrides]); + }, [composerSource, letterFilter, effectiveFilter, starredOnly, starredOverrides, ignoredArticles]); const visible = useMemo(() => filtered.slice(0, visibleCount), [filtered, visibleCount]); const hasMore = visibleCount < filtered.length; @@ -219,13 +216,12 @@ export default function Composers() { if (viewMode !== 'list') return { groups: {} as Record, letters: [] as string[] }; const g: Record = {}; for (const a of visible) { - const letter = a.name[0]?.toUpperCase() ?? '#'; - const key = /^[A-Z]$/.test(letter) ? letter : '#'; + const key = artistLetterBucket(a, ignoredArticles); if (!g[key]) g[key] = []; g[key].push(a); } return { groups: g, letters: Object.keys(g).sort() }; - }, [visible, viewMode]); + }, [visible, viewMode, ignoredArticles]); const composerListFlatRows = useMemo((): ComposerListFlatRow[] => { if (viewMode !== 'list') return []; diff --git a/src/utils/componentHelpers/artistsHelpers.test.ts b/src/utils/componentHelpers/artistsHelpers.test.ts index 37ca5a03..06a4f1bf 100644 --- a/src/utils/componentHelpers/artistsHelpers.test.ts +++ b/src/utils/componentHelpers/artistsHelpers.test.ts @@ -1,5 +1,69 @@ import { describe, expect, it } from 'vitest'; -import { artistBucketKey, compareBuckets, OTHER_BUCKET, ALPHABET } from './artistsHelpers'; +import { + artistBucketKey, + artistLetterBucket, + compareBuckets, + DEFAULT_IGNORED_ARTICLES, + OTHER_BUCKET, + ALPHABET, + sortKeyFromDisplayName, + stripLeadingArticles, +} from './artistsHelpers'; + +describe('stripLeadingArticles', () => { + it('strips The from Beatles', () => { + expect(stripLeadingArticles('The Beatles', DEFAULT_IGNORED_ARTICLES)).toBe('Beatles'); + }); + + it('strips The from Kinks', () => { + expect(stripLeadingArticles('The Kinks', DEFAULT_IGNORED_ARTICLES)).toBe('Kinks'); + }); + + it('honours a custom ignoredArticles list', () => { + expect(stripLeadingArticles('Los Lobos', 'Los')).toBe('Lobos'); + }); +}); + +describe('sortKeyFromDisplayName', () => { + it('strips articles and lowercases', () => { + expect(sortKeyFromDisplayName('The Beatles')).toBe('beatles'); + }); +}); + +describe('artistLetterBucket', () => { + it('buckets a browse row by its display name, ignoring stale nameSort', () => { + const artist = { id: '1', name: 'The Chemical Brothers', nameSort: 'the chemical brothers' }; + expect(artistLetterBucket(artist)).toBe('C'); + }); + + it('uses a server ignoredArticles override when supplied', () => { + const artist = { id: '2', name: 'Los Lobos' }; + expect(artistLetterBucket(artist, 'Los')).toBe('L'); + }); +}); + +describe('screenshot T-filter artists (Navidrome article rules)', () => { + it('keeps Theme/Tossers/Temper/Tiger under T after stripping The', () => { + expect(artistBucketKey('The Theme Guys')).toBe('T'); + expect(artistBucketKey('The Tossers')).toBe('T'); + expect(artistBucketKey('The Temper Trap')).toBe('T'); + expect(artistBucketKey('The Tiger Lillies')).toBe('T'); + expect(artistBucketKey('TV Themes')).toBe('T'); + expect(artistBucketKey('Tribute To The N...')).toBe('T'); + }); + + it('moves Beatles/Chemical/Cure/Doors out of T', () => { + expect(artistBucketKey('The Beatles')).toBe('B'); + expect(artistBucketKey('The Chemical Brothers')).toBe('C'); + expect(artistBucketKey('The Cure')).toBe('C'); + expect(artistBucketKey('The Doors')).toBe('D'); + expect(artistBucketKey('The Fat Rat')).toBe('F'); + }); + + it('keeps glued TheFatRat under T (no space after article — Navidrome parity)', () => { + expect(artistBucketKey('TheFatRat')).toBe('T'); + }); +}); describe('artistBucketKey', () => { it('buckets A–Z names by their uppercased first letter', () => { @@ -8,6 +72,11 @@ describe('artistBucketKey', () => { expect(artistBucketKey('mGla')).toBe('M'); }); + it('puts The Beatles under B', () => { + expect(artistBucketKey('The Beatles')).toBe('B'); + expect(artistBucketKey('The Kinks')).toBe('K'); + }); + it('puts digit-leading names in #', () => { expect(artistBucketKey('2Pac')).toBe('#'); expect(artistBucketKey('50 Cent')).toBe('#'); diff --git a/src/utils/componentHelpers/artistsHelpers.ts b/src/utils/componentHelpers/artistsHelpers.ts index 113f162b..a6aac807 100644 --- a/src/utils/componentHelpers/artistsHelpers.ts +++ b/src/utils/componentHelpers/artistsHelpers.ts @@ -6,23 +6,68 @@ export const ALL_SENTINEL = 'ALL'; export const OTHER_BUCKET = 'OTHER'; export const ALPHABET = [ALL_SENTINEL, '#', ...'ABCDEFGHIJKLMNOPQRSTUVWXYZ'.split(''), OTHER_BUCKET]; +/** Navidrome default (`IgnoredArticles` when the server omits the field). */ +export const DEFAULT_IGNORED_ARTICLES = 'The El La Los Las Le Les Os As O A'; + /** Stable ordering index for a bucket key — '#' first, A–Z, then 'Other' last. */ const BUCKET_ORDER = new Map(ALPHABET.map((l, i) => [l, i])); +/** Strip leading articles for sort/bucket keys (Navidrome `RemoveArticle` parity). */ +export function stripLeadingArticles( + name: string, + ignoredArticles = DEFAULT_IGNORED_ARTICLES, +): string { + const trimmed = name.trim(); + for (const article of ignoredArticles.split(' ').filter(Boolean)) { + const prefix = `${article} `; + if ( + trimmed.length >= prefix.length + && trimmed.slice(0, prefix.length).toLowerCase() === prefix.toLowerCase() + ) { + return trimmed.slice(prefix.length).trimStart(); + } + } + return trimmed; +} + +/** Sort key from display name — article strip + lowercase (Navidrome parity). */ +export function sortKeyFromDisplayName( + displayName: string, + ignoredArticles?: string | null, +): string { + const articles = ignoredArticles?.trim() || DEFAULT_IGNORED_ARTICLES; + return stripLeadingArticles(displayName, articles).toLowerCase(); +} + /** - * Bucket an artist name into the alphabet index: + * Bucket an artist name into the alphabet index (after article stripping): * - `#` → starts with a digit (0–9) - * - `A`–`Z` → starts with an ASCII letter + * - `A`–`Z` → starts with an ASCII letter on the sort key * - `OTHER` → anything else (accents, CJK, Cyrillic, symbols, empty) + * + * Buckets always derive from the display `name` + `ignoredArticles`, never the + * persisted `nameSort` (which can lag a renamed artist until the next reconcile). */ -export function artistBucketKey(name: string): string { - const first = name?.trim()?.[0]; +export function artistBucketKey( + name: string, + ignoredArticles?: string | null, +): string { + const sortKey = sortKeyFromDisplayName(name, ignoredArticles); + const first = sortKey?.[0]; if (!first) return OTHER_BUCKET; if (/^[0-9]$/.test(first)) return '#'; const up = first.toUpperCase(); return /^[A-Z]$/.test(up) ? up : OTHER_BUCKET; } +/** Letter bucket for a browse row — uses the server's `ignoredArticles` when known. */ +export function artistLetterBucket( + artist: SubsonicArtist, + ignoredArticles?: string | null, +): string { + return artistBucketKey(artist.name, ignoredArticles); +} + /** Sort comparator for bucket keys following ALPHABET order (unknown keys last). */ export function compareBuckets(a: string, b: string): number { return (BUCKET_ORDER.get(a) ?? 999) - (BUCKET_ORDER.get(b) ?? 999); diff --git a/src/utils/library/advancedSearchLocal.ts b/src/utils/library/advancedSearchLocal.ts index 67f0edda..90ea4304 100644 --- a/src/utils/library/advancedSearchLocal.ts +++ b/src/utils/library/advancedSearchLocal.ts @@ -242,10 +242,17 @@ export function artistToArtist(ar: LibraryArtistDto): SubsonicArtist { const base: SubsonicArtist = { id: ar.id, name: ar.name, + nameSort: ar.nameSort ?? undefined, albumCount: ar.albumCount ?? undefined, coverArt: ar.id, }; - return { ...base, ...(raw as Partial) }; + const merged = mergeArtistRawJson(base, raw as Partial); + return merged; +} + +/** Hot columns from SQLite win over sparse `raw_json` (ADR-7). */ +function mergeArtistRawJson(base: SubsonicArtist, raw: Partial): SubsonicArtist { + return { ...raw, ...base }; } /**