diff --git a/cmd/root.go b/cmd/root.go index 4df5c1cb8..799dd6ab5 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -368,6 +368,9 @@ func scheduleArtworkHousekeeping(ctx context.Context, worker *artwork.Worker) fu if err := artwork.EnqueueStaleAbsentAll(ctx, ds); err != nil { log.Error(ctx, "Error enqueueing stale artwork rechecks", err) } + if err := artwork.EnqueueMissingAll(ctx, ds); err != nil { + log.Error(ctx, "Error enqueueing missing artwork rechecks", err) + } }); err != nil { log.Error(ctx, "Error scheduling artwork stale-absent recheck", err) } @@ -380,6 +383,12 @@ func scheduleArtworkHousekeeping(ctx context.Context, worker *artwork.Worker) fu log.Error(ctx, "Error scheduling artwork prune", err) } + // 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 { + log.Error(ctx, "Error enqueueing missing artwork rechecks", err) + } + backfilled, err := artwork.Backfill(ctx, ds) if err != nil { log.Error(ctx, "Error running artwork backfill", err) diff --git a/core/artwork/housekeeping.go b/core/artwork/housekeeping.go index 400c48a69..99172f017 100644 --- a/core/artwork/housekeeping.go +++ b/core/artwork/housekeeping.go @@ -8,7 +8,6 @@ import ( "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" @@ -21,17 +20,22 @@ const FingerprintPropertyKey = "artwork.fingerprint" // staleAbsentAge is how old an absent resolution must be before the recheck job retries it. const staleAbsentAge = 24 * time.Hour -// staleAbsentKinds are the item kinds eligible for the periodic stale-absent recheck. -var staleAbsentKinds = []model.Kind{ +// recheckKinds are the item kinds eligible for the periodic recheck jobs (stale-absent and +// missing-row). Media files are excluded: they resolve embedded-only, at scan or on view. +var recheckKinds = []model.Kind{ model.KindArtistArtwork, model.KindAlbumArtwork, model.KindPlaylistArtwork, model.KindRadioArtwork, } -// Fingerprint summarizes the config knobs that affect artwork resolution outcomes; a +// artworkEpoch invalidates all resolution state when bumped; bump it in the same change that +// 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 // change means previously resolved (or absent) state may no longer be correct. func Fingerprint() string { - raw := fmt.Sprintf("%s|%s|%s|%s|%t|%t|%s", + 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, consts.Version) + conf.Server.Agents, conf.Server.EnableExternalServices, conf.Server.EnableM3UExternalAlbumArt, artworkEpoch) sum := md5.Sum([]byte(raw)) //nolint:gosec // fingerprint, not security-sensitive return hex.EncodeToString(sum[:]) } @@ -95,10 +99,22 @@ func enqueueBackfillKind(ctx context.Context, ds model.DataStore, kind model.Kin func EnqueueStaleAbsentAll(ctx context.Context, ds model.DataStore) error { cutoff := time.Now().Add(-staleAbsentAge) queue := ds.ArtworkQueue(ctx) - for _, kind := range staleAbsentKinds { + for _, kind := range recheckKinds { if _, err := queue.EnqueueStaleAbsent(kind, cutoff); err != nil { return err } } return nil } + +// 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 { + queue := ds.ArtworkQueue(ctx) + for _, kind := range recheckKinds { + if _, err := queue.EnqueueMissing(kind); err != nil { + return err + } + } + return nil +} diff --git a/core/artwork/housekeeping_test.go b/core/artwork/housekeeping_test.go index 1df8312e8..cd182ca22 100644 --- a/core/artwork/housekeeping_test.go +++ b/core/artwork/housekeeping_test.go @@ -6,6 +6,7 @@ import ( "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" @@ -110,6 +111,15 @@ var _ = Describe("Housekeeping", func() { conf.Server.EnableM3UExternalAlbumArt = true 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() + consts.Version = original + "-next" + Expect(Fingerprint()).To(Equal(f1), + "the version must not invalidate artwork state: it would re-resolve every entity on every build") + }) }) Describe("Backfill", func() { @@ -223,4 +233,38 @@ var _ = Describe("Housekeeping", func() { Expect(findQueued(queueRepo.MockArtworkQueueRepo, "al", "al2")).To(BeNil()) }) }) + + Describe("EnqueueMissingAll", func() { + var artRepo *tests.MockArtworkRepo + + BeforeEach(func() { + artRepo = tests.CreateMockArtworkRepo() + ds.MockedArtwork = artRepo + queueRepo.ItemArtworkSource = artRepo + queueRepo.ExistingIDs = map[string]map[string]bool{ + "al": {"al1": true, "al2": true}, + "ar": {"ar1": true}, + "pl": {"pl1": true}, + "ra": {"ra1": true}, + } + }) + + It("enqueues only entities that have no item_artwork row, across all kinds", func() { + // al1 is already resolved and ar1 already absent: both must be skipped. + 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) + Expect(err).ToNot(HaveOccurred()) + + 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()) + }) + }) }) diff --git a/model/artwork.go b/model/artwork.go index 8bbd7a2cb..b2dae0024 100644 --- a/model/artwork.go +++ b/model/artwork.go @@ -112,6 +112,9 @@ type ArtworkQueueRepository interface { Count() (int64, error) // EnqueueStaleAbsent inserts queue rows (priority Recheck) for absent states older than cutoff. EnqueueStaleAbsent(kind Kind, attemptedBefore time.Time) (int64, error) + // EnqueueMissing inserts queue rows (priority Recheck) for entities of the kind that have no + // item_artwork row at all, so a never-processed entity is eventually resolved even without a scan. + EnqueueMissing(kind Kind) (int64, error) // PurgeDangling removes queue rows whose entity no longer exists. PurgeDangling() (int64, error) } diff --git a/persistence/artwork_queue_repository.go b/persistence/artwork_queue_repository.go index ba8c42f95..7fd7cd387 100644 --- a/persistence/artwork_queue_repository.go +++ b/persistence/artwork_queue_repository.go @@ -2,6 +2,7 @@ package persistence import ( "context" + "fmt" "slices" "time" @@ -120,4 +121,20 @@ func (r *artworkQueueRepository) EnqueueStaleAbsent(kind model.Kind, attemptedBe return r.executeSQL(ins) } +func (r *artworkQueueRepository) EnqueueMissing(kind model.Kind) (int64, error) { + entityTable, ok := danglingItemArtworkKinds[kind] + if !ok { + return 0, fmt.Errorf("artwork queue: no entity table for kind %q", kind.Prefix()) + } + now := time.Now() + // DO NOTHING is deliberate: rechecks must not bump priority/retry_at of already-queued items. + ins := Expr(`INSERT INTO `+r.tableName+` (item_kind, item_id, image_type, priority, attempts, retry_at, enqueued_at) + SELECT ?, id, ?, ?, 0, ?, ? + FROM `+entityTable+` + WHERE id NOT IN (SELECT item_id FROM `+itemArtworkTable+` WHERE item_kind = ?) + ON CONFLICT (item_kind, item_id, image_type) DO NOTHING`, + kind.Prefix(), model.ImageTypePrimary, model.ArtworkPriorityRecheck, now, now, kind.Prefix()) + return r.executeSQL(ins) +} + var _ model.ArtworkQueueRepository = (*artworkQueueRepository)(nil) diff --git a/persistence/artwork_queue_repository_test.go b/persistence/artwork_queue_repository_test.go index 10015a225..e7c0967a1 100644 --- a/persistence/artwork_queue_repository_test.go +++ b/persistence/artwork_queue_repository_test.go @@ -184,4 +184,45 @@ var _ = Describe("ArtworkQueueRepository", func() { Expect(items[0].ItemID).To(Equal("stale1")) Expect(items[0].Priority).To(Equal(model.ArtworkPriorityRecheck)) }) + + It("enqueues entities that have no item_artwork row at all", func() { + awRepo := NewArtworkRepository(context.Background(), GetDBXBuilder()) + // albumSgtPeppers has a resolved row and albumAbbeyRoad an absent row: both already + // processed, so neither should be enqueued as "missing". + Expect(awRepo.PutItemArtwork(&model.ItemArtwork{ItemKind: "al", ItemID: albumSgtPeppers.ID, ImageType: model.ImageTypePrimary, Hash: "hX", AttemptedAt: time.Now()})).To(Succeed()) + Expect(awRepo.PutItemArtwork(&model.ItemArtwork{ItemKind: "al", ItemID: albumAbbeyRoad.ID, ImageType: model.ImageTypePrimary, Hash: "", AttemptedAt: time.Now()})).To(Succeed()) + + n, err := repo.EnqueueMissing(model.KindAlbumArtwork) + Expect(err).ToNot(HaveOccurred()) + Expect(n).To(BeNumerically(">=", 1)) + + got, err := repo.DequeueBatch(1000) + Expect(err).ToNot(HaveOccurred()) + ids := make([]string, 0, len(got)) + for _, it := range got { + Expect(it.ItemKind).To(Equal("al")) + Expect(it.Priority).To(Equal(model.ArtworkPriorityRecheck)) + ids = append(ids, it.ItemID) + } + Expect(ids).To(ContainElement(albumRadioactivity.ID), "an album with no row must be enqueued") + Expect(ids).ToNot(ContainElement(albumSgtPeppers.ID), "a resolved album must not be re-enqueued") + Expect(ids).ToNot(ContainElement(albumAbbeyRoad.ID), "an absent-state album must not be enqueued as missing") + }) + + It("does not disturb an already-queued entity when enqueueing missing rows", func() { + Expect(repo.Enqueue(item("al", albumRadioactivity.ID, model.ArtworkPriorityBump))).To(Succeed()) + + _, err := repo.EnqueueMissing(model.KindAlbumArtwork) + Expect(err).ToNot(HaveOccurred()) + + got, _ := repo.DequeueBatch(1000) + var count int + for _, it := range got { + if it.ItemID == albumRadioactivity.ID { + count++ + Expect(it.Priority).To(Equal(model.ArtworkPriorityBump), "existing bump priority must survive") + } + } + Expect(count).To(Equal(1), "the already-queued row must not be duplicated") + }) }) diff --git a/tests/mock_artwork_queue_repo.go b/tests/mock_artwork_queue_repo.go index 43bc6218d..19bb3b6e8 100644 --- a/tests/mock_artwork_queue_repo.go +++ b/tests/mock_artwork_queue_repo.go @@ -214,3 +214,45 @@ func (m *MockArtworkQueueRepo) EnqueueStaleAbsent(kind model.Kind, attemptedBefo } return inserted, nil } + +// EnqueueMissing enqueues entities in ExistingIDs[kind] that have no item_artwork row in +// ItemArtworkSource and are not already queued, mirroring the SQL set-difference insert. +func (m *MockArtworkQueueRepo) EnqueueMissing(kind model.Kind) (int64, error) { + m.mu.Lock() + defer m.mu.Unlock() + if m.Err != nil { + return 0, m.Err + } + hasRow := func(id string) bool { + if m.ItemArtworkSource == nil { + return false + } + for _, ia := range m.ItemArtworkSource.ItemData { + if ia.ItemKind == kind.Prefix() && ia.ItemID == id { + return true + } + } + return false + } + now := time.Now() + var inserted int64 + for id := range m.ExistingIDs[kind.Prefix()] { + if hasRow(id) { + continue + } + k := iaKey(kind.Prefix(), id, model.ImageTypePrimary) + if _, ok := m.Data[k]; ok { // DO NOTHING: never touch existing queue rows + continue + } + m.Data[k] = model.ArtworkQueueItem{ + ItemKind: kind.Prefix(), + ItemID: id, + ImageType: model.ImageTypePrimary, + Priority: model.ArtworkPriorityRecheck, + RetryAt: now, + EnqueuedAt: now, + } + inserted++ + } + return inserted, nil +}