mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-31 07:30:32 +00:00
fix(artwork): clear a fresh-looking hash when the serve finds no source
In the window before a rescan records a deleted cover (or after cache eviction/restart), no row or mtime signal moves, so the freshness skip kept the stale hash. ErrUnavailable serves now carry a sourceGone signal that bypasses the skip only while a stored hash remains to clear — afterwards the negative cache applies, so artwork-less entities still don't re-resolve per serve. The e2e spec now covers the no-rescan window.
This commit is contained in:
parent
a7eec3afc3
commit
7a61776c1c
@ -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
|
||||
}
|
||||
|
||||
@ -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) {
|
||||
|
||||
@ -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() {
|
||||
|
||||
@ -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())
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user