425f12db38
Changes the AlbumCoverProvider and ArtistArtProvider interfaces from MBID-only string parameters to ref-struct signatures so name-based providers (Deezer, Last.fm in upcoming tasks) can act on rows without MBIDs. Today, 53 of 213 artists in the dev DB have NULL MBIDs — featured artists, remixers, and compilation contributors created from track-tag splits where the primary-artist MBID is in the file but collaborator IDs aren't. These rows are unreachable by the current MBID-only chain. Refactoring the interface unblocks future name-based providers from acting on them. TheAudioDB and MBCAA keep MBID-only behavior internally: they return ErrNotFound when ref.MBID is empty rather than attempting a name-based fallback. The MBID guard inside EnrichAlbum/EnrichArtist is removed; providers receive the ref unconditionally and decide whether they can act on it. GetAlbumWithFirstTrackPath now JOINs artists to return artist_name alongside the existing fields, supporting AlbumRef construction. No behavioral change today (only TheAudioDB + MBCAA registered, both keep MBID-only behavior). The change unblocks T3 (Deezer) and T4 (Last.fm) which can now register and act on every artist/album regardless of MBID presence. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
169 lines
4.8 KiB
Go
169 lines
4.8 KiB
Go
package coverart
|
|
|
|
import (
|
|
"context"
|
|
"errors"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"strings"
|
|
"testing"
|
|
"time"
|
|
)
|
|
|
|
// newMBCAAProviderForTest registers a fresh mbcaaProvider pointing at
|
|
// a test server.
|
|
func newMBCAAProviderForTest(t *testing.T, mockURL string) *mbcaaProvider {
|
|
t.Helper()
|
|
resetRegistryForTests()
|
|
t.Cleanup(resetRegistryForTests)
|
|
|
|
p := newMBCAAProvider(FetcherConfig{
|
|
BaseURL: mockURL,
|
|
UserAgent: "Minstrel/test (https://example.com)",
|
|
MinPeriod: 0,
|
|
HTTPClient: &http.Client{Timeout: 5 * time.Second},
|
|
})
|
|
Register(p)
|
|
p.enabled.Store(true)
|
|
return p
|
|
}
|
|
|
|
func TestMBCAAProvider_ID(t *testing.T) {
|
|
resetRegistryForTests()
|
|
defer resetRegistryForTests()
|
|
p := newMBCAAProvider(FetcherConfig{})
|
|
if p.ID() != "mbcaa" {
|
|
t.Errorf("ID = %q, want mbcaa", p.ID())
|
|
}
|
|
if p.DisplayName() != "Cover Art Archive" {
|
|
t.Errorf("DisplayName = %q, want Cover Art Archive", p.DisplayName())
|
|
}
|
|
if p.RequiresAPIKey() {
|
|
t.Error("RequiresAPIKey = true, want false")
|
|
}
|
|
if !p.DefaultEnabled() {
|
|
t.Error("DefaultEnabled = false, want true")
|
|
}
|
|
}
|
|
|
|
func TestMBCAAProvider_FetchAlbumCover_Success(t *testing.T) {
|
|
imageBody := []byte("FAKE_JPEG")
|
|
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: "+r.URL.Path, http.StatusBadRequest)
|
|
return
|
|
}
|
|
w.Header().Set("Content-Type", "image/jpeg")
|
|
_, _ = w.Write(imageBody)
|
|
}))
|
|
defer srv.Close()
|
|
|
|
p := newMBCAAProviderForTest(t, srv.URL)
|
|
body, err := p.FetchAlbumCover(context.Background(), AlbumRef{MBID: "abc-123"})
|
|
if err != nil {
|
|
t.Fatalf("FetchAlbumCover err = %v, want nil", err)
|
|
}
|
|
if string(body) != string(imageBody) {
|
|
t.Errorf("body = %q, want %q", body, imageBody)
|
|
}
|
|
}
|
|
|
|
func TestMBCAAProvider_FetchAlbumCover_NotFound(t *testing.T) {
|
|
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
|
http.Error(w, "not found", http.StatusNotFound)
|
|
}))
|
|
defer srv.Close()
|
|
|
|
p := newMBCAAProviderForTest(t, srv.URL)
|
|
_, err := p.FetchAlbumCover(context.Background(), AlbumRef{MBID: "missing"})
|
|
if !errors.Is(err, ErrNotFound) {
|
|
t.Errorf("err = %v, want ErrNotFound", err)
|
|
}
|
|
}
|
|
|
|
func TestMBCAAProvider_FetchAlbumCover_DisabledReturnsNotFound(t *testing.T) {
|
|
srv := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {
|
|
t.Error("server should not be hit when provider is disabled")
|
|
}))
|
|
defer srv.Close()
|
|
|
|
p := newMBCAAProviderForTest(t, srv.URL)
|
|
p.enabled.Store(false)
|
|
|
|
_, err := p.FetchAlbumCover(context.Background(), AlbumRef{MBID: "any"})
|
|
if !errors.Is(err, ErrNotFound) {
|
|
t.Errorf("err = %v, want ErrNotFound (disabled)", err)
|
|
}
|
|
}
|
|
|
|
func TestMBCAAProvider_Configure_TogglesEnabled(t *testing.T) {
|
|
resetRegistryForTests()
|
|
defer resetRegistryForTests()
|
|
p := newMBCAAProvider(FetcherConfig{})
|
|
|
|
if err := p.Configure(ProviderSettings{Enabled: true}); err != nil {
|
|
t.Fatalf("Configure: %v", err)
|
|
}
|
|
if !p.enabled.Load() {
|
|
t.Error("after Configure(Enabled: true), enabled = false")
|
|
}
|
|
|
|
if err := p.Configure(ProviderSettings{Enabled: false}); err != nil {
|
|
t.Fatalf("Configure: %v", err)
|
|
}
|
|
if p.enabled.Load() {
|
|
t.Error("after Configure(Enabled: false), enabled = true")
|
|
}
|
|
}
|
|
|
|
func TestMBCAAProvider_TestConnection_TreatsNotFoundAsSuccess(t *testing.T) {
|
|
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
|
http.Error(w, "not found", http.StatusNotFound)
|
|
}))
|
|
defer srv.Close()
|
|
|
|
p := newMBCAAProviderForTest(t, srv.URL)
|
|
if err := p.TestConnection(context.Background()); err != nil {
|
|
t.Errorf("TestConnection err = %v, want nil (404 should be treated as connection ok)", err)
|
|
}
|
|
}
|
|
|
|
func TestMBCAAProvider_TestConnection_PropagatesTransient(t *testing.T) {
|
|
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
|
http.Error(w, "boom", http.StatusInternalServerError)
|
|
}))
|
|
defer srv.Close()
|
|
|
|
p := newMBCAAProviderForTest(t, srv.URL)
|
|
if err := p.TestConnection(context.Background()); err == nil {
|
|
t.Error("TestConnection err = nil, want non-nil for 500")
|
|
}
|
|
}
|
|
|
|
func TestNewMBCAAProviderFromConfig_ReplacesRegisteredConfig(t *testing.T) {
|
|
resetRegistryForTests()
|
|
defer resetRegistryForTests()
|
|
|
|
// Pre-register with default config (mimics init() behaviour).
|
|
Register(newMBCAAProvider(FetcherConfig{}))
|
|
|
|
cfg := FetcherConfig{
|
|
UserAgent: "Minstrel/test-replaced (x)",
|
|
MinPeriod: 100 * time.Millisecond,
|
|
}
|
|
NewMBCAAProviderFromConfig(cfg)
|
|
|
|
p, err := ProviderByID("mbcaa")
|
|
if err != nil {
|
|
t.Fatalf("ProviderByID: %v", err)
|
|
}
|
|
mp := p.(*mbcaaProvider)
|
|
// Verify the cfg was replaced on the registered instance.
|
|
mp.mu.Lock()
|
|
gotUA := mp.cfg.UserAgent
|
|
mp.mu.Unlock()
|
|
if gotUA != "Minstrel/test-replaced (x)" {
|
|
t.Errorf("cfg.UserAgent = %q, want %q", gotUA, "Minstrel/test-replaced (x)")
|
|
}
|
|
}
|