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

A connected origin's repository browse pages name their Server-Timing phases: session, the origin read, render #3607

Merged opened this

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

Merged

scout/forge:scout/3604-a-connected-origin-s-repository-browse-p → main 1 commit fork 1 thread · 1 unresolved

  • Merged. scout/3604-a-connected-origin-s-repository-browse-p was merged into main as b218d42.

Review

Sign in to review this change.
2 files changed +98 −4
src/web/pages/browse.ts +23 −4
@@ -28,6 +28,7 @@
2828 import { mirrorJobFor, startMirrorJob } from "../../actions/origins.ts";
2929 import { forkOrigin } from "../../actions/contribute.ts";
3030 import { getOriginFor } from "../../core/origins.ts";
31 import { notePhase, takeSessionTime } from "../../core/route_timing.ts";
3132 import { isSafeOid, isSafeRef, isSafeTreePath } from "../../git/guard.ts";
3233 import type { Ctx } from "../../http/context.ts";
3334 import type { MirrorJob, MirrorStage } from "../../origins/jobs.ts";
@@ -325,6 +326,7 @@ function sourceQuery(
325326 * able to spend somebody's GitHub credential just because it can read a repo.
326327 */
327328 async function browseScope(ctx: Ctx): Promise<RepoScope | Response> {
329 notePhase(ctx.request, "session", takeSessionTime(ctx.request));
328330 const denied = accountOnly(ctx);
329331 if (denied) return denied;
330332 const originId = ctx.params.id ?? "";
@@ -333,7 +335,9 @@ async function browseScope(ctx: Ctx): Promise<RepoScope | Response> {
333335 if (origin === undefined) return notFoundPage(ctx);
334336 const source = sourceFor(ctx.app, ctx.auth.user, { originId, fullName });
335337 if (!source.ok) return notFoundPage(ctx);
338 const reading = performance.now();
336339 const scope = await originScope(origin, source.value);
340 notePhase(ctx.request, "repo", performance.now() - reading);
337341 if (scope === undefined) return unreachablePage(ctx, origin.webUrl);
338342 return scope;
339343 }
@@ -400,10 +404,18 @@ export async function browseHome(ctx: Ctx): Promise<Response> {
400404 const scope = await browseScope(ctx);
401405 if (scope instanceof Response) return scope;
402406 const ref = refOf(ctx, scope);
407 const reading = performance.now();
403408 const [refs, entries] = await Promise.all([
404 refSetFrom(scope.source),
405 scope.source.tree(ref, ""),
409 refSetFrom(scope.source).then((r) => {
410 notePhase(ctx.request, "refs", performance.now() - reading);
411 return r;
412 }),
413 scope.source.tree(ref, "").then((r) => {
414 notePhase(ctx.request, "tree", performance.now() - reading);
415 return r;
416 }),
406417 ]);
418 const rendering = performance.now();
407419 const body = await renderCode(scope, {
408420 refs,
409421 ref,
@@ -411,7 +423,7 @@ export async function browseHome(ctx: Ctx): Promise<Response> {
411423 entries: entries.ok ? entries.value : [],
412424 rail: aboutRail(scope),
413425 });
414 return browsePage(ctx, scope, {
426 const response = browsePage(ctx, scope, {
415427 title: titleFor(scope),
416428 header: scopeHeader(ctx, scope, "code", {
417429 branchCount: refs.branches.length,
@@ -419,6 +431,8 @@ export async function browseHome(ctx: Ctx): Promise<Response> {
419431 body: html`${body}${codeMount(scope, ref)}${finderDialog()}`,
420432 scriptJs: codeJs,
421433 });
434 notePhase(ctx.request, "render", performance.now() - rendering);
435 return response;
422436 }
423437
424438 // GET /origins/:id/r/:owner/:name/paths/:ref
@@ -426,13 +440,18 @@ export async function browsePaths(ctx: Ctx): Promise<Response> {
426440 const scope = await browseScope(ctx);
427441 if (scope instanceof Response) return notFound();
428442 const ref = refOf(ctx, scope);
443 const reading = performance.now();
429444 const listed = await scope.source.paths(ref);
445 notePhase(ctx.request, "paths", performance.now() - reading);
430446 if (!listed.ok) return notFound();
431 return json(
447 const rendering = performance.now();
448 const response = json(
432449 { ref, paths: listed.value.paths, truncated: listed.value.truncated },
433450 200,
434451 { "cache-control": "private, max-age=30" },
435452 );
453 notePhase(ctx.request, "render", performance.now() - rendering);

This render times only the JSON.stringify of a small object, so it is near zero. Consider dropping it from the paths route.

454 return response;
436455 }
437456
438457 // GET /origins/:id/r/:owner/:name/branches
test/browse-origin-timing.test.ts added +75
@@ -0,0 +1,75 @@
1 /** A connected origin's repository browse pages name their phases in Server-Timing. */
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 { serve, type Servers } from "../src/http/server.ts";
8 import { listOrigins } from "../src/core/origins.ts";
9 import { getUserByName } from "../src/core/users.ts";
10 import { forgetProviders } from "../src/origins/providers/index.ts";
11 import { clearViewCache } from "../src/view/cache.ts";
12 import { quietLogs } from "./helpers/globals.ts";
13 import { newFakeState, serveFakeForge, type FakeForge } from "./fake-origin.ts";
14 import { adminCookie } from "./helpers/admin.ts";
15
16 quietLogs();
17
18 const TOKEN = "octo-origin-secret-7c31";
19 const SHA = "a".repeat(40);
20
21 let dataDir = "";
22 let app: App, servers: Servers, fake: FakeForge;
23 let cookie = "", browse = "";
24
25 beforeAll(async () => {
26 dataDir = await mkdtemp(join(tmpdir(), "forge-borigtime-"));
27 const state = newFakeState("forgejo", "octo", TOKEN);
28 state.repos.push({
29 id: 41,
30 fullName: "octo/widgets",
31 cloneUrl: "http://127.0.0.1:1/octo/widgets.git",
32 htmlUrl: "http://127.0.0.1:1/octo/widgets",
33 defaultBranch: "main",
34 });
35 state.files.set("octo/widgets", new Map([["README.md", "# widgets\n"]]));
36 state.branches.set("octo/widgets", [{ name: "main", sha: SHA }]);
37 fake = serveFakeForge(state);
38 app = createApp({ dataDir, port: 0 });
39 servers = await serve(app);
40 delete process.env.FORGE_GITHUB_CLIENT_ID;
41 cookie = await adminCookie(app, { username: "alice", email: "[email protected]", password: "a-long-web-passphrase" });
42 forgetProviders(dataDir);
43 const connected = await fetch(`${servers.url}/settings/origins/token`, {
44 method: "POST",
45 headers: { cookie, origin: new URL(servers.url).origin },
46 body: new URLSearchParams({ host: fake.url, token: TOKEN }),
47 redirect: "manual",
48 });
49 expect(connected.status).toBe(303);
50 const id = listOrigins(app.db, getUserByName(app.db, "alice")!.id)[0]!.id;
51 browse = `/origins/${id}/r/octo/widgets`;
52 clearViewCache();
53 });
54
55 afterAll(async () => {
56 fake.stop();
57 await servers.stop();
58 await rm(dataDir, { recursive: true, force: true });
59 });
60
61 const names = (r: Response): string[] =>
62 (r.headers.get("server-timing") ?? "").split(",").map((p) => p.trim().split(/[;=]/)[0]!);
63
64 test("the repository page names session, the origin reads, and render", async () => {
65 const r = await fetch(`${servers.url}${browse}`, { headers: { cookie } });
66 expect(r.status).toBe(200);
67 expect(names(r)).toEqual(expect.arrayContaining(["session", "repo", "refs", "tree", "render"]));
68 });
69
70 test("the paths page names session, the origin reads, and render", async () => {
71 clearViewCache();
72 const r = await fetch(`${servers.url}${browse}/paths/main`, { headers: { cookie } });
73 expect(r.status).toBe(200);
74 expect(names(r)).toEqual(expect.arrayContaining(["session", "repo", "paths", "render"]));
75 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs