Public: anyone can read this; the crew can change it.
GET /api/v1/repos/:owner/:name/forks carries an ETag and answers a repeat with 304 until a fork is made, pushed or hidden, or the parent moves #9172
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/9168-get-api-v1-repos-owner-name-forks-carrie → main 3 commits fork 2 threads · 2 unresolved
2 files
changed +146
−4
src/http/api/upstream.ts +11 −4
| @@ -19,6 +19,7 @@ import { canAdmin, forkParentFor, visibleForks } from "../../core/repos.ts"; | ||
| 19 | 19 | import { notePhase, takeSessionTime, timePhase, timePhaseAsync } from "../../core/route_timing.ts"; |
| 20 | 20 | import { storageLimitMessage } from "../../core/storage.ts"; |
| 21 | 21 | import { getFollow } from "../../core/upstream_follow.ts"; |
| 22 | import { holds, weakEtag } from "../etag.ts"; | |
| 22 | 23 | import { loadRepo, pageParams, slicePage } from "../api.ts"; |
| 23 | 24 | import type { Handler } from "../context.ts"; |
| 24 | 25 | import { apiError, body, json, notFound, requireScope, respondAccess, str } from "../respond.ts"; |
| @@ -90,8 +91,10 @@ export function upstreamRoutes(router: Router<Handler>): void { | ||
| 90 | 91 | return ranked; |
| 91 | 92 | }); |
| 92 | 93 | const parent = forkParentFor(ctx.app.db, repo, ctx.auth.user); |
| 93 | return timePhase(ctx.request, "render", () => | |
| 94 | json({ | |
| 94 | // The tag is of the exact bytes sent: a fork made, pushed or hidden, or a parent that moved, changes | |
| 95 | // the body; limit and cursor choose the page. A 404 or bad page returned above carries none. | |
| 96 | return timePhase(ctx.request, "render", () => { | |
| 97 | const text = JSON.stringify({ | |
| 95 | 98 | parent: parent === undefined ? null : `${parent.ownerName}/${parent.name}`, |
| 96 | 99 | items: drifts.map(({ fork, drift, pushedAt }) => ({ |
| 97 | 100 | ...fork, |
| @@ -100,8 +103,12 @@ export function upstreamRoutes(router: Router<Handler>): void { | ||
| 100 | 103 | updatedAt: pushedAt, |
| 101 | 104 | })), |
| 102 | 105 | next_cursor, |
| 103 | }), | |
| 104 | ); | |
| 106 | }); | |
| 107 | const etag = weakEtag(text); | |
| 108 | const headers = { etag, "cache-control": "private, no-cache", vary: "authorization, cookie" }; | |
| 109 | if (holds(ctx.request, etag)) return new Response(null, { status: 304, headers }); | |
| 110 | return new Response(text, { headers: { "content-type": "application/json", ...headers } }); | |
| 111 | }); | |
| 105 | 112 | }); |
| 106 | 113 | |
| 107 | 114 | router.add("POST", "/api/v1/repos/:owner/:name/forks", async (ctx) => { |
test/forks-etag.test.ts added +135
| @@ -0,0 +1,135 @@ | ||
| 1 | /** GET …/forks carries a weak ETag and answers a repeat with 304 until a fork or the parent moves (#9168). */ | |
| 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 { commitToBranch, createRepo, deleteRepo, forkRepo, updateRepoSettings } from "../src/actions/repos.ts"; | |
| 7 | import { createApp, type App } from "../src/app.ts"; | |
| 8 | import { createUser } from "../src/core/users.ts"; | |
| 9 | import { serve, type Servers } from "../src/http/server.ts"; | |
| 10 | import { promote } from "./helpers/members.ts"; | |
| 11 | ||
| 12 | let dataDir = ""; | |
| 13 | let app: App, servers: Servers; | |
| 14 | let alice: any, bob: any, carol: any, parent: any, bobFork: any; | |
| 15 | const URL_ = () => `${servers.url}/api/v1/repos/alice/app/forks`; | |
| 16 | const get = (q = "", headers: Record<string, string> = {}) => fetch(`${URL_()}${q}`, { headers }); | |
| 17 | const tagOf = async (q = "") => { | |
| 18 | const r = await get(q); | |
| 19 | await r.text(); | |
| 20 | return r.headers.get("etag")!; | |
| 21 | }; | |
| 22 | async function changed(before: string, what: string): Promise<string> { | |
| 23 | const res = await get("", { "if-none-match": before }); | |
| 24 | expect(res.status, what).toBe(200); | |
| 25 | await res.text(); | |
| 26 | const tag = res.headers.get("etag")!; | |
| 27 | expect(tag, what).not.toBe(before); | |
| 28 | return tag; | |
| 29 | } | |
| 30 | const mk = async (username: string) => { | |
| 31 | const r = await createUser(app.db, { origin: "seed", trusted: true, username, email: `${username}@f.dev`, password: "a-good-passphrase", adminIfFirst: username === "alice" }); | |
| 32 | if (!r.ok) throw new Error("user"); | |
| 33 | promote(app.db, r.value); | |
| 34 | return r.value; | |
| 35 | }; | |
| 36 | ||
| 37 | beforeAll(async () => { | |
| 38 | dataDir = await mkdtemp(join(tmpdir(), "forge-forksetag-")); | |
| 39 | app = createApp({ dataDir, port: 0 }); | |
| 40 | servers = await serve(app); | |
| 41 | alice = await mk("alice"); | |
| 42 | bob = await mk("bob"); | |
| 43 | carol = await mk("carol"); | |
| 44 | const made = await createRepo(app, alice, { name: "app", visibility: "public" }); | |
| 45 | if (!made.ok) throw new Error("repo"); | |
| 46 | parent = made.value; | |
| 47 | await commitToBranch(app, alice, parent, { message: "one", files: [{ path: "a.txt", content: "x\n" }] }); | |
| 48 | const f = await forkRepo(app, bob, parent); | |
| 49 | if (!f.ok) throw new Error("fork"); | |
| 50 | bobFork = f.value; | |
| 51 | }); | |
| 52 | afterAll(async () => { | |
| 53 | servers.stop(); | |
| 54 | app.close(); | |
| 55 | await rm(dataDir, { recursive: true, force: true }); | |
| 56 | }); | |
| 57 | ||
| 58 | test("200 carries a weak tag, private no-cache and vary; a repeat is a bodiless 304 with the floor headers", async () => { | |
| 59 | const res = await get(); | |
| 60 | expect(res.status).toBe(200); | |
| 61 | const tag = res.headers.get("etag")!; | |
| 62 | expect(tag).toMatch(/^W\//); | |
| 63 | expect(res.headers.get("cache-control")).toBe("private, no-cache"); | |
| 64 | expect(res.headers.get("vary")).toContain("authorization, cookie"); | |
| 65 | await res.text(); | |
| 66 | const again = await get("", { "if-none-match": tag }); | |
| 67 | expect(again.status).toBe(304); | |
| 68 | expect(again.headers.get("etag")).toBe(tag); | |
| 69 | expect(again.headers.get("cache-control")).toBe("private, no-cache"); | |
| 70 | expect(again.headers.get("vary")).toContain("authorization, cookie"); | |
| 71 | expect(again.headers.get("x-content-type-options")).toBe("nosniff"); | |
| 72 | expect(again.headers.get("content-security-policy")).not.toBeNull(); | |
| 73 | expect(await again.text()).toBe(""); | |
| 74 | }); | |
| 75 | ||
| 76 | test("a fork pushed, a fork made, the parent moving and a fork deleted each change the tag", async () => { | |
| 77 | let tag = await tagOf(); | |
| 78 | await commitToBranch(app, bob, bobFork, { message: "two", files: [{ path: "b.txt", content: "y\n" }] }); | |
| 79 | tag = await changed(tag, "fork pushed"); | |
| 80 | const c = await forkRepo(app, carol, parent); | |
| 81 | if (!c.ok) throw new Error("fork"); | |
| 82 | tag = await changed(tag, "fork made"); | |
| 83 | await commitToBranch(app, alice, parent, { message: "three", files: [{ path: "c.txt", content: "z\n" }] }); | |
| 84 | tag = await changed(tag, "parent moved"); | |
| 85 | const gone = await deleteRepo(app, carol, c.value); | |
| 86 | expect(gone.ok).toBe(true); | |
| 87 | await changed(tag, "fork deleted"); | |
| 88 | }); | |
| 89 | ||
| 90 | test("limit and cursor make different tags", async () => { | |
| 91 | const d = await forkRepo(app, alice, parent, "mine"); | |
| 92 | expect(d.ok).toBe(true); | |
| 93 | const all = await tagOf(); | |
| 94 | const one = await tagOf("?limit=1"); | |
| 95 | expect(one).not.toBe(all); | |
| 96 | const first = await (await get("?limit=1")).json() as { next_cursor: string | null }; | |
| 97 | expect(first.next_cursor).not.toBeNull(); | |
| 98 | expect(await tagOf(`?limit=1&cursor=${first.next_cursor}`)).not.toBe(one); | |
| 99 | }); | |
| 100 | ||
| 101 | test("a viewer who cannot see a private fork never shares a tag with one who can", async () => { | |
outdated
Compare two different viewers (bob authenticated vs anonymous) after the fork is private, not one anonymous viewer before and after. Also assert bob's tag as If-None-Match gets 200 for anonymous. |
||
| 102 | const minted = await fetch(`${servers.url}/api/v1/tokens`, { | |
| 103 | method: "POST", | |
| 104 | headers: { "content-type": "application/json" }, | |
| 105 | body: JSON.stringify({ username: "bob", password: "a-good-passphrase", name: "cli", scope: "ro" }), | |
| 106 | }); | |
| 107 | const bearer = ((await minted.json()) as { token: string }).token; | |
| 108 | const r = await updateRepoSettings(app, bob, bobFork, { visibility: "private" }); | |
| 109 | expect(r.ok).toBe(true); | |
| 110 | const owner = await get("", { authorization: `Bearer ${bearer}` }); | |
| 111 | const stranger = await get(); | |
| 112 | expect(owner.status).toBe(200); | |
| 113 | expect(stranger.status).toBe(200); | |
| 114 | const ownerTag = owner.headers.get("etag")!; | |
| 115 | const strangerTag = stranger.headers.get("etag")!; | |
| 116 | expect(ownerTag).not.toBe(strangerTag); | |
| 117 | expect(await owner.text()).toContain('"bob"'); | |
| 118 | expect(await stranger.text()).not.toContain('"bob"'); | |
| 119 | // The owner's tag buys a stranger nothing. | |
| 120 | const replay = await get("", { "if-none-match": ownerTag }); | |
| 121 | expect(replay.status).toBe(200); | |
| 122 | expect(await replay.text()).not.toContain('"bob"'); | |
| 123 | // The owner's own repeat is still a 304. | |
| 124 | const again = await get("", { authorization: `Bearer ${bearer}`, "if-none-match": ownerTag }); | |
| 125 | expect(again.status).toBe(304); | |
| 126 | }); | |
| 127 | ||
| 128 | test("a 404 and a bad page parameter carry no ETag", async () => { | |
| 129 | const missing = await fetch(`${servers.url}/api/v1/repos/alice/nope/forks`); | |
| 130 | expect(missing.status).toBe(404); | |
| 131 | expect(missing.headers.get("etag")).toBeNull(); | |
| 132 | const bad = await get("?limit=abc"); | |
| 133 | expect(bad.status).toBeGreaterThanOrEqual(400); | |
| 134 | expect(bad.headers.get("etag")).toBeNull(); | |
| 135 | }); |
new above
offline
Import deleteRepo at the top with the other ../src/actions/repos.ts imports.