diff --git a/.changeset/unique-tenant-scoped-materialization.md b/.changeset/unique-tenant-scoped-materialization.md new file mode 100644 index 0000000000..49f6ced656 --- /dev/null +++ b/.changeset/unique-tenant-scoped-materialization.md @@ -0,0 +1,69 @@ +--- +"@objectstack/driver-mongodb": minor +"@objectstack/driver-sql": minor +"@objectstack/spec": minor +--- + +fix(driver-sql)!: `unique` materializes per tenant, ending its contradiction with the per-tenant autonumber sequence (#3696) + +`unique: true` became a **single-column global index that ignored `tenancy` +entirely**, while the autonumber sequence table is keyed by +`(object, tenant_id, field, scope)` and hands every tenant its own counter +starting at 1. Two subsystems of the same platform contradicted each other: +tenant B's `PROD-00001` was rejected by an index it could not see — **no user +did anything wrong**, the platform's left hand refused what its right hand +issued. + +The rejection also doubled as a **cross-tenant existence oracle**: a UNIQUE +violation told tenant B that some *other* tenant held the value, enumerable by +probing emails / codes / names. + +**The contract now:** + +| Declaration | Materializes as | +|---|---| +| `unique: true` + tenant column | composite `(tenantField, field)` — unique **within** the tenant | +| `unique: true`, no tenant column | single-column — single-tenant DDL is byte-identical to before | +| `unique: 'global'` | single-column, always platform-wide | + +The tenant column comes first in the composite, so the index also serves the +`WHERE tenant = ?` prefix scans every tenant-scoped read issues. + +**Declared `indexes[]` are deliberately unchanged.** They are materialized over +exactly the columns listed — no tenant column is injected. The author already +spells them out, per-tenant ones have always been written explicitly +(`fields: ['organization_id', 'code']`), and many are legitimately platform-wide +(a DNS hostname, a reserved slug, an external provider id). `'global'` is +accepted there as a synonym of `true` so one vocabulary covers both spellings. + +**Migration is automatic and cannot fail.** Legacy indexes +(`__unique` from knex, `uniq_
_` from the drift-rebuild +path) are retired inline at schema-sync time. The old global constraint is +strictly stronger than the new per-tenant one, so existing rows satisfy the +replacement by construction — no dedup, no cleanup, no data touched. It +converges at sync rather than waiting for a deliberate `os migrate` run because +a deployment that never ran migrate would otherwise stay broken. + +**Upgrading — audit your `unique: true` fields.** On a tenant-scoped object the +constraint is now per tenant. Anything that must stay platform-wide has to say +so: + +```ts +hostname: Field.text({ unique: 'global' }) // no two tenants may claim it +``` + +Note the reach: `applySystemFields` injects `organization_id` into every +registered object unless it opts out, and the driver falls back to that column +when no `tenancy.tenantField` is declared — so most objects are tenant-scoped. +Typical candidates for `'global'`: DNS hostnames, reserved slugs, external +provider ids (Stripe customer/subscription), device identities. + +Postgres materializes `col.unique()` as a table CONSTRAINT rather than a bare +index, so the retirement tries `DROP CONSTRAINT` before `DROP INDEX` — +`DROP INDEX` alone would have made the migration a no-op on exactly the +deployments that matter most. + +`@objectstack/driver-mongodb` accepts the new declaration but keeps single-field +indexes: it implements no row-level tenancy at all (no tenant predicate on read, +no tenant stamp on write), so a `(tenant, field)` index would advertise an +isolation it does not deliver. Tracked separately. diff --git a/content/docs/protocol/objectql/schema.mdx b/content/docs/protocol/objectql/schema.mdx index 3f79bc7544..c5b506a71d 100644 --- a/content/docs/protocol/objectql/schema.mdx +++ b/content/docs/protocol/objectql/schema.mdx @@ -278,7 +278,7 @@ fields: | `type` | `string` | All | **Required.** Field type (see [Types](/docs/protocol/objectql/types)). | | `label` | `string` | All | **Required.** Display label in UI. | | `required` | `boolean` | All | Validation: Field must have a value. | -| `unique` | `boolean` | All | Enforce uniqueness at database level. | +| `unique` | `boolean \| 'global'` | All | Enforce uniqueness at database level. `true` = unique **within the tenant**; `'global'` = unique platform-wide. See [Uniqueness and tenancy](#uniqueness-and-tenancy). | | `searchable` | `boolean` | All | Is searchable. | | `defaultValue` | `any` | All | Default value when creating new records. | | `description` | `string` | All | Tooltip/Help text. | @@ -566,6 +566,54 @@ Supported index `type` values: `btree` (default), `hash`, `gin`, `gist`, `fullte - Fields that change frequently - Large text fields (use full-text search instead) +### Uniqueness and tenancy + +On a tenant-scoped object, **field-level `unique: true` means unique *within* the +tenant**, not platform-wide. The driver materializes it as a composite index +`(tenantField, field)`: + +```yaml +fields: + code: + type: autonumber + autonumberFormat: 'PROD-{00000}' + unique: true # each tenant may hold its own PROD-00001 +``` + +This matches every other tenant-aware part of the platform: reads are filtered by +the tenant predicate, writes stamp the tenant column, and the auto-number sequence +gives each tenant a counter starting at 1. A platform-wide index would contradict +the sequence outright — the second tenant's `PROD-00001` would be rejected by an +index it cannot see, and the rejection itself would reveal that *some other tenant* +holds the value. + +For the identifiers that genuinely are platform-wide — a DNS hostname, a reserved +slug, an external provider id, a device identity — say so explicitly: + +```yaml +fields: + hostname: + type: text + unique: global # no two tenants may claim the same hostname +``` + +On an object with no tenant column (`tenancy: { enabled: false }`, or simply no +tenant field) both spellings behave identically. **Single-tenant deployments are +unaffected** — the tenant column is constant, so the composite index degenerates +to the single-column one. + +**Declared `indexes` are different:** they are materialized over exactly the +columns you list, with no tenant column injected. Write the tenant column yourself +when you want a per-tenant index: + +```yaml +indexes: + - fields: [organization_id, code] # unique per tenant + unique: true + - fields: [hostname] # unique platform-wide + unique: true +``` + ## Lifecycle Hooks Record-triggered logic is **not** an object-schema field — `triggers` (and `hooks`, diff --git a/content/docs/references/data/field.mdx b/content/docs/references/data/field.mdx index ff890236c2..985cc4ed78 100644 --- a/content/docs/references/data/field.mdx +++ b/content/docs/references/data/field.mdx @@ -14,8 +14,8 @@ Field Type Enum ## TypeScript Usage ```typescript -import { Address, ComputedFieldCache, CurrencyConfig, CurrencyValue, DataQualityRules, Field, FieldType, LocationCoordinates, SelectOption } from '@objectstack/spec/data'; -import type { Address, ComputedFieldCache, CurrencyConfig, CurrencyValue, DataQualityRules, Field, FieldType, LocationCoordinates, SelectOption } from '@objectstack/spec/data'; +import { Address, ComputedFieldCache, CurrencyConfig, CurrencyValue, DataQualityRules, Field, FieldType, LocationCoordinates, SelectOption, UniqueScope } from '@objectstack/spec/data'; +import type { Address, ComputedFieldCache, CurrencyConfig, CurrencyValue, DataQualityRules, Field, FieldType, LocationCoordinates, SelectOption, UniqueScope } from '@objectstack/spec/data'; // Validate data const result = Address.parse(data); @@ -105,7 +105,7 @@ const result = Address.parse(data); | **required** | `boolean` | optional | Is required | | **searchable** | `boolean` | optional | Is searchable | | **multiple** | `boolean` | optional | Allow multiple values (Stores as Array/JSON). Applicable for select, lookup, file, image. | -| **unique** | `boolean` | optional | Is unique constraint | +| **unique** | `boolean \| 'global'` | optional | Unique constraint. true = unique within the tenant (composite with the tenant column on tenant-scoped objects); 'global' = unique platform-wide across all tenants | | **defaultValue** | `any` | optional | Default value | | **maxLength** | `number` | optional | Max character length | | **minLength** | `number` | optional | Min character length | @@ -245,3 +245,24 @@ const result = Address.parse(data); --- +## UniqueScope + +### Union Options + +This schema accepts one of the following structures: + +#### Option 1 + +Type: `boolean` + +--- + +#### Option 2 + +Type: `'global'` + +--- + + +--- + diff --git a/content/docs/references/data/object.mdx b/content/docs/references/data/object.mdx index f4c025071b..3e6f6dc1f5 100644 --- a/content/docs/references/data/object.mdx +++ b/content/docs/references/data/object.mdx @@ -68,7 +68,7 @@ const result = ApiMethod.parse(data); | **name** | `string` | optional | Index name (auto-generated if not provided) | | **fields** | `string[]` | ✅ | Fields included in the index | | **type** | `Enum<'btree' \| 'hash' \| 'gin' \| 'gist' \| 'fulltext'>` | ✅ | Index algorithm type | -| **unique** | `boolean` | ✅ | Whether the index enforces uniqueness | +| **unique** | `boolean \| 'global'` | ✅ | Whether the index enforces uniqueness. Materialized over exactly `fields` — no tenant column is injected; list the tenant column explicitly for a per-tenant index. 'global' is a synonym of true, for symmetry with field-level `unique` | | **partial** | `string` | optional | Partial index condition (SQL WHERE clause for conditional indexes) | @@ -122,7 +122,7 @@ const result = ApiMethod.parse(data); | **datasource** | `string` | optional | Target Datasource ID. "default" is the primary DB. | | **external** | `{ remoteName?: string; remoteSchema?: string; writable?: boolean; columnMap?: Record; … }` | optional | Remote table binding for federated (external) objects. | | **fields** | `Record; description?: string; … }>` | ✅ | Field definitions map. Keys must be snake_case identifiers. | -| **indexes** | `{ name?: string; fields: string[]; type?: Enum<'btree' \| 'hash' \| 'gin' \| 'gist' \| 'fulltext'>; unique?: boolean; … }[]` | optional | Database performance indexes | +| **indexes** | `{ name?: string; fields: string[]; type?: Enum<'btree' \| 'hash' \| 'gin' \| 'gist' \| 'fulltext'>; unique?: boolean \| 'global'; … }[]` | optional | Database performance indexes | | **fieldGroups** | `{ key: string; label: string; icon?: string; description?: string; … }[]` | optional | Ordered list of field groups (array order = display order). See ObjectFieldGroupSchema. | | **tenancy** | `{ enabled: boolean; tenantField?: string }` | optional | Multi-tenancy configuration for SaaS applications | | **access** | `{ default?: Enum<'public' \| 'private'> }` | optional | [ADR-0066 D2] Object exposure posture (public-by-default vs private secure-by-default). | @@ -196,7 +196,7 @@ const result = ApiMethod.parse(data); | **pluralLabel** | `string` | optional | Override plural label for the extended object | | **description** | `string` | optional | Override description for the extended object | | **validations** | `any[]` | optional | Additional validation rules to merge into the target object | -| **indexes** | `{ name?: string; fields: string[]; type?: Enum<'btree' \| 'hash' \| 'gin' \| 'gist' \| 'fulltext'>; unique?: boolean; … }[]` | optional | Additional indexes to merge into the target object | +| **indexes** | `{ name?: string; fields: string[]; type?: Enum<'btree' \| 'hash' \| 'gin' \| 'gist' \| 'fulltext'>; unique?: boolean \| 'global'; … }[]` | optional | Additional indexes to merge into the target object | | **priority** | `integer` | optional | Merge priority (higher = applied later) | diff --git a/packages/plugins/driver-mongodb/src/mongodb-schema.ts b/packages/plugins/driver-mongodb/src/mongodb-schema.ts index 2a07880c66..c526468a7c 100644 --- a/packages/plugins/driver-mongodb/src/mongodb-schema.ts +++ b/packages/plugins/driver-mongodb/src/mongodb-schema.ts @@ -14,7 +14,19 @@ import type { Db, CreateIndexesOptions, IndexSpecification } from 'mongodb'; */ interface FieldDef { type?: string; - unique?: boolean; + /** + * `true` | `'global'` — see `UniqueScopeSchema` in @objectstack/spec. + * + * Both spellings materialize the SAME single-field unique index here, and + * that is currently correct for this driver: unlike the SQL driver it + * implements NO row-level tenancy at all (no tenant predicate on read, no + * tenant stamp on write), so there is no tenant column to compose with. A + * `(tenant, field)` index would advertise an isolation this driver does not + * deliver — worse than the single-field index, because it would read as + * fixed. The tenancy gap itself is tracked separately; when it lands, this + * must adopt the same scoping rule as `SqlDriver.uniqueIndexesFromFields`. + */ + unique?: boolean | 'global'; indexed?: boolean; required?: boolean; reference_to?: string; diff --git a/packages/plugins/driver-sql/src/sql-driver-unique-tenancy.test.ts b/packages/plugins/driver-sql/src/sql-driver-unique-tenancy.test.ts new file mode 100644 index 0000000000..e4a4a07cbc --- /dev/null +++ b/packages/plugins/driver-sql/src/sql-driver-unique-tenancy.test.ts @@ -0,0 +1,396 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { SqlDriver } from '../src/index.js'; + +/** + * Tenant-scoped `unique` materialization (#3696). + * + * The defect this locks down: `unique: true` used to become a single-column + * global index that ignored `tenancy` entirely, while the autonumber sequence + * table is keyed by `(object, tenant_id, field, scope)` and hands every tenant + * its own counter starting at 1. Two subsystems of the same platform therefore + * disagreed — tenant B's `PROD-00001` was rejected by an index it could not + * see, with no user error involved — and the rejection itself leaked that some + * OTHER tenant held the value (a cross-tenant existence oracle). + * + * The contract now: + * - `unique: true` + tenant column → composite `(tenantField, field)` + * - `unique: true`, no tenant column → single-column (single-tenant: unchanged) + * - `unique: 'global'` → single-column, always + * - declared `indexes[]` → verbatim columns, never rewritten + */ +describe('SqlDriver unique × tenancy (#3696)', () => { + let driver: SqlDriver; + + beforeEach(async () => { + driver = new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }); + }); + + afterEach(async () => { + await driver.disconnect(); + }); + + /** Index name → column list, for the non-PK indexes on a table. */ + async function indexColumns(table: string): Promise> { + const k = (driver as any).knex; + const list: any = await k.raw(`PRAGMA index_list(${table})`); + const out: Record = {}; + for (const idx of list) { + if (idx.origin === 'pk') continue; + const info: any = await k.raw(`PRAGMA index_info(${idx.name})`); + out[idx.name] = info.map((c: any) => c.name); + } + return out; + } + + async function uniqueIndexColumns(table: string): Promise> { + const k = (driver as any).knex; + const list: any = await k.raw(`PRAGMA index_list(${table})`); + const out: Record = {}; + for (const idx of list) { + if (idx.origin === 'pk' || idx.unique !== 1) continue; + const info: any = await k.raw(`PRAGMA index_info(${idx.name})`); + out[idx.name] = info.map((c: any) => c.name); + } + return out; + } + + // ── The scenario from the issue: autonumber + unique, two tenants ────────── + + it('lets two tenants hold the same unique code (the autonumber collision)', async () => { + await driver.initObjects([ + { + name: 'product', + fields: { + organization_id: { type: 'string' }, + code: { type: 'autonumber', format: 'PROD-{00000}', unique: true }, + name: { type: 'string' }, + }, + }, + ]); + + // Each tenant's sequence starts at 1 — so both mint PROD-00001. Before the + // fix the second insert died with SQLITE_CONSTRAINT_UNIQUE. + const a = await driver.create('product', { organization_id: 'org_a', name: 'A' }); + const b = await driver.create('product', { organization_id: 'org_b', name: 'B' }); + + expect(a.code).toBe('PROD-00001'); + expect(b.code).toBe('PROD-00001'); + }); + + it('still rejects a duplicate code WITHIN one tenant', async () => { + await driver.initObjects([ + { + name: 'product', + fields: { + organization_id: { type: 'string' }, + code: { type: 'string', unique: true }, + }, + }, + ]); + + await driver.create('product', { organization_id: 'org_a', code: 'X-1' }); + await expect( + driver.create('product', { organization_id: 'org_a', code: 'X-1' }), + ).rejects.toThrow(/UNIQUE constraint failed|duplicate key value/); + + // …and the same value in a different tenant is fine. + const other = await driver.create('product', { organization_id: 'org_b', code: 'X-1' }); + expect(other.code).toBe('X-1'); + }); + + it("keeps `unique: 'global'` unique across tenants", async () => { + await driver.initObjects([ + { + name: 'runtime', + fields: { + organization_id: { type: 'string' }, + hostname: { type: 'string', unique: 'global' }, + }, + }, + ]); + + await driver.create('runtime', { organization_id: 'org_a', hostname: 'app.example.com' }); + await expect( + driver.create('runtime', { organization_id: 'org_b', hostname: 'app.example.com' }), + ).rejects.toThrow(/UNIQUE constraint failed|duplicate key value/); + }); + + // ── Physical shape ──────────────────────────────────────────────────────── + + it('materializes a COMPOSITE (tenant, field) unique index, tenant column first', async () => { + await driver.initObjects([ + { + name: 'product', + fields: { + organization_id: { type: 'string' }, + code: { type: 'string', unique: true }, + }, + }, + ]); + + const uniques = await uniqueIndexColumns('product'); + expect(Object.values(uniques)).toContainEqual(['organization_id', 'code']); + // No leftover single-column global unique on `code`. + expect(Object.values(uniques)).not.toContainEqual(['code']); + }); + + it('materializes a SINGLE-column unique when the object has no tenant column', async () => { + await driver.initObjects([ + { name: 'widget', fields: { sku: { type: 'string', unique: true } } }, + ]); + + const uniques = await uniqueIndexColumns('widget'); + expect(Object.values(uniques)).toContainEqual(['sku']); + }); + + it('materializes a SINGLE-column unique when tenancy is explicitly disabled', async () => { + await driver.initObjects([ + { + name: 'catalog_entry', + tenancy: { enabled: false }, + fields: { + organization_id: { type: 'string' }, + slug: { type: 'string', unique: true }, + }, + } as any, + ]); + + const uniques = await uniqueIndexColumns('catalog_entry'); + expect(Object.values(uniques)).toContainEqual(['slug']); + expect(Object.values(uniques)).not.toContainEqual(['organization_id', 'slug']); + + // Behaviorally: a platform-global catalog still rejects cross-org dupes. + await driver.create('catalog_entry', { organization_id: 'org_a', slug: 's' }); + await expect( + driver.create('catalog_entry', { organization_id: 'org_b', slug: 's' }), + ).rejects.toThrow(/UNIQUE constraint failed|duplicate key value/); + }); + + it('does not compose a unique declared ON the tenant column itself', async () => { + // "one row per tenant" — `(organization_id, organization_id)` is not a + // constraint, so this must stay single-column. + await driver.initObjects([ + { + name: 'billing_customer', + fields: { organization_id: { type: 'string', unique: true } }, + }, + ]); + + const uniques = await uniqueIndexColumns('billing_customer'); + expect(Object.values(uniques)).toContainEqual(['organization_id']); + + await driver.create('billing_customer', { organization_id: 'org_a' }); + await expect(driver.create('billing_customer', { organization_id: 'org_a' })).rejects.toThrow( + /UNIQUE constraint failed|duplicate key value/, + ); + }); + + it('honors a declared tenancy.tenantField other than organization_id', async () => { + await driver.initObjects([ + { + name: 'env_setting', + tenancy: { enabled: true, tenantField: 'environment_id' }, + fields: { + environment_id: { type: 'string' }, + key: { type: 'string', unique: true }, + }, + } as any, + ]); + + const uniques = await uniqueIndexColumns('env_setting'); + 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' }); + expect(other.key).toBe('k'); + }); + + // ── Declared indexes are NOT rewritten ──────────────────────────────────── + + it('leaves declared object-level indexes exactly as authored', async () => { + // A declared index names its own columns. Many are platform-wide on + // purpose (a DNS hostname, a reserved slug, a Stripe customer id), so + // injecting a tenant column here would silently break them. + await driver.initObjects([ + { + name: 'slug_reservation', + fields: { + organization_id: { type: 'string' }, + slug: { type: 'string' }, + }, + indexes: [{ fields: ['slug'], unique: true }], + } as any, + ]); + + const uniques = await uniqueIndexColumns('slug_reservation'); + expect(Object.values(uniques)).toContainEqual(['slug']); + expect(Object.values(uniques)).not.toContainEqual(['organization_id', 'slug']); + + await driver.create('slug_reservation', { organization_id: 'org_a', slug: 'acme' }); + await expect( + driver.create('slug_reservation', { organization_id: 'org_b', slug: 'acme' }), + ).rejects.toThrow(/UNIQUE constraint failed|duplicate key value/); + }); + + it("accepts unique: 'global' on a declared index as a synonym of true", async () => { + await driver.initObjects([ + { + name: 'domain', + fields: { organization_id: { type: 'string' }, host: { type: 'string' } }, + indexes: [{ fields: ['host'], unique: 'global' }], + } as any, + ]); + + const uniques = await uniqueIndexColumns('domain'); + expect(Object.values(uniques)).toContainEqual(['host']); + }); + + // ── Legacy migration: drop the global index, create the composite ───────── + + it('retires a legacy global unique index and replaces it with the composite', async () => { + const k = (driver as any).knex; + + // Reproduce exactly what the OLD code left behind: knex's `col.unique()` + // naming from `createColumn`. + await k.schema.createTable('product', (t: any) => { + t.string('id').primary(); + t.timestamp('created_at'); + t.timestamp('updated_at'); + t.string('organization_id'); + t.string('code'); + }); + await k.raw('CREATE UNIQUE INDEX product_code_unique ON product (code)'); + + // Pre-existing rows satisfy the new (weaker) constraint by construction — + // the migration is a pure relaxation, so it cannot fail on real data. + await k('product').insert([ + { id: 'r1', organization_id: 'org_a', code: 'PROD-00001' }, + { id: 'r2', organization_id: 'org_a', code: 'PROD-00002' }, + ]); + + await driver.initObjects([ + { + name: 'product', + fields: { + organization_id: { type: 'string' }, + code: { type: 'string', unique: true }, + }, + }, + ]); + + const uniques = await uniqueIndexColumns('product'); + expect(uniques['product_code_unique']).toBeUndefined(); + expect(Object.values(uniques)).toContainEqual(['organization_id', 'code']); + + // Existing data survived, and the cross-tenant insert now works. + expect(await driver.count('product', {} as any)).toBe(2); + const b = await driver.create('product', { organization_id: 'org_b', code: 'PROD-00001' }); + expect(b.code).toBe('PROD-00001'); + }); + + it('retires the legacy `uniq_
_` index left by the drift rebuild path', async () => { + const k = (driver as any).knex; + await k.schema.createTable('product', (t: any) => { + t.string('id').primary(); + t.string('organization_id'); + t.string('code'); + }); + await k.raw('CREATE UNIQUE INDEX uniq_product_code ON product (code)'); + + await driver.initObjects([ + { + name: 'product', + fields: { + organization_id: { type: 'string' }, + code: { type: 'string', unique: true }, + }, + }, + ]); + + const uniques = await uniqueIndexColumns('product'); + expect(uniques['uniq_product_code']).toBeUndefined(); + expect(Object.values(uniques)).toContainEqual(['organization_id', 'code']); + }); + + it("does NOT retire the single-column index of a unique: 'global' field", async () => { + const k = (driver as any).knex; + await k.schema.createTable('runtime', (t: any) => { + t.string('id').primary(); + t.string('organization_id'); + t.string('hostname'); + }); + await k.raw('CREATE UNIQUE INDEX uniq_runtime_hostname ON runtime (hostname)'); + + await driver.initObjects([ + { + name: 'runtime', + fields: { + organization_id: { type: 'string' }, + hostname: { type: 'string', unique: 'global' }, + }, + }, + ]); + + const uniques = await uniqueIndexColumns('runtime'); + expect(uniques['uniq_runtime_hostname']).toEqual(['hostname']); + }); + + it('is idempotent across repeated initObjects runs', async () => { + const objects = [ + { + name: 'product', + fields: { + organization_id: { type: 'string' }, + code: { type: 'string', unique: true }, + }, + }, + ]; + + await driver.initObjects(objects); + const first = await indexColumns('product'); + await driver.initObjects(objects); + const second = await indexColumns('product'); + + expect(second).toEqual(first); + }); + + // ── The SQLite drift-rebuild path must land on the same DDL ─────────────── + + it('keeps the composite (not a global index) after a SQLite drift rebuild', async () => { + // The rebuild drops and recreates the table, so every index is + // re-materialized from metadata. It used to inline its own single-column + // `uniq_
_` DDL — silently re-introducing the global index on a + // tenant-scoped field after any drift reconcile. + await driver.initObjects([ + { + name: 'product', + fields: { + organization_id: { type: 'string' }, + code: { type: 'string', unique: true }, + note: { type: 'string', required: true }, + }, + }, + ]); + await driver.create('product', { organization_id: 'org_a', code: 'C1', note: 'n' }); + + // Relax `note` NOT NULL → forces the rebuild path on SQLite. + await (driver as any).rebuildSqliteTablePatched('product', [ + { table: 'product', column: 'note', op: { type: 'relax_not_null', table: 'product', column: 'note' } }, + ]); + + const uniques = await uniqueIndexColumns('product'); + expect(Object.values(uniques)).toContainEqual(['organization_id', 'code']); + expect(Object.values(uniques)).not.toContainEqual(['code']); + + // Data preserved, and cross-tenant reuse still works post-rebuild. + expect(await driver.count('product', {} as any)).toBe(1); + const b = await driver.create('product', { organization_id: 'org_b', code: 'C1', note: 'n' }); + expect(b.code).toBe('C1'); + }); +}); diff --git a/packages/plugins/driver-sql/src/sql-driver.ts b/packages/plugins/driver-sql/src/sql-driver.ts index 763259a790..687b5554dc 100644 --- a/packages/plugins/driver-sql/src/sql-driver.ts +++ b/packages/plugins/driver-sql/src/sql-driver.ts @@ -8,7 +8,7 @@ */ import type { QueryAST, DriverOptions, SchemaMode } from '@objectstack/spec/data'; -import { parseAutonumberFormat, renderAutonumber, missingFieldValues, isTenancyDisabled, type AutonumberToken } from '@objectstack/spec/data'; +import { parseAutonumberFormat, renderAutonumber, missingFieldValues, isTenancyDisabled, isGlobalUnique, isUniqueDeclared, type AutonumberToken } from '@objectstack/spec/data'; import { STRUCTURED_JSON_TYPES, FILE_REFERENCE_TYPES, MULTI_OPTION_TYPES, NUMERIC_VALUE_TYPES } from '@objectstack/spec/data'; import type { IDataDriver } from '@objectstack/spec/contracts'; import { StorageNameMapping } from '@objectstack/spec/system'; @@ -2257,16 +2257,29 @@ export class SqlDriver implements IDataDriver { }); } - // Materialize object-level declared indexes (`indexes: [{ fields, - // unique }]`). These are distinct from field-level `unique` (handled - // in `createColumn`) and carry the multi-column UNIQUE guarantees that - // dedup/convergence paths rely on (ADR-0030). Done after the table is - // created/altered so every referenced column physically exists. + // Materialize the table's index set: field-level `unique` (tenancy-aware + // — #3696) plus object-level declared indexes (`indexes: [{ fields, + // unique }]`, carrying the multi-column UNIQUE guarantees dedup/ + // convergence paths rely on, ADR-0030). Both go through one path so the + // create and alter branches above cannot produce different constraints + // for the same metadata. Done after the table is created/altered so every + // referenced column physically exists — which is also why field-level + // `unique` can no longer be emitted inline by `createColumn`: a composite + // needs the tenant column to already be there. const declaredIndexes = (obj as any).indexes; - if (Array.isArray(declaredIndexes) && declaredIndexes.length > 0) { + const uniqueFields = Object.values(obj.fields ?? {}).some((f) => + isUniqueDeclared(f?.unique), + ); + if (uniqueFields || (Array.isArray(declaredIndexes) && declaredIndexes.length > 0)) { const colInfo = await this.knex(tableName).columnInfo(); const physicalColumns = new Set(Object.keys(colInfo)); - await this.syncDeclaredIndexes(tableName, declaredIndexes, physicalColumns); + await this.syncTableIndexes( + tableName, + obj.fields ?? {}, + declaredIndexes, + tenantField, + physicalColumns, + ); } // #2186: the additive sync above only ever ADDs tables/columns. For a @@ -2556,19 +2569,22 @@ export class SqlDriver implements IDataDriver { await this.knex.raw('PRAGMA foreign_keys = ON'); } - // Recreate unique constraints + declared indexes from metadata. + // Recreate unique constraints + declared indexes from metadata. The rebuild + // dropped the original table, so every index went with it — this is a fresh + // materialization, and it must produce the SAME constraints as + // `initObjects` would. It used to inline its own `uniq_
_` + // single-column DDL, which is how a tenant-scoped field could silently + // regain a global unique index after any drift rebuild (#3696); both paths + // now share {@link syncTableIndexes}. try { const keptSet = new Set(keptNames); - for (const [name, field] of Object.entries(fields)) { - if (field?.unique && keptSet.has(name)) { - const idx = `uniq_${table}_${name}`; - await this.knex.raw('CREATE UNIQUE INDEX IF NOT EXISTS ?? ON ?? (??)', [idx, table, name]); - } - } - const declared = this.managedObjectIndexes.get(table); - if (Array.isArray(declared) && declared.length > 0) { - await this.syncDeclaredIndexes(table, declared, keptSet); - } + await this.syncTableIndexes( + table, + fields, + this.managedObjectIndexes.get(table), + this.resolveTenantField(table), + keptSet, + ); } catch (e: any) { this.logger.warn(`[schema-drift] could not fully recreate indexes for '${table}' after rebuild`, e?.message ?? e); } @@ -2637,6 +2653,167 @@ export class SqlDriver implements IDataDriver { return names; } + /** + * Translate field-level `unique` declarations into concrete index + * descriptors, applying tenant scoping (#3696). + * + * This is the ONLY place field-level uniqueness becomes physical, so the + * create-table, alter-table and SQLite-rebuild paths cannot drift apart the + * way they had (each used to inline its own single-column DDL). + * + * Scoping rule: + * - `unique: 'global'` → single-column `(field)`, platform-wide. + * - `unique: true` on a tenant-scoped table → composite `(tenantField, + * field)`: unique *within* the tenant, matching the per-tenant autonumber + * sequence, the RLS read predicate and the write-path tenant stamp. + * - `unique: true` with no tenant column → single-column `(field)`. Single- + * tenant deployments therefore see byte-identical DDL to before. + * + * The tenant column comes FIRST in the composite so the index also serves the + * `WHERE tenant = ?` prefix scans every tenant-scoped read issues. + */ + protected uniqueIndexesFromFields( + tableName: string, + fields: Record, + tenantField: string | null, + ): Array<{ name: string; fields: string[]; unique: true }> { + const out: Array<{ name: string; fields: string[]; unique: true }> = []; + for (const [name, field] of Object.entries(fields ?? {})) { + if (!isUniqueDeclared(field?.unique)) continue; + // A unique declaration ON the tenant column itself ("one row per tenant") + // cannot be tenant-scoped — `(organization_id, organization_id)` is not a + // constraint. Keep it single-column. + const scoped = + !isGlobalUnique(field.unique) && tenantField != null && tenantField !== name; + const cols = scoped ? [tenantField, name] : [name]; + out.push({ name: this.buildIndexName(tableName, cols, true), fields: cols, unique: true }); + } + return out; + } + + /** + * Legacy single-column unique indexes that a tenant-scoped field's constraint + * used to be materialized as, and which must be dropped once the composite + * replaces them (#3696). Two spellings ever existed: + * + * - `
__unique` — knex's default name for `col.unique()`, + * emitted by the old `createColumn`. A real CONSTRAINT on Postgres, a + * plain index on SQLite/MySQL. + * - `uniq_
_` — {@link buildIndexName}, emitted by the old + * SQLite rebuild path. + * + * Dropping them is a pure RELAXATION: the old global constraint is strictly + * stronger than the new per-tenant one, so existing data satisfies the + * replacement by construction. No dedup, no cleanup, no possible failure — + * which is why this converges inline at sync time instead of waiting for a + * deliberate `os migrate` run (a deployment that never ran migrate would + * otherwise stay broken, and the failure mode is cross-tenant data loss on + * insert). + */ + protected legacySingleColumnUniqueNames(tableName: string, column: string): string[] { + return [`${tableName}_${column}_unique`, this.buildIndexName(tableName, [column], true)]; + } + + /** + * Drop a unique index/constraint by name if present, across dialects. + * + * Postgres materializes knex's `col.unique()` as a table CONSTRAINT (not a + * bare index), so `DROP INDEX` alone silently leaves it in place — the exact + * failure that would have made this migration a no-op on the deployments that + * matter most. Try the constraint form first, then the index form. + * + * Returns true when something was actually dropped. + */ + protected async dropUniqueIndexIfExists(tableName: string, indexName: string): Promise { + const attempts: string[] = this.isPostgres + ? [ + `ALTER TABLE ?? DROP CONSTRAINT IF EXISTS ??`, + `DROP INDEX IF EXISTS ??`, + ] + : this.isMysql + ? [`ALTER TABLE ?? DROP INDEX ??`] + : [`DROP INDEX IF EXISTS ??`]; + + let dropped = false; + for (const sql of attempts) { + try { + const bindings = sql.startsWith('ALTER TABLE') ? [tableName, indexName] : [indexName]; + await this.knex.raw(sql, bindings); + dropped = true; + } catch (e: any) { + const msg = String(e?.message ?? e); + // "does not exist" / "check that column/key exists" — nothing to drop. + if (/does not exist|doesn't exist|no such index|check that column/i.test(msg)) continue; + throw e; + } + } + return dropped; + } + + /** + * Retire the legacy global unique indexes for every field whose constraint is + * now tenant-scoped (#3696). Runs before the composites are created so a + * Postgres constraint and its replacement never contend for a name. + */ + protected async dropLegacyGlobalUniques( + tableName: string, + fields: Record, + tenantField: string | null, + physicalColumns: Set, + ): Promise { + if (!tenantField) return; // Nothing was ever mis-scoped on a tenant-less table. + const existing = await this.getExistingIndexNames(tableName); + for (const [name, field] of Object.entries(fields ?? {})) { + if (!isUniqueDeclared(field?.unique)) continue; + // `'global'` keeps its single-column index — that is now the declared + // intent, not legacy debt. + if (isGlobalUnique(field.unique)) continue; + if (name === tenantField || !physicalColumns.has(name)) continue; + + for (const legacy of this.legacySingleColumnUniqueNames(tableName, name)) { + // On Postgres a unique CONSTRAINT may not surface in `pg_indexes` under + // a name we can match, so attempt the drop when either the name is + // known to exist or the dialect hides constraints from us. + if (!existing.has(legacy) && !this.isPostgres) continue; + try { + const dropped = await this.dropUniqueIndexIfExists(tableName, legacy); + if (dropped) { + (this.logger.info ?? this.logger.warn)( + `[sql-driver] dropped legacy global unique index '${legacy}' on '${tableName}' — ` + + `${tableName}.${name} is now unique per '${tenantField}' (#3696). ` + + `Declare \`unique: 'global'\` if platform-wide uniqueness was intended.`, + ); + } + } catch (e: any) { + this.logger.warn( + `[sql-driver] could not drop legacy unique index '${legacy}' on '${tableName}'`, + e?.message ?? e, + ); + } + } + } + } + + /** + * Materialize a table's full index set: field-level `unique` (tenancy-aware, + * see {@link uniqueIndexesFromFields}) plus the object's declared `indexes`. + * Single entry point so every caller — create, alter, SQLite rebuild — lands + * on identical DDL. + */ + protected async syncTableIndexes( + tableName: string, + fields: Record, + declaredIndexes: any[] | undefined, + tenantField: string | null, + physicalColumns: Set, + ): Promise { + await this.dropLegacyGlobalUniques(tableName, fields, tenantField, physicalColumns); + const fromFields = this.uniqueIndexesFromFields(tableName, fields, tenantField); + const declared = Array.isArray(declaredIndexes) ? declaredIndexes : []; + if (fromFields.length === 0 && declared.length === 0) return; + await this.syncDeclaredIndexes(tableName, [...fromFields, ...declared], physicalColumns); + } + /** * Materialize declared object-level indexes. * @@ -2645,6 +2822,11 @@ export class SqlDriver implements IDataDriver { * default across SQLite/Postgres/MySQL, so multiple NULL rows remain * allowed while non-NULL duplicates are rejected — matching the * convergence-on-conflict pattern the messaging pipeline relies on. + * - The column list is taken VERBATIM — no tenant column is injected here. + * Field-level `unique` is tenancy-scoped upstream in + * {@link uniqueIndexesFromFields}; a declared index already names its + * columns and is frequently platform-wide on purpose (a DNS hostname, a + * reserved slug, an external provider id), so guessing would break it. * - Idempotent: indexes already present (by deterministic name) are * skipped, and an "already exists" race is absorbed. * - Indexes referencing a column that wasn't materialized (e.g. a virtual @@ -2652,7 +2834,7 @@ export class SqlDriver implements IDataDriver { */ protected async syncDeclaredIndexes( tableName: string, - indexes: Array<{ name?: string; fields?: string[]; unique?: boolean }>, + indexes: Array<{ name?: string; fields?: string[]; unique?: boolean | 'global' }>, physicalColumns: Set, ): Promise { const existing = await this.getExistingIndexNames(tableName); @@ -2672,7 +2854,7 @@ export class SqlDriver implements IDataDriver { continue; } - const unique = idx.unique === true; + const unique = isUniqueDeclared(idx.unique); const name = typeof idx.name === 'string' && idx.name.trim() ? idx.name.trim() @@ -3617,7 +3799,18 @@ export class SqlDriver implements IDataDriver { } if (col) { - if (field.unique) col.unique(); + // NOTE: field-level `unique` is deliberately NOT emitted here (#3696). + // A column builder can only express a SINGLE-column constraint, but on a + // tenant-scoped object `unique: true` means unique *within the tenant* — + // a composite `(tenantField, field)` index. Emitting `col.unique()` here + // is what made every tenant share one global namespace, which collided + // head-on with the per-tenant autonumber sequence (each tenant counts + // from 1, so the second tenant's `PROD-00001` was rejected by an index it + // could not see). All unique materialization now goes through the single + // tenancy-aware path — {@link uniqueIndexesFromFields} + + // {@link syncDeclaredIndexes} — which runs after the table exists and can + // therefore build composites. `createColumn` owns type/nullability/ + // defaults only. if (field.required) col.notNullable(); // `defaultValue: 'NOW()'` is a framework convention for "use the // database clock at insert time". Translate it to the driver-native diff --git a/packages/spec/api-surface.json b/packages/spec/api-surface.json index 3a1a64ef71..f036ae787b 100644 --- a/packages/spec/api-surface.json +++ b/packages/spec/api-surface.json @@ -530,6 +530,8 @@ "TimeUpdateInterval (const)", "TitleEligibleFieldDef (interface)", "TransformType (const)", + "UniqueScope (type)", + "UniqueScopeSchema (const)", "VALID_AST_OPERATORS (const)", "ValidationRule (type)", "ValidationRuleSchema (const)", @@ -563,12 +565,14 @@ "isDateMacroToken (function)", "isFileIdToken (function)", "isFilterAST (function)", + "isGlobalUnique (function)", "isIncoherentAggregate (function)", "isKnownFilterToken (function)", "isLegacyApiMethod (function)", "isMultiValueField (function)", "isTenancyDisabled (function)", "isTitleEligible (function)", + "isUniqueDeclared (function)", "missingFieldValues (function)", "objectForm (const)", "objectTitleCompleteness (function)", diff --git a/packages/spec/json-schema.manifest.json b/packages/spec/json-schema.manifest.json index d73db3c904..995c958360 100644 --- a/packages/spec/json-schema.manifest.json +++ b/packages/spec/json-schema.manifest.json @@ -814,6 +814,7 @@ "data/TenancyConfig", "data/TimeUpdateInterval", "data/TransformType", + "data/UniqueScope", "data/ValidationRule", "data/WindowFunction", "data/WindowFunctionNode", diff --git a/packages/spec/src/data/field.zod.ts b/packages/spec/src/data/field.zod.ts index 7712331196..18b698f727 100644 --- a/packages/spec/src/data/field.zod.ts +++ b/packages/spec/src/data/field.zod.ts @@ -258,6 +258,54 @@ export const ComputedFieldCacheSchema = lazySchema(() => z.object({ * defaultValue: "open" * } */ +/** + * Uniqueness scope for a `unique` constraint (#3696). + * + * `unique: true` on a tenant-scoped object materializes as a COMPOSITE unique + * index `(tenantField, field)` — "unique within the tenant" — matching how + * every other tenant-aware subsystem already behaves (reads are RLS-filtered, + * writes stamp the tenant column, and the autonumber sequence table is keyed by + * `(object, tenant_id, field, scope)` so each tenant counts from 1). A + * single-column global index contradicted that: two tenants each issuing + * `PROD-00001` collided on an index neither of them could see, and the + * resulting UNIQUE violation doubled as a cross-tenant existence oracle + * (a rejected insert told tenant B that *somebody else* holds the value). + * + * `unique: 'global'` opts back into the old single-column behavior for the + * genuinely platform-wide identifiers where it is correct: an external + * provider id (`stripe_customer_id`), a DNS hostname, a globally reserved + * slug, a device identity. Global uniqueness is the special case and now has + * to say so. + * + * On an object with no tenant column (`tenancy.enabled: false`, or simply no + * tenant field) both spellings materialize identically — single-column unique. + * Single-tenant deployments are therefore unaffected: the tenant column is + * constant, so the composite index degenerates to the single-column one. + */ +export const UniqueScopeSchema = lazySchema(() => + z.union([z.boolean(), z.literal('global')]), +); + +/** @see UniqueScopeSchema */ +export type UniqueScope = boolean | 'global'; + +/** + * Does this `unique` declaration ask for platform-wide (cross-tenant) + * uniqueness? Single source of truth for every driver that materializes a + * unique constraint (SQL DDL, Mongo index sync) so they cannot drift. + */ +export function isGlobalUnique(unique: unknown): boolean { + return unique === 'global'; +} + +/** + * Does this `unique` declaration ask for a unique constraint at all? + * Both `true` and `'global'` do; `false`/absent do not. + */ +export function isUniqueDeclared(unique: unknown): boolean { + return unique === true || unique === 'global'; +} + export const FieldSchema = lazySchema(() => z.object({ /** Identity */ name: z.string().regex(/^[a-z_][a-z0-9_]*$/).describe('Machine name (snake_case)').optional(), @@ -275,7 +323,10 @@ export const FieldSchema = lazySchema(() => z.object({ required: z.boolean().default(false).describe('Is required'), searchable: z.boolean().default(false).describe('Is searchable'), multiple: z.boolean().default(false).describe('Allow multiple values (Stores as Array/JSON). Applicable for select, lookup, file, image.'), - unique: z.boolean().default(false).describe('Is unique constraint'), + // `true` = unique WITHIN the tenant on a tenant-scoped object (composite + // `(tenantField, field)` index); `'global'` = platform-wide single-column + // unique. See {@link UniqueScopeSchema} for why `true` is tenant-scoped. + unique: UniqueScopeSchema.default(false).describe("Unique constraint. true = unique within the tenant (composite with the tenant column on tenant-scoped objects); 'global' = unique platform-wide across all tenants"), defaultValue: z.unknown().optional().describe('Default value'), /** Text/String Constraints */ diff --git a/packages/spec/src/data/object.zod.ts b/packages/spec/src/data/object.zod.ts index e98ed94000..63b8877b6d 100644 --- a/packages/spec/src/data/object.zod.ts +++ b/packages/spec/src/data/object.zod.ts @@ -1,7 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { z } from 'zod'; -import { FieldSchema } from './field.zod'; +import { FieldSchema, UniqueScopeSchema } from './field.zod'; import { ValidationRuleSchema } from './validation.zod'; import { ActionSchema } from '../ui/action.zod'; import { ObjectListViewSchema } from '../ui/view.zod'; @@ -276,7 +276,18 @@ export const IndexSchema = lazySchema(() => z.object({ name: z.string().optional().describe('Index name (auto-generated if not provided)'), fields: z.array(z.string()).describe('Fields included in the index'), type: z.enum(['btree', 'hash', 'gin', 'gist', 'fulltext']).optional().default('btree').describe('Index algorithm type'), - unique: z.boolean().optional().default(false).describe('Whether the index enforces uniqueness'), + // A DECLARED index is materialized over exactly the columns listed in + // `fields` — no tenant column is injected, unlike field-level `unique` + // (#3696). The distinction is deliberate: field-level `unique` has no syntax + // for a composite, so its default had to carry the tenant scope; here the + // author already spells the columns out, and many declared indexes are + // legitimately platform-wide (a DNS hostname, a reserved slug, an external + // provider id). Tenant-scoped declared indexes are written explicitly and + // always have been — `fields: ['organization_id', 'code']`. + // + // `'global'` is accepted as a synonym of `true` so a schema can state the + // intent in one vocabulary across both spellings; it changes nothing here. + unique: UniqueScopeSchema.optional().default(false).describe("Whether the index enforces uniqueness. Materialized over exactly `fields` — no tenant column is injected; list the tenant column explicitly for a per-tenant index. 'global' is a synonym of true, for symmetry with field-level `unique`"), partial: z.string().optional().describe('Partial index condition (SQL WHERE clause for conditional indexes)'), })); diff --git a/packages/spec/src/data/unique-scope.test.ts b/packages/spec/src/data/unique-scope.test.ts new file mode 100644 index 0000000000..8b98f945fa --- /dev/null +++ b/packages/spec/src/data/unique-scope.test.ts @@ -0,0 +1,66 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect } from 'vitest'; +import { FieldSchema, isGlobalUnique, isUniqueDeclared } from './field.zod'; +import { IndexSchema } from './object.zod'; + +/** + * `unique` scope contract (#3696). + * + * `unique: true` means "unique within the tenant"; `unique: 'global'` means + * "unique platform-wide". The vocabulary lives here so the SQL driver and the + * Mongo driver read the same declaration the same way rather than each + * re-deriving it (the drift that let three DDL paths disagree in the first + * place). + */ +describe('UniqueScope (#3696)', () => { + describe('FieldSchema.unique', () => { + it('accepts true / false / global', () => { + for (const unique of [true, false, 'global'] as const) { + const parsed = FieldSchema.parse({ type: 'text', unique }); + expect(parsed.unique).toBe(unique); + } + }); + + it('defaults to false', () => { + expect(FieldSchema.parse({ type: 'text' }).unique).toBe(false); + }); + + it('rejects any other string — a typo must not silently mean "not unique"', () => { + // `'tenant'` / `'org'` / `'globl'` are the plausible mis-spellings. Each + // has to be a loud parse error: accepting it as truthy-but-unknown is how + // a constraint quietly changes scope. + for (const bad of ['tenant', 'org', 'globl', 'GLOBAL', '']) { + expect(() => FieldSchema.parse({ type: 'text', unique: bad })).toThrow(); + } + }); + }); + + describe('IndexSchema.unique', () => { + it('accepts true / false / global', () => { + for (const unique of [true, false, 'global'] as const) { + expect(IndexSchema.parse({ fields: ['a'], unique }).unique).toBe(unique); + } + }); + + it('defaults to false', () => { + expect(IndexSchema.parse({ fields: ['a'] }).unique).toBe(false); + }); + }); + + describe('predicates', () => { + it('isUniqueDeclared is true for both true and global', () => { + expect(isUniqueDeclared(true)).toBe(true); + expect(isUniqueDeclared('global')).toBe(true); + expect(isUniqueDeclared(false)).toBe(false); + expect(isUniqueDeclared(undefined)).toBe(false); + }); + + it('isGlobalUnique singles out the platform-wide spelling', () => { + expect(isGlobalUnique('global')).toBe(true); + expect(isGlobalUnique(true)).toBe(false); + expect(isGlobalUnique(false)).toBe(false); + expect(isGlobalUnique(undefined)).toBe(false); + }); + }); +});