feat: do not cache list types (#202)

* feat: do not cache list types

To simplify logic and make the cache simplier, we will not cache any list type. This means that the TypeCache is truely just caching fully qualified types. Lists will still be generated but they will not hit the cache to get their graphql type until they request a class.

This will also help if we want to support adding Sets to the types as it not just becomes one location we have to update for all the valid list types

* fix: delete unused extension method

Author: smyrick

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

@@ -60,6 +60,6 @@ internal fun KClass<*>.getSimpleName(isInputClass: Boolean = false): String {
     }
 }
 
-internal fun KClass<*>.getQualifiedName(): String = this.qualifiedName ?: ""
+internal fun KClass<*>.getQualifiedName(): String = this.qualifiedName.orEmpty()
 
 internal fun KClass<*>.isPublic(): Boolean = this.visibility == KVisibility.PUBLIC

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

@@ -29,16 +29,6 @@ internal fun KType.getWrappedType(): KType {
     }
 }
 
-internal fun KType.getWrappedName(isInputType: Boolean = false): String {
-    val isPrimitiveArray = primitiveArrayTypes.containsKey(this.getKClass())
-    return when {
-        isPrimitiveArray -> this.getSimpleName()
-        this.getKClass().isList() -> "List<${this.getWrappedType().getSimpleName(isInputType)}>"
-        this.getKClass().isArray() -> "Array<${this.getWrappedType().getSimpleName(isInputType)}>"
-        else -> this.getSimpleName(isInputType)
-    }
-}
-
 internal fun KType.getSimpleName(isInputType: Boolean = false): String = this.getKClass().getSimpleName(isInputType)
 
 internal val KType.qualifiedName: String

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

@@ -4,7 +4,6 @@ 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.getWrappedName
 import com.expedia.graphql.generator.extensions.isListType
 import com.expedia.graphql.generator.extensions.qualifiedName
 import graphql.schema.GraphQLType
