From bb85e38ba92e7799ad796f3be063535970ca107b Mon Sep 17 00:00:00 2001 From: Lee Byron Date: Fri, 8 Dec 2017 16:36:59 -0800 Subject: [PATCH] Preserve original coercion errors, improve error quality. This is a fairly major refactoring of coerceValue which returns an Either so it can return a complete collection of errors. This allows originalError to be preserved for scalar coercion errors and ensures *all* errors are represented in the response. This had a minor change to the logic in execute / subscribe to allow for buildExecutionContext to abrupt complete with multiple errors. --- src/execution/__tests__/variables-test.js | 72 +++--- src/execution/execute.js | 85 ++++--- src/execution/values.js | 189 ++++---------- src/index.js | 4 +- src/subscription/__tests__/subscribe-test.js | 6 +- src/subscription/subscribe.js | 39 +-- src/type/__tests__/enumType-test.js | 3 +- src/utilities/__tests__/coerceValue-test.js | 133 ++++++++++ .../__tests__/isValidJSValue-test.js | 100 -------- src/utilities/coerceValue.js | 232 ++++++++++++++++++ src/utilities/index.js | 5 +- src/utilities/isValidJSValue.js | 104 +------- 12 files changed, 546 insertions(+), 426 deletions(-) create mode 100644 src/utilities/__tests__/coerceValue-test.js delete mode 100644 src/utilities/__tests__/isValidJSValue-test.js create mode 100644 src/utilities/coerceValue.js diff --git a/src/execution/__tests__/variables-test.js b/src/execution/__tests__/variables-test.js index 01645fbbc1b..f2d400e347e 100644 --- a/src/execution/__tests__/variables-test.js +++ b/src/execution/__tests__/variables-test.js @@ -11,7 +11,6 @@ import { expect } from 'chai'; import { describe, it } from 'mocha'; import { execute } from '../execute'; -import { coerceValue } from '../values'; import { parse } from '../../language'; import { GraphQLSchema, @@ -301,8 +300,8 @@ describe('Execute: Handles inputs', () => { { message: 'Variable "$input" got invalid value ' + - '{"a":"foo","b":"bar","c":null}.' + - '\nIn field "c": Expected "String!", found null.', + '{"a":"foo","b":"bar","c":null}; ' + + 'Expected non-nullable type String! at value.c.', locations: [{ line: 2, column: 17 }], path: undefined, }, @@ -318,8 +317,8 @@ describe('Execute: Handles inputs', () => { errors: [ { message: - 'Variable "$input" got invalid value "foo bar".' + - '\nExpected "TestInputObject", found not an object.', + 'Variable "$input" got invalid value "foo bar"; ' + + 'Expected object type TestInputObject.', locations: [{ line: 2, column: 17 }], path: undefined, }, @@ -335,8 +334,8 @@ describe('Execute: Handles inputs', () => { errors: [ { message: - 'Variable "$input" got invalid value {"a":"foo","b":"bar"}.' + - '\nIn field "c": Expected "String!", found null.', + 'Variable "$input" got invalid value {"a":"foo","b":"bar"}; ' + + 'Field value.c of required type String! was not provided.', locations: [{ line: 2, column: 17 }], path: undefined, }, @@ -358,10 +357,15 @@ describe('Execute: Handles inputs', () => { errors: [ { message: - 'Variable "$input" got invalid value {"na":{"a":"foo"}}.' + - '\nIn field "na": In field "c": Expected "String!", ' + - 'found null.' + - '\nIn field "nb": Expected "String!", found null.', + 'Variable "$input" got invalid value {"na":{"a":"foo"}}; ' + + 'Field value.na.c of required type String! was not provided.', + locations: [{ line: 2, column: 19 }], + path: undefined, + }, + { + message: + 'Variable "$input" got invalid value {"na":{"a":"foo"}}; ' + + 'Field value.nb of required type String! was not provided.', locations: [{ line: 2, column: 19 }], path: undefined, }, @@ -380,8 +384,8 @@ describe('Execute: Handles inputs', () => { { message: 'Variable "$input" got invalid value ' + - '{"a":"foo","b":"bar","c":"baz","extra":"dog"}.' + - '\nIn field "extra": Unknown field.', + '{"a":"foo","b":"bar","c":"baz","extra":"dog"}; ' + + 'Field "extra" is not defined by type TestInputObject.', locations: [{ line: 2, column: 17 }], path: undefined, }, @@ -535,8 +539,8 @@ describe('Execute: Handles inputs', () => { errors: [ { message: - 'Variable "$value" got invalid value null.\n' + - 'Expected "String!", found null.', + 'Variable "$value" got invalid value null; ' + + 'Expected non-nullable type String!.', locations: [{ line: 2, column: 31 }], path: undefined, }, @@ -608,31 +612,21 @@ describe('Execute: Handles inputs', () => { const ast = parse(doc); const variables = { value: [1, 2, 3] }; - expect(await execute(schema, ast, null, null, variables)).to.deep.equal({ + const result = await execute(schema, ast, null, null, variables); + + expect(result).to.deep.equal({ errors: [ { message: - 'Variable "$value" got invalid value [1,2,3].\nExpected type ' + - '"String", found [1,2,3]; String cannot represent an array value: [1,2,3]', + 'Variable "$value" got invalid value [1,2,3]; Expected type ' + + 'String; String cannot represent an array value: [1,2,3]', locations: [{ line: 2, column: 31 }], path: undefined, }, ], }); - }); - - it('coercing an array to GraphQLString throws TypeError', async () => { - let caughtError; - try { - coerceValue(GraphQLString, [1, 2, 3]); - } catch (error) { - caughtError = error; - } - expect(caughtError instanceof TypeError).to.equal(true); - expect(caughtError && caughtError.message).to.equal( - 'String cannot represent an array value: [1,2,3]', - ); + expect(result.errors[0].originalError).not.to.equal(undefined); }); it('serializing an array via GraphQLString throws TypeError', async () => { @@ -744,8 +738,8 @@ describe('Execute: Handles inputs', () => { errors: [ { message: - 'Variable "$input" got invalid value null.\n' + - 'Expected "[String]!", found null.', + 'Variable "$input" got invalid value null; ' + + 'Expected non-nullable type [String]!.', locations: [{ line: 2, column: 17 }], path: undefined, }, @@ -835,8 +829,8 @@ describe('Execute: Handles inputs', () => { errors: [ { message: - 'Variable "$input" got invalid value ["A",null,"B"].' + - '\nIn element #1: Expected "String!", found null.', + 'Variable "$input" got invalid value ["A",null,"B"]; ' + + 'Expected non-nullable type String! at value[1].', locations: [{ line: 2, column: 17 }], path: undefined, }, @@ -857,8 +851,8 @@ describe('Execute: Handles inputs', () => { errors: [ { message: - 'Variable "$input" got invalid value null.\n' + - 'Expected "[String!]!", found null.', + 'Variable "$input" got invalid value null; ' + + 'Expected non-nullable type [String!]!.', locations: [{ line: 2, column: 17 }], path: undefined, }, @@ -897,8 +891,8 @@ describe('Execute: Handles inputs', () => { errors: [ { message: - 'Variable "$input" got invalid value ["A",null,"B"].' + - '\nIn element #1: Expected "String!", found null.', + 'Variable "$input" got invalid value ["A",null,"B"]; ' + + 'Expected non-nullable type String! at value[1].', locations: [{ line: 2, column: 17 }], path: undefined, }, diff --git a/src/execution/execute.js b/src/execution/execute.js index 6baeaf05533..a1dbb400a37 100644 --- a/src/execution/execute.js +++ b/src/execution/execute.js @@ -187,19 +187,19 @@ function executeImpl( // If a valid context cannot be created due to incorrect arguments, // a "Response" with only errors is returned. - let context; - try { - context = buildExecutionContext( - schema, - document, - rootValue, - contextValue, - variableValues, - operationName, - fieldResolver, - ); - } catch (error) { - return { errors: [error] }; + const context = buildExecutionContext( + schema, + document, + rootValue, + contextValue, + variableValues, + operationName, + fieldResolver, + ); + + // Return early errors if execution context failed. + if (Array.isArray(context)) { + return { errors: context }; } // Return a Promise that will eventually resolve to the data described by @@ -291,20 +291,18 @@ export function buildExecutionContext( rawVariableValues: ?ObjMap, operationName: ?string, fieldResolver: ?GraphQLFieldResolver, -): ExecutionContext { +): $ReadOnlyArray | ExecutionContext { const errors: Array = []; - let operation: ?OperationDefinitionNode; + let operation: OperationDefinitionNode | void; + let hasMultipleAssumedOperations = false; const fragments: ObjMap = Object.create(null); - document.definitions.forEach(definition => { + for (let i = 0; i < document.definitions.length; i++) { + const definition = document.definitions[i]; switch (definition.kind) { case Kind.OPERATION_DEFINITION: if (!operationName && operation) { - throw new GraphQLError( - 'Must provide operation name if query contains ' + - 'multiple operations.', - ); - } - if ( + hasMultipleAssumedOperations = true; + } else if ( !operationName || (definition.name && definition.name.value === operationName) ) { @@ -315,19 +313,46 @@ export function buildExecutionContext( fragments[definition.name.value] = definition; break; } - }); + } + if (!operation) { if (operationName) { - throw new GraphQLError(`Unknown operation named "${operationName}".`); + errors.push( + new GraphQLError(`Unknown operation named "${operationName}".`), + ); } else { - throw new GraphQLError('Must provide an operation.'); + errors.push(new GraphQLError('Must provide an operation.')); } + } else if (hasMultipleAssumedOperations) { + errors.push( + new GraphQLError( + 'Must provide operation name if query contains ' + + 'multiple operations.', + ), + ); } - const variableValues = getVariableValues( - schema, - operation.variableDefinitions || [], - rawVariableValues || {}, - ); + + let variableValues; + if (operation) { + const coercedVariableValues = getVariableValues( + schema, + operation.variableDefinitions || [], + rawVariableValues || {}, + ); + + if (coercedVariableValues.errors) { + errors.push(...coercedVariableValues.errors); + } else { + variableValues = coercedVariableValues.coerced; + } + } + + if (errors.length !== 0) { + return errors; + } + + invariant(operation, 'Has operation if no errors.'); + invariant(variableValues, 'Has variables if no errors.'); return { schema, diff --git a/src/execution/values.js b/src/execution/values.js index 2e4c679ad07..342fad3ecfd 100644 --- a/src/execution/values.js +++ b/src/execution/values.js @@ -7,30 +7,19 @@ * @flow */ -import { createIterator, isCollection } from 'iterall'; - import { GraphQLError } from '../error'; import find from '../jsutils/find'; -import invariant from '../jsutils/invariant'; -import isNullish from '../jsutils/isNullish'; import isInvalid from '../jsutils/isInvalid'; import keyMap from '../jsutils/keyMap'; +import { coerceValue } from '../utilities/coerceValue'; import { typeFromAST } from '../utilities/typeFromAST'; import { valueFromAST } from '../utilities/valueFromAST'; -import { isValidJSValue } from '../utilities/isValidJSValue'; import { isValidLiteralValue } from '../utilities/isValidLiteralValue'; import * as Kind from '../language/kinds'; import { print } from '../language/printer'; -import { - isInputType, - GraphQLScalarType, - GraphQLEnumType, - GraphQLInputObjectType, - GraphQLList, - GraphQLNonNull, -} from '../type/definition'; +import { isInputType, GraphQLNonNull } from '../type/definition'; import type { ObjMap } from '../jsutils/ObjMap'; -import type { GraphQLInputType, GraphQLField } from '../type/definition'; +import type { GraphQLField } from '../type/definition'; import type { GraphQLDirective } from '../type/directives'; import type { GraphQLSchema } from '../type/schema'; import type { @@ -40,6 +29,11 @@ import type { VariableDefinitionNode, } from '../language/ast'; +type CoercedVariableValues = {| + errors: $ReadOnlyArray | void, + coerced: { [variable: string]: mixed } | void, +|}; + /** * Prepares an object map of variableValues of the correct type based on the * provided variable definitions and arbitrary input. If the input cannot be @@ -53,50 +47,61 @@ export function getVariableValues( schema: GraphQLSchema, varDefNodes: Array, inputs: ObjMap, -): { [variable: string]: mixed } { +): CoercedVariableValues { + const errors = []; const coercedValues = {}; for (let i = 0; i < varDefNodes.length; i++) { const varDefNode = varDefNodes[i]; const varName = varDefNode.variable.name.value; const varType = typeFromAST(schema, varDefNode.type); if (!isInputType(varType)) { - throw new GraphQLError( - `Variable "$${varName}" expected value of type ` + - `"${print(varDefNode.type)}" which cannot be used as an input type.`, - [varDefNode.type], + errors.push( + new GraphQLError( + `Variable "$${varName}" expected value of type ` + + `"${print( + varDefNode.type, + )}" which cannot be used as an input type.`, + [varDefNode.type], + ), ); - } - - const value = inputs[varName]; - if (isInvalid(value)) { - const defaultValue = varDefNode.defaultValue; - if (defaultValue) { - coercedValues[varName] = valueFromAST(defaultValue, varType); - } - if (varType instanceof GraphQLNonNull) { - throw new GraphQLError( - `Variable "$${varName}" of required type ` + - `"${String(varType)}" was not provided.`, - [varDefNode], - ); - } } else { - const errors = isValidJSValue(value, varType); - if (errors.length) { - const message = errors ? '\n' + errors.join('\n') : ''; - throw new GraphQLError( - `Variable "$${varName}" got invalid value ` + - `${JSON.stringify(value)}.${message}`, - [varDefNode], - ); + const value = inputs[varName]; + if (isInvalid(value)) { + if (varType instanceof GraphQLNonNull) { + errors.push( + new GraphQLError( + `Variable "$${varName}" of required type ` + + `"${String(varType)}" was not provided.`, + [varDefNode], + ), + ); + } else { + const defaultValue = varDefNode.defaultValue; + if (defaultValue) { + coercedValues[varName] = valueFromAST(defaultValue, varType); + } + } + } else { + const coercedValue = coerceValue(value, varType, varDefNode); + if (coercedValue.errors) { + errors.push( + ...coercedValue.errors.map(error => { + error.message = + `Variable "$${varName}" got invalid value ${JSON.stringify( + value, + )}; ` + error.message; + return error; + }), + ); + } else { + coercedValues[varName] = coercedValue.value; + } } - - const coercedValue = coerceValue(varType, value); - invariant(!isInvalid(coercedValue), 'Should have reported error.'); - coercedValues[varName] = coercedValue; } } - return coercedValues; + return errors.length === 0 + ? { errors: undefined, coerced: coercedValues } + : { errors, coerced: undefined }; } /** @@ -200,93 +205,3 @@ export function getDirectiveValues( return getArgumentValues(directiveDef, directiveNode, variableValues); } } - -/** - * Given a type and any value, return a runtime value coerced to match the type. - */ -export function coerceValue(type: GraphQLInputType, value: mixed): mixed { - // Ensure flow knows that we treat function params as const. - const _value = value; - - if (isInvalid(_value)) { - return; // Intentionally return no value. - } - - if (type instanceof GraphQLNonNull) { - if (_value === null) { - return; // Intentionally return no value. - } - return coerceValue(type.ofType, _value); - } - - if (_value === null) { - // Intentionally return the value null. - return null; - } - - if (type instanceof GraphQLList) { - const itemType = type.ofType; - if (isCollection(_value)) { - const coercedValues = []; - const valueIter = createIterator(_value); - if (!valueIter) { - return; // Intentionally return no value. - } - let step; - while (!(step = valueIter.next()).done) { - const itemValue = coerceValue(itemType, step.value); - if (isInvalid(itemValue)) { - return; // Intentionally return no value. - } - coercedValues.push(itemValue); - } - return coercedValues; - } - const coercedValue = coerceValue(itemType, _value); - if (isInvalid(coercedValue)) { - return; // Intentionally return no value. - } - return [coerceValue(itemType, _value)]; - } - - if (type instanceof GraphQLInputObjectType) { - if (typeof _value !== 'object') { - return; // Intentionally return no value. - } - const coercedObj = Object.create(null); - const fields = type.getFields(); - const fieldNames = Object.keys(fields); - for (let i = 0; i < fieldNames.length; i++) { - const fieldName = fieldNames[i]; - const field = fields[fieldName]; - if (isInvalid(_value[fieldName])) { - if (!isInvalid(field.defaultValue)) { - coercedObj[fieldName] = field.defaultValue; - } else if (field.type instanceof GraphQLNonNull) { - return; // Intentionally return no value. - } - continue; - } - const fieldValue = coerceValue(field.type, _value[fieldName]); - if (isInvalid(fieldValue)) { - return; // Intentionally return no value. - } - coercedObj[fieldName] = fieldValue; - } - return coercedObj; - } - - invariant( - type instanceof GraphQLScalarType || type instanceof GraphQLEnumType, - 'Must be input type', - ); - - const parsed = type.parseValue(_value); - if (isNullish(parsed)) { - // null or invalid values represent a failure to parse correctly, - // in which case no value is returned. - return; - } - - return parsed; -} diff --git a/src/index.js b/src/index.js index 0ea928fbe6e..b595e5047fa 100644 --- a/src/index.js +++ b/src/index.js @@ -307,7 +307,9 @@ export { // A helper to use within recursive-descent visitors which need to be aware of // the GraphQL type system. TypeInfo, - // Determine if JavaScript values adhere to a GraphQL type. + // Coerces a JavaScript value to a GraphQL type, or produces errors. + coerceValue, + // @deprecated use coerceValue isValidJSValue, // Determine if AST values adhere to a GraphQL type. isValidLiteralValue, diff --git a/src/subscription/__tests__/subscribe-test.js b/src/subscription/__tests__/subscribe-test.js index 87e14227dd1..7dba52ceef2 100644 --- a/src/subscription/__tests__/subscribe-test.js +++ b/src/subscription/__tests__/subscribe-test.js @@ -474,14 +474,16 @@ describe('Subscription Initialization Phase', () => { errors: [ { message: - 'Variable "$priority" got invalid value "meow".\nExpected ' + - 'type "Int", found "meow"; Int cannot represent non 32-bit signed ' + + 'Variable "$priority" got invalid value "meow"; Expected ' + + 'type Int; Int cannot represent non 32-bit signed ' + 'integer value: meow', locations: [{ line: 2, column: 21 }], path: undefined, }, ], }); + + expect(result.errors[0].originalError).not.to.equal(undefined); }); }); diff --git a/src/subscription/subscribe.js b/src/subscription/subscribe.js index eac9ebe5e5e..4c808bc4ddb 100644 --- a/src/subscription/subscribe.js +++ b/src/subscription/subscribe.js @@ -159,11 +159,18 @@ function subscribeImpl( // Resolve the Source Stream, then map every source value to a // ExecutionResult value as described above. - return sourcePromise.then( - sourceStream => - mapAsyncIterator(sourceStream, mapSourceToResponse, reportGraphQLError), - reportGraphQLError, - ); + return sourcePromise.then(resultOrStream => { + if (isAsyncIterable(resultOrStream)) { + return mapAsyncIterator( + // Note: isAsyncIterable above ensures this will be correct. + ((resultOrStream: any): AsyncIterable), + mapSourceToResponse, + reportGraphQLError, + ); + } + // Note: isAsyncIterable above ensures this will be correct. + return ((resultOrStream: any): ExecutionResult); + }, reportGraphQLError); } /** @@ -192,7 +199,7 @@ export function createSourceEventStream( variableValues?: ObjMap, operationName?: ?string, fieldResolver?: ?GraphQLFieldResolver, -): Promise> { +): Promise | ExecutionResult> { // If arguments are missing or incorrectly typed, this is an internal // developer mistake which should throw an early error. assertValidExecutionArguments(schema, document, variableValues); @@ -210,6 +217,11 @@ export function createSourceEventStream( fieldResolver, ); + // Return early errors if execution context failed. + if (Array.isArray(exeContext)) { + return Promise.resolve({ errors: exeContext }); + } + const type = getOperationRootType(schema, exeContext.operation); const fields = collectFields( exeContext, @@ -260,15 +272,14 @@ export function createSourceEventStream( } // Assert field returned an event stream, otherwise yield an error. - if (!isAsyncIterable(eventStream)) { - throw new Error( - 'Subscription field must return Async Iterable. Received: ' + - String(eventStream), - ); + if (isAsyncIterable(eventStream)) { + // Note: isAsyncIterable above ensures this will be correct. + return ((eventStream: any): AsyncIterable); } - - // Note: isAsyncIterable above ensures this will be correct. - return ((eventStream: any): AsyncIterable); + throw new Error( + 'Subscription field must return Async Iterable. Received: ' + + String(eventStream), + ); }); } catch (error) { return Promise.reject(error); diff --git a/src/type/__tests__/enumType-test.js b/src/type/__tests__/enumType-test.js index cadfbf8dfd8..656d67f2428 100644 --- a/src/type/__tests__/enumType-test.js +++ b/src/type/__tests__/enumType-test.js @@ -274,8 +274,7 @@ describe('Type System: Enum Values', () => { errors: [ { message: - 'Variable "$color" got invalid value 2.' + - '\nExpected type "Color", found 2.', + 'Variable "$color" got invalid value 2; Expected type Color.', locations: [{ line: 1, column: 12 }], }, ], diff --git a/src/utilities/__tests__/coerceValue-test.js b/src/utilities/__tests__/coerceValue-test.js new file mode 100644 index 00000000000..451c2dad761 --- /dev/null +++ b/src/utilities/__tests__/coerceValue-test.js @@ -0,0 +1,133 @@ +/** + * Copyright (c) 2015-present, Facebook, Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import { describe, it } from 'mocha'; +import { expect } from 'chai'; +import { coerceValue } from '../coerceValue'; +import { GraphQLInt, GraphQLFloat, GraphQLString } from '../../type'; + +function expectNoErrors(result) { + expect(result.errors).to.equal(undefined); +} + +function expectError(result, expected) { + const messages = result.errors && result.errors.map(error => error.message); + expect(messages).to.deep.equal([expected]); +} + +describe('coerceValue', () => { + it('coercing an array to GraphQLString produces an error', () => { + const result = coerceValue([1, 2, 3], GraphQLString); + expectError( + result, + 'Expected type String; String cannot represent an array value: [1,2,3]', + ); + expect(result.errors[0].originalError.message).to.equal( + 'String cannot represent an array value: [1,2,3]', + ); + }); + + describe('for GraphQLInt', () => { + it('returns no error for int input', () => { + const result = coerceValue('1', GraphQLInt); + expectNoErrors(result); + }); + + it('returns no error for negative int input', () => { + const result = coerceValue('-1', GraphQLInt); + expectNoErrors(result); + }); + + it('returns no error for exponent input', () => { + const result = coerceValue('1e3', GraphQLInt); + expectNoErrors(result); + }); + + it('returns a single error for empty value', () => { + const result = coerceValue(null, GraphQLInt); + expectNoErrors(result); + }); + + it('returns a single error for empty value', () => { + const result = coerceValue('', GraphQLInt); + expectError( + result, + 'Expected type Int; Int cannot represent non 32-bit signed integer value: (empty string)', + ); + }); + + it('returns error for float input as int', () => { + const result = coerceValue('1.5', GraphQLInt); + expectError( + result, + 'Expected type Int; Int cannot represent non-integer value: 1.5', + ); + }); + + it('returns a single error for char input', () => { + const result = coerceValue('a', GraphQLInt); + expectError( + result, + 'Expected type Int; Int cannot represent non 32-bit signed integer value: a', + ); + }); + + it('returns a single error for char input', () => { + const result = coerceValue('meow', GraphQLInt); + expectError( + result, + 'Expected type Int; Int cannot represent non 32-bit signed integer value: meow', + ); + }); + }); + + describe('for GraphQLFloat', () => { + it('returns no error for int input', () => { + const result = coerceValue('1', GraphQLFloat); + expectNoErrors(result); + }); + + it('returns no error for exponent input', () => { + const result = coerceValue('1e3', GraphQLFloat); + expectNoErrors(result); + }); + + it('returns no error for float input', () => { + const result = coerceValue('1.5', GraphQLFloat); + expectNoErrors(result); + }); + + it('returns a single error for empty value', () => { + const result = coerceValue(null, GraphQLFloat); + expectNoErrors(result); + }); + + it('returns a single error for empty value', () => { + const result = coerceValue('', GraphQLFloat); + expectError( + result, + 'Expected type Float; Float cannot represent non numeric value: (empty string)', + ); + }); + + it('returns a single error for char input', () => { + const result = coerceValue('a', GraphQLFloat); + expectError( + result, + 'Expected type Float; Float cannot represent non numeric value: a', + ); + }); + + it('returns a single error for char input', () => { + const result = coerceValue('meow', GraphQLFloat); + expectError( + result, + 'Expected type Float; Float cannot represent non numeric value: meow', + ); + }); + }); +}); diff --git a/src/utilities/__tests__/isValidJSValue-test.js b/src/utilities/__tests__/isValidJSValue-test.js deleted file mode 100644 index 4c482e88e3f..00000000000 --- a/src/utilities/__tests__/isValidJSValue-test.js +++ /dev/null @@ -1,100 +0,0 @@ -/** - * Copyright (c) 2015-present, Facebook, Inc. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -import { describe, it } from 'mocha'; -import { expect } from 'chai'; -import { isValidJSValue } from '../isValidJSValue'; -import { GraphQLInt, GraphQLFloat } from '../../type'; - -function expectNoErrors(result) { - expect(result).to.be.an.instanceof(Array); - expect(result.length).to.equal(0); -} - -function expectErrorResult(result, size) { - expect(result).to.be.an.instanceof(Array); - expect(result.length).to.equal(size); -} - -describe('isValidJSValue for GraphQLInt', () => { - it('returns no error for int input', () => { - const result = isValidJSValue('1', GraphQLInt); - expectNoErrors(result); - }); - - it('returns no error for negative int input', () => { - const result = isValidJSValue('-1', GraphQLInt); - expectNoErrors(result); - }); - - it('returns no error for exponent input', () => { - const result = isValidJSValue('1e3', GraphQLInt); - expectNoErrors(result); - }); - - it('returns a single error for empty value', () => { - const result = isValidJSValue(null, GraphQLInt); - expectNoErrors(result); - }); - - it('returns a single error for empty value', () => { - const result = isValidJSValue('', GraphQLInt); - expectErrorResult(result, 1); - }); - - it('returns error for float input as int', () => { - const result = isValidJSValue('1.5', GraphQLInt); - expectErrorResult(result, 1); - }); - - it('returns a single error for char input', () => { - const result = isValidJSValue('a', GraphQLInt); - expectErrorResult(result, 1); - }); - - it('returns a single error for char input', () => { - const result = isValidJSValue('meow', GraphQLInt); - expectErrorResult(result, 1); - }); -}); - -describe('isValidJSValue for GraphQLFloat', () => { - it('returns no error for int input', () => { - const result = isValidJSValue('1', GraphQLFloat); - expectNoErrors(result); - }); - - it('returns no error for exponent input', () => { - const result = isValidJSValue('1e3', GraphQLFloat); - expectNoErrors(result); - }); - - it('returns no error for float input', () => { - const result = isValidJSValue('1.5', GraphQLFloat); - expectNoErrors(result); - }); - - it('returns a single error for empty value', () => { - const result = isValidJSValue(null, GraphQLFloat); - expectNoErrors(result); - }); - - it('returns a single error for empty value', () => { - const result = isValidJSValue('', GraphQLFloat); - expectErrorResult(result, 1); - }); - - it('returns a single error for char input', () => { - const result = isValidJSValue('a', GraphQLFloat); - expectErrorResult(result, 1); - }); - - it('returns a single error for char input', () => { - const result = isValidJSValue('meow', GraphQLFloat); - expectErrorResult(result, 1); - }); -}); diff --git a/src/utilities/coerceValue.js b/src/utilities/coerceValue.js new file mode 100644 index 00000000000..baad2b5e882 --- /dev/null +++ b/src/utilities/coerceValue.js @@ -0,0 +1,232 @@ +/** + * Copyright (c) 2015-present, Facebook, Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +import { forEach, isCollection } from 'iterall'; +import isInvalid from '../jsutils/isInvalid'; +import isNullish from '../jsutils/isNullish'; +import { GraphQLError } from '../error'; +import type { ASTNode } from '../language/ast'; +import { + GraphQLEnumType, + GraphQLInputObjectType, + GraphQLList, + GraphQLNonNull, + GraphQLScalarType, +} from '../type/definition'; +import type { GraphQLInputType } from '../type/definition'; + +type CoercedValue = {| + +errors: $ReadOnlyArray | void, + +value: mixed, +|}; + +type Path = {| +prev: Path | void, +key: string | number |}; + +/** + * Coerces a JavaScript value given a GraphQL Type. + * + * Returns either a value which is valid for the provided type or a list of + * encountered coercion errors. + * + */ +export function coerceValue( + value: mixed, + type: GraphQLInputType, + blameNode?: ASTNode, + path?: Path, +): CoercedValue { + // A value must be provided if the type is non-null. + if (type instanceof GraphQLNonNull) { + if (isNullish(value)) { + return ofErrors([ + coercionError( + `Expected non-nullable type ${String(type)}`, + blameNode, + path, + ), + ]); + } + return coerceValue(value, type.ofType, blameNode, path); + } + + if (isNullish(value)) { + // Explicitly return the value null. + return ofValue(null); + } + + if (type instanceof GraphQLScalarType) { + // Scalars determine if a value is valid via parseValue(), which can + // throw to indicate failure. If it throws, maintain a reference to + // the original error. + try { + const parseResult = type.parseValue(value); + if (isInvalid(parseResult)) { + return ofErrors([ + coercionError(`Expected type ${type.name}`, blameNode, path), + ]); + } + return ofValue(parseResult); + } catch (error) { + return ofErrors([ + coercionError(`Expected type ${type.name}`, blameNode, path, error), + ]); + } + } + + if (type instanceof GraphQLEnumType) { + if (typeof value === 'string') { + const enumValue = type.getValue(value); + if (enumValue) { + return ofValue(enumValue.value); + } + } + return ofErrors([ + coercionError(`Expected type ${type.name}`, blameNode, path), + ]); + } + + if (type instanceof GraphQLList) { + const itemType = type.ofType; + if (isCollection(value)) { + let errors; + const coercedValue = []; + forEach((value: any), (itemValue, index) => { + const coercedItem = coerceValue( + itemValue, + itemType, + blameNode, + atPath(path, index), + ); + if (coercedItem.errors) { + errors = add(errors, coercedItem.errors); + } else if (!errors) { + coercedValue.push(coercedItem.value); + } + }); + return errors ? ofErrors(errors) : ofValue(coercedValue); + } + // Lists accept a non-list value as a list of one. + const coercedItem = coerceValue(value, itemType, blameNode); + return coercedItem.errors ? coercedItem : ofValue([coercedItem.value]); + } + + if (type instanceof GraphQLInputObjectType) { + if (typeof value !== 'object') { + return ofErrors([ + coercionError(`Expected object type ${type.name}`, blameNode, path), + ]); + } + let errors; + const coercedValue = {}; + const fields = type.getFields(); + + // Ensure every defined field is valid. + for (const fieldName in fields) { + if (hasOwnProperty.call(fields, fieldName)) { + const field = fields[fieldName]; + const fieldValue = value[fieldName]; + if (isInvalid(fieldValue)) { + if (!isInvalid(field.defaultValue)) { + coercedValue[fieldName] = field.defaultValue; + } else if (field.type instanceof GraphQLNonNull) { + errors = add( + errors, + coercionError( + `Field ${printPath(atPath(path, fieldName))} of required ` + + `type ${String(field.type)} was not provided`, + blameNode, + ), + ); + } + } else { + const coercedField = coerceValue( + fieldValue, + field.type, + blameNode, + atPath(path, fieldName), + ); + if (coercedField.errors) { + errors = add(errors, coercedField.errors); + } else if (!errors) { + coercedValue[fieldName] = coercedField.value; + } + } + } + } + + // Ensure every provided field is defined. + for (const fieldName in value) { + if (hasOwnProperty.call(value, fieldName)) { + if (!fields[fieldName]) { + errors = add( + errors, + coercionError( + `Field "${fieldName}" is not defined by type ${type.name}`, + blameNode, + path, + ), + ); + } + } + } + + return errors ? ofErrors(errors) : ofValue(coercedValue); + } + + throw new Error(`Unexpected type ${String((type: empty))}`); +} + +function ofValue(value) { + return { errors: undefined, value }; +} + +function ofErrors(errors) { + return { errors, value: undefined }; +} + +function add(errors, moreErrors) { + return (errors || []).concat(moreErrors); +} + +function atPath(prev, key) { + return { prev, key }; +} + +function coercionError(message, blameNode, path, originalError) { + const pathStr = printPath(path); + // Return a GraphQLError instance + return new GraphQLError( + message + + (pathStr ? ' at ' + pathStr : '') + + (originalError && originalError.message + ? '; ' + originalError.message + : '.'), + blameNode, + undefined, + undefined, + undefined, + originalError, + ); +} + +// Build a string describing the path into the value where the error was found +function printPath(path) { + let pathStr = ''; + let currentPath = path; + while (currentPath) { + pathStr = + (typeof currentPath.key === 'string' + ? '.' + currentPath.key + : '[' + String(currentPath.key) + ']') + pathStr; + currentPath = currentPath.prev; + } + return pathStr ? 'value' + pathStr : ''; +} + +const hasOwnProperty = Object.prototype.hasOwnProperty; diff --git a/src/utilities/index.js b/src/utilities/index.js index 54f24d8b6a3..319bb0d8d8e 100644 --- a/src/utilities/index.js +++ b/src/utilities/index.js @@ -73,7 +73,10 @@ export { astFromValue } from './astFromValue'; // the GraphQL type system. export { TypeInfo } from './TypeInfo'; -// Determine if JavaScript values adhere to a GraphQL type. +// Coerces a JavaScript value to a GraphQL type, or produces errors. +export { coerceValue } from './coerceValue'; + +// @deprecated use coerceValue export { isValidJSValue } from './isValidJSValue'; // Determine if AST values adhere to a GraphQL type. diff --git a/src/utilities/isValidJSValue.js b/src/utilities/isValidJSValue.js index 04e03d20c1b..4c8c46e6253 100644 --- a/src/utilities/isValidJSValue.js +++ b/src/utilities/isValidJSValue.js @@ -7,112 +7,16 @@ * @flow */ -import { forEach, isCollection } from 'iterall'; - -import invariant from '../jsutils/invariant'; -import isInvalid from '../jsutils/isInvalid'; -import isNullish from '../jsutils/isNullish'; -import { - GraphQLScalarType, - GraphQLEnumType, - GraphQLInputObjectType, - GraphQLList, - GraphQLNonNull, -} from '../type/definition'; +import { coerceValue } from './coerceValue'; import type { GraphQLInputType } from '../type/definition'; /** - * Given a JavaScript value and a GraphQL type, determine if the value will be - * accepted for that type. This is primarily useful for validating the - * runtime values of query variables. + * Deprecated. Use coerceValue() directly for richer information. */ export function isValidJSValue( value: mixed, type: GraphQLInputType, ): Array { - // A value must be provided if the type is non-null. - if (type instanceof GraphQLNonNull) { - if (isNullish(value)) { - return [`Expected "${String(type)}", found null.`]; - } - return isValidJSValue(value, type.ofType); - } - - if (isNullish(value)) { - return []; - } - - // Lists accept a non-list value as a list of one. - if (type instanceof GraphQLList) { - const itemType = type.ofType; - if (isCollection(value)) { - const errors = []; - forEach((value: any), (item, index) => { - errors.push.apply( - errors, - isValidJSValue(item, itemType).map( - error => `In element #${index}: ${error}`, - ), - ); - }); - return errors; - } - return isValidJSValue(value, itemType); - } - - // Input objects check each defined field. - if (type instanceof GraphQLInputObjectType) { - if (typeof value !== 'object' || value === null) { - return [`Expected "${type.name}", found not an object.`]; - } - const fields = type.getFields(); - - const errors = []; - - // Ensure every provided field is defined. - Object.keys(value).forEach(providedField => { - if (!fields[providedField]) { - errors.push(`In field "${providedField}": Unknown field.`); - } - }); - - // Ensure every defined field is valid. - Object.keys(fields).forEach(fieldName => { - const newErrors = isValidJSValue( - (value: any)[fieldName], - fields[fieldName].type, - ); - errors.push( - ...newErrors.map(error => `In field "${fieldName}": ${error}`), - ); - }); - - return errors; - } - - // Enum types only accept certain values - if (type instanceof GraphQLEnumType) { - if (typeof value !== 'string' || !type.getValue(value)) { - const printed = JSON.stringify(value); - return [`Expected type "${type.name}", found ${printed}.`]; - } - - return []; - } - - invariant(type instanceof GraphQLScalarType, 'Must be scalar type'); - - // Scalars determine if a value is valid via parseValue(). - try { - const parseResult = type.parseValue(value); - if (isInvalid(parseResult)) { - return [`Expected type "${type.name}", found ${JSON.stringify(value)}.`]; - } - } catch (error) { - const printed = JSON.stringify(value); - const message = error.message; - return [`Expected type "${type.name}", found ${printed}; ${message}`]; - } - - return []; + const errors = coerceValue(value, type).errors; + return errors ? errors.map(error => error.message) : []; }