diff --git a/core/artwork/artwork.go b/core/artwork/artwork.go index 826e3ab19..93e117168 100644 --- a/core/artwork/artwork.go +++ b/core/artwork/artwork.go @@ -83,7 +83,7 @@ func (a *artwork) Get(ctx context.Context, artID model.ArtworkID, size int, squa // A vanished source must still reach the worker, or a stored hash would keep describing // artwork that no longer exists. if a.blurHashes != nil && errors.Is(err, ErrUnavailable) { - a.blurHashes.Enqueue(artID, artReader.LastUpdated(), false) + a.blurHashes.Enqueue(artID, artReader.LastUpdated(), false, true) } return nil, time.Time{}, err } @@ -93,7 +93,7 @@ func (a *artwork) Get(ctx context.Context, artID model.ArtworkID, size int, squa // readers re-fetch the original through Get, carrying the real signal); nor does cache // warmup/disabled, where every serve misses — there the LastUpdated signal applies. force := size == 0 && !square && !r.Cached && a.cache.Available(ctx) - a.blurHashes.Enqueue(artID, artReader.LastUpdated(), force) + a.blurHashes.Enqueue(artID, artReader.LastUpdated(), force, false) } return r, artReader.LastUpdated(), nil } diff --git a/core/artwork/blurhash_updater.go b/core/artwork/blurhash_updater.go index d44797642..fa0a75624 100644 --- a/core/artwork/blurhash_updater.go +++ b/core/artwork/blurhash_updater.go @@ -17,10 +17,12 @@ import ( "github.com/navidrome/navidrome/resources" ) -// enqueueRequest carries the staleness signals seen at serve time: force (image-cache miss) and -// the reader's LastUpdated, which tracks file mtimes that no entity row timestamp reflects. +// enqueueRequest carries the staleness signals seen at serve time: force (image-cache miss), +// sourceGone (the serve failed with ErrUnavailable) and the reader's LastUpdated, which tracks +// file mtimes that no entity row timestamp reflects. type enqueueRequest struct { force bool + sourceGone bool imageUpdatedAt time.Time } @@ -59,7 +61,7 @@ func newBlurHashUpdater(a *artwork) *blurHashUpdater { } } -func (u *blurHashUpdater) Enqueue(artID model.ArtworkID, imageUpdatedAt time.Time, force bool) { +func (u *blurHashUpdater) Enqueue(artID model.ArtworkID, imageUpdatedAt time.Time, force, sourceGone bool) { switch artID.Kind { case model.KindAlbumArtwork, model.KindArtistArtwork, model.KindPlaylistArtwork: default: @@ -80,6 +82,7 @@ func (u *blurHashUpdater) Enqueue(artID model.ArtworkID, imageUpdatedAt time.Tim } req := u.buffer[artID] req.force = req.force || force + req.sourceGone = req.sourceGone || sourceGone if imageUpdatedAt.After(req.imageUpdatedAt) { req.imageUpdatedAt = imageUpdatedAt } @@ -167,7 +170,10 @@ func (u *blurHashUpdater) process(ctx context.Context, artID model.ArtworkID, re if req.imageUpdatedAt.After(sig) { sig = req.imageUpdatedAt } - if !req.force { + // A gone source only forces while a hash remains to clear; afterwards the negative cache + // applies, so artwork-less entities don't re-resolve on every placeholder serve. + force := req.force || (req.sourceGone && stored != "") + if !force { // Current when computed from this row version or later; the snapshot may exceed the row // version because file mtimes (which don't move rows) are folded into it on persist. if stored != "" && storedAt != nil && !storedAt.Before(version) && !sig.After(*storedAt) { diff --git a/core/artwork/blurhash_updater_internal_test.go b/core/artwork/blurhash_updater_internal_test.go index 1807ca0d1..69c75c0c3 100644 --- a/core/artwork/blurhash_updater_internal_test.go +++ b/core/artwork/blurhash_updater_internal_test.go @@ -38,17 +38,18 @@ var _ = Describe("blurHashUpdater", func() { id := model.Album{ID: "al-1"}.CoverArtID() t1 := time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC) t2 := t1.Add(time.Hour) - u.Enqueue(id, t2, true) - u.Enqueue(id, t1, false) - u.Enqueue(model.Artist{ID: "ar-1"}.CoverArtID(), t1, false) + u.Enqueue(id, t2, true, false) + u.Enqueue(id, t1, false, true) + u.Enqueue(model.Artist{ID: "ar-1"}.CoverArtID(), t1, false, false) Expect(u.buffer).To(HaveLen(2)) Expect(u.buffer[id].force).To(BeTrue()) + Expect(u.buffer[id].sourceGone).To(BeTrue()) Expect(u.buffer[id].imageUpdatedAt).To(Equal(t2)) }) It("ignores other artwork kinds", func() { - u.Enqueue(model.ArtworkID{Kind: model.KindMediaFileArtwork, ID: "mf-1"}, time.Time{}, false) - u.Enqueue(model.ArtworkID{Kind: model.KindRadioArtwork, ID: "ra-1"}, time.Time{}, true) + u.Enqueue(model.ArtworkID{Kind: model.KindMediaFileArtwork, ID: "mf-1"}, time.Time{}, false, false) + u.Enqueue(model.ArtworkID{Kind: model.KindRadioArtwork, ID: "ra-1"}, time.Time{}, true, false) Expect(u.buffer).To(BeEmpty()) }) }) @@ -116,6 +117,21 @@ var _ = Describe("blurHashUpdater", func() { Expect(stored.BlurHash).To(BeEmpty()) }) + It("clears a fresh-looking stored hash when the source is gone", func() { + al := model.Album{ID: "al-1", UpdatedAt: version, BlurHash: "LEHV6nWB2yk8", BlurHashUpdatedAt: &version} + repo := tests.CreateMockAlbumRepo() + repo.SetData(model.Albums{al}) + ds.MockedAlbum = repo + ds.MockedFolder = failingFolderRepo{} + + // No row/mtime signal moved (eviction/restart window), but the serve 404ed: sourceGone + // must bypass the freshness skip so the stale hash is cleared. + u.process(GinkgoT().Context(), al.CoverArtID(), enqueueRequest{imageUpdatedAt: version, sourceGone: true}) + stored, err := ds.Album(GinkgoT().Context()).Get("al-1") + Expect(err).ToNot(HaveOccurred()) + Expect(stored.BlurHash).To(BeEmpty()) + }) + It("does nothing when the entity is gone", func() { ds.MockedAlbum = tests.CreateMockAlbumRepo() Expect(func() { diff --git a/core/artwork/e2e/blurhash_test.go b/core/artwork/e2e/blurhash_test.go index 90b759e6b..2bd9aaf18 100644 --- a/core/artwork/e2e/blurhash_test.go +++ b/core/artwork/e2e/blurhash_test.go @@ -49,10 +49,11 @@ var _ = Describe("BlurHash", func() { g.Expect(updated.BlurHash).ToNot(BeEmpty()) }, "10s", "100ms").Should(Succeed()) + // No rescan: the folder row still lists the cover, but the file is gone — the serve's + // ErrUnavailable alone must trigger the clear. setLayout(fstest.MapFS{ "Artist/Album/01 - Song.mp3": trackFile(1, "Song"), }) - scan() _, err := readArtworkOrErr(al.CoverArtID()) Expect(err).To(HaveOccurred())