fix(library): open track details by path, not from the whole library

Track details from the queue, a playlist or a smart playlist looked
the track up in libraryStore's whole-library array, and returned
silently when it had not loaded, so the menu item did nothing. That
dependency is also why every track was fetched eagerly at startup
(20.5 MB at 26k tracks).

GetTracksByPaths answers for the paths in hand, and trackCache holds
the answers: bounded, coalesced into one call per task, and forgetting
what the retag, removal and scan events name. Every opener, and
track-details' own refresh after a save, go through it. A source sweep
pins the whole-library array to the store and the Tracks view.

Closes #279
This commit is contained in:
yonlu committed 2026-10-05 23:01:37 -04:00
1 parent 3cddf70c4d
commit 4a9fe1d207
14 files changed
+747 -270

No files matched your search

+209
View File
@@ -0,0 +1,209 @@
/**
* `trackCache` answers "which track is at this path" for the rows a
* surface is showing (#279), instead of every surface reaching into the
* whole library's array — which had to be fetched eagerly at startup
* for that to work, and silently did nothing when it had not been.
*/
import { describe, expect, it, beforeEach } from 'vitest';
import { trackCache } from '@store/track-cache';
import { libraryStore } from '@store/library-store';
import { showBatchTrackDetailsForPaths } from '@utils/track-details-opener';
import type { TrackDetails } from '@components/track-details/track-details';
import { Events } from '../../src/events';
import {
calls,
emit,
flush,
resetHarness,
stub,
stubFailure,
} from '@test/support/harness';
const BY_PATHS = 'library.Library.GetTracksByPaths';
function track(path: string, album = 'Album') {
return {
FilePath: path,
TrackName: path,
ArtistName: 'Artist',
Album: album,
PlayCount: 0,
LastPlayed: '',
CoverArtPath: '',
};
}
/** The backend: answers every path that starts with /lib/. */
function stubLibrary(): void {
stub(BY_PATHS, (paths: string[]) =>
paths.filter((p) => p.startsWith('/lib/')).map((p) => track(p)),
);
}
describe('track cache', () => {
beforeEach(() => {
// The cache is a singleton; a scan completing is how the app empties it.
emit(Events.LibraryScanComplete);
resetHarness();
stubLibrary();
});
it('answers in the order asked and leaves out paths not in the library', async () => {
const got = await trackCache.get(['/lib/b', '/gone', '/lib/a']);
expect(got.map((t) => t.FilePath)).toEqual(['/lib/b', '/lib/a']);
});
it('coalesces lookups made together into one call', async () => {
await Promise.all([
trackCache.get(['/lib/a']),
trackCache.get(['/lib/b', '/lib/a']),
trackCache.getOne('/lib/c'),
]);
expect(calls(BY_PATHS)).toHaveLength(1);
expect((calls(BY_PATHS)[0]!.args[0] as string[]).sort()).toEqual([
'/lib/a',
'/lib/b',
'/lib/c',
]);
});
it('answers a repeat from cache, and asks again after a retag of that file', async () => {
await trackCache.get(['/lib/a', '/lib/b']);
await trackCache.get(['/lib/a', '/lib/b']);
expect(calls(BY_PATHS)).toHaveLength(1);
emit(Events.TrackMetadataChanged, { filePath: '/lib/a' });
await trackCache.get(['/lib/b']);
expect(calls(BY_PATHS), 'an unchanged file is still cached').toHaveLength(1);
await trackCache.get(['/lib/a']);
expect(calls(BY_PATHS)).toHaveLength(2);
});
it('patches a play count in place without a call', async () => {
await trackCache.getOne('/lib/a');
emit(Events.TrackPlayCountChanged, {
filePath: '/lib/a',
playCount: 7,
lastPlayed: '2026-10-05 10:00:00',
});
const got = await trackCache.getOne('/lib/a');
expect([got?.PlayCount, got?.LastPlayed, calls(BY_PATHS).length]).toEqual([
7,
'2026-10-05 10:00:00',
1,
]);
});
it('does not cache an answer that crossed an invalidation', async () => {
let answer: (v: unknown) => void = () => undefined;
stub(BY_PATHS, () => new Promise((resolve) => {
answer = resolve;
}));
const pending = trackCache.getOne('/lib/a');
// Sent, not yet answered: the invalidation lands in between.
await flush();
expect(calls(BY_PATHS)).toHaveLength(1);
emit(Events.TrackMetadataChanged, { batch: true });
answer([track('/lib/a')]);
expect((await pending)?.FilePath, 'the caller still gets it').toBe('/lib/a');
stubLibrary();
await trackCache.getOne('/lib/a');
expect(calls(BY_PATHS)).toHaveLength(2);
});
it('rejects every waiter in a batch when the call fails', async () => {
stubFailure(BY_PATHS);
const results = await Promise.allSettled([
trackCache.getOne('/lib/a'),
trackCache.getOne('/lib/b'),
]);
expect(results.map((r) => r.status)).toEqual(['rejected', 'rejected']);
});
});
/*
* The bug half of #279: the queue, playlist and smart-playlist openers
* read `libraryStore.getCachedTracks()` and returned silently when it
* was null. The batch opener is what all three call now, so it is
* asserted against a library store that has nothing loaded.
*/
describe('opening details with no library list loaded', () => {
beforeEach(() => {
emit(Events.LibraryScanComplete);
resetHarness();
stubLibrary();
stub('library.Library.GetTracks', []);
});
it('shows the dialog with the tracks asked for', async () => {
await flush();
expect(libraryStore.getCachedTracks()?.length ?? 0).toBe(0);
let shown: unknown[] | null = null;
const dialog = {
showBatch: (tracks: unknown[]) => {
shown = tracks;
},
} as unknown as TrackDetails;
const outcome = await showBatchTrackDetailsForPaths(
() => dialog,
['/lib/a', '/lib/b'],
() => undefined,
);
expect(outcome).toBe('shown');
expect((shown ?? []).map((t) => (t as { FilePath: string }).FilePath)).toEqual([
'/lib/a',
'/lib/b',
]);
});
});
/** Every source file, as text. */
const SOURCES = import.meta.glob<string>('../../src/**/*.ts', {
eager: true,
query: '?raw',
import: 'default',
});
/**
* Who may read the whole library's track array. The store owns it and
* the Tracks view draws it; anything else that needs a track by path
* asks `trackCache`, or the array becomes a startup dependency again.
*/
const MAY_READ_ALL_TRACKS = [
'store/library-store.ts',
'store/controllers/library-controller.ts',
'components/track-list/track-list.ts',
];
const ALL_TRACKS_READ = /getCachedTracks\(|libraryStore\.getTracks\(|\bcachedTracks\b/;
describe('the whole-library track array has one reader', () => {
it('reads the sources it sweeps', () => {
expect(Object.keys(SOURCES).length).toBeGreaterThan(100);
});
it('is read only by the store and the Tracks view', () => {
const readers = Object.entries(SOURCES)
.filter(([, src]) => ALL_TRACKS_READ.test(src))
.map(([path]) => path.replace('../../src/', ''))
.sort();
expect(readers).toEqual([...MAY_READ_ALL_TRACKS].sort());
});
});