diff --git a/.changeset/20308-blank-typed-value-null.md b/.changeset/20308-blank-typed-value-null.md new file mode 100644 index 00000000000..7366a072bd8 --- /dev/null +++ b/.changeset/20308-blank-typed-value-null.md @@ -0,0 +1,78 @@ +--- +"@objectstack/objectql": minor +--- + +fix(objectql)!: a cleared number, boolean, date, datetime or time field stores `null` on every backend, and a `progress` field refuses a non-numeric value (#20308) + +Clause-②: no (narrowing) + +**BREAKING** — shipped as `minor` under the launch-window convention +(`check-changeset-no-major` refuses `major` until GA; breaking-ness is carried by +this banner and the ADR-0087 disposition below, never by the level). The one +narrowing: a non-numeric string written to a `progress` field is now refused +with `invalid_number` on memory and SQLite, where it used to be stored. + +## What was wrong + +The record validator reads a blank string — `''`, or whitespace only — as +"missing", and returns before any type check. Nothing rewrote the value, so the +driver received the blank exactly as sent. The same clear of one field therefore +had three outcomes: + +- **memory and SQLite** stored `''` in a number, currency, percent, rating, + slider, progress, summary, boolean, toggle, date, datetime or time column, on + every write door (create, update, batch, `createMany`, `updateMany`). On SQLite + a stored `''` on a boolean then read back as `false`. +- **PostgreSQL** refused the statement (`invalid input syntax for type + numeric` / `boolean` / `date`), which REST answered as `500 DATABASE_ERROR` — + or as a failed row with `INTERNAL_ERROR` on the batch doors. + +objectui's edit form sends a cleared date, datetime or time box as `''`, so this +is the ordinary "clear the field and save" gesture. + +Separately, `progress` had no type check at all: a non-numeric string such as +`'abc'` was stored verbatim on memory and SQLite, and failed at the driver as a +`500` on PostgreSQL. + +## What changes + +- **The write door reads a blank on a non-string-typed column as `null`.** Every + field whose declared type is in the spec's `NON_TEXT_STORED_VALUE_TYPES` (the + numeric types including `progress` and `summary`, `boolean`, `toggle`, + `date`, `datetime`, `time`) has a blank string replaced by `null`. It happens + at the start of `ObjectQL.insert()` and `ObjectQL.update()`, before the + middleware, the hooks, the defaults and validation read the payload, and at + the same point in `ObjectQL.validate()` (the dry run). Every REST, batch and + import door writes through those methods. The caller's own objects are never + mutated. +- **What that means for a write:** the column stores `null` on every backend, + and PostgreSQL no longer refuses the request. A blank on a `required` field is + refused with `required`, exactly as `null` is. On create, a blank takes the + field's `defaultValue` exactly as `null` does, so a blank on a required field + that declares a `defaultValue` is now accepted with the default. +- **String-stored columns are untouched.** A text, lookup or select `''` is still + stored as `''`. +- **`progress` joins the numeric type check.** A non-numeric string on it is + refused with `invalid_number`, as on `number`. No `min`, `max` or `scale` is + newly enforced on it. This is the narrowing above. +- **`summary` is exempt from that type check.** It is in the spec's + `COMPUTED_VALUE_TYPES` ("never client-written; shape is producer-owned"), so + the roll-up producer decides its value's shape. A `max` or `min` roll-up over a + date, datetime or time child field keeps recomputing on memory and SQLite as + before, and a non-numeric value written to a `summary` is not judged by this + check. A blank on a `summary` still becomes `null`. + +## Rows already stored + +This fixes new writes only. Rows written earlier on SQLite (and on memory, +MongoDB or libSQL) may still hold `''` in such a column; on SQLite a boolean +holding `''` reads back as `false`, and one holding whitespace as `true`. +PostgreSQL never stored one. To repair a SQLite table, run this once per +non-string-typed column (a field of one of the types listed above): + +```sql +UPDATE "" SET "" = NULL + WHERE typeof("") = 'text' AND trim("", ' ' || char(9) || char(10) || char(13)) = ''; +``` + + diff --git a/packages/objectql/src/engine-blank-typed-value-door.test.ts b/packages/objectql/src/engine-blank-typed-value-door.test.ts new file mode 100644 index 00000000000..61aeed921be --- /dev/null +++ b/packages/objectql/src/engine-blank-typed-value-door.test.ts @@ -0,0 +1,242 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #20308 — a BLANK on a non-string-typed column reaches the driver as `null`, + * on every engine write door. + * + * ## The defect + * + * `validateOne` reads `''` (and whitespace) as missing and returns before its + * type checks, and nothing rewrote the value, so the driver received it as + * sent. Measured on `origin/main` de091b50e6 through the real engine: memory + * and SQLite stored `''` in number, currency, percent, rating, slider, + * progress, summary, boolean, toggle, date, datetime and time columns on every + * door (insert, insert[], insertMany, update by id, update by predicate), and + * PostgreSQL refused the statement (`22P02` / `22007`; `500 DATABASE_ERROR` at + * REST). One clear, three outcomes. + * + * ## What this file pins + * + * What the DRIVER receives, on each door — the one thing every backend then + * stores (memory and MongoDB store it verbatim; the SQL physical column is + * `packages/rest/src/rest-data-blank-typed-value.test.ts`). And the four + * interactions the door's placement decides: + * + * - `required` refuses a blank exactly as it refuses `null` (same code); + * - on insert a blank takes a declared `defaultValue` exactly as `null` does + * (#2706) — the door runs BEFORE the defaults; + * - a `readonlyWhen` lock still holds against a caller's blank — the door + * runs BEFORE the caller-value snapshot the lock judges against; + * - the dry run (`validate()`) agrees with the write. + * + * The string-stored controls (`text`, `lookup`, `select`) keep their `''` — + * the spec's `str_empty` side of the line. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { NON_TEXT_STORED_VALUE_TYPES } from '@objectstack/spec/data'; +import { ObjectQL } from './engine.js'; +import { ValidationError } from './validation/record-validator.js'; + +const TYPED = [...NON_TEXT_STORED_VALUE_TYPES]; +const f = (t: string) => `f_${t}`; + +interface Call { fn: string; data: Record } + +function makeStubDriver() { + const calls: Call[] = []; + const rows = new Map>(); + const idOf = (q: any): string | undefined => { + const w = q?.where ?? q?.filter ?? q; + const id = w?.id; + return typeof id === 'string' ? id : undefined; + }; + let n = 0; + const driver: any = { + name: 'stub', version: '0.0.0', supports: {}, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, + // Not a query-honouring `find`: nothing here reads rows back through it + // (the by-id prior read is `findOne`), so it answers the whole store. + async find() { return [...rows.values()]; }, + async findOne(_o: string, q: any) { + const id = idOf(q); + return (id ? rows.get(id) : rows.values().next().value) ?? null; + }, + async count() { return rows.size; }, + async create(_o: string, data: Record) { + calls.push({ fn: 'create', data: { ...data } }); + const row = { ...data, id: (data.id as string) ?? `r${++n}` }; + rows.set(row.id as string, row); + return row; + }, + async bulkCreate(_o: string, list: Record[]) { + calls.push({ fn: 'bulkCreate', data: { rows: list.map((r) => ({ ...r })) } }); + const out: Record[] = []; + for (const r of list) { + const row = { ...r, id: (r.id as string) ?? `r${++n}` }; + rows.set(row.id as string, row); + out.push(row); + } + return out; + }, + async update(_o: string, id: string, data: Record) { + calls.push({ fn: 'update', data: { ...data } }); + const row = { ...(rows.get(id) ?? {}), ...data, id }; + rows.set(id, row); + return row; + }, + async updateMany(_o: string, _ast: unknown, data: Record) { + calls.push({ fn: 'updateMany', data: { ...data } }); + return rows.size; + }, + async upsert(o: string, data: Record) { return this.create(o, data); }, + async delete() { return true; }, + async bulkUpdate() { return []; }, async bulkDelete() {}, + async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + }; + return { driver, calls, rows }; +} + +const OBJ = { + name: 'blank_door', + label: 'Blank door', + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true }, + c_text: { name: 'c_text', type: 'text' as const }, + c_lookup: { name: 'c_lookup', type: 'lookup' as const, reference: 'blank_door' }, + c_select: { name: 'c_select', type: 'select' as const, options: [{ value: 'a', label: 'A' }] }, + ...Object.fromEntries(TYPED.map((t) => [f(t), { name: f(t), type: t }])), + }, +}; + +const REQ = { + name: 'blank_req', + label: 'Blank required', + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true }, + status: { name: 'status', type: 'text' as const }, + r_number: { name: 'r_number', type: 'number' as const, required: true }, + rd_number: { name: 'rd_number', type: 'number' as const, required: true, defaultValue: 5 }, + od_date: { name: 'od_date', type: 'date' as const, defaultValue: '2020-01-01' }, + locked_amount: { name: 'locked_amount', type: 'number' as const, readonlyWhen: "record.status == 'closed'" }, + }, +}; + +const VALID: Record = { + f_number: 0, f_currency: 9.5, f_percent: 0.25, f_rating: 3, f_slider: 10, f_progress: 50, f_summary: 7, + f_boolean: false, f_toggle: true, f_date: '2026-09-27', f_datetime: '2026-09-27T10:00:00.000Z', f_time: '14:30:00', +}; + +const blankTyped = (v: string) => Object.fromEntries(TYPED.map((t) => [f(t), v])); +const STR_BLANK = { c_text: '', c_lookup: '', c_select: '' }; + +function expectTypedNull(data: Record) { + for (const t of TYPED) expect(data[f(t)], t).toBeNull(); +} + +describe('engine write doors: a blank on a non-string-typed column reaches the driver as null (#20308)', () => { + let engine: ObjectQL; + let stub: ReturnType; + + beforeEach(async () => { + stub = makeStubDriver(); + engine = new ObjectQL(); + engine.registerDriver(stub.driver, true); + await engine.init(); + engine.registry.registerObject(OBJ as any); + engine.registry.registerObject(REQ as any); + }); + + const last = (fn: string) => [...stub.calls].reverse().find((c) => c.fn === fn)!.data; + + it.each([[''], [' ']])('insert, one row: %j → null; the string-stored controls keep their blank', async (blank) => { + const payload: Record = { id: 'a', ...blankTyped(blank), ...STR_BLANK }; + await engine.insert('blank_door', payload); + const sent = last('create'); + expectTypedNull(sent); + expect([sent.c_text, sent.c_lookup, sent.c_select]).toEqual(['', '', '']); + // The caller's own object is never rewritten. + expect(payload.f_number).toBe(blank); + }); + + it('insert, an array of rows and insertMany: every row', async () => { + await engine.insert('blank_door', [{ id: 'b1', ...blankTyped('') }, { id: 'b2', ...blankTyped('') }]); + const sent = [...stub.calls].filter((c) => c.fn === 'create' || c.fn === 'bulkCreate'); + const written = sent.flatMap((c) => (c.fn === 'bulkCreate' ? (c.data.rows as Record[]) : [c.data])); + expect(written.map((r) => r.id)).toEqual(['b1', 'b2']); + for (const r of written) expectTypedNull(r); + + stub.calls.length = 0; + const outcomes = await engine.insertMany('blank_door', [{ id: 'm1', ...blankTyped('') }]); + expect(outcomes.map((o) => o.ok)).toEqual([true]); + const many = [...stub.calls].filter((c) => c.fn === 'create' || c.fn === 'bulkCreate') + .flatMap((c) => (c.fn === 'bulkCreate' ? (c.data.rows as Record[]) : [c.data])); + expect(many).toHaveLength(1); + expectTypedNull(many[0]!); + }); + + it('update by id and update by predicate', async () => { + await engine.insert('blank_door', { id: 'u1', ...VALID }); + await engine.update('blank_door', { id: 'u1', ...blankTyped(''), ...STR_BLANK }); + const byId = last('update'); + expectTypedNull(byId); + expect([byId.c_text, byId.c_lookup, byId.c_select]).toEqual(['', '', '']); + + await engine.update('blank_door', blankTyped(''), { where: { id: { $in: ['u1'] } }, multi: true } as any); + expectTypedNull(last('updateMany')); + }); + + it('a valid value on every typed column — falsy ones included — is unchanged', async () => { + await engine.insert('blank_door', { id: 'v1', ...VALID }); + const sent = last('create'); + for (const [k, v] of Object.entries(VALID)) expect(sent[k], k).toStrictEqual(v); + }); + + it('required refuses a blank exactly as it refuses null — insert and update', async () => { + const codes = async (fn: () => Promise) => { + try { await fn(); return null; } catch (e) { + expect(e).toBeInstanceOf(ValidationError); + return { code: (e as ValidationError).code, fields: (e as ValidationError).fields.map((x) => [x.field, x.code]) }; + } + }; + const blank = await codes(() => engine.insert('blank_req', { id: 'q1', r_number: '' })); + const nul = await codes(() => engine.insert('blank_req', { id: 'q2', r_number: null })); + expect(blank).toEqual({ code: 'VALIDATION_FAILED', fields: [['r_number', 'required']] }); + expect(blank).toEqual(nul); + + await engine.insert('blank_req', { id: 'q3', r_number: 1 }); + const cleared = await codes(() => engine.update('blank_req', { id: 'q3', r_number: '' })); + expect(cleared).toEqual({ code: 'VALIDATION_FAILED', fields: [['r_number', 'required']] }); + }); + + it('on insert a blank takes the declared defaultValue exactly as null does — required or not', async () => { + await engine.insert('blank_req', { id: 'd1', r_number: 1, rd_number: '', od_date: '' }); + const blank = last('create'); + await engine.insert('blank_req', { id: 'd2', r_number: 1, rd_number: null, od_date: null }); + const nul = last('create'); + expect([blank.rd_number, blank.od_date]).toEqual([5, '2020-01-01']); + expect([nul.rd_number, nul.od_date]).toEqual([5, '2020-01-01']); + }); + + it('the dry run agrees with the write on the same blank', async () => { + const preview = await engine.validate('blank_req', { id: 'p1', r_number: 1, rd_number: '' }); + expect(preview.valid).toBe(true); + const refused = await engine.validate('blank_req', { id: 'p2', r_number: '' }); + expect(refused.valid).toBe(false); + expect(refused.results?.[0]?.errors.map((e: any) => [e.field, e.code])).toEqual([['r_number', 'required']]); + }); + + it('a readonlyWhen lock still holds against a caller blank — the snapshot it judges is the normalised one', async () => { + await engine.insert('blank_req', { id: 'l1', r_number: 1, status: 'closed', locked_amount: 10 }); + await engine.update('blank_req', { id: 'l1', locked_amount: '' }); + const sent = last('update'); + expect('locked_amount' in sent).toBe(false); + expect(stub.rows.get('l1')?.locked_amount).toBe(10); + + // Unlocked, the same blank clears the column to null. + await engine.insert('blank_req', { id: 'l2', r_number: 1, status: 'open', locked_amount: 10 }); + await engine.update('blank_req', { id: 'l2', locked_amount: '' }); + expect(last('update').locked_amount).toBeNull(); + }); +}); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 9bb4a1fd172..0cc05ee8a8f 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -216,7 +216,7 @@ import { // `resolveMetadataItemName` below for why this registrar lost its fourth copy. import { deriveViewContainerObject } from '@objectstack/metadata/view-container'; import { bindHooksToEngine } from './hook-binder.js'; -import { validateRecord, normalizeMultiValueFields, coerceBooleanFields, ValidationError, buildFieldError, resolveFieldLabel, valueShapePostureSetByEnv, mediaPostureSetByEnv, isScannableValueShapeField, valueShapeStrictEffective, mediaStrictEffective } from './validation/record-validator.js'; +import { validateRecord, normalizeMultiValueFields, normalizeBlankTypedValues, coerceBooleanFields, ValidationError, buildFieldError, resolveFieldLabel, valueShapePostureSetByEnv, mediaPostureSetByEnv, isScannableValueShapeField, valueShapeStrictEffective, mediaStrictEffective } from './validation/record-validator.js'; import type { AdmittedValueShapeViolation, AdmittedValueShapeViolationSink } from './validation/record-validator.js'; import type { RelatedFieldBinding, RelatedRecordBinding } from './validation/rule-validator.js'; import { collectPredicateRelationships, evaluateValidationRules, optionVisibilityReadsPermissions, readsPermissionPredicate, referentialClearBinding, needsPriorRecord, stripReadonlyWhenFields, stripReadonlyWhenFieldsMulti, hasReadonlyWhenInPayload, hasParentScopedReadonlyWhenInPayload, hasParentScopedRequiredWhen, stripReadonlyFields, stripRuntimeOwnedFields, staticReadonlyInsertSubject, preserveAuditIgnoredOnInsertWarning } from './validation/rule-validator.js'; @@ -11754,7 +11754,11 @@ export class ObjectQL implements IObjectQLEngine { // those arms, and the families it does not carry). // `update()` deliberately does not default (#2706: a PATCH's explicit // `null` means "clear it"), so neither does an `update`-mode preview. - const rawRows = Array.isArray(data) ? data : [data]; + // [#20308] The write doors read a blank on a non-string-typed column as + // `null` before anything else; the preview does the same at the same point, + // or a blank on a required field with a `defaultValue` would preview + // `required` while the write takes the default. + const rawRows = normalizeBlankTypedValues(schemaForValidation, Array.isArray(data) ? data : [data]); const nowSnapshot = new Date(); // [#20082] The preview's ONE permission resolution, shared by its CEL // defaults and its option gates below, exactly as the write shares one. A @@ -11920,6 +11924,14 @@ export class ObjectQL implements IObjectQLEngine { // hook, default or validation runs, so a refusal costs nothing. this.enforceTransactionOrigin(object, driver, 'insert'); + // [#20308] A blank on a non-string-typed column is `null` from here on — + // before the middleware, the caller snapshot, the defaults, the hooks and + // validation read the payload, so all of them see one image (a blank then + // takes a `defaultValue` exactly as `null` does). See + // `normalizeBlankTypedValues` for the scope; it never mutates the caller's + // rows. + data = normalizeBlankTypedValues(this._registry.getObject(object), data); + const opCtx: OperationContext = { object, operation: 'insert', @@ -12958,6 +12970,13 @@ export class ObjectQL implements IObjectQLEngine { // itself. Resolve first, then extract. options = this.withResolvedWhere(options); + // [#20308] The insert door's rule, same place: a blank on a + // non-string-typed column is `null` before the middleware, the + // caller-value snapshot (`suppliedValues`), the hooks, the read-only + // strips and validation read the payload — so a `readonlyWhen` lock judges + // the value it snapshotted. See `normalizeBlankTypedValues`. + data = normalizeBlankTypedValues(this._registry.getObject(object), data); + // 1. Extract ID from data or where if it's a single update by ID. // Only a SCALAR `where.id` means "update one row by primary key". An // operator object ({ $in: [...] }, { $ne: ... }, …) is a multi-row diff --git a/packages/objectql/src/validation/record-validator.blank-typed-value.test.ts b/packages/objectql/src/validation/record-validator.blank-typed-value.test.ts new file mode 100644 index 00000000000..d053d2da244 --- /dev/null +++ b/packages/objectql/src/validation/record-validator.blank-typed-value.test.ts @@ -0,0 +1,163 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #20308 — the write door's reading of a BLANK string, and the numeric type + * door, at the validator module. + * + * `normalizeBlankTypedValues` rewrites `''` / whitespace on a non-string-typed + * column to `null` and leaves everything else alone. Both populations are read + * from the spec's own sets, so a type joining `NON_TEXT_STORED_VALUE_TYPES`, + * `NUMERIC_VALUE_TYPES` or `COMPUTED_VALUE_TYPES` tomorrow is covered here + * without an edit — and a type leaving the string side would turn the control + * half red. + * + * The engine-level half (what reaches the driver on every door) is + * `../engine-blank-typed-value-door.test.ts`; the physical-column half, through + * REST on SQLite, is `packages/rest/src/rest-data-blank-typed-value.test.ts`. + */ + +import { describe, it, expect } from 'vitest'; +import { + COMPUTED_VALUE_TYPES, + NON_TEXT_STORED_VALUE_TYPES, + NUMERIC_VALUE_TYPES, + STRING_VALUE_TYPES, +} from '@objectstack/spec/data'; +import { normalizeBlankTypedValues, validateRecord, ValidationError } from './record-validator.js'; + +const typed = [...NON_TEXT_STORED_VALUE_TYPES]; +// The string-stored controls: the whole string class, plus the other types +// whose stored value is a string (`str_empty`'s side of the line). +const stringStored = [...STRING_VALUE_TYPES, 'select', 'radio', 'lookup', 'master_detail', 'user', 'autonumber']; + +function schemaOf(types: readonly string[]) { + const fields: Record = {}; + for (const t of types) { + fields[`f_${t}`] = { + name: `f_${t}`, + type: t, + ...(t === 'lookup' || t === 'master_detail' ? { reference: 'other' } : {}), + ...(t === 'select' || t === 'radio' ? { options: [{ value: 'a', label: 'A' }] } : {}), + }; + } + return { fields } as any; +} + +function rowOf(types: readonly string[], value: unknown) { + return Object.fromEntries(types.map((t) => [`f_${t}`, value])); +} + +describe('normalizeBlankTypedValues (#20308)', () => { + it('the population is the spec set, and it is the one the card names', () => { + // A control on the set itself: if it ever emptied, every case below would + // pass over nothing. + expect(typed.sort()).toEqual( + ['boolean', 'currency', 'date', 'datetime', 'number', 'percent', 'progress', 'rating', 'slider', 'summary', 'time', 'toggle'], + ); + for (const t of stringStored) expect(NON_TEXT_STORED_VALUE_TYPES.has(t)).toBe(false); + }); + + it.each([[''], [' '], ['\t\n']])('a blank %j on every non-string-typed column becomes null', (blank) => { + const out = normalizeBlankTypedValues(schemaOf(typed), rowOf(typed, blank)) as Record; + for (const t of typed) expect(out[`f_${t}`], t).toBeNull(); + }); + + it("a string-stored column's blank is untouched — the str_empty side", () => { + for (const blank of ['', ' ']) { + const row = rowOf(stringStored, blank); + const out = normalizeBlankTypedValues(schemaOf(stringStored), row); + expect(out).toBe(row); + for (const t of stringStored) expect((out as Record)[`f_${t}`], t).toBe(blank); + } + }); + + it('a non-blank value of any kind passes through untouched — falsy is not blank', () => { + for (const v of [null, undefined, 0, false, 'abc', ' x ', '0', 'false', '2026-09-27', 42, true]) { + const row = rowOf(typed, v); + expect(normalizeBlankTypedValues(schemaOf(typed), row), String(v)).toBe(row); + } + }); + + it('never mutates the caller: a copy comes back only when something changed', () => { + const schema = schemaOf(['number', 'text']); + const row = { f_number: '', f_text: '', extra: 1 }; + const out = normalizeBlankTypedValues(schema, row) as Record; + expect(out).not.toBe(row); + expect(row).toEqual({ f_number: '', f_text: '', extra: 1 }); + expect(out).toEqual({ f_number: null, f_text: '', extra: 1 }); + }); + + it('an array: rows copied only where they changed, the array only when a row did', () => { + const schema = schemaOf(['date', 'text']); + const clean = { f_date: '2026-01-01' }; + const blank = { f_date: '' }; + const rows = [clean, blank]; + const out = normalizeBlankTypedValues(schema, rows) as Array>; + expect(out).not.toBe(rows); + expect(out[0]).toBe(clean); + expect(out[1]).toEqual({ f_date: null }); + expect(rows[1]).toBe(blank); + expect(blank.f_date).toBe(''); + const untouched = [clean, { f_text: '' }]; + expect(normalizeBlankTypedValues(schema, untouched)).toBe(untouched); + }); + + it('an undeclared key, an inherited name and a schema without fields are left alone', () => { + const schema = schemaOf(['number']); + const row = { nope: '', constructor: '' }; + expect(normalizeBlankTypedValues(schema, row)).toBe(row); + expect(normalizeBlankTypedValues({} as any, { f_number: '' })).toEqual({ f_number: '' }); + expect(normalizeBlankTypedValues(null, { f_number: '' })).toEqual({ f_number: '' }); + }); +}); + +// The door the number branch reads: the numeric class minus the server-computed +// class (seat ruling on #20308 — `summary` is producer-owned). +const typeChecked = [...NUMERIC_VALUE_TYPES].filter((t) => !COMPUTED_VALUE_TYPES.has(t)); +const computedNumeric = [...NUMERIC_VALUE_TYPES].filter((t) => COMPUTED_VALUE_TYPES.has(t)); + +describe('the numeric type door is NUMERIC_VALUE_TYPES minus COMPUTED_VALUE_TYPES (#20308)', () => { + function refusal(type: string, value: unknown) { + try { + validateRecord(schemaOf([type]), { [`f_${type}`]: value }, 'insert'); + return null; + } catch (e) { + expect(e).toBeInstanceOf(ValidationError); + return e as ValidationError; + } + } + + it('the two populations are the ones the ruling names — progress judged, summary exempt', () => { + // A control on the sets themselves: if either emptied, the cases below + // would pass over nothing. + expect(typeChecked.sort()).toEqual(['currency', 'number', 'percent', 'progress', 'rating', 'slider']); + expect(computedNumeric).toEqual(['summary']); + }); + + it.each(typeChecked)('%s refuses a non-numeric string with invalid_number', (type) => { + const e = refusal(type, 'abc'); + expect(e?.code).toBe('VALIDATION_FAILED'); + expect(e?.fields.map((f) => [f.field, f.code])).toEqual([[`f_${type}`, 'invalid_number']]); + }); + + it.each([...NUMERIC_VALUE_TYPES])('%s still accepts a finite number', (type) => { + expect(refusal(type, 7)).toBeNull(); + }); + + it.each(computedNumeric)('%s is exempt: its shape is the producer\'s (COMPUTED_VALUE_TYPES), not this check\'s', (type) => { + // What the roll-up recompute writes — a date string for a `max` over a + // temporal child field — is not refused here. (A blank still becomes null + // at the door; that is `normalizeBlankTypedValues`, above.) + expect(refusal(type, '2026-01-05')).toBeNull(); + expect(refusal(type, 'abc')).toBeNull(); + }); + + it('progress takes the type check only, and summary none — no bound or scale is newly enforced', () => { + // The boundary the branch states: a declared `max` / `scale` on these two + // was never enforced, and this change does not start (a separate decision). + for (const type of ['progress', 'summary']) { + const fields = { [`f_${type}`]: { name: `f_${type}`, type, max: 100, scale: 0 } } as any; + expect(() => validateRecord({ fields }, { [`f_${type}`]: 150.5 }, 'insert'), type).not.toThrow(); + } + }); +}); diff --git a/packages/objectql/src/validation/record-validator.ts b/packages/objectql/src/validation/record-validator.ts index 8f49d1cd14b..ea2e47c72f0 100644 --- a/packages/objectql/src/validation/record-validator.ts +++ b/packages/objectql/src/validation/record-validator.ts @@ -65,6 +65,9 @@ import { REFERENCE_VALUE_TYPES, FILE_REFERENCE_TYPES, STRUCTURED_JSON_TYPES, + NUMERIC_VALUE_TYPES, + COMPUTED_VALUE_TYPES, + NON_TEXT_STORED_VALUE_TYPES, percentScaleOf, } from '@objectstack/spec/data'; import type { FieldErrorCode } from '@objectstack/spec/api'; @@ -515,6 +518,84 @@ export function normalizeMultiValueFields( } } +/** + * [#20308] A BLANK string on a non-string-typed column is that column's typed + * blank, `null` — the write door's one reading of it. + * + * "Blank" is exactly what {@link isMissing} calls missing: `''` or a string of + * whitespace. "Non-string-typed" is the spec's `NON_TEXT_STORED_VALUE_TYPES` — + * the numeric class (`progress` and `summary` included), the boolean class, + * `date`, `datetime` and `time`. Read as the constant, never re-listed. + * + * ## Why the door has to say it + * + * The validator has always READ a blank as missing: `validateOne` returns on + * `isMissing` before any type check. Nothing WROTE it that way, so the blank + * went on to the driver exactly as sent — memory and SQLite stored `''` in a + * number, boolean, date, datetime or time column, and PostgreSQL refused the + * statement (`22P02` / `22007`, a `500` at REST). One clear of one field, three + * outcomes; objectui's date and time boxes send `''` when cleared. This makes + * storage agree with the platform's own reading. + * + * ## What it does NOT touch + * + * ⛔ Every string-stored column — text, lookup ids, select values. There `''` + * is a value, and the spec's value round-trip fixture pins it (`str_empty`: + * an empty string must not become null). What "empty" means for those types is + * a separate question, not answered here. ⛔ A non-blank value, and every + * non-string value, of any type: `null`, `0`, `false`, `'abc'` pass through + * untouched, so every type check and refusal downstream sees what it saw. + * + * ## Where it runs, and why there + * + * `ObjectQL.insert()` / `update()` apply it to the CALLER'S payload before + * anything else reads it — before the middleware chain, the caller-value + * snapshots, `defaultValue`s, the hooks, the strips and validation. So every + * stage agrees on one image: a `readonlyWhen` lock judges the same value it + * snapshots, a blank takes a declared `defaultValue` on insert exactly as an + * explicit `null` does (`applyFieldDefaults`, #2706), and `required` refuses + * it exactly as it refuses `null`. `ObjectQL.validate()` (the dry run) applies + * it at the same point, so a preview agrees with the write. ⛔ No driver copy: + * every REST, batch and import door reaches the engine through those methods. + * + * A value a `before*` hook writes AFTER the door is the hook's own, and is not + * normalised: a server-side producer writing `''` into a typed column is fixed + * at that producer. + * + * Takes one record or an array of them (the insert door's batch form). Pure: + * the caller's objects are never mutated — the same reference comes back when + * nothing changed, else a shallow copy (per row, and a copied array). + */ +export function normalizeBlankTypedValues( + objectSchema: { fields?: Record } | undefined | null, + data: T, +): T { + const fields = objectSchema?.fields; + if (!fields || !data || typeof data !== 'object') return data; + if (Array.isArray(data)) { + let rows: unknown[] | undefined; + for (let i = 0; i < data.length; i++) { + const row = normalizeBlankTypedRow(fields, data[i]); + if (row !== data[i]) (rows ??= data.slice())[i] = row; + } + return (rows ?? data) as T; + } + return normalizeBlankTypedRow(fields, data) as T; +} + +function normalizeBlankTypedRow(fields: Record, row: unknown): unknown { + if (!isPlainRecord(row)) return row; + let out: Record | undefined; + for (const [name, value] of Object.entries(row)) { + if (typeof value !== 'string' || !isMissing(value)) continue; + // Own-property: a field name may be `constructor` / `valueOf`. + const def = Object.prototype.hasOwnProperty.call(fields, name) ? fields[name] : undefined; + if (!def || !NON_TEXT_STORED_VALUE_TYPES.has(def.type)) continue; + (out ??= { ...row })[name] = null; + } + return out ?? row; +} + /** * Coerce `boolean`-typed fields from their SQL storage form (integer `0`/`1`, * or the strings `'0'`/`'1'`/`'true'`/`'false'`) into real JS booleans, on a @@ -758,12 +839,36 @@ function validateOne( return null; } - // ── number types ──────────────────────────────────────────────── - if (t === 'number' || t === 'currency' || t === 'percent' || t === 'rating' || t === 'slider') { + // ── number types (NUMERIC_VALUE_TYPES ∖ COMPUTED_VALUE_TYPES) ──── + // The door is the SPEC'S numeric class minus the spec's server-computed + // class, both read as constants for the reason the string branch above reads + // `BOUNDED_STRING_FIELD_TYPES` (#11875): a type joining either set there moves + // this door with no second list to forget. It was a hand-list of five until + // #20308 — `progress`, a member of the numeric class, had no type check at + // all, so `'abc'` was stored verbatim in a numeric column on memory and SQLite + // (and failed at the driver, as a 500, on PostgreSQL). + // + // ⛔ `summary` is subtracted, by the seat ruling on #20308: it is also in + // `COMPUTED_VALUE_TYPES` — 「Server-computed types: never client-written; + // shape is producer-owned」 — so its value's shape is the roll-up producer's + // to decide, not this caller-value check's. Judging it here refused the + // producer's own write: a `max` / `min` roll-up over a temporal child field + // recomputes to a date string, and the child write that triggered it then + // failed with `ERR_SUMMARY_RECOMPUTE` on memory and SQLite. A blank on a + // `summary` is still `null` at the door (`normalizeBlankTypedValues` reads the + // whole numeric class). + if (NUMERIC_VALUE_TYPES.has(t) && !COMPUTED_VALUE_TYPES.has(t)) { const n = typeof value === 'number' ? value : Number(value); if (!Number.isFinite(n)) { return fail('invalid_number'); } + // [#20308] `progress` joined the TYPE check above, and only that. The + // bounds and `scale` below keep the five types they always read: `scale`'s + // own contract names the types it is enforced on (`number`, `percent`, + // `rating`, `slider`), and `min` / `max` on `progress` were never enforced; + // starting to enforce either would narrow what a caller may write, which is + // a separate decision from "a numeric column holds a number". + if (t === 'progress') return null; if (def.min !== undefined && n < def.min) { return fail('min_value', { min: def.min }); } diff --git a/packages/rest/src/rest-data-blank-typed-value.test.ts b/packages/rest/src/rest-data-blank-typed-value.test.ts new file mode 100644 index 00000000000..f6ef2a32b3e --- /dev/null +++ b/packages/rest/src/rest-data-blank-typed-value.test.ts @@ -0,0 +1,232 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #20308 — a cleared number, date or boolean, through the REST write doors, on + * a real engine (`ObjectQL` + sqlite `SqlDriver`) and the real `RestServer` + * routes (the harness `rest-data-create-address-unknown-key.test.ts` boots). + * + * The PHYSICAL column is read with the driver's own query builder, beside the + * engine's `findOne`, because the two disagreed on the before-state: measured on + * `origin/main` de091b50e6, SQLite stored `''` in every non-string-typed column + * on every door below, and the engine's boolean read coercion then answered a + * stored `''` as `false` — a clear read back as a value. PostgreSQL refused the + * same bodies (`500 DATABASE_ERROR`; `failed` rows with `INTERNAL_ERROR` on the + * batch doors). The door now stores the typed blank, `null`, on every door. + * + * Controls: a text / lookup / select `''` is stored as `''` (the spec's + * `str_empty` — an empty string must not become null); a valid value on every + * typed column is stored unchanged; a required field's blank is refused as + * `400 VALIDATION_FAILED` / `required`; and a non-numeric string on `progress` + * is refused as `invalid_number`. + * + * `summary` is exempt from that type check (seat ruling on #20308: it is in the + * spec's `COMPUTED_VALUE_TYPES`, "never client-written; shape is + * producer-owned"), so a roll-up `max` over a temporal child field recomputes + * as it did at base: the child write succeeds and the recompute lands. Measured + * on this change before the exemption, that child write failed with + * `ERR_SUMMARY_RECOMPUTE` on memory and SQLite. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { NON_TEXT_STORED_VALUE_TYPES } from '@objectstack/spec/data'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { RestServer } from './rest-server'; + +const TYPED = [...NON_TEXT_STORED_VALUE_TYPES]; +const f = (t: string) => `f_${t}`; + +const REF = { + name: 'blank_ref', label: 'Ref', systemFields: false, + fields: { id: { name: 'id', type: 'text' as const, primaryKey: true } }, +}; +const OBJ = { + name: 'blank_rest', label: 'Blank', systemFields: false, + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true }, + c_text: { name: 'c_text', type: 'text' as const }, + c_lookup: { name: 'c_lookup', type: 'lookup' as const, reference: 'blank_ref' }, + c_select: { name: 'c_select', type: 'select' as const, options: [{ value: 'a', label: 'A' }] }, + ...Object.fromEntries(TYPED.map((t) => [f(t), { name: f(t), type: t }])), + }, +}; +const REQ = { + name: 'blank_rest_req', label: 'Required', systemFields: false, + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true }, + amount: { name: 'amount', type: 'number' as const, required: true }, + }, +}; + +// A roll-up whose value is not a number: `max` over a temporal child field. +const PARENT = { + name: 'blank_rollup_parent', label: 'Parent', systemFields: false, + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true }, + latest_due: { + name: 'latest_due', type: 'summary' as const, + summaryOperations: { object: 'blank_rollup_child', field: 'due', function: 'max' as const, relationshipField: 'parent' }, + }, + }, +}; +const CHILD = { + name: 'blank_rollup_child', label: 'Child', systemFields: false, + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true }, + parent: { name: 'parent', type: 'lookup' as const, reference: 'blank_rollup_parent' }, + due: { name: 'due', type: 'date' as const }, + }, +}; + +const VALID: Record = { + f_number: 42, f_currency: 9.5, f_percent: 0.25, f_rating: 3, f_slider: 10, f_progress: 50, f_summary: 7, + f_boolean: true, f_toggle: false, f_date: '2026-09-27', f_datetime: '2026-09-27T10:00:00.000Z', f_time: '14:30:00', +}; +const blankTyped = () => Object.fromEntries(TYPED.map((t) => [f(t), ''])); +const STR_BLANK = { c_text: '', c_lookup: '', c_select: '' }; + +const liveEngines: ObjectQL[] = []; +afterEach(async () => { + while (liveEngines.length) { + try { await liveEngines.pop()?.destroy(); } catch { /* noop */ } + } +}); + +function createMockServer() { + const noop = () => {}; + return { get: noop, post: noop, put: noop, delete: noop, patch: noop, use: noop, listen: async () => {}, close: async () => {} }; +} + +function makeRes() { + const res: any = { + write: () => true, end: () => {}, + header: () => res, + status: (code: number) => { res._status = code; return res; }, + json: (body: any) => { res._json = body; return res; }, + }; + return res; +} + +async function boot() { + const engine = new ObjectQL(); + liveEngines.push(engine); + const driver = new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }); + engine.registerDriver(driver, true); + await engine.init(); + engine.registry.registerObject(REF as any); + engine.registry.registerObject(OBJ as any); + engine.registry.registerObject(REQ as any); + engine.registry.registerObject(PARENT as any); + engine.registry.registerObject(CHILD as any); + await engine.syncSchemas(); + await engine.insert('blank_ref', { id: 'ref1' }); + + const protocol = new ObjectStackProtocolImplementation(engine as any); + const rest = new RestServer(createMockServer() as any, protocol as any, { + api: { requireAuth: false }, batch: { enableBatchEndpoint: true }, + } as any); + (rest as any).resolveExecCtx = async () => ({ userId: 'test-user' }); + rest.registerRoutes(); + const call = async (method: string, path: string, params: Record, body: unknown) => { + const route = rest.getRoutes().find((r: any) => r.method === method && r.path === path); + expect(route, `${method} ${path}`).toBeDefined(); + const res = makeRes(); + await route!.handler({ params, body, query: {}, headers: {} } as any, res); + return { status: res._status ?? 200, body: res._json }; + }; + /** The physical row, read past every engine read coercion. */ + const stored = async (object: string, id: string) => + (await (driver as any).knex(object).where({ id }).first()) as Record | undefined; + return { engine, call, stored }; +} + +function expectTypedNull(row: Record | undefined | null, where: string) { + expect(row, where).toBeTruthy(); + for (const t of TYPED) expect(row![f(t)], `${where} · ${t}`).toBeNull(); +} + +describe('REST write doors on SQLite: a cleared typed column stores null (#20308)', () => { + let ctx: Awaited>; + beforeEach(async () => { ctx = await boot(); }); + + const seed = (id: string) => ctx.engine.insert('blank_rest', { id, ...VALID }); + + it('POST create: the typed blanks store null; the string-stored blanks store ""', async () => { + const res = await ctx.call('POST', '/api/v1/data/:object', { object: 'blank_rest' }, { id: 'c1', ...blankTyped(), ...STR_BLANK }); + expect(res.status).toBe(201); + const raw = await ctx.stored('blank_rest', 'c1'); + expectTypedNull(raw, 'stored'); + expect([raw!.c_text, raw!.c_lookup, raw!.c_select]).toEqual(['', '', '']); + const read = await ctx.engine.findOne('blank_rest', { where: { id: 'c1' } }); + expectTypedNull(read as any, 'read'); + // The before-state's worst cell: a stored '' on a boolean read back as `false`. + expect((read as any).f_boolean).toBeNull(); + }); + + it('PATCH update: a cleared typed column stores null', async () => { + await seed('u1'); + const res = await ctx.call('PATCH', '/api/v1/data/:object/:id', { object: 'blank_rest', id: 'u1' }, blankTyped()); + expect(res.status).toBe(200); + expectTypedNull(await ctx.stored('blank_rest', 'u1'), 'stored'); + }); + + it('batch create and update, createMany and updateMany', async () => { + const bc = await ctx.call('POST', '/api/v1/data/:object/batch', { object: 'blank_rest' }, + { operation: 'create', records: [{ data: { id: 'b1', ...blankTyped() } }] }); + expect(bc.status).toBe(200); + expect(bc.body.results.map((r: any) => r.success)).toEqual([true]); + expectTypedNull(await ctx.stored('blank_rest', 'b1'), 'batch create'); + + await seed('b2'); + const bu = await ctx.call('POST', '/api/v1/data/:object/batch', { object: 'blank_rest' }, + { operation: 'update', records: [{ id: 'b2', data: blankTyped() }] }); + expect(bu.body.results.map((r: any) => r.success)).toEqual([true]); + expectTypedNull(await ctx.stored('blank_rest', 'b2'), 'batch update'); + + const cm = await ctx.call('POST', '/api/v1/data/:object/createMany', { object: 'blank_rest' }, [{ id: 'm1', ...blankTyped() }]); + expect(cm.status).toBe(201); + expectTypedNull(await ctx.stored('blank_rest', 'm1'), 'createMany'); + + await seed('m2'); + const um = await ctx.call('POST', '/api/v1/data/:object/updateMany', { object: 'blank_rest' }, + { records: [{ id: 'm2', data: blankTyped() }] }); + expect(um.body.results.map((r: any) => r.success)).toEqual([true]); + expectTypedNull(await ctx.stored('blank_rest', 'm2'), 'updateMany'); + }); + + it('a valid value on every typed column is stored and read back unchanged', async () => { + const res = await ctx.call('POST', '/api/v1/data/:object', { object: 'blank_rest' }, { id: 'v1', ...VALID }); + expect(res.status).toBe(201); + const read = await ctx.engine.findOne('blank_rest', { where: { id: 'v1' } }) as Record; + for (const [k, v] of Object.entries(VALID)) expect(read[k], k).toStrictEqual(v); + }); + + it('a required field refuses a blank: 400 VALIDATION_FAILED / required, nothing stored', async () => { + const res = await ctx.call('POST', '/api/v1/data/:object', { object: 'blank_rest_req' }, { id: 'q1', amount: '' }); + expect(res.status).toBe(400); + expect(res.body).toMatchObject({ code: 'VALIDATION_FAILED' }); + expect(res.body.fields.map((x: any) => [x.field, x.code])).toEqual([['amount', 'required']]); + expect(await ctx.stored('blank_rest_req', 'q1')).toBeUndefined(); + }); + + it('progress refuses a non-numeric string: 400 VALIDATION_FAILED / invalid_number', async () => { + const res = await ctx.call('POST', '/api/v1/data/:object', { object: 'blank_rest' }, { id: 'g1', f_progress: 'abc' }); + expect(res.status).toBe(400); + expect(res.body).toMatchObject({ code: 'VALIDATION_FAILED' }); + expect(res.body.fields.map((x: any) => [x.field, x.code])).toEqual([['f_progress', 'invalid_number']]); + expect(await ctx.stored('blank_rest', 'g1')).toBeUndefined(); + }); + + it('a roll-up max over a temporal child field still recomputes: the child write succeeds, as at base', async () => { + await ctx.engine.insert('blank_rollup_parent', { id: 'p1' }); + const first = await ctx.call('POST', '/api/v1/data/:object', { object: 'blank_rollup_child' }, { id: 'k1', parent: 'p1', due: '2026-01-05' }); + expect(first.status).toBe(201); + const second = await ctx.call('POST', '/api/v1/data/:object', { object: 'blank_rollup_child' }, { id: 'k2', parent: 'p1', due: '2026-02-07' }); + expect(second.status).toBe(201); + expect(await ctx.stored('blank_rollup_child', 'k2')).toBeTruthy(); + // The recompute landed — the base answer. What a date string in a summary + // column should be is a separate authoring question, not pinned here. + expect((await ctx.stored('blank_rollup_parent', 'p1'))?.latest_due).toBe('2026-02-07'); + }); +});