From f2321a91b523ba404257198acf5b057aa2f9efe8 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sat, 25 Jul 2026 14:33:47 -0400 Subject: [PATCH] fix(artwork): key disc art on folder image changes, not just the album MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- core/artwork/disc.go | 35 +++++++++++++++++++++++++++-------- core/artwork/serving.go | 6 +++--- core/artwork/serving_test.go | 23 +++++++++++++++++++++++ 3 files changed, 53 insertions(+), 11 deletions(-) diff --git a/core/artwork/disc.go b/core/artwork/disc.go index 8bc566af2..4344bf3e6 100644 --- a/core/artwork/disc.go +++ b/core/artwork/disc.go @@ -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 } diff --git a/core/artwork/serving.go b/core/artwork/serving.go index c0e486a00..9d30fd47f 100644 --- a/core/artwork/serving.go +++ b/core/artwork/serving.go @@ -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 diff --git a/core/artwork/serving_test.go b/core/artwork/serving_test.go index bb5a58f42..9cef4d106 100644 --- a/core/artwork/serving_test.go +++ b/core/artwork/serving_test.go @@ -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"}})