From bd3dd0818fdbce4030b6496a303cd58f12c7c1da Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 22:36:58 +0000 Subject: [PATCH 1/6] fix(objectql): read a blank on a non-string-typed column as null at the write door WIP: normalizeBlankTypedValues at insert/update/validate entry; progress and summary join the numeric type check. Claude-Session: https://claude.ai/code/session_01Bvd69VPa6puiNzzPUroDBx Co-authored-by: Claude --- packages/objectql/src/engine.ts | 23 +++- .../src/validation/record-validator.ts | 113 ++++++++++++++++-- 2 files changed, 127 insertions(+), 9 deletions(-) diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 8f21a12acbb..5b5ec84a5d9 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -213,7 +213,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'; @@ -11751,7 +11751,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 @@ -11917,6 +11921,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', @@ -12955,6 +12967,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.ts b/packages/objectql/src/validation/record-validator.ts index 8f49d1cd14b..435c72514f1 100644 --- a/packages/objectql/src/validation/record-validator.ts +++ b/packages/objectql/src/validation/record-validator.ts @@ -65,6 +65,8 @@ import { REFERENCE_VALUE_TYPES, FILE_REFERENCE_TYPES, STRUCTURED_JSON_TYPES, + NUMERIC_VALUE_TYPES, + NON_TEXT_STORED_VALUE_TYPES, percentScaleOf, } from '@objectstack/spec/data'; import type { FieldErrorCode } from '@objectstack/spec/api'; @@ -515,6 +517,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 +838,30 @@ function validateOne( return null; } - // ── number types ──────────────────────────────────────────────── - if (t === 'number' || t === 'currency' || t === 'percent' || t === 'rating' || t === 'slider') { + // ── number types (NUMERIC_VALUE_TYPES) ────────────────────────── + // The door is the SPEC'S numeric class, read as a constant for the reason the + // string branch above reads `BOUNDED_STRING_FIELD_TYPES` (#11875): a type + // joining the class there joins the type check here, with no second list to + // forget. It was a hand-list of five until #20308 — `progress` and `summary`, + // both members of the 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). + if (NUMERIC_VALUE_TYPES.has(t)) { const n = typeof value === 'number' ? value : Number(value); if (!Number.isFinite(n)) { return fail('invalid_number'); } + // [#20308] `progress` and `summary` joined the TYPE check above, and only + // that. The bounds and `scale` below keep the five types they always read: + // - `summary` is also PLATFORM-written — the roll-up recompute stores its + // aggregate through `update()` — and a platform-computed value has + // nobody to refuse (the `scale` note below says so for `formula`). + // - `scale`'s own contract names the types it is enforced on (`number`, + // `percent`, `rating`, `slider`), and `min` / `max` on these two 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' || t === 'summary') return null; if (def.min !== undefined && n < def.min) { return fail('min_value', { min: def.min }); } @@ -781,9 +879,9 @@ function validateOne( // refuse — so its `scale` is applied by rounding at the producer, in // `applyFormulaPlan`. Rounding here instead would convert #7501's rejection // into the silent alteration the ruling forbids. Nothing was carved out of - // #7501 to make that work: the type door below already excludes formula / - // summary / autonumber outputs from this function's reach, so the formula - // rounding fills a hole #7501 never covered. + // #7501 to make that work: formula / autonumber outputs never reach this + // branch, and a `summary` returns above, after the type check alone + // (#20308), so the formula rounding fills a hole #7501 never covered. // Only a well-formed declaration (integer ≥ 0) is enforced: `scale: 2.5` // has no defined meaning, and inventing one here (floor? round?) would be // the consumer-side guessing PD #12 forbids — a malformed declaration @@ -992,8 +1090,9 @@ function validateOne( return null; } - // Remaining types (formula/summary/autonumber outputs, json/code payloads) - // are explicitly open per the spec contract — see field-value.zod.ts. + // Remaining types (formula/autonumber outputs, json/code payloads) are + // explicitly open per the spec contract — see field-value.zod.ts. (`summary` + // is judged by the numeric branch above since #20308.) return null; } From e7ef2dbf96139a9b0091a96ae1230f2f9f3de502 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 22:42:36 +0000 Subject: [PATCH 2/6] test: pin the blank-typed-value write door on every engine and REST door Claude-Session: https://claude.ai/code/session_01Bvd69VPa6puiNzzPUroDBx Co-authored-by: Claude --- .../src/engine-blank-typed-value-door.test.ts | 247 ++++++++++++++++++ ...record-validator.blank-typed-value.test.ts | 141 ++++++++++ .../src/rest-data-blank-typed-value.test.ts | 191 ++++++++++++++ 3 files changed, 579 insertions(+) create mode 100644 packages/objectql/src/engine-blank-typed-value-door.test.ts create mode 100644 packages/objectql/src/validation/record-validator.blank-typed-value.test.ts create mode 100644 packages/rest/src/rest-data-blank-typed-value.test.ts 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..b24b4e2db34 --- /dev/null +++ b/packages/objectql/src/engine-blank-typed-value-door.test.ts @@ -0,0 +1,247 @@ +// 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; }, + async find(_o: string, q: any) { + const id = idOf(q); + const inIds = q?.where?.id?.$in as string[] | undefined; + const all = [...rows.values()]; + if (id) return all.filter((r) => r.id === id); + if (inIds) return all.filter((r) => inIds.includes(r.id as string)); + return all; + }, + 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 = { 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/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..05ce2656b15 --- /dev/null +++ b/packages/objectql/src/validation/record-validator.blank-typed-value.test.ts @@ -0,0 +1,141 @@ +// 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` or + * `NUMERIC_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 { + 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: '' }); + }); +}); + +describe('the numeric type door is NUMERIC_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.each([...NUMERIC_VALUE_TYPES])('%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('progress and summary take the type check only — 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/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..40f8ad99fb4 --- /dev/null +++ b/packages/rest/src/rest-data-blank-typed-value.test.ts @@ -0,0 +1,191 @@ +// 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` + * or `summary` is refused as `invalid_number`. + */ + +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 }, + }, +}; + +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); + 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 and summary refuse 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', f_summary: 'abc' }); + expect(res.status).toBe(400); + expect(res.body).toMatchObject({ code: 'VALIDATION_FAILED' }); + expect(res.body.fields.map((x: any) => [x.field, x.code]).sort()).toEqual([['f_progress', 'invalid_number'], ['f_summary', 'invalid_number']]); + expect(await ctx.stored('blank_rest', 'g1')).toBeUndefined(); + }); +}); From e0c193f4bdf9eca8a92576941851c59e2f51a5bf Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 22:48:01 +0000 Subject: [PATCH 3/6] chore(changeset): objectql patch for the blank-typed-value write door Claude-Session: https://claude.ai/code/session_01Bvd69VPa6puiNzzPUroDBx Co-authored-by: Claude --- .changeset/20308-blank-typed-value-null.md | 69 ++++++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 .changeset/20308-blank-typed-value-null.md diff --git a/.changeset/20308-blank-typed-value-null.md b/.changeset/20308-blank-typed-value-null.md new file mode 100644 index 00000000000..33a2ee1c19f --- /dev/null +++ b/.changeset/20308-blank-typed-value-null.md @@ -0,0 +1,69 @@ +--- +"@objectstack/objectql": patch +--- + +fix(objectql): a cleared number, boolean, date, datetime or time field stores `null`, on every backend (#20308) + +`patch` — a bug fix in a released package. No exported symbol, no spec key and +no API signature changes. + +## 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` and `summary` had no type check at all: a non-numeric +string such as `'abc'` was stored verbatim on memory and SQLite. + +## 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. +- **String-stored columns are untouched.** A text, lookup or select `''` is still + stored as `''`. +- **`progress` and `summary` join the numeric type check.** A non-numeric + string on either is refused with `invalid_number`, as on `number`. No `min`, + `max` or `scale` is newly enforced on them. +- **One consequence for roll-ups.** A `summary` whose `summaryOperations` takes + `min` or `max` over a `date`, `datetime` or `time` field writes a date string + into the numeric summary. PostgreSQL already refused that recompute + (`ERR_SUMMARY_RECOMPUTE`). Memory and SQLite now refuse it the same way, + instead of storing the string. + +## 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)) = ''; +``` From 4525303324693d74a15883964209c22e4f649047 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 23:15:19 +0000 Subject: [PATCH 4/6] test(objectql): type the blank-door test's caller payload and unused stub param Claude-Session: https://claude.ai/code/session_01Bvd69VPa6puiNzzPUroDBx Co-authored-by: Claude --- packages/objectql/src/engine-blank-typed-value-door.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/objectql/src/engine-blank-typed-value-door.test.ts b/packages/objectql/src/engine-blank-typed-value-door.test.ts index b24b4e2db34..c0d5fa105aa 100644 --- a/packages/objectql/src/engine-blank-typed-value-door.test.ts +++ b/packages/objectql/src/engine-blank-typed-value-door.test.ts @@ -74,7 +74,7 @@ function makeStubDriver() { rows.set(row.id as string, row); return row; }, - async bulkCreate(o: string, list: Record[]) { + async bulkCreate(_o: string, list: Record[]) { calls.push({ fn: 'bulkCreate', data: { rows: list.map((r) => ({ ...r })) } }); const out: Record[] = []; for (const r of list) { @@ -156,7 +156,7 @@ describe('engine write doors: a blank on a non-string-typed column reaches the d 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 = { id: 'a', ...blankTyped(blank), ...STR_BLANK }; + const payload: Record = { id: 'a', ...blankTyped(blank), ...STR_BLANK }; await engine.insert('blank_door', payload); const sent = last('create'); expectTypedNull(sent); From 7bae61b0d27296a8ec80c4de612fea8c72c1dbd3 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 23:29:59 +0000 Subject: [PATCH 5/6] test(objectql): the blank-door stub's find answers the store, not a query check:objectql-double-limit could not seat the id-only filter as a query-honouring double; nothing in the file reads rows back through find. Claude-Session: https://claude.ai/code/session_01Bvd69VPa6puiNzzPUroDBx Co-authored-by: Claude --- .../src/engine-blank-typed-value-door.test.ts | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/packages/objectql/src/engine-blank-typed-value-door.test.ts b/packages/objectql/src/engine-blank-typed-value-door.test.ts index c0d5fa105aa..61aeed921be 100644 --- a/packages/objectql/src/engine-blank-typed-value-door.test.ts +++ b/packages/objectql/src/engine-blank-typed-value-door.test.ts @@ -55,14 +55,9 @@ function makeStubDriver() { const driver: any = { name: 'stub', version: '0.0.0', supports: {}, async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, - async find(_o: string, q: any) { - const id = idOf(q); - const inIds = q?.where?.id?.$in as string[] | undefined; - const all = [...rows.values()]; - if (id) return all.filter((r) => r.id === id); - if (inIds) return all.filter((r) => inIds.includes(r.id as string)); - return all; - }, + // 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; From 076b82c19a01ee925c1e59680fa6818dfee443d1 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 23:56:49 +0000 Subject: [PATCH 6/6] fix(objectql)!: exempt summary from the numeric type check; progress keeps it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Seat ruling 5860986842 on #20308: the number branch's door is NUMERIC_VALUE_TYPES minus COMPUTED_VALUE_TYPES, so a roll-up max/min over a temporal child field recomputes as at base. The changeset declares the progress narrowing (Clause-② no, narrowing; BREAKING, minor). Claude-Session: https://claude.ai/code/session_01Bvd69VPa6puiNzzPUroDBx Co-authored-by: Claude --- .changeset/20308-blank-typed-value-null.md | 45 ++++++++------ ...record-validator.blank-typed-value.test.ts | 34 +++++++++-- .../src/validation/record-validator.ts | 58 ++++++++++--------- .../src/rest-data-blank-typed-value.test.ts | 49 ++++++++++++++-- 4 files changed, 132 insertions(+), 54 deletions(-) diff --git a/.changeset/20308-blank-typed-value-null.md b/.changeset/20308-blank-typed-value-null.md index 33a2ee1c19f..7366a072bd8 100644 --- a/.changeset/20308-blank-typed-value-null.md +++ b/.changeset/20308-blank-typed-value-null.md @@ -1,11 +1,16 @@ --- -"@objectstack/objectql": patch +"@objectstack/objectql": minor --- -fix(objectql): a cleared number, boolean, date, datetime or time field stores `null`, on every backend (#20308) +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) -`patch` — a bug fix in a released package. No exported symbol, no spec key and -no API signature changes. +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 @@ -25,8 +30,9 @@ had three outcomes: 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` and `summary` had no type check at all: a non-numeric -string such as `'abc'` was stored verbatim on memory and SQLite. +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 @@ -42,28 +48,31 @@ string such as `'abc'` was stored verbatim on memory and SQLite. - **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. + 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` and `summary` join the numeric type check.** A non-numeric - string on either is refused with `invalid_number`, as on `number`. No `min`, - `max` or `scale` is newly enforced on them. -- **One consequence for roll-ups.** A `summary` whose `summaryOperations` takes - `min` or `max` over a `date`, `datetime` or `time` field writes a date string - into the numeric summary. PostgreSQL already refused that recompute - (`ERR_SUMMARY_RECOMPUTE`). Memory and SQLite now refuse it the same way, - instead of storing the string. +- **`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): +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/validation/record-validator.blank-typed-value.test.ts b/packages/objectql/src/validation/record-validator.blank-typed-value.test.ts index 05ce2656b15..d053d2da244 100644 --- a/packages/objectql/src/validation/record-validator.blank-typed-value.test.ts +++ b/packages/objectql/src/validation/record-validator.blank-typed-value.test.ts @@ -6,9 +6,10 @@ * * `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` or - * `NUMERIC_VALUE_TYPES` tomorrow is covered here without an edit — and a type - * leaving the string side would turn the control half red. + * 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 @@ -17,6 +18,7 @@ import { describe, it, expect } from 'vitest'; import { + COMPUTED_VALUE_TYPES, NON_TEXT_STORED_VALUE_TYPES, NUMERIC_VALUE_TYPES, STRING_VALUE_TYPES, @@ -109,7 +111,12 @@ describe('normalizeBlankTypedValues (#20308)', () => { }); }); -describe('the numeric type door is NUMERIC_VALUE_TYPES (#20308)', () => { +// 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'); @@ -120,7 +127,14 @@ describe('the numeric type door is NUMERIC_VALUE_TYPES (#20308)', () => { } } - it.each([...NUMERIC_VALUE_TYPES])('%s refuses a non-numeric string with invalid_number', (type) => { + 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']]); @@ -130,7 +144,15 @@ describe('the numeric type door is NUMERIC_VALUE_TYPES (#20308)', () => { expect(refusal(type, 7)).toBeNull(); }); - it('progress and summary take the type check only — no bound or scale is newly enforced', () => { + 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']) { diff --git a/packages/objectql/src/validation/record-validator.ts b/packages/objectql/src/validation/record-validator.ts index 435c72514f1..ea2e47c72f0 100644 --- a/packages/objectql/src/validation/record-validator.ts +++ b/packages/objectql/src/validation/record-validator.ts @@ -66,6 +66,7 @@ import { FILE_REFERENCE_TYPES, STRUCTURED_JSON_TYPES, NUMERIC_VALUE_TYPES, + COMPUTED_VALUE_TYPES, NON_TEXT_STORED_VALUE_TYPES, percentScaleOf, } from '@objectstack/spec/data'; @@ -838,30 +839,36 @@ function validateOne( return null; } - // ── number types (NUMERIC_VALUE_TYPES) ────────────────────────── - // The door is the SPEC'S numeric class, read as a constant for the reason the - // string branch above reads `BOUNDED_STRING_FIELD_TYPES` (#11875): a type - // joining the class there joins the type check here, with no second list to - // forget. It was a hand-list of five until #20308 — `progress` and `summary`, - // both members of the 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). - if (NUMERIC_VALUE_TYPES.has(t)) { + // ── 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` and `summary` joined the TYPE check above, and only - // that. The bounds and `scale` below keep the five types they always read: - // - `summary` is also PLATFORM-written — the roll-up recompute stores its - // aggregate through `update()` — and a platform-computed value has - // nobody to refuse (the `scale` note below says so for `formula`). - // - `scale`'s own contract names the types it is enforced on (`number`, - // `percent`, `rating`, `slider`), and `min` / `max` on these two 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' || t === 'summary') return null; + // [#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 }); } @@ -879,9 +886,9 @@ function validateOne( // refuse — so its `scale` is applied by rounding at the producer, in // `applyFormulaPlan`. Rounding here instead would convert #7501's rejection // into the silent alteration the ruling forbids. Nothing was carved out of - // #7501 to make that work: formula / autonumber outputs never reach this - // branch, and a `summary` returns above, after the type check alone - // (#20308), so the formula rounding fills a hole #7501 never covered. + // #7501 to make that work: the type door below already excludes formula / + // summary / autonumber outputs from this function's reach, so the formula + // rounding fills a hole #7501 never covered. // Only a well-formed declaration (integer ≥ 0) is enforced: `scale: 2.5` // has no defined meaning, and inventing one here (floor? round?) would be // the consumer-side guessing PD #12 forbids — a malformed declaration @@ -1090,9 +1097,8 @@ function validateOne( return null; } - // Remaining types (formula/autonumber outputs, json/code payloads) are - // explicitly open per the spec contract — see field-value.zod.ts. (`summary` - // is judged by the numeric branch above since #20308.) + // Remaining types (formula/summary/autonumber outputs, json/code payloads) + // are explicitly open per the spec contract — see field-value.zod.ts. return null; } diff --git a/packages/rest/src/rest-data-blank-typed-value.test.ts b/packages/rest/src/rest-data-blank-typed-value.test.ts index 40f8ad99fb4..f6ef2a32b3e 100644 --- a/packages/rest/src/rest-data-blank-typed-value.test.ts +++ b/packages/rest/src/rest-data-blank-typed-value.test.ts @@ -17,7 +17,14 @@ * `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` - * or `summary` is refused as `invalid_number`. + * 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'; @@ -52,6 +59,26 @@ const REQ = { }, }; +// 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', @@ -90,6 +117,8 @@ async function boot() { 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' }); @@ -181,11 +210,23 @@ describe('REST write doors on SQLite: a cleared typed column stores null (#20308 expect(await ctx.stored('blank_rest_req', 'q1')).toBeUndefined(); }); - it('progress and summary refuse 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', f_summary: 'abc' }); + 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]).sort()).toEqual([['f_progress', 'invalid_number'], ['f_summary', 'invalid_number']]); + 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'); + }); });