[generator] Do not add interfaces for input additional types (#821)

* Generate unique input additional types

* Do not add additional type if they are input and interfaces

* Simplify isValidAdditionalType

Co-authored-by: Shane Myrick <[email protected]>

Author: smyrick

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

@@ -19,6 +19,7 @@ package com.expediagroup.graphql.generator
 import com.expediagroup.graphql.SchemaGeneratorConfig
 import com.expediagroup.graphql.TopLevelObject
 import com.expediagroup.graphql.exceptions.InvalidPackagesException
+import com.expediagroup.graphql.generator.extensions.isValidAdditionalType
 import com.expediagroup.graphql.generator.state.AdditionalType
 import com.expediagroup.graphql.generator.state.ClassScanner
 import com.expediagroup.graphql.generator.state.TypesCache
@@ -30,6 +31,7 @@ import graphql.schema.GraphQLCodeRegistry
 import graphql.schema.GraphQLDirective
 import graphql.schema.GraphQLSchema
 import graphql.schema.GraphQLType
+import graphql.schema.GraphQLTypeUtil
 import graphql.schema.visibility.NoIntrospectionGraphqlFieldVisibility
 import java.io.Closeable
 import java.util.concurrent.ConcurrentHashMap
@@ -100,7 +102,9 @@ open class SchemaGenerator(internal val config: SchemaGeneratorConfig) : Closeab
      */
     protected fun addAdditionalTypesWithAnnotation(annotation: KClass<*>, inputType: Boolean = false) {
         classScanner.getClassesWithAnnotation(annotation).forEach {
-            additionalTypes.add(AdditionalType(it.createType(), inputType))
+            if (it.isValidAdditionalType(inputType)) {
+                additionalTypes.add(AdditionalType(it.createType(), inputType))
+            }
         }
     }
 
@@ -117,10 +121,14 @@ open class SchemaGenerator(internal val config: SchemaGeneratorConfig) : Closeab
         while (this.additionalTypes.isNotEmpty()) {
             val currentlyProcessedTypes = LinkedHashSet(this.additionalTypes)
             this.additionalTypes.clear()
-            graphqlTypes.addAll(currentlyProcessedTypes.map { generateGraphQLType(this, it.kType, it.inputType) })
+            graphqlTypes.addAll(
+                currentlyProcessedTypes.map {
+                    GraphQLTypeUtil.unwrapNonNull(generateGraphQLType(this, it.kType, it.inputType))
+                }
+            )
         }
 
-        return graphqlTypes.toSet()
+        return graphqlTypes
     }
 
     /**

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

@@ -57,11 +57,19 @@ internal fun KClass<*>.getValidSuperclasses(hooks: SchemaGeneratorHooks): List<K
 internal fun KClass<*>.findConstructorParameter(name: String): KParameter? =
     this.primaryConstructor?.findParameterByName(name)
 
-internal fun KClass<*>.isInterface(): Boolean = this.java.isInterface || this.isAbstract || this.isSealed
+internal fun KClass<*>.isInterface(): Boolean =
+    this.java.isInterface || this.isAbstract || this.isSealed
 
 internal fun KClass<*>.isUnion(): Boolean =
     this.isInterface() && this.declaredMemberProperties.isEmpty() && this.declaredMemberFunctions.isEmpty()
 
+/**
+ * Do not add interfaces as additional types if it expects all the types
+ * to be input types. The isInteface() check works for both
+ * GraphQL Interfaces and GraphQL Unions
+ */
+internal fun KClass<*>.isValidAdditionalType(inputType: Boolean): Boolean = !(inputType && this.isInterface())
+
 internal fun KClass<*>.isEnum(): Boolean = this.isSubclassOf(Enum::class)
 
 internal fun KClass<*>.isListType(): Boolean = this.isSubclassOf(List::class) || this.java.isArray

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

@@ -40,8 +40,17 @@ class SchemaGeneratorTest {
         generator.addTypes(MyCustomAnnotation::class)
         assertEquals(1, generator.additionalTypes.size)
 
-        generator.addInputTypes(MyCustomAnnotation::class)
-        assertEquals(2, generator.additionalTypes.size)
+        // Verify interfaces and unions are not added when input types
+        generator.addInputTypes(MyInterfaceAnnotation::class)
+        assertEquals(1, generator.additionalTypes.size)
+
+        // Interfaces and unions can be added when not input types
+        generator.addTypes(MyInterfaceAnnotation::class)
+        assertEquals(3, generator.additionalTypes.size)
+
+        // Verify the interface implementations are picked up at generation time
+        val result = generator.generateCustomAdditionalTypes()
+        assertEquals(4, result.size)
     }
 
     @Test
@@ -53,7 +62,7 @@ class SchemaGeneratorTest {
         val result = generator.generateCustomAdditionalTypes()
 
         assertEquals(1, result.size)
-        assertEquals("SomeObjectWithAnnotation!", result.first().deepName)
+        assertEquals("SomeObjectWithAnnotation", result.first().deepName)
     }
 
     @Test
