feat(tracks): RemoveTrack service with Lidarr-aware delete + cascade

RemoveTrack handles both Lidarr-managed (delegates to existing
lidarrquarantine.DeleteViaLidarr) and non-Lidarr (direct os.Remove)
file deletion, then runs the cascade album-if-empty / artist-if-empty
DB cleanup in a single transaction. Lidarr errors propagate as typed
errors so the API layer can map them to the existing wire codes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-05-02 21:58:12 -04:00
parent 30b0edf97b
commit 50a231fdc1
2 changed files with 534 additions and 0 deletions
+188
View File
@@ -0,0 +1,188 @@
// Package tracks owns the track-level admin actions exposed by the
// M7 #372 track-actions menu. Today that's RemoveTrack, which deletes
// a single track (Lidarr-managed via lidarrquarantine.DeleteViaLidarr,
// or local via os.Remove) and cascades the album-empty / artist-empty
// tidy-up so we don't leak orphan rows.
package tracks
import (
"context"
"errors"
"fmt"
"log/slog"
"os"
"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"
"git.fabledsword.com/bvandeusen/minstrel/internal/lidarr"
)
// Public errors. Handlers map these to API codes.
var (
ErrNotFound = errors.New("tracks: not found")
ErrLidarrUnreachable = errors.New("tracks: lidarr unreachable")
ErrLidarrUnauthorized = errors.New("tracks: lidarr unauthorized")
ErrLidarrServerError = errors.New("tracks: lidarr server error")
)
// LidarrDeleter is the subset of *lidarrquarantine.Service the tracks
// service needs. Defined as an interface so tests can stub without
// spinning up a real Lidarr stub.
type LidarrDeleter interface {
DeleteViaLidarr(ctx context.Context, trackID, adminID pgtype.UUID) (dbq.LidarrQuarantineAction, int, error)
}
// Service owns RemoveTrack. lidarrDeleter may be nil — non-Lidarr-managed
// tracks (no mbid) still work fine; a Lidarr-managed track with a nil
// deleter falls back to direct os.Remove of the local file.
type Service struct {
pool *pgxpool.Pool
logger *slog.Logger
lidarrDeleter LidarrDeleter
}
// NewService constructs a Service. logger may be nil (defaults to
// slog.Default). lidarrDeleter may be nil to disable the Lidarr-aware
// path entirely.
func NewService(pool *pgxpool.Pool, logger *slog.Logger, lidarrDeleter LidarrDeleter) *Service {
if logger == nil {
logger = slog.Default()
}
return &Service{pool: pool, logger: logger, lidarrDeleter: lidarrDeleter}
}
// RemoveTrack deletes the track, runs the album-empty / artist-empty
// cascade, and returns the IDs of any rows deleted by the cascade so
// the API caller can invalidate caches.
//
// For Lidarr-managed tracks (mbid set on the track), Lidarr is told to
// delete the file first via lidarrquarantine.DeleteViaLidarr — this
// sets the import-list-exclusion flag so the next sync doesn't re-import
// the file. For local tracks (no mbid), os.Remove handles the file.
//
// Lidarr errors propagate as typed errors and the DB is left untouched
// so the operator can retry. A missing local file is tolerated — DB
// consistency takes priority over a noisy filesystem.
func (s *Service) RemoveTrack(ctx context.Context, trackID, adminID pgtype.UUID) (deletedAlbumID, deletedArtistID *pgtype.UUID, err error) {
q := dbq.New(s.pool)
track, err := q.GetTrackByID(ctx, trackID)
if err != nil {
if errors.Is(err, pgx.ErrNoRows) {
return nil, nil, ErrNotFound
}
return nil, nil, fmt.Errorf("get track: %w", err)
}
// Lidarr-managed → Lidarr deletes the album (and so the file) first.
// The lidarrquarantine path also deletes the local rows for every
// track on the album, so we pre-empt the cascade-tidy step here. If
// the lidarr deleter is nil, fall through to the local os.Remove path.
if track.Mbid != nil && *track.Mbid != "" && s.lidarrDeleter != nil {
_, _, lerr := s.lidarrDeleter.DeleteViaLidarr(ctx, trackID, adminID)
switch {
case lerr == nil:
// lidarrquarantine.DeleteViaLidarr already removed the tracks
// row(s) for the album. Cascade cleanup still needs to run
// against the album/artist IDs we captured before the call.
return s.cascadeAfterLidarr(ctx, q, track)
case errors.Is(lerr, lidarr.ErrUnreachable):
return nil, nil, ErrLidarrUnreachable
case errors.Is(lerr, lidarr.ErrAuthFailed):
return nil, nil, ErrLidarrUnauthorized
case errors.Is(lerr, lidarr.ErrServerError):
return nil, nil, ErrLidarrServerError
default:
return nil, nil, fmt.Errorf("lidarr delete: %w", lerr)
}
}
// Local path: best-effort os.Remove, then DB cleanup in a tx.
if track.FilePath != "" {
if rerr := os.Remove(track.FilePath); rerr != nil && !errors.Is(rerr, os.ErrNotExist) {
s.logger.Warn("track delete: file remove failed",
"path", track.FilePath, "track_id", trackID, "err", rerr)
// Proceed: DB consistency is the priority.
}
}
return s.deleteAndCascade(ctx, trackID)
}
// deleteAndCascade runs DeleteTrack → DeleteAlbumIfEmpty →
// DeleteArtistIfEmpty in a single transaction. Returns the cascaded
// album / artist IDs (nil pointers when the cascade didn't fire because
// other tracks/albums still reference the parent).
func (s *Service) deleteAndCascade(ctx context.Context, trackID pgtype.UUID) (deletedAlbumID, deletedArtistID *pgtype.UUID, err error) {
tx, err := s.pool.Begin(ctx)
if err != nil {
return nil, nil, fmt.Errorf("begin tx: %w", err)
}
defer func() { _ = tx.Rollback(ctx) }()
tq := dbq.New(tx)
deleted, err := tq.DeleteTrack(ctx, trackID)
if err != nil {
return nil, nil, fmt.Errorf("delete track: %w", err)
}
albumRow, aerr := tq.DeleteAlbumIfEmpty(ctx, deleted.AlbumID)
switch {
case aerr == nil:
albumID := albumRow.ID
deletedAlbumID = &albumID
artistID, arerr := tq.DeleteArtistIfEmpty(ctx, albumRow.ArtistID)
switch {
case arerr == nil:
id := artistID
deletedArtistID = &id
case errors.Is(arerr, pgx.ErrNoRows):
// artist still has other albums or stray tracks
default:
return nil, nil, fmt.Errorf("delete artist if empty: %w", arerr)
}
case errors.Is(aerr, pgx.ErrNoRows):
// album still has other tracks
default:
return nil, nil, fmt.Errorf("delete album if empty: %w", aerr)
}
if err := tx.Commit(ctx); err != nil {
return nil, nil, fmt.Errorf("commit: %w", err)
}
return deletedAlbumID, deletedArtistID, nil
}
// cascadeAfterLidarr runs the album-empty / artist-empty checks against
// the album/artist IDs from the track snapshot we took before delegating
// to lidarrquarantine. lidarrquarantine.DeleteViaLidarr already deleted
// the tracks row(s) for the entire album, so this is a tidy-up of the
// now-orphan album + artist parents.
func (s *Service) cascadeAfterLidarr(ctx context.Context, q *dbq.Queries, track dbq.Track) (deletedAlbumID, deletedArtistID *pgtype.UUID, err error) {
albumRow, aerr := q.DeleteAlbumIfEmpty(ctx, track.AlbumID)
switch {
case aerr == nil:
albumID := albumRow.ID
deletedAlbumID = &albumID
artistID, arerr := q.DeleteArtistIfEmpty(ctx, albumRow.ArtistID)
switch {
case arerr == nil:
id := artistID
deletedArtistID = &id
case errors.Is(arerr, pgx.ErrNoRows):
// artist still has other albums
default:
return nil, nil, fmt.Errorf("delete artist if empty: %w", arerr)
}
case errors.Is(aerr, pgx.ErrNoRows):
// album still has other tracks (unexpected after Lidarr delete,
// but tolerate — the operator may have re-imported between calls)
default:
return nil, nil, fmt.Errorf("delete album if empty: %w", aerr)
}
return deletedAlbumID, deletedArtistID, nil
}
+346
View File
@@ -0,0 +1,346 @@
package tracks
import (
"context"
"errors"
"io"
"log/slog"
"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"
"git.fabledsword.com/bvandeusen/minstrel/internal/lidarr"
)
// --- test seed helpers (inlined; mirrors lidarrquarantine patterns) -----
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 seedAdmin(t *testing.T, pool *pgxpool.Pool, name string) dbq.User {
t.Helper()
u, err := dbq.New(pool).CreateUser(context.Background(), dbq.CreateUserParams{
Username: dbtest.TestUserPrefix + name, PasswordHash: "x",
ApiToken: name + "-token", IsAdmin: true,
})
if err != nil {
t.Fatalf("seed admin %s: %v", name, err)
}
return u
}
// seedArtistAlbumTrack creates a (test-prefixed) artist + album + track
// row triple. mbid is set on both the album and the track when non-empty;
// empty mbid leaves both nil so the track exercises the local-delete path.
// filePath is the on-disk path stored on the track row. Returns the three
// rows. Uses unique names per call so multiple seeds in one test don't
// collide on UNIQUE (artist, name) etc.
func seedArtistAlbumTrack(t *testing.T, pool *pgxpool.Pool, prefix, mbid, filePath string) (dbq.Artist, dbq.Album, dbq.Track) {
t.Helper()
q := dbq.New(pool)
artist, err := q.UpsertArtist(context.Background(), dbq.UpsertArtistParams{
Name: prefix + " Artist", SortName: prefix + " Artist",
})
if err != nil {
t.Fatalf("artist: %v", err)
}
var albumMBIDPtr *string
if mbid != "" {
s := mbid + "-album"
albumMBIDPtr = &s
}
album, err := q.UpsertAlbum(context.Background(), dbq.UpsertAlbumParams{
Title: prefix + " Album", SortTitle: prefix + " Album",
ArtistID: artist.ID, Mbid: albumMBIDPtr,
})
if err != nil {
t.Fatalf("album: %v", err)
}
var trackMBIDPtr *string
if mbid != "" {
s := mbid
trackMBIDPtr = &s
}
track, err := q.UpsertTrack(context.Background(), dbq.UpsertTrackParams{
Title: prefix + " Track", AlbumID: album.ID, ArtistID: artist.ID,
DurationMs: 1000, FilePath: filePath, FileSize: 1, FileFormat: "mp3",
Mbid: trackMBIDPtr,
})
if err != nil {
t.Fatalf("track: %v", err)
}
return artist, album, track
}
// --- fake LidarrDeleter -------------------------------------------------
type fakeLidarrDeleter struct {
calls []pgtype.UUID
err error
}
func (f *fakeLidarrDeleter) DeleteViaLidarr(ctx context.Context, trackID, adminID pgtype.UUID) (dbq.LidarrQuarantineAction, int, error) {
f.calls = append(f.calls, trackID)
if f.err != nil {
return dbq.LidarrQuarantineAction{}, 0, f.err
}
return dbq.LidarrQuarantineAction{}, 1, nil
}
// --- tests --------------------------------------------------------------
func TestRemoveTrack_NotFound(t *testing.T) {
pool := newPool(t)
admin := seedAdmin(t, pool, "alice")
var bogus pgtype.UUID
bogus.Bytes = [16]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}
bogus.Valid = true
svc := NewService(pool, nil, nil)
_, _, err := svc.RemoveTrack(context.Background(), bogus, admin.ID)
if !errors.Is(err, ErrNotFound) {
t.Errorf("err = %v, want ErrNotFound", err)
}
}
func TestRemoveTrack_NonLidarr_HappyPath(t *testing.T) {
pool := newPool(t)
admin := seedAdmin(t, pool, "alice")
dir := t.TempDir()
path := filepath.Join(dir, "track.mp3")
if err := os.WriteFile(path, []byte("x"), 0o644); err != nil {
t.Fatal(err)
}
// mbid empty → local-delete path.
_, _, track := seedArtistAlbumTrack(t, pool, "Solo", "", path)
svc := NewService(pool, nil, nil)
_, _, err := svc.RemoveTrack(context.Background(), track.ID, admin.ID)
if err != nil {
t.Fatalf("RemoveTrack: %v", err)
}
if _, err := os.Stat(path); !errors.Is(err, os.ErrNotExist) {
t.Errorf("file still present: %v", err)
}
q := dbq.New(pool)
if _, err := q.GetTrackByID(context.Background(), track.ID); err == nil {
t.Errorf("track row still exists")
}
}
func TestRemoveTrack_NonLidarr_FileAlreadyMissing(t *testing.T) {
pool := newPool(t)
admin := seedAdmin(t, pool, "alice")
// Path doesn't exist on disk; service should tolerate and clean up DB.
missing := filepath.Join(t.TempDir(), "does-not-exist.mp3")
_, _, track := seedArtistAlbumTrack(t, pool, "Ghost", "", missing)
svc := NewService(pool, nil, nil)
_, _, err := svc.RemoveTrack(context.Background(), track.ID, admin.ID)
if err != nil {
t.Fatalf("RemoveTrack: %v", err)
}
q := dbq.New(pool)
if _, err := q.GetTrackByID(context.Background(), track.ID); err == nil {
t.Errorf("track row still exists")
}
}
func TestRemoveTrack_Lidarr_HappyPath(t *testing.T) {
pool := newPool(t)
admin := seedAdmin(t, pool, "alice")
// mbid set → Lidarr path. Note: real lidarrquarantine.DeleteViaLidarr
// would have removed the tracks row already; the fake doesn't, so we
// use a track whose row will linger but cascadeAfterLidarr only acts
// on album/artist parents. The DB row staying behind for this test
// case is acceptable; the unit-of-work here is "DeleteViaLidarr was
// called, with the right trackID".
dir := t.TempDir()
path := filepath.Join(dir, "lidarr.mp3")
if err := os.WriteFile(path, []byte("x"), 0o644); err != nil {
t.Fatal(err)
}
_, _, track := seedArtistAlbumTrack(t, pool, "Lidarr", "track-mbid", path)
fake := &fakeLidarrDeleter{}
svc := NewService(pool, nil, fake)
_, _, err := svc.RemoveTrack(context.Background(), track.ID, admin.ID)
if err != nil {
t.Fatalf("RemoveTrack: %v", err)
}
if len(fake.calls) != 1 {
t.Fatalf("DeleteViaLidarr call count = %d, want 1", len(fake.calls))
}
if fake.calls[0] != track.ID {
t.Errorf("DeleteViaLidarr called with %v, want %v", fake.calls[0], track.ID)
}
// File should still exist — the fake doesn't actually delete it,
// and the local-os.Remove fallback is not exercised on the Lidarr path.
if _, err := os.Stat(path); err != nil {
t.Errorf("file unexpectedly missing on lidarr path: %v", err)
}
}
func TestRemoveTrack_Lidarr_ErrorPropagation(t *testing.T) {
cases := []struct {
name string
lidarrE error
wantE error
}{
{"unreachable", lidarr.ErrUnreachable, ErrLidarrUnreachable},
{"unauthorized", lidarr.ErrAuthFailed, ErrLidarrUnauthorized},
{"server", lidarr.ErrServerError, ErrLidarrServerError},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
pool := newPool(t)
admin := seedAdmin(t, pool, "alice")
dir := t.TempDir()
path := filepath.Join(dir, "lidarr.mp3")
if err := os.WriteFile(path, []byte("x"), 0o644); err != nil {
t.Fatal(err)
}
_, _, track := seedArtistAlbumTrack(t, pool, "LidErr", "track-mbid-"+tc.name, path)
fake := &fakeLidarrDeleter{err: tc.lidarrE}
svc := NewService(pool, nil, fake)
_, _, err := svc.RemoveTrack(context.Background(), track.ID, admin.ID)
if !errors.Is(err, tc.wantE) {
t.Fatalf("err = %v, want %v", err, tc.wantE)
}
// DB row must remain — Lidarr error means no local mutation.
q := dbq.New(pool)
if _, gerr := q.GetTrackByID(context.Background(), track.ID); gerr != nil {
t.Errorf("track row missing after Lidarr error: %v", gerr)
}
})
}
}
func TestRemoveTrack_Cascade_AlbumEmpty(t *testing.T) {
pool := newPool(t)
admin := seedAdmin(t, pool, "alice")
q := dbq.New(pool)
// Two-album artist: removing the sole track of album A leaves A
// orphaned, but the artist still has album B (so the artist cascade
// must NOT fire).
artist, err := q.UpsertArtist(context.Background(), dbq.UpsertArtistParams{
Name: "Cascade Artist", SortName: "Cascade Artist",
})
if err != nil {
t.Fatalf("artist: %v", err)
}
albumA, err := q.UpsertAlbum(context.Background(), dbq.UpsertAlbumParams{
Title: "Album A", SortTitle: "Album A", ArtistID: artist.ID,
})
if err != nil {
t.Fatalf("albumA: %v", err)
}
albumB, err := q.UpsertAlbum(context.Background(), dbq.UpsertAlbumParams{
Title: "Album B", SortTitle: "Album B", ArtistID: artist.ID,
})
if err != nil {
t.Fatalf("albumB: %v", err)
}
dir := t.TempDir()
pathA := filepath.Join(dir, "a.mp3")
pathB := filepath.Join(dir, "b.mp3")
_ = os.WriteFile(pathA, []byte("x"), 0o644)
_ = os.WriteFile(pathB, []byte("x"), 0o644)
trackA, err := q.UpsertTrack(context.Background(), dbq.UpsertTrackParams{
Title: "TA", AlbumID: albumA.ID, ArtistID: artist.ID,
DurationMs: 1000, FilePath: pathA, FileSize: 1, FileFormat: "mp3",
})
if err != nil {
t.Fatalf("trackA: %v", err)
}
if _, err := q.UpsertTrack(context.Background(), dbq.UpsertTrackParams{
Title: "TB", AlbumID: albumB.ID, ArtistID: artist.ID,
DurationMs: 1000, FilePath: pathB, FileSize: 1, FileFormat: "mp3",
}); err != nil {
t.Fatalf("trackB: %v", err)
}
svc := NewService(pool, nil, nil)
delAlbum, delArtist, err := svc.RemoveTrack(context.Background(), trackA.ID, admin.ID)
if err != nil {
t.Fatalf("RemoveTrack: %v", err)
}
if delAlbum == nil || *delAlbum != albumA.ID {
t.Errorf("deletedAlbumID = %v, want %v", delAlbum, albumA.ID)
}
if delArtist != nil {
t.Errorf("deletedArtistID = %v, want nil (artist still has album B)", delArtist)
}
// Album A should be gone; album B should still exist.
if _, err := q.GetAlbumByID(context.Background(), albumA.ID); err == nil {
t.Errorf("album A still exists after cascade")
}
if _, err := q.GetAlbumByID(context.Background(), albumB.ID); err != nil {
t.Errorf("album B unexpectedly removed: %v", err)
}
if _, err := q.GetArtistByID(context.Background(), artist.ID); err != nil {
t.Errorf("artist unexpectedly removed: %v", err)
}
}
func TestRemoveTrack_Cascade_ArtistEmpty(t *testing.T) {
pool := newPool(t)
admin := seedAdmin(t, pool, "alice")
dir := t.TempDir()
path := filepath.Join(dir, "lone.mp3")
if err := os.WriteFile(path, []byte("x"), 0o644); err != nil {
t.Fatal(err)
}
artist, album, track := seedArtistAlbumTrack(t, pool, "Lone", "", path)
svc := NewService(pool, nil, nil)
delAlbum, delArtist, err := svc.RemoveTrack(context.Background(), track.ID, admin.ID)
if err != nil {
t.Fatalf("RemoveTrack: %v", err)
}
if delAlbum == nil || *delAlbum != album.ID {
t.Errorf("deletedAlbumID = %v, want %v", delAlbum, album.ID)
}
if delArtist == nil || *delArtist != artist.ID {
t.Errorf("deletedArtistID = %v, want %v", delArtist, artist.ID)
}
q := dbq.New(pool)
if _, err := q.GetAlbumByID(context.Background(), album.ID); err == nil {
t.Errorf("album still exists after cascade")
}
if _, err := q.GetArtistByID(context.Background(), artist.ID); err == nil {
t.Errorf("artist still exists after cascade")
}
}