nik/navidrome
1
0
Fork 0
mirror of https://github.com/navidrome/navidrome.git synced 2026-08-01 07:21:17 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(server): drop redundant error return from req.Strings parsing (#5812)

Browse Source 
* refactor(req): drop redundant error return from Strings

The error from Strings carried no information beyond emptiness — it fired
exactly when the param was absent — and nearly every caller discarded it with
a blank identifier. Strings now just returns the values (empty when absent),
making the common optional-list reads one clean expression.

The few required-param callers (scrobble, createShare) check for emptiness and
return the same Subsonic error code 10 as before; their e2e tests now pin that
code. Ints and Times keep their contracts by synthesizing ErrMissingParam
themselves, so selectedMusicFolderIds is untouched. The jellyfin parseFields
helper is inlined away, since ParseFields(p.Strings("fields")...) now
compiles directly.

* docs(req): clarify Strings returns nil when param is absent
This commit is contained in:
Deluan Quintão 2026-07-18 19:30:04 -04:00 committed by GitHub
parent 1e82f515c4
commit 3158451b8d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
14 changed files with 38 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
server/jellyfin/items.go
View File

@ -224,20 +224,13 @@ type itemsQuery struct {
albumIds []string
}
// parseFields reads the Fields param, accepting both repeated params (Fields=a&Fields=b) and a
// comma-separated value (Fields=a,b), matching real Jellyfin. StringOr would keep only one value.
func parseFields(p *req.Values) dto.Fields {
values, _ := p.Strings("fields")
return dto.ParseFields(values...)
}
// parseItemsQuery also resolves the entity types (inferring them from the parent when
// IncludeItemTypes is absent) and the library scope. Query keys are read lowercase because
// normalizeQueryKeys folded them (Jellyfin binds case-insensitively).
func (api *Router) parseItemsQuery(ctx context.Context, r *http.Request) itemsQuery {
p := req.Params(r)
q := itemsQuery{
fields: parseFields(p),
fields: dto.ParseFields(p.Strings("fields")...),
ids: decodedQueryIDs(r, "ids"),
rawTypes: p.StringOr("includeitemtypes", ""),
search: searchTerm(p),
@ -737,7 +730,7 @@ func (api *Router) itemsByIDs(ctx context.Context, ids []string, fields dto.Fiel
func (api *Router) getItem(w http.ResponseWriter, r *http.Request) {
id := api.resolveItemID(r.Context(), dto.DecodeID(chi.URLParam(r, "itemId")))
fields := parseFields(req.Params(r))
fields := dto.ParseFields(req.Params(r).Strings("fields")...)
if item, ok := api.resolveItemByID(r.Context(), id, fields); ok {
api.ok(w, r, item)
return

2
server/jellyfin/playlists.go
View File

@ -191,7 +191,7 @@ func (api *Router) getPlaylistItems(w http.ResponseWriter, r *http.Request) {
return
}
p := req.Params(r)
fields := parseFields(p)
fields := dto.ParseFields(p.Strings("fields")...)
res, err := api.playlistTrackPage(repo, fields, p.IntOr("startindex", 0), p.IntOr("limit", 0))
if err != nil {
api.internalError(w, r, err)

2
server/nativeapi/missing.go
View File

@ -68,7 +68,7 @@ func deleteMissingFiles(maintenance core.Maintenance) http.HandlerFunc {
ctx := r.Context()
p := req.Params(r)
ids, _ := p.Strings("id")
ids := p.Strings("id")
var err error
if len(ids) == 0 {

2
server/nativeapi/playlists.go
View File

@ -102,7 +102,7 @@ func deleteFromPlaylist(pls playlists.Playlists) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
p := req.Params(r)
playlistId, _ := p.String(":playlistId")
ids, _ := p.Strings("id")
ids := p.Strings("id")
err := pls.RemoveTracks(r.Context(), playlistId, ids)
if len(ids) == 1 && errors.Is(err, model.ErrNotFound) {
log.Warn(r.Context(), "Track not found in playlist", "playlistId", playlistId, "id", ids[0])

4
server/subsonic/bookmarks.go
View File

@ -103,7 +103,7 @@ func (api *Router) GetPlayQueue(r *http.Request) (*responses.Subsonic, error) {
func (api *Router) SavePlayQueue(r *http.Request) (*responses.Subsonic, error) {
p := req.Params(r)
ids, _ := p.Strings("id")
ids := p.Strings("id")
currentID, _ := p.String("current")
position := p.Int64Or("position", 0)
@ -176,7 +176,7 @@ func (api *Router) GetPlayQueueByIndex(r *http.Request) (*responses.Subsonic, er
func (api *Router) SavePlayQueueByIndex(r *http.Request) (*responses.Subsonic, error) {
p := req.Params(r)
ids, _ := p.Strings("id")
ids := p.Strings("id")
position := p.Int64Or("position", 0)

1
server/subsonic/e2e/subsonic_media_annotation_test.go
View File

@ -155,6 +155,7 @@ var _ = Describe("Media Annotation Endpoints", Ordered, func() {
Expect(resp.Status).To(Equal(responses.StatusFailed))
Expect(resp.Error).ToNot(BeNil())
Expect(resp.Error.Code).To(Equal(responses.ErrorMissingParameter))
})
})

1
server/subsonic/e2e/subsonic_sharing_test.go
View File

@ -109,6 +109,7 @@ var _ = Describe("Sharing Endpoints", Ordered, func() {
Expect(resp.Status).To(Equal(responses.StatusFailed))
Expect(resp.Error).ToNot(BeNil())
Expect(resp.Error.Code).To(Equal(responses.ErrorMissingParameter))
})
It("updateShare returns error when id parameter is missing", func() {

4
server/subsonic/jukebox.go
View File

@ -68,7 +68,7 @@ func (api *Router) JukeboxControl(r *http.Request) (*responses.Subsonic, error)
case ActionStatus:
return createResponse(pb.Status(ctx))
case ActionSet:
ids, _ := p.Strings("id")
ids := p.Strings("id")
return createResponse(pb.Set(ctx, ids))
case ActionStart:
return createResponse(pb.Start(ctx))
@ -82,7 +82,7 @@ func (api *Router) JukeboxControl(r *http.Request) (*responses.Subsonic, error)
offset := p.IntOr("offset", 0)
return createResponse(pb.Skip(ctx, index, offset))
case ActionAdd:
ids, _ := p.Strings("id")
ids := p.Strings("id")
return createResponse(pb.Add(ctx, ids))
case ActionClear:
return createResponse(pb.Clear(ctx))

3
server/subsonic/library_scanning.go
View File

@ -45,7 +45,8 @@ func (api *Router) StartScan(r *http.Request) (*responses.Subsonic, error) {
// Parse optional target parameters for selective scanning
var targets []model.ScanTarget
if targetParams, err := p.Strings("target"); err == nil && len(targetParams) > 0 {
if targetParams := p.Strings("target"); len(targetParams) > 0 {
var err error
targets, err = model.ParseTargets(targetParams)
if err != nil {
return nil, newError(responses.ErrorGeneric, fmt.Sprintf("Invalid target parameter: %v", err))

18
server/subsonic/media_annotation.go
View File

@ -71,9 +71,9 @@ func (api *Router) setRating(ctx context.Context, id string, rating int) error {
func (api *Router) Star(r *http.Request) (*responses.Subsonic, error) {
p := req.Params(r)
ids, _ := p.Strings("id")
albumIds, _ := p.Strings("albumId")
artistIds, _ := p.Strings("artistId")
ids := p.Strings("id")
albumIds := p.Strings("albumId")
artistIds := p.Strings("artistId")
if len(ids)+len(albumIds)+len(artistIds) == 0 {
return nil, newError(responses.ErrorMissingParameter, "Required id parameter is missing")
}
@ -90,9 +90,9 @@ func (api *Router) Star(r *http.Request) (*responses.Subsonic, error) {
func (api *Router) Unstar(r *http.Request) (*responses.Subsonic, error) {
p := req.Params(r)
ids, _ := p.Strings("id")
albumIds, _ := p.Strings("albumId")
artistIds, _ := p.Strings("artistId")
ids := p.Strings("id")
albumIds := p.Strings("albumId")
artistIds := p.Strings("artistId")
if len(ids)+len(albumIds)+len(artistIds) == 0 {
return nil, newError(responses.ErrorMissingParameter, "Required id parameter is missing")
}
@ -163,9 +163,9 @@ func (api *Router) setStar(ctx context.Context, star bool, ids ...string) error
func (api *Router) Scrobble(r *http.Request) (*responses.Subsonic, error) {
p := req.Params(r)
ids, err := p.Strings("id")
if err != nil {
return nil, err
ids := p.Strings("id")
if len(ids) == 0 {
return nil, newError(responses.ErrorMissingParameter, "missing parameter: 'id'")
}
times, _ := p.Times("time")
if len(times) > 0 && len(times) != len(ids) {

4
server/subsonic/playlists.go
View File

@ -62,7 +62,7 @@ func (api *Router) getPlaylist(ctx context.Context, id string) (*responses.Subso
func (api *Router) CreatePlaylist(r *http.Request) (*responses.Subsonic, error) {
ctx := r.Context()
p := req.Params(r)
songIds, _ := p.Strings("songId")
songIds := p.Strings("songId")
playlistId, _ := p.String("playlistId")
name, _ := p.String("name")
if playlistId == "" && name == "" {
@ -99,7 +99,7 @@ func (api *Router) UpdatePlaylist(r *http.Request) (*responses.Subsonic, error)
if err != nil {
return nil, err
}
songsToAdd, _ := p.Strings("songIdToAdd")
songsToAdd := p.Strings("songIdToAdd")
songIndexesToRemove, _ := p.Ints("songIndexToRemove")
var plsName *string
if s, err := p.String("name"); err == nil {

6
server/subsonic/sharing.go
View File

@ -52,9 +52,9 @@ func (api *Router) buildShare(r *http.Request, share model.Share) responses.Shar
func (api *Router) CreateShare(r *http.Request) (*responses.Subsonic, error) {
p := req.Params(r)
ids, err := p.Strings("id")
if err != nil {
return nil, err
ids := p.Strings("id")
if len(ids) == 0 {
return nil, newError(responses.ErrorMissingParameter, "missing parameter: 'id'")
}
description, _ := p.String("description")

22
utils/req/req.go
View File

@ -60,12 +60,10 @@ func (r *Values) StringOr(param, def string) string {
return v
}
func (r *Values) Strings(param string) ([]string, error) {
values := r.URL.Query()[param]
if len(values) == 0 {
return nil, newError(ErrMissingParam, param)
}
return values, nil
// Strings returns all occurrences of the param, or a nil (empty) slice when absent. Callers that
// require the param should check for emptiness themselves.
func (r *Values) Strings(param string) []string {
return r.URL.Query()[param]
}
func (r *Values) TimeOr(param string, def time.Time) time.Time {
@ -85,9 +83,9 @@ func (r *Values) TimeOr(param string, def time.Time) time.Time {
}
func (r *Values) Times(param string) ([]time.Time, error) {
pStr, err := r.Strings(param)
if err != nil {
return nil, err
pStr := r.Strings(param)
if len(pStr) == 0 {
return nil, newError(ErrMissingParam, param)
}
times := make([]time.Time, len(pStr))
for i, t := range pStr {
@ -139,9 +137,9 @@ func (r *Values) Int64Or(param string, def int64) int64 {
}
func (r *Values) Ints(param string) ([]int, error) {
pStr, err := r.Strings(param)
if err != nil {
return nil, err
pStr := r.Strings(param)
if len(pStr) == 0 {
return nil, newError(ErrMissingParam, param)
}
ints := make([]int, 0, len(pStr))
for _, s := range pStr {

4
utils/req/req_test.go
View File

@ -60,9 +60,7 @@ var _ = Describe("Request Helpers", func() {
})
It("returns empty array if param does not exist", func() {
v, err := r.Strings("xx")
Expect(err).To(MatchError(req.ErrMissingParam))
Expect(v).To(BeEmpty())
Expect(r.Strings("xx")).To(BeEmpty())
})
})