Skip to content

Commit 3606993

Browse files
authored
Merge branch 'master' into bugfix/436
2 parents 2095a2f + a653953 commit 3606993

8 files changed

Lines changed: 806 additions & 5 deletions

File tree

‎pom.xml‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,35 @@
360360
</build>
361361

362362
<profiles>
363+
<profile>
364+
<!-- Tests of Java sealed classes, which need Java 17 to compile -->
365+
<id>java17-tests</id>
366+
<activation>
367+
<jdk>[17,)</jdk>
368+
</activation>
369+
<build>
370+
<plugins>
371+
<plugin>
372+
<groupId>org.apache.maven.plugins</groupId>
373+
<artifactId>maven-compiler-plugin</artifactId>
374+
<executions>
375+
<execution>
376+
<id>java17-test-compile</id>
377+
<goals>
378+
<goal>testCompile</goal>
379+
</goals>
380+
<configuration>
381+
<compileSourceRoots>
382+
<compileSourceRoot>${project.basedir}/src/test/java17</compileSourceRoot>
383+
</compileSourceRoots>
384+
<release>17</release>
385+
</configuration>
386+
</execution>
387+
</executions>
388+
</plugin>
389+
</plugins>
390+
</build>
391+
</profile>
363392
<profile>
364393
<id>release</id>
365394
<build>

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

Lines changed: 109 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,17 @@ import graphql.language.*
88
import graphql.schema.GraphQLScalarType
99
import graphql.schema.idl.ScalarInfo
1010
import org.apache.commons.lang3.ClassUtils
11+
import org.apache.commons.lang3.reflect.TypeUtils
1112
import org.slf4j.LoggerFactory
1213
import java.lang.reflect.Member
1314
import java.lang.reflect.Method
15+
import java.lang.reflect.ParameterizedType
1416

1517
/**
1618
* @author Andrew Potter
1719
*/
1820
internal class SchemaClassScanner(
19-
initialDictionary: BiMap<String, Class<*>>,
21+
initialDictionary: Map<String, Class<*>>,
2022
allDefinitions: List<Definition<*>>,
2123
resolvers: List<GraphQLResolver<*>>,
2224
private val scalars: CustomScalarMap,
@@ -59,6 +61,17 @@ internal class SchemaClassScanner(
5961
}
6062
}
6163

64+
// Input object types never go into the type => class bimap that interface and union type resolution use, so several of
65+
// them can share a class, e.g. ObjectNode
66+
initialDictionary
67+
.filterKeys { definitionsByName[it] !is InputObjectTypeDefinition }
68+
.entries.groupBy({ it.value }, { it.key })
69+
.forEach { (clazz, names) ->
70+
if (names.size > 1) {
71+
throw SchemaClassScannerError("Class in supplied dictionary '${clazz.name}' specified type names ${names.joinToString { "'$it'" }}, but only input object types can share a class!")
72+
}
73+
}
74+
6275
if (options.allowUnimplementedResolvers) {
6376
log.warn("Option 'allowUnimplementedResolvers' should only be set to true during development, as it can cause schema errors to be moved to query time instead of schema creation time. Make sure this is turned off in production.")
6477
}
@@ -348,14 +361,103 @@ internal class SchemaClassScanner(
348361
}
349362

