Skip to content

Commit 716809e

Browse files
authored
fix(federation): skip fieldset validation when it includes type reference (#1861)
### 📝 Description We are performing federated directive validations when generating GraphQL schema. This is an extra convenience as it gives us information about invalid schema sooner. Subgraph schemas will then be re-validated during the supergraph composition logic. Currently it is possible to create `@key` that references the type still under construction (`GraphQLReferenceType`). Since we cannot validate fields on the type that doesn't exist yet, we cannot validate it either. Updating logic to log a warning when this happens and skip validation logic. ### 🔗 Related Issues Resolves: #1858
1 parent 8796749 commit 716809e

File tree

4 files changed

+84
-4
lines changed

4 files changed

+84
-4
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ plugins {
77
dependencies {
88
api(projects.graphqlKotlinSchemaGenerator)
99
api(libs.federation)
10+
implementation(libs.slf4j)
1011
testImplementation(libs.reactor.core)
1112
testImplementation(libs.reactor.extensions)
1213
testImplementation(libs.junit.params)

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

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,13 @@ import com.expediagroup.graphql.generator.federation.directives.REQUIRES_DIRECTI
2121
import graphql.schema.GraphQLDirectiveContainer
2222
import graphql.schema.GraphQLFieldDefinition
2323
import graphql.schema.GraphQLNamedType
24+
import graphql.schema.GraphQLType
25+
import graphql.schema.GraphQLTypeReference
2426
import graphql.schema.GraphQLTypeUtil
27+
import org.slf4j.Logger
28+
import org.slf4j.LoggerFactory
29+
30+
private val logger: Logger = LoggerFactory.getLogger("ValidateFieldSetSelection")
2531

2632
internal fun validateFieldSetSelection(
2733
validatedDirective: DirectiveInfo,
@@ -36,14 +42,27 @@ internal fun validateFieldSetSelection(
3642
errors.add("$validatedDirective specifies invalid field set - field set specifies field that does not exist, field=${selection.field}")
3743
} else {
3844
val currentFieldType = currentField.type
39-
val isExternal = isExternalPath || GraphQLTypeUtil.unwrapAll(currentFieldType).isExternalPath() || currentField.isExternalType()
40-
if (REQUIRES_DIRECTIVE_NAME == validatedDirective.directiveName && GraphQLTypeUtil.isLeaf(currentFieldType) && !isExternal) {
41-
errors.add("$validatedDirective specifies invalid field set - @requires should reference only @external fields, field=${selection.field}")
45+
if (currentFieldType.isReferenceType()) {
46+
logger.warn("Unable to validate field set selection as one of the fields is a type reference.")
47+
} else {
48+
val isExternal = isExternalPath || GraphQLTypeUtil.unwrapAll(currentFieldType).isExternalPath() || currentField.isExternalType()
49+
if (REQUIRES_DIRECTIVE_NAME == validatedDirective.directiveName && GraphQLTypeUtil.isLeaf(currentFieldType) && !isExternal) {
50+
errors.add("$validatedDirective specifies invalid field set - @requires should reference only @external fields, field=${selection.field}")
51+
}
52+
validateFieldSelection(validatedDirective, selection, currentFieldType, errors, isExternal)
4253
}
43-
validateFieldSelection(validatedDirective, selection, currentFieldType, errors, isExternal)
4454
}
4555
}
4656
}
4757

4858
private fun GraphQLDirectiveContainer.isExternalType(): Boolean = this.getAppliedDirectives(EXTERNAL_DIRECTIVE_NAME).isNotEmpty()
4959
private fun GraphQLNamedType.isExternalPath(): Boolean = this is GraphQLDirectiveContainer && this.isExternalType()
60+
61+
// workaround to GraphQLType.unwrapAll() which tries to cast GraphQLTypeReference to GraphQLUnmodifiedType
62+
private fun GraphQLType.isReferenceType(): Boolean {
63+
var type = this
64+
while (GraphQLTypeUtil.isWrapped(type)) {
65+
type = GraphQLTypeUtil.unwrapOne(type)
66+
}
67+
return type is GraphQLTypeReference
68+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/*
2+
* Copyright 2023 Expedia, Inc
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.expediagroup.graphql.generator.federation.data.integration.key.success._6
17+
18+
import com.expediagroup.graphql.generator.federation.directives.FieldSet
19+
import com.expediagroup.graphql.generator.federation.directives.KeyDirective
20+
import com.expediagroup.graphql.generator.scalars.ID
21+
import io.mockk.mockk
22+
23+
/*
24+
# example usage of a @key referencing parent
25+
type Child @key(fields: "parent { id }") {
26+
parent: Parent
27+
}
28+
29+
type Parent {
30+
id: ID!
31+
someChild: Child!
32+
}
33+
34+
type Query {
35+
parent: Parent!
36+
}
37+
*/
38+
data class Parent(
39+
val id: ID,
40+
val someChild: Child,
41+
)
42+
43+
@KeyDirective(fields = FieldSet("parent { id }"))
44+
data class Child(
45+
val parent: Parent,
46+
)
47+
48+
class EntityReferencingParent {
49+
fun parent(): Parent = mockk()
50+
}

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import com.expediagroup.graphql.generator.federation.data.integration.key.succes
3030
import com.expediagroup.graphql.generator.federation.data.integration.key.success._2.KeyWithMultipleFieldsQuery
3131
import com.expediagroup.graphql.generator.federation.data.integration.key.success._3.KeyWithNestedFieldsQuery
3232
import com.expediagroup.graphql.generator.federation.data.integration.key.success._4.MultipleKeyQuery
33+
import com.expediagroup.graphql.generator.federation.data.integration.key.success._6.EntityReferencingParent
3334
import com.expediagroup.graphql.generator.federation.exception.InvalidFederatedSchema
3435
import com.expediagroup.graphql.generator.federation.toFederatedSchema
3536
import graphql.schema.GraphQLSchema
@@ -216,4 +217,13 @@ class FederatedKeyDirectiveIT {
216217
" - @key(fields = \"upc\") directive on MultipleKeysOneInvalid specifies invalid field set - field set specifies field that does not exist, field=upc"
217218
assertEquals(expected, exception.message)
218219
}
220+
221+
@Test
222+
fun `verifies validations are skipped when processing GraphQL references`() {
223+
val schema = toFederatedSchema(
224+
config = federatedTestConfig("com.expediagroup.graphql.generator.federation.data.integration.key.success._6"),
225+
queries = listOf(TopLevelObject(EntityReferencingParent()))
226+
)
227+
validateTypeWasCreatedWithKeyDirective(schema, "Child")
228+
}
219229
}

0 commit comments

Comments
 (0)