refactor(coverart/theaudiodb): delegate HTTP plumbing to httpClient (C1 3/3)

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
+// REST API. Safe for concurrent use; the underlying httpClient
-// ALL HTTP calls (JSON metadata fetches and image GETs) so the
+// serialises all calls so the 2 req/sec rate limit applies uniformly.
-// 2 req/sec rate limit applies uniformly.
 type theAudioDBProvider struct {
-	enabled  atomic.Bool
+	enabled atomic.Bool
-	apiKey   atomic.Pointer[string] // nil until first Configure; falls back to test key when empty
+	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
+	client  *httpClient
-	lastCall time.Time
-	client   *http.Client
 }
 
 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=<mbid>, 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<<attempt)) * time.Millisecond
-	jitterRange := int64(base) / 5
-	if jitterRange <= 0 {
-		jitterRange = 1
-	}
-	jitter := time.Duration(rand.Int63n(jitterRange))
-	if rand.Intn(2) == 0 {
-		jitter = -jitter
-	}
-	d := base + jitter
-	timer := time.NewTimer(d)
-	defer timer.Stop()
-	select {
-	case <-ctx.Done():
-		return ctx.Err()
-	case <-timer.C:
-		return nil
-	}
-}