diff --git a/internal/coverart/provider_theaudiodb.go b/internal/coverart/provider_theaudiodb.go index b1d3a252..f25e45a5 100644 --- a/internal/coverart/provider_theaudiodb.go +++ b/internal/coverart/provider_theaudiodb.go @@ -2,13 +2,9 @@ package coverart import ( "context" - "encoding/json" "errors" "fmt" - "io" - "math/rand" "net/http" - "sync" "sync/atomic" "time" ) @@ -43,23 +39,32 @@ const ( // theAudioDBProvider implements Provider, AlbumCoverProvider, // ArtistArtProvider, and TestableProvider against the TheAudioDB -// REST API. Safe for concurrent use; the internal mutex serialises -// ALL HTTP calls (JSON metadata fetches and image GETs) so the -// 2 req/sec rate limit applies uniformly. +// REST API. Safe for concurrent use; the underlying httpClient +// serialises all calls so the 2 req/sec rate limit applies uniformly. type theAudioDBProvider struct { - enabled atomic.Bool - apiKey atomic.Pointer[string] // nil until first Configure; falls back to test key when empty - mu sync.Mutex // serialises ALL HTTP calls for rate-limit compliance - lastCall time.Time - client *http.Client + enabled atomic.Bool + apiKey atomic.Pointer[string] // nil until first Configure; falls back to test key when empty + client *httpClient } func init() { Register(&theAudioDBProvider{ - client: &http.Client{Timeout: 30 * time.Second}, + client: newHTTPClient(httpClientOptions{ + Name: "theaudiodb", + HTTPClient: &http.Client{Timeout: 30 * time.Second}, + MinInterval: theAudioDBMinPeriod, + MaxRetries: theAudioDBMaxRetries, + TreatAuthAsTransient: true, + }), }) } +// jsonURL builds a request URL with the API key embedded in the path +// segment, the auth strategy TheAudioDB uses. +func (p *theAudioDBProvider) jsonURL(path string) string { + return fmt.Sprintf("%s/%s/%s", theAudioDBBaseURL, p.currentKey(), path) +} + func (p *theAudioDBProvider) ID() string { return "theaudiodb" } func (p *theAudioDBProvider) DisplayName() string { return "TheAudioDB" } func (p *theAudioDBProvider) RequiresAPIKey() bool { return true } @@ -101,13 +106,13 @@ func (p *theAudioDBProvider) FetchAlbumCover(ctx context.Context, ref AlbumRef) StrAlbumThumb *string `json:"strAlbumThumb"` } `json:"album"` } - if err := p.getJSON(ctx, "album-mb.php?i="+ref.MBID, &resp); err != nil { + if err := p.client.getJSON(ctx, p.jsonURL("album-mb.php?i="+ref.MBID), &resp); err != nil { return nil, err } if len(resp.Album) == 0 || resp.Album[0].StrAlbumThumb == nil || *resp.Album[0].StrAlbumThumb == "" { return nil, ErrNotFound } - return p.getImage(ctx, *resp.Album[0].StrAlbumThumb) + return p.client.getImage(ctx,*resp.Album[0].StrAlbumThumb) } // FetchArtistArt hits /artist-mb.php?i=, parses strArtistThumb + @@ -135,7 +140,7 @@ func (p *theAudioDBProvider) FetchArtistArt(ctx context.Context, ref ArtistRef) StrArtistFanart *string `json:"strArtistFanart"` } `json:"artists"` } - if err := p.getJSON(ctx, "artist-mb.php?i="+ref.MBID, &resp); err != nil { + if err := p.client.getJSON(ctx, p.jsonURL("artist-mb.php?i="+ref.MBID), &resp); err != nil { return nil, nil, err } if len(resp.Artists) == 0 { @@ -155,7 +160,7 @@ func (p *theAudioDBProvider) FetchArtistArt(ctx context.Context, ref ArtistRef) } if thumbURL != "" { - thumb, err = p.getImage(ctx, thumbURL) + thumb, err = p.client.getImage(ctx,thumbURL) if err != nil && !errors.Is(err, ErrTransient) && !errors.Is(err, ErrNotFound) { return nil, nil, err } @@ -165,7 +170,7 @@ func (p *theAudioDBProvider) FetchArtistArt(ctx context.Context, ref ArtistRef) } } if fanartURL != "" { - fanart, err = p.getImage(ctx, fanartURL) + fanart, err = p.client.getImage(ctx,fanartURL) if err != nil && !errors.Is(err, ErrTransient) && !errors.Is(err, ErrNotFound) { return thumb, nil, err } @@ -197,147 +202,6 @@ func (p *theAudioDBProvider) TestConnection(ctx context.Context) error { IDAlbum *string `json:"idAlbum"` } `json:"album"` } - return p.getJSON(ctx, "album-mb.php?i="+theAudioDBTestSampleMBID, &resp) + return p.client.getJSON(ctx, p.jsonURL("album-mb.php?i="+theAudioDBTestSampleMBID), &resp) } -// getJSON performs a rate-limited GET against the JSON API, decodes -// the response into out, retries on 429 / 5xx with exponential -// backoff up to theAudioDBMaxRetries. -func (p *theAudioDBProvider) getJSON(ctx context.Context, path string, out any) error { - url := fmt.Sprintf("%s/%s/%s", theAudioDBBaseURL, p.currentKey(), path) - - for attempt := 0; attempt <= theAudioDBMaxRetries; attempt++ { - if err := p.rateLimitWait(ctx); err != nil { - return err - } - - req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) - if err != nil { - return fmt.Errorf("coverart: build request: %w", err) - } - req.Header.Set("Accept", "application/json") - - resp, err := p.client.Do(req) - if err != nil { - if attempt < theAudioDBMaxRetries { - if waitErr := p.backoff(ctx, attempt); waitErr != nil { - return waitErr - } - continue - } - return fmt.Errorf("%w: %v", ErrTransient, err) - } - - switch { - case resp.StatusCode == http.StatusNotFound: - _ = resp.Body.Close() - return ErrNotFound - case resp.StatusCode == http.StatusUnauthorized, - resp.StatusCode == http.StatusForbidden: - _ = resp.Body.Close() - return fmt.Errorf("%w: status %d (auth)", ErrTransient, resp.StatusCode) - case resp.StatusCode == http.StatusTooManyRequests || resp.StatusCode >= 500: - _ = resp.Body.Close() - if attempt < theAudioDBMaxRetries { - if waitErr := p.backoff(ctx, attempt); waitErr != nil { - return waitErr - } - continue - } - return fmt.Errorf("%w: status %d", ErrTransient, resp.StatusCode) - case resp.StatusCode != http.StatusOK: - _ = resp.Body.Close() - return fmt.Errorf("%w: status %d", ErrTransient, resp.StatusCode) - } - - body, err := io.ReadAll(io.LimitReader(resp.Body, 1*1024*1024)) // 1MB cap on JSON - _ = resp.Body.Close() - if err != nil { - return fmt.Errorf("%w: read body: %v", ErrTransient, err) - } - if err := json.Unmarshal(body, out); err != nil { - return fmt.Errorf("%w: decode body: %v", ErrTransient, err) - } - return nil - } - return fmt.Errorf("%w: exhausted retries", ErrTransient) -} - -// getImage performs a rate-limited GET of an image URL (CDN, key-less), -// returns the bytes. 404 → ErrNotFound (the URL got stale on -// TheAudioDB's CDN); 5xx / network → ErrTransient. -func (p *theAudioDBProvider) getImage(ctx context.Context, url string) ([]byte, error) { - if err := p.rateLimitWait(ctx); err != nil { - return nil, err - } - - req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) - if err != nil { - return nil, fmt.Errorf("coverart: build request: %w", err) - } - req.Header.Set("Accept", "image/*") - - resp, err := p.client.Do(req) - if err != nil { - return nil, fmt.Errorf("%w: %v", ErrTransient, err) - } - defer func() { _ = resp.Body.Close() }() - - switch { - case resp.StatusCode == http.StatusNotFound: - return nil, ErrNotFound - case resp.StatusCode != http.StatusOK: - return nil, fmt.Errorf("%w: status %d", ErrTransient, resp.StatusCode) - } - - body, err := io.ReadAll(io.LimitReader(resp.Body, 5*1024*1024)) // 5MB cap - if err != nil { - return nil, fmt.Errorf("%w: read body: %v", ErrTransient, err) - } - return body, nil -} - -// rateLimitWait blocks until at least theAudioDBMinPeriod has elapsed -// since the last call, then claims the slot. Returns ctx.Err() if -// the context expires while waiting. -func (p *theAudioDBProvider) rateLimitWait(ctx context.Context) error { - p.mu.Lock() - wait := time.Until(p.lastCall.Add(theAudioDBMinPeriod)) - if wait > 0 { - timer := time.NewTimer(wait) - p.mu.Unlock() - select { - case <-ctx.Done(): - timer.Stop() - return ctx.Err() - case <-timer.C: - } - p.mu.Lock() - } - p.lastCall = time.Now() - p.mu.Unlock() - return nil -} - -// backoff sleeps for 2^attempt × 250ms ± 20% jitter. Returns -// ctx.Err() if the context expires. -func (p *theAudioDBProvider) backoff(ctx context.Context, attempt int) error { - base := time.Duration(250*(1<