feat(server/coverart): Last.fm artist + album provider

Adds Last.fm as a hybrid MBID-or-name provider in the artist + album chains. Default-disabled; activates when the operator pastes a free API key into the admin Cover Art panel. Rate-limited to 4 req/s under their 5/s per-key ceiling. Prefers MBID lookup when present (artist.getInfo accepts &mbid=), falls back to artist-name search. For albums, requires both ArtistName and AlbumTitle; MBID is sent as an additional hint. Includes placeholder-image detection: Last.fm returns a generic "default star" image hash for many artists rather than 404'ing. Provider parses the response URL's basename hash and treats known placeholders as ErrNotFound so we never persist a placeholder thumb as real art. Fail-open: when in doubt, return ErrNotFound rather than risk persisting wrong art. When API key is empty, every call returns ErrNotFound regardless of the enabled flag — defensive so an operator who toggles enabled without setting a key doesn't see confusing transient errors. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-07 07:51:56 -04:00
parent 8f8e88c8b9
commit 238920540f
2 changed files with 849 additions and 0 deletions
@@ -0,0 +1,349 @@
+package coverart
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"io"
+	"math/rand"
+	"net/http"
+	"net/url"
+	"path"
+	"strings"
+	"sync"
+	"sync/atomic"
+	"time"
+)
+
+// lastfmBaseURL is a var (not const) so tests can override it.
+var lastfmBaseURL = "https://ws.audioscrobbler.com/2.0/"
+
+const (
+	// lastfmMinPeriod throttles to 4 req/s under their 5/s per-key ceiling.
+	lastfmMinPeriod = 250 * time.Millisecond
+
+	lastfmMaxRetries = 3
+)
+
+// lastfmPlaceholderHashes is the set of known "default" image basename
+// hashes Last.fm returns instead of 404'ing for artists/albums that
+// don't have art. Provider detects these in the response URL and
+// treats them as ErrNotFound to avoid persisting placeholders as art.
+//
+// The well-known generic-artist hash has been stable for years; if
+// Last.fm ships new placeholder hashes, we'll start persisting them
+// and need to add them here. Fail-open: when in doubt, return
+// ErrNotFound rather than risk persisting a placeholder.
+var lastfmPlaceholderHashes = map[string]bool{
+	"2a96cbd8b46e442fc41c2b86b821562f": true, // generic artist star
+	"c6f59c1e5e7240a4c0d427abd71f3dbb": true, // generic album cover
+}
+
+// lastfmProvider implements Provider, AlbumCoverProvider, and
+// ArtistArtProvider against Last.fm's audioscrobbler API. Requires an
+// 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
+}
+
+func init() {
+	Register(&lastfmProvider{
+		client: &http.Client{Timeout: 30 * time.Second},
+	})
+}
+
+func (p *lastfmProvider) ID() string           { return "lastfm" }
+func (p *lastfmProvider) DisplayName() string  { return "Last.fm" }
+func (p *lastfmProvider) RequiresAPIKey() bool { return true }
+func (p *lastfmProvider) DefaultEnabled() bool { return false }
+
+func (p *lastfmProvider) Configure(s ProviderSettings) error {
+	p.enabled.Store(s.Enabled)
+	key := s.APIKey
+	p.apiKey.Store(&key)
+	return nil
+}
+
+// currentKey returns the configured API key (empty string when unset).
+func (p *lastfmProvider) currentKey() string {
+	if k := p.apiKey.Load(); k != nil {
+		return *k
+	}
+	return ""
+}
+
+// FetchArtistArt prefers MBID-based lookup (artist.getInfo accepts
+// &mbid=) and falls back to artist-name search. Returns ErrNotFound
+// on:
+//   - provider disabled OR API key empty
+//   - both MBID and Name empty
+//   - Last.fm returns no image OR returns a known placeholder URL
+//
+// Last.fm has a single artist image (no separate fanart) — return
+// it as thumb, leave fanart nil.
+func (p *lastfmProvider) FetchArtistArt(ctx context.Context, ref ArtistRef) (thumb, fanart []byte, err error) {
+	if !p.enabled.Load() || p.currentKey() == "" {
+		return nil, nil, ErrNotFound
+	}
+	if ref.MBID == "" && ref.Name == "" {
+		return nil, nil, ErrNotFound
+	}
+
+	q := url.Values{
+		"method":  {"artist.getInfo"},
+		"api_key": {p.currentKey()},
+		"format":  {"json"},
+	}
+	if ref.MBID != "" {
+		q.Set("mbid", ref.MBID)
+	} else {
+		q.Set("artist", ref.Name)
+	}
+
+	var resp struct {
+		Artist struct {
+			Name  string        `json:"name"`
+			Image []lastfmImage `json:"image"`
+		} `json:"artist"`
+		Error int `json:"error"` // present on error responses
+	}
+	if err := p.getJSON(ctx, q, &resp); err != nil {
+		return nil, nil, err
+	}
+	if resp.Error != 0 || resp.Artist.Name == "" {
+		return nil, nil, ErrNotFound
+	}
+	imgURL := pickLargestImage(resp.Artist.Image)
+	if imgURL == "" || isLastfmPlaceholder(imgURL) {
+		return nil, nil, ErrNotFound
+	}
+	img, ferr := p.getImage(ctx, imgURL)
+	if ferr != nil {
+		return nil, nil, ferr
+	}
+	return img, nil, nil
+}
+
+// FetchAlbumCover requires both ArtistName and AlbumTitle (album.getInfo
+// has no MBID-only mode that's reliable). MBID is sent as an additional
+// hint when present.
+func (p *lastfmProvider) FetchAlbumCover(ctx context.Context, ref AlbumRef) ([]byte, error) {
+	if !p.enabled.Load() || p.currentKey() == "" {
+		return nil, ErrNotFound
+	}
+	if ref.ArtistName == "" || ref.AlbumTitle == "" {
+		return nil, ErrNotFound
+	}
+
+	q := url.Values{
+		"method":  {"album.getInfo"},
+		"api_key": {p.currentKey()},
+		"format":  {"json"},
+		"artist":  {ref.ArtistName},
+		"album":   {ref.AlbumTitle},
+	}
+	if ref.MBID != "" {
+		q.Set("mbid", ref.MBID)
+	}
+
+	var resp struct {
+		Album struct {
+			Name  string        `json:"name"`
+			Image []lastfmImage `json:"image"`
+		} `json:"album"`
+		Error int `json:"error"`
+	}
+	if err := p.getJSON(ctx, q, &resp); err != nil {
+		return nil, err
+	}
+	if resp.Error != 0 || resp.Album.Name == "" {
+		return nil, ErrNotFound
+	}
+	imgURL := pickLargestImage(resp.Album.Image)
+	if imgURL == "" || isLastfmPlaceholder(imgURL) {
+		return nil, ErrNotFound
+	}
+	return p.getImage(ctx, imgURL)
+}
+
+// lastfmImage models one entry of Last.fm's image[] array.
+type lastfmImage struct {
+	Text string `json:"#text"`
+	Size string `json:"size"`
+}
+
+// pickLargestImage returns the URL of the largest available image.
+// Preference order: mega → extralarge → large → medium → small.
+// Returns empty when no entry has a non-empty URL.
+func pickLargestImage(imgs []lastfmImage) string {
+	pref := []string{"mega", "extralarge", "large", "medium", "small"}
+	bySize := make(map[string]string, len(imgs))
+	for _, im := range imgs {
+		if im.Text != "" {
+			bySize[im.Size] = im.Text
+		}
+	}
+	for _, sz := range pref {
+		if u, ok := bySize[sz]; ok {
+			return u
+		}
+	}
+	return ""
+}
+
+// isLastfmPlaceholder returns true if the URL points at one of the
+// known generic placeholder image hashes.
+func isLastfmPlaceholder(rawURL string) bool {
+	u, err := url.Parse(rawURL)
+	if err != nil {
+		return false
+	}
+	base := path.Base(u.Path)
+	if i := strings.LastIndex(base, "."); i > 0 {
+		base = base[:i]
+	}
+	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)
+	_ ArtistArtProvider  = (*lastfmProvider)(nil)
+)
@@ -0,0 +1,500 @@
+package coverart
+
+import (
+	"context"
+	"errors"
+	"net/http"
+	"net/http/httptest"
+	"sync/atomic"
+	"testing"
+	"time"
+)
+
+// newLastfmTestProvider returns a fresh enabled provider configured
+// with the given base URL and API key, and restores lastfmBaseURL on
+// test cleanup.
+func newLastfmTestProvider(t *testing.T, baseURL, key string) *lastfmProvider {
+	t.Helper()
+	prevBase := lastfmBaseURL
+	lastfmBaseURL = baseURL
+	t.Cleanup(func() { lastfmBaseURL = prevBase })
+
+	p := &lastfmProvider{client: &http.Client{Timeout: 5 * time.Second}}
+	p.enabled.Store(true)
+	p.apiKey.Store(&key)
+	return p
+}
+
+// ── Identity / metadata ──────────────────────────────────────────────────────
+
+func TestLastfm_ID(t *testing.T) {
+	p := &lastfmProvider{}
+	if p.ID() != "lastfm" {
+		t.Errorf("ID = %q, want lastfm", p.ID())
+	}
+	if !p.RequiresAPIKey() {
+		t.Error("RequiresAPIKey = false, want true")
+	}
+	if p.DefaultEnabled() {
+		t.Error("DefaultEnabled = true, want false")
+	}
+}
+
+// ── FetchArtistArt — happy paths ─────────────────────────────────────────────
+
+func TestLastfm_FetchArtistArt_ByMBID(t *testing.T) {
+	mux := http.NewServeMux()
+	srv := httptest.NewServer(mux)
+	defer srv.Close()
+
+	mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Query().Get("mbid") == "" {
+			t.Error("expected mbid= query param, got none")
+		}
+		imgURL := "http://" + r.Host + "/img/a.jpg"
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`{"artist":{"name":"Radiohead","image":[
+			{"#text":"` + imgURL + `","size":"extralarge"}
+		]}}`))
+	})
+	mux.HandleFunc("/img/", func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Set("Content-Type", "image/jpeg")
+		_, _ = w.Write([]byte("artist-bytes"))
+	})
+
+	p := newLastfmTestProvider(t, srv.URL+"/", "test-key")
+	thumb, fanart, err := p.FetchArtistArt(context.Background(), ArtistRef{
+		MBID: "a74b1b7f-71a5-4011-9441-d0b5e4122711",
+		Name: "Radiohead",
+	})
+	if err != nil {
+		t.Fatalf("err = %v, want nil", err)
+	}
+	if string(thumb) != "artist-bytes" {
+		t.Errorf("thumb = %q, want artist-bytes", thumb)
+	}
+	if fanart != nil {
+		t.Errorf("fanart = %v, want nil (Last.fm has no separate fanart)", fanart)
+	}
+}
+
+func TestLastfm_FetchArtistArt_ByName(t *testing.T) {
+	mux := http.NewServeMux()
+	srv := httptest.NewServer(mux)
+	defer srv.Close()
+
+	mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Query().Get("mbid") != "" {
+			t.Error("expected no mbid= when MBID is empty, got one")
+		}
+		if r.URL.Query().Get("artist") == "" {
+			t.Error("expected artist= query param, got none")
+		}
+		imgURL := "http://" + r.Host + "/img/b.jpg"
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`{"artist":{"name":"Björk","image":[
+			{"#text":"` + imgURL + `","size":"large"}
+		]}}`))
+	})
+	mux.HandleFunc("/img/", func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Set("Content-Type", "image/jpeg")
+		_, _ = w.Write([]byte("bjork-bytes"))
+	})
+
+	p := newLastfmTestProvider(t, srv.URL+"/", "test-key")
+	thumb, _, err := p.FetchArtistArt(context.Background(), ArtistRef{Name: "Björk"})
+	if err != nil {
+		t.Fatalf("err = %v, want nil", err)
+	}
+	if string(thumb) != "bjork-bytes" {
+		t.Errorf("thumb = %q, want bjork-bytes", thumb)
+	}
+}
+
+// ── FetchArtistArt — error/edge paths ────────────────────────────────────────
+
+func TestLastfm_FetchArtistArt_PlaceholderImage(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`{"artist":{"name":"X","image":[
+			{"#text":"https://lastfm.freetls.fastly.net/i/u/300x300/2a96cbd8b46e442fc41c2b86b821562f.png","size":"large"}
+		]}}`))
+	}))
+	defer srv.Close()
+
+	p := newLastfmTestProvider(t, srv.URL+"/", "test-key")
+	_, _, err := p.FetchArtistArt(context.Background(), ArtistRef{Name: "X"})
+	if !errors.Is(err, ErrNotFound) {
+		t.Errorf("err = %v, want ErrNotFound for placeholder image", err)
+	}
+}
+
+func TestLastfm_FetchArtistArt_AlbumPlaceholderImage(t *testing.T) {
+	// Second known placeholder hash (album cover default).
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`{"artist":{"name":"Y","image":[
+			{"#text":"https://lastfm.freetls.fastly.net/i/u/300x300/c6f59c1e5e7240a4c0d427abd71f3dbb.jpg","size":"extralarge"}
+		]}}`))
+	}))
+	defer srv.Close()
+
+	p := newLastfmTestProvider(t, srv.URL+"/", "test-key")
+	_, _, err := p.FetchArtistArt(context.Background(), ArtistRef{Name: "Y"})
+	if !errors.Is(err, ErrNotFound) {
+		t.Errorf("err = %v, want ErrNotFound for album placeholder hash", err)
+	}
+}
+
+func TestLastfm_FetchArtistArt_EmptyImageArray(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`{"artist":{"name":"Noimages","image":[]}}`))
+	}))
+	defer srv.Close()
+
+	p := newLastfmTestProvider(t, srv.URL+"/", "test-key")
+	_, _, err := p.FetchArtistArt(context.Background(), ArtistRef{Name: "Noimages"})
+	if !errors.Is(err, ErrNotFound) {
+		t.Errorf("err = %v, want ErrNotFound for empty image array", err)
+	}
+}
+
+func TestLastfm_FetchArtistArt_ErrorResponse(t *testing.T) {
+	// Last.fm error response: {"error": 6, "message": "Artist not found"}
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`{"error":6,"message":"Artist not found"}`))
+	}))
+	defer srv.Close()
+
+	p := newLastfmTestProvider(t, srv.URL+"/", "test-key")
+	_, _, err := p.FetchArtistArt(context.Background(), ArtistRef{Name: "Ghost"})
+	if !errors.Is(err, ErrNotFound) {
+		t.Errorf("err = %v, want ErrNotFound for Last.fm error response", err)
+	}
+}
+
+func TestLastfm_FetchArtistArt_EmptyAPIKey(t *testing.T) {
+	// Enabled but no key: every call must return ErrNotFound, not a network error.
+	p := &lastfmProvider{client: &http.Client{Timeout: 5 * time.Second}}
+	p.enabled.Store(true)
+	emptyKey := ""
+	p.apiKey.Store(&emptyKey)
+
+	_, _, err := p.FetchArtistArt(context.Background(), ArtistRef{Name: "X"})
+	if !errors.Is(err, ErrNotFound) {
+		t.Errorf("err = %v, want ErrNotFound when API key is empty", err)
+	}
+}
+
+func TestLastfm_FetchArtistArt_Disabled(t *testing.T) {
+	// Key set but disabled.
+	p := &lastfmProvider{client: &http.Client{Timeout: 5 * time.Second}}
+	// enabled defaults to false
+	key := "some-key"
+	p.apiKey.Store(&key)
+
+	_, _, err := p.FetchArtistArt(context.Background(), ArtistRef{Name: "X"})
+	if !errors.Is(err, ErrNotFound) {
+		t.Errorf("err = %v, want ErrNotFound when disabled", err)
+	}
+}
+
+func TestLastfm_FetchArtistArt_EmptyRefs(t *testing.T) {
+	// Neither MBID nor Name: ErrNotFound without making any HTTP call.
+	p := &lastfmProvider{client: &http.Client{Timeout: 5 * time.Second}}
+	p.enabled.Store(true)
+	key := "test-key"
+	p.apiKey.Store(&key)
+
+	_, _, err := p.FetchArtistArt(context.Background(), ArtistRef{})
+	if !errors.Is(err, ErrNotFound) {
+		t.Errorf("err = %v, want ErrNotFound for empty ArtistRef", err)
+	}
+}
+
+func TestLastfm_FetchArtistArt_5xxTransient(t *testing.T) {
+	var attempts atomic.Int32
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		attempts.Add(1)
+		w.WriteHeader(http.StatusInternalServerError)
+	}))
+	defer srv.Close()
+
+	p := newLastfmTestProvider(t, srv.URL+"/", "test-key")
+	_, _, err := p.FetchArtistArt(context.Background(), ArtistRef{Name: "X"})
+	if !errors.Is(err, ErrTransient) {
+		t.Errorf("err = %v, want ErrTransient on 500", err)
+	}
+	if attempts.Load() < 2 {
+		t.Errorf("attempts = %d, want >= 2 (retry should have run)", attempts.Load())
+	}
+}
+
+// ── FetchAlbumCover — happy path ─────────────────────────────────────────────
+
+func TestLastfm_FetchAlbumCover_Success(t *testing.T) {
+	mux := http.NewServeMux()
+	srv := httptest.NewServer(mux)
+	defer srv.Close()
+
+	mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
+		imgURL := "http://" + r.Host + "/img/c.jpg"
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`{"album":{"name":"OK Computer","image":[
+			{"#text":"` + imgURL + `","size":"mega"}
+		]}}`))
+	})
+	mux.HandleFunc("/img/", func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Set("Content-Type", "image/jpeg")
+		_, _ = w.Write([]byte("album-bytes"))
+	})
+
+	p := newLastfmTestProvider(t, srv.URL+"/", "test-key")
+	body, err := p.FetchAlbumCover(context.Background(), AlbumRef{
+		ArtistName: "Radiohead",
+		AlbumTitle: "OK Computer",
+	})
+	if err != nil {
+		t.Fatalf("err = %v, want nil", err)
+	}
+	if string(body) != "album-bytes" {
+		t.Errorf("body = %q, want album-bytes", body)
+	}
+}
+
+func TestLastfm_FetchAlbumCover_WithMBIDHint(t *testing.T) {
+	mux := http.NewServeMux()
+	srv := httptest.NewServer(mux)
+	defer srv.Close()
+
+	mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Query().Get("mbid") == "" {
+			t.Error("expected mbid= hint in album query, got none")
+		}
+		imgURL := "http://" + r.Host + "/img/d.jpg"
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`{"album":{"name":"Kid A","image":[
+			{"#text":"` + imgURL + `","size":"extralarge"}
+		]}}`))
+	})
+	mux.HandleFunc("/img/", func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Set("Content-Type", "image/jpeg")
+		_, _ = w.Write([]byte("kida-bytes"))
+	})
+
+	p := newLastfmTestProvider(t, srv.URL+"/", "test-key")
+	body, err := p.FetchAlbumCover(context.Background(), AlbumRef{
+		MBID:       "b80a8a53-c9bf-3a3a-9a8c-bf76bf9a553a",
+		ArtistName: "Radiohead",
+		AlbumTitle: "Kid A",
+	})
+	if err != nil {
+		t.Fatalf("err = %v, want nil", err)
+	}
+	if string(body) != "kida-bytes" {
+		t.Errorf("body = %q, want kida-bytes", body)
+	}
+}
+
+// ── FetchAlbumCover — missing fields ─────────────────────────────────────────
+
+func TestLastfm_FetchAlbumCover_MissingFields(t *testing.T) {
+	p := &lastfmProvider{client: &http.Client{Timeout: 5 * time.Second}}
+	p.enabled.Store(true)
+	key := "test-key"
+	p.apiKey.Store(&key)
+
+	cases := []AlbumRef{
+		{},                // nothing
+		{ArtistName: "X"}, // no title
+		{AlbumTitle: "Y"}, // no artist
+	}
+	for _, ref := range cases {
+		_, err := p.FetchAlbumCover(context.Background(), ref)
+		if !errors.Is(err, ErrNotFound) {
+			t.Errorf("ref=%+v err=%v, want ErrNotFound", ref, err)
+		}
+	}
+}
+
+func TestLastfm_FetchAlbumCover_EmptyAPIKey(t *testing.T) {
+	p := &lastfmProvider{client: &http.Client{Timeout: 5 * time.Second}}
+	p.enabled.Store(true)
+	emptyKey := ""
+	p.apiKey.Store(&emptyKey)
+
+	_, err := p.FetchAlbumCover(context.Background(), AlbumRef{ArtistName: "X", AlbumTitle: "Y"})
+	if !errors.Is(err, ErrNotFound) {
+		t.Errorf("err = %v, want ErrNotFound when API key is empty", err)
+	}
+}
+
+func TestLastfm_FetchAlbumCover_Disabled(t *testing.T) {
+	p := &lastfmProvider{client: &http.Client{Timeout: 5 * time.Second}}
+	// enabled defaults to false
+	key := "some-key"
+	p.apiKey.Store(&key)
+
+	_, err := p.FetchAlbumCover(context.Background(), AlbumRef{ArtistName: "X", AlbumTitle: "Y"})
+	if !errors.Is(err, ErrNotFound) {
+		t.Errorf("err = %v, want ErrNotFound when disabled", err)
+	}
+}
+
+// ── Rate-limit serialization ─────────────────────────────────────────────────
+
+func TestLastfm_RateLimit_Serializes(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		// Return error=6 so both calls resolve to ErrNotFound cleanly.
+		_, _ = w.Write([]byte(`{"error":6,"message":"not found"}`))
+	}))
+	defer srv.Close()
+
+	p := newLastfmTestProvider(t, srv.URL+"/", "test-key")
+
+	start := time.Now()
+	_, _, _ = p.FetchArtistArt(context.Background(), ArtistRef{Name: "A"})
+	_, _, _ = p.FetchArtistArt(context.Background(), ArtistRef{Name: "B"})
+	elapsed := time.Since(start)
+
+	if elapsed < lastfmMinPeriod {
+		t.Errorf("two back-to-back calls completed in %v, want at least %v (rate-limit didn't serialize)",
+			elapsed, lastfmMinPeriod)
+	}
+}
+
+// ── isLastfmPlaceholder — unit tests ─────────────────────────────────────────
+
+func TestIsLastfmPlaceholder(t *testing.T) {
+	cases := []struct {
+		name   string
+		rawURL string
+		want   bool
+	}{
+		{
+			name:   "known artist star hash (png)",
+			rawURL: "https://lastfm.freetls.fastly.net/i/u/300x300/2a96cbd8b46e442fc41c2b86b821562f.png",
+			want:   true,
+		},
+		{
+			name:   "known album cover hash (jpg)",
+			rawURL: "https://lastfm.freetls.fastly.net/i/u/300x300/c6f59c1e5e7240a4c0d427abd71f3dbb.jpg",
+			want:   true,
+		},
+		{
+			name:   "unknown (real) hash",
+			rawURL: "https://lastfm.freetls.fastly.net/i/u/300x300/abcdef1234567890abcdef1234567890.jpg",
+			want:   false,
+		},
+		{
+			name:   "uppercase known hash is still detected",
+			rawURL: "https://lastfm.freetls.fastly.net/i/u/300x300/2A96CBD8B46E442FC41C2B86B821562F.png",
+			want:   true,
+		},
+		{
+			name:   "malformed URL does not panic",
+			rawURL: "://not a valid url",
+			want:   false,
+		},
+		{
+			name:   "URL with query string and fragment",
+			rawURL: "https://lastfm.freetls.fastly.net/i/u/300x300/2a96cbd8b46e442fc41c2b86b821562f.png?size=large#section",
+			want:   true,
+		},
+		{
+			name:   "empty URL",
+			rawURL: "",
+			want:   false,
+		},
+		{
+			name:   "hash in path prefix, not basename, does not match",
+			rawURL: "https://lastfm.freetls.fastly.net/2a96cbd8b46e442fc41c2b86b821562f/real_image.jpg",
+			want:   false,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			got := isLastfmPlaceholder(tc.rawURL)
+			if got != tc.want {
+				t.Errorf("isLastfmPlaceholder(%q) = %v, want %v", tc.rawURL, got, tc.want)
+			}
+		})
+	}
+}
+
+// ── pickLargestImage — unit tests ────────────────────────────────────────────
+
+func TestPickLargestImage(t *testing.T) {
+	cases := []struct {
+		name string
+		imgs []lastfmImage
+		want string
+	}{
+		{
+			name: "mega preferred over all others",
+			imgs: []lastfmImage{
+				{Text: "http://small", Size: "small"},
+				{Text: "http://mega", Size: "mega"},
+				{Text: "http://large", Size: "large"},
+			},
+			want: "http://mega",
+		},
+		{
+			name: "extralarge when no mega",
+			imgs: []lastfmImage{
+				{Text: "http://extralarge", Size: "extralarge"},
+				{Text: "http://large", Size: "large"},
+			},
+			want: "http://extralarge",
+		},
+		{
+			name: "large when no mega or extralarge",
+			imgs: []lastfmImage{
+				{Text: "http://large", Size: "large"},
+				{Text: "http://medium", Size: "medium"},
+			},
+			want: "http://large",
+		},
+		{
+			name: "medium fallback",
+			imgs: []lastfmImage{
+				{Text: "http://medium", Size: "medium"},
+				{Text: "http://small", Size: "small"},
+			},
+			want: "http://medium",
+		},
+		{
+			name: "small last resort",
+			imgs: []lastfmImage{
+				{Text: "http://small", Size: "small"},
+			},
+			want: "http://small",
+		},
+		{
+			name: "empty array returns empty",
+			imgs: []lastfmImage{},
+			want: "",
+		},
+		{
+			name: "entries with empty text are skipped",
+			imgs: []lastfmImage{
+				{Text: "", Size: "mega"},
+				{Text: "http://large", Size: "large"},
+			},
+			want: "http://large",
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			got := pickLargestImage(tc.imgs)
+			if got != tc.want {
+				t.Errorf("pickLargestImage() = %q, want %q", got, tc.want)
+			}
+		})
+	}
+}