nik/navidrome
1
0
Fork 0
mirror of https://github.com/navidrome/navidrome.git synced 2026-08-01 07:21:17 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(artwork): expose housekeeping through the Worker

Browse Source 
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.
This commit is contained in:
Deluan 2026-07-26 22:03:51 -04:00
parent 7efb4d1468
commit 1ceb8ca723
4 changed files with 44 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
cmd/root.go
View File

@ -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

18
core/artwork/housekeeping.go
View File

@ -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 {

32
core/artwork/housekeeping_test.go
View File

@ -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 {

15
core/artwork/worker.go
View File

@ -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.