diff --git a/core/extdata/extdata_helper_test.go b/core/extdata/provider_helper_test.go similarity index 100% rename from core/extdata/extdata_helper_test.go rename to core/extdata/provider_helper_test.go diff --git a/core/extdata/provider_updatealbuminfo_test.go b/core/extdata/provider_updatealbuminfo_test.go new file mode 100644 index 000000000..3270380bb --- /dev/null +++ b/core/extdata/provider_updatealbuminfo_test.go @@ -0,0 +1,225 @@ +package extdata_test + +import ( + "context" + "errors" + "time" + + "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/core/agents" + "github.com/navidrome/navidrome/core/extdata" + "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) +} + +// MockAgents implements the extdata.Agents interface for testing +type MockAgents struct { + mock.Mock + agents.AlbumInfoRetriever + agents.ArtistBiographyRetriever + agents.ArtistMBIDRetriever + agents.ArtistImageRetriever + agents.ArtistSimilarRetriever + agents.ArtistTopSongsRetriever + agents.ArtistURLRetriever +} + +func (m *MockAgents) GetAlbumInfo(ctx context.Context, albumName, artistName, mbid string) (*agents.AlbumInfo, error) { + args := m.Called(ctx, albumName, artistName, mbid) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).(*agents.AlbumInfo), args.Error(1) +} + +var _ = Describe("Provider UpdateAlbumInfo", func() { + var ( + ctx context.Context + p extdata.Provider + ds *tests.MockDataStore + ag *MockAgents + ) + + BeforeEach(func() { + ctx = context.Background() + ds = new(tests.MockDataStore) + ag = new(MockAgents) + p = extdata.NewProvider(ds, ag) + + // Default config + conf.Server.DevAlbumInfoTimeToLive = 1 * time.Hour + }) + + It("returns error when album is not found", func() { + // MockDataStore will return a MockAlbumRepo with an empty Data map by default + _ = ds.Album(ctx).(*tests.MockAlbumRepo) // Ensure MockAlbumRepo is initialized + + album, err := p.UpdateAlbumInfo(ctx, "al-not-found") + + Expect(err).To(MatchError(model.ErrNotFound)) + Expect(album).To(BeNil()) + // No AssertExpectations needed for custom mock + ag.AssertNotCalled(GinkgoT(), "GetAlbumInfo", mock.Anything, mock.Anything, mock.Anything, mock.Anything) + }) + + It("populates info when album exists but has no external info", func() { + // Setup: Album exists in DS, ExternalInfoUpdatedAt is nil + originalAlbum := &model.Album{ + ID: "al-existing", + Name: "Test Album", + AlbumArtist: "Test Artist", + MbzAlbumID: "mbid-album", + } + mockAlbumRepo := ds.Album(ctx).(*tests.MockAlbumRepo) + mockAlbumRepo.SetData(model.Albums{*originalAlbum}) + + // Mock agent response + 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) + + // Call the method under test + updatedAlbum, err := p.UpdateAlbumInfo(ctx, "al-existing") + + // Assertions + 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)) // Check timestamp was set + + ag.AssertExpectations(GinkgoT()) + }) + + It("returns cached info when album exists and info is not expired", func() { + // Setup: Album exists in DS, ExternalInfoUpdatedAt is recent + 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)), // Not expired + } + mockAlbumRepo := ds.Album(ctx).(*tests.MockAlbumRepo) + mockAlbumRepo.SetData(model.Albums{*originalAlbum}) + + // Call the method under test + updatedAlbum, err := p.UpdateAlbumInfo(ctx, "al-cached") + + // Assertions + Expect(err).NotTo(HaveOccurred()) + Expect(updatedAlbum).NotTo(BeNil()) + Expect(*updatedAlbum).To(Equal(*originalAlbum)) // Should return the exact cached data + + 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() { + // Setup: Album exists in DS, ExternalInfoUpdatedAt is old + now := time.Now() + expiredTime := now.Add(-conf.Server.DevAlbumInfoTimeToLive * 2) // Definitely expired + 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 := ds.Album(ctx).(*tests.MockAlbumRepo) + mockAlbumRepo.SetData(model.Albums{*originalAlbum}) + + // Call the method under test + // Note: We are not testing the background refresh directly here, only the immediate return + updatedAlbum, err := p.UpdateAlbumInfo(ctx, "al-expired") + + // Assertions: Should return the stale data immediately + Expect(err).NotTo(HaveOccurred()) + Expect(updatedAlbum).NotTo(BeNil()) + Expect(*updatedAlbum).To(Equal(*originalAlbum)) // Should return the exact cached (stale) data + + // Assertions: Agent should NOT be called synchronously + ag.AssertNotCalled(GinkgoT(), "GetAlbumInfo", mock.Anything, mock.Anything, mock.Anything, mock.Anything) + }) + + It("returns error when agent fails to get album info", func() { + // Setup: Album exists in DS, ExternalInfoUpdatedAt is nil + originalAlbum := &model.Album{ + ID: "al-agent-error", + Name: "Agent Error Album", + AlbumArtist: "Agent Error Artist", + MbzAlbumID: "mbid-agent-error", + } + mockAlbumRepo := ds.Album(ctx).(*tests.MockAlbumRepo) + mockAlbumRepo.SetData(model.Albums{*originalAlbum}) + + // Mock agent response with an error + expectedErr := errors.New("agent communication failed") + ag.On("GetAlbumInfo", ctx, "Agent Error Album", "Agent Error Artist", "mbid-agent-error").Return(nil, expectedErr) + + // Call the method under test + updatedAlbum, err := p.UpdateAlbumInfo(ctx, "al-agent-error") + + // Assertions + Expect(err).To(MatchError(expectedErr)) + Expect(updatedAlbum).To(BeNil()) + ag.AssertExpectations(GinkgoT()) + }) + + It("returns original album when agent returns ErrNotFound", func() { + // Setup: Album exists in DS, ExternalInfoUpdatedAt is nil + originalAlbum := &model.Album{ + ID: "al-agent-notfound", + Name: "Agent NotFound Album", + AlbumArtist: "Agent NotFound Artist", + MbzAlbumID: "mbid-agent-notfound", + // ExternalInfoUpdatedAt is nil + } + mockAlbumRepo := ds.Album(ctx).(*tests.MockAlbumRepo) + mockAlbumRepo.SetData(model.Albums{*originalAlbum}) + + // Mock agent response with ErrNotFound + ag.On("GetAlbumInfo", ctx, "Agent NotFound Album", "Agent NotFound Artist", "mbid-agent-notfound").Return(nil, agents.ErrNotFound) + + // Call the method under test + updatedAlbum, err := p.UpdateAlbumInfo(ctx, "al-agent-notfound") + + // Assertions + Expect(err).NotTo(HaveOccurred()) // No error should be returned + Expect(updatedAlbum).NotTo(BeNil()) + Expect(*updatedAlbum).To(Equal(*originalAlbum)) // Should return original data + Expect(updatedAlbum.ExternalInfoUpdatedAt).To(BeNil()) // Timestamp should not be set + + // Agent was called, but UpdateExternalInfo should not have been + ag.AssertExpectations(GinkgoT()) + // We can't assert mockAlbumRepo.AssertNotCalled for UpdateExternalInfo because it's not a testify mock method + }) + + // Test cases will go here + +}) diff --git a/tests/mock_album_repo.go b/tests/mock_album_repo.go index 555d91ac9..ab632b4d4 100644 --- a/tests/mock_album_repo.go +++ b/tests/mock_album_repo.go @@ -110,4 +110,13 @@ 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") + } + // Simple implementation to prevent nil pointer dereference in tests. + // We could optionally update m.Data here if needed for specific tests. + return nil +} + var _ model.AlbumRepository = (*MockAlbumRepo)(nil)