Skip to content

Commit

Permalink
Input Value Validation (#3813)
Browse files Browse the repository at this point in the history 
[#3086 rebased on
main](#3086).

Depends on #3812 

@leebyron comments from original PR:

> 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
> 
> Potentially breaking if you rely on the existing behavior of
`coerceInputValue` to call a callback function, as the call signature
has changed. GraphQL behavior should not change, though error messages
are now slightly different.

Note: also breaking if you rely on the default callback function to
throw. Grossly similar behavior is available with
`validateInputValue()`.

Co-authored-by: Lee Byron <lee.byron@robinhood.com>
  • Loading branch information
@yaacovCR @leebyron
yaacovCR and leebyron authored Oct 18, 2024
1 parent d77d661 commit beff1d5
Show file tree
Hide file tree
Showing 16 changed files with 1,884 additions and 921 deletions.
6 changes: 3 additions & 3 deletions src/execution/__tests__/nonnull-test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -647,7 +647,7 @@ describe('Execute: handles non-nullable types', () => {
errors: [
{
message:
'Argument "cannotBeNull" of non-null type "String!" must not be null.',
'Argument "cannotBeNull" has invalid value: Expected value of non-null type "String!" not to be null.',
locations: [{ line: 3, column: 42 }],
path: ['withNonNullArg'],
},
Expand Down Expand Up @@ -677,7 +677,7 @@ describe('Execute: handles non-nullable types', () => {
errors: [
{
message:
'Argument "cannotBeNull" of required type "String!" was provided the variable "$testVar" which was not provided a runtime value.',
'Argument "cannotBeNull" has invalid value: Expected variable "$testVar" provided to type "String!" to provide a runtime value.',
locations: [{ line: 3, column: 42 }],
path: ['withNonNullArg'],
},
Expand Down Expand Up @@ -705,7 +705,7 @@ describe('Execute: handles non-nullable types', () => {
errors: [
{
message:
'Argument "cannotBeNull" of non-null type "String!" must not be null.',
'Argument "cannotBeNull" has invalid value: Expected variable "$testVar" provided to non-null type "String!" not to be null.',
locations: [{ line: 3, column: 43 }],
path: ['withNonNullArg'],
},
Expand Down
153 changes: 149 additions & 4 deletions src/execution/__tests__/oneof-test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,12 @@ function executeQuery(
rootValue: unknown,
variableValues?: { [variable: string]: unknown },
): ExecutionResult | Promise<ExecutionResult> {
return execute({ schema, document: parse(query), rootValue, variableValues });
return execute({
schema,
document: parse(query, { experimentalFragmentArguments: true }),
rootValue,
variableValues,
});
}

describe('Execute: Handles OneOf Input Objects', () => {
Expand Down Expand Up @@ -83,7 +88,7 @@ describe('Execute: Handles OneOf Input Objects', () => {
message:
// This type of error would be caught at validation-time
// hence the vague error message here.
'Argument "input" of non-null type "TestInputObject!" must not be null.',
'Argument "input" has invalid value: Expected variable "$input" provided to type "TestInputObject!" to provide a runtime value.',
path: ['test'],
},
],
Expand Down Expand Up @@ -134,6 +139,28 @@ describe('Execute: Handles OneOf Input Objects', () => {
});
});

it('rejects a variable with a nulled key', () => {
const query = `
query ($input: TestInputObject!) {
test(input: $input) {
a
b
}
}
`;
const result = executeQuery(query, rootValue, { input: { a: null } });

expectJSON(result).toDeepEqual({
errors: [
{
message:
'Variable "$input" has invalid value: Field "a" for OneOf type "TestInputObject" must be non-null.',
locations: [{ line: 2, column: 16 }],
},
],
});
});

it('rejects a variable with multiple non-null keys', () => {
const query = `
query ($input: TestInputObject!) {
Expand All @@ -152,7 +179,7 @@ describe('Execute: Handles OneOf Input Objects', () => {
{
locations: [{ column: 16, line: 2 }],
message:
'Variable "$input" got invalid value { a: "abc", b: 123 }; Exactly one key must be specified for OneOf type "TestInputObject".',
'Variable "$input" has invalid value: Exactly one key must be specified for OneOf type "TestInputObject".',
},
],
});
Expand All @@ -176,7 +203,125 @@ describe('Execute: Handles OneOf Input Objects', () => {
{
locations: [{ column: 16, line: 2 }],
message:
'Variable "$input" got invalid value { a: "abc", b: null }; Exactly one key must be specified for OneOf type "TestInputObject".',
'Variable "$input" has invalid value: Exactly one key must be specified for OneOf type "TestInputObject".',
},
],
});
});

it('errors with nulled variable for field', () => {
const query = `
query ($a: String) {
test(input: { a: $a }) {
a
b
}
}
`;
const result = executeQuery(query, rootValue, { a: null });

expectJSON(result).toDeepEqual({
data: {
test: null,
},
errors: [
{
// A nullable variable in a oneOf field position would be caught at validation-time
// hence the vague error message here.
message:
'Argument "input" has invalid value: Expected variable "$a" provided to field "a" for OneOf Input Object type "TestInputObject" not to be null.',
locations: [{ line: 3, column: 23 }],
path: ['test'],
},
],
});
});

it('errors with missing variable for field', () => {
const query = `
query ($a: String) {
test(input: { a: $a }) {
a
b
}
}
`;
const result = executeQuery(query, rootValue);

expectJSON(result).toDeepEqual({
data: {
test: null,
},
errors: [
{
// A nullable variable in a oneOf field position would be caught at validation-time
// hence the vague error message here.
message:
'Argument "input" has invalid value: Expected variable "$a" provided to field "a" for OneOf Input Object type "TestInputObject" to provide a runtime value.',
locations: [{ line: 3, column: 23 }],
path: ['test'],
},
],
});
});

it('errors with nulled fragment variable for field', () => {
const query = `
query {
...TestFragment(a: null)
}
fragment TestFragment($a: String) on Query {
test(input: { a: $a }) {
a
b
}
}
`;
const result = executeQuery(query, rootValue, { a: null });

expectJSON(result).toDeepEqual({
data: {
test: null,
},
errors: [
{
// A nullable variable in a oneOf field position would be caught at validation-time
// hence the vague error message here.
message:
'Argument "input" has invalid value: Expected variable "$a" provided to field "a" for OneOf Input Object type "TestInputObject" not to be null.',
locations: [{ line: 6, column: 23 }],
path: ['test'],
},
],
});
});

it('errors with missing fragment variable for field', () => {
const query = `
query {
...TestFragment
}
fragment TestFragment($a: String) on Query {
test(input: { a: $a }) {
a
b
}
}
`;
const result = executeQuery(query, rootValue);

expectJSON(result).toDeepEqual({
data: {
test: null,
},
errors: [
{
// A nullable variable in a oneOf field position would be caught at validation-time
// hence the vague error message here.
message:
'Argument "input" has invalid value: Expected variable "$a" provided to field "a" for OneOf Input Object type "TestInputObject" to provide a runtime value.',
locations: [{ line: 6, column: 23 }],
path: ['test'],
},
],
});
Expand Down
2 changes: 1 addition & 1 deletion src/execution/__tests__/subscribe-test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -567,7 +567,7 @@ describe('Subscription Initialization Phase', () => {
errors: [
{
message:
'Variable "$arg" got invalid value "meow"; Int cannot represent non-integer value: "meow"',
'Variable "$arg" has invalid value: Int cannot represent non-integer value: "meow"',
locations: [{ line: 2, column: 21 }],
},
],
Expand Down
Loading

0 comments on commit beff1d5

Please sign in to comment.