|
| 1 | +--- |
| 2 | +"@objectstack/driver-memory": major |
| 3 | +"@objectstack/spec": major |
| 4 | +"@objectstack/plugin-dev": patch |
| 5 | +--- |
| 6 | + |
| 7 | +fix(driver-memory,spec): persistence is opt-in again — `new InMemoryDriver()` is pure in-memory (#4065) |
| 8 | + |
| 9 | +`InMemoryDriverConfig.persistence` defaulted to `'auto'`, and in Node.js `'auto'` |
| 10 | +means **file**. So a bare `new InMemoryDriver()` — the shape every caller in this |
| 11 | +repo used — silently wrote `.objectstack/data/memory-driver.json` into the process |
| 12 | +CWD and reloaded it on the next boot. The default is now `false`. |
| 13 | + |
| 14 | +**This restores the accepted design rather than replacing it.** #815, the issue |
| 15 | +that introduced the persistence capability, specified it as opt-in in requirement |
| 16 | +\#1 — "默认情况下不启用持久化(纯内存,行为不变)" — and listed |
| 17 | +`new InMemoryDriver()` under "纯内存" in its own config examples. The `'auto'` |
| 18 | +default was a drift from that spec. |
| 19 | + |
| 20 | +What let the drift survive is worth naming, because it is not "there was no |
| 21 | +test". `MemoryConfigSchema` *did* pin the default, and asserted `'auto'`; the |
| 22 | +driver honoured `'auto'`; so spec and implementation agreed, and the pair looked |
| 23 | +verified. What nothing checked was whether the value they agreed on was the one |
| 24 | +#815 accepted. The driver's own `persistence.test.ts` could not have caught it |
| 25 | +either — every case there passes `persistence` explicitly, so the omitted-value |
| 26 | +path was untested on the implementation side. Both sides are now covered: three |
| 27 | +behavioural tests in `persistence.test.ts` (no CWD write, no cross-instance row |
| 28 | +carry-over, opt-in still persists) and the flipped schema assertion. |
| 29 | + |
| 30 | +**The symptom this fixes.** `packages/runtime/src/datasource-autoconnect.test.ts` |
| 31 | +seeds two rows with fixed ids and asserts the exact set. Run 1 passed and wrote |
| 32 | +the rows to disk; run 2 loaded them back, appended two more, and failed with four |
| 33 | +rows; run N had 2N. CI never saw it — every job is a fresh clone, so every CI run |
| 34 | +is run 1 — but `pnpm test` twice in one working tree could only ever go green |
| 35 | +once. The persisted file's `created_at` values, one pair per run, were the proof. |
| 36 | + |
| 37 | +(#4083 fixed that particular suite from the factory side, and its regression |
| 38 | +test is kept as-is. The blast radius was wider than one suite, though: **every** |
| 39 | +bare `new InMemoryDriver()` inherited the default, so any code path constructing |
| 40 | +one directly wrote to its working directory. Unit tests should not have write |
| 41 | +side effects on the CWD at all.) |
| 42 | + |
| 43 | +**Migrating.** Callers that want durability now ask for it: |
| 44 | + |
| 45 | +```ts |
| 46 | +new InMemoryDriver() // pure in-memory (new default) |
| 47 | +new InMemoryDriver({ persistence: 'file' }) // Node.js, durable across restarts |
| 48 | +new InMemoryDriver({ persistence: 'local' }) // browser, durable across reloads |
| 49 | +new InMemoryDriver({ persistence: 'auto' }) // previous default behaviour |
| 50 | +``` |
| 51 | + |
| 52 | +The `'auto'` / `'file'` / `'local'` / custom-adapter paths are unchanged; only |
| 53 | +the value used when `persistence` is omitted moved. |
| 54 | + |
| 55 | +**Relationship to #4083.** That issue fixed the same hazard one consumer at a |
| 56 | +time, and landed first: `createDefaultDatasourceDriverFactory` now passes |
| 57 | +`persistence: false` for a declared `{ driver: 'memory' }` datasource and scopes |
| 58 | +an opted-in destination *per datasource*, and the dev sqlite step-down's |
| 59 | +last-resort rung passes `false` too. Both are kept exactly as #4083 wrote them. |
| 60 | +This change closes the half they deliberately left open — a directly-constructed |
| 61 | +`new InMemoryDriver()` — which is the path that still wrote into the working |
| 62 | +directory of whatever process happened to build one. |
| 63 | + |
| 64 | +The two are complementary, not redundant. #4083's per-datasource scoping is |
| 65 | +still the only thing that expands `'auto'`/`'file'`/`'local'` into a destination |
| 66 | +carrying the datasource name, so two pools that DO opt in never alias one file; |
| 67 | +its explicit `false` becomes belt-and-braces, which is the right posture for a |
| 68 | +path that must never persist. |
| 69 | + |
| 70 | +`DevPlugin`'s driver is now explicitly `persistence: false`, matching the cache, |
| 71 | +queue, job, i18n, storage and search stubs it ships beside — it was the one piece |
| 72 | +of that stack that quietly outlived the process. |
| 73 | + |
| 74 | +**One claim trimmed, no behaviour attached.** The class docstring called this a |
| 75 | +"production-ready implementation of the ObjectStack Driver Protocol". It stores |
| 76 | +no constraints at all — `create()` is a `table.push()` and `syncSchema()` only |
| 77 | +allocates an array — so there is no primary key, uniqueness, `NOT NULL`, foreign |
| 78 | +key or column typing, and `bulkCreate` lands duplicate ids where a SQL driver |
| 79 | +raises a violation (the second finding in #4065). The docstring now says so, and |
| 80 | +points test authors at in-memory SQLite. Per Prime Directive #10 the fix for |
| 81 | +`declared ≠ enforced` is to implement it, trim the claim, or file it; with this |
| 82 | +driver moving to maintenance-only the claim is what goes. |
0 commit comments