diff --git a/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt b/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt index b76548dc..c7a9920e 100644 --- a/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt +++ b/src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt @@ -14,8 +14,6 @@ import graphql.schema.DataFetchingEnvironment import graphql.schema.GraphQLTypeUtil.isScalar import kotlinx.coroutines.future.future import java.lang.reflect.Method -import java.lang.reflect.ParameterizedType -import java.lang.reflect.WildcardType import java.util.* import kotlin.coroutines.intrinsics.suspendCoroutineUninterceptedOrReturn import kotlin.reflect.full.valueParameters @@ -82,15 +80,13 @@ internal class MethodFieldResolver( return@add Optional.empty() } - if (value != null - && parameterType.unwrap().isAssignableFrom(value.javaClass) - && isScalarType(environment, definition.type, parameterType)) { - return@add value + if (value != null && shouldValueBeConverted(value, definition, parameterType, environment)) { + return@add mapper.convertValue(value, object : TypeReference() { + override fun getType() = parameterType + }) } - return@add mapper.convertValue(value, object : TypeReference() { - override fun getType() = parameterType - }) + return@add value } } @@ -110,22 +106,25 @@ internal class MethodFieldResolver( } } - private fun isScalarType(environment: DataFetchingEnvironment, type: Type<*>, genericParameterType: JavaType): Boolean = - when (type) { + private fun shouldValueBeConverted(value: Any, definition: InputValueDefinition, parameterType: JavaType, environment: DataFetchingEnvironment): Boolean { + return !parameterType.unwrap().isAssignableFrom(value.javaClass) || !isConcreteScalarType(environment, definition.type, parameterType) + } + + /** + * A concrete scalar type is a scalar type where values always coerce to the same Java type. The ID scalar type is not concrete + * because values can be coerced to multiple different Java types (eg. String, Long, UUID). All values of a non-concrete scalar + * type must be converted to the target method parameter type. + */ + private fun isConcreteScalarType(environment: DataFetchingEnvironment, type: Type<*>, genericParameterType: JavaType): Boolean { + return when (type) { is ListType -> List::class.java.isAssignableFrom(this.genericType.getRawClass(genericParameterType)) - && isScalarType(environment, type.type, this.genericType.unwrapGenericType(genericParameterType)) - is TypeName -> environment.graphQLSchema?.getType(type.name)?.let { isScalar(it) && !isJavaLanguageType(genericParameterType) } + && isConcreteScalarType(environment, type.type, this.genericType.unwrapGenericType(genericParameterType)) + is TypeName -> environment.graphQLSchema?.getType(type.name)?.let { isScalar(it) && type.name != "ID" } ?: false - is NonNullType -> isScalarType(environment, type.type, genericParameterType) + is NonNullType -> isConcreteScalarType(environment, type.type, genericParameterType) else -> false } - - private fun isJavaLanguageType(type: JavaType): Boolean = - when (type) { - is ParameterizedType -> isJavaLanguageType(type.actualTypeArguments[0]) - is WildcardType -> isJavaLanguageType(type.upperBounds[0]) - else -> genericType.getRawClass(type).`package`.name == "java.lang" - } + } override fun scanForMatches(): List { val unwrappedGenericType = genericType.unwrapGenericType(try { diff --git a/src/test/groovy/graphql/kickstart/tools/BuiltInIdSpec.groovy b/src/test/groovy/graphql/kickstart/tools/BuiltInIdSpec.groovy new file mode 100644 index 00000000..a9bfb62f --- /dev/null +++ b/src/test/groovy/graphql/kickstart/tools/BuiltInIdSpec.groovy @@ -0,0 +1,134 @@ +package graphql.kickstart.tools + +import graphql.GraphQL +import graphql.execution.AsyncExecutionStrategy +import graphql.schema.GraphQLSchema +import spock.lang.Shared +import spock.lang.Specification + +class BuiltInIdSpec extends Specification { + + @Shared + GraphQL gql + + def setupSpec() { + GraphQLSchema schema = SchemaParser.newParser().schemaString('''\ + type Query { + itemByLongId(id: ID!): Item1! + itemsByLongIds(ids: [ID!]!): [Item1!]! + itemByUuidId(id: ID!): Item2! + itemsByUuidIds(ids: [ID!]!): [Item2!]! + } + + type Item1 { + id: ID! + } + + type Item2 { + id: ID! + } + '''.stripIndent()) + .resolvers(new QueryWithLongItemResolver()) + .build() + .makeExecutableSchema() + gql = GraphQL.newGraphQL(schema) + .queryExecutionStrategy(new AsyncExecutionStrategy()) + .build() + } + + def "supports Long as ID as input"() { + when: + def data = Utils.assertNoGraphQlErrors(gql) { + ''' + { + itemByLongId(id: 1) { + id + } + } + ''' + } + + then: + data.itemByLongId != null + data.itemByLongId.id == "1" + } + + def "supports list of Long as ID as input"() { + when: + def data = Utils.assertNoGraphQlErrors(gql) { + ''' + { + itemsByLongIds(ids: [1,2,3]) { + id + } + } + ''' + } + + then: + data.itemsByLongIds != null + data.itemsByLongIds.size == 3 + data.itemsByLongIds[0].id == "1" + } + + def "supports UUID as ID as input"() { + when: + def data = Utils.assertNoGraphQlErrors(gql) { + ''' + { + itemByUuidId(id: "00000000-0000-0000-0000-000000000000") { + id + } + } + ''' + } + + then: + data.itemByUuidId != null + data.itemByUuidId.id == "00000000-0000-0000-0000-000000000000" + } + + def "supports list of UUID as ID as input"() { + when: + def data = Utils.assertNoGraphQlErrors(gql) { + ''' + { + itemsByUuidIds(ids: ["00000000-0000-0000-0000-000000000000","11111111-1111-1111-1111-111111111111","22222222-2222-2222-2222-222222222222"]) { + id + } + } + ''' + } + + then: + data.itemsByUuidIds != null + data.itemsByUuidIds.size == 3 + data.itemsByUuidIds[0].id == "00000000-0000-0000-0000-000000000000" + } + + class QueryWithLongItemResolver implements GraphQLQueryResolver { + Item1 itemByLongId(Long id) { + new Item1(id: id) + } + + List itemsByLongIds(List ids) { + ids.collect { new Item1(id: it) } + } + + Item2 itemByUuidId(UUID id) { + new Item2(id: id) + } + + List itemsByUuidIds(List ids) { + ids.collect { new Item2(id: it) } + } + } + + class Item1 { + Long id + } + + class Item2 { + UUID id + } +} diff --git a/src/test/groovy/graphql/kickstart/tools/BuiltInLongIdSpec.groovy b/src/test/groovy/graphql/kickstart/tools/BuiltInLongIdSpec.groovy deleted file mode 100644 index 1382467a..00000000 --- a/src/test/groovy/graphql/kickstart/tools/BuiltInLongIdSpec.groovy +++ /dev/null @@ -1,81 +0,0 @@ -package graphql.kickstart.tools - -import graphql.GraphQL -import graphql.execution.AsyncExecutionStrategy -import graphql.schema.GraphQLSchema -import spock.lang.Shared -import spock.lang.Specification - -class BuiltInLongIdSpec extends Specification { - - @Shared - GraphQL gql - - def setupSpec() { - GraphQLSchema schema = SchemaParser.newParser().schemaString('''\ - type Query { - itemByLongId(id: ID!): Item! - itemsByLongIds(ids: [ID!]!): [Item!]! - } - - type Item { - id: ID! - } - '''.stripIndent()) - .resolvers(new QueryWithLongItemResolver()) - .build() - .makeExecutableSchema() - gql = GraphQL.newGraphQL(schema) - .queryExecutionStrategy(new AsyncExecutionStrategy()) - .build() - } - - def "supports Long as ID as input"() { - when: - def data = Utils.assertNoGraphQlErrors(gql) { - ''' - { - itemByLongId(id: 1) { - id - } - } - ''' - } - - then: - data.itemByLongId != null - data.itemByLongId.id == "1" - } - - def "supports list of Long as ID as input"() { - when: - def data = Utils.assertNoGraphQlErrors(gql) { - ''' - { - itemsByLongIds(ids: [1,2,3]) { - id - } - } - ''' - } - - then: - data.itemsByLongIds != null - data.itemsByLongIds.size == 3 - data.itemsByLongIds[0].id == "1" - } - - class QueryWithLongItemResolver implements GraphQLQueryResolver { - Item itemByLongId(Long id) { - new Item(id: id) - } - - List itemsByLongIds(List ids) { - ids.collect { new Item(id: it) } - } - } - - class Item { - Long id - } -}