From d3853e181cf7b32ef9c38ee680bc63ec78232580 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 18 Aug 2026 22:31:38 +0000 Subject: [PATCH] fix(objectql): [] no longer satisfies required on a multi-value field (#9476) Per the #9447 maintainer ruling (2026-08-18): required on a multi-value field means non-empty array. Teach both required read sites (INSERT and the ADR-0113 UPDATE non-regression check) the def-aware emptiness judgment via isEmptyForRequired; pin the rejection envelope, the populated-array control, and the null controls' distinct sentences. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019yDEhPBC3tcGkW9bkce1HM --- ...quired-multi-value-empty-array-rejected.md | 31 ++++++ .../src/validation/record-validator.test.ts | 103 ++++++++++++++++++ .../src/validation/record-validator.ts | 26 ++++- 3 files changed, 157 insertions(+), 3 deletions(-) create mode 100644 .changeset/required-multi-value-empty-array-rejected.md diff --git a/.changeset/required-multi-value-empty-array-rejected.md b/.changeset/required-multi-value-empty-array-rejected.md new file mode 100644 index 0000000000..43ac95d90a --- /dev/null +++ b/.changeset/required-multi-value-empty-array-rejected.md @@ -0,0 +1,31 @@ +--- +"@objectstack/objectql": patch +--- + +fix(objectql): `[]` no longer satisfies `required` on a multi-value field — the #9447 ruling's enforcement half (#9476) + + + +Per the #9447 maintainer ruling (2026-08-18): `required` on a multi-value +field means **non-empty array**. The empty set is representable — it reads +back as `[]`, never `null` — so `required` judges emptiness. + +Before this, `validateRecord` judged `required` through `isMissing`, which +knows `undefined` / `null` / blank strings — an explicit `[]` sailed through +on both INSERT and UPDATE while `null` was correctly rejected. Now: + +- INSERT: `[]` on a required multi-value field is rejected — 400 + `VALIDATION_FAILED`, field code `required`, the same envelope a missing + value already got. +- UPDATE: a SUPPLIED `[]` is an explicit clear — rejected with the distinct + `required_cleared` sentence (wire code `required`), exactly like an + explicit `null`. An omitted field still never 400s — legacy rows rest. +- Scope is the spec's own multi-value predicate (ADR-0104 D1): + inherently-multi option types plus multi-capable types flagged + `multiple: true`. Structured-JSON fields are untouched — `[]` there is a + document, not an emptied set. Populated arrays and non-required + multi-value fields are untouched. diff --git a/packages/objectql/src/validation/record-validator.test.ts b/packages/objectql/src/validation/record-validator.test.ts index 3b12e6963c..a9af1a7b25 100644 --- a/packages/objectql/src/validation/record-validator.test.ts +++ b/packages/objectql/src/validation/record-validator.test.ts @@ -547,6 +547,109 @@ describe('validateRecord — ADR-0113 required write contract on update', () => }); }); +/** + * #9476 — the enforcement half of the #9447 maintainer ruling (2026-08-18): + * `required` on a multi-value field means NON-EMPTY array. The empty set is + * representable — it reads back as `[]`, never `null` — so `required` judges + * emptiness: an explicit `[]` is an empty value exactly as `null` / `''` are. + * + * Scope is the spec's own multi-value predicate (`isMultiValueField`, + * ADR-0104 D1): inherently-multi option types, plus multi-capable types + * flagged `multiple: true`. Structured-JSON types stay OUT — `[]` there is a + * legitimate document, not an emptied set (pinned below). + * + * Envelope: `ValidationError` (`code: 'VALIDATION_FAILED'`) carrying a + * `fields[]` entry with `code: 'required'` — the class the REST layer maps + * to HTTP 400 (packages/rest `error-response.ts`), exactly as every other + * required refusal already travels. + */ +describe('validateRecord — required judges array emptiness on multi-value fields (#9476)', () => { + const schema: any = { + fields: { + members: { type: 'lookup', reference: 'sys_user', multiple: true, required: true }, + }, + }; + + it('INSERT: `[]` on a required `multiple: true` lookup is rejected — full envelope pin', () => { + let err: any; + try { + validateRecord(schema, { members: [] }, 'insert'); + } catch (e) { + err = e; + } + expect(err).toBeInstanceOf(ValidationError); + expect(err.code).toBe('VALIDATION_FAILED'); // → HTTP 400 via rest error-response mapping + expect(err.fields).toHaveLength(1); + expect(err.fields[0]).toMatchObject({ field: 'members', code: 'required' }); + expect(err.fields[0].message).toMatch(/is required/); + }); + + it('UPDATE: supplying `[]` for a required multi-value field is an explicit clear — rejected', () => { + let err: any; + try { + validateRecord(schema, { members: [] }, 'update'); + } catch (e) { + err = e; + } + expect(err).toBeInstanceOf(ValidationError); + expect(err.code).toBe('VALIDATION_FAILED'); + expect(err.fields).toHaveLength(1); + expect(err.fields[0]).toMatchObject({ field: 'members', code: 'required' }); + // The clear-out case keeps its DISTINCT sentence (`required_cleared`). + expect(err.fields[0].message).toMatch(/required and cannot be cleared/); + }); + + it('control: a populated array still lands on insert AND update — the check must not over-fire', () => { + expect(() => validateRecord(schema, { members: ['u1'] }, 'insert')).not.toThrow(); + expect(() => validateRecord(schema, { members: ['u1', 'u2'] }, 'update')).not.toThrow(); + }); + + it('control: `null` keeps its two existing reasons — `required` on insert, `required_cleared` on update', () => { + let ins: any; + let upd: any; + try { + validateRecord(schema, { members: null }, 'insert'); + } catch (e) { + ins = e; + } + try { + validateRecord(schema, { members: null }, 'update'); + } catch (e) { + upd = e; + } + expect(ins).toBeInstanceOf(ValidationError); + expect(ins.fields[0]).toMatchObject({ field: 'members', code: 'required' }); + expect(ins.fields[0].message).toMatch(/is required/); + expect(ins.fields[0].message).not.toMatch(/cannot be cleared/); + expect(upd).toBeInstanceOf(ValidationError); + expect(upd.fields[0]).toMatchObject({ field: 'members', code: 'required' }); + expect(upd.fields[0].message).toMatch(/required and cannot be cleared/); + }); + + it('an inherently-multi option type (`multiselect`) is judged the same way', () => { + const s: any = { fields: { labels: { type: 'multiselect', required: true, options: ['a', 'b'] } } }; + expect(() => validateRecord(s, { labels: [] }, 'insert')).toThrow(/is required/); + expect(() => validateRecord(s, { labels: ['a'] }, 'insert')).not.toThrow(); + }); + + it('control: `[]` on a NON-required multi-value field still passes — emptiness is only judged under `required`', () => { + const s: any = { fields: { accounts: { type: 'lookup', reference: 'acct', multiple: true } } }; + expect(() => validateRecord(s, { accounts: [] }, 'insert')).not.toThrow(); + expect(() => validateRecord(s, { accounts: [] }, 'update')).not.toThrow(); + }); + + it('control: an UPDATE that omits the required multi-value field never 400s — legacy rows rest', () => { + const s: any = { fields: { members: schema.fields.members, notes: { type: 'textarea' } } }; + expect(() => validateRecord(s, { notes: 'touched only this' }, 'update')).not.toThrow(); + }); + + it('control: structured JSON stays out — `[]` on a required `json` field is a document, not an emptied set', () => { + const s: any = { fields: { payload: { type: 'json', required: true } } }; + expect(() => validateRecord(s, { payload: [] }, 'insert')).not.toThrow(); + expect(() => validateRecord(s, { payload: [] }, 'update')).not.toThrow(); + }); +}); + /** * #3957 — a rejected write must name the field the way the USER knows it, in * the language they read, and must hand a client the constraint as data. diff --git a/packages/objectql/src/validation/record-validator.ts b/packages/objectql/src/validation/record-validator.ts index e77356c593..12971aed10 100644 --- a/packages/objectql/src/validation/record-validator.ts +++ b/packages/objectql/src/validation/record-validator.ts @@ -18,7 +18,9 @@ * - `required` ADR-0113 write contract: on INSERT a missing/null/empty * value is rejected; on UPDATE a SUPPLIED missing value is * rejected (a PATCH may not null out a required field) while - * an omitted field never 400s — legacy null rows rest. + * an omitted field never 400s — legacy null rows rest. On a + * multi-value field `[]` is an empty value (#9476 — the + * #9447 ruling: required means non-empty array). * - `maxLength` / `minLength` (text/textarea/email/url/phone/password) * - `min` / `max` (number/currency/percent/rating/slider) * - `scale` more decimal places than declared → `max_scale` (#7501; @@ -172,6 +174,24 @@ function isMissing(v: unknown): boolean { return v === undefined || v === null || (typeof v === 'string' && v.trim() === ''); } +/** + * #9476 — the emptiness judgment the `required` check runs, def-aware where + * `isMissing` is not. The #9447 maintainer ruling (2026-08-18) sets the + * contract: `required` on a multi-value field means NON-EMPTY array — the + * empty set is representable (it reads back as `[]`, never `null`), so an + * explicit `[]` is an empty value exactly as `null` / `''` are. + * + * Judged only where the DECLARED value is a set (`isMultiValueField`, + * ADR-0104 D1): a structured-JSON field's `[]` is a legitimate document, not + * an emptied set, and every other type keeps the def-free `isMissing` + * semantics. Only the two `required` read sites call this — a bare `[]` on a + * non-required multi-value field still flows to the array-shape branch. + */ +function isEmptyForRequired(def: FieldDef, value: unknown): boolean { + if (isMissing(value)) return true; + return isMultiValueField(def) && Array.isArray(value) && value.length === 0; +} + /** * How many decimal places a finite number carries (#7501). * @@ -474,7 +494,7 @@ function validateOne( // `autonumber` is runtime-owned: the value is generated by the engine / // driver (the SQL driver assigns it from a persistent sequence AFTER this // validation runs), so a missing value is never a client error — see #1603. - if (!skipRequired && def.required && isMissing(value) && def.type !== 'autonumber') { + if (!skipRequired && def.required && isEmptyForRequired(def, value) && def.type !== 'autonumber') { return fail('required'); } if (isMissing(value)) return null; // nothing else to check @@ -1008,7 +1028,7 @@ export function validateRecord( // write that does not touch the field never reaches this check. (The // one over-approximation — explicit null onto an already-null legacy // row — is rejected too; that write was a no-op plus a false claim.) - if (def.required && isMissing(value) && def.type !== 'autonumber') { + if (def.required && isEmptyForRequired(def, value) && def.type !== 'autonumber') { // Same catalog as every other built-in message (#3957) — one wire code // (`required`), a distinct sentence for the clear-out case. errors.push(buildFieldError(