diff --git a/src-tauri/crates/psysonic-library/migrations/020_entity_user_rating.sql b/src-tauri/crates/psysonic-library/migrations/020_entity_user_rating.sql new file mode 100644 index 00000000..e4c2143a --- /dev/null +++ b/src-tauri/crates/psysonic-library/migrations/020_entity_user_rating.sql @@ -0,0 +1,10 @@ +-- Owner-scoped ratings for tracks, albums, and artists. The composite primary +-- key covers the batch cache lookups, so a secondary index is unnecessary. +CREATE TABLE IF NOT EXISTS entity_user_rating ( + server_id TEXT NOT NULL, + entity_kind TEXT NOT NULL CHECK (entity_kind IN ('track', 'album', 'artist')), + entity_id TEXT NOT NULL, + rating INTEGER NOT NULL, + fetched_at INTEGER NOT NULL, + PRIMARY KEY (server_id, entity_kind, entity_id) +); diff --git a/src-tauri/crates/psysonic-library/src/commands.rs b/src-tauri/crates/psysonic-library/src/commands.rs index 96ace1e5..2680ed84 100644 --- a/src-tauri/crates/psysonic-library/src/commands.rs +++ b/src-tauri/crates/psysonic-library/src/commands.rs @@ -21,7 +21,8 @@ use crate::cover_resolve::CoverEntryDto; use crate::cross_server; use crate::dto::{ count_local_tracks, local_tracks_max_updated_ms, track_index_nonempty, ArtifactInputDto, - FactInputDto, LibraryAdvancedSearchRequest, LibraryAdvancedSearchResponse, + EntityUserRatingDto, EntityUserRatingRefDto, FactInputDto, LibraryAdvancedSearchRequest, + LibraryAdvancedSearchResponse, LibraryCrossServerSearchResponse, LibraryLiveSearchRequest, LibraryLiveSearchResponse, LibraryMainstageAlbumsRequest, LibraryMainstageAlbumsResponse, LibraryScopeAlbumDetailRequest, LibraryScopeAlbumDetailResponse, LibraryScopeArtistDetailRequest, @@ -61,6 +62,8 @@ where /// Cap for `library_get_tracks_batch` per spec §7.1 ("max 100 refs/call"). const TRACKS_BATCH_LIMIT: usize = 100; +/// Shared cache callers can request or update at most 300 entity ratings per call. +const ENTITY_USER_RATINGS_BATCH_LIMIT: usize = 300; const ANALYSIS_PROGRESS_CACHE_TTL: Duration = Duration::from_secs(30); #[derive(Debug, Clone, serde::Deserialize, specta::Type)] @@ -406,6 +409,42 @@ pub async fn library_get_tracks_batch( hydrate_refs(&runtime, &refs) } +/// Read cached owner-scoped ratings. Invalid keys and cache misses are omitted. +#[tauri::command] +#[specta::specta] +pub async fn library_get_entity_user_ratings( + runtime: State<'_, LibraryRuntime>, + refs: Vec, +) -> Result, String> { + if refs.len() > ENTITY_USER_RATINGS_BATCH_LIMIT { + return Err(format!( + "library_get_entity_user_ratings: refs exceeds cap ({} > {})", + refs.len(), + ENTITY_USER_RATINGS_BATCH_LIMIT + )); + } + let store = runtime.store.clone(); + library_spawn_blocking(move || get_entity_user_ratings(&store, &refs)).await +} + +/// Upsert cached owner-scoped ratings. Invalid keys are ignored. +#[tauri::command] +#[specta::specta] +pub async fn library_put_entity_user_ratings( + runtime: State<'_, LibraryRuntime>, + ratings: Vec, +) -> Result<(), String> { + if ratings.len() > ENTITY_USER_RATINGS_BATCH_LIMIT { + return Err(format!( + "library_put_entity_user_ratings: ratings exceeds cap ({} > {})", + ratings.len(), + ENTITY_USER_RATINGS_BATCH_LIMIT + )); + } + let store = runtime.store.clone(); + library_spawn_blocking(move || put_entity_user_ratings(&store, &ratings, now_unix_ms())).await +} + // NOT specta-collected: returns a DTO carrying `raw_json: Value` (LibraryTrack/Album/ArtistDto) — specta rc.25 can't export serde_json::Value. Stays hand-written on generate_handler!. #[tauri::command] pub async fn library_get_tracks_by_album( @@ -1690,6 +1729,83 @@ fn now_unix_ms() -> i64 { .unwrap_or(0) } +fn valid_entity_user_rating_key(server_id: &str, entity_kind: &str, entity_id: &str) -> bool { + !server_id.is_empty() + && !entity_id.is_empty() + && matches!(entity_kind, "track" | "album" | "artist") +} + +fn get_entity_user_ratings( + store: &LibraryStore, + refs: &[EntityUserRatingRefDto], +) -> Result, String> { + store.with_read_conn(|conn| { + let mut statement = conn.prepare( + "SELECT server_id, entity_kind, entity_id, rating, fetched_at + FROM entity_user_rating + WHERE server_id = ?1 AND entity_kind = ?2 AND entity_id = ?3", + )?; + let mut ratings = Vec::new(); + for reference in refs { + let server_id = reference.server_id.trim(); + let entity_kind = reference.entity_kind.trim(); + let entity_id = reference.entity_id.trim(); + if !valid_entity_user_rating_key(server_id, entity_kind, entity_id) { + continue; + } + if let Some(rating) = statement + .query_row(params![server_id, entity_kind, entity_id], |row| { + Ok(EntityUserRatingDto { + server_id: row.get(0)?, + entity_kind: row.get(1)?, + entity_id: row.get(2)?, + rating: row.get(3)?, + fetched_at: row.get(4)?, + }) + }) + .optional()? + { + ratings.push(rating); + } + } + Ok(ratings) + }) +} + +fn put_entity_user_ratings( + store: &LibraryStore, + ratings: &[EntityUserRatingDto], + now: i64, +) -> Result<(), String> { + store.with_conn_mut("entity_user_rating.upsert_batch", |conn| { + let transaction = conn.transaction()?; + let mut statement = transaction.prepare( + "INSERT INTO entity_user_rating (server_id, entity_kind, entity_id, rating, fetched_at) + VALUES (?1, ?2, ?3, ?4, ?5) + ON CONFLICT(server_id, entity_kind, entity_id) DO UPDATE SET + rating = excluded.rating, + fetched_at = excluded.fetched_at", + )?; + for rating in ratings { + let server_id = rating.server_id.trim(); + let entity_kind = rating.entity_kind.trim(); + let entity_id = rating.entity_id.trim(); + if !valid_entity_user_rating_key(server_id, entity_kind, entity_id) { + continue; + } + statement.execute(params![ + server_id, + entity_kind, + entity_id, + rating.rating, + rating.fetched_at.max(now), + ])?; + } + drop(statement); + transaction.commit() + }) +} + #[derive(serde::Serialize)] #[serde(rename_all = "camelCase")] struct SyncIdleAck { @@ -1905,6 +2021,95 @@ mod tests { assert_eq!(TRACKS_BATCH_LIMIT, 100); } + #[test] + fn entity_user_rating_cache_is_owner_scoped_and_ignores_malformed_keys() { + let store = LibraryStore::open_in_memory(); + let ratings = vec![ + EntityUserRatingDto { + server_id: "s1".into(), + entity_kind: "track".into(), + entity_id: "same-id".into(), + rating: 4, + fetched_at: 10, + }, + EntityUserRatingDto { + server_id: "s2".into(), + entity_kind: "track".into(), + entity_id: "same-id".into(), + rating: 2, + fetched_at: 11, + }, + EntityUserRatingDto { + server_id: "s1".into(), + entity_kind: "invalid".into(), + entity_id: "ignored".into(), + rating: 5, + fetched_at: 12, + }, + ]; + put_entity_user_ratings(&store, &ratings, 100).unwrap(); + + let found = get_entity_user_ratings( + &store, + &[ + EntityUserRatingRefDto { + server_id: "s2".into(), + entity_kind: "track".into(), + entity_id: "same-id".into(), + }, + EntityUserRatingRefDto { + server_id: "s1".into(), + entity_kind: "track".into(), + entity_id: "same-id".into(), + }, + EntityUserRatingRefDto { + server_id: "".into(), + entity_kind: "track".into(), + entity_id: "same-id".into(), + }, + ], + ) + .unwrap(); + assert_eq!(found.len(), 2); + assert_eq!(found[0].rating, 2); + assert_eq!(found[1].rating, 4); + assert!(found.iter().all(|rating| rating.fetched_at >= 100)); + } + + #[test] + fn entity_user_rating_cache_upsert_replaces_existing_owner_key() { + let store = LibraryStore::open_in_memory(); + let rating = EntityUserRatingDto { + server_id: "s1".into(), + entity_kind: "album".into(), + entity_id: "a1".into(), + rating: 3, + fetched_at: 101, + }; + put_entity_user_ratings(&store, std::slice::from_ref(&rating), 100).unwrap(); + let mut updated = rating; + updated.rating = 5; + updated.fetched_at = 200; + put_entity_user_ratings(&store, &[updated], 100).unwrap(); + + let found = get_entity_user_ratings( + &store, + &[EntityUserRatingRefDto { + server_id: "s1".into(), + entity_kind: "album".into(), + entity_id: "a1".into(), + }], + ) + .unwrap(); + assert_eq!(found[0].rating, 5); + assert_eq!(found[0].fetched_at, 200); + } + + #[test] + fn entity_user_rating_batch_limit_matches_spec_cap() { + assert_eq!(ENTITY_USER_RATINGS_BATCH_LIMIT, 300); + } + #[test] fn normalize_base_url_adds_scheme_and_strips_trailing_slash() { assert_eq!(normalize_base_url("nas.example.com"), "http://nas.example.com"); diff --git a/src-tauri/crates/psysonic-library/src/dto.rs b/src-tauri/crates/psysonic-library/src/dto.rs index 330a21f7..90bd4b22 100644 --- a/src-tauri/crates/psysonic-library/src/dto.rs +++ b/src-tauri/crates/psysonic-library/src/dto.rs @@ -235,6 +235,26 @@ pub struct TrackRefDto { pub content_hash: Option, } +/// Owner-scoped cache key for a user rating on a track, album, or artist. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Hash, specta::Type)] +#[serde(rename_all = "camelCase")] +pub struct EntityUserRatingRefDto { + pub server_id: String, + pub entity_kind: String, + pub entity_id: String, +} + +/// Cached user rating together with the time it was fetched from its owner. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, specta::Type)] +#[serde(rename_all = "camelCase")] +pub struct EntityUserRatingDto { + pub server_id: String, + pub entity_kind: String, + pub entity_id: String, + pub rating: i64, + pub fetched_at: i64, +} + /// Input to `library_put_artifact`. Same shape as `TrackArtifactDto` /// minus the server-supplied `server_id` / `track_id` (provided as /// command args) and `fetched_at` (stamped server-side from `now`). diff --git a/src-tauri/crates/psysonic-library/src/lossless_albums.rs b/src-tauri/crates/psysonic-library/src/lossless_albums.rs index 10458f1e..3c157f57 100644 --- a/src-tauri/crates/psysonic-library/src/lossless_albums.rs +++ b/src-tauri/crates/psysonic-library/src/lossless_albums.rs @@ -39,7 +39,6 @@ pub fn list_lossless_albums( store: &LibraryStore, req: &LibraryLosslessAlbumsRequest, ) -> Result { - let total_started_at = Instant::now(); if !crate::dto::track_index_nonempty(store, &req.server_id)? { return Ok(empty_response()); } diff --git a/src-tauri/crates/psysonic-library/src/store.rs b/src-tauri/crates/psysonic-library/src/store.rs index 1714c3ce..fcafb5be 100644 --- a/src-tauri/crates/psysonic-library/src/store.rs +++ b/src-tauri/crates/psysonic-library/src/store.rs @@ -12,7 +12,7 @@ use tauri::Manager; /// /// 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 = 19; +pub const LIBRARY_DB_SCHEMA_VERSION: i64 = 20; /// One-time data repair after migration 014 (`artist.name_sort`). pub(crate) const ARTIST_NAME_SORT_RECONCILE_ID: &str = "artist_name_sort_reconcile_v1"; @@ -63,6 +63,9 @@ pub(crate) const MIGRATION_018_ARTIST_SYNCED_INDEX: &str = /// New Releases reads over one `(server_id, library_id)` scope. pub(crate) const MIGRATION_019_MAINSTAGE_FEED_INDEXES: &str = include_str!("../migrations/019_mainstage_feed_indexes.sql"); +/// Version 20: owner-scoped cache for track, album, and artist user ratings. +pub(crate) const MIGRATION_020_ENTITY_USER_RATING: &str = + include_str!("../migrations/020_entity_user_rating.sql"); /// Embedded migrations. Ordered ascending by `version`; the runner sorts /// defensively before applying so the source order can stay readable. @@ -76,6 +79,7 @@ const MIGRATIONS: &[(i64, &str)] = &[ (17, MIGRATION_017_LIBRARY_TAG_STATE), (18, MIGRATION_018_ARTIST_SYNCED_INDEX), (19, MIGRATION_019_MAINSTAGE_FEED_INDEXES), + (20, MIGRATION_020_ENTITY_USER_RATING), ]; /// Idempotent repair — also runs after the migration runner on every open so @@ -91,6 +95,12 @@ pub(crate) fn ensure_mainstage_feed_indexes(conn: &Connection) -> rusqlite::Resu conn.execute_batch(MIGRATION_019_MAINSTAGE_FEED_INDEXES) } +/// Repairs a rare partial-v20 state where its migration marker was written but +/// the additive cache table did not survive. +pub(crate) fn ensure_entity_user_rating_schema(conn: &Connection) -> rusqlite::Result<()> { + conn.execute_batch(MIGRATION_020_ENTITY_USER_RATING) +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum MigrationOutcome { /// Every missing migration was applied (or the DB was already at head). @@ -671,6 +681,7 @@ fn prepare_write_connection_for_open(conn: &Connection) -> rusqlite::Result<()> maybe_reconcile_orphan_browse_rows(conn)?; ensure_genre_tags_schema(conn)?; ensure_mainstage_feed_indexes(conn)?; + ensure_entity_user_rating_schema(conn)?; checkpoint_wal_conn(conn, "open")?; Ok(()) } @@ -1463,6 +1474,35 @@ mod tests { assert!(sql.contains("server_created_at IS NOT NULL")); } + #[test] + fn migration_020_creates_entity_user_rating_table_idempotently() { + let store = LibraryStore::open_in_memory(); + let version: i64 = store + .with_conn("test.entity_user_rating_version", |conn| { + conn.query_row( + "SELECT version FROM schema_migrations WHERE version = 20", + [], + |row| row.get(0), + ) + }) + .unwrap(); + assert_eq!(version, 20); + + store + .with_conn("test.entity_user_rating_ensure", ensure_entity_user_rating_schema) + .expect("repeated schema repair succeeds"); + let table_count: i64 = store + .with_conn("test.entity_user_rating_table", |conn| { + conn.query_row( + "SELECT COUNT(*) FROM sqlite_master WHERE type = 'table' AND name = 'entity_user_rating'", + [], + |row| row.get(0), + ) + }) + .unwrap(); + assert_eq!(table_count, 1); + } + #[test] fn library_id_backfill_reconcile_populates_from_raw_json() { let store = LibraryStore::open_in_memory(); diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index fa22b7e0..b813af86 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -118,6 +118,7 @@ fn specta_builder() -> tauri_specta::Builder { psysonic_library::commands::library_count_live_tracks, psysonic_library::commands::library_get_status, psysonic_library::commands::library_get_artifact, + psysonic_library::commands::library_get_entity_user_ratings, psysonic_library::commands::library_get_facts, psysonic_library::commands::library_get_offline_path, psysonic_library::commands::library_genre_tags_inspect, @@ -131,6 +132,7 @@ fn specta_builder() -> tauri_specta::Builder { psysonic_library::commands::library_sync_verify_integrity, psysonic_library::commands::library_sync_cancel, psysonic_library::commands::library_put_artifact, + psysonic_library::commands::library_put_entity_user_ratings, psysonic_library::commands::library_put_fact, psysonic_library::commands::library_record_play_session, psysonic_library::commands::library_get_player_stats_year_summary, @@ -1056,6 +1058,7 @@ pub fn run() { psysonic_library::commands::library_get_tracks_by_album, psysonic_library::commands::library_upsert_songs_from_api, psysonic_library::commands::library_get_artifact, + psysonic_library::commands::library_get_entity_user_ratings, psysonic_library::commands::library_get_facts, psysonic_library::commands::library_get_offline_path, psysonic_library::commands::library_analysis_progress, @@ -1073,6 +1076,7 @@ pub fn run() { psysonic_library::browse_support::library_get_catalog_year_bounds, psysonic_library::browse_support::library_get_genre_album_counts, psysonic_library::commands::library_put_artifact, + psysonic_library::commands::library_put_entity_user_ratings, psysonic_library::commands::library_put_fact, psysonic_library::commands::library_record_play_session, psysonic_library::commands::library_get_player_stats_year_summary, diff --git a/src/features/album/pages/AlbumDetail.tsx b/src/features/album/pages/AlbumDetail.tsx index 62e3bfab..8c83c348 100644 --- a/src/features/album/pages/AlbumDetail.tsx +++ b/src/features/album/pages/AlbumDetail.tsx @@ -236,7 +236,7 @@ const handleShuffleAll = () => { } try { - await setRating(albumId, rating); + await setRating(albumId, rating, { serverId, kind: 'album' }); setAlbum(cur => cur && cur.album.id === albumId ? { ...cur, album: { ...cur.album, userRating: rating } } diff --git a/src/features/artist/utils/runArtistDetailActions.ts b/src/features/artist/utils/runArtistDetailActions.ts index 61450860..0c861d49 100644 --- a/src/features/artist/utils/runArtistDetailActions.ts +++ b/src/features/artist/utils/runArtistDetailActions.ts @@ -30,7 +30,7 @@ export async function runArtistEntityRating(deps: RunArtistEntityRatingDeps): Pr if (artistEntityRatingSupport !== 'full') return; try { - await setRating(artistId, rating); + await setRating(artistId, rating, { serverId: artist.serverId ?? activeServerId, kind: 'artist' }); setArtist(a => (a && a.id === artistId ? { ...a, userRating: rating } : a)); } catch (err) { setArtistEntityRating(ratingAtStart); diff --git a/src/features/contextMenu/hooks/useContextMenuRating.ts b/src/features/contextMenu/hooks/useContextMenuRating.ts index 1e54f375..07a85254 100644 --- a/src/features/contextMenu/hooks/useContextMenuRating.ts +++ b/src/features/contextMenu/hooks/useContextMenuRating.ts @@ -39,7 +39,7 @@ export function useContextMenuRating({ const applyAlbumRating = useCallback((album: SubsonicAlbum, rating: number) => { setUserRatingOverride(album.id, rating); if (entityRatingSupport !== 'full') return; - setRating(album.id, rating).catch(err => { + setRating(album.id, rating, { serverId: album.serverId ?? activeServerId ?? undefined, kind: 'album' }).catch(err => { if (activeServerId) setEntityRatingSupport(activeServerId, 'track_only'); showToast( typeof err === 'string' ? err : err instanceof Error ? err.message : t('entityRating.saveFailed'), @@ -52,7 +52,7 @@ export function useContextMenuRating({ const applyArtistRating = useCallback((artist: SubsonicArtist, rating: number) => { setUserRatingOverride(artist.id, rating); if (entityRatingSupport !== 'full') return; - setRating(artist.id, rating).catch(err => { + setRating(artist.id, rating, { serverId: artist.serverId ?? activeServerId ?? undefined, kind: 'artist' }).catch(err => { if (activeServerId) setEntityRatingSupport(activeServerId, 'track_only'); showToast( typeof err === 'string' ? err : err instanceof Error ? err.message : t('entityRating.saveFailed'), diff --git a/src/features/playback/utils/mixRatingFilter.test.ts b/src/features/playback/utils/mixRatingFilter.test.ts index 7c69ff5b..e39b06f9 100644 --- a/src/features/playback/utils/mixRatingFilter.test.ts +++ b/src/features/playback/utils/mixRatingFilter.test.ts @@ -4,20 +4,14 @@ import { usePlayerStore } from '@/features/playback/store/playerStore'; import { resetPlayerStore } from '@/test/helpers/storeReset'; vi.mock('@/lib/api/subsonicRatings', () => ({ - prefetchArtistUserRatings: vi.fn(), - prefetchAlbumUserRatings: vi.fn(), - prefetchArtistUserRatingsForServer: vi.fn(), - prefetchAlbumUserRatingsForServer: vi.fn(), - parseSubsonicEntityStarRating: vi.fn((entity: { userRating?: unknown; rating?: unknown }) => - entity.userRating ?? entity.rating), + entityUserRatingKey: ({ serverId, entityKind, entityId }: { serverId: string; entityKind: string; entityId: string }) => `${serverId}\u0001${entityKind}\u0001${entityId}`, + rememberEntityUserRating: vi.fn(), + resolveEntityUserRatings: vi.fn(), + parseSubsonicEntityStarRating: vi.fn((entity: { userRating?: unknown; rating?: unknown }) => entity.userRating ?? entity.rating), })); +vi.mock('@/store/authStore', () => ({ useAuthStore: { getState: () => ({ activeServerId: 'server-a' }) } })); -import { - prefetchAlbumUserRatings, - prefetchAlbumUserRatingsForServer, - prefetchArtistUserRatings, - prefetchArtistUserRatingsForServer, -} from '@/lib/api/subsonicRatings'; +import { resolveEntityUserRatings } from '@/lib/api/subsonicRatings'; import { enrichSongsForMixRatingFilter, filterAlbumsByMixRatingsAcrossServers, @@ -26,48 +20,25 @@ import { } from '@/features/playback/utils/mixRatingFilter'; const enabledArtist2: { enabled: true; minSong: 0; minAlbum: 0; minArtist: 2 } = { - enabled: true, - minSong: 0, - minAlbum: 0, - minArtist: 2, + enabled: true, minSong: 0, minAlbum: 0, minArtist: 2, }; function song(partial: Partial & Pick): SubsonicSong { - return { - title: 't', - artist: 'A', - album: 'Al', - albumId: 'alb-1', - artistId: 'art-1', - duration: 180, - ...partial, - }; + return { title: 't', artist: 'A', album: 'Al', albumId: 'alb-1', artistId: 'art-1', duration: 180, ...partial }; } -function album( - partial: Pick & { serverId: string }, -): SubsonicAlbum & { serverId: string } { - return { - artist: 'Artist', - songCount: 1, - duration: 180, - ...partial, - }; +function album(partial: Pick & { serverId: string }): SubsonicAlbum & { serverId: string } { + return { artist: 'Artist', songCount: 1, duration: 180, ...partial }; } beforeEach(() => { resetPlayerStore(); - vi.mocked(prefetchArtistUserRatings).mockReset(); - vi.mocked(prefetchAlbumUserRatings).mockReset(); - vi.mocked(prefetchArtistUserRatingsForServer).mockReset(); - vi.mocked(prefetchAlbumUserRatingsForServer).mockReset(); - vi.mocked(prefetchAlbumUserRatings).mockResolvedValue(new Map()); - vi.mocked(prefetchArtistUserRatingsForServer).mockResolvedValue(new Map()); - vi.mocked(prefetchAlbumUserRatingsForServer).mockResolvedValue(new Map()); + vi.mocked(resolveEntityUserRatings).mockReset(); + vi.mocked(resolveEntityUserRatings).mockResolvedValue(new Map()); }); describe('filterAlbumsByMixRatingsAcrossServers', () => { - it('enriches per server and preserves the original merged order', async () => { + it('uses owner-scoped local ratings and preserves merged order', async () => { const config = { enabled: true, minSong: 0, minAlbum: 2, minArtist: 2 }; const albums: Array = [ album({ id: 'shared', name: 'A shared', artistId: 'artist-a', serverId: 'server-a' }), @@ -75,99 +46,62 @@ describe('filterAlbumsByMixRatingsAcrossServers', () => { album({ id: 'keep-a', name: 'A keep', artistId: 'artist-a2', serverId: 'server-a' }), album({ id: 'shared', name: 'B shared', artistId: 'artist-b2', serverId: 'server-b' }), ]; - vi.mocked(prefetchAlbumUserRatingsForServer).mockImplementation(async serverId => ( - serverId === 'server-a' - ? new Map([['shared', 1], ['keep-a', 4]]) - : new Map([['keep-b', 4], ['shared', 5]]) - )); - vi.mocked(prefetchArtistUserRatingsForServer).mockImplementation(async serverId => ( - serverId === 'server-a' - ? new Map([['artist-a', 5], ['artist-a2', 5]]) - : new Map([['artist-b', 5], ['artist-b2', 5]]) - )); + vi.mocked(resolveEntityUserRatings).mockResolvedValue(new Map([ + ['server-a\u0001album\u0001shared', 1], ['server-a\u0001album\u0001keep-a', 4], + ['server-b\u0001album\u0001keep-b', 4], ['server-b\u0001album\u0001shared', 5], + ['server-a\u0001artist\u0001artist-a', 5], ['server-a\u0001artist\u0001artist-a2', 5], + ['server-b\u0001artist\u0001artist-b', 5], ['server-b\u0001artist\u0001artist-b2', 5], + ])); const result = await filterAlbumsByMixRatingsAcrossServers(albums, config); - expect(result.map(album => `${album.serverId}:${album.id}`)).toEqual([ - 'server-b:keep-b', - 'server-a:keep-a', - 'server-b:shared', + expect(result.map(item => `${item.serverId}:${item.id}`)).toEqual([ + 'server-b:keep-b', 'server-a:keep-a', 'server-b:shared', ]); - expect(prefetchAlbumUserRatingsForServer).toHaveBeenCalledWith( - 'server-a', - ['shared', 'keep-a'], - ); - expect(prefetchAlbumUserRatingsForServer).toHaveBeenCalledWith( - 'server-b', - ['keep-b', 'shared'], - ); + expect(resolveEntityUserRatings).toHaveBeenCalledTimes(1); }); }); -describe('passesMixMinRatings — artist axis', () => { - it('excludes when artistUserRating is at or below threshold', () => { +describe('passesMixMinRatings - artist axis', () => { + it('excludes ratings at or below the threshold and keeps unrated artists', () => { expect(passesMixMinRatings(song({ id: '1', artistUserRating: 1 }), enabledArtist2)).toBe(false); expect(passesMixMinRatings(song({ id: '2', artistUserRating: 2 }), enabledArtist2)).toBe(false); expect(passesMixMinRatings(song({ id: '3', artistUserRating: 3 }), enabledArtist2)).toBe(true); + expect(passesMixMinRatings(song({ id: '4' }), enabledArtist2)).toBe(true); }); - it('keeps unrated artists (missing or zero)', () => { - expect(passesMixMinRatings(song({ id: '1' }), enabledArtist2)).toBe(true); - expect(passesMixMinRatings(song({ id: '2', artistUserRating: 0 }), enabledArtist2)).toBe(true); - }); - - it('uses playerStore userRatingOverrides before API fields', () => { + it('uses optimistic overrides before API fields', () => { usePlayerStore.getState().setUserRatingOverride('art-1', 1); - expect( - passesMixMinRatings(song({ id: '1', artistUserRating: 5 }), enabledArtist2), - ).toBe(false); - }); - - it('uses OpenSubsonic artists[] ref when artistUserRating is absent', () => { - const low = song({ - id: '1', - artists: [{ id: 'art-1', userRating: 1 }], - }); - expect(passesMixMinRatings(low, enabledArtist2)).toBe(false); + expect(passesMixMinRatings(song({ id: '1', artistUserRating: 5 }), enabledArtist2)).toBe(false); }); }); describe('enrichSongsForMixRatingFilter', () => { - it('prefetches entity artist rating even when song carries a misleading artists[] ref', async () => { - vi.mocked(prefetchArtistUserRatings).mockResolvedValue(new Map([['art-1', 1]])); + it('uses a local resolved rating when the song payload omits one', async () => { + vi.mocked(resolveEntityUserRatings).mockResolvedValue(new Map([['server-a\u0001artist\u0001art-1', 1]])); + const out = await enrichSongsForMixRatingFilter([song({ id: '1' })], enabledArtist2); - const input = [ - song({ - id: '1', - artists: [{ id: 'art-1', userRating: 5 }], - }), - ]; - const out = await enrichSongsForMixRatingFilter(input, enabledArtist2); - - expect(prefetchArtistUserRatings).toHaveBeenCalledWith(['art-1']); expect(out[0].artistUserRating).toBe(1); expect(passesMixMinRatings(out[0], enabledArtist2)).toBe(false); }); + + it('keeps a payload rating ahead of a local cache value', async () => { + vi.mocked(resolveEntityUserRatings).mockResolvedValue(new Map([['server-a\u0001artist\u0001art-1', 1]])); + const out = await enrichSongsForMixRatingFilter([song({ id: '1', artistUserRating: 5 })], enabledArtist2); + + expect(out[0].artistUserRating).toBe(5); + expect(passesMixMinRatings(out[0], enabledArtist2)).toBe(true); + }); }); describe('filterTopArtistsForMixRatings', () => { - it('drops artists rated at or below the threshold', async () => { - vi.mocked(prefetchArtistUserRatings).mockResolvedValue( - new Map([ - ['a1', 1], - ['a2', 3], - ]), - ); - - const out = await filterTopArtistsForMixRatings( - [ - { id: 'a1', name: 'Low' }, - { id: 'a2', name: 'Ok' }, - { id: 'a3', name: 'Unrated' }, - ], - enabledArtist2, - ); - - expect(out.map(a => a.id)).toEqual(['a2', 'a3']); + it('drops local ratings at or below the threshold', async () => { + vi.mocked(resolveEntityUserRatings).mockResolvedValue(new Map([ + ['server-a\u0001artist\u0001a1', 1], ['server-a\u0001artist\u0001a2', 3], + ])); + const out = await filterTopArtistsForMixRatings([ + { id: 'a1', name: 'Low' }, { id: 'a2', name: 'Ok' }, { id: 'a3', name: 'Unrated' }, + ], enabledArtist2); + expect(out.map(item => item.id)).toEqual(['a2', 'a3']); }); }); diff --git a/src/features/playback/utils/mixRatingFilter.ts b/src/features/playback/utils/mixRatingFilter.ts index 996c882f..b9631b5e 100644 --- a/src/features/playback/utils/mixRatingFilter.ts +++ b/src/features/playback/utils/mixRatingFilter.ts @@ -1,9 +1,9 @@ import { + entityUserRatingKey, + rememberEntityUserRating, parseSubsonicEntityStarRating, - prefetchAlbumUserRatings, - prefetchAlbumUserRatingsForServer, - prefetchArtistUserRatings, - prefetchArtistUserRatingsForServer, + resolveEntityUserRatings, + type EntityUserRatingRef, } from '@/lib/api/subsonicRatings'; import { getRandomSongs } from '@/lib/api/subsonicLibrary'; import type { SubsonicAlbum, SubsonicSong } from '@/lib/api/subsonicTypes'; @@ -198,9 +198,7 @@ export function passesMixMinRatingsForAlbum( return true; } -/** - * Fetches missing entity ratings (bounded concurrency) then filters. Used for random album grids / hero. - */ +/** Resolves cached entity ratings and schedules missing entries for background hydration. */ export async function filterAlbumsByMixRatings( albums: SubsonicAlbum[], c: MixMinRatingsConfig, @@ -208,35 +206,36 @@ export async function filterAlbumsByMixRatings( return filterAlbumsByMixRatingsWithPrefetch( albums, c, - prefetchArtistUserRatings, - prefetchAlbumUserRatings, + useAuthStore.getState().activeServerId, ); } async function filterAlbumsByMixRatingsWithPrefetch( albums: T[], c: MixMinRatingsConfig, - prefetchArtists: (ids: string[]) => Promise>, - prefetchAlbums: (ids: string[]) => Promise>, + serverId: string | null | undefined, ): Promise { - if (!c.enabled) return albums; - if (c.minAlbum <= 0 && c.minArtist <= 0) return albums; + if (!c.enabled || (c.minAlbum <= 0 && c.minArtist <= 0) || !serverId) return albums; const needArtist = c.minArtist > 0; const needAlbum = c.minAlbum > 0; - let byArtist = new Map(); - let byAlbum = new Map(); - if (needArtist) { - const ids = [...new Set(albums.map(a => a.artistId).filter(Boolean))] as string[]; - byArtist = await prefetchArtists(ids); - } - if (needAlbum) { - const ids = [...new Set(albums.filter(a => a.userRating === undefined).map(a => a.id))]; - if (ids.length) byAlbum = await prefetchAlbums(ids); + const refs: EntityUserRatingRef[] = [ + ...(needArtist ? albums.map(a => ({ serverId, entityKind: 'artist' as const, entityId: a.artistId })) : []), + ...(needAlbum ? albums.map(a => ({ serverId, entityKind: 'album' as const, entityId: a.id })) : []), + ]; + const payloadRatingRefs: EntityUserRatingRef[] = []; + for (const album of albums) { + if (needAlbum) { + const ref = { serverId, entityKind: 'album' as const, entityId: album.id }; + const rating = parseSubsonicEntityStarRating(album); + rememberEntityUserRating(ref, rating); + if (rating !== undefined) payloadRatingRefs.push(ref); + } } + const ratings = await resolveEntityUserRatings(refs, payloadRatingRefs); return albums.filter(a => passesMixMinRatingsForAlbum(a, c, { - artistUserRating: a.artistId ? byArtist.get(a.artistId) : undefined, - albumUserRating: byAlbum.get(a.id), + artistUserRating: ratings.get(entityUserRatingKey({ serverId, entityKind: 'artist', entityId: a.artistId })), + albumUserRating: ratings.get(entityUserRatingKey({ serverId, entityKind: 'album', entityId: a.id })), }), ); } @@ -247,23 +246,25 @@ export async function filterAlbumsByMixRatingsAcrossServers { if (!c.enabled || (c.minAlbum <= 0 && c.minArtist <= 0)) return albums; - const byServer = new Map(); + const refs: EntityUserRatingRef[] = []; for (const album of albums) { - const group = byServer.get(album.serverId); - if (group) group.push(album); - else byServer.set(album.serverId, [album]); + if (c.minArtist > 0) refs.push({ serverId: album.serverId, entityKind: 'artist', entityId: album.artistId }); + if (c.minAlbum > 0) refs.push({ serverId: album.serverId, entityKind: 'album', entityId: album.id }); } - const accepted = new Set(); - await Promise.all([...byServer].map(async ([serverId, group]) => { - const filtered = await filterAlbumsByMixRatingsWithPrefetch( - group, - c, - ids => prefetchArtistUserRatingsForServer(serverId, ids), - ids => prefetchAlbumUserRatingsForServer(serverId, ids), - ); - for (const album of filtered) accepted.add(album); + const payloadRatingRefs: EntityUserRatingRef[] = []; + for (const album of albums) { + if (c.minAlbum > 0) { + const ref = { serverId: album.serverId, entityKind: 'album' as const, entityId: album.id }; + const rating = parseSubsonicEntityStarRating(album); + rememberEntityUserRating(ref, rating); + if (rating !== undefined) payloadRatingRefs.push(ref); + } + } + const ratings = await resolveEntityUserRatings(refs, payloadRatingRefs); + return albums.filter(album => passesMixMinRatingsForAlbum(album, c, { + artistUserRating: ratings.get(entityUserRatingKey({ serverId: album.serverId, entityKind: 'artist', entityId: album.artistId })), + albumUserRating: ratings.get(entityUserRatingKey({ serverId: album.serverId, entityKind: 'album', entityId: album.id })), })); - return albums.filter(album => accepted.has(album)); } /** Enrich when needed, then drop songs excluded by Settings → Ratings → filter-by-rating. */ @@ -276,19 +277,18 @@ export async function filterSongsForLuckyMixRatings( return enriched.filter(s => passesMixMinRatings(s, c)); } -/** - * Merge `getArtist` / `getAlbum` ratings into songs when list payloads omit them, - * so `passesMixMinRatings` / Lucky Mix filtering see album and artist stars. - */ +/** Merge local ratings only where song payloads and optimistic overrides provide no rating. */ /** Drop low-rated seed artists before Lucky Mix picks from listening history. */ export async function filterTopArtistsForMixRatings( artists: T[], c: MixMinRatingsConfig, ): Promise { if (!c.enabled || c.minArtist <= 0 || !artists.length) return artists; - const byArtist = await prefetchArtistUserRatings(artists.map(a => a.id)); + const serverId = useAuthStore.getState().activeServerId; + if (!serverId) return artists; + const ratings = await resolveEntityUserRatings(artists.map(a => ({ serverId, entityKind: 'artist' as const, entityId: a.id }))); return artists.filter(a => { - const r = mixRatingOverrideForEntity(a.id) ?? byArtist.get(a.id); + const r = mixRatingOverrideForEntity(a.id) ?? ratings.get(entityUserRatingKey({ serverId, entityKind: 'artist', entityId: a.id })); if (r === undefined || r <= 0) return true; return r > c.minArtist; }); @@ -299,6 +299,8 @@ export async function enrichSongsForMixRatingFilter( c: MixMinRatingsConfig, ): Promise { if (!c.enabled || (c.minArtist <= 0 && c.minAlbum <= 0)) return songs; + const serverId = useAuthStore.getState().activeServerId; + if (!serverId) return songs; const artistIds = c.minArtist > 0 ? [...new Set(songs.map(s => artistEntityIdForMixRating(s)).filter((id): id is string => !!id))] @@ -307,17 +309,25 @@ export async function enrichSongsForMixRatingFilter( c.minAlbum > 0 ? [...new Set(songs.filter(s => s.albumId).map(s => s.albumId!))] : []; - const [byArtist, byAlbum] = await Promise.all([ - artistIds.length ? prefetchArtistUserRatings(artistIds) : Promise.resolve(new Map()), - albumIds.length ? prefetchAlbumUserRatings(albumIds) : Promise.resolve(new Map()), + const ratings = await resolveEntityUserRatings([ + ...artistIds.map(entityId => ({ serverId, entityKind: 'artist' as const, entityId })), + ...albumIds.map(entityId => ({ serverId, entityKind: 'album' as const, entityId })), ]); - if (!byArtist.size && !byAlbum.size) return songs; + if (!ratings.size) return songs; return songs.map(s => { const aid = artistEntityIdForMixRating(s); + const payloadArtistRating = effectiveArtistRatingForFilter(s); + const payloadAlbumRating = effectiveAlbumRatingOnSong(s); + const artistRating = aid + ? ratings.get(entityUserRatingKey({ serverId, entityKind: 'artist', entityId: aid })) + : undefined; + const albumRating = s.albumId + ? ratings.get(entityUserRatingKey({ serverId, entityKind: 'album', entityId: s.albumId })) + : undefined; const artistPatch = - aid && byArtist.has(aid) ? { artistUserRating: byArtist.get(aid)! } : {}; + payloadArtistRating === undefined && artistRating !== undefined ? { artistUserRating: artistRating } : {}; const albumPatch = - s.albumId && byAlbum.has(s.albumId) ? { albumUserRating: byAlbum.get(s.albumId)! } : {}; + payloadAlbumRating === undefined && albumRating !== undefined ? { albumUserRating: albumRating } : {}; return { ...s, ...artistPatch, ...albumPatch }; }); } diff --git a/src/generated/bindings.ts b/src/generated/bindings.ts index 7d8b05e7..324f891c 100644 --- a/src/generated/bindings.ts +++ b/src/generated/bindings.ts @@ -39,6 +39,8 @@ export const commands = { fetchedAt: number, expiresAt: number | null, } | null, string>(__TAURI_INVOKE("library_get_artifact", { serverId, trackId, artifactKind, sourceKind, sourceId, format })), + /** Read cached owner-scoped ratings. Invalid keys and cache misses are omitted. */ + libraryGetEntityUserRatings: (refs: EntityUserRatingRefDto[]) => typedError(__TAURI_INVOKE("library_get_entity_user_ratings", { refs })), libraryGetFacts: (serverId: string, trackId: string, factKinds: string[] | null) => typedError(__TAURI_INVOKE("library_get_facts", { serverId, trackId, factKinds })), libraryGetOfflinePath: (serverId: string, trackId: string) => typedError(__TAURI_INVOKE("library_get_offline_path", { serverId, trackId })), libraryGenreTagsInspect: () => typedError(__TAURI_INVOKE("library_genre_tags_inspect")), @@ -61,6 +63,8 @@ export const commands = { librarySyncVerifyIntegrity: (serverId: string, libraryScope: string | null) => typedError(__TAURI_INVOKE("library_sync_verify_integrity", { serverId, libraryScope })), librarySyncCancel: (jobId: string | null) => typedError(__TAURI_INVOKE("library_sync_cancel", { jobId })), libraryPutArtifact: (serverId: string, trackId: string, artifact: ArtifactInputDto) => typedError(__TAURI_INVOKE("library_put_artifact", { serverId, trackId, artifact })), + /** Upsert cached owner-scoped ratings. Invalid keys are ignored. */ + libraryPutEntityUserRatings: (ratings: EntityUserRatingDto[]) => typedError(__TAURI_INVOKE("library_put_entity_user_ratings", { ratings })), libraryPutFact: (serverId: string, trackId: string, fact: FactInputDto) => typedError(__TAURI_INVOKE("library_put_fact", { serverId, trackId, fact })), libraryRecordPlaySession: (input: PlaySessionInputDto) => typedError(__TAURI_INVOKE("library_record_play_session", { input })), libraryGetPlayerStatsYearSummary: (year: number) => typedError(__TAURI_INVOKE("library_get_player_stats_year_summary", { year })), @@ -944,6 +948,22 @@ export type CustomHeadersApplyTo = "local" | "public" | "both"; export type EndpointKind = "local" | "public"; +/** Cached user rating together with the time it was fetched from its owner. */ +export type EntityUserRatingDto = { + serverId: string, + entityKind: string, + entityId: string, + rating: number, + fetchedAt: number, +}; + +/** Owner-scoped cache key for a user rating on a track, album, or artist. */ +export type EntityUserRatingRefDto = { + serverId: string, + entityKind: string, + entityId: string, +}; + /** * Input to `library_put_fact`. Shape matches `TrackFactDto` minus the * indices. diff --git a/src/lib/api/subsonicRatings.test.ts b/src/lib/api/subsonicRatings.test.ts index 8f0617d7..ac1a26fa 100644 --- a/src/lib/api/subsonicRatings.test.ts +++ b/src/lib/api/subsonicRatings.test.ts @@ -1,99 +1,59 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; -vi.mock('@/lib/api/subsonicArtists', () => ({ getArtist: vi.fn(), getArtistForServer: vi.fn() })); -vi.mock('@/lib/api/subsonicLibrary', () => ({ getAlbum: vi.fn(), getAlbumForServer: vi.fn() })); -vi.mock('@/lib/network/subsonicNetworkGuard', () => ({ - shouldAttemptSubsonicForActiveServer: vi.fn(() => true), - shouldAttemptSubsonicForServer: vi.fn(() => true), +const mocks = vi.hoisted(() => ({ invoke: vi.fn() })); +vi.mock('@tauri-apps/api/core', () => ({ invoke: mocks.invoke })); +vi.mock('@/lib/api/subsonicArtists', () => ({ getArtistForServer: vi.fn() })); +vi.mock('@/lib/api/subsonicLibrary', () => ({ getAlbumForServer: vi.fn() })); +vi.mock('@/lib/network/subsonicNetworkGuard', () => ({ shouldAttemptSubsonicForServer: vi.fn(() => true) })); +vi.mock('@/lib/api/library/internal', () => ({ + serverIndexKeyForId: (serverId: string) => `index:${serverId}`, + mapServerIdFromIndexKey: (serverId: string) => serverId.replace('index:', ''), })); -import { getArtist, getArtistForServer } from '@/lib/api/subsonicArtists'; -import { getAlbumForServer } from '@/lib/api/subsonicLibrary'; -import { - invalidateEntityUserRatingCaches, - prefetchAlbumUserRatingsForServer, - prefetchArtistUserRatings, - prefetchArtistUserRatingsForServer, -} from '@/lib/api/subsonicRatings'; +vi.mock('@/store/authStore', () => ({ useAuthStore: { getState: () => ({ activeServerId: 'server-a' }) } })); + +import { getArtistForServer } from '@/lib/api/subsonicArtists'; +import { entityUserRatingKey, resolveEntityUserRatings } from '@/lib/api/subsonicRatings'; beforeEach(() => { - vi.mocked(getArtist).mockReset(); + mocks.invoke.mockReset(); vi.mocked(getArtistForServer).mockReset(); - vi.mocked(getAlbumForServer).mockReset(); - invalidateEntityUserRatingCaches('art-1'); - invalidateEntityUserRatingCaches('shared-id'); }); -describe('explicit-server rating prefetch', () => { - it('isolates same artist ids by server', async () => { - vi.mocked(getArtistForServer).mockImplementation(async serverId => ({ - artist: { id: 'shared-id', name: 'Artist', userRating: serverId === 'server-a' ? 1 : 5 }, - albums: [], - })); +describe('resolveEntityUserRatings', () => { + it('keeps identical ids isolated by server and entity kind', async () => { + mocks.invoke.mockResolvedValue([ + { serverId: 'index:server-a', entityKind: 'artist', entityId: 'shared', rating: 1, fetchedAt: 1 }, + { serverId: 'index:server-b', entityKind: 'artist', entityId: 'shared', rating: 5, fetchedAt: 1 }, + { serverId: 'index:server-a', entityKind: 'album', entityId: 'shared', rating: 3, fetchedAt: 1 }, + ]); - const fromA = await prefetchArtistUserRatingsForServer('server-a', ['shared-id']); - const fromB = await prefetchArtistUserRatingsForServer('server-b', ['shared-id']); - const cachedA = await prefetchArtistUserRatingsForServer('server-a', ['shared-id']); + const ratings = await resolveEntityUserRatings([ + { serverId: 'server-a', entityKind: 'artist', entityId: 'shared' }, + { serverId: 'server-b', entityKind: 'artist', entityId: 'shared' }, + { serverId: 'server-a', entityKind: 'album', entityId: 'shared' }, + ]); - expect(fromA.get('shared-id')).toBe(1); - expect(fromB.get('shared-id')).toBe(5); - expect(cachedA.get('shared-id')).toBe(1); - expect(getArtistForServer).toHaveBeenCalledTimes(2); + expect(ratings.get(entityUserRatingKey({ serverId: 'server-a', entityKind: 'artist', entityId: 'shared' }))).toBe(1); + expect(ratings.get(entityUserRatingKey({ serverId: 'server-b', entityKind: 'artist', entityId: 'shared' }))).toBe(5); + expect(ratings.get(entityUserRatingKey({ serverId: 'server-a', entityKind: 'album', entityId: 'shared' }))).toBe(3); + expect(getArtistForServer).not.toHaveBeenCalled(); }); - it('uses the explicit server for album fetches', async () => { - vi.mocked(getAlbumForServer).mockResolvedValue({ - album: { - id: 'shared-id', - name: 'Album', - artist: 'Artist', - artistId: 'artist-id', - songCount: 1, - duration: 180, - userRating: 3, - }, - songs: [], + it('returns after the local read while missing ratings hydrate in the background', async () => { + let releaseNetwork!: () => void; + const network = new Promise<{ artist: { id: string; name: string; userRating: number }; albums: [] }>(resolve => { + releaseNetwork = () => resolve({ artist: { id: 'artist-1', name: 'Artist', userRating: 1 }, albums: [] }); }); + mocks.invoke.mockResolvedValue([]); + vi.mocked(getArtistForServer).mockReturnValue(network); - const ratings = await prefetchAlbumUserRatingsForServer('server-b', ['shared-id']); + const ratings = await resolveEntityUserRatings([ + { serverId: 'server-a', entityKind: 'artist', entityId: 'artist-1' }, + ]); - expect(ratings.get('shared-id')).toBe(3); - expect(getAlbumForServer).toHaveBeenCalledWith('server-b', 'shared-id'); - }); -}); - -describe('prefetchArtistUserRatings', () => { - it('does not negative-cache unrated artists', async () => { - vi.mocked(getArtist).mockResolvedValue({ artist: { id: 'art-1', name: 'Artist' }, albums: [] }); - - const first = await prefetchArtistUserRatings(['art-1']); - expect(first.size).toBe(0); - expect(getArtist).toHaveBeenCalledTimes(1); - - vi.mocked(getArtist).mockResolvedValue({ - artist: { id: 'art-1', name: 'Artist', userRating: 1 }, - albums: [], - }); - const second = await prefetchArtistUserRatings(['art-1']); - expect(second.get('art-1')).toBe(1); - expect(getArtist).toHaveBeenCalledTimes(2); - }); - - it('does not reuse ratings across active-server calls', async () => { - vi.mocked(getArtist).mockResolvedValue({ - artist: { id: 'art-1', name: 'Artist', userRating: 2 }, - albums: [], - }); - - const first = await prefetchArtistUserRatings(['art-1']); - expect(first.get('art-1')).toBe(2); - expect(getArtist).toHaveBeenCalledTimes(1); - - vi.mocked(getArtist).mockResolvedValue({ - artist: { id: 'art-1', name: 'Artist', userRating: 4 }, - albums: [], - }); - const fresh = await prefetchArtistUserRatings(['art-1']); - expect(fresh.get('art-1')).toBe(4); - expect(getArtist).toHaveBeenCalledTimes(2); + expect(ratings.size).toBe(0); + expect(getArtistForServer).toHaveBeenCalledWith('server-a', 'artist-1'); + expect(mocks.invoke).toHaveBeenCalledTimes(1); + releaseNetwork(); }); }); diff --git a/src/lib/api/subsonicRatings.ts b/src/lib/api/subsonicRatings.ts index 0aeafdeb..86f849aa 100644 --- a/src/lib/api/subsonicRatings.ts +++ b/src/lib/api/subsonicRatings.ts @@ -1,37 +1,42 @@ -import { getArtist, getArtistForServer } from '@/lib/api/subsonicArtists'; -import { getAlbum, getAlbumForServer } from '@/lib/api/subsonicLibrary'; -import { - shouldAttemptSubsonicForActiveServer, - shouldAttemptSubsonicForServer, -} from '@/lib/network/subsonicNetworkGuard'; +import { invoke } from '@tauri-apps/api/core'; +import { getArtistForServer } from '@/lib/api/subsonicArtists'; +import { getAlbumForServer } from '@/lib/api/subsonicLibrary'; +import { mapServerIdFromIndexKey, serverIndexKeyForId } from '@/lib/api/library/internal'; +import { shouldAttemptSubsonicForServer } from '@/lib/network/subsonicNetworkGuard'; +import { useAuthStore } from '@/store/authStore'; const MIX_RATING_PREFETCH_CONCURRENCY = 8; -const RATING_CACHE_TTL = 7 * 60 * 1000; // 7 minutes -const ratingCache = new Map(); +const ENTITY_RATING_BATCH_LIMIT = 300; -function getCachedRating(key: string): number | null { - const entry = ratingCache.get(key); - if (!entry) return null; // cache miss - if (Date.now() > entry.expiresAt) { ratingCache.delete(key); return null; } - return entry.value; +export type EntityRatingKind = 'track' | 'album' | 'artist'; + +export interface EntityUserRatingRef { + serverId: string; + entityKind: EntityRatingKind; + entityId: string; } -function setCachedRating(key: string, value: number): void { - ratingCache.set(key, { value, expiresAt: Date.now() + RATING_CACHE_TTL }); +interface EntityUserRatingDto extends EntityUserRatingRef { + rating: number; + fetchedAt: number; } -/** Drop cached entity ratings after `setRating` so mixes see fresh stars. */ -export function invalidateEntityUserRatingCaches(id: string): void { - for (const key of ratingCache.keys()) { - if (key.endsWith(`:${id}`)) ratingCache.delete(key); +export function entityUserRatingKey({ serverId, entityKind, entityId }: EntityUserRatingRef): string { + return `${serverId}\u0001${entityKind}\u0001${entityId}`; +} + +function validRefs(refs: EntityUserRatingRef[]): EntityUserRatingRef[] { + const unique = new Map(); + for (const ref of refs) { + if (ref.serverId && ref.entityId) unique.set(entityUserRatingKey(ref), ref); } + return [...unique.values()]; } -function parseEntityUserRating(v: unknown): number | undefined { - if (v === null || v === undefined) return undefined; - const n = typeof v === 'number' ? v : Number(v); - if (!Number.isFinite(n)) return undefined; - return n; +function chunks(values: T[], size: number): T[][] { + const out: T[][] = []; + for (let index = 0; index < values.length; index += size) out.push(values.slice(index, index + size)); + return out; } /** Navidrome and some JSON shapes use `rating` where Subsonic docs say `userRating`. */ @@ -39,138 +44,136 @@ export function parseSubsonicEntityStarRating(entity: { userRating?: unknown; rating?: unknown; }): number | undefined { - return parseEntityUserRating(entity.userRating ?? entity.rating); + const value = entity.userRating ?? entity.rating; + if (value === null || value === undefined) return undefined; + const rating = typeof value === 'number' ? value : Number(value); + return Number.isFinite(rating) ? rating : undefined; } -/** Bump when rating parse keys change so stale cache entries are not reused. */ -const ENTITY_RATING_CACHE_KEY_VER = 'v2'; - -type RatingEntity = 'artist' | 'album'; - -function ratingCacheKey(entity: RatingEntity, serverId: string, id: string): string { - return `${entity}:${ENTITY_RATING_CACHE_KEY_VER}:${serverId}:${id}`; -} - -async function prefetchUserRatings( - entity: RatingEntity, - serverId: string, - ids: string[], - fetchEntity: (id: string) => Promise<{ userRating?: unknown; rating?: unknown }>, - concurrency: number, - networkAllowed: boolean, -): Promise> { - const unique = [...new Set(ids.filter(Boolean))]; - const out = new Map(); - if (!unique.length) return out; - const uncached: string[] = []; - for (const id of unique) { - const cached = getCachedRating(ratingCacheKey(entity, serverId, id)); - if (cached !== null) out.set(id, cached); - else uncached.push(id); +/** Read cached owner-scoped ratings. The index uses server keys; callers use saved-server ids. */ +export async function getLocalEntityUserRatings(refs: EntityUserRatingRef[]): Promise> { + const unique = validRefs(refs); + if (!unique.length) return new Map(); + const requestedByIndexKey = new Map(); + for (const ref of unique) { + requestedByIndexKey.set(entityUserRatingKey({ ...ref, serverId: serverIndexKeyForId(ref.serverId) }), ref); } - if (!uncached.length) return out; - if (!networkAllowed) return out; - let next = 0; - async function worker() { - for (;;) { - const i = next++; - if (i >= uncached.length) return; - const id = uncached[i]; - try { - const r = parseSubsonicEntityStarRating(await fetchEntity(id)); - if (r !== undefined && r > 0) { - setCachedRating(ratingCacheKey(entity, serverId, id), r); - out.set(id, r); - } - } catch { - /* ignore */ - } + try { + const responses = await Promise.all(chunks(unique, ENTITY_RATING_BATCH_LIMIT).map(batch => + invoke('library_get_entity_user_ratings', { + refs: batch.map(ref => ({ ...ref, serverId: serverIndexKeyForId(ref.serverId) })), + }), + )); + const out = new Map(); + for (const response of responses.flat()) { + const requested = requestedByIndexKey.get(entityUserRatingKey(response)); + const serverId = requested?.serverId ?? mapServerIdFromIndexKey(response.serverId); + out.set(entityUserRatingKey({ ...response, serverId }), response.rating); } + return out; + } catch { + return new Map(); } - const nWorkers = Math.min(concurrency, uncached.length); - await Promise.all(Array.from({ length: nWorkers }, () => worker())); - return out; } -async function prefetchActiveServerUserRatings( - ids: string[], - fetchEntity: (id: string) => Promise<{ userRating?: unknown; rating?: unknown }>, - concurrency: number, -): Promise> { - const unique = [...new Set(ids.filter(Boolean))]; - const out = new Map(); - if (!unique.length || !shouldAttemptSubsonicForActiveServer()) return out; - let next = 0; - async function worker() { - for (;;) { - const i = next++; - if (i >= unique.length) return; - const id = unique[i]; +/** Write a known owner-scoped rating without waiting for a full library sync. */ +export function putLocalEntityUserRatings(ratings: Array): void { + const valid = ratings.filter(rating => + rating.serverId && rating.entityId && Number.isFinite(rating.rating), + ); + for (const batch of chunks(valid, ENTITY_RATING_BATCH_LIMIT)) { + void invoke('library_put_entity_user_ratings', { + ratings: batch.map(rating => ({ ...rating, serverId: serverIndexKeyForId(rating.serverId), fetchedAt: 0 })), + }).catch(() => {}); + } +} + +const hydrationQueued = new Set(); +const hydrationQueue: EntityUserRatingRef[] = []; +let activeHydrations = 0; + +function scheduleEntityRatingHydration(refs: EntityUserRatingRef[]): void { + for (const ref of validRefs(refs)) { + const key = entityUserRatingKey(ref); + if (ref.entityKind === 'track' || hydrationQueued.has(key) || !shouldAttemptSubsonicForServer(ref.serverId)) continue; + hydrationQueued.add(key); + hydrationQueue.push(ref); + } + while (activeHydrations < MIX_RATING_PREFETCH_CONCURRENCY && hydrationQueue.length) { + const ref = hydrationQueue.shift()!; + activeHydrations++; + void (async () => { try { - const r = parseSubsonicEntityStarRating(await fetchEntity(id)); - if (r !== undefined && r > 0) out.set(id, r); + const entity = ref.entityKind === 'artist' + ? (await getArtistForServer(ref.serverId, ref.entityId)).artist + : (await getAlbumForServer(ref.serverId, ref.entityId)).album; + const rating = parseSubsonicEntityStarRating(entity); + if (rating !== undefined) putLocalEntityUserRatings([{ ...ref, rating }]); } catch { - /* ignore */ + // A later list pass may retry transient server failures. + } finally { + hydrationQueued.delete(entityUserRatingKey(ref)); + activeHydrations--; + scheduleEntityRatingHydration([]); } - } + })(); } - const nWorkers = Math.min(concurrency, unique.length); - await Promise.all(Array.from({ length: nWorkers }, () => worker())); - return out; } -/** Parallel `getArtist` calls to fill mix/album filters when list endpoints omit ratings. */ -export async function prefetchArtistUserRatings( - ids: string[], - concurrency = MIX_RATING_PREFETCH_CONCURRENCY, +/** Resolve local ratings, then schedule non-blocking hydration for missing albums and artists. */ +export async function resolveEntityUserRatings( + refs: EntityUserRatingRef[], + knownRefs: EntityUserRatingRef[] = [], ): Promise> { - return prefetchActiveServerUserRatings( - ids, - async id => (await getArtist(id)).artist, - concurrency, - ); + const local = await getLocalEntityUserRatings(refs); + const knownKeys = new Set(validRefs(knownRefs).map(entityUserRatingKey)); + scheduleEntityRatingHydration(validRefs(refs).filter(ref => ( + !local.has(entityUserRatingKey(ref)) && !knownKeys.has(entityUserRatingKey(ref)) + ))); + return local; } -/** Explicit-server variant for merged multi-server browse results. */ -export async function prefetchArtistUserRatingsForServer( - serverId: string, - ids: string[], - concurrency = MIX_RATING_PREFETCH_CONCURRENCY, -): Promise> { - return prefetchUserRatings( - 'artist', - serverId, - ids, - async id => (await getArtistForServer(serverId, id)).artist, - concurrency, - shouldAttemptSubsonicForServer(serverId), - ); +/** Persist ratings already supplied by a list/detail payload for the next local-first pass. */ +export function rememberEntityUserRating( + ref: EntityUserRatingRef, + payloadRating: unknown, +): void { + const rating = parseSubsonicEntityStarRating({ userRating: payloadRating }); + if (rating !== undefined) putLocalEntityUserRatings([{ ...ref, rating }]); } -/** Parallel `getAlbum` calls when `albumList2` entries lack `userRating`. */ -export async function prefetchAlbumUserRatings( +/** Legacy prefetch APIs now return local hits and schedule, rather than await, hydration. */ +async function prefetchForServer( + entityKind: 'artist' | 'album', + serverId: string | null | undefined, ids: string[], - concurrency = MIX_RATING_PREFETCH_CONCURRENCY, ): Promise> { - return prefetchActiveServerUserRatings( - ids, - async id => (await getAlbum(id)).album, - concurrency, - ); + if (!serverId) return new Map(); + const refs = ids.map(entityId => ({ serverId, entityKind, entityId })); + const ratings = await resolveEntityUserRatings(refs); + const byId = new Map(); + for (const ref of refs) { + const rating = ratings.get(entityUserRatingKey(ref)); + if (rating !== undefined) byId.set(ref.entityId, rating); + } + return byId; } -/** Explicit-server variant for merged multi-server browse results. */ -export async function prefetchAlbumUserRatingsForServer( - serverId: string, - ids: string[], - concurrency = MIX_RATING_PREFETCH_CONCURRENCY, -): Promise> { - return prefetchUserRatings( - 'album', - serverId, - ids, - async id => (await getAlbumForServer(serverId, id)).album, - concurrency, - shouldAttemptSubsonicForServer(serverId), - ); +export function prefetchArtistUserRatings(ids: string[], _concurrency = MIX_RATING_PREFETCH_CONCURRENCY): Promise> { + return prefetchForServer('artist', useAuthStore.getState().activeServerId, ids); } + +export function prefetchArtistUserRatingsForServer(serverId: string, ids: string[], _concurrency = MIX_RATING_PREFETCH_CONCURRENCY): Promise> { + return prefetchForServer('artist', serverId, ids); +} + +export function prefetchAlbumUserRatings(ids: string[], _concurrency = MIX_RATING_PREFETCH_CONCURRENCY): Promise> { + return prefetchForServer('album', useAuthStore.getState().activeServerId, ids); +} + +export function prefetchAlbumUserRatingsForServer(serverId: string, ids: string[], _concurrency = MIX_RATING_PREFETCH_CONCURRENCY): Promise> { + return prefetchForServer('album', serverId, ids); +} + +/** Kept for compatibility with prior callers that invalidated the frontend TTL cache. */ +export function invalidateEntityUserRatingCaches(_id: string): void {} diff --git a/src/lib/api/subsonicStarRating.ts b/src/lib/api/subsonicStarRating.ts index 9970a951..ecfd8b9c 100644 --- a/src/lib/api/subsonicStarRating.ts +++ b/src/lib/api/subsonicStarRating.ts @@ -1,5 +1,5 @@ import { api, apiForServer, libraryFilterParams, libraryFilterParamsForServer } from '@/lib/api/subsonicClient'; -import { invalidateEntityUserRatingCaches } from '@/lib/api/subsonicRatings'; +import { putLocalEntityUserRatings, type EntityRatingKind } from '@/lib/api/subsonicRatings'; import { useAuthStore } from '@/store/authStore'; import { patchLibraryAlbumOnUse, patchLibraryTrackOnUse, type StarPatchMeta } from '@/lib/library/patchOnUse'; import { useLibraryIndexStore } from '@/store/libraryIndexStore'; @@ -115,17 +115,24 @@ export async function unstar( .catch(() => {}); } -export async function setRating(id: string, rating: number): Promise { - await api('setRating.view', { id, rating }); - const serverId = useAuthStore.getState().activeServerId; - patchLibraryTrackOnUse(serverId, id, { userRating: rating }); - // Cached song lists keyed by rating (e.g. Tracks → Highly Rated rail) become - // stale immediately. `invalidateEntityUserRatingCaches` is static-imported: - // mix paths already pull `subsonicRatings` (e.g. mixRatingFilter), so a - // dynamic import would not split chunks and only triggered INEFFECTIVE_DYNAMIC_IMPORT. - // Navidrome browse stays lazy to keep this module free of that dependency when unused. +export async function setRating( + id: string, + rating: number, + options?: { serverId?: string; kind?: EntityRatingKind }, +): Promise { + const serverId = options?.serverId ?? useAuthStore.getState().activeServerId; + if (serverId && serverId !== useAuthStore.getState().activeServerId) { + await apiForServer(serverId, 'setRating.view', { id, rating }); + } else { + await api('setRating.view', { id, rating }); + } + if (options?.kind === 'album' || options?.kind === 'artist') { + if (serverId) putLocalEntityUserRatings([{ serverId, entityKind: options.kind, entityId: id, rating }]); + } else { + patchLibraryTrackOnUse(serverId, id, { userRating: rating }); + } + // Cached song lists keyed by rating (e.g. Tracks → Highly Rated rail) become stale immediately. void import('@/lib/api/navidromeBrowse').then(m => m.ndInvalidateSongsCache()).catch(() => {}); - invalidateEntityUserRatingCaches(id); } /**