From 4ffa5faf572b26543c2de4b0ec505a9a2551b445 Mon Sep 17 00:00:00 2001 From: vojtapol Date: Wed, 20 May 2020 22:31:30 -0400 Subject: [PATCH 1/3] Revert to old behavior of Optional mapping by default --- .../kickstart/tools/MethodFieldResolver.kt | 5 ++--- .../kickstart/tools/SchemaParserOptions.kt | 21 ++++++++++++++++--- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/src/main/kotlin/graphql/kickstart/tools/MethodFieldResolver.kt b/src/main/kotlin/graphql/kickstart/tools/MethodFieldResolver.kt index 82fd0b79..bae66310 100644 --- a/src/main/kotlin/graphql/kickstart/tools/MethodFieldResolver.kt +++ b/src/main/kotlin/graphql/kickstart/tools/MethodFieldResolver.kt @@ -92,11 +92,10 @@ internal class MethodFieldResolver( } if (value == null && isOptional) { - if (environment.containsArgument(definition.name)) { - return@add Optional.empty() - } else { + if (options.inputArgumentOptionalNullWhenOmitted && !environment.containsArgument(definition.name)) { return@add null } + return@add Optional.empty() } if (value != null diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt index 8e29112a..8f5f3be9 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt @@ -25,6 +25,7 @@ data class SchemaParserOptions internal constructor( val allowUnimplementedResolvers: Boolean, val objectMapperProvider: PerFieldObjectMapperProvider, val proxyHandlers: List, + val inputArgumentOptionalNullWhenOmitted: Boolean, val preferGraphQLResolver: Boolean, val introspectionEnabled: Boolean, val coroutineContextProvider: CoroutineContextProvider, @@ -50,6 +51,7 @@ data class SchemaParserOptions internal constructor( private var allowUnimplementedResolvers = false private var objectMapperProvider: PerFieldObjectMapperProvider = PerFieldConfiguringObjectMapperProvider() private val proxyHandlers: MutableList = mutableListOf(Spring4AopProxyHandler(), GuiceAopProxyHandler(), JavassistProxyHandler(), WeldProxyHandler()) + private var inputArgumentOptionalNullWhenOmitted = false private var preferGraphQLResolver = false private var introspectionEnabled = true private var coroutineContextProvider: CoroutineContextProvider? = null @@ -80,6 +82,10 @@ data class SchemaParserOptions internal constructor( this.allowUnimplementedResolvers = allowUnimplementedResolvers } + fun inputArgumentOptionalNullWhenOmitted(inputArgumentOptionalNullWhenOmitted: Boolean) = this.apply { + this.inputArgumentOptionalNullWhenOmitted = inputArgumentOptionalNullWhenOmitted + } + fun preferGraphQLResolver(preferGraphQLResolver: Boolean) = this.apply { this.preferGraphQLResolver = preferGraphQLResolver } @@ -146,9 +152,18 @@ data class SchemaParserOptions internal constructor( genericWrappers } - return SchemaParserOptions(contextClass, wrappers, allowUnimplementedResolvers, objectMapperProvider, - proxyHandlers, preferGraphQLResolver, introspectionEnabled, coroutineContextProvider, - typeDefinitionFactories, fieldVisibility + return SchemaParserOptions( + contextClass, + wrappers, + allowUnimplementedResolvers, + objectMapperProvider, + proxyHandlers, + inputArgumentOptionalNullWhenOmitted, + preferGraphQLResolver, + introspectionEnabled, + coroutineContextProvider, + typeDefinitionFactories, + fieldVisibility ) } } From c0b3c6a772dd641b9b36e70f86c6e2da43fa4e25 Mon Sep 17 00:00:00 2001 From: vojtapol Date: Wed, 20 May 2020 22:39:27 -0400 Subject: [PATCH 2/3] Add tests --- .../tools/MethodFieldResolverTest.kt | 96 ++++++++++++++++++- 1 file changed, 93 insertions(+), 3 deletions(-) diff --git a/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt b/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt index 36535eb7..36d26ac7 100644 --- a/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt @@ -10,11 +10,101 @@ import org.junit.Test import java.lang.reflect.InvocationHandler import java.lang.reflect.Method import java.lang.reflect.Proxy +import java.util.* class MethodFieldResolverTest { @Test - fun shouldHandleScalarTypesAsMethodInputArgument() { + fun `should handle Optional type as method input argument`() { + val schema = SchemaParser.newParser() + .schemaString(""" + type Query { + testValue(input: String): String + testOmitted(input: String): String + testNull(input: String): String + } + """ + ) + .scalars(customScalarType) + .resolvers(object : GraphQLQueryResolver { + fun testValue(input: Optional) = input.toString() + fun testOmitted(input: Optional) = input.toString() + fun testNull(input: Optional) = input.toString() + }) + .build() + .makeExecutableSchema() + + val gql = GraphQL.newGraphQL(schema).build() + + val result = gql + .execute(ExecutionInput.newExecutionInput() + .query(""" + query { + testValue(input: "test-value") + testOmitted + testNull(input: null) + } + """) + .context(Object()) + .root(Object())) + + val expected = mapOf( + "testValue" to "Optional[test-value]", + "testOmitted" to "Optional.empty", + "testNull" to "Optional.empty" + ) + + Assert.assertEquals(expected, result.getData()) + } + + @Test + fun `should handle Optional type as method input argument with omission detection`() { + val schema = SchemaParser.newParser() + .schemaString(""" + type Query { + testValue(input: String): String + testOmitted(input: String): String + testNull(input: String): String + } + """ + ) + .scalars(customScalarType) + .resolvers(object : GraphQLQueryResolver { + fun testValue(input: Optional) = input.toString() + fun testOmitted(input: Optional?) = input.toString() + fun testNull(input: Optional) = input.toString() + }) + .options(SchemaParserOptions.newOptions() + .inputArgumentOptionalNullWhenOmitted(true) + .build()) + .build() + .makeExecutableSchema() + + val gql = GraphQL.newGraphQL(schema).build() + + val result = gql + .execute(ExecutionInput.newExecutionInput() + .query(""" + query { + testValue(input: "test-value") + testOmitted + testNull(input: null) + } + """) + .context(Object()) + .root(Object())) + + val expected = mapOf( + "testValue" to "Optional[test-value]", + "testOmitted" to "null", + "testNull" to "Optional.empty" + ) + + Assert.assertEquals(expected, result.getData()) + } + + @Test + fun `should handle scalar types as method input argument`() { val schema = SchemaParser.newParser() .schemaString(""" scalar CustomScalar @@ -47,7 +137,7 @@ class MethodFieldResolverTest { } @Test - fun shouldHandleListsOfScalarTypes() { + fun `should handle lists of scalar types`() { val schema = SchemaParser.newParser() .schemaString(""" scalar CustomScalar @@ -80,7 +170,7 @@ class MethodFieldResolverTest { } @Test - fun shouldHandleProxies() { + fun `should handle proxies`() { val invocationHandler = object : InvocationHandler { override fun invoke(proxy: Any, method: Method, args: Array): Any { return when (method.name) { From d0e79c9d0e7f4e2e7472bb1ffba883f3f51feec3 Mon Sep 17 00:00:00 2001 From: vojtapol Date: Wed, 20 May 2020 23:03:04 -0400 Subject: [PATCH 3/3] Change option name --- .../graphql/kickstart/tools/MethodFieldResolver.kt | 2 +- .../graphql/kickstart/tools/SchemaParserOptions.kt | 10 +++++----- .../graphql/kickstart/tools/MethodFieldResolverTest.kt | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/main/kotlin/graphql/kickstart/tools/MethodFieldResolver.kt b/src/main/kotlin/graphql/kickstart/tools/MethodFieldResolver.kt index bae66310..54628faa 100644 --- a/src/main/kotlin/graphql/kickstart/tools/MethodFieldResolver.kt +++ b/src/main/kotlin/graphql/kickstart/tools/MethodFieldResolver.kt @@ -92,7 +92,7 @@ internal class MethodFieldResolver( } if (value == null && isOptional) { - if (options.inputArgumentOptionalNullWhenOmitted && !environment.containsArgument(definition.name)) { + if (options.inputArgumentOptionalDetectOmission && !environment.containsArgument(definition.name)) { return@add null } return@add Optional.empty() diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt index 8f5f3be9..c7c9c82b 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaParserOptions.kt @@ -25,7 +25,7 @@ data class SchemaParserOptions internal constructor( val allowUnimplementedResolvers: Boolean, val objectMapperProvider: PerFieldObjectMapperProvider, val proxyHandlers: List, - val inputArgumentOptionalNullWhenOmitted: Boolean, + val inputArgumentOptionalDetectOmission: Boolean, val preferGraphQLResolver: Boolean, val introspectionEnabled: Boolean, val coroutineContextProvider: CoroutineContextProvider, @@ -51,7 +51,7 @@ data class SchemaParserOptions internal constructor( private var allowUnimplementedResolvers = false private var objectMapperProvider: PerFieldObjectMapperProvider = PerFieldConfiguringObjectMapperProvider() private val proxyHandlers: MutableList = mutableListOf(Spring4AopProxyHandler(), GuiceAopProxyHandler(), JavassistProxyHandler(), WeldProxyHandler()) - private var inputArgumentOptionalNullWhenOmitted = false + private var inputArgumentOptionalDetectOmission = false private var preferGraphQLResolver = false private var introspectionEnabled = true private var coroutineContextProvider: CoroutineContextProvider? = null @@ -82,8 +82,8 @@ data class SchemaParserOptions internal constructor( this.allowUnimplementedResolvers = allowUnimplementedResolvers } - fun inputArgumentOptionalNullWhenOmitted(inputArgumentOptionalNullWhenOmitted: Boolean) = this.apply { - this.inputArgumentOptionalNullWhenOmitted = inputArgumentOptionalNullWhenOmitted + fun inputArgumentOptionalDetectOmission(inputArgumentOptionalDetectOmission: Boolean) = this.apply { + this.inputArgumentOptionalDetectOmission = inputArgumentOptionalDetectOmission } fun preferGraphQLResolver(preferGraphQLResolver: Boolean) = this.apply { @@ -158,7 +158,7 @@ data class SchemaParserOptions internal constructor( allowUnimplementedResolvers, objectMapperProvider, proxyHandlers, - inputArgumentOptionalNullWhenOmitted, + inputArgumentOptionalDetectOmission, preferGraphQLResolver, introspectionEnabled, coroutineContextProvider, diff --git a/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt b/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt index 36d26ac7..d2d69bd7 100644 --- a/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/MethodFieldResolverTest.kt @@ -75,7 +75,7 @@ class MethodFieldResolverTest { fun testNull(input: Optional) = input.toString() }) .options(SchemaParserOptions.newOptions() - .inputArgumentOptionalNullWhenOmitted(true) + .inputArgumentOptionalDetectOmission(true) .build()) .build() .makeExecutableSchema()