350363
private fun handleDictionaryTypes(types: List<TypeDefinition<*>>, failureMessage: (TypeDefinition<*>) -> String) {
364+
val typesWithoutClass = mutableListOf<TypeDefinition<*>>()
351365
types.forEach { type ->
352366
val dictionaryContainsType = dictionary.filter { it.key.name == type.name }.isNotEmpty()
353367
if (!unvalidatedTypes.contains(type) && !dictionaryContainsType) {
354368
val initialEntry = initialDictionary[type.name]
355-
?: throw SchemaClassScannerError(failureMessage(type))
356-
handleFoundType(type, initialEntry.get(), DictionaryReference())
369+
if (initialEntry != null) {
370+
handleFoundType(type, initialEntry.get(), DictionaryReference())
371+
} else {
372+
typesWithoutClass.add(type)
373+
}
374+
}
375+
}
376+
377+
// Scanning a sealed subclass can find the classes of the other types, so only fail when no type has exactly one sealed
378+
// subclass named after it. Types still without a class fail on the next pass.
379+
val sealedMatches = typesWithoutClass.filterIsInstance<ObjectTypeDefinition>().mapNotNull { sealedSubclassMatches(it).singleOrNull() }
380+
if (sealedMatches.isEmpty()) {
381+
typesWithoutClass.firstOrNull()?.let { throw SchemaClassScannerError(failureMessage(it) + sealedSubclassHint(it)) }
382+
}
383+
384+
sealedMatches.forEach { handleFoundType(it) }
385+
}
386+
387+
/**
388+
* Explains why no sealed subclass was used for the type, if any of its unions or interfaces has a sealed class.
389+
*/
390+
private fun sealedSubclassHint(type: TypeDefinition<*>): String {
391+
if (type !is ObjectTypeDefinition) {
392+
return ""
393+
}
394+
395+
val sealedTypes = sealedTypesOf(type)
396+
if (sealedTypes.isEmpty()) {
397+
return ""
398+
}
399+
400+
val sealedTypeNames = sealedTypes.joinToString(" or ") { it.unwrap().name }
401+
val matches = sealedSubclassMatches(type)
402+
return if (matches.isEmpty()) {
403+
" No subclass of sealed type $sealedTypeNames is named '${type.name}'."
404+
} else {
405+
" More than one subclass of sealed type $sealedTypeNames is named '${type.name}': ${matches.joinToString { it.javaType.unwrap().name }}."
406+
}
407+
}
408+
409+
/**
410+
* A member of a union or an implementor of an interface may never be returned directly, e.g. `NormalPet` when only
411+
* `pets(): List<Pet>` returns `union Pet`. If the class found for the union or interface is sealed, its subclass named
412+
* after the object type can be used instead.
413+
*/
414+
private fun sealedSubclassMatches(type: ObjectTypeDefinition): List<TypeClassMatcher.ValidMatch> =
415+
sealedTypesOf(type)
416+
.flatMap { sealedType ->
417+
sealedSubclasses(sealedType.unwrap())
418+
.filter { it.simpleName == type.name }
419+
.map { TypeClassMatcher.ValidMatch(type, parameterize(it, sealedType), SealedSubclassReference(sealedType)) }
420+
}
421+
// a subclass is found twice when it's under two of these sealed types, or extends both a sealed type and one of its
422+
// sealed subclasses
423+
.distinctBy { it.javaType.unwrap() }
424+
425+
/**
426+
* The sealed classes found for the unions the type is a member of and the interfaces it implements.
427+
*/
428+
private fun sealedTypesOf(type: ObjectTypeDefinition): List<JavaType> =
429+
dictionary
430+
.filterKeys { definition ->
431+
when (definition) {
432+
is UnionTypeDefinition -> definition.memberTypes.filterIsInstance<TypeName>().any { it.name == type.name }
433+
is InterfaceTypeDefinition -> type.implements.filterIsInstance<TypeName>().any { it.name == definition.name }
434+
else -> false
435+
}
357436
}
437+
.values.mapNotNull { it.javaType }
438+
.distinct()
439+
.filter { it.unwrap().kotlin.isSealed }
440+
441+
/**
442+
* All subclasses of a sealed class, including those of its sealed subclasses. kotlin-reflect also returns the permitted
443+
* subclasses of Java sealed classes.
444+
*/
445+
private fun sealedSubclasses(clazz: Class<*>): List<Class<*>> =
446+
clazz.kotlin.sealedSubclasses.flatMap { listOf(it.java) + sealedSubclasses(it.java) }
447+
448+
/**
449+
* Passes the type arguments of a parameterized sealed class on to its generic subclass, e.g. `Success<Pet>` for
450+
* `Result<Pet>`, so the types of the subclass's fields can be resolved.
451+
*/
452+
private fun parameterize(subclass: Class<*>, sealedType: JavaType): JavaType {
453+
if (sealedType !is ParameterizedType || subclass.typeParameters.isEmpty()) {
454+
return subclass
358455
}
456+
457+
return TypeUtils.determineTypeArguments(subclass, sealedType)
458+
?.takeIf { it.keys.containsAll(subclass.typeParameters.asList()) }
459+
?.let { TypeUtils.parameterize(subclass, it) }
460+
?: subclass
359461
}
360462

361463
/**
@@ -625,6 +727,10 @@ internal class SchemaClassScanner(
625727
override fun getDescription() = "interface declarations of ${type.name}"
626728
}
627729

730+
private class SealedSubclassReference(private val sealedType: JavaType) : Reference() {
731+
override fun getDescription() = "subclass of sealed type ${sealedType.typeName}"
732+
}
733+
628734
private class InputObjectReference(private val type: InputValueDefinition) : Reference() {
629735
override fun getDescription() = "input object $type"
630736
}

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,14 @@ import kotlin.reflect.KClass
55

66
class SchemaParserDictionary {
77

8-
private val dictionary: BiMap<String, Class<*>> = BiMap.create()
8+
// Not a BiMap, since several input types can share a class, e.g. ObjectNode. The scanner checks that other types don't.
9+
private val dictionary = linkedMapOf<String, Class<*>>()
910

10-
fun getDictionary(): BiMap<String, Class<*>> = BiMap.unmodifiableBiMap(dictionary)
11+
/**
12+
* Returns a copy of the entries added so far. If several types share a class, the inverse of the copy only maps that class
13+
* back to the first of them.
14+
*/
15+
fun getDictionary(): BiMap<String, Class<*>> = BiMap.unmodifiableCopyOf(dictionary)
1116

