feat(server/m7-353): coverart.Fetcher MBCAA HTTP client + rate limiter

2026-05-04 14:49:28 -04:00
parent ef6c09dbfb
commit 2d3841966e
2 changed files with 211 additions and 0 deletions
@@ -0,0 +1,102 @@
+package coverart
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"io"
+	"net/http"
+	"sync"
+	"time"
+)
+
+// ErrNotFound indicates the upstream responded with 404 — the album has no
+// front cover on MBCAA. Caller should record cover_art_source = 'none'.
+var ErrNotFound = errors.New("coverart: not found")
+
+// ErrTransient indicates a temporary failure (5xx, network, timeout). Caller
+// should leave cover_art_source NULL so the next pass can retry.
+var ErrTransient = errors.New("coverart: transient error")
+
+// FetcherConfig assembles the operator-tunable knobs for the MBCAA client.
+type FetcherConfig struct {
+	BaseURL    string        // e.g. "https://coverartarchive.org"
+	UserAgent  string        // e.g. "Minstrel/<version> (<contact>)"
+	MinPeriod  time.Duration // minimum delay between calls (1s default per MBCAA etiquette)
+	HTTPClient *http.Client  // optional override for tests; default below
+}
+
+// Fetcher pulls album front covers from MBCAA. Safe for concurrent use; the
+// internal mutex serialises requests so the rate limit is respected.
+type Fetcher struct {
+	cfg      FetcherConfig
+	mu       sync.Mutex
+	lastCall time.Time
+	client   *http.Client
+}
+
+// NewFetcher builds a Fetcher with sensible defaults.
+func NewFetcher(cfg FetcherConfig) *Fetcher {
+	if cfg.BaseURL == "" {
+		cfg.BaseURL = "https://coverartarchive.org"
+	}
+	client := cfg.HTTPClient
+	if client == nil {
+		client = &http.Client{Timeout: 30 * time.Second}
+	}
+	return &Fetcher{cfg: cfg, client: client}
+}
+
+// Fetch retrieves the 500px front cover for the given release MBID. Returns
+// ErrNotFound on 404 (record as 'none') or ErrTransient otherwise (retry later).
+func (f *Fetcher) Fetch(ctx context.Context, mbid string) ([]byte, error) {
+	if mbid == "" {
+		return nil, fmt.Errorf("coverart: empty mbid")
+	}
+
+	// Rate limit: serialise + wait until MinPeriod has elapsed since the last call.
+	f.mu.Lock()
+	if f.cfg.MinPeriod > 0 {
+		wait := time.Until(f.lastCall.Add(f.cfg.MinPeriod))
+		if wait > 0 {
+			timer := time.NewTimer(wait)
+			f.mu.Unlock()
+			select {
+			case <-ctx.Done():
+				timer.Stop()
+				return nil, ctx.Err()
+			case <-timer.C:
+			}
+			f.mu.Lock()
+		}
+	}
+	f.lastCall = time.Now()
+	f.mu.Unlock()
+
+	url := fmt.Sprintf("%s/release/%s/front-500", f.cfg.BaseURL, mbid)
+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
+	if err != nil {
+		return nil, fmt.Errorf("coverart: build request: %w", err)
+	}
+	req.Header.Set("User-Agent", f.cfg.UserAgent)
+	req.Header.Set("Accept", "image/*")
+
+	resp, err := f.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
+}
@@ -0,0 +1,109 @@
+package coverart
+
+import (
+	"context"
+	"errors"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+	"time"
+)
+
+func TestFetcher_Success(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if !strings.Contains(r.URL.Path, "/release/abc-123/front-500") {
+			http.Error(w, "wrong path", http.StatusBadRequest)
+			return
+		}
+		w.Header().Set("Content-Type", "image/jpeg")
+		_, _ = w.Write([]byte("FAKE_JPEG"))
+	}))
+	defer srv.Close()
+
+	f := NewFetcher(FetcherConfig{
+		BaseURL:    srv.URL,
+		UserAgent:  "Minstrel/test (https://example.com)",
+		MinPeriod:  0,
+		HTTPClient: srv.Client(),
+	})
+
+	body, err := f.Fetch(context.Background(), "abc-123")
+	if err != nil {
+		t.Fatalf("Fetch: %v", err)
+	}
+	if string(body) != "FAKE_JPEG" {
+		t.Errorf("body = %q, want FAKE_JPEG", string(body))
+	}
+}
+
+func TestFetcher_NotFound(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		http.Error(w, "not found", http.StatusNotFound)
+	}))
+	defer srv.Close()
+	f := NewFetcher(FetcherConfig{BaseURL: srv.URL, UserAgent: "test", MinPeriod: 0, HTTPClient: srv.Client()})
+	_, err := f.Fetch(context.Background(), "missing")
+	if !errors.Is(err, ErrNotFound) {
+		t.Errorf("err = %v, want ErrNotFound", err)
+	}
+}
+
+func TestFetcher_5xxIsTransient(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		http.Error(w, "boom", http.StatusInternalServerError)
+	}))
+	defer srv.Close()
+	f := NewFetcher(FetcherConfig{BaseURL: srv.URL, UserAgent: "test", MinPeriod: 0, HTTPClient: srv.Client()})
+	_, err := f.Fetch(context.Background(), "any")
+	if !errors.Is(err, ErrTransient) {
+		t.Errorf("err = %v, want ErrTransient", err)
+	}
+}
+
+func TestFetcher_SendsUserAgent(t *testing.T) {
+	var seen string
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		seen = r.Header.Get("User-Agent")
+		_, _ = w.Write([]byte("ok"))
+	}))
+	defer srv.Close()
+	f := NewFetcher(FetcherConfig{
+		BaseURL:    srv.URL,
+		UserAgent:  "Minstrel/0.1 (https://example.org)",
+		MinPeriod:  0,
+		HTTPClient: srv.Client(),
+	})
+	_, _ = f.Fetch(context.Background(), "ua-test")
+	if seen != "Minstrel/0.1 (https://example.org)" {
+		t.Errorf("UA = %q", seen)
+	}
+}
+
+func TestFetcher_RateLimitSpacesCalls(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		_, _ = w.Write([]byte("ok"))
+	}))
+	defer srv.Close()
+	f := NewFetcher(FetcherConfig{
+		BaseURL:    srv.URL,
+		UserAgent:  "test",
+		MinPeriod:  150 * time.Millisecond,
+		HTTPClient: srv.Client(),
+	})
+	start := time.Now()
+	_, _ = f.Fetch(context.Background(), "a")
+	_, _ = f.Fetch(context.Background(), "b")
+	elapsed := time.Since(start)
+	if elapsed < 150*time.Millisecond {
+		t.Errorf("two calls in %v — rate limiter not enforcing", elapsed)
+	}
+}
+
+func TestFetcher_EmptyMBID(t *testing.T) {
+	f := NewFetcher(FetcherConfig{UserAgent: "test", MinPeriod: 0})
+	_, err := f.Fetch(context.Background(), "")
+	if err == nil {
+		t.Error("expected error for empty mbid")
+	}
+}