Skip to content

Commit

Permalink
Removed more layers of indirection from the engine package
Browse files Browse the repository at this point in the history
  • Loading branch information
deluan committed Oct 27, 2020
1 parent acba4b1 commit 3037ea0
Show file tree
Hide file tree
Showing 7 changed files with 182 additions and 307 deletions.
91 changes: 55 additions & 36 deletions server/subsonic/album_lists.go
@@ -1,66 +1,71 @@
package subsonic

import (
"context"
"errors"
"net/http"

"github.com/deluan/navidrome/log"
"github.com/deluan/navidrome/model"
"github.com/deluan/navidrome/server/subsonic/engine"
"github.com/deluan/navidrome/server/subsonic/filter"
"github.com/deluan/navidrome/server/subsonic/responses"
"github.com/deluan/navidrome/utils"
)

type AlbumListController struct {
ds model.DataStore
listGen engine.ListGenerator
}

func NewAlbumListController(listGen engine.ListGenerator) *AlbumListController {
func NewAlbumListController(ds model.DataStore, listGen engine.ListGenerator) *AlbumListController {
c := &AlbumListController{
ds: ds,
listGen: listGen,
}
return c
}

func (c *AlbumListController) getAlbumList(r *http.Request) (engine.Entries, error) {
func (c *AlbumListController) getAlbumList(r *http.Request) (model.Albums, error) {
typ, err := requiredParamString(r, "type", "Required string parameter 'type' is not present")
if err != nil {
return nil, err
}

var filter engine.ListFilter
var opts filter.Options
switch typ {
case "newest":
filter = engine.ByNewest()
opts = filter.AlbumsByNewest()
case "recent":
filter = engine.ByRecent()
opts = filter.AlbumsByRecent()
case "random":
filter = engine.ByRandom()
opts = filter.AlbumsByRandom()
case "alphabeticalByName":
filter = engine.ByName()
opts = filter.AlbumsByName()
case "alphabeticalByArtist":
filter = engine.ByArtist()
opts = filter.AlbumsByArtist()
case "frequent":
filter = engine.ByFrequent()
opts = filter.AlbumsByFrequent()
case "starred":
filter = engine.ByStarred()
opts = filter.AlbumsByStarred()
case "highest":
filter = engine.ByRating()
opts = filter.AlbumsByRating()
case "byGenre":
filter = engine.ByGenre(utils.ParamString(r, "genre"))
opts = filter.AlbumsByGenre(utils.ParamString(r, "genre"))
case "byYear":
filter = engine.ByYear(utils.ParamInt(r, "fromYear", 0), utils.ParamInt(r, "toYear", 0))
opts = filter.AlbumsByYear(utils.ParamInt(r, "fromYear", 0), utils.ParamInt(r, "toYear", 0))
default:
log.Error(r, "albumList type not implemented", "type", typ)
return nil, errors.New("Not implemented!")
return nil, errors.New("not implemented")
}

offset := utils.ParamInt(r, "offset", 0)
size := utils.MinInt(utils.ParamInt(r, "size", 10), 500)
opts.Offset = utils.ParamInt(r, "offset", 0)
opts.Max = utils.MinInt(utils.ParamInt(r, "size", 10), 500)
albums, err := c.ds.Album(r.Context()).GetAll(model.QueryOptions(opts))

albums, err := c.listGen.GetAlbums(r.Context(), offset, size, filter)
if err != nil {
log.Error(r, "Error retrieving albums", "error", err)
return nil, errors.New("Internal Error")
return nil, errors.New("internal Error")
}

return albums, nil
Expand All @@ -73,7 +78,7 @@ func (c *AlbumListController) GetAlbumList(w http.ResponseWriter, r *http.Reques
}

response := newResponse()
response.AlbumList = &responses.AlbumList{Album: toChildren(r.Context(), albums)}
response.AlbumList = &responses.AlbumList{Album: childrenFromAlbums(r.Context(), albums)}
return response, nil
}

Expand All @@ -84,37 +89,45 @@ func (c *AlbumListController) GetAlbumList2(w http.ResponseWriter, r *http.Reque
}

response := newResponse()
response.AlbumList2 = &responses.AlbumList{Album: toAlbums(r.Context(), albums)}
response.AlbumList2 = &responses.AlbumList{Album: childrenFromAlbums(r.Context(), albums)}
return response, nil
}

func (c *AlbumListController) GetStarred(w http.ResponseWriter, r *http.Request) (*responses.Subsonic, error) {
artists, albums, mediaFiles, err := c.listGen.GetAllStarred(r.Context())
ctx := r.Context()
options := model.QueryOptions{Sort: "starred_at", Order: "desc"}
artists, err := c.ds.Artist(ctx).GetStarred(options)
if err != nil {
log.Error(r, "Error retrieving starred artists", "error", err)
return nil, newError(responses.ErrorGeneric, "Internal Error")
}
albums, err := c.ds.Album(ctx).GetStarred(options)
if err != nil {
log.Error(r, "Error retrieving starred media", "error", err)
log.Error(r, "Error retrieving starred albums", "error", err)
return nil, newError(responses.ErrorGeneric, "Internal Error")
}
mediaFiles, err := c.ds.MediaFile(ctx).GetStarred(options)
if err != nil {
log.Error(r, "Error retrieving starred mediaFiles", "error", err)
return nil, newError(responses.ErrorGeneric, "Internal Error")
}

response := newResponse()
response.Starred = &responses.Starred{}
response.Starred.Artist = toArtists(artists)
response.Starred.Album = toChildren(r.Context(), albums)
response.Starred.Song = toChildren(r.Context(), mediaFiles)
response.Starred.Artist = toArtists(ctx, artists)
response.Starred.Album = childrenFromAlbums(r.Context(), albums)
response.Starred.Song = childrenFromMediaFiles(r.Context(), mediaFiles)
return response, nil
}

func (c *AlbumListController) GetStarred2(w http.ResponseWriter, r *http.Request) (*responses.Subsonic, error) {
artists, albums, mediaFiles, err := c.listGen.GetAllStarred(r.Context())
resp, err := c.GetStarred(w, r)
if err != nil {
log.Error(r, "Error retrieving starred media", "error", err)
return nil, newError(responses.ErrorGeneric, "Internal Error")
return nil, err
}

response := newResponse()
response.Starred2 = &responses.Starred{}
response.Starred2.Artist = toArtists(artists)
response.Starred2.Album = toAlbums(r.Context(), albums)
response.Starred2.Song = toChildren(r.Context(), mediaFiles)
response.Starred2 = resp.Starred
return response, nil
}

Expand Down Expand Up @@ -144,15 +157,15 @@ func (c *AlbumListController) GetRandomSongs(w http.ResponseWriter, r *http.Requ
fromYear := utils.ParamInt(r, "fromYear", 0)
toYear := utils.ParamInt(r, "toYear", 0)

songs, err := c.listGen.GetSongs(r.Context(), 0, size, engine.SongsByRandom(genre, fromYear, toYear))
songs, err := c.getSongs(r.Context(), 0, size, filter.SongsByRandom(genre, fromYear, toYear))
if err != nil {
log.Error(r, "Error retrieving random songs", "error", err)
return nil, newError(responses.ErrorGeneric, "Internal Error")
}

response := newResponse()
response.RandomSongs = &responses.Songs{}
response.RandomSongs.Songs = toChildren(r.Context(), songs)
response.RandomSongs.Songs = childrenFromMediaFiles(r.Context(), songs)
return response, nil
}

Expand All @@ -161,14 +174,20 @@ func (c *AlbumListController) GetSongsByGenre(w http.ResponseWriter, r *http.Req
offset := utils.MinInt(utils.ParamInt(r, "offset", 0), 500)
genre := utils.ParamString(r, "genre")

songs, err := c.listGen.GetSongs(r.Context(), offset, count, engine.SongsByGenre(genre))
songs, err := c.getSongs(r.Context(), offset, count, filter.SongsByGenre(genre))
if err != nil {
log.Error(r, "Error retrieving random songs", "error", err)
return nil, newError(responses.ErrorGeneric, "Internal Error")
}

response := newResponse()
response.SongsByGenre = &responses.Songs{}
response.SongsByGenre.Songs = toChildren(r.Context(), songs)
response.SongsByGenre.Songs = childrenFromMediaFiles(r.Context(), songs)
return response, nil
}

func (c *AlbumListController) getSongs(ctx context.Context, offset, size int, opts filter.Options) (model.MediaFiles, error) {
opts.Offset = offset
opts.Max = size
return c.ds.MediaFile(ctx).GetAll(model.QueryOptions(opts))
}
55 changes: 19 additions & 36 deletions server/subsonic/album_lists_test.go
Expand Up @@ -2,55 +2,40 @@ package subsonic

import (
"context"
"errors"
"net/http/httptest"

"github.com/deluan/navidrome/server/subsonic/engine"
"github.com/deluan/navidrome/log"
"github.com/deluan/navidrome/model"
"github.com/deluan/navidrome/persistence"
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
)

type fakeListGen struct {
engine.ListGenerator
data engine.Entries
err error
recvOffset int
recvSize int
}

func (lg *fakeListGen) GetAlbums(ctx context.Context, offset int, size int, filter engine.ListFilter) (engine.Entries, error) {
if lg.err != nil {
return nil, lg.err
}
lg.recvOffset = offset
lg.recvSize = size
return lg.data, nil
}

var _ = Describe("AlbumListController", func() {
var controller *AlbumListController
var listGen *fakeListGen
var ds model.DataStore
var mockRepo *persistence.MockAlbum
var w *httptest.ResponseRecorder
ctx := log.NewContext(context.TODO())

BeforeEach(func() {
listGen = &fakeListGen{}
controller = NewAlbumListController(listGen)
ds = &persistence.MockDataStore{}
mockRepo = ds.Album(ctx).(*persistence.MockAlbum)
controller = NewAlbumListController(ds, nil)
w = httptest.NewRecorder()
})

Describe("GetAlbumList", func() {
It("should return list of the type specified", func() {
r := newGetRequest("type=newest", "offset=10", "size=20")
listGen.data = engine.Entries{
{Id: "1"}, {Id: "2"},
}
mockRepo.SetData(`[{"id": "1"},{"id": "2"}]`)
resp, err := controller.GetAlbumList(w, r)

Expect(err).To(BeNil())
Expect(resp.AlbumList.Album[0].Id).To(Equal("1"))
Expect(resp.AlbumList.Album[1].Id).To(Equal("2"))
Expect(listGen.recvOffset).To(Equal(10))
Expect(listGen.recvSize).To(Equal(20))
Expect(mockRepo.Options.Offset).To(Equal(10))
Expect(mockRepo.Options.Max).To(Equal(20))
})

It("should fail if missing type parameter", func() {
Expand All @@ -61,28 +46,26 @@ var _ = Describe("AlbumListController", func() {
})

It("should return error if call fails", func() {
listGen.err = errors.New("some issue")
mockRepo.SetError(true)
r := newGetRequest("type=newest")

_, err := controller.GetAlbumList(w, r)

Expect(err).To(MatchError("Internal Error"))
Expect(err).ToNot(BeNil())
})
})

Describe("GetAlbumList2", func() {
It("should return list of the type specified", func() {
r := newGetRequest("type=newest", "offset=10", "size=20")
listGen.data = engine.Entries{
{Id: "1"}, {Id: "2"},
}
mockRepo.SetData(`[{"id": "1"},{"id": "2"}]`)
resp, err := controller.GetAlbumList2(w, r)

Expect(err).To(BeNil())
Expect(resp.AlbumList2.Album[0].Id).To(Equal("1"))
Expect(resp.AlbumList2.Album[1].Id).To(Equal("2"))
Expect(listGen.recvOffset).To(Equal(10))
Expect(listGen.recvSize).To(Equal(20))
Expect(mockRepo.Options.Offset).To(Equal(10))
Expect(mockRepo.Options.Max).To(Equal(20))
})

It("should fail if missing type parameter", func() {
Expand All @@ -93,12 +76,12 @@ var _ = Describe("AlbumListController", func() {
})

It("should return error if call fails", func() {
listGen.err = errors.New("some issue")
mockRepo.SetError(true)
r := newGetRequest("type=newest")

_, err := controller.GetAlbumList2(w, r)

Expect(err).To(MatchError("Internal Error"))
Expect(err).ToNot(BeNil())
})
})
})
55 changes: 0 additions & 55 deletions server/subsonic/engine/common.go
Expand Up @@ -45,43 +45,6 @@ type Entry struct {

type Entries []Entry

func FromArtist(ar *model.Artist) Entry {
e := Entry{}
e.Id = ar.ID
e.Title = ar.Name
e.AlbumCount = ar.AlbumCount
e.IsDir = true
e.UserRating = ar.Rating
if ar.Starred {
e.Starred = ar.StarredAt
}
return e
}

func FromAlbum(al *model.Album) Entry {
e := Entry{}
e.Id = al.ID
e.Title = al.Name
e.IsDir = true
e.Parent = al.AlbumArtistID
e.Album = al.Name
e.Year = al.MaxYear
e.Artist = al.AlbumArtist
e.Genre = al.Genre
e.CoverArt = al.CoverArtId
e.Created = al.CreatedAt
e.AlbumId = al.ID
e.ArtistId = al.AlbumArtistID
e.Duration = int(al.Duration)
e.SongCount = al.SongCount
if al.Starred {
e.Starred = al.StarredAt
}
e.PlayCount = int32(al.PlayCount)
e.UserRating = al.Rating
return e
}

func FromMediaFile(mf *model.MediaFile) Entry {
e := Entry{}
e.Id = mf.ID
Expand Down Expand Up @@ -133,15 +96,6 @@ func realArtistName(mf *model.MediaFile) string {
return mf.Artist
}

func FromAlbums(albums model.Albums) Entries {
entries := make(Entries, len(albums))
for i := range albums {
al := albums[i]
entries[i] = FromAlbum(&al)
}
return entries
}

func FromMediaFiles(mfs model.MediaFiles) Entries {
entries := make(Entries, len(mfs))
for i := range mfs {
Expand All @@ -150,12 +104,3 @@ func FromMediaFiles(mfs model.MediaFiles) Entries {
}
return entries
}

func FromArtists(ars model.Artists) Entries {
entries := make(Entries, len(ars))
for i := range ars {
ar := ars[i]
entries[i] = FromArtist(&ar)
}
return entries
}

0 comments on commit 3037ea0

Please sign in to comment.