Skip to content

Commit baa76e7

Browse files
authored
fix(fetch): use pages for pagination instead of cursor (#4402)
* fix(fetch): use pages for pagination instead of cursor * Create orange-lamps-happen.md * Update whoami.ts * chore(hasMorePages): add tests for hasMorePages
1 parent 9982167 commit baa76e7

5 files changed

Lines changed: 111 additions & 4 deletions

File tree

.changeset/orange-lamps-happen.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
"wrangler": patch
3+
---
4+
5+
This PR adds a fetch handler that uses `page`, assuming `result_info` provided by the endpoint contains `page`, `per_page`, and `total`
6+
7+
This is needed as the existing `fetchListResult` handler for fetching potentially paginated results doesn't work for endpoints that don't implement `cursor`.
8+
9+
Fixes #4349
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
import { hasMorePages } from "../cfetch";
2+
3+
/**
4+
hasMorePages is a function that returns a boolean based on the result_info object returned from the cloudflare v4 API - if the current page is less than the total number of pages, it returns true, otherwise false.
5+
*/
6+
7+
describe("hasMorePages", () => {
8+
it("should handle result_info not having enough results to paginate", () => {
9+
expect(
10+
hasMorePages({
11+
page: 1,
12+
per_page: 10,
13+
count: 5,
14+
total_count: 5,
15+
})
16+
).toBe(false);
17+
});
18+
it("should return true if the current page is less than the total number of pages", () => {
19+
expect(
20+
hasMorePages({
21+
page: 1,
22+
per_page: 10,
23+
count: 10,
24+
total_count: 100,
25+
})
26+
).toBe(true);
27+
});
28+
it("should return false if we are on the last page of results", () => {
29+
expect(
30+
hasMorePages({
31+
page: 10,
32+
per_page: 10,
33+
count: 10,
34+
total_count: 100,
35+
})
36+
).toBe(false);
37+
});
38+
});

packages/wrangler/src/cfetch/index.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,66 @@ export async function fetchListResult<ResponseType>(
100100
return results;
101101
}
102102

103+
/**
104+
* Make a fetch request for a list of values using pages,
105+
* extracting the `result` from the JSON response,
106+
* and repeating the request if the results are paginated.
107+
*
108+
* This is similar to fetchListResult, but it uses the `page` query parameter instead of `cursor`.
109+
*/
110+
export async function fetchPagedListResult<ResponseType>(
111+
resource: string,
112+
init: RequestInit = {},
113+
queryParams?: URLSearchParams
114+
): Promise<ResponseType[]> {
115+
const results: ResponseType[] = [];
116+
let getMoreResults = true;
117+
let page = 1;
118+
while (getMoreResults) {
119+
queryParams = new URLSearchParams(queryParams);
120+
queryParams.set("page", String(page));
121+
122+
const json = await fetchInternal<FetchResult<ResponseType[]>>(
123+
resource,
124+
init,
125+
queryParams
126+
);
127+
if (json.success) {
128+
results.push(...json.result);
129+
if (hasMorePages(json.result_info)) {
130+
page = page + 1;
131+
} else {
132+
getMoreResults = false;
133+
}
134+
} else {
135+
throwFetchError(resource, json);
136+
}
137+
}
138+
return results;
139+
}
140+
141+
interface PageResultInfo {
142+
page: number;
143+
per_page: number;
144+
count: number;
145+
total_count: number;
146+
}
147+
148+
export function hasMorePages(
149+
result_info: unknown
150+
): result_info is PageResultInfo {
151+
const page = (result_info as PageResultInfo | undefined)?.page;
152+
const per_page = (result_info as PageResultInfo | undefined)?.per_page;
153+
const total = (result_info as PageResultInfo | undefined)?.total_count;
154+
155+
return (
156+
page !== undefined &&
157+
per_page !== undefined &&
158+
total !== undefined &&
159+
page * per_page < total
160+
);
161+
}
162+
103163
function throwFetchError(
104164
resource: string,
105165
response: FetchResult<unknown>

packages/wrangler/src/user/choose-account.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { fetchListResult } from "../cfetch";
1+
import { fetchPagedListResult } from "../cfetch";
22
import { getCloudflareAccountIdFromEnv } from "./auth-variables";
33

44
export type ChooseAccountItem = {
@@ -15,7 +15,7 @@ export async function getAccountChoices(): Promise<ChooseAccountItem[]> {
1515
return [{ id: accountIdFromEnv, name: "" }];
1616
} else {
1717
try {
18-
const response = await fetchListResult<{
18+
const response = await fetchPagedListResult<{
1919
account: ChooseAccountItem;
2020
}>(`/memberships`);
2121
const accounts = response.map((r) => r.account);

packages/wrangler/src/whoami.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import chalk from "chalk";
2-
import { fetchListResult, fetchResult } from "./cfetch";
2+
import { fetchPagedListResult, fetchResult } from "./cfetch";
33
import { logger } from "./logger";
44
import { getAPIToken, getAuthFromEnv, getScopes } from "./user";
55

@@ -91,7 +91,7 @@ async function getEmail(): Promise<string | undefined> {
9191
type AccountInfo = { name: string; id: string };
9292

9393
async function getAccounts(): Promise<AccountInfo[]> {
94-
return await fetchListResult<AccountInfo>("/accounts");
94+
return await fetchPagedListResult<AccountInfo>("/accounts");
9595
}
9696

9797
async function getTokenPermissions(): Promise<string[] | undefined> {

0 commit comments

Comments
 (0)