From 136cde0b563a1b04bf46900d9cc40384935bb1e9 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Wed, 18 Sep 2024 15:59:20 +0300 Subject: [PATCH 01/10] add runtime errors for pre-coercion OneOf errors --- src/execution/__tests__/oneof-test.ts | 357 +++++++++++++++++- .../__tests__/coerceInputValue-test.ts | 39 +- src/utilities/coerceInputValue.ts | 61 ++- src/utilities/valueFromAST.ts | 11 +- 4 files changed, 440 insertions(+), 28 deletions(-) diff --git a/src/execution/__tests__/oneof-test.ts b/src/execution/__tests__/oneof-test.ts index f4a11f8997..84fee50ccb 100644 --- a/src/execution/__tests__/oneof-test.ts +++ b/src/execution/__tests__/oneof-test.ts @@ -30,7 +30,12 @@ function executeQuery( rootValue: unknown, variableValues?: { [variable: string]: unknown }, ): ExecutionResult | Promise { - return execute({ schema, document: parse(query), rootValue, variableValues }); + return execute({ + schema, + document: parse(query, { experimentalFragmentArguments: true }), + rootValue, + variableValues, + }); } describe('Execute: Handles OneOf Input Objects', () => { @@ -158,7 +163,7 @@ describe('Execute: Handles OneOf Input Objects', () => { }); }); - it('rejects a variable with multiple nullable keys', () => { + it('rejects a variable with multiple keys, some set to null', () => { const query = ` query ($input: TestInputObject!) { test(input: $input) { @@ -178,6 +183,354 @@ describe('Execute: Handles OneOf Input Objects', () => { message: 'Variable "$input" got invalid value { a: "abc", b: null }; Exactly one key must be specified for OneOf type "TestInputObject".', }, + { + message: + 'Variable "$input" got invalid value null at "input.b"; Field "b" of OneOf type "TestInputObject" must be non-null.', + locations: [{ line: 2, column: 16 }], + }, + ], + }); + }); + + it('rejects a variable with multiple null keys', () => { + const query = ` + query ($input: TestInputObject!) { + test(input: $input) { + a + b + } + } + `; + const result = executeQuery(query, rootValue, { + input: { a: null, b: null }, + }); + + expectJSON(result).toDeepEqual({ + errors: [ + { + locations: [{ column: 16, line: 2 }], + message: + 'Variable "$input" got invalid value { a: null, b: null }; Exactly one key must be specified for OneOf type "TestInputObject".', + }, + { + message: + 'Variable "$input" got invalid value null at "input.a"; Field "a" of OneOf type "TestInputObject" must be non-null.', + locations: [{ line: 2, column: 16 }], + }, + { + message: + 'Variable "$input" got invalid value null at "input.b"; Field "b" of OneOf type "TestInputObject" must be non-null.', + locations: [{ line: 2, column: 16 }], + }, + ], + }); + }); + + it('accepts a valid variable for field', () => { + const query = ` + query ($a: String!) { + test(input: { a: $a }) { + a + b + } + } + `; + const result = executeQuery(query, rootValue, { a: 'abc' }); + + expectJSON(result).toDeepEqual({ + data: { + test: { + a: 'abc', + b: null, + }, + }, + }); + }); + + it('rejects multiple variables for fields', () => { + const query = ` + query ($a: String!, $b: Int!) { + test(input: { a: $a, b: $b }) { + a + b + } + } + `; + const result = executeQuery(query, rootValue, { a: 'abc', b: 123 }); + + 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" of type "TestInputObject!" has invalid value { a: $a, b: $b }.', + locations: [{ line: 3, column: 23 }], + path: ['test'], + }, + ], + }); + }); + + it('rejects variable for field explicitly set to undefined', () => { + const query = ` + query ($a: String) { + test(input: { a: $a }) { + a + b + } + } + `; + const result = executeQuery(query, rootValue, { a: undefined }); + + 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" of type "TestInputObject!" has invalid value { a: $a }.', + locations: [{ line: 3, column: 23 }], + path: ['test'], + }, + ], + }); + }); + + it('rejects 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" of type "TestInputObject!" has invalid value { a: $a }.', + locations: [{ line: 3, column: 23 }], + path: ['test'], + }, + ], + }); + }); + + it('rejects 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" of type "TestInputObject!" has invalid value { a: $a }.', + locations: [{ line: 3, column: 23 }], + path: ['test'], + }, + ], + }); + }); + + it('rejects missing second variable for field', () => { + const query = ` + query ($a: String, $b: String) { + test(input: { a: $a, b: $b }) { + a + b + } + } + `; + const result = executeQuery(query, rootValue, { a: '123' }); + + 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" of type "TestInputObject!" has invalid value { a: $a, b: $b }.', + locations: [{ line: 3, column: 23 }], + path: ['test'], + }, + ], + }); + }); + + it('accepts a valid fragment variable for field', () => { + const query = ` + query { + ...TestFragment(a: "abc") + } + fragment TestFragment($a: String) on Query { + test(input: { a: $a }) { + a + b + } + } + `; + const result = executeQuery(query, rootValue, { a: 'abc' }); + + expectJSON(result).toDeepEqual({ + data: { + test: { + a: 'abc', + b: null, + }, + }, + }); + }); + + it('rejects multiple fragment variables for fields', () => { + const query = ` + query { + ...TestFragment(a: "abc", b: 123) + } + fragment TestFragment($a: String, $b: Int) on Query { + test(input: { a: $a, b: $b }) { + a + b + } + } + `; + const result = executeQuery(query, rootValue, { a: 'abc', b: 123 }); + + 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" of type "TestInputObject!" has invalid value { a: $a, b: $b }.', + locations: [{ line: 6, column: 23 }], + path: ['test'], + }, + ], + }); + }); + + it('rejects 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" of type "TestInputObject!" has invalid value { a: $a }.', + locations: [{ line: 6, column: 23 }], + path: ['test'], + }, + ], + }); + }); + + it('rejects 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" of type "TestInputObject!" has invalid value { a: $a }.', + locations: [{ line: 6, column: 23 }], + path: ['test'], + }, + ], + }); + }); + + it('rejects missing second fragment variable for field', () => { + const query = ` + query { + ...TestFragment(a: "123") + } + fragment TestFragment($a: String, $b: string) on Query { + test(input: { a: $a, b: $b }) { + a + b + } + } + `; + const result = executeQuery(query, rootValue, { a: '123' }); + + 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" of type "TestInputObject!" has invalid value { a: $a, b: $b }.', + locations: [{ line: 6, column: 23 }], + path: ['test'], + }, ], }); }); diff --git a/src/utilities/__tests__/coerceInputValue-test.ts b/src/utilities/__tests__/coerceInputValue-test.ts index 15467e8456..52a3c1f65e 100644 --- a/src/utilities/__tests__/coerceInputValue-test.ts +++ b/src/utilities/__tests__/coerceInputValue-test.ts @@ -303,22 +303,47 @@ describe('coerceInputValue', () => { }); it('returns an error if more than one field is specified', () => { - const result = coerceValue({ foo: 123, bar: null }, TestInputObject); + const result = coerceValue({ foo: 123, bar: 456 }, TestInputObject); expectErrors(result).to.deep.equal([ { error: 'Exactly one key must be specified for OneOf type "TestInputObject".', path: [], - value: { foo: 123, bar: null }, + value: { foo: 123, bar: 456 }, }, ]); }); - it('returns an error the one field is null', () => { + it('returns an error if the one field is null', () => { const result = coerceValue({ bar: null }, TestInputObject); expectErrors(result).to.deep.equal([ { - error: 'Field "bar" must be non-null.', + error: + 'Field "bar" of OneOf type "TestInputObject" must be non-null.', + path: ['bar'], + value: null, + }, + ]); + }); + + it('returns descriptive errors if more than one field is null', () => { + const result = coerceValue({ foo: null, bar: null }, TestInputObject); + expectErrors(result).to.deep.equal([ + { + error: + 'Exactly one key must be specified for OneOf type "TestInputObject".', + path: [], + value: { foo: null, bar: null }, + }, + { + error: + 'Field "foo" of OneOf type "TestInputObject" must be non-null.', + path: ['foo'], + value: null, + }, + { + error: + 'Field "bar" of OneOf type "TestInputObject" must be non-null.', path: ['bar'], value: null, }, @@ -370,6 +395,12 @@ describe('coerceInputValue', () => { path: [], value: { foo: 123, unknownField: 123 }, }, + { + error: + 'Exactly one key must be specified for OneOf type "TestInputObject".', + path: [], + value: { foo: 123, unknownField: 123 }, + }, ]); }); diff --git a/src/utilities/coerceInputValue.ts b/src/utilities/coerceInputValue.ts index 88c97c8405..73fdfab66e 100644 --- a/src/utilities/coerceInputValue.ts +++ b/src/utilities/coerceInputValue.ts @@ -134,8 +134,28 @@ function coerceInputValueImpl( ); } + /** + * keys with the value "undefined" do not appear within the coerced input + * object, but do appear within the original input object. + * + * Note: in graphql-js, supplying the value "undefined" for a variable + * (as opposed to entirely omitting the variable from the map) will cause + * the variable value to be coerced to "null", but supplying "undefined" + * for an input object field will cause the field to be dropped as if the + * field did not exist in the map. We could consider changing this for + * input object fields to match the behavior of scalars variables and + * output variables where an actual value of "undefined" is considered to + * have the same intent as the "null" value. + * + * This use of `fieldsWithDefinedValues` is a workaround for this JavaScript- + * specific behavior. + * + * See: https://github.com/graphql/graphql-js/issues/2533 + */ + let fieldsWithDefinedValues = 0; + const keys = Object.keys(inputValue); // Ensure every provided field is defined. - for (const fieldName of Object.keys(inputValue)) { + for (const fieldName of keys) { if (fieldDefs[fieldName] == null) { const suggestions = suggestionList( fieldName, @@ -150,11 +170,15 @@ function coerceInputValueImpl( ), ); } + + if (inputValue[fieldName] !== undefined) { + fieldsWithDefinedValues++; + } } if (type.isOneOf) { - const keys = Object.keys(coercedValue); - if (keys.length !== 1) { + const coercedKeys = Object.keys(coercedValue); + if (fieldsWithDefinedValues !== 1 || coercedKeys.length !== 1) { onError( pathToArray(path), inputValue, @@ -163,15 +187,17 @@ function coerceInputValueImpl( ), ); } - - const key = keys[0]; - const value = coercedValue[key]; - if (value === null) { - onError( - pathToArray(path).concat(key), - value, - new GraphQLError(`Field "${key}" must be non-null.`), - ); + for (const key of keys) { + const value = inputValue[key]; + if (value === null || coercedValue[key] === null) { + onError( + pathToArray(path).concat(key), + value, + new GraphQLError( + `Field "${key}" of OneOf type "${type}" must be non-null.`, + ), + ); + } } } @@ -344,13 +370,14 @@ export function coerceInputLiteral( } if (type.isOneOf) { - const keys = Object.keys(coercedValue); - if (keys.length !== 1) { + const coercedKeys = Object.keys(coercedValue); + if (fieldNodes.size !== 1 || coercedKeys.length !== 1) { return; // Invalid: not exactly one key, intentionally return no value. } - - if (coercedValue[keys[0]] === null) { - return; // Invalid: value not non-null, intentionally return no value. + for (const [key, value] of fieldNodes) { + if (value.value.kind === Kind.NULL || coercedValue[key] === null) { + return; // Invalid: not exactly one key, intentionally return no value. + } } } diff --git a/src/utilities/valueFromAST.ts b/src/utilities/valueFromAST.ts index add9153680..51b98da1cf 100644 --- a/src/utilities/valueFromAST.ts +++ b/src/utilities/valueFromAST.ts @@ -129,13 +129,14 @@ export function valueFromAST( } if (type.isOneOf) { - const keys = Object.keys(coercedObj); - if (keys.length !== 1) { + const coercedKeys = Object.keys(coercedObj); + if (fieldNodes.size !== 1 || coercedKeys.length !== 1) { return; // Invalid: not exactly one key, intentionally return no value. } - - if (coercedObj[keys[0]] === null) { - return; // Invalid: value not non-null, intentionally return no value. + for (const [key, value] of fieldNodes) { + if (value.value.kind === Kind.NULL || coercedObj[key] === null) { + return; // Invalid: not exactly one key, intentionally return no value. + } } } From 70e5c4f9151ffa309c4c7986463842ee239c19ee Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Wed, 18 Sep 2024 16:31:52 +0300 Subject: [PATCH 02/10] only return a single error per value --- src/execution/__tests__/oneof-test.ts | 15 --------------- src/utilities/__tests__/coerceInputValue-test.ts | 14 +------------- src/utilities/coerceInputValue.ts | 4 ++-- src/utilities/valueFromAST.ts | 9 +++++---- 4 files changed, 8 insertions(+), 34 deletions(-) diff --git a/src/execution/__tests__/oneof-test.ts b/src/execution/__tests__/oneof-test.ts index 84fee50ccb..11bbf85dbc 100644 --- a/src/execution/__tests__/oneof-test.ts +++ b/src/execution/__tests__/oneof-test.ts @@ -183,11 +183,6 @@ describe('Execute: Handles OneOf Input Objects', () => { message: 'Variable "$input" got invalid value { a: "abc", b: null }; Exactly one key must be specified for OneOf type "TestInputObject".', }, - { - message: - 'Variable "$input" got invalid value null at "input.b"; Field "b" of OneOf type "TestInputObject" must be non-null.', - locations: [{ line: 2, column: 16 }], - }, ], }); }); @@ -212,16 +207,6 @@ describe('Execute: Handles OneOf Input Objects', () => { message: 'Variable "$input" got invalid value { a: null, b: null }; Exactly one key must be specified for OneOf type "TestInputObject".', }, - { - message: - 'Variable "$input" got invalid value null at "input.a"; Field "a" of OneOf type "TestInputObject" must be non-null.', - locations: [{ line: 2, column: 16 }], - }, - { - message: - 'Variable "$input" got invalid value null at "input.b"; Field "b" of OneOf type "TestInputObject" must be non-null.', - locations: [{ line: 2, column: 16 }], - }, ], }); }); diff --git a/src/utilities/__tests__/coerceInputValue-test.ts b/src/utilities/__tests__/coerceInputValue-test.ts index 52a3c1f65e..a32ae2cbd0 100644 --- a/src/utilities/__tests__/coerceInputValue-test.ts +++ b/src/utilities/__tests__/coerceInputValue-test.ts @@ -326,7 +326,7 @@ describe('coerceInputValue', () => { ]); }); - it('returns descriptive errors if more than one field is null', () => { + it('returns an error if more than one field is null', () => { const result = coerceValue({ foo: null, bar: null }, TestInputObject); expectErrors(result).to.deep.equal([ { @@ -335,18 +335,6 @@ describe('coerceInputValue', () => { path: [], value: { foo: null, bar: null }, }, - { - error: - 'Field "foo" of OneOf type "TestInputObject" must be non-null.', - path: ['foo'], - value: null, - }, - { - error: - 'Field "bar" of OneOf type "TestInputObject" must be non-null.', - path: ['bar'], - value: null, - }, ]); }); diff --git a/src/utilities/coerceInputValue.ts b/src/utilities/coerceInputValue.ts index 73fdfab66e..fd0655c5cd 100644 --- a/src/utilities/coerceInputValue.ts +++ b/src/utilities/coerceInputValue.ts @@ -186,8 +186,8 @@ function coerceInputValueImpl( `Exactly one key must be specified for OneOf type "${type}".`, ), ); - } - for (const key of keys) { + } else { + const key = keys[0]; const value = inputValue[key]; if (value === null || coercedValue[key] === null) { onError( diff --git a/src/utilities/valueFromAST.ts b/src/utilities/valueFromAST.ts index 51b98da1cf..0cf6d8c601 100644 --- a/src/utilities/valueFromAST.ts +++ b/src/utilities/valueFromAST.ts @@ -133,10 +133,11 @@ export function valueFromAST( if (fieldNodes.size !== 1 || coercedKeys.length !== 1) { return; // Invalid: not exactly one key, intentionally return no value. } - for (const [key, value] of fieldNodes) { - if (value.value.kind === Kind.NULL || coercedObj[key] === null) { - return; // Invalid: not exactly one key, intentionally return no value. - } + + const fieldNode = valueNode.fields[0]; + const key = fieldNode.name.value; + if (fieldNode.value.kind === Kind.NULL || coercedObj[key] === null) { + return; // Invalid: not exactly one key, intentionally return no value. } } From 5feb200b486078a7761990b8f52e1969dc4fb544 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Wed, 18 Sep 2024 23:05:37 +0300 Subject: [PATCH 03/10] combine number of fields and nullability into uniform error message --- src/execution/__tests__/oneof-test.ts | 6 +++--- src/utilities/__tests__/coerceInputValue-test.ts | 12 ++++++------ src/utilities/coerceInputValue.ts | 4 ++-- .../__tests__/ValuesOfCorrectTypeRule-test.ts | 7 ++++--- src/validation/rules/ValuesOfCorrectTypeRule.ts | 13 ++++++++----- 5 files changed, 23 insertions(+), 19 deletions(-) diff --git a/src/execution/__tests__/oneof-test.ts b/src/execution/__tests__/oneof-test.ts index 11bbf85dbc..d773340baa 100644 --- a/src/execution/__tests__/oneof-test.ts +++ b/src/execution/__tests__/oneof-test.ts @@ -157,7 +157,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" got invalid value { a: "abc", b: 123 }; Within OneOf Input Object Type "TestInputObject", exactly one field must be specified, and the value for that field must be non-null.', }, ], }); @@ -181,7 +181,7 @@ 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" got invalid value { a: "abc", b: null }; Within OneOf Input Object Type "TestInputObject", exactly one field must be specified, and the value for that field must be non-null.', }, ], }); @@ -205,7 +205,7 @@ describe('Execute: Handles OneOf Input Objects', () => { { locations: [{ column: 16, line: 2 }], message: - 'Variable "$input" got invalid value { a: null, b: null }; Exactly one key must be specified for OneOf type "TestInputObject".', + 'Variable "$input" got invalid value { a: null, b: null }; Within OneOf Input Object Type "TestInputObject", exactly one field must be specified, and the value for that field must be non-null.', }, ], }); diff --git a/src/utilities/__tests__/coerceInputValue-test.ts b/src/utilities/__tests__/coerceInputValue-test.ts index a32ae2cbd0..193bfbafdb 100644 --- a/src/utilities/__tests__/coerceInputValue-test.ts +++ b/src/utilities/__tests__/coerceInputValue-test.ts @@ -307,7 +307,7 @@ describe('coerceInputValue', () => { expectErrors(result).to.deep.equal([ { error: - 'Exactly one key must be specified for OneOf type "TestInputObject".', + 'Within OneOf Input Object Type "TestInputObject", exactly one field must be specified, and the value for that field must be non-null.', path: [], value: { foo: 123, bar: 456 }, }, @@ -319,7 +319,7 @@ describe('coerceInputValue', () => { expectErrors(result).to.deep.equal([ { error: - 'Field "bar" of OneOf type "TestInputObject" must be non-null.', + 'Within OneOf Input Object Type "TestInputObject", exactly one field must be specified, and the value for that field must be non-null.', path: ['bar'], value: null, }, @@ -331,7 +331,7 @@ describe('coerceInputValue', () => { expectErrors(result).to.deep.equal([ { error: - 'Exactly one key must be specified for OneOf type "TestInputObject".', + 'Within OneOf Input Object Type "TestInputObject", exactly one field must be specified, and the value for that field must be non-null.', path: [], value: { foo: null, bar: null }, }, @@ -364,7 +364,7 @@ describe('coerceInputValue', () => { }, { error: - 'Exactly one key must be specified for OneOf type "TestInputObject".', + 'Within OneOf Input Object Type "TestInputObject", exactly one field must be specified, and the value for that field must be non-null.', path: [], value: { foo: 'abc', bar: 'def' }, }, @@ -385,7 +385,7 @@ describe('coerceInputValue', () => { }, { error: - 'Exactly one key must be specified for OneOf type "TestInputObject".', + 'Within OneOf Input Object Type "TestInputObject", exactly one field must be specified, and the value for that field must be non-null.', path: [], value: { foo: 123, unknownField: 123 }, }, @@ -403,7 +403,7 @@ describe('coerceInputValue', () => { }, { error: - 'Exactly one key must be specified for OneOf type "TestInputObject".', + 'Within OneOf Input Object Type "TestInputObject", exactly one field must be specified, and the value for that field must be non-null.', path: [], value: { bart: 123 }, }, diff --git a/src/utilities/coerceInputValue.ts b/src/utilities/coerceInputValue.ts index fd0655c5cd..b38369c730 100644 --- a/src/utilities/coerceInputValue.ts +++ b/src/utilities/coerceInputValue.ts @@ -183,7 +183,7 @@ function coerceInputValueImpl( pathToArray(path), inputValue, new GraphQLError( - `Exactly one key must be specified for OneOf type "${type}".`, + `Within OneOf Input Object Type "${type}", exactly one field must be specified, and the value for that field must be non-null.`, ), ); } else { @@ -194,7 +194,7 @@ function coerceInputValueImpl( pathToArray(path).concat(key), value, new GraphQLError( - `Field "${key}" of OneOf type "${type}" must be non-null.`, + `Within OneOf Input Object Type "${type}", exactly one field must be specified, and the value for that field must be non-null.`, ), ); } diff --git a/src/validation/__tests__/ValuesOfCorrectTypeRule-test.ts b/src/validation/__tests__/ValuesOfCorrectTypeRule-test.ts index 819d103e6a..7c23b95b7d 100644 --- a/src/validation/__tests__/ValuesOfCorrectTypeRule-test.ts +++ b/src/validation/__tests__/ValuesOfCorrectTypeRule-test.ts @@ -1088,7 +1088,8 @@ describe('Validate: Values of correct type', () => { } `).toDeepEqual([ { - message: 'Field "OneOfInput.stringField" must be non-null.', + message: + 'Within OneOf Input Object Type "OneOfInput", exactly one field must be specified, and the value for that field must be non-null.', locations: [{ line: 4, column: 37 }], }, ]); @@ -1104,7 +1105,7 @@ describe('Validate: Values of correct type', () => { `).toDeepEqual([ { message: - 'Variable "$string" must be non-nullable to be used for OneOf Input Object "OneOfInput".', + 'Variable "$string" must be non-nullable to be used for Within OneOf Input Object Type "OneOfInput".', locations: [{ line: 4, column: 37 }], }, ]); @@ -1120,7 +1121,7 @@ describe('Validate: Values of correct type', () => { `).toDeepEqual([ { message: - 'OneOf Input Object "OneOfInput" must specify exactly one key.', + 'Within OneOf Input Object Type "OneOfInput", exactly one field must be specified, and the value for that field must be non-null.', locations: [{ line: 4, column: 37 }], }, ]); diff --git a/src/validation/rules/ValuesOfCorrectTypeRule.ts b/src/validation/rules/ValuesOfCorrectTypeRule.ts index a86a66bd47..32e15f433c 100644 --- a/src/validation/rules/ValuesOfCorrectTypeRule.ts +++ b/src/validation/rules/ValuesOfCorrectTypeRule.ts @@ -193,7 +193,7 @@ function validateOneOfInputObject( if (isNotExactlyOneField) { context.reportError( new GraphQLError( - `OneOf Input Object "${type}" must specify exactly one key.`, + `Within OneOf Input Object Type "${type}", exactly one field must be specified, and the value for that field must be non-null.`, { nodes: [node] }, ), ); @@ -206,9 +206,12 @@ function validateOneOfInputObject( if (isNullLiteral) { context.reportError( - new GraphQLError(`Field "${type}.${keys[0]}" must be non-null.`, { - nodes: [node], - }), + new GraphQLError( + `Within OneOf Input Object Type "${type}", exactly one field must be specified, and the value for that field must be non-null.`, + { + nodes: [node], + }, + ), ); return; } @@ -221,7 +224,7 @@ function validateOneOfInputObject( if (isNullableVariable) { context.reportError( new GraphQLError( - `Variable "$${variableName}" must be non-nullable to be used for OneOf Input Object "${type}".`, + `Variable "$${variableName}" must be non-nullable to be used for Within OneOf Input Object Type "${type}".`, { nodes: [node] }, ), ); From d4d1bcb13106166218001f28b16bc7fe287a9ea2 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Fri, 20 Sep 2024 15:19:53 +0300 Subject: [PATCH 04/10] don't capitalize "Type" --- src/execution/__tests__/oneof-test.ts | 6 +- src/execution/__tests__/variables-test.ts | 218 +++++++++++++++++- .../__tests__/coerceInputValue-test.ts | 55 ++++- src/utilities/coerceInputValue.ts | 4 +- .../__tests__/ValuesOfCorrectTypeRule-test.ts | 6 +- .../rules/ValuesOfCorrectTypeRule.ts | 6 +- 6 files changed, 274 insertions(+), 21 deletions(-) diff --git a/src/execution/__tests__/oneof-test.ts b/src/execution/__tests__/oneof-test.ts index d773340baa..0796c01871 100644 --- a/src/execution/__tests__/oneof-test.ts +++ b/src/execution/__tests__/oneof-test.ts @@ -157,7 +157,7 @@ describe('Execute: Handles OneOf Input Objects', () => { { locations: [{ column: 16, line: 2 }], message: - 'Variable "$input" got invalid value { a: "abc", b: 123 }; Within OneOf Input Object Type "TestInputObject", exactly one field must be specified, and the value for that field must be non-null.', + 'Variable "$input" got invalid value { a: "abc", b: 123 }; Within OneOf Input Object type "TestInputObject", exactly one field must be specified, and the value for that field must be non-null.', }, ], }); @@ -181,7 +181,7 @@ describe('Execute: Handles OneOf Input Objects', () => { { locations: [{ column: 16, line: 2 }], message: - 'Variable "$input" got invalid value { a: "abc", b: null }; Within OneOf Input Object Type "TestInputObject", exactly one field must be specified, and the value for that field must be non-null.', + 'Variable "$input" got invalid value { a: "abc", b: null }; Within OneOf Input Object type "TestInputObject", exactly one field must be specified, and the value for that field must be non-null.', }, ], }); @@ -205,7 +205,7 @@ describe('Execute: Handles OneOf Input Objects', () => { { locations: [{ column: 16, line: 2 }], message: - 'Variable "$input" got invalid value { a: null, b: null }; Within OneOf Input Object Type "TestInputObject", exactly one field must be specified, and the value for that field must be non-null.', + 'Variable "$input" got invalid value { a: null, b: null }; Within OneOf Input Object type "TestInputObject", exactly one field must be specified, and the value for that field must be non-null.', }, ], }); diff --git a/src/execution/__tests__/variables-test.ts b/src/execution/__tests__/variables-test.ts index 7fe5a8a10d..ab4e23975f 100644 --- a/src/execution/__tests__/variables-test.ts +++ b/src/execution/__tests__/variables-test.ts @@ -27,7 +27,11 @@ import { GraphQLDirective, GraphQLIncludeDirective, } from '../../type/directives.js'; -import { GraphQLBoolean, GraphQLString } from '../../type/scalars.js'; +import { + GraphQLBoolean, + GraphQLInt, + GraphQLString, +} from '../../type/scalars.js'; import { GraphQLSchema } from '../../type/schema.js'; import { executeSync, experimentalExecuteIncrementally } from '../execute.js'; @@ -90,6 +94,15 @@ const TestNestedInputObject = new GraphQLInputObjectType({ }, }); +const TestOneOfInputObject = new GraphQLInputObjectType({ + name: 'TestOneOfInputObject', + fields: { + a: { type: GraphQLString }, + b: { type: GraphQLInt }, + }, + isOneOf: true, +}); + const TestEnum = new GraphQLEnumType({ name: 'TestEnum', values: { @@ -140,6 +153,9 @@ const TestType = new GraphQLObjectType({ type: TestNestedInputObject, defaultValue: 'Hello World', }), + fieldWithOneOfObjectInput: fieldWithInputArg({ + type: TestOneOfInputObject, + }), list: fieldWithInputArg({ type: new GraphQLList(GraphQLString) }), nested: { type: NestedType, @@ -1114,6 +1130,206 @@ describe('Execute: Handles inputs', () => { }); }); + describe('Handles OneOf Input Object types', () => { + it('allows OneOf Input Object with single field', () => { + const result = executeQuery(` + { + fieldWithOneOfObjectInput(input: { a: "abc" }) + } + `); + + expectJSON(result).toDeepEqual({ + data: { + fieldWithOneOfObjectInput: '{ a: "abc" }', + }, + }); + }); + }); + + it('errors with OneOf Input Object with more than one field', () => { + const result = executeQuery(` + { + fieldWithOneOfObjectInput(input: { a: "abc", b: 123 }) + } + `); + + expectJSON(result).toDeepEqual({ + data: { + fieldWithOneOfObjectInput: null, + }, + errors: [ + { + message: + 'Argument "input" of type "TestOneOfInputObject" has invalid value { a: "abc", b: 123 }.', + path: ['fieldWithOneOfObjectInput'], + locations: [{ line: 3, column: 42 }], + }, + ], + }); + }); + + it('errors with OneOf Input Object with a single null value', () => { + const result = executeQuery(` + { + fieldWithOneOfObjectInput(input: { a: null }) + } + `); + + expectJSON(result).toDeepEqual({ + data: { + fieldWithOneOfObjectInput: null, + }, + errors: [ + { + message: + 'Argument "input" of type "TestOneOfInputObject" has invalid value { a: null }.', + path: ['fieldWithOneOfObjectInput'], + locations: [{ line: 3, column: 42 }], + }, + ], + }); + }); + + it('errors with OneOf Input Object with multiple values, only one non-null', () => { + const result = executeQuery(` + { + fieldWithOneOfObjectInput(input: { a: "abc", b: null }) + } + `); + + expectJSON(result).toDeepEqual({ + data: { + fieldWithOneOfObjectInput: null, + }, + errors: [ + { + message: + 'Argument "input" of type "TestOneOfInputObject" has invalid value { a: "abc", b: null }.', + path: ['fieldWithOneOfObjectInput'], + locations: [{ line: 3, column: 42 }], + }, + ], + }); + }); + + it('allows a variable for the entire OneOf Object with a single value', () => { + const result = executeQuery( + ` + query ($input: TestOneOfInputObject) { + fieldWithOneOfObjectInput(input: $input) + } + `, + { input: { a: 'abc' } }, + ); + + expectJSON(result).toDeepEqual({ + data: { + fieldWithOneOfObjectInput: '{ a: "abc" }', + }, + }); + }); + + it('allows a variable for the entire OneOf Object with a single defined value', () => { + const result = executeQuery( + ` + query ($input: TestOneOfInputObject) { + fieldWithOneOfObjectInput(input: $input) + } + `, + { input: { a: 'abc', b: undefined } }, + ); + + expectJSON(result).toDeepEqual({ + data: { + fieldWithOneOfObjectInput: '{ a: "abc" }', + }, + }); + }); + + it('errors with variable with no value', () => { + const result = executeQuery( + ` + query ($input: TestOneOfInputObject) { + fieldWithOneOfObjectInput(input: $input) + } + `, + { input: {} }, + ); + + expectJSON(result).toDeepEqual({ + errors: [ + { + message: + 'Variable "$input" got invalid value {}; Within OneOf Input Object type "TestOneOfInputObject", exactly one field must be specified, and the value for that field must be non-null.', + locations: [{ line: 2, column: 16 }], + }, + ], + }); + }); + + it('errors with variable with multiple values', () => { + const result = executeQuery( + ` + query ($input: TestOneOfInputObject) { + fieldWithOneOfObjectInput(input: $input) + } + `, + { input: { a: 'abc', b: 123 } }, + ); + + expectJSON(result).toDeepEqual({ + errors: [ + { + message: + 'Variable "$input" got invalid value { a: "abc", b: 123 }; Within OneOf Input Object type "TestOneOfInputObject", exactly one field must be specified, and the value for that field must be non-null.', + locations: [{ line: 2, column: 16 }], + }, + ], + }); + }); + + it('errors with variable with single null value', () => { + const result = executeQuery( + ` + query ($input: TestOneOfInputObject) { + fieldWithOneOfObjectInput(input: $input) + } + `, + { input: { a: null } }, + ); + + expectJSON(result).toDeepEqual({ + errors: [ + { + message: + 'Variable "$input" got invalid value null at "input.a"; Within OneOf Input Object type "TestOneOfInputObject", exactly one field must be specified, and the value for that field must be non-null.', + locations: [{ line: 2, column: 16 }], + }, + ], + }); + }); + + it('errors with variable with multiple values, only one non-null', () => { + const result = executeQuery( + ` + query ($input: TestOneOfInputObject) { + fieldWithOneOfObjectInput(input: $input) + } + `, + { input: { a: 'abc', b: null } }, + ); + + expectJSON(result).toDeepEqual({ + errors: [ + { + message: + 'Variable "$input" got invalid value { a: "abc", b: null }; Within OneOf Input Object type "TestOneOfInputObject", exactly one field must be specified, and the value for that field must be non-null.', + locations: [{ line: 2, column: 16 }], + }, + ], + }); + }); + describe('getVariableValues: limit maximum number of coercion errors', () => { const doc = parse(` query ($input: [String!]) { diff --git a/src/utilities/__tests__/coerceInputValue-test.ts b/src/utilities/__tests__/coerceInputValue-test.ts index 193bfbafdb..99c6b55007 100644 --- a/src/utilities/__tests__/coerceInputValue-test.ts +++ b/src/utilities/__tests__/coerceInputValue-test.ts @@ -302,12 +302,24 @@ describe('coerceInputValue', () => { expectValue(result).to.deep.equal({ foo: 123 }); }); + it('returns an error if no fields are specified', () => { + const result = coerceValue({}, TestInputObject); + expectErrors(result).to.deep.equal([ + { + error: + 'Within OneOf Input Object type "TestInputObject", exactly one field must be specified, and the value for that field must be non-null.', + path: [], + value: {}, + }, + ]); + }); + it('returns an error if more than one field is specified', () => { const result = coerceValue({ foo: 123, bar: 456 }, TestInputObject); expectErrors(result).to.deep.equal([ { error: - 'Within OneOf Input Object Type "TestInputObject", exactly one field must be specified, and the value for that field must be non-null.', + 'Within OneOf Input Object type "TestInputObject", exactly one field must be specified, and the value for that field must be non-null.', path: [], value: { foo: 123, bar: 456 }, }, @@ -319,25 +331,31 @@ describe('coerceInputValue', () => { expectErrors(result).to.deep.equal([ { error: - 'Within OneOf Input Object Type "TestInputObject", exactly one field must be specified, and the value for that field must be non-null.', + 'Within OneOf Input Object type "TestInputObject", exactly one field must be specified, and the value for that field must be non-null.', path: ['bar'], value: null, }, ]); }); - it('returns an error if more than one field is null', () => { - const result = coerceValue({ foo: null, bar: null }, TestInputObject); + it('returns an error if there are multiple fields with only a single non-null', () => { + const result = coerceValue({ foo: 123, bar: null }, TestInputObject); expectErrors(result).to.deep.equal([ { error: - 'Within OneOf Input Object Type "TestInputObject", exactly one field must be specified, and the value for that field must be non-null.', + 'Within OneOf Input Object type "TestInputObject", exactly one field must be specified, and the value for that field must be non-null.', path: [], - value: { foo: null, bar: null }, + value: { foo: 123, bar: null }, }, ]); }); + // special non-normative graphql-js behavior + it('returns no error for an additional fields with value of undefined', () => { + const result = coerceValue({ foo: 123, bar: undefined }, TestInputObject); + expectValue(result).to.deep.equal({ foo: 123 }); + }); + it('returns an error for an invalid field', () => { const result = coerceValue({ foo: NaN }, TestInputObject); expectErrors(result).to.deep.equal([ @@ -364,7 +382,7 @@ describe('coerceInputValue', () => { }, { error: - 'Within OneOf Input Object Type "TestInputObject", exactly one field must be specified, and the value for that field must be non-null.', + 'Within OneOf Input Object type "TestInputObject", exactly one field must be specified, and the value for that field must be non-null.', path: [], value: { foo: 'abc', bar: 'def' }, }, @@ -385,7 +403,7 @@ describe('coerceInputValue', () => { }, { error: - 'Within OneOf Input Object Type "TestInputObject", exactly one field must be specified, and the value for that field must be non-null.', + 'Within OneOf Input Object type "TestInputObject", exactly one field must be specified, and the value for that field must be non-null.', path: [], value: { foo: 123, unknownField: 123 }, }, @@ -403,7 +421,7 @@ describe('coerceInputValue', () => { }, { error: - 'Within OneOf Input Object Type "TestInputObject", exactly one field must be specified, and the value for that field must be non-null.', + 'Within OneOf Input Object type "TestInputObject", exactly one field must be specified, and the value for that field must be non-null.', path: [], value: { bart: 123 }, }, @@ -804,6 +822,25 @@ describe('coerceInputLiteral', () => { testWithVariables({ var: null }, '$var', nonNullBool, undefined); }); + it('accepts variable values for fields of OneOf Input Objects', () => { + testWithVariables({ a: 'abc' }, '{ a: $a }', testOneOfInputObj, { + a: 'abc', + }); + testWithVariables({ a: null }, '{ a: $a }', testOneOfInputObj, undefined); + testWithVariables( + { a: 'abc', b: 'def' }, + '{ a: $a, b: $b }', + testOneOfInputObj, + undefined, + ); + testWithVariables( + { a: 'abc', b: null }, + '{ a: $a, b: $b }', + testOneOfInputObj, + undefined, + ); + }); + it('asserts variables are provided as items in lists', () => { test('[ $foo ]', listOfBool, [null]); test('[ $foo ]', listOfNonNullBool, undefined); diff --git a/src/utilities/coerceInputValue.ts b/src/utilities/coerceInputValue.ts index b38369c730..4a8a8057fa 100644 --- a/src/utilities/coerceInputValue.ts +++ b/src/utilities/coerceInputValue.ts @@ -183,7 +183,7 @@ function coerceInputValueImpl( pathToArray(path), inputValue, new GraphQLError( - `Within OneOf Input Object Type "${type}", exactly one field must be specified, and the value for that field must be non-null.`, + `Within OneOf Input Object type "${type}", exactly one field must be specified, and the value for that field must be non-null.`, ), ); } else { @@ -194,7 +194,7 @@ function coerceInputValueImpl( pathToArray(path).concat(key), value, new GraphQLError( - `Within OneOf Input Object Type "${type}", exactly one field must be specified, and the value for that field must be non-null.`, + `Within OneOf Input Object type "${type}", exactly one field must be specified, and the value for that field must be non-null.`, ), ); } diff --git a/src/validation/__tests__/ValuesOfCorrectTypeRule-test.ts b/src/validation/__tests__/ValuesOfCorrectTypeRule-test.ts index 7c23b95b7d..7cee65fc0d 100644 --- a/src/validation/__tests__/ValuesOfCorrectTypeRule-test.ts +++ b/src/validation/__tests__/ValuesOfCorrectTypeRule-test.ts @@ -1089,7 +1089,7 @@ describe('Validate: Values of correct type', () => { `).toDeepEqual([ { message: - 'Within OneOf Input Object Type "OneOfInput", exactly one field must be specified, and the value for that field must be non-null.', + 'Within OneOf Input Object type "OneOfInput", exactly one field must be specified, and the value for that field must be non-null.', locations: [{ line: 4, column: 37 }], }, ]); @@ -1105,7 +1105,7 @@ describe('Validate: Values of correct type', () => { `).toDeepEqual([ { message: - 'Variable "$string" must be non-nullable to be used for Within OneOf Input Object Type "OneOfInput".', + 'Variable "$string" must be non-nullable to be used for Within OneOf Input Object type "OneOfInput".', locations: [{ line: 4, column: 37 }], }, ]); @@ -1121,7 +1121,7 @@ describe('Validate: Values of correct type', () => { `).toDeepEqual([ { message: - 'Within OneOf Input Object Type "OneOfInput", exactly one field must be specified, and the value for that field must be non-null.', + 'Within OneOf Input Object type "OneOfInput", exactly one field must be specified, and the value for that field must be non-null.', locations: [{ line: 4, column: 37 }], }, ]); diff --git a/src/validation/rules/ValuesOfCorrectTypeRule.ts b/src/validation/rules/ValuesOfCorrectTypeRule.ts index 32e15f433c..59dfb6b045 100644 --- a/src/validation/rules/ValuesOfCorrectTypeRule.ts +++ b/src/validation/rules/ValuesOfCorrectTypeRule.ts @@ -193,7 +193,7 @@ function validateOneOfInputObject( if (isNotExactlyOneField) { context.reportError( new GraphQLError( - `Within OneOf Input Object Type "${type}", exactly one field must be specified, and the value for that field must be non-null.`, + `Within OneOf Input Object type "${type}", exactly one field must be specified, and the value for that field must be non-null.`, { nodes: [node] }, ), ); @@ -207,7 +207,7 @@ function validateOneOfInputObject( if (isNullLiteral) { context.reportError( new GraphQLError( - `Within OneOf Input Object Type "${type}", exactly one field must be specified, and the value for that field must be non-null.`, + `Within OneOf Input Object type "${type}", exactly one field must be specified, and the value for that field must be non-null.`, { nodes: [node], }, @@ -224,7 +224,7 @@ function validateOneOfInputObject( if (isNullableVariable) { context.reportError( new GraphQLError( - `Variable "$${variableName}" must be non-nullable to be used for Within OneOf Input Object Type "${type}".`, + `Variable "$${variableName}" must be non-nullable to be used for Within OneOf Input Object type "${type}".`, { nodes: [node] }, ), ); From d6fe83b6d2a0e8bf3dc053c267788ec8bdba7d9b Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Fri, 20 Sep 2024 15:23:59 +0300 Subject: [PATCH 05/10] fix extra word/capitalization with Within --- src/validation/__tests__/ValuesOfCorrectTypeRule-test.ts | 2 +- src/validation/rules/ValuesOfCorrectTypeRule.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/validation/__tests__/ValuesOfCorrectTypeRule-test.ts b/src/validation/__tests__/ValuesOfCorrectTypeRule-test.ts index 7cee65fc0d..53a771da24 100644 --- a/src/validation/__tests__/ValuesOfCorrectTypeRule-test.ts +++ b/src/validation/__tests__/ValuesOfCorrectTypeRule-test.ts @@ -1105,7 +1105,7 @@ describe('Validate: Values of correct type', () => { `).toDeepEqual([ { message: - 'Variable "$string" must be non-nullable to be used for Within OneOf Input Object type "OneOfInput".', + 'Variable "$string" must be non-nullable to be used within OneOf Input Object type "OneOfInput".', locations: [{ line: 4, column: 37 }], }, ]); diff --git a/src/validation/rules/ValuesOfCorrectTypeRule.ts b/src/validation/rules/ValuesOfCorrectTypeRule.ts index 59dfb6b045..550bda562e 100644 --- a/src/validation/rules/ValuesOfCorrectTypeRule.ts +++ b/src/validation/rules/ValuesOfCorrectTypeRule.ts @@ -224,7 +224,7 @@ function validateOneOfInputObject( if (isNullableVariable) { context.reportError( new GraphQLError( - `Variable "$${variableName}" must be non-nullable to be used for Within OneOf Input Object type "${type}".`, + `Variable "$${variableName}" must be non-nullable to be used within OneOf Input Object type "${type}".`, { nodes: [node] }, ), ); From 20f873459d42d420c0874f7c9a666c59b118e817 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Sun, 22 Sep 2024 00:52:33 +0300 Subject: [PATCH 06/10] fix indentation --- src/execution/__tests__/variables-test.ts | 308 +++++++++++----------- 1 file changed, 154 insertions(+), 154 deletions(-) diff --git a/src/execution/__tests__/variables-test.ts b/src/execution/__tests__/variables-test.ts index ab4e23975f..561eaf20c0 100644 --- a/src/execution/__tests__/variables-test.ts +++ b/src/execution/__tests__/variables-test.ts @@ -1144,189 +1144,189 @@ describe('Execute: Handles inputs', () => { }, }); }); - }); - it('errors with OneOf Input Object with more than one field', () => { - const result = executeQuery(` - { - fieldWithOneOfObjectInput(input: { a: "abc", b: 123 }) - } - `); - - expectJSON(result).toDeepEqual({ - data: { - fieldWithOneOfObjectInput: null, - }, - errors: [ + it('errors with OneOf Input Object with more than one field', () => { + const result = executeQuery(` { - message: - 'Argument "input" of type "TestOneOfInputObject" has invalid value { a: "abc", b: 123 }.', - path: ['fieldWithOneOfObjectInput'], - locations: [{ line: 3, column: 42 }], + fieldWithOneOfObjectInput(input: { a: "abc", b: 123 }) + } + `); + + expectJSON(result).toDeepEqual({ + data: { + fieldWithOneOfObjectInput: null, }, - ], + errors: [ + { + message: + 'Argument "input" of type "TestOneOfInputObject" has invalid value { a: "abc", b: 123 }.', + path: ['fieldWithOneOfObjectInput'], + locations: [{ line: 3, column: 44 }], + }, + ], + }); }); - }); - it('errors with OneOf Input Object with a single null value', () => { - const result = executeQuery(` - { - fieldWithOneOfObjectInput(input: { a: null }) - } - `); - - expectJSON(result).toDeepEqual({ - data: { - fieldWithOneOfObjectInput: null, - }, - errors: [ + it('errors with OneOf Input Object with a single null value', () => { + const result = executeQuery(` { - message: - 'Argument "input" of type "TestOneOfInputObject" has invalid value { a: null }.', - path: ['fieldWithOneOfObjectInput'], - locations: [{ line: 3, column: 42 }], + fieldWithOneOfObjectInput(input: { a: null }) + } + `); + + expectJSON(result).toDeepEqual({ + data: { + fieldWithOneOfObjectInput: null, }, - ], + errors: [ + { + message: + 'Argument "input" of type "TestOneOfInputObject" has invalid value { a: null }.', + path: ['fieldWithOneOfObjectInput'], + locations: [{ line: 3, column: 44 }], + }, + ], + }); }); - }); - it('errors with OneOf Input Object with multiple values, only one non-null', () => { - const result = executeQuery(` - { - fieldWithOneOfObjectInput(input: { a: "abc", b: null }) - } - `); - - expectJSON(result).toDeepEqual({ - data: { - fieldWithOneOfObjectInput: null, - }, - errors: [ + it('errors with OneOf Input Object with multiple values, only one non-null', () => { + const result = executeQuery(` { - message: - 'Argument "input" of type "TestOneOfInputObject" has invalid value { a: "abc", b: null }.', - path: ['fieldWithOneOfObjectInput'], - locations: [{ line: 3, column: 42 }], + fieldWithOneOfObjectInput(input: { a: "abc", b: null }) + } + `); + + expectJSON(result).toDeepEqual({ + data: { + fieldWithOneOfObjectInput: null, }, - ], + errors: [ + { + message: + 'Argument "input" of type "TestOneOfInputObject" has invalid value { a: "abc", b: null }.', + path: ['fieldWithOneOfObjectInput'], + locations: [{ line: 3, column: 44 }], + }, + ], + }); }); - }); - it('allows a variable for the entire OneOf Object with a single value', () => { - const result = executeQuery( - ` - query ($input: TestOneOfInputObject) { - fieldWithOneOfObjectInput(input: $input) - } - `, - { input: { a: 'abc' } }, - ); + it('allows a variable for the entire OneOf Object with a single value', () => { + const result = executeQuery( + ` + query ($input: TestOneOfInputObject) { + fieldWithOneOfObjectInput(input: $input) + } + `, + { input: { a: 'abc' } }, + ); - expectJSON(result).toDeepEqual({ - data: { - fieldWithOneOfObjectInput: '{ a: "abc" }', - }, + expectJSON(result).toDeepEqual({ + data: { + fieldWithOneOfObjectInput: '{ a: "abc" }', + }, + }); }); - }); - it('allows a variable for the entire OneOf Object with a single defined value', () => { - const result = executeQuery( - ` - query ($input: TestOneOfInputObject) { - fieldWithOneOfObjectInput(input: $input) - } - `, - { input: { a: 'abc', b: undefined } }, - ); + it('allows a variable for the entire OneOf Object with a single defined value', () => { + const result = executeQuery( + ` + query ($input: TestOneOfInputObject) { + fieldWithOneOfObjectInput(input: $input) + } + `, + { input: { a: 'abc', b: undefined } }, + ); - expectJSON(result).toDeepEqual({ - data: { - fieldWithOneOfObjectInput: '{ a: "abc" }', - }, + expectJSON(result).toDeepEqual({ + data: { + fieldWithOneOfObjectInput: '{ a: "abc" }', + }, + }); }); - }); - it('errors with variable with no value', () => { - const result = executeQuery( - ` - query ($input: TestOneOfInputObject) { - fieldWithOneOfObjectInput(input: $input) - } - `, - { input: {} }, - ); + it('errors with variable with no value', () => { + const result = executeQuery( + ` + query ($input: TestOneOfInputObject) { + fieldWithOneOfObjectInput(input: $input) + } + `, + { input: {} }, + ); - expectJSON(result).toDeepEqual({ - errors: [ - { - message: - 'Variable "$input" got invalid value {}; Within OneOf Input Object type "TestOneOfInputObject", exactly one field must be specified, and the value for that field must be non-null.', - locations: [{ line: 2, column: 16 }], - }, - ], + expectJSON(result).toDeepEqual({ + errors: [ + { + message: + 'Variable "$input" got invalid value {}; Within OneOf Input Object type "TestOneOfInputObject", exactly one field must be specified, and the value for that field must be non-null.', + locations: [{ line: 2, column: 18 }], + }, + ], + }); }); - }); - it('errors with variable with multiple values', () => { - const result = executeQuery( - ` - query ($input: TestOneOfInputObject) { - fieldWithOneOfObjectInput(input: $input) - } - `, - { input: { a: 'abc', b: 123 } }, - ); + it('errors with variable with multiple values', () => { + const result = executeQuery( + ` + query ($input: TestOneOfInputObject) { + fieldWithOneOfObjectInput(input: $input) + } + `, + { input: { a: 'abc', b: 123 } }, + ); - expectJSON(result).toDeepEqual({ - errors: [ - { - message: - 'Variable "$input" got invalid value { a: "abc", b: 123 }; Within OneOf Input Object type "TestOneOfInputObject", exactly one field must be specified, and the value for that field must be non-null.', - locations: [{ line: 2, column: 16 }], - }, - ], + expectJSON(result).toDeepEqual({ + errors: [ + { + message: + 'Variable "$input" got invalid value { a: "abc", b: 123 }; Within OneOf Input Object type "TestOneOfInputObject", exactly one field must be specified, and the value for that field must be non-null.', + locations: [{ line: 2, column: 18 }], + }, + ], + }); }); - }); - it('errors with variable with single null value', () => { - const result = executeQuery( - ` - query ($input: TestOneOfInputObject) { - fieldWithOneOfObjectInput(input: $input) - } - `, - { input: { a: null } }, - ); + it('errors with variable with single null value', () => { + const result = executeQuery( + ` + query ($input: TestOneOfInputObject) { + fieldWithOneOfObjectInput(input: $input) + } + `, + { input: { a: null } }, + ); - expectJSON(result).toDeepEqual({ - errors: [ - { - message: - 'Variable "$input" got invalid value null at "input.a"; Within OneOf Input Object type "TestOneOfInputObject", exactly one field must be specified, and the value for that field must be non-null.', - locations: [{ line: 2, column: 16 }], - }, - ], + expectJSON(result).toDeepEqual({ + errors: [ + { + message: + 'Variable "$input" got invalid value null at "input.a"; Within OneOf Input Object type "TestOneOfInputObject", exactly one field must be specified, and the value for that field must be non-null.', + locations: [{ line: 2, column: 18 }], + }, + ], + }); }); - }); - it('errors with variable with multiple values, only one non-null', () => { - const result = executeQuery( - ` - query ($input: TestOneOfInputObject) { - fieldWithOneOfObjectInput(input: $input) - } - `, - { input: { a: 'abc', b: null } }, - ); + it('errors with variable with multiple values, only one non-null', () => { + const result = executeQuery( + ` + query ($input: TestOneOfInputObject) { + fieldWithOneOfObjectInput(input: $input) + } + `, + { input: { a: 'abc', b: null } }, + ); - expectJSON(result).toDeepEqual({ - errors: [ - { - message: - 'Variable "$input" got invalid value { a: "abc", b: null }; Within OneOf Input Object type "TestOneOfInputObject", exactly one field must be specified, and the value for that field must be non-null.', - locations: [{ line: 2, column: 16 }], - }, - ], + expectJSON(result).toDeepEqual({ + errors: [ + { + message: + 'Variable "$input" got invalid value { a: "abc", b: null }; Within OneOf Input Object type "TestOneOfInputObject", exactly one field must be specified, and the value for that field must be non-null.', + locations: [{ line: 2, column: 18 }], + }, + ], + }); }); }); From 15a67e88e2a77f833fe1ae7ffc21354a23016e7d Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Sun, 22 Sep 2024 00:59:47 +0300 Subject: [PATCH 07/10] move final oneOf tests into variables-test.ts --- src/execution/__tests__/oneof-test.ts | 523 ---------------------- src/execution/__tests__/variables-test.ts | 41 ++ 2 files changed, 41 insertions(+), 523 deletions(-) delete mode 100644 src/execution/__tests__/oneof-test.ts diff --git a/src/execution/__tests__/oneof-test.ts b/src/execution/__tests__/oneof-test.ts deleted file mode 100644 index 0796c01871..0000000000 --- a/src/execution/__tests__/oneof-test.ts +++ /dev/null @@ -1,523 +0,0 @@ -import { describe, it } from 'mocha'; - -import { expectJSON } from '../../__testUtils__/expectJSON.js'; - -import { parse } from '../../language/parser.js'; - -import { buildSchema } from '../../utilities/buildASTSchema.js'; - -import { execute } from '../execute.js'; -import type { ExecutionResult } from '../types.js'; - -const schema = buildSchema(` - type Query { - test(input: TestInputObject!): TestObject - } - - input TestInputObject @oneOf { - a: String - b: Int - } - - type TestObject { - a: String - b: Int - } -`); - -function executeQuery( - query: string, - rootValue: unknown, - variableValues?: { [variable: string]: unknown }, -): ExecutionResult | Promise { - return execute({ - schema, - document: parse(query, { experimentalFragmentArguments: true }), - rootValue, - variableValues, - }); -} - -describe('Execute: Handles OneOf Input Objects', () => { - describe('OneOf Input Objects', () => { - const rootValue = { - test({ input }: { input: { a?: string; b?: number } }) { - return input; - }, - }; - - it('accepts a good default value', () => { - const query = ` - query ($input: TestInputObject! = {a: "abc"}) { - test(input: $input) { - a - b - } - } - `; - const result = executeQuery(query, rootValue); - - expectJSON(result).toDeepEqual({ - data: { - test: { - a: 'abc', - b: null, - }, - }, - }); - }); - - it('rejects a bad default value', () => { - const query = ` - query ($input: TestInputObject! = {a: "abc", b: 123}) { - test(input: $input) { - a - b - } - } - `; - const result = executeQuery(query, rootValue); - - expectJSON(result).toDeepEqual({ - data: { - test: null, - }, - errors: [ - { - locations: [{ column: 23, line: 3 }], - 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.', - path: ['test'], - }, - ], - }); - }); - - it('accepts a good variable', () => { - const query = ` - query ($input: TestInputObject!) { - test(input: $input) { - a - b - } - } - `; - const result = executeQuery(query, rootValue, { input: { a: 'abc' } }); - - expectJSON(result).toDeepEqual({ - data: { - test: { - a: 'abc', - b: null, - }, - }, - }); - }); - - it('accepts a good variable with an undefined key', () => { - const query = ` - query ($input: TestInputObject!) { - test(input: $input) { - a - b - } - } - `; - const result = executeQuery(query, rootValue, { - input: { a: 'abc', b: undefined }, - }); - - expectJSON(result).toDeepEqual({ - data: { - test: { - a: 'abc', - b: null, - }, - }, - }); - }); - - it('rejects a variable with multiple non-null keys', () => { - const query = ` - query ($input: TestInputObject!) { - test(input: $input) { - a - b - } - } - `; - const result = executeQuery(query, rootValue, { - input: { a: 'abc', b: 123 }, - }); - - expectJSON(result).toDeepEqual({ - errors: [ - { - locations: [{ column: 16, line: 2 }], - message: - 'Variable "$input" got invalid value { a: "abc", b: 123 }; Within OneOf Input Object type "TestInputObject", exactly one field must be specified, and the value for that field must be non-null.', - }, - ], - }); - }); - - it('rejects a variable with multiple keys, some set to null', () => { - const query = ` - query ($input: TestInputObject!) { - test(input: $input) { - a - b - } - } - `; - const result = executeQuery(query, rootValue, { - input: { a: 'abc', b: null }, - }); - - expectJSON(result).toDeepEqual({ - errors: [ - { - locations: [{ column: 16, line: 2 }], - message: - 'Variable "$input" got invalid value { a: "abc", b: null }; Within OneOf Input Object type "TestInputObject", exactly one field must be specified, and the value for that field must be non-null.', - }, - ], - }); - }); - - it('rejects a variable with multiple null keys', () => { - const query = ` - query ($input: TestInputObject!) { - test(input: $input) { - a - b - } - } - `; - const result = executeQuery(query, rootValue, { - input: { a: null, b: null }, - }); - - expectJSON(result).toDeepEqual({ - errors: [ - { - locations: [{ column: 16, line: 2 }], - message: - 'Variable "$input" got invalid value { a: null, b: null }; Within OneOf Input Object type "TestInputObject", exactly one field must be specified, and the value for that field must be non-null.', - }, - ], - }); - }); - - it('accepts a valid variable for field', () => { - const query = ` - query ($a: String!) { - test(input: { a: $a }) { - a - b - } - } - `; - const result = executeQuery(query, rootValue, { a: 'abc' }); - - expectJSON(result).toDeepEqual({ - data: { - test: { - a: 'abc', - b: null, - }, - }, - }); - }); - - it('rejects multiple variables for fields', () => { - const query = ` - query ($a: String!, $b: Int!) { - test(input: { a: $a, b: $b }) { - a - b - } - } - `; - const result = executeQuery(query, rootValue, { a: 'abc', b: 123 }); - - 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" of type "TestInputObject!" has invalid value { a: $a, b: $b }.', - locations: [{ line: 3, column: 23 }], - path: ['test'], - }, - ], - }); - }); - - it('rejects variable for field explicitly set to undefined', () => { - const query = ` - query ($a: String) { - test(input: { a: $a }) { - a - b - } - } - `; - const result = executeQuery(query, rootValue, { a: undefined }); - - 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" of type "TestInputObject!" has invalid value { a: $a }.', - locations: [{ line: 3, column: 23 }], - path: ['test'], - }, - ], - }); - }); - - it('rejects 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" of type "TestInputObject!" has invalid value { a: $a }.', - locations: [{ line: 3, column: 23 }], - path: ['test'], - }, - ], - }); - }); - - it('rejects 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" of type "TestInputObject!" has invalid value { a: $a }.', - locations: [{ line: 3, column: 23 }], - path: ['test'], - }, - ], - }); - }); - - it('rejects missing second variable for field', () => { - const query = ` - query ($a: String, $b: String) { - test(input: { a: $a, b: $b }) { - a - b - } - } - `; - const result = executeQuery(query, rootValue, { a: '123' }); - - 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" of type "TestInputObject!" has invalid value { a: $a, b: $b }.', - locations: [{ line: 3, column: 23 }], - path: ['test'], - }, - ], - }); - }); - - it('accepts a valid fragment variable for field', () => { - const query = ` - query { - ...TestFragment(a: "abc") - } - fragment TestFragment($a: String) on Query { - test(input: { a: $a }) { - a - b - } - } - `; - const result = executeQuery(query, rootValue, { a: 'abc' }); - - expectJSON(result).toDeepEqual({ - data: { - test: { - a: 'abc', - b: null, - }, - }, - }); - }); - - it('rejects multiple fragment variables for fields', () => { - const query = ` - query { - ...TestFragment(a: "abc", b: 123) - } - fragment TestFragment($a: String, $b: Int) on Query { - test(input: { a: $a, b: $b }) { - a - b - } - } - `; - const result = executeQuery(query, rootValue, { a: 'abc', b: 123 }); - - 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" of type "TestInputObject!" has invalid value { a: $a, b: $b }.', - locations: [{ line: 6, column: 23 }], - path: ['test'], - }, - ], - }); - }); - - it('rejects 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" of type "TestInputObject!" has invalid value { a: $a }.', - locations: [{ line: 6, column: 23 }], - path: ['test'], - }, - ], - }); - }); - - it('rejects 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" of type "TestInputObject!" has invalid value { a: $a }.', - locations: [{ line: 6, column: 23 }], - path: ['test'], - }, - ], - }); - }); - - it('rejects missing second fragment variable for field', () => { - const query = ` - query { - ...TestFragment(a: "123") - } - fragment TestFragment($a: String, $b: string) on Query { - test(input: { a: $a, b: $b }) { - a - b - } - } - `; - const result = executeQuery(query, rootValue, { a: '123' }); - - 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" of type "TestInputObject!" has invalid value { a: $a, b: $b }.', - locations: [{ line: 6, column: 23 }], - path: ['test'], - }, - ], - }); - }); - }); -}); diff --git a/src/execution/__tests__/variables-test.ts b/src/execution/__tests__/variables-test.ts index 561eaf20c0..93dbd9c245 100644 --- a/src/execution/__tests__/variables-test.ts +++ b/src/execution/__tests__/variables-test.ts @@ -1328,6 +1328,47 @@ describe('Execute: Handles inputs', () => { ], }); }); + + it('accepts a good default value', () => { + const result = executeQuery( + ` + query ($input: TestOneOfInputObject = { a: "abc" }) { + fieldWithOneOfObjectInput(input: $input) + } + `, + ); + + expectJSON(result).toDeepEqual({ + data: { + fieldWithOneOfObjectInput: '{ a: "abc" }', + }, + }); + }); + + it('rejects a bad default value', () => { + const result = executeQuery( + ` + query ($input: TestOneOfInputObject = { a: "abc", b: 123 }) { + fieldWithOneOfObjectInput(input: $input) + } + `, + ); + + expectJSON(result).toDeepEqual({ + data: { + fieldWithOneOfObjectInput: null, + }, + errors: [ + { + locations: [{ line: 3, column: 46 }], + message: + // Default values are not currently validated separately, hence the vague error message here. + 'Argument "input" of type "TestOneOfInputObject" has invalid value $input.', + path: ['fieldWithOneOfObjectInput'], + }, + ], + }); + }); }); describe('getVariableValues: limit maximum number of coercion errors', () => { From d9342baeeae98f8f5a39f67e09a37157d544d333 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Sun, 22 Sep 2024 01:03:33 +0300 Subject: [PATCH 08/10] add non-normative note --- src/execution/__tests__/variables-test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/execution/__tests__/variables-test.ts b/src/execution/__tests__/variables-test.ts index 93dbd9c245..ea8e3ee8d6 100644 --- a/src/execution/__tests__/variables-test.ts +++ b/src/execution/__tests__/variables-test.ts @@ -1228,13 +1228,14 @@ describe('Execute: Handles inputs', () => { }); }); - it('allows a variable for the entire OneOf Object with a single defined value', () => { + it('allows a variable for the entire OneOf Object with a single defined value and additional undefined value', () => { const result = executeQuery( ` query ($input: TestOneOfInputObject) { fieldWithOneOfObjectInput(input: $input) } `, + // Note: this is non-normative graphql-js specific behavior. { input: { a: 'abc', b: undefined } }, ); From f2a6e9dcdbe15dc112457c984ca71c250fdc6998 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Sun, 22 Sep 2024 01:05:16 +0300 Subject: [PATCH 09/10] add less than one value case --- src/execution/__tests__/variables-test.ts | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/execution/__tests__/variables-test.ts b/src/execution/__tests__/variables-test.ts index ea8e3ee8d6..29d4348d89 100644 --- a/src/execution/__tests__/variables-test.ts +++ b/src/execution/__tests__/variables-test.ts @@ -1167,6 +1167,28 @@ describe('Execute: Handles inputs', () => { }); }); + it('errors with OneOf Input Object with no fields', () => { + const result = executeQuery(` + { + fieldWithOneOfObjectInput(input: {}) + } + `); + + expectJSON(result).toDeepEqual({ + data: { + fieldWithOneOfObjectInput: null, + }, + errors: [ + { + message: + 'Argument "input" of type "TestOneOfInputObject" has invalid value { }.', + path: ['fieldWithOneOfObjectInput'], + locations: [{ line: 3, column: 44 }], + }, + ], + }); + }); + it('errors with OneOf Input Object with a single null value', () => { const result = executeQuery(` { From cdd0b55529c994bfac336f25e9d4c09c80f41f33 Mon Sep 17 00:00:00 2001 From: Benjie Date: Thu, 17 Oct 2024 13:15:29 +0100 Subject: [PATCH 10/10] Apply suggestions from code review --- src/execution/__tests__/variables-test.ts | 10 +++++----- src/utilities/coerceInputValue.ts | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/execution/__tests__/variables-test.ts b/src/execution/__tests__/variables-test.ts index 29d4348d89..5810a77251 100644 --- a/src/execution/__tests__/variables-test.ts +++ b/src/execution/__tests__/variables-test.ts @@ -96,11 +96,11 @@ const TestNestedInputObject = new GraphQLInputObjectType({ const TestOneOfInputObject = new GraphQLInputObjectType({ name: 'TestOneOfInputObject', + isOneOf: true, fields: { a: { type: GraphQLString }, b: { type: GraphQLInt }, }, - isOneOf: true, }); const TestEnum = new GraphQLEnumType({ @@ -1268,7 +1268,7 @@ describe('Execute: Handles inputs', () => { }); }); - it('errors with variable with no value', () => { + it('errors with variable object with no fields', () => { const result = executeQuery( ` query ($input: TestOneOfInputObject) { @@ -1289,7 +1289,7 @@ describe('Execute: Handles inputs', () => { }); }); - it('errors with variable with multiple values', () => { + it('errors with variable object with multiple fields', () => { const result = executeQuery( ` query ($input: TestOneOfInputObject) { @@ -1310,7 +1310,7 @@ describe('Execute: Handles inputs', () => { }); }); - it('errors with variable with single null value', () => { + it('errors with variable object with single null field', () => { const result = executeQuery( ` query ($input: TestOneOfInputObject) { @@ -1331,7 +1331,7 @@ describe('Execute: Handles inputs', () => { }); }); - it('errors with variable with multiple values, only one non-null', () => { + it('errors with variable object with multiple fields, only one non-null', () => { const result = executeQuery( ` query ($input: TestOneOfInputObject) { diff --git a/src/utilities/coerceInputValue.ts b/src/utilities/coerceInputValue.ts index 4a8a8057fa..6d33386054 100644 --- a/src/utilities/coerceInputValue.ts +++ b/src/utilities/coerceInputValue.ts @@ -135,7 +135,7 @@ function coerceInputValueImpl( } /** - * keys with the value "undefined" do not appear within the coerced input + * Keys with the value "undefined" do not appear within the coerced input * object, but do appear within the original input object. * * Note: in graphql-js, supplying the value "undefined" for a variable