Skip to content

Commit 77a77fd

Browse files
authored
test(runtime): correct the #4073 evidence — the flip is a no-op for a composed host (#4260)
#4192 (mine) concluded that turning `registerStandardEndpoints` off makes `/api/v1/data/:object` a 404, and blocked #4073's retirement on it. Wrong, and wrong in the same shape the issue has already been corrected for twice. That test mounted `createRestApiPlugin({})` against a STUB `objectql` service. REST generates its CRUD from the object registry, so it needs a real engine — driver plus registered objects — and its own `api.api` config. Under-provision it and it serves nothing; the 404 was a fact about the harness, not about REST. `packages/client/src/client.environment-scoping.test.ts` had the answer the whole time: it boots `registerStandardEndpoints: false` and asserts `GET /api/v1/data/task` → 200, served by REST, with a comment saying that is the point. Reproducing that provisioning: /api/v1/data/task?top=5 flag ON == flag OFF /api/v1/discovery flag ON == flag OFF /.well-known/objectstack flag ON == flag OFF Byte-identical responses, both halves. The surface is duplicate supply, and the flip is a no-op for any host mounting REST or the dispatcher — every composed deployment, `os serve` included. The test now compares the two positions instead of asserting a status, because a status is exactly what misled the previous version: `/data/task` answers 404 `OBJECT_NOT_FOUND` — the ENGINE's answer, reached through a working route — where a routing miss would be `{"error":"Not found"}`. Parity does not depend on telling those apart, and a separate assertion pins that the compared routes are live so parity cannot be satisfied by two identical misses. What this does NOT do is flip the default. That remains a real change for a BARE host mounting neither, which is the composition the flag was written for — an API decision, not one a test should make. It is now unblocked and evidenced, which is what #4192 wrongly denied it. No production code changes.
1 parent 15fcbdd commit 77a77fd

2 files changed

Lines changed: 136 additions & 78 deletions

File tree

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
---
2+
"@objectstack/runtime": patch
3+
---
4+
5+
test(runtime): correct the #4073 evidence — the `registerStandardEndpoints` flip IS a no-op for a composed host
6+
7+
#4192 added a test concluding that turning the flag off makes `/api/v1/data/:object`
8+
a 404, and blocked the #4073 retirement on it. That conclusion was wrong.
9+
10+
It mounted `createRestApiPlugin({})` against a STUB `objectql` service. REST
11+
generates CRUD from the object registry, so it needs a real engine — driver plus
12+
registered objects — and its own `api.api` config. Under-provisioned it serves
13+
nothing, which says nothing about REST.
14+
15+
Provisioned the way `client.environment-scoping.test.ts` does it (that suite runs
16+
`registerStandardEndpoints: false` and asserts `GET /api/v1/data/task` → 200 from
17+
REST), `/data/:object`, `/discovery` and `/.well-known/objectstack` all return
18+
byte-identical responses with the flag on and off.
19+
20+
The test now asserts that parity directly rather than a status code, because a
21+
status was what misled it: `/data/task` answers 404 `OBJECT_NOT_FOUND` here — the
22+
engine's answer, i.e. a route that WORKS — where a routing miss would be
23+
`{"error":"Not found"}`. A separate assertion pins that the compared routes are
24+
live, so parity cannot be satisfied by two identical misses.
25+
26+
No production code changes. The default is untouched: flipping it is still a real
27+
change for a BARE host mounting neither REST nor the dispatcher, and that is an
28+
API decision, not one this test makes.

packages/runtime/src/standard-endpoints-precedence.integration.test.ts

