Skip to content

Commit c694d1a

Browse files
authored
Merge pull request #453 from alexandreBaronZnk/bugfix/367-list-ids-not-resolved-correctly
Always convert scalar ID values to the method parameter type fix #367
2 parents 62b823b + f6928c2 commit c694d1a

File tree

3 files changed

+154
-102
lines changed

3 files changed

+154
-102
lines changed

src/main/kotlin/graphql/kickstart/tools/resolver/MethodFieldResolver.kt

Lines changed: 20 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,6 @@ import graphql.schema.DataFetchingEnvironment
1414
import graphql.schema.GraphQLTypeUtil.isScalar
1515
import kotlinx.coroutines.future.future
1616
import java.lang.reflect.Method
17-
import java.lang.reflect.ParameterizedType
18-
import java.lang.reflect.WildcardType
1917
import java.util.*
2018
import kotlin.coroutines.intrinsics.suspendCoroutineUninterceptedOrReturn
2119
import kotlin.reflect.full.valueParameters
@@ -82,15 +80,13 @@ internal class MethodFieldResolver(
8280
return@add Optional.empty<Any>()
8381
}
8482

85-
if (value != null
86-
&& parameterType.unwrap().isAssignableFrom(value.javaClass)
87-
&& isScalarType(environment, definition.type, parameterType)) {
88-
return@add value
83+
if (value != null && shouldValueBeConverted(value, definition, parameterType, environment)) {
84+
return@add mapper.convertValue(value, object : TypeReference<Any>() {
85+
override fun getType() = parameterType
86+
})
8987
}
9088

91-
return@add mapper.convertValue(value, object : TypeReference<Any>() {
92-
override fun getType() = parameterType
93-
})
89+
return@add value
9490
}
9591
}
9692

@@ -110,22 +106,25 @@ internal class MethodFieldResolver(
110106
}
111107
}
112108

