diff --git a/core/artwork/agent_images.go b/core/artwork/agent_images.go index 2f7b5ceef..e9dddb7de 100644 --- a/core/artwork/agent_images.go +++ b/core/artwork/agent_images.go @@ -32,12 +32,6 @@ func passthroughGate(_ string, f func() (io.ReadCloser, string, error)) (io.Read return f() } -// denyGate refuses every external fetch with a definitive not-found, so local-only -// resolution never runs a network step even if an external branch is reached. -func denyGate(_ string, _ func() (io.ReadCloser, string, error)) (io.ReadCloser, string, error) { - return nil, "", model.ErrNotFound -} - // bestImageURL returns the largest-Size image URL, skipping empty or unparseable // URLs; nil when none qualifies. Parsing happens per candidate so a malformed largest // URL never shadows a valid smaller one. diff --git a/core/artwork/processor.go b/core/artwork/processor.go index f179370fc..7ce5a44b1 100644 --- a/core/artwork/processor.go +++ b/core/artwork/processor.go @@ -71,7 +71,7 @@ type acquired struct { func processItem(ctx context.Context, deps *workerDeps, item model.ArtworkQueueItem) (outcome, *acquired) { repo := deps.ds.Artwork(ctx) - res, err := resolveItem(ctx, deps.ds, deps.agents, deps.ffmpeg, item, deps.gate) + res, err := newResolver(deps.ds, deps.agents, deps.ffmpeg, deps.gate).resolve(ctx, item) if err != nil { log.Warn(ctx, "artwork: could not resolve item", "kind", item.ItemKind, "id", item.ItemID, err) return outcomeFailed, nil diff --git a/core/artwork/resolve.go b/core/artwork/resolve.go index af165c8ac..a621931d3 100644 --- a/core/artwork/resolve.go +++ b/core/artwork/resolve.go @@ -35,50 +35,80 @@ type resolution struct { localError bool } -// resolveItem walks the kind's priority chain and returns the first hit. -func resolveItem(ctx context.Context, ds model.DataStore, ag *agents.Agents, ffmpeg ffmpeg.FFmpeg, item model.ArtworkQueueItem, gate gateFunc) (resolution, error) { - return resolveItemMode(ctx, ds, ag, ffmpeg, item, gate, false) +// externalSource is the ability to reach the network: which agents to ask, and the per-agent +// rate limiter and circuit breaker to ask them through. +type externalSource struct { + agents *agents.Agents + gate gateFunc } -// resolveItemLocal resolves using only local sources for the serving path's provisional -// read-through: external steps are skipped and the worker-built playlist grid is not assembled. -func resolveItemLocal(ctx context.Context, ds model.DataStore, ffmpeg ffmpeg.FFmpeg, item model.ArtworkQueueItem) (resolution, error) { - return resolveItemMode(ctx, ds, nil, ffmpeg, item, denyGate, true) +// resolver walks a kind's priority chain and returns the first hit. A nil ext means local-only, +// so "may this resolution go external" is one fact rather than two that could disagree. +type resolver struct { + ds model.DataStore + ffmpeg ffmpeg.FFmpeg + ext *externalSource } -func resolveItemMode(ctx context.Context, ds model.DataStore, ag *agents.Agents, ffmpeg ffmpeg.FFmpeg, item model.ArtworkQueueItem, gate gateFunc, localOnly bool) (resolution, error) { +func newResolver(ds model.DataStore, ag *agents.Agents, ffm ffmpeg.FFmpeg, gate gateFunc) *resolver { if gate == nil { gate = passthroughGate } + return &resolver{ds: ds, ffmpeg: ffm, ext: &externalSource{agents: ag, gate: gate}} +} + +// newLocalResolver offers no parameter through which an external source could be supplied, so +// a request can neither reach the network nor sample album art for the worker-built grid. +func newLocalResolver(ds model.DataStore, ffm ffmpeg.FFmpeg) *resolver { + return &resolver{ds: ds, ffmpeg: ffm} +} + +func (r *resolver) resolve(ctx context.Context, item model.ArtworkQueueItem) (resolution, error) { kind, _ := model.ParseKind(item.ItemKind) switch kind { case model.KindAlbumArtwork: - return resolveAlbum(ctx, ds, ag, ffmpeg, item.ItemID, gate, localOnly) + return r.resolveAlbum(ctx, item.ItemID) case model.KindArtistArtwork: - return resolveArtist(ctx, ds, ag, ffmpeg, item.ItemID, gate, localOnly) + return r.resolveArtist(ctx, item.ItemID) case model.KindPlaylistArtwork: - return resolvePlaylist(ctx, ds, ag, ffmpeg, item.ItemID, gate, localOnly) + return r.resolvePlaylist(ctx, item.ItemID) case model.KindRadioArtwork: - return resolveRadio(ctx, ds, item.ItemID) + return r.resolveRadio(ctx, item.ItemID) case model.KindMediaFileArtwork: - return resolveMediaFile(ctx, ds, ffmpeg, item.ItemID) + return r.resolveMediaFile(ctx, item.ItemID) default: - return resolution{}, fmt.Errorf("resolveItem: kind %q is not resolvable by the worker", item.ItemKind) + return resolution{}, fmt.Errorf("artwork: kind %q is not resolvable by the worker", item.ItemKind) } } +// fetchExternalAlbum and fetchExternalArtist are the only places resolution touches the network, +// so a local-only resolver is stopped here rather than at each point in the chain walk. +func (r *resolver) fetchExternalAlbum(ctx context.Context, al model.Album) (io.ReadCloser, string, bool) { + if r.ext == nil { + return nil, "", false + } + return fetchAlbumImage(ctx, r.ext.agents, r.ext.gate, al) +} + +func (r *resolver) fetchExternalArtist(ctx context.Context, ar model.Artist) (io.ReadCloser, string, bool) { + if r.ext == nil { + return nil, "", false + } + return fetchArtistImage(ctx, r.ext.agents, r.ext.gate, ar) +} + // resolveAlbum ports the folder/embedded/external selection from // reader_album.go, walking conf.Server.CoverArtPriority. -func resolveAlbum(ctx context.Context, ds model.DataStore, ag *agents.Agents, ffm ffmpeg.FFmpeg, albumID string, gate gateFunc, localOnly bool) (resolution, error) { - al, err := ds.Album(ctx).Get(albumID) +func (r *resolver) resolveAlbum(ctx context.Context, albumID string) (resolution, error) { + al, err := r.ds.Album(ctx).Get(albumID) if err != nil { return resolution{}, err } - _, imgFiles, _, err := loadAlbumFoldersPaths(ctx, ds, *al) + _, imgFiles, _, err := loadAlbumFoldersPaths(ctx, r.ds, *al) if err != nil { return resolution{}, err } - lib, err := loadLibraryView(ctx, ds, al.LibraryID) + lib, err := loadLibraryView(ctx, r.ds, al.LibraryID) if err != nil { return resolution{}, err } @@ -88,18 +118,15 @@ func resolveAlbum(ctx context.Context, ds model.DataStore, ag *agents.Agents, ff pattern = strings.TrimSpace(pattern) switch { case pattern == "embedded": - res, ok := resolveEmbedded(ctx, lib, ffm, al.EmbedArtPath) + res, ok := resolveEmbedded(ctx, lib, r.ffmpeg, al.EmbedArtPath) if ok { res.extError = extErr return res, nil } localErr = localErr || res.localError case pattern == "external": - if localOnly { - continue - } - if r, name, isErr := fetchAlbumImage(ctx, ag, gate, *al); r != nil { - return resolution{reader: r, source: "external:" + name}, nil + if rd, name, isErr := r.fetchExternalAlbum(ctx, *al); rd != nil { + return resolution{reader: rd, source: "external:" + name}, nil } else if isErr { extErr = true } @@ -117,8 +144,8 @@ func resolveAlbum(ctx context.Context, ds model.DataStore, ag *agents.Agents, ff // resolveArtist ports the upload/folder/external selection from // reader_artist.go: upload always wins, then conf.Server.ArtistArtPriority. -func resolveArtist(ctx context.Context, ds model.DataStore, ag *agents.Agents, ffm ffmpeg.FFmpeg, artistID string, gate gateFunc, localOnly bool) (resolution, error) { - ar, err := ds.Artist(ctx).Get(artistID) +func (r *resolver) resolveArtist(ctx context.Context, artistID string) (resolution, error) { + ar, err := r.ds.Artist(ctx).Get(artistID) if err != nil { return resolution{}, err } @@ -133,7 +160,7 @@ func resolveArtist(ctx context.Context, ds model.DataStore, ag *agents.Agents, f } // Only consider albums where the artist is the sole album artist, same as reader_artist.go. - als, err := ds.Album(ctx).GetAll(model.QueryOptions{ + als, err := r.ds.Album(ctx).GetAll(model.QueryOptions{ Filters: squirrel.And{ squirrel.Eq{"album_artist_id": artistID}, squirrel.Eq{"json_array_length(participants, '$.albumartist')": 1}, @@ -142,17 +169,17 @@ func resolveArtist(ctx context.Context, ds model.DataStore, ag *agents.Agents, f if err != nil { return resolution{}, err } - albumPaths, imgFiles, _, err := loadAlbumFoldersPaths(ctx, ds, als...) + albumPaths, imgFiles, _, err := loadAlbumFoldersPaths(ctx, r.ds, als...) if err != nil { return resolution{}, err } - artistFolder, _, err := loadArtistFolder(ctx, ds, als, albumPaths) + artistFolder, _, err := loadArtistFolder(ctx, r.ds, als, albumPaths) if err != nil { return resolution{}, err } var lib libraryView if len(als) > 0 { - lib, err = loadLibraryView(ctx, ds, als[0].LibraryID) + lib, err = loadLibraryView(ctx, r.ds, als[0].LibraryID) if err != nil { return resolution{}, err } @@ -163,11 +190,8 @@ func resolveArtist(ctx context.Context, ds model.DataStore, ag *agents.Agents, f pattern = strings.TrimSpace(pattern) switch { case pattern == "external": - if localOnly { - continue - } - if r, name, isErr := fetchArtistImage(ctx, ag, gate, *ar); r != nil { - return resolution{reader: r, source: "external:" + name}, nil + if rd, name, isErr := r.fetchExternalArtist(ctx, *ar); rd != nil { + return resolution{reader: rd, source: "external:" + name}, nil } else if isErr { extErr = true } @@ -205,8 +229,8 @@ func resolveArtist(ctx context.Context, ds model.DataStore, ag *agents.Agents, f // resolvePlaylist ports reader_playlist.go's chain: uploaded image, sidecar, // ExternalImageURL, then the generated 2x2 grid sourced through resolveAlbum. -func resolvePlaylist(ctx context.Context, ds model.DataStore, ag *agents.Agents, ffm ffmpeg.FFmpeg, playlistID string, gate gateFunc, localOnly bool) (resolution, error) { - pl, err := ds.Playlist(ctx).Get(playlistID) +func (r *resolver) resolvePlaylist(ctx context.Context, playlistID string) (resolution, error) { + pl, err := r.ds.Playlist(ctx).Get(playlistID) if err != nil { return resolution{}, err } @@ -237,21 +261,21 @@ func resolvePlaylist(ctx context.Context, ds model.DataStore, ag *agents.Agents, return res, nil } } - if localOnly { + if r.ext == nil { // The remote ExternalImageURL fetch and the 2x2 grid are worker-only; a request must // not fetch remotely nor sample album art synchronously. return resolution{}, nil } if remoteImg != nil && conf.Server.EnableM3UExternalAlbumArt { sf := func() (io.ReadCloser, string, error) { return fromURL(ctx, remoteImg) } - if res, ok, isErr := resolveExternalStep(gate, "m3u", sf); ok { + if res, ok, isErr := resolveExternalStep(r.ext.gate, "m3u", sf); ok { return res, nil } else if isErr { extErr = true } } - albumIDs, err := ds.Playlist(ctx).Tracks(pl.ID, false).GetAlbumIDs(model.QueryOptions{Max: 4, Sort: "random()"}) + albumIDs, err := r.ds.Playlist(ctx).Tracks(pl.ID, false).GetAlbumIDs(model.QueryOptions{Max: 4, Sort: "random()"}) if err != nil { return resolution{}, err } @@ -259,7 +283,7 @@ func resolvePlaylist(ctx context.Context, ds model.DataStore, ag *agents.Agents, var tiles []image.Image var tileErr error // first internal (non-external) tile failure, e.g. album deleted mid-flight for _, albumID := range albumIDs { - res, err := resolveAlbum(ctx, ds, ag, ffm, albumID, gate, false) + res, err := r.resolveAlbum(ctx, albumID) if err != nil { if tileErr == nil { tileErr = err @@ -296,38 +320,38 @@ func resolvePlaylist(ctx context.Context, ds model.DataStore, ag *agents.Agents, case 3: tiles = append(tiles, tiles[0]) } - r, err := assembleTiles(tiles) + grid, err := assembleTiles(tiles) if err != nil { - return resolution{extError: extErr}, nil //nolint:nilerr // encode failure is a soft "no image", not a resolveItem error + return resolution{extError: extErr}, nil //nolint:nilerr // encode failure is a soft "no image", not a resolution error } - return resolution{reader: r, source: "generated", extError: extErr}, nil + return resolution{reader: grid, source: "generated", extError: extErr}, nil } // resolveRadio ports reader_radio.go: only an uploaded image, no fallback. -func resolveRadio(ctx context.Context, ds model.DataStore, radioID string) (resolution, error) { - r, err := ds.Radio(ctx).Get(radioID) +func (r *resolver) resolveRadio(ctx context.Context, radioID string) (resolution, error) { + radio, err := r.ds.Radio(ctx).Get(radioID) if err != nil { return resolution{}, err } - res, _ := resolveLocalFile(r.UploadedImagePath(), "upload") + res, _ := resolveLocalFile(radio.UploadedImagePath(), "upload") return res, nil } // resolveMediaFile resolves a track's own embedded art only; there is no folder or // external fallback, so disabled/missing cover art is a definitive absent. -func resolveMediaFile(ctx context.Context, ds model.DataStore, ffm ffmpeg.FFmpeg, id string) (resolution, error) { - mf, err := ds.MediaFile(ctx).Get(id) +func (r *resolver) resolveMediaFile(ctx context.Context, id string) (resolution, error) { + mf, err := r.ds.MediaFile(ctx).Get(id) if err != nil { return resolution{}, err } if !conf.Server.EnableMediaFileCoverArt || !mf.HasCoverArt { return resolution{}, nil } - lib, err := loadLibraryView(ctx, ds, mf.LibraryID) + lib, err := loadLibraryView(ctx, r.ds, mf.LibraryID) if err != nil { return resolution{}, err } - res, _ := resolveEmbedded(ctx, lib, ffm, mf.Path) + res, _ := resolveEmbedded(ctx, lib, r.ffmpeg, mf.Path) return res, nil } diff --git a/core/artwork/resolve_test.go b/core/artwork/resolve_test.go index 6a78dbf45..6c866aa5c 100644 --- a/core/artwork/resolve_test.go +++ b/core/artwork/resolve_test.go @@ -52,7 +52,7 @@ var _ = Describe("resolveItem", func() { Describe("kind dispatch", func() { It("returns an error for kinds the worker never enqueues", func() { - _, err := resolveItem(ctx, ds, ag, ffm, model.ArtworkQueueItem{ItemKind: "zz", ItemID: "x"}, nil) + _, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "zz", ItemID: "x"}) Expect(err).To(HaveOccurred()) }) }) @@ -68,7 +68,7 @@ var _ = Describe("resolveItem", func() { {ID: "mf1", LibraryID: 0, Path: "tests/fixtures/artist/an-album/test.mp3", HasCoverArt: true}, }) - res, err := resolveItem(ctx, ds, ag, ffm, model.ArtworkQueueItem{ItemKind: "mf", ItemID: "mf1"}, nil) + res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "mf", ItemID: "mf1"}) Expect(err).ToNot(HaveOccurred()) Expect(res.reader).ToNot(BeNil()) defer res.reader.Close() @@ -83,7 +83,7 @@ var _ = Describe("resolveItem", func() { {ID: "mf2", LibraryID: 0, Path: "tests/fixtures/artist/an-album/test.mp3", HasCoverArt: false}, }) - res, err := resolveItem(ctx, ds, ag, ffm, model.ArtworkQueueItem{ItemKind: "mf", ItemID: "mf2"}, nil) + res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "mf", ItemID: "mf2"}) Expect(err).ToNot(HaveOccurred()) Expect(res.reader).To(BeNil()) Expect(res.extError).To(BeFalse()) @@ -95,13 +95,13 @@ var _ = Describe("resolveItem", func() { {ID: "mf3", LibraryID: 0, Path: "tests/fixtures/artist/an-album/test.mp3", HasCoverArt: true}, }) - res, err := resolveItem(ctx, ds, ag, ffm, model.ArtworkQueueItem{ItemKind: "mf", ItemID: "mf3"}, nil) + res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "mf", ItemID: "mf3"}) Expect(err).ToNot(HaveOccurred()) Expect(res.reader).To(BeNil()) }) It("returns the error when the track is not in the DB", func() { - res, err := resolveItem(ctx, ds, ag, ffm, model.ArtworkQueueItem{ItemKind: "mf", ItemID: "missing"}, nil) + res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "mf", ItemID: "missing"}) Expect(err).To(MatchError(model.ErrNotFound)) Expect(res.reader).To(BeNil()) }) @@ -122,7 +122,7 @@ var _ = Describe("resolveItem", func() { {ID: "al1", Name: "Album", FolderIDs: []string{"f1"}}, }) - res, err := resolveItem(ctx, ds, ag, ffm, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al1"}, nil) + res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al1"}) Expect(err).ToNot(HaveOccurred()) Expect(res.reader).ToNot(BeNil()) defer res.reader.Close() @@ -138,7 +138,7 @@ var _ = Describe("resolveItem", func() { {ID: "al2", Name: "Album", EmbedArtPath: "tests/fixtures/artist/an-album/test.mp3", FolderIDs: []string{"f1"}}, }) - res, err := resolveItem(ctx, ds, ag, ffm, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al2"}, nil) + res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al2"}) Expect(err).ToNot(HaveOccurred()) Expect(res.reader).ToNot(BeNil()) defer res.reader.Close() @@ -154,7 +154,7 @@ var _ = Describe("resolveItem", func() { }) imageAgents(&fakeImageAgent{name: "failAgent", err: errors.New("agent timed out")}) - res, err := resolveItem(ctx, ds, ag, ffm, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al3"}, nil) + res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al3"}) Expect(err).ToNot(HaveOccurred()) Expect(res.reader).To(BeNil()) Expect(res.extError).To(BeTrue()) @@ -167,7 +167,7 @@ var _ = Describe("resolveItem", func() { }) // no image agents enabled -> the external step is a definitive not-found - res, err := resolveItem(ctx, ds, ag, ffm, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al4"}, nil) + res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al4"}) Expect(err).ToNot(HaveOccurred()) Expect(res.reader).To(BeNil()) Expect(res.extError).To(BeFalse()) @@ -184,7 +184,7 @@ var _ = Describe("resolveItem", func() { }) imageAgents(&fakeImageAgent{name: "failAgent", err: errors.New("agent timed out")}) - res, err := resolveItem(ctx, ds, ag, ffm, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al6"}, nil) + res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al6"}) Expect(err).ToNot(HaveOccurred()) Expect(res.reader).ToNot(BeNil()) defer res.reader.Close() @@ -203,7 +203,7 @@ var _ = Describe("resolveItem", func() { }) // no image agents enabled -> the external step is a definitive not-found - res, err := resolveItem(ctx, ds, ag, ffm, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al7"}, nil) + res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al7"}) Expect(err).ToNot(HaveOccurred()) Expect(res.reader).ToNot(BeNil()) defer res.reader.Close() @@ -223,7 +223,7 @@ var _ = Describe("resolveItem", func() { return f() } - res, err := resolveItem(ctx, ds, ag, ffm, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al5"}, gate) + res, err := newResolver(ds, ag, ffm, gate).resolve(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al5"}) Expect(err).ToNot(HaveOccurred()) Expect(res.extError).To(BeTrue()) Expect(gatedNames).To(Equal([]string{"failAgent"})) @@ -242,7 +242,7 @@ var _ = Describe("resolveItem", func() { artistRepo.SetData(model.Artists{{ID: "ar1", Name: "Artist", UploadedImage: "ar1_test.jpg"}}) ds.MockedArtist = artistRepo - res, err := resolveItem(ctx, ds, ag, ffm, model.ArtworkQueueItem{ItemKind: "ar", ItemID: "ar1"}, nil) + res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "ar", ItemID: "ar1"}) Expect(err).ToNot(HaveOccurred()) Expect(res.reader).ToNot(BeNil()) defer res.reader.Close() @@ -265,7 +265,7 @@ var _ = Describe("resolveItem", func() { {ID: "al9", Name: "Album", LibraryID: 0, FolderIDs: []string{"f1"}}, } - res, err := resolveItem(ctx, ds, ag, ffm, model.ArtworkQueueItem{ItemKind: "ar", ItemID: "ar2"}, nil) + res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "ar", ItemID: "ar2"}) Expect(err).ToNot(HaveOccurred()) Expect(res.reader).ToNot(BeNil()) defer res.reader.Close() @@ -280,7 +280,7 @@ var _ = Describe("resolveItem", func() { ds.MockedArtist = artistRepo imageAgents(&fakeImageAgent{name: "failAgent", err: errors.New("agent timed out")}) - res, err := resolveItem(ctx, ds, ag, ffm, model.ArtworkQueueItem{ItemKind: "ar", ItemID: "ar3"}, nil) + res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "ar", ItemID: "ar3"}) Expect(err).ToNot(HaveOccurred()) Expect(res.reader).To(BeNil()) Expect(res.extError).To(BeTrue()) @@ -293,7 +293,7 @@ var _ = Describe("resolveItem", func() { ds.MockedArtist = artistRepo // no image agents enabled -> the external step is a definitive not-found - res, err := resolveItem(ctx, ds, ag, ffm, model.ArtworkQueueItem{ItemKind: "ar", ItemID: "ar4"}, nil) + res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "ar", ItemID: "ar4"}) Expect(err).ToNot(HaveOccurred()) Expect(res.reader).To(BeNil()) Expect(res.extError).To(BeFalse()) @@ -311,7 +311,7 @@ var _ = Describe("resolveItem", func() { return f() } - res, err := resolveItem(ctx, ds, ag, ffm, model.ArtworkQueueItem{ItemKind: "ar", ItemID: "ar5"}, gate) + res, err := newResolver(ds, ag, ffm, gate).resolve(ctx, model.ArtworkQueueItem{ItemKind: "ar", ItemID: "ar5"}) Expect(err).ToNot(HaveOccurred()) Expect(res.extError).To(BeTrue()) Expect(gatedNames).To(Equal([]string{"failAgent"})) @@ -327,7 +327,7 @@ var _ = Describe("resolveItem", func() { radioRepo.Data = map[string]*model.Radio{"ra1": {ID: "ra1", Name: "Radio"}} ds.MockedRadio = radioRepo - res, err := resolveItem(ctx, ds, ag, ffm, model.ArtworkQueueItem{ItemKind: "ra", ItemID: "ra1"}, nil) + res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "ra", ItemID: "ra1"}) Expect(err).ToNot(HaveOccurred()) Expect(res).To(Equal(resolution{})) }) @@ -343,7 +343,7 @@ var _ = Describe("resolveItem", func() { radioRepo.Data = map[string]*model.Radio{"ra2": {ID: "ra2", Name: "Radio", UploadedImage: "ra2_test.jpg"}} ds.MockedRadio = radioRepo - res, err := resolveItem(ctx, ds, ag, ffm, model.ArtworkQueueItem{ItemKind: "ra", ItemID: "ra2"}, nil) + res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "ra", ItemID: "ra2"}) Expect(err).ToNot(HaveOccurred()) Expect(res.reader).ToNot(BeNil()) defer res.reader.Close() @@ -375,7 +375,7 @@ var _ = Describe("resolveItem", func() { plRepo.TracksRepo = &tests.MockPlaylistTrackRepo{AlbumIDs: albumIDs} ds.MockedPlaylist = plRepo - res, err := resolveItem(ctx, ds, ag, ffm, model.ArtworkQueueItem{ItemKind: "pl", ItemID: "pl1"}, nil) + res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "pl", ItemID: "pl1"}) Expect(err).ToNot(HaveOccurred()) Expect(res.reader).ToNot(BeNil()) defer res.reader.Close() @@ -407,7 +407,7 @@ var _ = Describe("resolveItem", func() { plRepo.TracksRepo = &tests.MockPlaylistTrackRepo{AlbumIDs: []string{"t1", "t2"}} ds.MockedPlaylist = plRepo - res, err := resolveItem(ctx, ds, ag, ffm, model.ArtworkQueueItem{ItemKind: "pl", ItemID: "plu"}, nil) + res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "pl", ItemID: "plu"}) Expect(err).ToNot(HaveOccurred()) Expect(res.reader).ToNot(BeNil()) defer res.reader.Close() @@ -425,7 +425,7 @@ var _ = Describe("resolveItem", func() { plRepo.TracksRepo = &tests.MockPlaylistTrackRepo{AlbumIDs: []string{"t1", "t2"}} ds.MockedPlaylist = plRepo - res, err := resolveItem(ctx, ds, ag, ffm, model.ArtworkQueueItem{ItemKind: "pl", ItemID: "pls"}, nil) + res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "pl", ItemID: "pls"}) Expect(err).ToNot(HaveOccurred()) Expect(res.reader).ToNot(BeNil()) defer res.reader.Close() @@ -443,7 +443,7 @@ var _ = Describe("resolveItem", func() { plRepo.TracksRepo = &tests.MockPlaylistTrackRepo{AlbumIDs: []string{"t1"}} ds.MockedPlaylist = plRepo - res, err := resolveItem(ctx, ds, ag, ffm, model.ArtworkQueueItem{ItemKind: "pl", ItemID: "pll"}, nil) + res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "pl", ItemID: "pll"}) Expect(err).ToNot(HaveOccurred()) Expect(res.reader).ToNot(BeNil()) defer res.reader.Close() @@ -467,7 +467,7 @@ var _ = Describe("resolveItem", func() { return nil, "", errors.New("network down") } - res, err := resolveItem(ctx, ds, ag, ffm, model.ArtworkQueueItem{ItemKind: "pl", ItemID: "ple"}, gate) + res, err := newResolver(ds, ag, ffm, gate).resolve(ctx, model.ArtworkQueueItem{ItemKind: "pl", ItemID: "ple"}) Expect(err).ToNot(HaveOccurred()) Expect(res.reader).To(BeNil()) Expect(res.extError).To(BeTrue()) @@ -482,7 +482,7 @@ var _ = Describe("resolveItem", func() { plRepo.TracksRepo = &tests.MockPlaylistTrackRepo{AlbumIDs: []string{"t1"}} ds.MockedPlaylist = plRepo - res, err := resolveItem(ctx, ds, ag, ffm, model.ArtworkQueueItem{ItemKind: "pl", ItemID: "plm"}, nil) + res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "pl", ItemID: "plm"}) Expect(err).ToNot(HaveOccurred()) Expect(res.reader).To(BeNil()) Expect(res.extError).To(BeFalse()) @@ -500,7 +500,7 @@ var _ = Describe("resolveItem", func() { plRepo.TracksRepo = &tests.MockPlaylistTrackRepo{AlbumIDs: []string{"t1"}} ds.MockedPlaylist = plRepo - res, err := resolveItem(ctx, ds, ag, ffm, model.ArtworkQueueItem{ItemKind: "pl", ItemID: "pl404"}, nil) + res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "pl", ItemID: "pl404"}) Expect(err).ToNot(HaveOccurred()) Expect(res.reader).ToNot(BeNil()) defer res.reader.Close() @@ -508,6 +508,19 @@ var _ = Describe("resolveItem", func() { Expect(res.extError).To(BeFalse()) }) + // A local resolver holds no agents, and the external branch dereferences them before the + // gate is ever consulted, so reaching it at all would panic rather than degrade. + It("skips the external step instead of dereferencing absent agents", func() { + conf.Server.CoverArtPriority = "external" + ds.MockedAlbum = tests.CreateMockAlbumRepo() + ds.MockedAlbum.(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "alx", Name: "Album"}}) + + res, err := newLocalResolver(ds, ffm).resolve(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alx"}) + Expect(err).ToNot(HaveOccurred()) + Expect(res.reader).To(BeNil()) + Expect(res.extError).To(BeFalse(), "a skipped step is not a failed one") + }) + // The request path must never reach the network nor sample album art synchronously. The // worker resolving the same playlist is asserted alongside, so this cannot pass vacuously. It("resolves a playlist locally without fetching remotely or building the grid", func() { @@ -525,12 +538,12 @@ var _ = Describe("resolveItem", func() { ds.MockedPlaylist = plRepo item := model.ArtworkQueueItem{ItemKind: "pl", ItemID: "pllocal"} - res, err := resolveItemLocal(ctx, ds, ffm, item) + res, err := newLocalResolver(ds, ffm).resolve(ctx, item) Expect(err).ToNot(HaveOccurred()) Expect(res.reader).To(BeNil(), "no local source, and the grid is worker-only") Expect(hits.Load()).To(BeZero(), "a request must never reach the network") - worker, err := resolveItem(ctx, ds, ag, ffm, item, nil) + worker, err := newResolver(ds, ag, ffm, nil).resolve(ctx, item) Expect(err).ToNot(HaveOccurred()) Expect(worker.reader).ToNot(BeNil()) defer worker.reader.Close() @@ -551,7 +564,7 @@ var _ = Describe("resolveItem", func() { plRepo.TracksRepo = &tests.MockPlaylistTrackRepo{AlbumIDs: []string{"t1"}} ds.MockedPlaylist = plRepo - res, err := resolveItem(ctx, ds, ag, ffm, model.ArtworkQueueItem{ItemKind: "pl", ItemID: "pl500"}, nil) + res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "pl", ItemID: "pl500"}) Expect(err).ToNot(HaveOccurred()) Expect(res.reader).To(BeNil()) Expect(res.extError).To(BeTrue()) @@ -567,7 +580,7 @@ var _ = Describe("resolveItem", func() { ds.MockedPlaylist = plRepo folderRepo.result = nil - res, err := resolveItem(ctx, ds, ag, ffm, model.ArtworkQueueItem{ItemKind: "pl", ItemID: "pl2"}, nil) + res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "pl", ItemID: "pl2"}) Expect(err).ToNot(HaveOccurred()) Expect(res.reader).To(BeNil()) Expect(res.source).To(BeEmpty()) @@ -586,7 +599,7 @@ var _ = Describe("resolveItem", func() { plRepo.TracksRepo = &tests.MockPlaylistTrackRepo{AlbumIDs: []string{"t1"}} ds.MockedPlaylist = plRepo - res, err := resolveItem(ctx, ds, ag, ffm, model.ArtworkQueueItem{ItemKind: "pl", ItemID: "plbomb"}, nil) + res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "pl", ItemID: "plbomb"}) Expect(err).ToNot(HaveOccurred()) Expect(res.reader).To(BeNil()) Expect(res.source).To(BeEmpty()) @@ -600,7 +613,7 @@ var _ = Describe("resolveItem", func() { plRepo.TracksRepo = &tests.MockPlaylistTrackRepo{AlbumIDs: []string{"missing1", "missing2"}} ds.MockedPlaylist = plRepo - res, err := resolveItem(ctx, ds, ag, ffm, model.ArtworkQueueItem{ItemKind: "pl", ItemID: "pl3"}, nil) + res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "pl", ItemID: "pl3"}) Expect(err).To(HaveOccurred()) Expect(res).To(Equal(resolution{})) }) diff --git a/core/artwork/serving.go b/core/artwork/serving.go index 38b86ce9c..2b8e61c81 100644 --- a/core/artwork/serving.go +++ b/core/artwork/serving.go @@ -188,7 +188,7 @@ func openOriginal(ia *model.ItemArtwork, mime string, store *ImageStore) (io.Rea // the worker (Bump) and serves any local bytes immediately, never writing a state row. func (s *service) provisional(ctx context.Context, artID model.ArtworkID, size int, square bool) (*Image, error) { item := model.ArtworkQueueItem{ItemKind: artID.Kind.Prefix(), ItemID: artID.ID, ImageType: model.ImageTypePrimary} - res, err := resolveItemLocal(ctx, s.ds, s.ffmpeg, item) + res, err := newLocalResolver(s.ds, s.ffmpeg).resolve(ctx, item) if err != nil { return nil, err }