Merge pull request rmosolgo#1602 from Tapjoy/fix/variable_coercion

Fix scalar coercion errors in variable values (including defaults) not being handled

Author: Robert Mosolgo

lib/graphql/query/variables.rb

@@ -30,7 +30,13 @@ def initialize(ctx, ast_variables, provided_variables)
             provided_value = @provided_variables[variable_name]
             value_was_provided =  @provided_variables.key?(variable_name)
 
-            validation_result = variable_type.validate_input(provided_value, ctx)
+            begin
+              validation_result = variable_type.validate_input(provided_value, ctx)
+            rescue GraphQL::CoercionError => ex
+              validation_result = GraphQL::Query::InputValidationResult.new
+              validation_result.add_problem(ex.message)
+            end
+
             if !validation_result.valid?
               # This finds variables that were required but not provided
               @errors << GraphQL::Query::VariableValidationError.new(ast_variable, variable_type, provided_value, validation_result)

lib/graphql/static_validation/rules/variable_default_values_are_correctly_typed.rb

@@ -20,8 +20,17 @@ def validate_default_value(node, context)
           type = context.schema.type_from_ast(node.type)
           if type.nil?
             # This is handled by another validator
-          elsif !context.valid_literal?(value, type)
-            context.errors << message("Default value for $#{node.name} doesn't match type #{type}", node, context: context)
+          else
+            begin
+              valid = context.valid_literal?(value, type)
+            rescue GraphQL::CoercionError => err
+              error_message = err.message
+            end
+
+            if !valid
+              error_message ||= "Default value for $#{node.name} doesn't match type #{type}"
+              context.errors << message(error_message, node, context: context)
+            end
           end
         end
       end

spec/graphql/query/variables_spec.rb

@@ -110,6 +110,26 @@
           assert_equal expected, variables.errors.first.message
         end
       end
+
+      describe "when provided input cannot be coerced" do
+        let(:query_string) {%|
+        query searchMyDairy (
+          $time: Time
+        ) {
+          searchDairy(expiresAfter: $time) {
+            ... on Cheese {
+              flavor
+            }
+          }
+        }
+        |}
+        let(:provided_variables) { { "time" => "a" } }
+
+        it "validates invalid input objects" do
+          expected = "Variable time of type Time was provided invalid value"
+          assert_equal expected, variables.errors.first.message
+        end
+      end
     end
 
     describe "nullable variables" do

spec/graphql/static_validation/rules/variable_default_values_are_correctly_typed_spec.rb

@@ -134,4 +134,56 @@
       end
     end
   end
+
+  describe "custom error messages" do
+    let(:schema) {
+      TimeType = GraphQL::ScalarType.define do
+        name "Time"
+        description "Time since epoch in seconds"
+
+        coerce_input ->(value, ctx) do
+          begin
+            Time.at(Float(value))
+          rescue ArgumentError
+            raise GraphQL::CoercionError, 'cannot coerce to Float'
+          end
+        end
+
+        coerce_result ->(value, ctx) { value.to_f }
+      end
+
+      QueryType = GraphQL::ObjectType.define do
+        name "Query"
+        description "The query root of this schema"
+
+        field :time do
+          type TimeType
+          argument :value, !TimeType
+          resolve ->(obj, args, ctx) { args[:value] }
+        end
+      end
+
+      GraphQL::Schema.define do
+        query QueryType
+      end
+    }
+
+    let(:query_string) {%|
+      query(
+        $value: Time = "a"
+      ) {
+        time(value: $value)
+      }
+    |}
+
+    it "sets error message from a CoercionError if raised" do
+      assert_equal 1, errors.length
+
+      assert_includes errors, {
+        "message"=> "cannot coerce to Float",
+        "locations"=>[{"line"=>3, "column"=>9}],
+        "fields"=>["query"]
+      }
+    end
+  end
 end

spec/support/dummy/schema.rb

@@ -248,6 +248,21 @@ class NoSuchDairyError < StandardError; end
     end
   end
 
+  TimeType = GraphQL::ScalarType.define do
+    name "Time"
+    description "Time since epoch in seconds"
+
+    coerce_input ->(value, ctx) do
+      begin
+        Time.at(Float(value))
+      rescue ArgumentError
+        raise GraphQL::CoercionError, 'cannot coerce to Float'
+      end
+    end
+
+    coerce_result ->(value, ctx) { value.to_f }
+  end
+
   class FetchItem < GraphQL::Function
     attr_reader :type, :description, :arguments
 
@@ -312,6 +327,7 @@ def call(obj, args, ctx)
       type !DairyProductUnion
       # This is a list just for testing 😬
       argument :product, types[DairyProductInputType], default_value: [{"source" => "SHEEP"}]
+      argument :expiresAfter, TimeType
       resolve ->(t, args, c) {
         source = args["product"][0][:source] # String or Sym is ok
         products = CHEESES.values + MILKS.values