Skip to content

Commit b57916a

Browse files
smyrickShane Myrick
smyrick
and
Shane Myrick
authored
Allow requires field set selection to be on lists (#685)
* Allow field set selection for requires to be on lists Resolves #637 * Update requires directive integration tests Co-authored-by: Shane Myrick <[email protected]>
1 parent 3facc66 commit b57916a

File tree

11 files changed

+396
-85
lines changed

11 files changed

+396
-85
lines changed

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/*
2+
* Copyright 2020 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.federation.validation
18+
19+
/**
20+
* Internal class to represent all the extracted info
21+
* about a directive that we are validating for federation.
22+
*/
23+
internal data class DirectiveInfo(
24+
val directiveName: String,
25+
val fieldSet: String,
26+
val typeName: String
27+
)
28+
29+
/**
30+
* Extension method to get the GraphQL schema format
31+
* of the directive info that we print to errors.
32+
*/
33+
internal fun DirectiveInfo.getErrorString() = "@$directiveName(fields = $fieldSet) directive on $typeName"

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,13 @@ internal fun validateDirective(
3838
if (fieldSet.isEmpty()) {
3939
validationErrors.add("@$targetDirective directive on $validatedType is missing field information")
4040
} else {
41-
// validate key field set
42-
val validatedDirectiveInfo = "@$targetDirective(fields = $fieldSetValue) directive on $validatedType"
43-
validateFieldSelection(validatedDirectiveInfo, fieldSet.iterator(), fieldMap, extendedType, validationErrors)
41+
// validate directive field set selection
42+
val directiveInfo = DirectiveInfo(
43+
directiveName = targetDirective,
44+
fieldSet = fieldSet.joinToString(" "),
45+
typeName = validatedType
46+
)
47+
validateFieldSelection(directiveInfo, fieldSet.iterator(), fieldMap, extendedType, validationErrors)
4448
}
4549
}
4650
return validationErrors

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import graphql.schema.GraphQLObjectType
2222
import graphql.schema.GraphQLTypeUtil
2323

2424
internal fun validateFieldSelection(
25-
validatedDirective: String,
25+
validatedDirective: DirectiveInfo,
2626
iterator: Iterator<String>,
2727
fields: Map<String, GraphQLFieldDefinition>,
2828
extendedType: Boolean,
@@ -34,14 +34,15 @@ internal fun validateFieldSelection(
3434
when (currentField) {
3535
"{" -> {
3636
val targetField = fields[previousField]?.type
37+
val errorMessage = validatedDirective.getErrorString()
3738
when (val unwrappedType = GraphQLTypeUtil.unwrapAll(targetField)) {
3839
is GraphQLInterfaceType -> validateFieldSelection(validatedDirective, iterator, unwrappedType.fieldDefinitions.associateBy { it.name }, extendedType, errors)
3940
is GraphQLObjectType -> validateFieldSelection(validatedDirective, iterator, unwrappedType.fieldDefinitions.associateBy { it.name }, extendedType, errors)
40-
else -> errors.add("$validatedDirective specifies invalid field set - field set defines nested selection set on unsupported type")
41+
else -> errors.add("$errorMessage specifies invalid field set - field set defines nested selection set on unsupported type")
4142
}
4243
}
4344
"}" -> return
44-
else -> validateKeySetField(fields[currentField], extendedType, errors, validatedDirective)
45+
else -> validateFieldSet(fields[currentField], extendedType, errors, validatedDirective)
4546
}
4647
previousField = currentField
4748
}

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

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/*
2+
* Copyright 2019 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.federation.validation
18+
19+
import com.expediagroup.graphql.federation.directives.EXTERNAL_DIRECTIVE_NAME
20+
import graphql.schema.GraphQLFieldDefinition
21+
import graphql.schema.GraphQLInterfaceType
22+
import graphql.schema.GraphQLList
23+
import graphql.schema.GraphQLTypeUtil
24+
import graphql.schema.GraphQLUnionType
25+
26+
internal fun validateFieldSet(targetField: GraphQLFieldDefinition?, extendedType: Boolean, errors: MutableList<String>, validatedDirective: DirectiveInfo) {
27+
val errorMessage = validatedDirective.getErrorString()
28+
if (null != targetField) {
29+
val externalField = targetField.getDirective(EXTERNAL_DIRECTIVE_NAME) != null
30+
if (extendedType && !externalField) {
31+
errors.add("$errorMessage specifies invalid field set - extended type incorrectly references local field=${targetField.name}")
32+
} else if (!extendedType && externalField) {
33+
errors.add("$errorMessage specifies invalid field set - type incorrectly references external field=${targetField.name}")
34+
}
35+
36+
/**
37+
* The @requires directive can have [list,interface,union] in the field selection.
38+
* All the other field set directives can not.
39+
*/
40+
if (validatedDirective.directiveName != "requires") {
41+
when (GraphQLTypeUtil.unwrapNonNull(targetField.type)) {
42+
is GraphQLList -> errors.add("$errorMessage specifies invalid field set - field set references GraphQLList, field=${targetField.name}")
43+
is GraphQLInterfaceType -> errors.add("$errorMessage specifies invalid field set - field set references GraphQLInterfaceType, field=${targetField.name}")
44+
is GraphQLUnionType -> errors.add("$errorMessage specifies invalid field set - field set references GraphQLUnionType, field=${targetField.name}")
45+
}
46+
}
47+
} else {
48+
errors.add("$errorMessage specifies invalid field set - field set specifies fields that do not exist")
49+
}
50+
}

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

