diff --git a/src/main/kotlin/graphql/kickstart/tools/GenericType.kt b/src/main/kotlin/graphql/kickstart/tools/GenericType.kt index 5d681f27..90a53d32 100644 --- a/src/main/kotlin/graphql/kickstart/tools/GenericType.kt +++ b/src/main/kotlin/graphql/kickstart/tools/GenericType.kt @@ -4,7 +4,6 @@ import com.fasterxml.classmate.ResolvedType import graphql.kickstart.tools.util.JavaType import graphql.kickstart.tools.util.ParameterizedTypeImpl import graphql.kickstart.tools.util.Primitives -import graphql.kickstart.tools.util.unwrap import org.apache.commons.lang3.reflect.TypeUtils import java.lang.reflect.ParameterizedType import java.lang.reflect.TypeVariable @@ -97,14 +96,7 @@ internal open class GenericType(protected val mostSpecificType: JavaType, protec val unwrapsTo = genericType.schemaWrapper.invoke(typeArguments[genericType.index]) return unwrapGenericType(unwrapsTo) } - is TypeVariable<*> -> { - val parameterizedDeclaringType = parameterizedDeclaringTypeOrSuperType(declaringType) - if (parameterizedDeclaringType != null) { - unwrapGenericType(parameterizedDeclaringType, type) - } else { - error("Could not resolve type variable '${TypeUtils.toLongString(type)}' because declaring type is not parameterized: ${TypeUtils.toString(declaringType)}") - } - } + is TypeVariable<*> -> error("Could not resolve type variable '${TypeUtils.toLongString(type)}' of ${TypeUtils.toString(declaringType)} relative to ${TypeUtils.toString(mostSpecificType)}") is WildcardType -> type.upperBounds.firstOrNull() ?: error("Unable to unwrap type, wildcard has no upper bound: $type") is Class<*> -> if (type.isPrimitive) Primitives.wrap(type) else type @@ -112,36 +104,16 @@ internal open class GenericType(protected val mostSpecificType: JavaType, protec } } - private fun parameterizedDeclaringTypeOrSuperType(declaringType: JavaType): ParameterizedType? = - if (declaringType is ParameterizedType) { - declaringType - } else { - val superclass = declaringType.unwrap().genericSuperclass - if (superclass != null) { - parameterizedDeclaringTypeOrSuperType(superclass) - } else { - null - } - } - - private fun unwrapGenericType(declaringType: ParameterizedType, type: TypeVariable<*>): JavaType { - val rawClass = getRawClass(mostSpecificType) - val arguments = TypeUtils.determineTypeArguments(rawClass, declaringType) - val matchingType = arguments - .filter { it.key.name == type.name } - .values - .firstOrNull() - ?: error("No type variable found for: ${TypeUtils.toLongString(type)}") - - return unwrapGenericType(matchingType) - } - - private fun replaceTypeVariable(type: JavaType): JavaType { + private fun replaceTypeVariable(type: JavaType, resolving: Set> = emptySet()): JavaType { return when (type) { is ParameterizedType -> { - val actualTypeArguments = type.actualTypeArguments.map { replaceTypeVariable(it) }.toTypedArray() - ParameterizedTypeImpl(type.rawType as Class<*>, actualTypeArguments, type.ownerType) + val actualTypeArguments = type.actualTypeArguments.map { replaceTypeVariable(it, resolving) }.toTypedArray() + ParameterizedTypeImpl(type.rawType as Class<*>, actualTypeArguments, type.ownerType?.let { replaceTypeVariable(it, resolving) }) } + is WildcardType -> TypeUtils.wildcardType() + .withUpperBounds(*type.upperBounds.map { replaceTypeVariable(it, resolving) }.toTypedArray()) + .withLowerBounds(*type.lowerBounds.map { replaceTypeVariable(it, resolving) }.toTypedArray()) + .build() is ResolvedType -> { if (type.typeParameters.isEmpty()) { type.erasedType @@ -152,14 +124,18 @@ internal open class GenericType(protected val mostSpecificType: JavaType, protec } is TypeVariable<*> -> { val genericDeclaration = type.genericDeclaration - if (declaringType is ParameterizedType && genericDeclaration is Class<*>) { - // keep the full type argument (e.g. List) rather than its raw class so nested generics aren't lost - TypeUtils.getTypeArguments(declaringType, genericDeclaration)?.get(type) - ?.takeIf { it != type } - ?.let { replaceTypeVariable(it) } + when { + // only a variable leaked from a raw type can be bound to a type containing itself (e.g. T -> List), + // erase it like the raw type does instead of expanding it forever + type in resolving -> TypeUtils.getRawType(type.bounds.first(), null) ?: Any::class.java + // the most specific type binds the variables of its supertypes and its owner types those of outer classes, + // the declaring type may not + genericDeclaration is Class<*> -> generateSequence(mostSpecificType) { (it as? ParameterizedType)?.ownerType } + .firstNotNullOfOrNull { TypeUtils.getTypeArguments(it, genericDeclaration)?.get(type) } + // keep the full type argument (e.g. List) rather than its raw class so nested generics aren't lost + ?.let { replaceTypeVariable(it, resolving + type) } ?: type - } else { - type + else -> type } } else -> { diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt index 6c3fbce1..aa899b6d 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt @@ -360,7 +360,7 @@ internal class SchemaClassScanner( if (java.util.Map::class.java.isAssignableFrom(javaType.unwrap())) { throw SchemaClassScannerError("Two different property map classes used for type ${type.name}:\n${realEntry.joinReferences()}\n\n- ${javaType}:\n| ${reference.getDescription()}") } - throw SchemaClassScannerError("Two different classes used for type ${type.name}:\n${realEntry.joinReferences()}\n\n- ${javaType.unwrap()}:\n| ${reference.getDescription()}") + throw SchemaClassScannerError("Two different classes used for type ${type.name}:\n${realEntry.joinReferences()}\n\n- ${javaType.typeName}:\n| ${reference.getDescription()}") } } } @@ -471,7 +471,7 @@ internal class SchemaClassScanner( references.add(reference) } - fun joinReferences() = "- ${typeClass()}:\n| " + references.joinToString("\n| ") { it.getDescription() } + fun joinReferences() = "- ${javaType?.typeName}:\n| " + references.joinToString("\n| ") { it.getDescription() } fun hasResolverRef(): Boolean { references.filterIsInstance().forEach { reference -> diff --git a/src/test/kotlin/graphql/kickstart/tools/GenericInputTypesTest.kt b/src/test/kotlin/graphql/kickstart/tools/GenericInputTypesTest.kt index 2cf3f6dc..6856a07f 100644 --- a/src/test/kotlin/graphql/kickstart/tools/GenericInputTypesTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/GenericInputTypesTest.kt @@ -166,6 +166,39 @@ class GenericInputTypesTest { assertEquals(data["audit"], "why:op:1") } + @Test + fun `generic list fields inherited from parameterized superclasses are parsed`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + type Query { + mutate(input: MutationInput!): String! + } + + input MutationInput { + items: [Item!]! + } + + input Item { + prop: String! + } + """) + .resolvers(InheritedListQueryResolver()) + .build() + .makeExecutableSchema() + val gql = GraphQL.newGraphQL(schema).build() + + val data = assertNoGraphQlErrors(gql) { + """ + query { + mutate(input: { items: [{ prop: "a" }, { prop: "b" }] }) + } + """ + } + + assertEquals(data["mutate"], "a,b") + } + class QueryResolver : GraphQLQueryResolver { fun audit(input: AuditWrapper): String = "${input.operator}:${input.content?.id}" fun audits(input: AuditWrapper>): String = "${input.operator}:${input.content?.joinToString(",") { it.id.orEmpty() }}" @@ -184,6 +217,10 @@ class GenericInputTypesTest { fun audit(input: AuditRequest): String = "${input.reason}:${input.audit?.operator}:${input.audit?.content?.id}" } + class InheritedListQueryResolver : GraphQLQueryResolver { + fun mutate(input: MutationInput): String = input.items.orEmpty().joinToString(",") { it.prop.orEmpty() } + } + open class AuditWrapper { var content: T? = null var operator: String? = null @@ -196,6 +233,19 @@ class GenericInputTypesTest { var reason: String? = null } + abstract class GenericMutationInput { + @JvmField + var items: List? = null + } + + abstract class RenamedMutationInput : GenericMutationInput() + + class MutationInput : RenamedMutationInput() { + class Item { + var prop: String? = null + } + } + class LanguageInput { var id: String? = null } diff --git a/src/test/kotlin/graphql/kickstart/tools/InheritedTypeVariablesTest.kt b/src/test/kotlin/graphql/kickstart/tools/InheritedTypeVariablesTest.kt new file mode 100644 index 00000000..e2452304 --- /dev/null +++ b/src/test/kotlin/graphql/kickstart/tools/InheritedTypeVariablesTest.kt @@ -0,0 +1,306 @@ +package graphql.kickstart.tools + +import graphql.GraphQL +import org.junit.Assert.assertThrows +import org.junit.Test + +class InheritedTypeVariablesTest { + + @Test + fun `type variables passed through several superclasses are resolved`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + type Query { + item: Item! + ownerItem: OwnerItem! + } + + type Item { + id: ID! + value: ID! + } + + type OwnerItem { + id: Owner! + value: Owner! + } + + type Owner { + name: String! + } + """) + .resolvers(QueryResolver()) + .build() + .makeExecutableSchema() + val gql = GraphQL.newGraphQL(schema).build() + + val data = assertNoGraphQlErrors(gql) { + """ + query { + item { id value } + ownerItem { + id { name } + value { name } + } + } + """ + } + + assertEquals(data["item"], mapOf("id" to "1", "value" to "1")) + assertEquals(data["ownerItem"], mapOf("id" to mapOf("name" to "owner"), "value" to mapOf("name" to "owner"))) + } + + @Test + fun `type variables renamed and reordered by superclasses are resolved`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + type Query { + account: Account! + } + + type Account { + id: ID! + owner: Owner! + } + + type Owner { + name: String! + } + """) + .resolvers(QueryResolver()) + .build() + .makeExecutableSchema() + val gql = GraphQL.newGraphQL(schema).build() + + val data = assertNoGraphQlErrors(gql) { + """ + query { + account { + id + owner { name } + } + } + """ + } + + assertEquals(data["account"], mapOf("id" to "2", "owner" to mapOf("name" to "owner"))) + } + + @Test + fun `type variables nested in types inherited from a superclass are resolved`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + type Query { + owners: OwnerConnection! + } + + type OwnerConnection { + edges: [OwnerEdge!]! + nodes: [Owner]! + entries: [OwnerEntry!]! + } + + type OwnerEdge { + node: Owner! + } + + type OwnerEntry { + position: Int! + node: Owner! + } + + type Owner { + name: String! + } + """) + .resolvers(QueryResolver()) + .build() + .makeExecutableSchema() + val gql = GraphQL.newGraphQL(schema).build() + + val data = assertNoGraphQlErrors(gql) { + """ + query { + owners { + edges { + node { name } + } + nodes { name } + entries { + position + node { name } + } + } + } + """ + } + + val owner = mapOf("name" to "owner") + assertEquals(data["owners"], mapOf( + "edges" to listOf(mapOf("node" to owner)), + "nodes" to listOf(owner), + "entries" to listOf(mapOf("position" to 0, "node" to owner)) + )) + } + + @Test + fun `type variables leaked from raw types don't overflow the stack`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + type Query { + tree: Tree! + } + + type Tree { + grouped: GroupedTree! + async: AsyncTree! + } + + type GroupedTree { + value: [String!]! + } + + type AsyncTree { + value: String! + } + """) + .resolvers(RawGenericFixtures.QueryResolver()) + .build() + .makeExecutableSchema() + val gql = GraphQL.newGraphQL(schema).build() + + val data = assertNoGraphQlErrors(gql) { + """ + query { + tree { + grouped { value } + async { value } + } + } + """ + } + + assertEquals(data["tree"], mapOf("grouped" to mapOf("value" to listOf("leaf")), "async" to mapOf("value" to "leaf"))) + } + + @Test + fun `generic types bound differently by subclasses can't share a type`() { + val error = assertThrows(SchemaClassScannerError::class.java) { + SchemaParser.newParser() + .schemaString( + """ + type Query { + ownerPage: OwnerPage! + accountPage: AccountPage! + } + + type OwnerPage { + meta: Meta! + } + + type AccountPage { + meta: Meta! + } + + type Meta { + total: Int! + } + """) + .resolvers(QueryResolver()) + .build() + .makeExecutableSchema() + } + + val message = error.message.orEmpty() + assert(message.startsWith("Two different classes used for type Meta")) { message } + assert(message.contains("${Meta::class.java.name}<${Owner::class.java.name}>")) { message } + assert(message.contains("${Meta::class.java.name}<${Account::class.java.name}>")) { message } + } + + @Test + fun `type variables of generic methods can't be resolved`() { + val error = assertThrows(IllegalStateException::class.java) { + SchemaParser.newParser() + .schemaString( + """ + type Query { + owner: Owner! + } + + type Owner { + name: String! + } + """) + .resolvers(GenericMethodQueryResolver()) + .build() + .makeExecutableSchema() + } + + assert(error.message.orEmpty().startsWith("Could not resolve type variable")) { error.message.orEmpty() } + } + + class QueryResolver : GraphQLQueryResolver { + fun item(): Item = Item(1) + fun ownerItem(): BaseItem = BaseItem(Owner("owner")) + fun account(): Account = Account(2, Owner("owner")) + fun owners(): OwnerConnection = OwnerConnection(listOf(Owner("owner"))) + fun ownerPage(): OwnerPage = OwnerPage() + fun accountPage(): AccountPage = AccountPage() + } + + // id is a public field, value a getter + open class AbstractItem(@JvmField val id: T, val value: T) + + open class BaseItem(value: T) : AbstractItem(value, value) + + class Item(value: Long) : BaseItem(value) + + interface Identifiable { + val id: I + } + + abstract class Entity(override val id: K, val owner: R) : Identifiable + + // passes its K as Entity's R and vice versa + abstract class SwappedEntity(id: R, owner: K) : Entity(id, owner) + + class Account(id: Long, owner: Owner) : SwappedEntity(id, owner) + + abstract class Connection(val nodes: List<@JvmWildcard T>) { + val edges: List> + get() = nodes.map { Edge(it) } + + val entries: List + get() = nodes.mapIndexed { position, node -> Entry(position, node) } + + inner class Entry(val position: Int, val node: T) + } + + class Edge(val node: T) + + class OwnerConnection(nodes: List) : Connection(nodes) + + class Meta(val total: Int) + + abstract class MetaPage { + val meta: Meta = Meta(0) + } + + class OwnerPage : MetaPage() + + class AccountPage : MetaPage() + + open class GenericMethodBase + + // the class binds its own T, which doesn't make the method's T resolvable + class GenericMethodQueryResolver : GenericMethodBase(), GraphQLQueryResolver { + @Suppress("UNCHECKED_CAST") + fun owner(): T = Owner("owner") as T + } + + class Owner(val name: String) +} diff --git a/src/test/kotlin/graphql/kickstart/tools/RawGenericFixtures.java b/src/test/kotlin/graphql/kickstart/tools/RawGenericFixtures.java new file mode 100644 index 00000000..d01b3076 --- /dev/null +++ b/src/test/kotlin/graphql/kickstart/tools/RawGenericFixtures.java @@ -0,0 +1,37 @@ +package graphql.kickstart.tools; + +import java.util.List; +import java.util.concurrent.CompletableFuture; + +// Raw types can't be expressed in Kotlin, so these fixtures must stay in Java. +public class RawGenericFixtures { + + public static class QueryResolver implements GraphQLQueryResolver { + + @SuppressWarnings({"rawtypes", "unused"}) + public Tree tree() { + return new Tree<>("leaf"); + } + } + + public static class Tree { + private final T value; + + public Tree(T value) { + this.value = value; + } + + public T getValue() { + return value; + } + + // returned from a raw Tree, these are trees whose T is bound to a type containing that same, unbound T + public Tree> getGrouped() { + return new Tree<>(List.of(value)); + } + + public Tree> getAsync() { + return new Tree<>(CompletableFuture.completedFuture(value)); + } + } +}