remote/navidrome
1
0
Fork 0
mirror of https://github.com/navidrome/navidrome.git synced 2026-10-08 02:17:25 +02:00
Code Issues Projects Releases Packages Wiki Activity

refactor(artwork): move the keeps-state fact into core, drop a redundant guard

Browse source 
keepsArtworkState lived in package cmd and re-derived by hand what RefreshableKinds
already encodes: the same five-of-six kinds. It is now artwork.KeepsState, beside the
list, with a test pinning the two together — nothing else stopped them drifting, and a
drift would have explain report stored state for a kind that keeps none.

serveDisc's closure also hand-rolled a nil-reader error that both consumers of open()
now produce themselves: serveSource for a full-size request, resizedItem.Reader for a
resized one.
This commit is contained in:
Deluan 2026-08-14 19:16:42 -04:00
commit 2b2e02d7a3
4 changed files with 23 additions and 15 deletions
Download patch file Download diff file Expand all files Collapse all files

8
cmd/artwork.go
View file

@ -573,10 +573,6 @@ func explainResult(source string, steps []artwork.TraceStep) string {
return "not resolved"
}
// keepsArtworkState reports whether a kind is recorded in item_artwork and the queue at all. Disc
// artwork is resolved on every request and cached by content key, so it has neither.
func keepsArtworkState(kind model.Kind) bool { return kind != model.KindDiscArtwork }
// explainConfig names the setting that decides where a kind's artwork comes from, and its value.
func explainConfig(kind model.Kind) (name, value string) {
switch kind {
@ -608,7 +604,7 @@ func formatExplain(rep explainReport) string {
var sb strings.Builder
w := newTabWriter(&sb)
explainable := artwork.Explainable(rep.kind)
stateful := keepsArtworkState(rep.kind)
stateful := artwork.KeepsState(rep.kind)
fmt.Fprintln(w, "Item")
fmt.Fprintf(w, " Kind:\t%s (%s)\n", rep.kind, rep.kind.Prefix())
@ -693,7 +689,7 @@ func runExplain(ctx context.Context, kind model.Kind, id string) {
log.Fatal(ctx, "Item not found", "kind", kind, "id", id, err)
}
rep := explainReport{kind: kind, id: id, name: name}
if keepsArtworkState(kind) {
if artwork.KeepsState(kind) {
rep.stored, err = ds.Artwork(ctx).GetItemArtwork(kind, id, model.ImageTypePrimary)
if err != nil && !errors.Is(err, model.ErrNotFound) {
log.Fatal(ctx, "Failed to read artwork state", "kind", kind, "id", id, err)

8
core/artwork/artwork.go
View file

@ -320,13 +320,7 @@ func (s *service) serveDisc(ctx context.Context, artID model.ArtworkID, size int
// Single-disc albums run the chain too: a disc can carry art distinct from the album cover.
selectImage := func() (io.ReadCloser, error) {
res, err := dr.selectImage(ctx, s.ffmpeg, conf.Server.DiscArtPriority, &chainState{})
if err != nil {
return nil, err
}
if res.reader == nil {
return nil, fmt.Errorf("could not get `%s` cover art for %s: %w", artID.Kind, artID, ErrUnavailable)
}
return res.reader, nil
return res.reader, err
}
albumArtID := model.ArtworkID{Kind: model.KindAlbumArtwork, ID: dr.album.ID}
// Disc art has no state row, hence no content hash: keying on id, album mtime and

9
core/artwork/housekeeping.go
View file

@ -26,8 +26,13 @@ var RecheckKinds = []model.Kind{
model.KindArtistArtwork, model.KindAlbumArtwork, model.KindPlaylistArtwork, model.KindRadioArtwork,
}
// RefreshableKinds is every kind Refresh can clear and re-queue. Media files are absent from
// RecheckKinds but belong here: the worker resolves them, it just never revisits them on its own.
// KeepsState reports whether a kind is recorded in item_artwork and the artwork queue. Disc
// artwork is read through on every request and cached by content key, so it has neither.
func KeepsState(kind model.Kind) bool { return kind != model.KindDiscArtwork }
// RefreshableKinds is every kind Refresh can clear and re-queue, so it holds exactly the kinds
// KeepsState admits. Media files are absent from RecheckKinds but belong here: the worker
// resolves them, it just never revisits them on its own.
var RefreshableKinds = append(slices.Clone(RecheckKinds), model.KindMediaFileArtwork)
// hasRecheckPath reports whether a periodic job will revisit this kind, making an absent settle recoverable.

13
core/artwork/housekeeping_test.go
View file

@ -52,6 +52,19 @@ func (o *orderTrackingQueueRepo) Enqueue(items ...model.ArtworkQueueItem) error
return o.MockArtworkQueueRepo.Enqueue(items...)
}
var _ = Describe("RefreshableKinds", func() {
// The two are meant to describe the same fact. Nothing but this test stops them from drifting,
// and a drift would have `artwork explain` report state for a kind that keeps none.
It("holds exactly the kinds that keep state", func() {
for _, k := range []model.Kind{
model.KindArtistArtwork, model.KindAlbumArtwork, model.KindPlaylistArtwork,
model.KindRadioArtwork, model.KindMediaFileArtwork, model.KindDiscArtwork,
} {
Expect(slices.Contains(RefreshableKinds, k)).To(Equal(KeepsState(k)), k.String())
}
})
})
var _ = Describe("Housekeeping", func() {
var (
ctx context.Context