feat(server/m7-352): BuildSystemPlaylists with atomic replace + concurrency guard

Implements T5 of #352 slice 2: adds CreateSystemPlaylist sqlc query, BuildSystemPlaylists function (For-You + Songs-like mixes, tx atomic replace, in_flight guard, tieBreakHash determinism), and 7 integration tests covering activity, quarantine exclusion, atomic replace, concurrency, daily nonce stability, ListActiveUsers, and stale-in-flight recovery. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-04 08:35:02 -04:00
parent ef478778ef
commit 46a9de8e9a
4 changed files with 668 additions and 0 deletions
@@ -0,0 +1,341 @@
+package playlists_test
+
+import (
+	"context"
+	"io"
+	"log/slog"
+	"sync"
+	"testing"
+	"time"
+
+	"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/playlists"
+)
+
+func discardLogger() *slog.Logger {
+	return slog.New(slog.NewTextHandler(io.Discard, nil))
+}
+
+// seedPlayEvent inserts a play_events row with the given was_skipped flag,
+// at startedAt, for the given user/track. Uses raw SQL because the sqlc
+// `InsertPlayEvent` doesn't accept was_skipped (that's set by an UPDATE).
+func seedPlayEvent(t *testing.T, pool *pgxpool.Pool, userID, trackID pgtype.UUID, startedAt time.Time, wasSkipped bool) {
+	t.Helper()
+	_, err := pool.Exec(context.Background(), `
+		INSERT INTO play_events (user_id, track_id, session_id, started_at, was_skipped)
+		VALUES ($1, $2, gen_random_uuid(), $3, $4)
+	`, userID, trackID, startedAt, wasSkipped)
+	if err != nil {
+		t.Fatalf("seed play_event: %v", err)
+	}
+}
+
+// seedQuarantine inserts a lidarr_quarantine row for (user, track).
+func seedQuarantine(t *testing.T, pool *pgxpool.Pool, userID, trackID pgtype.UUID) {
+	t.Helper()
+	_, err := pool.Exec(context.Background(), `
+		INSERT INTO lidarr_quarantine (user_id, track_id, reason)
+		VALUES ($1, $2, 'test-hide')
+	`, userID, trackID)
+	if err != nil {
+		t.Fatalf("seed quarantine: %v", err)
+	}
+}
+
+// seedActiveLibrary creates a user with N played tracks across M artists,
+// all within the last 7 days. Returns the user and the slice of tracks.
+// Each track gets ≥3 unskipped plays so PickSeedArtists has a real signal.
+func seedActiveLibrary(t *testing.T, pool *pgxpool.Pool, name string, numArtists, tracksPerArtist int) (dbq.User, []dbq.Track) {
+	t.Helper()
+	u := seedUser(t, pool, name)
+	now := time.Now().UTC()
+	var allTracks []dbq.Track
+	for a := 0; a < numArtists; a++ {
+		artistName := name + "-artist-" + string(rune('A'+a))
+		for k := 0; k < tracksPerArtist; k++ {
+			tk := seedTrack(t, pool, name+"-track-"+string(rune('A'+a))+string(rune('0'+k)), artistName)
+			allTracks = append(allTracks, tk)
+			// 3 unskipped plays per track to populate engagement stats.
+			for p := 0; p < 3; p++ {
+				seedPlayEvent(t, pool, u.ID, tk.ID, now.Add(-time.Duration(a*10+k+p)*time.Hour), false)
+			}
+		}
+	}
+	return u, allTracks
+}
+
+func TestBuildSystemPlaylists_SufficientActivity(t *testing.T) {
+	pool := newPool(t)
+	logger := discardLogger()
+	u, _ := seedActiveLibrary(t, pool, "act1", 4, 3)
+	if err := playlists.BuildSystemPlaylists(context.Background(), pool, logger, u.ID, time.Now().UTC()); err != nil {
+		t.Fatalf("build: %v", err)
+	}
+
+	rows, err := dbq.New(pool).ListPlaylistsByUserAndKind(context.Background(), dbq.ListPlaylistsByUserAndKindParams{
+		UserID: u.ID, Column2: "system",
+	})
+	if err != nil {
+		t.Fatalf("list: %v", err)
+	}
+	if len(rows) == 0 {
+		t.Fatalf("expected at least one system playlist; got 0")
+	}
+
+	var hasForYou, hasSongsLike bool
+	for _, r := range rows {
+		if r.Kind != "system" {
+			t.Errorf("row %s: kind=%q want 'system'", uuidString(r.ID), r.Kind)
+		}
+		if r.SystemVariant == nil {
+			t.Errorf("row %s: nil system_variant", uuidString(r.ID))
+			continue
+		}
+		switch *r.SystemVariant {
+		case "for_you":
+			hasForYou = true
+			if r.SeedArtistID.Valid {
+				t.Errorf("for_you row should have NULL seed_artist_id; got valid")
+			}
+		case "songs_like_artist":
+			hasSongsLike = true
+			if !r.SeedArtistID.Valid {
+				t.Errorf("songs_like_artist row should have non-NULL seed_artist_id")
+			}
+		default:
+			t.Errorf("unknown system_variant=%q", *r.SystemVariant)
+		}
+		if r.TrackCount == 0 {
+			t.Errorf("row %s: empty track_count", uuidString(r.ID))
+		}
+		if r.TrackCount > 25 {
+			t.Errorf("row %s: track_count=%d exceeds 25", uuidString(r.ID), r.TrackCount)
+		}
+	}
+	if !hasForYou {
+		t.Error("expected a for_you playlist")
+	}
+	if !hasSongsLike {
+		t.Error("expected at least one songs_like_artist playlist")
+	}
+}
+
+func TestBuildSystemPlaylists_QuarantineExcluded(t *testing.T) {
+	pool := newPool(t)
+	logger := discardLogger()
+	u, tracks := seedActiveLibrary(t, pool, "qact", 3, 3)
+
+	quarantined := tracks[0]
+	seedQuarantine(t, pool, u.ID, quarantined.ID)
+
+	if err := playlists.BuildSystemPlaylists(context.Background(), pool, logger, u.ID, time.Now().UTC()); err != nil {
+		t.Fatalf("build: %v", err)
+	}
+
+	// Inspect playlist_tracks for the user's system playlists; quarantined
+	// track must not appear.
+	rows, err := pool.Query(context.Background(), `
+		SELECT pt.track_id
+		  FROM playlist_tracks pt
+		  JOIN playlists p ON p.id = pt.playlist_id
+		 WHERE p.user_id = $1 AND p.kind = 'system'
+	`, u.ID)
+	if err != nil {
+		t.Fatalf("query playlist_tracks: %v", err)
+	}
+	defer rows.Close()
+	for rows.Next() {
+		var tid pgtype.UUID
+		if err := rows.Scan(&tid); err != nil {
+			t.Fatalf("scan: %v", err)
+		}
+		if tid.Bytes == quarantined.ID.Bytes {
+			t.Fatalf("quarantined track %s appeared in a system playlist", uuidString(quarantined.ID))
+		}
+	}
+}
+
+func TestBuildSystemPlaylists_AtomicReplace(t *testing.T) {
+	pool := newPool(t)
+	logger := discardLogger()
+	u, _ := seedActiveLibrary(t, pool, "atomic", 3, 3)
+	ctx := context.Background()
+
+	// First build.
+	if err := playlists.BuildSystemPlaylists(ctx, pool, logger, u.ID, time.Now().UTC()); err != nil {
+		t.Fatalf("first build: %v", err)
+	}
+	var ids1 []pgtype.UUID
+	rows1, _ := dbq.New(pool).ListPlaylistsByUserAndKind(ctx, dbq.ListPlaylistsByUserAndKindParams{
+		UserID: u.ID, Column2: "system",
+	})
+	for _, r := range rows1 {
+		ids1 = append(ids1, r.ID)
+	}
+	if len(ids1) == 0 {
+		t.Skip("no system playlists from first build; can't verify replace")
+	}
+
+	// Second build — old rows must be gone, replaced with fresh ones.
+	if err := playlists.BuildSystemPlaylists(ctx, pool, logger, u.ID, time.Now().UTC()); err != nil {
+		t.Fatalf("second build: %v", err)
+	}
+	rows2, _ := dbq.New(pool).ListPlaylistsByUserAndKind(ctx, dbq.ListPlaylistsByUserAndKindParams{
+		UserID: u.ID, Column2: "system",
+	})
+	for _, r := range rows2 {
+		for _, oldID := range ids1 {
+			if r.ID.Bytes == oldID.Bytes {
+				t.Errorf("playlist id %s persisted across rebuilds — atomic replace failed", uuidString(oldID))
+			}
+		}
+	}
+}
+
+func TestBuildSystemPlaylists_Concurrency(t *testing.T) {
+	pool := newPool(t)
+	logger := discardLogger()
+	u, _ := seedActiveLibrary(t, pool, "conc", 3, 3)
+	ctx := context.Background()
+	now := time.Now().UTC()
+
+	var wg sync.WaitGroup
+	wg.Add(2)
+	go func() { defer wg.Done(); _ = playlists.BuildSystemPlaylists(ctx, pool, logger, u.ID, now) }()
+	go func() { defer wg.Done(); _ = playlists.BuildSystemPlaylists(ctx, pool, logger, u.ID, now) }()
+	wg.Wait()
+
+	// Exactly one run must have completed; in_flight must be false either way.
+	run, err := dbq.New(pool).GetSystemPlaylistRun(ctx, u.ID)
+	if err != nil {
+		t.Fatalf("get run: %v", err)
+	}
+	if run.InFlight {
+		t.Errorf("in_flight should be false after both calls return")
+	}
+	if !run.LastRunAt.Valid {
+		t.Errorf("last_run_at should be set after at least one successful build")
+	}
+}
+
+func TestBuildSystemPlaylists_DailyNonceDeterminism(t *testing.T) {
+	pool := newPool(t)
+	logger := discardLogger()
+	u, _ := seedActiveLibrary(t, pool, "nonce", 4, 5)
+	ctx := context.Background()
+	day1 := time.Date(2026, 5, 4, 12, 0, 0, 0, time.UTC)
+
+	if err := playlists.BuildSystemPlaylists(ctx, pool, logger, u.ID, day1); err != nil {
+		t.Fatalf("build day1 first: %v", err)
+	}
+	snap1 := snapshotSystemTracks(t, pool, u.ID)
+
+	if err := playlists.BuildSystemPlaylists(ctx, pool, logger, u.ID, day1); err != nil {
+		t.Fatalf("build day1 second: %v", err)
+	}
+	snap2 := snapshotSystemTracks(t, pool, u.ID)
+
+	if !equalSnapshots(snap1, snap2) {
+		t.Errorf("same-day rebuild produced different track lists")
+	}
+}
+
+// snapshotSystemTracks returns a per-playlist-name → []track_id mapping
+// for the user's system playlists, in playlist_tracks position order.
+func snapshotSystemTracks(t *testing.T, pool *pgxpool.Pool, userID pgtype.UUID) map[string][]string {
+	t.Helper()
+	rows, err := pool.Query(context.Background(), `
+		SELECT p.name, pt.track_id
+		  FROM playlists p
+		  JOIN playlist_tracks pt ON pt.playlist_id = p.id
+		 WHERE p.user_id = $1 AND p.kind = 'system'
+		 ORDER BY p.name, pt.position
+	`, userID)
+	if err != nil {
+		t.Fatalf("snapshot query: %v", err)
+	}
+	defer rows.Close()
+	out := map[string][]string{}
+	for rows.Next() {
+		var name string
+		var tid pgtype.UUID
+		if err := rows.Scan(&name, &tid); err != nil {
+			t.Fatalf("snapshot scan: %v", err)
+		}
+		out[name] = append(out[name], uuidString(tid))
+	}
+	return out
+}
+
+func equalSnapshots(a, b map[string][]string) bool {
+	if len(a) != len(b) {
+		return false
+	}
+	for k, va := range a {
+		vb, ok := b[k]
+		if !ok || len(va) != len(vb) {
+			return false
+		}
+		for i := range va {
+			if va[i] != vb[i] {
+				return false
+			}
+		}
+	}
+	return true
+}
+
+func TestListActiveUsersForSystemPlaylists(t *testing.T) {
+	pool := newPool(t)
+	ctx := context.Background()
+	activeU, tracks := seedActiveLibrary(t, pool, "active", 1, 1)
+	inactiveU := seedUser(t, pool, "inactive")
+	// inactive user has plays older than 7 days.
+	seedPlayEvent(t, pool, inactiveU.ID, tracks[0].ID, time.Now().UTC().AddDate(0, 0, -30), false)
+
+	ids, err := dbq.New(pool).ListActiveUsersForSystemPlaylists(ctx)
+	if err != nil {
+		t.Fatalf("list: %v", err)
+	}
+
+	var sawActive, sawInactive bool
+	for _, id := range ids {
+		if id.Bytes == activeU.ID.Bytes {
+			sawActive = true
+		}
+		if id.Bytes == inactiveU.ID.Bytes {
+			sawInactive = true
+		}
+	}
+	if !sawActive {
+		t.Error("active user not returned")
+	}
+	if sawInactive {
+		t.Error("inactive user (plays > 7d old) was returned")
+	}
+}
+
+func TestStartupRecoveryClearsStaleInFlight(t *testing.T) {
+	pool := newPool(t)
+	ctx := context.Background()
+	u := seedUser(t, pool, "stale")
+	// Seed a stale in_flight=true row.
+	if _, err := pool.Exec(ctx, `INSERT INTO system_playlist_runs (user_id, in_flight) VALUES ($1, true)`, u.ID); err != nil {
+		t.Fatalf("seed runs: %v", err)
+	}
+
+	if err := dbq.New(pool).ClearStaleSystemPlaylistInFlight(ctx); err != nil {
+		t.Fatalf("clear: %v", err)
+	}
+
+	run, err := dbq.New(pool).GetSystemPlaylistRun(ctx, u.ID)
+	if err != nil {
+		t.Fatalf("get: %v", err)
+	}
+	if run.InFlight {
+		t.Errorf("in_flight should be false after ClearStale; got true")
+	}
+}