From 9712a5b1c66cf789b3f868296eceed17b960cb2c Mon Sep 17 00:00:00 2001 From: Deluan Date: Tue, 2 Nov 2021 21:38:08 -0400 Subject: [PATCH] Fix error codes for required parameters in getAlbumList --- server/subsonic/album_lists.go | 27 +++++++++++++++++++-------- server/subsonic/album_lists_test.go | 6 ++++++ 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/server/subsonic/album_lists.go b/server/subsonic/album_lists.go index bcd0b40b..7344510a 100644 --- a/server/subsonic/album_lists.go +++ b/server/subsonic/album_lists.go @@ -2,7 +2,6 @@ package subsonic import ( "context" - "errors" "net/http" "strconv" "time" @@ -53,12 +52,24 @@ func (c *AlbumListController) getAlbumList(r *http.Request) (model.Albums, int64 case "highest": opts = filter.AlbumsByRating() case "byGenre": - opts = filter.AlbumsByGenre(utils.ParamString(r, "genre")) + genre, err := requiredParamString(r, "genre") + if err != nil { + return nil, 0, err + } + opts = filter.AlbumsByGenre(genre) case "byYear": - opts = filter.AlbumsByYear(utils.ParamInt(r, "fromYear", 0), utils.ParamInt(r, "toYear", 0)) + fromYear, err := requiredParamInt(r, "fromYear") + if err != nil { + return nil, 0, err + } + toYear, err := requiredParamInt(r, "toYear") + if err != nil { + return nil, 0, err + } + opts = filter.AlbumsByYear(fromYear, toYear) default: log.Error(r, "albumList type not implemented", "type", typ) - return nil, 0, errors.New("not implemented") + return nil, 0, newError(responses.ErrorGeneric, "type '%s' not implemented", typ) } opts.Offset = utils.ParamInt(r, "offset", 0) @@ -67,13 +78,13 @@ func (c *AlbumListController) getAlbumList(r *http.Request) (model.Albums, int64 if err != nil { log.Error(r, "Error retrieving albums", "error", err) - return nil, 0, errors.New("internal error") + return nil, 0, newError(responses.ErrorGeneric, "internal error") } count, err := c.ds.Album(r.Context()).CountAll(opts) if err != nil { log.Error(r, "Error counting albums", "error", err) - return nil, 0, errors.New("internal error") + return nil, 0, newError(responses.ErrorGeneric, "internal error") } return albums, count, nil @@ -82,7 +93,7 @@ func (c *AlbumListController) getAlbumList(r *http.Request) (model.Albums, int64 func (c *AlbumListController) GetAlbumList(w http.ResponseWriter, r *http.Request) (*responses.Subsonic, error) { albums, count, err := c.getAlbumList(r) if err != nil { - return nil, newError(responses.ErrorGeneric, err.Error()) + return nil, err } w.Header().Set("x-total-count", strconv.Itoa(int(count))) @@ -95,7 +106,7 @@ func (c *AlbumListController) GetAlbumList(w http.ResponseWriter, r *http.Reques func (c *AlbumListController) GetAlbumList2(w http.ResponseWriter, r *http.Request) (*responses.Subsonic, error) { albums, pageCount, err := c.getAlbumList(r) if err != nil { - return nil, newError(responses.ErrorGeneric, err.Error()) + return nil, err } w.Header().Set("x-total-count", strconv.FormatInt(pageCount, 10)) diff --git a/server/subsonic/album_lists_test.go b/server/subsonic/album_lists_test.go index d3e982c2..0ae1d69f 100644 --- a/server/subsonic/album_lists_test.go +++ b/server/subsonic/album_lists_test.go @@ -4,6 +4,8 @@ import ( "context" "net/http/httptest" + "github.com/navidrome/navidrome/server/subsonic/responses" + "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/tests" @@ -46,6 +48,7 @@ var _ = Describe("AlbumListController", func() { _, err := controller.GetAlbumList(w, r) Expect(err).To(MatchError("required 'type' parameter is missing")) + Expect(err.(subError).code).To(Equal(responses.ErrorMissingParameter)) }) It("should return error if call fails", func() { @@ -55,6 +58,7 @@ var _ = Describe("AlbumListController", func() { _, err := controller.GetAlbumList(w, r) Expect(err).ToNot(BeNil()) + Expect(err.(subError).code).To(Equal(responses.ErrorGeneric)) }) }) @@ -79,6 +83,7 @@ var _ = Describe("AlbumListController", func() { _, err := controller.GetAlbumList2(w, r) Expect(err).To(MatchError("required 'type' parameter is missing")) + Expect(err.(subError).code).To(Equal(responses.ErrorMissingParameter)) }) It("should return error if call fails", func() { @@ -88,6 +93,7 @@ var _ = Describe("AlbumListController", func() { _, err := controller.GetAlbumList2(w, r) Expect(err).ToNot(BeNil()) + Expect(err.(subError).code).To(Equal(responses.ErrorGeneric)) }) }) })