From f11205448c990b0a2ad3ced48fa9dae055aa92ab Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Wed, 9 Aug 2023 13:27:59 +1000 Subject: [PATCH 1/2] deprecated default value method produces bad results --- .../schema/idl/SchemaPrinterTest.groovy | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/test/groovy/graphql/schema/idl/SchemaPrinterTest.groovy b/src/test/groovy/graphql/schema/idl/SchemaPrinterTest.groovy index e946a0627..7fdde6d46 100644 --- a/src/test/groovy/graphql/schema/idl/SchemaPrinterTest.groovy +++ b/src/test/groovy/graphql/schema/idl/SchemaPrinterTest.groovy @@ -2247,6 +2247,25 @@ type Query { type TestObjectB { field: String } +''' + } + + def "deprecated defaultValue on programmatic args prints as expected"() { + def queryObjType = newObject().name("Query") + .field(newFieldDefinition().name("f").type(GraphQLString) + .argument(newArgument().name("arg").type(GraphQLString).defaultValue(null))) + .build() + def schema = GraphQLSchema.newSchema().query(queryObjType).build() + + + when: + def options = defaultOptions().includeDirectiveDefinitions(false) + def sdl = new SchemaPrinter(options).print(schema) + then: + sdl == ''' +type Query { + f(arg: String = null): String +} ''' } } \ No newline at end of file From b79540b1db0a9fffe02765205762391c1271673b Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Thu, 10 Aug 2023 14:13:19 +1000 Subject: [PATCH 2/2] Fixed the ValuesResolverConversion to handle programmatically set nulls --- .../execution/ValuesResolverConversion.java | 8 +- .../introspection/IntrospectionTest.groovy | 86 ++++++++++++------- .../schema/idl/SchemaPrinterTest.groovy | 5 +- 3 files changed, 63 insertions(+), 36 deletions(-) diff --git a/src/main/java/graphql/execution/ValuesResolverConversion.java b/src/main/java/graphql/execution/ValuesResolverConversion.java index 83f5285b0..eff4d1cef 100644 --- a/src/main/java/graphql/execution/ValuesResolverConversion.java +++ b/src/main/java/graphql/execution/ValuesResolverConversion.java @@ -69,11 +69,17 @@ static Object valueToLiteralImpl(GraphqlFieldVisibility fieldVisibility, if (valueMode == NORMALIZED) { return assertShouldNeverHappen("can't infer normalized structure"); } - return ValuesResolverLegacy.valueToLiteralLegacy( + Value value = ValuesResolverLegacy.valueToLiteralLegacy( inputValueWithState.getValue(), type, graphqlContext, locale); + // + // the valueToLiteralLegacy() nominally cant know if null means never set or is set to a null value + // but this code can know - its is SET to a value so, it MUST be a Null Literal + // this method would assert at the end of it if inputValueWithState.isNotSet() were true + // + return value == null ? NullValue.of() : value; } if (inputValueWithState.isLiteral()) { return inputValueWithState.getValue(); diff --git a/src/test/groovy/graphql/introspection/IntrospectionTest.groovy b/src/test/groovy/graphql/introspection/IntrospectionTest.groovy index 712ac8048..bee6b161e 100644 --- a/src/test/groovy/graphql/introspection/IntrospectionTest.groovy +++ b/src/test/groovy/graphql/introspection/IntrospectionTest.groovy @@ -1,5 +1,6 @@ package graphql.introspection + import graphql.TestUtil import graphql.schema.DataFetcher import graphql.schema.FieldCoordinates @@ -438,7 +439,7 @@ class IntrospectionTest extends Specification { def "test AST printed introspection query is equivalent to original string"() { when: - def oldIntrospectionQuery = "\n" + + def oldIntrospectionQuery = "\n" + " query IntrospectionQuery {\n" + " __schema {\n" + " queryType { name }\n" + @@ -540,12 +541,11 @@ class IntrospectionTest extends Specification { " }\n" + "\n" - def newIntrospectionQuery = IntrospectionQuery.INTROSPECTION_QUERY; + def newIntrospectionQuery = IntrospectionQuery.INTROSPECTION_QUERY then: - oldIntrospectionQuery.replaceAll("\\s+","").equals( - newIntrospectionQuery.replaceAll("\\s+","") - ) + oldIntrospectionQuery.replaceAll("\\s+", "") == + newIntrospectionQuery.replaceAll("\\s+", "") } def "test parameterized introspection queries"() { @@ -582,44 +582,66 @@ class IntrospectionTest extends Specification { def parseExecutionResult = { [ - it.data["__schema"]["types"].find{it["name"] == "Query"}["fields"].find{it["name"] == "notDeprecated"}["description"] != null, // descriptions is true - it.data["__schema"]["types"].find{it["name"] == "UUID"}["specifiedByURL"] != null, // specifiedByUrl is true - it.data["__schema"]["directives"].find{it["name"] == "repeatableDirective"}["isRepeatable"] != null, // directiveIsRepeatable is true - it.data["__schema"]["description"] != null, // schemaDescription is true - it.data["__schema"]["types"].find { it['name'] == 'InputType' }["inputFields"].find({ it["name"] == "inputField" }) != null // inputValueDeprecation is true + it.data["__schema"]["types"].find { it["name"] == "Query" }["fields"].find { it["name"] == "notDeprecated" }["description"] != null, // descriptions is true + it.data["__schema"]["types"].find { it["name"] == "UUID" }["specifiedByURL"] != null, // specifiedByUrl is true + it.data["__schema"]["directives"].find { it["name"] == "repeatableDirective" }["isRepeatable"] != null, // directiveIsRepeatable is true + it.data["__schema"]["description"] != null, // schemaDescription is true + it.data["__schema"]["types"].find { it['name'] == 'InputType' }["inputFields"].find({ it["name"] == "inputField" }) != null // inputValueDeprecation is true ] } when: - def allFalseExecutionResult = graphQL.execute( + def allFalseExecutionResult = graphQL.execute( IntrospectionQueryBuilder.build( - IntrospectionQueryBuilder.Options.defaultOptions() - .descriptions(false) - .specifiedByUrl(false) - .directiveIsRepeatable(false) - .schemaDescription(false) - .inputValueDeprecation(false) - .typeRefFragmentDepth(5) + IntrospectionQueryBuilder.Options.defaultOptions() + .descriptions(false) + .specifiedByUrl(false) + .directiveIsRepeatable(false) + .schemaDescription(false) + .inputValueDeprecation(false) + .typeRefFragmentDepth(5) ) - ) + ) then: - !parseExecutionResult(allFalseExecutionResult).any() - allFalseExecutionResult.data["__schema"]["types"].find{it["name"] == "Query"}["fields"].find{it["name"] == "tenDimensionalList"}["type"]["ofType"]["ofType"]["ofType"]["ofType"]["ofType"]["ofType"] == null // typeRefFragmentDepth is 5 + !parseExecutionResult(allFalseExecutionResult).any() + allFalseExecutionResult.data["__schema"]["types"].find { it["name"] == "Query" }["fields"].find { it["name"] == "tenDimensionalList" }["type"]["ofType"]["ofType"]["ofType"]["ofType"]["ofType"]["ofType"] == null // typeRefFragmentDepth is 5 when: - def allTrueExecutionResult = graphQL.execute( + def allTrueExecutionResult = graphQL.execute( IntrospectionQueryBuilder.build( - IntrospectionQueryBuilder.Options.defaultOptions() - .descriptions(true) - .specifiedByUrl(true) - .directiveIsRepeatable(true) - .schemaDescription(true) - .inputValueDeprecation(true) - .typeRefFragmentDepth(7) + IntrospectionQueryBuilder.Options.defaultOptions() + .descriptions(true) + .specifiedByUrl(true) + .directiveIsRepeatable(true) + .schemaDescription(true) + .inputValueDeprecation(true) + .typeRefFragmentDepth(7) ) - ) + ) + then: + parseExecutionResult(allTrueExecutionResult).every() + allTrueExecutionResult.data["__schema"]["types"].find { it["name"] == "Query" }["fields"].find { it["name"] == "tenDimensionalList" }["type"]["ofType"]["ofType"]["ofType"]["ofType"]["ofType"]["ofType"]["ofType"]["ofType"] == null // typeRefFragmentDepth is 7 + } + + def "issue 3285 - deprecated defaultValue on programmatic args prints AST literal as expected"() { + def queryObjType = newObject().name("Query") + .field(newFieldDefinition().name("f").type(GraphQLString) + .argument(newArgument().name("arg").type(GraphQLString).defaultValue(null))) + .build() + def schema = newSchema().query(queryObjType).build() + def graphQL = newGraphQL(schema).build() + + + when: + def executionResult = graphQL.execute(IntrospectionQuery.INTROSPECTION_QUERY) then: - parseExecutionResult(allTrueExecutionResult).every() - allTrueExecutionResult.data["__schema"]["types"].find{it["name"] == "Query"}["fields"].find{it["name"] == "tenDimensionalList"}["type"]["ofType"]["ofType"]["ofType"]["ofType"]["ofType"]["ofType"]["ofType"]["ofType"] == null // typeRefFragmentDepth is 7 + executionResult.errors.isEmpty() + + def types = executionResult.data['__schema']['types'] as List + def queryType = types.find { it['name'] == 'Query' } + def fField = (queryType['fields'] as List)[0] + def arg = (fField['args'] as List)[0] + arg['name'] == "arg" + arg['defaultValue'] == "null" // printed AST } } diff --git a/src/test/groovy/graphql/schema/idl/SchemaPrinterTest.groovy b/src/test/groovy/graphql/schema/idl/SchemaPrinterTest.groovy index 7fdde6d46..cff7a5e6e 100644 --- a/src/test/groovy/graphql/schema/idl/SchemaPrinterTest.groovy +++ b/src/test/groovy/graphql/schema/idl/SchemaPrinterTest.groovy @@ -2250,7 +2250,7 @@ type TestObjectB { ''' } - def "deprecated defaultValue on programmatic args prints as expected"() { + def "issue 3285 - deprecated defaultValue on programmatic args prints as expected"() { def queryObjType = newObject().name("Query") .field(newFieldDefinition().name("f").type(GraphQLString) .argument(newArgument().name("arg").type(GraphQLString).defaultValue(null))) @@ -2262,8 +2262,7 @@ type TestObjectB { def options = defaultOptions().includeDirectiveDefinitions(false) def sdl = new SchemaPrinter(options).print(schema) then: - sdl == ''' -type Query { + sdl == '''type Query { f(arg: String = null): String } '''