diff --git a/core/artwork/e2e/resolution_harness_test.go b/core/artwork/e2e/resolution_harness_test.go index 6b90e7795..1a917cb2d 100644 --- a/core/artwork/e2e/resolution_harness_test.go +++ b/core/artwork/e2e/resolution_harness_test.go @@ -203,7 +203,7 @@ func expectAlbumFolderCover(al model.Album, suffix string) { Expect(serveBytes(al.CoverArtID())).To(Equal(libFileBytes(suffix))) ia := acquire(model.KindAlbumArtwork, al.ID) Expect(ia.Source).To(Equal("folder")) - Expect(ia.SourcePath).To(HaveSuffix(suffix)) + Expect(filepath.ToSlash(ia.SourcePath)).To(HaveSuffix(suffix)) } // requireNoStateRow guards the byte-level folder assertions: a drain resolves every ready queue @@ -232,7 +232,7 @@ func expectArtistFolder(ar model.Artist, suffix string) { Expect(serveBytes(model.NewArtworkID(model.KindArtistArtwork, ar.ID, nil))).To(Equal(libFileBytes(suffix))) ia := acquire(model.KindArtistArtwork, ar.ID) Expect(ia.Source).To(Equal("folder")) - Expect(ia.SourcePath).To(HaveSuffix(suffix)) + Expect(filepath.ToSlash(ia.SourcePath)).To(HaveSuffix(suffix)) } // writeUploadedImage drops raw bytes into the per-entity upload folder under DataFolder, matching diff --git a/core/artwork/folders_artist.go b/core/artwork/folders_artist.go index b75cddde6..b858a0db5 100644 --- a/core/artwork/folders_artist.go +++ b/core/artwork/folders_artist.go @@ -2,6 +2,7 @@ package artwork import ( "context" + "errors" "fmt" "io" "io/fs" @@ -38,17 +39,24 @@ func fromArtistFolder(ctx context.Context, libFS fs.FS, libPath, artistFolder, p // return backslash separators on Windows. rel = filepath.ToSlash(rel) current := artistFolder + var unreadable error for range maxArtistFolderTraversalDepth { reader, hit, err := findImageInFolder(ctx, libFS, rel, current, pattern) if err == nil { return reader, hit, nil } + if errors.Is(err, errSourceUnreadable) { + unreadable = err + } if rel == "." { break // reached library root; don't traverse above it } rel = path.Dir(rel) current = filepath.Dir(current) } + if unreadable != nil { + return nil, "", unreadable + } return nil, "", fmt.Errorf(`no matches for '%s' in '%s' or its parent directories (within library)`, pattern, artistFolder) } } @@ -82,15 +90,20 @@ func findImageInFolder(ctx context.Context, libFS fs.FS, relFolder, absFolder, p // suffixes (e.g., artist.jpg before artist.1.jpg) slices.SortFunc(imagePaths, compareImageFiles) + var openErr error for _, p := range imagePaths { f, err := libFS.Open(p) if err != nil { log.Warn(ctx, "Could not open cover art file", "file", p, err) + openErr = fmt.Errorf("%w: %s: %w", errSourceUnreadable, p, err) continue } _, name := path.Split(p) return f, filepath.Join(absFolder, name), nil } + if openErr != nil { + return nil, "", openErr + } return nil, "", fmt.Errorf(`no matches for '%s' in '%s'`, pattern, absFolder) } diff --git a/core/artwork/folders_artist_test.go b/core/artwork/folders_artist_test.go new file mode 100644 index 000000000..e27b591b2 --- /dev/null +++ b/core/artwork/folders_artist_test.go @@ -0,0 +1,48 @@ +package artwork + +import ( + "context" + "errors" + "os" + "path/filepath" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("findImageInFolder", func() { + var ctx context.Context + var dir string + + BeforeEach(func() { + ctx = context.Background() + dir = GinkgoT().TempDir() + }) + + It("returns the first matching image", func() { + Expect(os.WriteFile(filepath.Join(dir, "artist.jpg"), []byte("img"), 0o600)).To(Succeed()) + + r, hit, err := findImageInFolder(ctx, os.DirFS(dir), ".", dir, "artist.*") + Expect(err).ToNot(HaveOccurred()) + defer r.Close() + Expect(hit).To(HaveSuffix("artist.jpg")) + }) + + // The glob matched, so the image exists; failing to open it says nothing about whether the + // artist has one, and must not let the resolver settle on absent. + It("reports a matched but unreadable image as unreadable, not as a miss", func() { + img := filepath.Join(dir, "artist.jpg") + Expect(os.WriteFile(img, []byte("img"), 0o600)).To(Succeed()) + Expect(os.Chmod(img, 0o000)).To(Succeed()) + DeferCleanup(func() { _ = os.Chmod(img, 0o600) }) + + _, _, err := findImageInFolder(ctx, os.DirFS(dir), ".", dir, "artist.*") + Expect(err).To(MatchError(errSourceUnreadable)) + }) + + It("reports a plain miss when nothing matches", func() { + _, _, err := findImageInFolder(ctx, os.DirFS(dir), ".", dir, "artist.*") + Expect(err).To(HaveOccurred()) + Expect(errors.Is(err, errSourceUnreadable)).To(BeFalse(), "no match is definitive, not transient") + }) +}) diff --git a/core/artwork/resolve.go b/core/artwork/resolve.go index 0e15c1dad..af165c8ac 100644 --- a/core/artwork/resolve.go +++ b/core/artwork/resolve.go @@ -192,10 +192,12 @@ func resolveArtist(ctx context.Context, ds model.DataStore, ag *agents.Agents, f if lib.FS == nil || artistFolder == "" { continue } - if res, ok := resolveArtistFolderPattern(ctx, lib, artistFolder, pattern); ok { + res, ok := resolveArtistFolderPattern(ctx, lib, artistFolder, pattern) + if ok { res.extError = extErr return res, nil } + localErr = localErr || res.localError } } return resolution{extError: extErr, localError: localErr}, nil @@ -393,9 +395,9 @@ func resolveArtistImageFolder(ar *model.Artist) (resolution, bool) { } func resolveArtistFolderPattern(ctx context.Context, lib libraryView, artistFolder, pattern string) (resolution, bool) { - r, path, _ := fromArtistFolder(ctx, lib.FS, lib.absRoot, artistFolder, pattern)() + r, path, err := fromArtistFolder(ctx, lib.FS, lib.absRoot, artistFolder, pattern)() if r == nil { - return resolution{}, false + return resolution{localError: errors.Is(err, errSourceUnreadable)}, false } return resolution{reader: r, source: "folder", sourcePath: path, refMtime: mtimeOf(path)}, true }