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

2026-05-08 07:52:28 -04:00
parent b4edda5518
commit 13680221de
1 changed files with 14 additions and 147 deletions
@@ -2,15 +2,10 @@ package coverart

 import (
 	"context"
-	"encoding/json"
-	"fmt"
-	"io"
-	"math/rand"
 	"net/http"
 	"net/url"
 	"path"
 	"strings"
-	"sync"
 	"sync/atomic"
 	"time"
 )
@@ -44,16 +39,20 @@ var lastfmPlaceholderHashes = map[string]bool{
 // operator-supplied API key — provider stays effectively disabled
 // (returns ErrNotFound on every call) when key is empty.
 type lastfmProvider struct {
-	enabled  atomic.Bool
-	apiKey   atomic.Pointer[string]
-	mu       sync.Mutex
-	lastCall time.Time
-	client   *http.Client
+	enabled atomic.Bool
+	apiKey  atomic.Pointer[string]
+	client  *httpClient
 }

 func init() {
 	Register(&lastfmProvider{
-		client: &http.Client{Timeout: 30 * time.Second},
+		client: newHTTPClient(httpClientOptions{
+			Name:                 "lastfm",
+			HTTPClient:           &http.Client{Timeout: 30 * time.Second},
+			MinInterval:          lastfmMinPeriod,
+			MaxRetries:           lastfmMaxRetries,
+			TreatAuthAsTransient: true,
+		}),
 	})
 }

@@ -112,7 +111,7 @@ func (p *lastfmProvider) FetchArtistArt(ctx context.Context, ref ArtistRef) (thu
 		} `json:"artist"`
 		Error int `json:"error"` // present on error responses
 	}
-	if err := p.getJSON(ctx, q, &resp); err != nil {
+	if err := p.client.getJSON(ctx, lastfmBaseURL+"?"+q.Encode(), &resp); err != nil {
 		return nil, nil, err
 	}
 	if resp.Error != 0 || resp.Artist.Name == "" {
@@ -122,7 +121,7 @@ func (p *lastfmProvider) FetchArtistArt(ctx context.Context, ref ArtistRef) (thu
 	if imgURL == "" || isLastfmPlaceholder(imgURL) {
 		return nil, nil, ErrNotFound
 	}
-	img, ferr := p.getImage(ctx, imgURL)
+	img, ferr := p.client.getImage(ctx, imgURL)
 	if ferr != nil {
 		return nil, nil, ferr
 	}
@@ -158,7 +157,7 @@ func (p *lastfmProvider) FetchAlbumCover(ctx context.Context, ref AlbumRef) ([]b
 		} `json:"album"`
 		Error int `json:"error"`
 	}
-	if err := p.getJSON(ctx, q, &resp); err != nil {
+	if err := p.client.getJSON(ctx, lastfmBaseURL+"?"+q.Encode(), &resp); err != nil {
 		return nil, err
 	}
 	if resp.Error != 0 || resp.Album.Name == "" {
@@ -168,7 +167,7 @@ func (p *lastfmProvider) FetchAlbumCover(ctx context.Context, ref AlbumRef) ([]b
 	if imgURL == "" || isLastfmPlaceholder(imgURL) {
 		return nil, ErrNotFound
 	}
-	return p.getImage(ctx, imgURL)
+	return p.client.getImage(ctx, imgURL)
 }

 // lastfmImage models one entry of Last.fm's image[] array.
@@ -210,138 +209,6 @@ func isLastfmPlaceholder(rawURL string) bool {
 	return lastfmPlaceholderHashes[strings.ToLower(base)]
 }

-// getJSON performs a rate-limited GET against Last.fm's JSON API.
-// Retries on 429 / 5xx with exponential backoff up to lastfmMaxRetries.
-func (p *lastfmProvider) getJSON(ctx context.Context, q url.Values, out any) error {
-	urlStr := lastfmBaseURL + "?" + q.Encode()
-	for attempt := 0; attempt <= lastfmMaxRetries; attempt++ {
-		if err := p.rateLimitWait(ctx); err != nil {
-			return err
-		}
-		req, err := http.NewRequestWithContext(ctx, http.MethodGet, urlStr, nil)
-		if err != nil {
-			return fmt.Errorf("coverart lastfm: build request: %w", err)
-		}
-		req.Header.Set("Accept", "application/json")
-
-		resp, err := p.client.Do(req)
-		if err != nil {
-			if attempt < lastfmMaxRetries {
-				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 < lastfmMaxRetries {
-				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))
-		_ = 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. 404 → ErrNotFound;
-// 5xx / network → ErrTransient.
-func (p *lastfmProvider) getImage(ctx context.Context, urlStr string) ([]byte, error) {
-	if err := p.rateLimitWait(ctx); err != nil {
-		return nil, err
-	}
-	req, err := http.NewRequestWithContext(ctx, http.MethodGet, urlStr, nil)
-	if err != nil {
-		return nil, fmt.Errorf("coverart lastfm: 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))
-	if err != nil {
-		return nil, fmt.Errorf("%w: read body: %v", ErrTransient, err)
-	}
-	return body, nil
-}
-
-// rateLimitWait blocks until at least lastfmMinPeriod has elapsed since
-// the last call, then claims the slot.
-func (p *lastfmProvider) rateLimitWait(ctx context.Context) error {
-	p.mu.Lock()
-	wait := time.Until(p.lastCall.Add(lastfmMinPeriod))
-	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.
-func (p *lastfmProvider) 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
-	}
-}
-
 // Compile-time checks: lastfmProvider satisfies both capability interfaces.
 var (
 	_ AlbumCoverProvider = (*lastfmProvider)(nil)