425f12db38
Changes the AlbumCoverProvider and ArtistArtProvider interfaces from MBID-only string parameters to ref-struct signatures so name-based providers (Deezer, Last.fm in upcoming tasks) can act on rows without MBIDs. Today, 53 of 213 artists in the dev DB have NULL MBIDs — featured artists, remixers, and compilation contributors created from track-tag splits where the primary-artist MBID is in the file but collaborator IDs aren't. These rows are unreachable by the current MBID-only chain. Refactoring the interface unblocks future name-based providers from acting on them. TheAudioDB and MBCAA keep MBID-only behavior internally: they return ErrNotFound when ref.MBID is empty rather than attempting a name-based fallback. The MBID guard inside EnrichAlbum/EnrichArtist is removed; providers receive the ref unconditionally and decide whether they can act on it. GetAlbumWithFirstTrackPath now JOINs artists to return artist_name alongside the existing fields, supporting AlbumRef construction. No behavioral change today (only TheAudioDB + MBCAA registered, both keep MBID-only behavior). The change unblocks T3 (Deezer) and T4 (Last.fm) which can now register and act on every artist/album regardless of MBID presence. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
235 lines
7.8 KiB
Go
235 lines
7.8 KiB
Go
package coverart
|
|
|
|
import (
|
|
"context"
|
|
"errors"
|
|
"fmt"
|
|
"os"
|
|
"path/filepath"
|
|
|
|
"github.com/jackc/pgx/v5"
|
|
"github.com/jackc/pgx/v5/pgtype"
|
|
|
|
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
|
|
)
|
|
|
|
// EnrichArtist runs the chain for a single artist. Parallel to
|
|
// EnrichAlbum, but no sidecar layer (artists have no per-artist
|
|
// directory in a music library — images live in a managed cache
|
|
// under <dataDir>/artist-art/<artist_id>/).
|
|
//
|
|
// Eligibility:
|
|
// - artist_art_source IS NULL → eligible
|
|
// - artist_art_source IN (theaudiodb) → SKIP (found)
|
|
// - artist_art_source = 'none' AND version = current → SKIP
|
|
// - artist_art_source = 'none' AND version != current → flip to NULL, then enrich
|
|
//
|
|
// Chain: enabled ArtistArtProviders in registration order. First
|
|
// success returns. All-ErrNotFound settles to 'none'. Any
|
|
// ErrTransient leaves the row NULL for next-pass retry.
|
|
func (e *Enricher) EnrichArtist(ctx context.Context, artistID pgtype.UUID, dataDir string) error {
|
|
q := dbq.New(e.pool)
|
|
row, err := q.GetArtistByID(ctx, artistID)
|
|
if err != nil {
|
|
if errors.Is(err, pgx.ErrNoRows) {
|
|
return nil
|
|
}
|
|
return fmt.Errorf("get artist: %w", err)
|
|
}
|
|
|
|
currentVersion := e.settings.CurrentVersion()
|
|
|
|
// Eligibility check.
|
|
currentSource := ""
|
|
if row.ArtistArtSource != nil {
|
|
currentSource = *row.ArtistArtSource
|
|
}
|
|
switch currentSource {
|
|
case "theaudiodb":
|
|
return nil
|
|
case "none":
|
|
if row.ArtistArtSourcesVersion == currentVersion {
|
|
return nil
|
|
}
|
|
if err := q.ClearArtistArtNone(ctx, artistID); err != nil {
|
|
return fmt.Errorf("clear stale artist none: %w", err)
|
|
}
|
|
}
|
|
|
|
// Build the ref. Every provider receives it and decides whether it
|
|
// can act on the fields available (MBID-only providers return
|
|
// ErrNotFound when MBID is empty; name-based providers fall back to
|
|
// Name).
|
|
ref := ArtistRef{Name: row.Name}
|
|
if row.Mbid != nil {
|
|
ref.MBID = *row.Mbid
|
|
}
|
|
|
|
// Provider chain. allWere404 tracks whether every provider returned
|
|
// ErrNotFound — only then do we settle to 'none'. Any transient
|
|
// failure leaves the row NULL for next-pass retry.
|
|
allWere404 := true
|
|
for _, provider := range e.settings.EnabledArtistProviders() {
|
|
thumb, fanart, perr := provider.FetchArtistArt(ctx, ref)
|
|
if perr == nil {
|
|
// Atomic write — at least one of (thumb, fanart) is non-nil
|
|
// per the ArtistArtProvider contract; persist whichever
|
|
// landed.
|
|
artistDir := filepath.Join(dataDir, "artist-art", uuidString(artistID))
|
|
if mkErr := os.MkdirAll(artistDir, 0o755); mkErr != nil {
|
|
return fmt.Errorf("mkdir artist-art: %w", mkErr)
|
|
}
|
|
var thumbPath, fanartPath string
|
|
if thumb != nil {
|
|
thumbPath = filepath.Join(artistDir, "thumb.jpg")
|
|
if werr := os.WriteFile(thumbPath, thumb, 0o644); werr != nil {
|
|
e.logger.Error("coverart: write artist thumb failed",
|
|
"artist_id", uuidString(artistID), "err", werr)
|
|
thumbPath = ""
|
|
}
|
|
}
|
|
if fanart != nil {
|
|
fanartPath = filepath.Join(artistDir, "fanart.jpg")
|
|
if werr := os.WriteFile(fanartPath, fanart, 0o644); werr != nil {
|
|
e.logger.Error("coverart: write artist fanart failed",
|
|
"artist_id", uuidString(artistID), "err", werr)
|
|
fanartPath = ""
|
|
}
|
|
}
|
|
return e.recordArtistArt(ctx, artistID, thumbPath, fanartPath, provider.ID(), currentVersion)
|
|
}
|
|
if !errors.Is(perr, ErrNotFound) {
|
|
allWere404 = false
|
|
e.logger.Warn("coverart: artist provider fetch failed; trying next",
|
|
"artist_id", uuidString(artistID),
|
|
"provider", provider.ID(), "err", perr)
|
|
}
|
|
// ErrNotFound: continue to next provider; allWere404 stays true.
|
|
}
|
|
|
|
if allWere404 {
|
|
return e.recordArtistArt(ctx, artistID, "", "", "none", currentVersion)
|
|
}
|
|
return nil // at least one transient failure; leave NULL for retry
|
|
}
|
|
|
|
// recordArtistArt writes the new artist_thumb_path /
|
|
// artist_fanart_path / artist_art_source / artist_art_sources_version
|
|
// atomically.
|
|
func (e *Enricher) recordArtistArt(ctx context.Context, artistID pgtype.UUID, thumbPath, fanartPath, source string, version int32) error {
|
|
q := dbq.New(e.pool)
|
|
src := source
|
|
var thumbArg, fanartArg *string
|
|
if thumbPath != "" {
|
|
thumbArg = &thumbPath
|
|
}
|
|
if fanartPath != "" {
|
|
fanartArg = &fanartPath
|
|
}
|
|
return q.SetArtistArtWithVersion(ctx, dbq.SetArtistArtWithVersionParams{
|
|
ID: artistID,
|
|
ArtistThumbPath: thumbArg,
|
|
ArtistFanartPath: fanartArg,
|
|
ArtistArtSource: &src,
|
|
ArtistArtSourcesVersion: version,
|
|
})
|
|
}
|
|
|
|
// EnrichArtistBatch drains up to limit artists whose eligibility loop
|
|
// determines they need processing. Iterates serially. Returns
|
|
// per-batch tallies for the orchestrator's scan-run record.
|
|
//
|
|
// Emits a single Info log line at end-of-batch with a category
|
|
// breakdown (succeeded / settled-none / no-MBID / left-NULL /
|
|
// errored). The JSON tally written to scan_runs keeps the existing
|
|
// processed/succeeded/failed shape for backwards compat with the
|
|
// admin overview UI; the log line is the operator's diagnostic.
|
|
func (e *Enricher) EnrichArtistBatch(ctx context.Context, limit int, dataDir string,
|
|
progressCb func(processed, succeeded, failed int)) (processed, succeeded, failed int, err error) {
|
|
if limit <= 0 {
|
|
return 0, 0, 0, nil
|
|
}
|
|
q := dbq.New(e.pool)
|
|
rows, qerr := q.ListArtistsMissingArt(ctx, dbq.ListArtistsMissingArtParams{
|
|
ArtistArtSourcesVersion: e.settings.CurrentVersion(),
|
|
Limit: int32(limit),
|
|
})
|
|
if qerr != nil {
|
|
return 0, 0, 0, fmt.Errorf("list missing artist art: %w", qerr)
|
|
}
|
|
var settledNone, noMBID, leftNull, errored int
|
|
for _, id := range rows {
|
|
if ctx.Err() != nil {
|
|
e.logArtistBatchSummary(len(rows), processed, succeeded, settledNone, noMBID, leftNull, errored)
|
|
return processed, succeeded, failed, ctx.Err()
|
|
}
|
|
processed++
|
|
if eerr := e.EnrichArtist(ctx, id, dataDir); eerr != nil {
|
|
e.logger.Warn("coverart: artist batch entry failed",
|
|
"artist_id", uuidString(id), "err", eerr)
|
|
failed++
|
|
errored++
|
|
if progressCb != nil {
|
|
progressCb(processed, succeeded, failed)
|
|
}
|
|
continue
|
|
}
|
|
// Re-read to classify.
|
|
row, rerr := q.GetArtistByID(ctx, id)
|
|
if rerr != nil {
|
|
failed++
|
|
errored++
|
|
if progressCb != nil {
|
|
progressCb(processed, succeeded, failed)
|
|
}
|
|
continue
|
|
}
|
|
switch {
|
|
case row.ArtistArtSource != nil &&
|
|
*row.ArtistArtSource != "" &&
|
|
*row.ArtistArtSource != "none":
|
|
succeeded++
|
|
case row.ArtistArtSource != nil && *row.ArtistArtSource == "none":
|
|
settledNone++
|
|
failed++
|
|
case row.Mbid == nil || *row.Mbid == "":
|
|
noMBID++
|
|
failed++
|
|
default:
|
|
leftNull++
|
|
failed++
|
|
}
|
|
if progressCb != nil {
|
|
progressCb(processed, succeeded, failed)
|
|
}
|
|
}
|
|
e.logArtistBatchSummary(len(rows), processed, succeeded, settledNone, noMBID, leftNull, errored)
|
|
return processed, succeeded, failed, nil
|
|
}
|
|
|
|
// logArtistBatchSummary emits a single Info line at end-of-batch with
|
|
// the full category breakdown including MBID-skipped (artists eligible
|
|
// by the SQL filter but skipped inside EnrichArtist due to NULL MBID).
|
|
func (e *Enricher) logArtistBatchSummary(eligible, processed, succeeded, settledNone, noMBID, leftNull, errored int) {
|
|
e.logger.Info("coverart: artist enrichment batch complete",
|
|
"eligible", eligible,
|
|
"processed", processed,
|
|
"succeeded", succeeded,
|
|
"settled_none", settledNone,
|
|
"no_mbid", noMBID,
|
|
"left_null", leftNull,
|
|
"errored", errored)
|
|
}
|
|
|
|
// CleanupArtistArt removes the artist-art directory for a deleted
|
|
// artist. Idempotent — no-op if the directory doesn't exist. Called
|
|
// from the artist-delete cascade in tracks/service.go after the
|
|
// transaction commits.
|
|
func CleanupArtistArt(dataDir string, artistID pgtype.UUID) error {
|
|
dir := filepath.Join(dataDir, "artist-art", uuidString(artistID))
|
|
if err := os.RemoveAll(dir); err != nil {
|
|
return fmt.Errorf("remove artist-art dir: %w", err)
|
|
}
|
|
return nil
|
|
}
|