From 0ab549e1b2af4ecbb196088e82f6df621478a241 Mon Sep 17 00:00:00 2001 From: Deluan Date: Thu, 3 Sep 2026 23:01:29 -0400 Subject: [PATCH] refactor(artwork): build the explain report in core/artwork Moves report construction (stored state, queue row, chain walk or recorded trace) out of the CLI and into artwork.Explain, so a future HTTP handler can reuse it. cmd/artwork.go keeps only text formatting. --- cmd/artwork.go | 145 +++++++++++-------------------- cmd/artwork_test.go | 74 ++++++++-------- core/artwork/explain.go | 99 +++++++++++++++++++++ core/artwork/explain_test.go | 87 +++++++++++++++++++ tests/mock_artwork_queue_repo.go | 12 +++ 5 files changed, 286 insertions(+), 131 deletions(-) diff --git a/cmd/artwork.go b/cmd/artwork.go index 0ab391425..5d4c8a04b 100644 --- a/cmd/artwork.go +++ b/cmd/artwork.go @@ -749,32 +749,6 @@ func artworkKindAndID(ctx context.Context, ds model.DataStore, arg string) (mode // 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 - id string - name string - stored *model.ItemArtwork - queued *model.ArtworkQueueItem - agents string - // steps is the chain walk: recorded when the item was resolved, or performed just now when walked. - steps []artwork.TraceStep - source string - walked bool - resolveErr error -} - -// explainChainOrigin says whether the operator is reading history or a walk performed just now, -// since the two can disagree after a config change. -func explainChainOrigin(rep explainReport) string { - if rep.walked { - return "walked now" - } - if rep.stored != nil { - return "recorded " + formatTime(rep.stored.AttemptedAt) - } - return "not recorded" -} - // writeSteps prints the trace rows. An empty last cell would end tabwriter's column block and // break the alignment, so a missing detail is rendered as a dash. func writeSteps(w io.Writer, indent string, steps []artwork.TraceStep) { @@ -793,89 +767,89 @@ func writeStepTable(w io.Writer, title string, steps []artwork.TraceStep) { writeSteps(w, " ", steps) } -func formatExplain(rep explainReport) string { +func formatExplain(rep artwork.ExplainReport) string { var sb strings.Builder w := newTabWriter(&sb) - explainable := artwork.Explainable(rep.kind) - stateful := artwork.KeepsState(rep.kind) - unrecorded := !rep.walked && rep.stored == nil + explainable := artwork.Explainable(rep.Kind) + stateful := artwork.KeepsState(rep.Kind) + unrecorded := !rep.Walked && rep.Stored == nil fmt.Fprintln(w, "Item") - fmt.Fprintf(w, " Kind:\t%s (%s)\n", rep.kind, rep.kind.Prefix()) - fmt.Fprintf(w, " ID:\t%s\n", rep.id) - fmt.Fprintf(w, " Name:\t%s\n", rep.name) + fmt.Fprintf(w, " Kind:\t%s (%s)\n", rep.Kind, rep.Kind.Prefix()) + fmt.Fprintf(w, " ID:\t%s\n", rep.ID) + fmt.Fprintf(w, " Name:\t%s\n", rep.Name) fmt.Fprintln(w, "\nStored") switch { case !stateful: - fmt.Fprintf(w, " (%s artwork is resolved on every request and never recorded)\n", rep.kind) - case rep.stored == nil: + fmt.Fprintf(w, " (%s artwork is resolved on every request and never recorded)\n", rep.Kind) + case rep.Stored == nil: fmt.Fprintln(w, " (no artwork state recorded)") default: - fmt.Fprintf(w, " Source:\t%s\n", displaySource(rep.stored.Source)) - fmt.Fprintf(w, " Hash:\t%s\n", cmp.Or(rep.stored.Hash, "(absent)")) - if rep.stored.SourcePath != "" { - fmt.Fprintf(w, " Source path:\t%s\n", rep.stored.SourcePath) + fmt.Fprintf(w, " Source:\t%s\n", displaySource(rep.Stored.Source)) + fmt.Fprintf(w, " Hash:\t%s\n", cmp.Or(rep.Stored.Hash, "(absent)")) + if rep.Stored.SourcePath != "" { + fmt.Fprintf(w, " Source path:\t%s\n", rep.Stored.SourcePath) } - fmt.Fprintf(w, " Attempted at:\t%s\n", formatTime(rep.stored.AttemptedAt)) + fmt.Fprintf(w, " Attempted at:\t%s\n", formatTime(rep.Stored.AttemptedAt)) } fmt.Fprintln(w, "\nQueue") switch { case !stateful: fmt.Fprintln(w, " (never queued)") - case rep.queued == nil: + case rep.Queued == nil: fmt.Fprintln(w, " (not queued)") default: - fmt.Fprintf(w, " Priority:\t%s (%d)\n", priorityName(rep.queued.Priority), rep.queued.Priority) - fmt.Fprintf(w, " Attempts:\t%d\n", rep.queued.Attempts) - fmt.Fprintf(w, " Retry at:\t%s\n", formatTime(rep.queued.RetryAt)) + fmt.Fprintf(w, " Priority:\t%s (%d)\n", priorityName(rep.Queued.Priority), rep.Queued.Priority) + fmt.Fprintf(w, " Attempts:\t%d\n", rep.Queued.Attempts) + fmt.Fprintf(w, " Retry at:\t%s\n", formatTime(rep.Queued.RetryAt)) } - if rep.queued != nil { - writeStepTable(w, "Last attempt failed", artwork.DecodeTrace(rep.queued.Trace, "")) + if rep.Queued != nil { + writeStepTable(w, "Last attempt failed", rep.LastAttemptFailed()) } - if rep.stored != nil { - writeStepTable(w, "Gave up after", artwork.DecodeTrace(rep.stored.LastFailure, "")) + if rep.Stored != nil { + writeStepTable(w, "Gave up after", rep.GaveUpAfter()) } fmt.Fprintln(w, "\nConfig") - if setting, value := artwork.ConfigFor(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) - if rep.agents != "" { - fmt.Fprintf(w, " Agents:\t%s\n", rep.agents) + if rep.Agents != "" { + fmt.Fprintf(w, " Agents:\t%s\n", rep.Agents) } } - fmt.Fprintf(w, "\nChain (%s)\n", explainChainOrigin(rep)) + fmt.Fprintf(w, "\nChain (%s)\n", rep.ChainOrigin()) switch { case !explainable: - fmt.Fprintf(w, " (%s artwork does not walk a priority chain)\n", rep.kind) + fmt.Fprintf(w, " (%s artwork does not walk a priority chain)\n", rep.Kind) case unrecorded: fmt.Fprintln(w, " (no resolution recorded yet; re-run with --live to walk the chain now)") - case !rep.walked && len(rep.steps) == 0 && rep.stored.Hash != "": + case !rep.Walked && len(rep.Steps) == 0 && rep.Stored.Hash != "": // A stored image with no chain can only predate trace recording: a recorded resolution that // found an image always records its winning candidate. fmt.Fprintln(w, " (this item was resolved before traces were recorded; re-run with --live)") - case !rep.walked && len(rep.steps) == 0: + case !rep.Walked && len(rep.Steps) == 0: // Absent with no chain: an empty priority list walked nothing, or a pre-tracing absent row. fmt.Fprintln(w, " (no candidates were recorded; re-run with --live to walk the chain now)") default: fmt.Fprintln(w, " CANDIDATE\tOUTCOME\tDETAIL") - writeSteps(w, " ", rep.steps) + writeSteps(w, " ", rep.Steps) } fmt.Fprintln(w, "\nResult") switch { - case rep.resolveErr != nil: - fmt.Fprintf(w, " resolution failed: %s\n", rep.resolveErr) + case rep.ResolveErr != nil: + fmt.Fprintf(w, " resolution failed: %s\n", rep.ResolveErr) case !explainable: fmt.Fprintln(w, " not evaluated (no chain was walked; see Stored above)") case unrecorded: fmt.Fprintln(w, " not evaluated (nothing recorded; re-run with --live to walk the chain now)") default: - fmt.Fprintf(w, " %s\n", artwork.Result(rep.source, rep.steps)) + fmt.Fprintf(w, " %s\n", rep.Result()) } w.Flush() @@ -905,46 +879,29 @@ func runExplain(ctx context.Context, args []string) { } kind, id := targets[0].Kind, targets[0].ID - name, err := artwork.ItemName(ctx, ds, kind, id) - if err != nil { - log.Fatal(ctx, "Item not found", "kind", kind, "id", id, err) + opts := artwork.ExplainOptions{UnavailableNote: cliUnavailableNote} + // Only artist and album reach an agent, and the load must precede the resolver, which reads the + // same manager. Leaving ag nil elsewhere avoids handing agents.GetAgents a not-yet-loaded manager. + var ag *agents.Agents + if kind == model.KindArtistArtwork || kind == model.KindAlbumArtwork { + mgr := loadPluginAgents(ctx, explainLive) + defer func() { _ = mgr.Stop() }() + ag = agents.GetAgents(ds, mgr) } - rep := explainReport{kind: kind, id: id, name: name} - if artwork.KeepsState(kind) { - rep.stored, err = ds.Artwork(ctx).GetItemArtwork(kind, id, model.ImageTypePrimary) - if err != nil && !errors.Is(err, model.ErrNotFound) { - log.Fatal(ctx, "Failed to read artwork state", "kind", kind, "id", id, err) - } - rep.queued, err = ds.ArtworkQueue(ctx).Get(kind, id, model.ImageTypePrimary) - if err != nil && !errors.Is(err, model.ErrNotFound) { - log.Fatal(ctx, "Failed to read the artwork queue", "kind", kind, "id", id, err) + // Disc artwork keeps no row, so it has no stored trace and can only be explained by walking now. + if explainLive || !artwork.KeepsState(kind) { + opts.Walk = func(t *artwork.ChainTrace) *artwork.TracingResolver { + return CreateArtworkResolver(t, explainLive) } } + rep, err := artwork.Explain(ctx, ds, ag, kind, id, opts) + if err != nil { + log.Fatal(ctx, "Failed to explain artwork", "kind", kind, "id", id, err) + } - // Disc artwork keeps no row, so it has no stored trace and can only be explained by walking now. - rep.walked = explainLive || !artwork.KeepsState(kind) - if artwork.Explainable(kind) { - // Only artist and album reach an agent, and the load must precede the resolver, which reads - // the same manager. - if kind == model.KindArtistArtwork || kind == model.KindAlbumArtwork { - mgr := loadPluginAgents(ctx, explainLive) - defer func() { _ = mgr.Stop() }() - rep.agents = artwork.FormatAgents(conf.Server.Agents, - artwork.ImageAgentNames(agents.GetAgents(ds, mgr), kind), cliUnavailableNote) - } - switch { - case rep.walked: - trace := &artwork.ChainTrace{} - rep.source, rep.resolveErr = CreateArtworkResolver(trace, explainLive).Resolve(ctx, kind, id) - rep.steps = trace.Steps() - case rep.stored != nil: - rep.steps = artwork.DecodeTrace(rep.stored.Trace, rep.stored.SourcePath) - rep.source = rep.stored.Source - } - } fmt.Print(formatExplain(rep)) // The steps taken before a failed walk are the diagnosis, so report them before exiting. - if rep.resolveErr != nil { - log.Fatal(ctx, "Failed to resolve artwork", "kind", kind, "id", id, rep.resolveErr) + if rep.ResolveErr != nil { + log.Fatal(ctx, "Failed to resolve artwork", "kind", kind, "id", id, rep.ResolveErr) } } diff --git a/cmd/artwork_test.go b/cmd/artwork_test.go index dc33f9e13..0be9d85ec 100644 --- a/cmd/artwork_test.go +++ b/cmd/artwork_test.go @@ -123,22 +123,22 @@ var _ = Describe("resolveArtworkTargets", func() { }) var _ = Describe("formatExplain", func() { - var rep explainReport + var rep artwork.ExplainReport BeforeEach(func() { DeferCleanup(configtest.SetupConfig()) conf.Server.ArtistArtPriority = "external, artist.*" - rep = explainReport{ - kind: model.KindArtistArtwork, - id: "ar-1", - name: "Radiohead", - agents: "lastfm,spotify", - walked: true, - steps: []artwork.TraceStep{ + rep = artwork.ExplainReport{ + Kind: model.KindArtistArtwork, + ID: "ar-1", + Name: "Radiohead", + Agents: "lastfm,spotify", + Walked: true, + Steps: []artwork.TraceStep{ {Candidate: "upload", Outcome: "skipped", Detail: "no uploaded image"}, {Candidate: "external:deezer", Outcome: "error", Detail: "context deadline exceeded"}, }, - source: "", + Source: "", } }) @@ -160,11 +160,11 @@ var _ = Describe("formatExplain", func() { It("prints the stored state and the queue row when they exist", func() { attempted := time.Date(2026, 8, 13, 10, 0, 0, 0, time.UTC) - rep.stored = &model.ItemArtwork{Source: "folder", Hash: "abc123", + rep.Stored = &model.ItemArtwork{Source: "folder", Hash: "abc123", SourcePath: "/music/cover.jpg", AttemptedAt: attempted} - rep.queued = &model.ArtworkQueueItem{Priority: model.ArtworkPriorityScan, Attempts: 2, + rep.Queued = &model.ArtworkQueueItem{Priority: model.ArtworkPriorityScan, Attempts: 2, RetryAt: attempted.Add(time.Hour)} - rep.source = "folder" + rep.Source = "folder" out := formatExplain(rep) Expect(out).To(ContainSubstring("abc123")) @@ -175,12 +175,12 @@ var _ = Describe("formatExplain", func() { }) It("marks a known-absent stored state instead of printing an empty hash", func() { - rep.stored = &model.ItemArtwork{AttemptedAt: time.Now()} + rep.Stored = &model.ItemArtwork{AttemptedAt: time.Now()} Expect(formatExplain(rep)).To(ContainSubstring("absent")) }) It("reports a failed walk as failed, not as unresolved", func() { - rep.resolveErr = errors.New("no such directory") + rep.ResolveErr = errors.New("no such directory") out := formatExplain(rep) Expect(out).To(ContainSubstring("resolution failed: no such directory")) @@ -190,9 +190,9 @@ var _ = Describe("formatExplain", func() { It("says a kind that does not walk a chain has no chain, without an empty table", func() { conf.Server.CoverArtPriority = "cover.*, embedded" - rep.kind = model.KindPlaylistArtwork - rep.steps = nil - rep.agents = "" + rep.Kind = model.KindPlaylistArtwork + rep.Steps = nil + rep.Agents = "" out := formatExplain(rep) Expect(out).To(ContainSubstring("does not walk a priority chain")) @@ -206,11 +206,11 @@ var _ = Describe("formatExplain", func() { It("says disc artwork keeps no state instead of reporting it as unresolved state", func() { conf.Server.DiscArtPriority = "cover.jpg, embedded" - rep = explainReport{ - kind: model.KindDiscArtwork, id: "al-1:2", name: "OK Computer (disc 2)", - steps: []artwork.TraceStep{{Candidate: "cover.jpg", Outcome: "hit", Detail: "/music/cover.jpg"}}, - source: "folder", - walked: true, + rep = artwork.ExplainReport{ + Kind: model.KindDiscArtwork, ID: "al-1:2", Name: "OK Computer (disc 2)", + Steps: []artwork.TraceStep{{Candidate: "cover.jpg", Outcome: "hit", Detail: "/music/cover.jpg"}}, + Source: "folder", + Walked: true, } out := formatExplain(rep) @@ -225,15 +225,15 @@ var _ = Describe("formatExplain", func() { Context("stored traces", func() { BeforeEach(func() { - rep.walked = false - rep.steps = nil + rep.Walked = false + rep.Steps = nil }) It("labels a recorded chain with when it was recorded, not as a walk done now", func() { attempted := time.Date(2026, 8, 13, 10, 0, 0, 0, time.UTC) - rep.stored = &model.ItemArtwork{Source: "folder", Hash: "abc", AttemptedAt: attempted} - rep.steps = []artwork.TraceStep{{Candidate: "artist.*", Outcome: "hit", Detail: "/music/artist.jpg"}} - rep.source = "folder" + rep.Stored = &model.ItemArtwork{Source: "folder", Hash: "abc", AttemptedAt: attempted} + rep.Steps = []artwork.TraceStep{{Candidate: "artist.*", Outcome: "hit", Detail: "/music/artist.jpg"}} + rep.Source = "folder" out := formatExplain(rep) Expect(out).To(ContainSubstring("Chain (recorded 2026-08-13T10:00:00Z)")) @@ -250,7 +250,7 @@ var _ = Describe("formatExplain", func() { }) It("distinguishes a row written before traces existed from one with an empty chain", func() { - rep.stored = &model.ItemArtwork{Source: "folder", Hash: "abc", AttemptedAt: time.Now()} + rep.Stored = &model.ItemArtwork{Source: "folder", Hash: "abc", AttemptedAt: time.Now()} Expect(formatExplain(rep)).To(ContainSubstring("resolved before traces were recorded")) }) @@ -258,7 +258,7 @@ var _ = Describe("formatExplain", func() { It("does not call an absent row with an empty recorded chain a pre-tracing row", func() { // An empty priority list records a real but empty chain and resolves absent; that is not a // legacy row, so it must not be reported as resolved before tracing existed. - rep.stored = &model.ItemArtwork{Source: "", Hash: "", AttemptedAt: time.Now()} + rep.Stored = &model.ItemArtwork{Source: "", Hash: "", AttemptedAt: time.Now()} out := formatExplain(rep) Expect(out).ToNot(ContainSubstring("resolved before traces were recorded")) @@ -267,9 +267,9 @@ var _ = Describe("formatExplain", func() { }) It("prints why the last attempt failed and why it gave up", func() { - rep.queued = &model.ArtworkQueueItem{Priority: model.ArtworkPriorityScan, Attempts: 3, + rep.Queued = &model.ArtworkQueueItem{Priority: model.ArtworkPriorityScan, Attempts: 3, Trace: `[{"c":"decode","o":"error","d":"bad header"}]`} - rep.stored = &model.ItemArtwork{Source: "folder", Hash: "abc", AttemptedAt: time.Now(), + rep.Stored = &model.ItemArtwork{Source: "folder", Hash: "abc", AttemptedAt: time.Now(), LastFailure: `[{"c":"read","o":"error","d":"i/o timeout"}]`} out := formatExplain(rep) @@ -288,10 +288,10 @@ var _ = Describe("formatExplain", func() { It("reports the setting that governs media file artwork", func() { conf.Server.EnableMediaFileCoverArt = false - rep = explainReport{ - kind: model.KindMediaFileArtwork, id: "mf-1", name: "Airbag", - walked: true, - steps: []artwork.TraceStep{ + rep = artwork.ExplainReport{ + Kind: model.KindMediaFileArtwork, ID: "mf-1", Name: "Airbag", + Walked: true, + Steps: []artwork.TraceStep{ {Candidate: "embedded", Outcome: "skipped", Detail: "EnableMediaFileCoverArt is off"}, }, } @@ -376,8 +376,8 @@ var _ = Describe("explain/reprocess source round trip", func() { Expect(art.PutItemArtwork(&model.ItemArtwork{ItemKind: model.KindArtistArtwork.Prefix(), ItemID: "ar-1", ImageType: model.ImageTypePrimary})).To(Succeed()) - shown := storedSource(formatExplain(explainReport{kind: model.KindArtistArtwork, id: "ar-1", - stored: &model.ItemArtwork{AttemptedAt: time.Now()}})) + shown := storedSource(formatExplain(artwork.ExplainReport{Kind: model.KindArtistArtwork, ID: "ar-1", + Stored: &model.ItemArtwork{AttemptedAt: time.Now()}})) q := ds.ArtworkQueue(ctx) Expect(validateSources(q, repositorySources([]string{shown}))).To(Succeed(), diff --git a/core/artwork/explain.go b/core/artwork/explain.go index 4c0d465c9..c6bba8553 100644 --- a/core/artwork/explain.go +++ b/core/artwork/explain.go @@ -1,9 +1,13 @@ package artwork import ( + "context" + "errors" + "fmt" "slices" "strconv" "strings" + "time" "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/core/agents" @@ -83,3 +87,98 @@ func FormatAgents(configured string, available []string, unavailableNote string) } return line } + +// ExplainOptions configures a single explain. The zero value reads history and never +// touches the network. +type ExplainOptions struct { + // Walk builds a resolver that records into the trace; nil reads the recorded trace instead. + Walk func(*ChainTrace) *TracingResolver + // UnavailableNote is appended to the agent line when a configured agent is missing. + UnavailableNote string +} + +// ExplainReport is everything known about how one item's artwork resolved. +type ExplainReport struct { + Kind model.Kind + ID string + Name string + Stored *model.ItemArtwork + Queued *model.ArtworkQueueItem + Steps []TraceStep + Source string + Agents string + Walked bool + ResolveErr error +} + +// Explain gathers everything known about how kind/id's artwork resolved: stored state, the +// queue row, and either the recorded trace or a fresh walk, depending on opts.Walk. +func Explain(ctx context.Context, ds model.DataStore, ag *agents.Agents, kind model.Kind, id string, + opts ExplainOptions) (ExplainReport, error) { + name, err := ItemName(ctx, ds, kind, id) + if err != nil { + return ExplainReport{}, err + } + rep := ExplainReport{Kind: kind, ID: id, Name: name} + + if KeepsState(kind) { + rep.Stored, err = ds.Artwork(ctx).GetItemArtwork(kind, id, model.ImageTypePrimary) + if err != nil && !errors.Is(err, model.ErrNotFound) { + return ExplainReport{}, fmt.Errorf("reading artwork state: %w", err) + } + rep.Queued, err = ds.ArtworkQueue(ctx).Get(kind, id, model.ImageTypePrimary) + if err != nil && !errors.Is(err, model.ErrNotFound) { + return ExplainReport{}, fmt.Errorf("reading the artwork queue: %w", err) + } + } + if !Explainable(kind) { + return rep, nil + } + if ag != nil && (kind == model.KindArtistArtwork || kind == model.KindAlbumArtwork) { + rep.Agents = FormatAgents(conf.Server.Agents, ImageAgentNames(ag, kind), opts.UnavailableNote) + } + + rep.Walked = opts.Walk != nil + switch { + case rep.Walked: + trace := &ChainTrace{} + rep.Source, rep.ResolveErr = opts.Walk(trace).Resolve(ctx, kind, id) + rep.Steps = trace.Steps() + case rep.Stored != nil: + rep.Steps = DecodeTrace(rep.Stored.Trace, rep.Stored.SourcePath) + rep.Source = rep.Stored.Source + } + return rep, nil +} + +// Result reports this report's verdict; see the package-level Result for the rules. +func (r ExplainReport) Result() string { return Result(r.Source, r.Steps) } + +// ChainOrigin says whether the report reads history or a walk performed just now, since the two +// can disagree after a config change. +func (r ExplainReport) ChainOrigin() string { + if r.Walked { + return "walked now" + } + if r.Stored != nil { + return "recorded " + r.Stored.AttemptedAt.Format(time.RFC3339) + } + return "not recorded" +} + +// LastAttemptFailed decodes why the queued row's last attempt failed, if there is one queued. +func (r ExplainReport) LastAttemptFailed() []TraceStep { + if r.Queued == nil { + return nil + } + return DecodeTrace(r.Queued.Trace, "") +} + +// GaveUpAfter decodes the trace of the attempt that exhausted the retry budget, if the stored +// state recorded one. +func (r ExplainReport) GaveUpAfter() []TraceStep { + if r.Stored == nil { + return nil + } + return DecodeTrace(r.Stored.LastFailure, "") +} diff --git a/core/artwork/explain_test.go b/core/artwork/explain_test.go index 14df4bbbf..8f9abf224 100644 --- a/core/artwork/explain_test.go +++ b/core/artwork/explain_test.go @@ -1,10 +1,14 @@ package artwork_test import ( + "context" + "time" + "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/conf/configtest" "github.com/navidrome/navidrome/core/artwork" "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/tests" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -60,6 +64,13 @@ var _ = Describe("ConfigFor", func() { Expect(value).To(Equal("external")) }) + It("names the setting for the album kind", func() { + conf.Server.CoverArtPriority = "cover.*, embedded" + setting, value := artwork.ConfigFor(model.KindAlbumArtwork) + Expect(setting).To(Equal("CoverArtPriority")) + Expect(value).To(Equal("cover.*, embedded")) + }) + It("returns nothing for a kind with no source configuration", func() { setting, _ := artwork.ConfigFor(model.KindPlaylistArtwork) Expect(setting).To(BeEmpty()) @@ -80,3 +91,79 @@ var _ = Describe("FormatAgents", func() { Expect(artwork.FormatAgents("lastfm", []string{"lastfm"}, " (* missing)")).To(Equal("lastfm")) }) }) + +var _ = Describe("Explain", func() { + var ds *tests.MockDataStore + var artRepo *tests.MockArtworkRepo + var queueRepo *tests.MockArtworkQueueRepo + var ctx context.Context + + BeforeEach(func() { + ctx = context.Background() + artRepo = tests.CreateMockArtworkRepo() + queueRepo = tests.CreateMockArtworkQueueRepo() + ds = &tests.MockDataStore{MockedArtwork: artRepo, MockedArtworkQueue: queueRepo} + Expect(ds.Artist(ctx).Put(&model.Artist{ID: "ar-1", Name: "Radiohead"})).To(Succeed()) + }) + + It("returns an error when the item does not exist", func() { + _, err := artwork.Explain(ctx, ds, nil, model.KindArtistArtwork, "nope", artwork.ExplainOptions{}) + Expect(err).To(MatchError(model.ErrNotFound)) + }) + + It("reads the recorded trace when no walker is supplied", func() { + // Storage shape mirrors storedStep in trace.go: c=candidate, o=outcome, d=detail. + trace := `[{"c":"external:deezer","o":"hit","d":"https://cdn/x.jpg"}]` + Expect(artRepo.PutItemArtwork(&model.ItemArtwork{ + ItemKind: model.KindArtistArtwork.Prefix(), ItemID: "ar-1", ImageType: model.ImageTypePrimary, + Hash: "abc", Source: "external:deezer", + Trace: trace, + AttemptedAt: time.Date(2026, 9, 1, 10, 0, 0, 0, time.UTC), + })).To(Succeed()) + + rep, err := artwork.Explain(ctx, ds, nil, model.KindArtistArtwork, "ar-1", artwork.ExplainOptions{}) + Expect(err).ToNot(HaveOccurred()) + Expect(rep.Name).To(Equal("Radiohead")) + Expect(rep.Walked).To(BeFalse()) + Expect(rep.Steps).To(Equal([]artwork.TraceStep{ + {Candidate: "external:deezer", Outcome: artwork.OutcomeHit, Detail: "https://cdn/x.jpg"}, + })) + Expect(rep.Source).To(Equal("external:deezer")) + Expect(rep.Result()).To(Equal("resolved from external:deezer")) + Expect(rep.ChainOrigin()).To(ContainSubstring("recorded")) + }) + + It("reports nothing recorded when there is no stored state", func() { + rep, err := artwork.Explain(ctx, ds, nil, model.KindArtistArtwork, "ar-1", artwork.ExplainOptions{}) + Expect(err).ToNot(HaveOccurred()) + Expect(rep.Stored).To(BeNil()) + Expect(rep.Steps).To(BeEmpty()) + Expect(rep.ChainOrigin()).To(Equal("not recorded")) + }) + + It("includes the queue row and its failure trace", func() { + failure := `[{"c":"external:lastfm","o":"error","d":"429"}]` + Expect(queueRepo.Enqueue(model.ArtworkQueueItem{ + ItemKind: model.KindArtistArtwork.Prefix(), ItemID: "ar-1", ImageType: model.ImageTypePrimary, + Priority: model.ArtworkPriorityScan, + })).To(Succeed()) + queueRepo.SetTrace(model.KindArtistArtwork, "ar-1", model.ImageTypePrimary, failure) + + rep, err := artwork.Explain(ctx, ds, nil, model.KindArtistArtwork, "ar-1", artwork.ExplainOptions{}) + Expect(err).ToNot(HaveOccurred()) + Expect(rep.Queued).ToNot(BeNil()) + Expect(rep.Queued.Priority).To(Equal(model.ArtworkPriorityScan)) + Expect(rep.LastAttemptFailed()).To(Equal([]artwork.TraceStep{ + {Candidate: "external:lastfm", Outcome: artwork.OutcomeError, Detail: "429"}, + })) + }) + + It("records nothing for a kind that keeps no state and has no walker", func() { + Expect(ds.Album(ctx).Put(&model.Album{ID: "al-1", Name: "OK Computer"})).To(Succeed()) + + rep, err := artwork.Explain(ctx, ds, nil, model.KindDiscArtwork, "al-1:2", artwork.ExplainOptions{}) + Expect(err).ToNot(HaveOccurred()) + Expect(rep.Stored).To(BeNil()) + Expect(rep.Steps).To(BeEmpty()) + }) +}) diff --git a/tests/mock_artwork_queue_repo.go b/tests/mock_artwork_queue_repo.go index c482e2150..23ec51bad 100644 --- a/tests/mock_artwork_queue_repo.go +++ b/tests/mock_artwork_queue_repo.go @@ -39,6 +39,18 @@ func (m *MockArtworkQueueRepo) Get(kind model.Kind, id, imageType string) (*mode return &it, nil } +// SetTrace stores a queue row's trace directly, bypassing the retry-token check +// MarkFailedIfUnchanged enforces; tests use it to seed a failure trace outright. +func (m *MockArtworkQueueRepo) SetTrace(kind model.Kind, id, imageType, trace string) { + m.mu.Lock() + defer m.mu.Unlock() + k := iaKey(kind.Prefix(), id, imageType) + if it, ok := m.Data[k]; ok { + it.Trace = trace + m.Data[k] = it + } +} + func (m *MockArtworkQueueRepo) Enqueue(items ...model.ArtworkQueueItem) error { m.mu.Lock() defer m.mu.Unlock()