Skip to content

Commit bd3dd08

Browse files
committed
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 <noreply@anthropic.com>
1 parent de091b5 commit bd3dd08

2 files changed

Lines changed: 127 additions & 9 deletions

File tree

‎packages/objectql/src/engine.ts‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ import {
213213
// `resolveMetadataItemName` below for why this registrar lost its fourth copy.
214214
import { deriveViewContainerObject } from '@objectstack/metadata/view-container';
215215
import { bindHooksToEngine } from './hook-binder.js';
216-
import { validateRecord, normalizeMultiValueFields, coerceBooleanFields, ValidationError, buildFieldError, resolveFieldLabel, valueShapePostureSetByEnv, mediaPostureSetByEnv, isScannableValueShapeField, valueShapeStrictEffective, mediaStrictEffective } from './validation/record-validator.js';
216+
import { validateRecord, normalizeMultiValueFields, normalizeBlankTypedValues, coerceBooleanFields, ValidationError, buildFieldError, resolveFieldLabel, valueShapePostureSetByEnv, mediaPostureSetByEnv, isScannableValueShapeField, valueShapeStrictEffective, mediaStrictEffective } from './validation/record-validator.js';
217217
import type { AdmittedValueShapeViolation, AdmittedValueShapeViolationSink } from './validation/record-validator.js';
218218
import type { RelatedFieldBinding, RelatedRecordBinding } from './validation/rule-validator.js';
219219
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 {
1175111751
// those arms, and the families it does not carry).
1175211752
// `update()` deliberately does not default (#2706: a PATCH's explicit
1175311753
// `null` means "clear it"), so neither does an `update`-mode preview.
11754-
const rawRows = Array.isArray(data) ? data : [data];
11754+
// [#20308] The write doors read a blank on a non-string-typed column as
11755+
// `null` before anything else; the preview does the same at the same point,
11756+
// or a blank on a required field with a `defaultValue` would preview
11757+
// `required` while the write takes the default.
11758+
const rawRows = normalizeBlankTypedValues(schemaForValidation, Array.isArray(data) ? data : [data]);
1175511759
const nowSnapshot = new Date();
1175611760
// [#20082] The preview's ONE permission resolution, shared by its CEL
1175711761
// defaults and its option gates below, exactly as the write shares one. A
@@ -11917,6 +11921,14 @@ export class ObjectQL implements IObjectQLEngine {
1191711921
// hook, default or validation runs, so a refusal costs nothing.
1191811922
this.enforceTransactionOrigin(object, driver, 'insert');
1191911923

11924+
// [#20308] A blank on a non-string-typed column is `null` from here on —
11925+
// before the middleware, the caller snapshot, the defaults, the hooks and
11926+
// validation read the payload, so all of them see one image (a blank then
11927+
// takes a `defaultValue` exactly as `null` does). See
11928+
// `normalizeBlankTypedValues` for the scope; it never mutates the caller's
11929+
// rows.
11930+
data = normalizeBlankTypedValues(this._registry.getObject(object), data);
11931+
1192011932
const opCtx: OperationContext = {
1192111933
object,
1192211934
operation: 'insert',
@@ -12955,6 +12967,13 @@ export class ObjectQL implements IObjectQLEngine {
1295512967
// itself. Resolve first, then extract.
1295612968
options = this.withResolvedWhere(options);
1295712969

12970+
// [#20308] The insert door's rule, same place: a blank on a
12971+
// non-string-typed column is `null` before the middleware, the
12972+
// caller-value snapshot (`suppliedValues`), the hooks, the read-only
12973+
// strips and validation read the payload — so a `readonlyWhen` lock judges
12974+
// the value it snapshotted. See `normalizeBlankTypedValues`.
12975+
data = normalizeBlankTypedValues(this._registry.getObject(object), data);
12976+
1295812977
// 1. Extract ID from data or where if it's a single update by ID.
1295912978
// Only a SCALAR `where.id` means "update one row by primary key". An
1296012979
// operator object ({ $in: [...] }, { $ne: ... }, …) is a multi-row

‎packages/objectql/src/validation/record-validator.ts‎

Lines changed: 106 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,8 @@ import {
6565
REFERENCE_VALUE_TYPES,
6666
FILE_REFERENCE_TYPES,
6767
STRUCTURED_JSON_TYPES,
68+
NUMERIC_VALUE_TYPES,
69+
NON_TEXT_STORED_VALUE_TYPES,
6870
percentScaleOf,
6971
} from '@objectstack/spec/data';
7072
import type { FieldErrorCode } from '@objectstack/spec/api';
@@ -515,6 +517,84 @@ export function normalizeMultiValueFields(
515517
}
516518
}
517519

520+
/**
521+
* [#20308] A BLANK string on a non-string-typed column is that column's typed
522+
* blank, `null` — the write door's one reading of it.
523+
*
524+
* "Blank" is exactly what {@link isMissing} calls missing: `''` or a string of
525+
* whitespace. "Non-string-typed" is the spec's `NON_TEXT_STORED_VALUE_TYPES` —
526+
* the numeric class (`progress` and `summary` included), the boolean class,
527+
* `date`, `datetime` and `time`. Read as the constant, never re-listed.
528+
*
529+
* ## Why the door has to say it
530+
*
531+
* The validator has always READ a blank as missing: `validateOne` returns on
532+
* `isMissing` before any type check. Nothing WROTE it that way, so the blank
533+
* went on to the driver exactly as sent — memory and SQLite stored `''` in a
534+
* number, boolean, date, datetime or time column, and PostgreSQL refused the
535+
* statement (`22P02` / `22007`, a `500` at REST). One clear of one field, three
536+
* outcomes; objectui's date and time boxes send `''` when cleared. This makes
537+
* storage agree with the platform's own reading.
538+
*
539+
* ## What it does NOT touch
540+
*
541+
* ⛔ Every string-stored column — text, lookup ids, select values. There `''`
542+
* is a value, and the spec's value round-trip fixture pins it (`str_empty`:
543+
* an empty string must not become null). What "empty" means for those types is
544+
* a separate question, not answered here. ⛔ A non-blank value, and every
545+
* non-string value, of any type: `null`, `0`, `false`, `'abc'` pass through
546+
* untouched, so every type check and refusal downstream sees what it saw.
547+
*
548+
* ## Where it runs, and why there
549+
*
550+
* `ObjectQL.insert()` / `update()` apply it to the CALLER'S payload before
551+
* anything else reads it — before the middleware chain, the caller-value
552+
* snapshots, `defaultValue`s, the hooks, the strips and validation. So every
553+
* stage agrees on one image: a `readonlyWhen` lock judges the same value it
554+
* snapshots, a blank takes a declared `defaultValue` on insert exactly as an
555+
* explicit `null` does (`applyFieldDefaults`, #2706), and `required` refuses
556+
* it exactly as it refuses `null`. `ObjectQL.validate()` (the dry run) applies
557+
* it at the same point, so a preview agrees with the write. ⛔ No driver copy:
558+
* every REST, batch and import door reaches the engine through those methods.
559+
*
560+
* A value a `before*` hook writes AFTER the door is the hook's own, and is not
561+
* normalised: a server-side producer writing `''` into a typed column is fixed
562+
* at that producer.
563+
*
564+
* Takes one record or an array of them (the insert door's batch form). Pure:
565+
* the caller's objects are never mutated — the same reference comes back when
566+
* nothing changed, else a shallow copy (per row, and a copied array).
567+
*/
568+
export function normalizeBlankTypedValues<T>(
569+
objectSchema: { fields?: Record<string, FieldDef> } | undefined | null,
570+
data: T,
571+
): T {
572+
const fields = objectSchema?.fields;
573+
if (!fields || !data || typeof data !== 'object') return data;
574+
if (Array.isArray(data)) {
575+
let rows: unknown[] | undefined;
576+
for (let i = 0; i < data.length; i++) {
577+
const row = normalizeBlankTypedRow(fields, data[i]);
578+
if (row !== data[i]) (rows ??= data.slice())[i] = row;
579+
}
580+
return (rows ?? data) as T;
581+
}
582+
return normalizeBlankTypedRow(fields, data) as T;
583+
}
584+
585+
function normalizeBlankTypedRow(fields: Record<string, FieldDef>, row: unknown): unknown {
586+
if (!isPlainRecord(row)) return row;
587+
let out: Record<string, unknown> | undefined;
588+
for (const [name, value] of Object.entries(row)) {
589+
if (typeof value !== 'string' || !isMissing(value)) continue;
590+
// Own-property: a field name may be `constructor` / `valueOf`.
591+
const def = Object.prototype.hasOwnProperty.call(fields, name) ? fields[name] : undefined;
592+
if (!def || !NON_TEXT_STORED_VALUE_TYPES.has(def.type)) continue;
593+
(out ??= { ...row })[name] = null;
594+
}
595+
return out ?? row;
596+
}
597+
518598
/**
519599
* Coerce `boolean`-typed fields from their SQL storage form (integer `0`/`1`,
520600
* or the strings `'0'`/`'1'`/`'true'`/`'false'`) into real JS booleans, on a
@@ -758,12 +838,30 @@ function validateOne(
758838
return null;
759839
}
760840

761-
// ── number types ────────────────────────────────────────────────
762-
if (t === 'number' || t === 'currency' || t === 'percent' || t === 'rating' || t === 'slider') {
841+
// ── number types (NUMERIC_VALUE_TYPES) ──────────────────────────
842+
// The door is the SPEC'S numeric class, read as a constant for the reason the
843+
// string branch above reads `BOUNDED_STRING_FIELD_TYPES` (#11875): a type
844+
// joining the class there joins the type check here, with no second list to
845+
// forget. It was a hand-list of five until #20308 — `progress` and `summary`,
846+
// both members of the class, had no type check at all, so `'abc'` was stored
847+
// verbatim in a numeric column on memory and SQLite (and failed at the
848+
// driver, as a 500, on PostgreSQL).
849+
if (NUMERIC_VALUE_TYPES.has(t)) {
763850
const n = typeof value === 'number' ? value : Number(value);
764851
if (!Number.isFinite(n)) {
765852
return fail('invalid_number');
766853
}
854+
// [#20308] `progress` and `summary` joined the TYPE check above, and only
855+
// that. The bounds and `scale` below keep the five types they always read:
856+
// - `summary` is also PLATFORM-written — the roll-up recompute stores its
857+
// aggregate through `update()` — and a platform-computed value has
858+
// nobody to refuse (the `scale` note below says so for `formula`).
859+
// - `scale`'s own contract names the types it is enforced on (`number`,
860+
// `percent`, `rating`, `slider`), and `min` / `max` on these two were
861+
// never enforced; starting to enforce either would narrow what a caller
862+
// may write, which is a separate decision from "a numeric column holds a
863+
// number".
864+
if (t === 'progress' || t === 'summary') return null;
767865
if (def.min !== undefined && n < def.min) {
768866
return fail('min_value', { min: def.min });
769867
}
@@ -781,9 +879,9 @@ function validateOne(
781879
// refuse — so its `scale` is applied by rounding at the producer, in
782880
// `applyFormulaPlan`. Rounding here instead would convert #7501's rejection
783881
// into the silent alteration the ruling forbids. Nothing was carved out of
784-
// #7501 to make that work: the type door below already excludes formula /
785-
// summary / autonumber outputs from this function's reach, so the formula
786-
// rounding fills a hole #7501 never covered.
882+
// #7501 to make that work: formula / autonumber outputs never reach this
883+
// branch, and a `summary` returns above, after the type check alone
884+
// (#20308), so the formula rounding fills a hole #7501 never covered.
787885
// Only a well-formed declaration (integer ≥ 0) is enforced: `scale: 2.5`
788886
// has no defined meaning, and inventing one here (floor? round?) would be
789887
// the consumer-side guessing PD #12 forbids — a malformed declaration
@@ -992,8 +1090,9 @@ function validateOne(
9921090
return null;
9931091
}
9941092

995-
// Remaining types (formula/summary/autonumber outputs, json/code payloads)
996-
// are explicitly open per the spec contract — see field-value.zod.ts.
1093+
// Remaining types (formula/autonumber outputs, json/code payloads) are
1094+
// explicitly open per the spec contract — see field-value.zod.ts. (`summary`
1095+
// is judged by the numeric branch above since #20308.)
9971096
return null;
9981097
}
9991098

0 commit comments

Comments
 (0)