From 71a9e7229fc17bdf60f070ff51d8f790484918e2 Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Fri, 25 Feb 2022 18:54:12 +1100 Subject: [PATCH 1/3] A test to show the bug --- .../graphql/ParseAndValidateTest.groovy | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/test/groovy/graphql/ParseAndValidateTest.groovy b/src/test/groovy/graphql/ParseAndValidateTest.groovy index 8a4378b67..b39af0b4f 100644 --- a/src/test/groovy/graphql/ParseAndValidateTest.groovy +++ b/src/test/groovy/graphql/ParseAndValidateTest.groovy @@ -1,6 +1,12 @@ package graphql +import graphql.language.Document import graphql.parser.InvalidSyntaxException +import graphql.parser.Parser +import graphql.schema.GraphQLSchema +import graphql.schema.idl.SchemaParser +import graphql.schema.idl.TypeDefinitionRegistry +import graphql.schema.idl.UnExecutableSchemaGenerator import graphql.validation.ValidationError import graphql.validation.ValidationErrorType import graphql.validation.rules.NoUnusedFragments @@ -155,4 +161,30 @@ class ParseAndValidateTest extends Specification { then: !rs.errors.isEmpty() // all rules apply - we have errors } + + def "issue 2740 - evidence of not working"() { + def sdl = ''' + type Query { + myquery : String! + } + ''' + + def registry = new SchemaParser().parse(sdl) + def schema = UnExecutableSchemaGenerator.makeUnExecutableSchema(registry) + def graphQL = GraphQL.newGraphQL(schema).build() + + String request = "mutation MyMutation { mymutation }" + + when: + def er = graphQL.execute(request) + then: + er.errors.size() == 1 + + when: + Document inputDocument = new Parser().parseDocument(request) + List errors = ParseAndValidate.validate(schema, inputDocument) + + then: + errors.size() == 1 + } } From 48414326f9d525c7bfff6f86c2299d605213074b Mon Sep 17 00:00:00 2001 From: Felipe Reis Date: Fri, 20 Oct 2023 16:18:21 +1100 Subject: [PATCH 2/3] @oneOf handles non-null inputs --- .../graphql/execution/ValuesResolver.java | 6 +- .../execution/ValuesResolverTest.groovy | 55 ++++++++++++++++++- 2 files changed, 56 insertions(+), 5 deletions(-) diff --git a/src/main/java/graphql/execution/ValuesResolver.java b/src/main/java/graphql/execution/ValuesResolver.java index cc59de6f8..27487ca55 100644 --- a/src/main/java/graphql/execution/ValuesResolver.java +++ b/src/main/java/graphql/execution/ValuesResolver.java @@ -27,6 +27,7 @@ import graphql.schema.GraphQLScalarType; import graphql.schema.GraphQLSchema; import graphql.schema.GraphQLType; +import graphql.schema.GraphQLTypeUtil; import graphql.schema.InputValueWithState; import graphql.schema.visibility.GraphqlFieldVisibility; import org.jetbrains.annotations.NotNull; @@ -376,8 +377,9 @@ private static Map getArgumentValuesImpl( coercedValues.put(argumentName, value); } // @oneOf input must be checked now that all variables and literals have been converted - if (argumentType instanceof GraphQLInputObjectType) { - GraphQLInputObjectType inputObjectType = (GraphQLInputObjectType) argumentType; + GraphQLType unwrappedType = GraphQLTypeUtil.unwrapNonNull(argumentType); + if (unwrappedType instanceof GraphQLInputObjectType) { + GraphQLInputObjectType inputObjectType = (GraphQLInputObjectType) unwrappedType; if (inputObjectType.isOneOf() && ! ValuesResolverConversion.isNullValue(value)) { validateOneOfInputTypes(inputObjectType, argumentValue, argumentName, value, locale); } diff --git a/src/test/groovy/graphql/execution/ValuesResolverTest.groovy b/src/test/groovy/graphql/execution/ValuesResolverTest.groovy index dea17a2c4..0e53791a3 100644 --- a/src/test/groovy/graphql/execution/ValuesResolverTest.groovy +++ b/src/test/groovy/graphql/execution/ValuesResolverTest.groovy @@ -23,6 +23,7 @@ import graphql.language.Value import graphql.language.VariableDefinition import graphql.language.VariableReference import graphql.schema.CoercingParseValueException +import graphql.schema.GraphQLNonNull import spock.lang.Specification import spock.lang.Unroll @@ -360,16 +361,26 @@ class ValuesResolverTest extends Specification { .type(GraphQLInt) .build()) .build() - def fieldArgument = newArgument().name("arg").type(inputObjectType).build() - when: def argument = new Argument("arg", inputValue) + + when: + def fieldArgument = newArgument().name("arg").type(inputObjectType).build() ValuesResolver.getArgumentValues([fieldArgument], [argument], variables, graphQLContext, locale) then: def e = thrown(OneOfTooManyKeysException) e.message == "Exactly one key must be specified for OneOf type 'oneOfInputObject'." + when: "input type is wrapped in non-null" + def nonNullInputObjectType = GraphQLNonNull.nonNull(inputObjectType) + def fieldArgumentNonNull = newArgument().name("arg").type(nonNullInputObjectType).build() + ValuesResolver.getArgumentValues([fieldArgumentNonNull], [argument], variables, graphQLContext, locale) + + then: + def eNonNull = thrown(OneOfTooManyKeysException) + eNonNull.message == "Exactly one key must be specified for OneOf type 'oneOfInputObject'." + where: // from https://github.com/graphql/graphql-spec/pull/825/files#diff-30a69c5a5eded8e1aea52e53dad1181e6ec8f549ca2c50570b035153e2de1c43R1692 testCase | inputValue | variables @@ -502,6 +513,44 @@ class ValuesResolverTest extends Specification { } + def "getArgumentValues: invalid oneOf input no values where passed - #testCase"() { + given: "schema defining input object" + def inputObjectType = newInputObject() + .name("oneOfInputObject") + .withAppliedDirective(Directives.OneOfDirective.toAppliedDirective()) + .field(newInputObjectField() + .name("a") + .type(GraphQLString) + .build()) + .field(newInputObjectField() + .name("b") + .type(GraphQLInt) + .build()) + .build() + def fieldArgument = newArgument().name("arg").type(inputObjectType).build() + + when: + def argument = new Argument("arg", inputValue) + ValuesResolver.getArgumentValues([fieldArgument], [argument], variables, graphQLContext, locale) + + then: + def e = thrown(OneOfNullValueException) + e.message == "OneOf type field 'oneOfInputObject.a' must be non-null." + + where: + // from https://github.com/graphql/graphql-spec/pull/825/files#diff-30a69c5a5eded8e1aea52e53dad1181e6ec8f549ca2c50570b035153e2de1c43R1692 + testCase | inputValue | variables + + '`{ a: null }` {}' | buildObjectLiteral([ + a: NullValue.of() + ]) | CoercedVariables.emptyVariables() + + '`{ a: $var }` { var : null}' | buildObjectLiteral([ + a: VariableReference.of("var") + ]) | CoercedVariables.of(["var": null]) + + } + def "getVariableValues: enum as variable input"() { given: def enumDef = newEnum() @@ -839,4 +888,4 @@ class ValuesResolverTest extends Specification { executionResult.errors[0].message == "Variable 'input' has an invalid value: Expected a value that can be converted to type 'Float' but it was a 'String'" executionResult.errors[0].locations == [new SourceLocation(2, 35)] } -} \ No newline at end of file +} From 3f03bd11e788f08f7d350fd557710cc1ce19c5f6 Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Tue, 24 Oct 2023 15:47:33 +1100 Subject: [PATCH 3/3] Revert "A test to show that validation rules do not catch a mutation query to schema mismatch" --- .../graphql/ParseAndValidateTest.groovy | 32 ------------------- 1 file changed, 32 deletions(-) diff --git a/src/test/groovy/graphql/ParseAndValidateTest.groovy b/src/test/groovy/graphql/ParseAndValidateTest.groovy index a5ffd6071..949b4aeb5 100644 --- a/src/test/groovy/graphql/ParseAndValidateTest.groovy +++ b/src/test/groovy/graphql/ParseAndValidateTest.groovy @@ -1,12 +1,6 @@ package graphql -import graphql.language.Document import graphql.parser.InvalidSyntaxException -import graphql.parser.Parser -import graphql.schema.GraphQLSchema -import graphql.schema.idl.SchemaParser -import graphql.schema.idl.TypeDefinitionRegistry -import graphql.schema.idl.UnExecutableSchemaGenerator import graphql.validation.ValidationError import graphql.validation.ValidationErrorType import graphql.validation.rules.NoUnusedFragments @@ -161,30 +155,4 @@ class ParseAndValidateTest extends Specification { then: !rs.errors.isEmpty() // all rules apply - we have errors } - - def "issue 2740 - evidence of not working"() { - def sdl = ''' - type Query { - myquery : String! - } - ''' - - def registry = new SchemaParser().parse(sdl) - def schema = UnExecutableSchemaGenerator.makeUnExecutableSchema(registry) - def graphQL = GraphQL.newGraphQL(schema).build() - - String request = "mutation MyMutation { mymutation }" - - when: - def er = graphQL.execute(request) - then: - er.errors.size() == 1 - - when: - Document inputDocument = new Parser().parseDocument(request) - List errors = ParseAndValidate.validate(schema, inputDocument) - - then: - errors.size() == 1 - } }