Files
minstrel/internal/coverart/enricher_test.go
T
bvandeusen 385eb3b163 feat(server/m7-cover-sources): album enricher uses provider chain
Rewires EnrichAlbum to iterate EnabledAlbumProviders from the
SettingsService instead of a single hardcoded MBCAA fetcher. Sidecar
layer is unchanged (always tried first). Per-row eligibility check
honors cover_art_sources_version: terminal 'found' values skip;
'none' flips to NULL via ClearAlbumCoverNone and proceeds (the DB-
level ListAlbumsMissingCover query guards the version check for batch
callers; RetryAlbum clears via ClearAlbumCover first); NULL is eligible.

The provider chain uses an inline allWere404 accumulator to settle
the row to 'none' only when every provider returned ErrNotFound.
Any provider returning ErrTransient leaves the row NULL for next-
pass retry — even if other providers returned ErrNotFound — since
the transient call's MBID might succeed on a future attempt.

Boot wiring (cmd/minstrel/main.go) replaces the
NewFetcher+NewEnricher(fetcher, mbcaaOn) shape with
NewMBCAAProviderFromConfig (swaps in production User-Agent on the
registered provider) + NewSettingsService + NewEnricher(settings).
The old cfg.Library.CoverArtFromMBCAA flag is no-op'd; DB-backed
settings replace it. Field stays in the config struct for backward
compat with deployed config files.

Consolidates fetcher.go's HTTP logic into provider_mbcaa.go (no more
wrapper indirection): mbcaaProvider now holds cfg/mu/lastCall/client
directly. Deletes fetcher.go and fetcher_test.go. ErrNotFound and
ErrTransient move into provider.go alongside the other sentinels.

Updates admin_covers_test.go and provider_mbcaa_test.go to the new
constructor shapes. Adds multi-provider-chain, transient-leaves-null,
and stale-none-flips-and-retries test cases in enricher_test.go.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-06 12:52:57 -04:00

562 lines
18 KiB
Go

