diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt index 396670b9..ad13a6d1 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaClassScanner.kt @@ -16,7 +16,7 @@ import java.lang.reflect.Method * @author Andrew Potter */ internal class SchemaClassScanner( - initialDictionary: BiMap>, + initialDictionary: Map>, allDefinitions: List>, resolvers: List>, private val scalars: CustomScalarMap, @@ -59,6 +59,17 @@ internal class SchemaClassScanner( } } + // Input object types never go into the type => class bimap that interface and union type resolution use, so several of + // them can share a class, e.g. ObjectNode + initialDictionary + .filterKeys { definitionsByName[it] !is InputObjectTypeDefinition } + .entries.groupBy({ it.value }, { it.key }) + .forEach { (clazz, names) -> + if (names.size > 1) { + throw SchemaClassScannerError("Class in supplied dictionary '${clazz.name}' specified type names ${names.joinToString { "'$it'" }}, but only input object types can share a class!") + } + } + if (options.allowUnimplementedResolvers) { 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.") } diff --git a/src/main/kotlin/graphql/kickstart/tools/SchemaParserDictionary.kt b/src/main/kotlin/graphql/kickstart/tools/SchemaParserDictionary.kt index 116fa7ca..1fc10eeb 100644 --- a/src/main/kotlin/graphql/kickstart/tools/SchemaParserDictionary.kt +++ b/src/main/kotlin/graphql/kickstart/tools/SchemaParserDictionary.kt @@ -5,9 +5,14 @@ import kotlin.reflect.KClass class SchemaParserDictionary { - private val dictionary: BiMap> = BiMap.create() + // Not a BiMap, since several input types can share a class, e.g. ObjectNode. The scanner checks that other types don't. + private val dictionary = linkedMapOf>() - fun getDictionary(): BiMap> = BiMap.unmodifiableBiMap(dictionary) + /** + * Returns a copy of the entries added so far. If several types share a class, the inverse of the copy only maps that class + * back to the first of them. + */ + fun getDictionary(): BiMap> = BiMap.unmodifiableCopyOf(dictionary) /** * Add arbitrary classes to the parser's dictionary, overriding the generated type name. diff --git a/src/main/kotlin/graphql/kickstart/tools/util/BiMap.java b/src/main/kotlin/graphql/kickstart/tools/util/BiMap.java index 230f995c..e1ec9603 100644 --- a/src/main/kotlin/graphql/kickstart/tools/util/BiMap.java +++ b/src/main/kotlin/graphql/kickstart/tools/util/BiMap.java @@ -1,6 +1,7 @@ package graphql.kickstart.tools.util; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.Map; import java.util.Objects; import java.util.Set; @@ -21,6 +22,17 @@ public static BiMap unmodifiableBiMap(BiMap biMap) { return new BiMap<>(unmodifiableMap(biMap.delegate), unmodifiableMap(biMap.inverse)); } + /** + * Returns an unmodifiable bimap with the entries of the given map. Unlike {@link #put(Object, Object)}, several keys may + * have the same value. The inverse only maps such a value back to the first of its keys, so the inverse and + * {@link #values()} can have fewer entries than the bimap. + */ + public static BiMap unmodifiableCopyOf(Map map) { + Map inverse = new HashMap<>(); + map.forEach((key, value) -> inverse.putIfAbsent(value, key)); + return new BiMap<>(unmodifiableMap(new LinkedHashMap<>(map)), unmodifiableMap(inverse)); + } + private BiMap(Map delegate, Map inverse) { this.delegate = delegate; this.inverse = inverse; diff --git a/src/test/kotlin/graphql/kickstart/tools/SchemaParserDictionaryTest.kt b/src/test/kotlin/graphql/kickstart/tools/SchemaParserDictionaryTest.kt new file mode 100644 index 00000000..fffc1fbd --- /dev/null +++ b/src/test/kotlin/graphql/kickstart/tools/SchemaParserDictionaryTest.kt @@ -0,0 +1,190 @@ +package graphql.kickstart.tools + +import com.fasterxml.jackson.databind.node.ObjectNode +import graphql.GraphQL +import org.junit.Assert.assertThrows +import org.junit.Test + +class SchemaParserDictionaryTest { + + @Test + fun `input types can share a dictionary class`() { + val schema = SchemaParser.newParser() + .schemaString( + """ + type Query { + create(input: EntityCreateInput!): String + update(input: EntityUpdateInput!): String + } + + input EntityCreateInput { + name: String + a: NestedA + } + + input EntityUpdateInput { + id: ID! + b: [NestedB!] + } + + input NestedA { + x: Int + } + + input NestedB { + y: Int + } + """) + .resolvers(object : GraphQLQueryResolver { + fun create(input: ObjectNode): String = input.toString() + fun update(input: ObjectNode): String = input.toString() + }) + .dictionary("EntityCreateInput", ObjectNode::class.java) + .dictionary("EntityUpdateInput", ObjectNode::class.java) + // ObjectNode has no properties, so the classes of the nested inputs come from the dictionary + .dictionary("NestedA", ObjectNode::class.java) + .dictionary("NestedB", ObjectNode::class.java) + .build() + .makeExecutableSchema() + val gql = GraphQL.newGraphQL(schema).build() + + val data = assertNoGraphQlErrors(gql) { + """ + query { + create(input: { name: "n", a: { x: 1 } }) + update(input: { id: "2", b: [{ y: 3 }] }) + } + """ + } + + assertEquals(data["create"], """{"name":"n","a":{"x":1}}""") + assertEquals(data["update"], """{"id":"2","b":[{"y":3}]}""") + } + + @Test + fun `directive input types sharing a class can be passed to the builder from a SchemaParserDictionary`() { + val dictionary = SchemaParserDictionary() + .add("AInput", Map::class) + .add("BInput", Map::class) + + assertEquals(dictionary.getDictionary().toMap(), mapOf("AInput" to Map::class.java, "BInput" to Map::class.java)) + + // graphql-spring-boot reads a SchemaParserDictionary bean this way + val schema = SchemaParser.newParser() + .schemaString( + """ + directive @a(arg: AInput) on FIELD_DEFINITION + directive @b(arg: BInput) on FIELD_DEFINITION + + type Query { + test: String @a(arg: { x: 1 }) @b(arg: { y: "b" }) + } + + input AInput { + x: Int + } + + input BInput { + y: String + } + """) + .resolvers(object : GraphQLQueryResolver { + fun test(): String? = null + }) + .dictionary(dictionary.getDictionary()) + .build() + .makeExecutableSchema() + + val field = schema.queryType.getFieldDefinition("test") + assertEquals(field.getAppliedDirective("a").getArgument("arg")?.getValue>(), mapOf("x" to 1)) + assertEquals(field.getAppliedDirective("b").getArgument("arg")?.getValue>(), mapOf("y" to "b")) + } + + @Test + fun `input type and object type can share a dictionary class`() { + // DogInput is only used by a directive, so its class can only come from the dictionary + val schema = SchemaParser.newParser() + .schemaString( + """ + directive @d(arg: DogInput) on FIELD_DEFINITION + + type Query { + dog: Dog @d(arg: { name: "z" }) + } + + interface Dog { + name: String + } + + type LabradorRetriever implements Dog { + name: String + } + + input DogInput { + name: String + } + """) + .resolvers(object : GraphQLQueryResolver { + fun dog(): Dog = LabradorRetriever(name = "a", color = "black", fluffiness = 1, speed = 1f) + }) + .dictionary("LabradorRetriever", LabradorRetriever::class) + .dictionary("DogInput", LabradorRetriever::class) + .build() + .makeExecutableSchema() + + val field = schema.queryType.getFieldDefinition("dog") + assertEquals(field.getAppliedDirective("d").getArgument("arg")?.getValue>(), mapOf("name" to "z")) + + val gql = GraphQL.newGraphQL(schema).build() + val data = assertNoGraphQlErrors(gql) { + """ + query { + dog { + __typename + name + } + } + """ + } + + assertEquals(data["dog"], mapOf("__typename" to "LabradorRetriever", "name" to "a")) + } + + @Test + fun `object types can't share a dictionary class`() { + val error = assertThrows(SchemaClassScannerError::class.java) { + SchemaParser.newParser() + .schemaString( + """ + type Query { + a: A + b: B + } + + type A { + id: Int + } + + type B { + id: Int + } + + input AInput { + id: Int + } + """) + .resolvers(object : GraphQLQueryResolver { + fun a(): ThirdItem? = null + fun b(): ThirdItem? = null + }) + .dictionary("A", ThirdItem::class) + .dictionary("AInput", ThirdItem::class) + .dictionary("B", ThirdItem::class) + .build() + } + + // the input type sharing the class isn't part of the conflict + assertEquals(error.message, "Class in supplied dictionary '${ThirdItem::class.java.name}' specified type names 'A', 'B', " + + "but only input object types can share a class!") + } +} diff --git a/src/test/kotlin/graphql/kickstart/tools/util/BiMapTest.kt b/src/test/kotlin/graphql/kickstart/tools/util/BiMapTest.kt index fc753990..fdc56f30 100644 --- a/src/test/kotlin/graphql/kickstart/tools/util/BiMapTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/util/BiMapTest.kt @@ -67,4 +67,19 @@ class BiMapTest { val expected = setOf(1, 2, 3) assertEquals(expected, values) } + + @Test + fun `unmodifiable copy should keep keys with the same value`() { + val copy = BiMap.unmodifiableCopyOf(linkedMapOf("foo" to 1, "bar" to 2, "baz" to 1)) + + assertEquals(mapOf("foo" to 1, "bar" to 2, "baz" to 1), copy.toMap()) + assertEquals(listOf("foo", "bar", "baz"), copy.keys.toList()) + assertEquals(setOf(1, 2), copy.values) + assertEquals(mapOf(1 to "foo", 2 to "bar"), copy.inverse().toMap()) + } + + @Test(expected = UnsupportedOperationException::class) + fun `unmodifiable copy should not permit changes`() { + BiMap.unmodifiableCopyOf(mapOf("foo" to 1))["bar"] = 2 + } }