113-
private fun isScalarType(environment: DataFetchingEnvironment, type: Type<*>, genericParameterType: JavaType): Boolean =
114-
when (type) {
109+
private fun shouldValueBeConverted(value: Any, definition: InputValueDefinition, parameterType: JavaType, environment: DataFetchingEnvironment): Boolean {
110+
return !parameterType.unwrap().isAssignableFrom(value.javaClass) || !isConcreteScalarType(environment, definition.type, parameterType)
111+
}
112+
113+
/**
114+
* A concrete scalar type is a scalar type where values always coerce to the same Java type. The ID scalar type is not concrete
115+
* because values can be coerced to multiple different Java types (eg. String, Long, UUID). All values of a non-concrete scalar
116+
* type must be converted to the target method parameter type.
117+
*/
118+
private fun isConcreteScalarType(environment: DataFetchingEnvironment, type: Type<*>, genericParameterType: JavaType): Boolean {
119+
return when (type) {
115120
is ListType -> List::class.java.isAssignableFrom(this.genericType.getRawClass(genericParameterType))
116-
&& isScalarType(environment, type.type, this.genericType.unwrapGenericType(genericParameterType))
117-
is TypeName -> environment.graphQLSchema?.getType(type.name)?.let { isScalar(it) && !isJavaLanguageType(genericParameterType) }
121+
&& isConcreteScalarType(environment, type.type, this.genericType.unwrapGenericType(genericParameterType))
122+
is TypeName -> environment.graphQLSchema?.getType(type.name)?.let { isScalar(it) && type.name != "ID" }
118123
?: false
119-
is NonNullType -> isScalarType(environment, type.type, genericParameterType)
124+
is NonNullType -> isConcreteScalarType(environment, type.type, genericParameterType)
120125
else -> false
121126
}
122-
123-
private fun isJavaLanguageType(type: JavaType): Boolean =
124-
when (type) {
125-
is ParameterizedType -> isJavaLanguageType(type.actualTypeArguments[0])
126-
is WildcardType -> isJavaLanguageType(type.upperBounds[0])
127-
else -> genericType.getRawClass(type).`package`.name == "java.lang"
128-
}
127+
}
129128

130129
override fun scanForMatches(): List<TypeClassMatcher.PotentialMatch> {
131130
val unwrappedGenericType = genericType.unwrapGenericType(try {
Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
package graphql.kickstart.tools
2+
3+
import graphql.GraphQL
4+
import graphql.execution.AsyncExecutionStrategy
5+
import graphql.schema.GraphQLSchema
6+
import spock.lang.Shared
7+
import spock.lang.Specification
8+
9+
class BuiltInIdSpec extends Specification {
10+
11+
@Shared
12+
GraphQL gql
13+
14+
def setupSpec() {
15+
GraphQLSchema schema = SchemaParser.newParser().schemaString('''\
16+
type Query {
17+
itemByLongId(id: ID!): Item1!
18+
itemsByLongIds(ids: [ID!]!): [Item1!]!
19+
itemByUuidId(id: ID!): Item2!
20+
itemsByUuidIds(ids: [ID!]!): [Item2!]!
21+
}
22+
23+
type Item1 {
24+
id: ID!
25+
}
26+
27+
type Item2 {
28+
id: ID!
29+
}
30+
'''.stripIndent())
31+
.resolvers(new QueryWithLongItemResolver())
32+
.build()
33+
.makeExecutableSchema()
34+
gql = GraphQL.newGraphQL(schema)
35+
.queryExecutionStrategy(new AsyncExecutionStrategy())
36+
.build()
37+
}
38+
39+
def "supports Long as ID as input"() {
40+
when:
41+
def data = Utils.assertNoGraphQlErrors(gql) {
42+
'''
43+
{
44+
itemByLongId(id: 1) {
45+
id
46+
}
47+
}
48+
'''
49+
}
50+
51+
then:
52+
data.itemByLongId != null
53+
data.itemByLongId.id == "1"
54+
}
55+
56+
def "supports list of Long as ID as input"() {
57+
when:
58+
def data = Utils.assertNoGraphQlErrors(gql) {
59+
'''
60+
{
61+
itemsByLongIds(ids: [1,2,3]) {
62+
id
63+
}
64+
}
65+
'''
66+
}
67+
68+
then:
69+
data.itemsByLongIds != null
70+
data.itemsByLongIds.size == 3
71+
data.itemsByLongIds[0].id == "1"
72+
}
73+
74+
def "supports UUID as ID as input"() {
75+
when:
76+
def data = Utils.assertNoGraphQlErrors(gql) {
77+
'''
78+
{
79+
itemByUuidId(id: "00000000-0000-0000-0000-000000000000") {
80+
id
81+
}
82+
}
83+
'''
84+
}
85+
86+
then:
87+
data.itemByUuidId != null
88+
data.itemByUuidId.id == "00000000-0000-0000-0000-000000000000"
89+
}
90+
91+
def "supports list of UUID as ID as input"() {
92+
when:
93+
def data = Utils.assertNoGraphQlErrors(gql) {
94+
'''
95+
{
96+
itemsByUuidIds(ids: ["00000000-0000-0000-0000-000000000000","11111111-1111-1111-1111-111111111111","22222222-2222-2222-2222-222222222222"]) {
97+
id
98+
}
99+
}
100+
'''
101+
}
102+
103+
then:
104+
data.itemsByUuidIds != null
105+
data.itemsByUuidIds.size == 3
106+
data.itemsByUuidIds[0].id == "00000000-0000-0000-0000-000000000000"
107+
}
108+
109+
class QueryWithLongItemResolver implements GraphQLQueryResolver {
110+
Item1 itemByLongId(Long id) {
111+
new Item1(id: id)
112+
}
113+
114+
List<Item1> itemsByLongIds(List<Long> ids) {
115+
ids.collect { new Item1(id: it) }
116+
}
117+
118+
Item2 itemByUuidId(UUID id) {
119+
new Item2(id: id)
120+
}
121+
122+
List<Item2> itemsByUuidIds(List<UUID> ids) {
123+
ids.collect { new Item2(id: it) }
124+
}
125+
}
126+
127+
class Item1 {
128+
Long id
129+
}
130+
131+
class Item2 {
132+
UUID id
133+
}
134+
}

src/test/groovy/graphql/kickstart/tools/BuiltInLongIdSpec.groovy

Lines changed: 0 additions & 81 deletions
This file was deleted.

0 commit comments

Comments
 (0)