mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-31 07:30:32 +00:00
refactor(artwork): drop what the removed retry paths left behind
Deleting the absent-retry jobs and the startup backfill orphaned more than the code that went with them. `Worker.agents` lost its only reader when Backfill went. `RadioRepository.GetAllIDs` existed solely for that backfill's bulk enumeration, as its own comment said. `CountAbsent` became a second query for a number `artwork status` already had: only two paths write item_artwork, and only the absent one leaves `source` empty, so `hash = ''` and `source = ''` select the same rows. The absent count now comes from the source breakdown collected in the same loop, four queries cheaper. Two defects surfaced while pulling on that thread. `reprocess --all` returned on an empty match set before recording the fingerprint, so the command the startup warning names could not silence it. And the "did this run apply the config" flag was derived in the CLI from its own globals, which made `--kind ar --kind al --kind pl --kind rd` — work identical to `--all` — leave the fingerprint stale. It is now derived inside reprocessArtwork from the kinds and sources that drive the queries, so a filter added later has to pass through it. settlesAbsentOnGiveUp decided a worker persistence policy by testing membership in a CLI-facing kind list. It now states the policy directly. Its rationale was wrong too: media files already get an absent row on the definitive path, and the reason to skip them on give-up is that a track with no row still falls back to disc or album art. CheckConfigFingerprint wrote to the database on one of three branches, so it is ReconcileConfigFingerprint now.
This commit is contained in:
parent
8ce710558f
commit
8badec2d62
@ -113,7 +113,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 +122,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())
|
||||
@ -190,12 +190,11 @@ 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 == "" {
|
||||
rep.absent = append(rep.absent, absentCount{kind: k, count: n})
|
||||
}
|
||||
}
|
||||
n, err := q.CountAbsent(k)
|
||||
if err != nil {
|
||||
return rep, fmt.Errorf("counting absent %s artwork: %w", k, err)
|
||||
}
|
||||
rep.absent = append(rep.absent, absentCount{kind: k, count: n})
|
||||
}
|
||||
|
||||
rep.current, rep.inputs = artwork.ConfigFingerprint(), artwork.FingerprintInputs()
|
||||
@ -227,7 +226,7 @@ func formatStatus(rep statusReport) string {
|
||||
for _, a := range rep.absent {
|
||||
fmt.Fprintf(w, " %s\t%d\n", a.kind, a.count)
|
||||
}
|
||||
fmt.Fprintln(w, " (never retried on their own, not even by a backfill; run 'artwork reprocess --source absent')")
|
||||
fmt.Fprintln(w, " (never retried on their own; run 'artwork reprocess --source absent')")
|
||||
|
||||
fmt.Fprintln(w, "\nConfig")
|
||||
fmt.Fprintf(w, " State:\t%s\n", configState(rep))
|
||||
@ -277,8 +276,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.
|
||||
@ -322,10 +321,8 @@ func runReprocess(ctx context.Context) {
|
||||
imageAgents = artwork.NewImageAgentCount(agents.GetAgents(ds, mgr))
|
||||
}
|
||||
|
||||
// Only a whole-library, unfiltered run leaves nothing resolved under the old config.
|
||||
full := artworkAll && len(artworkSources) == 0
|
||||
if err := reprocessArtwork(ctx, ds, kinds, repositorySources(artworkSources), imageAgents,
|
||||
artworkDryRun, full, confirmUnlessYes(artworkYes, os.Stdin, "re-resolve"), os.Stdout); err != nil {
|
||||
artworkDryRun, confirmUnlessYes(artworkYes, os.Stdin, "re-resolve"), os.Stdout); err != nil {
|
||||
log.Fatal(ctx, err)
|
||||
}
|
||||
}
|
||||
@ -454,12 +451,24 @@ func validateSources(q model.ArtworkQueueRepository, sources []string) error {
|
||||
// reprocessArtwork previews from CountBySource — rows matched — then reports what EnqueueBySource
|
||||
// actually inserted; the two differ because an already-queued row is left untouched.
|
||||
func reprocessArtwork(ctx context.Context, ds model.DataStore, kinds []model.Kind, sources []string,
|
||||
imageAgents artwork.ImageAgentCount, dryRun, full bool, confirm confirmFunc, out io.Writer) error {
|
||||
imageAgents artwork.ImageAgentCount, dryRun bool, confirm confirmFunc, out io.Writer) error {
|
||||
q := ds.ArtworkQueue(ctx)
|
||||
if err := validateSources(q, sources); err != nil {
|
||||
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 {
|
||||
@ -478,8 +487,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
|
||||
@ -501,12 +511,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)
|
||||
}
|
||||
if full {
|
||||
if err := artwork.MarkConfigApplied(ctx, ds); err != nil {
|
||||
return fmt.Errorf("recording the applied artwork config: %w", err)
|
||||
}
|
||||
}
|
||||
return nil
|
||||
return markApplied()
|
||||
}
|
||||
|
||||
func runCancel(ctx context.Context) {
|
||||
|
||||
@ -556,10 +556,6 @@ var _ = Describe("confirmUnlessYes", func() {
|
||||
})
|
||||
})
|
||||
|
||||
// unchangedFingerprint is the seeded value, deferred like the real one: an Entry argument is
|
||||
// evaluated at tree-construction time, before the suite sets the config the fingerprint hashes.
|
||||
func unchangedFingerprint() string { return "stale-fingerprint" }
|
||||
|
||||
var _ = Describe("reprocessArtwork", func() {
|
||||
var ds *tests.MockDataStore
|
||||
var art *tests.MockArtworkRepo
|
||||
@ -593,7 +589,7 @@ var _ = Describe("reprocessArtwork", func() {
|
||||
})
|
||||
|
||||
It("previews the per-kind breakdown and queues nothing on a dry run", func() {
|
||||
Expect(reprocessArtwork(ctx, ds, kinds, []string{"external:deezer"}, imageAgents, true, false, accept, &out)).To(Succeed())
|
||||
Expect(reprocessArtwork(ctx, ds, kinds, []string{"external:deezer"}, imageAgents, true, accept, &out)).To(Succeed())
|
||||
|
||||
Expect(out.String()).To(ContainSubstring("external:deezer"))
|
||||
Expect(out.String()).To(ContainSubstring("artist"))
|
||||
@ -604,27 +600,33 @@ var _ = Describe("reprocessArtwork", func() {
|
||||
})
|
||||
|
||||
It("queues nothing when the operator declines", func() {
|
||||
Expect(reprocessArtwork(ctx, ds, kinds, nil, imageAgents, false, false, decline, &out)).To(Succeed())
|
||||
Expect(reprocessArtwork(ctx, ds, kinds, nil, imageAgents, false, decline, &out)).To(Succeed())
|
||||
|
||||
Expect(out.String()).To(ContainSubstring("Aborted"))
|
||||
Expect(queue.Count()).To(BeZero())
|
||||
})
|
||||
|
||||
DescribeTable("records the applied config only for a run that leaves nothing on the old one",
|
||||
func(sources []string, dryRun, full bool, expected func() string) {
|
||||
func(selected []model.Kind, sources []string, dryRun, applied bool) {
|
||||
Expect(ds.Property(ctx).Put(consts.ArtConfFingerprintPropertyKey, "stale-fingerprint")).To(Succeed())
|
||||
|
||||
Expect(reprocessArtwork(ctx, ds, kinds, sources, imageAgents, dryRun, full, accept, &out)).To(Succeed())
|
||||
Expect(reprocessArtwork(ctx, ds, selected, sources, imageAgents, dryRun, accept, &out)).To(Succeed())
|
||||
|
||||
Expect(ds.Property(ctx).Get(consts.ArtConfFingerprintPropertyKey)).To(Equal(expected()))
|
||||
want := "stale-fingerprint"
|
||||
if applied {
|
||||
want = artwork.ConfigFingerprint()
|
||||
}
|
||||
Expect(ds.Property(ctx).Get(consts.ArtConfFingerprintPropertyKey)).To(Equal(want))
|
||||
},
|
||||
Entry("--all, unfiltered", nil, false, true, artwork.ConfigFingerprint),
|
||||
Entry("filtered by source", []string{"external:deezer"}, false, false, unchangedFingerprint),
|
||||
Entry("a dry run applies nothing", nil, true, true, unchangedFingerprint),
|
||||
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, false, accept, &out)).To(Succeed())
|
||||
Expect(reprocessArtwork(ctx, ds, kinds, []string{"external:deezer"}, imageAgents, false, accept, &out)).To(Succeed())
|
||||
|
||||
Expect(queue.Count()).To(Equal(int64(2)))
|
||||
queued, err := queue.Get(model.KindAlbumArtwork, "al-1", model.ImageTypePrimary)
|
||||
@ -639,7 +641,7 @@ var _ = Describe("reprocessArtwork", func() {
|
||||
})
|
||||
|
||||
It("targets the absent state", func() {
|
||||
Expect(reprocessArtwork(ctx, ds, kinds, []string{""}, imageAgents, false, false, accept, &out)).To(Succeed())
|
||||
Expect(reprocessArtwork(ctx, ds, kinds, []string{""}, imageAgents, false, accept, &out)).To(Succeed())
|
||||
|
||||
Expect(queue.Count()).To(Equal(int64(1)))
|
||||
_, err := queue.Get(model.KindArtistArtwork, "ar-2", model.ImageTypePrimary)
|
||||
@ -650,7 +652,7 @@ var _ = Describe("reprocessArtwork", func() {
|
||||
Expect(queue.Enqueue(model.ArtworkQueueItem{ItemKind: "ar", ItemID: "ar-1",
|
||||
ImageType: model.ImageTypePrimary, Priority: model.ArtworkPriorityBump})).To(Succeed())
|
||||
|
||||
Expect(reprocessArtwork(ctx, ds, kinds, []string{"external:deezer"}, imageAgents, false, false, accept, &out)).To(Succeed())
|
||||
Expect(reprocessArtwork(ctx, ds, kinds, []string{"external:deezer"}, imageAgents, false, accept, &out)).To(Succeed())
|
||||
|
||||
Expect(out.String()).To(ContainSubstring("Queued 1 of 2 matched items"))
|
||||
Expect(out.String()).To(ContainSubstring("Already queued, left unchanged: 1"))
|
||||
@ -661,7 +663,7 @@ var _ = Describe("reprocessArtwork", func() {
|
||||
})
|
||||
|
||||
It("stops at a selection that matches nothing instead of prompting", func() {
|
||||
Expect(reprocessArtwork(ctx, ds, []model.Kind{model.KindRadioArtwork}, nil, imageAgents, false, false,
|
||||
Expect(reprocessArtwork(ctx, ds, []model.Kind{model.KindRadioArtwork}, nil, imageAgents, false,
|
||||
func(io.Writer, int64, int64) bool {
|
||||
Fail("must not prompt when there is nothing to queue")
|
||||
return true
|
||||
@ -672,14 +674,14 @@ var _ = Describe("reprocessArtwork", func() {
|
||||
})
|
||||
|
||||
It("reports an empty selection as a dry run when one was asked for", func() {
|
||||
Expect(reprocessArtwork(ctx, ds, []model.Kind{model.KindRadioArtwork}, nil, imageAgents, true, false, accept, &out)).To(Succeed())
|
||||
Expect(reprocessArtwork(ctx, ds, []model.Kind{model.KindRadioArtwork}, nil, imageAgents, true, accept, &out)).To(Succeed())
|
||||
|
||||
Expect(out.String()).To(ContainSubstring("Nothing matches"))
|
||||
Expect(out.String()).To(ContainSubstring("Dry run"))
|
||||
})
|
||||
|
||||
It("shows the external estimate on a dry run, which never reaches the prompt", func() {
|
||||
Expect(reprocessArtwork(ctx, ds, []model.Kind{model.KindAlbumArtwork}, nil, imageAgents, true, false, accept, &out)).To(Succeed())
|
||||
Expect(reprocessArtwork(ctx, ds, []model.Kind{model.KindAlbumArtwork}, nil, imageAgents, true, accept, &out)).To(Succeed())
|
||||
|
||||
Expect(out.String()).To(ContainSubstring("External lookups: ~2 estimated"))
|
||||
})
|
||||
@ -689,14 +691,14 @@ var _ = Describe("reprocessArtwork", func() {
|
||||
var external int64
|
||||
capture := func(_ io.Writer, _, e int64) bool { external = e; return false }
|
||||
|
||||
Expect(reprocessArtwork(ctx, ds, kinds, nil, imageAgents, false, false, capture, &out)).To(Succeed())
|
||||
Expect(reprocessArtwork(ctx, ds, kinds, nil, imageAgents, false, capture, &out)).To(Succeed())
|
||||
|
||||
Expect(external).To(Equal(int64(2*2+2*3)), "2 artists at 2 agents plus 2 albums at 3 agents")
|
||||
Expect(out.String()).To(ContainSubstring("External lookups: ~10 estimated"))
|
||||
})
|
||||
|
||||
It("names the estimate's blind spots instead of claiming a bound it cannot hold", func() {
|
||||
Expect(reprocessArtwork(ctx, ds, kinds, nil, imageAgents, true, false, accept, &out)).To(Succeed())
|
||||
Expect(reprocessArtwork(ctx, ds, kinds, nil, imageAgents, true, accept, &out)).To(Succeed())
|
||||
|
||||
// The count includes plugin agents once they load, so the caveat is about a failed load,
|
||||
// not about plugins being invisible to the CLI.
|
||||
@ -710,7 +712,7 @@ var _ = Describe("reprocessArtwork", func() {
|
||||
conf.Server.CoverArtPriority = "cover.*"
|
||||
put(model.KindPlaylistArtwork, "pl-1", "playlist")
|
||||
|
||||
Expect(reprocessArtwork(ctx, ds, []model.Kind{model.KindPlaylistArtwork}, nil, imageAgents, true, false, accept, &out)).To(Succeed())
|
||||
Expect(reprocessArtwork(ctx, ds, []model.Kind{model.KindPlaylistArtwork}, nil, imageAgents, true, accept, &out)).To(Succeed())
|
||||
|
||||
Expect(out.String()).To(ContainSubstring("External lookups: none"))
|
||||
})
|
||||
@ -722,7 +724,7 @@ var _ = Describe("reprocessArtwork", func() {
|
||||
var external int64
|
||||
capture := func(_ io.Writer, _, e int64) bool { external = e; return false }
|
||||
|
||||
Expect(reprocessArtwork(ctx, ds, []model.Kind{model.KindPlaylistArtwork}, nil, imageAgents, false, false, capture, &out)).To(Succeed())
|
||||
Expect(reprocessArtwork(ctx, ds, []model.Kind{model.KindPlaylistArtwork}, nil, imageAgents, false, capture, &out)).To(Succeed())
|
||||
|
||||
Expect(external).To(Equal(int64(1)))
|
||||
Expect(out.String()).To(ContainSubstring("External lookups: ~1 estimated"))
|
||||
@ -734,7 +736,7 @@ var _ = Describe("reprocessArtwork", func() {
|
||||
var external int64
|
||||
capture := func(_ io.Writer, _, e int64) bool { external = e; return false }
|
||||
|
||||
Expect(reprocessArtwork(ctx, ds, []model.Kind{model.KindPlaylistArtwork}, nil, imageAgents, false, false, capture, &out)).To(Succeed())
|
||||
Expect(reprocessArtwork(ctx, ds, []model.Kind{model.KindPlaylistArtwork}, nil, imageAgents, false, capture, &out)).To(Succeed())
|
||||
|
||||
Expect(external).To(Equal(int64(artwork.PlaylistGridSamples*3)),
|
||||
"one playlist samples 4 albums, each walking all 3 album agents")
|
||||
@ -746,14 +748,14 @@ var _ = Describe("reprocessArtwork", func() {
|
||||
capture := func(_ io.Writer, t, e int64) bool { total, external = t, e; return false }
|
||||
|
||||
Expect(reprocessArtwork(ctx, ds, []model.Kind{model.KindAlbumArtwork, model.KindRadioArtwork},
|
||||
nil, imageAgents, false, false, capture, &out)).To(Succeed())
|
||||
nil, imageAgents, false, capture, &out)).To(Succeed())
|
||||
|
||||
Expect(total).To(Equal(int64(3)))
|
||||
Expect(external).To(Equal(int64(2)), "radio artwork never reaches an external agent")
|
||||
})
|
||||
|
||||
It("rejects an unknown source and names the ones in use", func() {
|
||||
err := reprocessArtwork(ctx, ds, kinds, []string{"externa:deezer"}, imageAgents, true, false, accept, &out)
|
||||
err := reprocessArtwork(ctx, ds, kinds, []string{"externa:deezer"}, imageAgents, true, accept, &out)
|
||||
|
||||
Expect(err).To(HaveOccurred())
|
||||
Expect(err.Error()).To(ContainSubstring("externa:deezer"))
|
||||
@ -767,18 +769,18 @@ var _ = Describe("reprocessArtwork", func() {
|
||||
put(model.KindArtistArtwork, "ar-2", "folder")
|
||||
|
||||
Expect(reprocessArtwork(ctx, ds, kinds, repositorySources([]string{absentSource}),
|
||||
imageAgents, false, false, accept, &out)).To(Succeed(),
|
||||
imageAgents, false, accept, &out)).To(Succeed(),
|
||||
"a reserved source must stay valid once the library has none of it")
|
||||
Expect(out.String()).To(ContainSubstring("Nothing matches"))
|
||||
Expect(queue.Count()).To(BeZero())
|
||||
|
||||
Expect(reprocessArtwork(ctx, ds, kinds, repositorySources([]string{"absnt"}),
|
||||
imageAgents, true, false, accept, &out)).ToNot(Succeed(), "a typo must still be rejected")
|
||||
imageAgents, true, accept, &out)).ToNot(Succeed(), "a typo must still be rejected")
|
||||
})
|
||||
|
||||
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, false, decline, &out)).To(Succeed())
|
||||
imageAgents, false, decline, &out)).To(Succeed())
|
||||
|
||||
Expect(out.String()).To(ContainSubstring("Nothing matches"),
|
||||
"a well-formed filter must not be reported as a typo because of the kinds selected")
|
||||
|
||||
@ -385,7 +385,7 @@ func scheduleArtworkHousekeeping(ctx context.Context, worker *artwork.Worker) fu
|
||||
log.Error(ctx, "Error enqueueing missing artwork rechecks", err)
|
||||
}
|
||||
|
||||
if err := worker.CheckConfig(ctx); err != nil {
|
||||
if err := worker.ReconcileConfig(ctx); err != nil {
|
||||
log.Error(ctx, "Error checking the artwork config fingerprint", err)
|
||||
}
|
||||
return nil
|
||||
|
||||
@ -31,10 +31,10 @@ func KeepsState(kind model.Kind) bool { return kind != model.KindDiscArtwork }
|
||||
var RefreshableKinds = append(slices.Clone(ReprocessKinds), model.KindMediaFileArtwork)
|
||||
|
||||
// settlesAbsentOnGiveUp reports whether an exhausted retry budget records an absent state. Media
|
||||
// files are excluded: only a view enqueues them, and an absent row is what stops a view from doing so.
|
||||
// files are excluded: a track with no row still falls back to its disc or album art.
|
||||
func settlesAbsentOnGiveUp(prefix string) bool {
|
||||
kind, ok := model.ParseKind(prefix)
|
||||
return ok && slices.Contains(ReprocessKinds, kind)
|
||||
return ok && KeepsState(kind) && kind != model.KindMediaFileArtwork
|
||||
}
|
||||
|
||||
// artworkEpoch invalidates all resolution state when bumped; bump it whenever resolution semantics change.
|
||||
@ -65,9 +65,9 @@ func ConfigFingerprint() string {
|
||||
return fmt.Sprintf("%016x", xxh3.Hash([]byte(raw)))
|
||||
}
|
||||
|
||||
// CheckConfigFingerprint warns when the artwork config changed since the library was last
|
||||
// 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 CheckConfigFingerprint(ctx context.Context, ds model.DataStore) error {
|
||||
func ReconcileConfigFingerprint(ctx context.Context, ds model.DataStore) error {
|
||||
current := ConfigFingerprint()
|
||||
stored, err := ds.Property(ctx).DefaultGet(consts.ArtConfFingerprintPropertyKey, "")
|
||||
if err != nil {
|
||||
@ -76,7 +76,7 @@ func CheckConfigFingerprint(ctx context.Context, ds model.DataStore) error {
|
||||
switch stored {
|
||||
case current:
|
||||
case "":
|
||||
// A library with no stored fingerprint has nothing resolved under an older one to warn about.
|
||||
// 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 "+
|
||||
|
||||
@ -3,41 +3,16 @@ package artwork
|
||||
import (
|
||||
"context"
|
||||
"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
|
||||
}
|
||||
|
||||
// orderTrackingQueueRepo records the kind of each bulk insert, so tests can assert
|
||||
// phase ordering (artists-first) that same-priority timestamps can't guarantee.
|
||||
type orderTrackingQueueRepo struct {
|
||||
@ -138,9 +113,9 @@ var _ = Describe("Housekeeping", func() {
|
||||
})
|
||||
})
|
||||
|
||||
Describe("CheckConfigFingerprint", func() {
|
||||
Describe("ReconcileConfigFingerprint", func() {
|
||||
It("records the current fingerprint when none was ever stored", func() {
|
||||
Expect(CheckConfigFingerprint(ctx, ds)).To(Succeed())
|
||||
Expect(ReconcileConfigFingerprint(ctx, ds)).To(Succeed())
|
||||
|
||||
Expect(propRepo.Get(consts.ArtConfFingerprintPropertyKey)).To(Equal(ConfigFingerprint()))
|
||||
Expect(queueRepo.Count()).To(BeZero())
|
||||
@ -149,23 +124,10 @@ var _ = Describe("Housekeeping", func() {
|
||||
It("leaves a stale fingerprint stored, so the warning survives a restart", func() {
|
||||
Expect(propRepo.Put(consts.ArtConfFingerprintPropertyKey, "stale-fingerprint")).To(Succeed())
|
||||
|
||||
Expect(CheckConfigFingerprint(ctx, ds)).To(Succeed())
|
||||
Expect(ReconcileConfigFingerprint(ctx, ds)).To(Succeed())
|
||||
|
||||
Expect(propRepo.Get(consts.ArtConfFingerprintPropertyKey)).To(Equal("stale-fingerprint"))
|
||||
})
|
||||
|
||||
DescribeTable("never enqueues anything, whatever the stored fingerprint",
|
||||
func(stored func() string) {
|
||||
if v := stored(); v != "" {
|
||||
Expect(propRepo.Put(consts.ArtConfFingerprintPropertyKey, v)).To(Succeed())
|
||||
}
|
||||
Expect(CheckConfigFingerprint(ctx, ds)).To(Succeed())
|
||||
Expect(queueRepo.Count()).To(BeZero())
|
||||
},
|
||||
Entry("never stored", func() string { return "" }),
|
||||
Entry("changed", func() string { return "stale-fingerprint" }),
|
||||
Entry("unchanged", ConfigFingerprint),
|
||||
)
|
||||
})
|
||||
|
||||
Describe("MarkConfigApplied", func() {
|
||||
@ -194,8 +156,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())
|
||||
|
||||
@ -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,9 +131,9 @@ func (w *Worker) RunPrune(ctx context.Context) error {
|
||||
return prune(ctx, w.proc.ds, w.proc.store)
|
||||
}
|
||||
|
||||
// CheckConfig warns when the artwork config changed since the library was last resolved under it.
|
||||
func (w *Worker) CheckConfig(ctx context.Context) error {
|
||||
return CheckConfigFingerprint(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
|
||||
|
||||
@ -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
|
||||
|
||||
@ -103,7 +103,8 @@ type ArtworkQueueItem struct {
|
||||
// Queue priorities: higher drains first.
|
||||
const (
|
||||
ArtworkPriorityRecheck = 0
|
||||
// ArtworkPriorityBackfill is never enqueued; the CLI keeps it to name and cancel rows still on it.
|
||||
// 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
|
||||
@ -160,8 +161,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 how many states of a kind resolved to no image.
|
||||
CountAbsent(kind Kind) (int64, 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.
|
||||
|
||||
@ -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
|
||||
}
|
||||
|
||||
@ -223,11 +223,4 @@ func (r *artworkQueueRepository) Count() (int64, error) {
|
||||
return res.Count, err
|
||||
}
|
||||
|
||||
func (r *artworkQueueRepository) CountAbsent(kind model.Kind) (int64, error) {
|
||||
var res struct{ Count int64 }
|
||||
err := r.queryOne(Select("count(*) as count").From(itemArtworkTable).
|
||||
Where(Eq{"item_kind": kind.Prefix(), "hash": ""}), &res)
|
||||
return res.Count, err
|
||||
}
|
||||
|
||||
var _ model.ArtworkQueueRepository = (*artworkQueueRepository)(nil)
|
||||
|
||||
@ -404,24 +404,6 @@ var _ = Describe("ArtworkQueueRepository", func() {
|
||||
Expect(repo.CountQueued(nil, nil)).To(BeEmpty())
|
||||
})
|
||||
|
||||
It("counts absent states", 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},
|
||||
} {
|
||||
Expect(awRepo.PutItemArtwork(&ia)).To(Succeed())
|
||||
}
|
||||
|
||||
Expect(repo.CountAbsent(model.KindArtistArtwork)).To(Equal(int64(2)))
|
||||
})
|
||||
|
||||
It("reports a kind with no absent state as zero, not as an error", func() {
|
||||
Expect(repo.CountAbsent(model.KindRadioArtwork)).To(Equal(int64(0)))
|
||||
})
|
||||
})
|
||||
|
||||
Describe("PurgeQueued", func() {
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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{
|
||||
|
||||
@ -226,22 +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) (int64, error) {
|
||||
m.mu.Lock()
|
||||
defer m.mu.Unlock()
|
||||
if m.Err != nil || m.ItemArtworkSource == nil {
|
||||
return 0, m.Err
|
||||
}
|
||||
var total int64
|
||||
for _, ia := range m.ItemArtworkSource.ItemData {
|
||||
if ia.ItemKind == kind.Prefix() && ia.Hash == "" {
|
||||
total++
|
||||
}
|
||||
}
|
||||
return total, nil
|
||||
}
|
||||
|
||||
func (m *MockArtworkQueueRepo) EnqueuePreservingBackoff(items ...model.ArtworkQueueItem) error {
|
||||
m.mu.Lock()
|
||||
defer m.mu.Unlock()
|
||||
|
||||
@ -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")
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user