Add new hook to override the isValidAdditionalType check (#826)

* Add new hook to override the isValidAdditionalType check

* Add filter method locations where we add other additional types

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

Author: smyrick

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

@@ -19,7 +19,6 @@ 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
@@ -98,12 +97,12 @@ open class SchemaGenerator(internal val config: SchemaGeneratorConfig) : Closeab
     /**
      * Add all types with the following annotation to the schema.
      *
-     * This is helpful for things like federation or combining external schemas
+     * This is helpful for things like federation or combining external schemas.
      */
     protected fun addAdditionalTypesWithAnnotation(annotation: KClass<*>, inputType: Boolean = false) {
-        classScanner.getClassesWithAnnotation(annotation).forEach {
-            if (it.isValidAdditionalType(inputType)) {
-                additionalTypes.add(AdditionalType(it.createType(), inputType))
+        classScanner.getClassesWithAnnotation(annotation).forEach { kClass ->
+            if (config.hooks.isValidAdditionalType(kClass, inputType)) {
+                additionalTypes.add(AdditionalType(kClass.createType(), inputType))
             }
         }
     }

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

@@ -67,8 +67,10 @@ internal fun KClass<*>.isUnion(): Boolean =
  * 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
+ *
+ * Also do not add any classes that are marked as @GraphQLIgnore
  */
-internal fun KClass<*>.isValidAdditionalType(inputType: Boolean): Boolean = !(inputType && this.isInterface())
+internal fun KClass<*>.isValidAdditionalType(inputType: Boolean): Boolean = !(inputType && this.isInterface()) && !this.isGraphQLIgnored()
 
 internal fun KClass<*>.isEnum(): Boolean = this.isSubclassOf(Enum::class)
 

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

@@ -16,6 +16,7 @@
 
 package com.expediagroup.graphql.generator.types
 
+import com.expediagroup.graphql.extensions.unwrapType
 import com.expediagroup.graphql.generator.SchemaGenerator
 import com.expediagroup.graphql.generator.extensions.getPropertyDescription
 import com.expediagroup.graphql.generator.extensions.getPropertyName
@@ -27,7 +28,11 @@ import kotlin.reflect.KProperty
 
 internal fun generateInputProperty(generator: SchemaGenerator, prop: KProperty<*>, parentClass: KClass<*>): GraphQLInputObjectField {
     val builder = GraphQLInputObjectField.newInputObjectField()
-    val graphQLInputType = generateGraphQLType(generator = generator, type = prop.returnType, inputType = true).safeCast<GraphQLInputType>()
+
+    // Verfiy that the unwrapped GraphQL type is a valid input type
+    val graphQLInputType = generateGraphQLType(generator = generator, type = prop.returnType, inputType = true)
+        .unwrapType()
+        .safeCast<GraphQLInputType>()
 
     builder.description(prop.getPropertyDescription(parentClass))
     builder.name(prop.getPropertyName(parentClass))

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

@@ -16,16 +16,15 @@
 
 package com.expediagroup.graphql.generator.types
 
-import com.expediagroup.graphql.generator.state.AdditionalType
 import com.expediagroup.graphql.extensions.unwrapType
 import com.expediagroup.graphql.generator.SchemaGenerator
 import com.expediagroup.graphql.generator.extensions.getGraphQLDescription
 import com.expediagroup.graphql.generator.extensions.getSimpleName
 import com.expediagroup.graphql.generator.extensions.getValidFunctions
 import com.expediagroup.graphql.generator.extensions.getValidProperties
 import com.expediagroup.graphql.generator.extensions.getValidSuperclasses
-import com.expediagroup.graphql.generator.extensions.isGraphQLIgnored
 import com.expediagroup.graphql.generator.extensions.safeCast
+import com.expediagroup.graphql.generator.state.AdditionalType
 import graphql.TypeResolutionEnvironment
 import graphql.schema.GraphQLInterfaceType
 import graphql.schema.GraphQLTypeReference
@@ -58,7 +57,7 @@ internal fun generateInterface(generator: SchemaGenerator, kClass: KClass<*>): G
         .forEach { builder.field(generateFunction(generator, it, kClass.getSimpleName(), null, abstract = true)) }
 
     generator.classScanner.getSubTypesOf(kClass)
-        .filter { it.isGraphQLIgnored().not() }
+        .filter { generator.config.hooks.isValidAdditionalType(it, inputType = false) }
         .forEach { generator.additionalTypes.add(AdditionalType(it.createType(), inputType = false)) }
 
     val interfaceType = builder.build()

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

@@ -24,6 +24,7 @@ import com.expediagroup.graphql.exceptions.EmptyObjectTypeException
 import com.expediagroup.graphql.exceptions.EmptyQueryTypeException
 import com.expediagroup.graphql.exceptions.EmptySubscriptionTypeException
 import com.expediagroup.graphql.generator.extensions.isSubclassOf
+import com.expediagroup.graphql.generator.extensions.isValidAdditionalType
 import graphql.schema.FieldCoordinates
 import graphql.schema.GraphQLCodeRegistry
 import graphql.schema.GraphQLFieldDefinition
@@ -102,6 +103,14 @@ interface SchemaGeneratorHooks {
      */
     fun isValidSubscriptionReturnType(kClass: KClass<*>, function: KFunction<*>): Boolean = function.returnType.isSubclassOf(Publisher::class)
 
+    /**
+     * Allow for custom logic when adding additional types to filter out specific classes
+     * or classes with other annotations or metadata.
+     *
+     * The default logic just filters out interfaces if inputType is true.
+     */
+    fun isValidAdditionalType(kClass: KClass<*>, inputType: Boolean): Boolean = kClass.isValidAdditionalType(inputType)
+
     /**
      * Called after `willGenerateGraphQLType` and before `didGenerateGraphQLType`.
      * Enables you to change the wiring, e.g. apply directives to alter the target type.

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

@@ -35,7 +35,7 @@ import kotlin.test.assertNotNull
 import kotlin.test.assertNull
 import kotlin.test.assertTrue
 
-internal class PolymorphicTests {
+class PolymorphicTests {
 
     @Test
     fun `Schema generator creates union types from marked up interface`() {

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

@@ -19,10 +19,15 @@ package com.expediagroup.graphql.generator
 import com.expediagroup.graphql.SchemaGeneratorConfig
 import com.expediagroup.graphql.exceptions.InvalidPackagesException
 import com.expediagroup.graphql.extensions.deepName
+import com.expediagroup.graphql.hooks.SchemaGeneratorHooks
+import graphql.schema.GraphQLNamedType
+import graphql.schema.GraphQLObjectType
 import org.junit.jupiter.api.Test
 import kotlin.reflect.KClass
+import kotlin.reflect.full.findAnnotation
 import kotlin.test.assertEquals
 import kotlin.test.assertFailsWith
+import kotlin.test.assertNotNull
 
 class SchemaGeneratorTest {
 
@@ -79,15 +84,29 @@ class SchemaGeneratorTest {
 
     @Test
     fun generateBothInputAndOutputTypesWithSameName() {
-        val config = SchemaGeneratorConfig(listOf("com.expediagroup.graphql.generator"))
+        val customHooks = object : SchemaGeneratorHooks {
+            override fun isValidAdditionalType(kClass: KClass<*>, inputType: Boolean): Boolean {
+                val allowedForInput = kClass.findAnnotation<AnnotationOnAllTypes>()?.allowedForInput ?: true
+
+                if (inputType && !allowedForInput) {
+                    return false
+                }
+
+                return super.isValidAdditionalType(kClass, inputType)
+            }
+        }
+
+        val config = SchemaGeneratorConfig(listOf("com.expediagroup.graphql.generator"), hooks = customHooks)
         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)
+        assertEquals(7, result.size)
+        val interfaceImpl = assertNotNull(result.find { (it as? GraphQLNamedType)?.name == "InterfaceImpl" } as? GraphQLObjectType)
+        assertEquals(6, interfaceImpl.fieldDefinitions.size)
     }
 
     @Test
@@ -109,7 +128,7 @@ class SchemaGeneratorTest {
     annotation class MyCustomAnnotation
     annotation class MyOtherCustomAnnotation
     annotation class MyInterfaceAnnotation
-    annotation class AnnotationOnAllTypes
+    annotation class AnnotationOnAllTypes(val allowedForInput: Boolean = true)
 
     @MyCustomAnnotation
     @AnnotationOnAllTypes
@@ -128,6 +147,13 @@ class SchemaGeneratorTest {
         val id: String
     }
 
-    @AnnotationOnAllTypes
-    data class InterfaceImpl(override val id: String) : SomeInterface
+    @AnnotationOnAllTypes(allowedForInput = false)
+    data class InterfaceImpl(
+        override val id: String,
+        val listField: List<String>,
+        val optionalListField: List<String>?,
+        val optionalListOptionalField: List<String?>?,
+        val requiredUnionField: SomeUnion,
+        val optionalUnionField: SomeUnion?
+    ) : SomeInterface
 }

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

@@ -92,6 +92,9 @@ open class KClassExtensionsTest {
     @GraphQLIgnore
     interface IgnoredSecondLevelInterface : SomeInterface
 
+    @GraphQLIgnore
+    class IgnoredClass(val value: String)
+
     internal class ClassWithSecondLevelInterface : IgnoredSecondLevelInterface {
         override val someField: String = "hello"
 
@@ -348,5 +351,11 @@ open class KClassExtensionsTest {
         // Invalid cases
         assertFalse(SomeInterface::class.isValidAdditionalType(true))
         assertFalse(TestUnion::class.isValidAdditionalType(true))
+        assertFalse(IgnoredInterface::class.isValidAdditionalType(true))
+        assertFalse(IgnoredInterface::class.isValidAdditionalType(false))
+        assertFalse(IgnoredSecondLevelInterface::class.isValidAdditionalType(true))
+        assertFalse(IgnoredSecondLevelInterface::class.isValidAdditionalType(false))
+        assertFalse(IgnoredClass::class.isValidAdditionalType(true))
+        assertFalse(IgnoredClass::class.isValidAdditionalType(false))
     }
 }

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

@@ -16,12 +16,14 @@
 
 package com.expediagroup.graphql.hooks
 
+import com.expediagroup.graphql.SchemaGeneratorConfig
 import com.expediagroup.graphql.TopLevelObject
 import com.expediagroup.graphql.annotations.GraphQLIgnore
 import com.expediagroup.graphql.exceptions.EmptyInputObjectTypeException
 import com.expediagroup.graphql.exceptions.EmptyInterfaceTypeException
 import com.expediagroup.graphql.exceptions.EmptyObjectTypeException
 import com.expediagroup.graphql.extensions.deepName
+import com.expediagroup.graphql.generator.SchemaGenerator
 import com.expediagroup.graphql.generator.extensions.getSimpleName
 import com.expediagroup.graphql.getTestSchemaConfigWithHooks
 import com.expediagroup.graphql.test.utils.graphqlUUIDType
@@ -122,6 +124,34 @@ class SchemaGeneratorHooksTest {
         assertTrue(schema.queryType.fieldDefinitions.isEmpty())
     }
 
+    @Test
+    fun `calls hook to filter additionalTypes`() {
+        class MockSchemaGeneratorHooks : SchemaGeneratorHooks {
+            var calledFilterFunction = false
+
+            override fun isValidAdditionalType(kClass: KClass<*>, inputType: Boolean): Boolean {
+                calledFilterFunction = true
+                return true
+            }
+        }
+
+        class CustomGenerator(config: SchemaGeneratorConfig) : SchemaGenerator(config) {
+            fun addTypesWithAnnotation(annotation: KClass<*>) = super.addAdditionalTypesWithAnnotation(annotation, false)
+            fun getAdditionalTypesCount() = additionalTypes.size
+        }
+
+        val hooks = MockSchemaGeneratorHooks()
+        val generator = CustomGenerator(getTestSchemaConfigWithHooks(hooks))
+
+        assertFalse(hooks.calledFilterFunction)
+        assertEquals(0, generator.getAdditionalTypesCount())
+
+        generator.addTypesWithAnnotation(CustomAnnotation::class)
+
+        assertTrue(hooks.calledFilterFunction)
+        assertEquals(1, generator.getAdditionalTypesCount())
+    }
+
     @Test
     fun `calls hook after generating object type`() {
         class MockSchemaGeneratorHooks : SchemaGeneratorHooks {
@@ -348,6 +378,9 @@ class SchemaGeneratorHooksTest {
         fun emptyInterface(): EmptyInterface = EmptyImplementation("123")
     }
 
+    annotation class CustomAnnotation
+
+    @CustomAnnotation
     interface EmptyInterface {
         @GraphQLIgnore
         val id: String