Skip to content

Commit 2dbc802

Browse files
authored
Merge pull request #825 from graphql-java-kickstart/bugfix/460
Resolve type variables through generic superclasses
2 parents 43bfafe + c8ece74 commit 2dbc802

5 files changed

Lines changed: 415 additions & 46 deletions

File tree

‎src/main/kotlin/graphql/kickstart/tools/GenericType.kt‎

Lines changed: 20 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import com.fasterxml.classmate.ResolvedType
44
import graphql.kickstart.tools.util.JavaType
55
import graphql.kickstart.tools.util.ParameterizedTypeImpl
66
import graphql.kickstart.tools.util.Primitives
7-
import graphql.kickstart.tools.util.unwrap
87
import org.apache.commons.lang3.reflect.TypeUtils
98
import java.lang.reflect.ParameterizedType
109
import java.lang.reflect.TypeVariable
@@ -95,53 +94,26 @@ internal open class GenericType(protected val mostSpecificType: JavaType, protec
9594
}
9695

9796
val unwrapsTo = genericType.schemaWrapper.invoke(typeArguments[genericType.index])
98-
return unwrapGenericType(unwrapsTo)
99-
}
100-
is TypeVariable<*> -> {
101-
val parameterizedDeclaringType = parameterizedDeclaringTypeOrSuperType(declaringType)
102-
if (parameterizedDeclaringType != null) {
103-
unwrapGenericType(parameterizedDeclaringType, type)
104-
} else {
105-
error("Could not resolve type variable '${TypeUtils.toLongString(type)}' because declaring type is not parameterized: ${TypeUtils.toString(declaringType)}")
106-
}
97+
unwrapGenericType(unwrapsTo)
10798
}
99+
is TypeVariable<*> -> error("Could not resolve type variable '${TypeUtils.toLongString(type)}' of ${TypeUtils.toString(declaringType)} relative to ${TypeUtils.toString(mostSpecificType)}")
108100
is WildcardType -> type.upperBounds.firstOrNull()
109101
?: error("Unable to unwrap type, wildcard has no upper bound: $type")
110102
is Class<*> -> if (type.isPrimitive) Primitives.wrap(type) else type
111103
else -> error("Unable to unwrap type: $type")
112104
}
113105
}
114106

