Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Grafast: introduce inhibitOnNull #2015

Merged
merged 20 commits into from
Apr 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/curly-schools-dance.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"graphile-build-pg": patch
"postgraphile": patch
---

Use `inhibitOnNull` when decoding a spec ID to prevent it being consumed if
invalid (e.g. don't allow using a 'Post' node ID to fetch a 'Person' record).
5 changes: 5 additions & 0 deletions .changeset/great-cobras-play.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"grafast": patch
---

Introduce new `inhibitOnNull`, `assertNotNull` and `trap` steps.
19 changes: 19 additions & 0 deletions grafast/grafast/src/engine/StepTracker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { inspect } from "../inspect.js";
import type { AddStepDependencyOptions } from "../interfaces.js";
import { $$subroutine, ALL_FLAGS, TRAPPABLE_FLAGS } from "../interfaces.js";
import { ExecutableStep } from "../step.js";
import { __FlagStep } from "../steps/__flag.js";
import { sudo } from "../utils.js";
import type {
LayerPlan,
Expand Down Expand Up @@ -282,6 +283,19 @@ export class StepTracker {
): number {
const $dependent = sudo(raw$dependent);
const $dependency = sudo(raw$dependency);
if ($dependency instanceof __FlagStep) {
// See if we can inline this
const inlineDetails = $dependency.inline(options);
if (inlineDetails !== null) {
// We can inline it: tweak flags and try again
const { $source, acceptFlags, onReject } = inlineDetails;
return this.addStepDependency($dependent, $source, {
...options,
acceptFlags,
onReject,
});
}
}
if (!this.activeSteps.has($dependent)) {
throw new Error(
`Cannot add ${$dependency} as a dependency of ${$dependent}; the latter is deleted!`,
Expand Down Expand Up @@ -319,9 +333,13 @@ export class StepTracker {
const dependentDependencyForbiddenFlags = writeableArray(
$dependent.dependencyForbiddenFlags,
);
const dependentDependencyOnReject = writeableArray(
$dependent.dependencyOnReject,
);
const {
skipDeduplication,
acceptFlags = ALL_FLAGS & ~$dependent.defaultForbiddenFlags,
onReject,
} = options;
// When copying dependencies between classes, we might not want to
// deduplicate because we might refer to the dependency by its index. As
Expand All @@ -348,6 +366,7 @@ export class StepTracker {
this.stepsWithNoDependencies.delete($dependent);
const dependencyIndex = dependentDependencies.push($dependency) - 1;
dependentDependencyForbiddenFlags[dependencyIndex] = forbiddenFlags;
dependentDependencyOnReject[dependencyIndex] = onReject;
writeableArray($dependency.dependents).push({
step: $dependent,
dependencyIndex,
Expand Down
45 changes: 36 additions & 9 deletions grafast/grafast/src/engine/executeBucket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,10 @@ import {
$$streamMore,
$$timeout,
FLAG_ERROR,
FLAG_INHIBITED,
FLAG_NULL,
FLAG_POLY_SKIPPED,
FLAG_STOPPED,
NO_FLAGS,
} from "../interfaces.js";
import type { ExecutableStep, UnbatchedExecutableStep } from "../step.js";
Expand Down Expand Up @@ -592,15 +594,18 @@ export function executeBucket(
for (const $dep of step.dependencies) {
const depExecutionVal = bucket.store.get($dep.id)!;
const depVal = depExecutionVal.at(dataIndex);
let depFlags;
// if (bucket.hasNonZeroStatus && isGrafastError(depVal)) {
if (
(bucket.flagUnion & FLAG_ERROR) === FLAG_ERROR &&
(depExecutionVal._flagsAt(dataIndex) & FLAG_ERROR) ===
((depFlags = depExecutionVal._flagsAt(dataIndex)) &
FLAG_ERROR) ===
FLAG_ERROR
) {
if (step._isUnary) {
// COPY the unary value
bucket.store.set(step.id, depExecutionVal);
bucket.flagUnion |= depFlags;
} else {
bucket.setResult(step, dataIndex, depVal, FLAG_ERROR);
}
Expand All @@ -622,7 +627,6 @@ export function executeBucket(
stepResult == null ? FLAG_NULL : NO_FLAGS,
);
} catch (e) {
console.dir(e);
const error = newGrafastError(e, step.id);
bucket.setResult(step, dataIndex, error, FLAG_ERROR);
}
Expand Down Expand Up @@ -726,6 +730,7 @@ export function executeBucket(
step: ExecutableStep,
dependenciesIncludingSideEffects: ReadonlyArray<ExecutionValue>,
dependencyForbiddenFlags: ReadonlyArray<ExecutionEntryFlags>,
dependencyOnReject: ReadonlyArray<GrafastError | null | undefined>,
polymorphicPathList: readonly (string | null)[],
extra: ExecutionExtra,
): PromiseOrDirect<GrafastInternalResultsOrStream<any>> {
Expand Down Expand Up @@ -754,6 +759,7 @@ export function executeBucket(
// for (let index = 0, l = polymorphicPathList.length; index < l; index++) {
for (let index = 0; index < expectedSize; index++) {
let forceIndexValue: GrafastError | null | undefined = undefined;
let rejectValue: GrafastError | null | undefined = undefined;
let indexFlags: ExecutionEntryFlags = NO_FLAGS;
if (
stepPolymorphicPaths !== null &&
Expand All @@ -769,10 +775,20 @@ export function executeBucket(
) {
const dep = dependenciesIncludingSideEffects[i];
const forbiddenFlags = dependencyForbiddenFlags[i];
const onReject = dependencyOnReject[i];
const flags = dep._flagsAt(index);
const disallowedFlags = flags & forbiddenFlags;
if (disallowedFlags !== NO_FLAGS) {
indexFlags |= disallowedFlags;
// If there's a reject behavior and we're FRESHLY rejected (weren't
// already inhibited), use that as a fallback.
// TODO: validate this.
// If dep is inhibited and we don't allow inhibited, copy through (null or error).
// If dep is inhibited and we do allow inhibited, but we're disallowed, use our onReject.
// If dep is not inhibited, but we're disallowed, use our onReject.
if (onReject && (disallowedFlags & FLAG_INHIBITED) === NO_FLAGS) {
rejectValue ||= onReject;
}
if (!forceIndexValue) {
if (flags & FLAG_ERROR) {
const v = dep.at(index);
Expand Down Expand Up @@ -803,6 +819,11 @@ export function executeBucket(
: ev,
);
}
indexFlags |= FLAG_INHIBITED;
if (forceIndexValue == null && rejectValue != null) {
indexFlags |= FLAG_ERROR;
forceIndexValue = rejectValue;
}
forcedValues[0][index] = indexFlags;
forcedValues[1][index] = forceIndexValue;
} else {
Expand Down Expand Up @@ -881,16 +902,19 @@ export function executeBucket(
/** Only mutate this inside `addDependency` */
const _rawDependencies: Array<ExecutionValue> = [];
const _rawForbiddenFlags: Array<ExecutionEntryFlags> = [];
const _rawOnReject: Array<GrafastError | null | undefined> = [];
const dependencies: ReadonlyArray<ExecutionValue> = _rawDependencies;
let needsFiltering = false;
const defaultForbiddenFlags = sudo(step).defaultForbiddenFlags;
const addDependency = (
step: ExecutableStep,
forbiddenFlags: ExecutionEntryFlags,
onReject: GrafastError | null | undefined,
) => {
const executionValue = store.get(step.id)!;
_rawDependencies.push(executionValue);
_rawForbiddenFlags.push(forbiddenFlags);
_rawOnReject.push(onReject);
if (!needsFiltering && (bucket.flagUnion & forbiddenFlags) !== 0) {
const flags = executionValue._getStateUnion();
needsFiltering = (flags & forbiddenFlags) !== 0;
Expand All @@ -901,24 +925,26 @@ export function executeBucket(
if (depCount > 0 || sideEffectStepsWithErrors !== null) {
for (let i = 0, l = depCount; i < l; i++) {
const $dep = sstep.dependencies[i];
addDependency($dep, sstep.dependencyForbiddenFlags[i]);
// const executionValue = store.get($dep.id)!;
// dependencies[i] = executionValue;
addDependency(
$dep,
sstep.dependencyForbiddenFlags[i],
sstep.dependencyOnReject[i],
);
}
if (sideEffectStepsWithErrors !== null) {
if (sstep.polymorphicPaths) {
for (const path of sstep.polymorphicPaths) {
for (const sideEffectStep of sideEffectStepsWithErrors[path]) {
// TODO: revisit this, feels like we might be adding the same
// effect multiple times if it matches multiple paths.
addDependency(sideEffectStep, defaultForbiddenFlags);
addDependency(sideEffectStep, defaultForbiddenFlags, undefined);
}
}
} else {
for (const sideEffectStep of sideEffectStepsWithErrors[
NO_POLY_PATH
]) {
addDependency(sideEffectStep, defaultForbiddenFlags);
addDependency(sideEffectStep, defaultForbiddenFlags, undefined);
}
}
}
Expand All @@ -944,6 +970,7 @@ export function executeBucket(
step,
dependencies,
_rawForbiddenFlags,
_rawOnReject,
bucket.polymorphicPathList,
extra,
)
Expand All @@ -959,7 +986,7 @@ export function executeBucket(
// ExecutionValue is created:
// bucket.hasNonZeroStatus = true;
return [
arrayOfLength(size, FLAG_ERROR /* TODO: don't lose other flags */),
arrayOfLength(size, FLAG_ERROR | FLAG_STOPPED),
arrayOfLength(size, error),
];
});
Expand All @@ -971,7 +998,7 @@ export function executeBucket(
// ExecutionValue is created:
// bucket.hasNonZeroStatus = true;
return [
arrayOfLength(size, FLAG_ERROR /* TODO: don't lose other flags */),
arrayOfLength(size, FLAG_ERROR | FLAG_STOPPED),
arrayOfLength(size, error),
];
}
Expand Down
9 changes: 9 additions & 0 deletions grafast/grafast/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ import {
applyTransforms,
ApplyTransformsStep,
assertEdgeCapableStep,
assertNotNull,
assertPageInfoCapableStep,
connection,
ConnectionCapableStep,
Expand All @@ -161,6 +162,7 @@ import {
GraphQLResolverStep,
groupBy,
GroupByPlanMemo,
inhibitOnNull,
lambda,
LambdaStep,
last,
Expand Down Expand Up @@ -211,6 +213,7 @@ import {
specFromNodeId,
trackedContext,
trackedRootValue,
trap,
} from "./steps/index.js";
import { stringifyPayload } from "./stringifyPayload.js";
import { stripAnsi } from "./stripAnsi.js";
Expand Down Expand Up @@ -266,6 +269,7 @@ export {
assertExecutableStep,
assertListCapableStep,
assertModifierStep,
assertNotNull,
assertPageInfoCapableStep,
BaseEventMap,
BaseGraphQLArguments,
Expand Down Expand Up @@ -332,6 +336,7 @@ export {
GraphQLResolverStep,
groupBy,
GroupByPlanMemo,
inhibitOnNull,
InputObjectFieldApplyPlanResolver,
InputObjectFieldInputPlanResolver,
inputObjectFieldSpec,
Expand Down Expand Up @@ -439,6 +444,7 @@ export {
subscribe,
trackedContext,
trackedRootValue,
trap,
TypedEventEmitter,
UnbatchedExecutableStep,
UnbatchedExecutionExtra,
Expand Down Expand Up @@ -486,6 +492,9 @@ exportAsMany("grafast", {
rootValue,
trackedContext,
trackedRootValue,
inhibitOnNull,
assertNotNull,
trap,
isGrafastError,
debugPlans,
each,
Expand Down
14 changes: 9 additions & 5 deletions grafast/grafast/src/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -798,20 +798,22 @@ export interface UnbatchedExecutionExtra extends ExecutionExtraBase {}
* - 2: null (trappable)
* - 4: inhibited (trappable)
* - 8: disabled due to polymorphism (untrappable)
* - 16: ...
* - 16: stopped (untrappable) - e.g. due to fatal (unrecoverable) error
* - 32: ...
*
* @internal
*/
export type ExecutionEntryFlags = number & { readonly tsBrand?: unique symbol };

export const NO_FLAGS: ExecutionEntryFlags = 0;
export const FLAG_ERROR: ExecutionEntryFlags = 1 << 0;
export const FLAG_NULL: ExecutionEntryFlags = 1 << 1;
export const FLAG_INHIBITED: ExecutionEntryFlags = 1 << 2;
export const FLAG_POLY_SKIPPED: ExecutionEntryFlags = 1 << 3;

export const NO_FLAGS: ExecutionEntryFlags = 0;
export const FLAG_STOPPED: ExecutionEntryFlags = 1 << 4;
export const ALL_FLAGS: ExecutionEntryFlags =
FLAG_ERROR | FLAG_NULL | FLAG_INHIBITED | FLAG_POLY_SKIPPED;
FLAG_ERROR | FLAG_NULL | FLAG_INHIBITED | FLAG_POLY_SKIPPED | FLAG_STOPPED;

/** By default, accept null values as an input */
export const DEFAULT_ACCEPT_FLAGS: ExecutionEntryFlags = FLAG_NULL;
export const TRAPPABLE_FLAGS: ExecutionEntryFlags =
Expand All @@ -820,7 +822,7 @@ export const DEFAULT_FORBIDDEN_FLAGS: ExecutionEntryFlags =
ALL_FLAGS & ~DEFAULT_ACCEPT_FLAGS;
export const FORBIDDEN_BY_NULLABLE_BOUNDARY_FLAGS: ExecutionEntryFlags =
FLAG_NULL | FLAG_POLY_SKIPPED;
// TODO: make `FORBIDDEN_BY_NULLABLE_BOUNDARY_FLAGS = FLAG_ERROR | FLAG_NULL | FLAG_POLY_SKIPPED | FLAG_INHIBITED;`
// TODO: make `FORBIDDEN_BY_NULLABLE_BOUNDARY_FLAGS = FLAG_ERROR | FLAG_NULL | FLAG_POLY_SKIPPED | FLAG_INHIBITED | FLAG_STOPPED;`
// Currently this isn't enabled because the bucket has to exist for the output
// plan to throw the error; really the root should be evaluated before
// descending into the output plan rather than as part of descending?
Expand All @@ -838,6 +840,7 @@ interface ExecutionValueBase<TData = any> {
_getStateUnion(): ExecutionEntryFlags;
/** @internal */
_setResult(i: number, value: TData, flags: ExecutionEntryFlags): void;
/** @internal */
_copyResult(
targetIndex: number,
source: ExecutionValue,
Expand Down Expand Up @@ -920,4 +923,5 @@ export interface AddStepDependencyOptions {
skipDeduplication?: boolean;
/** @defaultValue `FLAG_NULL` */
acceptFlags?: ExecutionEntryFlags;
onReject?: null | GrafastError | undefined;
}
5 changes: 5 additions & 0 deletions grafast/grafast/src/step.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
} from "./engine/lib/withGlobalLayerPlan.js";
import { $$unlock } from "./engine/lock.js";
import type { OperationPlan } from "./engine/OperationPlan.js";
import type { GrafastError } from "./error.js";
import { getDebug } from "./global.js";
import { inspect } from "./inspect.js";
import type {
Expand Down Expand Up @@ -233,6 +234,9 @@ export /* abstract */ class ExecutableStep<TData = any> extends BaseStep {
* (default = this.defaultForbiddenFlags)
*/
protected readonly dependencyForbiddenFlags: ReadonlyArray<ExecutionEntryFlags>;
protected readonly dependencyOnReject: ReadonlyArray<
GrafastError | null | undefined
>;

/**
* Just for mermaid
Expand Down Expand Up @@ -294,6 +298,7 @@ export /* abstract */ class ExecutableStep<TData = any> extends BaseStep {
super();
this.dependencies = [];
this.dependencyForbiddenFlags = [];
this.dependencyOnReject = [];
this.dependents = [];
this.isOptimized = false;
this.allowMultipleOptimizations = false;
Expand Down