From 319662174e6f4e86cfd2baafbf4c02c61530e9be Mon Sep 17 00:00:00 2001 From: Deluan Date: Fri, 17 Jul 2026 21:08:16 -0400 Subject: [PATCH] fix(album): keep cover uploads out of Recently Added ordering Cover upload/delete bumped album.updated_at to bust the artwork cache, but recentlyAddedSort() and Subsonic albumCreatedAt() sort on updated_at when RecentlyAddedByModTime is enabled, so a cover edit reordered Recently Added and shifted the reported creation date. Add a dedicated cover_art_updated_at column that UpdateImage bumps instead of updated_at; the album artwork id, reader cache key, and UI cover URL fold it into their cache-busting, so covers refresh everywhere while updated_at (and Recently Added) stays untouched. Also preserve uploaded_image across the phase-2 moved-track album-id change, which previously copied only created_at. --- core/artwork/reader_album.go | 3 + ...0260717224137_add_album_uploaded_image.sql | 4 ++ model/album.go | 55 ++++++++++--------- model/artwork_id.go | 8 ++- persistence/album_repository.go | 6 +- persistence/album_repository_test.go | 13 +++++ scanner/phase_1_folders.go | 2 +- scanner/phase_2_missing_tracks.go | 8 +-- scanner/phase_2_missing_tracks_test.go | 25 +++++++++ tests/mock_album_repo.go | 6 ++ ui/src/subsonic/index.js | 8 ++- ui/src/subsonic/index.test.js | 15 +++++ 12 files changed, 116 insertions(+), 37 deletions(-) diff --git a/core/artwork/reader_album.go b/core/artwork/reader_album.go index 4ec01e22d..eae2f46a4 100644 --- a/core/artwork/reader_album.go +++ b/core/artwork/reader_album.go @@ -58,6 +58,9 @@ func newAlbumArtworkReader(ctx context.Context, artwork *artwork, artID model.Ar if imagesUpdateAt != nil { a.cacheKey.lastUpdate = utils.TimeNewest(a.cacheKey.lastUpdate, *imagesUpdateAt) } + if al.CoverArtUpdatedAt != nil { + a.cacheKey.lastUpdate = utils.TimeNewest(a.cacheKey.lastUpdate, *al.CoverArtUpdatedAt) + } return a, nil } diff --git a/db/migrations/20260717224137_add_album_uploaded_image.sql b/db/migrations/20260717224137_add_album_uploaded_image.sql index 9dcda817a..760ee3e0e 100644 --- a/db/migrations/20260717224137_add_album_uploaded_image.sql +++ b/db/migrations/20260717224137_add_album_uploaded_image.sql @@ -1,5 +1,9 @@ -- +goose Up ALTER TABLE album ADD COLUMN uploaded_image varchar NOT NULL DEFAULT ''; +-- cover_art_updated_at tracks manual cover edits separately from updated_at, so +-- busting the artwork cache never disturbs updated_at-based Recently Added ordering. +ALTER TABLE album ADD COLUMN cover_art_updated_at datetime; -- +goose Down ALTER TABLE album DROP COLUMN uploaded_image; +ALTER TABLE album DROP COLUMN cover_art_updated_at; diff --git a/model/album.go b/model/album.go index ef48ac7be..888338df3 100644 --- a/model/album.go +++ b/model/album.go @@ -24,33 +24,34 @@ type Album struct { EmbedArtPath string `structs:"embed_art_path" json:"-"` AlbumArtistID string `structs:"album_artist_id" json:"albumArtistId"` // Deprecated, use Participants // AlbumArtist is the display name used for the album artist. - AlbumArtist string `structs:"album_artist" json:"albumArtist"` - MaxYear int `structs:"max_year" json:"maxYear"` - MinYear int `structs:"min_year" json:"minYear"` - Date string `structs:"date" json:"date,omitempty"` - MaxOriginalYear int `structs:"max_original_year" json:"maxOriginalYear"` - MinOriginalYear int `structs:"min_original_year" json:"minOriginalYear"` - OriginalDate string `structs:"original_date" json:"originalDate,omitempty"` - ReleaseDate string `structs:"release_date" json:"releaseDate,omitempty"` - Compilation bool `structs:"compilation" json:"compilation"` - Comment string `structs:"comment" json:"comment,omitempty"` - SongCount int `structs:"song_count" json:"songCount"` - Duration float32 `structs:"duration" json:"duration"` - Size int64 `structs:"size" json:"size"` - Discs Discs `structs:"discs" json:"discs,omitempty"` - SortAlbumName string `structs:"sort_album_name" json:"sortAlbumName,omitempty"` - SortAlbumArtistName string `structs:"sort_album_artist_name" json:"sortAlbumArtistName,omitempty"` - OrderAlbumName string `structs:"order_album_name" json:"orderAlbumName"` - OrderAlbumArtistName string `structs:"order_album_artist_name" json:"orderAlbumArtistName"` - CatalogNum string `structs:"catalog_num" json:"catalogNum,omitempty"` - MbzAlbumID string `structs:"mbz_album_id" json:"mbzAlbumId,omitempty"` - MbzAlbumArtistID string `structs:"mbz_album_artist_id" json:"mbzAlbumArtistId,omitempty"` - MbzAlbumType string `structs:"mbz_album_type" json:"mbzAlbumType,omitempty"` - MbzAlbumComment string `structs:"mbz_album_comment" json:"mbzAlbumComment,omitempty"` - MbzReleaseGroupID string `structs:"mbz_release_group_id" json:"mbzReleaseGroupId,omitempty"` - FolderIDs []string `structs:"folder_ids" json:"-" hash:"set"` // All folders that contain media_files for this album - ExplicitStatus string `structs:"explicit_status" json:"explicitStatus"` - UploadedImage string `structs:"-" json:"uploadedImage,omitempty" hash:"ignore"` + AlbumArtist string `structs:"album_artist" json:"albumArtist"` + MaxYear int `structs:"max_year" json:"maxYear"` + MinYear int `structs:"min_year" json:"minYear"` + Date string `structs:"date" json:"date,omitempty"` + MaxOriginalYear int `structs:"max_original_year" json:"maxOriginalYear"` + MinOriginalYear int `structs:"min_original_year" json:"minOriginalYear"` + OriginalDate string `structs:"original_date" json:"originalDate,omitempty"` + ReleaseDate string `structs:"release_date" json:"releaseDate,omitempty"` + Compilation bool `structs:"compilation" json:"compilation"` + Comment string `structs:"comment" json:"comment,omitempty"` + SongCount int `structs:"song_count" json:"songCount"` + Duration float32 `structs:"duration" json:"duration"` + Size int64 `structs:"size" json:"size"` + Discs Discs `structs:"discs" json:"discs,omitempty"` + SortAlbumName string `structs:"sort_album_name" json:"sortAlbumName,omitempty"` + SortAlbumArtistName string `structs:"sort_album_artist_name" json:"sortAlbumArtistName,omitempty"` + OrderAlbumName string `structs:"order_album_name" json:"orderAlbumName"` + OrderAlbumArtistName string `structs:"order_album_artist_name" json:"orderAlbumArtistName"` + CatalogNum string `structs:"catalog_num" json:"catalogNum,omitempty"` + MbzAlbumID string `structs:"mbz_album_id" json:"mbzAlbumId,omitempty"` + MbzAlbumArtistID string `structs:"mbz_album_artist_id" json:"mbzAlbumArtistId,omitempty"` + MbzAlbumType string `structs:"mbz_album_type" json:"mbzAlbumType,omitempty"` + MbzAlbumComment string `structs:"mbz_album_comment" json:"mbzAlbumComment,omitempty"` + MbzReleaseGroupID string `structs:"mbz_release_group_id" json:"mbzReleaseGroupId,omitempty"` + FolderIDs []string `structs:"folder_ids" json:"-" hash:"set"` // All folders that contain media_files for this album + ExplicitStatus string `structs:"explicit_status" json:"explicitStatus"` + UploadedImage string `structs:"-" json:"uploadedImage,omitempty" hash:"ignore"` + CoverArtUpdatedAt *time.Time `structs:"-" json:"coverArtUpdatedAt,omitempty" hash:"ignore"` // External metadata fields Description string `structs:"description" json:"description,omitempty" hash:"ignore"` diff --git a/model/artwork_id.go b/model/artwork_id.go index 1bd146c1f..69b9baf1d 100644 --- a/model/artwork_id.go +++ b/model/artwork_id.go @@ -112,10 +112,16 @@ 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. + lastUpdate := al.UpdatedAt + if al.CoverArtUpdatedAt != nil && al.CoverArtUpdatedAt.After(lastUpdate) { + lastUpdate = *al.CoverArtUpdatedAt + } return ArtworkID{ Kind: KindAlbumArtwork, ID: al.ID, - LastUpdate: al.UpdatedAt, + LastUpdate: lastUpdate, } } diff --git a/persistence/album_repository.go b/persistence/album_repository.go index e8167cde4..6192b3d18 100644 --- a/persistence/album_repository.go +++ b/persistence/album_repository.go @@ -219,12 +219,12 @@ func (r *albumRepository) UpdateExternalInfo(al *model.Album) error { return err } -// UpdateImage is the sole writer of uploaded_image: it uses raw SQL because Put's -// structs.Map marshaling drops the structs:"-" tagged UploadedImage field. +// UpdateImage is the sole writer of uploaded_image (raw SQL: Put's structs.Map drops the +// structs:"-" field). Bumps cover_art_updated_at, not updated_at, to leave Recently Added put. func (r *albumRepository) UpdateImage(id, filename string) error { c, err := r.executeSQL(Update(r.tableName). Set("uploaded_image", filename). - Set("updated_at", time.Now()). + Set("cover_art_updated_at", time.Now()). Where(Eq{"id": id})) if err != nil { return err diff --git a/persistence/album_repository_test.go b/persistence/album_repository_test.go index 0f8e430df..7978557a8 100644 --- a/persistence/album_repository_test.go +++ b/persistence/album_repository_test.go @@ -70,6 +70,19 @@ var _ = Describe("AlbumRepository", func() { It("returns ErrNotFound for a missing album", func() { Expect(albumRepo.UpdateImage("does-not-exist", "x.jpg")).To(MatchError(model.ErrNotFound)) }) + It("bumps cover_art_updated_at without touching updated_at", func() { + before, err := albumRepo.Get("img-1") + Expect(err).ToNot(HaveOccurred()) + Expect(before.CoverArtUpdatedAt).To(BeNil()) + + Expect(albumRepo.UpdateImage("img-1", "img-1_cover.jpg")).To(Succeed()) + + after, err := albumRepo.Get("img-1") + Expect(err).ToNot(HaveOccurred()) + Expect(after.CoverArtUpdatedAt).ToNot(BeNil()) + Expect(*after.CoverArtUpdatedAt).To(BeTemporally("~", time.Now(), time.Minute)) + Expect(after.UpdatedAt).To(Equal(before.UpdatedAt)) + }) }) Describe("CopyAttributes", func() { diff --git a/scanner/phase_1_folders.go b/scanner/phase_1_folders.go index a80442239..7ac45d3fc 100644 --- a/scanner/phase_1_folders.go +++ b/scanner/phase_1_folders.go @@ -450,7 +450,7 @@ func (p *phaseFolders) persistAlbum(repo model.AlbumRepository, a *model.Album, } // Keep created_at and any uploaded cover from the previous instance of the album - if err := repo.CopyAttributes(prevID, a.ID, "created_at", "uploaded_image"); err != nil { + if err := repo.CopyAttributes(prevID, a.ID, "created_at", "uploaded_image", "cover_art_updated_at"); err != nil { // Silently ignore when the previous album is not found if !errors.Is(err, model.ErrNotFound) { log.Warn(p.ctx, "Scanner: Could not copy fields", "from", prevID, "to", a.ID, "album", a.Name, err) diff --git a/scanner/phase_2_missing_tracks.go b/scanner/phase_2_missing_tracks.go index 8c258b833..b6a7abd82 100644 --- a/scanner/phase_2_missing_tracks.go +++ b/scanner/phase_2_missing_tracks.go @@ -313,11 +313,11 @@ func (p *phaseMissingTracks) moveMatched(target, missing model.MediaFile) error log.Warn(p.ctx, "Scanner: Could not reassign album annotations", "from", oldAlbumID, "to", newAlbumID, err) } - // Keep created_at field from previous instance of the album, so moved albums - // don't appear in "Recently Added" - if err := tx.Album(p.ctx).CopyAttributes(oldAlbumID, newAlbumID, "created_at"); err != nil { + // Keep created_at (so moved albums don't resurface in "Recently Added") + // and any manually uploaded cover from the previous album instance. + if err := tx.Album(p.ctx).CopyAttributes(oldAlbumID, newAlbumID, "created_at", "uploaded_image", "cover_art_updated_at"); err != nil { if !errors.Is(err, model.ErrNotFound) { - log.Warn(p.ctx, "Scanner: Could not copy album created_at", "from", oldAlbumID, "to", newAlbumID, err) + log.Warn(p.ctx, "Scanner: Could not copy album attributes", "from", oldAlbumID, "to", newAlbumID, err) } } diff --git a/scanner/phase_2_missing_tracks_test.go b/scanner/phase_2_missing_tracks_test.go index d54ceee40..edc979eb4 100644 --- a/scanner/phase_2_missing_tracks_test.go +++ b/scanner/phase_2_missing_tracks_test.go @@ -841,6 +841,31 @@ var _ = Describe("phaseMissingTracks", func() { Expect(newAlbum.CreatedAt).To(Equal(originalTime)) }) + It("should preserve an uploaded cover during moves with album change", func() { + missingTrack := model.MediaFile{ + ID: "missing-img", PID: "C", Path: "lib1/song.mp3", + AlbumID: "old-album", LibraryID: 1, + } + matchedTrack := model.MediaFile{ + ID: "matched-img", PID: "C", Path: "lib2/song.mp3", + AlbumID: "new-album", LibraryID: 2, + } + + albumRepo.SetData(model.Albums{ + {ID: "old-album", LibraryID: 1, UploadedImage: "old-album_cover.jpg"}, + {ID: "new-album", LibraryID: 2}, + }) + + _ = ds.MediaFile(ctx).Put(&missingTrack) + _ = ds.MediaFile(ctx).Put(&matchedTrack) + + Expect(phase.moveMatched(matchedTrack, missingTrack)).To(Succeed()) + + newAlbum, err := albumRepo.Get("new-album") + Expect(err).ToNot(HaveOccurred()) + Expect(newAlbum.UploadedImage).To(Equal("old-album_cover.jpg")) + }) + It("should not copy album created_at when album ID does not change", func() { originalTime := time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC) missingTrack := model.MediaFile{ diff --git a/tests/mock_album_repo.go b/tests/mock_album_repo.go index ccd1eba15..1ac03394f 100644 --- a/tests/mock_album_repo.go +++ b/tests/mock_album_repo.go @@ -140,6 +140,8 @@ func (m *MockAlbumRepo) UpdateImage(id, filename string) error { } if al, ok := m.Data[id]; ok { al.UploadedImage = filename + now := time.Now() + al.CoverArtUpdatedAt = &now return nil } return model.ErrNotFound @@ -187,6 +189,10 @@ func (m *MockAlbumRepo) CopyAttributes(fromID, toID string, columns ...string) e switch col { case "created_at": to.CreatedAt = from.CreatedAt + case "uploaded_image": + to.UploadedImage = from.UploadedImage + case "cover_art_updated_at": + to.CoverArtUpdatedAt = from.CoverArtUpdatedAt } } if m.CopyAttributesCalls == nil { diff --git a/ui/src/subsonic/index.js b/ui/src/subsonic/index.js index 7d93972e0..d0271cac0 100644 --- a/ui/src/subsonic/index.js +++ b/ui/src/subsonic/index.js @@ -81,8 +81,14 @@ const getAvatarUrl = (username, size) => ) const getCoverArtUrl = (record, size, square) => { + // Bust the cache on the newest of updatedAt / coverArtUpdatedAt (album cover + // uploads bump the latter) — ISO timestamps sort chronologically. + const cacheKey = [record.updatedAt, record.coverArtUpdatedAt] + .filter(Boolean) + .sort() + .pop() const options = { - ...(record.updatedAt && { _: record.updatedAt }), + ...(cacheKey && { _: cacheKey }), ...(size && { size }), ...(square && { square }), } diff --git a/ui/src/subsonic/index.test.js b/ui/src/subsonic/index.test.js index ad4764c24..d1a613081 100644 --- a/ui/src/subsonic/index.test.js +++ b/ui/src/subsonic/index.test.js @@ -79,6 +79,21 @@ describe('getCoverArtUrl', () => { expect(url).toContain('square=true') }) + it('should bust the cache on coverArtUpdatedAt when it is newer', () => { + const albumRecord = { + id: 'album-123', + albumArtist: 'Test Artist', + updatedAt: '2023-01-01T00:00:00Z', + coverArtUpdatedAt: '2024-06-01T00:00:00Z', + } + + 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') + }) + it('should return media file cover art URL for records with album', () => { const songRecord = { id: 'song-123',