diff --git a/.changeset/24583-plain-organization-key-part.md b/.changeset/24583-plain-organization-key-part.md new file mode 100644 index 00000000000..7201687ede9 --- /dev/null +++ b/.changeset/24583-plain-organization-key-part.md @@ -0,0 +1,20 @@ +--- +'@objectstack/driver-sql': minor +'@objectstack/driver-turso': minor +'@objectstack/driver-memory': minor +'@objectstack/objectql': minor +'@objectstack/metadata-protocol': minor +'@objectstack/cli': minor +--- + +feat(driver-sql,driver-turso,driver-memory,objectql,metadata-protocol,cli): an organization-scoped unique index keys the plain organization column, and an index an earlier release built with `COALESCE(organization_id, '__global__')` is read as the same index on a certified database + +Clause-②: yes (widening) +Metadata-protocol: semantic + +18.x step 7b-1 + 7b-2 (ADR-0131 D1: the tenant column is NOT NULL, so the NULL-organization bucket of ADR-0120 D3 has no rows left to hold). + +- **New DDL is plain.** A field-level `unique: true` / `'organization'` on a tenant-scoped object, and a declared index with `unique: 'organization'`, now build `UNIQUE (organization_id, …)` instead of `UNIQUE (COALESCE(organization_id, '__global__'), …)`. Index names do not change. The same holds for the Turso remote face, the MySQL hash shadow (its generated column now hashes the plain column), the in-memory driver's unique enforcement, `os generate migration` (which now emits the organization composite it used to list as NOT EMITTED), and the `sys_setting` row-identity index (`COALESCE(user_id, '')` stays). +- **Existing indexes stay as they are.** Nothing is renamed, and no declared organization composite is dropped or rebuilt by the boot, `os migrate plan` or `os migrate apply`; on SQLite the ownership ceremony's NOT NULL step rebuilds the tables it tightens, and their composites come back plain, enforcing the same constraint. One index is re-keyed: the `sys_setting` row-identity index, which the metadata protocol rebuilds on every boot, keys the plain `organization_id` from the next boot on (probe-first; a pure relaxation, so it cannot fail on existing rows). The schema differ reads `COALESCE(, …)` and the plain tenant column as one key part, in both directions, where the column is physically `NOT NULL` or the database is certified (its organization-ownership birth marker, or the ownership ceremony's marker at the current shape generation). So `os migrate plan` and the boot report no `recreate_index` for those indexes. On an uncertified database with a nullable tenant column the two forms stay different constraints: the finding is reported, and it names `os migrate organization-ownership` as its remedy. +- **What widens.** Where a tenant column is still nullable (every table of a database 18.0.0 created, an uncertified 17.x / 18.0 database, a rotation shard, a Turso remote table, a table the ceremony reported), a unique index created from now on does not constrain rows whose organization is NULL, exactly like a NULL in any other key part; in the in-memory driver a record with no organization is exempt from an organization-scoped unique. The unique duplicate pre-flight no longer reports groups of rows with a NULL key part, since the index never refuses them. +- **Who sees it.** A fresh install and every ceremonied table whose tenant column is NOT NULL see nothing. A database 18.0.0 created from empty is certified by its birth marker yet keeps nullable tenant columns: its existing indexes are kept unchanged, and a new organization unique declared on one of its tables constrains only rows that carry an organization. diff --git a/packages/cli/src/commands/generate-declared-unique-index.pin.test.ts b/packages/cli/src/commands/generate-declared-unique-index.pin.test.ts index cee4962373e..cb6feeee89f 100644 --- a/packages/cli/src/commands/generate-declared-unique-index.pin.test.ts +++ b/packages/cli/src/commands/generate-declared-unique-index.pin.test.ts @@ -34,26 +34,31 @@ * constraint. The COLUMN — the #16091 result this change must not spend — read * `character varying(100)` on all three both before and after. * - * ## What this pin does NOT claim, stated so nobody reads it as closed + * ## The organization composite — emitted since ADR-0131 D1 + * + * Until ADR-0131 D1 the ORGANIZATION-SCOPED form was the ADR-0120 D3 + * expression `(COALESCE(, '__global__'), )`, which neither format + * could spell, so it was deliberately unemitted and named in a NOT EMITTED + * line. The driver now keys the PLAIN tenant column — a fresh table's tenant + * column is NOT NULL, so the ADR-0120 D3 NULL-organization bucket the COALESCE + * existed for is gone from new DDL — and the bare `(organization_id, field)` IS + * the platform's constraint. So both formats emit it, and this file pins that + * the generated table carries the SAME composite the driver creates and refuses + * the same in-organization duplicate (block A), and that no NOT EMITTED line is + * printed for it any more (block B). * - * Two declaration shapes are deliberately unemitted, and this file asserts that - * they are unemitted *and named*, never that they are handled: + * ## What this pin does NOT claim, stated so nobody reads it as closed * - * 1. The ADR-0120 D3 ORGANIZATION-SCOPED form — `(COALESCE(, - * '__global__'), )`. Emitting the bare composite instead would be - * worse than emitting nothing: under SQL's NULL-distinct UNIQUE a bare - * `(organization_id, field)` constrains NOTHING on rows with no - * organization, which on a single-tenant stack is every row (#5030) — a - * constraint advertised and not delivered, which is the failure Prime - * Directive #10 names. - * 2. OBJECT-LEVEL `indexes[]`. `normalizeDeclaredIndex` is a second - * normalizer that reads the same `unique: true` token DIFFERENTLY (verbatim - * as global, a maintainer ruling), so it is a second transcription with a - * second pin, not a loop added to this one. + * One declaration shape is deliberately unemitted, and this file asserts that it + * is unemitted *and named*, never that it is handled: OBJECT-LEVEL `indexes[]`. + * `normalizeDeclaredIndex` is a second normalizer that reads the same + * `unique: true` token DIFFERENTLY (verbatim as global, a maintainer ruling), so + * it is a second transcription with a second pin, not a loop added to this one. * - * ⭐ Measured for the record, because "can the TypeScript format express an - * expression key part through knex at all" was the open question that kept the - * scoped form off this change (knex 3.3.0, live PostgreSQL 16.13): + * ⭐ Measured for the record — HISTORICAL since ADR-0131 D1 retired the + * expression key part, kept because it is why the scoped form was once + * unemitted, and stays true for any future expression key part (knex 3.3.0, + * live PostgreSQL 16.13): * * ``` * table.unique([knex.raw("COALESCE(...)"), 'f'], {indexName}) @@ -72,8 +77,9 @@ * knex handle and `db.raw` is exactly the seam `SqlDriver.createNullSafeUniqueIndex` * already uses for this — but knex's SCHEMA BUILDER cannot express it in either * format, so the scoped form costs a raw statement rather than another - * `table.unique(...)` line. That is a fact for whoever takes the scoped half, - * recorded here rather than acted on. + * `table.unique(...)` line. ADR-0131 D1 made that moot for the organization + * composite (its key parts are plain columns now); it is recorded for whoever + * next meets an expression key part. * * ## Why this pin reads the driver instead of asserting the names * @@ -82,7 +88,7 @@ * transcribed `uniq_probe_keyed_unique` would re-create the defect one layer up * and stay green the day the driver's naming moves. Every name and every key-part * ordering here is recomputed from `driver-sql`'s own exported - * `uniqueIndexesFromFields` / `buildIndexName` / `GLOBAL_TENANT`. + * `uniqueIndexesFromFields` / `buildIndexName`. * * ⭐ And the leaves are not the authority. Above the differential sits THE REAL * CHAIN: all three producers driven into ONE in-memory better-sqlite3 database, @@ -109,7 +115,7 @@ * measuring the parser. */ -import { SqlDriver, GLOBAL_TENANT, buildIndexName, uniqueIndexesFromFields } from '@objectstack/driver-sql'; +import { SqlDriver, buildIndexName, uniqueIndexesFromFields } from '@objectstack/driver-sql'; import { afterAll, describe, expect, it } from 'vitest'; import { generateMigrationSql, generateMigrationTs } from './generate.js'; @@ -210,8 +216,12 @@ function loadEmittedTs(ts: string): { up: (db: any) => Promise; down: (db: interface Probe { label: string; object: Record; - /** What the object's declarations mean for the emitters, asserted below. */ - expect: 'emitted' | 'scoped-not-emitted' | 'no-column' | 'none'; + /** + * What the object's declarations mean for the emitters, asserted below. + * `scoped` is the organization composite — emitted like `emitted`, with the + * plain tenant column as its FIRST key part (ADR-0131 D1). + */ + expect: 'emitted' | 'scoped' | 'no-column' | 'none'; } const ORG = { type: 'text', maxLength: 64 } as const; @@ -229,14 +239,14 @@ const CORPUS: Probe[] = [ expect: 'emitted', }, { - label: '`unique: true` beside an organization column — the ADR-0120 D3 scoped composite', + label: '`unique: true` beside an organization column — the plain organization composite (ADR-0131 D1)', object: { name: 'p_scoped_true', fields: { organization_id: ORG, f: { ...KEYED, unique: true } } }, - expect: 'scoped-not-emitted', + expect: 'scoped', }, { label: "explicit `unique: 'organization'` — the same scoped composite", object: { name: 'p_scoped_word', fields: { organization_id: ORG, f: { ...KEYED, unique: 'organization' } } }, - expect: 'scoped-not-emitted', + expect: 'scoped', }, { label: 'unique ON the organization column itself — "one row per tenant" stays single-column', @@ -259,7 +269,7 @@ const CORPUS: Probe[] = [ tenancy: { tenantField: 'org' }, fields: { org: ORG, f: { ...KEYED, unique: true } }, }, - expect: 'scoped-not-emitted', + expect: 'scoped', }, { label: 'a name past the 60-character identifier budget — hash-suffixed', @@ -349,8 +359,9 @@ describe('#16317 — the index each generator emits is the index the driver CREA // ⭐ THE AUTHORITY. Whatever the driver's own table carries, the ts // format's table carries too — by NAME, because both go through knex's - // named-index spelling — except for the two declared exclusions. - const emittable = probe.expect === 'emitted'; + // named-index spelling — except for the declared exclusions. Since + // ADR-0131 D1 the organization composite is one of the emitted shapes. + const emittable = probe.expect === 'emitted' || probe.expect === 'scoped'; if (emittable) { expect(uniqueShapes(built.tsgen)).toEqual(uniqueShapes(built.driver)); expect(uniqueShapes(built.sqlgen)).toEqual(uniqueShapes(built.driver)); @@ -362,22 +373,28 @@ describe('#16317 — the index each generator emits is the index the driver CREA for (const want of wanted) expect(built.sqlText).toContain(`CONSTRAINT "${want.name}" UNIQUE (`); } - if (probe.expect === 'scoped-not-emitted') { - // The driver DOES build it — the expression key part is why we do not. + if (probe.expect === 'scoped') { + // ADR-0131 D1: the driver keys the PLAIN tenant column, so the + // composite it creates is a two-column list with the tenant FIRST and + // no expression key part — the shape both formats can spell, and do. + const tenant = newOracle().tenantFieldFor(probe.object); + expect(tenant).not.toBeNull(); expect(wanted.length).toBeGreaterThan(0); for (const want of wanted) { - expect(want.nullSafeColumns ?? []).not.toHaveLength(0); - // ⛔ Absence must be loud: named in the generated file, both formats. - expect(built.sqlText).toContain(`NOT EMITTED: UNIQUE index "${want.name}"`); - expect(built.tsText).toContain(`NOT EMITTED: UNIQUE index '${want.name}'`); - expect(built.sqlText).toContain(`COALESCE("${want.nullSafeColumns![0]}", '${GLOBAL_TENANT}')`); - // ...and NOT emitted as the bare composite, which would advertise a - // constraint the table does not carry (#5030). - expect(built.sqlText).not.toContain(`CONSTRAINT "${want.name}" UNIQUE (`); - expect(built.tsText).not.toContain(`indexName: '${want.name}'`); + expect(want.nullSafeColumns ?? []).toHaveLength(0); + expect(want.columns).toHaveLength(2); + expect(want.columns[0]).toBe(tenant); } - expect(uniqueShapes(built.tsgen)).toHaveLength(0); - expect(uniqueShapes(built.sqlgen)).toHaveLength(0); + // The catalog agrees: every producer's table carries the composite as + // two PLAIN key parts (SQLite reports `null` for an expression part). + for (const rows of [built.driver, built.sqlgen, built.tsgen]) { + expect(uniqueShapes(rows)).toEqual(wanted.map((w) => w.columns.join(',')).sort()); + } + // ...and nothing is announced as missing, because nothing is. + expect(built.sqlText).not.toContain('NOT EMITTED'); + expect(built.tsText).not.toContain('NOT EMITTED'); + expect(built.sqlText).not.toContain('COALESCE'); + expect(built.tsText).not.toContain('COALESCE'); } if (probe.expect === 'no-column') { @@ -425,6 +442,44 @@ describe('#16317 — the index each generator emits is the index the driver CREA expect(verdicts).toEqual({ driver: 'refused', sqlgen: 'refused', tsgen: 'refused' }); }); + /** + * ⭐ The same consequence for the ORGANIZATION composite (ADR-0131 D1): a + * duplicate inside ONE organization is refused by the driver's table and by + * BOTH generated ones, and the same value in a SECOND organization is accepted + * by all three — uniqueness is per organization, not platform-wide. Rows + * carry an explicit organization: a fresh driver table's tenant column is + * NOT NULL, so a NULL-organization row is not a case the driver's table has. + */ + it('a duplicate inside one organization is refused by BOTH generated tables and the driver; another organization is not', async () => { + const object = { name: 'p_enforce_org', fields: { organization_id: ORG, f: { ...KEYED, unique: true } } }; + const built = await threeTables(object); + const insert = (db: any, id: string, org: string) => + db.raw(`INSERT INTO "p_enforce_org" ("id", "organization_id", "f") VALUES (?, ?, 'dup')`, [id, org]); + const verdicts: Record = {}; + for (const [who, db] of Object.entries(built.dbs)) { + await insert(db, 'a', 'org_1'); + let sameOrg: string; + try { + await insert(db, 'b', 'org_1'); + sameOrg = 'accepted'; + } catch (err) { + // A UNIQUE refusal specifically — not any error the insert could raise. + expect(String((err as Error).message)).toMatch(/UNIQUE constraint failed/); + sameOrg = 'refused'; + } + let otherOrg: string; + try { + await insert(db, 'c', 'org_2'); + otherOrg = 'accepted'; + } catch { + otherOrg = 'refused'; + } + verdicts[who] = { sameOrg, otherOrg }; + } + const perOrg = { sameOrg: 'refused', otherOrg: 'accepted' }; + expect(verdicts).toEqual({ driver: perOrg, sqlgen: perOrg, tsgen: perOrg }); + }); + /** * NON-VACUITY for the whole block. A corpus that built no unique index at all * would satisfy every `toEqual` above by agreeing on emptiness. @@ -436,7 +491,7 @@ describe('#16317 — the index each generator emits is the index the driver CREA for (const probe of CORPUS) { const wanted = driverIndexes(oracle, probe.object); if (probe.expect === 'emitted') counts.emitted += wanted.length; - if (probe.expect === 'scoped-not-emitted') counts.scoped += wanted.length; + if (probe.expect === 'scoped') counts.scoped += wanted.length; if (probe.expect === 'no-column') counts.noColumn += wanted.length; if (probe.expect === 'none') counts.none += wanted.length; for (const w of wanted) if (/_[0-9a-f]{8}$/.test(w.name)) counts.hashed += 1; @@ -477,11 +532,29 @@ describe('#16317 — the transcriptions in generate.ts are the driver\'s own', ( expect(hashed).toBeGreaterThanOrEqual(3); }); - /** The `__global__` sentinel the unemitted-index note names. */ - it('the sentinel the NOT EMITTED note prints is the driver\'s GLOBAL_TENANT', () => { + /** + * The `__global__` sentinel retired with the ADR-0120 D3 expression key part + * (ADR-0131 D1), and with it the NOT EMITTED note that printed it. What is + * left to pin is that the generators stopped announcing the organization + * composite as missing — a stale note would tell the operator to hand-write + * an index the file already creates — and that no COALESCE spelling of it + * survives in either format, comment or code. + */ + it('no NOT EMITTED line is printed for an organization composite, in either format', () => { const object = { name: 'g_probe', fields: { organization_id: ORG, f: { ...KEYED, unique: true } } }; - expect(generateMigrationSql(configFor(object))).toContain(`'${GLOBAL_TENANT}'`); - expect(generateMigrationTs(configFor(object))).toContain(`'${GLOBAL_TENANT}'`); + const [want] = driverIndexes(oracle, object); + expect(want.columns).toEqual(['organization_id', 'f']); + const sql = generateMigrationSql(configFor(object)); + const ts = generateMigrationTs(configFor(object)); + expect(sql).not.toContain('NOT EMITTED'); + expect(ts).not.toContain('NOT EMITTED'); + expect(sql).not.toContain('COALESCE'); + expect(ts).not.toContain('COALESCE'); + expect(sql).not.toContain('__global__'); + expect(ts).not.toContain('__global__'); + // Non-vacuity: the index is in the file — as itself, not as a note. + expect(sql).toContain(`CONSTRAINT "${want.name}" UNIQUE ("organization_id", "f")`); + expect(ts).toContain(`table.unique(['organization_id', 'f'], { indexName: '${want.name}' });`); }); /** @@ -495,16 +568,19 @@ describe('#16317 — the transcriptions in generate.ts are the driver\'s own', ( const ts = generateMigrationTs(configFor(probe.object)); for (const want of driverIndexes(oracle, probe.object)) { checked += 1; - const scoped = (want.nullSafeColumns ?? []).length > 0; - const emitted = probe.expect === 'emitted'; + // ADR-0131 D1: every key part is a plain column, the organization + // composite's included, so every emitted index is a column list. + expect(want.nullSafeColumns ?? []).toHaveLength(0); + const emitted = probe.expect === 'emitted' || probe.expect === 'scoped'; const cols = want.columns.map((c) => `"${c}"`).join(', '); if (emitted) { - expect(scoped).toBe(false); expect(sql).toContain(`CONSTRAINT "${want.name}" UNIQUE (${cols})`); expect(ts).toContain( `table.unique([${want.columns.map((c) => `'${c}'`).join(', ')}], { indexName: '${want.name}' });`, ); } else { + // Only the no-column exclusion is left to announce. + expect(probe.expect).toBe('no-column'); expect(sql).toContain(`NOT EMITTED: UNIQUE index "${want.name}" on (${want.columns.join(', ')})`); expect(ts).toContain(`NOT EMITTED: UNIQUE index '${want.name}' on (${want.columns.join(', ')})`); } diff --git a/packages/cli/src/commands/generate.ts b/packages/cli/src/commands/generate.ts index bd9ccfc23f2..41bbfb23ab6 100644 --- a/packages/cli/src/commands/generate.ts +++ b/packages/cli/src/commands/generate.ts @@ -3226,17 +3226,6 @@ function indexKeyColumns(obj: Record): ReadonlySet { * is what keeps the transcription honest — it recomputes every name from the * driver's own exported `uniqueIndexesFromFields` and compares. */ -/** - * `driver-sql`'s `GLOBAL_TENANT` — the sentinel the ADR-0120 D3 NULL-safe - * organization key part folds a NULL organization onto. - * - * Transcribed for the same #5726 reason as the rest of this block, and it - * reaches only a COMMENT in the generated file: this format emits no expression - * key part, so the sentinel is here to NAME the index that was not emitted, not - * to build one. The pin compares it against the driver's own export. - */ -const GLOBAL_TENANT_KEY = '__global__'; - const INDEX_NAME_MAX = 60; /** Chars kept from `_` before the `_` suffix of a truncated name. */ const INDEX_NAME_HEAD = INDEX_NAME_MAX - 9; @@ -3253,15 +3242,13 @@ function buildIndexName(table: string, columns: string[], unique: boolean): stri interface MirroredUniqueIndex { /** {@link buildIndexName}'s answer — the identifier the driver would use. */ name: string; - /** The key parts, in the driver's order (tenant column first when scoped). */ - columns: string[]; /** - * The tenant column whose key part materializes as the ADR-0120 D3 NULL-safe - * expression `COALESCE(, '__global__')` rather than as a bare column, - * or `null` for a plain single-column unique. An expression key part is what - * neither format emits — see {@link uniqueIndexesForObject}. + * The key parts, in the driver's order (tenant column first when scoped). + * Every part is a plain column: since ADR-0131 D1 the driver keys the + * NOT NULL tenant column itself, so an organization composite is a column + * list both formats can emit. */ - nullSafeColumn: string | null; + columns: string[]; } /** @@ -3276,10 +3263,10 @@ interface MirroredUniqueIndex { * {@link isUniqueScopeDeclared}, {@link isOrganizationScopedUnique} and * {@link tenantFieldOf} — so they cannot disagree about which fields are keyed. * - * Scoping rule, transcribed from the driver (ADR-0120 D1/D3): + * Scoping rule, transcribed from the driver (ADR-0120 D1, ADR-0131 D1): * - `unique: 'global'` → single-column `(field)`, platform-wide. * - `unique: true` / `'organization'` on a tenant-scoped table → composite - * `(COALESCE(tenantField, '__global__'), field)`, tenant column FIRST. + * `(tenantField, field)`, tenant column FIRST. * - `unique: true` / `'organization'` with no tenant column → `(field)`. * - a unique declaration ON the tenant column itself stays single-column — * `(organization_id, organization_id)` is not a constraint. @@ -3301,11 +3288,7 @@ function uniqueIndexesForObject(obj: Record): MirroredUniqueIndex[] const scoped = isOrganizationScopedUnique(field.unique) && tenantField != null && tenantField !== name; const columns = scoped ? [tenantField as string, name] : [name]; - out.push({ - name: buildIndexName(table, columns, true), - columns, - nullSafeColumn: scoped ? (tenantField as string) : null, - }); + out.push({ name: buildIndexName(table, columns, true), columns }); } return out; } @@ -3314,24 +3297,19 @@ function uniqueIndexesForObject(obj: Record): MirroredUniqueIndex[] * The subset of {@link uniqueIndexesForObject} a format may actually emit, and * a line for every one it may not. * - * Two exclusions, and BOTH are the driver's own behaviour rather than a - * convenience here: + * One exclusion, and it is the driver's own behaviour rather than a + * convenience here: a key part with no column. `syncDeclaredIndexes` skips a + * declared index whose columns are not in `physicalColumns` and warns; the + * generator's equivalent of "not materialized" is a field this file emits no + * column for — a VIRTUAL `formula` (commit 08706f0e0). Emitting the index + * anyway produces DDL that refuses to run at all. * - * 1. A key part with no column. `syncDeclaredIndexes` skips a declared index - * whose columns are not in `physicalColumns` and warns; the generator's - * equivalent of "not materialized" is a field this file emits no column - * for — a VIRTUAL `formula` (commit 08706f0e0). Emitting the index anyway produces - * DDL that refuses to run at all. - * 2. An EXPRESSION key part. `COALESCE(, '__global__')` is what the - * driver builds through raw DDL precisely because knex's schema builder - * cannot express it, and it is NOT interchangeable with the bare composite: - * under SQL's NULL-distinct UNIQUE a bare `(organization_id, field)` - * enforces NOTHING on rows without an organization, which on a - * single-tenant stack is every row (#5030). So emitting the bare composite - * here would ADVERTISE a constraint the table does not carry — worse than - * emitting nothing, and the failure mode Prime Directive #10 names. + * The organization composite is emitted: since ADR-0131 D1 the driver keys the + * plain tenant column, so the bare `(organization_id, field)` IS the platform's + * constraint, no longer the ADR-0120 D3 `COALESCE` expression neither format + * could spell. * - * ⛔ Neither exclusion is silent. A skipped index is named in the generated file + * ⛔ The exclusion is not silent. A skipped index is named in the generated file * itself, with what it would have keyed, because the operator reading that file * is the only person who can act on it — "Absence must be loud". */ @@ -3347,15 +3325,6 @@ function partitionUniqueIndexes( skipped.push({ index, why: `no column is generated for ${missing.join(', ')}` }); continue; } - if (index.nullSafeColumn !== null) { - skipped.push({ - index, - why: - `its organization key part is COALESCE("${index.nullSafeColumn}", '${GLOBAL_TENANT_KEY}'), ` + - 'an expression key part this format does not emit; the platform creates it at boot', - }); - continue; - } emit.push(index); } return { emit, skipped }; diff --git a/packages/cli/src/utils/schema-migrate.dev-self-heal-fence.integration.test.ts b/packages/cli/src/utils/schema-migrate.dev-self-heal-fence.integration.test.ts index 9bd08f8a6fe..f76dc611a9c 100644 --- a/packages/cli/src/utils/schema-migrate.dev-self-heal-fence.integration.test.ts +++ b/packages/cli/src/utils/schema-migrate.dev-self-heal-fence.integration.test.ts @@ -20,14 +20,29 @@ * * ## The fixture * - * The card's reproduction, in-process: one org-scoped `unique` field, whose - * index a plain boot creates NULL-safe (`COALESCE(organization_id, …)`), then - * replaced by the bare `(organization_id, qa_code)` index an older release - * left behind. That is a `recreate_index` drift the duplicate probe finds - * clean, i.e. `safe`: exactly what the self-heal applies. The serving - * declaration (`createStandaloneStack({ dev: true })`, what `os dev` hands it) - * is the positive control — it heals the same staged file, so the fence cases - * cannot be green because the fixture has nothing to heal. + * One org-scoped `unique` field, whose `(organization_id, qa_code)` composite + * a plain boot creates, plus the platform-wide single-column UNIQUE on + * `qa_code` a pre-#3696 release left behind — the state every additive boot + * after such an upgrade produces, since the sync creates the composite by name + * and never drops anything. The differ reports it as `replace_unique_index`, a + * pure relaxation it classes `safe`: exactly what the self-heal applies, and + * what a one-shot boot must leave alone. The serving declaration + * (`createStandaloneStack({ dev: true })`, what `os dev` hands it) is the + * positive control — it heals the same staged file, so the fence cases cannot + * be green because the fixture has nothing to heal. + * + * ⚠️ Why not the drift the card staged. #21733 reproduced with the ADR-0120 D4 + * tightening — a bare composite against the NULL-safe expectation, a + * `recreate_index` the duplicate probe recategorised `safe`. ADR-0131 D1 + * retired it: the expectation is the plain composite now, so a bare composite + * MATCHES and there is no drift at all. Its nearest successor (a COALESCE + * index against the plain expectation on an uncertified table) is a + * DESTRUCTIVE `recreate_index` the self-heal never applies under any + * declaration, so it could not tell the fence from the heal; and a fresh + * table's tenant column is NOT NULL, which certifies it and makes the COALESCE + * form match too. The fence is about the self-heal's `safe` subset reaching a + * one-shot boot, not about any one op, so the fixture carries the `safe` op + * that is still live. * * Every case runs with `NODE_ENV=development` — the input the fence is about. */ @@ -38,7 +53,7 @@ import { randomBytes } from 'node:crypto'; import { tmpdir } from 'node:os'; import { dirname, join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; -import { SqlDriver } from '@objectstack/driver-sql'; +import { SqlDriver, legacyUniqueIndexNames } from '@objectstack/driver-sql'; import { isExitSignal } from './format.js'; import { bootSchemaStack } from './schema-migrate.js'; import MigratePlan from '../commands/migrate/plan.js'; @@ -55,7 +70,13 @@ const CLI_ROOT = resolve(HERE, '..', '..'); const CASE_TIMEOUT_MS = 120_000; +/** The organization composite the declaration asks for (ADR-0131 D1: plain key parts). */ const INDEX = 'uniq_scaf_item_organization_id_qa_code'; +/** + * The legacy platform-wide single-column UNIQUE, spelled by the driver's own + * `legacyUniqueIndexNames` — its `uniq_` arm, the one this differ recognises. + */ +const LEGACY = legacyUniqueIndexNames('scaf_item', 'qa_code')[1]; const ARTIFACT = { manifest: { id: 'com.example.scaf', namespace: 'scaf', name: 'scaf', version: '1.0.0', type: 'app' }, @@ -80,11 +101,11 @@ function probe(dbFile: string): SqlDriver { return new SqlDriver({ client: 'better-sqlite3', connection: { filename: dbFile }, useNullAsDefault: true }); } -/** The index's DDL exactly as `sqlite_master` stores it. */ -async function indexSql(dbFile: string): Promise { +/** An index's DDL exactly as `sqlite_master` stores it; `undefined` when absent. */ +async function indexSql(dbFile: string, name: string): Promise { const driver = probe(dbFile); try { - const rows = await (driver as any).knex.raw('SELECT sql FROM sqlite_master WHERE type = ? AND name = ?', ['index', INDEX]); + const rows = await (driver as any).knex.raw('SELECT sql FROM sqlite_master WHERE type = ? AND name = ?', ['index', name]); return (rows as Array<{ sql: string }>)[0]?.sql; } finally { await driver.disconnect(); @@ -153,7 +174,9 @@ async function runPlanJson(dbFile: string): Promise { } } -const isNullSafe = (sql: string | undefined): boolean => /COALESCE\(`?organization_id`?/.test(sql ?? ''); +/** The composite in its ADR-0131 D1 form: both key parts plain, no COALESCE. */ +const isPlainComposite = (sql: string | undefined): boolean => + /\(\s*[`"]?organization_id[`"]?\s*,\s*[`"]?qa_code[`"]?\s*\)/.test(sql ?? '') && !/COALESCE/i.test(sql ?? ''); beforeAll(async () => { for (const key of SAVED_KEYS) savedEnv[key] = process.env[key]; @@ -168,20 +191,20 @@ beforeAll(async () => { process.env.OS_ARTIFACT_PATH = join(dir, 'dist', 'objectstack.json'); writeFileSync(process.env.OS_ARTIFACT_PATH, JSON.stringify(ARTIFACT)); - // A plain boot creates the table and its NULL-safe unique index… + // A plain boot creates the table and its plain organization composite… stagedDb = join(dir, 'data', 'staged.db'); await bootServing(stagedDb, false); - expect(isNullSafe(await indexSql(stagedDb)), 'the fixture boot did not create the NULL-safe index').toBe(true); - // …which the card's step 3 replaces with the bare one an older release left. + const composite = await indexSql(stagedDb, INDEX); + expect(isPlainComposite(composite), `the fixture boot did not create the plain composite: ${composite}`).toBe(true); + expect(await indexSql(stagedDb, LEGACY), 'the fixture boot created the legacy index itself').toBeUndefined(); + // …beside which the staging step plants the global UNIQUE an older release left. const driver = probe(stagedDb); try { - const k = (driver as any).knex; - await k.raw(`DROP INDEX ${INDEX}`); - await k.raw(`CREATE UNIQUE INDEX ${INDEX} ON scaf_item (organization_id, qa_code)`); + await (driver as any).knex.raw(`CREATE UNIQUE INDEX ${LEGACY} ON scaf_item (qa_code)`); } finally { await driver.disconnect(); } - expect(isNullSafe(await indexSql(stagedDb)), 'the staging step did not leave the bare index').toBe(false); + expect(await indexSql(stagedDb, LEGACY), 'the staging step did not leave the legacy index').toBeDefined(); }, CASE_TIMEOUT_MS); afterAll(() => { @@ -194,11 +217,13 @@ afterAll(() => { }); describe('#21733 — the dev self-heal on the standalone stack, and its one-shot fence (NODE_ENV=development)', () => { - it('the serving declaration (`dev: true`, what `os dev` passes) heals the staged index on restart', async () => { + it('the serving declaration (`dev: true`, what `os dev` passes) relaxes the legacy global unique on restart', async () => { const dbFile = stagedCopy('serving-dev'); await bootServing(dbFile, true); - const sql = await indexSql(dbFile); - expect(isNullSafe(sql), `the dev boot left the bare index: ${sql}`).toBe(true); + const legacy = await indexSql(dbFile, LEGACY); + expect(legacy, `the dev boot left the legacy index: ${legacy}`).toBeUndefined(); + // A relaxation, not a removal: the per-organization constraint survives. + expect(isPlainComposite(await indexSql(dbFile, INDEX))).toBe(true); }, CASE_TIMEOUT_MS); it('a non-deferred one-shot boot (`bootSchemaStack`, every write mode) leaves the safe drift in place', async () => { @@ -212,9 +237,10 @@ describe('#21733 — the dev self-heal on the standalone stack, and its one-shot } // Still there, and still the kind the self-heal WOULD apply — so the // boot declined it rather than having nothing to do. - expect(drift.map((d) => `${d.category}:${d.op.type}`)).toContain('safe:recreate_index'); - const sql = await indexSql(dbFile); - expect(isNullSafe(sql), `a one-shot boot auto-applied safe drift: ${sql}`).toBe(false); + expect(drift.map((d) => `${d.category}:${d.op.type}`)).toContain('safe:replace_unique_index'); + const legacy = await indexSql(dbFile, LEGACY); + expect(legacy, 'a one-shot boot auto-applied safe drift').toBeDefined(); + expect(isPlainComposite(await indexSql(dbFile, INDEX))).toBe(true); }, CASE_TIMEOUT_MS); it('`os migrate plan` lists the drift as safe and leaves the staged database byte-identical', async () => { @@ -224,8 +250,8 @@ describe('#21733 — the dev self-heal on the standalone stack, and its one-shot expect( (plan.changes ?? []).map((d: { category: string; op: { type: string } }) => `${d.category}:${d.op.type}`), `plan output: ${JSON.stringify(plan).slice(0, 400)}`, - ).toContain('safe:recreate_index'); + ).toContain('safe:replace_unique_index'); expect(Buffer.compare(readFileSync(dbFile), before), 'os migrate plan wrote to the staged database').toBe(0); - expect(isNullSafe(await indexSql(dbFile))).toBe(false); + expect(await indexSql(dbFile, LEGACY)).toBeDefined(); }, CASE_TIMEOUT_MS); }); diff --git a/packages/cli/test/dev-standalone-self-heal.integration.test.ts b/packages/cli/test/dev-standalone-self-heal.integration.test.ts index 953bc47b48c..5bba0128cf1 100644 --- a/packages/cli/test/dev-standalone-self-heal.integration.test.ts +++ b/packages/cli/test/dev-standalone-self-heal.integration.test.ts @@ -23,14 +23,27 @@ * * ## What each leg pins * - * - boot 1 — a fresh `os dev` creates the NULL-safe index; with - * `OS_TELEMETRY_DB=0` it provisions no telemetry sibling (the opt-out). - * - boot 2 — after the card's step 3 restages the bare index, a plain `os dev` - * restart puts the NULL-safe one back, says `auto-reconciled`, and provisions + * - boot 1 — a fresh `os dev` creates the plain `(organization_id, qa_code)` + * composite (ADR-0131 D1: no COALESCE) and no legacy single-column UNIQUE; + * with `OS_TELEMETRY_DB=0` it provisions no telemetry sibling (the opt-out). + * - boot 2 — after the staging step plants the legacy platform-wide UNIQUE on + * `qa_code` beside the composite, a plain `os dev` restart drops it (a pure + * relaxation, the composite survives), says `auto-reconciled`, and provisions * `A.telemetry.db` (default-on in dev for a file-backed SQLite primary). * - boot 3 — a production boot (`os serve`, `NODE_ENV` unset) on the restaged * file reports the drift and leaves it: production never auto-migrates. * + * ⚠️ Why not the drift the card staged. #21733 reproduced with the ADR-0120 D4 + * tightening — a bare composite against the NULL-safe expectation, a + * `recreate_index` the duplicate probe recategorised `safe` (the table above). + * ADR-0131 D1 retired it: the expectation is the plain composite now, so a bare + * composite MATCHES and there is no drift at all. The defect is about the + * standalone stack never applying the self-heal's `safe` subset, not about any + * one op, so the staging carries the `safe` op that is still live: + * `replace_unique_index`, the legacy global UNIQUE an older release left (the + * same fixture `src/utils/schema-migrate.dev-self-heal-fence.integration.test.ts` + * stages). + * * The host-config path is pinned unchanged where it is decided * (`src/utils/storage-driver.test.ts`), and both hosts are held to one answer * per driver kind by `src/utils/dev-self-heal-host-parity.test.ts`; the @@ -53,6 +66,7 @@ import { tmpdir } from 'node:os'; import { join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; import Database from 'better-sqlite3'; +import { legacyUniqueIndexNames } from '@objectstack/driver-sql'; import { childEnv, E2E_SECRET_KEY, @@ -72,7 +86,13 @@ const READY = /Press Ctrl\+C to stop/; const BOOT_TIMEOUT_MS = 180_000; const ALL_BOOTS_TIMEOUT_MS = 3 * (BOOT_TIMEOUT_MS + 30_000); +/** The organization composite the declaration asks for (ADR-0131 D1: plain key parts). */ const INDEX = 'uniq_scaf_item_organization_id_qa_code'; +/** + * The legacy platform-wide single-column UNIQUE, spelled by the driver's own + * `legacyUniqueIndexNames` — its `uniq_` arm, the one the differ recognises. + */ +const LEGACY = legacyUniqueIndexNames('scaf_item', 'qa_code')[1]; /** The card's blank scaffold: one object, an org-scoped unique field, no plugins, no datasources. */ const SCAFFOLD = { @@ -86,8 +106,10 @@ const SCAFFOLD = { }; interface BootRecord { - /** The index DDL `sqlite_master` holds after the boot stopped. */ + /** The composite's DDL `sqlite_master` holds after the boot stopped. */ indexSql: string | undefined; + /** The legacy global UNIQUE's DDL after the boot stopped; `undefined` when absent. */ + legacySql: string | undefined; telemetryExists: boolean; output: string; } @@ -97,10 +119,10 @@ let dir: string; let db: string; const record: Partial> = {}; -function readIndexSql(): string | undefined { +function readIndexSql(name: string): string | undefined { const handle = new Database(db, { readonly: true }); try { - const row = handle.prepare("SELECT sql FROM sqlite_master WHERE type = 'index' AND name = ?").get(INDEX) as + const row = handle.prepare("SELECT sql FROM sqlite_master WHERE type = 'index' AND name = ?").get(name) as | { sql: string } | undefined; return row?.sql; @@ -109,11 +131,16 @@ function readIndexSql(): string | undefined { } } -/** The card's step 3: replace the NULL-safe unique index with the bare one an older release left. */ -function stageBareIndex(): void { +/** + * The staging step: plant, beside the plain composite, the platform-wide UNIQUE + * on `qa_code` a pre-#3696 release left — the state every additive boot after + * such an upgrade produces, since the sync creates the composite by name and + * never drops anything. Idempotent, so it restages after a heal. + */ +function stageLegacyIndex(): void { const handle = new Database(db); try { - handle.exec(`DROP INDEX ${INDEX}; CREATE UNIQUE INDEX ${INDEX} ON scaf_item (organization_id, qa_code);`); + handle.exec(`CREATE UNIQUE INDEX IF NOT EXISTS ${LEGACY} ON scaf_item (qa_code);`); } finally { handle.close(); } @@ -200,7 +227,12 @@ async function stopGroup(child: ChildProcess): Promise { async function recordBoot(leg: 'fresh' | 'restart' | 'production', boot: Promise): Promise { try { const output = await boot; - record[leg] = { indexSql: readIndexSql(), telemetryExists: existsSync(join(dir, 'A.telemetry.db')), output }; + record[leg] = { + indexSql: readIndexSql(INDEX), + legacySql: readIndexSql(LEGACY), + telemetryExists: existsSync(join(dir, 'A.telemetry.db')), + output, + }; } catch (error) { record[leg] = error instanceof Error ? error : new Error(String(error)); } @@ -213,7 +245,9 @@ function recorded(leg: 'fresh' | 'restart' | 'production'): BootRecord { return r; } -const isNullSafe = (sql: string | undefined): boolean => /COALESCE\(`?organization_id`?/.test(sql ?? ''); +/** The composite in its ADR-0131 D1 form: both key parts plain, no COALESCE. */ +const isPlainComposite = (sql: string | undefined): boolean => + /\(\s*[`"]?organization_id[`"]?\s*,\s*[`"]?qa_code[`"]?\s*\)/.test(sql ?? '') && !/COALESCE/i.test(sql ?? ''); beforeAll(async () => { requireBuiltCli(RUN_JS_RESOLVES_FROM_DIST); @@ -224,12 +258,12 @@ beforeAll(async () => { // Boot 1: fresh, with the telemetry opt-out. await recordBoot('fresh', bootUntilReady(['dev', '--no-watch', '-d', 'file:A.db', '-p', KERNEL_ASSIGNED_PORT], { OS_TELEMETRY_DB: '0' })); - // Boot 2: the card's restart, on the restaged bare index, telemetry default-on. - if (existsSync(db)) stageBareIndex(); + // Boot 2: the card's restart, on the staged legacy index, telemetry default-on. + if (existsSync(db)) stageLegacyIndex(); await recordBoot('restart', bootUntilReady(['dev', '--no-watch', '-d', 'file:A.db', '-p', KERNEL_ASSIGNED_PORT], {})); - // Boot 3: production (`os serve`, NODE_ENV unset) on the restaged bare index. - if (existsSync(db)) stageBareIndex(); + // Boot 3: production (`os serve`, NODE_ENV unset) on the restaged legacy index. + if (existsSync(db)) stageLegacyIndex(); await recordBoot('production', bootUntilReady(['serve', 'objectstack.config.ts', '-p', KERNEL_ASSIGNED_PORT], { OS_DATABASE_URL: 'file:A.db', OS_AUTH_SECRET: 'e2e-21733-production-leg-secret-not-for-real-use', @@ -243,16 +277,19 @@ afterAll(async () => { }); describe('#21733 — a plain `os dev` (standalone stack) self-heals and provisions telemetry', () => { - it('a fresh `os dev` creates the NULL-safe index, and OS_TELEMETRY_DB=0 provisions no sibling', () => { + it('a fresh `os dev` creates the plain organization composite, and OS_TELEMETRY_DB=0 provisions no sibling', () => { const fresh = recorded('fresh'); - expect(isNullSafe(fresh.indexSql), `fresh boot index: ${fresh.indexSql}`).toBe(true); + expect(isPlainComposite(fresh.indexSql), `fresh boot index: ${fresh.indexSql}`).toBe(true); + expect(fresh.legacySql, 'the fresh boot created the legacy global unique itself').toBeUndefined(); expect(fresh.telemetryExists, 'OS_TELEMETRY_DB=0 still created A.telemetry.db').toBe(false); }); - it('a plain `os dev` restart puts the staged bare unique index back NULL-safe', () => { + it('a plain `os dev` restart relaxes the staged legacy global unique to the organization composite', () => { const restart = recorded('restart'); - expect(isNullSafe(restart.indexSql), `the restart left the bare index: ${restart.indexSql}`).toBe(true); - expect(restart.output).toContain('[schema-drift] auto-reconciled recreate_index on scaf_item'); + expect(restart.legacySql, `the restart left the legacy index: ${restart.legacySql}`).toBeUndefined(); + // A relaxation, not a removal: the per-organization constraint survives. + expect(isPlainComposite(restart.indexSql), `the restart lost the composite: ${restart.indexSql}`).toBe(true); + expect(restart.output).toContain('[schema-drift] auto-reconciled replace_unique_index on scaf_item.qa_code'); }); it('the same restart provisions the `telemetry` sibling next to the file-backed primary', () => { @@ -262,8 +299,9 @@ describe('#21733 — a plain `os dev` (standalone stack) self-heals and provisio it('a production boot reports the staged drift and does not auto-migrate it', () => { const production = recorded('production'); - expect(production.output).toContain(`index '${INDEX}' tightens`); + expect(production.output).toContain(`a legacy platform-wide UNIQUE index (${LEGACY})`); expect(production.output).not.toContain('[schema-drift] auto-reconciled'); - expect(isNullSafe(production.indexSql), `production auto-migrated: ${production.indexSql}`).toBe(false); + expect(production.legacySql, 'production auto-migrated the legacy index away').toBeDefined(); + expect(isPlainComposite(production.indexSql)).toBe(true); }); }); diff --git a/packages/drivers/driver-memory/src/memory-declared-index-unique.test.ts b/packages/drivers/driver-memory/src/memory-declared-index-unique.test.ts index 8f3ed786c95..ce711a067c2 100644 --- a/packages/drivers/driver-memory/src/memory-declared-index-unique.test.ts +++ b/packages/drivers/driver-memory/src/memory-declared-index-unique.test.ts @@ -45,15 +45,18 @@ * ('acme', NULL, 'X') then ('acme', NULL, 'X') -> BOTH ACCEPTED (NULL-DISTINCT) * ('acme', 'A2', NULL) then ('acme', 'A2', NULL) -> BOTH ACCEPTED (NULL-DISTINCT) * - * UNIQUE (COALESCE(organization_id,'__global__'), account_id, code) -- 'organization' - * (NULL, 'A1', 'X') then (NULL, 'A1', 'X') -> second REFUSED (the org part FOLDS) - * (NULL, 'A2', NULL) then (NULL, 'A2', NULL) -> BOTH ACCEPTED (NULL-DISTINCT wins) + * UNIQUE (organization_id, account_id, code) -- 'organization' + * (NULL, 'A1', 'X') then (NULL, 'A1', 'X') -> BOTH ACCEPTED (NULL-DISTINCT) + * (NULL, 'A2', NULL) then (NULL, 'A2', NULL) -> BOTH ACCEPTED (NULL-DISTINCT) * ``` * * So the composite rule is the field-level rule with a wider key, and NOT a new - * invention: a NULL in any LISTED key column exempts the row, while a NULL - * ORGANIZATION folds onto one bucket because its key part is an expression that - * is never NULL. Both halves are pinned below. + * invention: a NULL in ANY key part exempts the row — the organization's + * included. Since ADR-0131 D1 the tenant column is NOT NULL and `driver-sql` + * keys it plain; until then ADR-0120 D3 keyed it as + * `COALESCE(organization_id,'__global__')`, which folded NULL organizations onto + * one bucket (the second `(NULL, 'A1', 'X')` above was REFUSED). That fold is + * retired on both surfaces, and the exemption is pinned below. */ import { describe, it, expect, beforeEach } from 'vitest'; @@ -177,9 +180,11 @@ describe('[#13239] the two `unique` surfaces disagree about bare `true`, and thi ); }); - it("only the explicit `'organization'` prepends the NULL-safe organization key part", async () => { + it("only the explicit `'organization'` prepends the organization key part — plain, ADR-0131 D1", async () => { + // `scopeColumns` names it for the refusal's wording; `nullSafeColumns` is + // empty — the organization key part no longer folds NULLs (ADR-0131 D1). expect(uniqueConstraintsFromDeclaredIndexes(ledger('organization'))).toEqual([ - { columns: ['organization_id', 'account_id', 'code'], nullSafeColumns: ['organization_id'] }, + { columns: ['organization_id', 'account_id', 'code'], nullSafeColumns: [], scopeColumns: ['organization_id'] }, ]); const driver = new InMemoryDriver(); @@ -232,16 +237,17 @@ describe('[#13239] the two `unique` surfaces disagree about bare `true`, and thi * ==================================================================== */ describe("[#13239] the arms are `normalizeDeclaredIndex`'s, reproduced without importing driver-sql", () => { - it('a listed column that IS the tenant column is not prepended again — its own key part becomes NULL-safe', () => { + it('a listed column that IS the tenant column is not prepended again — it stays a plain key part, named as the scope', () => { // The hand-written S6 spelling, opted in. Order is preserved: the author's - // column stays where they put it. + // column stays where they put it. Since ADR-0131 D1 its key part is plain, + // like the prepended one. const schema = { name: 'ledger', fields: { id: {}, organization_id: {}, code: {} }, indexes: [{ fields: ['code', 'organization_id'], unique: 'organization' }], }; expect(uniqueConstraintsFromDeclaredIndexes(schema)).toEqual([ - { columns: ['code', 'organization_id'], nullSafeColumns: ['organization_id'] }, + { columns: ['code', 'organization_id'], nullSafeColumns: [], scopeColumns: ['organization_id'] }, ]); }); @@ -276,7 +282,7 @@ describe("[#13239] the arms are `normalizeDeclaredIndex`'s, reproduced without i indexes: [{ fields: ['a'], unique: 'organization' }], }; expect(uniqueConstraintsFromDeclaredIndexes(schema)).toEqual([ - { columns: ['org', 'a'], nullSafeColumns: ['org'] }, + { columns: ['org', 'a'], nullSafeColumns: [], scopeColumns: ['org'] }, ]); }); @@ -337,7 +343,7 @@ describe("[#13239] the arms are `normalizeDeclaredIndex`'s, reproduced without i }; expect(uniqueConstraintsFromDeclaredIndexes(schema)).toEqual([ { columns: ['a'], nullSafeColumns: [] }, - { columns: ['organization_id', 'b', 'c'], nullSafeColumns: ['organization_id'] }, + { columns: ['organization_id', 'b', 'c'], nullSafeColumns: [], scopeColumns: ['organization_id'] }, ]); }); }); @@ -357,43 +363,46 @@ describe('[#13239] NULL handling matches the composite index SQL actually builds expect(await driver.count('ledger')).toBe(4); }); - it('a NULL ORGANIZATION does NOT exempt — the D3 fold, reached without the `__global__` token', async () => { - // SQL folds NULL organizations with COALESCE onto a reserved literal because - // an index EXPRESSION needs a non-NULL one. A JS key holds `null` directly, - // so the same bucket is reached with no token at all. + it('a NULL ORGANIZATION exempts the row, like any NULL key part — the D3 fold is retired (ADR-0131 D1)', async () => { + // The plain `(organization_id, …)` index SQL builds now is NULL-distinct in + // its organization part too, so rows without an organization (which only a + // pre-18 store can still hold) are not constrained among themselves. const driver = new InMemoryDriver(); await driver.syncSchema('ledger', ledger('organization')); await driver.create('ledger', { id: '1', account_id: 'A1', code: 'X' }); - const err = await refusalOf(() => driver.create('ledger', { id: '2', account_id: 'A1', code: 'X' })); + await driver.create('ledger', { id: '2', account_id: 'A1', code: 'X' }); + await driver.create('ledger', { id: '3', organization_id: null, account_id: 'A1', code: 'X' }); + // …while a row that DOES carry an organization is still constrained in it. + await driver.create('ledger', { id: '4', organization_id: 'acme', account_id: 'A1', code: 'X' }); + const err = await refusalOf(() => + driver.create('ledger', { id: '5', organization_id: 'acme', account_id: 'A1', code: 'X' }), + ); expectUniqueViolationEnvelope(err, 'account_id', 'code', 'organization_id'); - // …and a row that DOES carry an organization is untouched by that bucket. - await driver.create('ledger', { id: '3', organization_id: 'acme', account_id: 'A1', code: 'X' }); - expect(await driver.count('ledger')).toBe(2); + expect(await driver.count('ledger')).toBe(4); }); - it('an index whose ONLY key part is the NULL-safe organization is "one row per organization"', async () => { + it('an index whose ONLY key part is the organization is "one row per organization" — NULL-org rows exempt', async () => { // `normalizeDeclaredIndex` has no "a unique ON the tenant column stays // single-column" guard — that guard is the FIELD surface's. Here the listed - // organization column simply becomes the NULL-safe key part, so NULL-org - // rows share one bucket and are unique among themselves. + // organization column is the whole key, and plain since ADR-0131 D1: one + // row per organization, while rows without one are not constrained. const schema = { name: 'settings', fields: { id: {}, organization_id: {} }, indexes: [{ fields: ['organization_id'], unique: 'organization' }], }; expect(uniqueConstraintsFromDeclaredIndexes(schema)).toEqual([ - { columns: ['organization_id'], nullSafeColumns: ['organization_id'] }, + { columns: ['organization_id'], nullSafeColumns: [], scopeColumns: ['organization_id'] }, ]); const driver = new InMemoryDriver(); await driver.syncSchema('settings', schema); await driver.create('settings', { id: '1', organization_id: 'acme' }); - await driver.create('settings', { id: '2' }); // NULL organization: its own bucket + await driver.create('settings', { id: '2' }); // NULL organization: exempt const dupOrg = await refusalOf(() => driver.create('settings', { id: '3', organization_id: 'acme' })); expectUniqueViolationEnvelope(dupOrg, 'organization_id'); - const dupNull = await refusalOf(() => driver.create('settings', { id: '4' })); - expectUniqueViolationEnvelope(dupNull, 'organization_id'); - expect(await driver.count('settings')).toBe(2); + await driver.create('settings', { id: '4' }); // a second NULL-org row lands too + expect(await driver.count('settings')).toBe(3); }); }); @@ -411,7 +420,7 @@ describe('[#13239] the refusal names no single column, because a composite has n ); // Matches what SQLite answers for the same index — measured: the plain // composite prints `UNIQUE constraint failed: t.account_id, t.code` (two - // targets -> undefined) and the NULL-safe form prints + // targets -> undefined) and the 18.0 NULL-safe form printed // `UNIQUE constraint failed: index '…'` (an index name -> undefined). expect(isUniqueViolationError(err)).toBe(true); expect(uniqueViolationColumn(err)).toBeUndefined(); diff --git a/packages/drivers/driver-memory/src/memory-unique-constraint.test.ts b/packages/drivers/driver-memory/src/memory-unique-constraint.test.ts index 243444b8f63..9dd3b67c4fd 100644 --- a/packages/drivers/driver-memory/src/memory-unique-constraint.test.ts +++ b/packages/drivers/driver-memory/src/memory-unique-constraint.test.ts @@ -28,7 +28,9 @@ * 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` + * when the object has no tenant column. Since ADR-0131 D1 the organization key + * part is the PLAIN column there, so a row with no organization is exempt like a + * NULL in any key part. This package cannot import `driver-sql` * (dependency direction), so the arms are reproduced and pinned here against * that rule rather than shared. */ @@ -285,20 +287,36 @@ describe("[#13197] scoping matches driver-sql's ADR-0120 D1/D3 rule", () => { 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. + it('a row with NO organization is exempt — the plain organization key part (ADR-0131 D1), the D3 bucket retired', async () => { + // ADR-0120 D3 once folded NULL-org rows onto one bucket (COALESCE onto + // `__global__` in SQL). The tenant column is NOT NULL since ADR-0131 D1, so + // driver-sql keys `(organization_id, email)` plain and SQL's NULL-distinct + // UNIQUE exempts such rows (which only a pre-18 store can still hold) — + // this driver answers the same. const driver = new InMemoryDriver(); await driver.syncSchema('contact', withOrg(true)); await driver.create('contact', { id: '1', email: 'a@b.com' }); + await driver.create('contact', { id: '2', email: 'a@b.com' }); + await driver.create('contact', { id: '3', organization_id: null, email: 'a@b.com' }); + // …while a row that DOES carry an organization is still constrained in it. + await driver.create('contact', { id: '4', organization_id: 'acme', email: 'a@b.com' }); + const err = await refusalOf(() => + driver.create('contact', { id: '5', organization_id: 'acme', email: 'a@b.com' }), + ); + expectUniqueViolationEnvelope(err, 'email'); + expect(await driver.count('contact')).toBe(4); + }); + + it("a platform-wide constraint's constant scope still constrains rows with no organization", async () => { + // The one NULL-safe part left: `scopeField: null` is a constant, not a column, + // so `'global'` keeps refusing a second NULL-org row (ADR-0131 D1 changes + // only the organization key part). + const driver = new InMemoryDriver(); + await driver.syncSchema('contact', withOrg('global')); + 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); + expect(await driver.count('contact')).toBe(1); }); it('with NO tenant column both per-organization spellings degrade to a single-column constraint', () => { diff --git a/packages/drivers/driver-memory/src/memory-unique-constraint.ts b/packages/drivers/driver-memory/src/memory-unique-constraint.ts index eb0eb11358e..102b36b10a4 100644 --- a/packages/drivers/driver-memory/src/memory-unique-constraint.ts +++ b/packages/drivers/driver-memory/src/memory-unique-constraint.ts @@ -39,7 +39,7 @@ * | 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'` | present, ≠ field | `(tenant, 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 | @@ -61,8 +61,8 @@ * | `fields` empty / absent / no non-empty strings | any | `null` — unusable | no constraint | * | `unique` absent / `false` | any | a PLAIN index | not a constraint | * | `unique: true` / `'global'` | any | the listed columns VERBATIM | `columns` = listed, no NULL-safe part | - * | `unique: 'organization'`, tenant column NOT listed | present | `(COALESCE(tenant,'__global__'), …listed)` | `columns` = `[tenant, …listed]`, `nullSafeColumns` = `[tenant]` | - * | `unique: 'organization'`, tenant column ALREADY listed | present | listed verbatim, the tenant's own key part goes NULL-safe | `columns` = listed (order kept), `nullSafeColumns` = `[tenant]` | + * | `unique: 'organization'`, tenant column NOT listed | present | `(tenant, …listed)` | `columns` = `[tenant, …listed]`, `scopeColumns` = `[tenant]` | + * | `unique: 'organization'`, tenant column ALREADY listed | present | listed verbatim | `columns` = listed (order kept), `scopeColumns` = `[tenant]` | * | `unique: 'organization'` | absent | degrades to the listed columns alone | `columns` = listed, no NULL-safe part | * * ⚠️ **bare `true` inverts between the two surfaces.** On a declared index it @@ -105,9 +105,16 @@ * NULL key part exempts the row (below), so such an index constrains * nothing. The field surface has the same exposure and the same answer. * - * ### Why the NULL-organization fold needs no `'__global__'` token here + * ### The organization key part is plain since ADR-0131 D1 * - * ADR-0120 D3 materializes the organization key part as + * The tenant column is NOT NULL (ADR-0131 D1), so `driver-sql` keys the plain + * column and a row whose organization is NULL is exempt under its + * NULL-distinct UNIQUE, like a NULL in any other key part. This module answers + * the same: the organization key part is an ordinary one. The fold the next + * paragraphs describe is what ADR-0120 D3 did before, kept for the record of + * why `nullSafeColumns` exists on the constraint shape. + * + * ADR-0120 D3 materialized 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 @@ -148,9 +155,9 @@ * * So there is exactly ONE rule, and {@link uniqueKeyOf} is the one place it * lives: **a NULL in any key part EXEMPTS the row, except in a NULL-SAFE part, - * where it folds onto the shared `null` bucket.** The organization key part is - * the only NULL-safe one, on either surface, because on the SQL side it is the - * only one materialized as an expression that can never be NULL. + * where it folds onto the shared `null` bucket.** Since ADR-0131 D1 no + * organization key part is NULL-safe; the one NULL-safe part left is the + * constant platform scope of a field-level constraint with no tenant column. * * ## Which column conflicted: this driver answers `undefined`, always * @@ -244,10 +251,15 @@ export interface MemoryDeclaredIndexConstraint { readonly columns: readonly string[]; /** * The subset of {@link columns} whose NULL FOLDS onto one shared bucket - * instead of exempting the row (ADR-0120 D3). In practice the organization - * key part, and only on the `'organization'` spelling. + * instead of exempting the row (ADR-0120 D3). Empty from every declaration + * since ADR-0131 D1, which keys the plain organization column. */ readonly nullSafeColumns: readonly string[]; + /** + * The organization key part the `'organization'` spelling prepended or + * named — what the refusal states as the scope. Absent otherwise. + */ + readonly scopeColumns?: readonly string[]; } /** @@ -459,11 +471,11 @@ export function uniqueConstraintsFromDeclaredIndexes( if (!isUniqueDeclared(idx?.unique)) continue; if (isOrganizationUnique(idx?.unique) && tenantField) { - // A listed column that IS the tenant column is not prepended again — its - // own key part becomes the NULL-safe one instead (the hand-written S6 - // spelling, opted in), and the author's column order is kept. + // A listed column that IS the tenant column is not prepended again (the + // hand-written S6 spelling), and the author's column order is kept. The + // organization key part is plain (ADR-0131 D1). const columns = listed.includes(tenantField) ? listed : [tenantField, ...listed]; - out.push({ columns, nullSafeColumns: [tenantField] }); + out.push({ columns, nullSafeColumns: [], scopeColumns: [tenantField] }); continue; } // `'global'`, bare `true`, or `'organization'` on an object with no tenant @@ -491,8 +503,8 @@ export function uniqueKeyOf( const parts: unknown[] = []; for (const part of keyPartsOf(constraint)) { // A NULL-SAFE part folds: `null` IS its bucket, and the row stays - // constrained. That is the ADR-0120 D3 organization key part, and the only - // kind of part that behaves this way on either surface. + // constrained: the constant platform scope of a field-level constraint. + // The organization key part is plain (ADR-0131 D1). if (part.nullSafe) { parts.push(part.column === null ? null : (record[part.column] ?? null)); continue; @@ -530,7 +542,9 @@ function keyPartsOf(constraint: MemoryUniqueEnforcement): readonly UniqueKeyPart })); } return [ - { column: constraint.scopeField, nullSafe: true }, + // The constant platform scope (`null`) folds; an organization column is a + // plain key part (ADR-0131 D1), so a row with no organization is exempt. + { column: constraint.scopeField, nullSafe: constraint.scopeField === null }, { column: constraint.field, nullSafe: false }, ]; } @@ -574,10 +588,10 @@ export function declaredIndexViolationError( constraint: MemoryDeclaredIndexConstraint, record: Record, ): Error & { code: string; status: number } { - const scope = constraint.nullSafeColumns; + const scope = constraint.scopeColumns ?? constraint.nullSafeColumns; const keyed = constraint.columns.filter((c) => !scope.includes(c)); - // A key whose ONLY part is the NULL-safe organization ("one row per - // organization") has nothing left to list, so it reports its own column. + // A key whose ONLY part is the organization ("one row per organization") + // has nothing left to list, so it reports its own column. const reported = keyed.length > 0 ? keyed : constraint.columns; const scoped = keyed.length > 0 && scope.length > 0 diff --git a/packages/drivers/driver-sql/src/adr0120-three-posture-conformance.test.ts b/packages/drivers/driver-sql/src/adr0120-three-posture-conformance.test.ts index 152ad99f753..d4772c26148 100644 --- a/packages/drivers/driver-sql/src/adr0120-three-posture-conformance.test.ts +++ b/packages/drivers/driver-sql/src/adr0120-three-posture-conformance.test.ts @@ -69,7 +69,8 @@ const FIXTURE_APP = [ { // S1 — per-organization unique field. Two organizations may each hold // `a@b.com`; a duplicate WITHIN one may not. S8's mixed population rides - // along: NULL-organization rows form one platform bucket (D3). + // along: NULL-organization rows (pre-18 databases only) are exempt from + // the plain organization key part (ADR-0131 D1). name: 'crm_contact', fields: { organization_id: { type: 'string' }, @@ -152,22 +153,23 @@ const FIXTURE_APP = [ /** * The expected unique key parts, per table — the SAME map for every posture. - * `COALESCE(col)` is the literal-agnostic reading of the NULL-safe organization - * key part, matching the identity the drift differ compares on. + * The organization key part is the PLAIN column (ADR-0131 D1): a legacy + * NULL-safe part would read here as `COALESCE(col)` (literal elided, the + * differ's identity) and fail the map, so no posture can bring one back. */ const EXPECTED_UNIQUE_KEY_PARTS: Record> = { - crm_contact: { uniq_crm_contact_organization_id_email: ['COALESCE(organization_id)', 'email'] }, + crm_contact: { uniq_crm_contact_organization_id_email: ['organization_id', 'email'] }, crm_device: { uniq_crm_device_serial: ['serial'] }, // Note the index NAME: the deterministic name is derived from the RESOLVED // columns, so a declared `'organization'` index carries the organization // column in its name even though the author never listed it. That is the // differ's identity too, which is why declaring and detecting cannot drift. crm_case: { - uniq_crm_case_organization_id_department_code: ['COALESCE(organization_id)', 'department', 'code'], + uniq_crm_case_organization_id_department_code: ['organization_id', 'department', 'code'], }, crm_delivery: { uniq_crm_delivery_source_dedup_key: ['source', 'dedup_key'] }, crm_team: { uniq_crm_team_name_organization_id: ['name', 'organization_id'] }, - crm_product: { uniq_crm_product_organization_id_code: ['COALESCE(organization_id)', 'code'] }, + crm_product: { uniq_crm_product_organization_id_code: ['organization_id', 'code'] }, crm_registry: { uniq_crm_registry_slug: ['slug'] }, }; @@ -279,21 +281,29 @@ describe('ADR-0120 — one app package, three tenancy postures (S13/S14 acceptan .rejects.toThrow(REJECTED); }); - it('S7/S8 — the NULL-organization bucket is a real bucket, in every posture', async () => { - // THE #5030 headline, and the reason `'organization'` could ship at all: - // before D3 this pair inserted twice on a single-organization stack - // (SQL UNIQUE is NULL-distinct), so every field-level unique was a - // silent no-op there. It must now be enforced under `single` exactly as - // under `isolated` — the posture is not what makes a constraint real. - // The NULL bucket exists only on a database built before 18.x (ADR-0131 - // D1 makes a fresh tenant column NOT NULL). + it('S7/S8 — NULL organization: refused on a fresh table, exempt on a pre-18 one, in every posture', async () => { + // THE #5030 headline: before ADR-0120 D3 a NULL-organization pair + // inserted twice on a single-organization stack (SQL UNIQUE is + // NULL-distinct), so every field-level unique was a silent no-op there. + // ADR-0131 D1 closes that at the COLUMN, not the index: a fresh tenant + // column is NOT NULL, so the row cannot be written at all — under + // `single` exactly as under `isolated`; the posture is not what makes a + // constraint real. + await expect(driver.create('crm_contact', { email: 'platform@b.com' })) + .rejects.toThrow(/NOT NULL constraint failed|null value in column/); + // A database built before 18.x keeps its nullable column; there the + // plain key part is NULL-distinct, so NULL-organization rows are exempt + // (the D3 platform bucket is retired, not rebuilt)… await asPre18TenantColumn(driver, 'crm_contact'); await driver.create('crm_contact', { email: 'platform@b.com' }); await expect(driver.create('crm_contact', { email: 'platform@b.com' })) - .rejects.toThrow(REJECTED); - // …while an organization-owned row with the same value still fits (S8). + .resolves.toBeTruthy(); + // …while an organization-owned row with the same value still fits (S8), + // once — the organization stays constrained. await expect(driver.create('crm_contact', { organization_id: 'org_a', email: 'platform@b.com' })) .resolves.toBeTruthy(); + await expect(driver.create('crm_contact', { organization_id: 'org_a', email: 'platform@b.com' })) + .rejects.toThrow(REJECTED); }); it('S2 — a platform-wide unique field crosses organizations, in every posture', async () => { @@ -308,12 +318,15 @@ describe('ADR-0120 — one app package, three tenancy postures (S13/S14 acceptan .resolves.toBeTruthy(); await expect(driver.create('crm_case', { organization_id: 'org_a', department: 'ops', code: 'C1' })) .rejects.toThrow(REJECTED); - // …and the NULL bucket is constrained too (D3 reaches declared indexes). - // The NULL bucket exists only on a database built before 18.x (ADR-0131 - // D1 makes a fresh tenant column NOT NULL). + // …and NULL-organization rows, which exist only on a database built + // before 18.x, are exempt from the declared composite exactly as from + // the field-level one: its organization key part is plain (ADR-0131 D1). await asPre18TenantColumn(driver, 'crm_case'); await driver.create('crm_case', { department: 'ops', code: 'C9' }); await expect(driver.create('crm_case', { department: 'ops', code: 'C9' })) + .resolves.toBeTruthy(); + // The relaxation left the composite in force for organizations. + await expect(driver.create('crm_case', { organization_id: 'org_a', department: 'ops', code: 'C1' })) .rejects.toThrow(REJECTED); }); @@ -340,10 +353,11 @@ describe('ADR-0120 — one app package, three tenancy postures (S13/S14 acceptan .rejects.toThrow(REJECTED); await expect(driver.create('crm_team', { organization_id: 'org_b', name: 'Core' })) .resolves.toBeTruthy(); - // The un-closed NULL hole, pinned as the honest status quo (D5c nudges - // toward the respelling that closes it; nothing forces it). - // The NULL bucket exists only on a database built before 18.x (ADR-0131 - // D1 makes a fresh tenant column NOT NULL). + // NULL-organization rows are exempt, pinned as the honest status quo. + // Since ADR-0131 D1 the respelling D5c nudges toward is plain too, so + // this is no longer a hole one spelling has and the other closes: such + // rows exist only on a database built before 18.x (a fresh tenant + // column is NOT NULL). await asPre18TenantColumn(driver, 'crm_team'); await driver.create('crm_team', { name: 'Platform' }); await expect(driver.create('crm_team', { name: 'Platform' })).resolves.toBeTruthy(); diff --git a/packages/drivers/driver-sql/src/schema-drift.ts b/packages/drivers/driver-sql/src/schema-drift.ts index 7aed35b83a2..85bfef95b50 100644 --- a/packages/drivers/driver-sql/src/schema-drift.ts +++ b/packages/drivers/driver-sql/src/schema-drift.ts @@ -63,6 +63,12 @@ import type { SchemaDiffEntry } from '@objectstack/spec/shared'; * `(__global__, a@b.com)` says "platform bucket", not "corrupt data". * - **The token is reserved**: an organization id may never equal it * (guarded at the organization-creation seam in plugin-auth). + * + * Since ADR-0131 D1 the tenant column is NOT NULL, so new DDL keys the plain + * column and no longer emits this form. The COALESCE form stays READ: + * every database an earlier release built keeps those indexes, and the + * differ takes them as equivalent to the plain key on a certified database + * ({@link diffManagedIndexes}'s `tenantKeyCertified`). */ export const GLOBAL_TENANT = '__global__'; @@ -113,9 +119,12 @@ export function isOrganizationScopedUnique(unique: unknown): boolean { } /** - * The organization key part of an organization-scoped unique index, spelled - * once (ADR-0120 D3). Display/signature form — DDL emission quotes the - * identifier per dialect in `SqlDriver.syncDeclaredIndexes`. + * The NULL-safe organization key part, spelled once (ADR-0120 D3). Display and + * signature form — DDL emission quotes the identifier per dialect. + * + * Read vocabulary only since ADR-0131 D1: no generator marks the tenant key + * part NULL-safe any more, so this renders an index an earlier release built, + * or an op a caller hands in already normalized. */ export function organizationKeyPartSql(column: string): string { return `COALESCE(${column}, '${GLOBAL_TENANT}')`; @@ -1829,13 +1838,14 @@ export function isRuntimeManagedIndex( * Scoping rule (ADR-0120 D1/D3): * - `unique: 'global'` → single-column `(field)`, platform-wide. * - `unique: true` / `unique: 'organization'` on a tenant-scoped table → - * composite `(COALESCE(tenantField, '__global__'), field)`: unique - * *within* the organization, matching the per-tenant autonumber sequence, - * the RLS read predicate and the write-path tenant stamp. The organization - * key part is NULL-safe: rows without an organization form one platform - * bucket, unique among themselves — a bare `(tenantField, field)` under - * SQL's NULL-distinct UNIQUE enforced NOTHING on those rows, which on a - * single-tenant stack is every row (#5030). + * composite `(tenantField, field)`: unique *within* the organization, + * matching the per-tenant autonumber sequence, the RLS read predicate and + * the write-path tenant stamp. The organization key part is the plain + * column (ADR-0131 D1: the tenant column is NOT NULL, so no row escapes + * the NULL-distinct UNIQUE). ADR-0120 D3's `COALESCE(tenantField, + * '__global__')` form, which folded NULL-organization rows into one bucket + * (#5030), is what earlier releases built; the differ reads it as + * equivalent on a certified database. * - `unique: true` / `'organization'` with no tenant column → single-column * `(field)`. * @@ -1856,12 +1866,7 @@ export function uniqueIndexesFromFields( const scoped = isOrganizationScopedUnique(field.unique) && tenantField != null && tenantField !== name; const columns = scoped ? [tenantField, name] : [name]; - out.push({ - name: buildIndexName(table, columns, true), - columns, - unique: true, - ...(scoped ? { nullSafeColumns: [tenantField] } : {}), - }); + out.push({ name: buildIndexName(table, columns, true), columns, unique: true }); } return out; } @@ -1883,13 +1888,14 @@ export interface DeclaredIndexInput { * - `unique: true` / `'global'` → the listed columns, VERBATIM — the #3696 * contract, now the `'global'` arm of the explicit vocabulary. * - `unique: 'organization'` → the organization key part is PREPENDED to the - * listed columns in its NULL-safe form (`COALESCE(tenantField, - * '__global__')`), resolved against the table's tenant column at - * registration — the one place tenancy is knowable. With no tenant column - * the index degrades to the listed columns alone, mirroring field-level - * behavior (S11). A listed column that IS the tenant column is not - * prepended again — its own key part becomes the NULL-safe form instead - * (the hand-written S6 spelling, opted in). + * listed columns as the plain tenant column (ADR-0131 D1), resolved + * against the table's tenant column at registration — the one place + * tenancy is knowable. With no tenant column the index degrades to the + * listed columns alone, mirroring field-level behavior (S11). A listed + * column that IS the tenant column is not prepended again (the + * hand-written S6 spelling). + * - An input already carrying `nullSafeColumns` (an op normalized before + * ADR-0131 D1) is honoured verbatim. */ export function normalizeDeclaredIndex( table: string, @@ -1910,7 +1916,6 @@ export function normalizeDeclaredIndex( if (nullSafeColumns.length === 0) nullSafeColumns = undefined; } else if (idx?.unique === 'organization' && tenantField) { columns = listed.includes(tenantField) ? listed : [tenantField, ...listed]; - nullSafeColumns = [tenantField]; } const name = @@ -2196,16 +2201,11 @@ export function legacyUniqueReplacements(args: { column: name, legacyColumns: [name], legacyNames, - // NULL-safe organization key part (ADR-0120 D3). Still a pure - // relaxation to create from under the legacy GLOBAL single-column - // unique: any two rows colliding in the new key already collided in the - // old one, so the replacement can neither fail nor lose data. - replacement: { - name: buildIndexName(table, columns, true), - columns, - unique: true, - nullSafeColumns: [tenantField], - }, + // A pure relaxation to create from under the legacy GLOBAL + // single-column unique: any two rows colliding in the new key already + // collided in the old one, so the replacement can neither fail nor lose + // data. + replacement: { name: buildIndexName(table, columns, true), columns, unique: true }, }); } @@ -2214,8 +2214,8 @@ export function legacyUniqueReplacements(args: { // #8323: the same retirement, one level up. A declared index's bare // `unique: true` is the positional spelling of `'global'` — the listed // columns VERBATIM — so respelling it `'organization'` changes the - // materialized shape from `(…listed)` to `(COALESCE(tenant,'__global__'), - // …listed)`, and with it the generated NAME. On a deployed database that + // materialized shape from `(…listed)` to `(tenant, …listed)`, and with it + // the generated NAME. On a deployed database that // reads as two unrelated findings: the composite is missing (create, safe) // and the old global index is an orphan (drop, DESTRUCTIVE, opt-in). An // operator who applies only the safe half keeps the global index — and the @@ -2248,8 +2248,7 @@ export function legacyUniqueReplacements(args: { const legacyName = buildIndexName(table, listed, true); // The S6 hand-written composite already lists the tenant column, so // `normalizeDeclaredIndex` prepends nothing and the "legacy" name IS the - // current name. Nothing was superseded; the D4 NULL-safe tightening path - // owns that transition. + // current name. Nothing was superseded. if (legacyName === replacement.name) continue; // An index the CURRENT metadata declares is by definition not legacy // (#3955) — the same guard the field-level arm applies. @@ -2356,6 +2355,19 @@ export function canonicalIndexKey(columns: string[], nullSafeColumns?: ReadonlyA return columns.map((c) => (ns.has(c) ? `coalesce:${c}` : c)).join(','); } +/** + * [ADR-0131 D1] Does any physical index key the tenant column through the + * NULL-safe `COALESCE` form — directly, or through a #11627 hash shadow's + * resolved key? Those are the indexes an earlier release built, and the only + * ones whose verdict {@link diffManagedIndexes}'s `tenantKeyCertified` can + * change, so a caller asks the (costly) certification question only when this + * answers yes. + */ +export function carriesNullSafeTenantKeyPart(physical: PhysicalIndex[], tenantField?: string | null): boolean { + if (!tenantField) return false; + return physical.some((p) => (enforcedIndexKey(p).nullSafeColumns ?? []).includes(tenantField)); +} + /** * Diff a table's expected index set against the physical one. * @@ -2381,8 +2393,29 @@ export function diffManagedIndexes(args: { * (ADR-0120 D3) — see {@link isSyncReproducibleIndex}. */ tenantField?: string | null; + /** + * [ADR-0131 D1] Does the tenant column hold no NULL on this table — because + * the column is physically NOT NULL, or because the database is certified + * (a birth marker or an organization-ownership ceremony marker of the + * current shape generation)? Then `COALESCE(, …)` and the plain + * tenant column enforce the same constraint, and a key part in either form + * matches the other, in both directions: an index an earlier release built + * in the NULL-safe form is never proposed for a rebuild into the plain form + * new DDL emits. Absent or false on a database that may still hold + * NULL-organization rows, where the two forms are different constraints. + */ + tenantKeyCertified?: boolean; }): ManagedDriftEntry[] { - const { table, expected, legacy, physical, runtimeCreated, tenantField } = args; + const { table, expected, legacy, physical, runtimeCreated, tenantField, tenantKeyCertified } = args; + // The comparison identity of a key: the NULL-safe tenant key part reads as + // the plain column where the two are the same constraint (ADR-0131 D1). + const comparisonKey = (columns: string[], nullSafeColumns?: ReadonlyArray | null): string => + canonicalIndexKey( + columns, + tenantKeyCertified === true && tenantField + ? (nullSafeColumns ?? []).filter((c) => c !== tenantField) + : nullSafeColumns, + ); const out: ManagedDriftEntry[] = []; const byName = new Map(physical.map((p) => [p.name, p])); /** Physical index names accounted for — either declared, or already reported. */ @@ -2475,12 +2508,10 @@ export function diffManagedIndexes(args: { // Same normalization on BOTH sides (#4884, ADR-0120 D3): column identity // AND key-part form, literal-agnostic on the COALESCE literal — asked of // the key the index ENFORCES, which for a #11627 shadow-carried UNIQUE is - // not the column it stores (commit cd1348802). + // not the column it stores (commit cd1348802). On a certified table the + // tenant key part's form drops out of the identity (ADR-0131 D1). const pk = enforcedIndexKey(p); - if ( - p.unique === e.unique && - canonicalIndexKey(pk.columns, pk.nullSafeColumns) === canonicalIndexKey(e.columns, e.nullSafeColumns) - ) { + if (p.unique === e.unique && comparisonKey(pk.columns, pk.nullSafeColumns) === comparisonKey(e.columns, e.nullSafeColumns)) { continue; } // The framework's own runtime migrations own some declared names — ADR-0048 @@ -2526,6 +2557,22 @@ export function diffManagedIndexes(args: { // own (`nullSafeColumns` is only ever recorded alongside an expression key // part); a resolved shadow key can carry NULL-safe parts with no // expressions at all, so the implication no longer holds. + // [ADR-0131 D1] An earlier release's COALESCE tenant key part against the + // plain one new DDL declares — the divergence is ONLY that form, on a + // table where the two are still different constraints (not + // certified, nullable tenant column). The remedy is the ownership ceremony, + // which certifies the database and so retires this entry — never a rebuild: + // `--allow-destructive` would drop and re-key the index over rows that may + // still lack an organization. The op is unchanged (destructive, reported, + // never applied unattended); only what it tells the operator differs. + const uncertifiedTenantKeyOnly = + tenantKeyCertified !== true && + !!tenantField && + p.unique === e.unique && + (pk.nullSafeColumns ?? []).includes(tenantField) && + !(e.nullSafeColumns ?? []).includes(tenantField) && + canonicalIndexKey(pk.columns, (pk.nullSafeColumns ?? []).filter((c) => c !== tenantField)) === + canonicalIndexKey(e.columns, (e.nullSafeColumns ?? []).filter((c) => c !== tenantField)); const tightenNullSafeOnly = e.unique && p.unique && @@ -2553,7 +2600,15 @@ export function diffManagedIndexes(args: { ...(e.nullSafeColumns ? { nullSafeColumns: e.nullSafeColumns } : {}), ...(tightenNullSafeOnly ? { tightenNullSafeOnly: true } : {}), }, - message: tightenNullSafeOnly + message: uncertifiedTenantKeyOnly + ? `${table}: index '${e.name}' is ${indexSignature(pk.columns, p.unique, pk.nullSafeColumns)} and metadata declares ` + + `${indexSignature(e.columns, e.unique, e.nullSafeColumns)}: the same columns, differing only in how the ` + + `'${tenantField}' key part treats a row with no organization. This database is not certified for the ` + + `organization-ownership shape and '${tenantField}' is nullable, so the two are still different constraints. ` + + `Run "os migrate organization-ownership": once it certifies the database the two read as one index and this ` + + `entry is gone. Do not rebuild it with "os migrate apply --allow-destructive", which would drop and re-key ` + + `the index over rows that may still have no organization.` + : tightenNullSafeOnly ? `${table}: index '${e.name}' is ${indexSignature(pk.columns, p.unique, pk.nullSafeColumns)} but metadata declares ` + `${indexSignature(e.columns, e.unique, e.nullSafeColumns)} (ADR-0120 D3: the organization key part is NULL-safe, ` + `so rows without an organization are constrained too). Pure tightening — eligibility is decided by the ` + diff --git a/packages/drivers/driver-sql/src/sql-driver-14902-plain-unique-duplicate-preflight.test.ts b/packages/drivers/driver-sql/src/sql-driver-14902-plain-unique-duplicate-preflight.test.ts index 925f3451a03..ab78a9f0a67 100644 --- a/packages/drivers/driver-sql/src/sql-driver-14902-plain-unique-duplicate-preflight.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-14902-plain-unique-duplicate-preflight.test.ts @@ -38,12 +38,14 @@ import { SqlDriver } from '../src/index.js'; * * ## What this suite pins * - * Parity, in both halves, plus path A as the CONTROL. A change that quietly - * moved path A while making path B loud would otherwise read as success, so the - * last block asserts path A's message and classification are still their own — - * the `#5030` framing, which is precisely what the plain path must NOT claim - * (nothing ever admitted these rows; the constraint is simply newly declared - * over data that does not satisfy it). + * Parity, in both halves, plus the tenanted composite as the CONTROL. Path A + * as measured no longer exists: since ADR-0131 D1 the organization key part is + * PLAIN (no COALESCE), so a tenanted `unique: true` is path B's shape with the + * organization column in front. The last block pins that it reaches path B's + * disposition — loud, `destructive`, the conflicting groups named — and that + * it does NOT carry path A's retired `#5030` framing: no NULL-safe story, no + * `'__global__'` bucket, and NULL-organization rows (which never conflict under + * a plain UNIQUE) are not reported as duplicates. * * ⛔ Nothing here pins the DATABASE's raw error text: the `catch` shape is * dialect-independent, that text is not. Assertions are on our own messages and @@ -272,8 +274,8 @@ describe('#14902 plain unique index over duplicate rows', () => { }); }); - describe('CONTROL — path A is untouched', () => { - it('still logs its own NULL-safe #5030 message and still reports destructive with the key groups', async () => { + describe('CONTROL — the tenanted composite, plain since ADR-0131 D1', () => { + it('reaches the plain disposition with the organization key part, and never reports NULL-organization rows', async () => { await seed(true, [ { id: 'r1', organization_id: null, quote_number: 'QUO-00009' }, { id: 'r2', organization_id: null, quote_number: 'QUO-00009' }, @@ -282,7 +284,7 @@ describe('#14902 plain unique index over duplicate rows', () => { ]); // `unique: true` on a tenanted object == `unique: 'organization'` — the - // #13894 default, and the shape this card must not have disturbed. + // #13894 default. Its key is `(organization_id, quote_number)`, plain. await expect( driver.initObjects([ { @@ -297,20 +299,28 @@ describe('#14902 plain unique index over duplicate rows', () => { const durability = logs.filter((l) => l.level === 'error'); expect(durability).toHaveLength(1); - expect(durability[0].msg).toContain('NULL-safe unique index'); - expect(durability[0].msg).toContain('NULL-distinct'); - expect(durability[0].msg).toContain('ADR-0120 D4'); - expect(durability[0].msg).toContain("'organization_id, quote_number'"); + const msg = durability[0].msg; + expect(msg).toContain("'uniq_crm_quote_organization_id_quote_number'"); + expect(msg).toContain("'organization_id, quote_number'"); + expect(msg).toMatch(/Conflicting group\(s\)/); + expect(msg).toContain('(organization_id="org_x", quote_number="QUO-00010") × 2 rows'); + expect(msg).toMatch(/NOT enforced/); + expect(msg).toContain('os migrate plan'); + // The two NULL-organization rows never conflict under a plain UNIQUE + // (NULL-distinct), so the pre-flight does not report them — and the + // retired ADR-0120 D3/D4 framing is gone with the COALESCE key. + expect(msg).not.toContain('QUO-00009'); + expect(msg).not.toContain('NULL-safe'); + expect(msg).not.toContain('__global__'); const entry = (await driver.detectManagedDrift()).find((d) => d.op.type === 'create_index')!; expect(entry.category).toBe('destructive'); expect(entry.severity).toBe('error'); - expect(entry.message).toContain('NULL-distinct'); - expect(entry.message).toContain('__global__'); - // Both key groups, with counts — the NULL-organization bucket and a real - // organization, which is what makes the COALESCE key self-describing. - expect(entry.message).toContain('(organization_id="__global__", quote_number="QUO-00009")'); + expect(entry.message).toMatch(/BLOCKED/); expect(entry.message).toContain('(organization_id="org_x", quote_number="QUO-00010")'); + expect(entry.message).not.toContain('QUO-00009'); + expect(entry.message).not.toContain('__global__'); + expect(entry.message).not.toContain('NULL-safe'); }); }); }); diff --git a/packages/drivers/driver-sql/src/sql-driver-8577-tenant-scoped-declared-unique.test.ts b/packages/drivers/driver-sql/src/sql-driver-8577-tenant-scoped-declared-unique.test.ts index 449ce0fcc1a..9e6878d7e80 100644 --- a/packages/drivers/driver-sql/src/sql-driver-8577-tenant-scoped-declared-unique.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-8577-tenant-scoped-declared-unique.test.ts @@ -74,6 +74,16 @@ import { asPre18TenantColumn } from './pre-18-tenant-column.testkit.js'; * the safe half keeps the defect while the plan reads as applied. Section 3 is * therefore the load-bearing part of this file: it builds installations that * ALREADY HAVE the old index and real rows, then migrates them. + * + * ## The organization key part is PLAIN (ADR-0131 D1) + * + * The replacement once keyed `COALESCE(organization_id, '')` — ADR-0120 D3's + * NULL-safe part, which made the organization-less rows one more bucket that + * refused duplicates among themselves. Since 18.x a fresh tenant column is NOT + * NULL and the driver keys the PLAIN column: `(organization_id, …listed)`. On a + * pre-18 database whose tenant column is still nullable, rows with no + * organization are not constrained at all — SQL UNIQUE is NULL-distinct — and + * this file pins that exemption rather than the retired bucket. */ /** The wire shape a duplicate insert must produce, per `rest-server.ts`. */ @@ -291,12 +301,14 @@ describe('#8577 — two more tenant-scoped declared unique indexes become per-or // 1. Materialized shape — both spellings, kept side by side // ─────────────────────────────────────────────────────────────────── - it('the fixed declaration keys on the NULL-safe organization part', async () => { + it('the fixed declaration keys on the PLAIN organization part (ADR-0131 D1)', async () => { + // No COALESCE: a fresh tenant column is NOT NULL, so the plain column + // is already a total key part. const d = makeDriver(); await d.initObjects(app(sub, 'fixed') as any); expect(await uniqueKeyParts(sub.table)).toEqual({ - [sub.replacementName]: ['COALESCE(organization_id)', ...sub.keyColumns], + [sub.replacementName]: ['organization_id', ...sub.keyColumns], }); }); @@ -363,16 +375,18 @@ describe('#8577 — two more tenant-scoped declared unique indexes become per-or expect(await d.count(sub.table, {})).toBe(1); }); - it('AFTER: rows with no organization stay unique among THEMSELVES (ADR-0120 D3)', async () => { - // Platform/seed rows carry no organization. A bare - // `(organization_id, …)` composite would be NULL-DISTINCT under SQL, so - // every seeded row could be duplicated at will; the NULL-safe key part - // is what prevents that, and seed upsert-by-name relies on it. + it('AFTER: on a pre-18 nullable column, rows with no organization are NOT constrained (ADR-0131 D1)', async () => { + // Platform/seed rows carry no organization. ADR-0120 D3's NULL-safe + // key part once made them a bucket that refused duplicates among + // themselves; ADR-0131 D1 retires it. The key is the PLAIN + // `(organization_id, …)` composite, NULL-DISTINCT under SQL, so on a + // database that still holds such rows they are exempt from the key. // - // ⚠️ On `sys_audience_binding_suggestion` this is not a hypothetical - // bucket: the shipped `syncAudienceBindingSuggestions` writes with a - // hardcoded `{ isSystem: true }` context carrying no tenant, so every - // row it creates today lands organization-less. See + // ⚠️ On `sys_audience_binding_suggestion` such rows are not + // hypothetical on a pre-18 database: the shipped + // `syncAudienceBindingSuggestions` wrote with a hardcoded + // `{ isSystem: true }` context carrying no tenant, so the rows it + // created landed organization-less. See // `suggested-audience-bindings-install-path.test.ts` §3. const d = makeDriver(); await d.initObjects(app(sub, 'fixed') as any); @@ -381,13 +395,15 @@ describe('#8577 — two more tenant-scoped declared unique indexes become per-or await asPre18TenantColumn(d, sub.table); expect((await createAsApi(d, sub.table, row(sub, 'seed1', undefined, sub.key))).status).toBe(201); - expect(await createAsApi(d, sub.table, row(sub, 'seed2', undefined, sub.key))).toMatchObject( - CONFLICT_ENVELOPE, - ); + expect((await createAsApi(d, sub.table, row(sub, 'seed2', undefined, sub.key))).status).toBe(201); - // A tenant may still hold its OWN row of the same key — the platform - // bucket and the organization buckets are separate namespaces. + // A tenant may still hold its OWN row of the same key, and the + // organization's key stays enforced on the very same table — the + // exemption is the NULL organization's alone, not a dropped constraint. expect((await createAsApi(d, sub.table, row(sub, 'own1', 'org_jia', sub.key))).status).toBe(201); + expect(await createAsApi(d, sub.table, row(sub, 'own2', 'org_jia', sub.key))).toMatchObject( + CONFLICT_ENVELOPE, + ); }); it('the key is the whole COMPOSITE — varying a trailing column was always accepted', async () => { @@ -469,8 +485,9 @@ describe('#8577 — two more tenant-scoped declared unique indexes become per-or dropIndexNames: [sub.legacyName], createIndexName: sub.replacementName, createColumns: ['organization_id', ...sub.keyColumns], - nullSafeColumns: ['organization_id'], }); + // ADR-0131 D1: the replacement keys the PLAIN organization column. + expect((entry!.op as any).nullSafeColumns).toBeUndefined(); // ⛔ The old index must NOT ALSO surface as an orphan. An orphan drop // is `destructive`, so an operator applying only the safe half would @@ -493,8 +510,10 @@ describe('#8577 — two more tenant-scoped declared unique indexes become per-or expect(applied.some((e) => e.op.type === 'replace_unique_index')).toBe(true); expect(skipped).toHaveLength(0); + // The PLAIN organization part (ADR-0131 D1), even on this pre-18 + // nullable column. expect(await uniqueKeyParts(sub.table)).toEqual({ - [sub.replacementName]: ['COALESCE(organization_id)', ...sub.keyColumns], + [sub.replacementName]: ['organization_id', ...sub.keyColumns], }); expect(await d.count(sub.table, {})).toBe(3); @@ -517,10 +536,11 @@ describe('#8577 — two more tenant-scoped declared unique indexes become per-or CONFLICT_ENVELOPE, ); - // …and the organization-less bucket survived the migration intact: - // the seeded platform row still blocks a duplicate of ITSELF. - expect(await createAsApi(d, sub.table, row(sub, 'z3', undefined, sub.platformKey))).toMatchObject( - CONFLICT_ENVELOPE, + // …and the seeded organization-less row is exempt from the PLAIN key + // (ADR-0131 D1): SQL UNIQUE is NULL-distinct, so a duplicate of it is + // accepted — the retired ADR-0120 D3 bucket did not survive. + expect((await createAsApi(d, sub.table, row(sub, 'z3', undefined, sub.platformKey))).status).toBe( + 201, ); }); @@ -592,9 +612,10 @@ describe('#8577 — two more tenant-scoped declared unique indexes become per-or name: sub.replacementName, columns: ['organization_id', ...sub.keyColumns], unique: true, - nullSafeColumns: ['organization_id'], }, }); + // ADR-0131 D1: no NULL-safe part — the organization column is keyed plain. + expect(entry!.replacement.nullSafeColumns).toBeUndefined(); }); it('claims nothing for an EXPLICITLY NAMED index — that transition is a recreate', () => { @@ -654,7 +675,7 @@ describe('#8577 — two more tenant-scoped declared unique indexes become per-or const d = makeDriver(); await d.initObjects(app(sub, 'fixed') as any); expect(await uniqueKeyParts(sub.table), `${sub.table} posture=${posture}`).toEqual({ - [sub.replacementName]: ['COALESCE(organization_id)', ...sub.keyColumns], + [sub.replacementName]: ['organization_id', ...sub.keyColumns], }); await d.disconnect(); driver = undefined; diff --git a/packages/drivers/driver-sql/src/sql-driver-autonumber-resync.test.ts b/packages/drivers/driver-sql/src/sql-driver-autonumber-resync.test.ts index e50b4533a93..86ab3498b5f 100644 --- a/packages/drivers/driver-sql/src/sql-driver-autonumber-resync.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-autonumber-resync.test.ts @@ -40,11 +40,13 @@ * * - the filing's own message is a **composite** — * `UNIQUE constraint failed: crm_case.organization_id, crm_case.case_number` - * — and `uniqueViolationColumn` refuses composites by contract; - * - what this repo builds **today** is narrower still: ADR-0120 D3 makes the - * tenanted unique index `(COALESCE(organization_id,'__global__'), field)`, - * an *expression* index, on which SQLite reports - * `UNIQUE constraint failed: index 'uniq_…'` and names no column at all. + * — and `uniqueViolationColumn` refuses composites by contract. Since + * ADR-0131 D1 that is again exactly what this repo builds **today**: the + * tenanted unique index is the plain `(organization_id, field)`; + * - a database still carrying the ADR-0120 D3 index is narrower still: its + * `(COALESCE(organization_id,'__global__'), field)` is an *expression* + * index, on which SQLite reports `UNIQUE constraint failed: index 'uniq_…'` + * and names no column at all. * * So the "column not determinable" limb is not an edge case on this path — it * is the only limb that ever runs for a tenanted autonumber. The discriminator @@ -210,13 +212,16 @@ describe('[#5495] autonumber sequence re-seed on a provable collision', () => { it('pins WHY the column-name predicate cannot carry this decision', async () => { await driver.initObjects([CRM_CASE]); - // 1. What this repo builds today: an expression composite, on which SQLite - // reports the INDEX and no column. + // 1. What this repo builds today (ADR-0131 D1): the PLAIN tenant composite, + // on which SQLite reports both columns — a composite, which + // `uniqueViolationColumn` refuses by contract. This is the filing's own + // message, byte for byte. const indexes: any[] = await knex().raw( `select name, sql from sqlite_master where type='index' and tbl_name='crm_case'`, ); const declared = indexes.find((r) => r.name === 'uniq_crm_case_organization_id_case_number'); - expect(declared?.sql).toContain(`COALESCE(\`organization_id\`, '__global__')`); + expect(declared?.sql).toContain('(`organization_id`, `case_number`)'); + expect(declared?.sql).not.toContain('COALESCE'); await knex()('crm_case').insert({ id: 'd1', organization_id: 'orgA', case_number: 'CASE-00001', title: 'a' }); const caught = await knex()('crm_case') @@ -226,14 +231,28 @@ describe('[#5495] autonumber sequence re-seed on a provable collision', () => { (e: unknown) => e, ); expect(isUniqueViolationError(caught)).toBe(true); - expect((caught as Error).message).toMatch(/UNIQUE constraint failed: index '/); + expect((caught as Error).message).toContain( + 'UNIQUE constraint failed: crm_case.organization_id, crm_case.case_number', + ); expect(uniqueViolationColumn(caught)).toBeUndefined(); - // 2. And the message the filing itself reported — the plain composite that - // predates ADR-0120 D3 — is refused for the other reason. - expect( - uniqueViolationColumn('UNIQUE constraint failed: crm_case.organization_id, crm_case.case_number'), - ).toBeUndefined(); + // 2. And a database still carrying the ADR-0120 D3 expression composite — + // built before ADR-0131 D1, honoured as it stands — is refused for the + // other reason: SQLite reports the INDEX and no column. + await knex().raw('create table legacy_case (id text primary key, organization_id text, case_number text)'); + await knex().raw( + `create unique index uniq_legacy_case_organization_id_case_number on legacy_case (COALESCE(organization_id, '__global__'), case_number)`, + ); + await knex()('legacy_case').insert({ id: 'l1', organization_id: 'orgA', case_number: 'CASE-00001' }); + const caughtLegacy = await knex()('legacy_case') + .insert({ id: 'l2', organization_id: 'orgA', case_number: 'CASE-00001' }) + .then( + () => undefined, + (e: unknown) => e, + ); + expect(isUniqueViolationError(caughtLegacy)).toBe(true); + expect((caughtLegacy as Error).message).toMatch(/UNIQUE constraint failed: index '/); + expect(uniqueViolationColumn(caughtLegacy)).toBeUndefined(); }); /** diff --git a/packages/drivers/driver-sql/src/sql-driver-declared-index-organization-respelling.test.ts b/packages/drivers/driver-sql/src/sql-driver-declared-index-organization-respelling.test.ts index 1b5f6878fa5..6e6d1a82436 100644 --- a/packages/drivers/driver-sql/src/sql-driver-declared-index-organization-respelling.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-declared-index-organization-respelling.test.ts @@ -38,7 +38,9 @@ import { * pinned in `spellings` below. Field-level `unique: true` has meant "per * organization" since #3696; a DECLARED index's `unique: true` is the positional * spelling of `'global'` and takes the listed columns VERBATIM. Only the - * explicit `'organization'` prepends the NULL-safe organization key part. + * explicit `'organization'` prepends the organization key part — since + * ADR-0131 D1 the PLAIN tenant column (it was ADR-0120 D3's NULL-safe + * `COALESCE` part before 18.x). * `packages/lint/src/data-model-rules.ts` calls this "the #4986 trap" and warns * on it (`unique/unscoped-declared-index`); `sys_user_preference` and * `sys_capability` were two instances of it in the platform's own metadata. @@ -182,17 +184,18 @@ describe('#8323 — declared unique indexes on the platform’s tenant-scoped ob expect(bare).toEqual(explicit); }); - it('only the explicit `"organization"` spelling prepends the NULL-safe organization key part', () => { + it('only the explicit `"organization"` spelling prepends the PLAIN organization key part (ADR-0131 D1)', () => { const norm = normalizeDeclaredIndex( 'sys_user_preference', { fields: ['user_id', 'key'], unique: 'organization' }, 'organization_id', ); + // ADR-0131 D1: the tenant column is prepended PLAIN — no + // `nullSafeColumns`, so no COALESCE in the DDL it materializes. expect(norm).toEqual({ name: 'uniq_sys_user_preference_organization_id_user_id_key', columns: ['organization_id', 'user_id', 'key'], unique: true, - nullSafeColumns: ['organization_id'], }); }); @@ -206,11 +209,11 @@ describe('#8323 — declared unique indexes on the platform’s tenant-scoped ob { key: { type: 'string', unique: true } }, 'organization_id', ); + // Keyed on the PLAIN organization column since ADR-0131 D1. expect(fieldLevel).toEqual({ name: 'uniq_sys_user_preference_organization_id_key', columns: ['organization_id', 'key'], unique: true, - nullSafeColumns: ['organization_id'], }); }); }); @@ -277,19 +280,17 @@ describe('#8323 — declared unique indexes on the platform’s tenant-scoped ob ] as const; describe('materialized shape', () => { - it('the fixed declarations key on the NULL-safe organization part', async () => { + it('the fixed declarations key on the PLAIN organization column (ADR-0131 D1)', async () => { + // No COALESCE in new DDL: a fresh table's tenant column is NOT NULL, so + // the plain column is the whole key part. const d = makeDriver(); await d.initObjects(FIXED_APP as any); expect(await uniqueKeyParts('sys_user_preference')).toEqual({ - uniq_sys_user_preference_organization_id_user_id_key: [ - 'COALESCE(organization_id)', - 'user_id', - 'key', - ], + uniq_sys_user_preference_organization_id_user_id_key: ['organization_id', 'user_id', 'key'], }); expect(await uniqueKeyParts('sys_capability')).toEqual({ - uniq_sys_capability_organization_id_name: ['COALESCE(organization_id)', 'name'], + uniq_sys_capability_organization_id_name: ['organization_id', 'name'], }); }); @@ -376,10 +377,13 @@ describe('#8323 — declared unique indexes on the platform’s tenant-scoped ob expect(await d.count('sys_capability', {})).toBe(1); }); - it('AFTER: platform-seeded rows carry no organization and stay unique among THEMSELVES (D3)', async () => { - // The NULL-safe key part is what makes this hold: a bare - // `(organization_id, name)` composite would be NULL-distinct under SQL, - // so every platform-seeded capability could be duplicated at will. + it('AFTER: on a pre-18 nullable column, rows with no organization are NOT constrained by the plain key (ADR-0131 D1)', async () => { + // ADR-0120 D3's NULL-safe key part once kept a NULL-organization + // "bucket" unique among itself. ADR-0131 D1 retires it: the key is the + // PLAIN `(organization_id, name)` composite, and SQL UNIQUE is + // NULL-distinct, so two NULL-organization rows of one name do not + // conflict. A fresh table cannot hold such rows (its tenant column is + // NOT NULL); this pins the plain key on the shape that still can. const d = makeDriver(); await d.initObjects(FIXED_APP as any); // Rows with no organization exist only on a database built before 18.x @@ -387,16 +391,20 @@ describe('#8323 — declared unique indexes on the platform’s tenant-scoped ob await asPre18TenantColumn(d, 'sys_capability'); expect((await createAsApi(d, 'sys_capability', { id: 'p1', name: 'manage_metadata' })).status).toBe(201); - const duplicateSeed = await createAsApi(d, 'sys_capability', { id: 'p2', name: 'manage_metadata' }); - expect(duplicateSeed).toMatchObject(CONFLICT_ENVELOPE); + const nullOrgRepeat = await createAsApi(d, 'sys_capability', { id: 'p2', name: 'manage_metadata' }); + expect(nullOrgRepeat.status).toBe(201); + expect(await d.count('sys_capability', {})).toBe(2); - // An organization may still define its own row of the same name — that - // is the ADR-0066 "admins EXTEND the registry" case, and the reason the - // platform bucket and the organization buckets are separate. + // An organization may still define its own row of the same name — the + // ADR-0066 "admins EXTEND the registry" case — and the plain key still + // refuses that organization's second one: the exemption is the NULL. expect( (await createAsApi(d, 'sys_capability', { id: 'o1', organization_id: 'org_jia', name: 'manage_metadata' })) .status, ).toBe(201); + expect( + await createAsApi(d, 'sys_capability', { id: 'o2', organization_id: 'org_jia', name: 'manage_metadata' }), + ).toMatchObject(CONFLICT_ENVELOPE); }); }); @@ -529,8 +537,9 @@ describe('#8323 — declared unique indexes on the platform’s tenant-scoped ob dropIndexNames: ['uniq_sys_user_preference_user_id_key'], createIndexName: 'uniq_sys_user_preference_organization_id_user_id_key', createColumns: ['organization_id', 'user_id', 'key'], - nullSafeColumns: ['organization_id'], }); + // ADR-0131 D1: the replacement keys the PLAIN organization column. + expect(entry!.op).not.toHaveProperty('nullSafeColumns'); // The old index must NOT be reported as an orphan as well. An orphan drop // is `destructive`, so an operator applying only the safe half would keep @@ -555,12 +564,9 @@ describe('#8323 — declared unique indexes on the platform’s tenant-scoped ob expect(applied.some((e) => e.op.type === 'replace_unique_index')).toBe(true); expect(skipped).toHaveLength(0); + // The plain composite (ADR-0131 D1). expect(await uniqueKeyParts('sys_user_preference')).toEqual({ - uniq_sys_user_preference_organization_id_user_id_key: [ - 'COALESCE(organization_id)', - 'user_id', - 'key', - ], + uniq_sys_user_preference_organization_id_user_id_key: ['organization_id', 'user_id', 'key'], }); expect(await d.count('sys_user_preference', {})).toBe(2); @@ -793,13 +799,9 @@ describe('#8323 — declared unique indexes on the platform’s tenant-scoped ob driver = undefined; } // Positively asserted, not merely "all three agree" — three empty maps - // would satisfy sameness alone. + // would satisfy sameness alone. The plain key part is ADR-0131 D1's. expect(shapes.single).toEqual({ - uniq_sys_user_preference_organization_id_user_id_key: [ - 'COALESCE(organization_id)', - 'user_id', - 'key', - ], + uniq_sys_user_preference_organization_id_user_id_key: ['organization_id', 'user_id', 'key'], }); expect(shapes.group).toEqual(shapes.single); expect(shapes.isolated).toEqual(shapes.single); diff --git a/packages/drivers/driver-sql/src/sql-driver-index-drift.test.ts b/packages/drivers/driver-sql/src/sql-driver-index-drift.test.ts index 6d72cd14c6d..4d2e99c41ec 100644 --- a/packages/drivers/driver-sql/src/sql-driver-index-drift.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-index-drift.test.ts @@ -74,9 +74,10 @@ describe('SqlDriver index drift (#3728)', () => { /** * Unique index name → canonical key parts, read from the index DDL so * expression keys are visible (`PRAGMA index_info` reports a NULL name for - * them). A plain column reads as its name; the NULL-safe organization key - * part (ADR-0120 D3) reads as `COALESCE()` — literal elided, the - * same literal-agnostic identity the drift differ compares on. + * them). A plain column reads as its name; a legacy NULL-safe organization + * key part (ADR-0120 D3) reads as `COALESCE()` — literal elided, the + * same literal-agnostic identity the drift differ compares on. New DDL keys + * the plain organization column (ADR-0131 D1), so the pins below expect none. */ const uniqueIndexColumns = async (table: string): Promise> => { const list: any = await knexInstance.raw(`PRAGMA index_list(${table})`); @@ -212,10 +213,11 @@ describe('SqlDriver index drift (#3728)', () => { const driver = makeDriver(); await driver.initObjects(contactMeta); - // Both are current intent: the tenant composite from the field-level - // `unique: true`, and the verbatim declared global unique. + // Both are current intent: the plain tenant composite from the + // field-level `unique: true` (ADR-0131 D1), and the verbatim declared + // global unique. const uniques = await uniqueIndexColumns('hp_contact'); - expect(uniques['uniq_hp_contact_organization_id_email']).toEqual(['COALESCE(organization_id)', 'email']); + expect(uniques['uniq_hp_contact_organization_id_email']).toEqual(['organization_id', 'email']); expect(uniques['uniq_hp_contact_email']).toEqual(['email']); // Before the fix this reported `replace_unique_index` — proposing to drop @@ -290,7 +292,8 @@ describe('SqlDriver index drift (#3728)', () => { const uniques = await uniqueIndexColumns('product'); expect(uniques['product_code_unique']).toBeUndefined(); - expect(Object.values(uniques)).toContainEqual(['COALESCE(organization_id)', 'code']); + // The replacement is the plain composite (ADR-0131 D1). + expect(Object.values(uniques)).toContainEqual(['organization_id', 'code']); // Existing rows survived, and the cross-tenant insert the issue is about works. expect(await driver.count('product', {})).toBe(2); @@ -310,10 +313,7 @@ describe('SqlDriver index drift (#3728)', () => { await driver.applyMigrationEntries(drift, { allowDestructive: false }); const again = await driver.applyMigrationEntries(drift, { allowDestructive: false }); expect(again.skipped).toHaveLength(0); - expect(Object.values(await uniqueIndexColumns('product'))).toContainEqual([ - 'COALESCE(organization_id)', - 'code', - ]); + expect(Object.values(await uniqueIndexColumns('product'))).toContainEqual(['organization_id', 'code']); }); }); @@ -328,7 +328,7 @@ describe('SqlDriver index drift (#3728)', () => { const uniques = await uniqueIndexColumns('product'); expect(uniques['product_code_unique']).toBeUndefined(); - expect(Object.values(uniques)).toContainEqual(['COALESCE(organization_id)', 'code']); + expect(Object.values(uniques)).toContainEqual(['organization_id', 'code']); expect(await driver.detectManagedDrift()).toHaveLength(0); const b = await driver.create('product', { organization_id: 'org_b', code: 'PROD-00001' }); diff --git a/packages/drivers/driver-sql/src/sql-driver-legacy-tenant-composite-null-distinct.test.ts b/packages/drivers/driver-sql/src/sql-driver-legacy-tenant-composite-null-distinct.test.ts index b609f3f6ae1..b0bd828ebbe 100644 --- a/packages/drivers/driver-sql/src/sql-driver-legacy-tenant-composite-null-distinct.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-legacy-tenant-composite-null-distinct.test.ts @@ -8,9 +8,13 @@ import { asPre18TenantColumn } from './pre-18-tenant-column.testkit.js'; /** * #23603 row 7 — the OLD hand-written table composite * `indexes: [{ fields: ['organization_id', X], unique }]` constrains NOTHING on - * untenanted rows, because SQL UNIQUE is NULL-distinct; field-level - * `unique: true` does, because its organization key part is NULL-safe - * (ADR-0120 D3: `COALESCE(organization_id, '__global__')`). + * untenanted rows, because SQL UNIQUE is NULL-distinct. Field-level + * `unique: true` once differed — its organization key part was NULL-safe + * (ADR-0120 D3: `COALESCE(organization_id, '__global__')`). Since ADR-0131 D1 + * it is the SAME plain composite: the untenanted hole is closed at the column + * instead (a fresh tenant column is NOT NULL, so no untenanted row can be + * written), and on a database built before 18.x — whose column is still + * nullable — both spellings admit untenanted duplicates alike. * * Replaces the driver half of objectstack-ai/hotcrm * `test/case-number-tenant-scope.test.ts` (the R residue in #15953's @@ -27,14 +31,14 @@ import { asPre18TenantColumn } from './pre-18-tenant-column.testkit.js'; * were prevented only by the accident of a single sequence, never by the * database. The old index still refuses a duplicate WITHIN one organization, * which is exactly what hid the hole; both halves are pinned, against the - * field-level spelling on the same driver. + * field-level spelling on the same driver — which now measures the same. * * A declared index's `unique: true` is the positional spelling of `'global'` * (verbatim columns; ADR-0120 D2 converts stored rows to `'global'` and the * spec refuses bare `true` since protocol 18). Both spellings are measured, * because stored metadata older than the conversion reaches the driver bare. */ -describe('row 7 — the old table composite is NULL-distinct, field-level unique is NULL-safe (hotcrm case-number-tenant-scope)', () => { +describe('row 7 — the old table composite and field-level unique are one plain composite; the untenanted hole closes at the column (hotcrm case-number-tenant-scope)', () => { let driver: SqlDriver; beforeEach(() => { @@ -44,7 +48,7 @@ describe('row 7 — the old table composite is NULL-distinct, field-level unique await driver.disconnect(); }); - /** Unique index name → key parts; the NULL-safe part reads `COALESCE(col)`. */ + /** Unique index name → key parts; a legacy NULL-safe part would read `COALESCE(col)`. */ async function uniqueKeyParts(table: string): Promise> { const k = (driver as any).knex; const master: any = await k.raw(`SELECT name, sql FROM sqlite_master WHERE type = 'index' AND tbl_name = ?`, [table]); @@ -63,16 +67,16 @@ describe('row 7 — the old table composite is NULL-distinct, field-level unique const refusal = (p: Promise) => p.then(() => undefined, (e: unknown) => e); - describe('field-level unique: true (the fix)', () => { + describe('field-level unique: true', () => { beforeEach(async () => { await driver.initObjects([ { name: 'kase', fields: { organization_id: { type: 'string' }, case_number: { type: 'string', unique: true } } }, ] as any); }); - it('materializes the per-organization unique with a NULL-safe organization key part', async () => { + it('materializes the per-organization unique with a PLAIN organization key part — the old composite, byte for byte (ADR-0131 D1)', async () => { expect(await uniqueKeyParts('kase')).toEqual({ - uniq_kase_organization_id_case_number: ['COALESCE(organization_id)', 'case_number'], + uniq_kase_organization_id_case_number: ['organization_id', 'case_number'], }); }); @@ -83,13 +87,20 @@ describe('row 7 — the old table composite is NULL-distinct, field-level unique expect(isUniqueViolationError(err)).toBe(true); }); - it('refuses a duplicate on an UNTENANTED install too', async () => { - // Rows with no organization exist only on a database built before 18.x - // (ADR-0131 D1 makes a fresh tenant column NOT NULL). - await asPre18TenantColumn(driver, 'kase'); - await driver.create('kase', { case_number: 'CASE-2' }); + it('closes the UNTENANTED hole at the column: a fresh install refuses the row, a pre-18 one is NULL-distinct', async () => { + // A fresh tenant column is NOT NULL (ADR-0131 D1): the untenanted row the + // hole needed cannot be written at all — the refusal is the column's, + // not the unique index's. const err = await refusal(driver.create('kase', { case_number: 'CASE-2' })); - expect(isUniqueViolationError(err)).toBe(true); + expect(isUniqueViolationError(err)).toBe(false); + expect(String((err as Error)?.message)).toMatch(/NOT NULL constraint failed|null value in column/); + + // Rows with no organization exist only on a database built before 18.x; + // there the plain key part is NULL-distinct, like the old composite. + await asPre18TenantColumn(driver, 'kase'); + const first = await driver.create('kase', { case_number: 'CASE-2' }); + const second = await driver.create('kase', { case_number: 'CASE-2' }); + expect(second.id).not.toBe(first.id); }); }); diff --git a/packages/drivers/driver-sql/src/sql-driver-sys-position-organization-unique.test.ts b/packages/drivers/driver-sql/src/sql-driver-sys-position-organization-unique.test.ts index cdb04dc46ff..4387645ccd0 100644 --- a/packages/drivers/driver-sql/src/sql-driver-sys-position-organization-unique.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-sys-position-organization-unique.test.ts @@ -204,12 +204,15 @@ describe('#8468 — sys_position.name is unique per organization, not per instal // ─────────────────────────────────────────────────────────────────────────── describe('materialized shape', () => { - it('the fixed declaration keys on the NULL-safe organization part', async () => { + it('the fixed declaration keys on the PLAIN organization column (ADR-0131 D1)', async () => { + // ADR-0131 D1: new DDL keys the plain tenant column — no COALESCE. A + // fresh table's tenant column is NOT NULL, so the NULL-safe spelling + // ADR-0120 D3 needed would guard a value that can no longer occur. const d = makeDriver(); await d.initObjects(FIXED_APP as any); expect(await uniqueKeyParts('sys_position')).toEqual({ - uniq_sys_position_organization_id_name: ['COALESCE(organization_id)', 'name'], + uniq_sys_position_organization_id_name: ['organization_id', 'name'], }); }); @@ -307,14 +310,17 @@ describe('#8468 — sys_position.name is unique per organization, not per instal expect(await d.count('sys_position', {})).toBe(1); }); - it('AFTER: platform-seeded rows carry no organization and stay unique among THEMSELVES (D3)', async () => { + it('AFTER: on a pre-18 nullable column, rows with no organization are NOT constrained by the plain key (ADR-0131 D1)', async () => { // `bootstrapBuiltinRoles` seeds the framework-reserved identity positions // (platform_admin / org_*) and the ADR-0090 D9 audience anchors - // (everyone / guest) with `managed_by: 'platform'` and no organization. - // A bare `(organization_id, name)` composite would be NULL-DISTINCT under - // SQL, so every seeded position could be duplicated at will; the NULL-safe - // key part is what prevents that, and the bootstrap upsert-by-name relies - // on it. + // (everyone / guest) with `managed_by: 'platform'`. ADR-0120 D3 once keyed + // a NULL-safe organization part so a NULL-organization "bucket" stayed + // unique among itself. ADR-0131 D1 retires that bucket: new DDL keys the + // PLAIN `(organization_id, name)` composite, and SQL UNIQUE is + // NULL-distinct, so on a nullable column two NULL-organization rows of + // the same name do not conflict. A fresh table cannot hold such rows at + // all (its tenant column is NOT NULL); this pins what the plain key does + // on the only shape that still can. const d = makeDriver(); await d.initObjects(FIXED_APP as any); // Rows with no organization exist only on a database built before 18.x @@ -326,18 +332,27 @@ describe('#8468 — sys_position.name is unique per organization, not per instal id: 'seed1', name: 'platform_admin', label: 'Platform Admin', managed_by: 'platform', })).status, ).toBe(201); - const duplicateSeed = await createAsApi(d, 'sys_position', { + const nullOrgRepeat = await createAsApi(d, 'sys_position', { id: 'seed2', name: 'platform_admin', label: 'Platform Admin', managed_by: 'platform', }); - expect(duplicateSeed).toMatchObject(CONFLICT_ENVELOPE); + expect(nullOrgRepeat.status).toBe(201); + expect( + (await d.find('sys_position', {})).filter((r: any) => r.name === 'platform_admin').length, + ).toBe(2); - // A tenant may still author its OWN position of the same name — the - // platform bucket and the organization buckets are separate namespaces. + // A tenant may still author its OWN position of the same name, and the + // plain key still refuses that organization's second one — the exemption + // is the NULL, not the column. expect( (await createAsApi(d, 'sys_position', { id: 'own1', organization_id: 'org_jia', name: 'platform_admin', label: 'Local', managed_by: 'admin', })).status, ).toBe(201); + expect( + await createAsApi(d, 'sys_position', { + id: 'own2', organization_id: 'org_jia', name: 'platform_admin', label: 'Local again', managed_by: 'admin', + }), + ).toMatchObject(CONFLICT_ENVELOPE); }); }); @@ -400,8 +415,9 @@ describe('#8468 — sys_position.name is unique per organization, not per instal dropIndexNames: ['uniq_sys_position_name'], createIndexName: 'uniq_sys_position_organization_id_name', createColumns: ['organization_id', 'name'], - nullSafeColumns: ['organization_id'], }); + // ADR-0131 D1: the replacement keys the PLAIN organization column. + expect(entry!.op).not.toHaveProperty('nullSafeColumns'); // ⛔ The old index must NOT ALSO surface as an orphan. An orphan drop is // `destructive`, so an operator applying only the safe half would keep the @@ -425,8 +441,10 @@ describe('#8468 — sys_position.name is unique per organization, not per instal expect(applied.some((e) => e.op.type === 'replace_unique_index')).toBe(true); expect(skipped).toHaveLength(0); + // The plain composite (ADR-0131 D1) — and it converges on the pre-18 + // nullable column too, since the expectation is plain as well. expect(await uniqueKeyParts('sys_position')).toEqual({ - uniq_sys_position_organization_id_name: ['COALESCE(organization_id)', 'name'], + uniq_sys_position_organization_id_name: ['organization_id', 'name'], }); expect(await d.count('sys_position', {})).toBe(3); @@ -455,12 +473,15 @@ describe('#8468 — sys_position.name is unique per organization, not per instal }), ).toMatchObject(CONFLICT_ENVELOPE); - // …and the platform seed bucket survived the migration intact. + // …and the NULL-organization row the deployment already held is exempt + // from the migrated PLAIN key (ADR-0131 D1: SQL UNIQUE is NULL-distinct; + // the ADR-0120 D3 bucket is gone). Before the migration the global index + // refused this; afterwards only organization-keyed rows are constrained. expect( - await createAsApi(d, 'sys_position', { + (await createAsApi(d, 'sys_position', { id: 'seed2', name: 'platform_admin', label: 'Platform Admin', managed_by: 'platform', - }), - ).toMatchObject(CONFLICT_ENVELOPE); + })).status, + ).toBe(201); }); it('boot creates the replacement ADDITIVELY, so the defect is STILL LIVE until the plan runs', async () => { @@ -532,9 +553,10 @@ describe('#8468 — sys_position.name is unique per organization, not per instal name: 'uniq_sys_position_organization_id_name', columns: ['organization_id', 'name'], unique: true, - nullSafeColumns: ['organization_id'], }, }); + // ADR-0131 D1: the replacement is the PLAIN composite — no NULL-safe part. + expect(entry!.replacement).not.toHaveProperty('nullSafeColumns'); }); it('claims nothing for an EXPLICITLY NAMED index — that transition is a recreate', () => { @@ -588,7 +610,7 @@ describe('#8468 — sys_position.name is unique per organization, not per instal const d = makeDriver(); await d.initObjects(FIXED_APP as any); expect(await uniqueKeyParts('sys_position'), `posture=${posture}`).toEqual({ - uniq_sys_position_organization_id_name: ['COALESCE(organization_id)', 'name'], + uniq_sys_position_organization_id_name: ['organization_id', 'name'], }); await d.disconnect(); driver = undefined; diff --git a/packages/drivers/driver-sql/src/sql-driver-sys-setting-organization-unique.test.ts b/packages/drivers/driver-sql/src/sql-driver-sys-setting-organization-unique.test.ts index 742e1bd11d5..70d42fa8cc7 100644 --- a/packages/drivers/driver-sql/src/sql-driver-sys-setting-organization-unique.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-sys-setting-organization-unique.test.ts @@ -235,12 +235,15 @@ describe('#8555 — sys_setting: the declared unique index becomes per-organizat // 1. Materialized shape — both spellings, kept side by side // ───────────────────────────────────────────────────────────────────────── - it('the fixed declaration keys on the NULL-safe organization part', async () => { + it('the fixed declaration keys on the PLAIN organization column (ADR-0131 D1)', async () => { + // ADR-0131 D1: new DDL keys the plain tenant column — no COALESCE. A fresh + // table's tenant column is NOT NULL, so ADR-0120 D3's NULL-safe spelling + // would guard a value that can no longer occur. const d = makeDriver(); await d.initObjects(app('fixed') as any); expect(await uniqueKeyParts()).toEqual({ - [REPLACEMENT_NAME]: ['COALESCE(organization_id)', ...KEY_COLUMNS], + [REPLACEMENT_NAME]: ['organization_id', ...KEY_COLUMNS], }); }); @@ -306,14 +309,15 @@ describe('#8555 — sys_setting: the declared unique index becomes per-organizat expect(await d.count(TABLE, {})).toBe(1); }); - it('AFTER: rows with no organization stay unique among THEMSELVES (ADR-0120 D3)', async () => { - // This is the arm that answers the card's "argument for keeping it global". - // The `scope='global'` LAYER — the platform default every organization - // resolves against at rung 2 — is preserved by the NULL-safe key part, not - // by making the whole index installation-wide: platform rows carry no - // organization, so they share the one `__global__` bucket and remain unique - // within it, while an organization may still hold its own row of the same - // key at its own layer. + it('AFTER: on a pre-18 nullable column, rows with no organization are NOT constrained by the plain key (ADR-0131 D1)', async () => { + // This arm once answered the card's "argument for keeping it global" with + // ADR-0120 D3's NULL-safe key part: organization-less platform rows shared + // one `COALESCE` bucket and stayed unique within it. ADR-0131 D1 retires + // that bucket. New DDL keys the PLAIN `organization_id`, and SQL UNIQUE is + // NULL-distinct, so on a nullable column two organization-less rows of the + // same key do not conflict. A fresh table cannot hold such rows (its + // tenant column is NOT NULL; the ownership ceremony moves the platform + // layer's rows out); this pins the plain key on the shape that still can. const d = makeDriver(); await d.initObjects(app('fixed') as any); // Rows with no organization exist only on a database built before 18.x @@ -322,11 +326,14 @@ describe('#8555 — sys_setting: the declared unique index becomes per-organizat const platform = { scope: 'global', namespace: 'branding', key: 'logo_url' }; expect((await createAsApi(d, row('seed1', undefined, platform))).status).toBe(201); - expect(await createAsApi(d, row('seed2', undefined, platform))).toMatchObject( - CONFLICT_ENVELOPE, - ); + expect((await createAsApi(d, row('seed2', undefined, platform))).status).toBe(201); + expect(await d.count(TABLE, {})).toBe(2); + // An organization may still hold its own row of the same key, and the + // plain key still refuses that organization's second one — the exemption + // is the NULL, not the column. expect((await createAsApi(d, row('own1', 'org_jia', platform))).status).toBe(201); + expect(await createAsApi(d, row('own2', 'org_jia', platform))).toMatchObject(CONFLICT_ENVELOPE); }); it('the key is the whole COMPOSITE — varying only the trailing column was always accepted', async () => { @@ -404,8 +411,9 @@ describe('#8555 — sys_setting: the declared unique index becomes per-organizat dropIndexNames: [LEGACY_NAME], createIndexName: REPLACEMENT_NAME, createColumns: ['organization_id', ...KEY_COLUMNS], - nullSafeColumns: ['organization_id'], }); + // ADR-0131 D1: the replacement keys the PLAIN organization column. + expect(entry!.op).not.toHaveProperty('nullSafeColumns'); // ⛔ The old index must NOT ALSO surface as an orphan. An orphan drop is // `destructive`, so an operator applying only the safe half would keep the @@ -428,8 +436,10 @@ describe('#8555 — sys_setting: the declared unique index becomes per-organizat expect(applied.some((e) => e.op.type === 'replace_unique_index')).toBe(true); expect(skipped).toHaveLength(0); + // The plain composite (ADR-0131 D1) — and it converges on the pre-18 + // nullable column too, since the expectation is plain as well. expect(await uniqueKeyParts()).toEqual({ - [REPLACEMENT_NAME]: ['COALESCE(organization_id)', ...KEY_COLUMNS], + [REPLACEMENT_NAME]: ['organization_id', ...KEY_COLUMNS], }); // Every row survives — including the two pre-existing duplicates the old // index allowed. A migration that silently dropped them would be data loss @@ -453,11 +463,12 @@ describe('#8555 — sys_setting: the declared unique index becomes per-organizat // The anti-vacuity arm, on the SAME migrated index. expect(await createAsApi(d, row('z2', 'org_jia', KEY))).toMatchObject(CONFLICT_ENVELOPE); - // …and the organization-less bucket survived the migration intact: the - // seeded platform row still blocks a duplicate of ITSELF. + // …and the organization-less row the deployment already held is exempt + // from the migrated PLAIN key (ADR-0131 D1: SQL UNIQUE is NULL-distinct; + // the ADR-0120 D3 bucket is gone), so a second one of it is accepted. expect( - await createAsApi(d, row('z3', undefined, { scope: 'global', key: 'platform_default' })), - ).toMatchObject(CONFLICT_ENVELOPE); + (await createAsApi(d, row('z3', undefined, { scope: 'global', key: 'platform_default' }))).status, + ).toBe(201); }); it('boot creates the replacement ADDITIVELY, so the defect is STILL LIVE until the plan runs', async () => { @@ -540,9 +551,11 @@ describe('#8555 — sys_setting: the declared unique index becomes per-organizat * without weakening anything else, survives a later boot's additive sync, * and is not reported as drift the reconciler would undo. */ + // Since ADR-0131 D1 the builder keys `organization_id` PLAIN, exactly as + // the driver does; only `user_id` keeps its NULL-safe `COALESCE`. const NULL_SAFE_IDENTITY_DDL = `CREATE UNIQUE INDEX IF NOT EXISTS ${REPLACEMENT_NAME} ON ${TABLE} ` + - "(COALESCE(organization_id, '__global__'), namespace, key, scope, COALESCE(user_id, ''))"; + "(organization_id, namespace, key, scope, COALESCE(user_id, ''))"; /** The migration's probe-first order, through the driver's raw seam. */ const runIdentityMigration = async (d: SqlDriver): Promise => { @@ -590,7 +603,13 @@ describe('#8555 — sys_setting: the declared unique index becomes per-organizat expect(await d.count(TABLE, {})).toBe(1); }); - it('AFTER the migration: two platform defaults on the `scope=global` layer are refused', async () => { + it('AFTER the migration: a `scope=global` duplicate is refused within an organization; organization-less rows are exempt (ADR-0131 D1)', async () => { + // The `user_id` fold closes the hole on the `global` layer exactly as on + // the `tenant` one. What it no longer does is fold the ORGANIZATION: + // ADR-0131 D1 keys `organization_id` plain, so on a pre-18 nullable + // column two organization-less platform defaults are NULL-distinct and + // both accepted. On 18.x those rows do not live here at all (the tenant + // column is NOT NULL; the ownership ceremony moves them out). const d = makeDriver(); await d.initObjects(app('fixed') as any); // Rows with no organization exist only on a database built before 18.x @@ -598,8 +617,11 @@ describe('#8555 — sys_setting: the declared unique index becomes per-organizat await asPre18TenantColumn(d, TABLE); await runIdentityMigration(d); - expect((await createAsApi(d, row('a', undefined, globalRow))).status).toBe(201); - expect(await createAsApi(d, row('b', undefined, globalRow))).toMatchObject(CONFLICT_ENVELOPE); + expect((await createAsApi(d, row('a', 'org_jia', globalRow))).status).toBe(201); + expect(await createAsApi(d, row('b', 'org_jia', globalRow))).toMatchObject(CONFLICT_ENVELOPE); + + expect((await createAsApi(d, row('c', undefined, globalRow))).status).toBe(201); + expect((await createAsApi(d, row('d', undefined, globalRow))).status).toBe(201); }); it('AFTER the migration (anti-vacuity): the key is still PER-ORGANIZATION', async () => { @@ -648,7 +670,7 @@ describe('#8555 — sys_setting: the declared unique index becomes per-organizat await d.initObjects(app('fixed') as any); expect(await uniqueKeyParts()).toEqual({ - [REPLACEMENT_NAME]: ['COALESCE(organization_id)', 'namespace', 'key', 'scope', 'COALESCE(user_id)'], + [REPLACEMENT_NAME]: ['organization_id', 'namespace', 'key', 'scope', 'COALESCE(user_id)'], }); expect(await createAsApi(d, row('a', 'org_jia', tenantRow))).toMatchObject({ status: 201 }); expect(await createAsApi(d, row('b', 'org_jia', tenantRow))).toMatchObject(CONFLICT_ENVELOPE); @@ -692,8 +714,10 @@ describe('#8555 — sys_setting: the declared unique index becomes per-organizat expect(isUniqueViolationError(refusal)).toBe(true); expect(await d.count(TABLE, {})).toBe(2); expect(Object.keys(await uniqueKeyParts())).toEqual([REPLACEMENT_NAME]); + // The previous index — the driver's declared one, plain-keyed since + // ADR-0131 D1 — is still the one on the table. expect(await uniqueKeyParts()).toEqual({ - [REPLACEMENT_NAME]: ['COALESCE(organization_id)', ...KEY_COLUMNS], + [REPLACEMENT_NAME]: ['organization_id', ...KEY_COLUMNS], }); }); }); @@ -724,9 +748,10 @@ describe('#8555 — sys_setting: the declared unique index becomes per-organizat name: REPLACEMENT_NAME, columns: ['organization_id', ...KEY_COLUMNS], unique: true, - nullSafeColumns: ['organization_id'], }, }); + // ADR-0131 D1: the replacement is the PLAIN composite — no NULL-safe part. + expect(entry!.replacement).not.toHaveProperty('nullSafeColumns'); }); it('claims nothing for an EXPLICITLY NAMED index — that transition is a recreate', () => { @@ -781,7 +806,7 @@ describe('#8555 — sys_setting: the declared unique index becomes per-organizat const d = makeDriver(); await d.initObjects(app('fixed') as any); expect(await uniqueKeyParts(), `posture=${posture}`).toEqual({ - [REPLACEMENT_NAME]: ['COALESCE(organization_id)', ...KEY_COLUMNS], + [REPLACEMENT_NAME]: ['organization_id', ...KEY_COLUMNS], }); await d.disconnect(); driver = undefined; diff --git a/packages/drivers/driver-sql/src/sql-driver-tenant-key-equivalence.test.ts b/packages/drivers/driver-sql/src/sql-driver-tenant-key-equivalence.test.ts new file mode 100644 index 00000000000..6e3c304729d --- /dev/null +++ b/packages/drivers/driver-sql/src/sql-driver-tenant-key-equivalence.test.ts @@ -0,0 +1,202 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { SqlDriver } from './sql-driver.js'; +import { + carriesNullSafeTenantKeyPart, + diffManagedIndexes, + hashShadowColumnFor, + normalizeDeclaredIndex, + uniqueIndexesFromFields, + type ExpectedIndex, + type PhysicalIndex, +} from './schema-drift.js'; +import { asPre18TenantColumn } from './pre-18-tenant-column.testkit.js'; + +/** + * [18.x step 7b-1 + 7b-2] The organization key part of a unique index. + * + * 18.0 keyed every organization-scoped unique through ADR-0120 D3's + * `COALESCE(organization_id, '__global__')`. Since ADR-0131 D1 the tenant + * column is NOT NULL, so new DDL keys the plain column (7b-2), and the differ + * reads the old form as the same constraint wherever the tenant column holds + * no NULL: physically NOT NULL, or a certified database — a birth marker or an + * organization-ownership ceremony marker of the current shape generation + * (7b-1). Elsewhere the two forms are different constraints, and that + * reading stays: the uncertified, nullable legs below are each case's control. + */ + +const TABLE = 'eqv_account'; +const ORG = 'organization_id'; +const NAME = `uniq_${TABLE}_${ORG}_code`; + +/** What 18.0 built: the NULL-safe organization key part, attributed by introspection. */ +const coalesced: PhysicalIndex = { + name: NAME, + columns: [ORG, 'code'], + unique: true, + expressions: [`COALESCE("${ORG}", '__global__')`], + nullSafeColumns: [ORG], +}; +/** What HEAD builds: the plain composite. */ +const plain: PhysicalIndex = { name: NAME, columns: [ORG, 'code'], unique: true }; +const expectedPlain: ExpectedIndex = { name: NAME, columns: [ORG, 'code'], unique: true }; +const expectedNullSafe: ExpectedIndex = { ...expectedPlain, nullSafeColumns: [ORG] }; + +const diff = (expected: ExpectedIndex, physical: PhysicalIndex, tenantKeyCertified?: boolean, tenantField: string | null = ORG) => + diffManagedIndexes({ table: TABLE, expected: [expected], legacy: [], physical: [physical], tenantField, tenantKeyCertified }); + +describe('7b-2 — the generators key the plain organization column', () => { + it('a field-level organization unique is the plain composite, tenant column first', () => { + expect(uniqueIndexesFromFields(TABLE, { code: { type: 'text', unique: true } }, ORG)).toEqual([expectedPlain]); + }); + + it("a declared `unique: 'organization'` prepends the plain tenant column", () => { + expect(normalizeDeclaredIndex(TABLE, { fields: ['code'], unique: 'organization' }, ORG)).toEqual(expectedPlain); + }); + + it('an input already normalized with a NULL-safe part is honoured verbatim (the read vocabulary stays)', () => { + expect(normalizeDeclaredIndex(TABLE, { name: NAME, fields: [ORG, 'code'], unique: true, nullSafeColumns: [ORG] }, ORG)).toEqual( + expectedNullSafe, + ); + }); +}); + +describe('7b-1 — the differ: COALESCE(tenant) ≡ tenant where the tenant column holds no NULL', () => { + it('an 18.0 COALESCE index against the plain expectation: no drift when certified', () => { + expect(diff(expectedPlain, coalesced, true)).toEqual([]); + }); + + it('control — the same pair, uncertified: a destructive recreate_index (the two are different constraints)', () => { + const [entry, ...rest] = diff(expectedPlain, coalesced, false); + expect(rest).toEqual([]); + expect(entry).toMatchObject({ category: 'destructive', op: { type: 'recreate_index', indexName: NAME } }); + }); + + it('the uncertified entry names the ownership ceremony as its remedy, never --allow-destructive', () => { + const [entry] = diff(expectedPlain, coalesced, false); + expect(entry!.message).toContain('"os migrate organization-ownership"'); + expect(entry!.message).toContain('not certified'); + // The rebuild is named only to be ruled out, never prescribed as the fix. + expect(entry!.message).toContain('Do not rebuild it with "os migrate apply --allow-destructive"'); + expect(entry!.message).not.toMatch(/existing duplicates: "os migrate apply --allow-destructive"/); + // Control — a real redefinition keeps the rebuild remedy it always had. + const other: ExpectedIndex = { name: NAME, columns: [ORG, 'code', 'region'], unique: true }; + const [rebuild] = diff(other, coalesced, false); + expect(rebuild!.message).not.toContain('os migrate organization-ownership'); + expect(rebuild!.message).toContain('"os migrate apply --allow-destructive"'); + }); + + it('the other direction: a plain physical index against a NULL-safe expectation, no drift when certified', () => { + expect(diff(expectedNullSafe, plain, true)).toEqual([]); + expect(diff(expectedNullSafe, plain, false).map((d) => d.op.type)).toEqual(['recreate_index']); + }); + + it("the equivalence is the TENANT column's: on a table whose tenant column is another one, it does not apply", () => { + expect(diff(expectedNullSafe, plain, true, 'workspace_id').map((d) => d.op.type)).toEqual(['recreate_index']); + }); + + it('it changes the form of the key part only — a different column set stays drift, certified or not', () => { + const other: ExpectedIndex = { name: NAME, columns: [ORG, 'code', 'region'], unique: true }; + expect(diff(other, coalesced, true).map((d) => d.op.type)).toEqual(['recreate_index']); + }); + + it('a MySQL hash shadow carrying the COALESCE key reads as the plain key when certified', () => { + const carrier: PhysicalIndex = { + name: NAME, + columns: [hashShadowColumnFor(NAME)], + unique: true, + shadowKey: [ + { column: ORG, nullSafe: true }, + { column: 'code', nullSafe: false }, + ], + }; + expect(diff(expectedPlain, carrier, true)).toEqual([]); + expect(diff(expectedPlain, carrier, false).map((d) => d.op.type)).toEqual(['recreate_index']); + }); + + it('carriesNullSafeTenantKeyPart answers for direct and shadow-carried keys, and only for the tenant column', () => { + expect(carriesNullSafeTenantKeyPart([coalesced], ORG)).toBe(true); + expect(carriesNullSafeTenantKeyPart([plain], ORG)).toBe(false); + expect(carriesNullSafeTenantKeyPart([coalesced], null)).toBe(false); + expect(carriesNullSafeTenantKeyPart([{ ...coalesced, nullSafeColumns: ['package_id'] }], ORG)).toBe(false); + }); +}); + +describe('the driver, on real SQLite', () => { + let driver: SqlDriver; + const object = { name: TABLE, fields: { code: { type: 'text', unique: true }, [ORG]: { type: 'text' } } }; + + beforeEach(async () => { + driver = new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }); + await driver.initObjects([object]); + }); + + afterEach(async () => { + await driver.disconnect(); + }); + + const knex = () => (driver as unknown as { knex: { raw(sql: string, b?: unknown[]): Promise } }).knex; + const indexSql = async (): Promise => { + const rows: any[] = await knex().raw(`SELECT sql FROM sqlite_master WHERE type = 'index' AND name = ?`, [NAME]); + return String(rows[0]?.sql ?? ''); + }; + /** The table as 18.0 left it: a nullable tenant column and the COALESCE index under the declared name. */ + const as18Point0 = async (): Promise => { + await asPre18TenantColumn(driver, TABLE); + await knex().raw(`DROP INDEX "${NAME}"`); + await knex().raw(`CREATE UNIQUE INDEX "${NAME}" ON "${TABLE}" (COALESCE("${ORG}", '__global__'), "code")`); + }; + const indexOps = async () => (await driver.detectManagedDrift()).filter((d) => d.op.type.endsWith('_index')); + + it('a fresh table gets the plain composite — no COALESCE — and no index drift', async () => { + const sql = await indexSql(); + expect(sql).toMatch(/\(\s*[`"]?organization_id[`"]?\s*,\s*[`"]?code[`"]?\s*\)/); + expect(sql).not.toMatch(/COALESCE/i); + expect(await indexOps()).toEqual([]); + }); + + it('an 18.0 table on a certified database: no drift, and the certification is asked', async () => { + await as18Point0(); + let asked = 0; + driver.setTenantKeyCertifiedResolver(() => { + asked++; + return true; + }); + expect(await indexOps()).toEqual([]); + expect(asked).toBeGreaterThan(0); + expect(await indexSql()).toMatch(/COALESCE/); + }); + + it('control — the same table with no certification: the destructive recreate_index the equivalence exists to remove', async () => { + await as18Point0(); + expect((await indexOps()).map((d) => [d.op.type, d.category])).toEqual([['recreate_index', 'destructive']]); + }); + + it('a resolver that answers no, or throws, is "not certified"', async () => { + await as18Point0(); + driver.setTenantKeyCertifiedResolver(() => false); + expect((await indexOps()).map((d) => d.op.type)).toEqual(['recreate_index']); + driver.setTenantKeyCertifiedResolver(() => { + throw new Error('ledger unreadable'); + }); + expect((await indexOps()).map((d) => d.op.type)).toEqual(['recreate_index']); + }); + + it('a physically NOT NULL tenant column is its own witness: no drift without any certification', async () => { + await knex().raw(`DROP INDEX "${NAME}"`); + await knex().raw(`CREATE UNIQUE INDEX "${NAME}" ON "${TABLE}" (COALESCE("${ORG}", '__global__'), "code")`); + expect(await indexOps()).toEqual([]); + }); + + it('a table carrying no COALESCE tenant key part never asks the question', async () => { + let asked = 0; + driver.setTenantKeyCertifiedResolver(() => { + asked++; + return true; + }); + await asPre18TenantColumn(driver, TABLE); + expect(await indexOps()).toEqual([]); + expect(asked).toBe(0); + }); +}); diff --git a/packages/drivers/driver-sql/src/sql-driver-tenant-scoped-declared-unique.test.ts b/packages/drivers/driver-sql/src/sql-driver-tenant-scoped-declared-unique.test.ts index 6242d96eff2..d77e131a8e0 100644 --- a/packages/drivers/driver-sql/src/sql-driver-tenant-scoped-declared-unique.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-tenant-scoped-declared-unique.test.ts @@ -66,6 +66,16 @@ import { asPre18TenantColumn } from './pre-18-tenant-column.testkit.js'; * half keeps the defect while the plan reads as applied. Section 3 is therefore * the load-bearing part of this file: it builds installations that ALREADY HAVE * the old index and real rows, then migrates them. + * + * ## The organization key part is PLAIN (ADR-0131 D1) + * + * The replacement once keyed `COALESCE(organization_id, '')` — ADR-0120 D3's + * NULL-safe part, which made the organization-less rows one more bucket that + * refused duplicates among themselves. Since 18.x a fresh tenant column is NOT + * NULL and the driver keys the PLAIN column: `(organization_id, …listed)`. On a + * pre-18 database whose tenant column is still nullable, rows with no + * organization are not constrained at all — SQL UNIQUE is NULL-distinct — and + * this file pins that exemption rather than the retired bucket. */ /** The wire shape a duplicate insert must produce, per `rest-server.ts`. */ @@ -338,12 +348,14 @@ describe('#8554 — five tenant-scoped declared unique indexes become per-organi // 1. Materialized shape — both spellings, kept side by side // ─────────────────────────────────────────────────────────────────── - it('the fixed declaration keys on the NULL-safe organization part', async () => { + it('the fixed declaration keys on the PLAIN organization part (ADR-0131 D1)', async () => { + // No COALESCE: a fresh tenant column is NOT NULL, so the plain column + // is already a total key part. const d = makeDriver(); await d.initObjects(app(sub, 'fixed') as any); expect(await uniqueKeyParts(sub.table)).toEqual({ - [sub.replacementName]: ['COALESCE(organization_id)', ...sub.keyColumns], + [sub.replacementName]: ['organization_id', ...sub.keyColumns], }); }); @@ -410,11 +422,12 @@ describe('#8554 — five tenant-scoped declared unique indexes become per-organi expect(await d.count(sub.table, {})).toBe(1); }); - it('AFTER: rows with no organization stay unique among THEMSELVES (ADR-0120 D3)', async () => { - // Platform/seed rows carry no organization. A bare - // `(organization_id, …)` composite would be NULL-DISTINCT under SQL, so - // every seeded row could be duplicated at will; the NULL-safe key part - // is what prevents that, and seed upsert-by-name relies on it. + it('AFTER: on a pre-18 nullable column, rows with no organization are NOT constrained (ADR-0131 D1)', async () => { + // Platform/seed rows carry no organization. ADR-0120 D3's NULL-safe + // key part once made them a bucket that refused duplicates among + // themselves; ADR-0131 D1 retires it. The key is the PLAIN + // `(organization_id, …)` composite, NULL-DISTINCT under SQL, so on a + // database that still holds such rows they are exempt from the key. const d = makeDriver(); await d.initObjects(app(sub, 'fixed') as any); // Rows with no organization exist only on a database built before 18.x @@ -422,13 +435,15 @@ describe('#8554 — five tenant-scoped declared unique indexes become per-organi await asPre18TenantColumn(d, sub.table); expect((await createAsApi(d, sub.table, row(sub, 'seed1', undefined, sub.key))).status).toBe(201); - expect(await createAsApi(d, sub.table, row(sub, 'seed2', undefined, sub.key))).toMatchObject( - CONFLICT_ENVELOPE, - ); + expect((await createAsApi(d, sub.table, row(sub, 'seed2', undefined, sub.key))).status).toBe(201); - // A tenant may still hold its OWN row of the same key — the platform - // bucket and the organization buckets are separate namespaces. + // A tenant may still hold its OWN row of the same key, and the + // organization's key stays enforced on the very same table — the + // exemption is the NULL organization's alone, not a dropped constraint. expect((await createAsApi(d, sub.table, row(sub, 'own1', 'org_jia', sub.key))).status).toBe(201); + expect(await createAsApi(d, sub.table, row(sub, 'own2', 'org_jia', sub.key))).toMatchObject( + CONFLICT_ENVELOPE, + ); }); if (sub.trailingControl) { @@ -497,8 +512,9 @@ describe('#8554 — five tenant-scoped declared unique indexes become per-organi dropIndexNames: [sub.legacyName], createIndexName: sub.replacementName, createColumns: ['organization_id', ...sub.keyColumns], - nullSafeColumns: ['organization_id'], }); + // ADR-0131 D1: the replacement keys the PLAIN organization column. + expect((entry!.op as any).nullSafeColumns).toBeUndefined(); // ⛔ The old index must NOT ALSO surface as an orphan. An orphan drop // is `destructive`, so an operator applying only the safe half would @@ -521,8 +537,10 @@ describe('#8554 — five tenant-scoped declared unique indexes become per-organi expect(applied.some((e) => e.op.type === 'replace_unique_index')).toBe(true); expect(skipped).toHaveLength(0); + // The PLAIN organization part (ADR-0131 D1), even on this pre-18 + // nullable column. expect(await uniqueKeyParts(sub.table)).toEqual({ - [sub.replacementName]: ['COALESCE(organization_id)', ...sub.keyColumns], + [sub.replacementName]: ['organization_id', ...sub.keyColumns], }); expect(await d.count(sub.table, {})).toBe(3); @@ -545,10 +563,11 @@ describe('#8554 — five tenant-scoped declared unique indexes become per-organi CONFLICT_ENVELOPE, ); - // …and the organization-less bucket survived the migration intact: - // the seeded platform row still blocks a duplicate of ITSELF. - expect(await createAsApi(d, sub.table, row(sub, 'z3', undefined, sub.platformKey))).toMatchObject( - CONFLICT_ENVELOPE, + // …and the seeded organization-less row is exempt from the PLAIN key + // (ADR-0131 D1): SQL UNIQUE is NULL-distinct, so a duplicate of it is + // accepted — the retired ADR-0120 D3 bucket did not survive. + expect((await createAsApi(d, sub.table, row(sub, 'z3', undefined, sub.platformKey))).status).toBe( + 201, ); }); @@ -619,9 +638,10 @@ describe('#8554 — five tenant-scoped declared unique indexes become per-organi name: sub.replacementName, columns: ['organization_id', ...sub.keyColumns], unique: true, - nullSafeColumns: ['organization_id'], }, }); + // ADR-0131 D1: no NULL-safe part — the organization column is keyed plain. + expect(entry!.replacement.nullSafeColumns).toBeUndefined(); }); it('claims nothing for an EXPLICITLY NAMED index — that transition is a recreate', () => { @@ -681,7 +701,7 @@ describe('#8554 — five tenant-scoped declared unique indexes become per-organi const d = makeDriver(); await d.initObjects(app(sub, 'fixed') as any); expect(await uniqueKeyParts(sub.table), `${sub.table} posture=${posture}`).toEqual({ - [sub.replacementName]: ['COALESCE(organization_id)', ...sub.keyColumns], + [sub.replacementName]: ['organization_id', ...sub.keyColumns], }); await d.disconnect(); driver = undefined; diff --git a/packages/drivers/driver-sql/src/sql-driver-unique-tenancy.test.ts b/packages/drivers/driver-sql/src/sql-driver-unique-tenancy.test.ts index e43351665a5..345d2e71421 100644 --- a/packages/drivers/driver-sql/src/sql-driver-unique-tenancy.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-unique-tenancy.test.ts @@ -7,7 +7,7 @@ import { asPre18TenantColumn } from './pre-18-tenant-column.testkit.js'; /** * Unique-scope materialization: tenancy composites (#3696) + the explicit - * scope vocabulary and NULL-safe organization key part (ADR-0120, #5030). + * scope vocabulary (ADR-0120) + the plain organization key part (ADR-0131 D1). * * The defect #3696 locked down: `unique: true` used to become a single-column * global index that ignored `tenancy` entirely, while the autonumber sequence @@ -18,15 +18,20 @@ import { asPre18TenantColumn } from './pre-18-tenant-column.testkit.js'; * The defect #5030 measured on top of it: SQL UNIQUE is NULL-distinct, so the * bare `(organization_id, field)` composite enforced NOTHING on rows whose * organization is NULL — which on a single-tenant stack (where the kernel - * injects the column and never fills it) is EVERY row. ADR-0120 D3 makes the - * organization key part NULL-safe: `COALESCE(organization_id, '__global__')`. + * injects the column and never fills it) is EVERY row. ADR-0120 D3 answered it + * with a NULL-safe key part, `COALESCE(organization_id, '__global__')`. + * ADR-0131 D1 closes the hole at the column instead: a fresh table's tenant + * column is NOT NULL, so new DDL keys the PLAIN organization column and the + * NULL-organization "platform bucket" no longer exists. Only a database built + * before 18.x still holds a nullable tenant column (`asPre18TenantColumn`), + * and there NULL-organization rows are exempt from the plain index — SQL + * UNIQUE is NULL-distinct. * - * The contract now (ADR-0120 D1/D3; scope is a vocabulary, not a position): + * The contract now (ADR-0120 D1 vocabulary; ADR-0131 D1 key shape): * * field `unique: true` / `'organization'` + tenant column - * → composite `(COALESCE(tenantField, '__global__'), field)` — unique per - * organization; NULL-organization rows form one platform bucket, unique - * among themselves (#5030). + * → composite `(tenantField, field)` — unique per organization, the + * tenant key part plain (no COALESCE). * field `unique: true` / `'organization'`, no tenant column * → single-column `(field)`. * field `unique: 'global'` @@ -40,16 +45,17 @@ import { asPre18TenantColumn } from './pre-18-tenant-column.testkit.js'; * The old "`'global'` is a synonym of `true`" pin retired with the bare * spelling. * declared index `unique: 'organization'` - * → NULL-safe organization key part PREPENDED to the listed columns; with + * → the plain organization column PREPENDED to the listed columns; with * no tenant column it degrades to the listed columns (S11); a listed - * tenant column is made NULL-safe in place instead (S6 respelling). + * tenant column is kept in place, never prepended twice (S6 respelling). * * Retiring the legacy global index is no longer inline DDL at boot: since * #3728 it is a `replace_unique_index` drift entry, auto-applied on restart * only under the dev `autoMigrate: 'safe'` policy and otherwise left to - * `os migrate`. The #5030 tightening (bare composite → NULL-safe composite) - * goes through the same ceremony as `recreate_index`, gated by the ADR-0120 D4 - * duplicate pre-flight probe. The migration tests below opt into that policy. + * `os migrate`. The ADR-0120 D4 tightening (bare composite → NULL-safe + * composite) is retired with the COALESCE expectation: a bare composite IS the + * expected shape now, so no op is proposed (ADR-0131 D1). The migration tests + * below opt into that policy. */ describe('SqlDriver unique × tenancy (#3696)', () => { let driver: SqlDriver; @@ -86,9 +92,10 @@ describe('SqlDriver unique × tenancy (#3696)', () => { /** * Unique index name → canonical key parts, read from the index DDL so * expression keys are visible (`PRAGMA index_info` reports a NULL name for - * them). A plain column reads as its name; the NULL-safe organization key - * part (ADR-0120 D3) reads as `COALESCE()` — literal elided, the - * same literal-agnostic identity the drift differ compares on. + * them). A plain column reads as its name; a legacy NULL-safe organization + * key part (ADR-0120 D3) would read as `COALESCE()` — literal + * elided, the same literal-agnostic identity the drift differ compares on — + * which is how the pins below prove new DDL carries none (ADR-0131 D1). */ async function uniqueIndexColumns(table: string): Promise> { const k = (driver as any).knex; @@ -182,7 +189,7 @@ describe('SqlDriver unique × tenancy (#3696)', () => { // ── Physical shape ──────────────────────────────────────────────────────── - it('materializes a COMPOSITE (tenant, field) unique index, NULL-safe tenant key part first', async () => { + it('materializes a COMPOSITE (tenant, field) unique index, PLAIN tenant key part first (ADR-0131 D1)', async () => { await driver.initObjects([ { name: 'product', @@ -194,21 +201,21 @@ describe('SqlDriver unique × tenancy (#3696)', () => { ]); const uniques = await uniqueIndexColumns('product'); - expect(Object.values(uniques)).toContainEqual(['COALESCE(organization_id)', 'code']); - // No leftover single-column global unique on `code`, and no bare - // (NULL-distinct — i.e. void on NULL-org rows, #5030) composite. + expect(Object.values(uniques)).toContainEqual(['organization_id', 'code']); + // No leftover single-column global unique on `code`, and no retired + // NULL-safe (ADR-0120 D3) composite. expect(Object.values(uniques)).not.toContainEqual(['code']); - expect(Object.values(uniques)).not.toContainEqual(['organization_id', 'code']); + expect(Object.values(uniques)).not.toContainEqual(['COALESCE(organization_id)', 'code']); - // The literal is pinned too (ADR-0120 D3, maintainer-resolved): the - // constraint-violation error must read as "the platform bucket collided" - // — `('__global__', …)` — and the word must match the autonumber sequence - // table's GLOBAL_TENANT, not a bare ''. + // The DDL itself carries no expression key (ADR-0131 D1): the tenant + // column is NOT NULL on a fresh table, so the plain column is the whole + // per-organization key — no '__global__' platform bucket is built. const k = (driver as any).knex; const master: any = await k.raw( `SELECT sql FROM sqlite_master WHERE type = 'index' AND name = 'uniq_product_organization_id_code'`, ); - expect(String(master[0]?.sql)).toMatch(/COALESCE\([`"]?organization_id[`"]?, '__global__'\)/i); + expect(String(master[0]?.sql)).toMatch(/\(\s*[`"]?organization_id[`"]?\s*,\s*[`"]?code[`"]?\s*\)/i); + expect(String(master[0]?.sql)).not.toMatch(/COALESCE|__global__/i); }); it('materializes a SINGLE-column unique when the object has no tenant column', async () => { @@ -275,7 +282,7 @@ describe('SqlDriver unique × tenancy (#3696)', () => { ]); const uniques = await uniqueIndexColumns('env_setting'); - expect(Object.values(uniques)).toContainEqual(['COALESCE(environment_id)', 'key']); + expect(Object.values(uniques)).toContainEqual(['environment_id', 'key']); await driver.create('env_setting', { environment_id: 'env_a', key: 'k' }); const other = await driver.create('env_setting', { environment_id: 'env_b', key: 'k' }); @@ -447,7 +454,8 @@ describe('SqlDriver unique × tenancy (#3696)', () => { const uniques = await uniqueIndexColumns('product'); expect(uniques['product_code_unique']).toBeUndefined(); - expect(Object.values(uniques)).toContainEqual(['COALESCE(organization_id)', 'code']); + // The replacement is the plain composite (ADR-0131 D1). + expect(Object.values(uniques)).toContainEqual(['organization_id', 'code']); // Existing data survived, and the cross-tenant insert now works. expect(await driver.count('product', {})).toBe(2); @@ -477,7 +485,7 @@ describe('SqlDriver unique × tenancy (#3696)', () => { const uniques = await uniqueIndexColumns('product'); expect(uniques['uniq_product_code']).toBeUndefined(); - expect(Object.values(uniques)).toContainEqual(['COALESCE(organization_id)', 'code']); + expect(Object.values(uniques)).toContainEqual(['organization_id', 'code']); }); it("does NOT retire the single-column index of a unique: 'global' field", async () => { @@ -548,9 +556,10 @@ describe('SqlDriver unique × tenancy (#3696)', () => { ]); const uniques = await uniqueIndexColumns('product'); - expect(Object.values(uniques)).toContainEqual(['COALESCE(organization_id)', 'code']); + // The rebuild lands on the same plain composite as `initObjects` (ADR-0131 D1). + expect(Object.values(uniques)).toContainEqual(['organization_id', 'code']); expect(Object.values(uniques)).not.toContainEqual(['code']); - expect(Object.values(uniques)).not.toContainEqual(['organization_id', 'code']); + expect(Object.values(uniques)).not.toContainEqual(['COALESCE(organization_id)', 'code']); // Data preserved, and cross-tenant reuse still works post-rebuild. expect(await driver.count('product', {})).toBe(1); @@ -558,14 +567,14 @@ describe('SqlDriver unique × tenancy (#3696)', () => { expect(b.code).toBe('C1'); }); - // ── ADR-0120 D3: the NULL-organization bucket is constrained (#5030) ────── + // ── ADR-0131 D1: the NULL-organization bucket is retired (#5030) ────────── - describe('NULL-safe organization uniqueness (ADR-0120 D3, #5030)', () => { - it('#5030 regression: unique IS enforced when organization_id exists but is NULL on every row', async () => { - // The measured defect shape, verbatim from the issue: the kernel injects - // `organization_id` on single-tenant stacks too, every row leaves it - // NULL, and the bare NULL-distinct composite admitted any number of - // duplicates — field-level `unique: true` enforced NOTHING. + describe('NULL organization under the plain key part (ADR-0131 D1, #5030)', () => { + it('#5030 closed at the column: a fresh tenant column refuses a NULL organization outright', async () => { + // The measured #5030 shape — the kernel injects `organization_id` and a + // row leaves it NULL, slipping past a NULL-distinct composite — cannot + // be written on a fresh table: the tenant column is NOT NULL (ADR-0131 + // D1), so the plain composite has no NULL rows to be void on. await driver.initObjects([ { name: 'account', @@ -576,17 +585,13 @@ describe('SqlDriver unique × tenancy (#3696)', () => { }, ]); - // Rows with no organization exist only on a database built before 18.x - // (ADR-0131 D1 makes a fresh tenant column NOT NULL). - await asPre18TenantColumn(driver, 'account'); - const first = await driver.create('account', { email: 'dup@example.com' }); - expect(first.organization_id ?? null).toBeNull(); await expect(driver.create('account', { email: 'dup@example.com' })).rejects.toThrow( - /UNIQUE constraint failed|duplicate key value/, + /NOT NULL constraint failed|null value in column/, ); + expect(await driver.count('account', {})).toBe(0); }); - it('the NULL bucket and organizations coexist: one platform holder, plus one per organization (S8)', async () => { + it('on a pre-18 nullable column, NULL-organization rows are exempt; organizations stay unique (S8)', async () => { await driver.initObjects([ { name: 'template', @@ -597,7 +602,6 @@ describe('SqlDriver unique × tenancy (#3696)', () => { }, ]); - // Platform (NULL-organization) row takes the value… // Rows with no organization exist only on a database built before 18.x // (ADR-0131 D1 makes a fresh tenant column NOT NULL). await asPre18TenantColumn(driver, 'template'); @@ -605,9 +609,14 @@ describe('SqlDriver unique × tenancy (#3696)', () => { // …an organization may still hold the same value (per-org scope)… const org = await driver.create('template', { organization_id: 'org_a', key: 'welcome' }); expect(org.key).toBe('welcome'); - // …but a SECOND platform row may not: NULL rows are one bucket, unique - // among themselves. - await expect(driver.create('template', { key: 'welcome' })).rejects.toThrow( + // …a SECOND NULL-organization row is admitted too: the plain key part + // is NULL-distinct, and ADR-0131 D1 retired the ADR-0120 D3 platform + // bucket rather than rebuilding it… + const second = await driver.create('template', { key: 'welcome' }); + expect(second.key).toBe('welcome'); + expect(await driver.count('template', { where: { key: 'welcome' } })).toBe(3); + // …while the organization itself is still held to one row per value. + await expect(driver.create('template', { organization_id: 'org_a', key: 'welcome' })).rejects.toThrow( /UNIQUE constraint failed|duplicate key value/, ); }); @@ -623,8 +632,9 @@ describe('SqlDriver unique × tenancy (#3696)', () => { } as any, ]); + // Byte-for-byte the `unique: true` shape: the plain composite (ADR-0131 D1). const uniques = await uniqueIndexColumns('contact'); - expect(Object.values(uniques)).toContainEqual(['COALESCE(organization_id)', 'email']); + expect(Object.values(uniques)).toContainEqual(['organization_id', 'email']); await driver.create('contact', { organization_id: 'org_a', email: 'a@x.test' }); const other = await driver.create('contact', { organization_id: 'org_b', email: 'a@x.test' }); @@ -638,7 +648,7 @@ describe('SqlDriver unique × tenancy (#3696)', () => { // ── ADR-0120 D1: declared-index `unique: 'organization'` ────────────────── describe("declared-index unique: 'organization' (ADR-0120 D1)", () => { - it('prepends the NULL-safe organization key part to the listed columns', async () => { + it('prepends the PLAIN organization column to the listed columns (ADR-0131 D1)', async () => { await driver.initObjects([ { name: 'case_record', @@ -653,13 +663,14 @@ describe('SqlDriver unique × tenancy (#3696)', () => { const uniques = await uniqueIndexColumns('case_record'); expect(uniques['uniq_case_record_organization_id_department_code']).toEqual([ - 'COALESCE(organization_id)', + 'organization_id', 'department', 'code', ]); // Per-organization semantics: two orgs may hold the same (department, - // code); the same org may not; the NULL bucket is constrained too. + // code); the same org may not; NULL-organization rows (pre-18 only) + // are exempt — the plain key part is NULL-distinct (ADR-0131 D1). await driver.create('case_record', { organization_id: 'org_a', department: 'sales', code: 'C-1' }); await driver.create('case_record', { organization_id: 'org_b', department: 'sales', code: 'C-1' }); await expect( @@ -669,8 +680,11 @@ describe('SqlDriver unique × tenancy (#3696)', () => { // (ADR-0131 D1 makes a fresh tenant column NOT NULL). await asPre18TenantColumn(driver, 'case_record'); await driver.create('case_record', { department: 'sales', code: 'C-2' }); + const nullOrgTwin = await driver.create('case_record', { department: 'sales', code: 'C-2' }); + expect(nullOrgTwin.code).toBe('C-2'); + // The relaxation kept the index exactly as built: org_a is still refused. await expect( - driver.create('case_record', { department: 'sales', code: 'C-2' }), + driver.create('case_record', { organization_id: 'org_a', department: 'sales', code: 'C-1' }), ).rejects.toThrow(/UNIQUE constraint failed|duplicate key value/); }); @@ -687,11 +701,11 @@ describe('SqlDriver unique × tenancy (#3696)', () => { expect(uniques['uniq_plain_catalog_department_code']).toEqual(['department', 'code']); }); - it('makes a LISTED tenant column NULL-safe in place — the S6 respelling, not double-prepended', async () => { + it('keeps a LISTED tenant column plain in place — the S6 respelling, not double-prepended', async () => { // The hand-written legacy shape, opted into the explicit vocabulary: // the author already listed the tenant column, so the org key part is - // not prepended again — the listed column's own key part becomes the - // NULL-safe form. + // not prepended again — and since ADR-0131 D1 the listed column stays + // the plain column, no longer rewritten to the NULL-safe form. await driver.initObjects([ { name: 'team', @@ -701,24 +715,26 @@ describe('SqlDriver unique × tenancy (#3696)', () => { ]); const uniques = await uniqueIndexColumns('team'); - expect(uniques['uniq_team_organization_id_name']).toEqual([ - 'COALESCE(organization_id)', - 'name', - ]); + expect(uniques['uniq_team_organization_id_name']).toEqual(['organization_id', 'name']); + + await driver.create('team', { organization_id: 'org_a', name: 'Core' }); + await expect(driver.create('team', { organization_id: 'org_a', name: 'Core' })).rejects.toThrow( + /UNIQUE constraint failed|duplicate key value/, + ); // Rows with no organization exist only on a database built before 18.x - // (ADR-0131 D1 makes a fresh tenant column NOT NULL). + // (ADR-0131 D1 makes a fresh tenant column NOT NULL); there they are + // exempt from the plain key part. await asPre18TenantColumn(driver, 'team'); await driver.create('team', { name: 'Core' }); - await expect(driver.create('team', { name: 'Core' })).rejects.toThrow( - /UNIQUE constraint failed|duplicate key value/, - ); + const twin = await driver.create('team', { name: 'Core' }); + expect(twin.name).toBe('Core'); }); }); - // ── ADR-0120 D4: the tightening goes through the ceremony ───────────────── + // ── ADR-0120 D4 retired: a bare composite IS the expectation (ADR-0131 D1) ─ - describe('bare-composite tightening + duplicate pre-flight (ADR-0120 D4)', () => { + describe('bare composite from a pre-18 database — no tightening proposed (ADR-0131 D1)', () => { /** A pre-ADR-0120 database: the bare NULL-distinct composite, in place. */ const seedBareComposite = async () => { const k = (driver as any).knex; @@ -745,11 +761,13 @@ describe('SqlDriver unique × tenancy (#3696)', () => { }, ]; - it("auto-tightens at boot under autoMigrate: 'safe' when the probe is clean", async () => { + it("boots under autoMigrate: 'safe' with the bare composite untouched and ZERO drift", async () => { + // ADR-0120 D4 used to propose `recreate_index` (bare → COALESCE) here. + // The expectation is the plain composite now, which this index already + // is — so there is nothing to tighten. await driver.disconnect(); driver = makeDriver({ autoMigrate: 'safe' }); const k = await seedBareComposite(); - // Clean data: distinct codes, including one NULL-organization row. await k('product').insert([ { id: 'r1', organization_id: 'org_a', code: 'C1' }, { id: 'r2', organization_id: null, code: 'C1' }, @@ -758,75 +776,38 @@ describe('SqlDriver unique × tenancy (#3696)', () => { await driver.initObjects(productMeta); const uniques = await uniqueIndexColumns('product'); - expect(uniques['uniq_product_organization_id_code']).toEqual([ - 'COALESCE(organization_id)', - 'code', - ]); - // Healthy database: converged, zero drift. + expect(uniques['uniq_product_organization_id_code']).toEqual(['organization_id', 'code']); expect(await driver.detectManagedDrift()).toHaveLength(0); - // The NULL bucket is now enforced: a second platform-bucket C1 is refused. - await expect(driver.create('product', { code: 'C1' })).rejects.toThrow( + // Per organization still enforced; a NULL-organization row is exempt. + await expect(driver.create('product', { organization_id: 'org_a', code: 'C1' })).rejects.toThrow( /UNIQUE constraint failed|duplicate key value/, ); + const nullOrg = await driver.create('product', { code: 'C1' }); + expect(nullOrg.code).toBe('C1'); }); - it('BLOCKS the tightening on duplicate NULL-organization rows — old index kept, rows reported', async () => { + it('duplicate NULL-organization rows are no longer a blocker — no op proposed, rows and index kept', async () => { + // The data that used to BLOCK the D4 tightening: duplicates the + // NULL-distinct index admitted. Under the plain expectation they are + // not a violation of anything the driver asks for. await driver.disconnect(); driver = makeDriver({ autoMigrate: 'safe' }); const k = await seedBareComposite(); - // The #5030 defect made data: duplicates the NULL-distinct index admitted. await k('product').insert([ { id: 'r1', organization_id: null, code: 'DUP' }, { id: 'r2', organization_id: null, code: 'DUP' }, ]); - // Boot survives — the tightening is not applied, the old index stays. await driver.initObjects(productMeta); expect((await uniqueIndexColumns('product'))['uniq_product_organization_id_code']).toEqual([ 'organization_id', 'code', ]); - // The plan reports the op as BLOCKED, with the offending group named — - // and the COALESCE key makes the report self-describing ('__global__' = - // the platform bucket). const drift = await driver.detectManagedDrift(); - const entry = drift.find((d) => d.op.type === 'recreate_index'); - expect(entry).toBeDefined(); - expect(entry!.category).toBe('destructive'); - expect(entry!.severity).toBe('error'); - expect(entry!.message).toMatch(/BLOCKED/); - expect(entry!.message).toMatch(/__global__/); - expect(entry!.message).toMatch(/"DUP"/); - expect(entry!.message).toMatch(/NULL-distinct/); - - // Even `--allow-destructive` cannot force it: apply re-probes and - // refuses — at no point is a constraint dropped without its replacement - // being creatable. - const { applied, skipped } = await driver.applyMigrationEntries([entry!], { - allowDestructive: true, - }); - expect(applied).toHaveLength(0); - expect(skipped).toHaveLength(1); - expect((await uniqueIndexColumns('product'))['uniq_product_organization_id_code']).toEqual([ - 'organization_id', - 'code', - ]); - - // Deduplicating unblocks the op: the probe comes back clean, the entry - // re-grades `safe`, and a plain apply tightens the index. - await k('product').where({ id: 'r2' }).delete(); - const drift2 = await driver.detectManagedDrift(); - const entry2 = drift2.find((d) => d.op.type === 'recreate_index'); - expect(entry2).toBeDefined(); - expect(entry2!.category).toBe('safe'); - const res2 = await driver.applyMigrationEntries([entry2!], { allowDestructive: false }); - expect(res2.applied).toHaveLength(1); - expect((await uniqueIndexColumns('product'))['uniq_product_organization_id_code']).toEqual([ - 'COALESCE(organization_id)', - 'code', - ]); - expect(await driver.detectManagedDrift()).toHaveLength(0); + expect(drift.filter((d) => d.op.type === 'recreate_index')).toEqual([]); + expect(drift).toHaveLength(0); + expect(await driver.count('product', {})).toBe(2); }); }); }); diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index af50e57ea66..5fb03c41916 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -120,6 +120,7 @@ import { fieldHasColumn, GLOBAL_TENANT, canonicalIndexKey, + carriesNullSafeTenantKeyPart, hashShadowColumnFor, isHashShadowCarrier, parseHashShadowKeyParts, @@ -191,8 +192,9 @@ import { isComposedReadableIds, type ComposedReadableIds } from '@objectstack/sp * no columns. So the refusal is the ordinary state of a new database, and not * a `DATABASE_ERROR` to put in front of an operator. * - * Only one question reads through this scope, and only - * {@link SqlDriver.backendStatementFault} consults it. ⛔ It demotes one class, + * Two questions read through this scope — that one, and the ADR-0131 D1 + * certification {@link SqlDriver.isTenantKeyCertified} asks of the same + * ledger — and only {@link SqlDriver.backendStatementFault} consults it. ⛔ It demotes one class, * a missing table, recognised by the shared `isMissingTableError`. Any other * refusal inside the scope still warns, and so does a missing table outside it. * @@ -202,6 +204,21 @@ import { isComposedReadableIds, type ComposedReadableIds } from '@objectstack/sp */ const PRE_DDL_QUESTION_SCOPE = new AsyncLocalStorage(); +/** + * [ADR-0131 D1] The `WHERE` of a unique key's duplicate probe: every plain key + * part NOT NULL, so the probe groups only the rows the index constrains (SQL + * UNIQUE is NULL-distinct). Empty when every part is NULL-safe. The Turso + * remote face spells the same clause with its own identifier quoting. + */ +function uniqueProbeNotNullClause( + columns: readonly string[], + nullSafe: ReadonlySet, + quote: (identifier: string) => string, +): string { + const plain = columns.filter((c) => !nullSafe.has(c)); + return plain.length === 0 ? '' : ` WHERE ${plain.map((c) => `${quote(c)} IS NOT NULL`).join(' AND ')}`; +} + /** * Default ID length for auto-generated IDs. */ @@ -13974,7 +13991,19 @@ export class SqlDriver implements IDataDriver { // NOT NULL is the organization-ownership ceremony's, not drift. ceremonyOwnedNullability: this.resolveTenantField(tableName), }); - out.push(...(await this.detectTableIndexDrift(tableName, fields, declaredIndexes, new Set(cols.map((c) => c.name))))); + // [ADR-0131 D1] A physically NOT NULL tenant column is its own proof that + // the NULL-safe and the plain tenant key part are one constraint here. + const tenantField = this.resolveTenantField(tableName); + const tenantColumnNotNull = !!tenantField && cols.some((c) => c.name === tenantField && c.nullable === false); + out.push( + ...(await this.detectTableIndexDrift( + tableName, + fields, + declaredIndexes, + new Set(cols.map((c) => c.name)), + tenantColumnNotNull, + )), + ); return out; } @@ -13985,27 +14014,39 @@ export class SqlDriver implements IDataDriver { * introspection round-trip (indexes, not columns) and two inputs the column * differ has no use for: the object's declared `indexes[]` and its tenant * column. + * + * [ADR-0131 D1] `tenantColumnNotNull` and {@link isTenantKeyCertified} are + * the two witnesses that the tenant column holds no NULL here, which is what + * lets the differ read an earlier release's `COALESCE(, …)` key part + * as the plain column new DDL keys. The certification is asked only when a + * physical index carries that form and the column is not NOT NULL already. */ protected async detectTableIndexDrift( tableName: string, fields: Record, declaredIndexes: any[] | undefined, physicalColumns: Set, + tenantColumnNotNull = false, ): Promise { const tenantField = this.resolveTenantField(tableName); + const physical = await this.introspectIndexes(tableName); + const tenantKeyCertified = + tenantColumnNotNull || + (carriesNullSafeTenantKeyPart(physical, tenantField) && (await this.isTenantKeyCertified())); const entries = diffManagedIndexes({ table: tableName, expected: expectedIndexes({ table: tableName, fields, tenantField, declaredIndexes, physicalColumns }), // `declaredIndexes` goes to BOTH: it is what the table should have, and // therefore also what must never be mistaken for legacy debt (#3955). legacy: legacyUniqueReplacements({ table: tableName, fields, tenantField, physicalColumns, declaredIndexes }), - physical: await this.introspectIndexes(tableName), + physical, // Indexes the framework built through raw DDL on this boot are its own to // manage — never this differ's to propose dropping (#4884). runtimeCreated: this.runtimeCreatedIndexes.get(tableName), // Lets the differ recognise the NULL-safe organization key part as the // sync's own vocabulary (ADR-0120 D3). tenantField, + tenantKeyCertified, }); // ADR-0120 D4: the duplicate pre-flight probe decides each NULL-safe // unique op's fate — clean data upgrades the pure tightening to `safe` @@ -14124,10 +14165,17 @@ export class SqlDriver implements IDataDriver { } /** - * Find duplicate groups under a NULL-safe organization unique key: GROUP BY - * the exact key the index will enforce — `COALESCE(, '__global__')` for - * the NULL-safe parts, the bare column otherwise — HAVING COUNT(*) > 1. - * Returns one entry per conflicting group, `key` naming columns and values. + * Find duplicate groups under a unique key: GROUP BY the exact key the index + * will enforce — `COALESCE(, '__global__')` for the NULL-safe parts, + * the bare column otherwise — HAVING COUNT(*) > 1. Returns one entry per + * conflicting group, `key` naming columns and values. + * + * [ADR-0131 D1] A row with a NULL in a plain key part conflicts with nothing + * under SQL's NULL-distinct UNIQUE, while GROUP BY puts NULLs together, so + * such rows are left out ({@link uniqueProbeNotNullClause}). Since new DDL + * keys the plain tenant column, that is every organization-less row a + * nullable tenant column still holds — a group of them is no duplicate the + * index will refuse. */ protected async probeNullSafeUniqueDuplicates( table: string, @@ -14142,7 +14190,8 @@ export class SqlDriver implements IDataDriver { const selectList = parts.map((p, i) => `${p} AS k${i}`).join(', '); const groupList = parts.join(', '); const sql = - `SELECT ${selectList}, COUNT(*) AS n FROM ${q(table)} ` + + `SELECT ${selectList}, COUNT(*) AS n FROM ${q(table)}` + + `${uniqueProbeNotNullClause(columns, ns, q)} ` + `GROUP BY ${groupList} HAVING COUNT(*) > 1 ORDER BY n DESC LIMIT 20`; const res: any = await this.knex.raw(sql); const rows: any[] = Array.isArray(res) ? (Array.isArray(res[0]) ? res[0] : res) : (res?.rows ?? []); @@ -18903,6 +18952,14 @@ export class SqlDriver implements IDataDriver { * the CONCAT-NULL semantics above: an any-NULL tuple still conflicts with * nothing, matching MySQL's own composite-UNIQUE behaviour. * + * Since ADR-0131 D1 no generator marks the tenant key part NULL-safe, so a + * NEW shadow hashes the plain tenant column, which is what a direct index + * would key. An existing shadow is never rebuilt for it: the additive sync + * skips its index by name, and the differ reads its resolved COALESCE key + * as the plain one on a certified table. Only a shadow whose index is + * already gone (a `recreate_index`, a manual drop) meets the new key here, + * as `stale`, and its column is derived, so regenerating it moves no data. + * * Returns `true` when the shadow index now exists. */ protected async createHashShadowUniqueIndex( @@ -20481,6 +20538,70 @@ export class SqlDriver implements IDataDriver { /** Has {@link resolveFileColumnsMoved} already run to completion? */ private fileColumnsMovedAsked = false; + /** + * [ADR-0131 D1] The kernel→driver supply seam for "is this database + * certified": a birth marker, or an organization-ownership ceremony marker + * of the current shape generation. `ObjectQL.registerDriver` hands over a + * closure over the engine's `isOrganizationOwnershipCertified()`, which reads + * the ceremony's `sys_migration` row — a row in a table this driver serves, + * read through the registry and the pipeline the engine owns, so the driver + * receives the question, never the ledger's vocabulary. + * + * The answer decides one thing only: whether the differ reads an earlier + * release's `COALESCE(, …)` key part as the plain tenant column new + * DDL keys ({@link detectTableIndexDrift}). Without a resolver, or when it + * throws, the answer is "not certified": the two forms then stay distinct + * constraints, and the differ REPORTS the 18.0 index as a destructive + * `recreate_index`. That report is never applied unattended — the boot + * reconcile applies `safe` entries only, the artifact gate refuses it, and + * `os migrate apply` needs `--allow-destructive` — and its message names the + * ownership ceremony as the remedy. + */ + setTenantKeyCertifiedResolver(resolve: () => boolean | Promise): void { + this.tenantKeyCertifiedResolver = resolve; + this.tenantKeyCertified = undefined; + } + + private tenantKeyCertifiedResolver?: () => boolean | Promise; + /** Set once the resolver has answered yes — a certificate is never withdrawn. */ + private tenantKeyCertified?: true; + /** The resolver's failure is reported once per driver. */ + private tenantKeyCertifiedReadFailureReported = false; + + /** + * Asked lazily — only a table carrying the NULL-safe tenant key part needs + * it. A yes is kept; a no is asked again next time, so an answer taken + * before the ledger could be read never outlives that moment. + */ + protected async isTenantKeyCertified(): Promise { + if (this.tenantKeyCertified) return true; + const resolver = this.tenantKeyCertifiedResolver; + if (!resolver) return false; + try { + // Asked inside the pre-DDL scope: a store whose ledger does not exist + // yet answers with a refused read, which is not a fault (see + // {@link PRE_DDL_QUESTION_SCOPE}). + if ((await PRE_DDL_QUESTION_SCOPE.run(true, resolver)) !== true) return false; + } catch (e: any) { + // Not knowing reads as "not certified": the forms stay distinct and an + // 18.0 index is reported as a destructive recreate_index, which nothing + // applies unattended (safe-only boot reconcile, the artifact gate, + // `--allow-destructive`). Said once, so that report is not taken for a + // verdict on a certified store. + if (!this.tenantKeyCertifiedReadFailureReported) { + this.tenantKeyCertifiedReadFailureReported = true; + this.logger.warn( + `[schema-drift] could not read whether this database is certified for the organization-ownership shape ` + + `(${e?.message ?? e}); an index keyed COALESCE(, …) on a nullable tenant column is compared ` + + `as a different constraint from the plain key until it can be read.`, + ); + } + return false; + } + this.tenantKeyCertified = true; + return true; + } + protected async resolveFileColumnsMoved(): Promise { const resolver = this.fileColumnsMovedResolver; if (!resolver) { diff --git a/packages/drivers/driver-turso/src/turso-driver.ts b/packages/drivers/driver-turso/src/turso-driver.ts index cb53a3effd5..6515a301fe6 100644 --- a/packages/drivers/driver-turso/src/turso-driver.ts +++ b/packages/drivers/driver-turso/src/turso-driver.ts @@ -1099,6 +1099,11 @@ export const REMOTE_FACE_ANSWERS = { // `false`, the method's own "not taken" answer: only the Knex `initObjects` // asks the resolver, so on this face it would never be asked. setFileColumnsMovedResolver: 'remote', + // [ADR-0131 D1] In-memory bookkeeping with no Knex: the resolver is asked by + // the shared index differ, which the remote drift detector runs, so the + // remote face reads an earlier release's COALESCE tenant key part as the + // plain one on a certified database exactly as the local face does. + setTenantKeyCertifiedResolver: 'inherited', // `false`: remote mode opens no in-memory database. sqliteOpenedEmptyInMemory: 'inherited', // `'sqlite'`: libSQL speaks SQLite, and `execute()` sends what a caller @@ -4227,8 +4232,12 @@ export class TursoDriver extends SqlDriver { const parts = columns.map((c) => nullSafe.has(c) ? `COALESCE(${quoteRemoteIdentifier(c)}, '${GLOBAL_TENANT}')` : quoteRemoteIdentifier(c), ); + // [ADR-0131 D1] A row with a NULL plain key part conflicts with nothing + // under the NULL-distinct UNIQUE; the local face leaves it out the same way. + const notNull = columns.filter((c) => !nullSafe.has(c)).map((c) => `${quoteRemoteIdentifier(c)} IS NOT NULL`); const sql = - `SELECT ${parts.map((p, i) => `${p} AS k${i}`).join(', ')}, COUNT(*) AS n FROM ${quoteRemoteIdentifier(table)} ` + + `SELECT ${parts.map((p, i) => `${p} AS k${i}`).join(', ')}, COUNT(*) AS n FROM ${quoteRemoteIdentifier(table)}` + + `${notNull.length > 0 ? ` WHERE ${notNull.join(' AND ')}` : ''} ` + `GROUP BY ${parts.join(', ')} HAVING COUNT(*) > 1 ORDER BY n DESC LIMIT 20`; return (await this.remoteCatalogRead(sql)).map((r) => ({ key: columns.map((c, i) => `${c}=${JSON.stringify(r[`k${i}`])}`).join(', '), diff --git a/packages/drivers/driver-turso/src/turso-local-remote-declared-index-parity.test.ts b/packages/drivers/driver-turso/src/turso-local-remote-declared-index-parity.test.ts index 82c2c69526b..bf20fc638b6 100644 --- a/packages/drivers/driver-turso/src/turso-local-remote-declared-index-parity.test.ts +++ b/packages/drivers/driver-turso/src/turso-local-remote-declared-index-parity.test.ts @@ -23,8 +23,9 @@ * per-face suite cannot fail on the DIFFERENCE between faces, and the * difference is the defect. So each pin here drives one object set through * BOTH faces and compares what physically landed — index names, uniqueness, - * origin and key parts (NULL-safe organization expressions included) — rather - * than asserting either face against a literal. Each parity pin is ANCHORED + * origin and key parts (the organization key part included — PLAIN since + * ADR-0131 D1, so a face that still emitted a COALESCE expression would show + * here) — rather than asserting either face against a literal. Each parity pin is ANCHORED * by names computed through the shared `buildIndexName`, because two faces * that both lost every index would agree perfectly. * @@ -39,7 +40,8 @@ * `indexes` verbatim — because this package does not (and should not) depend * on `service-messaging` or `platform-objects`. `os17609_scoped` covers what * those two do not: a tenant column, a field-level `unique` scoped by it, a - * declared `unique: 'organization'` (NULL-safe key part), an explicit + * declared `unique: 'organization'` (plain organization key part, ADR-0131 + * D1), an explicit * `unique: 'global'`, two author-named indexes — one of them no SQL * identifier at all (a space and a double quote), which both faces must * accept — and an index over a virtual `formula` field that neither face may @@ -374,10 +376,15 @@ describe('[#17609] declared object-level indexes land identically on both TursoD expect(localShapes.map((i) => i.name), name).toEqual(EXPECTED_NAMES[name]); } - // The tenant key part is the NULL-safe expression on both faces, not the bare column. + // ADR-0131 D1: the tenant key part is the PLAIN column on both faces — no + // COALESCE expression — for the declared `unique: 'organization'` and the + // field-level `unique` alike (the remote face's agreement is pinned above). const scoped = await indexShapes(remote.query, SCOPED.name); expect(scoped.find((i) => i.name === buildIndexName(SCOPED.name, ['organization_id', 'slug'], true))).toEqual( - expect.objectContaining({ unique: true, keys: ["COALESCE(organization_id, '__global__')", 'slug'] }), + expect.objectContaining({ unique: true, keys: ['organization_id', 'slug'] }), + ); + expect(scoped.find((i) => i.name === buildIndexName(SCOPED.name, ['organization_id', 'code'], true))).toEqual( + expect.objectContaining({ unique: true, keys: ['organization_id', 'code'] }), ); }); diff --git a/packages/drivers/driver-turso/src/turso-remote-drift-detection.test.ts b/packages/drivers/driver-turso/src/turso-remote-drift-detection.test.ts index 81d10c3c9dc..66645352283 100644 --- a/packages/drivers/driver-turso/src/turso-remote-drift-detection.test.ts +++ b/packages/drivers/driver-turso/src/turso-remote-drift-detection.test.ts @@ -27,9 +27,12 @@ * nowhere, on either face. This is what the `os serve` boot gate reads: a * false finding here would refuse a healthy boot. * 2. Each drifted shape (an extra physical column, a declared index the table - * lacks, a bare organization key a NULL-safe unique must tighten, over - * clean and over duplicate rows) reads alike on both faces, with or without - * explicit objects. + * lacks, an 18.0 `COALESCE` organization key against the plain expectation + * on an uncertified nullable table, and a pre-normalized NULL-safe + * declaration over a bare key, over clean and over duplicate rows) reads + * alike on both faces, with or without explicit objects. On a certified + * database the 18.0 key reads as the plain one: no drift on either face + * (ADR-0131 D1). * 3. The DDL-free `registerObjectMetadata` (a `skipSchemaSync` boot) fills the * registry the no-argument call walks, and sends nothing to the database. * 4. A declared table the database does not hold is skipped, as locally. @@ -120,6 +123,21 @@ const TENANT = { }; const INDEXED = { name: 'indexed_t', fields: { name: { type: 'text' } }, indexes: [{ fields: ['name'] }] }; const ALL: Objects = [T, TENANT, INDEXED]; +const TENANT_UNIQUE = 'uniq_tenant_t_organization_id_code'; +/** What 18.0 built under the declared name: the ADR-0120 D3 NULL-safe key part. */ +const COALESCE_18_0 = `CREATE UNIQUE INDEX "${TENANT_UNIQUE}" ON "tenant_t" (COALESCE("organization_id", '__global__'), "code")`; +/** + * `tenant_t` with its unique declared ALREADY NORMALIZED, carrying the NULL-safe + * key part itself. ADR-0131 D1: the generators key the plain organization column + * now, so this is the one declaration that still yields the ADR-0120 D4 + * tightening (an input carrying `nullSafeColumns` is honoured verbatim). + */ +const TENANT_NULL_SAFE = { + name: 'tenant_t', + fields: { code: { type: 'text' }, label: { type: 'text' }, organization_id: { type: 'text' } }, + indexes: [{ name: TENANT_UNIQUE, fields: ['organization_id', 'code'], unique: true, nullSafeColumns: ['organization_id'] }], +}; +const ALL_NULL_SAFE: Objects = [T, TENANT_NULL_SAFE, INDEXED]; type Door = 'syncSchemasBatch' | 'syncSchema' | 'initObjects'; const SYNC: Record Promise> = { @@ -148,10 +166,12 @@ type Drift = Awaited>; describe('each drifted shape reads alike on the remote and the local face', () => { /** The physical drift each case puts on disk after a clean remote sync, and the finding it must produce. */ - // The fourth element: the case's rows carry no organization, which only a - // store built before 18.x can hold (ADR-0131 D1), so `tenant_t` is built in - // that shape first (`initObjectsPre18`). - const CASES: Array<[string, string[], (drift: Drift) => void, boolean?]> = [ + // The fourth element: the case needs a nullable tenant column — rows that + // carry no organization, or an uncertified table whose column could hold + // them — which only a store built before 18.x has (ADR-0131 D1), so + // `tenant_t` is built in that shape first (`initObjectsPre18`). The fifth: + // the declarations, when not `ALL`. + const CASES: Array<[string, string[], (drift: Drift) => void, boolean?, Objects?]> = [ [ 'an extra physical column the declaration omits', ['ALTER TABLE "t" ADD COLUMN "legacy" TEXT', 'CREATE INDEX "idx_t_legacy" ON "t" ("legacy")'], @@ -167,43 +187,80 @@ describe('each drifted shape reads alike on the remote and the local face', () = })), ], [ - 'a bare organization key a NULL-safe unique tightens, over clean rows', - [ - 'DROP INDEX "uniq_tenant_t_organization_id_code"', - 'CREATE UNIQUE INDEX "uniq_tenant_t_organization_id_code" ON "tenant_t" ("organization_id", "code")', - ], + // ADR-0131 D1: new DDL keys the plain column; on a nullable tenant column + // with no certification the 18.0 COALESCE key is a different constraint, + // so the rebuild is destructive (it is NOT the D4 tightening — the + // expectation carries no NULL-safe part). + 'an 18.0 COALESCE organization key against the plain expectation, on an uncertified nullable table', + [`DROP INDEX "${TENANT_UNIQUE}"`, COALESCE_18_0], + (drift) => { + const entry = drift.find((d) => d.table === 'tenant_t' && d.op.type === 'recreate_index'); + expect(entry).toEqual(expect.objectContaining({ + category: 'destructive', op: expect.objectContaining({ type: 'recreate_index', indexName: TENANT_UNIQUE }), + })); + expect(entry!.op).not.toHaveProperty('nullSafeColumns'); + expect(entry!.op).not.toHaveProperty('tightenNullSafeOnly'); + }, + true, + ], + [ + 'a pre-normalized NULL-safe declaration tightens a bare organization key, over clean rows', + [`DROP INDEX "${TENANT_UNIQUE}"`, `CREATE UNIQUE INDEX "${TENANT_UNIQUE}" ON "tenant_t" ("organization_id", "code")`], (drift) => expect(drift).toContainEqual(expect.objectContaining({ table: 'tenant_t', category: 'safe', op: expect.objectContaining({ type: 'recreate_index', tightenNullSafeOnly: true }), })), + true, + ALL_NULL_SAFE, ], [ 'the same tightening over duplicate rows the bare key admitted', [ - 'DROP INDEX "uniq_tenant_t_organization_id_code"', - 'CREATE UNIQUE INDEX "uniq_tenant_t_organization_id_code" ON "tenant_t" ("organization_id", "code")', + `DROP INDEX "${TENANT_UNIQUE}"`, + `CREATE UNIQUE INDEX "${TENANT_UNIQUE}" ON "tenant_t" ("organization_id", "code")`, `INSERT INTO "tenant_t" ("id", "code", "organization_id") VALUES ('a', 'X', NULL), ('b', 'X', NULL)`, ], (drift) => expect(drift).toContainEqual(expect.objectContaining({ - table: 'tenant_t', category: 'destructive', op: expect.objectContaining({ type: 'recreate_index' }), + table: 'tenant_t', category: 'destructive', op: expect.objectContaining({ type: 'recreate_index', tightenNullSafeOnly: true }), })), true, + ALL_NULL_SAFE, ], ]; - it.each(CASES)('%s', async (_label, statements, finding, pre18Tenant) => { + it.each(CASES)('%s', async (_label, statements, finding, pre18Tenant, objects = ALL) => { const file = nextFile(); const { driver: remote } = await remoteOver(file); - if (pre18Tenant) await initObjectsPre18(remote, TENANT); - await SYNC.syncSchemasBatch(remote, ALL); + if (pre18Tenant) await initObjectsPre18(remote, objects.find((o) => o.name === 'tenant_t')!); + await SYNC.syncSchemasBatch(remote, objects); for (const sql of statements) await onDisk(file, sql); - const local = await localOver(file, ALL); + const local = await localOver(file, objects); - const expected = await local.detectManagedDrift(ALL); + const expected = await local.detectManagedDrift(objects); // Non-vacuous: the local face sees the drift the case put on disk. finding(expected); - expect(await remote.detectManagedDrift(ALL)).toEqual(expected); + expect(await remote.detectManagedDrift(objects)).toEqual(expected); expect(await remote.detectManagedDrift()).toEqual(expected); }); + + it('the same 18.0 COALESCE key on a certified database reads as the plain one: no drift on either face', async () => { + // ADR-0131 D1 — the equivalence leg of the uncertified case above: same + // bytes on disk, the certification resolver answering yes on both faces. + const file = nextFile(); + const { driver: remote } = await remoteOver(file); + await initObjectsPre18(remote, TENANT); + await SYNC.syncSchemasBatch(remote, ALL); + for (const sql of [`DROP INDEX "${TENANT_UNIQUE}"`, COALESCE_18_0]) await onDisk(file, sql); + const local = await localOver(file, ALL); + // Non-vacuous: uncertified, both faces see the destructive rebuild. + expect((await local.detectManagedDrift(ALL)).map((d) => d.op.type)).toEqual(['recreate_index']); + expect((await remote.detectManagedDrift(ALL)).map((d) => d.op.type)).toEqual(['recreate_index']); + + local.setTenantKeyCertifiedResolver(() => true); + remote.setTenantKeyCertifiedResolver(() => true); + expect(await local.detectManagedDrift(ALL)).toEqual([]); + expect(await remote.detectManagedDrift(ALL)).toEqual([]); + expect(await remote.detectManagedDrift()).toEqual([]); + }); }); describe('the registry the no-argument call walks', () => { diff --git a/packages/drivers/driver-turso/src/turso-remote-migration-entries.test.ts b/packages/drivers/driver-turso/src/turso-remote-migration-entries.test.ts index 00d55e68e42..8d329658d7f 100644 --- a/packages/drivers/driver-turso/src/turso-remote-migration-entries.test.ts +++ b/packages/drivers/driver-turso/src/turso-remote-migration-entries.test.ts @@ -27,8 +27,11 @@ * and the indexes the same batch had dropped first are back: the batch is * one transaction. * 3. `drop_index`, `create_index` and `recreate_index` reach their target - * state; the NULL-safe tightening re-probes for duplicates and refuses over - * them, leaving the old index exactly as it was. + * state: an 18.0 `COALESCE` organization key on an uncertified nullable + * table is rebuilt to the plain key new DDL emits (ADR-0131 D1), and on a + * certified database is no entry at all; the NULL-safe tightening (reached + * now only through a pre-normalized declaration) re-probes for duplicates + * and refuses over them, leaving the old index exactly as it was. * 4. A column entry this transport cannot perform (it needs the SQLite table * rebuild) is skipped, not reported applied. * 5. Once the report is applied, the remote face reports no drift, and the @@ -165,6 +168,17 @@ const TENANT = { fields: { code: { type: 'text', unique: true }, organization_id: { type: 'text' } }, }; const TENANT_UNIQUE = 'uniq_tenant_t_organization_id_code'; +/** + * `tenant_t` with its unique declared ALREADY NORMALIZED, carrying the NULL-safe + * key part itself. ADR-0131 D1: the generators key the plain organization column + * now, so only an input carrying `nullSafeColumns` (honoured verbatim) still + * yields the ADR-0120 D4 tightening and its duplicate re-probe. + */ +const TENANT_NULL_SAFE = { + name: 'tenant_t', + fields: { code: { type: 'text' }, organization_id: { type: 'text' } }, + indexes: [{ name: TENANT_UNIQUE, fields: ['organization_id', 'code'], unique: true, nullSafeColumns: ['organization_id'] }], +}; describe('the index entries', () => { it('drop_index: an undeclared index is dropped, and a second run finds the step done', async () => { @@ -193,15 +207,57 @@ describe('the index entries', () => { expect(await indexesOf(file, 'indexed_t')).toEqual(synced); }); - /** A tenant table whose NULL-safe unique was left as the bare composite, optionally over duplicates. */ - async function bareTenantKey(duplicates: boolean) { + /** + * A tenant table as 18.0 left it: a nullable tenant column (built before + * 18.x, ADR-0131 D1) and the ADR-0120 D3 `COALESCE` key under the declared + * name, where HEAD's sync keys the plain column. + */ + async function coalescedTenantKey() { const file = nextFile(); const remote = await remoteOver(file); - // Duplicates the bare key admitted carry no organization, which only a - // store built before 18.x can hold (ADR-0131 D1). - if (duplicates) await initObjectsPre18(remote, TENANT); + await initObjectsPre18(remote, TENANT); await remote.syncSchemasBatch([{ object: 'tenant_t', schema: TENANT }]); const synced = await indexesOf(file, 'tenant_t'); + expect(synced[TENANT_UNIQUE]).not.toMatch(/COALESCE/i); + await onDisk( + file, + `DROP INDEX "${TENANT_UNIQUE}"`, + `CREATE UNIQUE INDEX "${TENANT_UNIQUE}" ON "tenant_t" (COALESCE("organization_id", '__global__'), "code")`, + ); + return { file, remote, synced }; + } + + it('recreate_index: an 18.0 COALESCE key on an uncertified nullable table is rebuilt to the plain key', async () => { + const { file, remote, synced } = await coalescedTenantKey(); + const recreate = (await remote.detectManagedDrift()).find((d) => d.op.type === 'recreate_index')!; + // Uncertified, the two forms are different constraints: a destructive rebuild, not the D4 tightening. + expect(recreate).toEqual(expect.objectContaining({ category: 'destructive' })); + expect(recreate.op).not.toHaveProperty('tightenNullSafeOnly'); + expect(await remote.applyMigrationEntries([recreate])).toEqual({ applied: [], skipped: [recreate] }); + expect(await remote.applyMigrationEntries([recreate], { allowDestructive: true })).toEqual({ applied: [recreate], skipped: [] }); + expect(await indexesOf(file, 'tenant_t')).toEqual(synced); + expect(await remote.detectManagedDrift()).toEqual([]); + }); + + it('a certified database reads the same 18.0 COALESCE key as the plain one: no entry, and nothing moves', async () => { + const { file, remote } = await coalescedTenantKey(); + const coalesced = await indexesOf(file, 'tenant_t'); + remote.setTenantKeyCertifiedResolver(() => true); + expect(await remote.detectManagedDrift()).toEqual([]); + expect(await indexesOf(file, 'tenant_t')).toEqual(coalesced); + }); + + /** A pre-18 tenant table whose pre-normalized NULL-safe unique was left as the bare composite, optionally over duplicates. */ + async function bareTenantKey(duplicates: boolean) { + const file = nextFile(); + const remote = await remoteOver(file); + // A nullable tenant column (a store built before 18.x, ADR-0131 D1): the + // duplicates the bare key admitted carry no organization, and a NOT NULL + // column would read the two key forms as one, proposing nothing. + await initObjectsPre18(remote, TENANT_NULL_SAFE); + await remote.syncSchemasBatch([{ object: 'tenant_t', schema: TENANT_NULL_SAFE }]); + const synced = await indexesOf(file, 'tenant_t'); + expect(synced[TENANT_UNIQUE]).toMatch(/COALESCE/i); await onDisk( file, `DROP INDEX "${TENANT_UNIQUE}"`, @@ -211,10 +267,11 @@ describe('the index entries', () => { const bare = await indexesOf(file, 'tenant_t'); const recreate = (await remote.detectManagedDrift()).find((d) => d.op.type === 'recreate_index')!; expect(recreate).toBeDefined(); + expect(recreate.op).toEqual(expect.objectContaining({ tightenNullSafeOnly: true, nullSafeColumns: ['organization_id'] })); return { file, remote, synced, bare, recreate }; } - it('recreate_index: the bare key tightens to the NULL-safe one over clean rows', async () => { + it('recreate_index: a pre-normalized NULL-safe declaration tightens the bare key over clean rows', async () => { const { file, remote, synced, recreate } = await bareTenantKey(false); expect(await remote.applyMigrationEntries([recreate], { allowDestructive: true })).toEqual({ applied: [recreate], skipped: [] }); expect(await indexesOf(file, 'tenant_t')).toEqual(synced); diff --git a/packages/metadata-protocol/src/migrations/sys-setting-identity-index.test.ts b/packages/metadata-protocol/src/migrations/sys-setting-identity-index.test.ts index 2d7521fa5dc..0d2d4ea691c 100644 --- a/packages/metadata-protocol/src/migrations/sys-setting-identity-index.test.ts +++ b/packages/metadata-protocol/src/migrations/sys-setting-identity-index.test.ts @@ -43,10 +43,19 @@ describe('sys_setting row-identity uniqueness (#8629)', () => { * declaration `{ fields: ['namespace','key','scope','user_id'], unique: * 'organization' }` after #8555 — captured by running the real driver over * the real declaration, the same way the sibling suites source their - * fixtures. It is the NULL-safe-on-organization, NULL-DISTINCT-on-user_id + * fixtures — re-measured at ADR-0131 D1, which keys the organization column + * PLAIN (the tenant column is NOT NULL). It is the NULL-DISTINCT-on-user_id * index this migration exists to replace, and what it must not destroy. */ const DECLARED_INDEX_DDL = + 'CREATE UNIQUE INDEX `uniq_sys_setting_organization_id_namespace_key_scope_user_id` ' + + 'on `sys_setting` (`organization_id`, `namespace`, `key`, `scope`, `user_id`)'; + /** + * What the driver emitted for the same declaration before ADR-0131 D1: + * ADR-0120 D3's NULL-safe organization key part. A store an 18.0 release + * built still carries it under the declared name. + */ + const DECLARED_INDEX_DDL_18_0 = 'CREATE UNIQUE INDEX `uniq_sys_setting_organization_id_namespace_key_scope_user_id` ' + "ON `sys_setting` (COALESCE(`organization_id`, '__global__'), `namespace`, `key`, `scope`, `user_id`)"; @@ -149,11 +158,24 @@ describe('sys_setting row-identity uniqueness (#8629)', () => { expect(dup.error).toContain(SYS_SETTING_IDENTITY_INDEX_NAME); }); - it('AFTER: two platform defaults on the global layer are refused', async () => { + it('AFTER: rows with NO organization are exempt — the organization key part is plain (ADR-0131 D1)', async () => { + // The platform layer lives in `sys_platform_setting` since the + // organization-ownership ceremony (ADR-0131 D7), and the tenant column + // is NOT NULL, so this index keys `organization_id` plain: SQL's + // NULL-distinct UNIQUE leaves the NULL-organization rows a pre-18 store + // may still hold unconstrained, as the driver's own index does. await ensureSysSettingIdentityIndex(exec); expect(insert('g1', { scope: 'global', namespace: 'mail', key: 'smtp_host' }).ok).toBe(true); - expect(insert('g2', { scope: 'global', namespace: 'mail', key: 'smtp_host' }).ok).toBe(false); + expect(insert('g2', { scope: 'global', namespace: 'mail', key: 'smtp_host' }).ok).toBe(true); + expect(insert('n1', { namespace: 'mail', key: 'smtp_host' }).ok).toBe(true); + expect(insert('n2', { namespace: 'mail', key: 'smtp_host' }).ok).toBe(true); + // Control: the same tenant-scope pair, NULL user_id, inside ONE + // organization is refused — the user_id fold is what this index adds. + expect(insert('o1', { organization_id: 'org_jia', namespace: 'mail', key: 'smtp_host' }).ok).toBe(true); + const dup = insert('o2', { organization_id: 'org_jia', namespace: 'mail', key: 'smtp_host' }); + expect(dup.ok).toBe(false); + expect(dup.error).toContain(SYS_SETTING_IDENTITY_INDEX_NAME); }); it('AFTER (anti-vacuity): the key stays PER-ORGANIZATION — two organizations may hold it', async () => { @@ -209,6 +231,21 @@ describe('sys_setting row-identity uniqueness (#8629)', () => { expect(indexNames()).toEqual([SYS_SETTING_IDENTITY_INDEX_NAME]); }); + it('an 18.0 index under the declared name (COALESCE organization) is rebuilt to the plain key', async () => { + // ADR-0131 D1: a store an earlier release built carries the NULL-safe + // organization key part; the run replaces the definition, so the key + // converges on the driver's plain form. + db.exec(`DROP INDEX ${SYS_SETTING_IDENTITY_INDEX_NAME}`); + db.exec(DECLARED_INDEX_DDL_18_0); + + expect(await ensureSysSettingIdentityIndex(exec)).toEqual({ status: 'created' }); + expect(indexDdl(SYS_SETTING_IDENTITY_INDEX_NAME)).toBe( + buildSysSettingIdentityIndexSql(SYS_SETTING_IDENTITY_INDEX_NAME).replace('IF NOT EXISTS ', ''), + ); + expect(indexDdl(SYS_SETTING_IDENTITY_INDEX_NAME)).not.toMatch(/__global__/); + expect(indexNames()).toEqual([SYS_SETTING_IDENTITY_INDEX_NAME]); + }); + it('leaves no probe index behind', async () => { await ensureSysSettingIdentityIndex(exec); expect(indexDdl(SYS_SETTING_IDENTITY_PROBE_INDEX_NAME)).toBeUndefined(); @@ -225,7 +262,7 @@ describe('sys_setting row-identity uniqueness (#8629)', () => { insert('a', { organization_id: 'org_jia' }); await ensureSysSettingIdentityIndex(exec); - // ADR-0120 D3's invariant: only the INDEX folds NULL into a bucket. A + // ADR-0120 D3's invariant, kept for `user_id`: only the INDEX folds NULL into a bucket. A // sentinel that reached storage would be a silent data rewrite. expect(rowCount()).toBe(1); expect( @@ -308,9 +345,11 @@ describe('sys_setting row-identity uniqueness (#8629)', () => { Record >; + // `organization_id` is a plain key part since ADR-0131 D1, so it is + // projected bare under its own name; only `user_id` is folded. expect(rows).toEqual([ { - organization_id_key: 'org_jia', + organization_id: 'org_jia', namespace: 'lifecycle', key: 'retention_overrides', scope: 'tenant', @@ -320,6 +359,17 @@ describe('sys_setting row-identity uniqueness (#8629)', () => { ]); }); + it('does not list organization-less twins: the plain organization key part never refuses them (ADR-0131 D1)', () => { + // A pre-18 table's rows with no organization share a NULL key part, + // so the index exempts them; listing them would send an operator + // after duplicates the constraint does not see. + expect(insert('n1', { organization_id: null, scope: 'tenant' }).ok).toBe(true); + expect(insert('n2', { organization_id: null, scope: 'tenant' }).ok).toBe(true); + const rows = db.prepare(buildSysSettingDuplicateProbeSql()).all() as Array>; + expect(rows.map((r) => r.organization_id)).toEqual(['org_jia']); + db.prepare("DELETE FROM sys_setting WHERE id IN ('n1', 'n2')").run(); + }); + it('keeps refusing until an operator resolves it — and then converges', async () => { expect((await ensureSysSettingIdentityIndex(exec)).status).toBe('conflict'); // The operator's decision, made by the operator. @@ -333,14 +383,15 @@ describe('sys_setting row-identity uniqueness (#8629)', () => { // ───────────────────────────────────────────────────────────────────────── describe('the statements', () => { - it('folds exactly the two nullable key columns, with the precedents’ sentinels', () => { - // Both literals are load-bearing and neither is this module's to - // choose: '__global__' is ADR-0120 D3 / `SqlDriver.GLOBAL_TENANT`, - // and '' is `ensureOverlayIndex`'s spelling for a non-tenant - // nullable discriminator. Editing either re-partitions the index. - expect(SYS_SETTING_NULL_SENTINELS).toEqual({ organization_id: '__global__', user_id: '' }); + it('folds exactly the one nullable non-tenant key column, with the precedent’s sentinel', () => { + // '' is `ensureOverlayIndex`'s spelling for a non-tenant nullable + // discriminator, and not this module's to choose: editing it + // re-partitions the index. `organization_id` takes no sentinel since + // ADR-0131 D1 — the tenant column is NOT NULL and the driver keys it + // plain, so ADR-0120 D3's '__global__' fold is gone from this key. + expect(SYS_SETTING_NULL_SENTINELS).toEqual({ user_id: '' }); expect(sysSettingIdentityKeyParts()).toEqual([ - "COALESCE(organization_id, '__global__')", + 'organization_id', 'namespace', 'key', 'scope', @@ -362,7 +413,7 @@ describe('sys_setting row-identity uniqueness (#8629)', () => { expect(buildSysSettingIdentityIndexSql(SYS_SETTING_IDENTITY_INDEX_NAME)).toBe( 'CREATE UNIQUE INDEX IF NOT EXISTS ' + 'uniq_sys_setting_organization_id_namespace_key_scope_user_id ON sys_setting ' + - "(COALESCE(organization_id, '__global__'), namespace, key, scope, COALESCE(user_id, ''))", + "(organization_id, namespace, key, scope, COALESCE(user_id, ''))", ); expect(buildSysSettingIdentityIndexSql(SYS_SETTING_IDENTITY_INDEX_NAME)).not.toMatch(/where/i); }); @@ -390,10 +441,13 @@ describe('sys_setting row-identity uniqueness (#8629)', () => { // Every folded column is projected through its OWN coalesce, never // bare: PostgreSQL rejects a bare projection of a column that appears // in GROUP BY only inside an expression (#6772), and this query has - // to be legal on both dialects that can build the index. - expect(sql).toContain("COALESCE(organization_id, '__global__') AS organization_id_key"); + // to be legal on both dialects that can build the index. A PLAIN key + // part (`organization_id` since ADR-0131 D1) is grouped bare, so it + // is projected bare — and never through a coalesce it is not keyed by. expect(sql).toContain("COALESCE(user_id, '') AS user_id_key"); - expect(sql).not.toMatch(/SELECT organization_id,/); + expect(sql).not.toMatch(/SELECT user_id,|, user_id,/); + expect(sql).toMatch(/^SELECT organization_id, /); + expect(sql).not.toContain('__global__'); }); // ───────────────────────────────────────────────────────────────────── @@ -410,20 +464,22 @@ describe('sys_setting row-identity uniqueness (#8629)', () => { // ───────────────────────────────────────────────────────────────────── it('the MySQL spelling quotes EVERY identifier, and leaves the bare one untouched', () => { expect(buildSysSettingDuplicateProbeSqlMysql()).toBe( - 'SELECT COALESCE(`organization_id`, \'__global__\') AS `organization_id_key`, ' + + 'SELECT `organization_id`, ' + '`namespace`, `key`, `scope`, COALESCE(`user_id`, \'\') AS `user_id_key`, ' + 'COUNT(*) AS `duplicate_rows` FROM `sys_setting` ' + - 'GROUP BY COALESCE(`organization_id`, \'__global__\'), `namespace`, `key`, ' + + 'WHERE `organization_id` IS NOT NULL AND `namespace` IS NOT NULL AND `key` IS NOT NULL AND `scope` IS NOT NULL ' + + 'GROUP BY `organization_id`, `namespace`, `key`, ' + '`scope`, COALESCE(`user_id`, \'\') HAVING COUNT(*) > 1', ); // The bare builder is the package's existing surface and several // callers execute it on SQLite/PostgreSQL — adding a variant must // not have moved it. expect(buildSysSettingDuplicateProbeSql()).toBe( - "SELECT COALESCE(organization_id, '__global__') AS organization_id_key, " + + 'SELECT organization_id, ' + "namespace, key, scope, COALESCE(user_id, '') AS user_id_key, " + 'COUNT(*) AS duplicate_rows FROM sys_setting ' + - "GROUP BY COALESCE(organization_id, '__global__'), namespace, key, scope, " + + 'WHERE organization_id IS NOT NULL AND namespace IS NOT NULL AND key IS NOT NULL AND scope IS NOT NULL ' + + 'GROUP BY organization_id, namespace, key, scope, ' + "COALESCE(user_id, '') HAVING COUNT(*) > 1", ); // No identifier left bare — not a list of MySQL's reserved words, @@ -458,7 +514,7 @@ describe('sys_setting row-identity uniqueness (#8629)', () => { expect(mysql).toEqual(bare); expect(bare).toEqual([ { - organization_id_key: 'org_jia', + organization_id: 'org_jia', namespace: 'lifecycle', key: 'retention_overrides', scope: 'tenant', diff --git a/packages/metadata-protocol/src/migrations/sys-setting-identity-index.ts b/packages/metadata-protocol/src/migrations/sys-setting-identity-index.ts index 971b734fe1b..087c4822df6 100644 --- a/packages/metadata-protocol/src/migrations/sys-setting-identity-index.ts +++ b/packages/metadata-protocol/src/migrations/sys-setting-identity-index.ts @@ -86,16 +86,11 @@ * * ## The KEY, and why each sentinel is copied rather than chosen * - * Both spellings already exist in this repo; neither is invented here. - * - * - `organization_id` → `'__global__'`. ADR-0120 D3's exact form for a tenant - * column, and the literal `SqlDriver`'s `GLOBAL_TENANT` / - * `organizationKeyPartSql` materializes. It is reproduced (not imported) - * because this package must not depend on a driver — so the literal is pinned - * by the sibling test, and a silent edit here cannot re-partition the index. - * ⚠️ Reproducing it is also what keeps this a pure tightening of the SHIPPED - * index rather than a re-keying of it: change this literal and the platform - * bucket moves. + * The spelling already exists in this repo; it is not invented here. + * + * - `organization_id` → none since ADR-0131 D1: the tenant column is NOT NULL, + * so its key part is the plain column, exactly as the driver now keys it. + * Before that it was `'__global__'`, ADR-0120 D3's form for a tenant column. * - `user_id` → `''`. The `ensureOverlayIndex` precedent for a NON-tenant * nullable discriminator (`COALESCE(package_id, '')`), whose comment states * this same NULL-distinct reason. A user id is never the empty string, so the @@ -208,14 +203,19 @@ export const SYS_SETTING_IDENTITY_INDEX_COLUMNS = [ * * A column listed here is materialized as `COALESCE(, '')` so * its NULL rows form ONE bucket that is unique among itself, instead of being - * mutually DISTINCT and therefore unconstrained. Both spellings are copied from - * an existing in-repo precedent — see the module header for which, and why - * neither may be edited casually. - * - * `namespace`, `key` and `scope` are required and take no sentinel. + * mutually DISTINCT and therefore unconstrained. The spelling is copied from + * an existing in-repo precedent — see the module header for which, and why it + * may not be edited casually. + * + * `namespace`, `key` and `scope` are required and take no sentinel, and since + * ADR-0131 D1 neither does `organization_id`: the tenant column is NOT NULL + * (the organization-ownership ceremony moves the `scope = 'global'` rows out + * to `sys_platform_setting`), so the driver keys it plain and so does this + * index. An index an earlier release built with `COALESCE(organization_id, + * '__global__')` is rebuilt to this key by the next boot's run, which rebuilds + * the definition on every boot anyway. */ export const SYS_SETTING_NULL_SENTINELS: Readonly> = { - organization_id: '__global__', user_id: '', }; @@ -326,9 +326,8 @@ export function buildSysSettingPresenceSql(): string { * exactly TWO dialects that can build the index this query explains, so it must * be legal on both, not on the lenient one. * - * Folding costs the operator nothing: neither sentinel can occur in real data, - * so `organization_id_key = '__global__'` reads as "organization_id IS NULL" and - * `user_id_key = ''` as "user_id IS NULL". + * Folding costs the operator nothing: the sentinel cannot occur in real data, + * so `user_id_key = ''` reads as "user_id IS NULL". */ export function buildSysSettingDuplicateProbeSql(): string { return duplicateProbeSqlIn(BARE); @@ -389,9 +388,15 @@ function duplicateProbeSqlIn(quote: IdentifierQuote): string { const keyPart = keyParts[i]!; return keyPart === quote(column) ? keyPart : `${keyPart} AS ${quote(`${column}_key`)}`; }); + // [ADR-0131 D1] A row with a NULL in a plain key part conflicts with nothing + // under the NULL-distinct UNIQUE, so it is no duplicate to list: the query + // groups only the rows the index constrains (the driver's own pre-flight + // leaves them out the same way). + const plain = SYS_SETTING_IDENTITY_INDEX_COLUMNS.filter((column) => SYS_SETTING_NULL_SENTINELS[column] === undefined); return ( `SELECT ${projected.join(', ')}, COUNT(*) AS ${quote('duplicate_rows')} ` + `FROM ${quote(SYS_SETTING_TABLE)} ` + + `WHERE ${plain.map((column) => `${quote(column)} IS NOT NULL`).join(' AND ')} ` + `GROUP BY ${keyParts.join(', ')} HAVING COUNT(*) > 1` ); } @@ -591,7 +596,7 @@ function reportDegradation( `'${SYS_SETTING_IDENTITY_INDEX_NAME}' on "${SYS_SETTING_TABLE}" stays NULL-distinct on user_id ` + `over (${columns}). The system keeps looking healthy while the declared row identity is void on ` + `every row that is not scope='user' — user_id is NULL there — so two tenant-scope rows for one ` + - `(namespace, key) in ONE organization, or two platform defaults on the global layer, can coexist ` + + `(namespace, key, scope) in ONE organization can coexist ` + `and SettingsService has no defined answer for which one wins. MySQL/MariaDB before ` + `8.0.13 has no functional key parts, so there is no in-dialect fix: run this platform on ` + `SQLite/PostgreSQL for the guarantee, and meanwhile watch for duplicates with this MySQL ` + diff --git a/packages/objectql/src/adr0131-tenant-key-certified-supply.test.ts b/packages/objectql/src/adr0131-tenant-key-certified-supply.test.ts new file mode 100644 index 00000000000..121358f36c9 --- /dev/null +++ b/packages/objectql/src/adr0131-tenant-key-certified-supply.test.ts @@ -0,0 +1,163 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [18.x step 7b-1] The kernel→driver supply seam for "is this database + * certified", from the ENGINE's side (ADR-0131 D1). + * + * A SQL driver reads an earlier release's `COALESCE(, '__global__')` + * key part as the plain tenant column new DDL keys only where the tenant + * column holds no NULL. A ceremony-upgraded store whose column stayed nullable + * (a rotation shard, a Turso remote table, a reported table) proves that only + * by its certificate: the ownership row in `sys_migration`, verified, at this + * release's shape generation — a birth marker or the ceremony's own marker. + * The engine reads it; the driver is handed the question. The driver's half + * is pinned in `@objectstack/driver-sql` + * (`sql-driver-tenant-key-equivalence.test.ts`). + * + * Every way of not knowing answers `false`, the reading under which the differ + * keeps the two forms distinct: the differ reports a destructive + * `recreate_index`, which is never applied unattended. + */ + +import { describe, it, expect } from 'vitest'; +import { ObjectQL } from './engine'; +import { + ORGANIZATION_OWNERSHIP_MIGRATION_ID, + ORGANIZATION_OWNERSHIP_SHAPE_GENERATION, +} from '@objectstack/spec/system'; +import type { IDataDriver } from '@objectstack/spec/contracts'; + +type Rows = Array>; + +/** Equality only; a combinator read as a field name would answer a silent "no rows". */ +function matches(row: Record, where: any): boolean { + if (!where || typeof where !== 'object') return true; + return Object.entries(where).every(([k, v]) => { + if (k.startsWith('$')) throw new Error(`fake driver: unsupported combinator ${k}`); + return row[k] === v; + }); +} + +interface Sink { + resolver: (() => Promise) | null; +} + +function boot(rows: Rows, opts: { sink?: Sink; readThrows?: string; withFlagObject?: boolean } = {}): ObjectQL { + const driver: Record = { + name: 'default', + version: '1.0.0', + async connect() {}, + async disconnect() {}, + async find(_object: string, ast: any) { + const found = rows.filter((r) => matches(r, ast?.where)); + return typeof ast?.limit === 'number' ? found.slice(0, ast.limit) : found; + }, + async findOne(_object: string, ast: any) { + if (opts.readThrows) throw new Error(opts.readThrows); + return rows.find((r) => matches(r, ast?.where)) ?? null; + }, + async count() { + return rows.length; + }, + async create(_object: string, data: any) { + rows.push({ ...data }); + return data; + }, + async update() { + return null; + }, + async delete() { + return true; + }, + async syncSchema() {}, + async dropTable() {}, + }; + if (opts.sink) { + const sink = opts.sink; + driver.setTenantKeyCertifiedResolver = (resolve: () => Promise) => { + sink.resolver = resolve; + }; + } + const engine = new ObjectQL(); + engine.registerDriver(driver as unknown as IDataDriver, true); + const flagObject = { + name: 'sys_migration', + fields: { + id: { type: 'text' }, + last_run_at: { type: 'datetime' }, + verified_at: { type: 'datetime' }, + blocking: { type: 'number' }, + details: { type: 'textarea' }, + }, + }; + const task = { name: 'showcase_task', fields: { id: { type: 'text' }, title: { type: 'text' } } }; + engine.registerApp({ + id: 'showcase_pkg', + name: 'Showcase', + objects: opts.withFlagObject === false ? [task] : [task, flagObject], + } as any); + return engine; +} + +const NOW = '2026-10-20T04:00:00.000Z'; +const certified = (over: Record = {}): Record => ({ + id: ORGANIZATION_OWNERSHIP_MIGRATION_ID, + last_run_at: NOW, + verified_at: NOW, + blocking: 0, + details: JSON.stringify({ checkpoint: { shapeGeneration: ORGANIZATION_OWNERSHIP_SHAPE_GENERATION } }), + ...over, +}); + +describe('[ADR-0131 D1] isOrganizationOwnershipCertified()', () => { + it("the ceremony's verified marker at the current shape generation certifies", async () => { + expect(await boot([certified()]).isOrganizationOwnershipCertified()).toBe(true); + }); + + it('so does a birth marker', async () => { + const birth = certified({ + details: JSON.stringify({ birth: { shapeGeneration: ORGANIZATION_OWNERSHIP_SHAPE_GENERATION } }), + }); + expect(await boot([birth]).isOrganizationOwnershipCertified()).toBe(true); + }); + + it('every way of not knowing answers false', async () => { + expect(await boot([]).isOrganizationOwnershipCertified(), 'no row').toBe(false); + expect(await boot([certified({ verified_at: null })]).isOrganizationOwnershipCertified(), 'unverified').toBe(false); + expect(await boot([certified({ blocking: 3 })]).isOrganizationOwnershipCertified(), 'blocking findings').toBe(false); + expect( + await boot([certified({ details: JSON.stringify({ checkpoint: { shapeGeneration: ORGANIZATION_OWNERSHIP_SHAPE_GENERATION - 1 } }) })]) + .isOrganizationOwnershipCertified(), + 'an older shape generation', + ).toBe(false); + expect(await boot([certified({ details: null })]).isOrganizationOwnershipCertified(), 'no generation named').toBe(false); + expect( + await boot([certified()], { readThrows: 'relation "sys_migration" does not exist' }).isOrganizationOwnershipCertified(), + 'no ledger table yet', + ).toBe(false); + expect(await boot([certified()], { withFlagObject: false }).isOrganizationOwnershipCertified(), 'no ledger object').toBe( + false, + ); + }); + + it('any other read failure is thrown, never answered as "not certified"', async () => { + await expect(boot([certified()], { readThrows: 'connection reset' }).isOrganizationOwnershipCertified()).rejects.toThrow( + 'connection reset', + ); + }); + + it('registerDriver hands a driver with the seam a closure over the same reading', async () => { + const rows: Rows = []; + const sink: Sink = { resolver: null }; + boot(rows, { sink }); + expect(sink.resolver).toBeTypeOf('function'); + expect(await sink.resolver!()).toBe(false); + // A question, not a value: the answer follows the ledger at the moment it is asked. + rows.push(certified()); + expect(await sink.resolver!()).toBe(true); + }); + + it('a driver without the seam is left alone', () => { + expect(() => boot([])).not.toThrow(); + }); +}); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index dc62bd2cccf..7ad62dca495 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -91,6 +91,9 @@ import { DATA_MIGRATION_FLAG_OBJECT, FILE_REFERENCES_MIGRATION_ID, VALUE_SHAPES_MIGRATION_ID, + ORGANIZATION_OWNERSHIP_MIGRATION_ID, + ORGANIZATION_OWNERSHIP_SHAPE_GENERATION, + organizationOwnershipShapeGenerationOf, isDataMigrationFlagVerified, hasMovedFileColumns, renderOperationMessage, @@ -9398,6 +9401,7 @@ export class ObjectQL implements IObjectQLEngine { this.drivers.set(driver.name, driver); this.supplyFileColumnsMovedResolver(driver); + this.supplyTenantKeyCertifiedResolver(driver); this.logger.info('Registered driver', { driverName: driver.name, version: driver.version @@ -12590,6 +12594,67 @@ export class ObjectQL implements IObjectQLEngine { return (await this.readFileReferencesFlagRow()).verified; } + /** + * [ADR-0131 D1] Hand a freshly-registered driver the question "is this + * database certified", the same closure-not-value seam as + * {@link supplyFileColumnsMovedResolver} and for the same reason: the answer + * is a `sys_migration` row on a store this driver has not connected to yet. + * A SQL driver asks it only when a table carries an index an earlier release + * built with the NULL-safe `COALESCE(, …)` key part, to decide + * whether that index already is the plain key new DDL emits. A driver with + * no such seam is left alone. + */ + private supplyTenantKeyCertifiedResolver(driver: IDataDriver): void { + const sink = driver as unknown as { + setTenantKeyCertifiedResolver?: (resolve: () => Promise) => void; + }; + if (typeof sink.setTenantKeyCertifiedResolver !== 'function') return; + try { + sink.setTenantKeyCertifiedResolver(() => this.isOrganizationOwnershipCertified()); + } catch (e: any) { + // A driver that did not take the resolver answers "not certified": it + // keeps the two forms distinct and REPORTS a destructive recreate_index, + // which is never applied unattended (safe-only boot reconcile, the + // artifact gate, `os migrate apply --allow-destructive`). + this.logger.debug('Driver declined the ADR-0131 certification resolver', { + driverName: driver.name, + error: e?.message ?? String(e), + }); + } + } + + /** + * [ADR-0131 D1] Is this database certified for the organization-ownership + * shape — a verified `sys_migration` row for the ceremony's id (the birth + * marker a store created from empty receives, or the ceremony's completion + * marker) at this release's shape generation or above? The same reading the + * runtime's boot guard boots on. No ledger object, no ledger table yet, no + * row, or an unverified or older row answers `false`. Any other read failure + * is thrown: "I could not check" is not "not certified", and the driver + * asking reports it (`SqlDriver.isTenantKeyCertified`). + */ + async isOrganizationOwnershipCertified(): Promise { + if (!this._registry.getObject(DATA_MIGRATION_FLAG_OBJECT)) return false; + try { + const row: any = await this.findOne(DATA_MIGRATION_FLAG_OBJECT, { + where: { id: ORGANIZATION_OWNERSHIP_MIGRATION_ID }, + context: { isSystem: true } as ExecutionContext, + }); + if (!row || row.id !== ORGANIZATION_OWNERSHIP_MIGRATION_ID) return false; + const verified = isDataMigrationFlagVerified({ + id: ORGANIZATION_OWNERSHIP_MIGRATION_ID, + last_run_at: String(row.last_run_at ?? ''), + verified_at: row.verified_at == null ? null : String(row.verified_at), + blocking: typeof row.blocking === 'number' ? row.blocking : Number(row.blocking ?? Number.NaN), + }); + return verified && organizationOwnershipShapeGenerationOf(row.details) >= ORGANIZATION_OWNERSHIP_SHAPE_GENERATION; + } catch (error) { + // A store whose ledger table does not exist yet has no certificate. + if (isMissingTableError(error, DATA_MIGRATION_FLAG_OBJECT)) return false; + throw error; + } + } + /** * The one memoized read both file-flag questions come off (#15989). * diff --git a/packages/plugins/plugin-security/src/grant-permission-set-name.test.ts b/packages/plugins/plugin-security/src/grant-permission-set-name.test.ts index cb0d4a6adfa..df52d0907c3 100644 --- a/packages/plugins/plugin-security/src/grant-permission-set-name.test.ts +++ b/packages/plugins/plugin-security/src/grant-permission-set-name.test.ts @@ -7,9 +7,11 @@ * as an undeclared field; a non-system write naming a set the security * catalog does not hold is refused, and a grant that carries no name is * refused as `required`. One grant per user, set and organization: the - * declared unique index (`unique: 'organization'`, NULL-safe on the - * organization) refuses a second, platform-wide grants included, on every - * write path, so a revoke leaves no grant of that set in force. + * declared unique index (`unique: 'organization'`) refuses a second on every + * write path, so a revoke leaves no grant of that set in force. Its + * organization key part is the plain column since ADR-0131 D1 (the tenant + * column is NOT NULL), so on the pre-18 nullable column this harness models a + * grant with no organization is exempt, like a NULL in any key part. * * Measured on a REAL `ObjectQL` engine over a real SQL driver with the REAL * `SecurityPlugin` started on it the way a kernel composition does, because @@ -309,42 +311,39 @@ describe('one grant per user, set and organization — the declared unique \'org expect(await grantsOfSet(h, 'u_d1', 'qa_alpha')).toEqual([]); }); - it('NEGATIVE — a second platform-wide grant (organization NULL) is refused 409 by the index itself, for every writer; the revoke leaves none in force', async () => { + it('a second grant with no organization is NOT refused by the index on a pre-18 nullable column: the plain organization key part exempts it (ADR-0131 D1)', async () => { const h = await boot(); await h.engine.insert('sys_user_permission_set', { id: 'ups_p1', user_id: 'u_p1', permission_set: 'qa_alpha', organization_id: null }, { context: ADMIN } as any); - for (const context of [ADMIN, SYS]) { - const err = await refusalOf(() => h.engine.insert( - 'sys_user_permission_set', { user_id: 'u_p1', permission_set: 'qa_alpha' }, { context } as any, - )); - expect(duplicateShape(err)).toEqual(DUPLICATE); - } - expect((await grantsOfSet(h, 'u_p1', 'qa_alpha')).map((g) => g.id)).toEqual(['ups_p1']); - - await h.engine.delete('sys_user_permission_set', { where: { id: 'ups_p1' }, context: ADMIN } as any); - expect(await grantsOfSet(h, 'u_p1', 'qa_alpha')).toEqual([]); + await knexOf(h)('sys_user_permission_set').insert({ id: 'ups_p2', user_id: 'u_p1', permission_set: 'qa_alpha', organization_id: null }); + expect((await grantsOfSet(h, 'u_p1', 'qa_alpha')).map((g) => g.id).sort()).toEqual(['ups_p1', 'ups_p2']); }); - it('NEGATIVE — the index refuses a platform-wide duplicate on paths no hook sees: a two-row batch insert stores nothing, and raw SQL beneath the engine is refused', async () => { + it('NEGATIVE — the index refuses an organization duplicate on paths no hook sees: a two-row batch insert stores nothing, and raw SQL beneath the engine is refused', async () => { const h = await boot(); + await h.engine.insert('sys_member', { id: 'mem_b1', user_id: 'u_b1', organization_id: ORG, role: 'member' }, { context: SYS } as any); const err = await refusalOf(() => h.engine.insert( 'sys_user_permission_set', - [{ user_id: 'u_b1', permission_set: 'qa_alpha' }, { user_id: 'u_b1', permission_set: 'qa_alpha' }], + [ + { user_id: 'u_b1', permission_set: 'qa_alpha', organization_id: ORG }, + { user_id: 'u_b1', permission_set: 'qa_alpha', organization_id: ORG }, + ], { context: SYS } as any, )); expect(duplicateShape(err)).toEqual(DUPLICATE); expect(await grantsOf(h, 'u_b1')).toEqual([]); - await knexOf(h)('sys_user_permission_set').insert({ id: 'raw_1', user_id: 'u_r1', permission_set: 'qa_alpha', organization_id: null }); + await knexOf(h)('sys_user_permission_set').insert({ id: 'raw_1', user_id: 'u_r1', permission_set: 'qa_alpha', organization_id: ORG }); await expect( - knexOf(h)('sys_user_permission_set').insert({ id: 'raw_2', user_id: 'u_r1', permission_set: 'qa_alpha', organization_id: null }), + knexOf(h)('sys_user_permission_set').insert({ id: 'raw_2', user_id: 'u_r1', permission_set: 'qa_alpha', organization_id: ORG }), ).rejects.toThrow(/UNIQUE constraint failed/); expect((await grantsOf(h, 'u_r1')).map((g) => g.id)).toEqual(['raw_1']); }); - it('NEGATIVE — an update that makes a platform-wide duplicate (re-naming the set) is refused 409 and the row is untouched', async () => { + it('NEGATIVE — an update that makes an organization duplicate (re-naming the set) is refused 409 and the row is untouched', async () => { const h = await boot(); - await h.engine.insert('sys_user_permission_set', { id: 'ups_u1', user_id: 'u_u1', permission_set: 'qa_alpha' }, { context: ADMIN } as any); - await h.engine.insert('sys_user_permission_set', { id: 'ups_u2', user_id: 'u_u1', permission_set: 'qa_beta' }, { context: ADMIN } as any); + await h.engine.insert('sys_member', { id: 'mem_u1', user_id: 'u_u1', organization_id: ORG, role: 'member' }, { context: SYS } as any); + await h.engine.insert('sys_user_permission_set', { id: 'ups_u1', user_id: 'u_u1', permission_set: 'qa_alpha', organization_id: ORG }, { context: ADMIN } as any); + await h.engine.insert('sys_user_permission_set', { id: 'ups_u2', user_id: 'u_u1', permission_set: 'qa_beta', organization_id: ORG }, { context: ADMIN } as any); const err = await refusalOf(() => h.engine.update( 'sys_user_permission_set', { id: 'ups_u2', permission_set: 'qa_alpha' }, { context: ADMIN } as any, )); diff --git a/packages/qa/dogfood/test/upgrade-18x-index-equivalence.dogfood.test.ts b/packages/qa/dogfood/test/upgrade-18x-index-equivalence.dogfood.test.ts new file mode 100644 index 00000000000..eb05567cf32 --- /dev/null +++ b/packages/qa/dogfood/test/upgrade-18x-index-equivalence.dogfood.test.ts @@ -0,0 +1,282 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [18.x step 7b-1 + 7b-2] The organization key part of a unique index, across +// the upgrade. +// +// An 18.0 database keys every organization-scoped unique index through the +// ADR-0120 D3 form `COALESCE(organization_id, '__global__')`. HEAD's new DDL +// keys the plain column (ADR-0131 D1: the tenant column is NOT NULL), and its +// differ reads the old form as equivalent where the tenant column holds no +// NULL: physically NOT NULL, or a certified database — a birth marker or the +// ownership ceremony's marker. Without that equivalence every one of those +// indexes reads as "same name, different definition", a `recreate_index`: a +// DROP of a live unique constraint, proposed at every boot and by +// `os migrate plan` to every upgraded deployment. +// +// What the ceremony itself does to them is measured, not assumed: on SQLite its +// NOT NULL step rebuilds each table through the driver's drift apply door, and +// the rebuild re-keys the table's indexes with this release's DDL — plain. A +// table the ceremony leaves nullable (`sys_email_template` here) keeps its +// 18.0 index, and only the certification makes it equivalent. PostgreSQL and +// MySQL take NOT NULL by `ALTER COLUMN` and a remote Turso store not at all, so +// there every 18.0 index survives the ceremony; a SQLite store an earlier 18.x +// ceremony certified holds them too. The second block models that store by +// restoring the 13 indexes exactly as 18.0 wrote them. +import { copyFileSync, mkdtempSync, readFileSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { DatabaseSync } from 'node:sqlite'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import type { VerifyStack } from '@objectstack/verify'; +import { ceremoniedFixture } from './upgrade-harness/ceremony.js'; +import { bootFixture } from './upgrade-harness/populate-fixture.js'; +import { relaxTenantColumns } from './pre-18-tenant-column.testkit.js'; + +const fixtureSql = readFileSync(new URL('./upgrade-harness/fixture-18.0/database.sql', import.meta.url), 'utf8'); + +/** One unique index of the file, as SQLite recorded its DDL. */ +interface UniqueIndex { + name: string; + table: string; + sql: string; +} + +/** A tenant key part in the ADR-0120 D3 form, quoted or not. */ +const COALESCE_ORG = /COALESCE\(\s*[`"]?organization_id[`"]?\s*,/i; + +/** The 18.0 fixture's own `CREATE UNIQUE INDEX` statements over the COALESCE organization key part. */ +const COALESCE_INDEXES_18_0: Array<{ name: string; sql: string }> = fixtureSql + .split('\n') + .filter((line) => /^CREATE UNIQUE INDEX/i.test(line) && COALESCE_ORG.test(line)) + .map((line) => ({ + name: /^CREATE UNIQUE INDEX\s+[`"]?([A-Za-z0-9_]+)/i.exec(line)![1]!, + sql: line.replace(/;\s*$/, ''), + })); + +/** Every UNIQUE index of `file` whose DDL keys the organization column. */ +function organizationUniqueIndexes(file: string): UniqueIndex[] { + const db = new DatabaseSync(file, { readOnly: true }); + try { + return ( + db + .prepare("SELECT name, tbl_name AS \"table\", sql FROM sqlite_master WHERE type = 'index' AND sql IS NOT NULL ORDER BY name") + .all() as unknown as UniqueIndex[] + ).filter((i) => /^CREATE UNIQUE INDEX/i.test(i.sql) && /organization_id/.test(i.sql)); + } finally { + db.close(); + } +} + +/** Whether `table`'s organization column is physically NOT NULL in `file`. */ +function organizationNotNull(file: string, table: string): boolean { + const db = new DatabaseSync(file, { readOnly: true }); + try { + const row = db.prepare("SELECT \"notnull\" AS nn FROM pragma_table_info(?) WHERE name = 'organization_id'").get(table) as + | { nn: number } + | undefined; + return row?.nn === 1; + } finally { + db.close(); + } +} + +type DriftEntry = { table: string; category: string; op: { type: string } }; + +/** The schema differ's verdict on the booted store, over every object it synced. */ +async function managedDrift(stack: VerifyStack): Promise { + const ql = (await stack.kernel.getServiceAsync('objectql')) as { + getDriverForObject(object: string): { detectManagedDrift?: () => Promise }; + }; + const driver = ql.getDriverForObject('sys_migration'); + expect(typeof driver.detectManagedDrift, 'the store serving the ledger is the SQL driver').toBe('function'); + return driver.detectManagedDrift!(); +} + +/** No index op the differ would propose to DROP anything: no recreate, nothing destructive. */ +function expectNoDestructiveIndexDrift(drift: DriftEntry[]): void { + const recreate = drift.filter((d) => d.op.type === 'recreate_index'); + expect(recreate, JSON.stringify(recreate, null, 2)).toEqual([]); + const destructive = drift.filter((d) => d.category === 'destructive'); + expect(destructive, JSON.stringify(destructive, null, 2)).toEqual([]); +} + +describe('[18.x step 7b] the organization key part across the 18.0 → HEAD upgrade', () => { + const dir = mkdtempSync(join(tmpdir(), 'dogfood-index-equivalence-')); + let ceremonied = ''; + + beforeAll(async () => { + ceremonied = (await ceremoniedFixture(dir, fixtureSql, 'equivalence')).file; + }, 900_000); + + afterAll(() => { + rmSync(dir, { recursive: true, force: true }); + }); + + it('the 18.0 fixture carries 13 COALESCE organization unique indexes (a vacuous fixture is no pin)', () => { + expect(COALESCE_INDEXES_18_0).toHaveLength(13); + }); + + describe('the ceremonied database as the ceremony left it', () => { + let stack: VerifyStack | undefined; + let before: UniqueIndex[] = []; + + beforeAll(async () => { + before = organizationUniqueIndexes(ceremonied); + stack = await bootFixture(ceremonied); + }, 300_000); + + afterAll(async () => { + await stack?.stop(); + }); + + it("sys_email_template stays nullable and keeps its 18.0 COALESCE index: the certification is what carries it", () => { + expect(organizationNotNull(ceremonied, 'sys_email_template')).toBe(false); + const kept = before.filter((i) => COALESCE_ORG.test(i.sql)).map((i) => i.name); + expect(kept).toContain('uniq_sys_email_template_organization_id_name_locale'); + }); + + it("HEAD's differ reports zero recreate_index and zero destructive drift", async () => { + expectNoDestructiveIndexDrift(await managedDrift(stack!)); + }); + }); + + describe('the ceremonied database holding all 13 indexes exactly as 18.0 wrote them', () => { + const restored = join(dir, 'restored.db'); + let stack: VerifyStack | undefined; + + beforeAll(async () => { + copyFileSync(ceremonied, restored); + const db = new DatabaseSync(restored); + try { + for (const index of COALESCE_INDEXES_18_0) { + db.exec(`DROP INDEX IF EXISTS "${index.name}"`); + db.exec(index.sql); + } + } finally { + db.close(); + } + stack = await bootFixture(restored); + }, 300_000); + + afterAll(async () => { + await stack?.stop(); + }); + + it("HEAD's differ reports zero recreate_index and zero destructive drift", async () => { + expectNoDestructiveIndexDrift(await managedDrift(stack!)); + }); + + it('the 12 declared organization composites survive the HEAD boot byte for byte', () => { + // `sys_setting`'s identity index is the thirteenth: the metadata + // protocol's own runtime migration rebuilds it on every boot, now over + // the plain organization key. + const declared = COALESCE_INDEXES_18_0.filter((i) => !i.name.startsWith('uniq_sys_setting_')); + expect(declared).toHaveLength(12); + const after = new Map(organizationUniqueIndexes(restored).map((i) => [i.name, i.sql])); + for (const index of declared) expect(after.get(index.name), index.name).toBe(index.sql); + }); + }); + + describe('a store 18.0.0 created from empty: birth-marked, nullable tenant columns, COALESCE organization indexes', () => { + // 18.0.0 (published) births a store with its ownership marker at shape + // generation 1 — the generation HEAD still requires — while it still built + // every tenant column nullable and keyed organization uniques through the + // COALESCE form. Measured on the published 18.0.0 for this part: 34 tenant + // columns, none NOT NULL; 10 of 14 organization uniques in the COALESCE + // form; then HEAD booted it with no refusal and `os migrate plan` read + // 0 changes. This leg models that store from a HEAD birth (same marker), + // the tenant columns relaxed through the drift door and the indexes + // re-spelled exactly as 18.0.0 wrote them, so the CERTIFICATION is the + // only witness: no tenant column here is NOT NULL. + const born = join(dir, 'born-18.0.0.db'); + let stack: VerifyStack | undefined; + let respelled: Array<{ name: string; sql: string }> = []; + + beforeAll(async () => { + const first = await bootFixture(born); + try { + const ql = (await first.kernel.getServiceAsync('objectql')) as { getDriverForObject(object: string): unknown }; + // The tables whose organization unique 18.0.0 keyed through COALESCE — + // the ones whose equivalence is under test. (`sys_setting`'s identity + // index is its runtime migration's, rebuilt every boot.) + const tenantTables = [ + ...new Set(organizationUniqueIndexes(born).map((i) => i.table).filter((t) => t !== 'sys_setting')), + ]; + await relaxTenantColumns(ql, tenantTables); + } finally { + await first.stop(); + } + const db = new DatabaseSync(born); + try { + for (const index of organizationUniqueIndexes(born)) { + // `sys_setting`'s identity index is its runtime migration's, rebuilt every boot. + if (index.table === 'sys_setting') continue; + const sql = index.sql.replace(/\(\s*([`"]?)organization_id\1\s*([,)])/, "(COALESCE($1organization_id$1, '__global__')$2") + .replace(/,\s*([`"]?)organization_id\1\s*([,)])/, ", COALESCE($1organization_id$1, '__global__')$2"); + if (sql === index.sql) continue; + db.exec(`DROP INDEX "${index.name}"`); + db.exec(sql); + respelled.push({ name: index.name, sql }); + } + } finally { + db.close(); + } + stack = await bootFixture(born); + }, 600_000); + + afterAll(async () => { + await stack?.stop(); + }); + + it('is the 18.0.0 shape: a verified generation-1 birth marker, no NOT NULL tenant column, COALESCE indexes', () => { + const db = new DatabaseSync(born, { readOnly: true }); + try { + const marker = db.prepare('SELECT verified_at, details FROM sys_migration WHERE id = ?').get( + 'adr-0131-organization-ownership', + ) as { verified_at: unknown; details: string } | undefined; + expect(marker?.verified_at).toBeTruthy(); + expect(JSON.parse(marker!.details).birth?.shapeGeneration).toBe(1); + } finally { + db.close(); + } + expect(respelled.length, 'the store holds COALESCE organization indexes').toBeGreaterThanOrEqual(10); + for (const index of respelled) { + const table = organizationUniqueIndexes(born).find((i) => i.name === index.name)!.table; + expect(organizationNotNull(born, table), `${table}.organization_id`).toBe(false); + } + }); + + it("HEAD boots it, and its differ reports zero recreate_index and zero destructive drift", async () => { + expectNoDestructiveIndexDrift(await managedDrift(stack!)); + }); + + it('every COALESCE index survives the HEAD boot byte for byte', () => { + const after = new Map(organizationUniqueIndexes(born).map((i) => [i.name, i.sql])); + for (const index of respelled) expect(after.get(index.name), index.name).toBe(index.sql); + }); + }); + + describe('a store HEAD creates from empty', () => { + const fresh = join(dir, 'fresh.db'); + let stack: VerifyStack | undefined; + + beforeAll(async () => { + stack = await bootFixture(fresh); + }, 300_000); + + afterAll(async () => { + await stack?.stop(); + }); + + it('keys every organization unique index on the plain column: no COALESCE organization key part anywhere', () => { + const indexes = organizationUniqueIndexes(fresh); + expect(indexes.length, 'the fresh store holds organization unique indexes').toBeGreaterThanOrEqual(13); + expect(indexes.filter((i) => COALESCE_ORG.test(i.sql)).map((i) => i.name)).toEqual([]); + }); + + it('and its differ reports no index drift', async () => { + const drift = await managedDrift(stack!); + expect(drift.filter((d) => d.op.type.endsWith('_index'))).toEqual([]); + }); + }); +});