navidrome/adapters/deezer/client_test.go

Ignoring revisions in .git-blame-ignore-revs. Click here to bypass and see the normal blame view.

242 lines
8.1 KiB
Go
Raw Permalink Normal View History

package deezer
import (
"bytes"
fix(deezer): treat an exhausted quota as a throttle, not as a missing artist (#6068) * fix(deezer): treat an exhausted quota as a throttle, not as a missing artist Deezer reports quota exhaustion in the response body, with HTTP 200 and no rate-limit headers. The client only looked for errors when the status was not 200, so a throttled reply was decoded into an empty result type, and an empty search became ErrNotFound. The agent then compounded it: it tested `errors.Is(err, ErrNotFound) || len(artists) == 0` before testing err, so any failed search — which also returns no artists — reported not-found too. The artwork worker settles an entity as "no image" on agents.ErrNotFound. So being throttled did not make Navidrome back off; it made it record the artist as having no artwork, and move on to do the same to the next one. Errors are now parsed out of the body regardless of status, and the quota code is joined with agents.RetryLaterError so the circuit breaker and the artwork retry budget see a throttle for what it is. The agent checks err before the empty-result case. Last.fm already handles this exact shape (client.go errCodeRateLimit, with a comment noting the 200-with-body-error pattern); this brings Deezer in line with it, including the zero-delay RetryLaterError so both providers share the default cooldown rather than a per-provider number. Measured against the live API to pin the shape: a 120-request burst returned 54 results and 66 quota replies, every one of them HTTP 200 with {"error":{"type":"Exception","message":"Quota limit exceeded","code":4}} and no Retry-After or rate-limit headers. A single request 5s later succeeded, so the window is short and a cooldown fully clears it. * refactor(deezer): fold the error envelope into one type The envelope declared the code and message inline, parseBodyError copied them field by field into a second struct with the same shape, and a zero Code stood in for "no error reported". Making the envelope hold a pointer to the error type removes all three: absent is nil, present is the error itself, and the value returned needs no conversion. searchArtist loses its empty-result branch. searchArtists converts an empty result to errNotFound and returns early on any error, so it never answers with no artists and no error, and the branch could not run. What it left behind was a comment explaining an ordering that only mattered while the branch existed. ErrNotFound is unexported: nothing outside this package referenced it, and it sat three lines from agents.ErrNotFound, which is a different error with the opposite meaning for callers. Throttling now joins agents.ErrRetryLater, the sentinel documented as the zero-delay RetryLaterError, rather than allocating an equivalent value. * refactor(deezer): return agents.ErrNotFound from the client The client raised a package-local sentinel that the agent then translated into agents.ErrNotFound, one call site each. Deezer was the only adapter carrying its own: last.fm and listenbrainz have none. The client already reports throttling with agents.ErrRetryLater, so it already speaks the agent vocabulary; saying "not found" in the same words costs nothing and lets searchArtist drop to plain error propagation. * test(scrobbler): remove a race in the longest-server-delay test newBufferedScrobbler starts its drain goroutine, and run() drains once before it ever waits on the wake signal. The test enqueued user2, then enqueued user1 via Scrobble, so that startup drain could land between the two: it saw only user2, took its 45s delay, and set backingOff. The wake from the second enqueue is then deliberately ignored — a wake during a backoff window must not drain, which is the hammering the window exists to prevent — so user1 was never attempted and the first assertion read 1 instead of 2. Buffering both users before the goroutine exists removes the window. The test no longer goes through Scrobble, which the sibling tests already cover; what this one is about is which delay wins. Reproduced deterministically by forcing the interleaving with a synctest.Wait between the two enqueues, which fails with the same "expected both users drained, got 1 attempts" seen in CI. With both enqueued first, that same forced drain passes.
2026-09-01 17:17:52 -04:00
"errors"
2025-11-21 15:09:24 -05:00
"fmt"
"io"
"net/http"
"os"
2025-11-21 15:09:24 -05:00
"time"
fix(deezer): treat an exhausted quota as a throttle, not as a missing artist (#6068) * fix(deezer): treat an exhausted quota as a throttle, not as a missing artist Deezer reports quota exhaustion in the response body, with HTTP 200 and no rate-limit headers. The client only looked for errors when the status was not 200, so a throttled reply was decoded into an empty result type, and an empty search became ErrNotFound. The agent then compounded it: it tested `errors.Is(err, ErrNotFound) || len(artists) == 0` before testing err, so any failed search — which also returns no artists — reported not-found too. The artwork worker settles an entity as "no image" on agents.ErrNotFound. So being throttled did not make Navidrome back off; it made it record the artist as having no artwork, and move on to do the same to the next one. Errors are now parsed out of the body regardless of status, and the quota code is joined with agents.RetryLaterError so the circuit breaker and the artwork retry budget see a throttle for what it is. The agent checks err before the empty-result case. Last.fm already handles this exact shape (client.go errCodeRateLimit, with a comment noting the 200-with-body-error pattern); this brings Deezer in line with it, including the zero-delay RetryLaterError so both providers share the default cooldown rather than a per-provider number. Measured against the live API to pin the shape: a 120-request burst returned 54 results and 66 quota replies, every one of them HTTP 200 with {"error":{"type":"Exception","message":"Quota limit exceeded","code":4}} and no Retry-After or rate-limit headers. A single request 5s later succeeded, so the window is short and a cooldown fully clears it. * refactor(deezer): fold the error envelope into one type The envelope declared the code and message inline, parseBodyError copied them field by field into a second struct with the same shape, and a zero Code stood in for "no error reported". Making the envelope hold a pointer to the error type removes all three: absent is nil, present is the error itself, and the value returned needs no conversion. searchArtist loses its empty-result branch. searchArtists converts an empty result to errNotFound and returns early on any error, so it never answers with no artists and no error, and the branch could not run. What it left behind was a comment explaining an ordering that only mattered while the branch existed. ErrNotFound is unexported: nothing outside this package referenced it, and it sat three lines from agents.ErrNotFound, which is a different error with the opposite meaning for callers. Throttling now joins agents.ErrRetryLater, the sentinel documented as the zero-delay RetryLaterError, rather than allocating an equivalent value. * refactor(deezer): return agents.ErrNotFound from the client The client raised a package-local sentinel that the agent then translated into agents.ErrNotFound, one call site each. Deezer was the only adapter carrying its own: last.fm and listenbrainz have none. The client already reports throttling with agents.ErrRetryLater, so it already speaks the agent vocabulary; saying "not found" in the same words costs nothing and lets searchArtist drop to plain error propagation. * test(scrobbler): remove a race in the longest-server-delay test newBufferedScrobbler starts its drain goroutine, and run() drains once before it ever waits on the wake signal. The test enqueued user2, then enqueued user1 via Scrobble, so that startup drain could land between the two: it saw only user2, took its 45s delay, and set backingOff. The wake from the second enqueue is then deliberately ignored — a wake during a backoff window must not drain, which is the hammering the window exists to prevent — so user1 was never attempted and the first assertion read 1 instead of 2. Buffering both users before the goroutine exists removes the window. The test no longer goes through Scrobble, which the sibling tests already cover; what this one is about is which delay wins. Reproduced deterministically by forcing the interleaving with a synctest.Wait between the two enqueues, which fails with the same "expected both users drained, got 1 attempts" seen in CI. With both enqueued first, that same forced drain passes.
2026-09-01 17:17:52 -04:00
"github.com/navidrome/navidrome/core/agents"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
var _ = Describe("client", func() {
var httpClient *fakeHttpClient
var client *client
BeforeEach(func() {
httpClient = &fakeHttpClient{}
client = newClient(httpClient)
})
Describe("ArtistImages", func() {
It("returns artist images from a successful request", func() {
f, err := os.Open("tests/fixtures/deezer.search.artist.json")
Expect(err).To(BeNil())
httpClient.mock("https://api.deezer.com/search/artist", http.Response{Body: f, StatusCode: 200})
2025-11-21 15:09:24 -05:00
artists, err := client.searchArtists(GinkgoT().Context(), "Michael Jackson", 20)
Expect(err).To(BeNil())
Expect(artists).To(HaveLen(17))
Expect(artists[0].Name).To(Equal("Michael Jackson"))
Expect(artists[0].PictureXl).To(Equal("https://cdn-images.dzcdn.net/images/artist/97fae13b2b30e4aec2e8c9e0c7839d92/1000x1000-000000-80-0-0.jpg"))
})
It("fails if artist was not found", func() {
httpClient.mock("https://api.deezer.com/search/artist", http.Response{
StatusCode: 200,
Body: io.NopCloser(bytes.NewBufferString(`{"data":[],"total":0}`)),
})
2025-11-21 15:09:24 -05:00
_, err := client.searchArtists(GinkgoT().Context(), "Michael Jackson", 20)
fix(deezer): treat an exhausted quota as a throttle, not as a missing artist (#6068) * fix(deezer): treat an exhausted quota as a throttle, not as a missing artist Deezer reports quota exhaustion in the response body, with HTTP 200 and no rate-limit headers. The client only looked for errors when the status was not 200, so a throttled reply was decoded into an empty result type, and an empty search became ErrNotFound. The agent then compounded it: it tested `errors.Is(err, ErrNotFound) || len(artists) == 0` before testing err, so any failed search — which also returns no artists — reported not-found too. The artwork worker settles an entity as "no image" on agents.ErrNotFound. So being throttled did not make Navidrome back off; it made it record the artist as having no artwork, and move on to do the same to the next one. Errors are now parsed out of the body regardless of status, and the quota code is joined with agents.RetryLaterError so the circuit breaker and the artwork retry budget see a throttle for what it is. The agent checks err before the empty-result case. Last.fm already handles this exact shape (client.go errCodeRateLimit, with a comment noting the 200-with-body-error pattern); this brings Deezer in line with it, including the zero-delay RetryLaterError so both providers share the default cooldown rather than a per-provider number. Measured against the live API to pin the shape: a 120-request burst returned 54 results and 66 quota replies, every one of them HTTP 200 with {"error":{"type":"Exception","message":"Quota limit exceeded","code":4}} and no Retry-After or rate-limit headers. A single request 5s later succeeded, so the window is short and a cooldown fully clears it. * refactor(deezer): fold the error envelope into one type The envelope declared the code and message inline, parseBodyError copied them field by field into a second struct with the same shape, and a zero Code stood in for "no error reported". Making the envelope hold a pointer to the error type removes all three: absent is nil, present is the error itself, and the value returned needs no conversion. searchArtist loses its empty-result branch. searchArtists converts an empty result to errNotFound and returns early on any error, so it never answers with no artists and no error, and the branch could not run. What it left behind was a comment explaining an ordering that only mattered while the branch existed. ErrNotFound is unexported: nothing outside this package referenced it, and it sat three lines from agents.ErrNotFound, which is a different error with the opposite meaning for callers. Throttling now joins agents.ErrRetryLater, the sentinel documented as the zero-delay RetryLaterError, rather than allocating an equivalent value. * refactor(deezer): return agents.ErrNotFound from the client The client raised a package-local sentinel that the agent then translated into agents.ErrNotFound, one call site each. Deezer was the only adapter carrying its own: last.fm and listenbrainz have none. The client already reports throttling with agents.ErrRetryLater, so it already speaks the agent vocabulary; saying "not found" in the same words costs nothing and lets searchArtist drop to plain error propagation. * test(scrobbler): remove a race in the longest-server-delay test newBufferedScrobbler starts its drain goroutine, and run() drains once before it ever waits on the wake signal. The test enqueued user2, then enqueued user1 via Scrobble, so that startup drain could land between the two: it saw only user2, took its 45s delay, and set backingOff. The wake from the second enqueue is then deliberately ignored — a wake during a backoff window must not drain, which is the hammering the window exists to prevent — so user1 was never attempted and the first assertion read 1 instead of 2. Buffering both users before the goroutine exists removes the window. The test no longer goes through Scrobble, which the sibling tests already cover; what this one is about is which delay wins. Reproduced deterministically by forcing the interleaving with a synctest.Wait between the two enqueues, which fails with the same "expected both users drained, got 1 attempts" seen in CI. With both enqueued first, that same forced drain passes.
2026-09-01 17:17:52 -04:00
Expect(err).To(MatchError(agents.ErrNotFound))
})
// Deezer answers 200 with no rate-limit headers when throttling, so this body is the only signal.
It("reports an exhausted quota as a retryable error, not as a missing artist", func() {
httpClient.mock("https://api.deezer.com/search/artist", http.Response{
StatusCode: 200,
Body: io.NopCloser(bytes.NewBufferString(
`{"error":{"type":"Exception","message":"Quota limit exceeded","code":4}}`)),
})
_, err := client.searchArtists(GinkgoT().Context(), "Michael Jackson", 20)
Expect(err).To(HaveOccurred())
Expect(err).ToNot(MatchError(agents.ErrNotFound),
"a throttled lookup would otherwise settle the artist as having no image")
Expect(errors.Is(err, agents.ErrRetryLater)).To(BeTrue())
Expect(err.Error()).To(ContainSubstring("Quota limit exceeded"))
})
It("reports a non-quota body error as a plain error", func() {
httpClient.mock("https://api.deezer.com/search/artist", http.Response{
StatusCode: 200,
Body: io.NopCloser(bytes.NewBufferString(
`{"error":{"type":"Exception","message":"Invalid query","code":100}}`)),
})
_, err := client.searchArtists(GinkgoT().Context(), "Michael Jackson", 20)
Expect(err).To(HaveOccurred())
Expect(err).ToNot(MatchError(agents.ErrNotFound))
Expect(errors.Is(err, agents.ErrRetryLater)).To(BeFalse(),
"only a throttle asks the caller to come back later")
})
})
2025-11-21 15:09:24 -05:00
feat: add similar songs functionality in agents, and Instant Mix (song-based) to UI (#4919) * refactor: rename ArtistRadio to SimilarSongs for clarity and consistency Signed-off-by: Deluan <deluan@navidrome.org> * feat: implement GetSimilarSongsByTrack and related functionality for song similarity retrieval Signed-off-by: Deluan <deluan@navidrome.org> * feat: enhance GetSimilarSongsByTrack to include artist and album details and update tests Signed-off-by: Deluan <deluan@navidrome.org> * feat: enhance song matching by implementing title and artist filtering in loadTracksByTitleAndArtist Signed-off-by: Deluan <deluan@navidrome.org> * test: add unit tests for song matching functionality in provider Signed-off-by: Deluan <deluan@navidrome.org> * refactor: extract song matching functionality into its own file Signed-off-by: Deluan <deluan@navidrome.org> * docs: clarify similarSongsFallback function description in provider.go Signed-off-by: Deluan <deluan@navidrome.org> * refactor: initialize result slice for songs with capacity based on response length Signed-off-by: Deluan <deluan@navidrome.org> * refactor: simplify agent method calls for retrieving images and similar songs Signed-off-by: Deluan <deluan@navidrome.org> * refactor: simplify agent method calls for retrieving images and similar songs Signed-off-by: Deluan <deluan@navidrome.org> * refactor: remove outdated comments in GetSimilarSongs methods Signed-off-by: Deluan <deluan@navidrome.org> * fix: use composite key for song matches to handle duplicates by title and artist Signed-off-by: Deluan <deluan@navidrome.org> * refactor: consolidate expectations setup for similar songs tests Signed-off-by: Deluan <deluan@navidrome.org> * feat: add instant mix action to song context menu and update translations Signed-off-by: Deluan <deluan@navidrome.org> * fix(provider): handle unknown entity types in GetSimilarSongs Signed-off-by: Deluan <deluan@navidrome.org> * refactor: move playSimilar action to playbackActions and streamline song processing Signed-off-by: Deluan <deluan@navidrome.org> * format Signed-off-by: Deluan <deluan@navidrome.org> * feat: enhance instant mix functionality with loading notification and shuffle option Signed-off-by: Deluan <deluan@navidrome.org> * feat: implement fuzzy matching for similar songs based on configurable threshold Signed-off-by: Deluan <deluan@navidrome.org> * refactor: implement track matching with multiple specificity levels Signed-off-by: Deluan <deluan@navidrome.org> * refactor: enhance track matching by implementing unified scoring with specificity levels Signed-off-by: Deluan <deluan@navidrome.org> * feat: enhance deezer top tracks result with album Signed-off-by: Deluan <deluan@navidrome.org> * feat: enhance track matching with fuzzy album similarity for improved scoring Signed-off-by: Deluan <deluan@navidrome.org> * docs: document multi-phase song matching algorithm with detailed scoring and prioritization Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org>
2026-01-25 16:16:43 -05:00
Describe("TopTracks", func() {
It("returns top tracks with artist and album info from a successful request", func() {
f, err := os.Open("tests/fixtures/deezer.artist.top.json")
Expect(err).To(BeNil())
httpClient.mock("https://api.deezer.com/artist/27/top", http.Response{Body: f, StatusCode: 200})
tracks, err := client.getTopTracks(GinkgoT().Context(), 27, 5)
Expect(err).To(BeNil())
Expect(tracks).To(HaveLen(5))
// Verify first track has all expected fields
Expect(tracks[0].Title).To(Equal("Instant Crush (feat. Julian Casablancas)"))
Expect(tracks[0].Artist.Name).To(Equal("Daft Punk"))
Expect(tracks[0].Album.Title).To(Equal("Random Access Memories"))
// Verify second track
Expect(tracks[1].Title).To(Equal("One More Time"))
Expect(tracks[1].Artist.Name).To(Equal("Daft Punk"))
Expect(tracks[1].Album.Title).To(Equal("Discovery"))
})
})
2025-11-21 15:09:24 -05:00
Describe("ArtistBio", func() {
BeforeEach(func() {
// Mock the JWT token endpoint with a valid JWT that expires in 5 minutes
testJWT := createTestJWT(5 * time.Minute)
httpClient.mock("https://auth.deezer.com/login/anonymous", http.Response{
StatusCode: 200,
Body: io.NopCloser(bytes.NewBufferString(fmt.Sprintf(`{"jwt":"%s","refresh_token":""}`, testJWT))),
})
})
It("returns artist bio from a successful request", func() {
f, err := os.Open("tests/fixtures/deezer.artist.bio.en.json")
2025-11-21 15:09:24 -05:00
Expect(err).To(BeNil())
httpClient.mock("https://pipe.deezer.com/api", http.Response{Body: f, StatusCode: 200})
bio, err := client.getArtistBio(GinkgoT().Context(), 27, "en")
2025-11-21 15:09:24 -05:00
Expect(err).To(BeNil())
Expect(bio).To(ContainSubstring("Schoolmates Thomas and Guy-Manuel"))
Expect(bio).ToNot(ContainSubstring("<p>"))
Expect(bio).ToNot(ContainSubstring("</p>"))
})
It("uses the provided language", func() {
f, err := os.Open("tests/fixtures/deezer.artist.bio.fr.json")
2025-11-21 15:09:24 -05:00
Expect(err).To(BeNil())
httpClient.mock("https://pipe.deezer.com/api", http.Response{Body: f, StatusCode: 200})
_, err = client.getArtistBio(GinkgoT().Context(), 27, "fr")
2025-11-21 15:09:24 -05:00
Expect(err).To(BeNil())
Expect(httpClient.lastRequest.Header.Get("Accept-Language")).To(Equal("fr"))
})
It("includes the JWT token in the request", func() {
f, err := os.Open("tests/fixtures/deezer.artist.bio.en.json")
2025-11-21 15:09:24 -05:00
Expect(err).To(BeNil())
httpClient.mock("https://pipe.deezer.com/api", http.Response{Body: f, StatusCode: 200})
_, err = client.getArtistBio(GinkgoT().Context(), 27, "en")
2025-11-21 15:09:24 -05:00
Expect(err).To(BeNil())
// Verify that the Authorization header has the Bearer token format
authHeader := httpClient.lastRequest.Header.Get("Authorization")
Expect(authHeader).To(HavePrefix("Bearer "))
Expect(len(authHeader)).To(BeNumerically(">", 20)) // JWT tokens are longer than 20 chars
})
It("handles GraphQL errors", func() {
errorResponse := `{
"data": {
"artist": {
"bio": {
"full": ""
}
}
},
"errors": [
{
"message": "Artist not found"
},
{
"message": "Invalid artist ID"
}
]
}`
httpClient.mock("https://pipe.deezer.com/api", http.Response{
StatusCode: 200,
Body: io.NopCloser(bytes.NewBufferString(errorResponse)),
})
_, err := client.getArtistBio(GinkgoT().Context(), 999, "en")
2025-11-21 15:09:24 -05:00
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("GraphQL error"))
Expect(err.Error()).To(ContainSubstring("Artist not found"))
Expect(err.Error()).To(ContainSubstring("Invalid artist ID"))
})
It("handles empty biography", func() {
emptyBioResponse := `{
"data": {
"artist": {
"bio": {
"full": ""
}
}
}
}`
httpClient.mock("https://pipe.deezer.com/api", http.Response{
StatusCode: 200,
Body: io.NopCloser(bytes.NewBufferString(emptyBioResponse)),
})
_, err := client.getArtistBio(GinkgoT().Context(), 27, "en")
2025-11-21 15:09:24 -05:00
Expect(err).To(MatchError("deezer: biography not found"))
})
It("handles JWT token fetch failure", func() {
httpClient.mock("https://auth.deezer.com/login/anonymous", http.Response{
StatusCode: 500,
Body: io.NopCloser(bytes.NewBufferString(`{"error":"Internal server error"}`)),
})
_, err := client.getArtistBio(GinkgoT().Context(), 27, "en")
2025-11-21 15:09:24 -05:00
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("failed to get JWT"))
})
It("handles JWT token that expires too soon", func() {
// Create a JWT that expires in 30 seconds (less than the 1-minute buffer)
expiredJWT := createTestJWT(30 * time.Second)
httpClient.mock("https://auth.deezer.com/login/anonymous", http.Response{
StatusCode: 200,
Body: io.NopCloser(bytes.NewBufferString(fmt.Sprintf(`{"jwt":"%s","refresh_token":""}`, expiredJWT))),
})
_, err := client.getArtistBio(GinkgoT().Context(), 27, "en")
2025-11-21 15:09:24 -05:00
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("JWT token already expired or expires too soon"))
})
})
})
type fakeHttpClient struct {
responses map[string]*http.Response
lastRequest *http.Request
}
func (c *fakeHttpClient) mock(url string, response http.Response) {
if c.responses == nil {
c.responses = make(map[string]*http.Response)
}
c.responses[url] = &response
}
func (c *fakeHttpClient) Do(req *http.Request) (*http.Response, error) {
c.lastRequest = req
u := req.URL
u.RawQuery = ""
if resp, ok := c.responses[u.String()]; ok {
return resp, nil
}
panic("URL not mocked: " + u.String())
}