Skip to content

Commit 94f0cf0

Browse files
committed
test(runtime): pin PATCH /data/:object/:id version handling on both doors (unfixed source)
Claude-Session: https://claude.ai/code/session_01Q9PZ5Pzqcus4woVvTZwWNU Co-authored-by: Claude <noreply@anthropic.com>
1 parent 3d231ba commit 94f0cf0

1 file changed

Lines changed: 373 additions & 0 deletions

File tree

Lines changed: 373 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,373 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#23462] Both doors of `PATCH /data/:object/:id` honour the caller's
5+
* optimistic-concurrency token alike: `RestServer`'s route and the runtime
6+
* dispatcher's `/data` domain, the only answer on a host that mounts just
7+
* `@objectstack/hono`'s `${prefix}/*` catch-all. The `DELETE` position of the
8+
* same family is pinned in `data-delete-expected-version.test.ts` (#23265);
9+
* this file is its `PATCH` sibling, on the same rig.
10+
*
11+
* ## What was wrong
12+
*
13+
* `RestServer` reads the body's `expectedVersion`, else `If-Match`, strips the
14+
* key out of the patch, and hands the token to `updateData`, which refuses an
15+
* update whose version moved (`409 CONCURRENT_UPDATE`). The dispatcher's
16+
* `PATCH` branch read neither and handed the whole body over as the patch, and
17+
* `callData`'s update arm forwarded no token. So through the catch-all a stale
18+
* `If-Match` update was APPLIED with a `200`, and a body `expectedVersion`
19+
* reached the engine as a field (refused as an unknown field, `500`). These
20+
* rows hold the two doors to the same answer over the same cases, and read the
21+
* store, not the door's word, after every request.
22+
*
23+
* ## The rig
24+
*
25+
* Each door gets its own REAL store: better-sqlite3 `:memory:`, the real
26+
* `ObjectQL` engine and a real `ObjectStackProtocolImplementation`, so a
27+
* conflict is the protocol's own version check, not a double's.
28+
*
29+
* - `RestServer`: its registered `PATCH` handler, called with the request
30+
* the `plugin-hono-server` adapter builds (a lowercased header record and
31+
* the parsed JSON body), the auth boundary (`resolveExecCtx`) stubbed;
32+
* - the catch-all: {@link catchAll} repeats the catch-all's statements over a
33+
* real Fetch `Request` into the real `HttpDispatcher.dispatch()`: the body
34+
* is parsed from a clone of the request's text (an unparsable body is
35+
* `{}`), and the raw `Request` rides as `context.request`, which is where an
36+
* `If-Match` is read. The identity step is the one stubbed seam.
37+
*
38+
* ## How a refusal is read
39+
*
40+
* The conflict is not returned by `dispatch()`: `/data` has no local catch, so
41+
* it throws the protocol's `ConcurrentUpdateError` and the host's exit renders
42+
* it. Its status and code are read here through `resolveThrownHttpError`
43+
* (`@objectstack/types`), the one rule the dispatcher's exits answer a throw
44+
* with, and set beside what `RestServer` answered through `mapDataError`. The
45+
* parity asserted is status, `code` and the store, never the envelope's bytes.
46+
*/
47+
48+
import { describe, it, expect, vi, afterEach } from 'vitest';
49+
import { ObjectQL } from '@objectstack/objectql';
50+
import { SqlDriver } from '@objectstack/driver-sql';
51+
import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol';
52+
import { RestServer } from '@objectstack/rest';
53+
import { resolveThrownHttpError } from '@objectstack/types';
54+
import { HttpDispatcher } from '../http-dispatcher.js';
55+
56+
const PREFIX = '/api/v1';
57+
const DATA = `${PREFIX}/data`;
58+
/** `registry.registerObject` requires a package id (see the sibling real-stack tests). */
59+
const TEST_PACKAGE_ID = 'objectstack-test';
60+
const CALLER = { userId: 'u_member', isSystem: false };
61+
62+
const TASK = {
63+
name: 'task',
64+
label: 'Task',
65+
fields: {
66+
id: { name: 'id', type: 'text' as const, primaryKey: true, label: 'ID' },
67+
name: { name: 'name', type: 'text' as const, label: 'Name' },
68+
},
69+
};
70+
71+
/** `r1`'s name before any request; every applied patch below writes {@link AFTER}. */
72+
const BEFORE = 'first';
73+
const AFTER = 'second';
74+
75+
/** A well-formed instant no row in these stores carries: a token read before a write that moved it. */
76+
const STALE = '2000-01-01T00:00:00.000Z';
77+
78+
const clone = <T>(v: T): T => (v === undefined ? v : JSON.parse(JSON.stringify(v)));
79+
80+
const liveEngines: ObjectQL[] = [];
81+
afterEach(async () => {
82+
while (liveEngines.length) {
83+
try { await liveEngines.pop()?.destroy(); } catch { /* noop */ }
84+
}
85+
});
86+
87+
/** One real store with one `task` row, `r1`, named {@link BEFORE}, whose current version is `version`. */
88+
async function bootStore() {
89+
const engine = new ObjectQL();
90+
liveEngines.push(engine);
91+
engine.registerDriver(new SqlDriver({
92+
client: 'better-sqlite3',
93+
connection: { filename: ':memory:' },
94+
useNullAsDefault: true,
95+
}), true);
96+
await engine.init();
97+
engine.registry.registerObject(TASK as any, TEST_PACKAGE_ID);
98+
await engine.syncSchemas();
99+
const protocol = new ObjectStackProtocolImplementation(engine as any);
100+
await protocol.createData({ object: 'task', data: { id: 'r1', name: BEFORE } } as any);
101+
const row: any = await engine.findOne('task', { where: { id: 'r1' } });
102+
const version = row?.updated_at instanceof Date ? row.updated_at.toISOString() : String(row?.updated_at ?? '');
103+
if (!version) throw new Error('the store minted no updated_at, so no version token exists to pin');
104+
const updateData = vi.spyOn(protocol, 'updateData');
105+
/** `r1`'s `name` in the store now. */
106+
const name = async () => ((await engine.findOne('task', { where: { id: 'r1' } })) as any)?.name;
107+
return { protocol, version, name, updateData };
108+
}
109+
110+
// ── The two doors ─────────────────────────────────────────────────────────────
111+
112+
interface Answer { status: number; code?: string; message?: string }
113+
interface PatchOptions { headers?: Record<string, string>; body?: unknown }
114+
/** What one `updateData` call received. */
115+
interface Forwarded { expectedVersion: string | undefined; data: unknown }
116+
interface Door {
117+
version: string;
118+
patch(opts?: PatchOptions): Promise<Answer>;
119+
name(): Promise<string | undefined>;
120+
forwarded(): Forwarded[];
121+
}
122+
123+
/** The refusal's `code` in either door's envelope: nested under `error`, or beside a string `error`. */
124+
const codeOf = (body: any): string | undefined =>
125+
body?.error && typeof body.error === 'object' ? body.error.code : body?.code;
126+
const messageOf = (body: any): string | undefined =>
127+
body?.error && typeof body.error === 'object' ? body.error.message : body?.error;
128+
129+
const forwardedBy = (updateData: { mock: { calls: any[][] } }) => (): Forwarded[] =>
130+
updateData.mock.calls.map((c) => ({ expectedVersion: c[0]?.expectedVersion, data: c[0]?.data }));
131+
132+
function makeRes() {
133+
const res: any = { statusCode: 200, body: undefined, headers: {} as Record<string, string> };
134+
res.status = (code: number) => { res.statusCode = code; return res; };
135+
res.json = (body: any) => { res.body = body; return res; };
136+
res.send = () => res;
137+
res.end = () => res;
138+
res.header = (k: string, v: string) => { res.headers[k] = v; return res; };
139+
res.setHeader = () => {};
140+
res.write = () => true;
141+
return res;
142+
}
143+
144+
async function bootRestServer(): Promise<Door> {
145+
const { protocol, version, name, updateData } = await bootStore();
146+
const noop = () => {};
147+
const server = { get: noop, post: noop, put: noop, delete: noop, patch: noop, use: noop, listen: async () => {}, close: async () => {} };
148+
const rest: any = new RestServer(server as any, protocol as any, { api: { requireAuth: false } } as any);
149+
rest.resolveExecCtx = async () => clone(CALLER);
150+
rest.registerRoutes();
151+
const route = rest.getRoutes().find((r: any) => r.method === 'PATCH' && r.path === `${DATA}/:object/:id`);
152+
if (!route) throw new Error(`PATCH ${DATA}/:object/:id is not registered`);
153+
return {
154+
version,
155+
name,
156+
forwarded: forwardedBy(updateData),
157+
patch: async ({ headers = {}, body } = {}) => {
158+
const res = makeRes();
159+
await route.handler({
160+
method: 'PATCH', path: `${DATA}/task/r1`, params: { object: 'task', id: 'r1' },
161+
// The adapter's header record is lowercased (`c.req.header()`).
162+
headers: Object.fromEntries(Object.entries(headers).map(([k, v]) => [k.toLowerCase(), v])),
163+
query: {},
164+
body: clone(body),
165+
}, res);
166+
return { status: res.statusCode, code: codeOf(res.body), message: messageOf(res.body) };
167+
},
168+
};
169+
}
170+
171+
/**
172+
* What `createHonoApp`'s `${prefix}/*` catch-all hands `dispatch()` for a
173+
* `PATCH`: the path below the prefix, the JSON body parsed from a CLONE of the
174+
* request's text (any failure is `{}`, the adapter's `readDispatchBody`), the
175+
* query flattened from the URL, and the raw Fetch `Request` as
176+
* `context.request`.
177+
*/
178+
async function catchAll(dispatcher: HttpDispatcher, raw: Request) {
179+
const url = new URL(raw.url);
180+
const subPath = url.pathname.substring(PREFIX.length);
181+
let body: unknown;
182+
try {
183+
body = JSON.parse(await raw.clone().text());
184+
} catch {
185+
body = {};
186+
}
187+
const query: Record<string, string> = {};
188+
url.searchParams.forEach((v, k) => { query[k] = v; });
189+
return dispatcher.dispatch(raw.method, subPath, body, query, { request: raw } as any, PREFIX);
190+
}
191+
192+
function dispatcherOver(services: Record<string, unknown>): HttpDispatcher {
193+
const get = (n: string) => services[n] ?? null;
194+
const kernel: any = { context: { getService: get }, getService: get, getServiceAsync: async (n: string) => get(n) };
195+
const dispatcher = new HttpDispatcher(kernel);
196+
// The seam `dispatch()` resolves identity through (the sibling
197+
// dispatch-level pins supply the principal the same way).
198+
(dispatcher as any).timedResolveExecutionContext = async () => clone(CALLER);
199+
return dispatcher;
200+
}
201+
202+
const patchRequest = ({ headers = {}, body }: PatchOptions = {}) =>
203+
new Request(`http://localhost${DATA}/task/r1`, {
204+
method: 'PATCH',
205+
headers: { 'content-type': 'application/json', ...headers },
206+
...(body === undefined ? {} : { body: JSON.stringify(body) }),
207+
});
208+
209+
/** A dispatch answer, returned or thrown, as status and code. */
210+
async function answerOf(run: () => Promise<any>): Promise<Answer> {
211+
try {
212+
const res = await run();
213+
const body = res.response?.body;
214+
return { status: res.response?.status ?? 0, code: codeOf(body), message: messageOf(body) };
215+
} catch (e) {
216+
const thrown = resolveThrownHttpError(e);
217+
return { status: thrown.status, code: thrown.code, message: thrown.message };
218+
}
219+
}
220+
221+
async function bootCatchAll(): Promise<Door> {
222+
const { protocol, version, name, updateData } = await bootStore();
223+
const dispatcher = dispatcherOver({ protocol });
224+
return {
225+
version,
226+
name,
227+
forwarded: forwardedBy(updateData),
228+
patch: (opts = {}) => answerOf(() => catchAll(dispatcher, patchRequest(opts))),
229+
};
230+
}
231+
232+
const DOORS: ReadonlyArray<readonly [string, () => Promise<Door>]> = [
233+
['RestServer', bootRestServer],
234+
['the @objectstack/hono catch-all', bootCatchAll],
235+
];
236+
237+
const updated = (a: Answer) => expect(a.status, JSON.stringify(a)).toBe(200);
238+
const conflict = (a: Answer) =>
239+
expect({ status: a.status, code: a.code }, JSON.stringify(a)).toEqual({ status: 409, code: 'CONCURRENT_UPDATE' });
240+
241+
// ── The cases, on each door ───────────────────────────────────────────────────
242+
243+
for (const [doorName, boot] of DOORS) {
244+
describe(`[#23462] ${doorName}: PATCH /data/:object/:id honours a body expectedVersion and If-Match`, () => {
245+
it('control: a patch with no version updates, and hands updateData no token', async () => {
246+
const door = await boot();
247+
updated(await door.patch({ body: { name: AFTER } }));
248+
expect(await door.name()).toBe(AFTER);
249+
expect(door.forwarded()).toEqual([{ expectedVersion: undefined, data: { name: AFTER } }]);
250+
}, 60_000);
251+
252+
it('a body expectedVersion naming the current version updates, and is never written as a field', async () => {
253+
const door = await boot();
254+
updated(await door.patch({ body: { name: AFTER, expectedVersion: door.version } }));
255+
expect(await door.name()).toBe(AFTER);
256+
// The token rides beside the patch, never inside it.
257+
expect(door.forwarded()).toEqual([{ expectedVersion: door.version, data: { name: AFTER } }]);
258+
}, 60_000);
259+
260+
it('a stale body expectedVersion is refused 409 CONCURRENT_UPDATE and the row is unchanged', async () => {
261+
const door = await boot();
262+
conflict(await door.patch({ body: { name: AFTER, expectedVersion: STALE } }));
263+
expect(await door.name()).toBe(BEFORE);
264+
expect(door.forwarded()).toEqual([{ expectedVersion: STALE, data: { name: AFTER } }]);
265+
}, 60_000);
266+
267+
it('If-Match behaves like the body token: the current version updates, a stale one is refused', async () => {
268+
const stale = await boot();
269+
conflict(await stale.patch({ body: { name: AFTER }, headers: { 'If-Match': `"${STALE}"` } }));
270+
expect(await stale.name()).toBe(BEFORE);
271+
272+
const current = await boot();
273+
updated(await current.patch({ body: { name: AFTER }, headers: { 'If-Match': `"${current.version}"` } }));
274+
expect(await current.name()).toBe(AFTER);
275+
}, 60_000);
276+
277+
it('the body token is read before If-Match, so the body token is the one judged', async () => {
278+
const staleBody = await boot();
279+
conflict(await staleBody.patch({ body: { name: AFTER, expectedVersion: STALE }, headers: { 'If-Match': staleBody.version } }));
280+
expect(await staleBody.name()).toBe(BEFORE);
281+
282+
const currentBody = await boot();
283+
updated(await currentBody.patch({ body: { name: AFTER, expectedVersion: currentBody.version }, headers: { 'If-Match': STALE } }));
284+
expect(await currentBody.name()).toBe(AFTER);
285+
}, 60_000);
286+
});
287+
}
288+
289+
// ── The same answers, side by side ────────────────────────────────────────────
290+
291+
/**
292+
* The shapes a caller can put the token in beyond the plain string. Each door
293+
* answers each one, and the two must agree on the status, the code and the
294+
* row. What either door does with them is `RestServer`'s reading, not this
295+
* file's: a token is the body key when it is not nullish (else `If-Match`),
296+
* the key is stripped from the patch whatever its value, and a token is
297+
* forwarded as its string only when it is truthy.
298+
*/
299+
const EDGE_SHAPES: ReadonlyArray<readonly [string, (version: string) => PatchOptions]> = [
300+
['a stale token', () => ({ body: { name: AFTER, expectedVersion: STALE } })],
301+
['a number token', () => ({ body: { name: AFTER, expectedVersion: 20000101 } })],
302+
['a null body token beside a stale If-Match', () => ({ body: { name: AFTER, expectedVersion: null }, headers: { 'If-Match': STALE } })],
303+
['a null body token beside the current If-Match', (v) => ({ body: { name: AFTER, expectedVersion: null }, headers: { 'If-Match': v } })],
304+
['an empty-string body token beside a stale If-Match', () => ({ body: { name: AFTER, expectedVersion: '' }, headers: { 'If-Match': STALE } })],
305+
['a body that is only the current token', (v) => ({ body: { expectedVersion: v } })],
306+
];
307+
308+
describe('[#23462] the two doors answer each token shape with the same status, code and row', () => {
309+
for (const [shape, opts] of EDGE_SHAPES) {
310+
it(shape, async () => {
311+
const seen: Array<{ status: number; code?: string; name?: string }> = [];
312+
for (const [, boot] of DOORS) {
313+
const door = await boot();
314+
const answer = await door.patch(opts(door.version));
315+
seen.push({ status: answer.status, code: answer.code, name: await door.name() });
316+
// Whatever the token's shape, the key never reaches the engine as a field.
317+
for (const call of door.forwarded()) {
318+
expect(call.data && typeof call.data === 'object' && 'expectedVersion' in call.data, JSON.stringify(call)).toBe(false);
319+
}
320+
}
321+
const [rest, catchAllAnswer] = seen;
322+
expect(catchAllAnswer).toEqual(rest);
323+
}, 60_000);
324+
}
325+
326+
it('a Node host\'s plain header record carries If-Match the way the Fetch Headers does', async () => {
327+
const { protocol, name } = await bootStore();
328+
const dispatcher = dispatcherOver({ protocol });
329+
const answer = await answerOf(() => dispatcher.dispatch(
330+
'PATCH', '/data/task/r1', { name: AFTER }, {}, { request: { headers: { 'if-match': `"${STALE}"` } } } as any, PREFIX,
331+
));
332+
conflict(answer);
333+
expect(await name()).toBe(BEFORE);
334+
}, 60_000);
335+
});
336+
337+
// ── The dispatcher's fallback updater ─────────────────────────────────────────
338+
339+
/**
340+
* The dispatcher alone has a second updater: a host whose `protocol` brings no
341+
* `updateData` falls to the ObjectQL engine's `update`, which takes no version.
342+
* A caller who sent one is refused `501` before anything is read or written,
343+
* never updated unguarded, which is the drop this card closes.
344+
*/
345+
describe('[#23462] the catch-all\'s fallback updater cannot judge a version, so sending one is refused', () => {
346+
const fallbackHost = () => {
347+
const objectql = {
348+
find: vi.fn(async () => [{ id: 'r1', name: BEFORE }]),
349+
update: vi.fn(async () => ({ id: 'r1', name: AFTER })),
350+
};
351+
return { dispatcher: dispatcherOver({ protocol: {}, objectql }), objectql };
352+
};
353+
354+
for (const [asked, opts] of [
355+
['a body expectedVersion', { body: { name: AFTER, expectedVersion: STALE } }],
356+
['If-Match', { body: { name: AFTER }, headers: { 'If-Match': `"${STALE}"` } }],
357+
] as const) {
358+
it(`${asked}: 501 NOT_IMPLEMENTED, and the fallback neither reads nor writes`, async () => {
359+
const { dispatcher, objectql } = fallbackHost();
360+
const answer = await answerOf(() => catchAll(dispatcher, patchRequest(opts as PatchOptions)));
361+
expect({ status: answer.status, code: answer.code }).toEqual({ status: 501, code: 'NOT_IMPLEMENTED' });
362+
expect(objectql.find).not.toHaveBeenCalled();
363+
expect(objectql.update).not.toHaveBeenCalled();
364+
});
365+
}
366+
367+
it('control: an unguarded update still reaches the fallback updater', async () => {
368+
const { dispatcher, objectql } = fallbackHost();
369+
const answer = await answerOf(() => catchAll(dispatcher, patchRequest({ body: { name: AFTER } })));
370+
expect(answer.status).toBe(200);
371+
expect(objectql.update).toHaveBeenCalledTimes(1);
372+
});
373+
});

0 commit comments

Comments
 (0)