diff --git a/src/error/locatedError.js b/src/error/locatedError.js index 49fb6d0cb8..a0e3c868fb 100644 --- a/src/error/locatedError.js +++ b/src/error/locatedError.js @@ -23,7 +23,7 @@ export function locatedError( ): GraphQLError { // Note: this uses a brand-check to support GraphQL errors originating from // other contexts. - if (originalError && originalError.locations) { + if (originalError && originalError.path) { return (originalError: any); } @@ -32,9 +32,9 @@ export function locatedError( 'An unknown error occurred.'; return new GraphQLError( message, - nodes, - undefined, - undefined, + originalError && (originalError: any).nodes || nodes, + originalError && (originalError: any).source, + originalError && (originalError: any).positions, path, originalError ); diff --git a/src/execution/__tests__/abstract-test.js b/src/execution/__tests__/abstract-test.js index 6d3c40de10..0b823ba5fd 100644 --- a/src/execution/__tests__/abstract-test.js +++ b/src/execution/__tests__/abstract-test.js @@ -258,7 +258,8 @@ describe('Execute: Handles execution of abstract types', () => { { message: 'Runtime Object type "Human" is not a possible type for "Pet".', - locations: [ { line: 2, column: 7 } ] + locations: [ { line: 2, column: 7 } ], + path: [ 'pets', 2 ] } ] }); @@ -347,7 +348,8 @@ describe('Execute: Handles execution of abstract types', () => { { message: 'Runtime Object type "Human" is not a possible type for "Pet".', - locations: [ { line: 2, column: 7 } ] + locations: [ { line: 2, column: 7 } ], + path: [ 'pets', 2 ] } ] }); diff --git a/src/execution/__tests__/variables-test.js b/src/execution/__tests__/variables-test.js index 12b4b3e215..078c2134bd 100644 --- a/src/execution/__tests__/variables-test.js +++ b/src/execution/__tests__/variables-test.js @@ -199,24 +199,30 @@ describe('Execute: Handles inputs', () => { const result = await execute(schema, ast); - expect(result).to.deep.equal({ + expect(result).to.containSubset({ data: { fieldWithObjectInput: null - } + }, + errors: [ { + message: + 'Argument "input" got invalid value ["foo", "bar", "baz"].\n' + + 'Expected "TestInputObject", found not an object.', + path: [ 'fieldWithObjectInput' ] + } ] }); }); it('properly runs parseLiteral on complex scalar types', async () => { const doc = ` { - fieldWithObjectInput(input: {a: "foo", d: "SerializedValue"}) + fieldWithObjectInput(input: {c: "foo", d: "SerializedValue"}) } `; const ast = parse(doc); return expect(await execute(schema, ast)).to.deep.equal({ data: { - fieldWithObjectInput: '{"a":"foo","d":"DeserializedValue"}', + fieldWithObjectInput: '{"c":"foo","d":"DeserializedValue"}', } }); }); @@ -482,7 +488,21 @@ describe('Execute: Handles inputs', () => { }); }); - describe('Handles non-nullable scalars', () => { + describe('Handles non-nullable scalars', async () => { + it('allows non-nullable inputs to be omitted given a default', async () => { + const doc = ` + query SetsNonNullable($value: String = "default") { + fieldWithNonNullableStringInput(input: $value) + } + `; + + expect(await execute(schema, parse(doc))).to.deep.equal({ + data: { + fieldWithNonNullableStringInput: '"default"' + } + }); + }); + it('does not allow non-nullable inputs to be omitted in a variable', async () => { const doc = ` query SetsNonNullable($value: String!) { @@ -522,7 +542,8 @@ describe('Execute: Handles inputs', () => { expect(caughtError).to.containSubset({ locations: [ { line: 2, column: 31 } ], message: - 'Variable "$value" of required type "String!" was not provided.' + 'Variable "$value" got invalid value null.\n' + + 'Expected "String!", found null.' }); }); @@ -558,7 +579,7 @@ describe('Execute: Handles inputs', () => { }); }); - it('passes along null for non-nullable inputs if explcitly set in the query', async () => { + it('reports error for missing non-nullable inputs', async () => { const doc = ` { fieldWithNonNullableStringInput @@ -569,7 +590,39 @@ describe('Execute: Handles inputs', () => { return expect(await execute(schema, ast)).to.deep.equal({ data: { fieldWithNonNullableStringInput: null - } + }, + errors: [ { + message: 'Argument "input" of required type "String!" was not provided.', + locations: [ { line: 3, column: 9 } ], + path: [ 'fieldWithNonNullableStringInput' ] + } ] + }); + }); + + it('reports error for non-provided variables for non-nullable inputs', async () => { + // Note: this test would typically fail validation before encountering + // this execution error, however for queries which previously validated + // and are being run against a new schema which have introduced a breaking + // change to make a formerly non-required argument required, this asserts + // failure before allowing the underlying code to receive a non-null value. + const doc = ` + { + fieldWithNonNullableStringInput(input: $foo) + } + `; + const ast = parse(doc); + + return expect(await execute(schema, ast)).to.deep.equal({ + data: { + fieldWithNonNullableStringInput: null + }, + errors: [ { + message: + 'Argument "input" of required type "String!" was provided the ' + + 'variable "$foo" which was not provided a runtime value.', + locations: [ { line: 3, column: 48 } ], + path: [ 'fieldWithNonNullableStringInput' ] + } ] }); }); }); @@ -644,7 +697,8 @@ describe('Execute: Handles inputs', () => { expect(caughtError).to.containSubset({ locations: [ { line: 2, column: 17 } ], message: - 'Variable "$input" of required type "[String]!" was not provided.' + 'Variable "$input" got invalid value null.\n' + + 'Expected "[String]!", found null.' }); }); @@ -758,7 +812,8 @@ describe('Execute: Handles inputs', () => { expect(caughtError).to.containSubset({ locations: [ { line: 2, column: 17 } ], message: - 'Variable "$input" of required type "[String!]!" was not provided.' + 'Variable "$input" got invalid value null.\n' + + 'Expected "[String!]!", found null.' }); }); @@ -820,7 +875,7 @@ describe('Execute: Handles inputs', () => { } expect(caughtError).to.containSubset({ - locations: [ { line: 2, column: 17 } ], + locations: [ { line: 2, column: 25 } ], message: 'Variable "$input" expected value of type "TestType!" which cannot ' + 'be used as an input type.' @@ -844,7 +899,7 @@ describe('Execute: Handles inputs', () => { } expect(caughtError).to.containSubset({ - locations: [ { line: 2, column: 17 } ], + locations: [ { line: 2, column: 25 } ], message: 'Variable "$input" expected value of type "UnknownType!" which ' + 'cannot be used as an input type.' @@ -867,7 +922,7 @@ describe('Execute: Handles inputs', () => { }); }); - it('when nullable variable provided', async () => { + it('when omitted variable provided', async () => { const ast = parse(`query optionalVariable($optional: String) { fieldWithDefaultArgumentValue(input: $optional) }`); @@ -879,15 +934,22 @@ describe('Execute: Handles inputs', () => { }); }); - it('when argument provided cannot be parsed', async () => { + it('not when argument cannot be coerced', async () => { const ast = parse(`{ fieldWithDefaultArgumentValue(input: WRONG_TYPE) }`); return expect(await execute(schema, ast)).to.deep.equal({ data: { - fieldWithDefaultArgumentValue: '"Hello World"' - } + fieldWithDefaultArgumentValue: null + }, + errors: [ { + message: + 'Argument "input" got invalid value WRONG_TYPE.\n' + + 'Expected type "String", found WRONG_TYPE.', + locations: [ { line: 2, column: 46 } ], + path: [ 'fieldWithDefaultArgumentValue' ] + } ] }); }); diff --git a/src/execution/execute.js b/src/execution/execute.js index ba649b598e..4358d6a844 100644 --- a/src/execution/execute.js +++ b/src/execution/execute.js @@ -458,8 +458,8 @@ function shouldIncludeNode( ); if (skipAST) { const { if: skipIf } = getArgumentValues( - GraphQLSkipDirective.args, - skipAST.arguments, + GraphQLSkipDirective, + skipAST, exeContext.variableValues ); if (skipIf === true) { @@ -473,8 +473,8 @@ function shouldIncludeNode( ); if (includeAST) { const { if: includeIf } = getArgumentValues( - GraphQLIncludeDirective.args, - includeAST.arguments, + GraphQLIncludeDirective, + includeAST, exeContext.variableValues ); if (includeIf === false) { @@ -559,15 +559,6 @@ function resolveField( const returnType = fieldDef.type; const resolveFn = fieldDef.resolve || defaultResolveFn; - // Build a JS object of arguments from the field.arguments AST, using the - // variables scope to fulfill any variable references. - // TODO: find a way to memoize, in case this field is within a List type. - const args = getArgumentValues( - fieldDef.args, - fieldAST.arguments, - exeContext.variableValues - ); - // The resolve function's optional third argument is a context value that // is provided to every resolve function within an execution. It is commonly // used to represent an authenticated user, or request-specific caches. @@ -590,7 +581,15 @@ function resolveField( // Get the resolve function, regardless of if its result is normal // or abrupt (error). - const result = resolveOrError(resolveFn, source, args, context, info); + const result = resolveOrError( + exeContext, + fieldDef, + fieldAST, + resolveFn, + source, + context, + info + ); return completeValueCatchingError( exeContext, @@ -605,13 +604,24 @@ function resolveField( // Isolates the "ReturnOrAbrupt" behavior to not de-opt the `resolveField` // function. Returns the result of resolveFn or the abrupt-return Error object. function resolveOrError( + exeContext: ExecutionContext, + fieldDef: GraphQLFieldDefinition, + fieldAST: Field, resolveFn: GraphQLFieldResolveFn<*>, source: mixed, - args: { [key: string]: mixed }, context: mixed, info: GraphQLResolveInfo ): Error | mixed { try { + // Build a JS object of arguments from the field.arguments AST, using the + // variables scope to fulfill any variable references. + // TODO: find a way to memoize, in case this field is within a List type. + const args = getArgumentValues( + fieldDef, + fieldAST, + exeContext.variableValues + ); + return resolveFn(source, args, context, info); } catch (error) { // Sometimes a non-error is thrown, wrap it as an Error for a diff --git a/src/execution/values.js b/src/execution/values.js index 9f3abdf13e..58eca5d335 100644 --- a/src/execution/values.js +++ b/src/execution/values.js @@ -8,7 +8,7 @@ * of patent rights can be found in the PATENTS file in the same directory. */ -import { forEach, isCollection } from 'iterall'; +import { createIterator, isCollection } from 'iterall'; import { GraphQLError } from '../error'; import invariant from '../jsutils/invariant'; @@ -18,6 +18,8 @@ import keyMap from '../jsutils/keyMap'; 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, @@ -27,9 +29,18 @@ import { GraphQLList, GraphQLNonNull, } from '../type/definition'; -import type { GraphQLInputType, GraphQLArgument } from '../type/definition'; +import type { + GraphQLInputType, + GraphQLFieldDefinition +} from '../type/definition'; +import type { GraphQLDirective } from '../type/directives'; import type { GraphQLSchema } from '../type/schema'; -import type { Argument, VariableDefinition } from '../language/ast'; +import type { + Field, + Directive, + Variable, + VariableDefinition, +} from '../language/ast'; /** @@ -42,84 +53,116 @@ export function getVariableValues( definitionASTs: Array, inputs: { [key: string]: mixed } ): { [key: string]: mixed } { - return definitionASTs.reduce((values, defAST) => { - const varName = defAST.variable.name.value; - values[varName] = getVariableValue(schema, defAST, inputs[varName]); - return values; - }, {}); -} + const coercedValues = Object.create(null); + for (let i = 0; i < definitionASTs.length; i++) { + const definitionAST = definitionASTs[i]; + const varName = definitionAST.variable.name.value; + let varType = typeFromAST(schema, definitionAST.type); + if (!isInputType(varType)) { + throw new GraphQLError( + `Variable "$${varName}" expected value of type ` + + `"${print(definitionAST.type)}" which cannot be used as an input type.`, + [ definitionAST.type ] + ); + } + varType = ((varType: any): GraphQLInputType); + const value = inputs[varName]; + if (isInvalid(value)) { + const defaultValue = definitionAST.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.`, + [ definitionAST ] + ); + } + } 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}`, + [ definitionAST ] + ); + } + + const coercedValue = coerceValue(varType, value); + invariant(!isInvalid(coercedValue), 'Should have reported error.'); + coercedValues[varName] = coercedValue; + } + } + return coercedValues; +} /** * Prepares an object map of argument values given a list of argument * definitions and list of argument AST nodes. */ export function getArgumentValues( - argDefs: ?Array, - argASTs: ?Array, + def: GraphQLFieldDefinition | GraphQLDirective, + node: Field | Directive, variableValues?: ?{ [key: string]: mixed } ): { [key: string]: mixed } { + const argDefs = def.args; + const argASTs = node.arguments; if (!argDefs || !argASTs) { return {}; } + const coercedValues = Object.create(null); const argASTMap = keyMap(argASTs, arg => arg.name.value); - return argDefs.reduce((result, argDef) => { + for (let i = 0; i < argDefs.length; i++) { + const argDef = argDefs[i]; const name = argDef.name; - const valueAST = argASTMap[name] ? argASTMap[name].value : null; - let value = valueFromAST(valueAST, argDef.type, variableValues); - if (isInvalid(value)) { - value = argDef.defaultValue; - } - if (!isInvalid(value)) { - result[name] = value; - } - return result; - }, {}); -} - - -/** - * Given a variable definition, and any value of input, return a value which - * adheres to the variable definition, or throw an error. - */ -function getVariableValue( - schema: GraphQLSchema, - definitionAST: VariableDefinition, - input: mixed -): mixed { - const type = typeFromAST(schema, definitionAST.type); - const variable = definitionAST.variable; - if (!type || !isInputType(type)) { - throw new GraphQLError( - `Variable "$${variable.name.value}" expected value of type ` + - `"${print(definitionAST.type)}" which cannot be used as an input type.`, - [ definitionAST ] - ); - } - const inputType = ((type: any): GraphQLInputType); - const errors = isValidJSValue(input, inputType); - if (!errors.length) { - if (isInvalid(input)) { - const defaultValue = definitionAST.defaultValue; - if (defaultValue) { - return valueFromAST(defaultValue, inputType); + const argType = argDef.type; + const argumentAST = argASTMap[name]; + const defaultValue = argDef.defaultValue; + if (!argumentAST) { + if (!isInvalid(defaultValue)) { + coercedValues[name] = defaultValue; + } else if (argType instanceof GraphQLNonNull) { + throw new GraphQLError( + `Argument "${name}" of required type ` + + `"${String(argType)}" was not provided.`, + [ node ] + ); } + } else if (argumentAST.value.kind === Kind.VARIABLE) { + const variableName = (argumentAST.value: Variable).name.value; + if (variableValues && !isInvalid(variableValues[variableName])) { + // Note: this does not check that this variable value is correct. + // This assumes that this query has been validated and the variable + // usage here is of the correct type. + coercedValues[name] = variableValues[variableName]; + } else if (!isInvalid(defaultValue)) { + coercedValues[name] = defaultValue; + } else if (argType instanceof GraphQLNonNull) { + throw new GraphQLError( + `Argument "${name}" of required type "${String(argType)}" was ` + + `provided the variable "$${variableName}" which was not provided ` + + 'a runtime value.', + [ argumentAST.value ] + ); + } + } else { + const valueAST = argumentAST.value; + const coercedValue = valueFromAST(valueAST, argType, variableValues); + if (isInvalid(coercedValue)) { + const errors = isValidLiteralValue(argType, valueAST); + const message = errors ? '\n' + errors.join('\n') : ''; + throw new GraphQLError( + `Argument "${name}" got invalid value ${print(valueAST)}.${message}`, + [ argumentAST.value ] + ); + } + coercedValues[name] = coercedValue; } - return coerceValue(inputType, input); - } - if (isNullish(input)) { - throw new GraphQLError( - `Variable "$${variable.name.value}" of required type ` + - `"${print(definitionAST.type)}" was not provided.`, - [ definitionAST ] - ); } - const message = errors ? '\n' + errors.join('\n') : ''; - throw new GraphQLError( - `Variable "$${variable.name.value}" got invalid value ` + - `${JSON.stringify(input)}.${message}`, - [ definitionAST ] - ); + return coercedValues; } /** @@ -129,48 +172,72 @@ 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) { - // Note: we're not checking that the result of coerceValue is non-null. - // We only call this function after calling isValidJSValue. + if (_value === null) { + return; // Intentionally return no value. + } return coerceValue(type.ofType, _value); } if (_value === null) { + // Intentionally return the value null. return null; } - if (isInvalid(_value)) { - return undefined; - } - if (type instanceof GraphQLList) { const itemType = type.ofType; if (isCollection(_value)) { const coercedValues = []; - forEach((_value: any), item => { - coercedValues.push(coerceValue(itemType, item)); - }); + 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' || _value === null) { - return null; + if (typeof _value !== 'object') { + return; // Intentionally return no value. } + const coercedObj = Object.create(null); const fields = type.getFields(); - return Object.keys(fields).reduce((obj, fieldName) => { + const fieldNames = Object.keys(fields); + for (let i = 0; i < fieldNames.length; i++) { + const fieldName = fieldNames[i]; const field = fields[fieldName]; - let fieldValue = coerceValue(field.type, _value[fieldName]); - if (isInvalid(fieldValue)) { - fieldValue = field.defaultValue; + if (isInvalid(_value[fieldName])) { + if (!isInvalid(field.defaultValue)) { + coercedObj[fieldName] = field.defaultValue; + } else if (field.type instanceof GraphQLNonNull) { + return; // Intentionally return no value. + } + continue; } - if (!isInvalid(fieldValue)) { - obj[fieldName] = fieldValue; + const fieldValue = coerceValue(field.type, _value[fieldName]); + if (isInvalid(fieldValue)) { + return; // Intentionally return no value. } - return obj; - }, {}); + coercedObj[fieldName] = fieldValue; + } + return coercedObj; } invariant( @@ -179,7 +246,11 @@ function coerceValue(type: GraphQLInputType, value: mixed): mixed { ); const parsed = type.parseValue(_value); - if (!isNullish(parsed)) { - return parsed; + 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/utilities/__tests__/valueFromAST-test.js b/src/utilities/__tests__/valueFromAST-test.js new file mode 100644 index 0000000000..56ee07ab65 --- /dev/null +++ b/src/utilities/__tests__/valueFromAST-test.js @@ -0,0 +1,198 @@ +/** + * Copyright (c) 2015, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ + +import { describe, it } from 'mocha'; +import { expect } from 'chai'; +import { valueFromAST } from '../valueFromAST'; +import { + GraphQLEnumType, + GraphQLInputObjectType, + GraphQLList, + GraphQLInt, + GraphQLFloat, + GraphQLString, + GraphQLBoolean, + GraphQLID, + GraphQLNonNull, +} from '../../type'; +import { parseValue } from '../../language'; + + +describe('valueFromAST', () => { + + function testCase(type, valueText, expected) { + expect( + valueFromAST(parseValue(valueText), type) + ).to.deep.equal(expected); + } + + function testCaseWithVars(variables, type, valueText, expected) { + expect( + valueFromAST(parseValue(valueText), type, variables) + ).to.deep.equal(expected); + } + + it('rejects empty input', () => { + expect(valueFromAST(null, GraphQLBoolean)).to.deep.equal(undefined); + }); + + it('converts according to input coercion rules', () => { + testCase(GraphQLBoolean, 'true', true); + testCase(GraphQLBoolean, 'false', false); + testCase(GraphQLInt, '123', 123); + testCase(GraphQLFloat, '123', 123); + testCase(GraphQLFloat, '123.456', 123.456); + testCase(GraphQLString, '"abc123"', 'abc123'); + testCase(GraphQLID, '123456', '123456'); + testCase(GraphQLID, '"123456"', '123456'); + }); + + it('does not convert when input coercion rules reject a value', () => { + testCase(GraphQLBoolean, '123', undefined); + testCase(GraphQLInt, '123.456', undefined); + testCase(GraphQLInt, 'true', undefined); + testCase(GraphQLInt, '"123"', undefined); + testCase(GraphQLFloat, '"123"', undefined); + testCase(GraphQLString, '123', undefined); + testCase(GraphQLString, 'true', undefined); + testCase(GraphQLID, '123.456', undefined); + }); + + const testEnum = new GraphQLEnumType({ + name: 'TestColor', + values: { RED: { value: 1 }, GREEN: { value: 2 }, BLUE: { value: 3 } } + }); + + it('converts enum values according to input coercion rules', () => { + testCase(testEnum, 'RED', 1); + testCase(testEnum, 'BLUE', 3); + testCase(testEnum, '3', undefined); + testCase(testEnum, '"BLUE"', undefined); + testCase(testEnum, 'null', null); + }); + + // Boolean! + const nonNullBool = new GraphQLNonNull(GraphQLBoolean); + // [Boolean] + const listOfBool = new GraphQLList(GraphQLBoolean); + // [Boolean!] + const listOfNonNullBool = new GraphQLList(nonNullBool); + // [Boolean]! + const nonNullListOfBool = new GraphQLNonNull(listOfBool); + // [Boolean!]! + const nonNullListOfNonNullBool = new GraphQLNonNull(listOfNonNullBool); + + it('coerces to null unless non-null', () => { + testCase(GraphQLBoolean, 'null', null); + testCase(nonNullBool, 'null', undefined); + }); + + it('coerces lists of values', () => { + testCase(listOfBool, 'true', [ true ]); + testCase(listOfBool, '123', undefined); + testCase(listOfBool, 'null', null); + testCase(listOfBool, '[true, false]', [ true, false ]); + testCase(listOfBool, '[true, 123]', undefined); + testCase(listOfBool, '[true, null]', [ true, null ]); + testCase(listOfBool, '{ true: true }', undefined); + }); + + it('coerces non-null lists of values', () => { + testCase(nonNullListOfBool, 'true', [ true ]); + testCase(nonNullListOfBool, '123', undefined); + testCase(nonNullListOfBool, 'null', undefined); + testCase(nonNullListOfBool, '[true, false]', [ true, false ]); + testCase(nonNullListOfBool, '[true, 123]', undefined); + testCase(nonNullListOfBool, '[true, null]', [ true, null ]); + }); + + it('coerces lists of non-null values', () => { + testCase(listOfNonNullBool, 'true', [ true ]); + testCase(listOfNonNullBool, '123', undefined); + testCase(listOfNonNullBool, 'null', null); + testCase(listOfNonNullBool, '[true, false]', [ true, false ]); + testCase(listOfNonNullBool, '[true, 123]', undefined); + testCase(listOfNonNullBool, '[true, null]', undefined); + }); + + it('coerces non-null lists of non-null values', () => { + testCase(nonNullListOfNonNullBool, 'true', [ true ]); + testCase(nonNullListOfNonNullBool, '123', undefined); + testCase(nonNullListOfNonNullBool, 'null', undefined); + testCase(nonNullListOfNonNullBool, '[true, false]', [ true, false ]); + testCase(nonNullListOfNonNullBool, '[true, 123]', undefined); + testCase(nonNullListOfNonNullBool, '[true, null]', undefined); + }); + + const testInputObj = new GraphQLInputObjectType({ + name: 'TestInput', + fields: { + int: { type: GraphQLInt, defaultValue: 42 }, + bool: { type: GraphQLBoolean }, + requiredBool: { type: nonNullBool }, + } + }); + + it('coerces input objects according to input coercion rules', () => { + testCase(testInputObj, 'null', null); + testCase(testInputObj, '123', undefined); + testCase(testInputObj, '[]', undefined); + testCase( + testInputObj, + '{ int: 123, requiredBool: false }', + { int: 123, requiredBool: false } + ); + testCase( + testInputObj, + '{ bool: true, requiredBool: false }', + { int: 42, bool: true, requiredBool: false } + ); + testCase(testInputObj, '{ int: true, requiredBool: true }', undefined); + testCase(testInputObj, '{ requiredBool: null }', undefined); + testCase(testInputObj, '{ bool: true }', undefined); + }); + + it('accepts variable values assuming already coerced', () => { + testCaseWithVars({}, GraphQLBoolean, '$var', undefined); + testCaseWithVars({ var: true }, GraphQLBoolean, '$var', true); + testCaseWithVars({ var: null }, GraphQLBoolean, '$var', null); + }); + + it('asserts variables are provided as items in lists', () => { + testCaseWithVars({}, listOfBool, '[ $foo ]', [ null ]); + testCaseWithVars({}, listOfNonNullBool, '[ $foo ]', undefined); + testCaseWithVars({ foo: true }, listOfNonNullBool, '[ $foo ]', [ true ]); + // Note: variables are expected to have already been coerced, so we + // do not expect the singleton wrapping behavior for variables. + testCaseWithVars({ foo: true }, listOfNonNullBool, '$foo', true); + testCaseWithVars({ foo: [ true ] }, listOfNonNullBool, '$foo', [ true ]); + }); + + it('omits input object fields for unprovided variables', () => { + testCaseWithVars( + {}, + testInputObj, + '{ int: $foo, bool: $foo, requiredBool: true }', + { int: 42, requiredBool: true } + ); + testCaseWithVars( + {}, + testInputObj, + '{ requiredBool: $foo }', + undefined + ); + testCaseWithVars( + { foo: true }, + testInputObj, + '{ requiredBool: $foo }', + { int: 42, requiredBool: true } + ); + }); + +}); diff --git a/src/utilities/buildASTSchema.js b/src/utilities/buildASTSchema.js index 909072da1f..3f2de8b3fe 100644 --- a/src/utilities/buildASTSchema.js +++ b/src/utilities/buildASTSchema.js @@ -469,8 +469,8 @@ function getDeprecationReason(directives: ?Array): ?string { return; } const { reason } = getArgumentValues( - GraphQLDeprecatedDirective.args, - deprecatedAST.arguments + GraphQLDeprecatedDirective, + deprecatedAST ); return (reason: any); } diff --git a/src/utilities/isValidJSValue.js b/src/utilities/isValidJSValue.js index 33ebd51641..880cb721ab 100644 --- a/src/utilities/isValidJSValue.js +++ b/src/utilities/isValidJSValue.js @@ -34,10 +34,7 @@ export function isValidJSValue( // A value must be provided if the type is non-null. if (type instanceof GraphQLNonNull) { if (isNullish(value)) { - if (type.ofType.name) { - return [ `Expected "${String(type.ofType.name)}!", found null.` ]; - } - return [ 'Expected non-null value, found null.' ]; + return [ `Expected "${String(type)}", found null.` ]; } return isValidJSValue(value, type.ofType); } diff --git a/src/utilities/isValidLiteralValue.js b/src/utilities/isValidLiteralValue.js index 96a74baaec..64ad7e4bc2 100644 --- a/src/utilities/isValidLiteralValue.js +++ b/src/utilities/isValidLiteralValue.js @@ -43,10 +43,7 @@ export function isValidLiteralValue( // A value must be provided if the type is non-null. if (type instanceof GraphQLNonNull) { if (!valueAST || (valueAST.kind === NULL)) { - if (type.ofType.name) { - return [ `Expected "${String(type.ofType.name)}!", found null.` ]; - } - return [ 'Expected non-null value, found null.' ]; + return [ `Expected "${String(type)}", found null.` ]; } return isValidLiteralValue(type.ofType, valueAST); } diff --git a/src/utilities/valueFromAST.js b/src/utilities/valueFromAST.js index 3d216a30da..73ee90ef8c 100644 --- a/src/utilities/valueFromAST.js +++ b/src/utilities/valueFromAST.js @@ -35,6 +35,9 @@ import type { * A GraphQL type must be provided, which will be used to interpret different * GraphQL Value literals. * + * Returns `undefined` when the value could not be validly coerced according to + * the provided type. + * * | GraphQL Value | JSON Value | * | -------------------- | ------------- | * | Input Object | Object | @@ -50,20 +53,20 @@ export function valueFromAST( valueAST: ?Value, type: GraphQLInputType, variables?: ?{ [key: string]: mixed } -): mixed { - if (type instanceof GraphQLNonNull) { - // Note: we're not checking that the result of valueFromAST is non-null. - // We're assuming that this query has been validated and the value used - // here is of the correct type. - return valueFromAST(valueAST, type.ofType, variables); - } - +): mixed | void { if (!valueAST) { // When there is no AST, then there is also no value. // Importantly, this is different from returning the value null. return; } + if (type instanceof GraphQLNonNull) { + if (valueAST.kind === Kind.NULL) { + return; // Invalid: intentionally return no value. + } + return valueFromAST(valueAST, type.ofType, variables); + } + if (valueAST.kind === Kind.NULL) { // This is explicitly returning the value null. return null; @@ -71,7 +74,7 @@ export function valueFromAST( if (valueAST.kind === Kind.VARIABLE) { const variableName = (valueAST: Variable).name.value; - if (!variables || !variables.hasOwnProperty(variableName)) { + if (!variables || isInvalid(variables[variableName])) { // No valid return value. return; } @@ -84,33 +87,63 @@ export function valueFromAST( if (type instanceof GraphQLList) { const itemType = type.ofType; if (valueAST.kind === Kind.LIST) { - return (valueAST: ListValue).values.map( - itemAST => valueFromAST(itemAST, itemType, variables) - ); + const coercedValues = []; + const itemASTs = (valueAST: ListValue).values; + for (let i = 0; i < itemASTs.length; i++) { + if (isMissingVariable(itemASTs[i], variables)) { + // If an array contains a missing variable, it is either coerced to + // null or if the item type is non-null, it considered invalid. + if (itemType instanceof GraphQLNonNull) { + return; // Invalid: intentionally return no value. + } + coercedValues.push(null); + } else { + const itemValue = valueFromAST(itemASTs[i], itemType, variables); + if (isInvalid(itemValue)) { + return; // Invalid: intentionally return no value. + } + coercedValues.push(itemValue); + } + } + return coercedValues; } - return [ valueFromAST(valueAST, itemType, variables) ]; + const coercedValue = valueFromAST(valueAST, itemType, variables); + if (isInvalid(coercedValue)) { + return; // Invalid: intentionally return no value. + } + return [ coercedValue ]; } if (type instanceof GraphQLInputObjectType) { if (valueAST.kind !== Kind.OBJECT) { - // No valid return value. - return; + return; // Invalid: intentionally return no value. } + const coercedObj = Object.create(null); const fields = type.getFields(); const fieldASTs = keyMap( (valueAST: ObjectValue).fields, field => field.name.value ); - return Object.keys(fields).reduce((obj, fieldName) => { + const fieldNames = Object.keys(fields); + for (let i = 0; i < fieldNames.length; i++) { + const fieldName = fieldNames[i]; const field = fields[fieldName]; const fieldAST = fieldASTs[fieldName]; - const fieldValue = - valueFromAST(fieldAST && fieldAST.value, field.type, variables); - - // If no valid field value was provided, use the default value - obj[fieldName] = isInvalid(fieldValue) ? field.defaultValue : fieldValue; - return obj; - }, {}); + if (!fieldAST || isMissingVariable(fieldAST.value, variables)) { + if (!isInvalid(field.defaultValue)) { + coercedObj[fieldName] = field.defaultValue; + } else if (field.type instanceof GraphQLNonNull) { + return; // Invalid: intentionally return no value. + } + continue; + } + const fieldValue = valueFromAST(fieldAST.value, field.type, variables); + if (isInvalid(fieldValue)) { + return; // Invalid: intentionally return no value. + } + coercedObj[fieldName] = fieldValue; + } + return coercedObj; } invariant( @@ -119,7 +152,18 @@ export function valueFromAST( ); const parsed = type.parseLiteral(valueAST); - if (!isNullish(parsed)) { - return parsed; + if (isNullish(parsed)) { + // null or invalid values represent a failure to parse correctly, + // in which case no value is returned. + return; } + + return parsed; +} + +// Returns true if the provided valueAST is a variable which is not defined +// in the set of variables. +function isMissingVariable(valueAST, variables) { + return valueAST.kind === Kind.VARIABLE && + (!variables || isInvalid(variables[(valueAST: Variable).name.value])); }