@@ -20,7 +19,7 @@ internal class TypesCache(private val supportedPackages: List<String>) {
 
     @Throws(ConflictingTypesException::class)
     fun get(cacheKey: TypesCacheKey): GraphQLType? {
-        val cacheKeyString = getCacheKeyString(cacheKey)
+        val cacheKeyString = getCacheKeyString(cacheKey) ?: return null
         val cachedType = cache[cacheKeyString]
 
         if (cachedType != null) {
@@ -36,39 +35,38 @@ internal class TypesCache(private val supportedPackages: List<String>) {
     }
 
     fun put(key: TypesCacheKey, kGraphQLType: KGraphQLType): KGraphQLType? {
-        typeUnderConstruction.remove(kGraphQLType.kClass)
-        return cache.put(getCacheKeyString(key), kGraphQLType)
+        val cacheKey = getCacheKeyString(key)
+
+        if (cacheKey != null) {
+            typeUnderConstruction.remove(kGraphQLType.kClass)
+            return cache.put(cacheKey, kGraphQLType)
+        }
+
+        return null
     }
 
     fun doesNotContainGraphQLType(graphQLType: GraphQLType) =
         cache.none { (_, v) -> v.graphQLType.name == graphQLType.name }
 
     fun doesNotContain(kClass: KClass<*>): Boolean = cache.none { (_, ktype) -> ktype.kClass == kClass }
 
-    private fun getCacheKeyString(cacheKey: TypesCacheKey): String {
-        val kClass = cacheKey.type.getKClass()
-
-        if (kClass.isSubclassOf(Enum::class)) {
-            return kClass.getSimpleName()
-        }
-
-        if (isTypeNotSupported(cacheKey.type)) {
-            throw TypeNotSupportedException(cacheKey.type, supportedPackages)
+    /**
+     * We do not want to cache list types since it is just a simple wrapper.
+     * Enums do not have a different name for input and output.
+     */
+    private fun getCacheKeyString(cacheKey: TypesCacheKey): String? {
+        val type = cacheKey.type
+        val kClass = type.getKClass()
+
+        return when {
+            kClass.isListType() -> null
+            kClass.isSubclassOf(Enum::class) -> kClass.getSimpleName()
+            isTypeNotSupported(type) -> throw TypeNotSupportedException(type, supportedPackages)
+            else -> type.getSimpleName(cacheKey.inputType)
         }
-
-        return cacheKey.type.getWrappedName(cacheKey.inputType)
     }
 
-    private fun isTypeNotSupported(type: KType): Boolean {
-
-        if (type.getKClass().isListType()) {
-            return false
-        }
-
-        return supportedPackages.none {
-            type.qualifiedName.startsWith(it)
-        }
-    }
+    private fun isTypeNotSupported(type: KType): Boolean = supportedPackages.none { type.qualifiedName.startsWith(it) }
 
     private fun putTypeUnderConstruction(kClass: KClass<*>) = typeUnderConstruction.add(kClass)
 

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

@@ -86,25 +86,4 @@ internal class KTypeExtensionsKtTest {
         assertEquals("com.expedia.graphql.generator.extensions.KTypeExtensionsKtTest.MyClass", MyClass::class.starProjectedType.qualifiedName)
         assertEquals("", object { }::class.starProjectedType.qualifiedName)
     }
-
-    @Test
-    fun getName() {
-        assertEquals("MyClass", MyClass::class.starProjectedType.getWrappedName())
-
-        assertEquals("MyClassInput", MyClass::class.starProjectedType.getWrappedName(isInputType = true))
-
-        assertEquals("List<String>", MyClass::listFun.findParameterByName("list")?.type?.getWrappedName())
-
-        assertEquals("List<StringInput>", MyClass::listFun.findParameterByName("list")?.type?.getWrappedName(isInputType = true))
-
-        assertEquals("Array<String>", MyClass::arrayFun.findParameterByName("array")?.type?.getWrappedName())
-
-        assertEquals("IntArray", MyClass::primitiveArrayFun.findParameterByName("intArray")?.type?.getWrappedName())
-
-        assertEquals("IntArray", MyClass::primitiveArrayFun.findParameterByName("intArray")?.type?.getWrappedName(isInputType = true))
-
-        assertFailsWith(CouldNotGetNameOfKClassException::class) {
-            object {}::class.starProjectedType.getWrappedName()
-        }
-    }
 }

src/test/kotlin/com/expedia/graphql/generator/state/TypesCacheTest.kt

@@ -5,6 +5,7 @@ import graphql.schema.GraphQLTypeVisitor
 import graphql.util.TraversalControl
 import graphql.util.TraverserContext
 import org.junit.jupiter.api.Test
+import kotlin.reflect.full.findParameterByName
 import kotlin.reflect.full.starProjectedType
 import kotlin.test.assertEquals
 import kotlin.test.assertFalse
@@ -28,6 +29,16 @@ class TypesCacheTest {
         override fun accept(context: TraverserContext<GraphQLType>, visitor: GraphQLTypeVisitor): TraversalControl = context.thisNode().accept(context, visitor)
     }
 
+    internal class MyClass {
+        fun listFun(list: List<String>) = list.joinToString(separator = ",") { it }
+
+        fun objectListFun(list: List<MyType>) = list.map { it.id.toString() }.joinToString(separator = ",") { it }
+
+        fun arrayFun(array: Array<String>) = array.joinToString(separator = ",") { it }
+
+        fun primitiveArrayFun(intArray: IntArray) = intArray.joinToString(separator = ",") { it.toString() }
+    }
+
     @Test
     fun `basic get and put with non input type`() {
         val cache = TypesCache(listOf("com.expedia.graphql"))
@@ -56,6 +67,66 @@ class TypesCacheTest {
         assertNotNull(cache.get(cacheKey))
     }
 
+    @Test
+    fun `list types are not cached`() {
+        val cache = TypesCache(listOf("com.expedia.graphql"))
+
+        val type = MyClass::listFun.findParameterByName("list")?.type
+
+        assertNotNull(type)
+
+        val cacheKey = TypesCacheKey(type, false)
+        val cacheValue = KGraphQLType(MyType::class, graphQLType)
+
+        assertNull(cache.get(cacheKey))
+        assertNull(cache.put(cacheKey, cacheValue))
+    }
+
+    @Test
+    fun `list of objects are not cached`() {
+        val cache = TypesCache(listOf("com.expedia.graphql"))
+
+        val type = MyClass::objectListFun.findParameterByName("list")?.type
+
+        assertNotNull(type)
+
+        val cacheKey = TypesCacheKey(type, false)
+        val cacheValue = KGraphQLType(MyType::class, graphQLType)
+
+        assertNull(cache.get(cacheKey))
+        assertNull(cache.put(cacheKey, cacheValue))
+    }
+
+    @Test
+    fun `primitive array types are not cached`() {
+        val cache = TypesCache(listOf("com.expedia.graphql"))
+
+        val type = MyClass::primitiveArrayFun.findParameterByName("intArray")?.type
+
+        assertNotNull(type)
+
+        val cacheKey = TypesCacheKey(type, false)
+        val cacheValue = KGraphQLType(MyType::class, graphQLType)
+
+        assertNull(cache.get(cacheKey))
+        assertNull(cache.put(cacheKey, cacheValue))
+    }
+
+    @Test
+    fun `array types are not cached`() {
+        val cache = TypesCache(listOf("com.expedia.graphql"))
+
+        val type = MyClass::arrayFun.findParameterByName("array")?.type
+
+        assertNotNull(type)
+
+        val cacheKey = TypesCacheKey(type, false)
+        val cacheValue = KGraphQLType(MyType::class, graphQLType)
+
+        assertNull(cache.get(cacheKey))
+        assertNull(cache.put(cacheKey, cacheValue))
+    }
+
     @Test
     fun `verify doesNotContainGraphQLType()`() {
         val cache = TypesCache(listOf("com.expedia.graphql"))