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{