From 07af29fbb4e8e79e5fbaa2eac160f917ea2345d3 Mon Sep 17 00:00:00 2001 From: Deluan Date: Fri, 17 Jul 2026 22:14:13 -0400 Subject: [PATCH] fix(jellyfin): fold folder image mtimes into the album blurhash version An in-place cover-file swap followed by a quick scan updates only the folder's images_updated_at: no tracks are imported, so the album row never moves and ArtworkUpdatedAt() stayed at the old value. The Jellyfin DTO then kept emitting the previous stored blurhash, and clients that key their cover caches on it never refetched the image, so the served-bytes recompute could never run. The album select now surfaces the newest folder images_updated_at (bare-column correlated subquery over json_each(folder_ids), so the datetime decltype survives and scans as time.Time) and ArtworkUpdatedAt() folds it in. Benchmarked on a 96K track production copy: ~90us/page added, no query-plan change; the subquery is a PK point lookup per folder with at most two rows to order. Artist images and playlist sidecars have the same theoretical gap but their timestamps cannot be derived in SQL; they remain a documented follow-up. --- core/artwork/e2e/blurhash_test.go | 29 +++++++++++++++++++ core/artwork/e2e/suite_test.go | 14 +++++++++- model/album.go | 7 +++++ model/album_test.go | 8 ++++++ persistence/album_repository.go | 6 +++- persistence/album_repository_test.go | 42 ++++++++++++++++++++++++++++ 6 files changed, 104 insertions(+), 2 deletions(-) diff --git a/core/artwork/e2e/blurhash_test.go b/core/artwork/e2e/blurhash_test.go index f78b6fe6f..d11171aaf 100644 --- a/core/artwork/e2e/blurhash_test.go +++ b/core/artwork/e2e/blurhash_test.go @@ -135,6 +135,35 @@ var _ = Describe("BlurHash", func() { Expect(storedAlbum(al.ID).BlurHash).ToNot(Equal(firstHash)) }) + It("advances the album artwork version when only the cover file changes (quick scan)", func() { + setLayout(fstest.MapFS{ + "Artist/Album/01 - Song.mp3": trackFile(1, "Song"), + "Artist/Album/cover.png": realPNG("p1-orig"), + }) + scan() + al := firstAlbum() + readArtwork(al.CoverArtID()) + first := storedAlbum(al.ID) + Expect(first.BlurHash).ToNot(BeEmpty()) + Expect(first.BlurHashUpdatedAt.Before(first.ArtworkUpdatedAt())).To(BeFalse()) + + // Replace only the cover and quick-scan: the album row stays untouched while the folder's + // images_updated_at advances the artwork version, so hash-keyed clients refetch. + fakeFS.Add("Artist/Album/cover.png", realPNG("p1-swapped"), time.Now()) + quickScan() + + stale := storedAlbum(al.ID) + Expect(stale.UpdatedAt).To(Equal(first.UpdatedAt), "premise: image-only change must not touch the album row") + Expect(stale.BlurHash).To(Equal(first.BlurHash)) + Expect(stale.BlurHashUpdatedAt.Before(stale.ArtworkUpdatedAt())).To(BeTrue(), "stored hash must read as stale") + + // The refetch serves the new bytes; the tee rotates the hash and its version catches up. + readArtwork(al.CoverArtID()) + fresh := storedAlbum(al.ID) + Expect(fresh.BlurHash).ToNot(Equal(first.BlurHash)) + Expect(fresh.BlurHashUpdatedAt.Before(fresh.ArtworkUpdatedAt())).To(BeFalse()) + }) + It("clears a stored playlist hash when it falls back to the placeholder", func() { // A playlist with a sidecar cover gets a real hash; removing the sidecar makes the reader chain // fall through to fromAlbumPlaceholder(), whose bytes flow through the tee on Get and clear it. diff --git a/core/artwork/e2e/suite_test.go b/core/artwork/e2e/suite_test.go index 06cc05b6f..7e2740c6e 100644 --- a/core/artwork/e2e/suite_test.go +++ b/core/artwork/e2e/suite_test.go @@ -91,10 +91,22 @@ func setupHarness() { } func scan() { + GinkgoHelper() + doScan(true) +} + +// quickScan runs a non-full scan: only outdated folders are processed and unchanged audio files are +// not reimported, so an image-only change reaches the folder row without touching the album row. +func quickScan() { + GinkgoHelper() + doScan(false) +} + +func doScan(full bool) { GinkgoHelper() s := scanner.New(ctx, ds, artwork.NoopCacheWarmer(), events.NoopBroker(), playlists.NewPlaylists(ds, core.NewImageUploadService()), metrics.NewNoopInstance()) - _, err := s.ScanAll(ctx, true) + _, err := s.ScanAll(ctx, full) Expect(err).ToNot(HaveOccurred()) } diff --git a/model/album.go b/model/album.go index 130cd376f..16a093393 100644 --- a/model/album.go +++ b/model/album.go @@ -72,6 +72,10 @@ type Album struct { // full-row writes (structs:"-"): only UpdateBlurHash writes it, so scans can't erase it. BlurHash string `structs:"-" json:"blurHash,omitempty" hash:"ignore"` BlurHashUpdatedAt *time.Time `structs:"-" json:"-" hash:"ignore"` + + // FolderImagesUpdatedAt is the newest images_updated_at among the album's folders (selected, not + // persisted): an in-place cover-file swap moves it even though the album row stays untouched. + FolderImagesUpdatedAt *time.Time `structs:"-" json:"-" hash:"ignore"` } func (a Album) CoverArtID() ArtworkID { @@ -86,6 +90,9 @@ func (a Album) ArtworkUpdatedAt() time.Time { if a.ImportedAt.After(t) { t = a.ImportedAt } + if a.FolderImagesUpdatedAt != nil && a.FolderImagesUpdatedAt.After(t) { + t = *a.FolderImagesUpdatedAt + } return t } diff --git a/model/album_test.go b/model/album_test.go index 9dcf2353e..d9f7ff624 100644 --- a/model/album_test.go +++ b/model/album_test.go @@ -69,4 +69,12 @@ var _ = Describe("Album.ArtworkUpdatedAt", func() { al := Album{UpdatedAt: base, ImportedAt: later, ExternalInfoUpdatedAt: &latest} Expect(al.ArtworkUpdatedAt()).To(Equal(later)) }) + It("returns FolderImagesUpdatedAt when it is the newest (in-place cover swap)", func() { + al := Album{UpdatedAt: base, ImportedAt: later, FolderImagesUpdatedAt: &latest} + Expect(al.ArtworkUpdatedAt()).To(Equal(latest)) + }) + It("ignores an older FolderImagesUpdatedAt", func() { + al := Album{UpdatedAt: later, ImportedAt: base, FolderImagesUpdatedAt: &base} + Expect(al.ArtworkUpdatedAt()).To(Equal(later)) + }) }) diff --git a/persistence/album_repository.go b/persistence/album_repository.go index cd5e3f1c0..144e860de 100644 --- a/persistence/album_repository.go +++ b/persistence/album_repository.go @@ -224,7 +224,11 @@ func (r *albumRepository) UpdateExternalInfo(al *model.Album) error { } func (r *albumRepository) selectAlbum(options ...model.QueryOptions) SelectBuilder { - sql := r.newSelect(options...).Columns("album.*", "library.path as library_path", "library.name as library_name"). + sql := r.newSelect(options...).Columns("album.*", "library.path as library_path", "library.name as library_name", + // Folds folder image mtimes into the artwork version: an in-place cover swap moves them without + // touching the album row. Bare column (not max()) keeps the decltype so the driver scans time.Time. + "(select f.images_updated_at from folder f, json_each(album.folder_ids) je where f.id = je.value"+ + " order by f.images_updated_at desc limit 1) as folder_images_updated_at"). LeftJoin("library on album.library_id = library.id") sql = r.withAnnotation(sql, "album.id") return r.applyLibraryFilter(sql) diff --git a/persistence/album_repository_test.go b/persistence/album_repository_test.go index c8a66f8b0..7effeca0b 100644 --- a/persistence/album_repository_test.go +++ b/persistence/album_repository_test.go @@ -900,6 +900,48 @@ func _p(id, name string, sortName ...string) model.Participant { return p } +var _ = Describe("AlbumRepository folder images version", func() { + var repo model.AlbumRepository + + BeforeEach(func() { + ctx := request.WithUser(GinkgoT().Context(), model.User{ID: "userid", UserName: "johndoe"}) + repo = NewAlbumRepository(ctx, GetDBXBuilder()) + var origFolderIDs string + Expect(GetDBXBuilder().NewQuery("select folder_ids from album where id = '103'"). + Row(&origFolderIDs)).To(Succeed()) + DeferCleanup(func() { + _, err := GetDBXBuilder().NewQuery("delete from folder where id = 'fold-blur-1'").Execute() + Expect(err).ToNot(HaveOccurred()) + _, err = GetDBXBuilder().NewQuery("update album set folder_ids = {:f} where id = '103'"). + Bind(map[string]any{"f": origFolderIDs}).Execute() + Expect(err).ToNot(HaveOccurred()) + }) + }) + + It("surfaces the newest folder images_updated_at on the selected album", func() { + // Newer than any fixture row timestamp, so it must win as the artwork version. + imagesAt := time.Date(2030, 6, 1, 12, 0, 0, 0, time.UTC) + _, err := GetDBXBuilder().NewQuery( + "insert into folder (id, library_id, path, name, images_updated_at) values ('fold-blur-1', 1, '.', 'Radioactivity', {:t})"). + Bind(map[string]any{"t": imagesAt}).Execute() + Expect(err).ToNot(HaveOccurred()) + _, err = GetDBXBuilder().NewQuery(`update album set folder_ids = '["fold-blur-1"]' where id = '103'`).Execute() + Expect(err).ToNot(HaveOccurred()) + + al, err := repo.Get("103") + Expect(err).ToNot(HaveOccurred()) + Expect(al.FolderImagesUpdatedAt).ToNot(BeNil()) + Expect(al.FolderImagesUpdatedAt.Equal(imagesAt)).To(BeTrue()) + Expect(al.ArtworkUpdatedAt().Equal(imagesAt)).To(BeTrue(), "folder image changes must advance the artwork version") + }) + + It("leaves FolderImagesUpdatedAt nil when the album has no folders", func() { + al, err := repo.Get("101") + Expect(err).ToNot(HaveOccurred()) + Expect(al.FolderImagesUpdatedAt).To(BeNil()) + }) +}) + var _ = Describe("AlbumRepository.UpdateBlurHash", func() { var repo model.AlbumRepository