minstrel/internal/api/library_browse_test.go

package api

import (
	"encoding/json"
	"net/http"
	"net/http/httptest"
	"net/url"
	"testing"
)

// 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")
	seedAlbum(t, pool, artist.ID, "Old Record", 1972)
	seedAlbum(t, pool, artist.ID, "Middle Record", 1995)
	seedAlbum(t, pool, artist.ID, "New Record", 2020)
	// An undated album must not appear in ANY year range.
	seedAlbum(t, pool, artist.ID, "Undated Record", 0)

	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")
	seedAlbum(t, pool, artist.ID, "Dated One", 1984)
	seedAlbum(t, pool, artist.ID, "No Date", 0)

	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
}