From 2d3841966e2fbea106be8f2f8b471317375468d9 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 4 May 2026 14:49:28 -0400 Subject: [PATCH] feat(server/m7-353): coverart.Fetcher MBCAA HTTP client + rate limiter --- internal/coverart/fetcher.go | 102 ++++++++++++++++++++++++++++ internal/coverart/fetcher_test.go | 109 ++++++++++++++++++++++++++++++ 2 files changed, 211 insertions(+) create mode 100644 internal/coverart/fetcher.go create mode 100644 internal/coverart/fetcher_test.go diff --git a/internal/coverart/fetcher.go b/internal/coverart/fetcher.go new file mode 100644 index 00000000..30b029fe --- /dev/null +++ b/internal/coverart/fetcher.go @@ -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/ ()" + 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 +} diff --git a/internal/coverart/fetcher_test.go b/internal/coverart/fetcher_test.go new file mode 100644 index 00000000..35a2d1d7 --- /dev/null +++ b/internal/coverart/fetcher_test.go @@ -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") + } +}