From 11e461a57cc4a8db93bbc8b4723854b52e538e0d Mon Sep 17 00:00:00 2001 From: Deluan Date: Sat, 23 May 2026 19:09:33 -0300 Subject: [PATCH] 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. --- model/metadata/map_participants_test.go | 20 ++++++++ model/metadata/metadata_test.go | 15 ++++++ model/tag_mappings_test.go | 64 +++++++++++++++++++++++++ 3 files changed, 99 insertions(+) create mode 100644 model/tag_mappings_test.go diff --git a/model/metadata/map_participants_test.go b/model/metadata/map_participants_test.go index 71cb9c1f2..5ee802ced 100644 --- a/model/metadata/map_participants_test.go +++ b/model/metadata/map_participants_test.go @@ -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() { diff --git a/model/metadata/metadata_test.go b/model/metadata/metadata_test.go index c9acdbce9..350731b89 100644 --- a/model/metadata/metadata_test.go +++ b/model/metadata/metadata_test.go @@ -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", diff --git a/model/tag_mappings_test.go b/model/tag_mappings_test.go new file mode 100644 index 000000000..1665d557b --- /dev/null +++ b/model/tag_mappings_test.go @@ -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"})) + }) + }) +})