From b8c758c7c96d83401bb124f97573b18fcbd14849 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 6 May 2026 12:31:00 -0400 Subject: [PATCH] feat(server/m7-cover-sources): Provider interface + registry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Defines the abstraction every cover-art source will implement: - Provider — base interface (ID, DisplayName, RequiresAPIKey, DefaultEnabled, Configure). - AlbumCoverProvider — opt-in capability for fetching album covers by MBID. - ArtistArtProvider — opt-in capability for fetching artist thumb + fanart atomically by MBID. - TestableProvider — opt-in capability for the admin Test-Connection button. Sentinel ErrNotTestable / ErrProviderNotFound. ErrNotFound and ErrTransient stay in fetcher.go for now; they move into provider.go in the next commit when fetcher.go is deleted as part of the MBCAA refactor. Compile-time registry: providers register from init() in their own file. Duplicate IDs panic at startup (compiled-in registration is a build-time bug, not a runtime concern). Tests cover: Register adds, duplicate panics, ProviderByID success + ErrProviderNotFound, capability-interface filtering via Go type assertion. --- internal/coverart/provider.go | 148 +++++++++++++++++++++++++++++ internal/coverart/provider_test.go | 116 ++++++++++++++++++++++ 2 files changed, 264 insertions(+) create mode 100644 internal/coverart/provider.go create mode 100644 internal/coverart/provider_test.go diff --git a/internal/coverart/provider.go b/internal/coverart/provider.go new file mode 100644 index 00000000..728f0e07 --- /dev/null +++ b/internal/coverart/provider.go @@ -0,0 +1,148 @@ +package coverart + +import ( + "context" + "errors" + "fmt" + "sync" +) + +// Provider is the base shape every cover-art source implements. +// Providers register themselves at init() time via Register(). The +// enricher reads the current per-provider settings and dispatches to +// the capability methods below. +type Provider interface { + // ID is a stable lowercase token persisted in DB columns + // (cover_art_source / artist_art_source) and the + // cover_art_provider_settings.provider_id key, plus referenced + // in the operator-facing UI. + ID() string + + // DisplayName is the human-readable label shown in the admin + // Settings card. + DisplayName() string + + // RequiresAPIKey reports whether the provider needs a key to + // function. True for theaudiodb / fanart.tv; false for mbcaa. + // The admin UI uses this to show or hide the api_key text field; + // it does NOT gate the enabled toggle (per the no-coercive- + // settings principle, providers with usable defaults — such as + // TheAudioDB's public test key — ship working). + RequiresAPIKey() bool + + // DefaultEnabled is whether a freshly-installed Minstrel ships + // with this provider on. Both v1 providers return true. + DefaultEnabled() bool + + // Configure receives the operator's settings (api_key, enabled + // flag) and applies them. Called at boot during reconciliation + // and on every admin PATCH. Implementations must be idempotent + // and goroutine-safe (the enricher may be reading concurrently). + Configure(ProviderSettings) error +} + +// ProviderSettings is the runtime config a provider receives when +// Configure() is called. Providers may interpret an empty APIKey +// however they wish — TheAudioDB falls back to its public test key, +// MBCAA ignores the field entirely. +type ProviderSettings struct { + Enabled bool + APIKey string +} + +// AlbumCoverProvider is an opt-in capability: providers implementing +// it can fetch album cover art by MusicBrainz release MBID. +type AlbumCoverProvider interface { + Provider + // FetchAlbumCover returns the image bytes for the given release + // MBID, or ErrNotFound (terminal — no art available) / + // ErrTransient (retry-eligible failure). + FetchAlbumCover(ctx context.Context, mbid string) ([]byte, error) +} + +// ArtistArtProvider is an opt-in capability: providers implementing +// it can fetch artist images by MusicBrainz artist MBID. Returns +// thumb + fanart atomically at the JSON metadata step (one round- +// trip yields both URLs); the two image GETs are independent — +// partial success returns whatever bytes landed and the source is +// still stamped, so the enricher persists asymmetric results +// (thumb-only or fanart-only) gracefully. +type ArtistArtProvider interface { + Provider + FetchArtistArt(ctx context.Context, mbid string) (thumb, fanart []byte, err error) +} + +// TestableProvider is an opt-in capability: providers that can answer +// "is my config working?" without performing a full enrichment cycle. +// Used by the admin Test-Connection button. v1: both MBCAA and +// TheAudioDB implement this. +type TestableProvider interface { + Provider + TestConnection(ctx context.Context) error +} + +// ErrNotTestable is returned by the SettingsService.TestProvider +// method when the named provider doesn't implement TestableProvider. +var ErrNotTestable = errors.New("coverart: provider does not support test connection") + +// ErrProviderNotFound is returned when the named provider is not +// registered. +var ErrProviderNotFound = errors.New("coverart: provider not registered") + +// (Note: ErrNotFound and ErrTransient remain declared in fetcher.go +// during this task. They move to this file in the next commit when +// fetcher.go is deleted as part of the MBCAA refactor.) + +// registry is the package-private list of all compiled-in providers. +// Providers register at init() via Register(). The enricher iterates +// over this list filtered by the current per-provider Settings. +var registry = struct { + mu sync.Mutex + providers []Provider +}{} + +// Register adds a provider to the registry. Must be called from +// init() of a provider file (provider_mbcaa.go, provider_theaudiodb.go, +// etc.). Calling twice with the same ID panics — providers are +// compiled-in, duplicate registration is a build-time bug. +func Register(p Provider) { + registry.mu.Lock() + defer registry.mu.Unlock() + for _, existing := range registry.providers { + if existing.ID() == p.ID() { + panic(fmt.Sprintf("coverart: duplicate provider ID %q", p.ID())) + } + } + registry.providers = append(registry.providers, p) +} + +// AllProviders returns a snapshot of all registered providers in +// registration order. Caller must not mutate the returned slice. +func AllProviders() []Provider { + registry.mu.Lock() + defer registry.mu.Unlock() + out := make([]Provider, len(registry.providers)) + copy(out, registry.providers) + return out +} + +// ProviderByID looks up a registered provider by ID. Returns +// ErrProviderNotFound if no such provider has registered. +func ProviderByID(id string) (Provider, error) { + registry.mu.Lock() + defer registry.mu.Unlock() + for _, p := range registry.providers { + if p.ID() == id { + return p, nil + } + } + return nil, ErrProviderNotFound +} + +// resetRegistryForTests clears the registry. Test-only helper +// (provider_test.go); never called from production code. +func resetRegistryForTests() { + registry.mu.Lock() + defer registry.mu.Unlock() + registry.providers = nil +} diff --git a/internal/coverart/provider_test.go b/internal/coverart/provider_test.go new file mode 100644 index 00000000..55f33423 --- /dev/null +++ b/internal/coverart/provider_test.go @@ -0,0 +1,116 @@ +package coverart + +import ( + "context" + "errors" + "testing" +) + +// fakeProvider is a minimal Provider for registry tests. Does not +// implement any capability interface. +type fakeProvider struct { + id string + display string + requiresKey bool + defaultOn bool +} + +func (f *fakeProvider) ID() string { return f.id } +func (f *fakeProvider) DisplayName() string { return f.display } +func (f *fakeProvider) RequiresAPIKey() bool { return f.requiresKey } +func (f *fakeProvider) DefaultEnabled() bool { return f.defaultOn } +func (f *fakeProvider) Configure(s ProviderSettings) error { return nil } + +// fakeAlbumProvider opts into the AlbumCoverProvider capability for +// the capability-filter test. +type fakeAlbumProvider struct { + fakeProvider +} + +func (f *fakeAlbumProvider) FetchAlbumCover(ctx context.Context, mbid string) ([]byte, error) { + return []byte("img"), nil +} + +func TestRegister_AddsProvider(t *testing.T) { + resetRegistryForTests() + defer resetRegistryForTests() + + p := &fakeProvider{id: "fake1", display: "Fake One"} + Register(p) + + all := AllProviders() + if len(all) != 1 { + t.Fatalf("AllProviders len = %d, want 1", len(all)) + } + if all[0].ID() != "fake1" { + t.Errorf("ID = %q, want %q", all[0].ID(), "fake1") + } +} + +func TestRegister_PanicsOnDuplicateID(t *testing.T) { + resetRegistryForTests() + defer resetRegistryForTests() + + Register(&fakeProvider{id: "dup"}) + + defer func() { + if r := recover(); r == nil { + t.Fatal("expected panic on duplicate ID") + } + }() + Register(&fakeProvider{id: "dup"}) // should panic +} + +func TestProviderByID_Found(t *testing.T) { + resetRegistryForTests() + defer resetRegistryForTests() + + Register(&fakeProvider{id: "x", display: "X"}) + + p, err := ProviderByID("x") + if err != nil { + t.Fatalf("err = %v, want nil", err) + } + if p.DisplayName() != "X" { + t.Errorf("display = %q, want %q", p.DisplayName(), "X") + } +} + +func TestProviderByID_NotFound(t *testing.T) { + resetRegistryForTests() + defer resetRegistryForTests() + + _, err := ProviderByID("missing") + if !errors.Is(err, ErrProviderNotFound) { + t.Errorf("err = %v, want ErrProviderNotFound", err) + } +} + +func TestCapabilityFiltering(t *testing.T) { + resetRegistryForTests() + defer resetRegistryForTests() + + plain := &fakeProvider{id: "plain"} + withAlbum := &fakeAlbumProvider{fakeProvider: fakeProvider{id: "alb"}} + Register(plain) + Register(withAlbum) + + if got := len(AllProviders()); got != 2 { + t.Fatalf("AllProviders len = %d, want 2", got) + } + + // Type-assert to count AlbumCoverProvider implementers — mimics + // what SettingsService.EnabledAlbumProviders does internally. + var albumProviders []AlbumCoverProvider + for _, p := range AllProviders() { + if ap, ok := p.(AlbumCoverProvider); ok { + albumProviders = append(albumProviders, ap) + } + } + if len(albumProviders) != 1 { + t.Fatalf("AlbumCoverProvider count = %d, want 1", len(albumProviders)) + } + if albumProviders[0].ID() != "alb" { + t.Errorf("AlbumCoverProvider[0].ID = %q, want %q", albumProviders[0].ID(), "alb") + } +}