Skip to content

Commit fec7848

Browse files
fix(rest): the direct-mount routes read the ONE API base, so apiPath moves all 92 (#6306) (#6822)
* fix(rest): direct-mount registrars consume RestServer.getApiBasePath() (#6306) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017uFVNMmTxLpmfQYiuKM1Yx * test(rest): pin the single API base end to end — the nine follow apiPath (#6306) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017uFVNMmTxLpmfQYiuKM1Yx * docs(changeset): @objectstack/rest patch — direct-mount 路由跟随 apiPath (#6306) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017uFVNMmTxLpmfQYiuKM1Yx * test(rest): the slot-lookup double inherits RestServer instead of re-listing it (#6306) `rest-api-plugin-slot-lookups.test.ts` replaced `RestServer` with a hand-written class carrying exactly two members — the constructor it captures args from, and a no-op `registerRoutes`. That is a contract restated by hand, and it goes stale the moment the composition root calls one more method on the instance: #6306 threads `getApiBasePath()` into the direct-mount registrars, and all five cases here died with `TypeError: restServer.getApiBasePath is not a function` — a file about slot WIRING failing on a question it does not ask. Extend the real class and override only `registerRoutes`, which is the one expensive thing the double existed to suppress. The rest of the contract is inherited, so the next collaborator call lands on the production method. Nothing is paid for it: the real constructor is field assignment plus `new RouteManager(server)` (a `Map`). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017uFVNMmTxLpmfQYiuKM1Yx * docs(changeset): judge the bump `minor`, and re-measure every number on the merged tree (#6306) The numbers in the body are re-measured on this branch after merging `origin/main` (the earlier ones were taken before ~77 commits landed and are void): with `apiPath: '/backend/api/v9'`, 92 routes mount, 83 under `{apiPath}` and exactly 9 at `/api/v1`; `{apiPath}/openapi.json` carries 71 paths before and 79 after; the default mount table, its OpenAPI document and its `/discovery` advertisement diff clean before vs after. Bump judged `minor`. Not `patch`: beyond fixing the defect this changes an observable URL surface under a real config key, and it adds public API — `RestServer.getApiBasePath()` goes `private` → public, which is the thing that carries the single source of truth. Not `major`: nothing authorable is removed or renamed, there is no metadata an author must migrate (so ADR-0087 has nothing to register), default deployments are byte-identical, and affected deployments' clients follow the moved base by construction since PR #6712. The only FROM → TO lands on the operator's own proxy config, and those deployments are split-brain today — this makes `apiPath` honoured in full rather than withdrawing a promise that was ever kept. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017uFVNMmTxLpmfQYiuKM1Yx --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9b9b70f commit fec7848

6 files changed

Lines changed: 415 additions & 26 deletions
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
---
2+
"@objectstack/rest": minor
3+
---
4+
5+
fix(rest): 设置了 `api.apiPath` 时,9 条 direct-mount 路由跟随同一个 API base(#6306)
6+
7+
`RestServer.getApiBasePath()` 回答 `api.apiPath ?? `${basePath}/${version}``,
8+
`rest-api-plugin.ts` 为两个 direct-mount registrar(`packages.*` ×4、
9+
`datasources/:name/external/*` ×5)自行重算了一次 `${basePath}/${version}`,
10+
从不读取 `apiPath`。两个表达式只在 `apiPath` 未设时相等——于是设置了
11+
`apiPath` 的部署同时出现两个 API 前缀。实测(`apiPath: '/backend/api/v9'`,
12+
真实 `createRestApiPlugin(...).start()` 组合、记录型 host server 枚举全部
13+
挂载):**92 条路由中 83 条迁到 `{apiPath}`,恰好 9 条滞留 `/api/v1`**;
14+
`{apiPath}/openapi.json``isUnderBase` 过滤把这 9 条排除在文档之外
15+
(**71 paths**);`/discovery` 也如实通告了滞留位置
16+
(`routes.packages: '/api/v1/packages'`)——通告没有说谎,是挂载本身分裂了。
17+
18+
按 maintainer 裁定(Option 1,单一真相源):registrar 现在直接消费
19+
`restServer.getApiBasePath()` 的返回值——共享同一个值,而不是把 `??`
20+
表达式复制到第二处(复制正是这个缺陷的成因)。`getApiBasePath()` 因此
21+
`private` 变为 public,职责写入其 doc comment。
22+
23+
**行为变化,仅限设置了 `api.apiPath` 的部署**:这 9 条路由的 URL 从
24+
`/api/v1/...` 移到 `{apiPath}/...`,旧前缀不再服务(无兼容双挂载)。
25+
修复后实测 92 条全部挂在 `{apiPath}` 下,`{apiPath}/openapi.json`
26+
完整列出这 9 条(**71 → 79 paths**),`/discovery` 通告 `{apiPath}/packages`
27+
`{apiPath}/datasources`
28+
29+
需要动手的只有**基础设施配置**:若反向代理、健康检查或外部监控里硬编码了
30+
`/api/v1/packages``/api/v1/datasources/*/external/*`,改成 `{apiPath}/…`
31+
**SDK 与应用代码无需改动**:`@objectstack/client`#6633 / PR #6712 起从
32+
`/discovery` 通告的 base 派生这两个面,而通告是已录制挂载的投影,因此客户端
33+
按构造跟随本次移动。该键也没有 authoring 路径可达
34+
(`defineStack({server:{api:…}})` 被 strict 块 loud 拒绝,`api:{apiPath}`
35+
静默 strip,`os serve` 只转发两个 scoping 键),只有程序化组合
36+
`createRestApiPlugin` 的 embedder 能设到它。
37+
38+
**默认配置(未设 `apiPath`)逐字节不变**:两个表达式在该情形下同值;实测
39+
修复前后默认挂载表(92 条)、`{base}/openapi.json`(79 paths)与
40+
`/discovery` 通告完全一致,逐行 diff 无差异。
41+
42+
另修复同一来源的第二处分歧:插件旧表达式用 `||` 兜底(空串 `basePath`
43+
`/api`),`RestServer` 规范化用 `??`(空串保留)——`basePath: ''`
44+
route-manager 面挂 `/v1` 而 9 条挂 `/api/v1`,同样的分裂不需要 `apiPath`
45+
也会出现(实测 83/9)。读同一个值后该分歧不复存在。
46+
47+
Bump 判定为 `minor` 而非 `patch` / `major`。不是 `patch`:除了修缺陷,它
48+
改变了一个真实配置键下可观测的 URL 表面,并且新增了公共 API 面
49+
(`RestServer.getApiBasePath()``private` 转 public,是这次单一真相源的
50+
承载物)。不是 `major`:没有任何可授权(authorable)的键被移除或重命名,
51+
没有需要作者迁移的元数据(因而 ADR-0087 无可登记项),默认部署逐字节不变,
52+
受影响部署的客户端按构造跟随;唯一的 FROM → TO 落在部署方自己的代理配置上,
53+
而这些部署今天本就是 split-brain——本次是让 `apiPath` 被完整遵守,不是收回
54+
一个曾被兑现的承诺。
Lines changed: 294 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,294 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// [#6306] ONE API base for the whole REST surface — pinned end to end through
4+
// the plugin that composes it in production.
5+
//
6+
// The defect this replaces: `RestServer.getApiBasePath()` answers
7+
// `api.apiPath ?? `${basePath}/${version}``, but `rest-api-plugin.ts` built
8+
// its own `${basePath}/${version}` for the two direct-mount registrars and
9+
// never read `apiPath`. The two expressions agree only while `apiPath` is
10+
// unset, so a deployment that set it served TWO API prefixes at once —
11+
// measured on `origin/main` @ 11066f681 with `apiPath: '/backend/api/v9'`:
12+
// 92 routes mounted, 83 under `{apiPath}`, and exactly 9 left behind at
13+
// `/api/v1` (`packages.*` ×4, `datasources/:name/external/*` ×5). Those 9
14+
// were also absent from `{apiPath}/openapi.json` (71 paths vs 79), because
15+
// that document is filtered to this server's base — the filter is what made
16+
// the split visible (#5822 / PR #6303).
17+
//
18+
// What is pinned here, and at which level. This file drives
19+
// `createRestApiPlugin(config).start(ctx)` — the real composition — over a
20+
// recording host server whose handler table IS the mounted surface, then asks
21+
// the three consumers that must agree: where the routes mount, what
22+
// `{base}/openapi.json` documents, and what `{base}/discovery` advertises.
23+
// That plugin-level wiring is deliberately NOT what
24+
// `discovery-advertised-direct-mounts.parity.test.ts` measures: it calls
25+
// `mountAndRecordDirectRoutes` directly with its own `versionedBase`, so it
26+
// pins mounted ⇒ advertised for whatever base it is handed and stays green
27+
// whichever base the plugin picks. The choice of base is this file's subject.
28+
//
29+
// The single-source assertion is the point, not the URLs: each case compares
30+
// the mounted base against `getApiBasePath()` read off an independently
31+
// constructed `RestServer` with the same config. A future edit that
32+
// re-derives the base at the registrars' call site — however correctly —
33+
// fails these, which is the intent: the bug was a second expression, so the
34+
// pin is on there being one.
35+
36+
// Relative imports carry their `.js` extension (see the note in
37+
// `direct-mount-introspection.test.ts`): under `moduleResolution: nodenext` an
38+
// extension-less one does not resolve and every symbol it names becomes `any`.
39+
import { describe, it, expect, vi } from 'vitest';
40+
import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol';
41+
import { RestServer } from './rest-server.js';
42+
import { createRestApiPlugin } from './rest-api-plugin.js';
43+
import { REST_ROUTE_LEDGER } from './rest-route-ledger.js';
44+
import { toTemplatePath } from './openapi-builtin-paths.js';
45+
46+
type Handler = (req: any, res: any) => any;
47+
48+
/** The ledger's own list of the nine, as `VERB {base-relative}` suffixes. */
49+
const DIRECT_MOUNT_SUFFIXES = REST_ROUTE_LEDGER
50+
.filter((e) => e.source === 'direct-mount')
51+
.map((e) => {
52+
const [method, path] = e.route.split(' ');
53+
return { method, suffix: path.replace(/^\/api\/v1/, '') };
54+
});
55+
56+
/**
57+
* A host server whose registrations land in a real handler table — the
58+
* RouteManager rows `RestServer` mounts and the direct-mount registrars' rows
59+
* alike, so one table answers "what is mounted" for the whole boot.
60+
*/
61+
function createRecordingServer() {
62+
const table = new Map<string, Handler>();
63+
const on = (method: string) => vi.fn((path: string, handler: Handler) => {
64+
table.set(`${method} ${path}`, handler);
65+
});
66+
const server = {
67+
table,
68+
get: on('GET'), post: on('POST'), put: on('PUT'), delete: on('DELETE'), patch: on('PATCH'),
69+
use: vi.fn(), listen: vi.fn().mockResolvedValue(undefined), close: vi.fn().mockResolvedValue(undefined),
70+
};
71+
return server;
72+
}
73+
74+
/** Match a concrete URL against the table's `:param` patterns. */
75+
function resolveRoute(table: Map<string, Handler>, method: string, url: string) {
76+
const urlSegs = url.split('/');
77+
for (const [key, handler] of table) {
78+
const [m, pattern] = key.split(' ');
79+
if (m !== method) continue;
80+
const patSegs = pattern.split('/');
81+
if (patSegs.length !== urlSegs.length) continue;
82+
const params: Record<string, string> = {};
83+
let ok = true;
84+
for (let i = 0; i < patSegs.length; i++) {
85+
if (patSegs[i].startsWith(':')) params[patSegs[i].slice(1)] = urlSegs[i];
86+
else if (patSegs[i] !== urlSegs[i]) { ok = false; break; }
87+
}
88+
if (ok) return { handler, params };
89+
}
90+
return undefined;
91+
}
92+
93+
async function drive(entry: { handler: Handler; params: Record<string, string> }, req: Record<string, unknown> = {}) {
94+
let body: any;
95+
let statusCode = 200;
96+
const res: any = {
97+
status: (c: number) => { statusCode = c; return res; },
98+
json: (b: any) => { body = b; },
99+
header: () => res,
100+
send: () => {},
101+
};
102+
await entry.handler({ params: entry.params, query: {}, body: {}, headers: { host: 'example.test' }, ...req }, res);
103+
return { statusCode, body };
104+
}
105+
106+
function makeProtocol() {
107+
const engine = { registry: { getObject: (_n: string) => undefined, getRegisteredTypes: () => [] } };
108+
const services = new Map<string, any>([['package', { list: async () => [] }]]);
109+
return new ObjectStackProtocolImplementation(engine as any, () => services);
110+
}
111+
112+
function createCtx(services: Record<string, unknown>) {
113+
return {
114+
registerService: vi.fn(),
115+
getService: vi.fn((name: string) => {
116+
if (name in services) return services[name];
117+
throw new Error(`Service '${name}' not found`);
118+
}),
119+
getServices: vi.fn(() => new Map(Object.entries(services))),
120+
hook: vi.fn(),
121+
trigger: vi.fn().mockResolvedValue(undefined),
122+
logger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() },
123+
getKernel: vi.fn(),
124+
};
125+
}
126+
127+
/**
128+
* Boot the REST plugin exactly as production does, and report both the
129+
* mounted surface and the base an independently constructed `RestServer`
130+
* computes from the same config — the two things every case compares.
131+
*/
132+
async function bootPlugin(apiConfig?: Record<string, unknown>) {
133+
const server = createRecordingServer();
134+
const ctx = createCtx({
135+
'http.server': server,
136+
protocol: makeProtocol(),
137+
package: { list: vi.fn(), get: vi.fn(), publish: vi.fn(), delete: vi.fn() },
138+
'external-datasource': { listRemoteTables: async () => [{ name: 'customers' }] },
139+
});
140+
await createRestApiPlugin(apiConfig as any).start!(ctx as any);
141+
142+
// The base the server that owns the surface computes — the single source.
143+
const expectedBase = new RestServer(
144+
createRecordingServer() as any,
145+
makeProtocol() as any,
146+
(apiConfig?.api ?? {}) as any,
147+
).getApiBasePath();
148+
149+
return { server, table: server.table, expectedBase };
150+
}
151+
152+
function mountedKeys(table: Map<string, Handler>): string[] {
153+
return [...table.keys()];
154+
}
155+
156+
async function serveOpenApi(table: Map<string, Handler>, base: string) {
157+
const entry = resolveRoute(table, 'GET', `${base}/openapi.json`);
158+
expect(entry, `GET ${base}/openapi.json must be mounted for this pin to mean anything`).toBeDefined();
159+
const { body } = await drive(entry!, { path: `${base}/openapi.json` });
160+
return body;
161+
}
162+
163+
async function readDiscovery(table: Map<string, Handler>, base: string) {
164+
const entry = resolveRoute(table, 'GET', `${base}/discovery`);
165+
expect(entry, `GET ${base}/discovery must be mounted`).toBeDefined();
166+
const { body } = await drive(entry!);
167+
return body;
168+
}
169+
170+
function documented(doc: any, wirePath: string, method: string): boolean {
171+
return Boolean(doc?.paths?.[toTemplatePath(wirePath)]?.[method.toLowerCase()]);
172+
}
173+
174+
// ---------------------------------------------------------------------------
175+
// the move — a deployment that sets `apiPath`
176+
// ---------------------------------------------------------------------------
177+
178+
describe('#6306 — with `apiPath` set, the direct-mount routes follow it', () => {
179+
const API_PATH = '/backend/api/v9';
180+
const config = { api: { api: { apiPath: API_PATH } } };
181+
182+
it('mounts all nine under {apiPath}, and leaves nothing behind at the convention prefix', async () => {
183+
const { table, expectedBase } = await bootPlugin(config);
184+
185+
// The base is the server's, not a second expression that happens to agree.
186+
expect(expectedBase).toBe(API_PATH);
187+
188+
for (const { method, suffix } of DIRECT_MOUNT_SUFFIXES) {
189+
expect(
190+
mountedKeys(table),
191+
`${method} ${expectedBase}${suffix} must mount under the one API base`,
192+
).toContain(`${method} ${expectedBase}${suffix}`);
193+
}
194+
195+
// The whole surface moved, not merely the nine: no route is left at the
196+
// `/api/v1` convention. This is the split itself — on `origin/main` this
197+
// set had exactly 9 members.
198+
const stragglers = mountedKeys(table).filter((k) => k.split(' ')[1].startsWith('/api/v1'));
199+
expect(stragglers, 'no route may stay at /api/v1 when apiPath moves the surface').toEqual([]);
200+
});
201+
202+
it('documents all nine in {apiPath}/openapi.json — the filter that made the split visible now includes them', async () => {
203+
const { table, expectedBase } = await bootPlugin(config);
204+
const doc = await serveOpenApi(table, expectedBase);
205+
206+
for (const { method, suffix } of DIRECT_MOUNT_SUFFIXES) {
207+
expect(
208+
documented(doc, `${expectedBase}${suffix}`, method),
209+
`${method} ${expectedBase}${suffix} is mounted but not documented`,
210+
).toBe(true);
211+
}
212+
// …and the stale prefix is documented nowhere, so the document describes
213+
// one surface rather than two.
214+
expect(Object.keys(doc.paths).filter((p) => p.startsWith('/api/v1'))).toEqual([]);
215+
});
216+
217+
it('advertises the moved bases in {apiPath}/discovery, and the advertised URLs answer', async () => {
218+
const { table, expectedBase } = await bootPlugin(config);
219+
const discovery = await readDiscovery(table, expectedBase);
220+
221+
// No edit was needed in the advertising code for this: `routes.packages` /
222+
// `routes.datasources` are projections of the recorded mounts (#6633), so
223+
// moving the mount moved the advertisement.
224+
expect(discovery.routes.packages).toBe(`${expectedBase}/packages`);
225+
expect(discovery.routes.datasources).toBe(`${expectedBase}/datasources`);
226+
227+
const pkg = resolveRoute(table, 'GET', discovery.routes.packages);
228+
expect(pkg, 'the advertised packages URL must be mounted').toBeDefined();
229+
expect((await drive(pkg!)).statusCode).toBe(200);
230+
231+
const ext = resolveRoute(table, 'GET', `${discovery.routes.datasources}/pg_main/external/tables`);
232+
expect(ext, 'the advertised datasources base must be the base of the mounted family').toBeDefined();
233+
expect((await drive(ext!)).statusCode).toBe(200);
234+
});
235+
});
236+
237+
// ---------------------------------------------------------------------------
238+
// the second divergent expression the single source also collapses
239+
// ---------------------------------------------------------------------------
240+
241+
describe('#6306 — the base is READ, not rebuilt: `??` and `||` no longer disagree', () => {
242+
it('an empty `basePath` puts the nine where the rest of the surface already was', async () => {
243+
// A second, independent way the two expressions differed: the plugin
244+
// defaulted with `||` (empty string ⇒ `/api`) while `RestServer`
245+
// normalizes with `??` (empty string kept). So `basePath: ''` mounted the
246+
// RouteManager surface at `/v1` and the nine at `/api/v1` — the same
247+
// split, reached without `apiPath` at all. Reading the base cannot
248+
// disagree with itself.
249+
const { table, expectedBase } = await bootPlugin({ api: { api: { basePath: '', version: 'v1' } } });
250+
expect(expectedBase).toBe('/v1');
251+
252+
for (const { method, suffix } of DIRECT_MOUNT_SUFFIXES) {
253+
expect(mountedKeys(table)).toContain(`${method} /v1${suffix}`);
254+
}
255+
expect(mountedKeys(table).filter((k) => k.split(' ')[1].startsWith('/api/v1'))).toEqual([]);
256+
});
257+
});
258+
259+
// ---------------------------------------------------------------------------
260+
// the baseline — unchanged where the two expressions always agreed
261+
// ---------------------------------------------------------------------------
262+
263+
describe('#6306 — default and conventional configs are unchanged', () => {
264+
// NOTE, honestly: these two cases are green both before and after the fix —
265+
// `apiPath ?? `${basePath}/${version}`` and `${basePath}/${version}` are the
266+
// same string here, which is exactly why the defect hid for so long. They
267+
// are not reverse-verification evidence; they are the regression floor,
268+
// pinning that single-sourcing moved nothing for deployments that never set
269+
// `apiPath` (measured: the default mount list is identical, 92 routes,
270+
// before and after).
271+
it('default config keeps all nine at /api/v1, documented and advertised there', async () => {
272+
const { table, expectedBase } = await bootPlugin(undefined);
273+
expect(expectedBase).toBe('/api/v1');
274+
275+
for (const { method, suffix } of DIRECT_MOUNT_SUFFIXES) {
276+
expect(mountedKeys(table)).toContain(`${method} /api/v1${suffix}`);
277+
}
278+
const doc = await serveOpenApi(table, '/api/v1');
279+
for (const { method, suffix } of DIRECT_MOUNT_SUFFIXES) {
280+
expect(documented(doc, `/api/v1${suffix}`, method)).toBe(true);
281+
}
282+
const discovery = await readDiscovery(table, '/api/v1');
283+
expect(discovery.routes.packages).toBe('/api/v1/packages');
284+
expect(discovery.routes.datasources).toBe('/api/v1/datasources');
285+
});
286+
287+
it('a conventional custom basePath/version behaves identically under both expressions', async () => {
288+
const { table, expectedBase } = await bootPlugin({ api: { api: { basePath: '/gateway', version: 'v3' } } });
289+
expect(expectedBase).toBe('/gateway/v3');
290+
for (const { method, suffix } of DIRECT_MOUNT_SUFFIXES) {
291+
expect(mountedKeys(table)).toContain(`${method} /gateway/v3${suffix}`);
292+
}
293+
});
294+
});

0 commit comments

Comments
 (0)