fix: move federation validations to didBuildSchema hook (#1883)

### 📝 Description

We were previously validating usage of `@key`/`@requires`/`@provides`
directives when constructing the schema. While it generally worked fine,
it is possible to craft a schema when generated types still reference
types under construction (see:
#1858) and we would
have to skip those validations. By moving the validation to the last
phase of schema generation, we always have a valid GraphQL schema at
that point with resolved references and we can apply validations against
all entities at once so we can have single error message with all
validations (vs failing the build for each invalid entity).

### 🔗 Related Issues
 #1858

generator/graphql-kotlin-federation/build.gradle.kts

@@ -7,7 +7,6 @@ plugins {
 dependencies {
     api(projects.graphqlKotlinSchemaGenerator)
     api(libs.federation)
-    implementation(libs.slf4j)
     testImplementation(libs.reactor.core)
     testImplementation(libs.reactor.extensions)
     testImplementation(libs.junit.params)

generator/graphql-kotlin-federation/src/main/kotlin/com/expediagroup/graphql/generator/federation/FederatedSchemaGeneratorHooks.kt

@@ -59,6 +59,7 @@ import com.expediagroup.graphql.generator.federation.directives.toAppliedLinkDir
 import com.expediagroup.graphql.generator.federation.directives.toAppliedRequiresScopesDirective
 import com.expediagroup.graphql.generator.federation.exception.DuplicateSpecificationLinkImport
 import com.expediagroup.graphql.generator.federation.exception.IncorrectFederatedDirectiveUsage
+import com.expediagroup.graphql.generator.federation.exception.InvalidFederatedSchema
 import com.expediagroup.graphql.generator.federation.exception.UnknownSpecificationException
 import com.expediagroup.graphql.generator.federation.execution.EntitiesDataFetcher
 import com.expediagroup.graphql.generator.federation.execution.FederatedTypeResolver
@@ -80,6 +81,7 @@ import graphql.schema.FieldCoordinates
 import graphql.schema.GraphQLAppliedDirective
 import graphql.schema.GraphQLCodeRegistry
 import graphql.schema.GraphQLDirective
+import graphql.schema.GraphQLDirectiveContainer
 import graphql.schema.GraphQLNamedType
 import graphql.schema.GraphQLObjectType
 import graphql.schema.GraphQLScalarType
@@ -264,11 +266,6 @@ open class FederatedSchemaGeneratorHooks(
         }
     }
 
-    override fun didGenerateGraphQLType(type: KType, generatedType: GraphQLType): GraphQLType {
-        validator.validateGraphQLType(generatedType)
-        return super.didGenerateGraphQLType(type, generatedType)
-    }
-
     override fun didGenerateDirective(directiveInfo: DirectiveMetaInformation, directive: GraphQLDirective): GraphQLDirective {
         if (optInFederationV2) {
             // namespace generated directive if needed
@@ -412,10 +409,19 @@ open class FederatedSchemaGeneratorHooks(
         } else {
             KEY_DIRECTIVE_NAME
         }
-        return originalSchema.allTypesAsList
-            .asSequence()
-            .filterIsInstance<GraphQLObjectType>()
+        val entities = originalSchema.allTypesAsList
+            .filterIsInstance<GraphQLDirectiveContainer>()
             .filter { type -> type.hasAppliedDirective(keyDirectiveName) }
+
+        val errors = entities
+            .filterIsInstance<GraphQLNamedType>()
+            .map { type -> validator.validateGraphQLType(type) }
+            .flatten()
+        if (errors.isNotEmpty()) {
+            throw InvalidFederatedSchema(errors)
+        }
+
+        return entities.filterIsInstance<GraphQLObjectType>()
             .map { it.name }
             .toSet()
     }

generator/graphql-kotlin-federation/src/main/kotlin/com/expediagroup/graphql/generator/federation/validation/FederatedSchemaValidator.kt

@@ -1,5 +1,5 @@
 /*
- * Copyright 2022 Expedia, Inc
+ * Copyright 2023 Expedia, Inc
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -19,7 +19,6 @@ package com.expediagroup.graphql.generator.federation.validation
 import com.expediagroup.graphql.generator.federation.directives.KEY_DIRECTIVE_NAME
 import com.expediagroup.graphql.generator.federation.directives.PROVIDES_DIRECTIVE_NAME
 import com.expediagroup.graphql.generator.federation.directives.REQUIRES_DIRECTIVE_NAME
-import com.expediagroup.graphql.generator.federation.exception.InvalidFederatedSchema
 import graphql.schema.GraphQLAppliedDirective
 import graphql.schema.GraphQLDirectiveContainer
 import graphql.schema.GraphQLFieldDefinition
@@ -44,16 +43,18 @@ internal class FederatedSchemaValidator {
      * - field sets cannot reference unions
      * - list and interfaces can only be referenced from `@requires` and `@provides`
      */
-    internal fun validateGraphQLType(type: GraphQLType) {
+    internal fun validateGraphQLType(type: GraphQLType): List<String> {
         val unwrappedType = GraphQLTypeUtil.unwrapAll(type)
-        if (unwrappedType is GraphQLObjectType && unwrappedType.isFederatedType()) {
+        return if (unwrappedType is GraphQLObjectType && unwrappedType.isFederatedType()) {
             validate(unwrappedType.name, unwrappedType.fieldDefinitions, unwrappedType.allAppliedDirectivesByName)
         } else if (unwrappedType is GraphQLInterfaceType && unwrappedType.isFederatedType()) {
             validate(unwrappedType.name, unwrappedType.fieldDefinitions, unwrappedType.allAppliedDirectivesByName)
+        } else {
+            emptyList()
         }
     }
 
-    private fun validate(federatedType: String, fields: List<GraphQLFieldDefinition>, directiveMap: Map<String, List<GraphQLAppliedDirective>>) {
+    private fun validate(federatedType: String, fields: List<GraphQLFieldDefinition>, directiveMap: Map<String, List<GraphQLAppliedDirective>>): List<String> {
         val errors = mutableListOf<String>()
         val fieldMap = fields.associateBy { it.name }
 
@@ -82,10 +83,7 @@ internal class FederatedSchemaValidator {
                 }
             }
         }
-
-        if (errors.isNotEmpty()) {
-            throw InvalidFederatedSchema(errors)
-        }
+        return errors
     }
 
     private fun GraphQLDirectiveContainer.isFederatedType() = this.getAppliedDirectives(KEY_DIRECTIVE_NAME).isNotEmpty()

generator/graphql-kotlin-federation/src/main/kotlin/com/expediagroup/graphql/generator/federation/validation/validateFieldSetSelection.kt

@@ -21,13 +21,7 @@ import com.expediagroup.graphql.generator.federation.directives.REQUIRES_DIRECTI
 import graphql.schema.GraphQLDirectiveContainer
 import graphql.schema.GraphQLFieldDefinition
 import graphql.schema.GraphQLNamedType
-import graphql.schema.GraphQLType
-import graphql.schema.GraphQLTypeReference
 import graphql.schema.GraphQLTypeUtil
-import org.slf4j.Logger
-import org.slf4j.LoggerFactory
-
-private val logger: Logger = LoggerFactory.getLogger("ValidateFieldSetSelection")
 
 internal fun validateFieldSetSelection(
     validatedDirective: DirectiveInfo,
@@ -42,27 +36,14 @@ internal fun validateFieldSetSelection(
             errors.add("$validatedDirective specifies invalid field set - field set specifies field that does not exist, field=${selection.field}")
         } else {
             val currentFieldType = currentField.type
-            if (currentFieldType.isReferenceType()) {
-                logger.warn("Unable to validate field set selection as one of the fields is a type reference.")
-            } else {
-                val isExternal = isExternalPath || GraphQLTypeUtil.unwrapAll(currentFieldType).isExternalPath() || currentField.isExternalType()
-                if (REQUIRES_DIRECTIVE_NAME == validatedDirective.directiveName && GraphQLTypeUtil.isLeaf(currentFieldType) && !isExternal) {
-                    errors.add("$validatedDirective specifies invalid field set - @requires should reference only @external fields, field=${selection.field}")
-                }
-                validateFieldSelection(validatedDirective, selection, currentFieldType, errors, isExternal)
+            val isExternal = isExternalPath || GraphQLTypeUtil.unwrapAll(currentFieldType).isExternalPath() || currentField.isExternalType()
+            if (REQUIRES_DIRECTIVE_NAME == validatedDirective.directiveName && GraphQLTypeUtil.isLeaf(currentFieldType) && !isExternal) {
+                errors.add("$validatedDirective specifies invalid field set - @requires should reference only @external fields, field=${selection.field}")
             }
+            validateFieldSelection(validatedDirective, selection, currentFieldType, errors, isExternal)
         }
     }
 }
 
 private fun GraphQLDirectiveContainer.isExternalType(): Boolean = this.getAppliedDirectives(EXTERNAL_DIRECTIVE_NAME).isNotEmpty()
 private fun GraphQLNamedType.isExternalPath(): Boolean = this is GraphQLDirectiveContainer && this.isExternalType()
-
-// workaround to GraphQLType.unwrapAll() which tries to cast GraphQLTypeReference to GraphQLUnmodifiedType
-private fun GraphQLType.isReferenceType(): Boolean {
-    var type = this
-    while (GraphQLTypeUtil.isWrapped(type)) {
-        type = GraphQLTypeUtil.unwrapOne(type)
-    }
-    return type is GraphQLTypeReference
-}

generator/graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/generator/federation/validation/FederatedSchemaValidatorTest.kt

@@ -1,5 +1,5 @@
 /*
- * Copyright 2022 Expedia, Inc
+ * Copyright 2023 Expedia, Inc
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -21,7 +21,6 @@ import com.expediagroup.graphql.generator.federation.directives.KEY_DIRECTIVE_NA
 import com.expediagroup.graphql.generator.federation.directives.PROVIDES_DIRECTIVE_NAME
 import com.expediagroup.graphql.generator.federation.directives.REQUIRES_DIRECTIVE_NAME
 import com.expediagroup.graphql.generator.federation.directives.keyDirectiveDefinition
-import com.expediagroup.graphql.generator.federation.exception.InvalidFederatedSchema
 import com.expediagroup.graphql.generator.federation.externalDirective
 import com.expediagroup.graphql.generator.federation.getKeyDirective
 import com.expediagroup.graphql.generator.federation.types.FIELD_SET_ARGUMENT
@@ -34,7 +33,7 @@ import graphql.schema.GraphQLObjectType
 import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.assertDoesNotThrow
 import kotlin.test.assertEquals
-import kotlin.test.assertFailsWith
+import kotlin.test.assertTrue
 
 class FederatedSchemaValidatorTest {
 
@@ -89,9 +88,10 @@ class FederatedSchemaValidatorTest {
             )
             .build()
 
-        assertFailsWith<InvalidFederatedSchema> {
-            FederatedSchemaValidator().validateGraphQLType(typeToValidate)
-        }
+        val errors = FederatedSchemaValidator().validateGraphQLType(typeToValidate)
+        assertTrue(errors.isNotEmpty())
+        assertEquals(1, errors.size)
+        assertEquals("@key directive on Foo is missing field information", errors[0])
     }
 
     /**
@@ -114,17 +114,10 @@ class FederatedSchemaValidatorTest {
             .withAppliedDirective(keyDirectiveType)
             .build()
 
-        val result = kotlin.runCatching {
-            FederatedSchemaValidator().validateGraphQLType(typeToValidate)
-        }
-
-        val expectedError =
-            """
-                Invalid federated schema:
-                 - @key directive on Foo is missing field information
-            """.trimIndent()
-
-        assertEquals(expectedError, result.exceptionOrNull()?.message)
+        val errors = FederatedSchemaValidator().validateGraphQLType(typeToValidate)
+        assertTrue(errors.isNotEmpty())
+        assertEquals(1, errors.size)
+        assertEquals("@key directive on Foo is missing field information", errors[0])
     }
 
     /**
@@ -155,17 +148,10 @@ class FederatedSchemaValidatorTest {
             .withAppliedDirective(EXTENDS_DIRECTIVE_TYPE.toAppliedDirective())
             .build()
 
-        val result = kotlin.runCatching {
-            FederatedSchemaValidator().validateGraphQLType(typeToValidate)
-        }
-
-        val expectedError =
-            """
-                Invalid federated schema:
-                 - @provides directive is specified on a Foo.bar field but it does not return an object type
-            """.trimIndent()
-
-        assertEquals(expectedError, result.exceptionOrNull()?.message)
+        val errors = FederatedSchemaValidator().validateGraphQLType(typeToValidate)
+        assertTrue(errors.isNotEmpty())
+        assertEquals(1, errors.size)
+        assertEquals("@provides directive is specified on a Foo.bar field but it does not return an object type", errors[0])
     }
 
     /**
@@ -196,16 +182,9 @@ class FederatedSchemaValidatorTest {
             .withAppliedDirective(EXTENDS_DIRECTIVE_TYPE.toAppliedDirective())
             .build()
 
-        val result = kotlin.runCatching {
-            FederatedSchemaValidator().validateGraphQLType(typeToValidate)
-        }
-
-        val expectedError =
-            """
-                Invalid federated schema:
-                 - @requires directive on Foo.bar is missing field information
-            """.trimIndent()
-
-        assertEquals(expectedError, result.exceptionOrNull()?.message)
+        val errors = FederatedSchemaValidator().validateGraphQLType(typeToValidate)
+        assertTrue(errors.isNotEmpty())
+        assertEquals(1, errors.size)
+        assertEquals("@requires directive on Foo.bar is missing field information", errors[0])
     }
 }