fix(artwork): key disc art on folder image changes, not just the album

The identity cache key used album.UpdatedAt alone, which a replaced
disc image does not necessarily move — the sized response would then
serve the old image indefinitely. The legacy reader folded ImportedAt
and the folder's ImagesUpdatedAt into its key for exactly this reason,
and loadAlbumFoldersPaths already returns that timestamp; the disc
reader was discarding it.
This commit is contained in:
Deluan 2026-07-25 14:33:47 -04:00
parent 04d5a55657
commit f2321a91b5
3 changed files with 53 additions and 11 deletions

View File

@ -8,11 +8,13 @@ import (
"path/filepath"
"strconv"
"strings"
"time"
"github.com/Masterminds/squirrel"
"github.com/navidrome/navidrome/core/ffmpeg"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/utils"
)
// discArtworkReader resolves disc-level artwork from a library's folder images
@ -25,6 +27,16 @@ type discArtworkReader struct {
isMultiFolder bool
firstTrackRel string // library-relative; for fromTag / ffmpeg via lib.Abs
lib libraryView
// imagesUpdatedAt is the newest ImagesUpdatedAt across the album's and this disc's folders.
// An image can be replaced without the album row changing, so this is what makes a cache
// key notice it.
imagesUpdatedAt time.Time
}
// cacheTime is the disc image's validity stamp: any of these moving means the selection may
// have changed. Mirrors what the legacy reader folded into its cache key.
func (d *discArtworkReader) cacheTime() time.Time {
return utils.TimeNewest(d.album.UpdatedAt, d.album.ImportedAt, d.imagesUpdatedAt)
}
func newDiscArtworkReader(ctx context.Context, ds model.DataStore, artID model.ArtworkID) (*discArtworkReader, error) {
@ -38,11 +50,16 @@ func newDiscArtworkReader(ctx context.Context, ds model.DataStore, artID model.A
return nil, err
}
_, imgFiles, _, err := loadAlbumFoldersPaths(ctx, ds, *al)
_, imgFiles, albumImagesAt, err := loadAlbumFoldersPaths(ctx, ds, *al)
if err != nil {
return nil, err
}
var imagesUpdatedAt time.Time
if albumImagesAt != nil {
imagesUpdatedAt = *albumImagesAt
}
// Query mediafiles for this album + disc to find folder associations and first track
mfs, err := ds.MediaFile(ctx).GetAll(model.QueryOptions{
Sort: "track_number",
@ -84,17 +101,19 @@ func newDiscArtworkReader(ctx context.Context, ds model.DataStore, artID model.A
for _, f := range folders {
rel := strings.TrimPrefix(path.Join(f.Path, f.Name), "/")
discFoldersRel[rel] = true
imagesUpdatedAt = utils.TimeNewest(imagesUpdatedAt, f.ImagesUpdatedAt)
}
}
return &discArtworkReader{
album: *al,
discNumber: discNumber,
imgFiles: imgFiles,
discFoldersRel: discFoldersRel,
isMultiFolder: len(al.FolderIDs) > 1,
firstTrackRel: firstTrackRel,
lib: lib,
album: *al,
discNumber: discNumber,
imgFiles: imgFiles,
discFoldersRel: discFoldersRel,
isMultiFolder: len(al.FolderIDs) > 1,
firstTrackRel: firstTrackRel,
lib: lib,
imagesUpdatedAt: imagesUpdatedAt,
}, nil
}

View File

@ -321,12 +321,12 @@ func (s *service) serveDisc(ctx context.Context, artID model.ArtworkID, size int
// answer without touching the filesystem at all; the chain runs only on a miss. The key
// carries DiscArtPriority so changing it invalidates. resizedItem.hash is key material
// here, not a content hash, so the response carries no Image.Hash.
key := fmt.Sprintf("%s|%d|%s", artID.ID, dr.album.UpdatedAt.UnixNano(), conf.Server.DiscArtPriority)
key := fmt.Sprintf("%s|%d|%s", artID.ID, dr.cacheTime().UnixNano(), conf.Server.DiscArtPriority)
item := &resizedItem{
hash: key,
size: size,
square: square,
lastUpdate: dr.album.UpdatedAt,
lastUpdate: dr.cacheTime(),
ffmpeg: s.ffmpeg,
open: func() (io.ReadCloser, error) { rc, _, err := selectImage(); return rc, err },
}
@ -334,7 +334,7 @@ func (s *service) serveDisc(ctx context.Context, artID model.ArtworkID, size int
if err != nil {
return s.Get(ctx, albumArtID, size, square)
}
return &Image{ReadCloser: stream, ETag: representationTag(key, size, square), LastUpdated: dr.album.UpdatedAt}, nil
return &Image{ReadCloser: stream, ETag: representationTag(key, size, square), LastUpdated: dr.cacheTime()}, nil
}
// dangling enqueues a re-resolution at Scan priority and reports the artwork as

View File

@ -353,6 +353,29 @@ var _ = Describe("Service", func() {
Expect(readAll(second)).To(Equal(warmed), "a warm sized request must not touch the source")
})
// A disc image can be replaced without the album row changing, so the key folds in the
// folder's ImagesUpdatedAt; keying on album.UpdatedAt alone would serve the old image.
It("invalidates the cached image when the folder's images change", func() {
folderRepo.result = []model.Folder{{
Path: "tests/fixtures/artist/an-album", ImageFiles: []string{"cover.jpg"},
ImagesUpdatedAt: time.Now().Add(-time.Hour),
}}
albumRepo.SetData(model.Albums{{ID: "aldc4", Name: "Album", FolderIDs: []string{"f1"}}})
discID := model.NewArtworkID(model.KindDiscArtwork, model.DiscArtworkID("aldc4", 1), nil)
first, err := svc.Get(ctx, discID, 64, false)
Expect(err).ToNot(HaveOccurred())
firstKey := first.ETag
readAll(first)
// The image was replaced: same album row, newer folder images timestamp.
folderRepo.result[0].ImagesUpdatedAt = time.Now()
second, err := svc.Get(ctx, discID, 64, false)
Expect(err).ToNot(HaveOccurred())
readAll(second)
Expect(second.ETag).ToNot(Equal(firstKey), "a replaced image must not keep the old cache entry")
})
It("falls back to album art when no disc image matches", func() {
folderRepo.result = nil
albumRepo.SetData(model.Albums{{ID: "aldc2", Name: "Album"}})