diff --git a/.pi/skills/yellowjacket-dev/references/android-tier.md b/.pi/skills/yellowjacket-dev/references/android-tier.md index 1af553a..e9bd4f5 100644 --- a/.pi/skills/yellowjacket-dev/references/android-tier.md +++ b/.pi/skills/yellowjacket-dev/references/android-tier.md @@ -647,7 +647,7 @@ window.__yj = { call(name, args) { That turns the device into a tier that can be *driven* rather than only looked at — `__yj.call("player.Player.LoadFile", [path])` and `__yj.call("library.Library.AddLibrary", ["/sdcard/Music/..."])` are how -#53 was measured. Names are the Go ones (`GetTracks`, not +#53 was measured. Names are the Go ones (`GetTrackTable`, not `GetAllTracks`); an unknown one comes back as a plain `unknown bound method name`, so a wrong guess is loud. diff --git a/.planning/NOTES.md b/.planning/NOTES.md index b429d49..8643283 100644 --- a/.planning/NOTES.md +++ b/.planning/NOTES.md @@ -5097,3 +5097,87 @@ beside what they explain. These three did not: The drawer-style gutter would buy the affordance by taking width off a full-screen surface on a 424px viewport; back and a 44px close button answer it instead. + +## What the library payloads actually cost (measured 2026-10-05, desktop dev build) + +Plan 023 (#279–#284). Measured against `make sandbox-seed-bulk` +(50 000 tracks, 371 MB `yj.db` in the smaller copy) with the desktop +dev build and `e2e/perf/measure.mjs`, which grew a `memory` section for +it: backend RSS and peak from `/proc`, the Go heap from pprof, the +page's JS heap and DOM counters from CDP, and the bytes every binding +returned, before and after the first open of Tracks. + +| | before | after #281 | after #280 | +|---|---|---|---| +| Backend RSS at rest | 543 MB | 296 MB | 189 MB | +| Backend peak RSS | 571 MB | 337 MB | 281 MB | +| Go heap held | 361 MB | 125 MB | 7 MB | +| JS heap at rest | 31.8 MB | 18.0 MB | 5.2 MB | +| Binding bytes at rest | 35.9 MB | 12.1 MB | 1.7 MB | +| JS heap after a browse | 36.5 MB | 22.7 MB | 23.1 MB | +| Tracks first open → first row | 38 ms | 26 ms | 1 255 ms | +| Slowest first view open | 44 ms | 57 ms | 76 ms | + +Six things worth keeping: + +- **The number that fell was not the number being optimised.** The + binding payload fell 20.5 → 10.45 MB (dictionary-encoded columns) and + 35.9 → 12.1 MB, but what made the backend's RSS fall by 354 MB was + the *fetch not happening*: `GetTracks` was the only thing in the app + that allocated 170 MB transiently (`sqlcgen.GetTracks` 80 MB, `json/v2` + 128 MB, `slices.Grow` 77 MB, `bytes.Clone` 47 MB of 484 MB total + `alloc_space`). Encoding smaller would not have touched that. +- **A `dictionary` is what makes dropping fields the wrong trade.** The + first version of the column table left out `LastPlayed` and the three + larger cover tiers, and then a "select all → edit tags" over 50 000 + tracks had to fetch every track back to open the dialog: 3 071 ms + against 88 ms before. Interning means the repeated strings are nearly + free, so the fields belong in the table; what fixed it in the end was + neither — the Tracks view hands the dialog the rows it already has. + **A payload optimisation that removes a field is a new fetch waiting + to be written.** +- **`mmap_size` is a bound, not an allocation.** #283 read as "64 MB of + mapping per connection", and the RSS of the database mapping is 46 MB + whether the bound is 64 MB or 16 MB, because a mapping costs what the + working set touches. Quartering it moved no query either (1 172 → + 1 197 ms for the whole track list, 106 → 124 ms for the album list, 6 + → 8 ms for an FTS search — all inside the run-to-run spread). Kept for + the phone, where the bound is the address space the low-memory killer + reads. +- **A view that is first paint is a view that is active.** `index.html` + renders a ``, so the track list was connected at launch + and fetched 12 MB whoever was looking at — and the fix was not in the + component but in the shell: markup is not a decision about which view + the launch lands on, so the seed is `view-hidden` and the first + navigation is what activates it. Hover and keyboard focus on a nav + item prefetch, which is the ~100 ms before the click. +- **The remaining second is the transport, not the data.** A cold open + of Tracks on 50 000 tracks is 1 255 ms, and a *raw* binding call for + the same table is 1 118–1 382 ms: none of it is the TypeScript decode + or the render. It is the Go-side query, the column encode and Wails + encoding every result twice — once for a debug log that is off + (#286). Splitting that number is what says where to go next, and the + answer was not where the payload work had been. +- **A test named for the behaviour caught the thing the design missed.** + The source sweep that pins "the whole-library track array has one + reader" was written to catch the four call sites #279 had already + converted; it found `smart-playlist-editor`, which built its value + suggestions from the same array and would have gone silently empty + once nothing loaded it. The suggestion box is a backend query now + (`SuggestSmartPlaylistValues`). + +And two things about this machine, since they cost a cycle each: + +- **`make ui-test` is not reliable at load average 14.** Under the + workstation's own background services, the browser provider's module + fetches fail in a different handful of files each run ("Failed to + import test file", "Cannot connect to the iframe") while every test + that runs passes. `--maxWorkers=1 --retry=2` reduces it; individual + files always pass. A failure list that changes between runs is the + environment, not the branch. +- **A measurement run inherits the machine's mood.** The first + after-#281 numbers said view opens had doubled (albums 27 → 78 ms, + settings 44 → 179 ms, Tracks first row 38 → 105 ms). Re-running + unchanged gave 26 ms and 57 ms. The tell was the same one + `NOTES.md` already records twice: before and after suspiciously + equal, or suspiciously worse, across *unrelated* measurements. diff --git a/backend/database/database.go b/backend/database/database.go index a82fa72..893eea8 100644 --- a/backend/database/database.go +++ b/backend/database/database.go @@ -54,9 +54,19 @@ type DB struct { // form is silently ignored, which is why WAL was never actually on. const ( writeDSNParams = "?_pragma=busy_timeout(5000)&_pragma=journal_mode(WAL)" - readDSNParams = "?_pragma=busy_timeout(5000)&_pragma=journal_mode(WAL)" + + // The read pool's own page cache and mapping bound, sized by + // measurement rather than by appetite (#283). On a 50 000-track + // library, halving the cache and quartering the mapping moved no + // query — 1 172 → 1 197 ms for the whole track list, 106 → 124 ms + // for the album list, 6 → 8 ms for an FTS search, all inside the + // run-to-run spread — and the RSS of the database mapping was 46 MB + // under both settings, because a mapping's cost is what the working + // set touches, not the bound. The bound is what matters on a + // phone, where five connections' worth of address space is the + // thing the low-memory killer reads (cf. #52). + readDSNParams = "?_pragma=busy_timeout(5000)&_pragma=journal_mode(WAL)" + "&_pragma=query_only(true)&_pragma=synchronous(NORMAL)" + - "&_pragma=cache_size(-8000)&_pragma=mmap_size(67108864)" + "&_pragma=cache_size(-2000)&_pragma=mmap_size(16777216)" // readPoolConns bounds concurrent read connections. A handful is // plenty for interactive search + art/lookup fan-out and keeps WAL // reader overhead small. @@ -354,8 +364,8 @@ func applyPRAGMAs(ctx context.Context, db *sql.DB) error { pragmas := []string{ "PRAGMA foreign_keys = ON", "PRAGMA synchronous = NORMAL", - "PRAGMA cache_size = -8000", - "PRAGMA mmap_size = 67108864", + "PRAGMA cache_size = -2000", + "PRAGMA mmap_size = 16777216", } for _, pragma := range pragmas { diff --git a/backend/database/sql/queries/audio_files.sql b/backend/database/sql/queries/audio_files.sql index de88294..4dc7a95 100644 --- a/backend/database/sql/queries/audio_files.sql +++ b/backend/database/sql/queries/audio_files.sql @@ -117,6 +117,9 @@ WHERE library_id = COALESCE(NULLIF(CAST(sqlc.arg(library_id) AS INTEGER), 0), li -- name: GetTrackByPath :one SELECT * FROM track_metadata WHERE file_path = ? LIMIT 1; +-- name: GetTracksByPaths :many +SELECT * FROM track_metadata WHERE file_path IN (sqlc.slice('paths')); + -- name: GetTracksByAlbum :many SELECT * FROM track_metadata WHERE album_id = sqlc.arg(album_id) diff --git a/backend/database/sql/sqlcgen/audio_files.sql.go b/backend/database/sql/sqlcgen/audio_files.sql.go index df3dabf..846e3ee 100644 --- a/backend/database/sql/sqlcgen/audio_files.sql.go +++ b/backend/database/sql/sqlcgen/audio_files.sql.go @@ -830,6 +830,71 @@ func (q *Queries) GetTracksByGenre(ctx context.Context, arg GetTracksByGenrePara return items, nil } +const getTracksByPaths = `-- name: GetTracksByPaths :many +SELECT id, file_path, length_milliseconds, title, artist_name, track_number, disc_number, album, genre, year, release_year, composer, file_type, sample_rate, bit_depth, channels, bitrate, file_size, library_id, play_count, last_played, cover_art_path, artist_mbid, release_group_mbid, recording_mbid, album_id, artist_id FROM track_metadata WHERE file_path IN (/*SLICE:paths*/?) +` + +func (q *Queries) GetTracksByPaths(ctx context.Context, paths []string) ([]TrackMetadatum, error) { + query := getTracksByPaths + var queryParams []interface{} + if len(paths) > 0 { + for _, v := range paths { + queryParams = append(queryParams, v) + } + query = strings.Replace(query, "/*SLICE:paths*/?", strings.Repeat(",?", len(paths))[1:], 1) + } else { + query = strings.Replace(query, "/*SLICE:paths*/?", "NULL", 1) + } + rows, err := q.db.QueryContext(ctx, query, queryParams...) + if err != nil { + return nil, err + } + defer rows.Close() + var items []TrackMetadatum + for rows.Next() { + var i TrackMetadatum + if err := rows.Scan( + &i.ID, + &i.FilePath, + &i.LengthMilliseconds, + &i.Title, + &i.ArtistName, + &i.TrackNumber, + &i.DiscNumber, + &i.Album, + &i.Genre, + &i.Year, + &i.ReleaseYear, + &i.Composer, + &i.FileType, + &i.SampleRate, + &i.BitDepth, + &i.Channels, + &i.Bitrate, + &i.FileSize, + &i.LibraryID, + &i.PlayCount, + &i.LastPlayed, + &i.CoverArtPath, + &i.ArtistMbid, + &i.ReleaseGroupMbid, + &i.RecordingMbid, + &i.AlbumID, + &i.ArtistID, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const lookupTrackMetaByPaths = `-- name: LookupTrackMetaByPaths :many SELECT id, file_path, title, artist_name, album, cover_art_path, artist_mbid, release_group_mbid, recording_mbid diff --git a/backend/explore/listenbrainz.go b/backend/explore/listenbrainz.go index 725d47b..bc05729 100644 --- a/backend/explore/listenbrainz.go +++ b/backend/explore/listenbrainz.go @@ -13,6 +13,7 @@ import ( "net/http" "slices" "strings" + "sync/atomic" "time" ) @@ -25,6 +26,19 @@ const ( // responds with a non-2xx status code. var ErrListenBrainzHTTP = errors.New("listenbrainz HTTP error") +// ErrListenBrainzUnauthorized is a 401: the endpoint wants a token, and +// no retry will change that. +// +// It is separate from ErrListenBrainzHTTP because it is the one +// failure that is about *this client* rather than about the thing being +// asked for — which is what makes it the one worth latching. The +// popularity endpoints answered 401 to every request on 2026-10-05, so +// a discography backfill spent one rate-limited request per artist to +// be told the same thing: 399 of them in a minute of a real library's +// backfill, each one a log line and a wasted slot in the shared +// limiter. +var ErrListenBrainzUnauthorized = errors.New("listenbrainz requires a token") + // ListenBrainzClient is a thin HTTP client for the ListenBrainz // popularity and labs APIs. All requests are rate-limited via the // shared RateLimiter and cached via the shared Cache. @@ -39,6 +53,13 @@ type ListenBrainzClient struct { // SetBaseURL shape — so a test that points one client at an // httptest server does not stop being parallel-safe. baseURL string + + // refused latches the first 401. A token is a property of the + // installation, not of the artist being asked about, so the answer + // is the same for every later request and asking again is pure + // cost. Per client rather than global so a test can have one that + // is refused and one that is not. + refused atomic.Bool } // NewListenBrainzClient creates a ListenBrainz API client. @@ -56,6 +77,14 @@ func NewListenBrainzClient( } } +// Unauthorized reports whether this client has been refused with a 401 +// during its life. A caller that is about to do a long pass of +// requests should ask before starting it: the answer will not change +// mid-pass. +func (c *ListenBrainzClient) Unauthorized() bool { + return c.refused.Load() +} + // SetBaseURL redirects this client at another host. Tests only. func (c *ListenBrainzClient) SetBaseURL(url string) { c.baseURL = strings.TrimSuffix(url, "/") @@ -489,6 +518,11 @@ func (c *ListenBrainzClient) doPost( func (c *ListenBrainzClient) doRequest( ctx context.Context, method string, url string, body []byte, ) ([]byte, error) { + // Asked and answered, for the rest of this client's life. + if c.refused.Load() { + return nil, fmt.Errorf("%w: %s", ErrListenBrainzUnauthorized, url) + } + c.logger.Debug("listenbrainz rate limiter wait", "url", url) if err := c.limiter.Wait(ctx); err != nil { @@ -533,6 +567,21 @@ func (c *ListenBrainzClient) doRequest( "status", resp.StatusCode, ) + if resp.StatusCode == http.StatusUnauthorized { + // Recorded once, at warning level, because the next thing this + // client does is stop asking: a log line per artist is the + // symptom this latch exists to remove. + if c.refused.CompareAndSwap(false, true) { + c.logger.Warn("listenbrainz refused this client: "+ + "popularity data needs a token, so the rest of this "+ + "run will not ask for it", + "url", url, + ) + } + + return nil, fmt.Errorf("%w: %s", ErrListenBrainzUnauthorized, url) + } + if resp.StatusCode < 200 || resp.StatusCode >= 300 { return nil, fmt.Errorf( "%w: %d %s", ErrListenBrainzHTTP, resp.StatusCode, truncateBody(respBody), diff --git a/backend/explore/listenbrainz_refusal_test.go b/backend/explore/listenbrainz_refusal_test.go new file mode 100644 index 0000000..033fcf7 --- /dev/null +++ b/backend/explore/listenbrainz_refusal_test.go @@ -0,0 +1,90 @@ +package explore + +import ( + "context" + "errors" + "log/slog" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + + "yellowjacket/backend/database" +) + +// #284: the popularity endpoints answered 401 to every request, so a +// backfill spent one rate-limited request per artist to be told the same +// thing — 399 of them in a minute against a real library. A token is a +// property of the installation, not of the artist, so the first refusal +// is the answer for the whole client. +func TestListenBrainzLatchesARefusal(t *testing.T) { + t.Parallel() + + var requests atomic.Int64 + + srv := httptest.NewServer(http.HandlerFunc( + func(w http.ResponseWriter, _ *http.Request) { + requests.Add(1) + w.WriteHeader(http.StatusUnauthorized) + }, + )) + t.Cleanup(srv.Close) + + c := NewListenBrainzClient( + NewRateLimiter(), NewCache(database.NewTestDB(t), slog.Default()), slog.Default(), + ) + c.SetBaseURL(srv.URL) + + for i := range 5 { + _, err := c.TopRecordingsForArtist(context.Background(), "an-mbid") + + if !errors.Is(err, ErrListenBrainzUnauthorized) { + t.Fatalf("call %d: err = %v, want ErrListenBrainzUnauthorized", i, err) + } + } + + if got := requests.Load(); got != 1 { + t.Errorf("requests = %d, want 1: the rest of the calls are the same answer", got) + } + + if !c.Unauthorized() { + t.Error("Unauthorized() = false after a 401") + } +} + +// A failure that a retry could fix must not latch: the artist stays +// unmarked and the next run asks again. +func TestListenBrainzDoesNotLatchATransientFailure(t *testing.T) { + t.Parallel() + + var requests atomic.Int64 + + srv := httptest.NewServer(http.HandlerFunc( + func(w http.ResponseWriter, _ *http.Request) { + requests.Add(1) + w.WriteHeader(http.StatusInternalServerError) + }, + )) + t.Cleanup(srv.Close) + + c := NewListenBrainzClient( + NewRateLimiter(), NewCache(database.NewTestDB(t), slog.Default()), slog.Default(), + ) + c.SetBaseURL(srv.URL) + + for range 3 { + _, err := c.TopRecordingsForArtist(context.Background(), "an-mbid") + + if !errors.Is(err, ErrListenBrainzHTTP) { + t.Fatalf("err = %v, want ErrListenBrainzHTTP", err) + } + } + + if got := requests.Load(); got != 3 { + t.Errorf("requests = %d, want 3", got) + } + + if c.Unauthorized() { + t.Error("Unauthorized() = true after a 500") + } +} diff --git a/backend/explore/searchindex.go b/backend/explore/searchindex.go index 4e4c7c5..4832a4c 100644 --- a/backend/explore/searchindex.go +++ b/backend/explore/searchindex.go @@ -518,6 +518,13 @@ func (si *SearchIndex) BackfillLibraryDiscographies(ctx context.Context) { break } + // A refused client is refused for every artist: the rest of + // this pass would be the same 401, once per artist (#284). The + // artists stay unmarked, so a run with a token picks them up. + if indexLB.Unauthorized() { + break + } + work <- mbid } @@ -534,6 +541,17 @@ func (si *SearchIndex) BackfillLibraryDiscographies(ctx context.Context) { return } + if indexLB.Unauthorized() { + si.logger.Warn("discography backfill stopped early: "+ + "listenbrainz refused this client, and a token is what it wants", + "artists", total, "of", len(mbids), + ) + + job.logf(jobs.LevelWarn, "Stopped early: ListenBrainz needs a token") + + return + } + job.logf(jobs.LevelInfo, "Filled in "+strconv.Itoa(total)+" artists") si.logger.Info("discography backfill complete", "artists", total) diff --git a/backend/library/query.go b/backend/library/query.go index efc05e0..02259b3 100644 --- a/backend/library/query.go +++ b/backend/library/query.go @@ -2,7 +2,6 @@ package library import ( "database/sql" - "errors" "fmt" "os" "path/filepath" @@ -18,8 +17,6 @@ import ( // searchTrackLimit bounds an FTS search's result set. const searchTrackLimit = 500 -var errNoTracksInLibrary = errors.New("no tracks in library") - // Track is one audio file with everything a list needs to draw it. type Track struct { TrackName string @@ -169,28 +166,46 @@ func (l *Library) GetTrackMBIDs(filePath string) TrackMBIDs { } } -// GetTracks returns every track in a library, or in all of them when -// libraryID is 0. +// pathLookupChunk bounds the paths bound into one IN (...) query, well +// under SQLite's bind-variable limit. +const pathLookupChunk = 500 + +// GetTracksByPaths returns whole tracks for the given file paths, in +// the order asked, dropping any path that is not in the library. // -// The library id is a parameter rather than a second method because the -// two used to be separate queries, separate bindings and a branch at -// every call site - and the scoped form costs nothing (measured: 23 ms -// against 21 ms over 26k rows). -func (l *Library) GetTracks(libraryID int64) ([]Track, error) { - rows, err := l.db.ReadQueries.GetTracks(l.ctx, libraryID) - if err != nil { - l.logger.Error("could not retrieve audio files", "error", err) +// It is how the frontend resolves the tracks a surface is actually +// showing (#279). Track details from the queue, a playlist or a smart +// playlist used to look the path up in the whole library's track +// array, which had to be loaded first — so it was fetched eagerly at +// startup, 20.5 MB at 26k tracks, to answer questions about a handful +// of rows. +func (l *Library) GetTracksByPaths(paths []string) ([]Track, error) { + byPath := make(map[string]Track, len(paths)) - return nil, fmt.Errorf("could not get tracks: %w", err) + for start := 0; start < len(paths); start += pathLookupChunk { + chunk := paths[start:min(start+pathLookupChunk, len(paths))] + + rows, err := l.db.ReadQueries.GetTracksByPaths(l.ctx, chunk) + if err != nil { + return nil, fmt.Errorf("could not get tracks by path: %w", err) + } + + for _, row := range rows { + byPath[row.FilePath] = trackFromRow(row) + } } - l.logger.Info("audio file list", "count", len(rows), "libraryID", libraryID) + tracks := make([]Track, 0, len(byPath)) - if len(rows) == 0 { - return nil, errNoTracksInLibrary + for _, path := range paths { + if t, ok := byPath[path]; ok { + tracks = append(tracks, t) + // A path asked twice is answered once. + delete(byPath, path) + } } - return tracksFromRows(rows), nil + return tracks, nil } // SearchTracks runs the library's FTS index and returns whole tracks. diff --git a/backend/library/scan_fixtures_test.go b/backend/library/scan_fixtures_test.go index 8f43720..e1b7c51 100644 --- a/backend/library/scan_fixtures_test.go +++ b/backend/library/scan_fixtures_test.go @@ -58,15 +58,15 @@ func TestScan_FixtureLibraryLeavesNothingBehind(t *testing.T) { t.Skip("fixture library is empty; run make testdata") } - tracks, err := lib.GetTracks(0) + table, err := lib.GetTrackTable(0) if err != nil { - t.Fatalf("GetTracks: %v", err) + t.Fatalf("GetTrackTable: %v", err) } // One track per file: the projection cannot multiply rows, because // there is no join table left to multiply them. - if int64(len(tracks)) != files { - t.Errorf("GetTracks returned %d rows for %d files", len(tracks), files) + if int64(len(table.FilePath)) != files { + t.Errorf("GetTrackTable returned %d rows for %d files", len(table.FilePath), files) } // Nothing shared outlives what refers to it. diff --git a/backend/library/trackpaths_test.go b/backend/library/trackpaths_test.go new file mode 100644 index 0000000..69226e1 --- /dev/null +++ b/backend/library/trackpaths_test.go @@ -0,0 +1,64 @@ +package library + +import ( + "fmt" + "testing" +) + +// #279: the track-details openers resolve the rows a surface is showing +// by path, instead of finding them in the whole library's array. The +// answer must keep the caller's order (a batch dialog lists them as +// selected), drop what is not in the library rather than invent it, and +// survive more paths than one IN (...) can bind. +func TestGetTracksByPaths(t *testing.T) { + t.Parallel() + + lib, _ := setupTestLibrary(t) + seedAlbumsAndGenres(t, lib) + + got, err := lib.GetTracksByPaths([]string{ + "/other/b1.mp3", "/music/missing.mp3", "/music/a1.mp3", "/other/b1.mp3", + }) + if err != nil { + t.Fatalf("GetTracksByPaths: %v", err) + } + + paths := make([]string, 0, len(got)) + for _, tr := range got { + paths = append(paths, tr.FilePath) + } + + if want := "[/other/b1.mp3 /music/a1.mp3]"; fmt.Sprint(paths) != want { + t.Fatalf("paths = %v, want %s (caller order, missing dropped, duplicate once)", paths, want) + } + + // Whole tracks, not just the key: details render every field. + if got[1].TrackName != "A1" || len(got[1].Genre) != 2 { + t.Errorf("a1 = %+v, want title A1 with two genres", got[1]) + } +} + +func TestGetTracksByPathsSpansChunks(t *testing.T) { + t.Parallel() + + lib, _ := setupTestLibrary(t) + seedAlbumsAndGenres(t, lib) + + // The real path last, behind more misses than one chunk binds, so a + // loop that only ran the first chunk would return nothing. + paths := make([]string, 0, pathLookupChunk+2) + for i := range pathLookupChunk + 1 { + paths = append(paths, fmt.Sprintf("/nowhere/%d.mp3", i)) + } + + paths = append(paths, "/music/a2.mp3") + + got, err := lib.GetTracksByPaths(paths) + if err != nil { + t.Fatalf("GetTracksByPaths: %v", err) + } + + if len(got) != 1 || got[0].FilePath != "/music/a2.mp3" { + t.Fatalf("got %d tracks (%v), want only /music/a2.mp3", len(got), got) + } +} diff --git a/backend/library/tracktable.go b/backend/library/tracktable.go new file mode 100644 index 0000000..bc701c7 --- /dev/null +++ b/backend/library/tracktable.go @@ -0,0 +1,190 @@ +package library + +import ( + "fmt" + "strconv" + "strings" +) + +// TrackTable is every track in a library as the Tracks view uses it: +// one array per column, and every repeated string stored once (#281). +// +// GetTracks used to answer with one object per track, which at 26 138 +// tracks was 20.5 MB of JSON — ~350 bytes a row of key names, four +// cover URLs identical across an album, and artist, album and genre +// strings repeated on every track of the album. Encoding it cost the +// backend ~170 MB of transient allocation and parsing it was the +// WebView's peak. Here the keys appear once, a repeated string is a +// small integer, and the columns the Tracks view does not read are not +// sent at all: LastPlayed and the three larger cover tiers belong to +// the details dialog, which fetches whole tracks by path. +// +// The projection is still trackFromRow's — each row goes through it — +// so this is an encoding of a Track, never a second description of +// one. frontend/src/utils/track-table.ts is the only decoder. +type TrackTable struct { + // Strings holds every distinct string value; a string column holds + // indexes into it. Index 0 is always "". + Strings []string `json:"strings"` + // GenreSets holds every distinct genre list, as indexes into + // Strings; Genre holds an index into it per track. + GenreSets [][]uint32 `json:"genreSets"` + + FilePath []string `json:"filePath"` + TrackName []uint32 `json:"trackName"` + ArtistName []uint32 `json:"artistName"` + Album []uint32 `json:"album"` + Composer []uint32 `json:"composer"` + FileType []uint32 `json:"fileType"` + Genre []uint32 `json:"genre"` + ArtistMBID []uint32 `json:"artistMbid"` + ReleaseGroupMBID []uint32 `json:"releaseGroupMbid"` + RecordingMBID []uint32 `json:"recordingMbid"` + CoverArtSmall []uint32 `json:"coverArtSmall"` + + // LengthMs is Track.TrackLength as the number it encodes. + LengthMs []int64 `json:"lengthMs"` + TrackNumber []int64 `json:"trackNumber"` + DiscNumber []int64 `json:"discNumber"` + Year []int64 `json:"year"` + SampleRate []int64 `json:"sampleRate"` + BitDepth []int64 `json:"bitDepth"` + Channels []int64 `json:"channels"` + Bitrate []int64 `json:"bitrate"` + FileSize []int64 `json:"fileSize"` + PlayCount []int64 `json:"playCount"` +} + +// trackTableBuilder interns strings and genre lists while rows are +// appended. +type trackTableBuilder struct { + table TrackTable + strings map[string]uint32 + genres map[string]uint32 +} + +func newTrackTableBuilder(capacity int) *trackTableBuilder { + b := &trackTableBuilder{ + strings: map[string]uint32{"": 0}, + genres: map[string]uint32{}, + } + + t := &b.table + t.Strings = []string{""} + t.FilePath = make([]string, 0, capacity) + + for _, col := range b.stringColumns() { + *col = make([]uint32, 0, capacity) + } + + for _, col := range b.intColumns() { + *col = make([]int64, 0, capacity) + } + + t.Genre = make([]uint32, 0, capacity) + + return b +} + +// stringColumns are the interned columns, in one place so the builder +// cannot allocate one and forget to fill it. +func (b *trackTableBuilder) stringColumns() []*[]uint32 { + t := &b.table + + return []*[]uint32{ + &t.TrackName, &t.ArtistName, &t.Album, &t.Composer, &t.FileType, + &t.ArtistMBID, &t.ReleaseGroupMBID, &t.RecordingMBID, &t.CoverArtSmall, + } +} + +func (b *trackTableBuilder) intColumns() []*[]int64 { + t := &b.table + + return []*[]int64{ + &t.LengthMs, &t.TrackNumber, &t.DiscNumber, &t.Year, &t.SampleRate, + &t.BitDepth, &t.Channels, &t.Bitrate, &t.FileSize, &t.PlayCount, + } +} + +func (b *trackTableBuilder) intern(s string) uint32 { + if i, ok := b.strings[s]; ok { + return i + } + + i := uint32(len(b.table.Strings)) //nolint:gosec // bounded by the row count + + b.table.Strings = append(b.table.Strings, s) + b.strings[s] = i + + return i +} + +func (b *trackTableBuilder) internGenres(genres []string) uint32 { + key := strings.Join(genres, genreDelimiter) + + if i, ok := b.genres[key]; ok { + return i + } + + set := make([]uint32, len(genres)) + for j, g := range genres { + set[j] = b.intern(g) + } + + i := uint32(len(b.table.GenreSets)) //nolint:gosec // bounded by the row count + + b.table.GenreSets = append(b.table.GenreSets, set) + b.genres[key] = i + + return i +} + +func (b *trackTableBuilder) add(tr Track) error { + lengthMs, err := strconv.ParseInt(tr.TrackLength, 10, 64) + if err != nil { + return fmt.Errorf("track %q length %q: %w", tr.FilePath, tr.TrackLength, err) + } + + t := &b.table + t.FilePath = append(t.FilePath, tr.FilePath) + + strs := []string{ + tr.TrackName, tr.ArtistName, tr.Album, tr.Composer, tr.FileType, + tr.ArtistMBID, tr.ReleaseGroupMBID, tr.RecordingMBID, tr.CoverArtSmall, + } + for i, col := range b.stringColumns() { + *col = append(*col, b.intern(strs[i])) + } + + ints := []int64{ + lengthMs, tr.TrackNumber, tr.DiscNumber, tr.Year, tr.SampleRate, + tr.BitDepth, tr.Channels, tr.Bitrate, tr.FileSize, tr.PlayCount, + } + for i, col := range b.intColumns() { + *col = append(*col, ints[i]) + } + + t.Genre = append(t.Genre, b.internGenres(tr.Genre)) + + return nil +} + +// GetTrackTable returns every track in a library, or in all of them +// when libraryID is 0, as a TrackTable. An empty library is an empty +// table, not an error. +func (l *Library) GetTrackTable(libraryID int64) (TrackTable, error) { + rows, err := l.db.ReadQueries.GetTracks(l.ctx, libraryID) + if err != nil { + return TrackTable{}, fmt.Errorf("could not get tracks: %w", err) + } + + b := newTrackTableBuilder(len(rows)) + + for i := range rows { + if err := b.add(trackFromRow(rows[i])); err != nil { + return TrackTable{}, err + } + } + + return b.table, nil +} diff --git a/backend/library/tracktable_test.go b/backend/library/tracktable_test.go new file mode 100644 index 0000000..864426e --- /dev/null +++ b/backend/library/tracktable_test.go @@ -0,0 +1,198 @@ +package library + +import ( + "encoding/json" + "fmt" + "strconv" + "testing" + + "yellowjacket/backend/database" +) + +// seedTableLibrary seeds n albums of perAlbum tracks each, with a cover +// on every album, so the table has the repetition it exists to remove. +func seedTableLibrary(t *testing.T, lib *Library, albums, perAlbum int) { + t.Helper() + + for a := range albums { + for n := range perAlbum { + database.InsertTestTrack(t, lib.db, database.TestTrack{ + FilePath: fmt.Sprintf( + "/music/artist-%d/album-%d/%02d - Some Track Title.flac", a%7, a, n+1, + ), + Title: fmt.Sprintf("Some Track Title %d", n+1), + Artist: fmt.Sprintf("Artist %d", a%7), + ArtistMBID: fmt.Sprintf("0b7a8d2e-0000-4000-8000-%012d", a%7), + Album: fmt.Sprintf("Album Name %d", a), + AlbumMBID: fmt.Sprintf("1c7a8d2e-0000-4000-8000-%012d", a), + RecordingMBID: fmt.Sprintf("2d7a8d2e-0000-4000-8000-%012d", a*100+n), + Genres: []string{"Ambient", fmt.Sprintf("Genre %d", a%3)}, + TrackNumber: int64(n + 1), + DiscNumber: 1, + Year: 2000 + int64(a%20), + LengthMs: 200_000 + int64(n), + PlayCount: int64(n), + }) + } + + res, err := lib.db.ExecContext( + "INSERT INTO cover_art (file_path, mime_type) VALUES (?, 'image/jpeg')", + fmt.Sprintf("/data/covers/%064d.jpg", a), + ) + if err != nil { + t.Fatalf("seed cover: %v", err) + } + + coverID, _ := res.LastInsertId() + + if _, err := lib.db.ExecContext( + "UPDATE albums SET cover_art_id = ? WHERE name = ?", + coverID, fmt.Sprintf("Album Name %d", a), + ); err != nil { + t.Fatalf("seed album cover: %v", err) + } + } +} + +// decodeTable is the Go mirror of frontend/src/utils/track-table.ts, +// for asserting the encoding loses nothing it claims to carry. +func decodeTable(t *testing.T, tbl TrackTable) []Track { + t.Helper() + + str := func(col []uint32, i int) string { return tbl.Strings[col[i]] } + tracks := make([]Track, len(tbl.FilePath)) + + for i := range tracks { + var genres []string + for _, g := range tbl.GenreSets[tbl.Genre[i]] { + genres = append(genres, tbl.Strings[g]) + } + + tracks[i] = Track{ + FilePath: tbl.FilePath[i], + TrackName: str(tbl.TrackName, i), + ArtistName: str(tbl.ArtistName, i), + Album: str(tbl.Album, i), + Composer: str(tbl.Composer, i), + FileType: str(tbl.FileType, i), + ArtistMBID: str(tbl.ArtistMBID, i), + ReleaseGroupMBID: str(tbl.ReleaseGroupMBID, i), + RecordingMBID: str(tbl.RecordingMBID, i), + CoverArtSmall: str(tbl.CoverArtSmall, i), + Genre: genres, + TrackLength: strconv.FormatInt(tbl.LengthMs[i], 10), + TrackNumber: tbl.TrackNumber[i], + DiscNumber: tbl.DiscNumber[i], + Year: tbl.Year[i], + SampleRate: tbl.SampleRate[i], + BitDepth: tbl.BitDepth[i], + Channels: tbl.Channels[i], + Bitrate: tbl.Bitrate[i], + FileSize: tbl.FileSize[i], + PlayCount: tbl.PlayCount[i], + } + } + + return tracks +} + +// #281: the table is an encoding of trackFromRow's Track, minus the +// fields the Tracks view does not read. Decoding it must give back +// exactly that, row for row. +func TestTrackTableRoundTrip(t *testing.T) { + t.Parallel() + + lib, _ := setupTestLibrary(t) + seedTableLibrary(t, lib, 6, 4) + + tbl, err := lib.GetTrackTable(0) + if err != nil { + t.Fatalf("GetTrackTable: %v", err) + } + + rows, err := lib.db.ReadQueries.GetTracks(lib.ctx, 0) + if err != nil { + t.Fatalf("GetTracks: %v", err) + } + + if len(rows) != 24 || len(tbl.FilePath) != len(rows) { + t.Fatalf("table has %d rows, query %d, want 24", len(tbl.FilePath), len(rows)) + } + + got := decodeTable(t, tbl) + + for i, row := range rows { + want := trackFromRow(row) + // Not carried: the details dialog fetches these by path. + want.LastPlayed, want.CoverArtPath, want.CoverArtMedium, want.CoverArtLarge = "", "", "", "" + + if fmt.Sprintf("%+v", got[i]) != fmt.Sprintf("%+v", want) { + t.Fatalf("row %d:\n got %+v\nwant %+v", i, got[i], want) + } + } + + // It was the cover, genre and name repetition that was paid for; a + // table that interned nothing would still round-trip. + if len(tbl.GenreSets) != 3 { + t.Errorf("genre sets = %d, want 3 distinct lists", len(tbl.GenreSets)) + } + + if a, b := tbl.CoverArtSmall[0], tbl.CoverArtSmall[1]; a != b || tbl.Strings[a] == "" { + t.Errorf("two tracks of one album hold cover indexes %d and %d", a, b) + } +} + +// The point of the table, pinned: the same rows encode to a fraction of +// the object-per-track JSON they replace. A new column has to fit +// under this or raise it on purpose. +func TestTrackTableSizeBudget(t *testing.T) { + t.Parallel() + + lib, _ := setupTestLibrary(t) + seedTableLibrary(t, lib, 40, 10) + + tbl, err := lib.GetTrackTable(0) + if err != nil { + t.Fatalf("GetTrackTable: %v", err) + } + + rows, err := lib.db.ReadQueries.GetTracks(lib.ctx, 0) + if err != nil { + t.Fatalf("GetTracks: %v", err) + } + + asTable, err := json.Marshal(tbl) + if err != nil { + t.Fatal(err) + } + + asObjects, err := json.Marshal(tracksFromRows(rows)) + if err != nil { + t.Fatal(err) + } + + perTrack := len(asTable) / len(rows) + ratio := float64(len(asTable)) / float64(len(asObjects)) + + t.Logf("table %d B (%d B/track), objects %d B, ratio %.2f", + len(asTable), perTrack, len(asObjects), ratio) + + if ratio > 0.25 { + t.Errorf("table is %.0f%% of the object encoding, budget 25%%", ratio*100) + } +} + +func TestTrackTableEmptyLibraryIsAnAnswer(t *testing.T) { + t.Parallel() + + lib, _ := setupTestLibrary(t) + + tbl, err := lib.GetTrackTable(0) + if err != nil { + t.Fatalf("an empty library is an empty table, not an error: %v", err) + } + + if len(tbl.FilePath) != 0 || len(tbl.Strings) != 1 { + t.Errorf("empty table = %+v", tbl) + } +} diff --git a/backend/playlist/playlist.go b/backend/playlist/playlist.go index 126b531..983dc40 100644 --- a/backend/playlist/playlist.go +++ b/backend/playlist/playlist.go @@ -3125,6 +3125,19 @@ func (s *Service) EvaluateSmartPlaylist( return tracks, nil } +// SuggestSmartPlaylistValues returns values of field present in the +// library that contain needle, for the rule editor's value box. +func (s *Service) SuggestSmartPlaylistValues( + field, needle string, +) ([]string, error) { + values, err := smartplaylist.SuggestValues(s.db, field, needle) + if err != nil { + return nil, fmt.Errorf("suggest smart playlist values: %w", err) + } + + return values, nil +} + // PreviewSmartPlaylist evaluates a rule set from raw JSON without // requiring a saved playlist. This powers live preview in the rule // editor — the frontend sends rules as they are being edited and diff --git a/backend/smartplaylist/smartplaylist.go b/backend/smartplaylist/smartplaylist.go index 9705419..9b5b5b1 100644 --- a/backend/smartplaylist/smartplaylist.go +++ b/backend/smartplaylist/smartplaylist.go @@ -614,7 +614,12 @@ const leanTrackQuery = `SELECT af.file_size, af.play_count, COALESCE(af.last_played, '') AS last_played -FROM ( +FROM ` + leanTrackSource + +// leanTrackSource is the joined row every rule's column resolves +// against, aliased af. Shared by Evaluate and SuggestValues so a +// suggestion is always a value a rule on the same field can match. +const leanTrackSource = `( SELECT af.id, af.file_path, @@ -1117,3 +1122,79 @@ func splitGenres(concatenated string) []string { return strings.Split(concatenated, genreDelimiter) } + +// suggestLimit bounds one suggestion answer: a combobox shows a +// screenful, and the user narrows by typing. +const suggestLimit = 50 + +// errNoSuggestions is a field the editor does not offer values for. +var errNoSuggestions = errors.New("field has no suggestions") + +// suggestFields are the fields whose values the rule editor suggests. +// Every one is in fieldMap; numeric fields other than the years are +// ranges, where a list of every value present is no help. +var suggestFields = map[string]bool{ + "title": true, "artist": true, "album": true, "genre": true, + "composer": true, "file_type": true, "year": true, "release_year": true, +} + +// SuggestValues returns up to suggestLimit distinct values of field +// present in the library that contain needle (case-insensitively), +// sorted. +// +// The rule editor used to build these lists from libraryStore's +// whole-library arrays, which made the editor one more reason to fetch +// every track at startup (#279) — and gave it an empty list when the +// arrays had not landed. Asking for the values that match what has +// been typed is a few hundred bytes, whatever the library's size. +func SuggestValues(db *database.DB, field, needle string) ([]string, error) { + col, ok := fieldMap[field] + if !ok || !suggestFields[field] { + return nil, fmt.Errorf("%w: %q", errNoSuggestions, field) + } + + pattern := "%" + escapeLike(needle) + "%" + + // SAFETY: col comes from fieldMap, never from the caller; the + // needle is a bound parameter. + query := `SELECT DISTINCT CAST(af.` + col + ` AS TEXT) AS v FROM ` + + leanTrackSource + ` + WHERE v != '' AND v != '0' AND v LIKE ? ESCAPE '\' + ORDER BY v COLLATE NOCASE LIMIT ?` + + if field == "genre" { + query = `SELECT name FROM genres + WHERE name != '' AND name LIKE ? ESCAPE '\' + ORDER BY name COLLATE NOCASE LIMIT ?` + } + + rows, err := db.QueryContext(query, pattern, suggestLimit) + if err != nil { + return nil, fmt.Errorf("suggest %s: %w", field, err) + } + + defer func() { _ = rows.Close() }() + + values := make([]string, 0, suggestLimit) + + for rows.Next() { + var v string + if err := rows.Scan(&v); err != nil { + return nil, fmt.Errorf("suggest %s: %w", field, err) + } + + values = append(values, v) + } + + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("suggest %s: %w", field, err) + } + + return values, nil +} + +// escapeLike makes needle match itself literally inside a LIKE pattern +// with ESCAPE '\'. +func escapeLike(needle string) string { + return strings.NewReplacer(`\`, `\\`, `%`, `\%`, `_`, `\_`).Replace(needle) +} diff --git a/backend/smartplaylist/suggest_test.go b/backend/smartplaylist/suggest_test.go new file mode 100644 index 0000000..2496947 --- /dev/null +++ b/backend/smartplaylist/suggest_test.go @@ -0,0 +1,61 @@ +package smartplaylist + +import ( + "errors" + "slices" + "testing" + + "yellowjacket/backend/database" +) + +// The rule editor's value box asks the backend for what matches the +// text typed (#279), rather than building every list from the whole +// library loaded into the frontend. +func TestSuggestValues(t *testing.T) { + t.Parallel() + + db := database.NewTestDB(t) + seedSmartPlaylistData(t, db) + + cases := []struct { + field, needle string + want []string + }{ + // Case-insensitive substring, distinct, sorted. + {"artist", "q", []string{"QOTSA", "Queen"}}, + {"album", "OPERA", []string{"A Night at the Opera"}}, + // Genres come from the genre table, one value per genre, not + // from a concatenated per-track column. + {"genre", "rock", []string{ + "Funk Rock", "Hard Rock", "Progressive Rock", "Rock", "Stoner Rock", + }}, + // Years are numbers and are suggested as their text. + {"year", "199", []string{"1990", "1991"}}, + // The needle is literal: a LIKE wildcard in it matches only itself. + {"title", "%", []string{}}, + {"composer", "_", []string{}}, + } + + for _, tc := range cases { + got, err := SuggestValues(db, tc.field, tc.needle) + if err != nil { + t.Fatalf("SuggestValues(%q, %q): %v", tc.field, tc.needle, err) + } + + if !slices.Equal(got, tc.want) { + t.Errorf("SuggestValues(%q, %q) = %q, want %q", tc.field, tc.needle, got, tc.want) + } + } +} + +func TestSuggestValuesRefusesFieldsItDoesNotSuggest(t *testing.T) { + t.Parallel() + + db := database.NewTestDB(t) + + for _, field := range []string{"duration", "file_path", "title; DROP TABLE x"} { + if _, err := SuggestValues(db, field, ""); !errors.Is(err, errNoSuggestions) { + t.Errorf("SuggestValues(%q) err = %v, want errNoSuggestions", field, err) + } + } +} diff --git a/e2e/bench-tmp.mjs b/e2e/bench-tmp.mjs new file mode 100644 index 0000000..ddb2517 --- /dev/null +++ b/e2e/bench-tmp.mjs @@ -0,0 +1,27 @@ +import { chromium } from '@playwright/test'; +const b = await chromium.launch({ executablePath: '/usr/bin/chromium' }); +const p = await b.newPage(); +await p.addInitScript({ path: '/mnt/vault/dev/golang/yellowjacket/.playwright/init-events.js' }); +await p.goto('http://localhost:34115', { waitUntil: 'load' }); +await p.evaluate(() => window.__yjEvents.ready(30000)); +const out = await p.evaluate(async () => { + const time = async (label, path, args, n = 5) => { + const ms = []; + for (let i = 0; i < n; i++) { + const t0 = performance.now(); + await window.__yjEvents.call(path, args, 60000); + ms.push(performance.now() - t0); + } + ms.sort((a, b) => a - b); + return { label, medianMs: Math.round(ms[Math.floor(n / 2)]), all: ms.map((m) => Math.round(m)) }; + }; + return [ + await time('GetTrackTable(0)', 'library.Library.GetTrackTable', [0], 4), + await time('GetAlbums(0)', 'library.Library.GetAlbums', [0], 4), + await time('SearchLocal(tide)', 'explore.Service.SearchLocal', ['tide'], 4), + await time('SearchLocal(rock)', 'explore.Service.SearchLocal', ['rock'], 4), + await time('SearchTracks(tide)', 'library.Library.SearchTracks', ['tide', 0], 4), + ]; +}); +console.log(JSON.stringify(out, null, 1)); +await b.close(); diff --git a/e2e/perf/measure.mjs b/e2e/perf/measure.mjs index ac4b125..119857d 100644 --- a/e2e/perf/measure.mjs +++ b/e2e/perf/measure.mjs @@ -42,6 +42,13 @@ * browse number below could not see this, because it * visits Explore without ever typing in it. * heap JS heap after a scripted browse, post-GC. m3. + * memory what the app holds at rest after launch, before any + * view but the landing one is opened: the backend + * process's RSS and peak, its Go heap, the page's JS + * heap, and the bytes each binding returned on the way. + * Then the same after the first open of Tracks, and how + * long that open took to its first *row* — the number + * eager loading was buying (#280/#281). * * Usage: * node e2e/perf/measure.mjs --label before @@ -51,7 +58,7 @@ * Requires a running app: `make dev-headless SEED=bulk`. */ -import { readFileSync, writeFileSync, mkdirSync, existsSync } from 'node:fs'; +import { readFileSync, writeFileSync, mkdirSync, existsSync, readdirSync } from 'node:fs'; import { dirname, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; import { chromium } from '@playwright/test'; @@ -190,6 +197,135 @@ const SEARCH_DEBOUNCE_MS = 150; /* -------------------------------------------------------------------- */ +/** + * The backend process's memory, from /proc and its own pprof endpoint. + * + * Linux and a dev build only (pprof is mounted by `-tags dev`); either + * missing is a null, not a failure — this is a measurement harness, and + * a number it cannot take is reported as absent rather than as zero. + */ +async function backendMemory() { + const out = { rssMB: null, peakMB: null, anonMB: null, goHeapInuseMB: null, goHeapHeldMB: null }; + const pid = findBackendPid(); + + if (pid) { + try { + const status = readFileSync(`/proc/${pid}/status`, 'utf8'); + const kb = (k) => Number(status.match(new RegExp(`^${k}:\\s+(\\d+)`, 'm'))?.[1] ?? NaN); + + out.rssMB = round(kb('VmRSS') / 1024, 1); + out.peakMB = round(kb('VmHWM') / 1024, 1); + out.anonMB = round(kb('RssAnon') / 1024, 1); + } catch { + // The process went away between finding it and reading it. + } + } + + try { + const text = await (await fetch('http://localhost:6060/debug/pprof/heap?debug=1')).text(); + const field = (k) => Number(text.match(new RegExp(`^# ${k} = (\\d+)`, 'm'))?.[1] ?? NaN); + + out.goHeapInuseMB = round(field('HeapInuse') / 1048576, 1); + // What the runtime holds from the OS: the RSS the heap accounts for. + out.goHeapHeldMB = round((field('HeapSys') - field('HeapReleased')) / 1048576, 1); + } catch { + // Not a dev build, or pprof is not listening. + } + + return out; +} + +/** The pid of the running `yj-dev` binary, found by name under /proc. */ +function findBackendPid() { + try { + for (const entry of readdirSync('/proc')) { + if (!/^\d+$/.test(entry)) continue; + + try { + if (readFileSync(`/proc/${entry}/comm`, 'utf8').trim() === 'yj-dev') return entry; + } catch { + // Raced with an exiting process. + } + } + } catch { + // No /proc: not Linux. + } + + return null; +} + +/** Bytes returned per binding name, largest first. */ +function bytesByBinding(calls) { + const by = {}; + + for (const c of calls) by[c.path] = (by[c.path] ?? 0) + (c.bytes ?? 0); + + return Object.fromEntries( + Object.entries(by) + .filter(([, b]) => b > 0) + .sort((a, b) => b[1] - a[1]) + .map(([k, b]) => [k, round(b / 1048576, 2)]), + ); +} + +async function pageMemory(page, client) { + await client.send('HeapProfiler.collectGarbage'); + await page.waitForTimeout(300); + + const heap = await client.send('Runtime.getHeapUsage'); + const dom = await client.send('Memory.getDOMCounters'); + + return { jsHeapMB: round(heap.usedSize / 1048576, 1), domNodes: dom.nodes }; +} + +async function measureMemory(page, client) { + // Settled: the landing view has loaded and any idle warming has run. + await page.waitForTimeout(5000); + + const startupCalls = await page.evaluate(() => window.__yjPerf.calls); + const atRest = { + backend: await backendMemory(), + page: await pageMemory(page, client), + bytesByBindingMB: bytesByBinding(startupCalls), + totalBindingMB: round(startupCalls.reduce((n, c) => n + (c.bytes ?? 0), 0) / 1048576, 2), + }; + + // First open of Tracks, to its first row: what a user waits for when + // nothing was loaded ahead of them. + const since = await page.evaluate(() => performance.now()); + const firstRowMs = await page.evaluate(async () => { + const t0 = performance.now(); + + document.dispatchEvent(new CustomEvent('navigate', { detail: { view: 'tracks' } })); + + for (;;) { + const list = document.querySelector('#main-content > track-list:not(.view-hidden)'); + + if (list?.shadowRoot?.querySelector('[data-testid="track-row"]')) { + return Math.round(performance.now() - t0); + } + + if (performance.now() - t0 > 60000) return null; + await new Promise((r) => requestAnimationFrame(r)); + } + }); + await page.waitForTimeout(2000); + + const tracksCalls = await page.evaluate((t) => window.__yjPerf.since(t), since); + + return { + atRest, + tracksOpen: { + firstRowMs, + bytesByBindingMB: bytesByBinding(tracksCalls), + backend: await backendMemory(), + page: await pageMemory(page, client), + }, + }; +} + +/* -------------------------------------------------------------------- */ + async function measureStartup(page) { const nav = await page.evaluate(() => { const n = performance.getEntriesByType('navigation')[0]; @@ -224,20 +360,33 @@ async function measureStartup(page) { scriptBytesBeforePaint, requests: res.length, crossOriginRequests: crossOrigin.length, - crossOriginHosts: [...new Set(crossOrigin.map((u) => new URL(u).host))], + crossOriginHosts: [...new Set(crossOrigin.map((u) => { + try { + return new URL(u).host; + } catch { + return u; + } + }))], }; }); - // "First row on screen" is the number a user experiences as startup; - // FCP fires on the chrome around an empty list. + // "First row on screen" is the number a user experiences as startup + // *when the app lands on Tracks*; FCP fires on the chrome around an + // empty list. + // + // The deadline is short because since #280 a landing on any other + // view leaves the track list unloaded, and this would otherwise + // spend a minute waiting for a row that is not coming. The number + // that means something either way is `memory.tracksOpen.firstRowMs`, + // which opens Tracks and waits for its first row deliberately. const firstRowMs = await page.evaluate(async () => { const t0 = performance.now(); - const deadline = t0 + 60000; + const deadline = t0 + 2000; for (;;) { const list = document.querySelector('track-list'); const row = list?.shadowRoot?.querySelector('[role="row"], .track-row'); - if (row) return Math.round(performance.now() - t0 + (performance.timeOrigin ? 0 : 0)); + if (row) return Math.round(performance.now() - t0); if (performance.now() > deadline) return null; await new Promise((r) => setTimeout(r, 16)); } @@ -337,7 +486,8 @@ async function measureTrackChange(page) { await ev.call('queue.Queue.Clear', [], 10000).catch(() => {}); - const tracks = await ev.call('library.Library.GetAllTracks', [], 60000); + // The columnar list (#281): only the paths are needed here. + const tracks = ((await ev.call('library.Library.GetTrackTable', [0], 60000))?.filePath ?? []).map((FilePath) => ({ FilePath })); const paths = (tracks ?? []).slice(0, 4).map((t) => t.FilePath); if (paths.length < 2) return { error: 'library too small to measure' }; @@ -435,7 +585,8 @@ async function measureFavouriteToggle(page) { const ev = window.__yjEvents; const perf = window.__yjPerf; - const tracks = await ev.call('library.Library.GetAllTracks', [], 60000); + // The columnar list (#281): only the paths are needed here. + const tracks = ((await ev.call('library.Library.GetTrackTable', [0], 60000))?.filePath ?? []).map((FilePath) => ({ FilePath })); const paths = (tracks ?? []).map((t) => t.FilePath); if (paths.length < per * count) { return { error: `library too small: ${paths.length} tracks` }; @@ -671,7 +822,8 @@ async function measurePlaylistOpen(page, client) { let pl = (existing ?? []).find((p) => (p.Name ?? p.name) === name); if (!pl) { - const tracks = await ev.call('library.Library.GetAllTracks', [], 60000); + // The columnar list (#281): only the paths are needed here. + const tracks = ((await ev.call('library.Library.GetTrackTable', [0], 60000))?.filePath ?? []).map((FilePath) => ({ FilePath })); const paths = (tracks ?? []).map((t) => t.FilePath); if (paths.length < n) return { error: `library too small: ${paths.length}` }; @@ -1600,7 +1752,8 @@ async function measurePlayerBarPass(page) { // Stage a loaded track. Deliberately the same first tracks the // track-change measurement already played, so this warms no cover // art that a later measurement counts requests for. - const tracks = await ev.call('library.Library.GetAllTracks', [], 60000); + // The columnar list (#281): only the paths are needed here. + const tracks = ((await ev.call('library.Library.GetTrackTable', [0], 60000))?.filePath ?? []).map((FilePath) => ({ FilePath })); const paths = (tracks ?? []).slice(0, 4).map((t) => t.FilePath); if (paths.length < 2) return { error: 'library too small to measure' }; @@ -1835,6 +1988,12 @@ async function run(label) { loadWallMs: Date.now() - t0, }; + // First of all: at rest means before any measurement opens a view. + console.log(' memory at rest, then Tracks first open…'); + report.memory = await measureMemory(page, client); + await page.evaluate(() => document.dispatchEvent( + new CustomEvent('navigate', { detail: { view: 'home' } }), + )); console.log(' startup…'); report.startup = await measureStartup(page); // Before anything else navigates: every view's first open has to be @@ -1897,6 +2056,14 @@ async function run(label) { const ROWS = [ ['First contentful paint', (r) => fmt(r.startup.firstContentfulPaintMs, 'ms')], + ['At rest: backend RSS', (r) => fmt(r.memory?.atRest.backend.rssMB, 'MB')], + ['At rest: backend peak RSS', (r) => fmt(r.memory?.atRest.backend.peakMB, 'MB')], + ['At rest: Go heap held', (r) => fmt(r.memory?.atRest.backend.goHeapHeldMB, 'MB')], + ['At rest: JS heap', (r) => fmt(r.memory?.atRest.page.jsHeapMB, 'MB')], + ['At rest: binding bytes', (r) => fmt(r.memory?.atRest.totalBindingMB, 'MB')], + ['Tracks first open: first row', (r) => fmt(r.memory?.tracksOpen.firstRowMs, 'ms')], + ['Tracks open: backend peak RSS', (r) => fmt(r.memory?.tracksOpen.backend.peakMB, 'MB')], + ['Tracks open: JS heap', (r) => fmt(r.memory?.tracksOpen.page.jsHeapMB, 'MB')], ['First track row', (r) => fmt(r.startup.firstRowAfterLoadMs, 'ms')], ['JS transferred', (r) => fmt(round(r.startup.scriptBytes / 1024), 'kB')], ['JS evaluated before first paint', (r) => fmt(round((r.startup.scriptBytesBeforePaint ?? 0) / 1024), 'kB')], @@ -2000,7 +2167,11 @@ function compare(a, b) { const p = resolve(OUT_DIR, `${l}.json`); if (!existsSync(p)) throw new Error(`no measurement labelled '${l}' at ${p}`); - return JSON.parse(readFileSync(p, 'utf8')); + try { + return JSON.parse(readFileSync(p, 'utf8')); + } catch (err) { + throw new Error(`measurement '${l}' at ${p} is not valid JSON: ${err.message}`); + } }; console.log(table([load(a), load(b)])); diff --git a/e2e/specs/binding-names.spec.ts b/e2e/specs/binding-names.spec.ts new file mode 100644 index 0000000..106ec1a --- /dev/null +++ b/e2e/specs/binding-names.spec.ts @@ -0,0 +1,80 @@ +/** + * Every bound method the suite names by hand must exist. + * + * A binding call carries only a method id, so a spec that names a method + * the Go side no longer has fails at *runtime*, in whichever spec + * happens to call it, with `unknown bound method name` — and nothing + * before that. `library.Library.GetTracks` was replaced by + * `GetTrackTable` (#281) and four specs kept calling the old name: the + * suite reported twenty failures across transport, bottom-bar and + * reduced-motion specs, none of which mention the library list. + * + * The known names come from `support/method-ids.mjs`, which derives them + * from the generated bindings tree that `make bindings-check` keeps + * current — so this cannot disagree with what the app can answer. + * + * It asserts first that it read something: a glob that matched nothing + * would pass over an empty list. + */ +import { readFileSync, readdirSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import { dirname, join } from 'node:path'; + +import { test, expect } from '../support/fixtures.js'; +import { methodIDs } from '../support/method-ids.mjs'; + +const here = dirname(fileURLToPath(import.meta.url)); + +/** The specs, and the harness that calls bindings on their behalf. */ +const DIRS = [here, join(here, '..', 'support')]; + +function sources(): Array<[string, string]> { + const out: Array<[string, string]> = []; + + for (const dir of DIRS) { + for (const entry of readdirSync(dir)) { + if (!entry.endsWith('.ts')) continue; + if (entry.endsWith('.spec.ts') && entry === 'binding-names.spec.ts') continue; + + const path = join(dir, entry); + + out.push([path, readFileSync(path, 'utf8')]); + } + } + + return out; +} + +/** `'library.Library.GetTrackTable'` — package, service, method. */ +const NAMED_BINDING = /'([a-z][A-Za-z0-9]*\.[A-Z]\w*\.[A-Z]\w*)'/g; + +/** + * Names that are deliberately not bindings. + * + * `harness.spec.ts` calls an unknown method on purpose, to assert that + * the bridge rejects with a ReferenceError naming it rather than + * hanging — that spec is the reason a bad call is loud. + */ +const DELIBERATE = new Set(['queue.Queue.Nope']); + +test('every binding named by a spec exists', () => { + // `methodIDs()` is the id -> name map the recorder names calls with. + const known = new Set(methodIDs().values()); + const files = sources(); + + // A sweep over an empty glob passes and proves nothing. + expect(files.length, 'specs and support files read').toBeGreaterThan(20); + expect(known.size, 'bound method names derived').toBeGreaterThan(100); + + const unknown: string[] = []; + + for (const [path, source] of files) { + for (const [, name] of source.matchAll(NAMED_BINDING)) { + if (known.has(name!) || DELIBERATE.has(name!)) continue; + + unknown.push(`${name} (${path.split('/').pop()})`); + } + } + + expect([...new Set(unknown)]).toEqual([]); +}); diff --git a/e2e/specs/bottom-bar.spec.ts b/e2e/specs/bottom-bar.spec.ts index 734b4a9..d4a21ca 100644 --- a/e2e/specs/bottom-bar.spec.ts +++ b/e2e/specs/bottom-bar.spec.ts @@ -1,4 +1,10 @@ -import { test, expect, callBinding, NO_QUEUE_SOURCE } from '../support/fixtures.js'; +import { + test, + expect, + callBinding, + libraryTracks, + NO_QUEUE_SOURCE, +} from '../support/fixtures.js'; import type { Page } from '@playwright/test'; /** @@ -39,15 +45,7 @@ const geometry = (app: Page) => /** Something has to be playing before the transport draws a seek bar. */ async function play(app: Page): Promise { - const paths = await app.evaluate(async () => { - const tracks = (await window.__yjEvents.call( - 'library.Library.GetTracks', - [0], - 10_000, - )) as { FilePath: string }[]; - - return tracks.slice(0, 3).map((t) => t.FilePath); - }); + const paths = (await libraryTracks(app)).slice(0, 3).map((t) => t.FilePath); await callBinding(app, 'queue.Queue.SetQueue', [ paths, diff --git a/e2e/specs/library-lazy.spec.ts b/e2e/specs/library-lazy.spec.ts new file mode 100644 index 0000000..fec0766 --- /dev/null +++ b/e2e/specs/library-lazy.spec.ts @@ -0,0 +1,58 @@ +/** + * #280: launching the app must not load the track list. + * + * Every collection used to be fetched at `DOMContentLoaded` and + * refetched on every invalidation, whichever view was showing. On a + * 26 138-track library the track list was 20.5 MB of that, and encoding + * it cost the backend ~170 MB of transient allocation — for someone + * looking at Home, which draws none of it. Measured on 50 000 tracks: + * 543 MB of backend RSS at rest before, 296 MB after. + * + * The negative assertion is the point, and it needs its complement: "no + * `GetTrackTable`" also holds on a build that fetches nothing at all, + * so the same spec opens Tracks and watches it arrive. + */ +import { test, expect, bindingCalls, navigateTo } from '../support/fixtures.js'; + +const TRACKS = 'library.Library.GetTrackTable'; + +/** The seed's default page, which is what a launch lands on. */ +const LANDING_VIEW = 'home'; + +test('launching on Home does not load the track list', async ({ app }) => { + // Past the store's idle warm-up: "nothing was fetched" has to mean + // nothing, not "nothing has happened yet". + await app.waitForTimeout(3_500); + + const calls = await bindingCalls(app); + + expect(calls, 'the warm-up for the small collections ran').toContain( + 'library.Library.GetAlbums', + ); + expect(calls, `the track list was fetched at launch (${LANDING_VIEW})`).not.toContain( + TRACKS, + ); +}); + +test('opening Tracks loads it, and only then', async ({ app }) => { + await app.waitForTimeout(1_000); + expect(await bindingCalls(app)).not.toContain(TRACKS); + + await navigateTo(app, 'tracks'); + await expect(app.getByTestId('track-row').first()).toBeVisible(); + + expect(await bindingCalls(app)).toContain(TRACKS); +}); + +test('a hover on the nav item starts the load before the click', async ({ app }) => { + await app.waitForTimeout(1_000); + + const nav = app.getByTestId('nav-tracks'); + + // The pointer entering the item is the whole mechanism; nothing is + // clicked, so the data must arrive because of the hover alone. + await nav.hover(); + await expect + .poll(async () => (await bindingCalls(app)).includes(TRACKS)) + .toBe(true); +}); diff --git a/e2e/specs/phone-entity-links.spec.ts b/e2e/specs/phone-entity-links.spec.ts index 3f0c0d5..032e613 100644 --- a/e2e/specs/phone-entity-links.spec.ts +++ b/e2e/specs/phone-entity-links.spec.ts @@ -1,6 +1,7 @@ import { test, expect, + libraryTracks, callBinding, openTheQueue, NO_QUEUE_SOURCE, @@ -56,18 +57,11 @@ async function menuLabels(app: Page): Promise { * the wrong "no link". */ async function queueThree(app: Page): Promise { - const paths = await app.evaluate(async () => { - const tracks = (await window.__yjEvents.call( - 'library.Library.GetTracks', - [0], - 10_000, - )) as { FilePath: string; Album: string; ArtistName: string }[]; - - return tracks - .filter((t) => t.Album !== '' && t.ArtistName !== '') - .slice(0, 3) - .map((t) => t.FilePath); - }); + const tracks = await libraryTracks(app); + const paths = tracks + .filter((t) => t.Album !== '' && t.ArtistName !== '') + .slice(0, 3) + .map((t) => t.FilePath); await callBinding(app, 'queue.Queue.SetQueue', [ paths, diff --git a/e2e/specs/phone-now-playing.spec.ts b/e2e/specs/phone-now-playing.spec.ts index 29d1595..b3cba16 100644 --- a/e2e/specs/phone-now-playing.spec.ts +++ b/e2e/specs/phone-now-playing.spec.ts @@ -1,4 +1,4 @@ -import { test, expect } from '../support/fixtures.js'; +import { test, expect, libraryTracks } from '../support/fixtures.js'; /** * Now Playing on a short screen (#51). @@ -57,19 +57,15 @@ const REFLOW_AT = 500; /** Put a track in the player, so the view has art and names to lay out. */ async function stageATrack(page: Page): Promise { - await page.evaluate(async () => { - const tracks = (await window.__yjEvents.call( - 'library.Library.GetTracks', - [0], - 10_000, - )) as { FilePath: string }[]; + const paths = (await libraryTracks(page)).slice(0, 4).map((t) => t.FilePath); + await page.evaluate(async (paths) => { await window.__yjEvents.call( 'queue.Queue.SetQueue', - [tracks.slice(0, 4).map((t) => t.FilePath), 0, false, { type: '', id: 0, label: '' }], + [paths, 0, false, { type: '', id: 0, label: '' }], 10_000, ); - }); + }, paths); } /** Open the full-screen view and wait for the shell to say so. */ diff --git a/e2e/specs/phone-progress-line.spec.ts b/e2e/specs/phone-progress-line.spec.ts index 006e79f..47b8182 100644 --- a/e2e/specs/phone-progress-line.spec.ts +++ b/e2e/specs/phone-progress-line.spec.ts @@ -1,4 +1,5 @@ import { + libraryTracks, test, expect, callBinding, @@ -49,11 +50,7 @@ async function rectOf(app: Page, selector: string): Promise { * three rectangles. */ async function play(app: Page): Promise { - const tracks = await callBinding<{ FilePath: string; TrackName: string }[]>( - app, - 'library.Library.GetTracks', - [0], - ); + const tracks = await libraryTracks(app); // `TrackName`, not `Title`: that is what the library model calls it. const long = tracks.find((t) => t.TrackName === LONG_TRACK); diff --git a/e2e/specs/phone-shell.spec.ts b/e2e/specs/phone-shell.spec.ts index 7653c34..5ed5721 100644 --- a/e2e/specs/phone-shell.spec.ts +++ b/e2e/specs/phone-shell.spec.ts @@ -1,4 +1,4 @@ -import { test, expect, LONG_TRACK } from '../support/fixtures.js'; +import { test, expect, LONG_TRACK, libraryTracks } from '../support/fixtures.js'; /** * The phone shell (plan 016 B2, phase 1). @@ -235,15 +235,8 @@ test.describe('the shell on a phone', () => { // nothing in particular and picked a 2-second track, which had // finished before the assertions ran. The placeholder check below // is what actually holds the property this test needs. - const started = await app.evaluate(async (longTitle) => { - const tracks = (await window.__yjEvents.call( - 'library.Library.GetTracks', - [0], - 10_000, - )) as { FilePath: string; TrackName: string }[]; - - const bare = tracks.find((t) => t.TrackName === longTitle); - + const bare = (await libraryTracks(app)).find((t) => t.TrackName === LONG_TRACK); + const started = await app.evaluate(async (bare) => { if (!bare) return null; await window.__yjEvents.call( @@ -254,7 +247,7 @@ test.describe('the shell on a phone', () => { await window.__yjEvents.call('queue.Queue.Play', [], 5_000); return bare.TrackName; - }, LONG_TRACK); + }, bare); expect(started).toBe(LONG_TRACK); diff --git a/e2e/specs/phone-transport.spec.ts b/e2e/specs/phone-transport.spec.ts index a6a40cc..3ab77e4 100644 --- a/e2e/specs/phone-transport.spec.ts +++ b/e2e/specs/phone-transport.spec.ts @@ -1,4 +1,4 @@ -import { test, expect } from '../support/fixtures.js'; +import { test, expect, libraryTracks } from '../support/fixtures.js'; /** * The phone's transport (#59, #56). @@ -64,19 +64,15 @@ async function sizeOf( /** Put something in the queue, so the transport has a track to act on. */ async function stageATrack(page: Page): Promise { - await page.evaluate(async () => { - const tracks = (await window.__yjEvents.call( - 'library.Library.GetTracks', - [0], - 10_000, - )) as { FilePath: string }[]; + const paths = (await libraryTracks(page)).slice(0, 4).map((t) => t.FilePath); + await page.evaluate(async (paths) => { await window.__yjEvents.call( 'queue.Queue.SetQueue', - [tracks.slice(0, 4).map((t) => t.FilePath), 0, false, { type: '', id: 0, label: '' }], + [paths, 0, false, { type: '', id: 0, label: '' }], 10_000, ); - }); + }, paths); } test.describe('the phone bar carries three controls', () => { diff --git a/e2e/specs/play-count.spec.ts b/e2e/specs/play-count.spec.ts index af24af0..b4eb625 100644 --- a/e2e/specs/play-count.spec.ts +++ b/e2e/specs/play-count.spec.ts @@ -40,6 +40,21 @@ const FINISH_TIMEOUT = 60_000; const libraryCalls = async (app: Page): Promise => (await bindingCalls(app)).filter((c) => c.startsWith('library.Library.')); +/** + * The bindings that fetch a collection. + * + * A prefix test (GetAll*) used to stand in for this, and matched + * exactly one name — GetAllLibrariesWithTrackCounts — so the assertion + * below held whatever the app refetched. The sweep in + * binding-names.spec.ts is what found it. + */ +const COLLECTION_FETCHES = [ + 'library.Library.GetTrackTable', + 'library.Library.GetAlbums', + 'library.Library.GetArtists', + 'library.Library.GetGenres', +]; + /** * Select rows by dispatching on the row rather than clicking it. * @@ -116,7 +131,7 @@ test.describe('a finished track', () => { const refetched = await libraryCalls(app); expect( - refetched.filter((c) => c.startsWith('library.Library.GetAll')), + refetched.filter((c) => COLLECTION_FETCHES.includes(c)), 'a play refetched a collection', ).toEqual([]); diff --git a/e2e/specs/queue-reorder.spec.ts b/e2e/specs/queue-reorder.spec.ts index cd37cf7..b8f19a5 100644 --- a/e2e/specs/queue-reorder.spec.ts +++ b/e2e/specs/queue-reorder.spec.ts @@ -1,4 +1,5 @@ import { + libraryTracks, test, expect, callBinding, @@ -30,18 +31,7 @@ async function order(app: Page): Promise { } async function queueFourAndOpen(app: Page): Promise { - const paths: string[] = await app.evaluate(async () => { - // One argument, and 0 means every library: the scoped and - // unscoped list queries collapsed into one when the schema did - // (plan 013 R3), so `GetTracks()` no longer exists to call. - const tracks = await window.__yjEvents.call( - 'library.Library.GetTracks', - [0], - 10_000, - ); - - return (tracks as { FilePath: string }[]).slice(0, 4).map((t) => t.FilePath); - }); + const paths = (await libraryTracks(app)).slice(0, 4).map((t) => t.FilePath); await callBinding(app, 'queue.Queue.SetQueue', [paths, 0, false, NO_QUEUE_SOURCE]); diff --git a/e2e/specs/queue-selection.spec.ts b/e2e/specs/queue-selection.spec.ts index 0e5023a..223056f 100644 --- a/e2e/specs/queue-selection.spec.ts +++ b/e2e/specs/queue-selection.spec.ts @@ -1,4 +1,5 @@ import { + libraryTracks, test, expect, callBinding, @@ -86,15 +87,11 @@ const selected = (app: Page) => * is read back. */ async function queueSixAndOpen(app: Page): Promise { - const paths = await app.evaluate(async (longTitle) => { - // `TrackName`, not `Title`: the library model names it after the - // tag, and the *queue* is what calls it `title`. - const tracks = (await window.__yjEvents.call( - 'library.Library.GetTracks', - [0], - 10_000, - )) as { FilePath: string; TrackName: string; Album: string }[]; - + // `TrackName`, not `Title`: the library model names it after the + // tag, and the *queue* is what calls it `title`. + const tracks = await libraryTracks(app); + const longTitle = LONG_TRACK; + const paths = (() => { const long = tracks.find((t) => t.TrackName === longTitle); /** @@ -131,7 +128,7 @@ async function queueSixAndOpen(app: Page): Promise { long!.FilePath, ...rest.slice(3).map((t) => t.FilePath), ]; - }, LONG_TRACK); + })(); await callBinding(app, 'queue.Queue.SetQueue', [ paths, diff --git a/e2e/specs/reduced-motion.spec.ts b/e2e/specs/reduced-motion.spec.ts index 8eacb70..d65a37b 100644 --- a/e2e/specs/reduced-motion.spec.ts +++ b/e2e/specs/reduced-motion.spec.ts @@ -2,6 +2,7 @@ import { test, expect, callBinding, + libraryTracks, waitForEvent, NO_QUEUE_SOURCE, } from '../support/fixtures.js'; @@ -56,20 +57,9 @@ async function playTheLongOne(app: Page): Promise { window.dispatchEvent(new CustomEvent('yj-scroll-mode-changed')); }); - const paths: string[] = await app.evaluate(async (needle) => { - // One argument, and 0 means every library: the scoped and - // unscoped list queries collapsed into one when the schema did - // (plan 013 R3), so `GetTracks()` no longer exists to call. - const tracks = await window.__yjEvents.call( - 'library.Library.GetTracks', - [0], - 10_000, - ); - - return (tracks as { TrackName: string; FilePath: string }[]) - .filter((t) => t.TrackName.startsWith(needle)) - .map((t) => t.FilePath); - }, LONG_TITLE); + const paths: string[] = (await libraryTracks(app)) + .filter((t) => t.TrackName.startsWith(LONG_TITLE)) + .map((t) => t.FilePath); expect(paths.length).toBeGreaterThan(0); diff --git a/e2e/support/fixtures.ts b/e2e/support/fixtures.ts index e495687..549237b 100644 --- a/e2e/support/fixtures.ts +++ b/e2e/support/fixtures.ts @@ -96,6 +96,39 @@ export async function callBinding( ) as Promise; } +/** A library track as the specs use it: the path and its names. */ +export interface LibraryTrack { + FilePath: string; + TrackName: string; + ArtistName: string; + Album: string; +} + +/** + * Every track in the library, in the order the backend lists them. + * + * The list arrives as a columnar `TrackTable` since #281 (repeated + * strings sent once, as indexes into `strings`), so it cannot be read + * as an array of tracks any more. This reads the three columns the + * specs use; `frontend/src/utils/track-table.ts` is the real decoder. + */ +export async function libraryTracks(page: Page): Promise { + const t = await callBinding<{ + strings: string[]; + filePath: string[]; + trackName: number[]; + artistName: number[]; + album: number[]; + }>(page, 'library.Library.GetTrackTable', [0]); + + return (t.filePath ?? []).map((FilePath, i) => ({ + FilePath, + TrackName: t.strings[t.trackName[i]!] ?? '', + ArtistName: t.strings[t.artistName[i]!] ?? '', + Album: t.strings[t.album[i]!] ?? '', + })); +} + /** * The binding calls the *app* made, newest last, as `pkg.Type.Method`. * diff --git a/frontend/bindings/yellowjacket/backend/library/index.ts b/frontend/bindings/yellowjacket/backend/library/index.ts index d18a806..e974b53 100644 --- a/frontend/bindings/yellowjacket/backend/library/index.ts +++ b/frontend/bindings/yellowjacket/backend/library/index.ts @@ -18,5 +18,6 @@ export type { ScanMetrics, ScanWarning, Track, - TrackMBIDs + TrackMBIDs, + TrackTable } from "./models.js"; diff --git a/frontend/bindings/yellowjacket/backend/library/library.ts b/frontend/bindings/yellowjacket/backend/library/library.ts index 0591049..e7e9e53 100644 --- a/frontend/bindings/yellowjacket/backend/library/library.ts +++ b/frontend/bindings/yellowjacket/backend/library/library.ts @@ -203,16 +203,12 @@ export function GetTrackMBIDs(filePath: string): $CancellablePromise<$models.Tra } /** - * GetTracks returns every track in a library, or in all of them when - * libraryID is 0. - * - * The library id is a parameter rather than a second method because the - * two used to be separate queries, separate bindings and a branch at - * every call site - and the scoped form costs nothing (measured: 23 ms - * against 21 ms over 26k rows). + * GetTrackTable returns every track in a library, or in all of them + * when libraryID is 0, as a TrackTable. An empty library is an empty + * table, not an error. */ -export function GetTracks(libraryID: number): $CancellablePromise<$models.Track[] | null> { - return $Call.ByID(933082923, libraryID); +export function GetTrackTable(libraryID: number): $CancellablePromise<$models.TrackTable> { + return $Call.ByID(1179690926, libraryID); } /** @@ -222,6 +218,21 @@ export function GetTracksByGenre(genre: string, libraryID: number): $Cancellable return $Call.ByID(1674220245, genre, libraryID); } +/** + * GetTracksByPaths returns whole tracks for the given file paths, in + * the order asked, dropping any path that is not in the library. + * + * It is how the frontend resolves the tracks a surface is actually + * showing (#279). Track details from the queue, a playlist or a smart + * playlist used to look the path up in the whole library's track + * array, which had to be loaded first — so it was fetched eagerly at + * startup, 20.5 MB at 26k tracks, to answer questions about a handful + * of rows. + */ +export function GetTracksByPaths(paths: string[] | null): $CancellablePromise<$models.Track[] | null> { + return $Call.ByID(3966945290, paths); +} + /** * IsScanActive returns whether a scan is currently running. */ diff --git a/frontend/bindings/yellowjacket/backend/library/models.ts b/frontend/bindings/yellowjacket/backend/library/models.ts index f2c9987..c1a14f9 100644 --- a/frontend/bindings/yellowjacket/backend/library/models.ts +++ b/frontend/bindings/yellowjacket/backend/library/models.ts @@ -239,3 +239,60 @@ export interface TrackMBIDs { "releaseGroupMbid": string; "artistMbid": string; } + +/** + * TrackTable is every track in a library as the Tracks view uses it: + * one array per column, and every repeated string stored once (#281). + * + * GetTracks used to answer with one object per track, which at 26 138 + * tracks was 20.5 MB of JSON — ~350 bytes a row of key names, four + * cover URLs identical across an album, and artist, album and genre + * strings repeated on every track of the album. Encoding it cost the + * backend ~170 MB of transient allocation and parsing it was the + * WebView's peak. Here the keys appear once, a repeated string is a + * small integer, and the columns the Tracks view does not read are not + * sent at all: LastPlayed and the three larger cover tiers belong to + * the details dialog, which fetches whole tracks by path. + * + * The projection is still trackFromRow's — each row goes through it — + * so this is an encoding of a Track, never a second description of + * one. frontend/src/utils/track-table.ts is the only decoder. + */ +export interface TrackTable { + /** + * Strings holds every distinct string value; a string column holds + * indexes into it. Index 0 is always "". + */ + "strings": string[] | null; + + /** + * GenreSets holds every distinct genre list, as indexes into + * Strings; Genre holds an index into it per track. + */ + "genreSets": (number[] | null)[] | null; + "filePath": string[] | null; + "trackName": number[] | null; + "artistName": number[] | null; + "album": number[] | null; + "composer": number[] | null; + "fileType": number[] | null; + "genre": number[] | null; + "artistMbid": number[] | null; + "releaseGroupMbid": number[] | null; + "recordingMbid": number[] | null; + "coverArtSmall": number[] | null; + + /** + * LengthMs is Track.TrackLength as the number it encodes. + */ + "lengthMs": number[] | null; + "trackNumber": number[] | null; + "discNumber": number[] | null; + "year": number[] | null; + "sampleRate": number[] | null; + "bitDepth": number[] | null; + "channels": number[] | null; + "bitrate": number[] | null; + "fileSize": number[] | null; + "playCount": number[] | null; +} diff --git a/frontend/bindings/yellowjacket/backend/playlist/service.ts b/frontend/bindings/yellowjacket/backend/playlist/service.ts index d3cde05..efb7dec 100644 --- a/frontend/bindings/yellowjacket/backend/playlist/service.ts +++ b/frontend/bindings/yellowjacket/backend/playlist/service.ts @@ -309,6 +309,14 @@ export function SearchLibrary(query: string): $CancellablePromise<$models.Candid return $Call.ByID(3912116995, query); } +/** + * SuggestSmartPlaylistValues returns values of field present in the + * library that contain needle, for the rule editor's value box. + */ +export function SuggestSmartPlaylistValues(field: string, needle: string): $CancellablePromise { + return $Call.ByID(3718198115, field, needle); +} + /** * ToggleDefaultPlaylistTrack adds or removes a single track * from the default playlist. Returns true if the track is now diff --git a/frontend/index.html b/frontend/index.html index cebe75a..bdcd761 100644 --- a/frontend/index.html +++ b/frontend/index.html @@ -51,7 +51,15 @@
- + +
diff --git a/frontend/index.ts b/frontend/index.ts index d97d80a..0527839 100644 --- a/frontend/index.ts +++ b/frontend/index.ts @@ -200,13 +200,18 @@ const mainContent = document.getElementById('main-content'); // tracked as currentViewEl, is never hidden: two visible primary views // splitting the main panel between them regardless of which is // selected. +// +// It is deliberately not activated here. It is markup, not a decision: +// the shell does not yet know which view the launch lands on, and +// activating it starts the Tracks view's work — its data fetch, above +// all (#280) — for a launch that is about to land on Home. The first +// navigation is what activates whichever view it lands on. if (mainContent) { const initialTrackList = mainContent.querySelector('track-list'); if (initialTrackList) { viewCache.set('tracks', initialTrackList as HTMLElement); currentViewEl = initialTrackList as HTMLElement; - activateView(currentViewEl); } } @@ -580,10 +585,14 @@ async function handleNavigate( } default: { const fallback = document.createElement('div'); + const message = document.createElement('p'); fallback.style.padding = '1em'; fallback.style.color = 'var(--yj-text-secondary, #b3b3b3)'; - fallback.innerHTML = `

Coming soon: ${view}

`; + // textContent, not innerHTML: `view` comes from a navigation + // detail, which is app-supplied but not app-owned. + message.textContent = `Coming soon: ${view}`; + fallback.append(message); mainContent.appendChild(fallback); currentDetailEl = fallback; } @@ -620,6 +629,10 @@ function warmViewChunks(): void { /** requestIdleCallback where it exists; WebKit2GTK does not have it. */ function schedule(fn: () => void): void { + // SAFETY: `requestIdleCallback` is not in this project's DOM lib + // types, so it is read through an optional-property shape; the + // property is either absent (undefined) or the browser's own + // function, which is what the guard below checks. const ric = ( window as unknown as { requestIdleCallback?: (cb: () => void) => number; diff --git a/frontend/src/components/combobox/combobox.ts b/frontend/src/components/combobox/combobox.ts index 94b166e..9df22c5 100644 --- a/frontend/src/components/combobox/combobox.ts +++ b/frontend/src/components/combobox/combobox.ts @@ -7,6 +7,11 @@ import { designTokens } from '../../styles/tokens.css'; * keyboard navigation. Accepts a flat `options` string array, filters as * the user types, and emits `combobox-change` when a value is selected. * + * It also emits `combobox-input` with `{ text }` whenever the text it + * filters by changes (typing, and the reset to empty on focus), so a + * host whose options are too many to hand over at once can fetch the + * ones matching what was typed instead (#279). + * * Key implementation detail: option `
  • ` elements use `@mousedown` with * `e.preventDefault()` so that the input's `blur` event does not close the * dropdown before the click registers. @@ -196,6 +201,7 @@ export class YjCombobox extends LitElement { this.filterText = input.value; this.open = true; this.highlightedIndex = -1; + this.announceInput(); } private handleFocus() { @@ -203,6 +209,17 @@ export class YjCombobox extends LitElement { this.filterText = ''; this.open = true; this.highlightedIndex = -1; + this.announceInput(); + } + + private announceInput() { + this.dispatchEvent( + new CustomEvent('combobox-input', { + detail: { text: this.filterText }, + bubbles: true, + composed: true, + }), + ); } private handleBlur() { diff --git a/frontend/src/components/playlist-details/playlist-details.ts b/frontend/src/components/playlist-details/playlist-details.ts index f38f359..4614f6b 100644 --- a/frontend/src/components/playlist-details/playlist-details.ts +++ b/frontend/src/components/playlist-details/playlist-details.ts @@ -56,12 +56,9 @@ import { createTrackCardDragImage, removeDragImage, } from '@utils/drag-image'; -import { libraryStore } from '@store/library-store'; import '@components/playlist-picker/playlist-picker.js'; -import { loadTrackDetails } from '@utils/lazy-track-details.js'; -import { tracksByFilePath, tracksForPaths } from '@utils/track-index.js'; +import { showTrackDetailsForPath, showBatchTrackDetailsForPaths } from '@utils/track-details-opener.js'; import type { TrackDetails } from '@components/track-details/track-details.js'; -import type { CoverArtUrls } from '@components/track-details/track-details.js'; import '@components/phantom-resolver/phantom-resolver.js'; import type { PhantomResolver } from '@components/phantom-resolver/phantom-resolver.js'; import '@components/duplicate-tracks-dialog/duplicate-tracks-dialog.js'; @@ -758,76 +755,21 @@ export class PlaylistDetails } private async openTrackDetails(filePath: string) { - const tracks = libraryStore.getCachedTracks(); - const track = tracks - ? tracksByFilePath(tracks).get(filePath) - : undefined; - - if (!track) return; - - const ready = await loadTrackDetails( + await showTrackDetailsForPath( + () => this.trackDetailsDialog, + filePath, () => void this.openTrackDetails(filePath), ); - - if (!ready) return; - - const coverArt = track.CoverArtPath - ? { - coverArtPath: track.CoverArtPath, - coverArtSmall: track.CoverArtSmall, - coverArtMedium: track.CoverArtMedium, - coverArtLarge: track.CoverArtLarge, - } - : undefined; - - this.trackDetailsDialog?.show( - track, - coverArt, - ); } private async openBatchTrackDetails( filePaths: string[], ) { - const cachedTracks = - libraryStore.getCachedTracks(); - - if (!cachedTracks) return; - - const tracks = tracksForPaths( - cachedTracks, + await showBatchTrackDetailsForPaths( + () => this.trackDetailsDialog, filePaths, - ); - - if (tracks.length === 0) return; - - const ready = await loadTrackDetails( () => void this.openBatchTrackDetails(filePaths), ); - - if (!ready) return; - - const first = tracks[0]!; - const albumNames = new Set(tracks.map((t) => t.Album)); - let coverArt: CoverArtUrls | null = null; - let coverArtMixed = false; - - if (albumNames.size === 1 && first.CoverArtPath) { - coverArt = { - coverArtPath: first.CoverArtPath, - coverArtSmall: first.CoverArtSmall, - coverArtMedium: first.CoverArtMedium, - coverArtLarge: first.CoverArtLarge, - }; - } else if (albumNames.size > 1) { - coverArtMixed = true; - } - - this.trackDetailsDialog?.showBatch( - tracks, - coverArt, - coverArtMixed, - ); } /** diff --git a/frontend/src/components/queue-panel/queue-panel.ts b/frontend/src/components/queue-panel/queue-panel.ts index 990ce2e..6bef4d6 100644 --- a/frontend/src/components/queue-panel/queue-panel.ts +++ b/frontend/src/components/queue-panel/queue-panel.ts @@ -51,12 +51,8 @@ import { createTrackCardDragImage, removeDragImage, } from '@utils/drag-image'; -import { libraryStore } from '@store/library-store'; -import type * as library from '@go/library/models.js'; -import { loadTrackDetails } from '@utils/lazy-track-details.js'; -import { tracksByFilePath } from '@utils/track-index.js'; +import { showTrackDetailsForPath, showBatchTrackDetailsForPaths } from '@utils/track-details-opener.js'; import type { TrackDetails } from '@components/track-details/track-details.js'; -import type { CoverArtUrls } from '@components/track-details/track-details.js'; import { creditLink, trackLink, @@ -1542,89 +1538,33 @@ export class QueuePanel } private async openTrackDetails(index: number) { - const queueTrack = - this.queue.tracks[index]; + const queueTrack = this.queue.tracks[index]; if (!queueTrack) return; - const tracks = - libraryStore.getCachedTracks(); - const track = tracks - ? tracksByFilePath(tracks).get( - queueTrack.filePath, - ) - : undefined; - - if (!track) return; - - const ready = await loadTrackDetails( + await showTrackDetailsForPath( + () => this.trackDetailsDialog, + queueTrack.filePath, () => void this.openTrackDetails(index), ); - - if (!ready) return; - - const coverArt = track.CoverArtPath - ? { - coverArtPath: track.CoverArtPath, - coverArtSmall: track.CoverArtSmall, - coverArtMedium: track.CoverArtMedium, - coverArtLarge: track.CoverArtLarge, - } - : undefined; - - this.trackDetailsDialog?.show( - track, - coverArt, - ); } private async openBatchTrackDetails( indices: number[], ) { const queueTracks = this.queue.tracks; - const cachedTracks = - libraryStore.getCachedTracks(); + const filePaths: string[] = []; - if (!cachedTracks) return; + for (const i of indices) { + const path = queueTracks[i]?.filePath; - const byPath = tracksByFilePath(cachedTracks); - const tracks = indices - .map((i) => queueTracks[i]) - .filter((qt) => qt != null) - .map((qt) => byPath.get(qt.filePath)) - .filter( - (t): t is library.Track => - t != null, - ); - - if (tracks.length === 0) return; - - const ready = await loadTrackDetails( - () => void this.openBatchTrackDetails(indices), - ); - - if (!ready) return; - - const first = tracks[0]!; - const albumNames = new Set(tracks.map((t) => t.Album)); - let coverArt: CoverArtUrls | null = null; - let coverArtMixed = false; - - if (albumNames.size === 1 && first.CoverArtPath) { - coverArt = { - coverArtPath: first.CoverArtPath, - coverArtSmall: first.CoverArtSmall, - coverArtMedium: first.CoverArtMedium, - coverArtLarge: first.CoverArtLarge, - }; - } else if (albumNames.size > 1) { - coverArtMixed = true; + if (path != null) filePaths.push(path); } - this.trackDetailsDialog?.showBatch( - tracks, - coverArt, - coverArtMixed, + await showBatchTrackDetailsForPaths( + () => this.trackDetailsDialog, + filePaths, + () => void this.openBatchTrackDetails(indices), ); } diff --git a/frontend/src/components/sidebar/app-sidebar.ts b/frontend/src/components/sidebar/app-sidebar.ts index 55696a7..ca7a99b 100644 --- a/frontend/src/components/sidebar/app-sidebar.ts +++ b/frontend/src/components/sidebar/app-sidebar.ts @@ -6,6 +6,7 @@ import { designTokens } from '../../styles/tokens.css'; import type { DragActiveDetail } from '@utils/drag-controller'; import { ActiveViewController } from '@store/controllers/active-view-controller'; import { ViewVisibilityController } from '@store/controllers/view-visibility-controller'; +import { libraryStore } from '@store/library-store'; import { VIEW_META } from '../../services/view-meta'; import type { View } from '../../services/view-meta'; @@ -352,6 +353,10 @@ export class AppSidebar extends LitElement { : 'false'} @click=${() => this.navigate(item.id)} + @mouseenter=${() => + this.prefetch(item.id)} + @focus=${() => + this.prefetch(item.id)} @dragover=${(e: DragEvent) => this.onNavDragOver( e, @@ -503,6 +508,19 @@ export class AppSidebar extends LitElement { } } + /** + * Start loading what this view draws, on hover or keyboard focus. + * + * The ~100 ms before the click is the whole point: #280 stopped + * fetching every collection at startup, and this is what keeps the + * view that *is* opened from paying the whole payload after the + * click. Nothing is awaited and nothing is reported here — the view + * itself reports a failure, and this is the same request. + */ + private prefetch(view: View) { + libraryStore.prefetch(view); + } + private navigate(view: View) { // No optimistic highlight: the shell answers, and it answers // synchronously in `handleNavigate` before it awaits anything. diff --git a/frontend/src/components/smart-playlist-details/smart-playlist-details.ts b/frontend/src/components/smart-playlist-details/smart-playlist-details.ts index 5dd008b..049bc7c 100644 --- a/frontend/src/components/smart-playlist-details/smart-playlist-details.ts +++ b/frontend/src/components/smart-playlist-details/smart-playlist-details.ts @@ -52,11 +52,8 @@ import '@lit-labs/virtualizer'; import type { LitVirtualizer } from '@lit-labs/virtualizer'; import { flow } from '@lit-labs/virtualizer/layouts/flow.js'; import '@components/playlist-picker/playlist-picker.js'; -import { loadTrackDetails } from '@utils/lazy-track-details.js'; -import { tracksByFilePath, tracksForPaths } from '@utils/track-index.js'; +import { showTrackDetailsForPath, showBatchTrackDetailsForPaths } from '@utils/track-details-opener.js'; import type { TrackDetails } from '@components/track-details/track-details.js'; -import type { CoverArtUrls } from '@components/track-details/track-details.js'; -import { libraryStore } from '@store/library-store'; import { formatMilliseconds } from '@utils/time'; import { creditLink, @@ -1145,76 +1142,21 @@ export class SmartPlaylistDetails } private async openTrackDetails(filePath: string) { - const tracks = libraryStore.getCachedTracks(); - const track = tracks - ? tracksByFilePath(tracks).get(filePath) - : undefined; - - if (!track) return; - - const ready = await loadTrackDetails( + await showTrackDetailsForPath( + () => this.trackDetailsDialog, + filePath, () => void this.openTrackDetails(filePath), ); - - if (!ready) return; - - const coverArt = track.CoverArtPath - ? { - coverArtPath: track.CoverArtPath, - coverArtSmall: track.CoverArtSmall, - coverArtMedium: track.CoverArtMedium, - coverArtLarge: track.CoverArtLarge, - } - : undefined; - - this.trackDetailsDialog?.show( - track, - coverArt, - ); } private async openBatchTrackDetails( filePaths: string[], ) { - const cachedTracks = - libraryStore.getCachedTracks(); - - if (!cachedTracks) return; - - const tracks = tracksForPaths( - cachedTracks, + await showBatchTrackDetailsForPaths( + () => this.trackDetailsDialog, filePaths, - ); - - if (tracks.length === 0) return; - - const ready = await loadTrackDetails( () => void this.openBatchTrackDetails(filePaths), ); - - if (!ready) return; - - const first = tracks[0]!; - const albumNames = new Set(tracks.map((t) => t.Album)); - let coverArt: CoverArtUrls | null = null; - let coverArtMixed = false; - - if (albumNames.size === 1 && first.CoverArtPath) { - coverArt = { - coverArtPath: first.CoverArtPath, - coverArtSmall: first.CoverArtSmall, - coverArtMedium: first.CoverArtMedium, - coverArtLarge: first.CoverArtLarge, - }; - } else if (albumNames.size > 1) { - coverArtMixed = true; - } - - this.trackDetailsDialog?.showBatch( - tracks, - coverArt, - coverArtMixed, - ); } // ================================================================= diff --git a/frontend/src/components/smart-playlist-editor/smart-playlist-editor.ts b/frontend/src/components/smart-playlist-editor/smart-playlist-editor.ts index c74d668..c515810 100644 --- a/frontend/src/components/smart-playlist-editor/smart-playlist-editor.ts +++ b/frontend/src/components/smart-playlist-editor/smart-playlist-editor.ts @@ -1,8 +1,12 @@ import { LitElement, html, css, nothing } from 'lit'; import { customElement, property, state } from 'lit/decorators.js'; import * as library from '@go/library/models.js'; -import { PreviewSmartPlaylist } from '@go/playlist/service.js'; -import { libraryStore } from '@store/library-store'; +import { + PreviewSmartPlaylist, + SuggestSmartPlaylistValues, +} from '@go/playlist/service.js'; +import { list } from '@utils/binding'; +import { LRUMap } from '@utils/lru-map'; import { describeError } from '@utils/describe-error'; import { designTokens } from '../../styles/tokens.css'; import '@components/combobox/combobox.ts'; @@ -91,51 +95,23 @@ function formatOperatorLabel(op: string): string { return op.replace(/_/g, ' '); } -/** Returns autocomplete suggestions for a given field from libraryStore. */ -function getAutocompleteOptions(field: string): string[] { - switch (field) { - case 'artist': - return libraryStore.getCachedArtists()?.map((a) => a.Name) ?? []; - case 'genre': - return libraryStore.getCachedGenres()?.map((g) => g.Name) ?? []; - case 'album': - return libraryStore.getCachedAlbums()?.map((a) => a.Name) ?? []; - case 'title': { - const tracks = libraryStore.getCachedTracks(); - if (!tracks) return []; - return [...new Set(tracks.map((t) => t.TrackName).filter(Boolean))]; - } - case 'composer': { - const tracks = libraryStore.getCachedTracks(); - if (!tracks) return []; - return [...new Set(tracks.map((t) => t.Composer).filter(Boolean))]; - } - case 'file_type': { - const tracks = libraryStore.getCachedTracks(); - if (!tracks) return []; - return [...new Set(tracks.map((t) => t.FileType).filter(Boolean))]; - } - case 'year': - case 'release_year': { - // Both year fields draw suggestions from the set of years - // present in the library. The cached Track only carries the - // display (original) year, so it seeds both datalists — the - // list is just a hint, and the real filter runs server-side. - const tracks = libraryStore.getCachedTracks(); - if (!tracks) return []; - return [ - ...new Set( - tracks - .map((t) => t.Year) - .filter((y) => y > 0) - .map(String), - ), - ].sort(); - } - default: - return []; - } -} +/** + * Fields whose value box suggests values present in the library. Must + * match `suggestFields` in backend/smartplaylist; any other field gets + * a plain box. + * + * Suggestions are asked for as the user types (#279): they used to be + * built from libraryStore's whole-library arrays, which made this + * editor one more reason to load every track at startup, and gave it + * empty lists whenever those arrays had not landed. + */ +const SUGGEST_FIELDS = new Set([ + 'title', 'artist', 'album', 'genre', 'composer', 'file_type', + 'year', 'release_year', +]); + +/** How long typing must pause before suggestions are asked for. */ +const SUGGEST_DEBOUNCE_MS = 120; /** * Overrides for fields whose title-cased name would be ambiguous. The @@ -206,6 +182,17 @@ export class SmartPlaylistEditor extends LitElement { private previewTimer: ReturnType | null = null; + /** The value suggestions each rule row is showing, by row index. */ + @state() private suggestions = new Map(); + + private suggestTimer: ReturnType | null = null; + + /** Answers already fetched this session, by field and typed text. */ + private suggestCache = new LRUMap(100); + + /** The latest request per row, so a slow answer cannot overwrite a newer one. */ + private suggestWanted = new Map(); + // ── Styles ────────────────────────────────────────────────────── static override styles = [ @@ -599,6 +586,7 @@ export class SmartPlaylistEditor extends LitElement { // Reset value when field changes to avoid stale autocomplete data row.value = ''; row.value2 = ''; + this.dropSuggestions(); this.ruleRows = [...this.ruleRows]; this.onRulesChanged(); @@ -628,6 +616,56 @@ export class SmartPlaylistEditor extends LitElement { this.onRulesChanged(); } + /** Ask for the values of row `index`'s field that contain `text`. */ + private requestSuggestions(index: number, text: string) { + const field = this.ruleRows[index]?.field ?? ''; + + if (!SUGGEST_FIELDS.has(field)) return; + + const key = `${field}\u0000${text}`; + + this.suggestWanted.set(index, key); + + const cached = this.suggestCache.get(key); + + if (cached) { + this.setSuggestions(index, cached); + + return; + } + + if (this.suggestTimer !== null) clearTimeout(this.suggestTimer); + + this.suggestTimer = setTimeout(() => { + this.suggestTimer = null; + void list(SuggestSmartPlaylistValues(field, text)) + .then((values) => { + this.suggestCache.set(key, values); + + if (this.suggestWanted.get(index) === key) { + this.setSuggestions(index, values); + } + }) + .catch((err: unknown) => { + // A suggestion is a hint; the box still takes any text. + console.error('smart playlist: suggestions failed', err); + }); + }, SUGGEST_DEBOUNCE_MS); + } + + private setSuggestions(index: number, values: readonly string[]) { + const next = new Map(this.suggestions); + + next.set(index, values); + this.suggestions = next; + } + + /** Row indexes and fields changed: what was suggested no longer applies. */ + private dropSuggestions() { + this.suggestions = new Map(); + this.suggestWanted.clear(); + } + private updateValue2(index: number, newValue: string) { const row = this.ruleRows[index]; if (!row) return; @@ -644,6 +682,7 @@ export class SmartPlaylistEditor extends LitElement { private removeRule(index: number) { if (this.ruleRows.length <= 1) return; this.ruleRows = this.ruleRows.filter((_, i) => i !== index); + this.dropSuggestions(); this.onRulesChanged(); } @@ -892,7 +931,9 @@ export class SmartPlaylistEditor extends LitElement { ` : html` ) => + this.requestSuggestions(index, e.detail.text)} .value=${row.value} placeholder=${isAnyOf ? 'Comma-separated values' diff --git a/frontend/src/components/track-details/track-details.ts b/frontend/src/components/track-details/track-details.ts index f63d6b2..37695e4 100644 --- a/frontend/src/components/track-details/track-details.ts +++ b/frontend/src/components/track-details/track-details.ts @@ -22,7 +22,9 @@ import { import { GetTrackMBIDs } from '@go/library/library.js'; type TrackMBIDs = library.TrackMBIDs; import { ImageFilePicker, ReadFile } from '@go/frontendutil/frontendutil.js'; -import { libraryStore } from '../../store/library-store'; +import { trackCache } from '../../store/track-cache'; +import { batchCoverArt } from '@utils/track-details-opener.js'; +import type { ListTrack } from '@utils/track-table'; import { EventsOn } from '@runtime/runtime'; import { Events } from '../../events'; @@ -87,7 +89,11 @@ export class TrackDetails extends LitElement { // -- Batch-specific state -- @state() private batchMode = false; - @state() private batchTracks: library.Track[] = []; + // `ListTrack`, not `Track`: every field the merged-values view reads + // is on the list's rows, so a caller that is already showing them + // can open this dialog without fetching anything (#281). The + // openers that hold no rows ask `trackCache` instead. + @state() private batchTracks: ListTrack[] = []; @state() private batchFilePaths: string[] = []; @state() private batchCoverArtMixed = false; @state() private batchProgress: { @@ -135,12 +141,12 @@ export class TrackDetails extends LitElement { /** Open the dialog for batch editing multiple tracks. */ showBatch( - tracks: library.Track[], + tracks: readonly ListTrack[], coverArt: CoverArtUrls | null, coverArtMixed: boolean, ): void { this.batchMode = true; - this.batchTracks = tracks; + this.batchTracks = [...tracks]; this.batchFilePaths = tracks.map( (t) => t.FilePath, ); @@ -1317,10 +1323,13 @@ export class TrackDetails extends LitElement { const container = img.parentElement; if (container) { - container.innerHTML = - '
    ' + - '' + - '
    '; + const placeholder = document.createElement('div'); + const icon = document.createElement('wa-icon'); + + placeholder.className = 'cover-placeholder'; + icon.setAttribute('name', 'music'); + placeholder.append(icon); + container.replaceChildren(placeholder); } }; @@ -1690,20 +1699,11 @@ export class TrackDetails extends LitElement { // invalidation, which refreshes all other views. this.exitEditMode(); - // Re-fetch track and album data so the dialog - // shows updated values and cover art. The store - // invalidation is already in-flight from the event; - // these calls await the pending fetch or start one. - // Awaited for the side effect of refreshing the - // store; the album list is consumed elsewhere. - const [tracks] = await Promise.all([ - libraryStore.getTracks(), - libraryStore.getAlbums(), - ]); - - const updated = tracks.find( - (t) => t.FilePath === filePath, - ); + // Re-read this one track so the dialog shows the + // values and cover art just written. Not the whole + // library (#279): the views that hold it refetch on + // the event, and only if something is showing them. + const [updated] = await trackCache.refresh([filePath]); if (updated) { this.track = updated; @@ -1828,39 +1828,18 @@ export class TrackDetails extends LitElement { this.errorMessage = ''; this.cleanupPendingCoverArt(); - // Refresh data from library store; album list is - // refreshed for side effects only. - const [tracks] = await Promise.all([ - libraryStore.getTracks(), - libraryStore.getAlbums(), - ]); + // Re-read the tracks just written, in the order the batch + // holds them (#279) — not the whole library to filter. + // `refresh`, because the write's event may not have reached + // the cache before its own result did. + const refreshed = await trackCache.refresh(this.batchFilePaths); - // Re-resolve batch tracks. - const pathSet = new Set(this.batchFilePaths); - const refreshed = tracks.filter((t) => - pathSet.has(t.FilePath), - ); this.batchTracks = refreshed; - // Re-resolve cover art state. - const first = refreshed[0]; - const albumNames = new Set(refreshed.map((t) => t.Album)); + const { coverArt, mixed } = batchCoverArt(refreshed); - if (albumNames.size === 1 && first?.CoverArtPath) { - this.coverArt = { - coverArtPath: first.CoverArtPath, - coverArtSmall: first.CoverArtSmall, - coverArtMedium: first.CoverArtMedium, - coverArtLarge: first.CoverArtLarge, - }; - this.batchCoverArtMixed = false; - } else if (albumNames.size > 1) { - this.coverArt = null; - this.batchCoverArtMixed = true; - } else { - this.coverArt = null; - this.batchCoverArtMixed = false; - } + this.coverArt = coverArt; + this.batchCoverArtMixed = mixed; }; // -- Shared edit logic -- @@ -2103,6 +2082,8 @@ export class TrackDetails extends LitElement { // as a base64-encoded string (standard encoding/json // behaviour for []byte). const result = await ReadFile(filePath); + // SAFETY: the binding is typed as the Go []byte, but the + // wire value is the base64 string encoding/json made of it. const b64 = result as unknown as string; const binary = atob(b64); const bytes = new Uint8Array(binary.length); @@ -2157,7 +2138,7 @@ export class TrackDetails extends LitElement { key: string; label: string; type: 'text' | 'number'; - extract: (t: library.Track) => string; + extract: (t: ListTrack) => string; }> = [ { key: 'title', @@ -2241,7 +2222,7 @@ export class TrackDetails extends LitElement { private countDistinctValues(key: string): number { const extractMap: Record< string, - (t: library.Track) => string + (t: ListTrack) => string > = { title: (t) => t.TrackName ?? '', artist: (t) => t.ArtistName ?? '', @@ -2265,7 +2246,7 @@ export class TrackDetails extends LitElement { if (!extract) return 0; const unique = new Set( - this.batchTracks.map(extract), + this.batchTracks.map((t) => extract(t)), ); return unique.size; diff --git a/frontend/src/components/track-list/columns.ts b/frontend/src/components/track-list/columns.ts index a0f8a38..dbcf19c 100644 --- a/frontend/src/components/track-list/columns.ts +++ b/frontend/src/components/track-list/columns.ts @@ -1,4 +1,4 @@ -import type * as library from '@go/library/models.js'; +import type { ListTrack } from '@utils/track-table'; import { formatSampleRate, formatBitDepth, @@ -45,7 +45,7 @@ export interface ColumnDef { */ configurable?: boolean; /** Extracts the display value from a track. */ - accessor: (track: library.Track) => string; + accessor: (track: ListTrack) => string; /** Default CSS width (used when no saved width exists). */ defaultWidth: string; /** Text alignment. Defaults to left. */ @@ -60,15 +60,15 @@ export interface ColumnDef { * needs it, which is why it is optional rather than a second * required parameter on all of them. */ - renderCell?: (track: library.Track, term?: string) => unknown; + renderCell?: (track: ListTrack, term?: string) => unknown; /** * Comparison function for sorting two tracks by this column. * Returns negative if a < b, positive if a > b, zero if equal. * If omitted the column is not sortable. */ comparator?: ( - a: library.Track, - b: library.Track, + a: ListTrack, + b: ListTrack, ) => number; } @@ -79,7 +79,7 @@ export const COLUMN_DEFS: Record = { label: 'Art', accessor: () => '', defaultWidth: '36px', - renderCell: (track: library.Track) => { + renderCell: (track: ListTrack) => { // `perf.M3`. This rendered `CoverArtPath` — the *original* // embedded artwork, commonly 1500×1500 and several hundred // kB — scaled by CSS into a 24 px box, while the 100 px @@ -90,10 +90,10 @@ export const COLUMN_DEFS: Record = { // `cover-grid.getCoverUrl()` has picked the right tier all // along; this is the same rule for a much smaller box, with // the two attributes that keep the decode off the scroll - // path. - const src = track.CoverArtSmall - || track.CoverArtMedium - || track.CoverArtPath; + // path. The list carries only this tier (#281): every tier + // is derived from the same file, so it is set whenever any + // of them would be. + const src = track.CoverArtSmall; if (!src) return nothing; diff --git a/frontend/src/components/track-list/search-ranking.ts b/frontend/src/components/track-list/search-ranking.ts index e295e33..20ba164 100644 --- a/frontend/src/components/track-list/search-ranking.ts +++ b/frontend/src/components/track-list/search-ranking.ts @@ -1,4 +1,4 @@ -import type * as library from '@go/library/models.js'; +import type { ListTrack } from '@utils/track-table'; import { html } from 'lit'; import type { TemplateResult } from 'lit'; @@ -99,7 +99,7 @@ function matchQuality( * fields are always included on top of these. */ function scoreTrack( - track: library.Track, + track: ListTrack, termLower: string, columns: ColumnDef[], ): number { @@ -147,7 +147,7 @@ function scoreTrack( /** A track paired with its relevance score. */ export interface RankedTrack { - track: library.Track; + track: ListTrack; score: number; } @@ -166,10 +166,10 @@ export interface RankedTrack { * `scores` (Map of FilePath → relevance score). */ export function rankTracks( - tracks: library.Track[], + tracks: ListTrack[], term: string, activeColumns: ColumnDef[], -): { tracks: library.Track[]; scores: Map } { +): { tracks: ListTrack[]; scores: Map } { const termLower = term.toLowerCase(); const ranked: RankedTrack[] = []; @@ -188,7 +188,7 @@ export function rankTracks( // Sort descending by score (highest relevance first). ranked.sort((a, b) => b.score - a.score); - const result: library.Track[] = []; + const result: ListTrack[] = []; const scores = new Map(); for (const r of ranked) { diff --git a/frontend/src/components/track-list/track-list.ts b/frontend/src/components/track-list/track-list.ts index 11c12f0..cbae111 100644 --- a/frontend/src/components/track-list/track-list.ts +++ b/frontend/src/components/track-list/track-list.ts @@ -1,5 +1,5 @@ -import * as library from '@go/library/models.js'; -import { LitElement, html, svg, css, nothing } from 'lit'; +import type { ListTrack } from '@utils/track-table'; +import { LitElement, html, svg, css, nothing, type TemplateResult } from 'lit'; import { designTokens } from '../../styles/tokens.css'; import { srOnly } from '../../styles/sr-only.css'; import { @@ -80,11 +80,10 @@ import { describeError } from '@utils/describe-error'; import { notificationStore } from '@store/notification-store'; import { confirmAction } from '@components/confirm-dialog/confirm-dialog'; import { RemoveFromLibrary } from '@go/library/library.js'; -import { loadTrackDetails } from '@utils/lazy-track-details.js'; +import { showTrackDetailsForPath, showBatchTrackDetails } from '@utils/track-details-opener.js'; import { tracksByFilePath, tracksForPaths } from '@utils/track-index.js'; import '@components/playlist-picker/playlist-picker.js'; import type { TrackDetails } from '@components/track-details/track-details.js'; -import type { CoverArtUrls } from '@components/track-details/track-details.js'; import { ICON_PLAY, ICON_PLAYLIST, @@ -155,7 +154,7 @@ export class TrackList * parent is responsible for reloading when data changes. */ @property({ type: Array, attribute: false }) - externalTracks?: library.Track[]; + externalTracks?: ListTrack[]; /** * What a host embedding this list (e.g. `genre-details`) should say @@ -199,7 +198,7 @@ export class TrackList private lastSearchTerm = ''; /** Tracks the store's cached array reference to detect refreshes. */ - private lastTracksRef: library.Track[] | null = + private lastTracksRef: ListTrack[] | null = null; /** @@ -242,7 +241,7 @@ export class TrackList } @state() - private tracks: library.Track[] = []; + private tracks: ListTrack[] = []; @query('#context-menu') private contextMenuPopup!: MenuSurface; @@ -269,16 +268,16 @@ export class TrackList private lastActiveTrackPath: string | null = null; // -- Memoisation caches for filtered / sorted tracks -- - private cachedFilteredTracks: library.Track[] = []; - private cachedSortedTracks: library.Track[] = []; + private cachedFilteredTracks: ListTrack[] = []; + private cachedSortedTracks: ListTrack[] = []; private cachedRelevanceScores = new Map< string, number >(); - private prevFilterTracks: library.Track[] = []; + private prevFilterTracks: ListTrack[] = []; private prevFilterTerm = ''; private prevFilterColIds = ''; - private prevSortFiltered: library.Track[] = []; + private prevSortFiltered: ListTrack[] = []; private prevSortField: string | null = null; private prevSortDir: SortDirection = 'asc'; @@ -523,7 +522,7 @@ export class TrackList } } - private computeFilteredTracks(): library.Track[] { + private computeFilteredTracks(): ListTrack[] { const term = this.searchCtrl.term; if (!term) { @@ -543,7 +542,7 @@ export class TrackList return result.tracks; } - private computeSortedTracks(): library.Track[] { + private computeSortedTracks(): ListTrack[] { const tracks = this.cachedFilteredTracks; const hasSearch = this.cachedRelevanceScores.size > 0; @@ -1337,11 +1336,6 @@ export class TrackList super.connectedCallback(); this.restoreSortPreferences(); - if (this.externalTracks) { - this.tracks = this.externalTracks; - } else { - this.loadTracks(); - } this.resizeObserver = new ResizeObserver( () => { this.onHostResize(); @@ -1391,10 +1385,33 @@ export class TrackList this.resizeObserver = null; } + /** + * Fetch the list when this becomes the view on screen (#280). + * + * Not on connection: `index.html` renders a `` as the + * main panel's first-paint content, so a connection-time fetch was + * the whole library loaded at launch for a landing on Home — 12 MB + * at 50 000 tracks, and the backend's peak RSS with it. The shell + * activates this element only when a navigation lands on Tracks. + * + * Called on *every* activation, not just the first: the store may + * have refetched while this view was off screen, and `getTracks()` + * answers from its cache when nothing changed. + */ + protected override onViewActivate(): void { + if (this.externalTracks) { + this.tracks = this.externalTracks; + } else { + void this.loadTracks(); + } + + this.attachListListeners(); + } + /** Document-level listeners belong to the *visible* list. A cached * list is never disconnected, so this is the only place they can be * taken down again. */ - protected override onViewActivate(): void { + private attachListListeners(): void { this.listenWhileActive(document, 'click', this.clearSelectionHandler); this.listenWhileActive( document, @@ -1561,9 +1578,10 @@ export class TrackList this.selection.clear(); } - // Re-fetch when the store delivers fresh - // data after eager refetch on invalidation. - if (!this.externalTracks) { + // Re-fetch when the store delivers fresh data after an + // invalidation. Off screen the list does nothing with it, and + // the next activation re-reads the store (#280). + if (!this.externalTracks && this.viewActive) { const cached = this.libraryCtrl.cachedTracks; @@ -1734,7 +1752,7 @@ export class TrackList */ private resolveTrackFromEvent( e: Event, - ): { track: library.Track; index: number } | null { + ): { track: ListTrack; index: number } | null { const row = (e.target as HTMLElement).closest( '.track-row', ) as HTMLElement | null; @@ -1875,7 +1893,7 @@ export class TrackList private onTrackRowClick( e: MouseEvent, - track: library.Track, + track: ListTrack, index: number, ) { // Clicking is also how the keyboard's starting point is chosen: @@ -1884,7 +1902,7 @@ export class TrackList this.selection.handleItemClick(e, track.FilePath, index); } - private onTrackRowDblClick(_track: library.Track, index: number) { + private onTrackRowDblClick(_track: ListTrack, index: number) { this.selection.clear(); this.playFromRow(index); } @@ -1925,7 +1943,7 @@ export class TrackList ); } - private onTrackContextMenu(e: MouseEvent, track: library.Track) { + private onTrackContextMenu(e: MouseEvent, track: ListTrack) { e.preventDefault(); e.stopPropagation(); @@ -1939,7 +1957,7 @@ export class TrackList private onTrackDragStart = ( e: DragEvent, - track: library.Track, + track: ListTrack, ) => { // Gather file paths: all selected if this track is selected, // otherwise just the dragged track. @@ -2129,74 +2147,26 @@ export class TrackList this.ctxMenu.close(); } + // The list's rows do not carry what the dialog shows (#281), so + // the dialog gets whole tracks by path, like every other opener. private async openTrackDetails(filePath: string) { - const track = tracksByFilePath(this.tracks).get( + await showTrackDetailsForPath( + () => this.trackDetailsDialog, filePath, - ); - - if (!track) return; - - const ready = await loadTrackDetails( () => void this.openTrackDetails(filePath), ); - - if (!ready) return; - - const coverArt = track.CoverArtPath - ? { - coverArtPath: track.CoverArtPath, - coverArtSmall: track.CoverArtSmall, - coverArtMedium: track.CoverArtMedium, - coverArtLarge: track.CoverArtLarge, - } - : undefined; - - this.trackDetailsDialog?.show( - track, - coverArt, - ); } private async openBatchTrackDetails( filePaths: string[], ) { - const tracks = tracksForPaths( - this.tracks, - filePaths, - ); - - if (tracks.length === 0) return; - - const ready = await loadTrackDetails( + // The rows in hand are what the dialog reads, so a "select all" + // on 50 000 tracks opens it without asking the backend for them. + await showBatchTrackDetails( + () => this.trackDetailsDialog, + tracksForPaths(this.tracks, filePaths), () => void this.openBatchTrackDetails(filePaths), ); - - if (!ready) return; - - // Use cover art from the first track. If all tracks share - // the same album, they share the same art. - const first = tracks[0]!; - let coverArt: CoverArtUrls | null = null; - let coverArtMixed = false; - - const albumNames = new Set(tracks.map((t) => t.Album)); - - if (albumNames.size === 1 && first.CoverArtPath) { - coverArt = { - coverArtPath: first.CoverArtPath, - coverArtSmall: first.CoverArtSmall, - coverArtMedium: first.CoverArtMedium, - coverArtLarge: first.CoverArtLarge, - }; - } else if (albumNames.size > 1) { - coverArtMixed = true; - } - - this.trackDetailsDialog?.showBatch( - tracks, - coverArt, - coverArtMixed, - ); } // ================================================================= @@ -2274,7 +2244,7 @@ export class TrackList this.saveSortPreferences(); } - private isActiveTrack(track: library.Track): boolean { + private isActiveTrack(track: ListTrack): boolean { const currentTrack = this.player.currentTrack; if (!currentTrack) return false; @@ -2283,9 +2253,9 @@ export class TrackList } private renderTrackRow = ( - track: library.Track, + track: ListTrack, index: number, - ): unknown => { + ): TemplateResult => { const active = this.isActiveTrack(track); const selected = this.selection.isSelected( track.FilePath, @@ -2568,7 +2538,7 @@ export class TrackList scroller .items=${visibleTracks} .renderItem=${this.renderTrackRow} - .keyFunction=${(track: library.Track) => track.FilePath} + .keyFunction=${(track: ListTrack) => track.FilePath} .layout=${this.rowLayout} > `} diff --git a/frontend/src/store/controllers/library-controller.ts b/frontend/src/store/controllers/library-controller.ts index dc00c3d..9458fd6 100644 --- a/frontend/src/store/controllers/library-controller.ts +++ b/frontend/src/store/controllers/library-controller.ts @@ -1,6 +1,7 @@ import type { ReactiveController, ReactiveControllerHost } from 'lit'; import type * as library from '@go/library/models.js'; import { libraryStore } from '../library-store'; +import type { ListTrack } from '@utils/track-table'; type ViewName = 'tracks' | 'albums' | 'artists' | 'genres'; @@ -55,7 +56,7 @@ export class LibraryController implements ReactiveController { // DATA ACCESS // =================================================================== - async getTracks(): Promise { + async getTracks(): Promise { return libraryStore.getTracks(); } @@ -85,7 +86,7 @@ export class LibraryController implements ReactiveController { ); } - get cachedTracks(): library.Track[] | null { + get cachedTracks(): ListTrack[] | null { return libraryStore.getCachedTracks(); } diff --git a/frontend/src/store/library-store.ts b/frontend/src/store/library-store.ts index 72ef0d0..ddb8a4f 100644 --- a/frontend/src/store/library-store.ts +++ b/frontend/src/store/library-store.ts @@ -1,6 +1,6 @@ import { EventsOn } from '@runtime/runtime'; import { - GetTracks, + GetTrackTable, GetAlbums, GetArtists, GetGenres, @@ -9,6 +9,8 @@ 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'; type ViewName = 'tracks' | 'albums' | 'artists' | 'genres'; @@ -27,8 +29,16 @@ const COVER_SIZE_DEFAULT = 176; /** localStorage key for persisted cover size. */ const COVER_SIZE_KEY = 'cover-grid-size'; +/** + * How long the small-collection warm-up will wait for idle before + * running anyway. It is speculative, but a busy main thread must not + * mean Albums is slow to open — the timeout is the promise that it is + * only ever deferred, never skipped. + */ +const WARM_IDLE_TIMEOUT_MS = 3_000; + class LibraryStore { - private tracks: library.Track[] | null = null; + private tracks: ListTrack[] | null = null; private albums: library.Album[] | null = null; private artists: library.Artist[] | null = null; private genres: library.GenreWithCount[] | null = null; @@ -81,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; @@ -98,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); @@ -109,34 +119,82 @@ class LibraryStore { }); this.loadCoverSize(); - this.deferEagerFetch(); + this.warmSmallCollectionsOnIdle(); } /** - * Schedules eagerFetch() to run after the DOM is ready. - * The LibraryStore singleton is instantiated during ES module - * evaluation (import time), so calling eagerFetch() in the - * constructor would fire 4 backend roundtrips before the app - * shell has rendered. Deferring to the 'DOMContentLoaded' - * event (or calling immediately if the DOM is already parsed) - * lets the shell paint first, then begins data loading. + * Warm the three small collections once the app is idle after first + * paint, and deliberately not the tracks (#280). + * + * All four used to be fetched together at `DOMContentLoaded`, + * whichever view was showing. On a 26 138-track library the track + * list was 20.5 MB of that, and encoding it cost the backend ~170 MB + * of transient allocation — paid by someone looking at Home, which + * needs none of it. Measured on 50 000 tracks: 543 MB of backend RSS + * at rest before, 296 MB after, and 12.1 MB of binding bytes instead + * of 35.9. + * + * Albums, artists and genres are 1.6 MB together, so they are still + * fetched ahead of the click — that is what made those views + * instant. Tracks are 12 MB at 50 000 and are fetched by the view + * that draws them, or by `prefetch` from a hover. + * + * Idle rather than immediate: this is speculative, so it must not + * compete with the first paint. */ - private deferEagerFetch(): void { - if (document.readyState === 'loading') { - window.addEventListener( - 'DOMContentLoaded', - () => { - this.eagerFetch(); - }, - { once: true }, - ); + private warmSmallCollectionsOnIdle(): void { + const warm = () => { + const logged = this.failureReporter(); + + void this.getAlbums().catch(logged('albums')); + void this.getArtists().catch(logged('artists')); + void this.getGenres().catch(logged('genres')); + }; + + if (typeof requestIdleCallback === 'function') { + requestIdleCallback(warm, { timeout: WARM_IDLE_TIMEOUT_MS }); } else { - // DOM already parsed (shouldn't happen during module - // eval, but handles dynamic instantiation safely). - this.eagerFetch(); + setTimeout(warm, 0); } } + /** + * Start loading what a view will need, without waiting for it. + * + * A nav item calls this on hover or focus: that is the ~100 ms + * before the click, and it is what #280 trades for not paying for + * every collection at startup whether or not anyone goes there. + * + * A view this store holds nothing for is ignored rather than an + * error — it is a hint, and a hint about Home is not a mistake. + * The failure is swallowed here because the view that wanted the + * data reports it: this is the same request, already deduplicated + * by `inFlight`, and it has no caller to reject to. + */ + prefetch(view: string): void { + const started = (() => { + switch (view) { + case 'tracks': + return this.getTracks(); + case 'albums': + return this.getAlbums(); + case 'artists': + return this.getArtists(); + case 'genres': + return this.getGenres(); + default: + return null; + } + })(); + + void started?.catch(() => undefined); + } + + private failureReporter(): (what: string) => (err: unknown) => void { + return (what) => (err) => + console.error(`library: could not load ${what}`, err); + } + // =================================================================== // DATA ACCESS // Returns cached data or fetches from backend on first access. @@ -214,18 +272,18 @@ class LibraryStore { } } - async getTracks(): Promise { + async getTracks(): Promise { if (this.tracks !== null) { return this.tracks; } - const pending = this.pending('tracks'); + const pending = this.pending('tracks'); if (pending) return pending; return this.track( 'tracks', - list(GetTracks(this.libraryFilter())), + GetTrackTable(this.libraryFilter()).then(decodeTrackTable), (tracks) => { this.tracks = tracks; }, @@ -341,7 +399,7 @@ class LibraryStore { // Synchronous access for controllers that need current cached values. // =================================================================== - getCachedTracks(): library.Track[] | null { + getCachedTracks(): ListTrack[] | null { return this.tracks; } @@ -497,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. * @@ -520,10 +688,11 @@ class LibraryStore { private applyPlayCount(payload: unknown): void { if (this.tracks === null) return; + // The list does not carry LastPlayed (#281); trackCache patches + // it on the whole tracks the details dialog reads. const p = payload as { filePath?: string; playCount?: number; - lastPlayed?: string; } | null; if (!p?.filePath) return; @@ -536,14 +705,10 @@ class LibraryStore { if (existing === undefined) return; - const patched = Object.assign( - Object.create(Object.getPrototypeOf(existing) as object), - existing, - { - PlayCount: p.playCount ?? existing.PlayCount, - LastPlayed: p.lastPlayed ?? existing.LastPlayed, - }, - ) as library.Track; + const patched: ListTrack = { + ...existing, + PlayCount: p.playCount ?? existing.PlayCount, + }; this.tracks = [ ...this.tracks.slice(0, idx), @@ -597,6 +762,8 @@ class LibraryStore { } } + const loaded = this.loadedCollections(); + this.albums = null; this.artists = null; this.genres = null; @@ -612,50 +779,63 @@ class LibraryStore { this.changeGen++; this.notify(); - const logged = (what: string) => (err: unknown) => - console.error(`library: could not reload ${what}`, err); - - void this.getAlbums().catch(logged('albums')); - void this.getArtists().catch(logged('artists')); - void this.getGenres().catch(logged('genres')); + // Only the summaries something was showing (#280): a removal + // nobody was looking at does not load a collection to correct it. + this.refetchLoaded({ + albums: loaded.albums, + artists: loaded.artists, + genres: loaded.genres, + }); } private invalidate(): void { + // Which collections something has actually loaded, before they + // are dropped. Refetching all four here would undo #280: a scan + // finishing would fetch the track list of a library nobody has + // opened the Tracks view on. + const loaded = this.loadedCollections(); + this.tracks = null; this.albums = null; this.artists = null; this.genres = null; // Anything still in flight was asked for on behalf of a - // selection that no longer applies: forget it, so the eager - // refetch below starts a request for the current one rather - // than adopting the old one's answer. + // selection that no longer applies: forget it, so the refetch + // below starts a request for the current one rather than + // adopting the old one's answer. this.inFlight.clear(); this.cacheGen++; this.changeGen++; this.scrollPositions = { tracks: 0, albums: 0, artists: 0, genres: 0 }; this.notify(); - this.eagerFetch(); + this.refetchLoaded(loaded); + } + + /** The collections currently held, keyed by the view that draws them. */ + private loadedCollections(): Partial> { + return { + tracks: this.tracks !== null, + albums: this.albums !== null, + artists: this.artists !== null, + genres: this.genres !== null, + }; } /** - * Fetches all library data. Called after DOM ready - * (initial load, via deferEagerFetch) and after cache - * invalidation so that controller subscribers receive - * fresh data on the next requestUpdate() cycle without - * needing their own LibraryScanComplete listener. + * Refetch exactly the collections named, and nothing else. + * + * A failed fetch is reported by whichever view asked for the data + * (it is that panel's failure, not the app's), but this refetch has + * no caller to reject to — without a catch it is an unhandled + * rejection. */ - private eagerFetch(): void { - // A failed fetch is reported by whichever view asked for the - // data (it is that panel's failure, not the app's), but the - // eager refetch has no caller to reject to — without a catch it - // is an unhandled rejection. - const logged = (what: string) => (err: unknown) => - console.error(`library: could not load ${what}`, err); + private refetchLoaded(loaded: Partial>): void { + const logged = this.failureReporter(); - void this.getTracks().catch(logged('tracks')); - void this.getAlbums().catch(logged('albums')); - void this.getArtists().catch(logged('artists')); - void this.getGenres().catch(logged('genres')); + if (loaded.tracks) void this.getTracks().catch(logged('tracks')); + if (loaded.albums) void this.getAlbums().catch(logged('albums')); + if (loaded.artists) void this.getArtists().catch(logged('artists')); + if (loaded.genres) void this.getGenres().catch(logged('genres')); } // =================================================================== diff --git a/frontend/src/store/track-cache.ts b/frontend/src/store/track-cache.ts new file mode 100644 index 0000000..6e496ce --- /dev/null +++ b/frontend/src/store/track-cache.ts @@ -0,0 +1,240 @@ +/** + * Whole tracks, looked up by file path, for the rows a surface is + * actually showing (#279). + * + * Track details from the queue, a playlist or a smart playlist used to + * find their track in `libraryStore`'s whole-library array. That made + * the array a dependency of every surface that can open details, so it + * was fetched eagerly at startup — 20.5 MB of JSON at 26 138 tracks — + * and when it had *not* landed yet, the openers that read it + * synchronously did nothing at all. This asks the backend for the + * paths in hand instead. + * + * Three things are load-bearing, the same three as `credit-store`: + * + * **Lookups are coalesced.** Every `get()` made in the same task joins + * one `GetTracksByPaths` call. A timer rather than a frame: these are + * user actions, not row renders, and a frame never fires in a hidden + * window, which would leave the caller waiting on nothing. + * + * **It is bounded.** A batch details dialog over "Select all" asks for + * every track in the library; it gets every one of them back, but the + * cache keeps only the most recent `TRACK_CACHE_LIMIT`. + * + * **It forgets what changed.** The events that make `library-store` + * refetch drop the affected entries here, and a play count is patched + * in place, so a cached track is never older than the last event about + * it. An answer that was in flight across an invalidation is still + * returned to its caller (it was correct when asked) but not cached. + */ + +import { EventsOn } from '@runtime/runtime'; +import { GetTracksByPaths } from '@go/library/library.js'; +import type * as library from '@go/library/models.js'; +import { list } from '@utils/binding'; +import { LRUMap } from '@utils/lru-map'; +import { registerCacheProbe } from '@utils/cache-stats'; +import { Events } from '../events'; + +/** + * Tracks retained. A track is ~25 short fields, so this is a couple of + * megabytes at most — sized above any batch a person edits by hand, + * far below a library. + */ +export const TRACK_CACHE_LIMIT = 2_000; + +interface Waiter { + paths: readonly string[]; + resolve: (tracks: library.Track[]) => void; + reject: (err: unknown) => void; +} + +class TrackCache { + private cache = new LRUMap(TRACK_CACHE_LIMIT); + + /** Requests collected in this task, answered by one binding call. */ + private waiting: Waiter[] = []; + + private flushHandle: ReturnType | null = null; + + /** Bumped by every invalidation; an older answer is not cached. */ + private gen = 0; + + constructor() { + EventsOn(Events.LibraryScanComplete, () => this.clear()); + EventsOn(Events.LibraryRemoved, () => this.clear()); + EventsOn(Events.TrackMetadataChanged, (payload: unknown) => { + const p = payload as { filePath?: string } | null; + + // A batch write names no paths: forget everything. + if (p?.filePath) this.forget([p.filePath]); + else this.clear(); + }); + EventsOn(Events.TracksRemovedFromLibrary, (payload: unknown) => { + const p = payload as { filePaths?: string[] } | null; + + this.forget(p?.filePaths ?? []); + }); + EventsOn(Events.TrackPlayCountChanged, (payload: unknown) => { + this.applyPlayCount(payload); + }); + + registerCacheProbe('tracks-by-path', () => ({ + entries: this.cache.size, + chars: this.retainedChars(), + limit: TRACK_CACHE_LIMIT, + })); + } + + /** + * The tracks at `paths`, in the order given, without the ones that + * are not in the library. Cached tracks are answered without a call. + */ + get(paths: readonly string[]): Promise { + const answered = this.fromCache(paths); + + if (answered) return Promise.resolve(answered); + + return new Promise((resolve, reject) => { + this.waiting.push({ paths, resolve, reject }); + this.flushHandle ??= setTimeout(() => void this.flush(), 0); + }); + } + + /** One track, or undefined when the path is not in the library. */ + async getOne(path: string): Promise { + return (await this.get([path]))[0]; + } + + /** + * Ask the backend again for `paths`, ignoring the cache. + * + * For a caller that has just written to these files and must not be + * answered from before the write, whether or not the event that + * invalidates them has arrived yet. + */ + refresh(paths: readonly string[]): Promise { + this.forget(paths); + + return this.get(paths); + } + + /** Every path cached, or null if any is missing. */ + private fromCache(paths: readonly string[]): library.Track[] | null { + const tracks: library.Track[] = []; + + for (const path of paths) { + const track = this.cache.get(path); + + if (!track) return null; + + tracks.push(track); + } + + return tracks; + } + + private async flush(): Promise { + this.flushHandle = null; + + const waiting = this.waiting; + + this.waiting = []; + + const missing = new Set(); + + for (const w of waiting) { + for (const path of w.paths) { + if (!this.cache.has(path)) missing.add(path); + } + } + + const gen = this.gen; + let fetched: library.Track[]; + + try { + fetched = missing.size > 0 + ? await list(GetTracksByPaths([...missing])) + : []; + } catch (err) { + for (const w of waiting) w.reject(err); + + return; + } + + // Answer from what this call returned plus what was cached when + // it was made — not from the cache afterwards, which an LRU + // eviction or an invalidation may have emptied in between. + const answer = new Map(); + + for (const w of waiting) { + for (const path of w.paths) { + const hit = this.cache.get(path); + + if (hit) answer.set(path, hit); + } + } + + for (const track of fetched) { + answer.set(track.FilePath, track); + + if (gen === this.gen) this.cache.set(track.FilePath, track); + } + + for (const w of waiting) { + const tracks: library.Track[] = []; + + for (const path of w.paths) { + const track = answer.get(path); + + if (track) tracks.push(track); + } + + w.resolve(tracks); + } + } + + private forget(paths: readonly string[]): void { + for (const path of paths) this.cache.delete(path); + + this.gen++; + } + + private clear(): void { + this.cache.clear(); + this.gen++; + } + + private applyPlayCount(payload: unknown): void { + const p = payload as { + filePath?: string; + playCount?: number; + lastPlayed?: string; + } | null; + + if (!p?.filePath) return; + + const existing = this.cache.get(p.filePath); + + if (!existing) return; + + this.cache.set(p.filePath, { + ...existing, + PlayCount: p.playCount ?? existing.PlayCount, + LastPlayed: p.lastPlayed ?? existing.LastPlayed, + }); + } + + private retainedChars(): number { + let total = 0; + + for (const t of this.cache.values()) { + total += t.FilePath.length + t.TrackName.length + + t.ArtistName.length + t.Album.length; + } + + return total; + } +} + +export const trackCache = new TrackCache(); diff --git a/frontend/src/utils/binding.ts b/frontend/src/utils/binding.ts index 1988e8d..55c8bc1 100644 --- a/frontend/src/utils/binding.ts +++ b/frontend/src/utils/binding.ts @@ -79,6 +79,14 @@ export function compact( return out; } +/** + * listField is list for a slice that arrived as a *field* of a struct + * rather than as a return value — a column of `TrackTable`, say. + */ +export function listField(field: T[] | null | undefined): T[] { + return field ?? []; +} + /** * value awaits a binding whose result is used as-is, dropping only the * cancellation the app never asks for. diff --git a/frontend/src/utils/track-details-opener.ts b/frontend/src/utils/track-details-opener.ts index bb21e5a..5e1aad0 100644 --- a/frontend/src/utils/track-details-opener.ts +++ b/frontend/src/utils/track-details-opener.ts @@ -1,40 +1,70 @@ /** * Open `` for a file path. * - * The five library-side hosts already hold the `library.Track` the - * dialog wants — they render it. Explore's rows do not: a tracklist row - * is an `MBTrack`/`LBTopRecording` from the catalog, and all it can say + * Explore's rows carry no library metadata at all: a tracklist row is + * an `MBTrack`/`LBTopRecording` from the catalog, and all it can say * about the library is *which file is behind it*. So the path is the * one key both sides share, and turning it back into a track is the - * work this does. + * work this does. The queue, playlists and smart playlists are the same + * shape — they render their own row type. * - * `libraryStore.getTracks()` is awaited rather than - * `getCachedTracks()`-and-bail (which is what `queue-panel` does): - * Explore is reachable without ever opening the library views, so a - * cold cache is ordinary here rather than a symptom, and silently doing - * nothing on a menu item the user just clicked is not an option. The - * fetch is the store's own, shared with every other reader. + * All of them used to find the track in `libraryStore`'s whole-library + * array, which meant that array had to be loaded for details to open — + * and the ones that read it synchronously silently did nothing when it + * was not (#279). They ask `trackCache` for the paths in hand instead: + * one small call, coalesced, answered from cache the second time. + * + * A caller that *is* showing the rows passes them + * (`showBatchTrackDetails`), because fetching back what is already in + * hand is how a "select all" over 50 000 tracks came to cost 3 s. */ -import type * as library from '@go/library/models.js'; import type { CoverArtUrls, TrackDetails, } from '@components/track-details/track-details.js'; -import { libraryStore } from '@store/library-store.js'; +import { trackCache } from '@store/track-cache.js'; import { loadTrackDetails } from '@utils/lazy-track-details.js'; -import { tracksByFilePath } from '@utils/track-index.js'; +import type * as library from '@go/library/models.js'; +import type { ListTrack } from '@utils/track-table'; -/** The cover art the dialog shows, or nothing when the track has none. */ -function coverArtOf(track: library.Track): CoverArtUrls | undefined { - return track.CoverArtPath - ? { - coverArtPath: track.CoverArtPath, - coverArtSmall: track.CoverArtSmall, - coverArtMedium: track.CoverArtMedium, - coverArtLarge: track.CoverArtLarge, - } - : undefined; +/** + * The cover art the dialog shows, or nothing when the track has none. + * + * A whole track carries every tier; the list's rows carry only the + * small one (#281), and every tier is derived from the same file — so a + * missing one falls back to the tier that is there rather than the + * dialog showing nothing. + */ +function coverArtOf(track: ListTrack): CoverArtUrls | undefined { + const small = track.CoverArtSmall; + + if (!small) return undefined; + + const tiers = track as Partial; + + return { + coverArtPath: tiers.CoverArtPath ?? small, + coverArtSmall: small, + coverArtMedium: tiers.CoverArtMedium ?? small, + coverArtLarge: tiers.CoverArtLarge ?? small, + }; +} + +/** + * The cover a batch dialog shows: the album's, when every track is on + * one album; none and `mixed` when they span several. + */ +export function batchCoverArt( + tracks: readonly ListTrack[], +): { coverArt: CoverArtUrls | null; mixed: boolean } { + const albums = new Set(tracks.map((t) => t.Album)); + + if (albums.size > 1) return { coverArt: null, mixed: true }; + + const first = tracks[0]; + + return { coverArt: first ? coverArtOf(first) ?? null : null, mixed: false }; } /** @@ -62,8 +92,7 @@ export async function showTrackDetailsForPath( filePath: string, retry: () => void, ): Promise { - const tracks = await libraryStore.getTracks(); - const track = tracksByFilePath(tracks).get(filePath); + const track = await trackCache.getOne(filePath); if (!track) return 'not-in-library'; @@ -75,3 +104,43 @@ export async function showTrackDetailsForPath( return 'shown'; } + +/** + * Show the batch details dialog for the library tracks at `filePaths`, + * in that order. Paths not in the library are left out; when none are, + * nothing is shown. + */ +export async function showBatchTrackDetailsForPaths( + dialog: () => TrackDetails | undefined, + filePaths: readonly string[], + retry: () => void, +): Promise { + return showBatchTrackDetails(dialog, await trackCache.get(filePaths), retry); +} + +/** + * Show the batch details dialog for tracks the caller already holds. + * + * The Tracks view is why this exists rather than only the path form: a + * selection there can be the whole library, and fetching every track of + * it back to read fields the rows in hand already carry cost 3 s and + * 700 ms of blocked main thread on 50 000 tracks (#281's own regression, + * measured). + */ +export async function showBatchTrackDetails( + dialog: () => TrackDetails | undefined, + tracks: readonly ListTrack[], + retry: () => void, +): Promise { + if (tracks.length === 0) return 'not-in-library'; + + const ready = await loadTrackDetails(retry); + + if (!ready) return 'chunk-failed'; + + const { coverArt, mixed } = batchCoverArt(tracks); + + dialog()?.showBatch(tracks, coverArt, mixed); + + return 'shown'; +} diff --git a/frontend/src/utils/track-index.ts b/frontend/src/utils/track-index.ts index 3685586..285d98c 100644 --- a/frontend/src/utils/track-index.ts +++ b/frontend/src/utils/track-index.ts @@ -20,22 +20,22 @@ * given array and never again. */ -import type * as library from '@go/library/models.js'; +/** Anything keyed by file path: a whole track, or the list's row. */ +interface HasFilePath { + FilePath: string; +} -const byArray = new WeakMap< - readonly library.Track[], - Map ->(); +const byArray = new WeakMap>(); /** The lookup for `tracks`, built once per array identity. */ -export function tracksByFilePath( - tracks: readonly library.Track[], -): Map { - let map = byArray.get(tracks); +export function tracksByFilePath( + tracks: readonly T[], +): Map { + let map = byArray.get(tracks) as Map | undefined; if (map) return map; - map = new Map(); + map = new Map(); for (const track of tracks) { // First wins: a duplicate path would be the same file, and @@ -49,12 +49,12 @@ export function tracksByFilePath( } /** Resolve file paths to tracks, dropping any that are not present. */ -export function tracksForPaths( - tracks: readonly library.Track[], +export function tracksForPaths( + tracks: readonly T[], filePaths: readonly string[], -): library.Track[] { +): T[] { const byPath = tracksByFilePath(tracks); - const result: library.Track[] = []; + const result: T[] = []; for (const filePath of filePaths) { const track = byPath.get(filePath); diff --git a/frontend/src/utils/track-table.ts b/frontend/src/utils/track-table.ts new file mode 100644 index 0000000..abc0c3f --- /dev/null +++ b/frontend/src/utils/track-table.ts @@ -0,0 +1,128 @@ +/** + * Decode the backend's `TrackTable` into the rows the Tracks view uses. + * + * The table is one array per column with every repeated string sent + * once (#281): ~167 bytes a track against the ~800 of the object-per- + * track JSON it replaced. This is its only decoder, and + * `backend/library/tracktable.go` its only encoder. + * + * Decoding builds plain objects of the same shape as before, so the + * filter, sort and selection code did not have to change — but every + * occurrence of a repeated string is now the *same* string, and every + * track with one genre list shares the array, which is part of why the + * JS heap shrinks along with the payload. Shared means read-only: no + * caller mutates a row's `Genre`. + */ + +import type * as library from '@go/library/models.js'; +import { listField } from '@utils/binding'; + +/** + * A track as the list carries it. The fields left out are the details + * dialog's, which reads whole tracks by path from `trackCache`. + */ +export type ListTrack = Omit< + library.Track, + 'LastPlayed' | 'CoverArtPath' | 'CoverArtMedium' | 'CoverArtLarge' +>; + +/** The `Track` fields a `ListTrack` does not carry. */ +export const LIST_TRACK_OMITS = [ + 'LastPlayed', + 'CoverArtPath', + 'CoverArtMedium', + 'CoverArtLarge', +] as const; + +/** A table whose columns disagree in length is a broken encoder, not data. */ +export class TrackTableError extends Error {} + +export function decodeTrackTable(table: library.TrackTable): ListTrack[] { + const strings = listField(table.strings); + const filePath = listField(table.filePath); + const n = filePath.length; + + const str = (col: number[] | null, name: string): string[] => { + const idx = listField(col); + + if (idx.length !== n) throw mismatch(name, idx.length, n); + + return idx.map((i) => { + const s = strings[i]; + + if (s === undefined) throw new TrackTableError(`${name}: string ${i} out of range`); + + return s; + }); + }; + const num = (col: number[] | null, name: string): number[] => { + const v = listField(col); + + if (v.length !== n) throw mismatch(name, v.length, n); + + return v; + }; + + const genreSets = listField(table.genreSets).map((set) => + listField(set).map((i) => strings[i] ?? ''), + ); + const genre = num(table.genre, 'genre'); + + const trackName = str(table.trackName, 'trackName'); + const artistName = str(table.artistName, 'artistName'); + const album = str(table.album, 'album'); + const composer = str(table.composer, 'composer'); + const fileType = str(table.fileType, 'fileType'); + const artistMbid = str(table.artistMbid, 'artistMbid'); + const releaseGroupMbid = str(table.releaseGroupMbid, 'releaseGroupMbid'); + const recordingMbid = str(table.recordingMbid, 'recordingMbid'); + const coverArtSmall = str(table.coverArtSmall, 'coverArtSmall'); + const lengthMs = num(table.lengthMs, 'lengthMs'); + const trackNumber = num(table.trackNumber, 'trackNumber'); + const discNumber = num(table.discNumber, 'discNumber'); + const year = num(table.year, 'year'); + const sampleRate = num(table.sampleRate, 'sampleRate'); + const bitDepth = num(table.bitDepth, 'bitDepth'); + const channels = num(table.channels, 'channels'); + const bitrate = num(table.bitrate, 'bitrate'); + const fileSize = num(table.fileSize, 'fileSize'); + const playCount = num(table.playCount, 'playCount'); + + const rows: ListTrack[] = new Array(n); + + for (let i = 0; i < n; i++) { + const g = genreSets[genre[i]!]; + + if (g === undefined) throw new TrackTableError(`genre: set ${genre[i]} out of range`); + + rows[i] = { + FilePath: filePath[i]!, + TrackName: trackName[i]!, + ArtistName: artistName[i]!, + Album: album[i]!, + Composer: composer[i]!, + FileType: fileType[i]!, + Genre: g, + ArtistMBID: artistMbid[i]!, + ReleaseGroupMBID: releaseGroupMbid[i]!, + RecordingMBID: recordingMbid[i]!, + CoverArtSmall: coverArtSmall[i]!, + TrackLength: String(lengthMs[i]!), + TrackNumber: trackNumber[i]!, + DiscNumber: discNumber[i]!, + Year: year[i]!, + SampleRate: sampleRate[i]!, + BitDepth: bitDepth[i]!, + Channels: channels[i]!, + Bitrate: bitrate[i]!, + FileSize: fileSize[i]!, + PlayCount: playCount[i]!, + }; + } + + return rows; +} + +function mismatch(name: string, got: number, want: number): TrackTableError { + return new TrackTableError(`${name}: ${got} values for ${want} tracks`); +} diff --git a/frontend/test/components/album-card-year.test.ts b/frontend/test/components/album-card-year.test.ts index ce57a48..bf4c4b4 100644 --- a/frontend/test/components/album-card-year.test.ts +++ b/frontend/test/components/album-card-year.test.ts @@ -19,6 +19,7 @@ import '@components/cover-grid/cover-grid'; import { emit, stub, flush, resetHarness } from '@test/support/harness'; import { Events } from '../../src/events'; import { fixture, shadowAll } from '@test/support/render'; +import { trackTable } from '@test/support/track-table'; const LONG = 'The Rise and Fall of a Midwest Princess in the Key of Everything'; @@ -49,7 +50,7 @@ describe('the album card’s year', () => { beforeEach(() => { resetHarness(); stub('library.Library.GetAlbums', ALBUMS); - stub('library.Library.GetTracks', []); + stub('library.Library.GetTrackTable', trackTable([])); emit(Events.LibraryScanComplete); }); diff --git a/frontend/test/components/album-dropdown.test.ts b/frontend/test/components/album-dropdown.test.ts index 8931287..c92adc9 100644 --- a/frontend/test/components/album-dropdown.test.ts +++ b/frontend/test/components/album-dropdown.test.ts @@ -22,6 +22,7 @@ import '@components/cover-grid/cover-grid'; import { emit, stub, flush, resetHarness } from '@test/support/harness'; import { Events } from '../../src/events'; import { fixture, shadow, shadowAll } from '@test/support/render'; +import { trackTable } from '@test/support/track-table'; /** * Enough albums to fill more than one row. @@ -82,7 +83,7 @@ describe('the album dropdown', () => { beforeEach(() => { resetHarness(); stub('library.Library.GetAlbums', ALBUMS); - stub('library.Library.GetTracks', []); + stub('library.Library.GetTrackTable', trackTable([])); stub('library.Library.GetAlbumTracks', TRACKS); stub('library.Library.GetAlbumTracks', TRACKS); emit(Events.LibraryScanComplete); @@ -155,7 +156,7 @@ describe('the albums grid scrolls', () => { beforeEach(() => { resetHarness(); stub('library.Library.GetAlbums', ALBUMS); - stub('library.Library.GetTracks', []); + stub('library.Library.GetTrackTable', trackTable([])); emit(Events.LibraryScanComplete); }); diff --git a/frontend/test/components/aria-tail.test.ts b/frontend/test/components/aria-tail.test.ts index 3d14a14..5cd1f10 100644 --- a/frontend/test/components/aria-tail.test.ts +++ b/frontend/test/components/aria-tail.test.ts @@ -20,6 +20,7 @@ import { emit, stub, flush, resetHarness } from '@test/support/harness'; import { Events } from '../../src/events'; import { fixture, shadow, shadowAll } from '@test/support/render'; import { searchStore } from '@store/search-store'; +import { trackTable } from '@test/support/track-table'; /** * The searchable columns' accessors read these fields and call @@ -76,7 +77,7 @@ describe('the track list says how it is sorted', () => { beforeEach(async () => { resetHarness(); searchStore.setTerm(''); - stub('library.Library.GetTracks', TRACKS); + stub('library.Library.GetTrackTable', trackTable(TRACKS)); stub('library.Library.GetAlbums', []); emit(Events.LibraryScanComplete); }); @@ -132,7 +133,7 @@ describe('the track list has a voice for its own state', () => { }); it('announces the result of a search that matches nothing', async () => { - stub('library.Library.GetTracks', TRACKS); + stub('library.Library.GetTrackTable', trackTable(TRACKS)); emit(Events.LibraryScanComplete); const el = await fixture('track-list'); @@ -163,7 +164,7 @@ describe('a selectable grid is a listbox, not a row of buttons', () => { searchStore.setTerm(''); stub('library.Library.GetArtists', ARTISTS); stub('library.Library.GetGenres', GENRES); - stub('library.Library.GetTracks', []); + stub('library.Library.GetTrackTable', trackTable([])); stub('library.Library.GetAlbums', []); emit(Events.LibraryScanComplete); }); @@ -197,7 +198,7 @@ describe('a clipped value is readable somewhere', () => { beforeEach(async () => { resetHarness(); searchStore.setTerm(''); - stub('library.Library.GetTracks', TRACKS); + stub('library.Library.GetTrackTable', trackTable(TRACKS)); stub('library.Library.GetAlbums', []); emit(Events.LibraryScanComplete); }); @@ -240,7 +241,7 @@ describe('the playing row is more than a colour', () => { beforeEach(async () => { resetHarness(); searchStore.setTerm(''); - stub('library.Library.GetTracks', TRACKS); + stub('library.Library.GetTrackTable', trackTable(TRACKS)); stub('library.Library.GetAlbums', []); emit(Events.LibraryScanComplete); }); diff --git a/frontend/test/components/art-prefetch.test.ts b/frontend/test/components/art-prefetch.test.ts index 80b88e0..b7922ca 100644 --- a/frontend/test/components/art-prefetch.test.ts +++ b/frontend/test/components/art-prefetch.test.ts @@ -35,6 +35,7 @@ import { imagePrefetched, resetImagePrefetch, } from '@utils/image-prefetch'; +import { trackTable } from '@test/support/track-table'; /** Enough albums that the virtualizer's own window is nowhere near the end. */ const ALBUMS = Array.from({ length: 400 }, (_, i) => { @@ -118,7 +119,7 @@ beforeEach(() => { localStorage.clear(); stub('library.Library.GetAlbums', ALBUMS); stub('library.Library.GetArtists', ARTISTS); - stub('library.Library.GetTracks', []); + stub('library.Library.GetTrackTable', trackTable([])); stub('library.Library.GetGenres', []); emit(Events.LibraryScanComplete); }); diff --git a/frontend/test/components/card-grid-repaint.test.ts b/frontend/test/components/card-grid-repaint.test.ts index 0894daa..26033b8 100644 --- a/frontend/test/components/card-grid-repaint.test.ts +++ b/frontend/test/components/card-grid-repaint.test.ts @@ -29,6 +29,7 @@ import '@components/genres-view/genres-view'; import { emit, stub, flush, resetHarness } from '@test/support/harness'; import { Events } from '../../src/events'; import { fixture, shadowAll } from '@test/support/render'; +import { trackTable } from '@test/support/track-table'; const ARTISTS = [ { ID: 1, Name: 'Alpha', AlbumCount: 2, TrackCount: 9 }, @@ -58,7 +59,7 @@ describe('a card grid shows its selection', () => { resetHarness(); stub('library.Library.GetArtists', ARTISTS); stub('library.Library.GetGenres', GENRES); - stub('library.Library.GetTracks', []); + stub('library.Library.GetTrackTable', trackTable([])); stub('library.Library.GetAlbums', []); // The views read through LibraryController, whose cache is only // primed by a scan-complete; without it they render nothing and the diff --git a/frontend/test/components/chrome.test.ts b/frontend/test/components/chrome.test.ts index b2dc871..d797c10 100644 --- a/frontend/test/components/chrome.test.ts +++ b/frontend/test/components/chrome.test.ts @@ -11,7 +11,8 @@ import '@components/library-filter/library-filter'; import '@components/library-status-indicator/library-status-indicator'; import { Events } from '../../src/events'; import { activeViewStore } from '@store/active-view-store'; -import { emit, stub, flush, calls, lastArgs } from '@test/support/harness'; +import { libraryStore } from '@store/library-store'; +import { emit, stub, flush, calls, lastArgs, resetHarness } from '@test/support/harness'; import { fixture, shadow, @@ -181,6 +182,14 @@ describe('', () => { it('selects a library by id, and the merged view by empty string', async () => { const el = await fixture('library-filter'); + // The list has to be in use for the filter change to refetch it + // (#280): a collection nothing has loaded is not loaded to correct + // it, so this is the state the app is in when the Tracks view is up. + stub('library.Library.GetTrackTable', { strings: [''] }); + await libraryStore.getTracks(); + resetHarness(); + stub('library.Library.GetTrackTable', { strings: [''] }); + await flush(); await el.updateComplete; @@ -191,7 +200,7 @@ describe('', () => { select?.dispatchEvent(new Event('change')); await flush(); - expect(lastArgs('library.Library.GetTracks')).toEqual([8]); + expect(lastArgs('library.Library.GetTrackTable')).toEqual([8]); }); it('picks up a library added while it was on screen', async () => { diff --git a/frontend/test/components/detail-touch-targets.test.ts b/frontend/test/components/detail-touch-targets.test.ts index 9cc5120..d308836 100644 --- a/frontend/test/components/detail-touch-targets.test.ts +++ b/frontend/test/components/detail-touch-targets.test.ts @@ -72,7 +72,7 @@ function boxOf(el: Element | null | undefined): { w: number; h: number } { describe('the way out of a detail view', () => { beforeEach(() => { for (const path of [ - 'library.Library.GetTracks', + 'library.Library.GetTrackTable', 'library.Library.GetAlbums', 'library.Library.GetArtists', 'library.Library.GetGenres', diff --git a/frontend/test/components/empty-states.test.ts b/frontend/test/components/empty-states.test.ts index 46fab5b..27b2ca9 100644 --- a/frontend/test/components/empty-states.test.ts +++ b/frontend/test/components/empty-states.test.ts @@ -11,11 +11,12 @@ import '@components/track-list/track-list'; import { Events } from '../../src/events'; import { emit, stub, stubFailure, flush, resetHarness } from '@test/support/harness'; import { fixture, shadow, text } from '@test/support/render'; +import { trackTable } from '@test/support/track-table'; /** Drop the library store's cache so the list has to fetch. */ async function emptyLibrary(): Promise { resetHarness(); - stub('library.Library.GetTracks', []); + stub('library.Library.GetTrackTable', trackTable([])); stub('library.Library.GetAlbums', []); stub('library.Library.GetArtists', []); stub('library.Library.GetGenres', []); @@ -40,7 +41,7 @@ describe(' empty, loading and failed', () => { }); it('says the query failed, and offers to try again', async () => { - stubFailure('library.Library.GetTracks', 'sql: database is locked'); + stubFailure('library.Library.GetTrackTable', 'sql: database is locked'); emit(Events.LibraryScanComplete); await flush(); diff --git a/frontend/test/components/explore-track-details.test.ts b/frontend/test/components/explore-track-details.test.ts index aab73a9..c051ee2 100644 --- a/frontend/test/components/explore-track-details.test.ts +++ b/frontend/test/components/explore-track-details.test.ts @@ -28,7 +28,7 @@ import type { TrackDetails } from '@components/track-details/track-details'; const ALBUM_TRACKS = 'library.Library.GetAlbumTracks'; const COMPLETENESS = 'library.Library.GetAlbumCompleteness'; const FILE_PATHS = 'library.Library.GetFilePathsByRecordingMBIDs'; -const ALL_TRACKS = 'library.Library.GetTracks'; +const TRACKS_BY_PATH = 'library.Library.GetTracksByPaths'; const LOOKUP_RG = 'explore.Service.LookupReleaseGroup'; const BROWSE_RELEASES = 'explore.Service.BrowseReleases'; @@ -127,14 +127,13 @@ describe('Explore track details', () => { stub(ALBUM_TRACKS, [albumTrack]); stub(COMPLETENESS, { known: true, complete: true, owned: 1, expected: 1 }); stub(FILE_PATHS, { [MBID]: [PATH] }); - stub(ALL_TRACKS, [libraryTrack]); + stub(TRACKS_BY_PATH, [libraryTrack]); }); /** - * `libraryStore` fetches at import and caches the empty list the - * shared setup stubs, for the life of the browser session — so a test - * that wants tracks in it has to say so. A scan-complete event is how - * the app itself invalidates that cache. + * `trackCache` keeps what it was answered for the life of the browser + * session, so a test that changes the answer has to drop it. A + * scan-complete event is how the app itself invalidates that cache. */ async function primeLibrary() { emit(Events.LibraryScanComplete); @@ -204,7 +203,7 @@ describe('Explore track details', () => { items[details]!.dispatchEvent(new MouseEvent('click', { bubbles: true })); // Polled rather than counted: the opener is three awaits deep — the - // path lookup, the store's tracks, and the dynamic `import()` of + // path lookup, the track by path, and the dynamic `import()` of // the dialog chunk — and a chunk fetch is the one of the three // whose cost depends on what else the suite is doing. const shown = await dialogTrack(el); @@ -230,7 +229,7 @@ describe('Explore track details', () => { }); it('reports a path with no library track rather than opening empty', async () => { - stub(ALL_TRACKS, []); + stub(TRACKS_BY_PATH, []); await primeLibrary(); const outcome = await showTrackDetailsForPath( diff --git a/frontend/test/components/keyboard-reach.test.ts b/frontend/test/components/keyboard-reach.test.ts index 1aabdd0..62333c8 100644 --- a/frontend/test/components/keyboard-reach.test.ts +++ b/frontend/test/components/keyboard-reach.test.ts @@ -14,6 +14,7 @@ import '@components/track-list/track-list'; import { stub, emit, flush } from '@test/support/harness'; import { Events } from '../../src/events'; import { fixture, shadow, shadowAll, update } from '@test/support/render'; +import { trackTable } from '@test/support/track-table'; /** Two fixture tracks, enough to move a focus ring between. */ const TRACKS = [ @@ -78,7 +79,7 @@ describe(' when closed', () => { describe(' roving tabindex', () => { beforeEach(() => { - stub('library.Library.GetTracks', TRACKS); + stub('library.Library.GetTrackTable', trackTable(TRACKS)); }); it('offers exactly one tab stop, however many rows there are', async () => { diff --git a/frontend/test/components/lazy-track-details.test.ts b/frontend/test/components/lazy-track-details.test.ts index 1c8de60..78cd979 100644 --- a/frontend/test/components/lazy-track-details.test.ts +++ b/frontend/test/components/lazy-track-details.test.ts @@ -51,8 +51,12 @@ describe('track-details stays out of the startup chunk', () => { }, ); + // Directly, or through `utils/track-details-opener`, which awaits + // it and is what the path-keyed openers share (#279). it.each(OPENERS)('%s loads it at the point of use', (_name, source) => { - expect(source).toContain('loadTrackDetails'); + expect(source).toMatch( + /loadTrackDetails|showTrackDetailsForPath|showBatchTrackDetailsForPaths/, + ); }); }); diff --git a/frontend/test/components/library-status.test.ts b/frontend/test/components/library-status.test.ts index 6945552..89f5b04 100644 --- a/frontend/test/components/library-status.test.ts +++ b/frontend/test/components/library-status.test.ts @@ -31,6 +31,7 @@ import { stubFailure, } from '@test/support/harness'; import { fixture, shadow, shadowAll, update } from '@test/support/render'; +import { trackTable } from '@test/support/track-table'; const SEARCH = 'explore.Service.SearchLocal'; @@ -133,7 +134,7 @@ describe(' badges', () => { stub('explore.Service.GetArtistImageURL', ''); stub('explore.Service.GetExploreShelves', { shelves: [], state: 'ready' }); stub('library.Library.GetAlbums', []); - stub('library.Library.GetTracks', []); + stub('library.Library.GetTrackTable', trackTable([])); await withRequests([]); }); diff --git a/frontend/test/components/list-render-cost.test.ts b/frontend/test/components/list-render-cost.test.ts index a4566d6..18c5ca5 100644 --- a/frontend/test/components/list-render-cost.test.ts +++ b/frontend/test/components/list-render-cost.test.ts @@ -48,21 +48,12 @@ describe('the track list Art column', () => { ]).toEqual(['lazy', 'async']); }); - it('falls back through the tiers rather than rendering nothing', () => { - const onlyOriginal = cell('albumArt', { - CoverArtPath: '/covers/abc.jpg', - CoverArtSmall: '', - CoverArtMedium: '', - }).querySelector('img'); - - expect(onlyOriginal?.getAttribute('src')).toBe('/covers/abc.jpg'); - }); - + // There is no fallback through the larger tiers any more: the list + // carries only the small one (#281), and the backend derives every + // tier from the same file, so it is set whenever any of them is. it('renders nothing at all when there is no art', () => { const none = cell('albumArt', { - CoverArtPath: '', CoverArtSmall: '', - CoverArtMedium: '', }); expect(none.querySelector('img')).toBeNull(); diff --git a/frontend/test/components/smart-playlist-suggestions.test.ts b/frontend/test/components/smart-playlist-suggestions.test.ts new file mode 100644 index 0000000..0c113da --- /dev/null +++ b/frontend/test/components/smart-playlist-suggestions.test.ts @@ -0,0 +1,86 @@ +/** + * The rule editor's value box suggests values from the library by + * asking for the ones that match what has been typed (#279). + * + * It used to build the lists from `libraryStore`'s whole-library + * arrays: one more reason to load every track at startup, and an empty + * list whenever they had not loaded. These tests stub *no* library + * collection — what is asserted is that the suggestions arrive anyway, + * from the one call made for them. + */ +import { describe, expect, it, beforeEach } from 'vitest'; + +import '@components/smart-playlist-editor/smart-playlist-editor'; +import type { YjCombobox } from '@components/combobox/combobox'; +import { calls, resetHarness, stub } from '@test/support/harness'; +import { fixture, shadow, shadowAll } from '@test/support/render'; +import type { LitElement } from 'lit'; + +const SUGGEST = 'playlist.Service.SuggestSmartPlaylistValues'; + +async function editor(field: string): Promise { + return fixture('smart-playlist-editor', { + rules: JSON.stringify({ rules: [{ field, operator: 'contains', value: '' }] }), + }); +} + +/** The row's second combobox: the value box (the first picks the field). */ +function valueBox(el: LitElement): YjCombobox { + return shadowAll(el, 'yj-combobox')[1]!; +} + +async function type(box: YjCombobox, text: string): Promise { + const input = shadow(box, 'input')!; + + input.focus(); + input.value = text; + input.dispatchEvent(new Event('input', { bubbles: true })); + await box.updateComplete; +} + +describe('smart playlist value suggestions', () => { + beforeEach(() => { + resetHarness(); + stub(SUGGEST, (field: string, needle: string) => + [`${field}:${needle}:1`, `${field}:${needle}:2`], + ); + }); + + it('asks for the values matching what was typed, once typing pauses', async () => { + const el = await editor('artist'); + const box = valueBox(el); + + await type(box, 'q'); + await type(box, 'qu'); + + await expect.poll(() => box.options).toEqual(['artist:qu:1', 'artist:qu:2']); + // Debounced: the pause after "qu" asked, the keystroke before did not. + expect(calls(SUGGEST).map((c) => c.args)).toEqual([['artist', 'qu']]); + }); + + it('answers a repeat from what it already fetched', async () => { + const el = await editor('album'); + const box = valueBox(el); + + await type(box, 'x'); + await expect.poll(() => box.options).toEqual(['album:x:1', 'album:x:2']); + await type(box, 'xy'); + await expect.poll(() => box.options).toEqual(['album:xy:1', 'album:xy:2']); + await type(box, 'x'); + + expect(box.options).toEqual(['album:x:1', 'album:x:2']); + expect(calls(SUGGEST)).toHaveLength(2); + }); + + it('asks nothing for a field that has no suggestions', async () => { + const el = await editor('duration'); + const boxes = shadowAll(el, 'yj-combobox'); + + // A numeric range field may render no value combobox at all; if it + // does, typing in it must not ask. + if (boxes[1]) await type(boxes[1], '3'); + await new Promise((r) => setTimeout(r, 250)); + + expect(calls(SUGGEST)).toHaveLength(0); + }); +}); diff --git a/frontend/test/components/smoke.test.ts b/frontend/test/components/smoke.test.ts index e4c3f16..0439b3a 100644 --- a/frontend/test/components/smoke.test.ts +++ b/frontend/test/components/smoke.test.ts @@ -117,7 +117,7 @@ const TAGS = [ */ function stubEmptyBackend(): void { const emptyLists = [ - 'library.Library.GetTracks', + 'library.Library.GetTrackTable', 'library.Library.GetAlbums', 'library.Library.GetArtists', 'library.Library.GetGenres', diff --git a/frontend/test/components/touch-selection.test.ts b/frontend/test/components/touch-selection.test.ts index a4fec7b..be7c656 100644 --- a/frontend/test/components/touch-selection.test.ts +++ b/frontend/test/components/touch-selection.test.ts @@ -24,6 +24,7 @@ import '@components/selection-bar/selection-bar'; import { calls, flush, resetHarness, stub } from '@test/support/harness'; import { fixture, shadow, shadowAll } from '@test/support/render'; import { installTouchGestures, LONG_PRESS_MS } from '@utils/touch-gestures'; +import { trackTable } from '@test/support/track-table'; const HELD = LONG_PRESS_MS + 120; @@ -117,7 +118,7 @@ function rows(el: HTMLElement): HTMLElement[] { describe('a finger on a track row', () => { beforeEach(() => { resetHarness(); - stub('library.Library.GetTracks', TRACKS); + stub('library.Library.GetTrackTable', trackTable(TRACKS)); stub('library.Library.GetAllLibrariesWithTrackCounts', []); stub('config.Config.GetShortcuts', {}); stub('queue.Queue.SetQueue', null); @@ -320,7 +321,7 @@ describe('', () => { describe('a tap on a name inside a row', () => { beforeEach(() => { resetHarness(); - stub('library.Library.GetTracks', TRACKS); + stub('library.Library.GetTrackTable', trackTable(TRACKS)); stub('library.Library.GetAllLibrariesWithTrackCounts', []); stub('config.Config.GetShortcuts', {}); stub('queue.Queue.SetQueue', null); diff --git a/frontend/test/components/touch-swipe.test.ts b/frontend/test/components/touch-swipe.test.ts index 07f9595..e343df8 100644 --- a/frontend/test/components/touch-swipe.test.ts +++ b/frontend/test/components/touch-swipe.test.ts @@ -33,6 +33,7 @@ import '@components/track-list/track-list'; import { calls, flush, resetHarness, stub } from '@test/support/harness'; import { fixture, shadow, shadowAll } from '@test/support/render'; import { installTouchGestures } from '@utils/touch-gestures'; +import { trackTable } from '@test/support/track-table'; const wait = (ms: number) => new Promise((r) => setTimeout(r, ms)); @@ -125,7 +126,7 @@ function threshold(row: HTMLElement): number { describe('a finger swiped right across a track row', () => { beforeEach(() => { resetHarness(); - stub('library.Library.GetTracks', TRACKS); + stub('library.Library.GetTrackTable', trackTable(TRACKS)); stub('library.Library.GetAllLibrariesWithTrackCounts', []); stub('config.Config.GetShortcuts', {}); stub('queue.Queue.SetQueue', null); diff --git a/frontend/test/components/view-lifecycle.test.ts b/frontend/test/components/view-lifecycle.test.ts index 14b3ed8..5b6a38c 100644 --- a/frontend/test/components/view-lifecycle.test.ts +++ b/frontend/test/components/view-lifecycle.test.ts @@ -231,7 +231,7 @@ const CACHED_VIEWS = [ * binding resolves undefined, which is not what Go sends. */ function stubEmptyBackend(): void { for (const path of [ - 'library.Library.GetTracks', + 'library.Library.GetTrackTable', 'library.Library.GetAlbums', 'library.Library.GetArtists', 'library.Library.GetGenres', diff --git a/frontend/test/setup.ts b/frontend/test/setup.ts index a2c2fef..3929699 100644 --- a/frontend/test/setup.ts +++ b/frontend/test/setup.ts @@ -35,7 +35,7 @@ const importTimeDefaults: Array<[string, unknown]> = [ // libraryStore and playlistStore fetch eagerly at import. Left // unstubbed they would cache `undefined` — not the empty list Go // sends — and every consumer would then crash on `.length`. - ['library.Library.GetTracks', []], + ['library.Library.GetTrackTable', { strings: [''] }], ['library.Library.GetAlbums', []], ['library.Library.GetArtists', []], ['library.Library.GetGenres', []], diff --git a/frontend/test/stores/library-lazy.test.ts b/frontend/test/stores/library-lazy.test.ts new file mode 100644 index 0000000..1c8dfdd --- /dev/null +++ b/frontend/test/stores/library-lazy.test.ts @@ -0,0 +1,99 @@ +/** + * #280: the app loads the library when a view needs it, not at startup. + * + * Every collection used to be fetched together at `DOMContentLoaded` + * and refetched on every invalidation, whether or not anything was + * showing it. On a 26 138-track library the track list was 20.5 MB of + * that and cost the backend ~170 MB of transient allocation — for + * someone looking at Home, which draws none of it. + * + * **This file must not load the track list before the first test.** It + * asserts the state the app is actually left in by its own startup, so + * a test added above that asks for tracks would invalidate the + * precondition rather than silently pass. + */ +import { describe, expect, it, beforeEach } from 'vitest'; + +import { libraryStore } from '@store/library-store'; +import { Events } from '../../src/events'; +import { + calls, + emit, + flush, + resetHarness, + stub, +} from '@test/support/harness'; + +const TRACKS = 'library.Library.GetTrackTable'; +const SMALL = [ + 'library.Library.GetAlbums', + 'library.Library.GetArtists', + 'library.Library.GetGenres', +]; + +/** + * Wait for the store's own idle warm-up to land. + * + * It asks for the three small collections on `requestIdleCallback`, so + * when it has happened is the browser's decision — waiting for the + * effect rather than for a duration is the only way this is not a race. + */ +function stubReads(): void { + stub(TRACKS, { strings: [''] }); + for (const path of SMALL) stub(path, []); +} + +async function waitForWarm(): Promise { + for (let i = 0; i < 200 && libraryStore.getCachedAlbums() === null; i++) { + await flush(); + } + + expect(libraryStore.getCachedAlbums(), 'the warm-up ran').not.toBeNull(); +} + +describe('what the app loads on its own (#280)', () => { + beforeEach(async () => { + stubReads(); + await waitForWarm(); + // resetHarness clears the stubs as well as the recorded calls. + resetHarness(); + stubReads(); + }); + + it('leaves the track list alone', () => { + expect(libraryStore.getCachedTracks()).toBeNull(); + expect(calls(TRACKS)).toEqual([]); + }); + + it('does not load it to answer a scan', async () => { + emit(Events.LibraryScanComplete); + await flush(); + + // The small collections are in use, so they are refreshed; the + // track list nobody has opened is not. + expect(calls().map((c) => c.path).sort()).toEqual([...SMALL].sort()); + }); + + it('loads it for the view that draws it, and refreshes it from then on', async () => { + libraryStore.prefetch('tracks'); + await flush(); + + expect(calls(TRACKS)).toHaveLength(1); + expect(libraryStore.getCachedTracks()).not.toBeNull(); + + resetHarness(); + emit(Events.LibraryScanComplete); + await flush(); + + expect(calls(TRACKS), 'in use now, so a scan refreshes it').toHaveLength(1); + }); + + it('is asked for nothing by a prefetch for a view it has no data for', async () => { + libraryStore.prefetch('home'); + libraryStore.prefetch('settings'); + libraryStore.prefetch('explore'); + await flush(); + + expect(calls()).toEqual([]); + }); +}); diff --git a/frontend/test/stores/library-store.test.ts b/frontend/test/stores/library-store.test.ts index 2430135..8be486a 100644 --- a/frontend/test/stores/library-store.test.ts +++ b/frontend/test/stores/library-store.test.ts @@ -19,10 +19,17 @@ import { lastArgs, resetHarness, } from '@test/support/harness'; +import { trackTable } from '@test/support/track-table'; + +const RETAGGED = { + TrackName: 'Retagged', + FilePath: '/a.mp3', + PlayCount: 0, +}; const TRACKS = [ - { ID: 1, Title: 'One', FilePath: '/a.mp3', PlayCount: 0, LastPlayed: '' }, - { ID: 2, Title: 'Two', FilePath: '/b.mp3', PlayCount: 4, LastPlayed: 'x' }, + { TrackName: 'One', FilePath: '/a.mp3', PlayCount: 0 }, + { TrackName: 'Two', FilePath: '/b.mp3', PlayCount: 4 }, ]; const ALBUMS = [{ ID: 1, Name: 'Album', ArtistName: 'Artist' }]; const OTHER_ALBUMS = [{ ID: 2, Name: 'Other', ArtistName: 'Other Artist' }]; @@ -33,30 +40,34 @@ const LIBRARIES = [{ id: 7, name: 'Music' }, { id: 8, name: 'Field' }]; /** Stub every read binding the store can reach. Unstubbed bindings * resolve undefined, which the store would cache as if it were data. */ function stubReads(): void { - stub('library.Library.GetTracks', TRACKS); + stub('library.Library.GetTrackTable', trackTable(TRACKS)); stub('library.Library.GetAlbums', ALBUMS); stub('library.Library.GetArtists', ARTISTS); stub('library.Library.GetGenres', GENRES); - stub('library.Library.GetTracks', TRACKS); - stub('library.Library.GetAlbums', ALBUMS); - stub('library.Library.GetArtists', ARTISTS); - stub('library.Library.GetGenres', GENRES); - stub('library.Library.GetAlbumsByArtist', ALBUMS); stub('library.Library.GetAlbumsByArtist', ALBUMS); stub('library.Library.GetAllLibrariesWithTrackCounts', LIBRARIES); } /** - * Drop the cache and let the eager refetch settle, so each test starts - * from the same place. The store has no reset of its own; a scan - * completing is how the app itself clears it. + * Start each test from a loaded store. The store has no reset of its + * own; a scan completing is how the app itself clears it — and since + * #280 it reloads only what something had loaded, so the four reads + * here are what says "this test is about a loaded store". */ async function reload(): Promise { stubReads(); emit(Events.LibraryScanComplete); await flush(); - // The eager refetch the invalidation kicks off is recorded like any - // other call; clear it, or every count in every test is off by one. + resetHarness(); + stubReads(); + await Promise.all([ + libraryStore.getTracks(), + libraryStore.getAlbums(), + libraryStore.getArtists(), + libraryStore.getGenres(), + ]); + // Those reads are recorded like any other call; clear them, or every + // count in every test is off by one. resetHarness(); stubReads(); } @@ -69,7 +80,7 @@ describe('library store: caching', () => { it('serves a second read from cache without touching the backend', async () => { await libraryStore.getTracks(); - expect(calls('library.Library.GetTracks')).toHaveLength(0); + expect(calls('library.Library.GetTrackTable')).toHaveLength(0); }); it('deduplicates concurrent first reads into one backend call', async () => { @@ -88,14 +99,14 @@ describe('library store: caching', () => { it('exposes cached collections synchronously once loaded', () => { expect([ - libraryStore.getCachedTracks(), + libraryStore.getCachedTracks()?.map((t) => t.FilePath), libraryStore.getCachedAlbums(), libraryStore.cachedArtists, libraryStore.getCachedGenres(), - ]).toEqual([TRACKS, ALBUMS, ARTISTS, GENRES]); + ]).toEqual([['/a.mp3', '/b.mp3'], ALBUMS, ARTISTS, GENRES]); }); - it('refetches everything when a scan completes', async () => { + it('refetches everything a view had loaded when a scan completes', async () => { emit(Events.LibraryScanComplete); await flush(); @@ -103,15 +114,98 @@ describe('library store: caching', () => { 'library.Library.GetAlbums', 'library.Library.GetArtists', 'library.Library.GetGenres', - 'library.Library.GetTracks', + 'library.Library.GetTrackTable', ]); }); - 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.GetTracks')).toHaveLength(1); + 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); }); /* @@ -140,10 +234,11 @@ describe('library store: caching', () => { it('patches the one track it names', () => { const tracks = libraryStore.getCachedTracks(); + // LastPlayed is not on the list's rows (#281); trackCache + // patches it on whole tracks. expect(tracks?.[0]).toMatchObject({ FilePath: '/a.mp3', PlayCount: 9, - LastPlayed: '2026-08-11 10:00:00', }); }); @@ -192,7 +287,7 @@ describe('library store: caching', () => { }); it('does not refetch the tracks', () => { - expect(calls('library.Library.GetTracks')).toHaveLength(0); + expect(calls('library.Library.GetTrackTable')).toHaveLength(0); }); it('splices the removed track out in place', () => { @@ -258,7 +353,7 @@ describe('library store: library filter', () => { libraryStore.setSelectedLibrary(7); await flush(); - expect(lastArgs('library.Library.GetTracks')).toEqual([7]); + expect(lastArgs('library.Library.GetTrackTable')).toEqual([7]); }); it('ignores a redundant selection instead of invalidating', async () => { @@ -304,12 +399,12 @@ describe('library store: a fetch that is overtaken', () => { it('serves the library that is selected, not the one that was in flight', async () => { const pending: Array<{ id: number; resolve: (v: unknown) => void }> = []; - const byLibrary = (id: number) => [{ ID: id, Title: `Library ${id}` }]; + const byLibrary = (id: number) => trackTable([{ FilePath: `/lib-${id}.mp3` }]); // Only the track fetch is held open; the other three settle at once, // so the test is about the overtaking and nothing else. stub( - 'library.Library.GetTracks', + 'library.Library.GetTrackTable', (id: number) => new Promise((resolve) => { pending.push({ id, resolve }); @@ -327,11 +422,13 @@ describe('library store: a fetch that is overtaken', () => { pending.find((p) => p.id === 8)?.resolve(byLibrary(8)); await flush(); - expect(libraryStore.getCachedTracks()).toEqual(byLibrary(8)); + expect(libraryStore.getCachedTracks()?.map((t) => t.FilePath)).toEqual([ + '/lib-8.mp3', + ]); }); it('settles the waiters when the fetch they are waiting on fails', async () => { - stubFailure('library.Library.GetTracks', 'sql: database is locked'); + stubFailure('library.Library.GetTrackTable', 'sql: database is locked'); // Invalidation drops the cache and starts the fetch that fails. emit(Events.LibraryScanComplete); diff --git a/frontend/test/stores/track-cache.test.ts b/frontend/test/stores/track-cache.test.ts new file mode 100644 index 0000000..8804e19 --- /dev/null +++ b/frontend/test/stores/track-cache.test.ts @@ -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('../../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()); + }); +}); diff --git a/frontend/test/support/track-table.ts b/frontend/test/support/track-table.ts new file mode 100644 index 0000000..66d4a2d --- /dev/null +++ b/frontend/test/support/track-table.ts @@ -0,0 +1,86 @@ +/** + * Encode test tracks as the backend's `TrackTable` (#281), so a stub of + * `library.Library.GetTrackTable` can be written as a list of tracks. + * + * Test fixtures are partial tracks; a missing field encodes as the + * zero value Go would have sent. The encoding mirrors + * `backend/library/tracktable.go` closely enough to exercise the real + * decoder: shared string table with "" at 0, genre lists interned. + */ +import type * as library from '@go/library/models.js'; + +/** A fixture: any subset of a track's fields, loosely typed as fixtures are. */ +type PartialTrack = { FilePath: string } & Record; + +const STRING_COLS = [ + ['trackName', 'TrackName'], + ['artistName', 'ArtistName'], + ['album', 'Album'], + ['composer', 'Composer'], + ['fileType', 'FileType'], + ['artistMbid', 'ArtistMBID'], + ['releaseGroupMbid', 'ReleaseGroupMBID'], + ['recordingMbid', 'RecordingMBID'], + ['coverArtSmall', 'CoverArtSmall'], +] as const; + +const INT_COLS = [ + ['trackNumber', 'TrackNumber'], + ['discNumber', 'DiscNumber'], + ['year', 'Year'], + ['sampleRate', 'SampleRate'], + ['bitDepth', 'BitDepth'], + ['channels', 'Channels'], + ['bitrate', 'Bitrate'], + ['fileSize', 'FileSize'], + ['playCount', 'PlayCount'], +] as const; + +export function trackTable(tracks: readonly PartialTrack[]): library.TrackTable { + const strings = ['']; + const index = new Map([['', 0]]); + const intern = (s: string | undefined): number => { + const v = s ?? ''; + let i = index.get(v); + + if (i === undefined) { + i = strings.length; + strings.push(v); + index.set(v, i); + } + + return i; + }; + + const genreSets: number[][] = []; + const genreIndex = new Map(); + const table: Record = { + strings, + genreSets, + filePath: tracks.map((t) => t.FilePath), + lengthMs: tracks.map((t) => Number(t['TrackLength'] ?? 0) || 0), + genre: tracks.map((t) => { + const genres = (t['Genre'] as string[] | null | undefined) ?? []; + const key = genres.join('\u0000'); + let i = genreIndex.get(key); + + if (i === undefined) { + i = genreSets.length; + genreSets.push(genres.map(intern)); + genreIndex.set(key, i); + } + + return i; + }), + }; + + for (const [col, field] of STRING_COLS) { + table[col] = tracks.map((t) => intern(t[field] as string | undefined)); + } + + for (const [col, field] of INT_COLS) { + table[col] = tracks.map((t) => (t[field] as number | undefined) ?? 0); + } + + return table as unknown as library.TrackTable; +} diff --git a/frontend/test/utils/track-table.test.ts b/frontend/test/utils/track-table.test.ts new file mode 100644 index 0000000..47a630a --- /dev/null +++ b/frontend/test/utils/track-table.test.ts @@ -0,0 +1,134 @@ +/** + * `decodeTrackTable` is the only reader of the backend's columnar track + * list (#281). The encoder is Go and the row type is TypeScript, so the + * last block reads both sides' declarations and fails if a column is + * added on one and not the other. + */ +import { describe, expect, it } from 'vitest'; + +import { + decodeTrackTable, + LIST_TRACK_OMITS, + TrackTableError, +} from '@utils/track-table'; +import { trackTable } from '@test/support/track-table'; + +const FIXTURE = [ + { + FilePath: '/m/a/1.flac', + TrackName: 'One', + ArtistName: 'Artist', + Album: 'Album', + Genre: ['Ambient', 'Drone'], + TrackLength: '215000', + TrackNumber: 1, + CoverArtSmall: '/covers/x_sm.jpg', + PlayCount: 3, + }, + { + FilePath: '/m/a/2.flac', + TrackName: 'Two', + ArtistName: 'Artist', + Album: 'Album', + Genre: ['Ambient', 'Drone'], + TrackLength: '1000', + TrackNumber: 2, + CoverArtSmall: '/covers/x_sm.jpg', + }, +]; + +describe('decodeTrackTable', () => { + it('gives back each track it was given', () => { + const [a, b] = decodeTrackTable(trackTable(FIXTURE)); + + expect(a).toMatchObject({ + FilePath: '/m/a/1.flac', + TrackName: 'One', + Album: 'Album', + Genre: ['Ambient', 'Drone'], + TrackLength: '215000', + TrackNumber: 1, + PlayCount: 3, + Composer: '', + }); + expect(b).toMatchObject({ TrackName: 'Two', TrackLength: '1000', PlayCount: 0 }); + }); + + it('shares one genre list between tracks that have the same one', () => { + const [a, b] = decodeTrackTable(trackTable(FIXTURE)); + + expect(a!.Genre).toBe(b!.Genre); + }); + + it('decodes an empty library to no rows', () => { + expect(decodeTrackTable(trackTable([]))).toEqual([]); + }); + + it('refuses a table whose columns disagree in length', () => { + const broken = trackTable(FIXTURE); + + (broken as unknown as { album: number[] }).album = [1]; + + expect(() => decodeTrackTable(broken)).toThrow(TrackTableError); + }); + + it('refuses a string index past the table', () => { + const broken = trackTable(FIXTURE); + + (broken as unknown as { album: number[] }).album = [1, 999]; + + expect(() => decodeTrackTable(broken)).toThrow(/out of range/); + }); +}); + +/** The generated Track interface and the Go encoder, as text. */ +const MODELS = Object.values( + import.meta.glob('../../bindings/yellowjacket/backend/library/models.ts', { + eager: true, + query: '?raw', + import: 'default', + }), +)[0] ?? ''; +const ENCODER = Object.values( + import.meta.glob('../../../backend/library/tracktable.go', { + eager: true, + query: '?raw', + import: 'default', + }), +)[0] ?? ''; + +function interfaceKeys(source: string, name: string): string[] { + const body = source.split(`export interface ${name} {`)[1]?.split('\n}')[0] ?? ''; + + return [...body.matchAll(/^\s+"(\w+)"\??:/gm)].map((m) => m[1]!); +} + +describe('the list row and the Go table agree', () => { + const trackKeys = interfaceKeys(MODELS, 'Track'); + const tableKeys = [...ENCODER.matchAll(/json:"(\w+)"/g)].map((m) => m[1]!); + + it('read both declarations', () => { + expect(trackKeys.length).toBeGreaterThan(20); + expect(tableKeys.length).toBeGreaterThan(20); + }); + + it('decodes every Track field the list keeps, and only those', () => { + const decoded = Object.keys(decodeTrackTable(trackTable(FIXTURE))[0]!).sort(); + const omitted = new Set(LIST_TRACK_OMITS); + + expect(decoded).toEqual(trackKeys.filter((k) => !omitted.has(k)).sort()); + }); + + it('has a column for every field it decodes', () => { + // Columns are the field names in lower camel case, except the two + // that are not columns of a field and the length, which is sent as + // the number it encodes. + const columns = new Set(tableKeys.filter((k) => !['strings', 'genreSets'].includes(k))); + const expected = trackKeys + .filter((k) => !(LIST_TRACK_OMITS as readonly string[]).includes(k)) + .map((k) => (k === 'TrackLength' ? 'lengthMs' : k[0]!.toLowerCase() + k.slice(1))) + .map((k) => k.replace(/MBID$/, 'Mbid')); + + expect([...columns].sort()).toEqual(expected.sort()); + }); +});