mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-01 07:21:17 +00:00
fix(artwork): restore synthetic-artist guard and unicode normalization in agent lookups
Moving agent calls into the worker bypassed two behaviors of the aggregate provider: Agents.GetArtistImages' guard for Unknown/Various Artists (a direct retriever call could assign an unrelated image to a synthetic artist), and auxAlbum/auxArtist.Name's DevPreserveUnicodeInExternalCalls normalization (records with typographic quotes/dashes missed exact-name searches). Re-apply both before enumerating retrievers.
This commit is contained in:
parent
50ada9ad29
commit
8d715ba2fa
@ -6,10 +6,23 @@ import (
|
||||
"io"
|
||||
"net/url"
|
||||
|
||||
"github.com/navidrome/navidrome/conf"
|
||||
"github.com/navidrome/navidrome/consts"
|
||||
"github.com/navidrome/navidrome/core/agents"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/utils/str"
|
||||
)
|
||||
|
||||
// externalName applies the DevPreserveUnicodeInExternalCalls normalization the aggregate
|
||||
// provider used, so agent searches match the same way (typographic quotes/dashes cleared
|
||||
// unless preserved).
|
||||
func externalName(name string) string {
|
||||
if conf.Server.DevPreserveUnicodeInExternalCalls {
|
||||
return name
|
||||
}
|
||||
return str.Clear(name)
|
||||
}
|
||||
|
||||
// gateFunc gates one named external fetch (rate limit + circuit breaker per name).
|
||||
// resolveItem defaults to passthroughGate; the worker injects the per-agent gate.
|
||||
type gateFunc = func(name string, f func() (io.ReadCloser, string, error)) (io.ReadCloser, string, error)
|
||||
@ -49,9 +62,16 @@ func bestImageURL(imgs []agents.ExternalImage) *url.URL {
|
||||
// Returns the winning reader + agent name; extErr is true only when NO agent succeeded and
|
||||
// at least one failed transiently (a later success beats an earlier agent error).
|
||||
func fetchArtistImage(ctx context.Context, ag *agents.Agents, gate gateFunc, ar model.Artist) (r io.ReadCloser, agentName string, extErr bool) {
|
||||
// Synthetic artists have no real external image; mirror Agents.GetArtistImages' guard so a
|
||||
// direct retriever call can't assign an unrelated result to Unknown/Various Artists.
|
||||
switch ar.ID {
|
||||
case consts.UnknownArtistID, consts.VariousArtistsID:
|
||||
return nil, "", false
|
||||
}
|
||||
name := externalName(ar.Name)
|
||||
for _, a := range ag.ArtistImageAgents() {
|
||||
reader, _, err := gate(a.Name, func() (io.ReadCloser, string, error) {
|
||||
imgs, err := a.Retriever.GetArtistImages(ctx, ar.ID, ar.Name, ar.MbzArtistID)
|
||||
imgs, err := a.Retriever.GetArtistImages(ctx, ar.ID, name, ar.MbzArtistID)
|
||||
if err != nil {
|
||||
return nil, "", err
|
||||
}
|
||||
@ -73,9 +93,10 @@ func fetchArtistImage(ctx context.Context, ag *agents.Agents, gate gateFunc, ar
|
||||
|
||||
// fetchAlbumImage is the album counterpart of fetchArtistImage.
|
||||
func fetchAlbumImage(ctx context.Context, ag *agents.Agents, gate gateFunc, al model.Album) (r io.ReadCloser, agentName string, extErr bool) {
|
||||
name, artist := externalName(al.Name), externalName(al.AlbumArtist)
|
||||
for _, a := range ag.AlbumImageAgents() {
|
||||
reader, _, err := gate(a.Name, func() (io.ReadCloser, string, error) {
|
||||
imgs, err := a.Retriever.GetAlbumImages(ctx, al.Name, al.AlbumArtist, al.MbzAlbumID)
|
||||
imgs, err := a.Retriever.GetAlbumImages(ctx, name, artist, al.MbzAlbumID)
|
||||
if err != nil {
|
||||
return nil, "", err
|
||||
}
|
||||
|
||||
@ -9,9 +9,11 @@ import (
|
||||
|
||||
"github.com/navidrome/navidrome/conf"
|
||||
"github.com/navidrome/navidrome/conf/configtest"
|
||||
"github.com/navidrome/navidrome/consts"
|
||||
"github.com/navidrome/navidrome/core/agents"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/tests"
|
||||
"github.com/navidrome/navidrome/utils/str"
|
||||
. "github.com/onsi/ginkgo/v2"
|
||||
. "github.com/onsi/gomega"
|
||||
)
|
||||
@ -19,22 +21,26 @@ import (
|
||||
// fakeImageAgent is a built-in agent stub implementing both image retrievers; it
|
||||
// records call counts so per-agent ordering and short-circuiting can be asserted.
|
||||
type fakeImageAgent struct {
|
||||
name string
|
||||
imgs []agents.ExternalImage
|
||||
err error
|
||||
artistCalls int
|
||||
albumCalls int
|
||||
name string
|
||||
imgs []agents.ExternalImage
|
||||
err error
|
||||
artistCalls int
|
||||
albumCalls int
|
||||
gotArtistName string
|
||||
gotAlbumName string
|
||||
}
|
||||
|
||||
func (f *fakeImageAgent) AgentName() string { return f.name }
|
||||
|
||||
func (f *fakeImageAgent) GetArtistImages(context.Context, string, string, string) ([]agents.ExternalImage, error) {
|
||||
func (f *fakeImageAgent) GetArtistImages(_ context.Context, _, name, _ string) ([]agents.ExternalImage, error) {
|
||||
f.artistCalls++
|
||||
f.gotArtistName = name
|
||||
return f.imgs, f.err
|
||||
}
|
||||
|
||||
func (f *fakeImageAgent) GetAlbumImages(context.Context, string, string, string) ([]agents.ExternalImage, error) {
|
||||
func (f *fakeImageAgent) GetAlbumImages(_ context.Context, name, _, _ string) ([]agents.ExternalImage, error) {
|
||||
f.albumCalls++
|
||||
f.gotAlbumName = name
|
||||
return f.imgs, f.err
|
||||
}
|
||||
|
||||
@ -109,6 +115,28 @@ var _ = Describe("agent images", func() {
|
||||
Expect(extErr).To(BeFalse())
|
||||
})
|
||||
|
||||
It("skips the external lookup for synthetic artists", func() {
|
||||
a := &fakeImageAgent{name: "agentA", imgs: []agents.ExternalImage{img("/a", 100)}}
|
||||
ag := imageAgents(a)
|
||||
|
||||
for _, id := range []string{consts.UnknownArtistID, consts.VariousArtistsID} {
|
||||
r, name, extErr := fetchArtistImage(ctx, ag, passthroughGate, model.Artist{ID: id, Name: "Various Artists"})
|
||||
Expect(r).To(BeNil())
|
||||
Expect(name).To(BeEmpty())
|
||||
Expect(extErr).To(BeFalse())
|
||||
}
|
||||
Expect(a.artistCalls).To(Equal(0), "synthetic artists never reach the agents")
|
||||
})
|
||||
|
||||
It("clears typographic characters from the query name unless preserving unicode", func() {
|
||||
conf.Server.DevPreserveUnicodeInExternalCalls = false
|
||||
a := &fakeImageAgent{name: "agentA"}
|
||||
ag := imageAgents(a)
|
||||
|
||||
_, _, _ = fetchArtistImage(ctx, ag, passthroughGate, model.Artist{ID: "ar1", Name: "AC’DC"})
|
||||
Expect(a.gotArtistName).To(Equal(str.Clear("AC’DC")))
|
||||
})
|
||||
|
||||
It("falls through to a later agent, and its success beats the earlier error", func() {
|
||||
a := &fakeImageAgent{name: "agentA", err: errBreakerOpen}
|
||||
b := &fakeImageAgent{name: "agentB", imgs: []agents.ExternalImage{img("/b", 50)}}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user