package coverart
import (
"context"
"io"
"log/slog"
"net/http"
"net/http/httptest"
"os"
"path/filepath"
"testing"
"github.com/jackc/pgx/v5/pgtype"
"github.com/jackc/pgx/v5/pgxpool"
"git.fabledsword.com/bvandeusen/minstrel/internal/db"
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
"git.fabledsword.com/bvandeusen/minstrel/internal/dbtest"
)
func newPool(t *testing.T) *pgxpool.Pool {
t.Helper()
if testing.Short() {
t.Skip("skipping integration test in -short mode")
}
dsn := os.Getenv("MINSTREL_TEST_DATABASE_URL")
if dsn == "" {
t.Skip("MINSTREL_TEST_DATABASE_URL not set")
}
if err := db.Migrate(dsn, slog.New(slog.NewTextHandler(io.Discard, nil))); err != nil {
t.Fatalf("migrate: %v", err)
}
pool, err := pgxpool.New(context.Background(), dsn)
if err != nil {
t.Fatalf("pool: %v", err)
}
t.Cleanup(pool.Close)
dbtest.ResetDB(t, pool)
return pool
}
func discardLogger() *slog.Logger {
return slog.New(slog.NewTextHandler(io.Discard, nil))
}
// newTestEnricher constructs an Enricher backed by a real DB pool for
// integration tests. Callers should register any fakes they need in
// the registry before calling this; SettingsService.reconcile() will
// pick them up.
func newTestEnricher(t *testing.T, pool *pgxpool.Pool) *Enricher {
t.Helper()
resetRegistryForTests()
t.Cleanup(resetRegistryForTests)
s, err := NewSettingsService(context.Background(), pool, discardLogger())
if err != nil {
t.Fatalf("NewSettingsService: %v", err)
}
return NewEnricher(pool, discardLogger(), s)
}
// seedAlbumWithTrack creates an artist + album + track triple in a temp
// dir. Returns album ID and the album-dir path.
func seedAlbumWithTrack(t *testing.T, pool *pgxpool.Pool, title, artistName, mbid string) (pgtype.UUID, string) {
t.Helper()
q := dbq.New(pool)
dir := t.TempDir()
a, err := q.UpsertArtist(context.Background(), dbq.UpsertArtistParams{
Name: artistName, SortName: artistName,
})
if err != nil {
t.Fatalf("seed artist: %v", err)
}
var mbidParam *string
if mbid != "" {
mbidParam = &mbid
}
al, err := q.UpsertAlbum(context.Background(), dbq.UpsertAlbumParams{
Title: title, SortTitle: title, ArtistID: a.ID, Mbid: mbidParam,
})
if err != nil {
t.Fatalf("seed album: %v", err)
}
trackPath := filepath.Join(dir, title+".mp3")
if err := os.WriteFile(trackPath, []byte("audio"), 0o644); err != nil {
t.Fatalf("write track: %v", err)
}
if _, err := q.UpsertTrack(context.Background(), dbq.UpsertTrackParams{
Title: title, AlbumID: al.ID, ArtistID: a.ID,
DurationMs: 1000, FilePath: trackPath, FileSize: 5, FileFormat: "mp3",
}); err != nil {
t.Fatalf("seed track: %v", err)
}
return al.ID, dir
}
func TestEnrichAlbum_SidecarFound(t *testing.T) {
pool := newPool(t)
id, dir := seedAlbumWithTrack(t, pool, "Sid", "Artist", "")
if err := os.WriteFile(filepath.Join(dir, "cover.jpg"), []byte("img"), 0o644); err != nil {
t.Fatal(err)
}
e := newTestEnricher(t, pool)
if err := e.EnrichAlbum(context.Background(), id); err != nil {
t.Fatalf("enrich: %v", err)
}
row, err := dbq.New(pool).GetAlbumWithFirstTrackPath(context.Background(), id)
if err != nil {
t.Fatalf("get: %v", err)
}
if row.CoverArtSource == nil || *row.CoverArtSource != "sidecar" {
t.Errorf("source = %v, want 'sidecar'", row.CoverArtSource)
}
if row.CoverArtPath == nil || *row.CoverArtPath != filepath.Join(dir, "cover.jpg") {
t.Errorf("path = %v", row.CoverArtPath)
}
}
func TestEnrichAlbum_NoSidecarNoMBID_LeavesNull(t *testing.T) {
pool := newPool(t)
id, _ := seedAlbumWithTrack(t, pool, "NoMbid", "Artist", "")
e := newTestEnricher(t, pool)
if err := e.EnrichAlbum(context.Background(), id); err != nil {
t.Fatalf("enrich: %v", err)
}
row, err := dbq.New(pool).GetAlbumWithFirstTrackPath(context.Background(), id)
if err != nil {
t.Fatalf("get: %v", err)
}
if row.CoverArtSource != nil {
t.Errorf("source = %v, want nil (NULL — eligible for retry on next scan)", row.CoverArtSource)
}
}
func TestEnrichAlbum_ProviderFetchSuccess(t *testing.T) {
pool := newPool(t)
id, dir := seedAlbumWithTrack(t, pool, "Prov", "Artist", "test-mbid-123")
upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.Header().Set("Content-Type", "image/jpeg")
_, _ = w.Write([]byte("MBCAA_BYTES"))
}))
defer upstream.Close()
// Register the MBCAA provider pointing at the test server, then
// build the Enricher so SettingsService picks it up.
resetRegistryForTests()
t.Cleanup(resetRegistryForTests)
p := newMBCAAProvider(FetcherConfig{
BaseURL: upstream.URL,
UserAgent: "test",
MinPeriod: 0,
HTTPClient: upstream.Client(),
})
Register(p)
s, err := NewSettingsService(context.Background(), pool, discardLogger())
if err != nil {
t.Fatalf("NewSettingsService: %v", err)
}
e := NewEnricher(pool, discardLogger(), s)
if err := e.EnrichAlbum(context.Background(), id); err != nil {
t.Fatalf("enrich: %v", err)
}
row, err := dbq.New(pool).GetAlbumWithFirstTrackPath(context.Background(), id)
if err != nil {
t.Fatalf("get: %v", err)
}
if row.CoverArtSource == nil || *row.CoverArtSource != "mbcaa" {
t.Errorf("source = %v, want 'mbcaa'", row.CoverArtSource)
}
expectedPath := filepath.Join(dir, "cover.jpg")
if row.CoverArtPath == nil || *row.CoverArtPath != expectedPath {
t.Errorf("path = %v, want %s", row.CoverArtPath, expectedPath)
}
body, err := os.ReadFile(expectedPath)
if err != nil {
t.Fatalf("read file: %v", err)
}
if string(body) != "MBCAA_BYTES" {
t.Errorf("file body = %q", string(body))
}
}
func TestEnrichAlbum_All404_RecordsNone(t *testing.T) {
pool := newPool(t)
id, _ := seedAlbumWithTrack(t, pool, "Missing", "Artist", "missing-mbid")
upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
http.Error(w, "not found", http.StatusNotFound)
}))
defer upstream.Close()
resetRegistryForTests()
t.Cleanup(resetRegistryForTests)
p := newMBCAAProvider(FetcherConfig{
BaseURL: upstream.URL,
UserAgent: "test",
MinPeriod: 0,
HTTPClient: upstream.Client(),
})
Register(p)
s, err := NewSettingsService(context.Background(), pool, discardLogger())
if err != nil {
t.Fatalf("NewSettingsService: %v", err)
}
e := NewEnricher(pool, discardLogger(), s)
if err := e.EnrichAlbum(context.Background(), id); err != nil {
t.Fatalf("enrich: %v", err)
}
row, err := dbq.New(pool).GetAlbumWithFirstTrackPath(context.Background(), id)
if err != nil {
t.Fatalf("get: %v", err)
}
if row.CoverArtSource == nil || *row.CoverArtSource != "none" {
t.Errorf("source = %v, want 'none'", row.CoverArtSource)
}
}
func TestEnrichAlbum_AlreadySidecar_NoOp(t *testing.T) {
pool := newPool(t)
id, dir := seedAlbumWithTrack(t, pool, "Already", "Artist", "")
if err := os.WriteFile(filepath.Join(dir, "cover.jpg"), []byte("first"), 0o644); err != nil {
t.Fatal(err)
}
e := newTestEnricher(t, pool)
if err := e.EnrichAlbum(context.Background(), id); err != nil {
t.Fatalf("first enrich: %v", err)
}
// Second call should be a no-op even if we change the file content.
if err := os.WriteFile(filepath.Join(dir, "cover.jpg"), []byte("changed"), 0o644); err != nil {
t.Fatal(err)
}
if err := e.EnrichAlbum(context.Background(), id); err != nil {
t.Fatalf("second enrich: %v", err)
}
row, _ := dbq.New(pool).GetAlbumWithFirstTrackPath(context.Background(), id)
if row.CoverArtSource == nil || *row.CoverArtSource != "sidecar" {
t.Errorf("source = %v, want 'sidecar' (preserved)", row.CoverArtSource)
}
}
func TestEnrichBatch_DrainsNullSourceOnly(t *testing.T) {
pool := newPool(t)
id1, dir1 := seedAlbumWithTrack(t, pool, "Drain1", "A", "")
if err := os.WriteFile(filepath.Join(dir1, "cover.jpg"), []byte("c"), 0o644); err != nil {
t.Fatal(err)
}
id2, _ := seedAlbumWithTrack(t, pool, "Drain2", "B", "")
// Pre-mark id2 as 'none' with current version — should NOT be drained by EnrichBatch
// (ListAlbumsMissingCover only returns stale-version 'none').
src := "none"
if err := dbq.New(pool).SetAlbumCover(context.Background(), dbq.SetAlbumCoverParams{
ID: id2, Column2: "", CoverArtSource: &src,
}); err != nil {
t.Fatal(err)
}
e := newTestEnricher(t, pool)
processed, _, _, err := e.EnrichBatch(context.Background(), 100)
if err != nil {
t.Fatalf("batch: %v", err)
}
if processed != 1 {
t.Errorf("processed = %d, want 1 (only id1 NULL)", processed)
}
row1, _ := dbq.New(pool).GetAlbumWithFirstTrackPath(context.Background(), id1)
if row1.CoverArtSource == nil || *row1.CoverArtSource != "sidecar" {
t.Errorf("id1 source = %v", row1.CoverArtSource)
}
row2, _ := dbq.New(pool).GetAlbumWithFirstTrackPath(context.Background(), id2)
if row2.CoverArtSource == nil || *row2.CoverArtSource != "none" {
t.Errorf("id2 source = %v (should still be 'none')", row2.CoverArtSource)
}
}
func TestRetryAlbum_DeletesWrittenFileAndRefetches(t *testing.T) {
pool := newPool(t)
id, dir := seedAlbumWithTrack(t, pool, "Retry", "Artist", "retry-mbid")
hits := 0
upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
hits++
_, _ = w.Write([]byte("data"))
}))
defer upstream.Close()
resetRegistryForTests()
t.Cleanup(resetRegistryForTests)
p := newMBCAAProvider(FetcherConfig{
BaseURL: upstream.URL,
UserAgent: "test",
MinPeriod: 0,
HTTPClient: upstream.Client(),
})
Register(p)
s, err := NewSettingsService(context.Background(), pool, discardLogger())
if err != nil {
t.Fatalf("NewSettingsService: %v", err)
}
e := NewEnricher(pool, discardLogger(), s)
if err := e.EnrichAlbum(context.Background(), id); err != nil {
t.Fatalf("first enrich: %v", err)
}
written := filepath.Join(dir, "cover.jpg")
if _, err := os.Stat(written); err != nil {
t.Fatalf("expected file at %s: %v", written, err)
}
if err := e.RetryAlbum(context.Background(), id); err != nil {
t.Fatalf("retry: %v", err)
}
if hits < 2 {
t.Errorf("expected ≥2 upstream hits (initial + retry), got %d", hits)
}
row, _ := dbq.New(pool).GetAlbumWithFirstTrackPath(context.Background(), id)
if row.CoverArtSource == nil || *row.CoverArtSource != "mbcaa" {
t.Errorf("source after retry = %v, want 'mbcaa'", row.CoverArtSource)
}
}
func TestEnrichRetryMissing_DrainsNullAndNone(t *testing.T) {
pool := newPool(t)
// Three albums:
// id1: NULL source, has sidecar → drains, becomes 'sidecar'.
// id2: 'none' source, has sidecar → cleared then drained, becomes 'sidecar'.
// id3: 'sidecar' source already → NOT touched.
id1, dir1 := seedAlbumWithTrack(t, pool, "RM1", "A", "")
if err := os.WriteFile(filepath.Join(dir1, "cover.jpg"), []byte("c1"), 0o644); err != nil {
t.Fatal(err)
}
id2, dir2 := seedAlbumWithTrack(t, pool, "RM2", "B", "")
if err := os.WriteFile(filepath.Join(dir2, "cover.jpg"), []byte("c2"), 0o644); err != nil {
t.Fatal(err)
}
none := "none"
if err := dbq.New(pool).SetAlbumCover(context.Background(), dbq.SetAlbumCoverParams{
ID: id2, Column2: "", CoverArtSource: &none,
}); err != nil {
t.Fatal(err)
}
id3, dir3 := seedAlbumWithTrack(t, pool, "RM3", "C", "")
if err := os.WriteFile(filepath.Join(dir3, "cover.jpg"), []byte("c3"), 0o644); err != nil {
t.Fatal(err)
}
sidecar := "sidecar"
preservedPath := filepath.Join(dir3, "cover.jpg")
if err := dbq.New(pool).SetAlbumCover(context.Background(), dbq.SetAlbumCoverParams{
ID: id3, Column2: preservedPath, CoverArtSource: &sidecar,
}); err != nil {
t.Fatal(err)
}
e := newTestEnricher(t, pool)
processed, err := e.EnrichRetryMissing(context.Background(), 100)
if err != nil {
t.Fatalf("retry missing: %v", err)
}
if processed != 2 {
t.Errorf("processed = %d, want 2 (id1 NULL + id2 'none'; id3 already 'sidecar' not touched)", processed)
}
q := dbq.New(pool)
for _, c := range []struct {
id pgtype.UUID
want string
}{
{id1, "sidecar"},
{id2, "sidecar"},
{id3, "sidecar"},
} {
row, _ := q.GetAlbumWithFirstTrackPath(context.Background(), c.id)
if row.CoverArtSource == nil || *row.CoverArtSource != c.want {
t.Errorf("album %v source = %v, want %q", c.id, row.CoverArtSource, c.want)
}
}
}
// TestEnrichAlbum_MultiProviderChain verifies that when provider A
// returns ErrNotFound, the enricher continues to provider B.
func TestEnrichAlbum_MultiProviderChain(t *testing.T) {
pool := newPool(t)
id, dir := seedAlbumWithTrack(t, pool, "Multi", "Artist", "multi-mbid")
// Provider A: always 404.
srvA := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
http.Error(w, "not found", http.StatusNotFound)
}))
defer srvA.Close()
// Provider B: always succeeds.
srvB := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.Header().Set("Content-Type", "image/jpeg")
_, _ = w.Write([]byte("PROVIDER_B_BYTES"))
}))
defer srvB.Close()
resetRegistryForTests()
t.Cleanup(resetRegistryForTests)
pA := newMBCAAProvider(FetcherConfig{
BaseURL: srvA.URL,
UserAgent: "test",
MinPeriod: 0,
HTTPClient: srvA.Client(),
})
Register(pA)
// fakeAlbumProvider from provider_test.go doesn't exist here yet;
// use a second mbcaa-shaped provider by registering a distinct one.
// We need a second AlbumCoverProvider with a different ID.
// Use a simple inline struct that wraps the second test server.
Register(&testAlbumProvider{
id: "test-provider-b",
baseURL: srvB.URL,
client: srvB.Client(),
})
s, err := NewSettingsService(context.Background(), pool, discardLogger())
if err != nil {
t.Fatalf("NewSettingsService: %v", err)
}
e := NewEnricher(pool, discardLogger(), s)
if err := e.EnrichAlbum(context.Background(), id); err != nil {
t.Fatalf("enrich: %v", err)
}
row, err := dbq.New(pool).GetAlbumWithFirstTrackPath(context.Background(), id)
if err != nil {
t.Fatalf("get: %v", err)
}
if row.CoverArtSource == nil || *row.CoverArtSource != "test-provider-b" {
t.Errorf("source = %v, want 'test-provider-b' (provider B should win)", row.CoverArtSource)
}
if body, err := os.ReadFile(filepath.Join(dir, "cover.jpg")); err != nil || string(body) != "PROVIDER_B_BYTES" {
t.Errorf("cover.jpg body = %q, err = %v", body, err)
}
}
// TestEnrichAlbum_TransientLeavesNull verifies that a transient error
// from the provider leaves the row NULL for next-pass retry.
func TestEnrichAlbum_TransientLeavesNull(t *testing.T) {
pool := newPool(t)
id, _ := seedAlbumWithTrack(t, pool, "Transient", "Artist", "transient-mbid")
upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
http.Error(w, "boom", http.StatusInternalServerError)
}))
defer upstream.Close()
resetRegistryForTests()
t.Cleanup(resetRegistryForTests)
p := newMBCAAProvider(FetcherConfig{
BaseURL: upstream.URL,
UserAgent: "test",
MinPeriod: 0,
HTTPClient: upstream.Client(),
})
Register(p)
s, err := NewSettingsService(context.Background(), pool, discardLogger())
if err != nil {
t.Fatalf("NewSettingsService: %v", err)
}
e := NewEnricher(pool, discardLogger(), s)
if err := e.EnrichAlbum(context.Background(), id); err != nil {
t.Fatalf("enrich: %v", err)
}
row, err := dbq.New(pool).GetAlbumWithFirstTrackPath(context.Background(), id)
if err != nil {
t.Fatalf("get: %v", err)
}
// Transient failure: source must remain NULL (eligible for retry).
if row.CoverArtSource != nil {
t.Errorf("source = %v, want nil (NULL — transient should not settle)", row.CoverArtSource)
}
}
// TestEnrichAlbum_StaleNoneFlipsAndRetries verifies that a row with
// cover_art_source='none' and a stale version gets cleared and
// re-enriched (sidecar wins because the album dir now has a cover file).
func TestEnrichAlbum_StaleNoneFlipsAndRetries(t *testing.T) {
pool := newPool(t)
id, dir := seedAlbumWithTrack(t, pool, "StaleNone", "Artist", "")
// Mark the album as 'none' (simulating a previous pass).
src := "none"
if err := dbq.New(pool).SetAlbumCover(context.Background(), dbq.SetAlbumCoverParams{
ID: id, Column2: "", CoverArtSource: &src,
}); err != nil {
t.Fatal(err)
}
// Now add a sidecar — EnrichAlbum should flip 'none' → NULL and pick it up.
if err := os.WriteFile(filepath.Join(dir, "cover.jpg"), []byte("new_art"), 0o644); err != nil {
t.Fatal(err)
}
e := newTestEnricher(t, pool)
if err := e.EnrichAlbum(context.Background(), id); err != nil {
t.Fatalf("enrich: %v", err)
}
row, err := dbq.New(pool).GetAlbumWithFirstTrackPath(context.Background(), id)
if err != nil {
t.Fatalf("get: %v", err)
}
if row.CoverArtSource == nil || *row.CoverArtSource != "sidecar" {
t.Errorf("source = %v, want 'sidecar' (stale none flipped and sidecar found)", row.CoverArtSource)
}
}
// testAlbumProvider is a minimal AlbumCoverProvider for multi-chain tests.
type testAlbumProvider struct {
id string
baseURL string
client *http.Client
}
func (p *testAlbumProvider) ID() string { return p.id }
func (p *testAlbumProvider) DisplayName() string { return p.id }
func (p *testAlbumProvider) RequiresAPIKey() bool { return false }
func (p *testAlbumProvider) DefaultEnabled() bool { return true }
func (p *testAlbumProvider) Configure(ProviderSettings) error { return nil }
func (p *testAlbumProvider) FetchAlbumCover(ctx context.Context, mbid string) ([]byte, error) {
req, err := http.NewRequestWithContext(ctx, http.MethodGet, p.baseURL+"/"+mbid, nil)
if err != nil {
return nil, err
}
resp, err := p.client.Do(req)
if err != nil {
return nil, err
}
defer func() { _ = resp.Body.Close() }()
if resp.StatusCode == http.StatusNotFound {
return nil, ErrNotFound
}
if resp.StatusCode != http.StatusOK {
return nil, ErrTransient
}
body := make([]byte, 0, 512)
buf := make([]byte, 512)
for {
n, rerr := resp.Body.Read(buf)
body = append(body, buf[:n]...)
if rerr != nil {
break
}
}
return body, nil
}