Lines changed: 0 additions & 43 deletions
This file was deleted.

‎graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/federation/data/integration/requires/success/_2/RequiresSelectionOnList.kt‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
/*
2+
* Copyright 2020 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.federation.data.integration.requires.success._2
18+
19+
import com.expediagroup.graphql.federation.directives.ExtendsDirective
20+
import com.expediagroup.graphql.federation.directives.ExternalDirective
21+
import com.expediagroup.graphql.federation.directives.FieldSet
22+
import com.expediagroup.graphql.federation.directives.KeyDirective
23+
import com.expediagroup.graphql.federation.directives.RequiresDirective
24+
import kotlin.properties.Delegates
25+
26+
class RequiresSelectionOnList {
27+
28+
@KeyDirective(fields = FieldSet("id"))
29+
@ExtendsDirective
30+
data class User(@ExternalDirective val id: Int) {
31+
32+
@ExternalDirective
33+
var email: String by Delegates.notNull()
34+
35+
@RequiresDirective(FieldSet("email"))
36+
var reviews: List<UserReview> by Delegates.notNull()
37+
}
38+
39+
data class UserReview(val id: Int)
40+
}

‎graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/federation/data/integration/requires/success/_3/RequiresSelectionOnInterface.kt‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/*
2+
* Copyright 2020 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.federation.data.integration.requires.success._3
18+
19+
import com.expediagroup.graphql.federation.directives.ExtendsDirective
20+
import com.expediagroup.graphql.federation.directives.ExternalDirective
21+
import com.expediagroup.graphql.federation.directives.FieldSet
22+
import com.expediagroup.graphql.federation.directives.KeyDirective
23+
import com.expediagroup.graphql.federation.directives.RequiresDirective
24+
import kotlin.properties.Delegates
25+
26+
class RequiresSelectionOnInterface {
27+
28+
@KeyDirective(fields = FieldSet("id"))
29+
@ExtendsDirective
30+
data class User(@ExternalDirective val id: Int) {
31+
32+
@ExternalDirective
33+
var email: String by Delegates.notNull()
34+
35+
@RequiresDirective(FieldSet("email"))
36+
var reviews: List<UserReview> by Delegates.notNull()
37+
}
38+
39+
interface UserReview {
40+
val id: Int
41+
}
42+
43+
data class ProductUserReview(override val id: Int) : UserReview
44+
}

‎graphql-kotlin-federation/src/test/kotlin/com/expediagroup/graphql/federation/data/integration/requires/success/_4/RequiresSelectionOnUnion.kt‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/*
2+
* Copyright 2020 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.federation.data.integration.requires.success._4
18+
19+
import com.expediagroup.graphql.federation.directives.ExtendsDirective
20+
import com.expediagroup.graphql.federation.directives.ExternalDirective
21+
import com.expediagroup.graphql.federation.directives.FieldSet
22+
import com.expediagroup.graphql.federation.directives.KeyDirective
23+
import com.expediagroup.graphql.federation.directives.RequiresDirective
24+
import kotlin.properties.Delegates
25+
26+
class RequiresSelectionOnUnion {
27+
28+
@KeyDirective(fields = FieldSet("id"))
29+
@ExtendsDirective
30+
data class User(@ExternalDirective val id: Int) {
31+
32+
@ExternalDirective
33+
var email: String by Delegates.notNull()
34+
35+
@RequiresDirective(FieldSet("email"))
36+
var reviews: List<UserReview> by Delegates.notNull()
37+
}
38+
39+
interface UserReview
40+
41+
data class ProductUserReview(val id: Int) : UserReview
42+
}

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

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,17 @@ import kotlin.test.assertTrue
2626

2727
class ValidateFieldSelectionKtTest {
2828

29+
private val mockDirectiveInfo = DirectiveInfo(
30+
directiveName = "foo",
31+
fieldSet = "id",
32+
typeName = "Bar"
33+
)
34+
2935
@Test
3036
fun `empty list returns no errors`() {
3137
val errors = mutableListOf<String>()
3238
validateFieldSelection(
33-
validatedDirective = "",
39+
validatedDirective = mockDirectiveInfo,
3440
iterator = emptyList<String>().iterator(),
3541
fields = emptyMap(),
3642
extendedType = false,
@@ -65,15 +71,15 @@ class ValidateFieldSelectionKtTest {
6571
.build()
6672
val errors = mutableListOf<String>()
6773
validateFieldSelection(
68-
validatedDirective = "taco",
74+
validatedDirective = mockDirectiveInfo,
6975
iterator = listOf("foo", "{", "bar", "}").iterator(),
7076
fields = mapOf("foo" to fieldDefinition),
7177
extendedType = false,
7278
errors = errors
7379
)
7480

7581
assertEquals(expected = 1, actual = errors.size)
76-
assertEquals(expected = "taco specifies invalid field set - field set references GraphQLInterfaceType, field=foo", actual = errors.first())
82+
assertEquals(expected = "@foo(fields = id) directive on Bar specifies invalid field set - field set references GraphQLInterfaceType, field=foo", actual = errors.first())
7783
}
7884

7985
/**
@@ -101,7 +107,7 @@ class ValidateFieldSelectionKtTest {
101107
.build()
102108
val errors = mutableListOf<String>()
103109
validateFieldSelection(
104-
validatedDirective = "taco",
110+
validatedDirective = mockDirectiveInfo,
105111
iterator = listOf("foo", "{", "bar", "}").iterator(),
106112
fields = mapOf("foo" to fieldDefinition),
107113
extendedType = false,
@@ -124,7 +130,7 @@ class ValidateFieldSelectionKtTest {
124130
.build()
125131
val errors = mutableListOf<String>()
126132
validateFieldSelection(
127-
validatedDirective = "taco",
133+
validatedDirective = mockDirectiveInfo,
128134
iterator = listOf("foo").iterator(),
129135
fields = mapOf("foo" to fieldDefinition),
130136
extendedType = false,
@@ -148,7 +154,7 @@ class ValidateFieldSelectionKtTest {
148154

149155
val errors = mutableListOf<String>()
150156
validateFieldSelection(
151-
validatedDirective = "",
157+
validatedDirective = mockDirectiveInfo,
152158
iterator = listOf("bar", "{", "foo", "}").iterator(),
153159
fields = mapOf("foo" to fieldDefinition),
154160
extendedType = false,

0 commit comments

Comments
 (0)