mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-12 12:27:09 +02:00
refactor(artwork): own the kind list and the chain predicate in the package
Export RecheckKinds and add WalksPriorityChain so the CLI stops keeping its own copies of both, and unexport externalCandidate, which nothing outside the package consumes.
This commit is contained in:
parent
7a09e3e08d
commit
b04d0c42cc
8 changed files with 37 additions and 32 deletions
|
|
@ -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
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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 != "" {
|
||||
|
|
|
|||
|
|
@ -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
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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())
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue