Public: anyone can read this; the crew can change it.
GET /api/v1/repos/:owner/:name/issues/:number carries an ETag and answers a repeat with 304 until the issue changes #9150
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/9143-get-api-v1-repos-owner-name-issues-numbe → main 4 commits fork 3 threads · 3 unresolved
5 files
changed +100
−5
src/contract/openapi/routes/getApiV1ReposOwnerNameIssuesNumber.ts +1 −1
| @@ -1,3 +1,3 @@ | ||
| 1 | 1 | import { REPO, type Route } from "../shared.ts"; |
| 2 | 2 | |
| 3 | export const route: Route = { method: "get", path: `${REPO}/issues/{number}`, summary: "Read one issue with its newest 50 comments, oldest first, commentsTotal and commentsNext (a cursor for …/comments?cursor=, null when no older comment is left to fetch, and with comments=0, which shows none: read …/comments); comments=N (0 to 200) asks for another count, adds commentCount, any other value is a 422 invalid_comments", auth: false, query: { comments: "integer" } }; | |
| 3 | export const route: Route = { method: "get", path: `${REPO}/issues/{number}`, summary: "Read one issue with its newest 50 comments, oldest first, commentsTotal and commentsNext (a cursor for …/comments?cursor=, null when no older comment is left to fetch, and with comments=0, which shows none: read …/comments); comments=N (0 to 200) asks for another count, adds commentCount, any other value is a 422 invalid_comments; the 200 carries a weak etag of the exact JSON and cache-control private, no-cache (it varies by reader), and a repeat sending If-None-Match gets 304 with no body once access is checked", auth: false, alsoStatus: [304], query: { comments: "integer" } }; |
src/contract/openapi/routes/index.ts +1 −1
| @@ -98,7 +98,6 @@ export { route as getApiV1ReposOwnerNameAppNotes } from "./getApiV1ReposOwnerNam | ||
| 98 | 98 | export { route as getApiV1ReposOwnerNameAppProductionOpen } from "./getApiV1ReposOwnerNameAppProductionOpen.ts"; |
| 99 | 99 | export { route as getApiV1ReposOwnerNameBranches } from "./getApiV1ReposOwnerNameBranches.ts"; |
| 100 | 100 | export { route as getApiV1ReposOwnerNameBuilds } from "./getApiV1ReposOwnerNameBuilds.ts"; |
| 101 | export { route as getApiV1ReposOwnerNameIterations } from "./getApiV1ReposOwnerNameIterations.ts"; | |
| 102 | 101 | export { route as getApiV1ReposOwnerNameCommits } from "./getApiV1ReposOwnerNameCommits.ts"; |
| 103 | 102 | export { route as getApiV1ReposOwnerNameCommitsOid } from "./getApiV1ReposOwnerNameCommitsOid.ts"; |
| 104 | 103 | export { route as getApiV1ReposOwnerNameCommitsOidCheck } from "./getApiV1ReposOwnerNameCommitsOidCheck.ts"; |
| @@ -121,6 +120,7 @@ export { route as getApiV1ReposOwnerNameIssuesNumberComments } from "./getApiV1R | ||
| 121 | 120 | export { route as getApiV1ReposOwnerNameIssuesNumberDependencies } from "./getApiV1ReposOwnerNameIssuesNumberDependencies.ts"; |
| 122 | 121 | export { route as getApiV1ReposOwnerNameIssuesNumberPlan } from "./getApiV1ReposOwnerNameIssuesNumberPlan.ts"; |
| 123 | 122 | export { route as getApiV1ReposOwnerNameIssuesNumberTeams } from "./getApiV1ReposOwnerNameIssuesNumberTeams.ts"; |
| 123 | export { route as getApiV1ReposOwnerNameIterations } from "./getApiV1ReposOwnerNameIterations.ts"; | |
| 124 | 124 | export { route as getApiV1ReposOwnerNameLabels } from "./getApiV1ReposOwnerNameLabels.ts"; |
| 125 | 125 | export { route as getApiV1ReposOwnerNameMap } from "./getApiV1ReposOwnerNameMap.ts"; |
| 126 | 126 | export { route as getApiV1ReposOwnerNameMilestones } from "./getApiV1ReposOwnerNameMilestones.ts"; |
src/http/api/issues.ts +2 −2
| @@ -241,7 +241,7 @@ export function issueRoutes(router: Router<Handler>): void { | ||
| 241 | 241 | }); |
| 242 | 242 | const labelled = timePhase(ctx.request, "labels", () => withLabels(ctx, issue)); |
| 243 | 243 | return timePhase(ctx.request, "render", () => |
| 244 | json({ | |
| 244 | tagged(ctx.request, JSON.stringify({ | |
| 245 | 245 | ...labelled, |
| 246 | 246 | ...(issue.kind === "pull" |
| 247 | 247 | ? { |
| @@ -253,7 +253,7 @@ export function issueRoutes(router: Router<Handler>): void { | ||
| 253 | 253 | commentsTotal: total, |
| 254 | 254 | commentsNext: shown.length > 0 && shown.length < total ? shown[0]!.id : null, |
| 255 | 255 | ...(asked === null ? {} : { commentCount: total }), |
| 256 | }), | |
| 256 | })), | |
| 257 | 257 | ); |
| 258 | 258 | }); |
| 259 | 259 |
test/api-pulls-check-preview.test.ts +3 −1
| @@ -33,10 +33,12 @@ const addPull = (title: string, head: string) => { | ||
| 33 | 33 | .run(made.value.id, `h-${head}`, head); |
| 34 | 34 | return made.value; |
| 35 | 35 | }; |
| 36 | // Ids rise with each call: the list reads the greatest id per pull as the latest run. | |
| 37 | let runSeq = 0; | |
| 36 | 38 | const run = (issueId: string, oid: string, state: string) => |
| 37 | 39 | app.db |
| 38 | 40 | .query("INSERT INTO run (id, repo_id, kind, issue_id, commit_oid, state) VALUES (?, ?, 'check', ?, ?, ?)") |
| 39 | .run(`zzzz-${crypto.randomUUID()}`, repoId, issueId, oid, state); | |
| 41 | .run(`zzzz-${String(++runSeq).padStart(6, "0")}`, repoId, issueId, oid, state); | |
| 40 | 42 | let userId = ""; |
| 41 | 43 | |
| 42 | 44 | beforeAll(async () => { |
test/issues-etag.test.ts +93
| @@ -4,6 +4,7 @@ import { tmpdir } from "node:os"; | ||
| 4 | 4 | import { join } from "node:path"; |
| 5 | 5 | import { createApp, type App } from "../src/app.ts"; |
| 6 | 6 | import { serve, type Servers } from "../src/http/server.ts"; |
| 7 | import { createLabel } from "../src/core/issues.ts"; | |
| 7 | 8 | import { seedAdmin } from "./helpers/admin.ts"; |
| 8 | 9 | |
| 9 | 10 | describe("issues list ETag", () => { |
| @@ -159,3 +160,95 @@ describe("issue comments ETag", () => { | ||
| 159 | 160 | expect(star.status).not.toBe(200); |
| 160 | 161 | }); |
| 161 | 162 | }); |
| 163 | ||
| 164 | describe("single issue ETag", () => { | |
| 165 | let dataDir: string; | |
| 166 | let app: App; | |
| 167 | let servers: Servers; | |
| 168 | let base: string; | |
| 169 | let token = ""; | |
| 170 | const send = (path: string, method = "GET", bodyObj?: unknown, headers: Record<string, string> = {}, auth = true) => | |
| 171 | fetch(`${base}${path}`, { | |
| 172 | method, | |
| 173 | headers: { | |
| 174 | ...(auth ? { authorization: `Bearer ${token}` } : {}), | |
| 175 | "content-type": "application/json", | |
| 176 | ...headers, | |
| 177 | }, | |
| 178 | ...(bodyObj === undefined ? {} : { body: JSON.stringify(bodyObj) }), | |
| 179 | }); | |
| 180 | const R = "/api/v1/repos/dev/lib/issues"; | |
| 181 | ||
| 182 | beforeAll(async () => { | |
| 183 | dataDir = await mkdtemp(join(tmpdir(), "forge-issue-etag-")); | |
| 184 | app = createApp({ dataDir, port: 0 }); | |
| 185 | servers = await serve(app); | |
| 186 | base = servers.url; | |
| 187 | await seedAdmin(app, { username: "dev", email: "[email protected]", password: "etag-pass-1" }); | |
| 188 | token = await fetch(`${base}/api/v1/tokens`, { | |
| 189 | method: "POST", | |
| 190 | headers: { "content-type": "application/json" }, | |
| 191 | body: JSON.stringify({ adminConfirm: true, username: "dev", password: "etag-pass-1", name: "c", scope: "rw" }), | |
| 192 | }) | |
| 193 | .then((r) => r.json()) | |
| 194 | .then((j) => (j as { token: string }).token); | |
| 195 | await send("/api/v1/repos", "POST", { name: "lib", visibility: "public" }); | |
| 196 | await send(R, "POST", { title: "T", body: "b" }); | |
| 197 | await send(R, "POST", { title: "Hidden", body: "x", sealed: true }); | |
| 198 | }); | |
| 199 | ||
| 200 | afterAll(async () => { | |
| 201 | await servers.stop(); | |
| 202 | await rm(dataDir, { recursive: true, force: true }); | |
| 203 | }); | |
| 204 | ||
| 205 | test("200, 304, then 200 with a new tag after a comment", async () => { | |
| 206 | const first = await send(`${R}/1`); | |
outdated
Add the close, retitle, assign and label cases here (each: GET, change, GET with the old tag, expect 200 and a new etag). Box 2 names all five. |
||
| 207 | expect(first.status).toBe(200); | |
| 208 | const tag = first.headers.get("etag")!; | |
| 209 | expect(tag).toMatch(/^W\//); | |
| 210 | expect(first.headers.get("cache-control")).toBe("private, no-cache"); | |
| 211 | expect(first.headers.get("vary")).toContain("authorization, cookie"); | |
| 212 | await first.text(); | |
| 213 | const again = await send(`${R}/1`, "GET", undefined, { "if-none-match": tag }); | |
| 214 | expect(again.status).toBe(304); | |
| 215 | expect(again.headers.get("etag")).toBe(tag); | |
| 216 | expect(await again.text()).toBe(""); | |
| 217 | await send(`${R}/1/comments`, "POST", { body: "hi" }); | |
| 218 | const changed = await send(`${R}/1`, "GET", undefined, { "if-none-match": tag }); | |
| 219 | expect(changed.status).toBe(200); | |
| 220 | expect(changed.headers.get("etag")).not.toBe(tag); | |
| 221 | expect(((await changed.json()) as { commentsTotal: number }).commentsTotal).toBe(1); | |
| 222 | }); | |
| 223 | ||
| 224 | test("closing, retitling, assigning and labelling each change the tag", async () => { | |
| 225 | const repoId = app.db.query<{ id: string }, []>("SELECT id FROM repo WHERE name = 'lib'").get()!.id; | |
outdated
Assert the exact refusal status and the absence of an etag header, not just 'not 200 and not 304'. A 500 passes as written. |
||
| 226 | createLabel(app.db, repoId, "bug", "d73a4a"); | |
| 227 | const changes: [string, string, unknown][] = [ | |
| 228 | ["retitle", "PATCH", { title: "Renamed" }], | |
| 229 | ["assign", "PUT", { assignee: "dev" }], | |
| 230 | ["label", "PUT", { labels: ["bug"] }], | |
| 231 | ["close", "PATCH", { state: "closed", note: "fixed elsewhere" }], | |
| 232 | ]; | |
| 233 | for (const [what, method, payload] of changes) { | |
| 234 | const before = (await send(`${R}/1`)).headers.get("etag")!; | |
| 235 | const path = what === "assign" ? `${R}/1/assignee` : what === "label" ? `${R}/1/labels` : `${R}/1`; | |
| 236 | const done = await send(path, method, payload); | |
| 237 | expect(done.status, what).toBe(200); | |
| 238 | const after = await send(`${R}/1`, "GET", undefined, { "if-none-match": before }); | |
| 239 | expect(after.status, what).toBe(200); | |
| 240 | expect(after.headers.get("etag"), what).not.toBe(before); | |
| 241 | } | |
| 242 | }); | |
| 243 | ||
| 244 | test("a stranger is refused a sealed issue whatever tag they hold", async () => { | |
| 245 | const own = (await send(`${R}/2`)).headers.get("etag")!; | |
| 246 | const bare = await send(`${R}/2`, "GET", undefined, {}, false); | |
| 247 | expect(bare.status).toBe(404); | |
| 248 | for (const t of [own, "*"]) { | |
| 249 | const out = await send(`${R}/2`, "GET", undefined, { "if-none-match": t }, false); | |
| 250 | expect(out.status).toBe(bare.status); | |
| 251 | expect(out.headers.get("etag")).toBeNull(); | |
| 252 | } | |
| 253 | }); | |
| 254 | }); |
new above
offline
Flake fix unrelated to issue #9143. Move it to its own pull.