fix(artwork): stop advertising a hash for bytes that won't be served

Hydration stamped the album's hash and blurhash onto an embedded-eligible
track that had no state row yet. Serving takes provisionalEmbedded for
exactly that case and returns the track's own embedded image, so the id
carried a content-version belonging to a different picture: every such
request fell back to no-cache instead of immutable, and a client keying
its cover cache on the blurhash paired the album's with the track's art.

AlbumCoverArtID had the mirror-image problem, building the album id from
the track's own ItemImage. It happened to work only because hydration
overwrote ImageHash in precisely the fallback cases; a track with its own
resolved art would have stamped that hash onto the album's id.
This commit is contained in:
Deluan 2026-07-25 10:25:46 -04:00
parent c66ef971cf
commit d48c7b04da
4 changed files with 26 additions and 10 deletions

View File

@ -151,8 +151,10 @@ func (mf MediaFile) DiscCoverArtID() ArtworkID {
return mf.AlbumCoverArtID()
}
// AlbumCoverArtID uses AlbumImage, not the track's own ItemImage: an album id must carry the
// album's content hash even when the track resolved art of its own.
func (mf MediaFile) AlbumCoverArtID() ArtworkID {
return artworkIDFromAlbum(Album{ID: mf.AlbumID, ItemImage: mf.ItemImage})
return artworkIDFromAlbum(Album{ID: mf.AlbumID, ItemImage: mf.AlbumImage})
}
func (mf MediaFile) StructuredLyrics() (LyricList, error) {

View File

@ -102,12 +102,14 @@ func hydrateMediaFileArtwork(ctx context.Context, db dbx.Builder, mfs model.Medi
mf.BlurHash = ownInfo.BlurHash
continue
}
ownWontResolve := !eligible || (ownResolved && ownInfo.Absent())
// Fallback (see MediaFile.CoverArtID): inherit a found album hash for optimistic caching,
// but only for a single-disc track. A multi-disc track emits a dc- id served from
// disc-specific art of unknown identity, so stamping the album hash would advertise a
// wrong content-version; leave it bare (the served response still carries a correct ETag).
// but only when the album's bytes are what serving will actually return. A multi-disc track
// emits a dc- id served from disc art, and an eligible-but-unresolved track still extracts
// its own embedded image — stamping the album hash on either advertises a content-version
// (and a blurhash) belonging to a different image.
if album, ok := albumInfos[mf.AlbumID]; ok && !album.Absent() {
if mf.DiscNumber == 0 {
if mf.DiscNumber == 0 && ownWontResolve {
mf.ImageHash = album.Hash
mf.BlurHash = album.BlurHash
}
@ -121,7 +123,6 @@ func hydrateMediaFileArtwork(ctx context.Context, db dbx.Builder, mfs model.Medi
if mf.DiscNumber > 0 {
continue
}
ownWontResolve := !eligible || (ownResolved && ownInfo.Absent())
if album, ok := albumInfos[mf.AlbumID]; ok && album.Absent() && ownWontResolve {
mf.ImageAbsent = true
}

View File

@ -374,14 +374,18 @@ var _ = Describe("Artwork hydration", func() {
Expect(byID["2002"].ImageHash).To(BeEmpty())
})
It("uses the album hash for an eligible file whose own art is unresolved but album is found", func() {
// serveMediaFile extracts this track's own embedded art (provisionalEmbedded), so the
// album's hash would advertise a content-version for bytes nobody will serve.
It("leaves the hash bare for an eligible file whose own art is unresolved", func() {
setCover("1004", true)
DeferCleanup(func() { setCover("1004", false) })
putInfo("al", "103", "alh103found11111")
byID := getByID()
Expect(byID["1004"].ImageAbsent).To(BeFalse())
Expect(byID["1004"].ImageHash).To(Equal("alh103found11111"))
Expect(byID["1004"].ImageHash).To(BeEmpty())
Expect(byID["1004"].AlbumImage.ImageHash).To(Equal("alh103found11111"),
"the album's own hash still hydrates, for AlbumCoverArtID")
})
It("uses album info for an eligible file when EnableMediaFileCoverArt is off", func() {

View File

@ -349,8 +349,17 @@ var _ = Describe("helpers", func() {
})
Describe("childFromMediaFile", func() {
It("suffixes coverArt with the content hash when resolved", func() {
mf := model.MediaFile{ID: "mf-1", AlbumID: "al-1", ItemImage: model.ItemImage{ImageHash: hash}}
// The album id carries the album's hash, which hydration puts in AlbumImage; the
// track's own ItemImage describes its own art and must not be stamped onto an al- id.
It("suffixes coverArt with the album's content hash when resolved", func() {
mf := model.MediaFile{ID: "mf-1", AlbumID: "al-1", AlbumImage: model.ItemImage{ImageHash: hash}}
Expect(childFromMediaFile(ctx, mf).CoverArt).To(Equal("al-al-1_" + hash))
})
It("does not stamp a track's own art hash onto the album id", func() {
mf := model.MediaFile{ID: "mf-1", AlbumID: "al-1",
ItemImage: model.ItemImage{ImageHash: "0000ownarthash00"},
AlbumImage: model.ItemImage{ImageHash: hash}}
Expect(childFromMediaFile(ctx, mf).CoverArt).To(Equal("al-al-1_" + hash))
})
It("omits coverArt when known absent", func() {