fix(api): clamp negative offset instead of erroring + clarify duration_sec=0
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -124,8 +124,8 @@ func trackRefFrom(t dbq.Track, albumTitle, artistName string) TrackRef {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// parsePaging reads limit/offset from the query string, applying defaults
|
// parsePaging reads limit/offset from the query string, applying defaults
|
||||||
// and clamping. Returns a 400-worthy error when the values are non-numeric
|
// and clamping. Returns a 400-worthy error when the values are non-numeric;
|
||||||
// or negative; out-of-range values silently clamp (deliberate — UX).
|
// out-of-range values silently clamp (deliberate — UX).
|
||||||
func parsePaging(raw url.Values) (limit, offset int, err error) {
|
func parsePaging(raw url.Values) (limit, offset int, err error) {
|
||||||
const (
|
const (
|
||||||
defLimit = 50
|
defLimit = 50
|
||||||
@@ -151,7 +151,7 @@ func parsePaging(raw url.Values) (limit, offset int, err error) {
|
|||||||
return 0, 0, errBadPaging
|
return 0, 0, errBadPaging
|
||||||
}
|
}
|
||||||
if n < 0 {
|
if n < 0 {
|
||||||
return 0, 0, errBadPaging
|
n = 0
|
||||||
}
|
}
|
||||||
offset = n
|
offset = n
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
package api
|
package api
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"net/url"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
"github.com/jackc/pgx/v5/pgtype"
|
"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)
|
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)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -125,6 +125,7 @@ func (h *handlers) handleGetArtist(w http.ResponseWriter, r *http.Request) {
|
|||||||
writeErr(w, http.StatusInternalServerError, "server_error", "lookup failed")
|
writeErr(w, http.StatusInternalServerError, "server_error", "lookup failed")
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
// durationSec=0: not aggregated for nested album lists per spec data flow.
|
||||||
refs = append(refs, albumRefFrom(a, artist.Name, int(count), 0))
|
refs = append(refs, albumRefFrom(a, artist.Name, int(count), 0))
|
||||||
}
|
}
|
||||||
detail := ArtistDetail{
|
detail := ArtistDetail{
|
||||||
|
|||||||
@@ -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)
|
h.logger.Error("api: count tracks for album failed", "err", cerr)
|
||||||
return nil, 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))
|
items = append(items, albumRefFrom(a, names[uuidToString(a.ArtistID)], int(count), 0))
|
||||||
}
|
}
|
||||||
return items, nil
|
return items, nil
|
||||||
|
|||||||
Reference in New Issue
Block a user