remove duplication

Signed-off-by: Deluan <deluan@navidrome.org>

core/extdata/extdata_helper_test.go

@@ -0,0 +1,245 @@
+package extdata
+
+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()
+}
+
+// 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)
+}
+
+// mockCombinedAgents mocks the main Agents interface used by Provider
+type mockCombinedAgents struct {
+	topSongsAgent    agents.ArtistTopSongsRetriever
+	similarAgent     agents.ArtistSimilarRetriever
+	agents.Interface // Embed to satisfy non-overridden methods
+}
+
+func (m *mockCombinedAgents) AgentName() string {
+	return "mockCombined"
+}
+
+func (m *mockCombinedAgents) 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)
+	}
+	return nil, agents.ErrNotFound
+}
+
+func (m *mockCombinedAgents) 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)
+	}
+	return nil, agents.ErrNotFound
+}
+
+// --- Stubs for other Agents interface methods ---
+
+func (m *mockCombinedAgents) GetAlbumInfo(ctx context.Context, name, artist, mbid string) (*agents.AlbumInfo, error) {
+	if m.topSongsAgent != nil {
+		if ar, ok := m.topSongsAgent.(agents.AlbumInfoRetriever); ok {
+			return ar.GetAlbumInfo(ctx, name, artist, mbid)
+		}
+	}
+	return nil, agents.ErrNotFound
+}
+
+func (m *mockCombinedAgents) GetArtistMBID(ctx context.Context, id string, name string) (string, error) {
+	if m.topSongsAgent != nil {
+		if ar, ok := m.topSongsAgent.(agents.ArtistMBIDRetriever); ok {
+			return ar.GetArtistMBID(ctx, id, name)
+		}
+	}
+	if m.similarAgent != nil {
+		if ar, ok := m.similarAgent.(agents.ArtistMBIDRetriever); ok {
+			return ar.GetArtistMBID(ctx, id, name)
+		}
+	}
+	return "", agents.ErrNotFound
+}
+
+func (m *mockCombinedAgents) GetArtistURL(ctx context.Context, id, name, mbid string) (string, error) {
+	if m.topSongsAgent != nil {
+		if ar, ok := m.topSongsAgent.(agents.ArtistURLRetriever); ok {
+			return ar.GetArtistURL(ctx, id, name, mbid)
+		}
+	}
+	if m.similarAgent != nil {
+		if ar, ok := m.similarAgent.(agents.ArtistURLRetriever); ok {
+			return ar.GetArtistURL(ctx, id, name, mbid)
+		}
+	}
+	return "", agents.ErrNotFound
+}
+
+func (m *mockCombinedAgents) GetArtistBiography(ctx context.Context, id, name, mbid string) (string, error) {
+	if m.topSongsAgent != nil {
+		if ar, ok := m.topSongsAgent.(agents.ArtistBiographyRetriever); ok {
+			return ar.GetArtistBiography(ctx, id, name, mbid)
+		}
+	}
+	if m.similarAgent != nil {
+		if ar, ok := m.similarAgent.(agents.ArtistBiographyRetriever); ok {
+			return ar.GetArtistBiography(ctx, id, name, mbid)
+		}
+	}
+	return "", agents.ErrNotFound
+}
+
+func (m *mockCombinedAgents) GetArtistImages(ctx context.Context, id, name, mbid string) ([]agents.ExternalImage, error) {
+	if m.topSongsAgent != nil {
+		if ar, ok := m.topSongsAgent.(agents.ArtistImageRetriever); ok {
+			return ar.GetArtistImages(ctx, id, name, mbid)
+		}
+	}
+	if m.similarAgent != nil {
+		if ar, ok := m.similarAgent.(agents.ArtistImageRetriever); ok {
+			return ar.GetArtistImages(ctx, id, name, mbid)
+		}
+	}
+	return nil, agents.ErrNotFound
+}

core/extdata/provider_similarsongs_test.go