115-
private fun parameterizedDeclaringTypeOrSuperType(declaringType: JavaType): ParameterizedType? =
116-
if (declaringType is ParameterizedType) {
117-
declaringType
118-
} else {
119-
val superclass = declaringType.unwrap().genericSuperclass
120-
if (superclass != null) {
121-
parameterizedDeclaringTypeOrSuperType(superclass)
122-
} else {
123-
null
124-
}
125-
}
126-
127-
private fun unwrapGenericType(declaringType: ParameterizedType, type: TypeVariable<*>): JavaType {
128-
val rawClass = getRawClass(mostSpecificType)
129-
val arguments = TypeUtils.determineTypeArguments(rawClass, declaringType)
130-
val matchingType = arguments
131-
.filter { it.key.name == type.name }
132-
.values
133-
.firstOrNull()
134-
?: error("No type variable found for: ${TypeUtils.toLongString(type)}")
135-
136-
return unwrapGenericType(matchingType)
137-
}
138-
139-
private fun replaceTypeVariable(type: JavaType): JavaType {
107+
private fun replaceTypeVariable(type: JavaType, resolving: Set<TypeVariable<*>> = emptySet()): JavaType {
140108
return when (type) {
141109
is ParameterizedType -> {
142-
val actualTypeArguments = type.actualTypeArguments.map { replaceTypeVariable(it) }.toTypedArray()
143-
ParameterizedTypeImpl(type.rawType as Class<*>, actualTypeArguments, type.ownerType)
110+
val actualTypeArguments = type.actualTypeArguments.map { replaceTypeVariable(it, resolving) }.toTypedArray()
111+
ParameterizedTypeImpl(type.rawType as Class<*>, actualTypeArguments, type.ownerType?.let { replaceTypeVariable(it, resolving) })
144112
}
113+
is WildcardType -> TypeUtils.wildcardType()
114+
.withUpperBounds(*type.upperBounds.map { replaceTypeVariable(it, resolving) }.toTypedArray())
115+
.withLowerBounds(*type.lowerBounds.map { replaceTypeVariable(it, resolving) }.toTypedArray())
116+
.build()
145117
is ResolvedType -> {
146118
if (type.typeParameters.isEmpty()) {
147119
type.erasedType
@@ -152,14 +124,18 @@ internal open class GenericType(protected val mostSpecificType: JavaType, protec
152124
}
153125
is TypeVariable<*> -> {
154126
val genericDeclaration = type.genericDeclaration
155-
if (declaringType is ParameterizedType && genericDeclaration is Class<*>) {
156-
// keep the full type argument (e.g. List<Foo>) rather than its raw class so nested generics aren't lost
157-
TypeUtils.getTypeArguments(declaringType, genericDeclaration)?.get(type)
158-
?.takeIf { it != type }
159-
?.let { replaceTypeVariable(it) }
127+
when {
128+
// only a variable leaked from a raw type can be bound to a type containing itself (e.g. T -> List<T>),
129+
// erase it like the raw type does instead of expanding it forever
130+
type in resolving -> TypeUtils.getRawType(type.bounds.first(), null) ?: Any::class.java
131+
// the most specific type binds the variables of all its supertypes
132+
genericDeclaration is Class<*> -> generateSequence(mostSpecificType) { (it as? ParameterizedType)?.ownerType }
133+
// an inner class can also use the variables of its outer class, those are bound by its owner type (e.g. Connection<Owner>.Entry)
134+
.firstNotNullOfOrNull { TypeUtils.getTypeArguments(it, genericDeclaration)?.get(type) }
135+
// keep the full type argument (e.g. List<Foo>) rather than its raw class so nested generics aren't lost
136+
?.let { replaceTypeVariable(it, resolving + type) }
160137
?: type
161-
} else {
162-
type
138+
else -> type
163139
}
164140
}
165141
else -> {

‎src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,7 @@ internal class SchemaClassScanner(
361361
if (java.util.Map::class.java.isAssignableFrom(javaType.unwrap())) {
362362
throw SchemaClassScannerError("Two different property map classes used for type ${type.name}:\n${realEntry.joinReferences()}\n\n- ${javaType}:\n| ${reference.getDescription()}")
363363
}
364-
throw SchemaClassScannerError("Two different classes used for type ${type.name}:\n${realEntry.joinReferences()}\n\n- ${javaType.unwrap()}:\n| ${reference.getDescription()}")
364+
throw SchemaClassScannerError("Two different classes used for type ${type.name}:\n${realEntry.joinReferences()}\n\n- ${javaType.typeName}:\n| ${reference.getDescription()}")
365365
}
366366
}
367367
}
@@ -472,7 +472,7 @@ internal class SchemaClassScanner(
472472
references.add(reference)
473473
}
474474

475-
fun joinReferences() = "- ${typeClass()}:\n| " + references.joinToString("\n| ") { it.getDescription() }
475+
fun joinReferences() = "- ${javaType?.typeName}:\n| " + references.joinToString("\n| ") { it.getDescription() }
476476

477477
fun hasResolverRef(): Boolean {
478478
references.filterIsInstance<ReturnValueReference>().forEach { reference ->

‎src/test/kotlin/graphql/kickstart/tools/GenericInputTypesTest.kt‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,39 @@ class GenericInputTypesTest {
166166
assertEquals(data["audit"], "why:op:1")
167167
}
168168

169+
@Test
170+
fun `generic list fields inherited from parameterized superclasses are parsed`() {
171+
val schema = SchemaParser.newParser()
172+
.schemaString(
173+
"""
174+
type Query {
175+
mutate(input: MutationInput!): String!
176+
}
177+
178+
input MutationInput {
179+
items: [Item!]!
180+
}
181+
182+
input Item {
183+
prop: String!
184+
}
185+
""")
186+
.resolvers(InheritedListQueryResolver())
187+
.build()
188+
.makeExecutableSchema()
189+
val gql = GraphQL.newGraphQL(schema).build()
190+
191+
val data = assertNoGraphQlErrors(gql) {
192+
"""
193+
query {
194+
mutate(input: { items: [{ prop: "a" }, { prop: "b" }] })
195+
}
196+
"""
197+
}
198+
199+
assertEquals(data["mutate"], "a,b")
200+
}
201+
169202
class QueryResolver : GraphQLQueryResolver {
170203
fun audit(input: AuditWrapper<LanguageInput>): String = "${input.operator}:${input.content?.id}"
171204
fun audits(input: AuditWrapper<List<LanguageInput>>): String = "${input.operator}:${input.content?.joinToString(",") { it.id.orEmpty() }}"
@@ -184,6 +217,10 @@ class GenericInputTypesTest {
184217
fun audit(input: AuditRequest): String = "${input.reason}:${input.audit?.operator}:${input.audit?.content?.id}"
185218
}
186219

220+
class InheritedListQueryResolver : GraphQLQueryResolver {
221+
fun mutate(input: MutationInput): String = input.items.orEmpty().joinToString(",") { it.prop.orEmpty() }
222+
}
223+
187224
open class AuditWrapper<T> {
188225
var content: T? = null
189226
var operator: String? = null
@@ -196,6 +233,19 @@ class GenericInputTypesTest {
196233
var reason: String? = null
197234
}
198235

236+
abstract class GenericMutationInput<T> {
237+
@JvmField
238+
var items: List<T>? = null
239+
}
240+
241+
abstract class RenamedMutationInput<U> : GenericMutationInput<U>()
242+
243+
class MutationInput : RenamedMutationInput<MutationInput.Item>() {
244+
class Item {
245+
var prop: String? = null
246+
}
247+
}
248+
199249
class LanguageInput {
200250
var id: String? = null
201251
}

0 commit comments

Comments
 (0)