Skip to content

Commit 12273f4

Browse files
dondonzdondonz
authored
Merge pull request #3871 from graphql-java/subscriptions-root-field
New spec validation: Subscriptions root field must not contain @Skip nor @include on root selection set
2 parents 4c17673 + 0219c47 commit 12273f4

File tree

8 files changed

+199
-15
lines changed

8 files changed

+199
-15
lines changed

‎src/main/java/graphql/validation/ValidationErrorType.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ public enum ValidationErrorType implements ValidationErrorClassification {
4343
NullValueForNonNullArgument,
4444
SubscriptionMultipleRootFields,
4545
SubscriptionIntrospectionRootField,
46+
ForbidSkipAndIncludeOnSubscriptionRoot,
4647
UniqueObjectFieldName,
4748
UnknownOperation
4849
}

‎src/main/java/graphql/validation/Validator.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
import graphql.validation.rules.PossibleFragmentSpreads;
2828
import graphql.validation.rules.ProvidedNonNullArguments;
2929
import graphql.validation.rules.ScalarLeaves;
30-
import graphql.validation.rules.SubscriptionUniqueRootField;
30+
import graphql.validation.rules.SubscriptionRootField;
3131
import graphql.validation.rules.UniqueArgumentNames;
3232
import graphql.validation.rules.UniqueDirectiveNamesPerLocation;
3333
import graphql.validation.rules.UniqueFragmentNames;
@@ -155,7 +155,7 @@ public List<AbstractRule> createRules(ValidationContext validationContext, Valid
155155
UniqueVariableNames uniqueVariableNamesRule = new UniqueVariableNames(validationContext, validationErrorCollector);
156156
rules.add(uniqueVariableNamesRule);
157157

158-
SubscriptionUniqueRootField uniqueSubscriptionRootField = new SubscriptionUniqueRootField(validationContext, validationErrorCollector);
158+
SubscriptionRootField uniqueSubscriptionRootField = new SubscriptionRootField(validationContext, validationErrorCollector);
159159
rules.add(uniqueSubscriptionRootField);
160160

161161
UniqueObjectFieldName uniqueObjectFieldName = new UniqueObjectFieldName(validationContext, validationErrorCollector);

‎src/main/java/graphql/validation/rules/SubscriptionUniqueRootField.java renamed to ‎src/main/java/graphql/validation/rules/SubscriptionRootField.java

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,30 +6,38 @@
66
import graphql.execution.FieldCollectorParameters;
77
import graphql.execution.MergedField;
88
import graphql.execution.MergedSelectionSet;
9+
import graphql.execution.RawVariables;
10+
import graphql.execution.ValuesResolver;
11+
import graphql.language.Directive;
912
import graphql.language.NodeUtil;
1013
import graphql.language.OperationDefinition;
11-
import graphql.language.Selection;
14+
import graphql.language.VariableDefinition;
1215
import graphql.schema.GraphQLObjectType;
1316
import graphql.validation.AbstractRule;
1417
import graphql.validation.ValidationContext;
1518
import graphql.validation.ValidationErrorCollector;
1619

1720
import java.util.List;
1821

22+
import static graphql.Directives.INCLUDE_DIRECTIVE_DEFINITION;
23+
import static graphql.Directives.SKIP_DIRECTIVE_DEFINITION;
1924
import static graphql.language.OperationDefinition.Operation.SUBSCRIPTION;
2025
import static graphql.validation.ValidationErrorType.SubscriptionIntrospectionRootField;
2126
import static graphql.validation.ValidationErrorType.SubscriptionMultipleRootFields;
27+
import static graphql.validation.ValidationErrorType.ForbidSkipAndIncludeOnSubscriptionRoot;
2228

2329

2430
/**
2531
* A subscription operation must only have one root field
2632
* A subscription operation's single root field must not be an introspection field
2733
* https://spec.graphql.org/draft/#sec-Single-root-field
34+
*
35+
* A subscription operation's root field must not have neither @skip nor @include directives
2836
*/
2937
@Internal
30-
public class SubscriptionUniqueRootField extends AbstractRule {
38+
public class SubscriptionRootField extends AbstractRule {
3139
private final FieldCollector fieldCollector = new FieldCollector();
32-
public SubscriptionUniqueRootField(ValidationContext validationContext, ValidationErrorCollector validationErrorCollector) {
40+
public SubscriptionRootField(ValidationContext validationContext, ValidationErrorCollector validationErrorCollector) {
3341
super(validationContext, validationErrorCollector);
3442
}
3543

@@ -39,16 +47,24 @@ public void checkOperationDefinition(OperationDefinition operationDef) {
3947

4048
GraphQLObjectType subscriptionType = getValidationContext().getSchema().getSubscriptionType();
4149

50+
// This coercion takes into account default values for variables
51+
List<VariableDefinition> variableDefinitions = operationDef.getVariableDefinitions();
52+
CoercedVariables coercedVariableValues = ValuesResolver.coerceVariableValues(
53+
getValidationContext().getSchema(),
54+
variableDefinitions,
55+
RawVariables.emptyVariables(),
56+
getValidationContext().getGraphQLContext(),
57+
getValidationContext().getI18n().getLocale());
58+
4259
FieldCollectorParameters collectorParameters = FieldCollectorParameters.newParameters()
4360
.schema(getValidationContext().getSchema())
4461
.fragments(NodeUtil.getFragmentsByName(getValidationContext().getDocument()))
45-
.variables(CoercedVariables.emptyVariables().toMap())
62+
.variables(coercedVariableValues.toMap())
4663
.objectType(subscriptionType)
4764
.graphQLContext(getValidationContext().getGraphQLContext())
4865
.build();
4966

5067
MergedSelectionSet fields = fieldCollector.collectFields(collectorParameters, operationDef.getSelectionSet());
51-
List<Selection> subscriptionSelections = operationDef.getSelectionSet().getSelections();
5268

5369
if (fields.size() > 1) {
5470
String message = i18n(SubscriptionMultipleRootFields, "SubscriptionUniqueRootField.multipleRootFields", operationDef.getName());
@@ -57,16 +73,30 @@ public void checkOperationDefinition(OperationDefinition operationDef) {
5773

5874
MergedField mergedField = fields.getSubFieldsList().get(0);
5975

60-
6176
if (isIntrospectionField(mergedField)) {
6277
String message = i18n(SubscriptionIntrospectionRootField, "SubscriptionIntrospectionRootField.introspectionRootField", operationDef.getName(), mergedField.getName());
6378
addError(SubscriptionIntrospectionRootField, mergedField.getSingleField().getSourceLocation(), message);
6479
}
80+
81+
if (hasSkipOrIncludeDirectives(mergedField)) {
82+
String message = i18n(ForbidSkipAndIncludeOnSubscriptionRoot, "SubscriptionRootField.forbidSkipAndIncludeOnSubscriptionRoot", operationDef.getName(), mergedField.getName());
83+
addError(ForbidSkipAndIncludeOnSubscriptionRoot, mergedField.getSingleField().getSourceLocation(), message);
84+
}
6585
}
6686
}
6787
}
6888

6989
private boolean isIntrospectionField(MergedField field) {
7090
return field.getName().startsWith("__");
7191
}
92+
93+
private boolean hasSkipOrIncludeDirectives(MergedField field) {
94+
List<Directive> directives = field.getSingleField().getDirectives();
95+
for (Directive directive : directives) {
96+
if (directive.getName().equals(SKIP_DIRECTIVE_DEFINITION.getName()) || directive.getName().equals(INCLUDE_DIRECTIVE_DEFINITION.getName())) {
97+
return true;
98+
}
99+
}
100+
return false;
101+
}
72102
}

‎src/main/resources/i18n/Validation.properties

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,8 @@ ScalarLeaves.subselectionOnLeaf=Validation error ({0}) : Subselection not allowe
6868
ScalarLeaves.subselectionRequired=Validation error ({0}) : Subselection required for type ''{1}'' of field ''{2}''
6969
#
7070
SubscriptionUniqueRootField.multipleRootFields=Validation error ({0}) : Subscription operation ''{1}'' must have exactly one root field
71-
SubscriptionUniqueRootField.multipleRootFieldsWithFragment=Validation error ({0}) : Subscription operation ''{1}'' must have exactly one root field with fragments
7271
SubscriptionIntrospectionRootField.introspectionRootField=Validation error ({0}) : Subscription operation ''{1}'' root field ''{2}'' cannot be an introspection field
73-
SubscriptionIntrospectionRootField.introspectionRootFieldWithFragment=Validation error ({0}) : Subscription operation ''{1}'' fragment root field ''{2}'' cannot be an introspection field
72+
SubscriptionRootField.forbidSkipAndIncludeOnSubscriptionRoot=Validation error ({0}) : Subscription operation ''{1}'' root field ''{2}'' must not use @skip nor @include directives in top level selection
7473
#
7574
UniqueArgumentNames.uniqueArgument=Validation error ({0}) : There can be only one argument named ''{1}''
7675
#

‎src/main/resources/i18n/Validation_de.properties

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,9 @@ ScalarLeaves.subselectionOnLeaf=Validierungsfehler ({0}) : Unterauswahl für Bla
6060
ScalarLeaves.subselectionRequired=Validierungsfehler ({0}) : Unterauswahl erforderlich für Typ ''{1}'' des Feldes ''{2}''
6161
#
6262
SubscriptionUniqueRootField.multipleRootFields=Validierungsfehler ({0}) : Subscription operation ''{1}'' muss genau ein root field haben
63-
SubscriptionUniqueRootField.multipleRootFieldsWithFragment=Validierungsfehler ({0}) : Subscription operation ''{1}'' muss genau ein root field mit Fragmenten haben
6463
SubscriptionIntrospectionRootField.introspectionRootField=Validierungsfehler ({0}) : Subscription operation ''{1}'' root field ''{2}'' kann kein introspection field sein
65-
SubscriptionIntrospectionRootField.introspectionRootFieldWithFragment=Validierungsfehler ({0}) : Subscription operation ''{1}'' fragment root field ''{2}'' kann kein introspection field sein
64+
SubscriptionRootField.forbidSkipAndIncludeOnSubscriptionRoot=Validierungsfehler ({0}) : Subscription operation ''{1}'' root field ''{2}'' darf weder @skip noch @include directive in top level selection
65+
#
6666
#
6767
UniqueArgumentNames.uniqueArgument=Validierungsfehler ({0}) : Es kann nur ein Argument namens ''{1}'' geben
6868
#

‎src/main/resources/i18n/Validation_nl.properties

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,8 @@ ScalarLeaves.subselectionOnLeaf=Validatiefout ({0}) : Sub-selectie niet toegesta
5858
ScalarLeaves.subselectionRequired=Validatiefout ({0}) : Sub-selectie verplicht voor type ''{1}'' van veld ''{2}''
5959
#
6060
SubscriptionUniqueRootField.multipleRootFields=Validatiefout ({0}) : Subscription operation ''{1}'' moet exact één root field hebben
61-
SubscriptionUniqueRootField.multipleRootFieldsWithFragment=Validatiefout ({0}) : Subscription operation ''{1}'' moet exact één root field met fragmenten hebben
6261
SubscriptionIntrospectionRootField.introspectionRootField=Validatiefout ({0}) : Subscription operation ''{1}'' root field ''{2}'' kan geen introspectieveld zijn
63-
SubscriptionIntrospectionRootField.introspectionRootFieldWithFragment=Validatiefout ({0}) : Subscription operation ''{1}'' fragment root field ''{2}'' kan geen introspectieveld zijn
62+
SubscriptionRootField.forbidSkipAndIncludeOnSubscriptionRoot=Validation error ({0}) : Subscription operation ''{1}'' root field ''{2}'' must not use @skip nor @include directives in top level selection
6463
#
6564
UniqueArgumentNames.uniqueArgument=Validatiefout ({0}) : Er mag maar één argument met naam ''{1}'' bestaan
6665
#

‎src/test/groovy/graphql/validation/rules/SubscriptionRootFieldNoSkipNoIncludeTest.groovy

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
package graphql.validation.rules
2+
3+
import graphql.parser.Parser
4+
import graphql.validation.SpecValidationSchema
5+
import graphql.validation.ValidationError
6+
import graphql.validation.Validator
7+
import spock.lang.Specification
8+
9+
class SubscriptionRootFieldNoSkipNoIncludeTest extends Specification {
10+
11+
def "valid subscription with @skip and @include directives on subfields"() {
12+
given:
13+
def query = """
14+
subscription MySubscription(\$bool: Boolean = true) {
15+
dog {
16+
name @skip(if: \$bool)
17+
nickname @include(if: \$bool)
18+
}
19+
}
20+
"""
21+
22+
when:
23+
def validationErrors = validate(query)
24+
25+
then:
26+
validationErrors.isEmpty()
27+
}
28+
29+
def "invalid subscription with @skip directive on root field"() {
30+
given:
31+
def query = """
32+
subscription MySubscription(\$bool: Boolean = false) {
33+
dog @skip(if: \$bool) {
34+
name
35+
}
36+
dog @include(if: true) {
37+
nickname
38+
}
39+
}
40+
"""
41+
42+
when:
43+
def validationErrors = validate(query)
44+
45+
then:
46+
validationErrors.size() == 1
47+
validationErrors.first().getMessage().contains("Subscription operation 'MySubscription' root field 'dog' must not use @skip nor @include directives in top level selection")
48+
}
49+
50+
def "invalid subscription with @include directive on root field"() {
51+
given:
52+
def query = """
53+
subscription MySubscription(\$bool: Boolean = true) {
54+
dog @include(if: \$bool) {
55+
name
56+
}
57+
}
58+
"""
59+
60+
when:
61+
def validationErrors = validate(query)
62+
63+
then:
64+
validationErrors.size() == 1
65+
validationErrors.first().getMessage().contains("Subscription operation 'MySubscription' root field 'dog' must not use @skip nor @include directives in top level selection")
66+
}
67+
68+
def "invalid subscription with directive on root field in fragment spread"() {
69+
given:
70+
def query = """
71+
subscription MySubscription(\$bool: Boolean = false) {
72+
...dogFragment
73+
}
74+
75+
fragment dogFragment on SubscriptionRoot {
76+
dog @skip(if: \$bool) {
77+
name
78+
}
79+
}
80+
"""
81+
82+
when:
83+
def validationErrors = validate(query)
84+
85+
then:
86+
validationErrors.size() == 1
87+
validationErrors.first().getMessage().contains("Subscription operation 'MySubscription' root field 'dog' must not use @skip nor @include directives in top level selection")
88+
}
89+
90+
def "invalid subscription with directive on root field in inline fragment"() {
91+
given:
92+
def query = """
93+
subscription MySubscription(\$bool: Boolean = true) {
94+
... on SubscriptionRoot {
95+
dog @include(if: \$bool) {
96+
name
97+
}
98+
}
99+
}
100+
"""
101+
102+
when:
103+
def validationErrors = validate(query)
104+
105+
then:
106+
validationErrors.size() == 1
107+
validationErrors.first().getMessage().contains("Subscription operation 'MySubscription' root field 'dog' must not use @skip nor @include directives in top level selection")
108+
}
109+
110+
def "@skip and @include directives are valid on query root fields"() {
111+
given:
112+
def query = """
113+
query MyQuery {
114+
pet @skip(if: false) {
115+
name
116+
}
117+
pet @include(if: true) {
118+
name
119+
}
120+
}
121+
"""
122+
123+
when:
124+
def validationErrors = validate(query)
125+
126+
then:
127+
validationErrors.size() == 0
128+
}
129+
130+
def "@skip and @include directives are valid on mutation root fields"() {
131+
given:
132+
def query = """
133+
mutation MyMutation {
134+
createDog(input: {id: "a"}) @skip(if: false) {
135+
name
136+
}
137+
createDog(input: {id: "a"}) @include(if: true) {
138+
name
139+
}
140+
}
141+
"""
142+
143+
when:
144+
def validationErrors = validate(query)
145+
146+
then:
147+
validationErrors.size() == 0
148+
}
149+
150+
static List<ValidationError> validate(String query) {
151+
def document = new Parser().parseDocument(query)
152+
return new Validator().validateDocument(SpecValidationSchema.specValidationSchema, document, Locale.ENGLISH)
153+
}
154+
}

‎src/test/groovy/graphql/validation/rules/SubscriptionUniqueRootFieldTest.groovy renamed to ‎src/test/groovy/graphql/validation/rules/SubscriptionRootFieldTest.groovy

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import graphql.validation.ValidationErrorType
77
import graphql.validation.Validator
88
import spock.lang.Specification
99

10-
class SubscriptionUniqueRootFieldTest extends Specification {
10+
class SubscriptionRootFieldTest extends Specification {
1111
def "5.2.3.1 subscription with only one root field passes validation"() {
1212
given:
1313
def subscriptionOneRoot = '''
@@ -286,6 +286,7 @@ class SubscriptionUniqueRootFieldTest extends Specification {
286286
then:
287287
validationErrors.empty
288288
}
289+
289290
static List<ValidationError> validate(String query) {
290291
def document = new Parser().parseDocument(query)
291292
return new Validator().validateDocument(SpecValidationSchema.specValidationSchema, document, Locale.ENGLISH)

0 commit comments

Comments
 (0)