diff --git a/CHANGELOG.md b/CHANGELOG.md index ae08597c..ca04eea1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -159,14 +159,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Fixed +### 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)** + +* The "from this album", "discography" and "most played" sections on the Now Playing page disappeared after a track change once the next track started playing from the local cache, and didn't come back. The page now keeps loading that info whenever the server is reachable, regardless of whether the audio plays from cached or offline bytes. + + + ### Library DB — named slow-write ops for stall diagnosis **By [@cucadmuh](https://github.com/cucadmuh), PR [#1043](https://github.com/Psychotoxical/psysonic/pull/1043)** * Production `library-db` write paths now log stable `module.action` op names instead of the generic `misc`, so the next `[library-db] SLOW write` line on macOS (or elsewhere) identifies the call site — diagnostic step for playback stalls under long write-lock holds ([#1040](https://github.com/Psychotoxical/psysonic/issues/1040)). - - ### Servers — complete border on the active server card **By [@Psychotoxical](https://github.com/Psychotoxical), PR [#998](https://github.com/Psychotoxical/psysonic/pull/998)** diff --git a/src/hooks/useNowPlayingFetchers.test.ts b/src/hooks/useNowPlayingFetchers.test.ts index adc88bcb..96aad498 100644 --- a/src/hooks/useNowPlayingFetchers.test.ts +++ b/src/hooks/useNowPlayingFetchers.test.ts @@ -18,9 +18,10 @@ vi.mock('../api/subsonicLibrary'); vi.mock('../api/bandsintown'); vi.mock('../api/lastfm'); vi.mock('../utils/network/subsonicNetworkGuard', () => ({ - shouldAttemptSubsonicForServer: () => true, + shouldAttemptSubsonicForServer: vi.fn(() => true), })); +import { shouldAttemptSubsonicForServer } from '../utils/network/subsonicNetworkGuard'; import { getArtistForServer, getArtistInfoForServer, getTopSongsForServer } from '../api/subsonicArtists'; import { getAlbumForServer, getSongForServer } from '../api/subsonicLibrary'; import { fetchBandsintownEvents } from '../api/bandsintown'; @@ -212,3 +213,53 @@ describe('useNowPlayingFetchers — id-gated songMeta / albumData / discography' await waitFor(() => expect(result.current.discography.map(a => a.id)).toEqual(['al-D2'])); }); }); + +describe('useNowPlayingFetchers — local-playback metadata', () => { + // Regression: the metadata gate must never pass the playing track id, or the + // guard's `psysonic-local://` skip would blank every Subsonic card whenever + // the track plays from hot-cache / offline bytes. Guard is called with the + // server id only. + // Keep the shared guard mock at its permissive default after the behaviour + // case below swaps in a trackId-sensitive implementation. + afterEach(() => { + vi.mocked(shouldAttemptSubsonicForServer).mockImplementation(() => true); + }); + + it('queries the network guard without a trackId', async () => { + const guard = vi.mocked(shouldAttemptSubsonicForServer); + renderHook(() => useNowPlayingFetchers({ ...baseDeps, songId: 'song-1', albumId: 'al-1', artistId: 'art-1', artistName: 'Artist' })); + await waitFor(() => expect(guard).toHaveBeenCalled()); + for (const call of guard.mock.calls) { + expect(call).toHaveLength(1); + expect(call[1]).toBeUndefined(); + } + }); + + it('still loads album / discography / top songs when the playback bytes are local', async () => { + // Mirror the real guard: a byte-style call (with a trackId resolving to + // psysonic-local://) is blocked, but the metadata gate (server id only) is + // allowed. If the hook ever passed the trackId again, every fetch below + // would be gated off and the cards would blank — exactly the #1042 bug. + // Ids are unique to this test so the shared module caches don't short-circuit it. + vi.mocked(shouldAttemptSubsonicForServer).mockImplementation( + (_serverId, trackId) => trackId === undefined, + ); + vi.mocked(getSongForServer).mockResolvedValue({ id: 'np-song', title: 'Local Track' } as SubsonicSong); + vi.mocked(getAlbumForServer).mockResolvedValue( + { album: { id: 'np-al', name: 'Album' } as SubsonicAlbum, songs: [] } as any, + ); + vi.mocked(getArtistForServer).mockResolvedValue({ albums: [{ id: 'np-al' }] } as any); + vi.mocked(getTopSongsForServer).mockResolvedValue([{ id: 'np-top' }] as any); + + const { result } = renderHook(() => + useNowPlayingFetchers({ ...baseDeps, songId: 'np-song', albumId: 'np-al', artistId: 'np-art', artistName: 'NP Artist' }), + ); + + await waitFor(() => expect(getAlbumForServer).toHaveBeenCalledWith('srv1', 'np-al')); + await waitFor(() => expect(getArtistForServer).toHaveBeenCalledWith('srv1', 'np-art')); + await waitFor(() => expect(getTopSongsForServer).toHaveBeenCalledWith('srv1', 'NP Artist')); + await waitFor(() => expect(result.current.albumData?.album.id).toBe('np-al')); + await waitFor(() => expect(result.current.discography.map(a => a.id)).toEqual(['np-al'])); + await waitFor(() => expect(result.current.topSongs.map(s => s.id)).toEqual(['np-top'])); + }); +}); diff --git a/src/hooks/useNowPlayingFetchers.ts b/src/hooks/useNowPlayingFetchers.ts index 90f3869f..871c295f 100644 --- a/src/hooks/useNowPlayingFetchers.ts +++ b/src/hooks/useNowPlayingFetchers.ts @@ -32,7 +32,12 @@ export interface NowPlayingFetchersDeps { currentTrack: { artist: string; title: string } | null; /** Subsonic server for API calls — must match the playing queue server. */ subsonicServerId: string; - /** When false, skip network fetches (e.g. no server id). */ + /** + * Caller intent / prerequisites only (e.g. "we have a playback server id"). + * The network reachability decision — online, server reachable, and no + * trackId so local-cache playback still loads metadata — is made here via + * `shouldAttemptSubsonicForServer`; callers must not pre-apply that guard. + */ fetchEnabled?: boolean; } @@ -81,7 +86,10 @@ export async function prewarmNowPlayingFetchers( } = deps; if (!fetchEnabled || !subsonicServerId) return; - if (!shouldAttemptSubsonicForServer(subsonicServerId, songId)) return; + // The only network gate in this function. No trackId: metadata must be fetched + // even when the track's audio is local (hot-cache / offline). Offline is still + // covered by the online/reachability checks inside the guard. + if (!shouldAttemptSubsonicForServer(subsonicServerId)) return; const jobs: Array> = []; @@ -200,9 +208,11 @@ export function useNowPlayingFetchers(deps: NowPlayingFetchersDeps): NowPlayingF seedKeySlot(lfmArtistKey, k => lfmArtistCache.get(k))); const { status: connStatus } = useConnectionStatus(); + // No trackId: see prewarm note — metadata is fetched whenever the server is + // reachable, regardless of whether the track's audio plays from local cache. const subsonicFetchAllowed = fetchEnabled && !!subsonicServerId - && shouldAttemptSubsonicForServer(subsonicServerId, songId); + && shouldAttemptSubsonicForServer(subsonicServerId); // Fetch batch per entity change (not per song switch — same-artist songs share artist/top/tour fetches) useEffect(() => { diff --git a/src/hooks/useNowPlayingPrewarm.ts b/src/hooks/useNowPlayingPrewarm.ts index efd41ddc..6279b392 100644 --- a/src/hooks/useNowPlayingPrewarm.ts +++ b/src/hooks/useNowPlayingPrewarm.ts @@ -13,7 +13,6 @@ import { useAuthStore } from '../store/authStore'; import { usePlayerStore } from '../store/playerStore'; import { usePlaybackServerId } from './usePlaybackServerId'; import { primaryTrackArtistRef } from '../utils/playback/trackArtistRefs'; -import { shouldAttemptSubsonicForServer } from '../utils/network/subsonicNetworkGuard'; const NOW_PLAYING_COVER_CSS_PX = 800; @@ -65,7 +64,9 @@ export function useNowPlayingPrewarm(): void { lastfmUsername, currentTrack, subsonicServerId: playbackServerId, - fetchEnabled: shouldAttemptSubsonicForServer(playbackServerId, currentTrack.id), + // No `fetchEnabled` / no trackId: prewarmNowPlayingFetchers owns the single + // reachability gate, and metadata must warm even when the track's audio + // plays from local cache. }); if (currentTrack.albumId && currentTrack.id) { diff --git a/src/pages/NowPlaying.tsx b/src/pages/NowPlaying.tsx index 2307156c..0bdc8940 100644 --- a/src/pages/NowPlaying.tsx +++ b/src/pages/NowPlaying.tsx @@ -6,7 +6,6 @@ import type { SubsonicArtistInfo, SubsonicSong } from '../api/subsonicTypes'; import React, { useState, useRef, useEffect, useCallback, useMemo, memo } from 'react'; import { usePlaybackLibraryNavigate } from '../hooks/usePlaybackLibraryNavigate'; import { usePlaybackServerId } from '../hooks/usePlaybackServerId'; -import { shouldAttemptSubsonicForServer } from '../utils/network/subsonicNetworkGuard'; import { useTranslation } from 'react-i18next'; import { Music, ExternalLink, Cast, Users, Radio, Clock, SkipForward, Info, Calendar, Disc3, Play, EyeOff, LayoutGrid, RotateCcw, Eye } from 'lucide-react'; import { open as shellOpen } from '@tauri-apps/plugin-shell'; @@ -94,8 +93,10 @@ export default function NowPlaying() { enableBandsintown, audiomuseNavidromeEnabled, lastfmUsername, currentTrack, subsonicServerId: playbackServerId, - fetchEnabled: Boolean(playbackServerId) - && shouldAttemptSubsonicForServer(playbackServerId, currentTrack?.id), + // `fetchEnabled` = "we have a playback server id". The reachability decision + // (online / server reachable, no trackId so local-cache playback still loads + // metadata) lives in one place inside the hook — see its JSDoc. + fetchEnabled: Boolean(playbackServerId), }); // Star + Last.fm love + their toggle callbacks diff --git a/src/utils/network/subsonicNetworkGuard.test.ts b/src/utils/network/subsonicNetworkGuard.test.ts index 37c83ac0..f740601c 100644 --- a/src/utils/network/subsonicNetworkGuard.test.ts +++ b/src/utils/network/subsonicNetworkGuard.test.ts @@ -37,4 +37,14 @@ describe('shouldAttemptSubsonicForServer', () => { setActiveServerReachable(true); expect(shouldAttemptSubsonicForServer('srv-1', 't1')).toBe(true); }); + + it('bypasses the local-url skip when called without a trackId (metadata gate)', () => { + 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); + // …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); + }); });