Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .changeset/client-honours-data-prefix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
"@objectstack/client": patch
---

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.

`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.

- **`_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.
- **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.
- **`_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.

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.
228 changes: 228 additions & 0 deletions packages/client/src/client.data-prefix.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,228 @@
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.

/**
* `crud.dataPrefix` is honoured by the SDK, not restated by it (#14879).
*
* THE CONTRACT. `crud.dataPrefix` is a live `RestServerConfig` key: REST mounts
* every CRUD route under `dataPath = ${basePath}${crud.dataPrefix}` and the
* discovery handler advertises the same value as
* `routes.data = ${realBase}${crud.dataPrefix}`. Three surfaces describe one
* set of paths — the mounts, the discovery document, and this SDK — and the
* liveness ledger classifies the key `live` precisely because it "moves the
* mounted paths and the advertised discovery document together".
*
* WHAT WAS WRONG. The SDK's scoped surface restated `/data` as a literal in
* every one of its data methods, so on a deployment that moved the prefix it
* called paths the server does not mount. The unscoped twin of each of those
* methods was already correct — it builds `${baseUrl}${getRoute('data')}` and
* `routes.data` already carries the prefix — so ONE SDK disagreed with itself:
* the unscoped half read the advertised value while the scoped half guessed.
*
* WHY THE FIXTURE CREATES THE CONDITION. Measured on `origin/main`, no in-repo
* caller sets a non-default `dataPrefix`, so no existing fixture exercises
* this and nothing in the tree is broken today; the exposure is external
* deployments. So this suite BOOTS a server on a non-default prefix rather
* than looking for one.
*
* WHY A LIVE SERVER AND A RECORDED URL. A mock that answers 200 to whatever it
* is asked cannot tell a mounted path from an unmounted one — it would go
* green against the very bug this pins. So the server is real, and the suite
* asserts BOTH halves of the claim: that the URL the client puts on the wire
* is the one the server actually mounts, and (`serves nothing at /data`) that
* the old hard-coded path is genuinely dead on this deployment, which is what
* makes the first assertion mean something.
*
* THE POSITIVE CONTROL. The same drive runs against a default-prefix server
* built by the same helper. It is what distinguishes "the SDK follows the
* advertised prefix" from "the SDK broke and now sends something else": the
* default deployment must still be reached at `/data`, byte-for-byte as
* before.
*/

import { describe, it, expect, beforeAll, afterAll } from 'vitest';
import { LiteKernel } from '@objectstack/core';
import { ObjectQL, ObjectQLPlugin } from '@objectstack/objectql';
import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm';
import { HonoServerPlugin } from '@objectstack/plugin-hono-server';
import { createRestApiPlugin } from '@objectstack/runtime';
import { ObjectStackClient } from './index';
import type { IHttpServer } from '@objectstack/spec/contracts';

const ENV_ID = 'proj-alpha';
const CUSTOM_PREFIX = '/objects';

interface Fixture {
baseUrl: string;
kernel: LiteKernel;
}

/**
* One boot recipe, two prefixes — so the non-default case and the control
* differ in exactly the key under test and nothing else.
*/
async function bootServer(dataPrefix?: string): Promise<Fixture> {
const kernel = new LiteKernel();
kernel.use(new ObjectQLPlugin());
// Same reason as the sibling scoping suite (#3963): the anonymous-deny
// gate is unconditional, so a live-server client suite needs a session.
kernel.use({
metadata: { name: 'test-auth', version: '1.0.0' },
async init(ctx: any) {
ctx.registerService('auth', {
api: { getSession: async () => ({ user: { id: 'test-user' } }) },
});
},
} as any);

const honoPlugin = new HonoServerPlugin({ port: 0 });
kernel.use(honoPlugin);

kernel.use(
createRestApiPlugin({
api: {
api: {
// Routing test, no auth stack mounted (ADR-0056 D2).
requireAuth: false,
enableProjectScoping: true,
projectResolution: 'auto',
} as any,
// The key under test. Omitted entirely for the control, so the
// control runs the schema's own `.default('/data')` rather than
// a second literal written here.
...(dataPrefix ? { crud: { dataPrefix } as any } : {}),
},
}),
);

await kernel.bootstrap();

const ql = kernel.getService<ObjectQL>('objectql');
ql.registerDriver(new SqliteWasmDriver({ filename: ':memory:' }) as never, true);
ql.registerObject({
name: 'task',
label: 'Task',
fields: { title: { type: 'text', label: 'Title' } },
});
// Registered after bootstrap, so nothing has issued the DDL yet (#4065).
await ql.syncObjectSchema('task');

const httpServer = kernel.getService<IHttpServer>('http.server');
const port = httpServer.getPort!();
return { baseUrl: `http://localhost:${port}`, kernel };
}

async function shutdown(fixture: Fixture | undefined): Promise<void> {
if (!fixture?.kernel) return;
await Promise.race([
fixture.kernel.shutdown(),
new Promise<void>((resolve) => setTimeout(resolve, 10_000)),
]);
}

/**
* A client whose every request URL is recorded. The recorder DELEGATES to the
* real fetch, so the recorded URL and the server's real answer are the same
* exchange — the assertion cannot pass on a URL that was never served.
*/
function recordingClient(baseUrl: string): { client: ObjectStackClient; urls: string[] } {
const urls: string[] = [];
const client = new ObjectStackClient({
baseUrl,
fetch: (input: RequestInfo | URL, init?: RequestInit) => {
urls.push(typeof input === 'string' ? input : String(input));
return globalThis.fetch(input as any, init);
},
} as any);
return { client, urls };
}

describe('SDK honours crud.dataPrefix (#14879)', () => {
describe(`non-default prefix (${CUSTOM_PREFIX})`, () => {
let fx: Fixture;

beforeAll(async () => { fx = await bootServer(CUSTOM_PREFIX); }, 30_000);
afterAll(async () => { await shutdown(fx); }, 30_000);

it('mounts scoped CRUD under the configured prefix, and serves nothing at /data', async () => {
const mounted = await fetch(`${fx.baseUrl}/api/v1/environments/${ENV_ID}${CUSTOM_PREFIX}/task?top=5`);
expect(mounted.status).toBe(200);

// The half that makes this fixture worth anything: the path the
// SDK used to hard-code is genuinely not mounted here.
const hardCoded = await fetch(`${fx.baseUrl}/api/v1/environments/${ENV_ID}/data/task?top=5`);
expect(hardCoded.status).toBe(404);
});

it('advertises the prefix on the discovery document', async () => {
const res = await fetch(`${fx.baseUrl}/api/v1/discovery`);
expect(res.status).toBe(200);
const body = await res.json();
const routes = (body?.data ?? body)?.routes;
expect(routes?.data).toBe(`/api/v1${CUSTOM_PREFIX}`);
});

it('scoped data.find() calls the mounted path, not /data', async () => {
const { client, urls } = recordingClient(fx.baseUrl);
await client.connect();

const scoped = client.environment(ENV_ID);
await expect(scoped.data.find('task')).resolves.toBeDefined();

const dataCalls = urls.filter((u) => u.includes('/task'));
expect(dataCalls).toHaveLength(1);
expect(dataCalls[0]).toContain(`/api/v1/environments/${ENV_ID}${CUSTOM_PREFIX}/task`);
expect(dataCalls[0]).not.toContain('/data/');
});

it('scoped data.query() calls the mounted path, not /data', async () => {
const { client, urls } = recordingClient(fx.baseUrl);
await client.connect();

const scoped = client.environment(ENV_ID);
await expect(scoped.data.query('task', { top: 1 })).resolves.toBeDefined();

const queryCalls = urls.filter((u) => u.includes('/task/query'));
expect(queryCalls).toHaveLength(1);
expect(queryCalls[0]).toContain(`/api/v1/environments/${ENV_ID}${CUSTOM_PREFIX}/task/query`);
expect(queryCalls[0]).not.toContain('/data/');
});
});

describe('positive control — default prefix on the same fixture', () => {
let fx: Fixture;

beforeAll(async () => { fx = await bootServer(); }, 30_000);
afterAll(async () => { await shutdown(fx); }, 30_000);

it('advertises /api/v1/data', async () => {
const res = await fetch(`${fx.baseUrl}/api/v1/discovery`);
const body = await res.json();
const routes = (body?.data ?? body)?.routes;
expect(routes?.data).toBe('/api/v1/data');
});

it('scoped data.find() still calls /data — unchanged by the derivation', async () => {
const { client, urls } = recordingClient(fx.baseUrl);
await client.connect();

const scoped = client.environment(ENV_ID);
await expect(scoped.data.find('task')).resolves.toBeDefined();

const dataCalls = urls.filter((u) => u.includes('/task'));
expect(dataCalls).toHaveLength(1);
expect(dataCalls[0]).toContain(`/api/v1/environments/${ENV_ID}/data/task`);
});

it('an unconnected client declines to the /data convention', async () => {
// No `connect()`, so there is no advertised document to read. The
// derivation must fall back to today's literal rather than invent
// a prefix -- this is the "declines rather than guess" leg.
const { client, urls } = recordingClient(fx.baseUrl);
const scoped = client.environment(ENV_ID);
await expect(scoped.data.find('task')).resolves.toBeDefined();

expect(urls).toHaveLength(1);
expect(urls[0]).toContain(`/api/v1/environments/${ENV_ID}/data/task`);
});
});
});
56 changes: 51 additions & 5 deletions packages/client/src/client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2129,16 +2129,62 @@ describe('ScopedEnvironmentClient', () => {
expect(String(fetchMock.mock.calls[4][0])).toBe(`${base}/batch`);
});

it('[#6714] a custom dataPrefix makes the base underivable — the convention holds, byte-identical (case B)', async () => {
it('[#14879] a custom dataPrefix no longer makes the base underivable — `routes.metadata` is the second equation (case B1)', async () => {
const { client, fetchMock } = createMockClient({ types: [] });
// routes.data does not end with the conventional `/data`, so the base
// cannot be derived honestly; the client must NOT guess (contract-first
// — no lenient re-parsing) and falls back to the convention,
// byte-identical to the pre-#6714 behavior.
// WAS pinned the other way. Until #14879 this case asserted the
// convention `/api/v1/...`, because the only suffix `_apiBase()` knew
// how to strip was the literal `/data`, so a custom `crud.dataPrefix`
// made the base undetectable and the client fell back.
//
// That fallback was never RIGHT on this deployment — it is a 404; it
// was merely honest, which is why it was pinned rather than fixed.
// `_dataPrefix()` now recovers the split without guessing:
// `routes.metadata` is `{realBase}{metadata.prefix}` over the SAME
// base, so the two advertised routes share exactly `realBase` plus
// whatever their prefixes share, and cutting that common run back to
// its last `/` lands on the boundary. Here that yields `/records`, the
// base `/backend/api/v9`, and the path the server actually mounts.
//
// Contract-first is unchanged: this reads a second ADVERTISED value,
// it does not loosen the parse of the first.
(client as any)['discoveryInfo'] = {
routes: { data: '/backend/api/v9/records', metadata: '/backend/api/v9/meta' },
};
await client.environment('proj-123').meta.getTypes();
expect(String(fetchMock.mock.calls[0][0])).toBe(
'http://localhost:3000/backend/api/v9/environments/proj-123/meta',
);
});

it('[#14879] a custom dataPrefix with no second equation still declines to the convention (case B2)', async () => {
const { client, fetchMock } = createMockClient({ types: [] });
// The decline leg the case above used to carry, kept alive on the
// shape that is genuinely still underivable: `routes.data` does not end
// with the conventional `/data` AND there is no `routes.metadata` to
// supply the missing equation, so `{realBase}{dataPrefix}` stays one
// string with two unknowns. The client must NOT guess a split — it
// falls back to the convention, byte-identical to the pre-#14879
// behavior.
(client as any)['discoveryInfo'] = {
routes: { data: '/backend/api/v9/records' },
};
await client.environment('proj-123').meta.getTypes();
expect(String(fetchMock.mock.calls[0][0])).toBe(
'http://localhost:3000/api/v1/environments/proj-123/meta',
);
});

it('[#14879] two advertised routes sharing no base decline rather than mistake the whole path for a prefix (case B3)', async () => {
const { client, fetchMock } = createMockClient({ types: [] });
// `routes.metadata` present but NOT substituted from this deployment's
// base (its endpoints are off, so it still carries the conventional
// literal). The two routes then share nothing but the leading `/`,
// which is not a shared `realBase` — deriving from it would hand back
// the whole of `routes.data` as the prefix. Decline instead.
(client as any)['discoveryInfo'] = {
routes: { data: '/backend/api/v9/records', metadata: '/api/v1/meta' },
};
await client.environment('proj-123').meta.getTypes();
expect(String(fetchMock.mock.calls[0][0])).toBe(
'http://localhost:3000/api/v1/environments/proj-123/meta',
);
Expand Down
Loading
Loading