mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-31 07:30:32 +00:00
bd9fa1c602
* refactor(agents): drop single Artist fields from Song, keep only Artists Song now represents credited artists solely via the Artists slice; the single Artist/ArtistMBID fields and the ArtistList passthrough are removed. Equals continues to hash the whole value. * refactor(lastfm,listenbrainz): build Song.Artists in built-in agents Last.fm and ListenBrainz now populate the Artists slice directly. For ListenBrainz top songs, all credited artist MBIDs are mapped (the combined display name on the first credit plus MBID-only collaborators) instead of keeping only the first MBID, feeding the matcher's per-MBID specificity. * refactor(external): set Song.Artists in top-songs enrichment getMatchingTopSongs now seeds an Artists entry from the known artist when a song carries none, replacing the single Artist/ArtistMBID writes. * refactor(plugins): fold single-artist SongRef into Song.Artists SongRef keeps its single Artist/ArtistMBID fields as part of the plugin wire contract; songRefToAgentSong now folds them into a one-element Artists list when a plugin sends no artists array. * refactor(matcher): read Song.Artists directly groupQueries consumes s.Artists now that ArtistList is gone; test inputs build the Artists slice. * fix(matcher): keep MBID-only artists as identity signals Review follow-up: an artist credited only by MBID (empty ID and name) was dropped before resolution in four places, defeating the multi-MBID matching path this PR adds. - matcher.groupQueries: treat a non-empty MBID as a usable artist signal - external.getMatchingTopSongs: backfill the primary credit's name/MBID when the agent left them empty - plugins.songRefToAgentSong: fold a single-artist SongRef when only ArtistMBID is set (not just when Artist name is set) - listenbrainz.topSongArtists: return nil instead of an empty-name placeholder when neither name nor MBIDs are present * fix(external): only backfill top-song artist onto an unnamed credit Review follow-up (codex P2): the previous backfill stamped the queried artist's MBID onto Artists[0] whenever it was empty, even when that credit already named a different (e.g. featured) artist — producing a mismatched name+MBID pair that could mis-rank matches. Now only an unnamed first credit is filled (it is, by construction, the queried artist); an already-named credit is left untouched. Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org>
410 lines
18 KiB
Go
410 lines
18 KiB
Go
package external_test
|
|
|
|
import (
|
|
"context"
|
|
"errors"
|
|
|
|
_ "github.com/navidrome/navidrome/adapters/lastfm"
|
|
_ "github.com/navidrome/navidrome/adapters/listenbrainz"
|
|
"github.com/navidrome/navidrome/conf"
|
|
"github.com/navidrome/navidrome/conf/configtest"
|
|
"github.com/navidrome/navidrome/core/agents"
|
|
. "github.com/navidrome/navidrome/core/external"
|
|
"github.com/navidrome/navidrome/core/matcher"
|
|
"github.com/navidrome/navidrome/model"
|
|
"github.com/navidrome/navidrome/tests"
|
|
. "github.com/onsi/ginkgo/v2"
|
|
. "github.com/onsi/gomega"
|
|
"github.com/stretchr/testify/mock"
|
|
)
|
|
|
|
var _ = Describe("Provider - TopSongs", func() {
|
|
var (
|
|
p Provider
|
|
artistRepo *mockArtistRepo // From provider_helper_test.go
|
|
mediaFileRepo *mockMediaFileRepo // From provider_helper_test.go
|
|
ag *mockAgents // Consolidated mock from export_test.go
|
|
ctx context.Context
|
|
)
|
|
|
|
BeforeEach(func() {
|
|
DeferCleanup(configtest.SetupConfig())
|
|
// Disable fuzzy matching for these tests to avoid unexpected GetAll calls
|
|
conf.Server.Matcher.FuzzyThreshold = 100
|
|
|
|
ctx = GinkgoT().Context()
|
|
|
|
artistRepo = newMockArtistRepo() // Use helper mock
|
|
mediaFileRepo = newMockMediaFileRepo() // Use helper mock
|
|
|
|
// Configure tests.MockDataStore to use the testify/mock-based repos
|
|
ds := &tests.MockDataStore{
|
|
MockedArtist: artistRepo,
|
|
MockedMediaFile: mediaFileRepo,
|
|
}
|
|
|
|
ag = new(mockAgents)
|
|
|
|
p = NewProvider(ds, ag, matcher.New(ds))
|
|
})
|
|
|
|
It("returns top songs for a known artist", func() {
|
|
// Mock finding the artist
|
|
artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"}
|
|
artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once()
|
|
|
|
// Mock agent response
|
|
agentSongs := []agents.Song{
|
|
{Name: "Song One", MBID: "mbid-song-1"},
|
|
{Name: "Song Two", MBID: "mbid-song-2"},
|
|
}
|
|
ag.On("GetArtistTopSongs", ctx, "artist-1", "Artist One", "mbid-artist-1", 2).Return(agentSongs, nil).Once()
|
|
|
|
// Mock finding matching tracks (both returned in a single query)
|
|
song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1", MbzRecordingID: "mbid-song-1"}
|
|
song2 := model.MediaFile{ID: "song-2", Title: "Song Two", ArtistID: "artist-1", MbzRecordingID: "mbid-song-2"}
|
|
mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song1, song2}, nil).Once()
|
|
|
|
songs, err := p.TopSongs(ctx, "Artist One", 2)
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(songs).To(HaveLen(2))
|
|
Expect(songs[0].ID).To(Equal("song-1"))
|
|
Expect(songs[1].ID).To(Equal("song-2"))
|
|
artistRepo.AssertExpectations(GinkgoT())
|
|
ag.AssertExpectations(GinkgoT())
|
|
mediaFileRepo.AssertExpectations(GinkgoT())
|
|
})
|
|
|
|
It("backfills name and MBID onto an unnamed primary credit (the queried artist) and matches", func() {
|
|
artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"}
|
|
artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil)
|
|
|
|
// Agent leaves the first credit unnamed (e.g. an MBID-less collaborator slot). That blank
|
|
// credit IS the queried artist, so enrichment fills both name and MBID; the song then matches
|
|
// the queried artist's track via the backfilled identity.
|
|
agentSongs := []agents.Song{
|
|
{Name: "Song One", Artists: []agents.Artist{{}}},
|
|
}
|
|
ag.On("GetArtistTopSongs", ctx, "artist-1", "Artist One", "mbid-artist-1", 1).Return(agentSongs, nil).Once()
|
|
|
|
track := model.MediaFile{
|
|
ID: "song-1", Title: "Song One", ArtistID: "artist-1",
|
|
Participants: model.Participants{model.RoleArtist: model.ParticipantList{
|
|
{Artist: model.Artist{ID: "artist-1", Name: "Artist One", OrderArtistName: "artist one", MbzArtistID: "mbid-artist-1"}},
|
|
}},
|
|
}
|
|
mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{track}, nil)
|
|
|
|
songs, err := p.TopSongs(ctx, "Artist One", 1)
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(songs).To(HaveLen(1))
|
|
Expect(songs[0].ID).To(Equal("song-1"))
|
|
})
|
|
|
|
It("does not stamp the queried MBID onto an already-named different first credit", func() {
|
|
// The queried artist (One) appears only as a featured collaborator; the displayed first credit
|
|
// is a DIFFERENT artist (Two) returned without an MBID. Enrichment must NOT assign One's MBID
|
|
// to Two — only Two's name match (which fails here) or One's own credit may resolve the track.
|
|
artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"}
|
|
artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil)
|
|
|
|
agentSongs := []agents.Song{
|
|
{Name: "Collab Song", Artists: []agents.Artist{{Name: "Artist Two"}}},
|
|
}
|
|
ag.On("GetArtistTopSongs", ctx, "artist-1", "Artist One", "mbid-artist-1", 1).Return(agentSongs, nil).Once()
|
|
|
|
// Library track is credited to Artist One (the queried artist) under a same title. If the
|
|
// queried MBID were wrongly stamped onto the "Artist Two" credit, that mismatched name+MBID
|
|
// could mis-resolve. With the guard, "Artist Two" stays MBID-less and does not match One's track.
|
|
track := model.MediaFile{
|
|
ID: "one-track", Title: "Collab Song", ArtistID: "artist-1",
|
|
Participants: model.Participants{model.RoleArtist: model.ParticipantList{
|
|
{Artist: model.Artist{ID: "artist-1", Name: "Artist One", OrderArtistName: "artist one", MbzArtistID: "mbid-artist-1"}},
|
|
}},
|
|
}
|
|
mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{track}, nil)
|
|
|
|
songs, err := p.TopSongs(ctx, "Artist One", 1)
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
// "Artist Two" (named, MBID-less, not in the library) does not resolve to One's track.
|
|
Expect(songs).To(BeEmpty())
|
|
})
|
|
|
|
It("returns nil for an unknown artist", func() {
|
|
// Mock artist not found
|
|
artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{}, nil).Once()
|
|
|
|
songs, err := p.TopSongs(ctx, "Unknown Artist", 5)
|
|
|
|
Expect(err).ToNot(HaveOccurred()) // TopSongs returns nil error if artist not found
|
|
Expect(songs).To(BeNil())
|
|
artistRepo.AssertExpectations(GinkgoT())
|
|
ag.AssertNotCalled(GinkgoT(), "GetArtistTopSongs", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
|
|
})
|
|
|
|
It("returns error when the agent returns an error", func() {
|
|
// Mock finding the artist
|
|
artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"}
|
|
artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once()
|
|
|
|
// Mock agent error
|
|
agentErr := errors.New("agent error")
|
|
ag.On("GetArtistTopSongs", ctx, "artist-1", "Artist One", "mbid-artist-1", 5).Return(nil, agentErr).Once()
|
|
|
|
songs, err := p.TopSongs(ctx, "Artist One", 5)
|
|
|
|
Expect(err).To(MatchError(agentErr))
|
|
Expect(songs).To(BeNil())
|
|
artistRepo.AssertExpectations(GinkgoT())
|
|
ag.AssertExpectations(GinkgoT())
|
|
})
|
|
|
|
It("returns ErrNotFound when the agent returns ErrNotFound", func() {
|
|
// Mock finding the artist
|
|
artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"}
|
|
artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once()
|
|
|
|
// Mock agent ErrNotFound
|
|
ag.On("GetArtistTopSongs", ctx, "artist-1", "Artist One", "mbid-artist-1", 5).Return(nil, agents.ErrNotFound).Once()
|
|
|
|
songs, err := p.TopSongs(ctx, "Artist One", 5)
|
|
|
|
Expect(err).To(MatchError(model.ErrNotFound))
|
|
Expect(songs).To(BeNil())
|
|
artistRepo.AssertExpectations(GinkgoT())
|
|
ag.AssertExpectations(GinkgoT())
|
|
})
|
|
|
|
It("returns fewer songs if count is less than available top songs", func() {
|
|
// Mock finding the artist
|
|
artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"}
|
|
artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once()
|
|
|
|
// Mock agent response (only need 1 for the test)
|
|
agentSongs := []agents.Song{{Name: "Song One", MBID: "mbid-song-1"}}
|
|
ag.On("GetArtistTopSongs", ctx, "artist-1", "Artist One", "mbid-artist-1", 1).Return(agentSongs, nil).Once()
|
|
|
|
// Mock finding matching track
|
|
song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1", MbzRecordingID: "mbid-song-1"}
|
|
mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song1}, nil).Once()
|
|
|
|
songs, err := p.TopSongs(ctx, "Artist One", 1)
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(songs).To(HaveLen(1))
|
|
Expect(songs[0].ID).To(Equal("song-1"))
|
|
artistRepo.AssertExpectations(GinkgoT())
|
|
ag.AssertExpectations(GinkgoT())
|
|
mediaFileRepo.AssertExpectations(GinkgoT())
|
|
})
|
|
|
|
It("returns fewer songs if fewer matching tracks are found", func() {
|
|
// Mock finding the artist
|
|
artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"}
|
|
artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once()
|
|
// Matcher artist resolution for the title-match path (song2 falls through).
|
|
artistRepo.On("GetAll", mock.Anything).Return(model.Artists{artist1}, nil).Maybe()
|
|
|
|
// Mock agent response
|
|
agentSongs := []agents.Song{
|
|
{Name: "Song One", MBID: "mbid-song-1"},
|
|
{Name: "Song Two", MBID: "mbid-song-2"},
|
|
}
|
|
ag.On("GetArtistTopSongs", ctx, "artist-1", "Artist One", "mbid-artist-1", 2).Return(agentSongs, nil).Once()
|
|
|
|
// Mock finding matching tracks (only find song 1 on bulk query)
|
|
song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1", MbzRecordingID: "mbid-song-1"}
|
|
mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song1}, nil).Once() // bulk MBID query
|
|
mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{}, nil).Once() // title track-fetch for song2: no match
|
|
|
|
songs, err := p.TopSongs(ctx, "Artist One", 2)
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(songs).To(HaveLen(1))
|
|
Expect(songs[0].ID).To(Equal("song-1"))
|
|
artistRepo.AssertExpectations(GinkgoT())
|
|
ag.AssertExpectations(GinkgoT())
|
|
mediaFileRepo.AssertExpectations(GinkgoT())
|
|
})
|
|
|
|
It("returns error when context is canceled during agent call", func() {
|
|
// Mock finding the artist
|
|
artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"}
|
|
artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once()
|
|
|
|
// Setup context that will be canceled
|
|
canceledCtx, cancel := context.WithCancel(ctx)
|
|
|
|
// Mock agent call to return context canceled error
|
|
ag.On("GetArtistTopSongs", canceledCtx, "artist-1", "Artist One", "mbid-artist-1", 5).Return(nil, context.Canceled).Once()
|
|
|
|
cancel() // Cancel the context before calling
|
|
songs, err := p.TopSongs(canceledCtx, "Artist One", 5)
|
|
|
|
Expect(err).To(MatchError(context.Canceled))
|
|
Expect(songs).To(BeNil())
|
|
artistRepo.AssertExpectations(GinkgoT())
|
|
ag.AssertExpectations(GinkgoT())
|
|
})
|
|
|
|
It("falls back to title matching when MbzRecordingID is missing", func() {
|
|
// Mock finding the artist
|
|
artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"}
|
|
artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once()
|
|
// Matcher artist resolution for both title-fallback songs.
|
|
artistRepo.On("GetAll", mock.Anything).Return(model.Artists{artist1}, nil).Maybe()
|
|
|
|
// Mock agent response with songs that have NO MBID (empty string)
|
|
agentSongs := []agents.Song{
|
|
{Name: "Song One", MBID: ""}, // No MBID, should fall back to title matching
|
|
{Name: "Song Two", MBID: ""}, // No MBID, should fall back to title matching
|
|
}
|
|
ag.On("GetArtistTopSongs", ctx, "artist-1", "Artist One", "mbid-artist-1", 2).Return(agentSongs, nil).Once()
|
|
|
|
// Title track-fetch: tracks must carry RoleArtist participants so back-mapping routes them.
|
|
participant1 := model.Participant{Artist: model.Artist{ID: "artist-1", Name: "Artist One", OrderArtistName: "artist one"}}
|
|
song1 := model.MediaFile{
|
|
ID: "song-1", Title: "Song One", Artist: "Artist One", ArtistID: "artist-1",
|
|
Participants: model.Participants{model.RoleArtist: model.ParticipantList{participant1}},
|
|
}
|
|
song2 := model.MediaFile{
|
|
ID: "song-2", Title: "Song Two", Artist: "Artist One", ArtistID: "artist-1",
|
|
Participants: model.Participants{model.RoleArtist: model.ParticipantList{participant1}},
|
|
}
|
|
mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song1, song2}, nil).Once()
|
|
|
|
songs, err := p.TopSongs(ctx, "Artist One", 2)
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(songs).To(HaveLen(2))
|
|
Expect(songs[0].ID).To(Equal("song-1"))
|
|
Expect(songs[1].ID).To(Equal("song-2"))
|
|
artistRepo.AssertExpectations(GinkgoT())
|
|
ag.AssertExpectations(GinkgoT())
|
|
mediaFileRepo.AssertExpectations(GinkgoT())
|
|
})
|
|
|
|
It("combines MBID and title matching when some songs have missing MbzRecordingID", func() {
|
|
// Mock finding the artist
|
|
artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"}
|
|
artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once()
|
|
// Matcher artist resolution for song2's title-fallback path.
|
|
artistRepo.On("GetAll", mock.Anything).Return(model.Artists{artist1}, nil).Maybe()
|
|
|
|
// Mock agent response with mixed MBID availability
|
|
agentSongs := []agents.Song{
|
|
{Name: "Song One", MBID: "mbid-song-1"}, // Has MBID, should match by MBID
|
|
{Name: "Song Two", MBID: ""}, // No MBID, should fall back to title matching
|
|
}
|
|
ag.On("GetArtistTopSongs", ctx, "artist-1", "Artist One", "mbid-artist-1", 2).Return(agentSongs, nil).Once()
|
|
|
|
// Mock the MBID query (finds song1 by MBID)
|
|
song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1", MbzRecordingID: "mbid-song-1", OrderTitle: "song one"}
|
|
mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song1}, nil).Once()
|
|
|
|
// Title track-fetch: song2 must carry RoleArtist participants for back-mapping.
|
|
participant1 := model.Participant{Artist: model.Artist{ID: "artist-1", Name: "Artist One", OrderArtistName: "artist one"}}
|
|
song2 := model.MediaFile{
|
|
ID: "song-2", Title: "Song Two", Artist: "Artist One", ArtistID: "artist-1",
|
|
Participants: model.Participants{model.RoleArtist: model.ParticipantList{participant1}},
|
|
}
|
|
mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song2}, nil).Once()
|
|
|
|
songs, err := p.TopSongs(ctx, "Artist One", 2)
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(songs).To(HaveLen(2))
|
|
Expect(songs[0].ID).To(Equal("song-1")) // Found by MBID
|
|
Expect(songs[1].ID).To(Equal("song-2")) // Found by title
|
|
artistRepo.AssertExpectations(GinkgoT())
|
|
ag.AssertExpectations(GinkgoT())
|
|
mediaFileRepo.AssertExpectations(GinkgoT())
|
|
})
|
|
|
|
It("only returns requested count when provider returns additional items", func() {
|
|
// Mock finding the artist
|
|
artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"}
|
|
artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once()
|
|
|
|
// Mock agent response
|
|
agentSongs := []agents.Song{
|
|
{Name: "Song One", MBID: "mbid-song-1"},
|
|
{Name: "Song Two", MBID: "mbid-song-2"},
|
|
}
|
|
ag.On("GetArtistTopSongs", ctx, "artist-1", "Artist One", "mbid-artist-1", 1).Return(agentSongs, nil).Once()
|
|
|
|
// Mock finding matching tracks (both returned in a single query)
|
|
song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1", MbzRecordingID: "mbid-song-1"}
|
|
song2 := model.MediaFile{ID: "song-2", Title: "Song Two", ArtistID: "artist-1", MbzRecordingID: "mbid-song-2"}
|
|
mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song1, song2}, nil).Once()
|
|
|
|
songs, err := p.TopSongs(ctx, "Artist One", 1)
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(songs).To(HaveLen(1))
|
|
Expect(songs[0].ID).To(Equal("song-1"))
|
|
artistRepo.AssertExpectations(GinkgoT())
|
|
ag.AssertExpectations(GinkgoT())
|
|
mediaFileRepo.AssertExpectations(GinkgoT())
|
|
})
|
|
|
|
It("matches songs by ID first when agent provides IDs", func() {
|
|
// Mock finding the artist
|
|
artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"}
|
|
artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once()
|
|
|
|
// Mock agent response with IDs provided (highest priority matching)
|
|
// Note: Songs have no MBID to ensure only ID matching is used
|
|
agentSongs := []agents.Song{
|
|
{ID: "song-1", Name: "Song One"},
|
|
{ID: "song-2", Name: "Song Two"},
|
|
}
|
|
ag.On("GetArtistTopSongs", ctx, "artist-1", "Artist One", "mbid-artist-1", 2).Return(agentSongs, nil).Once()
|
|
|
|
// Mock ID lookup (first query - should match both songs directly)
|
|
song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1"}
|
|
song2 := model.MediaFile{ID: "song-2", Title: "Song Two", ArtistID: "artist-1"}
|
|
mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song1, song2}, nil).Once()
|
|
|
|
songs, err := p.TopSongs(ctx, "Artist One", 2)
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(songs).To(HaveLen(2))
|
|
Expect(songs[0].ID).To(Equal("song-1"))
|
|
Expect(songs[1].ID).To(Equal("song-2"))
|
|
artistRepo.AssertExpectations(GinkgoT())
|
|
ag.AssertExpectations(GinkgoT())
|
|
mediaFileRepo.AssertExpectations(GinkgoT())
|
|
})
|
|
|
|
It("falls back to MBID when ID is not found", func() {
|
|
// Mock finding the artist
|
|
artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"}
|
|
artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once()
|
|
|
|
// Mock agent response with ID that won't be found, but MBID that will
|
|
agentSongs := []agents.Song{
|
|
{ID: "non-existent-id", Name: "Song One", MBID: "mbid-song-1"},
|
|
}
|
|
ag.On("GetArtistTopSongs", ctx, "artist-1", "Artist One", "mbid-artist-1", 1).Return(agentSongs, nil).Once()
|
|
|
|
// Mock ID lookup - returns empty (ID not found)
|
|
mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{}, nil).Once()
|
|
// Mock MBID lookup - finds the song
|
|
song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1", MbzRecordingID: "mbid-song-1"}
|
|
mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song1}, nil).Once()
|
|
|
|
songs, err := p.TopSongs(ctx, "Artist One", 1)
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(songs).To(HaveLen(1))
|
|
Expect(songs[0].ID).To(Equal("song-1"))
|
|
artistRepo.AssertExpectations(GinkgoT())
|
|
ag.AssertExpectations(GinkgoT())
|
|
mediaFileRepo.AssertExpectations(GinkgoT())
|
|
})
|
|
})
|