diff --git a/frontend/src/store/library-store.ts b/frontend/src/store/library-store.ts index 9aba95c..ddb8a4f 100644 --- a/frontend/src/store/library-store.ts +++ b/frontend/src/store/library-store.ts @@ -9,6 +9,7 @@ import { } from '@go/library/library.js'; import type * as library from '@go/library/models.js'; import { list } from '@utils/binding'; +import { trackCache } from './track-cache'; import { decodeTrackTable, type ListTrack } from '@utils/track-table'; import { Events } from '../events'; @@ -90,8 +91,8 @@ class LibraryStore { private cacheGen = 0; constructor() { - EventsOn(Events.LibraryScanComplete, () => { - this.invalidate(); + EventsOn(Events.LibraryScanComplete, (payload: unknown) => { + this.applyScanComplete(payload); }); EventsOn(Events.LibraryRemoved, () => { this.libraries = null; @@ -107,8 +108,8 @@ class LibraryStore { this.changeGen++; this.notify(); }); - EventsOn(Events.TrackMetadataChanged, () => { - this.invalidate(); + EventsOn(Events.TrackMetadataChanged, (payload: unknown) => { + this.applyMetadataChanged(payload); }); EventsOn(Events.TrackPlayCountChanged, (payload: unknown) => { this.applyPlayCount(payload); @@ -554,6 +555,116 @@ class LibraryStore { // INVALIDATION // =================================================================== + /** + * A scan finished. Reload only if it changed something (#282). + * + * `ScanMetrics` says how many files were added, updated and removed, + * and a soft rescan of an unchanged library reports zero of each — + * yet every loaded collection was thrown away and refetched, which + * at 26 138 tracks is 20.5 MB across the IPC for a scan that found + * nothing. Anything non-zero still reloads everything: a scan can + * change a tag, an album name or a genre on any file it touched, and + * it reports counts rather than paths. + * + * A payload that says nothing at all is treated as a change, not as + * a no-op: an unknown shape must not be able to leave a stale list + * on screen. + */ + private applyScanComplete(payload: unknown): void { + const m = payload as { + added?: number; + updated?: number; + removed?: number; + } | null; + + if (!m) { + this.invalidate(); + + return; + } + + const changed = (m.added ?? 0) + (m.updated ?? 0) + (m.removed ?? 0); + + if (changed === 0) return; + + this.invalidate(); + } + + /** + * Tags were rewritten on disk. A single file is patched; a batch is + * a reload (#282). + * + * The event names the one file it rewrote, and that file's row is + * the only row that changed — so re-reading the whole list to pick + * up one new title is 20.5 MB at 26 138 tracks. A batch write + * carries no paths (it can be thousands of files), and the summaries + * really do change with it, so that one still reloads. + */ + private applyMetadataChanged(payload: unknown): void { + const p = payload as { filePath?: string; batch?: boolean } | null; + + if (!p?.filePath || p.batch) { + this.invalidate(); + + return; + } + + this.patchOneTrack(p.filePath); + } + + /** + * Re-read one file's row and splice it in. + * + * The summaries are dropped and refetched, because a retag can move + * a track between albums and change a genre — they are the small + * collections, and they are what makes the album and artist views + * agree with the row that was just patched. + */ + private patchOneTrack(filePath: string): void { + const loaded = this.loadedCollections(); + + void trackCache + .refresh([filePath]) + .then(([fresh]) => { + if (!fresh || this.tracks === null) return; + + const idx = this.tracks.findIndex( + (t) => t.FilePath === filePath, + ); + + // Not in this view's list (another library's file, or + // gone): the list is right as it stands. + if (idx === -1) return; + + this.tracks = [ + ...this.tracks.slice(0, idx), + fresh, + ...this.tracks.slice(idx + 1), + ]; + this.changeGen++; + this.notify(); + }) + .catch((err: unknown) => { + // The patch failed, so the list may now be stale. Fall + // back to the answer that cannot be wrong. + console.error('library: could not re-read a retagged track', err); + this.invalidate(); + }); + + this.albums = null; + this.artists = null; + this.genres = null; + this.cacheGen++; + this.inFlight.delete('albums'); + this.inFlight.delete('artists'); + this.inFlight.delete('genres'); + this.refetchLoaded({ + albums: loaded.albums, + artists: loaded.artists, + genres: loaded.genres, + }); + } + /** * Patch one track's play statistics in place. * diff --git a/frontend/test/stores/library-store.test.ts b/frontend/test/stores/library-store.test.ts index 8fcf88b..8be486a 100644 --- a/frontend/test/stores/library-store.test.ts +++ b/frontend/test/stores/library-store.test.ts @@ -21,6 +21,12 @@ import { } from '@test/support/harness'; import { trackTable } from '@test/support/track-table'; +const RETAGGED = { + TrackName: 'Retagged', + FilePath: '/a.mp3', + PlayCount: 0, +}; + const TRACKS = [ { TrackName: 'One', FilePath: '/a.mp3', PlayCount: 0 }, { TrackName: 'Two', FilePath: '/b.mp3', PlayCount: 4 }, @@ -112,10 +118,93 @@ describe('library store: caching', () => { ]); }); - it('refetches when a track is retagged', async () => { + /* + * #282: a retag names the one file it rewrote, so the whole list does + * not have to come back — 20.5 MB at 26 138 tracks to pick up one new + * title. The summaries are still refetched, because a retag can move + * a track between albums. + */ + it('re-reads the one file a retag names, not the whole list', async () => { + stub('library.Library.GetTracksByPaths', [RETAGGED]); + emit(Events.TrackMetadataChanged, { filePath: '/a.mp3' }); await flush(); + expect(calls('library.Library.GetTrackTable')).toHaveLength(0); + expect(lastArgs('library.Library.GetTracksByPaths')).toEqual([['/a.mp3']]); + expect(calls().map((c) => c.path).sort()).toEqual([ + 'library.Library.GetAlbums', + 'library.Library.GetArtists', + 'library.Library.GetGenres', + 'library.Library.GetTracksByPaths', + ]); + }); + + it('splices the re-read row in, so consumers notice', async () => { + stub('library.Library.GetTracksByPaths', [RETAGGED]); + + const before = libraryStore.getCachedTracks(); + + emit(Events.TrackMetadataChanged, { filePath: '/a.mp3' }); + await flush(); + + const after = libraryStore.getCachedTracks(); + + expect(after?.map((t) => t.TrackName)).toEqual(['Retagged', 'Two']); + expect(after, 'a new array, which is what memoized consumers key on').not.toBe( + before, + ); + }); + + it('falls back to a full reload when the patch cannot be read', async () => { + stubFailure('library.Library.GetTracksByPaths', 'sql: database is locked'); + + emit(Events.TrackMetadataChanged, { filePath: '/a.mp3' }); + await flush(); + + expect(calls('library.Library.GetTrackTable')).toHaveLength(1); + }); + + it('reloads everything for a batch write, which names no paths', async () => { + emit(Events.TrackMetadataChanged, { batch: true, total: 40 }); + await flush(); + + expect(calls('library.Library.GetTrackTable')).toHaveLength(1); + expect(calls('library.Library.GetTracksByPaths')).toHaveLength(0); + }); + + /* + * #282, the other half: a soft rescan of an unchanged library reports + * zero added, updated and removed, and every loaded collection was + * thrown away and refetched anyway. + */ + describe('a scan that changed nothing', () => { + beforeEach(async () => { + emit(Events.LibraryScanComplete, { + added: 0, + updated: 0, + removed: 0, + skipped: 31, + }); + await flush(); + }); + + it('refetches nothing', () => { + expect(calls()).toEqual([]); + }); + + it('keeps the data it already had', () => { + expect(libraryStore.getCachedTracks()?.map((t) => t.FilePath)).toEqual([ + '/a.mp3', + '/b.mp3', + ]); + }); + }); + + it('reloads when a scan reports a change', async () => { + emit(Events.LibraryScanComplete, { added: 1, updated: 0, removed: 0 }); + await flush(); + expect(calls('library.Library.GetTrackTable')).toHaveLength(1); });