Files
bvandeusen 412b9e37eb test(coverart): no-MBID enrich settles 'none' (per canonical behavior)
Operator decision: the enricher is canonical. No MBID still runs the
provider chain (name-based providers — Deezer/Last.fm — resolve
without an MBID); if every provider returns ErrNotFound the row
settles cover/artist source 'none' at the current sources version
(re-eligible only when the registered provider set changes). It does
NOT skip-and-leave-NULL.

The two _NoMBID_LeavesNull tests predated the name-based providers
(0020 slice) and asserted the old skip→NULL contract. Updated:
- TestEnrichArtist_NoMBID_SettlesNone: stub now returns ErrNotFound
  (realistic MBID-only-provider-with-empty-MBID), expect source 'none'.
- TestEnrichAlbum_NoSidecarNoMBID_SettlesNone: empty registry →
  allWere404 stays true → expect 'none'.

Last failing cluster from the CI-integration initiative; suite should
now be fully green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-17 22:27:01 -04:00

585 lines
19 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()
// Do NOT reset the registry here: callers register their fakes
// before calling this (see doc above) and resetting would wipe them
// so reconcile() sees zero providers — every art source then stays
// NULL. Registry lifecycle is the caller's (each test does
// resetRegistryForTests + t.Cleanup before Register()).
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)
}
resetRegistryForTests() // deterministic empty registry (sidecar-only path)
t.Cleanup(resetRegistryForTests)
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_SettlesNone(t *testing.T) {
pool := newPool(t)
id, _ := seedAlbumWithTrack(t, pool, "NoMbid", "Artist", "")
resetRegistryForTests() // deterministic empty registry (no provider can supply)
t.Cleanup(resetRegistryForTests)
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)
}
// No sidecar, no MBID, no provider yields → the chain settles
// 'none' (allWere404 stays true), version-stamped so it is only
// re-tried when the registered provider set changes.
if row.CoverArtSource == nil || *row.CoverArtSource != "none" {
t.Errorf("source = %v, want 'none' (settled)", 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)
}
resetRegistryForTests() // deterministic empty registry (sidecar-only path)
t.Cleanup(resetRegistryForTests)
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", "")
resetRegistryForTests() // deterministic empty registry
t.Cleanup(resetRegistryForTests)
e := newTestEnricher(t, pool)
// Pre-mark id2 as 'none' AT the current sources version so
// ListAlbumsMissingCover excludes it (it only returns NULL or
// stale-version 'none'). SetAlbumCover does NOT stamp the version;
// SetAlbumCoverWithVersion does — use the live current version, the
// same value EnrichBatch compares against.
curVer, err := dbq.New(pool).GetCurrentSourcesVersion(context.Background())
if err != nil {
t.Fatalf("current version: %v", err)
}
src := "none"
if err := dbq.New(pool).SetAlbumCoverWithVersion(context.Background(), dbq.SetAlbumCoverWithVersionParams{
ID: id2, CoverArtPath: nil, CoverArtSource: &src, CoverArtSourcesVersion: curVer,
}); err != nil {
t.Fatal(err)
}
processed, _, _, err := e.EnrichBatch(context.Background(), 100, nil)
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, ref AlbumRef) ([]byte, error) {
req, err := http.NewRequestWithContext(ctx, http.MethodGet, p.baseURL+"/"+ref.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
}