Properly validate SDL directive arguments

Author: rmosolgo

lib/graphql/schema/directive.rb

@@ -111,6 +111,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,6 +122,14 @@ def initialize(owner, **arguments)
         # - lazy resolution
         # Probably, those won't be needed here, since these are configuration arguments,
         # not runtime arguments.
+        self.class.all_argument_definitions.each do |arg_defn|
+          value = arguments[arg_defn.keyword]
+          arg_type = arg_defn.type
+          result = arg_defn.type.validate_input(value, Query::NullContext.instance)
+          if !result.valid?
+            raise InvalidArgumentError, "@#{graphql_name}.#{arg_defn.graphql_name} on #{owner.path} is invalid (#{value.inspect}): #{result.problems.first["explanation"]}"
+          end
+        end
         @arguments = self.class.coerce_arguments(nil, arguments, Query::NullContext.instance)
       end
 

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,38 @@ 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 "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