feat(server/m7-cover-sources): SettingsService

Reconciles the compile-time provider registry with the
cover_art_provider_settings table at boot, and on every admin
update. Calls Configure() on each provider with the row data so
runtime config (enabled, api_key) propagates immediately.

Owns the cover_art_sources_meta.current_version stamp.
UpdateProvider returns versionBumped=true only when the change
altered the *enabled set* of providers — key rotations don't bump
version (the recheck trigger is for "we have a new source to ask,"
not config rotations).

Capability filtering: EnabledAlbumProviders / EnabledArtistProviders
return only registered providers that (a) are enabled per DB and
(b) implement the requested capability interface. Snapshots —
callers don't mutate.

ListProviderInfo builds the wire-shaped slice the admin GET handler
returns. APIKeySet bool replaces echoing the key (credential never
leaves the server).

TestProvider dispatches to TestableProvider.TestConnection on the
named provider, ErrNotTestable if the provider doesn't implement
the capability, ErrProviderNotFound if not registered.

UpdateProviderSettingsParams uses the sqlc-generated field names:
Enabled bool (not *bool — the COALESCE is handled by passing the
cached current value when patch.Enabled is nil), Column3 bool
(apiKeyChanged flag), ApiKey *string (the api_key value — sqlc
named this from the column, not Column4 as the task spec assumed).

Tests gated on MINSTREL_TEST_DATABASE_URL: boot-inserts-defaults,
flip-enabled-bumps-version, key-only-doesn't-bump, clear-key,
TestProvider routing for testable / non-testable / missing,
ListProviderInfo capability badges. Reuses newPool / discardLogger
and fakeProvider / fakeAlbumProvider from enricher_test.go and
provider_test.go (same package).

internal/coverart/settings.go