@@ -20,100 +20,80 @@ var _ = Describe("Provider - SimilarSongs", func() {
 	var mockTopAgent agents.ArtistTopSongsRetriever
 	var mockSimilarAgent agents.ArtistSimilarRetriever
 	var agentsCombined Agents
-	var artistRepo *mockSimArtistRepo
-	var mediaFileRepo *mockSimMediaFileRepo
+	var artistRepo *mockArtistRepo
+	var mediaFileRepo *mockMediaFileRepo
 	var ctx context.Context
 	var originalAgentsConfig string
 
 	BeforeEach(func() {
 		ctx = context.Background()
-
-		// Store the original agents config to restore it later
 		originalAgentsConfig = conf.Server.Agents
 
-		// Setup mocks - Initialize here, but set expectations within each test
-		artistRepo = newMockSimArtistRepo()
-		mediaFileRepo = newMockSimMediaFileRepo()
+		artistRepo = newMockArtistRepo()
+		mediaFileRepo = newMockMediaFileRepo()
 
 		ds = &tests.MockDataStore{
 			MockedArtist:    artistRepo,
 			MockedMediaFile: mediaFileRepo,
 		}
 
-		// Clear the agents map to prevent interference from previous tests
 		agents.Map = nil
 
-		// Create a mock agent that implements both required interfaces
-		// Re-initialize mockAgent in each test if necessary, or ensure Calls are cleared
 		mockAgent = &mockSimilarArtistAgent{}
 		mockTopAgent = mockAgent
 		mockSimilarAgent = mockAgent
 
-		// Create a mock for the combined Agents interface
 		agentsCombined = &mockCombinedAgents{
 			topSongsAgent: mockTopAgent,
 			similarAgent:  mockSimilarAgent,
 		}
 
-		// Create the provider instance with our mock Agents implementation
 		provider = NewProvider(ds, agentsCombined)
 	})
 
 	AfterEach(func() {
-		// Restore original config
 		conf.Server.Agents = originalAgentsConfig
 	})
 
 	Describe("SimilarSongs", func() {
 		It("returns similar songs from main artist and similar artists", func() {
-			// --- Test-specific setup ---
 			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"}
 
-			// Configure the Get method (needed for GetEntityByID in getArtist)
 			artistRepo.On("Get", "artist-1").Return(&artist1, nil).Maybe()
-			artistRepo.On("Get", "artist-3").Return(&similarArtist, nil).Maybe() // For similar artist lookup if needed
+			artistRepo.On("Get", "artist-3").Return(&similarArtist, nil).Maybe()
 
-			// Configure the GetAll mock for finding the main artist in getArtist
 			artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool {
 				return opt.Max == 1 && opt.Filters != nil
 			})).Return(model.Artists{artist1}, nil).Once()
 
-			// Setup similar artists response from agent
 			similarAgentsResp := []agents.Artist{
 				{Name: "Similar Artist", MBID: "similar-mbid"},
 			}
 			mockAgent.On("GetSimilarArtists", mock.Anything, "artist-1", "Artist One", "", 15).
 				Return(similarAgentsResp, nil).Once()
 
-			// Setup for mapping similar artists in mapSimilarArtists
 			artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool {
-				// This will match the query for similar artists (no Max limit set by caller)
 				return opt.Max == 0 && opt.Filters != nil
 			})).Return(model.Artists{similarArtist}, nil).Once()
 
-			// Setup Top Songs responses
-			// Main artist songs
 			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()
 
-			// Similar artist songs
 			mockAgent.On("GetArtistTopSongs", mock.Anything, "artist-3", "Similar Artist", "", mock.Anything).
 				Return([]agents.Song{
 					{Name: "Song Three", MBID: "mbid-3"},
 				}, nil).Once()
 
-			// Setup mediafile repository to find songs by MBID (via GetAll)
 			mediaFileRepo.FindByMBID("mbid-1", song1)
 			mediaFileRepo.FindByMBID("mbid-2", song2)
 			mediaFileRepo.FindByMBID("mbid-3", song3)
-			// --- End Test-specific setup ---
 
 			songs, err := provider.SimilarSongs(ctx, "artist-1", 3)
 
@@ -125,18 +105,13 @@ var _ = Describe("Provider - SimilarSongs", func() {
 		})
 
 		It("returns nil when artist is not found", func() {
-			// --- Test-specific setup ---
-			// Use prefixed ID for GetEntityByID
 			artistRepo.On("Get", "artist-unknown-artist").Return(nil, model.ErrNotFound)
 			mediaFileRepo.On("Get", "artist-unknown-artist").Return(nil, model.ErrNotFound)
 
-			// Setup for getArtist fallback to GetAll
 			artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool {
 				return opt.Max == 1 && opt.Filters != nil
 			})).Return(model.Artists{}, nil).Maybe()
-			// --- End Test-specific setup ---
 
-			// Use prefixed ID
 			songs, err := provider.SimilarSongs(ctx, "artist-unknown-artist", 5)
 
 			Expect(err).To(Equal(model.ErrNotFound))
@@ -144,35 +119,27 @@ var _ = Describe("Provider - SimilarSongs", func() {
 		})
 
 		It("returns songs from main artist when GetSimilarArtists returns error", func() {
-			// --- Test-specific setup ---
 			artist1 := model.Artist{ID: "artist-1", Name: "Artist One"}
 			song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1"}
 
-			// Configure artist repo Get method for getArtist
 			artistRepo.On("Get", "artist-1").Return(&artist1, nil).Maybe()
-			// Configure GetAll fallback for getArtist
 			artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool {
 				return opt.Max == 1 && opt.Filters != nil
-			})).Return(model.Artists{artist1}, nil).Maybe() // Maybe because Get should find it
+			})).Return(model.Artists{artist1}, nil).Maybe()
 
-			// Set the error for GetSimilarArtists
 			mockAgent.On("GetSimilarArtists", mock.Anything, "artist-1", "Artist One", "", 15).
 				Return(nil, errors.New("error getting similar artists")).Once()
 
-			// Expect call to mapSimilarArtists -> artistRepo.GetAll (returns empty)
 			artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool {
-				return opt.Max == 0 && opt.Filters != nil // Matcher for mapSimilarArtists
+				return opt.Max == 0 && opt.Filters != nil
 			})).Return(model.Artists{}, nil).Once()
 
-			// Setup for main artist's top songs
 			mockAgent.On("GetArtistTopSongs", mock.Anything, "artist-1", "Artist One", "", mock.Anything).
 				Return([]agents.Song{
 					{Name: "Song One", MBID: "mbid-1"},
 				}, nil).Once()
 
-			// Setup mediafile repo for finding the song
 			mediaFileRepo.FindByMBID("mbid-1", song1)
-			// --- End Test-specific setup ---
 
 			songs, err := provider.SimilarSongs(ctx, "artist-1", 5)
 
