mirror of
https://github.com/Psychotoxical/psysonic.git
synced 2026-07-22 15:25:46 +00:00
fix(lyrics): preserve playback server ownership
This commit is contained in:
@@ -6,6 +6,7 @@ import { useOfflineStore } from '@/features/offline';
|
||||
import { lyricsCache } from '@/features/lyrics/hooks/useLyrics';
|
||||
import { useLyrics } from '@/features/lyrics/hooks/useLyrics';
|
||||
import type { Track } from '@/lib/media/trackTypes';
|
||||
import { lyricsCacheKey } from '@/features/lyrics/utils/lyricsPersistentCache';
|
||||
|
||||
/**
|
||||
* The embedded path reads a local file's tags through Rust and never touches the
|
||||
@@ -68,4 +69,25 @@ describe('useLyrics — embedded Enhanced LRC', () => {
|
||||
expect(result.current.syncedLines).toHaveLength(2);
|
||||
expect(result.current.wordLines).toBeNull();
|
||||
});
|
||||
|
||||
it('reads local media and RAM cache from the playing track owner', async () => {
|
||||
const getLocalUrl = vi.fn(() => 'psysonic-local://C:/music/song.flac');
|
||||
vi.spyOn(useOfflineStore, 'getState').mockReturnValue({
|
||||
getLocalUrl,
|
||||
} as unknown as ReturnType<typeof useOfflineStore.getState>);
|
||||
onInvoke('get_embedded_lyrics', () => PLAIN_LRC);
|
||||
const ownedTrack = { ...track, serverId: 'srv-owner' };
|
||||
|
||||
const { result, unmount } = renderHook(() => useLyrics(ownedTrack));
|
||||
await waitFor(() => expect(result.current.source).toBe('embedded'));
|
||||
expect(getLocalUrl).toHaveBeenCalledWith('track-1', 'srv-owner');
|
||||
expect(lyricsCache.has(lyricsCacheKey('srv-owner', 'track-1'))).toBe(true);
|
||||
expect(lyricsCache.has(lyricsCacheKey('srv-a', 'track-1'))).toBe(false);
|
||||
unmount();
|
||||
|
||||
useAuthStore.setState({ activeServerId: 'srv-b' });
|
||||
const { result: cachedResult } = renderHook(() => useLyrics(ownedTrack));
|
||||
await waitFor(() => expect(cachedResult.current.source).toBe('embedded'));
|
||||
expect(getLocalUrl).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,100 @@
|
||||
import { act, renderHook, waitFor } from '@testing-library/react';
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import type { Track } from '@/lib/media/trackTypes';
|
||||
import { useAuthStore } from '@/store/authStore';
|
||||
import { lyricsCache, useLyrics } from '@/features/lyrics/hooks/useLyrics';
|
||||
import { lyricsCacheKey } from '@/features/lyrics/utils/lyricsPersistentCache';
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
getLyricsBySongId: vi.fn(),
|
||||
getCachedLyrics: vi.fn(),
|
||||
putCachedLyrics: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('@/lib/api/subsonicLyrics', () => ({ getLyricsBySongId: mocks.getLyricsBySongId }));
|
||||
vi.mock('@/features/lyrics/utils/lyricsPersistentCache', async importOriginal => {
|
||||
const actual = await importOriginal<typeof import('@/features/lyrics/utils/lyricsPersistentCache')>();
|
||||
return {
|
||||
...actual,
|
||||
getCachedLyrics: mocks.getCachedLyrics,
|
||||
putCachedLyrics: mocks.putCachedLyrics,
|
||||
};
|
||||
});
|
||||
|
||||
const track: Track = {
|
||||
id: 'shared-song-id',
|
||||
title: 'Owned song',
|
||||
artist: 'Artist',
|
||||
album: 'Album',
|
||||
albumId: 'album-1',
|
||||
duration: 180,
|
||||
serverId: 'srv-owner',
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
lyricsCache.clear();
|
||||
mocks.getLyricsBySongId.mockReset();
|
||||
mocks.getCachedLyrics.mockReset().mockResolvedValue(null);
|
||||
mocks.putCachedLyrics.mockReset().mockResolvedValue(undefined);
|
||||
useAuthStore.setState({
|
||||
activeServerId: 'srv-active',
|
||||
servers: [],
|
||||
youLyPlusEnabled: false,
|
||||
lyricsSources: [
|
||||
{ id: 'server', enabled: true },
|
||||
{ id: 'lrclib', enabled: false },
|
||||
{ id: 'netease', enabled: false },
|
||||
],
|
||||
});
|
||||
});
|
||||
|
||||
describe('useLyrics owner scope', () => {
|
||||
it('keeps identical raw song ids in separate RAM buckets', async () => {
|
||||
lyricsCache.set(lyricsCacheKey('srv-owner', 'shared-song-id'), {
|
||||
syncedLines: null,
|
||||
wordLines: null,
|
||||
plainLyrics: 'Owner lyrics',
|
||||
source: 'server',
|
||||
notFound: false,
|
||||
});
|
||||
lyricsCache.set(lyricsCacheKey('srv-other', 'shared-song-id'), {
|
||||
syncedLines: null,
|
||||
wordLines: null,
|
||||
plainLyrics: 'Other lyrics',
|
||||
source: 'lrclib',
|
||||
notFound: false,
|
||||
});
|
||||
|
||||
const { result, rerender } = renderHook(
|
||||
({ current }) => useLyrics(current),
|
||||
{ initialProps: { current: track } },
|
||||
);
|
||||
await waitFor(() => expect(result.current.plainLyrics).toBe('Owner lyrics'));
|
||||
|
||||
rerender({ current: { ...track, serverId: 'srv-other' } });
|
||||
await waitFor(() => expect(result.current.plainLyrics).toBe('Other lyrics'));
|
||||
expect(mocks.getLyricsBySongId).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('keeps an in-flight server fetch and cache write pinned to the track owner', async () => {
|
||||
let resolveLyrics!: (value: unknown) => void;
|
||||
mocks.getLyricsBySongId.mockReturnValue(new Promise(resolve => { resolveLyrics = resolve; }));
|
||||
|
||||
const { result } = renderHook(() => useLyrics(track));
|
||||
await waitFor(() => expect(mocks.getLyricsBySongId).toHaveBeenCalledWith(
|
||||
'shared-song-id',
|
||||
{ enhanced: false, serverId: 'srv-owner' },
|
||||
));
|
||||
|
||||
act(() => useAuthStore.setState({ activeServerId: 'srv-other' }));
|
||||
resolveLyrics({ line: [{ start: 0, value: 'Owner lyrics' }], synced: true });
|
||||
|
||||
await waitFor(() => expect(result.current.source).toBe('server'));
|
||||
expect(mocks.putCachedLyrics).toHaveBeenCalledWith(
|
||||
lyricsCacheKey('srv-owner', 'shared-song-id'),
|
||||
expect.objectContaining({ source: 'server', notFound: false }),
|
||||
);
|
||||
expect(lyricsCache.has(lyricsCacheKey('srv-owner', 'shared-song-id'))).toBe(true);
|
||||
expect(lyricsCache.has(lyricsCacheKey('srv-other', 'shared-song-id'))).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -15,6 +15,7 @@ import { parseStructuredLyrics, parseStructuredWordLines } from '@/features/lyri
|
||||
import { FEATURE_ENHANCED_LYRICS } from '@/lib/serverCapabilities/catalog';
|
||||
import { isFeatureActiveForServer } from '@/lib/serverCapabilities/storeView';
|
||||
import type { CachedLyrics, LrcLine, LyricsSource, WordLyricsLine } from '@/features/lyrics/types';
|
||||
import { playbackCacheKeyForTrack, playbackProfileIdForTrack } from '@/features/playback';
|
||||
|
||||
// L1 cache: RAM, survives tab switches and component remount within a session.
|
||||
// L2 (IndexedDB) lives in `utils/lyricsPersistentCache.ts` — only touched on
|
||||
@@ -37,7 +38,10 @@ export function useLyrics(currentTrack: Track | null): UseLyricsResult {
|
||||
})));
|
||||
// Lyrics are fully off when YouLyPlus is off and no source is enabled.
|
||||
const lyricsActive = youLyPlusEnabled || lyricsSources.some(s => s.enabled);
|
||||
const cached = (currentTrack && lyricsActive) ? lyricsCache.get(currentTrack.id) : undefined;
|
||||
const ownerServerKey = currentTrack ? playbackCacheKeyForTrack(currentTrack) : '';
|
||||
const ownerServerId = currentTrack ? playbackProfileIdForTrack(currentTrack) : '';
|
||||
const cacheKey = currentTrack ? lyricsCacheKey(ownerServerKey, currentTrack.id) : '';
|
||||
const cached = (currentTrack && lyricsActive) ? lyricsCache.get(cacheKey) : undefined;
|
||||
|
||||
const [loading, setLoading] = useState(!cached && !!currentTrack);
|
||||
const [syncedLines, setSyncedLines] = useState<LrcLine[] | null>(cached?.syncedLines ?? null);
|
||||
@@ -64,7 +68,7 @@ export function useLyrics(currentTrack: Track | null): UseLyricsResult {
|
||||
return;
|
||||
}
|
||||
|
||||
const hit = lyricsCache.get(currentTrack.id);
|
||||
const hit = lyricsCache.get(cacheKey);
|
||||
if (hit) {
|
||||
setSyncedLines(hit.syncedLines);
|
||||
setWordLines(hit.wordLines);
|
||||
@@ -85,7 +89,7 @@ export function useLyrics(currentTrack: Track | null): UseLyricsResult {
|
||||
|
||||
const applyEntry = (entry: CachedLyrics) => {
|
||||
if (cancelled) return;
|
||||
lyricsCache.set(currentTrack.id, entry);
|
||||
lyricsCache.set(cacheKey, entry);
|
||||
setSyncedLines(entry.syncedLines);
|
||||
setWordLines(entry.wordLines);
|
||||
setPlainLyrics(entry.plainLyrics);
|
||||
@@ -98,8 +102,7 @@ export function useLyrics(currentTrack: Track | null): UseLyricsResult {
|
||||
if (cancelled) return;
|
||||
applyEntry(entry);
|
||||
// Persist for the next session (fire-and-forget — failures are silent).
|
||||
const serverId = useAuthStore.getState().activeServerId ?? '';
|
||||
putCachedLyrics(lyricsCacheKey(serverId, currentTrack.id), entry);
|
||||
putCachedLyrics(cacheKey, entry);
|
||||
};
|
||||
|
||||
// For offline / hot-cached tracks we have the file locally — read SYLT /
|
||||
@@ -107,10 +110,9 @@ export function useLyrics(currentTrack: Track | null): UseLyricsResult {
|
||||
// Fast path: both store lookups are synchronous; returns false immediately
|
||||
// for streaming tracks so it has zero impact on the normal fetch sequence.
|
||||
const fetchEmbedded = async (): Promise<boolean> => {
|
||||
const serverId = useAuthStore.getState().activeServerId ?? '';
|
||||
const localUrl =
|
||||
useOfflineStore.getState().getLocalUrl(currentTrack.id, serverId) ??
|
||||
useHotCacheStore.getState().getLocalUrl(currentTrack.id, serverId);
|
||||
useOfflineStore.getState().getLocalUrl(currentTrack.id, ownerServerKey) ??
|
||||
useHotCacheStore.getState().getLocalUrl(currentTrack.id, ownerServerKey);
|
||||
if (!localUrl) return false;
|
||||
|
||||
const prefix = 'psysonic-local://';
|
||||
@@ -138,10 +140,12 @@ export function useLyrics(currentTrack: Track | null): UseLyricsResult {
|
||||
const fetchServer = async (): Promise<boolean> => {
|
||||
// `songLyrics` v2 adds word-level cues, but only where the catalog says the
|
||||
// server speaks it. On a v1 server this stays a plain v1 request.
|
||||
const serverId = useAuthStore.getState().activeServerId ?? '';
|
||||
const enhanced = !!serverId && isFeatureActiveForServer(serverId, FEATURE_ENHANCED_LYRICS);
|
||||
const enhanced = !!ownerServerId && isFeatureActiveForServer(ownerServerId, FEATURE_ENHANCED_LYRICS);
|
||||
|
||||
const structured = await getLyricsBySongId(currentTrack.id, { enhanced });
|
||||
const structured = await getLyricsBySongId(currentTrack.id, {
|
||||
enhanced,
|
||||
serverId: ownerServerId || undefined,
|
||||
});
|
||||
if (!structured) return false;
|
||||
const parsed = parseStructuredLyrics(structured);
|
||||
if (!parsed.syncedLines && !parsed.plainLyrics) return false;
|
||||
@@ -245,12 +249,11 @@ export function useLyrics(currentTrack: Track | null): UseLyricsResult {
|
||||
// the standard pipeline (no word-level sync) and the user explicitly
|
||||
// wants a fresh lyricsplus attempt.
|
||||
if (!youLyPlusEnabled) {
|
||||
const serverId = useAuthStore.getState().activeServerId ?? '';
|
||||
const persisted = await getCachedLyrics(lyricsCacheKey(serverId, currentTrack.id));
|
||||
const persisted = await getCachedLyrics(cacheKey);
|
||||
if (cancelled) return;
|
||||
if (persisted) {
|
||||
// Don't re-write to L2 (it's already there); just hydrate RAM + UI.
|
||||
lyricsCache.set(currentTrack.id, persisted);
|
||||
lyricsCache.set(cacheKey, persisted);
|
||||
applyEntry(persisted);
|
||||
return;
|
||||
}
|
||||
@@ -274,7 +277,7 @@ export function useLyrics(currentTrack: Track | null): UseLyricsResult {
|
||||
})();
|
||||
|
||||
return () => { cancelled = true; };
|
||||
}, [currentTrack?.id, lyricsSources, youLyPlusEnabled]); // eslint-disable-line react-hooks/exhaustive-deps
|
||||
}, [cacheKey, currentTrack?.id, lyricsSources, ownerServerId, ownerServerKey, youLyPlusEnabled]); // eslint-disable-line react-hooks/exhaustive-deps
|
||||
|
||||
return { syncedLines, wordLines, plainLyrics, source, loading, notFound };
|
||||
}
|
||||
|
||||
@@ -8,5 +8,9 @@ export { seedQueueResolver } from './store/queueTrackResolver';
|
||||
export { queueSongStar } from './store/pendingStarSync';
|
||||
export { getPlaybackProgressSnapshot, subscribePlaybackProgress } from './store/playbackProgress';
|
||||
export type { PlaybackProgressSnapshot } from './store/playbackProgress';
|
||||
export { playbackCoverArtForAlbum } from './utils/playback/playbackServer';
|
||||
export {
|
||||
playbackCacheKeyForTrack,
|
||||
playbackCoverArtForAlbum,
|
||||
playbackProfileIdForTrack,
|
||||
} from './utils/playback/playbackServer';
|
||||
export { useVolumeToggle } from './hooks/useVolumeToggle';
|
||||
|
||||
@@ -1,9 +1,12 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import type { SubsonicStructuredLyrics } from '@/lib/api/subsonicTypes';
|
||||
|
||||
const { apiMock } = vi.hoisted(() => ({ apiMock: vi.fn() }));
|
||||
const { apiMock, apiForServerMock } = vi.hoisted(() => ({
|
||||
apiMock: vi.fn(),
|
||||
apiForServerMock: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('@/lib/api/subsonicClient', () => ({ api: apiMock }));
|
||||
vi.mock('@/lib/api/subsonicClient', () => ({ api: apiMock, apiForServer: apiForServerMock }));
|
||||
|
||||
import { getLyricsBySongId, isMainLyricsKind, pickMainStructuredLyrics } from '@/lib/api/subsonicLyrics';
|
||||
|
||||
@@ -13,6 +16,7 @@ function lyrics(overrides: Partial<SubsonicStructuredLyrics> = {}): SubsonicStru
|
||||
|
||||
beforeEach(() => {
|
||||
apiMock.mockReset();
|
||||
apiForServerMock.mockReset();
|
||||
});
|
||||
|
||||
describe('isMainLyricsKind', () => {
|
||||
@@ -69,6 +73,17 @@ describe('getLyricsBySongId', () => {
|
||||
expect(apiMock).toHaveBeenCalledWith('getLyricsBySongId.view', { id: 'song-1', enhanced: true });
|
||||
});
|
||||
|
||||
it('uses the explicit owning server without consulting the active client', async () => {
|
||||
apiForServerMock.mockResolvedValue({ lyricsList: { structuredLyrics: [lyrics()] } });
|
||||
await getLyricsBySongId('song-1', { serverId: 'srv-owner' });
|
||||
expect(apiForServerMock).toHaveBeenCalledWith(
|
||||
'srv-owner',
|
||||
'getLyricsBySongId.view',
|
||||
{ id: 'song-1' },
|
||||
);
|
||||
expect(apiMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('returns null when the track has no lyrics', async () => {
|
||||
apiMock.mockResolvedValue({ lyricsList: {} });
|
||||
await expect(getLyricsBySongId('song-1')).resolves.toBeNull();
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { api } from '@/lib/api/subsonicClient';
|
||||
import { api, apiForServer } from '@/lib/api/subsonicClient';
|
||||
import type { SubsonicStructuredLyrics } from '@/lib/api/subsonicTypes';
|
||||
|
||||
export interface GetLyricsOptions {
|
||||
@@ -8,6 +8,8 @@ export interface GetLyricsOptions {
|
||||
* unknown query parameter is not guaranteed to be ignored by every server.
|
||||
*/
|
||||
enhanced?: boolean;
|
||||
/** Explicit owning server for mixed-server playback. */
|
||||
serverId?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -43,13 +45,18 @@ export function pickMainStructuredLyrics(
|
||||
*/
|
||||
export async function getLyricsBySongId(
|
||||
id: string,
|
||||
{ enhanced = false }: GetLyricsOptions = {},
|
||||
{ enhanced = false, serverId }: GetLyricsOptions = {},
|
||||
): Promise<SubsonicStructuredLyrics | null> {
|
||||
try {
|
||||
const data = await api<{ lyricsList: { structuredLyrics?: SubsonicStructuredLyrics[] } }>(
|
||||
'getLyricsBySongId.view',
|
||||
enhanced ? { id, enhanced: true } : { id },
|
||||
);
|
||||
const endpoint = 'getLyricsBySongId.view';
|
||||
const params = enhanced ? { id, enhanced: true } : { id };
|
||||
const data = serverId
|
||||
? await apiForServer<{ lyricsList: { structuredLyrics?: SubsonicStructuredLyrics[] } }>(
|
||||
serverId,
|
||||
endpoint,
|
||||
params,
|
||||
)
|
||||
: await api<{ lyricsList: { structuredLyrics?: SubsonicStructuredLyrics[] } }>(endpoint, params);
|
||||
return pickMainStructuredLyrics(data.lyricsList?.structuredLyrics ?? []);
|
||||
} catch {
|
||||
// Server doesn't support the endpoint or track has no embedded lyrics
|
||||
|
||||
Reference in New Issue
Block a user