feat(server/m7-352): kind filter on GET /api/playlists + lazy fallback
Adds ?kind= filter (user|system|all, default user) to GET /api/playlists so system mixes don't leak into slice-1 SPA views. Lazy background build fires when kind=system|all and the caller's run row is stale (>24h). Propagates Kind/SystemVariant/SeedArtistID through PlaylistRow and playlistRowView wire types. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
+88
-11
@@ -1,17 +1,22 @@
|
||||
package api
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"path/filepath"
|
||||
"strconv"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/go-chi/chi/v5"
|
||||
"github.com/jackc/pgx/v5"
|
||||
"github.com/jackc/pgx/v5/pgtype"
|
||||
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/auth"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/playlists"
|
||||
)
|
||||
|
||||
@@ -20,17 +25,20 @@ import (
|
||||
// derives the cover_url from cover_path so clients don't have to know
|
||||
// the on-disk layout.
|
||||
type playlistRowView struct {
|
||||
ID string `json:"id"`
|
||||
UserID string `json:"user_id"`
|
||||
OwnerUsername string `json:"owner_username"`
|
||||
Name string `json:"name"`
|
||||
Description string `json:"description"`
|
||||
IsPublic bool `json:"is_public"`
|
||||
CoverURL string `json:"cover_url,omitempty"`
|
||||
TrackCount int32 `json:"track_count"`
|
||||
DurationSec int32 `json:"duration_sec"`
|
||||
CreatedAt string `json:"created_at"`
|
||||
UpdatedAt string `json:"updated_at"`
|
||||
ID string `json:"id"`
|
||||
UserID string `json:"user_id"`
|
||||
OwnerUsername string `json:"owner_username"`
|
||||
Name string `json:"name"`
|
||||
Description string `json:"description"`
|
||||
IsPublic bool `json:"is_public"`
|
||||
Kind string `json:"kind"`
|
||||
SystemVariant *string `json:"system_variant,omitempty"`
|
||||
SeedArtistID *string `json:"seed_artist_id,omitempty"`
|
||||
CoverURL string `json:"cover_url,omitempty"`
|
||||
TrackCount int32 `json:"track_count"`
|
||||
DurationSec int32 `json:"duration_sec"`
|
||||
CreatedAt string `json:"created_at"`
|
||||
UpdatedAt string `json:"updated_at"`
|
||||
}
|
||||
|
||||
// playlistTrackView is one row of a playlist's track list on the wire.
|
||||
@@ -70,12 +78,39 @@ type listPlaylistsResponse struct {
|
||||
// every playlist visible to the caller (own + others' public); we split
|
||||
// the result here so the wire shape carries the distinction the UI cares
|
||||
// about without a second DB query.
|
||||
//
|
||||
// An optional ?kind= query parameter filters the Owned bucket:
|
||||
// - "user" (default): only user-created playlists
|
||||
// - "system": only system-generated mixes
|
||||
// - "all": no filter
|
||||
//
|
||||
// The Public bucket (other users' public playlists) is always unfiltered
|
||||
// since system mixes are always private.
|
||||
func (h *handlers) handleListPlaylists(w http.ResponseWriter, r *http.Request) {
|
||||
caller, ok := auth.UserFromContext(r.Context())
|
||||
if !ok {
|
||||
writeErr(w, http.StatusUnauthorized, "unauthenticated", "no session")
|
||||
return
|
||||
}
|
||||
|
||||
kind := strings.TrimSpace(r.URL.Query().Get("kind"))
|
||||
if kind == "" {
|
||||
kind = "user"
|
||||
}
|
||||
switch kind {
|
||||
case "user", "system", "all":
|
||||
// valid
|
||||
default:
|
||||
writeErr(w, http.StatusBadRequest, "bad_kind", "kind must be one of: user, system, all")
|
||||
return
|
||||
}
|
||||
|
||||
// Lazy fallback: when the caller wants system mixes and they're stale,
|
||||
// kick a background build but serve current state.
|
||||
if kind == "system" || kind == "all" {
|
||||
h.maybeKickBackgroundBuild(r.Context(), caller.ID)
|
||||
}
|
||||
|
||||
rows, err := h.playlists.List(r.Context(), caller.ID)
|
||||
if err != nil {
|
||||
h.logger.Error("api: list playlists failed", "err", err)
|
||||
@@ -86,14 +121,50 @@ func (h *handlers) handleListPlaylists(w http.ResponseWriter, r *http.Request) {
|
||||
for i := range rows {
|
||||
v := playlistRowToView(&rows[i])
|
||||
if uuidEqual(rows[i].UserID, caller.ID) {
|
||||
// Filter own playlists by the kind selector.
|
||||
if kind != "all" && rows[i].Kind != kind {
|
||||
continue
|
||||
}
|
||||
resp.Owned = append(resp.Owned, v)
|
||||
} else {
|
||||
// Public-from-others: kind filter doesn't apply (system mixes
|
||||
// are always private; this slot is for public-share feature).
|
||||
resp.Public = append(resp.Public, v)
|
||||
}
|
||||
}
|
||||
writeJSON(w, http.StatusOK, resp)
|
||||
}
|
||||
|
||||
// maybeKickBackgroundBuild fires a background BuildSystemPlaylists when the
|
||||
// run is stale (>24h) AND not currently in_flight. Best-effort: any error
|
||||
// fetching the run row is silently swallowed. A real cron tick will catch
|
||||
// up the user within 24h regardless.
|
||||
func (h *handlers) maybeKickBackgroundBuild(ctx context.Context, userID pgtype.UUID) {
|
||||
run, err := dbq.New(h.pool).GetSystemPlaylistRun(ctx, userID)
|
||||
var stale bool
|
||||
switch {
|
||||
case errors.Is(err, pgx.ErrNoRows):
|
||||
stale = true // no run yet — first request — definitely stale
|
||||
case err != nil:
|
||||
h.logger.Warn("lazy system build: get run row failed", "err", err)
|
||||
return
|
||||
case !run.LastRunAt.Valid:
|
||||
stale = true
|
||||
case run.LastRunAt.Time.Before(time.Now().Add(-24 * time.Hour)):
|
||||
stale = true
|
||||
}
|
||||
if !stale || run.InFlight {
|
||||
return
|
||||
}
|
||||
go func() {
|
||||
bgCtx := context.Background()
|
||||
if err := playlists.BuildSystemPlaylists(bgCtx, h.pool, h.logger, userID, time.Now()); err != nil {
|
||||
h.logger.Warn("lazy system playlist build failed",
|
||||
"user_id", "redacted", "err", err)
|
||||
}
|
||||
}()
|
||||
}
|
||||
|
||||
type createPlaylistBody struct {
|
||||
Name string `json:"name"`
|
||||
Description string `json:"description"`
|
||||
@@ -370,11 +441,17 @@ func playlistRowToView(r *playlists.PlaylistRow) playlistRowView {
|
||||
Name: r.Name,
|
||||
Description: r.Description,
|
||||
IsPublic: r.IsPublic,
|
||||
Kind: r.Kind,
|
||||
SystemVariant: r.SystemVariant,
|
||||
TrackCount: r.TrackCount,
|
||||
DurationSec: r.DurationSec,
|
||||
CreatedAt: formatTimestamp(r.CreatedAt),
|
||||
UpdatedAt: formatTimestamp(r.UpdatedAt),
|
||||
}
|
||||
if r.SeedArtistID.Valid {
|
||||
s := uuidToString(r.SeedArtistID)
|
||||
v.SeedArtistID = &s
|
||||
}
|
||||
if r.CoverPath != nil && *r.CoverPath != "" {
|
||||
v.CoverURL = "/api/playlists/" + v.ID + "/cover"
|
||||
}
|
||||
|
||||
@@ -454,3 +454,118 @@ func TestPlaylists_CoverNoneReturns404(t *testing.T) {
|
||||
t.Errorf("status = %d, want 404; body = %s", w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// TestListPlaylists_KindFilterDefaultsToUser verifies that an absent ?kind=
|
||||
// parameter defaults to "user" and excludes system-kind playlists.
|
||||
func TestListPlaylists_KindFilterDefaultsToUser(t *testing.T) {
|
||||
h, pool := testHandlers(t)
|
||||
truncateLibrary(t, pool)
|
||||
u := seedUser(t, pool, "klistdefault", "pw", false)
|
||||
|
||||
// Create one user-kind playlist via the service.
|
||||
userPL, err := h.playlists.Create(context.Background(), u.ID, "My PL", "", false)
|
||||
if err != nil {
|
||||
t.Fatalf("create user playlist: %v", err)
|
||||
}
|
||||
// Insert a system-kind playlist directly via SQL.
|
||||
if _, err := pool.Exec(context.Background(), `
|
||||
INSERT INTO playlists (user_id, name, description, is_public, kind, system_variant)
|
||||
VALUES ($1, 'For You', '', false, 'system', 'for_you')
|
||||
`, u.ID); err != nil {
|
||||
t.Fatalf("seed system playlist: %v", err)
|
||||
}
|
||||
|
||||
w := doPlaylistsReq(h, u, "GET", "/api/playlists", nil)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("status: got %d, want 200; body=%s", w.Code, w.Body.String())
|
||||
}
|
||||
var resp listPlaylistsResponse
|
||||
if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
|
||||
t.Fatalf("decode: %v", err)
|
||||
}
|
||||
if len(resp.Owned) != 1 {
|
||||
t.Fatalf("default should return 1 user-kind playlist; got %d", len(resp.Owned))
|
||||
}
|
||||
if resp.Owned[0].Kind != "user" {
|
||||
t.Errorf("expected kind=user; got %q", resp.Owned[0].Kind)
|
||||
}
|
||||
if resp.Owned[0].ID != uuidToString(userPL.ID) {
|
||||
t.Errorf("returned wrong playlist id")
|
||||
}
|
||||
}
|
||||
|
||||
// TestListPlaylists_KindSystem verifies ?kind=system returns only
|
||||
// system-generated playlists and excludes user-created ones.
|
||||
func TestListPlaylists_KindSystem(t *testing.T) {
|
||||
h, pool := testHandlers(t)
|
||||
truncateLibrary(t, pool)
|
||||
u := seedUser(t, pool, "klistsys", "pw", false)
|
||||
|
||||
if _, err := h.playlists.Create(context.Background(), u.ID, "User PL", "", false); err != nil {
|
||||
t.Fatalf("create user playlist: %v", err)
|
||||
}
|
||||
if _, err := pool.Exec(context.Background(), `
|
||||
INSERT INTO playlists (user_id, name, description, is_public, kind, system_variant)
|
||||
VALUES ($1, 'For You', '', false, 'system', 'for_you')
|
||||
`, u.ID); err != nil {
|
||||
t.Fatalf("seed system playlist: %v", err)
|
||||
}
|
||||
|
||||
w := doPlaylistsReq(h, u, "GET", "/api/playlists?kind=system", nil)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("status: got %d, want 200; body=%s", w.Code, w.Body.String())
|
||||
}
|
||||
var resp listPlaylistsResponse
|
||||
if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
|
||||
t.Fatalf("decode: %v", err)
|
||||
}
|
||||
if len(resp.Owned) != 1 {
|
||||
t.Fatalf("kind=system should return 1 system playlist; got %d", len(resp.Owned))
|
||||
}
|
||||
if resp.Owned[0].Kind != "system" {
|
||||
t.Errorf("expected kind=system; got %q", resp.Owned[0].Kind)
|
||||
}
|
||||
}
|
||||
|
||||
// TestListPlaylists_KindAll verifies ?kind=all returns all playlists
|
||||
// regardless of kind.
|
||||
func TestListPlaylists_KindAll(t *testing.T) {
|
||||
h, pool := testHandlers(t)
|
||||
truncateLibrary(t, pool)
|
||||
u := seedUser(t, pool, "klistall", "pw", false)
|
||||
|
||||
if _, err := h.playlists.Create(context.Background(), u.ID, "User PL", "", false); err != nil {
|
||||
t.Fatalf("create user playlist: %v", err)
|
||||
}
|
||||
if _, err := pool.Exec(context.Background(), `
|
||||
INSERT INTO playlists (user_id, name, description, is_public, kind, system_variant)
|
||||
VALUES ($1, 'For You', '', false, 'system', 'for_you')
|
||||
`, u.ID); err != nil {
|
||||
t.Fatalf("seed system playlist: %v", err)
|
||||
}
|
||||
|
||||
w := doPlaylistsReq(h, u, "GET", "/api/playlists?kind=all", nil)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("status: got %d, want 200; body=%s", w.Code, w.Body.String())
|
||||
}
|
||||
var resp listPlaylistsResponse
|
||||
if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
|
||||
t.Fatalf("decode: %v", err)
|
||||
}
|
||||
if len(resp.Owned) != 2 {
|
||||
t.Fatalf("kind=all should return both playlists; got %d", len(resp.Owned))
|
||||
}
|
||||
}
|
||||
|
||||
// TestListPlaylists_BadKindReturns400 verifies an unrecognised kind value
|
||||
// is rejected with 400 bad_kind.
|
||||
func TestListPlaylists_BadKindReturns400(t *testing.T) {
|
||||
h, pool := testHandlers(t)
|
||||
u := seedUser(t, pool, "klistbad", "pw", false)
|
||||
_ = pool
|
||||
|
||||
w := doPlaylistsReq(h, u, "GET", "/api/playlists?kind=garbage", nil)
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Fatalf("expected 400; got %d, body=%s", w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -65,6 +65,9 @@ type PlaylistRow struct {
|
||||
Name string
|
||||
Description string
|
||||
IsPublic bool
|
||||
Kind string // 'user' | 'system'
|
||||
SystemVariant *string // 'for_you' | 'songs_like_artist' | nil
|
||||
SeedArtistID pgtype.UUID // Valid=true only for 'songs_like_artist'
|
||||
CoverPath *string
|
||||
TrackCount int32
|
||||
DurationSec int32
|
||||
@@ -498,6 +501,9 @@ func playlistFromGetRow(r dbq.GetPlaylistRow) *PlaylistRow {
|
||||
Name: r.Name,
|
||||
Description: r.Description,
|
||||
IsPublic: r.IsPublic,
|
||||
Kind: r.Kind,
|
||||
SystemVariant: r.SystemVariant,
|
||||
SeedArtistID: r.SeedArtistID,
|
||||
CoverPath: r.CoverPath,
|
||||
TrackCount: r.TrackCount,
|
||||
DurationSec: r.DurationSec,
|
||||
@@ -514,6 +520,9 @@ func playlistFromListRow(r dbq.ListPlaylistsForUserRow) *PlaylistRow {
|
||||
Name: r.Name,
|
||||
Description: r.Description,
|
||||
IsPublic: r.IsPublic,
|
||||
Kind: r.Kind,
|
||||
SystemVariant: r.SystemVariant,
|
||||
SeedArtistID: r.SeedArtistID,
|
||||
CoverPath: r.CoverPath,
|
||||
TrackCount: r.TrackCount,
|
||||
DurationSec: r.DurationSec,
|
||||
|
||||
Reference in New Issue
Block a user