diff --git a/model/participants.go b/model/participants.go index d8d4eab62..e4dcc8c76 100644 --- a/model/participants.go +++ b/model/participants.go @@ -156,16 +156,25 @@ func (p Participants) Merge(other Participants) { } func (p Participants) add(role Role, participants ...Participant) { - seen := make(map[string]struct{}, len(p[role])) - for _, artist := range p[role] { - seen[artist.ID+artist.SubRole] = struct{}{} + seen := make(map[string]int, len(p[role])) + for i, artist := range p[role] { + seen[artist.ID+artist.SubRole] = i } for _, participant := range participants { key := participant.ID + participant.SubRole - if _, ok := seen[key]; !ok { - seen[key] = struct{}{} - p[role] = append(p[role], participant) + if idx, ok := seen[key]; ok { + // Same artist/sub-role seen before. The merge (e.g. building + // album.Participants from per-track participants) is inherently + // lossy when tracks differ on CreditedAs, but silently dropping + // the later value can mean the wrong credit ends up on the album. + // Prefer the most recently observed non-empty CreditedAs. + if participant.CreditedAs != "" { + p[role][idx].CreditedAs = participant.CreditedAs + } + continue } + seen[key] = len(p[role]) + p[role] = append(p[role], participant) } } diff --git a/model/participants_test.go b/model/participants_test.go index 3e7ba2942..29e87b550 100644 --- a/model/participants_test.go +++ b/model/participants_test.go @@ -116,6 +116,40 @@ var _ = Describe("Participants", func() { RoleAlbumArtist: []Participant{_p("3", "AlbumArtist1"), _p("4", "AlbumArtist2"), _p("7", "AlbumArtist3"), _p("8", "AlbumArtist4")}, })) }) + + It("upgrades CreditedAs from a later occurrence when the dedup key matches", func() { + // Same artist (same ID+SubRole) credited differently on two tracks. + // The merge is inherently lossy; the later non-empty credit wins so + // it isn't silently dropped just because of arrival order. + p1 := Participants{ + RoleArtist: []Participant{ + {Artist: Artist{ID: "a1", Name: "Aphex Twin"}, CreditedAs: "AFX"}, + }, + } + p2 := Participants{ + RoleArtist: []Participant{ + {Artist: Artist{ID: "a1", Name: "Aphex Twin"}, CreditedAs: "Aphex Twin"}, + }, + } + p1.Merge(p2) + Expect(p1[RoleArtist]).To(HaveLen(1)) + Expect(p1[RoleArtist][0].CreditedAs).To(Equal("Aphex Twin")) + }) + + It("does not overwrite an existing CreditedAs with an empty one", func() { + p1 := Participants{ + RoleArtist: []Participant{ + {Artist: Artist{ID: "a1", Name: "Aphex Twin"}, CreditedAs: "AFX"}, + }, + } + p2 := Participants{ + RoleArtist: []Participant{ + {Artist: Artist{ID: "a1", Name: "Aphex Twin"}}, // empty CreditedAs + }, + } + p1.Merge(p2) + Expect(p1[RoleArtist][0].CreditedAs).To(Equal("AFX")) + }) }) Describe("Hash", func() {