bvandeusen
955a61194e
Filed as a product decision, but the code had already made it: the genre queries filter tracks.missing_since inside their EXISTS, so an album whose every file had gone was ALREADY absent from genre while still listed under its year — where opening it found nothing playable. The two browse axes disagreed, and whichever answer won, one of them had to change. Hiding is the answer. Browsing is how you go looking for something to play, and the rule for that case is to take it out of view; the admin missing-files surface is where absence gets reported, with far more detail than a silent gap in a grid. It also means changing the axis that was inconsistent rather than the one that was already right. All three year queries move together — index, list and count. That is the invariant #367 needed care for at the genre level: if the index groups differently from the filter, a year leads to an empty page, and if the count disagrees with the list then "Load more" promises rows that never arrive. The predicate is "has at least one playable track", which also excludes an album carrying no tracks at all. Same answer for the same reason — nothing to play, nothing to browse to — and it is what genre has always done, since an album with no tracks contributes no genres either. That last part changed two existing tests, which had been seeding trackless albums as a convenience. Their intent (undated albums never appear in a range) is untouched; they now seed a track each, which is what a real album looks like anyway. Two new tests pin the actual behaviour: a fully-missing album leaves the axis while a half-missing one stays, and the count agrees with the filtered list.
430 lines
14 KiB
Go
430 lines
14 KiB
Go
package api
|
|
|
|
import (
|
|
"context"
|
|
"encoding/json"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"net/url"
|
|
"testing"
|
|
|
|
"github.com/jackc/pgx/v5/pgtype"
|
|
|
|
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
|
|
)
|
|
|
|
// parseYearFilter is pure, so this runs in the fast lane rather than waiting
|
|
// on the integration job.
|
|
func TestParseYearFilter(t *testing.T) {
|
|
tests := []struct {
|
|
name string
|
|
query string
|
|
wantActive bool
|
|
wantFrom int32
|
|
wantTo int32
|
|
wantErr error
|
|
}{
|
|
{name: "no params means no filtering", query: "", wantActive: false},
|
|
{
|
|
name: "both bounds", query: "year_from=1990&year_to=1999",
|
|
wantActive: true, wantFrom: 1990, wantTo: 1999,
|
|
},
|
|
{
|
|
// "everything from 2000 onward" shouldn't require the caller to
|
|
// invent an upper bound.
|
|
name: "from only leaves the upper edge open", query: "year_from=2000",
|
|
wantActive: true, wantFrom: 2000, wantTo: maxBrowseYear,
|
|
},
|
|
{
|
|
name: "to only leaves the lower edge open", query: "year_to=1979",
|
|
wantActive: true, wantFrom: minBrowseYear, wantTo: 1979,
|
|
},
|
|
{
|
|
name: "a single year is a degenerate range", query: "year_from=1985&year_to=1985",
|
|
wantActive: true, wantFrom: 1985, wantTo: 1985,
|
|
},
|
|
{name: "non-numeric from", query: "year_from=nineteen", wantErr: errBadYear},
|
|
{name: "non-numeric to", query: "year_to=x", wantErr: errBadYear},
|
|
{
|
|
// Rejected, not silently swapped — reordering would answer a
|
|
// question the caller didn't ask.
|
|
name: "inverted range", query: "year_from=2000&year_to=1990",
|
|
wantErr: errInvertedYearRange,
|
|
},
|
|
{
|
|
name: "whitespace-only values are treated as absent",
|
|
query: "year_from=%20&year_to=%20", wantActive: false,
|
|
},
|
|
}
|
|
for _, tc := range tests {
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
raw, err := url.ParseQuery(tc.query)
|
|
if err != nil {
|
|
t.Fatalf("ParseQuery: %v", err)
|
|
}
|
|
got, gotErr := parseYearFilter(raw)
|
|
if tc.wantErr != nil {
|
|
if gotErr != tc.wantErr {
|
|
t.Fatalf("error = %v, want %v", gotErr, tc.wantErr)
|
|
}
|
|
return
|
|
}
|
|
if gotErr != nil {
|
|
t.Fatalf("unexpected error: %v", gotErr)
|
|
}
|
|
if got.active != tc.wantActive {
|
|
t.Errorf("active = %v, want %v", got.active, tc.wantActive)
|
|
}
|
|
if tc.wantActive && (got.from != tc.wantFrom || got.to != tc.wantTo) {
|
|
t.Errorf("range = [%d,%d], want [%d,%d]",
|
|
got.from, got.to, tc.wantFrom, tc.wantTo)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// The crux of #367: a track tagged "Rock;Pop" must be reachable from BOTH
|
|
// genres. An exact-string match — which is what ListAlbumsByGenre did before
|
|
// this task — makes every multi-genre track invisible from either of its
|
|
// genres, so the index would list a genre whose page is empty.
|
|
func TestListGenres_SplitsMultiGenreTags(t *testing.T) {
|
|
h, pool := testHandlers(t)
|
|
artist := seedArtist(t, pool, "Genre Splitter")
|
|
album := seedAlbum(t, pool, artist.ID, "Split Album", 1995)
|
|
seedTrackWithGenre(t, pool, album.ID, artist.ID, "Both Genres", 1, 200000, "Rock;Pop")
|
|
|
|
req := httptest.NewRequest(http.MethodGet, "/api/library/genres", nil)
|
|
w := httptest.NewRecorder()
|
|
h.handleListGenres(w, req)
|
|
|
|
if w.Code != http.StatusOK {
|
|
t.Fatalf("status = %d, want 200", w.Code)
|
|
}
|
|
var got []genreCount
|
|
if err := json.NewDecoder(w.Body).Decode(&got); err != nil {
|
|
t.Fatalf("decode: %v", err)
|
|
}
|
|
counts := map[string]int{}
|
|
for _, g := range got {
|
|
counts[g.Genre] = g.TrackCount
|
|
}
|
|
for _, want := range []string{"Rock", "Pop"} {
|
|
if counts[want] < 1 {
|
|
t.Errorf("genre %q missing from index (got %v)", want, counts)
|
|
}
|
|
}
|
|
// The undivided string must NOT appear as its own genre.
|
|
if _, ok := counts["Rock;Pop"]; ok {
|
|
t.Error(`"Rock;Pop" surfaced as a single genre — the split didn't happen`)
|
|
}
|
|
}
|
|
|
|
// Splitting produces leading spaces on every fragment after the first, and
|
|
// showing " Pop" as a genre distinct from "Pop" would be a bug. Trimming is a
|
|
// repair for our own splitting, not normalisation of the operator's tags.
|
|
func TestListGenres_TrimsFragmentWhitespace(t *testing.T) {
|
|
h, pool := testHandlers(t)
|
|
artist := seedArtist(t, pool, "Spacey Tags")
|
|
album := seedAlbum(t, pool, artist.ID, "Spacey Album", 2001)
|
|
seedTrackWithGenre(t, pool, album.ID, artist.ID, "Spaced", 1, 200000, "Jazz; Blues ;")
|
|
|
|
req := httptest.NewRequest(http.MethodGet, "/api/library/genres", nil)
|
|
w := httptest.NewRecorder()
|
|
h.handleListGenres(w, req)
|
|
|
|
var got []genreCount
|
|
if err := json.NewDecoder(w.Body).Decode(&got); err != nil {
|
|
t.Fatalf("decode: %v", err)
|
|
}
|
|
seen := map[string]bool{}
|
|
for _, g := range got {
|
|
seen[g.Genre] = true
|
|
if g.Genre == "" {
|
|
t.Error("empty genre in index — a trailing delimiter leaked through")
|
|
}
|
|
}
|
|
for _, want := range []string{"Jazz", "Blues"} {
|
|
if !seen[want] {
|
|
t.Errorf("genre %q missing (got %v)", want, keysOf(seen))
|
|
}
|
|
}
|
|
for _, unwanted := range []string{" Blues", "Blues ", " Blues "} {
|
|
if seen[unwanted] {
|
|
t.Errorf("untrimmed genre %q present", unwanted)
|
|
}
|
|
}
|
|
}
|
|
|
|
// Genre filtering must agree with the index: every genre the index lists has
|
|
// to lead to a non-empty page, which is exactly what the old exact-match
|
|
// query could not guarantee.
|
|
func TestListLibraryAlbums_GenreFilterReachesMultiGenreTracks(t *testing.T) {
|
|
h, pool := testHandlers(t)
|
|
artist := seedArtist(t, pool, "Reachable")
|
|
album := seedAlbum(t, pool, artist.ID, "Reachable Album", 1998)
|
|
seedTrackWithGenre(t, pool, album.ID, artist.ID, "Multi", 1, 200000, "Rock;Pop")
|
|
|
|
for _, genre := range []string{"Rock", "Pop"} {
|
|
t.Run(genre, func(t *testing.T) {
|
|
req := httptest.NewRequest(http.MethodGet,
|
|
"/api/library/albums?genre="+url.QueryEscape(genre), nil)
|
|
w := httptest.NewRecorder()
|
|
h.handleListLibraryAlbums(w, req)
|
|
|
|
if w.Code != http.StatusOK {
|
|
t.Fatalf("status = %d, want 200", w.Code)
|
|
}
|
|
var page Page[AlbumRef]
|
|
if err := json.NewDecoder(w.Body).Decode(&page); err != nil {
|
|
t.Fatalf("decode: %v", err)
|
|
}
|
|
if page.Total < 1 {
|
|
t.Fatalf("total = %d, want >=1 — genre %q led to an empty page",
|
|
page.Total, genre)
|
|
}
|
|
found := false
|
|
for _, a := range page.Items {
|
|
if a.Title == "Reachable Album" {
|
|
found = true
|
|
}
|
|
}
|
|
if !found {
|
|
t.Errorf("seeded album absent from genre %q results", genre)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// A genre containing a slash is why filtering is a query parameter rather
|
|
// than a path segment — "Rock/Pop" cannot survive a path.
|
|
func TestListLibraryAlbums_GenreWithSlashSurvives(t *testing.T) {
|
|
h, pool := testHandlers(t)
|
|
artist := seedArtist(t, pool, "Slashed")
|
|
album := seedAlbum(t, pool, artist.ID, "Slashed Album", 2003)
|
|
seedTrackWithGenre(t, pool, album.ID, artist.ID, "Slashy", 1, 200000, "Rock/Pop")
|
|
|
|
req := httptest.NewRequest(http.MethodGet,
|
|
"/api/library/albums?genre="+url.QueryEscape("Rock/Pop"), nil)
|
|
w := httptest.NewRecorder()
|
|
h.handleListLibraryAlbums(w, req)
|
|
|
|
if w.Code != http.StatusOK {
|
|
t.Fatalf("status = %d, want 200", w.Code)
|
|
}
|
|
var page Page[AlbumRef]
|
|
if err := json.NewDecoder(w.Body).Decode(&page); err != nil {
|
|
t.Fatalf("decode: %v", err)
|
|
}
|
|
if page.Total < 1 {
|
|
t.Errorf(`total = %d, want >=1 for genre "Rock/Pop"`, page.Total)
|
|
}
|
|
}
|
|
|
|
func TestListLibraryAlbums_YearRangeFilter(t *testing.T) {
|
|
h, pool := testHandlers(t)
|
|
artist := seedArtist(t, pool, "Chronology")
|
|
// Each album needs a playable track: since #2702 the year axis lists only
|
|
// albums with something to play, matching what the genre axis already did.
|
|
for _, a := range []struct {
|
|
title string
|
|
year int
|
|
}{
|
|
{"Old Record", 1972},
|
|
{"Middle Record", 1995},
|
|
{"New Record", 2020},
|
|
// An undated album must not appear in ANY year range.
|
|
{"Undated Record", 0},
|
|
} {
|
|
album := seedAlbum(t, pool, artist.ID, a.title, a.year)
|
|
seedTrack(t, pool, album.ID, artist.ID, a.title+" T1", 1, 120_000)
|
|
}
|
|
|
|
titles := func(query string) map[string]bool {
|
|
t.Helper()
|
|
req := httptest.NewRequest(http.MethodGet, "/api/library/albums?"+query, nil)
|
|
w := httptest.NewRecorder()
|
|
h.handleListLibraryAlbums(w, req)
|
|
if w.Code != http.StatusOK {
|
|
t.Fatalf("status = %d for %q, want 200", w.Code, query)
|
|
}
|
|
var page Page[AlbumRef]
|
|
if err := json.NewDecoder(w.Body).Decode(&page); err != nil {
|
|
t.Fatalf("decode: %v", err)
|
|
}
|
|
out := map[string]bool{}
|
|
for _, a := range page.Items {
|
|
out[a.Title] = true
|
|
}
|
|
return out
|
|
}
|
|
|
|
got := titles("year_from=1990&year_to=2000&limit=200")
|
|
if !got["Middle Record"] {
|
|
t.Error("Middle Record (1995) missing from 1990-2000")
|
|
}
|
|
for _, absent := range []string{"Old Record", "New Record", "Undated Record"} {
|
|
if got[absent] {
|
|
t.Errorf("%s present in 1990-2000 range", absent)
|
|
}
|
|
}
|
|
|
|
// Open upper edge.
|
|
got = titles("year_from=1990&limit=200")
|
|
if !got["Middle Record"] || !got["New Record"] {
|
|
t.Error("open-ended year_from should include 1995 and 2020")
|
|
}
|
|
if got["Old Record"] {
|
|
t.Error("Old Record (1972) present in year_from=1990")
|
|
}
|
|
if got["Undated Record"] {
|
|
t.Error("undated album present in an open-ended range")
|
|
}
|
|
}
|
|
|
|
func TestListLibraryAlbums_RejectsGenreAndYearTogether(t *testing.T) {
|
|
h, _ := testHandlers(t)
|
|
req := httptest.NewRequest(http.MethodGet,
|
|
"/api/library/albums?genre=Rock&year_from=1990", nil)
|
|
w := httptest.NewRecorder()
|
|
h.handleListLibraryAlbums(w, req)
|
|
|
|
if w.Code != http.StatusBadRequest {
|
|
t.Errorf("status = %d, want 400 for combined filters", w.Code)
|
|
}
|
|
}
|
|
|
|
func TestListAlbumYears_ExcludesUndatedAlbums(t *testing.T) {
|
|
h, pool := testHandlers(t)
|
|
artist := seedArtist(t, pool, "Years Only")
|
|
dated := seedAlbum(t, pool, artist.ID, "Dated One", 1984)
|
|
seedTrack(t, pool, dated.ID, artist.ID, "Dated T1", 1, 120_000)
|
|
undated := seedAlbum(t, pool, artist.ID, "No Date", 0)
|
|
seedTrack(t, pool, undated.ID, artist.ID, "Undated T1", 1, 120_000)
|
|
|
|
req := httptest.NewRequest(http.MethodGet, "/api/library/years", nil)
|
|
w := httptest.NewRecorder()
|
|
h.handleListAlbumYears(w, req)
|
|
|
|
if w.Code != http.StatusOK {
|
|
t.Fatalf("status = %d, want 200", w.Code)
|
|
}
|
|
var got []yearCount
|
|
if err := json.NewDecoder(w.Body).Decode(&got); err != nil {
|
|
t.Fatalf("decode: %v", err)
|
|
}
|
|
found1984 := false
|
|
for _, y := range got {
|
|
if y.Year == 1984 {
|
|
found1984 = true
|
|
}
|
|
if y.Year == 0 {
|
|
t.Error("year 0 present — undated albums leaked into the index")
|
|
}
|
|
}
|
|
if !found1984 {
|
|
t.Error("1984 missing from the year index")
|
|
}
|
|
// Newest-first ordering, so a picker reads chronologically without the
|
|
// client re-sorting.
|
|
for i := 1; i < len(got); i++ {
|
|
if got[i-1].Year < got[i].Year {
|
|
t.Errorf("years not descending at %d: %d then %d", i, got[i-1].Year, got[i].Year)
|
|
}
|
|
}
|
|
}
|
|
|
|
func keysOf(m map[string]bool) []string {
|
|
out := make([]string, 0, len(m))
|
|
for k := range m {
|
|
out = append(out, k)
|
|
}
|
|
return out
|
|
}
|
|
|
|
// #2702: an album whose every file has gone leaves the year axis, matching
|
|
// what the genre axis already did. Before this the two disagreed — the same
|
|
// album was absent from genre and still listed under its year, where opening
|
|
// it found nothing playable.
|
|
func TestListAlbumYears_ExcludesFullyMissingAlbums(t *testing.T) {
|
|
h, pool := testHandlers(t)
|
|
q := dbq.New(pool)
|
|
artist := seedArtist(t, pool, "Gone Records")
|
|
|
|
// Every file missing — should vanish from the axis entirely.
|
|
dead := seedAlbum(t, pool, artist.ID, "All Gone", 1991)
|
|
deadTrack := seedTrack(t, pool, dead.ID, artist.ID, "Gone A", 1, 120_000)
|
|
// One of two missing — the album still has something to play, so it stays.
|
|
partial := seedAlbum(t, pool, artist.ID, "Half Gone", 1992)
|
|
partialGone := seedTrack(t, pool, partial.ID, artist.ID, "Half A", 1, 120_000)
|
|
seedTrack(t, pool, partial.ID, artist.ID, "Half B", 2, 120_000)
|
|
|
|
if _, err := q.MarkTracksMissing(
|
|
context.Background(), []pgtype.UUID{deadTrack.ID, partialGone.ID},
|
|
); err != nil {
|
|
t.Fatalf("mark missing: %v", err)
|
|
}
|
|
|
|
req := httptest.NewRequest(http.MethodGet, "/api/library/years", nil)
|
|
w := httptest.NewRecorder()
|
|
h.handleListAlbumYears(w, req)
|
|
var years []yearCount
|
|
if err := json.NewDecoder(w.Body).Decode(&years); err != nil {
|
|
t.Fatalf("decode years: %v", err)
|
|
}
|
|
for _, y := range years {
|
|
if y.Year == 1991 {
|
|
t.Error("1991 still on the axis — its only album has no playable files")
|
|
}
|
|
}
|
|
found1992 := false
|
|
for _, y := range years {
|
|
if y.Year == 1992 {
|
|
found1992 = true
|
|
}
|
|
}
|
|
if !found1992 {
|
|
t.Error("1992 missing — its album still has a playable track")
|
|
}
|
|
}
|
|
|
|
// The index, the list and the count must agree. #367 needed care for exactly
|
|
// this reason at the genre level: if they diverge, a year leads to an empty
|
|
// page or "Load more" promises rows that never arrive.
|
|
func TestListLibraryAlbums_YearFilterAndCountAgreeOnMissing(t *testing.T) {
|
|
h, pool := testHandlers(t)
|
|
q := dbq.New(pool)
|
|
artist := seedArtist(t, pool, "Agreement")
|
|
|
|
dead := seedAlbum(t, pool, artist.ID, "Vanished", 2003)
|
|
deadTrack := seedTrack(t, pool, dead.ID, artist.ID, "Vanished A", 1, 120_000)
|
|
alive := seedAlbum(t, pool, artist.ID, "Present", 2003)
|
|
seedTrack(t, pool, alive.ID, artist.ID, "Present A", 1, 120_000)
|
|
|
|
if _, err := q.MarkTracksMissing(
|
|
context.Background(), []pgtype.UUID{deadTrack.ID},
|
|
); err != nil {
|
|
t.Fatalf("mark missing: %v", err)
|
|
}
|
|
|
|
req := httptest.NewRequest(http.MethodGet, "/api/library/albums?year_from=2003&year_to=2003&limit=200", nil)
|
|
w := httptest.NewRecorder()
|
|
h.handleListLibraryAlbums(w, req)
|
|
var page Page[AlbumRef]
|
|
if err := json.NewDecoder(w.Body).Decode(&page); err != nil {
|
|
t.Fatalf("decode: %v", err)
|
|
}
|
|
|
|
for _, a := range page.Items {
|
|
if a.Title == "Vanished" {
|
|
t.Error("a fully-missing album is still listed under its year")
|
|
}
|
|
}
|
|
if len(page.Items) != 1 {
|
|
t.Fatalf("want 1 album listed, got %d", len(page.Items))
|
|
}
|
|
// The count drives paging; a stale one is how "Load more" starts lying.
|
|
if page.Total != 1 {
|
|
t.Errorf("total = %d, want 1 — the count must match the filtered list", page.Total)
|
|
}
|
|
}
|