@@ -182,29 +149,22 @@ var _ = Describe("Provider - SimilarSongs", func() {
 		})
 
 		It("returns empty list when GetArtistTopSongs returns error", func() {
-			// --- Test-specific setup ---
 			artist1 := model.Artist{ID: "artist-1", Name: "Artist One"}
 
-			// Configure artist repo Get method for getArtist
 			artistRepo.On("Get", "artist-1").Return(&artist1, nil).Maybe()
-			// Configure GetAll fallback for getArtist
 			artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool {
 				return opt.Max == 1 && opt.Filters != nil
-			})).Return(model.Artists{artist1}, nil).Maybe() // Maybe because Get should find it
+			})).Return(model.Artists{artist1}, nil).Maybe()
 
-			// Expect GetSimilarArtists call (returns empty)
 			mockAgent.On("GetSimilarArtists", mock.Anything, "artist-1", "Artist One", "", 15).
 				Return([]agents.Artist{}, nil).Once()
 
-			// Expect call to mapSimilarArtists -> artistRepo.GetAll (returns empty)
 			artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool {
-				return opt.Max == 0 && opt.Filters != nil // Matcher for mapSimilarArtists
+				return opt.Max == 0 && opt.Filters != nil
 			})).Return(model.Artists{}, nil).Once()
 
-			// Set error for GetArtistTopSongs (for the main artist)
 			mockAgent.On("GetArtistTopSongs", mock.Anything, "artist-1", "Artist One", "", mock.Anything).
 				Return(nil, errors.New("error getting top songs")).Once()
-			// --- End Test-specific setup ---
 
 			songs, err := provider.SimilarSongs(ctx, "artist-1", 5)
 
@@ -213,38 +173,30 @@ var _ = Describe("Provider - SimilarSongs", func() {
 		})
 
 		It("respects count parameter", func() {
-			// --- Test-specific setup ---
 			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"}
 
-			// Configure artist repo Get method for getArtist
 			artistRepo.On("Get", "artist-1").Return(&artist1, nil).Maybe()
-			// Configure GetAll fallback for getArtist
 			artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool {
 				return opt.Max == 1 && opt.Filters != nil
-			})).Return(model.Artists{artist1}, nil).Maybe() // Maybe because Get should find it
+			})).Return(model.Artists{artist1}, nil).Maybe()
 
-			// Expect GetSimilarArtists call (returns empty)
 			mockAgent.On("GetSimilarArtists", mock.Anything, "artist-1", "Artist One", "", 15).
 				Return([]agents.Artist{}, nil).Once()
 
-			// Expect call to mapSimilarArtists -> artistRepo.GetAll (returns empty)
 			artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool {
-				return opt.Max == 0 && opt.Filters != nil // Matcher for mapSimilarArtists
+				return opt.Max == 0 && opt.Filters != nil
 			})).Return(model.Artists{}, nil).Once()
 
-			// Setup for main artist's top songs
 			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()
 
-			// Setup mediafile repo for finding the songs
 			mediaFileRepo.FindByMBID("mbid-1", song1)
 			mediaFileRepo.FindByMBID("mbid-2", song2)
-			// --- End Test-specific setup ---
 
 			songs, err := provider.SimilarSongs(ctx, "artist-1", 1)
 
@@ -254,194 +206,3 @@ var _ = Describe("Provider - SimilarSongs", func() {
 		})
 	})
 })
