diff --git a/backend/database/sql/queries/albums.sql b/backend/database/sql/queries/albums.sql index 6ee599c..ec7001f 100644 --- a/backend/database/sql/queries/albums.sql +++ b/backend/database/sql/queries/albums.sql @@ -135,3 +135,49 @@ SELECT ) AS INTEGER) AS known FROM audio_files a WHERE a.album_id = sqlc.arg(album_id); + +-- name: GetAlbumsCompleteness :many +-- The same question as GetAlbumCompleteness, asked of a screenful of +-- albums at once. +-- +-- A card grid cannot afford one query per card, and the answer it wants +-- is the one thing a badge cannot guess: an album held 9 tracks of 12 +-- must show the count, never a bare tick. So this is one query for the +-- whole grid, asked only of the cards that have a local album id. +-- +-- It is two grouping levels rather than the single-album form's +-- correlated subqueries, because a correlated subquery in the FROM +-- clause is not something SQLite will reliably do -- and because the +-- slice may only be spelled once, or sqlc expands it twice with +-- independently numbered placeholders. +-- +-- The per-disc level is where the meaning is, and it is the same +-- meaning as the single-album query. `owned` counts DISTINCT track +-- numbers within a disc (this app detects duplicates, and counting two +-- files of track 3 twice would report a short album as complete), with +-- a file that declares no track number falling back to its own id +-- because three untagged files are three tracks and not one. +-- `expected` takes each disc's declared total and sums over discs, +-- since a total is declared per disc and a release total written on +-- every file of a two-disc album would double its expectation. A disc +-- whose files declared nothing contributes a NULL that SUM ignores, +-- and `known` is what says the album is therefore unanswerable. +WITH per_disc AS ( + SELECT + album_id AS album_id, + COUNT(DISTINCT COALESCE(CAST(track_number AS TEXT), 'f' || id)) + AS owned_on_disc, + MAX(total_tracks) AS disc_total, + SUM(CASE WHEN total_tracks IS NULL THEN 1 ELSE 0 END) + AS discs_without_a_total + FROM audio_files + WHERE album_id IN (sqlc.slice('album_ids')) + GROUP BY album_id, COALESCE(disc_number, 1) +) +SELECT + CAST(album_id AS INTEGER) AS album_id, + CAST(SUM(owned_on_disc) AS INTEGER) AS owned, + CAST(COALESCE(SUM(disc_total), 0) AS INTEGER) AS expected, + CAST(SUM(discs_without_a_total) = 0 AS INTEGER) AS known +FROM per_disc +GROUP BY album_id; diff --git a/backend/database/sql/sqlcgen/albums.sql.go b/backend/database/sql/sqlcgen/albums.sql.go index 709eb20..3acb204 100644 --- a/backend/database/sql/sqlcgen/albums.sql.go +++ b/backend/database/sql/sqlcgen/albums.sql.go @@ -8,6 +8,7 @@ package sqlcgen import ( "context" "database/sql" + "strings" ) const deleteAlbum = `-- name: DeleteAlbum :exec @@ -234,6 +235,98 @@ func (q *Queries) GetAlbumsByArtistName(ctx context.Context, arg GetAlbumsByArti return items, nil } +const getAlbumsCompleteness = `-- name: GetAlbumsCompleteness :many +WITH per_disc AS ( + SELECT + album_id AS album_id, + COUNT(DISTINCT COALESCE(CAST(track_number AS TEXT), 'f' || id)) + AS owned_on_disc, + MAX(total_tracks) AS disc_total, + SUM(CASE WHEN total_tracks IS NULL THEN 1 ELSE 0 END) + AS discs_without_a_total + FROM audio_files + WHERE album_id IN (/*SLICE:album_ids*/?) + GROUP BY album_id, COALESCE(disc_number, 1) +) +SELECT + CAST(album_id AS INTEGER) AS album_id, + CAST(SUM(owned_on_disc) AS INTEGER) AS owned, + CAST(COALESCE(SUM(disc_total), 0) AS INTEGER) AS expected, + CAST(SUM(discs_without_a_total) = 0 AS INTEGER) AS known +FROM per_disc +GROUP BY album_id +` + +type GetAlbumsCompletenessRow struct { + AlbumID int64 + Owned int64 + Expected int64 + Known int64 +} + +// The same question as GetAlbumCompleteness, asked of a screenful of +// albums at once. +// +// A card grid cannot afford one query per card, and the answer it wants +// is the one thing a badge cannot guess: an album held 9 tracks of 12 +// must show the count, never a bare tick. So this is one query for the +// whole grid, asked only of the cards that have a local album id. +// +// It is two grouping levels rather than the single-album form's +// correlated subqueries, because a correlated subquery in the FROM +// clause is not something SQLite will reliably do -- and because the +// slice may only be spelled once, or sqlc expands it twice with +// independently numbered placeholders. +// +// The per-disc level is where the meaning is, and it is the same +// meaning as the single-album query. `owned` counts DISTINCT track +// numbers within a disc (this app detects duplicates, and counting two +// files of track 3 twice would report a short album as complete), with +// a file that declares no track number falling back to its own id +// because three untagged files are three tracks and not one. +// `expected` takes each disc's declared total and sums over discs, +// since a total is declared per disc and a release total written on +// every file of a two-disc album would double its expectation. A disc +// whose files declared nothing contributes a NULL that SUM ignores, +// and `known` is what says the album is therefore unanswerable. +func (q *Queries) GetAlbumsCompleteness(ctx context.Context, albumIds []sql.NullInt64) ([]GetAlbumsCompletenessRow, error) { + query := getAlbumsCompleteness + var queryParams []interface{} + if len(albumIds) > 0 { + for _, v := range albumIds { + queryParams = append(queryParams, v) + } + query = strings.Replace(query, "/*SLICE:album_ids*/?", strings.Repeat(",?", len(albumIds))[1:], 1) + } else { + query = strings.Replace(query, "/*SLICE:album_ids*/?", "NULL", 1) + } + rows, err := q.db.QueryContext(ctx, query, queryParams...) + if err != nil { + return nil, err + } + defer rows.Close() + var items []GetAlbumsCompletenessRow + for rows.Next() { + var i GetAlbumsCompletenessRow + if err := rows.Scan( + &i.AlbumID, + &i.Owned, + &i.Expected, + &i.Known, + ); 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 getAlbumsWithPendingReleaseMBID = `-- name: GetAlbumsWithPendingReleaseMBID :many SELECT id, pending_release_mbid FROM albums WHERE pending_release_mbid IS NOT NULL AND pending_release_mbid != '' diff --git a/backend/library/completeness_test.go b/backend/library/completeness_test.go index 3d7e601..f2a3c30 100644 --- a/backend/library/completeness_test.go +++ b/backend/library/completeness_test.go @@ -232,3 +232,112 @@ func TestGetAlbumCompleteness_EmptyAlbum(t *testing.T) { t.Errorf("empty album reported %+v, want zero and unknown", got) } } + +// The batch and the single-album query are two spellings of one +// question, and the thing worth pinning is that they never disagree. +// +// They are genuinely different SQL — the single-album form is +// correlated subqueries over one album, the batch is two grouping +// levels over a slice — so the risk is not a typo but a drift in +// meaning: a disc's total counted once per file, a duplicate counted +// twice, a disc with no total silently covered by one that had one. +// Every shape the table above cares about is staged here at once, +// because a batch that is only ever asked about one album is not being +// asked the question that can go wrong. +func TestGetAlbumsCompletenessAgreesWithTheSingleAlbumQuery(t *testing.T) { + t.Parallel() + + lib, _ := setupTestLibrary(t) + + shapes := map[int][]track{ + 1: disc(1, 100, 12, 12), + 2: disc(1, 200, 9, 12), + 3: disc(1, 300, 13, 12), + 4: {{recordingID: 400, disc: 1, number: 1}}, + 5: append(disc(1, 500, 10, 10), disc(2, 600, 2, 5)...), + 6: append( + disc(1, 700, 10, 10), + track{recordingID: 750, disc: 2, number: 1}, + ), + 7: append( + disc(1, 800, 5, 6), + track{recordingID: 899, disc: 1, number: 3, total: 6}, + ), + } + + ids := make([]int64, 0, len(shapes)) + + for albumID, tracks := range shapes { + stageAlbum(t, lib, albumID, tracks) + ids = append(ids, albumIDFor(t, lib, albumID)) + } + + batch, err := lib.GetAlbumsCompleteness(ids) + if err != nil { + t.Fatalf("GetAlbumsCompleteness: %v", err) + } + + if len(batch) != len(ids) { + t.Fatalf("batch answered for %d albums, want %d", len(batch), len(ids)) + } + + for _, id := range ids { + one, err := lib.GetAlbumCompleteness(id) + if err != nil { + t.Fatalf("GetAlbumCompleteness(%d): %v", id, err) + } + + if got := batch[id]; got != one { + t.Errorf("album %d: batch says %+v, single says %+v", id, got, one) + } + } +} + +// An album with no files is absent from the batch, not zeroed. +// +// "I have none of this" and "I have no idea" are the third state Known +// exists to keep apart, and a caller reading a missing key gets nothing +// rather than a confident zero it would have to know to distrust. +func TestGetAlbumsCompletenessOmitsAnAlbumWithNoFiles(t *testing.T) { + t.Parallel() + + lib, _ := setupTestLibrary(t) + + stageAlbum(t, lib, 1, disc(1, 100, 3, 3)) + + held := albumIDFor(t, lib, 1) + + got, err := lib.GetAlbumsCompleteness([]int64{held, 4242}) + if err != nil { + t.Fatalf("GetAlbumsCompleteness: %v", err) + } + + if _, ok := got[4242]; ok { + t.Errorf("an album with no files answered %+v, want absent", got[4242]) + } + + if !got[held].Complete { + t.Errorf("held album reported %+v, want complete", got[held]) + } +} + +// A caller with nothing to ask about must not issue a query at all — +// sqlc's empty-slice branch rewrites the placeholder to NULL, which is +// a perfectly valid query returning nothing, so this is about the round +// trip rather than the answer. +func TestGetAlbumsCompletenessAsksNothingForAnEmptyList(t *testing.T) { + t.Parallel() + + lib, _ := setupTestLibrary(t) + + for _, ids := range [][]int64{nil, {}, {0}, {-1, 0}} { + got, err := lib.GetAlbumsCompleteness(ids) + if err != nil { + t.Fatalf("GetAlbumsCompleteness(%v): %v", ids, err) + } + + if len(got) != 0 { + t.Errorf("GetAlbumsCompleteness(%v) = %+v, want empty", ids, got) + } + } +} diff --git a/backend/library/query.go b/backend/library/query.go index b137980..efc05e0 100644 --- a/backend/library/query.go +++ b/backend/library/query.go @@ -244,6 +244,65 @@ func (l *Library) GetAlbumCompleteness(albumID int64) (AlbumCompleteness, error) }, nil } +// GetAlbumsCompleteness answers the same question for a screenful of +// albums in one query, keyed by album id. +// +// A card grid asks this about every card that has a local album behind +// it, and one query per card is how a grid of fifty albums becomes +// fifty round trips. The answer matters there for the reason it +// matters on the album page: an album held 9 tracks of 12 has to show +// the count, and a bare tick saying "in your library" is the complaint +// this whole rule came from. +// +// An album with no row in the result is one with no files, and it is +// absent rather than zeroed — "I have none of this" and "I have no +// idea" are the same third state `Known` exists to keep apart, and a +// caller reading a missing key gets nothing rather than a confident 0. +func (l *Library) GetAlbumsCompleteness( + albumIDs []int64, +) (map[int64]AlbumCompleteness, error) { + out := make(map[int64]AlbumCompleteness, len(albumIDs)) + + if len(albumIDs) == 0 { + return out, nil + } + + keys := make([]sql.NullInt64, 0, len(albumIDs)) + + for _, id := range albumIDs { + if id <= 0 { + continue + } + + keys = append(keys, sql.NullInt64{Int64: id, Valid: true}) + } + + if len(keys) == 0 { + return out, nil + } + + rows, err := l.db.ReadQueries.GetAlbumsCompleteness(l.ctx, keys) + if err != nil { + l.logger.Error("could not get album completeness in batch", + "albums", len(keys), "error", err) + + return nil, fmt.Errorf("could not get album completeness: %w", err) + } + + for _, row := range rows { + known := row.Known != 0 && row.Expected > 0 + + out[row.AlbumID] = AlbumCompleteness{ + Owned: int(row.Owned), + Expected: int(row.Expected), + Known: known, + Complete: known && row.Owned >= row.Expected, + } + } + + return out, nil +} + // GetAlbumTracks returns one album's tracks in disc/track order. func (l *Library) GetAlbumTracks(albumID, libraryID int64) ([]Track, error) { rows, err := l.db.ReadQueries.GetTracksByAlbum( diff --git a/frontend/bindings/yellowjacket/backend/library/library.ts b/frontend/bindings/yellowjacket/backend/library/library.ts index 7d15a11..0591049 100644 --- a/frontend/bindings/yellowjacket/backend/library/library.ts +++ b/frontend/bindings/yellowjacket/backend/library/library.ts @@ -98,6 +98,26 @@ export function GetAlbumsByArtist(artist: string, libraryID: number): $Cancellab return $Call.ByID(1456840721, artist, libraryID); } +/** + * GetAlbumsCompleteness answers the same question for a screenful of + * albums in one query, keyed by album id. + * + * A card grid asks this about every card that has a local album behind + * it, and one query per card is how a grid of fifty albums becomes + * fifty round trips. The answer matters there for the reason it + * matters on the album page: an album held 9 tracks of 12 has to show + * the count, and a bare tick saying "in your library" is the complaint + * this whole rule came from. + * + * An album with no row in the result is one with no files, and it is + * absent rather than zeroed — "I have none of this" and "I have no + * idea" are the same third state `Known` exists to keep apart, and a + * caller reading a missing key gets nothing rather than a confident 0. + */ +export function GetAlbumsCompleteness(albumIDs: number[] | null): $CancellablePromise<{ [_ in `${number}`]?: $models.AlbumCompleteness } | null> { + return $Call.ByID(531636827, albumIDs); +} + /** * GetAllLibrariesWithTrackCounts lists the libraries and their sizes. */