Replace BiMap with plain maps

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

@@ -1,7 +1,6 @@
 package graphql.kickstart.tools
 
 import graphql.TypeResolutionEnvironment
-import graphql.kickstart.tools.util.BiMap
 import graphql.kickstart.tools.util.JavaType
 import graphql.kickstart.tools.util.unwrap
 import graphql.language.TypeDefinition
@@ -15,7 +14,7 @@ import java.lang.reflect.ParameterizedType
  * @author Andrew Potter
  */
 internal abstract class DictionaryTypeResolver(
-        private val dictionary: BiMap<JavaType, TypeDefinition<*>>
+        private val dictionary: Map<JavaType, TypeDefinition<*>>
 ) : TypeResolver {
     // Raw class of each parameterized entry, e.g. Box for Box<String>, unless several entries share it
     private val rawClassDictionary by lazy {
@@ -46,7 +45,7 @@ internal abstract class DictionaryTypeResolver(
 }
 
 internal class InterfaceTypeResolver(
-    dictionary: BiMap<JavaType, TypeDefinition<*>>,
+    dictionary: Map<JavaType, TypeDefinition<*>>,
     private val thisInterface: GraphQLInterfaceType
 ) : DictionaryTypeResolver(
         dictionary
@@ -55,7 +54,7 @@ internal class InterfaceTypeResolver(
 }
 
 internal class UnionTypeResolver(
-    dictionary: BiMap<JavaType, TypeDefinition<*>>,
+    dictionary: Map<JavaType, TypeDefinition<*>>,
     private val thisUnion: GraphQLUnionType
 ) : DictionaryTypeResolver(
         dictionary

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

@@ -1,7 +1,6 @@
 package graphql.kickstart.tools
 
 import graphql.kickstart.tools.resolver.FieldResolver
-import graphql.kickstart.tools.util.BiMap
 import graphql.kickstart.tools.util.JavaType
 import graphql.language.EnumTypeDefinition
 import graphql.language.FieldDefinition
@@ -24,6 +23,15 @@ internal data class ScannedSchemaObjects(
     val directiveOnlyEnums: Set<EnumTypeDefinition>
 )
 
-internal typealias TypeClassDictionary = BiMap<TypeDefinition<*>, JavaType>
+internal class TypeClassDictionary(val classByType: Map<TypeDefinition<*>, JavaType>) {
+    val typeByClass: Map<JavaType, TypeDefinition<*>> = classByType.entries
+        .groupBy({ it.value }, { it.key })
+        .mapValues { (javaType, types) ->
+            types.singleOrNull() ?: throw SchemaClassScannerError(
+                "Class '${javaType.typeName}' is used for types ${types.joinToString { "'${it.name}'" }}, but only input object types can share a class!"
+            )
+        }
+}
+
 internal typealias CustomScalarMap = Map<String, GraphQLScalarType>
 internal typealias TypeExtensions = Map<String, List<TypeDefinition<*>>>

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

@@ -264,23 +264,18 @@ internal class SchemaClassScanner(
         val observedDefinitions = dictionary.keys.toSet() + unvalidatedTypes
 
         // The dictionary doesn't need to know what classes are used with scalars.
-        // In addition, scalars can have duplicate classes so that breaks the bi-map.
         // Input types can also be excluded from the dictionary, since it's only used for interfaces, unions, and enums.
         // Union types can also be excluded, as their possible types are resolved recursively later
-        val dictionary = try {
-            BiMap.unmodifiableBiMap(BiMap.create<TypeDefinition<*>, JavaType>().also {
-                dictionary
-                    .filter {
-                        it.value.javaType != null
-                            && it.value.typeClass() != Any::class.java
-                            && !Map::class.java.isAssignableFrom(it.value.typeClass())
-                            && it.key !is InputObjectTypeDefinition
-                            && it.key !is UnionTypeDefinition
-                    }.mapValuesTo(it) { it.value.javaType }
-            })
-        } catch (t: Throwable) {
-            throw SchemaClassScannerError("Error creating bimap of type => class", t)
-        }
+        val dictionary = TypeClassDictionary(
+            dictionary
+                .filter {
+                    it.value.javaType != null
+                        && it.value.typeClass() != Any::class.java
+                        && !Map::class.java.isAssignableFrom(it.value.typeClass())
+                        && it.key !is InputObjectTypeDefinition
+                        && it.key !is UnionTypeDefinition
+                }.mapValues { it.value.javaType!! }
+        )
         // SchemaParser builds the scalars, this only checks that each one was provided
         val scalars = observedDefinitions.filterIsInstance<ScalarTypeDefinition>()
             .filter {

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

@@ -104,8 +104,8 @@ class SchemaParser internal constructor(
         val enums = enumDefinitions.map { createEnumObject(it) }
 
         // Assign type resolver to interfaces now that we know all of the object types
-        interfaces.forEach { codeRegistryBuilder.typeResolver(it, InterfaceTypeResolver(dictionary.inverse(), it)) }
-        unions.forEach { codeRegistryBuilder.typeResolver(it, UnionTypeResolver(dictionary.inverse(), it)) }
+        interfaces.forEach { codeRegistryBuilder.typeResolver(it, InterfaceTypeResolver(dictionary.typeByClass, it)) }
+        unions.forEach { codeRegistryBuilder.typeResolver(it, UnionTypeResolver(dictionary.typeByClass, it)) }
 
         // Find query type and mutation/subscription type (if mutation/subscription type exists)
         val queryName = rootInfo.getQueryName()
@@ -240,7 +240,7 @@ class SchemaParser internal constructor(
     private fun createEnumObject(definition: EnumTypeDefinition): GraphQLEnumType {
         val name = definition.name
         // enums only used by directives can do without a class, see SchemaClassScanner.handleDirectiveEnum
-        val enumClass = dictionary[definition]?.unwrap()
+        val enumClass = dictionary.classByType[definition]?.unwrap()
         if (enumClass == null && definition !in directiveOnlyEnums) throw SchemaError("Expected enum with name '$name' but found none!")
         if (enumClass != null && !enumClass.isEnum) throw SchemaError("Type '$name' is declared as an enum in the GraphQL schema but is not a Java enum!")
 

src/main/kotlin/graphql/kickstart/tools/SchemaParserBuilder.kt

@@ -165,7 +165,7 @@ class SchemaParserBuilder {
         val definitions = appendDynamicDefinitions(parseDefinitions())
         val customScalars = scalars.associateBy { it.name }
 
-        return SchemaClassScanner(dictionary.getDictionary(), definitions, resolvers, customScalars, options)
+        return SchemaClassScanner(dictionary.toMap(), definitions, resolvers, customScalars, options)
             .scanForClasses()
     }
 

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

@@ -1,17 +1,25 @@
 package graphql.kickstart.tools
 
-import graphql.kickstart.tools.util.BiMap
 import kotlin.reflect.KClass
 
 class SchemaParserDictionary {
 
     private val dictionary = linkedMapOf<String, Class<*>>()
 
+    /**
+     * Returns a copy of the entries added so far, in the order they were added. Several names may share a class.
+     */
+    fun toMap(): Map<String, Class<*>> = dictionary.toMap()
+
+    // TODO remove in 15.0.0 (#900)
     /**
      * 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<String, Class<*>> = BiMap.unmodifiableCopyOf(dictionary)
+    @Deprecated("Use toMap() instead", ReplaceWith("toMap()"))
+    @Suppress("DEPRECATION")
+    fun getDictionary(): graphql.kickstart.tools.util.BiMap<String, Class<*>> =
+        graphql.kickstart.tools.util.BiMap.unmodifiableCopyOf(dictionary)
 
     /**
      * Add arbitrary classes to the parser's dictionary, overriding the generated type name.

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

@@ -8,8 +8,13 @@
 
 import static java.util.Collections.unmodifiableMap;
 
+// TODO remove in 15.0.0 (#900)
+/**
+ * @deprecated Use a plain {@link java.util.Map}.
+ */
+@Deprecated
 @SuppressWarnings("SuspiciousMethodCalls")
 public class BiMap<K, V> implements Map<K, V> {
 
     private final Map<K, V> delegate;
     private final Map<V, K> inverse;

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

@@ -7,6 +7,7 @@ import graphql.Scalars
 import graphql.execution.CoercedVariables
 import graphql.language.Value
 import graphql.schema.*
+import org.junit.Assert.assertThrows
 import org.junit.Test
 import java.util.*
 import java.util.concurrent.CompletableFuture
@@ -561,6 +562,136 @@ class SchemaClassScannerTest {
         var name: String? = null
     }
 
+    @Test
+    fun `scanner allows input object types to share a class`() {
+        val schema = SchemaParser.newParser()
+            .resolvers(object : GraphQLQueryResolver {
+                fun test(a: Pojo, b: Pojo): String = "${a.name} ${b.name}"
+            })
+            .schemaString(
+                """
+                type Query {
+                    test(a: AInput, b: BInput): String
+                }
+
+                input AInput {
+                    name: String
+                }
+
+                input BInput {
+                    name: String
+                }
+                """)
+            .build()
+            .makeExecutableSchema()
+            .assertEveryFieldResolves()
+
+        val data = assertNoGraphQlErrors(GraphQL.newGraphQL(schema).build()) {
+            """
+            query {
+                test(a: { name: "x" }, b: { name: "y" })
+            }
+            """
+        }
+
+        assertEquals(data["test"], "x y")
+    }
+
+    @Test
+    fun `scanner rejects two object types that share a class`() {
+        val error = assertThrows(SchemaClassScannerError::class.java) {
+            SchemaParser.newParser()
+                .resolvers(object : GraphQLQueryResolver {
+                    fun a(): SharedObject? = null
+                    fun b(): SharedObject? = null
+                })
+                .schemaString(
+                    """
+                    type Query {
+                        a: A
+                        b: B
+                    }
+
+                    type A {
+                        id: Int
+                    }
+
+                    type B {
+                        id: Int
+                    }
+                    """)
+                .build()
+        }
+
+        assertEquals(error.message, "Class '${SharedObject::class.java.name}' is used for types 'A', 'B', " +
+            "but only input object types can share a class!")
+    }
+
+    @Test
+    fun `scanner rejects an interface type and an object type that share a class`() {
+        val error = assertThrows(SchemaClassScannerError::class.java) {
+            SchemaParser.newParser()
+                .resolvers(object : GraphQLQueryResolver {
+                    fun node(): SharedObject? = null
+                })
+                .schemaString(
+                    """
+                    type Query {
+                        node: Node
+                    }
+
+                    interface Node {
+                        id: Int
+                    }
+
+                    type Item implements Node {
+                        id: Int
+                    }
+                    """)
+                .dictionary("Item", SharedObject::class)
+                .build()
+        }
+
+        assertEquals(error.message, "Class '${SharedObject::class.java.name}' is used for types 'Node', 'Item', " +
+            "but only input object types can share a class!")
+    }
+
+    @Test
+    fun `scanner rejects two enum types that share a class`() {
+        val error = assertThrows(SchemaClassScannerError::class.java) {
+            SchemaParser.newParser()
+                .resolvers(object : GraphQLQueryResolver {
+                    fun a(): SharedEnum? = null
+                    fun b(): SharedEnum? = null
+                })
+                .schemaString(
+                    """
+                    type Query {
+                        a: EnumA
+                        b: EnumB
+                    }
+
+                    enum EnumA {
+                        X
+                    }
+
+                    enum EnumB {
+                        X
+                    }
+                    """)
+                .build()
+        }
+
+        assertEquals(error.message, "Class '${SharedEnum::class.java.name}' is used for types 'EnumA', 'EnumB', " +
+            "but only input object types can share a class!")
+    }
+
+    class SharedObject {
+        var id: Int? = null
+    }
+
+    enum class SharedEnum { X }
+
     @Test
     fun `scanner should handle nested types in input types`() {
         val schema = SchemaParser.newParser()

src/test/kotlin/graphql/kickstart/tools/SchemaParserDictionaryTest.kt

@@ -67,7 +67,7 @@ class SchemaParserDictionaryTest {
             .add("AInput", Map::class)
             .add("BInput", Map::class)
 
-        assertEquals(dictionary.getDictionary().toMap(), mapOf("AInput" to Map::class.java, "BInput" to Map::class.java))
+        assertEquals(dictionary.toMap(), mapOf("AInput" to Map::class.java, "BInput" to Map::class.java))
 
         val schema = SchemaParser.newParser()
             .schemaString(
@@ -90,7 +90,7 @@ class SchemaParserDictionaryTest {
             .resolvers(object : GraphQLQueryResolver {
                 fun test(): String? = null
             })
-            .dictionary(dictionary.getDictionary())
+            .dictionary(dictionary.toMap())
             .build()
             .makeExecutableSchema()
             .assertEveryFieldResolves()
@@ -100,6 +100,30 @@ class SchemaParserDictionaryTest {
         assertEquals(field.getAppliedDirective("b").getArgument("arg")?.getValue<Map<*, *>>(), mapOf("y" to "b"))
     }
 
+    @Test
+    fun `toMap returns the entries in insertion order and keeps every name of a shared class`() {
+        val dictionary = SchemaParserDictionary()
+            .add("B", Map::class)
+            .add("A", String::class)
+            .add("C", Map::class)
+
+        assertEquals(dictionary.toMap().entries.map { it.key to it.value }, listOf("B" to Map::class.java, "A" to String::class.java, "C" to Map::class.java))
+    }
+
+    @Test
+    @Suppress("DEPRECATION")
+    fun `getDictionary keeps working and only maps a shared class back to its first name`() {
+        val dictionary = SchemaParserDictionary()
+            .add("B", Map::class)
+            .add("A", String::class)
+            .add("C", Map::class)
+
+        val deprecated = dictionary.getDictionary()
+
+        assertEquals(deprecated.toMap(), dictionary.toMap())
+        assertEquals(deprecated.inverse().toMap(), mapOf<Class<*>, String>(Map::class.java to "B", String::class.java to "A"))
+    }
+
     @Test
     fun `input type and object type can share a dictionary class`() {
         val schema = SchemaParser.newParser()

src/test/kotlin/graphql/kickstart/tools/util/BiMapTest.kt

@@ -1,3 +1,5 @@
+@file:Suppress("DEPRECATION")
+
 package graphql.kickstart.tools.util
 
 import graphql.kickstart.tools.assertEquals