mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-01 07:21:17 +00:00
fix(playlist): rebuild the generated cover only when the tracks change
The enqueue sat in refreshCounters, which Put also reaches for an ordinary metadata update, so renaming a playlist or editing its comment re-resolved the cover. The 2x2 grid samples albums with random(), so that silently handed the playlist a different cover for an edit that touched no tracks -- and refetched remote artwork to do it. It now happens where the track set actually changes: addTracks (which Put-with-tracks and updatePlaylist both funnel through) and renumber (reached from removeOrphans). Creation still enqueues even with no tracks, since an imported m3u can carry an ExternalImageURL. Reported by Codex on #5847.
This commit is contained in:
parent
4cac0b1401
commit
20c40521c6
@ -112,7 +112,8 @@ func (r *playlistRepository) Put(p *model.Playlist, cols ...string) error {
|
||||
_, err := r.put(pls.ID, pls, cols...)
|
||||
return err
|
||||
}
|
||||
if pls.ID == "" {
|
||||
isNew := pls.ID == ""
|
||||
if isNew {
|
||||
pls.CreatedAt = time.Now()
|
||||
}
|
||||
pls.UpdatedAt = time.Now()
|
||||
@ -131,7 +132,12 @@ func (r *playlistRepository) Put(p *model.Playlist, cols ...string) error {
|
||||
if len(pls.Tracks) > 0 {
|
||||
return r.updateTracks(id, p.MediaFiles())
|
||||
}
|
||||
pls.ID = id // r.put assigns the generated id to p, not to this copy; refreshCounters enqueues by it
|
||||
pls.ID = id // r.put assigns the generated id to p, not to this copy
|
||||
if isNew {
|
||||
// A brand-new playlist has art to find even with no tracks (an imported m3u can carry an
|
||||
// ExternalImageURL). An update landing here changed only metadata, so leave its cover be.
|
||||
r.enqueueCoverRebuild(id)
|
||||
}
|
||||
return r.refreshCounters(&pls.Playlist)
|
||||
}
|
||||
|
||||
@ -295,6 +301,7 @@ func (r *playlistRepository) addTracks(playlistId string, startingPos int, media
|
||||
}
|
||||
}
|
||||
|
||||
r.enqueueCoverRebuild(playlistId)
|
||||
return r.refreshCounters(&model.Playlist{ID: playlistId})
|
||||
}
|
||||
|
||||
@ -328,16 +335,21 @@ func (r *playlistRepository) refreshCounters(pls *model.Playlist) error {
|
||||
pls.SongCount = int(res.Count)
|
||||
pls.Duration = res.Duration
|
||||
pls.Size = int64(res.Size)
|
||||
// The generated 2x2 grid depends on the track set, so re-resolve the cover whenever it
|
||||
// changes. No clear: the old cover keeps serving until the worker rebuilds (no flicker).
|
||||
item := model.ArtworkQueueItem{ItemKind: model.KindPlaylistArtwork.Prefix(), ItemID: pls.ID, ImageType: model.ImageTypePrimary,
|
||||
Priority: model.ArtworkPriorityScan}
|
||||
if err := NewArtworkQueueRepository(r.ctx, r.db).Enqueue(item); err != nil {
|
||||
log.Warn(r.ctx, "could not enqueue playlist artwork after content change", "id", pls.ID, err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// enqueueCoverRebuild re-resolves the generated 2x2 grid, which depends on the track set. Called
|
||||
// only where that set actually changes: the grid samples albums at random, so rebuilding after a
|
||||
// mere rename would hand the playlist a different cover. No clear -- the old cover keeps serving
|
||||
// until the worker rebuilds, so there is no flicker.
|
||||
func (r *playlistRepository) enqueueCoverRebuild(id string) {
|
||||
item := model.ArtworkQueueItem{ItemKind: model.KindPlaylistArtwork.Prefix(), ItemID: id,
|
||||
ImageType: model.ImageTypePrimary, Priority: model.ArtworkPriorityScan}
|
||||
if err := NewArtworkQueueRepository(r.ctx, r.db).Enqueue(item); err != nil {
|
||||
log.Warn(r.ctx, "could not enqueue playlist artwork after content change", "id", id, err)
|
||||
}
|
||||
}
|
||||
|
||||
// tracksQuery is shared by loadTracks and GetCursor, so both hydrate rows identically.
|
||||
func (r *playlistRepository) tracksQuery(query SelectBuilder, id string) SelectBuilder {
|
||||
query = r.applyLibraryFilter(query, "f")
|
||||
@ -473,6 +485,7 @@ func (r *playlistRepository) renumber(id string) error {
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
r.enqueueCoverRebuild(id)
|
||||
return r.refreshCounters(&model.Playlist{ID: id})
|
||||
}
|
||||
|
||||
|
||||
@ -273,6 +273,32 @@ var _ = Describe("PlaylistRepository", func() {
|
||||
Expect(queued).ToNot(ContainElement(HaveField("ItemID", "")), "must not enqueue an empty playlist id")
|
||||
})
|
||||
|
||||
// The grid samples albums at random, so re-resolving after a rename would silently hand the
|
||||
// playlist a different cover.
|
||||
It("does not enqueue artwork when only metadata changes", func() {
|
||||
ctx := request.WithUser(log.NewContext(GinkgoT().Context()), model.User{ID: "userid", UserName: "userid", IsAdmin: true})
|
||||
newPls := model.Playlist{Name: "Rename Me", OwnerID: "userid"}
|
||||
Expect(repo.Put(&newPls)).To(Succeed())
|
||||
DeferCleanup(func() { _ = repo.Delete(newPls.ID) })
|
||||
// Clear the row creation just enqueued, so anything present afterwards came from the update.
|
||||
queueRepo := NewArtworkQueueRepository(ctx, GetDBXBuilder())
|
||||
queued, err := queueRepo.DequeueBatch(1000)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
for _, q := range queued {
|
||||
if q.ItemID == newPls.ID {
|
||||
Expect(queueRepo.DeleteIfUnchanged(q.ItemKind, q.ItemID, q.ImageType, q.RetryAt)).To(Succeed())
|
||||
}
|
||||
}
|
||||
|
||||
newPls.Name = "Renamed"
|
||||
newPls.Comment = "edited"
|
||||
Expect(repo.Put(&newPls)).To(Succeed())
|
||||
|
||||
queued, err = queueRepo.DequeueBatch(1000)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(queued).ToNot(ContainElement(HaveField("ItemID", newPls.ID)))
|
||||
})
|
||||
|
||||
It("enqueues the playlist's artwork when its track set changes", func() {
|
||||
ctx := request.WithUser(log.NewContext(GinkgoT().Context()), model.User{ID: "userid", UserName: "userid", IsAdmin: true})
|
||||
newPls := model.Playlist{Name: "Grid PL", OwnerID: "userid"}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user