diff --git a/src/main/java/graphql/validation/ValidationErrorType.java b/src/main/java/graphql/validation/ValidationErrorType.java index e701a5d778..e480c5c360 100644 --- a/src/main/java/graphql/validation/ValidationErrorType.java +++ b/src/main/java/graphql/validation/ValidationErrorType.java @@ -43,6 +43,7 @@ public enum ValidationErrorType implements ValidationErrorClassification { NullValueForNonNullArgument, SubscriptionMultipleRootFields, SubscriptionIntrospectionRootField, + ForbidSkipAndIncludeOnSubscriptionRoot, UniqueObjectFieldName, UnknownOperation } diff --git a/src/main/java/graphql/validation/Validator.java b/src/main/java/graphql/validation/Validator.java index 52709109d6..8bda10839b 100644 --- a/src/main/java/graphql/validation/Validator.java +++ b/src/main/java/graphql/validation/Validator.java @@ -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; import graphql.validation.rules.UniqueArgumentNames; import graphql.validation.rules.UniqueDirectiveNamesPerLocation; import graphql.validation.rules.UniqueFragmentNames; @@ -155,7 +155,7 @@ public List 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); diff --git a/src/main/java/graphql/validation/rules/SubscriptionUniqueRootField.java b/src/main/java/graphql/validation/rules/SubscriptionRootField.java similarity index 58% rename from src/main/java/graphql/validation/rules/SubscriptionUniqueRootField.java rename to src/main/java/graphql/validation/rules/SubscriptionRootField.java index 0ded9ca632..758ecff605 100644 --- a/src/main/java/graphql/validation/rules/SubscriptionUniqueRootField.java +++ b/src/main/java/graphql/validation/rules/SubscriptionRootField.java @@ -6,9 +6,12 @@ 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; @@ -16,20 +19,25 @@ 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); } @@ -39,16 +47,24 @@ public void checkOperationDefinition(OperationDefinition operationDef) { GraphQLObjectType subscriptionType = getValidationContext().getSchema().getSubscriptionType(); + // This coercion takes into account default values for variables + List 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 subscriptionSelections = operationDef.getSelectionSet().getSelections(); if (fields.size() > 1) { String message = i18n(SubscriptionMultipleRootFields, "SubscriptionUniqueRootField.multipleRootFields", operationDef.getName()); @@ -57,11 +73,15 @@ 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); + } } } } @@ -69,4 +89,14 @@ public void checkOperationDefinition(OperationDefinition operationDef) { private boolean isIntrospectionField(MergedField field) { return field.getName().startsWith("__"); } + + private boolean hasSkipOrIncludeDirectives(MergedField field) { + List directives = field.getSingleField().getDirectives(); + for (Directive directive : directives) { + if (directive.getName().equals(SKIP_DIRECTIVE_DEFINITION.getName()) || directive.getName().equals(INCLUDE_DIRECTIVE_DEFINITION.getName())) { + return true; + } + } + return false; + } } diff --git a/src/main/resources/i18n/Validation.properties b/src/main/resources/i18n/Validation.properties index a9403bea5b..e638233cf2 100644 --- a/src/main/resources/i18n/Validation.properties +++ b/src/main/resources/i18n/Validation.properties @@ -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 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}'' # diff --git a/src/main/resources/i18n/Validation_de.properties b/src/main/resources/i18n/Validation_de.properties index 7823c9d511..fa58577fa1 100644 --- a/src/main/resources/i18n/Validation_de.properties +++ b/src/main/resources/i18n/Validation_de.properties @@ -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 +# # UniqueArgumentNames.uniqueArgument=Validierungsfehler ({0}) : Es kann nur ein Argument namens ''{1}'' geben # diff --git a/src/main/resources/i18n/Validation_nl.properties b/src/main/resources/i18n/Validation_nl.properties index e30b342640..4cef5f2a0a 100644 --- a/src/main/resources/i18n/Validation_nl.properties +++ b/src/main/resources/i18n/Validation_nl.properties @@ -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 # UniqueArgumentNames.uniqueArgument=Validatiefout ({0}) : Er mag maar één argument met naam ''{1}'' bestaan # diff --git a/src/test/groovy/graphql/validation/rules/SubscriptionRootFieldNoSkipNoIncludeTest.groovy b/src/test/groovy/graphql/validation/rules/SubscriptionRootFieldNoSkipNoIncludeTest.groovy new file mode 100644 index 0000000000..31b526fe4c --- /dev/null +++ b/src/test/groovy/graphql/validation/rules/SubscriptionRootFieldNoSkipNoIncludeTest.groovy @@ -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) { + 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 validate(String query) { + def document = new Parser().parseDocument(query) + return new Validator().validateDocument(SpecValidationSchema.specValidationSchema, document, Locale.ENGLISH) + } +} diff --git a/src/test/groovy/graphql/validation/rules/SubscriptionUniqueRootFieldTest.groovy b/src/test/groovy/graphql/validation/rules/SubscriptionRootFieldTest.groovy similarity index 99% rename from src/test/groovy/graphql/validation/rules/SubscriptionUniqueRootFieldTest.groovy rename to src/test/groovy/graphql/validation/rules/SubscriptionRootFieldTest.groovy index 9b171f2256..53f0d7fe50 100644 --- a/src/test/groovy/graphql/validation/rules/SubscriptionUniqueRootFieldTest.groovy +++ b/src/test/groovy/graphql/validation/rules/SubscriptionRootFieldTest.groovy @@ -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 = ''' @@ -286,6 +286,7 @@ class SubscriptionUniqueRootFieldTest extends Specification { then: validationErrors.empty } + static List validate(String query) { def document = new Parser().parseDocument(query) return new Validator().validateDocument(SpecValidationSchema.specValidationSchema, document, Locale.ENGLISH)