diff --git a/.changeset/24768-not-null-refusal-every-door.md b/.changeset/24768-not-null-refusal-every-door.md new file mode 100644 index 00000000000..4d78d94f689 --- /dev/null +++ b/.changeset/24768-not-null-refusal-every-door.md @@ -0,0 +1,26 @@ +--- +'@objectstack/objectql': patch +'@objectstack/types': patch +'@objectstack/rest': patch +--- + +fix(objectql,types,rest)!: a database NOT NULL refusal on a declared field is the record validator's `ValidationError` at every door, named by the field's label + +A field declared `storage: { notNull: true }` without `required` is ADR-0113's engine-populated column: the record validator judges `required` only, so a write with no value for it reaches the driver and the database refuses it. The engine let the driver's error out, and each HTTP door classified it from the driver's text its own way. For the same refused write, the runtime dispatcher's `/data` POST and PATCH answered `500 INTERNAL_ERROR`, the actions door answered `400 VALIDATION_ERROR` with the driver's text and `declaredCode: 'SQLITE_CONSTRAINT_NOTNULL'`, and `RestServer` answered `400 VALIDATION_FAILED` relabelled by a second object read on its own `POST` / `PATCH` only. + +Clause-②: yes (narrowing: `@objectstack/types` adds the export `notNullViolationSite` and retires `notNullRefusalSite` and `relabelNotNullRefusal`; the HTTP doors' answer for this refusal moves to the record validator's envelope) +Metadata-protocol: none + + + +**BREAKING** (an export retirement and a wire-answer change), shipped as `patch`: no metadata protocol kind moves. + +- **`@objectstack/objectql`.** The write doors' driver exits (single-row and batch insert, the autonumber retry, by-id and predicate update) translate a driver NOT NULL refusal on a field the object's field map holds into the record validator's own `ValidationError` (`code: 'VALIDATION_FAILED'`), built by the validator's own builder with the validator's message context: one field error, `code: 'required'`, the field named by its label (translation bundle, then declared `label`, then API name) in the caller's locale, with the sentence the validator gives the same verb (`required` on insert, `required_cleared` on update). The driver's error rides `cause`, and the doors' log line still carries the database's own diagnosis. Left as the driver threw it: a column no field declares, a refusal the driver attributes to another table, and a NOT NULL code whose column the driver did not name. `required` and `storage.notNull` stay apart (ADR-0113 D1 / D2): no pre-write check reads `storage.notNull`. +- **Every door answers it alike**, because every door already answers that class alike: the dispatcher `/data` POST and PATCH now answer `400 VALIDATION_FAILED` (was `500 INTERNAL_ERROR`), the actions door `400 VALIDATION_FAILED` with the field envelope (was `400 VALIDATION_ERROR` with the driver's text), and `RestServer`'s `POST` / `PATCH` and its `/data/:object/batch` rows answer the validator's envelope and sentence. Import rows, the public form submit and MCP write through the same engine doors and answer the same class through the same arms; they were not driven separately. On `RestServer` the body is now exactly the validator's `required` shape: `fields[0]` carries `label`, there is no `hint`, and a `PATCH` writing null reads the clear-out sentence. +- **`@objectstack/types`.** New export `notNullViolationSite(error)`: the column (and table, where the dialect names it) of a driver's NOT NULL refusal, read from the error's codes (better-sqlite3 `SQLITE_CONSTRAINT_NOTNULL`; libsql `extendedCode` / `rawCode: 1299`; node-postgres `23502` with its `column` / `table` fields; mysql2 `ER_BAD_NULL_ERROR` / `errno: 1048`), then its text (all sql.js gives), then one step down its `cause` chain. An error whose code names another refusal is not read for NOT NULL text. `mapDataError`'s NOT NULL branch is unchanged, and answers the column no field declares. +- **`@objectstack/rest`.** The `/data` create and update doors' NOT NULL relabel (`not-null-refusal-label.ts`) retires: every path it served is an engine write. + +Migration, for a TypeScript caller of the retired exports: + +- FROM `notNullRefusalSite(answer)` TO `notNullViolationSite(error)`: ask the driver's error, not `mapDataError`'s answer. +- FROM `relabelNotNullRefusal(answer, { message, declaresNotNull })` TO nothing (no replacement): write through the engine, which answers a declared field's NOT NULL refusal as a labelled `ValidationError` itself. diff --git a/packages/objectql/src/driver-fault-boundary-redaction.test.ts b/packages/objectql/src/driver-fault-boundary-redaction.test.ts index 7dd353a6bc7..0d94b4ee035 100644 --- a/packages/objectql/src/driver-fault-boundary-redaction.test.ts +++ b/packages/objectql/src/driver-fault-boundary-redaction.test.ts @@ -45,6 +45,7 @@ import { } from '@objectstack/types'; import { ObjectQL } from './engine.js'; import { DuplicateRecordError } from './duplicate-record-error.js'; +import { NotNullRefusalError } from './not-null-refusal-error.js'; import { SummaryRecomputeError } from './summary-errors.js'; import { redactBoundStatement, redactPropagatedDriverFault } from '@objectstack/types'; @@ -489,9 +490,20 @@ const DOORS: ReadonlyArray Promise { for (const shape of DOOR_SHAPES) { for (const [door, failing, run] of DOORS) { + if (NOT_NULL_SHAPES.includes(shape) && NOT_NULL_DOORS.has(door)) continue; it(`${door} — ${shape.id}`, async () => { const { engine, state } = await bootEngine(shape.make, failing); const error = await caught(run(engine)); @@ -527,6 +539,23 @@ describe('[#21274] every engine door that reaches a driver exits through the cut } } + for (const shape of NOT_NULL_SHAPES) { + for (const [door, failing, run] of DOORS.filter(([name]) => NOT_NULL_DOORS.has(name))) { + it(`${door}: a NOT NULL refusal on a declared field leaves as the validator's ValidationError, and a cut cause — ${shape.id}`, async () => { + const { engine, state } = await bootEngine(shape.make, failing); + const error = (await caught(run(engine))) as NotNullRefusalError; + expectNoSentinel(error); + expect(error).toBeInstanceOf(NotNullRefusalError); + expect(error.code).toBe('VALIDATION_FAILED'); + expect(error.fields.map((f) => [f.field, f.code])).toEqual([['title', 'required']]); + const cause = error.cause as Error & { code?: unknown }; + expect(Object.getPrototypeOf(cause)).toBe(Object.getPrototypeOf(state.lastRaw)); + expect(cause.code).toBe((state.lastRaw as Error & { code?: unknown }).code); + expect(cause.message).toContain(shape.diagnostic); + }); + } + } + it('a middleware that logs what it caught sees the cut error too', async () => { const { engine } = await bootEngine(SHAPES[1].make, 'write'); const seen: unknown[] = []; diff --git a/packages/objectql/src/engine-insert-duplicate-record.test.ts b/packages/objectql/src/engine-insert-duplicate-record.test.ts index 85fdd1f0b61..9325cd63a9e 100644 --- a/packages/objectql/src/engine-insert-duplicate-record.test.ts +++ b/packages/objectql/src/engine-insert-duplicate-record.test.ts @@ -30,7 +30,8 @@ * via the per-row fallback loop, `insertMany`'s partial mode, and the * last-chance create the autonumber resync issues when the field vanished * mid-flight; and - * - **nothing else moves**: a NOT NULL violation, a deadlock, a missing table + * - **nothing else moves**: a NOT NULL violation on a column no field declares + * ([#24768]: on a declared field it is the validator's `ValidationError`), a deadlock, a missing table * and an unreachable store leave the door as the very object the driver * threw — asserted on IDENTITY, not on a message match, so a future * "helpful" re-wrap cannot pass this file. @@ -125,8 +126,14 @@ const memoryDuplicate = () => /* -------- the negative side: failures that must NOT change shape --------- */ +/** + * [#24768] On a column NO field of `doc` declares: a NOT NULL refusal on a + * declared field is the engine's `ValidationError` now + * (`engine-not-null-refusal.test.ts`), and this one is what still leaves as the + * driver threw it. + */ const notNullViolation = () => - Object.assign(new Error('NOT NULL constraint failed: doc.title'), { + Object.assign(new Error('NOT NULL constraint failed: doc.legacy_col'), { code: 'SQLITE_CONSTRAINT_NOTNULL', }); @@ -408,7 +415,7 @@ describe('engine.insert — a driver unique violation is a DUPLICATE_RECORD enve describe('nothing that is not a unique violation changes shape', () => { const controls: Array<[string, () => unknown]> = [ - ['a NOT NULL violation', notNullViolation], + ['a NOT NULL violation on an undeclared column', notNullViolation], ['a missing table', missingTable], ['a deadlock', deadlock], ['an unreachable store', unreachableStore], diff --git a/packages/objectql/src/engine-not-null-refusal.test.ts b/packages/objectql/src/engine-not-null-refusal.test.ts new file mode 100644 index 00000000000..c8cbffced87 --- /dev/null +++ b/packages/objectql/src/engine-not-null-refusal.test.ts @@ -0,0 +1,309 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#24768] A driver's NOT NULL refusal on a field the object declares leaves the + * write doors as the record validator's own `ValidationError`, named by the + * field's label in the caller's locale — on every driver exit and every dialect. + * + * `unit_price` (label "Sales Price") is ADR-0113's engine-populated column: + * `storage: { notNull: true }`, no `required`. The validator judges `required` + * only, so a write with no value for it reaches the driver, and the DATABASE + * refuses it. Before this change the door let the driver's error out, and each + * HTTP door classified it from the text its own way: the runtime dispatcher's + * `/data` answered `500 INTERNAL_ERROR`, the actions door `400 VALIDATION_ERROR` + * with the driver's text, `RestServer` `400` with the column key relabelled by a + * second object read. The doors, on a real driver, are pinned in + * `packages/runtime/src/dispatcher-not-null-refusal-24768.test.ts` and + * `packages/rest/src/data-not-null-field-label-24715.test.ts`. + * + * Refusals assert `code` and the field envelope, never `toThrow()` alone. The + * sentence is asked of `renderValidationMessage`, the validator's renderer, and + * compared with the validator's own refusal on the same request where the + * locale is what is under test. + * + * The driver fixtures are the shapes `notNullViolationSite`'s codes table + * records per client. + */ + +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { redactPropagatedDriverFault } from '@objectstack/types'; +import { renderValidationMessage } from '@objectstack/spec/system'; +import { ObjectQL } from './engine'; +import { ValidationError, VALIDATION_FAILED_CODE } from './validation/record-validator'; +import { DuplicateRecordError } from './duplicate-record-error'; +import { NotNullRefusalError } from './not-null-refusal-error'; +import { SchemaRegistry } from './registry'; +import type { IDataDriver } from '@objectstack/spec/contracts'; + +vi.mock('./registry', async () => { + const { createRegistryModuleMock } = await import('./registry-module-mock.js'); + return createRegistryModuleMock(); +}); + +/** The double's `getObject`, typed (see `engine-insert-duplicate-record.test.ts`). */ +const registryDouble = SchemaRegistry as unknown as { getObject: ReturnType }; + +type Row = Record; + +const OBJECT = 'line_item'; + +const SCHEMA = { + name: OBJECT, + label: 'Line item', + fields: { + name: { type: 'text', label: 'Name' }, + // ADR-0113's engine-populated column: NOT NULL on purpose, not `required`. + unit_price: { type: 'number', label: 'Sales Price', storage: { notNull: true } }, + // The validator's own `required`, for the parity cases. + list_price: { type: 'number', label: 'List Price', required: true }, + }, +}; + +/* -------------------------------------------------------------------------- + * Driver fixtures — the dialect shapes, each naming `line_item.unit_price`. + * ----------------------------------------------------------------------- */ + +const sqlite = (table = OBJECT, column = 'unit_price') => () => + Object.assign( + new Error(`insert into [statement and bound values redacted] - NOT NULL constraint failed: ${table}.${column}`), + { code: 'SQLITE_CONSTRAINT_NOTNULL' }, + ); +const libsql = () => + Object.assign(new Error(`SQLITE_CONSTRAINT: NOT NULL constraint failed: ${OBJECT}.unit_price`), { + code: 'SQLITE_CONSTRAINT', extendedCode: 'SQLITE_CONSTRAINT_NOTNULL', rawCode: 1299, + }); +const postgres = () => + Object.assign(new Error(`null value in column "unit_price" of relation "${OBJECT}" violates not-null constraint`), { + code: '23502', column: 'unit_price', table: OBJECT, + }); +const mysql = () => + Object.assign(new Error("Column 'unit_price' cannot be null"), { code: 'ER_BAD_NULL_ERROR', errno: 1048 }); +const sqljs = () => new Error(`NOT NULL constraint failed: ${OBJECT}.unit_price`); + +/* -------------------------------------------------------------------------- + * Rig + * ----------------------------------------------------------------------- */ + +const STORED: Row[] = [ + { id: 'r1', name: 'a', unit_price: 5, list_price: 7 }, + { id: 'r2', name: 'b', unit_price: 6, list_price: 8 }, +]; + +interface DriverOpts { + /** What every write method rejects with. */ + refuse: () => unknown; + /** Omit `bulkCreate` so a batch takes the engine's per-row fallback loop. */ + noBulkCreate?: boolean; +} + +function makeDriver(opts: DriverOpts) { + const driver: any = { + name: 'fake', + version: '0.0.0', + supports: {}, + connect: vi.fn().mockResolvedValue(undefined), + disconnect: vi.fn().mockResolvedValue(undefined), + checkHealth: vi.fn().mockResolvedValue(true), + execute: vi.fn(), + find: vi.fn(async () => []), + // The by-id update reads the prior row before it writes (#7867's not-found gate). + findOne: vi.fn(async (_obj: string, ast: { where?: { id?: unknown } }) => + STORED.find((row) => row.id === ast?.where?.id) ?? null, + ), + create: vi.fn(async () => { throw opts.refuse(); }), + update: vi.fn(async () => { throw opts.refuse(); }), + updateMany: vi.fn(async () => { throw opts.refuse(); }), + delete: vi.fn(), + count: vi.fn(), + }; + if (!opts.noBulkCreate) driver.bulkCreate = vi.fn(async () => { throw opts.refuse(); }); + return driver as IDataDriver & { create: any; bulkCreate?: any; update: any; updateMany: any }; +} + +/** A logger that records the doors' `… operation failed` lines (`warn`, `meta.error = { message, stack }`). */ +function recordingLogger() { + const lines: Array<{ msg: string; err: any }> = []; + const logger = { + info: vi.fn(), debug: vi.fn(), trace: vi.fn(), error: vi.fn(), + warn: vi.fn((msg: string, meta?: { error?: unknown }) => { lines.push({ msg, err: meta?.error }); }), + }; + return { logger, lines }; +} + +async function makeRig(opts: DriverOpts, schema: unknown = SCHEMA) { + registryDouble.getObject.mockReturnValue(schema); + const driver = makeDriver(opts); + const { logger, lines } = recordingLogger(); + const engine = new ObjectQL({ logger } as any); + engine.registerDriver(driver, true); + await engine.init(); + return { engine, driver, lines }; +} + +/** The rejection, as the value the caller actually receives. */ +async function refusalOf(run: () => Promise): Promise { + return run().then( + () => { throw new Error('expected the write to be refused'); }, + (e) => e as any, + ); +} + +/** The catalog sentence the validator renders for `messageKey`. */ +const sentence = (messageKey: 'required' | 'required_cleared', label: string, field = 'unit_price', locale?: string) => + renderValidationMessage({ messageKey, label, field, params: {} }, { locale }); + +/** The validator's envelope, for the one field. */ +function expectRefusalFor(failure: any, messageKey: 'required' | 'required_cleared', label = 'Sales Price') { + expect(failure).toBeInstanceOf(ValidationError); + expect(failure.code).toBe(VALIDATION_FAILED_CODE); + expect(failure.name).toBe('ValidationError'); + const message = sentence(messageKey, label); + expect(failure.fields).toEqual([{ field: 'unit_price', code: 'required', message, label }]); + expect(failure.message).toBe(message); +} + +const insertOne = (engine: ObjectQL, opts?: unknown) => + engine.insert(OBJECT, { name: 'n', list_price: 1 }, opts as any); +const insertBatch = (engine: ObjectQL) => + engine.insert(OBJECT, [{ name: 'n', list_price: 1 }, { name: 'm', list_price: 2 }]); +const updateById = (engine: ObjectQL, opts?: unknown) => + engine.update(OBJECT, { id: 'r2', unit_price: null }, opts as any); +const updateByPredicate = (engine: ObjectQL) => + engine.update(OBJECT, { unit_price: null }, { where: { name: 'b' }, multi: true } as any); + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe('[#24768] every driver exit answers with the validator\'s `ValidationError`', () => { + it('insert, one row: the label, the `required` sentence, the driver\'s error on `cause`', async () => { + const raw = sqlite()(); + const { engine } = await makeRig({ refuse: () => raw }); + + const failure = await refusalOf(() => insertOne(engine)); + + expectRefusalFor(failure, 'required'); + expect(failure).toBeInstanceOf(NotNullRefusalError); + expect(failure.cause).toBeInstanceOf(Error); + expect(failure.cause.code).toBe('SQLITE_CONSTRAINT_NOTNULL'); + expect(failure.cause.message).toBe((redactPropagatedDriverFault(raw) as Error).message); + // The driver's text does not become the message a door ships. + expect(failure.message).not.toMatch(/constraint failed|unit_price/); + }); + + it('insert, a batch through `bulkCreate`', async () => { + const { engine, driver } = await makeRig({ refuse: sqlite() }); + expectRefusalFor(await refusalOf(() => insertBatch(engine)), 'required'); + expect(driver.bulkCreate).toHaveBeenCalledTimes(1); + }); + + it('insert, a batch through the per-row fallback loop', async () => { + const { engine, driver } = await makeRig({ refuse: sqlite(), noBulkCreate: true }); + expectRefusalFor(await refusalOf(() => insertBatch(engine)), 'required'); + expect(driver.create).toHaveBeenCalled(); + }); + + it('update by id: the sentence the validator gives a clear-out (`required_cleared`)', async () => { + const { engine, driver } = await makeRig({ refuse: sqlite() }); + expectRefusalFor(await refusalOf(() => updateById(engine)), 'required_cleared'); + expect(driver.update).toHaveBeenCalledTimes(1); + }); + + it('update by predicate: the same', async () => { + const { engine, driver } = await makeRig({ refuse: sqlite() }); + expectRefusalFor(await refusalOf(() => updateByPredicate(engine)), 'required_cleared'); + expect(driver.updateMany).toHaveBeenCalledTimes(1); + }); + + it('the door\'s log line carries the DATABASE\'s diagnosis, not the envelope\'s sentence', async () => { + const { engine, lines } = await makeRig({ refuse: sqlite() }); + await refusalOf(() => insertOne(engine)); + await refusalOf(() => updateById(engine)); + + const insertLine = lines.find((l) => l.msg === 'Insert operation failed'); + const updateLine = lines.find((l) => l.msg === 'Update operation failed'); + expect(String(insertLine?.err?.message)).toMatch(/NOT NULL constraint failed: line_item\.unit_price/); + expect(String(updateLine?.err?.message)).toMatch(/NOT NULL constraint failed: line_item\.unit_price/); + }); +}); + +describe('[#24768] every dialect', () => { + it.each([ + ['better-sqlite3', sqlite()], + ['libsql', libsql], + ['node-postgres', postgres], + ['mysql2', mysql], + ['sql.js (text only)', sqljs], + ])('%s', async (_driver, refuse) => { + const { engine } = await makeRig({ refuse }); + expectRefusalFor(await refusalOf(() => insertOne(engine)), 'required'); + }); +}); + +describe('[#24768] localized the way the validator localizes its own `required`', () => { + const ZH: Record = { + [`objects.${OBJECT}.fields.unit_price.label`]: '销售价格', + [`objects.${OBJECT}.fields.list_price.label`]: '标价', + }; + const i18n = { + t: (key: string, locale: string) => (locale === 'zh-CN' && ZH[key]) || key, + getLocales: () => ['en', 'zh-CN'], + }; + + it.each([['zh-CN'], ['zh']])('%s: the translated label in the zh-CN sentence, word for word the validator\'s', async (locale) => { + const { engine } = await makeRig({ refuse: sqlite() }); + engine.setI18nService(i18n); + const context = { locale }; + + const refusal = await refusalOf(() => insertOne(engine, { context })); + const expected = sentence('required', '销售价格', 'unit_price', 'zh-CN'); + expect(refusal.fields).toEqual([{ field: 'unit_price', code: 'required', message: expected, label: '销售价格' }]); + + // The validator, on the same request shape, for the field it refuses itself. + const own = await refusalOf(() => engine.insert(OBJECT, { name: 'n', unit_price: 1 }, { context } as any)); + const ownMessage = own.fields.find((f: any) => f.field === 'list_price')?.message; + expect(ownMessage).toBe(sentence('required', '标价', 'list_price', 'zh-CN')); + expect(refusal.fields[0].message.replace('销售价格', 'LABEL')).toBe(ownMessage.replace('标价', 'LABEL')); + }); +}); + +describe('[#24768] the controls: what the field map cannot name leaves as the driver threw it', () => { + it('a column no field declares (drift): the driver\'s error, as it leaves the engine', async () => { + const raw = sqlite(OBJECT, 'legacy_col')(); + const { engine } = await makeRig({ refuse: () => raw }); + const failure = await refusalOf(() => insertOne(engine)); + expect(failure).not.toBeInstanceOf(ValidationError); + expect(failure.code).toBe('SQLITE_CONSTRAINT_NOTNULL'); + }); + + it('a refusal the driver attributes to ANOTHER table: that object\'s door names it, not this one', async () => { + const { engine } = await makeRig({ refuse: sqlite('other_object', 'unit_price') }); + const failure = await refusalOf(() => insertOne(engine)); + expect(failure).not.toBeInstanceOf(ValidationError); + expect(failure.code).toBe('SQLITE_CONSTRAINT_NOTNULL'); + }); + + it('a NOT NULL code whose column the driver did not name: the driver\'s error', async () => { + const { engine } = await makeRig({ refuse: () => Object.assign(new Error('insert failed'), { code: '23502' }) }); + const failure = await refusalOf(() => insertOne(engine)); + expect(failure).not.toBeInstanceOf(ValidationError); + expect(failure.code).toBe('23502'); + }); + + it('a unique violation is still the duplicate envelope, never this one', async () => { + const { engine } = await makeRig({ + refuse: () => Object.assign(new Error(`UNIQUE constraint failed: ${OBJECT}.name`), { code: 'SQLITE_CONSTRAINT_UNIQUE' }), + }); + const failure = await refusalOf(() => insertOne(engine)); + expect(failure).toBeInstanceOf(DuplicateRecordError); + expect(failure).not.toBeInstanceOf(ValidationError); + }); + + it('`required` is still the validator\'s, before any driver runs — no pre-write check reads `storage.notNull`', async () => { + const { engine, driver } = await makeRig({ refuse: sqlite() }); + const failure = await refusalOf(() => engine.insert(OBJECT, { name: 'n' })); + // `list_price` is refused by the validator; `unit_price` is not judged before the driver. + expect(failure.fields.map((f: any) => f.field)).toEqual(['list_price']); + expect(driver.create).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/objectql/src/engine-update-duplicate-record.test.ts b/packages/objectql/src/engine-update-duplicate-record.test.ts index 66665ce86b9..8486367c1c4 100644 --- a/packages/objectql/src/engine-update-duplicate-record.test.ts +++ b/packages/objectql/src/engine-update-duplicate-record.test.ts @@ -32,7 +32,8 @@ * violation into the envelope — the by-id `driver.update` call and the * predicate `driver.updateMany` call — and the scoped-repository facade a * hook reaches as `ctx.api.object(name)` inherits it; and - * - **nothing else moves**: a NOT NULL violation, a deadlock, a missing table + * - **nothing else moves**: a NOT NULL violation on a column no field declares + * ([#24768]: on a declared field it is the validator's `ValidationError`), a deadlock, a missing table * and an unreachable store leave BOTH exits as the very object the driver * threw — asserted on IDENTITY, not on a message match, one pin per class * per exit (triage ruling, 2026-09-02). @@ -140,8 +141,14 @@ const memoryDuplicate = () => /* -------- the negative side: failures that must NOT change shape --------- */ +/** + * [#24768] On a column NO field of `doc` declares: a NOT NULL refusal on a + * declared field is the engine's `ValidationError` now + * (`engine-not-null-refusal.test.ts`), and this one is what still leaves as the + * driver threw it. + */ const notNullViolation = () => - Object.assign(new Error('NOT NULL constraint failed: doc.title'), { + Object.assign(new Error('NOT NULL constraint failed: doc.legacy_col'), { code: 'SQLITE_CONSTRAINT_NOTNULL', }); @@ -484,7 +491,7 @@ describe('engine.update — a driver unique violation is a DUPLICATE_RECORD enve describe('nothing that is not a unique violation changes shape', () => { const controls: Array<[string, () => unknown]> = [ - ['a NOT NULL violation', notNullViolation], + ['a NOT NULL violation on an undeclared column', notNullViolation], ['a missing table', missingTable], ['a deadlock', deadlock], ['an unreachable store', unreachableStore], diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 257dbfc44ee..86801524441 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -222,7 +222,8 @@ import { applyArtifactForwardConversions, resolveDeclaredRange } from '@objectst // engine is the consumer-side tolerant parsing PD #12 forbids and precedent // #5841 retired. import { isUniqueViolationError, uniqueViolationColumn } from '@objectstack/types'; -import { DuplicateRecordError, envelopeUniqueViolation } from './duplicate-record-error.js'; +import { DuplicateRecordError } from './duplicate-record-error.js'; +import { NotNullRefusalError, envelopeDriverWriteRefusal } from './not-null-refusal-error.js'; // [#8682] The write-path loggers' redaction — bound values never reach the log. // [#21274] …and its boundary face — nor the error that leaves the engine. // [#21385] It lives in `@objectstack/types` now, so `driver-sql`'s own log lines @@ -8627,13 +8628,15 @@ export class ObjectQL implements IObjectQLEngine { // driver's error. `envelopeUniqueViolation` returns everything that is // not a unique violation untouched (a deadlock, a NOT NULL, an // unreachable store), so only the recognised conflict changes shape. + // [#24768] …and a NOT NULL refusal on a declared field becomes the + // validator's own `ValidationError` (`envelopeDriverWriteRefusal`). // [#25073] On a driver-issued number the member answers instead, and // in answering re-seeds the counter it proved stale. const collided: string[] = byDriver ? ((await driver.issueAutonumbers(object, [row], driverOptions, error)) as string[][] | undefined)?.[0] ?? [] : this.isIssuedAutonumberCollision(error, issued) ? issued.map((one) => one.field) : []; if (collided.length === 0) { - throw envelopeUniqueViolation(error, object, this.getSchema(object)); + throw envelopeDriverWriteRefusal(error, object, this.getSchema(object), 'insert', this.validationMessageContext(object, execCtx)); } // Whatever happens next, the stale counter must not survive this call: // leaving it in place is what turned one collision into a storm. @@ -8676,7 +8679,7 @@ export class ObjectQL implements IObjectQLEngine { try { return await driver.create(object, row, driverOptions); } catch (retryError) { - throw envelopeUniqueViolation(retryError, object, this.getSchema(object)); + throw envelopeDriverWriteRefusal(retryError, object, this.getSchema(object), 'insert', this.validationMessageContext(object, execCtx)); } } } @@ -16981,7 +16984,7 @@ export class ObjectQL implements IObjectQLEngine { }); } } - throw envelopeUniqueViolation(error, object, this.getSchema(object)); + throw envelopeDriverWriteRefusal(error, object, this.getSchema(object), 'insert', msgCtx); } } } else { @@ -17169,7 +17172,9 @@ export class ObjectQL implements IObjectQLEngine { // `writeFailureLogMeta` rebuilds the `{ error: { message, stack } }` // context the `error` slot used to build (see its header: `warn` has no // slot, and an Error's fields are non-enumerable). - const logged = e instanceof DuplicateRecordError ? e.cause : e; + // [#24768] A NOT NULL refusal's `ValidationError` carries the driver's + // error the same way, for the same reason. + const logged = e instanceof DuplicateRecordError || e instanceof NotNullRefusalError ? e.cause : e; this.logger.warn( 'Insert operation failed', writeFailureLogMeta(redactBoundStatement(logged), { object }), @@ -18603,6 +18608,8 @@ export class ObjectQL implements IObjectQLEngine { // returns everything else untouched (a NOT NULL, a deadlock, a // missing table, an unreachable store), so only the conflict // changes shape; the contract is stated on `update()` above. + // [#24768] A NOT NULL refusal on a declared field is the second + // shape that changes: the validator's own `ValidationError`. // Wrapped HERE rather than at the outer `catch` below, and // deliberately: that `try` also encloses the `afterUpdate` // dispatch and the roll-up recompute, so an envelope applied @@ -18611,7 +18618,7 @@ export class ObjectQL implements IObjectQLEngine { try { result = await driver.update(object, hookContext.input.id as string, hookContext.input.data as Record, hookContext.input.options as any); } catch (driverError) { - throw envelopeUniqueViolation(driverError, object, this.getSchema(object)); + throw envelopeDriverWriteRefusal(driverError, object, this.getSchema(object), 'update', updateMsgCtx); } // [#21613] The by-id update returns the driver's post-write // readback (`select *` on driver-sql), so a column no metadata @@ -18974,7 +18981,7 @@ export class ObjectQL implements IObjectQLEngine { try { result = await driver.updateMany!(object, ast, hookContext.input.data as Record, hookContext.input.options as any); } catch (driverError) { - throw envelopeUniqueViolation(driverError, object, this.getSchema(object)); + throw envelopeDriverWriteRefusal(driverError, object, this.getSchema(object), 'update', updateMsgCtx); } isPredicateWrite = true; } @@ -19134,7 +19141,7 @@ export class ObjectQL implements IObjectQLEngine { // // [#17052] The insert door's twin here too: the level is `warn`, // because `throw e` on the next line IS the answer to the caller. - const logged = e instanceof DuplicateRecordError ? e.cause : e; + const logged = e instanceof DuplicateRecordError || e instanceof NotNullRefusalError ? e.cause : e; this.logger.warn( 'Update operation failed', writeFailureLogMeta(redactBoundStatement(logged), { object }), diff --git a/packages/objectql/src/not-null-refusal-error.ts b/packages/objectql/src/not-null-refusal-error.ts new file mode 100644 index 00000000000..adabd5e337d --- /dev/null +++ b/packages/objectql/src/not-null-refusal-error.ts @@ -0,0 +1,126 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { notNullViolationSite } from '@objectstack/types'; +import { envelopeUniqueViolation } from './duplicate-record-error.js'; +import { buildFieldError, ValidationError, type ValidationMessageContext } from './validation/record-validator.js'; + +/** + * [#24768] The engine's answer when the DATABASE refuses a write as a NOT NULL + * violation on a field the object declares: its own `ValidationError`, the one + * the record validator throws for `required`, so every door answers it alike. + * + * ## The defect this retires + * + * A field declared `storage: { notNull: true }` without `required` is ADR-0113's + * engine-populated column. D1 / D2 keep the write contract (`required`) and the + * column constraint apart on purpose, so the record validator judges `required` + * only and a write with no value for that field reaches the driver, which + * refuses it. The write doors let the DRIVER's error out, and each door then + * classified it from the driver's text in its own way. Measured on a real store + * (`driver-sql` over better-sqlite3), for one refused write: + * + * - the runtime dispatcher's `/data` POST and PATCH let the raw `SqliteError` + * out of `dispatch()`; neither exit recognised it, so both answered + * `500 INTERNAL_ERROR`; + * - the actions door answered `400 VALIDATION_ERROR` with the driver's text as + * the message and `SQLITE_CONSTRAINT_NOTNULL` as `declaredCode`; + * - `RestServer` answered `400 VALIDATION_FAILED`, naming the column key, and + * relabelled it by a second read of the object (#24715), only on its own + * `POST` / `PATCH`. + * + * ## The answer, and why each half is here + * + * - **A `ValidationError`, `code: 'VALIDATION_FAILED'`** — the class the record + * validator throws, recognised by every door already + * (`validationFailureDetails`, the classification's validation arm), so no + * door learns anything new and none answers it differently. + * - **One field error, built by `buildFieldError`** — the validator's own + * builder, with the validator's own message context, so the field is named by + * its label (translation bundle → declared `label` → API name) in the + * caller's locale by construction, and `code: 'required'`, the wire code a + * form maps to its input. The message key is the one the validator picks for + * the same verb: `required` on insert, `required_cleared` on update (an update + * reaches this refusal only by writing a null over the column). + * - **`cause`** — the driver's own error, whole, for the door's log line: the + * platform logger serializes `message` and `stack` only, so the write doors + * log the `cause` (as they do for `DuplicateRecordError`). Where the error + * leaves the engine its `cause` is cut like any driver error + * (`redactPropagatedDriverFault`). + * + * ## ⛔ What is left alone + * + * - A column the object's field map does not hold — the drift where metadata + * dropped a field and the column stayed NOT NULL. There is no field to name; + * the driver's error goes out as before, and the classification's column + * sentence and drift hint answer it. + * - A refusal the driver attributes to another table: a nested write to a + * second object inside this one. That object's own door translates it, with + * that object's field map. + * - `required` itself. ⛔ No pre-write check reads `storage.notNull`: ADR-0113 + * D1 / D2 rule it out, because a column a hook or a default fills is not one + * the form should demand. This only renames the database's own refusal. + */ +export class NotNullRefusalError extends ValidationError { + /** + * The driver's own error, whole. DECLARED rather than merely assigned: this + * repo compiles against `lib: ES2020`, where `Error` has no `cause` member. + */ + readonly cause: unknown; + + constructor(fields: ConstructorParameters[0], cause: unknown) { + super(fields); + this.cause = cause; + } +} + +/** The verb whose driver exit refused the write; it picks the validator's sentence. */ +export type NotNullRefusalVerb = 'insert' | 'update'; + +/** + * A write door's driver-error exit: the engine's `ValidationError` for a NOT NULL + * refusal on a field `schema` declares, or the caller's own error unchanged for + * anything else (see the module header for each case left alone). + * + * Idempotent, as `envelopeUniqueViolation` is: an error that is already this + * envelope is returned as-is. + */ +export function envelopeNotNullViolation( + error: unknown, + object: string, + schema: unknown, + verb: NotNullRefusalVerb, + messages: ValidationMessageContext | undefined, +): unknown { + if (error instanceof NotNullRefusalError) return error; + const site = notNullViolationSite(error); + if (!site) return error; + if (site.table !== undefined && site.table !== object) return error; + const fields = (schema as { fields?: Record } | undefined)?.fields; + const def = fields && Object.prototype.hasOwnProperty.call(fields, site.column) ? fields[site.column] : undefined; + if (!def || typeof def !== 'object') return error; + return new NotNullRefusalError( + [buildFieldError( + { field: site.column, code: 'required', def, messageKey: verb === 'update' ? 'required_cleared' : 'required' }, + messages, + )], + error, + ); +} + +/** + * The write doors' driver-error exit, both translations in one call: a unique + * violation becomes `DuplicateRecordError` (#14095), a NOT NULL refusal on a + * declared field this module's `ValidationError`, and anything else leaves as + * the driver threw it. The two cannot both answer: the unique predicate refuses + * NOT NULL text at its gate, and an envelope it built carries no NOT NULL code. + */ +export function envelopeDriverWriteRefusal( + error: unknown, + object: string, + schema: unknown, + verb: NotNullRefusalVerb, + messages: ValidationMessageContext | undefined, +): unknown { + const unique = envelopeUniqueViolation(error, object, schema); + return unique !== error ? unique : envelopeNotNullViolation(error, object, schema, verb, messages); +} diff --git a/packages/plugins/plugin-auth/src/last-admin-guard.anchor-parity.test.ts b/packages/plugins/plugin-auth/src/last-admin-guard.anchor-parity.test.ts index db5c7cb018d..6387a57e91b 100644 --- a/packages/plugins/plugin-auth/src/last-admin-guard.anchor-parity.test.ts +++ b/packages/plugins/plugin-auth/src/last-admin-guard.anchor-parity.test.ts @@ -162,13 +162,20 @@ async function resolverStanding(engine: ObjectQL, userId: string, email?: string const ban = (engine: ObjectQL, id: string) => engine.update('sys_user', { id, banned: true }, SYSTEM); /** The refusal envelope, or `undefined` when the write landed. */ -async function refusalOf(write: Promise): Promise<{ code?: unknown; status?: unknown } | undefined> { +async function refusalOf( + write: Promise, +): Promise<{ code?: unknown; status?: unknown; fields?: unknown; causeCode?: unknown } | undefined> { try { await write; return undefined; } catch (err) { - const e = err as { code?: unknown; status?: unknown }; - return { code: e.code, status: e.status }; + const e = err as { code?: unknown; status?: unknown; fields?: Array<{ field?: unknown; code?: unknown }>; cause?: { code?: unknown } }; + return { + code: e.code, + status: e.status, + fields: e.fields?.map((f) => [f.field, f.code]), + causeCode: e.cause?.code, + }; } } @@ -210,7 +217,12 @@ describe('[ADR-0131 D1/D9] where an organization-less admin_full_access grant ca expect(refusal?.code).toBe('ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED'); expect(rows).toEqual([]); } else if (c.expected === 'refused-not-null') { - expect(refusal?.code).toBe('SQLITE_CONSTRAINT_NOTNULL'); + // The COLUMN refused it: since #24768 the engine answers a NOT NULL + // refusal on a declared field as its own `ValidationError`, the + // driver's refusal on `cause`. + expect(refusal?.code).toBe('VALIDATION_FAILED'); + expect(refusal?.fields).toEqual([['organization_id', 'required']]); + expect(refusal?.causeCode).toBe('SQLITE_CONSTRAINT_NOTNULL'); expect(rows).toEqual([]); } else if (c.expected === 'derived') { expect(refusal).toBeUndefined(); diff --git a/packages/plugins/plugin-security/src/objects/shipped-sets-identity-default-row-scope.test.ts b/packages/plugins/plugin-security/src/objects/shipped-sets-identity-default-row-scope.test.ts index c501900d90e..fd7a0a9c007 100644 --- a/packages/plugins/plugin-security/src/objects/shipped-sets-identity-default-row-scope.test.ts +++ b/packages/plugins/plugin-security/src/objects/shipped-sets-identity-default-row-scope.test.ts @@ -380,26 +380,27 @@ describe.each(SHAPES)('the shipped sets serve the identity objects as before — restored = await boot(shape, withCopiesRestored(await registeredObjects())); }); - it('every persona is served exactly the listed ids on every touched object', async () => { + // One case per object, not one sweep over all of them: the restored-copies + // sweep was about 400 real-stack reads in a single case, which a loaded + // runner pushed past the 5000ms default on `main` as on any branch (#26065). + // Per object it is one persona column: the same reads and the same verdicts, + // each case a few dozen of them, and a failure names its object. + it.each(TOUCHED)('every persona is served exactly the listed ids on %s', async (object) => { const wrong: string[] = []; for (const persona of shape.personas) { - for (const object of TOUCHED) { - const now = await shipped.served(object, PERSONAS[persona]); - const want = expectedServed(shape, persona, object); - if (JSON.stringify(now) !== JSON.stringify(want)) wrong.push(`${persona} on ${object}: ${JSON.stringify(now)} (expected ${JSON.stringify(want)})`); - } + const now = await shipped.served(object, PERSONAS[persona]); + const want = expectedServed(shape, persona, object); + if (JSON.stringify(now) !== JSON.stringify(want)) wrong.push(`${persona} on ${object}: ${JSON.stringify(now)} (expected ${JSON.stringify(want)})`); } expect(wrong).toEqual([]); }); - it('…and exactly what the same deployment serves with the dropped copies spread back into the sets', async () => { + it.each([...TOUCHED, ...KEPT_ALL, 'sys_user'])('…and exactly what the same deployment serves on %s with the dropped copies spread back into the sets', async (object) => { const moved: string[] = []; for (const persona of shape.personas) { - for (const object of [...TOUCHED, ...KEPT_ALL, 'sys_user']) { - const was = await restored.served(object, PERSONAS[persona]); - const now = await shipped.served(object, PERSONAS[persona]); - if (JSON.stringify(now) !== JSON.stringify(was)) moved.push(`${persona} on ${object}: ${JSON.stringify(was)} -> ${JSON.stringify(now)}`); - } + const was = await restored.served(object, PERSONAS[persona]); + const now = await shipped.served(object, PERSONAS[persona]); + if (JSON.stringify(now) !== JSON.stringify(was)) moved.push(`${persona} on ${object}: ${JSON.stringify(was)} -> ${JSON.stringify(now)}`); } expect(moved).toEqual([]); }); diff --git a/packages/rest/src/crud-routes.ts b/packages/rest/src/crud-routes.ts index 6628fb5ff31..cb8dee7fb28 100644 --- a/packages/rest/src/crud-routes.ts +++ b/packages/rest/src/crud-routes.ts @@ -96,7 +96,6 @@ export interface CrudRouteHost { resolveExecCtx: RestServer['resolveExecCtx']; enforceAuth: RestServer['enforceAuth']; enforceApiAccess: RestServer['enforceApiAccess']; - labelNotNullRefusal: RestServer['labelNotNullRefusal']; } /** @@ -243,13 +242,10 @@ export function registerCrudEndpoints(this: CrudRouteHost, basePath: string): vo method: 'POST', path: `${dataPath}/:object`, handler: async (req: any, res: any) => { - // [#24715] Declared out here so the catch can read the - // caller's locale off it. - let context: any; try { const environmentId = isScoped ? req.params?.environmentId : undefined; const p = await this.resolveProtocol(environmentId, req); - context = await this.resolveExecCtx(environmentId, req); + const context = await this.resolveExecCtx(environmentId, req); if (this.enforceAuth(req, res, context)) return; if (await this.enforceApiAccess(req, res, p, environmentId, 'create')) return; // [#3899] The wire body IS the record. Validate the @@ -295,14 +291,10 @@ export function registerCrudEndpoints(this: CrudRouteHost, basePath: string): vo applyDroppedFieldsHeader(res, result); res.status(201).json(result); } catch (error: any) { - // [#24715] A NOT NULL refusal names the field by its label, - // in the caller's locale; every other answer is unchanged. - const mapped = await this.labelNotNullRefusal( - mapDataError(error, req.params?.object), - req, - isScoped ? req.params?.environmentId : undefined, - context, - ); + // [#24768] A NOT NULL refusal on a declared field arrives as + // the engine's `ValidationError`, already named by its label + // in the caller's locale; this door relabels nothing. + const mapped = mapDataError(error, req.params?.object); logUnexpectedRouteError(error, mapped); res.status(mapped.status).json(mapped.body); } @@ -393,12 +385,10 @@ export function registerCrudEndpoints(this: CrudRouteHost, basePath: string): vo method: 'PATCH', path: `${dataPath}/:object/:id`, handler: async (req: any, res: any) => { - // [#24715] As on the create: read by the catch. - let context: any; try { const environmentId = isScoped ? req.params?.environmentId : undefined; const p = await this.resolveProtocol(environmentId, req); - context = await this.resolveExecCtx(environmentId, req); + const context = await this.resolveExecCtx(environmentId, req); if (this.enforceAuth(req, res, context)) return; // OCC: clients opt in by sending either the standard // `If-Match` header or an `expectedVersion` field in @@ -485,13 +475,8 @@ export function registerCrudEndpoints(this: CrudRouteHost, basePath: string): vo applyDroppedFieldsHeader(res, result); res.json(result); } catch (error: any) { - // [#24715] The create's relabel, on the update. - const mapped = await this.labelNotNullRefusal( - mapDataError(error, req.params?.object), - req, - isScoped ? req.params?.environmentId : undefined, - context, - ); + // [#24768] As on the create: the engine named the field. + const mapped = mapDataError(error, req.params?.object); logUnexpectedRouteError(error, mapped); res.status(mapped.status).json(mapped.body); } diff --git a/packages/rest/src/data-not-null-field-label-24715.test.ts b/packages/rest/src/data-not-null-field-label-24715.test.ts index 8e75e0512df..eae75f21180 100644 --- a/packages/rest/src/data-not-null-field-label-24715.test.ts +++ b/packages/rest/src/data-not-null-field-label-24715.test.ts @@ -22,16 +22,26 @@ * 'os migrate' …", "object": "OBJECT" } * * Read through the public doors, over a REAL `ObjectQL` + - * `ObjectStackProtocolImplementation` + SQLite `:memory:`, with the stored - * metadata family registered so the protocol's by-name object read answers - * (the door reads the field's label and `storage` there). + * `ObjectStackProtocolImplementation` + SQLite `:memory:`. + * + * [#24768] The label now comes from the ENGINE, not from this door: a NOT NULL + * refusal on a field the object declares leaves `engine.insert` / + * `engine.update` as the record validator's own `ValidationError`, built by the + * validator's builder with the validator's message context, and this door + * answers it as it answers every `ValidationError` (the classification's + * validation arm). So the answer is the validator's envelope, byte for byte the + * shape of its own `required` refusal: `fields[0]` carries `label`, there is no + * `hint` (the validator's `required` has none), and a `PATCH` writing null reads + * the validator's clear-out sentence (`required_cleared`), as a `PATCH` clearing + * a `required` field does. The door's second object read and its relabel + * (`not-null-refusal-label.ts`) retired. * * The controls: a field the metadata declares with neither `required` nor - * `storage.notNull` on a NOT NULL column (drift) keeps the drift hint; a column - * the metadata does not declare at all keeps the column's sentence. Pinned by - * kind: the sentence is the catalog's `required` sentence for the label (asked - * of `renderValidationMessage`, the record validator's own renderer), and the - * hint is judged by what it prescribes. + * `storage.notNull` over a NOT NULL column (drift) is declared, so it is named + * by its label the same way; a column the metadata does not declare at all + * keeps the classification's column sentence and its drift hint. Pinned by + * kind: the sentence is the catalog's sentence for the label (asked of + * `renderValidationMessage`, the record validator's own renderer). */ import { describe, it, expect, afterEach } from 'vitest'; @@ -144,35 +154,34 @@ async function boot(opts: { locale?: string; i18n?: boolean } = {}) { call('PATCH', '/api/v1/data/:object/:id', data, { object: OBJECT, id: 'e1' }); const create = (data: Record) => call('POST', '/api/v1/data/:object', data); const stored = async () => (await engine.find(OBJECT, { where: { id: 'e1' } }) as any[])[0]; - return { engine, patch, create, stored }; + return { engine, patch, create, stored, call }; } -/** The catalog's `required` sentence for `label`, in `locale` — what the record validator renders. */ -const requiredSentence = (label: string, field: string, locale?: string) => - renderValidationMessage({ messageKey: 'required', label, field, params: {} }, { locale }); +/** The catalog's sentence for `label`, in `locale` — what the record validator renders. */ +const requiredSentence = (label: string, field: string, locale?: string, messageKey: 'required' | 'required_cleared' = 'required') => + renderValidationMessage({ messageKey, label, field, params: {} }, { locale }); -/** The envelope the classification answers, with the field's sentence `message`. */ -function expectRefusal(door: { status: number; body: any }, field: string, message: string) { +/** The record validator's envelope for one field, named by `label` with `message`. */ +function expectRefusal(door: { status: number; body: any }, field: string, label: string, message: string) { expect(door.status, JSON.stringify(door.body)).toBe(400); - expect(door.body).toMatchObject({ + expect(door.body).toEqual({ code: 'VALIDATION_FAILED', error: message, - fields: [{ field, code: 'required', message }], + fields: [{ field, code: 'required', message, label }], object: OBJECT, }); - // No key is added or dropped, on the body or on the field. - expect(Object.keys(door.body).sort()).toEqual(['code', 'error', 'fields', 'hint', 'object']); - expect(Object.keys(door.body.fields[0]).sort()).toEqual(['code', 'field', 'message']); } -/** The declared column's hint: the value, or the default / hook that supplies it — never `required`, never drift. */ -function expectDeclaredNotNullHint(hint: unknown, field: string) { - const text = String(hint); - expect(text).toContain(`'${field}'`); - expect(text).toContain('`storage: { notNull: true }`'); - expect(text).toMatch(/requires a value/); - expect(text).not.toContain('`required`'); - expect(text).not.toMatch(/drift|os migrate/); +/** The classification's answer for a column no field declares: the column's sentence, and its hint. */ +function expectColumnRefusal(door: { status: number; body: any }, field: string) { + expect(door.status, JSON.stringify(door.body)).toBe(400); + expect(door.body).toMatchObject({ + code: 'VALIDATION_FAILED', + error: `${field} is required`, + fields: [{ field, code: 'required', message: `${field} is required` }], + object: OBJECT, + }); + expect(Object.keys(door.body).sort()).toEqual(['code', 'error', 'fields', 'hint', 'object']); } describe('[#24715] a declared `storage.notNull` field\'s refusal names its label', () => { @@ -180,63 +189,85 @@ describe('[#24715] a declared `storage.notNull` field\'s refusal names its label const { engine } = await boot(); expect(AS_SYNCED.fields.unit_price).toMatchObject({ label: 'Sales Price', storage: { notNull: true } }); expect('required' in AS_SYNCED.fields.unit_price).toBe(false); - await expect(engine.update(OBJECT, { unit_price: null } as any, { where: { id: 'e1' } } as any)).rejects.toThrow(/not null/i); + // [#24768] …and the engine names it: the validator's class, not the driver's error. + const refused = await engine.update(OBJECT, { unit_price: null } as any, { where: { id: 'e1' } } as any).then( + () => undefined, + (e: any) => e, + ); + expect(refused?.name).toBe('ValidationError'); + expect(refused?.code).toBe('VALIDATION_FAILED'); }); - it('PATCH writing null: the label in `error` and `fields[0].message`, the declared column\'s hint', async () => { + it('PATCH writing null: the label, in the validator\'s clear-out sentence', async () => { const { patch, stored } = await boot(); const door = await patch({ unit_price: null }); - expectRefusal(door, 'unit_price', requiredSentence('Sales Price', 'unit_price')); + expectRefusal(door, 'unit_price', 'Sales Price', requiredSentence('Sales Price', 'unit_price', undefined, 'required_cleared')); expect(door.body.error).not.toContain('unit_price'); - expectDeclaredNotNullHint(door.body.hint, 'unit_price'); expect((await stored()).unit_price).toBe(5); }); - it('POST with no value: the same answer', async () => { + it('POST with no value: the label, in the validator\'s `required` sentence', async () => { const { create, engine } = await boot(); const door = await create({ id: 'n1', name: 'new', list_price: 1, drift_col: 'd' }); - expectRefusal(door, 'unit_price', requiredSentence('Sales Price', 'unit_price')); - expectDeclaredNotNullHint(door.body.hint, 'unit_price'); + expectRefusal(door, 'unit_price', 'Sales Price', requiredSentence('Sales Price', 'unit_price')); expect(await engine.find(OBJECT, { where: { id: 'n1' } })).toHaveLength(0); }); }); +describe('[#24768] the batch rows answer the same refusal by the same label', () => { + it('`POST /data/:object/batch`, a create row with no value: the row\'s error is the validator\'s code and sentence', async () => { + const { call, engine } = await boot(); + const door = await call('POST', '/api/v1/data/:object/batch', { + operation: 'create', + records: [{ data: { id: 'b1', name: 'new', list_price: 1, drift_col: 'd' } }], + options: { atomic: false }, + }); + expect(door.status, JSON.stringify(door.body)).toBe(200); + expect(door.body).toMatchObject({ succeeded: 0, failed: 1 }); + expect(door.body.results[0].errors).toEqual([ + expect.objectContaining({ code: 'VALIDATION_FAILED', message: requiredSentence('Sales Price', 'unit_price') }), + ]); + expect(JSON.stringify(door.body)).not.toMatch(/constraint failed|unit_price is required/); + expect(await engine.find(OBJECT, { where: { id: 'b1' } })).toHaveLength(0); + }); +}); + describe('[#24715] localized the way the record validator localizes its own `required`', () => { it('zh-CN: the translated label in the catalog\'s zh-CN sentence — the validator\'s template, word for word', async () => { const { patch, create } = await boot({ locale: 'zh-CN', i18n: true }); const door = await patch({ unit_price: null }); - expectRefusal(door, 'unit_price', requiredSentence('销售价格', 'unit_price', 'zh-CN')); - expectDeclaredNotNullHint(door.body.hint, 'unit_price'); + expectRefusal(door, 'unit_price', '销售价格', requiredSentence('销售价格', 'unit_price', 'zh-CN', 'required_cleared')); + + const missing = await create({ id: 'n2', name: 'new', list_price: 1, drift_col: 'd' }); + expectRefusal(missing, 'unit_price', '销售价格', requiredSentence('销售价格', 'unit_price', 'zh-CN')); // The validator, on the same request shape, for the field it refuses itself. const own = await create({ id: 'n1', name: 'new', unit_price: 1, drift_col: 'd' }); expect(own.status, JSON.stringify(own.body)).toBe(400); const validatorMessage = own.body.fields.find((f: any) => f.field === 'list_price')?.message; expect(validatorMessage).toBe(requiredSentence('标价', 'list_price', 'zh-CN')); - expect(door.body.error.replace('销售价格', 'LABEL')).toBe(validatorMessage.replace('标价', 'LABEL')); + expect(missing.body.error.replace('销售价格', 'LABEL')).toBe(validatorMessage.replace('标价', 'LABEL')); }); it('a bare `zh` reaches the zh-CN bundle, as the validator\'s locale negotiation does', async () => { const { patch } = await boot({ locale: 'zh', i18n: true }); const door = await patch({ unit_price: null }); - expectRefusal(door, 'unit_price', requiredSentence('销售价格', 'unit_price', 'zh-CN')); + expectRefusal(door, 'unit_price', '销售价格', requiredSentence('销售价格', 'unit_price', 'zh-CN', 'required_cleared')); }); }); -describe('[#24715] the controls: what the field map cannot name keeps today\'s answer', () => { - it('drift — declared with neither `required` nor `storage.notNull` over a NOT NULL column: the label, and the drift hint', async () => { +describe('[#24715] the controls', () => { + it('drift — declared with neither `required` nor `storage.notNull` over a NOT NULL column: still a declared field, so its label', async () => { const { patch } = await boot(); const door = await patch({ drift_col: null }); - expectRefusal(door, 'drift_col', requiredSentence('Drift Column', 'drift_col')); - expect(String(door.body.hint)).toMatch(/drifted from metadata/); - expect(String(door.body.hint)).toMatch(/os migrate/); + expectRefusal(door, 'drift_col', 'Drift Column', requiredSentence('Drift Column', 'drift_col', undefined, 'required_cleared')); }); it('a column the metadata does not declare: the column\'s sentence and the drift hint', async () => { const { create } = await boot(); const door = await create({ id: 'n1', name: 'new', unit_price: 1, list_price: 1, drift_col: 'd' }); - expectRefusal(door, 'gone_col', 'gone_col is required'); + expectColumnRefusal(door, 'gone_col'); expect(String(door.body.hint)).toMatch(/os migrate/); }); }); diff --git a/packages/rest/src/data-not-null-hint-20963.test.ts b/packages/rest/src/data-not-null-hint-20963.test.ts index 52f4b5e7b24..518b05c4925 100644 --- a/packages/rest/src/data-not-null-hint-20963.test.ts +++ b/packages/rest/src/data-not-null-hint-20963.test.ts @@ -25,15 +25,18 @@ * ⚠️ Not pinned: telling the two cases apart. That needs a registry read, which * the mapper does not make. * - * [#24715] The door now makes that read, and pins it in - * `data-not-null-field-label-24715.test.ts`: the field's label in the sentence, - * and a declared `storage.notNull` column's own hint. THIS harness registers no - * stored-metadata family, so the protocol's object read fails, the door's - * relabel declines, and the classification's answer goes out as it is — which - * makes these cases the fail-safe control for that read. The hint they pin - * changed with #24715 too: its first sentence no longer adds "or declare the - * field `required`", which for a column a hook populates is the move ADR-0113 - * D1 / D2 rule out. + * [#24768] A field the object DECLARES no longer reaches the classification at + * all: the engine refuses that write as the record validator's own + * `ValidationError`, named by the field's label, and this door relabels + * nothing (`data-not-null-field-label-24715.test.ts` pins both `/data` doors' + * answer for it, the `PATCH` included). What the classification's hint still + * answers is a NOT NULL column the served metadata does not declare: the drift + * where a field was dropped and its column stayed NOT NULL. So `must` is + * declared when the table is built, and dropped from the metadata the door + * serves afterwards; only the create can reach that column then (a `PATCH` + * naming it is refused as an undeclared field before any driver runs). The + * hint pinned below is the one a declared column used to get, and it holds for + * this case too: it leads with the value and names drift only as a condition. */ import { describe, it, expect, afterEach } from 'vitest'; @@ -44,16 +47,22 @@ import { RestServer } from './rest-server'; const OBJECT = 'proj_notnull_hint_20963'; +/** The metadata as the DDL saw it: `must` is built NOT NULL. */ const PROJ = { name: OBJECT, label: 'Proj notnull hint 20963', systemFields: false, fields: { id: { name: 'id', type: 'text' as const, primaryKey: true }, title: { name: 'title', type: 'text' as const }, - // Physically NOT NULL, not `required`: declared on purpose, so nothing has drifted. must: { name: 'must', type: 'text' as const, storage: { notNull: true } }, }, }; +/** The metadata the door serves afterwards: `must` is no longer declared. */ +const SERVED = (() => { + const { must: _must, ...fields } = PROJ.fields; + return { ...PROJ, fields }; +})(); + function createMockServer() { const noop = () => {}; return { get: noop, post: noop, put: noop, delete: noop, patch: noop, use: noop, listen: async () => {}, close: async () => {} }; @@ -84,6 +93,7 @@ async function boot() { engine.registry.registerObject(PROJ as any); await engine.syncSchemas(); await engine.insert(OBJECT, { id: 'e1', title: 'existing', must: 'm' } as any); + engine.registry.registerObject(SERVED as any); const protocol = new ObjectStackProtocolImplementation(engine as any); const rest = new RestServer(createMockServer() as any, protocol as any, { api: { requireAuth: false } } as any); @@ -138,28 +148,17 @@ function expectHintLeadsWithTheRemedy(hint: unknown) { expect(sentences.indexOf(drift[0]!), text).toBeGreaterThan(0); } -describe('[#20963] a declared `storage.notNull` column\'s refusal does not assert drift', () => { - it('the fixture is the ADR-0113 posture: the database refuses what the validator lets through', async () => { +describe('[#20963] a NOT NULL column the metadata does not declare: the hint leads with the remedy', () => { + it('the fixture: the database refuses the omitted column, and no field is there to name', async () => { const { engine } = await boot(); - expect(PROJ.fields.must).toMatchObject({ storage: { notNull: true } }); - expect('required' in PROJ.fields.must).toBe(false); + expect('must' in SERVED.fields).toBe(false); await expect(engine.insert(OBJECT, { id: 'x', title: 't' } as any)).rejects.toThrow(/not null/i); }); - it('POST /data/:object with no value for the column: the same answer, a hint that leads with the remedy', async () => { + it('POST /data/:object with no value for the column: the column\'s answer, a hint that leads with the remedy', async () => { const { call } = await boot(); const door = await call('POST', '/api/v1/data/:object', { title: 't' }); expectNotNullAnswer(door); expectHintLeadsWithTheRemedy(door.body.hint); }); - - it('PATCH /data/:object/:id writing null onto the column: the same answer, the same hint', async () => { - const { call } = await boot(); - const created = await call('POST', '/api/v1/data/:object', { title: 't' }); - const door = await call('PATCH', '/api/v1/data/:object/:id', { must: null }, { object: OBJECT, id: 'e1' }); - expectNotNullAnswer(door); - expectHintLeadsWithTheRemedy(door.body.hint); - // One branch answers both doors: the update says what the create said. - expect(door.body.hint).toBe(created.body.hint); - }); }); diff --git a/packages/rest/src/not-null-refusal-label.test.ts b/packages/rest/src/not-null-refusal-label.test.ts deleted file mode 100644 index 01f43fa4eed..00000000000 --- a/packages/rest/src/not-null-refusal-label.test.ts +++ /dev/null @@ -1,66 +0,0 @@ -// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. - -/** - * [#24715] `labelNotNullRefusal`'s declines, each with the source it must not - * ask. The doors' answer on a real driver is pinned in - * `data-not-null-field-label-24715.test.ts`; these are the cases a real - * boot reaches only through a hook's write or a failing read. - */ - -import { describe, it, expect, vi } from 'vitest'; -import { mapDataError } from '@objectstack/types'; -import { labelNotNullRefusal, type NotNullRefusalLabelSources } from './not-null-refusal-label.js'; - -const OBJECT = 'line_item'; -const notNull = (table: string) => - mapDataError(Object.assign(new Error(`NOT NULL constraint failed: ${table}.unit_price`), { code: 'SQLITE_CONSTRAINT_NOTNULL' }), OBJECT); - -function sources(overrides: Partial = {}) { - const s = { - object: OBJECT, - readObject: vi.fn(async () => ({ - name: OBJECT, - fields: { unit_price: { type: 'number', label: 'Sales Price', storage: { notNull: true } } }, - })), - locale: undefined, - resolveI18nService: vi.fn(async () => undefined), - ...overrides, - }; - return s as typeof s & NotNullRefusalLabelSources; -} - -describe('[#24715] labelNotNullRefusal', () => { - it('CONTROL: the requested object\'s refusal is named by the label', async () => { - const s = sources(); - const answer = await labelNotNullRefusal(notNull(OBJECT), s); - expect(answer.body.error).toBe('Sales Price is required'); - expect(s.readObject).toHaveBeenCalledWith(OBJECT); - }); - - it('a refusal the driver attributes to ANOTHER table is not the requested object\'s field: unchanged, nothing read', async () => { - const s = sources(); - const original = notNull('audit_row'); - expect(await labelNotNullRefusal(original, s)).toBe(original); - expect(s.readObject).not.toHaveBeenCalled(); - }); - - it('any other answer: unchanged, nothing read', async () => { - const s = sources(); - const original = mapDataError(Object.assign(new Error('UNIQUE constraint failed: line_item.code'), { code: 'SQLITE_CONSTRAINT_UNIQUE' }), OBJECT); - expect(await labelNotNullRefusal(original, s)).toBe(original); - expect(s.readObject).not.toHaveBeenCalled(); - expect(s.resolveI18nService).not.toHaveBeenCalled(); - }); - - it('an object read that fails leaves the classification\'s answer as it is', async () => { - const original = notNull(OBJECT); - const s = sources({ readObject: vi.fn(async () => { throw new Error('metadata store unreachable'); }) }); - expect(await labelNotNullRefusal(original, s)).toBe(original); - }); - - it('a column the object does not declare: unchanged', async () => { - const original = notNull(OBJECT); - const s = sources({ readObject: vi.fn(async () => ({ name: OBJECT, fields: {} })) }); - expect(await labelNotNullRefusal(original, s)).toBe(original); - }); -}); diff --git a/packages/rest/src/not-null-refusal-label.ts b/packages/rest/src/not-null-refusal-label.ts deleted file mode 100644 index f2a76857df4..00000000000 --- a/packages/rest/src/not-null-refusal-label.ts +++ /dev/null @@ -1,173 +0,0 @@ -// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. - -/** - * [#24715] A `/data` write's NOT NULL refusal, named by the field's LABEL in the - * caller's language. - * - * ## What was wrong - * - * A field declared `storage: { notNull: true }` without `required` is - * ADR-0113's engine-populated column: the record validator judges `required` - * only, so a write with no value for it reaches the driver, and the DATABASE - * refuses it. `mapDataError` classifies that refusal from the driver's text, - * which carries the column and nothing else, so `PATCH` with - * `{ "unit_price": null }` answered `error: "unit_price is required"` for a - * field labelled "Sales Price", and the hint told the author to declare the - * field `required` — the move ADR-0113 D1 / D2 rule out for a column a hook - * fills. The same field declared `required` was refused by the validator - * instead, by its label, so the posture the ADR recommends read worse. - * - * ## The split - * - * The classification stays field-map-free: it sees the driver's text and the - * object's name, and its answer names the column. THIS door holds the object, - * so it names the field — and only the field — for that one answer - * (`notNullRefusalSite` recognises it, `relabelNotNullRefusal` rewrites it, both - * in `@objectstack/types`). Nothing else moves: `status`, `code`, - * `fields[0].field`, `fields[0].code`, `object` and the key set are the - * classification's. - * - * ## Localized the way the record validator localizes its own `required` - * - * The validator (`@objectstack/objectql`, `record-validator.ts`) renders - * `{{label}} is required` from the shared catalog in the caller's locale, and - * names the field translation bundle → declared `label` → API name. Its - * builder is not on `@objectstack/objectql`'s public entry, and this package - * does not depend on the engine at run time, so what is shared is what that - * builder is made of — every piece a `@objectstack/spec/system` export: - * - * - the sentence: `renderValidationMessage`, message key `required`, with the - * deployment's `validation.field.required` override honoured through the - * i18n service's `t`; - * - the label: `t(objectFieldLabelKey(object, field), locale)`, a miss being - * the key echoed back, then the declared `label`, then the API name; - * - the locale: `ExecutionContext.locale` — the value the engine's validator - * received for this same request — negotiated against the locales the i18n - * service holds by `resolveBundleLocale`, the one negotiation rule the engine - * asks too (`zh` reaches a `zh-CN` bundle). - * - * ## What is left alone - * - * - A column the object does not declare: the drift where metadata dropped a - * field and the column stayed NOT NULL. There is no label to give; the - * column's sentence and the drift hint are the answer. - * - A refusal the driver attributes to ANOTHER table (SQLite and Postgres name - * it): a hook's write to a second object failed inside this request, and the - * column is that object's, not one of the requested object's fields. - * - Any read that fails: the classification's answer goes out unchanged. A - * label is a courtesy on an answer that is already correct; it never turns a - * `400` into anything else. - */ - -import { - objectFieldLabelKey, - renderValidationMessage, - resolveBundleLocale, - type ValidationMessageTranslator, -} from '@objectstack/spec/system'; -import { notNullRefusalSite, relabelNotNullRefusal } from '@objectstack/types'; - -/** The I/O {@link labelNotNullRefusal} needs from the door; each is asked only for a NOT NULL refusal. */ -export interface NotNullRefusalLabelSources { - /** The object the request addressed — the route's `:object` segment. */ - readonly object: string | undefined; - /** That object's definition by name, or `undefined` when nothing is behind the name. */ - readObject(name: string): Promise; - /** The caller's `ExecutionContext.locale`, or `undefined` when it has none. */ - readonly locale: string | undefined; - /** The request environment's i18n service, or `undefined` when none is registered. */ - resolveI18nService(): Promise; -} - -type ClassifiedAnswer = { status: number; body: Record }; - -/** - * Name the field of a NOT NULL refusal by its label, in the caller's locale — - * or hand `answer` back unchanged when it is not one, or when the field cannot - * be named (see the module header for each case). - */ -export async function labelNotNullRefusal( - answer: ClassifiedAnswer, - sources: NotNullRefusalLabelSources, -): Promise { - const site = notNullRefusalSite(answer); - const object = sources.object; - if (!site || !object) return answer; - if (site.table !== undefined && site.table !== object) return answer; - try { - const definition = (await sources.readObject(object)) as { fields?: Record } | undefined; - const field = definition?.fields?.[site.column] as - | { label?: unknown; storage?: { notNull?: unknown } } - | undefined; - if (!field || typeof field !== 'object') return answer; - - const i18n = (await sources.resolveI18nService()) as - | { t?: (key: string, locale: string, params?: Record) => string; getLocales?: () => unknown } - | undefined; - const translate: ValidationMessageTranslator | undefined = typeof i18n?.t === 'function' - ? (key, locale, params) => i18n.t!(key, locale, params) - : undefined; - const locale = negotiatedLocale(i18n, sources.locale); - const label = fieldLabel(object, site.column, field.label, translate, locale); - const message = renderValidationMessage( - { messageKey: 'required', label, field: site.column, params: {} }, - { locale, translate }, - ); - return relabelNotNullRefusal(answer, { - message, - // Truthiness, as the driver reads it when it builds the column. - declaresNotNull: Boolean(field.storage?.notNull), - }); - } catch { - return answer; - } -} - -/** - * The field's display name: the translation bundle in `locale`, then the - * declared `label`, then the API name. A translator echoing the key back is a - * miss (the `II18nService.t` contract), and a throwing one is a miss too. - */ -function fieldLabel( - object: string, - column: string, - declared: unknown, - translate: ValidationMessageTranslator | undefined, - locale: string | undefined, -): string { - if (translate && locale) { - const key = objectFieldLabelKey(object, column); - try { - const translated = translate(key, locale); - if (typeof translated === 'string' && translated.length > 0 && translated !== key) return translated; - } catch { - // The declared label below. - } - } - const label = typeof declared === 'string' ? declared.trim() : ''; - return label.length > 0 ? label : column; -} - -/** - * The locale the i18n service can answer in, for the one the caller asked for: - * `resolveBundleLocale` over the service's `getLocales()`. The requested tag - * passes through untouched when there is nothing to negotiate against. - */ -function negotiatedLocale( - i18n: { getLocales?: () => unknown } | undefined, - requested: string | undefined, -): string | undefined { - if (!requested || typeof i18n?.getLocales !== 'function') return requested; - let available: unknown; - try { - available = i18n.getLocales(); - } catch { - return requested; - } - if (!Array.isArray(available) || available.length === 0) return requested; - const offered: Record = {}; - for (const code of available) { - if (typeof code === 'string' && code.length > 0) offered[code] = true; - } - return resolveBundleLocale(offered, requested) ?? requested; -} diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index 6ad1c9ab5d0..061a128be69 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -336,8 +336,6 @@ import { registerUiEndpoints, type UiRouteHost } from './ui-routes.js'; import { registerFormEndpoints, type FormRouteHost } from './form-routes.js'; // [#23050 slice 12] The CRUD routes' body moved to its own module — see its header. import { registerCrudEndpoints, type CrudRouteHost } from './crud-routes.js'; -// [#24715] A `/data` write's NOT NULL refusal, named by the field's label. -import { labelNotNullRefusal as labelNotNullRefusalAnswer } from './not-null-refusal-label.js'; // [#23050 slice 13a] The `/meta` type list, whole-store maintenance and per-type list // groups moved to their own module — see its header. That module is also the home of the // meta doors' request envelope `TransportScopedMetaRequest` and of `MetaRouteScope`. @@ -2605,9 +2603,9 @@ export class RestServer { * envelope. Same env-scoped request shape the list read sends * (`metaListSource`). * - * [#24535] On the protocol's EXECUTION face where it has one: neither - * reader serves a byte of the item (the exposure gate reads `enable`, the - * NOT NULL relabel a field's `label` and `storage`), and the served face's + * [#24535] On the protocol's EXECUTION face where it has one: the reader + * serves no byte of the item (the exposure gate reads `enable`; the NOT NULL + * relabel that read a field's `label` retired with #24768), and the served face's * decoration was a full diagnostic `safeParse` of the object's definition on * every `/data` request. Same sources, overlay cache and governance either * way; a protocol without it is asked `getMetaItem`. @@ -2630,40 +2628,6 @@ export class RestServer { }; } - /** - * [#24715] Name a `/data` write's NOT NULL refusal by the field's label, in - * the caller's locale (`./not-null-refusal-label.ts` carries the rule, the - * localization and every case it leaves alone). - * - * Every other answer is handed back before anything is read. For a NOT NULL - * refusal this door resolves the protocol again (the handler's own was - * scoped to its `try`), reads the object by name ({@link objectItemReader}, - * the read the exposure gate makes on every `/data` request) and resolves - * the i18n service. `context` is the execution context the handler handed - * the write, so its `locale` is the one the engine's validator was given; - * it is absent when the handler threw before resolving it. - */ - private async labelNotNullRefusal( - answer: { status: number; body: Record }, - req: any, - environmentId: string | undefined, - context: { locale?: unknown } | undefined, - ): Promise<{ status: number; body: Record }> { - const locale = context?.locale; - return labelNotNullRefusalAnswer(answer, { - object: typeof req?.params?.object === 'string' ? req.params.object : undefined, - readObject: async (name) => { - const read = this.objectItemReader(await this.resolveProtocol(environmentId, req), environmentId); - const item = ((read ? await read(name) : undefined) as { item?: unknown } | undefined)?.item; - return item !== null && typeof item === 'object' && (item as { name?: unknown }).name === name - ? item - : undefined; - }, - locale: typeof locale === 'string' && locale.length > 0 ? locale : undefined, - resolveI18nService: () => this.resolveI18nService(environmentId, req), - }); - } - /** * Enforce object-level API exposure (ObjectSchema `enable.apiEnabled` / * `enable.apiMethods`) on the REST data surface — the *external* API boundary @@ -5584,7 +5548,6 @@ export class RestServer { resolveExecCtx: (...a) => this.resolveExecCtx(...a), enforceAuth: (...a) => this.enforceAuth(...a), enforceApiAccess: (...a) => this.enforceApiAccess(...a), - labelNotNullRefusal: (...a) => this.labelNotNullRefusal(...a), }; registerCrudEndpoints.call(host, basePath); } diff --git a/packages/runtime/src/dispatcher-not-null-refusal-24768.test.ts b/packages/runtime/src/dispatcher-not-null-refusal-24768.test.ts new file mode 100644 index 00000000000..96e3a6000a0 --- /dev/null +++ b/packages/runtime/src/dispatcher-not-null-refusal-24768.test.ts @@ -0,0 +1,178 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#24768] The runtime dispatcher's doors answer a database NOT NULL refusal on + * a declared field as the record validator answers `required`: + * `400 VALIDATION_FAILED`, the field named by its label. + * + * ## What was wrong + * + * `unit_price` (label "Sales Price") is ADR-0113's engine-populated column: + * `storage: { notNull: true }`, no `required`, so a write with no value for it + * reaches the driver and the DATABASE refuses it. The engine let the driver's + * error out, and each door classified it from the text its own way. Measured on + * `origin/main` `0c37228826`, on the rig below: + * + * - `POST /data/OBJECT` without the field and `PATCH /data/OBJECT/ID` with + * `{ "unit_price": null }`: the raw `SqliteError` + * (`SQLITE_CONSTRAINT_NOTNULL`) left `dispatch()`, and the Hono exit + * (`declaredEnvelopeForThrow`) declined it, so the door answered + * `500 INTERNAL_ERROR`; `dispatcher-plugin`'s exit reads the same + * declaration (`validationFailureDetails`) and answered `500` too; + * - `POST /actions/OBJECT/clear_price/ID` (a declarative update writing null): + * `400 VALIDATION_ERROR`, the driver's text as the message and + * `declaredCode: 'SQLITE_CONSTRAINT_NOTNULL'`. + * + * The engine now refuses that write as its own `ValidationError` + * (`@objectstack/objectql`, `not-null-refusal-error.ts`), the class every door + * already answers alike, so no door here changed. + * + * ## The rig + * + * A REAL store: better-sqlite3 `:memory:` under `driver-sql`, the real + * `ObjectQL` engine and protocol. The dispatcher is reached the way + * `@objectstack/hono`'s catch-all reaches it, `dispatch()` over a real Fetch + * `Request`, with the identity step the one stubbed seam; a throw out of + * `dispatch()` is rendered by the exits' own recognisers. + * + * Codes, the field envelope and the label are asserted; the sentence is asked + * of the validator's renderer, never spelled here. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { declaredEnvelopeForThrow } from '@objectstack/plugin-hono-server'; +import { validationFailureDetails } from '@objectstack/types'; +import { renderValidationMessage } from '@objectstack/spec/system'; +import { HttpDispatcher } from './http-dispatcher.js'; +import { installDefaultOrganization } from './default-organization.testkit.js'; + +const PREFIX = '/api/v1'; +/** `registry.registerObject` requires a package id (see the sibling real-stack tests). */ +const TEST_PACKAGE_ID = 'objectstack-test'; +const OBJECT = 'line_item_24768'; + +const LINE = { + name: OBJECT, + label: 'Line item', + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true, label: 'ID' }, + name: { name: 'name', type: 'text' as const, label: 'Name' }, + // ADR-0113's engine-populated column: NOT NULL on purpose, not `required`. + unit_price: { name: 'unit_price', type: 'number' as const, label: 'Sales Price', storage: { notNull: true } }, + }, + actions: [ + { name: 'clear_price', label: 'Clear price', type: 'script', operation: 'update', patch: { unit_price: null } }, + ], +}; + +const MEMBER = { userId: 'u_member', isSystem: false }; + +let engine: ObjectQL; +let dispatcher: HttpDispatcher; + +afterAll(async () => { + try { await engine?.destroy(); } catch { /* noop */ } +}); + +beforeAll(async () => { + engine = new ObjectQL(); + engine.registerDriver(new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }), true); + await engine.init(); + engine.registry.registerObject(LINE as any, TEST_PACKAGE_ID); + await engine.syncSchemas(); + // A real `single` install's Default Organization (ADR-0131 D9). + await installDefaultOrganization(engine); + await engine.insert(OBJECT, { id: 'e1', name: 'existing', unit_price: 5 } as any); + const protocol = new ObjectStackProtocolImplementation(engine as any); + const services: Record = { objectql: engine, data: engine, protocol }; + const get = (n: string) => services[n] ?? null; + const kernel: any = { context: { getService: get }, getService: get, getServiceAsync: async (n: string) => get(n) }; + dispatcher = new HttpDispatcher(kernel); + (dispatcher as any).timedResolveExecutionContext = async () => ({ ...MEMBER }); +}, 60_000); + +/** + * A request as `@objectstack/hono`'s catch-all hands it to `dispatch()`: the + * answer `dispatch()` returned, or — for a throw — what each exit makes of it. + */ +async function door(method: string, path: string, body: unknown) { + const request = new Request(`http://localhost${PREFIX}${path}`, { + method, + headers: { 'content-type': 'application/json' }, + body: JSON.stringify(body), + }); + try { + const res: any = await dispatcher.dispatch(method, path, body, {}, { request } as any, PREFIX); + return { returned: res?.response as { status: number; body: any } | undefined }; + } catch (thrown) { + return { + thrown, + // `@objectstack/hono`'s catch-all and `plugin-hono-server`'s exit. + hono: declaredEnvelopeForThrow(thrown), + // `dispatcher-plugin`'s `errorResponseBase` reads this declaration. + validation: validationFailureDetails(thrown), + }; + } +} + +const sentence = (messageKey: 'required' | 'required_cleared') => + renderValidationMessage({ messageKey, label: 'Sales Price', field: 'unit_price', params: {} }, {}); + +/** The validator's field envelope for `unit_price`, by its label. */ +function expectFieldEnvelope(fields: unknown, messageKey: 'required' | 'required_cleared') { + expect(fields).toEqual([ + { field: 'unit_price', code: 'required', message: sentence(messageKey), label: 'Sales Price' }, + ]); +} + +/** A throw out of `dispatch()` that both exits answer `400 VALIDATION_FAILED` with the field's label. */ +function expectThrownRefusal(answer: Awaited>, messageKey: 'required' | 'required_cleared') { + expect('thrown' in answer, JSON.stringify(answer)).toBe(true); + const { hono, validation } = answer as { hono: any; validation: any }; + expect(hono?.status, JSON.stringify(hono)).toBe(400); + expect(hono.body.error.code).toBe('VALIDATION_FAILED'); + expect(hono.body.error.message).toBe(sentence(messageKey)); + expectFieldEnvelope(hono.body.error.details?.fields, messageKey); + expect(validation?.code).toBe('VALIDATION_FAILED'); + expectFieldEnvelope(validation.fields, messageKey); + // Neither the driver's text nor its code crosses. + const wire = JSON.stringify(hono.body); + expect(wire).not.toMatch(/constraint failed|SQLITE_/); +} + +const storedPrice = async () => ((await engine.findOne(OBJECT, { where: { id: 'e1' } })) as any)?.unit_price; + +describe('[#24768] the dispatcher `/data` doors answer a NOT NULL refusal as `400 VALIDATION_FAILED`, by label', () => { + it('`POST /data/:object` without the field', async () => { + const answer = await door('POST', `/data/${OBJECT}`, { id: 'n1', name: 'new' }); + expectThrownRefusal(answer, 'required'); + expect(await engine.find(OBJECT, { where: { id: 'n1' } })).toHaveLength(0); + }); + + it('`PATCH /data/:object/:id` writing null', async () => { + const answer = await door('PATCH', `/data/${OBJECT}/e1`, { unit_price: null }); + expectThrownRefusal(answer, 'required_cleared'); + expect(await storedPrice()).toBe(5); + }); +}); + +describe('[#24768] the actions door answers the same refusal the same way', () => { + it('a declarative `operation: \'update\'` action writing null', async () => { + const answer = await door('POST', `/actions/${OBJECT}/clear_price/e1`, {}); + const response = (answer as { returned?: { status: number; body: any } }).returned; + expect(response?.status, JSON.stringify(answer)).toBe(400); + expect(response!.body.error.code).toBe('VALIDATION_FAILED'); + expect(response!.body.error.message).toBe(sentence('required_cleared')); + expectFieldEnvelope(response!.body.error.details?.fields, 'required_cleared'); + expect(response!.body.error).not.toHaveProperty('declaredCode'); + expect(JSON.stringify(response!.body)).not.toMatch(/constraint failed|SQLITE_/); + expect(await storedPrice()).toBe(5); + }); +}); diff --git a/packages/types/src/data-error-classification.not-null-label.test.ts b/packages/types/src/data-error-classification.not-null-label.test.ts index 2b3e5408313..edfcc2686a8 100644 --- a/packages/types/src/data-error-classification.not-null-label.test.ts +++ b/packages/types/src/data-error-classification.not-null-label.test.ts @@ -1,60 +1,90 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * [#24715] The classification's half of a NOT NULL refusal named by its field's - * label: the site it recognises, the relabel it hands the caller that holds the - * field map, and the hint that has to hold when nobody relabels. + * [#24768] The classification's half of a NOT NULL refusal: the site a driver + * error names (`notNullViolationSite`, which the engine asks at its write doors + * before it names the field by its label), and the hint `mapDataError` gives a + * refusal nothing translated — a column no field declares. * - * The door half — `@objectstack/rest`'s `/data` create and update, on a real - * driver — is pinned in `packages/rest/src/data-not-null-field-label-24715.test.ts`. + * The engine half is pinned in `packages/objectql/src/engine-not-null-refusal.test.ts`; + * the doors, on a real driver, in `packages/runtime/src/dispatcher-not-null-refusal-24768.test.ts` + * and `packages/rest/src/data-not-null-field-label-24715.test.ts`. + * + * The driver shapes below are the ones measured on each client (the codes table + * on `notNullViolationSite`): better-sqlite3 through knex, libsql, node-postgres + * and mysql2. sql.js states no code, so its error is the bare text. */ import { describe, it, expect } from 'vitest'; -import { mapDataError, notNullRefusalSite, relabelNotNullRefusal } from './data-error-classification.js'; -import { validationFailure } from './validation-failure.js'; +import { mapDataError, notNullViolationSite } from './data-error-classification.js'; -const driverError = (message: string, code: string | number) => Object.assign(new Error(message), { code }); +const driverError = (message: string, fields: Record) => Object.assign(new Error(message), fields); const SQLITE = driverError( 'update [statement and bound values redacted] - NOT NULL constraint failed: line_item.unit_price', - 'SQLITE_CONSTRAINT_NOTNULL', + { code: 'SQLITE_CONSTRAINT_NOTNULL' }, ); +const LIBSQL = driverError('SQLITE_CONSTRAINT: NOT NULL constraint failed: line_item.unit_price', { + code: 'SQLITE_CONSTRAINT', extendedCode: 'SQLITE_CONSTRAINT_NOTNULL', rawCode: 1299, +}); +const SQLJS = new Error('NOT NULL constraint failed: line_item.unit_price'); const POSTGRES = driverError( 'null value in column "unit_price" of relation "line_item" violates not-null constraint', - '23502', + { code: '23502' }, ); -const POSTGRES_NO_RELATION = driverError('null value in column "unit_price" violates not-null constraint', '23502'); -const MYSQL = driverError("ER_BAD_NULL_ERROR: Column 'unit_price' cannot be null", 'ER_BAD_NULL_ERROR'); +const POSTGRES_NO_RELATION = driverError('null value in column "unit_price" violates not-null constraint', { code: '23502' }); +/** node-postgres keeps the column and the table on fields of their own; a wrapper may cut the text. */ +const POSTGRES_FIELDS_ONLY = driverError('insert failed', { code: '23502', column: 'unit_price', table: 'line_item' }); +const MYSQL = driverError("ER_BAD_NULL_ERROR: Column 'unit_price' cannot be null", { code: 'ER_BAD_NULL_ERROR', errno: 1048 }); -describe('[#24715] notNullRefusalSite — the NOT NULL branch\'s own answer, and no other', () => { +describe('[#24768] notNullViolationSite — the column, and the table where the dialect says it', () => { it.each([ - ['SQLite', SQLITE, { column: 'unit_price', table: 'line_item' }], + ['better-sqlite3', SQLITE, { column: 'unit_price', table: 'line_item' }], + ['libsql', LIBSQL, { column: 'unit_price', table: 'line_item' }], + ['sql.js, text only', SQLJS, { column: 'unit_price', table: 'line_item' }], ['Postgres, naming the relation', POSTGRES, { column: 'unit_price', table: 'line_item' }], ['Postgres, naming none', POSTGRES_NO_RELATION, { column: 'unit_price' }], + ['Postgres, its own fields', POSTGRES_FIELDS_ONLY, { column: 'unit_price', table: 'line_item' }], ['MySQL, which never names the table', MYSQL, { column: 'unit_price' }], - ])('%s: the column the sentence always named, and the table where the dialect says it', (_dialect, error, site) => { - const answer = mapDataError(error, 'line_item'); - expect(answer.body.fields).toEqual([{ field: 'unit_price', code: 'required', message: 'unit_price is required' }]); - expect(notNullRefusalSite(answer)).toEqual(site); + ])('%s', (_driver, error, site) => { + expect(notNullViolationSite(error)).toEqual(site); + }); + + it('one step down a `cause` chain, as pool and query-builder layers wrap', () => { + expect(notNullViolationSite(Object.assign(new Error('write failed'), { cause: SQLITE }))) + .toEqual({ column: 'unit_price', table: 'line_item' }); }); - it('a record validator\'s `required` refusal is the same envelope and is NOT recognised', () => { - const answer = mapDataError( - validationFailure('unit_price is required', [{ field: 'unit_price', code: 'required', message: 'unit_price is required' }]), - 'line_item', + it('a code naming another refusal is not read for NOT NULL text: a bound value can spell anything', () => { + const check = driverError( + "insert into t values ('NOT NULL constraint failed: line_item.unit_price') - CHECK constraint failed: t", + { code: 'SQLITE_CONSTRAINT_CHECK' }, ); - expect(answer).toMatchObject({ status: 400, body: { code: 'VALIDATION_FAILED' } }); - expect(notNullRefusalSite(answer)).toBeUndefined(); + expect(notNullViolationSite(check)).toBeUndefined(); + expect(notNullViolationSite(driverError('Column \'unit_price\' cannot be null', { code: 'ER_DUP_ENTRY', errno: 1062 }))) + .toBeUndefined(); }); - it('a copy of the branch\'s body is not the branch\'s answer', () => { - const answer = mapDataError(SQLITE, 'line_item'); - expect(notNullRefusalSite({ body: { ...answer.body } })).toBeUndefined(); + it('a NOT NULL code whose column cannot be determined, and every other error: undefined', () => { + expect(notNullViolationSite(driverError('insert failed', { code: '23502' }))).toBeUndefined(); + expect(notNullViolationSite(driverError('UNIQUE constraint failed: line_item.email', { code: 'SQLITE_CONSTRAINT_UNIQUE' }))) + .toBeUndefined(); + expect(notNullViolationSite(new Error('connection refused'))).toBeUndefined(); + expect(notNullViolationSite(undefined)).toBeUndefined(); }); }); -describe('[#24715] the classification\'s hint holds for both ways in, and prescribes no declaration', () => { - it('leads with the value, never with `required`, and names drift only as a condition', () => { +describe('[#24715] the classification\'s answer for a column no field declares', () => { + it.each([ + ['SQLite', SQLITE], ['Postgres', POSTGRES], ['MySQL', MYSQL], + ])('%s: the column\'s sentence, as it always was', (_dialect, error) => { + const answer = mapDataError(error, 'line_item'); + expect(answer.status).toBe(400); + expect(answer.body).toMatchObject({ code: 'VALIDATION_FAILED', error: 'unit_price is required', object: 'line_item' }); + expect(answer.body.fields).toEqual([{ field: 'unit_price', code: 'required', message: 'unit_price is required' }]); + }); + + it('the hint leads with the value, never with `required`, and names drift only as a condition', () => { const hint = String(mapDataError(SQLITE, 'line_item').body.hint); const sentences = hint.trim().split(/(?<=\.)\s+/); expect(sentences[0], hint).toMatch(/requires a value/); @@ -66,47 +96,3 @@ describe('[#24715] the classification\'s hint holds for both ways in, and prescr expect(drift[0], hint).toMatch(/^If the object declares neither `required` nor `storage: \{ notNull: true \}`/); }); }); - -describe('[#24715] relabelNotNullRefusal — only the text moves', () => { - const MESSAGE = 'Sales Price is required'; - - it('a field declared `storage.notNull`: the label in both sentences, the declared column\'s hint', () => { - const answer = mapDataError(SQLITE, 'line_item'); - const relabelled = relabelNotNullRefusal(answer, { message: MESSAGE, declaresNotNull: true }); - - expect(relabelled.status).toBe(400); - expect(relabelled.body).toMatchObject({ - error: MESSAGE, - code: 'VALIDATION_FAILED', - fields: [{ field: 'unit_price', code: 'required', message: MESSAGE }], - object: 'line_item', - }); - expect(Object.keys(relabelled.body)).toEqual(Object.keys(answer.body)); - expect(Object.keys((relabelled.body.fields as object[])[0])).toEqual(['field', 'code', 'message']); - const hint = String(relabelled.body.hint); - expect(hint).toContain("'unit_price'"); - expect(hint).toContain('`storage: { notNull: true }`'); - expect(hint).not.toContain('`required`'); - expect(hint).not.toMatch(/drift|os migrate/); - // The classification's own answer is not mutated. - expect(answer.body.error).toBe('unit_price is required'); - }); - - it('a field that does not declare it: the label, and the classification\'s hint, drift remedy included', () => { - const answer = mapDataError(SQLITE, 'line_item'); - const relabelled = relabelNotNullRefusal(answer, { message: MESSAGE, declaresNotNull: false }); - - expect(relabelled.body.error).toBe(MESSAGE); - expect((relabelled.body.fields as any[])[0].message).toBe(MESSAGE); - expect(relabelled.body.hint).toBe(answer.body.hint); - expect(String(relabelled.body.hint)).toMatch(/os migrate/); - }); - - it('any other answer is handed back as it came', () => { - const answer = mapDataError( - validationFailure('unit_price is required', [{ field: 'unit_price', code: 'required', message: 'unit_price is required' }]), - 'line_item', - ); - expect(relabelNotNullRefusal(answer, { message: MESSAGE, declaresNotNull: true })).toBe(answer); - }); -}); diff --git a/packages/types/src/data-error-classification.ts b/packages/types/src/data-error-classification.ts index 61ebd27afc2..1994553ff50 100644 --- a/packages/types/src/data-error-classification.ts +++ b/packages/types/src/data-error-classification.ts @@ -1914,7 +1914,7 @@ function classifyDataError(error: any, object?: string): { status: number; body: }; } - const notNull = notNullViolationSite(raw); + const notNull = notNullSiteInText(raw); if (notNull) { const field = notNull.column; // The metadata required-check (`record-validator`) runs BEFORE the @@ -1936,22 +1936,27 @@ function classifyDataError(error: any, object?: string): { status: number; body: // are the same for both. // // [#24715] …and the sentence names the COLUMN, because the column is - // all the driver's text carries. The caller that holds the object's - // field map names the field by its label instead, and gives a field - // declared `storage: { notNull: true }` its own hint - // ({@link relabelNotNullRefusal}). The hint here therefore prescribes - // nothing that is wrong for either case: it used to add "or declare the - // field `required`", which for a column an engine hook populates is the - // move ADR-0113 D1 / D2 rule out (it puts the demand back on the form). - const body: Record = { - error: `${field} is required`, - code: 'VALIDATION_FAILED', - fields: [{ field, code: 'required', message: `${field} is required` }], - hint: notNullRefusalHint(field), - ...(object ? { object } : {}), + // all the driver's text carries. The hint therefore prescribes nothing + // that is wrong for either case: it used to add "or declare the field + // `required`", which for a column an engine hook populates is the move + // ADR-0113 D1 / D2 rule out (it puts the demand back on the form). + // + // [#24768] A refusal on a field the object DECLARES no longer reaches + // this branch: the engine holds the field map at its write doors, and + // refuses that write as its own `ValidationError`, named by the field's + // label (`notNullViolationSite` below is what it asks). What is left + // here is the column no field declares, where the column is all there + // is to name, and any driver error a non-engine write lets out. + return { + status: 400, + body: { + error: `${field} is required`, + code: 'VALIDATION_FAILED', + fields: [{ field, code: 'required', message: `${field} is required` }], + hint: notNullRefusalHint(field), + ...(object ? { object } : {}), + }, }; - NOT_NULL_REFUSAL_SITES.set(body, notNull); - return { status: 400, body }; } // [#5462] A driver saying "that relation is missing" is an unknown-OBJECT @@ -2065,7 +2070,7 @@ function classifyDataError(error: any, object?: string): { status: number; body: return UNCLASSIFIED_FAULT(); } -// ── The NOT NULL refusal, and the caller that names its field ([#24715]) ───── +// ── The NOT NULL refusal, and the engine that names its field ([#24768]) ───── /** * [#24715] Where a driver's NOT NULL refusal points: the column, and the table @@ -2088,7 +2093,7 @@ export interface NotNullRefusalSite { readonly table?: string; } -function notNullViolationSite(raw: string): NotNullRefusalSite | undefined { +function notNullSiteInText(raw: string): NotNullRefusalSite | undefined { const sqlite = /not null constraint failed:\s*(\S*?)\.([a-z0-9_]+)/i.exec(raw); if (sqlite) return sqlite[1] ? { column: sqlite[2], table: sqlite[1] } : { column: sqlite[2] }; const postgres = /null value in column\s+["'`]([a-z0-9_]+)["'`](?:\s+of\s+relation\s+["'`]([^"'`]+)["'`])?/i.exec(raw); @@ -2098,19 +2103,90 @@ function notNullViolationSite(raw: string): NotNullRefusalSite | undefined { } /** - * [#24715] The bodies the NOT NULL branch of {@link classifyDataError} built, - * keyed to the site the driver named. - * - * Why a brand and not a key on the body: the body IS the wire answer, and the - * caller that relabels it ({@link relabelNotNullRefusal}) must recognise THIS - * branch's answer exactly — never a lookalike with the same `code` and - * `fields[0].code` built by another branch (a record validator's own - * `required` refusal is one) — without the wire growing a key to say so. A - * body copied on the way out (the declared-`userMessage` wrapper builds a new - * one, and a driver error carries no such mark) is simply not recognised, and - * keeps the column's sentence: the safe way to be wrong. + * [#24768] The codes a driver error states a NOT NULL refusal by, per dialect, + * as measured: better-sqlite3 `code: 'SQLITE_CONSTRAINT_NOTNULL'`; libsql + * `code: 'SQLITE_CONSTRAINT'` with `extendedCode: 'SQLITE_CONSTRAINT_NOTNULL'` + * (`rawCode: 1299`), and the same code again on its `cause`; node-postgres + * SQLSTATE `23502`; mysql2 `code: 'ER_BAD_NULL_ERROR'`, `errno: 1048`. sql.js + * (`driver-sqlite-wasm`) states no code at all, only the text. */ -const NOT_NULL_REFUSAL_SITES = new WeakMap(); +const NOT_NULL_CODES = new Set(['SQLITE_CONSTRAINT_NOTNULL', '23502', 'ER_BAD_NULL_ERROR']); +const NOT_NULL_ERRNOS = new Set([1048]); +/** SQLite's extended result code for `SQLITE_CONSTRAINT_NOTNULL`, as libsql states it on `rawCode`. */ +const SQLITE_CONSTRAINT_NOTNULL_RAW = 1299; +/** The PRIMARY code libsql states beside the extended one: a constraint, not WHICH constraint. */ +const SQLITE_CONSTRAINT_PRIMARY = 'SQLITE_CONSTRAINT'; + +/** How far to follow an `error.cause` chain — drivers wrap, but not deeply. */ +const NOT_NULL_MAX_CAUSE_DEPTH = 4; + +/** + * [#24768] The site of a driver's NOT NULL refusal — the column it names, and + * the table when its dialect names one — or `undefined` when `error` is not one. + * + * The engine asks this at its write doors' driver exits, where it holds the + * object's field map, and refuses the write as its own `ValidationError` for the + * field that column is, named by its label (`@objectstack/objectql`, + * `not-null-refusal-error.ts`). {@link mapDataError} reads the same text below, + * for a refusal nothing translated: a column no field declares. + * + * Reads, in turn: the error's codes (table above), then its text, then one step + * down the `cause` chain, bounded. A code that names NOT NULL vouches for the + * text, and Postgres' own `column` / `table` fields answer when the text does + * not carry them. An error that states a code naming ANOTHER refusal (a CHECK or + * a foreign key) is not read for NOT NULL text at all: its message may quote the + * statement, and a bound value can spell anything. An error with no code is read + * by its text alone, which is all sql.js gives. A plain string is read as text. + * + * **Unrecognised is `undefined`**, and so is a refusal whose column cannot be + * determined: the caller then has no field to name and keeps the answer it had. + */ +export function notNullViolationSite(error: unknown): NotNullRefusalSite | undefined { + return findNotNullViolationSite(error, 0); +} + +function findNotNullViolationSite(error: unknown, depth: number): NotNullRefusalSite | undefined { + if (error === null || error === undefined || depth > NOT_NULL_MAX_CAUSE_DEPTH) return undefined; + if (typeof error === 'string') return notNullSiteInText(error); + if (typeof error !== 'object') return undefined; + + const err = error as { + code?: unknown; extendedCode?: unknown; rawCode?: unknown; errno?: unknown; + message?: unknown; column?: unknown; table?: unknown; cause?: unknown; + }; + const stated = statedNotNull(err); + if (stated !== false) { + const fromText = typeof err.message === 'string' ? notNullSiteInText(err.message) : undefined; + if (fromText) return fromText; + // node-postgres keeps the column and the table on fields of their own. + if (stated === true && typeof err.column === 'string' && err.column !== '') { + return typeof err.table === 'string' && err.table !== '' + ? { column: err.column, table: err.table } + : { column: err.column }; + } + } + return findNotNullViolationSite(err.cause, depth + 1); +} + +/** + * Whether `err`'s codes state a NOT NULL refusal (`true`), state some other + * refusal (`false`), or state none that decides it (`undefined`: no code, or + * libsql's bare `SQLITE_CONSTRAINT` without its extended code). + */ +function statedNotNull(err: { + code?: unknown; extendedCode?: unknown; rawCode?: unknown; errno?: unknown; +}): boolean | undefined { + if (typeof err.extendedCode === 'string') return NOT_NULL_CODES.has(err.extendedCode); + if (typeof err.rawCode === 'number') return err.rawCode === SQLITE_CONSTRAINT_NOTNULL_RAW; + if (typeof err.code === 'string' && err.code !== '') { + if (NOT_NULL_CODES.has(err.code)) return true; + return err.code === SQLITE_CONSTRAINT_PRIMARY ? undefined : false; + } + // A numeric `code` is MySQL's errno wearing the other field's name. + if (typeof err.code === 'number') return NOT_NULL_ERRNOS.has(err.code); + if (typeof err.errno === 'number') return NOT_NULL_ERRNOS.has(err.errno); + return undefined; +} /** * The NOT NULL branch's hint, for a column whose field map this branch never @@ -2127,75 +2203,6 @@ function notNullRefusalHint(field: string): string { + `(or reset the dev database).`; } -/** - * The hint for a field the object declares `storage: { notNull: true }` — set - * by the caller that holds the field map, the one that can tell this case from - * drift. Nothing has drifted and nothing is missing from the metadata, so it - * names neither `os migrate` nor `required`: per ADR-0113 D1 / D2 the column - * constraint is deliberately not the write contract, and a field a hook or a - * default populates is not one the form should demand. - */ -function declaredNotNullHint(field: string): string { - return `The field '${field}' is declared \`storage: { notNull: true }\`, so its database column requires a value: ` - + `send one, or omit the field when a default value or a hook populates it ` - + `(an update that omits it keeps the stored value).`; -} - -/** - * [#24715] The site of the NOT NULL refusal `answer` is — when, and only when, - * `answer` is the one {@link mapDataError} built for a driver's NOT NULL text. - * `undefined` for every other answer. - * - * Asked by the caller that holds the object's field map before it relabels the - * refusal ({@link relabelNotNullRefusal}). The `table` lets that caller decline - * a refusal the driver attributes to a table other than the object it holds: - * a hook's write to another object fails inside the request, and its column is - * not the requested object's field. - */ -export function notNullRefusalSite(answer: { body: Record }): NotNullRefusalSite | undefined { - return NOT_NULL_REFUSAL_SITES.get(answer.body); -} - -/** - * [#24715] Name the field of a NOT NULL refusal the way the caller's field map - * names it. - * - * The classification above sees the driver's text and the object's name, never - * the field map, so its sentence names the column (`unit_price is required`) - * and its hint must hold for every field. The caller that holds the field map - * hands in what only it can know: - * - * - `message` — the field's `required` sentence, rendered the way the record - * validator renders its own (the shared catalog, the caller's locale, the - * field's label). It replaces `error` and `fields[0].message`. - * - `declaresNotNull` — whether the field declares `storage: { notNull: true }`. - * When it does, the hint is the declared column's, which prescribes neither - * `required` nor `os migrate`. When it does not, the classification's hint - * stands, drift remedy included. - * - * Everything else is byte-for-byte the classification's: `status`, `code`, - * `fields[0].field` (the column key a client maps to its input), `fields[0].code`, - * `object`, and the key set. An answer that is not the NOT NULL branch's is - * handed back untouched. - */ -export function relabelNotNullRefusal( - answer: { status: number; body: Record }, - field: { message: string; declaresNotNull: boolean }, -): { status: number; body: Record } { - const site = notNullRefusalSite(answer); - if (!site) return answer; - const [first] = answer.body.fields as Array>; - return { - status: answer.status, - body: { - ...answer.body, - error: field.message, - fields: [{ ...first, message: field.message }], - ...(field.declaresNotNull ? { hint: declaredNotNullHint(site.column) } : {}), - }, - }; -} - // The members above that are not `export`ed where they are declared, but that // `@objectstack/rest`'s emission half (`error-response.ts`) composes. Listed // here rather than by editing their declarations, so the moved block stays