refactor: simplify type cache (#127)

* refactor: simplify type cache

* test: recursive example

example/src/main/kotlin/com/expedia/graphql/sample/extension/CustomSchemaGeneratorHooks.kt

@@ -10,7 +10,6 @@ import graphql.schema.GraphQLScalarType
 import graphql.schema.GraphQLType
 import java.util.UUID
 import javax.validation.Validator
-import kotlin.reflect.KClass
 import kotlin.reflect.KType
 
 /**
@@ -21,7 +20,7 @@ class CustomSchemaGeneratorHooks(validator: Validator, private val directiveWiri
     /**
      * Register additional GraphQL scalar types.
      */
-    override fun willGenerateGraphQLType(type: KType): GraphQLType? = when (type.classifier as? KClass<*>) {
+    override fun willGenerateGraphQLType(type: KType): GraphQLType? = when (type.classifier) {
         UUID::class -> graphqlUUIDType
         else -> null
     }

example/src/main/kotlin/com/expedia/graphql/sample/model/Node.kt

@@ -0,0 +1,12 @@
+package com.expedia.graphql.sample.model
+
+/**
+ * Represents a recursive type that references itself,
+ * similar to a tree node structure.
+ */
+data class Node(
+    val id: Int,
+    val value: String,
+    val parent: Node?,
+    var children: List<Node>
+)

example/src/main/kotlin/com/expedia/graphql/sample/query/RecursiveQuery.kt

@@ -0,0 +1,22 @@
+package com.expedia.graphql.sample.query
+
+import com.expedia.graphql.annotations.GraphQLDescription
+import com.expedia.graphql.sample.model.Node
+import org.springframework.stereotype.Component
+
+@Component
+class RecursiveQuery : Query {
+
+    private final val root = Node(id = 0, value = "root", parent = null, children = emptyList())
+    private final val nodeA = Node(id = 1, value = "A", parent = root, children = emptyList())
+    private final val nodeB = Node(id = 2, value = "B", parent = root, children = emptyList())
+    private final val nodeC = Node(id = 3, value = "C", parent = nodeB, children = emptyList())
+
+    init {
+        root.children = listOf(nodeA, nodeB)
+        nodeB.children = listOf(nodeC)
+    }
+
+    @GraphQLDescription("Returns the root of a node graph")
+    fun nodeGraph(): Node = root
+}

src/main/kotlin/com/expedia/graphql/exceptions/TypeNotSupportedException.kt

@@ -1,7 +1,9 @@
 package com.expedia.graphql.exceptions
 
+import kotlin.reflect.KType
+
 /**
  * Thrown when the generator does not have a type to map to in GraphQL or in the hooks.
  */
-class TypeNotSupportedException(typeName: String, packageList: List<String>)
-    : GraphQLKotlinException("Cannot convert $typeName since it is outside the supported packages $packageList")
+class TypeNotSupportedException(kType: KType, packageList: List<String>)
+    : GraphQLKotlinException("Cannot convert $kType since it is outside the supported packages $packageList")

src/main/kotlin/com/expedia/graphql/generator/extensions/kClassExtensions.kt

@@ -38,6 +38,8 @@ internal fun KClass<*>.isList(): Boolean = this.isSubclassOf(List::class)
 
 internal fun KClass<*>.isArray(): Boolean = this.java.isArray
 
+internal fun KClass<*>.isListType(): Boolean = this.isList() || this.isArray()
+
 @Throws(CouldNotGetNameOfKClassException::class)
 internal fun KClass<*>.getSimpleName(): String =
     this.simpleName ?: throw CouldNotGetNameOfKClassException(this)

src/main/kotlin/com/expedia/graphql/generator/extensions/kParameterExtensions.kt

@@ -4,9 +4,8 @@ import com.expedia.graphql.annotations.GraphQLContext
 import com.expedia.graphql.exceptions.CouldNotGetNameOfKParameterException
 import kotlin.reflect.KParameter
 import kotlin.reflect.full.findAnnotation
-import kotlin.reflect.jvm.jvmErasure
 
-internal fun KParameter.isInterface() = this.type.jvmErasure.isInterface()
+internal fun KParameter.isInterface() = this.type.getKClass().isInterface()
 
 internal fun KParameter.isGraphQLContext() = this.findAnnotation<GraphQLContext>() != null
 

src/main/kotlin/com/expedia/graphql/generator/extensions/kTypeExtensions.kt

@@ -1,38 +1,45 @@
 package com.expedia.graphql.generator.extensions
 
-import com.expedia.graphql.exceptions.CouldNotCastToKClassException
-import com.expedia.graphql.exceptions.CouldNotGetNameOfKTypeException
 import com.expedia.graphql.exceptions.InvalidListTypeException
-import kotlin.reflect.KClass
 import kotlin.reflect.KType
 import kotlin.reflect.full.createType
-import kotlin.reflect.full.isSubclassOf
 import kotlin.reflect.jvm.jvmErasure
 
+private val primitiveArrayTypes = mapOf(
+    IntArray::class to Int::class,
+    LongArray::class to Long::class,
+    ShortArray::class to Short::class,
+    FloatArray::class to Float::class,
+    DoubleArray::class to Double::class,
+    CharArray::class to Char::class,
+    BooleanArray::class to Boolean::class
+)
+
+internal fun KType.getKClass() = this.jvmErasure
+
 @Throws(InvalidListTypeException::class)
 internal fun KType.getTypeOfFirstArgument(): KType =
     this.arguments.firstOrNull()?.type ?: throw InvalidListTypeException(this)
 
-@Throws(CouldNotCastToKClassException::class)
-internal fun KType.getKClass() = this.classifier as? KClass<*> ?: throw CouldNotCastToKClassException(this)
-
-internal fun KType.getArrayType(): KType {
-    val kClass = this.getKClass()
+internal fun KType.getWrappedType(): KType {
+    val primitiveClass = primitiveArrayTypes[this.getKClass()]
     return when {
-        kClass.isSubclassOf(IntArray::class) -> Int::class.createType()
-        kClass.isSubclassOf(LongArray::class) -> Long::class.createType()
-        kClass.isSubclassOf(ShortArray::class) -> Short::class.createType()
-        kClass.isSubclassOf(FloatArray::class) -> Float::class.createType()
-        kClass.isSubclassOf(DoubleArray::class) -> Double::class.createType()
-        kClass.isSubclassOf(CharArray::class) -> Char::class.createType()
-        kClass.isSubclassOf(BooleanArray::class) -> Boolean::class.createType()
+        primitiveClass != null -> primitiveClass.createType()
         else -> this.getTypeOfFirstArgument()
     }
 }
 
-@Throws(CouldNotGetNameOfKTypeException::class)
-internal fun KType.getSimpleName(): String =
-    this.jvmErasure.simpleName ?: throw CouldNotGetNameOfKTypeException(this)
+internal fun KType.getWrappedName(): String {
+    val isPrimitiveArray = primitiveArrayTypes.containsKey(this.getKClass())
+    return when {
+        isPrimitiveArray -> this.getSimpleName()
+        this.getKClass().isList() -> "List<${this.getWrappedType().getSimpleName()}>"
+        this.getKClass().isArray() -> "Array<${this.getWrappedType().getSimpleName()}>"
+        else -> this.getSimpleName()
+    }
+}
+
+internal fun KType.getSimpleName(): String = this.getKClass().getSimpleName()
 
 internal val KType.qualifiedName: String
-    get() = this.jvmErasure.getQualifiedName()
+    get() = this.getKClass().getQualifiedName()

src/main/kotlin/com/expedia/graphql/generator/state/TypesCache.kt

@@ -4,9 +4,8 @@ import com.expedia.graphql.exceptions.ConflictingTypesException
 import com.expedia.graphql.exceptions.TypeNotSupportedException
 import com.expedia.graphql.generator.extensions.getKClass
 import com.expedia.graphql.generator.extensions.getSimpleName
-import com.expedia.graphql.generator.extensions.getTypeOfFirstArgument
-import com.expedia.graphql.generator.extensions.isArray
-import com.expedia.graphql.generator.extensions.isList
+import com.expedia.graphql.generator.extensions.getWrappedName
+import com.expedia.graphql.generator.extensions.isListType
 import com.expedia.graphql.generator.extensions.qualifiedName
 import graphql.schema.GraphQLType
 import graphql.schema.GraphQLTypeReference
@@ -53,53 +52,23 @@ internal class TypesCache(private val supportedPackages: List<String>) {
             return kClass.getSimpleName()
         }
 
-        val cacheKeyFromTypeName = when {
-            kClass.isList() -> "List<${getNameOfFirstArgument(cacheKey.type)}>"
-            kClass.isArray() -> getArrayTypeName(kClass, cacheKey.type)
-            else -> getCacheTypeName(cacheKey.type)
+        if (isTypeNotSupported(cacheKey.type)) {
+            throw TypeNotSupportedException(cacheKey.type, supportedPackages)
         }
 
-        return "$cacheKeyFromTypeName:${cacheKey.inputType}"
-    }
-
-    private fun getArrayTypeName(kClass: KClass<*>, kType: KType): String {
-        val kClassName = getArrayTypeNameFromKClass(kClass)
-
-        return when {
-            kClassName != null -> kClassName
-            else -> "Array<${getNameOfFirstArgument(kType)}>"
-        }
-    }
-
-    private fun getArrayTypeNameFromKClass(kClass: KClass<*>): String? = when {
-        kClass.isSubclassOf(IntArray::class) -> IntArray::class.simpleName
-        kClass.isSubclassOf(LongArray::class) -> LongArray::class.simpleName
-        kClass.isSubclassOf(ShortArray::class) -> ShortArray::class.simpleName
-        kClass.isSubclassOf(FloatArray::class) -> FloatArray::class.simpleName
-        kClass.isSubclassOf(DoubleArray::class) -> DoubleArray::class.simpleName
-        kClass.isSubclassOf(CharArray::class) -> CharArray::class.simpleName
-        kClass.isSubclassOf(BooleanArray::class) -> BooleanArray::class.simpleName
-        else -> null
-    }
+        val cacheKeyFromTypeName = cacheKey.type.getWrappedName()
 
-    private fun getCacheTypeName(kType: KType): String {
-        throwIfTypeIsNotSupported(kType)
-        return kType.getSimpleName()
+        return "$cacheKeyFromTypeName:${cacheKey.inputType}"
     }
 
-    private fun getNameOfFirstArgument(type: KType): String =
-        type.getTypeOfFirstArgument().getSimpleName()
-
-    @Throws(TypeNotSupportedException::class)
-    private fun throwIfTypeIsNotSupported(type: KType) {
-        val qualifiedName = type.qualifiedName
+    private fun isTypeNotSupported(type: KType): Boolean {
 
-        val comesFromSupportedPackageName = supportedPackages.any {
-            qualifiedName.startsWith(it)
+        if (type.getKClass().isListType()) {
+            return false
         }
 
-        if (!comesFromSupportedPackageName) {
-            throw TypeNotSupportedException(qualifiedName, supportedPackages)
+        return supportedPackages.none {
+            type.qualifiedName.startsWith(it)
         }
     }
 

src/main/kotlin/com/expedia/graphql/generator/types/ListTypeBuilder.kt

@@ -1,6 +1,6 @@
 package com.expedia.graphql.generator.types
 
-import com.expedia.graphql.generator.extensions.getArrayType
+import com.expedia.graphql.generator.extensions.getWrappedType
 import com.expedia.graphql.generator.extensions.getTypeOfFirstArgument
 import com.expedia.graphql.generator.SchemaGenerator
 import com.expedia.graphql.generator.TypeBuilder
@@ -9,7 +9,7 @@ import kotlin.reflect.KType
 
 internal class ListTypeBuilder(generator: SchemaGenerator) : TypeBuilder(generator) {
     internal fun arrayType(type: KType, inputType: Boolean): GraphQLList =
-        GraphQLList.list(graphQLTypeOf(type.getArrayType(), inputType))
+        GraphQLList.list(graphQLTypeOf(type.getWrappedType(), inputType))
 
     internal fun listType(type: KType, inputType: Boolean): GraphQLList =
         GraphQLList.list(graphQLTypeOf(type.getTypeOfFirstArgument(), inputType))

src/test/kotlin/com/expedia/graphql/generator/SchemaGeneratorTest.kt

@@ -5,6 +5,7 @@ import com.expedia.graphql.annotations.GraphQLDescription
 import com.expedia.graphql.annotations.GraphQLID
 import com.expedia.graphql.annotations.GraphQLIgnore
 import com.expedia.graphql.exceptions.ConflictingTypesException
+import com.expedia.graphql.exceptions.GraphQLKotlinException
 import com.expedia.graphql.exceptions.InvalidIdTypeException
 import com.expedia.graphql.extensions.deepName
 import com.expedia.graphql.testSchemaConfig
@@ -197,11 +198,18 @@ class SchemaGeneratorTest {
 
     @Test
     fun `SchemaGenerator throws when encountering java stdlib`() {
-        assertFailsWith(RuntimeException::class) {
+        assertFailsWith(GraphQLKotlinException::class) {
             toSchema(listOf(TopLevelObject(QueryWithJavaClass())), config = testSchemaConfig)
         }
     }
 
+    @Test
+    fun `SchemaGenerator throws when encountering list of java stdlib`() {
+        assertFailsWith(GraphQLKotlinException::class) {
+            toSchema(listOf(TopLevelObject(QueryWithListOfJavaClass())), config = testSchemaConfig)
+        }
+    }
+
     @Test
     fun `SchemaGenerator throws when encountering conflicting types`() {
         assertFailsWith(ConflictingTypesException::class) {
@@ -369,6 +377,10 @@ class SchemaGeneratorTest {
         fun query(): java.net.CookieManager? = CookieManager()
     }
 
+    class QueryWithListOfJavaClass {
+        fun listQuery(): List<java.net.CookieManager> = listOf(CookieManager())
+    }
+
     class QueryWithConflictingTypes {
         @GraphQLDescription("A conflicting GraphQL query method")
         fun type1() = GeoType.CITY

src/test/kotlin/com/expedia/graphql/generator/extensions/KClassExtensionsTest.kt

@@ -114,6 +114,14 @@ internal class KClassExtensionsTest {
         assertFalse(MyTestClass::class.isArray())
     }
 
+    @Test
+    fun `test listType extension`() {
+        assertTrue(arrayOf(1)::class.isListType())
+        assertTrue(intArrayOf(1)::class.isListType())
+        assertTrue(listOf(1)::class.isListType())
+        assertFalse(MyTestClass::class.isListType())
+    }
+
     @Test
     fun `test graphql interface extension`() {
         assertTrue(TestInterface::class.isInterface())