diff --git a/internal/api/admin_tracks_test.go b/internal/api/admin_tracks_test.go index dc93f7db..90b78447 100644 --- a/internal/api/admin_tracks_test.go +++ b/internal/api/admin_tracks_test.go @@ -42,7 +42,7 @@ func (f *fakeLidarrUnmonitorer) UnmonitorTrack(_ context.Context, trackMbid, alb func installTracksLidarrStub(t *testing.T, h *handlers) *fakeLidarrUnmonitorer { t.Helper() fake := &fakeLidarrUnmonitorer{} - h.tracks = tracks.NewService(h.pool, h.logger, fake) + h.tracks = tracks.NewService(h.pool, h.logger, fake, h.dataDir) return fake } diff --git a/internal/api/auth_test.go b/internal/api/auth_test.go index d43d5327..30c9ef30 100644 --- a/internal/api/auth_test.go +++ b/internal/api/auth_test.go @@ -66,8 +66,8 @@ func testHandlers(t *testing.T) (*handlers, *pgxpool.Pool) { // tracks.Service has no Lidarr unmonitorer in tests by default; the // admin-tracks tests below override h.tracks via installTracksLidarrStub // when they need a stubbed Lidarr. - tracksSvc := tracks.NewService(pool, logger, nil) dataDir := t.TempDir() + tracksSvc := tracks.NewService(pool, logger, nil, dataDir) playlistsSvc := playlists.NewService(pool, logger, dataDir) h := &handlers{pool: pool, logger: logger, events: w, recCfg: recCfg, rng: func() float64 { return 0.5 }, lidarrCfg: lidarrCfg, lidarrRequests: lidarrReqs, lidarrQuarantine: lidarrQuar, tracks: tracksSvc, playlists: playlistsSvc, dataDir: dataDir, scanner: nil, scanCfg: library.RunScanConfig{}} return h, pool diff --git a/internal/coverart/artist_enricher.go b/internal/coverart/artist_enricher.go new file mode 100644 index 00000000..e03778cb --- /dev/null +++ b/internal/coverart/artist_enricher.go @@ -0,0 +1,188 @@ +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 /artist-art//). +// +// 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) + } + } + + // MBID guard. + if row.Mbid == nil || *row.Mbid == "" { + return nil + } + 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, mbid) + 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. +func (e *Enricher) EnrichArtistBatch(ctx context.Context, limit int, dataDir string) (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) + } + for _, id := range rows { + if ctx.Err() != nil { + 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++ + continue + } + // Re-read to classify. + row, rerr := q.GetArtistByID(ctx, id) + if rerr != nil { + failed++ + continue + } + if row.ArtistArtSource != nil && + *row.ArtistArtSource != "" && + *row.ArtistArtSource != "none" { + succeeded++ + } else { + failed++ + } + } + return processed, succeeded, failed, nil +} + +// 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 +} diff --git a/internal/coverart/artist_enricher_test.go b/internal/coverart/artist_enricher_test.go new file mode 100644 index 00000000..15ede5d5 --- /dev/null +++ b/internal/coverart/artist_enricher_test.go @@ -0,0 +1,529 @@ +package coverart + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/jackc/pgx/v5/pgtype" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +// stubArtistProvider is a configurable ArtistArtProvider for artist enricher +// tests. Embeds fakeProvider for the base Provider methods; overrides +// FetchArtistArt with controllable byte/error responses. +type stubArtistProvider struct { + fakeProvider + thumb, fanart []byte + err error +} + +func (f *stubArtistProvider) FetchArtistArt(_ context.Context, _ string) ([]byte, []byte, error) { + return f.thumb, f.fanart, f.err +} + +// insertArtist seeds a fresh artist via UpsertArtist and returns its UUID. +// mbid may be "" to seed a MBID-less artist. +func insertArtist(t *testing.T, q *dbq.Queries, ctx context.Context, name, mbid string) pgtype.UUID { + t.Helper() + var mbidParam *string + if mbid != "" { + mbidParam = &mbid + } + a, err := q.UpsertArtist(ctx, dbq.UpsertArtistParams{ + Name: name, + SortName: name, + Mbid: mbidParam, + }) + if err != nil { + t.Fatalf("insertArtist %q: %v", name, err) + } + return a.ID +} + +// --- Eligibility tests --- + +func TestEnrichArtist_NullSource_Eligible(t *testing.T) { + pool := newPool(t) + ctx := context.Background() + q := dbq.New(pool) + + resetRegistryForTests() + t.Cleanup(resetRegistryForTests) + + stub := &stubArtistProvider{ + fakeProvider: fakeProvider{id: "stub-artist", defaultOn: true}, + thumb: []byte("thumb_bytes"), + fanart: []byte("fanart_bytes"), + } + Register(stub) + + e := newTestEnricher(t, pool) + artistID := insertArtist(t, q, ctx, "Null Artist", "11111111-1111-1111-1111-111111111111") + + dataDir := t.TempDir() + if err := e.EnrichArtist(ctx, artistID, dataDir); err != nil { + t.Fatalf("EnrichArtist: %v", err) + } + + row, err := q.GetArtistByID(ctx, artistID) + if err != nil { + t.Fatalf("GetArtistByID: %v", err) + } + if row.ArtistArtSource == nil || *row.ArtistArtSource != "stub-artist" { + t.Errorf("source = %v, want 'stub-artist'", row.ArtistArtSource) + } +} + +func TestEnrichArtist_FoundSource_Skip(t *testing.T) { + pool := newPool(t) + ctx := context.Background() + q := dbq.New(pool) + + resetRegistryForTests() + t.Cleanup(resetRegistryForTests) + + stub := &stubArtistProvider{ + fakeProvider: fakeProvider{id: "stub-artist", defaultOn: true}, + thumb: []byte("should_not_write"), + } + Register(stub) + + e := newTestEnricher(t, pool) + artistID := insertArtist(t, q, ctx, "Found Artist", "22222222-2222-2222-2222-222222222222") + + // Pre-mark with a "found" source. + src := "theaudiodb" + if err := q.SetArtistArtWithVersion(ctx, dbq.SetArtistArtWithVersionParams{ + ID: artistID, + ArtistArtSource: &src, + ArtistArtSourcesVersion: e.settings.CurrentVersion(), + }); err != nil { + t.Fatalf("set: %v", err) + } + + dataDir := t.TempDir() + if err := e.EnrichArtist(ctx, artistID, dataDir); err != nil { + t.Fatalf("EnrichArtist: %v", err) + } + + // Source must remain 'theaudiodb'; no files should have been written. + row, err := q.GetArtistByID(ctx, artistID) + if err != nil { + t.Fatalf("GetArtistByID: %v", err) + } + if row.ArtistArtSource == nil || *row.ArtistArtSource != "theaudiodb" { + t.Errorf("source = %v, want 'theaudiodb' (preserved)", row.ArtistArtSource) + } + thumbPath := filepath.Join(dataDir, "artist-art", uuidString(artistID), "thumb.jpg") + if _, err := os.Stat(thumbPath); !os.IsNotExist(err) { + t.Errorf("expected no thumb.jpg (skip), got stat err = %v", err) + } +} + +func TestEnrichArtist_NoneCurrentVersion_Skip(t *testing.T) { + pool := newPool(t) + ctx := context.Background() + q := dbq.New(pool) + + resetRegistryForTests() + t.Cleanup(resetRegistryForTests) + + stub := &stubArtistProvider{ + fakeProvider: fakeProvider{id: "stub-artist", defaultOn: true}, + thumb: []byte("should_not_write"), + } + Register(stub) + + e := newTestEnricher(t, pool) + artistID := insertArtist(t, q, ctx, "None Current Artist", "33333333-3333-3333-3333-333333333333") + + src := "none" + if err := q.SetArtistArtWithVersion(ctx, dbq.SetArtistArtWithVersionParams{ + ID: artistID, + ArtistArtSource: &src, + ArtistArtSourcesVersion: e.settings.CurrentVersion(), + }); err != nil { + t.Fatalf("set: %v", err) + } + + dataDir := t.TempDir() + if err := e.EnrichArtist(ctx, artistID, dataDir); err != nil { + t.Fatalf("EnrichArtist: %v", err) + } + + // Source must remain 'none' at current version. + row, err := q.GetArtistByID(ctx, artistID) + if err != nil { + t.Fatalf("GetArtistByID: %v", err) + } + if row.ArtistArtSource == nil || *row.ArtistArtSource != "none" { + t.Errorf("source = %v, want 'none' (skip — current version)", row.ArtistArtSource) + } +} + +func TestEnrichArtist_NoneStaleVersion_FlipAndRetry(t *testing.T) { + pool := newPool(t) + ctx := context.Background() + q := dbq.New(pool) + + resetRegistryForTests() + t.Cleanup(resetRegistryForTests) + + stub := &stubArtistProvider{ + fakeProvider: fakeProvider{id: "stub-artist", defaultOn: true}, + thumb: []byte("retry_thumb"), + fanart: []byte("retry_fanart"), + } + Register(stub) + + e := newTestEnricher(t, pool) + artistID := insertArtist(t, q, ctx, "Stale None Artist", "44444444-4444-4444-4444-444444444444") + + // Set 'none' with a stale version (currentVersion - 1). + staleVersion := e.settings.CurrentVersion() - 1 + src := "none" + if err := q.SetArtistArtWithVersion(ctx, dbq.SetArtistArtWithVersionParams{ + ID: artistID, + ArtistArtSource: &src, + ArtistArtSourcesVersion: staleVersion, + }); err != nil { + t.Fatalf("set stale none: %v", err) + } + + dataDir := t.TempDir() + if err := e.EnrichArtist(ctx, artistID, dataDir); err != nil { + t.Fatalf("EnrichArtist: %v", err) + } + + row, err := q.GetArtistByID(ctx, artistID) + if err != nil { + t.Fatalf("GetArtistByID: %v", err) + } + // Stale 'none' should be flipped and re-enriched; stub succeeds so + // source becomes the provider ID. + if row.ArtistArtSource == nil || *row.ArtistArtSource != "stub-artist" { + t.Errorf("source = %v, want 'stub-artist' (stale none flipped and retried)", row.ArtistArtSource) + } +} + +// --- Atomicity: thumb-only / fanart-only --- + +func TestEnrichArtist_ThumbOnly_ThumbPathSetFanartNull(t *testing.T) { + pool := newPool(t) + ctx := context.Background() + q := dbq.New(pool) + + resetRegistryForTests() + t.Cleanup(resetRegistryForTests) + + stub := &stubArtistProvider{ + fakeProvider: fakeProvider{id: "thumb-only-provider", defaultOn: true}, + thumb: []byte("thumb_only"), + fanart: nil, + } + Register(stub) + + e := newTestEnricher(t, pool) + artistID := insertArtist(t, q, ctx, "Thumb Only Artist", "55555555-5555-5555-5555-555555555555") + + dataDir := t.TempDir() + if err := e.EnrichArtist(ctx, artistID, dataDir); err != nil { + t.Fatalf("EnrichArtist: %v", err) + } + + row, err := q.GetArtistByID(ctx, artistID) + if err != nil { + t.Fatalf("GetArtistByID: %v", err) + } + if row.ArtistArtSource == nil || *row.ArtistArtSource != "thumb-only-provider" { + t.Errorf("source = %v, want 'thumb-only-provider'", row.ArtistArtSource) + } + if row.ArtistThumbPath == nil { + t.Errorf("artist_thumb_path is nil, want a path") + } + if row.ArtistFanartPath != nil { + t.Errorf("artist_fanart_path = %v, want nil (fanart not provided)", row.ArtistFanartPath) + } + if row.ArtistThumbPath != nil { + body, rerr := os.ReadFile(*row.ArtistThumbPath) + if rerr != nil { + t.Fatalf("read thumb: %v", rerr) + } + if string(body) != "thumb_only" { + t.Errorf("thumb content = %q, want 'thumb_only'", string(body)) + } + } +} + +func TestEnrichArtist_FanartOnly_FanartPathSetThumbNull(t *testing.T) { + pool := newPool(t) + ctx := context.Background() + q := dbq.New(pool) + + resetRegistryForTests() + t.Cleanup(resetRegistryForTests) + + stub := &stubArtistProvider{ + fakeProvider: fakeProvider{id: "fanart-only-provider", defaultOn: true}, + thumb: nil, + fanart: []byte("fanart_only"), + } + Register(stub) + + e := newTestEnricher(t, pool) + artistID := insertArtist(t, q, ctx, "Fanart Only Artist", "66666666-6666-6666-6666-666666666666") + + dataDir := t.TempDir() + if err := e.EnrichArtist(ctx, artistID, dataDir); err != nil { + t.Fatalf("EnrichArtist: %v", err) + } + + row, err := q.GetArtistByID(ctx, artistID) + if err != nil { + t.Fatalf("GetArtistByID: %v", err) + } + if row.ArtistArtSource == nil || *row.ArtistArtSource != "fanart-only-provider" { + t.Errorf("source = %v, want 'fanart-only-provider'", row.ArtistArtSource) + } + if row.ArtistThumbPath != nil { + t.Errorf("artist_thumb_path = %v, want nil (thumb not provided)", row.ArtistThumbPath) + } + if row.ArtistFanartPath == nil { + t.Errorf("artist_fanart_path is nil, want a path") + } +} + +// --- All-404 settles to 'none' --- + +func TestEnrichArtist_All404_SettlesNone(t *testing.T) { + pool := newPool(t) + ctx := context.Background() + q := dbq.New(pool) + + resetRegistryForTests() + t.Cleanup(resetRegistryForTests) + + stub := &stubArtistProvider{ + fakeProvider: fakeProvider{id: "404-provider", defaultOn: true}, + err: ErrNotFound, + } + Register(stub) + + e := newTestEnricher(t, pool) + artistID := insertArtist(t, q, ctx, "All404 Artist", "77777777-7777-7777-7777-777777777777") + + dataDir := t.TempDir() + if err := e.EnrichArtist(ctx, artistID, dataDir); err != nil { + t.Fatalf("EnrichArtist: %v", err) + } + + row, err := q.GetArtistByID(ctx, artistID) + if err != nil { + t.Fatalf("GetArtistByID: %v", err) + } + if row.ArtistArtSource == nil || *row.ArtistArtSource != "none" { + t.Errorf("source = %v, want 'none' (all providers 404)", row.ArtistArtSource) + } + if row.ArtistArtSourcesVersion != e.settings.CurrentVersion() { + t.Errorf("version = %d, want %d", row.ArtistArtSourcesVersion, e.settings.CurrentVersion()) + } +} + +// --- Empty providers list settles vacuously to 'none' --- + +func TestEnrichArtist_EmptyProviders_SettlesNone(t *testing.T) { + pool := newPool(t) + ctx := context.Background() + q := dbq.New(pool) + + // Register a plain provider that does NOT implement ArtistArtProvider — + // EnabledArtistProviders will return an empty slice. + resetRegistryForTests() + t.Cleanup(resetRegistryForTests) + Register(&fakeProvider{id: "album-only", defaultOn: true}) + + e := newTestEnricher(t, pool) + artistID := insertArtist(t, q, ctx, "No Provider Artist", "88888888-8888-8888-8888-888888888888") + + dataDir := t.TempDir() + if err := e.EnrichArtist(ctx, artistID, dataDir); err != nil { + t.Fatalf("EnrichArtist: %v", err) + } + + row, err := q.GetArtistByID(ctx, artistID) + if err != nil { + t.Fatalf("GetArtistByID: %v", err) + } + if row.ArtistArtSource == nil || *row.ArtistArtSource != "none" { + t.Errorf("source = %v, want 'none' (vacuous all-404 with empty provider list)", row.ArtistArtSource) + } +} + +// --- ErrTransient leaves row NULL --- + +func TestEnrichArtist_ErrTransient_LeavesNull(t *testing.T) { + pool := newPool(t) + ctx := context.Background() + q := dbq.New(pool) + + resetRegistryForTests() + t.Cleanup(resetRegistryForTests) + + stub := &stubArtistProvider{ + fakeProvider: fakeProvider{id: "transient-provider", defaultOn: true}, + err: ErrTransient, + } + Register(stub) + + e := newTestEnricher(t, pool) + artistID := insertArtist(t, q, ctx, "Transient Artist", "99999999-9999-9999-9999-999999999999") + + dataDir := t.TempDir() + if err := e.EnrichArtist(ctx, artistID, dataDir); err != nil { + t.Fatalf("EnrichArtist: %v", err) + } + + row, err := q.GetArtistByID(ctx, artistID) + if err != nil { + t.Fatalf("GetArtistByID: %v", err) + } + // Transient: source must remain NULL (eligible for retry). + if row.ArtistArtSource != nil { + t.Errorf("source = %v, want nil (transient should not settle)", row.ArtistArtSource) + } +} + +// --- Filesystem layout --- + +func TestEnrichArtist_FilesystemLayout(t *testing.T) { + pool := newPool(t) + ctx := context.Background() + q := dbq.New(pool) + + resetRegistryForTests() + t.Cleanup(resetRegistryForTests) + + thumbBytes := []byte("thumb_content") + fanartBytes := []byte("fanart_content") + stub := &stubArtistProvider{ + fakeProvider: fakeProvider{id: "layout-provider", defaultOn: true}, + thumb: thumbBytes, + fanart: fanartBytes, + } + Register(stub) + + e := newTestEnricher(t, pool) + artistID := insertArtist(t, q, ctx, "Layout Artist", "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa") + + dataDir := t.TempDir() + if err := e.EnrichArtist(ctx, artistID, dataDir); err != nil { + t.Fatalf("EnrichArtist: %v", err) + } + + artistIDStr := uuidString(artistID) + thumbPath := filepath.Join(dataDir, "artist-art", artistIDStr, "thumb.jpg") + fanartPath := filepath.Join(dataDir, "artist-art", artistIDStr, "fanart.jpg") + + thumbBody, rerr := os.ReadFile(thumbPath) + if rerr != nil { + t.Fatalf("read thumb.jpg: %v", rerr) + } + if string(thumbBody) != string(thumbBytes) { + t.Errorf("thumb.jpg = %q, want %q", thumbBody, thumbBytes) + } + + fanartBody, rerr := os.ReadFile(fanartPath) + if rerr != nil { + t.Fatalf("read fanart.jpg: %v", rerr) + } + if string(fanartBody) != string(fanartBytes) { + t.Errorf("fanart.jpg = %q, want %q", fanartBody, fanartBytes) + } + + row, err := q.GetArtistByID(ctx, artistID) + if err != nil { + t.Fatalf("GetArtistByID: %v", err) + } + if row.ArtistThumbPath == nil || *row.ArtistThumbPath != thumbPath { + t.Errorf("DB thumb_path = %v, want %s", row.ArtistThumbPath, thumbPath) + } + if row.ArtistFanartPath == nil || *row.ArtistFanartPath != fanartPath { + t.Errorf("DB fanart_path = %v, want %s", row.ArtistFanartPath, fanartPath) + } +} + +// --- CleanupArtistArt --- + +func TestCleanupArtistArt_RemovesDirectory(t *testing.T) { + dataDir := t.TempDir() + + artistID := pgtype.UUID{ + Bytes: [16]byte{0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb}, + Valid: true, + } + artistDir := filepath.Join(dataDir, "artist-art", uuidString(artistID)) + if err := os.MkdirAll(artistDir, 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + if err := os.WriteFile(filepath.Join(artistDir, "thumb.jpg"), []byte("x"), 0o644); err != nil { + t.Fatalf("write: %v", err) + } + + if err := CleanupArtistArt(dataDir, artistID); err != nil { + t.Fatalf("CleanupArtistArt: %v", err) + } + + if _, err := os.Stat(artistDir); !os.IsNotExist(err) { + t.Errorf("directory still exists after cleanup: stat err = %v", err) + } +} + +func TestCleanupArtistArt_IdempotentMissingDir(t *testing.T) { + dataDir := t.TempDir() + artistID := pgtype.UUID{ + Bytes: [16]byte{0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc}, + Valid: true, + } + + // Directory does not exist — should be a no-op. + if err := CleanupArtistArt(dataDir, artistID); err != nil { + t.Fatalf("CleanupArtistArt on missing dir: %v", err) + } +} + +// --- MBID guard --- + +func TestEnrichArtist_NoMBID_LeavesNull(t *testing.T) { + pool := newPool(t) + ctx := context.Background() + q := dbq.New(pool) + + resetRegistryForTests() + t.Cleanup(resetRegistryForTests) + + stub := &stubArtistProvider{ + fakeProvider: fakeProvider{id: "stub-artist", defaultOn: true}, + thumb: []byte("should_not_write"), + } + Register(stub) + + e := newTestEnricher(t, pool) + // Seed with no MBID. + artistID := insertArtist(t, q, ctx, "No MBID Artist", "") + + dataDir := t.TempDir() + if err := e.EnrichArtist(ctx, artistID, dataDir); err != nil { + t.Fatalf("EnrichArtist: %v", err) + } + + row, err := q.GetArtistByID(ctx, artistID) + if err != nil { + t.Fatalf("GetArtistByID: %v", err) + } + if row.ArtistArtSource != nil { + t.Errorf("source = %v, want nil (no MBID — skip)", row.ArtistArtSource) + } +} diff --git a/internal/server/server.go b/internal/server/server.go index 7c119033..3120fdd7 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -109,7 +109,7 @@ func (s *Server) Router() http.Handler { } lidarrReqs := lidarrrequests.NewService(s.Pool, lidarrCfg, lidarrClientFn, nil) lidarrQuar := lidarrquarantine.NewService(s.Pool, lidarrCfg, lidarrClientFn) - tracksSvc := tracks.NewService(s.Pool, s.Logger, lidarrUnmonitorAdapter{fn: lidarrClientFn}) + tracksSvc := tracks.NewService(s.Pool, s.Logger, lidarrUnmonitorAdapter{fn: lidarrClientFn}, s.DataDir) playlistsSvc := playlists.NewService(s.Pool, s.Logger, s.DataDir) api.Mount(r, s.Pool, s.Logger, writer, s.RecommendationCfg, lidarrCfg, lidarrReqs, lidarrQuar, tracksSvc, playlistsSvc, s.CoverEnricher, s.CoverArtBackfillCap, s.LibraryScanner, s.ScanCfg, s.DataDir) // /api/admin/scan is the only admin route owned by the server package diff --git a/internal/tracks/service.go b/internal/tracks/service.go index 53574157..37baffca 100644 --- a/internal/tracks/service.go +++ b/internal/tracks/service.go @@ -25,6 +25,7 @@ import ( "github.com/jackc/pgx/v5/pgtype" "github.com/jackc/pgx/v5/pgxpool" + "git.fabledsword.com/bvandeusen/minstrel/internal/coverart" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" ) @@ -48,18 +49,21 @@ type LidarrUnmonitorer interface { // the right fallback when Lidarr isn't configured: file + DB delete // still happen. type Service struct { - pool *pgxpool.Pool - logger *slog.Logger - lidarr LidarrUnmonitorer + pool *pgxpool.Pool + logger *slog.Logger + lidarr LidarrUnmonitorer + dataDir string } // NewService constructs a Service. logger may be nil (defaults to // slog.Default). lidarr may be nil to disable the unmonitor branch. -func NewService(pool *pgxpool.Pool, logger *slog.Logger, lidarr LidarrUnmonitorer) *Service { +// dataDir is the on-disk root for cached artifacts; used to clean up +// artist-art on artist delete. Empty string disables the cleanup. +func NewService(pool *pgxpool.Pool, logger *slog.Logger, lidarr LidarrUnmonitorer, dataDir string) *Service { if logger == nil { logger = slog.Default() } - return &Service{pool: pool, logger: logger, lidarr: lidarr} + return &Service{pool: pool, logger: logger, lidarr: lidarr, dataDir: dataDir} } // RemoveTrack deletes the file from disk and the DB rows, runs the @@ -159,6 +163,16 @@ func (s *Service) RemoveTrack( return nil, nil, false, fmt.Errorf("commit: %w", err) } + // Cleanup artist-art filesystem cache if the delete cascade orphaned + // an artist. Non-fatal — the destructive part is done; we just want + // to keep the dataDir tidy. + if deletedArtistID != nil && s.dataDir != "" { + if err := coverart.CleanupArtistArt(s.dataDir, *deletedArtistID); err != nil { + s.logger.Warn("track delete: artist-art cleanup failed", + "artist_id", *deletedArtistID, "err", err) + } + } + // Lidarr unmonitor — non-fatal. The destructive part is done; any // failure here is informational so the operator can retry manually. if unmonitor && track.Mbid != nil && *track.Mbid != "" && s.lidarr != nil {