|
| 1 | +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. |
| 2 | + |
| 3 | +/** |
| 4 | + * `syncDeclaredIndexes` judges "did existing rows violate the NULL-safe unique |
| 5 | + * I just tried to create?" — the #5030 branch that keeps a dirty database |
| 6 | + * BOOTING (the constraint is logged as not-enforced and reported by the |
| 7 | + * ADR-0120 D4 drift pre-flight) instead of taking the process down. |
| 8 | + * |
| 9 | + * It used to judge that with a private inline regex over the stringified |
| 10 | + * message — `unique constraint failed|duplicate entry|duplicate key value` — |
| 11 | + * the fourth hand-written vocabulary #6250 inventoried. #6543 migrates it onto |
| 12 | + * `@objectstack/types`' `isUniqueViolationError`, passing the ERROR OBJECT so |
| 13 | + * the `code` / `errno` channels are read at all. |
| 14 | + * |
| 15 | + * ## Why this is a live defect and not only a structural one |
| 16 | + * |
| 17 | + * The issue graded the migration `finding`, on the reasoning that "on the three |
| 18 | + * dialects the repo ships, the message channel happens to carry the words". |
| 19 | + * That holds for the DML path (a duplicate INSERT), which is what this |
| 20 | + * package's other tests exercise. It does not hold for the DDL path this |
| 21 | + * branch is in: |
| 22 | + * |
| 23 | + * | dialect | `CREATE UNIQUE INDEX` over duplicate rows says | old regex | |
| 24 | + * |:---|:---|:---| |
| 25 | + * | SQLite | `UNIQUE constraint failed: product.code` | ✅ matched | |
| 26 | + * | MySQL | `ER_DUP_ENTRY: Duplicate entry 'DUP' for key 'uniq_…'` | ✅ matched | |
| 27 | + * | Postgres | `could not create unique index "uniq_…"`, SQLSTATE 23505 | ❌ **missed** | |
| 28 | + * |
| 29 | + * Postgres does not reuse its DML phrasing here: `duplicate key value violates |
| 30 | + * unique constraint` is what a conflicting INSERT says, while a conflicting |
| 31 | + * index BUILD says `could not create unique index "…"` and carries the verdict |
| 32 | + * on `error.code` (SQLSTATE `23505`, `ERRCODE_UNIQUE_VIOLATION`) with the |
| 33 | + * offending tuple on `error.detail`. None of the three old message limbs |
| 34 | + * appear in it — so on Postgres, the one dialect where the boot-survival |
| 35 | + * branch was needed most, it never fired and `throw e` took the boot down. |
| 36 | + * Postgres is a first-class shipped dialect for this package |
| 37 | + * (`description: "… Supports PostgreSQL, MySQL, SQLite via Knex"`). |
| 38 | + * |
| 39 | + * The failures are injected rather than driven through a live Postgres because |
| 40 | + * this package's unit suite boots SQLite only; the shapes below are the wire |
| 41 | + * shapes `pg`/`mysql2` hand knex, message prefix included. |
| 42 | + * |
| 43 | + * ## Why this package's OTHER tests keep their own spelling |
| 44 | + * |
| 45 | + * `sql-driver-schema.test.ts`, `sql-driver-unique-tenancy.test.ts` and |
| 46 | + * `adr0120-three-posture-conformance.test.ts` assert on |
| 47 | + * `/UNIQUE constraint failed|duplicate key value/`. #6543 asked for a decision |
| 48 | + * on those rather than leaving them to the next reader. **They stay as they |
| 49 | + * are, deliberately.** |
| 50 | + * |
| 51 | + * They are not discriminators — they are assertions on what a real driver |
| 52 | + * actually emitted when a real duplicate INSERT was refused, and their job is |
| 53 | + * to prove the constraint EXISTS in the database. Routing them through |
| 54 | + * `isUniqueViolationError` would make them strictly weaker in two ways: |
| 55 | + * |
| 56 | + * 1. The predicate is deliberately broad (four message limbs, three codes, an |
| 57 | + * errno, and a `cause` walk). An assertion through it can no longer |
| 58 | + * distinguish "SQLite refused this row on a unique index" from "some error |
| 59 | + * the predicate happens to accept", which is the whole content of those |
| 60 | + * tests. |
| 61 | + * 2. A test that judges with the same predicate the production path judges |
| 62 | + * with shares that predicate's blind spots — the two stop being |
| 63 | + * independent, and a wrong predicate passes its own tests. That |
| 64 | + * independence is exactly what caught the Postgres hole above. |
| 65 | + * |
| 66 | + * The narrow spelling is therefore the right one THERE, and the shared |
| 67 | + * predicate the right one in `src/sql-driver.ts`. The rule that reconciles |
| 68 | + * them: **judge with the predicate, assert on the literal.** |
| 69 | + */ |
| 70 | + |
| 71 | +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; |
| 72 | +import { SqlDriver } from '../src/index.js'; |
| 73 | +import type { DeclaredIndexInput } from '../src/index.js'; |
| 74 | + |
| 75 | +/** The NULL-safe unique of the #5030 scenario: `COALESCE(organization_id), code`. */ |
| 76 | +const NULL_SAFE_INDEX: DeclaredIndexInput = { |
| 77 | + name: 'uniq_product_organization_id_code', |
| 78 | + fields: ['organization_id', 'code'], |
| 79 | + unique: 'organization', |
| 80 | + nullSafeColumns: ['organization_id'], |
| 81 | +}; |
| 82 | + |
| 83 | +/** The same index with no NULL-safe key part — the `nullSafe.size > 0` guard's false arm. */ |
| 84 | +const PLAIN_INDEX: DeclaredIndexInput = { |
| 85 | + name: 'uniq_product_code', |
| 86 | + fields: ['code'], |
| 87 | + unique: true, |
| 88 | +}; |
| 89 | + |
| 90 | +const PHYSICAL_COLUMNS = new Set(['id', 'organization_id', 'code']); |
| 91 | + |
| 92 | +/** |
| 93 | + * What `pg` hands knex when `CREATE UNIQUE INDEX` finds duplicate rows. |
| 94 | + * knex prefixes the failing statement onto the message; the primary message is |
| 95 | + * `could not create unique index "…"` and the tuple lives on `detail`. |
| 96 | + */ |
| 97 | +function postgresIndexBuildConflict(): Error { |
| 98 | + const err = new Error( |
| 99 | + `create unique index "uniq_product_organization_id_code" on "product" ` + |
| 100 | + `(COALESCE("organization_id", '__global__'), "code") - ` + |
| 101 | + `could not create unique index "uniq_product_organization_id_code"`, |
| 102 | + ); |
| 103 | + Object.assign(err, { |
| 104 | + code: '23505', |
| 105 | + detail: `Key (COALESCE(organization_id, '__global__'::text), code)=(__global__, DUP) is duplicated.`, |
| 106 | + severity: 'ERROR', |
| 107 | + routine: '_bt_check_unique', |
| 108 | + }); |
| 109 | + return err; |
| 110 | +} |
| 111 | + |
| 112 | +/** mysql2's numeric channel with prose the old regex could not read. */ |
| 113 | +function mysqlErrnoOnlyConflict(): Error { |
| 114 | + const err = new Error('alter table `product` add unique `uniq_product_organization_id_code` - ER_DUP_ENTRY'); |
| 115 | + Object.assign(err, { errno: 1062, sqlState: '23000' }); |
| 116 | + return err; |
| 117 | +} |
| 118 | + |
| 119 | +/** A failure that is NOT a unique violation and must keep taking the boot down. */ |
| 120 | +function unrelatedDdlFailure(): Error { |
| 121 | + const err = new Error('create unique index "uniq_product_organization_id_code" - permission denied for table product'); |
| 122 | + Object.assign(err, { code: '42501' }); |
| 123 | + return err; |
| 124 | +} |
| 125 | + |
| 126 | +describe('syncDeclaredIndexes unique-violation discriminator (#6543)', () => { |
| 127 | + let driver: SqlDriver; |
| 128 | + let realKnex: any; |
| 129 | + let errors: string[]; |
| 130 | + let warns: string[]; |
| 131 | + |
| 132 | + /** Make the NULL-safe index creation fail with `err`, and capture the log. */ |
| 133 | + function arm(err: Error): void { |
| 134 | + (driver as any).createNullSafeUniqueIndex = async () => { |
| 135 | + throw err; |
| 136 | + }; |
| 137 | + errors = []; |
| 138 | + warns = []; |
| 139 | + (driver as any).logger = { |
| 140 | + warn: (msg: string) => warns.push(String(msg)), |
| 141 | + error: (msg: string) => errors.push(String(msg)), |
| 142 | + }; |
| 143 | + } |
| 144 | + |
| 145 | + /** Drive the branch under test directly — `initObjects` is not needed to reach it. */ |
| 146 | + function sync(indexes: DeclaredIndexInput[]): Promise<void> { |
| 147 | + return (driver as any).syncDeclaredIndexes('product', indexes, PHYSICAL_COLUMNS, 'organization_id'); |
| 148 | + } |
| 149 | + |
| 150 | + beforeEach(async () => { |
| 151 | + driver = new SqlDriver({ |
| 152 | + client: 'better-sqlite3', |
| 153 | + connection: { filename: ':memory:' }, |
| 154 | + useNullAsDefault: true, |
| 155 | + }); |
| 156 | + realKnex = (driver as any).knex; |
| 157 | + await realKnex.schema.createTable('product', (t: any) => { |
| 158 | + t.string('id').primary(); |
| 159 | + t.string('organization_id'); |
| 160 | + t.string('code'); |
| 161 | + }); |
| 162 | + }); |
| 163 | + |
| 164 | + afterEach(async () => { |
| 165 | + // One test stands in for `knex`; put the real one back so teardown closes it. |
| 166 | + (driver as any).knex = realKnex; |
| 167 | + await driver.disconnect(); |
| 168 | + }); |
| 169 | + |
| 170 | + // ── The channels the old message-only read could not see ────────────────── |
| 171 | + |
| 172 | + it('absorbs a Postgres index-build conflict that names the verdict only on `code` (SQLSTATE 23505)', async () => { |
| 173 | + arm(postgresIndexBuildConflict()); |
| 174 | + |
| 175 | + // Before #6543 this REJECTED: none of `unique constraint failed`, |
| 176 | + // `duplicate entry`, `duplicate key value` appears in Postgres' DDL |
| 177 | + // phrasing, so the branch fell through to `throw e` and the boot died on |
| 178 | + // exactly the dirty database it exists to survive. |
| 179 | + await expect(sync([NULL_SAFE_INDEX])).resolves.toBeUndefined(); |
| 180 | + |
| 181 | + // Absorbed the way the branch promises: the durability-degradation |
| 182 | + // channel, naming the constraint that is NOT enforced and the way out. |
| 183 | + expect(errors).toHaveLength(1); |
| 184 | + expect(errors[0]).toMatch(/cannot create NULL-safe unique index/); |
| 185 | + expect(errors[0]).toMatch(/uniq_product_organization_id_code/); |
| 186 | + expect(errors[0]).toMatch(/NOT enforced/); |
| 187 | + expect(errors[0]).toMatch(/#5030/); |
| 188 | + expect(errors[0]).toMatch(/ADR-0120 D4/); |
| 189 | + }); |
| 190 | + |
| 191 | + it('absorbs a MySQL conflict carried only on `errno` (1062)', async () => { |
| 192 | + arm(mysqlErrnoOnlyConflict()); |
| 193 | + |
| 194 | + await expect(sync([NULL_SAFE_INDEX])).resolves.toBeUndefined(); |
| 195 | + expect(errors).toHaveLength(1); |
| 196 | + expect(errors[0]).toMatch(/#5030/); |
| 197 | + }); |
| 198 | + |
| 199 | + it('reads the violation through a driver `cause` wrapper', async () => { |
| 200 | + const wrapped = new Error('index sync failed'); |
| 201 | + Object.assign(wrapped, { cause: postgresIndexBuildConflict() }); |
| 202 | + arm(wrapped); |
| 203 | + |
| 204 | + await expect(sync([NULL_SAFE_INDEX])).resolves.toBeUndefined(); |
| 205 | + expect(errors).toHaveLength(1); |
| 206 | + expect(errors[0]).toMatch(/#5030/); |
| 207 | + }); |
| 208 | + |
| 209 | + // ── Nothing the old regex caught may be narrowed ────────────────────────── |
| 210 | + |
| 211 | + it.each([ |
| 212 | + ['sqlite', 'UNIQUE constraint failed: product.organization_id, product.code'], |
| 213 | + ['mysql', "ER_DUP_ENTRY: Duplicate entry 'DUP' for key 'uniq_product_organization_id_code'"], |
| 214 | + ['postgres dml', 'duplicate key value violates unique constraint "uniq_product_organization_id_code"'], |
| 215 | + ])('still absorbs the %s message spelling the inline regex used to match', async (_dialect, message) => { |
| 216 | + arm(new Error(message)); |
| 217 | + |
| 218 | + await expect(sync([NULL_SAFE_INDEX])).resolves.toBeUndefined(); |
| 219 | + expect(errors).toHaveLength(1); |
| 220 | + expect(errors[0]).toMatch(/#5030/); |
| 221 | + }); |
| 222 | + |
| 223 | + // ── The site's own business logic, untouched by the migration ───────────── |
| 224 | + |
| 225 | + it('leaves the `nullSafe.size > 0` guard intact — a plain unique still fails the sync', async () => { |
| 226 | + arm(postgresIndexBuildConflict()); |
| 227 | + // The plain arm goes through knex's schema builder rather than the |
| 228 | + // overridden method, and `knex.schema` is a fresh builder on every access |
| 229 | + // — so the failure is injected by standing in for `knex` itself. |
| 230 | + (driver as any).getExistingIndexNames = async () => new Set<string>(); |
| 231 | + (driver as any).knex = { |
| 232 | + schema: { |
| 233 | + alterTable: () => Promise.reject(postgresIndexBuildConflict()), |
| 234 | + }, |
| 235 | + }; |
| 236 | + |
| 237 | + // A unique violation on a NON-NULL-safe index is not the #5030 case and |
| 238 | + // must still surface: absorbing it would silently ship an unenforced |
| 239 | + // constraint the drift pre-flight was never told about. |
| 240 | + const rejected: any = await sync([PLAIN_INDEX]).then( |
| 241 | + () => undefined, |
| 242 | + (e: unknown) => e, |
| 243 | + ); |
| 244 | + expect(rejected).toBeInstanceOf(Error); |
| 245 | + expect(rejected.code).toBe('23505'); |
| 246 | + expect(errors).toHaveLength(0); |
| 247 | + }); |
| 248 | + |
| 249 | + it('rethrows a failure that is not a unique violation, identity preserved', async () => { |
| 250 | + const original = unrelatedDdlFailure(); |
| 251 | + arm(original); |
| 252 | + |
| 253 | + const rejected: any = await sync([NULL_SAFE_INDEX]).then( |
| 254 | + () => undefined, |
| 255 | + (e: unknown) => e, |
| 256 | + ); |
| 257 | + expect(rejected).toBe(original); |
| 258 | + expect(rejected.code).toBe('42501'); |
| 259 | + expect(errors).toHaveLength(0); |
| 260 | + }); |
| 261 | + |
| 262 | + it('still treats an "already exists" race as benign, ahead of the conflict branch', async () => { |
| 263 | + const race = new Error('create unique index - index "uniq_product_organization_id_code" already exists'); |
| 264 | + Object.assign(race, { code: '42P07' }); |
| 265 | + arm(race); |
| 266 | + |
| 267 | + await expect(sync([NULL_SAFE_INDEX])).resolves.toBeUndefined(); |
| 268 | + // Benign: absorbed WITHOUT the durability-degradation log, because the |
| 269 | + // constraint IS enforced — a different outcome from the #5030 branch. |
| 270 | + expect(errors).toHaveLength(0); |
| 271 | + }); |
| 272 | +}); |
0 commit comments