Skip to content

Commit 67c29ee

Browse files
os-zhuangclaude
andauthored
fix(objectql): thread execution context into read-time formula evaluation (#1979) (#1988)
`applyFormulaPlan` computes Field.formula virtual fields after find/findOne, but evaluated each expression with only `{ record }`. So a formula using now()/today() ran against a fresh wall-clock read on every evaluation (no determinism), and a formula referencing the caller (os.user.id / os.org.id) faulted and fell back to null because user/org were never in scope. Build the eval context the same way applyFieldDefaults already does: a `now` snapshot pinned once per operation (every row + every formula field in one read observes the same instant) plus os.user / os.org resolved from the ExecutionContext. The two call sites in find()/findOne() pass opCtx.context. Independent of timezone; the read-path prerequisite for ADR-0053 Phase 2 (#1980 will additionally thread `timezone` here once ExecutionContext.timezone exists). Tests: read-time formula resolves os.user.id/os.org.id from context; `now()` is pinned identically across all rows in one find. Full objectql suite green (639). Closes #1979. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 134043a commit 67c29ee

3 files changed

Lines changed: 97 additions & 4 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
"@objectstack/objectql": patch
3+
---
4+
5+
fix(objectql): thread execution context into read-time formula evaluation
6+
7+
`applyFormulaPlan` — which computes `Field.formula` virtual fields after a `find`/`findOne` — evaluated each expression with only `{ record }`. So a formula using `now()`/`today()` ran against a fresh wall-clock read on every evaluation (no determinism), and a formula referencing the caller (`os.user.id`, `os.org.id`) faulted and fell back to `null` because the user/org were never in scope.
8+
9+
It now builds the eval context the same way `applyFieldDefaults` already does: a `now` snapshot **pinned once per operation** (every row and every formula field in one read observes the same instant) plus `os.user` / `os.org` resolved from the `ExecutionContext`. Read-time formulas behave consistently with default-value expressions, and computed fields can reference the caller.
10+
11+
This is independent of timezone; it is the read-path prerequisite for ADR-0053 Phase 2 (#1980 will additionally thread `timezone` here once `ExecutionContext.timezone` exists).

packages/objectql/src/engine.test.ts

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -511,6 +511,65 @@ describe('ObjectQL Engine', () => {
511511
expect(result[0].response_rate).toBe(70);
512512
});
513513

514+
it('evaluates read-time formula fields with execution-context user/org (#1979)', async () => {
515+
// Regression: applyFormulaPlan used to pass only `{ record }`, so a
516+
// computed field referencing the caller (`os.user.id` / `os.org.id`)
517+
// faulted and fell back to null. The context now threads through.
518+
vi.mocked(SchemaRegistry.getObject).mockReturnValue({
519+
name: 'memo',
520+
fields: {
521+
id: { type: 'text' },
522+
body: { type: 'text' },
523+
author_ref: {
524+
type: 'formula',
525+
expression: { dialect: 'cel', source: 'os.user.id' },
526+
},
527+
org_ref: {
528+
type: 'formula',
529+
expression: { dialect: 'cel', source: 'os.org.id' },
530+
},
531+
},
532+
} as any);
533+
534+
vi.mocked(mockDriver.find).mockResolvedValueOnce([
535+
{ id: 'm1', body: 'hello' },
536+
{ id: 'm2', body: 'world' },
537+
]);
538+
539+
const result = await engine.find(
540+
'memo',
541+
{ fields: ['id', 'author_ref', 'org_ref'] } as any,
542+
{ context: { userId: 'u-42', tenantId: 'org-7', roles: ['admin'] } as any },
543+
);
544+
545+
expect(result.map((r: any) => r.author_ref)).toEqual(['u-42', 'u-42']);
546+
expect(result.map((r: any) => r.org_ref)).toEqual(['org-7', 'org-7']);
547+
});
548+
549+
it('pins `now` once per find so every row sees the same instant (#1979)', async () => {
550+
vi.mocked(SchemaRegistry.getObject).mockReturnValue({
551+
name: 'ping',
552+
fields: {
553+
id: { type: 'text' },
554+
ts: {
555+
type: 'formula',
556+
expression: { dialect: 'cel', source: 'now()' },
557+
},
558+
},
559+
} as any);
560+
561+
vi.mocked(mockDriver.find).mockResolvedValueOnce([
562+
{ id: 'a' }, { id: 'b' }, { id: 'c' },
563+
]);
564+
565+
const result = await engine.find('ping', { fields: ['id', 'ts'] } as any);
566+
567+
// Determinism: a single operation snapshots one `now`, shared across
568+
// every row — not a fresh wall-clock read per evaluation.
569+
expect(result[0].ts).toEqual(result[1].ts);
570+
expect(result[1].ts).toEqual(result[2].ts);
571+
});
572+
514573
it('should handle null values gracefully during expand', async () => {
515574
vi.mocked(SchemaRegistry.getObject).mockImplementation((name) => {
516575
if (name === 'task') return {

packages/objectql/src/engine.ts

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,12 +85,35 @@ function planFormulaProjection(
8585
return { plan };
8686
}
8787

88-
function applyFormulaPlan(plan: FormulaPlanEntry[], records: any[]): void {
88+
/**
89+
* Evaluate read-time formula virtual fields against the raw rows.
90+
*
91+
* The eval context mirrors `applyFieldDefaults` so formula and default
92+
* expressions see the same shape: a `now` pinned ONCE per operation (every row
93+
* and every formula field in one `find()` observes the same instant —
94+
* determinism, and no per-eval `new Date()` drift), plus `os.user` / `os.org`
95+
* resolved from the execution context (so a computed field can reference the
96+
* caller, e.g. `os.user.id`). Previously this passed only `{ record }`, so
97+
* `now()`/`today()` ran against live wall-clock and user/org were unreachable.
98+
*
99+
* (ADR-0053 Phase 2 will additionally thread `timezone` here once
100+
* `ExecutionContext.timezone` exists — see #1980; this change is independent
101+
* of timezone.)
102+
*/
103+
function applyFormulaPlan(
104+
plan: FormulaPlanEntry[],
105+
records: any[],
106+
execCtx?: ExecutionContext,
107+
nowSnapshot?: Date,
108+
): void {
89109
if (!plan.length) return;
110+
const now = nowSnapshot ?? new Date();
111+
const user = execCtx?.userId ? { id: String(execCtx.userId), role: execCtx?.roles?.[0] } : undefined;
112+
const org = execCtx?.tenantId ? { id: String(execCtx.tenantId) } : undefined;
90113
for (const rec of records) {
91114
if (rec == null) continue;
92115
for (const fp of plan) {
93-
const r = ExpressionEngine.evaluate(fp.expression, { record: rec });
116+
const r = ExpressionEngine.evaluate(fp.expression, { now, user, org, record: rec });
94117
rec[fp.name] = r.ok ? r.value : null;
95118
}
96119
}
@@ -1866,7 +1889,7 @@ export class ObjectQL implements IDataEngine {
18661889
let result = await driver.find(object, hookContext.input.ast as QueryAST, hookContext.input.options as any);
18671890

18681891
// Post-process: evaluate formula virtual fields against the raw rows
1869-
if (Array.isArray(result)) applyFormulaPlan(_findFormula.plan, result);
1892+
if (Array.isArray(result)) applyFormulaPlan(_findFormula.plan, result, opCtx.context);
18701893

18711894
// Post-process: expand related records if expand is requested
18721895
if (ast.expand && Object.keys(ast.expand).length > 0 && Array.isArray(result)) {
@@ -1932,7 +1955,7 @@ export class ObjectQL implements IDataEngine {
19321955
let result = await driver.findOne(objectName, opCtx.ast as QueryAST, findOneOpts);
19331956

19341957
// Post-process: evaluate formula virtual fields against the raw row
1935-
if (result != null) applyFormulaPlan(_findOneFormula.plan, [result]);
1958+
if (result != null) applyFormulaPlan(_findOneFormula.plan, [result], opCtx.context);
19361959

19371960
// Post-process: expand related records if expand is requested
19381961
if (ast.expand && Object.keys(ast.expand).length > 0 && result != null) {

0 commit comments

Comments
 (0)