trevato/forge

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

More
Forks1g fMapg aRunsg rEventsFlows

GET /api/v1/repos/:owner/:name/following/proposals carries an ETag and answers a repeat with 304 until a proposal to the parent changes #9297

Merged masoncrew opened this

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

ConversationFiles
Merged

mason/forge:mason/9246-get-api-v1-repos-owner-name-following-pr → main 3 commits fork

  • Merged. mason/9246-get-api-v1-repos-owner-name-following-pr was merged into main as a95e5c5.

Review

Sign in to review this change.
2 files changed +152 −2
src/http/api/following.ts +2 −2
@@ -23,7 +23,7 @@ import { apiError, body, json, notFound, requireScope, respondAccess } from "../
2323 import type { Router } from "../router.ts";
2424
2525 /**
26 * The standing, tagged by its own bytes: a push to the fork or to the parent's
26 * A body tagged by its own bytes (the standing, or the proposals to the parent): a push to the fork or to the parent's
2727 * default branch, or a severed chain, changes them, and a viewer who cannot see
2828 * the parent is rendered a different body, so never shares a tag with one who can.
2929 */
@@ -211,7 +211,7 @@ export function followingRoutes(router: Router<Handler>): void {
211211 const items = await timePhaseAsync(ctx.request, "read", () =>
212212 forgeProposals(ctx.app, repo, Date.now(), ctx.auth.user !== undefined),
213213 );
214 return timePhase(ctx.request, "render", () => json({ items }));
214 return timePhase(ctx.request, "render", () => tagged(ctx.request, { items }));
215215 });
216216
217217 router.add(
test/following-proposals-etag.test.ts added +150
@@ -0,0 +1,150 @@
1 /** GET …/following/proposals carries an ETag and answers a repeat with 304 (#9246). */
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, forkRepo } from "../src/actions/repos.ts";
8 import { createToken } from "../src/core/tokens.ts";
9 import { createUser, type User } from "../src/core/users.ts";
10 import { serve, type Servers } from "../src/http/server.ts";
11 import { promote } from "./helpers/members.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 bobTok = "";
20 let bobId = "";
21 const PATH = "/api/v1/repos/bob/up/following/proposals";
22 const get = (path: string, headers: Record<string, string> = {}) =>
23 fetch(`${servers.url}${path}`, { headers });
24 const bob = (headers: Record<string, string> = {}) =>
25 get(PATH, { authorization: `Bearer ${bobTok}`, ...headers });
26 const FAR = Date.now() + 10 * 365 * 86_400_000; // checked "recently": no ask of the parent
27 const add = (branch: string, number: number, state: string) =>
28 app.db
29 .query(
30 `INSERT INTO upstream_proposal (repo_id, branch, number, title, source, state, reachable, checked_at, created_at)
31 VALUES (?1, ?2, ?3, 't', 's', ?4, 1, ?5, ?5)`,
32 )
33 .run(bobId, branch, number, state, FAR);
34 const tagOf = async () => {
35 const res = await bob();
36 expect(res.status).toBe(200);
37 await res.text();
38 return res.headers.get("etag")!;
39 };
40
41 beforeAll(async () => {
42 dataDir = await mkdtemp(join(tmpdir(), "forge-proposals-etag-"));
43 app = createApp({ dataDir, port: 0 });
44 servers = await serve(app);
45 const mk = async (username: string, first: boolean) => {
46 const made = await createUser(app.db, {
47 origin: "seed",
48 trusted: true,
49 username,
50 email: `${username}@forge.local`,
51 password: "a-good-passphrase",
52 adminIfFirst: first,
53 });
54 if (!made.ok) throw new Error(String(made.error));
55 return made.value;
56 };
57 const alice: User = await mk("alice", true);
58 const b: User = await mk("bob", false);
59 promote(app.db, alice, b);
60 const up = await createRepo(app, alice, { name: "up", visibility: "public" });
61 if (!up.ok) throw new Error("repo");
62 const solo = await createRepo(app, alice, { name: "solo", visibility: "private" });
63 if (!solo.ok) throw new Error("repo");
64 const fork = await forkRepo(app, b, up.value);
65 if (!fork.ok) throw new Error(JSON.stringify(fork.error));
66 bobId = fork.value.id;
67 await Bun.write(
68 join(dataDir, "parent.json"),
69 JSON.stringify({
70 upstream: "https://parent.invalid",
71 proposal: { child: "kid", token: "tok-abcdefgh" },
72 forgeCode: { repo: "bob/up", upstream: "root/forge", mode: "tell-me", tip: "a".repeat(40), chain: [] },
73 }),
74 );
75 bobTok = createToken(app.db, b.id, "b", "ro").token;
76 });
77 afterAll(async () => {
78 await servers.stop();
79 app.close();
80 await rm(dataDir, { recursive: true, force: true });
81 });
82
83 test("a weak tag over the body, private no-cache, vary", async () => {
84 add("b1", 1, "open");
85 const res = await bob();
86 expect(res.status).toBe(200);
87 expect(res.headers.get("etag")).toMatch(/^W\/"/);
88 expect(res.headers.get("cache-control")).toBe("private, no-cache");
89 expect(res.headers.get("vary")).toContain("authorization, cookie");
90 expect(res.headers.get("content-type")).toContain("application/json");
91 expect(((await res.json()) as { items: unknown[] }).items.length).toBe(1);
92 });
93
94 test("a repeat is a bodiless 304 with the same headers and the floor", async () => {
95 const tag = await tagOf();
96 const again = await bob({ "if-none-match": tag });
97 expect(again.status).toBe(304);
98 expect(await again.text()).toBe("");
99 expect(again.headers.get("etag")).toBe(tag);
100 expect(again.headers.get("cache-control")).toBe("private, no-cache");
101 expect(again.headers.get("vary")).toContain("authorization, cookie");
102 expect(again.headers.get("x-content-type-options")).toBe("nosniff");
103 expect(again.headers.get("referrer-policy")).not.toBeNull();
104 expect(again.headers.get("content-security-policy")).not.toBeNull();
105 });
106
107 test("the tag moves when a proposal is made, answered, landed or withdrawn", async () => {
108 let tag = await tagOf();
109 const step = async (change: () => void) => {
110 change();
111 const res = await bob({ "if-none-match": tag });
112 expect(res.status).toBe(200);
113 const next = res.headers.get("etag")!;
114 expect(next).not.toBe(tag);
115 await res.text();
116 tag = next;
117 };
118 await step(() => add("b2", 2, "open"));
119 await step(() => app.db.query("UPDATE upstream_proposal SET state = 'merged' WHERE number = 2").run());
120 await step(() => app.db.query("UPDATE upstream_proposal SET state = 'closed' WHERE number = 1").run());
121 await step(() => app.db.query("DELETE FROM upstream_proposal WHERE number = 1").run());
122 });
123
124 test("viewers shown different bodies never share a tag", async () => {
125 const signedIn = await tagOf();
126 const anon = await get(PATH);
127 expect(anon.status).toBe(200);
128 await anon.text();
129 // Same bytes share a tag; the vary header keeps caches apart. Different bytes must not.
130 app.db.query("UPDATE upstream_proposal SET title = 'other' WHERE number = 2").run();
131 expect(await tagOf()).not.toBe(signedIn);
132 });
133
134 test("a missing repository answers as before with no tag, even for *", async () => {
135 const res = await get("/api/v1/repos/bob/nope/following/proposals", {
136 authorization: `Bearer ${bobTok}`,
137 "if-none-match": "*",
138 });
139 expect(res.status).toBe(404);
140 expect(res.headers.get("etag")).toBeNull();
141 });
142
143 test("a repository the viewer cannot see answers 404 with no tag, even for *", async () => {
144 const res = await get("/api/v1/repos/alice/solo/following/proposals", {
145 authorization: `Bearer ${bobTok}`,
146 "if-none-match": "*",
147 });
148 expect(res.status).toBe(404);
149 expect(res.headers.get("etag")).toBeNull();
150 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs