Skip to content

Commit 56cac78

Browse files
committed
fix(objectql): report a stored_expression replacement under strictReadonlyWrites instead of refusing it, deferring the refusal to 19.0
Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D6F9axfoPzR7t8G1J7Ytzz
1 parent 2cbfb27 commit 56cac78

6 files changed

Lines changed: 122 additions & 91 deletions

File tree

‎.changeset/24690-stored-expression-dropped-fields.md‎

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,21 +4,15 @@
44
'@objectstack/service-automation': patch
55
---
66

7-
feat(objectql)!: a caller value that a stored field's `expression` replaced is reported through `onFieldsDropped` / `droppedFields` under the new `DroppedFieldsEvent.reason` value `stored_expression`
7+
feat(objectql): a caller value that a stored field's `expression` replaced is reported through `onFieldsDropped` / `droppedFields` under the new `DroppedFieldsEvent.reason` value `stored_expression`
88

9-
Clause-②: no (narrowing)
9+
Clause-②: yes (widening)
1010

11-
<!-- adr-0087: not-required (no-migration-prescription) A narrowing of what a `strictReadonlyWrites` write is accepted as, plus one value added to an output enum. Nothing an author or a caller can write is removed, renamed or re-shaped: `DroppedFieldsEventSchema.reason` gains `stored_expression` and keeps its four values, so there is no tombstone and nothing for `objectstack migrate meta` to rewrite. The other categories are closed on facts: all three packages publish (not unpublished); no ADR-0087 id covers this behaviour and this diff adds none (not registered or already-registered); and the change is a write-path behaviour, not only an interface or type change (not runtime-interface-only or type-surface-only). -->
11+
The engine computes a stored (non-`formula`) field's declared `expression` on every write, so a value the caller sends for such a field never stands. Until now nothing said so. `onFieldsDropped` stayed silent, so the PATCH and `/batch` doors answered with no `droppedFields`, and a flow's `update_record` reported success over a row that holds the computed value instead of the one sent.
1212

