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', + ) }) })