diff --git a/CHANGELOG.md b/CHANGELOG.md index 74f65e28..d7b550f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -170,6 +170,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Fixed +### Navidrome Now Playing and scrobble with local playback + +**By [@cucadmuh](https://github.com/cucadmuh), PR [#1055](https://github.com/Psychotoxical/psysonic/pull/1055)** + +* **Show in Now Playing** and Navidrome play-count scrobbles no longer silently skip when audio plays from hot cache, offline library pins, or favorites-auto bytes. +* Presence and queue sync target the **playback server** reachability gate, so a queue on server A still reports to Navidrome while browsing server B. + + + ### Now Playing — cards no longer blank out on track change **By [@Psychotoxical](https://github.com/Psychotoxical), PR [#1042](https://github.com/Psychotoxical/psysonic/pull/1042)** diff --git a/src/api/subsonicScrobble.test.ts b/src/api/subsonicScrobble.test.ts index 7478bf75..ca81017b 100644 --- a/src/api/subsonicScrobble.test.ts +++ b/src/api/subsonicScrobble.test.ts @@ -1,7 +1,8 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { useAuthStore } from '../store/authStore'; import { usePlayerStore } from '../store/playerStore'; -import { scrobbleSong } from './subsonicScrobble'; +import { reportNowPlaying, scrobbleSong } from './subsonicScrobble'; +import { shouldAttemptSubsonicForServer } from '../utils/network/subsonicNetworkGuard'; const { apiForServerMock } = vi.hoisted(() => ({ apiForServerMock: vi.fn(async () => ({})), @@ -12,12 +13,13 @@ vi.mock('./subsonicClient', () => ({ apiForServer: apiForServerMock, })); vi.mock('../utils/network/subsonicNetworkGuard', () => ({ - shouldAttemptSubsonicForServer: () => true, + shouldAttemptSubsonicForServer: vi.fn(() => true), })); describe('subsonicScrobble', () => { beforeEach(() => { apiForServerMock.mockClear(); + vi.mocked(shouldAttemptSubsonicForServer).mockImplementation(() => true); useAuthStore.setState({ servers: [ { id: 'a', name: 'A', url: 'http://a.test', username: 'u', password: 'p' }, @@ -41,4 +43,29 @@ describe('subsonicScrobble', () => { expect.objectContaining({ id: 't1', submission: true, time: 1_700_000_000_000 }), ); }); + + it('reportNowPlaying and scrobbleSong use the presence guard without trackId', async () => { + vi.mocked(shouldAttemptSubsonicForServer).mockImplementation( + (_serverId: string, trackId?: string) => trackId === undefined, + ); + + await reportNowPlaying('t-local', 'a'); + await scrobbleSong('t-local', 1_700_000_000_000, 'a'); + + expect(shouldAttemptSubsonicForServer).toHaveBeenCalledWith('a'); + expect(shouldAttemptSubsonicForServer).not.toHaveBeenCalledWith('a', expect.anything()); + expect(apiForServerMock).toHaveBeenCalledTimes(2); + expect(apiForServerMock).toHaveBeenNthCalledWith( + 1, + 'a', + 'scrobble.view', + expect.objectContaining({ id: 't-local', submission: false }), + ); + expect(apiForServerMock).toHaveBeenNthCalledWith( + 2, + 'a', + 'scrobble.view', + expect.objectContaining({ id: 't-local', submission: true }), + ); + }); }); diff --git a/src/api/subsonicScrobble.ts b/src/api/subsonicScrobble.ts index 5333abd6..98e3ffbc 100644 --- a/src/api/subsonicScrobble.ts +++ b/src/api/subsonicScrobble.ts @@ -9,7 +9,9 @@ async function scrobbleOnServer( submission: boolean, time?: number, ): Promise { - if (!shouldAttemptSubsonicForServer(serverId, id)) return; + // Presence / play-count updates are not playback-byte fetches — omit trackId so + // hot cache, offline library, and favorites-auto do not suppress Navidrome calls. + if (!shouldAttemptSubsonicForServer(serverId)) return; const params: Record = { id, submission }; if (time !== undefined) params.time = time; await apiForServer(serverId, 'scrobble.view', params); diff --git a/src/config/settingsCredits.ts b/src/config/settingsCredits.ts index 3deec39e..cd8994e7 100644 --- a/src/config/settingsCredits.ts +++ b/src/config/settingsCredits.ts @@ -159,6 +159,7 @@ const CONTRIBUTOR_ENTRIES = [ 'PsyLab Connections tab with server capability readout, Navidrome admin-role probe, and tab-bar layout fix; server-capability framework with AudioMuse detection via OpenSubsonic sonicSimilarity and sonic Instant Mix routing on Navidrome ≥0.62 (PR #1033)', 'Library DB: named slow-write op labels for macOS playback-stall diagnosis (PR #1043)', 'Settings → Servers: compact two-line cards, capability header badges, unified use/active action, delete in edit form, click-pinned version tooltip (PR #1054)', + 'Navidrome Now Playing and scrobble with hot cache, offline pins, and mixed-server playback reachability (PR #1055)', ], }, { diff --git a/src/store/queueSync.test.ts b/src/store/queueSync.test.ts index 22a865e6..b3b6361e 100644 --- a/src/store/queueSync.test.ts +++ b/src/store/queueSync.test.ts @@ -7,9 +7,9 @@ */ import type { QueueItemRef, Track } from './playerStoreTypes'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -const { savePlayQueueMock, playerState, progressSnapshot, isActiveServerReachableMock } = vi.hoisted(() => ({ +const { savePlayQueueMock, playerState, progressSnapshot, isSubsonicServerReachableMock } = vi.hoisted(() => ({ savePlayQueueMock: vi.fn(async (_ids: string[], _currentId: string | undefined, _pos: number, _serverId: string) => undefined), - isActiveServerReachableMock: vi.fn(() => true), + isSubsonicServerReachableMock: vi.fn((_serverId: string) => true), playerState: { queueItems: [] as QueueItemRef[], currentTrack: null as Track | null, @@ -19,8 +19,8 @@ const { savePlayQueueMock, playerState, progressSnapshot, isActiveServerReachabl })); vi.mock('../api/subsonicPlayQueue', () => ({ savePlayQueue: savePlayQueueMock })); -vi.mock('../utils/network/activeServerReachability', () => ({ - isActiveServerReachable: () => isActiveServerReachableMock(), +vi.mock('../utils/network/subsonicNetworkGuard', () => ({ + isSubsonicServerReachable: (serverId: string) => isSubsonicServerReachableMock(serverId), })); vi.mock('../utils/playback/playbackServer', () => ({ getPlaybackServerId: () => 'srv-a', @@ -52,7 +52,7 @@ function ref(id: string): QueueItemRef { beforeEach(() => { vi.useFakeTimers(); vi.setSystemTime(new Date('2026-05-12T12:00:00Z')); - isActiveServerReachableMock.mockReturnValue(true); + isSubsonicServerReachableMock.mockReturnValue(true); savePlayQueueMock.mockClear(); savePlayQueueMock.mockResolvedValue(undefined); playerState.queueItems = []; @@ -69,8 +69,8 @@ afterEach(() => { describe('syncQueueToServer (debounced)', () => { const queue = [ref('a'), ref('b')]; - it('skips sync while the active server is unreachable', () => { - isActiveServerReachableMock.mockReturnValue(false); + it('skips sync while the playback server is unreachable', () => { + isSubsonicServerReachableMock.mockReturnValue(false); syncQueueToServer(queue, track('a'), 30); vi.advanceTimersByTime(5000); expect(savePlayQueueMock).not.toHaveBeenCalled(); diff --git a/src/store/queueSync.ts b/src/store/queueSync.ts index ada98c59..98daa3c1 100644 --- a/src/store/queueSync.ts +++ b/src/store/queueSync.ts @@ -1,6 +1,6 @@ import { savePlayQueue } from '../api/subsonicPlayQueue'; import type { QueueItemRef, Track } from './playerStoreTypes'; -import { isActiveServerReachable } from '../utils/network/activeServerReachability'; +import { isSubsonicServerReachable } from '../utils/network/subsonicNetworkGuard'; import { getPlaybackServerId } from '../utils/playback/playbackServer'; import { getPlaybackProgressSnapshot } from './playbackProgress'; import { usePlayerStore } from './playerStore'; @@ -27,12 +27,17 @@ const QUEUE_ID_LIMIT = 1000; let syncTimeout: ReturnType | null = null; let lastQueueHeartbeatAt = 0; +function isPlaybackServerReachable(): boolean { + const serverId = getPlaybackServerId(); + return serverId ? isSubsonicServerReachable(serverId) : false; +} + export function syncQueueToServer(queue: QueueItemRef[], currentTrack: Track | null, currentTime: number): void { - if (!isActiveServerReachable()) return; + if (!isPlaybackServerReachable()) return; if (syncTimeout) clearTimeout(syncTimeout); syncTimeout = setTimeout(() => { syncTimeout = null; - if (!isActiveServerReachable()) return; + if (!isPlaybackServerReachable()) return; const ids = queue.slice(0, QUEUE_ID_LIMIT).map(r => r.trackId); const pos = Math.floor(currentTime * 1000); const serverId = getPlaybackServerId(); @@ -47,7 +52,7 @@ export function flushQueueSyncToServer(queue: QueueItemRef[], currentTrack: Trac clearTimeout(syncTimeout); syncTimeout = null; } - if (!isActiveServerReachable()) return Promise.resolve(); + if (!isPlaybackServerReachable()) return Promise.resolve(); if (!currentTrack || queue.length === 0) return Promise.resolve(); lastQueueHeartbeatAt = Date.now(); const ids = queue.slice(0, QUEUE_ID_LIMIT).map(r => r.trackId); diff --git a/src/utils/network/subsonicNetworkGuard.test.ts b/src/utils/network/subsonicNetworkGuard.test.ts index f740601c..c759c671 100644 --- a/src/utils/network/subsonicNetworkGuard.test.ts +++ b/src/utils/network/subsonicNetworkGuard.test.ts @@ -1,4 +1,6 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { useAuthStore } from '../../store/authStore'; +import { resetAuthStore } from '../../test/helpers/storeReset'; import { setActiveServerReachable } from './activeServerReachability'; import { shouldAttemptSubsonicForServer } from './subsonicNetworkGuard'; @@ -13,6 +15,7 @@ vi.mock('../playback/resolvePlaybackUrl', () => ({ describe('shouldAttemptSubsonicForServer', () => { beforeEach(() => { + resetAuthStore(); setActiveServerReachable(null); resolvePlaybackUrlMock.mockReturnValue('https://music.test/stream'); }); @@ -22,8 +25,35 @@ describe('shouldAttemptSubsonicForServer', () => { }); it('returns false when the active server probe failed', () => { + const activeId = useAuthStore.getState().addServer({ + name: 'Active', + url: 'http://active.test', + username: 'u', + password: 'p', + }); + useAuthStore.getState().setActiveServer(activeId); setActiveServerReachable(false); - expect(shouldAttemptSubsonicForServer('srv-1', 't1')).toBe(false); + expect(shouldAttemptSubsonicForServer(activeId, 't1')).toBe(false); + }); + + it('allows a non-active playback server when the active probe failed', () => { + const playbackId = useAuthStore.getState().addServer({ + name: 'Playback', + url: 'http://playback.test', + username: 'u', + password: 'p', + }); + const activeId = useAuthStore.getState().addServer({ + name: 'Browse', + url: 'http://browse.test', + username: 'u', + password: 'p', + }); + useAuthStore.getState().setActiveServer(activeId); + setActiveServerReachable(false); + expect(shouldAttemptSubsonicForServer(playbackId, 't1')).toBe(true); + expect(shouldAttemptSubsonicForServer(playbackId)).toBe(true); + expect(shouldAttemptSubsonicForServer(activeId)).toBe(false); }); it('returns false when the track resolves to a local playback url', () => { @@ -34,17 +64,31 @@ describe('shouldAttemptSubsonicForServer', () => { }); it('returns true for stream playback when the active server is reachable', () => { + const activeId = useAuthStore.getState().addServer({ + name: 'Active', + url: 'http://active.test', + username: 'u', + password: 'p', + }); + useAuthStore.getState().setActiveServer(activeId); setActiveServerReachable(true); - expect(shouldAttemptSubsonicForServer('srv-1', 't1')).toBe(true); + expect(shouldAttemptSubsonicForServer(activeId, 't1')).toBe(true); }); it('bypasses the local-url skip when called without a trackId (metadata gate)', () => { + const activeId = useAuthStore.getState().addServer({ + name: 'Active', + url: 'http://active.test', + username: 'u', + password: 'p', + }); + useAuthStore.getState().setActiveServer(activeId); setActiveServerReachable(true); resolvePlaybackUrlMock.mockReturnValue('psysonic-local:///favorites/t1.flac'); // Byte-style call (with the track id) is blocked because the bytes are local… - expect(shouldAttemptSubsonicForServer('srv-1', 't1')).toBe(false); + expect(shouldAttemptSubsonicForServer(activeId, 't1')).toBe(false); // …but the metadata gate omits the track id, so it never consults // resolvePlaybackUrl and stays allowed while the server is reachable. - expect(shouldAttemptSubsonicForServer('srv-1')).toBe(true); + expect(shouldAttemptSubsonicForServer(activeId)).toBe(true); }); }); diff --git a/src/utils/network/subsonicNetworkGuard.ts b/src/utils/network/subsonicNetworkGuard.ts index 97f748fc..37e636e3 100644 --- a/src/utils/network/subsonicNetworkGuard.ts +++ b/src/utils/network/subsonicNetworkGuard.ts @@ -1,11 +1,34 @@ import { useAuthStore } from '../../store/authStore'; import { resolvePlaybackUrl } from '../playback/resolvePlaybackUrl'; import { isDevOfflineBrowseForced } from '../../store/devOfflineBrowseStore'; +import { resolveServerIdForIndexKey } from '../server/serverLookup'; import { isActiveServerReachable } from './activeServerReachability'; +function isSameServerProfile(a: string, b: string): boolean { + if (!a || !b) return false; + if (a === b) return true; + return resolveServerIdForIndexKey(a) === resolveServerIdForIndexKey(b); +} + +/** + * Whether `serverId` is worth a best-effort Subsonic call while the browser is + * online. The active profile uses the connection-status probe; other profiles + * (e.g. queue playback while browsing another server) attempt optimistically. + */ +export function isSubsonicServerReachable(serverId: string): boolean { + if (!serverId) return false; + if (isDevOfflineBrowseForced()) return false; + if (typeof navigator !== 'undefined' && !navigator.onLine) return false; + const activeId = useAuthStore.getState().activeServerId; + if (!activeId || isSameServerProfile(serverId, activeId)) { + return isActiveServerReachable(); + } + return true; +} + /** * Whether a Subsonic API call for `serverId` is worth attempting. - * Skips when the browser or active server is down, or when the track already + * Skips when the browser or target server is down, or when the track already * plays from a local `psysonic-local://` URL (offline / favorite-auto bytes). */ export function shouldAttemptSubsonicForServer(serverId: string, trackId?: string): boolean { @@ -16,7 +39,7 @@ export function shouldAttemptSubsonicForServer(serverId: string, trackId?: strin const url = resolvePlaybackUrl(trackId, serverId); if (url.startsWith('psysonic-local://')) return false; } - return isActiveServerReachable(); + return isSubsonicServerReachable(serverId); } /** Convenience for call sites that only know the active server id. */