diff --git a/cmd/artwork.go b/cmd/artwork.go index db7d5e1a5..fd3df3465 100644 --- a/cmd/artwork.go +++ b/cmd/artwork.go @@ -394,7 +394,7 @@ func reprocessArtwork(ctx context.Context, ds model.DataStore, kinds []model.Kin } matched[i] = n total += n - if artwork.WalksPriorityChain(k) { + if artwork.MayFetchExternal(k) { external += n } } diff --git a/cmd/artwork_test.go b/cmd/artwork_test.go index b00e42ab7..85d6af9fe 100644 --- a/cmd/artwork_test.go +++ b/cmd/artwork_test.go @@ -7,6 +7,8 @@ import ( "strings" "time" + "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/conf/configtest" "github.com/navidrome/navidrome/consts" "github.com/navidrome/navidrome/core/artwork" "github.com/navidrome/navidrome/model" @@ -318,6 +320,10 @@ var _ = Describe("reprocessArtwork", func() { } BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + conf.Server.CoverArtPriority = "cover.*, external" + conf.Server.ArtistArtPriority = "artist.*, external" + conf.Server.EnableM3UExternalAlbumArt = false ds = &tests.MockDataStore{} art = ds.Artwork(ctx).(*tests.MockArtworkRepo) queue = ds.ArtworkQueue(ctx).(*tests.MockArtworkQueueRepo) @@ -408,6 +414,7 @@ var _ = Describe("reprocessArtwork", func() { }) It("says so when the selection needs no external lookup", func() { + conf.Server.CoverArtPriority = "cover.*" put(model.KindPlaylistArtwork, "pl-1", "playlist") Expect(reprocessArtwork(ctx, ds, []model.Kind{model.KindPlaylistArtwork}, nil, true, accept, &out)).To(Succeed()) @@ -415,16 +422,39 @@ var _ = Describe("reprocessArtwork", func() { Expect(out.String()).To(ContainSubstring("External lookups: none")) }) - It("counts only the kinds that call an external agent as external cost", func() { + It("counts playlists as external cost when the m3u image fetch is enabled", func() { + conf.Server.CoverArtPriority = "cover.*" + conf.Server.EnableM3UExternalAlbumArt = true put(model.KindPlaylistArtwork, "pl-1", "playlist") + var external int64 + capture := func(_ io.Writer, _, e int64) bool { external = e; return false } + + Expect(reprocessArtwork(ctx, ds, []model.Kind{model.KindPlaylistArtwork}, nil, false, capture, &out)).To(Succeed()) + + Expect(external).To(Equal(int64(1))) + Expect(out.String()).To(ContainSubstring("External lookups: up to 1")) + }) + + It("counts playlists as external cost when their grid tiles walk an external album chain", func() { + put(model.KindPlaylistArtwork, "pl-1", "playlist") + var external int64 + capture := func(_ io.Writer, _, e int64) bool { external = e; return false } + + Expect(reprocessArtwork(ctx, ds, []model.Kind{model.KindPlaylistArtwork}, nil, false, capture, &out)).To(Succeed()) + + Expect(external).To(Equal(int64(1)), "sampling album art for the grid reaches the agents") + }) + + It("counts only the kinds that call an external agent as external cost", func() { + put(model.KindRadioArtwork, "ra-1", "upload") var total, external int64 capture := func(_ io.Writer, t, e int64) bool { total, external = t, e; return false } - Expect(reprocessArtwork(ctx, ds, []model.Kind{model.KindAlbumArtwork, model.KindPlaylistArtwork}, + Expect(reprocessArtwork(ctx, ds, []model.Kind{model.KindAlbumArtwork, model.KindRadioArtwork}, nil, false, capture, &out)).To(Succeed()) Expect(total).To(Equal(int64(3))) - Expect(external).To(Equal(int64(2)), "playlist artwork never reaches an external agent") + Expect(external).To(Equal(int64(2)), "radio artwork never reaches an external agent") }) It("rejects an unknown source and names the ones in use", func() { diff --git a/core/artwork/resolve.go b/core/artwork/resolve.go index 2a36e6dfd..456287d69 100644 --- a/core/artwork/resolve.go +++ b/core/artwork/resolve.go @@ -114,8 +114,32 @@ func WalksPriorityChain(kind model.Kind) bool { return kind == model.KindArtistArtwork || kind == model.KindAlbumArtwork } -// fetchExternalAlbum and fetchExternalArtist are the only places resolution touches the network, -// so a local-only resolver is stopped here rather than at each point in the chain walk. +// MayFetchExternal reports whether resolving this kind can issue an external request under the +// current config. Playlists inherit the album chain: the generated grid resolves album art. +func MayFetchExternal(kind model.Kind) bool { + switch kind { + case model.KindArtistArtwork: + return chainFetchesExternal(conf.Server.ArtistArtPriority) + case model.KindAlbumArtwork: + return chainFetchesExternal(conf.Server.CoverArtPriority) + case model.KindPlaylistArtwork: + return conf.Server.EnableM3UExternalAlbumArt || chainFetchesExternal(conf.Server.CoverArtPriority) + default: + return false + } +} + +func chainFetchesExternal(priority string) bool { + for pattern := range strings.SplitSeq(strings.ToLower(priority), ",") { + if strings.TrimSpace(pattern) == externalCandidate { + return true + } + } + return false +} + +// Album and artist fetches stop here when the resolver is local-only, rather than at each point in +// the chain walk; resolvePlaylist gates the third network path, the m3u image URL, itself. func (r *resolver) fetchExternalAlbum(ctx context.Context, al model.Album) (io.ReadCloser, string, bool) { if r.ext == nil { return nil, "", false diff --git a/core/artwork/resolve_test.go b/core/artwork/resolve_test.go index 23b1ef436..634f7e604 100644 --- a/core/artwork/resolve_test.go +++ b/core/artwork/resolve_test.go @@ -394,6 +394,28 @@ var _ = Describe("resolveItem", func() { Entry("4 albums -> full grid", []string{"t1", "t2", "t3", "t4"}, tileSize-1), ) + // The grid samples album art through the full album chain, so a playlist reaches the + // network even with the m3u fetch off. + It("calls the album image agents for its grid tiles when m3u art is disabled", func() { + conf.Server.EnableM3UExternalAlbumArt = false + conf.Server.CoverArtPriority = "external" + folderRepo.result = nil + plRepo := tests.CreateMockPlaylistRepo() + plRepo.SetData(model.Playlists{{ID: "plgrid", Name: "Playlist"}}) + plRepo.TracksRepo = &tests.MockPlaylistTrackRepo{AlbumIDs: []string{"t1", "t2"}} + ds.MockedPlaylist = plRepo + imageAgents(&fakeImageAgent{name: "failAgent", err: errors.New("boom")}) + var gatedNames []string + gate := func(name string, f func() (io.ReadCloser, string, error)) (io.ReadCloser, string, error) { + gatedNames = append(gatedNames, name) + return f() + } + + _, err := newResolver(ds, ag, ffm, gate).resolve(ctx, model.ArtworkQueueItem{ItemKind: "pl", ItemID: "plgrid"}) + Expect(err).ToNot(HaveOccurred()) + Expect(gatedNames).To(Equal([]string{"failAgent", "failAgent"}), "one lookup per sampled album") + }) + It("resolves the uploaded image before the generated grid", func() { tmpDir := GinkgoT().TempDir() conf.Server.DataFolder = conf.NewDir(tmpDir) @@ -642,3 +664,46 @@ var _ = Describe("WalksPriorityChain", func() { Expect(WalksPriorityChain(model.KindRadioArtwork)).To(BeFalse()) }) }) + +var _ = Describe("MayFetchExternal", func() { + BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + conf.Server.CoverArtPriority = "cover.*, embedded" + conf.Server.ArtistArtPriority = "artist.*" + conf.Server.EnableM3UExternalAlbumArt = false + }) + + It("is true for the kinds whose chain includes the external candidate", func() { + conf.Server.CoverArtPriority = "cover.*, external" + conf.Server.ArtistArtPriority = "artist.*, external" + Expect(MayFetchExternal(model.KindAlbumArtwork)).To(BeTrue()) + Expect(MayFetchExternal(model.KindArtistArtwork)).To(BeTrue()) + }) + + It("is false for a chain with no external candidate", func() { + Expect(MayFetchExternal(model.KindAlbumArtwork)).To(BeFalse()) + Expect(MayFetchExternal(model.KindArtistArtwork)).To(BeFalse()) + }) + + It("is true for playlists when the m3u image fetch is enabled", func() { + conf.Server.EnableM3UExternalAlbumArt = true + Expect(MayFetchExternal(model.KindPlaylistArtwork)).To(BeTrue()) + }) + + It("is true for playlists whose grid tiles resolve through an external album chain", func() { + conf.Server.CoverArtPriority = "cover.*, external" + Expect(MayFetchExternal(model.KindPlaylistArtwork)).To(BeTrue()) + }) + + It("is false for playlists with both paths off", func() { + Expect(MayFetchExternal(model.KindPlaylistArtwork)).To(BeFalse()) + }) + + It("is false for the kinds that only read local files", func() { + conf.Server.CoverArtPriority = "external" + conf.Server.ArtistArtPriority = "external" + conf.Server.EnableM3UExternalAlbumArt = true + Expect(MayFetchExternal(model.KindRadioArtwork)).To(BeFalse()) + Expect(MayFetchExternal(model.KindMediaFileArtwork)).To(BeFalse()) + }) +})