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

fix(artwork): never retry absent artwork on its own (#6054)

Browse source 
An absent artwork state was revisited by an hourly job, by viewing the entity, and
by the startup backfill on any artwork config change. On a large library the last
one queued tens of thousands of external lookups at once and got the provider to
rate-limit us for hours.

Nothing revisits an absent state now. Retrying is explicit: `artwork reprocess` on
the CLI, or the refresh button in the UI. The config fingerprint survives only as an
advisory, warning at startup and naming the command that clears it.

Since absent is terminal, `artwork status` splits it into two disjoint columns, and
`--source failed` targets only the ones that gave up rather than being answered.
Both read through the filter CountBySource and EnqueueBySource already share, so the
reported number is the set the command acts on.

Also fixes the last_failure default left by 20260819204637, which marked every
pre-existing absent row as failed, and removes the code the deleted retry paths
orphaned.
This commit is contained in:
Deluan Quintão 2026-09-01 20:48:41 -04:00 • committed by GitHub
commit 47bc3c00f3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
35 changed files with 340 additions and 717 deletions
Download patch file Download diff file Expand all files Collapse all files

123
cmd/artwork.go
View file

@ -42,9 +42,10 @@ func init() {
"stored trace of the last resolution; also initializes plugin agents, which may open "+
"external connections")
artworkReprocessCmd.Flags().StringSliceVar(&artworkKinds, "kind", nil,
"kinds to reprocess ("+kindPrefixes(artwork.RecheckKinds)+"); repeatable")
"kinds to reprocess ("+kindPrefixes(artwork.ReprocessKinds)+"); repeatable")
artworkReprocessCmd.Flags().StringSliceVar(&artworkSources, "source", nil,
"only items currently resolved from these sources (e.g. folder, external:deezer, absent)")
"only items currently resolved from these sources (e.g. folder, external:deezer, absent, "+
"or failed for the absent ones that gave up)")
artworkReprocessCmd.Flags().BoolVar(&artworkAll, "all", false, "reprocess every kind")
artworkReprocessCmd.Flags().BoolVar(&artworkDryRun, "dry-run", false,
"report what would be queued and exit without queueing")
@ -113,7 +114,7 @@ var artworkCancelCmd = &cobra.Command{
"Work already picked up is not interrupted, and an item with no artwork yet can be\n" +
"queued again by the hourly re-check. The selection is applied again when you confirm,\n" +
"so anything queued after the preview is cancelled too. Use it to call off a bulk\n" +
"backfill, not to stop the worker.",
"reprocess, not to stop the worker.",
Args: cobra.NoArgs,
Run: func(cmd *cobra.Command, args []string) {
runCancel(cmd.Context())
@ -122,7 +123,7 @@ var artworkCancelCmd = &cobra.Command{
var artworkStatusCmd = &cobra.Command{
Use: "status",
Short: "Report the artwork queue, where artwork resolves from, and the backfill state",
Short: "Report the artwork queue, where artwork resolves from, and the config state",
Args: cobra.NoArgs,
Run: func(cmd *cobra.Command, args []string) {
runStatus(cmd.Context())
@ -146,9 +147,11 @@ type sourceCount struct {
count int64
}
// absentCount partitions a kind's absent states: noImage was answered, failed gave up.
type absentCount struct {
kind model.Kind
model.ArtworkAbsentStat
kind model.Kind
noImage int64
failed int64
}
type statusReport struct {
@ -170,16 +173,6 @@ func queueTotal(stats []model.ArtworkQueueStat) int64 {
return n
}
func (r statusReport) backfillQueued() int64 {
var n int64
for _, s := range r.queue {
if s.Priority == model.ArtworkPriorityBackfill {
n += s.Count
}
}
return n
}
func collectStatus(ctx context.Context, ds model.DataStore) (statusReport, error) {
q := ds.ArtworkQueue(ctx)
var rep statusReport
@ -188,8 +181,7 @@ func collectStatus(ctx context.Context, ds model.DataStore) (statusReport, error
return rep, fmt.Errorf("breaking the artwork queue down by kind: %w", err)
}
cutoff := time.Now().Add(-artwork.StaleAbsentAge)
for _, k := range artwork.RecheckKinds {
for _, k := range artwork.ReprocessKinds {
sources, err := q.SourcesInUse(k)
if err != nil {
return rep, fmt.Errorf("listing the sources in use by %s artwork: %w", k, err)
@ -201,12 +193,15 @@ func collectStatus(ctx context.Context, ds model.DataStore) (statusReport, error
return rep, fmt.Errorf("counting %s artwork resolved from %s: %w", k, displaySource(s), err)
}
rep.sources = append(rep.sources, sourceCount{kind: k, source: s, count: n})
// An absent state is exactly a row with no source, so it needs no second query.
if s == "" {
failed, err := q.CountBySource(k, []string{model.ArtworkSourceFailed})
if err != nil {
return rep, fmt.Errorf("counting failed %s artwork: %w", k, err)
}
rep.absent = append(rep.absent, absentCount{kind: k, noImage: n - failed, failed: failed})
}
}
stat, err := q.CountAbsent(k, cutoff)
if err != nil {
return rep, fmt.Errorf("counting absent %s artwork: %w", k, err)
}
rep.absent = append(rep.absent, absentCount{kind: k, ArtworkAbsentStat: stat})
}
rep.current, rep.inputs = artwork.ConfigFingerprint(), artwork.FingerprintInputs()
@ -234,19 +229,20 @@ func formatStatus(rep statusReport) string {
}
fmt.Fprintln(w, "\nAbsent (resolved, no image found)")
fmt.Fprintln(w, " KIND\tABSENT\tDUE FOR RECHECK")
fmt.Fprintln(w, " KIND\tNO IMAGE\tFAILED")
for _, a := range rep.absent {
fmt.Fprintf(w, " %s\t%d\t%d\n", a.kind, a.Total, a.Stale)
fmt.Fprintf(w, " %s\t%d\t%d\n", a.kind, a.noImage, a.failed)
}
fmt.Fprintf(w, " (eligible once the last attempt is older than %gh; re-queued %d per kind per hour, oldest first)\n",
artwork.StaleAbsentAge.Hours(), artwork.StaleAbsentRecheckBatch)
fmt.Fprintln(w, " (nothing retries these; 'artwork reprocess --source absent' retries both columns)")
fmt.Fprintln(w, " (failed = gave up rather than being answered, so the ones most likely to resolve;\n"+
" 'artwork reprocess --source failed' retries just those)")
fmt.Fprintln(w, "\nBackfill")
fmt.Fprintf(w, " State:\t%s\n", backfillState(rep))
fmt.Fprintln(w, "\nConfig")
fmt.Fprintf(w, " State:\t%s\n", configState(rep))
fmt.Fprintf(w, " Stored fingerprint:\t%s\n", cmp.Or(rep.stored, "(none)"))
fmt.Fprintf(w, " Current fingerprint:\t%s\n", rep.current)
if len(rep.inputs) > 0 {
fmt.Fprintln(w, " Fingerprint inputs (changing any of these re-resolves the whole library):")
fmt.Fprintln(w, " Fingerprint inputs (changing any of these makes the stored artwork stale):")
for _, in := range rep.inputs {
fmt.Fprintf(w, " %s:\t%s\n", in.Name, in.Value)
}
@ -256,18 +252,10 @@ func formatStatus(rep statusReport) string {
return sb.String()
}
// backfillState leads with the queued backlog: by the time anyone runs this, backfill has usually
// already stored the new fingerprint, and "up to date" would bury the flood it is still working through.
func backfillState(rep statusReport) string {
pending := "fingerprint changed — every artist, album, playlist and radio will be re-enqueued on the next startup"
if n := rep.backfillQueued(); n > 0 {
if rep.stored != rep.current {
return fmt.Sprintf("backfill running: %d items queued, and %s", n, pending)
}
return fmt.Sprintf("backfill running: %d items queued (fingerprint up to date)", n)
}
func configState(rep statusReport) string {
if rep.stored != rep.current {
return pending
return "fingerprint changed — stored artwork keeps the old resolution; " +
"run 'artwork reprocess --all' to apply it"
}
return "up to date"
}
@ -297,8 +285,8 @@ type artworkPriority struct {
var knownPriorities = []artworkPriority{
{"bump", model.ArtworkPriorityBump},
{"scan", model.ArtworkPriorityScan},
{"backfill", model.ArtworkPriorityBackfill},
{"recheck", model.ArtworkPriorityRecheck},
{"backfill", model.ArtworkPriorityBackfill},
}
// priorityName falls back to the number: a row written by a newer version still has to print.
@ -351,29 +339,41 @@ func runReprocess(ctx context.Context) {
func selectedKinds(kinds, sources []string, all bool) ([]model.Kind, error) {
// A source filter on its own is already a complete selection, so it does not also need a kind.
if all || (len(kinds) == 0 && len(sources) > 0) {
return artwork.RecheckKinds, nil
return artwork.ReprocessKinds, nil
}
if len(kinds) == 0 {
return nil, fmt.Errorf("no selector given: pass --kind, --source or --all")
}
return parseAll(kinds, func(s string) (model.Kind, error) {
return parseArtworkKind(s, artwork.RecheckKinds)
return parseArtworkKind(s, artwork.ReprocessKinds)
})
}
// absentSource is how the stored empty source — resolved, no image — is spelled on the CLI.
const absentSource = "absent"
// absentSource is how the stored empty source — resolved, no image — is spelled on the CLI, and
// failedSource the subset of it that gave up rather than being answered.
const (
absentSource = "absent"
failedSource = "failed"
)
func repositorySources(sources []string) []string {
return slice.Map(sources, func(s string) string {
if s == absentSource {
switch s {
case absentSource:
return ""
case failedSource:
return model.ArtworkSourceFailed
}
return s
})
}
func displaySource(s string) string { return cmp.Or(s, absentSource) }
func displaySource(s string) string {
if s == model.ArtworkSourceFailed {
return failedSource
}
return cmp.Or(s, absentSource)
}
type confirmFunc func(out io.Writer, total, external int64) bool
@ -447,7 +447,7 @@ func validateSources(q model.ArtworkQueueRepository, sources []string) error {
return nil
}
var inUse []string
for _, k := range artwork.RecheckKinds {
for _, k := range artwork.ReprocessKinds {
found, err := q.SourcesInUse(k)
if err != nil {
return fmt.Errorf("listing the sources in use by %s artwork: %w", k, err)
@ -456,14 +456,16 @@ func validateSources(q model.ArtworkQueueRepository, sources []string) error {
}
var unknown []string
for _, s := range sources {
if s != "" && !slices.Contains(inUse, s) { // the reserved absent source is valid even when nothing is absent
// The reserved absent and failed sources are valid even when nothing currently matches them.
if s != "" && s != model.ArtworkSourceFailed && !slices.Contains(inUse, s) {
unknown = append(unknown, displaySource(s))
}
}
if len(unknown) == 0 {
return nil
}
valid := slice.Map(inUse, displaySource)
// failed is accepted but never stored, so listing only what is in use would hide it.
valid := append(slice.Map(inUse, displaySource), failedSource)
slices.Sort(valid)
return fmt.Errorf("no artwork resolves from %s; sources in use: %s",
strings.Join(unknown, ", "), cmp.Or(strings.Join(valid, ", "), "(none)"))
@ -478,6 +480,18 @@ func reprocessArtwork(ctx context.Context, ds model.DataStore, kinds []model.Kin
return err
}
// Derived from what actually drives the queries, so a filter added to this signature cannot
// silently keep stamping the fingerprint for a partial run.
markApplied := func() error {
if len(sources) > 0 || len(kinds) < len(artwork.ReprocessKinds) {
return nil
}
if err := artwork.MarkConfigApplied(ctx, ds); err != nil {
return fmt.Errorf("recording the applied artwork config: %w", err)
}
return nil
}
matched := make([]int64, len(kinds))
var total, external int64
for i, k := range kinds {
@ -496,8 +510,9 @@ func reprocessArtwork(ctx context.Context, ds model.DataStore, kinds []model.Kin
fmt.Fprintln(out, "\nDry run: nothing was queued.")
return nil
case total == 0:
// An empty match set still leaves nothing resolved under the old config.
fmt.Fprintln(out, "Nothing was queued.")
return nil
return markApplied()
case !confirm(out, total, external):
fmt.Fprintln(out, "Aborted: nothing was queued.")
return nil
@ -519,7 +534,7 @@ func reprocessArtwork(ctx context.Context, ds model.DataStore, kinds []model.Kin
if skipped := total - queued; skipped > 0 {
fmt.Fprintf(out, "Already queued, left unchanged: %d (priority and retry backoff untouched).\n", skipped)
}
return nil
return markApplied()
}
func runCancel(ctx context.Context) {
@ -546,7 +561,7 @@ func cancelSelection(kinds, priorities []string, all bool) ([]model.Kind, []int,
if len(kinds) == 0 && len(priorities) == 0 {
return nil, nil, fmt.Errorf("no selector given: pass --kind, --priority or --all")
}
// RefreshableKinds, not RecheckKinds: media files are queued, so --kind must reach them.
// RefreshableKinds, not ReprocessKinds: media files are queued, so --kind must reach them.
outKinds, err := parseAll(kinds, func(s string) (model.Kind, error) {
return parseArtworkKind(s, artwork.RefreshableKinds)
})

102
cmd/artwork_test.go
View file

@ -3,7 +3,6 @@ package cmd
import (
"context"
"errors"
"fmt"
"io"
"strings"
"time"
@ -20,20 +19,20 @@ import (
var _ = Describe("parseArtworkKind", func() {
It("accepts a supported kind", func() {
k, err := parseArtworkKind("ar", artwork.RecheckKinds)
k, err := parseArtworkKind("ar", artwork.ReprocessKinds)
Expect(err).ToNot(HaveOccurred())
Expect(k).To(Equal(model.KindArtistArtwork))
})
It("rejects an unknown kind and lists the valid ones", func() {
_, err := parseArtworkKind("zz", artwork.RecheckKinds)
_, err := parseArtworkKind("zz", artwork.ReprocessKinds)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("ar"))
Expect(err.Error()).To(ContainSubstring("al"))
})
It("rejects a known kind the command does not accept", func() {
_, err := parseArtworkKind("mf", artwork.RecheckKinds)
_, err := parseArtworkKind("mf", artwork.ReprocessKinds)
Expect(err).To(HaveOccurred())
})
@ -442,13 +441,13 @@ var _ = Describe("artwork reprocess selection", func() {
It("returns every kind for --all", func() {
ks, err := selectedKinds(nil, nil, true)
Expect(err).ToNot(HaveOccurred())
Expect(ks).To(ConsistOf(artwork.RecheckKinds))
Expect(ks).To(ConsistOf(artwork.ReprocessKinds))
})
It("returns every kind for a source filter given without a kind", func() {
ks, err := selectedKinds(nil, []string{"folder"}, false)
Expect(err).ToNot(HaveOccurred())
Expect(ks).To(ConsistOf(artwork.RecheckKinds), "--source alone is already a complete selection")
Expect(ks).To(ConsistOf(artwork.ReprocessKinds), "--source alone is already a complete selection")
})
It("returns only the named kinds", func() {
@ -512,6 +511,11 @@ var _ = Describe("repositorySources", func() {
Expect(repositorySources([]string{"absent", "folder"})).To(Equal([]string{"", "folder"}))
})
It("maps the failed name onto the pseudo-source, and back for display", func() {
Expect(repositorySources([]string{failedSource})).To(Equal([]string{model.ArtworkSourceFailed}))
Expect(displaySource(model.ArtworkSourceFailed)).To(Equal(failedSource))
})
It("keeps an empty selection empty, meaning every source", func() {
Expect(repositorySources(nil)).To(BeEmpty())
})
@ -607,6 +611,25 @@ var _ = Describe("reprocessArtwork", func() {
Expect(queue.Count()).To(BeZero())
})
DescribeTable("records the applied config only for a run that leaves nothing on the old one",
func(selected []model.Kind, sources []string, dryRun, applied bool) {
Expect(ds.Property(ctx).Put(consts.ArtConfFingerprintPropertyKey, "stale-fingerprint")).To(Succeed())
Expect(reprocessArtwork(ctx, ds, selected, sources, imageAgents, dryRun, accept, &out)).To(Succeed())
want := "stale-fingerprint"
if applied {
want = artwork.ConfigFingerprint()
}
Expect(ds.Property(ctx).Get(consts.ArtConfFingerprintPropertyKey)).To(Equal(want))
},
Entry("every kind, unfiltered", artwork.ReprocessKinds, nil, false, true),
Entry("every kind, but nothing matched", artwork.ReprocessKinds, []string{}, false, true),
Entry("filtered by source", artwork.ReprocessKinds, []string{"external:deezer"}, false, false),
Entry("a subset of kinds", []model.Kind{model.KindAlbumArtwork}, nil, false, false),
Entry("a dry run applies nothing", artwork.ReprocessKinds, nil, true, false),
)
It("queues the matching items at recheck priority, leaving their artwork state alone", func() {
Expect(reprocessArtwork(ctx, ds, kinds, []string{"external:deezer"}, imageAgents, false, accept, &out)).To(Succeed())
@ -760,6 +783,14 @@ var _ = Describe("reprocessArtwork", func() {
imageAgents, true, accept, &out)).ToNot(Succeed(), "a typo must still be rejected")
})
It("names failed among the valid sources when rejecting a typo", func() {
err := reprocessArtwork(ctx, ds, kinds, []string{"faild"}, imageAgents, true, accept, &out)
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("failed"),
"failed is accepted but never stored, so it has to be named explicitly")
})
It("accepts a source another kind uses, letting the empty selection report itself", func() {
Expect(reprocessArtwork(ctx, ds, []model.Kind{model.KindArtistArtwork}, []string{"folder"},
imageAgents, false, decline, &out)).To(Succeed())
@ -792,14 +823,16 @@ var _ = Describe("collectStatus", func() {
ImageType: model.ImageTypePrimary, Source: source, Hash: hash, AttemptedAt: attempted})).To(Succeed())
}
put(model.KindArtistArtwork, "ar-1", "external:deezer", "h1", time.Now())
put(model.KindArtistArtwork, "ar-2", "", "", time.Now().Add(-artwork.StaleAbsentAge-time.Hour))
put(model.KindArtistArtwork, "ar-3", "", "", time.Now())
put(model.KindArtistArtwork, "ar-2", "", "", time.Now().Add(-24*time.Hour))
// ar-3 is absent because it gave up, so the two absent artists split across the columns.
Expect(art.PutItemArtwork(&model.ItemArtwork{ItemKind: "ar", ItemID: "ar-3",
ImageType: model.ImageTypePrimary, LastFailure: "[]", AttemptedAt: time.Now()})).To(Succeed())
put(model.KindAlbumArtwork, "al-1", "folder", "h2", time.Now())
Expect(queue.Enqueue(model.ArtworkQueueItem{ItemKind: "ar", ItemID: "ar-9",
ImageType: model.ImageTypePrimary, Priority: model.ArtworkPriorityBackfill})).To(Succeed())
})
It("reports the queue, the source distribution and the absent ages", func() {
It("reports the queue, the source distribution and the absent totals", func() {
rep, err := collectStatus(ctx, ds)
Expect(err).ToNot(HaveOccurred())
@ -810,8 +843,8 @@ var _ = Describe("collectStatus", func() {
sourceCount{kind: model.KindArtistArtwork, source: "", count: 2},
sourceCount{kind: model.KindAlbumArtwork, source: "folder", count: 1},
))
Expect(rep.absent).To(ContainElement(absentCount{kind: model.KindArtistArtwork,
ArtworkAbsentStat: model.ArtworkAbsentStat{Total: 2, Stale: 1}}))
Expect(rep.absent).To(ContainElement(absentCount{kind: model.KindArtistArtwork, noImage: 1, failed: 1}),
"two absent artists, one answered and one that gave up")
})
It("compares the stored fingerprint against the current one", func() {
@ -845,7 +878,7 @@ var _ = Describe("formatStatus", func() {
{kind: model.KindArtistArtwork, source: "", count: 2},
},
absent: []absentCount{
{kind: model.KindArtistArtwork, ArtworkAbsentStat: model.ArtworkAbsentStat{Total: 2, Stale: 1}},
{kind: model.KindArtistArtwork, noImage: 1, failed: 1},
},
inputs: []artwork.FingerprintInput{{Name: "Agents", Value: "deezer,lastfm"}},
stored: "abc123",
@ -878,50 +911,37 @@ var _ = Describe("formatStatus", func() {
Expect(sources).To(MatchRegexp(`artist\s+absent\s+2`))
})
It("prints the absent total and how many are due for recheck", func() {
absent := block(formatStatus(rep), "Absent (resolved, no image found)")
Expect(absent).To(MatchRegexp(`artist\s+2\s+1`))
It("partitions the absent states into answered and gave up", func() {
out := block(formatStatus(rep), "Absent (resolved, no image found)")
Expect(out).To(ContainSubstring("NO IMAGE"))
Expect(out).To(MatchRegexp(`artist\s+1\s+1`), "1 answered plus 1 failed, summing to 2 absent")
Expect(formatStatus(rep)).To(ContainSubstring("artwork reprocess --source failed"))
})
It("states the recheck window and the drip rate the absent counts are bucketed against", func() {
Expect(formatStatus(rep)).To(ContainSubstring(fmt.Sprintf("%gh", artwork.StaleAbsentAge.Hours())))
Expect(formatStatus(rep)).To(ContainSubstring("100 per kind per hour"))
It("says absent states are never retried on their own, and names both commands that do", func() {
out := formatStatus(rep)
Expect(out).To(ContainSubstring("nothing retries these"))
Expect(out).To(ContainSubstring("artwork reprocess --source absent"))
Expect(out).To(ContainSubstring("artwork reprocess --source failed"))
})
It("leads with the queued backlog, which is the finding, not with the fingerprint verdict", func() {
out := block(formatStatus(rep), "Backfill")
Expect(out).To(MatchRegexp(`State:\s+backfill running: 2 items queued`),
"an operator scanning for trouble must not read 'up to date' while 2 items churn")
Expect(out).To(ContainSubstring("fingerprint up to date"))
})
It("keeps the re-enqueue warning while a backfill is already running", func() {
rep.stored = "older"
out := block(formatStatus(rep), "Backfill")
Expect(out).To(MatchRegexp(`State:\s+backfill running: 2 items queued`))
Expect(out).To(ContainSubstring("re-enqueued"),
"the stored fingerprint is still stale, so a second full re-enqueue is pending on top of this one")
})
It("reports up to date only once the backfill has drained", func() {
rep.queue = []model.ArtworkQueueStat{{ItemKind: "al", Priority: model.ArtworkPriorityScan, Count: 1}}
Expect(block(formatStatus(rep), "Backfill")).To(MatchRegexp(`State:\s+up to date`))
It("reports a matching fingerprint as up to date, whatever else is queued", func() {
Expect(block(formatStatus(rep), "Config")).To(MatchRegexp(`State:\s+up to date`))
})
It("echoes the config inputs a fingerprint change would have come from", func() {
out := block(formatStatus(rep), "Backfill")
out := block(formatStatus(rep), "Config")
Expect(out).To(MatchRegexp(`Agents:\s+deezer,lastfm`))
Expect(out).To(ContainSubstring("abc123"), "the fingerprint values themselves must be printed")
})
It("reports a changed fingerprint as a pending re-resolve of everything", func() {
It("reports a changed fingerprint as stale artwork, and names the command that applies it", func() {
rep.stored = "older"
rep.queue = nil
out := formatStatus(rep)
Expect(out).To(ContainSubstring("fingerprint changed"))
Expect(out).To(ContainSubstring("artwork reprocess --all"))
Expect(out).ToNot(ContainSubstring("up to date"))
})
@ -1015,7 +1035,7 @@ var _ = Describe("needsImageAgents", func() {
It("is false once the chains no longer reach an agent", func() {
conf.Server.CoverArtPriority = "cover.*"
conf.Server.ArtistArtPriority = "artist.*"
Expect(needsImageAgents(artwork.RecheckKinds)).To(BeFalse())
Expect(needsImageAgents(artwork.ReprocessKinds)).To(BeFalse())
})
})

30
cmd/root.go
View file

@ -366,21 +366,18 @@ func startArtworkWorker(ctx context.Context, worker *artwork.Worker) func() erro
}
}
// scheduleArtworkHousekeeping runs the startup fingerprint backfill and registers the
// recurring stale-absent recheck and prune jobs.
// scheduleArtworkHousekeeping registers the recurring missing-state and prune jobs, and
// reports an artwork config change without acting on it.
func scheduleArtworkHousekeeping(ctx context.Context, worker *artwork.Worker) func() error {
return func() error {
schedulerInstance := scheduler.GetInstance()
if _, err := schedulerInstance.Add(consts.ArtworkStaleAbsentRecheckSchedule, func() {
if err := worker.EnqueueStaleAbsentAll(ctx); err != nil {
log.Error(ctx, "Error enqueueing stale artwork rechecks", err)
}
if _, err := schedulerInstance.Add(consts.ArtworkEnqueueMissingSchedule, func() {
if err := worker.EnqueueMissingAll(ctx); err != nil {
log.Error(ctx, "Error enqueueing missing artwork rechecks", err)
}
}); err != nil {
log.Error(ctx, "Error scheduling artwork stale-absent recheck", err)
log.Error(ctx, "Error scheduling artwork missing-state recheck", err)
}
if _, err := schedulerInstance.Add(consts.ArtworkPruneSchedule, func() {
@ -397,23 +394,8 @@ func scheduleArtworkHousekeeping(ctx context.Context, worker *artwork.Worker) fu
log.Error(ctx, "Error enqueueing missing artwork rechecks", err)
}
backfilled, err := worker.Backfill(ctx)
if err != nil {
log.Error(ctx, "Error running artwork backfill", err)
return nil
}
if !backfilled {
return nil
}
log.Info(ctx, "Artwork backfill enqueued, scheduling a follow-up prune")
timer := time.NewTimer(consts.ArtworkPostBackfillPruneDelay)
defer timer.Stop()
select {
case <-timer.C:
if err := worker.RunPrune(ctx); err != nil {
log.Error(ctx, "Error running post-backfill artwork prune", err)
}
case <-ctx.Done():
if err := worker.ReconcileConfig(ctx); err != nil {
log.Error(ctx, "Error checking the artwork config fingerprint", err)
}
return nil
}

9
consts/consts.go
View file

@ -24,8 +24,8 @@ const (
LastDBAnalyzeAttemptAtKey = "LastDBAnalyzeAttemptAt"
DBAnalyzePendingKey = "DBAnalyzePending"
DBAnalyzeFailureCountKey = "DBAnalyzeFailureCount"
// ArtConfFingerprintPropertyKey is the model.PropertyRepository key Backfill compares against
// to detect artwork-affecting config changes across restarts.
// ArtConfFingerprintPropertyKey is the model.PropertyRepository key the artwork config check
// compares against to detect artwork-affecting config changes across restarts.
ArtConfFingerprintPropertyKey = "ArtConfFingerprint"
UIAuthorizationHeader = "X-ND-Authorization"
@ -39,9 +39,8 @@ const (
DBAnalyzeCheckSchedule = "@every 30m"
DBAnalyzeMaxAge = 24 * time.Hour
ArtworkStaleAbsentRecheckSchedule = "@every 1h"
ArtworkPruneSchedule = "@daily"
ArtworkPostBackfillPruneDelay = 10 * time.Minute
ArtworkEnqueueMissingSchedule = "@every 1h"
ArtworkPruneSchedule = "@daily"
// DefaultEncryptionKey This is the encryption key used if none is specified in the `PasswordEncryptionKey` option
// Never ever change this! Or it will break all Navidrome installations that don't set the config option

9
core/artwork/artwork.go
View file

@ -118,10 +118,6 @@ func (s *service) Get(ctx context.Context, artID model.ArtworkID, size int, squa
}
}
// requestRecheckAge throttles view-triggered rechecks so reopening a genuinely-absent page can't
// hammer external services; below StaleAbsentAge to catch younger absences.
const requestRecheckAge = time.Hour
func (s *service) serveEntity(ctx context.Context, artID model.ArtworkID, size int, square bool) (*Image, error) {
ia, err := s.ds.Artwork(ctx).GetItemArtwork(artID.Kind, artID.ID, model.ImageTypePrimary)
switch {
@ -130,10 +126,7 @@ func (s *service) serveEntity(ctx context.Context, artID model.ArtworkID, size i
case err != nil:
return nil, err
case ia.Hash == "":
// Inserts an immediately-eligible recheck for a settled absent row.
if time.Since(ia.AttemptedAt) > requestRecheckAge {
s.enqueue(ctx, artID, model.ArtworkPriorityBump)
}
// Settled absent: only an explicit reprocess or refresh retries it.
return nil, ErrUnavailable
default:
return s.serveHash(ctx, artID, ia, size, square)

17
core/artwork/artwork_test.go
View file

@ -204,28 +204,15 @@ var _ = Describe("Artwork", func() {
Expect(err).To(MatchError(ErrUnavailable))
})
It("does not re-enqueue a recently-attempted absent state", func() {
It("never re-enqueues an absent state on view, however old", func() {
Expect(artRepo.PutItemArtwork(&model.ItemArtwork{
ItemKind: "al", ItemID: "al4", AttemptedAt: time.Now(),
ItemKind: "al", ItemID: "al4", AttemptedAt: time.Now().Add(-365 * 24 * time.Hour),
})).To(Succeed())
_, err := svc.Get(ctx, model.MustParseArtworkID("al-al4"), 0, false)
Expect(err).To(MatchError(ErrUnavailable))
Expect(queueRepo.Data).To(BeEmpty())
})
It("promotes a stale absent state at Bump priority on view", func() {
Expect(artRepo.PutItemArtwork(&model.ItemArtwork{
ItemKind: "al", ItemID: "al4b", AttemptedAt: time.Now().Add(-2 * requestRecheckAge),
})).To(Succeed())
_, err := svc.Get(ctx, model.MustParseArtworkID("al-al4b"), 0, false)
Expect(err).To(MatchError(ErrUnavailable))
Expect(queueRepo.Data[primaryKey("al", "al4b")].Priority).To(Equal(model.ArtworkPriorityBump))
ia, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "al4b", model.ImageTypePrimary)
Expect(err).ToNot(HaveOccurred())
Expect(ia.Hash).To(BeEmpty())
})
})
Describe("provisional read-through", func() {

125
core/artwork/housekeeping.go
View file

@ -6,26 +6,18 @@ import (
"slices"
"strconv"
"strings"
"time"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/consts"
"github.com/navidrome/navidrome/core/auth"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/utils/slice"
"github.com/zeebo/xxh3"
)
// StaleAbsentAge is how long an absent state is trusted before a recheck retries it.
const StaleAbsentAge = 30 * 24 * time.Hour
// StaleAbsentRecheckBatch caps how many absent states each hourly tick re-queues per kind,
// oldest first, so external agents see a flat drip instead of a daily burst.
const StaleAbsentRecheckBatch = 100
// RecheckKinds omits media files: they resolve embedded-only, at scan or on view.
var RecheckKinds = []model.Kind{
// ReprocessKinds omits media files: they resolve embedded-only, at scan or on view. Artists lead
// so bulk enqueues give the most external-dependent kind a queue headstart.
var ReprocessKinds = []model.Kind{
model.KindArtistArtwork, model.KindAlbumArtwork, model.KindPlaylistArtwork, model.KindRadioArtwork,
}
@ -34,14 +26,16 @@ var RecheckKinds = []model.Kind{
func KeepsState(kind model.Kind) bool { return kind != model.KindDiscArtwork }
// RefreshableKinds is every kind Refresh can clear and re-queue, so it holds exactly the kinds
// KeepsState admits. Media files are absent from RecheckKinds but belong here: the worker
// resolves them, it just never revisits them on its own.
var RefreshableKinds = append(slices.Clone(RecheckKinds), model.KindMediaFileArtwork)
// KeepsState admits. Media files are absent from ReprocessKinds but belong here: the worker
// resolves them, it just never enumerates them in bulk.
var RefreshableKinds = append(slices.Clone(ReprocessKinds), model.KindMediaFileArtwork)
// hasRecheckPath reports whether a periodic job will revisit this kind, making an absent settle recoverable.
func hasRecheckPath(prefix string) bool {
// settlesAbsentOnGiveUp reports whether an exhausted retry budget records an absent state. Media
// files are excluded because retrying one costs nothing: they resolve embedded-only, from a local
// read, and only a view ever enqueues them.
func settlesAbsentOnGiveUp(prefix string) bool {
kind, ok := model.ParseKind(prefix)
return ok && slices.Contains(RecheckKinds, kind)
return ok && KeepsState(kind) && kind != model.KindMediaFileArtwork
}
// artworkEpoch invalidates all resolution state when bumped; bump it whenever resolution semantics change.
@ -72,93 +66,36 @@ func ConfigFingerprint() string {
return fmt.Sprintf("%016x", xxh3.Hash([]byte(raw)))
}
// backfillSummary is what a backfill enqueued. MaxExternalLookups is an upper estimate for one
// attempt per item, not a bound: a local hit ends the walk, and a retry asks the agents again.
type backfillSummary struct {
Ran bool
PerKind map[string]int64
Items int64
MaxExternalLookups int64
}
// backfill enqueues artwork resolution for every entity when the config fingerprint changed.
func backfill(ctx context.Context, ds model.DataStore, agentCount func() ImageAgentCount) (backfillSummary, error) {
start := time.Now()
ctx = auth.WithAdminUser(ctx, ds)
// ReconcileConfigFingerprint warns when the artwork config changed since the library was last
// resolved under it. Nothing re-resolves on its own; applying a change is an explicit reprocess.
func ReconcileConfigFingerprint(ctx context.Context, ds model.DataStore) error {
current := ConfigFingerprint()
props := ds.Property(ctx)
stored, err := props.DefaultGet(consts.ArtConfFingerprintPropertyKey, "")
stored, err := ds.Property(ctx).DefaultGet(consts.ArtConfFingerprintPropertyKey, "")
if err != nil {
return backfillSummary{}, err
return err
}
if stored == current {
return backfillSummary{}, nil
}
// Artists first: few entities, most external-dependent, so they get a queue headstart.
kinds := []struct {
kind model.Kind
fetch func() ([]string, error)
}{
{model.KindArtistArtwork, func() ([]string, error) { return ds.Artist(ctx).GetAllIDs() }},
{model.KindAlbumArtwork, func() ([]string, error) { return ds.Album(ctx).GetAllIDs() }},
{model.KindPlaylistArtwork, func() ([]string, error) { return ds.Playlist(ctx).GetAllIDs() }},
{model.KindRadioArtwork, func() ([]string, error) { return ds.Radio(ctx).GetAllIDs() }},
}
// Counted here, not by the caller: building the agent list constructs every enabled agent, and
// an unchanged fingerprint returns above without ever needing the number.
agents := agentCount()
summary := backfillSummary{Ran: true, PerKind: map[string]int64{}}
for _, k := range kinds {
ids, err := k.fetch()
if err != nil {
return backfillSummary{}, err
}
if err := enqueueBackfillKind(ctx, ds, k.kind, ids); err != nil {
return backfillSummary{}, err
}
n := int64(len(ids))
summary.PerKind[k.kind.Prefix()] = n
summary.Items += n
summary.MaxExternalLookups += n * ExternalLookupsPerItem(k.kind, agents)
}
if err := props.Put(consts.ArtConfFingerprintPropertyKey, current); err != nil {
return backfillSummary{}, err
}
log.Info(ctx, "Artwork: Config fingerprint changed, backfill enqueued", "items", summary.Items,
"byKind", summary.PerKind, "maxExternalLookups", summary.MaxExternalLookups,
"elapsed", time.Since(start))
return summary, nil
}
func enqueueBackfillKind(ctx context.Context, ds model.DataStore, kind model.Kind, ids []string) error {
if len(ids) == 0 {
return nil
}
items := slice.Map(ids, func(id string) model.ArtworkQueueItem {
return model.ArtworkQueueItem{
ItemKind: kind.Prefix(), ItemID: id, ImageType: model.ImageTypePrimary, Priority: model.ArtworkPriorityBackfill,
}
})
return ds.ArtworkQueue(ctx).Enqueue(items...)
}
func enqueueStaleAbsentAll(ctx context.Context, ds model.DataStore) error {
cutoff := time.Now().Add(-StaleAbsentAge)
queue := ds.ArtworkQueue(ctx)
for _, kind := range RecheckKinds {
if _, err := queue.EnqueueStaleAbsent(kind, cutoff, StaleAbsentRecheckBatch); err != nil {
return err
}
switch stored {
case current:
case "":
// An unset fingerprint counts as current; the alternative warns every upgrading install once.
return MarkConfigApplied(ctx, ds)
default:
log.Warn(ctx, "Artwork: Config changed since the last full reprocess. Stored artwork keeps "+
"the old resolution; run 'navidrome artwork reprocess --all' to apply the change",
"stored", stored, "current", current, "inputs", FingerprintInputs())
}
return nil
}
// MarkConfigApplied records the current fingerprint as the one the library is resolved under.
func MarkConfigApplied(ctx context.Context, ds model.DataStore) error {
return ds.Property(ctx).Put(consts.ArtConfFingerprintPropertyKey, ConfigFingerprint())
}
// 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 ReprocessKinds {
if _, err := queue.EnqueueAllMissing(kind, model.ArtworkPriorityRecheck); err != nil {
return err
}

248
core/artwork/housekeeping_test.go
View file

@ -2,59 +2,17 @@ package artwork
import (
"context"
"fmt"
"slices"
"time"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/conf/configtest"
"github.com/navidrome/navidrome/consts"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/request"
"github.com/navidrome/navidrome/tests"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
// visibilityPlaylistDS models playlist_repository's userFilter: a private playlist is only
// visible when the ctx carries an admin, so headless work must wrap ctx with one first.
type visibilityPlaylistDS struct {
*tests.MockDataStore
private model.Playlist
tracks model.PlaylistTrackRepository
}
func (v *visibilityPlaylistDS) Playlist(ctx context.Context) model.PlaylistRepository {
repo := tests.CreateMockPlaylistRepo()
repo.TracksRepo = v.tracks
if u, ok := request.UserFrom(ctx); ok && u.IsAdmin {
repo.SetData(model.Playlists{v.private})
}
return repo
}
func adminUserRepo() *tests.MockedUserRepo {
repo := tests.CreateMockUserRepo()
Expect(repo.Put(&model.User{ID: "admin", UserName: "admin", IsAdmin: true})).To(Succeed())
return repo
}
func noAgents() ImageAgentCount { return ImageAgentCount{} }
// orderTrackingQueueRepo records the item kind of each Enqueue call, so tests can
// assert phase ordering (artists-first) that same-priority timestamps can't guarantee.
type orderTrackingQueueRepo struct {
*tests.MockArtworkQueueRepo
callKinds []string
}
func (o *orderTrackingQueueRepo) Enqueue(items ...model.ArtworkQueueItem) error {
if len(items) > 0 {
o.callKinds = append(o.callKinds, items[0].ItemKind)
}
return o.MockArtworkQueueRepo.Enqueue(items...)
}
var _ = Describe("RefreshableKinds", func() {
// The two are meant to describe the same fact. Nothing but this test stops them from drifting,
// and a drift would have `artwork explain` report state for a kind that keeps none.
@ -72,7 +30,7 @@ var _ = Describe("Housekeeping", func() {
var (
ctx context.Context
ds *tests.MockDataStore
queueRepo *orderTrackingQueueRepo
queueRepo *tests.MockArtworkQueueRepo
propRepo *tests.MockedPropertyRepo
)
@ -84,52 +42,24 @@ var _ = Describe("Housekeeping", func() {
conf.Server.Agents = "spotify"
conf.Server.EnableExternalServices = true
queueRepo = &orderTrackingQueueRepo{MockArtworkQueueRepo: tests.CreateMockArtworkQueueRepo()}
queueRepo = tests.CreateMockArtworkQueueRepo()
propRepo = &tests.MockedPropertyRepo{}
ds = &tests.MockDataStore{MockedArtworkQueue: queueRepo, MockedProperty: propRepo}
})
seedEntities := func() {
artistRepo := tests.CreateMockArtistRepo()
artistRepo.SetData(model.Artists{{ID: "ar1"}, {ID: "ar2"}})
ds.MockedArtist = artistRepo
albumRepo := tests.CreateMockAlbumRepo()
albumRepo.SetData(model.Albums{{ID: "al1"}})
ds.MockedAlbum = albumRepo
playlistRepo := tests.CreateMockPlaylistRepo()
playlistRepo.SetData(model.Playlists{{ID: "pl1"}})
ds.MockedPlaylist = playlistRepo
radioRepo := tests.CreateMockedRadioRepo()
radioRepo.All = model.Radios{{ID: "ra1"}}
ds.MockedRadio = radioRepo
}
Describe("Fingerprint", func() {
It("changes when a fingerprint-affecting config value changes", func() {
f1 := ConfigFingerprint()
conf.Server.CoverArtPriority = "folder, embedded"
f2 := ConfigFingerprint()
Expect(f1).NotTo(Equal(f2))
})
DescribeTable("changes when a fingerprint-affecting config value changes",
func(change func()) {
before := ConfigFingerprint()
change()
Expect(ConfigFingerprint()).NotTo(Equal(before))
},
Entry("CoverArtPriority", func() { conf.Server.CoverArtPriority = "folder, embedded" }),
Entry("ArtistImageFolder", func() { conf.Server.ArtistImageFolder = "/after" }),
Entry("EnableM3UExternalAlbumArt", func() { conf.Server.EnableM3UExternalAlbumArt = true }),
)
It("changes when ArtistImageFolder changes", func() {
conf.Server.ArtistImageFolder = "/before"
f1 := ConfigFingerprint()
conf.Server.ArtistImageFolder = "/after"
Expect(ConfigFingerprint()).NotTo(Equal(f1))
})
It("changes when EnableM3UExternalAlbumArt is toggled", func() {
conf.Server.EnableM3UExternalAlbumArt = false
f1 := ConfigFingerprint()
conf.Server.EnableM3UExternalAlbumArt = true
Expect(ConfigFingerprint()).NotTo(Equal(f1))
})
// Pinned: a changed formula re-resolves every library on upgrade, flooding external providers.
// Pinned: a changed formula tells every existing install its artwork config went stale.
It("hashes a given config to a stable value", func() {
conf.Server.CoverArtPriority = "cover.*, embedded"
conf.Server.ArtistArtPriority = "artist.*, external"
@ -157,145 +87,23 @@ var _ = Describe("Housekeeping", func() {
f1 := ConfigFingerprint()
consts.Version = original + "-next"
Expect(ConfigFingerprint()).To(Equal(f1),
"the version must not invalidate artwork state: it would re-resolve every entity on every build")
"the version must not invalidate artwork state: every build would report a stale config")
})
})
Describe("Backfill", func() {
It("enqueues nothing and returns false when the stored fingerprint matches", func() {
seedEntities()
Expect(propRepo.Put(consts.ArtConfFingerprintPropertyKey, ConfigFingerprint())).To(Succeed())
Describe("ReconcileConfigFingerprint", func() {
It("records the current fingerprint when none was ever stored", func() {
Expect(ReconcileConfigFingerprint(ctx, ds)).To(Succeed())
counted := false
s, err := backfill(ctx, ds, func() ImageAgentCount {
counted = true
return ImageAgentCount{Artist: 3, Album: 2}
})
Expect(err).ToNot(HaveOccurred())
Expect(s).To(Equal(backfillSummary{}))
Expect(counted).To(BeFalse(), "building the agent list constructs every agent; an unchanged fingerprint must not pay for it")
count, err := queueRepo.Count()
Expect(err).ToNot(HaveOccurred())
Expect(count).To(BeZero())
Expect(propRepo.Get(consts.ArtConfFingerprintPropertyKey)).To(Equal(ConfigFingerprint()))
})
It("runs the backfill when no fingerprint was ever stored", func() {
seedEntities()
s, err := backfill(ctx, ds, noAgents)
Expect(err).ToNot(HaveOccurred())
Expect(s.Ran).To(BeTrue())
count, err := queueRepo.Count()
Expect(err).ToNot(HaveOccurred())
Expect(count).To(Equal(int64(5))) // 2 artists + 1 album + 1 playlist + 1 radio
stored, err := propRepo.Get(consts.ArtConfFingerprintPropertyKey)
Expect(err).ToNot(HaveOccurred())
Expect(stored).To(Equal(ConfigFingerprint()))
})
It("enqueues a private playlist by resolving it under an admin context", func() {
ds.MockedUser = adminUserRepo()
vds := &visibilityPlaylistDS{
MockDataStore: ds,
private: model.Playlist{ID: "plPrivate", OwnerID: "admin"},
tracks: &tests.MockPlaylistTrackRepo{},
}
s, err := backfill(ctx, vds, noAgents)
Expect(err).ToNot(HaveOccurred())
Expect(s.Ran).To(BeTrue())
Expect(findQueued(queueRepo.MockArtworkQueueRepo, "pl", "plPrivate")).ToNot(BeNil())
})
It("enqueues artists before albums/playlists/radios, all at Backfill priority", func() {
seedEntities()
It("leaves a stale fingerprint stored, so the warning survives a restart", func() {
Expect(propRepo.Put(consts.ArtConfFingerprintPropertyKey, "stale-fingerprint")).To(Succeed())
s, err := backfill(ctx, ds, noAgents)
Expect(err).ToNot(HaveOccurred())
Expect(s.Ran).To(BeTrue())
Expect(ReconcileConfigFingerprint(ctx, ds)).To(Succeed())
Expect(queueRepo.callKinds).ToNot(BeEmpty())
firstOther := slices.IndexFunc(queueRepo.callKinds, func(k string) bool { return k != "ar" })
Expect(firstOther).ToNot(Equal(0), "artists must be the first Enqueue call")
if firstOther >= 0 {
Expect(queueRepo.callKinds[firstOther:]).ToNot(ContainElement("ar"),
"no artist Enqueue may follow another kind")
}
for _, it := range queueRepo.Data {
Expect(it.Priority).To(Equal(model.ArtworkPriorityBackfill))
Expect(it.ItemKind).To(BeElementOf("ar", "al", "pl", "ra"))
}
})
It("reports what it enqueued, per kind and as an external-lookup ceiling", func() {
conf.Server.ArtistArtPriority = "artist.*, external"
conf.Server.CoverArtPriority = "cover.*, external"
conf.Server.EnableM3UExternalAlbumArt = false
seedEntities()
s, err := backfill(ctx, ds, func() ImageAgentCount { return ImageAgentCount{Artist: 3, Album: 2} })
Expect(err).ToNot(HaveOccurred())
Expect(s.Ran).To(BeTrue())
Expect(s.PerKind).To(Equal(map[string]int64{"ar": 2, "al": 1, "pl": 1, "ra": 1}))
Expect(s.Items).To(Equal(int64(5)))
// 2 artists x 3 agents, 1 album x 2, 1 playlist grid x 2, and radios never fetch.
Expect(s.MaxExternalLookups).To(Equal(int64(6 + 2 + PlaylistGridSamples*2)))
})
})
Describe("EnqueueStaleAbsentAll", func() {
var artRepo *tests.MockArtworkRepo
BeforeEach(func() {
artRepo = tests.CreateMockArtworkRepo()
ds.MockedArtwork = artRepo
queueRepo.ItemArtworkSource = artRepo
})
It("enqueues only absent entries older than the recheck window, across all kinds", func() {
old := time.Now().Add(-StaleAbsentAge - time.Hour)
recent := time.Now().Add(-StaleAbsentAge + time.Hour)
artRepo.ItemData["ar-stale"] = model.ItemArtwork{ItemKind: "ar", ItemID: "ar1", ImageType: model.ImageTypePrimary, Hash: "", AttemptedAt: old}
artRepo.ItemData["al-stale"] = model.ItemArtwork{ItemKind: "al", ItemID: "al1", ImageType: model.ImageTypePrimary, Hash: "", AttemptedAt: old}
artRepo.ItemData["pl-stale"] = model.ItemArtwork{ItemKind: "pl", ItemID: "pl1", ImageType: model.ImageTypePrimary, Hash: "", AttemptedAt: old}
artRepo.ItemData["ra-stale"] = model.ItemArtwork{ItemKind: "ra", ItemID: "ra1", ImageType: model.ImageTypePrimary, Hash: "", AttemptedAt: old}
artRepo.ItemData["ar-recent"] = model.ItemArtwork{ItemKind: "ar", ItemID: "ar2", ImageType: model.ImageTypePrimary, Hash: "", AttemptedAt: recent}
artRepo.ItemData["al-resolved"] = model.ItemArtwork{ItemKind: "al", ItemID: "al2", ImageType: model.ImageTypePrimary, Hash: "somehash", AttemptedAt: old}
err := enqueueStaleAbsentAll(ctx, ds)
Expect(err).ToNot(HaveOccurred())
Expect(queueRepo.Data).To(HaveLen(4))
for _, it := range queueRepo.Data {
Expect(it.Priority).To(Equal(model.ArtworkPriorityRecheck))
}
Expect(findQueued(queueRepo.MockArtworkQueueRepo, "ar", "ar1")).ToNot(BeNil())
Expect(findQueued(queueRepo.MockArtworkQueueRepo, "al", "al1")).ToNot(BeNil())
Expect(findQueued(queueRepo.MockArtworkQueueRepo, "pl", "pl1")).ToNot(BeNil())
Expect(findQueued(queueRepo.MockArtworkQueueRepo, "ra", "ra1")).ToNot(BeNil())
Expect(findQueued(queueRepo.MockArtworkQueueRepo, "ar", "ar2")).To(BeNil())
Expect(findQueued(queueRepo.MockArtworkQueueRepo, "al", "al2")).To(BeNil())
})
It("caps each tick at the recheck batch, oldest attempts first", func() {
for i := range StaleAbsentRecheckBatch + 1 {
id := fmt.Sprintf("ar%d", i)
artRepo.ItemData[id] = model.ItemArtwork{ItemKind: "ar", ItemID: id, ImageType: model.ImageTypePrimary,
Hash: "", AttemptedAt: time.Now().Add(-StaleAbsentAge - time.Duration(i+1)*time.Minute)}
}
Expect(enqueueStaleAbsentAll(ctx, ds)).To(Succeed())
Expect(queueRepo.Data).To(HaveLen(StaleAbsentRecheckBatch))
// ar0 has the newest attempted_at of the cohort, so it is the one left out.
Expect(findQueued(queueRepo.MockArtworkQueueRepo, "ar", "ar0")).To(BeNil())
Expect(propRepo.Get(consts.ArtConfFingerprintPropertyKey)).To(Equal("stale-fingerprint"))
})
})
@ -315,8 +123,8 @@ var _ = Describe("Housekeeping", func() {
})
It("enqueues only entities that have no item_artwork row, across all kinds", func() {
artRepo.ItemData["al-resolved"] = model.ItemArtwork{ItemKind: "al", ItemID: "al1", ImageType: model.ImageTypePrimary, Hash: "somehash", AttemptedAt: time.Now()}
artRepo.ItemData["ar-absent"] = model.ItemArtwork{ItemKind: "ar", ItemID: "ar1", ImageType: model.ImageTypePrimary, Hash: "", AttemptedAt: time.Now()}
artRepo.ItemData["al-resolved"] = model.ItemArtwork{ItemKind: "al", ItemID: "al1", ImageType: model.ImageTypePrimary, Hash: "somehash"}
artRepo.ItemData["ar-absent"] = model.ItemArtwork{ItemKind: "ar", ItemID: "ar1", ImageType: model.ImageTypePrimary, Hash: ""}
err := enqueueMissingAll(ctx, ds)
Expect(err).ToNot(HaveOccurred())
@ -324,11 +132,11 @@ var _ = Describe("Housekeeping", func() {
for _, it := range queueRepo.Data {
Expect(it.Priority).To(Equal(model.ArtworkPriorityRecheck))
}
Expect(findQueued(queueRepo.MockArtworkQueueRepo, "al", "al2")).ToNot(BeNil())
Expect(findQueued(queueRepo.MockArtworkQueueRepo, "pl", "pl1")).ToNot(BeNil())
Expect(findQueued(queueRepo.MockArtworkQueueRepo, "ra", "ra1")).ToNot(BeNil())
Expect(findQueued(queueRepo.MockArtworkQueueRepo, "al", "al1")).To(BeNil())
Expect(findQueued(queueRepo.MockArtworkQueueRepo, "ar", "ar1")).To(BeNil())
Expect(findQueued(queueRepo, "al", "al2")).ToNot(BeNil())
Expect(findQueued(queueRepo, "pl", "pl1")).ToNot(BeNil())
Expect(findQueued(queueRepo, "ra", "ra1")).ToNot(BeNil())
Expect(findQueued(queueRepo, "al", "al1")).To(BeNil())
Expect(findQueued(queueRepo, "ar", "ar1")).To(BeNil())
})
})
})

25
core/artwork/worker.go
View file

@ -23,8 +23,8 @@ import (
const (
workerPollInterval = 5 * time.Second
backoffBase = 5 * time.Second
// giveUpAfter bounds the retry budget from enqueue; past it the item falls to the
// periodic stale-absent recheck.
// giveUpAfter bounds the retry budget from enqueue; past it the item settles and only an
// explicit reprocess retries it.
giveUpAfter = 12 * time.Hour
)
@ -40,7 +40,6 @@ type drainPool struct {
// independently, and pruneMu serializes prune against the store-write window.
type Worker struct {
proc *processor
agents *agents.Agents
cache cache.FileCache
ffmpeg ffmpeg.FFmpeg
broker events.Broker
@ -55,7 +54,6 @@ type Worker struct {
func NewWorker(ds model.DataStore, store *ImageStore, ag *agents.Agents, ffmpeg ffmpeg.FFmpeg, broker events.Broker, imgCache cache.FileCache) *Worker {
w := &Worker{
proc: &processor{ds: ds, store: store},
agents: ag,
cache: imgCache,
ffmpeg: ffmpeg,
broker: broker,
@ -133,17 +131,9 @@ func (w *Worker) RunPrune(ctx context.Context) error {
return prune(ctx, w.proc.ds, w.proc.store)
}
// Backfill enqueues every entity for re-resolution when the artwork config fingerprint changed,
// artists first. It reports whether the backfill ran.
func (w *Worker) Backfill(ctx context.Context) (bool, error) {
s, err := backfill(ctx, w.proc.ds, func() ImageAgentCount { return NewImageAgentCount(w.agents) })
return s.Ran, err
}
// EnqueueStaleAbsentAll requeues known-absent entries older than StaleAbsentAge, at most
// StaleAbsentRecheckBatch per kind, oldest first.
func (w *Worker) EnqueueStaleAbsentAll(ctx context.Context) error {
return enqueueStaleAbsentAll(ctx, w.proc.ds)
// ReconcileConfig records the artwork config fingerprint, or warns when it changed.
func (w *Worker) ReconcileConfig(ctx context.Context) error {
return ReconcileConfigFingerprint(ctx, w.proc.ds)
}
// EnqueueMissingAll requeues entities with no artwork state row: the safety net for anything
@ -265,10 +255,9 @@ func (w *Worker) process(ctx context.Context, item model.ArtworkQueueItem) (outc
"budgetLeft", time.Until(item.EnqueuedAt.Add(giveUpAfter)))
break
}
// Absent is only recoverable where a periodic recheck revisits it, so other kinds keep
// no row; art already being served is kept, as exhaustion means unreachable, not removed.
// Art already being served is kept: exhaustion means unreachable, not removed.
settled := "kept previous state"
if out == outcomeFailed && hasRecheckPath(item.ItemKind) && !w.hasResolvedArtwork(ctx, item) {
if out == outcomeFailed && settlesAbsentOnGiveUp(item.ItemKind) && !w.hasResolvedArtwork(ctx, item) {
writeAbsent(ctx, w.proc.ds.Artwork(ctx), item)
settled = "recorded absent"
}

30
core/artwork/worker_test.go
View file

@ -15,6 +15,7 @@ import (
"github.com/navidrome/navidrome/conf/configtest"
"github.com/navidrome/navidrome/core/agents"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/request"
"github.com/navidrome/navidrome/server/events"
"github.com/navidrome/navidrome/tests"
"github.com/navidrome/navidrome/utils/cache"
@ -115,6 +116,29 @@ func findQueued(q *tests.MockArtworkQueueRepo, kind, id string) *model.ArtworkQu
return nil
}
// visibilityPlaylistDS models playlist_repository's userFilter: a private playlist is only
// visible when the ctx carries an admin, so headless work must wrap ctx with one first.
type visibilityPlaylistDS struct {
*tests.MockDataStore
private model.Playlist
tracks model.PlaylistTrackRepository
}
func (v *visibilityPlaylistDS) Playlist(ctx context.Context) model.PlaylistRepository {
repo := tests.CreateMockPlaylistRepo()
repo.TracksRepo = v.tracks
if u, ok := request.UserFrom(ctx); ok && u.IsAdmin {
repo.SetData(model.Playlists{v.private})
}
return repo
}
func adminUserRepo() *tests.MockedUserRepo {
repo := tests.CreateMockUserRepo()
Expect(repo.Put(&model.User{ID: "admin", UserName: "admin", IsAdmin: true})).To(Succeed())
return repo
}
var _ = Describe("Worker", func() {
var (
ctx context.Context
@ -441,9 +465,9 @@ var _ = Describe("Worker", func() {
Expect(ia.Hash).To(Equal("cafebabe"), "recording the failure must not disturb the served art")
})
// 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() {
// Only a view enqueues a media file, and an absent row is exactly what stops a view from
// doing so: a transient read error would look permanent.
It("does not settle absent on exhaustion for a media file", func() {
conf.Server.EnableMediaFileCoverArt = true
ds.MockedMediaFile = tests.CreateMockMediaFileRepo()
ds.MockedMediaFile.(*tests.MockMediaFileRepo).SetData(model.MediaFiles{

9
db/migrations/20260901225726_normalize_artwork_last_failure.sql Normal file
View file

@ -0,0 +1,9 @@
-- +goose Up
-- 20260819204637 added last_failure with DEFAULT '[]', so every row already in the table got a
-- non-empty value. That is how a give-up is now told apart from a definitive "no image", which
-- would report every pre-existing absent row as failed.
UPDATE item_artwork SET last_failure = '' WHERE last_failure = '[]';
-- +goose Down
-- Irreversible: a genuine give-up and a backfilled default are indistinguishable once normalized.
SELECT 1;

1
model/album.go
View file

@ -143,7 +143,6 @@ type AlbumRepository interface {
UpdateExternalInfo(*Album) error
Get(id string) (*Album, error)
GetAll(...QueryOptions) (Albums, error)
GetAllIDs(...QueryOptions) ([]string, error)
// GetSoleAlbumArtistIDsInSubtrees returns the sole album artists of the albums with folders in
// any of the given library-relative subtrees.
GetSoleAlbumArtistIDsInSubtrees(lib Library, paths ...string) ([]string, error)

1
model/artist.go
View file

@ -90,7 +90,6 @@ type ArtistRepository interface {
UpdateExternalInfo(a *Artist) error
Get(id string) (*Artist, error)
GetAll(options ...QueryOptions) (Artists, error)
GetAllIDs(options ...QueryOptions) ([]string, error)
GetCursor(options ...QueryOptions) (ArtistCursor, error)
GetIndex(includeMissing bool, libraryIds []int, roles ...Role) (ArtistIndexes, error)

33
model/artwork.go
View file

@ -18,6 +18,10 @@ type Artwork struct {
const ImageTypePrimary = "primary"
// ArtworkSourceFailed is a pseudo-source selecting absent states that exhausted the retry budget
// rather than being answered. The "!" keeps it from colliding with a stored source value.
const ArtworkSourceFailed = "!failed"
// ItemImage is per-entity artwork state hydrated at query time; never persisted.
type ItemImage struct {
ImageHash string `structs:"-" json:"imageHash,omitempty"`
@ -88,11 +92,12 @@ func (i ItemArtworkInfo) Image() ItemImage {
}
type ArtworkQueueItem struct {
ItemKind string `structs:"item_kind"`
ItemID string `structs:"item_id"`
ImageType string `structs:"image_type"`
Priority int `structs:"priority"`
Attempts int `structs:"attempts"`
ItemKind string `structs:"item_kind"`
ItemID string `structs:"item_id"`
ImageType string `structs:"image_type"`
Priority int `structs:"priority"`
Attempts int `structs:"attempts"`
// RetryAt is the earliest time the drain may take this row, not when it will run.
RetryAt time.Time `structs:"retry_at"`
EnqueuedAt time.Time `structs:"enqueued_at"`
// Trace is why the last attempt failed. Only Get reads it; the drain projects it away.
@ -101,7 +106,9 @@ type ArtworkQueueItem struct {
// Queue priorities: higher drains first.
const (
ArtworkPriorityRecheck = 0
ArtworkPriorityRecheck = 0
// ArtworkPriorityBackfill sits between the hourly sweep and scan-driven work. Nothing enqueues
// it today; it stays named so a row still carrying it can be reported and cancelled.
ArtworkPriorityBackfill = 10
ArtworkPriorityScan = 50
ArtworkPriorityBump = 100
@ -134,15 +141,13 @@ type ArtworkQueueRepository interface {
// EnqueuePreservingBackoff upserts like Enqueue but preserves an existing row's retry_at, so a
// request-triggered read-through never resets a failed resolution's backoff.
EnqueuePreservingBackoff(items ...ArtworkQueueItem) error
// EnqueueStaleAbsent inserts queue rows (priority Recheck) for absent states older than cutoff, oldest
// first; limit caps the selection, so already-queued rows use up budget (backpressure when the drain stalls).
EnqueueStaleAbsent(kind Kind, attemptedBefore time.Time, limit int) (int64, error)
// EnqueueAllMissing inserts queue rows for all entities with no item_artwork row, at the given priority.
EnqueueAllMissing(kind Kind, priority int) (int64, error)
// EnqueueIfMissing inserts only for items with no item_artwork row yet.
EnqueueIfMissing(items ...ArtworkQueueItem) error
// CountBySource reports how many items of a kind currently resolve from the given sources.
// An empty sources slice means every source; "" matches absent state.
// An empty sources slice means every source; "" matches absent state, and the pseudo-source
// ArtworkSourceFailed matches the absent states that gave up.
CountBySource(kind Kind, sources []string) (int64, error)
// SourcesInUse lists the distinct sources items of a kind currently resolve from, "" included.
SourcesInUse(kind Kind) ([]string, error)
@ -161,9 +166,6 @@ type ArtworkQueueRepository interface {
// CountQueued reports the pending rows matching the kinds and priorities, grouped by both;
// an empty filter means every one.
CountQueued(kinds []Kind, priorities []int) ([]ArtworkQueueStat, error)
// CountAbsent reports the absent states of a kind, and how many are past the given cutoff,
// eligible for EnqueueStaleAbsent (which drains them limit rows per call).
CountAbsent(kind Kind, attemptedBefore time.Time) (ArtworkAbsentStat, error)
// PurgeDangling removes queue rows whose entity no longer exists.
PurgeDangling() (int64, error)
// PurgeQueued removes pending rows matching the kinds and priorities; an empty filter means every one.
@ -175,8 +177,3 @@ type ArtworkQueueStat struct {
Priority int
Count int64
}
type ArtworkAbsentStat struct {
Total int64
Stale int64
}

2
model/mediafile.go
View file

@ -553,8 +553,6 @@ type MediaFileRepository interface {
// expression, using the logged user's annotations. Limit and offset are ignored.
MatchesCriteria(id string, c criteria.Criteria) (bool, error)
GetCursor(options ...QueryOptions) (MediaFileCursor, error)
// GetAllIDs returns just the media_file IDs for the same row set as GetAll.
GetAllIDs(options ...QueryOptions) ([]string, error)
// GetAlbumIDsByFolder returns the distinct IDs of albums with non-missing tracks in the given
// folders or their direct children.
GetAlbumIDsByFolder(lib Library, folderIDs ...string) ([]string, error)

1
model/playlist.go
View file

@ -150,7 +150,6 @@ type PlaylistRepository interface {
Get(id string) (*Playlist, error)
GetWithTracks(id string, refreshSmartPlaylist, includeMissing bool) (*Playlist, error)
GetAll(options ...QueryOptions) (Playlists, error)
GetAllIDs(options ...QueryOptions) ([]string, error)
GetCursor(options ...QueryOptions) (PlaylistCursor, error)
FindByPath(path string) (*Playlist, error)
Delete(id string) error

1
model/radio.go
View file

@ -35,6 +35,5 @@ type RadioRepository interface {
Exists(id string) (bool, error)
Get(id string) (*Radio, error)
GetAll(options ...QueryOptions) (Radios, error)
GetAllIDs(options ...QueryOptions) ([]string, error)
Put(u *Radio, colsToUpdate ...string) error
}

6
persistence/album_repository.go
View file

@ -259,8 +259,8 @@ func (r *albumRepository) hydrateArtwork(albums model.Albums) {
func(a *model.Album) (string, *model.ItemImage) { return a.ID, &a.ItemImage })
}
// GetAllIDs returns the IDs of GetAll's row set, skipping its column projection and JSON decoding.
func (r *albumRepository) GetAllIDs(options ...model.QueryOptions) ([]string, error) {
// getAllIDs returns the IDs of GetAll's row set, skipping its column projection and JSON decoding.
func (r *albumRepository) getAllIDs(options ...model.QueryOptions) ([]string, error) {
sq := r.applyLibraryFilter(r.newSelect(options...).Columns("album.id"))
if filtersNeedAnnotation(sq) {
sq = r.withAnnotation(sq, "album.id")
@ -304,7 +304,7 @@ func (r *albumRepository) GetSoleAlbumArtistIDsInSubtrees(lib model.Library, pat
}
func (r *albumRepository) GetCursor(options ...model.QueryOptions) (model.AlbumCursor, error) {
ids, err := r.GetAllIDs(options...)
ids, err := r.getAllIDs(options...)
if err != nil {
return nil, err
}

4
persistence/album_repository_test.go
View file

@ -152,12 +152,12 @@ var _ = Describe("AlbumRepository", func() {
})
})
Describe("GetAllIDs", func() {
Describe("getAllIDs", func() {
It("returns the same id set as GetAll", func() {
want, err := albumRepo.GetAll()
Expect(err).ToNot(HaveOccurred())
Expect(want).ToNot(BeEmpty())
ids, err := albumRepo.GetAllIDs()
ids, err := albumRepo.getAllIDs()
Expect(err).ToNot(HaveOccurred())
Expect(ids).To(ConsistOf(slice.Map(want, func(a model.Album) string { return a.ID })))
})

6
persistence/artist_repository.go
View file

@ -265,9 +265,9 @@ func (r *artistRepository) GetAll(options ...model.QueryOptions) (model.Artists,
return res, err
}
// GetAllIDs returns just the artist IDs for the same row set as GetAll, skipping the
// getAllIDs returns just the artist IDs for the same row set as GetAll, skipping the
// heavy stats columns and JSON post-processing.
func (r *artistRepository) GetAllIDs(options ...model.QueryOptions) ([]string, error) {
func (r *artistRepository) getAllIDs(options ...model.QueryOptions) ([]string, error) {
sq := r.applyLibraryFilterToArtistQuery(r.newSelect(options...).Columns("artist.id")).GroupBy("artist.id")
if filtersNeedAnnotation(sq) {
sq = r.withAnnotation(sq, "artist.id")
@ -284,7 +284,7 @@ func (r *artistRepository) hydrateArtwork(artists model.Artists) {
}
func (r *artistRepository) GetCursor(options ...model.QueryOptions) (model.ArtistCursor, error) {
ids, err := r.GetAllIDs(options...)
ids, err := r.getAllIDs(options...)
if err != nil {
return nil, err
}

4
persistence/artist_repository_test.go
View file

@ -285,12 +285,12 @@ var _ = Describe("ArtistRepository", func() {
})
})
Describe("GetAllIDs", func() {
Describe("getAllIDs", func() {
It("returns the same id set as GetAll", func() {
want, err := repo.GetAll()
Expect(err).ToNot(HaveOccurred())
Expect(want).ToNot(BeEmpty())
ids, err := repo.GetAllIDs()
ids, err := repo.(*artistRepository).getAllIDs()
Expect(err).ToNot(HaveOccurred())
Expect(ids).To(ConsistOf(slice.Map(want, func(a model.Artist) string { return a.ID })))
})

2
persistence/artwork_hydration_test.go
View file

@ -578,7 +578,7 @@ var _ = Describe("Artwork hydration", func() {
opts := model.QueryOptions{Sort: "name", Filters: onlyPlaylists}
// Both phases must filter on their own: the id pre-pass and the chunk fetch.
Expect(repo.GetAllIDs(opts)).To(ConsistOf(plsBest.ID))
Expect(repo.(*playlistRepository).getAllIDs(opts)).To(ConsistOf(plsBest.ID))
all, err := repo.GetAll(model.QueryOptions{Filters: onlyPlaylists})
Expect(err).ToNot(HaveOccurred())
Expect(slice.Map(all, func(p model.Playlist) string { return p.ID })).To(ConsistOf(plsBest.ID))

35
persistence/artwork_queue_repository.go
View file

@ -56,14 +56,6 @@ func (r *artworkQueueRepository) EnqueuePreservingBackoff(items ...model.Artwork
priority = MAX(priority, excluded.priority)`, items)
}
func (r *artworkQueueRepository) EnqueueStaleAbsent(kind model.Kind, attemptedBefore time.Time, limit int) (int64, error) {
now := time.Now()
return r.insertIfNotQueued("", `SELECT item_kind, item_id, image_type, ?, 0, ?, ?
FROM `+itemArtworkTable+` WHERE item_kind = ? AND hash = '' AND attempted_at < ?
ORDER BY attempted_at LIMIT ?`,
model.ArtworkPriorityRecheck, now, now, kind.Prefix(), attemptedBefore, limit)
}
func (r *artworkQueueRepository) EnqueueAllMissing(kind model.Kind, priority int) (int64, error) {
entityTable, ok := artworkOwnerTables[kind]
if !ok {
@ -110,13 +102,23 @@ func (r *artworkQueueRepository) insertIfNotQueued(with, sql string, args ...any
` (`+strings.Join(enqueueColumns, ", ")+`) `+sql+skipIfQueued, args...))
}
// artworkSourceFilter selects item_artwork rows of a kind; no sources means every source, "" the absent state.
// artworkSourceFilter selects item_artwork rows of a kind; no sources means every source, "" the
// absent state, and ArtworkSourceFailed the absent states that gave up. Several are a union, so
// asking for both absent and failed is just absent.
func artworkSourceFilter(kind model.Kind, sources []string) Sqlizer {
f := And{Eq{"item_kind": kind.Prefix()}}
if len(sources) > 0 {
f = append(f, Eq{"source": sources})
if len(sources) == 0 {
return f
}
return f
stored := slices.DeleteFunc(slices.Clone(sources), func(s string) bool { return s == model.ArtworkSourceFailed })
var match Or
if len(stored) > 0 {
match = append(match, Eq{"source": stored})
}
if len(stored) != len(sources) {
match = append(match, And{Eq{"hash": ""}, NotEq{"last_failure": ""}})
}
return append(f, match)
}
func (r *artworkQueueRepository) CountBySource(kind model.Kind, sources []string) (int64, error) {
@ -231,13 +233,4 @@ func (r *artworkQueueRepository) Count() (int64, error) {
return res.Count, err
}
// CountAbsent matches EnqueueStaleAbsent on hash, so the stale count is the pool a recheck drains from.
func (r *artworkQueueRepository) CountAbsent(kind model.Kind, attemptedBefore time.Time) (model.ArtworkAbsentStat, error) {
var res model.ArtworkAbsentStat
err := r.queryOne(Select("count(*) as total").
Column(Expr("coalesce(sum(attempted_at < ?), 0) as stale", attemptedBefore)).
From(itemArtworkTable).Where(Eq{"item_kind": kind.Prefix(), "hash": ""}), &res)
return res, err
}
var _ model.ArtworkQueueRepository = (*artworkQueueRepository)(nil)

83
persistence/artwork_queue_repository_test.go
View file

@ -237,41 +237,6 @@ var _ = Describe("ArtworkQueueRepository", func() {
Expect(ids).To(ConsistOf(albumSgtPeppers.ID, artistKraftwerk.ID, plsBest.ID, radioWithHomePage.ID, songDayInALife.ID))
})
It("enqueues stale absent states for recheck", func() {
awRepo := NewArtworkRepository(context.Background(), GetDBXBuilder())
old := time.Now().Add(-48 * time.Hour)
Expect(awRepo.PutItemArtwork(&model.ItemArtwork{ItemKind: "ar", ItemID: "stale1", ImageType: model.ImageTypePrimary, Hash: "", AttemptedAt: old})).To(Succeed())
Expect(awRepo.PutItemArtwork(&model.ItemArtwork{ItemKind: "ar", ItemID: "fresh1", ImageType: model.ImageTypePrimary, Hash: "", AttemptedAt: time.Now()})).To(Succeed())
Expect(awRepo.PutItemArtwork(&model.ItemArtwork{ItemKind: "ar", ItemID: "found1", ImageType: model.ImageTypePrimary, Hash: "hX", AttemptedAt: old})).To(Succeed())
n, err := repo.EnqueueStaleAbsent(model.KindArtistArtwork, time.Now().Add(-24*time.Hour), 100)
Expect(err).ToNot(HaveOccurred())
Expect(n).To(Equal(int64(1)))
items, err := repo.DequeueBatch(10)
Expect(err).ToNot(HaveOccurred())
Expect(items).To(HaveLen(1))
Expect(items[0].ItemID).To(Equal("stale1"))
Expect(items[0].Priority).To(Equal(model.ArtworkPriorityRecheck))
})
It("enqueues only the oldest stale absent states up to the limit", func() {
awRepo := NewArtworkRepository(context.Background(), GetDBXBuilder())
now := time.Now()
Expect(awRepo.PutItemArtwork(&model.ItemArtwork{ItemKind: "ar", ItemID: "oldest", ImageType: model.ImageTypePrimary, Hash: "", AttemptedAt: now.Add(-72 * time.Hour)})).To(Succeed())
Expect(awRepo.PutItemArtwork(&model.ItemArtwork{ItemKind: "ar", ItemID: "older", ImageType: model.ImageTypePrimary, Hash: "", AttemptedAt: now.Add(-60 * time.Hour)})).To(Succeed())
Expect(awRepo.PutItemArtwork(&model.ItemArtwork{ItemKind: "ar", ItemID: "old", ImageType: model.ImageTypePrimary, Hash: "", AttemptedAt: now.Add(-48 * time.Hour)})).To(Succeed())
n, err := repo.EnqueueStaleAbsent(model.KindArtistArtwork, now.Add(-24*time.Hour), 2)
Expect(err).ToNot(HaveOccurred())
Expect(n).To(Equal(int64(2)))
items, err := repo.DequeueBatch(10)
Expect(err).ToNot(HaveOccurred())
ids := slice.Map(items, func(it model.ArtworkQueueItem) string { return it.ItemID })
Expect(ids).To(ConsistOf("oldest", "older"))
})
It("enqueues entities that have no item_artwork row at all", func() {
awRepo := NewArtworkRepository(context.Background(), GetDBXBuilder())
Expect(awRepo.PutItemArtwork(&model.ItemArtwork{ItemKind: "al", ItemID: albumSgtPeppers.ID, ImageType: model.ImageTypePrimary, Hash: "hX", AttemptedAt: time.Now()})).To(Succeed())
@ -435,29 +400,49 @@ var _ = Describe("ArtworkQueueRepository", func() {
))
})
It("reports an empty queue as no rows", func() {
Expect(repo.CountQueued(nil, nil)).To(BeEmpty())
})
It("counts absent states and how many are due for recheck", func() {
It("selects only the absent states that gave up, not those a source answered", func() {
awRepo := NewArtworkRepository(context.Background(), GetDBXBuilder())
old := time.Now().Add(-48 * time.Hour)
for _, ia := range []model.ItemArtwork{
{ItemKind: "ar", ItemID: "stale1", ImageType: model.ImageTypePrimary, Hash: "", AttemptedAt: old},
{ItemKind: "ar", ItemID: "fresh1", ImageType: model.ImageTypePrimary, Hash: "", AttemptedAt: time.Now()},
{ItemKind: "ar", ItemID: "found1", ImageType: model.ImageTypePrimary, Hash: "hX", AttemptedAt: old},
{ItemKind: "al", ItemID: "stale2", ImageType: model.ImageTypePrimary, Hash: "", AttemptedAt: old},
{ItemKind: "ar", ItemID: "gaveup", ImageType: model.ImageTypePrimary, LastFailure: "[]"},
{ItemKind: "ar", ItemID: "toldno", ImageType: model.ImageTypePrimary},
{ItemKind: "ar", ItemID: "hasart", ImageType: model.ImageTypePrimary, Hash: "hX", LastFailure: "[]"},
} {
Expect(awRepo.PutItemArtwork(&ia)).To(Succeed())
}
Expect(repo.CountAbsent(model.KindArtistArtwork, time.Now().Add(-24*time.Hour))).
To(Equal(model.ArtworkAbsentStat{Total: 2, Stale: 1}))
Expect(repo.CountBySource(model.KindArtistArtwork, []string{model.ArtworkSourceFailed})).To(Equal(int64(1)),
"an item still serving art is not absent, however its last attempt went")
// A later success rewrites the row, clearing the record.
Expect(awRepo.PutItemArtwork(&model.ItemArtwork{ItemKind: "ar", ItemID: "gaveup",
ImageType: model.ImageTypePrimary, Hash: "hZ"})).To(Succeed())
Expect(repo.CountBySource(model.KindArtistArtwork, []string{model.ArtworkSourceFailed})).To(Equal(int64(0)))
})
It("reports a kind with no absent state as zero, not as an error", func() {
Expect(repo.CountAbsent(model.KindRadioArtwork, time.Now())).To(Equal(model.ArtworkAbsentStat{}))
It("unions the failed pseudo-source with a real one, so absent plus failed is just absent", func() {
awRepo := NewArtworkRepository(context.Background(), GetDBXBuilder())
for _, ia := range []model.ItemArtwork{
{ItemKind: "ar", ItemID: "gaveup", ImageType: model.ImageTypePrimary, LastFailure: "[]"},
{ItemKind: "ar", ItemID: "toldno", ImageType: model.ImageTypePrimary},
{ItemKind: "ar", ItemID: "folder", ImageType: model.ImageTypePrimary, Hash: "hX", Source: "folder"},
} {
Expect(awRepo.PutItemArtwork(&ia)).To(Succeed())
}
failedAndAbsent := []string{model.ArtworkSourceFailed, ""}
Expect(repo.CountBySource(model.KindArtistArtwork, failedAndAbsent)).To(Equal(int64(2)),
"failed is a subset of absent, so asking for both is asking for absent")
Expect(repo.CountBySource(model.KindArtistArtwork, []string{model.ArtworkSourceFailed, "folder"})).
To(Equal(int64(2)), "a pseudo-source and a stored source combine as a union")
})
It("reports a kind with nothing failed as zero", func() {
Expect(repo.CountBySource(model.KindRadioArtwork, []string{model.ArtworkSourceFailed})).To(Equal(int64(0)))
})
It("reports an empty queue as no rows", func() {
Expect(repo.CountQueued(nil, nil)).To(BeEmpty())
})
})
Describe("PurgeQueued", func() {

6
persistence/mediafile_repository.go
View file

@ -308,8 +308,8 @@ func (r *mediaFileRepository) GetCursor(options ...model.QueryOptions) (model.Me
return wrapMediaFileCursor(cursor), nil
}
// GetAllIDs returns the IDs of GetAll's row set, skipping its wide column projection.
func (r *mediaFileRepository) GetAllIDs(options ...model.QueryOptions) ([]string, error) {
// getAllIDs returns the IDs of GetAll's row set, skipping its wide column projection.
func (r *mediaFileRepository) getAllIDs(options ...model.QueryOptions) ([]string, error) {
sq := r.applyLibraryFilter(r.newSelect(options...).Columns("media_file.id"))
if filtersNeedAnnotation(sq) {
sq = r.withAnnotation(sq, "media_file.id")
@ -341,7 +341,7 @@ func (r *mediaFileRepository) GetAlbumIDsByFolder(lib model.Library, folderIDs .
// GetCursorWithArtwork streams the same rows as GetCursor, hydrated, via an id pre-pass.
func (r *mediaFileRepository) GetCursorWithArtwork(options ...model.QueryOptions) (model.MediaFileCursor, error) {
ids, err := r.GetAllIDs(options...)
ids, err := r.getAllIDs(options...)
if err != nil {
return nil, err
}

6
persistence/playlist_repository.go
View file

@ -208,8 +208,8 @@ func (r *playlistRepository) GetAll(options ...model.QueryOptions) (model.Playli
return playlists, err
}
// GetAllIDs returns the IDs of GetAll's row set, skipping its per-row processing.
func (r *playlistRepository) GetAllIDs(options ...model.QueryOptions) ([]string, error) {
// getAllIDs returns the IDs of GetAll's row set, skipping its per-row processing.
func (r *playlistRepository) getAllIDs(options ...model.QueryOptions) ([]string, error) {
// Joins a projection of user, not the table: its name/created_at columns would make an ORDER BY
// on the playlist's own ambiguous.
sq := r.newSelect(options...).Columns("playlist.id", "user.user_name as owner_name").
@ -224,7 +224,7 @@ func (r *playlistRepository) GetAllIDs(options ...model.QueryOptions) ([]string,
func (r *playlistRepository) GetCursor(options ...model.QueryOptions) (model.PlaylistCursor, error) {
// Both passes apply userFilter, so a visibility change between them cannot widen the cursor.
ids, err := r.GetAllIDs(options...)
ids, err := r.getAllIDs(options...)
if err != nil {
return nil, err
}

4
persistence/playlist_repository_test.go
View file

@ -74,12 +74,12 @@ var _ = Describe("PlaylistRepository", func() {
})
})
Describe("GetAllIDs", func() {
Describe("getAllIDs", func() {
It("returns the same id set as GetAll", func() {
want, err := repo.GetAll()
Expect(err).ToNot(HaveOccurred())
Expect(want).ToNot(BeEmpty())
ids, err := repo.GetAllIDs()
ids, err := repo.(*playlistRepository).getAllIDs()
Expect(err).ToNot(HaveOccurred())
Expect(ids).To(ConsistOf(slice.Map(want, func(p model.Playlist) string { return p.ID })))
})

8
persistence/radio_repository.go
View file

@ -79,14 +79,6 @@ func (r *radioRepository) hydrateArtwork(radios model.Radios) {
func(rd *model.Radio) (string, *model.ItemImage) { return rd.ID, &rd.ItemImage })
}
// GetAllIDs returns just the radio IDs. Used by bulk enumeration (artwork backfill).
func (r *radioRepository) GetAllIDs(options ...model.QueryOptions) ([]string, error) {
sel := r.newSelect(options...).Columns("id")
ids := []string{}
err := r.queryAllSlice(sel, &ids)
return ids, err
}
func (r *radioRepository) Put(radio *model.Radio, colsToUpdate ...string) error {
if !r.isPermitted() {
return rest.ErrPermissionDenied

12
persistence/radio_repository_test.go
View file

@ -7,7 +7,6 @@ import (
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/request"
"github.com/navidrome/navidrome/utils/slice"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
@ -79,17 +78,6 @@ var _ = Describe("RadioRepository", func() {
})
})
Describe("GetAllIDs", func() {
It("returns the same id set as GetAll", func() {
want, err := repo.GetAll()
Expect(err).To(BeNil())
Expect(want).ToNot(BeEmpty())
ids, err := repo.GetAllIDs()
Expect(err).To(BeNil())
Expect(ids).To(ConsistOf(slice.Map(want, func(r model.Radio) string { return r.ID })))
})
})
Describe("Put", func() {
It("successfully updates item", func() {
err := repo.Put(&model.Radio{

9
tests/mock_album_repo.go
View file

@ -7,7 +7,6 @@ import (
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/id"
"github.com/navidrome/navidrome/utils/slice"
)
func CreateMockAlbumRepo() *MockAlbumRepo {
@ -85,14 +84,6 @@ func (m *MockAlbumRepo) GetAll(qo ...model.QueryOptions) (model.Albums, error) {
return m.All, nil
}
func (m *MockAlbumRepo) GetAllIDs(qo ...model.QueryOptions) ([]string, error) {
all, err := m.GetAll(qo...)
if err != nil {
return nil, err
}
return slice.Map(all, func(a model.Album) string { return a.ID }), nil
}
func (m *MockAlbumRepo) GetCursor(qo ...model.QueryOptions) (model.AlbumCursor, error) {
res, err := m.GetAll(qo...)
if err != nil {

9
tests/mock_artist_repo.go
View file

@ -6,7 +6,6 @@ import (
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/id"
"github.com/navidrome/navidrome/utils/slice"
)
func CreateMockArtistRepo() *MockArtistRepo {
@ -117,14 +116,6 @@ func (m *MockArtistRepo) GetAll(options ...model.QueryOptions) (model.Artists, e
return allArtists, nil
}
func (m *MockArtistRepo) GetAllIDs(options ...model.QueryOptions) ([]string, error) {
all, err := m.GetAll(options...)
if err != nil {
return nil, err
}
return slice.Map(all, func(a model.Artist) string { return a.ID }), nil
}
func (m *MockArtistRepo) GetCursor(options ...model.QueryOptions) (model.ArtistCursor, error) {
res, err := m.GetAll(options...)
if err != nil {

73
tests/mock_artwork_queue_repo.go
View file

@ -16,7 +16,7 @@ type MockArtworkQueueRepo struct {
mu sync.Mutex
Data map[string]model.ArtworkQueueItem // keyed by iaKey(kind, id, imageType)
Err error
// ItemArtworkSource, when set, backs EnqueueStaleAbsent with real item_artwork state.
// ItemArtworkSource, when set, backs the set-difference insert with real item_artwork state.
ItemArtworkSource *MockArtworkRepo
// ExistingIDs is keyed by item_kind; a nil per-kind map means PurgeDangling keeps that kind.
ExistingIDs map[string]map[string]bool
@ -226,26 +226,6 @@ func (m *MockArtworkQueueRepo) CountQueued(kinds []model.Kind, priorities []int)
return res, nil
}
// CountAbsent mirrors the SQL predicate: an absent state is one with no hash.
func (m *MockArtworkQueueRepo) CountAbsent(kind model.Kind, attemptedBefore time.Time) (model.ArtworkAbsentStat, error) {
m.mu.Lock()
defer m.mu.Unlock()
var res model.ArtworkAbsentStat
if m.Err != nil || m.ItemArtworkSource == nil {
return res, m.Err
}
for _, ia := range m.ItemArtworkSource.ItemData {
if ia.ItemKind != kind.Prefix() || ia.Hash != "" {
continue
}
res.Total++
if ia.AttemptedAt.Before(attemptedBefore) {
res.Stale++
}
}
return res, nil
}
func (m *MockArtworkQueueRepo) EnqueuePreservingBackoff(items ...model.ArtworkQueueItem) error {
m.mu.Lock()
defer m.mu.Unlock()
@ -272,49 +252,24 @@ func (m *MockArtworkQueueRepo) EnqueuePreservingBackoff(items ...model.ArtworkQu
return nil
}
func (m *MockArtworkQueueRepo) EnqueueStaleAbsent(kind model.Kind, attemptedBefore time.Time, limit int) (int64, error) {
m.mu.Lock()
defer m.mu.Unlock()
if m.Err != nil || m.ItemArtworkSource == nil {
return 0, m.Err
}
var stale []model.ItemArtwork
for _, ia := range m.ItemArtworkSource.ItemData {
if ia.ItemKind == kind.Prefix() && ia.Hash == "" && ia.AttemptedAt.Before(attemptedBefore) {
stale = append(stale, ia)
}
}
slices.SortFunc(stale, func(a, b model.ItemArtwork) int { return a.AttemptedAt.Compare(b.AttemptedAt) })
// The limit caps the selection, like the SQL's LIMIT before ON CONFLICT: queued rows use up budget.
stale = stale[:min(limit, len(stale))]
now := time.Now()
var inserted int64
for _, ia := range stale {
k := iaKey(ia.ItemKind, ia.ItemID, ia.ImageType)
if _, ok := m.Data[k]; ok { // DO NOTHING: never touch existing queue rows
continue
}
m.Data[k] = model.ArtworkQueueItem{
ItemKind: ia.ItemKind,
ItemID: ia.ItemID,
ImageType: ia.ImageType,
Priority: model.ArtworkPriorityRecheck,
RetryAt: now,
EnqueuedAt: now,
}
inserted++
}
return inserted, nil
}
// matchingSource mirrors the SQL filter: no sources means every source, "" the absent state.
// matchingSource mirrors the SQL filter: no sources means every source, "" the absent state, and
// ArtworkSourceFailed the absent states that gave up.
func (m *MockArtworkQueueRepo) matchingSource(kind model.Kind, sources []string) []model.ItemArtwork {
if m.ItemArtworkSource == nil {
return nil
}
matches := func(ia model.ItemArtwork) bool {
if len(sources) == 0 {
return true
}
if slices.Contains(sources, ia.Source) {
return true
}
return slices.Contains(sources, model.ArtworkSourceFailed) && ia.Hash == "" && ia.LastFailure != ""
}
var res []model.ItemArtwork
for _, ia := range m.ItemArtworkSource.ItemData {
if ia.ItemKind == kind.Prefix() && (len(sources) == 0 || slices.Contains(sources, ia.Source)) {
if ia.ItemKind == kind.Prefix() && matches(ia) {
res = append(res, ia)
}
}
@ -366,7 +321,7 @@ func (m *MockArtworkQueueRepo) EnqueueBySource(kind model.Kind, sources []string
return inserted, nil
}
// EnqueueMissing mirrors the SQL set-difference insert: ExistingIDs[kind] minus ItemArtworkSource.
// EnqueueAllMissing mirrors the SQL set-difference insert: ExistingIDs[kind] minus ItemArtworkSource.
func (m *MockArtworkQueueRepo) EnqueueAllMissing(kind model.Kind, priority int) (int64, error) {
m.mu.Lock()
defer m.mu.Unlock()

8
tests/mock_mediafile_repo.go
View file

@ -130,14 +130,6 @@ func (m *MockMediaFileRepo) GetCursorWithArtwork(qo ...model.QueryOptions) (mode
return m.GetCursor(qo...)
}
func (m *MockMediaFileRepo) GetAllIDs(qo ...model.QueryOptions) ([]string, error) {
all, err := m.GetAll(qo...)
if err != nil {
return nil, err
}
return slice.Map(all, func(mf model.MediaFile) string { return mf.ID }), nil
}
func (m *MockMediaFileRepo) Put(mf *model.MediaFile) error {
if m.Err {
return errors.New("error")

9
tests/mock_playlist_repo.go
View file

@ -7,7 +7,6 @@ import (
"github.com/deluan/rest"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/id"
"github.com/navidrome/navidrome/utils/slice"
)
func CreateMockPlaylistRepo() *MockPlaylistRepo {
@ -54,14 +53,6 @@ func (m *MockPlaylistRepo) GetAll(options ...model.QueryOptions) (model.Playlist
return m.All, nil
}
func (m *MockPlaylistRepo) GetAllIDs(options ...model.QueryOptions) ([]string, error) {
all, err := m.GetAll(options...)
if err != nil {
return nil, err
}
return slice.Map(all, func(p model.Playlist) string { return p.ID }), nil
}
func (m *MockPlaylistRepo) GetCursor(options ...model.QueryOptions) (model.PlaylistCursor, error) {
res, err := m.GetAll(options...)
if err != nil {

9
tests/mock_radio_repository.go
View file

@ -5,7 +5,6 @@ import (
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/id"
"github.com/navidrome/navidrome/utils/slice"
)
type MockedRadioRepo struct {
@ -74,14 +73,6 @@ func (m *MockedRadioRepo) GetAll(qo ...model.QueryOptions) (model.Radios, error)
return m.All, nil
}
func (m *MockedRadioRepo) GetAllIDs(qo ...model.QueryOptions) ([]string, error) {
all, err := m.GetAll(qo...)
if err != nil {
return nil, err
}
return slice.Map(all, func(r model.Radio) string { return r.ID }), nil
}
func (m *MockedRadioRepo) Put(radio *model.Radio, _ ...string) error {
if m.Err {
return errors.New("error")