diff --git a/src/store/infiniteQueueState.test.ts b/src/store/infiniteQueueState.test.ts new file mode 100644 index 00000000..ad591cdb --- /dev/null +++ b/src/store/infiniteQueueState.test.ts @@ -0,0 +1,29 @@ +import { afterEach, describe, expect, it } from 'vitest'; +import { + _resetInfiniteQueueStateForTest, + isInfiniteQueueFetching, + setInfiniteQueueFetching, +} from './infiniteQueueState'; + +afterEach(() => { + _resetInfiniteQueueStateForTest(); +}); + +describe('infiniteQueueFetching', () => { + it('starts false', () => { + expect(isInfiniteQueueFetching()).toBe(false); + }); + + it('round-trips through set/get', () => { + setInfiniteQueueFetching(true); + expect(isInfiniteQueueFetching()).toBe(true); + setInfiniteQueueFetching(false); + expect(isInfiniteQueueFetching()).toBe(false); + }); + + it('_resetInfiniteQueueStateForTest resets to false', () => { + setInfiniteQueueFetching(true); + _resetInfiniteQueueStateForTest(); + expect(isInfiniteQueueFetching()).toBe(false); + }); +}); diff --git a/src/store/infiniteQueueState.ts b/src/store/infiniteQueueState.ts new file mode 100644 index 00000000..02310c23 --- /dev/null +++ b/src/store/infiniteQueueState.ts @@ -0,0 +1,22 @@ +/** + * Concurrent-fetch guard for the infinite-queue feature. Stops a second + * `buildInfiniteQueueCandidates` request from running while the first + * is still pending — without it, switching tracks quickly while the + * infinite tail is loading would race two enqueue actions and the + * second would clobber the first's results. + */ + +let infiniteQueueFetching = false; + +export function isInfiniteQueueFetching(): boolean { + return infiniteQueueFetching; +} + +export function setInfiniteQueueFetching(value: boolean): void { + infiniteQueueFetching = value; +} + +/** Test-only: reset the guard. */ +export function _resetInfiniteQueueStateForTest(): void { + infiniteQueueFetching = false; +} diff --git a/src/store/playerStore.ts b/src/store/playerStore.ts index d72392c7..74dfacc0 100644 --- a/src/store/playerStore.ts +++ b/src/store/playerStore.ts @@ -142,6 +142,20 @@ import { getPlayGeneration, setIsAudioPaused, } from './engineState'; +import { + addRadioSessionSeen, + clearRadioSessionSeenIds, + deleteRadioSessionSeen, + getCurrentRadioArtistId, + hasRadioSessionSeen, + isRadioFetching, + setCurrentRadioArtistId, + setRadioFetching, +} from './radioSessionState'; +import { + isInfiniteQueueFetching, + setInfiniteQueueFetching, +} from './infiniteQueueState'; // Re-export so TauriEventBridge + persistence test keep their existing // `from './playerStore'` imports. @@ -392,22 +406,6 @@ type NormalizationStatePayload = { // ─── Module-level playback primitives ───────────────────────────────────────── -// Guard against concurrent infinite-queue fetches. -let infiniteQueueFetching = false; -// Guard against concurrent radio top-up fetches. -let radioFetching = false; - -// Artist ID used to start the current radio session — persists across track -// advances so proactive loading works even when songs lack artistId. -let currentRadioArtistId: string | null = null; -// Track ids the current radio session has already enqueued — *including* -// entries that were trimmed off the front of the queue when it grew too long -// (`HISTORY_KEEP` in next()'s top-up path). Without this the queue's own -// id-set wasn't enough to dedupe: a song played 8 tracks ago is gone from -// the queue and the next Last.fm/topSongs response could re-add it. Reset -// on `setRadioArtistId(other)` and on `clearQueue()`. Issue #500. -let radioSessionSeenIds = new Set(); - /** Reload Rust audio to match a queue-undo snapshot (Zustand alone does not move the engine). */ function queueUndoRestoreAudioEngine(opts: { generation: number; @@ -2216,10 +2214,10 @@ export const usePlayerStore = create()( // drift this client off the host or pop the bulk-add modal at // the next track-end fallback. const { infiniteQueueEnabled } = useAuthStore.getState(); - if (infiniteQueueEnabled && repeatMode === 'off' && !infiniteQueueFetching && !isInOrbitSession()) { + if (infiniteQueueEnabled && repeatMode === 'off' && !isInfiniteQueueFetching() && !isInOrbitSession()) { const remainingAuto = queue.slice(nextIdx + 1).filter(t => t.autoAdded).length; if (remainingAuto <= 2) { - infiniteQueueFetching = true; + setInfiniteQueueFetching(true); const existingIds = new Set(get().queue.map(t => t.id)); buildInfiniteQueueCandidates(currentTrack, existingIds, 5).then(newTracks => { // Re-check at resolution time — the user may have joined @@ -2228,19 +2226,19 @@ export const usePlayerStore = create()( if (newTracks.length > 0) { set(state => ({ queue: [...state.queue, ...newTracks] })); } - }).catch(() => {}).finally(() => { infiniteQueueFetching = false; }); + }).catch(() => {}).finally(() => { setInfiniteQueueFetching(false); }); } } // Proactively top up radio tracks when ≤ 2 remain — always, regardless // of infinite queue setting. const nextTrack = queue[nextIdx]; - if (nextTrack.radioAdded && !radioFetching) { + if (nextTrack.radioAdded && !isRadioFetching()) { const remainingRadio = queue.slice(nextIdx + 1).filter(t => t.radioAdded).length; if (remainingRadio <= 2) { - const artistId = nextTrack.artistId ?? currentRadioArtistId ?? null; + const artistId = nextTrack.artistId ?? getCurrentRadioArtistId() ?? null; const artistName = nextTrack.artist; if (artistId) { - radioFetching = true; + setRadioFetching(true); Promise.all([getSimilarSongs2(artistId), getTopSongs(artistName)]) .then(([similar, top]) => { const existingIds = new Set(get().queue.map(t => t.id)); @@ -2253,8 +2251,8 @@ export const usePlayerStore = create()( for (const raw of sourceList) { if (fresh.length >= 10) break; const t = songToTrack(raw); - if (existingIds.has(t.id) || radioSessionSeenIds.has(t.id)) continue; - radioSessionSeenIds.add(t.id); + if (existingIds.has(t.id) || hasRadioSessionSeen(t.id)) continue; + addRadioSessionSeen(t.id); fresh.push({ ...t, radioAdded: true as const }); } if (fresh.length > 0) { @@ -2274,7 +2272,7 @@ export const usePlayerStore = create()( } }) .catch(() => {}) - .finally(() => { radioFetching = false; }); + .finally(() => { setRadioFetching(false); }); } } } @@ -2297,13 +2295,13 @@ export const usePlayerStore = create()( } // Queue exhausted. Check radio first (independent of infinite queue setting), // then infinite queue, then stop. - if (currentTrack?.radioAdded && !radioFetching) { - const artistId = currentTrack.artistId ?? currentRadioArtistId ?? null; + if (currentTrack?.radioAdded && !isRadioFetching()) { + const artistId = currentTrack.artistId ?? getCurrentRadioArtistId() ?? null; if (artistId) { - radioFetching = true; + setRadioFetching(true); Promise.all([getSimilarSongs2(artistId), getTopSongs(currentTrack.artist)]) .then(([similar, top]) => { - radioFetching = false; + setRadioFetching(false); // The user may have joined an Orbit session while this // fetch was in flight — bail without touching the queue. if (isInOrbitSession()) { @@ -2320,8 +2318,8 @@ export const usePlayerStore = create()( for (const raw of sourceList) { if (fresh.length >= 10) break; const t = songToTrack(raw); - if (existingIds.has(t.id) || radioSessionSeenIds.has(t.id)) continue; - radioSessionSeenIds.add(t.id); + if (existingIds.has(t.id) || hasRadioSessionSeen(t.id)) continue; + addRadioSessionSeen(t.id); fresh.push({ ...t, radioAdded: true as const }); } if (fresh.length > 0) { @@ -2335,7 +2333,7 @@ export const usePlayerStore = create()( } }) .catch(() => { - radioFetching = false; + setRadioFetching(false); invoke('audio_stop').catch(console.error); setIsAudioPaused(false); set({ isPlaying: false, progress: 0, buffered: 0, currentTime: 0 }); @@ -2345,11 +2343,11 @@ export const usePlayerStore = create()( } const { infiniteQueueEnabled } = useAuthStore.getState(); if (infiniteQueueEnabled && repeatMode === 'off') { - if (infiniteQueueFetching) return; - infiniteQueueFetching = true; + if (isInfiniteQueueFetching()) return; + setInfiniteQueueFetching(true); const existingIds = new Set(get().queue.map(t => t.id)); buildInfiniteQueueCandidates(currentTrack, existingIds, 5).then(newTracks => { - infiniteQueueFetching = false; + setInfiniteQueueFetching(false); // The user may have joined an Orbit session while this // fetch was in flight — bail without invoking playTrack. if (isInOrbitSession()) { @@ -2368,7 +2366,7 @@ export const usePlayerStore = create()( const newQueue = [...currentQueue, ...newTracks]; get().playTrack(newTracks[0], newQueue, false); }).catch(() => { - infiniteQueueFetching = false; + setInfiniteQueueFetching(false); invoke('audio_stop').catch(console.error); setIsAudioPaused(false); set({ isPlaying: false, progress: 0, buffered: 0, currentTime: 0 }); @@ -2509,18 +2507,18 @@ export const usePlayerStore = create()( }, setRadioArtistId: (artistId) => { - if (artistId !== currentRadioArtistId) { - radioSessionSeenIds = new Set(); + if (artistId !== getCurrentRadioArtistId()) { + clearRadioSessionSeenIds(); } - currentRadioArtistId = artistId; + setCurrentRadioArtistId(artistId); }, enqueueRadio: (tracks, artistId) => { if (artistId !== undefined) { - if (artistId !== currentRadioArtistId) { - radioSessionSeenIds = new Set(); + if (artistId !== getCurrentRadioArtistId()) { + clearRadioSessionSeenIds(); } - currentRadioArtistId = artistId; + setCurrentRadioArtistId(artistId); } pushQueueUndoFromGetter(get); set(state => { @@ -2535,11 +2533,11 @@ export const usePlayerStore = create()( .slice(state.queueIndex + 1) .filter(t => t.radioAdded) .map(t => t.id); - for (const id of droppedRadioIds) radioSessionSeenIds.delete(id); + for (const id of droppedRadioIds) deleteRadioSessionSeen(id); // Capture surviving queue ids in the seen-set so the next radio top-up // can dedupe against the seed track + already-queued non-radio items. - for (const t of beforeAndCurrent) radioSessionSeenIds.add(t.id); - for (const t of upcoming) radioSessionSeenIds.add(t.id); + for (const t of beforeAndCurrent) addRadioSessionSeen(t.id); + for (const t of upcoming) addRadioSessionSeen(t.id); // Drop incoming tracks already seen earlier this session AND // intra-batch duplicates (top + similar Last.fm responses commonly // overlap). The seen-set is mutated inside the loop so a repeated @@ -2547,8 +2545,8 @@ export const usePlayerStore = create()( // the first occurrence (issue #500). const dedupedTracks: Track[] = []; for (const t of tracks) { - if (radioSessionSeenIds.has(t.id)) continue; - radioSessionSeenIds.add(t.id); + if (hasRadioSessionSeen(t.id)) continue; + addRadioSessionSeen(t.id); dedupedTracks.push(t); } // Insert new radio tracks before any autoAdded tracks in the upcoming section. @@ -2612,8 +2610,8 @@ export const usePlayerStore = create()( setIsAudioPaused(false); clearSeekFallbackRetry(); clearSeekDebounce(); clearSeekTarget(); - radioSessionSeenIds = new Set(); - currentRadioArtistId = null; + clearRadioSessionSeenIds(); + setCurrentRadioArtistId(null); set({ queue: [], queueIndex: 0, currentTrack: null, isPlaying: false, progress: 0, buffered: 0, currentTime: 0 }); syncQueueToServer([], null, 0); }, diff --git a/src/store/radioSessionState.test.ts b/src/store/radioSessionState.test.ts new file mode 100644 index 00000000..a4313ffa --- /dev/null +++ b/src/store/radioSessionState.test.ts @@ -0,0 +1,76 @@ +import { afterEach, describe, expect, it } from 'vitest'; +import { + _resetRadioSessionStateForTest, + addRadioSessionSeen, + clearRadioSessionSeenIds, + deleteRadioSessionSeen, + getCurrentRadioArtistId, + hasRadioSessionSeen, + isRadioFetching, + setCurrentRadioArtistId, + setRadioFetching, +} from './radioSessionState'; + +afterEach(() => { + _resetRadioSessionStateForTest(); +}); + +describe('radioFetching', () => { + it('starts false + round-trips through set/get', () => { + expect(isRadioFetching()).toBe(false); + setRadioFetching(true); + expect(isRadioFetching()).toBe(true); + setRadioFetching(false); + expect(isRadioFetching()).toBe(false); + }); +}); + +describe('currentRadioArtistId', () => { + it('starts null + round-trips', () => { + expect(getCurrentRadioArtistId()).toBeNull(); + setCurrentRadioArtistId('artist-1'); + expect(getCurrentRadioArtistId()).toBe('artist-1'); + setCurrentRadioArtistId(null); + expect(getCurrentRadioArtistId()).toBeNull(); + }); +}); + +describe('radioSessionSeenIds', () => { + it('starts empty', () => { + expect(hasRadioSessionSeen('any')).toBe(false); + }); + + it('add + has round-trip', () => { + addRadioSessionSeen('t1'); + expect(hasRadioSessionSeen('t1')).toBe(true); + expect(hasRadioSessionSeen('t2')).toBe(false); + }); + + it('delete removes individual ids without affecting others', () => { + addRadioSessionSeen('t1'); + addRadioSessionSeen('t2'); + deleteRadioSessionSeen('t1'); + expect(hasRadioSessionSeen('t1')).toBe(false); + expect(hasRadioSessionSeen('t2')).toBe(true); + }); + + it('clearRadioSessionSeenIds wipes the set', () => { + addRadioSessionSeen('t1'); + addRadioSessionSeen('t2'); + clearRadioSessionSeenIds(); + expect(hasRadioSessionSeen('t1')).toBe(false); + expect(hasRadioSessionSeen('t2')).toBe(false); + }); +}); + +describe('_resetRadioSessionStateForTest', () => { + it('resets all three pieces of state', () => { + setRadioFetching(true); + setCurrentRadioArtistId('artist-1'); + addRadioSessionSeen('t1'); + _resetRadioSessionStateForTest(); + expect(isRadioFetching()).toBe(false); + expect(getCurrentRadioArtistId()).toBeNull(); + expect(hasRadioSessionSeen('t1')).toBe(false); + }); +}); diff --git a/src/store/radioSessionState.ts b/src/store/radioSessionState.ts new file mode 100644 index 00000000..ca6951c0 --- /dev/null +++ b/src/store/radioSessionState.ts @@ -0,0 +1,66 @@ +/** + * Per-session bookkeeping for the radio-feature (auto-mix tail of + * tracks based on a seed artist): + * + * - **radioFetching** — concurrent-fetch guard. Stops two parallel + * `getSimilarSongs` / `getTopSongs` requests from racing each other + * when both `next()` and the proactive top-up path fire close + * together. + * + * - **currentRadioArtistId** — the seed artist that started the + * current radio session. Survives track advances so subsequent + * top-ups can resolve a new tail even when the now-playing track + * has no `artistId` of its own. + * + * - **radioSessionSeenIds** — every id the current radio session has + * enqueued so far, *including* entries that were trimmed off the + * front of the queue once it grew past `HISTORY_KEEP`. Without this + * set, the queue's own id-set wasn't enough to dedupe: a song + * played 8 tracks ago is gone from the queue and the next + * Last.fm / topSongs response could re-add it (issue #500). Reset + * on `setRadioArtistId(other)` and on `clearQueue()`. + */ + +let radioFetching = false; +let currentRadioArtistId: string | null = null; +let radioSessionSeenIds = new Set(); + +export function isRadioFetching(): boolean { + return radioFetching; +} + +export function setRadioFetching(value: boolean): void { + radioFetching = value; +} + +export function getCurrentRadioArtistId(): string | null { + return currentRadioArtistId; +} + +export function setCurrentRadioArtistId(id: string | null): void { + currentRadioArtistId = id; +} + +export function hasRadioSessionSeen(id: string): boolean { + return radioSessionSeenIds.has(id); +} + +export function addRadioSessionSeen(id: string): void { + radioSessionSeenIds.add(id); +} + +export function deleteRadioSessionSeen(id: string): void { + radioSessionSeenIds.delete(id); +} + +/** Drop every id the current session has remembered — call when the seed artist changes or the queue is cleared. */ +export function clearRadioSessionSeenIds(): void { + radioSessionSeenIds = new Set(); +} + +/** Test-only: reset all three pieces of state. */ +export function _resetRadioSessionStateForTest(): void { + radioFetching = false; + currentRadioArtistId = null; + radioSessionSeenIds = new Set(); +}