diff --git a/adapters/lastfm/agent.go b/adapters/lastfm/agent.go index eb8f3d36e..98850fdde 100644 --- a/adapters/lastfm/agent.go +++ b/adapters/lastfm/agent.go @@ -93,7 +93,7 @@ func (l *lastfmAgent) GetAlbumInfo(ctx context.Context, name, artist, mbid strin var resp agents.AlbumInfo for _, lang := range l.languages { var err error - a, err = l.callAlbumGetInfo(ctx, name, artist, mbid, lang) + a, err = l.callAlbumGetInfo(ctx, name, artist, lang) if err != nil { return nil, err } @@ -114,7 +114,7 @@ func (l *lastfmAgent) GetAlbumInfo(ctx context.Context, name, artist, mbid strin } func (l *lastfmAgent) GetAlbumImages(ctx context.Context, name, artist, mbid string) ([]agents.ExternalImage, error) { - a, err := l.callAlbumGetInfo(ctx, name, artist, mbid, l.languages[0]) + a, err := l.callAlbumGetInfo(ctx, name, artist, l.languages[0]) if err != nil { return nil, err } @@ -286,21 +286,16 @@ func (l *lastfmAgent) GetArtistImages(ctx context.Context, _, name, mbid string) return res, nil } -func (l *lastfmAgent) callAlbumGetInfo(ctx context.Context, name, artist, mbid string, lang string) (*Album, error) { - a, err := l.client.albumGetInfo(ctx, name, artist, mbid, lang) - var lfErr *lastFMError - isLastFMError := errors.As(err, &lfErr) - - if mbid != "" && (isLastFMError && lfErr.Code == 6) { - log.Debug(ctx, "LastFM/album.getInfo could not find album by mbid, trying again", "album", name, "mbid", mbid) - return l.callAlbumGetInfo(ctx, name, artist, "", lang) - } - +// callAlbumGetInfo matches on name+artist only. Last.fm's album.getInfo by MBID is unreliable — +// a correct MBID can return a different album (or none) — so the MBID is deliberately not passed. +func (l *lastfmAgent) callAlbumGetInfo(ctx context.Context, name, artist, lang string) (*Album, error) { + a, err := l.client.albumGetInfo(ctx, name, artist, "", lang) if err != nil { - if isLastFMError && lfErr.Code == 6 { - log.Debug(ctx, "Album not found", "album", name, "mbid", mbid, err) + var lfErr *lastFMError + if errors.As(err, &lfErr) && lfErr.Code == 6 { + log.Debug(ctx, "Album not found", "album", name, "artist", artist, err) } else { - log.Error(ctx, "Error calling LastFM/album.getInfo", "album", name, "mbid", mbid, err) + log.Error(ctx, "Error calling LastFM/album.getInfo", "album", name, "artist", artist, err) } return nil, err } diff --git a/adapters/lastfm/agent_test.go b/adapters/lastfm/agent_test.go index 7e4e29294..1d1400620 100644 --- a/adapters/lastfm/agent_test.go +++ b/adapters/lastfm/agent_test.go @@ -539,7 +539,10 @@ var _ = Describe("lastfmAgent", func() { URL: "https://www.last.fm/music/Cher/Believe", })) Expect(httpClient.RequestCount).To(Equal(1)) - Expect(httpClient.SavedRequest.URL.Query().Get("mbid")).To(Equal("03c91c40-49a6-44a7-90e7-a700edf97a62")) + // MBID is deliberately not sent — album.getInfo matches on name+artist only. + Expect(httpClient.SavedRequest.URL.Query().Get("mbid")).To(BeEmpty()) + Expect(httpClient.SavedRequest.URL.Query().Get("album")).To(Equal("Believe")) + Expect(httpClient.SavedRequest.URL.Query().Get("artist")).To(Equal("Cher")) }) It("returns empty images if no images are available", func() { @@ -558,7 +561,7 @@ var _ = Describe("lastfmAgent", func() { _, err := agent.GetAlbumInfo(ctx, "123", "U2", "mbid-1234") Expect(err).To(HaveOccurred()) Expect(httpClient.RequestCount).To(Equal(1)) - Expect(httpClient.SavedRequest.URL.Query().Get("mbid")).To(Equal("mbid-1234")) + Expect(httpClient.SavedRequest.URL.Query().Get("mbid")).To(BeEmpty()) }) It("returns an error if Last.fm call returns an error", func() { @@ -566,23 +569,16 @@ var _ = Describe("lastfmAgent", func() { _, err := agent.GetAlbumInfo(ctx, "123", "U2", "mbid-1234") Expect(err).To(HaveOccurred()) Expect(httpClient.RequestCount).To(Equal(1)) - Expect(httpClient.SavedRequest.URL.Query().Get("mbid")).To(Equal("mbid-1234")) + Expect(httpClient.SavedRequest.URL.Query().Get("mbid")).To(BeEmpty()) }) - It("returns an error if Last.fm call returns an error 6 and mbid is empty", func() { + It("returns an error when Last.fm returns an error 6 (album not found)", func() { httpClient.Res = http.Response{Body: io.NopCloser(bytes.NewBufferString(lastfmError6)), StatusCode: 200} - _, err := agent.GetAlbumInfo(ctx, "123", "U2", "") + _, err := agent.GetAlbumInfo(ctx, "123", "U2", "mbid-1234") Expect(err).To(HaveOccurred()) + // No MBID retry: album.getInfo is queried by name+artist only, in a single call. Expect(httpClient.RequestCount).To(Equal(1)) - }) - - Context("MBID non existent in Last.fm", func() { - It("calls again when last.fm returns an error 6", func() { - httpClient.Res = http.Response{Body: io.NopCloser(bytes.NewBufferString(lastfmError6)), StatusCode: 200} - _, _ = agent.GetAlbumInfo(ctx, "123", "U2", "mbid-1234") - Expect(httpClient.RequestCount).To(Equal(2)) - Expect(httpClient.SavedRequest.URL.Query().Get("mbid")).To(BeEmpty()) - }) + Expect(httpClient.SavedRequest.URL.Query().Get("mbid")).To(BeEmpty()) }) })