diff --git a/adapters/lastfm/agent.go b/adapters/lastfm/agent.go index 02c198120..eb8f3d36e 100644 --- a/adapters/lastfm/agent.go +++ b/adapters/lastfm/agent.go @@ -231,10 +231,9 @@ func (l *lastfmAgent) GetSimilarSongsByTrack(ctx context.Context, id, name, arti res := make([]agents.Song, 0, len(resp)) for _, t := range resp { res = append(res, agents.Song{ - Name: t.Name, - MBID: t.MBID, - Artist: t.Artist.Name, - ArtistMBID: t.Artist.MBID, + Name: t.Name, + MBID: t.MBID, + Artists: []agents.Artist{{Name: t.Artist.Name, MBID: t.Artist.MBID}}, }) } return res, nil diff --git a/adapters/lastfm/agent_test.go b/adapters/lastfm/agent_test.go index 94788b8bd..7e4e29294 100644 --- a/adapters/lastfm/agent_test.go +++ b/adapters/lastfm/agent_test.go @@ -309,11 +309,11 @@ var _ = Describe("lastfmAgent", func() { f, _ := os.Open("tests/fixtures/lastfm.track.getsimilar.json") httpClient.Res = http.Response{Body: f, StatusCode: 200} Expect(agent.GetSimilarSongsByTrack(ctx, "123", "Just Can't Get Enough", "Depeche Mode", "", 5)).To(Equal([]agents.Song{ - {Name: "Dreaming of Me", MBID: "027b553e-7c74-3ed4-a95e-1d4fea51f174", Artist: "Depeche Mode", ArtistMBID: "8538e728-ca0b-4321-b7e5-cff6565dd4c0"}, - {Name: "Everything Counts", MBID: "5a5a3ca4-bdb8-4641-a674-9b54b9b319a6", Artist: "Depeche Mode", ArtistMBID: "8538e728-ca0b-4321-b7e5-cff6565dd4c0"}, - {Name: "Don't You Want Me", MBID: "", Artist: "The Human League", ArtistMBID: "7adaabfb-acfb-47bc-8c7c-59471c2f0db8"}, - {Name: "Tainted Love", MBID: "", Artist: "Soft Cell", ArtistMBID: "7fb50287-029d-47cc-825a-235ca28024b2"}, - {Name: "Blue Monday", MBID: "727e84c6-1b56-31dd-a958-a5f46305cec0", Artist: "New Order", ArtistMBID: "f1106b17-dcbb-45f6-b938-199ccfab50cc"}, + {Name: "Dreaming of Me", MBID: "027b553e-7c74-3ed4-a95e-1d4fea51f174", Artists: []agents.Artist{{Name: "Depeche Mode", MBID: "8538e728-ca0b-4321-b7e5-cff6565dd4c0"}}}, + {Name: "Everything Counts", MBID: "5a5a3ca4-bdb8-4641-a674-9b54b9b319a6", Artists: []agents.Artist{{Name: "Depeche Mode", MBID: "8538e728-ca0b-4321-b7e5-cff6565dd4c0"}}}, + {Name: "Don't You Want Me", MBID: "", Artists: []agents.Artist{{Name: "The Human League", MBID: "7adaabfb-acfb-47bc-8c7c-59471c2f0db8"}}}, + {Name: "Tainted Love", MBID: "", Artists: []agents.Artist{{Name: "Soft Cell", MBID: "7fb50287-029d-47cc-825a-235ca28024b2"}}}, + {Name: "Blue Monday", MBID: "727e84c6-1b56-31dd-a958-a5f46305cec0", Artists: []agents.Artist{{Name: "New Order", MBID: "f1106b17-dcbb-45f6-b938-199ccfab50cc"}}}, })) Expect(httpClient.RequestCount).To(Equal(1)) Expect(httpClient.SavedRequest.URL.Query().Get("track")).To(Equal("Just Can't Get Enough")) diff --git a/adapters/listenbrainz/agent.go b/adapters/listenbrainz/agent.go index 826a9672e..76beed921 100644 --- a/adapters/listenbrainz/agent.go +++ b/adapters/listenbrainz/agent.go @@ -141,24 +141,37 @@ func (l *listenBrainzAgent) GetArtistTopSongs(ctx context.Context, id, artistNam res := make([]agents.Song, len(resp)) for i, t := range resp { - mbid := "" - if len(t.ArtistMBIDs) > 0 { - mbid = t.ArtistMBIDs[0] - } - res[i] = agents.Song{ - Album: t.ReleaseName, - AlbumMBID: t.ReleaseMBID, - Artist: t.ArtistName, - ArtistMBID: mbid, - Duration: t.DurationMs, - Name: t.RecordingName, - MBID: t.RecordingMbid, + Album: t.ReleaseName, + AlbumMBID: t.ReleaseMBID, + Artists: topSongArtists(t.ArtistName, t.ArtistMBIDs), + Duration: t.DurationMs, + Name: t.RecordingName, + MBID: t.RecordingMbid, } } return res, nil } +// topSongArtists maps the top-recordings response, which carries a single combined display name +// (e.g. "X feat. Y") plus a per-artist MBID list, onto agents.Artist. Names and MBIDs are not +// positionally pairable, so the display name attaches to the first credit and any further MBIDs +// become MBID-only collaborators — still valid identity signals for the matcher. +func topSongArtists(name string, mbids []string) []agents.Artist { + if len(mbids) == 0 { + if name == "" { + return nil + } + return []agents.Artist{{Name: name}} + } + artists := make([]agents.Artist, len(mbids)) + artists[0] = agents.Artist{Name: name, MBID: mbids[0]} + for i, m := range mbids[1:] { + artists[i+1] = agents.Artist{MBID: m} + } + return artists +} + func (l *listenBrainzAgent) GetSimilarArtists(ctx context.Context, id string, name string, mbid string, limit int) ([]agents.Artist, error) { if mbid == "" { return nil, agents.ErrNotFound @@ -203,7 +216,7 @@ func (l *listenBrainzAgent) GetSimilarSongsByTrack(ctx context.Context, id strin songs[i] = agents.Song{ Album: song.ReleaseName, AlbumMBID: song.ReleaseMBID, - Artist: song.Artist, + Artists: []agents.Artist{{Name: song.Artist}}, MBID: song.MBID, Name: song.Name, } diff --git a/adapters/listenbrainz/agent_test.go b/adapters/listenbrainz/agent_test.go index df70ec9c4..2c4668296 100644 --- a/adapters/listenbrainz/agent_test.go +++ b/adapters/listenbrainz/agent_test.go @@ -249,24 +249,22 @@ var _ = Describe("listenBrainzAgent", func() { Expect(err).ToNot(HaveOccurred()) Expect(data).To(Equal([]agents.Song{ { - ID: "", - Name: "world.execute(me);", - MBID: "9980309d-3480-4e7e-89ce-fce971a452be", - Artist: "Mili", - ArtistMBID: "d2a92ee2-27ce-4e71-bfc5-12e34fe8ef56", - Album: "Miracle Milk", - AlbumMBID: "38a8f6e1-0e34-4418-a89d-78240a367408", - Duration: 211912, + ID: "", + Name: "world.execute(me);", + MBID: "9980309d-3480-4e7e-89ce-fce971a452be", + Artists: []agents.Artist{{Name: "Mili", MBID: "d2a92ee2-27ce-4e71-bfc5-12e34fe8ef56"}}, + Album: "Miracle Milk", + AlbumMBID: "38a8f6e1-0e34-4418-a89d-78240a367408", + Duration: 211912, }, { - ID: "", - Name: "String Theocracy", - MBID: "afa2c83d-b17f-4029-b9da-790ea9250cf9", - Artist: "Mili", - ArtistMBID: "d2a92ee2-27ce-4e71-bfc5-12e34fe8ef56", - Album: "String Theocracy", - AlbumMBID: "d79a38e3-7016-4f39-a31a-f495ce914b8e", - Duration: 174000, + ID: "", + Name: "String Theocracy", + MBID: "afa2c83d-b17f-4029-b9da-790ea9250cf9", + Artists: []agents.Artist{{Name: "Mili", MBID: "d2a92ee2-27ce-4e71-bfc5-12e34fe8ef56"}}, + Album: "String Theocracy", + AlbumMBID: "d79a38e3-7016-4f39-a31a-f495ce914b8e", + Duration: 174000, }, })) }) @@ -278,17 +276,45 @@ var _ = Describe("listenBrainzAgent", func() { Expect(err).ToNot(HaveOccurred()) Expect(data).To(Equal([]agents.Song{ { - ID: "", - Name: "world.execute(me);", - MBID: "9980309d-3480-4e7e-89ce-fce971a452be", - Artist: "Mili", - ArtistMBID: "d2a92ee2-27ce-4e71-bfc5-12e34fe8ef56", - Album: "Miracle Milk", - AlbumMBID: "38a8f6e1-0e34-4418-a89d-78240a367408", - Duration: 211912, + ID: "", + Name: "world.execute(me);", + MBID: "9980309d-3480-4e7e-89ce-fce971a452be", + Artists: []agents.Artist{{Name: "Mili", MBID: "d2a92ee2-27ce-4e71-bfc5-12e34fe8ef56"}}, + Album: "Miracle Milk", + AlbumMBID: "38a8f6e1-0e34-4418-a89d-78240a367408", + Duration: 211912, }, })) }) + + It("maps a multi-artist top song to one named artist plus MBID-only collaborators", func() { + body := `[{ + "recording_name": "Collab", + "recording_mbid": "rec-1", + "artist_name": "Drake feat. Future", + "artist_mbids": ["mbid-drake", "mbid-future"], + "release_name": "Album", + "release_mbid": "rel-1", + "length": 200000 + }]` + httpClient.Res = http.Response{Body: io.NopCloser(bytes.NewBufferString(body)), StatusCode: 200} + data, err := agent.GetArtistTopSongs(ctx, "", "", "mbid-drake", 1) + Expect(err).ToNot(HaveOccurred()) + Expect(data).To(HaveLen(1)) + Expect(data[0].Artists).To(Equal([]agents.Artist{ + {Name: "Drake feat. Future", MBID: "mbid-drake"}, + {MBID: "mbid-future"}, + })) + }) + + It("leaves Artists nil when the top song carries no name or MBIDs", func() { + body := `[{"recording_name": "Anon", "recording_mbid": "rec-1", "artist_name": "", "artist_mbids": []}]` + httpClient.Res = http.Response{Body: io.NopCloser(bytes.NewBufferString(body)), StatusCode: 200} + data, err := agent.GetArtistTopSongs(ctx, "", "", "x", 1) + Expect(err).ToNot(HaveOccurred()) + Expect(data).To(HaveLen(1)) + Expect(data[0].Artists).To(BeNil()) + }) }) Describe("GetSimilarArtists", func() { @@ -393,26 +419,24 @@ var _ = Describe("listenBrainzAgent", func() { Expect(httpClient.SavedRequest.URL.String()).To(Equal(baseUrl + mbid)) Expect(resp).To(Equal([]agents.Song{ { - ID: "", - Name: "Take On Me", - MBID: "12f65dca-de8f-43fe-a65d-f12a02aaadf3", - ISRC: "", - Artist: "a‐ha", - ArtistMBID: "", - Album: "Hunting High and Low", - AlbumMBID: "4ec07fe8-e7c6-3106-a0aa-fdf92f13f7fc", - Duration: 0, + ID: "", + Name: "Take On Me", + MBID: "12f65dca-de8f-43fe-a65d-f12a02aaadf3", + ISRC: "", + Artists: []agents.Artist{{Name: "a‐ha"}}, + Album: "Hunting High and Low", + AlbumMBID: "4ec07fe8-e7c6-3106-a0aa-fdf92f13f7fc", + Duration: 0, }, { - ID: "", - Name: "Wake Me Up Before You Go‐Go", - MBID: "80033c72-aa19-4ba8-9227-afb075fec46e", - ISRC: "", - Artist: "Wham!", - ArtistMBID: "", - Album: "Make It Big", - AlbumMBID: "c143d542-48dc-446b-b523-1762da721638", - Duration: 0, + ID: "", + Name: "Wake Me Up Before You Go‐Go", + MBID: "80033c72-aa19-4ba8-9227-afb075fec46e", + ISRC: "", + Artists: []agents.Artist{{Name: "Wham!"}}, + Album: "Make It Big", + AlbumMBID: "c143d542-48dc-446b-b523-1762da721638", + Duration: 0, }, })) }) @@ -427,15 +451,14 @@ var _ = Describe("listenBrainzAgent", func() { Expect(httpClient.SavedRequest.URL.String()).To(Equal(baseUrl + mbid)) Expect(resp).To(Equal([]agents.Song{ { - ID: "", - Name: "Take On Me", - MBID: "12f65dca-de8f-43fe-a65d-f12a02aaadf3", - ISRC: "", - Artist: "a‐ha", - ArtistMBID: "", - Album: "Hunting High and Low", - AlbumMBID: "4ec07fe8-e7c6-3106-a0aa-fdf92f13f7fc", - Duration: 0, + ID: "", + Name: "Take On Me", + MBID: "12f65dca-de8f-43fe-a65d-f12a02aaadf3", + ISRC: "", + Artists: []agents.Artist{{Name: "a‐ha"}}, + Album: "Hunting High and Low", + AlbumMBID: "4ec07fe8-e7c6-3106-a0aa-fdf92f13f7fc", + Duration: 0, }, })) }) diff --git a/core/agents/interfaces.go b/core/agents/interfaces.go index 1fe8c2a23..d5f4a6580 100644 --- a/core/agents/interfaces.go +++ b/core/agents/interfaces.go @@ -34,16 +34,14 @@ type ExternalImage struct { } type Song struct { - ID string - Name string - MBID string - ISRC string - Artist string - ArtistMBID string - Artists []Artist // optional full artist list; ArtistList normalizes against Artist/ArtistMBID - Album string - AlbumMBID string - Duration uint32 // Duration in milliseconds, 0 means unknown + ID string + Name string + MBID string + ISRC string + Artists []Artist + Album string + AlbumMBID string + Duration uint32 // Duration in milliseconds, 0 means unknown } // Equals reports strict whole-value equality, used to dedup identical input songs. It hashes @@ -54,17 +52,6 @@ func (s Song) Equals(other Song) bool { return h1 == h2 } -// ArtistList normalizes the single/multi-artist representations so callers never branch on len(Artists). -func (s Song) ArtistList() []Artist { - if len(s.Artists) > 0 { - return s.Artists - } - if s.Artist != "" { - return []Artist{{Name: s.Artist, MBID: s.ArtistMBID}} - } - return nil -} - var ( ErrNotFound = errors.New("not found") ) diff --git a/core/agents/interfaces_test.go b/core/agents/interfaces_test.go index 46bd2f93b..c13710a38 100644 --- a/core/agents/interfaces_test.go +++ b/core/agents/interfaces_test.go @@ -6,7 +6,7 @@ import ( ) var _ = Describe("Song.Equals", func() { - base := Song{ID: "1", Name: "S", Artist: "A", Artists: []Artist{{ID: "x", Name: "A"}}} + base := Song{ID: "1", Name: "S", Artists: []Artist{{ID: "x", Name: "A"}}} It("true for identical songs incl Artists", func() { Expect(base.Equals(base)).To(BeTrue()) }) @@ -21,27 +21,7 @@ var _ = Describe("Song.Equals", func() { Expect(base.Equals(other)).To(BeFalse()) }) It("true when both have empty Artists and equal scalars", func() { - a := Song{ID: "1", Name: "S", Artist: "A"} + a := Song{ID: "1", Name: "S"} Expect(a.Equals(a)).To(BeTrue()) }) }) - -var _ = Describe("Song.ArtistList", func() { - It("returns the Artists slice when present", func() { - s := Song{Artist: "Primary", ArtistMBID: "mbid-primary", Artists: []Artist{ - {ID: "id-drake", Name: "Drake", MBID: "mbid-drake"}, - {Name: "Future", MBID: "mbid-future"}, - }} - Expect(s.ArtistList()).To(Equal([]Artist{ - {ID: "id-drake", Name: "Drake", MBID: "mbid-drake"}, - {Name: "Future", MBID: "mbid-future"}, - })) - }) - It("falls back to the single Artist field with empty ID", func() { - s := Song{Artist: "Drake", ArtistMBID: "mbid-drake"} - Expect(s.ArtistList()).To(Equal([]Artist{{Name: "Drake", MBID: "mbid-drake"}})) - }) - It("returns empty when no artist is set", func() { - Expect(Song{}.ArtistList()).To(BeEmpty()) - }) -}) diff --git a/core/external/provider.go b/core/external/provider.go index 74dab4972..459e8a205 100644 --- a/core/external/provider.go +++ b/core/external/provider.go @@ -471,13 +471,19 @@ func (e *provider) getMatchingTopSongs(ctx context.Context, agent agents.ArtistT return nil, fmt.Errorf("failed to get top songs for artist %s: %w", artistName, err) } - // Enrich songs with artist info if not already present (for top songs, we know the artist) + // Enrich top songs with the queried artist. A song with no artists, or whose first credit the + // agent left unnamed, is attributed to the queried artist. A first credit that already names an + // artist is left as-is: it may be a different (e.g. featured) artist, so stamping the queried + // MBID onto it would create a false name+MBID pairing. for i := range songs { - if songs[i].Artist == "" { - songs[i].Artist = artistName - } - if songs[i].ArtistMBID == "" { - songs[i].ArtistMBID = artist.MbzArtistID + switch { + case len(songs[i].Artists) == 0: + songs[i].Artists = []agents.Artist{{Name: artistName, MBID: artist.MbzArtistID}} + case songs[i].Artists[0].Name == "": + songs[i].Artists[0].Name = artistName + if songs[i].Artists[0].MBID == "" { + songs[i].Artists[0].MBID = artist.MbzArtistID + } } } diff --git a/core/external/provider_similarsongs_test.go b/core/external/provider_similarsongs_test.go index f109bf8b1..563003f83 100644 --- a/core/external/provider_similarsongs_test.go +++ b/core/external/provider_similarsongs_test.go @@ -73,7 +73,7 @@ var _ = Describe("Provider - SimilarSongs", func() { agentsCombined.On("GetSimilarSongsByTrack", mock.Anything, "track-1", "Just Can't Get Enough", "Depeche Mode", "track-mbid", 5). Return([]agents.Song{ - {Name: "Dreaming of Me", MBID: "", Artist: "Depeche Mode", ArtistMBID: "artist-mbid"}, + {Name: "Dreaming of Me", MBID: "", Artists: []agents.Artist{{Name: "Depeche Mode", MBID: "artist-mbid"}}}, }, nil).Once() // Matcher artist resolution: resolve Depeche Mode in the artist table. @@ -177,7 +177,7 @@ var _ = Describe("Provider - SimilarSongs", func() { agentsCombined.On("GetSimilarSongsByAlbum", mock.Anything, "album-1", "Speak & Spell", "Depeche Mode", "album-mbid", 5). Return([]agents.Song{ - {Name: "New Life", MBID: "song-mbid", Artist: "Depeche Mode"}, + {Name: "New Life", MBID: "song-mbid", Artists: []agents.Artist{{Name: "Depeche Mode"}}}, }, nil).Once() // Mock loadTracksByID - no ID matches @@ -254,7 +254,7 @@ var _ = Describe("Provider - SimilarSongs", func() { artistRepo.On("Get", "artist-1").Return(&artist, nil).Once() agentsCombined.On("GetSimilarSongsByArtist", mock.Anything, "artist-1", "Depeche Mode", "artist-mbid", 5). Return([]agents.Song{ - {Name: "Enjoy the Silence", MBID: "song-mbid", Artist: "Depeche Mode"}, + {Name: "Enjoy the Silence", MBID: "song-mbid", Artists: []agents.Artist{{Name: "Depeche Mode"}}}, }, nil).Once() // Mock loadTracksByID - no ID matches diff --git a/core/external/provider_topsongs_test.go b/core/external/provider_topsongs_test.go index 600524819..86f9110e2 100644 --- a/core/external/provider_topsongs_test.go +++ b/core/external/provider_topsongs_test.go @@ -76,6 +76,63 @@ var _ = Describe("Provider - TopSongs", func() { mediaFileRepo.AssertExpectations(GinkgoT()) }) + It("backfills name and MBID onto an unnamed primary credit (the queried artist) and matches", func() { + artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"} + artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil) + + // Agent leaves the first credit unnamed (e.g. an MBID-less collaborator slot). That blank + // credit IS the queried artist, so enrichment fills both name and MBID; the song then matches + // the queried artist's track via the backfilled identity. + agentSongs := []agents.Song{ + {Name: "Song One", Artists: []agents.Artist{{}}}, + } + ag.On("GetArtistTopSongs", ctx, "artist-1", "Artist One", "mbid-artist-1", 1).Return(agentSongs, nil).Once() + + track := model.MediaFile{ + ID: "song-1", Title: "Song One", ArtistID: "artist-1", + Participants: model.Participants{model.RoleArtist: model.ParticipantList{ + {Artist: model.Artist{ID: "artist-1", Name: "Artist One", OrderArtistName: "artist one", MbzArtistID: "mbid-artist-1"}}, + }}, + } + mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{track}, nil) + + songs, err := p.TopSongs(ctx, "Artist One", 1) + + Expect(err).ToNot(HaveOccurred()) + Expect(songs).To(HaveLen(1)) + Expect(songs[0].ID).To(Equal("song-1")) + }) + + It("does not stamp the queried MBID onto an already-named different first credit", func() { + // The queried artist (One) appears only as a featured collaborator; the displayed first credit + // is a DIFFERENT artist (Two) returned without an MBID. Enrichment must NOT assign One's MBID + // to Two — only Two's name match (which fails here) or One's own credit may resolve the track. + artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"} + artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil) + + agentSongs := []agents.Song{ + {Name: "Collab Song", Artists: []agents.Artist{{Name: "Artist Two"}}}, + } + ag.On("GetArtistTopSongs", ctx, "artist-1", "Artist One", "mbid-artist-1", 1).Return(agentSongs, nil).Once() + + // Library track is credited to Artist One (the queried artist) under a same title. If the + // queried MBID were wrongly stamped onto the "Artist Two" credit, that mismatched name+MBID + // could mis-resolve. With the guard, "Artist Two" stays MBID-less and does not match One's track. + track := model.MediaFile{ + ID: "one-track", Title: "Collab Song", ArtistID: "artist-1", + Participants: model.Participants{model.RoleArtist: model.ParticipantList{ + {Artist: model.Artist{ID: "artist-1", Name: "Artist One", OrderArtistName: "artist one", MbzArtistID: "mbid-artist-1"}}, + }}, + } + mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{track}, nil) + + songs, err := p.TopSongs(ctx, "Artist One", 1) + + Expect(err).ToNot(HaveOccurred()) + // "Artist Two" (named, MBID-less, not in the library) does not resolve to One's track. + Expect(songs).To(BeEmpty()) + }) + It("returns nil for an unknown artist", func() { // Mock artist not found artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{}, nil).Once() diff --git a/core/matcher/matcher.go b/core/matcher/matcher.go index d472f4a95..25b8fda5f 100644 --- a/core/matcher/matcher.go +++ b/core/matcher/matcher.go @@ -360,9 +360,9 @@ func groupQueries(songs []agents.Song, result map[int]model.MediaFile) []indexed continue } var artists []queryArtist - for _, a := range s.ArtistList() { + for _, a := range s.Artists { name := str.SanitizeFieldForSortingNoArticle(a.Name) - if a.ID == "" && name == "" { + if a.ID == "" && name == "" && a.MBID == "" { continue } artists = append(artists, queryArtist{id: a.ID, name: name, mbid: a.MBID}) diff --git a/core/matcher/matcher_internal_test.go b/core/matcher/matcher_internal_test.go index b81fa9012..5b987d937 100644 --- a/core/matcher/matcher_internal_test.go +++ b/core/matcher/matcher_internal_test.go @@ -143,7 +143,7 @@ var _ = Describe("groupQueries", func() { It("strips leading articles from artist names", func() { songs := []agents.Song{ - {Name: "Song A", Artist: "The Drake"}, + {Name: "Song A", Artists: []agents.Artist{{Name: "The Drake"}}}, } queries := groupQueries(songs, map[int]model.MediaFile{}) Expect(queries).To(HaveLen(1)) @@ -162,32 +162,35 @@ var _ = Describe("groupQueries", func() { Expect(queries[0].query.artists[0].name).To(Equal("")) }) - It("drops an artist with empty id and empty name but keeps usable ones", func() { + It("keeps an artist that carries only an MBID (empty id and name)", func() { + // ListenBrainz collaborators arrive as MBID-only when the API supplies a combined display + // name; the MBID is a usable identity signal and must not be dropped. songs := []agents.Song{ - {Name: "Song A", Artists: []agents.Artist{{Name: ""}, {Name: "Future"}}}, + {Name: "Song A", Artists: []agents.Artist{{MBID: "mbz-future"}}}, } queries := groupQueries(songs, map[int]model.MediaFile{}) Expect(queries).To(HaveLen(1)) Expect(queries[0].query.artists).To(HaveLen(1)) - Expect(queries[0].query.artists[0].name).To(Equal("future")) + Expect(queries[0].query.artists[0].id).To(Equal("")) + Expect(queries[0].query.artists[0].name).To(Equal("")) + Expect(queries[0].query.artists[0].mbid).To(Equal("mbz-future")) }) - It("falls back to the single Artist field via ArtistList", func() { + It("drops a fully-empty artist (no id, name, or mbid) but keeps usable ones", func() { songs := []agents.Song{ - {Name: "Song A", Artist: "Future", ArtistMBID: "mbid-1"}, + {Name: "Song A", Artists: []agents.Artist{{}, {Name: "Future"}}}, } queries := groupQueries(songs, map[int]model.MediaFile{}) Expect(queries).To(HaveLen(1)) Expect(queries[0].query.artists).To(HaveLen(1)) Expect(queries[0].query.artists[0].name).To(Equal("future")) - Expect(queries[0].query.artists[0].mbid).To(Equal("mbid-1")) }) It("skips already-matched songs and songs with no usable artist", func() { songs := []agents.Song{ - {Name: "Already Matched", Artist: "Drake"}, + {Name: "Already Matched", Artists: []agents.Artist{{Name: "Drake"}}}, {Name: "No Artist"}, - {Name: "Song C", Artist: "Future"}, + {Name: "Song C", Artists: []agents.Artist{{Name: "Future"}}}, } queries := groupQueries(songs, map[int]model.MediaFile{0: {ID: "done"}}) Expect(queries).To(HaveLen(1)) diff --git a/core/matcher/matcher_test.go b/core/matcher/matcher_test.go index 1b3af3aa3..a46db8a09 100644 --- a/core/matcher/matcher_test.go +++ b/core/matcher/matcher_test.go @@ -107,7 +107,7 @@ var _ = Describe("Matcher", func() { It("matches songs with an ID field to MediaFiles by ID", func() { conf.Server.Matcher.FuzzyThreshold = 100 songs := []agents.Song{ - {ID: "track-1", Name: "Some Song", Artist: "Some Artist"}, + {ID: "track-1", Name: "Some Song", Artists: []agents.Artist{{Name: "Some Artist"}}}, } idMatch := model.MediaFile{ ID: "track-1", Title: "Some Song", Artist: "Some Artist", @@ -125,7 +125,7 @@ var _ = Describe("Matcher", func() { It("matches songs with MBID to tracks with matching mbz_recording_id", func() { conf.Server.Matcher.FuzzyThreshold = 100 songs := []agents.Song{ - {Name: "Paranoid Android", MBID: "abc-123", Artist: "Radiohead"}, + {Name: "Paranoid Android", MBID: "abc-123", Artists: []agents.Artist{{Name: "Radiohead"}}}, } mbidMatch := model.MediaFile{ ID: "track-mbid", Title: "Paranoid Android", Artist: "Radiohead", @@ -144,7 +144,7 @@ var _ = Describe("Matcher", func() { It("matches songs with ISRC to tracks with matching ISRC tag", func() { conf.Server.Matcher.FuzzyThreshold = 100 songs := []agents.Song{ - {Name: "Paranoid Android", ISRC: "GBAYE0000351", Artist: "Radiohead"}, + {Name: "Paranoid Android", ISRC: "GBAYE0000351", Artists: []agents.Artist{{Name: "Radiohead"}}}, } isrcMatch := model.MediaFile{ ID: "track-isrc", Title: "Paranoid Android", Artist: "Radiohead", @@ -163,7 +163,7 @@ var _ = Describe("Matcher", func() { It("matches songs by title and artist name", func() { conf.Server.Matcher.FuzzyThreshold = 100 songs := []agents.Song{ - {Name: "Enjoy the Silence", Artist: "Depeche Mode"}, + {Name: "Enjoy the Silence", Artists: []agents.Artist{{Name: "Depeche Mode"}}}, } titleMatch := model.MediaFile{ ID: "track-title", Title: "Enjoy the Silence", Artist: "Depeche Mode", @@ -179,7 +179,7 @@ var _ = Describe("Matcher", func() { It("matches songs with fuzzy title similarity", func() { conf.Server.Matcher.FuzzyThreshold = 85 songs := []agents.Song{ - {Name: "Bohemian Rhapsody", Artist: "Queen"}, + {Name: "Bohemian Rhapsody", Artists: []agents.Artist{{Name: "Queen"}}}, } fuzzyMatch := model.MediaFile{ ID: "track-fuzzy", Title: "Bohemian Rhapsody (Live)", Artist: "Queen", @@ -195,7 +195,7 @@ var _ = Describe("Matcher", func() { It("does not match completely different titles", func() { conf.Server.Matcher.FuzzyThreshold = 85 songs := []agents.Song{ - {Name: "Yesterday", Artist: "The Beatles"}, + {Name: "Yesterday", Artists: []agents.Artist{{Name: "The Beatles"}}}, } differentTracks := model.MediaFiles{ {ID: "different", Title: "Tomorrow Never Knows", Artist: "The Beatles", @@ -213,8 +213,8 @@ var _ = Describe("Matcher", func() { It("removes duplicates when different input songs match the same library track", func() { conf.Server.Matcher.FuzzyThreshold = 85 songs := []agents.Song{ - {Name: "Bohemian Rhapsody (Live)", Artist: "Queen"}, - {Name: "Bohemian Rhapsody (Original Mix)", Artist: "Queen"}, + {Name: "Bohemian Rhapsody (Live)", Artists: []agents.Artist{{Name: "Queen"}}}, + {Name: "Bohemian Rhapsody (Original Mix)", Artists: []agents.Artist{{Name: "Queen"}}}, } libraryTrack := model.MediaFile{ ID: "br-live", Title: "Bohemian Rhapsody (Live)", Artist: "Queen", @@ -230,8 +230,8 @@ var _ = Describe("Matcher", func() { It("preserves duplicates when identical input songs match the same library track", func() { conf.Server.Matcher.FuzzyThreshold = 85 songs := []agents.Song{ - {Name: "Bohemian Rhapsody", Artist: "Queen", Album: "A Night at the Opera"}, - {Name: "Bohemian Rhapsody", Artist: "Queen", Album: "A Night at the Opera"}, + {Name: "Bohemian Rhapsody", Artists: []agents.Artist{{Name: "Queen"}}, Album: "A Night at the Opera"}, + {Name: "Bohemian Rhapsody", Artists: []agents.Artist{{Name: "Queen"}}, Album: "A Night at the Opera"}, } libraryTrack := model.MediaFile{ ID: "br", Title: "Bohemian Rhapsody", Artist: "Queen", Album: "A Night at the Opera", @@ -253,7 +253,7 @@ var _ = Describe("Matcher", func() { // and short-circuit the MBID phase entirely, so no MBID fetch should // occur even though an mbz_recording_id exists in the input. songs := []agents.Song{ - {ID: "track-id", Name: "Song", MBID: "mbid-1", Artist: "Artist"}, + {ID: "track-id", Name: "Song", MBID: "mbid-1", Artists: []agents.Artist{{Name: "Artist"}}}, } idMatch := model.MediaFile{ ID: "track-id", Title: "Song", Artist: "Artist", @@ -271,9 +271,9 @@ var _ = Describe("Matcher", func() { It("returns at most 'count' results", func() { conf.Server.Matcher.FuzzyThreshold = 100 songs := []agents.Song{ - {Name: "Song A", Artist: "Artist"}, - {Name: "Song B", Artist: "Artist"}, - {Name: "Song C", Artist: "Artist"}, + {Name: "Song A", Artists: []agents.Artist{{Name: "Artist"}}}, + {Name: "Song B", Artists: []agents.Artist{{Name: "Artist"}}}, + {Name: "Song C", Artists: []agents.Artist{{Name: "Artist"}}}, } tracks := model.MediaFiles{ {ID: "a", Title: "Song A", Artist: "Artist", @@ -304,7 +304,7 @@ var _ = Describe("Matcher", func() { Context("artist grouping", func() { It("groups title-phase tracks by participant artist ID, not display Artist", func() { songs := []agents.Song{ - {Name: "Song A", Artist: "Daft Punk"}, + {Name: "Song A", Artists: []agents.Artist{{Name: "Daft Punk"}}}, } // Display Artist differs from the query artist; only the participant // with order_artist_name "daft punk" routes to this query bucket. @@ -326,7 +326,7 @@ var _ = Describe("Matcher", func() { It("matches a track that credits the searched artist as a collaborator", func() { songs := []agents.Song{ - {Name: "Crazy", Artist: "INXS"}, + {Name: "Crazy", Artists: []agents.Artist{{Name: "INXS"}}}, } // "Par-T-One vs. INXS" — display Artist is the collaboration, but INXS is a // credited artist participant. Searching INXS must match it. @@ -347,7 +347,7 @@ var _ = Describe("Matcher", func() { It("does not match a track where the searched artist is only the album artist", func() { songs := []agents.Song{ - {Name: "Qmart", Artist: "808 State"}, + {Name: "Qmart", Artists: []agents.Artist{{Name: "808 State"}}}, } // Track performed by Björk on an "808 State" compilation: 808 State is the // albumartist, Björk is the performer. Searching 808 State must NOT match it. @@ -379,7 +379,7 @@ var _ = Describe("Matcher", func() { It("resolves the artist by ArtistMBID when the name differs", func() { songs := []agents.Song{ - {Name: "Song A", Artist: "Typo Artist", ArtistMBID: "mbid-9"}, + {Name: "Song A", Artists: []agents.Artist{{Name: "Typo Artist", MBID: "mbid-9"}}}, } track := model.MediaFile{ ID: "by-mbid", Title: "Song A", Artist: "Correct Artist", @@ -403,8 +403,8 @@ var _ = Describe("Matcher", func() { // Two agent results for the same MusicBrainz artist but spelled differently // (an alias). Both must match the artist's track via the shared MBID. songs := []agents.Song{ - {Name: "Song A", Artist: "Alias One", ArtistMBID: "mbid-shared"}, - {Name: "Song B", Artist: "Alias Two", ArtistMBID: "mbid-shared"}, + {Name: "Song A", Artists: []agents.Artist{{Name: "Alias One", MBID: "mbid-shared"}}}, + {Name: "Song B", Artists: []agents.Artist{{Name: "Alias Two", MBID: "mbid-shared"}}}, } artist := model.Artist{ID: "a-shared", Name: "Canonical", OrderArtistName: "canonical", MbzArtistID: "mbid-shared"} trackA := model.MediaFile{ID: "ta", Title: "Song A", Artist: "Canonical", Participants: artistParticipants(artist)} @@ -427,8 +427,8 @@ var _ = Describe("Matcher", func() { Context("title phase DB errors", func() { It("returns an error when the title query fails and nothing else matched", func() { songs := []agents.Song{ - {Name: "Song A", Artist: "Artist One"}, - {Name: "Song B", Artist: "Artist Two"}, + {Name: "Song A", Artists: []agents.Artist{{Name: "Artist One"}}}, + {Name: "Song B", Artists: []agents.Artist{{Name: "Artist Two"}}}, } allowIdentifierPhases() artistRepo.On("GetAll", mock.Anything).Return(model.Artists{ @@ -445,8 +445,8 @@ var _ = Describe("Matcher", func() { It("keeps exact-phase matches when the title query fails", func() { songs := []agents.Song{ - {ID: "track-1", Name: "Exact Song", Artist: "Exact Artist"}, - {Name: "Fuzzy Song", Artist: "Fuzzy Artist"}, + {ID: "track-1", Name: "Exact Song", Artists: []agents.Artist{{Name: "Exact Artist"}}}, + {Name: "Fuzzy Song", Artists: []agents.Artist{{Name: "Fuzzy Artist"}}}, } idMatch := model.MediaFile{ID: "track-1", Title: "Exact Song", Artist: "Exact Artist"} expectIDPhase(model.MediaFiles{idMatch}) @@ -500,7 +500,7 @@ var _ = Describe("Matcher", func() { It("matches a single-artist song against a track crediting several artists", func() { conf.Server.Matcher.FuzzyThreshold = 85 songs := []agents.Song{ - {Name: "Life Is Good", Artist: "Future"}, + {Name: "Life Is Good", Artists: []agents.Artist{{Name: "Future"}}}, } track := model.MediaFile{ ID: "multi", Title: "Life Is Good", Artist: "Future feat. Drake", @@ -568,9 +568,9 @@ var _ = Describe("Matcher", func() { Describe("MatchSongsIndexed", func() { It("returns index-keyed map of matched songs", func() { songs := []agents.Song{ - {ID: "track-1", Name: "Song One", Artist: "Artist A"}, - {ID: "track-2", Name: "Song Two", Artist: "Artist B"}, - {ID: "track-3", Name: "Song Three", Artist: "Artist C"}, + {ID: "track-1", Name: "Song One", Artists: []agents.Artist{{Name: "Artist A"}}}, + {ID: "track-2", Name: "Song Two", Artists: []agents.Artist{{Name: "Artist B"}}}, + {ID: "track-3", Name: "Song Three", Artists: []agents.Artist{{Name: "Artist C"}}}, } mf1 := model.MediaFile{ID: "track-1", Title: "Song One", Artist: "Artist A"} mf2 := model.MediaFile{ID: "track-2", Title: "Song Two", Artist: "Artist B"} @@ -589,8 +589,8 @@ var _ = Describe("Matcher", func() { It("preserves original indices when some songs don't match", func() { songs := []agents.Song{ - {Name: "Unknown Song", Artist: "Unknown Artist"}, - {ID: "track-1", Name: "Known Song", Artist: "Known Artist"}, + {Name: "Unknown Song", Artists: []agents.Artist{{Name: "Unknown Artist"}}}, + {ID: "track-1", Name: "Known Song", Artists: []agents.Artist{{Name: "Known Artist"}}}, } mf1 := model.MediaFile{ID: "track-1", Title: "Known Song", Artist: "Known Artist"} @@ -629,7 +629,7 @@ var _ = Describe("Matcher", func() { Participants: artistParticipants(model.Artist{ID: "dm", Name: "Depeche Mode", OrderArtistName: "depeche mode", MbzArtistID: "artist-mbid-123"}), } songs := []agents.Song{ - {Name: "Similar Song", Artist: "Depeche Mode", ArtistMBID: "artist-mbid-123", Album: "Violator", AlbumMBID: "album-mbid-456"}, + {Name: "Similar Song", Artists: []agents.Artist{{Name: "Depeche Mode", MBID: "artist-mbid-123"}}, Album: "Violator", AlbumMBID: "album-mbid-456"}, } allowTitlePhase(model.MediaFiles{wrongMatch, correctMatch}) @@ -651,7 +651,7 @@ var _ = Describe("Matcher", func() { Participants: artistParticipants(model.Artist{ID: "oa", Name: "Other Artist", OrderArtistName: "other artist"}), } songs := []agents.Song{ - {Name: "Similar Song", Artist: "Depeche Mode", Album: "Violator"}, + {Name: "Similar Song", Artists: []agents.Artist{{Name: "Depeche Mode"}}, Album: "Violator"}, } allowTitlePhase(model.MediaFiles{wrongMatch, correctMatch}) @@ -673,7 +673,7 @@ var _ = Describe("Matcher", func() { Participants: artistParticipants(model.Artist{ID: "oa", Name: "Other Artist", OrderArtistName: "other artist"}), } songs := []agents.Song{ - {Name: "Similar Song", Artist: "Depeche Mode"}, + {Name: "Similar Song", Artists: []agents.Artist{{Name: "Depeche Mode"}}}, } allowTitlePhase(model.MediaFiles{wrongMatch, correctMatch}) @@ -710,9 +710,9 @@ var _ = Describe("Matcher", func() { } songs := []agents.Song{ - {Name: "Yesterday", Artist: "The Beatles", Album: "Help!"}, - {Name: "Yesterday", Artist: "Ray Charles", Album: "Greatest Hits"}, - {Name: "Yesterday", Artist: "Frank Sinatra", Album: "My Way"}, + {Name: "Yesterday", Artists: []agents.Artist{{Name: "The Beatles"}}, Album: "Help!"}, + {Name: "Yesterday", Artists: []agents.Artist{{Name: "Ray Charles"}}, Album: "Greatest Hits"}, + {Name: "Yesterday", Artists: []agents.Artist{{Name: "Frank Sinatra"}}, Album: "My Way"}, } allowTitlePhase(model.MediaFiles{cover1, cover2, cover3}) @@ -742,8 +742,8 @@ var _ = Describe("Matcher", func() { } songs := []agents.Song{ - {Name: "Song A", Artist: "Artist One", ArtistMBID: "mbid-1", Album: "Album One", AlbumMBID: "album-mbid-1"}, - {Name: "Song B", Artist: "Artist Two"}, + {Name: "Song A", Artists: []agents.Artist{{Name: "Artist One", MBID: "mbid-1"}}, Album: "Album One", AlbumMBID: "album-mbid-1"}, + {Name: "Song B", Artists: []agents.Artist{{Name: "Artist Two"}}}, } allowTitlePhase(model.MediaFiles{lessAccurateMatch, preciseMatch, artistTwoMatch}) @@ -758,7 +758,7 @@ var _ = Describe("Matcher", func() { It("uses the resolved artist MBID for specificity (level 5)", func() { songs := []agents.Song{ - {Name: "Song A", Artist: "Artist One", ArtistMBID: "mbid-1", Album: "Album One", AlbumMBID: "album-mbid-1"}, + {Name: "Song A", Artists: []agents.Artist{{Name: "Artist One", MBID: "mbid-1"}}, Album: "Album One", AlbumMBID: "album-mbid-1"}, } // Two tracks with the same title and album; only the one whose resolved artist // carries mbid-1 (and whose album MBID matches) wins via Level 5. Without the @@ -788,7 +788,7 @@ var _ = Describe("Matcher", func() { conf.Server.Matcher.FuzzyThreshold = 85 songs := []agents.Song{ - {Name: "Paranoid Android", Artist: "Radiohead"}, + {Name: "Paranoid Android", Artists: []agents.Artist{{Name: "Radiohead"}}}, } artistTracks := model.MediaFiles{ {ID: "remastered", Title: "Paranoid Android - Remastered", Artist: "Radiohead", @@ -809,7 +809,7 @@ var _ = Describe("Matcher", func() { conf.Server.Matcher.FuzzyThreshold = 85 songs := []agents.Song{ - {Name: "Bohemian Rhapsody", Artist: "Queen"}, + {Name: "Bohemian Rhapsody", Artists: []agents.Artist{{Name: "Queen"}}}, } artistTracks := model.MediaFiles{ {ID: "live", Title: "Bohemian Rhapsody (Live)", Artist: "Queen", @@ -832,7 +832,7 @@ var _ = Describe("Matcher", func() { conf.Server.Matcher.FuzzyThreshold = 100 songs := []agents.Song{ - {Name: "Paranoid Android", Artist: "Radiohead"}, + {Name: "Paranoid Android", Artists: []agents.Artist{{Name: "Radiohead"}}}, } artistTracks := model.MediaFiles{ {ID: "remastered", Title: "Paranoid Android - Remastered", Artist: "Radiohead", @@ -854,7 +854,7 @@ var _ = Describe("Matcher", func() { conf.Server.Matcher.FuzzyThreshold = 75 songs := []agents.Song{ - {Name: "Song", Artist: "Artist"}, + {Name: "Song", Artists: []agents.Artist{{Name: "Artist"}}}, } artistTracks := model.MediaFiles{ {ID: "extended", Title: "Song (Extended Mix)", Artist: "Artist", @@ -881,7 +881,7 @@ var _ = Describe("Matcher", func() { It("matches album with (Remaster) suffix", func() { songs := []agents.Song{ - {Name: "Bohemian Rhapsody", Artist: "Queen", Album: "A Night at the Opera"}, + {Name: "Bohemian Rhapsody", Artists: []agents.Artist{{Name: "Queen"}}, Album: "A Night at the Opera"}, } correctMatch := model.MediaFile{ ID: "correct", Title: "Bohemian Rhapsody", Artist: "Queen", Album: "A Night at the Opera (2011 Remaster)", @@ -903,7 +903,7 @@ var _ = Describe("Matcher", func() { It("matches album with (Deluxe Edition) suffix", func() { songs := []agents.Song{ - {Name: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Violator"}, + {Name: "Enjoy the Silence", Artists: []agents.Artist{{Name: "Depeche Mode"}}, Album: "Violator"}, } correctMatch := model.MediaFile{ ID: "correct", Title: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Violator (Deluxe Edition)", @@ -925,7 +925,7 @@ var _ = Describe("Matcher", func() { It("prefers exact album match over fuzzy album match", func() { songs := []agents.Song{ - {Name: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Violator"}, + {Name: "Enjoy the Silence", Artists: []agents.Artist{{Name: "Depeche Mode"}}, Album: "Violator"}, } exactMatch := model.MediaFile{ ID: "exact", Title: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Violator", @@ -948,7 +948,7 @@ var _ = Describe("Matcher", func() { It("prefers a more specific match over a starred track when PreferStarred is enabled", func() { conf.Server.Matcher.PreferStarred = true songs := []agents.Song{ - {Name: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Violator"}, + {Name: "Enjoy the Silence", Artists: []agents.Artist{{Name: "Depeche Mode"}}, Album: "Violator"}, } albumMatch := model.MediaFile{ ID: "album-match", Title: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Violator", @@ -969,7 +969,7 @@ var _ = Describe("Matcher", func() { It("prefers a more specific match over a 4-star track when PreferStarred is enabled", func() { conf.Server.Matcher.PreferStarred = true songs := []agents.Song{ - {Name: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Violator"}, + {Name: "Enjoy the Silence", Artists: []agents.Artist{{Name: "Depeche Mode"}}, Album: "Violator"}, } albumMatch := model.MediaFile{ ID: "album-match", Title: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Violator", @@ -990,7 +990,7 @@ var _ = Describe("Matcher", func() { It("prefers a starred track when specificity and overlap are equal", func() { conf.Server.Matcher.PreferStarred = true songs := []agents.Song{ - {Name: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Violator"}, + {Name: "Enjoy the Silence", Artists: []agents.Artist{{Name: "Depeche Mode"}}, Album: "Violator"}, } // Both credit the same single artist and the same album → equal specificity AND equal overlap. plain := model.MediaFile{ @@ -1017,7 +1017,7 @@ var _ = Describe("Matcher", func() { It("prefers tracks with matching duration", func() { songs := []agents.Song{ - {Name: "Similar Song", Artist: "Test Artist", Duration: 180000}, + {Name: "Similar Song", Artists: []agents.Artist{{Name: "Test Artist"}}, Duration: 180000}, } correctMatch := model.MediaFile{ ID: "correct", Title: "Similar Song", Artist: "Test Artist", Duration: 180.0, @@ -1039,7 +1039,7 @@ var _ = Describe("Matcher", func() { It("matches tracks with close duration", func() { songs := []agents.Song{ - {Name: "Similar Song", Artist: "Test Artist", Duration: 180000}, + {Name: "Similar Song", Artists: []agents.Artist{{Name: "Test Artist"}}, Duration: 180000}, } closeDuration := model.MediaFile{ ID: "close-duration", Title: "Similar Song", Artist: "Test Artist", Duration: 182.5, @@ -1057,7 +1057,7 @@ var _ = Describe("Matcher", func() { It("prefers closer duration over farther duration", func() { songs := []agents.Song{ - {Name: "Similar Song", Artist: "Test Artist", Duration: 180000}, + {Name: "Similar Song", Artists: []agents.Artist{{Name: "Test Artist"}}, Duration: 180000}, } closeDuration := model.MediaFile{ ID: "close", Title: "Similar Song", Artist: "Test Artist", Duration: 181.0, @@ -1079,7 +1079,7 @@ var _ = Describe("Matcher", func() { It("still matches when no tracks have matching duration", func() { songs := []agents.Song{ - {Name: "Similar Song", Artist: "Test Artist", Duration: 180000}, + {Name: "Similar Song", Artists: []agents.Artist{{Name: "Test Artist"}}, Duration: 180000}, } differentDuration := model.MediaFile{ ID: "different", Title: "Similar Song", Artist: "Test Artist", Duration: 300.0, @@ -1097,7 +1097,7 @@ var _ = Describe("Matcher", func() { It("prefers title match over duration match when titles differ", func() { songs := []agents.Song{ - {Name: "Similar Song", Artist: "Test Artist", Duration: 180000}, + {Name: "Similar Song", Artists: []agents.Artist{{Name: "Test Artist"}}, Duration: 180000}, } differentTitle := model.MediaFile{ ID: "wrong-title", Title: "Different Song", Artist: "Test Artist", Duration: 180.0, @@ -1119,7 +1119,7 @@ var _ = Describe("Matcher", func() { It("matches without duration filtering when agent duration is 0", func() { songs := []agents.Song{ - {Name: "Similar Song", Artist: "Test Artist", Duration: 0}, + {Name: "Similar Song", Artists: []agents.Artist{{Name: "Test Artist"}}, Duration: 0}, } anyTrack := model.MediaFile{ ID: "any", Title: "Similar Song", Artist: "Test Artist", Duration: 999.0, @@ -1137,7 +1137,7 @@ var _ = Describe("Matcher", func() { It("handles very short songs with close duration", func() { songs := []agents.Song{ - {Name: "Short Song", Artist: "Test Artist", Duration: 30000}, + {Name: "Short Song", Artists: []agents.Artist{{Name: "Test Artist"}}, Duration: 30000}, } shortTrack := model.MediaFile{ ID: "short", Title: "Short Song", Artist: "Test Artist", Duration: 31.0, @@ -1155,8 +1155,8 @@ var _ = Describe("Matcher", func() { It("matches same title+artist songs to their own closest-duration track", func() { songs := []agents.Song{ - {Name: "Same Song", Artist: "Same Artist", Duration: 180000}, - {Name: "Same Song", Artist: "Same Artist", Duration: 240000}, + {Name: "Same Song", Artists: []agents.Artist{{Name: "Same Artist"}}, Duration: 180000}, + {Name: "Same Song", Artists: []agents.Artist{{Name: "Same Artist"}}, Duration: 240000}, } shortTrack := model.MediaFile{ ID: "short", Title: "Same Song", Artist: "Same Artist", Duration: 180.0, @@ -1185,10 +1185,10 @@ var _ = Describe("Matcher", func() { It("handles mixed scenario with both identical and different input songs", func() { songs := []agents.Song{ - {Name: "Yesterday", Artist: "The Beatles", Album: "Help!"}, - {Name: "Yesterday (Remastered)", Artist: "The Beatles", Album: "1"}, - {Name: "Yesterday", Artist: "The Beatles", Album: "Help!"}, - {Name: "Yesterday (Anthology)", Artist: "The Beatles", Album: "Anthology"}, + {Name: "Yesterday", Artists: []agents.Artist{{Name: "The Beatles"}}, Album: "Help!"}, + {Name: "Yesterday (Remastered)", Artists: []agents.Artist{{Name: "The Beatles"}}, Album: "1"}, + {Name: "Yesterday", Artists: []agents.Artist{{Name: "The Beatles"}}, Album: "Help!"}, + {Name: "Yesterday (Anthology)", Artists: []agents.Artist{{Name: "The Beatles"}}, Album: "Anthology"}, } libraryTrack := model.MediaFile{ ID: "yesterday", Title: "Yesterday", Artist: "The Beatles", Album: "Help!", @@ -1207,9 +1207,9 @@ var _ = Describe("Matcher", func() { It("does not deduplicate songs that match different library tracks", func() { songs := []agents.Song{ - {Name: "Song A", Artist: "Artist"}, - {Name: "Song B", Artist: "Artist"}, - {Name: "Song C", Artist: "Artist"}, + {Name: "Song A", Artists: []agents.Artist{{Name: "Artist"}}}, + {Name: "Song B", Artists: []agents.Artist{{Name: "Artist"}}}, + {Name: "Song C", Artists: []agents.Artist{{Name: "Artist"}}}, } trackA := model.MediaFile{ID: "track-a", Title: "Song A", Artist: "Artist", Participants: artistParticipants(model.Artist{ID: "art", Name: "Artist", OrderArtistName: "artist"}), @@ -1234,10 +1234,10 @@ var _ = Describe("Matcher", func() { It("respects count limit after deduplication", func() { songs := []agents.Song{ - {Name: "Song A", Artist: "Artist"}, - {Name: "Song A (Live)", Artist: "Artist"}, - {Name: "Song B", Artist: "Artist"}, - {Name: "Song B (Remix)", Artist: "Artist"}, + {Name: "Song A", Artists: []agents.Artist{{Name: "Artist"}}}, + {Name: "Song A (Live)", Artists: []agents.Artist{{Name: "Artist"}}}, + {Name: "Song B", Artists: []agents.Artist{{Name: "Artist"}}}, + {Name: "Song B (Remix)", Artists: []agents.Artist{{Name: "Artist"}}}, } trackA := model.MediaFile{ID: "track-a", Title: "Song A", Artist: "Artist", Participants: artistParticipants(model.Artist{ID: "art", Name: "Artist", OrderArtistName: "artist"}), diff --git a/core/sonic/sonic_test.go b/core/sonic/sonic_test.go index 81739b726..813fceea9 100644 --- a/core/sonic/sonic_test.go +++ b/core/sonic/sonic_test.go @@ -101,7 +101,7 @@ var _ = Describe("Sonic", func() { provider := &mockProvider{ similarResults: []sonic.SimilarResult{ - {Song: agents.Song{ID: "song-2", Name: "Similar Song", Artist: "Test Artist"}, Similarity: 0.85}, + {Song: agents.Song{ID: "song-2", Name: "Similar Song", Artists: []agents.Artist{{Name: "Test Artist"}}}, Similarity: 0.85}, }, } loader.names = []string{"test-plugin"} diff --git a/plugins/metadata_agent.go b/plugins/metadata_agent.go index f3c26411c..b565ef6f2 100644 --- a/plugins/metadata_agent.go +++ b/plugins/metadata_agent.go @@ -227,26 +227,29 @@ func (a *MetadataAgent) GetSimilarSongsByArtist(ctx context.Context, id, name, m return callSimilarSongsPluginFunction[capabilities.SimilarSongsByArtistRequest](ctx, a.plugin, FuncGetSimilarSongsByArtist, capabilities.SimilarSongsByArtistRequest{ID: id, Name: name, MBID: mbid, Count: int32(count)}) } -// songRefToAgentSong converts a single SongRef to agents.Song +// songRefToAgentSong converts a single SongRef to agents.Song. SongRef keeps the single +// Artist/ArtistMBID fields as part of the plugin wire contract; when a plugin sends those instead +// of the artists array, they are folded into a one-element Artists list here. func songRefToAgentSong(s capabilities.SongRef) agents.Song { var artists []agents.Artist - if len(s.Artists) > 0 { + switch { + case len(s.Artists) > 0: artists = make([]agents.Artist, len(s.Artists)) for i, a := range s.Artists { artists[i] = agents.Artist{ID: a.ID, Name: a.Name, MBID: a.MBID} } + case s.Artist != "" || s.ArtistMBID != "": + artists = []agents.Artist{{Name: s.Artist, MBID: s.ArtistMBID}} } return agents.Song{ - ID: s.ID, - Name: s.Name, - MBID: s.MBID, - ISRC: s.ISRC, - Artist: s.Artist, - ArtistMBID: s.ArtistMBID, - Artists: artists, - Album: s.Album, - AlbumMBID: s.AlbumMBID, - Duration: uint32(s.Duration * 1000), + ID: s.ID, + Name: s.Name, + MBID: s.MBID, + ISRC: s.ISRC, + Artists: artists, + Album: s.Album, + AlbumMBID: s.AlbumMBID, + Duration: uint32(s.Duration * 1000), } } diff --git a/plugins/metadata_agent_test.go b/plugins/metadata_agent_test.go index be9d309ae..a14db5d34 100644 --- a/plugins/metadata_agent_test.go +++ b/plugins/metadata_agent_test.go @@ -119,7 +119,8 @@ var _ = Describe("MetadataAgent", Ordered, func() { Expect(err).ToNot(HaveOccurred()) Expect(songs).To(HaveLen(3)) Expect(songs[0].Name).To(Equal("Similar to Yesterday #1")) - Expect(songs[0].Artist).To(Equal("The Beatles")) + Expect(songs[0].Artists).To(HaveLen(1)) + Expect(songs[0].Artists[0].Name).To(Equal("The Beatles")) }) }) @@ -343,11 +344,18 @@ var _ = Describe("songRefToAgentSong multi-artist", func() { {Name: "Future", MBID: "m-future"}, })) }) - It("leaves Artists nil and keeps the single Artist when no Artists provided", func() { + It("folds the single Artist/ArtistMBID into a one-element Artists when no Artists provided", func() { ref := capabilities.SongRef{Name: "Solo", Artist: "Drake", ArtistMBID: "m-drake"} got := songRefToAgentSong(ref) + Expect(got.Artists).To(Equal([]agents.Artist{{Name: "Drake", MBID: "m-drake"}})) + }) + It("folds an MBID-only single artist (empty name) so the MBID is not dropped", func() { + ref := capabilities.SongRef{Name: "Solo", ArtistMBID: "m-drake"} + got := songRefToAgentSong(ref) + Expect(got.Artists).To(Equal([]agents.Artist{{MBID: "m-drake"}})) + }) + It("leaves Artists nil when neither Artists nor the single Artist/ArtistMBID are provided", func() { + got := songRefToAgentSong(capabilities.SongRef{Name: "Anon"}) Expect(got.Artists).To(BeNil()) - Expect(got.Artist).To(Equal("Drake")) - Expect(got.ArtistMBID).To(Equal("m-drake")) }) }) diff --git a/plugins/sonic_similarity_adapter_test.go b/plugins/sonic_similarity_adapter_test.go index 1b8d4efa4..52079732b 100644 --- a/plugins/sonic_similarity_adapter_test.go +++ b/plugins/sonic_similarity_adapter_test.go @@ -43,7 +43,8 @@ var _ = Describe("SonicSimilarityPlugin", Ordered, func() { Expect(err).ToNot(HaveOccurred()) Expect(results).To(HaveLen(3)) Expect(results[0].Song.Name).To(Equal("Similar to Yesterday #1")) - Expect(results[0].Song.Artist).To(Equal("The Beatles")) + Expect(results[0].Song.Artists).To(HaveLen(1)) + Expect(results[0].Song.Artists[0].Name).To(Equal("The Beatles")) Expect(results[0].Similarity).To(Equal(1.0)) Expect(results[1].Similarity).To(Equal(0.9)) Expect(results[2].Similarity).To(Equal(0.8)) @@ -68,7 +69,8 @@ var _ = Describe("SonicSimilarityPlugin", Ordered, func() { Expect(err).ToNot(HaveOccurred()) Expect(results).To(HaveLen(3)) Expect(results[0].Song.Name).To(Equal("Path Yesterday to Tomorrow Never Knows #1")) - Expect(results[0].Song.Artist).To(Equal("The Beatles")) + Expect(results[0].Song.Artists).To(HaveLen(1)) + Expect(results[0].Song.Artists[0].Name).To(Equal("The Beatles")) Expect(results[0].Similarity).To(Equal(1.0)) Expect(results[1].Similarity).To(Equal(0.95)) Expect(results[2].Similarity).To(Equal(0.9))