Skip to content

Commit aee1806

Browse files
os-zhuangclaude
andauthored
feat(spec,service-datasource)!: graduate the driver factory's 4 legacy config ?? fallbacks into an ADR-0087 conversion (#4456) (#4637)
The four undeclared read-side fallbacks createDefaultDatasourceDriverFactory kept after #4410 — sqlite `file`/`database` -> `filename`, pg/mysql `connectionString` -> `url`, pg/mysql/mongo `user` -> `username`, mongo `uri` -> `url` — become the declared, driver-aware conversion entry `datasource-config-driver-key-aliases` (retired from the load path: the authoring gate already rejects each spelling with a rename hint), and the `??` chains are deleted. Stored sys_metadata rows written before the #4410 gate keep loading: every rehydration seam replays the full chain (applyConversionsToStoredItem, #3903), including the DatasourceAdminServicePlugin restore path, which read raw JSON and now converts. Without that, deleting the fallbacks would have silently re-pointed a stored sqlite `file:` datasource at `:memory:`. Also adds the mapDatasources walker (conversions/walk.ts), registers the entry in the step-17 migration chain, regenerates spec-changes.json / protocol-upgrade-guide / driver-sqlite reference, and pins: stored rows with each legacy key load canonical; the factory no longer honours any legacy spelling handed to it directly. Claude-Session: https://claude.ai/code/session_012C2cd7tL8QDoZ2QKN3djJ5 Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0a936ea commit aee1806

14 files changed

Lines changed: 454 additions & 34 deletions

File tree

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/service-datasource": minor
4+
---
5+
6+
feat(spec,service-datasource): graduate the driver factory's four legacy `datasource.config` `??` fallbacks into an ADR-0087 conversion (#4456)
7+
8+
`createDefaultDatasourceDriverFactory` still carried four undeclared read-side
9+
`??` fallbacks that predate the #4410 config gate: sqlite `file`/`database`
10+
(canonical `filename`), postgres/mysql `connectionString` (canonical `url`),
11+
postgres/mysql/mongo `user` (canonical `username`), and mongo `uri` (canonical
12+
`url`). They were never part of the contract — no schema, form, doc or example
13+
ever named them — and they kept working only because the reader was lenient
14+
(AGENTS.md Prime Directive #12 debt).
15+
16+
**FROM → TO, applied automatically at load** by the new conversion entry
17+
`datasource-config-driver-key-aliases` (retired-from-load-path; replayed over
18+
stored `sys_metadata` rows by `applyConversionsToStoredItem` and by
19+
`os migrate meta`):
20+
21+
- sqlite / sqlite-wasm: `config.file` / `config.database``config.filename`
22+
- postgres / mysql: `config.connectionString``config.url`, `config.user``config.username`
23+
- mongo: `config.uri``config.url`, `config.user``config.username`
24+
25+
The mapping is driver-aware — `database` renames only under sqlite, where it
26+
aliased the file path; for postgres/mysql/mongo it is a canonical key and is
27+
untouched. A canonical key already present wins; the legacy alias is left
28+
shadowed (the factory's `??` precedence, preserved).
29+
30+
**Behaviour change (the deletion):** the factory now reads exactly one spelling
31+
per key. A `DatasourceConnectionSpec` handed to the factory *directly* with a
32+
legacy spelling is no longer honoured — authored metadata was already rejected
33+
by the per-driver zod gate with a rename hint (#4410), and stored runtime
34+
datasource rows are canonicalized at every rehydration seam (including the
35+
`sys_metadata` restore path in `DatasourceAdminServicePlugin`, which now
36+
replays the full conversion chain), so no supported path still produces the
37+
legacy shape. One-line fix for hand-built specs: use the canonical key from
38+
the table above.

content/docs/references/data/driver-sqlite.mdx

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,17 +19,15 @@ fell back to `:memory:`, and their data vanished on restart with every signal
1919

2020
saying the datasource was configured.
2121

22-
`file` and `database` are a different case — the factory reads them as
22+
`file` and `database` once also worked, purely because the factory read them
2323

24-
undeclared `??` fallbacks, so they happened to work while being documented
24+
as undeclared `??` fallbacks. That tolerance has graduated into the declared
2525

26-
nowhere. They are named as renames here rather than blessed: one strict
26+
ADR-0087 conversion `datasource-config-driver-key-aliases` (#4456): stored
2727

28-
contract beats a spelling that works only because a reader is lenient
28+
rows are rewritten to `filename` at load, the factory reads one spelling,
2929

30-
(AGENTS.md Prime Directive #12). The factory keeps its tolerance for records
31-
32-
already persisted that way; no new one can be authored.
30+
and authoring rejects both with the rename hint below.
3331

3432
<Callout type="info">
3533
**Source:** `packages/spec/src/data/driver/sqlite.zod.ts`

docs/protocol-upgrade-guide.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,8 @@ The same kind of retirement covers `wait`'s timeout pair (#4158). `waitEventConf
158158

159159
Closing the same audit on the data side, `datasource.readReplicas` is removed (#4468). It described replica connections nothing ever opened: `ConnectableDatasource` and `DatasourceConnectionSpec` carry no replicas field, the driver factory never reads the key, and no query path distinguishes a read from a write — read/write splitting does not exist in the platform, so every statement always went to the primary. A lossless delete with no target to move to; front replicas behind one endpoint (pgpool, ProxySQL, an RDS reader endpoint) and point `config` at it. Notable as the case that shows how a key gets MORE convincing as it stays dead: #4410, closing the datasource-config gap, taught the schema to validate each replica entry against the declared driver's config contract, so sources written in between carry replica blocks that were genuinely checked — precise hosts, correct port types, typos rejected. Precision applied to an inert slot reads as evidence the slot is live, which is why ADR-0049 asks for a consumer rather than for rigor. Retired from the load path with the rest of the keys that misdescribed themselves.
160160

161+
The datasource close-out also graduates the four legacy `datasource.config` spellings the shared driver factory still tolerated via undeclared read-side `??` fallbacks (#4456, the #4410 follow-up): sqlite `file`/`database` (use `filename`), postgres/mysql `connectionString` (use `url`) and `user` (use `username`), and mongo `uri` (use `url`) and `user` (use `username`). #4410 made the authoring gate reject each with a rename hint, but a runtime datasource persisted in `sys_metadata` before the gate kept working only because the factory read leniently — and deleting that tolerance without a conversion would have silently moved data (a stored sqlite `file:` row falls back to `:memory:`). The `datasource-config-driver-key-aliases` conversion rewrites the stored shape to the canonical keys at every rehydration seam, the factory now reads exactly one spelling per key, and the four `??` chains are deleted. Driver-aware by construction: `database` renames only under sqlite, where it aliased the file path — for every other driver it is a canonical key and is untouched. Retired from the load path not for lying but because the authoring gate already rejects the spellings loudly; the chain and the stored-row replay are the seams that accept them.
162+
161163
The `script` flow node converges on its one real path (#4343). It had four ways to name what it ran and only one of them ran anything: `config.actionType: 'email' | 'slack'` were logger-backed stubs that wrote a line, reported success and delivered nothing under any configuration — with `config.template` / `.recipients` / `.variables` feeding a message no channel ever sent; inline `config.script` was recognized and never executed (the built-in runtime has no server-side JS sandbox), so the node warned and no-op'd; and every other `actionType` value was shorthand for a registered-function name, a second spelling of `config.function`. All five keys are retired and `function` becomes required, which is also what finally made the contract PARSEABLE: while the legal key set depended on `actionType`, a flat parse would either reject valid shapes or wave everything through, so `script` (with `subflow`) now runs through the same execute-time contract parse #4277 gave the flat builtins. A shorthand `actionType` CONVERTS into `function` — that is what it meant — unless `function` is already set, in which case it was dead metadata the executor never reached. The other four are dropped outright: nothing read them, so there is no value to preserve, and rebuilding the intent is an authoring decision the tombstones prescribe per branch (a `notify` node for mail — it delivers through the messaging service, the in-app inbox by default and real email once `@objectstack/plugin-email` is installed; a `connector_action` with the Slack connector, or an `http` node posting to a webhook, for Slack; a registered function for an inline body). Retired from the load path for the same reason as the rest: absorbing `actionType: 'email'` silently would let an author keep believing the flow sends mail.
162164

163165
### Mechanical (applied for you)
@@ -190,6 +192,7 @@ The `script` flow node converges on its one real path (#4343). It had four ways
190192
| `datasource-read-replicas-removed` | `datasource.readReplicas` | datasource key 'readReplicas' removed (#4468 — no driver opened a replica connection and no query path splits reads from writes; front replicas behind one endpoint and point `config` at it) | retired — `migrate meta` only |
191193
| `datasource-capabilities-removed` | `datasource.capabilities` | datasource key 'capabilities' removed (#4583 — eleven flags no code read; pushdown comes from the driver's own supports.*, and `readOnly` never made anything read-only) | retired — `migrate meta` only |
192194
| `datasource-inert-blocks-removed` | `datasource.retryPolicy / datasource.healthCheck / datasource.external.label / datasource.external.requirePermission` | datasource keys 'retryPolicy'/'healthCheck' and external 'label'/'requirePermission' removed (#4583 — nothing retried, nothing probed on a schedule, and the federation label/permission were read by nobody) | retired — `migrate meta` only |
195+
| `datasource-config-driver-key-aliases` | `datasource.config` | datasource config keys → canonical per driver: sqlite 'file'/'database' → 'filename', postgres/mysql 'connectionString' → 'url' and 'user' → 'username', mongo 'uri' → 'url' and 'user' → 'username' (#4456 — driver-factory `??` fallback graduation) | retired — `migrate meta` only |
193196
| `flow-node-script-branch-keys-removed` | `flow.node.script.config.actionType / flow.node.script.config.template / flow.node.script.config.recipients / flow.node.script.config.variables / flow.node.script.config.script` | script flow-node config keys 'actionType' (→ 'function' when it was shorthand for one; otherwise removed — 'email'/'slack' were logger-backed stubs that delivered nothing), plus 'template' / 'recipients' / 'variables' (fed those stubs) and 'script' (inline JS the runtime never executed) (#4343) | retired — `migrate meta` only |
194197

195198
### Semantic (delegated to you, with acceptance criteria)

packages/services/service-datasource/src/__tests__/datasource-admin-plugin.test.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import { describe, it, expect } from 'vitest';
44
import type { IDatasourceAdminService, IDatasourceDriverFactory } from '../contracts/index.js';
5+
import type { DatasourceAdminService } from '../datasource-admin-service.js';
56
import {
67
DatasourceAdminServicePlugin,
78
type DatasourceAdminServicePluginOptions,
@@ -285,6 +286,48 @@ describe('DatasourceAdminServicePlugin: runtime datasource durability', () => {
285286
expect(after.find((d) => d.name === 'demo_ext')?.origin).toBe('runtime');
286287
});
287288

289+
// #4456 — this restore path is a stored-row rehydration seam (ADR-0087 D2
290+
// addendum, #3903): it reads sys_metadata directly, so it must replay the
291+
// conversion chain itself. A row persisted before the #4410 config gate may
292+
// carry the legacy spellings the factory's deleted `??` fallbacks used to
293+
// tolerate; without the replay, a sqlite `file:` row would silently fall
294+
// back to `:memory:` — the data-loss shape the conversion exists to prevent.
295+
it('restores a pre-#4410 row with legacy config keys CANONICAL (conversion chain replayed)', async () => {
296+
const data = fakeSysMetadataEngine();
297+
const now = new Date().toISOString();
298+
for (const [name, driver, config] of [
299+
['legacy_sqlite', 'sqlite', { file: '/tmp/legacy.db' }],
300+
['legacy_pg', 'postgres', { connectionString: 'postgresql://db.internal/analytics', user: 'analyst' }],
301+
['legacy_mongo', 'mongo', { uri: 'mongodb://mongo.internal:27017/events' }],
302+
] as const) {
303+
data.rows.push({
304+
id: `meta_${name}`,
305+
name,
306+
type: 'datasource',
307+
scope: 'platform',
308+
metadata: JSON.stringify({ name, driver, config, origin: 'runtime' }),
309+
state: 'active',
310+
version: 1,
311+
created_at: now,
312+
updated_at: now,
313+
});
314+
}
315+
316+
const b = await boot({ services: { data } });
317+
await b.plugin.start(b.ctx);
318+
// The list DTO is a summary; `getDatasource` (concrete service) is the
319+
// config-bearing read the admin routes serve.
320+
const svc = b.service as unknown as DatasourceAdminService;
321+
expect((await svc.getDatasource('legacy_sqlite'))?.config).toEqual({ filename: '/tmp/legacy.db' });
322+
expect((await svc.getDatasource('legacy_pg'))?.config).toEqual({
323+
url: 'postgresql://db.internal/analytics',
324+
username: 'analyst',
325+
});
326+
expect((await svc.getDatasource('legacy_mongo'))?.config).toEqual({
327+
url: 'mongodb://mongo.internal:27017/events',
328+
});
329+
});
330+
288331
it('removes the durable sys_metadata row when a datasource is deleted', async () => {
289332
const data = fakeSysMetadataEngine();
290333
const b = await boot({ services: { data } });

packages/services/service-datasource/src/__tests__/default-datasource-driver-factory.test.ts

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,3 +263,94 @@ describe('createDefaultDatasourceDriverFactory — declared keys reach the drive
263263
try { await handle.disconnect?.(); } catch { /* pool never opened */ }
264264
});
265265
});
266+
267+
// #4456 — the four undeclared read-side `??` fallbacks are DELETED. The factory
268+
// reads exactly the canonical key of each driver's config contract; the legacy
269+
// spellings a pre-#4410 stored record may carry are rewritten to canonical at
270+
// every rehydration seam by the ADR-0087 conversion
271+
// `datasource-config-driver-key-aliases`, so they must never reach this code —
272+
// and when one does anyway, it is ignored rather than quietly honoured.
273+
describe('createDefaultDatasourceDriverFactory — legacy config spellings are no longer read (#4456)', () => {
274+
function knexConfigOf(driver: any): any {
275+
return driver?.config ?? driver?.knexConfig ?? driver?.options ?? {};
276+
}
277+
278+
async function pgConnection(config: Record<string, unknown>): Promise<any> {
279+
const handle: any = await factory().create({ driver: 'postgres', config });
280+
try { return knexConfigOf(handle.driver ?? handle).connection; }
281+
finally { try { await handle.disconnect?.(); } catch { /* pool never opened */ } }
282+
}
283+
284+
it('pg: `connectionString` no longer selects the DSN path — discrete fields are used instead', async () => {
285+
const conn = await pgConnection({
286+
connectionString: 'postgresql://legacy@db.internal/analytics',
287+
host: 'db.internal',
288+
database: 'analytics',
289+
});
290+
expect(conn.connectionString).toBeUndefined();
291+
expect(conn).toMatchObject({ host: 'db.internal', database: 'analytics' });
292+
});
293+
294+
it('pg: `user` no longer reaches the client — only the canonical `username` does', async () => {
295+
const legacyOnly = await pgConnection({ host: 'h', database: 'd', user: 'legacy' });
296+
expect(legacyOnly.user).toBeUndefined();
297+
const both = await pgConnection({ host: 'h', database: 'd', user: 'legacy', username: 'svc' });
298+
expect(both.user).toBe('svc');
299+
});
300+
301+
it('mysql: `connectionString`/`user` are ignored the same way', async () => {
302+
const handle: any = await factory().create({
303+
driver: 'mysql',
304+
config: { connectionString: 'mysql://legacy@db/orders', host: 'db', database: 'orders', user: 'legacy' },
305+
});
306+
const conn = knexConfigOf(handle.driver ?? handle).connection;
307+
// Not the DSN string passthrough — the discrete-field object, with no user.
308+
expect(typeof conn).toBe('object');
309+
expect(conn).toMatchObject({ host: 'db', database: 'orders' });
310+
expect(conn.user).toBeUndefined();
311+
try { await handle.disconnect?.(); } catch { /* pool never opened */ }
312+
});
313+
314+
it('sqlite-wasm: `file`/`database` no longer name the database — the driver builds `:memory:`', async () => {
315+
const dir = mkdtempSync(join(tmpdir(), 'os-4456-'));
316+
try {
317+
const legacyFile = join(dir, 'legacy.db');
318+
const handle: any = await factory().create({
319+
driver: 'sqlite-wasm',
320+
config: { file: legacyFile, database: legacyFile },
321+
});
322+
const driver = handle.driver ?? handle;
323+
await driver.connect();
324+
try {
325+
await driver.syncSchema('note', { name: 'note', fields: { id: { type: 'text' } } });
326+
await driver.create('note', { id: 'n1' });
327+
} finally {
328+
try { await driver.disconnect(); } catch { /* noop */ }
329+
}
330+
// An ephemeral `:memory:` database writes nothing at the legacy path.
331+
expect(existsSync(legacyFile)).toBe(false);
332+
} finally {
333+
try { rmSync(dir, { recursive: true, force: true }); } catch { /* noop */ }
334+
}
335+
}, 30_000);
336+
337+
it('mongo: `uri`/`user` are ignored — the URL is composed from canonical keys only', async () => {
338+
const handle: any = await factory().create({
339+
driver: 'mongo',
340+
config: { uri: 'mongodb://legacy.internal:27017/legacy', database: 'events', user: 'legacy' },
341+
});
342+
const driver: any = handle.driver ?? handle;
343+
// No canonical `url`/`host`/`username` → composed from defaults + `database`,
344+
// with no auth part; the legacy `uri` never passes through.
345+
expect(driver.config.url).toBe('mongodb://localhost:27017/events');
346+
});
347+
348+
it('mongo: the canonical spellings still compose the URL (control)', async () => {
349+
const handle: any = await factory().create({
350+
driver: 'mongo',
351+
config: { host: 'mongo.internal', port: 27017, database: 'events', username: 'svc', password: 'pw' },
352+
});
353+
const driver: any = handle.driver ?? handle;
354+
expect(driver.config.url).toBe('mongodb://svc:pw@mongo.internal:27017/events');
355+
});
356+
});

packages/services/service-datasource/src/datasource-admin-plugin.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
22

33
import type { Plugin, PluginContext } from '@objectstack/core';
4+
import { applyConversionsToStoredItem } from '@objectstack/spec';
45
import { registerMetadataTypeActions } from '@objectstack/spec/kernel';
56
import type {
67
IDatasourceDriverFactory,
@@ -107,7 +108,15 @@ async function loadDatasourceRows(engine: DataEngineLike | undefined): Promise<A
107108
for (const r of rows ?? []) {
108109
const raw = (r as { metadata?: unknown }).metadata;
109110
try {
110-
out.push(typeof raw === 'string' ? JSON.parse(raw) : (raw as Record<string, unknown>));
111+
const parsed = typeof raw === 'string' ? JSON.parse(raw) : (raw as Record<string, unknown>);
112+
// This is a stored-row rehydration seam (ADR-0087 D2 addendum, #3903):
113+
// rows written under a past protocol replay the FULL conversion chain —
114+
// e.g. a pre-#4410 sqlite record whose config still says `file:` is
115+
// served with the canonical `filename`, which is the only spelling the
116+
// driver factory reads since #4456. The direct sys_metadata read here
117+
// bypasses the metadata service's own converting loaders, so the pass
118+
// must happen locally.
119+
out.push(applyConversionsToStoredItem(DS_META_TYPE, parsed));
111120
} catch {
112121
/* skip corrupt row */
113122
}

0 commit comments

Comments
 (0)