fix(objectql)!: a number field reads a string by the spec's numeric grammar and stores its number (#20309)

.changeset/20309-number-arm-numeric-string-grammar.md

@@ -0,0 +1,92 @@
+---
+"@objectstack/objectql": minor
+---
+
+fix(objectql)!: a number, currency, percent, rating, slider or progress field reads a string by the platform's numeric grammar and stores the number it denotes (#20309)
+
+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
+narrowing: a string that `Number()` reads as a finite number but the platform's
+numeric grammar does not is now refused with `400 VALIDATION_FAILED` /
+`invalid_number`. It used to be accepted.
+
+**What a caller sees, before → after.** One of these strings written to one of
+those fields: `201`, stored as sent (memory kept the string; SQLite kept
+`'0x10'` as TEXT and the others as numbers by column affinity) → `400
+VALIDATION_FAILED` with the field code `invalid_number`, nothing stored. The
+REST create, batch, update and updateMany routes all answer it, and `validate`
+(the dry run) predicts it. The forms:
+
+- a radix literal: `'0x10'`, `'0X1A'`, `'0o17'`, `'0b101'`;
+- a whitespace-padded number: `' 12 '`, `'12\n'`, `'\t-3'`;
+- a spelling that is not a JSON number: `'+5'`, `'.5'`, `'5.'`, `'007'`.
+
+The fix, when a write is refused: send a JS number, or the number's plain JSON
+spelling — `'16'`, `'12'`, `'-3'`, `'5'`, `'0.5'`, `'7'`. `String(n)` of any
+finite number always qualifies, exponent forms included (`'1e-7'`, `'1e+21'`).
+
+## What was wrong
+
+This is the separate change the earlier #20309 note (arrays, booleans and
+objects refused) left open. The record validator judged a string by `Number()`
+while the write carried the string itself, so an accepted string reached the
+driver as sent:
+
+- **memory** stored `'12'` as the string `'12'` and read it back as a string;
+- **SQLite** stored `'0x10'` as the TEXT `'0x10'` (read back as `16`), and the
+  other accepted strings as numbers through the column's affinity.
+
+One write, two stored shapes, depending on the backend.
+
+## What changes
+
+- A string is judged by `parseNumericString` from `@objectstack/spec/data`, the
+  one numeric grammar the filter door also reads: the whole string is a JSON
+  number literal naming a finite double. Its case table,
+  `NUMERIC_STRING_GRAMMAR_CASES`, decides every form. No second grammar lives in
+  the engine.
+- An admitted string is stored as the number it denotes, on every backend:
+  `'12'` is written as `12`, `'1e3'` as `1000`. The rewrite runs at the write
+  door, before the middleware, the hooks, the `readonlyWhen` locks and
+  validation read the payload, so a `before*` hook now sees the number. The
+  caller's own object is not mutated.
+- `min`, `max`, `scale` and `precision` read that number, exactly as they read
+  a number: `'12.50'` passes `scale: 1` (it is `12.5`), and `'150'` over
+  `max: 100` is `max_value`.
+- This holds on every engine, REST, batch and updateMany door, and in
+  `validate` (the dry run). The server `/import` route is unchanged: its own
+  cell reader turns a numeric cell into a number before the write, so the
+  grammar never sees a string from it.
+- A blank is still `null` before the check (#20308). `summary` is still not
+  judged. A number, and an array, boolean or object, are answered as before.
+
+## Who sends numeric strings
+
+objectui's CSV import wizard, on its legacy per-row fallback (`legacyImport`,
+used only when the connected client cannot reach the server `/import` route),
+posts each raw cell to `create` after a client check of
+`!isNaN(Number(value))`. Its parser trims cells, so of the refused forms it can
+send the radix literals and the non-JSON spellings. Those rows now fail with
+`invalid_number` instead of storing a string. The fix there is the wizard's
+default path: import through the server `/import` route, whose cell reader
+converts the number before the write. Every interactive form widget sends a JS
+number or `null`, and is unaffected.
+
+## Rows already stored
+
+This judges new writes only; a stored value is never re-read by the check. On
+memory, an accepted string stayed a string until the record is next written.
+On SQLite, the earlier #20309 note's query finds a numeric column holding TEXT
+(such as `'0x10'`):
+
+```sql
+SELECT id, "FIELD" FROM "OBJECT" WHERE typeof("FIELD") = 'text';
+```
+
+OBJECT is the object name and FIELD is the field name. Nothing here rewrites
+such a cell; decide its number by hand.
+
+<!-- adr-0087: not-required (no-migration-prescription) Nothing authored moves: `packages/spec` is untouched and no metadata key is added, removed or reshaped, so `objectstack migrate meta` has nothing to rewrite and the ledger has no row to gain. What narrows is the set of caller-written string VALUES a number-typed field accepts at the write door, judged by the spec's existing numeric grammar; stored rows are never re-read, and a caller that sends a number, a blank or a grammar-admitted string is unaffected. The other categories are closed on facts: the package publishes (not `unpublished`); no ADR-0087 id covers a write-door value check (not `registered` / `already-registered`); and the change is runtime behaviour, not a TypeScript declaration (not `runtime-interface-only` / `type-surface-only`). -->

