From 61e5523457641fd662f69ff569f38671a61f9301 Mon Sep 17 00:00:00 2001 From: Deluan Date: Wed, 28 Dec 2022 12:32:46 -0500 Subject: [PATCH] Handle "naked" CoverArtIDs (IDs of album, mediafiles and playlists) --- cmd/wire_gen.go | 2 +- cmd/wire_injectors.go | 2 ++ core/artwork/artwork.go | 37 ++++++++++++++++++++++++----- core/artwork/reader_playlist.go | 8 +++---- core/artwork/wire_providers.go | 11 +++++++++ core/external_metadata.go | 2 +- core/wire_providers.go | 4 ---- model/artwork_id.go | 4 ++++ {core => model}/get_entity.go | 6 ++--- server/subsonic/browsing.go | 3 +-- server/subsonic/media_annotation.go | 3 +-- server/subsonic/stream.go | 2 +- utils/slice/slice.go | 8 +++++++ utils/slice/slice_test.go | 15 ++++++++++++ 14 files changed, 82 insertions(+), 25 deletions(-) create mode 100644 core/artwork/wire_providers.go rename {core => model}/get_entity.go (72%) diff --git a/cmd/wire_gen.go b/cmd/wire_gen.go index f0db7a774..6cacfe2c7 100644 --- a/cmd/wire_gen.go +++ b/cmd/wire_gen.go @@ -92,7 +92,7 @@ func createScanner() scanner.Scanner { // wire_injectors.go: -var allProviders = wire.NewSet(core.Set, subsonic.New, nativeapi.New, persistence.New, lastfm.NewRouter, listenbrainz.NewRouter, events.GetBroker, db.Db) +var allProviders = wire.NewSet(core.Set, artwork.Set, subsonic.New, nativeapi.New, persistence.New, lastfm.NewRouter, listenbrainz.NewRouter, events.GetBroker, db.Db) // Scanner must be a Singleton var ( diff --git a/cmd/wire_injectors.go b/cmd/wire_injectors.go index a11dc3e83..a83881c55 100644 --- a/cmd/wire_injectors.go +++ b/cmd/wire_injectors.go @@ -9,6 +9,7 @@ import ( "github.com/navidrome/navidrome/core" "github.com/navidrome/navidrome/core/agents/lastfm" "github.com/navidrome/navidrome/core/agents/listenbrainz" + "github.com/navidrome/navidrome/core/artwork" "github.com/navidrome/navidrome/db" "github.com/navidrome/navidrome/persistence" "github.com/navidrome/navidrome/scanner" @@ -20,6 +21,7 @@ import ( var allProviders = wire.NewSet( core.Set, + artwork.Set, subsonic.New, nativeapi.New, persistence.New, diff --git a/core/artwork/artwork.go b/core/artwork/artwork.go index 96dee8c62..05209cd5e 100644 --- a/core/artwork/artwork.go +++ b/core/artwork/artwork.go @@ -38,12 +38,9 @@ func (a *artwork) Get(ctx context.Context, id string, size int) (reader io.ReadC ctx, cancel := context.WithTimeout(ctx, 10*time.Second) defer cancel() - var artID model.ArtworkID - if id != "" { - artID, err = model.ParseArtworkID(id) - if err != nil { - return nil, time.Time{}, errors.New("invalid ID") - } + artID, err := a.getArtworkId(ctx, id) + if err != nil { + return nil, time.Time{}, err } artReader, err := a.getArtworkReader(ctx, artID, size) @@ -61,6 +58,34 @@ func (a *artwork) Get(ctx context.Context, id string, size int) (reader io.ReadC return r, artReader.LastUpdated(), nil } +func (a *artwork) getArtworkId(ctx context.Context, id string) (model.ArtworkID, error) { + if id == "" { + return model.ArtworkID{}, nil + } + artID, err := model.ParseArtworkID(id) + if err == nil { + return artID, nil + } + + log.Trace(ctx, "ArtworkID invalid. Trying to figure out kind based on the ID", "id", id) + entity, err := model.GetEntityByID(ctx, a.ds, id) + if err != nil { + return model.ArtworkID{}, err + } + switch e := entity.(type) { + case *model.Album: + artID = model.NewArtworkID(model.KindAlbumArtwork, e.ID) + log.Trace(ctx, "ID is for an Album", "id", id, "name", e.Name, "artist", e.AlbumArtist) + case *model.MediaFile: + artID = model.NewArtworkID(model.KindMediaFileArtwork, e.ID) + log.Trace(ctx, "ID is for a MediaFile", "id", id, "title", e.Title, "album", e.Album) + case *model.Playlist: + artID = model.NewArtworkID(model.KindPlaylistArtwork, e.ID) + log.Trace(ctx, "ID is for a Playlist", "id", id, "name", e.Name) + } + return artID, nil +} + func (a *artwork) getArtworkReader(ctx context.Context, artID model.ArtworkID, size int) (artworkReader, error) { var artReader artworkReader var err error diff --git a/core/artwork/reader_playlist.go b/core/artwork/reader_playlist.go index 440cab383..f42d1249b 100644 --- a/core/artwork/reader_playlist.go +++ b/core/artwork/reader_playlist.go @@ -13,6 +13,7 @@ import ( "github.com/disintegration/imaging" "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/utils/slice" "golang.org/x/exp/slices" ) @@ -66,10 +67,9 @@ func (a *playlistArtworkReader) fromGeneratedTile(ctx context.Context, tracks mo func compactIDs(tracks model.PlaylistTracks) []model.ArtworkID { slices.SortFunc(tracks, func(a, b model.PlaylistTrack) bool { return a.AlbumID < b.AlbumID }) tracks = slices.CompactFunc(tracks, func(a, b model.PlaylistTrack) bool { return a.AlbumID == b.AlbumID }) - ids := make([]model.ArtworkID, len(tracks)) - for i, t := range tracks { - ids[i] = t.AlbumCoverArtID() - } + ids := slice.Map(tracks, func(e model.PlaylistTrack) model.ArtworkID { + return e.AlbumCoverArtID() + }) rand.Seed(time.Now().UnixNano()) rand.Shuffle(len(ids), func(i, j int) { ids[i], ids[j] = ids[j], ids[i] }) return ids diff --git a/core/artwork/wire_providers.go b/core/artwork/wire_providers.go new file mode 100644 index 000000000..63231b54a --- /dev/null +++ b/core/artwork/wire_providers.go @@ -0,0 +1,11 @@ +package artwork + +import ( + "github.com/google/wire" +) + +var Set = wire.NewSet( + NewArtwork, + GetImageCache, + NewCacheWarmer, +) diff --git a/core/external_metadata.go b/core/external_metadata.go index 3678ad38a..ffa5c7da9 100644 --- a/core/external_metadata.go +++ b/core/external_metadata.go @@ -47,7 +47,7 @@ func NewExternalMetadata(ds model.DataStore, agents *agents.Agents) ExternalMeta func (e *externalMetadata) getArtist(ctx context.Context, id string) (*auxArtist, error) { var entity interface{} - entity, err := GetEntityByID(ctx, e.ds, id) + entity, err := model.GetEntityByID(ctx, e.ds, id) if err != nil { return nil, err } diff --git a/core/wire_providers.go b/core/wire_providers.go index 699db3bfc..d7ee2b694 100644 --- a/core/wire_providers.go +++ b/core/wire_providers.go @@ -3,7 +3,6 @@ package core import ( "github.com/google/wire" "github.com/navidrome/navidrome/core/agents" - "github.com/navidrome/navidrome/core/artwork" "github.com/navidrome/navidrome/core/ffmpeg" "github.com/navidrome/navidrome/core/scrobbler" ) @@ -19,7 +18,4 @@ var Set = wire.NewSet( agents.New, ffmpeg.New, scrobbler.GetPlayTracker, - artwork.NewArtwork, - artwork.GetImageCache, - artwork.NewCacheWarmer, ) diff --git a/model/artwork_id.go b/model/artwork_id.go index cf5c1cdc4..b88a05439 100644 --- a/model/artwork_id.go +++ b/model/artwork_id.go @@ -34,6 +34,10 @@ func (id ArtworkID) String() string { return fmt.Sprintf("%s-%s", id.Kind.prefix, id.ID) } +func NewArtworkID(kind Kind, id string) ArtworkID { + return ArtworkID{kind, id} +} + func ParseArtworkID(id string) (ArtworkID, error) { parts := strings.SplitN(id, "-", 2) if len(parts) != 2 { diff --git a/core/get_entity.go b/model/get_entity.go similarity index 72% rename from core/get_entity.go rename to model/get_entity.go index 341a1d3c1..f51d8c36a 100644 --- a/core/get_entity.go +++ b/model/get_entity.go @@ -1,13 +1,11 @@ -package core +package model import ( "context" - - "github.com/navidrome/navidrome/model" ) // TODO: Should the type be encoded in the ID? -func GetEntityByID(ctx context.Context, ds model.DataStore, id string) (interface{}, error) { +func GetEntityByID(ctx context.Context, ds DataStore, id string) (interface{}, error) { ar, err := ds.Artist(ctx).Get(id) if err == nil { return ar, nil diff --git a/server/subsonic/browsing.go b/server/subsonic/browsing.go index 3531e4dbf..9d17dcd47 100644 --- a/server/subsonic/browsing.go +++ b/server/subsonic/browsing.go @@ -8,7 +8,6 @@ import ( "time" "github.com/navidrome/navidrome/conf" - "github.com/navidrome/navidrome/core" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/server/subsonic/filter" @@ -95,7 +94,7 @@ func (api *Router) GetMusicDirectory(r *http.Request) (*responses.Subsonic, erro id := utils.ParamString(r, "id") ctx := r.Context() - entity, err := core.GetEntityByID(ctx, api.ds, id) + entity, err := model.GetEntityByID(ctx, api.ds, id) if errors.Is(err, model.ErrNotFound) { log.Error(r, "Requested ID not found ", "id", id) return nil, newError(responses.ErrorDataNotFound, "Directory not found") diff --git a/server/subsonic/media_annotation.go b/server/subsonic/media_annotation.go index 10c36804b..f6576b157 100644 --- a/server/subsonic/media_annotation.go +++ b/server/subsonic/media_annotation.go @@ -7,7 +7,6 @@ import ( "net/http" "time" - "github.com/navidrome/navidrome/core" "github.com/navidrome/navidrome/core/scrobbler" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" @@ -46,7 +45,7 @@ func (api *Router) setRating(ctx context.Context, id string, rating int) error { var repo model.AnnotatedRepository var resource string - entity, err := core.GetEntityByID(ctx, api.ds, id) + entity, err := model.GetEntityByID(ctx, api.ds, id) if err != nil { return err } diff --git a/server/subsonic/stream.go b/server/subsonic/stream.go index 1d547394a..259107168 100644 --- a/server/subsonic/stream.go +++ b/server/subsonic/stream.go @@ -91,7 +91,7 @@ func (api *Router) Download(w http.ResponseWriter, r *http.Request) (*responses. return nil, newError(responses.ErrorAuthorizationFail, "downloads are disabled") } - entity, err := core.GetEntityByID(ctx, api.ds, id) + entity, err := model.GetEntityByID(ctx, api.ds, id) if err != nil { return nil, err } diff --git a/utils/slice/slice.go b/utils/slice/slice.go index d7dba0a61..60606020d 100644 --- a/utils/slice/slice.go +++ b/utils/slice/slice.go @@ -1,5 +1,13 @@ package slice +func Map[T any, R any](t []T, mapFunc func(T) R) []R { + r := make([]R, len(t)) + for i, e := range t { + r[i] = mapFunc(e) + } + return r +} + func Group[T any, K comparable](s []T, keyFunc func(T) K) map[K][]T { m := map[K][]T{} for _, item := range s { diff --git a/utils/slice/slice_test.go b/utils/slice/slice_test.go index a9cee7b3a..7a33e01df 100644 --- a/utils/slice/slice_test.go +++ b/utils/slice/slice_test.go @@ -1,6 +1,7 @@ package slice_test import ( + "strconv" "testing" "github.com/navidrome/navidrome/utils/slice" @@ -13,6 +14,20 @@ func TestSlice(t *testing.T) { RunSpecs(t, "Slice Suite") } +var _ = Describe("Map", func() { + It("returns empty slice for an empty input", func() { + mapFunc := func(v int) string { return strconv.Itoa(v * 2) } + result := slice.Map([]int{}, mapFunc) + Expect(result).To(BeEmpty()) + }) + + It("returns a new slice with elements mapped", func() { + mapFunc := func(v int) string { return strconv.Itoa(v * 2) } + result := slice.Map([]int{1, 2, 3, 4}, mapFunc) + Expect(result).To(ConsistOf("2", "4", "6", "8")) + }) +}) + var _ = Describe("Group", func() { It("returns empty map for an empty input", func() { keyFunc := func(v int) int { return v % 2 }