Reduce number of class scanners created in tests (#646)

smyrick · Shane Myrick · web-flow · commit acf8866ec7e6 · 2020-03-22T23:37:18.000-05:00
In an attempt to fix the out of memory errors, I simplified some of our unit tests to create few ClassScanners and SchemaGenerators, and close all usages of the ones we still have left

Co-authored-by: Shane Myrick &lt;accounts@shanemyrick.com&gt;
diff --git a/graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/SchemaGenerator.kt b/graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/SchemaGenerator.kt
@@ -42,7 +42,7 @@ import kotlin.reflect.full.createType
  */
 open class SchemaGenerator(internal val config: SchemaGeneratorConfig) : Closeable {
 
-    internal val additionalTypes = mutableSetOf<KType>()
+    internal val additionalTypes: MutableSet<KType> = mutableSetOf()
     internal val classScanner = ClassScanner(config.supportedPackages)
     internal val cache = TypesCache(config.supportedPackages)
     internal val codeRegistry = GraphQLCodeRegistry.newCodeRegistry()
@@ -66,11 +66,8 @@ open class SchemaGenerator(internal val config: SchemaGeneratorConfig) : Closeab
         builder.additionalTypes(generateAdditionalTypes())
         builder.additionalDirectives(directives.values.toSet())
         builder.codeRegistry(codeRegistry.build())
-        val schema = config.hooks.willBuildSchema(builder).build()
 
-        classScanner.close()
-
-        return schema
+        return config.hooks.willBuildSchema(builder).build()
     }
 
     /**
@@ -113,7 +110,7 @@ open class SchemaGenerator(internal val config: SchemaGeneratorConfig) : Closeab
      */
     override fun close() {
         classScanner.close()
-        cache.clear()
+        cache.close()
         additionalTypes.clear()
         directives.clear()
     }
diff --git a/graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/state/TypesCache.kt b/graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/state/TypesCache.kt
@@ -25,12 +25,13 @@ import com.expediagroup.graphql.generator.extensions.qualifiedName
 import graphql.schema.GraphQLNamedType
 import graphql.schema.GraphQLType
 import graphql.schema.GraphQLTypeReference
+import java.io.Closeable
 import kotlin.reflect.KClass
 import kotlin.reflect.KType
 import kotlin.reflect.full.isSubclassOf
 import kotlin.reflect.full.starProjectedType
 
-internal class TypesCache(private val supportedPackages: List<String>) {
+internal class TypesCache(private val supportedPackages: List<String>) : Closeable {
 
     private val cache: MutableMap<String, KGraphQLType> = mutableMapOf()
     private val typesUnderConstruction: MutableSet<TypesCacheKey> = mutableSetOf()
@@ -64,9 +65,12 @@ internal class TypesCache(private val supportedPackages: List<String>) {
     }
 
     /**
-     * Clear the map of all saved values
+     * Clear the cache of all saved values
      */
-    internal fun clear() = cache.clear()
+    override fun close() {
+        cache.clear()
+        typesUnderConstruction.clear()
+    }
 
     internal fun doesNotContainGraphQLType(graphQLType: GraphQLNamedType) =
         cache.none { (_, v) -> v.graphQLType.name == graphQLType.name }
diff --git a/graphql-kotlin-schema-generator/src/test/kotlin/com/expediagroup/graphql/generator/ToSchemaTest.kt b/graphql-kotlin-schema-generator/src/test/kotlin/com/expediagroup/graphql/generator/ToSchemaTest.kt
@@ -72,11 +72,13 @@ class ToSchemaTest {
     @Test
     fun `SchemaGenerator generates a simple GraphQL schema with default builder`() {
         val schemaGenerator = SchemaGenerator(testSchemaConfig)
-        val schema = schemaGenerator.generateSchema(
-            queries = listOf(TopLevelObject(QueryObject())),
-            mutations = listOf(TopLevelObject(MutationObject())),
-            subscriptions = emptyList()
-        )
+        val schema = schemaGenerator.use {
+            it.generateSchema(
+                queries = listOf(TopLevelObject(QueryObject())),
+                mutations = listOf(TopLevelObject(MutationObject())),
+                subscriptions = emptyList()
+            )
+        }
 
         val graphQL = GraphQL.newGraphQL(schema).build()
 
diff --git a/graphql-kotlin-schema-generator/src/test/kotlin/com/expediagroup/graphql/generator/state/ClassScannerTest.kt b/graphql-kotlin-schema-generator/src/test/kotlin/com/expediagroup/graphql/generator/state/ClassScannerTest.kt
@@ -16,6 +16,8 @@
 
 package com.expediagroup.graphql.generator.state
 
+import com.expediagroup.graphql.defaultSupportedPackages
+import org.junit.jupiter.api.AfterAll
 import org.junit.jupiter.api.Test
 import kotlin.test.assertEquals
 
@@ -59,54 +61,53 @@ internal class ClassScannerTest {
     @SimpleAnnotation
     private class MyClassWithAnnotaiton
 
+    private val basicClassScanner = ClassScanner(defaultSupportedPackages)
+
     @Test
     fun `valid subtypes`() {
-        val classScanner = ClassScanner(listOf("com.expediagroup.graphql"))
-        val list = classScanner.getSubTypesOf(MyInterface::class)
-
+        val list = basicClassScanner.getSubTypesOf(MyInterface::class)
         assertEquals(expected = 2, actual = list.size)
     }
 
     @Test
     fun `abstract subtypes`() {
-        val classScanner = ClassScanner(listOf("com.expediagroup.graphql"))
-        val list = classScanner.getSubTypesOf(MyAbstractClass::class)
-
+        val list = basicClassScanner.getSubTypesOf(MyAbstractClass::class)
         assertEquals(expected = 1, actual = list.size)
     }
 
     @Test
     fun `subtypes of non-supported packages`() {
-        val classScanner = ClassScanner(listOf("com.example"))
-        val list = classScanner.getSubTypesOf(MyInterface::class)
-
+        val classScannerOfOtherPackages = ClassScanner(listOf("com.example"))
+        val list = classScannerOfOtherPackages.getSubTypesOf(MyInterface::class)
         assertEquals(expected = 0, actual = list.size)
+        classScannerOfOtherPackages.close()
     }
 
     @Test
     fun `interface with no subtypes`() {
-        val classScanner = ClassScanner(listOf("com.expediagroup.graphql"))
-        val list = classScanner.getSubTypesOf(NoSubTypesInterface::class)
-
+        val list = basicClassScanner.getSubTypesOf(NoSubTypesInterface::class)
         assertEquals(expected = 0, actual = list.size)
     }
 
     @Test
     fun `abstract class with no subtypes`() {
-        val classScanner = ClassScanner(listOf("com.expediagroup.graphql"))
-        val list = classScanner.getSubTypesOf(NoSubTypesClass::class)
-
+        val list = basicClassScanner.getSubTypesOf(NoSubTypesClass::class)
         assertEquals(expected = 0, actual = list.size)
     }
 
     @Test
     fun `classes with annotation returns all values`() {
-        val classScanner = ClassScanner(listOf("com.expediagroup.graphql.generator"))
-
-        val invalidClasses = classScanner.getClassesWithAnnotation(OtherSimpleAnnotation::class)
+        val invalidClasses = basicClassScanner.getClassesWithAnnotation(OtherSimpleAnnotation::class)
         assertEquals(0, invalidClasses.size)
 
-        val validClasses = classScanner.getClassesWithAnnotation(SimpleAnnotation::class)
+        val validClasses = basicClassScanner.getClassesWithAnnotation(SimpleAnnotation::class)
         assertEquals(1, validClasses.size)
     }
+
+    companion object {
+        @AfterAll
+        fun afterAll(classScannerTest: ClassScannerTest) {
+            classScannerTest.basicClassScanner.close()
+        }
+    }
 }
diff --git a/graphql-kotlin-schema-generator/src/test/kotlin/com/expediagroup/graphql/generator/types/GenerateDirectiveTest.kt b/graphql-kotlin-schema-generator/src/test/kotlin/com/expediagroup/graphql/generator/types/GenerateDirectiveTest.kt
@@ -22,7 +22,7 @@ import com.expediagroup.graphql.generator.SchemaGenerator
 import com.expediagroup.graphql.generator.extensions.isTrue
 import com.expediagroup.graphql.getTestSchemaConfigWithMockedDirectives
 import com.expediagroup.graphql.test.utils.SimpleDirective
-import org.junit.jupiter.api.BeforeEach
+import org.junit.jupiter.api.AfterAll
 import org.junit.jupiter.api.Test
 import kotlin.reflect.KClass
 import kotlin.test.assertEquals
@@ -77,12 +77,7 @@ internal class GenerateDirectiveTest {
         val noDirective: String
     )
 
-    private lateinit var basicGenerator: SchemaGenerator
-
-    @BeforeEach
-    fun setUp() {
-        basicGenerator = SchemaGenerator(getTestSchemaConfigWithMockedDirectives())
-    }
+    private val basicGenerator = SchemaGenerator(getTestSchemaConfigWithMockedDirectives())
 
     @Test
     fun `no annotation`() {
@@ -182,4 +177,11 @@ internal class GenerateDirectiveTest {
         val noPrefixResult = generateDirectives(basicGenerator, MyClassWithConstructorArgs::noPrefix, null)
         assertEquals(expected = 0, actual = noPrefixResult.size)
     }
+
+    companion object {
+        @AfterAll
+        fun cleanUp(generateDirectiveTest: GenerateDirectiveTest) {
+            generateDirectiveTest.basicGenerator.close()
+        }
+    }
 }
diff --git a/graphql-kotlin-schema-generator/src/test/kotlin/com/expediagroup/graphql/generator/types/GeneratePropertyTest.kt b/graphql-kotlin-schema-generator/src/test/kotlin/com/expediagroup/graphql/generator/types/GeneratePropertyTest.kt
@@ -207,5 +207,6 @@ internal class GeneratePropertyTest : TypeTestHelper() {
         val targetDataFetcher = localGenerator.codeRegistry.getDataFetcher(coordinates, result)
         assertFalse(targetDataFetcher is PropertyDataFetcher)
         assertEquals(expected = mockDataFetcher, actual = targetDataFetcher)
+        localGenerator.close()
     }
 }
diff --git a/graphql-kotlin-schema-generator/src/test/kotlin/com/expediagroup/graphql/generator/types/TypeTestHelper.kt b/graphql-kotlin-schema-generator/src/test/kotlin/com/expediagroup/graphql/generator/types/TypeTestHelper.kt
@@ -23,7 +23,6 @@ import com.expediagroup.graphql.directives.KotlinSchemaDirectiveWiring
 import com.expediagroup.graphql.execution.KotlinDataFetcherFactoryProvider
 import com.expediagroup.graphql.execution.SimpleKotlinDataFetcherFactoryProvider
 import com.expediagroup.graphql.generator.SchemaGenerator
-import com.expediagroup.graphql.generator.state.ClassScanner
 import com.expediagroup.graphql.hooks.SchemaGeneratorHooks
 import io.mockk.every
 import io.mockk.spyk
@@ -40,7 +39,6 @@ import org.junit.jupiter.api.TestInstance.Lifecycle
 @TestInstance(Lifecycle.PER_CLASS)
 open class TypeTestHelper {
     private val supportedPackages = listOf("com.expediagroup.graphql")
-    private val classScanner = ClassScanner(supportedPackages)
     private val dataFetcherFactory: KotlinDataFetcherFactoryProvider = SimpleKotlinDataFetcherFactoryProvider()
     private val topLevelNames = TopLevelNames(
         query = "TestTopLevelQuery",
@@ -62,7 +60,7 @@ open class TypeTestHelper {
 
         generator.additionalTypes.clear()
         generator.directives.clear()
-        generator.cache.clear()
+        generator.cache.close()
 
         every { config.hooks } returns hooks
         every { config.dataFetcherFactoryProvider } returns dataFetcherFactory
@@ -73,7 +71,7 @@ open class TypeTestHelper {
 
     @AfterAll
     fun cleanup() {
-        classScanner.close()
+        generator.close()
     }
 
     open fun beforeTest() {}
diff --git a/graphql-kotlin-spring-server/src/main/kotlin/com/expediagroup/graphql/spring/exception/SimpleKotlinGraphQLError.kt b/graphql-kotlin-spring-server/src/main/kotlin/com/expediagroup/graphql/spring/exception/SimpleKotlinGraphQLError.kt
@@ -32,13 +32,12 @@ open class SimpleKotlinGraphQLError(
 ) : GraphQLError {
     override fun getErrorType(): ErrorClassification = errorType
 
-    override fun getExtensions(): Map<String, Any> {
-        val newExtensions = mutableMapOf<String, Any>()
+    override fun getExtensions(): Map<String, Any> =
         if (exception is GraphQLError && exception.extensions != null) {
-            newExtensions.putAll(exception.extensions)
+            exception.extensions
+        } else {
+            emptyMap()
         }
-        return newExtensions
-    }
 
     override fun getLocations(): List<SourceLocation> = locations
 
diff --git a/graphql-kotlin-spring-server/src/test/kotlin/com/expediagroup/graphql/spring/instrumentation/InstrumentationIT.kt b/graphql-kotlin-spring-server/src/test/kotlin/com/expediagroup/graphql/spring/instrumentation/InstrumentationIT.kt
@@ -45,13 +45,10 @@ class InstrumentationIT(@Autowired private val testClient: WebTestClient) {
 
     class OrderedInstrumentation(private val instrumentationOrder: Int, private val counter: AtomicInteger) : SimpleInstrumentation(), Ordered {
         override fun instrumentExecutionResult(executionResult: ExecutionResult, parameters: InstrumentationExecutionParameters): CompletableFuture<ExecutionResult> {
-            val extensions = mutableMapOf<Any, Any>()
-            extensions[instrumentationOrder] = counter.getAndIncrement()
-            val currentExt: Map<Any, Any>? = executionResult.extensions
-            if (currentExt != null) {
-                extensions.putAll(currentExt)
-            }
-            return CompletableFuture.completedFuture(ExecutionResultImpl(executionResult.getData(), executionResult.errors, extensions))
+            val currentExt: MutableMap<Any, Any> = executionResult.extensions?.toMutableMap() ?: mutableMapOf()
+            currentExt[instrumentationOrder] = counter.getAndIncrement()
+
+            return CompletableFuture.completedFuture(ExecutionResultImpl(executionResult.getData(), executionResult.errors, currentExt))
         }
 
         override fun getOrder(): Int = instrumentationOrder

Original file line number	Diff line number	Diff line change
`@@ -207,5 +207,6 @@ internal class GeneratePropertyTest : TypeTestHelper() {`
`207`	`207`	`val targetDataFetcher = localGenerator.codeRegistry.getDataFetcher(coordinates, result)`
`208`	`208`	`assertFalse(targetDataFetcher is PropertyDataFetcher)`
`209`	`209`	`assertEquals(expected = mockDataFetcher, actual = targetDataFetcher)`
	`210`	`+ localGenerator.close()`
`210`	`211`	`}`
`211`	`212`	`}`