Skip to content

Commit ee63ab3

Browse files
oryan-blockoryan-block
committed
Check that object, interface and enum types don't share a class instead of using a BiMap
1 parent fd35dbf commit ee63ab3

5 files changed

Lines changed: 157 additions & 24 deletions

File tree

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

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

33
import graphql.TypeResolutionEnvironment
4-
import graphql.kickstart.tools.util.BiMap
54
import graphql.kickstart.tools.util.JavaType
65
import graphql.kickstart.tools.util.unwrap
76
import graphql.language.TypeDefinition
@@ -15,7 +14,7 @@ import java.lang.reflect.ParameterizedType
1514
* @author Andrew Potter
1615
*/
1716
internal abstract class DictionaryTypeResolver(
18-
private val dictionary: BiMap<JavaType, TypeDefinition<*>>
17+
private val dictionary: Map<JavaType, TypeDefinition<*>>
1918
) : TypeResolver {
2019
// Raw class of each parameterized entry, e.g. Box for Box<String>, unless several entries share it
2120
private val rawClassDictionary by lazy {
@@ -46,7 +45,7 @@ internal abstract class DictionaryTypeResolver(
4645
}
4746

4847
internal class InterfaceTypeResolver(
49-
dictionary: BiMap<JavaType, TypeDefinition<*>>,
48+
dictionary: Map<JavaType, TypeDefinition<*>>,
5049
private val thisInterface: GraphQLInterfaceType
5150
) : DictionaryTypeResolver(
5251
dictionary
@@ -55,7 +54,7 @@ internal class InterfaceTypeResolver(
5554
}
5655

5756
internal class UnionTypeResolver(
58-
dictionary: BiMap<JavaType, TypeDefinition<*>>,
57+
dictionary: Map<JavaType, TypeDefinition<*>>,
5958
private val thisUnion: GraphQLUnionType
6059
) : DictionaryTypeResolver(
6160
dictionary

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

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

33
import graphql.kickstart.tools.resolver.FieldResolver
4-
import graphql.kickstart.tools.util.BiMap
54
import graphql.kickstart.tools.util.JavaType
65
import graphql.language.EnumTypeDefinition
76
import graphql.language.FieldDefinition
@@ -24,6 +23,15 @@ internal data class ScannedSchemaObjects(
2423
val directiveOnlyEnums: Set<EnumTypeDefinition>
2524
)
2625

27-
internal typealias TypeClassDictionary = BiMap<TypeDefinition<*>, JavaType>
26+
internal class TypeClassDictionary(val classByType: Map<TypeDefinition<*>, JavaType>) {
27+
val typeByClass: Map<JavaType, TypeDefinition<*>> = classByType.entries
28+
.groupBy({ it.value }, { it.key })
29+
.mapValues { (javaType, types) ->
30+
types.singleOrNull() ?: throw SchemaClassScannerError(
31+
"Class '${javaType.typeName}' is used for types ${types.joinToString { "'${it.name}'" }}, but only input object types can share a class!"
32+
)
33+
}
34+
}
35+
2836
internal typealias CustomScalarMap = Map<String, GraphQLScalarType>
2937
internal typealias TypeExtensions = Map<String, List<TypeDefinition<*>>>

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

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -264,23 +264,18 @@ internal class SchemaClassScanner(
264264
val observedDefinitions = dictionary.keys.toSet() + unvalidatedTypes
265265

266266
// The dictionary doesn't need to know what classes are used with scalars.
267-
// In addition, scalars can have duplicate classes so that breaks the bi-map.
268267
// Input types can also be excluded from the dictionary, since it's only used for interfaces, unions, and enums.
269268
// Union types can also be excluded, as their possible types are resolved recursively later
270-
val dictionary = try {
271-
BiMap.unmodifiableBiMap(BiMap.create<TypeDefinition<*>, JavaType>().also {
272-
dictionary
273-
.filter {
274-
it.value.javaType != null
275-
&& it.value.typeClass() != Any::class.java
276-
&& !Map::class.java.isAssignableFrom(it.value.typeClass())
277-
&& it.key !is InputObjectTypeDefinition
278-
&& it.key !is UnionTypeDefinition
279-
}.mapValuesTo(it) { it.value.javaType }
280-
})
281-
} catch (t: Throwable) {
282-
throw SchemaClassScannerError("Error creating bimap of type => class", t)
283-
}
269+
val dictionary = TypeClassDictionary(
270+
dictionary
271+
.filter {
272+
it.value.javaType != null
273+
&& it.value.typeClass() != Any::class.java
274+
&& !Map::class.java.isAssignableFrom(it.value.typeClass())
275+
&& it.key !is InputObjectTypeDefinition
276+
&& it.key !is UnionTypeDefinition
277+
}.mapValues { it.value.javaType!! }
278+
)
284279
// SchemaParser builds the scalars, this only checks that each one was provided
285280
val scalars = observedDefinitions.filterIsInstance<ScalarTypeDefinition>()
286281
.filter {

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,8 @@ class SchemaParser internal constructor(
104104
val enums = enumDefinitions.map { createEnumObject(it) }
105105

106106
// Assign type resolver to interfaces now that we know all of the object types
107-
interfaces.forEach { codeRegistryBuilder.typeResolver(it, InterfaceTypeResolver(dictionary.inverse(), it)) }
108-
unions.forEach { codeRegistryBuilder.typeResolver(it, UnionTypeResolver(dictionary.inverse(), it)) }
107+
interfaces.forEach { codeRegistryBuilder.typeResolver(it, InterfaceTypeResolver(dictionary.typeByClass, it)) }
108+
unions.forEach { codeRegistryBuilder.typeResolver(it, UnionTypeResolver(dictionary.typeByClass, it)) }
109109

110110
// Find query type and mutation/subscription type (if mutation/subscription type exists)
111111
val queryName = rootInfo.getQueryName()
@@ -240,7 +240,7 @@ class SchemaParser internal constructor(
240240
private fun createEnumObject(definition: EnumTypeDefinition): GraphQLEnumType {
241241
val name = definition.name
242242
// enums only used by directives can do without a class, see SchemaClassScanner.handleDirectiveEnum
243-
val enumClass = dictionary[definition]?.unwrap()
243+
val enumClass = dictionary.classByType[definition]?.unwrap()
244244
if (enumClass == null && definition !in directiveOnlyEnums) throw SchemaError("Expected enum with name '$name' but found none!")
245245
if (enumClass != null && !enumClass.isEnum) throw SchemaError("Type '$name' is declared as an enum in the GraphQL schema but is not a Java enum!")
246246

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

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import graphql.Scalars
77
import graphql.execution.CoercedVariables
88
import graphql.language.Value
99
import graphql.schema.*
10+
import org.junit.Assert.assertThrows
1011
import org.junit.Test
1112
import java.util.*
1213
import java.util.concurrent.CompletableFuture
@@ -561,6 +562,136 @@ class SchemaClassScannerTest {
561562
var name: String? = null
562563
}
563564

565+
@Test
566+
fun `scanner allows input object types to share a class`() {
567+
val schema = SchemaParser.newParser()
568+
.resolvers(object : GraphQLQueryResolver {
569+
fun test(a: Pojo, b: Pojo): String = "${a.name} ${b.name}"
570+
})
571+
.schemaString(
572+
"""
573+
type Query {
574+
test(a: AInput, b: BInput): String
575+
}
576+
577+
input AInput {
578+
name: String
579+
}
580+
581+
input BInput {
582+
name: String
583+
}
584+
""")
585+
.build()
586+
.makeExecutableSchema()
587+
.assertEveryFieldResolves()
588+
589+
val data = assertNoGraphQlErrors(GraphQL.newGraphQL(schema).build()) {
590+
"""
591+
query {
592+
test(a: { name: "x" }, b: { name: "y" })
593+
}
594+
"""
595+
}
596+
597+
assertEquals(data["test"], "x y")
598+
}
599+
600+
@Test
601+
fun `scanner rejects two object types that share a class`() {
602+
val error = assertThrows(SchemaClassScannerError::class.java) {
603+
SchemaParser.newParser()
604+
.resolvers(object : GraphQLQueryResolver {
605+
fun a(): SharedObject? = null
606+
fun b(): SharedObject? = null
607+
})
608+
.schemaString(
609+
"""
610+
type Query {
611+
a: A
612+
b: B
613+
}
614+
615+
type A {
616+
id: Int
617+
}
618+
619+
type B {
620+
id: Int
621+
}
622+
""")
623+
.build()
624+
}
625+
626+
assertEquals(error.message, "Class '${SharedObject::class.java.name}' is used for types 'A', 'B', " +
627+
"but only input object types can share a class!")
628+
}
629+
630+
@Test
631+
fun `scanner rejects an interface type and an object type that share a class`() {
632+
val error = assertThrows(SchemaClassScannerError::class.java) {
633+
SchemaParser.newParser()
634+
.resolvers(object : GraphQLQueryResolver {
635+
fun node(): SharedObject? = null
636+
})
637+
.schemaString(
638+
"""
639+
type Query {
640+
node: Node
641+
}
642+
643+
interface Node {
644+
id: Int
645+
}
646+
647+
type Item implements Node {
648+
id: Int
649+
}
650+
""")
651+
.dictionary("Item", SharedObject::class)
652+
.build()
653+
}
654+
655+
assertEquals(error.message, "Class '${SharedObject::class.java.name}' is used for types 'Node', 'Item', " +
656+
"but only input object types can share a class!")
657+
}
658+
659+
@Test
660+
fun `scanner rejects two enum types that share a class`() {
661+
val error = assertThrows(SchemaClassScannerError::class.java) {
662+
SchemaParser.newParser()
663+
.resolvers(object : GraphQLQueryResolver {
664+
fun a(): SharedEnum? = null
665+
fun b(): SharedEnum? = null
666+
})
667+
.schemaString(
668+
"""
669+
type Query {
670+
a: EnumA
671+
b: EnumB
672+
}
673+
674+
enum EnumA {
675+
X
676+
}
677+
678+
enum EnumB {
679+
X
680+
}
681+
""")
682+
.build()
683+
}
684+
685+
assertEquals(error.message, "Class '${SharedEnum::class.java.name}' is used for types 'EnumA', 'EnumB', " +
686+
"but only input object types can share a class!")
687+
}
688+
689+
class SharedObject {
690+
var id: Int? = null
691+
}
692+
693+
enum class SharedEnum { X }
694+
564695
@Test
565696
fun `scanner should handle nested types in input types`() {
566697
val schema = SchemaParser.newParser()

0 commit comments

Comments
 (0)