mirror of
https://github.com/navidrome/navidrome.git
synced 2025-04-23 07:10:31 +03:00
clean up
Signed-off-by: Deluan <deluan@navidrome.org>
This commit is contained in:
parent
28becd5c64
commit
99a04e65d5
@ -186,15 +186,15 @@ func (m *mockSimilarArtistAgent) GetSimilarArtists(ctx context.Context, id, name
|
||||
|
||||
// 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 // Added field for clarity
|
||||
mbidAgent agents.ArtistMBIDRetriever // Added field for clarity
|
||||
urlAgent agents.ArtistURLRetriever // Added field for clarity
|
||||
agents.Interface // Embed to satisfy non-overridden methods
|
||||
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 {
|
||||
@ -205,7 +205,6 @@ func (m *mockAgents) GetSimilarArtists(ctx context.Context, id, name, mbid strin
|
||||
if m.similarAgent != nil {
|
||||
return m.similarAgent.GetSimilarArtists(ctx, id, name, mbid, limit)
|
||||
}
|
||||
// Fallback to testify mock
|
||||
args := m.Called(ctx, id, name, mbid, limit)
|
||||
if args.Get(0) != nil {
|
||||
return args.Get(0).([]agents.Artist), args.Error(1)
|
||||
@ -217,7 +216,6 @@ func (m *mockAgents) GetArtistTopSongs(ctx context.Context, id, artistName, mbid
|
||||
if m.topSongsAgent != nil {
|
||||
return m.topSongsAgent.GetArtistTopSongs(ctx, id, artistName, mbid, count)
|
||||
}
|
||||
// Fallback to testify mock
|
||||
args := m.Called(ctx, id, artistName, mbid, count)
|
||||
if args.Get(0) != nil {
|
||||
return args.Get(0).([]agents.Song), args.Error(1)
|
||||
@ -225,13 +223,10 @@ func (m *mockAgents) GetArtistTopSongs(ctx context.Context, id, artistName, mbid
|
||||
return nil, args.Error(1)
|
||||
}
|
||||
|
||||
// --- Stubs for other Agents interface methods ---
|
||||
|
||||
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)
|
||||
}
|
||||
// Fallback to testify mock
|
||||
args := m.Called(ctx, name, artist, mbid)
|
||||
if args.Get(0) != nil {
|
||||
return args.Get(0).(*agents.AlbumInfo), args.Error(1)
|
||||
@ -243,7 +238,6 @@ func (m *mockAgents) GetArtistMBID(ctx context.Context, id string, name string)
|
||||
if m.mbidAgent != nil {
|
||||
return m.mbidAgent.GetArtistMBID(ctx, id, name)
|
||||
}
|
||||
// Fallback to testify mock
|
||||
args := m.Called(ctx, id, name)
|
||||
return args.String(0), args.Error(1)
|
||||
}
|
||||
@ -252,7 +246,6 @@ func (m *mockAgents) GetArtistURL(ctx context.Context, id, name, mbid string) (s
|
||||
if m.urlAgent != nil {
|
||||
return m.urlAgent.GetArtistURL(ctx, id, name, mbid)
|
||||
}
|
||||
// Fallback to testify mock
|
||||
args := m.Called(ctx, id, name, mbid)
|
||||
return args.String(0), args.Error(1)
|
||||
}
|
||||
@ -261,7 +254,6 @@ func (m *mockAgents) GetArtistBiography(ctx context.Context, id, name, mbid stri
|
||||
if m.bioAgent != nil {
|
||||
return m.bioAgent.GetArtistBiography(ctx, id, name, mbid)
|
||||
}
|
||||
// Fallback to testify mock
|
||||
args := m.Called(ctx, id, name, mbid)
|
||||
return args.String(0), args.Error(1)
|
||||
}
|
||||
@ -270,7 +262,6 @@ func (m *mockAgents) GetArtistImages(ctx context.Context, id, name, mbid string)
|
||||
if m.imageAgent != nil {
|
||||
return m.imageAgent.GetArtistImages(ctx, id, name, mbid)
|
||||
}
|
||||
// Fallback to testify mock
|
||||
args := m.Called(ctx, id, name, mbid)
|
||||
if args.Get(0) != nil {
|
||||
return args.Get(0).([]agents.ExternalImage), args.Error(1)
|
||||
|
@ -23,10 +23,11 @@ func init() {
|
||||
|
||||
var _ = Describe("Provider UpdateAlbumInfo", func() {
|
||||
var (
|
||||
ctx context.Context
|
||||
p extdata.Provider
|
||||
ds *tests.MockDataStore
|
||||
ag *extdata.MockAgents
|
||||
ctx context.Context
|
||||
p extdata.Provider
|
||||
ds *tests.MockDataStore
|
||||
ag *extdata.MockAgents
|
||||
mockAlbumRepo *tests.MockAlbumRepo
|
||||
)
|
||||
|
||||
BeforeEach(func() {
|
||||
@ -34,35 +35,27 @@ var _ = Describe("Provider UpdateAlbumInfo", func() {
|
||||
ds = new(tests.MockDataStore)
|
||||
ag = new(extdata.MockAgents)
|
||||
p = extdata.NewProvider(ds, ag)
|
||||
|
||||
// Default config
|
||||
mockAlbumRepo = ds.Album(ctx).(*tests.MockAlbumRepo)
|
||||
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",
|
||||
@ -74,10 +67,8 @@ var _ = Describe("Provider UpdateAlbumInfo", func() {
|
||||
}
|
||||
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"))
|
||||
@ -87,13 +78,12 @@ var _ = Describe("Provider UpdateAlbumInfo", func() {
|
||||
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
|
||||
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() {
|
||||
// Setup: Album exists in DS, ExternalInfoUpdatedAt is recent
|
||||
now := time.Now()
|
||||
originalAlbum := &model.Album{
|
||||
ID: "al-cached",
|
||||
@ -102,26 +92,22 @@ var _ = Describe("Provider UpdateAlbumInfo", func() {
|
||||
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
|
||||
ExternalInfoUpdatedAt: gg.P(now.Add(-conf.Server.DevAlbumInfoTimeToLive / 2)),
|
||||
}
|
||||
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
|
||||
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() {
|
||||
// Setup: Album exists in DS, ExternalInfoUpdatedAt is old
|
||||
now := time.Now()
|
||||
expiredTime := now.Add(-conf.Server.DevAlbumInfoTimeToLive * 2) // Definitely expired
|
||||
expiredTime := now.Add(-conf.Server.DevAlbumInfoTimeToLive * 2)
|
||||
originalAlbum := &model.Album{
|
||||
ID: "al-expired",
|
||||
Name: "Expired Album",
|
||||
@ -131,75 +117,54 @@ var _ = Describe("Provider UpdateAlbumInfo", func() {
|
||||
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
|
||||
Expect(*updatedAlbum).To(Equal(*originalAlbum))
|
||||
|
||||
// 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(err).NotTo(HaveOccurred())
|
||||
Expect(updatedAlbum).NotTo(BeNil())
|
||||
Expect(*updatedAlbum).To(Equal(*originalAlbum)) // Should return original data
|
||||
Expect(updatedAlbum.ExternalInfoUpdatedAt).To(BeNil()) // Timestamp should not be set
|
||||
Expect(*updatedAlbum).To(Equal(*originalAlbum))
|
||||
Expect(updatedAlbum.ExternalInfoUpdatedAt).To(BeNil())
|
||||
|
||||
// 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
|
||||
|
||||
})
|
||||
|
@ -27,7 +27,7 @@ var _ = Describe("Provider UpdateArtistInfo", func() {
|
||||
p extdata.Provider
|
||||
ds *tests.MockDataStore
|
||||
ag *extdata.MockAgents
|
||||
mockArtistRepo *tests.MockArtistRepo // Convenience variable
|
||||
mockArtistRepo *tests.MockArtistRepo
|
||||
)
|
||||
|
||||
BeforeEach(func() {
|
||||
@ -35,16 +35,11 @@ var _ = Describe("Provider UpdateArtistInfo", func() {
|
||||
ds = new(tests.MockDataStore)
|
||||
ag = new(extdata.MockAgents)
|
||||
p = extdata.NewProvider(ds, ag)
|
||||
|
||||
mockArtistRepo = ds.Artist(ctx).(*tests.MockArtistRepo)
|
||||
|
||||
// Default config
|
||||
conf.Server.DevArtistInfoTimeToLive = 1 * time.Hour
|
||||
})
|
||||
|
||||
It("returns error when artist is not found", func() {
|
||||
// MockArtistRepo.Get returns ErrNotFound by default if data is empty
|
||||
|
||||
artist, err := p.UpdateArtistInfo(ctx, "ar-not-found", 10, false)
|
||||
|
||||
Expect(err).To(MatchError(model.ErrNotFound))
|
||||
@ -57,18 +52,14 @@ var _ = Describe("Provider UpdateArtistInfo", func() {
|
||||
})
|
||||
|
||||
It("populates info when artist exists but has no external info", func() {
|
||||
// Setup: Artist exists in DS, ExternalInfoUpdatedAt is nil
|
||||
originalArtist := &model.Artist{
|
||||
ID: "ar-existing",
|
||||
Name: "Test Artist",
|
||||
// MbzArtistID is empty initially
|
||||
// ExternalInfoUpdatedAt is nil
|
||||
}
|
||||
mockArtistRepo.SetData(model.Artists{*originalArtist})
|
||||
|
||||
// Mock agent responses
|
||||
expectedMBID := "mbid-artist-123"
|
||||
expectedBio := "Artist Bio" // Raw bio from agent
|
||||
expectedBio := "Artist Bio"
|
||||
expectedURL := "http://artist.url"
|
||||
expectedImages := []agents.ExternalImage{
|
||||
{URL: "http://large.jpg", Size: 300},
|
||||
@ -77,8 +68,8 @@ var _ = Describe("Provider UpdateArtistInfo", func() {
|
||||
}
|
||||
rawSimilar := []agents.Artist{
|
||||
{Name: "Similar Artist 1", MBID: "mbid-similar-1"},
|
||||
{Name: "Similar Artist 2", MBID: "mbid-similar-2"}, // This one exists in DS
|
||||
{Name: "Similar Artist 3", MBID: "mbid-similar-3"}, // This one doesn't exist
|
||||
{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"}
|
||||
|
||||
@ -88,18 +79,14 @@ var _ = Describe("Provider UpdateArtistInfo", func() {
|
||||
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()
|
||||
|
||||
// Mock loading similar artists from DS (only finds ar-similar-2)
|
||||
mockArtistRepo.SetData(model.Artists{*originalArtist, similarInDS}) // Update repo data to include similar
|
||||
mockArtistRepo.SetData(model.Artists{*originalArtist, similarInDS})
|
||||
|
||||
// Call the method under test (includeNotPresent = false for this part)
|
||||
updatedArtist, err := p.UpdateArtistInfo(ctx, "ar-existing", 10, false) // 10 similar requested, includeNotPresent=false
|
||||
updatedArtist, err := p.UpdateArtistInfo(ctx, "ar-existing", 10, false)
|
||||
|
||||
// Assertions
|
||||
Expect(err).NotTo(HaveOccurred())
|
||||
Expect(updatedArtist).NotTo(BeNil())
|
||||
Expect(updatedArtist.ID).To(Equal("ar-existing"))
|
||||
Expect(updatedArtist.MbzArtistID).To(Equal(expectedMBID))
|
||||
// Biography gets sanitized and space-replaced in populateArtistInfo
|
||||
Expect(updatedArtist.Biography).To(Equal("Artist Bio"))
|
||||
Expect(updatedArtist.ExternalUrl).To(Equal(expectedURL))
|
||||
Expect(updatedArtist.LargeImageUrl).To(Equal("http://large.jpg"))
|
||||
@ -108,17 +95,14 @@ var _ = Describe("Provider UpdateArtistInfo", func() {
|
||||
Expect(updatedArtist.ExternalInfoUpdatedAt).NotTo(BeNil())
|
||||
Expect(*updatedArtist.ExternalInfoUpdatedAt).To(BeTemporally("~", time.Now(), time.Second))
|
||||
|
||||
// Assert similar artists loaded (only the one present in DS)
|
||||
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())
|
||||
// Cannot assert UpdateExternalInfo on mockArtistRepo
|
||||
})
|
||||
|
||||
It("returns cached info when artist exists and info is not expired", func() {
|
||||
// Setup: Artist exists in DS, ExternalInfoUpdatedAt is recent
|
||||
now := time.Now()
|
||||
originalArtist := &model.Artist{
|
||||
ID: "ar-cached",
|
||||
@ -127,20 +111,17 @@ var _ = Describe("Provider UpdateArtistInfo", func() {
|
||||
ExternalUrl: "http://cached.url",
|
||||
Biography: "Cached Bio",
|
||||
LargeImageUrl: "http://cached_large.jpg",
|
||||
ExternalInfoUpdatedAt: gg.P(now.Add(-conf.Server.DevArtistInfoTimeToLive / 2)), // Not expired
|
||||
// Pre-populate with some similar artist IDs for the loading part
|
||||
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"}, // This one won't be found in DS
|
||||
{ID: "ar-similar-absent", Name: "Similar Absent"},
|
||||
},
|
||||
}
|
||||
similarInDS := model.Artist{ID: "ar-similar-present", Name: "Similar Present Updated"}
|
||||
mockArtistRepo.SetData(model.Artists{*originalArtist, similarInDS})
|
||||
|
||||
// Call the method under test
|
||||
updatedArtist, err := p.UpdateArtistInfo(ctx, "ar-cached", 5, false) // 5 similar requested, includeNotPresent=false
|
||||
updatedArtist, err := p.UpdateArtistInfo(ctx, "ar-cached", 5, false)
|
||||
|
||||
// Assertions: Should return the cached data (except similar)
|
||||
Expect(err).NotTo(HaveOccurred())
|
||||
Expect(updatedArtist).NotTo(BeNil())
|
||||
Expect(updatedArtist.ID).To(Equal(originalArtist.ID))
|
||||
@ -151,28 +132,23 @@ var _ = Describe("Provider UpdateArtistInfo", func() {
|
||||
Expect(updatedArtist.LargeImageUrl).To(Equal(originalArtist.LargeImageUrl))
|
||||
Expect(updatedArtist.ExternalInfoUpdatedAt).To(Equal(originalArtist.ExternalInfoUpdatedAt))
|
||||
|
||||
// Assert similar artists loaded (only the one found in DS)
|
||||
Expect(updatedArtist.SimilarArtists).To(HaveLen(1))
|
||||
Expect(updatedArtist.SimilarArtists[0].ID).To(Equal(similarInDS.ID))
|
||||
Expect(updatedArtist.SimilarArtists[0].Name).To(Equal(similarInDS.Name)) // Check if it loaded the updated name
|
||||
Expect(updatedArtist.SimilarArtists[0].Name).To(Equal(similarInDS.Name))
|
||||
|
||||
// Assertions: Core info agents should NOT be called
|
||||
ag.AssertNotCalled(GinkgoT(), "GetArtistMBID")
|
||||
ag.AssertNotCalled(GinkgoT(), "GetArtistImages")
|
||||
ag.AssertNotCalled(GinkgoT(), "GetArtistBiography")
|
||||
ag.AssertNotCalled(GinkgoT(), "GetArtistURL")
|
||||
// GetSimilarArtists *might* be called if similar artists weren't cached, but we check the main ones
|
||||
})
|
||||
|
||||
It("returns cached info and triggers background refresh when info is expired", func() {
|
||||
// Setup: Artist exists in DS, ExternalInfoUpdatedAt is old
|
||||
now := time.Now()
|
||||
expiredTime := now.Add(-conf.Server.DevArtistInfoTimeToLive * 2) // Definitely expired
|
||||
expiredTime := now.Add(-conf.Server.DevArtistInfoTimeToLive * 2)
|
||||
originalArtist := &model.Artist{
|
||||
ID: "ar-expired",
|
||||
Name: "Expired Artist",
|
||||
ExternalInfoUpdatedAt: gg.P(expiredTime),
|
||||
// Include some similar artists to test loading
|
||||
SimilarArtists: model.Artists{
|
||||
{ID: "ar-exp-similar", Name: "Expired Similar"},
|
||||
},
|
||||
@ -180,22 +156,18 @@ var _ = Describe("Provider UpdateArtistInfo", func() {
|
||||
similarInDS := model.Artist{ID: "ar-exp-similar", Name: "Expired Similar Updated"}
|
||||
mockArtistRepo.SetData(model.Artists{*originalArtist, similarInDS})
|
||||
|
||||
// Call the method under test
|
||||
updatedArtist, err := p.UpdateArtistInfo(ctx, "ar-expired", 5, false)
|
||||
|
||||
// Assertions: Should return the stale cached data
|
||||
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))
|
||||
|
||||
// Assert similar artists loaded
|
||||
Expect(updatedArtist.SimilarArtists).To(HaveLen(1))
|
||||
Expect(updatedArtist.SimilarArtists[0].ID).To(Equal(similarInDS.ID))
|
||||
Expect(updatedArtist.SimilarArtists[0].Name).To(Equal(similarInDS.Name))
|
||||
|
||||
// Assertions: Core info agents should NOT be called synchronously
|
||||
ag.AssertNotCalled(GinkgoT(), "GetArtistMBID")
|
||||
ag.AssertNotCalled(GinkgoT(), "GetArtistImages")
|
||||
ag.AssertNotCalled(GinkgoT(), "GetArtistBiography")
|
||||
@ -203,73 +175,53 @@ var _ = Describe("Provider UpdateArtistInfo", func() {
|
||||
})
|
||||
|
||||
It("includes non-present similar artists when includeNotPresent is true", func() {
|
||||
// Setup: Artist exists with cached similar artists (some present, some not)
|
||||
now := time.Now()
|
||||
originalArtist := &model.Artist{
|
||||
ID: "ar-similar-test",
|
||||
Name: "Similar Test Artist",
|
||||
ExternalInfoUpdatedAt: gg.P(now.Add(-conf.Server.DevArtistInfoTimeToLive / 2)), // Not expired
|
||||
ExternalInfoUpdatedAt: gg.P(now.Add(-conf.Server.DevArtistInfoTimeToLive / 2)),
|
||||
SimilarArtists: model.Artists{
|
||||
{ID: "ar-sim-present", Name: "Similar Present"},
|
||||
{ID: "", Name: "Similar Absent Raw"}, // Already marked as absent in cache
|
||||
{ID: "ar-sim-absent-lookup", Name: "Similar Absent Lookup"}, // Will fail DS lookup
|
||||
{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})
|
||||
|
||||
// Call the method under test with includeNotPresent = true
|
||||
updatedArtist, err := p.UpdateArtistInfo(ctx, "ar-similar-test", 5, true)
|
||||
|
||||
// Assertions
|
||||
Expect(err).NotTo(HaveOccurred())
|
||||
Expect(updatedArtist).NotTo(BeNil())
|
||||
|
||||
// Assert similar artists loaded (should include present and absent ones)
|
||||
Expect(updatedArtist.SimilarArtists).To(HaveLen(3))
|
||||
// Check present artist (updated from DS)
|
||||
Expect(updatedArtist.SimilarArtists[0].ID).To(Equal(similarInDS.ID))
|
||||
Expect(updatedArtist.SimilarArtists[0].Name).To(Equal(similarInDS.Name))
|
||||
// Check already absent artist (ID remains empty)
|
||||
Expect(updatedArtist.SimilarArtists[1].ID).To(BeEmpty())
|
||||
Expect(updatedArtist.SimilarArtists[1].Name).To(Equal("Similar Absent Raw"))
|
||||
// Check artist that became absent after DS lookup (ID should be empty)
|
||||
Expect(updatedArtist.SimilarArtists[2].ID).To(BeEmpty())
|
||||
Expect(updatedArtist.SimilarArtists[2].Name).To(Equal("Similar Absent Lookup"))
|
||||
})
|
||||
|
||||
It("returns error when an agent fails during info population", func() {
|
||||
// Setup: Artist exists, ExternalInfoUpdatedAt is nil
|
||||
originalArtist := &model.Artist{
|
||||
ID: "ar-agent-fail",
|
||||
Name: "Agent Fail Artist",
|
||||
}
|
||||
mockArtistRepo.SetData(model.Artists{*originalArtist})
|
||||
|
||||
// Mock agent response (GetArtistMBID fails)
|
||||
expectedErr := errors.New("agent MBID failed")
|
||||
ag.On("GetArtistMBID", ctx, "ar-agent-fail", "Agent Fail Artist").Return("", expectedErr).Once()
|
||||
// Add non-strict expectations for other concurrent agent calls that might happen before error propagation
|
||||
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()
|
||||
|
||||
// Call the method under test
|
||||
updatedArtist, err := p.UpdateArtistInfo(ctx, "ar-agent-fail", 10, false)
|
||||
|
||||
// Assertions
|
||||
Expect(err).NotTo(HaveOccurred()) // Expect no error based on current implementation
|
||||
Expect(updatedArtist).NotTo(BeNil()) // The original artist data should still be returned after refresh attempt
|
||||
Expect(err).NotTo(HaveOccurred())
|
||||
Expect(updatedArtist).NotTo(BeNil())
|
||||
Expect(updatedArtist.ID).To(Equal("ar-agent-fail"))
|
||||
ag.AssertExpectations(GinkgoT()) // Verifies that GetArtistMBID was called as expected
|
||||
// Ensure other agent calls were not made after the first failure - Not strictly true due to concurrency
|
||||
// ag.AssertNotCalled(GinkgoT(), "GetArtistImages")
|
||||
// ag.AssertNotCalled(GinkgoT(), "GetArtistBiography")
|
||||
// ag.AssertNotCalled(GinkgoT(), "GetArtistURL")
|
||||
// ag.AssertNotCalled(GinkgoT(), "GetSimilarArtists")
|
||||
ag.AssertExpectations(GinkgoT())
|
||||
})
|
||||
|
||||
// Test cases will go here
|
||||
|
||||
})
|
||||
|
@ -114,8 +114,6 @@ 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
|
||||
}
|
||||
|
||||
|
@ -76,7 +76,6 @@ func (m *MockArtistRepo) GetAll(options ...model.QueryOptions) (model.Artists, e
|
||||
if m.Err {
|
||||
return nil, errors.New("mock repo error")
|
||||
}
|
||||
// Convert map back to slice for GetAll simulation
|
||||
var allArtists model.Artists
|
||||
for _, artist := range m.Data {
|
||||
allArtists = append(allArtists, *artist)
|
||||
@ -92,8 +91,6 @@ func (m *MockArtistRepo) UpdateExternalInfo(artist *model.Artist) error {
|
||||
if m.Err {
|
||||
return errors.New("mock repo error")
|
||||
}
|
||||
// Simple implementation to prevent nil pointer dereference
|
||||
// Could update m.Data if needed for specific tests
|
||||
return nil
|
||||
}
|
||||
|
||||
|
Loading…
x
Reference in New Issue
Block a user