nik/navidrome
1
0
Fork 0
mirror of https://github.com/navidrome/navidrome.git synced 2026-08-01 07:21:17 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(lastfm): match album.getInfo on name+artist only, not MBID

Browse Source 
Last.fm's album.getInfo by MBID is unreliable: a correct MBID can return a
different album, or none. Observed with black midi's "7-eleven" (whose correct
MBID returned a FLEETWOOD release) and both missing The Chats albums (one MBID
404s, the other resolves to a different self-titled release). The worker then
recorded covers absent — or would fetch the wrong art — even though the correct
cover is on Last.fm by name+artist.

Stop passing the MBID to album.getInfo; query by name+artist only, which also
drops the now-dead error-6 MBID-retry fallback. The low-level client keeps its
MBID support for other callers; only the album lookup changes.
This commit is contained in:
Deluan 2026-07-24 17:35:02 -04:00
parent c3eea27b4f
commit 8b844ee102
2 changed files with 20 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
adapters/lastfm/agent.go
View File

@ -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
}

24
adapters/lastfm/agent_test.go
View File

@ -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())
})
})