@@ -0,0 +1,307 @@
+package coverart
+
+import (
+	"context"
+	"fmt"
+	"log/slog"
+	"sort"
+	"strings"
+	"sync"
+
+	"github.com/jackc/pgx/v5/pgxpool"
+
+	"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
+)
+
+// SettingsService reconciles the registry of compiled-in providers
+// with the cover_art_provider_settings table at boot, and on every
+// admin-initiated update. It owns the cover_art_sources_meta
+// singleton's current_version stamp, bumping it whenever the
+// *enabled set* of providers changes.
+type SettingsService struct {
+	pool   *pgxpool.Pool
+	logger *slog.Logger
+
+	mu             sync.RWMutex
+	enabledIDs     map[string]bool   // provider_id → enabled
+	apiKeys        map[string]string // provider_id → api_key (empty if NULL)
+	currentVersion int32
+}
+
+// ProviderUpdatePatch is the admin-PATCH body shape. nil pointers
+// mean "leave unchanged"; non-nil means "set to this value (which
+// may be empty string for api_key clearing)".
+type ProviderUpdatePatch struct {
+	Enabled *bool
+	APIKey  *string
+}
+
+// ProviderInfo is the SettingsService's view of a provider for the
+// admin GET handler. Combines registry metadata with DB settings.
+type ProviderInfo struct {
+	ID             string
+	DisplayName    string
+	RequiresAPIKey bool
+	Supports       []string // "album_cover", "artist_thumb", "artist_fanart"
+	Enabled        bool
+	APIKeySet      bool
+	DisplayOrder   int32
+	Testable       bool
+}
+
+// NewSettingsService boots the service: reads the DB rows, INSERTs
+// defaults for any registered provider missing a row, calls
+// Configure() on every registered provider with the row data, and
+// loads the current_version. Returns the populated service.
+func NewSettingsService(ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger) (*SettingsService, error) {
+	s := &SettingsService{
+		pool:       pool,
+		logger:     logger,
+		enabledIDs: map[string]bool{},
+		apiKeys:    map[string]string{},
+	}
+	if err := s.reconcile(ctx); err != nil {
+		return nil, fmt.Errorf("settings service boot: %w", err)
+	}
+	return s, nil
+}
+
+// reconcile reads the DB, INSERTs defaults for missing providers,
+// calls Configure() on each registered provider, and loads the
+// current_version into the cache.
+func (s *SettingsService) reconcile(ctx context.Context) error {
+	q := dbq.New(s.pool)
+
+	// Insert defaults for any registered provider missing a row.
+	registered := AllProviders()
+	for i, p := range registered {
+		if err := q.UpsertProviderSettings(ctx, dbq.UpsertProviderSettingsParams{
+			ProviderID:   p.ID(),
+			Enabled:      p.DefaultEnabled(),
+			DisplayOrder: int32(i),
+		}); err != nil {
+			return fmt.Errorf("upsert provider %q: %w", p.ID(), err)
+		}
+	}
+
+	// Read all rows back.
+	rows, err := q.ListProviderSettings(ctx)
+	if err != nil {
+		return fmt.Errorf("list provider settings: %w", err)
+	}
+
+	// Build the lookup maps and call Configure on each registered
+	// provider with the corresponding row data.
+	settingsByID := map[string]dbq.CoverArtProviderSetting{}
+	for _, r := range rows {
+		settingsByID[r.ProviderID] = r
+	}
+
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
+	s.enabledIDs = map[string]bool{}
+	s.apiKeys = map[string]string{}
+	for _, p := range registered {
+		row, ok := settingsByID[p.ID()]
+		if !ok {
+			s.logger.Warn("settings: missing row after upsert", "provider", p.ID())
+			continue
+		}
+		key := ""
+		if row.ApiKey != nil {
+			key = *row.ApiKey
+		}
+		s.enabledIDs[p.ID()] = row.Enabled
+		s.apiKeys[p.ID()] = key
+		if err := p.Configure(ProviderSettings{Enabled: row.Enabled, APIKey: key}); err != nil {
+			s.logger.Warn("settings: provider Configure failed",
+				"provider", p.ID(), "err", err)
+		}
+	}
+
+	v, err := q.GetCurrentSourcesVersion(ctx)
+	if err != nil {
+		return fmt.Errorf("get current sources version: %w", err)
+	}
+	s.currentVersion = v
+	return nil
+}
+
+// EnabledAlbumProviders returns the enabled providers that implement
+// AlbumCoverProvider, in registration order. Snapshot — caller must
+// not mutate.
+func (s *SettingsService) EnabledAlbumProviders() []AlbumCoverProvider {
+	s.mu.RLock()
+	defer s.mu.RUnlock()
+	var out []AlbumCoverProvider
+	for _, p := range AllProviders() {
+		if !s.enabledIDs[p.ID()] {
+			continue
+		}
+		if ap, ok := p.(AlbumCoverProvider); ok {
+			out = append(out, ap)
+		}
+	}
+	return out
+}
+
+// EnabledArtistProviders is the parallel for ArtistArtProvider.
+func (s *SettingsService) EnabledArtistProviders() []ArtistArtProvider {
+	s.mu.RLock()
+	defer s.mu.RUnlock()
+	var out []ArtistArtProvider
+	for _, p := range AllProviders() {
+		if !s.enabledIDs[p.ID()] {
+			continue
+		}
+		if ap, ok := p.(ArtistArtProvider); ok {
+			out = append(out, ap)
+		}
+	}
+	return out
+}
+
+// CurrentVersion returns the current sources-set version (the stamp
+// the enricher writes onto rows it processes).
+func (s *SettingsService) CurrentVersion() int32 {
+	s.mu.RLock()
+	defer s.mu.RUnlock()
+	return s.currentVersion
+}
+
+// UpdateProvider applies an admin-UI patch. Returns versionBumped=true
+// if the change altered the *enabled set* (not on key changes).
+// Re-runs the provider's Configure() with the new settings.
+func (s *SettingsService) UpdateProvider(ctx context.Context, providerID string, patch ProviderUpdatePatch) (bool, error) {
+	p, err := ProviderByID(providerID)
+	if err != nil {
+		return false, err
+	}
+
+	q := dbq.New(s.pool)
+
+	// Compute current enabled-set signature before applying the patch.
+	s.mu.RLock()
+	beforeSig := enabledSetSignature(s.enabledIDs)
+	// Use the cached enabled value when the patch doesn't specify it,
+	// since the sqlc-generated Enabled field is bool (not *bool) and
+	// COALESCE($2, enabled) only works with a nullable param. Passing
+	// the current value when nil achieves the same "leave unchanged"
+	// semantics.
+	currentEnabled := s.enabledIDs[providerID]
+	s.mu.RUnlock()
+
+	enabledVal := currentEnabled
+	if patch.Enabled != nil {
+		enabledVal = *patch.Enabled
+	}
+
+	// Apply DB update. The sqlc-generated UpdateProviderSettingsParams
+	// uses Column3/ApiKey because the underlying SQL has $3::boolean
+	// (apiKeyChanged flag) and $4 (api_key value). The actual generated
+	// field for $4 is ApiKey *string.
+	apiKeyChanged := patch.APIKey != nil
+	if err := q.UpdateProviderSettings(ctx, dbq.UpdateProviderSettingsParams{
+		ProviderID: providerID,
+		Enabled:    enabledVal,
+		Column3:    apiKeyChanged,
+		ApiKey:     patch.APIKey,
+	}); err != nil {
+		return false, fmt.Errorf("update provider settings: %w", err)
+	}
+
+	// Re-read rows + re-cache + Configure the changed provider.
+	rows, err := q.ListProviderSettings(ctx)
+	if err != nil {
+		return false, fmt.Errorf("re-list provider settings: %w", err)
+	}
+
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
+	for _, r := range rows {
+		key := ""
+		if r.ApiKey != nil {
+			key = *r.ApiKey
+		}
+		s.enabledIDs[r.ProviderID] = r.Enabled
+		s.apiKeys[r.ProviderID] = key
+		if r.ProviderID == providerID {
+			if err := p.Configure(ProviderSettings{Enabled: r.Enabled, APIKey: key}); err != nil {
+				s.logger.Warn("provider Configure failed after update",
+					"provider", providerID, "err", err)
+			}
+		}
+	}
+
+	afterSig := enabledSetSignature(s.enabledIDs)
+	if beforeSig == afterSig {
+		return false, nil
+	}
+
+	newVersion, err := q.BumpSourcesVersion(ctx)
+	if err != nil {
+		return false, fmt.Errorf("bump sources version: %w", err)
+	}
+	s.currentVersion = newVersion
+	return true, nil
+}
+
+// enabledSetSignature returns a deterministic string of the
+// currently-enabled provider IDs.
+func enabledSetSignature(enabledIDs map[string]bool) string {
+	ids := make([]string, 0, len(enabledIDs))
+	for id, on := range enabledIDs {
+		if on {
+			ids = append(ids, id)
+		}
+	}
+	sort.Strings(ids)
+	return strings.Join(ids, ",")
+}
+
+// TestProvider invokes TestConnection on the named provider if it
+// implements TestableProvider. Returns ErrNotTestable if the
+// provider doesn't, ErrProviderNotFound if not registered.
+func (s *SettingsService) TestProvider(ctx context.Context, providerID string) error {
+	p, err := ProviderByID(providerID)
+	if err != nil {
+		return err
+	}
+	tp, ok := p.(TestableProvider)
+	if !ok {
+		return ErrNotTestable
+	}
+	return tp.TestConnection(ctx)
+}
+
+// ListProviderInfo builds the wire-shaped slice for GET
+// /api/admin/cover-sources. Returns providers in registration order.
+func (s *SettingsService) ListProviderInfo() []ProviderInfo {
+	s.mu.RLock()
+	defer s.mu.RUnlock()
+
+	var out []ProviderInfo
+	for i, p := range AllProviders() {
+		info := ProviderInfo{
+			ID:             p.ID(),
+			DisplayName:    p.DisplayName(),
+			RequiresAPIKey: p.RequiresAPIKey(),
+			Enabled:        s.enabledIDs[p.ID()],
+			APIKeySet:      s.apiKeys[p.ID()] != "",
+			DisplayOrder:   int32(i),
+		}
+		if _, ok := p.(AlbumCoverProvider); ok {
+			info.Supports = append(info.Supports, "album_cover")
+		}
+		if _, ok := p.(ArtistArtProvider); ok {
+			info.Supports = append(info.Supports, "artist_thumb", "artist_fanart")
+		}
+		if _, ok := p.(TestableProvider); ok {
+			info.Testable = true
+		}
+		out = append(out, info)
+	}
+	return out
+}