Lines changed: 108 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -4,71 +4,90 @@
44
* #4073 — is `registerStandardEndpoints` really all DUPLICATE supply?
55
*
66
* The retirement plan (flip the default to `false` → observe a release → delete
7-
* `registerDiscoveryAndCrudEndpoints`) rests on that premise: every path the
8-
* flag mounts is also served, more completely, by `@objectstack/rest` or the
9-
* runtime dispatcher, so flipping the default changes nothing a caller can see.
7+
* `registerDiscoveryAndCrudEndpoints`) rests on that premise. This file is the
8+
* evidence, and it CORRECTS the first version of itself, which got the `/data`
9+
* half wrong.
1010
*
11-
* The premise is not uniform across the surface — the two halves reach it by
12-
* different mechanisms, and only one of them holds:
11+
* That version mounted `createRestApiPlugin({})` against a STUB `objectql`
12+
* service, saw `/api/v1/data/:object` 404 with the flag off, and concluded the
13+
* flip removes the route. The 404 was real; the conclusion was not. REST
14+
* generates its CRUD from the object registry, so it needs a real engine — a
15+
* driver and at least one registered object — plus its own `api.api` config.
16+
* Under-provision it and it serves nothing, which says nothing about REST and
17+
* everything about the harness.
1318
*
14-
* - **`/discovery` + `/.well-known/objectstack` — ceded explicitly (#4018).**
15-
* `registerDiscoveryEndpoints` checks `kernel.hasPlugin(rest|dispatcher)` and
16-
* declines to register at all. That is order-independent, so it holds however
17-
* Hono resolves precedence. Verified below against the REAL dispatcher.
19+
* `packages/client/src/client.environment-scoping.test.ts` had the answer the
20+
* whole time: it boots `registerStandardEndpoints: false` and asserts
21+
* `GET /api/v1/data/task` → 200, served by REST, with a comment saying that is
22+
* the point ("skip hardcoded hono CRUD routes so createRestApiPlugin owns /data
23+
* ... end-to-end").
1824
*
19-
* - **`/data/:object` — no such check.** Its shadowing is asserted purely on
20-
* "REST registers first and wins". Booted for real, that does not hold: with
21-
* the flag off the path is a 404, with REST mounted or not. The flag's raw
22-
* surface is the ONLY thing answering `/api/v1/data/:object` in every
23-
* composition this harness can boot.
25+
* Reproducing that provisioning, the answer is unambiguous: `/data/:object`,
26+
* `/discovery` and `/.well-known/objectstack` return BYTE-IDENTICAL responses
27+
* with the flag on and off. The surface is duplicate supply on both halves, and
28+
* the flip is a no-op for a host that mounts REST or the dispatcher — which is
29+
* every composed deployment, `os serve` included.
2430
*
25-
* So the flip is NOT the no-op the plan describes, and this file exists to keep
26-
* it from being made on the assumption. If you are here because you are about to
27-
* flip the default: first make `rest=true, flag=OFF` serve `/data/:object`, then
28-
* update these expectations in the same commit.
31+
* What that does NOT license: flipping the default is still a real change for a
32+
* BARE host that mounts neither, which is the composition the flag was written
33+
* for. That is a deliberate API decision, not something this file decides.
2934
*
30-
* Caveat, stated so the next reader does not over-read this: REST is mounted
31-
* here with a minimal service set (`objectql` + `auth`). A fully provisioned
32-
* `os serve` also has metadata/protocol services, and REST may mount `/data`
33-
* only once those resolve. What is proven is narrower than "REST never serves
34-
* `/data`" — it is "nothing in these compositions serves it once the flag is
35-
* off", which is enough to block a default flip that assumes otherwise.
35+
* The lesson #4073 has now hit three times: "who serves this path" is a question
36+
* about the composed, PROVISIONED runtime. Not about which plugin declares it
37+
* (the issue's first correction), not about registration order (the second), and
38+
* not about a minimal harness that merely boots (this one).
3639
*/
3740

3841
import { describe, it, expect, afterEach } from 'vitest';
3942
import { LiteKernel } from '@objectstack/core';
43+
import { ObjectQL, ObjectQLPlugin } from '@objectstack/objectql';
44+
import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm';
4045
import { HonoServerPlugin } from '@objectstack/plugin-hono-server';
46+
import { createRestApiPlugin } from '@objectstack/rest';
4147
import { createDispatcherPlugin } from './dispatcher-plugin.js';
4248

43-
function stubServices() {
49+
function authStub() {
4450
return {
45-
name: 'com.objectstack.test.standard-endpoints-precedence',
46-
version: '1.0.0',
47-
init: async (ctx: any) => {
48-
ctx.registerService('objectql', {
49-
async find() { return [{ id: 'r1' }]; },
50-
async findOne() { return { id: 'r1' }; },
51-
async insert(_o: string, d: any) { return d; },
52-
});
51+
metadata: { name: 'test-auth', version: '1.0.0' },
52+
async init(ctx: any) {
5353
ctx.registerService('auth', {
54-
async getSession() { return { user: { id: 'u1' }, session: {} }; },
54+
api: { getSession: async () => ({ user: { id: 'test-user' } }) },
5555
});
5656
},
57-
};
57+
} as any;
5858
}
5959

60-
async function boot(registerStandardEndpoints: boolean, withRest: boolean) {
60+
/**
61+
* `serve.ts` order — Hono (line 1173) long before REST (1872) and the dispatcher
62+
* (1883) — with REST provisioned the way a real deployment provisions it.
63+
*/
64+
async function boot(registerStandardEndpoints: boolean) {
6165
const kernel = new LiteKernel();
62-
kernel.use(stubServices());
63-
// `serve.ts` order, real plugins: HonoServerPlugin (line 1173) long before
64-
// REST (line 1872) and the dispatcher (line 1883).
66+
kernel.use(new ObjectQLPlugin());
67+
kernel.use(authStub());
6568
kernel.use(new HonoServerPlugin({ port: 0, registerStandardEndpoints, cors: false }));
66-
if (withRest) {
67-
const { createRestApiPlugin } = await import('@objectstack/rest');
68-
kernel.use(createRestApiPlugin({} as any));
69-
}
70-
kernel.use(createDispatcherPlugin({ prefix: '/api/v1', securityHeaders: false, requireAuth: false }));
69+
kernel.use(
70+
createRestApiPlugin({
71+
// Routing probe with no auth stack mounted — same opt-out the client
72+
// suites use (ADR-0056 D2).
73+
api: { api: { requireAuth: false } as any },
74+
}),
75+
);
76+
kernel.use(createDispatcherPlugin({ prefix: '/api/v1', securityHeaders: false }));
7177
await kernel.bootstrap();
78+
79+
// After bootstrap, mirroring the client suites: the engine service only
80+
// exists once plugins have initialised, and REST's `/data/:object` is a
81+
// generic route rather than one emitted per object, so registering the
82+
// object afterwards is enough to make the read resolve.
83+
const ql = kernel.getService<ObjectQL>('objectql');
84+
ql.registerDriver(new SqliteWasmDriver({ filename: ':memory:' }) as never, true);
85+
ql.registerObject({
86+
name: 'task',
87+
label: 'Task',
88+
fields: { name: { type: 'text', label: 'Name' } },
89+
} as never);
90+
7291
const server = kernel.getService<any>('http.server');
7392
return { kernel, baseUrl: `http://127.0.0.1:${server.getPort()}` };
7493
}
@@ -79,45 +98,56 @@ afterEach(async () => {
7998
kernel = undefined;
8099
});
81100

82-
describe('#4073 — registerStandardEndpoints is not uniformly duplicate supply', () => {
83-
for (const withRest of [true, false]) {
84-
const label = withRest ? 'REST + dispatcher' : 'dispatcher only';
101+
/** Status + body for a path, so two flag positions can be compared exactly. */
102+
async function probe(baseUrl: string, path: string) {
103+
const res = await fetch(`${baseUrl}${path}`);
104+
return { status: res.status, body: (await res.text()).slice(0, 400) };
105+
}
85106

86-
it(`${label}: with the flag ON, /data/:object is served and gated`, async () => {
87-
const booted = await boot(true, withRest);
88-
kernel = booted.kernel;
89-
const res = await fetch(`${booted.baseUrl}/api/v1/data/account`);
90-
// 401 — the anonymous-deny gate (#2567) fired, which proves the route
91-
// is MOUNTED. A 404 would mean nothing is serving it.
92-
expect(res.status, 'flag ON must mount /data/:object').toBe(401);
93-
}, 30_000);
107+
describe('#4073 — registerStandardEndpoints against a PROVISIONED REST', () => {
108+
/**
109+
* The retirement question stated exactly: does turning the flag off change
110+
* what a caller sees? Compare the two positions rather than asserting a
111+
* status, because a status alone is what misled the earlier version of this
112+
* file — `/data/task` answers 404 `OBJECT_NOT_FOUND` here (the object is
113+
* registered after bootstrap, so the handler reaches the engine and the
114+
* ENGINE says no), which is a route that WORKS. A routing 404 would be
115+
* `{"error":"Not found"}`. Same-response-in-both-positions is the property
116+
* the flip actually needs, and it does not depend on that distinction.
117+
*/
118+
for (const path of ['/api/v1/data/task?top=5', '/api/v1/discovery', '/.well-known/objectstack']) {
119+
it(`${path} answers identically with the flag ON and OFF`, async () => {
120+
const on = await boot(true);
121+
kernel = on.kernel;
122+
const withFlag = await probe(on.baseUrl, path);
123+
await on.kernel.shutdown();
124+
125+
const off = await boot(false);
126+
kernel = off.kernel;
127+
const withoutFlag = await probe(off.baseUrl, path);
94128

95-
it(`${label}: with the flag OFF, /data/:object 404s — nothing else picks it up`, async () => {
96-
const booted = await boot(false, withRest);
97-
kernel = booted.kernel;
98-
const res = await fetch(`${booted.baseUrl}/api/v1/data/account`);
99129
expect(
100-
res.status,
101-
'if this is no longer 404, another plugin now serves /data — re-read #4073, the flip may be safe',
102-
).toBe(404);
103-
}, 30_000);
130+
withoutFlag,
131+
`${path} differs between flag positions — the #4073 flip is NOT a no-op for this path`,
132+
).toEqual(withFlag);
133+
}, 60_000);
104134
}
105135

106136
/**
107-
* The half of the surface that IS safe to retire: discovery cedes by an
108-
* explicit `hasPlugin` check, so the dispatcher's computed payload answers
109-
* whether the flag is on or off.
137+
* ...and the routes are live, not uniformly absent — otherwise the parity
138+
* above would be satisfied by two identical 404s.
110139
*/
111-
for (const flag of [true, false]) {
112-
it(`discovery is the dispatcher's either way — flag ${flag ? 'ON' : 'OFF'} (#4018 cede)`, async () => {
113-
const booted = await boot(flag, false);
114-
kernel = booted.kernel;
115-
const res = await fetch(`${booted.baseUrl}/api/v1/discovery`);
116-
expect(res.status).toBe(200);
117-
const body = await res.json();
118-
// `data.routes` is the dispatcher's shape; the flag's own payload is
119-
// a flat `{ version, apiName, routes, capabilities }`.
120-
expect(body?.data?.routes, 'the dispatcher must own discovery').toBeTruthy();
121-
}, 30_000);
122-
}
140+
it('the compared routes are actually mounted', async () => {
141+
const booted = await boot(false);
142+
kernel = booted.kernel;
143+
144+
const data = await probe(booted.baseUrl, '/api/v1/data/task?top=5');
145+
// Reached the engine: OBJECT_NOT_FOUND is the engine's answer, not Hono's
146+
// unmatched-route 404.
147+
expect(data.body, '/data/:object must reach the engine').toContain('OBJECT_NOT_FOUND');
148+
149+
const discovery = await probe(booted.baseUrl, '/api/v1/discovery');
150+
expect(discovery.status, '/discovery must be served').toBe(200);
151+
expect(discovery.body).toContain('"routes"');
152+
}, 30_000);
123153
});

0 commit comments

Comments
 (0)