Skip to content

Commit 5cad531

Browse files
authored
fix(federation): relaxes @requires and @external constraints (#1778)
Relaxing `@requires` and `@external` constraints. Previously ALL fields referenced from the `@requires` selection set had to be marked `@external`. This was too strict as it is possible to define `@requires` selection set against **local** object type with some `@external` fields. As a result only leaf fields (i.e. scalar/enum fields) have to be explicitly marked `@external` OR implicitly inherit `@external` characteristic through any of the leaf field ancestors.
1 parent 98e900e commit 5cad531

File tree

8 files changed

+248
-23
lines changed

8 files changed

+248
-23
lines changed

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import graphql.schema.GraphQLType
2626
import graphql.schema.GraphQLTypeUtil
2727
import graphql.schema.GraphQLUnionType
2828

29-
internal fun validateFieldSelection(validatedDirective: DirectiveInfo, selection: FieldSetSelection, targetType: GraphQLType, errors: MutableList<String>) {
29+
internal fun validateFieldSelection(validatedDirective: DirectiveInfo, selection: FieldSetSelection, targetType: GraphQLType, errors: MutableList<String>, isExternalPath: Boolean = false) {
3030
when (val unwrapped = GraphQLTypeUtil.unwrapNonNull(targetType)) {
3131
is GraphQLScalarType, is GraphQLEnumType -> {
3232
if (selection.subSelections.isNotEmpty()) {
@@ -38,7 +38,7 @@ internal fun validateFieldSelection(validatedDirective: DirectiveInfo, selection
3838
if (KEY_DIRECTIVE_NAME == validatedDirective.directiveName) {
3939
errors.add("$validatedDirective specifies invalid field set - field set references GraphQLList, field=${selection.field}")
4040
} else {
41-
validateFieldSelection(validatedDirective, selection, GraphQLTypeUtil.unwrapOne(targetType), errors)
41+
validateFieldSelection(validatedDirective, selection, GraphQLTypeUtil.unwrapOne(targetType), errors, isExternalPath)
4242
}
4343
}
4444
is GraphQLInterfaceType -> {
@@ -51,7 +51,8 @@ internal fun validateFieldSelection(validatedDirective: DirectiveInfo, selection
5151
validatedDirective,
5252
selection.subSelections,
5353
unwrapped.fieldDefinitions.associateBy { it.name },
54-
errors
54+
errors,
55+
isExternalPath
5556
)
5657
}
5758
}
@@ -63,7 +64,8 @@ internal fun validateFieldSelection(validatedDirective: DirectiveInfo, selection
6364
validatedDirective,
6465
selection.subSelections,
6566
unwrapped.fieldDefinitions.associateBy { it.name },
66-
errors
67+
errors,
68+
isExternalPath
6769
)
6870
}
6971
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2022 Expedia, Inc
2+
* Copyright 2023 Expedia, Inc
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -26,7 +26,7 @@ internal fun validateDirective(
2626
validatedType: String,
2727
targetDirective: String,
2828
directiveMap: Map<String, List<GraphQLAppliedDirective>>,
29-
fieldMap: Map<String, GraphQLFieldDefinition>,
29+
fieldMap: Map<String, GraphQLFieldDefinition>
3030
): List<String> {
3131
val validationErrors = mutableListOf<String>()
3232
val directives = directiveMap[targetDirective]

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

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2022 Expedia, Inc
2+
* Copyright 2023 Expedia, Inc
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -18,24 +18,32 @@ package com.expediagroup.graphql.generator.federation.validation
1818

1919
import com.expediagroup.graphql.generator.federation.directives.EXTERNAL_DIRECTIVE_NAME
2020
import com.expediagroup.graphql.generator.federation.directives.REQUIRES_DIRECTIVE_NAME
21+
import graphql.schema.GraphQLDirectiveContainer
2122
import graphql.schema.GraphQLFieldDefinition
23+
import graphql.schema.GraphQLNamedType
24+
import graphql.schema.GraphQLTypeUtil
2225

2326
internal fun validateFieldSetSelection(
2427
validatedDirective: DirectiveInfo,
2528
selections: List<FieldSetSelection>,
2629
fields: Map<String, GraphQLFieldDefinition>,
27-
errors: MutableList<String>
30+
errors: MutableList<String>,
31+
isExternalPath: Boolean = false
2832
) {
2933
for (selection in selections) {
3034
val currentField = fields[selection.field]
3135
if (currentField == null) {
3236
errors.add("$validatedDirective specifies invalid field set - field set specifies field that does not exist, field=${selection.field}")
3337
} else {
3438
val currentFieldType = currentField.type
35-
if (REQUIRES_DIRECTIVE_NAME == validatedDirective.directiveName && currentField.getAppliedDirective(EXTERNAL_DIRECTIVE_NAME) == null) {
39+
val isExternal = isExternalPath || GraphQLTypeUtil.unwrapAll(currentFieldType).isExternalPath() || currentField.isExternalType()
40+
if (REQUIRES_DIRECTIVE_NAME == validatedDirective.directiveName && GraphQLTypeUtil.isLeaf(currentFieldType) && !isExternal) {
3641
errors.add("$validatedDirective specifies invalid field set - @requires should reference only @external fields, field=${selection.field}")
3742
}
38-
validateFieldSelection(validatedDirective, selection, currentFieldType, errors)
43+
validateFieldSelection(validatedDirective, selection, currentFieldType, errors, isExternal)
3944
}
4045
}
4146
}
47+
48+
private fun GraphQLDirectiveContainer.isExternalType(): Boolean = this.getAppliedDirectives(EXTERNAL_DIRECTIVE_NAME).isNotEmpty()
49+
private fun GraphQLNamedType.isExternalPath(): Boolean = this is GraphQLDirectiveContainer && this.isExternalType()
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
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+
17+
package com.expediagroup.graphql.generator.federation.data.integration.requires.success._5
18+
19+
import com.expediagroup.graphql.generator.federation.directives.ExternalDirective
20+
import com.expediagroup.graphql.generator.federation.directives.FieldSet
21+
import com.expediagroup.graphql.generator.federation.directives.KeyDirective
22+
import com.expediagroup.graphql.generator.federation.directives.RequiresDirective
23+
import kotlin.properties.Delegates
24+
25+
/*
26+
# only leaf fields have to be external when @requires references complex types
27+
type LeafRequires @key(fields : "id") {
28+
complexType: ComplexType!
29+
description: String!
30+
id: String!
31+
shippingCost: String! @requires(fields : "complexType { weight }")
32+
}
33+
34+
type ComplexType {
35+
localField: String
36+
weight: Float! @external
37+
}
38+
*/
39+
@KeyDirective(fields = FieldSet("id"))
40+
class LeafRequires(val id: String, val description: String, val complexType: ComplexType) {
41+
42+
@RequiresDirective(FieldSet("complexType { weight }"))
43+
fun shippingCost(): String = "$${complexType.weight * 9.99}"
44+
}
45+
46+
class ComplexType(val localField: String) {
47+
@ExternalDirective
48+
var weight: Double by Delegates.notNull()
49+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
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+
17+
package com.expediagroup.graphql.generator.federation.data.integration.requires.success._6
18+
19+
import com.expediagroup.graphql.generator.federation.directives.ExternalDirective
20+
import com.expediagroup.graphql.generator.federation.directives.FieldSet
21+
import com.expediagroup.graphql.generator.federation.directives.KeyDirective
22+
import com.expediagroup.graphql.generator.federation.directives.RequiresDirective
23+
import kotlin.properties.Delegates
24+
25+
/*
26+
# @external information is applied recursively through parent fields
27+
type RecursiveExternalRequires @key(fields : "id") {
28+
complexType: ComplexType! @external
29+
description: String!
30+
id: String!
31+
shippingCost: String! @requires(fields : "complexType { weight }")
32+
}
33+
34+
type ComplexType {
35+
potentiallyExternal: String
36+
weight: Float!
37+
}
38+
*/
39+
@KeyDirective(fields = FieldSet("id"))
40+
class RecursiveExternalRequires(val id: String, val description: String, @ExternalDirective val complexType: ComplexType) {
41+
42+
@RequiresDirective(FieldSet("complexType { weight }"))
43+
fun shippingCost(): String = "$${complexType.weight * 9.99}"
44+
}
45+
46+
class ComplexType(val potentiallyExternal: String) {
47+
var weight: Double by Delegates.notNull()
48+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
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+
17+
package com.expediagroup.graphql.generator.federation.data.integration.requires.success._7
18+
19+
import com.expediagroup.graphql.generator.federation.directives.ExternalDirective
20+
import com.expediagroup.graphql.generator.federation.directives.FieldSet
21+
import com.expediagroup.graphql.generator.federation.directives.KeyDirective
22+
import com.expediagroup.graphql.generator.federation.directives.RequiresDirective
23+
import kotlin.properties.Delegates
24+
25+
/*
26+
# @external information is applied to fields within type
27+
type RecursiveExternalRequires @key(fields : "id") {
28+
externalType: ExternalType!
29+
description: String!
30+
id: String!
31+
shippingCost: String! @requires(fields : "complexType { weight }")
32+
}
33+
34+
type ExternalType @external {
35+
allExternal: String
36+
weight: Float!
37+
}
38+
*/
39+
@KeyDirective(fields = FieldSet("id"))
40+
class ExternalRequiresType(val id: String, val description: String, val externalType: ExternalType) {
41+
42+
@RequiresDirective(FieldSet("externalType { weight }"))
43+
fun shippingCost(): String = "$${externalType.weight * 9.99}"
44+
}
45+
46+
@ExternalDirective
47+
class ExternalType(val allExternal: String) {
48+
var weight: Double by Delegates.notNull()
49+
}

0 commit comments

Comments
 (0)