From 8f5ec4c30445fd91d8fc7cc6753ec21fa84d567a Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 7 May 2026 07:55:07 -0400 Subject: [PATCH] feat(server/coverart): auto-bump version on registered-provider change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On boot, the SettingsService computes a SHA-256 hash of the sorted registered-provider IDs and compares to the hash stored on cover_art_sources_meta. Mismatch → atomic bump of current_version + update of stored hash. The next enrichment pass picks up the new version, all 'none' rows become eligible, and they get retried against the new chain (now including Deezer / Last.fm). One-shot: subsequent boots without a provider-set change don't bump again. The check is best-effort — failures are logged at Warn but don't refuse startup; a temporarily unreachable DB at boot just means 'none' rows stay parked until a manual re-search (next task). Tests cover first-boot (empty stored hash → bump), repeat-boot (no change → no bump), and the registered-providers-hash determinism. Co-Authored-By: Claude Sonnet 4.6 --- cmd/minstrel/main.go | 6 +++ internal/coverart/settings.go | 50 ++++++++++++++++++ internal/coverart/settings_test.go | 81 ++++++++++++++++++++++++++++++ 3 files changed, 137 insertions(+) diff --git a/cmd/minstrel/main.go b/cmd/minstrel/main.go index 945f6180..9c577762 100644 --- a/cmd/minstrel/main.go +++ b/cmd/minstrel/main.go @@ -90,6 +90,12 @@ func run() error { logger.Error("coverart settings service init failed", "err", err) os.Exit(1) } + if newVer, bumped, berr := coverSettings.BumpVersionIfProvidersChanged(ctx); berr != nil { + logger.Warn("coverart: provider-hash boot check failed", "err", berr) + } else if bumped { + logger.Info("coverart: registered provider set changed; version bumped", + "new_version", newVer) + } coverEnricher := coverart.NewEnricher(pool, logger.With("component", "coverart"), coverSettings) coverEnricher.DataDir = cfg.Storage.DataDir diff --git a/internal/coverart/settings.go b/internal/coverart/settings.go index fbd57d3b..c1ce6ebb 100644 --- a/internal/coverart/settings.go +++ b/internal/coverart/settings.go @@ -2,6 +2,8 @@ package coverart import ( "context" + "crypto/sha256" + "encoding/hex" "fmt" "log/slog" "sort" @@ -261,6 +263,54 @@ func enabledSetSignature(enabledIDs map[string]bool) string { return strings.Join(ids, ",") } +// BumpVersionIfProvidersChanged compares the current registered-provider +// set (by ID) to the last set seen on a prior successful boot. If +// different, it atomically increments cover_art_sources_meta.current_version +// and stores the new hash, returning (newVersion, true, nil). If +// unchanged, returns (currentVersion, false, nil) without touching the DB. +// +// Used at boot to ensure rows previously settled to 'none' get re-tried +// once after a provider chain change (e.g. operator updates the binary +// and a new provider is registered, or one is removed). The version +// bump triggers eligibility for 'none' rows on the next enrichment pass +// without requiring a manual operator action. +func (s *SettingsService) BumpVersionIfProvidersChanged(ctx context.Context) (version int32, bumped bool, err error) { + hash := registeredProvidersHash() + q := dbq.New(s.pool) + + stored, qerr := q.GetCoverArtProvidersHash(ctx) + if qerr != nil { + return 0, false, fmt.Errorf("get providers hash: %w", qerr) + } + if stored == hash { + return s.CurrentVersion(), false, nil + } + + newVer, berr := q.BumpVersionAndSetProvidersHash(ctx, hash) + if berr != nil { + return 0, false, fmt.Errorf("bump version: %w", berr) + } + // Update the in-memory version so the next enrichment pass sees the new value. + s.mu.Lock() + s.currentVersion = newVer + s.mu.Unlock() + return newVer, true, nil +} + +// registeredProvidersHash returns the SHA-256 hex of the sorted, +// colon-joined registered provider IDs. +func registeredProvidersHash() string { + provs := AllProviders() + ids := make([]string, 0, len(provs)) + for _, p := range provs { + ids = append(ids, p.ID()) + } + sort.Strings(ids) + joined := strings.Join(ids, ":") + sum := sha256.Sum256([]byte(joined)) + return hex.EncodeToString(sum[:]) +} + // TestProvider invokes TestConnection on the named provider if it // implements TestableProvider. Returns ErrNotTestable if the // provider doesn't, ErrProviderNotFound if not registered. diff --git a/internal/coverart/settings_test.go b/internal/coverart/settings_test.go index bf6ebceb..9dd1a6e7 100644 --- a/internal/coverart/settings_test.go +++ b/internal/coverart/settings_test.go @@ -231,3 +231,84 @@ func TestSettingsService_ListProviderInfo_Capabilities(t *testing.T) { t.Errorf("artist-only.Supports = %v, want [artist_thumb, artist_fanart]", got) } } + +// TestSettingsService_BumpVersionIfProvidersChanged_FirstBoot: stored +// hash is empty (column default ""), so a real provider set produces a bump. +func TestSettingsService_BumpVersionIfProvidersChanged_FirstBoot(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.Fatalf("init: %v", err) + } + + before := s.CurrentVersion() + newVer, bumped, err := s.BumpVersionIfProvidersChanged(context.Background()) + if err != nil { + t.Fatalf("bump: %v", err) + } + if !bumped { + t.Errorf("bumped = false, want true on first boot (stored hash is empty)") + } + if newVer <= before { + t.Errorf("newVer = %d, want > %d", newVer, before) + } + if s.CurrentVersion() != newVer { + t.Errorf("in-memory version = %d, want %d (synchronized with DB)", s.CurrentVersion(), newVer) + } +} + +// TestSettingsService_BumpVersionIfProvidersChanged_NoChange: second +// call back-to-back returns bumped=false because the stored hash now +// matches the registered set. +func TestSettingsService_BumpVersionIfProvidersChanged_NoChange(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.Fatalf("init: %v", err) + } + + // First call bumps (empty stored hash → mismatch). + if _, _, err := s.BumpVersionIfProvidersChanged(context.Background()); err != nil { + t.Fatalf("first bump: %v", err) + } + + afterFirst := s.CurrentVersion() + _, bumped, err := s.BumpVersionIfProvidersChanged(context.Background()) + if err != nil { + t.Fatalf("second bump: %v", err) + } + if bumped { + t.Errorf("bumped = true on second call, want false (no change)") + } + if s.CurrentVersion() != afterFirst { + t.Errorf("version changed without a provider-set change") + } +} + +// TestRegisteredProvidersHash_StableAcrossOrder: registry order +// shouldn't matter because we sort before hashing. +func TestRegisteredProvidersHash_StableAcrossOrder(t *testing.T) { + // Hash is a function of the registered set; in this test it just + // needs to be deterministic and non-empty. + h1 := registeredProvidersHash() + h2 := registeredProvidersHash() + if h1 != h2 { + t.Errorf("hash is not deterministic: %q vs %q", h1, h2) + } + if h1 == "" { + t.Errorf("hash empty; expected at least one provider registered (or empty set is valid)") + } + if len(h1) != 64 { + t.Errorf("hash length = %d, want 64 (SHA-256 hex)", len(h1)) + } +}