diff --git a/lib/api-client/core.ts b/lib/api-client/core.ts index d924e17f..5a221365 100644 --- a/lib/api-client/core.ts +++ b/lib/api-client/core.ts @@ -21,7 +21,7 @@ import { ConfigurationError, createApiError, } from "./errors.js"; -import { API_MAX_PER_PAGE } from "./pagination.js"; +import { API_MAX_PER_PAGE, paginate } from "./pagination.js"; import { type ErrorResponseBody, readErrorResponse, @@ -294,16 +294,17 @@ export function createApiClientCore({ /** * Fetch up to `limit` rows from a list endpoint, starting at `cursor`. * - * Each page asks for exactly the rows still missing, up to - * {@link API_MAX_PER_PAGE}, and is validated with `schema`. The walk stops - * once it has `limit` rows or the server has no next page, and returns the - * server's cursor for the page after the last one fetched. + * Walks pages with {@link paginate}, so every request in a walk uses the same + * `per_page`: a fresh walk splits `limit` evenly, and a page-number cursor + * (`OffsetPaginator`) keeps the page size it encodes. Each page is validated + * with `schema`. The walk stops once it has `limit` rows, the server has no + * next page, or it has fetched {@link MAX_LIST_PAGES} pages, and returns a + * cursor that resumes at the first row not returned. * * @throws {ApiValidationError} When `limit` is not between 1 and * `API_MAX_PER_PAGE * MAX_LIST_PAGES` * @throws {ApiResponseValidationError} When a page does not match `schema` - * @throws {Error} When a page has more rows than requested, a cursor - * repeats, or the walk passes {@link MAX_LIST_PAGES} + * @throws {Error} When a page has more rows than requested or a cursor repeats */ export async function listPage( fetchPage: ( @@ -323,26 +324,24 @@ export async function listPage( `limit must be an integer between 1 and ${API_MAX_PER_PAGE * MAX_LIST_PAGES}`, ); } - const items: T[] = []; const seenCursors = new Set(); - let cursor = initialCursor ?? undefined; - for (let page = 0; page < MAX_LIST_PAGES; page += 1) { - const perPage = Math.min(API_MAX_PER_PAGE, limit - items.length); - const { data, response } = await fetchPage(perPage, cursor); - const rows = validateResponse(schema, data); - if (rows.length > perPage) { - throw new Error("API returned more items than requested"); - } - items.push(...rows); - const nextCursor = nextPageCursor(response); - if (items.length >= limit || !nextCursor) { - return { items, nextCursor }; - } - if (seenCursors.has(nextCursor) || nextCursor === cursor) { - throw new Error("API returned a repeated pagination cursor"); - } - seenCursors.add(nextCursor); - cursor = nextCursor; - } - throw new Error("API pagination exceeded its page limit"); + const result = await paginate( + { limit, cursor: initialCursor ?? undefined, maxPages: MAX_LIST_PAGES }, + async (perPage, cursor) => { + const { data, response } = await fetchPage(perPage, cursor); + const rows = validateResponse(schema, data); + if (rows.length > perPage) { + throw new Error("API returned more items than requested"); + } + const nextCursor = nextPageCursor(response); + if (nextCursor) { + if (seenCursors.has(nextCursor) || nextCursor === cursor) { + throw new Error("API returned a repeated pagination cursor"); + } + seenCursors.add(nextCursor); + } + return nextCursor ? { data: rows, nextCursor } : { data: rows }; + }, + ); + return { items: result.data, nextCursor: result.nextCursor ?? null }; } diff --git a/test/api-client/core.test.ts b/test/api-client/core.test.ts index 403f10f5..edac09a3 100644 --- a/test/api-client/core.test.ts +++ b/test/api-client/core.test.ts @@ -312,6 +312,72 @@ describe("listPage", () => { ).rejects.toThrow("API returned a repeated pagination cursor"); }); + /** + * Sentry's `OffsetPaginator` (`src/sentry/api/paginator.py`) over `total` + * rows. Its cursor is `::`; a request whose + * `per_page` differs from the cursor's page size reads the cursor's window + * and keeps only its last `per_page` rows. + */ + function offsetPaginator(total: number) { + const requests: Array<{ perPage: number; cursor: string | undefined }> = + []; + async function fetchPage(perPage: number, cursor: string | undefined) { + requests.push({ perPage, cursor }); + const [value = 0, pageIndex = 0] = (cursor ?? "0:0:0") + .split(":") + .map(Number); + const start = pageIndex * value; + const stop = Math.min(total, start + (value || perPage) + 1); + let window = ids(start, stop); + if (value !== perPage) { + window = window.slice(-(perPage + 1)); + } + return page( + window.slice(0, perPage), + window.length > perPage ? `${perPage}:${pageIndex + 1}:0` : undefined, + ); + } + return { fetchPage, requests }; + } + + function ids(start: number, stop: number) { + return Array.from({ length: stop - start }, (_, index) => start + index); + } + + function idsOf(result: { items: Array<{ id: number }> }) { + return result.items.map(({ id }) => id); + } + + it("keeps per_page stable across an OffsetPaginator walk", async () => { + const { fetchPage, requests } = offsetPaginator(300); + + const first = await listPage(fetchPage, rows, 150); + const second = await listPage(fetchPage, rows, 150, first.nextCursor); + + expect(idsOf(first)).toEqual(ids(0, 150)); + expect(idsOf(second)).toEqual(ids(150, 300)); + expect(second.nextCursor).toBeNull(); + expect(new Set(requests.map(({ perPage }) => perPage))).toEqual( + new Set([75]), + ); + }); + + it("resumes an OffsetPaginator cursor with the page size it encodes", async () => { + const { fetchPage, requests } = offsetPaginator(300); + // Pairing `100:1:0` with per_page=50 reads rows 150-199 and skips 100-149. + expect((await fetchPage(50, "100:1:0")).data).toEqual( + ids(150, 200).map((id) => ({ id })), + ); + requests.length = 0; + + const first = await listPage(fetchPage, rows, 50, "100:1:0"); + const second = await listPage(fetchPage, rows, 50, first.nextCursor); + + expect(idsOf(first)).toEqual(ids(100, 150)); + expect(idsOf(second)).toEqual(ids(150, 200)); + expect(requests.map(({ perPage }) => perPage)).toEqual([100, 100]); + }); + it("validates each page with the schema", async () => { await expect( listPage( diff --git a/test/api-client/endpoints/organizations.test.ts b/test/api-client/endpoints/organizations.test.ts index 49307514..1546b8d9 100644 --- a/test/api-client/endpoints/organizations.test.ts +++ b/test/api-client/endpoints/organizations.test.ts @@ -108,7 +108,7 @@ describe("listOrganizations", () => { ]); expect(urls[0]?.searchParams.get("per_page")).toBe("3"); expect(urls[0]?.searchParams.get("query")).toBe("ac"); - expect(urls[1]?.searchParams.get("per_page")).toBe("1"); + expect(urls[1]?.searchParams.get("per_page")).toBe("3"); expect(urls[1]?.searchParams.get("cursor")).toBe("0:2:0"); });