From a8506dceac3fb92859a88dde36a894d30ab18e08 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 12:44:29 +0000 Subject: [PATCH 1/2] wip(types,rest): uniqueViolationColumn export + import-runner migration --- packages/rest/src/import-runner.ts | 37 +++++- packages/types/src/unique-violation.ts | 170 ++++++++++++++++++++++++- 2 files changed, 194 insertions(+), 13 deletions(-) diff --git a/packages/rest/src/import-runner.ts b/packages/rest/src/import-runner.ts index 5ab14a847a..5b5903dce8 100644 --- a/packages/rest/src/import-runner.ts +++ b/packages/rest/src/import-runner.ts @@ -6,6 +6,7 @@ import type { ExportFieldMeta } from './export-format.js'; import type { ValidationMessageTranslator } from '@objectstack/spec/system'; import type { ValidateDataIssue, ValidateDataRequest, ValidateDataResponse } from '@objectstack/spec/api'; import { bulkWrite, withTransientRetry, defaultIsTransientError, type BulkWriteRowResult } from '@objectstack/core'; +import { isUniqueViolationError, uniqueViolationColumn } from '@objectstack/types'; /** * import-runner — the shared row-processing core for bulk import. @@ -205,6 +206,14 @@ function bareColumn(raw: string): string { return dot >= 0 ? col.slice(dot + 1) : col; } +/** + * The sentence used when the row conflicts but no column is determinable + * (#6544). Deliberately the same wording `mapDataError` puts in the 409 + * `UNIQUE_VIOLATION` body, so the importer and the API say one thing about one + * condition rather than two. + */ +const UNNAMED_CONFLICT = 'A record with this value already exists.'; + /** * Turn a raw write error into a message safe to hand back to the importer. * @@ -214,18 +223,32 @@ function bareColumn(raw: string): string { * verbatim is both unreadable and an information disclosure of the schema * (framework#3566). This maps the common constraint failures to human wording * and, as a backstop, never lets a raw SQL statement escape to the client. + * + * The unique-violation verdict and the conflicting column both come from + * `@objectstack/types` (#6544). This site used to carry its own three-dialect + * regex chain — one of the four private vocabularies #6250 inventoried, which + * between them disagreed about MySQL. Two consequences of adopting the shared + * pair, both intended: + * + * - the verdict widens: a conflict recognised only by a channel the old chain + * did not read (Postgres' bare constraint name, for one) now gets conflict + * wording instead of falling through to the SQL backstop; and + * - the *naming* narrows: `uniqueViolationColumn` refuses to answer with an + * index name, so **MySQL rows no longer name a column** — they used to name + * the index (`for key 'idx_email_unique'`) as if it were one, pointing the + * user at a field that does not exist. See that function's doc comment. */ export function sanitizeRowError(raw: unknown): string { const msg = typeof raw === 'string' ? raw.trim() : ''; if (!msg) return 'Row failed'; - // UNIQUE — surface the offending column (it maps to a user-facing import - // column, so naming it is helpful, not a schema leak). - const unique = - /unique constraint failed:\s*([^\s,)]+)/i.exec(msg) ?? // sqlite - /duplicate entry .* for key '([^']+)'/i.exec(msg) ?? // mysql - /duplicate key value violates unique constraint.*?[Kk]ey \(([^)]+)\)/is.exec(msg); // postgres - if (unique) return `A record with this ${bareColumn(unique[1])} already exists.`; + // UNIQUE — surface the offending column when the dialect determinably named + // one (it maps to a user-facing import column, so naming it is helpful, not a + // schema leak); otherwise say so generically rather than guess. + if (isUniqueViolationError(msg)) { + const column = uniqueViolationColumn(msg); + return column ? `A record with this ${column} already exists.` : UNNAMED_CONFLICT; + } // NOT NULL — a required value is missing. const notNull = /not null constraint failed:\s*([^\s,)]+)/i.exec(msg); diff --git a/packages/types/src/unique-violation.ts b/packages/types/src/unique-violation.ts index 2092218b81..00abcfe516 100644 --- a/packages/types/src/unique-violation.ts +++ b/packages/types/src/unique-violation.ts @@ -55,13 +55,14 @@ * on it, so adopting the predicate never adds an edge. This module deliberately * imports nothing. * - * ## What this predicate does NOT do + * ## The second question, answered separately * - * It does not name the **conflicting column**. `sanitizeRowError` extracts one - * for the import path, and #5495 wants one to decide whether an autonumber - * collision is retryable — but a structured conflict-column export is a new - * contract surface, so it is deliberately not decided here (#6250's ruling). - * This answers the yes/no question only. + * `isUniqueViolationError` answers yes/no. **Which column** conflicted is a + * different question with a different failure mode, so it is a different export: + * {@link uniqueViolationColumn}, added by #6544 under the maintainer's + * 2026-08-08 ruling. Read its doc comment before touching either — the two are + * gated on each other and the column answer is deliberately narrower than the + * boolean. */ /** @@ -172,3 +173,160 @@ function matchesUniqueViolation(error: unknown, depth: number): boolean { return matchesUniqueViolation(err.cause, depth + 1); } + +/* ------------------------------------------------------------------------- * + * #6544 — which column conflicted + * ------------------------------------------------------------------------- */ + +/** + * SQLite names the offending **columns** directly, as `table.column` pairs: + * `UNIQUE constraint failed: sys_user.email`. Captured to end-of-line because + * knex prefixes the failing statement, so the useful part is always the tail. + */ +const SQLITE_TARGETS = /unique constraint failed:\s*([^\n]*)/i; + +/** + * Postgres names the offending **columns** only in its `DETAIL:` line — + * `Key (email)=(acme@example.com) already exists.` — which node-postgres puts + * on `error.detail` and knex flattens into the message. The trailing `=(` + * is required: it is what separates this form from the constraint-name form + * (`violates unique constraint "sys_user_email_key"`), which names an INDEX. + * + * An expression index (`Key (lower(email))=(…)`) cannot match, because the + * capture forbids `)` — which is the correct answer: `lower(email)` is not a + * column. + */ +const POSTGRES_DETAIL_TARGETS = /\bkey \(([^)]+)\)=\(/i; + +/** SQLite's other spelling, for a partial or expression index: `UNIQUE constraint failed: index 'x'`. */ +const SQLITE_INDEX_FORM = /^index\b/i; + +/** What a column name may look like once the table qualifier and quoting are stripped. */ +const PLAIN_IDENTIFIER = /^[A-Za-z_][A-Za-z0-9_$]*$/; + +/** Strip quoting and any `table.` qualifier from one constraint target. */ +function bareIdentifier(raw: string): string { + const stripped = raw.trim().replace(/[`"'[\]]/g, ''); + const dot = stripped.lastIndexOf('.'); + return dot >= 0 ? stripped.slice(dot + 1) : stripped; +} + +/** + * Reduce one dialect's list of constraint targets to THE conflicting column, + * or `undefined` when there is not exactly one that is determinably a column. + * + * A composite key resolves to `undefined` on purpose: there is no single + * offending column, and picking the first is the same class of wrong answer as + * returning an index name — it points a form at `tenant_id` when what the user + * typed twice was `email`. + */ +function soleColumn(targets: string): string | undefined { + const names = targets.split(',').map(bareIdentifier); + if (names.length !== 1) return undefined; + const [name] = names; + return PLAIN_IDENTIFIER.test(name) ? name : undefined; +} + +function columnFromText(text: string): string | undefined { + const sqlite = SQLITE_TARGETS.exec(text); + if (sqlite) { + const targets = sqlite[1].trim(); + // `index 'idx_email_unique'` is an index name, not a column. Refuse. + return SQLITE_INDEX_FORM.test(targets) ? undefined : soleColumn(targets); + } + + const postgres = POSTGRES_DETAIL_TARGETS.exec(text); + if (postgres) return soleColumn(postgres[1]); + + // MySQL deliberately has no limb here — see the doc comment on + // `uniqueViolationColumn`. `Duplicate entry 'x' for key 'i'` names `i`, + // which is an INDEX, and this function does not guess columns from indexes. + return undefined; +} + +function findUniqueViolationColumn(error: unknown, depth: number): string | undefined { + if (error === null || error === undefined || depth > MAX_CAUSE_DEPTH) return undefined; + + if (typeof error === 'string') return columnFromText(error); + if (typeof error !== 'object') return undefined; + + const err = error as { message?: unknown; detail?: unknown; cause?: unknown }; + + if (typeof err.message === 'string') { + const fromMessage = columnFromText(err.message); + if (fromMessage !== undefined) return fromMessage; + } + // node-postgres keeps the `DETAIL:` line off the message and on its own + // field, so for the driver we actually ship this is where the column is. + if (typeof err.detail === 'string') { + const fromDetail = columnFromText(err.detail); + if (fromDetail !== undefined) return fromDetail; + } + + return findUniqueViolationColumn(err.cause, depth + 1); +} + +/** + * Which column a unique-constraint violation was raised on — or `undefined` + * when the dialect did not determinably name one (#6544). + * + * ## The contract, and why it is this narrow + * + * **A value comes back only when the identifier the driver printed is + * determinably a COLUMN.** When a dialect names an *index* instead — MySQL's + * `Duplicate entry 'a@b.com' for key 'idx_email_unique'`, Postgres' + * `violates unique constraint "sys_user_email_key"`, SQLite's + * `UNIQUE constraint failed: index 'idx_lower_email'` — the answer is + * `undefined`, never the index name. + * + * That is the maintainer's 2026-08-08 ruling on #6544, and the reasoning is the + * caller's, not this module's: **an index name mistaken for a column is worse + * than no answer at all.** + * + * - `@objectstack/rest`'s import runner renders this into a form field — + * "A record with this `email` already exists." An index name there points + * the user at a field that does not exist on the object, so they cannot act + * on it; `undefined` degrades to generic copy, which is merely less helpful. + * - #5495's autonumber-retry branch asks a yes/no question of the answer — + * "is the conflicting column the autonumber field?" — and an index name + * produces a *wrong retry decision*, not a vaguer one. + * + * ⛔ **The accepted cost: MySQL deployments usually get no column.** MySQL's + * duplicate-entry message names the index and never the column, so there is + * nothing here to read. That is deliberate. Do not "improve" this by deriving a + * column from an index name (`idx_email_unique` → `email`, or MySQL 8's + * `for key 'sys_user.email'` → `email`): index names are free-form, a + * deployment's may match no column at all, and a plausible-looking wrong field + * is exactly the failure this export exists to avoid. If MySQL must name + * columns, the answer is a schema lookup of the index — a different, wider + * contract — not a guess in this function. + * + * A **composite** key is `undefined` for the same reason: `Key (tenant_id, + * email)=(…)` has no single offending column, and naming the first is the same + * class of wrong answer. + * + * ## What it reads + * + * Gated on {@link isUniqueViolationError}, so a NOT NULL or FOREIGN KEY failure + * can never reach the extraction — SQLite's `NOT NULL constraint failed: t.c` + * shares its shape with the positive and is refused at the gate, not by the + * patterns. Then `message`, then `detail` (node-postgres keeps its `DETAIL:` + * line there), then one step down the `cause` chain, bounded exactly as the + * predicate's walk is. A bare string is read as a message, so a caller holding + * only `err.message` can pass it straight in. + * + * @param error - the thrown value, of any shape. + * @returns the conflicting column, or `undefined` when none is determinable. + * + * @example + * ```ts + * const column = uniqueViolationColumn(error); + * return column + * ? `A record with this ${column} already exists.` + * : 'A record with this value already exists.'; + * ``` + */ +export function uniqueViolationColumn(error: unknown): string | undefined { + if (!isUniqueViolationError(error)) return undefined; + return findUniqueViolationColumn(error, 0); +} From 76119a6d1c2f5dd3e5a5de979b3d548d4cfe283a Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 12:59:36 +0000 Subject: [PATCH 2/2] test(types,rest): one dialect table driven through both faces; changeset --- .changeset/unique-violation-column.md | 55 +++ .../src/import-runner-error-sanitize.test.ts | 346 +++++++++++++++++- packages/types/src/unique-violation.test.ts | 119 +++++- 3 files changed, 499 insertions(+), 21 deletions(-) create mode 100644 .changeset/unique-violation-column.md diff --git a/.changeset/unique-violation-column.md b/.changeset/unique-violation-column.md new file mode 100644 index 0000000000..8a757999dc --- /dev/null +++ b/.changeset/unique-violation-column.md @@ -0,0 +1,55 @@ +--- +"@objectstack/types": minor +"@objectstack/rest": patch +--- + +fix(types,rest): one named answer for "which column conflicted" — an index name is never returned as one (#6544) + +#6250 retired four private "is this a unique violation?" vocabularies into +`isUniqueViolationError`. It left the harder half of the question behind: the +import runner's `sanitizeRowError` still carried its own three-dialect regex +chain, because it does **more** than answer yes/no — it names the offending +column so the importer can say *"A record with this `email` already exists."* +This lands that second answer as a shared export and migrates the last private +copy onto it. + +**New — `uniqueViolationColumn(error)` in `@objectstack/types`** (`string | +undefined`), sibling to `isUniqueViolationError` and gated on it, reading the +same channels one step down the same bounded `cause` chain, plus +node-postgres' `detail` field. + +**Its contract, per the maintainer's 2026-08-08 ruling: a value comes back only +when the identifier the driver printed is determinably a COLUMN.** When a +dialect names an *index* instead — MySQL's `Duplicate entry … for key +'idx_email_unique'`, Postgres' `violates unique constraint "sys_user_email_key"`, +SQLite's `UNIQUE constraint failed: index 'x'` — the answer is `undefined`, +never the index name. Callers render this into a form field, and an index name +mistaken for a column points the user at a field that does not exist, whereas +`undefined` degrades to generic copy. A **composite** key (`Key (tenant_id, +email)=(…)`) is `undefined` for the same reason: there is no single offending +column, and naming the first is the same class of wrong answer. + +**⚠️ User-visible change on MySQL imports.** MySQL's duplicate-entry message +names the index and never the column, so the importer no longer names a column +there: rows that used to read *"A record with this `idx_email_unique` already +exists."* — or, on MySQL 8's table-qualified `for key 'sys_user.email'`, a +plausible-looking *`email`* that was still an index name — now read **"A record +with this value already exists."** That is deliberate and is the accepted cost +of the ruling. The conflict is still recognised as a conflict; only the naming +narrowed. + +Three smaller import messages improve in the same move, all previously wrong +rather than merely vague: + +- SQLite's expression/partial-index form used to render as *"A record with this + **index** already exists."* +- Postgres' expression index used to render the truncated fragment *"A record + with this **lower(email** already exists."* +- A Postgres conflict with no `DETAIL:` line used to fall through to the SQL + backstop and echo the driver's own sentence — index name included — at the + importer. It now gets the same generic conflict copy, which is also the exact + wording `mapDataError` puts in the 409 `UNIQUE_VIOLATION` body, so the + importer and the API say one thing about one condition. + +Not changed: the NOT NULL branch, the raw-SQL backstop, and every non-conflict +message, which pass through exactly as before. diff --git a/packages/rest/src/import-runner-error-sanitize.test.ts b/packages/rest/src/import-runner-error-sanitize.test.ts index 3925b5835c..fe66f15872 100644 --- a/packages/rest/src/import-runner-error-sanitize.test.ts +++ b/packages/rest/src/import-runner-error-sanitize.test.ts @@ -4,39 +4,339 @@ * sanitizeRowError() — driver/query-builder errors embed the whole failing SQL * statement in `err.message`; it must never reach the importer verbatim * (framework#3566). Common constraint failures map to human wording. + * + * ## #6544 — one dialect table, driven through BOTH faces + * + * This site used to carry its own three-dialect regex chain: one of the four + * private vocabularies #6250 inventoried, which between them disagreed about + * MySQL. It now reads `@objectstack/types`' `isUniqueViolationError` / + * `uniqueViolationColumn` pair. + * + * {@link DIALECT_SAMPLES} is the single corpus and every sample runs through + * both faces in the same file: + * + * face 1 — `uniqueViolationColumn` (`@objectstack/types`), the column answer; + * face 2 — `sanitizeRowError` (this package), the sentence the importer shows. + * + * A dialect that regresses on either goes red here, and teaching one face a + * dialect without the other cannot go green. The table lives in this package + * for the same reason #6250's does: `@objectstack/types` cannot import + * `@objectstack/rest`, so this is the side that can see both faces at once. + * `packages/types/src/unique-violation.test.ts` is its complement — it pins the + * *shapes* (objects, `cause` depth, the `detail` channel) a table of flat driver + * messages cannot express, and deliberately does not restate the vocabulary. + * + * The samples are message strings rather than error objects because that is + * exactly what reaches this function: `toFailedResult` hands it `err.message`. */ import { describe, it, expect } from 'vitest'; +import { isUniqueViolationError, uniqueViolationColumn } from '@objectstack/types'; import { sanitizeRowError } from './import-runner'; -describe('sanitizeRowError', () => { - it('maps a sqlite UNIQUE violation to a friendly, SQL-free message', () => { - const raw = - "insert into `sys_user` (`email`, `name`, `phone_number`) values " + - "('a@b.com', 'zhoujunyi', '13800000000') - UNIQUE constraint failed: sys_user.phone_number"; - const out = sanitizeRowError(raw); - expect(out).toBe('A record with this phone_number already exists.'); - expect(out).not.toMatch(/insert into/i); +/** The wording used when the row conflicts but no column is determinable. */ +const UNNAMED_CONFLICT = 'A record with this value already exists.'; + +/** The offending user data MySQL and Postgres interpolate into their conflict text. */ +const OFFENDING_VALUE = 'acme@example.com'; + +interface DialectSample { + /** Which driver family emits this text. */ + readonly dialect: 'postgres' | 'mysql' | 'sqlite'; + /** Human label — also the test name. */ + readonly label: string; + /** The driver text exactly as it reaches `sanitizeRowError` (`err.message`). */ + readonly message: string; + /** Is this a unique violation at all? (the shared predicate's answer) */ + readonly conflict: boolean; + /** Face 1: the column `uniqueViolationColumn` must name, or `undefined`. */ + readonly column: string | undefined; + /** Face 2: the exact sentence the importer must show. */ + readonly sanitized: string; +} + +/** + * The shared table. + * + * Positives cover each dialect in both the "names a column" and the "names an + * index" spelling, because the ruling this implements is precisely about + * telling those apart. Negatives are the sibling constraint failures the same + * INSERT can raise — SQLite's are the sharp ones, since `NOT NULL constraint + * failed:` shares its shape with the unique positive. + */ +const DIALECT_SAMPLES: readonly DialectSample[] = [ + // ------------------------------------------------------------------ SQLite + // SQLite names `table.column` pairs — real columns, so it answers. + { + dialect: 'sqlite', + label: 'UNIQUE constraint failed — knex-prefixed, names a column', + message: + 'insert into `sys_user` (`email`, `name`, `phone_number`) values ' + + "('a@b.com', 'zhoujunyi', '13800000000') - UNIQUE constraint failed: sys_user.phone_number", + conflict: true, + column: 'phone_number', + sanitized: 'A record with this phone_number already exists.', + }, + { + dialect: 'sqlite', + label: 'UNIQUE constraint failed — bare driver message', + message: 'UNIQUE constraint failed: sys_user.email', + conflict: true, + column: 'email', + sanitized: 'A record with this email already exists.', + }, + // Composite: no single offending column, so naming the first would point the + // form at `tenant_id` when what was typed twice is `email`. + { + dialect: 'sqlite', + label: 'UNIQUE constraint failed — composite key names no single column', + message: 'UNIQUE constraint failed: sys_user.tenant_id, sys_user.email', + conflict: true, + column: undefined, + sanitized: UNNAMED_CONFLICT, + }, + // SQLite's other spelling, for a partial or expression index. An INDEX name. + { + dialect: 'sqlite', + label: 'UNIQUE constraint failed: index — an index name is not a column', + message: "UNIQUE constraint failed: index 'idx_lower_email'", + conflict: true, + column: undefined, + sanitized: UNNAMED_CONFLICT, + }, + // Negative — shares "constraint failed:" and the `table.column` shape with + // the positives above, and must not be swept into the conflict branch. + { + dialect: 'sqlite', + label: 'NOT NULL constraint failed — a required value, not a taken one', + message: 'insert into `sys_user` (...) values (...) - NOT NULL constraint failed: sys_user.name', + conflict: false, + column: undefined, + sanitized: 'name is required.', + }, + // Negative — the other "constraint failed" sibling. + { + dialect: 'sqlite', + label: 'FOREIGN KEY constraint failed — not a unique violation', + message: 'insert into `sys_order` (`customer_id`) values (?) - FOREIGN KEY constraint failed', + conflict: false, + column: undefined, + sanitized: 'FOREIGN KEY constraint failed', + }, + + // ------------------------------------------------------------------- MySQL + // ⚠️ The user-visible change this PR discloses. MySQL's duplicate-entry text + // names the INDEX and never the column — including MySQL 8's table-qualified + // `sys_user.email`, which LOOKS like `table.column` and is not one. The old + // private regex read it as a column; the ruling says `undefined`. + { + dialect: 'mysql', + label: 'ER_DUP_ENTRY — MySQL 8 `table.index`, which is NOT `table.column`', + message: + 'insert into `sys_user` (`email`) values (?) - ' + + `ER_DUP_ENTRY: Duplicate entry '${OFFENDING_VALUE}' for key 'sys_user.email'`, + conflict: true, + column: undefined, + sanitized: UNNAMED_CONFLICT, + }, + { + dialect: 'mysql', + label: 'ER_DUP_ENTRY — a named index resolves to no column', + message: `ER_DUP_ENTRY: Duplicate entry '${OFFENDING_VALUE}' for key 'idx_email_unique'`, + conflict: true, + column: undefined, + sanitized: UNNAMED_CONFLICT, + }, + // Negative — MySQL's not-null. + { + dialect: 'mysql', + label: "ER_BAD_NULL_ERROR — not-null is not a unique violation", + message: "ER_BAD_NULL_ERROR: Column 'email' cannot be null", + conflict: false, + column: undefined, + sanitized: "ER_BAD_NULL_ERROR: Column 'email' cannot be null", + }, + // Negative — near-miss wording: it carries "constraint" AND a `KEY (...)` + // parenthesised column list, and must still not read as unique. + { + dialect: 'mysql', + label: 'ER_NO_REFERENCED_ROW_2 — foreign key text carries a `KEY (col)` list', + message: + 'ER_NO_REFERENCED_ROW_2: Cannot add or update a child row: a foreign key constraint fails ' + + '(`app`.`sys_order`, CONSTRAINT `fk_customer` FOREIGN KEY (`customer_id`) REFERENCES `sys_user` (`id`))', + conflict: false, + column: undefined, + sanitized: + 'ER_NO_REFERENCED_ROW_2: Cannot add or update a child row: a foreign key constraint fails ' + + '(`app`.`sys_order`, CONSTRAINT `fk_customer` FOREIGN KEY (`customer_id`) REFERENCES `sys_user` (`id`))', + }, + + // -------------------------------------------------------------- PostgreSQL + // Only the `DETAIL:` line names columns; the constraint in the first sentence + // is an index name. + { + dialect: 'postgres', + label: 'duplicate key + DETAIL — the DETAIL line names the column', + message: + 'insert into "sys_user" ... - duplicate key value violates unique constraint "sys_user_email_key" ' + + `Detail: Key (email)=(${OFFENDING_VALUE}) already exists.`, + conflict: true, + column: 'email', + sanitized: 'A record with this email already exists.', + }, + { + dialect: 'postgres', + label: 'duplicate key + DETAIL — a quoted mixed-case column survives', + message: + 'duplicate key value violates unique constraint "sys_user_email_key" ' + + `Detail: Key ("emailAddress")=(${OFFENDING_VALUE}) already exists.`, + conflict: true, + column: 'emailAddress', + sanitized: 'A record with this emailAddress already exists.', + }, + // No DETAIL line: the only identifier present is the constraint (index) name. + // Before #6544 this fell through to the SQL backstop and echoed the driver's + // sentence — index name and all — at the importer. + { + dialect: 'postgres', + label: 'duplicate key without DETAIL — only an index name is present', + message: + 'insert into "sys_user" ("email") values ($1) - ' + + 'duplicate key value violates unique constraint "sys_user_email_key"', + conflict: true, + column: undefined, + sanitized: UNNAMED_CONFLICT, + }, + { + dialect: 'postgres', + label: 'duplicate key + DETAIL — composite key names no single column', + message: + 'duplicate key value violates unique constraint "sys_user_tenant_email_key" ' + + `Detail: Key (tenant_id, email)=(1, ${OFFENDING_VALUE}) already exists.`, + conflict: true, + column: undefined, + sanitized: UNNAMED_CONFLICT, + }, + // An expression index: `lower(email)` is not a column, and the old regex used + // to hand back the fragment `lower(email` as if it were one. + { + dialect: 'postgres', + label: 'duplicate key + DETAIL — an expression index is not a column', + message: + 'duplicate key value violates unique constraint "idx_lower_email" ' + + `Detail: Key (lower(email))=(${OFFENDING_VALUE}) already exists.`, + conflict: true, + column: undefined, + sanitized: UNNAMED_CONFLICT, + }, + // Negative — Postgres not-null (23502). + { + dialect: 'postgres', + label: '23502 not-null — not a unique violation', + message: 'null value in column "email" of relation "sys_user" violates not-null constraint', + conflict: false, + column: undefined, + sanitized: 'null value in column "email" of relation "sys_user" violates not-null constraint', + }, + // Negative — Postgres foreign key (23503). Its own sentence opens with the + // word `insert`, so the SQL backstop claims it and hands back generic copy. + // Unchanged by #6544, and pinned because it is the one negative whose + // sentence is not its own text. + { + dialect: 'postgres', + label: '23503 foreign key — not a unique violation (claimed by the SQL backstop)', + message: + 'insert or update on table "sys_order" violates foreign key constraint "sys_order_customer_fkey"', + conflict: false, + column: undefined, + sanitized: 'The database rejected this row (a value may be invalid or already in use).', + }, +]; + +const named = (s: DialectSample) => [`${s.dialect}: ${s.label}`, s] as const; + +describe('#6544 face 1 — uniqueViolationColumn (@objectstack/types)', () => { + it.each(DIALECT_SAMPLES.map(named))('%s', (_name, sample) => { + expect(uniqueViolationColumn(sample.message)).toBe(sample.column); }); - it('maps a mysql duplicate entry', () => { - const raw = - "insert into `sys_user` ... - ER_DUP_ENTRY: Duplicate entry 'a@b.com' for key 'sys_user.email'"; - expect(sanitizeRowError(raw)).toBe('A record with this email already exists.'); + it.each(DIALECT_SAMPLES.map(named))('is a conflict? %s', (_name, sample) => { + expect(isUniqueViolationError(sample.message)).toBe(sample.conflict); }); - it('maps a postgres unique violation', () => { - const raw = - 'insert into "sys_user" ... - duplicate key value violates unique constraint "sys_user_email_key" ' + - 'Detail: Key (email)=(a@b.com) already exists.'; - expect(sanitizeRowError(raw)).toBe('A record with this email already exists.'); + it('never answers with an index name, on any dialect', () => { + const answers = DIALECT_SAMPLES.map((s) => uniqueViolationColumn(s.message)).filter( + (c): c is string => c !== undefined, + ); + // Every sample naming an index spells it `idx_*`, `*_key` or `sqlite_autoindex_*`. + for (const answer of answers) { + expect(answer).not.toMatch(/^idx_|_key$|^sqlite_autoindex/); + } + // …and the table really does exercise the refusal, on all three dialects. + const refused = DIALECT_SAMPLES.filter((s) => s.conflict && s.column === undefined); + expect(new Set(refused.map((s) => s.dialect))).toEqual( + new Set(['mysql', 'postgres', 'sqlite']), + ); }); +}); - it('maps a NOT NULL violation', () => { - const raw = 'insert into `sys_user` (...) values (...) - NOT NULL constraint failed: sys_user.name'; - expect(sanitizeRowError(raw)).toBe('name is required.'); +describe('#6544 face 2 — sanitizeRowError (the sentence the importer shows)', () => { + it.each(DIALECT_SAMPLES.map(named))('%s', (_name, sample) => { + expect(sanitizeRowError(sample.message)).toBe(sample.sanitized); }); + it('never leaks the failing SQL statement, on any sample', () => { + for (const sample of DIALECT_SAMPLES) { + expect(sanitizeRowError(sample.message)).not.toMatch(/insert into/i); + } + }); + + it('a conflict with no determinable column never echoes the driver identifier', () => { + const unnamed = DIALECT_SAMPLES.filter((s) => s.conflict && s.column === undefined); + expect(unnamed.length).toBeGreaterThan(0); + for (const sample of unnamed) { + const out = sanitizeRowError(sample.message); + expect(out).toBe(UNNAMED_CONFLICT); + expect(out).not.toContain('idx_'); + expect(out).not.toContain('_key'); + expect(out).not.toContain(OFFENDING_VALUE); + } + }); +}); + +/** + * #6544's disclosed behaviour change, pinned on its own so it cannot be undone + * by accident or "fixed" by someone reading it as a regression. + * + * MySQL's `Duplicate entry … for key ''` names an INDEX. The retired regex + * chain rendered that identifier as if it were a column, so a deployment whose + * index is `idx_email_unique` told the user "A record with this + * idx_email_unique already exists." and one on MySQL 8 (`for key + * 'sys_user.email'`) got a plausible-looking `email` that is still an index + * name that merely happens to match. Under the maintainer's ruling both are + * `undefined`: pointing a form at a field that does not exist is worse than + * generic copy. The cost — MySQL importers usually no longer name a column — + * is accepted, not accidental. + */ +describe('#6544 — MySQL stops naming a column (deliberate, user-visible)', () => { + it.each([ + ["a named index", 'idx_email_unique'], + ["MySQL 8's table-qualified index", 'sys_user.email'], + ["an index that happens to match no column", 'uniq_2'], + ])('%s yields the generic sentence, not the identifier', (_label, key) => { + const message = `ER_DUP_ENTRY: Duplicate entry '${OFFENDING_VALUE}' for key '${key}'`; + + // It is still recognised as a conflict — only the NAMING narrowed. + expect(isUniqueViolationError(message)).toBe(true); + expect(uniqueViolationColumn(message)).toBeUndefined(); + + const out = sanitizeRowError(message); + expect(out).toBe(UNNAMED_CONFLICT); + expect(out).not.toContain(key.split('.').pop()); + }); +}); + +describe('sanitizeRowError — the non-dialect behaviour', () => { it('never leaks a raw SQL statement even for an unrecognized reason', () => { const raw = 'insert into `sys_user` (`email`) values (?) - some cryptic driver failure'; const out = sanitizeRowError(raw); @@ -58,6 +358,12 @@ describe('sanitizeRowError', () => { ); }); + it('a business rule from a hook is not swept into the conflict branch', () => { + expect(sanitizeRowError('删除被阻断:该客户下仍有未结订单')).toBe( + '删除被阻断:该客户下仍有未结订单', + ); + }); + it('handles empty / non-string input', () => { expect(sanitizeRowError('')).toBe('Row failed'); expect(sanitizeRowError(undefined)).toBe('Row failed'); diff --git a/packages/types/src/unique-violation.test.ts b/packages/types/src/unique-violation.test.ts index 82f83cfd75..1523f52a3f 100644 --- a/packages/types/src/unique-violation.test.ts +++ b/packages/types/src/unique-violation.test.ts @@ -20,7 +20,7 @@ */ import { describe, it, expect } from 'vitest'; -import { isUniqueViolationError } from './unique-violation.js'; +import { isUniqueViolationError, uniqueViolationColumn } from './unique-violation.js'; describe('isUniqueViolationError — input shapes', () => { it('accepts a bare string, for callers that already unwrapped `err.message`', () => { @@ -92,3 +92,120 @@ describe('isUniqueViolationError — an unrecognised error is never a conflict', expect(isUniqueViolationError(new Error(message))).toBe(false); }); }); + +/* ------------------------------------------------------------------------- * + * #6544 — uniqueViolationColumn + * + * Same division of labour as above: the DIALECT VOCABULARY (which driver text + * names a column and which names an index, per dialect, with its negatives) + * lives in exactly one place — `@objectstack/rest`'s + * `import-runner-error-sanitize.test.ts`, where it is driven through this + * export AND through `sanitizeRowError` in the same run. Restating it here + * would rebuild the fork these exports exist to retire. + * + * What this file covers is what a table of flat driver messages cannot: the + * SHAPES a caller hands in, the channels an error object carries, and the + * boundaries of the search. + * ------------------------------------------------------------------------- */ + +describe('uniqueViolationColumn — input shapes', () => { + it('accepts a bare string, for callers that already unwrapped `err.message`', () => { + expect(uniqueViolationColumn('UNIQUE constraint failed: sys_user.email')).toBe('email'); + }); + + it('reads an Error object on the message channel', () => { + expect(uniqueViolationColumn(new Error('UNIQUE constraint failed: sys_user.email'))).toBe( + 'email', + ); + }); + + it.each([ + ['undefined', undefined], + ['null', null], + ['a number', 42], + ['a boolean', false], + ['an empty object', {}], + ['an error with no message', new Error()], + ])('names no column: %s', (_label, value) => { + expect(uniqueViolationColumn(value)).toBeUndefined(); + }); +}); + +describe('uniqueViolationColumn — the channels an error object carries', () => { + /** + * node-postgres keeps the `DETAIL:` line off `message` and on its own + * `detail` field, so for the Postgres driver we actually ship, the column + * is in neither the message nor any code — reading `detail` is what makes + * the export answer at all there. + */ + it("reads node-postgres' `detail` field when the message names only the index", () => { + const err = Object.assign( + new Error('duplicate key value violates unique constraint "sys_user_email_key"'), + { code: '23505', detail: 'Key (email)=(acme@example.com) already exists.' }, + ); + expect(uniqueViolationColumn(err)).toBe('email'); + }); + + it('prefers the message when both channels name a column', () => { + const err = Object.assign(new Error('UNIQUE constraint failed: sys_user.email'), { + detail: 'Key (other_column)=(x) already exists.', + }); + expect(uniqueViolationColumn(err)).toBe('email'); + }); + + it('a `detail` that is not a string is ignored rather than thrown on', () => { + const err = Object.assign(new Error('duplicate key value violates unique constraint "i"'), { + code: '23505', + detail: { key: 'email' }, + }); + expect(uniqueViolationColumn(err)).toBeUndefined(); + }); +}); + +describe('uniqueViolationColumn — the `cause` chain', () => { + const nest = (depth: number): unknown => { + let err: unknown = new Error('UNIQUE constraint failed: sys_user.email'); + for (let i = 0; i < depth; i += 1) err = Object.assign(new Error('Write failed'), { cause: err }); + return err; + }; + + it.each([1, 2, 3, 4])('finds the column wrapped %i level(s) deep', (depth) => { + expect(uniqueViolationColumn(nest(depth))).toBe('email'); + }); + + it('stops rather than walking an unbounded chain', () => { + expect(uniqueViolationColumn(nest(5))).toBeUndefined(); + }); + + it('a self-referential `cause` terminates instead of recursing forever', () => { + const err: { message: string; cause?: unknown } = { message: 'Write failed' }; + err.cause = err; + expect(uniqueViolationColumn(err)).toBeUndefined(); + }); +}); + +describe('uniqueViolationColumn — gated on the predicate', () => { + /** + * The gate is what keeps SQLite's sibling failures out: `NOT NULL + * constraint failed: sys_user.email` has the same `table.column` shape as + * the unique positive, and is refused because it is not a conflict — not + * because a pattern happened to miss it. + */ + it.each([ + ['NOT NULL constraint failed: sys_user.email'], + ['CHECK constraint failed: sys_user_age_check'], + ['FOREIGN KEY constraint failed'], + ['null value in column "email" of relation "sys_user" violates not-null constraint'], + ])('names no column for a non-conflict: %s', (message) => { + expect(isUniqueViolationError(message)).toBe(false); + expect(uniqueViolationColumn(message)).toBeUndefined(); + }); + + it('a conflict the predicate recognises may still name no column', () => { + // The two answers are independent: yes/no is wider than which-column, + // deliberately (#6544's ruling). A caller must handle `true` + `undefined`. + const mysql = "ER_DUP_ENTRY: Duplicate entry 'a@b.com' for key 'idx_email_unique'"; + expect(isUniqueViolationError(mysql)).toBe(true); + expect(uniqueViolationColumn(mysql)).toBeUndefined(); + }); +});