mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-01 07:21:17 +00:00
fix(artwork): refresh songs when their album's artwork changes
A track with no art of its own is served its album's, so hydration copies the album's hash onto the track record. When an album resolution changed, the worker broadcast only an `album` refresh, leaving song and now-playing surfaces holding the previous hash-suffixed URL until something else refetched them. Pair the album refresh with a song one. The dependent id list is unbounded — an album has arbitrarily many tracks and a drained batch arbitrarily many albums — so this refreshes the resource as a whole via the protocol's existing wildcard rather than enumerating ids. Reported by Codex on #5847.
This commit is contained in:
parent
2bee12f2fb
commit
d94bf0953e
@ -250,6 +250,13 @@ func (w *Worker) broadcastRefresh(ctx context.Context, found []model.ArtworkQueu
|
||||
for res, ids := range byResource {
|
||||
event = event.With(res, ids...)
|
||||
}
|
||||
// A track with no art of its own is served its album's, so an album change moves the track's
|
||||
// hash too. The dependent id list is unbounded, so refresh the resource as a whole.
|
||||
if _, ok := byResource["album"]; ok {
|
||||
if _, ok := byResource["song"]; !ok {
|
||||
event = event.With("song")
|
||||
}
|
||||
}
|
||||
w.broker.SendBroadcastMessage(ctx, event)
|
||||
}
|
||||
|
||||
|
||||
@ -18,6 +18,7 @@ import (
|
||||
"github.com/navidrome/navidrome/server/events"
|
||||
"github.com/navidrome/navidrome/tests"
|
||||
"github.com/navidrome/navidrome/utils/cache"
|
||||
"github.com/navidrome/navidrome/utils/slice"
|
||||
. "github.com/onsi/ginkgo/v2"
|
||||
. "github.com/onsi/gomega"
|
||||
"go.uber.org/goleak"
|
||||
@ -439,8 +440,32 @@ var _ = Describe("Worker", func() {
|
||||
Expect(data).To(ContainSubstring("al2"))
|
||||
Expect(data).ToNot(ContainSubstring("artist"), "a failed (unresolved) artist must not be refreshed")
|
||||
Expect(data).ToNot(ContainSubstring("ar1"))
|
||||
Expect(data).To(ContainSubstring(`"song"`), "tracks with no art of their own are served the album's")
|
||||
})
|
||||
|
||||
DescribeTable("only pairs songs with album refreshes",
|
||||
func(kinds []string, wantSong bool) {
|
||||
items := slice.Map(kinds, func(k string) model.ArtworkQueueItem {
|
||||
return model.ArtworkQueueItem{ItemKind: k, ItemID: k + "1"}
|
||||
})
|
||||
w.broadcastRefresh(ctx, items)
|
||||
|
||||
evts := broker.getEvents()
|
||||
Expect(evts).To(HaveLen(1))
|
||||
data := evts[0].(*events.RefreshResource).Data(evts[0])
|
||||
if wantSong {
|
||||
Expect(data).To(ContainSubstring(`"song"`))
|
||||
} else {
|
||||
Expect(data).ToNot(ContainSubstring(`"song"`))
|
||||
}
|
||||
},
|
||||
Entry("album alone drags songs along", []string{"al"}, true),
|
||||
Entry("artist alone does not", []string{"ar"}, false),
|
||||
Entry("playlist alone does not", []string{"pl"}, false),
|
||||
Entry("album mixed with others still does", []string{"ar", "al"}, true),
|
||||
Entry("songs resolving on their own stay single-listed", []string{"mf"}, true),
|
||||
)
|
||||
|
||||
It("broadcasts a refresh when an item resolves to absent (removed cover)", func() {
|
||||
conf.Server.CoverArtPriority = "cover.*" // local-only; no folder image → absent
|
||||
ds.MockedAlbum.(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "al3", Name: "Artless"}})
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user