diff --git a/src/main/java/graphql/validation/ValidationErrorType.java b/src/main/java/graphql/validation/ValidationErrorType.java index e480c5c360..e701a5d778 100644 --- a/src/main/java/graphql/validation/ValidationErrorType.java +++ b/src/main/java/graphql/validation/ValidationErrorType.java @@ -43,7 +43,6 @@ 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 8bda10839b..52709109d6 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.SubscriptionRootField; +import graphql.validation.rules.SubscriptionUniqueRootField; 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); - SubscriptionRootField uniqueSubscriptionRootField = new SubscriptionRootField(validationContext, validationErrorCollector); + SubscriptionUniqueRootField uniqueSubscriptionRootField = new SubscriptionUniqueRootField(validationContext, validationErrorCollector); rules.add(uniqueSubscriptionRootField); UniqueObjectFieldName uniqueObjectFieldName = new UniqueObjectFieldName(validationContext, validationErrorCollector); diff --git a/src/main/java/graphql/validation/rules/SubscriptionRootField.java b/src/main/java/graphql/validation/rules/SubscriptionUniqueRootField.java similarity index 58% rename from src/main/java/graphql/validation/rules/SubscriptionRootField.java rename to src/main/java/graphql/validation/rules/SubscriptionUniqueRootField.java index 758ecff605..0ded9ca632 100644 --- a/src/main/java/graphql/validation/rules/SubscriptionRootField.java +++ b/src/main/java/graphql/validation/rules/SubscriptionUniqueRootField.java @@ -6,12 +6,9 @@ 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.VariableDefinition; +import graphql.language.Selection; import graphql.schema.GraphQLObjectType; import graphql.validation.AbstractRule; import graphql.validation.ValidationContext; @@ -19,25 +16,20 @@ 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 SubscriptionRootField extends AbstractRule { +public class SubscriptionUniqueRootField extends AbstractRule { private final FieldCollector fieldCollector = new FieldCollector(); - public SubscriptionRootField(ValidationContext validationContext, ValidationErrorCollector validationErrorCollector) { + public SubscriptionUniqueRootField(ValidationContext validationContext, ValidationErrorCollector validationErrorCollector) { super(validationContext, validationErrorCollector); } @@ -47,24 +39,16 @@ 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(coercedVariableValues.toMap()) + .variables(CoercedVariables.emptyVariables().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()); @@ -73,15 +57,11 @@ 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); - } } } } @@ -89,14 +69,4 @@ 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 e638233cf2..a9403bea5b 100644 --- a/src/main/resources/i18n/Validation.properties +++ b/src/main/resources/i18n/Validation.properties @@ -68,8 +68,9 @@ 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 -SubscriptionRootField.forbidSkipAndIncludeOnSubscriptionRoot=Validation error ({0}) : Subscription operation ''{1}'' root field ''{2}'' must not use @skip nor @include directives in top level selection +SubscriptionIntrospectionRootField.introspectionRootFieldWithFragment=Validation error ({0}) : Subscription operation ''{1}'' fragment root field ''{2}'' cannot be an introspection field # 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 fa58577fa1..7823c9d511 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 -SubscriptionRootField.forbidSkipAndIncludeOnSubscriptionRoot=Validierungsfehler ({0}) : Subscription operation ''{1}'' root field ''{2}'' darf weder @skip noch @include directive in top level selection -# +SubscriptionIntrospectionRootField.introspectionRootFieldWithFragment=Validierungsfehler ({0}) : Subscription operation ''{1}'' fragment root field ''{2}'' kann kein introspection field sein # 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 4cef5f2a0a..e30b342640 100644 --- a/src/main/resources/i18n/Validation_nl.properties +++ b/src/main/resources/i18n/Validation_nl.properties @@ -58,8 +58,9 @@ 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 -SubscriptionRootField.forbidSkipAndIncludeOnSubscriptionRoot=Validation error ({0}) : Subscription operation ''{1}'' root field ''{2}'' must not use @skip nor @include directives in top level selection +SubscriptionIntrospectionRootField.introspectionRootFieldWithFragment=Validatiefout ({0}) : Subscription operation ''{1}'' fragment root field ''{2}'' kan geen introspectieveld zijn # 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 deleted file mode 100644 index 31b526fe4c..0000000000 --- a/src/test/groovy/graphql/validation/rules/SubscriptionRootFieldNoSkipNoIncludeTest.groovy +++ /dev/null @@ -1,154 +0,0 @@ -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/SubscriptionRootFieldTest.groovy b/src/test/groovy/graphql/validation/rules/SubscriptionUniqueRootFieldTest.groovy similarity index 99% rename from src/test/groovy/graphql/validation/rules/SubscriptionRootFieldTest.groovy rename to src/test/groovy/graphql/validation/rules/SubscriptionUniqueRootFieldTest.groovy index 53f0d7fe50..9b171f2256 100644 --- a/src/test/groovy/graphql/validation/rules/SubscriptionRootFieldTest.groovy +++ b/src/test/groovy/graphql/validation/rules/SubscriptionUniqueRootFieldTest.groovy @@ -7,7 +7,7 @@ import graphql.validation.ValidationErrorType import graphql.validation.Validator import spock.lang.Specification -class SubscriptionRootFieldTest extends Specification { +class SubscriptionUniqueRootFieldTest extends Specification { def "5.2.3.1 subscription with only one root field passes validation"() { given: def subscriptionOneRoot = ''' @@ -286,7 +286,6 @@ class SubscriptionRootFieldTest 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)