From 97d55eff6d16e1e9a1587c1863b414cc1f3d0bc9 Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Sun, 27 Mar 2022 10:45:26 +1100 Subject: [PATCH 1/3] Add full error message for NonNullableValueCoercedAsNullException --- .../NonNullableValueCoercedAsNullException.java | 12 ++++++++++++ src/main/java/graphql/execution/ValuesResolver.java | 4 +++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/main/java/graphql/execution/NonNullableValueCoercedAsNullException.java b/src/main/java/graphql/execution/NonNullableValueCoercedAsNullException.java index 5a9a5ca3ea..f593c7f53c 100644 --- a/src/main/java/graphql/execution/NonNullableValueCoercedAsNullException.java +++ b/src/main/java/graphql/execution/NonNullableValueCoercedAsNullException.java @@ -43,6 +43,18 @@ public NonNullableValueCoercedAsNullException(VariableDefinition variableDefinit this.path = path; } + public NonNullableValueCoercedAsNullException(List path, GraphQLType graphQLType) { + super(format("Coerced Null value for NonNull type '%s'", + GraphQLTypeUtil.simplePrint(graphQLType))); + this.path = path; + } + + public NonNullableValueCoercedAsNullException(VariableDefinition variableDefinition, String causeMessage) { + super(format("Variable '%s' has invalid value: %s", + variableDefinition.getName(), causeMessage)); + this.sourceLocations = Collections.singletonList(variableDefinition.getSourceLocation()); + } + public NonNullableValueCoercedAsNullException(String fieldName, List path, GraphQLType graphQLType) { super(format("Field '%s' has coerced Null value for NonNull type '%s'", fieldName, GraphQLTypeUtil.simplePrint(graphQLType))); diff --git a/src/main/java/graphql/execution/ValuesResolver.java b/src/main/java/graphql/execution/ValuesResolver.java index b38a1086f9..0867394b74 100644 --- a/src/main/java/graphql/execution/ValuesResolver.java +++ b/src/main/java/graphql/execution/ValuesResolver.java @@ -418,6 +418,8 @@ private Map externalValueToInternalValueForVariables(GraphQLSche .cause(e.getCause()) .sourceLocation(variableDefinition.getSourceLocation()) .build(); + } catch (NonNullableValueCoercedAsNullException e) { + throw new NonNullableValueCoercedAsNullException(variableDefinition, e.getMessage()); } } @@ -495,7 +497,7 @@ private Object externalValueToInternalValue(GraphqlFieldVisibility fieldVisibili Object returnValue = externalValueToInternalValue(fieldVisibility, unwrapOne(graphQLType), value); if (returnValue == null) { - throw new NonNullableValueCoercedAsNullException("", emptyList(), graphQLType); + throw new NonNullableValueCoercedAsNullException(emptyList(), graphQLType); } return returnValue; } From 99a73f0cb36c97efcde94b03b4c7f9996a446b07 Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Sun, 27 Mar 2022 10:46:10 +1100 Subject: [PATCH 2/3] Add tests for improved NonNullableValueCoercedAsNullException --- src/test/groovy/graphql/Issue743.groovy | 2 +- .../graphql/NullVariableCoercionTest.groovy | 6 +++--- .../execution/ValuesResolverTest.groovy | 20 ++++++++++++++++++- 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/src/test/groovy/graphql/Issue743.groovy b/src/test/groovy/graphql/Issue743.groovy index 183d63c800..09e9ea9437 100644 --- a/src/test/groovy/graphql/Issue743.groovy +++ b/src/test/groovy/graphql/Issue743.groovy @@ -28,6 +28,6 @@ class Issue743 extends Specification { executionResult.data == null executionResult.errors.size() == 1 executionResult.errors[0].errorType == ErrorType.ValidationError - executionResult.errors[0].message == "Variable 'isTrue' has coerced Null value for NonNull type 'Boolean!'" + executionResult.errors[0].message == "Variable 'isTrue' has invalid value: Variable 'isTrue' has coerced Null value for NonNull type 'Boolean!'" } } diff --git a/src/test/groovy/graphql/NullVariableCoercionTest.groovy b/src/test/groovy/graphql/NullVariableCoercionTest.groovy index 253b9a58c8..fcc8ccb5ef 100644 --- a/src/test/groovy/graphql/NullVariableCoercionTest.groovy +++ b/src/test/groovy/graphql/NullVariableCoercionTest.groovy @@ -1,6 +1,6 @@ package graphql - +import graphql.language.SourceLocation import graphql.schema.DataFetcher import graphql.schema.GraphQLObjectType import graphql.schema.idl.RuntimeWiring @@ -68,8 +68,8 @@ class NullVariableCoercionTest extends Specification { varResult.data == null varResult.errors.size() == 1 varResult.errors[0].errorType == ErrorType.ValidationError - varResult.errors[0].message == "Field 'baz' has coerced Null value for NonNull type 'String!'" -// varResult.errors[0].locations == [new SourceLocation(1, 11)] + varResult.errors[0].message == "Variable 'input' has invalid value: Field 'baz' has coerced Null value for NonNull type 'String!'" + varResult.errors[0].locations == [new SourceLocation(1, 11)] } def "can handle defaulting on complex input objects"() { diff --git a/src/test/groovy/graphql/execution/ValuesResolverTest.groovy b/src/test/groovy/graphql/execution/ValuesResolverTest.groovy index 83a3577256..22c7007c65 100644 --- a/src/test/groovy/graphql/execution/ValuesResolverTest.groovy +++ b/src/test/groovy/graphql/execution/ValuesResolverTest.groovy @@ -495,7 +495,25 @@ class ValuesResolverTest extends Specification { then: def error = thrown(NonNullableValueCoercedAsNullException) - error.message == "Variable 'foo' has coerced Null value for NonNull type 'String!'" + error.message == "Variable 'foo' has invalid value: Variable 'foo' has coerced Null value for NonNull type 'String!'" + } + + def "coerceVariableValues: if variableType is a list of Non-Nullable type, and element value is null, throw a query error"() { + given: + def schema = TestUtil.schemaWithInputType(list(nonNull(GraphQLString))) + + def defaultValueForFoo = new ArrayValue([new StringValue("defaultValueForFoo")]) + def type = new ListType(new NonNullType(new TypeName("String"))) + VariableDefinition fooVarDef = new VariableDefinition("foo", type, defaultValueForFoo) + + def variableValuesMap = ["foo": [null]] + + when: + resolver.coerceVariableValues(schema, [fooVarDef], variableValuesMap) + + then: + def error = thrown(NonNullableValueCoercedAsNullException) + error.message == "Variable 'foo' has invalid value: Coerced Null value for NonNull type 'String!'" } // Note: use NullValue defined in Field when it exists, From e5db4d557ea9fc135fdea3532897f16392cc1857 Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Sun, 27 Mar 2022 16:19:21 +1100 Subject: [PATCH 3/3] Remove empty path and improve grammar --- .../NonNullableValueCoercedAsNullException.java | 9 +++------ src/main/java/graphql/execution/ValuesResolver.java | 2 +- src/test/groovy/graphql/Issue743.groovy | 2 +- src/test/groovy/graphql/NullVariableCoercionTest.groovy | 2 +- .../groovy/graphql/execution/ValuesResolverTest.groovy | 4 ++-- 5 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/main/java/graphql/execution/NonNullableValueCoercedAsNullException.java b/src/main/java/graphql/execution/NonNullableValueCoercedAsNullException.java index f593c7f53c..4dfb680815 100644 --- a/src/main/java/graphql/execution/NonNullableValueCoercedAsNullException.java +++ b/src/main/java/graphql/execution/NonNullableValueCoercedAsNullException.java @@ -43,15 +43,12 @@ public NonNullableValueCoercedAsNullException(VariableDefinition variableDefinit this.path = path; } - public NonNullableValueCoercedAsNullException(List path, GraphQLType graphQLType) { - super(format("Coerced Null value for NonNull type '%s'", - GraphQLTypeUtil.simplePrint(graphQLType))); - this.path = path; + public NonNullableValueCoercedAsNullException(GraphQLType graphQLType) { + super(format("Coerced Null value for NonNull type '%s'", GraphQLTypeUtil.simplePrint(graphQLType))); } public NonNullableValueCoercedAsNullException(VariableDefinition variableDefinition, String causeMessage) { - super(format("Variable '%s' has invalid value: %s", - variableDefinition.getName(), causeMessage)); + super(format("Variable '%s' has an invalid value: %s", variableDefinition.getName(), causeMessage)); this.sourceLocations = Collections.singletonList(variableDefinition.getSourceLocation()); } diff --git a/src/main/java/graphql/execution/ValuesResolver.java b/src/main/java/graphql/execution/ValuesResolver.java index 0867394b74..23c51a5525 100644 --- a/src/main/java/graphql/execution/ValuesResolver.java +++ b/src/main/java/graphql/execution/ValuesResolver.java @@ -497,7 +497,7 @@ private Object externalValueToInternalValue(GraphqlFieldVisibility fieldVisibili Object returnValue = externalValueToInternalValue(fieldVisibility, unwrapOne(graphQLType), value); if (returnValue == null) { - throw new NonNullableValueCoercedAsNullException(emptyList(), graphQLType); + throw new NonNullableValueCoercedAsNullException(graphQLType); } return returnValue; } diff --git a/src/test/groovy/graphql/Issue743.groovy b/src/test/groovy/graphql/Issue743.groovy index 09e9ea9437..b6466c65d3 100644 --- a/src/test/groovy/graphql/Issue743.groovy +++ b/src/test/groovy/graphql/Issue743.groovy @@ -28,6 +28,6 @@ class Issue743 extends Specification { executionResult.data == null executionResult.errors.size() == 1 executionResult.errors[0].errorType == ErrorType.ValidationError - executionResult.errors[0].message == "Variable 'isTrue' has invalid value: Variable 'isTrue' has coerced Null value for NonNull type 'Boolean!'" + executionResult.errors[0].message == "Variable 'isTrue' has an invalid value: Variable 'isTrue' has coerced Null value for NonNull type 'Boolean!'" } } diff --git a/src/test/groovy/graphql/NullVariableCoercionTest.groovy b/src/test/groovy/graphql/NullVariableCoercionTest.groovy index fcc8ccb5ef..ffd42ac362 100644 --- a/src/test/groovy/graphql/NullVariableCoercionTest.groovy +++ b/src/test/groovy/graphql/NullVariableCoercionTest.groovy @@ -68,7 +68,7 @@ class NullVariableCoercionTest extends Specification { varResult.data == null varResult.errors.size() == 1 varResult.errors[0].errorType == ErrorType.ValidationError - varResult.errors[0].message == "Variable 'input' has invalid value: Field 'baz' has coerced Null value for NonNull type 'String!'" + varResult.errors[0].message == "Variable 'input' has an invalid value: Field 'baz' has coerced Null value for NonNull type 'String!'" varResult.errors[0].locations == [new SourceLocation(1, 11)] } diff --git a/src/test/groovy/graphql/execution/ValuesResolverTest.groovy b/src/test/groovy/graphql/execution/ValuesResolverTest.groovy index 22c7007c65..783fb1cdc1 100644 --- a/src/test/groovy/graphql/execution/ValuesResolverTest.groovy +++ b/src/test/groovy/graphql/execution/ValuesResolverTest.groovy @@ -495,7 +495,7 @@ class ValuesResolverTest extends Specification { then: def error = thrown(NonNullableValueCoercedAsNullException) - error.message == "Variable 'foo' has invalid value: Variable 'foo' has coerced Null value for NonNull type 'String!'" + error.message == "Variable 'foo' has an invalid value: Variable 'foo' has coerced Null value for NonNull type 'String!'" } def "coerceVariableValues: if variableType is a list of Non-Nullable type, and element value is null, throw a query error"() { @@ -513,7 +513,7 @@ class ValuesResolverTest extends Specification { then: def error = thrown(NonNullableValueCoercedAsNullException) - error.message == "Variable 'foo' has invalid value: Coerced Null value for NonNull type 'String!'" + error.message == "Variable 'foo' has an invalid value: Coerced Null value for NonNull type 'String!'" } // Note: use NullValue defined in Field when it exists,