From c87b13307133581263466c59b79838097e2e4386 Mon Sep 17 00:00:00 2001 From: Deluan Date: Wed, 23 Mar 2016 12:35:10 -0400 Subject: [PATCH] Polishing --- api/album_lists.go | 8 ++++---- api/album_lists_test.go | 6 +++--- api/base_api_controller.go | 4 ++-- api/browsing.go | 6 +++--- api/browsing_test.go | 10 +++++----- api/media_annotation.go | 2 +- api/media_retrieval.go | 6 +++--- api/media_retrieval_test.go | 6 +++--- api/playlists.go | 6 +++--- api/responses/errors.go | 34 ++++++++++++++++----------------- api/stream.go | 4 ++-- api/stream_test.go | 4 ++-- api/validation.go | 4 ++-- engine/search.go | 13 ++++++++++--- engine/stream.go | 2 +- persistence/ledis_repository.go | 6 +++--- utils/index_group_parser.go | 22 ++++++++++----------- 17 files changed, 74 insertions(+), 69 deletions(-) diff --git a/api/album_lists.go b/api/album_lists.go index e6407a379..faeef50b0 100644 --- a/api/album_lists.go +++ b/api/album_lists.go @@ -36,7 +36,7 @@ func (c *AlbumListController) GetAlbumList() { if !found { beego.Error("albumList type", typ, "not implemented!") - c.SendError(responses.ERROR_GENERIC, "Not implemented!") + c.SendError(responses.ErrorGeneric, "Not implemented!") } offset := c.ParamInt("offset", 0) @@ -45,7 +45,7 @@ func (c *AlbumListController) GetAlbumList() { albums, err := method(offset, size) if err != nil { beego.Error("Error retrieving albums:", err) - c.SendError(responses.ERROR_GENERIC, "Internal Error") + c.SendError(responses.ErrorGeneric, "Internal Error") } response := c.NewEmpty() @@ -57,7 +57,7 @@ func (c *AlbumListController) GetStarred() { albums, err := c.listGen.GetStarred(0, -1) if err != nil { beego.Error("Error retrieving starred albums:", err) - c.SendError(responses.ERROR_GENERIC, "Internal Error") + c.SendError(responses.ErrorGeneric, "Internal Error") } response := c.NewEmpty() @@ -71,7 +71,7 @@ func (c *AlbumListController) GetNowPlaying() { npInfos, err := c.listGen.GetNowPlaying() if err != nil { beego.Error("Error retrieving now playing list:", err) - c.SendError(responses.ERROR_GENERIC, "Internal Error") + c.SendError(responses.ErrorGeneric, "Internal Error") } response := c.NewEmpty() diff --git a/api/album_lists_test.go b/api/album_lists_test.go index d63045684..27f302a8e 100644 --- a/api/album_lists_test.go +++ b/api/album_lists_test.go @@ -34,18 +34,18 @@ func TestGetAlbumList(t *testing.T) { Convey("Should fail if missing 'type' parameter", func() { _, w := Get(AddParams("/rest/getAlbumList.view"), "TestGetAlbumList") - So(w.Body, ShouldReceiveError, responses.ERROR_MISSING_PARAMETER) + So(w.Body, ShouldReceiveError, responses.ErrorMissingParameter) }) Convey("Return fail on Album Table error", func() { mockAlbumRepo.SetError(true) _, w := Get(AddParams("/rest/getAlbumList.view", "type=newest"), "TestGetAlbumList") - So(w.Body, ShouldReceiveError, responses.ERROR_GENERIC) + So(w.Body, ShouldReceiveError, responses.ErrorGeneric) }) Convey("Type is invalid", func() { _, w := Get(AddParams("/rest/getAlbumList.view", "type=not_implemented"), "TestGetAlbumList") - So(w.Body, ShouldReceiveError, responses.ERROR_GENERIC) + So(w.Body, ShouldReceiveError, responses.ErrorGeneric) }) Convey("Max size = 500", func() { _, w := Get(AddParams("/rest/getAlbumList.view", "type=newest", "size=501"), "TestGetAlbumList") diff --git a/api/base_api_controller.go b/api/base_api_controller.go index 0952a0f50..6287c25f7 100644 --- a/api/base_api_controller.go +++ b/api/base_api_controller.go @@ -22,7 +22,7 @@ func (c *BaseAPIController) NewEmpty() responses.Subsonic { func (c *BaseAPIController) RequiredParamString(param string, msg string) string { p := c.Input().Get(param) if p == "" { - c.SendError(responses.ERROR_MISSING_PARAMETER, msg) + c.SendError(responses.ErrorMissingParameter, msg) } return p } @@ -30,7 +30,7 @@ func (c *BaseAPIController) RequiredParamString(param string, msg string) string func (c *BaseAPIController) RequiredParamStrings(param string, msg string) []string { ps := c.Input()[param] if len(ps) == 0 { - c.SendError(responses.ERROR_MISSING_PARAMETER, msg) + c.SendError(responses.ErrorMissingParameter, msg) } return ps } diff --git a/api/browsing.go b/api/browsing.go index 66a5fe038..85e156bd7 100644 --- a/api/browsing.go +++ b/api/browsing.go @@ -39,7 +39,7 @@ func (c *BrowsingController) GetIndexes() { indexes, lastModified, err := c.browser.Indexes(ifModifiedSince) if err != nil { beego.Error("Error retrieving Indexes:", err) - c.SendError(responses.ERROR_GENERIC, "Internal Error") + c.SendError(responses.ErrorGeneric, "Internal Error") } res := responses.Indexes{ @@ -71,10 +71,10 @@ func (c *BrowsingController) GetDirectory() { switch { case err == engine.ErrDataNotFound: beego.Error("Requested Id", id, "not found:", err) - c.SendError(responses.ERROR_DATA_NOT_FOUND, "Directory not found") + c.SendError(responses.ErrorDataNotFound, "Directory not found") case err != nil: beego.Error(err) - c.SendError(responses.ERROR_GENERIC, "Internal Error") + c.SendError(responses.ErrorGeneric, "Internal Error") } response.Directory = c.buildDirectory(dir) diff --git a/api/browsing_test.go b/api/browsing_test.go index 0636d7161..93a847dfd 100644 --- a/api/browsing_test.go +++ b/api/browsing_test.go @@ -54,13 +54,13 @@ func TestGetIndexes(t *testing.T) { mockRepo.SetError(true) _, w := Get(AddParams("/rest/getIndexes.view", "ifModifiedSince=0"), "TestGetIndexes") - So(w.Body, ShouldReceiveError, responses.ERROR_GENERIC) + So(w.Body, ShouldReceiveError, responses.ErrorGeneric) }) Convey("Return fail on Property Table error", func() { propRepo.SetError(true) _, w := Get(AddParams("/rest/getIndexes.view"), "TestGetIndexes") - So(w.Body, ShouldReceiveError, responses.ERROR_GENERIC) + So(w.Body, ShouldReceiveError, responses.ErrorGeneric) }) Convey("When the index is empty", func() { _, w := Get(AddParams("/rest/getIndexes.view"), "TestGetIndexes") @@ -133,7 +133,7 @@ func TestGetMusicDirectory(t *testing.T) { Convey("Should fail if missing Id parameter", func() { _, w := Get(AddParams("/rest/getMusicDirectory.view"), "TestGetMusicDirectory") - So(w.Body, ShouldReceiveError, responses.ERROR_MISSING_PARAMETER) + So(w.Body, ShouldReceiveError, responses.ErrorMissingParameter) }) Convey("Id is for an artist", func() { Convey("Return fail on Artist Table error", func() { @@ -141,14 +141,14 @@ func TestGetMusicDirectory(t *testing.T) { mockArtistRepo.SetError(true) _, w := Get(AddParams("/rest/getMusicDirectory.view", "id=1"), "TestGetMusicDirectory") - So(w.Body, ShouldReceiveError, responses.ERROR_GENERIC) + So(w.Body, ShouldReceiveError, responses.ErrorGeneric) }) }) Convey("When id is not found", func() { mockArtistRepo.SetData(`[{"Id":"1","Name":"The Charlatans"}]`, 1) _, w := Get(AddParams("/rest/getMusicDirectory.view", "id=NOT_FOUND"), "TestGetMusicDirectory") - So(w.Body, ShouldReceiveError, responses.ERROR_DATA_NOT_FOUND) + So(w.Body, ShouldReceiveError, responses.ErrorDataNotFound) }) Convey("When id matches an artist", func() { mockArtistRepo.SetData(`[{"Id":"1","Name":"The KLF"}]`, 1) diff --git a/api/media_annotation.go b/api/media_annotation.go index 66b486f8d..ffb368eb5 100644 --- a/api/media_annotation.go +++ b/api/media_annotation.go @@ -23,7 +23,7 @@ func (c *MediaAnnotationController) Scrobble() { ids := c.RequiredParamStrings("id", "Required id parameter is missing") times := c.ParamTimes("time") if len(times) > 0 && len(times) != len(ids) { - c.SendError(responses.ERROR_GENERIC, fmt.Sprintf("Wrong number of timestamps: %d", len(times))) + c.SendError(responses.ErrorGeneric, fmt.Sprintf("Wrong number of timestamps: %d", len(times))) } submission := c.ParamBool("submission", true) playerId := 1 // TODO Multiple players, based on playerName/username/clientIP(?) diff --git a/api/media_retrieval.go b/api/media_retrieval.go index 1d6d19976..efcfe6ac5 100644 --- a/api/media_retrieval.go +++ b/api/media_retrieval.go @@ -24,7 +24,7 @@ func (c *MediaRetrievalController) GetAvatar() { f, err := os.Open("static/itunes.png") if err != nil { beego.Error(err, "Image not found") - c.SendError(responses.ERROR_DATA_NOT_FOUND, "Avatar image not found") + c.SendError(responses.ErrorDataNotFound, "Avatar image not found") } defer f.Close() io.Copy(c.Ctx.ResponseWriter, f) @@ -39,9 +39,9 @@ func (c *MediaRetrievalController) GetCover() { switch { case err == engine.ErrDataNotFound: beego.Error(err, "Id:", id) - c.SendError(responses.ERROR_DATA_NOT_FOUND, "Directory not found") + c.SendError(responses.ErrorDataNotFound, "Directory not found") case err != nil: beego.Error(err) - c.SendError(responses.ERROR_GENERIC, "Internal Error") + c.SendError(responses.ErrorGeneric, "Internal Error") } } diff --git a/api/media_retrieval_test.go b/api/media_retrieval_test.go index f582bd3d0..0c4e81931 100644 --- a/api/media_retrieval_test.go +++ b/api/media_retrieval_test.go @@ -37,7 +37,7 @@ func TestGetCoverArt(t *testing.T) { Convey("Should fail if missing Id parameter", func() { _, w := getCoverArt() - So(w.Body, ShouldReceiveError, responses.ERROR_MISSING_PARAMETER) + So(w.Body, ShouldReceiveError, responses.ErrorMissingParameter) }) Convey("When id is found", func() { mockMediaFileRepo.SetData(`[{"Id":"2","HasCoverArt":true,"Path":"tests/fixtures/01 Invisible (RED) Edit Version.mp3"}]`, 1) @@ -50,14 +50,14 @@ func TestGetCoverArt(t *testing.T) { mockMediaFileRepo.SetData(`[{"Id":"2","HasCoverArt":true,"Path":"tests/fixtures/NOT_FOUND.mp3"}]`, 1) _, w := getCoverArt("id=2") - So(w.Body, ShouldReceiveError, responses.ERROR_DATA_NOT_FOUND) + So(w.Body, ShouldReceiveError, responses.ErrorDataNotFound) }) Convey("When the engine reports an error", func() { mockMediaFileRepo.SetData(`[{"Id":"2","HasCoverArt":true,"Path":"tests/fixtures/NOT_FOUND.mp3"}]`, 1) mockMediaFileRepo.SetError(true) _, w := getCoverArt("id=2") - So(w.Body, ShouldReceiveError, responses.ERROR_GENERIC) + So(w.Body, ShouldReceiveError, responses.ErrorGeneric) }) Convey("When specifying a size", func() { mockMediaFileRepo.SetData(`[{"Id":"2","HasCoverArt":true,"Path":"tests/fixtures/01 Invisible (RED) Edit Version.mp3"}]`, 1) diff --git a/api/playlists.go b/api/playlists.go index f428e1374..d05e997f4 100644 --- a/api/playlists.go +++ b/api/playlists.go @@ -20,7 +20,7 @@ func (c *PlaylistsController) GetAll() { allPls, err := c.pls.GetAll() if err != nil { beego.Error(err) - c.SendError(responses.ERROR_GENERIC, "Internal error") + c.SendError(responses.ErrorGeneric, "Internal error") } playlists := make([]responses.Playlist, len(allPls)) for i, p := range allPls { @@ -44,10 +44,10 @@ func (c *PlaylistsController) Get() { switch { case err == engine.ErrDataNotFound: beego.Error(err, "Id:", id) - c.SendError(responses.ERROR_DATA_NOT_FOUND, "Directory not found") + c.SendError(responses.ErrorDataNotFound, "Directory not found") case err != nil: beego.Error(err) - c.SendError(responses.ERROR_GENERIC, "Internal Error") + c.SendError(responses.ErrorGeneric, "Internal Error") } response := c.NewEmpty() diff --git a/api/responses/errors.go b/api/responses/errors.go index ddfb11868..5e61fd530 100644 --- a/api/responses/errors.go +++ b/api/responses/errors.go @@ -1,14 +1,14 @@ package responses const ( - ERROR_GENERIC = iota * 10 - ERROR_MISSING_PARAMETER - ERROR_CLIENT_TOO_OLD - ERROR_SERVER_TOO_OLD - ERROR_AUTHENTICATION_FAIL - ERROR_AUTHORIZATION_FAIL - ERROR_TRIAL_EXPIRED - ERROR_DATA_NOT_FOUND + ErrorGeneric = iota * 10 + ErrorMissingParameter + ErrorClientTooOld + ErrorServerTooOld + ErrorAuthenticationFail + ErrorAuthorizationFail + ErrorTrialExpired + ErrorDataNotFound ) var ( @@ -17,19 +17,19 @@ var ( func init() { errors = make(map[int]string) - errors[ERROR_GENERIC] = "A generic error" - errors[ERROR_MISSING_PARAMETER] = "Required parameter is missing" - errors[ERROR_CLIENT_TOO_OLD] = "Incompatible Subsonic REST protocol version. Client must upgrade" - errors[ERROR_SERVER_TOO_OLD] = "Incompatible Subsonic REST protocol version. Server must upgrade" - errors[ERROR_AUTHENTICATION_FAIL] = "Wrong username or password" - errors[ERROR_AUTHORIZATION_FAIL] = "User is not authorized for the given operation" - errors[ERROR_TRIAL_EXPIRED] = "The trial period for the Subsonic server is over. Please upgrade to Subsonic Premium. Visit subsonic.org for details" - errors[ERROR_DATA_NOT_FOUND] = "The requested data was not found" + errors[ErrorGeneric] = "A generic error" + errors[ErrorMissingParameter] = "Required parameter is missing" + errors[ErrorClientTooOld] = "Incompatible Subsonic REST protocol version. Client must upgrade" + errors[ErrorServerTooOld] = "Incompatible Subsonic REST protocol version. Server must upgrade" + errors[ErrorAuthenticationFail] = "Wrong username or password" + errors[ErrorAuthorizationFail] = "User is not authorized for the given operation" + errors[ErrorTrialExpired] = "The trial period for the Subsonic server is over. Please upgrade to Subsonic Premium. Visit subsonic.org for details" + errors[ErrorDataNotFound] = "The requested data was not found" } func ErrorMsg(code int) string { if v, found := errors[code]; found { return v } - return errors[ERROR_GENERIC] + return errors[ErrorGeneric] } diff --git a/api/stream.go b/api/stream.go index 174cec70a..9da3fca63 100644 --- a/api/stream.go +++ b/api/stream.go @@ -24,10 +24,10 @@ func (c *StreamController) Prepare() { switch { case err == domain.ErrNotFound: beego.Error("MediaFile", c.id, "not found!") - c.SendError(responses.ERROR_DATA_NOT_FOUND) + c.SendError(responses.ErrorDataNotFound) case err != nil: beego.Error("Error reading mediafile", c.id, "from the database", ":", err) - c.SendError(responses.ERROR_GENERIC, "Internal error") + c.SendError(responses.ErrorGeneric, "Internal error") } c.mf = mf diff --git a/api/stream_test.go b/api/stream_test.go index 1f9d7b2a9..3551c3c20 100644 --- a/api/stream_test.go +++ b/api/stream_test.go @@ -37,13 +37,13 @@ func TestStream(t *testing.T) { Convey("Should fail if missing Id parameter", func() { _, w := stream() - So(w.Body, ShouldReceiveError, responses.ERROR_MISSING_PARAMETER) + So(w.Body, ShouldReceiveError, responses.ErrorMissingParameter) }) Convey("When id is not found", func() { mockMediaFileRepo.SetData(`[]`, 1) _, w := stream("id=NOT_FOUND") - So(w.Body, ShouldReceiveError, responses.ERROR_DATA_NOT_FOUND) + So(w.Body, ShouldReceiveError, responses.ErrorDataNotFound) }) Convey("When id is found", func() { mockMediaFileRepo.SetData(`[{"Id":"2","HasCoverArt":true,"Path":"tests/fixtures/01 Invisible (RED) Edit Version.mp3"}]`, 1) diff --git a/api/validation.go b/api/validation.go index 0548628a3..31c1b4d93 100644 --- a/api/validation.go +++ b/api/validation.go @@ -30,7 +30,7 @@ func checkParameters(c BaseAPIController) { for _, p := range requiredParameters { if c.GetString(p) == "" { logWarn(c, fmt.Sprintf(`Missing required parameter "%s"`, p)) - abortRequest(c, responses.ERROR_MISSING_PARAMETER) + abortRequest(c, responses.ErrorMissingParameter) } } } @@ -46,7 +46,7 @@ func authenticate(c BaseAPIController) { } if user != beego.AppConfig.String("user") || pass != beego.AppConfig.String("password") { logWarn(c, fmt.Sprintf(`Invalid login for user "%s"`, user)) - abortRequest(c, responses.ERROR_AUTHENTICATION_FAIL) + abortRequest(c, responses.ErrorAuthenticationFail) } } diff --git a/engine/search.go b/engine/search.go index 53c3cf301..30657b95a 100644 --- a/engine/search.go +++ b/engine/search.go @@ -48,9 +48,16 @@ func NewSearch(ar domain.ArtistRepository, alr domain.AlbumRepository, mr domain } func (s *search) ClearAll() error { - return s.idxArtist.Clear() - return s.idxAlbum.Clear() - return s.idxSong.Clear() + if err := s.idxArtist.Clear(); err != nil { + return err + } + if err := s.idxAlbum.Clear(); err != nil { + return err + } + if err := s.idxSong.Clear(); err != nil { + return err + } + return nil } func (s *search) IndexArtist(ar *domain.Artist) error { diff --git a/engine/stream.go b/engine/stream.go index b53bbe93a..cd82d8830 100644 --- a/engine/stream.go +++ b/engine/stream.go @@ -53,5 +53,5 @@ func createDownsamplingCommand(path string, maxBitRate int) (string, []string) { split[i] = s } - return split[0], split[1:len(split)] + return split[0], split[1:] } diff --git a/persistence/ledis_repository.go b/persistence/ledis_repository.go index 652a0cbe9..01dfedff3 100644 --- a/persistence/ledis_repository.go +++ b/persistence/ledis_repository.go @@ -176,13 +176,13 @@ func (r *ledisRepository) saveOrUpdate(id string, entity interface{}) error { for idx, fn := range r.indexes { idxName := fmt.Sprintf("%s:idx:%s", r.table, idx) score := calcScore(entity, fn) - sidx := ledis.ScorePair{score, []byte(id)} + sidx := ledis.ScorePair{Score: score, Member: []byte(id)} if _, err = Db().ZAdd([]byte(idxName), sidx); err != nil { return err } } - sid := ledis.ScorePair{0, []byte(id)} + sid := ledis.ScorePair{Score: 0, Member: []byte(id)} if _, err = Db().ZAdd([]byte(allKey), sid); err != nil { return err } @@ -190,7 +190,7 @@ func (r *ledisRepository) saveOrUpdate(id string, entity interface{}) error { if parentCollectionKey := r.getParentRelationKey(entity); parentCollectionKey != "" { _, err = Db().ZAdd([]byte(parentCollectionKey), sid) } - return nil + return err } func calcScore(entity interface{}, fieldName string) int64 { diff --git a/utils/index_group_parser.go b/utils/index_group_parser.go index fad0137ca..2ca2f2796 100644 --- a/utils/index_group_parser.go +++ b/utils/index_group_parser.go @@ -1,15 +1,3 @@ -// Let's you specify how the index should look like. -// -// The specification is a space-separated list of index entries. Normally, each entry is just a single character, -// but you may also specify multiple characters. For instance, the entry "The" will link to all files and -// folders starting with "The". -// -// You may also create an entry using a group of index characters in paranthesis. For instance, the entry -// "A-E(ABCDE)" will display as "A-E" and link to all files and folders starting with either -// A, B, C, D or E. This may be useful for grouping less-frequently used characters (such and X, Y and Z), or -// for grouping accented characters (such as A, \u00C0 and \u00C1) -// -// Files and folders that are not covered by an index entry will be placed under the index entry "#". package utils import ( @@ -19,6 +7,16 @@ import ( type IndexGroups map[string]string +// The specification is a space-separated list of index entries. Normally, each entry is just a single character, +// but you may also specify multiple characters. For instance, the entry "The" will link to all files and +// folders starting with "The". +// +// You may also create an entry using a group of index characters in parenthesis. For instance, the entry +// "A-E(ABCDE)" will display as "A-E" and link to all files and folders starting with either +// A, B, C, D or E. This may be useful for grouping less-frequently used characters (such and X, Y and Z), or +// for grouping accented characters (such as A, \u00C0 and \u00C1) +// +// Files and folders that are not covered by an index entry will be placed under the index entry "#". func ParseIndexGroups(spec string) IndexGroups { parsed := make(IndexGroups) split := strings.Split(spec, " ")