From 238920540f7c6d9897233aaa2569a599a2686029 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 7 May 2026 07:51:56 -0400 Subject: [PATCH] feat(server/coverart): Last.fm artist + album provider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- internal/coverart/provider_lastfm.go | 349 +++++++++++++++ internal/coverart/provider_lastfm_test.go | 500 ++++++++++++++++++++++ 2 files changed, 849 insertions(+) create mode 100644 internal/coverart/provider_lastfm.go create mode 100644 internal/coverart/provider_lastfm_test.go diff --git a/internal/coverart/provider_lastfm.go b/internal/coverart/provider_lastfm.go new file mode 100644 index 00000000..9a253409 --- /dev/null +++ b/internal/coverart/provider_lastfm.go @@ -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<= 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) + } + }) + } +}