13-
**BREAKING** accept-set narrowing for `strictReadonlyWrites` callers only, shipped as `minor` under the launch-window convention for breaking changes.
14-
15-
Since the engine computes a stored (non-`formula`) field's declared `expression` on every write, a value the caller sends for such a field never stands. Until now nothing said so: `onFieldsDropped` stayed silent, so the PATCH and `/batch` doors answered with no `droppedFields`, and a flow's `update_record` reported success, over a row that holds the computed value instead of the one sent.
16-
17-
- **New `reason` value.** `DroppedFieldsEventSchema.reason` (`@objectstack/spec/data`) gains `stored_expression`: the field is a stored column the engine computes from its `expression`, and the caller's value was replaced by the computed one. The field IS written, so `computed` (a `formula` with no column, whose key is dropped) would be wrong for it. The name follows the existing arms, which name the field's role, not what happened to the value.
18-
- **When it is reported.** On insert (single, batch, and `insertMany`'s per-row `droppedFields`), on update by id, and on a predicate update, in every context, `isSystem` included. Only when the caller sent the field and the value stored differs from the caller's value. A client that sends the computed value (the console's line-item grid) or no value is told nothing. A value another strip already took, such as a stored expression field that is also `readonly`, is reported once, under that strip's reason.
19-
- **`strictReadonlyWrites`.** Its coverage is every reason `onFieldsDropped` reports, so a strict write whose value for such a field differs from the computed one is now **refused** with `ERR_READONLY_FIELD_REJECTED`, and nothing is written. Before this change it succeeded and the value was silently replaced. A strict write that sends the computed value, or none, is not affected.
13+
- **New `reason` value.** `DroppedFieldsEventSchema.reason` (`@objectstack/spec/data`) gains `stored_expression`. It means the field is a stored column the engine computes from its `expression`, and the caller's value was replaced by the computed one. The field IS written, so `computed` (a `formula` with no column, whose key is dropped) would be wrong for it. The four existing values are unchanged.
14+
- **When it is reported.** On insert (single, batch, and `insertMany`'s per-row `droppedFields`), on update by id, and on a predicate update, in every context, `isSystem` included. It is reported only when the caller sent the field and the value stored differs from the caller's value. A client that sends the computed value (the console's line-item grid) or no value is told nothing. A value another strip already took, such as a stored expression field that is also `readonly`, is reported once, under that strip's reason.
15+
- **`strictReadonlyWrites` is unchanged.** The maintainer's ruling 「甲」 says only the loud half ships in 18.x. A strict write whose value for such a field differs from the computed one is NOT refused: it completes with the computed value, and the replacement is reported through `onFieldsDropped` under strict as well. A refusal is runtime behaviour that no conversion can carry, so the strict refusal of a `stored_expression` replacement is deferred to 19.0. A strict write that also carries a refusable drop (`readonly`, `readonly_when`, `primary_key`, `computed`) is refused as before, for those drops only, and fires no listener.
2016
- **Flow step warning.** `@objectstack/service-automation` words the new reason. Its warning says the requested value(s) were not stored, rather than that the field was not written, and its closing sentence says the write succeeded with the computed value(s).
2117

22-
**For consumers that branch on `reason`.** A `Record<DroppedFieldsEvent['reason'], …>` table fails its typecheck until it has a `stored_expression` row. That is the intended signal: add the row.
23-
24-
**Fix.** Nothing, if your client sends the computed value or none. Under `strictReadonlyWrites`, leave the field out of the payload or send the value its expression computes.
18+
**For consumers that branch on `reason`.** A `Record<DroppedFieldsEvent['reason'], …>` table fails its typecheck until it has a `stored_expression` row. That is the intended signal: add the row. A client running behind the server should treat a reason it does not recognise as "not applied", never as one of the known reasons.

‎packages/objectql/src/engine-stored-expression-dropped-fields.test.ts‎

Lines changed: 40 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,11 @@
2020
// grid's consistent save stays silent), nor is a key the caller never sent;
2121
// - a caller value another strip already took is reported once, under that
2222
// strip's reason, never twice;
23-
// - `strictReadonlyWrites` refuses a replaced value, writing nothing — its
24-
// coverage is derived from what `onFieldsDropped` reports.
23+
// - `strictReadonlyWrites` does NOT refuse a replaced value in 18.x (the
24+
// maintainer's ruling 「甲」: only the loud half ships; the refusal waits
25+
// for 19.0) — the write completes with the computed value and the report
26+
// reaches the listener under strict too, while a strict write that ALSO
27+
// carries a refusable drop is still refused, and fires no listener.
2528

2629
import { describe, it, expect, beforeEach } from 'vitest';
2730
import type { DroppedFieldsEvent } from '@objectstack/spec/data';
@@ -251,38 +254,51 @@ describe('[#24690] a caller value a stored expression replaced is reported as `s
251254
});
252255
});
253256

254-
describe('strictReadonlyWrites — coverage derived from the reported set refuses a replaced value', () => {
255-
it('update by id: ERR_READONLY_FIELD_REJECTED carrying the `stored_expression` drop, nothing written', async () => {
256-
const err: any = await engine
257-
.update(LINE, { id: 'line_draft', quantity: 4, amount: 5 }, { strictReadonlyWrites: true } as any)
258-
.catch((e: unknown) => e);
259-
expect(err).toBeInstanceOf(ReadonlyFieldRejectedError);
260-
expect(err.code).toBe('ERR_READONLY_FIELD_REJECTED');
261-
expect(err.drops).toEqual([AMOUNT]);
262-
expect(line('line_draft')).toMatchObject({ quantity: 3, amount: 30 });
257+
describe('strictReadonlyWrites — a replaced value is REPORTED, not refused, in 18.x (ruling 「甲」)', () => {
258+
it('update by id: the write completes with the computed value, and the report reaches the listener', async () => {
259+
const l = listener();
260+
await engine.update(LINE, { id: 'line_draft', quantity: 4, amount: 5 }, { strictReadonlyWrites: true, onFieldsDropped: l.onFieldsDropped } as any);
261+
expect(line('line_draft')).toMatchObject({ quantity: 4, amount: 40 });
262+
expect(l.events).toEqual([AMOUNT]);
263+
});
264+
265+
it('insert: the row is written with the computed value, and reported', async () => {
266+
const l = listener();
267+
await engine.insert(LINE, { invoice: 'INV-1004', quantity: 2, unit_price: 10, amount: 999 }, { strictReadonlyWrites: true, onFieldsDropped: l.onFieldsDropped });
268+
const rows = [...storeFor(LINE).values()];
269+
expect(rows).toHaveLength(4);
270+
expect(rows.find((r) => r.amount === 20 && r.quantity === 2)).toBeDefined();
271+
expect(l.events).toEqual([AMOUNT]);
272+
});
273+
274+
it('predicate update: every matched row is written, and the replacement is reported', async () => {
275+
const l = listener();
276+
await engine.update(LINE, { quantity: 2, unit_price: 5, amount: 1 }, { where: { invoice: 'INV-1004' }, multi: true, strictReadonlyWrites: true, onFieldsDropped: l.onFieldsDropped } as any);
277+
expect([line('line_draft').amount, line('line_draft_2').amount]).toEqual([10, 10]);
278+
expect(l.events).toEqual([AMOUNT]);
263279
});
264280

265-
it('insert: refused the same way, before the driver', async () => {
281+
it('a strict write that ALSO carries a refusable drop is refused for THAT drop alone, writes nothing, and fires no listener', async () => {
282+
const l = listener();
266283
const err: any = await engine
267-
.insert(LINE, { invoice: 'INV-1004', quantity: 2, unit_price: 10, amount: 999 }, { strictReadonlyWrites: true })
284+
.update(LINE, { id: 'line_paid', quantity: 60, amount: 2999.4 }, { strictReadonlyWrites: true, onFieldsDropped: l.onFieldsDropped } as any)
268285
.catch((e: unknown) => e);
269286
expect(err).toBeInstanceOf(ReadonlyFieldRejectedError);
270-
expect(err.drops).toEqual([AMOUNT]);
271-
expect([...storeFor(LINE).values()]).toHaveLength(3);
287+
expect(err.code).toBe('ERR_READONLY_FIELD_REJECTED');
288+
expect(err.drops).toEqual([{ object: LINE, fields: ['quantity'], reason: 'readonly_when' }]);
289+
expect(line('line_paid')).toMatchObject({ quantity: 6, amount: 299.94 });
290+
expect(l.events).toEqual([]);
272291
});
273292

274-
it('predicate update: refused before any row is written', async () => {
293+
it('insert: a strict write with a refusable drop is refused before the computation, carrying no stored_expression drop', async () => {
294+
const l = listener();
275295
const err: any = await engine
276-
.update(LINE, { quantity: 2, unit_price: 5, amount: 1 }, { where: { invoice: 'INV-1004' }, multi: true, strictReadonlyWrites: true } as any)
296+
.insert('qa_locked_line', { quantity: 4, unit_price: 2, amount: 5 }, { strictReadonlyWrites: true, onFieldsDropped: l.onFieldsDropped })
277297
.catch((e: unknown) => e);
278298
expect(err).toBeInstanceOf(ReadonlyFieldRejectedError);
279-
expect(err.drops).toEqual([AMOUNT]);
280-
expect([line('line_draft').amount, line('line_draft_2').amount]).toEqual([30, 14]);
281-
});
282-
283-
it('control: a consistent value is not refused — nothing was replaced', async () => {
284-
await engine.update(LINE, { id: 'line_draft', quantity: 5, unit_price: 20, amount: 100 }, { strictReadonlyWrites: true } as any);
285-
expect(line('line_draft')).toMatchObject({ quantity: 5, amount: 100 });
299+
expect(err.drops.map((d: DroppedFieldsEvent) => d.reason)).not.toContain('stored_expression');
300+
expect([...storeFor('qa_locked_line').values()]).toHaveLength(1);
301+
expect(l.events).toEqual([]);
286302
});
287303
});
288304
});

0 commit comments

Comments
 (0)