diff --git a/core/artwork/agent_images.go b/core/artwork/agent_images.go index 35e3c666a..a6f746959 100644 --- a/core/artwork/agent_images.go +++ b/core/artwork/agent_images.go @@ -47,13 +47,13 @@ func fetchArtistImage(ctx context.Context, ag *agents.Agents, gate gateFunc, ar // Synthetic artists would otherwise get an unrelated agent result assigned to them. switch ar.ID { case consts.UnknownArtistID, consts.VariousArtistsID: - traceFrom(ctx).add(TraceStep{Candidate: ExternalCandidate, Outcome: OutcomeSkipped, Detail: "synthetic artist"}) + traceFrom(ctx).add(TraceStep{Candidate: externalCandidate, Outcome: OutcomeSkipped, Detail: "synthetic artist"}) return nil, "", false } name := externalName(ar.Name) imageAgents := ag.ArtistImageAgents() if len(imageAgents) == 0 { - traceFrom(ctx).add(TraceStep{Candidate: ExternalCandidate, Outcome: OutcomeSkipped, + traceFrom(ctx).add(TraceStep{Candidate: externalCandidate, Outcome: OutcomeSkipped, Detail: "no enabled agent provides artist images"}) return nil, "", false } @@ -85,7 +85,7 @@ func fetchAlbumImage(ctx context.Context, ag *agents.Agents, gate gateFunc, al m name, artist := externalName(al.Name), externalName(al.AlbumArtist) imageAgents := ag.AlbumImageAgents() if len(imageAgents) == 0 { - traceFrom(ctx).add(TraceStep{Candidate: ExternalCandidate, Outcome: OutcomeSkipped, + traceFrom(ctx).add(TraceStep{Candidate: externalCandidate, Outcome: OutcomeSkipped, Detail: "no enabled agent provides album images"}) return nil, "", false } diff --git a/core/artwork/disc.go b/core/artwork/disc.go index cfc372142..33789d910 100644 --- a/core/artwork/disc.go +++ b/core/artwork/disc.go @@ -123,7 +123,7 @@ func (d *discArtworkReader) fromDiscArtPriority(ctx context.Context, ffmpeg ffmp fromTag(ctx, d.lib.FS, d.firstTrackRel), fromFFmpegTag(ctx, ffmpeg, d.lib.Abs(d.firstTrackRel)), ) - case pattern == ExternalCandidate: + case pattern == externalCandidate: // Not supported for disc art, silently ignore case pattern == "discsubtitle": if subtitle := strings.TrimSpace(d.album.Discs[d.discNumber]); subtitle != "" { diff --git a/core/artwork/housekeeping.go b/core/artwork/housekeeping.go index 9cfad32bc..5a0937fd1 100644 --- a/core/artwork/housekeeping.go +++ b/core/artwork/housekeeping.go @@ -21,15 +21,15 @@ import ( // StaleAbsentAge is how long an absent state is trusted before a recheck retries it. const StaleAbsentAge = 24 * time.Hour -// recheckKinds omits media files: they resolve embedded-only, at scan or on view. -var recheckKinds = []model.Kind{ +// RecheckKinds omits media files: they resolve embedded-only, at scan or on view. +var RecheckKinds = []model.Kind{ model.KindArtistArtwork, model.KindAlbumArtwork, model.KindPlaylistArtwork, model.KindRadioArtwork, } // hasRecheckPath reports whether a periodic job will revisit this kind, making an absent settle recoverable. func hasRecheckPath(prefix string) bool { kind, ok := model.ParseKind(prefix) - return ok && slices.Contains(recheckKinds, kind) + return ok && slices.Contains(RecheckKinds, kind) } // artworkEpoch invalidates all resolution state when bumped; bump it whenever resolution semantics change. @@ -41,8 +41,7 @@ type FingerprintInput struct { Value string } -// FingerprintInputs is the single listing of what ConfigFingerprint hashes, so a caller can report -// which setting a fingerprint change came from without keeping a second copy of the list. +// FingerprintInputs is the single listing of what ConfigFingerprint hashes. func FingerprintInputs() []FingerprintInput { return []FingerprintInput{ {"CoverArtPriority", conf.Server.CoverArtPriority}, @@ -55,7 +54,6 @@ func FingerprintInputs() []FingerprintInput { } // ConfigFingerprint covers the inputs that affect resolution outcomes; a change invalidates stored state. -// Exported so the CLI reports the value backfill compares instead of computing one that can drift. func ConfigFingerprint() string { values := slice.Map(FingerprintInputs(), func(i FingerprintInput) string { return i.Value }) raw := fmt.Sprintf("%s|%d", strings.Join(values, "|"), artworkEpoch) @@ -119,7 +117,7 @@ func enqueueBackfillKind(ctx context.Context, ds model.DataStore, kind model.Kin func enqueueStaleAbsentAll(ctx context.Context, ds model.DataStore) error { cutoff := time.Now().Add(-StaleAbsentAge) queue := ds.ArtworkQueue(ctx) - for _, kind := range recheckKinds { + for _, kind := range RecheckKinds { if _, err := queue.EnqueueStaleAbsent(kind, cutoff); err != nil { return err } @@ -130,7 +128,7 @@ func enqueueStaleAbsentAll(ctx context.Context, ds model.DataStore) error { // enqueueMissingAll is the safety net for entities a scan never enqueued (added between scans, or scanner off). func enqueueMissingAll(ctx context.Context, ds model.DataStore) error { queue := ds.ArtworkQueue(ctx) - for _, kind := range recheckKinds { + for _, kind := range RecheckKinds { if _, err := queue.EnqueueAllMissing(kind, model.ArtworkPriorityRecheck); err != nil { return err } diff --git a/core/artwork/resolve.go b/core/artwork/resolve.go index cc5bbf1b5..2a36e6dfd 100644 --- a/core/artwork/resolve.go +++ b/core/artwork/resolve.go @@ -108,6 +108,12 @@ func (r *resolver) resolve(ctx context.Context, item model.ArtworkQueueItem) (re } } +// WalksPriorityChain reports whether the resolver picks this kind's artwork from a configurable +// priority chain; playlists and radios resolve from a fixed order, so there is no walk to explain. +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. func (r *resolver) fetchExternalAlbum(ctx context.Context, al model.Album) (io.ReadCloser, string, bool) { @@ -151,7 +157,7 @@ func (r *resolver) resolveAlbum(ctx context.Context, albumID string) (resolution if res, ok = chain.try(pattern, res, ok); ok { return res, nil } - case pattern == ExternalCandidate: + case pattern == externalCandidate: if rd, name, isErr := r.fetchExternalAlbum(ctx, *al); rd != nil { return resolution{reader: rd, source: ExternalPrefix + name}, nil } else if isErr { @@ -218,7 +224,7 @@ func (r *resolver) resolveArtist(ctx context.Context, artistID string) (resoluti continue } switch { - case pattern == ExternalCandidate: + case pattern == externalCandidate: if rd, name, isErr := r.fetchExternalArtist(ctx, *ar); rd != nil { return resolution{reader: rd, source: ExternalPrefix + name}, nil } else if isErr { @@ -385,7 +391,7 @@ func (r *resolver) resolveMediaFile(ctx context.Context, id string) (resolution, func resolveExternalStep(gate gateFunc, name string, sf sourceFunc) (res resolution, ok bool, extErr bool) { r, path, err := gate(name, sf) if r != nil { - return resolution{reader: r, source: ExternalCandidate, sourcePath: path}, true, false + return resolution{reader: r, source: externalCandidate, sourcePath: path}, true, false } return resolution{}, false, err != nil && !errors.Is(err, model.ErrNotFound) } diff --git a/core/artwork/resolve_test.go b/core/artwork/resolve_test.go index 9a0962cf2..23b1ef436 100644 --- a/core/artwork/resolve_test.go +++ b/core/artwork/resolve_test.go @@ -630,3 +630,15 @@ var _ = Describe("decodeTile", func() { Expect(err).To(HaveOccurred()) }) }) + +var _ = Describe("WalksPriorityChain", func() { + It("is true for the kinds the resolver walks", func() { + Expect(WalksPriorityChain(model.KindArtistArtwork)).To(BeTrue()) + Expect(WalksPriorityChain(model.KindAlbumArtwork)).To(BeTrue()) + }) + + It("is false for the kinds resolved directly", func() { + Expect(WalksPriorityChain(model.KindPlaylistArtwork)).To(BeFalse()) + Expect(WalksPriorityChain(model.KindRadioArtwork)).To(BeFalse()) + }) +}) diff --git a/core/artwork/trace.go b/core/artwork/trace.go index 26e5c7b99..5d02fa4ff 100644 --- a/core/artwork/trace.go +++ b/core/artwork/trace.go @@ -8,8 +8,7 @@ import ( "sync" ) -// Outcome is what the priority chain observed for one candidate. The CLI branches on these, -// so they are part of the package's API, not private labels. +// Outcome is what the priority chain observed for one candidate; the CLI renders and branches on these. type Outcome string const ( @@ -22,10 +21,10 @@ const ( ) const ( - // ExternalCandidate labels the external tier itself, for the cases that never reach an agent. - ExternalCandidate = "external" + // externalCandidate labels the external tier itself, for the cases that never reach an agent. + externalCandidate = "external" // ExternalPrefix qualifies a candidate or a stored source with the agent that produced it. - ExternalPrefix = ExternalCandidate + ":" + ExternalPrefix = externalCandidate + ":" ) // TraceStep is one candidate the priority chain considered. diff --git a/core/artwork/trace_test.go b/core/artwork/trace_test.go index 24c32a4b0..06068323a 100644 --- a/core/artwork/trace_test.go +++ b/core/artwork/trace_test.go @@ -26,7 +26,7 @@ var _ = Describe("trace vocabulary", func() { Expect([]Outcome{ OutcomeHit, OutcomeMiss, OutcomeUnreadable, OutcomeSkipped, OutcomeWouldTry, OutcomeError, }).To(Equal([]Outcome{"hit", "miss", "unreadable", "skipped", "would-try", "error"})) - Expect(ExternalCandidate).To(Equal("external")) + Expect(externalCandidate).To(Equal("external")) Expect(ExternalPrefix).To(Equal("external:")) }) }) @@ -111,11 +111,6 @@ var _ = Describe("chainState tracing", func() { {Candidate: "embedded", Outcome: OutcomeHit, Detail: "/music/a.flac"}, })) }) - - It("does not panic when no trace is attached", func() { - c := chainState{} - Expect(func() { _, _ = c.try("cover.*", resolution{}, false) }).ToNot(Panic()) - }) }) var _ = Describe("external gate tracing", func() { @@ -414,11 +409,6 @@ var _ = Describe("NewTracingResolver", func() { t = &ChainTrace{} }) - It("uses the offline gate when live is false", func() { - r := NewTracingResolver(nil, nil, nil, t, false) - Expect(r).ToNot(BeNil()) - }) - Context("offline", func() { var fake *fakeImageAgent diff --git a/core/artwork/worker_test.go b/core/artwork/worker_test.go index 9fad4b503..53b6a43b2 100644 --- a/core/artwork/worker_test.go +++ b/core/artwork/worker_test.go @@ -362,7 +362,7 @@ var _ = Describe("Worker", func() { Expect(ia.Hash).To(Equal("cafebabe"), "a persistent outage must not discard served art") }) - // Media files are excluded from recheckKinds, so an absent row here would never be + // Media files are excluded from RecheckKinds, so an absent row here would never be // revisited: a transient read error would look permanent. It("does not settle absent on exhaustion for a kind with no recheck path", func() { conf.Server.EnableMediaFileCoverArt = true