Skip to content

Commit f7f624e

Browse files
leebyronleebyron
committed
Input Value Validation
Depends on #3065 Factors out input validation to reusable functions: * Introduces `validateInputLiteral` by extracting this behavior from `ValuesOfCorrectTypeRule`. * Introduces `validateInputValue` by extracting this behavior from `coerceInputValue` * Simplifies `coerceInputValue` to return early on validation error * Unifies error reporting between `validateInputValue` and `validateInputLiteral`, causing some error message strings to change, but error data (eg locations) are preserved. These two parallel functions will be used to validate default values in #3049
1 parent 01652d0 commit f7f624e

File tree

18 files changed

+1493
-659
lines changed

18 files changed

+1493
-659
lines changed

‎src/execution/__tests__/nonnull-test.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -643,7 +643,7 @@ describe('Execute: handles non-nullable types', () => {
643643
errors: [
644644
{
645645
message:
646-
'Argument "cannotBeNull" of non-null type "String!" must not be null.',
646+
'Argument "cannotBeNull" has invalid value: Expected value of non-null type "String!" not to be null.',
647647
locations: [{ line: 3, column: 42 }],
648648
path: ['withNonNullArg'],
649649
},
@@ -673,7 +673,7 @@ describe('Execute: handles non-nullable types', () => {
673673
errors: [
674674
{
675675
message:
676-
'Argument "cannotBeNull" of required type "String!" was provided the variable "$testVar" which was not provided a runtime value.',
676+
'Argument "cannotBeNull" has invalid value: Expected variable "$testVar" provided to type "String!" to provide a runtime value.',
677677
locations: [{ line: 3, column: 42 }],
678678
path: ['withNonNullArg'],
679679
},
@@ -701,7 +701,7 @@ describe('Execute: handles non-nullable types', () => {
701701
errors: [
702702
{
703703
message:
704-
'Argument "cannotBeNull" of non-null type "String!" must not be null.',
704+
'Argument "cannotBeNull" has invalid value: Expected variable "$testVar" provided to non-null type "String!" not to be null.',
705705
locations: [{ line: 3, column: 43 }],
706706
path: ['withNonNullArg'],
707707
},

‎src/execution/__tests__/variables-test.js‎

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ describe('Execute: Handles inputs', () => {
198198
errors: [
199199
{
200200
message:
201-
'Argument "input" has invalid value ["foo", "bar", "baz"].',
201+
'Argument "input" has invalid value: Expected value of type "TestInputObject" to be an object, found ["foo", "bar", "baz"].',
202202
path: ['fieldWithObjectInput'],
203203
locations: [{ line: 3, column: 41 }],
204204
},
@@ -368,7 +368,7 @@ describe('Execute: Handles inputs', () => {
368368
errors: [
369369
{
370370
message:
371-
'Variable "$input" got invalid value null at "input.c"; Expected non-nullable type "String!" not to be null.',
371+
'Variable "$input" has invalid value at .c: Expected value of non-null type "String!" not to be null.',
372372
locations: [{ line: 2, column: 16 }],
373373
},
374374
],
@@ -382,7 +382,7 @@ describe('Execute: Handles inputs', () => {
382382
errors: [
383383
{
384384
message:
385-
'Variable "$input" got invalid value "foo bar"; Expected type "TestInputObject" to be an object.',
385+
'Variable "$input" has invalid value: Expected value of type "TestInputObject" to be an object, found "foo bar".',
386386
locations: [{ line: 2, column: 16 }],
387387
},
388388
],
@@ -396,7 +396,7 @@ describe('Execute: Handles inputs', () => {
396396
errors: [
397397
{
398398
message:
399-
'Variable "$input" got invalid value { a: "foo", b: "bar" }; Field "c" of required type "String!" was not provided.',
399+
'Variable "$input" has invalid value: Expected value of type "TestInputObject" to include required field "c", found { a: "foo", b: "bar" }',
400400
locations: [{ line: 2, column: 16 }],
401401
},
402402
],
@@ -415,12 +415,12 @@ describe('Execute: Handles inputs', () => {
415415
errors: [
416416
{
417417
message:
418-
'Variable "$input" got invalid value { a: "foo" } at "input.na"; Field "c" of required type "String!" was not provided.',
418+
'Variable "$input" has invalid value at .na: Expected value of type "TestInputObject" to include required field "c", found { a: "foo" }',
419419
locations: [{ line: 2, column: 18 }],
420420
},
421421
{
422422
message:
423-
'Variable "$input" got invalid value { na: { a: "foo" } }; Field "nb" of required type "String!" was not provided.',
423+
'Variable "$input" has invalid value: Expected value of type "TestNestedInputObject" to include required field "nb", found { na: { a: "foo" } }',
424424
locations: [{ line: 2, column: 18 }],
425425
},
426426
],
@@ -437,7 +437,7 @@ describe('Execute: Handles inputs', () => {
437437
errors: [
438438
{
439439
message:
440-
'Variable "$input" got invalid value { a: "foo", b: "bar", c: "baz", extra: "dog" }; Field "extra" is not defined by type "TestInputObject".',
440+
'Variable "$input" has invalid value: Expected value of type "TestInputObject" not to include unknown field "extra", found { a: "foo", b: "bar", c: "baz", extra: "dog" }',
441441
locations: [{ line: 2, column: 16 }],
442442
},
443443
],
@@ -612,7 +612,7 @@ describe('Execute: Handles inputs', () => {
612612
errors: [
613613
{
614614
message:
615-
'Variable "$value" of required type "String!" was not provided.',
615+
'Variable "$value" has invalid value: Expected a value of non-null type "String!" to be provided.',
616616
locations: [{ line: 2, column: 16 }],
617617
},
618618
],
@@ -631,7 +631,7 @@ describe('Execute: Handles inputs', () => {
631631
errors: [
632632
{
633633
message:
634-
'Variable "$value" of non-null type "String!" must not be null.',
634+
'Variable "$value" has invalid value: Expected value of non-null type "String!" not to be null.',
635635
locations: [{ line: 2, column: 16 }],
636636
},
637637
],
@@ -697,7 +697,7 @@ describe('Execute: Handles inputs', () => {
697697
errors: [
698698
{
699699
message:
700-
'Variable "$value" got invalid value [1, 2, 3]; String cannot represent a non string value: [1, 2, 3]',
700+
'Variable "$value" has invalid value: String cannot represent a non string value: [1, 2, 3]',
701701
locations: [{ line: 2, column: 16 }],
702702
},
703703
],
@@ -725,7 +725,7 @@ describe('Execute: Handles inputs', () => {
725725
errors: [
726726
{
727727
message:
728-
'Argument "input" of required type "String!" was provided the variable "$foo" which was not provided a runtime value.',
728+
'Argument "input" has invalid value: Expected variable "$foo" provided to type "String!" to provide a runtime value.',
729729
locations: [{ line: 3, column: 50 }],
730730
path: ['fieldWithNonNullableStringInput'],
731731
},
@@ -780,7 +780,7 @@ describe('Execute: Handles inputs', () => {
780780
errors: [
781781
{
782782
message:
783-
'Variable "$input" of non-null type "[String]!" must not be null.',
783+
'Variable "$input" has invalid value: Expected value of non-null type "[String]!" not to be null.',
784784
locations: [{ line: 2, column: 16 }],
785785
},
786786
],
@@ -843,7 +843,7 @@ describe('Execute: Handles inputs', () => {
843843
errors: [
844844
{
845845
message:
846-
'Variable "$input" got invalid value null at "input[1]"; Expected non-nullable type "String!" not to be null.',
846+
'Variable "$input" has invalid value at [1]: Expected value of non-null type "String!" not to be null.',
847847
locations: [{ line: 2, column: 16 }],
848848
},
849849
],
@@ -862,7 +862,7 @@ describe('Execute: Handles inputs', () => {
862862
errors: [
863863
{
864864
message:
865-
'Variable "$input" of non-null type "[String!]!" must not be null.',
865+
'Variable "$input" has invalid value: Expected value of non-null type "[String!]!" not to be null.',
866866
locations: [{ line: 2, column: 16 }],
867867
},
868868
],
@@ -892,7 +892,7 @@ describe('Execute: Handles inputs', () => {
892892
errors: [
893893
{
894894
message:
895-
'Variable "$input" got invalid value null at "input[1]"; Expected non-nullable type "String!" not to be null.',
895+
'Variable "$input" has invalid value at [1]: Expected value of non-null type "String!" not to be null.',
896896
locations: [{ line: 2, column: 16 }],
897897
},
898898
],
@@ -976,7 +976,8 @@ describe('Execute: Handles inputs', () => {
976976
},
977977
errors: [
978978
{
979-
message: 'Argument "input" has invalid value WRONG_TYPE.',
979+
message:
980+
'Argument "input" has invalid value: String cannot represent a non string value: WRONG_TYPE',
980981
locations: [{ line: 3, column: 48 }],
981982
path: ['fieldWithDefaultArgumentValue'],
982983
},
@@ -1016,7 +1017,7 @@ describe('Execute: Handles inputs', () => {
10161017

10171018
function invalidValueError(value: number, index: number) {
10181019
return {
1019-
message: `Variable "$input" got invalid value ${value} at "input[${index}]"; String cannot represent a non string value: ${value}`,
1020+
message: `Variable "$input" has invalid value at [${index}]: String cannot represent a non string value: ${value}`,
10201021
locations: [{ line: 2, column: 14 }],
10211022
};
10221023
}

‎src/execution/values.js‎

Lines changed: 62 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { ReadOnlyObjMap, ReadOnlyObjMapLike } from '../jsutils/ObjMap';
2-
import { inspect } from '../jsutils/inspect';
2+
import { hasOwnProperty } from '../jsutils/hasOwnProperty';
3+
import { invariant } from '../jsutils/invariant';
34
import { keyMap } from '../jsutils/keyMap';
45
import { printPathArray } from '../jsutils/printPathArray';
56

@@ -16,14 +17,22 @@ import { print } from '../language/printer';
1617
import type { GraphQLSchema } from '../type/schema';
1718
import type { GraphQLInputType, GraphQLField } from '../type/definition';
1819
import type { GraphQLDirective } from '../type/directives';
19-
import { isInputType, isNonNullType } from '../type/definition';
20+
import {
21+
isInputType,
22+
isNonNullType,
23+
isRequiredInput,
24+
} from '../type/definition';
2025

2126
import { typeFromAST } from '../utilities/typeFromAST';
2227
import {
2328
coerceInputValue,
2429
coerceInputLiteral,
2530
coerceDefaultValue,
2631
} from '../utilities/coerceInputValue';
32+
import {
33+
validateInputValue,
34+
validateInputLiteral,
35+
} from '../utilities/validateInputValue';
2736

2837
export type VariableValues = {|
2938
+sources: ReadOnlyObjMap<{|
@@ -106,61 +115,38 @@ function coerceVariableValues(
106115
continue;
107116
}
108117

109-
if (!hasOwnProperty(inputs, varName)) {
110-
const defaultValue = varDefNode.defaultValue;
111-
if (defaultValue) {
112-
sources[varName] = {
113-
variable: varDefNode,
114-
type: varType,
115-
value: undefined,
116-
};
117-
coerced[varName] = coerceInputLiteral(defaultValue, varType);
118-
} else if (isNonNullType(varType)) {
119-
const varTypeStr = inspect(varType);
120-
onError(
121-
new GraphQLError(
122-
`Variable "$${varName}" of required type "${varTypeStr}" was not provided.`,
123-
varDefNode,
124-
),
125-
);
126-
}
127-
continue;
128-
}
118+
const value = hasOwnProperty(inputs, varName) ? inputs[varName] : undefined;
119+
sources[varName] = { variable: varDefNode, type: varType, value };
129120

130-
const value = inputs[varName];
131-
if (value === null && isNonNullType(varType)) {
132-
const varTypeStr = inspect(varType);
133-
onError(
134-
new GraphQLError(
135-
`Variable "$${varName}" of non-null type "${varTypeStr}" must not be null.`,
136-
varDefNode,
137-
),
138-
);
139-
continue;
121+
if (value === undefined) {
122+
if (varDefNode.defaultValue) {
123+
coerced[varName] = coerceInputLiteral(varDefNode.defaultValue, varType);
124+
continue;
125+
} else if (!isNonNullType(varType)) {
126+
// Non-provided values for nullable variables are omitted.
127+
continue;
128+
}
140129
}
141130

142-
sources[varName] = { variable: varDefNode, type: varType, value };
143-
coerced[varName] = coerceInputValue(
144-
value,
145-
varType,
146-
(path, invalidValue, error) => {
147-
let prefix =
148-
`Variable "$${varName}" got invalid value ` + inspect(invalidValue);
149-
if (path.length > 0) {
150-
prefix += ` at "${varName}${printPathArray(path)}"`;
151-
}
131+
const coercedValue = coerceInputValue(value, varType);
132+
if (coercedValue !== undefined) {
133+
coerced[varName] = coercedValue;
134+
} else {
135+
validateInputValue(value, varType, (error, path) => {
152136
onError(
153137
new GraphQLError(
154-
prefix + '; ' + error.message,
138+
`Variable "$${varName}" has invalid value${printPathArray(path)}: ${
139+
error.message
140+
}`,
155141
varDefNode,
156142
undefined,
157143
undefined,
158144
undefined,
159145
error.originalError,
160146
),
161147
);
162-
},
163-
);
148+
});
149+
}
164150
}
165151

166152
return { sources, coerced };
@@ -192,65 +178,54 @@ export function getArgumentValues(
192178
const argType = argDef.type;
193179
const argumentNode = argNodeMap[name];
194180

195-
if (!argumentNode) {
181+
if (!argumentNode && isRequiredInput(argDef)) {
182+
// Note: ProvidedRequiredArgumentsRule validation should catch this before
183+
// execution. This is a runtime check to ensure execution does not
184+
// continue with an invalid argument value.
185+
throw new GraphQLError(
186+
`Argument "${name}" of required type "${String(
187+
argType,
188+
)}" was not provided.`,
189+
node,
190+
);
191+
}
192+
193+
// Variables without a value are treated as if no argument was provided if
194+
// the argument is not required.
195+
if (
196+
!argumentNode ||
197+
(argumentNode.value.kind === Kind.VARIABLE &&
198+
variableValues?.coerced[argumentNode.value.name.value] === undefined &&
199+
!isRequiredInput(argDef))
200+
) {
196201
if (argDef.defaultValue) {
197202
coercedValues[name] = coerceDefaultValue(
198203
argDef.defaultValue,
199204
argDef.type,
200205
);
201-
} else if (isNonNullType(argType)) {
202-
throw new GraphQLError(
203-
`Argument "${name}" of required type "${inspect(argType)}" ` +
204-
'was not provided.',
205-
node,
206-
);
207206
}
208207
continue;
209208
}
210209

211210
const valueNode = argumentNode.value;
212-
let isNull = valueNode.kind === Kind.NULL;
213-
214-
if (valueNode.kind === Kind.VARIABLE) {
215-
const variableName = valueNode.name.value;
216-
if (
217-
variableValues == null ||
218-
variableValues.coerced[variableName] === undefined
219-
) {
220-
if (argDef.defaultValue) {
221-
coercedValues[name] = coerceDefaultValue(
222-
argDef.defaultValue,
223-
argDef.type,
224-
);
225-
} else if (isNonNullType(argType)) {
226-
throw new GraphQLError(
227-
`Argument "${name}" of required type "${inspect(argType)}" ` +
228-
`was provided the variable "$${variableName}" which was not provided a runtime value.`,
229-
valueNode,
230-
);
231-
}
232-
continue;
233-
}
234-
isNull = variableValues.coerced[variableName] == null;
235-
}
236-
237-
if (isNull && isNonNullType(argType)) {
238-
throw new GraphQLError(
239-
`Argument "${name}" of non-null type "${inspect(argType)}" ` +
240-
'must not be null.',
241-
valueNode,
242-
);
243-
}
244-
245211
const coercedValue = coerceInputLiteral(valueNode, argType, variableValues);
246212
if (coercedValue === undefined) {
247213
// Note: ValuesOfCorrectTypeRule validation should catch this before
248214
// execution. This is a runtime check to ensure execution does not
249215
// continue with an invalid argument value.
250-
throw new GraphQLError(
251-
`Argument "${name}" has invalid value ${print(valueNode)}.`,
216+
validateInputLiteral(
252217
valueNode,
218+
argType,
219+
variableValues,
220+
(error, path) => {
221+
error.message = `Argument "${name}" has invalid value${printPathArray(
222+
path,
223+
)}: ${error.message}`;
224+
throw error;
225+
},
253226
);
227+
// istanbul ignore next (validateInputLiteral should throw)
228+
invariant(false, 'Invalid argument');
254229
}
255230
coercedValues[name] = coercedValue;
256231
}
@@ -282,7 +257,3 @@ export function getDirectiveValues(
282257
return getArgumentValues(directiveDef, directiveNode, variableValues);
283258
}
284259
}
285-
286-
function hasOwnProperty(obj: mixed, prop: string): boolean {
287-
return Object.prototype.hasOwnProperty.call(obj, prop);
288-
}

0 commit comments

Comments
 (0)