From 9d4227052ffea79d645703125b9f8f71876a36da Mon Sep 17 00:00:00 2001 From: cucadmuh <49571317+cucadmuh@users.noreply.github.com> Date: Fri, 17 Jul 2026 23:17:34 +0300 Subject: [PATCH] fix(library): preserve folder browse identities and durations --- .../psysonic-library/src/identity/norm.rs | 5 +- .../psysonic-library/src/identity/rebuild.rs | 82 ++++++++++- .../psysonic-library/src/scope_merge.rs | 83 ++++++++++- .../crates/psysonic-library/src/store.rs | 135 ++++++++++++++++++ .../psysonic-library/src/sync/mapping.rs | 28 +++- .../components/FolderBrowserColumn.tsx | 15 +- .../hooks/useFolderBrowserKeyboardNav.ts | 8 +- .../hooks/useFolderBrowserNowPlayingPath.ts | 102 ++++++++----- .../hooks/useFolderBrowserScrolling.ts | 15 +- src/features/folderBrowser/index.ts | 5 +- .../pages/FolderBrowser.test.tsx | 95 +++++++++--- .../folderBrowser/pages/FolderBrowser.tsx | 104 ++++++++------ .../utils/folderBrowserHelpers.ts | 65 ++++++++- .../playback/store/playerStore.queue.test.ts | 9 ++ .../timelinePlayHistory.integration.test.ts | 16 +++ src/lib/api/subsonicLibrary.server.test.ts | 17 --- src/lib/api/subsonicLibrary.ts | 20 --- 17 files changed, 634 insertions(+), 170 deletions(-) diff --git a/src-tauri/crates/psysonic-library/src/identity/norm.rs b/src-tauri/crates/psysonic-library/src/identity/norm.rs index 548dd5e1..550be9b9 100644 --- a/src-tauri/crates/psysonic-library/src/identity/norm.rs +++ b/src-tauri/crates/psysonic-library/src/identity/norm.rs @@ -19,9 +19,10 @@ /// Separator for composite keys — U+001F cannot appear in normalized output. pub(crate) const KEY_SEP: char = '\u{001f}'; -/// Bump when normalization rules change; stored in `cluster.cluster_meta.norm_version`. +/// Bump when cluster-key derivation changes; stored in `cluster.cluster_meta.norm_version`. /// v2: locale-aware folding (ß→ss, æ→ae, œ→oe, Romanian ș/ț, Cyrillic ё/й). -pub const NORM_VERSION: &str = "2"; +/// v3: artist keys use the canonical artist entity name when the track has an artist id. +pub const NORM_VERSION: &str = "3"; /// Normalize one identity field. Returns `None` when input is empty/whitespace-only /// or when normalization strips everything (punctuation-only, etc.). diff --git a/src-tauri/crates/psysonic-library/src/identity/rebuild.rs b/src-tauri/crates/psysonic-library/src/identity/rebuild.rs index 579bf23a..1b82d492 100644 --- a/src-tauri/crates/psysonic-library/src/identity/rebuild.rs +++ b/src-tauri/crates/psysonic-library/src/identity/rebuild.rs @@ -8,7 +8,7 @@ use crate::store::LibraryStore; use super::attach::CLUSTER_SCHEMA; use super::keys::build_track_cluster_keys; -use super::norm::NORM_VERSION; +use super::norm::{norm_part, NORM_VERSION}; const UPSERT_CLUSTER_KEY_SQL: &str = " INSERT INTO cluster.track_cluster_key ( @@ -67,6 +67,7 @@ type SourceTrackRow = ( String, String, Option, + Option, String, Option, String, @@ -81,11 +82,14 @@ pub fn rebuild_cluster_keys( store.with_conn_mut("identity.rebuild_cluster_keys", |conn| { let tx = conn.transaction()?; let mut select = String::from( - "SELECT server_id, COALESCE(library_id, ''), id, artist, title, album_artist, album, duration_sec \ - FROM track WHERE deleted = 0", + "SELECT t.server_id, COALESCE(t.library_id, ''), t.id, t.artist, ar.name, t.title, \ + t.album_artist, t.album, t.duration_sec \ + FROM track t \ + LEFT JOIN artist ar ON ar.server_id = t.server_id AND ar.id = t.artist_id \ + WHERE t.deleted = 0", ); if server_id.is_some() { - select.push_str(" AND server_id = ?1"); + select.push_str(" AND t.server_id = ?1"); } // Stream rows straight from the `track` SELECT into the sidecar UPSERT // (both statements borrow the same tx; the SELECT reads `track`, the @@ -98,14 +102,28 @@ pub fn rebuild_cluster_keys( let mut upserted = 0u64; let mut rows = stmt.query(rusqlite::params_from_iter(filter_params.iter()))?; while let Some(row) = rows.next()? { - let (server_id, library_id, track_id, artist, title, album_artist, album, duration_sec) = - map_source_track_row(row)?; - let keys = build_track_cluster_keys( + let ( + server_id, + library_id, + track_id, + artist, + canonical_artist, + title, + album_artist, + album, + duration_sec, + ) = map_source_track_row(row)?; + let mut keys = build_track_cluster_keys( artist.as_deref(), &title, &album, album_artist.as_deref(), ); + keys.artist_key = canonical_artist + .as_deref() + .filter(|name| !name.trim().is_empty()) + .or(artist.as_deref()) + .and_then(norm_part); upsert.execute(params![ server_id, library_id, @@ -199,6 +217,7 @@ fn map_source_track_row(row: &rusqlite::Row<'_>) -> rusqlite::Result Result<(), String> { + let mut seen: Vec<&str> = Vec::new(); + for pair in scopes { + if !seen.contains(&pair.server_id.as_str()) { + seen.push(pair.server_id.as_str()); + crate::identity::ensure_cluster_keys_built(store, &pair.server_id)?; + } + } + Ok(()) +} + pub fn list_albums( store: &LibraryStore, request: &LibraryScopeListRequest, @@ -346,7 +362,7 @@ pub fn list_artists( request: &LibraryScopeListRequest, ) -> Result, String> { let scopes = non_empty_scopes(&request.scopes)?; - ensure_cluster_keys_for_scopes(store, scopes)?; + ensure_artist_cluster_keys_for_scopes(store, scopes)?; let limit = clamp_limit(request.limit); let offset = clamp_offset(request.offset); let order = artist_order_sql(request.sort.as_deref()); @@ -598,6 +614,7 @@ pub(crate) fn list_artists_layer1_filtered( skip_totals: bool, ) -> Result<(Vec, u32), String> { let scopes = non_empty_scopes(scopes)?; + ensure_artist_cluster_keys_for_scopes(store, scopes)?; let (cte, scope_binds) = scope_cte_sql(scopes); let scoped = if scopes.len() == 1 { scoped_track_join_layer1() @@ -697,6 +714,7 @@ pub(crate) fn list_index_artists_layer1_filtered( skip_totals: bool, ) -> Result<(Vec, u32), String> { let scopes = non_empty_scopes(scopes)?; + ensure_artist_cluster_keys_for_scopes(store, scopes)?; let (cte, scope_binds) = scope_cte_sql(scopes); let scoped_from = "FROM scope s \ CROSS JOIN track t ON t.server_id = s.server_id AND t.library_id = s.library_id"; @@ -996,6 +1014,7 @@ pub(crate) fn list_artists_filtered( skip_totals: bool, ) -> Result<(Vec, u32), String> { let scopes = non_empty_scopes(scopes)?; + ensure_artist_cluster_keys_for_scopes(store, scopes)?; let (cte, scope_binds) = scope_cte_sql(scopes); let base_where = append_extra_where( &format!( @@ -1377,6 +1396,7 @@ pub(crate) fn live_search_artists( limit: u32, ) -> Result, String> { let scopes = non_empty_scopes(scopes)?; + ensure_artist_cluster_keys_for_scopes(store, scopes)?; let (cte, mut binds) = scope_cte_sql(scopes); let sql = format!( "{cte}, \ @@ -1871,6 +1891,7 @@ pub fn artist_detail( request: &LibraryScopeArtistDetailRequest, ) -> Result { let scopes = non_empty_scopes(&request.scopes)?; + ensure_artist_cluster_keys_for_scopes(store, scopes)?; let server_id = request.server_id.trim(); let artist_id = request.artist_id.trim(); if server_id.is_empty() || artist_id.is_empty() { @@ -2029,6 +2050,66 @@ mod tests { assert!(response.tracks.is_empty()); } + #[test] + fn list_artists_collapses_collaboration_track_names_for_one_artist_id() { + let store = LibraryStore::open_in_memory(); + TrackRepository::new(&store) + .upsert_batch(&[ + track( + "s1", + "t1", + "Song 1", + Some("Andromida • Daedric"), + "Album 1", + "album-1", + Some("artist-1"), + 200, + "lib-a", + None, + None, + None, + ), + track( + "s1", + "t2", + "Song 2", + Some("Andromida • Nevertel"), + "Album 2", + "album-2", + Some("artist-1"), + 220, + "lib-a", + None, + None, + None, + ), + ]) + .unwrap(); + store + .with_conn_mut("test.canonical_artist_scope", |conn| { + conn.execute( + "INSERT INTO artist (server_id, id, name, synced_at) VALUES ('s1', 'artist-1', 'Andromida', 1)", + [], + )?; + Ok(()) + }) + .unwrap(); + rebuild_cluster_keys(&store, Some("s1")).unwrap(); + + let artists = list_artists( + &store, + &LibraryScopeListRequest { + scopes: vec![scope_pair("s1", "lib-a"), scope_pair("s1", "lib-b")], + sort: Some("name".into()), + limit: Some(50), + offset: Some(0), + }, + ) + .unwrap(); + + assert_eq!(artists.iter().filter(|artist| artist.id == "artist-1").count(), 1); + } + #[test] fn dedup_collapses_same_album_and_priority_winner_flips() { let store = LibraryStore::open_in_memory(); diff --git a/src-tauri/crates/psysonic-library/src/store.rs b/src-tauri/crates/psysonic-library/src/store.rs index 4741e4d8..a96d920b 100644 --- a/src-tauri/crates/psysonic-library/src/store.rs +++ b/src-tauri/crates/psysonic-library/src/store.rs @@ -29,6 +29,11 @@ pub(crate) const LIBRARY_ID_BACKFILL_RECONCILE_ID: &str = "library_id_backfill_r /// prune these inline; this clears already-accumulated rows at first open. pub(crate) const ORPHAN_BROWSE_RECONCILE_ID: &str = "orphan_browse_rows_reconcile_v1"; +/// One-time repair of Navidrome decimal durations stored as zero before the +/// native mapper began rounding them to whole seconds. +pub(crate) const DURATION_SEC_BACKFILL_RECONCILE_ID: &str = "duration_sec_decimal_backfill_v1"; +const DURATION_SEC_BACKFILL_BATCH_SIZE: i64 = 1_000; + /// Lowest applied schema version the current code can advance from purely /// additively. If a DB carries a version below this, the breaking-bump hook /// fires (spec §5.7 / P22): the library is treated as incompatible, must be @@ -920,6 +925,7 @@ fn prepare_write_connection_for_open(conn: &Connection) -> rusqlite::Result<()> maybe_reconcile_artist_name_fold(conn)?; maybe_reconcile_replay_gain_peak(conn)?; maybe_reconcile_library_id_backfill(conn)?; + maybe_reconcile_duration_sec_backfill(conn)?; maybe_reconcile_orphan_browse_rows(conn)?; ensure_genre_tags_schema(conn)?; ensure_mainstage_feed_indexes(conn)?; @@ -1226,6 +1232,78 @@ fn maybe_reconcile_library_id_backfill(conn: &Connection) -> rusqlite::Result<() Ok(()) } +fn duration_sec_backfill_completed(conn: &Connection) -> rusqlite::Result { + let completed: Option> = conn + .query_row( + "SELECT completed_at FROM library_data_migration WHERE id = ?1", + params![DURATION_SEC_BACKFILL_RECONCILE_ID], + |row| row.get(0), + ) + .optional()?; + Ok(completed.flatten().is_some()) +} + +/// Restore zeroed decimal durations from `raw_json` in bounded transactions. +/// `cursor_rowid` lets an interrupted startup continue from the last batch. +fn maybe_reconcile_duration_sec_backfill(conn: &Connection) -> rusqlite::Result<()> { + if duration_sec_backfill_completed(conn)? { + return Ok(()); + } + conn.execute( + "INSERT INTO library_data_migration (id, cursor_rowid, started_at) \ + VALUES (?1, 0, strftime('%s','now')) \ + ON CONFLICT(id) DO UPDATE SET \ + started_at = COALESCE(library_data_migration.started_at, excluded.started_at)", + params![DURATION_SEC_BACKFILL_RECONCILE_ID], + )?; + + loop { + let cursor: i64 = conn.query_row( + "SELECT cursor_rowid FROM library_data_migration WHERE id = ?1", + params![DURATION_SEC_BACKFILL_RECONCILE_ID], + |row| row.get(0), + )?; + let last_rowid: Option = conn.query_row( + "SELECT MAX(rowid) FROM ( \ + SELECT rowid FROM track \ + WHERE rowid > ?1 \ + AND duration_sec = 0 \ + AND json_valid(raw_json) \ + AND json_type(raw_json, '$.duration') IN ('integer', 'real') \ + AND CAST(json_extract(raw_json, '$.duration') AS REAL) > 0 \ + ORDER BY rowid LIMIT ?2 \ + )", + params![cursor, DURATION_SEC_BACKFILL_BATCH_SIZE], + |row| row.get(0), + )?; + let Some(last_rowid) = last_rowid else { + conn.execute( + "UPDATE library_data_migration \ + SET completed_at = strftime('%s','now') WHERE id = ?1", + params![DURATION_SEC_BACKFILL_RECONCILE_ID], + )?; + return Ok(()); + }; + + let tx = conn.unchecked_transaction()?; + tx.execute( + "UPDATE track \ + SET duration_sec = CAST(ROUND(CAST(json_extract(raw_json, '$.duration') AS REAL)) AS INTEGER) \ + WHERE rowid > ?1 AND rowid <= ?2 \ + AND duration_sec = 0 \ + AND json_valid(raw_json) \ + AND json_type(raw_json, '$.duration') IN ('integer', 'real') \ + AND CAST(json_extract(raw_json, '$.duration') AS REAL) > 0", + params![cursor, last_rowid], + )?; + tx.execute( + "UPDATE library_data_migration SET cursor_rowid = ?2 WHERE id = ?1", + params![DURATION_SEC_BACKFILL_RECONCILE_ID, last_rowid], + )?; + tx.commit()?; + } +} + fn orphan_browse_reconcile_completed(conn: &Connection) -> rusqlite::Result { let completed: Option> = conn .query_row( @@ -2103,6 +2181,63 @@ mod tests { assert_eq!(library_id_after, ""); } + #[test] + fn duration_sec_backfill_rounds_decimal_raw_duration_once() { + let store = LibraryStore::open_in_memory(); + store + .with_conn_mut("test.seed_duration_backfill", |conn| { + conn.execute( + "DELETE FROM library_data_migration WHERE id = ?1", + params![DURATION_SEC_BACKFILL_RECONCILE_ID], + )?; + conn.execute( + "INSERT INTO track (server_id, id, title, album, duration_sec, deleted, synced_at, raw_json) \ + VALUES ('s1', 'decimal', 'Decimal', 'Al', 0, 0, 1, '{\"duration\":229.85}')", + [], + )?; + conn.execute( + "INSERT INTO track (server_id, id, title, album, duration_sec, deleted, synced_at, raw_json) \ + VALUES ('s1', 'zero', 'Zero', 'Al', 0, 0, 1, '{\"duration\":0}')", + [], + )?; + conn.execute( + "INSERT INTO track (server_id, id, title, album, duration_sec, deleted, synced_at, raw_json) \ + VALUES ('s1', 'set', 'Set', 'Al', 100, 0, 1, '{\"duration\":200}')", + [], + )?; + Ok(()) + }) + .expect("seed tracks"); + + store + .with_conn("test.duration_backfill", maybe_reconcile_duration_sec_backfill) + .expect("duration backfill"); + + let durations: Vec<(String, i64)> = store + .with_read_conn(|conn| { + conn.prepare("SELECT id, duration_sec FROM track WHERE server_id = 's1' ORDER BY id")? + .query_map([], |row| Ok((row.get(0)?, row.get(1)?)))? + .collect() + }) + .expect("backfilled durations"); + assert_eq!(durations, vec![("decimal".into(), 230), ("set".into(), 100), ("zero".into(), 0)]); + + store + .with_conn_mut("test.clear_decimal_duration", |conn| { + conn.execute("UPDATE track SET duration_sec = 0 WHERE id = 'decimal'", []) + }) + .expect("clear duration"); + store + .with_conn("test.duration_backfill_again", maybe_reconcile_duration_sec_backfill) + .expect("guarded duration backfill"); + let duration_after: i64 = store + .with_read_conn(|conn| { + conn.query_row("SELECT duration_sec FROM track WHERE id = 'decimal'", [], |row| row.get(0)) + }) + .expect("duration after guarded re-run"); + assert_eq!(duration_after, 0); + } + #[test] fn read_conn_recovers_after_closure_panic() { let store = LibraryStore::open_in_memory(); diff --git a/src-tauri/crates/psysonic-library/src/sync/mapping.rs b/src-tauri/crates/psysonic-library/src/sync/mapping.rs index b49cdf36..a4ce9afb 100644 --- a/src-tauri/crates/psysonic-library/src/sync/mapping.rs +++ b/src-tauri/crates/psysonic-library/src/sync/mapping.rs @@ -122,7 +122,7 @@ pub fn navidrome_song_to_track_row( album: string_field(raw, "album").unwrap_or_default(), album_id: string_field(raw, "albumId"), album_artist: string_field(raw, "albumArtist"), - duration_sec: raw.get("duration").and_then(|v| v.as_i64()).unwrap_or(0), + duration_sec: duration_seconds(raw), track_number: raw.get("trackNumber").and_then(|v| v.as_i64()), disc_number: raw.get("discNumber").and_then(|v| v.as_i64()), year: raw.get("year").and_then(|v| v.as_i64()), @@ -167,6 +167,19 @@ fn string_field(raw: &Value, key: &str) -> Option { json_string_field(raw, key) } +/// Navidrome's native API reports seconds as either an integer or a decimal. +/// The local index stores whole seconds, so round rather than silently dropping +/// a valid fractional value to zero. +fn duration_seconds(raw: &Value) -> i64 { + let seconds = raw.get("duration").and_then(Value::as_f64).unwrap_or(0.0); + let rounded = seconds.round(); + if rounded.is_finite() && (0.0..=i64::MAX as f64).contains(&rounded) { + rounded as i64 + } else { + 0 + } +} + fn parse_iso_ms(s: Option<&str>) -> Option { s.and_then(parse_iso_ms_str) } @@ -376,6 +389,19 @@ mod tests { assert_eq!(row.library_id.as_deref(), Some("3")); } + #[test] + fn navidrome_song_rounds_decimal_duration_seconds() { + let raw = json!({ + "id": "tr_1", + "title": "Hello", + "duration": 229.85, + }); + + let row = navidrome_song_to_track_row("s1", &raw, 1, None).unwrap(); + + assert_eq!(row.duration_sec, 230); + } + #[test] fn navidrome_song_skips_rows_without_id() { let row = navidrome_song_to_track_row("s1", &json!({"title": "no id"}), 1, None); diff --git a/src/features/folderBrowser/components/FolderBrowserColumn.tsx b/src/features/folderBrowser/components/FolderBrowserColumn.tsx index c9d4f283..b47ee50c 100644 --- a/src/features/folderBrowser/components/FolderBrowserColumn.tsx +++ b/src/features/folderBrowser/components/FolderBrowserColumn.tsx @@ -4,7 +4,7 @@ import { ChevronRight, Folder, FolderOpen, Music } from 'lucide-react'; import type { SubsonicDirectoryEntry } from '@/lib/api/subsonicTypes'; import type { Track } from '@/lib/media/trackTypes'; import { - folderBrowserHasKeyModifiers, isFolderBrowserArrowKey, + folderBrowserEntryKey, folderBrowserHasKeyModifiers, isFolderBrowserArrowKey, type Column, } from '@/features/folderBrowser/utils/folderBrowserHelpers'; @@ -89,19 +89,22 @@ export default function FolderBrowserColumn({
{t('folderBrowser.empty')}
) : ( filteredItems.map((item, rowIndex) => { - const isSelected = col.selectedId === item.id; + const itemKey = folderBrowserEntryKey(item); + const isSelected = col.selectedKey === itemKey; const isContextRow = contextRowIndex === rowIndex; const isKeyboardRow = keyboardRowIndex === rowIndex; - const isNowPlayingTrack = !item.isDir && currentTrack?.id === item.id; - const isPathPlayingIcon = !!(isSelectedPathForCurrentTrack && playingPathIds.includes(item.id)); + const isNowPlayingTrack = !item.isDir && currentTrack?.id === item.id && ( + !currentTrack.serverId || !item.serverId || currentTrack.serverId === item.serverId + ); + const isPathPlayingIcon = !!(isSelectedPathForCurrentTrack && playingPathIds.includes(itemKey)); return (