Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(fetch): use pages for pagination instead of cursor #4402

Merged
merged 4 commits into from
Nov 13, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .changeset/orange-lamps-happen.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"wrangler": patch
---

This PR adds a fetch handler that uses `page`, assuming `result_info` provided by the endpoint contains `page`, `per_page`, and `total`

This is needed as the existing `fetchListResult` handler for fetching potentially paginated results doesn't work for endpoints that don't implement `cursor`.

Fixes #4349
58 changes: 58 additions & 0 deletions packages/wrangler/src/cfetch/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,64 @@
return results;
}

/**
* Make a fetch request for a list of values using pages,
* extracting the `result` from the JSON response,
* and repeating the request if the results are paginated.
*
* This is similar to fetchListResult, but it uses the `page` query parameter instead of `cursor`.
*/
export async function fetchPagedListResult<ResponseType>(
resource: string,
init: RequestInit = {},
queryParams?: URLSearchParams
): Promise<ResponseType[]> {
const results: ResponseType[] = [];
let getMoreResults = true;
let page = 1;
while (getMoreResults) {
queryParams = new URLSearchParams(queryParams);
queryParams.set("page", String(page));

const json = await fetchInternal<FetchResult<ResponseType[]>>(
resource,
init,
queryParams
);
if (json.success) {
results.push(...json.result);
if (hasMorePages(json.result_info)) {
page = page + 1;

Check warning on line 130 in packages/wrangler/src/cfetch/index.ts

View check run for this annotation

Codecov / codecov/patch

packages/wrangler/src/cfetch/index.ts#L130

Added line #L130 was not covered by tests
} else {
getMoreResults = false;
}
} else {
throwFetchError(resource, json);
}
}
return results;
}

interface PageResultInfo {
page: number;
per_page: number;
count: number;
total_count: number;
}

function hasMorePages(result_info: unknown): result_info is PageResultInfo {
const page = (result_info as PageResultInfo | undefined)?.page;
const per_page = (result_info as PageResultInfo | undefined)?.per_page;
const total = (result_info as PageResultInfo | undefined)?.total_count;

return (
page !== undefined &&
per_page !== undefined &&
total !== undefined &&
page * per_page < total

Check warning on line 157 in packages/wrangler/src/cfetch/index.ts

View check run for this annotation

Codecov / codecov/patch

packages/wrangler/src/cfetch/index.ts#L155-L157

Added lines #L155 - L157 were not covered by tests
);
}

function throwFetchError(
resource: string,
response: FetchResult<unknown>
Expand Down
4 changes: 2 additions & 2 deletions packages/wrangler/src/user/choose-account.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { fetchListResult } from "../cfetch";
import { fetchPagedListResult } from "../cfetch";
import { getCloudflareAccountIdFromEnv } from "./auth-variables";

export type ChooseAccountItem = {
Expand All @@ -15,7 +15,7 @@ export async function getAccountChoices(): Promise<ChooseAccountItem[]> {
return [{ id: accountIdFromEnv, name: "" }];
} else {
try {
const response = await fetchListResult<{
const response = await fetchPagedListResult<{
account: ChooseAccountItem;
}>(`/memberships`);
const accounts = response.map((r) => r.account);
Expand Down
Loading