diff --git a/internal/coverart/settings.go b/internal/coverart/settings.go new file mode 100644 index 00000000..fbd57d3b --- /dev/null +++ b/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 +} diff --git a/internal/coverart/settings_test.go b/internal/coverart/settings_test.go new file mode 100644 index 00000000..3b250e82 --- /dev/null +++ b/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) + } +}