From 084ce6e645d35fdea4851945011c1ac7ac41c0b5 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 11:58:01 +0000 Subject: [PATCH] feat(core): a computed field runs only when it is going to be returned (#855, ADR-0027) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A computed field (any field with a resolveOutput hook, virtual or not) is now computed, and its declared relations (needs) fetched, only when a fragment `query` read is actually going to return it — projection-aware, applied recursively at every nesting level. Bare and include-based reads are unaffected. A computed field's hook also no longer sees another computed field's resolved output as part of its item, on any read path, closing the declaration-order dependency bug ADR-0025 left open. Closes #855 Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_018wgSw8qrPmMBy8d97LuGoh --- .changeset/silent-hooks-compute-once.md | 41 ++ packages/core/CLAUDE.md | 12 +- .../core/src/access/declared-dependencies.ts | 37 +- packages/core/src/access/field-visibility.ts | 111 ++++- packages/core/src/context/index.ts | 56 ++- packages/core/src/query/index.ts | 53 +++ ...omputed-field-selective-evaluation.test.ts | 418 ++++++++++++++++++ 7 files changed, 696 insertions(+), 32 deletions(-) create mode 100644 .changeset/silent-hooks-compute-once.md create mode 100644 packages/core/tests/computed-field-selective-evaluation.test.ts diff --git a/.changeset/silent-hooks-compute-once.md b/.changeset/silent-hooks-compute-once.md new file mode 100644 index 00000000..7a1904fd --- /dev/null +++ b/.changeset/silent-hooks-compute-once.md @@ -0,0 +1,41 @@ +--- +'@opensaas/stack-core': minor +--- + +A computed field — any field carrying a `resolveOutput` hook, virtual or not — is now computed if and only if a read is actually going to return it. A fragment `query` that selects three fields no longer runs every `resolveOutput` on the list and discards the rest: an unselected field's field-level read access is never evaluated and its hook never runs. Its declared relations (`needs`, ADR-0025) are fetched under exactly the same condition, folded recursively at every nesting level — a nested fragment selecting a subset computes only that subset, while a nested `include` still computes every computed field at that level, matching bare and `include`-based reads, which are unaffected: they still compute every computed field on the list, exactly as before. See ADR-0027. + +**This is a silent break — detect it before you upgrade, the same way ADR-0024's and ADR-0026's were.** Two independent behaviors changed with no thrown error: + +1. **A hook's `item` never carries another computed field's resolved output, on any read path.** Previously a virtual field received the already-assembled, already-resolved object, so a virtual field could read an _earlier-declared_ virtual (or any field carrying its own `resolveOutput`, e.g. a `password()`'s wrapper or a formatted display field) and see its resolved value — working only by declaration order, with reordering two fields silently changing the result. Now every computed field's hook sees only the row's stored columns and its own declared dependencies; reaching for a sibling that is itself computed finds nothing there (or its raw stored form, never the wrapped/resolved value), the same as reaching for a field that was never declared. **Grep your config for a `resolveOutput` whose `item` reads a field that is itself computed** — virtual fields reading other virtual fields, or a hook reading a stored field that carries its own `resolveOutput` (a password wrapper, a formatted date) — and recompute from the shared stored columns instead of relying on another field's hook having already run. +2. **A field's hook no longer runs just because it's on the list — only because a read selects it.** If you relied on a `resolveOutput` hook running for a side effect (logging, cache warming) on every read regardless of a fragment's own field selection, that side effect now only fires when the fragment actually names the field. **Grep for a fragment `query` that intentionally omits a field whose hook you were relying on for a side effect**, and select that field explicitly (or move the side effect to a hook that isn't projection-gated, e.g. `afterOperation`). + +A hookless virtual field (one with `access.read` but no `resolveOutput`) no longer has its read access evaluated at all on any read — such a field can never produce output, so under this rule it does no work at all. + +```typescript +// Before: `displayName` (declared after `fullNameCached`) could read the +// latter's resolved value purely because of declaration order. +User: list({ + fields: { + firstName: text(), + lastName: text(), + fullNameCached: virtual({ + type: 'string', + hooks: { resolveOutput: ({ item }) => `${item.firstName} ${item.lastName}` }, + }), + displayName: virtual({ + type: 'string', + // item.fullNameCached is now always undefined here — recompute from + // the shared stored columns instead. + hooks: { resolveOutput: ({ item }) => `${item.fullNameCached} (${item.firstName[0]}.)` }, + }), + }, +}) + +// After: compute from the stored columns both fields actually share. +displayName: virtual({ + type: 'string', + hooks: { + resolveOutput: ({ item }) => `${item.firstName} ${item.lastName} (${item.firstName[0]}.)`, + }, +}), +``` diff --git a/packages/core/CLAUDE.md b/packages/core/CLAUDE.md index 4f44b310..ad9e1925 100644 --- a/packages/core/CLAUDE.md +++ b/packages/core/CLAUDE.md @@ -261,6 +261,14 @@ The read pipeline is **caller-directed**: `buildAccessScopedInclude` walks only A computed field's declared dependency (`needs`, ADR-0025, below) folds in at **every** relation it's reached through, including one added purely to satisfy another field's own `needs` — the fold recurses through `foldDeclaredDependencies` rather than riding a caller-named relation's auto-expanded subtree, since nothing auto-expands anymore. See `docs/adr/0026-naming-a-relation-fetches-its-columns-not-its-subtree.md`. +### A Computed Field Runs Only When It Is Going To Be Returned (ADR-0027) + +A computed field — any field carrying a `resolveOutput` hook, virtual or not — is computed **if and only if the read is actually going to return it**, and its declared relations (`needs`) are fetched under exactly the same condition. A fragment `query` selecting three fields runs only those three fields' hooks (and folds only their `needs`); a field it doesn't select does no work at all — neither its field-level `read` access nor its hook runs. This is **projection-aware, never access-aware**: a fragment's own field selection is the only thing that restricts a level this way. A bare read or an `include`-based read is unaffected — every computed field on the list still computes, exactly as before, since neither ever had a narrower field selection to restrict by. The rule applies at every nesting level: a nested fragment selecting a subset computes only that subset there; a nested `include` still computes every computed field at that level. + +**A computed field's hook never sees another computed field's resolved output**, on any read path — only the row's stored columns and its own declared dependencies. A sibling field that was skipped (unselected by a fragment) or denied by field-level access is absent from what the hook sees, never present holding its raw pre-hook value — reaching for it finds nothing there, the same as reaching for a relation never declared via `needs`. Before this, a virtual field received the already-assembled, already-resolved object, so a virtual field could accidentally read an earlier-declared virtual's resolved value purely by declaration order; reordering two such fields silently changed the result. That accidental coupling is gone: recompute from the stored columns both fields share instead. + +A hookless virtual field (one with `access.read` but no `resolveOutput`) has its read access evaluated on no read at all — such a field can never produce output, so there's nothing to preserve access side effects for. See `docs/adr/0027-a-computed-field-runs-only-when-it-is-going-to-be-returned.md` and the "Computed field" glossary entry in `CONTEXT.md`. + ### Context Type Safety Context uses generic typing to preserve Prisma types: @@ -412,13 +420,13 @@ User: list({ // Usage const user = await context.db.user.findUnique({ where: { id } }) -console.log(user.fullName) // "John Doe" — computed via resolveOutput on every read +console.log(user.fullName) // "John Doe" — computed via resolveOutput whenever the read returns it ``` **Key characteristics:** - Not stored in database (no Prisma column created) -- Computed via `resolveOutput` on every read (`select` is not honoured — narrow with `include`/fragment `query`) +- Computed via `resolveOutput` on every bare/`include`-based read; on a fragment `query` read, only when the fragment selects it (ADR-0027) — `select` is still not honoured, narrow with `include`/fragment `query` - Must provide `type` (TypeScript type string) and `resolveOutput` hook - Can optionally provide `resolveInput` for write side effects - Useful for derived values, computed properties, and external API sync diff --git a/packages/core/src/access/declared-dependencies.ts b/packages/core/src/access/declared-dependencies.ts index d3984dbc..0e617d65 100644 --- a/packages/core/src/access/declared-dependencies.ts +++ b/packages/core/src/access/declared-dependencies.ts @@ -1,5 +1,6 @@ import type { FieldConfig, OpenSaasConfig } from '../config/types.js' import { getRelatedListConfig } from './engine.js' +import type { FieldSelectionScope } from '../query/index.js' /** * Declared Dependencies — folding a computed field's `needs` into a read's @@ -102,11 +103,22 @@ function getExplicitInclude(value: unknown): Record | undefined * list that have a `resolveOutput` hook. A `needs` entry on a field without * one is inert — there is no hook to feed it to — so it contributes nothing * to fetch. + * + * `selectedFields`, when given, restricts the union to fields the read is + * actually going to return (ADR-0027) — a field a fragment did not select is + * never computed, so its declared relation is never fetched for it either. + * `undefined` means unrestricted: every field with a hook contributes, + * matching a bare or `include`-based read, which always returns every + * computed field on the list. */ -export function getDeclaredRelationNames(fieldConfigs: Record): string[] { +export function getDeclaredRelationNames( + fieldConfigs: Record, + selectedFields?: ReadonlySet, +): string[] { const names = new Set() - for (const fieldConfig of Object.values(fieldConfigs)) { + for (const [fieldName, fieldConfig] of Object.entries(fieldConfigs)) { if (!fieldConfig?.hooks?.resolveOutput) continue + if (selectedFields && !selectedFields.has(fieldName)) continue for (const name of fieldConfig.needs ?? []) { names.add(name) } @@ -127,6 +139,15 @@ export function getDeclaredRelationNames(fieldConfigs: Record | undefined, @@ -134,8 +155,9 @@ export function foldDeclaredDependencies( config: OpenSaasConfig, listKey: string, visitedLists: readonly string[] = [listKey], + selection?: FieldSelectionScope, ): { include: Record | undefined; declaredOnly: DeclaredOnlyTree } { - const declaredNames = getDeclaredRelationNames(fieldConfigs) + const declaredNames = getDeclaredRelationNames(fieldConfigs, selection?.fields) if (declaredNames.length === 0 && !rawInclude) { return { include: rawInclude, declaredOnly: emptyDeclaredOnlyTree() } @@ -166,6 +188,14 @@ export function foldDeclaredDependencies( // that merely revisits a list (e.g. `Post → author → posts`). if (declaredOnly.keys.has(key) && visitedLists.includes(relatedConfig.listName)) continue + // A branch added purely by the fold has no fragment scope of its own — + // it folds unrestricted, as before ADR-0027. A branch the request itself + // named (caller include or fragment) carries that name's own nested + // scope, if the fragment gave it one (a bare `true` selector leaves it + // `undefined` — also unrestricted, since the caller asked for + // "everything" there). + const nestedSelection = declaredOnly.keys.has(key) ? undefined : selection?.nested[key] + const explicitNested = getExplicitInclude(value) const nested = foldDeclaredDependencies( explicitNested, @@ -173,6 +203,7 @@ export function foldDeclaredDependencies( config, relatedConfig.listName, [...visitedLists, relatedConfig.listName], + nestedSelection, ) if (nested.include) { diff --git a/packages/core/src/access/field-visibility.ts b/packages/core/src/access/field-visibility.ts index 57752bee..8b168edb 100644 --- a/packages/core/src/access/field-visibility.ts +++ b/packages/core/src/access/field-visibility.ts @@ -6,6 +6,7 @@ import { RESOLVE_CHAIN_MAX_LENGTH } from './depth-limits.js' import { ResolveOutputCycleError } from './errors.js' import type { DeclaredOnlyTree } from './declared-dependencies.js' import { emptyDeclaredOnlyTree } from './declared-dependencies.js' +import type { FieldSelectionScope } from '../query/index.js' // NOTE: `context/index.ts` imports `filterReadableFields` from this module // (via the `access/index.ts` barrel) — this is an intentional cyclic // dependency, the same shape and for the same reason as the one documented in @@ -23,6 +24,14 @@ import { buildDbDelegate } from '../context/index.js' * virtual fields. None of this can move into phase 1: virtual fields are * computed in JavaScript and field access can depend on the fetched row. * + * A computed field — any field carrying a `resolveOutput` hook, virtual or + * not — is produced only where the read is going to return it (ADR-0027). A + * fragment `query`'s own field selection is the only thing that restricts a + * level this way; a bare or `include`-based read, and any relation reached + * purely to satisfy a `needs` declaration, still compute every field, as + * before. A field the read is not going to return does no work at all — + * neither its read-access evaluation nor its hook. + * * Phase 1 (pre-query row/relation scoping) lives in `access-filter.ts`. See * `docs/adr/0001-access-control-is-a-two-phase-read.md` and the access-control * glossary in `CONTEXT.md`. @@ -96,8 +105,11 @@ function deriveResolveOutputContext( * from the result. * * `accessItem` is the row used to evaluate field access; `hookItem` is the - * object passed to the hook as `item` (these differ for virtual fields, which - * see the already-filtered output so they can read sibling fields). + * object passed to the hook as `item`. For a stored field, both are + * `workingItem` (the row's own stored/fetched columns). For a virtual field, + * `hookItem` is `computedFieldItem` instead — the same stored columns with + * every skipped-or-denied key removed — so it never sees another computed + * field's resolved value (ADR-0027). */ async function resolveReadableFieldValue(params: { fieldConfig: FieldConfig | undefined @@ -189,6 +201,13 @@ export async function filterReadableFields>( // returned — after resolveOutput has had a chance to read them — so a // declared dependency never widens what the caller receives. declaredOnly: DeclaredOnlyTree = emptyDeclaredOnlyTree(), + // The fragment scope this level was reached under (ADR-0027), and the same + // tree one level down for each nested relation. `undefined` — the default, + // and what a bare/`include`-based read passes at every level — means + // unrestricted: every field on the list is computed, unchanged from + // before ADR-0027. Only a `query` fragment's own field selection ever + // restricts a level. + selection?: FieldSelectionScope, ): Promise> { const filtered: Record = {} @@ -214,6 +233,15 @@ export async function filterReadableFields>( workingItem[fieldName] = assembled } + // Keys denied by field-level read access during the pass below — as opposed + // to a key merely skipped by `selection` or held back only for + // `declaredOnly` stripping. Tracked separately because a denied key must + // stay invisible to a computed field's hook (below), while a declared-only + // key must stay VISIBLE to one — that is the entire point of declaring it + // (ADR-0025) — even though `selection` above skipped adding it to + // `filtered` because the caller's fragment never asked for it. + const accessDeniedKeys = new Set() + // Process existing fields from the database result for (const [fieldName, value] of Object.entries(workingItem)) { const fieldConfig = fieldConfigs[fieldName] @@ -224,6 +252,17 @@ export async function filterReadableFields>( continue } + // Projection-aware skip (ADR-0027): a fragment read that does not select + // this field does no work for it at all — no field-level read-access + // check, no resolveOutput, no recursion into a relation — because the + // read is never going to return it. `selection` is only ever restricted + // by a fragment's own field selection; a bare/`include`-based read, and a + // relation reached only to satisfy another field's `needs`, pass no + // selection at all and compute every field here, unchanged. + if (selection?.fields && !selection.fields.has(fieldName)) { + continue + } + // Handle relationship fields - recursively filter fields within related items // Note: Access control filtering is now done at database level via buildAccessScopedInclude // This only handles field-level access (hiding sensitive fields) @@ -251,6 +290,7 @@ export async function filterReadableFields>( }) if (!canRead) { + accessDeniedKeys.add(fieldName) continue } @@ -260,6 +300,12 @@ export async function filterReadableFields>( // back to an empty tree when this relation isn't declaration-related at // all — the common case. const nestedDeclaredOnly = declaredOnly.nested[fieldName] ?? emptyDeclaredOnlyTree() + // This relation's own fragment scope, if the caller's fragment named it + // with a nested Fragment/RelationSelector. `undefined` (a bare `true` + // selector, or no `selection` at all) means the nested list computes + // unrestricted — matching what naming a relation without narrowing it + // further has always meant. + const nestedSelection = selection?.nested[fieldName] if (relatedConfig) { // For many relationships (arrays) - recursively filter fields in each item @@ -275,6 +321,7 @@ export async function filterReadableFields>( depth + 1, relatedConfig.listName, nestedDeclaredOnly, + nestedSelection, ), ), ) @@ -290,6 +337,7 @@ export async function filterReadableFields>( depth + 1, relatedConfig.listName, nestedDeclaredOnly, + nestedSelection, ) } } else { @@ -314,7 +362,36 @@ export async function filterReadableFields>( if (result.readable) { filtered[fieldName] = result.value + } else { + accessDeniedKeys.add(fieldName) + } + } + + // The item a virtual field's hook sees: stored columns and fetched + // relations (from `workingItem`, never a resolved value — no hook's output + // is ever written back into `workingItem`). A key is visible here if it + // either survived into `filtered` (selected and allowed) OR exists only to + // satisfy a `needs` declaration (`declaredOnly` — that IS the point of + // declaring it: fetched for a hook, never for the caller, ADR-0025). + // Everything else — field-level denied, or skipped by `selection` and + // declared by no one — is deleted. A computed field reaches for exactly its + // own declared dependencies and nothing another field's hook produced + // (ADR-0027): reaching for a sibling that was denied or skipped-and- + // undeclared finds nothing there, the same as reaching for one never + // declared at all, and reaching for a sibling that DID survive finds its + // raw stored form, never another hook's resolved value — a virtual field + // computed earlier in declaration order is exactly as invisible as one + // computed later. + const computedFieldItem: Record = { ...workingItem } + for (const key of Object.keys(workingItem)) { + if (['id', 'createdAt', 'updatedAt'].includes(key)) continue + if (accessDeniedKeys.has(key)) { + delete computedFieldItem[key] + continue } + if (key in filtered) continue + if (declaredOnly.keys.has(key)) continue + delete computedFieldItem[key] } // Process virtual fields - compute values from other fields @@ -330,22 +407,29 @@ export async function filterReadableFields>( continue } - // Virtual fields must have a resolveOutput hook to compute their value; - // without one there is nothing to add to the result. + // Projection-aware skip (ADR-0027): same rule as the stored-field pass + // above — a fragment that does not select this virtual field does no + // work for it at all. + if (selection?.fields && !selection.fields.has(fieldName)) { + continue + } + + // A virtual field with no resolveOutput hook can never produce a value + // on ANY read — there is nothing to compute, so there is nothing to do, + // including evaluating its read access (ADR-0027 reconciles the + // access-only evaluation this branch used to preserve: a field that + // never has output has no side effect worth preserving access for). if (!(fieldConfig.hooks?.resolveOutput && listKey)) { - // Still evaluate read access to preserve any access-fn side effects. - await checkFieldAccess(fieldConfig.access, 'read', { ...args, item: workingItem }) continue } - // Check read access and compute the value via the shared helper. Virtual - // fields see the already-filtered item so they can read sibling fields. + // Check read access and compute the value via the shared helper. const result = await resolveReadableFieldValue({ fieldConfig, fieldName, value: undefined, // Virtual fields don't have a database value accessItem: workingItem, - hookItem: filtered, + hookItem: computedFieldItem, listKey, args, config, @@ -357,10 +441,11 @@ export async function filterReadableFields>( } // Strip relations that were fetched ONLY to satisfy a `needs` declaration - // (ADR-0025), now that every resolveOutput hook at this level — including - // virtual fields, which read the assembled `filtered` object above — has - // had the chance to see them. A declared dependency is private plumbing, - // not an implicit `include`: it never widens what the caller receives. + // (ADR-0025), now that every resolveOutput hook at this level has had the + // chance to see them (via `computedFieldItem`, never `filtered` itself — a + // declared dependency is read from stored columns, not from another + // field's resolved output). A declared dependency is private plumbing, not + // an implicit `include`: it never widens what the caller receives. for (const key of declaredOnly.keys) { delete filtered[key] } diff --git a/packages/core/src/context/index.ts b/packages/core/src/context/index.ts index 564c6b31..67a82b1e 100644 --- a/packages/core/src/context/index.ts +++ b/packages/core/src/context/index.ts @@ -12,8 +12,8 @@ import type { DeclaredOnlyTree } from '../access/index.js' import { ValidationError, DatabaseError } from '../hooks/index.js' import { getDbKey } from '../lib/case-utils.js' import type { PrismaClientLike } from '../access/types.js' -import { buildInclude, pickFields, isFragment } from '../query/index.js' -import type { FieldSelection } from '../query/index.js' +import { buildInclude, pickFields, isFragment, buildFieldSelectionScope } from '../query/index.js' +import type { FieldSelection, FieldSelectionScope } from '../query/index.js' import { getRelationshipOptions } from '../query/relationship-options.js' import { runWritePipeline, @@ -926,6 +926,13 @@ export function buildDbDelegate( * stays on the exact ADR-0024 path — `include: undefined`, no related * `query` access evaluated — unless folding actually added something, which * only happens when a field on this list declares `needs`. + * + * Also returns the `FieldSelectionScope` a fragment's own field selection + * produces (ADR-0027), so the caller can pass it to `filterReadableFields` + * and make computation itself projection-aware, not only the fold above. + * `undefined` for every non-fragment path: a caller `include` (sudo or not) + * and a bare read both mean "compute every field," matching what they + * already fetch. */ async function resolveReadInclude( callerInclude: Record | undefined, @@ -935,19 +942,33 @@ async function resolveReadInclude( listConfig: ListConfig, context: AccessContext & { _isSudo?: boolean }, config: OpenSaasConfig, -): Promise<{ include: Record | undefined; declaredOnly: DeclaredOnlyTree }> { +): Promise<{ + include: Record | undefined + declaredOnly: DeclaredOnlyTree + selection: FieldSelectionScope | undefined +}> { if (fragmentFields !== undefined) { const fragmentInclude = buildInclude(fragmentFields) ?? undefined - return foldDeclaredDependencies(fragmentInclude, listConfig.fields, config, listName) + const selection = buildFieldSelectionScope(fragmentFields) + const folded = foldDeclaredDependencies( + fragmentInclude, + listConfig.fields, + config, + listName, + [listName], + selection, + ) + return { ...folded, selection } } if (context._isSudo) { - return foldDeclaredDependencies(callerInclude, listConfig.fields, config, listName) + const folded = foldDeclaredDependencies(callerInclude, listConfig.fields, config, listName) + return { ...folded, selection: undefined } } const folded = foldDeclaredDependencies(callerInclude, listConfig.fields, config, listName) if (!folded.include) { - return folded + return { ...folded, selection: undefined } } const include = await buildAccessScopedInclude( @@ -957,7 +978,7 @@ async function resolveReadInclude( config, listName, ) - return { include, declaredOnly: folded.declaredOnly } + return { include, declaredOnly: folded.declaredOnly, selection: undefined } } /** @@ -1023,7 +1044,7 @@ function createFindUnique( // Resolve `include`, folding any declared dependencies (`needs`, // ADR-0025) in alongside whatever the fragment/caller/sudo/bare path // already produces — see `resolveReadInclude`'s doc comment. - let { include, declaredOnly } = await resolveReadInclude( + let { include, declaredOnly, selection } = await resolveReadInclude( args.include, fragment ? fragment._fields : undefined, listName, @@ -1035,8 +1056,10 @@ function createFindUnique( // Virtual fields have no database column. Whichever path produced // `include` (fragment, access-controlled merge, or sudo passthrough), a // virtual key must never reach Prisma — it would throw "Unknown field" - // (#628). The virtual value is still computed unconditionally below by - // `filterReadableFields`, independent of what was requested here. + // (#628). Below, `filterReadableFields` computes a virtual field's value + // exactly when `selection` says the read is going to return it (ADR-0027) + // — every one of them for a bare/`include`-based read (`selection` is + // `undefined`), only the ones a fragment named otherwise. include = stripVirtualFieldsFromInclude(include, listConfig.fields, config) // Execute query with optimized includes @@ -1065,6 +1088,7 @@ function createFindUnique( 0, listName, declaredOnly, + selection, ) // When a fragment is provided, pick only the requested fields from the result @@ -1136,7 +1160,7 @@ function createFindMany( // Resolve `include`, folding any declared dependencies (`needs`, // ADR-0025) in alongside whatever the fragment/caller/sudo/bare path // already produces — see `resolveReadInclude`'s doc comment. - let { include, declaredOnly } = await resolveReadInclude( + let { include, declaredOnly, selection } = await resolveReadInclude( args?.include, fragment ? fragment._fields : undefined, listName, @@ -1148,8 +1172,10 @@ function createFindMany( // Virtual fields have no database column. Whichever path produced // `include` (fragment, access-controlled merge, or sudo passthrough), a // virtual key must never reach Prisma — it would throw "Unknown field" - // (#628). The virtual value is still computed unconditionally below by - // `filterReadableFields`, independent of what was requested here. + // (#628). Below, `filterReadableFields` computes a virtual field's value + // exactly when `selection` says the read is going to return it (ADR-0027) + // — every one of them for a bare/`include`-based read (`selection` is + // `undefined`), only the ones a fragment named otherwise. include = stripVirtualFieldsFromInclude(include, listConfig.fields, config) // Execute query with optimized includes @@ -1179,6 +1205,7 @@ function createFindMany( 0, listName, declaredOnly, + selection, ), ), ) @@ -1452,7 +1479,7 @@ function createGet( // Resolve `include`, folding any declared dependencies (`needs`, // ADR-0025) in alongside whatever the fragment/caller/sudo/bare path // already produces — see `resolveReadInclude`'s doc comment. - let { include, declaredOnly } = await resolveReadInclude( + let { include, declaredOnly, selection } = await resolveReadInclude( args?.include, fragment ? fragment._fields : undefined, listName, @@ -1484,6 +1511,7 @@ function createGet( 0, listName, declaredOnly, + selection, ) // When a fragment is provided, pick only the requested fields from the result if (fragment) { diff --git a/packages/core/src/query/index.ts b/packages/core/src/query/index.ts index ad86f57d..dee27f07 100644 --- a/packages/core/src/query/index.ts +++ b/packages/core/src/query/index.ts @@ -345,6 +345,59 @@ export function buildInclude(fields: FieldSelection): Record | undefined + readonly nested: Readonly> +} + +/** + * Build the `FieldSelectionScope` for one fragment's field selection, + * recursing into nested Fragment/RelationSelector entries the same way + * `buildInclude` does. A relation named with the bare `true` shorthand (no + * narrower nested Fragment) gets no entry in `nested`, so a level reached + * through it is treated as unrestricted — the caller asked for "everything" + * there and gave no narrower shape to restrict it with. + * @internal + */ +export function buildFieldSelectionScope(fields: FieldSelection): FieldSelectionScope { + const fieldNames = new Set(Object.keys(fields as Record)) + const nested: Record = {} + + for (const [key, value] of Object.entries(fields as Record)) { + if (value === null || value === true || typeof value !== 'object') continue + const val = value as Record + + if (isFragment(val)) { + nested[key] = buildFieldSelectionScope(val._fields as FieldSelection) + continue + } + + if ('query' in val && isFragment(val.query)) { + nested[key] = buildFieldSelectionScope( + (val.query as Fragment>) + ._fields as FieldSelection, + ) + } + } + + return { fields: fieldNames, nested } +} + /** * Recursively pick only the fields requested by a fragment from a raw Prisma * result object. This ensures the runtime shape exactly matches the type diff --git a/packages/core/tests/computed-field-selective-evaluation.test.ts b/packages/core/tests/computed-field-selective-evaluation.test.ts new file mode 100644 index 00000000..7070c95f --- /dev/null +++ b/packages/core/tests/computed-field-selective-evaluation.test.ts @@ -0,0 +1,418 @@ +import { describe, it, expect, vi } from 'vitest' +import { getContext } from '../src/context/index.js' +import { config, list } from '../src/config/index.js' +import { text, integer, relationship, virtual } from '../src/fields/index.js' +import { defineFragment } from '../src/query/index.js' +import type { FieldConfig } from '../src/config/types.js' + +/** + * Coverage for issue #855 / ADR-0027: a computed field — any field carrying a + * `resolveOutput` hook, virtual or not — is computed if and only if the read + * is going to return it, its declared relations (`needs`, ADR-0025) are + * fetched under exactly that same condition, and no computed field ever sees + * another computed field's resolved output. + * + * `tests/needs-declared-dependencies.test.ts` covers ADR-0025 itself (fetch + * folding, access scoping of a declared relation); this file covers the + * selectivity ADR-0027 adds on top of it. + */ + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +function createMockPrisma(): any { + const model = () => ({ + findFirst: vi.fn(), + findMany: vi.fn(), + }) + return { order: model(), lineItem: model(), product: model() } +} + +function buildTestConfig(spies: { + totalHook: ReturnType + doubleTotalHook: ReturnType + secretAccess: ReturnType + secretHook: ReturnType + unreachableAccess: ReturnType + unreachableHook: ReturnType + summaryHook: ReturnType + hooklessAccess: ReturnType + peekerHook: ReturnType +}) { + return config({ + db: { provider: 'postgresql', url: 'postgresql://localhost:5432/test' }, + lists: { + Product: list({ + fields: { name: text() }, + access: { operation: { query: () => true } }, + }), + LineItem: list({ + fields: { + price: integer(), + order: relationship({ ref: 'Order.lineItems' }), + product: relationship({ ref: 'Product' }), + summary: virtual({ + type: 'string', + needs: ['product'], + hooks: { + resolveOutput: (hookArgs: unknown) => { + spies.summaryHook(hookArgs) + const typedItem = (hookArgs as { item: { product?: { name?: string } | null } }) + .item + return typedItem.product ? `${typedItem.product.name} x1` : 'unknown product x1' + }, + }, + }), + }, + access: { operation: { query: () => true } }, + }), + Order: list({ + fields: { + title: text(), + lineItems: relationship({ ref: 'LineItem.order', many: true }), + total: virtual({ + type: 'number', + needs: ['lineItems'], + hooks: { + resolveOutput: (hookArgs: unknown) => { + spies.totalHook(hookArgs) + const typedItem = (hookArgs as { item: { lineItems?: Array<{ price?: number }> } }) + .item + return (typedItem.lineItems ?? []).reduce((sum, li) => sum + (li.price ?? 0), 0) + }, + }, + }), + // Declares the SAME relation as `total` — exercises "fetched once, + // even when only one of the two declaring fields is selected." + doubleTotal: virtual({ + type: 'number', + needs: ['lineItems'], + hooks: { + resolveOutput: (hookArgs: unknown) => { + spies.doubleTotalHook(hookArgs) + const typedItem = (hookArgs as { item: { lineItems?: Array<{ price?: number }> } }) + .item + return (typedItem.lineItems ?? []).reduce((sum, li) => sum + (li.price ?? 0), 0) * 2 + }, + }, + }), + // A stored field with its own resolveOutput (a "computed field" per + // ADR-0027, not only virtual ones) whose access AND hook are spied + // on so a fragment that never selects it can be asserted to have + // invoked neither. + secret: text({ + access: { + read: spies.secretAccess, + }, + hooks: { + resolveOutput: (hookArgs: unknown) => { + spies.secretHook(hookArgs) + return `wrapped:${(hookArgs as { value: unknown }).value}` + }, + }, + }), + // A virtual field never selected by any fragment in these tests — + // stands in for "a field the read is never going to return." + unreachable: virtual({ + type: 'string', + access: { + read: (accessArgs: unknown) => { + spies.unreachableAccess(accessArgs) + return true + }, + }, + hooks: { + resolveOutput: (hookArgs: unknown) => { + spies.unreachableHook(hookArgs) + return 'unreachable-value' + }, + }, + }), + // Reads a SIBLING computed field (`secret`) without declaring it — + // must see `undefined`, never `secret`'s resolved ("wrapped:...") + // value, and never its raw stored value either when `secret` is + // skipped by a fragment's own selection. + peeker: virtual({ + type: 'string', + hooks: { + resolveOutput: (hookArgs: unknown) => { + spies.peekerHook(hookArgs) + const value = (hookArgs as { item: Record }).item.secret + return value === undefined ? 'saw-nothing' : `saw:${String(value)}` + }, + }, + }), + // A hookless virtual field — can never produce a value on ANY + // read. Constructed as a raw FieldConfig (bypassing the `virtual()` + // builder, which throws without a `resolveOutput`) the way a + // third-party field package might legitimately shape one. + hooklessVirtual: { + type: 'text', + virtual: true, + access: { + read: (accessArgs: unknown) => { + spies.hooklessAccess(accessArgs) + return true + }, + }, + hooks: {}, + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- minimal raw field config for a test double + } as any as FieldConfig, + }, + access: { operation: { query: () => true } }, + }), + }, + }) +} + +function makeSpies() { + return { + totalHook: vi.fn(), + doubleTotalHook: vi.fn(), + secretAccess: vi.fn(() => true), + secretHook: vi.fn(), + unreachableAccess: vi.fn(), + unreachableHook: vi.fn(), + summaryHook: vi.fn(), + hooklessAccess: vi.fn(), + peekerHook: vi.fn(), + } +} + +describe('a computed field runs only when it is going to be returned (#855, ADR-0027)', () => { + it('a fragment that does not select a computed field runs neither its read access nor its hook, and does not fold its needs into the include', async () => { + const spies = makeSpies() + const testConfig = await buildTestConfig(spies) + const mockPrisma = createMockPrisma() + mockPrisma.order.findFirst.mockResolvedValue({ id: 'o1', title: 'Order 1' }) + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const fragment = defineFragment()({ title: true } as const) + const context = getContext(testConfig, mockPrisma, null) + const result = await context.db.order.findUnique({ where: { id: 'o1' }, query: fragment }) + + // None of the unselected computed fields' declared relations were folded + // into the include — `lineItems` is only ever needed by `total`/ + // `doubleTotal`, neither of which was selected, so there is nothing to + // fold and `include` stays exactly `undefined` (the bare-read shape). + const callArgs = mockPrisma.order.findFirst.mock.calls[0][0] + expect(callArgs.include).toBeUndefined() + + expect(spies.totalHook).not.toHaveBeenCalled() + expect(spies.doubleTotalHook).not.toHaveBeenCalled() + expect(spies.secretAccess).not.toHaveBeenCalled() + expect(spies.secretHook).not.toHaveBeenCalled() + expect(spies.unreachableAccess).not.toHaveBeenCalled() + expect(spies.unreachableHook).not.toHaveBeenCalled() + + expect(result).toEqual({ title: 'Order 1' }) + }) + + it('a fragment that DOES select a computed field computes it correctly and fetches its declared relation, unchanged', async () => { + const spies = makeSpies() + const testConfig = await buildTestConfig(spies) + const mockPrisma = createMockPrisma() + mockPrisma.order.findFirst.mockResolvedValue({ + id: 'o1', + title: 'Order 1', + lineItems: [{ id: 'li1', price: 10, orderId: 'o1' }], + }) + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const fragment = defineFragment()({ title: true, total: true } as const) + const context = getContext(testConfig, mockPrisma, null) + const result = await context.db.order.findUnique({ where: { id: 'o1' }, query: fragment }) + + const callArgs = mockPrisma.order.findFirst.mock.calls[0][0] + expect(callArgs.include).toMatchObject({ lineItems: expect.anything() }) + expect(spies.totalHook).toHaveBeenCalledTimes(1) + expect(result?.total).toBe(10) + expect(result).not.toHaveProperty('lineItems') + }) + + it('two fields declaring the same relation, only one selected, still fetch that relation exactly once — and only the selected field computes', async () => { + const spies = makeSpies() + const testConfig = await buildTestConfig(spies) + const mockPrisma = createMockPrisma() + mockPrisma.order.findFirst.mockResolvedValue({ + id: 'o1', + title: 'Order 1', + lineItems: [{ id: 'li1', price: 10, orderId: 'o1' }], + }) + + // `doubleTotal` also needs `lineItems` but is NOT selected. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const fragment = defineFragment()({ title: true, total: true } as const) + const context = getContext(testConfig, mockPrisma, null) + const result = await context.db.order.findUnique({ where: { id: 'o1' }, query: fragment }) + + expect(mockPrisma.order.findFirst).toHaveBeenCalledTimes(1) + const callArgs = mockPrisma.order.findFirst.mock.calls[0][0] + // Exactly one `lineItems` entry in the include — folded once for `total`. + expect(Object.keys(callArgs.include)).toEqual(['lineItems']) + + expect(result?.total).toBe(10) + expect(spies.totalHook).toHaveBeenCalledTimes(1) + expect(spies.doubleTotalHook).not.toHaveBeenCalled() + }) + + it('nested level: a nested fragment selecting a subset computes only that subset', async () => { + const spies = makeSpies() + const testConfig = await buildTestConfig(spies) + const mockPrisma = createMockPrisma() + mockPrisma.order.findFirst.mockResolvedValue({ + id: 'o1', + title: 'Order 1', + lineItems: [{ id: 'li1', price: 10, orderId: 'o1' }], + }) + + const lineItemFragment = defineFragment<{ price: number }>()({ price: true } as const) + const orderFragment = defineFragment<{ title: string; lineItems: unknown[] }>()({ + title: true, + lineItems: lineItemFragment, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any) + + const context = getContext(testConfig, mockPrisma, null) + const result = await context.db.order.findUnique({ + where: { id: 'o1' }, + query: orderFragment, + }) + + // `summary` (needs `product`) was not selected inside the nested + // fragment, so neither its hook ran nor was `product` folded in beneath + // `lineItems`. + expect(spies.summaryHook).not.toHaveBeenCalled() + const callArgs = mockPrisma.order.findFirst.mock.calls[0][0] + expect(callArgs.include.lineItems).not.toMatchObject({ + include: expect.objectContaining({ product: expect.anything() }), + }) + expect(result?.lineItems?.[0]).toEqual({ price: 10 }) + }) + + it('nested level: an include (not a fragment) still computes every computed field, unchanged', async () => { + const spies = makeSpies() + const testConfig = await buildTestConfig(spies) + const mockPrisma = createMockPrisma() + mockPrisma.order.findFirst.mockResolvedValue({ + id: 'o1', + title: 'Order 1', + lineItems: [{ id: 'li1', price: 10, orderId: 'o1', product: { id: 'p1', name: 'Widget' } }], + }) + + const context = getContext(testConfig, mockPrisma, null) + const result = await context.db.order.findUnique({ + where: { id: 'o1' }, + include: { lineItems: { include: { product: true } } }, + }) + + expect(spies.summaryHook).toHaveBeenCalledTimes(1) + expect(result?.lineItems?.[0].summary).toBe('Widget x1') + }) + + it("a hook's item never carries another computed field's resolved output, even on a bare read where both survive", async () => { + const spies = makeSpies() + const testConfig = await buildTestConfig(spies) + const mockPrisma = createMockPrisma() + mockPrisma.order.findFirst.mockResolvedValue({ id: 'o1', title: 'Order 1', secret: 'hunter2' }) + + const context = getContext(testConfig, mockPrisma, null) + // Bare read: every computed field computes, including `secret` (a stored + // field with its own resolveOutput) and `peeker` (a virtual field with no + // `needs` at all, reading `item.secret` without declaring it). `secret` + // is a plain stored scalar column, always fetched on any read (ADR-0024) + // regardless of declarations — only RELATIONS are conditionally fetched. + const result = await context.db.order.findUnique({ where: { id: 'o1' } }) + + // `secret`'s OWN resolveOutput wraps its stored value for the caller... + expect(result?.secret).toBe('wrapped:hunter2') + // ...but `peeker`, reading the same key from its own hook's `item`, sees + // the raw STORED column ('hunter2'), never `secret`'s resolved output + // ('wrapped:hunter2') — the "no computed field sees another's computed + // value" rule, proven by the two hooks disagreeing about the same key. + expect(result?.peeker).toBe('saw:hunter2') + }) + + it("a skipped field's key is absent from a sibling hook's item, never present holding its raw stored value", async () => { + const spies = makeSpies() + const testConfig = await buildTestConfig(spies) + const mockPrisma = createMockPrisma() + mockPrisma.order.findFirst.mockResolvedValue({ id: 'o1', title: 'Order 1', secret: 'hunter2' }) + + // `secret` is NOT selected; `peeker` is, and reads `item.secret`. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const fragment = defineFragment()({ title: true, peeker: true } as const) + const context = getContext(testConfig, mockPrisma, null) + const result = await context.db.order.findUnique({ where: { id: 'o1' }, query: fragment }) + + // `secret`'s own access/hook never ran (it was never going to be returned). + expect(spies.secretAccess).not.toHaveBeenCalled() + expect(spies.secretHook).not.toHaveBeenCalled() + // `peeker` sees the key absent, never the raw pre-hook stored value. + expect(result?.peeker).toBe('saw-nothing') + expect(result).not.toHaveProperty('secret') + }) + + it('field-level read access still gates a field that IS selected: denied means absent and its hook does not run', async () => { + const spies = makeSpies() + spies.secretAccess.mockImplementation(() => false) + const testConfigDenied = await buildTestConfig(spies) + const mockPrisma = createMockPrisma() + mockPrisma.order.findFirst.mockResolvedValue({ id: 'o1', title: 'Order 1', secret: 'hunter2' }) + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const fragment = defineFragment()({ title: true, secret: true } as const) + const context = getContext(testConfigDenied, mockPrisma, null) + const result = await context.db.order.findUnique({ where: { id: 'o1' }, query: fragment }) + + expect(spies.secretAccess).toHaveBeenCalled() + expect(spies.secretHook).not.toHaveBeenCalled() + expect(result?.secret).toBeUndefined() + }) + + it('a hookless virtual field does no work at all — its read access is never invoked on any read', async () => { + const spies = makeSpies() + const testConfig = await buildTestConfig(spies) + const mockPrisma = createMockPrisma() + mockPrisma.order.findFirst.mockResolvedValue({ id: 'o1', title: 'Order 1' }) + mockPrisma.order.findMany.mockResolvedValue([{ id: 'o1', title: 'Order 1' }]) + + const context = getContext(testConfig, mockPrisma, null) + + // Bare read. + await context.db.order.findUnique({ where: { id: 'o1' } }) + expect(spies.hooklessAccess).not.toHaveBeenCalled() + + // include-based read naming it explicitly. + await context.db.order.findUnique({ where: { id: 'o1' }, include: { hooklessVirtual: true } }) + expect(spies.hooklessAccess).not.toHaveBeenCalled() + + // fragment selecting it explicitly. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const fragment = defineFragment()({ title: true, hooklessVirtual: true } as const) + await context.db.order.findUnique({ where: { id: 'o1' }, query: fragment }) + expect(spies.hooklessAccess).not.toHaveBeenCalled() + }) + + it('include/bare reads still return every computed field on the list, unchanged', async () => { + const spies = makeSpies() + const testConfig = await buildTestConfig(spies) + const mockPrisma = createMockPrisma() + mockPrisma.order.findMany.mockResolvedValue([ + { + id: 'o1', + title: 'Order 1', + secret: 'hunter2', + lineItems: [{ id: 'li1', price: 10, orderId: 'o1' }], + }, + ]) + + const context = getContext(testConfig, mockPrisma, null) + const result = await context.db.order.findMany({}) + + expect(result[0].total).toBe(10) + expect(result[0].doubleTotal).toBe(20) + expect(result[0].secret).toBe('wrapped:hunter2') + expect(spies.totalHook).toHaveBeenCalledTimes(1) + expect(spies.doubleTotalHook).toHaveBeenCalledTimes(1) + expect(spies.secretHook).toHaveBeenCalledTimes(1) + }) +})