fix(artwork): retry remaining fallbacks when the first one fails to open

Review feedback: the previous shape remembered only the first unnumbered
candidate and fell through to a generic error if os.Open failed on it,
even though other matching unnumbered files in imgFiles could have
succeeded. The pre-PR code was more resilient because it looped and
continued on open failure.

fromExternalFile now collects every viable unnumbered candidate into a
slice during the scan, then tries them in order after the loop, mirroring
the pre-PR retry-on-open-failure behavior. Numbered matches still return
immediately on first success and skip the candidate list entirely — an
open failure on a numbered match means no other file has that number
anyway.

Also:
- globMetaChars doc comment now notes that '\' escape is intentionally
  excluded (filepath.Match supports it but treating it as a metachar here
  would misalign extractDiscNumber's literal-prefix extraction with no
  benefit for realistic config patterns).
- The 'cover.jpg doesn't match disc*.*' Entry in the extractDiscNumber
  table is renamed to 'cover.jpg with disc*.* (no prefix match)' to
  reflect that the test now exercises the HasPrefix defensive guard,
  not the removed internal filepath.Match check.

Regression test added: a single-folder album with a deleted first
candidate file resolves to the second candidate.
This commit is contained in:
Deluan
2026-04-11 20:56:36 -04:00
parent 3fd106d675
commit 5d79f751c4
2 changed files with 33 additions and 13 deletions

View File

@@ -168,7 +168,11 @@ func (d *discArtworkReader) fromDiscSubtitle(ctx context.Context, subtitle strin
}
}
// globMetaChars lists every metacharacter understood by filepath.Match.
// globMetaChars holds the substitution metacharacters understood by
// filepath.Match. The '\' escape character is intentionally excluded:
// disc art patterns come from user config and never include escaped
// metachars in practice, and treating '\' as a metachar would misalign
// the literal-prefix extraction in extractDiscNumber.
const globMetaChars = "*?["
// extractDiscNumber parses the disc number from a filename matched by a
@@ -208,7 +212,7 @@ func extractDiscNumber(pattern, filename string) (int, bool) {
func (d *discArtworkReader) fromExternalFile(ctx context.Context, pattern string) sourceFunc {
isLiteral := !strings.ContainsAny(pattern, globMetaChars)
return func() (io.ReadCloser, string, error) {
var fallback string
var fallbacks []string
for _, file := range d.imgFiles {
_, name := filepath.Split(file)
name = strings.ToLower(name)
@@ -235,13 +239,10 @@ func (d *discArtworkReader) fromExternalFile(ctx context.Context, pattern string
}
}
if fallback != "" {
continue
}
if d.isMultiFolder && !d.discFolders[filepath.Dir(file)] {
continue
}
fallback = file
fallbacks = append(fallbacks, file)
// A literal filename can only match one file, so stop as soon
// as we have a viable fallback.
if isLiteral {
@@ -249,13 +250,13 @@ func (d *discArtworkReader) fromExternalFile(ctx context.Context, pattern string
}
}
if fallback != "" {
f, err := os.Open(fallback)
for _, file := range fallbacks {
f, err := os.Open(file)
if err != nil {
log.Warn(ctx, "Could not open disc art file", "file", fallback, err)
} else {
return f, fallback, nil
log.Warn(ctx, "Could not open disc art file", "file", file, err)
continue
}
return f, file, nil
}
return nil, "", fmt.Errorf("disc %d: pattern '%s' not matched by files", d.discNumber, pattern)
}

View File

@@ -42,8 +42,8 @@ var _ = Describe("Disc Artwork Reader", func() {
// Case insensitive (filename already lowered by caller)
Entry("Disc1.jpg lowered", "disc*.*", "disc1.jpg", 1, true),
// Pattern doesn't match
Entry("cover.jpg doesn't match disc*.*", "disc*.*", "cover.jpg", 0, false),
// HasPrefix guard: filename doesn't share the pattern's literal prefix
Entry("cover.jpg with disc*.* (no prefix match)", "disc*.*", "cover.jpg", 0, false),
// Pattern with no wildcard before dot
Entry("front1.jpg with front*.*", "front*.*", "front1.jpg", 1, true),
@@ -207,6 +207,25 @@ var _ = Describe("Disc Artwork Reader", func() {
Entry("disc 3 falls back to disc.jpg when no numbered match exists", 3, 0),
)
It("tries the next fallback candidate when the first one cannot be opened", func() {
f1 := createFile("album/cover.jpg")
f2 := createFile("album/cover.png")
// Remove f1 so os.Open will fail on it; f2 should still win.
Expect(os.Remove(f1)).To(Succeed())
reader := &discArtworkReader{
discNumber: 1,
imgFiles: []string{f1, f2},
discFolders: map[string]bool{filepath.Join(tmpDir, "album"): true},
}
sf := reader.fromExternalFile(ctx, "cover.*")
r, path, err := sf()
Expect(err).ToNot(HaveOccurred())
Expect(r).ToNot(BeNil())
r.Close()
Expect(path).To(Equal(f2))
})
DescribeTable("filters by disc number for non-'*' wildcard patterns",
func(pattern string, discNumber, expectedIdx int) {
files := []string{