From 8fb2f4e184b7a4af1d4fcf56fbeb24c0420a1f59 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 21 Apr 2026 08:06:03 -0400 Subject: [PATCH] feat(api): GET /api/artists with alpha/newest sort and paging Co-Authored-By: Claude Sonnet 4.6 --- internal/api/convert.go | 42 +++++++++ internal/api/library.go | 59 +++++++++++- internal/api/library_test.go | 169 +++++++++++++++++++++++++++++++++++ 3 files changed, 268 insertions(+), 2 deletions(-) diff --git a/internal/api/convert.go b/internal/api/convert.go index 60635789..339b8dbc 100644 --- a/internal/api/convert.go +++ b/internal/api/convert.go @@ -1,11 +1,18 @@ package api import ( + "errors" + "net/url" + "strconv" + "strings" + "github.com/jackc/pgx/v5/pgtype" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" ) +var errBadPaging = errors.New("invalid limit or offset") + // uuidToString renders a pgtype.UUID as a canonical hyphenated string. // Returns "" when the UUID is invalid (unset). Duplicated from the subsonic // package intentionally — the two packages must not depend on each other. @@ -115,3 +122,38 @@ func trackRefFrom(t dbq.Track, albumTitle, artistName string) TrackRef { } return ref } + +// parsePaging reads limit/offset from the query string, applying defaults +// and clamping. Returns a 400-worthy error when the values are non-numeric +// or negative; out-of-range values silently clamp (deliberate — UX). +func parsePaging(raw url.Values) (limit, offset int, err error) { + const ( + defLimit = 50 + maxLimit = 200 + ) + limit = defLimit + if s := strings.TrimSpace(raw.Get("limit")); s != "" { + n, perr := strconv.Atoi(s) + if perr != nil { + return 0, 0, errBadPaging + } + if n < 1 { + n = 1 + } + if n > maxLimit { + n = maxLimit + } + limit = n + } + if s := strings.TrimSpace(raw.Get("offset")); s != "" { + n, perr := strconv.Atoi(s) + if perr != nil { + return 0, 0, errBadPaging + } + if n < 0 { + return 0, 0, errBadPaging + } + offset = n + } + return limit, offset, nil +} diff --git a/internal/api/library.go b/internal/api/library.go index dd7effc4..120e5fec 100644 --- a/internal/api/library.go +++ b/internal/api/library.go @@ -134,9 +134,64 @@ func (h *handlers) handleGetArtist(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusOK, detail) } -// handleListArtists is implemented in Task 9. +// handleListArtists implements GET /api/artists with two sort modes +// (alpha|newest) and enveloped pagination. album_count per artist is +// computed via an N+1 — acceptable at limit=200. func (h *handlers) handleListArtists(w http.ResponseWriter, r *http.Request) { - writeErr(w, http.StatusNotImplemented, "not_implemented", "stub") + sort := r.URL.Query().Get("sort") + if sort == "" { + sort = "alpha" + } + if sort != "alpha" && sort != "newest" { + writeErr(w, http.StatusBadRequest, "bad_request", "sort must be alpha or newest") + return + } + limit, offset, err := parsePaging(r.URL.Query()) + if err != nil { + writeErr(w, http.StatusBadRequest, "bad_request", err.Error()) + return + } + + q := dbq.New(h.pool) + var artists []dbq.Artist + switch sort { + case "newest": + artists, err = q.ListArtistsNewest(r.Context(), dbq.ListArtistsNewestParams{ + Limit: int32(limit), Offset: int32(offset), + }) + default: // alpha + artists, err = q.ListArtistsAlpha(r.Context(), dbq.ListArtistsAlphaParams{ + Limit: int32(limit), Offset: int32(offset), + }) + } + if err != nil { + h.logger.Error("api: list artists failed", "err", err) + writeErr(w, http.StatusInternalServerError, "server_error", "lookup failed") + return + } + total, err := q.CountArtists(r.Context()) + if err != nil { + h.logger.Error("api: count artists failed", "err", err) + writeErr(w, http.StatusInternalServerError, "server_error", "count failed") + return + } + + items := make([]ArtistRef, 0, len(artists)) + for _, a := range artists { + albums, aerr := q.ListAlbumsByArtist(r.Context(), a.ID) + if aerr != nil { + h.logger.Error("api: list albums for artist failed", "err", aerr) + writeErr(w, http.StatusInternalServerError, "server_error", "lookup failed") + return + } + items = append(items, artistRefFrom(a, len(albums))) + } + writeJSON(w, http.StatusOK, Page[ArtistRef]{ + Items: items, + Total: int(total), + Limit: limit, + Offset: offset, + }) } // handleSearch is implemented in Task 10 (file: search.go). diff --git a/internal/api/library_test.go b/internal/api/library_test.go index 0a48216e..fab8591f 100644 --- a/internal/api/library_test.go +++ b/internal/api/library_test.go @@ -254,3 +254,172 @@ func TestHandleGetArtist_NotFound(t *testing.T) { t.Errorf("status = %d, want 404", w.Code) } } + +func TestHandleListArtists_DefaultAlpha(t *testing.T) { + h, pool := testHandlers(t) + truncateLibrary(t, pool) + seedArtist(t, pool, "Beatles") + seedArtist(t, pool, "ABBA") + seedArtist(t, pool, "Zeppelin") + + req := httptest.NewRequest(http.MethodGet, "/api/artists", nil) + w := httptest.NewRecorder() + newLibraryRouter(h).ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d body = %s", w.Code, w.Body.String()) + } + var got Page[ArtistRef] + if err := json.Unmarshal(w.Body.Bytes(), &got); err != nil { + t.Fatalf("decode: %v", err) + } + if got.Total != 3 { + t.Errorf("total = %d, want 3", got.Total) + } + if got.Limit != 50 || got.Offset != 0 { + t.Errorf("pagination defaults = limit=%d offset=%d, want 50/0", got.Limit, got.Offset) + } + if len(got.Items) != 3 { + t.Fatalf("items len = %d", len(got.Items)) + } + if got.Items[0].Name != "ABBA" || got.Items[2].Name != "Zeppelin" { + t.Errorf("alpha order wrong: %q, %q, %q", got.Items[0].Name, got.Items[1].Name, got.Items[2].Name) + } +} + +func TestHandleListArtists_SortNewest(t *testing.T) { + h, pool := testHandlers(t) + truncateLibrary(t, pool) + // Seeded in this order; created_at DESC means Third first. + seedArtist(t, pool, "First") + seedArtist(t, pool, "Second") + seedArtist(t, pool, "Third") + + req := httptest.NewRequest(http.MethodGet, "/api/artists?sort=newest", nil) + w := httptest.NewRecorder() + newLibraryRouter(h).ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d", w.Code) + } + var got Page[ArtistRef] + _ = json.Unmarshal(w.Body.Bytes(), &got) + if len(got.Items) != 3 { + t.Fatalf("items len = %d", len(got.Items)) + } + if got.Items[0].Name != "Third" || got.Items[2].Name != "First" { + t.Errorf("newest order wrong: %q, %q, %q", got.Items[0].Name, got.Items[1].Name, got.Items[2].Name) + } +} + +func TestHandleListArtists_InvalidSort(t *testing.T) { + h, pool := testHandlers(t) + truncateLibrary(t, pool) + seedArtist(t, pool, "Only") + + req := httptest.NewRequest(http.MethodGet, "/api/artists?sort=garbage", nil) + w := httptest.NewRecorder() + newLibraryRouter(h).ServeHTTP(w, req) + + if w.Code != http.StatusBadRequest { + t.Errorf("status = %d, want 400", w.Code) + } +} + +func TestHandleListArtists_Pagination(t *testing.T) { + h, pool := testHandlers(t) + truncateLibrary(t, pool) + for _, n := range []string{"A", "B", "C", "D", "E"} { + seedArtist(t, pool, n) + } + + req := httptest.NewRequest(http.MethodGet, "/api/artists?limit=2&offset=2", nil) + w := httptest.NewRecorder() + newLibraryRouter(h).ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d", w.Code) + } + var got Page[ArtistRef] + _ = json.Unmarshal(w.Body.Bytes(), &got) + if got.Total != 5 { + t.Errorf("total = %d, want 5", got.Total) + } + if got.Limit != 2 || got.Offset != 2 { + t.Errorf("page = limit=%d offset=%d", got.Limit, got.Offset) + } + if len(got.Items) != 2 { + t.Fatalf("items len = %d", len(got.Items)) + } + if got.Items[0].Name != "C" || got.Items[1].Name != "D" { + t.Errorf("slice = %q, %q", got.Items[0].Name, got.Items[1].Name) + } +} + +func TestHandleListArtists_LimitClamped(t *testing.T) { + h, pool := testHandlers(t) + truncateLibrary(t, pool) + seedArtist(t, pool, "One") + + req := httptest.NewRequest(http.MethodGet, "/api/artists?limit=9999", nil) + w := httptest.NewRecorder() + newLibraryRouter(h).ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d", w.Code) + } + var got Page[ArtistRef] + _ = json.Unmarshal(w.Body.Bytes(), &got) + if got.Limit != 200 { + t.Errorf("limit = %d, want 200 (max clamp)", got.Limit) + } +} + +func TestHandleListArtists_LimitNonNumeric(t *testing.T) { + h, _ := testHandlers(t) + req := httptest.NewRequest(http.MethodGet, "/api/artists?limit=abc", nil) + w := httptest.NewRecorder() + newLibraryRouter(h).ServeHTTP(w, req) + if w.Code != http.StatusBadRequest { + t.Errorf("status = %d, want 400", w.Code) + } +} + +func TestHandleListArtists_AlbumCount(t *testing.T) { + h, pool := testHandlers(t) + truncateLibrary(t, pool) + artist := seedArtist(t, pool, "A") + seedAlbum(t, pool, artist.ID, "X", 0) + seedAlbum(t, pool, artist.ID, "Y", 0) + + req := httptest.NewRequest(http.MethodGet, "/api/artists", nil) + w := httptest.NewRecorder() + newLibraryRouter(h).ServeHTTP(w, req) + + var got Page[ArtistRef] + _ = json.Unmarshal(w.Body.Bytes(), &got) + if len(got.Items) != 1 || got.Items[0].AlbumCount != 2 { + t.Errorf("items = %+v", got.Items) + } +} + +func TestHandleListArtists_EmptyDB(t *testing.T) { + h, pool := testHandlers(t) + truncateLibrary(t, pool) + + req := httptest.NewRequest(http.MethodGet, "/api/artists", nil) + w := httptest.NewRecorder() + newLibraryRouter(h).ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d", w.Code) + } + var got Page[ArtistRef] + _ = json.Unmarshal(w.Body.Bytes(), &got) + if got.Items == nil { + t.Error("items = nil, want []") + } + if got.Total != 0 { + t.Errorf("total = %d", got.Total) + } +}