diff --git a/cmd/artwork.go b/cmd/artwork.go index d75bc09e9..915a3a393 100644 --- a/cmd/artwork.go +++ b/cmd/artwork.go @@ -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) diff --git a/core/artwork/artwork.go b/core/artwork/artwork.go index 91028521d..7edc80e99 100644 --- a/core/artwork/artwork.go +++ b/core/artwork/artwork.go @@ -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 diff --git a/core/artwork/housekeeping.go b/core/artwork/housekeeping.go index d6dddf402..ae1fc0a0d 100644 --- a/core/artwork/housekeeping.go +++ b/core/artwork/housekeeping.go @@ -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. diff --git a/core/artwork/housekeeping_test.go b/core/artwork/housekeeping_test.go index 45fbdb4f9..32a7688b4 100644 --- a/core/artwork/housekeeping_test.go +++ b/core/artwork/housekeeping_test.go @@ -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