From 32fb3fec202b49d370fb478fe6e19090ef32afd5 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sun, 26 Apr 2026 16:43:20 -0400 Subject: [PATCH] feat(subsonic): add /rest/star and /rest/unstar handlers Validate-all-first atomicity on star: a missing entity refuses the whole call with Subsonic error 70. Unstar is a best-effort delete (missing entities are no-ops, matching client expectations after library moves). --- internal/subsonic/star.go | 128 ++++++++++++++++++++++ internal/subsonic/star_test.go | 192 +++++++++++++++++++++++++++++++++ internal/subsonic/subsonic.go | 2 + 3 files changed, 322 insertions(+) create mode 100644 internal/subsonic/star.go create mode 100644 internal/subsonic/star_test.go diff --git a/internal/subsonic/star.go b/internal/subsonic/star.go new file mode 100644 index 00000000..4bffb6f5 --- /dev/null +++ b/internal/subsonic/star.go @@ -0,0 +1,128 @@ +package subsonic + +import ( + "context" + "errors" + "net/http" + + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgtype" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +// handleStar implements /rest/star. Accepts any combination of id (track), +// albumId, artistId. All entities are validated before any insert; a single +// missing entity fails the whole call with Subsonic error 70. +func (m *mediaHandlers) handleStar(w http.ResponseWriter, r *http.Request) { + user, ok := UserFromContext(r.Context()) + if !ok { + WriteFail(w, r, ErrGeneric, "Unauthenticated") + return + } + params := r.URL.Query() + q := dbq.New(m.pool) + trackID, err := resolveStarID(r.Context(), q, params.Get("id"), entityKindTrack) + if err != nil { + WriteFail(w, r, ErrDataNotFound, err.Error()) + return + } + albumID, err := resolveStarID(r.Context(), q, params.Get("albumId"), entityKindAlbum) + if err != nil { + WriteFail(w, r, ErrDataNotFound, err.Error()) + return + } + artistID, err := resolveStarID(r.Context(), q, params.Get("artistId"), entityKindArtist) + if err != nil { + WriteFail(w, r, ErrDataNotFound, err.Error()) + return + } + if trackID.Valid { + if err := q.LikeTrack(r.Context(), dbq.LikeTrackParams{UserID: user.ID, TrackID: trackID}); err != nil { + WriteFail(w, r, ErrGeneric, "Could not star track") + return + } + } + if albumID.Valid { + if err := q.LikeAlbum(r.Context(), dbq.LikeAlbumParams{UserID: user.ID, AlbumID: albumID}); err != nil { + WriteFail(w, r, ErrGeneric, "Could not star album") + return + } + } + if artistID.Valid { + if err := q.LikeArtist(r.Context(), dbq.LikeArtistParams{UserID: user.ID, ArtistID: artistID}); err != nil { + WriteFail(w, r, ErrGeneric, "Could not star artist") + return + } + } + Write(w, r, struct{ Envelope }{Envelope: NewEnvelope("ok")}) +} + +// handleUnstar mirrors handleStar but DELETEs. Missing entities are treated +// as a no-op rather than an error — Subsonic clients commonly call unstar +// on entities the server has never seen (cross-server library moves). +func (m *mediaHandlers) handleUnstar(w http.ResponseWriter, r *http.Request) { + user, ok := UserFromContext(r.Context()) + if !ok { + WriteFail(w, r, ErrGeneric, "Unauthenticated") + return + } + params := r.URL.Query() + q := dbq.New(m.pool) + if t, ok := parseUUID(params.Get("id")); ok { + _ = q.UnlikeTrack(r.Context(), dbq.UnlikeTrackParams{UserID: user.ID, TrackID: t}) + } + if a, ok := parseUUID(params.Get("albumId")); ok { + _ = q.UnlikeAlbum(r.Context(), dbq.UnlikeAlbumParams{UserID: user.ID, AlbumID: a}) + } + if a, ok := parseUUID(params.Get("artistId")); ok { + _ = q.UnlikeArtist(r.Context(), dbq.UnlikeArtistParams{UserID: user.ID, ArtistID: a}) + } + Write(w, r, struct{ Envelope }{Envelope: NewEnvelope("ok")}) +} + +type entityKind int + +const ( + entityKindTrack entityKind = iota + entityKindAlbum + entityKindArtist +) + +// resolveStarID returns (uuid, nil) if the param is empty (no-op), or +// (uuid, nil) if the entity exists, or (zero, error) if the entity is +// missing/malformed. Non-empty malformed UUID is treated as a 'not found' +// per Subsonic semantics. +func resolveStarID(ctx context.Context, q *dbq.Queries, raw string, kind entityKind) (pgtype.UUID, error) { + if raw == "" { + return pgtype.UUID{}, nil + } + id, ok := parseUUID(raw) + if !ok { + return pgtype.UUID{}, errors.New("not found") + } + switch kind { + case entityKindTrack: + if _, err := q.GetTrackByID(ctx, id); err != nil { + if errors.Is(err, pgx.ErrNoRows) { + return pgtype.UUID{}, errors.New("track not found") + } + return pgtype.UUID{}, err + } + case entityKindAlbum: + if _, err := q.GetAlbumByID(ctx, id); err != nil { + if errors.Is(err, pgx.ErrNoRows) { + return pgtype.UUID{}, errors.New("album not found") + } + return pgtype.UUID{}, err + } + case entityKindArtist: + if _, err := q.GetArtistByID(ctx, id); err != nil { + if errors.Is(err, pgx.ErrNoRows) { + return pgtype.UUID{}, errors.New("artist not found") + } + return pgtype.UUID{}, err + } + } + return id, nil +} diff --git a/internal/subsonic/star_test.go b/internal/subsonic/star_test.go new file mode 100644 index 00000000..d6dd006a --- /dev/null +++ b/internal/subsonic/star_test.go @@ -0,0 +1,192 @@ +package subsonic + +import ( + "context" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "net/url" + "os" + "testing" + "time" + + "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/playevents" +) + +func testStarPool(t *testing.T) (*pgxpool.Pool, dbq.User, dbq.Track, dbq.Album, dbq.Artist) { + t.Helper() + if testing.Short() { + t.Skip("skipping 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) + if _, err := pool.Exec(context.Background(), + "TRUNCATE general_likes, general_likes_albums, general_likes_artists, play_events, skip_events, play_sessions, sessions, users, tracks, albums, artists RESTART IDENTITY CASCADE"); err != nil { + t.Fatalf("truncate: %v", err) + } + q := dbq.New(pool) + u, _ := q.CreateUser(context.Background(), dbq.CreateUserParams{ + Username: "tester", PasswordHash: "x", ApiToken: "x", IsAdmin: false, + }) + a, _ := q.UpsertArtist(context.Background(), dbq.UpsertArtistParams{Name: "X", SortName: "X"}) + al, _ := q.UpsertAlbum(context.Background(), dbq.UpsertAlbumParams{Title: "X", SortTitle: "X", ArtistID: a.ID}) + tr, _ := q.UpsertTrack(context.Background(), dbq.UpsertTrackParams{ + Title: "Y", AlbumID: al.ID, ArtistID: a.ID, FilePath: "/tmp/star.flac", DurationMs: 100_000, + }) + return pool, u, tr, al, a +} + +func newStarHandlers(pool *pgxpool.Pool) *mediaHandlers { + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + w := playevents.NewWriter(pool, logger, 30*time.Minute, 0.5, 30000) + return newMediaHandlers(pool, w) +} + +func TestHandleStar_TrackIDInsertsRow(t *testing.T) { + pool, user, track, _, _ := testStarPool(t) + m := newStarHandlers(pool) + + q := url.Values{} + q.Set("id", uuidToID(track.ID)) + req := httptest.NewRequest(http.MethodGet, "/rest/star?"+q.Encode(), nil) + ctx := context.WithValue(req.Context(), userCtxKey, user) + w := httptest.NewRecorder() + m.handleStar(w, req.WithContext(ctx)) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d", w.Code) + } + var count int + _ = pool.QueryRow(context.Background(), + "SELECT count(*) FROM general_likes WHERE user_id=$1", user.ID).Scan(&count) + if count != 1 { + t.Errorf("count = %d", count) + } +} + +func TestHandleStar_AllThreeIDsAtOnce(t *testing.T) { + pool, user, track, album, artist := testStarPool(t) + m := newStarHandlers(pool) + + q := url.Values{} + q.Set("id", uuidToID(track.ID)) + q.Set("albumId", uuidToID(album.ID)) + q.Set("artistId", uuidToID(artist.ID)) + req := httptest.NewRequest(http.MethodGet, "/rest/star?"+q.Encode(), nil) + ctx := context.WithValue(req.Context(), userCtxKey, user) + w := httptest.NewRecorder() + m.handleStar(w, req.WithContext(ctx)) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d", w.Code) + } + var tCount, alCount, arCount int + _ = pool.QueryRow(context.Background(), "SELECT count(*) FROM general_likes WHERE user_id=$1", user.ID).Scan(&tCount) + _ = pool.QueryRow(context.Background(), "SELECT count(*) FROM general_likes_albums WHERE user_id=$1", user.ID).Scan(&alCount) + _ = pool.QueryRow(context.Background(), "SELECT count(*) FROM general_likes_artists WHERE user_id=$1", user.ID).Scan(&arCount) + if tCount != 1 || alCount != 1 || arCount != 1 { + t.Errorf("counts: tracks=%d albums=%d artists=%d", tCount, alCount, arCount) + } +} + +func TestHandleStar_UnknownTrackReturnsError70(t *testing.T) { + pool, user, _, _, _ := testStarPool(t) + m := newStarHandlers(pool) + + q := url.Values{} + q.Set("id", "00000000-0000-0000-0000-000000000000") + req := httptest.NewRequest(http.MethodGet, "/rest/star?"+q.Encode(), nil) + ctx := context.WithValue(req.Context(), userCtxKey, user) + w := httptest.NewRecorder() + m.handleStar(w, req.WithContext(ctx)) + + body := w.Body.String() + // Subsonic error envelope returns 200 OK with status="failed" inside the body. + // The exact code is 70 (data not found). + if w.Code != http.StatusOK || !contains(body, `"code":70`) && !contains(body, `code="70"`) { + t.Errorf("expected error 70 envelope, got status=%d body=%s", w.Code, body) + } +} + +func TestHandleStar_PartialMissingAtomic(t *testing.T) { + pool, user, track, _, _ := testStarPool(t) + m := newStarHandlers(pool) + + q := url.Values{} + q.Set("id", uuidToID(track.ID)) + q.Set("albumId", "00000000-0000-0000-0000-000000000000") + req := httptest.NewRequest(http.MethodGet, "/rest/star?"+q.Encode(), nil) + ctx := context.WithValue(req.Context(), userCtxKey, user) + w := httptest.NewRecorder() + m.handleStar(w, req.WithContext(ctx)) + + // Album missing → atomic refusal: track NOT starred. + var count int + _ = pool.QueryRow(context.Background(), + "SELECT count(*) FROM general_likes WHERE user_id=$1", user.ID).Scan(&count) + if count != 0 { + t.Errorf("track was starred despite album miss; count = %d, want 0", count) + } +} + +func TestHandleUnstar_RemovesAndIsIdempotent(t *testing.T) { + pool, user, track, _, _ := testStarPool(t) + m := newStarHandlers(pool) + + // First star. + q := url.Values{} + q.Set("id", uuidToID(track.ID)) + star := httptest.NewRequest(http.MethodGet, "/rest/star?"+q.Encode(), nil) + starCtx := context.WithValue(star.Context(), userCtxKey, user) + starResp := httptest.NewRecorder() + m.handleStar(starResp, star.WithContext(starCtx)) + + // Unstar. + un := httptest.NewRequest(http.MethodGet, "/rest/unstar?"+q.Encode(), nil) + unCtx := context.WithValue(un.Context(), userCtxKey, user) + unResp := httptest.NewRecorder() + m.handleUnstar(unResp, un.WithContext(unCtx)) + + if unResp.Code != http.StatusOK { + t.Fatalf("unstar status = %d", unResp.Code) + } + var count int + _ = pool.QueryRow(context.Background(), + "SELECT count(*) FROM general_likes WHERE user_id=$1", user.ID).Scan(&count) + if count != 0 { + t.Errorf("count after unstar = %d, want 0", count) + } + + // Idempotent — unstar again. + un2 := httptest.NewRequest(http.MethodGet, "/rest/unstar?"+q.Encode(), nil) + un2 = un2.WithContext(context.WithValue(un2.Context(), userCtxKey, user)) + un2Resp := httptest.NewRecorder() + m.handleUnstar(un2Resp, un2) + if un2Resp.Code != http.StatusOK { + t.Errorf("second unstar status = %d", un2Resp.Code) + } +} + +func contains(s, substr string) bool { + for i := 0; i+len(substr) <= len(s); i++ { + if s[i:i+len(substr)] == substr { + return true + } + } + return false +} diff --git a/internal/subsonic/subsonic.go b/internal/subsonic/subsonic.go index ed3f0069..c5af689b 100644 --- a/internal/subsonic/subsonic.go +++ b/internal/subsonic/subsonic.go @@ -38,6 +38,8 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, cfg Config, ev register(sub, "/download", m.handleDownload) register(sub, "/getCoverArt", m.handleGetCoverArt) register(sub, "/scrobble", m.handleScrobble) + register(sub, "/star", m.handleStar) + register(sub, "/unstar", m.handleUnstar) register(sub, "/getUser", handleGetUser) // Subsonic clients expect every /rest/* miss to come back as a