Public: anyone can read this; the crew can change it.
GET /api/v1/repos/:owner/:name/pulls carries an ETag and answers a repeat with 304 when the list it would send has not changed #9121
Check · Review · Land · Live done — Landed and live in production.
mason/forge:mason/9118-get-api-v1-repos-owner-name-pulls-carrie → main 2 commits fork
3 files
changed +31
−4
src/contract/openapi/routes/getApiV1ReposOwnerNamePulls.ts +1 −1
| @@ -1,3 +1,3 @@ | ||
| 1 | 1 | import { LIST_QUERY, REPO, type Route } from "../shared.ts"; |
| 2 | 2 | |
| 3 | export const route: Route = { method: "get", path: `${REPO}/pulls`, summary: "List a repository's pulls", auth: false, query: LIST_QUERY, refusals: { invalid_limit: "limit is not a whole number from 1 to 200 (422)", invalid_cursor: "cursor is not the next_cursor of a previous page (422)", invalid_state: "state is not open, closed or all (422)" } }; | |
| 3 | export const route: Route = { method: "get", path: `${REPO}/pulls`, summary: "List a repository's pulls; carries a weak `etag` of the page sent (cache-control private, no-cache; vary authorization, cookie), and a repeat sending If-None-Match gets 304 with no body until a pull in the page is opened, pushed to, checked, merged or closed", auth: false, alsoStatus: [304], query: LIST_QUERY, refusals: { invalid_limit: "limit is not a whole number from 1 to 200 (422)", invalid_cursor: "cursor is not the next_cursor of a previous page (422)", invalid_state: "state is not open, closed or all (422)" } }; |
src/http/api.ts +10 −3
| @@ -274,6 +274,7 @@ import { recentGates } from "../checks/gate_result.ts"; | ||
| 274 | 274 | import { readRunLog, subscribeRunLog } from "../runs/log.ts"; |
| 275 | 275 | import { blobForRender, commitLog, listRefs, listTree, refTip } from "../git/read.ts"; |
| 276 | 276 | import { repoDir } from "../git/repos.ts"; |
| 277 | import { holds, weakEtag } from "./etag.ts"; | |
| 277 | 278 | import { carriedPulls, pullPath } from "../story/wave_pulls.ts"; |
| 278 | 279 | import { pullRefs, readWave, readWaves } from "../story/waves.ts"; |
| 279 | 280 | import { originForRepo, type OriginRepoRow } from "../core/origins.ts"; |
| @@ -2358,9 +2359,15 @@ export function apiRoutes(router: Router<Handler>): void { | ||
| 2358 | 2359 | ...(states[i] ?? {}), |
| 2359 | 2360 | })); |
| 2360 | 2361 | }); |
| 2361 | return timePhase(ctx.request, "render", () => | |
| 2362 | json(paged(items, query.limit, (p) => p.number)), | |
| 2363 | ); | |
| 2362 | // The tag is of the exact bytes sent, so a new verdict, head or state is never hidden behind a 304; | |
| 2363 | // the page, state, limit, cursor and fields are in the body or the query that chose it. | |
| 2364 | return timePhase(ctx.request, "render", () => { | |
| 2365 | const text = JSON.stringify(paged(items, query.limit, (p) => p.number)); | |
| 2366 | const etag = weakEtag(text); | |
| 2367 | const headers = { etag, "cache-control": "private, no-cache", vary: "authorization, cookie" }; | |
| 2368 | if (holds(ctx.request, etag)) return new Response(null, { status: 304, headers }); | |
| 2369 | return new Response(text, { headers: { "content-type": "application/json", ...headers } }); | |
| 2370 | }); | |
| 2364 | 2371 | }); |
| 2365 | 2372 | |
| 2366 | 2373 | // A writer hands a child its own narrow credential for this repository |
test/api-pulls-check-preview.test.ts +20
| @@ -151,3 +151,23 @@ test("a closed pull has no preview address", async () => { | ||
| 151 | 151 | const by = await listed({ cookie }, "closed"); |
| 152 | 152 | expect(by["done"]!["preview"]).toBeNull(); |
| 153 | 153 | }, SLOW); |
| 154 | ||
| 155 | test("the list carries an ETag: a repeat is 304 until a pull's verdict changes", async () => { | |
| 156 | const url = `${servers.url}/api/v1/repos/alice/notes/pulls?state=open`; | |
| 157 | const first = await fetch(url, { headers: { cookie } }); | |
| 158 | expect(first.status).toBe(200); | |
| 159 | const tag = first.headers.get("etag")!; | |
| 160 | expect(tag).toMatch(/^W\//); | |
| 161 | expect(first.headers.get("cache-control")).toBe("private, no-cache"); | |
| 162 | expect(first.headers.get("vary")).toStartWith("authorization, cookie"); | |
| 163 | await first.text(); | |
| 164 | const again = await fetch(url, { headers: { cookie, "if-none-match": tag } }); | |
| 165 | expect(again.status).toBe(304); | |
| 166 | expect(again.headers.get("etag")).toBe(tag); | |
| 167 | expect(await again.text()).toBe(""); | |
| 168 | run(pulls["going"]!.id, "n1", "ok"); | |
| 169 | const changed = await fetch(url, { headers: { cookie, "if-none-match": tag } }); | |
| 170 | expect(changed.status).toBe(200); | |
| 171 | expect(changed.headers.get("etag")).not.toBe(tag); | |
| 172 | await changed.text(); | |
| 173 | }, SLOW); |
new above
offline