mirror of
https://github.com/Psychotoxical/psysonic.git
synced 2026-07-21 23:05:46 +00:00
fix(queue): suspend idle pull after local queue edits (#1132)
This commit is contained in:
@@ -25,6 +25,8 @@ export default function ConnectionIndicator({ status, isLan, serverName }: Props
|
||||
const activeServerId = useAuthStore(s => s.activeServerId);
|
||||
const {
|
||||
ledVariant,
|
||||
localQueueSyncPaused,
|
||||
queueHandoffReason,
|
||||
pullInFlight,
|
||||
syncRingVisible,
|
||||
pullFromActiveServer,
|
||||
@@ -114,7 +116,9 @@ export default function ConnectionIndicator({ status, isLan, serverName }: Props
|
||||
const tooltip = pullInFlight
|
||||
? t('connection.queuePulling')
|
||||
: ledVariant === 'queue-handoff'
|
||||
? t('connection.queuePullHint', { server: serverName })
|
||||
? localQueueSyncPaused && !queueHandoffReason
|
||||
? t('connection.queueLocalEditHint')
|
||||
: t('connection.queuePullHint', { server: serverName })
|
||||
: ledVariant === 'connected'
|
||||
? t('connection.queueSynced')
|
||||
: multi
|
||||
|
||||
@@ -5,15 +5,15 @@ import { useOrbitStore } from '../store/orbitStore';
|
||||
import { usePlayerStore } from '../store/playerStore';
|
||||
import {
|
||||
getPlaybackIdleSinceMs,
|
||||
hasRecentQueueMutation,
|
||||
isIdleQueuePullSuspended,
|
||||
isPlaybackIdleLongEnough,
|
||||
markPlaybackIdle,
|
||||
} from '../store/queuePlaybackIdle';
|
||||
import { hasPendingQueueSync } from '../store/queueSync';
|
||||
import type { ConnectionStatus } from './useConnectionStatus';
|
||||
import { canAutoIdlePlayQueuePull } from './usePlayQueueSyncLedState';
|
||||
|
||||
const IDLE_THRESHOLD_MS = 30_000;
|
||||
const MUTATION_QUIET_MS = 15_000;
|
||||
const POLL_INTERVAL_MS = 10_000;
|
||||
|
||||
/** Background pull when paused/stopped long enough on a single-server, in-sync browse context. */
|
||||
@@ -37,7 +37,8 @@ export function useIdlePlayQueuePull(status: ConnectionStatus) {
|
||||
if (!canAutoIdlePlayQueuePull(status, orbitRole)) return;
|
||||
if (isPlaying) return;
|
||||
if (!isPlaybackIdleLongEnough(IDLE_THRESHOLD_MS)) return;
|
||||
if (hasRecentQueueMutation(MUTATION_QUIET_MS)) return;
|
||||
if (isIdleQueuePullSuspended()) return;
|
||||
if (hasPendingQueueSync()) return;
|
||||
if (!activeServerId) return;
|
||||
|
||||
inFlightRef.current = true;
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { useCallback, useEffect, useMemo, useState } from 'react';
|
||||
import { useCallback, useEffect, useMemo, useState, useSyncExternalStore } from 'react';
|
||||
import { useTranslation } from 'react-i18next';
|
||||
import type { ConnectionStatus } from '../hooks/useConnectionStatus';
|
||||
import { pullPlayQueueFromActiveServer } from '../store/applyServerPlayQueue';
|
||||
@@ -6,6 +6,10 @@ import { useAuthStore } from '../store/authStore';
|
||||
import { useOrbitStore } from '../store/orbitStore';
|
||||
import { usePlayerStore } from '../store/playerStore';
|
||||
import { getPlaybackServerId, queueIsMultiServer } from '../utils/playback/playbackServer';
|
||||
import {
|
||||
getIdleQueuePullSuspendedSnapshot,
|
||||
subscribeIdleQueuePullSuspended,
|
||||
} from '../store/queuePlaybackIdle';
|
||||
import { clearQueueHandoffPending, isQueueHandoffPending } from '../store/queueSyncUiState';
|
||||
import { showToast } from '../utils/ui/toast';
|
||||
|
||||
@@ -13,9 +17,12 @@ export function usePlayQueueSyncLedState(status: ConnectionStatus) {
|
||||
const { t } = useTranslation();
|
||||
const activeServerId = useAuthStore(s => s.activeServerId);
|
||||
const orbitRole = useOrbitStore(s => s.role);
|
||||
const isPlaying = usePlayerStore(s => s.isPlaying);
|
||||
const currentRadio = usePlayerStore(s => s.currentRadio);
|
||||
const [pullInFlight, setPullInFlight] = useState(false);
|
||||
const idlePullSuspended = useSyncExternalStore(
|
||||
subscribeIdleQueuePullSuspended,
|
||||
getIdleQueuePullSuspendedSnapshot,
|
||||
);
|
||||
|
||||
const queueItems = usePlayerStore(s => s.queueItems);
|
||||
const queueIndex = usePlayerStore(s => s.queueIndex);
|
||||
@@ -32,13 +39,22 @@ export function usePlayQueueSyncLedState(status: ConnectionStatus) {
|
||||
}
|
||||
}, [activeServerId, playbackServerId]);
|
||||
|
||||
const autoSyncContext = canAutoIdlePlayQueuePull(status, orbitRole);
|
||||
const localQueueSyncPaused = autoSyncContext && idlePullSuspended;
|
||||
|
||||
const needsQueuePull = status === 'connected'
|
||||
&& Boolean(activeServerId)
|
||||
&& (
|
||||
(Boolean(playbackServerId) && activeServerId !== playbackServerId)
|
||||
|| isQueueHandoffPending()
|
||||
|| localQueueSyncPaused
|
||||
);
|
||||
|
||||
const queueHandoffReason = status === 'connected'
|
||||
&& Boolean(activeServerId)
|
||||
&& Boolean(playbackServerId)
|
||||
&& activeServerId !== playbackServerId;
|
||||
|
||||
const ledVariant = status === 'checking'
|
||||
? 'checking'
|
||||
: status === 'disconnected'
|
||||
@@ -81,6 +97,8 @@ export function usePlayQueueSyncLedState(status: ConnectionStatus) {
|
||||
return {
|
||||
ledVariant,
|
||||
needsQueuePull,
|
||||
localQueueSyncPaused,
|
||||
queueHandoffReason,
|
||||
pullInFlight,
|
||||
syncRingVisible,
|
||||
pullFromActiveServer,
|
||||
|
||||
@@ -37,6 +37,7 @@ export const connection = {
|
||||
switchFailed: 'Could not switch — server unreachable.',
|
||||
queueSynced: 'Queue is in sync with the server',
|
||||
queuePullHint: 'Click to pull the play queue from {{server}}',
|
||||
queueLocalEditHint: 'Play queue changed locally — auto-sync paused. Click to pull from server.',
|
||||
queuePulling: 'Pulling play queue…',
|
||||
queuePullSuccess: 'Play queue updated from server',
|
||||
queuePullEmpty: 'The server play queue is empty',
|
||||
|
||||
@@ -40,6 +40,7 @@ export const connection = {
|
||||
switchFailed: 'Не удалось переключиться — сервер недоступен.',
|
||||
queueSynced: 'Очередь синхронизирована с сервером',
|
||||
queuePullHint: 'Нажмите, чтобы подтянуть очередь с {{server}}',
|
||||
queueLocalEditHint: 'Очередь изменена локально — авто-синхронизация приостановлена. Нажмите, чтобы подтянуть с сервера.',
|
||||
queuePulling: 'Подтягиваем очередь…',
|
||||
queuePullSuccess: 'Очередь обновлена с сервера',
|
||||
queuePullEmpty: 'Очередь на сервере пуста',
|
||||
|
||||
@@ -0,0 +1,114 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import type { QueueItemRef } from './playerStoreTypes';
|
||||
|
||||
const getPlayQueueForServerMock = vi.fn();
|
||||
|
||||
vi.mock('../api/subsonicPlayQueue', () => ({
|
||||
getPlayQueueForServer: (...args: unknown[]) => getPlayQueueForServerMock(...args),
|
||||
}));
|
||||
|
||||
vi.mock('../utils/server/serverLookup', () => ({
|
||||
resolveServerIdForIndexKey: (id: string) => id,
|
||||
}));
|
||||
|
||||
vi.mock('../utils/playback/songToTrack', () => ({
|
||||
songToTrack: (s: { id: string }) => ({
|
||||
id: s.id,
|
||||
title: s.id,
|
||||
artist: '',
|
||||
album: '',
|
||||
albumId: '',
|
||||
duration: 60,
|
||||
serverId: 'srv-a',
|
||||
}),
|
||||
}));
|
||||
|
||||
vi.mock('./pausedRestorePrepare', () => ({
|
||||
preparePausedRestoreOnStartup: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('./waveformRefresh', () => ({
|
||||
refreshWaveformForTrack: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('./queueSyncUiState', () => ({
|
||||
clearQueueHandoffPending: vi.fn(),
|
||||
}));
|
||||
|
||||
const playerState = {
|
||||
queueItems: [] as QueueItemRef[],
|
||||
queueIndex: 0,
|
||||
currentTrack: null as { id: string; title: string; artist: string; album: string; albumId: string; duration: number } | null,
|
||||
currentTime: 0,
|
||||
isPlaying: false,
|
||||
};
|
||||
|
||||
vi.mock('./playerStore', () => ({
|
||||
usePlayerStore: {
|
||||
getState: () => playerState,
|
||||
setState: (partial: Partial<typeof playerState>) => {
|
||||
Object.assign(playerState, partial);
|
||||
},
|
||||
},
|
||||
}));
|
||||
|
||||
import { applyServerPlayQueue } from './applyServerPlayQueue';
|
||||
import {
|
||||
_resetQueuePlaybackIdleForTest,
|
||||
getIdlePullGeneration,
|
||||
isIdleQueuePullSuspended,
|
||||
touchQueueMutationClock,
|
||||
} from './queuePlaybackIdle';
|
||||
|
||||
describe('applyServerPlayQueue idle guards', () => {
|
||||
beforeEach(() => {
|
||||
_resetQueuePlaybackIdleForTest();
|
||||
getPlayQueueForServerMock.mockReset();
|
||||
playerState.queueItems = [{ serverId: 'srv-a', trackId: 'local-only' }];
|
||||
playerState.queueIndex = 0;
|
||||
playerState.currentTrack = {
|
||||
id: 'local-only',
|
||||
title: 'local-only',
|
||||
artist: '',
|
||||
album: '',
|
||||
albumId: '',
|
||||
duration: 60,
|
||||
};
|
||||
playerState.currentTime = 12;
|
||||
playerState.isPlaying = false;
|
||||
});
|
||||
|
||||
it('does not apply server queue in idle mode while local edits suspend pull', async () => {
|
||||
getPlayQueueForServerMock.mockResolvedValue({
|
||||
songs: [{ id: 'remote-a' }, { id: 'remote-b' }],
|
||||
current: 'remote-a',
|
||||
position: 5000,
|
||||
});
|
||||
touchQueueMutationClock();
|
||||
|
||||
const result = await applyServerPlayQueue('srv-a', { mode: 'idle' });
|
||||
|
||||
expect(result).toBe('noop');
|
||||
expect(getPlayQueueForServerMock).not.toHaveBeenCalled();
|
||||
expect(playerState.queueItems).toEqual([{ serverId: 'srv-a', trackId: 'local-only' }]);
|
||||
expect(isIdleQueuePullSuspended()).toBe(true);
|
||||
});
|
||||
|
||||
it('ignores stale idle pull responses after a local mutation during fetch', async () => {
|
||||
const generationAtFetch = getIdlePullGeneration();
|
||||
getPlayQueueForServerMock.mockImplementation(async () => {
|
||||
touchQueueMutationClock();
|
||||
expect(getIdlePullGeneration()).toBe(generationAtFetch + 1);
|
||||
return {
|
||||
songs: [{ id: 'remote-a' }],
|
||||
current: 'remote-a',
|
||||
position: 0,
|
||||
};
|
||||
});
|
||||
|
||||
const result = await applyServerPlayQueue('srv-a', { mode: 'idle' });
|
||||
|
||||
expect(result).toBe('noop');
|
||||
expect(playerState.queueItems).toEqual([{ serverId: 'srv-a', trackId: 'local-only' }]);
|
||||
});
|
||||
});
|
||||
@@ -10,6 +10,11 @@ import { usePlayerStore } from './playerStore';
|
||||
import { preparePausedRestoreOnStartup } from './pausedRestorePrepare';
|
||||
import { pushQueueUndoFromGetter } from './queueUndo';
|
||||
import { refreshWaveformForTrack } from './waveformRefresh';
|
||||
import {
|
||||
getIdlePullGeneration,
|
||||
isIdleQueuePullSuspended,
|
||||
resumeIdleQueuePull,
|
||||
} from './queuePlaybackIdle';
|
||||
import { clearQueueHandoffPending } from './queueSyncUiState';
|
||||
|
||||
export type ApplyPlayQueueMode = 'startup' | 'idle' | 'manual';
|
||||
@@ -125,12 +130,19 @@ export async function applyServerPlayQueue(
|
||||
const profileId = resolveServerProfileId(serverId);
|
||||
if (!profileId) return 'error';
|
||||
|
||||
if (options.mode === 'idle' && isIdleQueuePullSuspended()) {
|
||||
return 'noop';
|
||||
}
|
||||
const idleGenerationAtStart = options.mode === 'idle' ? getIdlePullGeneration() : null;
|
||||
|
||||
try {
|
||||
const q = await getPlayQueueForServer(profileId);
|
||||
if (q.songs.length === 0) return 'empty';
|
||||
|
||||
const preferServerPosition = options.preferServerPosition ?? options.mode !== 'startup';
|
||||
if (options.mode === 'idle') {
|
||||
if (isIdleQueuePullSuspended()) return 'noop';
|
||||
if (idleGenerationAtStart !== getIdlePullGeneration()) return 'noop';
|
||||
const serverFp = fingerprintFromServer(q);
|
||||
const localFp = fingerprintFromLocalQueue();
|
||||
if (playQueueFingerprintsEqual(serverFp, localFp)) return 'noop';
|
||||
@@ -169,17 +181,27 @@ export async function pullPlayQueueFromActiveServer(): Promise<ApplyPlayQueueRes
|
||||
|
||||
try {
|
||||
const q = await getPlayQueueForServer(activeId);
|
||||
if (q.songs.length === 0) return 'empty';
|
||||
if (q.songs.length === 0) {
|
||||
resumeIdleQueuePull();
|
||||
return 'empty';
|
||||
}
|
||||
|
||||
const serverFp = fingerprintFromServer(q);
|
||||
const localFp = fingerprintFromLocalQueue();
|
||||
if (playQueueFingerprintsEqual(serverFp, localFp)) return 'noop';
|
||||
if (playQueueFingerprintsEqual(serverFp, localFp)) {
|
||||
resumeIdleQueuePull();
|
||||
return 'noop';
|
||||
}
|
||||
|
||||
return applyServerPlayQueue(activeId, {
|
||||
const result = await applyServerPlayQueue(activeId, {
|
||||
mode: 'manual',
|
||||
preferServerPosition: true,
|
||||
pushUndo: true,
|
||||
});
|
||||
if (result === 'applied' || result === 'noop') {
|
||||
resumeIdleQueuePull();
|
||||
}
|
||||
return result;
|
||||
} catch (e) {
|
||||
console.error('[psysonic] pullPlayQueueFromActiveServer failed', e);
|
||||
return 'error';
|
||||
|
||||
@@ -0,0 +1,50 @@
|
||||
import { beforeEach, describe, expect, it } from 'vitest';
|
||||
import {
|
||||
_resetQueuePlaybackIdleForTest,
|
||||
getIdlePullGeneration,
|
||||
isIdleQueuePullSuspended,
|
||||
resumeIdleQueuePull,
|
||||
subscribeIdleQueuePullSuspended,
|
||||
touchQueueMutationClock,
|
||||
} from './queuePlaybackIdle';
|
||||
|
||||
describe('idle queue pull suspension', () => {
|
||||
beforeEach(() => {
|
||||
_resetQueuePlaybackIdleForTest();
|
||||
});
|
||||
|
||||
it('starts unsuspended', () => {
|
||||
expect(isIdleQueuePullSuspended()).toBe(false);
|
||||
});
|
||||
|
||||
it('suspends on local queue mutation', () => {
|
||||
touchQueueMutationClock();
|
||||
expect(isIdleQueuePullSuspended()).toBe(true);
|
||||
});
|
||||
|
||||
it('resumes after explicit resume', () => {
|
||||
touchQueueMutationClock();
|
||||
resumeIdleQueuePull();
|
||||
expect(isIdleQueuePullSuspended()).toBe(false);
|
||||
});
|
||||
|
||||
it('bumps idle pull generation on mutation', () => {
|
||||
expect(getIdlePullGeneration()).toBe(0);
|
||||
touchQueueMutationClock();
|
||||
expect(getIdlePullGeneration()).toBe(1);
|
||||
touchQueueMutationClock();
|
||||
expect(getIdlePullGeneration()).toBe(2);
|
||||
});
|
||||
|
||||
it('notifies subscribers when suspension toggles', () => {
|
||||
let count = 0;
|
||||
const unsub = subscribeIdleQueuePullSuspended(() => {
|
||||
count += 1;
|
||||
});
|
||||
touchQueueMutationClock();
|
||||
expect(count).toBe(1);
|
||||
resumeIdleQueuePull();
|
||||
expect(count).toBe(2);
|
||||
unsub();
|
||||
});
|
||||
});
|
||||
@@ -1,7 +1,28 @@
|
||||
/** Timestamps for idle auto-pull guards (play queue sync). */
|
||||
/** Timestamps and flags for idle auto-pull guards (play queue sync). */
|
||||
|
||||
let playbackIdleSinceMs = 0;
|
||||
let lastQueueMutationAt = 0;
|
||||
/** When true, idle auto-pull is disabled until manual pull re-enables it. */
|
||||
let idleQueuePullSuspended = false;
|
||||
/** Bumped on each local queue mutation; stale in-flight idle pulls must not apply. */
|
||||
let idlePullGeneration = 0;
|
||||
|
||||
const idlePullSuspensionListeners = new Set<() => void>();
|
||||
|
||||
function emitIdlePullSuspensionChange(): void {
|
||||
for (const listener of idlePullSuspensionListeners) {
|
||||
listener();
|
||||
}
|
||||
}
|
||||
|
||||
export function subscribeIdleQueuePullSuspended(listener: () => void): () => void {
|
||||
idlePullSuspensionListeners.add(listener);
|
||||
return () => idlePullSuspensionListeners.delete(listener);
|
||||
}
|
||||
|
||||
export function getIdleQueuePullSuspendedSnapshot(): boolean {
|
||||
return idleQueuePullSuspended;
|
||||
}
|
||||
|
||||
export function markPlaybackIdle(): void {
|
||||
if (playbackIdleSinceMs === 0) playbackIdleSinceMs = Date.now();
|
||||
@@ -19,8 +40,30 @@ export function isPlaybackIdleLongEnough(thresholdMs: number): boolean {
|
||||
return playbackIdleSinceMs > 0 && Date.now() - playbackIdleSinceMs >= thresholdMs;
|
||||
}
|
||||
|
||||
export function suspendIdleQueuePull(): void {
|
||||
if (idleQueuePullSuspended) return;
|
||||
idleQueuePullSuspended = true;
|
||||
emitIdlePullSuspensionChange();
|
||||
}
|
||||
|
||||
export function resumeIdleQueuePull(): void {
|
||||
if (!idleQueuePullSuspended) return;
|
||||
idleQueuePullSuspended = false;
|
||||
emitIdlePullSuspensionChange();
|
||||
}
|
||||
|
||||
export function isIdleQueuePullSuspended(): boolean {
|
||||
return idleQueuePullSuspended;
|
||||
}
|
||||
|
||||
export function getIdlePullGeneration(): number {
|
||||
return idlePullGeneration;
|
||||
}
|
||||
|
||||
export function touchQueueMutationClock(): void {
|
||||
lastQueueMutationAt = Date.now();
|
||||
suspendIdleQueuePull();
|
||||
idlePullGeneration += 1;
|
||||
}
|
||||
|
||||
export function getLastQueueMutationAt(): number {
|
||||
@@ -35,4 +78,6 @@ export function hasRecentQueueMutation(withinMs: number): boolean {
|
||||
export function _resetQueuePlaybackIdleForTest(): void {
|
||||
playbackIdleSinceMs = 0;
|
||||
lastQueueMutationAt = 0;
|
||||
idleQueuePullSuspended = false;
|
||||
idlePullGeneration = 0;
|
||||
}
|
||||
|
||||
@@ -39,8 +39,13 @@ import {
|
||||
flushPlayQueuePosition,
|
||||
flushQueueSyncToServer,
|
||||
getLastQueueHeartbeatAt,
|
||||
hasPendingQueueSync,
|
||||
syncQueueToServer,
|
||||
} from './queueSync';
|
||||
import {
|
||||
_resetQueuePlaybackIdleForTest,
|
||||
isIdleQueuePullSuspended,
|
||||
} from './queuePlaybackIdle';
|
||||
|
||||
function track(id: string, serverId = 'srv-a'): Track {
|
||||
return { id, title: id, artist: 'A', album: 'X', albumId: 'X', duration: 100, serverId };
|
||||
@@ -60,6 +65,7 @@ beforeEach(() => {
|
||||
playerState.currentTrack = null;
|
||||
playerState.currentRadio = null;
|
||||
progressSnapshot.currentTime = 0;
|
||||
_resetQueuePlaybackIdleForTest();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
@@ -89,6 +95,24 @@ describe('syncQueueToServer (debounced)', () => {
|
||||
vi.advanceTimersByTime(5000);
|
||||
expect(savePlayQueueMock).toHaveBeenCalledWith(['a'], 'a', 12000, 'srv-a');
|
||||
});
|
||||
|
||||
it('suspends idle pull on mutation and stays suspended after successful debounced push', async () => {
|
||||
syncQueueToServer(queue, track('a'), 30);
|
||||
expect(isIdleQueuePullSuspended()).toBe(true);
|
||||
expect(hasPendingQueueSync()).toBe(true);
|
||||
vi.advanceTimersByTime(5000);
|
||||
await Promise.resolve();
|
||||
expect(savePlayQueueMock).toHaveBeenCalled();
|
||||
expect(isIdleQueuePullSuspended()).toBe(true);
|
||||
});
|
||||
|
||||
it('keeps idle pull suspended when debounced push fails', async () => {
|
||||
savePlayQueueMock.mockRejectedValueOnce(new Error('offline'));
|
||||
syncQueueToServer(queue, track('a'), 30);
|
||||
vi.advanceTimersByTime(5000);
|
||||
await Promise.resolve();
|
||||
expect(isIdleQueuePullSuspended()).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('flushPlayQueueForServer', () => {
|
||||
|
||||
@@ -99,6 +99,11 @@ export function flushPlayQueueForServer(serverProfileId: string): Promise<void>
|
||||
return pushRefsForServer(refs, s.currentTrack, currentTime, serverProfileId);
|
||||
}
|
||||
|
||||
/** True while a debounced savePlayQueue is scheduled. */
|
||||
export function hasPendingQueueSync(): boolean {
|
||||
return syncTimeout !== null;
|
||||
}
|
||||
|
||||
/** Last heartbeat timestamp (ms epoch). Used by the playback heartbeat to throttle the 15-second auto-flush cadence. */
|
||||
export function getLastQueueHeartbeatAt(): number {
|
||||
return lastQueueHeartbeatAt;
|
||||
|
||||
Reference in New Issue
Block a user