diff --git a/cmd/wire_gen.go b/cmd/wire_gen.go index e5e72bf4f..d57aadc71 100644 --- a/cmd/wire_gen.go +++ b/cmd/wire_gen.go @@ -14,6 +14,7 @@ import ( "github.com/navidrome/navidrome/core/agents/lastfm" "github.com/navidrome/navidrome/core/agents/listenbrainz" "github.com/navidrome/navidrome/core/artwork" + "github.com/navidrome/navidrome/core/external" "github.com/navidrome/navidrome/core/ffmpeg" "github.com/navidrome/navidrome/core/metrics" "github.com/navidrome/navidrome/core/playback" @@ -66,8 +67,8 @@ func CreateSubsonicAPIRouter(ctx context.Context) *subsonic.Router { fileCache := artwork.GetImageCache() fFmpeg := ffmpeg.New() agentsAgents := agents.GetAgents(dataStore) - externalMetadata := core.NewExternalMetadata(dataStore, agentsAgents) - artworkArtwork := artwork.NewArtwork(dataStore, fileCache, fFmpeg, externalMetadata) + provider := external.NewProvider(dataStore, agentsAgents) + artworkArtwork := artwork.NewArtwork(dataStore, fileCache, fFmpeg, provider) transcodingCache := core.GetTranscodingCache() mediaStreamer := core.NewMediaStreamer(dataStore, fFmpeg, transcodingCache) share := core.NewShare(dataStore) @@ -80,7 +81,7 @@ func CreateSubsonicAPIRouter(ctx context.Context) *subsonic.Router { scannerScanner := scanner.New(ctx, dataStore, cacheWarmer, broker, playlists, metricsMetrics) playTracker := scrobbler.GetPlayTracker(dataStore, broker) playbackServer := playback.GetInstance(dataStore) - router := subsonic.New(dataStore, artworkArtwork, mediaStreamer, archiver, players, externalMetadata, scannerScanner, broker, playlists, playTracker, share, playbackServer) + router := subsonic.New(dataStore, artworkArtwork, mediaStreamer, archiver, players, provider, scannerScanner, broker, playlists, playTracker, share, playbackServer) return router } @@ -90,8 +91,8 @@ func CreatePublicRouter() *public.Router { fileCache := artwork.GetImageCache() fFmpeg := ffmpeg.New() agentsAgents := agents.GetAgents(dataStore) - externalMetadata := core.NewExternalMetadata(dataStore, agentsAgents) - artworkArtwork := artwork.NewArtwork(dataStore, fileCache, fFmpeg, externalMetadata) + provider := external.NewProvider(dataStore, agentsAgents) + artworkArtwork := artwork.NewArtwork(dataStore, fileCache, fFmpeg, provider) transcodingCache := core.GetTranscodingCache() mediaStreamer := core.NewMediaStreamer(dataStore, fFmpeg, transcodingCache) share := core.NewShare(dataStore) @@ -134,8 +135,8 @@ func CreateScanner(ctx context.Context) scanner.Scanner { fileCache := artwork.GetImageCache() fFmpeg := ffmpeg.New() agentsAgents := agents.GetAgents(dataStore) - externalMetadata := core.NewExternalMetadata(dataStore, agentsAgents) - artworkArtwork := artwork.NewArtwork(dataStore, fileCache, fFmpeg, externalMetadata) + provider := external.NewProvider(dataStore, agentsAgents) + artworkArtwork := artwork.NewArtwork(dataStore, fileCache, fFmpeg, provider) cacheWarmer := artwork.NewCacheWarmer(artworkArtwork, fileCache) broker := events.GetBroker() playlists := core.NewPlaylists(dataStore) @@ -150,8 +151,8 @@ func CreateScanWatcher(ctx context.Context) scanner.Watcher { fileCache := artwork.GetImageCache() fFmpeg := ffmpeg.New() agentsAgents := agents.GetAgents(dataStore) - externalMetadata := core.NewExternalMetadata(dataStore, agentsAgents) - artworkArtwork := artwork.NewArtwork(dataStore, fileCache, fFmpeg, externalMetadata) + provider := external.NewProvider(dataStore, agentsAgents) + artworkArtwork := artwork.NewArtwork(dataStore, fileCache, fFmpeg, provider) cacheWarmer := artwork.NewCacheWarmer(artworkArtwork, fileCache) broker := events.GetBroker() playlists := core.NewPlaylists(dataStore) diff --git a/core/artwork/artwork.go b/core/artwork/artwork.go index 3570dd7b4..f1f571e0d 100644 --- a/core/artwork/artwork.go +++ b/core/artwork/artwork.go @@ -8,7 +8,7 @@ import ( "time" "github.com/navidrome/navidrome/consts" - "github.com/navidrome/navidrome/core" + "github.com/navidrome/navidrome/core/external" "github.com/navidrome/navidrome/core/ffmpeg" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" @@ -24,15 +24,15 @@ type Artwork interface { GetOrPlaceholder(ctx context.Context, id string, size int, square bool) (io.ReadCloser, time.Time, error) } -func NewArtwork(ds model.DataStore, cache cache.FileCache, ffmpeg ffmpeg.FFmpeg, em core.ExternalMetadata) Artwork { - return &artwork{ds: ds, cache: cache, ffmpeg: ffmpeg, em: em} +func NewArtwork(ds model.DataStore, cache cache.FileCache, ffmpeg ffmpeg.FFmpeg, provider external.Provider) Artwork { + return &artwork{ds: ds, cache: cache, ffmpeg: ffmpeg, provider: provider} } type artwork struct { - ds model.DataStore - cache cache.FileCache - ffmpeg ffmpeg.FFmpeg - em core.ExternalMetadata + ds model.DataStore + cache cache.FileCache + ffmpeg ffmpeg.FFmpeg + provider external.Provider } type artworkReader interface { @@ -115,9 +115,9 @@ func (a *artwork) getArtworkReader(ctx context.Context, artID model.ArtworkID, s } else { switch artID.Kind { case model.KindArtistArtwork: - artReader, err = newArtistReader(ctx, a, artID, a.em) + artReader, err = newArtistReader(ctx, a, artID, a.provider) case model.KindAlbumArtwork: - artReader, err = newAlbumArtworkReader(ctx, a, artID, a.em) + artReader, err = newAlbumArtworkReader(ctx, a, artID, a.provider) case model.KindMediaFileArtwork: artReader, err = newMediafileArtworkReader(ctx, a, artID) case model.KindPlaylistArtwork: diff --git a/core/artwork/reader_album.go b/core/artwork/reader_album.go index f1ed9b63c..a320533ed 100644 --- a/core/artwork/reader_album.go +++ b/core/artwork/reader_album.go @@ -12,6 +12,7 @@ import ( "github.com/Masterminds/squirrel" "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/core" + "github.com/navidrome/navidrome/core/external" "github.com/navidrome/navidrome/core/ffmpeg" "github.com/navidrome/navidrome/model" ) @@ -19,14 +20,14 @@ import ( type albumArtworkReader struct { cacheKey a *artwork - em core.ExternalMetadata + provider external.Provider album model.Album updatedAt *time.Time imgFiles []string rootFolder string } -func newAlbumArtworkReader(ctx context.Context, artwork *artwork, artID model.ArtworkID, em core.ExternalMetadata) (*albumArtworkReader, error) { +func newAlbumArtworkReader(ctx context.Context, artwork *artwork, artID model.ArtworkID, provider external.Provider) (*albumArtworkReader, error) { al, err := artwork.ds.Album(ctx).Get(artID.ID) if err != nil { return nil, err @@ -37,7 +38,7 @@ func newAlbumArtworkReader(ctx context.Context, artwork *artwork, artID model.Ar } a := &albumArtworkReader{ a: artwork, - em: em, + provider: provider, album: *al, updatedAt: imagesUpdateAt, imgFiles: imgFiles, @@ -82,7 +83,7 @@ func (a *albumArtworkReader) fromCoverArtPriority(ctx context.Context, ffmpeg ff embedArtPath := filepath.Join(a.rootFolder, a.album.EmbedArtPath) ff = append(ff, fromTag(ctx, embedArtPath), fromFFmpegTag(ctx, ffmpeg, embedArtPath)) case pattern == "external": - ff = append(ff, fromAlbumExternalSource(ctx, a.album, a.em)) + ff = append(ff, fromAlbumExternalSource(ctx, a.album, a.provider)) case len(a.imgFiles) > 0: ff = append(ff, fromExternalFile(ctx, a.imgFiles, pattern)) } diff --git a/core/artwork/reader_artist.go b/core/artwork/reader_artist.go index e910ef93e..217044b7a 100644 --- a/core/artwork/reader_artist.go +++ b/core/artwork/reader_artist.go @@ -14,6 +14,7 @@ import ( "github.com/Masterminds/squirrel" "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/core" + "github.com/navidrome/navidrome/core/external" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/utils/str" @@ -22,13 +23,13 @@ import ( type artistReader struct { cacheKey a *artwork - em core.ExternalMetadata + provider external.Provider artist model.Artist artistFolder string imgFiles []string } -func newArtistReader(ctx context.Context, artwork *artwork, artID model.ArtworkID, em core.ExternalMetadata) (*artistReader, error) { +func newArtistReader(ctx context.Context, artwork *artwork, artID model.ArtworkID, provider external.Provider) (*artistReader, error) { ar, err := artwork.ds.Artist(ctx).Get(artID.ID) if err != nil { return nil, err @@ -53,7 +54,7 @@ func newArtistReader(ctx context.Context, artwork *artwork, artID model.ArtworkI } a := &artistReader{ a: artwork, - em: em, + provider: provider, artist: *ar, artistFolder: artistFolder, imgFiles: imgFiles, @@ -95,7 +96,7 @@ func (a *artistReader) fromArtistArtPriority(ctx context.Context, priority strin pattern = strings.TrimSpace(pattern) switch { case pattern == "external": - ff = append(ff, fromArtistExternalSource(ctx, a.artist, a.em)) + ff = append(ff, fromArtistExternalSource(ctx, a.artist, a.provider)) case strings.HasPrefix(pattern, "album/"): ff = append(ff, fromExternalFile(ctx, a.imgFiles, strings.TrimPrefix(pattern, "album/"))) default: diff --git a/core/artwork/sources.go b/core/artwork/sources.go index f89708255..121e6c38b 100644 --- a/core/artwork/sources.go +++ b/core/artwork/sources.go @@ -17,7 +17,7 @@ import ( "github.com/dhowden/tag" "github.com/navidrome/navidrome/consts" - "github.com/navidrome/navidrome/core" + "github.com/navidrome/navidrome/core/external" "github.com/navidrome/navidrome/core/ffmpeg" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" @@ -157,9 +157,9 @@ func fromAlbumPlaceholder() sourceFunc { return r, consts.PlaceholderAlbumArt, nil } } -func fromArtistExternalSource(ctx context.Context, ar model.Artist, em core.ExternalMetadata) sourceFunc { +func fromArtistExternalSource(ctx context.Context, ar model.Artist, provider external.Provider) sourceFunc { return func() (io.ReadCloser, string, error) { - imageUrl, err := em.ArtistImage(ctx, ar.ID) + imageUrl, err := provider.ArtistImage(ctx, ar.ID) if err != nil { return nil, "", err } @@ -168,9 +168,9 @@ func fromArtistExternalSource(ctx context.Context, ar model.Artist, em core.Exte } } -func fromAlbumExternalSource(ctx context.Context, al model.Album, em core.ExternalMetadata) sourceFunc { +func fromAlbumExternalSource(ctx context.Context, al model.Album, provider external.Provider) sourceFunc { return func() (io.ReadCloser, string, error) { - imageUrl, err := em.AlbumImage(ctx, al.ID) + imageUrl, err := provider.AlbumImage(ctx, al.ID) if err != nil { return nil, "", err } diff --git a/core/external/extdata_helper_test.go b/core/external/extdata_helper_test.go new file mode 100644 index 000000000..367437815 --- /dev/null +++ b/core/external/extdata_helper_test.go @@ -0,0 +1,270 @@ +package external_test + +import ( + "context" + "errors" + + "github.com/navidrome/navidrome/core/agents" + "github.com/navidrome/navidrome/model" + "github.com/stretchr/testify/mock" +) + +// --- Shared Mock Implementations --- + +// mockArtistRepo mocks model.ArtistRepository +type mockArtistRepo struct { + mock.Mock + model.ArtistRepository +} + +func newMockArtistRepo() *mockArtistRepo { + return &mockArtistRepo{} +} + +// SetData sets up basic Get expectations. +func (m *mockArtistRepo) SetData(artists model.Artists) { + for _, a := range artists { + artistCopy := a + m.On("Get", artistCopy.ID).Return(&artistCopy, nil) + } +} + +// Get implements model.ArtistRepository. +func (m *mockArtistRepo) Get(id string) (*model.Artist, error) { + args := m.Called(id) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).(*model.Artist), args.Error(1) +} + +// GetAll implements model.ArtistRepository. +func (m *mockArtistRepo) GetAll(options ...model.QueryOptions) (model.Artists, error) { + argsSlice := make([]interface{}, len(options)) + for i, v := range options { + argsSlice[i] = v + } + args := m.Called(argsSlice...) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).(model.Artists), args.Error(1) +} + +// SetError is a helper to set up a generic error for GetAll. +func (m *mockArtistRepo) SetError(hasError bool) { + if hasError { + m.On("GetAll", mock.Anything).Return(nil, errors.New("mock repo error")) + } +} + +// FindByName is a helper to set up a GetAll expectation for finding by name. +func (m *mockArtistRepo) FindByName(name string, artist model.Artist) { + m.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + return opt.Filters != nil + })).Return(model.Artists{artist}, nil).Once() +} + +// mockMediaFileRepo mocks model.MediaFileRepository +type mockMediaFileRepo struct { + mock.Mock + model.MediaFileRepository +} + +func newMockMediaFileRepo() *mockMediaFileRepo { + return &mockMediaFileRepo{} +} + +// SetData sets up basic Get expectations. +func (m *mockMediaFileRepo) SetData(mediaFiles model.MediaFiles) { + for _, mf := range mediaFiles { + mfCopy := mf + m.On("Get", mfCopy.ID).Return(&mfCopy, nil) + } +} + +// Get implements model.MediaFileRepository. +func (m *mockMediaFileRepo) Get(id string) (*model.MediaFile, error) { + args := m.Called(id) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).(*model.MediaFile), args.Error(1) +} + +// GetAll implements model.MediaFileRepository. +func (m *mockMediaFileRepo) GetAll(options ...model.QueryOptions) (model.MediaFiles, error) { + argsSlice := make([]interface{}, len(options)) + for i, v := range options { + argsSlice[i] = v + } + args := m.Called(argsSlice...) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).(model.MediaFiles), args.Error(1) +} + +// SetError is a helper to set up a generic error for GetAll. +func (m *mockMediaFileRepo) SetError(hasError bool) { + if hasError { + m.On("GetAll", mock.Anything).Return(nil, errors.New("mock repo error")) + } +} + +// FindByMBID is a helper to set up a GetAll expectation for finding by MBID. +func (m *mockMediaFileRepo) FindByMBID(mbid string, mediaFile model.MediaFile) { + m.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + return opt.Filters != nil + })).Return(model.MediaFiles{mediaFile}, nil).Once() +} + +// FindByArtistAndTitle is a helper to set up a GetAll expectation for finding by artist/title. +func (m *mockMediaFileRepo) FindByArtistAndTitle(artistID string, title string, mediaFile model.MediaFile) { + m.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + return opt.Filters != nil + })).Return(model.MediaFiles{mediaFile}, nil).Once() +} + +// mockAlbumRepo mocks model.AlbumRepository +type mockAlbumRepo struct { + mock.Mock + model.AlbumRepository +} + +func newMockAlbumRepo() *mockAlbumRepo { + return &mockAlbumRepo{} +} + +// Get implements model.AlbumRepository. +func (m *mockAlbumRepo) Get(id string) (*model.Album, error) { + args := m.Called(id) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).(*model.Album), args.Error(1) +} + +// GetAll implements model.AlbumRepository. +func (m *mockAlbumRepo) GetAll(options ...model.QueryOptions) (model.Albums, error) { + argsSlice := make([]interface{}, len(options)) + for i, v := range options { + argsSlice[i] = v + } + args := m.Called(argsSlice...) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).(model.Albums), args.Error(1) +} + +// mockSimilarArtistAgent mocks agents implementing ArtistTopSongsRetriever and ArtistSimilarRetriever +type mockSimilarArtistAgent struct { + mock.Mock + agents.Interface // Embed to satisfy methods not explicitly mocked +} + +func (m *mockSimilarArtistAgent) AgentName() string { + return "mockSimilar" +} + +func (m *mockSimilarArtistAgent) GetArtistTopSongs(ctx context.Context, id, artistName, mbid string, count int) ([]agents.Song, error) { + args := m.Called(ctx, id, artistName, mbid, count) + if args.Get(0) != nil { + return args.Get(0).([]agents.Song), args.Error(1) + } + return nil, args.Error(1) +} + +func (m *mockSimilarArtistAgent) GetSimilarArtists(ctx context.Context, id, name, mbid string, limit int) ([]agents.Artist, error) { + args := m.Called(ctx, id, name, mbid, limit) + if args.Get(0) != nil { + return args.Get(0).([]agents.Artist), args.Error(1) + } + return nil, args.Error(1) +} + +// mockAgents mocks the main Agents interface used by Provider +type mockAgents struct { + mock.Mock // Embed testify mock + topSongsAgent agents.ArtistTopSongsRetriever + similarAgent agents.ArtistSimilarRetriever + imageAgent agents.ArtistImageRetriever + albumInfoAgent agents.AlbumInfoRetriever + bioAgent agents.ArtistBiographyRetriever + mbidAgent agents.ArtistMBIDRetriever + urlAgent agents.ArtistURLRetriever + agents.Interface +} + +func (m *mockAgents) AgentName() string { + return "mockCombined" +} + +func (m *mockAgents) GetSimilarArtists(ctx context.Context, id, name, mbid string, limit int) ([]agents.Artist, error) { + if m.similarAgent != nil { + return m.similarAgent.GetSimilarArtists(ctx, id, name, mbid, limit) + } + args := m.Called(ctx, id, name, mbid, limit) + if args.Get(0) != nil { + return args.Get(0).([]agents.Artist), args.Error(1) + } + return nil, args.Error(1) +} + +func (m *mockAgents) GetArtistTopSongs(ctx context.Context, id, artistName, mbid string, count int) ([]agents.Song, error) { + if m.topSongsAgent != nil { + return m.topSongsAgent.GetArtistTopSongs(ctx, id, artistName, mbid, count) + } + args := m.Called(ctx, id, artistName, mbid, count) + if args.Get(0) != nil { + return args.Get(0).([]agents.Song), args.Error(1) + } + return nil, args.Error(1) +} + +func (m *mockAgents) GetAlbumInfo(ctx context.Context, name, artist, mbid string) (*agents.AlbumInfo, error) { + if m.albumInfoAgent != nil { + return m.albumInfoAgent.GetAlbumInfo(ctx, name, artist, mbid) + } + args := m.Called(ctx, name, artist, mbid) + if args.Get(0) != nil { + return args.Get(0).(*agents.AlbumInfo), args.Error(1) + } + return nil, args.Error(1) +} + +func (m *mockAgents) GetArtistMBID(ctx context.Context, id string, name string) (string, error) { + if m.mbidAgent != nil { + return m.mbidAgent.GetArtistMBID(ctx, id, name) + } + args := m.Called(ctx, id, name) + return args.String(0), args.Error(1) +} + +func (m *mockAgents) GetArtistURL(ctx context.Context, id, name, mbid string) (string, error) { + if m.urlAgent != nil { + return m.urlAgent.GetArtistURL(ctx, id, name, mbid) + } + args := m.Called(ctx, id, name, mbid) + return args.String(0), args.Error(1) +} + +func (m *mockAgents) GetArtistBiography(ctx context.Context, id, name, mbid string) (string, error) { + if m.bioAgent != nil { + return m.bioAgent.GetArtistBiography(ctx, id, name, mbid) + } + args := m.Called(ctx, id, name, mbid) + return args.String(0), args.Error(1) +} + +func (m *mockAgents) GetArtistImages(ctx context.Context, id, name, mbid string) ([]agents.ExternalImage, error) { + if m.imageAgent != nil { + return m.imageAgent.GetArtistImages(ctx, id, name, mbid) + } + args := m.Called(ctx, id, name, mbid) + if args.Get(0) != nil { + return args.Get(0).([]agents.ExternalImage), args.Error(1) + } + return nil, args.Error(1) +} diff --git a/core/external/extdata_suite_test.go b/core/external/extdata_suite_test.go new file mode 100644 index 000000000..f059e76b3 --- /dev/null +++ b/core/external/extdata_suite_test.go @@ -0,0 +1,17 @@ +package external + +import ( + "testing" + + "github.com/navidrome/navidrome/log" + "github.com/navidrome/navidrome/tests" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestExternal(t *testing.T) { + tests.Init(t, false) + log.SetLevel(log.LevelFatal) + RegisterFailHandler(Fail) + RunSpecs(t, "External Suite") +} diff --git a/core/external_metadata.go b/core/external/provider.go similarity index 79% rename from core/external_metadata.go rename to core/external/provider.go index d402c3a36..f27ded11b 100644 --- a/core/external_metadata.go +++ b/core/external/provider.go @@ -1,4 +1,4 @@ -package core +package external import ( "context" @@ -31,7 +31,7 @@ const ( refreshQueueLength = 2000 ) -type ExternalMetadata interface { +type Provider interface { UpdateAlbumInfo(ctx context.Context, id string) (*model.Album, error) UpdateArtistInfo(ctx context.Context, id string, count int, includeNotPresent bool) (*model.Artist, error) SimilarSongs(ctx context.Context, id string, count int) (model.MediaFiles, error) @@ -40,9 +40,9 @@ type ExternalMetadata interface { AlbumImage(ctx context.Context, id string) (*url.URL, error) } -type externalMetadata struct { +type provider struct { ds model.DataStore - ag *agents.Agents + ag Agents artistQueue refreshQueue[auxArtist] albumQueue refreshQueue[auxAlbum] } @@ -57,14 +57,24 @@ type auxArtist struct { Name string } -func NewExternalMetadata(ds model.DataStore, agents *agents.Agents) ExternalMetadata { - e := &externalMetadata{ds: ds, ag: agents} +type Agents interface { + agents.AlbumInfoRetriever + agents.ArtistBiographyRetriever + agents.ArtistMBIDRetriever + agents.ArtistImageRetriever + agents.ArtistSimilarRetriever + agents.ArtistTopSongsRetriever + agents.ArtistURLRetriever +} + +func NewProvider(ds model.DataStore, agents Agents) Provider { + e := &provider{ds: ds, ag: agents} e.artistQueue = newRefreshQueue(context.TODO(), e.populateArtistInfo) e.albumQueue = newRefreshQueue(context.TODO(), e.populateAlbumInfo) return e } -func (e *externalMetadata) getAlbum(ctx context.Context, id string) (auxAlbum, error) { +func (e *provider) getAlbum(ctx context.Context, id string) (auxAlbum, error) { var entity interface{} entity, err := model.GetEntityByID(ctx, e.ds, id) if err != nil { @@ -81,10 +91,11 @@ func (e *externalMetadata) getAlbum(ctx context.Context, id string) (auxAlbum, e default: return auxAlbum{}, model.ErrNotFound } + return album, nil } -func (e *externalMetadata) UpdateAlbumInfo(ctx context.Context, id string) (*model.Album, error) { +func (e *provider) UpdateAlbumInfo(ctx context.Context, id string) (*model.Album, error) { album, err := e.getAlbum(ctx, id) if err != nil { log.Info(ctx, "Not found", "id", id) @@ -109,7 +120,7 @@ func (e *externalMetadata) UpdateAlbumInfo(ctx context.Context, id string) (*mod return &album.Album, nil } -func (e *externalMetadata) populateAlbumInfo(ctx context.Context, album auxAlbum) (auxAlbum, error) { +func (e *provider) populateAlbumInfo(ctx context.Context, album auxAlbum) (auxAlbum, error) { start := time.Now() info, err := e.ag.GetAlbumInfo(ctx, album.Name, album.AlbumArtist, album.MbzAlbumID) if errors.Is(err, agents.ErrNotFound) { @@ -155,7 +166,7 @@ func (e *externalMetadata) populateAlbumInfo(ctx context.Context, album auxAlbum return album, nil } -func (e *externalMetadata) getArtist(ctx context.Context, id string) (auxArtist, error) { +func (e *provider) getArtist(ctx context.Context, id string) (auxArtist, error) { var entity interface{} entity, err := model.GetEntityByID(ctx, e.ds, id) if err != nil { @@ -177,7 +188,7 @@ func (e *externalMetadata) getArtist(ctx context.Context, id string) (auxArtist, return artist, nil } -func (e *externalMetadata) UpdateArtistInfo(ctx context.Context, id string, similarCount int, includeNotPresent bool) (*model.Artist, error) { +func (e *provider) UpdateArtistInfo(ctx context.Context, id string, similarCount int, includeNotPresent bool) (*model.Artist, error) { artist, err := e.refreshArtistInfo(ctx, id) if err != nil { return nil, err @@ -187,7 +198,7 @@ func (e *externalMetadata) UpdateArtistInfo(ctx context.Context, id string, simi return &artist.Artist, err } -func (e *externalMetadata) refreshArtistInfo(ctx context.Context, id string) (auxArtist, error) { +func (e *provider) refreshArtistInfo(ctx context.Context, id string) (auxArtist, error) { artist, err := e.getArtist(ctx, id) if err != nil { return auxArtist{}, err @@ -211,7 +222,7 @@ func (e *externalMetadata) refreshArtistInfo(ctx context.Context, id string) (au return artist, nil } -func (e *externalMetadata) populateArtistInfo(ctx context.Context, artist auxArtist) (auxArtist, error) { +func (e *provider) populateArtistInfo(ctx context.Context, artist auxArtist) (auxArtist, error) { start := time.Now() // Get MBID first, if it is not yet available if artist.MbzArtistID == "" { @@ -246,7 +257,7 @@ func (e *externalMetadata) populateArtistInfo(ctx context.Context, artist auxArt return artist, nil } -func (e *externalMetadata) SimilarSongs(ctx context.Context, id string, count int) (model.MediaFiles, error) { +func (e *provider) SimilarSongs(ctx context.Context, id string, count int) (model.MediaFiles, error) { artist, err := e.getArtist(ctx, id) if err != nil { return nil, err @@ -304,7 +315,7 @@ func (e *externalMetadata) SimilarSongs(ctx context.Context, id string, count in return similarSongs, nil } -func (e *externalMetadata) ArtistImage(ctx context.Context, id string) (*url.URL, error) { +func (e *provider) ArtistImage(ctx context.Context, id string) (*url.URL, error) { artist, err := e.getArtist(ctx, id) if err != nil { return nil, err @@ -318,24 +329,35 @@ func (e *externalMetadata) ArtistImage(ctx context.Context, id string) (*url.URL imageUrl := artist.ArtistImageUrl() if imageUrl == "" { - return nil, agents.ErrNotFound + return nil, model.ErrNotFound } return url.Parse(imageUrl) } -func (e *externalMetadata) AlbumImage(ctx context.Context, id string) (*url.URL, error) { +func (e *provider) AlbumImage(ctx context.Context, id string) (*url.URL, error) { album, err := e.getAlbum(ctx, id) if err != nil { return nil, err } info, err := e.ag.GetAlbumInfo(ctx, album.Name, album.AlbumArtist, album.MbzAlbumID) - if errors.Is(err, agents.ErrNotFound) { + if err != nil { + switch { + case errors.Is(err, agents.ErrNotFound): + log.Trace(ctx, "Album not found in agent", "albumID", id, "name", album.Name, "artist", album.AlbumArtist) + return nil, model.ErrNotFound + case errors.Is(err, context.Canceled): + log.Debug(ctx, "GetAlbumInfo call canceled", err) + default: + log.Warn(ctx, "Error getting album info from agent", "albumID", id, "name", album.Name, "artist", album.AlbumArtist, err) + } + return nil, err } - if utils.IsCtxDone(ctx) { - log.Warn(ctx, "AlbumImage call canceled", ctx.Err()) - return nil, ctx.Err() + + if info == nil { + log.Warn(ctx, "Agent returned nil info without error", "albumID", id, "name", album.Name, "artist", album.AlbumArtist) + return nil, model.ErrNotFound } // Return the biggest image @@ -346,26 +368,37 @@ func (e *externalMetadata) AlbumImage(ctx context.Context, id string) (*url.URL, } } if img.URL == "" { - return nil, agents.ErrNotFound + return nil, model.ErrNotFound } return url.Parse(img.URL) } -func (e *externalMetadata) TopSongs(ctx context.Context, artistName string, count int) (model.MediaFiles, error) { +func (e *provider) TopSongs(ctx context.Context, artistName string, count int) (model.MediaFiles, error) { artist, err := e.findArtistByName(ctx, artistName) if err != nil { log.Error(ctx, "Artist not found", "name", artistName, err) return nil, nil } - return e.getMatchingTopSongs(ctx, e.ag, artist, count) + songs, err := e.getMatchingTopSongs(ctx, e.ag, artist, count) + if err != nil { + switch { + case errors.Is(err, agents.ErrNotFound): + log.Trace(ctx, "TopSongs not found", "name", artistName) + return nil, model.ErrNotFound + case errors.Is(err, context.Canceled): + log.Debug(ctx, "TopSongs call canceled", err) + default: + log.Warn(ctx, "Error getting top songs from agent", "artist", artistName, err) + } + + return nil, err + } + return songs, nil } -func (e *externalMetadata) getMatchingTopSongs(ctx context.Context, agent agents.ArtistTopSongsRetriever, artist *auxArtist, count int) (model.MediaFiles, error) { +func (e *provider) getMatchingTopSongs(ctx context.Context, agent agents.ArtistTopSongsRetriever, artist *auxArtist, count int) (model.MediaFiles, error) { songs, err := agent.GetArtistTopSongs(ctx, artist.ID, artist.Name, artist.MbzArtistID, count) - if errors.Is(err, agents.ErrNotFound) { - return nil, nil - } if err != nil { return nil, err } @@ -386,10 +419,11 @@ func (e *externalMetadata) getMatchingTopSongs(ctx context.Context, agent agents } else { log.Debug(ctx, "Found matching top songs", "name", artist.Name, "numSongs", len(mfs)) } + return mfs, nil } -func (e *externalMetadata) findMatchingTrack(ctx context.Context, mbid string, artistID, title string) (*model.MediaFile, error) { +func (e *provider) findMatchingTrack(ctx context.Context, mbid string, artistID, title string) (*model.MediaFile, error) { if mbid != "" { mfs, err := e.ds.MediaFile(ctx).GetAll(model.QueryOptions{ Filters: squirrel.And{ @@ -420,7 +454,7 @@ func (e *externalMetadata) findMatchingTrack(ctx context.Context, mbid string, a return &mfs[0], nil } -func (e *externalMetadata) callGetURL(ctx context.Context, agent agents.ArtistURLRetriever, artist *auxArtist) { +func (e *provider) callGetURL(ctx context.Context, agent agents.ArtistURLRetriever, artist *auxArtist) { artisURL, err := agent.GetArtistURL(ctx, artist.ID, artist.Name, artist.MbzArtistID) if err != nil { return @@ -428,7 +462,7 @@ func (e *externalMetadata) callGetURL(ctx context.Context, agent agents.ArtistUR artist.ExternalUrl = artisURL } -func (e *externalMetadata) callGetBiography(ctx context.Context, agent agents.ArtistBiographyRetriever, artist *auxArtist) { +func (e *provider) callGetBiography(ctx context.Context, agent agents.ArtistBiographyRetriever, artist *auxArtist) { bio, err := agent.GetArtistBiography(ctx, artist.ID, str.Clear(artist.Name), artist.MbzArtistID) if err != nil { return @@ -438,7 +472,7 @@ func (e *externalMetadata) callGetBiography(ctx context.Context, agent agents.Ar artist.Biography = strings.ReplaceAll(bio, "<a ", "<a target='_blank' ") } -func (e *externalMetadata) callGetImage(ctx context.Context, agent agents.ArtistImageRetriever, artist *auxArtist) { +func (e *provider) callGetImage(ctx context.Context, agent agents.ArtistImageRetriever, artist *auxArtist) { images, err := agent.GetArtistImages(ctx, artist.ID, artist.Name, artist.MbzArtistID) if err != nil { return @@ -456,7 +490,7 @@ func (e *externalMetadata) callGetImage(ctx context.Context, agent agents.Artist } } -func (e *externalMetadata) callGetSimilar(ctx context.Context, agent agents.ArtistSimilarRetriever, artist *auxArtist, +func (e *provider) callGetSimilar(ctx context.Context, agent agents.ArtistSimilarRetriever, artist *auxArtist, limit int, includeNotPresent bool) { similar, err := agent.GetSimilarArtists(ctx, artist.ID, artist.Name, artist.MbzArtistID, limit) if len(similar) == 0 || err != nil { @@ -471,7 +505,7 @@ func (e *externalMetadata) callGetSimilar(ctx context.Context, agent agents.Arti artist.SimilarArtists = sa } -func (e *externalMetadata) mapSimilarArtists(ctx context.Context, similar []agents.Artist, includeNotPresent bool) (model.Artists, error) { +func (e *provider) mapSimilarArtists(ctx context.Context, similar []agents.Artist, includeNotPresent bool) (model.Artists, error) { var result model.Artists var notPresent []string @@ -515,7 +549,7 @@ func (e *externalMetadata) mapSimilarArtists(ctx context.Context, similar []agen return result, nil } -func (e *externalMetadata) findArtistByName(ctx context.Context, artistName string) (*auxArtist, error) { +func (e *provider) findArtistByName(ctx context.Context, artistName string) (*auxArtist, error) { artists, err := e.ds.Artist(ctx).GetAll(model.QueryOptions{ Filters: squirrel.Like{"artist.name": artistName}, Max: 1, @@ -533,7 +567,7 @@ func (e *externalMetadata) findArtistByName(ctx context.Context, artistName stri return artist, nil } -func (e *externalMetadata) loadSimilar(ctx context.Context, artist *auxArtist, count int, includeNotPresent bool) error { +func (e *provider) loadSimilar(ctx context.Context, artist *auxArtist, count int, includeNotPresent bool) error { var ids []string for _, sa := range artist.SimilarArtists { if sa.ID == "" { diff --git a/core/external/provider_albumimage_test.go b/core/external/provider_albumimage_test.go new file mode 100644 index 000000000..e248813c1 --- /dev/null +++ b/core/external/provider_albumimage_test.go @@ -0,0 +1,303 @@ +package external_test + +import ( + "context" + "errors" + "net/url" + + "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/conf/configtest" + "github.com/navidrome/navidrome/core/agents" + . "github.com/navidrome/navidrome/core/external" + "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/tests" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/stretchr/testify/mock" +) + +var _ = Describe("Provider - AlbumImage", func() { + var ds *tests.MockDataStore + var provider Provider + var mockArtistRepo *mockArtistRepo + var mockAlbumRepo *mockAlbumRepo + var mockMediaFileRepo *mockMediaFileRepo + var mockAlbumAgent *mockAlbumInfoAgent + var agentsCombined *mockAgents + var ctx context.Context + + BeforeEach(func() { + ctx = GinkgoT().Context() + DeferCleanup(configtest.SetupConfig()) + conf.Server.Agents = "mockAlbum" // Configure mock agent + + mockArtistRepo = newMockArtistRepo() + mockAlbumRepo = newMockAlbumRepo() + mockMediaFileRepo = newMockMediaFileRepo() + + ds = &tests.MockDataStore{ + MockedArtist: mockArtistRepo, + MockedAlbum: mockAlbumRepo, + MockedMediaFile: mockMediaFileRepo, + } + + mockAlbumAgent = newMockAlbumInfoAgent() + + agentsCombined = &mockAgents{ + albumInfoAgent: mockAlbumAgent, + } + + provider = NewProvider(ds, agentsCombined) + + // Default mocks + // Mocks for GetEntityByID sequence (initial failed lookups) + mockArtistRepo.On("Get", "album-1").Return(nil, model.ErrNotFound).Once() + mockArtistRepo.On("Get", "mf-1").Return(nil, model.ErrNotFound).Once() + mockAlbumRepo.On("Get", "mf-1").Return(nil, model.ErrNotFound).Once() + + // Default mock for non-existent entities - Use Maybe() for flexibility + mockArtistRepo.On("Get", "not-found").Return(nil, model.ErrNotFound).Maybe() + mockAlbumRepo.On("Get", "not-found").Return(nil, model.ErrNotFound).Maybe() + mockMediaFileRepo.On("Get", "not-found").Return(nil, model.ErrNotFound).Maybe() + }) + + It("returns the largest image URL when successful", func() { + // Arrange + mockArtistRepo.On("Get", "album-1").Return(nil, model.ErrNotFound).Once() // Expect GetEntityByID sequence + mockAlbumRepo.On("Get", "album-1").Return(&model.Album{ID: "album-1", Name: "Album One", AlbumArtistID: "artist-1"}, nil).Once() + // Explicitly mock agent call for this test + mockAlbumAgent.On("GetAlbumInfo", ctx, "Album One", "", ""). + Return(&agents.AlbumInfo{ + Images: []agents.ExternalImage{ + {URL: "http://example.com/large.jpg", Size: 1000}, + {URL: "http://example.com/medium.jpg", Size: 500}, + {URL: "http://example.com/small.jpg", Size: 200}, + }, + }, nil).Once() + + expectedURL, _ := url.Parse("http://example.com/large.jpg") + imgURL, err := provider.AlbumImage(ctx, "album-1") + + Expect(err).ToNot(HaveOccurred()) + Expect(imgURL).To(Equal(expectedURL)) + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "album-1") // From GetEntityByID + mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "album-1") + mockArtistRepo.AssertNotCalled(GinkgoT(), "Get", "artist-1") // Artist lookup no longer happens in getAlbum + mockAlbumAgent.AssertCalled(GinkgoT(), "GetAlbumInfo", ctx, "Album One", "", "") // Expect empty artist name + }) + + It("returns ErrNotFound if the album is not found in the DB", func() { + // Arrange: Explicitly expect the full GetEntityByID sequence for "not-found" + mockArtistRepo.On("Get", "not-found").Return(nil, model.ErrNotFound).Once() + mockAlbumRepo.On("Get", "not-found").Return(nil, model.ErrNotFound).Once() + mockMediaFileRepo.On("Get", "not-found").Return(nil, model.ErrNotFound).Once() + + imgURL, err := provider.AlbumImage(ctx, "not-found") + + Expect(err).To(MatchError("data not found")) + Expect(imgURL).To(BeNil()) + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "not-found") + mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "not-found") + mockMediaFileRepo.AssertCalled(GinkgoT(), "Get", "not-found") + mockAlbumAgent.AssertNotCalled(GinkgoT(), "GetAlbumInfo", mock.Anything, mock.Anything, mock.Anything, mock.Anything) + }) + + It("returns the agent error if the agent fails", func() { + // Arrange + mockArtistRepo.On("Get", "album-1").Return(nil, model.ErrNotFound).Once() // Expect GetEntityByID sequence + mockAlbumRepo.On("Get", "album-1").Return(&model.Album{ID: "album-1", Name: "Album One", AlbumArtistID: "artist-1"}, nil).Once() + + agentErr := errors.New("agent failure") + // Explicitly mock agent call for this test + mockAlbumAgent.On("GetAlbumInfo", ctx, "Album One", "", "").Return(nil, agentErr).Once() // Expect empty artist + + imgURL, err := provider.AlbumImage(ctx, "album-1") + + Expect(err).To(MatchError("agent failure")) + Expect(imgURL).To(BeNil()) + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "album-1") + mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "album-1") + mockArtistRepo.AssertNotCalled(GinkgoT(), "Get", "artist-1") + mockAlbumAgent.AssertCalled(GinkgoT(), "GetAlbumInfo", ctx, "Album One", "", "") // Expect empty artist + }) + + It("returns ErrNotFound if the agent returns ErrNotFound", func() { + // Arrange + mockArtistRepo.On("Get", "album-1").Return(nil, model.ErrNotFound).Once() // Expect GetEntityByID sequence + mockAlbumRepo.On("Get", "album-1").Return(&model.Album{ID: "album-1", Name: "Album One", AlbumArtistID: "artist-1"}, nil).Once() + + // Explicitly mock agent call for this test + mockAlbumAgent.On("GetAlbumInfo", ctx, "Album One", "", "").Return(nil, agents.ErrNotFound).Once() // Expect empty artist + + imgURL, err := provider.AlbumImage(ctx, "album-1") + + Expect(err).To(MatchError("data not found")) + Expect(imgURL).To(BeNil()) + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "album-1") + mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "album-1") + mockAlbumAgent.AssertCalled(GinkgoT(), "GetAlbumInfo", ctx, "Album One", "", "") // Expect empty artist + }) + + It("returns ErrNotFound if the agent returns no images", func() { + // Arrange + mockArtistRepo.On("Get", "album-1").Return(nil, model.ErrNotFound).Once() // Expect GetEntityByID sequence + mockAlbumRepo.On("Get", "album-1").Return(&model.Album{ID: "album-1", Name: "Album One", AlbumArtistID: "artist-1"}, nil).Once() + + // Explicitly mock agent call for this test + mockAlbumAgent.On("GetAlbumInfo", ctx, "Album One", "", ""). + Return(&agents.AlbumInfo{Images: []agents.ExternalImage{}}, nil).Once() // Expect empty artist + + imgURL, err := provider.AlbumImage(ctx, "album-1") + + Expect(err).To(MatchError("data not found")) + Expect(imgURL).To(BeNil()) + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "album-1") + mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "album-1") + mockAlbumAgent.AssertCalled(GinkgoT(), "GetAlbumInfo", ctx, "Album One", "", "") // Expect empty artist + }) + + It("returns context error if context is canceled", func() { + // Arrange + cctx, cancelCtx := context.WithCancel(ctx) + // Mock the necessary DB calls *before* canceling the context + mockArtistRepo.On("Get", "album-1").Return(nil, model.ErrNotFound).Once() + mockAlbumRepo.On("Get", "album-1").Return(&model.Album{ID: "album-1", Name: "Album One", AlbumArtistID: "artist-1"}, nil).Once() + // Expect the agent call even if context is cancelled, returning the context error + mockAlbumAgent.On("GetAlbumInfo", cctx, "Album One", "", "").Return(nil, context.Canceled).Once() + // Cancel the context *before* calling the function under test + cancelCtx() + + imgURL, err := provider.AlbumImage(cctx, "album-1") + + Expect(err).To(MatchError("context canceled")) + Expect(imgURL).To(BeNil()) + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "album-1") + mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "album-1") + // Agent should now be called, verify this expectation + mockAlbumAgent.AssertCalled(GinkgoT(), "GetAlbumInfo", cctx, "Album One", "", "") + }) + + It("derives album ID from MediaFile ID", func() { + // Arrange: Mock full GetEntityByID for "mf-1" and recursive "album-1" + mockArtistRepo.On("Get", "mf-1").Return(nil, model.ErrNotFound).Once() + mockAlbumRepo.On("Get", "mf-1").Return(nil, model.ErrNotFound).Once() + mockMediaFileRepo.On("Get", "mf-1").Return(&model.MediaFile{ID: "mf-1", Title: "Track One", ArtistID: "artist-1", AlbumID: "album-1"}, nil).Once() + mockArtistRepo.On("Get", "album-1").Return(nil, model.ErrNotFound).Once() + mockAlbumRepo.On("Get", "album-1").Return(&model.Album{ID: "album-1", Name: "Album One", AlbumArtistID: "artist-1"}, nil).Once() + + // Explicitly mock agent call for this test + mockAlbumAgent.On("GetAlbumInfo", ctx, "Album One", "", ""). + Return(&agents.AlbumInfo{ + Images: []agents.ExternalImage{ + {URL: "http://example.com/large.jpg", Size: 1000}, + {URL: "http://example.com/medium.jpg", Size: 500}, + {URL: "http://example.com/small.jpg", Size: 200}, + }, + }, nil).Once() + + expectedURL, _ := url.Parse("http://example.com/large.jpg") + imgURL, err := provider.AlbumImage(ctx, "mf-1") + + Expect(err).ToNot(HaveOccurred()) + Expect(imgURL).To(Equal(expectedURL)) + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "mf-1") + mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "mf-1") + mockMediaFileRepo.AssertCalled(GinkgoT(), "Get", "mf-1") + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "album-1") + mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "album-1") + mockArtistRepo.AssertNotCalled(GinkgoT(), "Get", "artist-1") + mockAlbumAgent.AssertCalled(GinkgoT(), "GetAlbumInfo", ctx, "Album One", "", "") + }) + + It("handles different image orders from agent", func() { + // Arrange + mockArtistRepo.On("Get", "album-1").Return(nil, model.ErrNotFound).Once() // Expect GetEntityByID sequence + mockAlbumRepo.On("Get", "album-1").Return(&model.Album{ID: "album-1", Name: "Album One", AlbumArtistID: "artist-1"}, nil).Once() + // Explicitly mock agent call for this test + mockAlbumAgent.On("GetAlbumInfo", ctx, "Album One", "", ""). + Return(&agents.AlbumInfo{ + Images: []agents.ExternalImage{ + {URL: "http://example.com/small.jpg", Size: 200}, + {URL: "http://example.com/large.jpg", Size: 1000}, + {URL: "http://example.com/medium.jpg", Size: 500}, + }, + }, nil).Once() + + expectedURL, _ := url.Parse("http://example.com/large.jpg") + imgURL, err := provider.AlbumImage(ctx, "album-1") + + Expect(err).ToNot(HaveOccurred()) + Expect(imgURL).To(Equal(expectedURL)) // Should still pick the largest + mockAlbumAgent.AssertCalled(GinkgoT(), "GetAlbumInfo", ctx, "Album One", "", "") + }) + + It("handles agent returning only one image", func() { + // Arrange + mockArtistRepo.On("Get", "album-1").Return(nil, model.ErrNotFound).Once() // Expect GetEntityByID sequence + mockAlbumRepo.On("Get", "album-1").Return(&model.Album{ID: "album-1", Name: "Album One", AlbumArtistID: "artist-1"}, nil).Once() + // Explicitly mock agent call for this test + mockAlbumAgent.On("GetAlbumInfo", ctx, "Album One", "", ""). + Return(&agents.AlbumInfo{ + Images: []agents.ExternalImage{ + {URL: "http://example.com/single.jpg", Size: 700}, + }, + }, nil).Once() + + expectedURL, _ := url.Parse("http://example.com/single.jpg") + imgURL, err := provider.AlbumImage(ctx, "album-1") + + Expect(err).ToNot(HaveOccurred()) + Expect(imgURL).To(Equal(expectedURL)) + mockAlbumAgent.AssertCalled(GinkgoT(), "GetAlbumInfo", ctx, "Album One", "", "") + }) + + It("returns ErrNotFound if deriving album ID fails", func() { + // Arrange: Mock full GetEntityByID for "mf-no-album" and recursive "not-found" + mockArtistRepo.On("Get", "mf-no-album").Return(nil, model.ErrNotFound).Once() + mockAlbumRepo.On("Get", "mf-no-album").Return(nil, model.ErrNotFound).Once() + mockMediaFileRepo.On("Get", "mf-no-album").Return(&model.MediaFile{ID: "mf-no-album", Title: "Track No Album", ArtistID: "artist-1", AlbumID: "not-found"}, nil).Once() + mockArtistRepo.On("Get", "not-found").Return(nil, model.ErrNotFound).Once() + mockAlbumRepo.On("Get", "not-found").Return(nil, model.ErrNotFound).Once() + mockMediaFileRepo.On("Get", "not-found").Return(nil, model.ErrNotFound).Once() + + imgURL, err := provider.AlbumImage(ctx, "mf-no-album") + + Expect(err).To(MatchError("data not found")) + Expect(imgURL).To(BeNil()) + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "mf-no-album") + mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "mf-no-album") + mockMediaFileRepo.AssertCalled(GinkgoT(), "Get", "mf-no-album") + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "not-found") + mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "not-found") + mockMediaFileRepo.AssertCalled(GinkgoT(), "Get", "not-found") + mockAlbumAgent.AssertNotCalled(GinkgoT(), "GetAlbumInfo", mock.Anything, mock.Anything, mock.Anything, mock.Anything) + }) +}) + +// mockAlbumInfoAgent implementation +type mockAlbumInfoAgent struct { + mock.Mock + agents.AlbumInfoRetriever // Embed interface +} + +func newMockAlbumInfoAgent() *mockAlbumInfoAgent { + m := new(mockAlbumInfoAgent) + m.On("AgentName").Return("mockAlbum").Maybe() + return m +} + +func (m *mockAlbumInfoAgent) AgentName() string { + args := m.Called() + return args.String(0) +} + +func (m *mockAlbumInfoAgent) GetAlbumInfo(ctx context.Context, name, artist, mbid string) (*agents.AlbumInfo, error) { + args := m.Called(ctx, name, artist, mbid) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).(*agents.AlbumInfo), args.Error(1) +} + +// Ensure mockAgent implements the interface +var _ agents.AlbumInfoRetriever = (*mockAlbumInfoAgent)(nil) diff --git a/core/external/provider_artistimage_test.go b/core/external/provider_artistimage_test.go new file mode 100644 index 000000000..96341836a --- /dev/null +++ b/core/external/provider_artistimage_test.go @@ -0,0 +1,301 @@ +package external_test + +import ( + "context" + "errors" + "net/url" + + "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/conf/configtest" + "github.com/navidrome/navidrome/core/agents" + . "github.com/navidrome/navidrome/core/external" + "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/tests" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/stretchr/testify/mock" +) + +var _ = Describe("Provider - ArtistImage", func() { + var ds *tests.MockDataStore + var provider Provider + var mockArtistRepo *mockArtistRepo + var mockAlbumRepo *mockAlbumRepo + var mockMediaFileRepo *mockMediaFileRepo + var mockImageAgent *mockArtistImageAgent + var agentsCombined *mockAgents + var ctx context.Context + + BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + conf.Server.Agents = "mockImage" // Configure only the mock agent + ctx = GinkgoT().Context() + + mockArtistRepo = newMockArtistRepo() + mockAlbumRepo = newMockAlbumRepo() + mockMediaFileRepo = newMockMediaFileRepo() + + ds = &tests.MockDataStore{ + MockedArtist: mockArtistRepo, + MockedAlbum: mockAlbumRepo, + MockedMediaFile: mockMediaFileRepo, + } + + mockImageAgent = newMockArtistImageAgent() + + // Use the mockAgents from helper, setting the specific agent + agentsCombined = &mockAgents{ + imageAgent: mockImageAgent, + } + + provider = NewProvider(ds, agentsCombined) + + // Default mocks for successful Get calls + mockArtistRepo.On("Get", "artist-1").Return(&model.Artist{ID: "artist-1", Name: "Artist One"}, nil).Maybe() + mockAlbumRepo.On("Get", "album-1").Return(&model.Album{ID: "album-1", Name: "Album One", AlbumArtistID: "artist-1"}, nil).Maybe() + mockMediaFileRepo.On("Get", "mf-1").Return(&model.MediaFile{ID: "mf-1", Title: "Track One", ArtistID: "artist-1"}, nil).Maybe() + // Default mock for non-existent entities + mockArtistRepo.On("Get", "not-found").Return(nil, model.ErrNotFound).Maybe() + mockAlbumRepo.On("Get", "not-found").Return(nil, model.ErrNotFound).Maybe() + mockMediaFileRepo.On("Get", "not-found").Return(nil, model.ErrNotFound).Maybe() + + // Default successful image agent response + mockImageAgent.On("GetArtistImages", mock.Anything, "artist-1", "Artist One", ""). + Return([]agents.ExternalImage{ + {URL: "http://example.com/large.jpg", Size: 1000}, + {URL: "http://example.com/medium.jpg", Size: 500}, + {URL: "http://example.com/small.jpg", Size: 200}, + }, nil).Maybe() + }) + + AfterEach(func() { + mockArtistRepo.AssertExpectations(GinkgoT()) + mockAlbumRepo.AssertExpectations(GinkgoT()) + mockMediaFileRepo.AssertExpectations(GinkgoT()) + mockImageAgent.AssertExpectations(GinkgoT()) + }) + + It("returns the largest image URL when successful", func() { + // Arrange + expectedURL, _ := url.Parse("http://example.com/large.jpg") + + // Act + imgURL, err := provider.ArtistImage(ctx, "artist-1") + + // Assert + Expect(err).ToNot(HaveOccurred()) + Expect(imgURL).To(Equal(expectedURL)) + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "artist-1") + mockImageAgent.AssertCalled(GinkgoT(), "GetArtistImages", ctx, "artist-1", "Artist One", "") + }) + + It("returns ErrNotFound if the artist is not found in the DB", func() { + // Arrange + + // Act + imgURL, err := provider.ArtistImage(ctx, "not-found") + + // Assert + Expect(err).To(MatchError(model.ErrNotFound)) + Expect(imgURL).To(BeNil()) + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "not-found") + mockImageAgent.AssertNotCalled(GinkgoT(), "GetArtistImages", mock.Anything, mock.Anything, mock.Anything, mock.Anything) + }) + + It("returns the agent error if the agent fails", func() { + // Arrange + agentErr := errors.New("agent failure") + mockImageAgent.Mock = mock.Mock{} // Reset default expectation + mockImageAgent.On("GetArtistImages", ctx, "artist-1", "Artist One", "").Return(nil, agentErr).Once() + + // Act + imgURL, err := provider.ArtistImage(ctx, "artist-1") + + // Assert + Expect(err).To(MatchError(model.ErrNotFound)) // Corrected Expectation: The provider maps agent errors (other than canceled) to ErrNotFound if no image was found/populated + Expect(imgURL).To(BeNil()) + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "artist-1") + mockImageAgent.AssertCalled(GinkgoT(), "GetArtistImages", ctx, "artist-1", "Artist One", "") + }) + + It("returns ErrNotFound if the agent returns ErrNotFound", func() { + // Arrange + mockImageAgent.Mock = mock.Mock{} // Reset default expectation + mockImageAgent.On("GetArtistImages", ctx, "artist-1", "Artist One", "").Return(nil, agents.ErrNotFound).Once() + + // Act + imgURL, err := provider.ArtistImage(ctx, "artist-1") + + // Assert + Expect(err).To(MatchError(model.ErrNotFound)) + Expect(imgURL).To(BeNil()) + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "artist-1") + mockImageAgent.AssertCalled(GinkgoT(), "GetArtistImages", ctx, "artist-1", "Artist One", "") + }) + + It("returns ErrNotFound if the agent returns no images", func() { + // Arrange + mockImageAgent.Mock = mock.Mock{} // Reset default expectation + mockImageAgent.On("GetArtistImages", ctx, "artist-1", "Artist One", "").Return([]agents.ExternalImage{}, nil).Once() + + // Act + imgURL, err := provider.ArtistImage(ctx, "artist-1") + + // Assert + Expect(err).To(MatchError(model.ErrNotFound)) // Implementation maps empty result to ErrNotFound + Expect(imgURL).To(BeNil()) + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "artist-1") + mockImageAgent.AssertCalled(GinkgoT(), "GetArtistImages", ctx, "artist-1", "Artist One", "") + }) + + It("returns context error if context is canceled before agent call", func() { + // Arrange + cctx, cancelCtx := context.WithCancel(context.Background()) + mockArtistRepo.Mock = mock.Mock{} // Reset default expectation for artist repo as well + mockArtistRepo.On("Get", "artist-1").Return(&model.Artist{ID: "artist-1", Name: "Artist One"}, nil).Run(func(args mock.Arguments) { + cancelCtx() // Cancel context *during* the DB call simulation + }).Once() + + // Act + imgURL, err := provider.ArtistImage(cctx, "artist-1") + + // Assert + Expect(err).To(MatchError(context.Canceled)) + Expect(imgURL).To(BeNil()) + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "artist-1") + }) + + It("derives artist ID from MediaFile ID", func() { + // Arrange: Add mocks for the initial GetEntityByID lookups + mockArtistRepo.On("Get", "mf-1").Return(nil, model.ErrNotFound).Once() + mockAlbumRepo.On("Get", "mf-1").Return(nil, model.ErrNotFound).Once() + // Default mocks for MediaFileRepo.Get("mf-1") and ArtistRepo.Get("artist-1") handle the rest + expectedURL, _ := url.Parse("http://example.com/large.jpg") + + // Act + imgURL, err := provider.ArtistImage(ctx, "mf-1") + + // Assert + Expect(err).ToNot(HaveOccurred()) + Expect(imgURL).To(Equal(expectedURL)) + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "mf-1") // GetEntityByID sequence + mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "mf-1") // GetEntityByID sequence + mockMediaFileRepo.AssertCalled(GinkgoT(), "Get", "mf-1") + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "artist-1") // Should be called after getting MF + mockImageAgent.AssertCalled(GinkgoT(), "GetArtistImages", ctx, "artist-1", "Artist One", "") + }) + + It("derives artist ID from Album ID", func() { + // Arrange: Add mock for the initial GetEntityByID lookup + mockArtistRepo.On("Get", "album-1").Return(nil, model.ErrNotFound).Once() + // Default mocks for AlbumRepo.Get("album-1") and ArtistRepo.Get("artist-1") handle the rest + expectedURL, _ := url.Parse("http://example.com/large.jpg") + + // Act + imgURL, err := provider.ArtistImage(ctx, "album-1") + + // Assert + Expect(err).ToNot(HaveOccurred()) + Expect(imgURL).To(Equal(expectedURL)) + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "album-1") // GetEntityByID sequence + mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "album-1") + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "artist-1") // Should be called after getting Album + mockImageAgent.AssertCalled(GinkgoT(), "GetArtistImages", ctx, "artist-1", "Artist One", "") + }) + + It("returns ErrNotFound if derived artist is not found", func() { + // Arrange + // Add mocks for the initial GetEntityByID lookups + mockArtistRepo.On("Get", "mf-bad-artist").Return(nil, model.ErrNotFound).Once() + mockAlbumRepo.On("Get", "mf-bad-artist").Return(nil, model.ErrNotFound).Once() + mockMediaFileRepo.On("Get", "mf-bad-artist").Return(&model.MediaFile{ID: "mf-bad-artist", ArtistID: "not-found"}, nil).Once() + // Add expectation for the recursive GetEntityByID call for the MediaFileRepo + mockMediaFileRepo.On("Get", "not-found").Return(nil, model.ErrNotFound).Maybe() + // The default mocks for ArtistRepo/AlbumRepo handle the final "not-found" lookups + + // Act + imgURL, err := provider.ArtistImage(ctx, "mf-bad-artist") + + // Assert + Expect(err).To(MatchError(model.ErrNotFound)) + Expect(imgURL).To(BeNil()) + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "mf-bad-artist") // GetEntityByID sequence + mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "mf-bad-artist") // GetEntityByID sequence + mockMediaFileRepo.AssertCalled(GinkgoT(), "Get", "mf-bad-artist") + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "not-found") + mockImageAgent.AssertNotCalled(GinkgoT(), "GetArtistImages", mock.Anything, mock.Anything, mock.Anything, mock.Anything) + }) + + It("handles different image orders from agent", func() { + // Arrange + mockImageAgent.Mock = mock.Mock{} // Reset default expectation + mockImageAgent.On("GetArtistImages", ctx, "artist-1", "Artist One", ""). + Return([]agents.ExternalImage{ + {URL: "http://example.com/small.jpg", Size: 200}, + {URL: "http://example.com/large.jpg", Size: 1000}, + {URL: "http://example.com/medium.jpg", Size: 500}, + }, nil).Once() + expectedURL, _ := url.Parse("http://example.com/large.jpg") + + // Act + imgURL, err := provider.ArtistImage(ctx, "artist-1") + + // Assert + Expect(err).ToNot(HaveOccurred()) + Expect(imgURL).To(Equal(expectedURL)) // Still picks the largest + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "artist-1") + mockImageAgent.AssertCalled(GinkgoT(), "GetArtistImages", ctx, "artist-1", "Artist One", "") + }) + + It("handles agent returning only one image", func() { + // Arrange + mockImageAgent.Mock = mock.Mock{} // Reset default expectation + mockImageAgent.On("GetArtistImages", ctx, "artist-1", "Artist One", ""). + Return([]agents.ExternalImage{ + {URL: "http://example.com/medium.jpg", Size: 500}, + }, nil).Once() + expectedURL, _ := url.Parse("http://example.com/medium.jpg") + + // Act + imgURL, err := provider.ArtistImage(ctx, "artist-1") + + // Assert + Expect(err).ToNot(HaveOccurred()) + Expect(imgURL).To(Equal(expectedURL)) + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "artist-1") + mockImageAgent.AssertCalled(GinkgoT(), "GetArtistImages", ctx, "artist-1", "Artist One", "") + }) +}) + +// mockArtistImageAgent implementation using testify/mock +// This remains local as it's specific to testing the ArtistImage functionality +type mockArtistImageAgent struct { + mock.Mock + agents.ArtistImageRetriever // Embed interface +} + +// Constructor for the mock agent +func newMockArtistImageAgent() *mockArtistImageAgent { + mock := new(mockArtistImageAgent) + // Set default AgentName if needed, although usually called via mockAgents + mock.On("AgentName").Return("mockImage").Maybe() + return mock +} + +func (m *mockArtistImageAgent) AgentName() string { + args := m.Called() + return args.String(0) +} + +func (m *mockArtistImageAgent) GetArtistImages(ctx context.Context, id, artistName, mbid string) ([]agents.ExternalImage, error) { + args := m.Called(ctx, id, artistName, mbid) + // Need careful type assertion for potentially nil slice + var res []agents.ExternalImage + if args.Get(0) != nil { + res = args.Get(0).([]agents.ExternalImage) + } + return res, args.Error(1) +} + +// Ensure mockAgent implements the interface +var _ agents.ArtistImageRetriever = (*mockArtistImageAgent)(nil) diff --git a/core/external/provider_similarsongs_test.go b/core/external/provider_similarsongs_test.go new file mode 100644 index 000000000..fd622746a --- /dev/null +++ b/core/external/provider_similarsongs_test.go @@ -0,0 +1,198 @@ +package external_test + +import ( + "context" + "errors" + + "github.com/navidrome/navidrome/core/agents" + . "github.com/navidrome/navidrome/core/external" + "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/tests" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/stretchr/testify/mock" +) + +var _ = Describe("Provider - SimilarSongs", func() { + var ds model.DataStore + var provider Provider + var mockAgent *mockSimilarArtistAgent + var mockTopAgent agents.ArtistTopSongsRetriever + var mockSimilarAgent agents.ArtistSimilarRetriever + var agentsCombined Agents + var artistRepo *mockArtistRepo + var mediaFileRepo *mockMediaFileRepo + var ctx context.Context + + BeforeEach(func() { + ctx = GinkgoT().Context() + + artistRepo = newMockArtistRepo() + mediaFileRepo = newMockMediaFileRepo() + + ds = &tests.MockDataStore{ + MockedArtist: artistRepo, + MockedMediaFile: mediaFileRepo, + } + + mockAgent = &mockSimilarArtistAgent{} + mockTopAgent = mockAgent + mockSimilarAgent = mockAgent + + agentsCombined = &mockAgents{ + topSongsAgent: mockTopAgent, + similarAgent: mockSimilarAgent, + } + + provider = NewProvider(ds, agentsCombined) + }) + + It("returns similar songs from main artist and similar artists", func() { + artist1 := model.Artist{ID: "artist-1", Name: "Artist One"} + similarArtist := model.Artist{ID: "artist-3", Name: "Similar Artist"} + song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1"} + song2 := model.MediaFile{ID: "song-2", Title: "Song Two", ArtistID: "artist-1"} + song3 := model.MediaFile{ID: "song-3", Title: "Song Three", ArtistID: "artist-3"} + + artistRepo.On("Get", "artist-1").Return(&artist1, nil).Maybe() + artistRepo.On("Get", "artist-3").Return(&similarArtist, nil).Maybe() + + artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + return opt.Max == 1 && opt.Filters != nil + })).Return(model.Artists{artist1}, nil).Once() + + similarAgentsResp := []agents.Artist{ + {Name: "Similar Artist", MBID: "similar-mbid"}, + } + mockAgent.On("GetSimilarArtists", mock.Anything, "artist-1", "Artist One", "", 15). + Return(similarAgentsResp, nil).Once() + + artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + return opt.Max == 0 && opt.Filters != nil + })).Return(model.Artists{similarArtist}, nil).Once() + + mockAgent.On("GetArtistTopSongs", mock.Anything, "artist-1", "Artist One", "", mock.Anything). + Return([]agents.Song{ + {Name: "Song One", MBID: "mbid-1"}, + {Name: "Song Two", MBID: "mbid-2"}, + }, nil).Once() + + mockAgent.On("GetArtistTopSongs", mock.Anything, "artist-3", "Similar Artist", "", mock.Anything). + Return([]agents.Song{ + {Name: "Song Three", MBID: "mbid-3"}, + }, nil).Once() + + mediaFileRepo.FindByMBID("mbid-1", song1) + mediaFileRepo.FindByMBID("mbid-2", song2) + mediaFileRepo.FindByMBID("mbid-3", song3) + + songs, err := provider.SimilarSongs(ctx, "artist-1", 3) + + Expect(err).ToNot(HaveOccurred()) + Expect(songs).To(HaveLen(3)) + for _, song := range songs { + Expect(song.ID).To(BeElementOf("song-1", "song-2", "song-3")) + } + }) + + It("returns ErrNotFound when artist is not found", func() { + artistRepo.On("Get", "artist-unknown-artist").Return(nil, model.ErrNotFound) + mediaFileRepo.On("Get", "artist-unknown-artist").Return(nil, model.ErrNotFound) + + artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + return opt.Max == 1 && opt.Filters != nil + })).Return(model.Artists{}, nil).Maybe() + + songs, err := provider.SimilarSongs(ctx, "artist-unknown-artist", 5) + + Expect(err).To(Equal(model.ErrNotFound)) + Expect(songs).To(BeNil()) + }) + + It("returns songs from main artist when GetSimilarArtists returns error", func() { + artist1 := model.Artist{ID: "artist-1", Name: "Artist One"} + song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1"} + + artistRepo.On("Get", "artist-1").Return(&artist1, nil).Maybe() + artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + return opt.Max == 1 && opt.Filters != nil + })).Return(model.Artists{artist1}, nil).Maybe() + + mockAgent.On("GetSimilarArtists", mock.Anything, "artist-1", "Artist One", "", 15). + Return(nil, errors.New("error getting similar artists")).Once() + + artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + return opt.Max == 0 && opt.Filters != nil + })).Return(model.Artists{}, nil).Once() + + mockAgent.On("GetArtistTopSongs", mock.Anything, "artist-1", "Artist One", "", mock.Anything). + Return([]agents.Song{ + {Name: "Song One", MBID: "mbid-1"}, + }, nil).Once() + + mediaFileRepo.FindByMBID("mbid-1", song1) + + songs, err := provider.SimilarSongs(ctx, "artist-1", 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(songs).To(HaveLen(1)) + Expect(songs[0].ID).To(Equal("song-1")) + }) + + It("returns empty list when GetArtistTopSongs returns error", func() { + artist1 := model.Artist{ID: "artist-1", Name: "Artist One"} + + artistRepo.On("Get", "artist-1").Return(&artist1, nil).Maybe() + artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + return opt.Max == 1 && opt.Filters != nil + })).Return(model.Artists{artist1}, nil).Maybe() + + mockAgent.On("GetSimilarArtists", mock.Anything, "artist-1", "Artist One", "", 15). + Return([]agents.Artist{}, nil).Once() + + artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + return opt.Max == 0 && opt.Filters != nil + })).Return(model.Artists{}, nil).Once() + + mockAgent.On("GetArtistTopSongs", mock.Anything, "artist-1", "Artist One", "", mock.Anything). + Return(nil, errors.New("error getting top songs")).Once() + + songs, err := provider.SimilarSongs(ctx, "artist-1", 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(songs).To(BeEmpty()) + }) + + It("respects count parameter", func() { + artist1 := model.Artist{ID: "artist-1", Name: "Artist One"} + song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1"} + song2 := model.MediaFile{ID: "song-2", Title: "Song Two", ArtistID: "artist-1"} + + artistRepo.On("Get", "artist-1").Return(&artist1, nil).Maybe() + artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + return opt.Max == 1 && opt.Filters != nil + })).Return(model.Artists{artist1}, nil).Maybe() + + mockAgent.On("GetSimilarArtists", mock.Anything, "artist-1", "Artist One", "", 15). + Return([]agents.Artist{}, nil).Once() + + artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + return opt.Max == 0 && opt.Filters != nil + })).Return(model.Artists{}, nil).Once() + + mockAgent.On("GetArtistTopSongs", mock.Anything, "artist-1", "Artist One", "", mock.Anything). + Return([]agents.Song{ + {Name: "Song One", MBID: "mbid-1"}, + {Name: "Song Two", MBID: "mbid-2"}, + }, nil).Once() + + mediaFileRepo.FindByMBID("mbid-1", song1) + mediaFileRepo.FindByMBID("mbid-2", song2) + + songs, err := provider.SimilarSongs(ctx, "artist-1", 1) + + Expect(err).ToNot(HaveOccurred()) + Expect(songs).To(HaveLen(1)) + Expect(songs[0].ID).To(BeElementOf("song-1", "song-2")) + }) +}) diff --git a/core/external/provider_topsongs_test.go b/core/external/provider_topsongs_test.go new file mode 100644 index 000000000..4ce7911de --- /dev/null +++ b/core/external/provider_topsongs_test.go @@ -0,0 +1,193 @@ +package external_test + +import ( + "context" + "errors" + + "github.com/navidrome/navidrome/core/agents" + _ "github.com/navidrome/navidrome/core/agents/lastfm" + _ "github.com/navidrome/navidrome/core/agents/listenbrainz" + _ "github.com/navidrome/navidrome/core/agents/spotify" + . "github.com/navidrome/navidrome/core/external" + "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/tests" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/stretchr/testify/mock" +) + +var _ = Describe("Provider - TopSongs", func() { + var ( + p Provider + artistRepo *mockArtistRepo // From provider_helper_test.go + mediaFileRepo *mockMediaFileRepo // From provider_helper_test.go + ag *mockAgents // Consolidated mock from export_test.go + ctx context.Context + ) + + BeforeEach(func() { + ctx = GinkgoT().Context() + + artistRepo = newMockArtistRepo() // Use helper mock + mediaFileRepo = newMockMediaFileRepo() // Use helper mock + + // Configure tests.MockDataStore to use the testify/mock-based repos + ds := &tests.MockDataStore{ + MockedArtist: artistRepo, + MockedMediaFile: mediaFileRepo, + } + + ag = new(mockAgents) + + p = NewProvider(ds, ag) + }) + + BeforeEach(func() { + // Setup expectations in individual tests + }) + + It("returns top songs for a known artist", func() { + // Mock finding the artist + artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"} + artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once() + + // Mock agent response + agentSongs := []agents.Song{ + {Name: "Song One", MBID: "mbid-song-1"}, + {Name: "Song Two", MBID: "mbid-song-2"}, + } + ag.On("GetArtistTopSongs", ctx, "artist-1", "Artist One", "mbid-artist-1", 2).Return(agentSongs, nil).Once() + + // Mock finding matching tracks + song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1", MbzRecordingID: "mbid-song-1"} + song2 := model.MediaFile{ID: "song-2", Title: "Song Two", ArtistID: "artist-1", MbzRecordingID: "mbid-song-2"} + mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song1}, nil).Once() + mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song2}, nil).Once() + + songs, err := p.TopSongs(ctx, "Artist One", 2) + + Expect(err).ToNot(HaveOccurred()) + Expect(songs).To(HaveLen(2)) + Expect(songs[0].ID).To(Equal("song-1")) + Expect(songs[1].ID).To(Equal("song-2")) + artistRepo.AssertExpectations(GinkgoT()) + ag.AssertExpectations(GinkgoT()) + mediaFileRepo.AssertExpectations(GinkgoT()) + }) + + It("returns nil for an unknown artist", func() { + // Mock artist not found + artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{}, nil).Once() + + songs, err := p.TopSongs(ctx, "Unknown Artist", 5) + + Expect(err).ToNot(HaveOccurred()) // TopSongs returns nil error if artist not found + Expect(songs).To(BeNil()) + artistRepo.AssertExpectations(GinkgoT()) + ag.AssertNotCalled(GinkgoT(), "GetArtistTopSongs", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything) + }) + + It("returns error when the agent returns an error", func() { + // Mock finding the artist + artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"} + artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once() + + // Mock agent error + agentErr := errors.New("agent error") + ag.On("GetArtistTopSongs", ctx, "artist-1", "Artist One", "mbid-artist-1", 5).Return(nil, agentErr).Once() + + songs, err := p.TopSongs(ctx, "Artist One", 5) + + Expect(err).To(MatchError(agentErr)) + Expect(songs).To(BeNil()) + artistRepo.AssertExpectations(GinkgoT()) + ag.AssertExpectations(GinkgoT()) + }) + + It("returns ErrNotFound when the agent returns ErrNotFound", func() { + // Mock finding the artist + artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"} + artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once() + + // Mock agent ErrNotFound + ag.On("GetArtistTopSongs", ctx, "artist-1", "Artist One", "mbid-artist-1", 5).Return(nil, agents.ErrNotFound).Once() + + songs, err := p.TopSongs(ctx, "Artist One", 5) + + Expect(err).To(MatchError(model.ErrNotFound)) + Expect(songs).To(BeNil()) + artistRepo.AssertExpectations(GinkgoT()) + ag.AssertExpectations(GinkgoT()) + }) + + It("returns fewer songs if count is less than available top songs", func() { + // Mock finding the artist + artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"} + artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once() + + // Mock agent response (only need 1 for the test) + agentSongs := []agents.Song{{Name: "Song One", MBID: "mbid-song-1"}} + ag.On("GetArtistTopSongs", ctx, "artist-1", "Artist One", "mbid-artist-1", 1).Return(agentSongs, nil).Once() + + // Mock finding matching track + song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1", MbzRecordingID: "mbid-song-1"} + mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song1}, nil).Once() + + songs, err := p.TopSongs(ctx, "Artist One", 1) + + Expect(err).ToNot(HaveOccurred()) + Expect(songs).To(HaveLen(1)) + Expect(songs[0].ID).To(Equal("song-1")) + artistRepo.AssertExpectations(GinkgoT()) + ag.AssertExpectations(GinkgoT()) + mediaFileRepo.AssertExpectations(GinkgoT()) + }) + + It("returns fewer songs if fewer matching tracks are found", func() { + // Mock finding the artist + artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"} + artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once() + + // Mock agent response + agentSongs := []agents.Song{ + {Name: "Song One", MBID: "mbid-song-1"}, + {Name: "Song Two", MBID: "mbid-song-2"}, + } + ag.On("GetArtistTopSongs", ctx, "artist-1", "Artist One", "mbid-artist-1", 2).Return(agentSongs, nil).Once() + + // Mock finding matching tracks (only find song 1) + song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1", MbzRecordingID: "mbid-song-1"} + mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song1}, nil).Once() + mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{}, nil).Once() // For mbid-song-2 (fails) + mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{}, nil).Once() // For title fallback (fails) + + songs, err := p.TopSongs(ctx, "Artist One", 2) + + Expect(err).ToNot(HaveOccurred()) + Expect(songs).To(HaveLen(1)) + Expect(songs[0].ID).To(Equal("song-1")) + artistRepo.AssertExpectations(GinkgoT()) + ag.AssertExpectations(GinkgoT()) + mediaFileRepo.AssertExpectations(GinkgoT()) + }) + + It("returns error when context is canceled during agent call", func() { + // Mock finding the artist + artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"} + artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once() + + // Setup context that will be canceled + canceledCtx, cancel := context.WithCancel(ctx) + + // Mock agent call to return context canceled error + ag.On("GetArtistTopSongs", canceledCtx, "artist-1", "Artist One", "mbid-artist-1", 5).Return(nil, context.Canceled).Once() + + cancel() // Cancel the context before calling + songs, err := p.TopSongs(canceledCtx, "Artist One", 5) + + Expect(err).To(MatchError(context.Canceled)) + Expect(songs).To(BeNil()) + artistRepo.AssertExpectations(GinkgoT()) + ag.AssertExpectations(GinkgoT()) + }) +}) diff --git a/core/external/provider_updatealbuminfo_test.go b/core/external/provider_updatealbuminfo_test.go new file mode 100644 index 000000000..0622849f0 --- /dev/null +++ b/core/external/provider_updatealbuminfo_test.go @@ -0,0 +1,170 @@ +package external_test + +import ( + "context" + "errors" + "time" + + "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/core/agents" + "github.com/navidrome/navidrome/core/external" + "github.com/navidrome/navidrome/log" + "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/tests" + "github.com/navidrome/navidrome/utils/gg" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/stretchr/testify/mock" +) + +func init() { + log.SetLevel(log.LevelDebug) +} + +var _ = Describe("Provider - UpdateAlbumInfo", func() { + var ( + ctx context.Context + p external.Provider + ds *tests.MockDataStore + ag *mockAgents + mockAlbumRepo *tests.MockAlbumRepo + ) + + BeforeEach(func() { + ctx = GinkgoT().Context() + ds = new(tests.MockDataStore) + ag = new(mockAgents) + p = external.NewProvider(ds, ag) + mockAlbumRepo = ds.Album(ctx).(*tests.MockAlbumRepo) + conf.Server.DevAlbumInfoTimeToLive = 1 * time.Hour + }) + + It("returns error when album is not found", func() { + album, err := p.UpdateAlbumInfo(ctx, "al-not-found") + + Expect(err).To(MatchError(model.ErrNotFound)) + Expect(album).To(BeNil()) + ag.AssertNotCalled(GinkgoT(), "GetAlbumInfo", mock.Anything, mock.Anything, mock.Anything, mock.Anything) + }) + + It("populates info when album exists but has no external info", func() { + originalAlbum := &model.Album{ + ID: "al-existing", + Name: "Test Album", + AlbumArtist: "Test Artist", + MbzAlbumID: "mbid-album", + } + mockAlbumRepo.SetData(model.Albums{*originalAlbum}) + + expectedInfo := &agents.AlbumInfo{ + URL: "http://example.com/album", + Description: "Album Description", + Images: []agents.ExternalImage{ + {URL: "http://example.com/large.jpg", Size: 300}, + {URL: "http://example.com/medium.jpg", Size: 200}, + {URL: "http://example.com/small.jpg", Size: 100}, + }, + } + ag.On("GetAlbumInfo", ctx, "Test Album", "Test Artist", "mbid-album").Return(expectedInfo, nil) + + updatedAlbum, err := p.UpdateAlbumInfo(ctx, "al-existing") + + Expect(err).NotTo(HaveOccurred()) + Expect(updatedAlbum).NotTo(BeNil()) + Expect(updatedAlbum.ID).To(Equal("al-existing")) + Expect(updatedAlbum.ExternalUrl).To(Equal("http://example.com/album")) + Expect(updatedAlbum.Description).To(Equal("Album Description")) + Expect(updatedAlbum.LargeImageUrl).To(Equal("http://example.com/large.jpg")) + Expect(updatedAlbum.MediumImageUrl).To(Equal("http://example.com/medium.jpg")) + Expect(updatedAlbum.SmallImageUrl).To(Equal("http://example.com/small.jpg")) + Expect(updatedAlbum.ExternalInfoUpdatedAt).NotTo(BeNil()) + Expect(*updatedAlbum.ExternalInfoUpdatedAt).To(BeTemporally("~", time.Now(), time.Second)) + + ag.AssertExpectations(GinkgoT()) + }) + + It("returns cached info when album exists and info is not expired", func() { + now := time.Now() + originalAlbum := &model.Album{ + ID: "al-cached", + Name: "Cached Album", + AlbumArtist: "Cached Artist", + ExternalUrl: "http://cached.com/album", + Description: "Cached Desc", + LargeImageUrl: "http://cached.com/large.jpg", + ExternalInfoUpdatedAt: gg.P(now.Add(-conf.Server.DevAlbumInfoTimeToLive / 2)), + } + mockAlbumRepo.SetData(model.Albums{*originalAlbum}) + + updatedAlbum, err := p.UpdateAlbumInfo(ctx, "al-cached") + + Expect(err).NotTo(HaveOccurred()) + Expect(updatedAlbum).NotTo(BeNil()) + Expect(*updatedAlbum).To(Equal(*originalAlbum)) + + ag.AssertNotCalled(GinkgoT(), "GetAlbumInfo", mock.Anything, mock.Anything, mock.Anything, mock.Anything) + }) + + It("returns cached info and triggers background refresh when info is expired", func() { + now := time.Now() + expiredTime := now.Add(-conf.Server.DevAlbumInfoTimeToLive * 2) + originalAlbum := &model.Album{ + ID: "al-expired", + Name: "Expired Album", + AlbumArtist: "Expired Artist", + ExternalUrl: "http://expired.com/album", + Description: "Expired Desc", + LargeImageUrl: "http://expired.com/large.jpg", + ExternalInfoUpdatedAt: gg.P(expiredTime), + } + mockAlbumRepo.SetData(model.Albums{*originalAlbum}) + + updatedAlbum, err := p.UpdateAlbumInfo(ctx, "al-expired") + + Expect(err).NotTo(HaveOccurred()) + Expect(updatedAlbum).NotTo(BeNil()) + Expect(*updatedAlbum).To(Equal(*originalAlbum)) + + ag.AssertNotCalled(GinkgoT(), "GetAlbumInfo", mock.Anything, mock.Anything, mock.Anything, mock.Anything) + }) + + It("returns error when agent fails to get album info", func() { + originalAlbum := &model.Album{ + ID: "al-agent-error", + Name: "Agent Error Album", + AlbumArtist: "Agent Error Artist", + MbzAlbumID: "mbid-agent-error", + } + mockAlbumRepo.SetData(model.Albums{*originalAlbum}) + + expectedErr := errors.New("agent communication failed") + ag.On("GetAlbumInfo", ctx, "Agent Error Album", "Agent Error Artist", "mbid-agent-error").Return(nil, expectedErr) + + updatedAlbum, err := p.UpdateAlbumInfo(ctx, "al-agent-error") + + Expect(err).To(MatchError(expectedErr)) + Expect(updatedAlbum).To(BeNil()) + ag.AssertExpectations(GinkgoT()) + }) + + It("returns original album when agent returns ErrNotFound", func() { + originalAlbum := &model.Album{ + ID: "al-agent-notfound", + Name: "Agent NotFound Album", + AlbumArtist: "Agent NotFound Artist", + MbzAlbumID: "mbid-agent-notfound", + } + mockAlbumRepo.SetData(model.Albums{*originalAlbum}) + + ag.On("GetAlbumInfo", ctx, "Agent NotFound Album", "Agent NotFound Artist", "mbid-agent-notfound").Return(nil, agents.ErrNotFound) + + updatedAlbum, err := p.UpdateAlbumInfo(ctx, "al-agent-notfound") + + Expect(err).NotTo(HaveOccurred()) + Expect(updatedAlbum).NotTo(BeNil()) + Expect(*updatedAlbum).To(Equal(*originalAlbum)) + Expect(updatedAlbum.ExternalInfoUpdatedAt).To(BeNil()) + + ag.AssertExpectations(GinkgoT()) + }) +}) diff --git a/core/external/provider_updateartistinfo_test.go b/core/external/provider_updateartistinfo_test.go new file mode 100644 index 000000000..9b1e8d866 --- /dev/null +++ b/core/external/provider_updateartistinfo_test.go @@ -0,0 +1,229 @@ +package external_test + +import ( + "context" + "errors" + "time" + + "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/conf/configtest" + "github.com/navidrome/navidrome/core/agents" + "github.com/navidrome/navidrome/core/external" + "github.com/navidrome/navidrome/log" + "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/tests" + "github.com/navidrome/navidrome/utils/gg" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/stretchr/testify/mock" +) + +func init() { + log.SetLevel(log.LevelDebug) +} + +var _ = Describe("Provider - UpdateArtistInfo", func() { + var ( + ctx context.Context + p external.Provider + ds *tests.MockDataStore + ag *mockAgents + mockArtistRepo *tests.MockArtistRepo + ) + + BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + conf.Server.DevArtistInfoTimeToLive = 1 * time.Hour + ctx = GinkgoT().Context() + ds = new(tests.MockDataStore) + ag = new(mockAgents) + p = external.NewProvider(ds, ag) + mockArtistRepo = ds.Artist(ctx).(*tests.MockArtistRepo) + }) + + It("returns error when artist is not found", func() { + artist, err := p.UpdateArtistInfo(ctx, "ar-not-found", 10, false) + + Expect(err).To(MatchError(model.ErrNotFound)) + Expect(artist).To(BeNil()) + ag.AssertNotCalled(GinkgoT(), "GetArtistMBID") + ag.AssertNotCalled(GinkgoT(), "GetArtistImages") + ag.AssertNotCalled(GinkgoT(), "GetArtistBiography") + ag.AssertNotCalled(GinkgoT(), "GetArtistURL") + ag.AssertNotCalled(GinkgoT(), "GetSimilarArtists") + }) + + It("populates info when artist exists but has no external info", func() { + originalArtist := &model.Artist{ + ID: "ar-existing", + Name: "Test Artist", + } + mockArtistRepo.SetData(model.Artists{*originalArtist}) + + expectedMBID := "mbid-artist-123" + expectedBio := "Artist Bio" + expectedURL := "http://artist.url" + expectedImages := []agents.ExternalImage{ + {URL: "http://large.jpg", Size: 300}, + {URL: "http://medium.jpg", Size: 200}, + {URL: "http://small.jpg", Size: 100}, + } + rawSimilar := []agents.Artist{ + {Name: "Similar Artist 1", MBID: "mbid-similar-1"}, + {Name: "Similar Artist 2", MBID: "mbid-similar-2"}, + {Name: "Similar Artist 3", MBID: "mbid-similar-3"}, + } + similarInDS := model.Artist{ID: "ar-similar-2", Name: "Similar Artist 2"} + + ag.On("GetArtistMBID", ctx, "ar-existing", "Test Artist").Return(expectedMBID, nil).Once() + ag.On("GetArtistImages", ctx, "ar-existing", "Test Artist", expectedMBID).Return(expectedImages, nil).Once() + ag.On("GetArtistBiography", ctx, "ar-existing", "Test Artist", expectedMBID).Return(expectedBio, nil).Once() + ag.On("GetArtistURL", ctx, "ar-existing", "Test Artist", expectedMBID).Return(expectedURL, nil).Once() + ag.On("GetSimilarArtists", ctx, "ar-existing", "Test Artist", expectedMBID, 100).Return(rawSimilar, nil).Once() + + mockArtistRepo.SetData(model.Artists{*originalArtist, similarInDS}) + + updatedArtist, err := p.UpdateArtistInfo(ctx, "ar-existing", 10, false) + + Expect(err).NotTo(HaveOccurred()) + Expect(updatedArtist).NotTo(BeNil()) + Expect(updatedArtist.ID).To(Equal("ar-existing")) + Expect(updatedArtist.MbzArtistID).To(Equal(expectedMBID)) + Expect(updatedArtist.Biography).To(Equal("Artist Bio")) + Expect(updatedArtist.ExternalUrl).To(Equal(expectedURL)) + Expect(updatedArtist.LargeImageUrl).To(Equal("http://large.jpg")) + Expect(updatedArtist.MediumImageUrl).To(Equal("http://medium.jpg")) + Expect(updatedArtist.SmallImageUrl).To(Equal("http://small.jpg")) + Expect(updatedArtist.ExternalInfoUpdatedAt).NotTo(BeNil()) + Expect(*updatedArtist.ExternalInfoUpdatedAt).To(BeTemporally("~", time.Now(), time.Second)) + + Expect(updatedArtist.SimilarArtists).To(HaveLen(1)) + Expect(updatedArtist.SimilarArtists[0].ID).To(Equal("ar-similar-2")) + Expect(updatedArtist.SimilarArtists[0].Name).To(Equal("Similar Artist 2")) + + ag.AssertExpectations(GinkgoT()) + }) + + It("returns cached info when artist exists and info is not expired", func() { + now := time.Now() + originalArtist := &model.Artist{ + ID: "ar-cached", + Name: "Cached Artist", + MbzArtistID: "mbid-cached", + ExternalUrl: "http://cached.url", + Biography: "Cached Bio", + LargeImageUrl: "http://cached_large.jpg", + ExternalInfoUpdatedAt: gg.P(now.Add(-conf.Server.DevArtistInfoTimeToLive / 2)), + SimilarArtists: model.Artists{ + {ID: "ar-similar-present", Name: "Similar Present"}, + {ID: "ar-similar-absent", Name: "Similar Absent"}, + }, + } + similarInDS := model.Artist{ID: "ar-similar-present", Name: "Similar Present Updated"} + mockArtistRepo.SetData(model.Artists{*originalArtist, similarInDS}) + + updatedArtist, err := p.UpdateArtistInfo(ctx, "ar-cached", 5, false) + + Expect(err).NotTo(HaveOccurred()) + Expect(updatedArtist).NotTo(BeNil()) + Expect(updatedArtist.ID).To(Equal(originalArtist.ID)) + Expect(updatedArtist.Name).To(Equal(originalArtist.Name)) + Expect(updatedArtist.MbzArtistID).To(Equal(originalArtist.MbzArtistID)) + Expect(updatedArtist.ExternalUrl).To(Equal(originalArtist.ExternalUrl)) + Expect(updatedArtist.Biography).To(Equal(originalArtist.Biography)) + Expect(updatedArtist.LargeImageUrl).To(Equal(originalArtist.LargeImageUrl)) + Expect(updatedArtist.ExternalInfoUpdatedAt).To(Equal(originalArtist.ExternalInfoUpdatedAt)) + + Expect(updatedArtist.SimilarArtists).To(HaveLen(1)) + Expect(updatedArtist.SimilarArtists[0].ID).To(Equal(similarInDS.ID)) + Expect(updatedArtist.SimilarArtists[0].Name).To(Equal(similarInDS.Name)) + + ag.AssertNotCalled(GinkgoT(), "GetArtistMBID") + ag.AssertNotCalled(GinkgoT(), "GetArtistImages") + ag.AssertNotCalled(GinkgoT(), "GetArtistBiography") + ag.AssertNotCalled(GinkgoT(), "GetArtistURL") + }) + + It("returns cached info and triggers background refresh when info is expired", func() { + now := time.Now() + expiredTime := now.Add(-conf.Server.DevArtistInfoTimeToLive * 2) + originalArtist := &model.Artist{ + ID: "ar-expired", + Name: "Expired Artist", + ExternalInfoUpdatedAt: gg.P(expiredTime), + SimilarArtists: model.Artists{ + {ID: "ar-exp-similar", Name: "Expired Similar"}, + }, + } + similarInDS := model.Artist{ID: "ar-exp-similar", Name: "Expired Similar Updated"} + mockArtistRepo.SetData(model.Artists{*originalArtist, similarInDS}) + + updatedArtist, err := p.UpdateArtistInfo(ctx, "ar-expired", 5, false) + + Expect(err).NotTo(HaveOccurred()) + Expect(updatedArtist).NotTo(BeNil()) + Expect(updatedArtist.ID).To(Equal(originalArtist.ID)) + Expect(updatedArtist.Name).To(Equal(originalArtist.Name)) + Expect(updatedArtist.ExternalInfoUpdatedAt).To(Equal(originalArtist.ExternalInfoUpdatedAt)) + + Expect(updatedArtist.SimilarArtists).To(HaveLen(1)) + Expect(updatedArtist.SimilarArtists[0].ID).To(Equal(similarInDS.ID)) + Expect(updatedArtist.SimilarArtists[0].Name).To(Equal(similarInDS.Name)) + + ag.AssertNotCalled(GinkgoT(), "GetArtistMBID") + ag.AssertNotCalled(GinkgoT(), "GetArtistImages") + ag.AssertNotCalled(GinkgoT(), "GetArtistBiography") + ag.AssertNotCalled(GinkgoT(), "GetArtistURL") + }) + + It("includes non-present similar artists when includeNotPresent is true", func() { + now := time.Now() + originalArtist := &model.Artist{ + ID: "ar-similar-test", + Name: "Similar Test Artist", + ExternalInfoUpdatedAt: gg.P(now.Add(-conf.Server.DevArtistInfoTimeToLive / 2)), + SimilarArtists: model.Artists{ + {ID: "ar-sim-present", Name: "Similar Present"}, + {ID: "", Name: "Similar Absent Raw"}, + {ID: "ar-sim-absent-lookup", Name: "Similar Absent Lookup"}, + }, + } + similarInDS := model.Artist{ID: "ar-sim-present", Name: "Similar Present Updated"} + mockArtistRepo.SetData(model.Artists{*originalArtist, similarInDS}) + + updatedArtist, err := p.UpdateArtistInfo(ctx, "ar-similar-test", 5, true) + + Expect(err).NotTo(HaveOccurred()) + Expect(updatedArtist).NotTo(BeNil()) + + Expect(updatedArtist.SimilarArtists).To(HaveLen(3)) + Expect(updatedArtist.SimilarArtists[0].ID).To(Equal(similarInDS.ID)) + Expect(updatedArtist.SimilarArtists[0].Name).To(Equal(similarInDS.Name)) + Expect(updatedArtist.SimilarArtists[1].ID).To(BeEmpty()) + Expect(updatedArtist.SimilarArtists[1].Name).To(Equal("Similar Absent Raw")) + Expect(updatedArtist.SimilarArtists[2].ID).To(BeEmpty()) + Expect(updatedArtist.SimilarArtists[2].Name).To(Equal("Similar Absent Lookup")) + }) + + It("updates ArtistInfo even if an optional agent call fails", func() { + originalArtist := &model.Artist{ + ID: "ar-agent-fail", + Name: "Agent Fail Artist", + } + mockArtistRepo.SetData(model.Artists{*originalArtist}) + + expectedErr := errors.New("agent MBID failed") + ag.On("GetArtistMBID", ctx, "ar-agent-fail", "Agent Fail Artist").Return("", expectedErr).Once() + ag.On("GetArtistImages", ctx, "ar-agent-fail", "Agent Fail Artist", mock.Anything).Return(nil, nil).Maybe() + ag.On("GetArtistBiography", ctx, "ar-agent-fail", "Agent Fail Artist", mock.Anything).Return("", nil).Maybe() + ag.On("GetArtistURL", ctx, "ar-agent-fail", "Agent Fail Artist", mock.Anything).Return("", nil).Maybe() + ag.On("GetSimilarArtists", ctx, "ar-agent-fail", "Agent Fail Artist", mock.Anything, 100).Return(nil, nil).Maybe() + + updatedArtist, err := p.UpdateArtistInfo(ctx, "ar-agent-fail", 10, false) + + Expect(err).NotTo(HaveOccurred()) + Expect(updatedArtist).NotTo(BeNil()) + Expect(updatedArtist.ID).To(Equal("ar-agent-fail")) + ag.AssertExpectations(GinkgoT()) + }) +}) diff --git a/core/wire_providers.go b/core/wire_providers.go index 6f9d326ec..482cfbefe 100644 --- a/core/wire_providers.go +++ b/core/wire_providers.go @@ -3,6 +3,7 @@ package core import ( "github.com/google/wire" "github.com/navidrome/navidrome/core/agents" + "github.com/navidrome/navidrome/core/external" "github.com/navidrome/navidrome/core/ffmpeg" "github.com/navidrome/navidrome/core/metrics" "github.com/navidrome/navidrome/core/playback" @@ -13,11 +14,12 @@ var Set = wire.NewSet( NewMediaStreamer, GetTranscodingCache, NewArchiver, - NewExternalMetadata, NewPlayers, NewShare, NewPlaylists, agents.GetAgents, + external.NewProvider, + wire.Bind(new(external.Agents), new(*agents.Agents)), ffmpeg.New, scrobbler.GetPlayTracker, playback.GetInstance, diff --git a/server/subsonic/api.go b/server/subsonic/api.go index dfa945086..fd8c3af28 100644 --- a/server/subsonic/api.go +++ b/server/subsonic/api.go @@ -12,6 +12,7 @@ import ( "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/core" "github.com/navidrome/navidrome/core/artwork" + "github.com/navidrome/navidrome/core/external" "github.com/navidrome/navidrome/core/playback" "github.com/navidrome/navidrome/core/scrobbler" "github.com/navidrome/navidrome/log" @@ -30,37 +31,37 @@ type handlerRaw = func(http.ResponseWriter, *http.Request) (*responses.Subsonic, type Router struct { http.Handler - ds model.DataStore - artwork artwork.Artwork - streamer core.MediaStreamer - archiver core.Archiver - players core.Players - externalMetadata core.ExternalMetadata - playlists core.Playlists - scanner scanner.Scanner - broker events.Broker - scrobbler scrobbler.PlayTracker - share core.Share - playback playback.PlaybackServer + ds model.DataStore + artwork artwork.Artwork + streamer core.MediaStreamer + archiver core.Archiver + players core.Players + provider external.Provider + playlists core.Playlists + scanner scanner.Scanner + broker events.Broker + scrobbler scrobbler.PlayTracker + share core.Share + playback playback.PlaybackServer } func New(ds model.DataStore, artwork artwork.Artwork, streamer core.MediaStreamer, archiver core.Archiver, - players core.Players, externalMetadata core.ExternalMetadata, scanner scanner.Scanner, broker events.Broker, + players core.Players, provider external.Provider, scanner scanner.Scanner, broker events.Broker, playlists core.Playlists, scrobbler scrobbler.PlayTracker, share core.Share, playback playback.PlaybackServer, ) *Router { r := &Router{ - ds: ds, - artwork: artwork, - streamer: streamer, - archiver: archiver, - players: players, - externalMetadata: externalMetadata, - playlists: playlists, - scanner: scanner, - broker: broker, - scrobbler: scrobbler, - share: share, - playback: playback, + ds: ds, + artwork: artwork, + streamer: streamer, + archiver: archiver, + players: players, + provider: provider, + playlists: playlists, + scanner: scanner, + broker: broker, + scrobbler: scrobbler, + share: share, + playback: playback, } r.Handler = r.routes() return r diff --git a/server/subsonic/browsing.go b/server/subsonic/browsing.go index edc45a7c7..c00a9f1ab 100644 --- a/server/subsonic/browsing.go +++ b/server/subsonic/browsing.go @@ -210,7 +210,7 @@ func (api *Router) GetAlbumInfo(r *http.Request) (*responses.Subsonic, error) { return nil, err } - album, err := api.externalMetadata.UpdateAlbumInfo(ctx, id) + album, err := api.provider.UpdateAlbumInfo(ctx, id) if err != nil { return nil, err @@ -278,7 +278,7 @@ func (api *Router) getArtistInfo(r *http.Request) (*responses.ArtistInfoBase, *m count := p.IntOr("count", 20) includeNotPresent := p.BoolOr("includeNotPresent", false) - artist, err := api.externalMetadata.UpdateArtistInfo(ctx, id, count, includeNotPresent) + artist, err := api.provider.UpdateArtistInfo(ctx, id, count, includeNotPresent) if err != nil { return nil, nil, err } @@ -343,7 +343,7 @@ func (api *Router) GetSimilarSongs(r *http.Request) (*responses.Subsonic, error) } count := p.IntOr("count", 50) - songs, err := api.externalMetadata.SimilarSongs(ctx, id, count) + songs, err := api.provider.SimilarSongs(ctx, id, count) if err != nil { return nil, err } @@ -377,8 +377,8 @@ func (api *Router) GetTopSongs(r *http.Request) (*responses.Subsonic, error) { } count := p.IntOr("count", 50) - songs, err := api.externalMetadata.TopSongs(ctx, artist, count) - if err != nil { + songs, err := api.provider.TopSongs(ctx, artist, count) + if err != nil && !errors.Is(err, model.ErrNotFound) { return nil, err } diff --git a/tests/mock_album_repo.go b/tests/mock_album_repo.go index a4e0d1289..58c33c97f 100644 --- a/tests/mock_album_repo.go +++ b/tests/mock_album_repo.go @@ -10,56 +10,56 @@ import ( func CreateMockAlbumRepo() *MockAlbumRepo { return &MockAlbumRepo{ - data: make(map[string]*model.Album), + Data: make(map[string]*model.Album), } } type MockAlbumRepo struct { model.AlbumRepository - data map[string]*model.Album - all model.Albums - err bool + Data map[string]*model.Album + All model.Albums + Err bool Options model.QueryOptions } func (m *MockAlbumRepo) SetError(err bool) { - m.err = err + m.Err = err } func (m *MockAlbumRepo) SetData(albums model.Albums) { - m.data = make(map[string]*model.Album, len(albums)) - m.all = albums - for i, a := range m.all { - m.data[a.ID] = &m.all[i] + m.Data = make(map[string]*model.Album, len(albums)) + m.All = albums + for i, a := range m.All { + m.Data[a.ID] = &m.All[i] } } func (m *MockAlbumRepo) Exists(id string) (bool, error) { - if m.err { + if m.Err { return false, errors.New("unexpected error") } - _, found := m.data[id] + _, found := m.Data[id] return found, nil } func (m *MockAlbumRepo) Get(id string) (*model.Album, error) { - if m.err { + if m.Err { return nil, errors.New("unexpected error") } - if d, ok := m.data[id]; ok { + if d, ok := m.Data[id]; ok { return d, nil } return nil, model.ErrNotFound } func (m *MockAlbumRepo) Put(al *model.Album) error { - if m.err { + if m.Err { return errors.New("unexpected error") } if al.ID == "" { al.ID = id.NewRandom() } - m.data[al.ID] = al + m.Data[al.ID] = al return nil } @@ -67,17 +67,17 @@ func (m *MockAlbumRepo) GetAll(qo ...model.QueryOptions) (model.Albums, error) { if len(qo) > 0 { m.Options = qo[0] } - if m.err { + if m.Err { return nil, errors.New("unexpected error") } - return m.all, nil + return m.All, nil } func (m *MockAlbumRepo) IncPlayCount(id string, timestamp time.Time) error { - if m.err { + if m.Err { return errors.New("unexpected error") } - if d, ok := m.data[id]; ok { + if d, ok := m.Data[id]; ok { d.PlayCount++ d.PlayDate = ×tamp return nil @@ -85,15 +85,15 @@ func (m *MockAlbumRepo) IncPlayCount(id string, timestamp time.Time) error { return model.ErrNotFound } func (m *MockAlbumRepo) CountAll(...model.QueryOptions) (int64, error) { - return int64(len(m.all)), nil + return int64(len(m.All)), nil } func (m *MockAlbumRepo) GetTouchedAlbums(libID int) (model.AlbumCursor, error) { - if m.err { + if m.Err { return nil, errors.New("unexpected error") } return func(yield func(model.Album, error) bool) { - for _, a := range m.data { + for _, a := range m.Data { if a.ID == "error" { if !yield(*a, errors.New("error")) { break @@ -110,4 +110,11 @@ func (m *MockAlbumRepo) GetTouchedAlbums(libID int) (model.AlbumCursor, error) { }, nil } +func (m *MockAlbumRepo) UpdateExternalInfo(album *model.Album) error { + if m.Err { + return errors.New("unexpected error") + } + return nil +} + var _ model.AlbumRepository = (*MockAlbumRepo)(nil) diff --git a/tests/mock_artist_repo.go b/tests/mock_artist_repo.go index fad7c78d3..7058cead0 100644 --- a/tests/mock_artist_repo.go +++ b/tests/mock_artist_repo.go @@ -10,61 +10,61 @@ import ( func CreateMockArtistRepo() *MockArtistRepo { return &MockArtistRepo{ - data: make(map[string]*model.Artist), + Data: make(map[string]*model.Artist), } } type MockArtistRepo struct { model.ArtistRepository - data map[string]*model.Artist - err bool + Data map[string]*model.Artist + Err bool } func (m *MockArtistRepo) SetError(err bool) { - m.err = err + m.Err = err } func (m *MockArtistRepo) SetData(artists model.Artists) { - m.data = make(map[string]*model.Artist) + m.Data = make(map[string]*model.Artist) for i, a := range artists { - m.data[a.ID] = &artists[i] + m.Data[a.ID] = &artists[i] } } func (m *MockArtistRepo) Exists(id string) (bool, error) { - if m.err { + if m.Err { return false, errors.New("Error!") } - _, found := m.data[id] + _, found := m.Data[id] return found, nil } func (m *MockArtistRepo) Get(id string) (*model.Artist, error) { - if m.err { + if m.Err { return nil, errors.New("Error!") } - if d, ok := m.data[id]; ok { + if d, ok := m.Data[id]; ok { return d, nil } return nil, model.ErrNotFound } func (m *MockArtistRepo) Put(ar *model.Artist, columsToUpdate ...string) error { - if m.err { + if m.Err { return errors.New("error") } if ar.ID == "" { ar.ID = id.NewRandom() } - m.data[ar.ID] = ar + m.Data[ar.ID] = ar return nil } func (m *MockArtistRepo) IncPlayCount(id string, timestamp time.Time) error { - if m.err { + if m.Err { return errors.New("error") } - if d, ok := m.data[id]; ok { + if d, ok := m.Data[id]; ok { d.PlayCount++ d.PlayDate = ×tamp return nil @@ -72,4 +72,26 @@ func (m *MockArtistRepo) IncPlayCount(id string, timestamp time.Time) error { return model.ErrNotFound } +func (m *MockArtistRepo) GetAll(options ...model.QueryOptions) (model.Artists, error) { + if m.Err { + return nil, errors.New("mock repo error") + } + var allArtists model.Artists + for _, artist := range m.Data { + allArtists = append(allArtists, *artist) + } + // Apply Max=1 if present (simple simulation for findArtistByName) + if len(options) > 0 && options[0].Max == 1 && len(allArtists) > 0 { + return allArtists[:1], nil + } + return allArtists, nil +} + +func (m *MockArtistRepo) UpdateExternalInfo(artist *model.Artist) error { + if m.Err { + return errors.New("mock repo error") + } + return nil +} + var _ model.ArtistRepository = (*MockArtistRepo)(nil) diff --git a/tests/mock_genre_repo.go b/tests/mock_genre_repo.go index a24f2b0c8..122ccc278 100644 --- a/tests/mock_genre_repo.go +++ b/tests/mock_genre_repo.go @@ -6,12 +6,12 @@ import ( type MockedGenreRepo struct { Error error - data map[string]model.Genre + Data map[string]model.Genre } func (r *MockedGenreRepo) init() { - if r.data == nil { - r.data = make(map[string]model.Genre) + if r.Data == nil { + r.Data = make(map[string]model.Genre) } } @@ -22,7 +22,7 @@ func (r *MockedGenreRepo) GetAll(...model.QueryOptions) (model.Genres, error) { r.init() var all model.Genres - for _, g := range r.data { + for _, g := range r.Data { all = append(all, g) } return all, nil @@ -33,6 +33,6 @@ func (r *MockedGenreRepo) Put(g *model.Genre) error { return r.Error } r.init() - r.data[g.ID] = *g + r.Data[g.ID] = *g return nil } diff --git a/tests/mock_library_repo.go b/tests/mock_library_repo.go index 264dbe24c..907a9d487 100644 --- a/tests/mock_library_repo.go +++ b/tests/mock_library_repo.go @@ -7,14 +7,14 @@ import ( type MockLibraryRepo struct { model.LibraryRepository - data map[int]model.Library + Data map[int]model.Library Err error } func (m *MockLibraryRepo) SetData(data model.Libraries) { - m.data = make(map[int]model.Library) + m.Data = make(map[int]model.Library) for _, d := range data { - m.data[d.ID] = d + m.Data[d.ID] = d } } @@ -22,14 +22,14 @@ func (m *MockLibraryRepo) GetAll(...model.QueryOptions) (model.Libraries, error) if m.Err != nil { return nil, m.Err } - return maps.Values(m.data), nil + return maps.Values(m.Data), nil } func (m *MockLibraryRepo) GetPath(id int) (string, error) { if m.Err != nil { return "", m.Err } - if lib, ok := m.data[id]; ok { + if lib, ok := m.Data[id]; ok { return lib.Path, nil } return "", model.ErrNotFound diff --git a/tests/mock_mediafile_repo.go b/tests/mock_mediafile_repo.go index 4978e88bb..01d82e03b 100644 --- a/tests/mock_mediafile_repo.go +++ b/tests/mock_mediafile_repo.go @@ -14,40 +14,40 @@ import ( func CreateMockMediaFileRepo() *MockMediaFileRepo { return &MockMediaFileRepo{ - data: make(map[string]*model.MediaFile), + Data: make(map[string]*model.MediaFile), } } type MockMediaFileRepo struct { model.MediaFileRepository - data map[string]*model.MediaFile - err bool + Data map[string]*model.MediaFile + Err bool } func (m *MockMediaFileRepo) SetError(err bool) { - m.err = err + m.Err = err } func (m *MockMediaFileRepo) SetData(mfs model.MediaFiles) { - m.data = make(map[string]*model.MediaFile) + m.Data = make(map[string]*model.MediaFile) for i, mf := range mfs { - m.data[mf.ID] = &mfs[i] + m.Data[mf.ID] = &mfs[i] } } func (m *MockMediaFileRepo) Exists(id string) (bool, error) { - if m.err { + if m.Err { return false, errors.New("error") } - _, found := m.data[id] + _, found := m.Data[id] return found, nil } func (m *MockMediaFileRepo) Get(id string) (*model.MediaFile, error) { - if m.err { + if m.Err { return nil, errors.New("error") } - if d, ok := m.data[id]; ok { + if d, ok := m.Data[id]; ok { // Intentionally clone the file and remove participants. This should // catch any caller that actually means to call GetWithParticipants res := *d @@ -58,52 +58,52 @@ func (m *MockMediaFileRepo) Get(id string) (*model.MediaFile, error) { } func (m *MockMediaFileRepo) GetWithParticipants(id string) (*model.MediaFile, error) { - if m.err { + if m.Err { return nil, errors.New("error") } - if d, ok := m.data[id]; ok { + if d, ok := m.Data[id]; ok { return d, nil } return nil, model.ErrNotFound } func (m *MockMediaFileRepo) GetAll(...model.QueryOptions) (model.MediaFiles, error) { - if m.err { + if m.Err { return nil, errors.New("error") } - values := slices.Collect(maps.Values(m.data)) + values := slices.Collect(maps.Values(m.Data)) return slice.Map(values, func(p *model.MediaFile) model.MediaFile { return *p }), nil } func (m *MockMediaFileRepo) Put(mf *model.MediaFile) error { - if m.err { + if m.Err { return errors.New("error") } if mf.ID == "" { mf.ID = id.NewRandom() } - m.data[mf.ID] = mf + m.Data[mf.ID] = mf return nil } func (m *MockMediaFileRepo) Delete(id string) error { - if m.err { + if m.Err { return errors.New("error") } - if _, ok := m.data[id]; !ok { + if _, ok := m.Data[id]; !ok { return model.ErrNotFound } - delete(m.data, id) + delete(m.Data, id) return nil } func (m *MockMediaFileRepo) IncPlayCount(id string, timestamp time.Time) error { - if m.err { + if m.Err { return errors.New("error") } - if d, ok := m.data[id]; ok { + if d, ok := m.Data[id]; ok { d.PlayCount++ d.PlayDate = ×tamp return nil @@ -112,12 +112,12 @@ func (m *MockMediaFileRepo) IncPlayCount(id string, timestamp time.Time) error { } func (m *MockMediaFileRepo) FindByAlbum(artistId string) (model.MediaFiles, error) { - if m.err { + if m.Err { return nil, errors.New("error") } - var res = make(model.MediaFiles, len(m.data)) + var res = make(model.MediaFiles, len(m.Data)) i := 0 - for _, a := range m.data { + for _, a := range m.Data { if a.AlbumID == artistId { res[i] = *a i++ @@ -128,17 +128,17 @@ func (m *MockMediaFileRepo) FindByAlbum(artistId string) (model.MediaFiles, erro } func (m *MockMediaFileRepo) GetMissingAndMatching(libId int) (model.MediaFileCursor, error) { - if m.err { + if m.Err { return nil, errors.New("error") } var res model.MediaFiles - for _, a := range m.data { + for _, a := range m.Data { if a.LibraryID == libId && a.Missing { res = append(res, *a) } } - for _, a := range m.data { + for _, a := range m.Data { if a.LibraryID == libId && !(*a).Missing && slices.IndexFunc(res, func(mediaFile model.MediaFile) bool { return mediaFile.PID == a.PID }) != -1 { diff --git a/tests/mock_property_repo.go b/tests/mock_property_repo.go index 39dec17b3..9adc66e6d 100644 --- a/tests/mock_property_repo.go +++ b/tests/mock_property_repo.go @@ -5,12 +5,12 @@ import "github.com/navidrome/navidrome/model" type MockedPropertyRepo struct { model.PropertyRepository Error error - data map[string]string + Data map[string]string } func (p *MockedPropertyRepo) init() { - if p.data == nil { - p.data = make(map[string]string) + if p.Data == nil { + p.Data = make(map[string]string) } } @@ -19,7 +19,7 @@ func (p *MockedPropertyRepo) Put(id string, value string) error { return p.Error } p.init() - p.data[id] = value + p.Data[id] = value return nil } @@ -28,7 +28,7 @@ func (p *MockedPropertyRepo) Get(id string) (string, error) { return "", p.Error } p.init() - if v, ok := p.data[id]; ok { + if v, ok := p.Data[id]; ok { return v, nil } return "", model.ErrNotFound @@ -39,8 +39,8 @@ func (p *MockedPropertyRepo) Delete(id string) error { return p.Error } p.init() - if _, ok := p.data[id]; ok { - delete(p.data, id) + if _, ok := p.Data[id]; ok { + delete(p.Data, id) return nil } return model.ErrNotFound diff --git a/tests/mock_radio_repository.go b/tests/mock_radio_repository.go index a1a584320..279b735db 100644 --- a/tests/mock_radio_repository.go +++ b/tests/mock_radio_repository.go @@ -9,9 +9,9 @@ import ( type MockedRadioRepo struct { model.RadioRepository - data map[string]*model.Radio - all model.Radios - err bool + Data map[string]*model.Radio + All model.Radios + Err bool Options model.QueryOptions } @@ -20,44 +20,44 @@ func CreateMockedRadioRepo() *MockedRadioRepo { } func (m *MockedRadioRepo) SetError(err bool) { - m.err = err + m.Err = err } func (m *MockedRadioRepo) CountAll(options ...model.QueryOptions) (int64, error) { - if m.err { + if m.Err { return 0, errors.New("error") } - return int64(len(m.data)), nil + return int64(len(m.Data)), nil } func (m *MockedRadioRepo) Delete(id string) error { - if m.err { + if m.Err { return errors.New("Error!") } - _, found := m.data[id] + _, found := m.Data[id] if !found { return errors.New("not found") } - delete(m.data, id) + delete(m.Data, id) return nil } func (m *MockedRadioRepo) Exists(id string) (bool, error) { - if m.err { + if m.Err { return false, errors.New("Error!") } - _, found := m.data[id] + _, found := m.Data[id] return found, nil } func (m *MockedRadioRepo) Get(id string) (*model.Radio, error) { - if m.err { + if m.Err { return nil, errors.New("Error!") } - if d, ok := m.data[id]; ok { + if d, ok := m.Data[id]; ok { return d, nil } return nil, model.ErrNotFound @@ -67,19 +67,19 @@ func (m *MockedRadioRepo) GetAll(qo ...model.QueryOptions) (model.Radios, error) if len(qo) > 0 { m.Options = qo[0] } - if m.err { + if m.Err { return nil, errors.New("Error!") } - return m.all, nil + return m.All, nil } func (m *MockedRadioRepo) Put(radio *model.Radio) error { - if m.err { + if m.Err { return errors.New("error") } if radio.ID == "" { radio.ID = id.NewRandom() } - m.data[radio.ID] = radio + m.Data[radio.ID] = radio return nil } diff --git a/tests/mock_scrobble_buffer_repo.go b/tests/mock_scrobble_buffer_repo.go index 06b28af75..407c673eb 100644 --- a/tests/mock_scrobble_buffer_repo.go +++ b/tests/mock_scrobble_buffer_repo.go @@ -8,7 +8,7 @@ import ( type MockedScrobbleBufferRepo struct { Error error - data model.ScrobbleEntries + Data model.ScrobbleEntries } func CreateMockedScrobbleBufferRepo() *MockedScrobbleBufferRepo { @@ -20,7 +20,7 @@ func (m *MockedScrobbleBufferRepo) UserIDs(service string) ([]string, error) { return nil, m.Error } userIds := make(map[string]struct{}) - for _, e := range m.data { + for _, e := range m.Data { if e.Service == service { userIds[e.UserID] = struct{}{} } @@ -36,7 +36,7 @@ func (m *MockedScrobbleBufferRepo) Enqueue(service, userId, mediaFileId string, if m.Error != nil { return m.Error } - m.data = append(m.data, model.ScrobbleEntry{ + m.Data = append(m.Data, model.ScrobbleEntry{ MediaFile: model.MediaFile{ID: mediaFileId}, Service: service, UserID: userId, @@ -50,7 +50,7 @@ func (m *MockedScrobbleBufferRepo) Next(service, userId string) (*model.Scrobble if m.Error != nil { return nil, m.Error } - for _, e := range m.data { + for _, e := range m.Data { if e.Service == service && e.UserID == userId { return &e, nil } @@ -63,13 +63,13 @@ func (m *MockedScrobbleBufferRepo) Dequeue(entry *model.ScrobbleEntry) error { return m.Error } newData := model.ScrobbleEntries{} - for _, e := range m.data { + for _, e := range m.Data { if e.Service == entry.Service && e.UserID == entry.UserID && e.PlayTime == entry.PlayTime && e.MediaFile.ID == entry.MediaFile.ID { continue } newData = append(newData, e) } - m.data = newData + m.Data = newData return nil } @@ -77,5 +77,5 @@ func (m *MockedScrobbleBufferRepo) Length() (int64, error) { if m.Error != nil { return 0, m.Error } - return int64(len(m.data)), nil + return int64(len(m.Data)), nil } diff --git a/tests/mock_user_props_repo.go b/tests/mock_user_props_repo.go index b1880c999..1b1e17650 100644 --- a/tests/mock_user_props_repo.go +++ b/tests/mock_user_props_repo.go @@ -5,12 +5,12 @@ import "github.com/navidrome/navidrome/model" type MockedUserPropsRepo struct { model.UserPropsRepository Error error - data map[string]string + Data map[string]string } func (p *MockedUserPropsRepo) init() { - if p.data == nil { - p.data = make(map[string]string) + if p.Data == nil { + p.Data = make(map[string]string) } } @@ -19,7 +19,7 @@ func (p *MockedUserPropsRepo) Put(userId, key string, value string) error { return p.Error } p.init() - p.data[userId+key] = value + p.Data[userId+key] = value return nil } @@ -28,7 +28,7 @@ func (p *MockedUserPropsRepo) Get(userId, key string) (string, error) { return "", p.Error } p.init() - if v, ok := p.data[userId+key]; ok { + if v, ok := p.Data[userId+key]; ok { return v, nil } return "", model.ErrNotFound @@ -39,8 +39,8 @@ func (p *MockedUserPropsRepo) Delete(userId, key string) error { return p.Error } p.init() - if _, ok := p.data[userId+key]; ok { - delete(p.data, userId+key) + if _, ok := p.Data[userId+key]; ok { + delete(p.Data, userId+key) return nil } return model.ErrNotFound