fix(cli): count playlists in the artwork reprocess external estimate

The estimate used WalksPriorityChain, which is true only for artist and album,
so a playlist-only reprocess reported "External lookups: none" and the
confirmation prompt dropped the external-cost warning. Playlists do reach the
network: through the m3u ExternalImageURL fetch when EnableM3UExternalAlbumArt
is on, and — verified by test — through the generated grid, whose tiles resolve
album art via the full album priority chain.

Adds artwork.MayFetchExternal, a config-aware predicate for "can this kind's
resolver reach the network", and uses it for the estimate. WalksPriorityChain
keeps its separate job of deciding whether explain prints a chain block.
This commit is contained in:
Deluan 2026-08-14 16:31:37 -04:00
commit 08160f3cc0
4 changed files with 125 additions and 6 deletions

View file

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

View file

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

View file

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

View file

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