From 454fd248339484ad415a79a64284b3b5eb544555 Mon Sep 17 00:00:00 2001 From: Deluan Date: Wed, 22 Jul 2026 11:57:32 -0400 Subject: [PATCH] fix(artwork): resolve full playlist source chain resolvePlaylist only built the generated grid, dropping the uploaded-image, sidecar and ExternalImageURL sources the old reader_playlist.go chain serves. Port the full chain before the grid fallback: uploaded (upload), sidecar (folder), and ExternalImageURL routed through extGate with the same extError semantics as the other external steps. Also rewires the artist external step onto ArtistImageResult. --- core/artwork/resolve.go | 46 +++++++++++++++++++++++--- core/artwork/resolve_test.go | 64 ++++++++++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+), 5 deletions(-) diff --git a/core/artwork/resolve.go b/core/artwork/resolve.go index 5aafdf5c5..272234c9d 100644 --- a/core/artwork/resolve.go +++ b/core/artwork/resolve.go @@ -10,6 +10,7 @@ import ( "image/png" "io" "io/fs" + "net/url" "os" "strings" @@ -29,7 +30,7 @@ type resolution struct { extError bool // an external source errored/timed out (forces failed, never absent) } -// extGateFunc is an alias for the external-step wrapper Task 4 injects (rate +// extGateFunc is an alias for the external-step wrapper the worker injects (rate // limiter + circuit breaker); resolveItem defaults to a plain passthrough. type extGateFunc = func(func() (io.ReadCloser, string, error)) (io.ReadCloser, string, error) @@ -137,7 +138,7 @@ func resolveArtist(ctx context.Context, ds model.DataStore, prov external.Provid pattern = strings.TrimSpace(pattern) switch { case pattern == "external": - if res, ok, isErr := resolveExternalStep(extGate, fromArtistExternalSource(ctx, *ar, prov)); ok { + if res, ok, isErr := resolveExternalStep(extGate, fromArtistExternalResult(ctx, *ar, prov)); ok { return res, nil } else if isErr { extErr = true @@ -165,20 +166,33 @@ func resolveArtist(ctx context.Context, ds model.DataStore, prov external.Provid return resolution{extError: extErr}, nil } -// resolvePlaylist ports the 2x2 generated grid from reader_playlist.go, -// sourcing tiles through resolveAlbum instead of the old cached reader. +// 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, prov external.Provider, ffm ffmpeg.FFmpeg, playlistID string, extGate extGateFunc) (resolution, error) { pl, err := ds.Playlist(ctx).Get(playlistID) if err != nil { return resolution{}, err } + + var extErr bool + if res, ok := resolveLocalFile(pl.UploadedImagePath(), "upload"); ok { + return res, nil + } + if res, ok := resolveLocalFile(findPlaylistSidecarPath(ctx, pl.Path), "folder"); ok { + return res, nil + } + if res, ok, isErr := resolveExternalStep(extGate, fromPlaylistExternalSource(ctx, *pl)); 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()"}) if err != nil { return resolution{}, err } var tiles []image.Image - var extErr bool var tileErr error // first internal (non-external) tile failure, e.g. album deleted mid-flight for _, albumID := range albumIDs { res, err := resolveAlbum(ctx, ds, prov, ffm, albumID, extGate) @@ -246,6 +260,28 @@ func resolveExternalStep(extGate extGateFunc, sf func() (io.ReadCloser, string, return resolution{}, false, err != nil && !errors.Is(err, model.ErrNotFound) } +// fromPlaylistExternalSource mirrors reader_playlist.go's ExternalImageURL step: +// a remote URL (gated) when M3U external art is enabled, else a local file path. +func fromPlaylistExternalSource(ctx context.Context, pl model.Playlist) sourceFunc { + return func() (io.ReadCloser, string, error) { + imgURL := pl.ExternalImageURL + if imgURL == "" { + return nil, "", nil + } + parsed, err := url.Parse(imgURL) + if err != nil { + return nil, "", err + } + if parsed.Scheme == "http" || parsed.Scheme == "https" { + if !conf.Server.EnableM3UExternalAlbumArt { + return nil, "", nil + } + return fromURL(ctx, parsed) + } + return fromLocalFile(imgURL)() + } +} + func resolveEmbedded(ctx context.Context, lib libraryView, ffm ffmpeg.FFmpeg, embedRel string) (resolution, bool) { if embedRel == "" { return resolution{}, false diff --git a/core/artwork/resolve_test.go b/core/artwork/resolve_test.go index c4ccc3733..cab757dbb 100644 --- a/core/artwork/resolve_test.go +++ b/core/artwork/resolve_test.go @@ -40,6 +40,10 @@ func (f *fakeExternalProvider) ArtistImage(ctx context.Context, id string) (*url return nil, model.ErrNotFound } +func (f *fakeExternalProvider) ArtistImageResult(ctx context.Context, id string) (*url.URL, error) { + return f.ArtistImage(ctx, id) +} + var _ = Describe("resolveItem", func() { var ( ctx context.Context @@ -334,6 +338,66 @@ var _ = Describe("resolveItem", func() { Entry("4 albums -> full grid", []string{"t1", "t2", "t3", "t4"}, tileSize-1), ) + It("resolves the uploaded image before the generated grid", func() { + tmpDir := GinkgoT().TempDir() + conf.Server.DataFolder = conf.NewDir(tmpDir) + Expect(os.MkdirAll(filepath.Join(tmpDir, "artwork", "playlist"), 0755)).To(Succeed()) + imgPath := filepath.Join(tmpDir, "artwork", "playlist", "plu_test.jpg") + Expect(os.WriteFile(imgPath, []byte("uploaded playlist image"), 0600)).To(Succeed()) + + plRepo := tests.CreateMockPlaylistRepo() + plRepo.SetData(model.Playlists{{ID: "plu", Name: "Playlist", UploadedImage: "plu_test.jpg"}}) + plRepo.TracksRepo = &tests.MockPlaylistTrackRepo{AlbumIDs: []string{"t1", "t2"}} + ds.MockedPlaylist = plRepo + + res, err := resolveItem(ctx, ds, prov, ffm, model.ArtworkQueueItem{ItemKind: "pl", ItemID: "plu"}, nil) + Expect(err).ToNot(HaveOccurred()) + Expect(res.reader).ToNot(BeNil()) + defer res.reader.Close() + Expect(res.source).To(Equal("upload")) + Expect(res.sourcePath).To(Equal(imgPath)) + }) + + It("resolves a sidecar image next to the playlist file before the grid", func() { + plDir := GinkgoT().TempDir() + Expect(os.WriteFile(filepath.Join(plDir, "list.m3u"), []byte("#EXTM3U"), 0600)).To(Succeed()) + Expect(os.WriteFile(filepath.Join(plDir, "list.jpg"), []byte("sidecar image"), 0600)).To(Succeed()) + + plRepo := tests.CreateMockPlaylistRepo() + plRepo.SetData(model.Playlists{{ID: "pls", Name: "Playlist", Path: filepath.Join(plDir, "list.m3u")}}) + plRepo.TracksRepo = &tests.MockPlaylistTrackRepo{AlbumIDs: []string{"t1", "t2"}} + ds.MockedPlaylist = plRepo + + res, err := resolveItem(ctx, ds, prov, ffm, model.ArtworkQueueItem{ItemKind: "pl", ItemID: "pls"}, nil) + Expect(err).ToNot(HaveOccurred()) + Expect(res.reader).ToNot(BeNil()) + defer res.reader.Close() + Expect(res.source).To(Equal("folder")) + Expect(filepath.ToSlash(res.sourcePath)).To(HaveSuffix("list.jpg")) + }) + + It("routes ExternalImageURL through extGate and sets extError on transient failure", func() { + conf.Server.EnableM3UExternalAlbumArt = true + folderRepo.result = nil // no grid tiles, so the external failure is what surfaces + + plRepo := tests.CreateMockPlaylistRepo() + plRepo.SetData(model.Playlists{{ID: "ple", Name: "Playlist", ExternalImageURL: "http://example.com/cover.jpg"}}) + plRepo.TracksRepo = &tests.MockPlaylistTrackRepo{AlbumIDs: []string{"t1"}} + ds.MockedPlaylist = plRepo + + var extGateCalls int + extGate := func(f func() (io.ReadCloser, string, error)) (io.ReadCloser, string, error) { + extGateCalls++ + return nil, "", errors.New("network down") + } + + res, err := resolveItem(ctx, ds, prov, ffm, model.ArtworkQueueItem{ItemKind: "pl", ItemID: "ple"}, extGate) + Expect(err).ToNot(HaveOccurred()) + Expect(res.reader).To(BeNil()) + Expect(res.extError).To(BeTrue()) + Expect(extGateCalls).To(Equal(1)) + }) + It("yields an empty resolution when no album has art", func() { ds.MockedAlbum.(*tests.MockAlbumRepo).SetData(model.Albums{ {ID: "empty1", Name: "Empty"},