Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/main/java/graphql/validation/ValidationErrorType.java
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ public enum ValidationErrorType implements ValidationErrorClassification {
NullValueForNonNullArgument,
SubscriptionMultipleRootFields,
SubscriptionIntrospectionRootField,
ForbidSkipAndIncludeOnSubscriptionRoot,
UniqueObjectFieldName,
UnknownOperation
}
4 changes: 2 additions & 2 deletions src/main/java/graphql/validation/Validator.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
import graphql.validation.rules.PossibleFragmentSpreads;
import graphql.validation.rules.ProvidedNonNullArguments;
import graphql.validation.rules.ScalarLeaves;
import graphql.validation.rules.SubscriptionUniqueRootField;
import graphql.validation.rules.SubscriptionRootField;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed this rule name to be more generic, reflecting that it does 2 checks on the root field

import graphql.validation.rules.UniqueArgumentNames;
import graphql.validation.rules.UniqueDirectiveNamesPerLocation;
import graphql.validation.rules.UniqueFragmentNames;
Expand Down Expand Up @@ -155,7 +155,7 @@ public List<AbstractRule> createRules(ValidationContext validationContext, Valid
UniqueVariableNames uniqueVariableNamesRule = new UniqueVariableNames(validationContext, validationErrorCollector);
rules.add(uniqueVariableNamesRule);

SubscriptionUniqueRootField uniqueSubscriptionRootField = new SubscriptionUniqueRootField(validationContext, validationErrorCollector);
SubscriptionRootField uniqueSubscriptionRootField = new SubscriptionRootField(validationContext, validationErrorCollector);
rules.add(uniqueSubscriptionRootField);

UniqueObjectFieldName uniqueObjectFieldName = new UniqueObjectFieldName(validationContext, validationErrorCollector);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,30 +6,38 @@
import graphql.execution.FieldCollectorParameters;
import graphql.execution.MergedField;
import graphql.execution.MergedSelectionSet;
import graphql.execution.RawVariables;
import graphql.execution.ValuesResolver;
import graphql.language.Directive;
import graphql.language.NodeUtil;
import graphql.language.OperationDefinition;
import graphql.language.Selection;
import graphql.language.VariableDefinition;
import graphql.schema.GraphQLObjectType;
import graphql.validation.AbstractRule;
import graphql.validation.ValidationContext;
import graphql.validation.ValidationErrorCollector;

import java.util.List;

import static graphql.Directives.INCLUDE_DIRECTIVE_DEFINITION;
import static graphql.Directives.SKIP_DIRECTIVE_DEFINITION;
import static graphql.language.OperationDefinition.Operation.SUBSCRIPTION;
import static graphql.validation.ValidationErrorType.SubscriptionIntrospectionRootField;
import static graphql.validation.ValidationErrorType.SubscriptionMultipleRootFields;
import static graphql.validation.ValidationErrorType.ForbidSkipAndIncludeOnSubscriptionRoot;


/**
* A subscription operation must only have one root field
* A subscription operation's single root field must not be an introspection field
* https://spec.graphql.org/draft/#sec-Single-root-field
*
* A subscription operation's root field must not have neither @skip nor @include directives
*/
@Internal
public class SubscriptionUniqueRootField extends AbstractRule {
public class SubscriptionRootField extends AbstractRule {
private final FieldCollector fieldCollector = new FieldCollector();
public SubscriptionUniqueRootField(ValidationContext validationContext, ValidationErrorCollector validationErrorCollector) {
public SubscriptionRootField(ValidationContext validationContext, ValidationErrorCollector validationErrorCollector) {
super(validationContext, validationErrorCollector);
}

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

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

// This coercion takes into account default values for variables
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fixing an edge case - this check previously didn't take into account default variable values because there was previously no coercion.

That caused my tests to fail (see later) because I had set a default boolean value for include and skip.

List<VariableDefinition> variableDefinitions = operationDef.getVariableDefinitions();
CoercedVariables coercedVariableValues = ValuesResolver.coerceVariableValues(
getValidationContext().getSchema(),
variableDefinitions,
RawVariables.emptyVariables(),
getValidationContext().getGraphQLContext(),
getValidationContext().getI18n().getLocale());

FieldCollectorParameters collectorParameters = FieldCollectorParameters.newParameters()
.schema(getValidationContext().getSchema())
.fragments(NodeUtil.getFragmentsByName(getValidationContext().getDocument()))
.variables(CoercedVariables.emptyVariables().toMap())
.variables(coercedVariableValues.toMap())
.objectType(subscriptionType)
.graphQLContext(getValidationContext().getGraphQLContext())
.build();

MergedSelectionSet fields = fieldCollector.collectFields(collectorParameters, operationDef.getSelectionSet());
List<Selection> subscriptionSelections = operationDef.getSelectionSet().getSelections();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused variable


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

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


if (isIntrospectionField(mergedField)) {
String message = i18n(SubscriptionIntrospectionRootField, "SubscriptionIntrospectionRootField.introspectionRootField", operationDef.getName(), mergedField.getName());
addError(SubscriptionIntrospectionRootField, mergedField.getSingleField().getSourceLocation(), message);
}

if (hasSkipOrIncludeDirectives(mergedField)) {
String message = i18n(ForbidSkipAndIncludeOnSubscriptionRoot, "SubscriptionRootField.forbidSkipAndIncludeOnSubscriptionRoot", operationDef.getName(), mergedField.getName());
addError(ForbidSkipAndIncludeOnSubscriptionRoot, mergedField.getSingleField().getSourceLocation(), message);
}
}
}
}

private boolean isIntrospectionField(MergedField field) {
return field.getName().startsWith("__");
}

private boolean hasSkipOrIncludeDirectives(MergedField field) {
List<Directive> directives = field.getSingleField().getDirectives();
for (Directive directive : directives) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given recent performance improvements I am using plain for loops

if (directive.getName().equals(SKIP_DIRECTIVE_DEFINITION.getName()) || directive.getName().equals(INCLUDE_DIRECTIVE_DEFINITION.getName())) {
return true;
}
}
return false;
}
}
3 changes: 1 addition & 2 deletions src/main/resources/i18n/Validation.properties
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,8 @@ ScalarLeaves.subselectionOnLeaf=Validation error ({0}) : Subselection not allowe
ScalarLeaves.subselectionRequired=Validation error ({0}) : Subselection required for type ''{1}'' of field ''{2}''
#
SubscriptionUniqueRootField.multipleRootFields=Validation error ({0}) : Subscription operation ''{1}'' must have exactly one root field
SubscriptionUniqueRootField.multipleRootFieldsWithFragment=Validation error ({0}) : Subscription operation ''{1}'' must have exactly one root field with fragments
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused messages

