From 46b02d23af1a79c134932e2a54cb8fd2eede2fb1 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sat, 10 Oct 2026 10:24:24 -0400 Subject: [PATCH] fix(playlists): generate smart playlist covers once they have tracks Smart playlists only get tracks when they are first evaluated, which happens when a client opens them. Their cover was resolved before that, while the playlist was still empty, so the generated grid had no albums to sample and settled as absent for good. Queue a cover rebuild when an evaluation takes the playlist from empty to populated. Rebuilding on every refresh would re-sample the random grid each time, and re-run the full album art lookup for playlists whose albums have no art. A migration re-queues covers of populated smart playlists that are absent or were never resolved. Fixes #6308 --- ...010142248_requeue_smart_playlist_covers.go | 31 ++++++++ .../requeue_smart_playlist_covers_test.go | 78 +++++++++++++++++++ persistence/smart_playlist_repository.go | 7 ++ persistence/smart_playlist_repository_test.go | 41 ++++++++++ 4 files changed, 157 insertions(+) create mode 100644 db/migrations/20261010142248_requeue_smart_playlist_covers.go create mode 100644 db/migrations/requeue_smart_playlist_covers_test.go diff --git a/db/migrations/20261010142248_requeue_smart_playlist_covers.go b/db/migrations/20261010142248_requeue_smart_playlist_covers.go new file mode 100644 index 000000000..5eecfd787 --- /dev/null +++ b/db/migrations/20261010142248_requeue_smart_playlist_covers.go @@ -0,0 +1,31 @@ +package migrations + +import ( + "context" + "database/sql" + "time" + + "github.com/pressly/goose/v3" +) + +func init() { + goose.AddMigrationContext(upRequeueSmartPlaylistCovers, downRequeueSmartPlaylistCovers) +} + +// Re-queue smart playlist covers that resolved while the playlist was still empty. Times are bound +// from Go: retry_at is compared as text against Go-formatted times, which CURRENT_TIMESTAMP doesn't match. +func upRequeueSmartPlaylistCovers(ctx context.Context, tx *sql.Tx) error { + now := time.Now() + _, err := tx.ExecContext(ctx, ` +INSERT INTO artwork_queue (item_kind, item_id, image_type, priority, attempts, retry_at, enqueued_at) +SELECT 'pl', p.id, 'primary', 50, 0, ?, ? +FROM playlist p +LEFT JOIN item_artwork ia ON ia.item_kind = 'pl' AND ia.item_id = p.id AND ia.image_type = 'primary' +WHERE p.rules IS NOT NULL AND p.rules != '' AND p.song_count > 0 AND COALESCE(ia.hash, '') = '' +ON CONFLICT (item_kind, item_id, image_type) DO NOTHING`, now, now) + return err +} + +func downRequeueSmartPlaylistCovers(context.Context, *sql.Tx) error { + return nil +} diff --git a/db/migrations/requeue_smart_playlist_covers_test.go b/db/migrations/requeue_smart_playlist_covers_test.go new file mode 100644 index 000000000..4296eefe9 --- /dev/null +++ b/db/migrations/requeue_smart_playlist_covers_test.go @@ -0,0 +1,78 @@ +package migrations + +import ( + "context" + "database/sql" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("upRequeueSmartPlaylistCovers", func() { + var db *sql.DB + var ctx context.Context + + BeforeEach(func() { + ctx = GinkgoT().Context() + var err error + db, err = sql.Open("sqlite3", "file::memory:") + Expect(err).ToNot(HaveOccurred()) + db.SetMaxOpenConns(1) // non-shared :memory: — every new conn is a fresh empty DB + DeferCleanup(func() { _ = db.Close() }) + + _, err = db.Exec(` + CREATE TABLE playlist (id text, rules text, song_count integer); + CREATE TABLE item_artwork (item_kind text, item_id text, image_type text, hash text NOT NULL DEFAULT '', + PRIMARY KEY (item_kind, item_id, image_type)); + CREATE TABLE artwork_queue (item_kind text NOT NULL, item_id text NOT NULL, image_type text NOT NULL DEFAULT 'primary', + priority integer NOT NULL DEFAULT 0, attempts integer NOT NULL DEFAULT 0, + retry_at timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP, enqueued_at timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP, + trace jsonb NOT NULL DEFAULT '[]', PRIMARY KEY (item_kind, item_id, image_type)); + + INSERT INTO playlist VALUES + ('absent', '{"all":[]}', 5), ('no-row', '{"all":[]}', 5), ('has-cover', '{"all":[]}', 5), + ('static-null', NULL, 5), ('static-empty', '', 5), ('empty-smart', '{"all":[]}', 0), + ('queued', '{"all":[]}', 5); + INSERT INTO item_artwork VALUES + ('pl', 'absent', 'primary', ''), ('pl', 'has-cover', 'primary', 'h1'), + ('pl', 'static-null', 'primary', ''), ('pl', 'static-empty', 'primary', ''), + ('pl', 'empty-smart', 'primary', ''), ('pl', 'queued', 'primary', ''), + ('al', 'no-row', 'primary', ''); + `) + Expect(err).ToNot(HaveOccurred()) + _, err = db.Exec(`INSERT INTO artwork_queue (item_kind, item_id, priority, retry_at, enqueued_at) + VALUES ('pl', 'queued', 100, ?, ?)`, time.Now(), time.Now()) + Expect(err).ToNot(HaveOccurred()) + + tx, err := db.BeginTx(ctx, nil) + Expect(err).ToNot(HaveOccurred()) + Expect(upRequeueSmartPlaylistCovers(ctx, tx)).To(Succeed()) + Expect(tx.Commit()).To(Succeed()) + }) + + // Queued ids and priorities, limited to rows whose retry_at has passed, as DequeueBatch reads them. + dequeueable := func() map[string]int { + rows, err := db.QueryContext(ctx, `SELECT item_id, priority FROM artwork_queue + WHERE item_kind = 'pl' AND retry_at <= ?`, time.Now()) + Expect(err).ToNot(HaveOccurred()) + defer rows.Close() + res := map[string]int{} + for rows.Next() { + var id string + var priority int + Expect(rows.Scan(&id, &priority)).To(Succeed()) + res[id] = priority + } + Expect(rows.Err()).ToNot(HaveOccurred()) + return res + } + + It("re-queues populated smart playlists that have no cover, ready for the next drain", func() { + Expect(dequeueable()).To(Equal(map[string]int{ + "absent": 50, + "no-row": 50, + "queued": 100, // already queued: left untouched + })) + }) +}) diff --git a/persistence/smart_playlist_repository.go b/persistence/smart_playlist_repository.go index 871f53dd2..0ea33a318 100644 --- a/persistence/smart_playlist_repository.go +++ b/persistence/smart_playlist_repository.go @@ -68,6 +68,7 @@ func (r *playlistRepository) refreshSmartPlaylistTree(ctx context.Context, pls * return false } + wasEmpty := pls.SongCount == 0 if err = r.refreshCounters(ctx, pls); err != nil { log.Error(ctx, "Error updating smart playlist stats", "playlist", pls.Name, "id", pls.ID, err) return false @@ -82,6 +83,12 @@ func (r *playlistRepository) refreshSmartPlaylistTree(ctx context.Context, pls * } pls.EvaluatedAt = &now + // A cover resolved while the playlist was empty has no grid. Rebuild only on that transition: + // rebuilding on every refresh would re-sample the grid, and re-run a grid that has no art. + if wasEmpty && pls.SongCount > 0 { + r.enqueueCoverRebuild(ctx, pls.ID) + } + log.Debug(ctx, "Refreshed playlist", "playlist", pls.Name, "id", pls.ID, "numTracks", pls.SongCount, "elapsed", time.Since(start)) return true } diff --git a/persistence/smart_playlist_repository_test.go b/persistence/smart_playlist_repository_test.go index 4d4c5edb0..39e8ea050 100644 --- a/persistence/smart_playlist_repository_test.go +++ b/persistence/smart_playlist_repository_test.go @@ -64,6 +64,47 @@ var _ = Describe("PlaylistRepository - Smart Playlists", func() { }) }) + Context("cover artwork", func() { + var pls model.Playlist + + queuedIDs := func() []string { + queued, err := NewArtworkQueueRepository(GetDBXBuilder()).DequeueBatch(ctx, 1000) + Expect(err).ToNot(HaveOccurred()) + return slice.Map(queued, func(q model.ArtworkQueueItem) string { return q.ItemID }) + } + + BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + conf.Server.SmartPlaylistRefreshDelay = -1 * time.Second + clearArtworkTables() + DeferCleanup(clearArtworkTables) + pls = model.Playlist{Name: "Covered", OwnerID: "userid", Rules: &criteria.Criteria{ + Expression: criteria.All{criteria.Contains{"title": ""}}, + }} + Expect(repo.Put(ctx, &pls)).To(Succeed()) + DeferCleanup(func() { _ = repo.Delete(ctx, pls.ID) }) + }) + + It("enqueues the cover when an evaluation fills an empty playlist", func() { + refreshed, err := repo.GetWithTracks(ctx, pls.ID, true, false) + Expect(err).ToNot(HaveOccurred()) + Expect(refreshed.Tracks).ToNot(BeEmpty()) + + Expect(queuedIDs()).To(ContainElement(pls.ID)) + }) + + It("does not enqueue the cover when re-evaluating a playlist that already had tracks", func() { + _, err := repo.GetWithTracks(ctx, pls.ID, true, false) + Expect(err).ToNot(HaveOccurred()) + clearArtworkTables() + + _, err = repo.GetWithTracks(ctx, pls.ID, true, false) + Expect(err).ToNot(HaveOccurred()) + + Expect(queuedIDs()).ToNot(ContainElement(pls.ID)) + }) + }) + Context("invalid rules", func() { It("fails to Put it in the DB", func() { rules = &criteria.Criteria{