diff --git a/core/external/extdata_helper_test.go b/core/external/extdata_helper_test.go index d68147168..f7a155cd9 100644 --- a/core/external/extdata_helper_test.go +++ b/core/external/extdata_helper_test.go @@ -6,6 +6,7 @@ import ( "github.com/navidrome/navidrome/core/agents" "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/utils/slice" "github.com/stretchr/testify/mock" ) @@ -324,3 +325,7 @@ func (m *mockAgents) GetSimilarSongsByArtist(ctx context.Context, id, name, mbid } return nil, args.Error(1) } + +func ids(mfs model.MediaFiles) []string { + return slice.Map(mfs, func(mf model.MediaFile) string { return mf.ID }) +} diff --git a/core/external/provider.go b/core/external/provider.go index 383e7d4c9..782c7c3aa 100644 --- a/core/external/provider.go +++ b/core/external/provider.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "math/rand/v2" "sort" "strings" "time" @@ -15,21 +14,15 @@ import ( "github.com/navidrome/navidrome/core/matcher" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" - "github.com/navidrome/navidrome/persistence" "github.com/navidrome/navidrome/utils" . "github.com/navidrome/navidrome/utils/gg" - "github.com/navidrome/navidrome/utils/random" "github.com/navidrome/navidrome/utils/slice" "github.com/navidrome/navidrome/utils/str" "golang.org/x/sync/errgroup" ) const ( - maxSimilarArtists = 100 - maxSeeds = 5 - // Subsonic passes the client's count through unbounded, and it ends up as a SQL limit. 500 is - // what the widest caller (similarAlbums, limit*5) legitimately asks for. - maxSimilarSongs = 500 + maxSimilarArtists = 100 refreshDelay = 5 * time.Second refreshTimeout = 15 * time.Second refreshQueueLength = 2000 @@ -283,252 +276,6 @@ func (e *provider) populateArtistInfo(ctx context.Context, artist auxArtist) (au return artist, nil } -func (e *provider) SimilarSongs(ctx context.Context, id string, count int) (model.MediaFiles, error) { - // Subsonic passes the client's count straight through: a non-positive one has no valid - // interpretation, and an enormous one overflows the +1 in the local agent's query limit. - if count <= 0 { - return nil, nil - } - count = min(count, maxSimilarSongs) - entity, err := model.GetEntityByID(ctx, e.ds, id) - if err != nil { - // Genre ids don't resolve via GetEntityByID; look them up before giving up. - if !errors.Is(err, model.ErrNotFound) { - return nil, err - } - genre, err := e.ds.Genre(ctx).Get(id) - if err != nil { - return nil, err - } - return e.seedMix(ctx, count, func() (model.MediaFiles, error) { - return e.sampleGenreTracks(ctx, genre, maxSeeds) - }) - } - - // Try entity-specific similarity first, then fall back to seed-track sampling. - switch v := entity.(type) { - case *model.MediaFile: - return e.mixFromAgent(ctx, count, - func() ([]agents.Song, error) { - return e.ag.GetSimilarSongsByTrack(ctx, v.ID, v.Title, v.Artist, v.MbzRecordingID, count) - }, - func() (model.MediaFiles, error) { - return e.similarSongsFallback(ctx, id, count) - }) - case *model.Album: - return e.mixFromAgent(ctx, count, - func() ([]agents.Song, error) { - return e.ag.GetSimilarSongsByAlbum(ctx, v.ID, v.Name, v.AlbumArtist, v.MbzAlbumID, count) - }, - func() (model.MediaFiles, error) { - return e.seedMix(ctx, count, func() (model.MediaFiles, error) { - return e.sampleAlbumTracks(ctx, v.ID, maxSeeds) - }) - }) - case *model.Artist: - return e.mixFromAgent(ctx, count, - func() ([]agents.Song, error) { - return e.ag.GetSimilarSongsByArtist(ctx, v.ID, v.Name, v.MbzArtistID, count) - }, - func() (model.MediaFiles, error) { - if res, ferr := e.similarSongsFallback(ctx, id, count); ferr == nil && len(res) > 0 { - return res, nil - } - return e.seedMix(ctx, count, func() (model.MediaFiles, error) { - return e.sampleArtistTracks(ctx, v.ID, maxSeeds) - }) - }) - case *model.Playlist: - return e.seedMix(ctx, count, func() (model.MediaFiles, error) { - return e.samplePlaylistTracks(ctx, v.ID, maxSeeds) - }) - default: - log.Warn(ctx, "Unknown entity type", "id", id, "type", fmt.Sprintf("%T", entity)) - return nil, model.ErrNotFound - } -} - -// mixFromAgent returns the agent's recommendations matched to library tracks, or the fallback -// when the agent errors or none of its picks are in the library. -func (e *provider) mixFromAgent(ctx context.Context, count int, fetch func() ([]agents.Song, error), fallback func() (model.MediaFiles, error)) (model.MediaFiles, error) { - songs, err := fetch() - if err == nil { - matched, merr := e.matcher.MatchSongs(ctx, songs, count) - if merr != nil { - return nil, merr - } - if len(matched) > 0 { - return matched, nil - } - } - return fallback() -} - -// seedMix samples seed tracks, runs each through the agent chain's per-track similarity and merges -// the results, falling back to the seeds themselves so the result is never empty. -func (e *provider) seedMix(ctx context.Context, count int, sample func() (model.MediaFiles, error)) (model.MediaFiles, error) { - seeds, err := sample() - if err != nil { - return nil, err - } - if len(seeds) == 0 { - return nil, nil - } - seeds = seeds[:min(len(seeds), maxSeeds)] - - // The per-seed similarity calls are independent and hit the (possibly remote) agent chain, so - // run them concurrently. Best-effort: a seed that errors just contributes nothing. - perSeed := make([][]agents.Song, len(seeds)) - var g errgroup.Group - for i, seed := range seeds { - g.Go(func() error { - if s, err := e.ag.GetSimilarSongsByTrack(ctx, seed.ID, seed.Title, seed.Artist, seed.MbzRecordingID, count); err == nil { - perSeed[i] = s - } - return nil - }) - } - _ = g.Wait() - - var songs []agents.Song - for _, s := range perSeed { - songs = append(songs, s...) - } - // Match the whole merged set, not just count of it: the matcher re-emits a track when two - // seeds recommend it identically, so the duplicates have to be dropped before trimming. Every - // seed reaches the shuffle, so no seed can crowd out the others. - matched, err := e.matcher.MatchSongs(ctx, songs, len(songs)) - if err != nil { - return nil, err - } - matched = dedupByID(matched) - if len(matched) == 0 { - matched = seeds - } - rand.Shuffle(len(matched), func(i, j int) { matched[i], matched[j] = matched[j], matched[i] }) - if len(matched) > count { - matched = matched[:count] - } - return matched, nil -} - -func (e *provider) samplePlaylistTracks(ctx context.Context, playlistID string, n int) (model.MediaFiles, error) { - // Refresh: a smart playlist materializes no tracks until it is evaluated, so skipping it would - // mix an empty seed set. It is a no-op for regular playlists and inside the refresh delay. - repo := e.ds.Playlist(ctx).Tracks(playlistID, true) - if repo == nil { - return nil, model.ErrNotFound - } - // A playlist can hold the same file at several positions, so over-fetch and dedup: a repeated - // seed wastes an agent call and can reach the mix twice through the seed fallback. - tracks, err := repo.GetAll(model.QueryOptions{ - Sort: "random", - Max: n * 4, - Filters: squirrel.Eq{"missing": false}, - }) - if err != nil { - return nil, err - } - mfs := dedupByID(tracks.MediaFiles()) - return mfs[:min(len(mfs), n)], nil -} - -func dedupByID(mfs model.MediaFiles) model.MediaFiles { - seen := make(map[string]struct{}, len(mfs)) - return slice.Filter(mfs, func(mf model.MediaFile) bool { - if _, dup := seen[mf.ID]; dup { - return false - } - seen[mf.ID] = struct{}{} - return true - }) -} - -func (e *provider) sampleAlbumTracks(ctx context.Context, albumID string, n int) (model.MediaFiles, error) { - return e.sampleTracks(ctx, squirrel.Eq{"album_id": albumID}, n) -} - -func (e *provider) sampleArtistTracks(ctx context.Context, artistID string, n int) (model.MediaFiles, error) { - // media_file.artist_id is the deprecated primary artist, so it misses an artist credited only - // on the album, as on compilations. Same filter the artist listings use. - filter := persistence.ParticipantIDFilter("media_file", artistID, model.RoleArtist, model.RoleAlbumArtist) - return e.sampleTracks(ctx, filter, n) -} - -func (e *provider) sampleGenreTracks(ctx context.Context, genre *model.Genre, n int) (model.MediaFiles, error) { - return e.sampleTracks(ctx, persistence.SongGenres.ByID(genre.ID), n) -} - -// sampleTracks returns up to n random present tracks. Seeds can end up in the mix verbatim, so -// missing files would surface as unplayable entries. -func (e *provider) sampleTracks(ctx context.Context, filter squirrel.Sqlizer, n int) (model.MediaFiles, error) { - return e.ds.MediaFile(ctx).GetRandom(model.QueryOptions{ - Filters: squirrel.And{filter, squirrel.Eq{"missing": false}}, - Max: n, - }) -} - -// similarSongsFallback uses the original similar artists + top songs algorithm. The idea is to -// get the artist of the given entity, retrieve similar artists, get their top songs, and pick -// a weighted random selection of songs to return as similar songs. -func (e *provider) similarSongsFallback(ctx context.Context, id string, count int) (model.MediaFiles, error) { - artist, err := e.getArtist(ctx, id) - if err != nil { - return nil, err - } - - e.callGetSimilarArtists(ctx, e.ag, &artist, 15, false) - if utils.IsCtxDone(ctx) { - log.Warn(ctx, "SimilarSongs call canceled", ctx.Err()) - return nil, ctx.Err() - } - - weightedSongs := random.NewWeightedChooser[model.MediaFile]() - addArtist := func(a model.Artist, weightedSongs *random.WeightedChooser[model.MediaFile], count, artistWeight int) error { - if utils.IsCtxDone(ctx) { - log.Warn(ctx, "SimilarSongs call canceled", ctx.Err()) - return ctx.Err() - } - - topCount := max(count, 20) - topSongs, err := e.getMatchingTopSongs(ctx, e.ag, &auxArtist{Artist: a}, topCount) - if err != nil { - log.Warn(ctx, "Error getting artist's top songs", "artist", a.Name, err) - return nil - } - - weight := topCount * (4 + artistWeight) - for _, mf := range topSongs { - weightedSongs.Add(mf, weight) - weight -= 4 - } - return nil - } - - err = addArtist(artist.Artist, weightedSongs, count, 10) - if err != nil { - return nil, err - } - for _, a := range artist.SimilarArtists { - err := addArtist(a, weightedSongs, count, 0) - if err != nil { - return nil, err - } - } - - var similarSongs model.MediaFiles - for len(similarSongs) < count && weightedSongs.Size() > 0 { - s, err := weightedSongs.Pick() - if err != nil { - log.Warn(ctx, "Error getting weighted song", err) - continue - } - similarSongs = append(similarSongs, s) - } - - return similarSongs, nil -} - func (e *provider) TopSongs(ctx context.Context, artistName, id string, count int) (model.MediaFiles, error) { artist, err := e.findArtist(ctx, artistName, id) if err != nil { diff --git a/core/external/provider_similarsongs.go b/core/external/provider_similarsongs.go new file mode 100644 index 000000000..22b20565b --- /dev/null +++ b/core/external/provider_similarsongs.go @@ -0,0 +1,299 @@ +package external + +import ( + "context" + "errors" + "fmt" + "math/rand/v2" + + "github.com/Masterminds/squirrel" + "github.com/navidrome/navidrome/core/agents" + "github.com/navidrome/navidrome/log" + "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/persistence" + "github.com/navidrome/navidrome/utils" + "github.com/navidrome/navidrome/utils/random" + "github.com/navidrome/navidrome/utils/slice" + "golang.org/x/sync/errgroup" +) + +const ( + maxSeeds = 5 + // Subsonic passes the client's count through unbounded, and it ends up as a SQL limit. 500 is + // what the widest caller (similarAlbums, limit*5) legitimately asks for. + maxSimilarSongs = 500 +) + +func (e *provider) SimilarSongs(ctx context.Context, id string, count int) (model.MediaFiles, error) { + // Subsonic passes the client's count straight through: a non-positive one has no valid + // interpretation, and an enormous one overflows the +1 in the local agent's query limit. + if count <= 0 { + return nil, nil + } + count = min(count, maxSimilarSongs) + entity, err := model.GetEntityByID(ctx, e.ds, id) + if err != nil { + // Genre ids don't resolve via GetEntityByID; look them up before giving up. + if !errors.Is(err, model.ErrNotFound) { + return nil, err + } + genre, err := e.ds.Genre(ctx).Get(id) + if err != nil { + return nil, err + } + return e.seedMix(ctx, count, func() (model.MediaFiles, error) { + return e.sampleGenreTracks(ctx, genre, maxSeeds) + }) + } + + // Try entity-specific similarity first, then fall back to seed-track sampling. + switch v := entity.(type) { + case *model.MediaFile: + return e.mixFromAgent(ctx, count, + func() ([]agents.Song, error) { + return e.ag.GetSimilarSongsByTrack(ctx, v.ID, v.Title, v.Artist, v.MbzRecordingID, count) + }, + func() (model.MediaFiles, error) { + return e.similarSongsFallback(ctx, id, count) + }) + case *model.Album: + return e.mixFromAgent(ctx, count, + func() ([]agents.Song, error) { + return e.ag.GetSimilarSongsByAlbum(ctx, v.ID, v.Name, v.AlbumArtist, v.MbzAlbumID, count) + }, + func() (model.MediaFiles, error) { + return e.seedMix(ctx, count, func() (model.MediaFiles, error) { + return e.sampleAlbumTracks(ctx, v.ID, maxSeeds) + }) + }) + case *model.Artist: + return e.mixFromAgent(ctx, count, + func() ([]agents.Song, error) { + return e.ag.GetSimilarSongsByArtist(ctx, v.ID, v.Name, v.MbzArtistID, count) + }, + func() (model.MediaFiles, error) { + return e.similarSongsFallback(ctx, id, count) + }, + func() (model.MediaFiles, error) { + return e.seedMix(ctx, count, func() (model.MediaFiles, error) { + return e.sampleArtistTracks(ctx, v.ID, maxSeeds) + }) + }) + case *model.Playlist: + return e.seedMix(ctx, count, func() (model.MediaFiles, error) { + return e.samplePlaylistTracks(ctx, v.ID, maxSeeds) + }) + default: + log.Warn(ctx, "Unknown entity type", "id", id, "type", fmt.Sprintf("%T", entity)) + return nil, model.ErrNotFound + } +} + +// mixFromAgent returns the agent's recommendations matched to library tracks, topped up from the +// fallbacks in order when they don't fill the mix on their own. +func (e *provider) mixFromAgent(ctx context.Context, count int, fetch func() ([]agents.Song, error), fallbacks ...func() (model.MediaFiles, error)) (model.MediaFiles, error) { + var matched model.MediaFiles + if songs, err := fetch(); err == nil { + // Match the whole response: capping at count can stop before a later unique pick. + matched, err = e.matcher.MatchSongs(ctx, songs, len(songs)) + if err != nil { + return nil, err + } + } + return topUp(ctx, matched, count, fallbacks...) +} + +// topUp draws on each source in turn until the mix holds count distinct tracks. +func topUp(ctx context.Context, res model.MediaFiles, count int, sources ...func() (model.MediaFiles, error)) (model.MediaFiles, error) { + // The matcher can re-emit a track, so a full-looking res may hold fewer than count unique ones. + res = dedupByID(res) + var lastErr error + for _, more := range sources { + if len(res) >= count { + break + } + extra, err := more() + if err != nil { + log.Debug(ctx, "Could not top up a short mix", "have", len(res), "want", count, err) + lastErr = err + continue + } + res = dedupByID(append(res, extra...)) + } + if len(res) == 0 { + return nil, lastErr + } + return res[:min(len(res), count)], nil +} + +// seedMix samples seed tracks, runs each through the agent chain's per-track similarity and merges +// the results, falling back to the seeds themselves so the result is never empty. +func (e *provider) seedMix(ctx context.Context, count int, sample func() (model.MediaFiles, error)) (model.MediaFiles, error) { + seeds, err := sample() + if err != nil { + return nil, err + } + if len(seeds) == 0 { + return nil, nil + } + seeds = seeds[:min(len(seeds), maxSeeds)] + + // The per-seed similarity calls are independent and hit the (possibly remote) agent chain, so + // run them concurrently. Best-effort: a seed that errors just contributes nothing. + perSeed := make([][]agents.Song, len(seeds)) + var g errgroup.Group + for i, seed := range seeds { + g.Go(func() error { + if s, err := e.ag.GetSimilarSongsByTrack(ctx, seed.ID, seed.Title, seed.Artist, seed.MbzRecordingID, count); err == nil { + perSeed[i] = s + } + return nil + }) + } + _ = g.Wait() + + var songs []agents.Song + for _, s := range perSeed { + songs = append(songs, s...) + } + // Match the whole merged set, not just count of it: the matcher re-emits a track when two + // seeds recommend it identically, so the duplicates have to be dropped before trimming. Every + // seed reaches the shuffle, so no seed can crowd out the others. + matched, err := e.matcher.MatchSongs(ctx, songs, len(songs)) + if err != nil { + return nil, err + } + matched = dedupByID(matched) + if len(matched) == 0 { + matched = seeds + } + rand.Shuffle(len(matched), func(i, j int) { matched[i], matched[j] = matched[j], matched[i] }) + if len(matched) > count { + matched = matched[:count] + } + return matched, nil +} + +func (e *provider) samplePlaylistTracks(ctx context.Context, playlistID string, n int) (model.MediaFiles, error) { + // Refresh: a smart playlist materializes no tracks until it is evaluated, so skipping it would + // mix an empty seed set. It is a no-op for regular playlists and inside the refresh delay. + repo := e.ds.Playlist(ctx).Tracks(playlistID, true) + if repo == nil { + return nil, model.ErrNotFound + } + // A playlist can hold the same file at several positions, so over-fetch and dedup: a repeated + // seed wastes an agent call and can reach the mix twice through the seed fallback. + tracks, err := repo.GetAll(model.QueryOptions{ + Sort: "random", + Max: n * 4, + Filters: squirrel.Eq{"missing": false}, + }) + if err != nil { + return nil, err + } + mfs := dedupByID(tracks.MediaFiles()) + return mfs[:min(len(mfs), n)], nil +} + +func dedupByID(mfs model.MediaFiles) model.MediaFiles { + seen := make(map[string]struct{}, len(mfs)) + return slice.Filter(mfs, func(mf model.MediaFile) bool { + if _, dup := seen[mf.ID]; dup { + return false + } + seen[mf.ID] = struct{}{} + return true + }) +} + +func (e *provider) sampleAlbumTracks(ctx context.Context, albumID string, n int) (model.MediaFiles, error) { + return e.sampleTracks(ctx, squirrel.Eq{"album_id": albumID}, n) +} + +func (e *provider) sampleArtistTracks(ctx context.Context, artistID string, n int) (model.MediaFiles, error) { + // media_file.artist_id is the deprecated primary artist, so it misses an artist credited only + // on the album, as on compilations. Same filter the artist listings use. + filter := persistence.ParticipantIDFilter("media_file", artistID, model.RoleArtist, model.RoleAlbumArtist) + return e.sampleTracks(ctx, filter, n) +} + +func (e *provider) sampleGenreTracks(ctx context.Context, genre *model.Genre, n int) (model.MediaFiles, error) { + return e.sampleTracks(ctx, persistence.SongGenres.ByID(genre.ID), n) +} + +// sampleTracks returns up to n random present tracks. Seeds can end up in the mix verbatim, so +// missing files would surface as unplayable entries. +func (e *provider) sampleTracks(ctx context.Context, filter squirrel.Sqlizer, n int) (model.MediaFiles, error) { + return e.ds.MediaFile(ctx).GetRandom(model.QueryOptions{ + Filters: squirrel.And{filter, squirrel.Eq{"missing": false}}, + Max: n, + }) +} + +// similarSongsFallback uses the original similar artists + top songs algorithm. The idea is to +// get the artist of the given entity, retrieve similar artists, get their top songs, and pick +// a weighted random selection of songs to return as similar songs. +func (e *provider) similarSongsFallback(ctx context.Context, id string, count int) (model.MediaFiles, error) { + artist, err := e.getArtist(ctx, id) + if err != nil { + return nil, err + } + + e.callGetSimilarArtists(ctx, e.ag, &artist, 15, false) + if utils.IsCtxDone(ctx) { + log.Warn(ctx, "SimilarSongs call canceled", ctx.Err()) + return nil, ctx.Err() + } + + weightedSongs := random.NewWeightedChooser[model.MediaFile]() + addArtist := func(a model.Artist, weightedSongs *random.WeightedChooser[model.MediaFile], count, artistWeight int) error { + if utils.IsCtxDone(ctx) { + log.Warn(ctx, "SimilarSongs call canceled", ctx.Err()) + return ctx.Err() + } + + topCount := max(count, 20) + topSongs, err := e.getMatchingTopSongs(ctx, e.ag, &auxArtist{Artist: a}, topCount) + if err != nil { + log.Warn(ctx, "Error getting artist's top songs", "artist", a.Name, err) + return nil + } + + weight := topCount * (4 + artistWeight) + for _, mf := range topSongs { + weightedSongs.Add(mf, weight) + weight -= 4 + } + return nil + } + + err = addArtist(artist.Artist, weightedSongs, count, 10) + if err != nil { + return nil, err + } + for _, a := range artist.SimilarArtists { + err := addArtist(a, weightedSongs, count, 0) + if err != nil { + return nil, err + } + } + + // Count distinct tracks, not picks: a collaboration sits in the chooser once per artist that + // lists it, and letting those repeats consume the budget strands unique candidates. + var similarSongs model.MediaFiles + picked := map[string]struct{}{} + for len(similarSongs) < count && weightedSongs.Size() > 0 { + s, err := weightedSongs.Pick() + if err != nil { + log.Warn(ctx, "Error getting weighted song", err) + continue + } + if _, dup := picked[s.ID]; dup { + continue + } + picked[s.ID] = struct{}{} + similarSongs = append(similarSongs, s) + } + + return similarSongs, nil +} diff --git a/core/external/provider_similarsongs_test.go b/core/external/provider_similarsongs_test.go index 5617269b3..ac54495f4 100644 --- a/core/external/provider_similarsongs_test.go +++ b/core/external/provider_similarsongs_test.go @@ -13,7 +13,6 @@ import ( "github.com/navidrome/navidrome/core/matcher" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/tests" - "github.com/navidrome/navidrome/utils/slice" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/stretchr/testify/mock" @@ -65,6 +64,18 @@ var _ = Describe("Provider - SimilarSongs", func() { provider = NewProvider(ds, agentsCombined, matcher.New(ds)) }) + // Resolves track-1 through the GetEntityByID probe order and on to its artist. Left permissive: + // no spec here asserts how many times the entity is looked up. + stubTrackEntity := func() { + track := model.MediaFile{ID: "track-1", Title: "Track", Artist: "Artist", ArtistID: "artist-1"} + artist := model.Artist{ID: "artist-1", Name: "Artist"} + artistRepo.On("Get", "track-1").Return(nil, model.ErrNotFound).Maybe() + albumRepo.On("Get", "track-1").Return(nil, model.ErrNotFound).Maybe() + mediaFileRepo.On("Get", "track-1").Return(&track, nil).Maybe() + artistRepo.On("Get", "artist-1").Return(&artist, nil).Maybe() + artistRepo.On("GetAll", mock.Anything).Return(model.Artists{artist}, nil).Maybe() + } + Describe("dispatch by entity type", func() { Context("when ID is a MediaFile (track)", func() { It("calls GetSimilarSongsByTrack and returns matched songs", func() { @@ -83,7 +94,7 @@ var _ = Describe("Provider - SimilarSongs", func() { albumRepo.On("Get", "track-1").Return(nil, model.ErrNotFound).Once() mediaFileRepo.On("Get", "track-1").Return(&track, nil).Once() - agentsCombined.On("GetSimilarSongsByTrack", mock.Anything, "track-1", "Just Can't Get Enough", "Depeche Mode", "track-mbid", 5). + agentsCombined.On("GetSimilarSongsByTrack", mock.Anything, "track-1", "Just Can't Get Enough", "Depeche Mode", "track-mbid", 1). Return([]agents.Song{ {Name: "Dreaming of Me", MBID: "", Artists: []agents.Artist{{Name: "Depeche Mode", MBID: "artist-mbid"}}}, }, nil).Once() @@ -126,7 +137,7 @@ var _ = Describe("Provider - SimilarSongs", func() { return false })).Return(model.MediaFiles{matchedSong}, nil).Maybe() - songs, err := provider.SimilarSongs(ctx, "track-1", 5) + songs, err := provider.SimilarSongs(ctx, "track-1", 1) Expect(err).ToNot(HaveOccurred()) Expect(songs).To(HaveLen(1)) @@ -176,6 +187,76 @@ var _ = Describe("Provider - SimilarSongs", func() { Expect(songs).To(HaveLen(1)) Expect(songs[0].ID).To(Equal("song-1")) }) + + It("tops the mix up with the fallback when the agent's picks alone are too few", func() { + stubTrackEntity() + + // The agent knows one track of the three asked for. + agentsCombined.On("GetSimilarSongsByTrack", mock.Anything, "track-1", "Track", "Artist", "", 3). + Return([]agents.Song{{Name: "Agent Pick", MBID: "mbid-agent"}}, nil).Once() + mediaFileRepo.On("GetAll", mock.Anything). + Return(model.MediaFiles{{ID: "agent-1", Title: "Agent Pick", MbzRecordingID: "mbid-agent"}}, nil).Once() + + mockAgent.On("GetSimilarArtists", mock.Anything, "artist-1", "Artist", "", 15). + Return([]agents.Artist{}, nil).Once() + mockAgent.On("GetArtistTopSongs", mock.Anything, "artist-1", "Artist", "", mock.Anything). + Return([]agents.Song{{Name: "Song One", MBID: "mbid-1"}}, nil).Once() + mediaFileRepo.On("GetAll", mock.Anything). + Return(model.MediaFiles{{ID: "song-1", Title: "Song One", MbzRecordingID: "mbid-1"}}, nil).Once() + + songs, err := provider.SimilarSongs(ctx, "track-1", 3) + + Expect(err).ToNot(HaveOccurred()) + Expect(ids(songs)).To(ConsistOf("agent-1", "song-1")) + }) + + It("reaches a unique pick that sits past the count-th repeat", func() { + stubTrackEntity() + mockAgent.On("GetSimilarArtists", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return([]agents.Artist{}, nil).Maybe() + mockAgent.On("GetArtistTopSongs", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return([]agents.Song{}, nil).Maybe() + + // "Song B" sits past the second repeat, so a matcher capped at count never reaches it. + repeated := agents.Song{Name: "Song A", MBID: "mbid-a"} + agentsCombined.On("GetSimilarSongsByTrack", mock.Anything, "track-1", "Track", "Artist", "", 2). + Return([]agents.Song{repeated, repeated, {Name: "Song B", MBID: "mbid-b"}}, nil).Once() + mediaFileRepo.On("GetAll", mock.Anything).Return(model.MediaFiles{ + {ID: "t-a", Title: "Song A", MbzRecordingID: "mbid-a"}, + {ID: "t-b", Title: "Song B", MbzRecordingID: "mbid-b"}, + }, nil).Once() + + songs, err := provider.SimilarSongs(ctx, "track-1", 2) + + Expect(err).ToNot(HaveOccurred()) + Expect(ids(songs)).To(ConsistOf("t-a", "t-b")) + }) + + It("keeps topping up when the agent's picks repeat a track", func() { + stubTrackEntity() + + // The matcher re-emits a track when the same input song repeats, so these three + // picks resolve to only two distinct library tracks. + repeated := agents.Song{Name: "Song A", MBID: "mbid-a"} + agentsCombined.On("GetSimilarSongsByTrack", mock.Anything, "track-1", "Track", "Artist", "", 3). + Return([]agents.Song{repeated, repeated, {Name: "Song B", MBID: "mbid-b"}}, nil).Once() + mediaFileRepo.On("GetAll", mock.Anything).Return(model.MediaFiles{ + {ID: "t-a", Title: "Song A", MbzRecordingID: "mbid-a"}, + {ID: "t-b", Title: "Song B", MbzRecordingID: "mbid-b"}, + }, nil).Once() + + mockAgent.On("GetSimilarArtists", mock.Anything, "artist-1", "Artist", "", 15). + Return([]agents.Artist{}, nil).Once() + mockAgent.On("GetArtistTopSongs", mock.Anything, "artist-1", "Artist", "", mock.Anything). + Return([]agents.Song{{Name: "Song C", MBID: "mbid-c"}}, nil).Once() + mediaFileRepo.On("GetAll", mock.Anything). + Return(model.MediaFiles{{ID: "t-c", Title: "Song C", MbzRecordingID: "mbid-c"}}, nil).Once() + + songs, err := provider.SimilarSongs(ctx, "track-1", 3) + + Expect(err).ToNot(HaveOccurred()) + Expect(ids(songs)).To(ConsistOf("t-a", "t-b", "t-c")) + }) }) Context("when ID is an Album", func() { @@ -187,7 +268,7 @@ var _ = Describe("Provider - SimilarSongs", func() { artistRepo.On("Get", "album-1").Return(nil, model.ErrNotFound).Once() albumRepo.On("Get", "album-1").Return(&album, nil).Once() - agentsCombined.On("GetSimilarSongsByAlbum", mock.Anything, "album-1", "Speak & Spell", "Depeche Mode", "album-mbid", 5). + agentsCombined.On("GetSimilarSongsByAlbum", mock.Anything, "album-1", "Speak & Spell", "Depeche Mode", "album-mbid", 1). Return([]agents.Song{ {Name: "New Life", MBID: "song-mbid", Artists: []agents.Artist{{Name: "Depeche Mode"}}}, }, nil).Once() @@ -208,7 +289,7 @@ var _ = Describe("Provider - SimilarSongs", func() { return hasEq })).Return(model.MediaFiles{matchedSong}, nil).Once() - songs, err := provider.SimilarSongs(ctx, "album-1", 5) + songs, err := provider.SimilarSongs(ctx, "album-1", 1) Expect(err).ToNot(HaveOccurred()) Expect(songs).To(HaveLen(1)) @@ -286,7 +367,7 @@ var _ = Describe("Provider - SimilarSongs", func() { matchedSong := model.MediaFile{ID: "matched-1", Title: "Enjoy the Silence", Artist: "Depeche Mode", MbzRecordingID: "song-mbid"} artistRepo.On("Get", "artist-1").Return(&artist, nil).Once() - agentsCombined.On("GetSimilarSongsByArtist", mock.Anything, "artist-1", "Depeche Mode", "artist-mbid", 5). + agentsCombined.On("GetSimilarSongsByArtist", mock.Anything, "artist-1", "Depeche Mode", "artist-mbid", 1). Return([]agents.Song{ {Name: "Enjoy the Silence", MBID: "song-mbid", Artists: []agents.Artist{{Name: "Depeche Mode"}}}, }, nil).Once() @@ -307,7 +388,7 @@ var _ = Describe("Provider - SimilarSongs", func() { return hasEq })).Return(model.MediaFiles{matchedSong}, nil).Once() - songs, err := provider.SimilarSongs(ctx, "artist-1", 5) + songs, err := provider.SimilarSongs(ctx, "artist-1", 1) Expect(err).ToNot(HaveOccurred()) Expect(songs).To(HaveLen(1)) @@ -348,6 +429,72 @@ var _ = Describe("Provider - SimilarSongs", func() { }) }) + Context("when ID is an Artist and the similar-artists fallback can't fill the mix", func() { + It("tops the mix up with the artist's own track-similars", func() { + artist := model.Artist{ID: "ar-1", Name: "Thin Artist"} + topSong := model.MediaFile{ID: "top-1", Title: "Top Song", ArtistID: "ar-1", MbzRecordingID: "mbid-top"} + + artistRepo.On("Get", "ar-1").Return(&artist, nil).Maybe() + artistRepo.On("GetAll", mock.Anything).Return(model.Artists{artist}, nil).Maybe() + + agentsCombined.On("GetSimilarSongsByArtist", mock.Anything, "ar-1", "Thin Artist", "", 5). + Return([]agents.Song{}, nil).Once() + + // No similar artist is in the library, so the fallback yields only the seed artist's + // own matching top song: one track for a mix of five. + mockAgent.On("GetSimilarArtists", mock.Anything, "ar-1", "Thin Artist", "", 15). + Return([]agents.Artist{}, nil).Once() + mockAgent.On("GetArtistTopSongs", mock.Anything, "ar-1", "Thin Artist", "", mock.Anything). + Return([]agents.Song{{Name: "Top Song", MBID: "mbid-top"}}, nil).Once() + mediaFileRepo.On("GetAll", mock.Anything).Return(model.MediaFiles{topSong}, nil).Once() + + mediaFileRepo.On("GetRandom", mock.Anything). + Return(model.MediaFiles{{ID: "s1", Title: "Seed", Artist: "Thin Artist"}}, nil).Once() + agentsCombined.On("GetSimilarSongsByTrack", mock.Anything, "s1", "Seed", "Thin Artist", "", mock.Anything). + Return([]agents.Song{{Name: "Mix Song", MBID: "mbid-mix"}}, nil).Once() + mediaFileRepo.On("GetAll", mock.Anything). + Return(model.MediaFiles{{ID: "mix-1", Title: "Mix Song", MbzRecordingID: "mbid-mix"}}, nil).Once() + + songs, err := provider.SimilarSongs(ctx, "ar-1", 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(ids(songs)).To(ConsistOf("top-1", "mix-1")) + }) + }) + + Context("when ID is an Artist and the agent plus the similar-artists fallback already fill the mix", func() { + It("does not pay for seed-track sampling", func() { + artist := model.Artist{ID: "ar-1", Name: "The Artist"} + artistRepo.On("Get", "ar-1").Return(&artist, nil).Maybe() + artistRepo.On("GetAll", mock.Anything).Return(model.Artists{artist}, nil).Maybe() + + agentsCombined.On("GetSimilarSongsByArtist", mock.Anything, "ar-1", "The Artist", "", 3). + Return([]agents.Song{{Name: "A", MBID: "mbid-a"}, {Name: "B", MBID: "mbid-b"}}, nil).Once() + mediaFileRepo.On("GetAll", mock.Anything).Return(model.MediaFiles{ + {ID: "t-a", Title: "A", MbzRecordingID: "mbid-a"}, + {ID: "t-b", Title: "B", MbzRecordingID: "mbid-b"}, + }, nil).Once() + + // The similar-artists fallback supplies the third track, so the mix is full. + mockAgent.On("GetSimilarArtists", mock.Anything, "ar-1", "The Artist", "", 15). + Return([]agents.Artist{}, nil).Once() + mockAgent.On("GetArtistTopSongs", mock.Anything, "ar-1", "The Artist", "", mock.Anything). + Return([]agents.Song{{Name: "C", MBID: "mbid-c"}}, nil).Once() + mediaFileRepo.On("GetAll", mock.Anything). + Return(model.MediaFiles{{ID: "t-c", Title: "C", MbzRecordingID: "mbid-c"}}, nil).Once() + + mediaFileRepo.On("GetRandom", mock.Anything).Return(model.MediaFiles{{ID: "seed"}}, nil).Maybe() + agentsCombined.On("GetSimilarSongsByTrack", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return([]agents.Song{}, nil).Maybe() + + songs, err := provider.SimilarSongs(ctx, "ar-1", 3) + + Expect(err).ToNot(HaveOccurred()) + Expect(ids(songs)).To(ConsistOf("t-a", "t-b", "t-c")) + mediaFileRepo.AssertNotCalled(GinkgoT(), "GetRandom", mock.Anything) + }) + }) + Context("when ID is a Playlist", func() { It("samples playlist tracks and returns their track-similars", func() { pls := model.Playlist{ID: "pl-1", Name: "My List"} @@ -553,7 +700,7 @@ var _ = Describe("Provider - SimilarSongs", func() { Expect(err).ToNot(HaveOccurred()) Expect(songs).To(HaveLen(3)) - ids := slice.Map(songs, func(mf model.MediaFile) string { return mf.ID }) + ids := ids(songs) Expect(ids).To(ContainElement(BeElementOf("b1", "b2")), "seed two must be represented in the mix") }) @@ -750,6 +897,60 @@ var _ = Describe("Provider - SimilarSongs", func() { } }) + It("keeps picking until the fallback holds count distinct tracks", func() { + // A collaboration in two artists' top songs lands in the chooser twice. Picking a fixed + // count of entries lets those duplicates eat the budget and strand unique candidates. + track := model.MediaFile{ID: "track-1", Title: "Track", Artist: "Artist One", ArtistID: "artist-1"} + artist1 := model.Artist{ID: "artist-1", Name: "Artist One"} + similarArtist := model.Artist{ID: "artist-3", Name: "Similar Artist"} + + artistRepo.On("Get", "track-1").Return(nil, model.ErrNotFound).Twice() + albumRepo.On("Get", "track-1").Return(nil, model.ErrNotFound).Twice() + mediaFileRepo.On("Get", "track-1").Return(&track, nil).Twice() + artistRepo.On("Get", "artist-1").Return(&artist1, nil).Maybe() + artistRepo.On("Get", "artist-3").Return(&similarArtist, nil).Maybe() + artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + return opt.Max == 1 && opt.Filters != nil + })).Return(model.Artists{artist1}, nil).Maybe() + + agentsCombined.On("GetSimilarSongsByTrack", mock.Anything, "track-1", "Track", "Artist One", "", 3). + Return([]agents.Song{}, nil).Once() + + mockAgent.On("GetSimilarArtists", mock.Anything, "artist-1", "Artist One", "", 15). + Return([]agents.Artist{{Name: "Similar Artist"}}, nil).Once() + artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + _, ok := opt.Filters.(squirrel.Eq) + return opt.Max == 0 && ok + })).Return(model.Artists{}, nil).Once() + artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + _, ok := opt.Filters.(squirrel.Or) + return opt.Max == 0 && ok + })).Return(model.Artists{similarArtist}, nil).Once() + + shared := model.MediaFile{ID: "t-a", Title: "Shared", MbzRecordingID: "mbid-a"} + mockAgent.On("GetArtistTopSongs", mock.Anything, "artist-1", "Artist One", "", mock.Anything). + Return([]agents.Song{{Name: "Shared", MBID: "mbid-a"}}, nil).Once() + mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")). + Return(model.MediaFiles{shared}, nil).Once() + + mockAgent.On("GetArtistTopSongs", mock.Anything, "artist-3", "Similar Artist", "", mock.Anything). + Return([]agents.Song{ + {Name: "Shared", MBID: "mbid-a"}, + {Name: "B", MBID: "mbid-b"}, + {Name: "C", MBID: "mbid-c"}, + }, nil).Once() + mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{ + shared, + {ID: "t-b", Title: "B", MbzRecordingID: "mbid-b"}, + {ID: "t-c", Title: "C", MbzRecordingID: "mbid-c"}, + }, nil).Once() + + songs, err := provider.SimilarSongs(ctx, "track-1", 3) + + Expect(err).ToNot(HaveOccurred()) + Expect(ids(songs)).To(ConsistOf("t-a", "t-b", "t-c")) + }) + It("returns ErrNotFound when artist is not found", func() { artistRepo.On("Get", "artist-unknown-artist").Return(nil, model.ErrNotFound) mediaFileRepo.On("Get", "artist-unknown-artist").Return(nil, model.ErrNotFound) @@ -792,7 +993,7 @@ var _ = Describe("Provider - SimilarSongs", func() { mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song1}, nil).Once() - songs, err := provider.SimilarSongs(ctx, "artist-1", 5) + songs, err := provider.SimilarSongs(ctx, "artist-1", 1) Expect(err).ToNot(HaveOccurred()) Expect(songs).To(HaveLen(1))