trevato/forge

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

More
Forks1g fMapg aRuns18g rEventsFlows

GET /api/v1/conversations carries an ETag and answers a repeat with 304 until one of the viewer's conversations changes #9298

Merged scoutcrew opened this

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

ConversationFiles
Merged

scout/forge:scout/9247-get-api-v1-conversations-carries-an-etag → main 3 commits fork

  • Merged. scout/9247-get-api-v1-conversations-carries-an-etag was merged into main as 8dcad6e.

Review

Sign in to review this change.
2 files changed +115 −1
src/http/api/conversation_talk.ts +7 −1
@@ -31,6 +31,7 @@ import type { User } from "../../core/users.ts";
3131 import { worksOf } from "../../core/talk_work.ts";
3232 import { fileQueued, queuedOf, turnRunning } from "../../crew/conversation.ts";
3333 import { crossOrigin, type Handler } from "../context.ts";
34 import { holds, weakEtag } from "../etag.ts";
3435 import { readBounded } from "../limits.ts";
3536 import { apiError, boundedBody, forbidden, json, unauthorized } from "../respond.ts";
3637 import type { Router } from "../router.ts";
@@ -178,7 +179,12 @@ export function conversationTalkRoutes(router: Router<Handler>): void {
178179 router.add("GET", "/api/v1/conversations", (ctx) => {
179180 const user = ctx.auth.user;
180181 if (user === undefined) return unauthorized();
181 return json({ conversations: listConversations(ctx.app.db, user.id, { archived: ctx.url.searchParams.get("archived") === "1" }) });
182 // The tag is over the viewer and the rendered body, so any create, rename, archive or turn moves it; the 304 is after the access check.
183 const text = JSON.stringify({ conversations: listConversations(ctx.app.db, user.id, { archived: ctx.url.searchParams.get("archived") === "1" }) });
184 const etag = weakEtag(`${user.id}\n${text}`);
185 const headers = { etag, "cache-control": "private, no-cache", vary: "authorization, cookie" };
186 if (holds(ctx.request, etag)) return new Response(null, { status: 304, headers });
187 return new Response(text, { status: 200, headers: { "content-type": "application/json", ...headers } });
182188 });
183189
184190 router.add("POST", "/api/v1/conversations", async (ctx) => {
test/conversations-etag.test.ts added +108
@@ -0,0 +1,108 @@
1 /** GET /api/v1/conversations carries a weak ETag and answers a repeat with 304 until a conversation changes (#9247). */
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 { appendTurn } from "../src/core/conversations.ts";
8 import { createToken } from "../src/core/tokens.ts";
9 import { createUser } from "../src/core/users.ts";
10 import { serve, type Servers } from "../src/http/server.ts";
11 import { quietLogs } from "./helpers/globals.ts";
12
13 quietLogs();
14
15 let dataDir = "";
16 let app: App, servers: Servers;
17 const tok: Record<string, string> = {};
18 const ids: Record<string, string> = {};
19 const call = (method: string, path: string, token: string | undefined, extra: Record<string, string> = {}, body?: unknown) =>
20 fetch(`${servers.url}/api/v1/${path}`, {
21 method,
22 headers: { ...(token ? { authorization: `Bearer ${token}` } : {}), ...(body ? { "content-type": "application/json" } : {}), ...extra },
23 ...(body ? { body: JSON.stringify(body) } : {}),
24 });
25 const tagOf = async (path = "conversations", token = tok["ada"]!) => {
26 const r = await call("GET", path, token);
27 expect(r.status).toBe(200);
28 await r.text();
29 return r.headers.get("etag")!;
30 };
31 const stale = async (tag: string, path = "conversations") => {
32 const r = await call("GET", path, tok["ada"]!, { "if-none-match": tag });
33 await r.text();
34 return r.status;
35 };
36
37 beforeAll(async () => {
38 dataDir = await mkdtemp(join(tmpdir(), "forge-conv-etag-"));
39 app = createApp({ dataDir, port: 0 });
40 servers = await serve(app);
41 for (const [username, first] of [["ada", true], ["bob", false]] as const) {
42 const made = await createUser(app.db, { origin: "seed", trusted: true, username, email: `${username}@forge.local`, password: "a-good-passphrase", adminIfFirst: first });
43 if (!made.ok) throw new Error(String(made.error));
44 ids[username] = made.value.id;
45 tok[username] = createToken(app.db, made.value.id, username, "admin").token;
46 }
47 });
48
49 afterAll(async () => {
50 servers?.stop();
51 app?.close();
52 await rm(dataDir, { recursive: true, force: true });
53 });
54
55 test("200 carries a weak etag and the cache headers; a repeat is a bodiless 304 with the floor", async () => {
56 expect((await call("POST", "conversations", tok["ada"]!, {}, { title: "one" })).status).toBe(201);
57 const r = await call("GET", "conversations", tok["ada"]!);
58 const tag = r.headers.get("etag")!;
59 expect(tag).toMatch(/^W\//);
60 expect(r.headers.get("cache-control")).toBe("private, no-cache");
61 expect(r.headers.get("vary")).toContain("authorization, cookie");
62 expect(((await r.json()) as { conversations: unknown[] }).conversations).toHaveLength(1);
63 const again = await call("GET", "conversations", tok["ada"]!, { "if-none-match": tag });
64 expect(again.status).toBe(304);
65 expect(await again.text()).toBe("");
66 expect(again.headers.get("etag")).toBe(tag);
67 expect(again.headers.get("cache-control")).toBe("private, no-cache");
68 expect(again.headers.get("vary")).toContain("authorization, cookie");
69 expect(again.headers.get("x-content-type-options")).toBe("nosniff");
70 expect(again.headers.get("content-security-policy")).toBeTruthy();
71 });
72
73 test("create, rename, archive, unarchive and a turn each move the tag", async () => {
74 let tag = await tagOf();
75 const step = async (what: () => Promise<unknown> | unknown) => {
76 await what();
77 expect(await stale(tag)).toBe(200);
78 tag = await tagOf();
79 };
80 let id = "";
81 await step(async () => {
82 id = ((await (await call("POST", "conversations", tok["ada"]!, {}, {})).json()) as { conversation: { id: string } }).conversation.id;
83 });
84 await step(() => call("PATCH", `conversations/${id}`, tok["ada"]!, {}, { title: "renamed" }));
85 await step(async () => { await Bun.sleep(5); appendTurn(app.db, ids["ada"]!, id, { role: "person", text: "hi" }); });
86 await step(() => call("PATCH", `conversations/${id}`, tok["ada"]!, {}, { archived: true }));
87 await step(() => call("PATCH", `conversations/${id}`, tok["ada"]!, {}, { archived: false }));
88 expect(await stale(tag)).toBe(304);
89 });
90
91 test("archived=1 has its own etag; two viewers never share one", async () => {
92 const live = await tagOf();
93 const archived = await tagOf("conversations?archived=1");
94 expect(archived).not.toBe(live);
95 expect(await stale(live, "conversations?archived=1")).toBe(200);
96 const bob = await call("GET", "conversations", tok["bob"]!, { "if-none-match": live });
97 expect(bob.status).toBe(200);
98 expect(bob.headers.get("etag")).not.toBe(live);
99 });
100
101 test("no session is 401 with no etag, even with If-None-Match: *", async () => {
102 const plain = await call("GET", "conversations", undefined);
103 const star = await call("GET", "conversations", undefined, { "if-none-match": "*" });
104 expect(plain.status).toBe(401);
105 expect(star.status).toBe(401);
106 expect(star.headers.get("etag")).toBeNull();
107 expect(await star.text()).toBe(await plain.text());
108 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs