From dfcc189cffbea00bcbe283fcc88becb713748cce Mon Sep 17 00:00:00 2001 From: Deluan Date: Thu, 21 Dec 2023 17:41:09 -0500 Subject: [PATCH] Replace all `utils.Param*` with `req.Params` --- core/agents/lastfm/auth_router.go | 13 +- server/nativeapi/playlists.go | 15 ++- server/public/handle_downloads.go | 12 +- server/public/handle_images.go | 12 +- server/public/handle_shares.go | 21 +-- server/public/handle_streams.go | 11 +- server/public/public.go | 14 +- server/subsonic/album_lists.go | 31 +++-- server/subsonic/album_lists_test.go | 22 ++-- server/subsonic/api.go | 6 +- server/subsonic/bookmarks.go | 19 +-- server/subsonic/browsing.go | 41 +++--- server/subsonic/helpers.go | 25 ---- server/subsonic/jukebox.go | 17 +-- server/subsonic/library_scanning.go | 5 +- server/subsonic/media_annotation.go | 2 +- server/subsonic/media_retrieval.go | 15 ++- server/subsonic/middlewares.go | 30 ++--- server/subsonic/playlists.go | 33 ++--- server/subsonic/radio.go | 21 +-- server/subsonic/searching.go | 17 +-- server/subsonic/sharing.go | 35 ++--- server/subsonic/stream.go | 20 +-- utils/req/req.go | 32 +++-- utils/req/req_test.go | 27 +++- utils/request_helpers.go | 90 ------------- utils/request_helpers_test.go | 196 ---------------------------- 27 files changed, 269 insertions(+), 513 deletions(-) delete mode 100644 utils/request_helpers.go delete mode 100644 utils/request_helpers_test.go diff --git a/core/agents/lastfm/auth_router.go b/core/agents/lastfm/auth_router.go index 372b5b63..ebcf7bcb 100644 --- a/core/agents/lastfm/auth_router.go +++ b/core/agents/lastfm/auth_router.go @@ -18,7 +18,7 @@ import ( "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/request" "github.com/navidrome/navidrome/server" - "github.com/navidrome/navidrome/utils" + "github.com/navidrome/navidrome/utils/req" ) //go:embed token_received.html @@ -89,13 +89,14 @@ func (s *Router) unlink(w http.ResponseWriter, r *http.Request) { } func (s *Router) callback(w http.ResponseWriter, r *http.Request) { - token := utils.ParamString(r, "token") - if token == "" { + p := req.Params(r) + token, err := p.String("token") + if err != nil { _ = rest.RespondWithError(w, http.StatusBadRequest, "token not received") return } - uid := utils.ParamString(r, "uid") - if uid == "" { + uid, err := p.String("uid") + if err != nil { _ = rest.RespondWithError(w, http.StatusBadRequest, "uid not received") return } @@ -103,7 +104,7 @@ func (s *Router) callback(w http.ResponseWriter, r *http.Request) { // Need to add user to context, as this is a non-authenticated endpoint, so it does not // automatically contain any user info ctx := request.WithUser(r.Context(), model.User{ID: uid}) - err := s.fetchSessionKey(ctx, uid, token) + err = s.fetchSessionKey(ctx, uid, token) if err != nil { w.Header().Set("Content-Type", "text/plain; charset=utf-8") w.WriteHeader(http.StatusBadRequest) diff --git a/server/nativeapi/playlists.go b/server/nativeapi/playlists.go index 9abf80e4..8f625aba 100644 --- a/server/nativeapi/playlists.go +++ b/server/nativeapi/playlists.go @@ -14,7 +14,7 @@ import ( "github.com/navidrome/navidrome/core" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" - "github.com/navidrome/navidrome/utils" + "github.com/navidrome/navidrome/utils/req" ) type restHandler = func(rest.RepositoryConstructor, ...rest.Logger) http.HandlerFunc @@ -95,8 +95,9 @@ func handleExportPlaylist(ds model.DataStore) http.HandlerFunc { func deleteFromPlaylist(ds model.DataStore) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - playlistId := utils.ParamString(r, ":playlistId") - ids := r.URL.Query()["id"] + p := req.Params(r) + playlistId, _ := p.String(":playlistId") + ids, _ := p.Strings("id") err := ds.WithTx(func(tx model.DataStore) error { tracksRepo := tx.Playlist(r.Context()).Tracks(playlistId, true) return tracksRepo.Delete(ids...) @@ -139,7 +140,8 @@ func addToPlaylist(ds model.DataStore) http.HandlerFunc { } return func(w http.ResponseWriter, r *http.Request) { - playlistId := utils.ParamString(r, ":playlistId") + p := req.Params(r) + playlistId, _ := p.String(":playlistId") var payload addTracksPayload err := json.NewDecoder(r.Body).Decode(&payload) if err != nil { @@ -183,8 +185,9 @@ func reorderItem(ds model.DataStore) http.HandlerFunc { } return func(w http.ResponseWriter, r *http.Request) { - playlistId := utils.ParamString(r, ":playlistId") - id := utils.ParamInt(r, ":id", 0) + p := req.Params(r) + playlistId, _ := p.String(":playlistId") + id := p.IntOr(":id", 0) if id == 0 { http.Error(w, "invalid id", http.StatusBadRequest) return diff --git a/server/public/handle_downloads.go b/server/public/handle_downloads.go index c6cf5a52..6aa35c34 100644 --- a/server/public/handle_downloads.go +++ b/server/public/handle_downloads.go @@ -2,15 +2,17 @@ package public import ( "net/http" + + "github.com/navidrome/navidrome/utils/req" ) -func (p *Router) handleDownloads(w http.ResponseWriter, r *http.Request) { - id := r.URL.Query().Get(":id") - if id == "" { - http.Error(w, "invalid id", http.StatusBadRequest) +func (pub *Router) handleDownloads(w http.ResponseWriter, r *http.Request) { + id, err := req.Params(r).String(":id") + if err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) return } - err := p.archiver.ZipShare(r.Context(), id, w) + err = pub.archiver.ZipShare(r.Context(), id, w) checkShareError(r.Context(), w, err, id) } diff --git a/server/public/handle_images.go b/server/public/handle_images.go index 7d73c8b9..2e6ee31a 100644 --- a/server/public/handle_images.go +++ b/server/public/handle_images.go @@ -10,10 +10,10 @@ import ( "github.com/navidrome/navidrome/core/artwork" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" - "github.com/navidrome/navidrome/utils" + "github.com/navidrome/navidrome/utils/req" ) -func (p *Router) handleImages(w http.ResponseWriter, r *http.Request) { +func (pub *Router) handleImages(w http.ResponseWriter, r *http.Request) { // If context is already canceled, discard request without further processing if r.Context().Err() != nil { return @@ -21,7 +21,9 @@ func (p *Router) handleImages(w http.ResponseWriter, r *http.Request) { ctx, cancel := context.WithTimeout(r.Context(), 10*time.Second) defer cancel() - id := r.URL.Query().Get(":id") + + p := req.Params(r) + id, _ := p.String(":id") if id == "" { http.Error(w, "invalid id", http.StatusBadRequest) return @@ -32,9 +34,9 @@ func (p *Router) handleImages(w http.ResponseWriter, r *http.Request) { http.Error(w, err.Error(), http.StatusBadRequest) return } - size := utils.ParamInt(r, "size", 0) + size := p.IntOr("size", 0) - imgReader, lastUpdate, err := p.artwork.Get(ctx, artId, size) + imgReader, lastUpdate, err := pub.artwork.Get(ctx, artId, size) switch { case errors.Is(err, context.Canceled): return diff --git a/server/public/handle_shares.go b/server/public/handle_shares.go index aa09cfa1..a4fa99d8 100644 --- a/server/public/handle_shares.go +++ b/server/public/handle_shares.go @@ -10,31 +10,32 @@ import ( "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/server" "github.com/navidrome/navidrome/ui" + "github.com/navidrome/navidrome/utils/req" ) -func (p *Router) handleShares(w http.ResponseWriter, r *http.Request) { - id := r.URL.Query().Get(":id") - if id == "" { - http.Error(w, "invalid id", http.StatusBadRequest) +func (pub *Router) handleShares(w http.ResponseWriter, r *http.Request) { + id, err := req.Params(r).String(":id") + if err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) return } // If requested file is a UI asset, just serve it - _, err := ui.BuildAssets().Open(id) + _, err = ui.BuildAssets().Open(id) if err == nil { - p.assetsHandler.ServeHTTP(w, r) + pub.assetsHandler.ServeHTTP(w, r) return } // If it is not, consider it a share ID - s, err := p.share.Load(r.Context(), id) + s, err := pub.share.Load(r.Context(), id) if err != nil { checkShareError(r.Context(), w, err, id) return } - s = p.mapShareInfo(r, *s) - server.IndexWithShare(p.ds, ui.BuildAssets(), s)(w, r) + s = pub.mapShareInfo(r, *s) + server.IndexWithShare(pub.ds, ui.BuildAssets(), s)(w, r) } func checkShareError(ctx context.Context, w http.ResponseWriter, err error, id string) { @@ -54,7 +55,7 @@ func checkShareError(ctx context.Context, w http.ResponseWriter, err error, id s } } -func (p *Router) mapShareInfo(r *http.Request, s model.Share) *model.Share { +func (pub *Router) mapShareInfo(r *http.Request, s model.Share) *model.Share { s.URL = ShareURL(r, s.ID) s.ImageURL = ImageURL(r, s.CoverArtID(), consts.UICoverArtSize) for i := range s.Tracks { diff --git a/server/public/handle_streams.go b/server/public/handle_streams.go index e9f60d7a..c36e4b44 100644 --- a/server/public/handle_streams.go +++ b/server/public/handle_streams.go @@ -10,12 +10,13 @@ import ( "github.com/lestrrat-go/jwx/v2/jwt" "github.com/navidrome/navidrome/core/auth" "github.com/navidrome/navidrome/log" - "github.com/navidrome/navidrome/utils" + "github.com/navidrome/navidrome/utils/req" ) -func (p *Router) handleStream(w http.ResponseWriter, r *http.Request) { +func (pub *Router) handleStream(w http.ResponseWriter, r *http.Request) { ctx := r.Context() - tokenId := r.URL.Query().Get(":id") + p := req.Params(r) + tokenId, _ := p.String(":id") info, err := decodeStreamInfo(tokenId) if err != nil { log.Error(ctx, "Error parsing shared stream info", err) @@ -23,7 +24,7 @@ func (p *Router) handleStream(w http.ResponseWriter, r *http.Request) { return } - stream, err := p.streamer.NewStream(ctx, info.id, info.format, info.bitrate, 0) + stream, err := pub.streamer.NewStream(ctx, info.id, info.format, info.bitrate, 0) if err != nil { log.Error(ctx, "Error starting shared stream", err) http.Error(w, "invalid request", http.StatusInternalServerError) @@ -46,7 +47,7 @@ func (p *Router) handleStream(w http.ResponseWriter, r *http.Request) { w.Header().Set("Accept-Ranges", "none") w.Header().Set("Content-Type", stream.ContentType()) - estimateContentLength := utils.ParamBool(r, "estimateContentLength", false) + estimateContentLength := p.BoolOr("estimateContentLength", false) // if Client requests the estimated content-length, send it if estimateContentLength { diff --git a/server/public/public.go b/server/public/public.go index 5ead6b32..825e7633 100644 --- a/server/public/public.go +++ b/server/public/public.go @@ -35,7 +35,7 @@ func New(ds model.DataStore, artwork artwork.Artwork, streamer core.MediaStreame return p } -func (p *Router) routes() http.Handler { +func (pub *Router) routes() http.Handler { r := chi.NewRouter() r.Group(func(r chi.Router) { @@ -48,16 +48,16 @@ func (p *Router) routes() http.Handler { r.Use(middleware.ThrottleBacklog(conf.Server.DevArtworkMaxRequests, conf.Server.DevArtworkThrottleBacklogLimit, conf.Server.DevArtworkThrottleBacklogTimeout)) } - r.HandleFunc("/img/{id}", p.handleImages) + r.HandleFunc("/img/{id}", pub.handleImages) }) if conf.Server.EnableSharing { - r.HandleFunc("/s/{id}", p.handleStream) + r.HandleFunc("/s/{id}", pub.handleStream) if conf.Server.EnableDownloads { - r.HandleFunc("/d/{id}", p.handleDownloads) + r.HandleFunc("/d/{id}", pub.handleDownloads) } - r.HandleFunc("/{id}", p.handleShares) - r.HandleFunc("/", p.handleShares) - r.Handle("/*", p.assetsHandler) + r.HandleFunc("/{id}", pub.handleShares) + r.HandleFunc("/", pub.handleShares) + r.Handle("/*", pub.assetsHandler) } }) return r diff --git a/server/subsonic/album_lists.go b/server/subsonic/album_lists.go index 6fdabff1..792553d4 100644 --- a/server/subsonic/album_lists.go +++ b/server/subsonic/album_lists.go @@ -10,12 +10,13 @@ import ( "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/server/subsonic/filter" "github.com/navidrome/navidrome/server/subsonic/responses" - "github.com/navidrome/navidrome/utils" "github.com/navidrome/navidrome/utils/number" + "github.com/navidrome/navidrome/utils/req" ) func (api *Router) getAlbumList(r *http.Request) (model.Albums, int64, error) { - typ, err := requiredParamString(r, "type") + p := req.Params(r) + typ, err := p.String("type") if err != nil { return nil, 0, err } @@ -39,17 +40,17 @@ func (api *Router) getAlbumList(r *http.Request) (model.Albums, int64, error) { case "highest": opts = filter.AlbumsByRating() case "byGenre": - genre, err := requiredParamString(r, "genre") + genre, err := p.String("genre") if err != nil { return nil, 0, err } opts = filter.AlbumsByGenre(genre) case "byYear": - fromYear, err := requiredParamInt(r, "fromYear") + fromYear, err := p.Int("fromYear") if err != nil { return nil, 0, err } - toYear, err := requiredParamInt(r, "toYear") + toYear, err := p.Int("toYear") if err != nil { return nil, 0, err } @@ -59,8 +60,8 @@ func (api *Router) getAlbumList(r *http.Request) (model.Albums, int64, error) { return nil, 0, newError(responses.ErrorGeneric, "type '%s' not implemented", typ) } - opts.Offset = utils.ParamInt(r, "offset", 0) - opts.Max = number.Min(utils.ParamInt(r, "size", 10), 500) + opts.Offset = p.IntOr("offset", 0) + opts.Max = number.Min(p.IntOr("size", 10), 500) albums, err := api.ds.Album(r.Context()).GetAllWithoutGenres(opts) if err != nil { @@ -163,10 +164,11 @@ func (api *Router) GetNowPlaying(r *http.Request) (*responses.Subsonic, error) { } func (api *Router) GetRandomSongs(r *http.Request) (*responses.Subsonic, error) { - size := number.Min(utils.ParamInt(r, "size", 10), 500) - genre := utils.ParamString(r, "genre") - fromYear := utils.ParamInt(r, "fromYear", 0) - toYear := utils.ParamInt(r, "toYear", 0) + p := req.Params(r) + size := number.Min(p.IntOr("size", 10), 500) + genre, _ := p.String("genre") + fromYear := p.IntOr("fromYear", 0) + toYear := p.IntOr("toYear", 0) songs, err := api.getSongs(r.Context(), 0, size, filter.SongsByRandom(genre, fromYear, toYear)) if err != nil { @@ -181,9 +183,10 @@ func (api *Router) GetRandomSongs(r *http.Request) (*responses.Subsonic, error) } func (api *Router) GetSongsByGenre(r *http.Request) (*responses.Subsonic, error) { - count := number.Min(utils.ParamInt(r, "count", 10), 500) - offset := utils.ParamInt(r, "offset", 0) - genre := utils.ParamString(r, "genre") + p := req.Params(r) + count := number.Min(p.IntOr("count", 10), 500) + offset := p.IntOr("offset", 0) + genre, _ := p.String("genre") songs, err := api.getSongs(r.Context(), offset, count, filter.SongsByGenre(genre)) if err != nil { diff --git a/server/subsonic/album_lists_test.go b/server/subsonic/album_lists_test.go index 85a99448..88f2e0ac 100644 --- a/server/subsonic/album_lists_test.go +++ b/server/subsonic/album_lists_test.go @@ -6,6 +6,7 @@ import ( "net/http/httptest" "github.com/navidrome/navidrome/server/subsonic/responses" + "github.com/navidrome/navidrome/utils/req" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" @@ -36,7 +37,7 @@ var _ = Describe("Album Lists", func() { }) resp, err := router.GetAlbumList(w, r) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(resp.AlbumList.Album[0].Id).To(Equal("1")) Expect(resp.AlbumList.Album[1].Id).To(Equal("2")) Expect(w.Header().Get("x-total-count")).To(Equal("2")) @@ -47,12 +48,8 @@ var _ = Describe("Album Lists", func() { It("should fail if missing type parameter", func() { r := newGetRequest() _, err := router.GetAlbumList(w, r) - var subErr subError - isSubError := errors.As(err, &subErr) - Expect(isSubError).To(BeTrue()) - Expect(subErr).To(MatchError("required 'type' parameter is missing")) - Expect(subErr.code).To(Equal(responses.ErrorMissingParameter)) + Expect(err).To(MatchError(req.ErrMissingParam)) }) It("should return error if call fails", func() { @@ -61,7 +58,7 @@ var _ = Describe("Album Lists", func() { _, err := router.GetAlbumList(w, r) - Expect(err).ToNot(BeNil()) + Expect(err).To(MatchError(errSubsonic)) var subErr subError errors.As(err, &subErr) Expect(subErr.code).To(Equal(responses.ErrorGeneric)) @@ -76,7 +73,7 @@ var _ = Describe("Album Lists", func() { }) resp, err := router.GetAlbumList2(w, r) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(resp.AlbumList2.Album[0].Id).To(Equal("1")) Expect(resp.AlbumList2.Album[1].Id).To(Equal("2")) Expect(w.Header().Get("x-total-count")).To(Equal("2")) @@ -86,13 +83,10 @@ var _ = Describe("Album Lists", func() { It("should fail if missing type parameter", func() { r := newGetRequest() + _, err := router.GetAlbumList2(w, r) - var subErr subError - errors.As(err, &subErr) - - Expect(subErr).To(MatchError("required 'type' parameter is missing")) - Expect(subErr.code).To(Equal(responses.ErrorMissingParameter)) + Expect(err).To(MatchError(req.ErrMissingParam)) }) It("should return error if call fails", func() { @@ -101,9 +95,9 @@ var _ = Describe("Album Lists", func() { _, err := router.GetAlbumList2(w, r) + Expect(err).To(MatchError(errSubsonic)) var subErr subError errors.As(err, &subErr) - Expect(subErr).ToNot(BeNil()) Expect(subErr.code).To(Equal(responses.ErrorGeneric)) }) }) diff --git a/server/subsonic/api.go b/server/subsonic/api.go index 467d1d7e..64022491 100644 --- a/server/subsonic/api.go +++ b/server/subsonic/api.go @@ -18,7 +18,6 @@ import ( "github.com/navidrome/navidrome/scanner" "github.com/navidrome/navidrome/server/events" "github.com/navidrome/navidrome/server/subsonic/responses" - "github.com/navidrome/navidrome/utils" "github.com/navidrome/navidrome/utils/req" ) @@ -283,7 +282,8 @@ func sendError(w http.ResponseWriter, r *http.Request, err error) { } func sendResponse(w http.ResponseWriter, r *http.Request, payload *responses.Subsonic) { - f := utils.ParamString(r, "f") + p := req.Params(r) + f, _ := p.String("f") var response []byte switch f { case "json": @@ -292,7 +292,7 @@ func sendResponse(w http.ResponseWriter, r *http.Request, payload *responses.Sub response, _ = json.Marshal(wrapper) case "jsonp": w.Header().Set("Content-Type", "application/javascript") - callback := utils.ParamString(r, "callback") + callback, _ := p.String("callback") wrapper := &responses.JsonWrapper{Subsonic: *payload} data, _ := json.Marshal(wrapper) response = []byte(fmt.Sprintf("%s(%s)", callback, data)) diff --git a/server/subsonic/bookmarks.go b/server/subsonic/bookmarks.go index 86d5d768..42fe9e17 100644 --- a/server/subsonic/bookmarks.go +++ b/server/subsonic/bookmarks.go @@ -7,7 +7,7 @@ import ( "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/request" "github.com/navidrome/navidrome/server/subsonic/responses" - "github.com/navidrome/navidrome/utils" + "github.com/navidrome/navidrome/utils/req" ) func (api *Router) GetBookmarks(r *http.Request) (*responses.Subsonic, error) { @@ -36,13 +36,14 @@ func (api *Router) GetBookmarks(r *http.Request) (*responses.Subsonic, error) { } func (api *Router) CreateBookmark(r *http.Request) (*responses.Subsonic, error) { - id, err := requiredParamString(r, "id") + p := req.Params(r) + id, err := p.String("id") if err != nil { return nil, err } - comment := utils.ParamString(r, "comment") - position := utils.ParamInt(r, "position", int64(0)) + comment, _ := p.String("comment") + position := p.Int64Or("position", 0) repo := api.ds.MediaFile(r.Context()) err = repo.AddBookmark(id, comment, position) @@ -53,7 +54,8 @@ func (api *Router) CreateBookmark(r *http.Request) (*responses.Subsonic, error) } func (api *Router) DeleteBookmark(r *http.Request) (*responses.Subsonic, error) { - id, err := requiredParamString(r, "id") + p := req.Params(r) + id, err := p.String("id") if err != nil { return nil, err } @@ -88,13 +90,14 @@ func (api *Router) GetPlayQueue(r *http.Request) (*responses.Subsonic, error) { } func (api *Router) SavePlayQueue(r *http.Request) (*responses.Subsonic, error) { - ids, err := requiredParamStrings(r, "id") + p := req.Params(r) + ids, err := p.Strings("id") if err != nil { return nil, err } - current := utils.ParamString(r, "current") - position := utils.ParamInt(r, "position", int64(0)) + current, _ := p.String("current") + position := p.Int64Or("position", 0) user, _ := request.UserFrom(r.Context()) client, _ := request.ClientFrom(r.Context()) diff --git a/server/subsonic/browsing.go b/server/subsonic/browsing.go index b3371579..88afa6d6 100644 --- a/server/subsonic/browsing.go +++ b/server/subsonic/browsing.go @@ -14,6 +14,7 @@ import ( "github.com/navidrome/navidrome/server/subsonic/filter" "github.com/navidrome/navidrome/server/subsonic/responses" "github.com/navidrome/navidrome/utils" + "github.com/navidrome/navidrome/utils/req" ) func (api *Router) GetMusicFolders(r *http.Request) (*responses.Subsonic, error) { @@ -67,8 +68,9 @@ func (api *Router) getArtistIndex(r *http.Request, mediaFolderId int, ifModified } func (api *Router) GetIndexes(r *http.Request) (*responses.Subsonic, error) { - musicFolderId := utils.ParamInt(r, "musicFolderId", 0) - ifModifiedSince := utils.ParamTime(r, "ifModifiedSince", time.Time{}) + p := req.Params(r) + musicFolderId := p.IntOr("musicFolderId", 0) + ifModifiedSince := p.TimeOr("ifModifiedSince", time.Time{}) res, err := api.getArtistIndex(r, musicFolderId, ifModifiedSince) if err != nil { @@ -81,7 +83,8 @@ func (api *Router) GetIndexes(r *http.Request) (*responses.Subsonic, error) { } func (api *Router) GetArtists(r *http.Request) (*responses.Subsonic, error) { - musicFolderId := utils.ParamInt(r, "musicFolderId", 0) + p := req.Params(r) + musicFolderId := p.IntOr("musicFolderId", 0) res, err := api.getArtistIndex(r, musicFolderId, time.Time{}) if err != nil { return nil, err @@ -93,7 +96,8 @@ func (api *Router) GetArtists(r *http.Request) (*responses.Subsonic, error) { } func (api *Router) GetMusicDirectory(r *http.Request) (*responses.Subsonic, error) { - id := utils.ParamString(r, "id") + p := req.Params(r) + id, _ := p.String("id") ctx := r.Context() entity, err := model.GetEntityByID(ctx, api.ds, id) @@ -129,7 +133,8 @@ func (api *Router) GetMusicDirectory(r *http.Request) (*responses.Subsonic, erro } func (api *Router) GetArtist(r *http.Request) (*responses.Subsonic, error) { - id := utils.ParamString(r, "id") + p := req.Params(r) + id, _ := p.String("id") ctx := r.Context() artist, err := api.ds.Artist(ctx).Get(id) @@ -151,7 +156,8 @@ func (api *Router) GetArtist(r *http.Request) (*responses.Subsonic, error) { } func (api *Router) GetAlbum(r *http.Request) (*responses.Subsonic, error) { - id := utils.ParamString(r, "id") + p := req.Params(r) + id, _ := p.String("id") ctx := r.Context() @@ -177,7 +183,8 @@ func (api *Router) GetAlbum(r *http.Request) (*responses.Subsonic, error) { } func (api *Router) GetAlbumInfo(r *http.Request) (*responses.Subsonic, error) { - id, err := requiredParamString(r, "id") + p := req.Params(r) + id, err := p.String("id") ctx := r.Context() if err != nil { @@ -204,7 +211,8 @@ func (api *Router) GetAlbumInfo(r *http.Request) (*responses.Subsonic, error) { } func (api *Router) GetSong(r *http.Request) (*responses.Subsonic, error) { - id := utils.ParamString(r, "id") + p := req.Params(r) + id, _ := p.String("id") ctx := r.Context() mf, err := api.ds.MediaFile(ctx).Get(id) @@ -243,12 +251,13 @@ func (api *Router) GetGenres(r *http.Request) (*responses.Subsonic, error) { func (api *Router) GetArtistInfo(r *http.Request) (*responses.Subsonic, error) { ctx := r.Context() - id, err := requiredParamString(r, "id") + p := req.Params(r) + id, err := p.String("id") if err != nil { return nil, err } - count := utils.ParamInt(r, "count", 20) - includeNotPresent := utils.ParamBool(r, "includeNotPresent", false) + count := p.IntOr("count", 20) + includeNotPresent := p.BoolOr("includeNotPresent", false) artist, err := api.externalMetadata.UpdateArtistInfo(ctx, id, count, includeNotPresent) if err != nil { @@ -295,11 +304,12 @@ func (api *Router) GetArtistInfo2(r *http.Request) (*responses.Subsonic, error) func (api *Router) GetSimilarSongs(r *http.Request) (*responses.Subsonic, error) { ctx := r.Context() - id, err := requiredParamString(r, "id") + p := req.Params(r) + id, err := p.String("id") if err != nil { return nil, err } - count := utils.ParamInt(r, "count", 50) + count := p.IntOr("count", 50) songs, err := api.externalMetadata.SimilarSongs(ctx, id, count) if err != nil { @@ -328,11 +338,12 @@ func (api *Router) GetSimilarSongs2(r *http.Request) (*responses.Subsonic, error func (api *Router) GetTopSongs(r *http.Request) (*responses.Subsonic, error) { ctx := r.Context() - artist, err := requiredParamString(r, "artist") + p := req.Params(r) + artist, err := p.String("artist") if err != nil { return nil, err } - count := utils.ParamInt(r, "count", 50) + count := p.IntOr("count", 50) songs, err := api.externalMetadata.TopSongs(ctx, artist, count) if err != nil { diff --git a/server/subsonic/helpers.go b/server/subsonic/helpers.go index d006bd7a..706a12c8 100644 --- a/server/subsonic/helpers.go +++ b/server/subsonic/helpers.go @@ -14,7 +14,6 @@ import ( "github.com/navidrome/navidrome/model/request" "github.com/navidrome/navidrome/server/public" "github.com/navidrome/navidrome/server/subsonic/responses" - "github.com/navidrome/navidrome/utils" ) func newResponse() *responses.Subsonic { @@ -27,30 +26,6 @@ func newResponse() *responses.Subsonic { } } -func requiredParamString(r *http.Request, param string) (string, error) { - p := utils.ParamString(r, param) - if p == "" { - return "", newError(responses.ErrorMissingParameter, "required '%s' parameter is missing", param) - } - return p, nil -} - -func requiredParamStrings(r *http.Request, param string) ([]string, error) { - ps := utils.ParamStrings(r, param) - if len(ps) == 0 { - return nil, newError(responses.ErrorMissingParameter, "required '%s' parameter is missing", param) - } - return ps, nil -} - -func requiredParamInt(r *http.Request, param string) (int, error) { - p := utils.ParamString(r, param) - if p == "" { - return 0, newError(responses.ErrorMissingParameter, "required '%s' parameter is missing", param) - } - return utils.ParamInt(r, param, 0), nil -} - type subError struct { code int messages []interface{} diff --git a/server/subsonic/jukebox.go b/server/subsonic/jukebox.go index 2a33fa1e..b61d6429 100644 --- a/server/subsonic/jukebox.go +++ b/server/subsonic/jukebox.go @@ -7,7 +7,7 @@ import ( "github.com/navidrome/navidrome/core/playback" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/server/subsonic/responses" - "github.com/navidrome/navidrome/utils" + "github.com/navidrome/navidrome/utils/req" ) const ( @@ -27,8 +27,9 @@ const ( func (api *Router) JukeboxControl(r *http.Request) (*responses.Subsonic, error) { ctx := r.Context() user := getUser(ctx) + p := req.Params(r) - actionString, err := requiredParamString(r, "action") + actionString, err := p.String("action") if err != nil { return nil, err } @@ -58,31 +59,31 @@ func (api *Router) JukeboxControl(r *http.Request) (*responses.Subsonic, error) case ActionStatus: return createResponse(pb.Status(ctx)) case ActionSet: - ids := utils.ParamStrings(r, "id") + ids, _ := p.Strings("id") return createResponse(pb.Set(ctx, ids)) case ActionStart: return createResponse(pb.Start(ctx)) case ActionStop: return createResponse(pb.Stop(ctx)) case ActionSkip: - index, err := requiredParamInt(r, "index") + index, err := p.Int("index") if err != nil { return nil, newError(responses.ErrorMissingParameter, "missing parameter index, err: %s", err) } - offset := utils.ParamInt(r, "offset", 0) + offset := p.IntOr("offset", 0) if err != nil { offset = 0 } return createResponse(pb.Skip(ctx, index, offset)) case ActionAdd: - ids := utils.ParamStrings(r, "id") + ids, _ := p.Strings("id") return createResponse(pb.Add(ctx, ids)) case ActionClear: return createResponse(pb.Clear(ctx)) case ActionRemove: - index, err := requiredParamInt(r, "index") + index, err := p.Int("index") if err != nil { return nil, err } @@ -91,7 +92,7 @@ func (api *Router) JukeboxControl(r *http.Request) (*responses.Subsonic, error) case ActionShuffle: return createResponse(pb.Shuffle(ctx)) case ActionSetGain: - gainStr, err := requiredParamString(r, "gain") + gainStr, err := p.String("gain") if err != nil { return nil, newError(responses.ErrorMissingParameter, "missing parameter gain, err: %s", err) } diff --git a/server/subsonic/library_scanning.go b/server/subsonic/library_scanning.go index dc0838a9..2ef75887 100644 --- a/server/subsonic/library_scanning.go +++ b/server/subsonic/library_scanning.go @@ -8,7 +8,7 @@ import ( "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model/request" "github.com/navidrome/navidrome/server/subsonic/responses" - "github.com/navidrome/navidrome/utils" + "github.com/navidrome/navidrome/utils/req" ) func (api *Router) GetScanStatus(r *http.Request) (*responses.Subsonic, error) { @@ -41,7 +41,8 @@ func (api *Router) StartScan(r *http.Request) (*responses.Subsonic, error) { return nil, newError(responses.ErrorAuthorizationFail) } - fullScan := utils.ParamBool(r, "fullScan", false) + p := req.Params(r) + fullScan := p.BoolOr("fullScan", false) go func() { start := time.Now() diff --git a/server/subsonic/media_annotation.go b/server/subsonic/media_annotation.go index 36c67373..c9656d06 100644 --- a/server/subsonic/media_annotation.go +++ b/server/subsonic/media_annotation.go @@ -160,7 +160,7 @@ func (api *Router) Scrobble(r *http.Request) (*responses.Subsonic, error) { if err != nil { return nil, err } - times := p.Times("time") + times, _ := p.Times("time") if len(times) > 0 && len(times) != len(ids) { return nil, newError(responses.ErrorGeneric, "Wrong number of timestamps: %d, should be %d", len(times), len(ids)) } diff --git a/server/subsonic/media_retrieval.go b/server/subsonic/media_retrieval.go index 1190c630..5d4383d6 100644 --- a/server/subsonic/media_retrieval.go +++ b/server/subsonic/media_retrieval.go @@ -15,15 +15,16 @@ import ( "github.com/navidrome/navidrome/resources" "github.com/navidrome/navidrome/server/subsonic/filter" "github.com/navidrome/navidrome/server/subsonic/responses" - "github.com/navidrome/navidrome/utils" "github.com/navidrome/navidrome/utils/gravatar" + "github.com/navidrome/navidrome/utils/req" ) func (api *Router) GetAvatar(w http.ResponseWriter, r *http.Request) (*responses.Subsonic, error) { if !conf.Server.EnableGravatar { return api.getPlaceHolderAvatar(w, r) } - username, err := requiredParamString(r, "username") + p := req.Params(r) + username, err := p.String("username") if err != nil { return nil, err } @@ -61,8 +62,9 @@ func (api *Router) GetCoverArt(w http.ResponseWriter, r *http.Request) (*respons ctx, cancel := context.WithTimeout(r.Context(), 10*time.Second) defer cancel() - id := utils.ParamString(r, "id") - size := utils.ParamInt(r, "size", 0) + p := req.Params(r) + id, _ := p.String("id") + size := p.IntOr("size", 0) imgReader, lastUpdate, err := api.artwork.GetOrPlaceholder(ctx, id, size) w.Header().Set("cache-control", "public, max-age=315360000") @@ -99,8 +101,9 @@ func isSynced(rawLyrics string) bool { } func (api *Router) GetLyrics(r *http.Request) (*responses.Subsonic, error) { - artist := utils.ParamString(r, "artist") - title := utils.ParamString(r, "title") + p := req.Params(r) + artist, _ := p.String("artist") + title, _ := p.String("title") response := newResponse() lyrics := responses.Lyrics{} response.Lyrics = &lyrics diff --git a/server/subsonic/middlewares.go b/server/subsonic/middlewares.go index 1efe6166..a5f36832 100644 --- a/server/subsonic/middlewares.go +++ b/server/subsonic/middlewares.go @@ -20,8 +20,8 @@ import ( "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/request" "github.com/navidrome/navidrome/server/subsonic/responses" - "github.com/navidrome/navidrome/utils" . "github.com/navidrome/navidrome/utils/gg" + "github.com/navidrome/navidrome/utils/req" ) func postFormToQueryParams(next http.Handler) http.Handler { @@ -45,19 +45,18 @@ func postFormToQueryParams(next http.Handler) http.Handler { func checkRequiredParameters(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { requiredParameters := []string{"u", "v", "c"} - - for _, p := range requiredParameters { - if utils.ParamString(r, p) == "" { - msg := fmt.Sprintf(`Missing required parameter "%s"`, p) - log.Warn(r, msg) - sendError(w, r, newError(responses.ErrorMissingParameter, msg)) + p := req.Params(r) + for _, param := range requiredParameters { + if _, err := p.String(param); err != nil { + log.Warn(r, err) + sendError(w, r, err) return } } - username := utils.ParamString(r, "u") - client := utils.ParamString(r, "c") - version := utils.ParamString(r, "v") + username, _ := p.String("u") + client, _ := p.String("c") + version, _ := p.String("v") ctx := r.Context() ctx = request.WithUsername(ctx, username) ctx = request.WithClient(ctx, client) @@ -73,12 +72,13 @@ func authenticate(ds model.DataStore) func(next http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { ctx := r.Context() - username := utils.ParamString(r, "u") + p := req.Params(r) + username, _ := p.String("u") - pass := utils.ParamString(r, "p") - token := utils.ParamString(r, "t") - salt := utils.ParamString(r, "s") - jwt := utils.ParamString(r, "jwt") + pass, _ := p.String("p") + token, _ := p.String("t") + salt, _ := p.String("s") + jwt, _ := p.String("jwt") usr, err := validateUser(ctx, ds, username, pass, token, salt, jwt) if errors.Is(err, model.ErrInvalidAuth) { diff --git a/server/subsonic/playlists.go b/server/subsonic/playlists.go index 2927f219..4b01d5e8 100644 --- a/server/subsonic/playlists.go +++ b/server/subsonic/playlists.go @@ -10,7 +10,7 @@ import ( "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/server/subsonic/responses" - "github.com/navidrome/navidrome/utils" + "github.com/navidrome/navidrome/utils/req" ) func (api *Router) GetPlaylists(r *http.Request) (*responses.Subsonic, error) { @@ -31,7 +31,8 @@ func (api *Router) GetPlaylists(r *http.Request) (*responses.Subsonic, error) { func (api *Router) GetPlaylist(r *http.Request) (*responses.Subsonic, error) { ctx := r.Context() - id, err := requiredParamString(r, "id") + p := req.Params(r) + id, err := p.String("id") if err != nil { return nil, err } @@ -84,9 +85,10 @@ func (api *Router) create(ctx context.Context, playlistId, name string, ids []st func (api *Router) CreatePlaylist(r *http.Request) (*responses.Subsonic, error) { ctx := r.Context() - songIds := utils.ParamStrings(r, "songId") - playlistId := utils.ParamString(r, "playlistId") - name := utils.ParamString(r, "name") + p := req.Params(r) + songIds, _ := p.Strings("songId") + playlistId, _ := p.String("playlistId") + name, _ := p.String("name") if playlistId == "" && name == "" { return nil, errors.New("required parameter name is missing") } @@ -99,7 +101,8 @@ func (api *Router) CreatePlaylist(r *http.Request) (*responses.Subsonic, error) } func (api *Router) DeletePlaylist(r *http.Request) (*responses.Subsonic, error) { - id, err := requiredParamString(r, "id") + p := req.Params(r) + id, err := p.String("id") if err != nil { return nil, err } @@ -115,23 +118,23 @@ func (api *Router) DeletePlaylist(r *http.Request) (*responses.Subsonic, error) } func (api *Router) UpdatePlaylist(r *http.Request) (*responses.Subsonic, error) { - playlistId, err := requiredParamString(r, "playlistId") + p := req.Params(r) + playlistId, err := p.String("playlistId") if err != nil { return nil, err } - songsToAdd := utils.ParamStrings(r, "songIdToAdd") - songIndexesToRemove := utils.ParamInts(r, "songIndexToRemove") + songsToAdd, _ := p.Strings("songIdToAdd") + songIndexesToRemove, _ := p.Ints("songIndexToRemove") var plsName *string - if s, ok := r.URL.Query()["name"]; ok { - plsName = &s[0] + if s, err := p.String("name"); err == nil { + plsName = &s } var comment *string - if c, ok := r.URL.Query()["comment"]; ok { - comment = &c[0] + if s, err := p.String("comment"); err == nil { + comment = &s } var public *bool - if _, ok := r.URL.Query()["public"]; ok { - p := utils.ParamBool(r, "public", false) + if p, err := p.Bool("public"); err == nil { public = &p } diff --git a/server/subsonic/radio.go b/server/subsonic/radio.go index bc2497f5..9f2cd48f 100644 --- a/server/subsonic/radio.go +++ b/server/subsonic/radio.go @@ -5,21 +5,22 @@ import ( "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/server/subsonic/responses" - "github.com/navidrome/navidrome/utils" + "github.com/navidrome/navidrome/utils/req" ) func (api *Router) CreateInternetRadio(r *http.Request) (*responses.Subsonic, error) { - streamUrl, err := requiredParamString(r, "streamUrl") + p := req.Params(r) + streamUrl, err := p.String("streamUrl") if err != nil { return nil, err } - name, err := requiredParamString(r, "name") + name, err := p.String("name") if err != nil { return nil, err } - homepageUrl := utils.ParamString(r, "homepageUrl") + homepageUrl, _ := p.String("homepageUrl") ctx := r.Context() radio := &model.Radio{ @@ -36,7 +37,8 @@ func (api *Router) CreateInternetRadio(r *http.Request) (*responses.Subsonic, er } func (api *Router) DeleteInternetRadio(r *http.Request) (*responses.Subsonic, error) { - id, err := requiredParamString(r, "id") + p := req.Params(r) + id, err := p.String("id") if err != nil { return nil, err @@ -75,22 +77,23 @@ func (api *Router) GetInternetRadios(r *http.Request) (*responses.Subsonic, erro } func (api *Router) UpdateInternetRadio(r *http.Request) (*responses.Subsonic, error) { - id, err := requiredParamString(r, "id") + p := req.Params(r) + id, err := p.String("id") if err != nil { return nil, err } - streamUrl, err := requiredParamString(r, "streamUrl") + streamUrl, err := p.String("streamUrl") if err != nil { return nil, err } - name, err := requiredParamString(r, "name") + name, err := p.String("name") if err != nil { return nil, err } - homepageUrl := utils.ParamString(r, "homepageUrl") + homepageUrl, _ := p.String("homepageUrl") ctx := r.Context() radio := &model.Radio{ diff --git a/server/subsonic/searching.go b/server/subsonic/searching.go index d40d42be..a2ba2d1c 100644 --- a/server/subsonic/searching.go +++ b/server/subsonic/searching.go @@ -14,7 +14,7 @@ import ( "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/server/public" "github.com/navidrome/navidrome/server/subsonic/responses" - "github.com/navidrome/navidrome/utils" + "github.com/navidrome/navidrome/utils/req" ) type searchParams struct { @@ -28,18 +28,19 @@ type searchParams struct { } func (api *Router) getParams(r *http.Request) (*searchParams, error) { + p := req.Params(r) var err error sp := &searchParams{} - sp.query, err = requiredParamString(r, "query") + sp.query, err = p.String("query") if err != nil { return nil, err } - sp.artistCount = utils.ParamInt(r, "artistCount", 20) - sp.artistOffset = utils.ParamInt(r, "artistOffset", 0) - sp.albumCount = utils.ParamInt(r, "albumCount", 20) - sp.albumOffset = utils.ParamInt(r, "albumOffset", 0) - sp.songCount = utils.ParamInt(r, "songCount", 20) - sp.songOffset = utils.ParamInt(r, "songOffset", 0) + sp.artistCount = p.IntOr("artistCount", 20) + sp.artistOffset = p.IntOr("artistOffset", 0) + sp.albumCount = p.IntOr("albumCount", 20) + sp.albumOffset = p.IntOr("albumOffset", 0) + sp.songCount = p.IntOr("songCount", 20) + sp.songOffset = p.IntOr("songOffset", 0) return sp, nil } diff --git a/server/subsonic/sharing.go b/server/subsonic/sharing.go index 792ecfc4..02110b08 100644 --- a/server/subsonic/sharing.go +++ b/server/subsonic/sharing.go @@ -9,7 +9,7 @@ import ( "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/server/public" "github.com/navidrome/navidrome/server/subsonic/responses" - "github.com/navidrome/navidrome/utils" + "github.com/navidrome/navidrome/utils/req" ) func (api *Router) GetShares(r *http.Request) (*responses.Subsonic, error) { @@ -50,13 +50,14 @@ func (api *Router) buildShare(r *http.Request, share model.Share) responses.Shar } func (api *Router) CreateShare(r *http.Request) (*responses.Subsonic, error) { - ids := utils.ParamStrings(r, "id") - if len(ids) == 0 { - return nil, newError(responses.ErrorMissingParameter, "Required id parameter is missing") + p := req.Params(r) + ids, err := p.Strings("id") + if err != nil { + return nil, err } - description := utils.ParamString(r, "description") - expires := utils.ParamTime(r, "expires", time.Time{}) + description, _ := p.String("description") + expires := p.TimeOr("expires", time.Time{}) repo := api.share.NewRepository(r.Context()) share := &model.Share{ @@ -81,13 +82,14 @@ func (api *Router) CreateShare(r *http.Request) (*responses.Subsonic, error) { } func (api *Router) UpdateShare(r *http.Request) (*responses.Subsonic, error) { - id := utils.ParamString(r, "id") - if id == "" { - return nil, newError(responses.ErrorMissingParameter, "Required id parameter is missing") + p := req.Params(r) + id, err := p.String("id") + if err != nil { + return nil, err } - description := utils.ParamString(r, "description") - expires := utils.ParamTime(r, "expires", time.Time{}) + description, _ := p.String("description") + expires := p.TimeOr("expires", time.Time{}) repo := api.share.NewRepository(r.Context()) share := &model.Share{ @@ -96,7 +98,7 @@ func (api *Router) UpdateShare(r *http.Request) (*responses.Subsonic, error) { ExpiresAt: expires, } - err := repo.(rest.Persistable).Update(id, share) + err = repo.(rest.Persistable).Update(id, share) if err != nil { return nil, err } @@ -105,13 +107,14 @@ func (api *Router) UpdateShare(r *http.Request) (*responses.Subsonic, error) { } func (api *Router) DeleteShare(r *http.Request) (*responses.Subsonic, error) { - id := utils.ParamString(r, "id") - if id == "" { - return nil, newError(responses.ErrorMissingParameter, "Required id parameter is missing") + p := req.Params(r) + id, err := p.String("id") + if err != nil { + return nil, err } repo := api.share.NewRepository(r.Context()) - err := repo.(rest.Persistable).Delete(id) + err = repo.(rest.Persistable).Delete(id) if err != nil { return nil, err } diff --git a/server/subsonic/stream.go b/server/subsonic/stream.go index 1da20159..07ce8e18 100644 --- a/server/subsonic/stream.go +++ b/server/subsonic/stream.go @@ -14,7 +14,7 @@ import ( "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/request" "github.com/navidrome/navidrome/server/subsonic/responses" - "github.com/navidrome/navidrome/utils" + "github.com/navidrome/navidrome/utils/req" ) func (api *Router) serveStream(ctx context.Context, w http.ResponseWriter, r *http.Request, stream *core.Stream, id string) { @@ -25,7 +25,7 @@ func (api *Router) serveStream(ctx context.Context, w http.ResponseWriter, r *ht w.Header().Set("Accept-Ranges", "none") w.Header().Set("Content-Type", stream.ContentType()) - estimateContentLength := utils.ParamBool(r, "estimateContentLength", false) + estimateContentLength := req.Params(r).BoolOr("estimateContentLength", false) // if Client requests the estimated content-length, send it if estimateContentLength { @@ -51,13 +51,14 @@ func (api *Router) serveStream(ctx context.Context, w http.ResponseWriter, r *ht func (api *Router) Stream(w http.ResponseWriter, r *http.Request) (*responses.Subsonic, error) { ctx := r.Context() - id, err := requiredParamString(r, "id") + p := req.Params(r) + id, err := p.String("id") if err != nil { return nil, err } - maxBitRate := utils.ParamInt(r, "maxBitRate", 0) - format := utils.ParamString(r, "format") - timeOffset := utils.ParamInt(r, "timeOffset", 0) + maxBitRate := p.IntOr("maxBitRate", 0) + format, _ := p.String("format") + timeOffset := p.IntOr("timeOffset", 0) stream, err := api.streamer.NewStream(ctx, id, format, maxBitRate, timeOffset) if err != nil { @@ -82,7 +83,8 @@ func (api *Router) Stream(w http.ResponseWriter, r *http.Request) (*responses.Su func (api *Router) Download(w http.ResponseWriter, r *http.Request) (*responses.Subsonic, error) { ctx := r.Context() username, _ := request.UsernameFrom(ctx) - id, err := requiredParamString(r, "id") + p := req.Params(r) + id, err := p.String("id") if err != nil { return nil, err } @@ -97,8 +99,8 @@ func (api *Router) Download(w http.ResponseWriter, r *http.Request) (*responses. return nil, err } - maxBitRate := utils.ParamInt(r, "bitrate", 0) - format := utils.ParamString(r, "format") + maxBitRate := p.IntOr("bitrate", 0) + format, _ := p.String("format") if format == "" { if conf.Server.AutoTranscodeDownload { diff --git a/utils/req/req.go b/utils/req/req.go index a314556c..54cb7e5f 100644 --- a/utils/req/req.go +++ b/utils/req/req.go @@ -68,8 +68,11 @@ func (r *Values) TimeOr(param string, def time.Time) time.Time { return t } -func (r *Values) Times(param string) []time.Time { - pStr, _ := r.Strings(param) +func (r *Values) Times(param string) ([]time.Time, error) { + pStr, err := r.Strings(param) + if err != nil { + return nil, err + } times := make([]time.Time, len(pStr)) for i, t := range pStr { ti, err := strconv.ParseInt(t, 10, 64) @@ -80,7 +83,7 @@ func (r *Values) Times(param string) []time.Time { } times[i] = utils.ToTime(ti) } - return times + return times, nil } func (r *Values) Int64(param string) (int64, error) { @@ -119,8 +122,11 @@ func (r *Values) Int64Or(param string, def int64) int64 { return v } -func (r *Values) Ints(param string) []int { - pStr, _ := r.Strings(param) +func (r *Values) Ints(param string) ([]int, error) { + pStr, err := r.Strings(param) + if err != nil { + return nil, err + } ints := make([]int, 0, len(pStr)) for _, s := range pStr { i, err := strconv.ParseInt(s, 10, 64) @@ -128,13 +134,21 @@ func (r *Values) Ints(param string) []int { ints = append(ints, int(i)) } } - return ints + return ints, nil +} + +func (r *Values) Bool(param string) (bool, error) { + v, err := r.String(param) + if err != nil { + return false, err + } + return strings.Contains("/true/on/1/", "/"+strings.ToLower(v)+"/"), nil } func (r *Values) BoolOr(param string, def bool) bool { - v, _ := r.String(param) - if v == "" { + v, err := r.Bool(param) + if err != nil { return def } - return strings.Contains("/true/on/1/", "/"+strings.ToLower(v)+"/") + return v } diff --git a/utils/req/req_test.go b/utils/req/req_test.go index 0433ae6d..5e305c41 100644 --- a/utils/req/req_test.go +++ b/utils/req/req_test.go @@ -102,13 +102,16 @@ var _ = Describe("Request Helpers", func() { }) It("returns empty string if param does not exist", func() { - Expect(r.Times("xx")).To(BeEmpty()) + v, err := r.Times("xx") + Expect(err).To(MatchError(req.ErrMissingParam)) + Expect(v).To(BeEmpty()) }) It("returns current time as default if param is invalid", func() { now := time.Now() r = req.Params(httptest.NewRequest("GET", "/ping?t=null", nil)) - times := r.Times("t") + times, err := r.Times("t") + Expect(err).ToNot(HaveOccurred()) Expect(times).To(HaveLen(1)) Expect(times[0]).To(BeTemporally(">=", now)) }) @@ -130,18 +133,28 @@ var _ = Describe("Request Helpers", func() { It("returns default value if param is an invalid int", func() { Expect(r.IntOr("inv", 999)).To(Equal(999)) }) + + It("returns error if param is an invalid int", func() { + _, err := r.Int("inv") + Expect(err).To(MatchError(req.ErrInvalidParam)) + }) }) Context("int64", func() { It("returns parsed int64", func() { - Expect(r.IntOr("i", 999)).To(Equal(123)) + Expect(r.Int64Or("i", 999)).To(Equal(int64(123))) }) It("returns default value if param does not exist", func() { - Expect(r.IntOr("xx", 999)).To(Equal(999)) + Expect(r.Int64Or("xx", 999)).To(Equal(int64(999))) }) It("returns default value if param is an invalid int", func() { - Expect(r.IntOr("inv", 999)).To(Equal(999)) + Expect(r.Int64Or("inv", 999)).To(Equal(int64(999))) + }) + + It("returns error if param is an invalid int", func() { + _, err := r.Int64("inv") + Expect(err).To(MatchError(req.ErrInvalidParam)) }) }) }) @@ -156,7 +169,9 @@ var _ = Describe("Request Helpers", func() { }) It("returns empty array if param does not exist", func() { - Expect(r.Ints("xx")).To(BeEmpty()) + v, err := r.Ints("xx") + Expect(err).To(MatchError(req.ErrMissingParam)) + Expect(v).To(BeEmpty()) }) }) diff --git a/utils/request_helpers.go b/utils/request_helpers.go deleted file mode 100644 index 104260de..00000000 --- a/utils/request_helpers.go +++ /dev/null @@ -1,90 +0,0 @@ -package utils - -import ( - "net/http" - "strconv" - "strings" - "time" - - "github.com/navidrome/navidrome/log" - "golang.org/x/exp/constraints" -) - -func ParamString(r *http.Request, param string) string { - return r.URL.Query().Get(param) -} - -func ParamStringDefault(r *http.Request, param, def string) string { - v := ParamString(r, param) - if v == "" { - return def - } - return v -} - -func ParamStrings(r *http.Request, param string) []string { - return r.URL.Query()[param] -} - -func ParamTimes(r *http.Request, param string) []time.Time { - pStr := ParamStrings(r, param) - times := make([]time.Time, len(pStr)) - for i, t := range pStr { - ti, err := strconv.ParseInt(t, 10, 64) - if err != nil { - log.Warn(r.Context(), "Ignoring invalid time param", "time", t, err) - times[i] = time.Now() - continue - } - times[i] = ToTime(ti) - } - return times -} - -func ParamTime(r *http.Request, param string, def time.Time) time.Time { - v := ParamString(r, param) - if v == "" || v == "-1" { - return def - } - value, err := strconv.ParseInt(v, 10, 64) - if err != nil { - return def - } - t := ToTime(value) - if t.Before(time.Date(1970, time.January, 2, 0, 0, 0, 0, time.UTC)) { - return def - } - return t -} - -func ParamInt[T constraints.Integer](r *http.Request, param string, def T) T { - v := ParamString(r, param) - if v == "" { - return def - } - value, err := strconv.ParseInt(v, 10, 64) - if err != nil { - return def - } - return T(value) -} - -func ParamInts(r *http.Request, param string) []int { - pStr := ParamStrings(r, param) - ints := make([]int, 0, len(pStr)) - for _, s := range pStr { - i, err := strconv.ParseInt(s, 10, 32) - if err == nil { - ints = append(ints, int(i)) - } - } - return ints -} - -func ParamBool(r *http.Request, param string, def bool) bool { - p := strings.ToLower(ParamString(r, param)) - if p == "" { - return def - } - return strings.Contains("/true/on/1/", "/"+p+"/") -} diff --git a/utils/request_helpers_test.go b/utils/request_helpers_test.go deleted file mode 100644 index a7a82911..00000000 --- a/utils/request_helpers_test.go +++ /dev/null @@ -1,196 +0,0 @@ -package utils - -import ( - "fmt" - "net/http" - "net/http/httptest" - "time" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" -) - -var _ = Describe("Request Helpers", func() { - var r *http.Request - - Describe("ParamString", func() { - BeforeEach(func() { - r = httptest.NewRequest("GET", "/ping?a=123", nil) - }) - - It("returns empty string if param does not exist", func() { - Expect(ParamString(r, "xx")).To(Equal("")) - }) - - It("returns param as string", func() { - Expect(ParamString(r, "a")).To(Equal("123")) - }) - }) - - Describe("ParamStringDefault", func() { - BeforeEach(func() { - r = httptest.NewRequest("GET", "/ping?a=123", nil) - }) - - It("returns default string if param does not exist", func() { - Expect(ParamStringDefault(r, "xx", "default_value")).To(Equal("default_value")) - }) - - It("returns param as string", func() { - Expect(ParamStringDefault(r, "a", "default_value")).To(Equal("123")) - }) - }) - - Describe("ParamStrings", func() { - BeforeEach(func() { - r = httptest.NewRequest("GET", "/ping?a=123&a=456", nil) - }) - - It("returns empty array if param does not exist", func() { - Expect(ParamStrings(r, "xx")).To(BeEmpty()) - }) - - It("returns all param occurrences as []string", func() { - Expect(ParamStrings(r, "a")).To(Equal([]string{"123", "456"})) - }) - }) - - Describe("ParamTime", func() { - d := time.Date(2002, 8, 9, 12, 11, 13, 1000000, time.Local) - t := ToMillis(d) - now := time.Now() - BeforeEach(func() { - r = httptest.NewRequest("GET", fmt.Sprintf("/ping?t=%d&inv=abc", t), nil) - }) - - It("returns default time if param does not exist", func() { - Expect(ParamTime(r, "xx", now)).To(Equal(now)) - }) - - It("returns default time if param is an invalid timestamp", func() { - Expect(ParamTime(r, "inv", now)).To(Equal(now)) - }) - - It("returns parsed time", func() { - Expect(ParamTime(r, "t", now)).To(Equal(d)) - }) - }) - - Describe("ParamTimes", func() { - d1 := time.Date(2002, 8, 9, 12, 11, 13, 1000000, time.Local) - d2 := time.Date(2002, 8, 9, 12, 13, 56, 0000000, time.Local) - t1 := ToMillis(d1) - t2 := ToMillis(d2) - BeforeEach(func() { - r = httptest.NewRequest("GET", fmt.Sprintf("/ping?t=%d&t=%d", t1, t2), nil) - }) - - It("returns empty string if param does not exist", func() { - Expect(ParamTimes(r, "xx")).To(BeEmpty()) - }) - - It("returns all param occurrences as []time.Time", func() { - Expect(ParamTimes(r, "t")).To(Equal([]time.Time{d1, d2})) - }) - It("returns current time as default if param is invalid", func() { - now := time.Now() - r = httptest.NewRequest("GET", "/ping?t=null", nil) - times := ParamTimes(r, "t") - Expect(times).To(HaveLen(1)) - Expect(times[0]).To(BeTemporally(">=", now)) - }) - }) - - Describe("ParamInt", func() { - BeforeEach(func() { - r = httptest.NewRequest("GET", "/ping?i=123&inv=123.45", nil) - }) - Context("int", func() { - It("returns default value if param does not exist", func() { - Expect(ParamInt(r, "xx", 999)).To(Equal(999)) - }) - - It("returns default value if param is an invalid int", func() { - Expect(ParamInt(r, "inv", 999)).To(Equal(999)) - }) - - It("returns parsed time", func() { - Expect(ParamInt(r, "i", 999)).To(Equal(123)) - }) - }) - Context("int64", func() { - It("returns default value if param does not exist", func() { - Expect(ParamInt(r, "xx", int64(999))).To(Equal(int64(999))) - }) - - It("returns default value if param is an invalid int", func() { - Expect(ParamInt(r, "inv", int64(999))).To(Equal(int64(999))) - }) - - It("returns parsed time", func() { - Expect(ParamInt(r, "i", int64(999))).To(Equal(int64(123))) - }) - - }) - }) - - Describe("ParamInts", func() { - BeforeEach(func() { - r = httptest.NewRequest("GET", "/ping?i=123&i=456", nil) - }) - - It("returns empty array if param does not exist", func() { - Expect(ParamInts(r, "xx")).To(BeEmpty()) - }) - - It("returns array of occurrences found", func() { - Expect(ParamInts(r, "i")).To(Equal([]int{123, 456})) - }) - }) - - Describe("ParamBool", func() { - Context("value is true", func() { - BeforeEach(func() { - r = httptest.NewRequest("GET", "/ping?b=true&c=on&d=1&e=True", nil) - }) - - It("parses 'true'", func() { - Expect(ParamBool(r, "b", false)).To(BeTrue()) - }) - - It("parses 'on'", func() { - Expect(ParamBool(r, "c", false)).To(BeTrue()) - }) - - It("parses '1'", func() { - Expect(ParamBool(r, "d", false)).To(BeTrue()) - }) - - It("parses 'True'", func() { - Expect(ParamBool(r, "e", false)).To(BeTrue()) - }) - }) - - Context("value is false", func() { - BeforeEach(func() { - r = httptest.NewRequest("GET", "/ping?b=false&c=off&d=0", nil) - }) - - It("returns default value if param does not exist", func() { - Expect(ParamBool(r, "xx", true)).To(BeTrue()) - }) - - It("parses 'false'", func() { - Expect(ParamBool(r, "b", true)).To(BeFalse()) - }) - - It("parses 'off'", func() { - Expect(ParamBool(r, "c", true)).To(BeFalse()) - }) - - It("parses '0'", func() { - Expect(ParamBool(r, "d", true)).To(BeFalse()) - }) - }) - }) -})