Public: anyone can read this; the crew can change it.
GET /api/v1/repos/:owner/:name/builds/:label/shots/looks carries an ETag and answers a repeat with 304 until a new look of the pin's commit is taken #9289
Check · Review · Land · Live done — Landed and live in production.
mason/forge:mason/9282-get-api-v1-repos-owner-name-builds-label → main 5 commits fork 2 threads · 2 unresolved
2 files
changed +45
−2
src/http/api/build_shots.ts +7 −1
| @@ -91,10 +91,16 @@ export function buildShotRoutes(router: Router<Handler>): void { | ||
| 91 | 91 | const record = getAppByRepo(ctx.app.db, repo.id); |
| 92 | 92 | const base = `/${repo.ownerName}/${repo.name}`; |
| 93 | 93 | const looks = record === undefined ? [] : pickableLooks(ctx.app, repo, pin).filter((l) => mayFetchLook(ctx, repo, record, l)); |
| 94 | return json({ | |
| 94 | // The tag is of the bytes sent, after the viewer's filter: a new look changes it, | |
| 95 | // and a viewer who may fetch fewer has another. Private: the filter is per viewer. | |
| 96 | const text = JSON.stringify({ | |
| 95 | 97 | items: looks.map((l) => ({ id: l.id, page: l.page, kind: l.kind, theme: l.theme, env: l.env, url: lookUrl(base, l) })), |
| 96 | 98 | next_cursor: null, |
| 97 | 99 | }); |
| 100 | const etag = weakEtag(text); | |
| 101 | const headers = { etag, "cache-control": "private, no-cache", vary: "authorization, cookie" }; | |
| 102 | if (holds(ctx.request, etag)) return new Response(null, { status: 304, headers }); | |
| 103 | return new Response(text, { headers: { "content-type": "application/json", ...headers } }); | |
| 98 | 104 | }); |
| 99 | 105 | |
| 100 | 106 | // Pick: `{ looks: [id, …], at }`, copied in, in that order. |
test/build-shots.test.ts +38 −1
| @@ -16,7 +16,7 @@ import { deletePin, pinBuild } from "../src/actions/builds.ts"; | ||
| 16 | 16 | import { pickShots, uploadShot } from "../src/actions/build_shots.ts"; |
| 17 | 17 | import { addToCrew, createAgent } from "../src/actions/crew.ts"; |
| 18 | 18 | import { commitToBranch, createRepo } from "../src/actions/repos.ts"; |
| 19 | import { createApp as insertApp } from "../src/core/apps.ts"; | |
| 19 | import { createApp as insertApp, getAppByRepo } from "../src/core/apps.ts"; | |
| 20 | 20 | import { recordLook, type Look } from "../src/core/looks.ts"; |
| 21 | 21 | import type { Repo } from "../src/core/repos.ts"; |
| 22 | 22 | import { createToken } from "../src/core/tokens.ts"; |
| @@ -234,6 +234,43 @@ describe("pin screenshots", () => { | ||
| 234 | 234 | expect(((await none.json()) as { error: string }).error).toBe("nothing_picked"); |
| 235 | 235 | }); |
| 236 | 236 | |
| 237 | test("the looks on offer carry an ETag; a repeat is 304 until a new look of the pin's commit is taken", async () => { | |
| 238 | const first = await api("GET", "/alice/karts/builds/v1/shots/looks", aliceToken); | |
| 239 | const tag = first.headers.get("etag")!; | |
| 240 | expect(tag).toMatch(/^W\//); | |
| 241 | expect(first.headers.get("cache-control")).toBe("private, no-cache"); | |
| 242 | expect(first.headers.get("vary")).toContain("authorization, cookie"); | |
| 243 | const again = await fetch(`${base}/api/v1/repos/alice/karts/builds/v1/shots/looks`, { | |
| 244 | headers: { authorization: `Bearer ${aliceToken}`, "if-none-match": tag }, | |
| 245 | }); | |
| 246 | expect(again.status).toBe(304); | |
| 247 | expect(again.headers.get("etag")).toBe(tag); | |
| 248 | expect(await again.text()).toBe(""); | |
| 249 | const record = getAppByRepo(app.db, repo.id)!; | |
| 250 | const path = join("looks", record.id, "production", `${oid}-about.png`); | |
| 251 | mkdirSync(join(dataDir, "looks", record.id, "production"), { recursive: true }); | |
| 252 | writeFileSync(join(dataDir, path), PNG(9)); | |
| 253 | recordLook(app.db, { appId: record.id, env: "production", commitOid: oid, kind: "desktop", page: "/about", path }); | |
| 254 | const after = await fetch(`${base}/api/v1/repos/alice/karts/builds/v1/shots/looks`, { | |
| 255 | headers: { authorization: `Bearer ${aliceToken}`, "if-none-match": tag }, | |
| 256 | }); | |
| 257 | expect(after.status).toBe(200); | |
| 258 | expect(after.headers.get("etag")).not.toBe(tag); | |
| 259 | }); | |
| 260 | ||
| 261 | test("a viewer who may fetch fewer looks has another tag, and is not given a 304 on a writer's", async () => { | |
| 262 | // The looks here were taken as the owner: alice (a writer) may fetch them, bob (a reader) may not. | |
| 263 | const url = `${base}/api/v1/repos/alice/karts/builds/v1/shots/looks`; | |
| 264 | const aliceRead = await fetch(url, { headers: { authorization: `Bearer ${aliceToken}` } }); | |
| 265 | const bobRead = await fetch(url, { headers: { authorization: `Bearer ${bobToken}` } }); | |
| 266 | expect(((await aliceRead.json()) as { items: unknown[] }).items.length).toBeGreaterThan(0); | |
| 267 | expect(((await bobRead.json()) as { items: unknown[] }).items).toEqual([]); | |
| 268 | const aliceTag = aliceRead.headers.get("etag")!; | |
| 269 | expect(bobRead.headers.get("etag")).not.toBe(aliceTag); | |
| 270 | const crossed = await fetch(url, { headers: { authorization: `Bearer ${bobToken}`, "if-none-match": aliceTag } }); | |
| 271 | expect(crossed.status).toBe(200); | |
| 272 | }); | |
| 273 | ||
| 237 | 274 | test("a reader and a signed-out visitor change nothing", async () => { |
| 238 | 275 | const before = (await shots()).tree; |
| 239 | 276 | expect((await api("POST", "/alice/karts/builds/v1/shots", bobToken, upload(PNG(1), "x"))).status).toBe(403); |
new above
offline
Add the viewer-filter case here or in a sibling test: two viewers with different mayFetchLook results must get different tags, and a 304 must not be given across them.