Public: anyone can read this; the crew can change it.
The API's read of one issue names its Server-Timing phases: session, issue, comments, labels, render #3884
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/3766-the-api-s-read-of-one-issue-names-its-se → main 9 commits fork 1 thread · 1 unresolved
2 files
changed +66
−15
src/http/api.ts +25 −15
| @@ -3247,7 +3247,8 @@ export function apiRoutes(router: Router<Handler>): void { | ||
| 3247 | 3247 | router.add("GET", "/api/v1/repos/:owner/:name/issues/:number", (ctx) => { |
| 3248 | 3248 | // Numbers are shared with pulls: a pull's number answers as the same |
| 3249 | 3249 | // shape, marked `kind: "pull"`, with the path where its diff lives (#3299). |
| 3250 | const found = loadConversation(ctx); | |
| 3250 | notePhase(ctx.request, "session", takeSessionTime(ctx.request)); | |
| 3251 | const found = timePhase(ctx.request, "issue", () => loadConversation(ctx)); | |
| 3251 | 3252 | if ("error" in found) return found.error; |
| 3252 | 3253 | const { repo, issue } = found; |
| 3253 | 3254 | // comments=N answers the newest N, oldest first, and the total (#4097). |
| @@ -3258,21 +3259,30 @@ export function apiRoutes(router: Router<Handler>): void { | ||
| 3258 | 3259 | return apiError("invalid_comments", 422, "comments must be a whole number from 0 to 200"); |
| 3259 | 3260 | tail = Number(asked); |
| 3260 | 3261 | } |
| 3261 | const all = listComments(ctx.app.db, found.issue.id); | |
| 3262 | return json({ | |
| 3263 | ...withLabels(ctx, issue), | |
| 3264 | ...(issue.kind === "pull" | |
| 3265 | ? { | |
| 3266 | pull: `/api/v1/repos/${repo.ownerName}/${repo.name}/pulls/${issue.number}`, | |
| 3267 | } | |
| 3268 | : {}), | |
| 3269 | comments: readerMarked( | |
| 3270 | ctx, | |
| 3271 | found.repo.id, | |
| 3272 | tail === null ? all : tail === 0 ? [] : all.slice(-tail), | |
| 3273 | ), | |
| 3274 | ...(tail === null ? {} : { commentCount: all.length }), | |
| 3262 | const { all, shown } = timePhase(ctx.request, "comments", () => { | |
| 3263 | const rows = listComments(ctx.app.db, found.issue.id); | |
| 3264 | return { | |
| 3265 | all: rows, | |
| 3266 | shown: readerMarked( | |
| 3267 | ctx, | |
| 3268 | found.repo.id, | |
| 3269 | tail === null ? rows : tail === 0 ? [] : rows.slice(-tail), | |
| 3270 | ), | |
| 3271 | }; | |
| 3275 | 3272 | }); |
| 3273 | const labelled = timePhase(ctx.request, "labels", () => withLabels(ctx, issue)); | |
| 3274 | return timePhase(ctx.request, "render", () => | |
| 3275 | json({ | |
| 3276 | ...labelled, | |
| 3277 | ...(issue.kind === "pull" | |
| 3278 | ? { | |
| 3279 | pull: `/api/v1/repos/${repo.ownerName}/${repo.name}/pulls/${issue.number}`, | |
| 3280 | } | |
| 3281 | : {}), | |
| 3282 | comments: shown, | |
| 3283 | ...(tail === null ? {} : { commentCount: all.length }), | |
| 3284 | }), | |
| 3285 | ); | |
| 3276 | 3286 | }); |
| 3277 | 3287 | |
| 3278 | 3288 | // A thread, a page at a time: oldest first, `next` is the `after` of the |
test/api-issue-timing.test.ts added +41
| @@ -0,0 +1,41 @@ | ||
| 1 | /** The API's read of one issue names its Server-Timing phases, found or not. */ | |
| 2 | import { afterAll, beforeAll, expect, test } from "bun:test"; | |
| 3 | import { snappyFixture, type SnappyFixture } from "../bench/snappy.ts"; | |
| 4 | import { addComment, getIssue } from "../src/core/issues.ts"; | |
| 5 | ||
| 6 | let f: SnappyFixture; | |
| 7 | ||
| 8 | beforeAll(async () => { | |
| 9 | f = await snappyFixture({ repos: 1, issuesPerRepo: 4 }); | |
| 10 | const repo = f.repos[0]!; | |
| 11 | const issue = getIssue(f.app.db, repo.id, 2)!; | |
| 12 | addComment(f.app.db, { issueId: issue.id, authorId: f.owner.id, body: "first" }); | |
| 13 | }, 120_000); | |
| 14 | afterAll(async () => { | |
| 15 | await f.close(); | |
| 16 | }); | |
| 17 | ||
| 18 | async function read(n: number) { | |
| 19 | const repo = f.repos[0]!; | |
| 20 | const res = await fetch(`${f.base}/api/v1/repos/${repo.ownerName}/${repo.name}/issues/${n}`, { | |
| 21 | headers: { cookie: f.ownerCookie }, | |
| 22 | }); | |
| 23 | const body = (await res.json()) as { comments: unknown[] }; | |
| 24 | const names = (res.headers.get("server-timing") ?? "").split(", ").map((p) => p.split(";")[0]); | |
| 25 | return { res, body, names }; | |
| 26 | } | |
| 27 | ||
| 28 | test("an issue with comments names session, issue, comments, labels and render", async () => { | |
| 29 | const { res, body, names } = await read(2); | |
| 30 | expect(res.status).toBe(200); | |
| 31 | for (const n of ["session", "issue", "comments", "labels", "render"]) expect(names).toContain(n); | |
| 32 | expect(body.comments.length).toBe(1); | |
| 33 | }); | |
| 34 | ||
| 35 | test("a missing issue still names session and issue", async () => { | |
| 36 | const { res, names } = await read(99); | |
| 37 | expect(res.status).toBe(404); | |
| 38 | expect(names).toContain("session"); | |
| 39 | expect(names).toContain("issue"); | |
| 40 | expect(names).not.toContain("render"); | |
| 41 | }); |
new above
offline
This query is the comments phase's real cost, but it runs before
timePhase("comments")starts, so no phase covers it. Move it inside thecommentsclosure. Returnallalongside the marked slice socommentCountcan still useall.length.