diff --git a/core/artwork/agent_images.go b/core/artwork/agent_images.go index 95991629f..762c2edb5 100644 --- a/core/artwork/agent_images.go +++ b/core/artwork/agent_images.go @@ -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 } diff --git a/core/artwork/agent_images_test.go b/core/artwork/agent_images_test.go index 0bcd7667a..ea58fe0ac 100644 --- a/core/artwork/agent_images_test.go +++ b/core/artwork/agent_images_test.go @@ -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)}}