refactor: move types cache to class (#37)

smyrick · dariuszkuc · commit c3fb1ed498ef · 2018-10-08T08:47:40.000-05:00
* refactor: move types cache to class

* rename test file

* undo uneeded change
diff --git a/detekt_baseline.xml b/detekt_baseline.xml
@@ -15,4 +15,4 @@
     <ID>UnsafeCast:SchemaGeneratorAsyncTests.kt$SchemaGeneratorAsyncTests$schema.getObjectType("TopLevelQuery").getFieldDefinition("asynchronouslyDoSingle").type as GraphQLNonNull</ID>
     <ID>UnsafeCast:SchemaGeneratorAsyncTests.kt$SchemaGeneratorAsyncTests$schema.getObjectType("TopLevelQuery").getFieldDefinition("maybe").type as GraphQLNonNull</ID>
   </Whitelist>
-</SmellBaseline>
+</SmellBaseline>
diff --git a/src/main/kotlin/com/expedia/graphql/schema/exceptions/CouldNotGetJvmNameOfKTypeException.kt b/src/main/kotlin/com/expedia/graphql/schema/exceptions/CouldNotGetJvmNameOfKTypeException.kt
@@ -0,0 +1,9 @@
+package com.expedia.graphql.schema.exceptions
+
+import kotlin.reflect.KType
+
+/**
+ * Thrown when trying to generate a class and cannot resolve the jvm erasure name.
+ */
+class CouldNotGetJvmNameOfKTypeException(kType: KType?)
+    : RuntimeException("Could not get the name of the KClass $kType")
diff --git a/src/main/kotlin/com/expedia/graphql/schema/generator/SchemaGenerator.kt b/src/main/kotlin/com/expedia/graphql/schema/generator/SchemaGenerator.kt
@@ -4,10 +4,7 @@ import com.expedia.graphql.TopLevelObjectDef
 import com.expedia.graphql.schema.KotlinDataFetcher
 import com.expedia.graphql.schema.Parameter
 import com.expedia.graphql.schema.SchemaGeneratorConfig
-import com.expedia.graphql.schema.exceptions.ConflictingTypesException
-import com.expedia.graphql.schema.exceptions.CouldNotGetNameOfEnumException
 import com.expedia.graphql.schema.exceptions.InvalidInputFieldTypeException
-import com.expedia.graphql.schema.exceptions.TypeNotSupportedException
 import com.expedia.graphql.schema.extensions.directives
 import com.expedia.graphql.schema.extensions.getDeprecationReason
 import com.expedia.graphql.schema.extensions.graphQLDescription
@@ -52,7 +49,7 @@ internal class SchemaGenerator(
     private val config: SchemaGeneratorConfig
 ) {
 
-    private val typesCache: MutableMap<String, KGraphQLType> = mutableMapOf()
+    private val cache = TypesCache(config.supportedPackages)
     private val additionTypes = mutableSetOf<GraphQLType>()
     private val directives = mutableSetOf<GraphQLDirective>()
     private val reflections = Reflections(config.supportedPackages)
@@ -73,7 +70,7 @@ internal class SchemaGenerator(
 
     private fun addAdditionalTypes(builder: GraphQLSchema.Builder) {
         additionTypes
-            .filter { typesCache.none { (_, v) -> v.graphQLType.name == it.name } }
+            .filter { cache.doesNotContainGraphQLType(it) }
             .forEach { builder.additionalType(it) }
     }
 
@@ -83,9 +80,7 @@ internal class SchemaGenerator(
         val queryBuilder = GraphQLObjectType.Builder()
         queryBuilder.name(config.topLevelQueryName)
         for (query in queries) {
-            query.klazz.declaredMemberFunctions
-                .filter { config.hooks.isValidFunction(it) }
-                .filter { func -> functionFilters.all { it.invoke(func) } }
+            query.klazz.getValidFunctions(config.hooks)
                 .forEach {
                     val function = function(it, query.obj)
                     val functionFromHook = config.hooks.didGenerateQueryType(it, function)
@@ -101,9 +96,7 @@ internal class SchemaGenerator(
             mutationBuilder.name(config.topLevelMutationName)
 
             for (mutation in mutations) {
-                mutation.klazz.declaredMemberFunctions
-                    .filter { config.hooks.isValidFunction(it) }
-                    .filter { func -> functionFilters.all { it.invoke(func) } }
+                mutation.klazz.getValidFunctions(config.hooks)
                     .forEach {
                         val function = function(it, mutation.obj)
                         val functionFromHook = config.hooks.didGenerateMutationType(it, function)
@@ -163,10 +156,10 @@ internal class SchemaGenerator(
     private fun argument(parameter: KParameter): GraphQLArgument {
         throwIfInterfaceIsNotAuthorized(parameter)
         return GraphQLArgument.newArgument()
-                .name(parameter.name)
-                .description(parameter.graphQLDescription() ?: parameter.type.graphQLDescription())
-                .type(graphQLTypeOf(parameter.type, true) as GraphQLInputType)
-                .build()
+            .name(parameter.name)
+            .description(parameter.graphQLDescription() ?: parameter.type.graphQLDescription())
+            .type(graphQLTypeOf(parameter.type, true) as GraphQLInputType)
+            .build()
     }
 
     private fun graphQLTypeOf(type: KType, inputType: Boolean = false): GraphQLType {
@@ -178,21 +171,18 @@ internal class SchemaGenerator(
     }
 
     private fun objectFromReflection(type: KType, inputType: Boolean): GraphQLType {
-        val kClass = type.classifier as KClass<*>
-        val cacheKey = getCacheKey(kClass, type, inputType)
-        val cachedType = typesCache[cacheKey]
+        val cacheKey = TypesCacheKey(type, inputType)
+        val cachedType = cache.get(cacheKey)
 
         if (cachedType != null) {
-            val isSameNameButNotSameClass = cachedType.kClass != kClass
-            when {
-                isSameNameButNotSameClass -> throw ConflictingTypesException(cachedType.kClass, kClass)
-                else -> return cachedType.graphQLType
-            }
+            return cachedType
         }
 
+        val kClass = type.classifier as KClass<*>
         val graphQLType = getGraphQLType(kClass, inputType, type)
+        val kGraphQLType = KGraphQLType(kClass, graphQLType)
 
-        typesCache[cacheKey] = KGraphQLType(kClass, graphQLType)
+        cache.put(cacheKey, kGraphQLType)
 
         return graphQLType
     }
@@ -210,31 +200,6 @@ internal class SchemaGenerator(
     private fun KClass<*>.canBeGraphQLUnion(): Boolean =
             this.canBeGraphQLInterface() && this.declaredMemberProperties.isEmpty() && this.declaredMemberFunctions.isEmpty()
 
-    private fun getCacheKey(kClass: KClass<*>, type: KType, inputType: Boolean): String {
-        if (kClass.isSubclassOf(Enum::class)) {
-            return kClass.simpleName ?: throw CouldNotGetNameOfEnumException(kClass)
-        }
-        val cacheKeyFromTypeName = when {
-            kClass.isSubclassOf(List::class) -> "List<${type.arguments.first().type!!.jvmErasure.simpleName}>"
-            kClass.java.isArray -> "Array<${type.arguments.first().type!!.jvmErasure.simpleName}>"
-            else -> {
-                throwIfTypeIsNotSupported(type)
-                type.jvmErasure.simpleName
-            }
-        }
-
-        return "$cacheKeyFromTypeName:$inputType"
-    }
-
-    @Throws(TypeNotSupportedException::class)
-    private fun throwIfTypeIsNotSupported(type: KType) {
-        val qualifiedName = type.jvmErasure.qualifiedName ?: ""
-        val comesFromSupportedPackageName = qualifiedName.startsWith(config.supportedPackages)
-        if (!comesFromSupportedPackageName) {
-            throw TypeNotSupportedException(qualifiedName, config.supportedPackages)
-        }
-    }
-
     @Throws(InvalidInputFieldTypeException::class)
     private fun throwIfInterfaceIsNotAuthorized(parameter: KParameter) {
         if (parameter.type.jvmErasure.java.isInterface) throw InvalidInputFieldTypeException()
@@ -253,51 +218,45 @@ internal class SchemaGenerator(
     private fun listType(type: KType, inputType: Boolean): GraphQLList =
         GraphQLList.list(graphQLTypeOf(type.arguments.first().type!!, inputType))
 
-    private fun objectType(klass: KClass<*>, interfaceType: GraphQLInterfaceType? = null): GraphQLType {
+    private fun objectType(kClass: KClass<*>, interfaceType: GraphQLInterfaceType? = null): GraphQLType {
         val builder = GraphQLObjectType.newObject()
 
-        builder.name(klass.simpleName)
-        builder.description(klass.graphQLDescription())
+        builder.name(kClass.simpleName)
+        builder.description(kClass.graphQLDescription())
 
-        klass.directives().map {
+        kClass.directives().map {
             builder.withDirective(it)
             directives.add(it)
         }
 
         if (interfaceType != null) {
             builder.withInterface(interfaceType)
         } else {
-            klass.superclasses
+            kClass.superclasses
                 .asSequence()
                 .filter { it.canBeGraphQLInterface() && !it.canBeGraphQLUnion() }
                 .map { objectFromReflection(it.createType(), false) as GraphQLInterfaceType }
                 .forEach { builder.withInterface(it) }
         }
 
-        klass.declaredMemberProperties
-            .filter { config.hooks.isValidProperty(it) }
-            .filter { prop -> propertyFilters.all { it.invoke(prop) } }
+        kClass.getValidProperties(config.hooks)
             .forEach { builder.field(property(it)) }
 
-        klass.declaredMemberFunctions
-            .filter { config.hooks.isValidFunction(it) }
-            .filter { func -> functionFilters.all { it.invoke(func) } }
+        kClass.getValidFunctions(config.hooks)
             .forEach { builder.field(function(it)) }
 
         return builder.build()
     }
 
-    private fun inputObjectType(klass: KClass<*>): GraphQLType {
+    private fun inputObjectType(kClass: KClass<*>): GraphQLType {
         val builder = GraphQLInputObjectType.newInputObject()
-        val name = getGraphQLClassName(klass, true)
+        val name = getGraphQLClassName(kClass, true)
 
         builder.name(name)
-        builder.description(klass.graphQLDescription())
+        builder.description(kClass.graphQLDescription())
 
         // It does not make sense to run functions against the input types so we only process data fields
-        klass.declaredMemberProperties
-            .filter { config.hooks.isValidProperty(it) }
-            .filter { prop -> propertyFilters.all { it.invoke(prop) } }
+        kClass.getValidProperties(config.hooks)
             .forEach { builder.field(inputProperty(it)) }
 
         return builder.build()
@@ -319,20 +278,16 @@ internal class SchemaGenerator(
         builder.name(kClass.simpleName)
         builder.description(kClass.graphQLDescription())
 
-        kClass.declaredMemberProperties
-                .filter { config.hooks.isValidProperty(it) }
-                .filter { prop -> propertyFilters.all { it.invoke(prop) } }
-                .forEach { builder.field(property(it)) }
+        kClass.getValidProperties(config.hooks)
+            .forEach { builder.field(property(it)) }
 
-        kClass.declaredMemberFunctions
-                .filter { config.hooks.isValidFunction(it) }
-                .filter { func -> functionFilters.all { it.invoke(func) } }
-                .forEach { builder.field(function(it, abstract = true)) }
+        kClass.getValidFunctions(config.hooks)
+            .forEach { builder.field(function(it, abstract = true)) }
 
         builder.typeResolver { env: TypeResolutionEnvironment -> env.schema.getObjectType(env.getObject<Any>().javaClass.simpleName) }
         val interfaceType = builder.build()
 
-        val implementations = reflections.getSubTypesOf(Class.forName(kClass.javaObjectType.name))
+        val implementations = getSubTypesOf(kClass)
         implementations.forEach {
             additionTypes.add(objectType(it.kotlin, interfaceType))
         }
@@ -347,12 +302,15 @@ internal class SchemaGenerator(
         builder.description(kClass.graphQLDescription())
         builder.typeResolver { env: TypeResolutionEnvironment -> env.schema.getObjectType(env.getObject<Any>().javaClass.simpleName) }
 
-        val implementations = reflections.getSubTypesOf(Class.forName(kClass.javaObjectType.name))
+        val implementations = getSubTypesOf(kClass)
         implementations
             .forEach {
                 builder.possibleType(objectType(it.kotlin) as GraphQLObjectType)
             }
 
         return builder.build()
     }
+
+    private fun getSubTypesOf(kclass: KClass<*>): MutableSet<out Class<out Any>> =
+        reflections.getSubTypesOf(Class.forName(kclass.javaObjectType.name))
 }
diff --git a/src/main/kotlin/com/expedia/graphql/schema/generator/TypesCache.kt b/src/main/kotlin/com/expedia/graphql/schema/generator/TypesCache.kt
@@ -0,0 +1,81 @@
+package com.expedia.graphql.schema.generator
+
+import com.expedia.graphql.schema.exceptions.ConflictingTypesException
+import com.expedia.graphql.schema.exceptions.CouldNotGetJvmNameOfKTypeException
+import com.expedia.graphql.schema.exceptions.CouldNotGetNameOfEnumException
+import com.expedia.graphql.schema.exceptions.TypeNotSupportedException
+import com.expedia.graphql.schema.models.KGraphQLType
+import graphql.schema.GraphQLType
+import kotlin.reflect.KClass
+import kotlin.reflect.KType
+import kotlin.reflect.full.isSubclassOf
+import kotlin.reflect.jvm.jvmErasure
+
+internal data class TypesCacheKey(val type: KType, val inputType: Boolean)
+
+internal class TypesCache(private val supportedPackages: String) {
+
+    private val cache: MutableMap<String, KGraphQLType> = mutableMapOf()
+
+    fun get(cacheKey: TypesCacheKey): GraphQLType? {
+        val cacheKeyString = getCacheKeyString(cacheKey)
+        val cachedType = cache[cacheKeyString]
+
+        if (cachedType != null) {
+            val kClass = getKClassFromKType(cacheKey.type)
+            val isSameNameButNotSameClass = cachedType.kClass != kClass
+            when {
+                isSameNameButNotSameClass -> throw ConflictingTypesException(cachedType.kClass, kClass)
+                else -> return cachedType.graphQLType
+            }
+        }
+
+        return null
+    }
+
+    fun put(key: TypesCacheKey, kGraphQLType: KGraphQLType) = cache.put(getCacheKeyString(key), kGraphQLType)
+
+    fun doesNotContainGraphQLType(graphQLType: GraphQLType) =
+        cache.none { (_, v) -> v.graphQLType.name == graphQLType.name }
+
+    @Throws(CouldNotGetNameOfEnumException::class)
+    private fun getCacheKeyString(cacheKey: TypesCacheKey): String {
+        val kClass = getKClassFromKType(cacheKey.type)
+
+        if (kClass.isSubclassOf(Enum::class)) {
+            return kClass.simpleName ?: throw CouldNotGetNameOfEnumException(kClass)
+        }
+
+        val cacheKeyFromTypeName = when {
+            kClass.isSubclassOf(List::class) -> "List<${getJvmErasureNameFromList(cacheKey.type)}>"
+            kClass.java.isArray -> "Array<${getJvmErasureNameFromList(cacheKey.type)}>"
+            else -> getCacheTypeName(cacheKey.type)
+        }
+
+        return "$cacheKeyFromTypeName:${cacheKey.inputType}"
+    }
+
+    @Suppress("Detekt.UnsafeCast")
+    private fun getKClassFromKType(kType: KType) = kType.classifier as KClass<*>
+
+    private fun getCacheTypeName(kType: KType): String {
+        throwIfTypeIsNotSupported(kType)
+        return getJvmErasureName(kType)
+    }
+
+    private fun getJvmErasureNameFromList(type: KType): String =
+        getJvmErasureName(type.arguments.first().type)
+
+    @Throws(CouldNotGetJvmNameOfKTypeException::class)
+    private fun getJvmErasureName(kType: KType?): String =
+        kType?.jvmErasure?.simpleName ?: throw CouldNotGetJvmNameOfKTypeException(kType)
+
+    @Throws(TypeNotSupportedException::class)
+    private fun throwIfTypeIsNotSupported(type: KType) {
+        val qualifiedName = type.jvmErasure.qualifiedName ?: ""
+        val comesFromSupportedPackageName = qualifiedName.startsWith(supportedPackages)
+        if (!comesFromSupportedPackageName) {
+            throw TypeNotSupportedException(qualifiedName, supportedPackages)
+        }
+    }
+}
diff --git a/src/main/kotlin/com/expedia/graphql/schema/generator/kClassExtensions.kt b/src/main/kotlin/com/expedia/graphql/schema/generator/kClassExtensions.kt
@@ -0,0 +1,14 @@
+package com.expedia.graphql.schema.generator
+
+import com.expedia.graphql.schema.hooks.SchemaGeneratorHooks
+import kotlin.reflect.KClass
+import kotlin.reflect.full.declaredMemberFunctions
+import kotlin.reflect.full.declaredMemberProperties
+
+internal fun KClass<*>.getValidProperties(hooks: SchemaGeneratorHooks) = this.declaredMemberProperties
+    .filter { hooks.isValidProperty(it) }
+    .filter { prop -> propertyFilters.all { it.invoke(prop) } }
+
+internal fun KClass<*>.getValidFunctions(hooks: SchemaGeneratorHooks) = this.declaredMemberFunctions
+    .filter { hooks.isValidFunction(it) }
+    .filter { func -> functionFilters.all { it.invoke(func) } }
diff --git a/src/test/kotlin/com/expedia/graphql/schema/generator/KClassExtensionsTest.kt b/src/test/kotlin/com/expedia/graphql/schema/generator/KClassExtensionsTest.kt
diff --git a/src/test/kotlin/com/expedia/graphql/schema/generator/TypesCacheTest.kt b/src/test/kotlin/com/expedia/graphql/schema/generator/TypesCacheTest.kt
