Skip to content

Commit 5ac93d4

Browse files
authored
feat(rest): surface silently-dropped write fields on PATCH/POST /data (#3431) (#3448)
Wire the engine's onFieldsDropped strip-observability channel (#3413) through the DataProtocol and REST write path. updateData collects the engine's readonly/readonlyWhen strips; createData surfaces the #3043 static-readonly ingress strip via a payload diff. Both attach an optional droppedFields to the response. spec: Update/CreateDataResponseSchema gain optional droppedFields (backward-compatible). rest: PATCH/POST echo drops as the X-ObjectStack-Dropped-Fields header and keep the structured list on the body; status/success unchanged. Deferred to a follow-up: bulk (updateManyData/createManyData/batchData) + GraphQL wiring, typed @objectstack/client warnings, CORS exposeHeaders.
1 parent 32ff033 commit 5ac93d4

7 files changed

Lines changed: 398 additions & 8 deletions

File tree

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/metadata-protocol": minor
4+
"@objectstack/rest": minor
5+
---
6+
7+
feat(rest): surface silently-dropped write fields on PATCH/POST /data (#3431)
8+
9+
#3413 (closes #3407) built the engine-level strip-observability channel
10+
(`WriteObservabilityOptions.onFieldsDropped`) and wired the flow side
11+
(`update_record` / `create_record` emit a step warning + `droppedFields`). The
12+
**REST write path was never wired**, so an external API caller writing N fields
13+
still got a bare `200 + record` when `readonly` (#2948) / `readonlyWhen` (#3042)
14+
stripping meant `< N` actually landed — the same silent-success class #3407
15+
fixed flow-side, just on HTTP. The only way to notice was a per-field diff of
16+
the returned row (which need not echo every field). This wires the channel
17+
through the protocol → REST, on both write verbs.
18+
19+
**Passthrough (metadata-protocol).** `updateData` now registers an
20+
`onFieldsDropped` collector on `engine.update` and returns the events on the
21+
response as `droppedFields`. `createData` surfaces the #3043 static-`readonly`
22+
INGRESS strip too — that strip runs at the protocol ingress
23+
(`stripReadonlyForInsert`), *before* the engine, so it is recovered by diffing
24+
the supplied payload against the stripped one (the engine's `onFieldsDropped` is
25+
also wired for a future insert-side engine strip). A faulty listener never
26+
breaks the write — the engine catches and logs.
27+
28+
**Contract (spec).** `UpdateDataResponseSchema` / `CreateDataResponseSchema`
29+
gain an **optional** `droppedFields: DroppedFieldsEvent[]` — present only when
30+
≥1 field was dropped. Optional + omit-when-empty keeps the response shape
31+
backward-compatible for clients that only read `record`.
32+
33+
**REST surface.** PATCH `/data/:object/:id` and POST `/data/:object` echo the
34+
drops as an `X-ObjectStack-Dropped-Fields` response header
35+
(`field;reason=<reason>` tokens, comma-joined — e.g.
36+
`approval_status;reason=readonly`) and keep the structured `droppedFields` on
37+
the body. **Status/success semantics are unchanged** (200 update / 201 create) —
38+
a strip is legitimate semantics, not a failure (same principle as #3413). The
39+
FLS write gate is untouched (it already fails closed with 403).
40+
41+
Out of scope (issue #3431 D2 open questions, deferred): bulk
42+
(`updateManyData` / `createManyData` / `batchData`) and GraphQL mutation wiring,
43+
typed `@objectstack/client` warnings, and adding the header to the Hono CORS
44+
`exposeHeaders` allow-list for cross-origin browser reads (the body
45+
`droppedFields` is the cross-origin-safe channel meanwhile).

content/docs/references/api/protocol.mdx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,7 @@ const result = AiInsightsRequest.parse(data);
219219
| **object** | `string` || The object name. |
220220
| **id** | `string` || The ID of the newly created record. |
221221
| **record** | `Record<string, any>` || The created record, including server-generated fields (created_at, owner). |
222+
| **droppedFields** | `{ object: string; fields: string[]; reason: Enum<'readonly' \| 'readonly_when'> }[]` | optional | Write-observability (#3407/#3431): caller-supplied fields that were LEGALLY stripped before the record was written — a non-system create cannot seed a static `readonly` column (#3043 ingress strip), so those keys are dropped and the field re-derives its default. Present ONLY when ≥1 field was dropped; the create still succeeded without them (status/success semantics unchanged). REST additionally surfaces this as the `X-ObjectStack-Dropped-Fields` response header. Optional — omit-when-empty keeps the shape backward-compatible for existing clients. |
222223

223224

224225
---
@@ -1179,6 +1180,7 @@ const result = AiInsightsRequest.parse(data);
11791180
| **object** | `string` || Object name |
11801181
| **id** | `string` || Updated record ID |
11811182
| **record** | `Record<string, any>` || Updated record |
1183+
| **droppedFields** | `{ object: string; fields: string[]; reason: Enum<'readonly' \| 'readonly_when'> }[]` | optional | Write-observability (#3407/#3431): caller-supplied fields the engine LEGALLY stripped from the write before persisting — static `readonly` (#2948) or a TRUE `readonlyWhen` predicate (#3042). Present ONLY when ≥1 field was dropped; the update still succeeded without them (status/success semantics unchanged — stripping is legitimate, not an error). REST additionally surfaces this as the `X-ObjectStack-Dropped-Fields` response header. Optional — omit-when-empty keeps the shape backward-compatible for existing clients that only read `record`. |
11821184

11831185

11841186
---
Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// [#3431] REST/API write paths must not SILENTLY drop caller-supplied fields.
4+
// #3413 built the engine-level `onFieldsDropped` channel and wired the flow
5+
// (`update_record`) side; this locks the DataProtocol passthrough that carries
6+
// the strip back to the REST layer:
7+
// - updateData forwards the engine's onFieldsDropped events (readonly /
8+
// readonly_when) onto the response as `droppedFields`;
9+
// - createData surfaces the #3043 static-`readonly` INGRESS strip, which runs
10+
// BEFORE the engine (so it is recovered by diffing the payload, not via the
11+
// engine listener) — symmetric with update;
12+
// - no strip → NO `droppedFields` key, so the response shape stays
13+
// backward-compatible for clients that only read `record`.
14+
15+
import { describe, it, expect, vi } from 'vitest';
16+
import { ObjectStackProtocolImplementation } from './protocol.js';
17+
18+
const SCHEMA = {
19+
name: 'approval_case',
20+
fields: {
21+
title: { name: 'title', type: 'text' },
22+
approval_status: { name: 'approval_status', type: 'text', readonly: true, defaultValue: 'draft' },
23+
},
24+
};
25+
26+
describe('updateData — forwards engine write strips as droppedFields (#3431)', () => {
27+
it('surfaces a readonly strip the engine reports via onFieldsDropped', async () => {
28+
const engine = {
29+
registry: { getObject: () => SCHEMA },
30+
// Stand in for the engine stripping `approval_status` and reporting it.
31+
update: vi.fn(async (object: string, data: any, options?: any) => {
32+
options?.onFieldsDropped?.({ object, fields: ['approval_status'], reason: 'readonly' });
33+
return { id: 'rec-1', title: data.title };
34+
}),
35+
findOne: vi.fn(async () => null),
36+
};
37+
const p = new ObjectStackProtocolImplementation(engine as any);
38+
const res: any = await p.updateData({
39+
object: 'approval_case',
40+
id: 'rec-1',
41+
data: { title: 'B', approval_status: 'approved' },
42+
context: { userId: 'u1' },
43+
});
44+
expect(res.droppedFields).toEqual([
45+
{ object: 'approval_case', fields: ['approval_status'], reason: 'readonly' },
46+
]);
47+
// The write still succeeded; the returned record is unchanged in shape.
48+
expect(res.record).toEqual({ id: 'rec-1', title: 'B' });
49+
});
50+
51+
it('forwards multiple strip passes in order (readonly_when then readonly)', async () => {
52+
const engine = {
53+
registry: { getObject: () => SCHEMA },
54+
update: vi.fn(async (object: string, _data: any, options?: any) => {
55+
options?.onFieldsDropped?.({ object, fields: ['locked'], reason: 'readonly_when' });
56+
options?.onFieldsDropped?.({ object, fields: ['approval_status'], reason: 'readonly' });
57+
return { id: 'rec-1' };
58+
}),
59+
findOne: vi.fn(async () => null),
60+
};
61+
const p = new ObjectStackProtocolImplementation(engine as any);
62+
const res: any = await p.updateData({ object: 'approval_case', id: 'rec-1', data: {} });
63+
expect(res.droppedFields).toEqual([
64+
{ object: 'approval_case', fields: ['locked'], reason: 'readonly_when' },
65+
{ object: 'approval_case', fields: ['approval_status'], reason: 'readonly' },
66+
]);
67+
});
68+
69+
it('omits droppedFields entirely when the engine stripped nothing', async () => {
70+
const engine = {
71+
registry: { getObject: () => SCHEMA },
72+
update: vi.fn(async (_o: string, data: any) => ({ id: 'rec-1', ...data })),
73+
findOne: vi.fn(async () => null),
74+
};
75+
const p = new ObjectStackProtocolImplementation(engine as any);
76+
const res: any = await p.updateData({ object: 'approval_case', id: 'rec-1', data: { title: 'B' } });
77+
expect(res).not.toHaveProperty('droppedFields');
78+
});
79+
});
80+
81+
describe('createData — surfaces the #3043 ingress readonly strip as droppedFields (#3431)', () => {
82+
function makeProtocol() {
83+
const engine = {
84+
registry: { getObject: (n: string) => (n === 'approval_case' ? SCHEMA : undefined) },
85+
insert: vi.fn(async (_object: string, data: any) => ({ id: 'rec-1', ...data })),
86+
};
87+
return { p: new ObjectStackProtocolImplementation(engine as any), engine };
88+
}
89+
90+
it('reports the forged readonly field a non-system create dropped', async () => {
91+
const { p } = makeProtocol();
92+
const res: any = await p.createData({
93+
object: 'approval_case',
94+
data: { title: 'A', approval_status: 'approved' },
95+
context: { userId: 'u1' },
96+
});
97+
expect(res.droppedFields).toEqual([
98+
{ object: 'approval_case', fields: ['approval_status'], reason: 'readonly' },
99+
]);
100+
// Stripped field is absent from the persisted payload (existing #3043 behaviour).
101+
expect(res.record).not.toHaveProperty('approval_status');
102+
});
103+
104+
it('omits droppedFields when the create seeds no readonly field', async () => {
105+
const { p } = makeProtocol();
106+
const res: any = await p.createData({
107+
object: 'approval_case',
108+
data: { title: 'A' },
109+
context: { userId: 'u1' },
110+
});
111+
expect(res).not.toHaveProperty('droppedFields');
112+
});
113+
114+
it('a system-context create keeps the field — no strip, no droppedFields', async () => {
115+
const { p } = makeProtocol();
116+
const res: any = await p.createData({
117+
object: 'approval_case',
118+
data: { title: 'A', approval_status: 'approved' },
119+
context: { isSystem: true },
120+
});
121+
expect(res).not.toHaveProperty('droppedFields');
122+
expect(res.record.approval_status).toBe('approved');
123+
});
124+
});

packages/metadata-protocol/src/protocol.ts

Lines changed: 54 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import type {
1616
} from '@objectstack/spec/api';
1717
import type { MetadataCacheRequest, MetadataCacheResponse, ServiceInfo, ApiRoutes, WellKnownCapabilities } from '@objectstack/spec/api';
1818
import { readServiceSelfInfo } from '@objectstack/spec/api';
19-
import { parseFilterAST, isFilterAST } from '@objectstack/spec/data';
19+
import { parseFilterAST, isFilterAST, type DroppedFieldsEvent } from '@objectstack/spec/data';
2020
import { PLURAL_TO_SINGULAR, SINGULAR_TO_PLURAL } from '@objectstack/spec/shared';
2121
import { type FormView, isAggregatedViewContainer } from '@objectstack/spec/ui';
2222
import { METADATA_FORM_REGISTRY } from '@objectstack/spec/system';
@@ -588,6 +588,34 @@ function stripReadonlyForInsert(schema: any, data: any, context: any): any {
588588
return Array.isArray(data) ? data.map(stripRow) : stripRow(data);
589589
}
590590

591+
/**
592+
* [#3431] Recover a `DroppedFieldsEvent` from a before/after write-payload diff.
593+
*
594+
* The UPDATE strips (static `readonly` / `readonlyWhen`) run INSIDE the engine,
595+
* which reports them via the `onFieldsDropped` listener (wired in `updateData`).
596+
* The CREATE `readonly` strip, however, runs at THIS protocol ingress
597+
* (`stripReadonlyForInsert`, #3043) — BEFORE the engine — so the engine listener
598+
* never sees it. Diffing the caller-supplied keys against the stripped payload
599+
* recovers exactly which supplied fields the ingress strip removed, so the create
600+
* path can surface them symmetrically with update.
601+
*
602+
* Returns `null` when nothing was dropped (same reference, non-object, array, or
603+
* no key delta) so callers can `if (ev) dropped.push(ev)` without emitting empty
604+
* events. Mirrors the engine's own before/after key-set diff (`reportDroppedFields`
605+
* in objectql/engine.ts) so both channels agree on what "dropped" means.
606+
*/
607+
function diffDroppedFields(
608+
object: string,
609+
before: unknown,
610+
after: unknown,
611+
reason: DroppedFieldsEvent['reason'],
612+
): DroppedFieldsEvent | null {
613+
if (before === after || before == null || typeof before !== 'object' || Array.isArray(before)) return null;
614+
const afterObj = (after ?? {}) as Record<string, unknown>;
615+
const fields = Object.keys(before as Record<string, unknown>).filter((k) => !(k in afterObj));
616+
return fields.length > 0 ? { object, fields, reason } : null;
617+
}
618+
591619
/**
592620
* Service Configuration for Discovery
593621
* Maps service names to their routes and plugin providers.
@@ -2829,15 +2857,24 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol {
28292857
request.data,
28302858
request.context,
28312859
);
2832-
const result = await this.engine.insert(
2833-
request.object,
2834-
data,
2835-
request.context !== undefined ? { context: request.context } as any : undefined,
2836-
);
2860+
// [#3431] The #3043 ingress strip above is SILENT by contract; surface it
2861+
// so a REST/API caller learns which supplied fields were dropped, symmetric
2862+
// with `updateData`. The strip lives at THIS ingress (not the engine, which
2863+
// is INSERT-readonly-exempt, #3413), so recover it by diffing the supplied
2864+
// payload against the stripped one. The engine's `onFieldsDropped` is ALSO
2865+
// wired below so a FUTURE insert-side engine strip surfaces automatically
2866+
// through the same list instead of going silent.
2867+
const dropped: DroppedFieldsEvent[] = [];
2868+
const ingressDropped = diffDroppedFields(request.object, request.data, data, 'readonly');
2869+
if (ingressDropped) dropped.push(ingressDropped);
2870+
const opts: any = { onFieldsDropped: (e: DroppedFieldsEvent) => { dropped.push(e); } };
2871+
if (request.context !== undefined) opts.context = request.context;
2872+
const result = await this.engine.insert(request.object, data, opts);
28372873
return {
28382874
object: request.object,
28392875
id: result.id,
2840-
record: result
2876+
record: result,
2877+
...(dropped.length > 0 ? { droppedFields: dropped } : {}),
28412878
};
28422879
}
28432880

@@ -2928,11 +2965,20 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol {
29282965
await this.assertVersionMatch(request.object, request.id, request.expectedVersion, request.context);
29292966
const opts: any = { where: { id: request.id } };
29302967
if (request.context !== undefined) opts.context = request.context;
2968+
// [#3407/#3431] Capture the engine's LEGAL write strips (static `readonly`
2969+
// (#2948) / TRUE `readonlyWhen` (#3042)) so a REST/API caller is not left
2970+
// to field-by-field diff the returned row to discover a value never
2971+
// landed. The write still succeeds — this only makes the strip observable,
2972+
// mirroring service-automation's `update_record` wiring (#3413). A faulty
2973+
// listener never breaks the write (the engine catches + logs).
2974+
const dropped: DroppedFieldsEvent[] = [];
2975+
opts.onFieldsDropped = (e: DroppedFieldsEvent) => { dropped.push(e); };
29312976
const result = await this.engine.update(request.object, request.data, opts);
29322977
return {
29332978
object: request.object,
29342979
id: request.id,
2935-
record: result
2980+
record: result,
2981+
...(dropped.length > 0 ? { droppedFields: dropped } : {}),
29362982
};
29372983
}
29382984

0 commit comments

Comments
 (0)