From e10e730af13749673435c4f6e3025133271f6555 Mon Sep 17 00:00:00 2001 From: Deluan Date: Tue, 8 Mar 2016 08:48:47 -0500 Subject: [PATCH] Refactored logic from getMusicDirectory.view to the new engine layer. This reveled a nasty bug in the DI code. Tests are broken --- api/get_music_directory.go | 140 +++++++-------------------- api/get_music_directory_test.go | 1 + conf/inject_definitions.go | 10 +- engine/browser.go | 164 +++++++++++++++++++++++++++++++- tests/mocks/mock_album_repo.go | 4 +- tests/mocks/mock_artist_repo.go | 4 +- 6 files changed, 200 insertions(+), 123 deletions(-) diff --git a/api/get_music_directory.go b/api/get_music_directory.go index 7f5552a98..4c55b9215 100644 --- a/api/get_music_directory.go +++ b/api/get_music_directory.go @@ -3,23 +3,18 @@ package api import ( "github.com/astaxie/beego" "github.com/deluan/gosonic/api/responses" - "github.com/deluan/gosonic/domain" + "github.com/deluan/gosonic/engine" "github.com/deluan/gosonic/utils" "github.com/karlkfi/inject" - "time" ) type GetMusicDirectoryController struct { BaseAPIController - artistRepo domain.ArtistRepository - albumRepo domain.AlbumRepository - mFileRepo domain.MediaFileRepository + browser engine.Browser } func (c *GetMusicDirectoryController) Prepare() { - inject.ExtractAssignable(utils.Graph, &c.artistRepo) - inject.ExtractAssignable(utils.Graph, &c.albumRepo) - inject.ExtractAssignable(utils.Graph, &c.mFileRepo) + inject.ExtractAssignable(utils.Graph, &c.browser) } func (c *GetMusicDirectoryController) Get() { @@ -27,115 +22,44 @@ func (c *GetMusicDirectoryController) Get() { response := c.NewEmpty() + dir, err := c.browser.Directory(id) switch { - case c.isArtist(id): - a, albums := c.retrieveArtist(id) - response.Directory = c.buildArtistDir(a, albums) - case c.isAlbum(id): - al, tracks := c.retrieveAlbum(id) - response.Directory = c.buildAlbumDir(al, tracks) - default: - beego.Info("Id", id, "not found") + case err == engine.DataNotFound: + beego.Error(err, "Id:", id) c.SendError(responses.ERROR_DATA_NOT_FOUND, "Directory not found") + case err != nil: + beego.Error(err) + c.SendError(responses.ERROR_GENERIC, "Internal Error") } + response.Directory = c.buildDirectory(dir) + c.SendResponse(response) } -func (c *GetMusicDirectoryController) buildArtistDir(a *domain.Artist, albums []domain.Album) *responses.Directory { - dir := &responses.Directory{Id: a.Id, Name: a.Name} +func (c *GetMusicDirectoryController) buildDirectory(d *engine.DirectoryInfo) *responses.Directory { + dir := &responses.Directory{Id: d.Id, Name: d.Name} - dir.Child = make([]responses.Child, len(albums)) - for i, al := range albums { - dir.Child[i].Id = al.Id - dir.Child[i].Title = al.Name - dir.Child[i].IsDir = true - dir.Child[i].Parent = al.ArtistId - dir.Child[i].Album = al.Name - dir.Child[i].Year = al.Year - dir.Child[i].Artist = al.AlbumArtist - dir.Child[i].Genre = al.Genre - dir.Child[i].CoverArt = al.CoverArtId - if al.Starred { - t := time.Now() - dir.Child[i].Starred = &t + dir.Child = make([]responses.Child, len(d.Children)) + for i, child := range d.Children { + dir.Child[i].Id = child.Id + dir.Child[i].Title = child.Title + dir.Child[i].IsDir = child.IsDir + dir.Child[i].Parent = child.Parent + dir.Child[i].Album = child.Album + dir.Child[i].Year = child.Year + dir.Child[i].Artist = child.Artist + dir.Child[i].Genre = child.Genre + dir.Child[i].CoverArt = child.CoverArt + dir.Child[i].Track = child.Track + dir.Child[i].Duration = child.Duration + dir.Child[i].Size = child.Size + dir.Child[i].Suffix = child.Suffix + dir.Child[i].BitRate = child.BitRate + dir.Child[i].ContentType = child.ContentType + if !child.Starred.IsZero() { + dir.Child[i].Starred = &child.Starred } - } return dir } - -func (c *GetMusicDirectoryController) buildAlbumDir(al *domain.Album, tracks []domain.MediaFile) *responses.Directory { - dir := &responses.Directory{Id: al.Id, Name: al.Name} - - dir.Child = make([]responses.Child, len(tracks)) - for i, mf := range tracks { - dir.Child[i].Id = mf.Id - dir.Child[i].Title = mf.Title - dir.Child[i].IsDir = false - dir.Child[i].Parent = mf.AlbumId - dir.Child[i].Album = mf.Album - dir.Child[i].Year = mf.Year - dir.Child[i].Artist = mf.Artist - dir.Child[i].Genre = mf.Genre - dir.Child[i].Track = mf.TrackNumber - dir.Child[i].Duration = mf.Duration - dir.Child[i].Size = mf.Size - dir.Child[i].Suffix = mf.Suffix - dir.Child[i].BitRate = mf.BitRate - if mf.Starred { - dir.Child[i].Starred = &mf.UpdatedAt - } - if mf.HasCoverArt { - dir.Child[i].CoverArt = mf.Id - } - dir.Child[i].ContentType = mf.ContentType() - } - return dir -} - -func (c *GetMusicDirectoryController) isArtist(id string) bool { - found, err := c.artistRepo.Exists(id) - if err != nil { - beego.Error("Error searching for Artist", id, " - ", err) - c.SendError(responses.ERROR_GENERIC, "Internal Error") - } - return found -} - -func (c *GetMusicDirectoryController) isAlbum(id string) bool { - found, err := c.albumRepo.Exists(id) - if err != nil { - beego.Error("Error searching for Album:", err) - c.SendError(responses.ERROR_GENERIC, "Internal Error") - } - return found -} - -func (c *GetMusicDirectoryController) retrieveArtist(id string) (a *domain.Artist, as []domain.Album) { - a, err := c.artistRepo.Get(id) - if err != nil { - beego.Error("Error reading Artist from DB", err) - c.SendError(responses.ERROR_GENERIC, "Internal Error") - } - - if as, err = c.albumRepo.FindByArtist(id); err != nil { - beego.Error("Error reading Artist's Albums from DB", err) - c.SendError(responses.ERROR_GENERIC, "Internal Error") - } - return -} - -func (c *GetMusicDirectoryController) retrieveAlbum(id string) (al *domain.Album, mfs []domain.MediaFile) { - al, err := c.albumRepo.Get(id) - if err != nil { - beego.Error("Error reading Album from DB", err) - c.SendError(responses.ERROR_GENERIC, "Internal Error") - } - - if mfs, err = c.mFileRepo.FindByAlbum(id); err != nil { - beego.Error("Error reading Album's Tracks from DB", err) - c.SendError(responses.ERROR_GENERIC, "Internal Error") - } - return -} diff --git a/api/get_music_directory_test.go b/api/get_music_directory_test.go index 8f1f1a868..2cd1d8070 100644 --- a/api/get_music_directory_test.go +++ b/api/get_music_directory_test.go @@ -35,6 +35,7 @@ func TestGetMusicDirectory(t *testing.T) { }) Convey("Id is for an artist", func() { Convey("Return fail on Artist Table error", func() { + mockArtistRepo.SetData(`[{"Id":"1","Name":"The Charlatans"}]`, 1) mockArtistRepo.SetError(true) _, w := Get(AddParams("/rest/getMusicDirectory.view", "id=1"), "TestGetMusicDirectory") diff --git a/conf/inject_definitions.go b/conf/inject_definitions.go index 86d1f5318..8e13b1f1a 100644 --- a/conf/inject_definitions.go +++ b/conf/inject_definitions.go @@ -11,11 +11,11 @@ func init() { // Persistence ir := utils.DefineSingleton(new(domain.ArtistIndexRepository), persistence.NewArtistIndexRepository) pr := utils.DefineSingleton(new(domain.PropertyRepository), persistence.NewPropertyRepository) - mfr := utils.DefineSingleton(new(domain.MediaFolderRepository), persistence.NewMediaFolderRepository) - utils.DefineSingleton(new(domain.ArtistRepository), persistence.NewArtistRepository) - utils.DefineSingleton(new(domain.AlbumRepository), persistence.NewAlbumRepository) - utils.DefineSingleton(new(domain.MediaFileRepository), persistence.NewMediaFileRepository) + fr := utils.DefineSingleton(new(domain.MediaFolderRepository), persistence.NewMediaFolderRepository) + ar := utils.DefineSingleton(new(domain.ArtistRepository), persistence.NewArtistRepository) + alr := utils.DefineSingleton(new(domain.AlbumRepository), persistence.NewAlbumRepository) + mr := utils.DefineSingleton(new(domain.MediaFileRepository), persistence.NewMediaFileRepository) // Engine (Use cases) - utils.DefineSingleton(new(engine.Browser), engine.NewBrowser, pr, mfr, ir) + utils.DefineSingleton(new(engine.Browser), engine.NewBrowser, pr, fr, ir, ar, alr, mr) } diff --git a/engine/browser.go b/engine/browser.go index 1aa85e06a..1ed9d5b09 100644 --- a/engine/browser.go +++ b/engine/browser.go @@ -3,26 +3,37 @@ package engine import ( "errors" "fmt" + "strconv" + "time" + + "github.com/astaxie/beego" "github.com/deluan/gosonic/consts" "github.com/deluan/gosonic/domain" "github.com/deluan/gosonic/utils" - "strconv" - "time" +) + +var ( + DataNotFound = errors.New("Data Not Found") ) type Browser interface { MediaFolders() (domain.MediaFolders, error) Indexes(ifModifiedSince time.Time) (domain.ArtistIndexes, time.Time, error) + Directory(id string) (*DirectoryInfo, error) } -func NewBrowser(propRepo domain.PropertyRepository, folderRepo domain.MediaFolderRepository, indexRepo domain.ArtistIndexRepository) Browser { - return browser{propRepo, folderRepo, indexRepo} +func NewBrowser(pr domain.PropertyRepository, fr domain.MediaFolderRepository, ir domain.ArtistIndexRepository, + ar domain.ArtistRepository, alr domain.AlbumRepository, mr domain.MediaFileRepository) Browser { + return browser{pr, fr, ir, ar, alr, mr} } type browser struct { propRepo domain.PropertyRepository folderRepo domain.MediaFolderRepository indexRepo domain.ArtistIndexRepository + artistRepo domain.ArtistRepository + albumRepo domain.AlbumRepository + mfileRepo domain.MediaFileRepository } func (b browser) MediaFolders() (domain.MediaFolders, error) { @@ -45,3 +56,148 @@ func (b browser) Indexes(ifModifiedSince time.Time) (domain.ArtistIndexes, time. return domain.ArtistIndexes{}, lastModified, nil } + +type Child struct { + Id string + Title string + IsDir bool + Parent string + Album string + Year int + Artist string + Genre string + CoverArt string + Starred time.Time + Track int + Duration int + Size string + Suffix string + BitRate int + ContentType string +} + +type DirectoryInfo struct { + Id string + Name string + Children []Child +} + +func (c browser) Directory(id string) (*DirectoryInfo, error) { + var dir *DirectoryInfo + switch { + case c.isArtist(id): + beego.Info("Found Artist with id", id) + a, albums, err := c.retrieveArtist(id) + if err != nil { + return nil, err + } + dir = c.buildArtistDir(a, albums) + case c.isAlbum(id): + beego.Info("Found Album with id", id) + al, tracks, err := c.retrieveAlbum(id) + if err != nil { + return nil, err + } + dir = c.buildAlbumDir(al, tracks) + default: + beego.Info("Id", id, "not found") + return nil, DataNotFound + } + + return dir, nil +} + +func (c browser) buildArtistDir(a *domain.Artist, albums []domain.Album) *DirectoryInfo { + dir := &DirectoryInfo{Id: a.Id, Name: a.Name} + + dir.Children = make([]Child, len(albums)) + for i, al := range albums { + dir.Children[i].Id = al.Id + dir.Children[i].Title = al.Name + dir.Children[i].IsDir = true + dir.Children[i].Parent = al.ArtistId + dir.Children[i].Album = al.Name + dir.Children[i].Year = al.Year + dir.Children[i].Artist = al.AlbumArtist + dir.Children[i].Genre = al.Genre + dir.Children[i].CoverArt = al.CoverArtId + if al.Starred { + dir.Children[i].Starred = al.UpdatedAt + } + + } + return dir +} + +func (c browser) buildAlbumDir(al *domain.Album, tracks []domain.MediaFile) *DirectoryInfo { + dir := &DirectoryInfo{Id: al.Id, Name: al.Name} + + dir.Children = make([]Child, len(tracks)) + for i, mf := range tracks { + dir.Children[i].Id = mf.Id + dir.Children[i].Title = mf.Title + dir.Children[i].IsDir = false + dir.Children[i].Parent = mf.AlbumId + dir.Children[i].Album = mf.Album + dir.Children[i].Year = mf.Year + dir.Children[i].Artist = mf.Artist + dir.Children[i].Genre = mf.Genre + dir.Children[i].Track = mf.TrackNumber + dir.Children[i].Duration = mf.Duration + dir.Children[i].Size = mf.Size + dir.Children[i].Suffix = mf.Suffix + dir.Children[i].BitRate = mf.BitRate + if mf.Starred { + dir.Children[i].Starred = mf.UpdatedAt + } + if mf.HasCoverArt { + dir.Children[i].CoverArt = mf.Id + } + dir.Children[i].ContentType = mf.ContentType() + } + return dir +} + +func (c browser) isArtist(id string) bool { + found, err := c.artistRepo.Exists(id) + if err != nil { + beego.Error(fmt.Errorf("Error searching for Artist %s: %v", id, err)) + return false + } + return found +} + +func (c browser) isAlbum(id string) bool { + found, err := c.albumRepo.Exists(id) + if err != nil { + beego.Error(fmt.Errorf("Error searching for Album %s: %v", id, err)) + return false + } + return found +} + +func (c browser) retrieveArtist(id string) (a *domain.Artist, as []domain.Album, err error) { + a, err = c.artistRepo.Get(id) + if err != nil { + err = fmt.Errorf("Error reading Artist %s from DB: %v", id, err) + return + } + + if as, err = c.albumRepo.FindByArtist(id); err != nil { + err = fmt.Errorf("Error reading %s's albums from DB: %v", a.Name, err) + } + return +} + +func (c browser) retrieveAlbum(id string) (al *domain.Album, mfs []domain.MediaFile, err error) { + al, err = c.albumRepo.Get(id) + if err != nil { + err = fmt.Errorf("Error reading Album %s from DB: %v", id, err) + return + } + + if mfs, err = c.mfileRepo.FindByAlbum(id); err != nil { + err = fmt.Errorf("Error reading %s's tracks from DB: %v", al.Name, err) + } + return +} diff --git a/tests/mocks/mock_album_repo.go b/tests/mocks/mock_album_repo.go index f3c923f48..e4aa304fe 100644 --- a/tests/mocks/mock_album_repo.go +++ b/tests/mocks/mock_album_repo.go @@ -4,6 +4,7 @@ import ( "encoding/json" "errors" "fmt" + "github.com/deluan/gosonic/domain" ) @@ -36,9 +37,6 @@ func (m *MockAlbum) SetData(j string, size int) { } func (m *MockAlbum) Exists(id string) (bool, error) { - if m.err { - return false, errors.New("Error!") - } _, found := m.data[id] return found, nil } diff --git a/tests/mocks/mock_artist_repo.go b/tests/mocks/mock_artist_repo.go index 29cf51786..2d97fd71d 100644 --- a/tests/mocks/mock_artist_repo.go +++ b/tests/mocks/mock_artist_repo.go @@ -4,6 +4,7 @@ import ( "encoding/json" "errors" "fmt" + "github.com/deluan/gosonic/domain" ) @@ -34,9 +35,6 @@ func (m *MockArtist) SetData(j string, size int) { } func (m *MockArtist) Exists(id string) (bool, error) { - if m.err { - return false, errors.New("Error!") - } _, found := m.data[id] return found, nil }