@@ -65,7 +74,20 @@ class SchemaGeneratorTest {
         val result = generator.generateCustomAdditionalTypes()
 
         assertEquals(1, result.size)
-        assertEquals("SomeObjectWithAnnotationInput!", result.first().deepName)
+        assertEquals("SomeObjectWithAnnotationInput", result.first().deepName)
+    }
+
+    @Test
+    fun generateBothInputAndOutputTypesWithSameName() {
+        val config = SchemaGeneratorConfig(listOf("com.expediagroup.graphql.generator"))
+        val generator = CustomSchemaGenerator(config)
+        generator.addTypes(AnnotationOnAllTypes::class)
+        generator.addInputTypes(AnnotationOnAllTypes::class)
+
+        val result = generator.generateCustomAdditionalTypes()
+
+        // Verify there are no duplicates
+        assertEquals(8, result.size)
     }
 
     @Test
@@ -86,7 +108,26 @@ class SchemaGeneratorTest {
 
     annotation class MyCustomAnnotation
     annotation class MyOtherCustomAnnotation
+    annotation class MyInterfaceAnnotation
+    annotation class AnnotationOnAllTypes
 
     @MyCustomAnnotation
+    @AnnotationOnAllTypes
     data class SomeObjectWithAnnotation(val name: String)
+
+    @MyInterfaceAnnotation
+    @AnnotationOnAllTypes
+    interface SomeUnion
+
+    @AnnotationOnAllTypes
+    data class UnionImpl(val id: String) : SomeUnion
+
+    @MyInterfaceAnnotation
+    @AnnotationOnAllTypes
+    interface SomeInterface {
+        val id: String
+    }
+
+    @AnnotationOnAllTypes
+    data class InterfaceImpl(override val id: String) : SomeInterface
 }

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

@@ -60,7 +60,7 @@ open class KClassExtensionsTest {
         val publicProperty: String = "public",
         val filteredProperty: String = "filtered",
         private val privateVal: String = "hidden"
-    ) : TestInterface {
+    ) : TestUnion {
         fun publicFunction() = "public function"
 
         fun filteredFunction() = "filtered function"
@@ -82,7 +82,7 @@ open class KClassExtensionsTest {
 
     class MyPublicClass
 
-    internal class UnionSuperclass : TestInterface
+    internal class UnionSuperclass : TestUnion
 
     @GraphQLIgnore
     internal interface IgnoredInterface {
@@ -129,7 +129,7 @@ open class KClassExtensionsTest {
         val id = 1
     }
 
-    interface TestInterface
+    interface TestUnion
 
     sealed class Pet(val name: String) {
         class Dog(name: String, val goodBoysReceived: Int) : Pet(name)
@@ -242,7 +242,7 @@ open class KClassExtensionsTest {
         assertNotNull(MyTestClass::class.findConstructorParameter("publicProperty"))
         assertNull(MyTestClass::class.findConstructorParameter("foobar"))
         assertNull(EmptyConstructorClass::class.findConstructorParameter("id"))
-        assertNull(TestInterface::class.findConstructorParameter("foobar"))
+        assertNull(TestUnion::class.findConstructorParameter("foobar"))
     }
 
     @Test
@@ -261,15 +261,15 @@ open class KClassExtensionsTest {
 
     @Test
     fun `test graphql interface extension`() {
-        assertTrue(TestInterface::class.isInterface())
+        assertTrue(TestUnion::class.isInterface())
         assertTrue(SomeAbstractClass::class.isInterface())
         assertTrue(Pet::class.isInterface())
         assertFalse(MyTestClass::class.isInterface())
     }
 
     @Test
     fun `test graphql union extension`() {
-        assertTrue(TestInterface::class.isUnion())
+        assertTrue(TestUnion::class.isUnion())
         assertFalse(InvalidPropertyUnionInterface::class.isUnion())
         assertFalse(InvalidFunctionUnionInterface::class.isUnion())
         assertFalse(Pet::class.isUnion())
@@ -334,4 +334,19 @@ open class KClassExtensionsTest {
         assertTrue(MyProtectedClass::class.isNotPublic())
         assertTrue(MyTestClass::class.isNotPublic())
     }
+
+    @Test
+    fun isValidAdditionalType() {
+        // Valid cases
+        assertTrue(MyPublicClass::class.isValidAdditionalType(false))
+        assertTrue(MyPublicClass::class.isValidAdditionalType(true))
+        assertTrue(SomeInterface::class.isValidAdditionalType(false))
+        assertTrue(TestUnion::class.isValidAdditionalType(false))
+        assertTrue(MyTestEnum::class.isValidAdditionalType(false))
+        assertTrue(MyTestEnum::class.isValidAdditionalType(true))
+
+        // Invalid cases
+        assertFalse(SomeInterface::class.isValidAdditionalType(true))
+        assertFalse(TestUnion::class.isValidAdditionalType(true))
+    }
 }