1217
/**
1318
* Add arbitrary classes to the parser's dictionary, overriding the generated type name.

‎src/main/kotlin/graphql/kickstart/tools/util/BiMap.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package graphql.kickstart.tools.util;
22

33
import java.util.HashMap;
4+
import java.util.LinkedHashMap;
45
import java.util.Map;
56
import java.util.Objects;
67
import java.util.Set;
@@ -21,6 +22,17 @@ public static <K, V> BiMap<K, V> unmodifiableBiMap(BiMap<K, V> biMap) {
2122
return new BiMap<>(unmodifiableMap(biMap.delegate), unmodifiableMap(biMap.inverse));
2223
}
2324

25+
/**
26+
* Returns an unmodifiable bimap with the entries of the given map. Unlike {@link #put(Object, Object)}, several keys may
27+
* have the same value. The inverse only maps such a value back to the first of its keys, so the inverse and
28+
* {@link #values()} can have fewer entries than the bimap.
29+
*/
30+
public static <K, V> BiMap<K, V> unmodifiableCopyOf(Map<K, V> map) {
31+
Map<V, K> inverse = new HashMap<>();
32+
map.forEach((key, value) -> inverse.putIfAbsent(value, key));
33+
return new BiMap<>(unmodifiableMap(new LinkedHashMap<>(map)), unmodifiableMap(inverse));
34+
}
35+
2436
private BiMap(Map<K, V> delegate, Map<V, K> inverse) {
2537
this.delegate = delegate;
2638
this.inverse = inverse;
Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
package graphql.kickstart.tools;
2+
3+
import graphql.ExecutionInput;
4+
import graphql.ExecutionResult;
5+
import graphql.GraphQL;
6+
import graphql.schema.GraphQLSchema;
7+
import org.junit.Test;
8+
9+
import java.util.List;
10+
import java.util.Map;
11+
12+
import static org.junit.Assert.assertEquals;
13+
import static org.junit.Assert.assertTrue;
14+
15+
// Sealed classes need Java 17, so the java17-tests profile only compiles and runs this on Java 17 and later.
16+
public class SchemaClassScannerSealedJavaTypesTest {
17+
18+
@Test
19+
public void scannerFindsUnionMembersAndInterfaceImplementorsAmongThePermittedSubclassesOfSealedTypes() {
20+
GraphQLSchema schema = SchemaParser.newParser()
21+
.schemaString("""
22+
type Query {
23+
pets: [Pet]
24+
animals: [Animal]
25+
}
26+
27+
union Pet = Dog | Fish
28+
29+
type Dog {
30+
name: String
31+
}
32+
33+
type Fish {
34+
fins: Int
35+
}
36+
37+
interface Animal {
38+
name: String
39+
}
40+
41+
type Cat implements Animal {
42+
name: String
43+
lives: Int
44+
}
45+
46+
type Bird implements Animal {
47+
name: String
48+
wings: Int
49+
}
50+
""")
51+
.resolvers(new PetQuery(), new AnimalQuery())
52+
.build()
53+
.makeExecutableSchema();
54+
55+
ExecutionResult result = GraphQL.newGraphQL(schema).build().execute(ExecutionInput.newExecutionInput().query("""
56+
query {
57+
pets { __typename ... on Dog { name } ... on Fish { fins } }
58+
animals { __typename name ... on Cat { lives } ... on Bird { wings } }
59+
}
60+
"""));
61+
62+
assertTrue(result.getErrors().toString(), result.getErrors().isEmpty());
63+
assertEquals(Map.of(
64+
"pets", List.of(Map.of("__typename", "Dog", "name", "Rex"), Map.of("__typename", "Fish", "fins", 2)),
65+
"animals", List.of(Map.of("__typename", "Cat", "name", "Tom", "lives", 9), Map.of("__typename", "Bird", "name", "Tweety", "wings", 2))
66+
), result.getData());
67+
}
68+
69+
public static class PetQuery implements GraphQLQueryResolver {
70+
public List<Pet> pets() {
71+
return List.of(new Dog("Rex"), new Fish(2));
72+
}
73+
}
74+
75+
public sealed interface Pet permits Mammal, Fish {
76+
}
77+
78+
public sealed interface Mammal extends Pet permits Dog {
79+
}
80+
81+
public record Dog(String name) implements Mammal {
82+
}
83+
84+
public record Fish(Integer fins) implements Pet {
85+
}
86+
87+
public static class AnimalQuery implements GraphQLQueryResolver {
88+
public List<Animal> animals() {
89+
return List.of(new Cat("Tom", 9), new Bird("Tweety", 2));
90+
}
91+
}
92+
93+
public abstract static sealed class Animal permits Cat, Bird {
94+
final String name;
95+
96+
Animal(String name) {
97+
this.name = name;
98+
}
99+
}
100+
101+
public static final class Cat extends Animal {
102+
final Integer lives;
103+
104+
Cat(String name, Integer lives) {
105+
super(name);
106+
this.lives = lives;
107+
}
108+
}
109+
110+
public static final class Bird extends Animal {
111+
final Integer wings;
112+
113+
Bird(String name, Integer wings) {
114+
super(name);
115+
this.wings = wings;
116+
}
117+
}
118+
}

0 commit comments

Comments
 (0)