SubscriptionIntrospectionRootField.introspectionRootField=Validation error ({0}) : Subscription operation ''{1}'' root field ''{2}'' cannot be an introspection field
SubscriptionIntrospectionRootField.introspectionRootFieldWithFragment=Validation error ({0}) : Subscription operation ''{1}'' fragment root field ''{2}'' cannot be an introspection field
SubscriptionRootField.forbidSkipAndIncludeOnSubscriptionRoot=Validation error ({0}) : Subscription operation ''{1}'' root field ''{2}'' must not use @skip nor @include directives in top level selection
#
UniqueArgumentNames.uniqueArgument=Validation error ({0}) : There can be only one argument named ''{1}''
#
Expand Down
4 changes: 2 additions & 2 deletions src/main/resources/i18n/Validation_de.properties
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,9 @@ ScalarLeaves.subselectionOnLeaf=Validierungsfehler ({0}) : Unterauswahl für Bla
ScalarLeaves.subselectionRequired=Validierungsfehler ({0}) : Unterauswahl erforderlich für Typ ''{1}'' des Feldes ''{2}''
#
SubscriptionUniqueRootField.multipleRootFields=Validierungsfehler ({0}) : Subscription operation ''{1}'' muss genau ein root field haben
SubscriptionUniqueRootField.multipleRootFieldsWithFragment=Validierungsfehler ({0}) : Subscription operation ''{1}'' muss genau ein root field mit Fragmenten haben
SubscriptionIntrospectionRootField.introspectionRootField=Validierungsfehler ({0}) : Subscription operation ''{1}'' root field ''{2}'' kann kein introspection field sein
SubscriptionIntrospectionRootField.introspectionRootFieldWithFragment=Validierungsfehler ({0}) : Subscription operation ''{1}'' fragment root field ''{2}'' kann kein introspection field sein
SubscriptionRootField.forbidSkipAndIncludeOnSubscriptionRoot=Validierungsfehler ({0}) : Subscription operation ''{1}'' root field ''{2}'' darf weder @skip noch @include directive in top level selection
#
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check my German please

#
UniqueArgumentNames.uniqueArgument=Validierungsfehler ({0}) : Es kann nur ein Argument namens ''{1}'' geben
#
Expand Down
3 changes: 1 addition & 2 deletions src/main/resources/i18n/Validation_nl.properties
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,8 @@ ScalarLeaves.subselectionOnLeaf=Validatiefout ({0}) : Sub-selectie niet toegesta
ScalarLeaves.subselectionRequired=Validatiefout ({0}) : Sub-selectie verplicht voor type ''{1}'' van veld ''{2}''
#
SubscriptionUniqueRootField.multipleRootFields=Validatiefout ({0}) : Subscription operation ''{1}'' moet exact één root field hebben
SubscriptionUniqueRootField.multipleRootFieldsWithFragment=Validatiefout ({0}) : Subscription operation ''{1}'' moet exact één root field met fragmenten hebben
SubscriptionIntrospectionRootField.introspectionRootField=Validatiefout ({0}) : Subscription operation ''{1}'' root field ''{2}'' kan geen introspectieveld zijn
SubscriptionIntrospectionRootField.introspectionRootFieldWithFragment=Validatiefout ({0}) : Subscription operation ''{1}'' fragment root field ''{2}'' kan geen introspectieveld zijn
SubscriptionRootField.forbidSkipAndIncludeOnSubscriptionRoot=Validation error ({0}) : Subscription operation ''{1}'' root field ''{2}'' must not use @skip nor @include directives in top level selection
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add English placeholder

