From b94b7514f9583dd5113438542437ce186db5851a Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 21 Apr 2026 19:27:21 -0400 Subject: [PATCH] fix(api): clamp negative offset instead of erroring + clarify duration_sec=0 Co-Authored-By: Claude Opus 4.7 --- internal/api/convert.go | 6 +-- internal/api/convert_test.go | 76 ++++++++++++++++++++++++++++++++++++ internal/api/library.go | 1 + internal/api/search.go | 1 + 4 files changed, 81 insertions(+), 3 deletions(-) diff --git a/internal/api/convert.go b/internal/api/convert.go index 339b8dbc..89421420 100644 --- a/internal/api/convert.go +++ b/internal/api/convert.go @@ -124,8 +124,8 @@ func trackRefFrom(t dbq.Track, albumTitle, artistName string) TrackRef { } // 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). +// and clamping. Returns a 400-worthy error when the values are non-numeric; +// out-of-range values silently clamp (deliberate — UX). func parsePaging(raw url.Values) (limit, offset int, err error) { const ( defLimit = 50 @@ -151,7 +151,7 @@ func parsePaging(raw url.Values) (limit, offset int, err error) { return 0, 0, errBadPaging } if n < 0 { - return 0, 0, errBadPaging + n = 0 } offset = n } diff --git a/internal/api/convert_test.go b/internal/api/convert_test.go index 4b319bf5..fb9e5d6b 100644 --- a/internal/api/convert_test.go +++ b/internal/api/convert_test.go @@ -1,6 +1,7 @@ package api import ( + "net/url" "testing" "github.com/jackc/pgx/v5/pgtype" @@ -149,3 +150,78 @@ func TestTrackRefFromNilPositions(t *testing.T) { t.Errorf("nil positions should be 0, got %+v", got) } } + +func TestParsePaging(t *testing.T) { + tests := []struct { + name string + query url.Values + wantLimit int + wantOffset int + wantErr bool + }{ + { + name: "default (empty values)", + query: url.Values{}, + wantLimit: 50, + wantOffset: 0, + wantErr: false, + }, + { + name: "limit=0 clamps to 1", + query: url.Values{"limit": {"0"}}, + wantLimit: 1, + wantOffset: 0, + wantErr: false, + }, + { + name: "limit=500 clamps to 200", + query: url.Values{"limit": {"500"}}, + wantLimit: 200, + wantOffset: 0, + wantErr: false, + }, + { + name: "offset=-5 clamps to 0", + query: url.Values{"offset": {"-5"}}, + wantLimit: 50, + wantOffset: 0, + wantErr: false, + }, + { + name: "limit=abc errors", + query: url.Values{"limit": {"abc"}}, + wantLimit: 0, + wantOffset: 0, + wantErr: true, + }, + { + name: "offset=xyz errors", + query: url.Values{"offset": {"xyz"}}, + wantLimit: 0, + wantOffset: 0, + wantErr: true, + }, + { + name: "normal values", + query: url.Values{"limit": {"25"}, "offset": {"100"}}, + wantLimit: 25, + wantOffset: 100, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotLimit, gotOffset, err := parsePaging(tt.query) + if (err != nil) != tt.wantErr { + t.Errorf("parsePaging() err = %v, wantErr %v", err, tt.wantErr) + return + } + if gotLimit != tt.wantLimit { + t.Errorf("parsePaging() limit = %d, want %d", gotLimit, tt.wantLimit) + } + if gotOffset != tt.wantOffset { + t.Errorf("parsePaging() offset = %d, want %d", gotOffset, tt.wantOffset) + } + }) + } +} diff --git a/internal/api/library.go b/internal/api/library.go index c34b58e1..087db1de 100644 --- a/internal/api/library.go +++ b/internal/api/library.go @@ -125,6 +125,7 @@ func (h *handlers) handleGetArtist(w http.ResponseWriter, r *http.Request) { writeErr(w, http.StatusInternalServerError, "server_error", "lookup failed") return } + // durationSec=0: not aggregated for nested album lists per spec data flow. refs = append(refs, albumRefFrom(a, artist.Name, int(count), 0)) } detail := ArtistDetail{ diff --git a/internal/api/search.go b/internal/api/search.go index bb42f910..b077453e 100644 --- a/internal/api/search.go +++ b/internal/api/search.go @@ -117,6 +117,7 @@ func (h *handlers) resolveAlbumRefs(ctx context.Context, q *dbq.Queries, albums h.logger.Error("api: count tracks for album failed", "err", cerr) return nil, cerr } + // durationSec=0: not aggregated for search album refs per spec data flow. items = append(items, albumRefFrom(a, names[uuidToString(a.ArtistID)], int(count), 0)) } return items, nil