diff --git a/src-tauri/crates/psysonic-library/src/identity/rebuild.rs b/src-tauri/crates/psysonic-library/src/identity/rebuild.rs index edc52f23..614496b2 100644 --- a/src-tauri/crates/psysonic-library/src/identity/rebuild.rs +++ b/src-tauri/crates/psysonic-library/src/identity/rebuild.rs @@ -668,22 +668,38 @@ pub fn ensure_cluster_keys_built(store: &LibraryStore, server_id: &str) -> Resul }) } +fn pending_identity_server_ids(conn: &Connection) -> rusqlite::Result> { + let mut statement = conn.prepare( + "WITH RECURSIVE track_server(server_id) AS ( \ + SELECT MIN(server_id) FROM track INDEXED BY idx_track_album WHERE deleted = 0 \ + UNION ALL \ + SELECT ( \ + SELECT MIN(server_id) FROM track INDEXED BY idx_track_album \ + WHERE deleted = 0 AND server_id > track_server.server_id \ + ) FROM track_server WHERE server_id IS NOT NULL \ + ) \ + SELECT server_id FROM track_server WHERE server_id IS NOT NULL \ + UNION SELECT server_id FROM sync_state \ + UNION SELECT server_id FROM identity_invalidation \ + UNION SELECT substr(key, length(?1) + 1) FROM cluster.cluster_meta \ + WHERE key LIKE ?2 \ + ORDER BY server_id", + )?; + let server_ids = statement + .query_map( + params![DIRTY_META_PREFIX, format!("{DIRTY_META_PREFIX}%")], + |row| row.get::<_, String>(0), + )? + .collect(); + server_ids +} + /// Drain persisted invalidations at process start. This runs off the Tauri main -/// thread; normal healthy starts perform only one distinct-server query plus -/// O(1) journal checks per server. +/// thread; normal healthy starts use prefix seeks over the track index plus +/// compact metadata tables, then perform O(1) journal checks per server. pub fn ensure_pending_cluster_keys(store: &LibraryStore) -> Result { let server_ids = store - .with_read_conn(|conn| { - let mut statement = conn.prepare( - "SELECT server_id FROM track WHERE deleted = 0 \ - UNION SELECT server_id FROM identity_invalidation \ - ORDER BY server_id", - )?; - let server_ids = statement - .query_map([], |row| row.get::<_, String>(0))? - .collect::>>()?; - Ok(server_ids) - }) + .with_read_conn(pending_identity_server_ids) .map_err(|error| error.to_string())?; let mut refreshed = 0u64; for server_id in server_ids { @@ -813,6 +829,46 @@ mod tests { row } + #[test] + fn pending_identity_servers_include_indexed_tracks_and_compact_metadata() { + let store = LibraryStore::open_in_memory(); + store + .with_conn_mut("test.identity.pending_servers", |conn| { + conn.execute( + "INSERT INTO track (server_id, id, title, album, synced_at, raw_json) \ + VALUES ('track-server', 'track', 'Track', 'Album', 1, '{}'), \ + ('deleted-server', 'deleted', 'Deleted', 'Album', 1, '{}')", + [], + )?; + conn.execute( + "UPDATE track SET deleted = 1 WHERE server_id = 'deleted-server'", + [], + )?; + conn.execute( + "INSERT INTO sync_state (server_id, library_scope) \ + VALUES ('sync-server', ''), ('track-server', '')", + [], + )?; + conn.execute( + "INSERT INTO identity_invalidation (server_id, kind, entity_id) \ + VALUES ('journal-server', 'server', '')", + [], + )?; + conn.execute( + "INSERT INTO cluster.cluster_meta (key, value) VALUES (?1, '1')", + params![dirty_meta_key("dirty-server")], + )?; + Ok(()) + }) + .unwrap(); + + let server_ids = store.with_read_conn(pending_identity_server_ids).unwrap(); + assert_eq!( + server_ids, + vec!["dirty-server", "journal-server", "sync-server", "track-server"] + ); + } + #[test] fn rebuild_populates_keys_and_duration() { let store = LibraryStore::open_in_memory(); diff --git a/src-tauri/crates/psysonic-library/src/scope_browse.rs b/src-tauri/crates/psysonic-library/src/scope_browse.rs index 58a29c96..f6592bba 100644 --- a/src-tauri/crates/psysonic-library/src/scope_browse.rs +++ b/src-tauri/crates/psysonic-library/src/scope_browse.rs @@ -276,44 +276,41 @@ fn query_scope_candidates( .map_err(|error| error.to_string()) } -fn exists_in_higher_priority_scope( +fn album_identity_priorities( store: &LibraryStore, scopes: &[LibraryScopePair], - priority: usize, - identity_key: &str, -) -> Result { - if priority == 0 || identity_key.is_empty() { - return Ok(false); - } - let clauses = scopes + candidates: &[Vec], +) -> Result, String> { + let identities = candidates .iter() - .take(priority) - .map(|scope| { - if scope.library_id.is_some() { - "(server_id = ? AND library_id = ?)" - } else { - "(server_id = ?)" - } - }) - .collect::>() - .join(" OR "); - let sql = format!( - "SELECT 1 FROM album_browse_projection \ - WHERE identity_key = ? AND ({clauses}) LIMIT 1", - ); - let mut binds = vec![SqlValue::Text(identity_key.to_string())]; - for scope in scopes.iter().take(priority) { - binds.push(SqlValue::Text(scope.server_id.clone())); - if let Some(library_id) = &scope.library_id { - binds.push(SqlValue::Text(library_id.clone())); - } + .flatten() + .filter_map(|candidate| candidate.identity_key.clone()) + .collect::>(); + if identities.is_empty() { + return Ok(HashMap::new()); } + let (scope_cte, mut binds) = crate::scope_merge::scope_cte_sql(scopes); + let placeholders = (0..identities.len()) + .map(|_| "?") + .collect::>() + .join(", "); + let sql = format!( + "{scope_cte} SELECT projection.identity_key, MIN(scope.pr) \ + FROM scope \ + INNER JOIN album_browse_projection projection \ + ON projection.server_id = scope.server_id \ + AND projection.library_id = scope.library_id \ + WHERE projection.identity_key IN ({placeholders}) \ + GROUP BY projection.identity_key", + ); + binds.extend(identities.into_iter().map(SqlValue::Text)); store .with_read_conn(|conn| { - let present = conn - .query_row(&sql, params_from_iter(binds.iter()), |_| Ok(())) - .is_ok(); - Ok(present) + let mut stmt = conn.prepare(&sql)?; + let rows = stmt.query_map(params_from_iter(binds.iter()), |row| { + Ok((row.get::<_, String>(0)?, row.get::<_, i64>(1)? as usize)) + })?; + rows.collect::>>() }) .map_err(|error| error.to_string()) } @@ -343,6 +340,7 @@ fn browse_albums( stream_exhausted.push(stream.len() < candidate_limit); candidates.push(stream); } + let mut identity_priorities = album_identity_priorities(store, &request.scopes, &candidates)?; let mut albums = Vec::with_capacity(limit.saturating_add(1)); let mut offsets = vec![0usize; candidates.len()]; @@ -365,6 +363,7 @@ fn browse_albums( stream_exhausted[scope_index] = stream.len() < candidate_limit; candidates[scope_index] = stream; offsets[scope_index] = 0; + identity_priorities = album_identity_priorities(store, &request.scopes, &candidates)?; } let next_scope = candidates .iter() @@ -383,7 +382,10 @@ fn browse_albums( offsets[scope_index] += 1; positions[scope_index] = Some(cursor_position(candidate)); if let Some(identity_key) = candidate.identity_key.as_deref() { - if exists_in_higher_priority_scope(store, &request.scopes, candidate.priority, identity_key)? { + if identity_priorities + .get(identity_key) + .is_some_and(|priority| *priority < candidate.priority) + { continue; } } @@ -780,6 +782,23 @@ mod tests { ); } + #[test] + fn album_priority_dedup_holds_across_cursor_pages() { + let store = LibraryStore::open_in_memory(); + insert_projection(&store, "high", "lib", "high-dup", "Zulu", Some("same")); + insert_projection(&store, "low", "lib", "low-dup", "Alpha", Some("same")); + insert_projection(&store, "low", "lib", "low-unique", "Bravo", Some("other")); + let scopes = vec![ + LibraryScopePair { server_id: "high".into(), library_id: Some("lib".into()) }, + LibraryScopePair { server_id: "low".into(), library_id: Some("lib".into()) }, + ]; + + let first = browse(&store, &request(scopes.clone(), 1, None)).unwrap(); + assert_eq!(first.albums.iter().map(|album| album.id.as_str()).collect::>(), vec!["low-unique"]); + let second = browse(&store, &request(scopes, 1, first.next_cursor)).unwrap(); + assert_eq!(second.albums.iter().map(|album| album.id.as_str()).collect::>(), vec!["high-dup"]); + } + #[test] fn cursor_keeps_each_scope_position_without_skipping_tied_global_order() { let store = LibraryStore::open_in_memory(); diff --git a/src/features/artist/pages/ArtistDetail.tsx b/src/features/artist/pages/ArtistDetail.tsx index c5a554dc..fc79d1dc 100644 --- a/src/features/artist/pages/ArtistDetail.tsx +++ b/src/features/artist/pages/ArtistDetail.tsx @@ -105,7 +105,7 @@ export default function ArtistDetail() { // call order will mismatch between renders. const sectionConfig = useArtistLayoutStore(s => s.sections); const entityRatingSupportByServer = useAuthStore(s => s.entityRatingSupportByServer); - const artistEntityRatingSupport = entityRatingSupportByServer[activeServerId] ?? 'unknown'; + const artistEntityRatingSupport = entityRatingSupportByServer[artistOwnerServerId] ?? 'unknown'; const offlineCtx = useOfflineBrowseContext(); const artistActionPolicy = offlineActionPolicy('artistDetail', offlineCtx.active); @@ -122,7 +122,7 @@ export default function ArtistDetail() { }, [id, artist?.id, artist?.userRating]); const handleArtistEntityRating = (rating: number) => runArtistEntityRating({ - artist, id, rating, artistEntityRatingSupport, activeServerId, t, + artist, id, rating, artistEntityRatingSupport, serverId: artistOwnerServerId, t, setArtistEntityRating, setArtist, }); @@ -150,7 +150,7 @@ export default function ArtistDetail() { const handleShareArtist = () => { if (!id || !artist) return; - return runArtistShare({ artist, serverId: activeServerId, t }); + return runArtistShare({ artist, serverId: artistOwnerServerId, t }); }; const playTopSongWithContinuation = (startIndex: number) => runArtistDetailPlayTopSong({ @@ -163,7 +163,7 @@ export default function ArtistDetail() { }); const handleImageUpload = (e: React.ChangeEvent) => runArtistImageUpload({ - e, artist, serverId: activeServerId, t, setUploading, setCoverRevision, + e, artist, serverId: artistOwnerServerId, t, setUploading, setCoverRevision, }); // Cover URLs — must run every render (before early returns) or hook order breaks. diff --git a/src/features/artist/utils/runArtistDetailActions.test.ts b/src/features/artist/utils/runArtistDetailActions.test.ts index 49dce85c..2b752e0f 100644 --- a/src/features/artist/utils/runArtistDetailActions.test.ts +++ b/src/features/artist/utils/runArtistDetailActions.test.ts @@ -2,12 +2,16 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import type { TFunction } from 'i18next'; import type React from 'react'; import { + runArtistEntityRating, runArtistImageUpload, runArtistShare, } from '@/features/artist/utils/runArtistDetailActions'; import { uploadArtistImageForServer } from '@/lib/api/subsonicArtists'; import { copyEntityShareLink } from '@/lib/share/copyEntityShareLink'; import { invalidateCoverArt } from '@/cover'; +import { setRating } from '@/lib/api/subsonicStarRating'; +import { useAuthStore } from '@/store/authStore'; +import { resetAuthStore } from '@/test/helpers/storeReset'; vi.mock('@/lib/api/subsonicArtists', () => ({ uploadArtistImageForServer: vi.fn(async () => undefined), @@ -18,15 +22,48 @@ vi.mock('@/lib/share/copyEntityShareLink', () => ({ vi.mock('@/cover', () => ({ invalidateCoverArt: vi.fn(async () => undefined), })); +vi.mock('@/lib/api/subsonicStarRating', () => ({ + setRating: vi.fn(async () => undefined), + star: vi.fn(async () => undefined), + unstar: vi.fn(async () => undefined), +})); vi.mock('@/lib/dom/toast', () => ({ showToast: vi.fn() })); const t = ((key: string) => key) as TFunction; beforeEach(() => { vi.clearAllMocks(); + resetAuthStore(); }); describe('artist detail explicit-server actions', () => { + it('downgrades rating support on the artist owner after a rejected write', async () => { + vi.mocked(setRating).mockRejectedValueOnce(new Error('unsupported')); + useAuthStore.setState({ + entityRatingSupportByServer: { 'srv-active': 'full', 'srv-owner': 'full' }, + }); + + await runArtistEntityRating({ + artist: { id: 'artist-1', name: 'Artist', serverId: 'srv-owner', userRating: 2 }, + id: 'artist-1', + rating: 4, + artistEntityRatingSupport: 'full', + serverId: 'srv-owner', + t, + setArtistEntityRating: vi.fn(), + setArtist: vi.fn(), + }); + + expect(setRating).toHaveBeenCalledWith('artist-1', 4, { + serverId: 'srv-owner', + kind: 'artist', + }); + expect(useAuthStore.getState().entityRatingSupportByServer).toEqual({ + 'srv-active': 'full', + 'srv-owner': 'track_only', + }); + }); + it('shares through the artist owner instead of the active server', async () => { await runArtistShare({ artist: { id: 'artist-1', name: 'Artist', serverId: 'srv-owner' }, diff --git a/src/features/artist/utils/runArtistDetailActions.ts b/src/features/artist/utils/runArtistDetailActions.ts index 30b13c9a..b4ef509a 100644 --- a/src/features/artist/utils/runArtistDetailActions.ts +++ b/src/features/artist/utils/runArtistDetailActions.ts @@ -13,14 +13,14 @@ export interface RunArtistEntityRatingDeps { id: string | undefined; rating: number; artistEntityRatingSupport: string; - activeServerId: string; + serverId: string; t: TFunction; setArtistEntityRating: (v: number) => void; setArtist: React.Dispatch>; } export async function runArtistEntityRating(deps: RunArtistEntityRatingDeps): Promise { - const { artist, id, rating, artistEntityRatingSupport, activeServerId, t, setArtistEntityRating, setArtist } = deps; + const { artist, id, rating, artistEntityRatingSupport, serverId, t, setArtistEntityRating, setArtist } = deps; if (!artist || artist.id !== id) return; const artistId = artist.id; const ratingAtStart = artist.userRating ?? 0; @@ -29,12 +29,13 @@ export async function runArtistEntityRating(deps: RunArtistEntityRatingDeps): Pr if (artistEntityRatingSupport !== 'full') return; + const ownerServerId = artist.serverId ?? serverId; try { - await setRating(artistId, rating, { serverId: artist.serverId ?? activeServerId, kind: 'artist' }); + await setRating(artistId, rating, { serverId: ownerServerId, kind: 'artist' }); setArtist(a => (a && a.id === artistId ? { ...a, userRating: rating } : a)); } catch (err) { setArtistEntityRating(ratingAtStart); - useAuthStore.getState().setEntityRatingSupport(activeServerId, 'track_only'); + useAuthStore.getState().setEntityRatingSupport(ownerServerId, 'track_only'); showToast( typeof err === 'string' ? err : err instanceof Error ? err.message : t('entityRating.saveFailed'), 4500, diff --git a/src/features/favorites/hooks/useFavoritesData.ts b/src/features/favorites/hooks/useFavoritesData.ts index 644c1ba6..39f67b1e 100644 --- a/src/features/favorites/hooks/useFavoritesData.ts +++ b/src/features/favorites/hooks/useFavoritesData.ts @@ -109,7 +109,6 @@ export function useFavoritesData(): FavoritesDataResult { const migrated = new Set(migrateRadioStationKeys( [...favIds], available, - activeServerId, )); localStorage.setItem('psysonic_radio_favorites', JSON.stringify([...migrated])); return available.filter(station => migrated.has(radioStationKey(station))); diff --git a/src/features/orbit/utils/cleanup.test.ts b/src/features/orbit/utils/cleanup.test.ts index 6bd6325d..f9a3affd 100644 --- a/src/features/orbit/utils/cleanup.test.ts +++ b/src/features/orbit/utils/cleanup.test.ts @@ -15,7 +15,9 @@ const { getPlaylistsForServer, deletePlaylist } = vi.hoisted(() => ({ })); const { authState, orbitState } = vi.hoisted(() => ({ - authState: { username: 'me' as string | undefined }, + authState: { + servers: [{ id: 'srv-owner', username: 'me' }] as Array<{ id: string; username?: string }>, + }, orbitState: { sessionId: null as string | null, serverId: 'srv-owner' as string | null }, })); @@ -23,7 +25,7 @@ vi.mock('@/lib/api/subsonicPlaylists', () => ({ getPlaylistsForServer, deletePla vi.mock('@/store/authStore', () => ({ useAuthStore: { getState: () => ({ - getActiveServer: () => (authState.username ? { id: 'srv-owner', username: authState.username } : undefined), + servers: authState.servers, }), }, })); @@ -55,7 +57,7 @@ function outboxComment(ageMs: number): string { } beforeEach(() => { - authState.username = 'me'; + authState.servers = [{ id: 'srv-owner', username: 'me' }]; orbitState.sessionId = null; orbitState.serverId = 'srv-owner'; deletePlaylist.mockReset().mockResolvedValue(undefined); @@ -171,4 +173,23 @@ describe('cleanupOrphanedOrbitPlaylists', () => { ]); expect(deleted).toEqual([]); }); + + it('sweeps stale owned playlists on every configured server', async () => { + authState.servers = [ + { id: 'srv-owner', username: 'me' }, + { id: 'srv-other', username: 'also-me' }, + ]; + getPlaylistsForServer.mockImplementation(async (serverId: string) => ([{ + id: `${serverId}-stale`, + name: orbitSessionPlaylistName('abcd1234'), + owner: serverId === 'srv-owner' ? 'me' : 'also-me', + comment: sessionComment('abcd1234', ORBIT_ORPHAN_TTL_MS + 60_000), + }])); + + await expect(cleanupOrphanedOrbitPlaylists()).resolves.toBe(2); + expect(getPlaylistsForServer).toHaveBeenCalledWith('srv-owner', true); + expect(getPlaylistsForServer).toHaveBeenCalledWith('srv-other', true); + expect(deletePlaylist).toHaveBeenCalledWith('srv-owner-stale', 'srv-owner'); + expect(deletePlaylist).toHaveBeenCalledWith('srv-other-stale', 'srv-other'); + }); }); diff --git a/src/features/orbit/utils/cleanup.ts b/src/features/orbit/utils/cleanup.ts index 167842dc..387ab8ab 100644 --- a/src/features/orbit/utils/cleanup.ts +++ b/src/features/orbit/utils/cleanup.ts @@ -18,20 +18,11 @@ import { orbitServerMatches } from '@/features/orbit/utils/orbitServerScope'; * of playlists actually deleted, for logging. */ export async function cleanupOrphanedOrbitPlaylists(): Promise { - const server = useAuthStore.getState().getActiveServer(); - const username = server?.username; - if (!server || !username) return 0; - const serverId = server.id; - - const all = await getPlaylistsForServer(serverId, true).catch( - () => [] as Awaited>, - ); + const servers = useAuthStore.getState().servers.filter(server => Boolean(server.username)); + if (servers.length === 0) return 0; const now = Date.now(); const TTL = ORBIT_ORPHAN_TTL_MS; const orbit = useOrbitStore.getState(); - const currentSid = orbit.serverId && orbitServerMatches(serverId, orbit.serverId) - ? orbit.sessionId - : null; // The trailing `__` is part of *both* the session name (`__psyorbit___`) // and the outbox name (`__psyorbit__from___`), so it must sit @@ -39,52 +30,63 @@ export async function cleanupOrphanedOrbitPlaylists(): Promise { // the bare session name never matched and fell into the unconditional-prune // branch below — deleting live sessions on the user's other devices. const nameRe = new RegExp(`^${ORBIT_PLAYLIST_PREFIX}([a-f0-9]+)(_from_.+)?__$`); - let deleted = 0; + const deletedByServer = await Promise.all(servers.map(async server => { + const serverId = server.id; + const username = server.username; + const all = await getPlaylistsForServer(serverId, true).catch( + () => [] as Awaited>, + ); + const currentSid = orbit.serverId && orbitServerMatches(serverId, orbit.serverId) + ? orbit.sessionId + : null; + let deleted = 0; - for (const p of all) { - if (!p.name.startsWith(ORBIT_PLAYLIST_PREFIX)) continue; - // Only touch our own — Navidrome rejects deletes on foreign playlists anyway. - if (p.owner && p.owner !== username) continue; + for (const p of all) { + if (!p.name.startsWith(ORBIT_PLAYLIST_PREFIX)) continue; + // Only touch our own — Navidrome rejects deletes on foreign playlists anyway. + if (p.owner && p.owner !== username) continue; - const match = p.name.match(nameRe); - // Not one we recognise — assume corrupt, prune. - if (!match) { - try { await deletePlaylist(p.id, serverId); deleted++; } catch { /* best-effort */ } - continue; - } - const sid = match[1]; - const isOutbox = !!match[2]; - if (sid === currentSid) continue; + const match = p.name.match(nameRe); + // Not one we recognise — assume corrupt, prune. + if (!match) { + try { await deletePlaylist(p.id, serverId); deleted++; } catch { /* best-effort */ } + continue; + } + const sid = match[1]; + const isOutbox = !!match[2]; + if (sid === currentSid) continue; - let timestamp = 0; - let ended = false; - if (p.comment) { - try { - const parsed = JSON.parse(p.comment); - if (isOutbox) { - if (parsed && typeof parsed.ts === 'number') timestamp = parsed.ts; - } else { - const state = parseOrbitState(parsed); - if (state) { - timestamp = state.positionAt ?? 0; - ended = state.ended === true; + let timestamp = 0; + let ended = false; + if (p.comment) { + try { + const parsed = JSON.parse(p.comment); + if (isOutbox) { + if (parsed && typeof parsed.ts === 'number') timestamp = parsed.ts; + } else { + const state = parseOrbitState(parsed); + if (state) { + timestamp = state.positionAt ?? 0; + ended = state.ended === true; + } } - } - } catch { /* unparseable → treat as dead */ } - } + } catch { /* unparseable → treat as dead */ } + } - // Fall back to Navidrome's `changed` timestamp when there's no - // orbit-authored heartbeat in the comment — saves us from deleting a - // playlist that was just created seconds ago. - if (timestamp === 0 && p.changed) { - const parsed = Date.parse(p.changed); - if (!isNaN(parsed)) timestamp = parsed; - } + // Fall back to Navidrome's `changed` timestamp when there's no + // orbit-authored heartbeat in the comment — saves us from deleting a + // playlist that was just created seconds ago. + if (timestamp === 0 && p.changed) { + const parsed = Date.parse(p.changed); + if (!isNaN(parsed)) timestamp = parsed; + } - const stale = timestamp === 0 || (now - timestamp > TTL); - if (ended || stale) { - try { await deletePlaylist(p.id, serverId); deleted++; } catch { /* best-effort */ } + const stale = timestamp === 0 || (now - timestamp > TTL); + if (ended || stale) { + try { await deletePlaylist(p.id, serverId); deleted++; } catch { /* best-effort */ } + } } - } - return deleted; + return deleted; + })); + return deletedByServer.reduce((sum, count) => sum + count, 0); } diff --git a/src/features/playback/store/applyServerPlayQueue.test.ts b/src/features/playback/store/applyServerPlayQueue.test.ts index 10e59b82..04dc0012 100644 --- a/src/features/playback/store/applyServerPlayQueue.test.ts +++ b/src/features/playback/store/applyServerPlayQueue.test.ts @@ -68,7 +68,7 @@ describe('mergeQueueServerProjection', () => { ]); }); - it('removes old surplus slots and inserts remote surplus after the last prior slot', () => { + it('preserves local surplus by default and inserts remote surplus after the last prior slot', () => { const b1 = { serverId: 'b', trackId: 'b1' }; expect(mergeQueueServerProjection( [ @@ -93,6 +93,13 @@ describe('mergeQueueServerProjection', () => { [{ serverId: 'a', trackId: 'a1' }, b1, { serverId: 'a', trackId: 'a2' }], 'a', [{ serverId: 'a', trackId: 'a3' }], + )).toEqual([{ serverId: 'a', trackId: 'a3' }, b1, { serverId: 'a', trackId: 'a2' }]); + + expect(mergeQueueServerProjection( + [{ serverId: 'a', trackId: 'a1' }, b1, { serverId: 'a', trackId: 'a2' }], + 'a', + [{ serverId: 'a', trackId: 'a3' }], + false, )).toEqual([{ serverId: 'a', trackId: 'a3' }, b1]); }); }); diff --git a/src/features/playback/store/applyServerPlayQueue.ts b/src/features/playback/store/applyServerPlayQueue.ts index 65b380a6..18f74b57 100644 --- a/src/features/playback/store/applyServerPlayQueue.ts +++ b/src/features/playback/store/applyServerPlayQueue.ts @@ -142,6 +142,7 @@ export function mergeQueueServerProjection( existing: QueueItemRef[], serverProfileId: string, remote: QueueItemRef[], + preserveLocalSurplus = true, ): QueueItemRef[] { const merged: QueueItemRef[] = []; let remoteIndex = 0; @@ -158,6 +159,8 @@ export function mergeQueueServerProjection( if (remoteIndex < remote.length) { merged.push(remote[remoteIndex]); remoteIndex++; + } else if (preserveLocalSurplus) { + merged.push(ref); } insertionIndex = merged.length; } @@ -173,12 +176,18 @@ export function applyMappedQueueProjection( mappedTracks: Track[], q: PlayQueueResult, serverProfileId: string, + preserveLocalSurplus = true, ): void { seedQueueResolver(serverProfileId, mappedTracks); const remoteRefs = toQueueItemRefs(serverProfileId, mappedTracks); const player = usePlayerStore.getState(); const previousCurrentRef = player.queueItems[player.queueIndex]; - const queueItems = mergeQueueServerProjection(player.queueItems, serverProfileId, remoteRefs); + const queueItems = mergeQueueServerProjection( + player.queueItems, + serverProfileId, + remoteRefs, + preserveLocalSurplus, + ); const exactPreservedIndex = previousCurrentRef ? queueItems.indexOf(previousCurrentRef) : -1; const preservedIndex = exactPreservedIndex >= 0 @@ -271,7 +280,9 @@ export async function applyServerPlayQueue( const localTime = usePlayerStore.getState().currentTime; if (queueIsMultiServer()) { // Keep the other owners' slots in place while refreshing this server's order. - applyMappedQueueProjection(mappedTracks, q, profileId); + // Background pulls preserve local surplus; an explicit manual pull has an + // undo snapshot and may intentionally accept the shorter remote queue. + applyMappedQueueProjection(mappedTracks, q, profileId, !options.pushUndo); } else { applyMappedQueue(mappedTracks, q, profileId, preferServerPosition, localTime); } diff --git a/src/features/playback/store/playTrackAction.ts b/src/features/playback/store/playTrackAction.ts index 30c61017..91455c80 100644 --- a/src/features/playback/store/playTrackAction.ts +++ b/src/features/playback/store/playTrackAction.ts @@ -4,6 +4,8 @@ import { audioSeek } from '@/lib/api/audio'; import { getMusicNetworkRuntimeOrNull } from '@/music-network'; import { setDeferHotCachePrefetch } from '@/lib/cache/hotCacheGate'; import { orbitAllowsTrackServer, orbitBulkGuard, orbitSnapshot } from '@/store/orbitRuntime'; +import i18n from '@/lib/i18n'; +import { showToast } from '@/lib/dom/toast'; import { queueItemRefMatchesTrack, queueItemIdentityKey, @@ -134,8 +136,13 @@ export function runPlayTrack( targetQueueIndex: number | undefined, ): void { if (orbitSnapshot().role === 'host') { - if (!orbitAllowsTrackServer(track.serverId)) return; - if (queue?.some(queueTrack => !orbitAllowsTrackServer(queueTrack.serverId))) return; + if ( + !orbitAllowsTrackServer(track.serverId) + || queue?.some(queueTrack => !orbitAllowsTrackServer(queueTrack.serverId)) + ) { + showToast(i18n.t('queue.crossServerEnqueueBlocked'), 4000, 'error'); + return; + } } // Orbit bulk-gate: only gate when the `queue` argument *replaces* diff --git a/src/features/playback/store/playerStore.playbackActions.test.ts b/src/features/playback/store/playerStore.playbackActions.test.ts index b4dfc72f..b8dec0dd 100644 --- a/src/features/playback/store/playerStore.playbackActions.test.ts +++ b/src/features/playback/store/playerStore.playbackActions.test.ts @@ -13,6 +13,12 @@ */ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +const orbitMocks = vi.hoisted(() => ({ + role: null as 'host' | null, + allowsTrackServer: vi.fn((_serverId?: string) => true), + showToast: vi.fn(), +})); + vi.mock('@/lib/api/subsonic', async () => { const actual = await vi.importActual('@/lib/api/subsonic'); return { @@ -50,8 +56,15 @@ vi.mock('@/music-network', () => { vi.mock('@/store/orbitRuntime', async (importOriginal) => ({ ...(await importOriginal()), orbitBulkGuard: vi.fn(async () => true), - orbitAllowsTrackServer: vi.fn(() => true), + orbitAllowsTrackServer: orbitMocks.allowsTrackServer, + orbitSnapshot: () => ({ + role: orbitMocks.role, + phase: orbitMocks.role ? 'active' : 'idle', + state: null, + serverId: orbitMocks.role ? 'srv-a' : null, + }), })); +vi.mock('@/lib/dom/toast', () => ({ showToast: orbitMocks.showToast })); import { usePlayerStore } from '@/features/playback/store/playerStore'; import { onInvoke, invokeMock } from '@/test/mocks/tauri'; @@ -81,6 +94,10 @@ beforeEach(() => { vi.useFakeTimers(); resetPlayerStore(); resetAuthStore(); + orbitMocks.role = null; + orbitMocks.allowsTrackServer.mockReset(); + orbitMocks.allowsTrackServer.mockReturnValue(true); + orbitMocks.showToast.mockReset(); stubPlaybackInvokes(); }); @@ -295,6 +312,19 @@ describe('audio_play failure', () => { }); }); +describe('Orbit host server ownership', () => { + it('reports a blocked cross-server play instead of failing silently', () => { + orbitMocks.role = 'host'; + orbitMocks.allowsTrackServer.mockReturnValue(false); + const foreign = makeTrack({ id: 'foreign', serverId: 'srv-b' }); + + usePlayerStore.getState().playTrack(foreign, [foreign]); + + expect(invokeMock).not.toHaveBeenCalledWith('audio_play', expect.anything()); + expect(orbitMocks.showToast).toHaveBeenCalledWith(expect.any(String), 4000, 'error'); + }); +}); + describe('previous', () => { it('restarts the current track when currentTime > 3 s', () => { const queue = makeTracks(3); diff --git a/src/features/playback/store/playerStore.queue.test.ts b/src/features/playback/store/playerStore.queue.test.ts index 10afc6c6..5d9d2a01 100644 --- a/src/features/playback/store/playerStore.queue.test.ts +++ b/src/features/playback/store/playerStore.queue.test.ts @@ -11,6 +11,12 @@ */ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +const orbitMocks = vi.hoisted(() => ({ + role: null as 'host' | null, + allowsTrackServer: vi.fn((_serverId?: string) => true), + showToast: vi.fn(), +})); + // `playerStore` pulls `savePlayQueue` from `@/lib/api/subsonic`, which talks to a // real server. Override only what the queue path touches; everything else // stays as the actual module so unrelated imports don't break. @@ -29,8 +35,15 @@ vi.mock('@/lib/api/subsonic', async () => { vi.mock('@/store/orbitRuntime', async (importOriginal) => ({ ...(await importOriginal()), orbitBulkGuard: vi.fn(async () => true), - orbitAllowsTrackServer: vi.fn(() => true), + orbitAllowsTrackServer: orbitMocks.allowsTrackServer, + orbitSnapshot: () => ({ + role: orbitMocks.role, + phase: orbitMocks.role ? 'active' : 'idle', + state: null, + serverId: orbitMocks.role ? 'srv-a' : null, + }), })); +vi.mock('@/lib/dom/toast', () => ({ showToast: orbitMocks.showToast })); import { usePlayerStore } from '@/features/playback/store/playerStore'; import { @@ -46,6 +59,10 @@ import { _resetRadioSessionStateForTest } from '@/features/playback/store/radioS beforeEach(() => { resetPlayerStore(); _resetRadioSessionStateForTest(); + orbitMocks.role = null; + orbitMocks.allowsTrackServer.mockReset(); + orbitMocks.allowsTrackServer.mockReturnValue(true); + orbitMocks.showToast.mockReset(); // `clearQueue` fires `invoke('audio_stop')`; every queue mutation triggers a // debounced `syncQueueToServer` we don't need to advance. onInvoke('audio_stop', () => undefined); @@ -77,6 +94,18 @@ describe('enqueue', () => { expect(usePlayerStore.getState().queueItems.map(r => r.trackId)).toEqual(tracks.map(t => t.id)); }); + it('keeps allowed Orbit-host tracks and reports rejected server owners', () => { + orbitMocks.role = 'host'; + orbitMocks.allowsTrackServer.mockImplementation(serverId => serverId === 'srv-a'); + const allowed = makeTrack({ id: 'allowed', serverId: 'srv-a' }); + const rejected = makeTrack({ id: 'rejected', serverId: 'srv-b' }); + + usePlayerStore.getState().enqueue([allowed, rejected], true); + + expect(usePlayerStore.getState().queueItems.map(ref => ref.trackId)).toEqual(['allowed']); + expect(orbitMocks.showToast).toHaveBeenCalledWith(expect.any(String), 4000, 'error'); + }); + it('inserts before the first upcoming auto-added separator', () => { const head = makeTrack({ id: 'head' }); const auto = makeTrack({ id: 'auto', autoAdded: true }); diff --git a/src/features/playback/store/playerStoreTypes.ts b/src/features/playback/store/playerStoreTypes.ts index 094dc83b..b6676752 100644 --- a/src/features/playback/store/playerStoreTypes.ts +++ b/src/features/playback/store/playerStoreTypes.ts @@ -132,7 +132,12 @@ export interface PlayerState { reorderQueue: (startIndex: number, endIndex: number) => void; removeTrack: (index: number) => void; /** Replace one frozen queue slot only when its concrete owner/id still match. */ - replaceQueueItemSource: (index: number, expected: QueueItemRef, replacement: QueueItemRef) => boolean; + replaceQueueItemSource: ( + index: number, + expected: QueueItemRef, + replacement: QueueItemRef, + userInitiated?: boolean, + ) => boolean; shuffleQueue: () => void; /** Shuffle only the tracks after the current one — leaves played history intact. */ shuffleUpcomingQueue: () => void; diff --git a/src/features/playback/store/queueMutationActions.ts b/src/features/playback/store/queueMutationActions.ts index 1ef4f956..09411ee2 100644 --- a/src/features/playback/store/queueMutationActions.ts +++ b/src/features/playback/store/queueMutationActions.ts @@ -7,6 +7,7 @@ import { toQueueItemRefs } from '@/features/playback/store/queueItemRef'; import { seedQueueResolver } from '@/features/playback/store/queueTrackResolver'; import { pushQueueUndoFromGetter } from '@/features/playback/store/queueUndo'; import { + syncAutomaticQueueMutationToServers, syncUserQueueClearToServers, syncUserQueueMutationToServer, } from '@/features/playback/store/queueSync'; @@ -41,6 +42,8 @@ import { sameQueueItemRef, } from '@/features/playback/utils/playback/queueIdentity'; import { canonicalQueueServerKey } from '@/lib/server/serverIndexKey'; +import i18n from '@/lib/i18n'; +import { showToast } from '@/lib/dom/toast'; type SetState = ( partial: Partial | ((state: PlayerState) => Partial), @@ -89,7 +92,7 @@ export function createQueueMutationActions(set: SetState, get: GetState): Pick< | 'replaceQueueItemSource' > { return { - replaceQueueItemSource: (index, expected, replacement) => { + replaceQueueItemSource: (index, expected, replacement, userInitiated = true) => { const state = get(); const current = state.queueItems[index]; if (!current || !sameQueueItemRef(current, expected)) return false; @@ -98,12 +101,10 @@ export function createQueueMutationActions(set: SetState, get: GetState): Pick< const nextItems = [...state.queueItems]; nextItems[index] = { ...replacement }; set({ queueItems: nextItems }); - syncUserQueueMutationToServer( - state.queueItems, - nextItems, - state.currentTrack, - state.currentTime, - ); + const sync = userInitiated + ? syncUserQueueMutationToServer + : syncAutomaticQueueMutationToServers; + sync(state.queueItems, nextItems, state.currentTrack, state.currentTime); return true; }, @@ -152,7 +153,11 @@ export function createQueueMutationActions(set: SetState, get: GetState): Pick< enqueue: (tracks, _orbitConfirmed = false, skipQueueUndo = false) => { if (orbitSnapshot().role === 'host') { - tracks = tracks.filter(track => orbitAllowsTrackServer(track.serverId)); + const allowed = tracks.filter(track => orbitAllowsTrackServer(track.serverId)); + if (allowed.length !== tracks.length) { + showToast(i18n.t('queue.crossServerEnqueueBlocked'), 4000, 'error'); + } + tracks = allowed; if (tracks.length === 0) return; } if (!_orbitConfirmed && tracks.length > 1) { @@ -189,7 +194,11 @@ export function createQueueMutationActions(set: SetState, get: GetState): Pick< enqueueRadio: (tracks, artistId, serverId) => { if (orbitSnapshot().role === 'host') { - tracks = tracks.filter(track => orbitAllowsTrackServer(track.serverId)); + const allowed = tracks.filter(track => orbitAllowsTrackServer(track.serverId)); + if (allowed.length !== tracks.length) { + showToast(i18n.t('queue.crossServerEnqueueBlocked'), 4000, 'error'); + } + tracks = allowed; if (tracks.length === 0) return; } if (artistId !== undefined) { @@ -250,7 +259,11 @@ export function createQueueMutationActions(set: SetState, get: GetState): Pick< enqueueAt: (tracks, insertIndex, _orbitConfirmed = false) => { if (orbitSnapshot().role === 'host') { - tracks = tracks.filter(track => orbitAllowsTrackServer(track.serverId)); + const allowed = tracks.filter(track => orbitAllowsTrackServer(track.serverId)); + if (allowed.length !== tracks.length) { + showToast(i18n.t('queue.crossServerEnqueueBlocked'), 4000, 'error'); + } + tracks = allowed; if (tracks.length === 0) return; } if (!_orbitConfirmed && tracks.length > 1) { @@ -278,7 +291,11 @@ export function createQueueMutationActions(set: SetState, get: GetState): Pick< playNext: (tracks) => { if (orbitSnapshot().role === 'host') { - tracks = tracks.filter(track => orbitAllowsTrackServer(track.serverId)); + const allowed = tracks.filter(track => orbitAllowsTrackServer(track.serverId)); + if (allowed.length !== tracks.length) { + showToast(i18n.t('queue.crossServerEnqueueBlocked'), 4000, 'error'); + } + tracks = allowed; } if (tracks.length === 0) return; ensureQueueServerPinned(tracks); diff --git a/src/features/playback/store/queueSync.test.ts b/src/features/playback/store/queueSync.test.ts index 55255e72..1171bd95 100644 --- a/src/features/playback/store/queueSync.test.ts +++ b/src/features/playback/store/queueSync.test.ts @@ -54,6 +54,7 @@ import { hasPendingQueueSync, pushQueueOnPlaybackStart, syncQueueToServer, + syncAutomaticQueueMutationToServers, syncUserQueueClearToServers, syncUserQueueMutationToServer, } from '@/features/playback/store/queueSync'; @@ -191,6 +192,22 @@ describe('syncUserQueueMutationToServer (debounced)', () => { }); }); +describe('syncAutomaticQueueMutationToServers', () => { + it('syncs every affected owner without suspending idle pull', async () => { + syncAutomaticQueueMutationToServers( + [ref('a1', 'a.test')], + [ref('b1', 'b.test')], + track('b1', 'srv-b'), + 7, + ); + + expect(isIdleQueuePullSuspended()).toBe(false); + await vi.advanceTimersByTimeAsync(5000); + expect(savePlayQueueMock).toHaveBeenCalledWith([], undefined, undefined, 'srv-a'); + expect(savePlayQueueMock).toHaveBeenCalledWith(['b1'], 'b1', 7000, 'srv-b'); + }); +}); + describe('syncUserQueueClearToServers', () => { it('clears every remote server queue represented by the previous mixed queue', async () => { syncUserQueueClearToServers([ @@ -237,6 +254,14 @@ describe('pushQueueOnPlaybackStart', () => { }); describe('flushQueueSyncToServer failure', () => { + it('reports unreachable as failure and blocks stale idle pull', async () => { + isSubsonicServerReachableMock.mockReturnValue(false); + const ok = await flushQueueSyncToServer([ref('a')], track('a'), 12); + expect(ok).toBe(false); + expect(savePlayQueueMock).not.toHaveBeenCalled(); + expect(isQueuePushFailed()).toBe(true); + }); + it('flags the failed push (blocking idle pull) without lighting the handoff LED', async () => { savePlayQueueMock.mockRejectedValueOnce(new Error('offline')); const ok = await flushQueueSyncToServer([ref('a')], track('a'), 12); diff --git a/src/features/playback/store/queueSync.ts b/src/features/playback/store/queueSync.ts index fcb16ceb..1a965624 100644 --- a/src/features/playback/store/queueSync.ts +++ b/src/features/playback/store/queueSync.ts @@ -122,7 +122,7 @@ function scheduleQueueSyncToServer( syncTimeoutByServer.set(serverId, timeout); } -function scheduleUserQueueSyncByServer( +function scheduleQueueSyncByServer( previousQueue: QueueItemRef[], nextQueue: QueueItemRef[], currentTrack: Track | null, @@ -175,7 +175,17 @@ export function syncUserQueueMutationToServer( currentTime: number, ): void { touchQueueMutationClock(); - scheduleUserQueueSyncByServer(previousQueue, nextQueue, currentTrack, currentTime); + scheduleQueueSyncByServer(previousQueue, nextQueue, currentTrack, currentTime); +} + +/** Debounced multi-owner sync for automatic queue repairs; does not suspend idle pull. */ +export function syncAutomaticQueueMutationToServers( + previousQueue: QueueItemRef[], + nextQueue: QueueItemRef[], + currentTrack: Track | null, + currentTime: number, +): void { + scheduleQueueSyncByServer(previousQueue, nextQueue, currentTrack, currentTime); } /** Debounced remote clear for every server represented by the removed local refs. */ @@ -194,7 +204,12 @@ export function flushQueueSyncToServer( ): Promise { const serverId = getPlaybackServerId(); cancelPendingQueueSync(serverId); - if (!isPlaybackServerReachable()) return Promise.resolve(true); + if (!isPlaybackServerReachable()) { + if (serverId && serverId !== NAVIDROME_PUBLIC_SHARE_SERVER_ID) { + markQueuePushFailed(serverId); + } + return Promise.resolve(false); + } if (!currentTrack || queue.length === 0) return Promise.resolve(true); lastQueueHeartbeatAtByServer.set(serverId, Date.now()); const refs = filterQueueRefsForPlaybackServer(queue); @@ -207,7 +222,11 @@ export function flushQueueSyncToServer( */ export function flushPlayQueueForServer(serverProfileId: string): Promise { cancelPendingQueueSync(serverProfileId); - if (!serverProfileId || !isSubsonicServerReachable(serverProfileId)) return Promise.resolve(true); + if (!serverProfileId) return Promise.resolve(false); + if (!isSubsonicServerReachable(serverProfileId)) { + markQueuePushFailed(serverProfileId); + return Promise.resolve(false); + } const s = usePlayerStore.getState(); if (s.currentRadio) return Promise.resolve(true); const refs = filterQueueRefsForServerProfile(s.queueItems, serverProfileId); diff --git a/src/features/playback/store/selectPlaybackAlternative.test.ts b/src/features/playback/store/selectPlaybackAlternative.test.ts index dc3ba984..e47553ba 100644 --- a/src/features/playback/store/selectPlaybackAlternative.test.ts +++ b/src/features/playback/store/selectPlaybackAlternative.test.ts @@ -19,6 +19,11 @@ import { resetAuthStore, resetPlayerStore } from '@/test/helpers/storeReset'; import { makeServer, makeTrack } from '@/test/helpers/factories'; import { useAuthStore } from '@/store/authStore'; import type { PlaybackAlternativeSource } from '@/features/playback/utils/playback/availablePlaybackAlternativeSources'; +import { + _resetQueuePlaybackIdleForTest, + isIdleQueuePullSuspended, +} from '@/features/playback/store/queuePlaybackIdle'; +import { _resetQueueSyncForTest } from '@/features/playback/store/queueSync'; const serverA = makeServer({ id: 'srv-a', url: 'https://a.test' }); const serverB = makeServer({ id: 'srv-b', url: 'https://b.test' }); @@ -60,6 +65,8 @@ beforeEach(() => { resetAuthStore(); resetPlayerStore(); _resetEngineStateForTest(); + _resetQueuePlaybackIdleForTest(); + _resetQueueSyncForTest(); Object.values(mocks).forEach(mock => mock.mockReset()); mocks.resolveBatch.mockResolvedValue(undefined); useAuthStore.setState({ servers: [serverA, serverB], activeServerId: serverA.id }); @@ -133,5 +140,6 @@ describe('selectPlaybackAlternative', () => { 1, ); expect(usePlaybackAlternativeStore.getState().failure).toBeNull(); + expect(isIdleQueuePullSuspended()).toBe(false); }); }); diff --git a/src/features/playback/store/selectPlaybackAlternative.ts b/src/features/playback/store/selectPlaybackAlternative.ts index 92c35649..d5b61461 100644 --- a/src/features/playback/store/selectPlaybackAlternative.ts +++ b/src/features/playback/store/selectPlaybackAlternative.ts @@ -49,7 +49,7 @@ export async function selectPlaybackAlternative(source: LibraryEntitySourceDto): setPlaybackAlternativeActionError(); return false; } - if (!latest.replaceQueueItemSource(failure.queueIndex, failure.expectedRef, nextRef)) { + if (!latest.replaceQueueItemSource(failure.queueIndex, failure.expectedRef, nextRef, false)) { setPlaybackAlternativeActionError(); return false; } diff --git a/src/features/radio/pages/InternetRadio.tsx b/src/features/radio/pages/InternetRadio.tsx index c281e77c..d64db662 100644 --- a/src/features/radio/pages/InternetRadio.tsx +++ b/src/features/radio/pages/InternetRadio.tsx @@ -151,7 +151,7 @@ export default function InternetRadio() { try { return JSON.parse(localStorage.getItem('psysonic_radio_order') ?? '[]'); } catch { return []; } })(); - const merged = migrateRadioStationKeys(saved, stations, activeServerId); + const merged = migrateRadioStationKeys(saved, stations); stations.forEach(s => { const key = radioStationKey(s); if (!merged.includes(key)) merged.push(key); @@ -167,7 +167,7 @@ export default function InternetRadio() { // React Compiler set-state-in-effect rule: migrate persisted raw ids after owners load. // eslint-disable-next-line react-hooks/set-state-in-effect setFavorites(previous => { - const migrated = new Set(migrateRadioStationKeys([...previous], stations, activeServerId)); + const migrated = new Set(migrateRadioStationKeys([...previous], stations)); localStorage.setItem('psysonic_radio_favorites', JSON.stringify([...migrated])); return migrated; }); diff --git a/src/features/radio/utils/radioStationIdentity.test.ts b/src/features/radio/utils/radioStationIdentity.test.ts index 03291667..635c1ffc 100644 --- a/src/features/radio/utils/radioStationIdentity.test.ts +++ b/src/features/radio/utils/radioStationIdentity.test.ts @@ -17,19 +17,17 @@ describe('radioStationIdentity', () => { expect(sameRadioStation(stations[0], stations[1])).toBe(false); }); - it('migrates a legacy raw id to the preferred owner and preserves unavailable keys', () => { + it('does not bind an ambiguous legacy raw id to the active owner', () => { expect(migrateRadioStationKeys( ['shared', 'srv-c:missing'], stations, - 'srv-b', - )).toEqual(['srv-b:shared', 'srv-c:missing']); + )).toEqual(['shared', 'srv-c:missing']); }); - it('does not assign a raw id to another owner while the preferred owner is absent', () => { + it('migrates an unambiguous raw id and preserves unavailable keys', () => { expect(migrateRadioStationKeys( - ['shared'], + ['shared', 'missing'], [stations[0]], - 'srv-b', - )).toEqual(['shared']); + )).toEqual(['srv-a:shared', 'missing']); }); }); diff --git a/src/features/radio/utils/radioStationIdentity.ts b/src/features/radio/utils/radioStationIdentity.ts index 462b9758..9f02a408 100644 --- a/src/features/radio/utils/radioStationIdentity.ts +++ b/src/features/radio/utils/radioStationIdentity.ts @@ -12,11 +12,10 @@ export function sameRadioStation( return Boolean(a && b && radioStationKey(a) === radioStationKey(b)); } -/** Convert persisted raw ids to one concrete owner without dropping unavailable-owner keys. */ +/** Convert only unambiguous persisted raw ids without dropping unavailable-owner keys. */ export function migrateRadioStationKeys( keys: readonly string[], stations: readonly InternetRadioStation[], - preferredServerId?: string | null, ): string[] { const exactKeys = new Set(stations.map(radioStationKey)); const stationsByRawId = new Map(); @@ -30,10 +29,6 @@ export function migrateRadioStationKeys( if (exactKeys.has(key)) return key; const candidates = stationsByRawId.get(key); if (!candidates?.length) return key; - if (preferredServerId) { - const preferred = candidates.find(station => station.serverId === preferredServerId); - return preferred ? radioStationKey(preferred) : key; - } return candidates.length === 1 ? radioStationKey(candidates[0]) : key; }); return [...new Set(migrated)]; diff --git a/src/features/share/enqueueShareSearchPayload.test.ts b/src/features/share/enqueueShareSearchPayload.test.ts index 16ec0525..82331e2c 100644 --- a/src/features/share/enqueueShareSearchPayload.test.ts +++ b/src/features/share/enqueueShareSearchPayload.test.ts @@ -12,9 +12,9 @@ const mocks = vi.hoisted(() => ({ }, enqueue: vi.fn(), getAlbum: vi.fn(), - getAlbumWithCredentials: vi.fn(), + resolveAlbum: vi.fn(), getArtist: vi.fn(), - getArtistWithCredentials: vi.fn(), + resolveArtist: vi.fn(), getSongForServer: vi.fn(), orbitBulkGuard: vi.fn(), showToast: vi.fn(), @@ -30,9 +30,9 @@ vi.mock('@/lib/api/subsonicArtists', () => ({ getArtist: mocks.getArtist, })); -vi.mock('@/lib/api/subsonicEntityWithCredentials', () => ({ - getAlbumWithCredentials: mocks.getAlbumWithCredentials, - getArtistWithCredentials: mocks.getArtistWithCredentials, +vi.mock('@/store/mediaResolver', () => ({ + resolveAlbum: mocks.resolveAlbum, + resolveArtist: mocks.resolveArtist, })); vi.mock('@/store/authStore', () => ({ @@ -105,11 +105,11 @@ describe('share search payload resolution', () => { setActiveServer: vi.fn(), }; mocks.getSongForServer.mockResolvedValue({ ...sharedSong, serverId: 'shared' }); - mocks.getAlbumWithCredentials.mockResolvedValue({ + mocks.resolveAlbum.mockResolvedValue({ album: { id: 'album-1', name: 'Shared Album', artist: 'Shared Artist' }, songs: [], }); - mocks.getArtistWithCredentials.mockResolvedValue({ + mocks.resolveArtist.mockResolvedValue({ artist: { id: 'artist-1', name: 'Shared Artist' }, albums: [], }); @@ -150,20 +150,8 @@ describe('share search payload resolution', () => { id: 'artist-1', }); - expect(mocks.getAlbumWithCredentials).toHaveBeenCalledWith( - sharedServer.url, - sharedServer.username, - sharedServer.password, - 'album-1', - sharedServer, - ); - expect(mocks.getArtistWithCredentials).toHaveBeenCalledWith( - sharedServer.url, - sharedServer.username, - sharedServer.password, - 'artist-1', - sharedServer, - ); + expect(mocks.resolveAlbum).toHaveBeenCalledWith('shared', 'album-1'); + expect(mocks.resolveArtist).toHaveBeenCalledWith('shared', 'artist-1'); expect(mocks.getAlbum).not.toHaveBeenCalled(); expect(mocks.getArtist).not.toHaveBeenCalled(); expect(mocks.authState.current.setActiveServer).not.toHaveBeenCalled(); @@ -179,13 +167,7 @@ describe('share search payload resolution', () => { }); expect(result.type).toBe('ok'); - expect(mocks.getArtistWithCredentials).toHaveBeenCalledWith( - sharedServer.url, - sharedServer.username, - sharedServer.password, - 'composer-1', - sharedServer, - ); + expect(mocks.resolveArtist).toHaveBeenCalledWith('shared', 'composer-1'); expect(mocks.authState.current.setActiveServer).not.toHaveBeenCalled(); }); diff --git a/src/features/share/enqueueShareSearchPayload.ts b/src/features/share/enqueueShareSearchPayload.ts index 2198d7cc..387836cf 100644 --- a/src/features/share/enqueueShareSearchPayload.ts +++ b/src/features/share/enqueueShareSearchPayload.ts @@ -1,15 +1,11 @@ import type { TFunction } from 'i18next'; -import { - getAlbumWithCredentials, - getArtistWithCredentials, -} from '@/lib/api/subsonicEntityWithCredentials'; import { getSongForServer } from '@/lib/api/subsonicLibrary'; import type { SubsonicAlbum, SubsonicArtist, SubsonicSong } from '@/lib/api/subsonicTypes'; import { usePlayerStore } from '@/features/playback/store/playerStore'; import { songToTrack } from '@/lib/media/songToTrack'; import type { Track } from '@/lib/media/trackTypes'; import { orbitBulkGuard } from '@/features/orbit'; -import { connectBaseUrlForServer } from '@/lib/server/serverEndpoint'; +import { resolveAlbum, resolveArtist } from '@/store/mediaResolver'; import type { AlbumShareSearchPayload, ArtistShareSearchPayload, @@ -113,14 +109,10 @@ export async function resolveShareSearchAlbum( } try { - const { album } = await getAlbumWithCredentials( - connectBaseUrlForServer(lookup.server), - lookup.server.username, - lookup.server.password, - payload.id, - lookup.server, - ); - return { type: 'ok', album: { ...album, serverId: lookup.serverId } }; + const resolved = await resolveAlbum(lookup.serverId, payload.id); + return resolved + ? { type: 'ok', album: { ...resolved.album, serverId: lookup.serverId } } + : { type: 'unavailable' }; } catch { return { type: 'unavailable' }; } @@ -138,14 +130,10 @@ export async function resolveShareSearchArtist( } try { - const { artist } = await getArtistWithCredentials( - connectBaseUrlForServer(lookup.server), - lookup.server.username, - lookup.server.password, - payload.id, - lookup.server, - ); - return { type: 'ok', artist: { ...artist, serverId: lookup.serverId } }; + const resolved = await resolveArtist(lookup.serverId, payload.id); + return resolved + ? { type: 'ok', artist: { ...resolved.artist, serverId: lookup.serverId } } + : { type: 'unavailable' }; } catch { return { type: 'unavailable' }; } diff --git a/src/features/stats/pages/Statistics.tsx b/src/features/stats/pages/Statistics.tsx index 3c68112b..52b78fe0 100644 --- a/src/features/stats/pages/Statistics.tsx +++ b/src/features/stats/pages/Statistics.tsx @@ -64,6 +64,7 @@ export default function Statistics() { const [genres, setGenres] = useState([]); const [overviewLoading, setOverviewLoading] = useState(true); const [aggregateLoading, setAggregateLoading] = useState(true); + const [aggregateUnavailable, setAggregateUnavailable] = useState(false); const [totalPlaytime, setTotalPlaytime] = useState(null); const [playtimeCapped, setPlaytimeCapped] = useState(false); @@ -99,19 +100,12 @@ export default function Statistics() { return; } setOverviewLoading(true); - const startedAt = performance.now(); fetchStatisticsOverview() .then(d => { setRecent(d.recent); setFrequent(d.frequent); setHighest(d.highest); setOverviewLoading(false); - console.info('[statistics] overview loaded', { - elapsedMs: Math.round(performance.now() - startedAt), - recent: d.recent.length, - frequent: d.frequent.length, - highest: d.highest.length, - }); }) .catch(() => setOverviewLoading(false)); }, [musicLibraryFilterVersion, libraryBrowseScopeVersion, offlineBrowseActive, isPlayerStats]); @@ -131,7 +125,7 @@ export default function Statistics() { setFormatData(null); setFormatTrackCount(0); setAggregateLoading(true); - const startedAt = performance.now(); + setAggregateUnavailable(false); (async () => { try { const agg = await fetchStatisticsLibraryAggregates(); @@ -143,27 +137,21 @@ export default function Statistics() { setPlaytimeCapped(agg.capped); setGenres(agg.genres); setFormatData(agg.formats); - setFormatTrackCount(agg.songsCounted); + setFormatTrackCount(agg.formatTrackCount ?? agg.songsCounted); setAggregateLoading(false); - console.info('[statistics] index aggregates loaded', { - elapsedMs: Math.round(performance.now() - startedAt), - artists: agg.artistCount, - albums: agg.albumsCounted, - songs: agg.songsCounted, - genres: agg.genres.length, - formats: agg.formats.length, - }); + setAggregateUnavailable(false); } catch { if (!cancelled) { - setArtistCount(0); - setTotalPlaytime(0); - setTotalAlbums(0); - setTotalSongs(0); + setArtistCount(null); + setTotalPlaytime(null); + setTotalAlbums(null); + setTotalSongs(null); setPlaytimeCapped(false); setGenres([]); setFormatData([]); setFormatTrackCount(0); setAggregateLoading(false); + setAggregateUnavailable(true); } } })(); @@ -199,15 +187,19 @@ export default function Statistics() { }).catch(() => setLfmLoading(false)); }, [lfmPeriod, enrichmentPrimaryId, offlineBrowseActive, isPlayerStats]); - const playtimeDisplay = totalPlaytime === null + const playtimeDisplay = aggregateUnavailable + ? '—' + : totalPlaytime === null ? t('statistics.computing') : (playtimeCapped ? '≥ ' : '') + formatHumanHoursMinutes(totalPlaytime); const countDisplay = (n: number | null) => - n === null ? t('statistics.computing') : (playtimeCapped ? '≥ ' : '') + n.toLocaleString(); + aggregateUnavailable + ? '—' + : n === null ? t('statistics.computing') : (playtimeCapped ? '≥ ' : '') + n.toLocaleString(); const stats = [ - { label: t('statistics.statArtists'), value: artistCount?.toLocaleString() ?? t('statistics.computing'), tooltip: t('statistics.statArtistsTooltip') }, + { label: t('statistics.statArtists'), value: aggregateUnavailable ? '—' : artistCount?.toLocaleString() ?? t('statistics.computing'), tooltip: t('statistics.statArtistsTooltip') }, { label: t('statistics.statAlbums'), value: countDisplay(totalAlbums) }, { label: t('statistics.statSongs'), value: countDisplay(totalSongs) }, { label: t('statistics.statPlaytime'), value: playtimeDisplay }, diff --git a/src/lib/api/subsonicStatistics.test.ts b/src/lib/api/subsonicStatistics.test.ts index 0c85a83e..ad2e65b5 100644 --- a/src/lib/api/subsonicStatistics.test.ts +++ b/src/lib/api/subsonicStatistics.test.ts @@ -16,11 +16,24 @@ const apiMock = vi.fn(); const indexStatisticsMock = vi.fn(); const indexMostPlayedMock = vi.fn(); const getAlbumListForServerMock = vi.fn(); +const getRandomSongsForServerMock = vi.fn(); +const getArtistsForServerMock = vi.fn(); +const readyLibraryServerKeysMock = vi.fn(); beforeEach(resetServerReachabilitySnapshot); vi.mock('@/lib/api/subsonicLibrary', () => ({ getAlbumListForServer: (...args: unknown[]) => getAlbumListForServerMock(...args), + getRandomSongsForServer: (...args: unknown[]) => getRandomSongsForServerMock(...args), +})); + +vi.mock('@/lib/api/subsonicArtists', async importOriginal => ({ + ...(await importOriginal()), + getArtistsForServer: (...args: unknown[]) => getArtistsForServerMock(...args), +})); + +vi.mock('@/lib/library/libraryReady', () => ({ + readyLibraryServerKeys: (...args: unknown[]) => readyLibraryServerKeysMock(...args), })); vi.mock('@/lib/api/subsonicClient', async importOriginal => { @@ -89,6 +102,9 @@ describe('fetchStatisticsLibraryAggregates', () => { beforeEach(() => { indexStatisticsMock.mockReset(); getAlbumListForServerMock.mockReset(); + getRandomSongsForServerMock.mockReset().mockResolvedValue([]); + getArtistsForServerMock.mockReset().mockResolvedValue([]); + readyLibraryServerKeysMock.mockReset().mockImplementation(async (serverIds: string[]) => serverIds); useAuthStore.setState({ activeServerId: 'stats-a', servers: [ @@ -121,6 +137,7 @@ describe('fetchStatisticsLibraryAggregates', () => { capped: false, genres: [{ value: 'Rock', songCount: 40, albumCount: 10 }], formats: [{ format: 'FLAC', count: 60 }], + formatTrackCount: 80, }); expect(second).toBe(first); expect(indexStatisticsMock).toHaveBeenCalledTimes(1); @@ -147,6 +164,72 @@ describe('fetchStatisticsLibraryAggregates', () => { { serverId: 'stats-a', libraryIds: ['rock'] }, ]); }); + + it('falls back to explicit-server APIs when the local index is not ready', async () => { + useAuthStore.setState({ + activeServerId: 'fallback-a', + servers: [ + { id: 'fallback-a', name: 'A', url: 'https://a.test', username: 'u', password: 'p' }, + ], + libraryBrowseServerIds: ['fallback-a'], + libraryBrowseSelectionByServer: { 'fallback-a': [] }, + }); + readyLibraryServerKeysMock.mockResolvedValue(null); + getArtistsForServerMock.mockResolvedValue([{ id: 'artist-1', name: 'Artist' }]); + getAlbumListForServerMock.mockResolvedValue([{ + id: 'album-1', + name: 'Album', + duration: 600, + songCount: 5, + genre: 'Rock', + }]); + getRandomSongsForServerMock.mockResolvedValue([ + { id: 'song-1', title: 'Song', suffix: 'flac' }, + ]); + + await expect(fetchStatisticsLibraryAggregates()).resolves.toEqual(expect.objectContaining({ + artistCount: 1, + albumsCounted: 1, + songsCounted: 5, + playtimeSec: 600, + formats: [{ format: 'FLAC', count: 1 }], + formatTrackCount: 1, + })); + expect(indexStatisticsMock).not.toHaveBeenCalled(); + expect(getAlbumListForServerMock).toHaveBeenCalledWith( + 'fallback-a', + 'alphabeticalByName', + 500, + 0, + {}, + ); + }); + + it('falls back to the network when the ready index query fails', async () => { + useAuthStore.setState({ + activeServerId: 'ipc-fallback', + servers: [ + { id: 'ipc-fallback', name: 'Fallback', url: 'https://fallback.test', username: 'u', password: 'p' }, + ], + libraryBrowseServerIds: ['ipc-fallback'], + libraryBrowseSelectionByServer: { 'ipc-fallback': [] }, + }); + indexStatisticsMock.mockRejectedValueOnce(new Error('ipc unavailable')); + getAlbumListForServerMock.mockResolvedValue([{ + id: 'album-1', + name: 'Album', + duration: 300, + songCount: 3, + }]); + + await expect(fetchStatisticsLibraryAggregates()).resolves.toEqual(expect.objectContaining({ + albumsCounted: 1, + songsCounted: 3, + playtimeSec: 300, + })); + expect(indexStatisticsMock).toHaveBeenCalledOnce(); + expect(getAlbumListForServerMock).toHaveBeenCalledOnce(); + }); }); describe('fetchStatisticsOverview', () => { @@ -154,6 +237,10 @@ describe('fetchStatisticsOverview', () => { getAlbumListForServerMock.mockReset(); useAuthStore.setState({ activeServerId: 'stats-a', + servers: [ + { id: 'stats-a', name: 'A', url: 'https://a.test', username: 'u', password: 'p' }, + { id: 'stats-b', name: 'B', url: 'https://b.test', username: 'u', password: 'p' }, + ], libraryBrowseServerIds: ['stats-a', 'stats-b'], libraryBrowseSelectionByServer: { 'stats-a': ['rock'], 'stats-b': [] }, }); @@ -181,6 +268,10 @@ describe('fetchMostPlayedAlbums', () => { indexMostPlayedMock.mockReset(); useAuthStore.setState({ activeServerId: 'stats-a', + servers: [ + { id: 'stats-a', name: 'A', url: 'https://a.test', username: 'u', password: 'p' }, + { id: 'stats-b', name: 'B', url: 'https://b.test', username: 'u', password: 'p' }, + ], libraryBrowseServerIds: ['stats-a', 'stats-b'], libraryBrowseSelectionByServer: { 'stats-a': ['rock'], 'stats-b': [] }, }); diff --git a/src/lib/api/subsonicStatistics.ts b/src/lib/api/subsonicStatistics.ts index 3b921941..34e14dd7 100644 --- a/src/lib/api/subsonicStatistics.ts +++ b/src/lib/api/subsonicStatistics.ts @@ -1,5 +1,6 @@ import { useAuthStore } from '@/store/authStore'; -import { getAlbumListForServer } from '@/lib/api/subsonicLibrary'; +import { getArtistsForServer } from '@/lib/api/subsonicArtists'; +import { getAlbumListForServer, getRandomSongsForServer } from '@/lib/api/subsonicLibrary'; import { libraryScopeCacheKeyForServer } from '@/lib/api/subsonicClient'; import { libraryScopeStatistics, @@ -13,6 +14,8 @@ import type { SubsonicAlbum, } from '@/lib/api/subsonicTypes'; import { deriveLibraryBrowseIndexScopes } from '@/lib/library/libraryBrowseScope'; +import { genreTagsFor } from '@/lib/library/genreTags'; +import { readyLibraryServerKeys } from '@/lib/library/libraryReady'; /** Cache TTL for statistics page aggregates — same 7-minute window as * the rating prefetch cache in subsonicRatings.ts. */ @@ -49,6 +52,98 @@ function statisticsAggregateCacheKey(scopes: LibraryStatisticsScope[]): string | const statisticsAggregatesCache = new Map(); +async function fetchStatisticsAlbumsForScope( + scope: LibraryStatisticsScope, + pageSize: number, + limit: number, +): Promise<{ albums: SubsonicAlbum[]; capped: boolean }> { + const albumsById = new Map(); + const libraryIds = scope.libraryIds.length > 1 ? scope.libraryIds : [null]; + let capped = false; + for (const libraryId of libraryIds) { + let offset = 0; + while (albumsById.size < limit) { + const size = Math.min(pageSize, limit - albumsById.size); + const albums = await getAlbumListForServer( + scope.serverId, + 'alphabeticalByName', + size, + offset, + libraryId ? { musicFolderId: libraryId } : {}, + ); + for (const album of albums) albumsById.set(album.id, album); + if (albums.length < size) break; + offset += size; + } + if (albumsById.size >= limit) { + capped = true; + break; + } + } + return { albums: [...albumsById.values()], capped }; +} + +async function fetchStatisticsNetworkAggregates( + scopes: LibraryStatisticsScope[], +): Promise { + const pageSize = 500; + const albumLimitPerServer = 5_000; + const byServer = await Promise.all(scopes.map(async scope => { + const [artists, formatSongs, albumResult] = await Promise.all([ + getArtistsForServer(scope.serverId).catch(() => []), + getRandomSongsForServer(scope.serverId, 500).catch(() => []), + fetchStatisticsAlbumsForScope(scope, pageSize, albumLimitPerServer), + ]); + return { artists, formatSongs, ...albumResult }; + })); + + const genreAgg = new Map(); + const formatCounts = new Map(); + let artistCount = 0; + let playtimeSec = 0; + let albumsCounted = 0; + let songsCounted = 0; + let formatTrackCount = 0; + let capped = false; + for (const server of byServer) { + artistCount += server.artists.length; + capped ||= server.capped; + for (const album of server.albums) { + playtimeSec += album.duration ?? 0; + albumsCounted += 1; + const songCount = album.songCount ?? 0; + songsCounted += songCount; + const labels = genreTagsFor(album); + for (const label of labels.length > 0 ? labels : ['']) { + const aggregate = genreAgg.get(label) ?? { songCount: 0, albumCount: 0 }; + aggregate.songCount += songCount; + aggregate.albumCount += 1; + genreAgg.set(label, aggregate); + } + } + for (const song of server.formatSongs) { + const format = song.suffix?.toUpperCase() ?? 'Unknown'; + formatCounts.set(format, (formatCounts.get(format) ?? 0) + 1); + formatTrackCount += 1; + } + } + + return { + artistCount, + playtimeSec, + albumsCounted, + songsCounted, + capped, + genres: [...genreAgg.entries()] + .map(([value, counts]) => ({ value, ...counts })) + .sort((a, b) => b.songCount - a.songCount), + formats: [...formatCounts.entries()] + .map(([format, count]) => ({ format, count })) + .sort((a, b) => b.count - a.count), + formatTrackCount, + }; +} + /** * Reads aggregate counts from the local index. Cache keys include every selected * server/folder, and intentionally preserve duplicate entities across scopes. @@ -61,16 +156,19 @@ export async function fetchStatisticsLibraryAggregates(): Promise ({ format: format.value, count: format.songCount })), - }; + const indexReady = await readyLibraryServerKeys(scopes.map(scope => scope.serverId)); + const result = indexReady + ? await libraryScopeStatistics(scopes).then(aggregate => ({ + artistCount: aggregate.artistCount, + playtimeSec: aggregate.playtimeSec, + albumsCounted: aggregate.albumCount, + songsCounted: aggregate.songCount, + capped: false, + genres: aggregate.genres, + formats: aggregate.formats.map(format => ({ format: format.value, count: format.songCount })), + formatTrackCount: aggregate.songCount, + })).catch(() => fetchStatisticsNetworkAggregates(scopes)) + : await fetchStatisticsNetworkAggregates(scopes); if (key) { statisticsAggregatesCache.set(key, { value: result, expiresAt: Date.now() + STATS_CACHE_TTL }); } diff --git a/src/lib/api/subsonicTypes.ts b/src/lib/api/subsonicTypes.ts index 1fc9d425..48b5487c 100644 --- a/src/lib/api/subsonicTypes.ts +++ b/src/lib/api/subsonicTypes.ts @@ -250,6 +250,7 @@ export interface StatisticsLibraryAggregates { capped: boolean; genres: SubsonicGenre[]; formats: { format: string; count: number }[]; + formatTrackCount?: number; } export interface StatisticsOverviewData {