remote/navidrome
1
0
Fork 0
mirror of https://github.com/navidrome/navidrome.git synced 2026-10-25 02:37:22 +02:00
Code Issues Projects Releases Packages Wiki Activity

refactor(artwork): move the explain verdict helpers into core/artwork

Browse source
This commit is contained in:
Deluan 2026-09-03 22:48:23 -04:00
commit 3290759e62
4 changed files with 173 additions and 197 deletions
Download patch file Download diff file Expand all files Collapse all files

84
cmd/artwork.go
View file

@ -746,81 +746,8 @@ func artworkKindAndID(ctx context.Context, ds model.DataStore, arg string) (mode
return model.ArtworkID{Kind: kind, ID: arg}, nil
}
// explainAgents accounts for every configured agent: one the CLI cannot construct (a plugin, or a
// built-in missing its credentials) never reaches the Chain, so the raw list alone overstates it.
func explainAgents(configured string, available []string) string {
if strings.TrimSpace(configured) == "" {
return "(none)"
}
var unavailable bool
names := slice.Map(strings.Split(configured, ","), func(name string) string {
name = strings.TrimSpace(name)
if slices.Contains(available, name) {
return name
}
unavailable = true
return name + "*"
})
line := strings.Join(names, ", ")
if unavailable {
line += " (* not available to the CLI)"
}
return line
}
// availableImageAgents names the agents that can actually supply an image for kind.
func availableImageAgents(ds model.DataStore, mgr *plugins.Manager, kind model.Kind) []string {
ag := agents.GetAgents(ds, mgr)
if kind == model.KindArtistArtwork {
return slice.Map(ag.ArtistImageAgents(), func(a agents.ArtistImageAgent) string { return a.Name })
}
return slice.Map(ag.AlbumImageAgents(), func(a agents.AlbumImageAgent) string { return a.Name })
}
// explainResult states the verdict of the walk. A skipped or failed external tier, or a local
// candidate that would not open, leaves the outcome unknown: nothing observed that there is no artwork.
func explainResult(source string, steps []artwork.TraceStep) string {
if source != "" {
for _, s := range steps {
if s.Outcome == artwork.OutcomeHit {
break
}
// An external winner discards the earlier error, so the resolver settles it with no retry.
if s.Outcome == artwork.OutcomeError && strings.HasPrefix(s.Candidate, artwork.ExternalPrefix) &&
!strings.HasPrefix(source, artwork.ExternalPrefix) {
return "resolved from " + source +
" (indeterminate: a higher-priority external lookup failed; this may resolve differently on a retry)"
}
}
return "resolved from " + source
}
for _, s := range steps {
switch {
case s.Outcome == artwork.OutcomeError && strings.HasPrefix(s.Candidate, artwork.ExternalPrefix):
return "indeterminate (an external lookup failed; the item may resolve on a later attempt)"
// A stage error or an unreadable candidate means a source was found but not processed; the
// worker retries rather than settling absent, so neither reads as a clean miss.
case s.Outcome == artwork.OutcomeError, s.Outcome == artwork.OutcomeUnreadable:
return "indeterminate (a candidate was found but could not be processed; the worker retries rather than settling absent)"
}
}
return "not resolved"
}
// explainConfig names the setting that decides where a kind's artwork comes from, and its value.
func explainConfig(kind model.Kind) (name, value string) {
switch kind {
case model.KindArtistArtwork:
return "ArtistArtPriority", conf.Server.ArtistArtPriority
case model.KindAlbumArtwork:
return "CoverArtPriority", conf.Server.CoverArtPriority
case model.KindDiscArtwork:
return "DiscArtPriority", conf.Server.DiscArtPriority
case model.KindMediaFileArtwork:
return "EnableMediaFileCoverArt", strconv.FormatBool(conf.Server.EnableMediaFileCoverArt)
}
return "", ""
}
// cliUnavailableNote marks agents the CLI cannot construct; a running server loads them all.
const cliUnavailableNote = " (* not available to the CLI)"
type explainReport struct {
kind model.Kind
@ -912,7 +839,7 @@ func formatExplain(rep explainReport) string {
}
fmt.Fprintln(w, "\nConfig")
if setting, value := explainConfig(rep.kind); setting == "" {
if setting, value := artwork.ConfigFor(rep.kind); setting == "" {
fmt.Fprintln(w, " (no artwork source configuration applies)")
} else {
fmt.Fprintf(w, " %s:\t%s\n", setting, value)
@ -948,7 +875,7 @@ func formatExplain(rep explainReport) string {
case unrecorded:
fmt.Fprintln(w, " not evaluated (nothing recorded; re-run with --live to walk the chain now)")
default:
fmt.Fprintf(w, " %s\n", explainResult(rep.source, rep.steps))
fmt.Fprintf(w, " %s\n", artwork.Result(rep.source, rep.steps))
}
w.Flush()
@ -1002,7 +929,8 @@ func runExplain(ctx context.Context, args []string) {
if kind == model.KindArtistArtwork || kind == model.KindAlbumArtwork {
mgr := loadPluginAgents(ctx, explainLive)
defer func() { _ = mgr.Stop() }()
rep.agents = explainAgents(conf.Server.Agents, availableImageAgents(ds, mgr, kind))
rep.agents = artwork.FormatAgents(conf.Server.Agents,
artwork.ImageAgentNames(agents.GetAgents(ds, mgr), kind), cliUnavailableNote)
}
switch {
case rep.walked:

119
cmd/artwork_test.go
View file

@ -122,106 +122,6 @@ var _ = Describe("resolveArtworkTargets", func() {
})
})
var _ = Describe("explainResult", func() {
It("reports the winning source", func() {
steps := []artwork.TraceStep{{Candidate: "folder", Outcome: "hit", Detail: "/music/a.jpg"}}
Expect(explainResult("folder", steps)).To(ContainSubstring("resolved from folder"))
})
It("reports not resolved when every candidate was tried and missed", func() {
steps := []artwork.TraceStep{
{Candidate: "artist.*", Outcome: "miss"},
{Candidate: "external:deezer", Outcome: "miss"},
}
Expect(explainResult("", steps)).To(Equal("not resolved"))
})
It("reports indeterminate when a local candidate exists but could not be read", func() {
steps := []artwork.TraceStep{
{Candidate: "cover.*", Outcome: "miss"},
{Candidate: "embedded", Outcome: "unreadable"},
}
Expect(explainResult("", steps)).To(ContainSubstring("indeterminate"),
"the worker retries an unreadable candidate instead of settling absent, so this is not a clean miss")
})
It("reports indeterminate when a processing stage errored after a candidate was found", func() {
steps := []artwork.TraceStep{
{Candidate: "cover.*", Outcome: "hit", Detail: "/music/cover.jpg"},
{Candidate: "store", Outcome: "error", Detail: "disk full"},
}
Expect(explainResult("", steps)).To(ContainSubstring("indeterminate"),
"a stage error is a processing failure the worker retries, not a definitive miss")
})
It("does not qualify a hit that an earlier unreadable candidate preceded", func() {
// chainState.try stamps only the external error onto a hit and drops the local one, so the
// worker settles this as found; warning about it would be a false alarm.
steps := []artwork.TraceStep{
{Candidate: "embedded", Outcome: "unreadable"},
{Candidate: "cover.*", Outcome: "hit", Detail: "/music/cover.jpg"},
}
Expect(explainResult("folder", steps)).To(Equal("resolved from folder"))
})
It("reports indeterminate when an external lookup failed transiently", func() {
steps := []artwork.TraceStep{
{Candidate: "artist.*", Outcome: "miss"},
{Candidate: "external:deezer", Outcome: "error", Detail: "context deadline exceeded"},
}
Expect(explainResult("", steps)).To(ContainSubstring("indeterminate"),
"a failed network call is not evidence that the item has no artwork")
})
It("qualifies a win a failed higher-priority external lookup could have taken", func() {
steps := []artwork.TraceStep{
{Candidate: "external:deezer", Outcome: "error", Detail: "context deadline exceeded"},
{Candidate: "artist.*", Outcome: "hit", Detail: "/music/artist.jpg"},
}
res := explainResult("artist.*", steps)
Expect(res).To(ContainSubstring("resolved from artist.*"))
Expect(res).To(ContainSubstring("indeterminate"),
"the resolver serves this hit but retries later, so the winner is provisional")
})
It("does not qualify an external win that followed a failed external lookup", func() {
steps := []artwork.TraceStep{
{Candidate: "external:deezer", Outcome: "error", Detail: "context deadline exceeded"},
{Candidate: "external:lastfm", Outcome: "hit", Detail: "http://img"},
}
Expect(explainResult("external:lastfm", steps)).To(Equal("resolved from external:lastfm"),
"a later agent supplying the image discards the earlier error, so there is no retry to warn about")
})
It("does not qualify a win that outranked the failed external lookup", func() {
steps := []artwork.TraceStep{
{Candidate: "artist.*", Outcome: "hit"},
{Candidate: "external:deezer", Outcome: "error", Detail: "context deadline exceeded"},
}
Expect(explainResult("artist.*", steps)).To(Equal("resolved from artist.*"))
})
})
var _ = Describe("explainAgents", func() {
It("accounts for every configured agent, marking the ones the CLI could not use", func() {
out := explainAgents("artist-nfo-metadata,apple-music,deezer,lastfm", []string{"deezer"})
for _, name := range []string{"artist-nfo-metadata", "apple-music", "deezer", "lastfm"} {
Expect(out).To(ContainSubstring(name),
"a configured agent missing from this line reads as if it had never been configured")
}
Expect(out).To(ContainSubstring("not available to the CLI"))
})
It("does not mark anything when every configured agent is available", func() {
out := explainAgents("deezer, lastfm", []string{"lastfm", "deezer"})
Expect(out).To(Equal("deezer, lastfm"))
})
It("reports an empty configuration as none, not as an unavailable agent", func() {
Expect(explainAgents("", nil)).To(Equal("(none)"))
})
})
var _ = Describe("formatExplain", func() {
var rep explainReport
@ -404,25 +304,6 @@ var _ = Describe("formatExplain", func() {
})
})
var _ = Describe("explainConfig", func() {
BeforeEach(func() {
DeferCleanup(configtest.SetupConfig())
conf.Server.DiscArtPriority = "cover.jpg"
conf.Server.EnableMediaFileCoverArt = true
})
DescribeTable("names the setting that decides where a kind's artwork comes from",
func(kind model.Kind, setting, value string) {
gotSetting, gotValue := explainConfig(kind)
Expect(gotSetting).To(Equal(setting))
Expect(gotValue).To(Equal(value))
},
Entry("disc", model.KindDiscArtwork, "DiscArtPriority", "cover.jpg"),
Entry("media file", model.KindMediaFileArtwork, "EnableMediaFileCoverArt", "true"),
Entry("playlist has none", model.KindPlaylistArtwork, "", ""),
)
})
var _ = Describe("artwork refresh command", func() {
It("requires at least one argument", func() {
Expect(artworkRefreshCmd.Args(artworkRefreshCmd, []string{})).To(HaveOccurred())

85
core/artwork/explain.go Normal file
View file

@ -0,0 +1,85 @@
package artwork
import (
"slices"
"strconv"
"strings"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/core/agents"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/utils/slice"
)
// Result states the verdict of the walk. A skipped or failed external tier, or a local candidate
// that would not open, leaves the outcome unknown: nothing observed that there is no artwork.
func Result(source string, steps []TraceStep) string {
if source != "" {
for _, s := range steps {
if s.Outcome == OutcomeHit {
break
}
// An external winner discards the earlier error, so the resolver settles it with no retry.
if s.Outcome == OutcomeError && strings.HasPrefix(s.Candidate, ExternalPrefix) &&
!strings.HasPrefix(source, ExternalPrefix) {
return "resolved from " + source +
" (indeterminate: a higher-priority external lookup failed; this may resolve differently on a retry)"
}
}
return "resolved from " + source
}
for _, s := range steps {
switch {
case s.Outcome == OutcomeError && strings.HasPrefix(s.Candidate, ExternalPrefix):
return "indeterminate (an external lookup failed; the item may resolve on a later attempt)"
case s.Outcome == OutcomeError, s.Outcome == OutcomeUnreadable:
return "indeterminate (a candidate was found but could not be processed; the worker retries rather than settling absent)"
}
}
return "not resolved"
}
// ConfigFor names the setting that decides where a kind's artwork comes from, and its value.
func ConfigFor(kind model.Kind) (setting, value string) {
switch kind {
case model.KindArtistArtwork:
return "ArtistArtPriority", conf.Server.ArtistArtPriority
case model.KindAlbumArtwork:
return "CoverArtPriority", conf.Server.CoverArtPriority
case model.KindDiscArtwork:
return "DiscArtPriority", conf.Server.DiscArtPriority
case model.KindMediaFileArtwork:
return "EnableMediaFileCoverArt", strconv.FormatBool(conf.Server.EnableMediaFileCoverArt)
}
return "", ""
}
// ImageAgentNames names the agents that can actually supply an image for kind.
func ImageAgentNames(ag *agents.Agents, kind model.Kind) []string {
if kind == model.KindArtistArtwork {
return slice.Map(ag.ArtistImageAgents(), func(a agents.ArtistImageAgent) string { return a.Name })
}
return slice.Map(ag.AlbumImageAgents(), func(a agents.AlbumImageAgent) string { return a.Name })
}
// FormatAgents accounts for every configured agent: one that cannot be constructed never reaches
// the chain, so the raw list alone overstates it. unavailableNote is appended only if some are.
func FormatAgents(configured string, available []string, unavailableNote string) string {
if strings.TrimSpace(configured) == "" {
return "(none)"
}
var unavailable bool
names := slice.Map(strings.Split(configured, ","), func(name string) string {
name = strings.TrimSpace(name)
if slices.Contains(available, name) {
return name
}
unavailable = true
return name + "*"
})
line := strings.Join(names, ", ")
if unavailable {
line += unavailableNote
}
return line
}

82
core/artwork/explain_test.go Normal file
View file

@ -0,0 +1,82 @@
package artwork_test
import (
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/conf/configtest"
"github.com/navidrome/navidrome/core/artwork"
"github.com/navidrome/navidrome/model"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
var _ = Describe("Result", func() {
It("reports the source when one was found", func() {
steps := []artwork.TraceStep{{Candidate: "cover.*", Outcome: artwork.OutcomeHit}}
Expect(artwork.Result("folder", steps)).To(Equal("resolved from folder"))
})
It("flags a local win that a failed external lookup could have outranked", func() {
steps := []artwork.TraceStep{
{Candidate: artwork.ExternalPrefix + "deezer", Outcome: artwork.OutcomeError, Detail: "timeout"},
{Candidate: "cover.*", Outcome: artwork.OutcomeHit},
}
Expect(artwork.Result("folder", steps)).To(ContainSubstring("indeterminate"))
})
It("does not flag an external winner", func() {
steps := []artwork.TraceStep{
{Candidate: artwork.ExternalPrefix + "lastfm", Outcome: artwork.OutcomeError},
{Candidate: artwork.ExternalPrefix + "deezer", Outcome: artwork.OutcomeHit},
}
Expect(artwork.Result(artwork.ExternalPrefix+"deezer", steps)).
To(Equal("resolved from external:deezer"))
})
It("is indeterminate when an external lookup failed and nothing resolved", func() {
steps := []artwork.TraceStep{{Candidate: artwork.ExternalPrefix + "deezer", Outcome: artwork.OutcomeError}}
Expect(artwork.Result("", steps)).To(ContainSubstring("an external lookup failed"))
})
It("is indeterminate when a candidate could not be processed", func() {
steps := []artwork.TraceStep{{Candidate: "cover.*", Outcome: artwork.OutcomeUnreadable}}
Expect(artwork.Result("", steps)).To(ContainSubstring("could not be processed"))
})
It("reports a clean miss", func() {
steps := []artwork.TraceStep{{Candidate: "cover.*", Outcome: artwork.OutcomeMiss}}
Expect(artwork.Result("", steps)).To(Equal("not resolved"))
})
})
var _ = Describe("ConfigFor", func() {
BeforeEach(func() {
DeferCleanup(configtest.SetupConfig())
conf.Server.ArtistArtPriority = "external"
})
It("names the setting for a kind that has one", func() {
setting, value := artwork.ConfigFor(model.KindArtistArtwork)
Expect(setting).To(Equal("ArtistArtPriority"))
Expect(value).To(Equal("external"))
})
It("returns nothing for a kind with no source configuration", func() {
setting, _ := artwork.ConfigFor(model.KindPlaylistArtwork)
Expect(setting).To(BeEmpty())
})
})
var _ = Describe("FormatAgents", func() {
It("reports none when nothing is configured", func() {
Expect(artwork.FormatAgents(" ", nil, "")).To(Equal("(none)"))
})
It("keeps the configured order and marks what is unavailable", func() {
got := artwork.FormatAgents("spotify, lastfm", []string{"lastfm"}, " (* missing)")
Expect(got).To(Equal("spotify*, lastfm (* missing)"))
})
It("omits the note when every configured agent is available", func() {
Expect(artwork.FormatAgents("lastfm", []string{"lastfm"}, " (* missing)")).To(Equal("lastfm"))
})
})