test: cover SplitTagValue with multi-frame regression cases

Add tests pinning the behavior fixed by SplitTagValue iterating over each
input value. The previous len(values) != 1 short-circuit silently skipped
splitting whenever TagLib merged multiple ID3v2 frames into the same
property (e.g. TMOO + TXXX:MOOD for mood, or duplicate TIPL entries for
composer), as reported in #5065.

Three layers of coverage:
- model/tag_mappings_test.go: direct unit tests on TagConf.SplitTagValue
  covering single/multi-value input, case-insensitive separators, missing
  SplitRx, empty input, and the empty-strings-passed-through contract that
  the downstream metadata pipeline relies on.
- model/metadata/metadata_test.go: end-to-end check that a "mood" tag
  surfaced as two raw values (the exact shape from the bug report) is
  split, trimmed, and deduplicated to the expected three moods.
- model/metadata/map_participants_test.go: parallel multi-value case for
  the COMPOSER tag, ensuring the same fix also corrects multi-frame role
  parsing.

All three new specs fail on the pre-fix code and pass on the patched
SplitTagValue.
This commit is contained in:
Deluan 2026-05-23 19:09:33 -03:00
parent 0f9e1736a9
commit 11e461a57c
3 changed files with 99 additions and 0 deletions

View File

@ -684,6 +684,26 @@ var _ = Describe("Participants", func() {
Expect(composers[2].Name).To(Equal("The Album Artist"))
})
})
// Sibling fix to https://github.com/navidrome/navidrome/issues/5065: when
// multiple frames map to the same role tag (e.g. TIPL producer entries),
// the configured split separator must still apply to each value.
When("the tag has multiple values", func() {
It("should split each value individually", func() {
mf = toMediaFile(model.RawTags{
"COMPOSER": {"John Doe/Jane Doe", "Someone Else"},
})
participants := mf.Participants
Expect(participants).To(HaveKeyWithValue(model.RoleComposer, HaveLen(3)))
composers := participants[model.RoleComposer]
Expect(composers).To(ConsistOf(
HaveField("Name", "John Doe"),
HaveField("Name", "Jane Doe"),
HaveField("Name", "Someone Else"),
))
})
})
})
Describe("MBID tags", func() {

View File

@ -129,6 +129,21 @@ var _ = Describe("Metadata", func() {
Expect(md.Strings(model.TagGenre)).To(Equal([]string{"Rock", "Pop", "Punk"}))
})
// Regression test for https://github.com/navidrome/navidrome/issues/5065
//
// MP3s with both an ID3v2 TMOO frame and a TXXX:MOOD frame are surfaced by
// TagLib's PropertyMap as a single "mood" key with multiple values. The split
// configuration must still apply to each value individually.
It("should split values from multiple frames mapping to the same tag", func() {
props.Tags = model.RawTags{
// Same shape as the bug report: two frames, comma-separated content.
"mood": {"Love, Emotional, Ballad", "Love; Emotional; Ballad"},
}
md = metadata.New(filePath, props)
Expect(md.Strings(model.TagMood)).To(ConsistOf("Love", "Emotional", "Ballad"))
})
})
DescribeTable("Date",

View File

@ -0,0 +1,64 @@
package model
import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
var _ = Describe("TagConf", func() {
Describe("SplitTagValue", func() {
var conf TagConf
BeforeEach(func() {
conf = TagConf{Split: []string{";", "/", ","}}
conf.SplitRx = compileSplitRegex("test", conf.Split)
})
It("splits a single value on configured separators", func() {
Expect(conf.SplitTagValue([]string{"Rock/Pop;Punk"})).To(Equal([]string{"Rock", "Pop", "Punk"}))
})
It("trims whitespace around split values", func() {
Expect(conf.SplitTagValue([]string{"Love, Emotional, Ballad"})).To(Equal([]string{"Love", "Emotional", "Ballad"}))
})
// Regression test for https://github.com/navidrome/navidrome/issues/5065
//
// When multiple ID3v2 frames map to the same logical tag (e.g. TMOO + TXXX:MOOD),
// TagLib's PropertyMap merges them into a slice with several entries. Previously
// SplitTagValue had a `len(values) != 1` guard that skipped splitting in this case.
It("splits each value individually when given multiple inputs", func() {
input := []string{"Love, Emotional, Ballad", "Love; Emotional; Ballad"}
Expect(conf.SplitTagValue(input)).To(Equal([]string{
"Love", "Emotional", "Ballad",
"Love", "Emotional", "Ballad",
}))
})
It("matches separators case-insensitively when the split pattern allows", func() {
c := TagConf{Split: []string{" AND "}}
c.SplitRx = compileSplitRegex("test", c.Split)
Expect(c.SplitTagValue([]string{"foo and bar AND baz"})).To(Equal([]string{"foo", "bar", "baz"}))
})
It("returns values unchanged when no separators are configured", func() {
c := TagConf{}
Expect(c.SplitTagValue([]string{"Foo, Bar"})).To(Equal([]string{"Foo, Bar"}))
Expect(c.SplitTagValue([]string{"a", "b"})).To(Equal([]string{"a", "b"}))
})
It("returns an empty slice for empty input", func() {
Expect(conf.SplitTagValue([]string{})).To(BeEmpty())
})
It("handles a value with no separator as a single-element result", func() {
Expect(conf.SplitTagValue([]string{"JustOneMood"})).To(Equal([]string{"JustOneMood"}))
})
It("produces empty strings when separators are adjacent (dedup happens downstream)", func() {
// SplitTagValue itself does not filter empties; that is the job of
// filterDuplicatedOrEmptyValues in the metadata pipeline.
Expect(conf.SplitTagValue([]string{"Rock//Pop"})).To(Equal([]string{"Rock", "", "Pop"}))
})
})
})