Public: anyone can read this; the crew can change it.
The API's comment pages names its Server-Timing phases: session, comments, render #4939
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/4923-the-api-s-comment-pages-names-its-server → main 1 commit fork
2 files
changed +81
−8
src/http/api.ts +13 −8
| @@ -3286,6 +3286,7 @@ export function apiRoutes(router: Router<Handler>): void { | ||
| 3286 | 3286 | // A thread, a page at a time: oldest first, `next` is the `after` of the |
| 3287 | 3287 | // following page (#4150). A pull's number answers the same. |
| 3288 | 3288 | router.add("GET", "/api/v1/repos/:owner/:name/issues/:number/comments", (ctx) => { |
| 3289 | notePhase(ctx.request, "session", takeSessionTime(ctx.request)); | |
| 3289 | 3290 | const found = loadConversation(ctx); |
| 3290 | 3291 | if ("error" in found) return found.error; |
| 3291 | 3292 | const rawLimit = ctx.url.searchParams.get("limit"); |
| @@ -3295,15 +3296,19 @@ export function apiRoutes(router: Router<Handler>): void { | ||
| 3295 | 3296 | const after = ctx.url.searchParams.get("after"); |
| 3296 | 3297 | if (after !== null && !/^[A-Za-z0-9_-]{1,64}$/.test(after)) |
| 3297 | 3298 | return apiError("invalid_after", 422, "after is a comment id"); |
| 3298 | const rows = listComments(ctx.app.db, found.issue.id, { | |
| 3299 | limit: limit + 1, | |
| 3300 | ...(after === null ? {} : { after }), | |
| 3301 | }); | |
| 3302 | const page = rows.slice(0, limit); | |
| 3303 | return json({ | |
| 3304 | comments: readerMarked(ctx, found.repo.id, page), | |
| 3305 | next: rows.length > limit ? page[page.length - 1]!.id : null, | |
| 3299 | const { rows, page } = timePhase(ctx.request, "comments", () => { | |
| 3300 | const read = listComments(ctx.app.db, found.issue.id, { | |
| 3301 | limit: limit + 1, | |
| 3302 | ...(after === null ? {} : { after }), | |
| 3303 | }); | |
| 3304 | return { rows: read, page: read.slice(0, limit) }; | |
| 3306 | 3305 | }); |
| 3306 | return timePhase(ctx.request, "render", () => | |
| 3307 | json({ | |
| 3308 | comments: readerMarked(ctx, found.repo.id, page), | |
| 3309 | next: rows.length > limit ? page[page.length - 1]!.id : null, | |
| 3310 | }), | |
| 3311 | ); | |
| 3307 | 3312 | }); |
| 3308 | 3313 | |
| 3309 | 3314 | // What an issue waits for, and what waits for it. |
test/api-comments-timing.test.ts added +68
| @@ -0,0 +1,68 @@ | ||
| 1 | /** The API comments page names its Server-Timing phases (#3889). */ | |
| 2 | import { afterAll, beforeAll, expect, test } from "bun:test"; | |
| 3 | import { mkdtemp, rm } from "node:fs/promises"; | |
| 4 | import { tmpdir } from "node:os"; | |
| 5 | import { join } from "node:path"; | |
| 6 | import { createApp, type App } from "../src/app.ts"; | |
| 7 | import { createRepo } from "../src/actions/repos.ts"; | |
| 8 | import { addComment, createIssue } from "../src/core/issues.ts"; | |
| 9 | import { createSession } from "../src/core/sessions.ts"; | |
| 10 | import { createUser } from "../src/core/users.ts"; | |
| 11 | import { serve, type Servers } from "../src/http/server.ts"; | |
| 12 | import { quietLogs } from "./helpers/globals.ts"; | |
| 13 | ||
| 14 | quietLogs(); | |
| 15 | ||
| 16 | let dataDir: string; | |
| 17 | let app: App; | |
| 18 | let servers: Servers; | |
| 19 | let cookie = ""; | |
| 20 | ||
| 21 | const cells = process.env["FORGE_CELLS"]; | |
| 22 | ||
| 23 | beforeAll(async () => { | |
| 24 | process.env["FORGE_CELLS"] = "off"; | |
| 25 | dataDir = await mkdtemp(join(tmpdir(), "forge-api-comments-timing-")); | |
| 26 | app = createApp({ dataDir, port: 0 }); | |
| 27 | servers = await serve(app); | |
| 28 | const made = await createUser(app.db, { | |
| 29 | origin: "seed", | |
| 30 | trusted: true, | |
| 31 | username: "alice", | |
| 32 | email: "[email protected]", | |
| 33 | password: "a-good-passphrase", | |
| 34 | adminIfFirst: true, | |
| 35 | }); | |
| 36 | if (!made.ok) throw new Error(String(made.error)); | |
| 37 | const alice = made.value; | |
| 38 | cookie = `forge_session=${createSession(app.db, alice.id).token}`; | |
| 39 | const repo = await createRepo(app, alice, { name: "r", visibility: "public" }); | |
| 40 | if (!repo.ok) throw new Error(String(repo.error)); | |
| 41 | }); | |
| 42 | afterAll(async () => { | |
| 43 | await servers.stop(); | |
| 44 | app.close(); | |
| 45 | if (cells === undefined) delete process.env["FORGE_CELLS"]; | |
| 46 | else process.env["FORGE_CELLS"] = cells; | |
| 47 | await rm(dataDir, { recursive: true, force: true }); | |
| 48 | }); | |
| 49 | ||
| 50 | async function phases(path: string): Promise<{ status: number; names: string[]; body: string }> { | |
| 51 | const res = await fetch(`${servers.url}${path}`, { headers: { cookie } }); | |
| 52 | const body = await res.text(); | |
| 53 | const names = (res.headers.get("server-timing") ?? "").split(", ").map((p) => p.split(";")[0]!); | |
| 54 | return { status: res.status, names: names.slice(names.indexOf("app") + 1), body }; | |
| 55 | } | |
| 56 | ||
| 57 | ||
| 58 | test("the comments page names session, comments, render in order", async () => { | |
| 59 | const repo = app.db.query("SELECT id FROM repo WHERE name = 'r'").get() as { id: string }; | |
| 60 | const user = app.db.query("SELECT id FROM user WHERE username = 'alice'").get() as { id: string }; | |
| 61 | const issue = createIssue(app.db, { repoId: repo.id, authorId: user.id, title: "t" }); | |
| 62 | if (!issue.ok) throw new Error(String(issue.error)); | |
| 63 | addComment(app.db, { issueId: issue.value.id, authorId: user.id, body: "hello" }); | |
| 64 | const got = await phases(`/api/v1/repos/alice/r/issues/${issue.value.number}/comments`); | |
| 65 | expect(got.status).toBe(200); | |
| 66 | expect(got.names).toEqual(["session", "comments", "render"]); | |
| 67 | expect(JSON.parse(got.body).comments).toHaveLength(1); | |
| 68 | }); |
new above
offline