From 9a182c2c07a69dc66c82984f4774f8296d2b615d Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 29 Aug 2026 14:21:47 +0000 Subject: [PATCH] fix(driver-memory): enforce field-level `unique` so a colliding write is refused, not landed (#13197) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `InMemoryDriver` enforced no uniqueness at all: `create` was a `table.push()` and `syncSchema` allocated an array, so a `unique: true` field was declared-and-not-enforced and a colliding write LANDED, with a read returning both rows. The motivating instance is the worst-shaped one. `createWithAutonumberResync` re-seeds and re-issues a record number only when the STORE rejects it, so on a store that rejected nothing the branch was unreachable and an out-of-process autonumber duplicated a business identifier with no error anywhere. The remedy's location was already ruled in-tree at that method — uniqueness in the driver, never a pre-issue existence probe in the engine — and this is that remedy. - `memory-unique-constraint.ts` is the single judgment point: constraint derivation, the NULL-distinct bucket key, and the refusal. - The refusal carries the ADR-0112 envelope the SQL family answers a conflict with: `code: 'UNIQUE_VIOLATION'`, `status: 409`, no driver prefix. It is checked before the row is written, and `updateMany` checks the whole batch before mutating any of it. - Scoping is `driver-sql`'s `uniqueIndexesFromFields` (ADR-0120 D1/D3), reproduced arm for arm: `'global'` platform-wide; bare `true` and `'organization'` per-organization; both degrade to a single column with no tenant column; a `unique` on the tenant column itself stays single-column. NULL values stay NULL-DISTINCT. - `@objectstack/types`: `isUniqueViolationError` reads the platform's own `UNIQUE_VIOLATION` code. Load-bearing — an unrecognised refusal would leave the counter warm and turn a silent duplicate into a non-converging insert loop. - The `engine-autonumber-resync` pin that asserted the DEFECT is INVERTED in place, not deleted or re-baselined. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LZbWd2jNV1FErXTPSS4Dry --- .../driver-memory-field-level-uniqueness.md | 67 ++++ content/docs/data-modeling/drivers.mdx | 6 +- packages/drivers/driver-memory/src/index.ts | 14 + .../driver-memory/src/memory-driver.ts | 121 +++++- .../src/memory-unique-constraint.test.ts | 364 ++++++++++++++++++ .../src/memory-unique-constraint.ts | 244 ++++++++++++ .../src/engine-autonumber-resync.test.ts | 131 +++++-- packages/objectql/src/engine.ts | 48 ++- packages/types/src/unique-violation.test.ts | 46 +++ packages/types/src/unique-violation.ts | 24 +- 10 files changed, 986 insertions(+), 79 deletions(-) create mode 100644 .changeset/driver-memory-field-level-uniqueness.md create mode 100644 packages/drivers/driver-memory/src/memory-unique-constraint.test.ts create mode 100644 packages/drivers/driver-memory/src/memory-unique-constraint.ts diff --git a/.changeset/driver-memory-field-level-uniqueness.md b/.changeset/driver-memory-field-level-uniqueness.md new file mode 100644 index 0000000000..8c9a0df3e0 --- /dev/null +++ b/.changeset/driver-memory-field-level-uniqueness.md @@ -0,0 +1,67 @@ +--- +"@objectstack/driver-memory": minor +"@objectstack/types": patch +--- + +fix(driver-memory): enforce field-level `unique`, so a colliding write is refused instead of landing silently (#13197) + +`InMemoryDriver` enforced **no uniqueness at all**. `create` was a +`table.push()` and `syncSchema` allocated an array, so a `unique: true` field +was declared-and-not-enforced — the ADR-0078 / Prime-Directive-#10 shape the +platform refuses everywhere else. A colliding write did not fail; it landed, and +a read returned both rows. + +The motivating instance is the worst-shaped one. The engine's +`createWithAutonumberResync` re-seeds the counter and re-issues a record number +when the STORE rejects it as a duplicate, so on a store that rejected nothing +the whole branch was unreachable: an autonumber allocated out of process +duplicated an existing business identifier with **no error anywhere**. The +remedy's location was already ruled in-tree at that method — «uniqueness +enforcement in the driver, NOT a pre-issue existence probe here» — and this is +that remedy. Nothing in the new code knows what an autonumber is; the defect was +that the driver constrained nothing. + +**The refusal** carries the ADR-0112 envelope the SQL family answers a conflict +with: `code: 'UNIQUE_VIOLATION'`, `status: 409`, no `[driver-memory]` prefix. So +a suite that swaps this driver for SQLite sees one envelope — the parity +`memory-filter-refusal-envelope.test.ts` already states for the filter family, +now held for the constraint family. It is checked before the row is written, so +a refused write leaves the table exactly as it found it, and `updateMany` +prepares and checks the whole batch before mutating any of it. + +**The scoping is `driver-sql`'s, measured — not a simpler invention.** Read off +`uniqueIndexesFromFields` (ADR-0120 D1/D3) and reproduced arm for arm: +`unique: 'global'` is platform-wide; bare `true` and `'organization'` are +per-organization (bare `true` is the POSITIONAL spelling of `'organization'` at +FIELD level — reading it as `'global'` is the #4986 trap and would make two +organizations' identical values collide on a constraint neither can see); both +degrade to a single column when the object has no tenant column, and a `unique` +declaration on the tenant column itself stays single-column. NULL values stay +NULL-DISTINCT, exactly as under SQL `UNIQUE`. The D3 NULL-organization fold +needs no `'__global__'` token here — that sentinel is a SQL-expression artefact, +and a JavaScript key holds `null` directly. + +**Not** widened into: object-level declared `indexes[]` (composite uniques), +primary keys, or row-level tenant isolation. This driver still refuses to boot +multi-tenant (#6915) and that guard is untouched. + +`@objectstack/types` (`patch`): `isUniqueViolationError` now reads the +platform's own registered `UNIQUE_VIOLATION` code on the `code` channel. Not +cosmetic — a conflict that predicate does not recognise leaves the autonumber +resync unable to re-seed, so the counter stays warm and every following insert +collides too (#5495's PROBE3 storm), i.e. a silent duplicate traded for a +non-converging insert loop. It is a tautology rather than a widened heuristic +(the code already MEANS this condition), and no existing in-repo producer's +classification changes: `@objectstack/rest`'s own response body is the only +other site carrying that string, and it is downstream of the predicate. + +**Grade.** `minor` for the driver, not `patch`: a write that previously +succeeded is now refused (`409`), which is an accept-set narrowing under the +repo's launch-window convention for breaking changes, and the package also gains +public exports (`UNIQUE_VIOLATION_CODE`, `uniqueConstraintsFromFields`, +`tenantFieldOf`, `uniqueKeyOf`, `assertNoUniqueViolation`, +`uniqueViolationError`). `patch` for `@objectstack/types`: no API added or +removed and no in-repo verdict changes — the limb exists to serve the new +producer. Fixtures that relied on duplicates landing on a declared-unique field +must stop declaring `unique`, or stop writing the duplicate; the repo's own +suites were measured and none did. diff --git a/content/docs/data-modeling/drivers.mdx b/content/docs/data-modeling/drivers.mdx index 28d80c5e42..3e188aed8a 100644 --- a/content/docs/data-modeling/drivers.mdx +++ b/content/docs/data-modeling/drivers.mdx @@ -841,8 +841,10 @@ pass `persistence: 'auto'` (or `'file'`) explicitly. See For tests, prefer in-memory **SQLite** — `SqlDriver` with `connection: { filename: ':memory:' }`, or `SqliteWasmDriver({ filename: ':memory:' })` when you want no native build. Both give the SQL semantics production runs on; -mingo does not enforce primary keys, uniqueness, `NOT NULL` or column types, so a -green run against the memory driver is weaker evidence than it looks. The +the memory driver enforces **field-level `unique`** (with the same +per-organization scoping the SQL family applies) and nothing else — no primary +keys, no `NOT NULL`, no column types and no object-level composite `indexes[]` +— so a green run against it is still weaker evidence than it looks. The framework's own dogfood gate boots on WASM SQLite at `:memory:` for this reason. The memory driver remains fine where you want no setup at all and the assertions diff --git a/packages/drivers/driver-memory/src/index.ts b/packages/drivers/driver-memory/src/index.ts index d81d6a75e9..4bd7644000 100644 --- a/packages/drivers/driver-memory/src/index.ts +++ b/packages/drivers/driver-memory/src/index.ts @@ -22,6 +22,20 @@ export { } from './memory-tenancy-guard.js'; export type { TenancyAwareSchema } from './memory-tenancy-guard.js'; +// [#13197] Field-level uniqueness — the refusal's wire identity and the +// scoping helpers, exported so a consumer can assert the envelope (`code` AND +// `status`, never merely "it threw") without string-matching the message. +export { + UNIQUE_VIOLATION_CODE, + UNIQUE_VIOLATION_STATUS, + assertNoUniqueViolation, + tenantFieldOf, + uniqueConstraintsFromFields, + uniqueKeyOf, + uniqueViolationError, +} from './memory-unique-constraint.js'; +export type { MemoryUniqueConstraint, UniqueAwareSchema } from './memory-unique-constraint.js'; + export default { id: 'com.objectstack.driver.memory', version: '1.0.0', diff --git a/packages/drivers/driver-memory/src/memory-driver.ts b/packages/drivers/driver-memory/src/memory-driver.ts index 895637a5dd..65ae6cec73 100644 --- a/packages/drivers/driver-memory/src/memory-driver.ts +++ b/packages/drivers/driver-memory/src/memory-driver.ts @@ -36,6 +36,15 @@ import { indexTemporalFields, type TemporalFieldKind, } from './memory-temporal.js'; +// [#13197] Field-level uniqueness — the constraint this driver enforced +// NOWHERE, and the reason an out-of-process duplicate autonumber used to land +// silently. The scoping semantics are `driver-sql`'s, measured; see the +// module docblock. +import { + assertNoUniqueViolation, + uniqueConstraintsFromFields, + type MemoryUniqueConstraint, +} from './memory-unique-constraint.js'; /** * Persistence adapter interface. @@ -176,16 +185,28 @@ interface MemoryTransaction { * - Field projection and distinct values * - Strict mode and initial data loading * - * ## What this driver does NOT enforce + * ## What this driver enforces, and what it still does not + * + * Since #13197 it enforces **field-level `unique`**, with `driver-sql`'s + * ADR-0120 D1/D3 scoping (`memory-unique-constraint.ts` carries the measured + * arm-for-arm table): a colliding write is REFUSED — `code: 'UNIQUE_VIOLATION'`, + * `status: 409` — instead of landing silently. That closes the one gap whose + * failure mode was a wrong ANSWER rather than a missing check: an autonumber + * allocated out-of-process used to duplicate an existing business identifier + * with no error anywhere, because the engine's collision resync + * (`createWithAutonumberResync`) is triggered by the STORE rejecting the + * duplicate and this store rejected nothing. * - * It stores no constraints of any kind. {@link create} is a `table.push()` and - * {@link syncSchema} only allocates an array and indexes temporal fields, so - * there is no primary key, no uniqueness, no `NOT NULL`, no foreign key and no - * column typing. `bulkCreate` will happily land two rows with the same `id` - * where a SQL driver raises a constraint violation, and a read returns both. + * Everything else is still unenforced. {@link syncSchema} allocates an array, + * indexes temporal fields and records those unique constraints — and nothing + * more — so there is no primary key, no `NOT NULL`, no foreign key and no + * column typing. `bulkCreate` will still happily land two rows with the same + * `id` (unless `id` itself declares `unique`) where a SQL driver raises a + * constraint violation, and a read returns both. Object-level declared + * `indexes[]` — composite uniques — are not enforced either. * - * That makes it a WEAK oracle: code green against this driver can still be - * broken against the SQL engines production runs on. Prefer in-memory SQLite + * That still makes it a WEAK oracle: code green against this driver can still + * be broken against the SQL engines production runs on. Prefer in-memory SQLite * for tests — `SqlDriver` with `connection: { filename: ':memory:' }`, or * `SqliteWasmDriver({ filename: ':memory:' })` where no native build is wanted. * This driver's remaining roles are the last-resort rung of the dev step-down @@ -214,6 +235,15 @@ export class InMemoryDriver implements IDataDriver { * for it: the driver does not guess types from values. */ private temporalFields: Map> = new Map(); + + /** + * [#13197] Declared field-level unique constraints per object, populated by + * {@link syncSchema} — the same shape and the same lifetime as + * {@link temporalFields} above, and for the same reason: an object absent + * from this map was never declared, so nothing is enforced for it. This + * driver does not infer a constraint from the data it happens to hold. + */ + private uniqueConstraints: Map = new Map(); private transactions: Map = new Map(); private persistenceAdapter: PersistenceAdapterInterface | null = null; @@ -468,6 +498,12 @@ export class InMemoryDriver implements IDataDriver { updated_at: data.updated_at || new Date().toISOString(), }); + // [#13197] Refuse a declared-unique collision instead of landing it. Checked + // on the STORED form, so a temporal value is compared in the one shape this + // driver stores (#4047), and BEFORE the push, so a refused write leaves the + // table exactly as it found it. + this.assertUnique(object, newRecord); + table.push(newRecord); this.markDirty(); this.logger.debug('Record created', { object, id: newRecord.id, tableSize: table.length }); @@ -495,7 +531,11 @@ export class InMemoryDriver implements IDataDriver { created_at: table[index].created_at, // Preserve created_at updated_at: new Date().toISOString(), }); - + + // [#13197] The row being updated is excluded from its own check — an update + // that does not touch the unique field must not collide with itself. + this.assertUnique(object, updatedRecord, table[index].id); + table[index] = updatedRecord; this.markDirty(); this.logger.debug('Record updated', { object, id }); @@ -584,17 +624,26 @@ export class InMemoryDriver implements IDataDriver { const count = targetRecords.length; + // [#13197] Prepare and CHECK every row before mutating any of them: an + // `updateMany` that stamps the same unique value onto two rows collides + // by construction, and a half-applied batch is worse than a refusal. The + // pending rows are checked against each other too, which a per-row check + // against `table` alone would miss (nothing is written yet). + const pending: Array<{ index: number; row: Record }> = []; + const targetIds = new Set(targetRecords.map((r) => r.id)); + const settled = table.filter((r) => !targetIds.has(r.id)); for (const record of targetRecords) { const index = table.findIndex(r => r.id === record.id); - if (index !== -1) { - const updated = this.toStoredRecord(object, { - ...table[index], - ...data, - updated_at: new Date().toISOString() - }); - table[index] = updated; - } + if (index === -1) continue; + const updated = this.toStoredRecord(object, { + ...table[index], + ...data, + updated_at: new Date().toISOString() + }); + this.assertUnique(object, updated, table[index].id, [...settled, ...pending.map((p) => p.row)]); + pending.push({ index, row: updated }); } + for (const { index, row } of pending) table[index] = row; if (count > 0) this.markDirty(); this.logger.debug('UpdateMany completed', { object, count }); @@ -1439,6 +1488,15 @@ export class InMemoryDriver implements IDataDriver { // (ADR-0053 D-B3) and, like it, is idempotent. const kinds = indexTemporalFields(schema?.fields); this.temporalFields.set(object, kinds); + // [#13197] Learn the object's field-level unique constraints in the same + // pass. Deliberately NOT retroactive: rows already in the table arrived + // from `initialData` or a persistence adapter, before any schema existed, + // and REFUSING them here would turn a declaration into a boot failure over + // data this driver did not write. From here on every write is checked, and + // an already-duplicated pair is reported by the first write that touches + // it — the same posture `driver-sql` takes when a unique index cannot be + // built over dirty data (it announces, it does not delete rows). + this.uniqueConstraints.set(object, uniqueConstraintsFromFields(schema)); if (kinds.size > 0) { const table = this.db[object]; for (let i = 0; i < table.length; i++) { @@ -1452,6 +1510,10 @@ export class InMemoryDriver implements IDataDriver { if (this.db[object]) { const recordCount = this.db[object].length; delete this.db[object]; + // [#13197] The declaration dies with the table. A constraint left behind + // would be enforced over a table nobody declared — the inverse of the + // gap this closes, and just as invisible. + this.uniqueConstraints.delete(object); this.logger.info('Dropped in-memory table', { object, recordCount }); } } @@ -1624,6 +1686,31 @@ export class InMemoryDriver implements IDataDriver { return result; } + /** + * [#13197] Refuse `candidate` if it violates one of `object`'s declared + * field-level unique constraints. + * + * The ONE seam every write path goes through, so create, update and + * update-many cannot disagree about what `unique` means — the same + * single-judgment-point discipline `assertFilterConditionShape` enforces for + * the filter faces. `rows` defaults to the live table; `updateMany` passes a + * projected set instead, because its pending rows are not in the table yet. + * + * An object never passed through {@link syncSchema} has no entry and is + * unconstrained: this driver does not guess a constraint from the data, for + * the same reason it does not guess types from values. + */ + private assertUnique( + object: string, + candidate: Record, + exceptId?: unknown, + rows?: readonly Record[], + ): void { + const constraints = this.uniqueConstraints.get(object); + if (!constraints || constraints.length === 0) return; + assertNoUniqueViolation(object, rows ?? this.getTable(object), candidate, constraints, exceptId); + } + private getTable(name: string) { if (!this.db[name]) { this.db[name] = []; diff --git a/packages/drivers/driver-memory/src/memory-unique-constraint.test.ts b/packages/drivers/driver-memory/src/memory-unique-constraint.test.ts new file mode 100644 index 0000000000..57baca4279 --- /dev/null +++ b/packages/drivers/driver-memory/src/memory-unique-constraint.test.ts @@ -0,0 +1,364 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#13197] `driver-memory` enforces field-level `unique` — a colliding write is + * REFUSED, not landed. + * + * The card's motivating instance is an autonumber allocated out-of-process: + * the engine's `createWithAutonumberResync` re-seeds and re-issues only when + * the STORE rejects the duplicate, so on a store that rejected nothing a + * duplicate business identifier landed with no error anywhere. Nothing in this + * file knows what an autonumber is — the defect was that the driver enforced no + * uniqueness AT ALL, and that is what is pinned here. + * + * ## Two things every refusal test in this package must do + * + * 1. **Assert the ENVELOPE, never merely "it threw"** (#6144). A bare `Error` + * from an unrelated fault satisfies `toThrow()` and says nothing about the + * contract; `code` AND `status` are the contract (ADR-0112), and they are + * the SQL family's values so a suite that swaps this driver for SQLite sees + * one envelope. + * 2. **Assert the store is UNCHANGED.** "Refused" and "refused after writing + * the row" are different facts, and only the second one is the bug wearing + * an error message. + * + * ## The scoping arms are `driver-sql`'s, and are pinned as such + * + * ADR-0120 D1/D3, read off `uniqueIndexesFromFields`: `'global'` is + * platform-wide, bare `true` and `'organization'` are per-organization (bare + * `true` is the POSITIONAL spelling of `'organization'` at FIELD level, not of + * `'global'` — the #4986 trap), and both degrade to a single-column constraint + * when the object has no tenant column. This package cannot import `driver-sql` + * (dependency direction), so the arms are reproduced and pinned here against + * that rule rather than shared. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { isUniqueViolationError } from '@objectstack/types'; +import { InMemoryDriver } from './memory-driver.js'; +import { + UNIQUE_VIOLATION_CODE, + UNIQUE_VIOLATION_STATUS, + tenantFieldOf, + uniqueConstraintsFromFields, +} from './memory-unique-constraint.js'; + +interface WireBearingError extends Error { + code?: string; + status?: number; +} + +/** Run `fn`, requiring it to reject; hand back the rejection for inspection. */ +async function refusalOf(fn: () => Promise): Promise { + try { + await fn(); + } catch (e) { + return e as WireBearingError; + } + throw new Error('expected the driver to refuse this write, but it resolved'); +} + +/** The envelope assertion, in one place — `code` AND `status`, never just "it threw". */ +function expectUniqueViolationEnvelope(err: WireBearingError, field: string) { + expect(err.code).toBe(UNIQUE_VIOLATION_CODE); + expect(err.status).toBe(UNIQUE_VIOLATION_STATUS); + expect(err.code).toBe('UNIQUE_VIOLATION'); + expect(err.status).toBe(409); + expect(err.message).toContain(field); + // Same no-driver-prefix rule the filter refusals hold: the wire identity is + // the SQL family's, and a driver name in the sentence breaks that parity. + expect(err.message).not.toContain('[driver-memory]'); +} + +const DOC_SCHEMA = { + name: 'doc', + fields: { + id: { type: 'text', name: 'id' }, + title: { type: 'text', name: 'title' }, + doc_no: { type: 'autonumber', name: 'doc_no', unique: true }, + }, +}; + +describe('[#13197] the motivating instance: a duplicate autonumber is REFUSED, not landed', () => { + let driver: InMemoryDriver; + + beforeEach(async () => { + driver = new InMemoryDriver(); + await driver.syncSchema('doc', DOC_SCHEMA); + }); + + it('the second row carrying an already-stored record number is refused, and nothing is written', async () => { + await driver.create('doc', { id: '1', title: 'first', doc_no: 'D-0005' }); + + const err = await refusalOf(() => driver.create('doc', { id: '2', title: 'second', doc_no: 'D-0005' })); + + expectUniqueViolationEnvelope(err, 'doc_no'); + // The half that makes it a fix rather than a louder bug: ONE row, not two. + const rows = await driver.find('doc', { fields: ['id', 'doc_no'] }); + expect(rows).toHaveLength(1); + expect(rows[0].id).toBe('1'); + }); + + it('the refusal is recognisable to the ENGINE, so the autonumber resync converges', async () => { + // Load-bearing, not cosmetic. `ObjectQL.createWithAutonumberResync` drops + // the stale counter, re-seeds and re-issues ONLY when + // `isUniqueViolationError` says the rejection was a conflict. A refusal it + // does not recognise propagates with the counter still warm and the next + // insert collides too — #5495's PROBE3 storm, i.e. a silent duplicate + // traded for a non-converging insert loop. #13197 added the platform's own + // `UNIQUE_VIOLATION` code to that predicate's `codes` channel for exactly + // this edge; if this assertion goes red, the trade is no longer honest. + await driver.create('doc', { id: '1', doc_no: 'D-0005' }); + const err = await refusalOf(() => driver.create('doc', { id: '2', doc_no: 'D-0005' })); + + expect(isUniqueViolationError(err)).toBe(true); + }); + + it('a DIFFERENT record number still lands — the constraint is not a blanket refusal', async () => { + await driver.create('doc', { id: '1', doc_no: 'D-0005' }); + const written = await driver.create('doc', { id: '2', doc_no: 'D-0006' }); + expect(written.doc_no).toBe('D-0006'); + expect(await driver.count('doc')).toBe(2); + }); +}); + +describe('[#13197] every write path goes through the constraint, not just create', () => { + let driver: InMemoryDriver; + + beforeEach(async () => { + driver = new InMemoryDriver(); + await driver.syncSchema('doc', DOC_SCHEMA); + await driver.create('doc', { id: '1', doc_no: 'D-0001' }); + await driver.create('doc', { id: '2', doc_no: 'D-0002' }); + }); + + it('update onto a taken value is refused and the row keeps its old value', async () => { + const err = await refusalOf(() => driver.update('doc', '2', { doc_no: 'D-0001' })); + expectUniqueViolationEnvelope(err, 'doc_no'); + expect((await driver.findOne('doc', { fields: ['doc_no'], where: { id: '2' } }))!.doc_no).toBe('D-0002'); + }); + + it('a row does not collide with ITSELF — an update that leaves the unique field alone passes', async () => { + const updated = await driver.update('doc', '2', { title: 'renamed' }); + expect(updated!.doc_no).toBe('D-0002'); + // And re-writing the row's OWN value is not a collision either. + expect((await driver.update('doc', '2', { doc_no: 'D-0002' }))!.doc_no).toBe('D-0002'); + }); + + it('bulkCreate catches a duplicate WITHIN the batch, not only against stored rows', async () => { + const err = await refusalOf(() => + driver.bulkCreate('doc', [{ id: 'a', doc_no: 'D-0100' }, { id: 'b', doc_no: 'D-0100' }]), + ); + expectUniqueViolationEnvelope(err, 'doc_no'); + }); + + it('updateMany refuses BEFORE mutating anything — no half-applied batch', async () => { + // Stamping one value onto two rows collides by construction. The refusal + // has to leave both rows alone: a partially applied batch is the shape that + // makes a caller's retry unsafe. + const err = await refusalOf(() => driver.updateMany('doc', { where: {} }, { doc_no: 'D-0009' })); + expectUniqueViolationEnvelope(err, 'doc_no'); + + const rows = await driver.find('doc', { fields: ['id', 'doc_no'], orderBy: [{ field: 'id', order: 'asc' }] }); + expect(rows.map((r: any) => r.doc_no)).toEqual(['D-0001', 'D-0002']); + }); + + it('upsert on a conflict key UPDATES the existing row rather than colliding with it', async () => { + const out = await driver.upsert('doc', { doc_no: 'D-0001', title: 'upserted' }, ['doc_no']); + expect(out!.id).toBe('1'); + expect(await driver.count('doc')).toBe(2); + }); +}); + +describe('[#13197] what is NOT constrained — the boundaries, stated so they are not read as gaps', () => { + it('a field with no `unique` declaration takes duplicates as before', async () => { + const driver = new InMemoryDriver(); + await driver.syncSchema('note', { name: 'note', fields: { id: { type: 'text' }, body: { type: 'text' } } }); + await driver.create('note', { id: '1', body: 'same' }); + await driver.create('note', { id: '2', body: 'same' }); + expect(await driver.count('note')).toBe(2); + }); + + it('an object that never passed through syncSchema is unconstrained — the driver does not infer constraints from data', async () => { + const driver = new InMemoryDriver(); + await driver.create('undeclared', { id: '1', doc_no: 'D-1' }); + await driver.create('undeclared', { id: '2', doc_no: 'D-1' }); + expect(await driver.count('undeclared')).toBe(2); + }); + + it('NULL stays NULL-DISTINCT, exactly as under SQL UNIQUE', async () => { + // Folding empty values together would refuse the second row of every table + // with an optional unique column — a refusal driver-sql does not issue, so + // it would be a fresh divergence introduced by the fix for a divergence. + const driver = new InMemoryDriver(); + await driver.syncSchema('doc', DOC_SCHEMA); + await driver.create('doc', { id: '1', doc_no: null }); + await driver.create('doc', { id: '2', doc_no: null }); + await driver.create('doc', { id: '3' }); // absent, not null + expect(await driver.count('doc')).toBe(3); + }); + + it('dropTable forgets the declaration — a constraint must not outlive its table', async () => { + const driver = new InMemoryDriver(); + await driver.syncSchema('doc', DOC_SCHEMA); + await driver.dropTable('doc'); + await driver.create('doc', { id: '1', doc_no: 'D-1' }); + await driver.create('doc', { id: '2', doc_no: 'D-1' }); + expect(await driver.count('doc')).toBe(2); + }); + + it('rows already present when the schema arrives are not retroactively refused', async () => { + // They came from `initialData` or a persistence adapter, before any schema + // existed. Refusing them at syncSchema would turn a declaration into a boot + // failure over data this driver did not write. + const driver = new InMemoryDriver({ initialData: { doc: [{ id: '1', doc_no: 'D-1' }, { id: '2', doc_no: 'D-1' }] } }); + await driver.connect(); + await expect(driver.syncSchema('doc', DOC_SCHEMA)).resolves.toBeUndefined(); + expect(await driver.count('doc')).toBe(2); + // From here on, every WRITE is checked. + const err = await refusalOf(() => driver.create('doc', { id: '3', doc_no: 'D-1' })); + expectUniqueViolationEnvelope(err, 'doc_no'); + }); +}); + +/* ====================================================================== * + * The ADR-0120 D1/D3 scoping arms — measured against driver-sql's + * `uniqueIndexesFromFields`, arm for arm. + * ==================================================================== */ + +describe("[#13197] scoping matches driver-sql's ADR-0120 D1/D3 rule", () => { + const withOrg = (unique: unknown) => ({ + name: 'contact', + fields: { + id: { type: 'text' }, + organization_id: { type: 'text' }, + email: { type: 'text', unique }, + }, + }); + + it('`unique: true` on an object WITH a tenant column scopes per organization', async () => { + // Bare `true` is the positional spelling of `'organization'` at field level. + // Reading it as `'global'` makes two organizations' identical values + // collide on a constraint neither can see — ADR-0120 D1's whole point. + expect(uniqueConstraintsFromFields(withOrg(true))).toEqual([ + { field: 'email', scopeField: 'organization_id' }, + ]); + + const driver = new InMemoryDriver(); + await driver.syncSchema('contact', withOrg(true)); + await driver.create('contact', { id: '1', organization_id: 'acme', email: 'a@b.com' }); + await driver.create('contact', { id: '2', organization_id: 'globex', email: 'a@b.com' }); + expect(await driver.count('contact')).toBe(2); + + const err = await refusalOf(() => + driver.create('contact', { id: '3', organization_id: 'acme', email: 'a@b.com' }), + ); + expectUniqueViolationEnvelope(err, 'email'); + expect(err.message).toContain('organization_id'); + }); + + it("`unique: 'organization'` is the explicit synonym — same materialization", () => { + expect(uniqueConstraintsFromFields(withOrg('organization'))).toEqual([ + { field: 'email', scopeField: 'organization_id' }, + ]); + }); + + it("`unique: 'global'` is platform-wide even WITH a tenant column", async () => { + expect(uniqueConstraintsFromFields(withOrg('global'))).toEqual([ + { field: 'email', scopeField: null }, + ]); + + const driver = new InMemoryDriver(); + await driver.syncSchema('contact', withOrg('global')); + await driver.create('contact', { id: '1', organization_id: 'acme', email: 'a@b.com' }); + const err = await refusalOf(() => + driver.create('contact', { id: '2', organization_id: 'globex', email: 'a@b.com' }), + ); + expectUniqueViolationEnvelope(err, 'email'); + expect(err.message).not.toContain('organization_id'); + }); + + it('the NULL-organization rows form ONE bucket — the D3 fold, without needing the `__global__` token', async () => { + // SQL UNIQUE is NULL-distinct, so a raw `(organization_id, email)` composite + // enforced NOTHING on NULL-org rows — every row on a single-organization + // stack (#5030). ADR-0120 D3 folds them with COALESCE onto a reserved + // literal because an index EXPRESSION needs one; a JS key holds `null` + // directly, so the same bucket is reached with no token at all. + const driver = new InMemoryDriver(); + await driver.syncSchema('contact', withOrg(true)); + await driver.create('contact', { id: '1', email: 'a@b.com' }); + const err = await refusalOf(() => driver.create('contact', { id: '2', email: 'a@b.com' })); + expectUniqueViolationEnvelope(err, 'email'); + // …and a row that DOES carry an organization is untouched by that bucket. + await driver.create('contact', { id: '3', organization_id: 'acme', email: 'a@b.com' }); + expect(await driver.count('contact')).toBe(2); + }); + + it('with NO tenant column both per-organization spellings degrade to a single-column constraint', () => { + const noOrg = { name: 'doc', fields: { id: { type: 'text' }, code: { type: 'text', unique: true } } }; + expect(uniqueConstraintsFromFields(noOrg)).toEqual([{ field: 'code', scopeField: null }]); + }); + + it('a unique declaration ON the tenant column itself stays single-column', () => { + // `(organization_id, organization_id)` is not a constraint. + const oneRowPerOrg = { + name: 'settings', + fields: { id: { type: 'text' }, organization_id: { type: 'text', unique: true } }, + }; + expect(uniqueConstraintsFromFields(oneRowPerOrg)).toEqual([ + { field: 'organization_id', scopeField: null }, + ]); + }); + + it('`unique: false` / absent declares no constraint at all', () => { + expect(uniqueConstraintsFromFields(withOrg(false))).toEqual([]); + expect(uniqueConstraintsFromFields(withOrg(undefined))).toEqual([]); + }); +}); + +describe('[#13197] tenantFieldOf mirrors SqlDriver.computeTenantField arm for arm', () => { + it('an explicit opt-out wins over any column-presence heuristic', () => { + expect( + tenantFieldOf({ fields: { organization_id: {} }, tenancy: { enabled: false } }), + ).toBeNull(); + }); + + it('a declared tenantField that exists on the object is used', () => { + expect( + tenantFieldOf({ fields: { org: {}, organization_id: {} }, tenancy: { tenantField: 'org' } }), + ).toBe('org'); + }); + + it('a declared tenantField that does NOT exist falls through to the implicit column', () => { + expect( + tenantFieldOf({ fields: { organization_id: {} }, tenancy: { tenantField: 'missing' } }), + ).toBe('organization_id'); + }); + + it('the implicit `organization_id` column is detected with no tenancy block at all', () => { + expect(tenantFieldOf({ fields: { organization_id: {} } })).toBe('organization_id'); + }); + + it('no candidate column answers null', () => { + expect(tenantFieldOf({ fields: { id: {} } })).toBeNull(); + expect(tenantFieldOf(undefined)).toBeNull(); + }); +}); + +describe('[#13197] value identity', () => { + it('distinguishes a number from its string spelling, as `upsert` already does', async () => { + // A SQL column has one type so the question cannot arise there; here it can, + // and folding them would refuse a write SQL accepts. + const driver = new InMemoryDriver(); + await driver.syncSchema('doc', { + name: 'doc', + fields: { id: { type: 'text' }, code: { type: 'text', unique: 'global' } }, + }); + await driver.create('doc', { id: '1', code: 5 }); + await driver.create('doc', { id: '2', code: '5' }); + expect(await driver.count('doc')).toBe(2); + + const err = await refusalOf(() => driver.create('doc', { id: '3', code: 5 })); + expectUniqueViolationEnvelope(err, 'code'); + }); +}); diff --git a/packages/drivers/driver-memory/src/memory-unique-constraint.ts b/packages/drivers/driver-memory/src/memory-unique-constraint.ts new file mode 100644 index 0000000000..123b785674 --- /dev/null +++ b/packages/drivers/driver-memory/src/memory-unique-constraint.ts @@ -0,0 +1,244 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Field-level uniqueness for the in-memory driver — the constraint this driver + * enforced NOWHERE until now (#13197). + * + * ## The defect this closes + * + * `InMemoryDriver.create` was a `table.push()` and `syncSchema` allocated an + * array, so a `unique: true` field was **declared and not enforced** — the + * ADR-0078 / Prime-Directive-#10 shape the platform refuses everywhere else. A + * colliding write did not fail; it LANDED, and a read returned both rows. + * + * The motivating instance is the autonumber one, and it is the worst-shaped: + * `packages/objectql/src/engine.ts`'s `createWithAutonumberResync` re-seeds and + * re-issues a record number when the store REJECTS it as a duplicate, so on a + * driver that rejects nothing the whole branch is unreachable and a duplicate + * business identifier lands with no error anywhere. That comment already ruled + * where the remedy belongs — «uniqueness enforcement in the driver, NOT a + * pre-issue existence probe here» — and this module is that remedy. The + * autonumber case is a consequence, not a special case: nothing here knows what + * an autonumber is. + * + * ## The semantics are driver-sql's, MEASURED — not a simpler invention + * + * A second, easier answer to "what does `unique` mean" is the + * one-contract-two-numbers defect this repo keeps closing, so the scoping rule + * below is read off `driver-sql`'s `uniqueIndexesFromFields` + * (`packages/drivers/driver-sql/src/schema-drift.ts`, ADR-0120 D1/D3) and + * reproduced, arm for arm: + * + * | declaration | tenant column | driver-sql index | here | + * |:---|:---|:---|:---| + * | `unique: 'global'` | any | `(field)` | scope `null` | + * | `unique: true` / `'organization'` | present, ≠ field | `(COALESCE(tenant,'__global__'), field)` | scope = tenant column | + * | `unique: true` / `'organization'` | absent | `(field)` | scope `null` | + * | `unique: true` / `'organization'` | IS the field | `(field)` | scope `null` | + * | absent / `false` | any | no index | not a constraint | + * + * ⚠️ **`unique: true` is the POSITIONAL spelling of `'organization'`, not of + * `'global'`** — at FIELD level. (On a declared `indexes[]` entry bare `true` + * means `'global'`; that surface is not this module's, see "Deliberately out of + * scope".) Getting that backwards makes two organizations' identical record + * numbers collide on a constraint neither can see, which is the exact + * cross-tenant existence oracle ADR-0120 D1 exists to remove. + * + * ### Why the NULL-organization fold needs no `'__global__'` token here + * + * ADR-0120 D3 materializes the organization key part as + * `COALESCE(organization_id, '__global__')` because SQL `UNIQUE` is + * NULL-DISTINCT: a raw `(organization_id, field)` composite enforces NOTHING on + * rows whose organization is NULL, which on a single-organization stack is + * every row (#5030). The sentinel is a SQL-EXPRESSION artefact — an index + * expression needs a non-NULL literal to fold onto — and its value is reserved + * precisely so no real organization can land in the bucket. + * + * A JavaScript key can hold `null` directly, so {@link uniqueKeyOf} folds every + * NULL-organization row onto the key part `null` and needs no token at all. + * That is the same bucket, reached without copying a constant across a package + * boundary this driver must not depend on (`driver-memory` is the last rung of + * the dev step-down; it does not pull in `driver-sql`). It is also marginally + * STRICTER in one unreachable case: a row whose organization id literally + * equals `'__global__'` would share SQL's platform bucket and gets its own + * here — a case the reserved-token guard at the organization-creation seam + * makes unconstructible. + * + * ### NULL values stay NULL-DISTINCT, deliberately + * + * A row whose unique FIELD is `null`/absent is exempt, exactly as under SQL + * `UNIQUE`. Folding those together instead would refuse the second row of every + * table with an optional unique column — a refusal `driver-sql` does not issue, + * i.e. a fresh divergence introduced by the fix for a divergence. + * + * ## Deliberately out of scope (#13197's dispatch, and stated so it is not read as done) + * + * - **Declared `indexes[]`** — object-level composite uniques + * (`normalizeDeclaredIndex`) are NOT enforced here. Same defect class, wider + * surface, its own card. + * - **Primary keys.** A duplicate `id` still lands unless `id` itself declares + * `unique`. The driver docstring says so. + * - **Row-level tenant isolation.** This scopes a uniqueness KEY the way + * ADR-0120 does; it does not make reads tenant-filtered. This driver still + * refuses to boot multi-tenant (`memory-tenancy-guard.ts`, #6915) and that + * guard is untouched — which is also why the scope arm above is, in + * practice, reached only through an object carrying an `organization_id` + * column WITHOUT an explicit `tenancy` block. + */ + +import { isGlobalUnique, isUniqueDeclared, isTenancyDisabled } from '@objectstack/spec/data'; + +/** + * The wire identity of the refusal (ADR-0112). `UNIQUE_VIOLATION` is the + * registered code `@objectstack/rest` already answers a SQL conflict with + * (`error-code-ledger.zod.ts`), and 409 is the status it answers it at, so a + * suite that swaps this driver for SQLite sees ONE envelope — the parity + * `memory-filter-refusal-envelope.test.ts` states for the filter family, held + * here for the constraint family. + * + * ⛔ Never assert merely "it threw" against this (#6144): a bare `Error` from + * an unrelated fault passes that assertion and says nothing about the contract. + * Assert `code` AND `status`. + */ +export const UNIQUE_VIOLATION_CODE = 'UNIQUE_VIOLATION'; + +/** @see UNIQUE_VIOLATION_CODE */ +export const UNIQUE_VIOLATION_STATUS = 409; + +/** One field-level unique constraint, resolved against the object's tenancy. */ +export interface MemoryUniqueConstraint { + /** The field the constraint is on. */ + readonly field: string; + /** + * The organization key part, or `null` for a platform-wide constraint. + * ADR-0120 D1: `'global'` and "no tenant column" both answer `null`. + */ + readonly scopeField: string | null; +} + +/** The minimal schema shape this module reads. */ +export interface UniqueAwareSchema { + fields?: Record | null; + tenancy?: { enabled?: boolean; tenantField?: string } | null; +} + +/** + * The object's tenant column, or `null`. + * + * Mirrors `SqlDriver.computeTenantField` arm for arm — explicit opt-out wins, + * then a declared `tenancy.tenantField` that actually exists on the object, + * then the implicit `organization_id` column the kernel injects. Reproduced + * rather than imported because this package must not depend on `driver-sql`; + * the two are held together by `memory-unique-constraint.test.ts`, which pins + * each arm against the rule quoted above. + */ +export function tenantFieldOf(schema: UniqueAwareSchema | null | undefined): string | null { + if (isTenancyDisabled(schema)) return null; + const fields = schema?.fields; + const declared = schema?.tenancy?.tenantField; + if (typeof declared === 'string' && declared.length > 0) { + if (fields && Object.prototype.hasOwnProperty.call(fields, declared)) return declared; + } + if (fields && Object.prototype.hasOwnProperty.call(fields, 'organization_id')) return 'organization_id'; + return null; +} + +/** + * The constraints an object's field-level `unique` declarations ask for. + * + * The single place a `unique` declaration becomes a constraint in this package, + * so the create, update and update-many paths cannot disagree about what one + * means — the same reason `uniqueIndexesFromFields` is the single place on the + * SQL side. + */ +export function uniqueConstraintsFromFields( + schema: UniqueAwareSchema | null | undefined, +): MemoryUniqueConstraint[] { + const fields = schema?.fields; + if (!fields) return []; + const tenantField = tenantFieldOf(schema); + const out: MemoryUniqueConstraint[] = []; + for (const [name, field] of Object.entries(fields)) { + const unique = (field as { unique?: unknown } | null | undefined)?.unique; + if (!isUniqueDeclared(unique)) continue; + // `'global'` opts out of organization scoping; a unique declaration ON the + // tenant column itself cannot be scoped by it (`(org_id, org_id)` is not a + // constraint) and stays single-column, exactly as on the SQL side. + const scoped = !isGlobalUnique(unique) && tenantField != null && tenantField !== name; + out.push({ field: name, scopeField: scoped ? tenantField : null }); + } + return out; +} + +/** + * The bucket key a record occupies under one constraint, or `null` when the + * record is EXEMPT because its unique field carries no value (SQL `UNIQUE` is + * NULL-distinct — see the module note). + * + * The key is a canonical JSON encoding, so `5` and `'5'` are different values, + * matching both SQL's one-type-per-column reality and this driver's own + * `upsert` conflict-key comparison (`r[key] === data[key]`). It is computed on + * the STORED form of the record, after temporal coercion, so a filter and a + * constraint cannot disagree about what a datetime is (#4047). + */ +export function uniqueKeyOf( + record: Record, + constraint: MemoryUniqueConstraint, +): string | null { + const value = record[constraint.field]; + if (value === null || value === undefined) return null; + const scope = constraint.scopeField === null ? null : (record[constraint.scopeField] ?? null); + return JSON.stringify([scope, value]); +} + +/** + * The refusal, in the ADR-0112 envelope. + * + * No `[driver-memory]` prefix: the wire identity has to be the SQL family's, + * and a driver name in the sentence is the leak `memory-filter-refusal-envelope` + * pins the absence of for the filter family. + */ +export function uniqueViolationError( + object: string, + constraint: MemoryUniqueConstraint, + value: unknown, +): Error & { code: string; status: number } { + const scoped = constraint.scopeField ? ` within the same \`${constraint.scopeField}\`` : ''; + const err = new Error( + `Unique constraint violated on \`${object}.${constraint.field}\`: a record with the value ` + + `${JSON.stringify(value ?? null)} already exists${scoped}. No record was written.`, + ) as Error & { code: string; status: number }; + err.code = UNIQUE_VIOLATION_CODE; + err.status = UNIQUE_VIOLATION_STATUS; + return err; +} + +/** + * Refuse `candidate` if it collides with any row in `rows` under any of + * `constraints`. `exceptId` excludes the row being updated from its own check. + * + * A linear scan per constraint, deliberately: this driver's whole shape is + * "plain arrays, no indexes", and an incremental index would be a second copy + * of the table to keep in step with `create`/`update`/`updateMany`/`delete`/ + * transaction rollback — five seams for a store whose documented role is dev, + * demo and in-process fixtures. + */ +export function assertNoUniqueViolation( + object: string, + rows: readonly Record[], + candidate: Record, + constraints: readonly MemoryUniqueConstraint[], + exceptId?: unknown, +): void { + if (constraints.length === 0) return; + for (const constraint of constraints) { + const key = uniqueKeyOf(candidate, constraint); + if (key === null) continue; + for (const row of rows) { + if (exceptId !== undefined && row.id === exceptId) continue; + if (uniqueKeyOf(row, constraint) === key) { + throw uniqueViolationError(object, constraint, candidate[constraint.field]); + } + } + } +} diff --git a/packages/objectql/src/engine-autonumber-resync.test.ts b/packages/objectql/src/engine-autonumber-resync.test.ts index df33bea829..1a2e0671b3 100644 --- a/packages/objectql/src/engine-autonumber-resync.test.ts +++ b/packages/objectql/src/engine-autonumber-resync.test.ts @@ -46,21 +46,30 @@ * driver-memory (`supports = {}`) and driver-mongodb (absent) take this path; * driver-sql declares `autonumber: true`, and driver-sqlite-wasm and * driver-turso inherit it (`extends SqlDriver`; Turso spreads - * `...super.supports`), so none of the three ever reaches here. Of the two that - * do, only driver-mongodb can raise anything — a single-field unique index, - * when the field declares `unique`. **driver-memory never can**: `create` is a - * `table.push()` storing no constraints at all (#4065), so a duplicate lands - * SILENTLY and this branch is unreachable. + * `...super.supports`), so none of the three ever reaches here. * - * That is the repo's EXISTING ruled reading, not a fresh claim by this file: + * ⚠️ **Both of the two that do can now raise.** driver-mongodb raises a + * single-field unique index's `E11000` when the field declares `unique`; since + * #13197 driver-memory refuses a declared-unique collision too, in the ADR-0112 + * envelope (`code: 'UNIQUE_VIOLATION'`, `status: 409`), with `driver-sql`'s + * ADR-0120 D1/D3 scoping. Until #13197 it never could — `create` was a + * `table.push()` storing no constraints at all (#4065) — so a duplicate landed + * SILENTLY and this branch was unreachable there. Section (3b) carries the + * pin, INVERTED in place rather than deleted, so the change of fact stays + * readable from `main`. + * + * The ownership reading behind (3b) is unchanged and is the repo's EXISTING + * ruled one, not a fresh claim by this file: * `scripts/driver-memory-census.ledger.json` records it for * `packages/runtime/src/autonumber-seed-cross-side-parity.integration.test.ts` * as `ruled-permanent` («#6664 A, maintainer 2026-08-08 — inherits #5704 * Q2 = B») — "InMemoryDriver declares `supports = {}`, so the ENGINE's - * autonumber seeding owns the counter. No SQL backend can stand in". Section - * (3b) pins the consequence rather than authoring a second answer to the same - * question (#6832's one-contract-two-numbers shape). What covers driver-memory - * is adoption, which waits for no rejection. + * autonumber seeding owns the counter. No SQL backend can stand in". What + * #13197 moved is which store can REJECT, never who issues the number, so that + * ruling is untouched. Section (3b) pins the consequence rather than authoring + * a second answer to the same question (#6832's one-contract-two-numbers + * shape). Adoption still covers the drift no store can report, and still waits + * for no rejection. * * NOTE this file imports no driver package — the rig below is a hand-rolled * fake driver — so it adds no `driver-memory` consumer and @@ -149,10 +158,10 @@ function matches(row: Row, where: any): boolean { * signature limb) and the message names the INDEX, never the column — so * `isUniqueViolationError` says yes on the `duplicate key` limb while * `uniqueViolationColumn` answers `undefined`. That combination is exactly why - * the resync treats an unnamed column as attributable: MongoDB is the ONE - * in-repo fallback driver that can raise a collision at all (driver-memory has - * no uniqueness constraints), and demanding a named column would make the - * resync unreachable on it. + * the resync treats an unnamed column as attributable: neither in-repo fallback + * driver names a column this predicate can read — MongoDB names the INDEX, and + * driver-memory (since #13197) raises a coded envelope with no dialect prose in + * it — so demanding a named column would make the resync unreachable on both. */ const mongoDuplicate = (field: string, value: string) => Object.assign( @@ -162,6 +171,26 @@ const mongoDuplicate = (field: string, value: string) => { code: 11000 }, ); +/** + * [#13197] `driver-memory`'s duplicate refusal — the ADR-0112 envelope, not a + * dialect. It carries `code: 'UNIQUE_VIOLATION'` (the platform's own registered + * code) and `status: 409`, and names no column in any spelling + * `uniqueViolationColumn` parses. So, exactly like the MongoDB shape above, + * `isUniqueViolationError` says yes while `uniqueViolationColumn` answers + * `undefined` — which is why the resync's unnamed-column attribution reaches + * this driver too. Reproduced here rather than imported: this file imports no + * driver package (see the header note), and the fields asserted on are the + * contract `driver-memory`'s own suite pins. + */ +const memoryDuplicate = (field: string, value: string) => + Object.assign( + new Error( + `Unique constraint violated on \`doc.${field}\`: a record with the value ` + + `${JSON.stringify(value)} already exists. No record was written.`, + ), + { code: 'UNIQUE_VIOLATION', status: 409 }, + ); + /** Postgres names the conflicting COLUMN in its DETAIL line — `uniqueViolationColumn` reads it. */ const postgresDuplicate = (column: string, value: string) => Object.assign(new Error(`duplicate key value violates unique constraint "doc_${column}_key"`), { @@ -650,26 +679,53 @@ describe('ObjectQL autonumber resync (#6806)', () => { /* ====================================================================== * * (3b) The collision half is STORAGE-DEPENDENT — name which driver gives - * which guarantee, rather than implying one that is not delivered + * which guarantee, rather than implying one that is not delivered. + * #13197 changed the ANSWER for driver-memory (it constrains now); the + * question, and the duty to answer it by driver name, are unchanged. * ==================================================================== */ - describe('what a driver with no uniqueness constraint gets (driver-memory)', () => { + describe('what driver-memory gets, now that it DOES constrain (#13197)', () => { const SCHEMA = schemaWith('doc_no', 'D-{0000}'); - it('a duplicate lands SILENTLY — the collision branch cannot be reached', async () => { - // `InMemoryDriver.create` is a `table.push()` storing no constraints of - // any kind (its own docstring since #4065 — it calls itself a WEAK - // oracle). So a duplicate raises nothing, there is no error to catch, and - // the re-issue this file pins elsewhere never runs. Recorded rather than - // papered over (PD #10): "collisions are handled" would be FALSE on one - // of the two drivers this fallback path serves — and the fallback path is - // only those two of five, the other three inheriting SqlDriver's - // `autonumber: true`. So the retry protects ONE backend, and this is the - // other one. + it('the duplicate is REFUSED and the number re-issued — it used to land silently', async () => { + // ⚠️ INVERTED IN PLACE by #13197. Until then this test asserted the + // DEFECT as correct behaviour — `written.doc_no === 'D-0005'` and + // `rows.filter(…D-0005).toHaveLength(2)`, over a comment calling two rows + // carrying one business identifier "the honest outcome". It was honest: + // `InMemoryDriver.create` was a `table.push()` storing no constraints of + // any kind (#4065's WEAK-oracle docstring), so a duplicate raised + // nothing, there was no error to catch, and the re-issue this file pins + // elsewhere never ran. + // + // It is kept inverted rather than deleted or re-baselined, deliberately: + // read from `main`, a vanished pin and a silently-returned defect look + // identical, and this one's fact was falsified by a later card rather + // than being wrong when written. + // + // **What refuses it now.** `@objectstack/driver-memory` enforces + // field-level `unique` (`memory-unique-constraint.ts`), with + // `driver-sql`'s ADR-0120 D1/D3 scoping, and raises the ADR-0112 + // envelope `code: 'UNIQUE_VIOLATION'` / `status: 409` — the shape + // `memoryDuplicate` below reproduces. `isUniqueViolationError` reads that + // code (#13197 added the limb), so the collision branch is REACHABLE on + // this driver for the first time: the stale counter is dropped, the + // counter re-seeds from the store's real max, and the number is re-issued. + // + // **What did NOT change, and must not.** The engine-side pre-issue + // existence probe stays REJECTED, for the cost reason `engine.ts` states + // at `createWithAutonumberResync`: a probe costs a query on every insert + // — the cost this resync exists to avoid — and is still racy, so it would + // trade a silent duplicate for a rarer silent duplicate at double the + // read cost. The fix landed in the driver precisely so the engine did not + // need one. // - // No `uniqueOn` — this rig is the memory driver's shape exactly. + // `uniqueOn` + the memory envelope: this rig is the memory driver's shape + // exactly, as it is now. const rows = storedRows('doc_no', ['D-0003']); - const { engine, driver } = makeRig(SCHEMA, rows); + const { engine, driver } = makeRig(SCHEMA, rows, { + uniqueOn: 'doc_no', + duplicateError: memoryDuplicate, + }); await engine.init(); await engine.insert('doc', { title: 'first' }); // D-0004 @@ -678,17 +734,14 @@ describe('ObjectQL autonumber resync (#6806)', () => { const written = await engine.insert('doc', { title: 'second' }); - // The honest outcome: the number is issued a second time, the write - // SUCCEEDS, and nothing anywhere says so. Fixing this needs uniqueness in - // the driver, and a pre-issue existence probe in the engine would cost a - // query per insert and still be racy. (`packages/drivers/**` was under - // the #5499 freeze when this was written; it was lifted on 2026-08-11, - // and the remedy is still the driver's.) Reported as a follow-up, not - // implemented here. - expect(written.doc_no).toBe('D-0005'); - expect(rows.filter((r) => r.doc_no === 'D-0005')).toHaveLength(2); - // One create attempt: with no rejection there is nothing to retry. - expect(driver.create).toHaveBeenCalledTimes(2); // 'first' + 'second' + // The number is issued ONCE. D-0005 was refused, the counter re-seeded + // from the real max (5) and the re-issue landed above it. + expect(written.doc_no).toBe('D-0006'); + expect(rows.filter((r) => r.doc_no === 'D-0005')).toHaveLength(1); + // Three creates: 'first', the REFUSED 'second', and its re-issue. The + // refused attempt is what the old assertion could not observe, because + // there was nothing to refuse it. + expect(driver.create).toHaveBeenCalledTimes(3); }); it('...but ADOPTION still holds there — it needs no constraint at all', async () => { diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 456995a3d7..2fd8872234 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -4427,15 +4427,17 @@ export class ObjectQL implements IObjectQLEngine { * * | driver | `supports.autonumber` | fallback path? | uniqueness on the column | a collision appears as | * |:---|:---|:---|:---|:---| - * | driver-memory | `supports = {}` | **yes** | **none, ever** | **nothing** — a silent duplicate | + * | driver-memory | `supports = {}` | **yes** | field-level `unique`, since #13197 (ADR-0120 D1/D3 scoping) | `UNIQUE_VIOLATION` / 409 → re-seed + re-issue, here | * | driver-mongodb | absent (`{ batchSchemaSync: true }`) | **yes** | single-field unique index when the field declares `unique` | `E11000 duplicate key` → re-seed + re-issue, here | * | driver-sql | `autonumber: true` | no | — | — | * | driver-sqlite-wasm | inherited (`extends SqlDriver`, no `supports` override) | no | — | — | * | driver-turso | inherited (`...super.supports`) | no | — | — | * - * So the retry protects essentially ONE backend: driver-mongodb with a - * `unique` autonumber field. That is not a new claim — it is the reading the - * repo already ruled and gates, in + * So the retry protects the TWO fallback backends — and it protected only one + * of them until #13197, because `driver-memory` enforced no uniqueness at all + * and had nothing to reject with. Which driver ISSUES the number is a + * separate question and is unmoved by that: it is the reading the repo + * already ruled and gates, in * `scripts/driver-memory-census.ledger.json`'s disposition for * `packages/runtime/src/autonumber-seed-cross-side-parity.integration.test.ts` * (axis `ruled-permanent`, «#6664 A, maintainer 2026-08-08 — inherits #5704 @@ -4446,27 +4448,33 @@ export class ObjectQL implements IObjectQLEngine { * a second answer to "who owns the autonumber counter" is the same * one-contract-two-numbers defect this lane keeps closing (#6832). * - * `InMemoryDriver.create` is a `table.push()` and it stores no constraints of - * any kind — its own docstring says so since #4065, and calls itself a WEAK - * oracle for exactly this reason. So on driver-memory an out-of-process - * duplicate cannot raise anything for this method to catch, and the number - * lands twice in the rendered field with no error anywhere. - * - * That is stated rather than papered over (PD #10: never advertise a - * capability the runtime does not deliver). What covers driver-memory is the - * OTHER half of this resync — {@link adoptExplicitAutonumber} — which needs no - * constraint at all because it never waits for a rejection. Between them: drift - * the engine can observe is fixed on every driver; drift only the store can - * report is fixed wherever the store reports it. + * ⚠️ **This paragraph used to record a live defect; #13197 closed it, and the + * ⛔ below is why the closure went where it did.** `InMemoryDriver.create` was + * a `table.push()` storing no constraints of any kind (#4065's WEAK-oracle + * docstring), so an out-of-process duplicate raised nothing for this method to + * catch and the number landed twice in the rendered field with no error + * anywhere. Since #13197 that driver enforces field-level `unique` + * (`driver-memory`'s `memory-unique-constraint.ts`, with `driver-sql`'s + * ADR-0120 D1/D3 scoping) and refuses the collision in the ADR-0112 envelope, + * which `isUniqueViolationError` reads — so the branch below is reachable + * there and the number converges instead of duplicating. + * + * What covers driver-memory in ADDITION is the other half of this resync — + * {@link adoptExplicitAutonumber} — which needs no constraint at all because it + * never waits for a rejection. Between them: drift the engine can observe is + * fixed on every driver; drift only the store can report is fixed wherever the + * store reports it. * * ⛔ The remedy for the silent-duplicate row is uniqueness enforcement in the * driver, NOT a pre-issue existence probe here: a probe costs a query on every * insert (the cost this resync was designed to avoid) and is still racy, so it * would trade a silent duplicate for a rarer silent duplicate at double the - * read cost. `packages/drivers/**` was under the #5499 investment freeze when - * this was written, so that work was not this change's to do; the freeze was - * lifted on 2026-08-11 and the remedy is still the driver's, still not done - * here. + * read cost. That argument is UNCHANGED by #13197 and is not a historical + * note — it is the standing reason no probe is added here, and it is what the + * driver-side fix was chosen over. (`packages/drivers/**` was under the #5499 + * investment freeze when this comment was first written, which is why the work + * was deferred rather than misplaced; the freeze lifted on 2026-08-11 and the + * remedy landed where it always belonged, in the driver.) * * # And when it does not converge * diff --git a/packages/types/src/unique-violation.test.ts b/packages/types/src/unique-violation.test.ts index 1523f52a3f..933400f23b 100644 --- a/packages/types/src/unique-violation.test.ts +++ b/packages/types/src/unique-violation.test.ts @@ -43,6 +43,52 @@ describe('isUniqueViolationError — input shapes', () => { expect(isUniqueViolationError({ code: 1062 })).toBe(true); expect(isUniqueViolationError({ code: 1452 })).toBe(false); }); + + /** + * [#13197] The platform's OWN registered code, on the `code` channel. + * + * Not a dialect and not a heuristic — `UNIQUE_VIOLATION` is the value + * `error-code-ledger.zod.ts` registers for this exact condition and the one + * `@objectstack/rest` answers a SQL conflict with, so reading it is a + * tautology rather than a widened limb. It became load-bearing when + * `driver-memory` grew field-level uniqueness: an in-process driver raises + * no dialect prose and no SQLSTATE, and a conflict this predicate does not + * recognise leaves `ObjectQL.createWithAutonumberResync` unable to re-seed + * — the counter stays warm and every following insert collides too + * (#5495's PROBE3 storm), i.e. a silent duplicate traded for a + * non-converging insert loop. + */ + it('recognises the platform\'s own `UNIQUE_VIOLATION` code, which in-process drivers raise (#13197)', () => { + expect(isUniqueViolationError({ code: 'UNIQUE_VIOLATION', status: 409 })).toBe(true); + expect( + isUniqueViolationError( + Object.assign(new Error('Unique constraint violated on `doc.doc_no`: a record with the value "D-0005" already exists. No record was written.'), { + code: 'UNIQUE_VIOLATION', + status: 409, + }), + ), + ).toBe(true); + // Still narrow: a NEIGHBOURING platform code is not this condition. + expect(isUniqueViolationError({ code: 'RESOURCE_CONFLICT', status: 409 })).toBe(false); + expect(isUniqueViolationError({ code: 'INVALID_FILTER', status: 400 })).toBe(false); + }); + + /** + * [#13197] The column question is answered `undefined` for that refusal, + * and that is the CORRECT answer rather than a gap: the driver names no + * column in a dialect spelling this module parses, and inventing one would + * mean imitating SQLite or Postgres prose. `undefined` is the documented + * fallback the autonumber resync already handles — the same answer MongoDB + * gives, and the reason an unnamed column counts as attributable there. + */ + it('names no column for the in-process refusal — the documented `undefined` fallback (#13197)', () => { + expect( + uniqueViolationColumn({ + code: 'UNIQUE_VIOLATION', + message: 'Unique constraint violated on `doc.doc_no`: a record with the value "D-0005" already exists. No record was written.', + }), + ).toBeUndefined(); + }); }); describe('isUniqueViolationError — the `cause` chain', () => { diff --git a/packages/types/src/unique-violation.ts b/packages/types/src/unique-violation.ts index 1d360f653c..d9b796305d 100644 --- a/packages/types/src/unique-violation.ts +++ b/packages/types/src/unique-violation.ts @@ -121,6 +121,13 @@ interface UniqueViolationSignature { * one addition, and not a new dialect: `@objectstack/metadata`'s * `schema-sync-errors.ts` already reads `errno` alongside `code` for exactly * these drivers, so a code-only read is a known gap rather than a decision. + * - `UNIQUE_VIOLATION` — the PLATFORM's own registered code + * (`error-code-ledger.zod.ts`), added by #13197 when `driver-memory` grew + * field-level uniqueness. It is not a dialect and not a heuristic: it is + * the value the platform already uses to MEAN "unique violation", so a + * limb reading it is a tautology, with none of the false-positive risk the + * message limbs are rationed against. It is also load-bearing rather than + * cosmetic — see the note below. * * The message limbs are a **superset of what `mapDataError` already treated as * 409**, which is what makes routing REST through this predicate incapable of @@ -179,9 +186,24 @@ interface UniqueViolationSignature { * servers for #8590, in both directions, including MySQL's `Duplicate entry` * path. A dialect added later needs its violation spelling added HERE, measured * off a thrown error — not a loosened limb. + * + * ## Why an in-process driver's refusal had to be recognised here (#13197) + * + * A driver that raises a conflict this predicate does not recognise is not + * merely "less well mapped" — it WEDGES the engine's autonumber resync. + * `ObjectQL.createWithAutonumberResync` drops the stale counter, re-seeds from + * the store and re-issues only when `isUniqueViolationError` says the rejection + * was a conflict; when it says no, the error propagates with the counter still + * warm, so the next insert collides too, one number at a time — #5495's PROBE3 + * storm, which that branch exists to eliminate. Before #13197 `driver-memory` + * enforced no uniqueness at all and the question never arose; the moment it + * refuses a duplicate, an unrecognised refusal would trade a silent duplicate + * for a non-converging insert loop. Recognising the platform's own code is what + * keeps the trade honest, and it is why the limb belongs on the `codes` channel + * rather than in prose the driver would have to imitate a dialect to emit. */ const UNIQUE_VIOLATION: UniqueViolationSignature = { - codes: new Set(['23505', 'ER_DUP_ENTRY', 'SQLITE_CONSTRAINT_UNIQUE']), + codes: new Set(['23505', 'ER_DUP_ENTRY', 'SQLITE_CONSTRAINT_UNIQUE', 'UNIQUE_VIOLATION']), errnos: new Set([1062]), message: /unique constraint failed|violates unique constraint|unique violation|duplicate key|duplicate entry/i, };