Public: anyone can read this; the crew can change it.
/releases reads each pull's repository once: 20 releases cost one repository lookup, not 20 #6118
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/6112-releases-reads-each-pull-s-repository-on → main 3 commits fork
2 files
changed +39
−4
src/web/pages/releases.ts +7 −4
| @@ -7,7 +7,7 @@ | ||
| 7 | 7 | * `--rollback`, then this process drains. |
| 8 | 8 | */ |
| 9 | 9 | |
| 10 | import { getRepoById } from "../../core/repos.ts"; | |
| 10 | import { getReposByIds, type Repo } from "../../core/repos.ts"; | |
| 11 | 11 | import { notePhase, takeSessionTime } from "../../core/route_timing.ts"; |
| 12 | 12 | import { running } from "../../kernel/version.ts"; |
| 13 | 13 | import type { Ctx } from "../../http/context.ts"; |
| @@ -29,9 +29,9 @@ export function liveLine(ctx: Ctx): Html | undefined { | ||
| 29 | 29 | </p>`; |
| 30 | 30 | } |
| 31 | 31 | |
| 32 | function pullCell(ctx: Ctx, r: Release): Html { | |
| 32 | function pullCell(repos: ReadonlyMap<string, Repo>, r: Release): Html { | |
| 33 | 33 | if (r.pullNumber === null) return html`—`; |
| 34 | const repo = getRepoById(ctx.app.db, r.repoId); | |
| 34 | const repo = repos.get(r.repoId); | |
| 35 | 35 | return repo === undefined |
| 36 | 36 | ? html`#${r.pullNumber}` |
| 37 | 37 | : html`<a href="/${repo.ownerName}/${repo.name}/pulls/${r.pullNumber}">#${r.pullNumber}</a>`; |
| @@ -45,6 +45,9 @@ export async function releasesPage(ctx: Ctx): Promise<Response> { | ||
| 45 | 45 | const reading = performance.now(); |
| 46 | 46 | const releases = listReleases(ctx.app.db); |
| 47 | 47 | notePhase(ctx.request, "releases", performance.now() - reading); |
| 48 | const looking = performance.now(); | |
| 49 | const repos = getReposByIds(ctx.app.db, releases.flatMap((r) => (r.pullNumber === null ? [] : [r.repoId]))); | |
| 50 | notePhase(ctx.request, "repos", performance.now() - looking); | |
| 48 | 51 | const live = running().commit; |
| 49 | 52 | const writer = mayRollBack(ctx.app, user); |
| 50 | 53 | const checking = performance.now(); |
| @@ -60,7 +63,7 @@ export async function releasesPage(ctx: Ctx): Promise<Response> { | ||
| 60 | 63 | rows.push(html`<tr> |
| 61 | 64 | <td>${sha(r.sha)} ${when(isLive, () => badge("live", { tone: "success" }))} |
| 62 | 65 | ${when(!r.ok, () => badge("preflight failed", { tone: "danger" }))}</td> |
| 63 | <td>${pullCell(ctx, r)}</td> | |
| 66 | <td>${pullCell(repos, r)}</td> | |
| 64 | 67 | <td>${time(r.createdAt)}</td> |
| 65 | 68 | <td>${isLive ? html`since ${time(BOOTED)}` : html`—`}</td> |
| 66 | 69 | <td>${when(button, () => |
test/releases-timing.test.ts +32
| @@ -8,6 +8,7 @@ import { createApp, type App } from "../src/app.ts"; | ||
| 8 | 8 | import { createUser } from "../src/core/users.ts"; |
| 9 | 9 | import { serve, type Servers } from "../src/http/server.ts"; |
| 10 | 10 | import { releaseDir } from "../src/release/checkout.ts"; |
| 11 | import { createSession } from "../src/core/sessions.ts"; | |
| 11 | 12 | import { recordRelease } from "../src/release/record.ts"; |
| 12 | 13 | import { proveAll } from "./helpers/verified.ts"; |
| 13 | 14 | import { quietLogs } from "./helpers/globals.ts"; |
| @@ -73,3 +74,34 @@ test("a writer's GET /releases names its phases and has a Roll back button per n | ||
| 73 | 74 | expect(body.match(/Roll back/g)?.length).toBe(KEPT.length); |
| 74 | 75 | expect(body).not.toContain(`/releases/${LIVE}/rollback`); |
| 75 | 76 | }); |
| 77 | ||
| 78 | test("20 listed releases over 2 repositories cost one repository read; a deleted repository's pull shows #n unlinked", async () => { | |
| 79 | const second = await createRepo(app, { id: app.db.query<{ id: string }, []>("SELECT id FROM user LIMIT 1").get()!.id, username: "alice" } as never, { name: "other" }); | |
| 80 | if (!second.ok) throw new Error(second.error); | |
| 81 | const ids = app.db.query<{ id: string }, []>("SELECT id FROM repo ORDER BY name").all().map((r) => r.id); | |
| 82 | const gone = "g".repeat(40); | |
| 83 | app.db.query("PRAGMA foreign_keys = OFF").run(); | |
| 84 | for (let i = 0; i < 19; i++) | |
| 85 | recordRelease(app.db, { sha: i.toString(16).padStart(40, "9"), repoId: ids[i % 2]!, pullNumber: 100 + i, ok: true, line: "ok" }); | |
| 86 | recordRelease(app.db, { sha: gone, repoId: "deleted-repo", pullNumber: 777, ok: true, line: "ok" }); | |
| 87 | app.db.query("PRAGMA foreign_keys = ON").run(); | |
| 88 | const user = app.db.query<{ id: string }, []>("SELECT id FROM user LIMIT 1").get()!.id; | |
| 89 | const cookie = `forge_session=${createSession(app.db, user).token}`; | |
| 90 | const real = app.db.query.bind(app.db); | |
| 91 | let reads = 0; | |
| 92 | (app.db as { query: unknown }).query = (sql: string) => { | |
| 93 | if (/FROM repo\b/i.test(sql) && /repo\.id\s+(IN|=)/i.test(sql)) reads++; | |
| 94 | return real(sql as never); | |
| 95 | }; | |
| 96 | let res: Response; | |
| 97 | try { | |
| 98 | res = await fetch(`${servers.url}/releases`, { headers: { cookie } }); | |
| 99 | } finally { | |
| 100 | (app.db as { query: unknown }).query = real; | |
| 101 | } | |
| 102 | const body = await res.text(); | |
| 103 | expect(reads).toBe(1); | |
| 104 | expect(res.headers.get("server-timing")).toMatch(/repos;dur=\d+\.\d/); | |
| 105 | expect(body).toContain("/pulls/100\">#100</a>"); | |
| 106 | expect(body).toMatch(/<td>#777<\/td>/); | |
| 107 | }); |
new above
offline