From fb7fd21984d4895eef2fab820652b8524db1cf8c Mon Sep 17 00:00:00 2001
From: Deluan <deluan@navidrome.org>
Date: Wed, 24 Jan 2024 19:19:06 -0500
Subject: [PATCH] Don't add empty TIPL roles

---
 scanner/metadata/taglib/taglib.go      |  2 +-
 scanner/metadata/taglib/taglib_test.go | 23 +++++++++++++++--------
 2 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/scanner/metadata/taglib/taglib.go b/scanner/metadata/taglib/taglib.go
index 3b3f4104..d29fb286 100644
--- a/scanner/metadata/taglib/taglib.go
+++ b/scanner/metadata/taglib/taglib.go
@@ -78,7 +78,7 @@ func parseTIPL(tags metadata.ParsedTags) {
 	}
 
 	addRole := func(tags metadata.ParsedTags, currentRole string, currentValue []string) {
-		if currentRole != "" {
+		if currentRole != "" && len(currentValue) > 0 {
 			role := tiplMapping[currentRole]
 			tags[role] = append(tags[currentRole], strings.Join(currentValue, " "))
 		}
diff --git a/scanner/metadata/taglib/taglib_test.go b/scanner/metadata/taglib/taglib_test.go
index 9a21f0be..56c1e5f2 100644
--- a/scanner/metadata/taglib/taglib_test.go
+++ b/scanner/metadata/taglib/taglib_test.go
@@ -24,7 +24,8 @@ var _ = Describe("Extractor", func() {
 			Expect(err).ToNot(HaveOccurred())
 		})
 	})
-	Context("Parse", func() {
+
+	Describe("Parse", func() {
 		It("correctly parses metadata from all files in folder", func() {
 			mds, err := e.Parse(
 				"tests/fixtures/test.mp3",
@@ -90,6 +91,7 @@ var _ = Describe("Extractor", func() {
 			Expect(m).To(HaveKey("bitrate"))
 			Expect(m["bitrate"][0]).To(BeElementOf("18", "39", "40", "43", "49"))
 		})
+
 		DescribeTable("Format-Specific tests",
 			func(file, duration, channels, albumGain, albumPeak, trackGain, trackPeak string, id3Lyrics bool) {
 				file = "tests/fixtures/" + file
@@ -178,7 +180,7 @@ var _ = Describe("Extractor", func() {
 		)
 	})
 
-	Context("Error Checking", func() {
+	Describe("Error Checking", func() {
 		It("correctly handle unreadable file due to insufficient read permission", func() {
 			_, err := e.extractMetadata(accessForbiddenFile)
 			Expect(err).To(MatchError(os.ErrPermission))
@@ -207,15 +209,22 @@ var _ = Describe("Extractor", func() {
 			It("correctly parses roles and names", func() {
 				tags["tipl"] = []string{"arranger Andrew Powell dj-mix François Kevorkian engineer Chris Blair"}
 				parseTIPL(tags)
-				Expect(tags["arranger"]).To(Equal([]string{"Andrew Powell"}))
-				Expect(tags["engineer"]).To(Equal([]string{"Chris Blair"}))
-				Expect(tags["djmixer"]).To(Equal([]string{"François Kevorkian"}))
+				Expect(tags["arranger"]).To(ConsistOf("Andrew Powell"))
+				Expect(tags["engineer"]).To(ConsistOf("Chris Blair"))
+				Expect(tags["djmixer"]).To(ConsistOf("François Kevorkian"))
 			})
 
 			It("handles multiple names for a single role", func() {
 				tags["tipl"] = []string{"engineer Pat Stapley producer Eric Woolfson engineer Chris Blair"}
 				parseTIPL(tags)
-				Expect(tags["producer"]).To(Equal([]string{"Eric Woolfson"}))
+				Expect(tags["producer"]).To(ConsistOf("Eric Woolfson"))
+				Expect(tags["engineer"]).To(ConsistOf("Pat Stapley", "Chris Blair"))
+			})
+
+			It("discards roles without names", func() {
+				tags["tipl"] = []string{"engineer Pat Stapley producer engineer Chris Blair"}
+				parseTIPL(tags)
+				Expect(tags).ToNot(HaveKey("producer"))
 				Expect(tags["engineer"]).To(ConsistOf("Pat Stapley", "Chris Blair"))
 			})
 		})
@@ -234,8 +243,6 @@ var _ = Describe("Extractor", func() {
 				Expect(tags).To(BeEmpty())
 			})
 		})
-
-		// Add any additional edge cases if necessary
 	})
 
 })