mirror of
https://github.com/navidrome/navidrome.git
synced 2025-04-23 15:20:33 +03:00
UpdateArtistInfo tests - wip
Signed-off-by: Deluan <deluan@navidrome.org>
This commit is contained in:
Deluan
parent
61240375db
commit
28becd5c64
275
core/extdata/provider_updateartistinfo_test.go
Normal file
275
core/extdata/provider_updateartistinfo_test.go
Normal file
@ -0,0 +1,275 @@
|
||||
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)
|
||||
}
|
||||
|
||||
var _ = Describe("Provider UpdateArtistInfo", func() {
|
||||
var (
|
||||
ctx context.Context
|
||||
p extdata.Provider
|
||||
ds *tests.MockDataStore
|
||||
ag *extdata.MockAgents
|
||||
mockArtistRepo *tests.MockArtistRepo // Convenience variable
|
||||
)
|
||||
|
||||
BeforeEach(func() {
|
||||
ctx = context.Background()
|
||||
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))
|
||||
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() {
|
||||
// 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
|
||||
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"}, // This one exists in DS
|
||||
{Name: "Similar Artist 3", MBID: "mbid-similar-3"}, // This one doesn't exist
|
||||
}
|
||||
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()
|
||||
|
||||
// Mock loading similar artists from DS (only finds ar-similar-2)
|
||||
mockArtistRepo.SetData(model.Artists{*originalArtist, similarInDS}) // Update repo data to include similar
|
||||
|
||||
// Call the method under test (includeNotPresent = false for this part)
|
||||
updatedArtist, err := p.UpdateArtistInfo(ctx, "ar-existing", 10, false) // 10 similar requested, includeNotPresent=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"))
|
||||
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))
|
||||
|
||||
// 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",
|
||||
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)), // Not expired
|
||||
// Pre-populate with some similar artist IDs for the loading part
|
||||
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
|
||||
},
|
||||
}
|
||||
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
|
||||
|
||||
// Assertions: Should return the cached data (except similar)
|
||||
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))
|
||||
|
||||
// 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
|
||||
|
||||
// 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
|
||||
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"},
|
||||
},
|
||||
}
|
||||
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")
|
||||
ag.AssertNotCalled(GinkgoT(), "GetArtistURL")
|
||||
})
|
||||
|
||||
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
|
||||
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
|
||||
},
|
||||
}
|
||||
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(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")
|
||||
})
|
||||
|
||||
// Test cases will go here
|
||||
|
||||
})
|
||||
@ -72,4 +72,29 @@ 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")
|
||||
}
|
||||
// Convert map back to slice for GetAll simulation
|
||||
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")
|
||||
}
|
||||
// Simple implementation to prevent nil pointer dereference
|
||||
// Could update m.Data if needed for specific tests
|
||||
return nil
|
||||
}
|
||||
|
||||
var _ model.ArtistRepository = (*MockArtistRepo)(nil)
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user