internal/coverart/settings_test.go

@@ -0,0 +1,233 @@
+package coverart
+
+import (
+	"context"
+	"errors"
+	"testing"
+)
+
+// fakeArtistProvider opts into ArtistArtProvider for capability tests.
+type fakeArtistProvider struct {
+	fakeProvider
+}
+
+func (f *fakeArtistProvider) FetchArtistArt(_ context.Context, _ string) (thumb, fanart []byte, err error) {
+	return []byte("t"), []byte("f"), nil
+}
+
+// fakeTestableAlbumProvider implements TestableProvider for the
+// TestProvider test.
+type fakeTestableAlbumProvider struct {
+	fakeAlbumProvider
+	testErr error
+}
+
+func (f *fakeTestableAlbumProvider) TestConnection(_ context.Context) error {
+	return f.testErr
+}
+
+func TestSettingsService_BootInsertsDefaultRows(t *testing.T) {
+	pool := newPool(t)
+	resetRegistryForTests()
+	defer resetRegistryForTests()
+
+	Register(&fakeAlbumProvider{fakeProvider: fakeProvider{id: "fake-album", display: "Fake Album", defaultOn: true}})
+	Register(&fakeArtistProvider{fakeProvider: fakeProvider{id: "fake-artist", display: "Fake Artist", defaultOn: false}})
+
+	s, err := NewSettingsService(context.Background(), pool, discardLogger())
+	if err != nil {
+		t.Fatalf("NewSettingsService: %v", err)
+	}
+
+	// Both providers should now have rows; only fake-album is enabled.
+	enabled := s.EnabledAlbumProviders()
+	if len(enabled) != 1 {
+		t.Fatalf("EnabledAlbumProviders = %d, want 1", len(enabled))
+	}
+	if enabled[0].ID() != "fake-album" {
+		t.Errorf("enabled[0].ID = %q, want fake-album", enabled[0].ID())
+	}
+
+	// fake-artist defaultOn=false, so it should NOT be in EnabledArtistProviders.
+	artists := s.EnabledArtistProviders()
+	if len(artists) != 0 {
+		t.Errorf("EnabledArtistProviders = %d, want 0", len(artists))
+	}
+
+	if s.CurrentVersion() != 1 {
+		t.Errorf("CurrentVersion = %d, want 1", s.CurrentVersion())
+	}
+}
+
+func TestSettingsService_UpdateProvider_FlippingEnabledBumpsVersion(t *testing.T) {
+	pool := newPool(t)
+	resetRegistryForTests()
+	defer resetRegistryForTests()
+
+	Register(&fakeAlbumProvider{fakeProvider: fakeProvider{id: "fake-album", defaultOn: true}})
+
+	s, err := NewSettingsService(context.Background(), pool, discardLogger())
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	startVersion := s.CurrentVersion()
+
+	disabled := false
+	bumped, err := s.UpdateProvider(context.Background(), "fake-album", ProviderUpdatePatch{Enabled: &disabled})
+	if err != nil {
+		t.Fatalf("UpdateProvider: %v", err)
+	}
+	if !bumped {
+		t.Error("versionBumped = false, want true after disabling")
+	}
+	if s.CurrentVersion() <= startVersion {
+		t.Errorf("CurrentVersion = %d, want > %d", s.CurrentVersion(), startVersion)
+	}
+
+	enabled := s.EnabledAlbumProviders()
+	if len(enabled) != 0 {
+		t.Errorf("after disable: EnabledAlbumProviders = %d, want 0", len(enabled))
+	}
+}
+
+func TestSettingsService_UpdateProvider_KeyOnlyDoesNotBumpVersion(t *testing.T) {
+	pool := newPool(t)
+	resetRegistryForTests()
+	defer resetRegistryForTests()
+
+	Register(&fakeAlbumProvider{fakeProvider: fakeProvider{id: "fake-album", defaultOn: true}})
+
+	s, err := NewSettingsService(context.Background(), pool, discardLogger())
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	startVersion := s.CurrentVersion()
+
+	newKey := "abc123"
+	bumped, err := s.UpdateProvider(context.Background(), "fake-album", ProviderUpdatePatch{APIKey: &newKey})
+	if err != nil {
+		t.Fatalf("UpdateProvider: %v", err)
+	}
+	if bumped {
+		t.Error("versionBumped = true, want false for key-only change")
+	}
+	if s.CurrentVersion() != startVersion {
+		t.Errorf("CurrentVersion changed: %d → %d, want unchanged", startVersion, s.CurrentVersion())
+	}
+
+	infos := s.ListProviderInfo()
+	var info ProviderInfo
+	for _, i := range infos {
+		if i.ID == "fake-album" {
+			info = i
+			break
+		}
+	}
+	if !info.APIKeySet {
+		t.Error("APIKeySet = false, want true after setting key")
+	}
+}
+
+func TestSettingsService_UpdateProvider_ClearKey(t *testing.T) {
+	pool := newPool(t)
+	resetRegistryForTests()
+	defer resetRegistryForTests()
+
+	Register(&fakeAlbumProvider{fakeProvider: fakeProvider{id: "fake-album", defaultOn: true}})
+
+	s, err := NewSettingsService(context.Background(), pool, discardLogger())
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Set a key, then clear it.
+	setKey := "abc"
+	if _, err := s.UpdateProvider(context.Background(), "fake-album", ProviderUpdatePatch{APIKey: &setKey}); err != nil {
+		t.Fatal(err)
+	}
+	emptyKey := ""
+	if _, err := s.UpdateProvider(context.Background(), "fake-album", ProviderUpdatePatch{APIKey: &emptyKey}); err != nil {
+		t.Fatal(err)
+	}
+
+	infos := s.ListProviderInfo()
+	for _, i := range infos {
+		if i.ID == "fake-album" && i.APIKeySet {
+			t.Error("APIKeySet = true after clear, want false")
+		}
+	}
+}
+
+func TestSettingsService_TestProvider_Routing(t *testing.T) {
+	pool := newPool(t)
+	resetRegistryForTests()
+	defer resetRegistryForTests()
+
+	plain := &fakeAlbumProvider{fakeProvider: fakeProvider{id: "plain", defaultOn: true}}
+	testable := &fakeTestableAlbumProvider{
+		fakeAlbumProvider: fakeAlbumProvider{fakeProvider: fakeProvider{id: "testable", defaultOn: true}},
+		testErr:           nil,
+	}
+	Register(plain)
+	Register(testable)
+
+	s, err := NewSettingsService(context.Background(), pool, discardLogger())
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Testable returns nil → success.
+	if err := s.TestProvider(context.Background(), "testable"); err != nil {
+		t.Errorf("TestProvider(testable) = %v, want nil", err)
+	}
+
+	// Plain doesn't implement TestableProvider → ErrNotTestable.
+	if err := s.TestProvider(context.Background(), "plain"); !errors.Is(err, ErrNotTestable) {
+		t.Errorf("TestProvider(plain) = %v, want ErrNotTestable", err)
+	}
+
+	// Missing → ErrProviderNotFound.
+	if err := s.TestProvider(context.Background(), "missing"); !errors.Is(err, ErrProviderNotFound) {
+		t.Errorf("TestProvider(missing) = %v, want ErrProviderNotFound", err)
+	}
+}
+
+func TestSettingsService_ListProviderInfo_Capabilities(t *testing.T) {
+	pool := newPool(t)
+	resetRegistryForTests()
+	defer resetRegistryForTests()
+
+	plain := &fakeProvider{id: "plain", defaultOn: true}
+	album := &fakeAlbumProvider{fakeProvider: fakeProvider{id: "album-only", defaultOn: true}}
+	artist := &fakeArtistProvider{fakeProvider: fakeProvider{id: "artist-only", defaultOn: true}}
+	Register(plain)
+	Register(album)
+	Register(artist)
+
+	s, err := NewSettingsService(context.Background(), pool, discardLogger())
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	infos := s.ListProviderInfo()
+	if len(infos) != 3 {
+		t.Fatalf("ListProviderInfo len = %d, want 3", len(infos))
+	}
+
+	byID := map[string]ProviderInfo{}
+	for _, i := range infos {
+		byID[i.ID] = i
+	}
+
+	if got := byID["plain"].Supports; len(got) != 0 {
+		t.Errorf("plain.Supports = %v, want empty", got)
+	}
+	if got := byID["album-only"].Supports; len(got) != 1 || got[0] != "album_cover" {
+		t.Errorf("album-only.Supports = %v, want [album_cover]", got)
+	}
+	if got := byID["artist-only"].Supports; len(got) != 2 || got[0] != "artist_thumb" || got[1] != "artist_fanart" {
+		t.Errorf("artist-only.Supports = %v, want [artist_thumb, artist_fanart]", got)
+	}
+}