mirror of
https://github.com/Psychotoxical/psysonic.git
synced 2026-07-21 23:05:46 +00:00
cucadmuh
32a21ec2c3
* fix(playlist): batch playlist writes past the GET URL limit (#1227) Adding tracks failed past ~341 songs because the write path re-sent the entire song list as createPlaylist.view?songId=<all> query params on a GET, blowing past the server's ~8 KiB URL limit. Writes now append incrementally via updatePlaylist.view?songIdToAdd=<batch> (and songIndexToRemove for clears/removals) in 150-id batches, so there is no practical size cap. A per-server in-memory membership cache removes the full getPlaylist refetch on every dedup, fixing the "slow add on big playlists" report. Layering detangle (keeps the new cache from adding dep-cruiser cycles): - lib/api/subsonicPlaylists.ts is pure of the store again; cache invalidation on write failure moved to the feature callers' catch blocks. - membership cache extracted to the core layer (src/store/playlistMembershipStore.ts) so offline/orbit/contextMenu/playlist read it directly instead of routing through the @/features/playlist barrel. - severed the offline -> playlist-barrel edge (pinnedOfflineSync name fallback through the live playlist list was dead: nameless callers are all gated by isSourcePinnedOffline, where offline meta already carries the name). - confirmAddAllDuplicates moved into the playlist feature; contextMenu submenus import the add/merge helpers via the playlist barrel, not deep paths. dep-cruiser baseline regenerated: 742 -> 714 (net -28, all no-circular). The churn in the baseline is path-shift of the frozen playerStore SCC (cover/ playback/orbit), not new coupling; the new playlist modules have zero violations. * docs(changelog): record #1235 playlist URL-limit fix (changelog + credits) * fix(playlist): seed membership cache from full list, not library-scoped view (#1235 review) F1: runPlaylistLoad seeded the dedup membership cache from the library-scope-filtered songs, so out-of-scope members looked "new" and addTracksToPlaylistWithDedup/collectMergeSongIds could re-add them as duplicates. Cache now holds the full unfiltered server list while the UI still shows the filtered view; add a regression test. Also documents the two accepted trade-offs flagged in review: - F2: >batch updatePlaylist clears then appends non-atomically (URL-limit workaround); a mid-step failure truncates server state, cache invalidation lets the client re-read truth. - F3: dedup read-modify-append isn't atomic across the await; rare missed dedup on concurrent adds, self-heals on next load.
66 lines
2.7 KiB
TypeScript
66 lines
2.7 KiB
TypeScript
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
|
import { addTracksToPlaylistWithDedup } from '@/features/playlist/utils/addTracksToPlaylistWithDedup';
|
|
import { usePlaylistMembershipStore } from '@/store/playlistMembershipStore';
|
|
|
|
const addSongsToPlaylistMock = vi.fn(async (_id: string, _ids: string[]) => {});
|
|
const getPlaylistMock = vi.fn(async (_id: string) => ({ playlist: { id: 'pl-1' }, songs: [{ id: 'a' }, { id: 'b' }] }));
|
|
const confirmMock = vi.fn(async () => false);
|
|
|
|
vi.mock('@/lib/api/subsonicPlaylists', () => ({
|
|
addSongsToPlaylist: (id: string, ids: string[]) => addSongsToPlaylistMock(id, ids),
|
|
getPlaylist: (id: string) => getPlaylistMock(id),
|
|
}));
|
|
|
|
vi.mock('@/store/confirmModalStore', () => ({
|
|
useConfirmModalStore: {
|
|
getState: () => ({ request: () => confirmMock() }),
|
|
},
|
|
}));
|
|
|
|
vi.mock('@/store/authStore', () => ({
|
|
useAuthStore: {
|
|
getState: () => ({ activeServerId: 'srv-1' }),
|
|
},
|
|
}));
|
|
|
|
describe('addTracksToPlaylistWithDedup', () => {
|
|
beforeEach(() => {
|
|
addSongsToPlaylistMock.mockClear();
|
|
getPlaylistMock.mockClear();
|
|
confirmMock.mockReset();
|
|
confirmMock.mockResolvedValue(false);
|
|
usePlaylistMembershipStore.setState({
|
|
songIdsByCacheKey: { 'srv-1:pl-1': ['a', 'b'] },
|
|
});
|
|
});
|
|
|
|
it('dedupes against cached ids without getPlaylist', async () => {
|
|
const result = await addTracksToPlaylistWithDedup('pl-1', 'Mix', ['b', 'c'], k => k);
|
|
expect(result).toMatchObject({ outcome: 'partial', addedCount: 1, skippedCount: 1 });
|
|
expect(getPlaylistMock).not.toHaveBeenCalled();
|
|
expect(addSongsToPlaylistMock).toHaveBeenCalledWith('pl-1', ['c']);
|
|
expect(usePlaylistMembershipStore.getState().getPlaylistSongIds('pl-1')).toEqual(['a', 'b', 'c']);
|
|
});
|
|
|
|
it('fetches membership once on cold cache and dedupes', async () => {
|
|
usePlaylistMembershipStore.setState({ songIdsByCacheKey: {} });
|
|
getPlaylistMock.mockResolvedValue({
|
|
playlist: { id: 'pl-2' },
|
|
songs: [{ id: 'x' }],
|
|
});
|
|
const result = await addTracksToPlaylistWithDedup('pl-2', 'Cold', ['x', 'y'], k => k);
|
|
expect(result).toMatchObject({ outcome: 'partial', addedCount: 1, skippedCount: 1 });
|
|
expect(getPlaylistMock).toHaveBeenCalledTimes(1);
|
|
expect(addSongsToPlaylistMock).toHaveBeenCalledWith('pl-2', ['y']);
|
|
expect(usePlaylistMembershipStore.getState().getPlaylistSongIds('pl-2')).toEqual(['x', 'y']);
|
|
});
|
|
|
|
it('invalidates cache when the write fails', async () => {
|
|
addSongsToPlaylistMock.mockRejectedValueOnce(new Error('boom'));
|
|
await expect(
|
|
addTracksToPlaylistWithDedup('pl-1', 'Mix', ['c'], k => k),
|
|
).rejects.toThrow('boom');
|
|
expect(usePlaylistMembershipStore.getState().getPlaylistSongIds('pl-1')).toBeUndefined();
|
|
});
|
|
});
|