diff --git a/.changeset/25000-strict-stored-expression-refusal.md b/.changeset/25000-strict-stored-expression-refusal.md new file mode 100644 index 00000000000..6c25f379ea6 --- /dev/null +++ b/.changeset/25000-strict-stored-expression-refusal.md @@ -0,0 +1,26 @@ +--- +'@objectstack/objectql': minor +'@objectstack/spec': patch +--- + +fix(objectql)!: under `strictReadonlyWrites`, a write whose value a stored field's `expression` replaces (`stored_expression`) is refused, and nothing is written + +Clause-②: no (narrowing: under `strictReadonlyWrites`, a write whose value a stored expression replaces is refused instead of reported; it ships now under the amended 「甲」, #6015 `6303865025`) + + + +**BREAKING** (the strict mode's refusal set grows by one reason), shipped as `minor` under the amended ruling 「甲」: a narrowing that no conversion carries ships now, in 18.x. It applies only to a caller that opts into the in-process `strictReadonlyWrites` option. No REST or wire caller can set that option, and every write without it behaves exactly as before. + +`strictReadonlyWrites` refuses every drop that `onFieldsDropped` reports. Since the `stored_expression` reason landed, one reported drop was left out: a caller's value for a stored (non-`formula`) field that declares an `expression`, which the engine replaces with the computed value. Under strict, such a write completed and was only reported. It is now refused like every other strip's drop. + +- **FROM:** a write with `strictReadonlyWrites: true` that sent a value for a stored expression field, different from the value the engine computes, completed. The row held the computed value, and `onFieldsDropped` received a `stored_expression` event. +- **TO:** the same write throws `ReadonlyFieldRejectedError` (`code: 'ERR_READONLY_FIELD_REJECTED'`), whose `drops` carries the `stored_expression` event. Nothing is written, and `onFieldsDropped` does not fire. This holds on insert (a single row, a batch, and `insertMany`, which refuses the whole call as it does for the other strict drops), on update by id, and on a predicate update. +- **One refusal names everything.** When the same payload also carries another strip's drop (`readonly`, `readonly_when`, `primary_key`, `computed`), the one refusal names all of them together, `stored_expression` last. Before, such a write was refused for the other drops alone. The message's per-reason form gains a `stored_expression` remedy, spoken only when that reason is present, so every other refusal's message is unchanged. +- **Ordering is kept.** The strips' refusal still comes first: if the stored-expression pass itself throws, for example a predicate update whose patch computes different values on different matched rows (`VALIDATION_FAILED`), a strict write that also carries a strip's drop is still refused with `ERR_READONLY_FIELD_REJECTED` naming those drops, as before. +- **`@objectstack/spec` (patch):** the TSDoc on `WriteObservabilityOptions.strictReadonlyWrites` and `DroppedFieldsEvent.reason`, which ships in the published declarations, now says the `stored_expression` arm is refused under strict. No schema, `.describe()` text or export changes. +- **Unchanged:** a strict write that sends the computed value, or no value for the field, is not refused, because nothing it sent was replaced. A stored expression field also declared `readonly` is still refused once, as `readonly`. Without `strictReadonlyWrites`, the write completes with the computed value and the replacement is reported, as before. The error code is unchanged. +- **The fix for a strict caller:** leave the stored expression field out of the payload, or send the value its expression computes. To let the replacement happen and merely observe it, drop `strictReadonlyWrites` and pass `onFieldsDropped`. + +This supersedes the `strictReadonlyWrites` bullet of the `stored_expression` reason's entry, which deferred this refusal to a later major. + +Measured before the change: no non-test caller in this repository passes `strictReadonlyWrites` (the examples, dogfood suites and runtime doors pass none), so no in-repo producer moves. diff --git a/packages/objectql/src/engine-stored-expression-dropped-fields.test.ts b/packages/objectql/src/engine-stored-expression-dropped-fields.test.ts index fd517cba0f1..cc17cb9641d 100644 --- a/packages/objectql/src/engine-stored-expression-dropped-fields.test.ts +++ b/packages/objectql/src/engine-stored-expression-dropped-fields.test.ts @@ -20,11 +20,13 @@ // grid's consistent save stays silent), nor is a key the caller never sent; // - a caller value another strip already took is reported once, under that // strip's reason, never twice; -// - `strictReadonlyWrites` does NOT refuse a replaced value in 18.x (the -// maintainer's ruling 「甲」: only the loud half ships; the refusal waits -// for 19.0) — the write completes with the computed value and the report -// reaches the listener under strict too, while a strict write that ALSO -// carries a refusable drop is still refused, and fires no listener. +// - #25000: under `strictReadonlyWrites` a replaced value is REFUSED like +// every other strip's drop (the amended ruling 「甲」 ships it in 18.x) — +// on insert (single, batch, `insertMany`), update by id and predicate +// update: `ERR_READONLY_FIELD_REJECTED`, nothing written, no listener — +// in one refusal with any other strip's drop; the computed value sent, or +// no value, is never refused; and the same payload WITHOUT strict still +// completes and is reported (the controls beside each refusal). import { describe, it, expect, beforeEach } from 'vitest'; import type { DroppedFieldsEvent } from '@objectstack/spec/data'; @@ -128,6 +130,8 @@ describe('[#24690] a caller value a stored expression replaced is reported as `s quantity: { type: 'number', readonlyWhen: "parent.status == 'paid'" }, unit_price: { type: 'currency', readonlyWhen: "parent.status == 'paid'" }, description: { type: 'text' }, + // A static lock, so a predicate update can carry a strip's drop. + note: { type: 'text', readonly: true }, amount: { type: 'currency', min: 0, expression: { dialect: 'cel', source: 'record.quantity * record.unit_price' } }, }, } as any); @@ -254,49 +258,150 @@ describe('[#24690] a caller value a stored expression replaced is reported as `s }); }); - describe('strictReadonlyWrites — a replaced value is REPORTED, not refused, in 18.x (ruling 「甲」)', () => { - it('update by id: the write completes with the computed value, and the report reaches the listener', async () => { + describe('[#25000] strictReadonlyWrites — a replaced value is REFUSED, and nothing is written', () => { + const STRICT = { strictReadonlyWrites: true } as const; + async function refusal(write: Promise): Promise { + const err: any = await write.then(() => undefined, (e: unknown) => e); + expect(err).toBeInstanceOf(ReadonlyFieldRejectedError); + expect(err.code).toBe('ERR_READONLY_FIELD_REJECTED'); + return err; + } + const snapshot = () => [...storeFor(LINE).values()].map((r) => ({ ...r })); + + it('update by id: refused, the row keeps its stored values, and no listener fires', async () => { + const l = listener(); + const before = snapshot(); + const err = await refusal(engine.update(LINE, { id: 'line_draft', quantity: 4, amount: 5 }, { ...STRICT, onFieldsDropped: l.onFieldsDropped } as any)); + expect(err.operation).toBe('update'); + expect(err.fields).toEqual(['amount']); + expect(err.drops).toEqual([AMOUNT]); + expect(snapshot()).toEqual(before); + expect(l.events).toEqual([]); + }); + + it('control, update by id: the same payload without strict completes with the computed value, and is reported', async () => { const l = listener(); - await engine.update(LINE, { id: 'line_draft', quantity: 4, amount: 5 }, { strictReadonlyWrites: true, onFieldsDropped: l.onFieldsDropped } as any); + await engine.update(LINE, { id: 'line_draft', quantity: 4, amount: 5 }, { onFieldsDropped: l.onFieldsDropped }); expect(line('line_draft')).toMatchObject({ quantity: 4, amount: 40 }); expect(l.events).toEqual([AMOUNT]); }); - it('insert: the row is written with the computed value, and reported', async () => { + it('update by id under strict: the computed value sent, or no value, is not refused', async () => { + await engine.update(LINE, { id: 'line_draft', quantity: 5, unit_price: 20, amount: 100 }, STRICT as any); + expect(line('line_draft').amount).toBe(100); + await engine.update(LINE, { id: 'line_draft', quantity: 7 }, STRICT as any); + expect(line('line_draft').amount).toBe(140); + }); + + it('insert, single row: refused, nothing written', async () => { + const l = listener(); + const err = await refusal(engine.insert(LINE, { invoice: 'INV-1004', quantity: 2, unit_price: 10, amount: 999 }, { ...STRICT, onFieldsDropped: l.onFieldsDropped })); + expect(err.operation).toBe('insert'); + expect(err.drops).toEqual([AMOUNT]); + expect(storeFor(LINE).size).toBe(3); + expect(l.events).toEqual([]); + }); + + it('control, insert: the same row without strict is written with the computed value, and reported', async () => { const l = listener(); - await engine.insert(LINE, { invoice: 'INV-1004', quantity: 2, unit_price: 10, amount: 999 }, { strictReadonlyWrites: true, onFieldsDropped: l.onFieldsDropped }); + await engine.insert(LINE, { invoice: 'INV-1004', quantity: 2, unit_price: 10, amount: 999 }, { onFieldsDropped: l.onFieldsDropped }); const rows = [...storeFor(LINE).values()]; expect(rows).toHaveLength(4); expect(rows.find((r) => r.amount === 20 && r.quantity === 2)).toBeDefined(); expect(l.events).toEqual([AMOUNT]); }); - it('predicate update: every matched row is written, and the replacement is reported', async () => { + it('insert under strict: the computed value sent, or no value, is written', async () => { + await engine.insert(LINE, { invoice: 'INV-1004', quantity: 2, unit_price: 10, amount: 20 }, STRICT); + await engine.insert(LINE, { invoice: 'INV-1004', quantity: 3, unit_price: 10 }, STRICT); + expect(storeFor(LINE).size).toBe(5); + }); + + it('insert, batch: one replaced row refuses the whole batch, and no row is written', async () => { + const l = listener(); + const err = await refusal(engine.insert(LINE, [ + { invoice: 'INV-1004', quantity: 4, unit_price: 3, amount: 12 }, + { invoice: 'INV-1004', quantity: 5, unit_price: 3, amount: 1 }, + ], { ...STRICT, onFieldsDropped: l.onFieldsDropped })); + expect(err.drops).toEqual([AMOUNT]); + expect(storeFor(LINE).size).toBe(3); + expect(l.events).toEqual([]); + }); + + it('insertMany: refused like the other strict drops, and no row is written', async () => { const l = listener(); - await engine.update(LINE, { quantity: 2, unit_price: 5, amount: 1 }, { where: { invoice: 'INV-1004' }, multi: true, strictReadonlyWrites: true, onFieldsDropped: l.onFieldsDropped } as any); + const err = await refusal(engine.insertMany(LINE, [ + { invoice: 'INV-1004', quantity: 1, unit_price: 3, amount: 0 }, + { invoice: 'INV-1004', quantity: 4, unit_price: 3, amount: 12 }, + ], { ...STRICT, onFieldsDropped: l.onFieldsDropped })); + expect(err.drops).toEqual([AMOUNT]); + expect(storeFor(LINE).size).toBe(3); + expect(l.events).toEqual([]); + }); + + it('control, insertMany: without strict both rows are written and the replaced one carries its per-row report', async () => { + const outcomes: any[] = await engine.insertMany(LINE, [ + { invoice: 'INV-1004', quantity: 1, unit_price: 3, amount: 0 }, + { invoice: 'INV-1004', quantity: 4, unit_price: 3, amount: 12 }, + ]); + expect(outcomes.map((o) => o.ok)).toEqual([true, true]); + expect(outcomes[0].droppedFields).toEqual([AMOUNT]); + expect(storeFor(LINE).size).toBe(5); + }); + + it('predicate update: refused, and no matched row is written', async () => { + const l = listener(); + const before = snapshot(); + const err = await refusal(engine.update(LINE, { quantity: 2, unit_price: 5, amount: 1 }, { where: { invoice: 'INV-1004' }, multi: true, ...STRICT, onFieldsDropped: l.onFieldsDropped } as any)); + expect(err.drops).toEqual([AMOUNT]); + expect(snapshot()).toEqual(before); + expect(l.events).toEqual([]); + }); + + it('control, predicate update: without strict every matched row is written, and the replacement is reported', async () => { + const l = listener(); + await engine.update(LINE, { quantity: 2, unit_price: 5, amount: 1 }, { where: { invoice: 'INV-1004' }, multi: true, onFieldsDropped: l.onFieldsDropped } as any); expect([line('line_draft').amount, line('line_draft_2').amount]).toEqual([10, 10]); expect(l.events).toEqual([AMOUNT]); }); - it('a strict write that ALSO carries a refusable drop is refused for THAT drop alone, writes nothing, and fires no listener', async () => { + it('a strict write that ALSO carries another strip\'s drop is refused ONCE, naming both, and writes nothing', async () => { const l = listener(); - const err: any = await engine - .update(LINE, { id: 'line_paid', quantity: 60, amount: 2999.4 }, { strictReadonlyWrites: true, onFieldsDropped: l.onFieldsDropped } as any) - .catch((e: unknown) => e); - expect(err).toBeInstanceOf(ReadonlyFieldRejectedError); - expect(err.code).toBe('ERR_READONLY_FIELD_REJECTED'); - expect(err.drops).toEqual([{ object: LINE, fields: ['quantity'], reason: 'readonly_when' }]); + const err = await refusal(engine.update(LINE, { id: 'line_paid', quantity: 60, amount: 2999.4 }, { ...STRICT, onFieldsDropped: l.onFieldsDropped } as any)); + expect(err.drops).toEqual([ + { object: LINE, fields: ['quantity'], reason: 'readonly_when' }, + AMOUNT, + ]); + expect(err.fields).toEqual(['quantity', 'amount']); expect(line('line_paid')).toMatchObject({ quantity: 6, amount: 299.94 }); expect(l.events).toEqual([]); }); - it('insert: a strict write with a refusable drop is refused before the computation, carrying no stored_expression drop', async () => { + it('predicate update: a strip\'s drop is refused BEFORE a divergent settle — ERR_READONLY_FIELD_REJECTED, not VALIDATION_FAILED (#5126 ordering)', async () => { const l = listener(); + const before = snapshot(); + // quantity: 10 computes amount 100 on line_draft and 70 on line_draft_2, + // which one statement cannot store; `note` is taken by the static strip. + const err = await refusal(engine.update(LINE, { note: 'x', quantity: 10 }, { where: { invoice: 'INV-1004' }, multi: true, ...STRICT, onFieldsDropped: l.onFieldsDropped } as any)); + expect(err.drops).toEqual([{ object: LINE, fields: ['note'], reason: 'readonly' }]); + expect(snapshot()).toEqual(before); + expect(l.events).toEqual([]); + }); + + it('control, predicate update: the same divergent payload without strict is refused by the settle, VALIDATION_FAILED', async () => { + const before = snapshot(); const err: any = await engine - .insert('qa_locked_line', { quantity: 4, unit_price: 2, amount: 5 }, { strictReadonlyWrites: true, onFieldsDropped: l.onFieldsDropped }) + .update(LINE, { note: 'x', quantity: 10 }, { where: { invoice: 'INV-1004' }, multi: true } as any) .catch((e: unknown) => e); - expect(err).toBeInstanceOf(ReadonlyFieldRejectedError); - expect(err.drops.map((d: DroppedFieldsEvent) => d.reason)).not.toContain('stored_expression'); + expect(err?.code).toBe('VALIDATION_FAILED'); + expect(err?.fields).toEqual([expect.objectContaining({ field: 'amount', code: 'invalid_value' })]); + expect(snapshot()).toEqual(before); + }); + + it('insert: a value the static `readonly` strip took is refused as `readonly` alone, never also as `stored_expression`', async () => { + const l = listener(); + const err = await refusal(engine.insert('qa_locked_line', { quantity: 4, unit_price: 2, amount: 5 }, { ...STRICT, onFieldsDropped: l.onFieldsDropped })); + expect(err.drops).toEqual([{ object: 'qa_locked_line', fields: ['amount'], reason: 'readonly' }]); expect([...storeFor('qa_locked_line').values()]).toHaveLength(1); expect(l.events).toEqual([]); }); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 16e2a168b36..dc62bd2cccf 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -16799,31 +16799,38 @@ export class ObjectQL implements IObjectQLEngine { // (`replacedStoredExpressionFields`). A key a strip above already took // is that strip's report. // - // ⛔ NOT refused under `strictReadonlyWrites`, by the maintainer's - // ruling 「甲」: in 18.x only the loud half ships — the reason and its - // report. The refusal is runtime behaviour no conversion can carry, so - // it waits for 19.0. So the strict check judges the strips' drops - // alone, BEFORE the computation, exactly where it stood, and a - // `stored_expression` report reaches the listener under strict too. - const refusableInsertDrops = droppedFieldEvents(object, computedDropped, insertDropped); - if (refusableInsertDrops.length > 0 && options?.strictReadonlyWrites === true) { - // Before the driver write and before validation — nothing is - // written, and "you sent a runtime-owned field" should not depend on - // whether some other field also failed a business rule (#5126's - // ordering on the update path, mirrored). - throw new ReadonlyFieldRejectedError( - object, [...new Set(refusableInsertDrops.flatMap((d) => d.fields))], refusableInsertDrops, 'insert', - ); - } + // [#25000] REFUSED under `strictReadonlyWrites` like every other + // strip's drop: the strict contract follows what `onFieldsDropped` + // reports, and a replaced value is one (the amended ruling 「甲」 ships + // the refusal in 18.x). So the strict check judges the strips' drops + // AND the replacement, AFTER the computation — one refusal names + // everything the write would have taken or replaced, rather than a + // round-trip per class. The computation writes nothing (it evaluates + // over copies and assigns onto the in-memory rows), so the refusal + // still lands before validation and before any driver dispatch. const storedExpressionReplaced: string[] = []; const storedExpressionReplacedPerRow: string[][] = rows.map(() => []); if (storedExpressionPlan.length > 0) { const liveIndexes = rows .map((row, i) => (rowErrors[i] === undefined && row != null && typeof row === 'object' ? i : -1)) .filter((i) => i >= 0); - const computed = await computeStoredExpressions( - storedExpressionPlan, liveIndexes.map((i) => rows[i] as Record), opCtx.context, storedExpressionServing, - ); + let computed: Array>; + try { + computed = await computeStoredExpressions( + storedExpressionPlan, liveIndexes.map((i) => rows[i] as Record), opCtx.context, storedExpressionServing, + ); + } catch (err) { + // A pass that THROWS is pre-empted by the strict verdict on the + // strips' drops (#5126's ordering — the check used to sit ahead + // of this pass), then rethrown. + const stripDrops = droppedFieldEvents(object, computedDropped, insertDropped); + if (stripDrops.length > 0 && options?.strictReadonlyWrites === true) { + throw new ReadonlyFieldRejectedError( + object, [...new Set(stripDrops.flatMap((d) => d.fields))], stripDrops, 'insert', + ); + } + throw err; + } liveIndexes.forEach((i, k) => { const row = rows[i] as Record; const taken = insertDroppedPerRow[i] ?? []; @@ -16835,11 +16842,19 @@ export class ObjectQL implements IObjectQLEngine { }); } const insertDrops = droppedFieldEvents(object, computedDropped, insertDropped, storedExpressionReplaced); + if (insertDrops.length > 0 && options?.strictReadonlyWrites === true) { + // Before the driver write and before validation — nothing is + // written, and "you sent a runtime-owned field" should not depend on + // whether some other field also failed a business rule (#5126's + // ordering on the update path, mirrored). + throw new ReadonlyFieldRejectedError( + object, [...new Set(insertDrops.flatMap((d) => d.fields))], insertDrops, 'insert', + ); + } if (insertDrops.length > 0 && typeof options?.onFieldsDropped === 'function') { - // A strict write that reaches here had no refusable drop (above), so - // every event left is a `stored_expression` report on a write that - // completes — reported under strict as well. A REFUSED write never - // fires the listener: it did not complete. + // A strict write never reaches here with a drop (refused above), so + // the listener hears only a write that completes. A REFUSED write + // never fires the listener: it did not complete. for (const drop of insertDrops) { try { options.onFieldsDropped(drop); @@ -17484,11 +17499,10 @@ export class ObjectQL implements IObjectQLEngine { notifyDrop(fields, reason); }; /** - * One drop to the listener ONLY, in every mode — never a strict refusal. - * [#24690] The `stored_expression` report goes here: by the maintainer's - * ruling 「甲」 18.x ships its report and not its refusal (runtime - * behaviour no conversion can carry waits for 19.0). Called after - * `assertNoStrictDrops()`, so a refused write never fires the listener. + * One drop to the listener, on a non-strict write — `emitDrop`'s other arm. + * [#25000] The `stored_expression` report no longer calls this directly: + * it goes through `emitDrop` like every strip's, so under strict it is + * refused, and a refused write never fires the listener. */ const notifyDrop = (fields: string[], reason: DroppedFieldsEvent['reason']): void => { if (fields.length === 0 || !onFieldsDropped) return; @@ -18494,21 +18508,35 @@ export class ObjectQL implements IObjectQLEngine { // strict is about the payload the caller SENT, and reporting // "you sent a read-only field" should not depend on whether some // other field also failed a business rule. - assertNoStrictDrops(); + // // [#24619] The stored computed fields over the stored row merged // with the FINAL payload — after the hooks and both strips, so a // locked input the `readonlyWhen` strip took back out feeds the // stored value, never the caller's — and before the statement. // [#24690] A caller value the computation replaces is reported - // under `stored_expression`, to the listener only (`notifyDrop`): - // never refused under strict in 18.x (ruling 「甲」). + // under `stored_expression`; [#25000] through `emitDrop`, the + // channel every strip reports through, so under strict it is + // REFUSED like them and nothing is written. The one judgment + // below therefore follows the computation, and one refusal names + // every drop and replacement together. The computation writes + // nothing (it evaluates over a copy), and an object declaring no + // stored expression skips it, so its refusal is where it stood. + // A pass that THROWS (a permission or related-row read) is + // pre-empted by the strict verdict on the strips' drops already + // collected — #5126's ordering, as on the predicate branch. if (storedExpressionPlan.length > 0) { const patch = hookContext.input.data as Record; - const [computed] = await computeStoredExpressions(storedExpressionPlan, [{ ...priorRecord, ...patch }], opCtx.context, storedExpressionServing); - const replaced = replacedStoredExpressionFields(storedExpressionPlan, suppliedValues, patch, [computed!]); + let computed: Record | undefined; + try { + [computed] = await computeStoredExpressions(storedExpressionPlan, [{ ...priorRecord, ...patch }], opCtx.context, storedExpressionServing); + } catch (err) { + assertNoStrictDrops(); + throw err; + } + emitDrop(replacedStoredExpressionFields(storedExpressionPlan, suppliedValues, patch, [computed!]), 'stored_expression'); applyStoredExpressionsToPatch(patch, computed!, priorRecord); - notifyDrop(replaced, 'stored_expression'); } + assertNoStrictDrops(); // [#21663] The payload is FINAL here (see the seam below), so // every readonly value on it is one the driver will store: a // system writer's (the strip above never ran for it), a hook's, @@ -18781,30 +18809,48 @@ export class ObjectQL implements IObjectQLEngine { // [#5126] Same refusal on the predicate path. A bulk strip is // "locked in ≥1 matched row ⇒ dropped for ALL", so a strict bulk // caller is told before N rows are written with a column missing - // — the failure mode a bulk write makes N times larger. - assertNoStrictDrops(); + // — the failure mode a bulk write makes N times larger. The + // judgment itself follows the stored-expression pass below, as on + // the by-id branch. + // // [#24619] The stored computed fields on the predicate path, over // the FINAL payload as on the by-id branch. One patch reaches // every matched row, so {@link settleBulkStoredExpressions} keeps // a computed value only when it is the same on every row, and // refuses a patch that would compute different ones. The rows // are the D7 read (`readPriorRows` is a memo — no second fetch). - // [#24690] As on the by-id branch, to the listener only: a caller - // value differing from what any matched row then stores (the one - // computed value, or the column left as it is) is reported under - // `stored_expression`, and never refused under strict in 18.x. + // [#24690] As on the by-id branch: a caller value differing from + // what any matched row then stores (the one computed value, or + // the column left as it is) is reported under + // `stored_expression`; [#25000] through `emitDrop`, so under + // strict it is REFUSED before the statement and no row is written, + // in one refusal with every strip's drop. The pass writes nothing. + // ⛔ #5126's ordering survives it: a pass that THROWS — above + // all `settleBulkStoredExpressions` refusing a patch that computes + // different values on different rows (`VALIDATION_FAILED`) — is + // pre-empted by the strict verdict on the strips' drops already + // collected, so a strict caller whose payload a strip took is told + // `ERR_READONLY_FIELD_REJECTED` first, exactly as when the assert + // sat ahead of the whole pass. With no throw, one refusal names + // every strip drop plus the replacement. if (storedExpressionPlan.length > 0) { - const matched = ((await readPriorRows()) ?? []).filter((row) => row != null && typeof row === 'object'); - const patch = hookContext.input.data as Record; - const before = await computeStoredExpressions(storedExpressionPlan, matched, opCtx.context, storedExpressionServing); - const after = await computeStoredExpressions(storedExpressionPlan, matched.map((row) => ({ ...row, ...patch })), opCtx.context, storedExpressionServing); - const settled = settleBulkStoredExpressions(storedExpressionPlan, patch, before, after, object); - notifyDrop( - replacedStoredExpressionFields(storedExpressionPlan, suppliedValues, patch, matched.map((row) => ({ ...row, ...settled }))), - 'stored_expression', - ); - hookContext.input.data = settled as any; + try { + const matched = ((await readPriorRows()) ?? []).filter((row) => row != null && typeof row === 'object'); + const patch = hookContext.input.data as Record; + const before = await computeStoredExpressions(storedExpressionPlan, matched, opCtx.context, storedExpressionServing); + const after = await computeStoredExpressions(storedExpressionPlan, matched.map((row) => ({ ...row, ...patch })), opCtx.context, storedExpressionServing); + const settled = settleBulkStoredExpressions(storedExpressionPlan, patch, before, after, object); + emitDrop( + replacedStoredExpressionFields(storedExpressionPlan, suppliedValues, patch, matched.map((row) => ({ ...row, ...settled }))), + 'stored_expression', + ); + hookContext.input.data = settled as any; + } catch (err) { + assertNoStrictDrops(); + throw err; + } } + assertNoStrictDrops(); // [#21663] The predicate-path twin of the by-id second pass, at the // same point and for the same reason: the readonly values left on // the final payload are stored, so their SHAPE is judged — before diff --git a/packages/objectql/src/readonly-strict-errors.ts b/packages/objectql/src/readonly-strict-errors.ts index 46ba6d4688a..a8888fad2ae 100644 --- a/packages/objectql/src/readonly-strict-errors.ts +++ b/packages/objectql/src/readonly-strict-errors.ts @@ -19,7 +19,10 @@ import type { DroppedFieldsEvent } from '@objectstack/spec/data'; * implicitly-readonly runtime-owned types (#5503), and the `primary_key` strip * of a payload `id` the update dispatch ruled is not an identifier (#6437); on * both verbs, in every context, the `computed` strip of a `formula` value - * (#20805); on INSERT the runtime-owned ones and, since the 2026-09-03 ruling (#14147) + * (#20805) and, since #25000, the `stored_expression` replacement of a caller + * value a stored field's `expression` computes differently (#24690 reported + * it; the write is now refused, on insert, update by id and predicate update); + * on INSERT the runtime-owned ones and, since the 2026-09-03 ruling (#14147) * superseded the create-side exemption #3413 had granted, static `readonly` * too — judged over `staticReadonlyInsertSubject`, which leaves a * `sys_`-prefixed object, and one in a PLATFORM-INTERNAL `managedBy` bucket, to @@ -87,11 +90,10 @@ const REASON_PHRASE: Record = { computed: 'a computed `formula` field — the engine computes it on every read, so no driver has a ' + 'column for a written value', - // [#24690] Required by the exhaustive key type, and NOT refused in 18.x: - // by the maintainer's ruling 「甲」 a `stored_expression` drop is reported - // through `onFieldsDropped` only, under strict too, and its refusal waits - // for 19.0. No refusal this module builds carries one today; the row is - // the wording that refusal will use. + // [#24690 / #25000] NOT in READONLY_CLASS_REASONS: the field is written, + // with the computed value, and `isSystem` does not exempt the computation. + // Refused under strict since #25000 (the amended ruling 「甲」 ships it in + // 18.x); #24690 wrote this wording ahead of that refusal. stored_expression: 'a stored field the engine computes from its declared `expression` on every write — the ' + 'value sent differs from the computed one, so it would have been replaced', @@ -166,6 +168,13 @@ function buildRefusalMessage( ? `for 'computed', leave the formula field out of the payload — the engine computes it ` + `on every read, and isSystem does not exempt it; ` : ''; + // [#25000] Same rule: spoken ONLY when a `stored_expression` drop is + // present, so every payload without one reads its earlier text byte for byte. + const storedExpressionRemedy = drops.some((d) => d.reason === 'stored_expression') + ? `for 'stored_expression', leave the field out of the payload or send the value its ` + + `expression computes — the engine computes it on every write, and isSystem does not ` + + `exempt it; ` + : ''; return ( head + `${fields.length} caller-supplied field(s) ` + @@ -177,7 +186,7 @@ function buildRefusalMessage( `statically 'readonly' columns (that exempts NEITHER a TRUE 'readonlyWhen' predicate ` + `NOR the primary-key strip — both apply to every API-boundary caller, isSystem included; ` + `a 'readonlyWhen' value DERIVED by a beforeUpdate hook is not a caller write and is never ` + - `stripped); ${computedRemedy}for ` + + `stripped); ${computedRemedy}${storedExpressionRemedy}for ` + `'primary_key', pass a SCALAR id to update ONE row (\`update(object, { id, ...fields })\` ` + `or \`{ where: { id } }\`), or put the id set in \`where\` to SELECT rows ` + `(\`{ where: { id: { $in: [...] } }, multi: true }\`). ` + diff --git a/packages/spec/src/contracts/data-engine.ts b/packages/spec/src/contracts/data-engine.ts index 60af557a190..acebfc49fbb 100644 --- a/packages/spec/src/contracts/data-engine.ts +++ b/packages/spec/src/contracts/data-engine.ts @@ -106,15 +106,16 @@ export interface WriteObservabilityOptions { * learns WHICH fields would go — but their result is discarded. * * It covers every drop `onFieldsDropped` reports — coverage DERIVED from the - * reported set, never an enumeration frozen at #5126. Today that is four of - * the five `DroppedFieldsEvent['reason']` arms: static `readonly: true` (#2948, which + * reported set, never an enumeration frozen at #5126. Today that is all five + * `DroppedFieldsEvent['reason']` arms: static `readonly: true` (#2948, which * only runs for non-system callers), a TRUE `readonlyWhen` predicate (#3042, * which runs for every API-BOUNDARY caller, `isSystem` included — but judges * only the keys the caller supplied at engine entry, so a value a * `beforeUpdate` hook derived or overwrote is never stripped, #9107), the * `primary_key` - * strip (#6437), and the `computed` strip of a `formula` value (#20805, every - * context, `isSystem` included) — plus, since #5503, the implicitly-readonly runtime-owned + * strip (#6437), the `computed` strip of a `formula` value (#20805, every + * context, `isSystem` included), and the `stored_expression` replacement + * (#24690 reported it; #25000 refuses it) — plus, since #5503, the implicitly-readonly runtime-owned * strip, which reports under the same `'readonly'` arm rather than adding one * (see the INSERT section below). Covering only the static arm would leave a * trusted caller — the very caller this option exists for, one that already @@ -122,14 +123,16 @@ export interface WriteObservabilityOptions { * static strip — still losing `readonlyWhen` fields in silence, which is the * bug this option exists to abolish. * - * ⛔ **The one arm outside that coverage, for the 18.x line: `stored_expression`** - * (#24690) — a caller's value for a stored field the engine computes from - * its `expression`, replaced by the computed one. By the maintainer's ruling - * 「甲」 only its loud half ships in 18.x: it is REPORTED through - * `onFieldsDropped` / `droppedFields`, under `strictReadonlyWrites` as well, - * and the write completes with the computed value. Its refusal is runtime - * behaviour no conversion can carry, so it waits for 19.0, where the derived - * coverage below reaches it like any other arm. + * `stored_expression` (#24690) is a caller's value for a stored field the + * engine computes from its `expression`, replaced by the computed one. The + * field IS written, but not with the caller's value, so a strict caller is + * told exactly as it is for a strip: since #25000 (18.x, a BREAKING minor + * under the amended ruling 「甲」) such a write is REFUSED, on insert + * (single, batch, `insertMany`), update by id and predicate update, and + * nothing is written. Before it, the write completed with the computed value + * and was only reported. A caller that sends the computed value, or no value + * for the field, is never refused: nothing it sent was replaced. Remedy: leave + * the field out of the payload, or send the value its expression computes. * * ⚠️ **A new `reason` therefore adds a new REFUSAL, by construction** — the * price of the derived coverage above, paid deliberately when `primary_key` diff --git a/packages/spec/src/data/data-engine.zod.ts b/packages/spec/src/data/data-engine.zod.ts index f96be389b1f..ece3d06ebfd 100644 --- a/packages/spec/src/data/data-engine.zod.ts +++ b/packages/spec/src/data/data-engine.zod.ts @@ -317,10 +317,10 @@ export const DroppedFieldsEventSchema = lazySchema(() => z.object({ * declared `readonly` is reported once, as `readonly`). On a predicate * update the value stored is the one computed value, or the column left * as it is when the patch moves no input; a caller value that differs - * from what any matched row stores is reported. The one arm - * `strictReadonlyWrites` does NOT refuse in 18.x (ruling 「甲」): the - * report reaches `onFieldsDropped` under strict too, and the refusal - * waits for 19.0 (see `WriteObservabilityOptions`). + * from what any matched row stores is reported. Under + * `strictReadonlyWrites` it is REFUSED like every other arm, and nothing + * is written (#25000, 18.x, under the amended ruling 「甲」; before it the + * write completed and was only reported — see `WriteObservabilityOptions`). * * `primary_key` names the FIELD's role, not the offending value's shape, on * purpose: `not_a_primary_key` would describe the value and become false the