packages/objectql/src/engine-number-value-door.test.ts

@@ -15,11 +15,18 @@
  * (`Object.is`), so for a number what the arm judged is what the driver
  * receives. Memory and MongoDB store exactly this payload. The SQL physical
  * column is pinned in `packages/rest/src/rest-data-number-value.test.ts`.
- * A string is not judged differently here: that half waits on #20336.
+ *
+ * The string half (#20309, second part): a string the spec's numeric grammar
+ * reads (`parseNumericString`) reaches the driver as the NUMBER it denotes on
+ * every door, and every stage between the door and the driver (a `before*`
+ * hook, the dry run) sees that number. Measured on `origin/main` 851af0c27
+ * before it: memory stored `'12'` as the string `'12'` and read it back as a
+ * string, and `'0x10'` / `' 12 '` / `'+5'` were accepted and stored as sent. A
+ * string the grammar does not read is refused and never reaches the driver.
  */
 
 import { describe, it, expect, beforeEach } from 'vitest';
-import { COMPUTED_VALUE_TYPES, NUMERIC_VALUE_TYPES } from '@objectstack/spec/data';
+import { COMPUTED_VALUE_TYPES, NUMERIC_STRING_GRAMMAR_CASES, NUMERIC_VALUE_TYPES } from '@objectstack/spec/data';
 import { ObjectQL } from './engine.js';
 import { ValidationError } from './validation/record-validator.js';
 
@@ -155,3 +162,80 @@ describe('engine write doors: the number arm judges what the driver receives (#2
     }
   });
 });
+
+/** The spec grammar's own verdicts (#20336): admitted strings with their number, and refused non-blank strings. */
+const ADMITTED = NUMERIC_STRING_GRAMMAR_CASES.flatMap((c) => (c.numeric ? [[JSON.stringify(c.input), c.input, c.value] as const] : []));
+const REFUSED_STRINGS = NUMERIC_STRING_GRAMMAR_CASES.flatMap((c) => (!c.numeric && c.form !== 'empty' ? [[JSON.stringify(c.input), c.input] as const] : []));
+
+describe('engine write doors: a numeric string reaches the driver as its number (#20309, the string half)', () => {
+  let engine: ObjectQL;
+  let stub: ReturnType<typeof makeStubDriver>;
+
+  beforeEach(async () => {
+    stub = makeStubDriver();
+    engine = new ObjectQL();
+    engine.registerDriver(stub.driver, true);
+    await engine.init();
+    engine.registry.registerObject(OBJ as any);
+  });
+
+  const written = (field: string) =>
+    stub.calls.flatMap((c) => c.rows).filter((r) => field in r).map((r) => r[field]);
+
+  it('CONTROL: the grammar table has both halves', () => {
+    expect(ADMITTED.length).toBeGreaterThanOrEqual(10);
+    expect(REFUSED_STRINGS.length).toBeGreaterThanOrEqual(20);
+  });
+
+  describe.each(JUDGED)('%s', (type) => {
+    it.each(ADMITTED)('%s arrives as the number on insert, insert([...]), insertMany, update by id and update by predicate', async (_l, input, value) => {
+      await engine.insert('num_door', { id: 'seed', [f(type)]: 1 });
+      stub.calls.length = 0;
+      const caller = { id: 'a', [f(type)]: input };
+      await engine.insert('num_door', caller);
+      await engine.insert('num_door', [{ id: 'b', [f(type)]: input }]);
+      const outcomes = await engine.insertMany('num_door', [{ id: 'm', [f(type)]: input }]);
+      expect(outcomes.map((o) => o.ok)).toEqual([true]);
+      await engine.update('num_door', { id: 'seed', [f(type)]: input });
+      await engine.update('num_door', { [f(type)]: input }, { where: { id: { $in: ['seed'] } }, multi: true } as any);
+
+      const got = written(f(type));
+      expect(got).toHaveLength(5);
+      for (const g of got) expect(Object.is(g, value), `${JSON.stringify(input)} -> ${String(g)}`).toBe(true);
+      // The rewrite is copy-on-write: the caller's object still holds its string.
+      expect(caller[f(type)]).toBe(input);
+    });
+
+    it.each(REFUSED_STRINGS)('%s is refused on every door and never reaches the driver', async (_l, input) => {
+      await engine.insert('num_door', { id: 'seed', [f(type)]: 1 });
+      stub.calls.length = 0;
+      const expected = { code: 'VALIDATION_FAILED', fields: [[f(type), 'invalid_number']] };
+
+      expect(await refusal(() => engine.insert('num_door', { id: 'a', [f(type)]: input }))).toEqual(expected);
+      expect(await refusal(() => engine.insert('num_door', [{ id: 'b', [f(type)]: input }]))).toEqual(expected);
+      expect(await refusal(() => engine.update('num_door', { id: 'seed', [f(type)]: input }))).toEqual(expected);
+      expect(await refusal(() => engine.update('num_door', { [f(type)]: input }, { where: { id: { $in: ['seed'] } }, multi: true } as any))).toEqual(expected);
+      const outcomes = await engine.insertMany('num_door', [{ id: 'm', [f(type)]: input }]);
+      expect(outcomes.map((o) => o.ok)).toEqual([false]);
+
+      expect(written(f(type))).toEqual([]);
+    });
+  });
+
+  it('a before-hook sees the number, on insert and on update: every stage after the door reads one image', async () => {
+    const seen: Array<[string, unknown]> = [];
+    engine.registerHook('beforeInsert', async (ctx: any) => { seen.push(['insert', ctx.input.data.f_number]); }, { object: 'num_door' });
+    engine.registerHook('beforeUpdate', async (ctx: any) => { seen.push(['update', ctx.input.data.f_number]); }, { object: 'num_door' });
+    await engine.insert('num_door', { id: 'h1', f_number: '12.5' });
+    await engine.update('num_door', { id: 'h1', f_number: '-3' });
+    expect(seen).toEqual([['insert', 12.5], ['update', -3]]);
+  });
+
+  it('the dry run agrees with the write on a string', async () => {
+    const refused = await engine.validate('num_door', { id: 'p1', f_number: '0x10' });
+    expect(refused.valid).toBe(false);
+    expect(refused.results?.[0]?.errors.map((e: any) => [e.field, e.code])).toEqual([['f_number', 'invalid_number']]);
+    expect((await engine.validate('num_door', { id: 'p2', f_number: '12' })).valid).toBe(true);
+    expect((await engine.validate('num_door', { id: 'p3', f_number: ' 12 ' })).valid).toBe(false);
+  });
+});

packages/objectql/src/engine.ts

@@ -219,7 +219,7 @@ import { deriveViewContainerObject } from '@objectstack/metadata/view-container'
 // registrar and `os validate` both call.
 import { viewContainerNameRefusal } from './view-container-name-refusal.js';
 import { bindHooksToEngine } from './hook-binder.js';
-import { validateRecord, normalizeMultiValueFields, normalizeBlankTypedValues, coerceBooleanFields, ValidationError, buildFieldError, resolveFieldLabel, valueShapePostureSetByEnv, mediaPostureSetByEnv, isScannableValueShapeField, valueShapeStrictEffective, mediaStrictEffective } from './validation/record-validator.js';
+import { validateRecord, normalizeMultiValueFields, normalizeBlankTypedValues, normalizeNumericStringValues, 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';
@@ -11742,8 +11742,12 @@ export class ObjectQL implements IObjectQLEngine {
     // [#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]);
+    // `required` while the write takes the default. [#20309] Likewise a
+    // numeric string on a number field is its number here, as on the write.
+    const rawRows = normalizeNumericStringValues(
+      schemaForValidation,
+      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
@@ -11914,8 +11918,11 @@ export class ObjectQL implements IObjectQLEngine {
     // 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.
+    // rows. [#20309] At the same point, a string on a number field that the
+    // spec's numeric grammar reads becomes that number, so the validator judges
+    // the value the driver stores (`normalizeNumericStringValues`).
     data = normalizeBlankTypedValues(this._registry.getObject(object), data);
+    data = normalizeNumericStringValues(this._registry.getObject(object), data);
 
     const opCtx: OperationContext = {
       object,
@@ -12959,8 +12966,11 @@ export class ObjectQL implements IObjectQLEngine {
      // 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`.
+     // the value it snapshotted. See `normalizeBlankTypedValues`. [#20309] The
+     // insert door's numeric-string rewrite, same place and same reason (see
+     // `normalizeNumericStringValues`).
      data = normalizeBlankTypedValues(this._registry.getObject(object), data);
+     data = normalizeNumericStringValues(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