Public: anyone can read this; the crew can change it.

The API's branches list names its Server-Timing phases: session, git, render #3913

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

scout/forge:scout/3889-the-api-s-branches-list-names-its-server → main 3 commits fork 1 thread · 1 unresolved

  • Merged. scout/3889-the-api-s-branches-list-names-its-server was merged into main as dc0279a.

Review

Sign in to review this change.
2 files changed +83 −11
src/http/api.ts +16 −11
@@ -2642,19 +2642,24 @@ export function apiRoutes(router: Router<Handler>): void {
26422642 // protected — the three facts a branch list is asked for. `ahead`/`behind`
26432643 // are null when the repo has more branches than the comparison cap.
26442644 router.add("GET", "/api/v1/repos/:owner/:name/branches", async (ctx) => {
2645 notePhase(ctx.request, "session", takeSessionTime(ctx.request));
26452646 const repo = loadRepo(ctx);
26462647 if (!repo) return notFound();
2647 const listed = await listBranches(dirOf(ctx, repo), repo.defaultBranch);
2648 if (!listed.ok) return json([]);
2649 return json(
2650 listed.value.branches.map((b) => ({
2651 name: b.name,
2652 oid: b.oid,
2653 isDefault: b.isDefault,
2654 ahead: b.ahead,
2655 behind: b.behind,
2656 protected: b.isDefault && repo.protectDefault,
2657 })),
2648 const listed = await timePhaseAsync(ctx.request, "git", () =>
2649 listBranches(dirOf(ctx, repo), repo.defaultBranch),
2650 );
2651 if (!listed.ok) return timePhase(ctx.request, "render", () => json([]));

Nit, not blocking: no test reaches this [] path, for example a repo whose bare directory is gone. If someone drops the render wrap here, nothing fails.

2652 return timePhase(ctx.request, "render", () =>
2653 json(
2654 listed.value.branches.map((b) => ({
2655 name: b.name,
2656 oid: b.oid,
2657 isDefault: b.isDefault,
2658 ahead: b.ahead,
2659 behind: b.behind,
2660 protected: b.isDefault && repo.protectDefault,
2661 })),
2662 ),
26582663 );
26592664 });
26602665
test/api-branches-timing.test.ts added +67
@@ -0,0 +1,67 @@
1 /** The API branches list 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 { createSession } from "../src/core/sessions.ts";
9 import { createUser } from "../src/core/users.ts";
10 import { serve, type Servers } from "../src/http/server.ts";
11 import { quietLogs } from "./helpers/globals.ts";
12
13 quietLogs();
14
15 let dataDir: string;
16 let app: App;
17 let servers: Servers;
18 let cookie = "";
19
20 const cells = process.env["FORGE_CELLS"];
21
22 beforeAll(async () => {
23 process.env["FORGE_CELLS"] = "off";
24 dataDir = await mkdtemp(join(tmpdir(), "forge-api-branches-timing-"));
25 app = createApp({ dataDir, port: 0 });
26 servers = await serve(app);
27 const made = await createUser(app.db, {
28 origin: "seed",
29 trusted: true,
30 username: "alice",
31 email: "[email protected]",
32 password: "a-good-passphrase",
33 adminIfFirst: true,
34 });
35 if (!made.ok) throw new Error(String(made.error));
36 const alice = made.value;
37 cookie = `forge_session=${createSession(app.db, alice.id).token}`;
38 const repo = await createRepo(app, alice, { name: "r", visibility: "public" });
39 if (!repo.ok) throw new Error(String(repo.error));
40 });
41 afterAll(async () => {
42 await servers.stop();
43 app.close();
44 if (cells === undefined) delete process.env["FORGE_CELLS"];
45 else process.env["FORGE_CELLS"] = cells;
46 await rm(dataDir, { recursive: true, force: true });
47 });
48
49 async function phases(path: string): Promise<{ status: number; names: string[]; body: string }> {
50 const res = await fetch(`${servers.url}${path}`, { headers: { cookie } });
51 const body = await res.text();
52 const names = (res.headers.get("server-timing") ?? "").split(", ").map((p) => p.split(";")[0]!);
53 return { status: res.status, names: names.slice(names.indexOf("app") + 1), body };
54 }
55
56 test("the branches list names session, git, render in order", async () => {
57 const got = await phases("/api/v1/repos/alice/r/branches");
58 expect(got.status).toBe(200);
59 expect(got.names).toEqual(["session", "git", "render"]);
60 expect(Array.isArray(JSON.parse(got.body))).toBe(true);
61 });
62
63 test("a missing repository is a 404 naming session", async () => {
64 const got = await phases("/api/v1/repos/alice/nope/branches");
65 expect(got.status).toBe(404);
66 expect(got.names).toEqual(["session"]);
67 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs