From f498dfde7df671105587112235800382833a20a2 Mon Sep 17 00:00:00 2001 From: Deluan Date: Fri, 17 Jul 2026 23:03:20 -0400 Subject: [PATCH] fix(album): make cover cache keys immune to future file mtimes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All cover cache keys used max(updated_at, cover_art_updated_at), so an album whose updated_at is in the future (bad file mtimes) would mask a cover edit entirely — id unchanged, reader cache key unchanged, UI URL unchanged — and keep serving the old image. Treat the keys as discriminators instead of dates: the album artwork id adds the two timestamps, the reader cache keys (album/disc/mediafile) carry the cover stamp as a separate component, and the UI cache-buster joins both timestamps instead of picking the newest. --- core/artwork/image_cache.go | 8 ++++++++ core/artwork/reader_album.go | 5 ++++- core/artwork/reader_disc.go | 3 ++- core/artwork/reader_mediafile.go | 3 ++- model/album_test.go | 12 ++++++++++++ model/artwork_id.go | 8 ++++---- model/mediafile_test.go | 2 -- ui/src/subsonic/index.js | 9 ++++----- ui/src/subsonic/index.test.js | 14 ++++++++------ 9 files changed, 44 insertions(+), 20 deletions(-) diff --git a/core/artwork/image_cache.go b/core/artwork/image_cache.go index ac0f63794..6306aa098 100644 --- a/core/artwork/image_cache.go +++ b/core/artwork/image_cache.go @@ -27,6 +27,14 @@ func (k *cacheKey) Key() string { ) } +// coverStamp renders the album's manual-cover timestamp for cache keys (0 when unset). +func coverStamp(t *time.Time) int64 { + if t == nil { + return 0 + } + return t.UnixMilli() +} + type imageCache struct { cache.FileCache } diff --git a/core/artwork/reader_album.go b/core/artwork/reader_album.go index eae2f46a4..6048d2ca5 100644 --- a/core/artwork/reader_album.go +++ b/core/artwork/reader_album.go @@ -70,11 +70,14 @@ func (a *albumArtworkReader) Key() string { hashInput = conf.Server.Agents + hashInput } hash := md5.Sum([]byte(hashInput)) + // coverStamp is a separate component: folded into lastUpdate it could be masked + // by a newer updated_at (files with future mtimes). return fmt.Sprintf( - "%s.%x.%t", + "%s.%x.%t.%d", a.cacheKey.Key(), hash, conf.Server.EnableExternalServices, + coverStamp(a.album.CoverArtUpdatedAt), ) } func (a *albumArtworkReader) LastUpdated() time.Time { diff --git a/core/artwork/reader_disc.go b/core/artwork/reader_disc.go index 44d199023..1c9d7ff78 100644 --- a/core/artwork/reader_disc.go +++ b/core/artwork/reader_disc.go @@ -119,9 +119,10 @@ func newDiscArtworkReader(ctx context.Context, a *artwork, artID model.ArtworkID func (d *discArtworkReader) Key() string { hash := md5.Sum([]byte(conf.Server.DiscArtPriority)) return fmt.Sprintf( - "%s.%x", + "%s.%x.%d", d.cacheKey.Key(), hash, + coverStamp(d.album.CoverArtUpdatedAt), ) } diff --git a/core/artwork/reader_mediafile.go b/core/artwork/reader_mediafile.go index 96c3635fd..191266671 100644 --- a/core/artwork/reader_mediafile.go +++ b/core/artwork/reader_mediafile.go @@ -57,9 +57,10 @@ func newMediafileArtworkReader(ctx context.Context, artwork *artwork, artID mode func (a *mediafileArtworkReader) Key() string { return fmt.Sprintf( - "%s.%t", + "%s.%t.%d", a.cacheKey.Key(), conf.Server.EnableMediaFileCoverArt, + coverStamp(a.album.CoverArtUpdatedAt), ) } func (a *mediafileArtworkReader) LastUpdated() time.Time { diff --git a/model/album_test.go b/model/album_test.go index 3413d3859..06c5e64ee 100644 --- a/model/album_test.go +++ b/model/album_test.go @@ -3,6 +3,7 @@ package model_test import ( "encoding/json" "path/filepath" + "time" "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/conf/configtest" @@ -39,6 +40,17 @@ var _ = Describe("Album", func() { Expect(a.UploadedImagePath()).To(Equal(filepath.Join("/data", "artwork", "album", "al-1_cover.jpg"))) }) }) + + Describe("CoverArtID", func() { + It("changes when a cover is uploaded, even if updated_at is in the future", func() { + future := time.Now().Add(365 * 24 * time.Hour) + stamp := time.Now() + a := Album{ID: "al-1", UpdatedAt: future} + before := a.CoverArtID().String() + a.CoverArtUpdatedAt = &stamp + Expect(a.CoverArtID().String()).ToNot(Equal(before)) + }) + }) }) var _ = Describe("Albums", func() { diff --git a/model/artwork_id.go b/model/artwork_id.go index 69b9baf1d..d100414b1 100644 --- a/model/artwork_id.go +++ b/model/artwork_id.go @@ -112,11 +112,11 @@ func ParseDiscArtworkID(id string) (albumID string, discNumber int, err error) { } func artworkIDFromAlbum(al Album) ArtworkID { - // A manual cover edit bumps cover_art_updated_at (not updated_at), so fold it - // in here to refresh clients without disturbing Recently Added ordering. + // The suffix is a cache discriminator, not a date: cover edits are added (not max'd) + // so the id changes even when updated_at is newer, e.g. files with future mtimes. lastUpdate := al.UpdatedAt - if al.CoverArtUpdatedAt != nil && al.CoverArtUpdatedAt.After(lastUpdate) { - lastUpdate = *al.CoverArtUpdatedAt + if al.CoverArtUpdatedAt != nil { + lastUpdate = time.Unix(max(al.UpdatedAt.Unix(), 0)+al.CoverArtUpdatedAt.Unix(), 0) } return ArtworkID{ Kind: KindAlbumArtwork, diff --git a/model/mediafile_test.go b/model/mediafile_test.go index 2462adc84..98de68ec2 100644 --- a/model/mediafile_test.go +++ b/model/mediafile_test.go @@ -557,9 +557,7 @@ var _ = Describe("MediaFile", func() { Expect(album.CoverArtID().String()).To(HaveSuffix("_0")) disc.CoverArtUpdatedAt = &stamp album.CoverArtUpdatedAt = &stamp - Expect(disc.CoverArtID().LastUpdate).To(Equal(stamp)) Expect(disc.CoverArtID().String()).ToNot(HaveSuffix("_0")) - Expect(album.CoverArtID().LastUpdate).To(Equal(stamp)) Expect(album.CoverArtID().String()).ToNot(HaveSuffix("_0")) }) }) diff --git a/ui/src/subsonic/index.js b/ui/src/subsonic/index.js index 778453247..e4a965f3a 100644 --- a/ui/src/subsonic/index.js +++ b/ui/src/subsonic/index.js @@ -80,12 +80,11 @@ const getAvatarUrl = (username, size) => }), ) -// Newest of updatedAt / coverArtUpdatedAt (album cover uploads bump the latter). +// Cache-buster from both timestamps (cover uploads bump coverArtUpdatedAt); joined, +// not max'd, so the URL changes even when updatedAt is newer (future file mtimes). const artCacheKey = (record) => - [record.updatedAt, record.coverArtUpdatedAt] - .filter(Boolean) - .sort((a, b) => new Date(a) - new Date(b)) - .pop() + [record.updatedAt, record.coverArtUpdatedAt].filter(Boolean).join('|') || + undefined const getCoverArtUrl = (record, size, square) => { const cacheKey = artCacheKey(record) diff --git a/ui/src/subsonic/index.test.js b/ui/src/subsonic/index.test.js index 0f3963852..0634e70c2 100644 --- a/ui/src/subsonic/index.test.js +++ b/ui/src/subsonic/index.test.js @@ -79,7 +79,7 @@ describe('getCoverArtUrl', () => { expect(url).toContain('square=true') }) - it('should bust the cache on coverArtUpdatedAt when it is newer', () => { + it('should include coverArtUpdatedAt in the cache key', () => { const albumRecord = { id: 'album-123', albumArtist: 'Test Artist', @@ -90,8 +90,9 @@ describe('getCoverArtUrl', () => { const url = subsonic.getCoverArtUrl(albumRecord) expect(url).toContain('al-album-123') - expect(url).toContain('_=2024-06-01T00%3A00%3A00Z') - expect(url).not.toContain('_=2023-01-01') + expect(url).toContain( + '_=2023-01-01T00%3A00%3A00Z%7C2024-06-01T00%3A00%3A00Z', + ) }) it('should return media file cover art URL for records with album', () => { @@ -191,7 +192,7 @@ describe('getDiscCoverArtUrl', () => { expect(url).not.toContain('size=') }) - it('should bust the cache on coverArtUpdatedAt when it is newer', () => { + it('should include coverArtUpdatedAt in the cache key', () => { const url = subsonic.getDiscCoverArtUrl({ albumId: 'album-123', discNumber: 1, @@ -200,8 +201,9 @@ describe('getDiscCoverArtUrl', () => { }) expect(url).toContain('id=dc-album-123%3A1') - expect(url).toContain('_=2024-06-01T00%3A00%3A00Z') - expect(url).not.toContain('_=2023-01-01') + expect(url).toContain( + '_=2023-01-01T00%3A00%3A00Z%7C2024-06-01T00%3A00%3A00Z', + ) }) })