mirror of
https://github.com/Psychotoxical/psysonic.git
synced 2026-07-22 07:15:47 +00:00
fix(playback): cross-server browse, Lucky Mix, and Now Playing (#768)
* fix(playback): keep browsed server when queue plays elsewhere Lucky Mix on a non-playback server now clears the old queue and pins the active server before building. Now Playing metadata uses apiForServer against the queue server instead of forcing ensurePlaybackServerActive on every activeServerId change. * docs: note PR #768 in CHANGELOG and settings credits * fix(now-playing): gate AudioMuse similar artists on playback server Match getArtistInfoForServer credentials: when browsing another server while the queue plays elsewhere, similar-artist count follows the queue server's AudioMuse flag, not the browsed server.
This commit is contained in:
@@ -1,33 +0,0 @@
|
||||
import { useEffect, useState } from 'react';
|
||||
import { useAuthStore } from '../store/authStore';
|
||||
import { usePlayerStore } from '../store/playerStore';
|
||||
import {
|
||||
ensurePlaybackServerActive,
|
||||
playbackServerDiffersFromActive,
|
||||
} from '../utils/playback/playbackServer';
|
||||
|
||||
/**
|
||||
* On Now Playing surfaces, switch the browsed server to {@link queueServerId}
|
||||
* before Subsonic fetches run. Returns false while a switch is in flight.
|
||||
*/
|
||||
export function useEnsurePlaybackServerOnMount(): boolean {
|
||||
const queueServerId = usePlayerStore(s => s.queueServerId);
|
||||
const queueLength = usePlayerStore(s => s.queue.length);
|
||||
const activeServerId = useAuthStore(s => s.activeServerId);
|
||||
const [ready, setReady] = useState(() => !playbackServerDiffersFromActive());
|
||||
|
||||
useEffect(() => {
|
||||
if (!playbackServerDiffersFromActive()) {
|
||||
setReady(true);
|
||||
return;
|
||||
}
|
||||
let cancelled = false;
|
||||
setReady(false);
|
||||
void ensurePlaybackServerActive().then(ok => {
|
||||
if (!cancelled) setReady(ok);
|
||||
});
|
||||
return () => { cancelled = true; };
|
||||
}, [queueServerId, queueLength, activeServerId]);
|
||||
|
||||
return ready;
|
||||
}
|
||||
@@ -18,8 +18,8 @@ vi.mock('../api/subsonicLibrary');
|
||||
vi.mock('../api/bandsintown');
|
||||
vi.mock('../api/lastfm');
|
||||
|
||||
import { getArtist, getArtistInfo, getTopSongs } from '../api/subsonicArtists';
|
||||
import { getAlbum, getSong } from '../api/subsonicLibrary';
|
||||
import { getArtistForServer, getArtistInfoForServer, getTopSongsForServer } from '../api/subsonicArtists';
|
||||
import { getAlbumForServer, getSongForServer } from '../api/subsonicLibrary';
|
||||
import { fetchBandsintownEvents } from '../api/bandsintown';
|
||||
import { lastfmGetArtistStats, lastfmGetTrackInfo, lastfmIsConfigured } from '../api/lastfm';
|
||||
import { useNowPlayingFetchers, type NowPlayingFetchersDeps } from './useNowPlayingFetchers';
|
||||
@@ -28,8 +28,8 @@ import { useNowPlayingFetchers, type NowPlayingFetchersDeps } from './useNowPlay
|
||||
// the hook treats `null` as the "no info available" case and stores it as
|
||||
// such in its tuple. The tests mirror that — cast to a nullable-returning
|
||||
// shape so we can mock the empty case without `as any` at every site.
|
||||
const mockArtistInfo = vi.mocked(getArtistInfo) as unknown as {
|
||||
mockImplementation: (impl: (id: string) => Promise<SubsonicArtistInfo | null>) => void;
|
||||
const mockArtistInfo = vi.mocked(getArtistInfoForServer) as unknown as {
|
||||
mockImplementation: (impl: (serverId: string, id: string) => Promise<SubsonicArtistInfo | null>) => void;
|
||||
mockResolvedValue: (v: SubsonicArtistInfo | null) => void;
|
||||
};
|
||||
|
||||
@@ -47,8 +47,8 @@ const baseDeps: NowPlayingFetchersDeps = {
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
vi.mocked(getTopSongs).mockResolvedValue([]);
|
||||
vi.mocked(getArtist).mockResolvedValue({ albums: [] } as any);
|
||||
vi.mocked(getTopSongsForServer).mockResolvedValue([]);
|
||||
vi.mocked(getArtistForServer).mockResolvedValue({ albums: [] } as any);
|
||||
vi.mocked(fetchBandsintownEvents).mockResolvedValue([]);
|
||||
vi.mocked(lastfmIsConfigured).mockReturnValue(false);
|
||||
vi.mocked(lastfmGetTrackInfo).mockResolvedValue(null);
|
||||
@@ -71,7 +71,7 @@ describe('useNowPlayingFetchers — id-gated artistInfo', () => {
|
||||
it('returns null artistInfo while the previously-resolved info belongs to a different artistId', async () => {
|
||||
const a = deferred<SubsonicArtistInfo | null>();
|
||||
const b = deferred<SubsonicArtistInfo | null>();
|
||||
mockArtistInfo.mockImplementation(async (id) => {
|
||||
mockArtistInfo.mockImplementation(async (_sid, id) => {
|
||||
if (id === 'art-A') return a.promise;
|
||||
if (id === 'art-B') return b.promise;
|
||||
return null;
|
||||
@@ -108,7 +108,7 @@ describe('useNowPlayingFetchers — id-gated artistInfo', () => {
|
||||
it('does not leak a late-arriving resolve for a stale artistId', async () => {
|
||||
// Race: artist A's fetch resolves AFTER the consumer switched to B.
|
||||
const a = deferred<SubsonicArtistInfo | null>();
|
||||
mockArtistInfo.mockImplementation(async (id) => {
|
||||
mockArtistInfo.mockImplementation(async (_sid, id) => {
|
||||
if (id === 'art-A') return a.promise;
|
||||
if (id === 'art-B') return { largeImageUrl: 'B.jpg' } as SubsonicArtistInfo;
|
||||
return null;
|
||||
@@ -136,7 +136,7 @@ describe('useNowPlayingFetchers — id-gated songMeta / albumData / discography'
|
||||
it('gates songMeta on songId match', async () => {
|
||||
const s1 = deferred<SubsonicSong | null>();
|
||||
const s2 = deferred<SubsonicSong | null>();
|
||||
vi.mocked(getSong).mockImplementation(async (id) => {
|
||||
vi.mocked(getSongForServer).mockImplementation(async (_sid, id) => {
|
||||
if (id === 's1') return s1.promise;
|
||||
if (id === 's2') return s2.promise;
|
||||
return null;
|
||||
@@ -161,7 +161,7 @@ describe('useNowPlayingFetchers — id-gated songMeta / albumData / discography'
|
||||
it('gates albumData on albumId match', async () => {
|
||||
const al1 = deferred<{ album: SubsonicAlbum; songs: SubsonicSong[] } | null>();
|
||||
const al2 = deferred<{ album: SubsonicAlbum; songs: SubsonicSong[] } | null>();
|
||||
vi.mocked(getAlbum).mockImplementation(async (id) => {
|
||||
vi.mocked(getAlbumForServer).mockImplementation(async (_sid, id) => {
|
||||
if (id === 'alb1') return al1.promise as any;
|
||||
if (id === 'alb2') return al2.promise as any;
|
||||
return null as any;
|
||||
@@ -186,7 +186,7 @@ describe('useNowPlayingFetchers — id-gated songMeta / albumData / discography'
|
||||
it('gates discography on artistId match (empty fallback while gated)', async () => {
|
||||
const d1 = deferred<{ artist: any; albums: SubsonicAlbum[] }>();
|
||||
const d2 = deferred<{ artist: any; albums: SubsonicAlbum[] }>();
|
||||
vi.mocked(getArtist).mockImplementation(async (id) => {
|
||||
vi.mocked(getArtistForServer).mockImplementation(async (_sid, id) => {
|
||||
if (id === 'art-D1') return d1.promise as any;
|
||||
if (id === 'art-D2') return d2.promise as any;
|
||||
return { albums: [] } as any;
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { useEffect, useState } from 'react';
|
||||
import { getArtist, getArtistInfo, getTopSongs } from '../api/subsonicArtists';
|
||||
import { getAlbum, getSong } from '../api/subsonicLibrary';
|
||||
import { getArtistForServer, getArtistInfoForServer, getTopSongsForServer } from '../api/subsonicArtists';
|
||||
import { getAlbumForServer, getSongForServer } from '../api/subsonicLibrary';
|
||||
import type { SubsonicAlbum, SubsonicArtistInfo, SubsonicSong } from '../api/subsonicTypes';
|
||||
import { fetchBandsintownEvents, type BandsintownEvent } from '../api/bandsintown';
|
||||
import {
|
||||
@@ -30,7 +30,7 @@ export interface NowPlayingFetchersDeps {
|
||||
currentTrack: { artist: string; title: string } | null;
|
||||
/** Subsonic server for API calls — must match the playing queue server. */
|
||||
subsonicServerId: string;
|
||||
/** False while switching active server to the queue server. */
|
||||
/** When false, skip network fetches (e.g. no server id). */
|
||||
fetchEnabled?: boolean;
|
||||
}
|
||||
|
||||
@@ -96,7 +96,7 @@ export function useNowPlayingFetchers(deps: NowPlayingFetchersDeps): NowPlayingF
|
||||
if (cached !== undefined) { setSongMetaEntry({ id: songId, value: cached }); return; }
|
||||
setSongMetaEntry(null);
|
||||
let cancelled = false;
|
||||
getSong(songId)
|
||||
getSongForServer(subsonicServerId, songId)
|
||||
.then(v => { if (!cancelled) { songMetaCache.set(cacheKey, v ?? null); setSongMetaEntry({ id: songId, value: v ?? null }); } })
|
||||
.catch(() => { if (!cancelled) { songMetaCache.set(cacheKey, null); setSongMetaEntry({ id: songId, value: null }); } });
|
||||
return () => { cancelled = true; };
|
||||
@@ -109,7 +109,7 @@ export function useNowPlayingFetchers(deps: NowPlayingFetchersDeps): NowPlayingF
|
||||
if (cached !== undefined) { setArtistInfoEntry({ id: artistId, value: cached }); return; }
|
||||
setArtistInfoEntry(null);
|
||||
let cancelled = false;
|
||||
getArtistInfo(artistId, { similarArtistCount: audiomuseNavidromeEnabled ? 24 : undefined })
|
||||
getArtistInfoForServer(subsonicServerId, artistId, { similarArtistCount: audiomuseNavidromeEnabled ? 24 : undefined })
|
||||
.then(v => { if (!cancelled) { artistInfoCache.set(cacheKey, v ?? null); setArtistInfoEntry({ id: artistId, value: v ?? null }); } })
|
||||
.catch(() => { if (!cancelled) { artistInfoCache.set(cacheKey, null); setArtistInfoEntry({ id: artistId, value: null }); } });
|
||||
return () => { cancelled = true; };
|
||||
@@ -122,7 +122,7 @@ export function useNowPlayingFetchers(deps: NowPlayingFetchersDeps): NowPlayingF
|
||||
if (cached !== undefined) { setAlbumDataEntry({ id: albumId, value: cached }); return; }
|
||||
setAlbumDataEntry(null);
|
||||
let cancelled = false;
|
||||
getAlbum(albumId)
|
||||
getAlbumForServer(subsonicServerId, albumId)
|
||||
.then(v => { if (!cancelled) { albumCache.set(cacheKey, v); setAlbumDataEntry({ id: albumId, value: v }); } })
|
||||
.catch(() => { if (!cancelled) { albumCache.set(cacheKey, null); setAlbumDataEntry({ id: albumId, value: null }); } });
|
||||
return () => { cancelled = true; };
|
||||
@@ -134,7 +134,7 @@ export function useNowPlayingFetchers(deps: NowPlayingFetchersDeps): NowPlayingF
|
||||
const cached = topSongsCache.get(cacheKey);
|
||||
if (cached !== undefined) { setTopSongs(cached); return; }
|
||||
let cancelled = false;
|
||||
getTopSongs(artistName)
|
||||
getTopSongsForServer(subsonicServerId, artistName)
|
||||
.then(v => { if (!cancelled) { topSongsCache.set(cacheKey, v); setTopSongs(v); } })
|
||||
.catch(() => { if (!cancelled) { topSongsCache.set(cacheKey, []); setTopSongs([]); } });
|
||||
return () => { cancelled = true; };
|
||||
@@ -160,7 +160,7 @@ export function useNowPlayingFetchers(deps: NowPlayingFetchersDeps): NowPlayingF
|
||||
if (cached !== undefined) { setDiscographyEntry({ id: artistId, value: cached }); return; }
|
||||
setDiscographyEntry(null);
|
||||
let cancelled = false;
|
||||
getArtist(artistId)
|
||||
getArtistForServer(subsonicServerId, artistId)
|
||||
.then(v => { if (!cancelled) { discographyCache.set(cacheKey, v.albums); setDiscographyEntry({ id: artistId, value: v.albums }); } })
|
||||
.catch(() => { if (!cancelled) { discographyCache.set(cacheKey, []); setDiscographyEntry({ id: artistId, value: [] }); } });
|
||||
return () => { cancelled = true; };
|
||||
|
||||
@@ -0,0 +1,42 @@
|
||||
import { renderHook } from '@testing-library/react';
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import { useAuthStore } from '../store/authStore';
|
||||
import { usePlayerStore } from '../store/playerStore';
|
||||
import { usePlaybackServerId } from './usePlaybackServerId';
|
||||
|
||||
vi.mock('../utils/server/switchActiveServer', () => ({
|
||||
switchActiveServer: vi.fn(async () => true),
|
||||
}));
|
||||
|
||||
describe('usePlaybackServerId', () => {
|
||||
beforeEach(() => {
|
||||
useAuthStore.setState({
|
||||
servers: [
|
||||
{ id: 'a', name: 'A', url: 'http://a.test', username: 'u', password: 'p' },
|
||||
{ id: 'b', name: 'B', url: 'http://b.test', username: 'u', password: 'p' },
|
||||
],
|
||||
activeServerId: 'a',
|
||||
isLoggedIn: true,
|
||||
});
|
||||
usePlayerStore.setState({
|
||||
queue: [{ id: 't1', title: 'T', artist: 'A', album: 'Al', albumId: 'al1', duration: 100 }],
|
||||
queueServerId: 'a',
|
||||
queueIndex: 0,
|
||||
});
|
||||
});
|
||||
|
||||
it('returns queue server while playback queue is non-empty', () => {
|
||||
useAuthStore.setState({ activeServerId: 'b' });
|
||||
const { result } = renderHook(() => usePlaybackServerId());
|
||||
expect(result.current).toBe('a');
|
||||
});
|
||||
|
||||
it('does not call switchActiveServer when browsed server changes', async () => {
|
||||
const { switchActiveServer } = await import('../utils/server/switchActiveServer');
|
||||
vi.mocked(switchActiveServer).mockClear();
|
||||
const { rerender } = renderHook(() => usePlaybackServerId());
|
||||
useAuthStore.setState({ activeServerId: 'b' });
|
||||
rerender();
|
||||
expect(switchActiveServer).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,18 @@
|
||||
import { useMemo } from 'react';
|
||||
import { useAuthStore } from '../store/authStore';
|
||||
import { usePlayerStore } from '../store/playerStore';
|
||||
import { getPlaybackServerId } from '../utils/playback/playbackServer';
|
||||
|
||||
/**
|
||||
* Subsonic server that owns the current queue / stream (may differ from the browsed
|
||||
* server). Use for Now Playing metadata without calling `ensurePlaybackServerActive`.
|
||||
*/
|
||||
export function usePlaybackServerId(): string {
|
||||
const queueServerId = usePlayerStore(s => s.queueServerId);
|
||||
const queueLength = usePlayerStore(s => s.queue.length);
|
||||
const activeServerId = useAuthStore(s => s.activeServerId);
|
||||
return useMemo(
|
||||
() => getPlaybackServerId(),
|
||||
[queueServerId, queueLength, activeServerId],
|
||||
);
|
||||
}
|
||||
Reference in New Issue
Block a user