-
-// Combined implementation of both ArtistTopSongsRetriever and ArtistSimilarRetriever interfaces
-type mockSimilarArtistAgent struct {
-	mock.Mock
-	agents.Interface
-}
-
-func (m *mockSimilarArtistAgent) AgentName() string {
-	return "mock"
-}
-
-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)
-}
-
-// A simple implementation of the Agents interface that combines separate implementations
-type mockCombinedAgents struct {
-	topSongsAgent agents.ArtistTopSongsRetriever
-	similarAgent  agents.ArtistSimilarRetriever
-}
-
-func (m *mockCombinedAgents) AgentName() string {
-	return "mockCombined"
-}
-
-func (m *mockCombinedAgents) GetArtistMBID(ctx context.Context, id string, name string) (string, error) {
-	return "", nil
-}
-
-func (m *mockCombinedAgents) GetArtistURL(ctx context.Context, id, name, mbid string) (string, error) {
-	return "", nil
-}
-
-func (m *mockCombinedAgents) GetArtistBiography(ctx context.Context, id, name, mbid string) (string, error) {
-	return "", nil
-}
-
-func (m *mockCombinedAgents) 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)
-	}
-	return nil, agents.ErrNotFound
-}
-
-func (m *mockCombinedAgents) GetArtistImages(ctx context.Context, id, name, mbid string) ([]agents.ExternalImage, error) {
-	return nil, agents.ErrNotFound
-}
-
-func (m *mockCombinedAgents) 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)
-	}
-	return nil, agents.ErrNotFound
-}
-
-func (m *mockCombinedAgents) GetAlbumInfo(ctx context.Context, name, artist, mbid string) (*agents.AlbumInfo, error) {
-	return nil, agents.ErrNotFound
-}
-
-// Mocked ArtistRepo for similar songs tests
-type mockSimArtistRepo struct {
-	mock.Mock
-	model.ArtistRepository
-}
-
-func newMockSimArtistRepo() *mockSimArtistRepo {
-	return &mockSimArtistRepo{}
-}
-
-func (m *mockSimArtistRepo) SetData(artists model.Artists) {
-	// Store the data for Get queries
-	for _, a := range artists {
-		// Capture the loop variable by value
-		artistCopy := a
-		// Revert: Get does not take context
-		m.On("Get", artistCopy.ID).Return(&artistCopy, nil)
-	}
-}
-
-// Revert: Get does not take context
-func (m *mockSimArtistRepo) Get(id string) (*model.Artist, error) {
-	// Revert: Remove context from Called
-	args := m.Called(id)
-	if args.Get(0) == nil {
-		return nil, args.Error(1)
-	}
-	return args.Get(0).(*model.Artist), args.Error(1)
-}
-
-func (m *mockSimArtistRepo) SetError(hasError bool) {
-	if hasError {
-		// Revert: Remove context from GetAll mock setup
-		m.On("GetAll", mock.Anything).Return(nil, errors.New("error"))
-	}
-}
-
-func (m *mockSimArtistRepo) FindByName(name string, artist model.Artist) {
-	// Set up a mock for finding an artist by name with LIKE filter, using Anything matcher for flexibility
-	// Revert: Remove context from GetAll mock setup
-	m.On("GetAll", mock.Anything).Return(model.Artists{artist}, nil).Once()
-}
-
-// Revert: Remove context from GetAll signature
-func (m *mockSimArtistRepo) GetAll(options ...model.QueryOptions) (model.Artists, error) {
-	// Pass options correctly to Called
-	// Convert options slice to []interface{} for Called
-	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)
-}
-
-// Mocked MediaFileRepo for similar songs tests
-type mockSimMediaFileRepo struct {
-	mock.Mock
-	model.MediaFileRepository
-}
-
-func newMockSimMediaFileRepo() *mockSimMediaFileRepo {
-	return &mockSimMediaFileRepo{}
-}
-
-func (m *mockSimMediaFileRepo) SetData(mediaFiles model.MediaFiles) {
-	// Store the data for Get queries
-	for _, mf := range mediaFiles {
-		mfCopy := mf // Capture loop variable
-		// Revert: Get does not take context
-		m.On("Get", mfCopy.ID).Return(&mfCopy, nil)
-	}
-}
-
-// Revert: Get does not take context
-func (m *mockSimMediaFileRepo) Get(id string) (*model.MediaFile, error) {
-	// Revert: Remove context from Called
-	args := m.Called(id)
-	if args.Get(0) == nil {
-		return nil, args.Error(1)
-	}
-	return args.Get(0).(*model.MediaFile), args.Error(1)
-}
-
-func (m *mockSimMediaFileRepo) SetError(hasError bool) {
-	if hasError {
-		// Revert: Remove context from GetAll mock setup
-		m.On("GetAll", mock.Anything).Return(nil, errors.New("error"))
-	}
-}
-
-func (m *mockSimMediaFileRepo) FindByMBID(mbid string, mediaFile model.MediaFile) {
-	// Set up a mock for finding a media file by MBID using Anything matcher for flexibility
-	// Revert: Remove context from GetAll mock setup
-	m.On("GetAll", mock.Anything).Return(model.MediaFiles{mediaFile}, nil).Once()
-}
-
-func (m *mockSimMediaFileRepo) FindByArtistAndTitle(artistID string, title string, mediaFile model.MediaFile) {
-	// Set up a mock for finding a media file by artist ID and title
-	// Revert: Remove context from GetAll mock setup
-	m.On("GetAll", mock.Anything).Return(model.MediaFiles{mediaFile}, nil).Once()
-}
-
-// Revert: Remove context from GetAll signature
-func (m *mockSimMediaFileRepo) GetAll(options ...model.QueryOptions) (model.MediaFiles, error) {
-	// Pass options correctly to Called
-	// Convert options slice to []interface{} for Called
-	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)
-}

core/extdata/provider_topsongs_test.go

