diff --git a/cmd/artwork.go b/cmd/artwork.go index 55aebd1bf..2fb696ca4 100644 --- a/cmd/artwork.go +++ b/cmd/artwork.go @@ -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) { diff --git a/cmd/artwork_test.go b/cmd/artwork_test.go index 8a3d92825..6a3ade04a 100644 --- a/cmd/artwork_test.go +++ b/cmd/artwork_test.go @@ -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") diff --git a/cmd/root.go b/cmd/root.go index a7876401b..a44eb743b 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -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 diff --git a/core/artwork/housekeeping.go b/core/artwork/housekeeping.go index cb714ac43..2e07fb296 100644 --- a/core/artwork/housekeeping.go +++ b/core/artwork/housekeeping.go @@ -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 "+ diff --git a/core/artwork/housekeeping_test.go b/core/artwork/housekeeping_test.go index 594613dba..9a108a0a9 100644 --- a/core/artwork/housekeeping_test.go +++ b/core/artwork/housekeeping_test.go @@ -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()) diff --git a/core/artwork/worker.go b/core/artwork/worker.go index 120d6ac31..bb09be55e 100644 --- a/core/artwork/worker.go +++ b/core/artwork/worker.go @@ -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 diff --git a/core/artwork/worker_test.go b/core/artwork/worker_test.go index bac46f12b..ebb8de251 100644 --- a/core/artwork/worker_test.go +++ b/core/artwork/worker_test.go @@ -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 diff --git a/model/artwork.go b/model/artwork.go index 578bdf144..d4413ec9c 100644 --- a/model/artwork.go +++ b/model/artwork.go @@ -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. diff --git a/model/radio.go b/model/radio.go index 466ff48b0..013a24beb 100644 --- a/model/radio.go +++ b/model/radio.go @@ -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 } diff --git a/persistence/artwork_queue_repository.go b/persistence/artwork_queue_repository.go index 6ea5b5fa3..3ad368772 100644 --- a/persistence/artwork_queue_repository.go +++ b/persistence/artwork_queue_repository.go @@ -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) diff --git a/persistence/artwork_queue_repository_test.go b/persistence/artwork_queue_repository_test.go index 830de4e5e..a4a1ea76f 100644 --- a/persistence/artwork_queue_repository_test.go +++ b/persistence/artwork_queue_repository_test.go @@ -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() { diff --git a/persistence/radio_repository.go b/persistence/radio_repository.go index b73487e40..816faf134 100644 --- a/persistence/radio_repository.go +++ b/persistence/radio_repository.go @@ -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 diff --git a/persistence/radio_repository_test.go b/persistence/radio_repository_test.go index e2564455d..65b800548 100644 --- a/persistence/radio_repository_test.go +++ b/persistence/radio_repository_test.go @@ -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{ diff --git a/tests/mock_artwork_queue_repo.go b/tests/mock_artwork_queue_repo.go index 2e954053c..a1b7b3e3d 100644 --- a/tests/mock_artwork_queue_repo.go +++ b/tests/mock_artwork_queue_repo.go @@ -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() diff --git a/tests/mock_radio_repository.go b/tests/mock_radio_repository.go index 2baeadc5c..20f81ec45 100644 --- a/tests/mock_radio_repository.go +++ b/tests/mock_radio_repository.go @@ -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")