Skip to content

Commit 974c6d4

Browse files
os-zhuangclaude
andauthored
fix(datasource): a memory datasource is ephemeral, and each pool gets its own store (#4083) (#4111)
The shared driver factory built `new InMemoryDriver()` for `driver: 'memory'` with no config, so the pool inherited that driver's own `persistence: 'auto'` default — in Node, a file adapter at the RELATIVE, process-global path `.objectstack/data/memory-driver.json`. It was therefore neither ephemeral (the store was flushed into the server's CWD and reloaded on the next boot) nor per-datasource (every memory pool in a process aliased the same file). Both surfaced as the ADR-0062 D1 federated-read acceptance reading 2 rows on a clean checkout and 2xN on the Nth run in the same tree — green in CI, which always runs #1 on a fresh checkout. The factory now builds the pool with `persistence: false`, honors the datasource's own `config` (previously dropped entirely), and scopes an opt-in persistence destination per datasource. The dev-only sqlite step-down's last-resort in-memory driver is built the same way. `InMemoryDriver`'s own documented defaults are unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent c20b875 commit 974c6d4

5 files changed

Lines changed: 294 additions & 6 deletions

File tree

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
---
2+
"@objectstack/service-datasource": patch
3+
---
4+
5+
fix(datasource): a `memory` datasource is ephemeral again, and each pool gets its own store (#4083)
6+
7+
The shared driver factory built `new InMemoryDriver()` for `driver: 'memory'` with
8+
no config, so the pool inherited that driver's own `persistence: 'auto'` default —
9+
in Node, a file adapter at the **relative, process-global** path
10+
`.objectstack/data/memory-driver.json`. Two consequences, neither intended:
11+
12+
- **It was not ephemeral.** The pool flushed its whole store into the server's
13+
working directory (on an unref'd 2s autosave timer, and again at teardown) and
14+
reloaded it on the next boot. That is the opposite of what the driver id
15+
promises the operator who asks for it — `OS_DATABASE_DRIVER=memory` is
16+
documented as *ephemeral, not real SQL* — and it means a "throwaway" datasource
17+
left state in the deploy directory.
18+
- **Every memory pool in a process shared one destination.** The default path
19+
carries no per-datasource component, so two `driver: 'memory'` datasources
20+
loaded and saved the same file: each saw the other's tables, and the last
21+
teardown to flush clobbered the other's rows.
22+
23+
Both were visible as an intermittent test failure. The ADR-0062 D1 federated-read
24+
acceptance seeds 2 rows into an auto-connected external memory datasource and
25+
reads them back; it returned 2 rows on a clean checkout and 2×N on the Nth run in
26+
the same tree — passing in CI (always run #1, always a fresh checkout) and
27+
failing locally for anyone who ran it twice. Whether a given run leaked depended
28+
on the autosave timer, which is what made it look flaky rather than wrong.
29+
30+
- The factory now builds the memory pool with **`persistence: false` by default**.
31+
- It also **honors the datasource's own `config`**, which was previously dropped
32+
entirely: `initialData` and `strictMode` never reached the driver.
33+
- When an author *does* opt into persistence (`config.persistence`), the default
34+
destination is **scoped to the datasource**
35+
`.objectstack/data/memory-<name>.json` / `objectstack:memory-db:<name>` — so
36+
pools stay independent. An explicit `path`/`key`, or a custom `adapter`, is
37+
left exactly as written.
38+
- The dev-only sqlite step-down's last-resort in-memory driver
39+
(`resolveSqliteDriver`, #2229) is built the same way, making its own
40+
"not persistent" contract true.
41+
42+
`InMemoryDriver`'s documented defaults are unchanged — constructing one directly
43+
still auto-detects persistence. Only the datasource-scoped pools this factory
44+
builds changed.
45+
46+
**Migration.** A deployment relying on `driver: 'memory'` state surviving a
47+
restart was relying on a bug, and should declare it: set
48+
`config: { persistence: 'file' }` on the datasource (now written to a
49+
per-datasource file), or use a real driver — `sqlite`/`sqlite-wasm` give durable
50+
storage with real SQL. Existing `.objectstack/data/memory-driver.json` files are
51+
no longer read; delete them.

packages/runtime/src/datasource-autoconnect.test.ts

Lines changed: 64 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
// without any native driver dependency.
1313

1414
import { describe, it, expect, beforeAll, afterAll, beforeEach, afterEach } from 'vitest';
15+
import { existsSync, rmSync } from 'node:fs';
16+
import { join } from 'node:path';
1517
import { Runtime } from './runtime.js';
1618
import { DriverPlugin } from './driver-plugin.js';
1719
import { AppPlugin } from './app-plugin.js';
@@ -71,7 +73,10 @@ async function boot(opts: { connectPolicy?: DatasourceConnectPolicy } = {}) {
7173

7274
const runtime = new Runtime({ cluster: false });
7375
const kernel = runtime.getKernel();
74-
await kernel.use(new DriverPlugin(new InMemoryDriver())); // default driver
76+
// `persistence: false` keeps the acceptance hermetic. The driver's own default
77+
// is `'auto'` — in Node a file adapter at `.objectstack/data/…` under the CWD,
78+
// which both reloads and rewrites ambient state between runs (#4083).
79+
await kernel.use(new DriverPlugin(new InMemoryDriver({ persistence: false }))); // default driver
7580
await kernel.use(new ObjectQLPlugin());
7681
await kernel.use(new AppPlugin(artifact()));
7782
await kernel.use(
@@ -131,6 +136,64 @@ describe('ADR-0062 declared-datasource auto-connect', () => {
131136
});
132137
});
133138

139+
// #4083 — the acceptance above passed on a clean checkout and failed on every
140+
// subsequent run, reading 2×N rows on the Nth: the auto-connected `memory`
141+
// datasource inherited `InMemoryDriver`'s `persistence: 'auto'` default, so it
142+
// flushed `ext_note` into `.objectstack/data/memory-driver.json` under the CWD
143+
// and the next boot's connect() loaded those rows back before this file seeded
144+
// its own. CI never caught it because CI always runs #1 on a fresh checkout.
145+
//
146+
// The intermittency ("passes once in four") came from WHEN the flush lands: the
147+
// file adapter writes on a 2s unref'd autosave timer, so a run short enough to
148+
// finish first left nothing behind. `flush()` below stands in for that timer, so
149+
// this pins the property that was actually broken — a federated in-memory pool
150+
// leaves nothing behind and does not outlive its kernel — without a timing race
151+
// and without depending on run-to-run state.
152+
describe('ADR-0062 D1 — the auto-connected in-memory pool leaves nothing behind (#4083)', () => {
153+
const STATE_DIR = join(process.cwd(), '.objectstack');
154+
const clearState = () => { try { rmSync(STATE_DIR, { recursive: true, force: true }); } catch { /* noop */ } };
155+
// Clear on both sides: a leftover from elsewhere would make this pass for the
156+
// wrong reason, and a failure that DID write must not leak into the next run
157+
// (that leak is the bug under test).
158+
beforeAll(clearState);
159+
afterAll(clearState);
160+
161+
async function seedAndRead(kernel: Awaited<ReturnType<typeof boot>>) {
162+
const engine = kernel.getService<{
163+
getDriverByName(n: string): any;
164+
find(object: string, query?: any): Promise<any[]>;
165+
}>('data');
166+
const driver = engine.getDriverByName('autoconn_ext');
167+
await driver.bulkCreate('ext_note', [
168+
{ id: 'n1', title: 'first' },
169+
{ id: 'n2', title: 'second' },
170+
]);
171+
const titles = (await engine.find('ext_note')).map((r) => r.title).sort();
172+
// Whatever the autosave timer would have written, written now.
173+
await driver.flush?.();
174+
return titles;
175+
}
176+
177+
it('writes no state file, and a second boot in the same process starts empty', async () => {
178+
const first = await boot();
179+
try {
180+
expect(await seedAndRead(first)).toEqual(['first', 'second']);
181+
// The seeded rows must not have reached the host filesystem at all.
182+
expect(existsSync(STATE_DIR)).toBe(false);
183+
} finally {
184+
try { await (first as any)?.stop?.(); } catch { /* noop */ }
185+
}
186+
187+
const second = await boot();
188+
try {
189+
// Was ['first','first','second','second'] — the first boot's rows, reloaded.
190+
expect(await seedAndRead(second)).toEqual(['first', 'second']);
191+
} finally {
192+
try { await (second as any)?.stop?.(); } catch { /* noop */ }
193+
}
194+
}, BOOT_TIMEOUT);
195+
});
196+
134197
describe('ADR-0062 credentials fail-closed (D3)', () => {
135198
// An external datasource that declares a credentialsRef the host cannot
136199
// resolve (no matching sys_secret row) must FAIL CLOSED — never connect with

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

Lines changed: 92 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
// kind. These are the first direct tests of the factory's id → driver mapping.
77

88
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
9-
import { mkdtempSync, rmSync } from 'node:fs';
9+
import { existsSync, mkdtempSync, rmSync } from 'node:fs';
1010
import { tmpdir } from 'node:os';
1111
import { join } from 'node:path';
1212
import { createDefaultDatasourceDriverFactory } from '../default-datasource-driver-factory.js';
@@ -76,3 +76,94 @@ describe('createDefaultDatasourceDriverFactory — mysql construction', () => {
7676
try { await handle.disconnect?.(); } catch { /* pool never opened */ }
7777
});
7878
});
79+
80+
// #4083 — the `memory` id built a bare `new InMemoryDriver()`, inheriting that
81+
// driver's own `persistence: 'auto'` default: in Node a file adapter at the
82+
// RELATIVE, process-global path `.objectstack/data/memory-driver.json`. So a
83+
// "memory" datasource flushed its store into the server's CWD at teardown and
84+
// reloaded it on the next boot, and every memory pool in the process aliased the
85+
// same file. That is what made the ADR-0062 D1 federated-read acceptance read 2
86+
// rows on a clean checkout and 2×N on the Nth run.
87+
describe('createDefaultDatasourceDriverFactory — memory construction (#4083)', () => {
88+
const STATE_DIR = join(process.cwd(), '.objectstack');
89+
90+
// Nothing here may write to the CWD. Asserting that directory's absence is the
91+
// point, so clear it on both sides: a leftover from another test file would
92+
// make this pass for the wrong reason, and a failure that DID create it must
93+
// not leak into the next run (that leak is the very bug under test).
94+
const clearState = () => { try { rmSync(STATE_DIR, { recursive: true, force: true }); } catch { /* noop */ } };
95+
beforeAll(clearState);
96+
afterAll(clearState);
97+
98+
async function pool(spec: { name?: string; config?: Record<string, unknown> } = {}) {
99+
const handle: any = await factory().create({
100+
driver: 'memory',
101+
config: spec.config ?? {},
102+
...(spec.name ? { name: spec.name } : {}),
103+
});
104+
const driver = handle.driver ?? handle;
105+
expect(driver?.constructor?.name).toMatch(/InMemoryDriver$/);
106+
await handle.connect?.();
107+
return { handle, driver };
108+
}
109+
110+
it('is ephemeral: a pool writes nothing to disk and a restart starts empty', async () => {
111+
const first = await pool({ name: 'ext' });
112+
await first.driver.bulkCreate('ext_note', [{ id: 'n1', title: 'first' }, { id: 'n2', title: 'second' }]);
113+
expect(await first.driver.find('ext_note', {})).toHaveLength(2);
114+
// Teardown is where the file adapter flushed. It must produce no file…
115+
await first.handle.disconnect?.();
116+
expect(existsSync(STATE_DIR)).toBe(false);
117+
118+
// …and the next boot of the SAME datasource must not inherit those rows.
119+
const second = await pool({ name: 'ext' });
120+
try {
121+
expect(await second.driver.find('ext_note', {})).toEqual([]);
122+
} finally {
123+
await second.handle.disconnect?.();
124+
}
125+
});
126+
127+
it('gives two memory datasources independent stores', async () => {
128+
const a = await pool({ name: 'ext_a' });
129+
const b = await pool({ name: 'ext_b' });
130+
try {
131+
await a.driver.create('ext_note', { id: 'a1', title: 'only-in-a' });
132+
expect(await a.driver.find('ext_note', {})).toHaveLength(1);
133+
expect(await b.driver.find('ext_note', {})).toEqual([]);
134+
} finally {
135+
await a.handle.disconnect?.();
136+
await b.handle.disconnect?.();
137+
}
138+
});
139+
140+
it("honors the datasource's own memory config (previously dropped entirely)", async () => {
141+
const { handle, driver } = await pool({
142+
name: 'seeded',
143+
config: { initialData: { ext_note: [{ id: 'seed', title: 'from-config' }] }, strictMode: true },
144+
});
145+
try {
146+
const rows = (await driver.find('ext_note', {})) as Array<{ title?: string }>;
147+
expect(rows.map((r) => r.title)).toEqual(['from-config']);
148+
} finally {
149+
await handle.disconnect?.();
150+
}
151+
});
152+
153+
it('scopes an OPT-IN persistence destination to the datasource, not the process', async () => {
154+
// The one white-box assertion here, and deliberately so: what needs pinning
155+
// is the destination handed to the driver, and the alternative (letting two
156+
// pools actually write) means writing into the repo checkout's CWD.
157+
const { handle, driver } = await pool({ name: 'warehouse', config: { persistence: 'file' } });
158+
const persistence = (driver as { config: { persistence: { type?: string; path?: string; key?: string } } }).config.persistence;
159+
expect(persistence.type).toBe('file');
160+
expect(persistence.path).toBe(join('.objectstack', 'data', 'memory-warehouse.json'));
161+
expect(persistence.key).toBe('objectstack:memory-db:warehouse');
162+
// Author-supplied destinations are theirs — never rewritten.
163+
const explicit = await pool({ name: 'warehouse', config: { persistence: { type: 'file', path: join(tmpdir(), 'os-4083-explicit.json') } } });
164+
expect((explicit.driver as { config: { persistence: { path?: string } } }).config.persistence.path)
165+
.toBe(join(tmpdir(), 'os-4083-explicit.json'));
166+
await handle.disconnect?.();
167+
await explicit.handle.disconnect?.();
168+
});
169+
});

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

Lines changed: 78 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@
1616
* - `sqlite-wasm` / `wasm-sqlite` → `@objectstack/driver-sqlite-wasm` (pure-JS)
1717
* - `mysql` / `mysql2` → `@objectstack/driver-sql` (client `mysql2`)
1818
* - `mongodb` / `mongo` → `@objectstack/driver-mongodb` (peer dep)
19-
* - `memory` / `inmemory` → `@objectstack/driver-memory`
19+
* - `memory` / `inmemory` → `@objectstack/driver-memory` (ephemeral,
20+
* per-datasource — see {@link buildMemoryConfig})
2021
*
2122
* `sqlite-wasm` joined for ADR-0062 D1 (#3826): the standalone stack's
2223
* `default` datasource is a *declared definition* connected through the shared
@@ -30,6 +31,7 @@
3031
* is never persisted or logged here.
3132
*/
3233

34+
import { join } from 'node:path';
3335
import type {
3436
IDatasourceDriverFactory,
3537
DatasourceConnectionSpec,
@@ -126,6 +128,78 @@ function buildMysqlConnection(spec: DatasourceConnectionSpec): unknown {
126128
};
127129
}
128130

131+
/**
132+
* Host-composition keys the CLI/standalone stack stamps into a `default`
133+
* datasource's `config` for the SQL builders (#3826). They are not memory-driver
134+
* config, so they are stripped before the rest of `config` is handed through.
135+
*/
136+
const NON_MEMORY_CONFIG_KEYS = ['schemaMode', 'autoMigrate', 'persist'] as const;
137+
138+
/** `.objectstack/data/memory-<datasource>.json` — one file per pool, never one for all. */
139+
function memoryStatePath(datasource: string): string {
140+
return join('.objectstack', 'data', `memory-${datasource}.json`);
141+
}
142+
143+
/** `objectstack:memory-db:<datasource>` — the localStorage equivalent of the above. */
144+
function memoryStateKey(datasource: string): string {
145+
return `objectstack:memory-db:${datasource}`;
146+
}
147+
148+
/**
149+
* Scope a REQUESTED persistence mode to one datasource.
150+
*
151+
* `InMemoryDriver`'s own persistence defaults are process-global — one file
152+
* (`.objectstack/data/memory-driver.json`), one localStorage key — so two
153+
* `driver: 'memory'` datasources in the same process load and save the SAME
154+
* store: each sees the other's tables, and the last teardown to flush clobbers
155+
* the other's rows. Expanding the string forms (`'auto'`/`'file'`/`'local'`) to
156+
* the object form is what lets the default path/key carry the datasource name.
157+
* An author-supplied `path`/`key` is theirs and is left alone, as is a custom
158+
* `adapter` — they chose the destination.
159+
*/
160+
function scopeMemoryPersistence(persistence: unknown, datasource: string): unknown {
161+
if (persistence === false) return false;
162+
if (typeof persistence === 'string') {
163+
return { type: persistence, path: memoryStatePath(datasource), key: memoryStateKey(datasource) };
164+
}
165+
if (persistence && typeof persistence === 'object' && !('adapter' in persistence)) {
166+
const p = persistence as { path?: string; key?: string };
167+
return {
168+
...p,
169+
...(p.path ? {} : { path: memoryStatePath(datasource) }),
170+
...(p.key ? {} : { key: memoryStateKey(datasource) }),
171+
};
172+
}
173+
return persistence;
174+
}
175+
176+
/**
177+
* Build the `InMemoryDriver` config for a `memory` datasource (#4083).
178+
*
179+
* Two things the bare `new InMemoryDriver()` this replaces got wrong:
180+
*
181+
* - **It was not ephemeral.** `InMemoryDriver`'s own `persistence` default is
182+
* `'auto'`, which in Node resolves to a file adapter at the *relative* path
183+
* `.objectstack/data/memory-driver.json`. Every memory datasource therefore
184+
* flushed its whole store into the server's CWD at teardown and reloaded it
185+
* on the next boot — the opposite of what this driver id promises the
186+
* operator who asks for it ("ephemeral, not real SQL", see
187+
* `cli/src/utils/storage-driver.ts`), and why the ADR-0062 D1 federated-read
188+
* acceptance read 2 rows on a clean checkout and 2×N on the Nth run (#4083).
189+
* - **Every pool shared one destination.** See {@link scopeMemoryPersistence}.
190+
*
191+
* So: default to no persistence, honor the datasource's own `config` (dropped on
192+
* the floor entirely before — `initialData`/`strictMode` never reached the
193+
* driver), and scope the destination per datasource when an author *does* ask
194+
* for persistence without naming a path/key of their own.
195+
*/
196+
function buildMemoryConfig(spec: DatasourceConnectionSpec): Record<string, unknown> {
197+
const cfg = { ...((spec.config ?? {}) as Record<string, unknown>) };
198+
for (const key of NON_MEMORY_CONFIG_KEYS) delete cfg[key];
199+
if (cfg.persistence === undefined) return { ...cfg, persistence: false };
200+
return { ...cfg, persistence: scopeMemoryPersistence(cfg.persistence, spec.name ?? 'default') };
201+
}
202+
129203
/** Build a mongodb connection URL from a spec's config + secret. */
130204
function buildMongoUrl(spec: DatasourceConnectionSpec): string {
131205
const cfg = (spec.config ?? {}) as Record<string, unknown>;
@@ -256,9 +330,10 @@ export function createDefaultDatasourceDriverFactory(
256330
return toHandle(driver);
257331
}
258332

259-
// memory
333+
// memory — ephemeral per datasource unless the author opts into
334+
// persistence, and then into a destination of its own (#4083).
260335
const { InMemoryDriver } = await import('@objectstack/driver-memory');
261-
return toHandle(new InMemoryDriver());
336+
return toHandle(new InMemoryDriver(buildMemoryConfig(spec)));
262337
},
263338
};
264339
}

packages/services/service-datasource/src/sqlite-driver-fallback.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,15 @@ export async function resolveSqliteDriver(
189189
}
190190

191191
// 3. In-memory (mingo) — dev-only last resort. Not real SQL, not persistent.
192+
// `persistence: false` is what makes that second half true: the driver's own
193+
// default is `'auto'`, which in Node flushes the whole store to
194+
// `.objectstack/data/memory-driver.json` in the CWD and reloads it next boot —
195+
// a shared file that every memory pool in the process would alias (#4083).
192196
const { InMemoryDriver } = await import('@objectstack/driver-memory');
193197
warn(NATIVE_SQLITE_MEMORY_FALLBACK_WARNING);
194-
return { driver: new InMemoryDriver(), engine: 'memory', label: 'InMemoryDriver' };
198+
return {
199+
driver: new InMemoryDriver({ persistence: false }),
200+
engine: 'memory',
201+
label: 'InMemoryDriver',
202+
};
195203
}

0 commit comments

Comments
 (0)