Skip to content

Commit ff648ad

Browse files
authored
fix(data): resolve field defaultValues before beforeInsert hook (#2703) (#2748)
Declarative field defaults (incl. the current_user token) were resolved after the beforeInsert hook, so a hook deriving one field from another read a stale null for any field about to be defaulted. Move applyFieldDefaults to record-initialization time, before beforeInsert — matching the standard order of execution (declarative defaults before before-triggers; autonumber/encryption/validation stay after). The hook still runs after and may override defaults; caller input is no longer mutated in place. Adds a regression test reproducing the os-tianshun-mtc#29 scenario.
1 parent bea4b92 commit ff648ad

3 files changed

Lines changed: 77 additions & 12 deletions

File tree

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
---
2+
'@objectstack/objectql': patch
3+
---
4+
5+
fix(data): resolve field `defaultValue`s BEFORE the `beforeInsert` hook (#2703)
6+
7+
Declarative field defaults (including the `current_user` token) were resolved
8+
by `applyFieldDefaults` *after* the user `beforeInsert` hook ran. A hook that
9+
DERIVED one field from another therefore read a stale `null` for any field that
10+
was about to be defaulted — e.g. `sales_person: Field.user({ defaultValue:
11+
'current_user' })` left `sales_person == null` inside the hook, so a derived
12+
`current_status` computed to `unassigned` unless the client passed the field
13+
explicitly.
14+
15+
`applyFieldDefaults` now runs at record-initialization time, before
16+
`beforeInsert`, matching the industry-standard order of execution (Salesforce
17+
field defaults / ServiceNow dictionary defaults are populated before before-
18+
triggers; engine-owned generation — autonumber sequences, encryption, timestamps
19+
— stays after the hook). The hook still has final say: it runs after and may
20+
override any defaulted field. Defaults still only fill fields left `undefined`,
21+
so client-supplied values are untouched, and the caller's input object is no
22+
longer mutated in place.
23+
24+
Behavior note: a `beforeInsert` hook can no longer distinguish "client omitted
25+
field X" from "field X received its default" for fields that declare a
26+
`defaultValue` — the hook now always sees the resolved default. This matches how
27+
Salesforce/ServiceNow behave (before logic sees a fully-initialized record) and
28+
is the intended fix.

packages/objectql/src/engine.test.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,36 @@ describe('ObjectQL Engine', () => {
319319
expect(arg.owner).toBeUndefined();
320320
});
321321

322+
it('resolves field defaults BEFORE beforeInsert so a hook can derive from them (#2703)', async () => {
323+
vi.mocked(SchemaRegistry.getObject).mockImplementation((name) => {
324+
if (name === 'ticket') return {
325+
name: 'ticket',
326+
fields: {
327+
title: { type: 'text' },
328+
owner: { type: 'user', reference: 'sys_user', defaultValue: 'current_user' },
329+
current_status: { type: 'text' },
330+
},
331+
} as any;
332+
if (name === 'sys_user') return { name: 'sys_user', fields: { name: { type: 'text' } } } as any;
333+
return undefined;
334+
});
335+
336+
// A beforeInsert hook that DERIVES `current_status` from the defaulted
337+
// `owner` field — the exact os-tianshun-mtc#29 scenario.
338+
engine.registerHook('beforeInsert', async (ctx: any) => {
339+
const data = ctx.input.data;
340+
data.current_status = data.owner ? 'assigned' : 'unassigned';
341+
}, { object: 'ticket' });
342+
343+
await engine.insert('ticket', { title: 'T1' }, { context: { userId: 'u-42' } as any });
344+
345+
expect(mockDriver.create).toHaveBeenCalledWith(
346+
'ticket',
347+
expect.objectContaining({ title: 'T1', owner: 'u-42', current_status: 'assigned' }),
348+
expect.anything(),
349+
);
350+
});
351+
322352
it('should execute find operation', async () => {
323353
const result = await engine.find('task', {});
324354
expect(mockDriver.find).toHaveBeenCalled();

packages/objectql/src/engine.ts

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2131,10 +2131,23 @@ export class ObjectQL implements IDataEngine {
21312131
};
21322132

21332133
await this.executeWithMiddleware(opCtx, async () => {
2134+
// Resolve field `defaultValue`s (including the `current_user` token)
2135+
// BEFORE the beforeInsert hook runs, so a hook that DERIVES one field
2136+
// from another can read the defaulted value instead of a stale `null`
2137+
// (#2703). The hook still has final say — it runs after and may override
2138+
// any defaulted field. `applyFieldDefaults` returns a fresh copy and only
2139+
// fills fields left `undefined`, so client-supplied values are untouched.
2140+
const nowSnap = new Date();
2141+
const defaultedData = Array.isArray(opCtx.data)
2142+
? (opCtx.data as any[]).map((row) =>
2143+
this.applyFieldDefaults(object, row as Record<string, unknown>, opCtx.context, nowSnap),
2144+
)
2145+
: this.applyFieldDefaults(object, opCtx.data as Record<string, unknown>, opCtx.context, nowSnap);
2146+
21342147
const hookContext: HookContext = {
21352148
object,
21362149
event: 'beforeInsert',
2137-
input: { data: opCtx.data, options: opCtx.options },
2150+
input: { data: defaultedData, options: opCtx.options },
21382151
session: this.buildSession(opCtx.context),
21392152
api: this.buildHookApi(opCtx.context),
21402153
transaction: opCtx.context?.transaction,
@@ -2150,16 +2163,14 @@ export class ObjectQL implements IDataEngine {
21502163

21512164
try {
21522165
let result;
2153-
const nowSnap = new Date();
21542166
const schemaForValidation = this._registry.getObject(object);
21552167
// When the driver generates autonumbers natively (persistent SQL
21562168
// sequence), the engine defers to it — see #1603.
21572169
const driverOwnsAutonumber = (driver as any)?.supports?.autonumber === true;
21582170
if (Array.isArray(hookContext.input.data)) {
2159-
// Bulk Create — apply defaults per row
2160-
const rows = (hookContext.input.data as any[]).map((row) =>
2161-
this.applyFieldDefaults(object, row as Record<string, unknown>, opCtx.context, nowSnap),
2162-
);
2171+
// Defaults are already resolved above (pre-hook, #2703); the hook may
2172+
// have overridden or added fields — take its data as-is.
2173+
const rows = hookContext.input.data as Array<Record<string, unknown>>;
21632174
for (const r of rows) {
21642175
await this.applyAutonumbers(object, r as Record<string, unknown>, opCtx.context, driverOwnsAutonumber);
21652176
}
@@ -2178,12 +2189,8 @@ export class ObjectQL implements IDataEngine {
21782189
result = await Promise.all(rows.map((item) => driver.create(object, item, hookContext.input.options as any)));
21792190
}
21802191
} else {
2181-
const row = this.applyFieldDefaults(
2182-
object,
2183-
hookContext.input.data as Record<string, unknown>,
2184-
opCtx.context,
2185-
nowSnap,
2186-
);
2192+
// Defaults already resolved pre-hook (#2703); use the hook's data.
2193+
const row = hookContext.input.data as Record<string, unknown>;
21872194
await this.applyAutonumbers(object, row, opCtx.context, driverOwnsAutonumber);
21882195
await this.encryptSecretFields(object, row, opCtx.context, hookContext.input.options);
21892196
normalizeMultiValueFields(schemaForValidation, row);

0 commit comments

Comments
 (0)