Skip to content

Commit

Permalink
RFC: Default value validation & coercion
Browse files Browse the repository at this point in the history 
Implements graphql/graphql-spec#793

* Adds validation of default values during schema validation.
* Adds coercion of default values anywhere a default value is used at runtime
  • Loading branch information
@Code-Hex @leebyron
Code-Hex authored and leebyron committed Apr 25, 2021
1 parent 2d48fbb commit 153e8b9
Show file tree
Hide file tree
Showing 9 changed files with 295 additions and 57 deletions.
1 change: 0 additions & 1 deletion src/execution/__tests__/variables-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ const TestType = new GraphQLObjectType({
}),
fieldWithNestedInputObject: fieldWithInputArg({
type: TestNestedInputObject,
defaultValue: 'Hello World',
}),
list: fieldWithInputArg({ type: new GraphQLList(GraphQLString) }),
nnList: fieldWithInputArg({
Expand Down
9 changes: 6 additions & 3 deletions src/execution/values.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@ import { isInputType, isNonNullType } from '../type/definition';

import { typeFromAST } from '../utilities/typeFromAST';
import { valueFromAST } from '../utilities/valueFromAST';
import { coerceInputValue } from '../utilities/coerceInputValue';
import {
coerceInputValue,
coerceDefaultValue,
} from '../utilities/coerceInputValue';

type CoercedVariableValues =
| {| errors: $ReadOnlyArray<GraphQLError> |}
Expand Down Expand Up @@ -174,7 +177,7 @@ export function getArgumentValues(

if (!argumentNode) {
if (argDef.defaultValue !== undefined) {
coercedValues[name] = argDef.defaultValue;
coercedValues[name] = coerceDefaultValue(argDef);
} else if (isNonNullType(argType)) {
throw new GraphQLError(
`Argument "${name}" of required type "${inspect(argType)}" ` +
Expand All @@ -195,7 +198,7 @@ export function getArgumentValues(
!hasOwnProperty(variableValues, variableName)
) {
if (argDef.defaultValue !== undefined) {
coercedValues[name] = argDef.defaultValue;
coercedValues[name] = coerceDefaultValue(argDef);
} else if (isNonNullType(argType)) {
throw new GraphQLError(
`Argument "${name}" of required type "${inspect(argType)}" ` +
Expand Down
26 changes: 15 additions & 11 deletions src/type/definition.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -809,21 +809,11 @@ function defineFieldMap<TSource, TContext>(
`${config.name}.${fieldName} args must be an object with argument names as keys.`,
);

const args = Object.entries(argsConfig).map(([argName, argConfig]) => ({
name: argName,
description: argConfig.description,
type: argConfig.type,
defaultValue: argConfig.defaultValue,
deprecationReason: argConfig.deprecationReason,
extensions: argConfig.extensions && toObjMap(argConfig.extensions),
astNode: argConfig.astNode,
}));

return {
name: fieldName,
description: fieldConfig.description,
type: fieldConfig.type,
args,
args: defineArguments(argsConfig),
resolve: fieldConfig.resolve,
subscribe: fieldConfig.subscribe,
deprecationReason: fieldConfig.deprecationReason,
Expand All @@ -833,6 +823,20 @@ function defineFieldMap<TSource, TContext>(
});
}

export function defineArguments(
config: GraphQLFieldConfigArgumentMap,
): $ReadOnlyArray<GraphQLArgument> {
return Object.entries(config).map(([argName, argConfig]) => ({
name: argName,
description: argConfig.description,
type: argConfig.type,
defaultValue: argConfig.defaultValue,
deprecationReason: argConfig.deprecationReason,
extensions: argConfig.extensions && toObjMap(argConfig.extensions),
astNode: argConfig.astNode,
}));
}

function isPlainObj(obj: mixed): boolean {
return isObjectLike(obj) && !Array.isArray(obj);
}
Expand Down
18 changes: 7 additions & 11 deletions src/type/directives.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@ import type {
GraphQLFieldConfigArgumentMap,
} from './definition';
import { GraphQLString, GraphQLBoolean } from './scalars';
import { argsToArgsConfig, GraphQLNonNull } from './definition';
import {
defineArguments,
argsToArgsConfig,
GraphQLNonNull,
} from './definition';

/**
* Test if the given value is a GraphQL directive.
Expand Down Expand Up @@ -44,7 +48,7 @@ export class GraphQLDirective {
name: string;
description: ?string;
locations: Array<DirectiveLocationEnum>;
args: Array<GraphQLArgument>;
args: $ReadOnlyArray<GraphQLArgument>;
isRepeatable: boolean;
extensions: ?ReadOnlyObjMap<mixed>;
astNode: ?DirectiveDefinitionNode;
Expand All @@ -69,15 +73,7 @@ export class GraphQLDirective {
`@${config.name} args must be an object with argument names as keys.`,
);

this.args = Object.entries(args).map(([argName, argConfig]) => ({
name: argName,
description: argConfig.description,
type: argConfig.type,
defaultValue: argConfig.defaultValue,
deprecationReason: argConfig.deprecationReason,
extensions: argConfig.extensions && toObjMap(argConfig.extensions),
astNode: argConfig.astNode,
}));
this.args = defineArguments(args);
}

toConfig(): GraphQLDirectiveNormalizedConfig {
Expand Down
208 changes: 192 additions & 16 deletions src/type/validate.js
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
import type { Path } from '../jsutils/Path';
import { addPath, pathToArray } from '../jsutils/Path';
import { didYouMean } from '../jsutils/didYouMean';
import { inspect } from '../jsutils/inspect';
import { invariant } from '../jsutils/invariant';
import { isIterableObject } from '../jsutils/isIterableObject';
import { isObjectLike } from '../jsutils/isObjectLike';
import { printPathArray } from '../jsutils/printPathArray';
import { suggestionList } from '../jsutils/suggestionList';

import { GraphQLError } from '../error/GraphQLError';
import { locatedError } from '../error/locatedError';
Expand All @@ -20,18 +28,22 @@ import type {
GraphQLUnionType,
GraphQLEnumType,
GraphQLInputObjectType,
GraphQLInputType,
} from './definition';
import { assertSchema } from './schema';
import { isIntrospectionType } from './introspection';
import { isDirective, GraphQLDeprecatedDirective } from './directives';
import {
getNamedType,
isObjectType,
isInterfaceType,
isUnionType,
isEnumType,
isInputObjectType,
isNamedType,
isListType,
isNonNullType,
isLeafType,
isInputType,
isOutputType,
isRequiredArgument,
Expand Down Expand Up @@ -190,6 +202,15 @@ function validateDirectives(context: SchemaValidationContext): void {
],
);
}

if (arg.defaultValue !== undefined) {
validateDefaultValue(arg.defaultValue, arg.type).forEach((error) => {
context.reportError(
`Argument @${directive.name}(${arg.name}:) has invalid default value: ${error}`,
arg.astNode?.defaultValue,
);
});
}
}
}
}
Expand Down Expand Up @@ -306,6 +327,15 @@ function validateFields(
],
);
}

if (arg.defaultValue !== undefined) {
validateDefaultValue(arg.defaultValue, arg.type).forEach((error) => {
context.reportError(
`Argument ${type.name}.${field.name}(${argName}:) has invalid default value: ${error}`,
arg.astNode?.defaultValue,
);
});
}
}
}
}
Expand Down Expand Up @@ -528,7 +558,7 @@ function validateInputFields(
);
}

// Ensure the arguments are valid
// Ensure the input fields are valid
for (const field of fields) {
// Ensure they are named correctly.
validateName(context, field);
Expand All @@ -552,6 +582,15 @@ function validateInputFields(
],
);
}

if (field.defaultValue !== undefined) {
validateDefaultValue(field.defaultValue, field.type).forEach((error) => {
context.reportError(
`Input field ${inputObj.name}.${field.name} has invalid default value: ${error}`,
field.astNode?.defaultValue,
);
});
}
}
}

Expand Down Expand Up @@ -584,29 +623,43 @@ function createInputObjectCircularRefsValidator(

const fields = Object.values(inputObj.getFields());
for (const field of fields) {
if (isNonNullType(field.type) && isInputObjectType(field.type.ofType)) {
const fieldType = field.type.ofType;
const cycleIndex = fieldPathIndexByTypeName[fieldType.name];

fieldPath.push(field);
if (cycleIndex === undefined) {
detectCycleRecursive(fieldType);
} else {
const cyclePath = fieldPath.slice(cycleIndex);
const pathStr = cyclePath.map((fieldObj) => fieldObj.name).join('.');
context.reportError(
`Cannot reference Input Object "${fieldType.name}" within itself through a series of non-null fields: "${pathStr}".`,
cyclePath.map((fieldObj) => fieldObj.astNode),
);
const fieldType = getNamedType(field.type);
if (isInputObjectType(fieldType)) {
const isNonNullField =
isNonNullType(field.type) && field.type.ofType === fieldType;
if (isNonNullField || !isEmptyValue(field.defaultValue)) {
const cycleIndex = fieldPathIndexByTypeName[fieldType.name];

fieldPath.push(field);
if (cycleIndex === undefined) {
detectCycleRecursive(fieldType);
} else {
const cyclePath = fieldPath.slice(cycleIndex);
const pathStr = cyclePath
.map((fieldObj) => fieldObj.name)
.join('.');
context.reportError(
`Cannot reference Input Object "${
fieldType.name
}" within itself through a series of ${
isNonNullField ? 'non-null fields' : 'non-empty default values'
}: "${pathStr}".`,
cyclePath.map((fieldObj) => fieldObj.astNode),
);
}
fieldPath.pop();
}
fieldPath.pop();
}
}

fieldPathIndexByTypeName[inputObj.name] = undefined;
}
}

function isEmptyValue(value: mixed) {
return value == null || (Array.isArray(value) && value.length === 0);
}

function getAllImplementsInterfaceNodes(
type: GraphQLObjectType | GraphQLInterfaceType,
iface: GraphQLInterfaceType,
Expand Down Expand Up @@ -643,3 +696,126 @@ function getDeprecatedDirectiveNode(
(node) => node.name.value === GraphQLDeprecatedDirective.name,
);
}

/**
* Coerce an internal JavaScript value given a GraphQL Input Type.
*/
function validateDefaultValue(
inputValue: mixed,
type: GraphQLInputType,
path?: Path,
): Array<string> {
if (isNonNullType(type)) {
if (inputValue !== null) {
return validateDefaultValue(inputValue, type.ofType, path);
}
return invalidDefaultValue(
`Expected non-nullable type "${inspect(type)}" not to be null.`,
path,
);
}

if (inputValue === null) {
return [];
}

if (isListType(type)) {
const itemType = type.ofType;
if (isIterableObject(inputValue)) {
const errors = [];
Array.from(inputValue).forEach((itemValue, index) => {
errors.push(
...validateDefaultValue(
itemValue,
itemType,
addPath(path, index, undefined),
),
);
});
return errors;
}
// Lists accept a non-list value as a list of one.
return validateDefaultValue(inputValue, itemType, path);
}

if (isInputObjectType(type)) {
if (!isObjectLike(inputValue)) {
return invalidDefaultValue(
`Expected type "${type.name}" to be an object.`,
path,
);
}

const errors = [];
const fieldDefs = type.getFields();

for (const field of Object.values(fieldDefs)) {
const fieldPath = addPath(path, field.name, type.name);
const fieldValue = inputValue[field.name];

if (fieldValue === undefined) {
if (field.defaultValue === undefined && isNonNullType(field.type)) {
return invalidDefaultValue(
`Field "${field.name}" of required type "${inspect(
field.type,
)}" was not provided.`,
fieldPath,
);
}
continue;
}

errors.push(...validateDefaultValue(fieldValue, field.type, fieldPath));
}

// Ensure every provided field is defined.
for (const fieldName of Object.keys(inputValue)) {
if (!fieldDefs[fieldName]) {
const suggestions = suggestionList(
fieldName,
Object.keys(type.getFields()),
);
errors.push(
...invalidDefaultValue(
`Field "${fieldName}" is not defined by type "${type.name}".` +
didYouMean(suggestions),
path,
),
);
}
}
return errors;
}

// istanbul ignore else (See: 'https://github.com/graphql/graphql-js/issues/2618')
if (isLeafType(type)) {
let parseResult;
let caughtError;

// Scalars and Enums determine if a input value is valid via serialize(),
// which can throw to indicate failure. If it throws, maintain a reference
// to the original error.
try {
parseResult = type.serialize(inputValue);
} catch (error) {
caughtError = error;
}
if (parseResult === undefined) {
return invalidDefaultValue(
caughtError?.message ?? `Expected type "${type.name}".`,
path,
);
}
return [];
}

// istanbul ignore next (Not reachable. All possible input types have been considered)
invariant(false, 'Unexpected input type: ' + inspect((type: empty)));
}

function invalidDefaultValue(message, path) {
return [
(path ? `(at defaultValue${printPathArray(pathToArray(path))}) ` : '') +
message,
];
}
Loading

0 comments on commit 153e8b9

Please sign in to comment.