diff --git a/.github/frontend-hot-path-files.txt b/.github/frontend-hot-path-files.txt index d9c4ce2a..b5a321a2 100644 --- a/.github/frontend-hot-path-files.txt +++ b/.github/frontend-hot-path-files.txt @@ -20,9 +20,9 @@ # # Deferred from the gate, with current coverage shown for reference: # - src/store/previewStore.ts (33 % — only `_on*` + `stopPreview` covered, Phase 4 target) -# - src/store/playerStore.ts (0 % — 3700-LOC monolith, Phase 1 target) -# - src/store/authStore.ts (18 % — Phase 2 target) -# - src/api/subsonic.ts (Phase 3 target) +# - src/store/playerStore.ts (40 % — F1 closed under the 50 % floor; further coverage TBD) +# - src/store/authStore.ts (79 % — F2 cleared 60 % floor; staying out one or two PRs to verify stability) +# - src/api/subsonic.ts (13 % — F3 covered the URL-builder + parser surface; async API endpoints need axios mocking, deferred) # ── utils (already at or above threshold) ──────────────────────────── src/utils/coverArtRegisteredSizes.ts @@ -30,3 +30,5 @@ src/utils/serverDisplayName.ts src/utils/serverMagicString.ts src/utils/shareLink.ts src/utils/dynamicColors.ts +src/utils/resolvePlaybackUrl.ts +src/utils/copyEntityShareLink.ts diff --git a/src/api/subsonic.contract.test.ts b/src/api/subsonic.contract.test.ts new file mode 100644 index 00000000..a0f649d7 --- /dev/null +++ b/src/api/subsonic.contract.test.ts @@ -0,0 +1,207 @@ +/** + * Subsonic API contract tests (Phase F3). + * + * Pins the URL-builder shapes (`buildStreamUrl`, `buildCoverArtUrl`, + * `buildDownloadUrl`, `coverArtCacheKey`) plus the small pure parsers + * (`parseSubsonicEntityStarRating`, `libraryFilterParams`, + * `getClient`) — the surface the rest of the frontend uses to talk to + * Subsonic / Navidrome. + * + * Network-bound endpoints (`getAlbum`, `search`, etc.) require axios + * mocking and are not in this PR. + */ +import { beforeEach, describe, expect, it } from 'vitest'; +import { + buildCoverArtUrl, + buildDownloadUrl, + buildStreamUrl, + coverArtCacheKey, + getClient, + libraryFilterParams, + parseSubsonicEntityStarRating, +} from './subsonic'; +import { useAuthStore } from '@/store/authStore'; +import { resetAuthStore } from '@/test/helpers/storeReset'; + +function setUpServer(overrides: { url?: string; username?: string; password?: string } = {}): string { + const id = useAuthStore.getState().addServer({ + name: 'Test', + url: overrides.url ?? 'https://music.example.com', + username: overrides.username ?? 'alice', + password: overrides.password ?? 'pw', + }); + useAuthStore.getState().setActiveServer(id); + return id; +} + +beforeEach(() => { + resetAuthStore(); +}); + +describe('parseSubsonicEntityStarRating', () => { + it('reads userRating first, falls back to rating', () => { + expect(parseSubsonicEntityStarRating({ userRating: 4 })).toBe(4); + expect(parseSubsonicEntityStarRating({ rating: 3 })).toBe(3); + expect(parseSubsonicEntityStarRating({ userRating: 5, rating: 2 })).toBe(5); + }); + + it('coerces numeric strings', () => { + expect(parseSubsonicEntityStarRating({ userRating: '4' as unknown as number })).toBe(4); + expect(parseSubsonicEntityStarRating({ rating: '3.5' as unknown as number })).toBe(3.5); + }); + + it('returns undefined for null / undefined / non-numeric values', () => { + expect(parseSubsonicEntityStarRating({})).toBeUndefined(); + expect(parseSubsonicEntityStarRating({ userRating: null as unknown as number })).toBeUndefined(); + expect(parseSubsonicEntityStarRating({ rating: 'nope' as unknown as number })).toBeUndefined(); + expect(parseSubsonicEntityStarRating({ userRating: Number.NaN })).toBeUndefined(); + }); +}); + +describe('libraryFilterParams', () => { + it('returns an empty object when no server is active', () => { + expect(libraryFilterParams()).toEqual({}); + }); + + it('returns an empty object when the filter is "all" or unset', () => { + setUpServer(); + expect(libraryFilterParams()).toEqual({}); + }); + + it('returns { musicFolderId } when the active server has a specific filter', () => { + const serverId = setUpServer(); + useAuthStore.setState({ + musicLibraryFilterByServer: { [serverId]: 'mf-7' }, + }); + expect(libraryFilterParams()).toEqual({ musicFolderId: 'mf-7' }); + }); +}); + +describe('getClient', () => { + it('throws when no server is configured', () => { + expect(() => getClient()).toThrow(/no server configured/i); + }); + + it('returns baseUrl + auth params shape from the active server', () => { + setUpServer({ url: 'https://music.example.com', username: 'alice', password: 'pw' }); + const { baseUrl, params } = getClient(); + expect(baseUrl).toBe('https://music.example.com/rest'); + expect(params).toMatchObject({ + u: 'alice', + v: '1.16.1', + f: 'json', + }); + expect(params.c).toMatch(/^psysonic\//); + expect(typeof params.t).toBe('string'); + expect(params.t).toHaveLength(32); // md5 hex + expect(typeof params.s).toBe('string'); + expect((params.s as string).length).toBeGreaterThan(0); + }); + + it('rotates token + salt across calls (random per request)', () => { + setUpServer(); + const a = getClient(); + const b = getClient(); + expect(a.params.t).not.toBe(b.params.t); + expect(a.params.s).not.toBe(b.params.s); + }); +}); + +describe('coverArtCacheKey', () => { + it('uses serverId + entity id + size as a stable cache key', () => { + const id = setUpServer(); + expect(coverArtCacheKey('cover-1')).toBe(`${id}:cover:cover-1:256`); + expect(coverArtCacheKey('cover-1', 200)).toBe(`${id}:cover:cover-1:200`); + }); + + it('falls back to "_" as the server-id segment when no server is active', () => { + expect(coverArtCacheKey('cover-99')).toBe('_:cover:cover-99:256'); + }); + + it('does not embed the ephemeral salt or token — keys stay cacheable across calls', () => { + setUpServer(); + const a = coverArtCacheKey('art-1', 256); + const b = coverArtCacheKey('art-1', 256); + expect(a).toBe(b); + }); +}); + +describe('buildStreamUrl', () => { + it('returns a /rest/stream.view URL on the configured base URL', () => { + setUpServer({ url: 'https://music.example.com', username: 'alice', password: 'pw' }); + const url = new URL(buildStreamUrl('track-1')); + expect(url.origin).toBe('https://music.example.com'); + expect(url.pathname).toBe('/rest/stream.view'); + }); + + it('carries the stable Subsonic auth params (id, u, t, s, v, c, f)', () => { + setUpServer({ url: 'https://music.example.com', username: 'alice', password: 'pw' }); + const url = new URL(buildStreamUrl('track-1')); + expect(url.searchParams.get('id')).toBe('track-1'); + expect(url.searchParams.get('u')).toBe('alice'); + expect(url.searchParams.get('v')).toBe('1.16.1'); + expect(url.searchParams.get('f')).toBe('json'); + expect(url.searchParams.get('c')?.startsWith('psysonic/')).toBe(true); + expect(url.searchParams.get('t')).toHaveLength(32); // md5 hex + expect(url.searchParams.get('s')).toBeTruthy(); + }); + + it('rotates t/s across calls — Rust matches playback identity by id, not by URL', () => { + setUpServer(); + const a = new URL(buildStreamUrl('track-1')); + const b = new URL(buildStreamUrl('track-1')); + expect(a.searchParams.get('id')).toBe(b.searchParams.get('id')); + expect(a.searchParams.get('t')).not.toBe(b.searchParams.get('t')); + expect(a.searchParams.get('s')).not.toBe(b.searchParams.get('s')); + }); + + it('URL-encodes ids with special characters once (not double-encoded)', () => { + setUpServer(); + const id = 'AC/DC — Back in Black'; + const url = new URL(buildStreamUrl(id)); + expect(url.searchParams.get('id')).toBe(id); + }); +}); + +describe('buildCoverArtUrl', () => { + it('returns a /rest/getCoverArt.view URL with size param', () => { + setUpServer({ url: 'https://music.example.com' }); + const url = new URL(buildCoverArtUrl('cover-1', 512)); + expect(url.pathname).toBe('/rest/getCoverArt.view'); + expect(url.searchParams.get('id')).toBe('cover-1'); + expect(url.searchParams.get('size')).toBe('512'); + }); + + it('defaults size to 256 when omitted', () => { + setUpServer(); + const url = new URL(buildCoverArtUrl('cover-1')); + expect(url.searchParams.get('size')).toBe('256'); + }); +}); + +describe('buildDownloadUrl', () => { + it('returns a /rest/download.view URL with id + auth params', () => { + setUpServer({ url: 'https://music.example.com' }); + const url = new URL(buildDownloadUrl('track-7')); + expect(url.pathname).toBe('/rest/download.view'); + expect(url.searchParams.get('id')).toBe('track-7'); + expect(url.searchParams.get('v')).toBe('1.16.1'); + }); +}); + +describe('URL builders — trailing-slash + scheme handling on base URL', () => { + it('strips trailing slash from base URL (getBaseUrl normalises)', () => { + setUpServer({ url: 'https://music.example.com/' }); + const url = new URL(buildStreamUrl('t1')); + expect(url.origin).toBe('https://music.example.com'); + // No `//rest` doubled-slash: + expect(url.pathname).toBe('/rest/stream.view'); + }); + + it('prepends http:// when the configured URL lacks a scheme', () => { + setUpServer({ url: 'music.local' }); + const url = new URL(buildStreamUrl('t1')); + expect(url.protocol).toBe('http:'); + expect(url.host).toBe('music.local'); + }); +}); diff --git a/src/utils/copyEntityShareLink.test.ts b/src/utils/copyEntityShareLink.test.ts new file mode 100644 index 00000000..4db1cdbd --- /dev/null +++ b/src/utils/copyEntityShareLink.test.ts @@ -0,0 +1,95 @@ +/** + * `copyEntityShareLink` composition tests (Phase F3). + * + * Thin wrapper around `getBaseUrl` + `encodeSharePayload` + + * `copyTextToClipboard` — tests pin its trimming + guard semantics so a + * refactor doesn't silently make empty-server / empty-id calls write + * garbage to the clipboard. + */ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { copyEntityShareLink } from './copyEntityShareLink'; +import { + decodeSharePayloadFromText, + PSYSONIC_SHARE_PREFIX, +} from './shareLink'; +import { useAuthStore } from '@/store/authStore'; +import { resetAuthStore } from '@/test/helpers/storeReset'; + +beforeEach(() => { + resetAuthStore(); + // The clipboard mock from src/test/mocks/browser.ts is installed globally; + // each test starts with a clean writeText state via resetBrowserMocks. + vi.mocked(navigator.clipboard.writeText).mockResolvedValue(); +}); + +describe('copyEntityShareLink', () => { + it('writes a psysonic2-prefixed payload that round-trips to the original entity', async () => { + const sid = useAuthStore.getState().addServer({ + name: 'Home', url: 'https://music.example.com', username: 'u', password: 'p', + }); + useAuthStore.getState().setActiveServer(sid); + + const ok = await copyEntityShareLink('track', 'tr-1'); + + expect(ok).toBe(true); + expect(navigator.clipboard.writeText).toHaveBeenCalledTimes(1); + const written = vi.mocked(navigator.clipboard.writeText).mock.calls[0]?.[0] as string; + expect(written.startsWith(PSYSONIC_SHARE_PREFIX)).toBe(true); + expect(decodeSharePayloadFromText(written)).toEqual({ + srv: 'https://music.example.com', + k: 'track', + id: 'tr-1', + }); + }); + + it('returns false when no server is active (empty base URL)', async () => { + // No addServer / setActiveServer → getBaseUrl() returns '' → bail. + const ok = await copyEntityShareLink('album', 'al-1'); + expect(ok).toBe(false); + expect(navigator.clipboard.writeText).not.toHaveBeenCalled(); + }); + + it('returns false on empty or whitespace-only id', async () => { + const sid = useAuthStore.getState().addServer({ + name: 'Home', url: 'https://x.test', username: 'u', password: 'p', + }); + useAuthStore.getState().setActiveServer(sid); + + expect(await copyEntityShareLink('artist', '')).toBe(false); + expect(await copyEntityShareLink('artist', ' ')).toBe(false); + expect(navigator.clipboard.writeText).not.toHaveBeenCalled(); + }); + + it('trims surrounding whitespace from the id before encoding', async () => { + const sid = useAuthStore.getState().addServer({ + name: 'Home', url: 'https://x.test', username: 'u', password: 'p', + }); + useAuthStore.getState().setActiveServer(sid); + + await copyEntityShareLink('composer', ' co-9 '); + + const written = vi.mocked(navigator.clipboard.writeText).mock.calls[0]?.[0] as string; + expect(decodeSharePayloadFromText(written)).toEqual({ + srv: 'https://x.test', + k: 'composer', + id: 'co-9', + }); + }); + + it('propagates a clipboard-failure return value (false)', async () => { + const sid = useAuthStore.getState().addServer({ + name: 'Home', url: 'https://x.test', username: 'u', password: 'p', + }); + useAuthStore.getState().setActiveServer(sid); + + vi.mocked(navigator.clipboard.writeText).mockRejectedValueOnce(new Error('blocked')); + // execCommand fallback also fails. + const originalExec = document.execCommand; + document.execCommand = vi.fn(() => false) as unknown as typeof document.execCommand; + + const ok = await copyEntityShareLink('album', 'al-1'); + expect(ok).toBe(false); + + document.execCommand = originalExec; + }); +}); diff --git a/src/utils/resolvePlaybackUrl.test.ts b/src/utils/resolvePlaybackUrl.test.ts new file mode 100644 index 00000000..7ace41fc --- /dev/null +++ b/src/utils/resolvePlaybackUrl.test.ts @@ -0,0 +1,111 @@ +/** + * `resolvePlaybackUrl` precedence + `streamUrlTrackId` parser tests (Phase F3). + * + * Precedence pinned by the function: offline → hot cache → HTTP stream. + * Refactors that reorder this break playback for users with offline / + * hot-cache entries. + */ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { useOfflineStore } from '@/store/offlineStore'; +import { useHotCacheStore } from '@/store/hotCacheStore'; + +import { + getPlaybackSourceKind, + resolvePlaybackUrl, + streamUrlTrackId, +} from './resolvePlaybackUrl'; +import { useAuthStore } from '@/store/authStore'; +import { resetAuthStore } from '@/test/helpers/storeReset'; + +beforeEach(() => { + resetAuthStore(); + // Reset the offline + hot-cache store getLocalUrl mocks before each test. + vi.spyOn(useOfflineStore.getState(), 'getLocalUrl').mockReturnValue(null); + vi.spyOn(useHotCacheStore.getState(), 'getLocalUrl').mockReturnValue(null); + // Set up an active server so buildStreamUrl works. + const id = useAuthStore.getState().addServer({ + name: 'Test', url: 'https://music.example.com', username: 'alice', password: 'pw', + }); + useAuthStore.getState().setActiveServer(id); +}); + +describe('resolvePlaybackUrl — precedence', () => { + it('returns the offline URL when present (1st priority)', () => { + vi.mocked(useOfflineStore.getState().getLocalUrl).mockReturnValue('psysonic-local://offline/track-1.flac'); + vi.mocked(useHotCacheStore.getState().getLocalUrl).mockReturnValue('psysonic-local://hot/track-1.flac'); + expect(resolvePlaybackUrl('track-1', 'srv-1')).toBe('psysonic-local://offline/track-1.flac'); + }); + + it('falls through to the hot-cache URL when offline is absent (2nd priority)', () => { + vi.mocked(useOfflineStore.getState().getLocalUrl).mockReturnValue(null); + vi.mocked(useHotCacheStore.getState().getLocalUrl).mockReturnValue('psysonic-local://hot/track-1.flac'); + expect(resolvePlaybackUrl('track-1', 'srv-1')).toBe('psysonic-local://hot/track-1.flac'); + }); + + it('falls through to the HTTP stream URL when neither local source is present', () => { + const url = resolvePlaybackUrl('track-1', 'srv-1'); + expect(url).toMatch(/^https:\/\/music\.example\.com\/rest\/stream\.view\?/); + expect(url).toContain('id=track-1'); + }); + + it('forwards trackId + serverId to both stores so per-server entries scope correctly', () => { + resolvePlaybackUrl('track-7', 'srv-3'); + expect(useOfflineStore.getState().getLocalUrl).toHaveBeenCalledWith('track-7', 'srv-3'); + expect(useHotCacheStore.getState().getLocalUrl).toHaveBeenCalledWith('track-7', 'srv-3'); + }); +}); + +describe('getPlaybackSourceKind', () => { + it('returns "offline" when the offline store has the track', () => { + vi.mocked(useOfflineStore.getState().getLocalUrl).mockReturnValue('psysonic-local://offline/t1.flac'); + expect(getPlaybackSourceKind('t1', 'srv-1')).toBe('offline'); + }); + + it('returns "hot" when only the hot-cache has the track', () => { + vi.mocked(useHotCacheStore.getState().getLocalUrl).mockReturnValue('psysonic-local://hot/t1.flac'); + expect(getPlaybackSourceKind('t1', 'srv-1')).toBe('hot'); + }); + + it('returns "stream" when neither has the track and no engine preload hint matches', () => { + expect(getPlaybackSourceKind('t1', 'srv-1')).toBe('stream'); + }); + + it('returns "hot" when the engine reported a preload for this trackId (RAM-loaded)', () => { + expect(getPlaybackSourceKind('t1', 'srv-1', 't1')).toBe('hot'); + }); + + it('returns "stream" when the engine preload hint is for a different track', () => { + expect(getPlaybackSourceKind('t1', 'srv-1', 'other-track')).toBe('stream'); + }); +}); + +describe('streamUrlTrackId', () => { + it('extracts the id query param from a stream.view URL', () => { + const url = 'https://music.example.com/rest/stream.view?id=track-1&u=alice&t=hash'; + expect(streamUrlTrackId(url)).toBe('track-1'); + }); + + it('returns null for URLs that are not stream.view', () => { + expect(streamUrlTrackId('https://music.example.com/rest/getCoverArt.view?id=cover')).toBeNull(); + }); + + it('returns null when the URL has no query string', () => { + expect(streamUrlTrackId('https://music.example.com/rest/stream.view')).toBeNull(); + }); + + it('returns null when stream.view URL lacks an id param', () => { + expect(streamUrlTrackId('https://music.example.com/rest/stream.view?u=alice')).toBeNull(); + }); + + it('decodes URL-encoded id values', () => { + expect(streamUrlTrackId('https://x/rest/stream.view?id=AC%2FDC%20Back')).toBe('AC/DC Back'); + }); + + it('falls back to manual query parsing when URL constructor would throw', () => { + // Relative path — `new URL(...)` requires a base, so the function's + // manual fallback parses the query directly. + const url = '/rest/stream.view?id=relative-track&u=u'; + expect(streamUrlTrackId(url)).toBe('relative-track'); + }); +});