diff --git a/pom.xml b/pom.xml index 98921e13..69df3183 100644 --- a/pom.xml +++ b/pom.xml @@ -360,6 +360,35 @@ + + + java17-tests + + [17,) + + + + + org.apache.maven.plugins + maven-compiler-plugin + + + java17-test-compile + + testCompile + + + + ${project.basedir}/src/test/java17 + + 17 + + + + + + + release diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt index ad13a6d1..006a86f7 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt @@ -8,9 +8,11 @@ import graphql.language.* import graphql.schema.GraphQLScalarType import graphql.schema.idl.ScalarInfo import org.apache.commons.lang3.ClassUtils +import org.apache.commons.lang3.reflect.TypeUtils import org.slf4j.LoggerFactory import java.lang.reflect.Member import java.lang.reflect.Method +import java.lang.reflect.ParameterizedType /** * @author Andrew Potter @@ -359,14 +361,103 @@ internal class SchemaClassScanner( } private fun handleDictionaryTypes(types: List>, failureMessage: (TypeDefinition<*>) -> String) { + val typesWithoutClass = mutableListOf>() types.forEach { type -> val dictionaryContainsType = dictionary.filter { it.key.name == type.name }.isNotEmpty() if (!unvalidatedTypes.contains(type) && !dictionaryContainsType) { val initialEntry = initialDictionary[type.name] - ?: throw SchemaClassScannerError(failureMessage(type)) - handleFoundType(type, initialEntry.get(), DictionaryReference()) + if (initialEntry != null) { + handleFoundType(type, initialEntry.get(), DictionaryReference()) + } else { + typesWithoutClass.add(type) + } + } + } + + // Scanning a sealed subclass can find the classes of the other types, so only fail when no type has exactly one sealed + // subclass named after it. Types still without a class fail on the next pass. + val sealedMatches = typesWithoutClass.filterIsInstance().mapNotNull { sealedSubclassMatches(it).singleOrNull() } + if (sealedMatches.isEmpty()) { + typesWithoutClass.firstOrNull()?.let { throw SchemaClassScannerError(failureMessage(it) + sealedSubclassHint(it)) } + } + + sealedMatches.forEach { handleFoundType(it) } + } + + /** + * Explains why no sealed subclass was used for the type, if any of its unions or interfaces has a sealed class. + */ + private fun sealedSubclassHint(type: TypeDefinition<*>): String { + if (type !is ObjectTypeDefinition) { + return "" + } + + val sealedTypes = sealedTypesOf(type) + if (sealedTypes.isEmpty()) { + return "" + } + + val sealedTypeNames = sealedTypes.joinToString(" or ") { it.unwrap().name } + val matches = sealedSubclassMatches(type) + return if (matches.isEmpty()) { + " No subclass of sealed type $sealedTypeNames is named '${type.name}'." + } else { + " More than one subclass of sealed type $sealedTypeNames is named '${type.name}': ${matches.joinToString { it.javaType.unwrap().name }}." + } + } + + /** + * A member of a union or an implementor of an interface may never be returned directly, e.g. `NormalPet` when only + * `pets(): List` returns `union Pet`. If the class found for the union or interface is sealed, its subclass named + * after the object type can be used instead. + */ + private fun sealedSubclassMatches(type: ObjectTypeDefinition): List = + sealedTypesOf(type) + .flatMap { sealedType -> + sealedSubclasses(sealedType.unwrap()) + .filter { it.simpleName == type.name } + .map { TypeClassMatcher.ValidMatch(type, parameterize(it, sealedType), SealedSubclassReference(sealedType)) } + } + // a subclass is found twice when it's under two of these sealed types, or extends both a sealed type and one of its + // sealed subclasses + .distinctBy { it.javaType.unwrap() } + + /** + * The sealed classes found for the unions the type is a member of and the interfaces it implements. + */ + private fun sealedTypesOf(type: ObjectTypeDefinition): List = + dictionary + .filterKeys { definition -> + when (definition) { + is UnionTypeDefinition -> definition.memberTypes.filterIsInstance().any { it.name == type.name } + is InterfaceTypeDefinition -> type.implements.filterIsInstance().any { it.name == definition.name } + else -> false + } } + .values.mapNotNull { it.javaType } + .distinct() + .filter { it.unwrap().kotlin.isSealed } + + /** + * All subclasses of a sealed class, including those of its sealed subclasses. kotlin-reflect also returns the permitted + * subclasses of Java sealed classes. + */ + private fun sealedSubclasses(clazz: Class<*>): List> = + clazz.kotlin.sealedSubclasses.flatMap { listOf(it.java) + sealedSubclasses(it.java) } + + /** + * Passes the type arguments of a parameterized sealed class on to its generic subclass, e.g. `Success` for + * `Result`, so the types of the subclass's fields can be resolved. + */ + private fun parameterize(subclass: Class<*>, sealedType: JavaType): JavaType { + if (sealedType !is ParameterizedType || subclass.typeParameters.isEmpty()) { + return subclass } + + return TypeUtils.determineTypeArguments(subclass, sealedType) + ?.takeIf { it.keys.containsAll(subclass.typeParameters.asList()) } + ?.let { TypeUtils.parameterize(subclass, it) } + ?: subclass } /** @@ -636,6 +727,10 @@ internal class SchemaClassScanner( override fun getDescription() = "interface declarations of ${type.name}" } + private class SealedSubclassReference(private val sealedType: JavaType) : Reference() { + override fun getDescription() = "subclass of sealed type ${sealedType.typeName}" + } + private class InputObjectReference(private val type: InputValueDefinition) : Reference() { override fun getDescription() = "input object $type" } diff --git a/src/test/java17/graphql/kickstart/tools/SchemaClassScannerSealedJavaTypesTest.java b/src/test/java17/graphql/kickstart/tools/SchemaClassScannerSealedJavaTypesTest.java new file mode 100644 index 00000000..18803f14 --- /dev/null +++ b/src/test/java17/graphql/kickstart/tools/SchemaClassScannerSealedJavaTypesTest.java @@ -0,0 +1,118 @@ +package graphql.kickstart.tools; + +import graphql.ExecutionInput; +import graphql.ExecutionResult; +import graphql.GraphQL; +import graphql.schema.GraphQLSchema; +import org.junit.Test; + +import java.util.List; +import java.util.Map; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +// Sealed classes need Java 17, so the java17-tests profile only compiles and runs this on Java 17 and later. +public class SchemaClassScannerSealedJavaTypesTest { + + @Test + public void scannerFindsUnionMembersAndInterfaceImplementorsAmongThePermittedSubclassesOfSealedTypes() { + GraphQLSchema schema = SchemaParser.newParser() + .schemaString(""" + type Query { + pets: [Pet] + animals: [Animal] + } + + union Pet = Dog | Fish + + type Dog { + name: String + } + + type Fish { + fins: Int + } + + interface Animal { + name: String + } + + type Cat implements Animal { + name: String + lives: Int + } + + type Bird implements Animal { + name: String + wings: Int + } + """) + .resolvers(new PetQuery(), new AnimalQuery()) + .build() + .makeExecutableSchema(); + + ExecutionResult result = GraphQL.newGraphQL(schema).build().execute(ExecutionInput.newExecutionInput().query(""" + query { + pets { __typename ... on Dog { name } ... on Fish { fins } } + animals { __typename name ... on Cat { lives } ... on Bird { wings } } + } + """)); + + assertTrue(result.getErrors().toString(), result.getErrors().isEmpty()); + assertEquals(Map.of( + "pets", List.of(Map.of("__typename", "Dog", "name", "Rex"), Map.of("__typename", "Fish", "fins", 2)), + "animals", List.of(Map.of("__typename", "Cat", "name", "Tom", "lives", 9), Map.of("__typename", "Bird", "name", "Tweety", "wings", 2)) + ), result.getData()); + } + + public static class PetQuery implements GraphQLQueryResolver { + public List pets() { + return List.of(new Dog("Rex"), new Fish(2)); + } + } + + public sealed interface Pet permits Mammal, Fish { + } + + public sealed interface Mammal extends Pet permits Dog { + } + + public record Dog(String name) implements Mammal { + } + + public record Fish(Integer fins) implements Pet { + } + + public static class AnimalQuery implements GraphQLQueryResolver { + public List animals() { + return List.of(new Cat("Tom", 9), new Bird("Tweety", 2)); + } + } + + public abstract static sealed class Animal permits Cat, Bird { + final String name; + + Animal(String name) { + this.name = name; + } + } + + public static final class Cat extends Animal { + final Integer lives; + + Cat(String name, Integer lives) { + super(name); + this.lives = lives; + } + } + + public static final class Bird extends Animal { + final Integer wings; + + Bird(String name, Integer wings) { + super(name); + this.wings = wings; + } + } +} diff --git a/src/test/kotlin/graphql/kickstart/tools/SchemaClassScannerSealedTypesTest.kt b/src/test/kotlin/graphql/kickstart/tools/SchemaClassScannerSealedTypesTest.kt new file mode 100644 index 00000000..20e59f4e --- /dev/null +++ b/src/test/kotlin/graphql/kickstart/tools/SchemaClassScannerSealedTypesTest.kt @@ -0,0 +1,326 @@ +package graphql.kickstart.tools + +import graphql.GraphQL +import graphql.schema.GraphQLUnionType +import org.junit.Assert.assertThrows +import org.junit.Test + +class SchemaClassScannerSealedTypesTest { + + private val petSchema = """ + type Query { + pets: [Pet] + } + + union Pet = NormalPet | WeirdPet + + type NormalPet { + name: String + } + + type WeirdPet { + classification: String + } + """ + + private val petsQuery = """ + query { + pets { + __typename + ... on NormalPet { name } + ... on WeirdPet { classification } + } + } + """ + + private val pets = listOf( + mapOf("__typename" to "NormalPet", "name" to "Rex"), + mapOf("__typename" to "WeirdPet", "classification" to "axolotl") + ) + + @Test + fun `scanner finds members of unions among the subclasses of sealed classes`() { + val schema = SchemaParser.newParser() + .schemaString(petSchema) + .resolvers(SealedClassUnion()) + .build() + .makeExecutableSchema() + + val data = assertNoGraphQlErrors(GraphQL.newGraphQL(schema).build()) { petsQuery } + + assertEquals(data["pets"], pets) + } + + class SealedClassUnion : GraphQLQueryResolver { + fun pets(): List = listOf(Pet.NormalPet("Rex"), Pet.Exotic.WeirdPet("axolotl")) + + sealed class Pet { + class NormalPet(val name: String?) : Pet() + + sealed class Exotic : Pet() { + class WeirdPet(val classification: String?) : Exotic() + } + } + } + + @Test + fun `scanner finds implementors of interfaces among the subclasses of sealed interfaces`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + type Query { + animals: [Animal] + } + + interface Animal { + name: String + } + + type Dog implements Animal { + name: String + barks: Boolean + } + + type Cat implements Animal { + name: String + lives: Int + } + """) + .resolvers(SealedInterfaceImplementors()) + .build() + .makeExecutableSchema() + + val data = assertNoGraphQlErrors(GraphQL.newGraphQL(schema).build()) { + """ + query { + animals { + __typename + name + ... on Dog { barks } + ... on Cat { lives } + } + } + """ + } + + assertEquals(data["animals"], listOf( + mapOf("__typename" to "Dog", "name" to "Rex", "barks" to true), + mapOf("__typename" to "Cat", "name" to "Tom", "lives" to 9) + )) + } + + class SealedInterfaceImplementors : GraphQLQueryResolver { + fun animals(): List = listOf(Dog("Rex", true), Cat("Tom", 9)) + + sealed interface Animal { + val name: String? + } + + sealed class Mammal : Animal + + // a subclass of both Animal and Mammal, so it's found twice among the subclasses of Animal + class Dog(override val name: String?, val barks: Boolean?) : Mammal(), Animal + + class Cat(override val name: String?, val lives: Int?) : Animal + } + + @Test + fun `scanner passes the type arguments of generic sealed classes on to their subclasses`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + type Query { + results: [PetResult] + } + + union PetResult = Success | Failure + + type Success { + value: Pet + } + + type Failure { + message: String + } + + type Pet { + name: String + } + """) + .resolvers(GenericSealedClass()) + .build() + .makeExecutableSchema() + + val data = assertNoGraphQlErrors(GraphQL.newGraphQL(schema).build()) { + """ + query { + results { + __typename + ... on Success { value { name } } + ... on Failure { message } + } + } + """ + } + + assertEquals(data["results"], listOf( + mapOf("__typename" to "Success", "value" to mapOf("name" to "Rex")), + mapOf("__typename" to "Failure", "message" to "Not found") + )) + } + + class GenericSealedClass : GraphQLQueryResolver { + fun results(): List> = listOf(Result.Success(Pet("Rex")), Result.Failure("Not found")) + + class Pet(val name: String?) + + sealed class Result { + class Success(val value: T) : Result() + + class Failure(val message: String?) : Result() + } + } + + @Test + fun `scanner scans the subclasses of sealed classes like any other class`() { + val searchSchema = """ + type Query { + results: [SearchResult] + } + + union SearchResult = Book | Author + + type Book { + title: String + author: Author + } + + type Author { + name: String + } + """ + + val schema = SchemaParser.newParser() + .schemaString(searchSchema) + .resolvers(MemberReachableThroughSealedSubclass()) + .build() + .makeExecutableSchema() + + assertEquals((schema.getType("SearchResult") as GraphQLUnionType).types.map { it.name }.toSet(), setOf("Book", "Author")) + + val error = assertThrows(SchemaClassScannerError::class.java) { + SchemaParser.newParser() + .schemaString(searchSchema) + .resolvers(ConflictingSealedSubclass()) + .build() + .makeExecutableSchema() + } + + val message = error.message.orEmpty() + assert(message.startsWith("Two different classes used for type Author:\n- ${ConflictingSealedSubclass.SearchResult.Author::class.java.name}:\n| subclass of sealed type ${ConflictingSealedSubclass.SearchResult::class.java.name}")) { message } + assert(message.contains("- ${ConflictingSealedSubclass.AuthorDto::class.java.name}:")) { message } + } + + // Author isn't a subclass of SearchResult, so it's only found by scanning Book + class MemberReachableThroughSealedSubclass : GraphQLQueryResolver { + fun results(): List = listOf() + + sealed class SearchResult { + class Book(val title: String?, val author: Author?) : SearchResult() + } + + class Author(val name: String?) + } + + // scanning Book finds another class for Author than its sealed subclass + class ConflictingSealedSubclass : GraphQLQueryResolver { + fun results(): List = listOf() + + sealed class SearchResult { + class Book(val title: String?, val author: AuthorDto?) : SearchResult() + + class Author(val name: String?) : SearchResult() + } + + class AuthorDto(val name: String?) + } + + @Test + fun `scanner prefers the dictionary over the subclasses of sealed classes`() { + val schema = SchemaParser.newParser() + .schemaString(petSchema) + .resolvers(DictionaryOverridesSealedSubclass()) + .dictionary("WeirdPet", DictionaryOverridesSealedSubclass.ClassifiedWeirdPet::class) + .build() + .makeExecutableSchema() + + val data = assertNoGraphQlErrors(GraphQL.newGraphQL(schema).build()) { petsQuery } + + assertEquals(data["pets"], pets) + } + + class DictionaryOverridesSealedSubclass : GraphQLQueryResolver { + fun pets(): List = listOf(Pet.NormalPet("Rex"), ClassifiedWeirdPet("axolotl")) + + sealed class Pet { + class NormalPet(val name: String?) : Pet() + + // no classification, so the build fails if this is scanned instead of the class in the dictionary + open class WeirdPet : Pet() + } + + class ClassifiedWeirdPet(val classification: String?) : Pet.WeirdPet() + } + + @Test + fun `scanner still requires classes for members of unions without a single sealed subclass named after them`() { + fun buildError(resolver: GraphQLQueryResolver) = assertThrows(SchemaClassScannerError::class.java) { + SchemaParser.newParser() + .schemaString(petSchema) + .resolvers(resolver) + .build() + .makeExecutableSchema() + }.message + + assertEquals(buildError(NonSealedInterfaceUnion()), "Object type 'NormalPet' is a member of a known union, but no class could be found for that type name. Please pass a class for type 'NormalPet' in the parser's dictionary.") + + assertEquals(buildError(PartlySealedUnion()), "Object type 'WeirdPet' is a member of a known union, but no class could be found for that type name. Please pass a class for type 'WeirdPet' in the parser's dictionary." + + " No subclass of sealed type ${PartlySealedUnion.Pet::class.java.name} is named 'WeirdPet'.") + + assertEquals(buildError(AmbiguousSealedSubclassNames()), "Object type 'NormalPet' is a member of a known union, but no class could be found for that type name. Please pass a class for type 'NormalPet' in the parser's dictionary." + + " More than one subclass of sealed type ${AmbiguousSealedSubclassNames.Pet::class.java.name} is named 'NormalPet': " + + "${AmbiguousSealedSubclassNames.Pet.Cat.NormalPet::class.java.name}, ${AmbiguousSealedSubclassNames.Pet.Dog.NormalPet::class.java.name}.") + } + + class NonSealedInterfaceUnion : GraphQLQueryResolver { + fun pets(): List = listOf() + + interface Pet + + class NormalPet(val name: String?) : Pet + + class WeirdPet(val classification: String?) : Pet + } + + class PartlySealedUnion : GraphQLQueryResolver { + fun pets(): List = listOf() + + sealed class Pet { + class NormalPet(val name: String?) : Pet() + } + } + + class AmbiguousSealedSubclassNames : GraphQLQueryResolver { + fun pets(): List = listOf() + + sealed class Pet { + sealed class Cat : Pet() { + class NormalPet(val name: String?) : Cat() + } + + sealed class Dog : Pet() { + class NormalPet(val name: String?) : Dog() + } + } + } +}