@@ -4,12 +4,12 @@ import (
 	"context"
 	"errors"
 
+	"github.com/Masterminds/squirrel"
 	"github.com/navidrome/navidrome/conf"
 	"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/log"
 	"github.com/navidrome/navidrome/model"
 	"github.com/navidrome/navidrome/tests"
 	. "github.com/onsi/ginkgo/v2"
@@ -17,23 +17,20 @@ import (
 	"github.com/stretchr/testify/mock"
 )
 
-var _ = Describe("Provider", func() {
+var _ = Describe("Provider - TopSongs", func() {
 	var ds model.DataStore
 	var provider Provider
-	var mockAgent *mockArtistTopSongsAgent
-	var mockAgents *mockAllAgents
 	var artistRepo *mockArtistRepo
 	var mediaFileRepo *mockMediaFileRepo
+	var mockTopSongsAgent *mockArtistTopSongsAgent
+	var agentsCombined Agents
 	var ctx context.Context
 	var originalAgentsConfig string
 
 	BeforeEach(func() {
 		ctx = context.Background()
-
-		// Store the original agents config to restore it later
 		originalAgentsConfig = conf.Server.Agents
 
-		// Setup mocks
 		artistRepo = newMockArtistRepo()
 		mediaFileRepo = newMockMediaFileRepo()
 
@@ -42,194 +39,110 @@ var _ = Describe("Provider", func() {
 			MockedMediaFile: mediaFileRepo,
 		}
 
-		// Clear the agents map to prevent interference from previous tests
+		mockTopSongsAgent = &mockArtistTopSongsAgent{}
+
+		agentsCombined = &mockCombinedAgents{
+			topSongsAgent: mockTopSongsAgent,
+			similarAgent:  nil,
+		}
+
 		agents.Map = nil
-
-		// Create a mock agent
-		mockAgent = &mockArtistTopSongsAgent{}
-		log.Debug(ctx, "Creating mock agent", "agent", mockAgent)
-
-		// Create a mock for the Agents interface that Provider depends on
-		mockAgents = newMockAllAgents()
-		mockAgents.topSongsRetriever = mockAgent
-
-		// Create the provider instance with our mock Agents implementation
-		provider = NewProvider(ds, mockAgents)
+		provider = NewProvider(ds, agentsCombined)
 	})
 
 	AfterEach(func() {
-		// Restore original config
 		conf.Server.Agents = originalAgentsConfig
+		agents.Map = nil
 	})
 
-	Describe("TopSongs with direct agent injection", func() {
+	Describe("TopSongs", func() {
 		BeforeEach(func() {
-			// Set up test data
 			artist1 := model.Artist{ID: "artist-1", Name: "Artist One"}
 			artist2 := model.Artist{ID: "artist-2", Name: "Artist Two"}
-
-			song1 := model.MediaFile{
-				ID:                "song-1",
-				Title:             "Song One",
-				Artist:            "Artist One",
-				ArtistID:          "artist-1",
-				AlbumArtistID:     "artist-1",
-				MbzReleaseTrackID: "mbid-1",
-				Missing:           false,
-			}
-
-			song2 := model.MediaFile{
-				ID:                "song-2",
-				Title:             "Song Two",
-				Artist:            "Artist One",
-				ArtistID:          "artist-1",
-				AlbumArtistID:     "artist-1",
-				MbzReleaseTrackID: "mbid-2",
-				Missing:           false,
-			}
-
-			song3 := model.MediaFile{
-				ID:                "song-3",
-				Title:             "Song Three",
-				Artist:            "Artist Two",
-				ArtistID:          "artist-2",
-				AlbumArtistID:     "artist-2",
-				MbzReleaseTrackID: "mbid-3",
-				Missing:           false,
-			}
-
-			// Set up basic data for the repos
 			artistRepo.SetData(model.Artists{artist1, artist2})
+
+			song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1", MbzReleaseTrackID: "mbid-1"}
+			song2 := model.MediaFile{ID: "song-2", Title: "Song Two", ArtistID: "artist-1", MbzReleaseTrackID: "mbid-2"}
+			song3 := model.MediaFile{ID: "song-3", Title: "Song Three", ArtistID: "artist-2", MbzReleaseTrackID: "mbid-3"}
 			mediaFileRepo.SetData(model.MediaFiles{song1, song2, song3})
 
+			mockTopSongsAgent.SetTopSongs([]agents.Song{
+				{Name: "Song One", MBID: "mbid-1"},
+				{Name: "Song Two", MBID: "mbid-2"},
+			})
 		})
 
-		It("returns matching songs from the agent results", func() {
-			// Setup data needed for this specific test
+		It("returns top songs for a known artist", func() {
 			artist1 := model.Artist{ID: "artist-1", Name: "Artist One"}
 			artistRepo.FindByName("Artist One", artist1)
 
-			song1 := model.MediaFile{
-				ID:                "song-1",
-				Title:             "Song One",
-				ArtistID:          "artist-1",
-				MbzReleaseTrackID: "mbid-1",
-				Missing:           false,
-			}
-
-			song2 := model.MediaFile{
-				ID:                "song-2",
-				Title:             "Song Two",
-				ArtistID:          "artist-1",
-				MbzReleaseTrackID: "mbid-2",
-				Missing:           false,
-			}
-
+			song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1", MbzReleaseTrackID: "mbid-1"}
+			song2 := model.MediaFile{ID: "song-2", Title: "Song Two", ArtistID: "artist-1", MbzReleaseTrackID: "mbid-2"}
 			mediaFileRepo.FindByMBID("mbid-1", song1)
 			mediaFileRepo.FindByMBID("mbid-2", song2)
 
-			// Configure the mockAgent to return some top songs
-			mockAgent.topSongs = []agents.Song{
-				{Name: "Song One", MBID: "mbid-1"},
-				{Name: "Song Two", MBID: "mbid-2"},
-			}
-
-			songs, err := provider.TopSongs(ctx, "Artist One", 5)
+			songs, err := provider.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"))
-
-			// Verify the agent was called with the right parameters
-			Expect(mockAgent.lastArtistID).To(Equal("artist-1"))
-			Expect(mockAgent.lastArtistName).To(Equal("Artist One"))
-			Expect(mockAgent.lastCount).To(Equal(5))
 		})
 
-		It("returns nil when artist is not found", func() {
-			// Clear any previous mock setup to avoid conflicts
-			artistRepo = newMockArtistRepo()
-
-			// Setup for artist not found scenario - return empty list
-			artistRepo.On("GetAll", mock.Anything).Return(model.Artists{}, nil).Once()
-
-			// We need to recreate the datastore with the new mocks
-			ds = &tests.MockDataStore{
-				MockedArtist:    artistRepo,
-				MockedMediaFile: mediaFileRepo,
-			}
-
-			// Create a new provider with the updated datastore
-			provider = NewProvider(ds, mockAgents)
+		It("returns nil for an unknown artist", func() {
+			artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool {
+				if opt.Max != 1 || opt.Filters == nil {
+					return false
+				}
+				_, ok := opt.Filters.(squirrel.Like)
+				return ok
+			})).Return(model.Artists{}, model.ErrNotFound).Once()
 
 			songs, err := provider.TopSongs(ctx, "Unknown Artist", 5)
 
-			Expect(err).To(BeNil())
+			Expect(err).ToNot(HaveOccurred())
 			Expect(songs).To(BeNil())
 		})
 
-		It("returns empty list when no matching songs are found", func() {
-			// Set up artist data
+		It("returns nil when the agent returns an error", func() {
 			artist1 := model.Artist{ID: "artist-1", Name: "Artist One"}
 			artistRepo.FindByName("Artist One", artist1)
 
-			// Configure the agent to return songs that don't match our repo
-			mockAgent.topSongs = []agents.Song{
-				{Name: "Nonexistent Song", MBID: "unknown-mbid"},
-			}
+			mockTopSongsAgent.SetError(errors.New("agent error"))
 
-			// Default to empty response for any queries
-			mediaFileRepo.On("GetAll", mock.Anything).Return(model.MediaFiles{}, nil).Maybe()
+			song1 := model.MediaFile{ID: "song-1"}
+			song2 := model.MediaFile{ID: "song-2"}
+			mediaFileRepo.FindByMBID("mbid-1", song1)
+			mediaFileRepo.FindByMBID("mbid-2", song2)
 
 			songs, err := provider.TopSongs(ctx, "Artist One", 5)
 
 			Expect(err).ToNot(HaveOccurred())
-			Expect(songs).To(HaveLen(0))
-		})
-
-		It("returns nil when agent returns errors", func() {
-			// Set up artist data
-			artist1 := model.Artist{ID: "artist-1", Name: "Artist One"}
-			artistRepo.SetData(model.Artists{artist1})
-			artistRepo.FindByName("Artist One", artist1)
-
-			// Set the error
-			testError := errors.New("some agent error")
-			mockAgent.err = testError
-
-			songs, err := provider.TopSongs(ctx, "Artist One", 5)
-
-			// Current behavior returns nil for both error and songs
-			Expect(err).To(BeNil())
 			Expect(songs).To(BeNil())
 		})
 
-		It("respects count parameter", func() {
-			// Set up test data
+		It("returns nil when the agent returns ErrNotFound", func() {
 			artist1 := model.Artist{ID: "artist-1", Name: "Artist One"}
-			song1 := model.MediaFile{
-				ID:                "song-1",
-				Title:             "Song One",
-				ArtistID:          "artist-1",
-				MbzReleaseTrackID: "mbid-1",
-				Missing:           false,
-			}
-
-			// Set up mocks
-			artistRepo.SetData(model.Artists{artist1})
 			artistRepo.FindByName("Artist One", artist1)
+
+			mockTopSongsAgent.SetError(agents.ErrNotFound)
+
+			song1 := model.MediaFile{ID: "song-1"}
+			song2 := model.MediaFile{ID: "song-2"}
 			mediaFileRepo.FindByMBID("mbid-1", song1)
+			mediaFileRepo.FindByMBID("mbid-2", song2)
 
-			// Configure the mockAgent
-			mockAgent.topSongs = []agents.Song{
-				{Name: "Song One", MBID: "mbid-1"},
-				{Name: "Song Two", MBID: "mbid-2"},
-				{Name: "Song Three", MBID: "mbid-3"},
-			}
+			songs, err := provider.TopSongs(ctx, "Artist One", 5)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(songs).To(BeNil())
+		})
 
-			// Default to empty response for any queries
-			mediaFileRepo.On("GetAll", mock.Anything).Return(model.MediaFiles{}, nil).Maybe()
+		It("returns fewer songs if count is less than available top songs", func() {
+			artist1 := model.Artist{ID: "artist-1", Name: "Artist One"}
+			artistRepo.FindByName("Artist One", artist1)
+
+			song1 := model.MediaFile{ID: "song-1"}
+			mediaFileRepo.FindByMBID("mbid-1", song1)
 
 			songs, err := provider.TopSongs(ctx, "Artist One", 1)
 
@@ -237,281 +150,99 @@ var _ = Describe("Provider", func() {
 			Expect(songs).To(HaveLen(1))
 			Expect(songs[0].ID).To(Equal("song-1"))
 		})
-	})
 
-	Describe("TopSongs with agent registration", func() {
-		BeforeEach(func() {
-			// Set our mock agent as the only agent
-			conf.Server.Agents = "mock"
-
-			// Set up test data
+		It("returns fewer songs if fewer matching tracks are found", func() {
 			artist1 := model.Artist{ID: "artist-1", Name: "Artist One"}
-
-			song1 := model.MediaFile{
-				ID:                "song-1",
-				Title:             "Song One",
-				Artist:            "Artist One",
-				ArtistID:          "artist-1",
-				MbzReleaseTrackID: "mbid-1",
-				Missing:           false,
-			}
-
-			song2 := model.MediaFile{
-				ID:                "song-2",
-				Title:             "Song Two",
-				Artist:            "Artist One",
-				ArtistID:          "artist-1",
-				MbzReleaseTrackID: "mbid-2",
-				Missing:           false,
-			}
-
-			// Set up basic data for the repos
-			artistRepo.SetData(model.Artists{artist1})
-			mediaFileRepo.SetData(model.MediaFiles{song1, song2})
-
-			// Set up the specific mock responses needed for the TopSongs method
 			artistRepo.FindByName("Artist One", artist1)
-			mediaFileRepo.FindByMBID("mbid-1", song1)
-			mediaFileRepo.FindByMBID("mbid-2", song2)
 
-			// Setup default behavior for empty searches
-			mediaFileRepo.On("GetAll", mock.Anything).Return(model.MediaFiles{}, nil).Maybe()
+			song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1", MbzReleaseTrackID: "mbid-1"}
 
-			// Configure and register the agent
-			mockAgent.topSongs = []agents.Song{
-				{Name: "Song One", MBID: "mbid-1"},
-				{Name: "Song Two", MBID: "mbid-2"},
+			matcherForMBID := func(expectedMBID string) func(opt model.QueryOptions) bool {
+				return func(opt model.QueryOptions) bool {
+					if opt.Filters == nil {
+						return false
+					}
+					andClause, ok := opt.Filters.(squirrel.And)
+					if !ok {
+						return false
+					}
+					for _, condition := range andClause {
+						if eqClause, ok := condition.(squirrel.Eq); ok {
+							if mbid, exists := eqClause["mbz_recording_id"]; exists {
+								return mbid == expectedMBID
+							}
+						}
+					}
+					return false
+				}
 			}
 
-			// Register our mock agent
-			agents.Register("mock", func(model.DataStore) agents.Interface { return mockAgent })
+			matcherForTitleArtistFallback := func(artistID, title string) func(opt model.QueryOptions) bool {
+				return func(opt model.QueryOptions) bool {
+					if opt.Filters == nil {
+						return false
+					}
+					andClause, ok := opt.Filters.(squirrel.And)
+					if !ok || len(andClause) < 3 {
+						return false
+					}
+					foundLike := false
+					for _, condition := range andClause {
+						if likeClause, ok := condition.(squirrel.Like); ok {
+							if _, exists := likeClause["order_title"]; exists {
+								foundLike = true
+								break
+							}
+						}
+					}
+					return foundLike
+				}
+			}
 
-			// Create the provider instance with registered agents
-			provider = NewProvider(ds, agents.GetAgents(ds))
-		})
+			mediaFileRepo.On("GetAll", mock.MatchedBy(matcherForMBID("mbid-1"))).Return(model.MediaFiles{song1}, nil).Once()
+			mediaFileRepo.On("GetAll", mock.MatchedBy(matcherForMBID("mbid-2"))).Return(model.MediaFiles{}, nil).Once()
+			mediaFileRepo.On("GetAll", mock.MatchedBy(matcherForTitleArtistFallback("artist-1", "Song Two"))).Return(model.MediaFiles{}, nil).Once()
 
-		It("returns matching songs from the registered agent", func() {
-			songs, err := provider.TopSongs(ctx, "Artist One", 5)
+			songs, err := provider.TopSongs(ctx, "Artist One", 2)
 
 			Expect(err).ToNot(HaveOccurred())
-			Expect(songs).To(HaveLen(2))
+			Expect(songs).To(HaveLen(1))
 			Expect(songs[0].ID).To(Equal("song-1"))
-			Expect(songs[1].ID).To(Equal("song-2"))
-		})
-	})
-
-	Describe("Error propagation from agents", func() {
-		BeforeEach(func() {
-			// Set up test data
-			artist1 := model.Artist{ID: "artist-1", Name: "Artist One"}
-
-			// Set up basic data for the repos
-			artistRepo.SetData(model.Artists{artist1})
-			artistRepo.FindByName("Artist One", artist1)
-
-			// Setup default behavior for empty searches
-			mediaFileRepo.On("GetAll", mock.Anything).Return(model.MediaFiles{}, nil).Maybe()
-
-			// Create a mock with a custom GetArtistTopSongs implementation that returns an error
-			testError := errors.New("direct agent error")
-			mockAgent.getArtistTopSongsFn = func(ctx context.Context, id, artistName, mbid string, count int) ([]agents.Song, error) {
-				return nil, testError
-			}
-		})
-
-		It("handles errors from the agent according to current behavior", func() {
-			songs, err := provider.TopSongs(ctx, "Artist One", 5)
-
-			// Current behavior returns nil for both error and songs
-			Expect(err).To(BeNil())
-			Expect(songs).To(BeNil())
 		})
 	})
 })
 
-// MockAllAgents implements the Agents interface that Provider depends on
-type mockAllAgents struct {
-	mock.Mock
-	topSongsRetriever agents.ArtistTopSongsRetriever
-}
-
-func newMockAllAgents() *mockAllAgents {
-	return &mockAllAgents{}
-}
-
-func (m *mockAllAgents) AgentName() string {
-	return "mockAllAgents"
-}
-
-func (m *mockAllAgents) GetArtistMBID(ctx context.Context, id string, name string) (string, error) {
-	args := m.Called(ctx, id, name)
-	return args.String(0), args.Error(1)
-}
-
-func (m *mockAllAgents) GetArtistURL(ctx context.Context, id, name, mbid string) (string, error) {
-	args := m.Called(ctx, id, name, mbid)
-	return args.String(0), args.Error(1)
-}
-
-func (m *mockAllAgents) GetArtistBiography(ctx context.Context, id, name, mbid string) (string, error) {
-	args := m.Called(ctx, id, name, mbid)
-	return args.String(0), args.Error(1)
-}
-
-func (m *mockAllAgents) 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 nil, args.Error(1)
-	}
-	return args.Get(0).([]agents.Artist), args.Error(1)
-}
-
-func (m *mockAllAgents) GetArtistImages(ctx context.Context, id, name, mbid string) ([]agents.ExternalImage, error) {
-	args := m.Called(ctx, id, name, mbid)
-	if args.Get(0) == nil {
-		return nil, args.Error(1)
-	}
-	return args.Get(0).([]agents.ExternalImage), args.Error(1)
-}
-
-func (m *mockAllAgents) GetArtistTopSongs(ctx context.Context, id, artistName, mbid string, count int) ([]agents.Song, error) {
-	// Delegate to the top songs retriever if it's set
-	if m.topSongsRetriever != nil {
-		return m.topSongsRetriever.GetArtistTopSongs(ctx, id, artistName, mbid, count)
-	}
-
-	args := m.Called(ctx, id, artistName, mbid, count)
-	if args.Get(0) == nil {
-		return nil, args.Error(1)
-	}
-	return args.Get(0).([]agents.Song), args.Error(1)
-}
-
-func (m *mockAllAgents) 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)
-}
-
-// Make sure mockAllAgents implements the Agents interface
-var _ Agents = (*mockAllAgents)(nil)
-
-// Mock agent implementation for testing
+// Mock implementation for ArtistTopSongsRetriever
+// This remains here as it's specific to TopSongs tests and simpler than mockSimilarArtistAgent
 type mockArtistTopSongsAgent struct {
-	agents.Interface
-	err                 error
-	topSongs            []agents.Song
-	lastArtistID        string
-	lastArtistName      string
-	lastMBID            string
-	lastCount           int
-	getArtistTopSongsFn func(ctx context.Context, id, artistName, mbid string, count int) ([]agents.Song, error)
+	mock.Mock
+	topSongs []agents.Song
+	err      error
 }
 
 func (m *mockArtistTopSongsAgent) AgentName() string {
-	return "mock"
+	return "mockTopSongs"
+}
+
+func (m *mockArtistTopSongsAgent) SetTopSongs(songs []agents.Song) {
+	m.topSongs = songs
+	m.err = nil
+}
+
+func (m *mockArtistTopSongsAgent) SetError(err error) {
+	m.err = err
+	m.topSongs = nil
 }
 
 func (m *mockArtistTopSongsAgent) GetArtistTopSongs(ctx context.Context, id, artistName, mbid string, count int) ([]agents.Song, error) {
-	m.lastCount = count
-	m.lastArtistID = id
-	m.lastArtistName = artistName
-	m.lastMBID = mbid
-
-	log.Debug(ctx, "MockAgent.GetArtistTopSongs called", "id", id, "name", artistName, "mbid", mbid, "count", count)
-
-	// Use the custom function if available
-	if m.getArtistTopSongsFn != nil {
-		return m.getArtistTopSongsFn(ctx, id, artistName, mbid, count)
-	}
-
 	if m.err != nil {
-		log.Debug(ctx, "MockAgent.GetArtistTopSongs returning error", "err", m.err)
 		return nil, m.err
 	}
 
-	log.Debug(ctx, "MockAgent.GetArtistTopSongs returning songs", "count", len(m.topSongs))
+	if len(m.topSongs) > count {
+		return m.topSongs[:count], nil
+	}
 	return m.topSongs, nil
 }
 
-// Make sure the mock agent implements the necessary interface
 var _ agents.ArtistTopSongsRetriever = (*mockArtistTopSongsAgent)(nil)
-
-// Mocked ArtistRepo that uses testify's mock
-type mockArtistRepo struct {
-	mock.Mock
-	model.ArtistRepository
-}
-
-func newMockArtistRepo() *mockArtistRepo {
-	return &mockArtistRepo{}
-}
-
-func (m *mockArtistRepo) SetData(artists model.Artists) {
-	// Store the data for Get queries
-	for _, a := range artists {
-		m.On("Get", a.ID).Return(&a, nil)
-	}
-}
-
-func (m *mockArtistRepo) SetError(hasError bool) {
-	if hasError {
-		m.On("GetAll", mock.Anything).Return(nil, errors.New("error"))
-	}
-}
-
-func (m *mockArtistRepo) FindByName(name string, artist model.Artist) {
-	// Set up a mock for finding an artist by name with LIKE filter, using Anything matcher for flexibility
-	m.On("GetAll", mock.Anything).Return(model.Artists{artist}, nil).Once()
-}
-
-func (m *mockArtistRepo) GetAll(options ...model.QueryOptions) (model.Artists, error) {
-	args := m.Called(mock.Anything)
-	if args.Get(0) == nil {
-		return nil, args.Error(1)
-	}
-	return args.Get(0).(model.Artists), args.Error(1)
-}
-
-// Mocked MediaFileRepo that uses testify's mock
-type mockMediaFileRepo struct {
-	mock.Mock
-	model.MediaFileRepository
-}
-
-func newMockMediaFileRepo() *mockMediaFileRepo {
-	return &mockMediaFileRepo{}
-}
-
-func (m *mockMediaFileRepo) SetData(mediaFiles model.MediaFiles) {
-	// Store the data for Get queries
-	for _, mf := range mediaFiles {
-		m.On("Get", mf.ID).Return(&mf, nil)
-	}
-}
-
-func (m *mockMediaFileRepo) SetError(hasError bool) {
-	if hasError {
-		m.On("GetAll", mock.Anything).Return(nil, errors.New("error"))
-	}
-}
-
-func (m *mockMediaFileRepo) FindByMBID(mbid string, mediaFile model.MediaFile) {
-	// Set up a mock for finding a media file by MBID using Anything matcher for flexibility
-	m.On("GetAll", mock.Anything).Return(model.MediaFiles{mediaFile}, nil).Once()
-}
-
-func (m *mockMediaFileRepo) FindByArtistAndTitle(artistID string, title string, mediaFile model.MediaFile) {
-	// Set up a mock for finding a media file by artist ID and title
-	m.On("GetAll", mock.Anything).Return(model.MediaFiles{mediaFile}, nil).Once()
-}
-
-func (m *mockMediaFileRepo) GetAll(options ...model.QueryOptions) (model.MediaFiles, error) {
-	args := m.Called(mock.Anything)
-	if args.Get(0) == nil {
-		return nil, args.Error(1)
-	}
-	return args.Get(0).(model.MediaFiles), args.Error(1)
-}