diff --git a/.changeset/29176-optional-unique-blank-null.md b/.changeset/29176-optional-unique-blank-null.md index a530292ae2d..9aed98d240d 100644 --- a/.changeset/29176-optional-unique-blank-null.md +++ b/.changeset/29176-optional-unique-blank-null.md @@ -10,6 +10,6 @@ Metadata-protocol: none A cleared text input is sent as `''`, and the write path stored it as sent. Every unique constraint the drivers build is NULL-distinct, so `null` never collides, but `''` is a value: the first record whose SKU (tax id, employee number, …) was cleared saved, and the second was refused "A record with this value already exists" although neither held a value. Measured on SQLite and the memory driver: two `''` rows collide, two `null` rows do not. - **Now.** The engine's write door, which `insert`, `insertMany`, `update` (by id and by predicate) and the dry run (`validate`) apply before anything else reads the payload, stores exactly `''` as `null` on a column that is text-like (`text`, `textarea`, `email`, `url`, `phone`, `markdown`, `html`, `richtext`, `code`, `color`, `signature`, `qrcode`), not `required`, and covered by a unique constraint: the field's own `unique`, or membership in a declared `indexes[]` entry carrying `unique`. Every REST, batch, upsert and import door reaches the engine through those methods. -- **Unchanged.** A `required` field given `''` is refused `required` exactly as before, also when it declares a `defaultValue`. A real duplicate is still `409 UNIQUE_VIOLATION`. Whitespace is a value and is stored as sent. `password` and `secret` keep their `''` refusal. A column no unique constraint covers, and `select` / `lookup` columns, keep storing `''`. +- **Unchanged.** A `required` field given `''` is refused `required` exactly as before, also when it declares a `defaultValue`. A real duplicate is still `409 UNIQUE_VIOLATION`. Whitespace is a value and is stored as sent. `password` and `secret` keep their `''` refusal. A text column no unique constraint covers keeps storing `''`. (Optional single-value `select` / `lookup` columns store a cleared `''` as `null` too, unique or not: a separate change in this release.) - **Defaults.** An optional unique field that declares a `defaultValue` now takes it on insert when sent `''`, exactly as when sent `null`. - **Rows already stored.** Existing `''` values are not migrated. They keep matching a `field = ''` filter; a newly cleared row matches `$null: true` instead. The `is_empty` filter (`$empty`) matches both, as before. A newly cleared row no longer collides with a stored `''`. diff --git a/.changeset/29277-single-value-blank-null.md b/.changeset/29277-single-value-blank-null.md new file mode 100644 index 00000000000..9de636a88d2 --- /dev/null +++ b/.changeset/29277-single-value-blank-null.md @@ -0,0 +1,20 @@ +--- +'@objectstack/objectql': patch +'@objectstack/core': patch +'@objectstack/metadata-protocol': patch +--- + +fix(objectql): a cleared optional single-value `select` or `lookup` stores `null`, so a second record cleared on a unique one is no longer refused `409 UNIQUE_VIOLATION` + +Clause-②: yes (widening) +Metadata-protocol: none + +A cleared `select`, `lookup` or single file field sent as `''` was stored as sent. `''` is neither an option code, a record id nor a file id, so it held no value, yet it was one to the store and to the `is_empty` filter: on a unique column the second record cleared was refused "A record with this value already exists", and on every column the cleared record did not match `is_empty` (`$empty`), whose ruled table reads only `null` as empty on these types. + +- **Now.** The engine's write door (`insert`, `insertMany`, `update` by id and by predicate, and the `validate` dry run; every REST, batch, upsert and import door reaches it) stores exactly `''` as `null` on every optional column the `$empty` table puts on its `null_only` row whose value is one string: the option types (`select`, `radio`), the reference types (`lookup`, `master_detail`, `user`, `tree`) and the file references (`image`, `file`, `avatar`, `video`, `audio`). Unique or not. Object- and array-valued types (`json`, `composite`, `location`, …) and the engine-owned `formula` / `autonumber` keep what they store. +- **Read shape.** Such a field cleared from now on reads back `null`, not `''`. It matches `is_empty` / `$empty: true`, `$null: true` and `field = null`, and no longer matches `field = ''`. +- **Unchanged.** The `$empty` table itself. A `required` field given `''` is refused `required` as before, also when it declares a `defaultValue`. Whitespace is a value. A `multiple: true` option or reference column keeps what it stores. A real duplicate is still `409 UNIQUE_VIOLATION`. +- **Defaults.** An optional field that declares a `defaultValue` now takes it on insert when sent `''`, exactly as when sent `null`. +- **Rows already stored.** Existing `''` values are not migrated by this change; they keep answering as a value (`$empty: false`, `field = ''`) until a data step cleans them. +- **Seed replay.** A seed that writes `''` on such a column, or on any column whose blank the door already stored as `null` (an optional unique text column, a number / boolean / date column), is compared in that stored form, so replaying it is no longer a change on every boot. Before, the replay compared the seed's `''` with the stored `null` and rewrote the row each time (bumping `updated_at`). +- **New export.** `@objectstack/core` exports the rule itself, `blankAsNullColumns` and `blankStorageForm` (with `ENGINE_LIFECYCLE_COLUMNS`), which the write door and the seed loader both read. diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index fb80e43666c..61e371db58c 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -169,6 +169,14 @@ export * from './utils/multi-value-storage-form.js'; // engine at runtime. export * from './utils/required-emptiness.js'; +// [#29277] …and the blank-as-null rule: which columns of an object store a +// blank string as `null`, and which blanks (the #20308, #29176 and #29277 +// arms). `@objectstack/objectql`'s write door applies it, and +// `@objectstack/metadata-protocol`'s seed replay compares a seed value in the +// form it stores — one rule, here because those two do not depend on each +// other. +export * from './utils/blank-storage-form.js'; + // [#21007] …and the refusal a scalar comparison gets on a field stored as a // JSON column: the operator set and the words. `driver-sql` refuses it on // `where`, and `@objectstack/objectql` on the per-aggregation `filter` it diff --git a/packages/core/src/utils/blank-storage-form.test.ts b/packages/core/src/utils/blank-storage-form.test.ts new file mode 100644 index 00000000000..acbfa8d6c1a --- /dev/null +++ b/packages/core/src/utils/blank-storage-form.test.ts @@ -0,0 +1,76 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect } from 'vitest'; +import { blankAsNullColumns, blankStorageForm, ENGINE_LIFECYCLE_COLUMNS } from './blank-storage-form.js'; + +const schema = { + fields: { + qty: { type: 'number', required: true }, + due: { type: 'date' }, + sku: { type: 'text', unique: true }, + code: { type: 'text' }, + idx_a: { type: 'text' }, + pw: { type: 'password', unique: true }, + stage: { type: 'select', options: [{ value: 'a' }] }, + stage_req: { type: 'select', required: true, options: [{ value: 'a' }] }, + tags: { type: 'select', multiple: true, options: [{ value: 'a' }] }, + ref: { type: 'lookup' }, + pic: { type: 'image' }, + blob: { type: 'json' }, + num: { type: 'autonumber' }, + created_by: { type: 'user' }, + created_at: { type: 'datetime' }, + }, + indexes: [{ fields: ['idx_a'], unique: true }, { fields: ['code'] }], +}; + +describe('blankAsNullColumns: the three arms of the write door\'s blank-as-null rule', () => { + const cols = blankAsNullColumns(schema); + + it('[#20308] a typed column takes any blank, required and lifecycle included', () => { + expect(cols.get('qty')).toBe('blank'); + expect(cols.get('due')).toBe('blank'); + expect(cols.get('created_at')).toBe('blank'); + }); + + it('[#29176] exactly "" on an optional text column a unique constraint covers', () => { + expect(cols.get('sku')).toBe('empty'); + expect(cols.get('idx_a')).toBe('empty'); + expect(cols.has('code')).toBe(false); + expect(cols.has('pw')).toBe(false); + }); + + it('[#29277] exactly "" on an optional one-string null_only column, unique or not', () => { + expect(cols.get('stage')).toBe('empty'); + expect(cols.get('ref')).toBe('empty'); + expect(cols.get('pic')).toBe('empty'); + for (const out of ['stage_req', 'tags', 'blob', 'num', 'created_by']) expect(cols.has(out), out).toBe(false); + }); + + it('lifecycle names are the five engine-owned columns', () => { + expect([...ENGINE_LIFECYCLE_COLUMNS]).toEqual(['id', 'created_at', 'created_by', 'updated_at', 'updated_by']); + }); + + it('an unresolvable definition declares none', () => { + expect(blankAsNullColumns(undefined).size).toBe(0); + expect(blankAsNullColumns({}).size).toBe(0); + }); +}); + +describe('blankStorageForm', () => { + it('"blank" nulls "" and whitespace; "empty" nulls only ""; no reach keeps everything', () => { + expect(blankStorageForm('', 'blank')).toBeNull(); + expect(blankStorageForm(' ', 'blank')).toBeNull(); + expect(blankStorageForm('', 'empty')).toBeNull(); + expect(blankStorageForm(' ', 'empty')).toBe(' '); + expect(blankStorageForm('', undefined)).toBe(''); + }); + + it('a non-blank value or a non-string comes back as it is', () => { + const obj = { a: 1 }; + for (const v of ['a', 0, false, null, undefined, obj]) { + expect(blankStorageForm(v, 'blank')).toBe(v); + expect(blankStorageForm(v, 'empty')).toBe(v); + } + }); +}); diff --git a/packages/core/src/utils/blank-storage-form.ts b/packages/core/src/utils/blank-storage-form.ts new file mode 100644 index 00000000000..66153ec550d --- /dev/null +++ b/packages/core/src/utils/blank-storage-form.ts @@ -0,0 +1,174 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * THE blank-as-null rule — which columns of an object store a blank string as + * `null`, and which blanks — read by the write door that stores it and by the + * seed loader's replay, which must compare a seed value in the form the door + * stores it. + * + * ## Two faces, one rule + * + * - `@objectstack/objectql`'s write door (`normalizeBlankTypedValues`) applies + * it to every insert and update payload before anything else reads it. + * - `@objectstack/metadata-protocol`'s seed loader asks it in `isNoOpReplay` + * (the `blankAsNull` replay comparand): a seed writing `''` on such a column + * lands as `null`, and compared as written it read the row as changed and + * rewrote it on every boot (#29277; the #20308 and #29176 arms churned the + * same way before it). + * + * The two packages do not depend on each other, and a copy each is how a + * replay comes to disagree with the store. So the rule lives here, beside + * `multiValueStorageForm` and `temporalStorageForm`, which are shared for the + * same reason. The type classes and the `$empty` table it reads are the + * spec's, never re-listed. + * + * ## The rule — three arms + * + * - [#20308] `'blank'`: a column whose type is non-string-typed + * (`NON_TEXT_STORED_VALUE_TYPES`: numeric, boolean, `date`, `datetime`, + * `time`) stores any blank (`isMissing`: `''` or whitespace) as `null` — + * required or not, lifecycle or not: a blank is never a value of its type. + * - [#29176] `'empty'`: exactly `''` on an optional text-like column + * (`STRING_VALUE_TYPES` minus `password` / `secret`) a unique constraint + * covers — its own `unique`, or a unique `indexes[]` entry, both read through + * `isUniqueDeclared`, the drivers' own predicate. + * - [#29277] `'empty'`: exactly `''` on an optional column on the `$empty` + * table's `null_only` row (`expandEmptyOperator`) whose value is ONE string + * ({@link ONE_STRING_STORED_TYPES}), unique or not (maintainer ruling B on + * #29346, record 6579813296). + * + * The two `'empty'` arms never reach a `required: true` column (there `''` is + * refused `required`, and must not first become a `null` a `defaultValue` + * would fill) nor a lifecycle column the engine owns + * ({@link ENGINE_LIFECYCLE_COLUMNS}), and never whitespace (a value on both + * rows of the `$empty` table they touch). + */ + +import { + NON_TEXT_STORED_VALUE_TYPES, + STRING_VALUE_TYPES, + SINGLE_OPTION_TYPES, + REFERENCE_VALUE_TYPES, + FILE_REFERENCE_TYPES, + expandEmptyOperator, + isUniqueDeclared, + type ValueShapeFieldDef, +} from '@objectstack/spec/data'; +import { isMissing } from './required-emptiness.js'; + +/** + * Lifecycle columns the engine always owns and the client never supplies, + * skipped by NAME because they are not author-declared business fields. The + * write door's validator skips the same names (its `SKIP_FIELDS` is this list). + */ +export const ENGINE_LIFECYCLE_COLUMNS: readonly string[] = Object.freeze([ + 'id', 'created_at', 'created_by', 'updated_at', 'updated_by', +]); +const LIFECYCLE: ReadonlySet = new Set(ENGINE_LIFECYCLE_COLUMNS); + +/** + * [#29176] The text-like types of the unique arm: the spec's + * `STRING_VALUE_TYPES` — exactly the "text-like" row of the `$empty` table, + * where `''` and `null` already answer as one — minus the two credential + * types. ⛔ `password` and `secret` stay out: a maintainer ruling refuses `''` + * on them (`EmptyCredentialWriteError`) while `null` means "clear", so + * rewriting the blank would turn that refusal into a silent clear. + */ +const UNIQUE_BLANK_TEXT_TYPES: ReadonlySet = new Set( + [...STRING_VALUE_TYPES].filter((t) => t !== 'password' && t !== 'secret'), +); + +/** + * [#29277] The storage-shape half of the `null_only` arm: the spec's value + * classes whose single value IS one string — an option code + * (`SINGLE_OPTION_TYPES`), a record id (`REFERENCE_VALUE_TYPES`), an opaque + * `sys_file` id (`FILE_REFERENCE_TYPES`; SQL stores it JSON-encoded, still one + * string to every reader). Read as the spec's constants, never re-listed. The + * spec has no predicate for "stores one string", so this set is stated here, + * and `record-validator.single-value-blank.test.ts` (`@objectstack/objectql`) + * ties it to the table: every type on the `null_only` row that is not typed + * must be in this set or in one of the two classes left out below, so a type + * that joins the row unclassified turns that pin red. + * + * Left out, each measured on SQLite through the engine: + * + * - `STRUCTURED_JSON_TYPES` (`json`, `composite`, `record`, `location`, + * `address`, `repeater`, `vector`): the value is an object or an array, so + * `''` is not a blank of its shape. What empty means for them is a + * separate question. + * - `COMPUTED_VALUE_TYPES` (`formula`, `autonumber`; `summary` is typed): the + * engine owns the value — a formula is never stored, an autonumber is + * issued on insert whatever the caller sent, `''` included. + */ +const ONE_STRING_STORED_TYPES: ReadonlySet = new Set([ + ...SINGLE_OPTION_TYPES, + ...REFERENCE_VALUE_TYPES, + ...FILE_REFERENCE_TYPES, +]); + +/** + * Which blanks a column stores as `null`: `'blank'` — any blank string + * (`isMissing`); `'empty'` — exactly `''`. + */ +export type BlankAsNullReach = 'blank' | 'empty'; + +/** The field shape the rule reads. */ +export interface BlankAsNullFieldDef extends ValueShapeFieldDef { + required?: boolean; + unique?: unknown; +} + +/** + * The columns of one object whose blank the write door stores as `null`, each + * with its reach. A column not in the map keeps every blank as written. + */ +export function blankAsNullColumns( + objectSchema: { fields?: Record; indexes?: unknown } | undefined | null, +): ReadonlyMap { + const out = new Map(); + const fields = objectSchema?.fields; + if (!fields || typeof fields !== 'object') return out; + let covered: Set | undefined; + const isCovered = (name: string) => (covered ??= uniqueCoveredColumns(fields, objectSchema?.indexes)).has(name); + for (const [name, def] of Object.entries(fields)) { + if (!def || typeof def.type !== 'string') continue; + if (NON_TEXT_STORED_VALUE_TYPES.has(def.type)) { + out.set(name, 'blank'); + continue; + } + if (LIFECYCLE.has(name) || def.required === true) continue; + const empty = UNIQUE_BLANK_TEXT_TYPES.has(def.type) + ? isCovered(name) + : expandEmptyOperator(def).arm === 'null_only' && ONE_STRING_STORED_TYPES.has(def.type); + if (empty) out.set(name, 'empty'); + } + return out; +} + +/** + * The form a value written to a column of the given reach is stored in: + * `null` when the value is a blank that reach takes, else the value itself + * (the same reference). `reach` `undefined` — a column not in + * {@link blankAsNullColumns} — keeps every value. + */ +export function blankStorageForm(value: unknown, reach: BlankAsNullReach | undefined): unknown { + if (reach === undefined || typeof value !== 'string') return value; + if (reach === 'empty') return value === '' ? null : value; + return isMissing(value) ? null : value; +} + +function uniqueCoveredColumns(fields: Record, indexes: unknown): Set { + const covered = new Set(); + for (const [name, def] of Object.entries(fields)) { + if (def && isUniqueDeclared(def.unique)) covered.add(name); + } + if (Array.isArray(indexes)) { + for (const idx of indexes) { + if (!idx || typeof idx !== 'object' || Array.isArray(idx)) continue; + const { unique, fields: members } = idx as { unique?: unknown; fields?: unknown }; + if (!isUniqueDeclared(unique) || !Array.isArray(members)) continue; + for (const name of members) if (typeof name === 'string') covered.add(name); + } + } + return covered; +} diff --git a/packages/metadata-protocol/src/seed-loader.ts b/packages/metadata-protocol/src/seed-loader.ts index 7a4b7288d98..76509986905 100644 --- a/packages/metadata-protocol/src/seed-loader.ts +++ b/packages/metadata-protocol/src/seed-loader.ts @@ -33,6 +33,7 @@ import { resolveSeedRecord } from '@objectstack/formula'; import { bulkWrite, withTransientRetry, defaultIsTransientError, type BulkWriteRowResult, runWithAdvisoryAggregation, type AdvisoryGroup, temporalComparandKind, temporalStorageForm, type TemporalComparandKind, isMissing, + blankAsNullColumns, blankStorageForm, type BlankAsNullReach, } from '@objectstack/core'; // [#8442] The repo's ONE recogniser for "this throw is a record-validation // failure" — duck-typed on `code`/`name`, the same predicate `mapDataError` and @@ -556,6 +557,12 @@ interface ReplayComparands { readonly temporal: ReadonlyMap; /** [#24624] Fields the read path masks: a set value is never compared. */ readonly masked: ReadonlySet; + /** + * [#29277] Fields whose blank the write door stores as `null`, with which + * blanks (`blankAsNullColumns`, `@objectstack/core` — the rule the door + * itself applies): a seed value is compared in that stored form. + */ + readonly blankAsNull: ReadonlyMap; } /** @@ -1467,10 +1474,12 @@ export class SeedLoaderService implements ISeedLoaderService { this.warnOnUndeclaredSeedColumns(dataset, objectDefinition); // How a replay compares each field (`isNoOpReplay`): [#24562] a temporal // field in its stored form, [#24624] a field the read path masks never by - // its set value. + // its set value, [#29277] a seed blank the write door stores as `null` as + // that `null`. const comparands: ReplayComparands = { temporal: temporalFieldKinds(objectDefinition), masked: maskedOnReadFields(objectDefinition), + blankAsNull: blankAsNullColumns(objectDefinition), }; // [#25779] The dataset's file fields: seed files stored and kept, and an // id-shaped file value that names no file refused (`seed-files.ts`). @@ -3114,6 +3123,15 @@ export class SeedLoaderService implements ISeedLoaderService { * change. No unmasked read is taken for this; the loader never sees a * stored credential. * + * [#29277] A seed BLANK is compared in the form the write door stores it + * (`comparands.blankAsNull`, from `blankAsNullColumns` in `@objectstack/core` + * — the rule the door itself applies, never a second spelling of it): a + * seed `''` on an optional `select` / `lookup`, on an optional unique text + * column, or any blank on a number / boolean / date column lands as `null`, + * so against a stored `null` it is no change. Only the SEED side is put in + * stored form, and only on those columns: elsewhere `''` against `null` is + * still a change, because there the door keeps `''`. + * * Pure: {@link decideWriteAction} asks it up to three times per record. */ private isNoOpReplay( @@ -3124,7 +3142,8 @@ export class SeedLoaderService implements ISeedLoaderService { for (const [key, value] of Object.entries(record)) { if (key === 'id') continue; if (SeedLoaderService.isUncomparableCredential(key, value, existing[key], comparands)) continue; - if (!this.seedValueEquals(value, existing[key], comparands.temporal.get(key))) return false; + const seedStored = blankStorageForm(value, comparands.blankAsNull.get(key)); + if (!this.seedValueEquals(seedStored, existing[key], comparands.temporal.get(key))) return false; } return true; } 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 61aeed921be..b6ec649644d 100644 --- a/packages/objectql/src/engine-blank-typed-value-door.test.ts +++ b/packages/objectql/src/engine-blank-typed-value-door.test.ts @@ -29,8 +29,9 @@ * 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. + * The text control keeps its `''` — the spec's `str_empty` side of the line; + * an optional `lookup` / `select` `''` is `null` since #29277 + * (`validation/record-validator.single-value-blank.test.ts`). */ import { describe, it, expect, beforeEach } from 'vitest'; @@ -155,7 +156,7 @@ describe('engine write doors: a blank on a non-string-typed column reaches the d await engine.insert('blank_door', payload); const sent = last('create'); expectTypedNull(sent); - expect([sent.c_text, sent.c_lookup, sent.c_select]).toEqual(['', '', '']); + expect([sent.c_text, sent.c_lookup, sent.c_select]).toEqual(['', null, null]); // The caller's own object is never rewritten. expect(payload.f_number).toBe(blank); }); @@ -181,7 +182,7 @@ describe('engine write doors: a blank on a non-string-typed column reaches the d 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(['', '', '']); + expect([byId.c_text, byId.c_lookup, byId.c_select]).toEqual(['', null, null]); await engine.update('blank_door', blankTyped(''), { where: { id: { $in: ['u1'] } }, multi: true } as any); expectTypedNull(last('updateMany')); diff --git a/packages/objectql/src/engine-optional-unique-blank-door.test.ts b/packages/objectql/src/engine-optional-unique-blank-door.test.ts index 6de7be67329..484b94f28e7 100644 --- a/packages/objectql/src/engine-optional-unique-blank-door.test.ts +++ b/packages/objectql/src/engine-optional-unique-blank-door.test.ts @@ -22,8 +22,9 @@ * of a declared `indexes[]` entry carrying `unique`; * - its edges, each a control: a column no unique constraint covers keeps * `''` (the spec's `str_empty` side of the line); whitespace is a value, as - * the `$empty` operator's ruled table reads it; `select` / `lookup` keep - * `''` (not text-like); a non-blank value is untouched; + * the `$empty` operator's ruled table reads it; a non-blank value is + * untouched. (A unique `select` / `lookup` `''` is `null` too since #29277, + * on every optional column, unique or not: `pick` / `ref` below.) * - `required` is unchanged: a required unique field's `''` is refused * `required` exactly as before — including when the field declares a * `defaultValue`, which a blank rewritten to `null` would have taken; @@ -135,10 +136,10 @@ const CRED = { }, }; -/** The covered columns of {@link OBJ}: both unique surfaces, three scopes. */ -const COVERED_NAMES = ['sku', 'tax_id', 'email', 'code']; -/** The columns that keep their `''`: no unique constraint, or not text-like. */ -const KEPT_NAMES = ['note', 'plain_idx', 'off', 'pick', 'ref']; +/** The covered columns of {@link OBJ}: both unique surfaces, three scopes; and [#29277] the single-value select / lookup. */ +const COVERED_NAMES = ['sku', 'tax_id', 'email', 'code', 'pick', 'ref']; +/** The text columns that keep their `''`: no unique constraint. */ +const KEPT_NAMES = ['note', 'plain_idx', 'off']; const blanksOf = (names: readonly string[]) => Object.fromEntries(names.map((n) => [n, ''])); const COVERED = blanksOf(COVERED_NAMES); const KEPT = blanksOf(KEPT_NAMES); diff --git a/packages/objectql/src/federated-injected-column-readers.test.ts b/packages/objectql/src/federated-injected-column-readers.test.ts index 30e9972393b..720712adb0f 100644 --- a/packages/objectql/src/federated-injected-column-readers.test.ts +++ b/packages/objectql/src/federated-injected-column-readers.test.ts @@ -342,10 +342,6 @@ const READERS: Record = { 'having-filter.ts#declaredReferenceNames :: updated_at': { disposition: 'not-a-read', why: VOCABULARY }, 'no-operator-object-door.ts# :: created_at': { disposition: 'not-a-read', why: VOCABULARY }, 'no-operator-object-door.ts# :: updated_at': { disposition: 'not-a-read', why: VOCABULARY }, - 'validation/record-validator.ts# :: created_at': { disposition: 'not-a-read', why: VOCABULARY }, - 'validation/record-validator.ts# :: created_by': { disposition: 'not-a-read', why: VOCABULARY }, - 'validation/record-validator.ts# :: updated_at': { disposition: 'not-a-read', why: VOCABULARY }, - 'validation/record-validator.ts# :: updated_by': { disposition: 'not-a-read', why: VOCABULARY }, 'util.ts#convertIntrospectedSchemaToObjects :: created_at': { disposition: 'not-a-read', why: "drafts objects from a datasource's introspected tables, skipping the platform's own columns", 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 5bf663a79da..9a105cbc46c 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 @@ -26,9 +26,12 @@ import { 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']; +// The string-stored controls: the whole string class, plus a computed string +// type (`str_empty`'s side of the line). The single-value option and reference +// types store `''` as `null` since #29277, by their own arm +// (`record-validator.single-value-blank.test.ts`); they are still not typed. +const stringStored = [...STRING_VALUE_TYPES, 'autonumber']; +const singleValueCode = ['select', 'radio', 'lookup', 'master_detail', 'user', 'tree']; function schemaOf(types: readonly string[]) { const fields: Record = {}; @@ -54,7 +57,7 @@ describe('normalizeBlankTypedValues (#20308)', () => { 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); + for (const t of [...stringStored, ...singleValueCode]) expect(NON_TEXT_STORED_VALUE_TYPES.has(t), t).toBe(false); }); it.each([[''], [' '], ['\t\n']])('a blank %j on every non-string-typed column becomes null', (blank) => { diff --git a/packages/objectql/src/validation/record-validator.optional-unique-blank.test.ts b/packages/objectql/src/validation/record-validator.optional-unique-blank.test.ts index 3afba818955..9a44f1d3d36 100644 --- a/packages/objectql/src/validation/record-validator.optional-unique-blank.test.ts +++ b/packages/objectql/src/validation/record-validator.optional-unique-blank.test.ts @@ -20,8 +20,10 @@ import { normalizeBlankTypedValues } from './record-validator.js'; const CREDENTIAL = ['password', 'secret']; const textLike = [...STRING_VALUE_TYPES].filter((t) => !CREDENTIAL.includes(t)); -// Single-value string-stored types the `$empty` table reads `''` as a value on. -const otherStringStored = ['select', 'radio', 'lookup', 'master_detail', 'user', 'autonumber']; +// A string-stored type outside both string arms: `unique` alone never pulls it +// in. (The single-value option and reference types have their own arm since +// #29277, unique or not: `record-validator.single-value-blank.test.ts`.) +const otherStringStored = ['autonumber']; type Def = Record; function schemaOf(types: readonly string[], extra: Def = {}, indexes?: unknown[]) { diff --git a/packages/objectql/src/validation/record-validator.single-value-blank.test.ts b/packages/objectql/src/validation/record-validator.single-value-blank.test.ts new file mode 100644 index 00000000000..a42c9f445a2 --- /dev/null +++ b/packages/objectql/src/validation/record-validator.single-value-blank.test.ts @@ -0,0 +1,124 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #29277 — the write door's arm for a cleared one-string column on the + * `$empty` table's `null_only` row, at the validator module: + * `normalizeBlankTypedValues` stores exactly `''` as `null` on every OPTIONAL + * column the table puts on `null_only` whose value is one string — an option + * code, a record id, a file id (maintainer ruling B on #29346, record + * 6579813296). + * + * The population is DERIVED from the table: every `FieldType` whose + * expansion is `null_only` and which is not typed (`NON_TEXT_STORED_VALUE_TYPES`, + * the #20308 arm) must be classified — one string (joins), object/array + * (`STRUCTURED_JSON_TYPES`, out) or engine-owned (`COMPUTED_VALUE_TYPES`, + * out) — so a type joining the row unclassified turns the first case red. The + * engine and physical-column halves, through REST on SQLite, are + * `packages/rest/src/rest-data-single-value-blank.test.ts`. + */ + +import { describe, it, expect } from 'vitest'; +import { + FieldType, + REFERENCE_VALUE_TYPES, + SINGLE_OPTION_TYPES, + FILE_REFERENCE_TYPES, + STRUCTURED_JSON_TYPES, + COMPUTED_VALUE_TYPES, + NON_TEXT_STORED_VALUE_TYPES, + MULTI_CAPABLE_TYPES, + expandEmptyOperator, +} from '@objectstack/spec/data'; +import { normalizeBlankTypedValues } from './record-validator.js'; + +/** Every declarable type the `$empty` table puts on `null_only` that the typed arm does not own. */ +const nullOnlyUntyped = FieldType.options.filter( + (t) => expandEmptyOperator({ type: t }).arm === 'null_only' && !NON_TEXT_STORED_VALUE_TYPES.has(t), +); +const oneString = new Set([...SINGLE_OPTION_TYPES, ...REFERENCE_VALUE_TYPES, ...FILE_REFERENCE_TYPES]); +const singleValue = nullOnlyUntyped.filter((t) => oneString.has(t)); +const leftOut = nullOnlyUntyped.filter((t) => !oneString.has(t)); +const multiCapable = singleValue.filter((t) => MULTI_CAPABLE_TYPES.has(t)); + +type Def = Record; +function schemaOf(types: readonly string[], extra: Def = {}) { + const fields: Record = {}; + for (const t of types) { + fields[`f_${t}`] = { + name: `f_${t}`, + type: t, + ...(REFERENCE_VALUE_TYPES.has(t) ? { reference: 'other' } : {}), + ...(SINGLE_OPTION_TYPES.has(t) ? { options: [{ value: 'a', label: 'A' }] } : {}), + ...extra, + }; + } + return { fields } as any; +} +const rowOf = (types: readonly string[], value: unknown) => Object.fromEntries(types.map((t) => [`f_${t}`, value])); + +describe('normalizeBlankTypedValues: optional single-value option / reference columns (#29277)', () => { + it('every untyped null_only type is classified: one string joins, object/array and engine-owned stay out', () => { + for (const t of nullOnlyUntyped) { + const classes = [oneString.has(t), STRUCTURED_JSON_TYPES.has(t), COMPUTED_VALUE_TYPES.has(t)].filter(Boolean); + expect(classes, `${t}: exactly one storage class`).toHaveLength(1); + } + // Controls on the sets themselves: if one ever emptied, every case below + // would pass over nothing. + expect([...singleValue].sort()).toEqual( + ['audio', 'avatar', 'file', 'image', 'lookup', 'master_detail', 'radio', 'select', 'tree', 'user', 'video'], + ); + expect([...leftOut].sort()).toEqual( + ['address', 'autonumber', 'composite', 'formula', 'json', 'location', 'record', 'repeater', 'vector'], + ); + expect(multiCapable.sort()).toEqual(['file', 'image', 'lookup', 'radio', 'select', 'user']); + }); + + it('the left-out types keep "": object/array-stored and engine-owned', () => { + const row = rowOf(leftOut, ''); + expect(normalizeBlankTypedValues(schemaOf(leftOut), row)).toBe(row); + }); + + it('"" on every optional member becomes null, unique or not', () => { + for (const extra of [{}, { unique: true }, { required: false }]) { + const out = normalizeBlankTypedValues(schemaOf(singleValue, extra), rowOf(singleValue, '')) as Record; + for (const t of singleValue) expect(out[`f_${t}`], `${t} ${JSON.stringify(extra)}`).toBeNull(); + } + }); + + it('a required column keeps "" (refused `required` downstream), with or without a defaultValue', () => { + for (const extra of [{ required: true }, { required: true, defaultValue: 'a' }]) { + const row = rowOf(singleValue, ''); + expect(normalizeBlankTypedValues(schemaOf(singleValue, extra), row), JSON.stringify(extra)).toBe(row); + } + }); + + it('a multiple: true column is the multi_value row, not this arm: "" passes through', () => { + const schema = schemaOf(multiCapable, { multiple: true }); + for (const t of multiCapable) expect(expandEmptyOperator(schema.fields[`f_${t}`]).arm, t).toBe('multi_value'); + const row = rowOf(multiCapable, ''); + expect(normalizeBlankTypedValues(schema, row)).toBe(row); + }); + + it('a lifecycle column the engine owns is never rewritten', () => { + const schema = { fields: { created_by: { name: 'created_by', type: 'user' }, updated_by: { name: 'updated_by', type: 'lookup', reference: 'sys_user' } } } as any; + const row = { created_by: '', updated_by: '' }; + expect(normalizeBlankTypedValues(schema, row)).toBe(row); + }); + + it('whitespace, and every non-blank value, pass through untouched', () => { + for (const v of [' ', '\t', ' a ', 'a', 'rec_1', null, undefined, 0, false]) { + const row = rowOf(singleValue, v); + expect(normalizeBlankTypedValues(schemaOf(singleValue), row), String(v)).toBe(row); + } + }); + + it('pure: an array is rewritten per row, the caller rows are never mutated', () => { + const schema = schemaOf(['select', 'lookup']); + const rows = [{ f_select: '', f_lookup: 'r1' }, { f_select: 'a', f_lookup: 'r2' }]; + const out = normalizeBlankTypedValues(schema, rows) as Array>; + expect(out).not.toBe(rows); + expect(out).toEqual([{ f_select: null, f_lookup: 'r1' }, { f_select: 'a', f_lookup: 'r2' }]); + expect(out[1]).toBe(rows[1]); + expect(rows[0]!.f_select).toBe(''); + }); +}); diff --git a/packages/objectql/src/validation/record-validator.ts b/packages/objectql/src/validation/record-validator.ts index bce552f0fba..fa965a5c683 100644 --- a/packages/objectql/src/validation/record-validator.ts +++ b/packages/objectql/src/validation/record-validator.ts @@ -99,9 +99,6 @@ import { STRUCTURED_JSON_TYPES, NUMERIC_VALUE_TYPES, COMPUTED_VALUE_TYPES, - NON_TEXT_STORED_VALUE_TYPES, - STRING_VALUE_TYPES, - isUniqueDeclared, percentScaleOf, parseNumericString, classifyFilterToken, @@ -110,7 +107,7 @@ import type { FieldErrorCode } from '@objectstack/spec/api'; // [#24131] `isMissing` / `isEmptyForRequired` — the emptiness rule `required` // is judged by — live in core, so the public-form submit door in // `@objectstack/rest` judges a form's `required` by the same rule. -import { SUPPORTED_TEMPORAL_YEARS, isOutsideTemporalYearRange, isUninterpretableTemporalComparand, multiValueStorageForm, isMissing, isEmptyForRequired } from '@objectstack/core'; +import { SUPPORTED_TEMPORAL_YEARS, isOutsideTemporalYearRange, isUninterpretableTemporalComparand, multiValueStorageForm, isMissing, isEmptyForRequired, ENGINE_LIFECYCLE_COLUMNS, blankAsNullColumns, blankStorageForm, type BlankAsNullReach } from '@objectstack/core'; import { isValueDomainMember, type ValueDomain } from '@objectstack/spec/shared'; import { renderValidationMessage, @@ -126,9 +123,7 @@ import { // business field (e.g. `sys_team`, a `managedBy: 'better-auth'` table where the // column is not injected) must get a normal required-check instead of silently // passing NULL through to the driver. -const SKIP_FIELDS = new Set([ - 'id', 'created_at', 'created_by', 'updated_at', 'updated_by', -]); +const SKIP_FIELDS = new Set(ENGINE_LIFECYCLE_COLUMNS); // Linear-time email check. Domain labels exclude '.', so the quantifiers on // either side of each '.' can't overlap — this avoids the polynomial @@ -717,8 +712,9 @@ function isInReadonlyScope(name: string, def: FieldDef, scope: ReadonlyValueScop /** * [#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. [#29176] So is `''` on - * an optional text-like column a unique constraint covers (its own section - * below). + * an optional text-like column a unique constraint covers, and [#29277] `''` + * on an optional single-value option, reference or file column (each its own + * section below). * * "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` — @@ -738,7 +734,7 @@ function isInReadonlyScope(name: string, def: FieldDef, scope: ReadonlyValueScop * ## [#29176] The one string-stored arm: an optional column a unique constraint covers * * Exactly `''` on a column that is ALL of: text-like - * ({@link UNIQUE_BLANK_TEXT_TYPES}), not `required: true`, and covered by a + * (`STRING_VALUE_TYPES` minus `password` / `secret`), not `required: true`, and covered by a * unique constraint — its own `unique`, or membership in a declared * `indexes[]` entry whose `unique` asks for one (`isUniqueDeclared`, the * predicate the drivers build their constraints by) — is `null` from the door @@ -756,16 +752,42 @@ function isInReadonlyScope(name: string, def: FieldDef, scope: ReadonlyValueScop * - a blank here also takes a declared `defaultValue` on insert, as the typed * arm's does — the door runs before the defaults, by design. * + * ## [#29277] One-string columns on the `$empty` table's `null_only` row + * + * Exactly `''` on a column that is ALL of: on the `$empty` table's + * `null_only` row (`expandEmptyOperator`, the table's own answer — so a + * `multiple: true` column, the `multi_value` row, is out), storing ONE string + * (`ONE_STRING_STORED_TYPES`: option, reference and file types), and not `required: true` — is `null` from + * the door on, unique or not (maintainer ruling B on #29346, record + * 6579813296). `''` is neither an option + * code, a record id nor a file id, so it holds no value there; the ruled table + * reads only `null` as empty on that row, and is unchanged — a row that + * already stores `''` keeps answering as a value until a data step cleans it. + * On a unique column the second record cleared is no longer refused `409`. + * The same edges as the text arm: never whitespace (a value), never a + * `required` column, and a blank on insert takes a declared `defaultValue`. + * * ## What it does NOT touch * - * ⛔ Every other string-stored column — text no unique constraint covers, - * 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 columns is a separate question, not - * answered here. ⛔ A non-blank value, and every non-string value, of any + * ⛔ Every other string-stored column — text no unique constraint covers, a + * `multiple: true` option, reference or file column (the table's + * `multi_value` row), and the `null_only` types that do not store one string + * (object/array-stored and engine-owned; `@objectstack/core`'s + * `blank-storage-form.ts` says why each is out). There `''` is a value, and the + * spec's value round-trip fixture pins it on text (`str_empty`: an empty + * string must not become null). ⛔ 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. * + * ## One rule, two readers + * + * Which columns, and which blanks, is `blankAsNullColumns` / + * `blankStorageForm` in `@objectstack/core` (`blank-storage-form.ts`) — the + * one spelling of all three arms. This door applies it; the seed loader's + * replay (`@objectstack/metadata-protocol`, the `blankAsNull` replay + * comparand) compares a seed value in the form it stores, so a seed `''` the + * door stores as `null` is not a change on every replay. + * * ## Where it runs, and why there * * `ObjectQL.insert()` / `update()` apply it to the CALLER'S payload before @@ -792,89 +814,34 @@ export function normalizeBlankTypedValues( ): T { const fields = objectSchema?.fields; if (!fields || !data || typeof data !== 'object') return data; - // Read once per call, and only when a row carries an `''` at all. - let uniqueBlank: ReadonlySet | undefined; - const uniqueBlankColumns = () => (uniqueBlank ??= optionalUniqueTextColumns(fields, objectSchema?.indexes)); + // Read once per call, and only when a row carries a blank at all. + let columns: ReadonlyMap | undefined; + const reachOf = (name: string) => (columns ??= blankAsNullColumns(objectSchema)).get(name); if (Array.isArray(data)) { let rows: unknown[] | undefined; for (let i = 0; i < data.length; i++) { - const row = normalizeBlankTypedRow(fields, uniqueBlankColumns, data[i]); + const row = normalizeBlankTypedRow(reachOf, data[i]); if (row !== data[i]) (rows ??= data.slice())[i] = row; } return (rows ?? data) as T; } - return normalizeBlankTypedRow(fields, uniqueBlankColumns, data) as T; + return normalizeBlankTypedRow(reachOf, data) as T; } function normalizeBlankTypedRow( - fields: Record, - uniqueBlankColumns: () => ReadonlySet, + reachOf: (name: string) => BlankAsNullReach | undefined, 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) continue; - if (!NON_TEXT_STORED_VALUE_TYPES.has(def.type) && !(value === '' && uniqueBlankColumns().has(name))) continue; + if (blankStorageForm(value, reachOf(name)) !== null) continue; (out ??= { ...row })[name] = null; } return out ?? row; } -/** - * [#29176] The text-like types whose `''` the write door stores as `null` on - * an optional column a unique constraint covers ({@link normalizeBlankTypedValues}). - * - * The spec's `STRING_VALUE_TYPES` — exactly the "text-like" row of the - * `$empty` operator's ruled table, where `''` and `null` already answer as one - * — read as the constant, never re-listed, minus the two credential types. - * ⛔ `password` and `secret` stay out: a maintainer ruling refuses `''` on them - * (`EmptyCredentialWriteError`) while `null` means "clear", so rewriting the - * blank here would turn that refusal into a silent clear. - * - * ⛔ Not `select` / `radio` / `lookup` / `user` and the other single-value - * string-stored types: the same `$empty` table reads their `''` as a value, - * so rewriting it would change an `is_empty` answer. That is a separate - * question from this one. - */ -const UNIQUE_BLANK_TEXT_TYPES: ReadonlySet = new Set( - [...STRING_VALUE_TYPES].filter((t) => t !== 'password' && t !== 'secret'), -); - -/** - * [#29176] The columns of one object whose `''` the write door stores as - * `null`: text-like ({@link UNIQUE_BLANK_TEXT_TYPES}), not `required: true`, - * not a lifecycle column the engine owns, and covered by a unique constraint - * on either surface the drivers build one from — a field's own `unique`, or a - * declared `indexes[]` entry whose `unique` asks for one. Both surfaces are - * read through `isUniqueDeclared`, the drivers' own predicate, so a column - * the store does not constrain is never rewritten. - */ -function optionalUniqueTextColumns(fields: Record, indexes: unknown): ReadonlySet { - const covered = new Set(); - for (const [name, def] of Object.entries(fields)) { - if (def && isUniqueDeclared(def.unique)) covered.add(name); - } - if (Array.isArray(indexes)) { - for (const idx of indexes) { - if (!isPlainRecord(idx) || !isUniqueDeclared(idx.unique) || !Array.isArray(idx.fields)) continue; - for (const name of idx.fields) if (typeof name === 'string') covered.add(name); - } - } - const out = new Set(); - for (const name of covered) { - if (SKIP_FIELDS.has(name)) continue; - // Own-property: a field name may be `constructor` / `valueOf`. - const def = Object.prototype.hasOwnProperty.call(fields, name) ? fields[name] : undefined; - if (!def || def.required === true || !UNIQUE_BLANK_TEXT_TYPES.has(def.type)) continue; - out.add(name); - } - return out; -} - /** * [#20309] The declared types the record validator's number arm judges: the * spec's numeric class minus its server-computed class, both read as constants. 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 f6ef2a32b3e..0e53ef945a3 100644 --- a/packages/rest/src/rest-data-blank-typed-value.test.ts +++ b/packages/rest/src/rest-data-blank-typed-value.test.ts @@ -13,8 +13,9 @@ * 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 + * Controls: a text `''` is stored as `''` (the spec's `str_empty` — an empty + * string must not become null), while an optional lookup / select `''` is + * `null` since #29277 (`rest-data-single-value-blank.test.ts`); a valid value on every * typed column is stored unchanged; a required field's blank is refused as * `400 VALIDATION_FAILED` / `required`; and a non-numeric string on `progress` * is refused as `invalid_number`. @@ -152,12 +153,12 @@ describe('REST write doors on SQLite: a cleared typed column stores null (#20308 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 () => { + it('POST create: the typed blanks store null; a text blank stores "", a lookup / select blank null (#29277)', 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(['', '', '']); + expect([raw!.c_text, raw!.c_lookup, raw!.c_select]).toEqual(['', null, null]); 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`. diff --git a/packages/rest/src/rest-data-single-value-blank.test.ts b/packages/rest/src/rest-data-single-value-blank.test.ts new file mode 100644 index 00000000000..4c57a906e46 --- /dev/null +++ b/packages/rest/src/rest-data-single-value-blank.test.ts @@ -0,0 +1,259 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #29277 — a cleared optional single-value option or reference field, through + * the REST write doors, on a real engine (`ObjectQL` + sqlite `SqlDriver`) and + * the real `RestServer` routes (the harness `rest-data-optional-unique-blank.test.ts` + * boots). + * + * The defect, measured on `origin/main` 5780fa49: a `select` or `lookup` + * written `''` stored `''`. On a unique column the second record cleared was + * refused `409 UNIQUE_VIOLATION` (SQL `UNIQUE` reads `''` as a value and + * `NULL` as distinct from every `NULL`), and on every column the clear was not + * empty to `$empty`, whose ruled table reads only `null` as empty on these + * types. Maintainer ruling B on #29346 (record 6579813296): the write door + * stores `''` as `null` on every optional column the table puts on its + * `null_only` row whose value is one string — `select`, `radio`, `lookup`, + * `master_detail`, `user`, `tree`, and the file references `image`, `file`, + * `avatar`, `video`, `audio` today (an opaque file id; SQL stores it + * JSON-encoded, and a cleared one collided the same way). + * + * The PHYSICAL column is read with the driver's own query builder. Pinned + * beside it: the `$empty` table is unchanged (a row that already holds `''` + * still answers as a value), `required` still refuses `''`, a blank on insert + * takes a declared `defaultValue`, and a real duplicate is still `409`. + * (Whitespace stays a value: the unit half pins it; on a reference column the + * referential-integrity check refuses it, before and after.) + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { RestServer } from './rest-server'; + +const OPTS = [{ value: 'gold', label: 'Gold' }, { value: 'silver', label: 'Silver' }]; +const TARGET = { + name: 'svb_target', label: 'Target', systemFields: false, + fields: { id: { name: 'id', type: 'text' as const, primaryKey: true }, name: { name: 'name', type: 'text' as const } }, +}; +// One optional column per ruled type, plus a unique `select` and `lookup` (the +// 409 the card names) and a unique-index member. +const OBJ = { + name: 'svb_obj', label: 'Single value blank', systemFields: false, + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true }, + name: { name: 'name', type: 'text' as const }, + c_select: { name: 'c_select', type: 'select' as const, options: OPTS }, + c_radio: { name: 'c_radio', type: 'radio' as const, options: OPTS }, + c_lookup: { name: 'c_lookup', type: 'lookup' as const, reference: 'svb_target' }, + c_master: { name: 'c_master', type: 'master_detail' as const, reference: 'svb_target' }, + c_user: { name: 'c_user', type: 'user' as const }, + c_tree: { name: 'c_tree', type: 'tree' as const }, + c_image: { name: 'c_image', type: 'image' as const }, + c_file: { name: 'c_file', type: 'file' as const }, + c_avatar: { name: 'c_avatar', type: 'avatar' as const }, + c_video: { name: 'c_video', type: 'video' as const }, + c_audio: { name: 'c_audio', type: 'audio' as const }, + u_image: { name: 'u_image', type: 'image' as const, unique: true }, + u_select: { name: 'u_select', type: 'select' as const, options: OPTS, unique: true }, + u_lookup: { name: 'u_lookup', type: 'lookup' as const, reference: 'svb_target', unique: true }, + grp: { name: 'grp', type: 'text' as const }, + i_lookup: { name: 'i_lookup', type: 'lookup' as const, reference: 'svb_target' }, + }, + indexes: [{ name: 'idx_svb_obj_grp_lookup', fields: ['grp', 'i_lookup'], unique: 'global' as const }], +}; +const PER_TYPE = [ + 'c_select', 'c_radio', 'c_lookup', 'c_master', 'c_user', 'c_tree', + 'c_image', 'c_file', 'c_avatar', 'c_video', 'c_audio', +] as const; +const REQ = { + name: 'svb_req', label: 'Required', systemFields: false, + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true }, + r_select: { name: 'r_select', type: 'select' as const, options: OPTS, required: true }, + r_lookup: { name: 'r_lookup', type: 'lookup' as const, reference: 'svb_target', required: true, defaultValue: 't1' }, + }, +}; +const DEF = { + name: 'svb_def', label: 'Defaults', systemFields: false, + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true }, + d_select: { name: 'd_select', type: 'select' as const, options: OPTS, defaultValue: 'silver' }, + d_lookup: { name: 'd_lookup', type: 'lookup' as const, reference: 'svb_target', defaultValue: 't1' }, + }, +}; + +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(); + for (const o of [TARGET, OBJ, REQ, DEF]) engine.registry.registerObject(o as any); + await engine.syncSchemas(); + + 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 }; + }; + const knex = (driver as any).knex; + /** The physical row, read past every engine read coercion. */ + const stored = async (object: string, id: string) => + (await knex(object).where({ id }).first()) as Record | undefined; + await engine.insert('svb_target', { id: 't1', name: 'T1' }); + return { engine, call, stored, knex }; +} + +type Ctx = Awaited>; +const create = (ctx: Ctx, body: Record, object = 'svb_obj') => + ctx.call('POST', '/api/v1/data/:object', { object }, body); +const patch = (ctx: Ctx, id: string, body: Record) => + ctx.call('PATCH', '/api/v1/data/:object/:id', { object: 'svb_obj', id }, body); +const allBlank = Object.fromEntries(PER_TYPE.map((f) => [f, ''])); + +describe('REST write doors on SQLite: a cleared optional single-value option / reference field stores null (#29277)', () => { + let ctx: Ctx; + beforeEach(async () => { ctx = await boot(); }); + + it('POST create with "" on every joined type: 201, every column stores NULL', async () => { + const res = await create(ctx, { id: 'p1', name: 'r1', ...allBlank }); + expect(res.status).toBe(201); + const row = await ctx.stored('svb_obj', 'p1'); + for (const f of PER_TYPE) expect(row?.[f], f).toBeNull(); + }); + + it('PATCH clearing every joined type: 200, every column stores NULL', async () => { + // `c_user` is left unset: no `sys_user` is registered here to resolve an id. + expect((await create(ctx, { id: 'e0', name: 'parent' })).status).toBe(201); + const set = { + c_select: 'gold', c_radio: 'gold', c_lookup: 't1', c_master: 't1', c_tree: 'e0', + c_image: 'img_1', c_file: 'fil_1', c_avatar: 'ava_1', c_video: 'vid_1', c_audio: 'aud_1', + }; + expect((await create(ctx, { id: 'e1', name: 'r1', ...set })).status).toBe(201); + const res = await patch(ctx, 'e1', allBlank); + expect(res.status).toBe(200); + const row = await ctx.stored('svb_obj', 'e1'); + for (const f of PER_TYPE) expect(row?.[f], f).toBeNull(); + }); + + it('unique select and unique lookup: two records created with "" both land (no 409)', async () => { + const a = await create(ctx, { id: 'u1', name: 'r1', u_select: '', u_lookup: '' }); + const b = await create(ctx, { id: 'u2', name: 'r2', u_select: '', u_lookup: '' }); + expect([a.status, b.status]).toEqual([201, 201]); + for (const id of ['u1', 'u2']) { + const row = await ctx.stored('svb_obj', id); + expect([row?.u_select, row?.u_lookup], id).toEqual([null, null]); + } + }); + + it('unique select and unique lookup: clearing two records by PATCH both land', async () => { + await ctx.engine.insert('svb_target', { id: 't2', name: 'T2' }); + expect((await create(ctx, { id: 'w1', name: 'r1', u_select: 'gold', u_lookup: 't1' })).status).toBe(201); + expect((await create(ctx, { id: 'w2', name: 'r2', u_select: 'silver', u_lookup: 't2' })).status).toBe(201); + const first = await patch(ctx, 'w1', { u_select: '', u_lookup: '' }); + const second = await patch(ctx, 'w2', { u_select: '', u_lookup: '' }); + expect([first.status, second.status]).toEqual([200, 200]); + for (const id of ['w1', 'w2']) expect((await ctx.stored('svb_obj', id))?.u_lookup, id).toBeNull(); + }); + + it('unique image: two records created with "" both land (no 409)', async () => { + const a = await create(ctx, { id: 'g1', name: 'r1', u_image: '' }); + const b = await create(ctx, { id: 'g2', name: 'r2', u_image: '' }); + expect([a.status, b.status]).toEqual([201, 201]); + for (const id of ['g1', 'g2']) expect((await ctx.stored('svb_obj', id))?.u_image, id).toBeNull(); + }); + + it('a member of a declared unique index: same group, cleared lookup, both land', async () => { + const a = await create(ctx, { id: 'i1', name: 'r1', grp: 'g', i_lookup: '' }); + const b = await create(ctx, { id: 'i2', name: 'r2', grp: 'g', i_lookup: '' }); + expect([a.status, b.status]).toEqual([201, 201]); + expect((await ctx.stored('svb_obj', 'i2'))?.i_lookup).toBeNull(); + }); + + it('batch create and createMany: every row lands, every blank stores NULL', async () => { + const bc = await ctx.call('POST', '/api/v1/data/:object/batch', { object: 'svb_obj' }, { + operation: 'create', + records: [{ data: { id: 'b1', name: 'r1', u_select: '' } }, { data: { id: 'b2', name: 'r2', u_select: '' } }], + }); + expect(bc.status).toBe(200); + expect(bc.body.results.map((r: any) => r.success)).toEqual([true, true]); + const cm = await ctx.call('POST', '/api/v1/data/:object/createMany', { object: 'svb_obj' }, + [{ id: 'm1', name: 'r3', u_lookup: '' }, { id: 'm2', name: 'r4', u_lookup: '' }]); + expect(cm.status).toBe(201); + for (const id of ['b1', 'b2']) expect((await ctx.stored('svb_obj', id))?.u_select, id).toBeNull(); + for (const id of ['m1', 'm2']) expect((await ctx.stored('svb_obj', id))?.u_lookup, id).toBeNull(); + }); + + it('a real duplicate is still refused: 409 UNIQUE_VIOLATION', async () => { + expect((await create(ctx, { id: 'd1', name: 'r1', u_lookup: 't1' })).status).toBe(201); + const dup = await create(ctx, { id: 'd2', name: 'r2', u_lookup: 't1' }); + expect(dup.status).toBe(409); + expect(dup.body).toMatchObject({ code: 'UNIQUE_VIOLATION' }); + expect(await ctx.stored('svb_obj', 'd2')).toBeUndefined(); + }); + + it('a required select / lookup refuses "": 400 VALIDATION_FAILED / required, with or without a defaultValue', async () => { + const res = await create(ctx, { id: 'q1', r_select: '', r_lookup: '' }, 'svb_req'); + 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([['r_lookup', 'required'], ['r_select', 'required']]); + expect(await ctx.stored('svb_req', 'q1')).toBeUndefined(); + }); + + it('a blank on insert takes a declared defaultValue, exactly as null does', async () => { + expect((await create(ctx, { id: 'f1', d_select: '', d_lookup: '' }, 'svb_def')).status).toBe(201); + expect((await create(ctx, { id: 'f2', d_select: null, d_lookup: null }, 'svb_def')).status).toBe(201); + for (const id of ['f1', 'f2']) { + const row = await ctx.stored('svb_def', id); + expect([row?.d_select, row?.d_lookup], id).toEqual(['silver', 't1']); + } + }); + + it('the $empty table is unchanged: null is empty, a stored "" is still a value on these types', async () => { + // Written past the engine, as the before-state stored it. + await ctx.knex('svb_obj').insert({ id: 'legacy', name: 'old', c_select: '', c_lookup: '' }); + expect((await create(ctx, { id: 'cleared', name: 'new', c_select: '', c_lookup: '' })).status).toBe(201); + expect((await create(ctx, { id: 'held', name: 'v', c_select: 'gold', c_lookup: 't1' })).status).toBe(201); + expect((await ctx.stored('svb_obj', 'legacy'))?.c_select).toBe(''); + + const ids = async (where: Record) => + ((await ctx.engine.find('svb_obj', { where, fields: ['id'] })) as Array<{ id: string }>).map((r) => r.id).sort(); + for (const f of ['c_select', 'c_lookup']) { + expect(await ids({ [f]: { $empty: true } }), f).toEqual(['cleared']); + expect(await ids({ [f]: { $empty: false } }), f).toEqual(['held', 'legacy']); + expect(await ids({ [f]: '' }), f).toEqual(['legacy']); + } + }); +}); diff --git a/packages/runtime/src/seed-blank-as-null-replay.integration.test.ts b/packages/runtime/src/seed-blank-as-null-replay.integration.test.ts new file mode 100644 index 00000000000..1b09b164901 --- /dev/null +++ b/packages/runtime/src/seed-blank-as-null-replay.integration.test.ts @@ -0,0 +1,144 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * A seed replay compares a blank in its STORED form — measured on a REAL + * `SqlDriver` under a real `ObjectQL`. + * + * The engine's write door stores some blanks as `null` (`blankStorageForm`, + * `@objectstack/core`): any blank on a non-string-typed column (#20308), + * exactly `''` on an optional unique text column (#29176), and exactly `''` on + * an optional one-string column on the `$empty` table's `null_only` row + * (#29277). A seed that writes `''` there lands as `null`, so a replay that + * compared the seed's `''` with the stored `null` literally read every such + * row as changed and rewrote it on every boot — the churn `isNoOpReplay` + * exists to prevent (`updated_at` bump, re-validation, a phantom history + * edit). The loader now asks the same predicate the door asks. + * + * Pinned, one arm per column, each against its own control: + * + * 1. A seed `''` on each arm's column lands as `null`, and a replay of the + * same seed skips every row (`totalUpdated: 0`). + * 2. Control: on a plain optional text column the door keeps `''`, so a + * stored `null` against a seed `''` is still a change: the row is + * rewritten, and stores `''`. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { ObjectQL } from '@objectstack/objectql'; +import { SeedLoaderService } from '@objectstack/metadata-protocol'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { + captureExpectedReadRefusals, + type ExpectedReadRefusalCapture, +} from './expected-read-refusal-noise.js'; + +const ROW = { + name: 'blank_replay_row', + fields: { + name: { type: 'text', required: true }, + qty: { type: 'number' }, + due: { type: 'date' }, + sku: { type: 'text', unique: true }, + stage: { type: 'select', options: [{ value: 'a', label: 'A' }] }, + note: { type: 'text' }, + }, +}; +const OBJECTS = [ROW]; + +const ENV = ['prod', 'dev', 'test']; +const SEED_CONFIG = { + dryRun: false, haltOnError: false, multiPass: true, + defaultMode: 'upsert', batchSize: 1000, transaction: false, +} as any; +const logger = { info() {}, warn() {}, error() {}, debug() {} }; + +/** One row per arm, each blank on its own column only. */ +const SEED = { + object: 'blank_replay_row', externalId: 'name', mode: 'upsert', env: ENV, + records: [ + { name: 'typed-number', qty: '' }, + { name: 'typed-date', due: '' }, + { name: 'unique-text', sku: '' }, + { name: 'one-string', stage: '' }, + ], +}; + +function metadataFor(objects: any[]) { + const byName = new Map(objects.map((o) => [o.name, o])); + return { + getObject: async (name: string) => byName.get(name), + listObjects: async () => objects, + register: async () => {}, get: async (_t: string, n: string) => byName.get(n), + list: async () => [], unregister: async () => {}, exists: async () => false, listNames: async () => [], + } as any; +} + +describe('a seed replay compares a blank in its stored form, on a REAL SqlDriver', () => { + let dir: string | null = null; + let engine: ObjectQL | null = null; + let noise: ExpectedReadRefusalCapture | null = null; + + afterEach(async () => { + try { await engine?.destroy(); } catch { /* noop */ } + engine = null; + if (dir) { rmSync(dir, { recursive: true, force: true }); dir = null; } + expect(noise?.tablesSeen() ?? ['no capture was installed']).toEqual([]); + noise = null; + }); + + async function boot() { + dir = mkdtempSync(join(tmpdir(), 'os-seed-blank-replay-')); + const driver = new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: join(dir, 'data.sqlite') }, + useNullAsDefault: true, + }); + noise = captureExpectedReadRefusals(['sys_organization']); + noise.captureDriver(driver); + await driver.initObjects(OBJECTS); + engine = new ObjectQL(); + noise.captureEngine(engine); + engine.registerDriver(driver, true); + await engine.init(); + for (const o of OBJECTS) engine.registry.registerObject(o as any); + const knex = (driver as any).knex; + const stored = async (name: string) => (await knex('blank_replay_row').where({ name }).first()) as Record; + return { loader: new SeedLoaderService(engine as any, metadataFor(OBJECTS), logger), stored }; + } + + it('each blank-as-null arm lands as null, and a replay skips every row', async () => { + const { loader, stored } = await boot(); + + const first = await loader.load({ seeds: [SEED] as any, config: SEED_CONFIG }); + expect(first.errors.map((x: any) => x.message)).toEqual([]); + expect(first.summary.totalInserted).toBe(4); + expect((await stored('typed-number')).qty).toBeNull(); + expect((await stored('typed-date')).due).toBeNull(); + expect((await stored('unique-text')).sku).toBeNull(); + expect((await stored('one-string')).stage).toBeNull(); + + const replay = await loader.load({ seeds: [SEED] as any, config: SEED_CONFIG }); + expect(replay.errors.map((x: any) => x.message)).toEqual([]); + expect(replay.summary.totalInserted).toBe(0); + expect(replay.summary.totalUpdated).toBe(0); + expect(replay.summary.totalSkipped).toBe(4); + }); + + it("control: a plain text column keeps '', so a stored null against a seed '' is still a change", async () => { + const { loader, stored } = await boot(); + const seedNull = { ...SEED, records: [{ name: 'plain', note: null }] }; + const seedBlank = { ...SEED, records: [{ name: 'plain', note: '' }] }; + + const first = await loader.load({ seeds: [seedNull] as any, config: SEED_CONFIG }); + expect(first.summary.totalInserted).toBe(1); + expect((await stored('plain')).note).toBeNull(); + + const changed = await loader.load({ seeds: [seedBlank] as any, config: SEED_CONFIG }); + expect(changed.errors.map((x: any) => x.message)).toEqual([]); + expect(changed.summary.totalUpdated).toBe(1); + expect((await stored('plain')).note).toBe(''); + }); +}); diff --git a/packages/runtime/src/seed-blank-reference-real-driver.integration.test.ts b/packages/runtime/src/seed-blank-reference-real-driver.integration.test.ts index 3400542fe4c..6ae532c26e4 100644 --- a/packages/runtime/src/seed-blank-reference-real-driver.integration.test.ts +++ b/packages/runtime/src/seed-blank-reference-real-driver.integration.test.ts @@ -146,8 +146,10 @@ describe('a blank seed reference is no reference, on a REAL SqlDriver', () => { expect(byName('loan-real').book).toBe(dune.id); // The card: the row lands and the blank field is empty — by the engine's // blank rule, not by a shape the loader picked: the value is left as - // written, and what the insert door stores for a blank string-stored - // lookup is the door's own (it does not null one). + // written, and what the insert door stores for a blank lookup is the + // door's own (an optional lookup's `''` is stored as `null` since #29277; + // whitespace as written). A replay compares the seed's `''` in that + // stored form, so it is no change. expect(isMissing(byName('loan-empty').book)).toBe(true); expect(isMissing(byName('loan-whitespace').book)).toBe(true); // A blank element is dropped like a null hole; the others resolve, in order.