diff --git a/.changeset/22445-update-preview-stored-row.md b/.changeset/22445-update-preview-stored-row.md new file mode 100644 index 00000000000..b0a1cb72449 --- /dev/null +++ b/.changeset/22445-update-preview-stored-row.md @@ -0,0 +1,21 @@ +--- +'@objectstack/objectql': minor +'@objectstack/core': minor +'@objectstack/spec': minor +--- + +An `update`-mode `validate()` preview judges the stored row merged with the patch, as the by-id update does, so an import dry run of a matched row admits the rows the import's update admits (#22445) + +Clause-②: yes + +The preview's accept set widens: rows it refused, and the write admits, are now admitted. No key, export or error code is added, removed or renamed. + +- **Before.** An `update`-mode preview (`engine.validate(object, rows, { mode: 'update' })`, `validateData`, and the import dry run of a row that matched a stored record) read no stored row, so the record its rules judged held only the patch. A `requiredWhen`, an option `visibleWhen` or a `validations[]` rule that reads a column the patch omits faulted there, and the preview refused the row with `rule_violation` / `unevaluable`, although the real update reads the stored row first and admits it. The refusal also said the omitted column was one "which this object does not declare", sending the author looking for a field that exists. +- **Now.** A row that carries its `id` (the address every update door already folds into the payload) is judged against the row that id names: the stored row is the rules' `previous`, and their record is the stored row merged with the patch, exactly as the by-id update judges it. A reference field a traversing rule reads is resolved from that merge too. The import dry run of a matched row sends the matched record's id this way, so nothing new crosses the protocol. +- **Read under the caller's access.** The engine's read door, under the caller's own context, decides whether there is a row to judge: a row the caller cannot read gets the verdict a missing row gets. The values judged are the row as stored, read the way the by-id update reads its prior row (so a formula column, which has no stored value, is judged as the update judges it). A stored column is judged only where the read door served this caller that very value. A column the read door hid, or served transformed (a partially masked field, a masked secret), is judged as empty, so the verdict never depends on a value the caller could not read in full. File fields are compared in their stored form, so a readable file reference is judged as the id it holds. Measured on `driver-sql` (SQLite) and `driver-memory`, a plain readable column (number, currency, percent, boolean, date, datetime, JSON, multiselect, select, text) reaches the rules as its stored value. +- **Unchanged.** A row with no `id`, or whose id names no row the caller can read, is judged on the patch alone, as before. `required` and value checks still look only at the supplied keys, matching a PATCH. A merge that really violates a rule is refused, as the write refuses it. The preview still runs no `readonlyWhen` or primary-key strip, so it can report fewer dropped fields than the update. +- **The refusal names the real cause.** Where a rule reads a column the object declares and the judged record does not hold it (a preview with no stored row), the refusal now says the value was not supplied and no stored row was read, instead of saying the object does not declare the column. A column the object really does not declare keeps the old sentence. + +`@objectstack/core`: the import runner's dry run passes the matched record's id with an update row. `@objectstack/spec`: the `ValidateDataRequest.mode` description and the `ValidateDataResponse` note say what an `update` preview now reads. + +The pending entry for the option-gate change in this release says an `update`-mode preview reads no stored row and refuses a cascade pick whose parent column the patch omits. That describes the preview before this change: with the row's `id`, the preview now reads the stored row and judges the pick as the write does. diff --git a/content/docs/references/api/protocol.mdx b/content/docs/references/api/protocol.mdx index 8ec5ecee16c..f05b0e279e8 100644 --- a/content/docs/references/api/protocol.mdx +++ b/content/docs/references/api/protocol.mdx @@ -2985,7 +2985,7 @@ A write-path strip event: caller-supplied fields legally dropped from the payloa | :--- | :--- | :--- | :--- | | **object** | `string` | ✅ | The object name. | | **data** | `Record \| Record[]` | ✅ | A candidate record, or an array of them. Nothing is persisted. | -| **mode** | `Enum<'insert' \| 'update'>` | optional | Which write the verdict should predict. `insert` (default) walks every declared field, so a missing required field is a finding; `update` judges only the supplied keys, matching a PATCH. | +| **mode** | `Enum<'insert' \| 'update'>` | optional | Which write the verdict should predict. `insert` (default) walks every declared field, so a missing required field is a finding; `update` checks only the supplied keys, matching a PATCH. An `update` row that carries its `id` is judged as the by-id update judges it: its rules read the stored row that id names, merged with the supplied keys. The stored row is read with the caller's own read access, so a row the caller cannot read is judged on the supplied keys alone, as a row with no `id` is. | --- diff --git a/packages/core/src/utils/import-runner-update-preview-address.test.ts b/packages/core/src/utils/import-runner-update-preview-address.test.ts new file mode 100644 index 00000000000..7720992745e --- /dev/null +++ b/packages/core/src/utils/import-runner-update-preview-address.test.ts @@ -0,0 +1,83 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#22445] The dry run of an update asks `validateData` about the row the + * update would write: the matched record's id rides in the payload exactly as + * the write's `updateData` folds it (`{ ...data, id }`), so the engine's + * `update`-mode preview can read that stored row and judge the merge. + * + * Driven against `ImportProtocolLike` doubles, recording what each call was + * asked. End to end over a real engine and the real protocol, where the + * preview reads the stored row: `packages/objectql/src/validate-update-stored-row.test.ts`. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { runImport, type ImportProtocolLike } from './import-runner'; +import type { ExportFieldMeta } from './import-field-meta.js'; + +type ValidateArgs = Parameters>[0]; +type UpdateArgs = Parameters[0]; + +const OBJECT = 'task'; +const metaMap = new Map([ + ['code', { name: 'code', type: 'text' }], + ['title', { name: 'title', type: 'text' }], +]); + +const baseOpts = { + objectName: OBJECT, + metaMap, + matchFields: ['code'], + runAutomations: false, + trimWhitespace: true, + createMissingOptions: false, + skipBlankMatchKey: false, +}; + +function protocol(matched: Record[]) { + const validateData = vi.fn(async (args: ValidateArgs) => ({ + object: OBJECT, mode: args.mode ?? 'insert', valid: true, + results: [{ valid: true, errors: [], warnings: [] }], + posture: { valueShapeStrict: false, mediaValueShapeStrict: false }, + })); + const updateData = vi.fn(async (args: UpdateArgs) => ({ object: OBJECT, id: args.id, record: { ...args.data, id: args.id } })); + const p: ImportProtocolLike = { + findData: vi.fn(async () => matched.map((r) => ({ ...r }))), + createData: vi.fn(), + updateData, + validateData, + }; + return { p, validateData, updateData }; +} + +describe('[#22445] the dry run of a matched row names the stored row it would update', () => { + it('an update row is previewed with the matched record\'s id in its payload', async () => { + const { p, validateData } = protocol([{ id: 'rec_7', code: 'c7', title: 'stored' }]); + const s = await runImport({ ...baseOpts, p, writeMode: 'update', dryRun: true, rows: [{ code: 'c7', title: 'new' }] }); + + expect(s.results[0]).toMatchObject({ ok: true, action: 'updated', id: 'rec_7' }); + expect(validateData).toHaveBeenCalledTimes(1); + expect(validateData.mock.calls[0]![0]).toMatchObject({ mode: 'update', data: { code: 'c7', title: 'new', id: 'rec_7' } }); + }); + + it('the matched id wins over an id cell, as the write\'s fold makes it win', async () => { + const { p, validateData, updateData } = protocol([{ id: 'rec_7', code: 'c7' }]); + const metaWithId = new Map(metaMap).set('id', { name: 'id', type: 'text' }); + const row = { code: 'c7', id: 'other', title: 'new' }; + + await runImport({ ...baseOpts, metaMap: metaWithId, p, writeMode: 'update', dryRun: true, rows: [row] }); + expect((validateData.mock.calls[0]![0].data as Record).id).toBe('rec_7'); + + // The write binds the matched id too (the protocol folds `{ ...data, id }`). + await runImport({ ...baseOpts, metaMap: metaWithId, p, writeMode: 'update', dryRun: false, rows: [row] }); + expect(updateData.mock.calls[0]![0]).toMatchObject({ id: 'rec_7' }); + }); + + it('control: a create row is previewed in insert mode, with no id added', async () => { + const { p, validateData } = protocol([]); + await runImport({ ...baseOpts, p, writeMode: 'upsert', dryRun: true, rows: [{ code: 'c8', title: 'fresh' }] }); + + expect(validateData.mock.calls[0]![0]).toMatchObject({ mode: 'insert' }); + expect(validateData.mock.calls[0]![0].data).toEqual({ code: 'c8', title: 'fresh' }); + }); +}); diff --git a/packages/core/src/utils/import-runner.ts b/packages/core/src/utils/import-runner.ts index fc8e26c964d..6209c50a87a 100644 --- a/packages/core/src/utils/import-runner.ts +++ b/packages/core/src/utils/import-runner.ts @@ -1022,7 +1022,18 @@ export function runImport(opts: RunImportOptions): Promise { // The write path needs no counterpart: `validateRecord` runs // there for real, after the hooks, and the row report is built // from the very same findings by `toFailedResult`. - const verdict = await previewVerdict(data, willUpdate ? 'update' : 'insert', rowCtx); + // + // [#22445] An update is asked about the row it would write: the + // matched record's id rides in the payload exactly as the + // write's `updateData` folds it (`{ ...data, id }`), and an + // `update`-mode preview reads the row that id names, under this + // row's context, and judges the stored row merged with the + // patch, as the by-id update does. Nothing new crosses the + // protocol: the id is the address every update door already + // carries in the payload. + const verdict = willUpdate + ? await previewVerdict({ ...data, id: (existing as Record).id }, 'update', rowCtx) + : await previewVerdict(data, 'insert', rowCtx); if (verdict && !verdict.valid) { errCount++; const first = verdict.errors[0]; diff --git a/packages/objectql/src/cel-fault.ts b/packages/objectql/src/cel-fault.ts index b530ac2a553..f78c9e5fb23 100644 --- a/packages/objectql/src/cel-fault.ts +++ b/packages/objectql/src/cel-fault.ts @@ -24,7 +24,10 @@ * 1. What broke, in one line? (`summary`) * 2. Did the expression read a key the record does not carry, and which? * (`missingKey` — after materialisation this can only mean an UNDECLARED - * key, i.e. an author typo or a retired field) + * key, i.e. an author typo or a retired field; on a record that was NOT + * materialised — an update judged without its stored row — it can also + * be a declared key the patch did not supply, which a caller holding the + * declared field set tells apart through `isDeclaredColumn`) * 3. Did it name a ROOT that is not in scope at all? (`unknownVariable` — * the fault an unbound `previous` produces, which is a different * diagnosis from a missing key on a bound root) @@ -106,6 +109,18 @@ export interface CelFaultSubject { what: string; /** How to fix an undeclared key, e.g. `"fix the rule's condition, or declare the field"`. */ undeclaredKeyFix: string; + /** + * [#22445] Whether a missing key is a column THIS object declares, read + * directly off the judged record. A record is total only when it was + * materialised — on insert, or on an update whose stored row was read. An + * update judged without its stored row (an `update`-mode preview that names + * no row) holds only the keys the patch supplied, so a declared column the + * patch omits faults as `No such key` too, and "this object does not + * declare" would send the author looking for a field that exists. A caller + * that holds the object's declared field set answers this; one that does + * not omits it and keeps the undeclared-key sentence. + */ + isDeclaredColumn?: (key: string) => boolean; } export interface CelFaultDescription { @@ -134,7 +149,11 @@ export function describeCelFault(error: CelFault, subject: CelFaultSubject): Cel const unknownVariable = missingKey ? undefined : unknownVariableOf(error); const nullOverload = isNullOverloadFault(error); let detail = ''; - if (missingKey) { + if (missingKey && subject.isDeclaredColumn?.(missingKey) === true) { + detail = + ` The ${subject.what} reads '${missingKey}', a field this object declares, but its value was not supplied` + + ' and no stored row was read to supply it.'; + } else if (missingKey) { detail = ` The ${subject.what} reads '${missingKey}', which this object does not declare` + ` — ${subject.undeclaredKeyFix}.`; diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index cbe25e03f77..5ca77a148de 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -3208,6 +3208,45 @@ export interface EngineReadOptions { } /** Merge read-path execution context from the query and the trailing options. */ +/** + * [#22445] Did the read door serve this value AS STORED — can the caller read + * the stored value in full? The `update`-mode preview keeps a stored column + * only when this holds for it (see `storedRows` in `ObjectQL.validate`). + * + * Both values come off the same driver read shape (`driver.findOne` under the + * same `buildDriverOptions`), so a column nothing transformed compares equal + * by construction; what differs is what the read path REWROTE for this caller + * (a partial mask, a masked secret, a read hook's rewrite). Structural, not + * identity: the two reads are two driver calls, so a `Date`, an array or a + * JSON object arrives as two equal objects. + * + * ⛔ Fails CLOSED: a pair it cannot prove equal — another object kind (a + * buffer, a class instance), a nesting past the depth bound — answers `false`, + * and the column is judged as empty. That direction can only make the preview + * refuse or admit on less than the write knows; the other direction would + * judge a value the caller may not read. + */ +function servedAsStored(served: unknown, stored: unknown, depth = 0): boolean { + if (Object.is(served, stored)) return true; + if (depth >= 16) return false; + if (served instanceof Date || stored instanceof Date) { + return served instanceof Date && stored instanceof Date && served.getTime() === stored.getTime(); + } + if (Array.isArray(served) || Array.isArray(stored)) { + return Array.isArray(served) && Array.isArray(stored) && served.length === stored.length + && served.every((item, i) => servedAsStored(item, stored[i], depth + 1)); + } + const plain = (v: unknown): v is Record => { + if (v === null || typeof v !== 'object') return false; + const proto = Object.getPrototypeOf(v); + return proto === Object.prototype || proto === null; + }; + if (!plain(served) || !plain(stored)) return false; + const servedKeys = Object.keys(served); + return servedKeys.length === Object.keys(stored).length + && servedKeys.every((k) => Object.prototype.hasOwnProperty.call(stored, k) && servedAsStored(served[k], stored[k], depth + 1)); +} + function mergeReadContext( fromQuery?: ExecutionContext, fromOptions?: ExecutionContext, @@ -12982,9 +13021,35 @@ export class ObjectQL implements IObjectQLEngine { * case of a hook deriving a *business* field that its object also * validates. * - * Nothing is written, no sequence is consumed, and no driver is touched — - * validation is in-process, which is what makes row-by-row dry run of a - * large import affordable. + * ## An `update`-mode preview judges the stored row (#22445) + * + * A by-id update reads its prior row and judges the stored row merged with + * the patch, so a rule that reads a column the patch omits sees the stored + * value. A preview that judged the patch alone refused rows the write + * admits: the omitted column was missing from the judged record, and the + * rule faulted. So a row that carries its address — the `id` every update + * door folds into the payload (`updateData`), classified by the write's + * own `resolveEngineUpdateDispatch` — is judged against the row that id + * names, as the by-id update judges it: the stored row is the rules' + * `previous`, and their record is that row merged with the patch. + * + * ⛔ The READ DOOR decides whether there is a row to judge: it is asked + * first, under the caller's own context (`findOne`). A row the caller + * cannot read is a row that does not exist (the by-id write answers 404 for + * it before it reads anything), so the preview judges no row for it and + * gives no verdict about its columns. The image itself is the write's own + * prior-row read ({@link readUpdatePriorRow}), the row as STORED, kept to + * the columns the read door served this caller UNCHANGED: a column it hid, + * and one it served transformed (a partial mask keeps the key and replaces + * the value), is judged as empty, so the verdict never depends on a value + * the caller could not have read in full. A refused read propagates as the + * refusal it is. A row with no address, or whose id names no row this + * caller can read, is judged on the patch alone, as before. + * + * Nothing is written and no sequence is consumed. Reads do happen: the + * stored row above, and the related rows a traversing rule names (see the + * `previewRelatedForRow` block below). Validation itself is in-process, + * which is what makes row-by-row dry run of a large import affordable. */ async validate( object: string, @@ -13022,8 +13087,13 @@ export class ObjectQL implements IObjectQLEngine { // ⚠️ "Nothing is executed" is no longer literally true and must not be // restated as if it were: a traversing validation rule needs its related // rows, so this operation issues a READ per reference field the rules name - // (see the `previewRelatedForRow` block below). Nothing is written, no hook - // runs, and the read happens only for a caller who passes the arms the + // (see the `previewRelatedForRow` block below), and [#22445] an + // `update`-mode row that carries its address issues two READS of the + // stored row — the read door under the caller's own context, then the + // write's own prior-row read (the `storedRows` block below). Nothing is + // written, no write hook runs (a read through the read door fires its read + // hooks, as the related read always has), and the + // RELATED read happens only for a caller who passes the arms the // write-gate probe RUNS — ⛔ never a category of the write decision, and // ⛔ not a promise the write would succeed (`registerWriteGateProbe` names // those arms, and the families it does not carry). @@ -13088,12 +13158,13 @@ export class ObjectQL implements IObjectQLEngine { // that verb. No logger: a preview writes nothing, and a strip line says // the value was not stored. // - // ⚠️ An `update`-mode preview has no bound row, so a supplied `id` is read - // as the address the write binds — every update door folds the target id - // into the payload (`updateData`) — and is never judged as a payload key - // (#8093, ADDRESSING IS NOT PAYLOAD). `readonlyWhen` and the primary-key - // strip are not run: both judge a prior record or a dispatch this - // operation does not have (the named limits below). + // ⚠️ On `update`, a supplied `id` is read as the address the write binds — + // every update door folds the target id into the payload (`updateData`) — + // and is never judged as a payload key (#8093, ADDRESSING IS NOT PAYLOAD). + // [#22445] That address now names the stored row the rules judge (the + // `storedRows` block below). `readonlyWhen` and the primary-key strip are + // still not run: they remain the named limit `ValidateDataResponse` + // states, so the preview can report fewer drops than the update. // // [#20922] Each strip's result is recorded twice at the strip itself: into // the batch-level union the listener reports (unchanged), and into the @@ -13167,6 +13238,78 @@ export class ObjectQL implements IObjectQLEngine { const currentUser = this.buildEvalUser(options?.context); const skipStateMachine = shouldSkipStateMachine(options?.context); + // [#22445] THE STORED ROW an `update`-mode row is judged against — the + // by-id update's prior row, read here for the same reason the write reads + // it: a rule that reads a column the patch omits judges the stored value, + // so the rules' record is the stored row merged with the patch and their + // `previous` is the stored row (`evaluateValidationRules` below, called as + // the by-id branch of `update()` calls it). + // + // The address is the write's own: `resolveEngineUpdateDispatch`, the + // ladder `update()` resolves its by-id branch with, asked of the row as + // submitted. Two reads, each answering one question: + // + // 1. ⛔ MAY THIS CALLER SEE THE ROW — the READ DOOR, under the caller's + // own context (`findOne`). The by-id write reads its row raw only after + // its middleware's write gates, which answer 404 for a row the caller + // cannot read; the preview runs none of them. So a row the read door + // does not return is a missing row here too: no image, no verdict about + // its columns. A refused read propagates as raised. + // 2. WHAT IS STORED — the write's own prior-row read + // ({@link readUpdatePriorRow}), so the rules judge the row as STORED, + // exactly as the write does, and not the read door's served image (a + // formula the read door evaluates has no stored column, and the write + // judges it absent). + // + // ⛔ A stored column is kept ONLY where the read door served this caller + // that very value ({@link servedAsStored}): the caller can already read it + // IN FULL. Every other column is left out and judged as empty — one the + // read door hid (field-level security deletes the key), and one it served + // TRANSFORMED: a partial mask, which keeps the key and replaces the value, + // a masked secret, an `internal` column, a read hook's rewrite. A verdict + // over a masked column's stored value is the probe the security plugin + // refuses on a query (a partially masked field is as probe-able as a + // hidden one), and the preview runs none of the write's gates, so it must + // not answer one. So the verdict never depends on a value this caller + // could not have read. The read door is asked for file fields in their + // stored form (`RAW_FILE_VALUES_CONTEXT_KEY`, the declared opt-out for a + // caller whose subject is the stored form), so a readable file reference + // compares as the id it is rather than as the expanded object. + // + // `undefined` = no stored row: the row is judged on the patch alone, as + // it was before these reads existed. + const storedRows: Array | undefined> = []; + for (const submitted of submittedRows) { + let stored: Record | undefined; + if (mode === 'update' && submitted && typeof submitted === 'object') { + const dispatch = resolveEngineUpdateDispatch(submitted as EngineUpdateDispatchData, undefined); + if (dispatch.kind === 'by-id') { + const visible = await this.findOne(object, { + where: { id: dispatch.id }, + context: { ...(options?.context ?? {}), [RAW_FILE_VALUES_CONTEXT_KEY]: true }, + } as EngineQueryOptions); + const prior = visible && typeof visible === 'object' + ? await this.readUpdatePriorRow(object, dispatch.id, options?.context) + : null; + if (visible && prior) { + const served = visible as Record; + stored = {}; + for (const [key, value] of Object.entries(prior)) { + if (Object.prototype.hasOwnProperty.call(served, key) && servedAsStored(served[key], value)) { + stored[key] = value; + } + } + } + } + } + storedRows.push(stored); + } + // The merged view each rule evaluates, and the one the reference reads + // below key on: the write reads a traversed FK off the same merge + // (`updateView` in `update()`), so a FK the patch omits resolves from the + // stored row. + const judgedViews = rows.map((row, i) => (storedRows[i] ? { ...storedRows[i], ...row } : row)); + // [#18682] The preview owes the SAME relationship resolution the real write // does. Without it a rule that reads one hop through a reference field // reports `valid: false` (unevaluable) against a row `insert()` happily @@ -13174,12 +13317,12 @@ export class ObjectQL implements IObjectQLEngine { // import dry run rides on it. // // Resolved once for the whole set, like every other posture input above. - // ⚠️ Named limit, not widened here: an `update`-mode preview - // carries no prior row (nothing is read), so a traversing rule whose FK the - // PATCH does not itself carry has no id to resolve and still refuses. The - // real update path reads the prior row and does resolve it; closing the - // preview's half needs a read this operation's "nothing is executed" - // contract does not make. + // [#22445] The named limit that sat here — an `update`-mode preview read + // no prior row, so a traversing rule whose FK the PATCH did not carry had + // no id to resolve — is CLOSED for a row that carries its address: the FK + // is read off `judgedViews`, the stored row merged with the patch. A row + // with no address (or no stored row this caller can read) still resolves + // only the FKs its patch carries. // [#20805] The second named limit that used to sit here is CLOSED: a // reference field the author declared static `readonly` is stripped from a // non-system caller's payload by the write, so the write resolves no @@ -13203,7 +13346,7 @@ export class ObjectQL implements IObjectQLEngine { ? await this._writeGateProbe(object, mode, options?.context, submittedRows).catch(() => false) : true; const previewRelatedForRow = mayWrite - ? await this.resolvePredicateRelated(schemaForValidation, rows, options?.context) + ? await this.resolvePredicateRelated(schemaForValidation, judgedViews, options?.context) : () => undefined; // [#18783] The preview answers a `can`-gated option with the SAME map the // write would — resolved once for the whole set, like every posture input @@ -13244,8 +13387,11 @@ export class ObjectQL implements IObjectQLEngine { keptOptionValues: options?.context?.keptOptionValues, }); evaluateValidationRules(schemaForValidation as any, row, mode, { + // [#22445] The stored row, when this row named one: the by-id + // update's `previous: priorRecord`. Absent, the record is the patch. + ...(storedRows[i] ? { previous: storedRows[i] } : {}), logger: this.logger, currentUser, skipStateMachine, messages, - related: previewRelatedForRow(row), + related: previewRelatedForRow(judgedViews[i]), permissions: previewPermissionsFor(row), }); } catch (e) { @@ -13284,6 +13430,41 @@ export class ObjectQL implements IObjectQLEngine { }; } + /** + * [#22445] The by-id update's prior-row read: the STORED row an update of + * `id` judges, or `null` when the store holds none. One spelling, read by + * `update()`'s by-id branch and by the `update`-mode preview + * ({@link ObjectQL.validate}), so the preview judges the image the write + * judges rather than a second reading of it. + * + * A raw driver read, not the read door: the rules judge the row as STORED, + * so no formula is evaluated, no file reference resolved, no read hook run. + * ⛔ It answers no access question — `update()` reaches it only after the + * write gates in its middleware chain, and the preview asks the read door + * first (see `storedRows` in `validate()`). + * + * `buildDriverOptions` carries the open transaction and the tenant scope onto + * the raw read, as on every engine read. [#21613] The row is shaped like + * every other row this engine reads off a driver: the declared field set, so + * the update's `previous` (bound for its hooks, and the audit ledger's side + * of every update diff) and its returned row are ONE view. Shaping only the + * returned row would make every update of a row carrying a retired column + * record that column as changed, its stored value included. + */ + private async readUpdatePriorRow( + object: string, + id: unknown, + context: ExecutionContext | undefined, + base?: unknown, + ): Promise | null> { + const priorAst: QueryAST = { object, where: { id }, limit: 1 } as QueryAST; + const preOpts = this.buildDriverOptions(object, context, base as any); + return withDeclaredColumnsOnly( + await this.getDriver(object).findOne(object, priorAst, preOpts) as Record | null, + declaredColumnSet(this._registry.getObject(object)), + ); + } + /** * Create one record, or a batch of them. * @@ -15072,18 +15253,10 @@ export class ObjectQL implements IObjectQLEngine { // post-phase write uses, built here because the write's own // merge has not happened yet. `delete()`'s pre-image read does // the same for the same reason. - const priorAst: QueryAST = { object, where: { id }, limit: 1 }; - const preOpts = this.buildDriverOptions(object, opCtx.context, hookContext.input.options as any); - // [#21613] The pre-image is shaped like every other row this engine - // reads off a driver: the declared field set, so `previous` (bound - // for the hooks below, and the audit ledger's side of every update - // diff) and the write's returned row are ONE view. Shaping only the - // returned row would make every update of a row carrying a retired - // column record that column as changed, its stored value included. - priorRecord = withDeclaredColumnsOnly( - await driver.findOne(object, priorAst, preOpts) as Record | null, - declaredColumnSet(updateSchema), - ); + // [#22445] Through {@link readUpdatePriorRow}, the ONE spelling of + // this read, which the `update`-mode preview reads its stored row + // with too — so the preview and this write judge one image. + priorRecord = await this.readUpdatePriorRow(object, id, opCtx.context, hookContext.input.options); // ── [#7867] The not-found gate ────────────────────────────────── // // A by-id update whose id names no row was a SILENT NO-OP that diff --git a/packages/objectql/src/validate-update-stored-row.test.ts b/packages/objectql/src/validate-update-stored-row.test.ts new file mode 100644 index 00000000000..5430bf069f6 --- /dev/null +++ b/packages/objectql/src/validate-update-stored-row.test.ts @@ -0,0 +1,480 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #22445 — an `update`-mode `validate()` preview judges the STORED ROW merged +// with the patch, the record the by-id update judges. +// +// ## The defect +// +// The preview read no stored row, so the record its rules judged held only +// the patch. A rule that reads a column the patch omits faulted there +// (`No such key`), and the preview refused a row that the real by-id update +// admits, because the update reads its prior row first. The refusal then said +// the omitted column was one "which this object does not declare", sending the +// author looking for a field that exists. +// +// ## The fix +// +// A row that carries its address — the `id` every update door folds into the +// payload — is judged against the row that id names, read through the READ +// DOOR under the caller's own context. With no stored row (no address, or an id +// naming no row this caller can read) the row is judged on the patch alone, and +// the refusal names the real cause: the value was not supplied. +// +// The fixture is the showcase's `showcase_cascade` shape (a `country` → +// `province` cascade whose option `visibleWhen`s read `country`), plus a +// `note` whose `requiredWhen` reads `country`. +// +// ## Pins +// +// (a) the two preview calls the card measured admit over a stored +// `country: 'cn'` row, and the by-id update admits the same patches; +// (b) control: a preview whose merge really violates a rule still refuses, +// with the refusal the write gives; +// (c) with no stored row the refusal says the value was not supplied, never +// "does not declare"; an undeclared key keeps "does not declare"; +// (d) a caller who cannot read the stored row gets no verdict about its +// columns: the verdict is the one a missing row gets, whatever the hidden +// row holds; a column the caller may not read is judged as empty; +// (e) the real door: the import dry run of a matched row, through the +// protocol, admits what the import's write admits and refuses what it +// refuses. +// +// (a), (c) and (e) were red before the fix. + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { ObjectKernel, runImport, type ImportProtocolLike } from '@objectstack/core'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { ObjectQL } from './engine.js'; +import { ObjectQLPlugin } from './plugin.js'; +import { ValidationError } from './validation/record-validator.js'; + +const OBJECT = 'pv_cascade'; + +const CASCADE = { + name: OBJECT, + label: 'Preview Cascade', + fields: { + name: { name: 'name', label: 'Name', type: 'text' }, + owner_name: { name: 'owner_name', label: 'Owner', type: 'text' }, + country: { + name: 'country', label: 'Country', type: 'select', + options: [{ label: 'China', value: 'cn' }, { label: 'United States', value: 'us' }], + }, + province: { + name: 'province', label: 'Province', type: 'select', + options: [ + { label: 'Zhejiang', value: 'zj', visibleWhen: "record.country == 'cn'" }, + { label: 'California', value: 'ca', visibleWhen: "record.country == 'us'" }, + ], + }, + note: { name: 'note', label: 'Note', type: 'text', requiredWhen: "record.country == 'us'" }, + }, +}; + +/** A store-backed driver: what is stored is what a later read answers. */ +function makeStoreDriver() { + const rows = new Map>(); + const matches = (row: any, where: any): boolean => { + if (!where || typeof where !== 'object') return true; + return Object.entries(where).every(([k, v]: [string, any]) => { + if (k === '$and') return (v as any[]).every((w) => matches(row, w)); + if (k.startsWith('$')) throw new Error(`store driver: unsupported combinator ${k}`); + if (v !== null && typeof v === 'object' && !Array.isArray(v)) { + return Object.entries(v).every(([op, target]) => { + if (op === '$eq') return row?.[k] === target; + if (op === '$in') return Array.isArray(target) && target.includes(row?.[k]); + throw new Error(`store driver: unsupported operator ${op}`); + }); + } + return row?.[k] === v; + }); + }; + let n = 0; + const driver: any = { + name: 'pv-store', version: '0.0.0', supports: {}, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, + async syncSchema() {}, + async find(_o: string, ast: any) { + const hits = Array.from(rows.values()).filter((r) => matches(r, ast?.where)); + return (typeof ast?.limit === 'number' ? hits.slice(0, ast.limit) : hits).map((r) => ({ ...r })); + }, + async findOne(_o: string, ast: any) { + for (const r of rows.values()) if (matches(r, ast?.where)) return { ...r }; + return null; + }, + async create(_o: string, data: Record) { + n += 1; + const id = (data.id as string) ?? `rec_${n}`; + const row = { ...data, id }; + rows.set(id, row); + return { ...row }; + }, + async update(_o: string, id: string, data: Record) { + const row = { ...rows.get(id), ...data, id }; + rows.set(id, row); + return { ...row }; + }, + async delete(_o: string, id: string) { return rows.delete(id); }, + async count(_o: string, ast: any) { + return Array.from(rows.values()).filter((r) => matches(r, ast?.where)).length; + }, + }; + return { driver, rows }; +} + +function makeEngine(objects: any[] = [CASCADE]) { + const store = makeStoreDriver(); + const engine = new ObjectQL(); + engine.registerDriver(store.driver, true); + engine.registerApp({ id: 'pv', name: 'Preview Stored Row', objects } as any); + return { engine, rows: store.rows }; +} + +/** The one row's verdict, reduced to what a caller branches on. */ +async function verdictOf(engine: ObjectQL, data: Record, context?: Record) { + const out = await engine.validate(OBJECT, data, { mode: 'update', ...(context ? { context: context as any } : {}) }); + expect(out.results).toHaveLength(1); + return out.results![0]!; +} + +/** The field findings of a refused write, or `null` when the write landed. */ +async function writeRefusal(write: () => Promise) { + try { + await write(); + return null; + } catch (e) { + expect(e).toBeInstanceOf(ValidationError); + const err = e as ValidationError; + expect(err.code).toBe('VALIDATION_FAILED'); + return err.fields.map((f) => ({ field: f.field, code: f.code })); + } +} + +const NOT_DECLARED = 'which this object does not declare'; +const NOT_SUPPLIED = 'its value was not supplied'; + +describe('#22445 — an update-mode preview judges the stored row merged with the patch', () => { + let engine: ObjectQL; + let rows: Map>; + + beforeEach(() => { + ({ engine, rows } = makeEngine()); + rows.set('c1', { id: 'c1', name: 'one', country: 'cn' }); + }); + + describe('(a) the card\'s two preview calls admit over a stored country: cn row', () => { + it.each([ + ['a cascade pick', { province: 'zj' }], + ['a note', { note: 'x' }], + ])('%s — the preview admits it, and so does the by-id update', async (_label, patch) => { + const preview = await verdictOf(engine, { ...patch, id: 'c1' }); + expect(preview).toMatchObject({ valid: true, errors: [] }); + + expect(await writeRefusal(() => engine.update(OBJECT, { ...patch, id: 'c1' }))).toBeNull(); + expect(rows.get('c1')).toMatchObject({ country: 'cn', ...patch }); + }); + }); + + describe('(b) control: a merge that really violates a rule still refuses, as the write does', () => { + it.each([ + // The patch flips the condition true and does not supply the field. + ['a requiredWhen turned true', { country: 'us' }, [{ field: 'note', code: 'required' }]], + // The stored country makes the picked option invisible. + ['an option the stored row does not offer', { province: 'ca' }, [{ field: 'province', code: 'invalid_option' }]], + ])('%s', async (_label, patch, expected) => { + const preview = await verdictOf(engine, { ...patch, id: 'c1' }); + expect(preview.valid).toBe(false); + expect(preview.errors.map((e) => ({ field: e.field, code: e.code }))).toEqual(expected); + + expect(await writeRefusal(() => engine.update(OBJECT, { ...patch, id: 'c1' }))).toEqual(expected); + expect(rows.get('c1')).toEqual({ id: 'c1', name: 'one', country: 'cn' }); + }); + }); + + describe('(a) the image is the row as STORED, the one the write judges', () => { + it('a formula column the read door evaluates is judged as the write judges it', async () => { + const { engine: e2, rows: r2 } = makeEngine([{ + name: OBJECT, label: 'Preview Amount', + fields: { + amount: { name: 'amount', label: 'Amount', type: 'number' }, + doubled: { name: 'doubled', label: 'Doubled', type: 'formula', expression: 'record.amount * 2', returnType: 'number' }, + memo: { name: 'memo', label: 'Memo', type: 'text', requiredWhen: 'record.doubled > 100' }, + title: { name: 'title', label: 'Title', type: 'text' }, + }, + }]); + r2.set('a1', { id: 'a1', amount: 80 }); + // Precondition: the read door SERVES the formula, the store holds no column for it. + expect((await e2.findOne(OBJECT, { where: { id: 'a1' } }))?.doubled).toBe(160); + + const preview = await verdictOf(e2, { title: 't', id: 'a1' }); + const write = await writeRefusal(() => e2.update(OBJECT, { title: 't', id: 'a1' })); + expect(preview.errors.map((e) => ({ field: e.field, code: e.code }))).toEqual(write ?? []); + }); + }); + + describe('(c) with no stored row the refusal says the value was not supplied', () => { + it.each([ + ['no address', {}], + ['an id naming no row', { id: 'missing' }], + ])('%s — the requiredWhen and the option gate name the real cause', async (_label, address) => { + const preview = await verdictOf(engine, { province: 'zj', ...address }); + expect(preview.valid).toBe(false); + const byField = Object.fromEntries(preview.errors.map((e) => [e.field, e])); + for (const [field, rule] of [['note', 'requiredWhen'], ['province', 'visibleWhen']] as const) { + expect(byField[field]).toMatchObject({ + code: 'rule_violation', + constraint: expect.objectContaining({ rule, reason: 'unevaluable', missingKey: 'country' }), + }); + expect(byField[field]!.message).toContain(NOT_SUPPLIED); + expect(byField[field]!.message).not.toContain(NOT_DECLARED); + } + }); + + it('a validations[] rule reading the omitted column names the same cause', async () => { + const { engine: e2 } = makeEngine([{ + ...CASCADE, + validations: [ + { name: 'us_needs_note', type: 'script', severity: 'error', message: 'US rows need a note', + condition: "record.country == 'us' && record.note == null" }, + { name: 'us_branch', type: 'conditional', severity: 'error', message: 'never shown', + when: "record.country == 'us'", + then: { name: 'us_branch_then', type: 'script', severity: 'error', message: 'never', + condition: 'false' } }, + ], + }]); + const out = await e2.validate(OBJECT, { name: 'renamed' }, { mode: 'update' }); + // The engine's findings carry `constraint` beyond the wire triple the response type names. + const ruleOf = (e: object) => (e as { constraint?: { rule?: string } }).constraint?.rule; + const entries = out.results![0]!.errors.filter((e) => ruleOf(e) !== undefined && ruleOf(e) !== 'requiredWhen'); + expect(entries.map(ruleOf).sort()).toEqual(['us_branch', 'us_needs_note']); + for (const entry of entries) { + // A `validations[]` rule with no `fields` reports against the record. + expect(entry).toMatchObject({ + field: '_record', code: 'rule_violation', constraint: expect.objectContaining({ reason: 'unevaluable' }), + }); + expect(entry.message).toContain(NOT_SUPPLIED); + expect(entry.message).not.toContain(NOT_DECLARED); + } + }); + + it('control: a key the object does not declare keeps "does not declare"', async () => { + const { engine: e2 } = makeEngine([{ + ...CASCADE, + fields: { ...CASCADE.fields, note: { ...CASCADE.fields.note, requiredWhen: "record.contry == 'us'" } }, + }]); + const out = await e2.validate(OBJECT, { note: 'x' }, { mode: 'update' }); + const entry = out.results![0]!.errors.find((e) => e.field === 'note')!; + expect(entry).toMatchObject({ + code: 'rule_violation', + constraint: expect.objectContaining({ rule: 'requiredWhen', reason: 'unevaluable', missingKey: 'contry' }), + }); + expect(entry.message).toContain(NOT_DECLARED); + expect(entry.message).not.toContain(NOT_SUPPLIED); + }); + }); + + describe('(d) the stored row is read under the caller\'s read scope', () => { + /** Read scoping shaped like the security/sharing middlewares: a caller reads only its own rows. */ + function scopeReadsToOwner(ql: ObjectQL) { + ql.registerMiddleware(async (ctx: any, next: () => Promise) => { + const userId = ctx.context?.userId; + if (userId && !ctx.context?.isSystem && ['find', 'findOne'].includes(ctx.operation)) { + const scoped = { owner_name: userId }; + const ast: any = ctx.ast ?? { object: ctx.object }; + ast.where = ast.where ? { $and: [ast.where, scoped] } : scoped; + ctx.ast = ast; + } + await next(); + }); + } + + /** Field-level read masking shaped like the security middleware's result mask. */ + function hideCountryFrom(ql: ObjectQL, userId: string) { + ql.registerMiddleware(async (ctx: any, next: () => Promise) => { + await next(); + if (ctx.context?.userId !== userId || !['find', 'findOne'].includes(ctx.operation)) return; + const strip = (r: any) => { if (r && typeof r === 'object') delete r.country; }; + if (Array.isArray(ctx.result)) ctx.result.forEach(strip); + else strip(ctx.result); + }); + } + + it('the owner\'s preview judges the stored row', async () => { + scopeReadsToOwner(engine); + rows.set('c1', { id: 'c1', name: 'one', owner_name: 'alice', country: 'cn' }); + expect(await verdictOf(engine, { province: 'zj', id: 'c1' }, { userId: 'alice' })).toMatchObject({ valid: true }); + }); + + it('a caller who cannot read the row gets the verdict a missing row gets, whatever the row holds', async () => { + scopeReadsToOwner(engine); + const missing = await verdictOf(engine, { province: 'zj', id: 'nowhere' }, { userId: 'bob' }); + expect(missing.valid).toBe(false); + for (const country of ['cn', 'us']) { + rows.set('c1', { id: 'c1', name: 'one', owner_name: 'alice', country }); + expect(await verdictOf(engine, { province: 'zj', id: 'c1' }, { userId: 'bob' })).toEqual(missing); + } + }); + + it('a column the caller may not read is judged as empty, whatever it holds', async () => { + hideCountryFrom(engine, 'carol'); + const verdicts = []; + for (const country of ['cn', 'us']) { + rows.set('c1', { id: 'c1', name: 'one', country }); + verdicts.push(await verdictOf(engine, { province: 'zj', id: 'c1' }, { userId: 'carol' })); + } + expect(verdicts[0]).toEqual(verdicts[1]); + // Judged as empty: the cascade predicate answers a clean false, not a fault. + expect(verdicts[0]!.errors.map((e) => ({ field: e.field, code: e.code }))).toEqual([ + { field: 'province', code: 'invalid_option' }, + ]); + // Control: a caller who may read the column gets the stored row's verdict. + expect(await verdictOf(engine, { province: 'zj', id: 'c1' }, { userId: 'dave' })).toMatchObject({ valid: false }); + rows.set('c1', { id: 'c1', name: 'one', country: 'cn' }); + expect(await verdictOf(engine, { province: 'zj', id: 'c1' }, { userId: 'dave' })).toMatchObject({ valid: true }); + }); + + /** + * A partial-masking rule shaped like the security plugin's read mask + * (`field-masker.ts` `maskRecord`): the KEY stays, the VALUE is replaced + * with its mask (a value too short to keep both ends is masked whole). + */ + function maskCountryFor(ql: ObjectQL, userId: string) { + ql.registerMiddleware(async (ctx: any, next: () => Promise) => { + await next(); + if (ctx.context?.userId !== userId || !['find', 'findOne'].includes(ctx.operation)) return; + const mask = (r: any) => { + if (r && typeof r === 'object' && typeof r.country === 'string') r.country = '*'.repeat(r.country.length); + }; + if (Array.isArray(ctx.result)) ctx.result.forEach(mask); + else mask(ctx.result); + }); + } + + it('(d2) a column served partially masked is judged as empty, whatever it holds', async () => { + maskCountryFor(engine, 'erin'); + const verdicts = []; + for (const country of ['cn', 'us']) { + rows.set('c1', { id: 'c1', name: 'one', country }); + // Precondition: the read door serves the key, masked. + expect(await engine.findOne(OBJECT, { where: { id: 'c1' }, context: { userId: 'erin' } })).toMatchObject({ country: '**' }); + verdicts.push(await verdictOf(engine, { province: 'zj', id: 'c1' }, { userId: 'erin' })); + } + expect(verdicts[0]).toEqual(verdicts[1]); + expect(verdicts[0]!.errors.map((e) => ({ field: e.field, code: e.code }))).toEqual([ + { field: 'province', code: 'invalid_option' }, + ]); + }); + + it('(d3) control: a caller served the column unmasked gets the stored row\'s verdict', async () => { + maskCountryFor(engine, 'erin'); + rows.set('c1', { id: 'c1', name: 'one', country: 'cn' }); + expect(await verdictOf(engine, { province: 'zj', id: 'c1' }, { userId: 'dave' })).toMatchObject({ valid: true, errors: [] }); + rows.set('c1', { id: 'c1', name: 'one', country: 'us' }); + const refused = await verdictOf(engine, { province: 'zj', id: 'c1' }, { userId: 'dave' }); + expect(refused.errors.map((e) => ({ field: e.field, code: e.code }))).toEqual([ + { field: 'province', code: 'invalid_option' }, + ]); + }); + }); + + describe('(a) a file reference the caller can read is judged as the id it stores', () => { + it('the read door\'s expansion of a file id does not turn the column empty', async () => { + const { engine: e2, rows: r2 } = makeEngine([ + { + name: 'sys_file', label: 'File', + fields: { + name: { name: 'name', label: 'Name', type: 'text' }, + status: { name: 'status', label: 'Status', type: 'text' }, + url: { name: 'url', label: 'URL', type: 'text' }, + }, + }, + { + name: OBJECT, label: 'Preview Attachment', + fields: { + attachment: { name: 'attachment', label: 'Attachment', type: 'file' }, + memo: { name: 'memo', label: 'Memo', type: 'text', requiredWhen: 'record.attachment != null' }, + title: { name: 'title', label: 'Title', type: 'text' }, + }, + }, + ]); + r2.set('f1', { id: 'f1', name: 'a.pdf', status: 'committed', url: '/files/f1' }); + r2.set('a1', { id: 'a1', attachment: 'f1', memo: 'kept' }); + // Precondition: the read door serves the file EXPANDED, the store holds the id. + const served = await e2.findOne(OBJECT, { where: { id: 'a1' } }); + expect(typeof served?.attachment).toBe('object'); + + // Clearing the memo the stored attachment requires: the write refuses it. + const preview = await verdictOf(e2, { memo: null, id: 'a1' }); + const write = await writeRefusal(() => e2.update(OBJECT, { memo: null, id: 'a1' })); + expect(write).toEqual([{ field: 'memo', code: 'required' }]); + expect(preview.errors.map((e) => ({ field: e.field, code: e.code }))).toEqual(write); + }); + }); +}); + +describe('#22445 (e) — the import dry run of a matched row, through the protocol', () => { + let kernel: ObjectKernel; + let objectql: ObjectQL; + let store: ReturnType; + + beforeEach(async () => { + kernel = new ObjectKernel({ logger: { level: 'silent' }, gracefulShutdown: false }); + store = makeStoreDriver(); + await kernel.use({ + name: 'pv-store-plugin', type: 'driver', version: '1.0.0', + init: async (ctx: any) => { ctx.registerService('driver.pv-store', store.driver); }, + } as any); + await kernel.use(new ObjectQLPlugin()); + await kernel.bootstrap(); + objectql = kernel.getService('objectql'); + objectql.registry.registerObject({ ...CASCADE, datasource: 'pv-store' } as any, 'test', 'test'); + store.rows.set('rec_u', { id: 'rec_u', name: 'u', country: 'cn' }); + }); + + afterEach(async () => { + if (kernel.getState() === 'running') await kernel.shutdown(); + }); + + const metaMap = new Map( + Object.values(CASCADE.fields).map((f: any) => [f.name, { name: f.name, type: f.type, ...(f.options ? { options: f.options } : {}) }]), + ); + + const importUpdate = (rowsIn: Record[], dryRun: boolean) => runImport({ + p: new ObjectStackProtocolImplementation(objectql as never) as unknown as ImportProtocolLike, + objectName: OBJECT, + metaMap, + writeMode: 'update', + matchFields: ['name'], + dryRun, + runAutomations: false, + trimWhitespace: true, + createMissingOptions: false, + skipBlankMatchKey: false, + context: { userId: 'usr_importer' }, + rows: rowsIn, + } as any); + + const outcome = (s: Awaited>) => + s.results.map((r) => ({ ok: r.ok, action: r.action, ...(r.ok ? {} : { field: r.field, code: r.code }) })); + + it.each([ + ['a cascade pick', { name: 'u', province: 'zj' }], + ['a note', { name: 'u', note: 'x' }], + ])('%s — the dry run admits the row the import updates', async (_label, cells) => { + const dry = await importUpdate([cells], true); + expect(outcome(dry)).toEqual([{ ok: true, action: 'updated' }]); + expect(store.rows.get('rec_u')).toEqual({ id: 'rec_u', name: 'u', country: 'cn' }); + + const real = await importUpdate([cells], false); + expect(outcome(real)).toEqual(outcome(dry)); + }); + + it('control: the dry run refuses the row the import refuses', async () => { + const cells = { name: 'u', country: 'us' }; + const dry = await importUpdate([cells], true); + expect(outcome(dry)).toEqual([{ ok: false, action: 'failed', field: 'note', code: 'required' }]); + const real = await importUpdate([cells], false); + expect(outcome(real)).toEqual(outcome(dry)); + expect(store.rows.get('rec_u')).toEqual({ id: 'rec_u', name: 'u', country: 'cn' }); + }); +}); diff --git a/packages/objectql/src/validation/rule-validator.ts b/packages/objectql/src/validation/rule-validator.ts index a08b8561dd0..dc5fea67624 100644 --- a/packages/objectql/src/validation/rule-validator.ts +++ b/packages/objectql/src/validation/rule-validator.ts @@ -3637,10 +3637,14 @@ function unevaluableRuleError( error: { kind: string; message: string }, what: 'predicate' | 'when-predicate', subject: { prose: string; detail?: string } = { prose: `Validation rule '${ruleName}'` }, + isDeclaredColumn?: (key: string) => boolean, ): FieldValidationError { const described = describeCelFault(error, { what, undeclaredKeyFix: "fix the rule's condition, or declare the field", + // [#22445] A declared column the judged record lacks was not supplied — + // see {@link declaredColumnRead}. + ...(isDeclaredColumn ? { isDeclaredColumn } : {}), }); const { summary, nullOverload } = described; // A subject that words its own detail has read the fault more precisely than @@ -3718,7 +3722,26 @@ function unevaluableFieldRuleError( return unevaluableRuleError(slot, name, error, 'predicate', { prose: `Field '${name}' ${slot}`, ...(detail !== undefined ? { detail } : {}), - }); + }, declaredColumnRead(fields, source)); +} + +/** + * [#22445] The `isDeclaredColumn` answer for one predicate: is `key` a column + * this object declares (`fields`), read directly off `record` / `previous` by + * `source`? Both halves, because a `No such key` fault on a key reached any + * other way (through a reference, a computed key or receiver) is not a column + * of the judged record, and "its value was not supplied" would be as wrong + * there as "this object does not declare" is for a declared column. + * + * `undefined` — keep the undeclared-key sentence — when there is no field map + * or no CEL source to read. + */ +function declaredColumnRead( + fields: Record | undefined, + source: string, +): ((key: string) => boolean) | undefined { + if (!fields || !source) return undefined; + return (key) => Object.prototype.hasOwnProperty.call(fields, key) && readsRecordColumn(source, key); } /** The CEL source of a predicate, or `''` for a non-CEL dialect or an AST-only envelope. */ @@ -3819,7 +3842,7 @@ function unevaluableOptionGateError( ...unevaluableRuleError('visibleWhen', name, error, 'predicate', { prose: `Option '${String(value)}' of field '${name}' visibleWhen`, ...(detail !== undefined ? { detail } : {}), - }), + }, declaredColumnRead(fields, source)), value: String(value), }; } @@ -4205,7 +4228,10 @@ function checkPredicate( logger?.warn?.( `Validation rule '${rule.name}' predicate failed to evaluate (${result.error.kind}: ${result.error.message}) — write rejected: a rule that cannot be evaluated fails closed, it is never skipped`, ); - const unevaluable = unevaluableRuleError(rule.name, field, result.error, 'predicate'); + const unevaluable = unevaluableRuleError( + rule.name, field, result.error, 'predicate', undefined, + declaredColumnRead(fields, celSourceOfPredicate(rule.condition)), + ); // [#20006] Same verdict; on a delete's reference cleanup, a text that names it. const onCleanup = expr.dialect === 'cel' ? referentialClearRefusal(rule.name, expr, unevaluable, record, previous, related, fields) @@ -4369,7 +4395,10 @@ function checkConditional( ctx.logger?.warn?.( `Validation rule '${rule.name}' when-predicate failed to evaluate (${result.error.kind}: ${result.error.message}) — write rejected: a rule that cannot be evaluated fails closed, it is never skipped`, ); - return unevaluableRuleError(rule.name, '_record', result.error, 'when-predicate'); + return unevaluableRuleError( + rule.name, '_record', result.error, 'when-predicate', undefined, + declaredColumnRead(ctx.fields, celSourceOfPredicate(rule.when)), + ); } const branch = result.value === true ? rule.then : rule.otherwise; diff --git a/packages/rest/src/validate-update-stored-row-representation.test.ts b/packages/rest/src/validate-update-stored-row-representation.test.ts new file mode 100644 index 00000000000..7e63b92c3fc --- /dev/null +++ b/packages/rest/src/validate-update-stored-row-representation.test.ts @@ -0,0 +1,98 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #22445 (d4) — the `update`-mode preview keeps a stored column only where the +// read door served the caller that very value, so a column the read door +// served TRANSFORMED (a partial mask) is judged as empty. This file is the +// representation control for that comparison, on a real driver (driver-sql over +// SQLite): a plain readable column of every common type must reach the rules +// as its stored value. If the read door and the prior-row read represented one +// of them differently, the comparison would judge a readable column empty and +// the preview would answer on less than the write knows. +// +// Each `n_` is required while its column holds the stored value. The +// stored row satisfies every rule; the patch clears every `n_`, so a +// rule that judges the STORED value refuses with `required` (a compliant row +// made violating), and a rule that judged the column empty would answer +// otherwise (a fault, or an admission). The by-id update is the reference. + +import { describe, it, expect, afterEach } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; + +const OBJECT = 'pv_repr'; + +/** column → [field definition, the predicate its `n_` is required under, stored value]. */ +const COLUMNS: Record, string, unknown]> = { + amount: [{ type: 'number' }, 'record.amount > 10', 50], + active: [{ type: 'boolean' }, 'record.active == true', true], + due_on: [{ type: 'date' }, 'record.due_on != null', '2026-01-15'], + happened_at: [{ type: 'datetime' }, 'record.happened_at != null', '2026-01-15T08:30:00.000Z'], + meta: [{ type: 'json' }, 'record.meta != null', { a: 1, b: [1, 2] }], + tags: [{ type: 'multiselect', options: [{ label: 'A', value: 'a' }, { label: 'B', value: 'b' }] }, 'record.tags != null', ['a', 'b']], + tier: [{ type: 'select', options: [{ label: 'Gold', value: 'gold' }, { label: 'Basic', value: 'basic' }] }, "record.tier == 'gold'", 'gold'], + code: [{ type: 'text' }, "record.code == 'X1'", 'X1'], +}; + +const fields: Record> = { title: { name: 'title', label: 'Title', type: 'text' } }; +for (const [column, [def, predicate]] of Object.entries(COLUMNS)) { + fields[column] = { name: column, label: column, ...def }; + fields[`n_${column}`] = { name: `n_${column}`, label: `n_${column}`, type: 'text', requiredWhen: predicate }; +} + +const liveEngines: ObjectQL[] = []; +afterEach(async () => { + while (liveEngines.length) { + try { await liveEngines.pop()?.destroy(); } catch { /* noop */ } + } +}); + +async function boot() { + const engine = new ObjectQL(); + liveEngines.push(engine); + engine.registerDriver(new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }), true); + await engine.init(); + engine.registry.registerObject({ name: OBJECT, label: 'Preview Representation', fields } as any); + await engine.syncSchemas(); + const stored: Record = { title: 'stored' }; + for (const [column, [, , value]] of Object.entries(COLUMNS)) { + stored[column] = value; + stored[`n_${column}`] = 'kept'; + } + const created = await engine.insert(OBJECT, stored) as Record; + return { engine, id: created.id as string }; +} + +const clearAll = () => Object.fromEntries(Object.keys(COLUMNS).map((c) => [`n_${c}`, null])); +const findings = (errors: Array<{ field: string; code: string }>) => + errors.map((e) => ({ field: e.field, code: e.code })).sort((a, b) => a.field.localeCompare(b.field)); + +describe('#22445 (d4) a plain readable column reaches the preview\'s rules as its stored value', () => { + it('every column type judges the stored value, as the by-id update does', async () => { + const { engine, id } = await boot(); + const expected = Object.keys(COLUMNS).map((c) => ({ field: `n_${c}`, code: 'required' })) + .sort((a, b) => a.field.localeCompare(b.field)); + + const preview = await engine.validate(OBJECT, { ...clearAll(), id }, { mode: 'update', context: { userId: 'reader' } }); + expect(findings(preview.results![0]!.errors)).toEqual(expected); + + let writeErrors: Array<{ field: string; code: string }> = []; + try { + await engine.update(OBJECT, { ...clearAll(), id }, { context: { userId: 'reader' } }); + } catch (e: any) { + expect(e?.code).toBe('VALIDATION_FAILED'); + writeErrors = e.fields; + } + expect(findings(writeErrors)).toEqual(expected); + }); + + it('control: each rule admits once its own column is cleared in the same patch', async () => { + const { engine, id } = await boot(); + const patch: Record = { ...clearAll(), id }; + for (const column of Object.keys(COLUMNS)) patch[column] = null; + const preview = await engine.validate(OBJECT, patch, { mode: 'update', context: { userId: 'reader' } }); + expect(findings(preview.results![0]!.errors)).toEqual( + // `null > 10` has no overload: the number rule cannot judge a cleared column. + [{ field: 'n_amount', code: 'rule_violation' }], + ); + }); +}); diff --git a/packages/spec/src/api/protocol.zod.ts b/packages/spec/src/api/protocol.zod.ts index e6069bc3b2e..f0014f92274 100644 --- a/packages/spec/src/api/protocol.zod.ts +++ b/packages/spec/src/api/protocol.zod.ts @@ -2225,7 +2225,11 @@ export const ValidateDataRequestSchema = lazySchema(() => z.object({ ]).describe('A candidate record, or an array of them. Nothing is persisted.'), mode: z.enum(['insert', 'update']).optional().describe( "Which write the verdict should predict. `insert` (default) walks every declared field, so a " + - "missing required field is a finding; `update` judges only the supplied keys, matching a PATCH.", + "missing required field is a finding; `update` checks only the supplied keys, matching a PATCH. " + + "An `update` row that carries its `id` is judged as the by-id update judges it: its rules read the " + + "stored row that id names, merged with the supplied keys. The stored row is read with the caller's " + + "own read access, so a row the caller cannot read is judged on the supplied keys alone, as a row " + + "with no `id` is.", ), })); @@ -2239,9 +2243,10 @@ export const ValidateDataRequestSchema = lazySchema(() => z.object({ * `readonly` and runtime-owned fields (`isSystem`-gated), records what each * takes from each row, and reports it on a row the verdict accepts; * `validateData` relays it. An `update`-mode preview does not run the - * `readonlyWhen` or primary-key strips, which judge a prior record and an - * update dispatch the preview does not have, so it can report fewer drops than - * the update it predicts. + * `readonlyWhen` or primary-key strips, which the update runs against its prior + * record and its dispatch, so it can report fewer drops than the update it + * predicts — including for a row whose stored row it reads (see `mode` on the + * request). * The element IS {@link DroppedFieldsEventSchema} — the same shape and `reason` * vocabulary the write reports, ⛔ never a second enum — so a preview and the * write it predicts answer in one vocabulary, exactly as `errors` / `warnings`