Merge pull request #5368 from rmosolgo/schema-directive-validations

Improve schema directive validations

Author: rmosolgo

lib/graphql/execution/interpreter/runtime.rb

@@ -826,6 +826,13 @@ def run_directive(method_name, object, directives, idx, &block)
           else
             dir_defn = @schema_directives.fetch(dir_node.name)
             raw_dir_args = arguments(nil, dir_defn, dir_node)
+            if !raw_dir_args.is_a?(GraphQL::ExecutionError)
+              begin
+                dir_defn.validate!(raw_dir_args, context)
+              rescue GraphQL::ExecutionError => err
+                raw_dir_args = err
+              end
+            end
             dir_args = continue_value(
               raw_dir_args, # value
               nil, # field

lib/graphql/schema/directive.rb

@@ -9,6 +9,7 @@ class Schema
     class Directive < GraphQL::Schema::Member
       extend GraphQL::Schema::Member::HasArguments
       extend GraphQL::Schema::Member::HasArguments::HasDirectiveArguments
+      extend GraphQL::Schema::Member::HasValidators
 
       class << self
         # Directives aren't types, they don't have kinds.
@@ -75,6 +76,10 @@ def resolve_each(object, arguments, context)
           yield
         end
 
+        def validate!(arguments, context)
+          Schema::Validator.validate!(validators, self, context, arguments)
+        end
+
         def on_field?
           locations.include?(FIELD)
         end
@@ -111,6 +116,9 @@ def inherited(subclass)
       # @return [GraphQL::Interpreter::Arguments]
       attr_reader :arguments
 
+      class InvalidArgumentError < GraphQL::Error
+      end
+
       def initialize(owner, **arguments)
         @owner = owner
         assert_valid_owner
@@ -119,7 +127,19 @@ def initialize(owner, **arguments)
         # - lazy resolution
         # Probably, those won't be needed here, since these are configuration arguments,
         # not runtime arguments.
-        @arguments = self.class.coerce_arguments(nil, arguments, Query::NullContext.instance)
+        context = Query::NullContext.instance
+        self.class.all_argument_definitions.each do |arg_defn|
+          value = arguments[arg_defn.keyword]
+          result = arg_defn.type.validate_input(value, context)
+          if !result.valid?
+            raise InvalidArgumentError, "@#{graphql_name}.#{arg_defn.graphql_name} on #{owner.path} is invalid (#{value.inspect}): #{result.problems.first["explanation"]}"
+          end
+        end
+        self.class.validate!(arguments, context)
+        @arguments = self.class.coerce_arguments(nil, arguments, context)
+        if @arguments.is_a?(GraphQL::ExecutionError)
+          raise @arguments
+        end
       end
 
       def graphql_name

lib/graphql/schema/input_object.rb

@@ -182,6 +182,10 @@ def validate_non_null_input(input, ctx, max_errors: nil)
 
           input.each do |argument_name, value|
             argument = types.argument(self, argument_name)
+            if argument.nil? && ctx.is_a?(Query::NullContext) && argument_name.is_a?(Symbol)
+              # Validating definition directive arguments which come in as Symbols
+              argument = types.arguments(self).find { |arg| arg.keyword == argument_name }
+            end
             # Items in the input that are unexpected
             if argument.nil?
               result ||= Query::InputValidationResult.new

spec/graphql/schema/directive_spec.rb

@@ -55,7 +55,7 @@ class Thing < GraphQL::Schema::Object
   end
 
   it "validates arguments" do
-    err = assert_raises ArgumentError do
+    err = assert_raises GraphQL::Schema::Directive::InvalidArgumentError do
       GraphQL::Schema::Field.from_options(
         name: :something,
         type: String,
@@ -65,9 +65,9 @@ class Thing < GraphQL::Schema::Object
       )
     end
 
-    assert_equal "@secret.topSecret is required, but no value was given", err.message
+    assert_equal "@secret.topSecret on Thing.something is invalid (nil): Expected value to not be null", err.message
 
-    err2 = assert_raises ArgumentError do
+    err2 = assert_raises GraphQL::Schema::Directive::InvalidArgumentError do
       GraphQL::Schema::Field.from_options(
         name: :something,
         type: String,
@@ -77,7 +77,7 @@ class Thing < GraphQL::Schema::Object
       )
     end
 
-    assert_equal "@secret.topSecret is required, but no value was given", err2.message
+    assert_equal "@secret.topSecret on Thing.something is invalid (12.5): Could not coerce value 12.5 to Boolean", err2.message
   end
 
   describe 'repeatable directives' do
@@ -400,4 +400,88 @@ def numbers
     enum_value = schema.get_type("Stuff").values["THING"]
     assert_equal [["tag", { name: "t7"}], ["tag", { name: "t8"}]], enum_value.directives.map { |dir| [dir.graphql_name, dir.arguments.to_h] }
   end
+
+  describe "Custom validations on definition directives" do
+    class DirectiveValidationSchema < GraphQL::Schema
+      class ValidatedDirective < GraphQL::Schema::Directive
+        locations OBJECT, FIELD
+        argument :f, Float, required: false, validates: { numericality: { greater_than: 0 } }
+        argument :s, String, required: false, validates: { format: { with: /^[a-z]{3}$/ } }
+        validates required: { one_of: [:f, :s]}
+      end
+
+      class Query < GraphQL::Schema::Object
+        field :i, Int, fallback_value: 100
+      end
+
+      query(Query)
+      directive(ValidatedDirective)
+    end
+
+    it "runs custom validation during execution" do
+      f_err_res = DirectiveValidationSchema.execute("{ i @validatedDirective(f: -10) }")
+      assert_equal [{"message" => "f must be greater than 0", "locations" => [{"line" => 1, "column" => 5}], "path" => ["i"]}], f_err_res["errors"]
+
+      s_err_res = DirectiveValidationSchema.execute("{ i @validatedDirective(s: \"wnrn\") }")
+      assert_equal [{"message" => "s is invalid", "locations" => [{"line" => 1, "column" => 5}], "path" => ["i"]}], s_err_res["errors"]
+
+      f_s_err_res = DirectiveValidationSchema.execute("{ i @validatedDirective }")
+      assert_equal [{"message" => "validatedDirective must include exactly one of the following arguments: f, s.", "locations" => [{"line" => 1, "column" => 5}], "path" => ["i"]}], f_s_err_res["errors"]
+    end
+
+    it "runs custom validation during definition" do
+      obj_type = Class.new(GraphQL::Schema::Object)
+      directive_defn = DirectiveValidationSchema::ValidatedDirective
+      obj_type.directive(directive_defn, f: 1)
+      f_err = assert_raises GraphQL::Schema::Validator::ValidationFailedError do
+        obj_type.directive(directive_defn, f: -1)
+      end
+      assert_equal "f must be greater than 0", f_err.message
+
+      obj_type.directive(directive_defn, s: "abc")
+      s_err = assert_raises GraphQL::Schema::Validator::ValidationFailedError do
+        obj_type.directive(directive_defn, s: "defg")
+      end
+      assert_equal "s is invalid", s_err.message
+
+      required_err = assert_raises GraphQL::Schema::Validator::ValidationFailedError do
+        obj_type.directive(directive_defn)
+      end
+      assert_equal "validatedDirective must include exactly one of the following arguments: f, s.", required_err.message
+    end
+  end
+
+  describe "Validating schema directives" do
+    def build_sdl(size:)
+      <<~GRAPHQL
+        directive @tshirt(size: Size!) on INTERFACE | OBJECT
+
+        type MyType @tshirt(size: #{size}) {
+          color: String
+        }
+
+        type Query {
+          myType: MyType
+        }
+
+        enum Size {
+          LARGE
+          MEDIUM
+          SMALL
+        }
+      GRAPHQL
+    end
+
+    it "Raises a nice error for invalid enum values" do
+      valid_sdl = build_sdl(size: "MEDIUM")
+      assert_equal valid_sdl, GraphQL::Schema.from_definition(valid_sdl).to_definition
+
+      typo_sdl = build_sdl(size: "BLAH")
+      err = assert_raises GraphQL::Schema::Directive::InvalidArgumentError do
+        GraphQL::Schema.from_definition(typo_sdl)
+      end
+      expected_msg = '@tshirt.size on MyType is invalid ("BLAH"): Expected "BLAH" to be one of: LARGE, MEDIUM, SMALL'
+      assert_equal expected_msg, err.message
+    end
+  end
 end