mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-09 02:47:29 +02:00
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.
This commit is contained in:
parent
27a0841065
commit
319662174e
12 changed files with 116 additions and 37 deletions
|
|
@ -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
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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"`
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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() {
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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{
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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 }),
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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',
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue