Skip to content

Commit 78aef4f

Browse files
committed
fix(client): read the CRUD data prefix from discovery instead of restating /data
`crud.dataPrefix` moves the mounted CRUD paths and the advertised discovery document together, and the SDK is the third surface that has to describe those same paths. Its scoped half was not reading the value -- it wrote `/data` into all seventeen of its data methods -- so a deployment on a non-default prefix had that half calling paths the server does not mount, while the unscoped half of the SAME SDK called the right ones (it builds `${baseUrl}${getRoute('data')}`, and `routes.data` already carries the prefix). One SDK disagreed with itself. `_dataPrefix()` recovers the prefix from the advertised routes: a value already ending in the conventional `/data` IS the default prefix -- taken first, so no deployment that works today can regress -- and otherwise `routes.metadata` supplies the missing equation, being `{realBase}{metadata.prefix}` over the same base. Where the document does not determine the split it declines to `/data`, byte-identical to today, following the rule `_apiBase()` already sets in this file. `_apiBase()` now strips that derived prefix rather than the literal `/data`, so base and prefix come from one rule and cannot disagree. No new client option: the value is read from the server that publishes it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YFY46JydE1gMxQG1TqBcMZ
1 parent 2539f4a commit 78aef4f

3 files changed

Lines changed: 374 additions & 23 deletions

File tree

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
"@objectstack/client": patch
3+
---
4+
5+
The client SDK reads the CRUD data prefix off the discovery document instead of restating `/data` as a literal, so a deployment that sets a non-default `crud.dataPrefix` is reachable through the scoped surface.
6+
7+
`crud.dataPrefix` moves two things together: REST mounts every CRUD route under `${basePath}${crud.dataPrefix}`, and the discovery handler advertises the same value as `routes.data = ${realBase}${crud.dataPrefix}`. The SDK is the third surface describing those same paths, and its scoped half was not reading the value — it wrote `/data` into all seventeen of its data methods. On a deployment that moved the prefix, that half called paths the server does not mount, while the unscoped half of the *same* SDK called the right ones: the unscoped methods build `${baseUrl}${getRoute('data')}` and `routes.data` already carries the prefix. One SDK disagreed with itself about where the data routes are.
8+
9+
- **`_dataPrefix()` recovers the prefix from the advertised routes.** `routes.data` is one string carrying two unknowns (`{realBase}{dataPrefix}`) and no discovery key carries either half alone, so the split is recovered in two steps. A value that already ends with the conventional `/data` *is* the default prefix — taken first, which is what makes the change unable to regress any deployment that works today: every later rule can only run in the branch where the previous single-literal code was already wrong. Otherwise `routes.metadata` supplies the missing equation, being `{realBase}{metadata.prefix}` over the same base, so the two advertised routes share exactly `realBase` plus whatever their prefixes share; cutting that common run back to its last `/` lands on the boundary. This also covers a document served from the environment-scoped mount, where both routes carry the same `/environments/{id}` segment.
10+
- **It declines rather than guess.** Where the document does not determine the split — no advertised routes, no `routes.metadata`, or a derived prefix of `/` or empty — the derivation returns the conventional `/data`, which is byte-identical to the previous behaviour. This follows the rule the neighbouring `_apiBase()` already sets in this file, and it is why an unconnected client is unaffected.
11+
- **`_apiBase()` strips the advertised prefix instead of the literal `/data`.** It previously declined whenever the prefix was non-default, because the only suffix it knew how to strip was `/data`. It now strips whatever `_dataPrefix()` read, so the base and the prefix are derived by one rule and cannot disagree. On every default-prefix deployment the result is unchanged.
12+
13+
No new client option and no new configuration: the value is read from the server that already publishes it. A client that never calls `connect()` builds exactly the URLs it built before.
Lines changed: 228 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,228 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* `crud.dataPrefix` is honoured by the SDK, not restated by it (#14879).
5+
*
6+
* THE CONTRACT. `crud.dataPrefix` is a live `RestServerConfig` key: REST mounts
7+
* every CRUD route under `dataPath = ${basePath}${crud.dataPrefix}` and the
8+
* discovery handler advertises the same value as
9+
* `routes.data = ${realBase}${crud.dataPrefix}`. Three surfaces describe one
10+
* set of paths — the mounts, the discovery document, and this SDK — and the
11+
* liveness ledger classifies the key `live` precisely because it "moves the
12+
* mounted paths and the advertised discovery document together".
13+
*
14+
* WHAT WAS WRONG. The SDK's scoped surface restated `/data` as a literal in
15+
* every one of its data methods, so on a deployment that moved the prefix it
16+
* called paths the server does not mount. The unscoped twin of each of those
17+
* methods was already correct — it builds `${baseUrl}${getRoute('data')}` and
18+
* `routes.data` already carries the prefix — so ONE SDK disagreed with itself:
19+
* the unscoped half read the advertised value while the scoped half guessed.
20+
*
21+
* WHY THE FIXTURE CREATES THE CONDITION. Measured on `origin/main`, no in-repo
22+
* caller sets a non-default `dataPrefix`, so no existing fixture exercises
23+
* this and nothing in the tree is broken today; the exposure is external
24+
* deployments. So this suite BOOTS a server on a non-default prefix rather
25+
* than looking for one.
26+
*
27+
* WHY A LIVE SERVER AND A RECORDED URL. A mock that answers 200 to whatever it
28+
* is asked cannot tell a mounted path from an unmounted one — it would go
29+
* green against the very bug this pins. So the server is real, and the suite
30+
* asserts BOTH halves of the claim: that the URL the client puts on the wire
31+
* is the one the server actually mounts, and (`serves nothing at /data`) that
32+
* the old hard-coded path is genuinely dead on this deployment, which is what
33+
* makes the first assertion mean something.
34+
*
35+
* THE POSITIVE CONTROL. The same drive runs against a default-prefix server
36+
* built by the same helper. It is what distinguishes "the SDK follows the
37+
* advertised prefix" from "the SDK broke and now sends something else": the
38+
* default deployment must still be reached at `/data`, byte-for-byte as
39+
* before.
40+
*/
41+
42+
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
43+
import { LiteKernel } from '@objectstack/core';
44+
import { ObjectQL, ObjectQLPlugin } from '@objectstack/objectql';
45+
import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm';
46+
import { HonoServerPlugin } from '@objectstack/plugin-hono-server';
47+
import { createRestApiPlugin } from '@objectstack/runtime';
48+
import { ObjectStackClient } from './index';
49+
import type { IHttpServer } from '@objectstack/spec/contracts';
50+
51+
const ENV_ID = 'proj-alpha';
52+
const CUSTOM_PREFIX = '/objects';
53+
54+
interface Fixture {
55+
baseUrl: string;
56+
kernel: LiteKernel;
57+
}
58+
59+
/**
60+
* One boot recipe, two prefixes — so the non-default case and the control
61+
* differ in exactly the key under test and nothing else.
62+
*/
63+
async function bootServer(dataPrefix?: string): Promise<Fixture> {
64+
const kernel = new LiteKernel();
65+
kernel.use(new ObjectQLPlugin());
66+
// Same reason as the sibling scoping suite (#3963): the anonymous-deny
67+
// gate is unconditional, so a live-server client suite needs a session.
68+
kernel.use({
69+
metadata: { name: 'test-auth', version: '1.0.0' },
70+
async init(ctx: any) {
71+
ctx.registerService('auth', {
72+
api: { getSession: async () => ({ user: { id: 'test-user' } }) },
73+
});
74+
},
75+
} as any);
76+
77+
const honoPlugin = new HonoServerPlugin({ port: 0 });
78+
kernel.use(honoPlugin);
79+
80+
kernel.use(
81+
createRestApiPlugin({
82+
api: {
83+
api: {
84+
// Routing test, no auth stack mounted (ADR-0056 D2).
85+
requireAuth: false,
86+
enableProjectScoping: true,
87+
projectResolution: 'auto',
88+
} as any,
89+
// The key under test. Omitted entirely for the control, so the
90+
// control runs the schema's own `.default('/data')` rather than
91+
// a second literal written here.
92+
...(dataPrefix ? { crud: { dataPrefix } as any } : {}),
93+
},
94+
}),
95+
);
96+
97+
await kernel.bootstrap();
98+
99+
const ql = kernel.getService<ObjectQL>('objectql');
100+
ql.registerDriver(new SqliteWasmDriver({ filename: ':memory:' }) as never, true);
101+
ql.registerObject({
102+
name: 'task',
103+
label: 'Task',
104+
fields: { title: { type: 'text', label: 'Title' } },
105+
});
106+
// Registered after bootstrap, so nothing has issued the DDL yet (#4065).
107+
await ql.syncObjectSchema('task');
108+
109+
const httpServer = kernel.getService<IHttpServer>('http.server');
110+
const port = httpServer.getPort!();
111+
return { baseUrl: `http://localhost:${port}`, kernel };
112+
}
113+
114+
async function shutdown(fixture: Fixture | undefined): Promise<void> {
115+
if (!fixture?.kernel) return;
116+
await Promise.race([
117+
fixture.kernel.shutdown(),
118+
new Promise<void>((resolve) => setTimeout(resolve, 10_000)),
119+
]);
120+
}
121+
122+
/**
123+
* A client whose every request URL is recorded. The recorder DELEGATES to the
124+
* real fetch, so the recorded URL and the server's real answer are the same
125+
* exchange — the assertion cannot pass on a URL that was never served.
126+
*/
127+
function recordingClient(baseUrl: string): { client: ObjectStackClient; urls: string[] } {
128+
const urls: string[] = [];
129+
const client = new ObjectStackClient({
130+
baseUrl,
131+
fetch: (input: RequestInfo | URL, init?: RequestInit) => {
132+
urls.push(typeof input === 'string' ? input : String(input));
133+
return globalThis.fetch(input as any, init);
134+
},
135+
} as any);
136+
return { client, urls };
137+
}
138+
139+
describe('SDK honours crud.dataPrefix (#14879)', () => {
140+
describe(`non-default prefix (${CUSTOM_PREFIX})`, () => {
141+
let fx: Fixture;
142+
143+
beforeAll(async () => { fx = await bootServer(CUSTOM_PREFIX); }, 30_000);
144+
afterAll(async () => { await shutdown(fx); }, 30_000);
145+
146+
it('mounts scoped CRUD under the configured prefix, and serves nothing at /data', async () => {
147+
const mounted = await fetch(`${fx.baseUrl}/api/v1/environments/${ENV_ID}${CUSTOM_PREFIX}/task?top=5`);
148+
expect(mounted.status).toBe(200);
149+
150+
// The half that makes this fixture worth anything: the path the
151+
// SDK used to hard-code is genuinely not mounted here.
152+
const hardCoded = await fetch(`${fx.baseUrl}/api/v1/environments/${ENV_ID}/data/task?top=5`);
153+
expect(hardCoded.status).toBe(404);
154+
});
155+
156+
it('advertises the prefix on the discovery document', async () => {
157+
const res = await fetch(`${fx.baseUrl}/api/v1/discovery`);
158+
expect(res.status).toBe(200);
159+
const body = await res.json();
160+
const routes = (body?.data ?? body)?.routes;
161+
expect(routes?.data).toBe(`/api/v1${CUSTOM_PREFIX}`);
162+
});
163+
164+
it('scoped data.find() calls the mounted path, not /data', async () => {
165+
const { client, urls } = recordingClient(fx.baseUrl);
166+
await client.connect();
167+
168+
const scoped = client.environment(ENV_ID);
169+
await expect(scoped.data.find('task')).resolves.toBeDefined();
170+
171+
const dataCalls = urls.filter((u) => u.includes('/task'));
172+
expect(dataCalls).toHaveLength(1);
173+
expect(dataCalls[0]).toContain(`/api/v1/environments/${ENV_ID}${CUSTOM_PREFIX}/task`);
174+
expect(dataCalls[0]).not.toContain('/data/');
175+
});
176+
177+
it('scoped data.query() calls the mounted path, not /data', async () => {
178+
const { client, urls } = recordingClient(fx.baseUrl);
179+
await client.connect();
180+
181+
const scoped = client.environment(ENV_ID);
182+
await expect(scoped.data.query('task', { top: 1 })).resolves.toBeDefined();
183+
184+
const queryCalls = urls.filter((u) => u.includes('/task/query'));
185+
expect(queryCalls).toHaveLength(1);
186+
expect(queryCalls[0]).toContain(`/api/v1/environments/${ENV_ID}${CUSTOM_PREFIX}/task/query`);
187+
expect(queryCalls[0]).not.toContain('/data/');
188+
});
189+
});
190+
191+
describe('positive control — default prefix on the same fixture', () => {
192+
let fx: Fixture;
193+
194+
beforeAll(async () => { fx = await bootServer(); }, 30_000);
195+
afterAll(async () => { await shutdown(fx); }, 30_000);
196+
197+
it('advertises /api/v1/data', async () => {
198+
const res = await fetch(`${fx.baseUrl}/api/v1/discovery`);
199+
const body = await res.json();
200+
const routes = (body?.data ?? body)?.routes;
201+
expect(routes?.data).toBe('/api/v1/data');
202+
});
203+
204+
it('scoped data.find() still calls /data — unchanged by the derivation', async () => {
205+
const { client, urls } = recordingClient(fx.baseUrl);
206+
await client.connect();
207+
208+
const scoped = client.environment(ENV_ID);
209+
await expect(scoped.data.find('task')).resolves.toBeDefined();
210+
211+
const dataCalls = urls.filter((u) => u.includes('/task'));
212+
expect(dataCalls).toHaveLength(1);
213+
expect(dataCalls[0]).toContain(`/api/v1/environments/${ENV_ID}/data/task`);
214+
});
215+
216+
it('an unconnected client declines to the /data convention', async () => {
217+
// No `connect()`, so there is no advertised document to read. The
218+
// derivation must fall back to today's literal rather than invent
219+
// a prefix -- this is the "declines rather than guess" leg.
220+
const { client, urls } = recordingClient(fx.baseUrl);
221+
const scoped = client.environment(ENV_ID);
222+
await expect(scoped.data.find('task')).resolves.toBeDefined();
223+
224+
expect(urls).toHaveLength(1);
225+
expect(urls[0]).toContain(`/api/v1/environments/${ENV_ID}/data/task`);
226+
});
227+
});
228+
});

0 commit comments

Comments
 (0)