fix: close remaining multi-server review findings

This commit is contained in:
cucadmuh
2026-07-21 11:53:01 +03:00
parent 4c030bfb91
commit ac83f88547
28 changed files with 671 additions and 233 deletions
@@ -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<Vec<String>> {
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<u64, String> {
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::<rusqlite::Result<Vec<_>>>()?;
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();
@@ -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<bool, String> {
if priority == 0 || identity_key.is_empty() {
return Ok(false);
}
let clauses = scopes
candidates: &[Vec<AlbumCandidate>],
) -> Result<HashMap<String, usize>, String> {
let identities = candidates
.iter()
.take(priority)
.map(|scope| {
if scope.library_id.is_some() {
"(server_id = ? AND library_id = ?)"
} else {
"(server_id = ?)"
}
})
.collect::<Vec<_>>()
.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::<HashSet<_>>();
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::<Vec<_>>()
.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::<rusqlite::Result<HashMap<_, _>>>()
})
.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<_>>(), 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<_>>(), vec!["high-dup"]);
}
#[test]
fn cursor_keeps_each_scope_position_without_skipping_tied_global_order() {
let store = LibraryStore::open_in_memory();
+4 -4
View File
@@ -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<HTMLInputElement>) => 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.
@@ -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' },
@@ -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<React.SetStateAction<SubsonicArtist | null>>;
}
export async function runArtistEntityRating(deps: RunArtistEntityRatingDeps): Promise<void> {
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,
@@ -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)));
+24 -3
View File
@@ -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');
});
});
+54 -52
View File
@@ -18,20 +18,11 @@ import { orbitServerMatches } from '@/features/orbit/utils/orbitServerScope';
* of playlists actually deleted, for logging.
*/
export async function cleanupOrphanedOrbitPlaylists(): Promise<number> {
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<ReturnType<typeof getPlaylistsForServer>>,
);
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_<sid>__`)
// and the outbox name (`__psyorbit_<sid>_from_<user>__`), so it must sit
@@ -39,52 +30,63 @@ export async function cleanupOrphanedOrbitPlaylists(): Promise<number> {
// 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<ReturnType<typeof getPlaylistsForServer>>,
);
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);
}
@@ -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]);
});
});
@@ -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);
}
@@ -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*
@@ -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<typeof import('@/lib/api/subsonic')>('@/lib/api/subsonic');
return {
@@ -50,8 +56,15 @@ vi.mock('@/music-network', () => {
vi.mock('@/store/orbitRuntime', async (importOriginal) => ({
...(await importOriginal<typeof import('@/store/orbitRuntime')>()),
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);
@@ -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<typeof import('@/store/orbitRuntime')>()),
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 });
@@ -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;
@@ -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<PlayerState> | ((state: PlayerState) => Partial<PlayerState>),
@@ -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);
@@ -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);
+23 -4
View File
@@ -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<boolean> {
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<boolean> {
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);
@@ -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);
});
});
@@ -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;
}
+2 -2
View File
@@ -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;
});
@@ -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']);
});
});
@@ -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<string, InternetRadioStation[]>();
@@ -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)];
@@ -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();
});
@@ -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' };
}
+16 -24
View File
@@ -64,6 +64,7 @@ export default function Statistics() {
const [genres, setGenres] = useState<SubsonicGenre[]>([]);
const [overviewLoading, setOverviewLoading] = useState(true);
const [aggregateLoading, setAggregateLoading] = useState(true);
const [aggregateUnavailable, setAggregateUnavailable] = useState(false);
const [totalPlaytime, setTotalPlaytime] = useState<number | null>(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 },
+91
View File
@@ -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<typeof import('@/lib/api/subsonicArtists')>()),
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': [] },
});
+109 -11
View File
@@ -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<string, { value: StatisticsLibraryAggregates; expiresAt: number }>();
async function fetchStatisticsAlbumsForScope(
scope: LibraryStatisticsScope,
pageSize: number,
limit: number,
): Promise<{ albums: SubsonicAlbum[]; capped: boolean }> {
const albumsById = new Map<string, SubsonicAlbum>();
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<StatisticsLibraryAggregates> {
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<string, { songCount: number; albumCount: number }>();
const formatCounts = new Map<string, number>();
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<StatisticsLibr
if (hit && Date.now() < hit.expiresAt) return hit.value;
}
const aggregate = await libraryScopeStatistics(scopes);
const result: StatisticsLibraryAggregates = {
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 })),
};
const indexReady = await readyLibraryServerKeys(scopes.map(scope => scope.serverId));
const result = indexReady
? await libraryScopeStatistics(scopes).then<StatisticsLibraryAggregates>(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 });
}
+1
View File
@@ -250,6 +250,7 @@ export interface StatisticsLibraryAggregates {
capped: boolean;
genres: SubsonicGenre[];
formats: { format: string; count: number }[];
formatTrackCount?: number;
}
export interface StatisticsOverviewData {