Skip to content

Commit 481f15f

Browse files
dariuszkucsmyrick
authored andcommitted
[federation] skip validation of nested objects under construction (ExpediaGroup#403)
When generating federated GraphQL schema we attempt to validate the generated objects. This is problematic when we are dealing with nested relationships as at validation time, those objects are still being built so we only have access to their `GraphQLTypeReference`. Those references are replaced as the last step of schema generation lifecycle which means we won't be able to perform any validations on those objects when they are still being built. This PR disables the validation of `@provides` directive when it references objects that are still being built.
1 parent 051f340 commit 481f15f

File tree

2 files changed

+66
-15
lines changed

2 files changed

+66
-15
lines changed

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

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package com.expediagroup.graphql.federation.validation
1919
import com.expediagroup.graphql.federation.extensions.isExtendedType
2020
import graphql.schema.GraphQLFieldDefinition
2121
import graphql.schema.GraphQLObjectType
22+
import graphql.schema.GraphQLTypeReference
2223
import graphql.schema.GraphQLTypeUtil
2324

2425
// [OK] @provides on base type references valid @external fields on @extend object
@@ -28,25 +29,22 @@ import graphql.schema.GraphQLTypeUtil
2829
// [OK] @provides references list of valid @extend objects
2930
// [ERROR] @provides references @external list field
3031
// [ERROR] @provides references @external interface field
31-
internal fun validateProvidesDirective(federatedType: String, field: GraphQLFieldDefinition): List<String> {
32-
val errors = mutableListOf<String>()
33-
val returnType = GraphQLTypeUtil.unwrapAll(field.type)
34-
if (returnType is GraphQLObjectType) {
32+
internal fun validateProvidesDirective(federatedType: String, field: GraphQLFieldDefinition): List<String> = when (val returnType = GraphQLTypeUtil.unwrapType(field.type).last()) {
33+
is GraphQLObjectType -> {
3534
if (!returnType.isExtendedType()) {
36-
errors.add("@provides directive is specified on a $federatedType.${field.name} field references local object")
35+
listOf("@provides directive is specified on a $federatedType.${field.name} field references local object")
3736
} else {
3837
val returnTypeFields = returnType.fieldDefinitions.associateBy { it.name }
3938
// @provides is applicable on both base and federated types and always references @external fields
40-
errors.addAll(
41-
validateDirective(
42-
"$federatedType.${field.name}",
43-
"provides",
44-
field.directivesByName,
45-
returnTypeFields,
46-
true))
39+
validateDirective(
40+
"$federatedType.${field.name}",
41+
"provides",
42+
field.directivesByName,
43+
returnTypeFields,
44+
true)
4745
}
48-
} else {
49-
errors.add("@provides directive is specified on a $federatedType.${field.name} field but it does not return an object type")
5046
}
51-
return errors
47+
// skip validation for nested object types as they are still under construction
48+
is GraphQLTypeReference -> emptyList()
49+
else -> listOf("@provides directive is specified on a $federatedType.${field.name} field but it does not return an object type")
5250
}

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

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
@file:Suppress("MethodOverloading")
12
/*
23
* Copyright 2019 Expedia, Inc
34
*
@@ -27,9 +28,11 @@ import com.expediagroup.graphql.federation.directives.KeyDirective
2728
import com.expediagroup.graphql.federation.directives.ProvidesDirective
2829
import com.expediagroup.graphql.federation.exception.InvalidFederatedSchema
2930
import graphql.schema.GraphQLObjectType
31+
import graphql.schema.GraphQLTypeReference
3032
import graphql.schema.GraphQLTypeUtil
3133
import io.mockk.mockk
3234
import org.junit.jupiter.api.BeforeEach
35+
import org.junit.jupiter.api.Test
3336
import org.junit.jupiter.api.TestInstance
3437
import org.junit.jupiter.params.ParameterizedTest
3538
import org.junit.jupiter.params.provider.Arguments
@@ -99,6 +102,27 @@ class FederatedSchemaValidatorProvidesDirectiveTest {
99102
}
100103
}
101104

105+
@Test
106+
fun `validated nested relationship with @provide directive`() {
107+
// order of object instantiation matters
108+
// - BUG #397 - typeA -> typeB with provides (apply validation) -> reference typeA
109+
// - NO ISSUE - typeB with provides -> typeA (no validation) -> reference typeB
110+
val validatedType = schemaGenerator.objectType(NestedProvidedType::class) as? GraphQLObjectType
111+
assertNotNull(validatedType)
112+
assertEquals(NestedProvidedType::class.simpleName, validatedType.name)
113+
validator.validateGraphQLType(validatedType)
114+
assertNotNull(validatedType.getDirective("key"))
115+
116+
val typeWithProvides = schemaGenerator.objectType(NestedProvides::class) as? GraphQLObjectType
117+
assertNotNull(typeWithProvides)
118+
validator.validateGraphQLType(typeWithProvides)
119+
val providedField = typeWithProvides.getFieldDefinition("provided")
120+
assertNotNull(providedField)
121+
assertNotNull(providedField.getDirective("provides"))
122+
val providedType = GraphQLTypeUtil.unwrapType(providedField.type).last() as? GraphQLTypeReference
123+
assertNotNull(providedType)
124+
}
125+
102126
// ======================= TEST DATA ===========
103127
/*
104128
type SimpleProvides @key(fields : "id") {
@@ -276,4 +300,33 @@ class FederatedSchemaValidatorProvidesDirectiveTest {
276300
@ExternalDirective val id: String,
277301
@ExternalDirective val data: ProvidedInterface = throw UnsupportedOperationException("not implemented")
278302
)
303+
304+
/*
305+
type NestedProvidedType @extends @key(fields : "id") {
306+
id: String! @external
307+
nested: NestedProvides!
308+
text: String! @external
309+
}
310+
311+
type NestedProvides @key(fields : "id") {
312+
description: String!
313+
id: String!
314+
provided: NestedProvidedType! @provides(fields : "text")
315+
}
316+
*/
317+
@KeyDirective(fields = FieldSet("id"))
318+
private class NestedProvides(val id: String, val description: String) {
319+
320+
@ProvidesDirective(fields = FieldSet("text"))
321+
fun provided() = NestedProvidedType(id, "some text")
322+
}
323+
324+
@KeyDirective(fields = FieldSet("id"))
325+
@ExtendsDirective
326+
private class NestedProvidedType(
327+
@ExternalDirective val id: String,
328+
@ExternalDirective val text: String
329+
) {
330+
fun nested(): NestedProvides = NestedProvides("123", "some description")
331+
}
279332
}

0 commit comments

Comments
 (0)