From 353aff2c88e287e9cc40d4f5266b1b8dd757960e Mon Sep 17 00:00:00 2001 From: Deluan Date: Wed, 19 Nov 2025 20:49:29 -0500 Subject: [PATCH] fix(lastfm): ignore artist placeholder image. Fix #4702 Signed-off-by: Deluan --- core/agents/lastfm/agent.go | 27 +++++--- core/agents/lastfm/agent_test.go | 69 +++++++++++++++++++ tests/fixtures/lastfm.artist.page.html | 7 ++ .../fixtures/lastfm.artist.page.ignored.html | 7 ++ .../fixtures/lastfm.artist.page.no_meta.html | 6 ++ 5 files changed, 106 insertions(+), 10 deletions(-) create mode 100644 tests/fixtures/lastfm.artist.page.html create mode 100644 tests/fixtures/lastfm.artist.page.ignored.html create mode 100644 tests/fixtures/lastfm.artist.page.no_meta.html diff --git a/core/agents/lastfm/agent.go b/core/agents/lastfm/agent.go index d01b496ec..fafa6afec 100644 --- a/core/agents/lastfm/agent.go +++ b/core/agents/lastfm/agent.go @@ -38,6 +38,7 @@ type lastfmAgent struct { secret string lang string client *client + httpClient httpDoer getInfoMutex sync.Mutex } @@ -56,6 +57,7 @@ func lastFMConstructor(ds model.DataStore) *lastfmAgent { Timeout: consts.DefaultHttpClientTimeOut, } chc := cache.NewHTTPClient(hc, consts.DefaultHttpClientTimeOut) + l.httpClient = chc l.client = newClient(l.apiKey, l.secret, l.lang, chc) return l } @@ -190,13 +192,13 @@ func (l *lastfmAgent) GetArtistTopSongs(ctx context.Context, id, artistName, mbi return res, nil } -var artistOpenGraphQuery = cascadia.MustCompile(`html > head > meta[property="og:image"]`) +var ( + artistOpenGraphQuery = cascadia.MustCompile(`html > head > meta[property="og:image"]`) + artistIgnoredImage = "2a96cbd8b46e442fc41c2b86b821562f" // Last.fm artist placeholder image name +) func (l *lastfmAgent) GetArtistImages(ctx context.Context, _, name, mbid string) ([]agents.ExternalImage, error) { log.Debug(ctx, "Getting artist images from Last.fm", "name", name) - hc := http.Client{ - Timeout: consts.DefaultHttpClientTimeOut, - } a, err := l.callArtistGetInfo(ctx, name) if err != nil { return nil, fmt.Errorf("get artist info: %w", err) @@ -205,7 +207,7 @@ func (l *lastfmAgent) GetArtistImages(ctx context.Context, _, name, mbid string) if err != nil { return nil, fmt.Errorf("create artist image request: %w", err) } - resp, err := hc.Do(req) + resp, err := l.httpClient.Do(req) if err != nil { return nil, fmt.Errorf("get artist url: %w", err) } @@ -222,11 +224,16 @@ func (l *lastfmAgent) GetArtistImages(ctx context.Context, _, name, mbid string) return res, nil } for _, attr := range n.Attr { - if attr.Key == "content" { - res = []agents.ExternalImage{ - {URL: attr.Val}, - } - break + if attr.Key != "content" { + continue + } + if strings.Contains(attr.Val, artistIgnoredImage) { + log.Debug(ctx, "Artist image is ignored default image", "name", name, "url", attr.Val) + return res, nil + } + + res = []agents.ExternalImage{ + {URL: attr.Val}, } } return res, nil diff --git a/core/agents/lastfm/agent_test.go b/core/agents/lastfm/agent_test.go index 4476d592f..18e7facf2 100644 --- a/core/agents/lastfm/agent_test.go +++ b/core/agents/lastfm/agent_test.go @@ -393,4 +393,73 @@ var _ = Describe("lastfmAgent", func() { }) }) }) + + Describe("GetArtistImages", func() { + var agent *lastfmAgent + var apiClient *tests.FakeHttpClient + var httpClient *tests.FakeHttpClient + + BeforeEach(func() { + apiClient = &tests.FakeHttpClient{} + httpClient = &tests.FakeHttpClient{} + client := newClient("API_KEY", "SECRET", "pt", apiClient) + agent = lastFMConstructor(ds) + agent.client = client + agent.httpClient = httpClient + }) + + It("returns the artist image from the page", func() { + fApi, _ := os.Open("tests/fixtures/lastfm.artist.getinfo.json") + apiClient.Res = http.Response{Body: fApi, StatusCode: 200} + + fScraper, _ := os.Open("tests/fixtures/lastfm.artist.page.html") + httpClient.Res = http.Response{Body: fScraper, StatusCode: 200} + + images, err := agent.GetArtistImages(ctx, "123", "U2", "") + Expect(err).ToNot(HaveOccurred()) + Expect(images).To(HaveLen(1)) + Expect(images[0].URL).To(Equal("https://lastfm.freetls.fastly.net/i/u/ar0/818148bf682d429dc21b59a73ef6f68e.png")) + }) + + It("returns empty list if image is the ignored default image", func() { + fApi, _ := os.Open("tests/fixtures/lastfm.artist.getinfo.json") + apiClient.Res = http.Response{Body: fApi, StatusCode: 200} + + fScraper, _ := os.Open("tests/fixtures/lastfm.artist.page.ignored.html") + httpClient.Res = http.Response{Body: fScraper, StatusCode: 200} + + images, err := agent.GetArtistImages(ctx, "123", "U2", "") + Expect(err).ToNot(HaveOccurred()) + Expect(images).To(BeEmpty()) + }) + + It("returns empty list if page has no meta tags", func() { + fApi, _ := os.Open("tests/fixtures/lastfm.artist.getinfo.json") + apiClient.Res = http.Response{Body: fApi, StatusCode: 200} + + fScraper, _ := os.Open("tests/fixtures/lastfm.artist.page.no_meta.html") + httpClient.Res = http.Response{Body: fScraper, StatusCode: 200} + + images, err := agent.GetArtistImages(ctx, "123", "U2", "") + Expect(err).ToNot(HaveOccurred()) + Expect(images).To(BeEmpty()) + }) + + It("returns error if API call fails", func() { + apiClient.Err = errors.New("api error") + _, err := agent.GetArtistImages(ctx, "123", "U2", "") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("get artist info")) + }) + + It("returns error if scraper call fails", func() { + fApi, _ := os.Open("tests/fixtures/lastfm.artist.getinfo.json") + apiClient.Res = http.Response{Body: fApi, StatusCode: 200} + + httpClient.Err = errors.New("scraper error") + _, err := agent.GetArtistImages(ctx, "123", "U2", "") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("get artist url")) + }) + }) }) diff --git a/tests/fixtures/lastfm.artist.page.html b/tests/fixtures/lastfm.artist.page.html new file mode 100644 index 000000000..1922e313b --- /dev/null +++ b/tests/fixtures/lastfm.artist.page.html @@ -0,0 +1,7 @@ + + + + + + + \ No newline at end of file diff --git a/tests/fixtures/lastfm.artist.page.ignored.html b/tests/fixtures/lastfm.artist.page.ignored.html new file mode 100644 index 000000000..96eda2377 --- /dev/null +++ b/tests/fixtures/lastfm.artist.page.ignored.html @@ -0,0 +1,7 @@ + + + + + + + \ No newline at end of file diff --git a/tests/fixtures/lastfm.artist.page.no_meta.html b/tests/fixtures/lastfm.artist.page.no_meta.html new file mode 100644 index 000000000..aa7b9c934 --- /dev/null +++ b/tests/fixtures/lastfm.artist.page.no_meta.html @@ -0,0 +1,6 @@ + + + + + + \ No newline at end of file