diff --git a/server/jellyfin/items.go b/server/jellyfin/items.go index 6ed24b056..c3c217cfc 100644 --- a/server/jellyfin/items.go +++ b/server/jellyfin/items.go @@ -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 diff --git a/server/jellyfin/playlists.go b/server/jellyfin/playlists.go index 4d3959a79..9afee5ac3 100644 --- a/server/jellyfin/playlists.go +++ b/server/jellyfin/playlists.go @@ -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) diff --git a/server/nativeapi/missing.go b/server/nativeapi/missing.go index 2b455e622..0ad9bb0cc 100644 --- a/server/nativeapi/missing.go +++ b/server/nativeapi/missing.go @@ -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 { diff --git a/server/nativeapi/playlists.go b/server/nativeapi/playlists.go index ea1cf579b..90b2f9e94 100644 --- a/server/nativeapi/playlists.go +++ b/server/nativeapi/playlists.go @@ -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]) diff --git a/server/subsonic/bookmarks.go b/server/subsonic/bookmarks.go index 337712750..4a7ebaa6c 100644 --- a/server/subsonic/bookmarks.go +++ b/server/subsonic/bookmarks.go @@ -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) diff --git a/server/subsonic/e2e/subsonic_media_annotation_test.go b/server/subsonic/e2e/subsonic_media_annotation_test.go index ec9b070de..74b5238f2 100644 --- a/server/subsonic/e2e/subsonic_media_annotation_test.go +++ b/server/subsonic/e2e/subsonic_media_annotation_test.go @@ -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)) }) }) diff --git a/server/subsonic/e2e/subsonic_sharing_test.go b/server/subsonic/e2e/subsonic_sharing_test.go index 03bf1f80f..0421d96ea 100644 --- a/server/subsonic/e2e/subsonic_sharing_test.go +++ b/server/subsonic/e2e/subsonic_sharing_test.go @@ -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() { diff --git a/server/subsonic/jukebox.go b/server/subsonic/jukebox.go index c4bc643ab..d8bf53360 100644 --- a/server/subsonic/jukebox.go +++ b/server/subsonic/jukebox.go @@ -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)) diff --git a/server/subsonic/library_scanning.go b/server/subsonic/library_scanning.go index e6f64456d..9630425d2 100644 --- a/server/subsonic/library_scanning.go +++ b/server/subsonic/library_scanning.go @@ -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)) diff --git a/server/subsonic/media_annotation.go b/server/subsonic/media_annotation.go index 27170c11b..cfbff3ecb 100644 --- a/server/subsonic/media_annotation.go +++ b/server/subsonic/media_annotation.go @@ -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) { diff --git a/server/subsonic/playlists.go b/server/subsonic/playlists.go index c58fb9ab9..17ba1b2c9 100644 --- a/server/subsonic/playlists.go +++ b/server/subsonic/playlists.go @@ -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 { diff --git a/server/subsonic/sharing.go b/server/subsonic/sharing.go index a9ccfdca4..540ae79d7 100644 --- a/server/subsonic/sharing.go +++ b/server/subsonic/sharing.go @@ -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") diff --git a/utils/req/req.go b/utils/req/req.go index 2757fc3f5..861cca9f7 100644 --- a/utils/req/req.go +++ b/utils/req/req.go @@ -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 { diff --git a/utils/req/req_test.go b/utils/req/req_test.go index d76b3b934..5f9de8483 100644 --- a/utils/req/req_test.go +++ b/utils/req/req_test.go @@ -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()) }) })