From cfca4a2433d64e75a8c60c909511653d11c1f374 Mon Sep 17 00:00:00 2001 From: Deluan Date: Thu, 23 Jul 2026 05:30:47 -0400 Subject: [PATCH] fix(artwork): serve a local playlist ExternalImageURL as a file-backed reference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A local ExternalImageURL was resolved through the external step and labelled external, so placeBytes copied it into the content-addressed store and dropped its path/mtime — replacing the file never tripped the staleness check. Classify local references as file-backed (resolved in place, even on the request path) and keep store-backed behaviour only for http(s) URLs. --- core/artwork/playlist_cover.go | 14 -------- core/artwork/resolve.go | 60 +++++++++++++++++++--------------- core/artwork/resolve_test.go | 19 +++++++++++ 3 files changed, 52 insertions(+), 41 deletions(-) diff --git a/core/artwork/playlist_cover.go b/core/artwork/playlist_cover.go index 627a06b49..4373e2007 100644 --- a/core/artwork/playlist_cover.go +++ b/core/artwork/playlist_cover.go @@ -4,7 +4,6 @@ import ( "context" "image" "image/draw" - "io" "os" "path/filepath" "strings" @@ -16,19 +15,6 @@ import ( const tileSize = 600 -func fromLocalFile(path string) sourceFunc { - return func() (io.ReadCloser, string, error) { - if path == "" { - return nil, "", nil - } - f, err := os.Open(path) - if err != nil { - return nil, "", err - } - return f, path, nil - } -} - // findPlaylistSidecarPath scans the directory of the playlist file for a sidecar // image file with the same base name (case-insensitive). Returns empty string if // no matching image is found or if plsPath is empty. diff --git a/core/artwork/resolve.go b/core/artwork/resolve.go index 5caab5332..862d0bee7 100644 --- a/core/artwork/resolve.go +++ b/core/artwork/resolve.go @@ -201,15 +201,26 @@ func resolvePlaylist(ctx context.Context, ds model.DataStore, ag *agents.Agents, if res, ok := resolveLocalFile(findPlaylistSidecarPath(ctx, pl.Path), "folder"); ok { return res, nil } + // A local ExternalImageURL is a file-backed reference: serve it in place (staleness-checked, + // and available even on the request path). Only http(s) URLs need the gated remote fetch. + localImg, remoteImg := classifyPlaylistImage(pl.ExternalImageURL) + if localImg != "" { + if res, ok := resolveLocalFile(localImg, "folder"); ok { + return res, nil + } + } if localOnly { - // The ExternalImageURL step and the 2x2 grid are worker-only; a request must + // 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 res, ok, isErr := resolveExternalStep(gate, "m3u", fromPlaylistExternalSource(ctx, *pl)); ok { - return res, nil - } else if isErr { - extErr = true + if remoteImg != nil && conf.Server.EnableM3UExternalAlbumArt { + sf := func() (io.ReadCloser, string, error) { return fetchPlaylistImageURL(ctx, remoteImg) } + if res, ok, isErr := resolveExternalStep(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()"}) @@ -303,28 +314,23 @@ func resolveExternalStep(gate gateFunc, name string, sf sourceFunc) (res resolut 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 fetchPlaylistImageURL(ctx, parsed) - } - // A missing/unreadable local file is a definitive miss, not a transient - // failure to retry: swallow the open error and fall through to the grid. - r, path, _ := fromLocalFile(imgURL)() - return r, path, nil +// classifyPlaylistImage splits a playlist ExternalImageURL into a local filesystem path +// (served file-backed) or a remote http(s) URL (fetched and stored); at most one is set. +func classifyPlaylistImage(imageURL string) (localPath string, remote *url.URL) { + if imageURL == "" { + return "", nil + } + u, err := url.Parse(imageURL) + if err != nil { + return imageURL, nil // unparseable → treat as a local path + } + switch u.Scheme { + case "http", "https": + return "", u + case "file": + return u.Path, nil + default: + return imageURL, nil } } diff --git a/core/artwork/resolve_test.go b/core/artwork/resolve_test.go index 8309a6cc3..63637240f 100644 --- a/core/artwork/resolve_test.go +++ b/core/artwork/resolve_test.go @@ -432,6 +432,25 @@ var _ = Describe("resolveItem", func() { Expect(filepath.ToSlash(res.sourcePath)).To(HaveSuffix("list.jpg")) }) + It("serves a local ExternalImageURL as a file-backed reference (staleness-checked)", func() { + dir := GinkgoT().TempDir() + imgPath := filepath.Join(dir, "cover.png") + Expect(os.WriteFile(imgPath, []byte("local external image"), 0600)).To(Succeed()) + + plRepo := tests.CreateMockPlaylistRepo() + plRepo.SetData(model.Playlists{{ID: "pll", Name: "Playlist", ExternalImageURL: imgPath}}) + plRepo.TracksRepo = &tests.MockPlaylistTrackRepo{AlbumIDs: []string{"t1"}} + ds.MockedPlaylist = plRepo + + res, err := resolveItem(ctx, ds, ag, ffm, model.ArtworkQueueItem{ItemKind: "pl", ItemID: "pll"}, nil) + Expect(err).ToNot(HaveOccurred()) + Expect(res.reader).ToNot(BeNil()) + defer res.reader.Close() + Expect(res.source).To(Equal("folder")) + Expect(res.sourcePath).To(Equal(imgPath)) + Expect(res.refMtime).To(BeNumerically(">", 0)) + }) + 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