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"}})