#
UniqueArgumentNames.uniqueArgument=Validatiefout ({0}) : Er mag maar één argument met naam ''{1}'' bestaan
#
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,154 @@
package graphql.validation.rules

import graphql.parser.Parser
import graphql.validation.SpecValidationSchema
import graphql.validation.ValidationError
import graphql.validation.Validator
import spock.lang.Specification

class SubscriptionRootFieldNoSkipNoIncludeTest extends Specification {

def "valid subscription with @skip and @include directives on subfields"() {
given:
def query = """
subscription MySubscription(\$bool: Boolean = true) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what I mean by default values. When an empty coerced variables map is passed, this test fails

dog {
name @skip(if: \$bool)
nickname @include(if: \$bool)
}
}
"""

when:
def validationErrors = validate(query)

then:
validationErrors.isEmpty()
}

def "invalid subscription with @skip directive on root field"() {
given:
def query = """
subscription MySubscription(\$bool: Boolean = false) {
dog @skip(if: \$bool) {
name
}
dog @include(if: true) {
nickname
}
}
"""

when:
def validationErrors = validate(query)

then:
validationErrors.size() == 1
validationErrors.first().getMessage().contains("Subscription operation 'MySubscription' root field 'dog' must not use @skip nor @include directives in top level selection")
}

def "invalid subscription with @include directive on root field"() {
given:
def query = """
subscription MySubscription(\$bool: Boolean = true) {
dog @include(if: \$bool) {
name
}
}
"""

when:
def validationErrors = validate(query)

then:
validationErrors.size() == 1
validationErrors.first().getMessage().contains("Subscription operation 'MySubscription' root field 'dog' must not use @skip nor @include directives in top level selection")
}

def "invalid subscription with directive on root field in fragment spread"() {
given:
def query = """
subscription MySubscription(\$bool: Boolean = false) {
...dogFragment
}

fragment dogFragment on SubscriptionRoot {
dog @skip(if: \$bool) {
name
}
}
"""

when:
def validationErrors = validate(query)

then:
validationErrors.size() == 1
validationErrors.first().getMessage().contains("Subscription operation 'MySubscription' root field 'dog' must not use @skip nor @include directives in top level selection")
}

def "invalid subscription with directive on root field in inline fragment"() {
given:
def query = """
subscription MySubscription(\$bool: Boolean = true) {
... on SubscriptionRoot {
dog @include(if: \$bool) {
name
}
}
}
"""

when:
def validationErrors = validate(query)

then:
validationErrors.size() == 1
validationErrors.first().getMessage().contains("Subscription operation 'MySubscription' root field 'dog' must not use @skip nor @include directives in top level selection")
}

def "@skip and @include directives are valid on query root fields"() {
given:
def query = """
query MyQuery {
pet @skip(if: false) {
name
}
pet @include(if: true) {
name
}
}
"""

when:
def validationErrors = validate(query)

then:
validationErrors.size() == 0
}

def "@skip and @include directives are valid on mutation root fields"() {
given:
def query = """
mutation MyMutation {
createDog(input: {id: "a"}) @skip(if: false) {
name
}
createDog(input: {id: "a"}) @include(if: true) {
name
}
}
"""

when:
def validationErrors = validate(query)

then:
validationErrors.size() == 0
}

static List<ValidationError> validate(String query) {
def document = new Parser().parseDocument(query)
return new Validator().validateDocument(SpecValidationSchema.specValidationSchema, document, Locale.ENGLISH)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import graphql.validation.ValidationErrorType
import graphql.validation.Validator
import spock.lang.Specification

class SubscriptionUniqueRootFieldTest extends Specification {
class SubscriptionRootFieldTest extends Specification {
def "5.2.3.1 subscription with only one root field passes validation"() {
given:
def subscriptionOneRoot = '''
Expand Down Expand Up @@ -286,6 +286,7 @@ class SubscriptionUniqueRootFieldTest extends Specification {
then:
validationErrors.empty
}

static List<ValidationError> validate(String query) {
def document = new Parser().parseDocument(query)
return new Validator().validateDocument(SpecValidationSchema.specValidationSchema, document, Locale.ENGLISH)
Expand Down
Loading