nik/navidrome
1
0
Fork 0
mirror of https://github.com/navidrome/navidrome.git synced 2026-10-18 07:30:29 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge branch 'master' into feat-podcast-ux-improvements/5420

Browse Source
This commit is contained in:
Jiho Persy Lee 2026-08-14 16:22:05 +09:00 committed by GitHub
commit 778601470f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 702 additions and 412 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
core/agents/interfaces.go
View File

@ -53,6 +53,8 @@ func (s Song) Equals(other Song) bool {
}
var (
// ErrNotFound means the provider answered and had nothing. Return the underlying error
// for a fault instead, or callers that back off on faults will treat it as definitive.
ErrNotFound = errors.New("not found")
)

8
core/artwork/gate.go
View File

@ -1,6 +1,7 @@
package artwork
import (
"context"
"errors"
"io"
"sync"
@ -101,10 +102,13 @@ func (b *breaker) allow() bool {
}
func (b *breaker) record(name string, err error) {
// A cancelled run says nothing about the provider, so it neither counts nor clears.
if errors.Is(err, context.Canceled) {
return
}
b.mu.Lock()
defer b.mu.Unlock()
// A not-found is a definitive answer, not a fault; keep in sync with isTransientExternal.
if err == nil || errors.Is(err, model.ErrNotFound) || errors.Is(err, agents.ErrNotFound) {
if !isTransientExternal(err) {
if b.failures >= breakerThreshold {
log.Info("Artwork: Circuit breaker closed for agent", "agent", name)
}

37
core/artwork/worker_test.go
View File

@ -539,6 +539,43 @@ var _ = Describe("Worker", func() {
Expect(calls).To(Equal(5), "the breaker should have re-closed after the success")
})
It("does not open the breaker when the run is cancelled", func() {
cancelled := func() (io.ReadCloser, string, error) { return nil, "", context.Canceled }
for range breakerThreshold + 3 {
_, _, err := w.gate("A", cancelled)
Expect(err).To(MatchError(context.Canceled), "a cancellation passes through, never errBreakerOpen")
}
var calls int
counting := func() (io.ReadCloser, string, error) {
calls++
return nil, "", errors.New("boom")
}
_, _, _ = w.gate("A", counting)
Expect(calls).To(Equal(1), "the breaker stayed closed, so the step still runs")
})
It("ignores a cancellation mid-run, neither counting nor clearing the failures", func() {
failing := func() (io.ReadCloser, string, error) { return nil, "", errors.New("boom") }
cancelled := func() (io.ReadCloser, string, error) { return nil, "", context.Canceled }
for range breakerThreshold - 1 {
_, _, _ = w.gate("A", failing)
}
_, _, _ = w.gate("A", cancelled)
var calls int
counting := func() (io.ReadCloser, string, error) {
calls++
return nil, "", errors.New("boom")
}
_, _, _ = w.gate("A", counting)
Expect(calls).To(Equal(1), "the cancellation must not have counted as the final failure")
_, _, err := w.gate("A", counting)
Expect(err).To(MatchError(errBreakerOpen), "the cancellation must not have cleared the earlier failures")
Expect(calls).To(Equal(1), "an open breaker must not call the external step")
})
It("does not open the breaker on a run of agent not-found misses", func() {
// agents.ErrNotFound is a definitive miss, not a fault: artless items must not
// trip the breaker, or they would loop in retry instead of settling absent.

5
core/external/extdata_helper_test.go vendored
View File

@ -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 })
}

255
core/external/provider.go vendored
View File

@ -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 {

299
core/external/provider_similarsongs.go vendored Normal file
View File

@ -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
}

219
core/external/provider_similarsongs_test.go vendored
View File

@ -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))

27
plugins/metadata_agent.go
View File

@ -50,6 +50,15 @@ func newMetadataAgent(p *plugin) *MetadataAgent {
return &MetadataAgent{name: p.name, plugin: p}
}
// agentErr keeps a plugin fault distinguishable from a definitive miss: a method the plugin
// simply does not implement has answered, so it must not count against a caller's back-off.
func agentErr(err error) error {
if errors.Is(err, errNotImplemented) || errors.Is(err, errFunctionNotFound) {
return errors.Join(agents.ErrNotFound, err)
}
return err
}
// MetadataAgent is an adapter that wraps an Extism plugin and implements
// the agents interfaces for metadata retrieval.
type MetadataAgent struct {
@ -69,7 +78,7 @@ func (a *MetadataAgent) GetArtistMBID(ctx context.Context, id string, name strin
input := capabilities.ArtistMBIDRequest{ID: id, Name: name}
result, err := callPluginFunction[capabilities.ArtistMBIDRequest, *capabilities.ArtistMBIDResponse](ctx, a.plugin, FuncGetArtistMBID, input)
if err != nil {
return "", errors.Join(agents.ErrNotFound, err)
return "", agentErr(err)
}
if result == nil || result.MBID == "" {
@ -84,7 +93,7 @@ func (a *MetadataAgent) GetArtistURL(ctx context.Context, id, name, mbid string)
input := capabilities.ArtistRequest{ID: id, Name: name, MBID: mbid}
result, err := callPluginFunction[capabilities.ArtistRequest, *capabilities.ArtistURLResponse](ctx, a.plugin, FuncGetArtistURL, input)
if err != nil {
return "", errors.Join(agents.ErrNotFound, err)
return "", agentErr(err)
}
if result == nil || result.URL == "" {
return "", agents.ErrNotFound
@ -97,7 +106,7 @@ func (a *MetadataAgent) GetArtistBiography(ctx context.Context, id, name, mbid s
input := capabilities.ArtistRequest{ID: id, Name: name, MBID: mbid}
result, err := callPluginFunction[capabilities.ArtistRequest, *capabilities.ArtistBiographyResponse](ctx, a.plugin, FuncGetArtistBiography, input)
if err != nil {
return "", errors.Join(agents.ErrNotFound, err)
return "", agentErr(err)
}
if result == nil || result.Biography == "" {
@ -112,7 +121,7 @@ func (a *MetadataAgent) GetSimilarArtists(ctx context.Context, id, name, mbid st
input := capabilities.SimilarArtistsRequest{ID: id, Name: name, MBID: mbid, Limit: int32(limit)}
result, err := callPluginFunction[capabilities.SimilarArtistsRequest, *capabilities.SimilarArtistsResponse](ctx, a.plugin, FuncGetSimilarArtists, input)
if err != nil {
return nil, errors.Join(agents.ErrNotFound, err)
return nil, agentErr(err)
}
if result == nil || len(result.Artists) == 0 {
@ -132,7 +141,7 @@ func (a *MetadataAgent) GetArtistImages(ctx context.Context, id, name, mbid stri
input := capabilities.ArtistRequest{ID: id, Name: name, MBID: mbid}
result, err := callPluginFunction[capabilities.ArtistRequest, *capabilities.ArtistImagesResponse](ctx, a.plugin, FuncGetArtistImages, input)
if err != nil {
return nil, errors.Join(agents.ErrNotFound, err)
return nil, agentErr(err)
}
if result == nil || len(result.Images) == 0 {
@ -152,7 +161,7 @@ func (a *MetadataAgent) GetArtistTopSongs(ctx context.Context, id, artistName, m
input := capabilities.TopSongsRequest{ID: id, Name: artistName, MBID: mbid, Count: int32(count)}
result, err := callPluginFunction[capabilities.TopSongsRequest, *capabilities.TopSongsResponse](ctx, a.plugin, FuncGetArtistTopSongs, input)
if err != nil {
return nil, errors.Join(agents.ErrNotFound, err)
return nil, agentErr(err)
}
if result == nil || len(result.Songs) == 0 {
@ -167,7 +176,7 @@ func (a *MetadataAgent) GetAlbumInfo(ctx context.Context, name, artist, mbid str
input := capabilities.AlbumRequest{Name: name, Artist: artist, MBID: mbid}
result, err := callPluginFunction[capabilities.AlbumRequest, *capabilities.AlbumInfoResponse](ctx, a.plugin, FuncGetAlbumInfo, input)
if err != nil {
return nil, errors.Join(agents.ErrNotFound, err)
return nil, agentErr(err)
}
if result == nil {
@ -187,7 +196,7 @@ func (a *MetadataAgent) GetAlbumImages(ctx context.Context, name, artist, mbid s
input := capabilities.AlbumRequest{Name: name, Artist: artist, MBID: mbid}
result, err := callPluginFunction[capabilities.AlbumRequest, *capabilities.AlbumImagesResponse](ctx, a.plugin, FuncGetAlbumImages, input)
if err != nil {
return nil, errors.Join(agents.ErrNotFound, err)
return nil, agentErr(err)
}
if result == nil || len(result.Images) == 0 {
@ -205,7 +214,7 @@ func (a *MetadataAgent) GetAlbumImages(ctx context.Context, name, artist, mbid s
func callSimilarSongsPluginFunction[T any](ctx context.Context, plugin *plugin, funcName string, input T) ([]agents.Song, error) {
result, err := callPluginFunction[T, *capabilities.SimilarSongsResponse](ctx, plugin, funcName, input)
if err != nil {
return nil, err
return nil, agentErr(err)
}
if result == nil || len(result.Songs) == 0 {
return nil, agents.ErrNotFound

262
plugins/metadata_agent_test.go
View File

@ -3,12 +3,36 @@
package plugins
import (
"errors"
"fmt"
"github.com/navidrome/navidrome/core/agents"
"github.com/navidrome/navidrome/plugins/capabilities"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
// The partial-metadata-agent fixture registers through the Go PDK, which exports every method
// and answers -2, so only errNotImplemented reaches agentErr through a real plugin. A plugin
// that omits the export entirely yields errFunctionNotFound, covered here directly.
var _ = Describe("agentErr", func() {
DescribeTable("classifies a plugin error as a miss or a fault",
func(err error, wantMiss bool) {
got := agentErr(err)
Expect(errors.Is(got, agents.ErrNotFound)).To(Equal(wantMiss))
Expect(got).To(MatchError(err), "the underlying reason must survive for diagnostics")
},
Entry("an unimplemented method is a miss",
fmt.Errorf("%w: %s", errNotImplemented, FuncGetArtistImages), true),
Entry("a missing export is a miss",
fmt.Errorf("%w: %s", errFunctionNotFound, FuncGetArtistImages), true),
Entry("a call failure is a fault",
fmt.Errorf("plugin call failed: %w", errors.New("returned status 429")), false),
Entry("a non-zero exit is a fault",
errors.New("plugin call exited with code 1"), false),
)
})
var _ = Describe("MetadataAgent", Ordered, func() {
var agent agents.Interface
@ -166,82 +190,57 @@ var _ = Describe("MetadataAgent error handling", Ordered, func() {
Expect(ok).To(BeTrue())
})
It("returns error from GetArtistMBID", func() {
retriever := errorAgent.(agents.ArtistMBIDRetriever)
_, err := retriever.GetArtistMBID(GinkgoT().Context(), "artist-1", "Test")
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("simulated plugin error"))
})
It("returns error from GetArtistURL", func() {
retriever := errorAgent.(agents.ArtistURLRetriever)
_, err := retriever.GetArtistURL(GinkgoT().Context(), "artist-1", "Test", "mbid")
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("simulated plugin error"))
})
It("returns error from GetArtistBiography", func() {
retriever := errorAgent.(agents.ArtistBiographyRetriever)
_, err := retriever.GetArtistBiography(GinkgoT().Context(), "artist-1", "Test", "mbid")
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("simulated plugin error"))
})
It("returns error from GetArtistImages", func() {
retriever := errorAgent.(agents.ArtistImageRetriever)
_, err := retriever.GetArtistImages(GinkgoT().Context(), "artist-1", "Test", "mbid")
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("simulated plugin error"))
})
It("returns error from GetSimilarArtists", func() {
retriever := errorAgent.(agents.ArtistSimilarRetriever)
_, err := retriever.GetSimilarArtists(GinkgoT().Context(), "artist-1", "Test", "mbid", 5)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("simulated plugin error"))
})
It("returns error from GetArtistTopSongs", func() {
retriever := errorAgent.(agents.ArtistTopSongsRetriever)
_, err := retriever.GetArtistTopSongs(GinkgoT().Context(), "artist-1", "Test", "mbid", 5)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("simulated plugin error"))
})
It("returns error from GetAlbumInfo", func() {
retriever := errorAgent.(agents.AlbumInfoRetriever)
_, err := retriever.GetAlbumInfo(GinkgoT().Context(), "Album", "Artist", "mbid")
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("simulated plugin error"))
})
It("returns error from GetAlbumImages", func() {
retriever := errorAgent.(agents.AlbumImageRetriever)
_, err := retriever.GetAlbumImages(GinkgoT().Context(), "Album", "Artist", "mbid")
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("simulated plugin error"))
})
It("returns error from GetSimilarSongsByTrack", func() {
retriever := errorAgent.(agents.SimilarSongsByTrackRetriever)
_, err := retriever.GetSimilarSongsByTrack(GinkgoT().Context(), "track-1", "Test", "Artist", "mbid", 5)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("simulated plugin error"))
})
It("returns error from GetSimilarSongsByAlbum", func() {
retriever := errorAgent.(agents.SimilarSongsByAlbumRetriever)
_, err := retriever.GetSimilarSongsByAlbum(GinkgoT().Context(), "album-1", "Album", "Artist", "mbid", 5)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("simulated plugin error"))
})
It("returns error from GetSimilarSongsByArtist", func() {
retriever := errorAgent.(agents.SimilarSongsByArtistRetriever)
_, err := retriever.GetSimilarSongsByArtist(GinkgoT().Context(), "artist-1", "Artist", "mbid", 5)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("simulated plugin error"))
})
DescribeTable("surfaces the plugin failure, not a definitive not-found",
func(call func() error) {
err := call()
Expect(err).To(MatchError(ContainSubstring("simulated plugin error")))
Expect(err).ToNot(MatchError(agents.ErrNotFound))
},
Entry("GetArtistMBID", func() error {
_, err := errorAgent.(agents.ArtistMBIDRetriever).GetArtistMBID(GinkgoT().Context(), "artist-1", "Test")
return err
}),
Entry("GetArtistURL", func() error {
_, err := errorAgent.(agents.ArtistURLRetriever).GetArtistURL(GinkgoT().Context(), "artist-1", "Test", "mbid")
return err
}),
Entry("GetArtistBiography", func() error {
_, err := errorAgent.(agents.ArtistBiographyRetriever).GetArtistBiography(GinkgoT().Context(), "artist-1", "Test", "mbid")
return err
}),
Entry("GetSimilarArtists", func() error {
_, err := errorAgent.(agents.ArtistSimilarRetriever).GetSimilarArtists(GinkgoT().Context(), "artist-1", "Test", "mbid", 5)
return err
}),
Entry("GetArtistImages", func() error {
_, err := errorAgent.(agents.ArtistImageRetriever).GetArtistImages(GinkgoT().Context(), "artist-1", "Test", "mbid")
return err
}),
Entry("GetArtistTopSongs", func() error {
_, err := errorAgent.(agents.ArtistTopSongsRetriever).GetArtistTopSongs(GinkgoT().Context(), "artist-1", "Test", "mbid", 5)
return err
}),
Entry("GetAlbumInfo", func() error {
_, err := errorAgent.(agents.AlbumInfoRetriever).GetAlbumInfo(GinkgoT().Context(), "Album", "Artist", "mbid")
return err
}),
Entry("GetAlbumImages", func() error {
_, err := errorAgent.(agents.AlbumImageRetriever).GetAlbumImages(GinkgoT().Context(), "Album", "Artist", "mbid")
return err
}),
Entry("GetSimilarSongsByTrack", func() error {
_, err := errorAgent.(agents.SimilarSongsByTrackRetriever).GetSimilarSongsByTrack(GinkgoT().Context(), "track-1", "Test", "Artist", "mbid", 5)
return err
}),
Entry("GetSimilarSongsByAlbum", func() error {
_, err := errorAgent.(agents.SimilarSongsByAlbumRetriever).GetSimilarSongsByAlbum(GinkgoT().Context(), "album-1", "Album", "Artist", "mbid", 5)
return err
}),
Entry("GetSimilarSongsByArtist", func() error {
_, err := errorAgent.(agents.SimilarSongsByArtistRetriever).GetSimilarSongsByArtist(GinkgoT().Context(), "artist-1", "Artist", "mbid", 5)
return err
}),
)
})
var _ = Describe("MetadataAgent partial implementation", Ordered, func() {
@ -268,68 +267,55 @@ var _ = Describe("MetadataAgent partial implementation", Ordered, func() {
Expect(bio).To(Equal("Partial agent biography for Test Artist"))
})
It("returns ErrNotFound for unimplemented method (GetArtistMBID)", func() {
retriever := partialAgent.(agents.ArtistMBIDRetriever)
_, err := retriever.GetArtistMBID(GinkgoT().Context(), "artist-1", "Test Artist")
Expect(err).To(MatchError(errNotImplemented))
})
It("returns ErrNotFound for unimplemented method (GetArtistURL)", func() {
retriever := partialAgent.(agents.ArtistURLRetriever)
_, err := retriever.GetArtistURL(GinkgoT().Context(), "artist-1", "Test Artist", "mbid")
Expect(err).To(MatchError(errNotImplemented))
})
It("returns ErrNotFound for unimplemented method (GetArtistImages)", func() {
retriever := partialAgent.(agents.ArtistImageRetriever)
_, err := retriever.GetArtistImages(GinkgoT().Context(), "artist-1", "Test Artist", "mbid")
Expect(err).To(MatchError(errNotImplemented))
})
It("returns ErrNotFound for unimplemented method (GetSimilarArtists)", func() {
retriever := partialAgent.(agents.ArtistSimilarRetriever)
_, err := retriever.GetSimilarArtists(GinkgoT().Context(), "artist-1", "Test Artist", "mbid", 5)
Expect(err).To(MatchError(errNotImplemented))
})
It("returns ErrNotFound for unimplemented method (GetArtistTopSongs)", func() {
retriever := partialAgent.(agents.ArtistTopSongsRetriever)
_, err := retriever.GetArtistTopSongs(GinkgoT().Context(), "artist-1", "Test Artist", "mbid", 5)
Expect(err).To(MatchError(errNotImplemented))
})
It("returns ErrNotFound for unimplemented method (GetAlbumInfo)", func() {
retriever := partialAgent.(agents.AlbumInfoRetriever)
_, err := retriever.GetAlbumInfo(GinkgoT().Context(), "Album", "Artist", "mbid")
Expect(err).To(MatchError(errNotImplemented))
})
It("returns ErrNotFound for unimplemented method (GetAlbumImages)", func() {
retriever := partialAgent.(agents.AlbumImageRetriever)
_, err := retriever.GetAlbumImages(GinkgoT().Context(), "Album", "Artist", "mbid")
Expect(err).To(MatchError(errNotImplemented))
})
It("returns ErrNotFound for unimplemented method (GetSimilarSongsByTrack)", func() {
retriever := partialAgent.(agents.SimilarSongsByTrackRetriever)
_, err := retriever.GetSimilarSongsByTrack(GinkgoT().Context(), "track-1", "Test", "Artist", "mbid", 5)
Expect(err).To(MatchError(errNotImplemented))
})
It("returns ErrNotFound for unimplemented method (GetSimilarSongsByAlbum)", func() {
retriever := partialAgent.(agents.SimilarSongsByAlbumRetriever)
_, err := retriever.GetSimilarSongsByAlbum(GinkgoT().Context(), "album-1", "Album", "Artist", "mbid", 5)
Expect(err).To(MatchError(errNotImplemented))
})
It("returns ErrNotFound for unimplemented method (GetSimilarSongsByArtist)", func() {
retriever := partialAgent.(agents.SimilarSongsByArtistRetriever)
_, err := retriever.GetSimilarSongsByArtist(GinkgoT().Context(), "artist-1", "Artist", "mbid", 5)
Expect(err).To(MatchError(errNotImplemented))
})
// An unimplemented optional method is a definitive miss. Reported as a fault it would
// count against the artwork circuit breaker and keep the item in the retry queue.
DescribeTable("reports an unimplemented method as a definitive not-found",
func(call func() error) {
err := call()
Expect(err).To(MatchError(errNotImplemented))
Expect(err).To(MatchError(agents.ErrNotFound))
},
Entry("GetArtistMBID", func() error {
_, err := partialAgent.(agents.ArtistMBIDRetriever).GetArtistMBID(GinkgoT().Context(), "artist-1", "Test Artist")
return err
}),
Entry("GetArtistURL", func() error {
_, err := partialAgent.(agents.ArtistURLRetriever).GetArtistURL(GinkgoT().Context(), "artist-1", "Test Artist", "mbid")
return err
}),
Entry("GetArtistImages", func() error {
_, err := partialAgent.(agents.ArtistImageRetriever).GetArtistImages(GinkgoT().Context(), "artist-1", "Test Artist", "mbid")
return err
}),
Entry("GetSimilarArtists", func() error {
_, err := partialAgent.(agents.ArtistSimilarRetriever).GetSimilarArtists(GinkgoT().Context(), "artist-1", "Test Artist", "mbid", 5)
return err
}),
Entry("GetArtistTopSongs", func() error {
_, err := partialAgent.(agents.ArtistTopSongsRetriever).GetArtistTopSongs(GinkgoT().Context(), "artist-1", "Test Artist", "mbid", 5)
return err
}),
Entry("GetAlbumInfo", func() error {
_, err := partialAgent.(agents.AlbumInfoRetriever).GetAlbumInfo(GinkgoT().Context(), "Album", "Artist", "mbid")
return err
}),
Entry("GetAlbumImages", func() error {
_, err := partialAgent.(agents.AlbumImageRetriever).GetAlbumImages(GinkgoT().Context(), "Album", "Artist", "mbid")
return err
}),
Entry("GetSimilarSongsByTrack", func() error {
_, err := partialAgent.(agents.SimilarSongsByTrackRetriever).GetSimilarSongsByTrack(GinkgoT().Context(), "track-1", "Test", "Artist", "mbid", 5)
return err
}),
Entry("GetSimilarSongsByAlbum", func() error {
_, err := partialAgent.(agents.SimilarSongsByAlbumRetriever).GetSimilarSongsByAlbum(GinkgoT().Context(), "album-1", "Album", "Artist", "mbid", 5)
return err
}),
Entry("GetSimilarSongsByArtist", func() error {
_, err := partialAgent.(agents.SimilarSongsByArtistRetriever).GetSimilarSongsByArtist(GinkgoT().Context(), "artist-1", "Artist", "mbid", 5)
return err
}),
)
})
var _ = Describe("songRefToAgentSong multi-artist", func() {