From 637d8b8a2e057d6b32ca925e22fdc56dd26101f3 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Mon, 6 Jul 2026 19:12:34 -0500 Subject: [PATCH 1/4] feat(catalogs): artist_account_id filter + no-cap pagination on GET /api/catalogs/measurements (chat#1850) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes on the same read path (v2 of api#757): - FIX the silent 1,000-row cap: the catalog tracklist, the measurement series, and the artist-link read all paginate to exhaustion with .range() loops (and the measurements IN clause is chunked at 500 ISRCs to stay under URL limits) BEFORE dedupe/derivation, so total_streams and the valuation band cover every measured song. Live repro was a 2,679-song catalog valued 3.4x low off exactly 1,000 rows. - ADD optional artist_account_id (uuid, 400 on malformed): scopes measurements + valuation to the catalog's songs linked to that artist account via song_artists (catalog_songs ∩ song_artists). The response echoes the applied filter as artist_account_id (null when unfiltered) so clients can verify the scope before rendering artist-labeled money. Per the docs contract in recoupable/docs#267. TDD: RED confirmed before GREEN for every unit; full suite 3,945 tests, tsc 0 new errors, lint clean. Co-Authored-By: Claude Fable 5 --- .../getCatalogMeasurementsHandler.test.ts | 90 ++++++++++++---- .../resolveCatalogSongsInScope.test.ts | 54 ++++++++++ ...alidateGetCatalogMeasurementsQuery.test.ts | 20 ++++ lib/catalog/getCatalogMeasurementsHandler.ts | 23 ++-- lib/catalog/resolveCatalogSongsInScope.ts | 29 +++++ .../validateGetCatalogMeasurementsQuery.ts | 1 + .../__tests__/selectCatalogSongTitles.test.ts | 64 ++++++++--- .../catalog_songs/selectCatalogSongTitles.ts | 33 ++++-- .../__tests__/selectSongArtistIsrcs.test.ts | 68 ++++++++++++ .../song_artists/selectSongArtistIsrcs.ts | 34 ++++++ .../selectAllSongMeasurements.test.ts | 102 ++++++++++++++++++ .../selectAllSongMeasurements.ts | 58 ++++++++++ 12 files changed, 522 insertions(+), 54 deletions(-) create mode 100644 lib/catalog/__tests__/resolveCatalogSongsInScope.test.ts create mode 100644 lib/catalog/resolveCatalogSongsInScope.ts create mode 100644 lib/supabase/song_artists/__tests__/selectSongArtistIsrcs.test.ts create mode 100644 lib/supabase/song_artists/selectSongArtistIsrcs.ts create mode 100644 lib/supabase/song_measurements/__tests__/selectAllSongMeasurements.test.ts create mode 100644 lib/supabase/song_measurements/selectAllSongMeasurements.ts diff --git a/lib/catalog/__tests__/getCatalogMeasurementsHandler.test.ts b/lib/catalog/__tests__/getCatalogMeasurementsHandler.test.ts index 17149e89c..fa2955ff3 100644 --- a/lib/catalog/__tests__/getCatalogMeasurementsHandler.test.ts +++ b/lib/catalog/__tests__/getCatalogMeasurementsHandler.test.ts @@ -4,10 +4,10 @@ import { NextRequest, NextResponse } from "next/server"; import { getCatalogMeasurementsHandler } from "../getCatalogMeasurementsHandler"; import { validateGetCatalogMeasurementsQuery } from "../validateGetCatalogMeasurementsQuery"; import { getCatalogEarliestReleaseDate } from "../getCatalogEarliestReleaseDate"; +import { resolveCatalogSongsInScope } from "../resolveCatalogSongsInScope"; import { validateAuthContext } from "@/lib/auth/validateAuthContext"; import { selectAccountCatalog } from "@/lib/supabase/account_catalogs/selectAccountCatalog"; -import { selectCatalogSongTitles } from "@/lib/supabase/catalog_songs/selectCatalogSongTitles"; -import { selectSongMeasurements } from "@/lib/supabase/song_measurements/selectSongMeasurements"; +import { selectAllSongMeasurements } from "@/lib/supabase/song_measurements/selectAllSongMeasurements"; vi.mock("@/lib/networking/getCorsHeaders", () => ({ getCorsHeaders: vi.fn(() => ({ "Access-Control-Allow-Origin": "*" })), @@ -16,19 +16,18 @@ vi.mock("../validateGetCatalogMeasurementsQuery", () => ({ validateGetCatalogMeasurementsQuery: vi.fn(), })); vi.mock("../getCatalogEarliestReleaseDate", () => ({ getCatalogEarliestReleaseDate: vi.fn() })); +vi.mock("../resolveCatalogSongsInScope", () => ({ resolveCatalogSongsInScope: vi.fn() })); vi.mock("@/lib/auth/validateAuthContext", () => ({ validateAuthContext: vi.fn() })); vi.mock("@/lib/supabase/account_catalogs/selectAccountCatalog", () => ({ selectAccountCatalog: vi.fn(), })); -vi.mock("@/lib/supabase/catalog_songs/selectCatalogSongTitles", () => ({ - selectCatalogSongTitles: vi.fn(), -})); -vi.mock("@/lib/supabase/song_measurements/selectSongMeasurements", () => ({ - selectSongMeasurements: vi.fn(), +vi.mock("@/lib/supabase/song_measurements/selectAllSongMeasurements", () => ({ + selectAllSongMeasurements: vi.fn(), })); const accountId = "550e8400-e29b-41d4-a716-446655440000"; const catalogId = "740d5050-40ec-4892-a040-b78bb50fef2f"; +const artistAccountId = "b1814076-8e19-4a77-9dea-2ec150e26aaa"; const makeRequest = () => new NextRequest(`http://localhost/api/catalogs/measurements?catalogId=${catalogId}`); @@ -40,7 +39,14 @@ const okAuth = () => authToken: "t", } as never); -const okQuery = () => vi.mocked(validateGetCatalogMeasurementsQuery).mockReturnValue({ catalogId }); +const okQuery = (query: Record = { catalogId }) => + vi.mocked(validateGetCatalogMeasurementsQuery).mockReturnValue(query as never); + +const okCatalog = () => + vi.mocked(selectAccountCatalog).mockResolvedValue({ + account: accountId, + catalog: catalogId, + } as never); describe("getCatalogMeasurementsHandler", () => { beforeEach(() => vi.clearAllMocks()); @@ -75,21 +81,18 @@ describe("getCatalogMeasurementsHandler", () => { expect(res.status).toBe(404); expect(selectAccountCatalog).toHaveBeenCalledWith({ accountId, catalogId }); - expect(selectCatalogSongTitles).not.toHaveBeenCalled(); + expect(resolveCatalogSongsInScope).not.toHaveBeenCalled(); }); it("returns latest per-ISRC measurements + the derived valuation band", async () => { okQuery(); okAuth(); - vi.mocked(selectAccountCatalog).mockResolvedValue({ - account: accountId, - catalog: catalogId, - } as never); - vi.mocked(selectCatalogSongTitles).mockResolvedValue([ + okCatalog(); + vi.mocked(resolveCatalogSongsInScope).mockResolvedValue([ { isrc: "ISRC1", title: "Song One" }, { isrc: "ISRC2", title: "Song Two" }, ]); - vi.mocked(selectSongMeasurements).mockResolvedValue([ + vi.mocked(selectAllSongMeasurements).mockResolvedValue([ // newest-first series; ISRC1 has an older superseded capture { song: "ISRC1", value: 200, captured_at: "2026-07-01T00:00:00Z" }, { song: "ISRC2", value: 50, captured_at: "2026-07-01T00:00:00Z" }, @@ -102,7 +105,11 @@ describe("getCatalogMeasurementsHandler", () => { const body = await res.json(); expect(res.status).toBe(200); - expect(selectSongMeasurements).toHaveBeenCalledWith({ + expect(resolveCatalogSongsInScope).toHaveBeenCalledWith({ + catalogId, + artistAccountId: undefined, + }); + expect(selectAllSongMeasurements).toHaveBeenCalledWith({ songs: ["ISRC1", "ISRC2"], platform: "spotify", metric: "platform_displayed_play_count", @@ -114,21 +121,59 @@ describe("getCatalogMeasurementsHandler", () => { ]); expect(body.total_streams).toBe(250); expect(body.catalog_age_years).toBe(10); + // whole-catalog read: the echo field states no artist filter was applied + expect(body.artist_account_id).toBeNull(); // 250 streams / 10y * $0.0035 * gross-up * 0.6375 net * multiple expect(body.valuation.low).toBeCloseTo(25 * 0.0035 * 1.25 * 0.6375 * 10, 5); expect(body.valuation.mid).toBeCloseTo(25 * 0.0035 * 1.4 * 0.6375 * 13, 5); expect(body.valuation.high).toBeCloseTo(25 * 0.0035 * 1.6 * 0.6375 * 16, 5); }); + it("scopes the read to the artist and echoes the applied filter", async () => { + okQuery({ catalogId, artist_account_id: artistAccountId }); + okAuth(); + okCatalog(); + vi.mocked(resolveCatalogSongsInScope).mockResolvedValue([{ isrc: "ISRC1", title: "Song One" }]); + vi.mocked(selectAllSongMeasurements).mockResolvedValue([ + { song: "ISRC1", value: 200, captured_at: "2026-07-01T00:00:00Z" }, + ] as never); + vi.mocked(getCatalogEarliestReleaseDate).mockResolvedValue("2016-07-01"); + + const res = await getCatalogMeasurementsHandler(makeRequest()); + const body = await res.json(); + + expect(res.status).toBe(200); + expect(resolveCatalogSongsInScope).toHaveBeenCalledWith({ catalogId, artistAccountId }); + expect(body.measurements).toEqual([ + { isrc: "ISRC1", title: "Song One", playcount: 200, measured_at: "2026-07-01T00:00:00Z" }, + ]); + expect(body.total_streams).toBe(200); + expect(body.artist_account_id).toBe(artistAccountId); + }); + + it("returns an empty result with a zero band when the artist has no songs in the catalog", async () => { + okQuery({ catalogId, artist_account_id: artistAccountId }); + okAuth(); + okCatalog(); + vi.mocked(resolveCatalogSongsInScope).mockResolvedValue([]); + vi.mocked(getCatalogEarliestReleaseDate).mockResolvedValue(null); + + const res = await getCatalogMeasurementsHandler(makeRequest()); + const body = await res.json(); + + expect(res.status).toBe(200); + expect(selectAllSongMeasurements).not.toHaveBeenCalled(); + expect(body.measurements).toEqual([]); + expect(body.total_streams).toBe(0); + expect(body.valuation).toEqual({ low: 0, mid: 0, high: 0 }); + expect(body.artist_account_id).toBe(artistAccountId); + }); + it("returns an empty result with a zero band for a catalog with no measurements", async () => { okQuery(); okAuth(); - vi.mocked(selectAccountCatalog).mockResolvedValue({ - account: accountId, - catalog: catalogId, - } as never); - vi.mocked(selectCatalogSongTitles).mockResolvedValue([]); - vi.mocked(selectSongMeasurements).mockResolvedValue([]); + okCatalog(); + vi.mocked(resolveCatalogSongsInScope).mockResolvedValue([]); vi.mocked(getCatalogEarliestReleaseDate).mockResolvedValue(null); const res = await getCatalogMeasurementsHandler(makeRequest()); @@ -138,6 +183,7 @@ describe("getCatalogMeasurementsHandler", () => { expect(body.measurements).toEqual([]); expect(body.total_streams).toBe(0); expect(body.valuation).toEqual({ low: 0, mid: 0, high: 0 }); + expect(body.artist_account_id).toBeNull(); }); it("returns 500 when a dependency throws", async () => { diff --git a/lib/catalog/__tests__/resolveCatalogSongsInScope.test.ts b/lib/catalog/__tests__/resolveCatalogSongsInScope.test.ts new file mode 100644 index 000000000..8f49fbc0e --- /dev/null +++ b/lib/catalog/__tests__/resolveCatalogSongsInScope.test.ts @@ -0,0 +1,54 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { resolveCatalogSongsInScope } from "../resolveCatalogSongsInScope"; +import { selectCatalogSongTitles } from "@/lib/supabase/catalog_songs/selectCatalogSongTitles"; +import { selectSongArtistIsrcs } from "@/lib/supabase/song_artists/selectSongArtistIsrcs"; + +vi.mock("@/lib/supabase/catalog_songs/selectCatalogSongTitles", () => ({ + selectCatalogSongTitles: vi.fn(), +})); +vi.mock("@/lib/supabase/song_artists/selectSongArtistIsrcs", () => ({ + selectSongArtistIsrcs: vi.fn(), +})); + +const catalogId = "7d3e5c35-1fac-4b88-aa90-e2e4a52dfe78"; +const artistAccountId = "b1814076-8e19-4a77-9dea-2ec150e26aaa"; + +const catalogSongs = [ + { isrc: "ISRC1", title: "Song One" }, + { isrc: "ISRC2", title: "Song Two" }, + { isrc: "ISRC3", title: "Song Three" }, +]; + +describe("resolveCatalogSongsInScope", () => { + beforeEach(() => vi.clearAllMocks()); + + it("returns every catalog song when no artist filter is given", async () => { + vi.mocked(selectCatalogSongTitles).mockResolvedValue(catalogSongs); + + const result = await resolveCatalogSongsInScope({ catalogId }); + + expect(result).toEqual(catalogSongs); + expect(selectSongArtistIsrcs).not.toHaveBeenCalled(); + }); + + it("restricts to the catalog ∩ song_artists intersection when filtered", async () => { + vi.mocked(selectCatalogSongTitles).mockResolvedValue(catalogSongs); + // ISRC4 is linked to the artist but not in this catalog — must not leak in + vi.mocked(selectSongArtistIsrcs).mockResolvedValue(["ISRC1", "ISRC3", "ISRC4"]); + + const result = await resolveCatalogSongsInScope({ catalogId, artistAccountId }); + + expect(selectSongArtistIsrcs).toHaveBeenCalledWith(artistAccountId); + expect(result).toEqual([ + { isrc: "ISRC1", title: "Song One" }, + { isrc: "ISRC3", title: "Song Three" }, + ]); + }); + + it("returns [] when the artist has no songs in the catalog", async () => { + vi.mocked(selectCatalogSongTitles).mockResolvedValue(catalogSongs); + vi.mocked(selectSongArtistIsrcs).mockResolvedValue(["OTHER1"]); + + expect(await resolveCatalogSongsInScope({ catalogId, artistAccountId })).toEqual([]); + }); +}); diff --git a/lib/catalog/__tests__/validateGetCatalogMeasurementsQuery.test.ts b/lib/catalog/__tests__/validateGetCatalogMeasurementsQuery.test.ts index 4748de478..e4d20b6fb 100644 --- a/lib/catalog/__tests__/validateGetCatalogMeasurementsQuery.test.ts +++ b/lib/catalog/__tests__/validateGetCatalogMeasurementsQuery.test.ts @@ -32,4 +32,24 @@ describe("validateGetCatalogMeasurementsQuery", () => { expect(result).toBeInstanceOf(NextResponse); expect((result as NextResponse).status).toBe(400); }); + + it("accepts an optional artist_account_id uuid", () => { + const artistAccountId = "b1814076-8e19-4a77-9dea-2ec150e26aaa"; + const result = validateGetCatalogMeasurementsQuery( + new URLSearchParams({ catalogId, artist_account_id: artistAccountId }), + ); + + expect(result).toEqual({ catalogId, artist_account_id: artistAccountId }); + }); + + it("returns 400 when artist_account_id is malformed", async () => { + const result = validateGetCatalogMeasurementsQuery( + new URLSearchParams({ catalogId, artist_account_id: "not-a-uuid" }), + ); + + expect(result).toBeInstanceOf(NextResponse); + expect((result as NextResponse).status).toBe(400); + const body = await (result as NextResponse).json(); + expect(body.status).toBe("error"); + }); }); diff --git a/lib/catalog/getCatalogMeasurementsHandler.ts b/lib/catalog/getCatalogMeasurementsHandler.ts index dcd8dfe08..92aee4367 100644 --- a/lib/catalog/getCatalogMeasurementsHandler.ts +++ b/lib/catalog/getCatalogMeasurementsHandler.ts @@ -6,20 +6,24 @@ import { validateGetCatalogMeasurementsQuery } from "./validateGetCatalogMeasure import { latestMeasurementsPerIsrc } from "./latestMeasurementsPerIsrc"; import { computeValuationBand } from "./computeValuationBand"; import { getCatalogEarliestReleaseDate } from "./getCatalogEarliestReleaseDate"; +import { resolveCatalogSongsInScope } from "./resolveCatalogSongsInScope"; import { selectAccountCatalog } from "@/lib/supabase/account_catalogs/selectAccountCatalog"; -import { selectCatalogSongTitles } from "@/lib/supabase/catalog_songs/selectCatalogSongTitles"; -import { selectSongMeasurements } from "@/lib/supabase/song_measurements/selectSongMeasurements"; +import { selectAllSongMeasurements } from "@/lib/supabase/song_measurements/selectAllSongMeasurements"; /** - * GET /api/catalogs/measurements?catalogId= + * GET /api/catalogs/measurements?catalogId=&artist_account_id= * * Latest Spotify play count per song (ISRC) in a catalog plus a valuation * band derived at read time with the same model as the marketing valuation - * card. The account is resolved from credentials (Privy bearer or x-api-key); - * a catalog that doesn't exist or belongs to another account is a 404. + * card. Optionally scoped to one artist's songs (catalog_songs ∩ + * song_artists) via artist_account_id; the applied filter is echoed back so + * clients can verify the response scope. The read is exhaustive — no + * 1,000-row cap. The account is resolved from credentials (Privy bearer or + * x-api-key); a catalog that doesn't exist or belongs to another account is + * a 404. * * @param request - The request object - * @returns `{ status, measurements, valuation, total_streams, catalog_age_years }` + * @returns `{ status, measurements, valuation, total_streams, artist_account_id, catalog_age_years }` */ export async function getCatalogMeasurementsHandler(request: NextRequest): Promise { try { @@ -34,18 +38,18 @@ export async function getCatalogMeasurementsHandler(request: NextRequest): Promi return authResult; } const { accountId } = authResult; - const { catalogId } = validated; + const { catalogId, artist_account_id: artistAccountId } = validated; const link = await selectAccountCatalog({ accountId, catalogId }); if (!link) { return errorResponse("Catalog not found", 404); } - const songs = await selectCatalogSongTitles(catalogId); + const songs = await resolveCatalogSongsInScope({ catalogId, artistAccountId }); const titles = new Map(songs.map(s => [s.isrc, s.title])); const [rows, earliestReleaseDate] = await Promise.all([ songs.length > 0 - ? selectSongMeasurements({ + ? selectAllSongMeasurements({ songs: songs.map(s => s.isrc), platform: "spotify", metric: "platform_displayed_play_count", @@ -64,6 +68,7 @@ export async function getCatalogMeasurementsHandler(request: NextRequest): Promi measurements, valuation, total_streams: totalStreams, + artist_account_id: artistAccountId ?? null, catalog_age_years: catalogAgeYears, }); } catch (error) { diff --git a/lib/catalog/resolveCatalogSongsInScope.ts b/lib/catalog/resolveCatalogSongsInScope.ts new file mode 100644 index 000000000..b85b3caa7 --- /dev/null +++ b/lib/catalog/resolveCatalogSongsInScope.ts @@ -0,0 +1,29 @@ +import { + selectCatalogSongTitles, + type CatalogSongTitle, +} from "@/lib/supabase/catalog_songs/selectCatalogSongTitles"; +import { selectSongArtistIsrcs } from "@/lib/supabase/song_artists/selectSongArtistIsrcs"; + +/** + * Resolve the songs a catalog measurements read covers: the whole catalog, + * or — when an artist filter is applied — only the catalog's songs linked + * to that artist account (catalog_songs ∩ song_artists). Songs the artist + * is linked to outside the catalog never leak in. + * + * @param params.catalogId - The catalog to list songs for + * @param params.artistAccountId - Optional artist account to scope the read to + * @returns ISRC + title pairs in scope, or [] when nothing matches + */ +export async function resolveCatalogSongsInScope({ + catalogId, + artistAccountId, +}: { + catalogId: string; + artistAccountId?: string; +}): Promise { + const songs = await selectCatalogSongTitles(catalogId); + if (!artistAccountId) return songs; + + const artistIsrcs = new Set(await selectSongArtistIsrcs(artistAccountId)); + return songs.filter(song => artistIsrcs.has(song.isrc)); +} diff --git a/lib/catalog/validateGetCatalogMeasurementsQuery.ts b/lib/catalog/validateGetCatalogMeasurementsQuery.ts index 0fc7801c9..deae52aa6 100644 --- a/lib/catalog/validateGetCatalogMeasurementsQuery.ts +++ b/lib/catalog/validateGetCatalogMeasurementsQuery.ts @@ -6,6 +6,7 @@ export const getCatalogMeasurementsQuerySchema = z.object({ catalogId: z .string({ message: "catalogId parameter is required" }) .uuid("catalogId must be a valid UUID"), + artist_account_id: z.string().uuid("artist_account_id must be a valid UUID").optional(), }); export type GetCatalogMeasurementsQuery = z.infer; diff --git a/lib/supabase/catalog_songs/__tests__/selectCatalogSongTitles.test.ts b/lib/supabase/catalog_songs/__tests__/selectCatalogSongTitles.test.ts index 5e09d6dcc..485fdb99f 100644 --- a/lib/supabase/catalog_songs/__tests__/selectCatalogSongTitles.test.ts +++ b/lib/supabase/catalog_songs/__tests__/selectCatalogSongTitles.test.ts @@ -7,41 +7,81 @@ vi.mock("../../serverClient", () => { return { default: { from: mockFrom } }; }); -function mockBuilder(result: { data: unknown; error: unknown }) { +type Result = { data: unknown; error: unknown }; + +function makeBuilder(result: Result) { const builder: Record> & { then?: (resolve: (v: unknown) => void) => void; } = {} as never; - for (const m of ["select", "eq"]) builder[m] = vi.fn().mockReturnValue(builder); + for (const m of ["select", "eq", "order", "range"]) builder[m] = vi.fn().mockReturnValue(builder); builder.then = resolve => resolve(result); - vi.mocked(supabase.from).mockReturnValue(builder as never); return builder; } +/** Queue one builder per supabase.from() call (one per page). */ +function mockPages(results: Result[]) { + const builders = results.map(makeBuilder); + let call = 0; + vi.mocked(supabase.from).mockImplementation(() => builders[call++] as never); + return builders; +} + +const row = (i: number) => ({ songs: { isrc: `ISRC${i}`, name: `Song ${i}` } }); + describe("selectCatalogSongTitles", () => { beforeEach(() => vi.clearAllMocks()); it("returns isrc + title pairs for the catalog's songs", async () => { - const builder = mockBuilder({ - data: [ - { songs: { isrc: "ISRC1", name: "Song One" } }, - { songs: { isrc: "ISRC2", name: null } }, - ], - error: null, - }); + mockPages([ + { + data: [ + { songs: { isrc: "ISRC1", name: "Song One" } }, + { songs: { isrc: "ISRC2", name: null } }, + ], + error: null, + }, + ]); const result = await selectCatalogSongTitles("cat_1"); expect(supabase.from).toHaveBeenCalledWith("catalog_songs"); - expect(builder.eq).toHaveBeenCalledWith("catalog", "cat_1"); expect(result).toEqual([ { isrc: "ISRC1", title: "Song One" }, { isrc: "ISRC2", title: null }, ]); }); + it("paginates past the 1,000-row page size to exhaustion", async () => { + const pageOne = Array.from({ length: 1000 }, (_, i) => row(i)); + const pageTwo = Array.from({ length: 680 }, (_, i) => row(1000 + i)); + const builders = mockPages([ + { data: pageOne, error: null }, + { data: pageTwo, error: null }, + ]); + + const result = await selectCatalogSongTitles("cat_big"); + + expect(result).toHaveLength(1680); + expect(result[0]).toEqual({ isrc: "ISRC0", title: "Song 0" }); + expect(result[1679]).toEqual({ isrc: "ISRC1679", title: "Song 1679" }); + expect(builders[0].eq).toHaveBeenCalledWith("catalog", "cat_big"); + expect(builders[0].range).toHaveBeenCalledWith(0, 999); + expect(builders[1].range).toHaveBeenCalledWith(1000, 1999); + // deterministic pagination requires a stable order + expect(builders[0].order).toHaveBeenCalledWith("song", { ascending: true }); + }); + + it("stops after one page when the catalog fits in a single page", async () => { + mockPages([{ data: [row(1), row(2)], error: null }]); + + await selectCatalogSongTitles("cat_small"); + + expect(supabase.from).toHaveBeenCalledTimes(1); + }); + it("returns [] on error", async () => { const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}); - mockBuilder({ data: null, error: { message: "boom" } }); + mockPages([{ data: null, error: { message: "boom" } }]); expect(await selectCatalogSongTitles("cat_1")).toEqual([]); consoleError.mockRestore(); diff --git a/lib/supabase/catalog_songs/selectCatalogSongTitles.ts b/lib/supabase/catalog_songs/selectCatalogSongTitles.ts index 9422348ae..aec557b1f 100644 --- a/lib/supabase/catalog_songs/selectCatalogSongTitles.ts +++ b/lib/supabase/catalog_songs/selectCatalogSongTitles.ts @@ -2,24 +2,35 @@ import supabase from "../serverClient"; export type CatalogSongTitle = { isrc: string; title: string | null }; +const PAGE_SIZE = 1000; + /** * Select the ISRC + title of every song in a catalog. Lighter than - * selectCatalogSongsWithArtists: no artists join, no pagination — used by - * reads that only need the tracklist (e.g. catalog measurements). + * selectCatalogSongsWithArtists: no artists join — used by reads that only + * need the tracklist (e.g. catalog measurements). Paginates past the + * Supabase 1,000-row default to exhaustion so large catalogs are complete. * * @param catalogId - The catalog to list songs for * @returns ISRC + title pairs, or [] if none exist or on error */ export async function selectCatalogSongTitles(catalogId: string): Promise { - const { data, error } = await supabase - .from("catalog_songs") - .select("songs!inner (isrc, name)") - .eq("catalog", catalogId); + const all: CatalogSongTitle[] = []; - if (error) { - console.error("Error fetching catalog_songs:", error); - return []; - } + for (let from = 0; ; from += PAGE_SIZE) { + const { data, error } = await supabase + .from("catalog_songs") + .select("songs!inner (isrc, name)") + .eq("catalog", catalogId) + .order("song", { ascending: true }) + .range(from, from + PAGE_SIZE - 1); - return (data ?? []).map(row => ({ isrc: row.songs.isrc, title: row.songs.name })); + if (error) { + console.error("Error fetching catalog_songs:", error); + return []; + } + + const rows = data ?? []; + all.push(...rows.map(row => ({ isrc: row.songs.isrc, title: row.songs.name }))); + if (rows.length < PAGE_SIZE) return all; + } } diff --git a/lib/supabase/song_artists/__tests__/selectSongArtistIsrcs.test.ts b/lib/supabase/song_artists/__tests__/selectSongArtistIsrcs.test.ts new file mode 100644 index 000000000..0afd9c272 --- /dev/null +++ b/lib/supabase/song_artists/__tests__/selectSongArtistIsrcs.test.ts @@ -0,0 +1,68 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { selectSongArtistIsrcs } from "../selectSongArtistIsrcs"; +import supabase from "../../serverClient"; + +vi.mock("../../serverClient", () => { + const mockFrom = vi.fn(); + return { default: { from: mockFrom } }; +}); + +type Result = { data: unknown; error: unknown }; + +function makeBuilder(result: Result) { + const builder: Record> & { + then?: (resolve: (v: unknown) => void) => void; + } = {} as never; + for (const m of ["select", "eq", "order", "range"]) builder[m] = vi.fn().mockReturnValue(builder); + builder.then = resolve => resolve(result); + return builder; +} + +/** Queue one builder per supabase.from() call (one per page). */ +function mockPages(results: Result[]) { + const builders = results.map(makeBuilder); + let call = 0; + vi.mocked(supabase.from).mockImplementation(() => builders[call++] as never); + return builders; +} + +const artistAccountId = "b1814076-8e19-4a77-9dea-2ec150e26aaa"; + +describe("selectSongArtistIsrcs", () => { + beforeEach(() => vi.clearAllMocks()); + + it("returns the ISRCs linked to the artist account", async () => { + const builders = mockPages([{ data: [{ song: "ISRC1" }, { song: "ISRC2" }], error: null }]); + + const result = await selectSongArtistIsrcs(artistAccountId); + + expect(supabase.from).toHaveBeenCalledWith("song_artists"); + expect(builders[0].eq).toHaveBeenCalledWith("artist", artistAccountId); + expect(result).toEqual(["ISRC1", "ISRC2"]); + }); + + it("paginates past the 1,000-row page size to exhaustion", async () => { + const pageOne = Array.from({ length: 1000 }, (_, i) => ({ song: `ISRC${i}` })); + const pageTwo = Array.from({ length: 200 }, (_, i) => ({ song: `ISRC${1000 + i}` })); + const builders = mockPages([ + { data: pageOne, error: null }, + { data: pageTwo, error: null }, + ]); + + const result = await selectSongArtistIsrcs(artistAccountId); + + expect(result).toHaveLength(1200); + expect(builders[0].range).toHaveBeenCalledWith(0, 999); + expect(builders[1].range).toHaveBeenCalledWith(1000, 1999); + // deterministic pagination requires a stable order + expect(builders[0].order).toHaveBeenCalledWith("song", { ascending: true }); + }); + + it("returns [] on error", async () => { + const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}); + mockPages([{ data: null, error: { message: "boom" } }]); + + expect(await selectSongArtistIsrcs(artistAccountId)).toEqual([]); + consoleError.mockRestore(); + }); +}); diff --git a/lib/supabase/song_artists/selectSongArtistIsrcs.ts b/lib/supabase/song_artists/selectSongArtistIsrcs.ts new file mode 100644 index 000000000..4f632cf3a --- /dev/null +++ b/lib/supabase/song_artists/selectSongArtistIsrcs.ts @@ -0,0 +1,34 @@ +import supabase from "../serverClient"; + +const PAGE_SIZE = 1000; + +/** + * Select the ISRC of every song linked to an artist account via + * song_artists. Paginates past the Supabase 1,000-row default to + * exhaustion so large discographies are complete — used to scope catalog + * measurement reads to one artist. + * + * @param artistAccountId - The artist account to list linked songs for + * @returns Song ISRCs, or [] if none exist or on error + */ +export async function selectSongArtistIsrcs(artistAccountId: string): Promise { + const all: string[] = []; + + for (let from = 0; ; from += PAGE_SIZE) { + const { data, error } = await supabase + .from("song_artists") + .select("song") + .eq("artist", artistAccountId) + .order("song", { ascending: true }) + .range(from, from + PAGE_SIZE - 1); + + if (error) { + console.error("Error fetching song_artists:", error); + return []; + } + + const rows = data ?? []; + all.push(...rows.map(row => row.song)); + if (rows.length < PAGE_SIZE) return all; + } +} diff --git a/lib/supabase/song_measurements/__tests__/selectAllSongMeasurements.test.ts b/lib/supabase/song_measurements/__tests__/selectAllSongMeasurements.test.ts new file mode 100644 index 000000000..abebd83c5 --- /dev/null +++ b/lib/supabase/song_measurements/__tests__/selectAllSongMeasurements.test.ts @@ -0,0 +1,102 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { selectAllSongMeasurements } from "../selectAllSongMeasurements"; +import supabase from "../../serverClient"; + +vi.mock("../../serverClient", () => { + const mockFrom = vi.fn(); + return { default: { from: mockFrom } }; +}); + +type Result = { data: unknown; error: unknown }; + +function makeBuilder(result: Result) { + const builder: Record> & { + then?: (resolve: (v: unknown) => void) => void; + } = {} as never; + for (const m of ["select", "eq", "in", "order", "range"]) + builder[m] = vi.fn().mockReturnValue(builder); + builder.then = resolve => resolve(result); + return builder; +} + +/** Queue one builder per supabase.from() call (one per page per chunk). */ +function mockPages(results: Result[]) { + const builders = results.map(makeBuilder); + let call = 0; + vi.mocked(supabase.from).mockImplementation(() => builders[call++] as never); + return builders; +} + +const row = (song: string, value: number, capturedAt = "2026-07-01T00:00:00Z") => ({ + song, + value, + captured_at: capturedAt, + platform: "spotify", + metric: "platform_displayed_play_count", +}); + +const params = { platform: "spotify", metric: "platform_displayed_play_count" }; + +describe("selectAllSongMeasurements", () => { + beforeEach(() => vi.clearAllMocks()); + + it("returns [] without querying when songs is empty", async () => { + expect(await selectAllSongMeasurements({ songs: [], ...params })).toEqual([]); + expect(supabase.from).not.toHaveBeenCalled(); + }); + + it("reads a small batch in one query, newest-first with filters applied", async () => { + const builders = mockPages([{ data: [row("ISRC1", 200), row("ISRC2", 50)], error: null }]); + + const result = await selectAllSongMeasurements({ songs: ["ISRC1", "ISRC2"], ...params }); + + expect(supabase.from).toHaveBeenCalledWith("song_measurements"); + expect(builders[0].in).toHaveBeenCalledWith("song", ["ISRC1", "ISRC2"]); + expect(builders[0].eq).toHaveBeenCalledWith("platform", "spotify"); + expect(builders[0].eq).toHaveBeenCalledWith("metric", "platform_displayed_play_count"); + expect(builders[0].order).toHaveBeenCalledWith("captured_at", { ascending: false }); + expect(builders[0].range).toHaveBeenCalledWith(0, 999); + expect(result).toEqual([row("ISRC1", 200), row("ISRC2", 50)]); + }); + + it("paginates each read past the 1,000-row page size to exhaustion", async () => { + const pageOne = Array.from({ length: 1000 }, (_, i) => row(`ISRC${i}`, i)); + const pageTwo = Array.from({ length: 300 }, (_, i) => row(`ISRC${1000 + i}`, i)); + const builders = mockPages([ + { data: pageOne, error: null }, + { data: pageTwo, error: null }, + ]); + + const songs = Array.from({ length: 400 }, (_, i) => `ISRC${i}`); + const result = await selectAllSongMeasurements({ songs, ...params }); + + expect(result).toHaveLength(1300); + expect(builders[0].range).toHaveBeenCalledWith(0, 999); + expect(builders[1].range).toHaveBeenCalledWith(1000, 1999); + }); + + it("chunks large ISRC lists so the IN clause stays bounded", async () => { + const songs = Array.from({ length: 1200 }, (_, i) => `ISRC${i}`); + const builders = mockPages([ + { data: [row("ISRC0", 1)], error: null }, + { data: [row("ISRC500", 2)], error: null }, + { data: [row("ISRC1000", 3)], error: null }, + ]); + + const result = await selectAllSongMeasurements({ songs, ...params }); + + expect(supabase.from).toHaveBeenCalledTimes(3); + expect(builders[0].in).toHaveBeenCalledWith("song", songs.slice(0, 500)); + expect(builders[1].in).toHaveBeenCalledWith("song", songs.slice(500, 1000)); + expect(builders[2].in).toHaveBeenCalledWith("song", songs.slice(1000)); + expect(result).toEqual([row("ISRC0", 1), row("ISRC500", 2), row("ISRC1000", 3)]); + }); + + it("returns [] on error", async () => { + const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}); + mockPages([{ data: null, error: { message: "boom" } }]); + + expect(await selectAllSongMeasurements({ songs: ["ISRC1"], ...params })).toEqual([]); + consoleError.mockRestore(); + }); +}); diff --git a/lib/supabase/song_measurements/selectAllSongMeasurements.ts b/lib/supabase/song_measurements/selectAllSongMeasurements.ts new file mode 100644 index 000000000..c99aa4515 --- /dev/null +++ b/lib/supabase/song_measurements/selectAllSongMeasurements.ts @@ -0,0 +1,58 @@ +import supabase from "../serverClient"; +import { Tables } from "@/types/database.types"; + +const CHUNK_SIZE = 500; // keeps the PostgREST IN clause well under URL length limits +const PAGE_SIZE = 1000; // Supabase default row cap — paginate past it explicitly + +/** + * Select every measurement for a batch of songs, newest-first per read. + * Unlike selectSongMeasurements this read is exhaustive: the ISRC list is + * chunked (bounded IN clause) and each chunk is paginated with .range() + * past the Supabase 1,000-row default, so no rows are silently dropped — + * required for catalog-scale reads (see api#757's silent 1,000-row cap). + * + * @param params.songs - Song ISRCs to read measurements for + * @param params.platform - Platform filter (e.g. "spotify") + * @param params.metric - Metric filter (e.g. "platform_displayed_play_count") + * @returns All measurement rows (newest-first within each chunk), or [] if none exist or on error + */ +export async function selectAllSongMeasurements({ + songs, + platform, + metric, +}: { + songs: string[]; + platform: string; + metric: string; +}): Promise[]> { + if (songs.length === 0) return []; + + const all: Tables<"song_measurements">[] = []; + + for (let i = 0; i < songs.length; i += CHUNK_SIZE) { + const chunk = songs.slice(i, i + CHUNK_SIZE); + + for (let from = 0; ; from += PAGE_SIZE) { + const { data, error } = await supabase + .from("song_measurements") + .select("*") + .in("song", chunk) + .eq("platform", platform) + .eq("metric", metric) + .order("captured_at", { ascending: false }) + .order("id", { ascending: false }) + .range(from, from + PAGE_SIZE - 1); + + if (error) { + console.error("Error fetching song_measurements:", error); + return []; + } + + const rows = data ?? []; + all.push(...rows); + if (rows.length < PAGE_SIZE) break; + } + } + + return all; +} From 03ed53c2a3c6daa2dc53b61b261f75be9c545c1c Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Tue, 7 Jul 2026 07:39:12 -0500 Subject: [PATCH 2/4] refactor(catalogs): measurements read = whole-scope SQL aggregate + caller-paginated page (chat#1850) Amendment per the 2026-07-07 decision on chat#1850: the app-code .range() loop-to-exhaustion is rejected. The read path becomes: - aggregates (measured_song_count, total_streams -> valuation band) from the get_catalog_measurements_aggregate RPC: one SQL aggregate over the latest-per-ISRC subquery, whole scope, no row cap - the measurements array from the get_catalog_measurements_page RPC: page/limit params (default 1/50, limit max 100, 400 on invalid), rows sorted by playcount desc, same pagination envelope as GET /api/catalogs/songs - artist_account_id filter + response echo unchanged Both RPCs ship in recoupable/database#42 (applied to the shared project). Deletes the loop-based helpers (selectAllSongMeasurements, selectSongArtistIsrcs, selectCatalogSongTitles, resolveCatalogSongsInScope, latestMeasurementsPerIsrc) whose only consumer was this handler. TDD: RED confirmed before GREEN per unit; full suite 3,940 tests, tsc 200 pre-existing errors (0 new), lint clean. Co-Authored-By: Claude Fable 5 --- .../getCatalogMeasurementsHandler.test.ts | 131 +++++++++++------- .../latestMeasurementsPerIsrc.test.ts | 59 -------- .../resolveCatalogSongsInScope.test.ts | 54 -------- ...alidateGetCatalogMeasurementsQuery.test.ts | 35 ++++- lib/catalog/getCatalogMeasurementsHandler.ts | 57 ++++---- lib/catalog/latestMeasurementsPerIsrc.ts | 35 ----- lib/catalog/resolveCatalogSongsInScope.ts | 29 ---- .../validateGetCatalogMeasurementsQuery.ts | 18 +++ .../__tests__/selectCatalogSongTitles.test.ts | 89 ------------ .../catalog_songs/selectCatalogSongTitles.ts | 36 ----- .../__tests__/selectSongArtistIsrcs.test.ts | 68 --------- .../song_artists/selectSongArtistIsrcs.ts | 34 ----- .../selectAllSongMeasurements.test.ts | 102 -------------- ...selectCatalogMeasurementsAggregate.test.ts | 74 ++++++++++ .../selectCatalogMeasurementsPage.test.ts | 64 +++++++++ .../selectAllSongMeasurements.ts | 58 -------- .../selectCatalogMeasurementsAggregate.ts | 41 ++++++ .../selectCatalogMeasurementsPage.ts | 47 +++++++ types/database.types.ts | 21 +++ 19 files changed, 410 insertions(+), 642 deletions(-) delete mode 100644 lib/catalog/__tests__/latestMeasurementsPerIsrc.test.ts delete mode 100644 lib/catalog/__tests__/resolveCatalogSongsInScope.test.ts delete mode 100644 lib/catalog/latestMeasurementsPerIsrc.ts delete mode 100644 lib/catalog/resolveCatalogSongsInScope.ts delete mode 100644 lib/supabase/catalog_songs/__tests__/selectCatalogSongTitles.test.ts delete mode 100644 lib/supabase/catalog_songs/selectCatalogSongTitles.ts delete mode 100644 lib/supabase/song_artists/__tests__/selectSongArtistIsrcs.test.ts delete mode 100644 lib/supabase/song_artists/selectSongArtistIsrcs.ts delete mode 100644 lib/supabase/song_measurements/__tests__/selectAllSongMeasurements.test.ts create mode 100644 lib/supabase/song_measurements/__tests__/selectCatalogMeasurementsAggregate.test.ts create mode 100644 lib/supabase/song_measurements/__tests__/selectCatalogMeasurementsPage.test.ts delete mode 100644 lib/supabase/song_measurements/selectAllSongMeasurements.ts create mode 100644 lib/supabase/song_measurements/selectCatalogMeasurementsAggregate.ts create mode 100644 lib/supabase/song_measurements/selectCatalogMeasurementsPage.ts diff --git a/lib/catalog/__tests__/getCatalogMeasurementsHandler.test.ts b/lib/catalog/__tests__/getCatalogMeasurementsHandler.test.ts index fa2955ff3..d5743ef56 100644 --- a/lib/catalog/__tests__/getCatalogMeasurementsHandler.test.ts +++ b/lib/catalog/__tests__/getCatalogMeasurementsHandler.test.ts @@ -4,10 +4,10 @@ import { NextRequest, NextResponse } from "next/server"; import { getCatalogMeasurementsHandler } from "../getCatalogMeasurementsHandler"; import { validateGetCatalogMeasurementsQuery } from "../validateGetCatalogMeasurementsQuery"; import { getCatalogEarliestReleaseDate } from "../getCatalogEarliestReleaseDate"; -import { resolveCatalogSongsInScope } from "../resolveCatalogSongsInScope"; import { validateAuthContext } from "@/lib/auth/validateAuthContext"; import { selectAccountCatalog } from "@/lib/supabase/account_catalogs/selectAccountCatalog"; -import { selectAllSongMeasurements } from "@/lib/supabase/song_measurements/selectAllSongMeasurements"; +import { selectCatalogMeasurementsAggregate } from "@/lib/supabase/song_measurements/selectCatalogMeasurementsAggregate"; +import { selectCatalogMeasurementsPage } from "@/lib/supabase/song_measurements/selectCatalogMeasurementsPage"; vi.mock("@/lib/networking/getCorsHeaders", () => ({ getCorsHeaders: vi.fn(() => ({ "Access-Control-Allow-Origin": "*" })), @@ -16,13 +16,15 @@ vi.mock("../validateGetCatalogMeasurementsQuery", () => ({ validateGetCatalogMeasurementsQuery: vi.fn(), })); vi.mock("../getCatalogEarliestReleaseDate", () => ({ getCatalogEarliestReleaseDate: vi.fn() })); -vi.mock("../resolveCatalogSongsInScope", () => ({ resolveCatalogSongsInScope: vi.fn() })); vi.mock("@/lib/auth/validateAuthContext", () => ({ validateAuthContext: vi.fn() })); vi.mock("@/lib/supabase/account_catalogs/selectAccountCatalog", () => ({ selectAccountCatalog: vi.fn(), })); -vi.mock("@/lib/supabase/song_measurements/selectAllSongMeasurements", () => ({ - selectAllSongMeasurements: vi.fn(), +vi.mock("@/lib/supabase/song_measurements/selectCatalogMeasurementsAggregate", () => ({ + selectCatalogMeasurementsAggregate: vi.fn(), +})); +vi.mock("@/lib/supabase/song_measurements/selectCatalogMeasurementsPage", () => ({ + selectCatalogMeasurementsPage: vi.fn(), })); const accountId = "550e8400-e29b-41d4-a716-446655440000"; @@ -39,7 +41,7 @@ const okAuth = () => authToken: "t", } as never); -const okQuery = (query: Record = { catalogId }) => +const okQuery = (query: Record = { catalogId, page: 1, limit: 50 }) => vi.mocked(validateGetCatalogMeasurementsQuery).mockReturnValue(query as never); const okCatalog = () => @@ -48,6 +50,11 @@ const okCatalog = () => catalog: catalogId, } as never); +const pageRows = [ + { isrc: "ISRC1", title: "Song One", playcount: 200, measured_at: "2026-07-01T00:00:00Z" }, + { isrc: "ISRC2", title: "Song Two", playcount: 50, measured_at: "2026-07-01T00:00:00Z" }, +]; + describe("getCatalogMeasurementsHandler", () => { beforeEach(() => vi.clearAllMocks()); @@ -81,23 +88,19 @@ describe("getCatalogMeasurementsHandler", () => { expect(res.status).toBe(404); expect(selectAccountCatalog).toHaveBeenCalledWith({ accountId, catalogId }); - expect(resolveCatalogSongsInScope).not.toHaveBeenCalled(); + expect(selectCatalogMeasurementsAggregate).not.toHaveBeenCalled(); }); - it("returns latest per-ISRC measurements + the derived valuation band", async () => { + it("returns one page of measurements + whole-scope aggregates and the derived band", async () => { okQuery(); okAuth(); okCatalog(); - vi.mocked(resolveCatalogSongsInScope).mockResolvedValue([ - { isrc: "ISRC1", title: "Song One" }, - { isrc: "ISRC2", title: "Song Two" }, - ]); - vi.mocked(selectAllSongMeasurements).mockResolvedValue([ - // newest-first series; ISRC1 has an older superseded capture - { song: "ISRC1", value: 200, captured_at: "2026-07-01T00:00:00Z" }, - { song: "ISRC2", value: 50, captured_at: "2026-07-01T00:00:00Z" }, - { song: "ISRC1", value: 100, captured_at: "2026-06-01T00:00:00Z" }, - ] as never); + // whole-scope aggregate is bigger than the returned page + vi.mocked(selectCatalogMeasurementsAggregate).mockResolvedValue({ + measuredSongCount: 120, + totalStreams: 250, + }); + vi.mocked(selectCatalogMeasurementsPage).mockResolvedValue(pageRows); // 10 years of catalog age vi.mocked(getCatalogEarliestReleaseDate).mockResolvedValue("2016-07-01"); @@ -105,20 +108,25 @@ describe("getCatalogMeasurementsHandler", () => { const body = await res.json(); expect(res.status).toBe(200); - expect(resolveCatalogSongsInScope).toHaveBeenCalledWith({ + expect(selectCatalogMeasurementsAggregate).toHaveBeenCalledWith({ catalogId, artistAccountId: undefined, }); - expect(selectAllSongMeasurements).toHaveBeenCalledWith({ - songs: ["ISRC1", "ISRC2"], - platform: "spotify", - metric: "platform_displayed_play_count", + expect(selectCatalogMeasurementsPage).toHaveBeenCalledWith({ + catalogId, + artistAccountId: undefined, + page: 1, + limit: 50, }); expect(body.status).toBe("success"); - expect(body.measurements).toEqual([ - { isrc: "ISRC1", title: "Song One", playcount: 200, measured_at: "2026-07-01T00:00:00Z" }, - { isrc: "ISRC2", title: "Song Two", playcount: 50, measured_at: "2026-07-01T00:00:00Z" }, - ]); + expect(body.measurements).toEqual(pageRows); + expect(body.pagination).toEqual({ + total_count: 120, + page: 1, + limit: 50, + total_pages: 3, + }); + expect(body.measured_song_count).toBe(120); expect(body.total_streams).toBe(250); expect(body.catalog_age_years).toBe(10); // whole-catalog read: the echo field states no artist filter was applied @@ -130,60 +138,87 @@ describe("getCatalogMeasurementsHandler", () => { }); it("scopes the read to the artist and echoes the applied filter", async () => { - okQuery({ catalogId, artist_account_id: artistAccountId }); + okQuery({ catalogId, artist_account_id: artistAccountId, page: 2, limit: 10 }); okAuth(); okCatalog(); - vi.mocked(resolveCatalogSongsInScope).mockResolvedValue([{ isrc: "ISRC1", title: "Song One" }]); - vi.mocked(selectAllSongMeasurements).mockResolvedValue([ - { song: "ISRC1", value: 200, captured_at: "2026-07-01T00:00:00Z" }, - ] as never); + vi.mocked(selectCatalogMeasurementsAggregate).mockResolvedValue({ + measuredSongCount: 11, + totalStreams: 200, + }); + vi.mocked(selectCatalogMeasurementsPage).mockResolvedValue([pageRows[0]]); vi.mocked(getCatalogEarliestReleaseDate).mockResolvedValue("2016-07-01"); const res = await getCatalogMeasurementsHandler(makeRequest()); const body = await res.json(); expect(res.status).toBe(200); - expect(resolveCatalogSongsInScope).toHaveBeenCalledWith({ catalogId, artistAccountId }); - expect(body.measurements).toEqual([ - { isrc: "ISRC1", title: "Song One", playcount: 200, measured_at: "2026-07-01T00:00:00Z" }, - ]); + expect(selectCatalogMeasurementsAggregate).toHaveBeenCalledWith({ + catalogId, + artistAccountId, + }); + expect(selectCatalogMeasurementsPage).toHaveBeenCalledWith({ + catalogId, + artistAccountId, + page: 2, + limit: 10, + }); + expect(body.measurements).toEqual([pageRows[0]]); + expect(body.pagination).toEqual({ total_count: 11, page: 2, limit: 10, total_pages: 2 }); + expect(body.measured_song_count).toBe(11); expect(body.total_streams).toBe(200); expect(body.artist_account_id).toBe(artistAccountId); }); - it("returns an empty result with a zero band when the artist has no songs in the catalog", async () => { - okQuery({ catalogId, artist_account_id: artistAccountId }); + it("returns an empty page with a zero band when nothing is measured in scope", async () => { + okQuery({ catalogId, artist_account_id: artistAccountId, page: 1, limit: 50 }); okAuth(); okCatalog(); - vi.mocked(resolveCatalogSongsInScope).mockResolvedValue([]); + vi.mocked(selectCatalogMeasurementsAggregate).mockResolvedValue({ + measuredSongCount: 0, + totalStreams: 0, + }); + vi.mocked(selectCatalogMeasurementsPage).mockResolvedValue([]); vi.mocked(getCatalogEarliestReleaseDate).mockResolvedValue(null); const res = await getCatalogMeasurementsHandler(makeRequest()); const body = await res.json(); expect(res.status).toBe(200); - expect(selectAllSongMeasurements).not.toHaveBeenCalled(); expect(body.measurements).toEqual([]); + expect(body.pagination).toEqual({ total_count: 0, page: 1, limit: 50, total_pages: 0 }); + expect(body.measured_song_count).toBe(0); expect(body.total_streams).toBe(0); expect(body.valuation).toEqual({ low: 0, mid: 0, high: 0 }); expect(body.artist_account_id).toBe(artistAccountId); }); - it("returns an empty result with a zero band for a catalog with no measurements", async () => { + it("returns 500 when the aggregate read fails", async () => { okQuery(); okAuth(); okCatalog(); - vi.mocked(resolveCatalogSongsInScope).mockResolvedValue([]); + vi.mocked(selectCatalogMeasurementsAggregate).mockResolvedValue(null); + vi.mocked(selectCatalogMeasurementsPage).mockResolvedValue(pageRows); vi.mocked(getCatalogEarliestReleaseDate).mockResolvedValue(null); const res = await getCatalogMeasurementsHandler(makeRequest()); - const body = await res.json(); - expect(res.status).toBe(200); - expect(body.measurements).toEqual([]); - expect(body.total_streams).toBe(0); - expect(body.valuation).toEqual({ low: 0, mid: 0, high: 0 }); - expect(body.artist_account_id).toBeNull(); + expect(res.status).toBe(500); + }); + + it("returns 500 when the page read fails", async () => { + okQuery(); + okAuth(); + okCatalog(); + vi.mocked(selectCatalogMeasurementsAggregate).mockResolvedValue({ + measuredSongCount: 1, + totalStreams: 1, + }); + vi.mocked(selectCatalogMeasurementsPage).mockResolvedValue(null); + vi.mocked(getCatalogEarliestReleaseDate).mockResolvedValue(null); + + const res = await getCatalogMeasurementsHandler(makeRequest()); + + expect(res.status).toBe(500); }); it("returns 500 when a dependency throws", async () => { diff --git a/lib/catalog/__tests__/latestMeasurementsPerIsrc.test.ts b/lib/catalog/__tests__/latestMeasurementsPerIsrc.test.ts deleted file mode 100644 index 6ff0b3503..000000000 --- a/lib/catalog/__tests__/latestMeasurementsPerIsrc.test.ts +++ /dev/null @@ -1,59 +0,0 @@ -import { describe, it, expect } from "vitest"; -import { latestMeasurementsPerIsrc } from "../latestMeasurementsPerIsrc"; -import type { Tables } from "@/types/database.types"; - -const row = (song: string, value: number, captured_at: string): Tables<"song_measurements"> => - ({ - id: `${song}-${captured_at}`, - song, - value, - captured_at, - created_at: captured_at, - data_source: "spotify_web", - metric: "platform_displayed_play_count", - platform: "spotify", - raw_ref: null, - snapshot: null, - }) as Tables<"song_measurements">; - -describe("latestMeasurementsPerIsrc", () => { - it("keeps only the newest capture per ISRC and sums the total", () => { - // rows arrive newest-first (selectSongMeasurements orders captured_at desc) - const rows = [ - row("ISRC1", 200, "2026-07-01T00:00:00Z"), - row("ISRC2", 50, "2026-07-01T00:00:00Z"), - row("ISRC1", 100, "2026-06-01T00:00:00Z"), - ]; - const titles = new Map([ - ["ISRC1", "Song One"], - ["ISRC2", null], - ]); - - const result = latestMeasurementsPerIsrc(rows, titles); - - expect(result.totalStreams).toBe(250); - expect(result.measurements).toEqual([ - { isrc: "ISRC1", title: "Song One", playcount: 200, measured_at: "2026-07-01T00:00:00Z" }, - { isrc: "ISRC2", title: null, playcount: 50, measured_at: "2026-07-01T00:00:00Z" }, - ]); - }); - - it("sorts measurements by playcount descending", () => { - const rows = [ - row("ISRC1", 10, "2026-07-01T00:00:00Z"), - row("ISRC2", 999, "2026-07-01T00:00:00Z"), - ]; - - const result = latestMeasurementsPerIsrc(rows, new Map()); - - expect(result.measurements.map(m => m.isrc)).toEqual(["ISRC2", "ISRC1"]); - expect(result.measurements[0].title).toBeNull(); - }); - - it("returns an empty result for no rows", () => { - expect(latestMeasurementsPerIsrc([], new Map())).toEqual({ - measurements: [], - totalStreams: 0, - }); - }); -}); diff --git a/lib/catalog/__tests__/resolveCatalogSongsInScope.test.ts b/lib/catalog/__tests__/resolveCatalogSongsInScope.test.ts deleted file mode 100644 index 8f49fbc0e..000000000 --- a/lib/catalog/__tests__/resolveCatalogSongsInScope.test.ts +++ /dev/null @@ -1,54 +0,0 @@ -import { describe, it, expect, vi, beforeEach } from "vitest"; -import { resolveCatalogSongsInScope } from "../resolveCatalogSongsInScope"; -import { selectCatalogSongTitles } from "@/lib/supabase/catalog_songs/selectCatalogSongTitles"; -import { selectSongArtistIsrcs } from "@/lib/supabase/song_artists/selectSongArtistIsrcs"; - -vi.mock("@/lib/supabase/catalog_songs/selectCatalogSongTitles", () => ({ - selectCatalogSongTitles: vi.fn(), -})); -vi.mock("@/lib/supabase/song_artists/selectSongArtistIsrcs", () => ({ - selectSongArtistIsrcs: vi.fn(), -})); - -const catalogId = "7d3e5c35-1fac-4b88-aa90-e2e4a52dfe78"; -const artistAccountId = "b1814076-8e19-4a77-9dea-2ec150e26aaa"; - -const catalogSongs = [ - { isrc: "ISRC1", title: "Song One" }, - { isrc: "ISRC2", title: "Song Two" }, - { isrc: "ISRC3", title: "Song Three" }, -]; - -describe("resolveCatalogSongsInScope", () => { - beforeEach(() => vi.clearAllMocks()); - - it("returns every catalog song when no artist filter is given", async () => { - vi.mocked(selectCatalogSongTitles).mockResolvedValue(catalogSongs); - - const result = await resolveCatalogSongsInScope({ catalogId }); - - expect(result).toEqual(catalogSongs); - expect(selectSongArtistIsrcs).not.toHaveBeenCalled(); - }); - - it("restricts to the catalog ∩ song_artists intersection when filtered", async () => { - vi.mocked(selectCatalogSongTitles).mockResolvedValue(catalogSongs); - // ISRC4 is linked to the artist but not in this catalog — must not leak in - vi.mocked(selectSongArtistIsrcs).mockResolvedValue(["ISRC1", "ISRC3", "ISRC4"]); - - const result = await resolveCatalogSongsInScope({ catalogId, artistAccountId }); - - expect(selectSongArtistIsrcs).toHaveBeenCalledWith(artistAccountId); - expect(result).toEqual([ - { isrc: "ISRC1", title: "Song One" }, - { isrc: "ISRC3", title: "Song Three" }, - ]); - }); - - it("returns [] when the artist has no songs in the catalog", async () => { - vi.mocked(selectCatalogSongTitles).mockResolvedValue(catalogSongs); - vi.mocked(selectSongArtistIsrcs).mockResolvedValue(["OTHER1"]); - - expect(await resolveCatalogSongsInScope({ catalogId, artistAccountId })).toEqual([]); - }); -}); diff --git a/lib/catalog/__tests__/validateGetCatalogMeasurementsQuery.test.ts b/lib/catalog/__tests__/validateGetCatalogMeasurementsQuery.test.ts index e4d20b6fb..bf88296b9 100644 --- a/lib/catalog/__tests__/validateGetCatalogMeasurementsQuery.test.ts +++ b/lib/catalog/__tests__/validateGetCatalogMeasurementsQuery.test.ts @@ -9,10 +9,10 @@ vi.mock("@/lib/networking/getCorsHeaders", () => ({ const catalogId = "740d5050-40ec-4892-a040-b78bb50fef2f"; describe("validateGetCatalogMeasurementsQuery", () => { - it("returns the validated query for a valid catalogId", () => { + it("returns the validated query for a valid catalogId with default pagination", () => { const result = validateGetCatalogMeasurementsQuery(new URLSearchParams({ catalogId })); - expect(result).toEqual({ catalogId }); + expect(result).toEqual({ catalogId, page: 1, limit: 50 }); }); it("returns 400 when catalogId is missing", async () => { @@ -39,7 +39,12 @@ describe("validateGetCatalogMeasurementsQuery", () => { new URLSearchParams({ catalogId, artist_account_id: artistAccountId }), ); - expect(result).toEqual({ catalogId, artist_account_id: artistAccountId }); + expect(result).toEqual({ + catalogId, + artist_account_id: artistAccountId, + page: 1, + limit: 50, + }); }); it("returns 400 when artist_account_id is malformed", async () => { @@ -52,4 +57,28 @@ describe("validateGetCatalogMeasurementsQuery", () => { const body = await (result as NextResponse).json(); expect(body.status).toBe("error"); }); + + it("accepts explicit page and limit", () => { + const result = validateGetCatalogMeasurementsQuery( + new URLSearchParams({ catalogId, page: "3", limit: "100" }), + ); + + expect(result).toEqual({ catalogId, page: 3, limit: 100 }); + }); + + it("returns 400 when page is not a positive integer", () => { + for (const page of ["0", "-1", "abc", "1.5"]) { + const result = validateGetCatalogMeasurementsQuery(new URLSearchParams({ catalogId, page })); + expect(result).toBeInstanceOf(NextResponse); + expect((result as NextResponse).status).toBe(400); + } + }); + + it("returns 400 when limit is out of range", () => { + for (const limit of ["0", "101", "abc"]) { + const result = validateGetCatalogMeasurementsQuery(new URLSearchParams({ catalogId, limit })); + expect(result).toBeInstanceOf(NextResponse); + expect((result as NextResponse).status).toBe(400); + } + }); }); diff --git a/lib/catalog/getCatalogMeasurementsHandler.ts b/lib/catalog/getCatalogMeasurementsHandler.ts index 92aee4367..aa7fcec70 100644 --- a/lib/catalog/getCatalogMeasurementsHandler.ts +++ b/lib/catalog/getCatalogMeasurementsHandler.ts @@ -3,27 +3,28 @@ import { errorResponse } from "@/lib/networking/errorResponse"; import { successResponse } from "@/lib/networking/successResponse"; import { validateAuthContext } from "@/lib/auth/validateAuthContext"; import { validateGetCatalogMeasurementsQuery } from "./validateGetCatalogMeasurementsQuery"; -import { latestMeasurementsPerIsrc } from "./latestMeasurementsPerIsrc"; import { computeValuationBand } from "./computeValuationBand"; import { getCatalogEarliestReleaseDate } from "./getCatalogEarliestReleaseDate"; -import { resolveCatalogSongsInScope } from "./resolveCatalogSongsInScope"; import { selectAccountCatalog } from "@/lib/supabase/account_catalogs/selectAccountCatalog"; -import { selectAllSongMeasurements } from "@/lib/supabase/song_measurements/selectAllSongMeasurements"; +import { selectCatalogMeasurementsAggregate } from "@/lib/supabase/song_measurements/selectCatalogMeasurementsAggregate"; +import { selectCatalogMeasurementsPage } from "@/lib/supabase/song_measurements/selectCatalogMeasurementsPage"; /** - * GET /api/catalogs/measurements?catalogId=&artist_account_id= + * GET /api/catalogs/measurements?catalogId=&artist_account_id=&page=&limit= * - * Latest Spotify play count per song (ISRC) in a catalog plus a valuation - * band derived at read time with the same model as the marketing valuation - * card. Optionally scoped to one artist's songs (catalog_songs ∩ - * song_artists) via artist_account_id; the applied filter is echoed back so - * clients can verify the response scope. The read is exhaustive — no - * 1,000-row cap. The account is resolved from credentials (Privy bearer or - * x-api-key); a catalog that doesn't exist or belongs to another account is - * a 404. + * One page of the latest Spotify play counts per song (ISRC) in a catalog + * plus whole-scope aggregates: measured_song_count, total_streams and a + * valuation band derived at read time with the same model as the marketing + * valuation card. The aggregates are computed in a single SQL aggregate over + * the entire scope — no row cap — regardless of the requested page. + * Optionally scoped to one artist's songs (catalog_songs ∩ song_artists) via + * artist_account_id; the applied filter is echoed back so clients can verify + * the response scope. The account is resolved from credentials (Privy bearer + * or x-api-key); a catalog that doesn't exist or belongs to another account + * is a 404. * * @param request - The request object - * @returns `{ status, measurements, valuation, total_streams, artist_account_id, catalog_age_years }` + * @returns `{ status, measurements, pagination, measured_song_count, total_streams, valuation, artist_account_id, catalog_age_years }` */ export async function getCatalogMeasurementsHandler(request: NextRequest): Promise { try { @@ -38,36 +39,38 @@ export async function getCatalogMeasurementsHandler(request: NextRequest): Promi return authResult; } const { accountId } = authResult; - const { catalogId, artist_account_id: artistAccountId } = validated; + const { catalogId, artist_account_id: artistAccountId, page, limit } = validated; const link = await selectAccountCatalog({ accountId, catalogId }); if (!link) { return errorResponse("Catalog not found", 404); } - const songs = await resolveCatalogSongsInScope({ catalogId, artistAccountId }); - const titles = new Map(songs.map(s => [s.isrc, s.title])); - const [rows, earliestReleaseDate] = await Promise.all([ - songs.length > 0 - ? selectAllSongMeasurements({ - songs: songs.map(s => s.isrc), - platform: "spotify", - metric: "platform_displayed_play_count", - }) - : Promise.resolve([]), + const [aggregate, measurements, earliestReleaseDate] = await Promise.all([ + selectCatalogMeasurementsAggregate({ catalogId, artistAccountId }), + selectCatalogMeasurementsPage({ catalogId, artistAccountId, page, limit }), getCatalogEarliestReleaseDate(catalogId), ]); + if (!aggregate || !measurements) { + return errorResponse("Internal server error", 500); + } - const { measurements, totalStreams } = latestMeasurementsPerIsrc(rows, titles); const { valuation, catalogAgeYears } = computeValuationBand({ - totalStreams, + totalStreams: aggregate.totalStreams, earliestReleaseDate, }); return successResponse({ measurements, + pagination: { + total_count: aggregate.measuredSongCount, + page, + limit, + total_pages: Math.ceil(aggregate.measuredSongCount / limit), + }, + measured_song_count: aggregate.measuredSongCount, + total_streams: aggregate.totalStreams, valuation, - total_streams: totalStreams, artist_account_id: artistAccountId ?? null, catalog_age_years: catalogAgeYears, }); diff --git a/lib/catalog/latestMeasurementsPerIsrc.ts b/lib/catalog/latestMeasurementsPerIsrc.ts deleted file mode 100644 index 3774bf387..000000000 --- a/lib/catalog/latestMeasurementsPerIsrc.ts +++ /dev/null @@ -1,35 +0,0 @@ -import type { Tables } from "@/types/database.types"; - -export type CatalogTrackMeasurement = { - isrc: string; - title: string | null; - playcount: number; - measured_at: string; -}; - -/** - * Reduce a newest-first measurement series to the latest capture per ISRC, - * shaped for the response, sorted by playcount descending, plus the total. - * - * @param rows - Measurement rows ordered captured_at desc (selectSongMeasurements order) - * @param titles - Song titles by ISRC (from the catalog's songs) - */ -export function latestMeasurementsPerIsrc( - rows: Tables<"song_measurements">[], - titles: Map, -): { measurements: CatalogTrackMeasurement[]; totalStreams: number } { - const latest = new Map(); - for (const row of rows) { - if (latest.has(row.song)) continue; // newest-first: first row per ISRC wins - latest.set(row.song, { - isrc: row.song, - title: titles.get(row.song) ?? null, - playcount: row.value, - measured_at: row.captured_at, - }); - } - - const measurements = [...latest.values()].sort((a, b) => b.playcount - a.playcount); - const totalStreams = measurements.reduce((sum, m) => sum + m.playcount, 0); - return { measurements, totalStreams }; -} diff --git a/lib/catalog/resolveCatalogSongsInScope.ts b/lib/catalog/resolveCatalogSongsInScope.ts deleted file mode 100644 index b85b3caa7..000000000 --- a/lib/catalog/resolveCatalogSongsInScope.ts +++ /dev/null @@ -1,29 +0,0 @@ -import { - selectCatalogSongTitles, - type CatalogSongTitle, -} from "@/lib/supabase/catalog_songs/selectCatalogSongTitles"; -import { selectSongArtistIsrcs } from "@/lib/supabase/song_artists/selectSongArtistIsrcs"; - -/** - * Resolve the songs a catalog measurements read covers: the whole catalog, - * or — when an artist filter is applied — only the catalog's songs linked - * to that artist account (catalog_songs ∩ song_artists). Songs the artist - * is linked to outside the catalog never leak in. - * - * @param params.catalogId - The catalog to list songs for - * @param params.artistAccountId - Optional artist account to scope the read to - * @returns ISRC + title pairs in scope, or [] when nothing matches - */ -export async function resolveCatalogSongsInScope({ - catalogId, - artistAccountId, -}: { - catalogId: string; - artistAccountId?: string; -}): Promise { - const songs = await selectCatalogSongTitles(catalogId); - if (!artistAccountId) return songs; - - const artistIsrcs = new Set(await selectSongArtistIsrcs(artistAccountId)); - return songs.filter(song => artistIsrcs.has(song.isrc)); -} diff --git a/lib/catalog/validateGetCatalogMeasurementsQuery.ts b/lib/catalog/validateGetCatalogMeasurementsQuery.ts index deae52aa6..4113dd946 100644 --- a/lib/catalog/validateGetCatalogMeasurementsQuery.ts +++ b/lib/catalog/validateGetCatalogMeasurementsQuery.ts @@ -7,6 +7,24 @@ export const getCatalogMeasurementsQuerySchema = z.object({ .string({ message: "catalogId parameter is required" }) .uuid("catalogId must be a valid UUID"), artist_account_id: z.string().uuid("artist_account_id must be a valid UUID").optional(), + page: z + .string() + .optional() + .default("1") + .transform(val => Number(val)) + .pipe(z.number().int("page must be a positive integer").positive("page must be positive")), + limit: z + .string() + .optional() + .default("50") + .transform(val => Number(val)) + .pipe( + z + .number() + .int("limit must be an integer") + .min(1, "limit must be at least 1") + .max(100, "limit must be at most 100"), + ), }); export type GetCatalogMeasurementsQuery = z.infer; diff --git a/lib/supabase/catalog_songs/__tests__/selectCatalogSongTitles.test.ts b/lib/supabase/catalog_songs/__tests__/selectCatalogSongTitles.test.ts deleted file mode 100644 index 485fdb99f..000000000 --- a/lib/supabase/catalog_songs/__tests__/selectCatalogSongTitles.test.ts +++ /dev/null @@ -1,89 +0,0 @@ -import { describe, it, expect, vi, beforeEach } from "vitest"; -import { selectCatalogSongTitles } from "../selectCatalogSongTitles"; -import supabase from "../../serverClient"; - -vi.mock("../../serverClient", () => { - const mockFrom = vi.fn(); - return { default: { from: mockFrom } }; -}); - -type Result = { data: unknown; error: unknown }; - -function makeBuilder(result: Result) { - const builder: Record> & { - then?: (resolve: (v: unknown) => void) => void; - } = {} as never; - for (const m of ["select", "eq", "order", "range"]) builder[m] = vi.fn().mockReturnValue(builder); - builder.then = resolve => resolve(result); - return builder; -} - -/** Queue one builder per supabase.from() call (one per page). */ -function mockPages(results: Result[]) { - const builders = results.map(makeBuilder); - let call = 0; - vi.mocked(supabase.from).mockImplementation(() => builders[call++] as never); - return builders; -} - -const row = (i: number) => ({ songs: { isrc: `ISRC${i}`, name: `Song ${i}` } }); - -describe("selectCatalogSongTitles", () => { - beforeEach(() => vi.clearAllMocks()); - - it("returns isrc + title pairs for the catalog's songs", async () => { - mockPages([ - { - data: [ - { songs: { isrc: "ISRC1", name: "Song One" } }, - { songs: { isrc: "ISRC2", name: null } }, - ], - error: null, - }, - ]); - - const result = await selectCatalogSongTitles("cat_1"); - - expect(supabase.from).toHaveBeenCalledWith("catalog_songs"); - expect(result).toEqual([ - { isrc: "ISRC1", title: "Song One" }, - { isrc: "ISRC2", title: null }, - ]); - }); - - it("paginates past the 1,000-row page size to exhaustion", async () => { - const pageOne = Array.from({ length: 1000 }, (_, i) => row(i)); - const pageTwo = Array.from({ length: 680 }, (_, i) => row(1000 + i)); - const builders = mockPages([ - { data: pageOne, error: null }, - { data: pageTwo, error: null }, - ]); - - const result = await selectCatalogSongTitles("cat_big"); - - expect(result).toHaveLength(1680); - expect(result[0]).toEqual({ isrc: "ISRC0", title: "Song 0" }); - expect(result[1679]).toEqual({ isrc: "ISRC1679", title: "Song 1679" }); - expect(builders[0].eq).toHaveBeenCalledWith("catalog", "cat_big"); - expect(builders[0].range).toHaveBeenCalledWith(0, 999); - expect(builders[1].range).toHaveBeenCalledWith(1000, 1999); - // deterministic pagination requires a stable order - expect(builders[0].order).toHaveBeenCalledWith("song", { ascending: true }); - }); - - it("stops after one page when the catalog fits in a single page", async () => { - mockPages([{ data: [row(1), row(2)], error: null }]); - - await selectCatalogSongTitles("cat_small"); - - expect(supabase.from).toHaveBeenCalledTimes(1); - }); - - it("returns [] on error", async () => { - const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}); - mockPages([{ data: null, error: { message: "boom" } }]); - - expect(await selectCatalogSongTitles("cat_1")).toEqual([]); - consoleError.mockRestore(); - }); -}); diff --git a/lib/supabase/catalog_songs/selectCatalogSongTitles.ts b/lib/supabase/catalog_songs/selectCatalogSongTitles.ts deleted file mode 100644 index aec557b1f..000000000 --- a/lib/supabase/catalog_songs/selectCatalogSongTitles.ts +++ /dev/null @@ -1,36 +0,0 @@ -import supabase from "../serverClient"; - -export type CatalogSongTitle = { isrc: string; title: string | null }; - -const PAGE_SIZE = 1000; - -/** - * Select the ISRC + title of every song in a catalog. Lighter than - * selectCatalogSongsWithArtists: no artists join — used by reads that only - * need the tracklist (e.g. catalog measurements). Paginates past the - * Supabase 1,000-row default to exhaustion so large catalogs are complete. - * - * @param catalogId - The catalog to list songs for - * @returns ISRC + title pairs, or [] if none exist or on error - */ -export async function selectCatalogSongTitles(catalogId: string): Promise { - const all: CatalogSongTitle[] = []; - - for (let from = 0; ; from += PAGE_SIZE) { - const { data, error } = await supabase - .from("catalog_songs") - .select("songs!inner (isrc, name)") - .eq("catalog", catalogId) - .order("song", { ascending: true }) - .range(from, from + PAGE_SIZE - 1); - - if (error) { - console.error("Error fetching catalog_songs:", error); - return []; - } - - const rows = data ?? []; - all.push(...rows.map(row => ({ isrc: row.songs.isrc, title: row.songs.name }))); - if (rows.length < PAGE_SIZE) return all; - } -} diff --git a/lib/supabase/song_artists/__tests__/selectSongArtistIsrcs.test.ts b/lib/supabase/song_artists/__tests__/selectSongArtistIsrcs.test.ts deleted file mode 100644 index 0afd9c272..000000000 --- a/lib/supabase/song_artists/__tests__/selectSongArtistIsrcs.test.ts +++ /dev/null @@ -1,68 +0,0 @@ -import { describe, it, expect, vi, beforeEach } from "vitest"; -import { selectSongArtistIsrcs } from "../selectSongArtistIsrcs"; -import supabase from "../../serverClient"; - -vi.mock("../../serverClient", () => { - const mockFrom = vi.fn(); - return { default: { from: mockFrom } }; -}); - -type Result = { data: unknown; error: unknown }; - -function makeBuilder(result: Result) { - const builder: Record> & { - then?: (resolve: (v: unknown) => void) => void; - } = {} as never; - for (const m of ["select", "eq", "order", "range"]) builder[m] = vi.fn().mockReturnValue(builder); - builder.then = resolve => resolve(result); - return builder; -} - -/** Queue one builder per supabase.from() call (one per page). */ -function mockPages(results: Result[]) { - const builders = results.map(makeBuilder); - let call = 0; - vi.mocked(supabase.from).mockImplementation(() => builders[call++] as never); - return builders; -} - -const artistAccountId = "b1814076-8e19-4a77-9dea-2ec150e26aaa"; - -describe("selectSongArtistIsrcs", () => { - beforeEach(() => vi.clearAllMocks()); - - it("returns the ISRCs linked to the artist account", async () => { - const builders = mockPages([{ data: [{ song: "ISRC1" }, { song: "ISRC2" }], error: null }]); - - const result = await selectSongArtistIsrcs(artistAccountId); - - expect(supabase.from).toHaveBeenCalledWith("song_artists"); - expect(builders[0].eq).toHaveBeenCalledWith("artist", artistAccountId); - expect(result).toEqual(["ISRC1", "ISRC2"]); - }); - - it("paginates past the 1,000-row page size to exhaustion", async () => { - const pageOne = Array.from({ length: 1000 }, (_, i) => ({ song: `ISRC${i}` })); - const pageTwo = Array.from({ length: 200 }, (_, i) => ({ song: `ISRC${1000 + i}` })); - const builders = mockPages([ - { data: pageOne, error: null }, - { data: pageTwo, error: null }, - ]); - - const result = await selectSongArtistIsrcs(artistAccountId); - - expect(result).toHaveLength(1200); - expect(builders[0].range).toHaveBeenCalledWith(0, 999); - expect(builders[1].range).toHaveBeenCalledWith(1000, 1999); - // deterministic pagination requires a stable order - expect(builders[0].order).toHaveBeenCalledWith("song", { ascending: true }); - }); - - it("returns [] on error", async () => { - const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}); - mockPages([{ data: null, error: { message: "boom" } }]); - - expect(await selectSongArtistIsrcs(artistAccountId)).toEqual([]); - consoleError.mockRestore(); - }); -}); diff --git a/lib/supabase/song_artists/selectSongArtistIsrcs.ts b/lib/supabase/song_artists/selectSongArtistIsrcs.ts deleted file mode 100644 index 4f632cf3a..000000000 --- a/lib/supabase/song_artists/selectSongArtistIsrcs.ts +++ /dev/null @@ -1,34 +0,0 @@ -import supabase from "../serverClient"; - -const PAGE_SIZE = 1000; - -/** - * Select the ISRC of every song linked to an artist account via - * song_artists. Paginates past the Supabase 1,000-row default to - * exhaustion so large discographies are complete — used to scope catalog - * measurement reads to one artist. - * - * @param artistAccountId - The artist account to list linked songs for - * @returns Song ISRCs, or [] if none exist or on error - */ -export async function selectSongArtistIsrcs(artistAccountId: string): Promise { - const all: string[] = []; - - for (let from = 0; ; from += PAGE_SIZE) { - const { data, error } = await supabase - .from("song_artists") - .select("song") - .eq("artist", artistAccountId) - .order("song", { ascending: true }) - .range(from, from + PAGE_SIZE - 1); - - if (error) { - console.error("Error fetching song_artists:", error); - return []; - } - - const rows = data ?? []; - all.push(...rows.map(row => row.song)); - if (rows.length < PAGE_SIZE) return all; - } -} diff --git a/lib/supabase/song_measurements/__tests__/selectAllSongMeasurements.test.ts b/lib/supabase/song_measurements/__tests__/selectAllSongMeasurements.test.ts deleted file mode 100644 index abebd83c5..000000000 --- a/lib/supabase/song_measurements/__tests__/selectAllSongMeasurements.test.ts +++ /dev/null @@ -1,102 +0,0 @@ -import { describe, it, expect, vi, beforeEach } from "vitest"; -import { selectAllSongMeasurements } from "../selectAllSongMeasurements"; -import supabase from "../../serverClient"; - -vi.mock("../../serverClient", () => { - const mockFrom = vi.fn(); - return { default: { from: mockFrom } }; -}); - -type Result = { data: unknown; error: unknown }; - -function makeBuilder(result: Result) { - const builder: Record> & { - then?: (resolve: (v: unknown) => void) => void; - } = {} as never; - for (const m of ["select", "eq", "in", "order", "range"]) - builder[m] = vi.fn().mockReturnValue(builder); - builder.then = resolve => resolve(result); - return builder; -} - -/** Queue one builder per supabase.from() call (one per page per chunk). */ -function mockPages(results: Result[]) { - const builders = results.map(makeBuilder); - let call = 0; - vi.mocked(supabase.from).mockImplementation(() => builders[call++] as never); - return builders; -} - -const row = (song: string, value: number, capturedAt = "2026-07-01T00:00:00Z") => ({ - song, - value, - captured_at: capturedAt, - platform: "spotify", - metric: "platform_displayed_play_count", -}); - -const params = { platform: "spotify", metric: "platform_displayed_play_count" }; - -describe("selectAllSongMeasurements", () => { - beforeEach(() => vi.clearAllMocks()); - - it("returns [] without querying when songs is empty", async () => { - expect(await selectAllSongMeasurements({ songs: [], ...params })).toEqual([]); - expect(supabase.from).not.toHaveBeenCalled(); - }); - - it("reads a small batch in one query, newest-first with filters applied", async () => { - const builders = mockPages([{ data: [row("ISRC1", 200), row("ISRC2", 50)], error: null }]); - - const result = await selectAllSongMeasurements({ songs: ["ISRC1", "ISRC2"], ...params }); - - expect(supabase.from).toHaveBeenCalledWith("song_measurements"); - expect(builders[0].in).toHaveBeenCalledWith("song", ["ISRC1", "ISRC2"]); - expect(builders[0].eq).toHaveBeenCalledWith("platform", "spotify"); - expect(builders[0].eq).toHaveBeenCalledWith("metric", "platform_displayed_play_count"); - expect(builders[0].order).toHaveBeenCalledWith("captured_at", { ascending: false }); - expect(builders[0].range).toHaveBeenCalledWith(0, 999); - expect(result).toEqual([row("ISRC1", 200), row("ISRC2", 50)]); - }); - - it("paginates each read past the 1,000-row page size to exhaustion", async () => { - const pageOne = Array.from({ length: 1000 }, (_, i) => row(`ISRC${i}`, i)); - const pageTwo = Array.from({ length: 300 }, (_, i) => row(`ISRC${1000 + i}`, i)); - const builders = mockPages([ - { data: pageOne, error: null }, - { data: pageTwo, error: null }, - ]); - - const songs = Array.from({ length: 400 }, (_, i) => `ISRC${i}`); - const result = await selectAllSongMeasurements({ songs, ...params }); - - expect(result).toHaveLength(1300); - expect(builders[0].range).toHaveBeenCalledWith(0, 999); - expect(builders[1].range).toHaveBeenCalledWith(1000, 1999); - }); - - it("chunks large ISRC lists so the IN clause stays bounded", async () => { - const songs = Array.from({ length: 1200 }, (_, i) => `ISRC${i}`); - const builders = mockPages([ - { data: [row("ISRC0", 1)], error: null }, - { data: [row("ISRC500", 2)], error: null }, - { data: [row("ISRC1000", 3)], error: null }, - ]); - - const result = await selectAllSongMeasurements({ songs, ...params }); - - expect(supabase.from).toHaveBeenCalledTimes(3); - expect(builders[0].in).toHaveBeenCalledWith("song", songs.slice(0, 500)); - expect(builders[1].in).toHaveBeenCalledWith("song", songs.slice(500, 1000)); - expect(builders[2].in).toHaveBeenCalledWith("song", songs.slice(1000)); - expect(result).toEqual([row("ISRC0", 1), row("ISRC500", 2), row("ISRC1000", 3)]); - }); - - it("returns [] on error", async () => { - const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}); - mockPages([{ data: null, error: { message: "boom" } }]); - - expect(await selectAllSongMeasurements({ songs: ["ISRC1"], ...params })).toEqual([]); - consoleError.mockRestore(); - }); -}); diff --git a/lib/supabase/song_measurements/__tests__/selectCatalogMeasurementsAggregate.test.ts b/lib/supabase/song_measurements/__tests__/selectCatalogMeasurementsAggregate.test.ts new file mode 100644 index 000000000..377eb0e4a --- /dev/null +++ b/lib/supabase/song_measurements/__tests__/selectCatalogMeasurementsAggregate.test.ts @@ -0,0 +1,74 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { selectCatalogMeasurementsAggregate } from "../selectCatalogMeasurementsAggregate"; +import supabase from "../../serverClient"; + +vi.mock("../../serverClient", () => ({ + default: { rpc: vi.fn() }, +})); + +const catalogId = "7d3e5c35-1fac-4b88-aa90-e2e4a52dfe78"; +const artistAccountId = "b1814076-8e19-4a77-9dea-2ec150e26aaa"; + +describe("selectCatalogMeasurementsAggregate", () => { + beforeEach(() => vi.clearAllMocks()); + + it("returns the whole-scope aggregate for a catalog", async () => { + vi.mocked(supabase.rpc).mockResolvedValue({ + data: [{ measured_song_count: 2679, total_streams: 16308837441 }], + error: null, + } as never); + + const result = await selectCatalogMeasurementsAggregate({ catalogId }); + + expect(supabase.rpc).toHaveBeenCalledWith("get_catalog_measurements_aggregate", { + p_catalog: catalogId, + }); + expect(result).toEqual({ measuredSongCount: 2679, totalStreams: 16308837441 }); + }); + + it("passes the artist filter through when scoped", async () => { + vi.mocked(supabase.rpc).mockResolvedValue({ + data: [{ measured_song_count: 322, total_streams: 1799402184 }], + error: null, + } as never); + + const result = await selectCatalogMeasurementsAggregate({ catalogId, artistAccountId }); + + expect(supabase.rpc).toHaveBeenCalledWith("get_catalog_measurements_aggregate", { + p_catalog: catalogId, + p_artist: artistAccountId, + }); + expect(result).toEqual({ measuredSongCount: 322, totalStreams: 1799402184 }); + }); + + it("coerces numeric strings from PostgREST", async () => { + vi.mocked(supabase.rpc).mockResolvedValue({ + data: [{ measured_song_count: 40, total_streams: "21325663" }], + error: null, + } as never); + + const result = await selectCatalogMeasurementsAggregate({ catalogId }); + + expect(result).toEqual({ measuredSongCount: 40, totalStreams: 21325663 }); + }); + + it("returns a zero aggregate when the rpc yields no row", async () => { + vi.mocked(supabase.rpc).mockResolvedValue({ data: [], error: null } as never); + + expect(await selectCatalogMeasurementsAggregate({ catalogId })).toEqual({ + measuredSongCount: 0, + totalStreams: 0, + }); + }); + + it("returns null on error", async () => { + const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}); + vi.mocked(supabase.rpc).mockResolvedValue({ + data: null, + error: { message: "boom" }, + } as never); + + expect(await selectCatalogMeasurementsAggregate({ catalogId })).toBeNull(); + consoleError.mockRestore(); + }); +}); diff --git a/lib/supabase/song_measurements/__tests__/selectCatalogMeasurementsPage.test.ts b/lib/supabase/song_measurements/__tests__/selectCatalogMeasurementsPage.test.ts new file mode 100644 index 000000000..0cf70d441 --- /dev/null +++ b/lib/supabase/song_measurements/__tests__/selectCatalogMeasurementsPage.test.ts @@ -0,0 +1,64 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { selectCatalogMeasurementsPage } from "../selectCatalogMeasurementsPage"; +import supabase from "../../serverClient"; + +vi.mock("../../serverClient", () => ({ + default: { rpc: vi.fn() }, +})); + +const catalogId = "7d3e5c35-1fac-4b88-aa90-e2e4a52dfe78"; +const artistAccountId = "b1814076-8e19-4a77-9dea-2ec150e26aaa"; + +const row = { + isrc: "USUYG1075006", + title: "Suavemente", + playcount: 717012221, + measured_at: "2026-07-06T00:00:00+00:00", +}; + +describe("selectCatalogMeasurementsPage", () => { + beforeEach(() => vi.clearAllMocks()); + + it("reads one page with limit/offset derived from page/limit", async () => { + vi.mocked(supabase.rpc).mockResolvedValue({ data: [row], error: null } as never); + + const result = await selectCatalogMeasurementsPage({ catalogId, page: 3, limit: 50 }); + + expect(supabase.rpc).toHaveBeenCalledWith("get_catalog_measurements_page", { + p_catalog: catalogId, + p_limit: 50, + p_offset: 100, + }); + expect(result).toEqual([row]); + }); + + it("passes the artist filter through when scoped", async () => { + vi.mocked(supabase.rpc).mockResolvedValue({ data: [row], error: null } as never); + + await selectCatalogMeasurementsPage({ catalogId, artistAccountId, page: 1, limit: 20 }); + + expect(supabase.rpc).toHaveBeenCalledWith("get_catalog_measurements_page", { + p_catalog: catalogId, + p_artist: artistAccountId, + p_limit: 20, + p_offset: 0, + }); + }); + + it("returns [] when the page is past the end", async () => { + vi.mocked(supabase.rpc).mockResolvedValue({ data: [], error: null } as never); + + expect(await selectCatalogMeasurementsPage({ catalogId, page: 99, limit: 50 })).toEqual([]); + }); + + it("returns null on error", async () => { + const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}); + vi.mocked(supabase.rpc).mockResolvedValue({ + data: null, + error: { message: "boom" }, + } as never); + + expect(await selectCatalogMeasurementsPage({ catalogId, page: 1, limit: 50 })).toBeNull(); + consoleError.mockRestore(); + }); +}); diff --git a/lib/supabase/song_measurements/selectAllSongMeasurements.ts b/lib/supabase/song_measurements/selectAllSongMeasurements.ts deleted file mode 100644 index c99aa4515..000000000 --- a/lib/supabase/song_measurements/selectAllSongMeasurements.ts +++ /dev/null @@ -1,58 +0,0 @@ -import supabase from "../serverClient"; -import { Tables } from "@/types/database.types"; - -const CHUNK_SIZE = 500; // keeps the PostgREST IN clause well under URL length limits -const PAGE_SIZE = 1000; // Supabase default row cap — paginate past it explicitly - -/** - * Select every measurement for a batch of songs, newest-first per read. - * Unlike selectSongMeasurements this read is exhaustive: the ISRC list is - * chunked (bounded IN clause) and each chunk is paginated with .range() - * past the Supabase 1,000-row default, so no rows are silently dropped — - * required for catalog-scale reads (see api#757's silent 1,000-row cap). - * - * @param params.songs - Song ISRCs to read measurements for - * @param params.platform - Platform filter (e.g. "spotify") - * @param params.metric - Metric filter (e.g. "platform_displayed_play_count") - * @returns All measurement rows (newest-first within each chunk), or [] if none exist or on error - */ -export async function selectAllSongMeasurements({ - songs, - platform, - metric, -}: { - songs: string[]; - platform: string; - metric: string; -}): Promise[]> { - if (songs.length === 0) return []; - - const all: Tables<"song_measurements">[] = []; - - for (let i = 0; i < songs.length; i += CHUNK_SIZE) { - const chunk = songs.slice(i, i + CHUNK_SIZE); - - for (let from = 0; ; from += PAGE_SIZE) { - const { data, error } = await supabase - .from("song_measurements") - .select("*") - .in("song", chunk) - .eq("platform", platform) - .eq("metric", metric) - .order("captured_at", { ascending: false }) - .order("id", { ascending: false }) - .range(from, from + PAGE_SIZE - 1); - - if (error) { - console.error("Error fetching song_measurements:", error); - return []; - } - - const rows = data ?? []; - all.push(...rows); - if (rows.length < PAGE_SIZE) break; - } - } - - return all; -} diff --git a/lib/supabase/song_measurements/selectCatalogMeasurementsAggregate.ts b/lib/supabase/song_measurements/selectCatalogMeasurementsAggregate.ts new file mode 100644 index 000000000..fe8affdfb --- /dev/null +++ b/lib/supabase/song_measurements/selectCatalogMeasurementsAggregate.ts @@ -0,0 +1,41 @@ +import supabase from "../serverClient"; + +export type CatalogMeasurementsAggregate = { + measuredSongCount: number; + totalStreams: number; +}; + +/** + * Whole-scope aggregate for a catalog's latest measurements via the + * get_catalog_measurements_aggregate RPC: count + total streams over the + * latest capture per ISRC, computed in a single SQL aggregate — no row cap, + * regardless of catalog size. Optionally scoped to one artist account + * (catalog_songs ∩ song_artists). + * + * @param params.catalogId - The catalog to aggregate measurements for + * @param params.artistAccountId - Optional artist account to scope the read to + * @returns The aggregate (zeros when nothing is measured), or null on error + */ +export async function selectCatalogMeasurementsAggregate({ + catalogId, + artistAccountId, +}: { + catalogId: string; + artistAccountId?: string; +}): Promise { + const { data, error } = await supabase.rpc("get_catalog_measurements_aggregate", { + p_catalog: catalogId, + ...(artistAccountId ? { p_artist: artistAccountId } : {}), + }); + + if (error) { + console.error("Error aggregating catalog measurements:", error); + return null; + } + + const row = data?.[0]; + return { + measuredSongCount: Number(row?.measured_song_count ?? 0), + totalStreams: Number(row?.total_streams ?? 0), + }; +} diff --git a/lib/supabase/song_measurements/selectCatalogMeasurementsPage.ts b/lib/supabase/song_measurements/selectCatalogMeasurementsPage.ts new file mode 100644 index 000000000..46820cf64 --- /dev/null +++ b/lib/supabase/song_measurements/selectCatalogMeasurementsPage.ts @@ -0,0 +1,47 @@ +import supabase from "../serverClient"; + +export type CatalogMeasurementRow = { + isrc: string; + title: string | null; + playcount: number; + measured_at: string; +}; + +/** + * One page of a catalog's latest-per-ISRC measurements via the + * get_catalog_measurements_page RPC, sorted by play count descending. + * Optionally scoped to one artist account (catalog_songs ∩ song_artists). + * Pagination only windows the rows — pair with + * selectCatalogMeasurementsAggregate for whole-scope totals. + * + * @param params.catalogId - The catalog to read measurements for + * @param params.artistAccountId - Optional artist account to scope the read to + * @param params.page - 1-based page number + * @param params.limit - Rows per page + * @returns The page's measurement rows ([] past the end), or null on error + */ +export async function selectCatalogMeasurementsPage({ + catalogId, + artistAccountId, + page, + limit, +}: { + catalogId: string; + artistAccountId?: string; + page: number; + limit: number; +}): Promise { + const { data, error } = await supabase.rpc("get_catalog_measurements_page", { + p_catalog: catalogId, + ...(artistAccountId ? { p_artist: artistAccountId } : {}), + p_limit: limit, + p_offset: (page - 1) * limit, + }); + + if (error) { + console.error("Error fetching catalog measurements page:", error); + return null; + } + + return data ?? []; +} diff --git a/types/database.types.ts b/types/database.types.ts index 45da666d6..5cdec74e6 100644 --- a/types/database.types.ts +++ b/types/database.types.ts @@ -3829,6 +3829,27 @@ export type Database = { Returns: undefined; }; extract_domain: { Args: { email: string }; Returns: string }; + get_catalog_measurements_aggregate: { + Args: { p_catalog: string; p_artist?: string }; + Returns: { + measured_song_count: number; + total_streams: number; + }[]; + }; + get_catalog_measurements_page: { + Args: { + p_catalog: string; + p_artist?: string; + p_limit?: number; + p_offset?: number; + }; + Returns: { + isrc: string; + title: string | null; + playcount: number; + measured_at: string; + }[]; + }; get_account_invitations: { Args: { account_slug: string }; Returns: { From 0e358f04a7a19d0af79ba5a5c26999808e664bf9 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Tue, 7 Jul 2026 07:55:33 -0500 Subject: [PATCH 3/4] =?UTF-8?q?refactor(catalogs):=20catalogId=20moves=20t?= =?UTF-8?q?o=20the=20path=20=E2=80=94=20GET=20/api/catalogs/[catalogId]/me?= =?UTF-8?q?asurements=20(chat#1850)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit REST amendment on chat#1850: path for identity, query for modifiers — mirrors the research measurements family (app/api/research/albums/[id]/measurements). The route becomes a dynamic segment; the validator takes the path id (uuid, 400 on malformed; a catalogId smuggled into the query string is ignored) and the query carries only artist_account_id/page/limit. A missing id no longer routes (404 from Next), so the 'catalogId is required' 400 is gone. Co-Authored-By: Claude Fable 5 --- .../[catalogId]/measurements/route.ts | 33 ++++++++++++++++ app/api/catalogs/measurements/route.ts | 26 ------------- .../getCatalogMeasurementsHandler.test.ts | 24 +++++++----- ...alidateGetCatalogMeasurementsQuery.test.ts | 39 ++++++++++--------- lib/catalog/getCatalogMeasurementsHandler.ts | 10 +++-- .../validateGetCatalogMeasurementsQuery.ts | 16 +++++--- 6 files changed, 86 insertions(+), 62 deletions(-) create mode 100644 app/api/catalogs/[catalogId]/measurements/route.ts delete mode 100644 app/api/catalogs/measurements/route.ts diff --git a/app/api/catalogs/[catalogId]/measurements/route.ts b/app/api/catalogs/[catalogId]/measurements/route.ts new file mode 100644 index 000000000..4108d2436 --- /dev/null +++ b/app/api/catalogs/[catalogId]/measurements/route.ts @@ -0,0 +1,33 @@ +import { NextRequest, NextResponse } from "next/server"; +import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; +import { getCatalogMeasurementsHandler } from "@/lib/catalog/getCatalogMeasurementsHandler"; + +/** + * OPTIONS /api/catalogs/{catalogId}/measurements — CORS preflight. + * + * @returns A NextResponse with CORS headers. + */ +export async function OPTIONS() { + return new NextResponse(null, { + status: 200, + headers: getCorsHeaders(), + }); +} + +/** + * GET /api/catalogs/{catalogId}/measurements — one page of a catalog's + * latest per-song play counts plus whole-scope aggregates and the derived + * valuation band. + * + * @param request - The request object. + * @param options - Route options containing params. + * @param options.params - Route params containing the catalogId. + * @returns A NextResponse with measurements, pagination, and aggregates. + */ +export async function GET( + request: NextRequest, + options: { params: Promise<{ catalogId: string }> }, +) { + const { catalogId } = await options.params; + return getCatalogMeasurementsHandler(request, catalogId); +} diff --git a/app/api/catalogs/measurements/route.ts b/app/api/catalogs/measurements/route.ts deleted file mode 100644 index 9ee1dfe97..000000000 --- a/app/api/catalogs/measurements/route.ts +++ /dev/null @@ -1,26 +0,0 @@ -import { NextRequest, NextResponse } from "next/server"; -import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; -import { getCatalogMeasurementsHandler } from "@/lib/catalog/getCatalogMeasurementsHandler"; - -/** - * OPTIONS handler for CORS preflight requests. - * - * @returns A NextResponse with CORS headers. - */ -export async function OPTIONS() { - return new NextResponse(null, { - status: 200, - headers: getCorsHeaders(), - }); -} - -/** - * GET handler for retrieving a catalog's latest per-song play counts and the - * derived valuation band. - * - * @param request - The request object containing the catalogId query parameter. - * @returns A NextResponse with measurements, valuation band, and totals. - */ -export async function GET(request: NextRequest) { - return getCatalogMeasurementsHandler(request); -} diff --git a/lib/catalog/__tests__/getCatalogMeasurementsHandler.test.ts b/lib/catalog/__tests__/getCatalogMeasurementsHandler.test.ts index d5743ef56..0707a2c4b 100644 --- a/lib/catalog/__tests__/getCatalogMeasurementsHandler.test.ts +++ b/lib/catalog/__tests__/getCatalogMeasurementsHandler.test.ts @@ -32,7 +32,7 @@ const catalogId = "740d5050-40ec-4892-a040-b78bb50fef2f"; const artistAccountId = "b1814076-8e19-4a77-9dea-2ec150e26aaa"; const makeRequest = () => - new NextRequest(`http://localhost/api/catalogs/measurements?catalogId=${catalogId}`); + new NextRequest(`http://localhost/api/catalogs/${catalogId}/measurements`); const okAuth = () => vi.mocked(validateAuthContext).mockResolvedValue({ @@ -62,7 +62,7 @@ describe("getCatalogMeasurementsHandler", () => { const err = NextResponse.json({ status: "error" }, { status: 400 }); vi.mocked(validateGetCatalogMeasurementsQuery).mockReturnValue(err); - const res = await getCatalogMeasurementsHandler(makeRequest()); + const res = await getCatalogMeasurementsHandler(makeRequest(), catalogId); expect(res).toBe(err); expect(validateAuthContext).not.toHaveBeenCalled(); @@ -73,7 +73,7 @@ describe("getCatalogMeasurementsHandler", () => { const authErr = NextResponse.json({ status: "error" }, { status: 401 }); vi.mocked(validateAuthContext).mockResolvedValue(authErr); - const res = await getCatalogMeasurementsHandler(makeRequest()); + const res = await getCatalogMeasurementsHandler(makeRequest(), catalogId); expect(res).toBe(authErr); expect(selectAccountCatalog).not.toHaveBeenCalled(); @@ -84,7 +84,7 @@ describe("getCatalogMeasurementsHandler", () => { okAuth(); vi.mocked(selectAccountCatalog).mockResolvedValue(null); - const res = await getCatalogMeasurementsHandler(makeRequest()); + const res = await getCatalogMeasurementsHandler(makeRequest(), catalogId); expect(res.status).toBe(404); expect(selectAccountCatalog).toHaveBeenCalledWith({ accountId, catalogId }); @@ -104,10 +104,14 @@ describe("getCatalogMeasurementsHandler", () => { // 10 years of catalog age vi.mocked(getCatalogEarliestReleaseDate).mockResolvedValue("2016-07-01"); - const res = await getCatalogMeasurementsHandler(makeRequest()); + const res = await getCatalogMeasurementsHandler(makeRequest(), catalogId); const body = await res.json(); expect(res.status).toBe(200); + expect(validateGetCatalogMeasurementsQuery).toHaveBeenCalledWith( + expect.any(URLSearchParams), + catalogId, + ); expect(selectCatalogMeasurementsAggregate).toHaveBeenCalledWith({ catalogId, artistAccountId: undefined, @@ -148,7 +152,7 @@ describe("getCatalogMeasurementsHandler", () => { vi.mocked(selectCatalogMeasurementsPage).mockResolvedValue([pageRows[0]]); vi.mocked(getCatalogEarliestReleaseDate).mockResolvedValue("2016-07-01"); - const res = await getCatalogMeasurementsHandler(makeRequest()); + const res = await getCatalogMeasurementsHandler(makeRequest(), catalogId); const body = await res.json(); expect(res.status).toBe(200); @@ -180,7 +184,7 @@ describe("getCatalogMeasurementsHandler", () => { vi.mocked(selectCatalogMeasurementsPage).mockResolvedValue([]); vi.mocked(getCatalogEarliestReleaseDate).mockResolvedValue(null); - const res = await getCatalogMeasurementsHandler(makeRequest()); + const res = await getCatalogMeasurementsHandler(makeRequest(), catalogId); const body = await res.json(); expect(res.status).toBe(200); @@ -200,7 +204,7 @@ describe("getCatalogMeasurementsHandler", () => { vi.mocked(selectCatalogMeasurementsPage).mockResolvedValue(pageRows); vi.mocked(getCatalogEarliestReleaseDate).mockResolvedValue(null); - const res = await getCatalogMeasurementsHandler(makeRequest()); + const res = await getCatalogMeasurementsHandler(makeRequest(), catalogId); expect(res.status).toBe(500); }); @@ -216,7 +220,7 @@ describe("getCatalogMeasurementsHandler", () => { vi.mocked(selectCatalogMeasurementsPage).mockResolvedValue(null); vi.mocked(getCatalogEarliestReleaseDate).mockResolvedValue(null); - const res = await getCatalogMeasurementsHandler(makeRequest()); + const res = await getCatalogMeasurementsHandler(makeRequest(), catalogId); expect(res.status).toBe(500); }); @@ -227,7 +231,7 @@ describe("getCatalogMeasurementsHandler", () => { okAuth(); vi.mocked(selectAccountCatalog).mockRejectedValue(new Error("boom")); - const res = await getCatalogMeasurementsHandler(makeRequest()); + const res = await getCatalogMeasurementsHandler(makeRequest(), catalogId); expect(res.status).toBe(500); consoleError.mockRestore(); diff --git a/lib/catalog/__tests__/validateGetCatalogMeasurementsQuery.test.ts b/lib/catalog/__tests__/validateGetCatalogMeasurementsQuery.test.ts index bf88296b9..3762d2aa3 100644 --- a/lib/catalog/__tests__/validateGetCatalogMeasurementsQuery.test.ts +++ b/lib/catalog/__tests__/validateGetCatalogMeasurementsQuery.test.ts @@ -9,14 +9,14 @@ vi.mock("@/lib/networking/getCorsHeaders", () => ({ const catalogId = "740d5050-40ec-4892-a040-b78bb50fef2f"; describe("validateGetCatalogMeasurementsQuery", () => { - it("returns the validated query for a valid catalogId with default pagination", () => { - const result = validateGetCatalogMeasurementsQuery(new URLSearchParams({ catalogId })); + it("returns the validated request for a valid path catalogId with default pagination", () => { + const result = validateGetCatalogMeasurementsQuery(new URLSearchParams(), catalogId); expect(result).toEqual({ catalogId, page: 1, limit: 50 }); }); - it("returns 400 when catalogId is missing", async () => { - const result = validateGetCatalogMeasurementsQuery(new URLSearchParams()); + it("returns 400 when the path catalogId is not a uuid", async () => { + const result = validateGetCatalogMeasurementsQuery(new URLSearchParams(), "not-a-uuid"); expect(result).toBeInstanceOf(NextResponse); expect((result as NextResponse).status).toBe(400); @@ -24,19 +24,11 @@ describe("validateGetCatalogMeasurementsQuery", () => { expect(body.status).toBe("error"); }); - it("returns 400 when catalogId is not a uuid", () => { - const result = validateGetCatalogMeasurementsQuery( - new URLSearchParams({ catalogId: "not-a-uuid" }), - ); - - expect(result).toBeInstanceOf(NextResponse); - expect((result as NextResponse).status).toBe(400); - }); - it("accepts an optional artist_account_id uuid", () => { const artistAccountId = "b1814076-8e19-4a77-9dea-2ec150e26aaa"; const result = validateGetCatalogMeasurementsQuery( - new URLSearchParams({ catalogId, artist_account_id: artistAccountId }), + new URLSearchParams({ artist_account_id: artistAccountId }), + catalogId, ); expect(result).toEqual({ @@ -49,7 +41,8 @@ describe("validateGetCatalogMeasurementsQuery", () => { it("returns 400 when artist_account_id is malformed", async () => { const result = validateGetCatalogMeasurementsQuery( - new URLSearchParams({ catalogId, artist_account_id: "not-a-uuid" }), + new URLSearchParams({ artist_account_id: "not-a-uuid" }), + catalogId, ); expect(result).toBeInstanceOf(NextResponse); @@ -60,7 +53,8 @@ describe("validateGetCatalogMeasurementsQuery", () => { it("accepts explicit page and limit", () => { const result = validateGetCatalogMeasurementsQuery( - new URLSearchParams({ catalogId, page: "3", limit: "100" }), + new URLSearchParams({ page: "3", limit: "100" }), + catalogId, ); expect(result).toEqual({ catalogId, page: 3, limit: 100 }); @@ -68,7 +62,7 @@ describe("validateGetCatalogMeasurementsQuery", () => { it("returns 400 when page is not a positive integer", () => { for (const page of ["0", "-1", "abc", "1.5"]) { - const result = validateGetCatalogMeasurementsQuery(new URLSearchParams({ catalogId, page })); + const result = validateGetCatalogMeasurementsQuery(new URLSearchParams({ page }), catalogId); expect(result).toBeInstanceOf(NextResponse); expect((result as NextResponse).status).toBe(400); } @@ -76,9 +70,18 @@ describe("validateGetCatalogMeasurementsQuery", () => { it("returns 400 when limit is out of range", () => { for (const limit of ["0", "101", "abc"]) { - const result = validateGetCatalogMeasurementsQuery(new URLSearchParams({ catalogId, limit })); + const result = validateGetCatalogMeasurementsQuery(new URLSearchParams({ limit }), catalogId); expect(result).toBeInstanceOf(NextResponse); expect((result as NextResponse).status).toBe(400); } }); + + it("ignores a catalogId smuggled into the query string in favor of the path", () => { + const result = validateGetCatalogMeasurementsQuery( + new URLSearchParams({ catalogId: "b1814076-8e19-4a77-9dea-2ec150e26aaa" }), + catalogId, + ); + + expect(result).toEqual({ catalogId, page: 1, limit: 50 }); + }); }); diff --git a/lib/catalog/getCatalogMeasurementsHandler.ts b/lib/catalog/getCatalogMeasurementsHandler.ts index aa7fcec70..76c8056e5 100644 --- a/lib/catalog/getCatalogMeasurementsHandler.ts +++ b/lib/catalog/getCatalogMeasurementsHandler.ts @@ -10,7 +10,7 @@ import { selectCatalogMeasurementsAggregate } from "@/lib/supabase/song_measurem import { selectCatalogMeasurementsPage } from "@/lib/supabase/song_measurements/selectCatalogMeasurementsPage"; /** - * GET /api/catalogs/measurements?catalogId=&artist_account_id=&page=&limit= + * GET /api/catalogs/{catalogId}/measurements?artist_account_id=&page=&limit= * * One page of the latest Spotify play counts per song (ISRC) in a catalog * plus whole-scope aggregates: measured_song_count, total_streams and a @@ -24,12 +24,16 @@ import { selectCatalogMeasurementsPage } from "@/lib/supabase/song_measurements/ * is a 404. * * @param request - The request object + * @param catalogIdParam - The catalogId path segment * @returns `{ status, measurements, pagination, measured_song_count, total_streams, valuation, artist_account_id, catalog_age_years }` */ -export async function getCatalogMeasurementsHandler(request: NextRequest): Promise { +export async function getCatalogMeasurementsHandler( + request: NextRequest, + catalogIdParam: string, +): Promise { try { const { searchParams } = new URL(request.url); - const validated = validateGetCatalogMeasurementsQuery(searchParams); + const validated = validateGetCatalogMeasurementsQuery(searchParams, catalogIdParam); if (validated instanceof NextResponse) { return validated; } diff --git a/lib/catalog/validateGetCatalogMeasurementsQuery.ts b/lib/catalog/validateGetCatalogMeasurementsQuery.ts index 4113dd946..7559ff1cd 100644 --- a/lib/catalog/validateGetCatalogMeasurementsQuery.ts +++ b/lib/catalog/validateGetCatalogMeasurementsQuery.ts @@ -30,17 +30,23 @@ export const getCatalogMeasurementsQuerySchema = z.object({ export type GetCatalogMeasurementsQuery = z.infer; /** - * Validates query parameters for GET /api/catalogs/measurements. + * Validates GET /api/catalogs/{catalogId}/measurements: the catalogId path + * segment (uuid) plus the optional query modifiers (artist_account_id, + * page, limit). The path id always wins — a catalogId smuggled into the + * query string is ignored. * * @param searchParams - The URL search parameters to validate. - * @returns A NextResponse with an error if validation fails, or the validated query. + * @param catalogId - The catalogId path segment. + * @returns A NextResponse with an error if validation fails, or the validated request. */ export function validateGetCatalogMeasurementsQuery( searchParams: URLSearchParams, + catalogId: string, ): NextResponse | GetCatalogMeasurementsQuery { - const result = getCatalogMeasurementsQuerySchema.safeParse( - Object.fromEntries(searchParams.entries()), - ); + const result = getCatalogMeasurementsQuerySchema.safeParse({ + ...Object.fromEntries(searchParams.entries()), + catalogId, + }); if (!result.success) { const firstError = result.error.issues[0]; From cfed6de49cf868b26561fd046536a4f75ce87adb Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Tue, 7 Jul 2026 08:15:39 -0500 Subject: [PATCH 4/4] refactor(catalogs): move auth into validateGetCatalogMeasurementsQuery (SRP) Review feedback on #763: the handler was doing auth validation itself. The validator now owns both auth and input validation (auth first), matching the measurements-family validator convention (validateGetResearchTrackHistoricStatsRequest et al) and returns { accountId, ...params }. Behavior note: an unauthenticated request with malformed params now 401s before the 400 param check, same as the research siblings. Co-Authored-By: Claude Fable 5 --- .../getCatalogMeasurementsHandler.test.ts | 38 ++------- ...alidateGetCatalogMeasurementsQuery.test.ts | 78 +++++++++++++------ lib/catalog/getCatalogMeasurementsHandler.ts | 15 +--- .../validateGetCatalogMeasurementsQuery.ts | 31 +++++--- 4 files changed, 86 insertions(+), 76 deletions(-) diff --git a/lib/catalog/__tests__/getCatalogMeasurementsHandler.test.ts b/lib/catalog/__tests__/getCatalogMeasurementsHandler.test.ts index 0707a2c4b..369f1de67 100644 --- a/lib/catalog/__tests__/getCatalogMeasurementsHandler.test.ts +++ b/lib/catalog/__tests__/getCatalogMeasurementsHandler.test.ts @@ -4,7 +4,6 @@ import { NextRequest, NextResponse } from "next/server"; import { getCatalogMeasurementsHandler } from "../getCatalogMeasurementsHandler"; import { validateGetCatalogMeasurementsQuery } from "../validateGetCatalogMeasurementsQuery"; import { getCatalogEarliestReleaseDate } from "../getCatalogEarliestReleaseDate"; -import { validateAuthContext } from "@/lib/auth/validateAuthContext"; import { selectAccountCatalog } from "@/lib/supabase/account_catalogs/selectAccountCatalog"; import { selectCatalogMeasurementsAggregate } from "@/lib/supabase/song_measurements/selectCatalogMeasurementsAggregate"; import { selectCatalogMeasurementsPage } from "@/lib/supabase/song_measurements/selectCatalogMeasurementsPage"; @@ -16,7 +15,6 @@ vi.mock("../validateGetCatalogMeasurementsQuery", () => ({ validateGetCatalogMeasurementsQuery: vi.fn(), })); vi.mock("../getCatalogEarliestReleaseDate", () => ({ getCatalogEarliestReleaseDate: vi.fn() })); -vi.mock("@/lib/auth/validateAuthContext", () => ({ validateAuthContext: vi.fn() })); vi.mock("@/lib/supabase/account_catalogs/selectAccountCatalog", () => ({ selectAccountCatalog: vi.fn(), })); @@ -34,16 +32,12 @@ const artistAccountId = "b1814076-8e19-4a77-9dea-2ec150e26aaa"; const makeRequest = () => new NextRequest(`http://localhost/api/catalogs/${catalogId}/measurements`); -const okAuth = () => - vi.mocked(validateAuthContext).mockResolvedValue({ +const okQuery = (query: Record = { accountId, catalogId, page: 1, limit: 50 }) => + vi.mocked(validateGetCatalogMeasurementsQuery).mockResolvedValue({ accountId, - orgId: null, - authToken: "t", + ...query, } as never); -const okQuery = (query: Record = { catalogId, page: 1, limit: 50 }) => - vi.mocked(validateGetCatalogMeasurementsQuery).mockReturnValue(query as never); - const okCatalog = () => vi.mocked(selectAccountCatalog).mockResolvedValue({ account: accountId, @@ -58,30 +52,18 @@ const pageRows = [ describe("getCatalogMeasurementsHandler", () => { beforeEach(() => vi.clearAllMocks()); - it("short-circuits with the validator error and never authenticates", async () => { - const err = NextResponse.json({ status: "error" }, { status: 400 }); - vi.mocked(validateGetCatalogMeasurementsQuery).mockReturnValue(err); + it("short-circuits with the validator error (auth + params live in the validator)", async () => { + const err = NextResponse.json({ status: "error" }, { status: 401 }); + vi.mocked(validateGetCatalogMeasurementsQuery).mockResolvedValue(err); const res = await getCatalogMeasurementsHandler(makeRequest(), catalogId); expect(res).toBe(err); - expect(validateAuthContext).not.toHaveBeenCalled(); - }); - - it("short-circuits with the auth error when unauthenticated", async () => { - okQuery(); - const authErr = NextResponse.json({ status: "error" }, { status: 401 }); - vi.mocked(validateAuthContext).mockResolvedValue(authErr); - - const res = await getCatalogMeasurementsHandler(makeRequest(), catalogId); - - expect(res).toBe(authErr); expect(selectAccountCatalog).not.toHaveBeenCalled(); }); it("returns 404 when the catalog does not belong to the caller", async () => { okQuery(); - okAuth(); vi.mocked(selectAccountCatalog).mockResolvedValue(null); const res = await getCatalogMeasurementsHandler(makeRequest(), catalogId); @@ -93,7 +75,6 @@ describe("getCatalogMeasurementsHandler", () => { it("returns one page of measurements + whole-scope aggregates and the derived band", async () => { okQuery(); - okAuth(); okCatalog(); // whole-scope aggregate is bigger than the returned page vi.mocked(selectCatalogMeasurementsAggregate).mockResolvedValue({ @@ -109,7 +90,7 @@ describe("getCatalogMeasurementsHandler", () => { expect(res.status).toBe(200); expect(validateGetCatalogMeasurementsQuery).toHaveBeenCalledWith( - expect.any(URLSearchParams), + expect.any(NextRequest), catalogId, ); expect(selectCatalogMeasurementsAggregate).toHaveBeenCalledWith({ @@ -143,7 +124,6 @@ describe("getCatalogMeasurementsHandler", () => { it("scopes the read to the artist and echoes the applied filter", async () => { okQuery({ catalogId, artist_account_id: artistAccountId, page: 2, limit: 10 }); - okAuth(); okCatalog(); vi.mocked(selectCatalogMeasurementsAggregate).mockResolvedValue({ measuredSongCount: 11, @@ -175,7 +155,6 @@ describe("getCatalogMeasurementsHandler", () => { it("returns an empty page with a zero band when nothing is measured in scope", async () => { okQuery({ catalogId, artist_account_id: artistAccountId, page: 1, limit: 50 }); - okAuth(); okCatalog(); vi.mocked(selectCatalogMeasurementsAggregate).mockResolvedValue({ measuredSongCount: 0, @@ -198,7 +177,6 @@ describe("getCatalogMeasurementsHandler", () => { it("returns 500 when the aggregate read fails", async () => { okQuery(); - okAuth(); okCatalog(); vi.mocked(selectCatalogMeasurementsAggregate).mockResolvedValue(null); vi.mocked(selectCatalogMeasurementsPage).mockResolvedValue(pageRows); @@ -211,7 +189,6 @@ describe("getCatalogMeasurementsHandler", () => { it("returns 500 when the page read fails", async () => { okQuery(); - okAuth(); okCatalog(); vi.mocked(selectCatalogMeasurementsAggregate).mockResolvedValue({ measuredSongCount: 1, @@ -228,7 +205,6 @@ describe("getCatalogMeasurementsHandler", () => { it("returns 500 when a dependency throws", async () => { const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}); okQuery(); - okAuth(); vi.mocked(selectAccountCatalog).mockRejectedValue(new Error("boom")); const res = await getCatalogMeasurementsHandler(makeRequest(), catalogId); diff --git a/lib/catalog/__tests__/validateGetCatalogMeasurementsQuery.test.ts b/lib/catalog/__tests__/validateGetCatalogMeasurementsQuery.test.ts index 3762d2aa3..05669c109 100644 --- a/lib/catalog/__tests__/validateGetCatalogMeasurementsQuery.test.ts +++ b/lib/catalog/__tests__/validateGetCatalogMeasurementsQuery.test.ts @@ -1,22 +1,47 @@ -import { describe, it, expect, vi } from "vitest"; -import { NextResponse } from "next/server"; +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { NextRequest, NextResponse } from "next/server"; import { validateGetCatalogMeasurementsQuery } from "../validateGetCatalogMeasurementsQuery"; +import { validateAuthContext } from "@/lib/auth/validateAuthContext"; vi.mock("@/lib/networking/getCorsHeaders", () => ({ getCorsHeaders: vi.fn(() => ({ "Access-Control-Allow-Origin": "*" })), })); +vi.mock("@/lib/auth/validateAuthContext", () => ({ validateAuthContext: vi.fn() })); + const catalogId = "740d5050-40ec-4892-a040-b78bb50fef2f"; +const accountId = "fb678396-a68f-4294-ae50-b8cacf9ce77b"; + +const makeRequest = (query = "") => + new NextRequest(`http://localhost/api/catalogs/${catalogId}/measurements${query}`); + +beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId, + orgId: null, + authToken: "t", + } as never); +}); describe("validateGetCatalogMeasurementsQuery", () => { - it("returns the validated request for a valid path catalogId with default pagination", () => { - const result = validateGetCatalogMeasurementsQuery(new URLSearchParams(), catalogId); + it("returns the auth error before validating params when credentials are missing", async () => { + const authErr = NextResponse.json({ status: "error" }, { status: 401 }); + vi.mocked(validateAuthContext).mockResolvedValue(authErr as never); + + const result = await validateGetCatalogMeasurementsQuery(makeRequest(), "not-a-uuid"); + + expect(result).toBe(authErr); + }); + + it("returns the validated request with the caller accountId for a valid path catalogId", async () => { + const result = await validateGetCatalogMeasurementsQuery(makeRequest(), catalogId); - expect(result).toEqual({ catalogId, page: 1, limit: 50 }); + expect(result).toEqual({ accountId, catalogId, page: 1, limit: 50 }); }); it("returns 400 when the path catalogId is not a uuid", async () => { - const result = validateGetCatalogMeasurementsQuery(new URLSearchParams(), "not-a-uuid"); + const result = await validateGetCatalogMeasurementsQuery(makeRequest(), "not-a-uuid"); expect(result).toBeInstanceOf(NextResponse); expect((result as NextResponse).status).toBe(400); @@ -24,14 +49,15 @@ describe("validateGetCatalogMeasurementsQuery", () => { expect(body.status).toBe("error"); }); - it("accepts an optional artist_account_id uuid", () => { + it("accepts an optional artist_account_id uuid", async () => { const artistAccountId = "b1814076-8e19-4a77-9dea-2ec150e26aaa"; - const result = validateGetCatalogMeasurementsQuery( - new URLSearchParams({ artist_account_id: artistAccountId }), + const result = await validateGetCatalogMeasurementsQuery( + makeRequest(`?artist_account_id=${artistAccountId}`), catalogId, ); expect(result).toEqual({ + accountId, catalogId, artist_account_id: artistAccountId, page: 1, @@ -40,8 +66,8 @@ describe("validateGetCatalogMeasurementsQuery", () => { }); it("returns 400 when artist_account_id is malformed", async () => { - const result = validateGetCatalogMeasurementsQuery( - new URLSearchParams({ artist_account_id: "not-a-uuid" }), + const result = await validateGetCatalogMeasurementsQuery( + makeRequest("?artist_account_id=not-a-uuid"), catalogId, ); @@ -51,37 +77,43 @@ describe("validateGetCatalogMeasurementsQuery", () => { expect(body.status).toBe("error"); }); - it("accepts explicit page and limit", () => { - const result = validateGetCatalogMeasurementsQuery( - new URLSearchParams({ page: "3", limit: "100" }), + it("accepts explicit page and limit", async () => { + const result = await validateGetCatalogMeasurementsQuery( + makeRequest("?page=3&limit=100"), catalogId, ); - expect(result).toEqual({ catalogId, page: 3, limit: 100 }); + expect(result).toEqual({ accountId, catalogId, page: 3, limit: 100 }); }); - it("returns 400 when page is not a positive integer", () => { + it("returns 400 when page is not a positive integer", async () => { for (const page of ["0", "-1", "abc", "1.5"]) { - const result = validateGetCatalogMeasurementsQuery(new URLSearchParams({ page }), catalogId); + const result = await validateGetCatalogMeasurementsQuery( + makeRequest(`?page=${page}`), + catalogId, + ); expect(result).toBeInstanceOf(NextResponse); expect((result as NextResponse).status).toBe(400); } }); - it("returns 400 when limit is out of range", () => { + it("returns 400 when limit is out of range", async () => { for (const limit of ["0", "101", "abc"]) { - const result = validateGetCatalogMeasurementsQuery(new URLSearchParams({ limit }), catalogId); + const result = await validateGetCatalogMeasurementsQuery( + makeRequest(`?limit=${limit}`), + catalogId, + ); expect(result).toBeInstanceOf(NextResponse); expect((result as NextResponse).status).toBe(400); } }); - it("ignores a catalogId smuggled into the query string in favor of the path", () => { - const result = validateGetCatalogMeasurementsQuery( - new URLSearchParams({ catalogId: "b1814076-8e19-4a77-9dea-2ec150e26aaa" }), + it("ignores a catalogId smuggled into the query string in favor of the path", async () => { + const result = await validateGetCatalogMeasurementsQuery( + makeRequest("?catalogId=b1814076-8e19-4a77-9dea-2ec150e26aaa"), catalogId, ); - expect(result).toEqual({ catalogId, page: 1, limit: 50 }); + expect(result).toEqual({ accountId, catalogId, page: 1, limit: 50 }); }); }); diff --git a/lib/catalog/getCatalogMeasurementsHandler.ts b/lib/catalog/getCatalogMeasurementsHandler.ts index 76c8056e5..ab1b1498e 100644 --- a/lib/catalog/getCatalogMeasurementsHandler.ts +++ b/lib/catalog/getCatalogMeasurementsHandler.ts @@ -1,7 +1,6 @@ import { NextRequest, NextResponse } from "next/server"; import { errorResponse } from "@/lib/networking/errorResponse"; import { successResponse } from "@/lib/networking/successResponse"; -import { validateAuthContext } from "@/lib/auth/validateAuthContext"; import { validateGetCatalogMeasurementsQuery } from "./validateGetCatalogMeasurementsQuery"; import { computeValuationBand } from "./computeValuationBand"; import { getCatalogEarliestReleaseDate } from "./getCatalogEarliestReleaseDate"; @@ -16,7 +15,8 @@ import { selectCatalogMeasurementsPage } from "@/lib/supabase/song_measurements/ * plus whole-scope aggregates: measured_song_count, total_streams and a * valuation band derived at read time with the same model as the marketing * valuation card. The aggregates are computed in a single SQL aggregate over - * the entire scope — no row cap — regardless of the requested page. + * the entire scope — no row cap — regardless of the requested page. Auth and + * input validation both live in validateGetCatalogMeasurementsQuery (SRP). * Optionally scoped to one artist's songs (catalog_songs ∩ song_artists) via * artist_account_id; the applied filter is echoed back so clients can verify * the response scope. The account is resolved from credentials (Privy bearer @@ -32,18 +32,11 @@ export async function getCatalogMeasurementsHandler( catalogIdParam: string, ): Promise { try { - const { searchParams } = new URL(request.url); - const validated = validateGetCatalogMeasurementsQuery(searchParams, catalogIdParam); + const validated = await validateGetCatalogMeasurementsQuery(request, catalogIdParam); if (validated instanceof NextResponse) { return validated; } - - const authResult = await validateAuthContext(request); - if (authResult instanceof NextResponse) { - return authResult; - } - const { accountId } = authResult; - const { catalogId, artist_account_id: artistAccountId, page, limit } = validated; + const { accountId, catalogId, artist_account_id: artistAccountId, page, limit } = validated; const link = await selectAccountCatalog({ accountId, catalogId }); if (!link) { diff --git a/lib/catalog/validateGetCatalogMeasurementsQuery.ts b/lib/catalog/validateGetCatalogMeasurementsQuery.ts index 7559ff1cd..c13d4f843 100644 --- a/lib/catalog/validateGetCatalogMeasurementsQuery.ts +++ b/lib/catalog/validateGetCatalogMeasurementsQuery.ts @@ -1,5 +1,6 @@ -import { NextResponse } from "next/server"; +import { type NextRequest, NextResponse } from "next/server"; import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; +import { validateAuthContext } from "@/lib/auth/validateAuthContext"; import { z } from "zod"; export const getCatalogMeasurementsQuerySchema = z.object({ @@ -27,22 +28,30 @@ export const getCatalogMeasurementsQuerySchema = z.object({ ), }); -export type GetCatalogMeasurementsQuery = z.infer; +export type GetCatalogMeasurementsQuery = z.infer & { + accountId: string; +}; /** - * Validates GET /api/catalogs/{catalogId}/measurements: the catalogId path - * segment (uuid) plus the optional query modifiers (artist_account_id, - * page, limit). The path id always wins — a catalogId smuggled into the - * query string is ignored. + * Validates GET /api/catalogs/{catalogId}/measurements — auth (Privy bearer + * or x-api-key, resolved to the caller's accountId), the catalogId path + * segment (uuid), and the optional query modifiers (artist_account_id, page, + * limit). Auth runs first, per the validator convention of the measurements + * family. The path id always wins — a catalogId smuggled into the query + * string is ignored. * - * @param searchParams - The URL search parameters to validate. + * @param request - The incoming HTTP request. * @param catalogId - The catalogId path segment. * @returns A NextResponse with an error if validation fails, or the validated request. */ -export function validateGetCatalogMeasurementsQuery( - searchParams: URLSearchParams, +export async function validateGetCatalogMeasurementsQuery( + request: NextRequest, catalogId: string, -): NextResponse | GetCatalogMeasurementsQuery { +): Promise { + const authResult = await validateAuthContext(request); + if (authResult instanceof NextResponse) return authResult; + + const { searchParams } = new URL(request.url); const result = getCatalogMeasurementsQuerySchema.safeParse({ ...Object.fromEntries(searchParams.entries()), catalogId, @@ -60,5 +69,5 @@ export function validateGetCatalogMeasurementsQuery( ); } - return result.data; + return { ...result.data, accountId: authResult.accountId }; }