feat(server/m7-353): coverart.Enricher orchestrates sidecar + MBCAA + record
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,197 @@
|
||||
package coverart
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
"os"
|
||||
"path/filepath"
|
||||
|
||||
"github.com/jackc/pgx/v5"
|
||||
"github.com/jackc/pgx/v5/pgtype"
|
||||
"github.com/jackc/pgx/v5/pgxpool"
|
||||
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
|
||||
)
|
||||
|
||||
// Enricher orchestrates the sidecar → MBCAA → record cycle for one or many
|
||||
// albums. Owns no DB pool itself; receives one per call so the boot
|
||||
// backfill, post-scan trigger, and admin endpoints share state.
|
||||
type Enricher struct {
|
||||
pool *pgxpool.Pool
|
||||
logger *slog.Logger
|
||||
fetcher *Fetcher
|
||||
mbcaaOn bool
|
||||
}
|
||||
|
||||
// NewEnricher constructs an Enricher. Pass mbcaaOn=false to disable upstream
|
||||
// fetches entirely (sidecar-only mode for paranoid operators).
|
||||
func NewEnricher(pool *pgxpool.Pool, logger *slog.Logger, fetcher *Fetcher, mbcaaOn bool) *Enricher {
|
||||
return &Enricher{pool: pool, logger: logger, fetcher: fetcher, mbcaaOn: mbcaaOn}
|
||||
}
|
||||
|
||||
// EnrichAlbum runs the chain for a single album. Idempotent: skips when
|
||||
// cover_art_source is already set to a terminal value (sidecar/embedded/
|
||||
// mbcaa/none). The admin retry endpoint clears the field before calling.
|
||||
func (e *Enricher) EnrichAlbum(ctx context.Context, albumID pgtype.UUID) error {
|
||||
q := dbq.New(e.pool)
|
||||
row, err := q.GetAlbumWithFirstTrackPath(ctx, albumID)
|
||||
if err != nil {
|
||||
if errors.Is(err, pgx.ErrNoRows) {
|
||||
return nil // album was deleted between queueing and processing
|
||||
}
|
||||
return fmt.Errorf("get album: %w", err)
|
||||
}
|
||||
|
||||
// Skip when already settled. NULL is the eligible state.
|
||||
current := ""
|
||||
if row.CoverArtSource != nil {
|
||||
current = *row.CoverArtSource
|
||||
}
|
||||
switch current {
|
||||
case "sidecar", "embedded", "mbcaa", "none":
|
||||
return nil
|
||||
}
|
||||
|
||||
// Step 1: sidecar lookup.
|
||||
if row.TrackFilePath != nil && *row.TrackFilePath != "" {
|
||||
albumDir := filepath.Dir(*row.TrackFilePath)
|
||||
if path := FindSidecar(albumDir); path != "" {
|
||||
return e.recordCover(ctx, albumID, path, "sidecar")
|
||||
}
|
||||
}
|
||||
|
||||
// Step 2: MBCAA fallback. Requires fetcher + toggle on + MBID.
|
||||
if !e.mbcaaOn || e.fetcher == nil {
|
||||
return e.recordCover(ctx, albumID, "", "none")
|
||||
}
|
||||
if row.Mbid == nil || *row.Mbid == "" {
|
||||
return e.recordCover(ctx, albumID, "", "none")
|
||||
}
|
||||
|
||||
body, err := e.fetcher.Fetch(ctx, *row.Mbid)
|
||||
if err != nil {
|
||||
if errors.Is(err, ErrNotFound) {
|
||||
return e.recordCover(ctx, albumID, "", "none")
|
||||
}
|
||||
// ErrTransient or context cancellation: leave NULL for retry.
|
||||
e.logger.Warn("coverart: fetch failed; leaving for retry",
|
||||
"album_id", uuidString(albumID), "err", err)
|
||||
return nil
|
||||
}
|
||||
|
||||
// Step 3: write to album dir as cover.jpg, record path + 'mbcaa'.
|
||||
if row.TrackFilePath == nil || *row.TrackFilePath == "" {
|
||||
// Defensive: shouldn't happen if MBID was set on an album with
|
||||
// no tracks, but record 'none' rather than crash.
|
||||
return e.recordCover(ctx, albumID, "", "none")
|
||||
}
|
||||
albumDir := filepath.Dir(*row.TrackFilePath)
|
||||
dest := filepath.Join(albumDir, "cover.jpg")
|
||||
if err := os.WriteFile(dest, body, 0o644); err != nil {
|
||||
e.logger.Error("coverart: write file failed; leaving NULL",
|
||||
"album_id", uuidString(albumID), "path", dest, "err", err)
|
||||
return nil
|
||||
}
|
||||
return e.recordCover(ctx, albumID, dest, "mbcaa")
|
||||
}
|
||||
|
||||
// EnrichBatch drains up to limit albums whose cover_art_source is NULL.
|
||||
// Iterates serially — the fetcher's rate limiter prevents bursts.
|
||||
func (e *Enricher) EnrichBatch(ctx context.Context, limit int) (int, error) {
|
||||
if limit <= 0 {
|
||||
return 0, nil
|
||||
}
|
||||
q := dbq.New(e.pool)
|
||||
rows, err := q.ListAlbumsMissingCover(ctx, int32(limit))
|
||||
if err != nil {
|
||||
return 0, fmt.Errorf("list missing: %w", err)
|
||||
}
|
||||
processed := 0
|
||||
for _, r := range rows {
|
||||
if ctx.Err() != nil {
|
||||
return processed, ctx.Err()
|
||||
}
|
||||
if err := e.EnrichAlbum(ctx, r.ID); err != nil {
|
||||
e.logger.Warn("coverart: batch entry failed",
|
||||
"album_id", uuidString(r.ID), "err", err)
|
||||
}
|
||||
processed++
|
||||
}
|
||||
return processed, nil
|
||||
}
|
||||
|
||||
// EnrichRetryMissing drains both NULL and 'none' rows. Used by the admin
|
||||
// bulk-retry endpoint. Clears 'none' to NULL first so EnrichAlbum picks them up.
|
||||
func (e *Enricher) EnrichRetryMissing(ctx context.Context, limit int) (int, error) {
|
||||
if limit <= 0 {
|
||||
return 0, nil
|
||||
}
|
||||
q := dbq.New(e.pool)
|
||||
rows, err := q.ListAlbumsRetryMissing(ctx, int32(limit))
|
||||
if err != nil {
|
||||
return 0, fmt.Errorf("list retry missing: %w", err)
|
||||
}
|
||||
for _, r := range rows {
|
||||
if err := q.ClearAlbumCover(ctx, r.ID); err != nil {
|
||||
e.logger.Warn("coverart: clear before retry failed",
|
||||
"album_id", uuidString(r.ID), "err", err)
|
||||
}
|
||||
}
|
||||
processed := 0
|
||||
for _, r := range rows {
|
||||
if ctx.Err() != nil {
|
||||
return processed, ctx.Err()
|
||||
}
|
||||
_ = e.EnrichAlbum(ctx, r.ID)
|
||||
processed++
|
||||
}
|
||||
return processed, nil
|
||||
}
|
||||
|
||||
// RetryAlbum is the admin per-album retry path. Clears the album's cover
|
||||
// state, deletes the file if we wrote it (cover_art_source = 'mbcaa'),
|
||||
// then re-runs EnrichAlbum synchronously.
|
||||
func (e *Enricher) RetryAlbum(ctx context.Context, albumID pgtype.UUID) error {
|
||||
q := dbq.New(e.pool)
|
||||
row, err := q.GetAlbumWithFirstTrackPath(ctx, albumID)
|
||||
if err != nil {
|
||||
if errors.Is(err, pgx.ErrNoRows) {
|
||||
return ErrNotFound
|
||||
}
|
||||
return fmt.Errorf("get album: %w", err)
|
||||
}
|
||||
// Delete file only when we previously wrote it (mbcaa source). Operator
|
||||
// sidecars stay; embedded extracts (future) live in a managed cache that
|
||||
// has its own lifecycle.
|
||||
if row.CoverArtSource != nil && *row.CoverArtSource == "mbcaa" &&
|
||||
row.CoverArtPath != nil && *row.CoverArtPath != "" {
|
||||
if err := os.Remove(*row.CoverArtPath); err != nil && !os.IsNotExist(err) {
|
||||
e.logger.Warn("coverart: remove old mbcaa file failed",
|
||||
"album_id", uuidString(albumID), "path", *row.CoverArtPath, "err", err)
|
||||
}
|
||||
}
|
||||
if err := q.ClearAlbumCover(ctx, albumID); err != nil {
|
||||
return fmt.Errorf("clear cover: %w", err)
|
||||
}
|
||||
return e.EnrichAlbum(ctx, albumID)
|
||||
}
|
||||
|
||||
func (e *Enricher) recordCover(ctx context.Context, albumID pgtype.UUID, path, source string) error {
|
||||
q := dbq.New(e.pool)
|
||||
src := source
|
||||
return q.SetAlbumCover(ctx, dbq.SetAlbumCoverParams{
|
||||
ID: albumID,
|
||||
Column2: path,
|
||||
CoverArtSource: &src,
|
||||
})
|
||||
}
|
||||
|
||||
func uuidString(u pgtype.UUID) string {
|
||||
if !u.Valid {
|
||||
return "<nil>"
|
||||
}
|
||||
return fmt.Sprintf("%x-%x-%x-%x-%x",
|
||||
u.Bytes[0:4], u.Bytes[4:6], u.Bytes[6:8], u.Bytes[8:10], u.Bytes[10:16])
|
||||
}
|
||||
@@ -0,0 +1,269 @@
|
||||
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))
|
||||
}
|
||||
|
||||
// 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 := NewEnricher(pool, discardLogger(), nil, false)
|
||||
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_RecordsNone(t *testing.T) {
|
||||
pool := newPool(t)
|
||||
id, _ := seedAlbumWithTrack(t, pool, "NoMbid", "Artist", "")
|
||||
e := NewEnricher(pool, discardLogger(), nil, true)
|
||||
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_MBCAAFetchSuccess(t *testing.T) {
|
||||
pool := newPool(t)
|
||||
id, dir := seedAlbumWithTrack(t, pool, "Mbcaa", "Artist", "test-mbid-123")
|
||||
|
||||
upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Content-Type", "image/jpeg")
|
||||
_, _ = w.Write([]byte("MBCAA_BYTES"))
|
||||
}))
|
||||
defer upstream.Close()
|
||||
|
||||
f := NewFetcher(FetcherConfig{BaseURL: upstream.URL, UserAgent: "test", MinPeriod: 0, HTTPClient: upstream.Client()})
|
||||
e := NewEnricher(pool, discardLogger(), f, true)
|
||||
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_MBCAA404_RecordsNone(t *testing.T) {
|
||||
pool := newPool(t)
|
||||
id, _ := seedAlbumWithTrack(t, pool, "Missing", "Artist", "missing-mbid")
|
||||
|
||||
upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
http.Error(w, "not found", http.StatusNotFound)
|
||||
}))
|
||||
defer upstream.Close()
|
||||
|
||||
f := NewFetcher(FetcherConfig{BaseURL: upstream.URL, UserAgent: "test", MinPeriod: 0, HTTPClient: upstream.Client()})
|
||||
e := NewEnricher(pool, discardLogger(), f, true)
|
||||
_ = e.EnrichAlbum(context.Background(), id)
|
||||
|
||||
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 := NewEnricher(pool, discardLogger(), nil, false)
|
||||
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' — should NOT be drained by EnrichBatch.
|
||||
src := "none"
|
||||
if err := dbq.New(pool).SetAlbumCover(context.Background(), dbq.SetAlbumCoverParams{
|
||||
ID: id2, Column2: "", CoverArtSource: &src,
|
||||
}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
e := NewEnricher(pool, discardLogger(), nil, false)
|
||||
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_DeletesMbcaaFileAndRefetches(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, r *http.Request) {
|
||||
hits++
|
||||
_, _ = w.Write([]byte("data"))
|
||||
}))
|
||||
defer upstream.Close()
|
||||
f := NewFetcher(FetcherConfig{BaseURL: upstream.URL, UserAgent: "test", MinPeriod: 0, HTTPClient: upstream.Client()})
|
||||
e := NewEnricher(pool, discardLogger(), f, true)
|
||||
|
||||
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)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user