[generator] Fix issue generating nested additional types (#615)

* Fix issue generating nested additional types

We would encounter a bug if generating the additional types producded more additional types. So we now call the function recursively and clear it out before generating more.

* Apply suggestions from code review

Co-Authored-By: Dariusz Kuc <[email protected]>

* Reverse addition order

Co-authored-by: Dariusz Kuc <[email protected]>

Author: smyrick

graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/SchemaGenerator.kt

@@ -60,7 +60,7 @@ open class SchemaGenerator(internal val config: SchemaGeneratorConfig) {
         builder.query(generateQueries(this, queries))
         builder.mutation(generateMutations(this, mutations))
         builder.subscription(generateSubscriptions(this, subscriptions))
-        builder.additionalTypes(generateAdditionalTypes(additionalTypes))
+        builder.additionalTypes(generateAdditionalTypes())
         builder.additionalDirectives(directives.values.toSet())
         builder.codeRegistry(codeRegistry.build())
 
@@ -86,6 +86,18 @@ open class SchemaGenerator(internal val config: SchemaGeneratorConfig) {
      * Generate the GraphQL type for all the `additionalTypes`. They are generated as non-inputs and not as IDs.
      * If you need to provide more custom additional types that were not picked up from reflection of the schema objects,
      * you can modify the set of `additionalTypes` before you call this method.
+     *
+     * This function is recursive because while generating the additionalTypes it is possible to create additional types that need to be processed.
      */
-    protected open fun generateAdditionalTypes(additionalTypes: Set<KType>): Set<GraphQLType> = additionalTypes.map { generateGraphQLType(this, it) }.toSet()
+    protected open fun generateAdditionalTypes(): Set<GraphQLType> {
+        val currentlyProcessedTypes = this.additionalTypes.toSet()
+        this.additionalTypes.clear()
+        val graphqlTypes = currentlyProcessedTypes.map { generateGraphQLType(this, it) }.toSet()
+
+        return if (this.additionalTypes.isNotEmpty()) {
+            graphqlTypes.plus(generateAdditionalTypes())
+        } else {
+            graphqlTypes
+        }
+    }
 }

graphql-kotlin-schema-generator/src/test/kotlin/com/expediagroup/graphql/generator/SchemaGeneratorTest.kt

@@ -20,8 +20,6 @@ import com.expediagroup.graphql.SchemaGeneratorConfig
 import com.expediagroup.graphql.extensions.deepName
 import org.junit.jupiter.api.Test
 import kotlin.reflect.KClass
-import kotlin.reflect.KType
-import kotlin.reflect.full.createType
 import kotlin.test.assertEquals
 
 class SchemaGeneratorTest {
@@ -45,9 +43,9 @@ class SchemaGeneratorTest {
     fun generateAdditionalTypes() {
         val config = SchemaGeneratorConfig(listOf("com.expediagroup.graphql.generator"))
         val generator = CustomSchemaGenerator(config)
-        val types = setOf(SomeObjectWithAnnotaiton::class.createType())
+        generator.addTypes(MyCustomAnnotation::class)
 
-        val result = generator.generateCustomAdditionalTypes(types)
+        val result = generator.generateCustomAdditionalTypes()
 
         assertEquals(1, result.size)
         assertEquals("SomeObjectWithAnnotaiton!", result.first().deepName)
@@ -56,7 +54,7 @@ class SchemaGeneratorTest {
     class CustomSchemaGenerator(config: SchemaGeneratorConfig) : SchemaGenerator(config) {
         internal fun addTypes(annotation: KClass<*>) = addAdditionalTypesWithAnnotation(annotation)
 
-        internal fun generateCustomAdditionalTypes(types: Set<KType>) = generateAdditionalTypes(types)
+        internal fun generateCustomAdditionalTypes() = generateAdditionalTypes()
     }
 
     annotation class MyCustomAnnotation

graphql-kotlin-schema-generator/src/test/kotlin/com/expediagroup/graphql/test/integration/ConcurrentAdditionalTypesTest.kt

@@ -0,0 +1,62 @@
+/*
+ * Copyright 2020 Expedia, Inc
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.expediagroup.graphql.test.integration
+
+import com.expediagroup.graphql.TopLevelObject
+import com.expediagroup.graphql.testSchemaConfig
+import com.expediagroup.graphql.toSchema
+import org.junit.jupiter.api.Test
+import kotlin.test.assertNotNull
+
+class ConcurrentAdditionalTypesTest {
+
+    @Test
+    fun `verify a concurrent exception is not thrown if there are additionalTypes added when generating the additionalTypes`() {
+        val queries = listOf(TopLevelObject(SimpleQuery()))
+        val schema = toSchema(testSchemaConfig, queries)
+        assertNotNull(schema)
+        assertNotNull(schema.getType("InterfaceOne"))
+        assertNotNull(schema.getType("InterfaceTwo"))
+        assertNotNull(schema.getType("ClassOneA"))
+        assertNotNull(schema.getType("ClassOneB"))
+        assertNotNull(schema.getType("ClassTwoA"))
+        assertNotNull(schema.getType("ClassTwoB"))
+    }
+
+    class SimpleQuery {
+        fun getClassOne(): InterfaceOne = ClassOneB("foo")
+    }
+
+    interface InterfaceOne {
+        val value: String
+    }
+
+    interface InterfaceTwo {
+        val value: String
+    }
+
+    class ClassOneA(
+        override val value: String,
+        val interfaceField: InterfaceTwo
+    ) : InterfaceOne
+
+    class ClassOneB(override val value: String) : InterfaceOne
+
+    class ClassTwoA(override val value: String) : InterfaceTwo
+
+    class ClassTwoB(override val value: String) : InterfaceTwo
+}