diff --git a/core/artwork/reader_disc_test.go b/core/artwork/reader_disc_test.go index 8264ee27b..08f8c3ee3 100644 --- a/core/artwork/reader_disc_test.go +++ b/core/artwork/reader_disc_test.go @@ -4,6 +4,7 @@ import ( "context" "os" "path/filepath" + "time" "github.com/navidrome/navidrome/model" . "github.com/onsi/ginkgo/v2" @@ -11,6 +12,17 @@ import ( ) var _ = Describe("Disc Artwork Reader", func() { + Describe("Key", func() { + It("changes when the album's cover stamp changes", func() { + r := &discArtworkReader{} + r.album = model.Album{ID: "al-1"} + before := r.Key() + stamp := time.Now() + r.album.CoverArtUpdatedAt = &stamp + Expect(r.Key()).ToNot(Equal(before)) + }) + }) + Describe("extractDiscNumber", func() { DescribeTable("extracts disc number from filename based on glob pattern", func(pattern, filename string, expectedNum int, expectedOk bool) { diff --git a/core/artwork/reader_mediafile_test.go b/core/artwork/reader_mediafile_test.go new file mode 100644 index 000000000..9976c7a13 --- /dev/null +++ b/core/artwork/reader_mediafile_test.go @@ -0,0 +1,22 @@ +package artwork + +import ( + "time" + + "github.com/navidrome/navidrome/model" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("MediaFile Artwork Reader", func() { + Describe("Key", func() { + It("changes when the album's cover stamp changes", func() { + r := &mediafileArtworkReader{} + r.album = model.Album{ID: "al-1"} + before := r.Key() + stamp := time.Now() + r.album.CoverArtUpdatedAt = &stamp + Expect(r.Key()).ToNot(Equal(before)) + }) + }) +}) diff --git a/persistence/album_repository.go b/persistence/album_repository.go index 6323669ce..1eb1b4422 100644 --- a/persistence/album_repository.go +++ b/persistence/album_repository.go @@ -285,8 +285,14 @@ func (r *albumRepository) GetCursor(options ...model.QueryOptions) (model.AlbumC } func (r *albumRepository) CopyAttributes(fromID, toID string, columns ...string) error { + // The cover-stamp guard below needs the source's uploaded_image even when the + // caller didn't request it + selectCols := columns + if slices.Contains(columns, "cover_art_updated_at") && !slices.Contains(columns, "uploaded_image") { + selectCols = append(slices.Clone(columns), "uploaded_image") + } var from dbx.NullStringMap - err := r.queryOne(Select(columns...).From(r.tableName).Where(Eq{"id": fromID}), &from) + err := r.queryOne(Select(selectCols...).From(r.tableName).Where(Eq{"id": fromID}), &from) if err != nil { return fmt.Errorf("getting album to copy fields from: %w", err) } diff --git a/persistence/album_repository_test.go b/persistence/album_repository_test.go index c18ca2e08..f30ebea31 100644 --- a/persistence/album_repository_test.go +++ b/persistence/album_repository_test.go @@ -174,6 +174,23 @@ var _ = Describe("AlbumRepository", func() { Expect(got.UploadedImage).To(Equal("copy-dst_cover.jpg")) Expect(got.CoverArtUpdatedAt).ToNot(BeNil()) }) + It("applies the cover-stamp guard even when uploaded_image is not requested", func() { + Expect(albumRepo.UpdateImage("copy-src", "copy-src_cover.jpg")).To(Succeed()) + Expect(albumRepo.CopyAttributes("copy-src", "copy-dst", "cover_art_updated_at")).To(Succeed()) + got, err := albumRepo.Get("copy-dst") + Expect(err).ToNot(HaveOccurred()) + Expect(got.CoverArtUpdatedAt).ToNot(BeNil(), "stamp must copy when the source has a cover") + + Expect(albumRepo.UpdateImage("copy-src", "")).To(Succeed()) + Expect(albumRepo.UpdateImage("copy-zero", "z.jpg")).To(Succeed()) + Expect(albumRepo.UpdateImage("copy-zero", "")).To(Succeed()) + before, err := albumRepo.Get("copy-dst") + Expect(err).ToNot(HaveOccurred()) + Expect(albumRepo.CopyAttributes("copy-zero", "copy-dst", "cover_art_updated_at")).To(Succeed()) + got, err = albumRepo.Get("copy-dst") + Expect(err).ToNot(HaveOccurred()) + Expect(got.CoverArtUpdatedAt.Equal(*before.CoverArtUpdatedAt)).To(BeTrue(), "coverless source must not contribute a stamp") + }) It("does not copy a stale cover stamp left behind by a cover removal", func() { // A removal clears uploaded_image but keeps cover_art_updated_at set Expect(albumRepo.UpdateImage("copy-src", "copy-src_cover.jpg")).To(Succeed()) diff --git a/persistence/playlist_track_repository_test.go b/persistence/playlist_track_repository_test.go index cd7865e1d..8e0615521 100644 --- a/persistence/playlist_track_repository_test.go +++ b/persistence/playlist_track_repository_test.go @@ -59,6 +59,11 @@ var _ = Describe("PlaylistTrackRepository", func() { trk, ok := got.(*model.PlaylistTrack) Expect(ok).To(BeTrue()) Expect(trk.CoverArtUpdatedAt).ToNot(BeNil()) + + // The list path (tracksQuery/loadTracks) must expose it too + all, err := repo.GetAll(model.QueryOptions{Sort: "id"}) + Expect(err).ToNot(HaveOccurred()) + Expect(all[0].CoverArtUpdatedAt).ToNot(BeNil()) }) }) diff --git a/tests/mock_album_repo.go b/tests/mock_album_repo.go index f54805ad3..2bb1ab3f0 100644 --- a/tests/mock_album_repo.go +++ b/tests/mock_album_repo.go @@ -208,7 +208,8 @@ func (m *MockAlbumRepo) CopyAttributes(fromID, toID string, columns ...string) e to.UploadedImage = from.UploadedImage } case "cover_art_updated_at": - if from.CoverArtUpdatedAt != nil { + // Mirrors the real repo: a coverless source never contributes a stale stamp + if from.CoverArtUpdatedAt != nil && from.UploadedImage != "" { to.CoverArtUpdatedAt = from.CoverArtUpdatedAt } }