From 1ceb8ca723c96e116cc7d71998608c955e9fdac0 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 26 Jul 2026 22:03:51 -0400 Subject: [PATCH] refactor(artwork): expose housekeeping through the Worker scheduleArtworkHousekeeping received a *artwork.Worker and then called CreateDataStore() for a second handle onto the state that Worker already owns, because Backfill, EnqueueStaleAbsentAll and EnqueueMissingAll were free functions taking a DataStore. They are now Worker methods over unexported implementations, the same shape prune/RunPrune already uses: one public path, and the specs keep calling the plain function with a mock store instead of standing up a Worker. Fingerprint is unexported too -- nothing outside the package used it. --- cmd/root.go | 9 ++++----- core/artwork/housekeeping.go | 18 ++++++++--------- core/artwork/housekeeping_test.go | 32 +++++++++++++++---------------- core/artwork/worker.go | 15 +++++++++++++++ 4 files changed, 44 insertions(+), 30 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index 799dd6ab5..c4f6e9473 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -361,14 +361,13 @@ func startArtworkWorker(ctx context.Context, worker *artwork.Worker) func() erro // recurring stale-absent recheck and prune jobs. Scan-triggered prune lands in a later phase. func scheduleArtworkHousekeeping(ctx context.Context, worker *artwork.Worker) func() error { return func() error { - ds := CreateDataStore() schedulerInstance := scheduler.GetInstance() if _, err := schedulerInstance.Add(consts.ArtworkStaleAbsentRecheckSchedule, func() { - if err := artwork.EnqueueStaleAbsentAll(ctx, ds); err != nil { + if err := worker.EnqueueStaleAbsentAll(ctx); err != nil { log.Error(ctx, "Error enqueueing stale artwork rechecks", err) } - if err := artwork.EnqueueMissingAll(ctx, ds); err != nil { + if err := worker.EnqueueMissingAll(ctx); err != nil { log.Error(ctx, "Error enqueueing missing artwork rechecks", err) } }); err != nil { @@ -385,11 +384,11 @@ func scheduleArtworkHousekeeping(ctx context.Context, worker *artwork.Worker) fu // Also run the missing-row recheck once at startup so a never-scanned entity is picked up // immediately, not only on the next hourly tick (e.g. after enabling the feature). - if err := artwork.EnqueueMissingAll(ctx, ds); err != nil { + if err := worker.EnqueueMissingAll(ctx); err != nil { log.Error(ctx, "Error enqueueing missing artwork rechecks", err) } - backfilled, err := artwork.Backfill(ctx, ds) + backfilled, err := worker.Backfill(ctx) if err != nil { log.Error(ctx, "Error running artwork backfill", err) return nil diff --git a/core/artwork/housekeeping.go b/core/artwork/housekeeping.go index aa444642d..fb7c9de4c 100644 --- a/core/artwork/housekeeping.go +++ b/core/artwork/housekeeping.go @@ -35,9 +35,9 @@ func hasRecheckPath(prefix string) bool { // alters resolution semantics. Deliberately not the server version, which changes every build. const artworkEpoch = 1 -// Fingerprint summarizes the inputs that affect artwork resolution outcomes; a +// fingerprint summarizes the inputs that affect artwork resolution outcomes; a // change means previously resolved (or absent) state may no longer be correct. -func Fingerprint() string { +func fingerprint() string { raw := fmt.Sprintf("%s|%s|%s|%s|%t|%t|%d", conf.Server.CoverArtPriority, conf.Server.ArtistArtPriority, conf.Server.ArtistImageFolder, conf.Server.Agents, conf.Server.EnableExternalServices, conf.Server.EnableM3UExternalAlbumArt, artworkEpoch) @@ -45,11 +45,11 @@ func Fingerprint() string { return hex.EncodeToString(sum[:]) } -// Backfill enqueues artwork resolution for every entity when the config fingerprint changed +// backfill enqueues artwork resolution for every entity when the config fingerprint changed // (or was never stored), artists first so those pages resolve before the larger backlog. -func Backfill(ctx context.Context, ds model.DataStore) (bool, error) { +func backfill(ctx context.Context, ds model.DataStore) (bool, error) { ctx = auth.WithAdminUser(ctx, ds) - current := Fingerprint() + current := fingerprint() props := ds.Property(ctx) stored, err := props.DefaultGet(consts.ArtConfFingerprintPropertyKey, "") if err != nil { @@ -99,9 +99,9 @@ func enqueueBackfillKind(ctx context.Context, ds model.DataStore, kind model.Kin return ds.ArtworkQueue(ctx).Enqueue(items...) } -// EnqueueStaleAbsentAll requeues absent-state entries older than staleAbsentAge, across +// enqueueStaleAbsentAll requeues absent-state entries older than staleAbsentAge, across // every artwork-bearing kind, for the periodic recheck job. -func EnqueueStaleAbsentAll(ctx context.Context, ds model.DataStore) error { +func enqueueStaleAbsentAll(ctx context.Context, ds model.DataStore) error { cutoff := time.Now().Add(-staleAbsentAge) queue := ds.ArtworkQueue(ctx) for _, kind := range recheckKinds { @@ -112,9 +112,9 @@ func EnqueueStaleAbsentAll(ctx context.Context, ds model.DataStore) error { return nil } -// EnqueueMissingAll requeues entities that have no item_artwork row yet, across every recheck +// enqueueMissingAll requeues entities that have no item_artwork row yet, across every recheck // kind: the safety net for entities a scan never enqueued (added between scans, or scanner off). -func EnqueueMissingAll(ctx context.Context, ds model.DataStore) error { +func enqueueMissingAll(ctx context.Context, ds model.DataStore) error { queue := ds.ArtworkQueue(ctx) for _, kind := range recheckKinds { if _, err := queue.EnqueueMissing(kind); err != nil { diff --git a/core/artwork/housekeeping_test.go b/core/artwork/housekeeping_test.go index f61bf652d..f556579b8 100644 --- a/core/artwork/housekeeping_test.go +++ b/core/artwork/housekeeping_test.go @@ -92,32 +92,32 @@ var _ = Describe("Housekeeping", func() { Describe("Fingerprint", func() { It("changes when a fingerprint-affecting config value changes", func() { - f1 := Fingerprint() + f1 := fingerprint() conf.Server.CoverArtPriority = "folder, embedded" - f2 := Fingerprint() + f2 := fingerprint() Expect(f1).NotTo(Equal(f2)) }) It("changes when ArtistImageFolder changes", func() { conf.Server.ArtistImageFolder = "/before" - f1 := Fingerprint() + f1 := fingerprint() conf.Server.ArtistImageFolder = "/after" - Expect(Fingerprint()).NotTo(Equal(f1)) + Expect(fingerprint()).NotTo(Equal(f1)) }) It("changes when EnableM3UExternalAlbumArt is toggled", func() { conf.Server.EnableM3UExternalAlbumArt = false - f1 := Fingerprint() + f1 := fingerprint() conf.Server.EnableM3UExternalAlbumArt = true - Expect(Fingerprint()).NotTo(Equal(f1)) + Expect(fingerprint()).NotTo(Equal(f1)) }) It("does not change when the server version changes", func() { original := consts.Version DeferCleanup(func() { consts.Version = original }) - f1 := Fingerprint() + f1 := fingerprint() consts.Version = original + "-next" - Expect(Fingerprint()).To(Equal(f1), + Expect(fingerprint()).To(Equal(f1), "the version must not invalidate artwork state: it would re-resolve every entity on every build") }) }) @@ -125,9 +125,9 @@ var _ = Describe("Housekeeping", func() { Describe("Backfill", func() { It("enqueues nothing and returns false when the stored fingerprint matches", func() { seedEntities() - Expect(propRepo.Put(consts.ArtConfFingerprintPropertyKey, Fingerprint())).To(Succeed()) + Expect(propRepo.Put(consts.ArtConfFingerprintPropertyKey, fingerprint())).To(Succeed()) - did, err := Backfill(ctx, ds) + did, err := backfill(ctx, ds) Expect(err).ToNot(HaveOccurred()) Expect(did).To(BeFalse()) @@ -139,7 +139,7 @@ var _ = Describe("Housekeeping", func() { It("runs the backfill when no fingerprint was ever stored", func() { seedEntities() - did, err := Backfill(ctx, ds) + did, err := backfill(ctx, ds) Expect(err).ToNot(HaveOccurred()) Expect(did).To(BeTrue()) @@ -149,7 +149,7 @@ var _ = Describe("Housekeeping", func() { stored, err := propRepo.Get(consts.ArtConfFingerprintPropertyKey) Expect(err).ToNot(HaveOccurred()) - Expect(stored).To(Equal(Fingerprint())) + Expect(stored).To(Equal(fingerprint())) }) It("enqueues a private playlist by resolving it under an admin context", func() { @@ -160,7 +160,7 @@ var _ = Describe("Housekeeping", func() { tracks: &tests.MockPlaylistTrackRepo{}, } - did, err := Backfill(ctx, vds) + did, err := backfill(ctx, vds) Expect(err).ToNot(HaveOccurred()) Expect(did).To(BeTrue()) Expect(findQueued(queueRepo.MockArtworkQueueRepo, "pl", "plPrivate")).ToNot(BeNil()) @@ -170,7 +170,7 @@ var _ = Describe("Housekeeping", func() { seedEntities() Expect(propRepo.Put(consts.ArtConfFingerprintPropertyKey, "stale-fingerprint")).To(Succeed()) - did, err := Backfill(ctx, ds) + did, err := backfill(ctx, ds) Expect(err).ToNot(HaveOccurred()) Expect(did).To(BeTrue()) @@ -218,7 +218,7 @@ var _ = Describe("Housekeeping", func() { // Not absent: has a resolved hash. artRepo.ItemData["al-resolved"] = model.ItemArtwork{ItemKind: "al", ItemID: "al2", ImageType: model.ImageTypePrimary, Hash: "somehash", AttemptedAt: old} - err := EnqueueStaleAbsentAll(ctx, ds) + err := enqueueStaleAbsentAll(ctx, ds) Expect(err).ToNot(HaveOccurred()) Expect(queueRepo.Data).To(HaveLen(4)) @@ -254,7 +254,7 @@ var _ = Describe("Housekeeping", 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()} - err := EnqueueMissingAll(ctx, ds) + err := enqueueMissingAll(ctx, ds) Expect(err).ToNot(HaveOccurred()) for _, it := range queueRepo.Data { diff --git a/core/artwork/worker.go b/core/artwork/worker.go index aa9b9bc35..3fa024c91 100644 --- a/core/artwork/worker.go +++ b/core/artwork/worker.go @@ -159,6 +159,21 @@ func (w *Worker) RunPrune(ctx context.Context) error { return prune(ctx, w.proc.ds, w.proc.store) } +// Backfill, EnqueueStaleAbsentAll and EnqueueMissingAll are the scheduler's entry points into +// housekeeping. Like RunPrune they exist so a caller needs only the Worker, not a second +// DataStore handle onto the state the Worker already owns. +func (w *Worker) Backfill(ctx context.Context) (bool, error) { + return backfill(ctx, w.proc.ds) +} + +func (w *Worker) EnqueueStaleAbsentAll(ctx context.Context) error { + return enqueueStaleAbsentAll(ctx, w.proc.ds) +} + +func (w *Worker) EnqueueMissingAll(ctx context.Context) error { + return enqueueMissingAll(ctx, w.proc.ds) +} + func (w *Worker) drain(ctx context.Context, concurrency int, kinds ...string) (int, error) { // Dequeue well past the worker pool so a slow item (an external lookup burning its // timeout) never idles the other slots: the pool stays fed until the batch runs out.