From 5e549255201e622c911621a7b770477b1f5a89be Mon Sep 17 00:00:00 2001 From: Deluan Date: Fri, 16 Jul 2021 17:15:34 -0400 Subject: [PATCH] Add multiple genres to Albums --- model/album.go | 3 +- model/artist.go | 1 - model/mediafile.go | 1 - persistence/album_repository.go | 74 +++++++++++++++++------- persistence/album_repository_test.go | 8 --- persistence/artist_repository.go | 8 --- persistence/artist_repository_test.go | 8 --- persistence/genre_repository.go | 5 +- persistence/genre_repository_test.go | 2 +- persistence/mediafile_repository.go | 8 --- persistence/mediafile_repository_test.go | 6 -- persistence/persistence_suite_test.go | 8 +-- persistence/sql_genres.go | 27 +++++++++ server/subsonic/album_lists.go | 12 ++-- server/subsonic/filter/filters.go | 10 +++- 15 files changed, 102 insertions(+), 79 deletions(-) diff --git a/model/album.go b/model/album.go index f79e5736..e1e4be57 100644 --- a/model/album.go +++ b/model/album.go @@ -22,6 +22,7 @@ type Album struct { Duration float32 `json:"duration"` Size int64 `json:"size"` Genre string `json:"genre"` + Genres Genres `json:"genres"` FullText string `json:"fullText"` SortAlbumName string `json:"sortAlbumName,omitempty"` SortArtistName string `json:"sortArtistName,omitempty"` @@ -42,11 +43,11 @@ type Albums []Album type AlbumRepository interface { CountAll(...QueryOptions) (int64, error) Exists(id string) (bool, error) + Put(*Album) error Get(id string) (*Album, error) FindByArtist(albumArtistId string) (Albums, error) GetAll(...QueryOptions) (Albums, error) GetRandom(...QueryOptions) (Albums, error) - GetStarred(options ...QueryOptions) (Albums, error) Search(q string, offset int, size int) (Albums, error) Refresh(ids ...string) error AnnotatedRepository diff --git a/model/artist.go b/model/artist.go index 86e5f604..ca15e83a 100644 --- a/model/artist.go +++ b/model/artist.go @@ -47,7 +47,6 @@ type ArtistRepository interface { Put(m *Artist) error Get(id string) (*Artist, error) GetAll(options ...QueryOptions) (Artists, error) - GetStarred(options ...QueryOptions) (Artists, error) Search(q string, offset int, size int) (Artists, error) Refresh(ids ...string) error GetIndex() (ArtistIndexes, error) diff --git a/model/mediafile.go b/model/mediafile.go index bb5d863a..f31cda87 100644 --- a/model/mediafile.go +++ b/model/mediafile.go @@ -68,7 +68,6 @@ type MediaFileRepository interface { FindAllByPath(path string) (MediaFiles, error) FindByPath(path string) (*MediaFile, error) FindPathsRecursively(basePath string) ([]string, error) - GetStarred(options ...QueryOptions) (MediaFiles, error) GetRandom(options ...QueryOptions) (MediaFiles, error) Search(q string, offset int, size int) (MediaFiles, error) Delete(id string) error diff --git a/persistence/album_repository.go b/persistence/album_repository.go index f9cc827b..2a3f3df6 100644 --- a/persistence/album_repository.go +++ b/persistence/album_repository.go @@ -89,7 +89,7 @@ func (r *albumRepository) Exists(id string) (bool, error) { } func (r *albumRepository) selectAlbum(options ...model.QueryOptions) SelectBuilder { - return r.newSelectWithAnnotation("album.id", options...).Columns("*") + return r.newSelectWithAnnotation("album.id", options...).Columns("album.*") } func (r *albumRepository) Get(id string) (*model.Album, error) { @@ -101,30 +101,51 @@ func (r *albumRepository) Get(id string) (*model.Album, error) { if len(res) == 0 { return nil, model.ErrNotFound } - return &res[0], nil + err := r.loadAlbumGenres(&res) + return &res[0], err +} + +func (r *albumRepository) Put(m *model.Album) error { + genres := m.Genres + m.Genres = nil + defer func() { m.Genres = genres }() + _, err := r.put(m.ID, m) + if err != nil { + return err + } + return r.updateGenres(m.ID, r.tableName, genres) } func (r *albumRepository) FindByArtist(artistId string) (model.Albums, error) { - sq := r.selectAlbum().Where(Eq{"album_artist_id": artistId}).OrderBy("max_year") - res := model.Albums{} - err := r.queryAll(sq, &res) - return res, err + options := model.QueryOptions{ + Sort: "max_year", + Filters: Eq{"album_artist_id": artistId}, + } + + return r.GetAll(options) } func (r *albumRepository) GetAll(options ...model.QueryOptions) (model.Albums, error) { - sq := r.selectAlbum(options...) + sq := r.selectAlbum(options...). + LeftJoin("album_genres ag on album.id = ag.album_id"). + LeftJoin("genre on ag.genre_id = genre.id"). + GroupBy("album.id") res := model.Albums{} err := r.queryAll(sq, &res) + if err != nil { + return nil, err + } + err = r.loadAlbumGenres(&res) return res, err } // TODO Keep order when paginating func (r *albumRepository) GetRandom(options ...model.QueryOptions) (model.Albums, error) { - sq := r.selectAlbum(options...) - sq = sq.OrderBy("RANDOM()") - results := model.Albums{} - err := r.queryAll(sq, &results) - return results, err + if len(options) == 0 { + options = []model.QueryOptions{{}} + } + options[0].Sort = "random()" + return r.GetAll(options...) } // Return a map of mediafiles that have embedded covers for the given album ids @@ -164,6 +185,7 @@ type refreshAlbum struct { SongArtists string SongArtistIds string AlbumArtistIds string + GenreIds string Years string DiscSubtitles string Comments string @@ -190,9 +212,11 @@ func (r *albumRepository) refresh(ids ...string) error { group_concat(f.artist, ' ') as song_artists, group_concat(f.artist_id, ' ') as song_artist_ids, group_concat(f.album_artist_id, ' ') as album_artist_ids, - group_concat(f.year, ' ') as years`). + group_concat(f.year, ' ') as years, + group_concat(mg.genre_id, ' ') as genre_ids`). From("media_file f"). LeftJoin("album a on f.album_id = a.id"). + LeftJoin("media_file_genres mg on mg.media_file_id = f.id"). Where(Eq{"f.album_id": ids}).GroupBy("f.album_id") err := r.queryAll(sel, &albums) if err != nil { @@ -246,7 +270,8 @@ func (r *albumRepository) refresh(ids ...string) error { al.AllArtistIDs = utils.SanitizeStrings(al.SongArtistIds, al.AlbumArtistID, al.ArtistID) al.FullText = getFullText(al.Name, al.Artist, al.AlbumArtist, al.SongArtists, al.SortAlbumName, al.SortArtistName, al.SortAlbumArtistName, al.DiscSubtitles) - _, err := r.put(al.ID, al.Album) + al.Genres = getGenres(al.GenreIds) + err := r.Put(&al.Album) if err != nil { return err } @@ -260,6 +285,20 @@ func (r *albumRepository) refresh(ids ...string) error { return err } +func getGenres(genreIds string) model.Genres { + ids := strings.Fields(genreIds) + var genres model.Genres + unique := map[string]struct{}{} + for _, id := range ids { + if _, ok := unique[id]; ok { + continue + } + genres = append(genres, model.Genre{ID: id}) + unique[id] = struct{}{} + } + return genres +} + func getAlbumArtist(al refreshAlbum) (id, name string) { if !al.Compilation { if al.AlbumArtist != "" { @@ -358,13 +397,6 @@ func (r *albumRepository) purgeEmpty() error { return err } -func (r *albumRepository) GetStarred(options ...model.QueryOptions) (model.Albums, error) { - sq := r.selectAlbum(options...).Where("starred = true") - starred := model.Albums{} - err := r.queryAll(sq, &starred) - return starred, err -} - func (r *albumRepository) Search(q string, offset int, size int) (model.Albums, error) { results := model.Albums{} err := r.doSearch(q, offset, size, &results, "name") diff --git a/persistence/album_repository_test.go b/persistence/album_repository_test.go index 84f3019f..a54ef224 100644 --- a/persistence/album_repository_test.go +++ b/persistence/album_repository_test.go @@ -62,14 +62,6 @@ var _ = Describe("AlbumRepository", func() { }) }) - Describe("GetStarred", func() { - It("returns all starred records", func() { - Expect(repo.GetStarred(model.QueryOptions{})).To(Equal(model.Albums{ - albumRadioactivity, - })) - }) - }) - Describe("FindByArtist", func() { It("returns all records from a given ArtistID", func() { Expect(repo.FindByArtist("3")).To(Equal(model.Albums{ diff --git a/persistence/artist_repository.go b/persistence/artist_repository.go index 76119c3c..ef86ef3b 100644 --- a/persistence/artist_repository.go +++ b/persistence/artist_repository.go @@ -213,14 +213,6 @@ func (r *artistRepository) refresh(ids ...string) error { return err } -func (r *artistRepository) GetStarred(options ...model.QueryOptions) (model.Artists, error) { - sq := r.selectArtist(options...).Where("starred = true") - var dba []dbArtist - err := r.queryAll(sq, &dba) - starred := r.toModels(dba) - return starred, err -} - func (r *artistRepository) purgeEmpty() error { del := Delete(r.tableName).Where("id not in (select distinct(album_artist_id) from album)") c, err := r.executeSQL(del) diff --git a/persistence/artist_repository_test.go b/persistence/artist_repository_test.go index 45196c33..e00db60c 100644 --- a/persistence/artist_repository_test.go +++ b/persistence/artist_repository_test.go @@ -42,14 +42,6 @@ var _ = Describe("ArtistRepository", func() { }) }) - Describe("GetStarred", func() { - It("returns all starred records", func() { - Expect(repo.GetStarred(model.QueryOptions{})).To(Equal(model.Artists{ - artistBeatles, - })) - }) - }) - Describe("GetIndex", func() { It("returns the index", func() { idx, err := repo.GetIndex() diff --git a/persistence/genre_repository.go b/persistence/genre_repository.go index d1bf4140..3b4eb61d 100644 --- a/persistence/genre_repository.go +++ b/persistence/genre_repository.go @@ -25,11 +25,10 @@ func NewGenreRepository(ctx context.Context, o orm.Ormer) model.GenreRepository func (r *genreRepository) GetAll() (model.Genres, error) { sq := Select("*", - "(select count(1) from album where album.genre = genre.name) as album_count", + "count(distinct a.album_id) as album_count", "count(distinct f.media_file_id) as song_count"). From(r.tableName). - // TODO Use relation table - // LeftJoin("album_genres a on a.genre_id = genre.id"). + LeftJoin("album_genres a on a.genre_id = genre.id"). LeftJoin("media_file_genres f on f.genre_id = genre.id"). GroupBy("genre.id") res := model.Genres{} diff --git a/persistence/genre_repository_test.go b/persistence/genre_repository_test.go index d86cf1b8..4d3b8fa4 100644 --- a/persistence/genre_repository_test.go +++ b/persistence/genre_repository_test.go @@ -23,7 +23,7 @@ var _ = Describe("GenreRepository", func() { Expect(err).To(BeNil()) Expect(genres).To(ConsistOf( model.Genre{ID: "gn-1", Name: "Electronic", AlbumCount: 1, SongCount: 2}, - model.Genre{ID: "gn-2", Name: "Rock", AlbumCount: 2, SongCount: 3}, + model.Genre{ID: "gn-2", Name: "Rock", AlbumCount: 3, SongCount: 3}, )) }) }) diff --git a/persistence/mediafile_repository.go b/persistence/mediafile_repository.go index 758df203..69492c0a 100644 --- a/persistence/mediafile_repository.go +++ b/persistence/mediafile_repository.go @@ -161,14 +161,6 @@ func (r *mediaFileRepository) deleteNotInPath(basePath string) error { return err } -func (r *mediaFileRepository) GetStarred(options ...model.QueryOptions) (model.MediaFiles, error) { - if len(options) == 0 { - options = []model.QueryOptions{{}} - } - options[0].Filters = Eq{"starred": true} - return r.GetAll(options...) -} - // TODO Keep order when paginating func (r *mediaFileRepository) GetRandom(options ...model.QueryOptions) (model.MediaFiles, error) { if len(options) == 0 { diff --git a/persistence/mediafile_repository_test.go b/persistence/mediafile_repository_test.go index 8d0cbbc4..496a034c 100644 --- a/persistence/mediafile_repository_test.go +++ b/persistence/mediafile_repository_test.go @@ -86,12 +86,6 @@ var _ = Describe("MediaRepository", func() { Expect(found[0].ID).To(Equal("7004")) }) - It("returns starred tracks", func() { - Expect(mr.GetStarred()).To(Equal(model.MediaFiles{ - songComeTogether, - })) - }) - It("delete tracks by id", func() { id := uuid.NewString() Expect(mr.Put(&model.MediaFile{ID: id})).To(BeNil()) diff --git a/persistence/persistence_suite_test.go b/persistence/persistence_suite_test.go index 62585bd5..b5ee52cb 100644 --- a/persistence/persistence_suite_test.go +++ b/persistence/persistence_suite_test.go @@ -46,9 +46,9 @@ var ( ) var ( - albumSgtPeppers = model.Album{ID: "101", Name: "Sgt Peppers", Artist: "The Beatles", OrderAlbumName: "sgt peppers", AlbumArtistID: "3", Genre: "Rock", CoverArtId: "1", CoverArtPath: P("/beatles/1/sgt/a day.mp3"), SongCount: 1, MaxYear: 1967, FullText: " beatles peppers sgt the"} - albumAbbeyRoad = model.Album{ID: "102", Name: "Abbey Road", Artist: "The Beatles", OrderAlbumName: "abbey road", AlbumArtistID: "3", Genre: "Rock", CoverArtId: "2", CoverArtPath: P("/beatles/1/come together.mp3"), SongCount: 1, MaxYear: 1969, FullText: " abbey beatles road the"} - albumRadioactivity = model.Album{ID: "103", Name: "Radioactivity", Artist: "Kraftwerk", OrderAlbumName: "radioactivity", AlbumArtistID: "2", Genre: "Electronic", CoverArtId: "3", CoverArtPath: P("/kraft/radio/radio.mp3"), SongCount: 2, FullText: " kraftwerk radioactivity"} + albumSgtPeppers = model.Album{ID: "101", Name: "Sgt Peppers", Artist: "The Beatles", OrderAlbumName: "sgt peppers", AlbumArtistID: "3", Genre: "Rock", Genres: model.Genres{genreRock}, CoverArtId: "1", CoverArtPath: P("/beatles/1/sgt/a day.mp3"), SongCount: 1, MaxYear: 1967, FullText: " beatles peppers sgt the"} + albumAbbeyRoad = model.Album{ID: "102", Name: "Abbey Road", Artist: "The Beatles", OrderAlbumName: "abbey road", AlbumArtistID: "3", Genre: "Rock", Genres: model.Genres{genreRock}, CoverArtId: "2", CoverArtPath: P("/beatles/1/come together.mp3"), SongCount: 1, MaxYear: 1969, FullText: " abbey beatles road the"} + albumRadioactivity = model.Album{ID: "103", Name: "Radioactivity", Artist: "Kraftwerk", OrderAlbumName: "radioactivity", AlbumArtistID: "2", Genre: "Electronic", Genres: model.Genres{genreElectronic, genreRock}, CoverArtId: "3", CoverArtPath: P("/kraft/radio/radio.mp3"), SongCount: 2, FullText: " kraftwerk radioactivity"} testAlbums = model.Albums{ albumSgtPeppers, albumAbbeyRoad, @@ -115,7 +115,7 @@ var _ = Describe("Initialize test DB", func() { alr := NewAlbumRepository(ctx, o).(*albumRepository) for i := range testAlbums { a := testAlbums[i] - _, err := alr.put(a.ID, &a) + err := alr.Put(&a) if err != nil { panic(err) } diff --git a/persistence/sql_genres.go b/persistence/sql_genres.go index a23089c0..67ed0775 100644 --- a/persistence/sql_genres.go +++ b/persistence/sql_genres.go @@ -54,3 +54,30 @@ func (r *sqlRepository) loadMediaFileGenres(mfs *model.MediaFiles) error { } return nil } + +func (r *sqlRepository) loadAlbumGenres(mfs *model.Albums) error { + var ids []string + m := map[string]*model.Album{} + for i := range *mfs { + mf := &(*mfs)[i] + ids = append(ids, mf.ID) + m[mf.ID] = mf + } + + sql := Select("g.*", "ag.album_id").From("genre g").Join("album_genres ag on ag.genre_id = g.id"). + Where(Eq{"ag.album_id": ids}).OrderBy("ag.album_id", "ag.rowid") + var genres []struct { + model.Genre + AlbumId string + } + + err := r.queryAll(sql, &genres) + if err != nil { + return err + } + for _, g := range genres { + mf := m[g.AlbumId] + mf.Genres = append(mf.Genres, g.Genre) + } + return nil +} diff --git a/server/subsonic/album_lists.go b/server/subsonic/album_lists.go index 8b1e5fef..95eb50fe 100644 --- a/server/subsonic/album_lists.go +++ b/server/subsonic/album_lists.go @@ -62,7 +62,7 @@ func (c *AlbumListController) getAlbumList(r *http.Request) (model.Albums, error 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.ds.Album(r.Context()).GetAll(opts) if err != nil { log.Error(r, "Error retrieving albums", "error", err) @@ -96,18 +96,18 @@ func (c *AlbumListController) GetAlbumList2(w http.ResponseWriter, r *http.Reque func (c *AlbumListController) GetStarred(w http.ResponseWriter, r *http.Request) (*responses.Subsonic, error) { ctx := r.Context() - options := model.QueryOptions{Sort: "starred_at", Order: "desc"} - artists, err := c.ds.Artist(ctx).GetStarred(options) + options := filter.Starred() + artists, err := c.ds.Artist(ctx).GetAll(options) if err != nil { log.Error(r, "Error retrieving starred artists", "error", err) return nil, err } - albums, err := c.ds.Album(ctx).GetStarred(options) + albums, err := c.ds.Album(ctx).GetAll(options) if err != nil { log.Error(r, "Error retrieving starred albums", "error", err) return nil, err } - mediaFiles, err := c.ds.MediaFile(ctx).GetStarred(options) + mediaFiles, err := c.ds.MediaFile(ctx).GetAll(options) if err != nil { log.Error(r, "Error retrieving starred mediaFiles", "error", err) return nil, err @@ -196,5 +196,5 @@ func (c *AlbumListController) GetSongsByGenre(w http.ResponseWriter, r *http.Req 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)) + return c.ds.MediaFile(ctx).GetAll(opts) } diff --git a/server/subsonic/filter/filters.go b/server/subsonic/filter/filters.go index a0561ba1..b1dd01a9 100644 --- a/server/subsonic/filter/filters.go +++ b/server/subsonic/filter/filters.go @@ -7,7 +7,7 @@ import ( "github.com/navidrome/navidrome/model" ) -type Options model.QueryOptions +type Options = model.QueryOptions func AlbumsByNewest() Options { return Options{Sort: "recently_added", Order: "desc"} @@ -43,8 +43,8 @@ func AlbumsByRating() Options { func AlbumsByGenre(genre string) Options { return Options{ - Sort: "genre asc, name asc", - Filters: squirrel.Eq{"genre": genre}, + Sort: "genre.name asc, name asc", + Filters: squirrel.Eq{"genre.name": genre}, } } @@ -93,3 +93,7 @@ func SongsByRandom(genre string, fromYear, toYear int) Options { options.Filters = ff return options } + +func Starred() Options { + return Options{Sort: "starred_at", Order: "desc", Filters: squirrel.Eq{"starred": true}} +}