applied directive values are validated

andimarek · andimarek · commit 5d3d775abe29 · 2021-05-21T15:55:55.000+10:00
diff --git a/src/main/java/graphql/schema/GraphQLDirective.java b/src/main/java/graphql/schema/GraphQLDirective.java
@@ -96,7 +96,7 @@ public DirectiveDefinition getDefinition() {
     public String toString() {
         return "GraphQLDirective{" +
                 "name='" + name + '\'' +
-                "repeatable='" + repeatable + '\'' +
+                ", repeatable='" + repeatable + '\'' +
                 ", arguments=" + arguments +
                 ", locations=" + locations +
                 '}';
diff --git a/src/main/java/graphql/schema/validation/AppliedDirectiveArgumentsAreValid.java b/src/main/java/graphql/schema/validation/AppliedDirectiveArgumentsAreValid.java
@@ -5,6 +5,7 @@
 import graphql.language.Value;
 import graphql.schema.CoercingParseValueException;
 import graphql.schema.GraphQLArgument;
+import graphql.schema.GraphQLDirective;
 import graphql.schema.GraphQLInputType;
 import graphql.schema.GraphQLSchema;
 import graphql.schema.GraphQLSchemaElement;
@@ -14,18 +15,27 @@
 import graphql.util.TraverserContext;
 import graphql.validation.ValidationUtil;
 
-import static graphql.schema.GraphQLTypeUtil.simplePrint;
 import static java.lang.String.format;
 
 public class AppliedDirectiveArgumentsAreValid extends GraphQLTypeVisitorStub {
 
     private ValidationUtil validationUtil = new ValidationUtil();
 
 
-    public TraversalControl visitGraphQLArgument(GraphQLArgument argument, TraverserContext<GraphQLSchemaElement> context) {
-        // a directive argument is represented as GraphQLArgument.value
+    @Override
+    public TraversalControl visitGraphQLDirective(GraphQLDirective directive, TraverserContext<GraphQLSchemaElement> context) {
+        // if there is no parent it means it is just a directive definition and not an applied directive
+        if (context.getParentNode() != null) {
+            for (GraphQLArgument graphQLArgument : directive.getArguments()) {
+                checkArgument(directive, graphQLArgument, context);
+            }
+        }
+        return TraversalControl.CONTINUE;
+    }
+
+    private void checkArgument(GraphQLDirective directive, GraphQLArgument argument, TraverserContext<GraphQLSchemaElement> context) {
         if (!argument.hasSetValue()) {
-            return TraversalControl.CONTINUE;
+            return;
         }
         GraphQLSchema schema = context.getVarFromParents(GraphQLSchema.class);
         SchemaValidationErrorCollector errorCollector = context.getVarFromParents(SchemaValidationErrorCollector.class);
@@ -39,10 +49,9 @@ public TraversalControl visitGraphQLArgument(GraphQLArgument argument, Traverser
             invalid = true;
         }
         if (invalid) {
-            String message = format("Invalid argument %s for type %s for applied directive", argument.getValueState(), simplePrint(argument.getType()));
+            String message = format("Invalid argument '%s' for applied directive of name '%s'", argument.getName(), directive.getName());
             errorCollector.addError(new SchemaValidationError(SchemaValidationErrorType.InvalidAppliedDirectiveArgument, message));
         }
-        return TraversalControl.CONTINUE;
     }
 
     private boolean isValidExternalValue(GraphQLSchema schema, Object externalValue, GraphQLInputType type) {
diff --git a/src/test/groovy/graphql/GraphQLTest.groovy b/src/test/groovy/graphql/GraphQLTest.groovy
@@ -1355,6 +1355,6 @@ many lines''']
                 .build()
         then:
         def e = thrown(InvalidSchemaException)
-        e.message.contains("Invalid argument")
+        e.message.contains("Invalid argument 'arg' for applied directive of name 'cached'")
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -1355,6 +1355,6 @@ many lines''']`
`1355`	`1355`	`.build()`
`1356`	`1356`	`then:`
`1357`	`1357`	`def e = thrown(InvalidSchemaException)`
`1358`		`- e.message.contains("Invalid argument")`
	`1358`	`+ e.message.contains("Invalid argument 'arg' for applied directive of name 'cached'")`
`1359`	`1359`	`}`
`1360`	`1360`	`}`