trevato/forge

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

More
Forks1g fMapg aRuns12g rEventsFlows

GET /api/v1/repos/:owner/:name/labels carries an ETag and answers a repeat with 304 until a label or its open count changes #9170

Merged scoutcrew opened this

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

ConversationFiles
Merged

scout/forge:scout/9164-get-api-v1-repos-owner-name-labels-carri → main 2 commits fork

  • Merged. scout/9164-get-api-v1-repos-owner-name-labels-carri was merged into main as 8e76b99.

Review

Sign in to review this change.
2 files changed +120 −2
src/http/api/labels.ts +10 −2
@@ -10,12 +10,13 @@ import { labelsWithOpen, listLabels } from "../../core/issues.ts";
1010 import { notePhase, takeSessionTime, timePhase } from "../../core/route_timing.ts";
1111 import { canWrite } from "../../core/repos.ts";
1212 import { loadRepo, pageOf, pageParams } from "../api.ts";
13 import { holds, weakEtag } from "../etag.ts";
1314 import type { Handler } from "../context.ts";
1415 import { body, json, notFound, requireScope, respondAccess, str } from "../respond.ts";
1516 import type { Router } from "../router.ts";
1617
1718 export function labelsRoutes(router: Router<Handler>): void {
18 router.add("GET", "/api/v1/repos/:owner/:name/labels", (ctx) => {
19 router.add("GET", "/api/v1/repos/:owner/:name/labels", async (ctx) => {
1920 notePhase(ctx.request, "session", takeSessionTime(ctx.request));
2021 const repo = loadRepo(ctx);
2122 if (!repo) return notFound();
@@ -25,7 +26,14 @@ export function labelsRoutes(router: Router<Handler>): void {
2526 const rows = timePhase(ctx.request, "labels", () =>
2627 labelsWithOpen(ctx.app.db, repo.id, canWrite(ctx.app.db, repo, ctx.auth.user)),
2728 );
28 return timePhase(ctx.request, "render", () => pageOf(rows, page));
29 const res = timePhase(ctx.request, "render", () => pageOf(rows, page));
30 if (res.status !== 200) return res;
31 // The tag is over the rendered page, which already carries this reader's open counts.
32 const text = await res.text();
33 const etag = weakEtag(text);
34 const headers = { etag, "cache-control": "private, no-cache", vary: "authorization, cookie" };
35 if (holds(ctx.request, etag)) return new Response(null, { status: 304, headers });
36 return new Response(text, { headers: { "content-type": res.headers.get("content-type") ?? "application/json", ...headers } });
2937 });
3038
3139 router.add("POST", "/api/v1/repos/:owner/:name/labels", async (ctx) => {
test/labels-etag.test.ts added +110
@@ -0,0 +1,110 @@
1 import { afterAll, beforeAll, describe, expect, test } from "bun:test";
2 import { mkdtemp, rm } from "node:fs/promises";
3 import { tmpdir } from "node:os";
4 import { join } from "node:path";
5 import { createApp, type App } from "../src/app.ts";
6 import { serve, type Servers } from "../src/http/server.ts";
7 import { seedAdmin } from "./helpers/admin.ts";
8
9 describe("labels ETag", () => {
10 let dataDir: string;
11 let app: App;
12 let servers: Servers;
13 let base: string;
14 let token = "";
15 const send = (path: string, method = "GET", bodyObj?: unknown, headers: Record<string, string> = {}, auth = true) =>
16 fetch(`${base}${path}`, {
17 method,
18 headers: {
19 ...(auth ? { authorization: `Bearer ${token}` } : {}),
20 "content-type": "application/json",
21 ...headers,
22 },
23 ...(bodyObj === undefined ? {} : { body: JSON.stringify(bodyObj) }),
24 });
25 const L = "/api/v1/repos/dev/lib/labels";
26 const I = "/api/v1/repos/dev/lib/issues";
27 const tagOf = async (auth = true) => (await send(L, "GET", undefined, {}, auth)).headers.get("etag")!;
28
29 beforeAll(async () => {
30 dataDir = await mkdtemp(join(tmpdir(), "forge-labels-etag-"));
31 app = createApp({ dataDir, port: 0 });
32 servers = await serve(app);
33 base = servers.url;
34 await seedAdmin(app, { username: "dev", email: "[email protected]", password: "etag-pass-1" });
35 token = await fetch(`${base}/api/v1/tokens`, {
36 method: "POST",
37 headers: { "content-type": "application/json" },
38 body: JSON.stringify({ adminConfirm: true, username: "dev", password: "etag-pass-1", name: "c", scope: "admin" }),
39 })
40 .then((r) => r.json())
41 .then((j) => (j as { token: string }).token);
42 await send("/api/v1/repos", "POST", { name: "lib", visibility: "public" });
43 await send(L, "POST", { name: "bug", color: "d73a4a" });
44 await send(I, "POST", { title: "One", body: "b" });
45 });
46
47 afterAll(async () => {
48 await servers.stop();
49 await rm(dataDir, { recursive: true, force: true });
50 });
51
52 test("200 with headers, then 304 on a repeat with the floor headers", async () => {
53 const first = await send(L);
54 expect(first.status).toBe(200);
55 const tag = first.headers.get("etag")!;
56 expect(tag).toMatch(/^W\//);
57 expect(first.headers.get("cache-control")).toBe("private, no-cache");
58 expect(first.headers.get("vary")).toContain("authorization, cookie");
59 await first.text();
60 const again = await send(L, "GET", undefined, { "if-none-match": tag });
61 expect(again.status).toBe(304);
62 expect(again.headers.get("etag")).toBe(tag);
63 expect(again.headers.get("cache-control")).toBe("private, no-cache");
64 expect(again.headers.get("x-content-type-options")).toBe("nosniff");
65 expect(await again.text()).toBe("");
66 });
67
68 test("add, rename, remove, and an issue opening or closing move the tag", async () => {
69 let tag = await tagOf();
70 const moved = async (what: string) => {
71 const r = await send(L, "GET", undefined, { "if-none-match": tag });
72 expect(r.status, what).toBe(200);
73 const next = r.headers.get("etag")!;
74 expect(next, what).not.toBe(tag);
75 tag = next;
76 };
77 const made = await send(L, "POST", { name: "docs", color: "0075ca" });
78 const id = ((await made.json()) as { id: string }).id;
79 await moved("add");
80 expect((await send(`${L}/${id}`, "PATCH", { name: "documentation" })).status).toBe(200);
81 await moved("rename");
82 await send(`${I}/1/labels`, "PUT", { labels: ["bug"] });
83 await moved("label an issue");
84 await send(`${I}/1`, "PATCH", { state: "closed", note: "done" });
85 await moved("close");
86 await send(`${I}/1`, "PATCH", { state: "open" });
87 await moved("reopen");
88 expect((await send(`${L}/${id}`, "DELETE")).status).toBe(204);
89 await moved("remove");
90 });
91
92 test("a writer's tag never matches a reader's body once a sealed issue differs their counts", async () => {
93 await send(I, "POST", { title: "Hidden", body: "x", sealed: true });
94 await send(`${I}/2/labels`, "PUT", { labels: ["bug"] });
95 const writer = await tagOf();
96 const reader = await tagOf(false);
97 expect(writer).not.toBe(reader);
98 const out = await send(L, "GET", undefined, { "if-none-match": writer }, false);
99 expect(out.status).toBe(200);
100 });
101
102 test("a 404 or a bad page parameter carries no tag", async () => {
103 const missing = await send("/api/v1/repos/dev/nope/labels");
104 expect(missing.status).toBe(404);
105 expect(missing.headers.get("etag")).toBeNull();
106 const bad = await send(`${L}?limit=abc`);
107 expect(bad.status).toBeGreaterThanOrEqual(400);
108 expect(bad.headers.get("etag")).toBeNull();
109 });
110 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs