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
51 changes: 51 additions & 0 deletions .changeset/memory-datasource-ephemeral-per-pool.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
---
"@objectstack/service-datasource": patch
---

fix(datasource): a `memory` datasource is ephemeral again, and each pool gets its own store (#4083)

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`. Two consequences, neither intended:

- **It was not ephemeral.** The pool flushed its whole store into the server's
working directory (on an unref'd 2s autosave timer, and again at teardown) and
reloaded it on the next boot. That is the opposite of what the driver id
promises the operator who asks for it — `OS_DATABASE_DRIVER=memory` is
documented as *ephemeral, not real SQL* — and it means a "throwaway" datasource
left state in the deploy directory.
- **Every memory pool in a process shared one destination.** The default path
carries no per-datasource component, so two `driver: 'memory'` datasources
loaded and saved the same file: each saw the other's tables, and the last
teardown to flush clobbered the other's rows.

Both were visible as an intermittent test failure. The ADR-0062 D1 federated-read
acceptance seeds 2 rows into an auto-connected external memory datasource and
reads them back; it returned 2 rows on a clean checkout and 2×N on the Nth run in
the same tree — passing in CI (always run #1, always a fresh checkout) and
failing locally for anyone who ran it twice. Whether a given run leaked depended
on the autosave timer, which is what made it look flaky rather than wrong.

- The factory now builds the memory pool with **`persistence: false` by default**.
- It also **honors the datasource's own `config`**, which was previously dropped
entirely: `initialData` and `strictMode` never reached the driver.
- When an author *does* opt into persistence (`config.persistence`), the default
destination is **scoped to the datasource** —
`.objectstack/data/memory-<name>.json` / `objectstack:memory-db:<name>` — so
pools stay independent. An explicit `path`/`key`, or a custom `adapter`, is
left exactly as written.
- The dev-only sqlite step-down's last-resort in-memory driver
(`resolveSqliteDriver`, #2229) is built the same way, making its own
"not persistent" contract true.

`InMemoryDriver`'s documented defaults are unchanged — constructing one directly
still auto-detects persistence. Only the datasource-scoped pools this factory
builds changed.

**Migration.** A deployment relying on `driver: 'memory'` state surviving a
restart was relying on a bug, and should declare it: set
`config: { persistence: 'file' }` on the datasource (now written to a
per-datasource file), or use a real driver — `sqlite`/`sqlite-wasm` give durable
storage with real SQL. Existing `.objectstack/data/memory-driver.json` files are
no longer read; delete them.
65 changes: 64 additions & 1 deletion packages/runtime/src/datasource-autoconnect.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -12,6 +12,8 @@
// without any native driver dependency.

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

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

// #4083 — the acceptance above passed on a clean checkout and failed on every
// subsequent run, reading 2×N rows on the Nth: the auto-connected `memory`
// datasource inherited `InMemoryDriver`'s `persistence: 'auto'` default, so it
// flushed `ext_note` into `.objectstack/data/memory-driver.json` under the CWD
// and the next boot's connect() loaded those rows back before this file seeded
// its own. CI never caught it because CI always runs #1 on a fresh checkout.
//
// The intermittency ("passes once in four") came from WHEN the flush lands: the
// file adapter writes on a 2s unref'd autosave timer, so a run short enough to
// finish first left nothing behind. `flush()` below stands in for that timer, so
// this pins the property that was actually broken — a federated in-memory pool
// leaves nothing behind and does not outlive its kernel — without a timing race
// and without depending on run-to-run state.
describe('ADR-0062 D1 — the auto-connected in-memory pool leaves nothing behind (#4083)', () => {
const STATE_DIR = join(process.cwd(), '.objectstack');
const clearState = () => { try { rmSync(STATE_DIR, { recursive: true, force: true }); } catch { /* noop */ } };
// Clear on both sides: a leftover from elsewhere would make this pass for the
// wrong reason, and a failure that DID write must not leak into the next run
// (that leak is the bug under test).
beforeAll(clearState);
afterAll(clearState);

async function seedAndRead(kernel: Awaited<ReturnType<typeof boot>>) {
const engine = kernel.getService<{
getDriverByName(n: string): any;
find(object: string, query?: any): Promise<any[]>;
}>('data');
const driver = engine.getDriverByName('autoconn_ext');
await driver.bulkCreate('ext_note', [
{ id: 'n1', title: 'first' },
{ id: 'n2', title: 'second' },
]);
const titles = (await engine.find('ext_note')).map((r) => r.title).sort();
// Whatever the autosave timer would have written, written now.
await driver.flush?.();
return titles;
}

it('writes no state file, and a second boot in the same process starts empty', async () => {
const first = await boot();
try {
expect(await seedAndRead(first)).toEqual(['first', 'second']);
// The seeded rows must not have reached the host filesystem at all.
expect(existsSync(STATE_DIR)).toBe(false);
} finally {
try { await (first as any)?.stop?.(); } catch { /* noop */ }
}

const second = await boot();
try {
// Was ['first','first','second','second'] — the first boot's rows, reloaded.
expect(await seedAndRead(second)).toEqual(['first', 'second']);
} finally {
try { await (second as any)?.stop?.(); } catch { /* noop */ }
}
}, BOOT_TIMEOUT);
});

describe('ADR-0062 credentials fail-closed (D3)', () => {
// An external datasource that declares a credentialsRef the host cannot
// resolve (no matching sys_secret row) must FAIL CLOSED — never connect with
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -6,7 +6,7 @@
// kind. These are the first direct tests of the factory's id → driver mapping.

import { describe, it, expect, beforeAll, afterAll } from 'vitest';
import { mkdtempSync, rmSync } from 'node:fs';
import { existsSync, mkdtempSync, rmSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { createDefaultDatasourceDriverFactory } from '../default-datasource-driver-factory.js';
Expand DownExpand Up@@ -76,3 +76,94 @@ describe('createDefaultDatasourceDriverFactory — mysql construction', () => {
try { await handle.disconnect?.(); } catch { /* pool never opened */ }
});
});

// #4083 — the `memory` id built a bare `new InMemoryDriver()`, inheriting that
// driver's own `persistence: 'auto'` default: in Node a file adapter at the
// RELATIVE, process-global path `.objectstack/data/memory-driver.json`. So a
// "memory" datasource flushed its store into the server's CWD at teardown and
// reloaded it on the next boot, and every memory pool in the process aliased the
// same file. That is what made the ADR-0062 D1 federated-read acceptance read 2
// rows on a clean checkout and 2×N on the Nth run.
describe('createDefaultDatasourceDriverFactory — memory construction (#4083)', () => {
const STATE_DIR = join(process.cwd(), '.objectstack');

// Nothing here may write to the CWD. Asserting that directory's absence is the
// point, so clear it on both sides: a leftover from another test file would
// make this pass for the wrong reason, and a failure that DID create it must
// not leak into the next run (that leak is the very bug under test).
const clearState = () => { try { rmSync(STATE_DIR, { recursive: true, force: true }); } catch { /* noop */ } };
beforeAll(clearState);
afterAll(clearState);

async function pool(spec: { name?: string; config?: Record<string, unknown> } = {}) {
const handle: any = await factory().create({
driver: 'memory',
config: spec.config ?? {},
...(spec.name ? { name: spec.name } : {}),
});
const driver = handle.driver ?? handle;
expect(driver?.constructor?.name).toMatch(/InMemoryDriver$/);
await handle.connect?.();
return { handle, driver };
}

it('is ephemeral: a pool writes nothing to disk and a restart starts empty', async () => {
const first = await pool({ name: 'ext' });
await first.driver.bulkCreate('ext_note', [{ id: 'n1', title: 'first' }, { id: 'n2', title: 'second' }]);
expect(await first.driver.find('ext_note', {})).toHaveLength(2);
// Teardown is where the file adapter flushed. It must produce no file…
await first.handle.disconnect?.();
expect(existsSync(STATE_DIR)).toBe(false);

// …and the next boot of the SAME datasource must not inherit those rows.
const second = await pool({ name: 'ext' });
try {
expect(await second.driver.find('ext_note', {})).toEqual([]);
} finally {
await second.handle.disconnect?.();
}
});

it('gives two memory datasources independent stores', async () => {
const a = await pool({ name: 'ext_a' });
const b = await pool({ name: 'ext_b' });
try {
await a.driver.create('ext_note', { id: 'a1', title: 'only-in-a' });
expect(await a.driver.find('ext_note', {})).toHaveLength(1);
expect(await b.driver.find('ext_note', {})).toEqual([]);
} finally {
await a.handle.disconnect?.();
await b.handle.disconnect?.();
}
});

it("honors the datasource's own memory config (previously dropped entirely)", async () => {
const { handle, driver } = await pool({
name: 'seeded',
config: { initialData: { ext_note: [{ id: 'seed', title: 'from-config' }] }, strictMode: true },
});
try {
const rows = (await driver.find('ext_note', {})) as Array<{ title?: string }>;
expect(rows.map((r) => r.title)).toEqual(['from-config']);
} finally {
await handle.disconnect?.();
}
});

it('scopes an OPT-IN persistence destination to the datasource, not the process', async () => {
// The one white-box assertion here, and deliberately so: what needs pinning
// is the destination handed to the driver, and the alternative (letting two
// pools actually write) means writing into the repo checkout's CWD.
const { handle, driver } = await pool({ name: 'warehouse', config: { persistence: 'file' } });
const persistence = (driver as { config: { persistence: { type?: string; path?: string; key?: string } } }).config.persistence;
expect(persistence.type).toBe('file');
expect(persistence.path).toBe(join('.objectstack', 'data', 'memory-warehouse.json'));
expect(persistence.key).toBe('objectstack:memory-db:warehouse');
// Author-supplied destinations are theirs — never rewritten.
const explicit = await pool({ name: 'warehouse', config: { persistence: { type: 'file', path: join(tmpdir(), 'os-4083-explicit.json') } } });
expect((explicit.driver as { config: { persistence: { path?: string } } }).config.persistence.path)
.toBe(join(tmpdir(), 'os-4083-explicit.json'));
await handle.disconnect?.();
await explicit.handle.disconnect?.();
});
});
Original file line numberDiff line numberDiff line change
Expand Up@@ -16,7 +16,8 @@
* - `sqlite-wasm` / `wasm-sqlite` → `@objectstack/driver-sqlite-wasm` (pure-JS)
* - `mysql` / `mysql2` → `@objectstack/driver-sql` (client `mysql2`)
* - `mongodb` / `mongo` → `@objectstack/driver-mongodb` (peer dep)
* - `memory` / `inmemory` → `@objectstack/driver-memory`
* - `memory` / `inmemory` → `@objectstack/driver-memory` (ephemeral,
* per-datasource — see {@link buildMemoryConfig})
*
* `sqlite-wasm` joined for ADR-0062 D1 (#3826): the standalone stack's
* `default` datasource is a *declared definition* connected through the shared
Expand All@@ -30,6 +31,7 @@
* is never persisted or logged here.
*/

import { join } from 'node:path';
import type {
IDatasourceDriverFactory,
DatasourceConnectionSpec,
Expand DownExpand Up@@ -126,6 +128,78 @@ function buildMysqlConnection(spec: DatasourceConnectionSpec): unknown {
};
}

/**
* Host-composition keys the CLI/standalone stack stamps into a `default`
* datasource's `config` for the SQL builders (#3826). They are not memory-driver
* config, so they are stripped before the rest of `config` is handed through.
*/
const NON_MEMORY_CONFIG_KEYS = ['schemaMode', 'autoMigrate', 'persist'] as const;

/** `.objectstack/data/memory-<datasource>.json` — one file per pool, never one for all. */
function memoryStatePath(datasource: string): string {
return join('.objectstack', 'data', `memory-${datasource}.json`);
}

/** `objectstack:memory-db:<datasource>` — the localStorage equivalent of the above. */
function memoryStateKey(datasource: string): string {
return `objectstack:memory-db:${datasource}`;
}

/**
* Scope a REQUESTED persistence mode to one datasource.
*
* `InMemoryDriver`'s own persistence defaults are process-global — one file
* (`.objectstack/data/memory-driver.json`), one localStorage key — so two
* `driver: 'memory'` datasources in the same process load and save the SAME
* store: each sees the other's tables, and the last teardown to flush clobbers
* the other's rows. Expanding the string forms (`'auto'`/`'file'`/`'local'`) to
* the object form is what lets the default path/key carry the datasource name.
* An author-supplied `path`/`key` is theirs and is left alone, as is a custom
* `adapter` — they chose the destination.
*/
function scopeMemoryPersistence(persistence: unknown, datasource: string): unknown {
if (persistence === false) return false;
if (typeof persistence === 'string') {
return { type: persistence, path: memoryStatePath(datasource), key: memoryStateKey(datasource) };
}
if (persistence && typeof persistence === 'object' && !('adapter' in persistence)) {
const p = persistence as { path?: string; key?: string };
return {
...p,
...(p.path ? {} : { path: memoryStatePath(datasource) }),
...(p.key ? {} : { key: memoryStateKey(datasource) }),
};
}
return persistence;
}

/**
* Build the `InMemoryDriver` config for a `memory` datasource (#4083).
*
* Two things the bare `new InMemoryDriver()` this replaces got wrong:
*
* - **It was not ephemeral.** `InMemoryDriver`'s own `persistence` default is
* `'auto'`, which in Node resolves to a file adapter at the *relative* path
* `.objectstack/data/memory-driver.json`. Every memory datasource therefore
* flushed its whole store into the server's CWD at teardown and reloaded it
* on the next boot — the opposite of what this driver id promises the
* operator who asks for it ("ephemeral, not real SQL", see
* `cli/src/utils/storage-driver.ts`), and why the ADR-0062 D1 federated-read
* acceptance read 2 rows on a clean checkout and 2×N on the Nth run (#4083).
* - **Every pool shared one destination.** See {@link scopeMemoryPersistence}.
*
* So: default to no persistence, honor the datasource's own `config` (dropped on
* the floor entirely before — `initialData`/`strictMode` never reached the
* driver), and scope the destination per datasource when an author *does* ask
* for persistence without naming a path/key of their own.
*/
function buildMemoryConfig(spec: DatasourceConnectionSpec): Record<string, unknown> {
const cfg = { ...((spec.config ?? {}) as Record<string, unknown>) };
for (const key of NON_MEMORY_CONFIG_KEYS) delete cfg[key];
if (cfg.persistence === undefined) return { ...cfg, persistence: false };
return { ...cfg, persistence: scopeMemoryPersistence(cfg.persistence, spec.name ?? 'default') };
}

/** Build a mongodb connection URL from a spec's config + secret. */
function buildMongoUrl(spec: DatasourceConnectionSpec): string {
const cfg = (spec.config ?? {}) as Record<string, unknown>;
Expand DownExpand Up@@ -256,9 +330,10 @@ export function createDefaultDatasourceDriverFactory(
return toHandle(driver);
}

// memory
// memory — ephemeral per datasource unless the author opts into
// persistence, and then into a destination of its own (#4083).
const { InMemoryDriver } = await import('@objectstack/driver-memory');
return toHandle(new InMemoryDriver());
return toHandle(new InMemoryDriver(buildMemoryConfig(spec)));
},
};
}
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -189,7 +189,15 @@ export async function resolveSqliteDriver(
}

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