Merge pull request #2416 from modosc/2356-required-default-values

Support required arguments with default values [fixes #2356]

Author: rmosolgo

lib/graphql/schema/validation.rb

@@ -104,10 +104,6 @@ def self.assert_named_items_are_valid(item_name, get_items_proc)
         ARGUMENTS_ARE_VALID =  Rules.assert_named_items_are_valid("argument", ->(type) { type.arguments.values })
 
         DEFAULT_VALUE_IS_VALID_FOR_TYPE = ->(type) {
-          if !type.default_value.nil? && type.type.is_a?(NonNullType)
-            return %Q(Variable #{type.name} of type "#{type.type}" is required and will not use the default value. Perhaps you meant to use type "#{type.type.of_type}".)
-          end
-
           if !type.default_value.nil?
             coerced_value = begin
               type.type.coerce_isolated_result(type.default_value)

lib/graphql/static_validation/rules/required_arguments_are_present.rb

@@ -18,7 +18,7 @@ def on_directive(node, _parent)
       def assert_required_args(ast_node, defn)
         present_argument_names = ast_node.arguments.map(&:name)
         required_argument_names = context.warden.arguments(defn)
-          .select { |a| a.type.kind.non_null? }
+          .select { |a| a.type.kind.non_null? && !a.default_value? }
           .map(&:name)
 
         missing_names = required_argument_names - present_argument_names

spec/graphql/schema/argument_spec.rb

@@ -10,7 +10,7 @@ class Query < GraphQL::Schema::Object
         argument :arg_with_block, String, required: false do
           description "test"
         end
-
+        argument :required_with_default_arg, Int, required: true, default_value: 1
         argument :aliased_arg, String, required: false, as: :renamed
         argument :prepared_arg, Int, required: false, prepare: :multiply
         argument :prepared_by_proc_arg, Int, required: false, prepare: ->(val, context) { context[:multiply_by] * val }
@@ -30,7 +30,11 @@ def call(val, context)
       end
 
       def field(**args)
-        args.inspect
+        # sort the fields so that they match the output of the new interpreter
+        sorted_keys = args.keys.sort
+        sorted_args = {}
+        sorted_keys.each  {|k| sorted_args[k] = args[k] }
+        sorted_args.inspect
       end
 
       def multiply(val)
@@ -48,7 +52,7 @@ class Schema < GraphQL::Schema
 
   describe "#keys" do
     it "is not overwritten by the 'keys' argument" do
-      expected_keys = ["aliasedArg", "arg", "argWithBlock", "explodingPreparedArg", "keys", "preparedArg", "preparedByCallableArg", "preparedByProcArg"]
+      expected_keys = ["aliasedArg", "arg", "argWithBlock", "explodingPreparedArg", "keys", "preparedArg", "preparedByCallableArg", "preparedByProcArg", "requiredWithDefaultArg"]
       assert_equal expected_keys, SchemaArgumentTest::Query.fields["field"].arguments.keys.sort
     end
   end
@@ -103,7 +107,7 @@ class Schema < GraphQL::Schema
 
       res = SchemaArgumentTest::Schema.execute(query_str)
       # Make sure it's getting the renamed symbol:
-      assert_equal '{:renamed=>"x"}', res["data"]["field"]
+      assert_equal '{:renamed=>"x", :required_with_default_arg=>1}', res["data"]["field"]
     end
   end
 
@@ -115,7 +119,7 @@ class Schema < GraphQL::Schema
 
       res = SchemaArgumentTest::Schema.execute(query_str, context: {multiply_by: 3})
       # Make sure it's getting the renamed symbol:
-      assert_equal '{:prepared_arg=>15}', res["data"]["field"]
+      assert_equal '{:prepared_arg=>15, :required_with_default_arg=>1}', res["data"]["field"]
     end
 
     it "calls the method on the provided Proc" do
@@ -125,7 +129,7 @@ class Schema < GraphQL::Schema
 
       res = SchemaArgumentTest::Schema.execute(query_str, context: {multiply_by: 3})
       # Make sure it's getting the renamed symbol:
-      assert_equal '{:prepared_by_proc_arg=>15}', res["data"]["field"]
+      assert_equal '{:prepared_by_proc_arg=>15, :required_with_default_arg=>1}', res["data"]["field"]
     end
 
     it "calls the method on the provided callable object" do
@@ -135,7 +139,7 @@ class Schema < GraphQL::Schema
 
       res = SchemaArgumentTest::Schema.execute(query_str, context: {multiply_by: 3})
       # Make sure it's getting the renamed symbol:
-      assert_equal '{:prepared_by_callable_arg=>15}', res["data"]["field"]
+      assert_equal '{:prepared_by_callable_arg=>15, :required_with_default_arg=>1}', res["data"]["field"]
     end
 
     it "handles exceptions raised by prepare" do
@@ -144,9 +148,39 @@ class Schema < GraphQL::Schema
       GRAPHQL
 
       res = SchemaArgumentTest::Schema.execute(query_str, context: {multiply_by: 3})
-      assert_equal({ 'f1' => '{:arg=>"echo"}', 'f2' => nil }, res['data'])
+      assert_equal({ 'f1' => '{:arg=>"echo", :required_with_default_arg=>1}', 'f2' => nil }, res['data'])
       assert_equal(res['errors'][0]['message'], 'boom!')
       assert_equal(res['errors'][0]['path'], ['f2'])
     end
   end
+
+  describe "default_value:" do
+    it 'uses default_value: with no input' do
+      query_str = <<-GRAPHQL
+      { field() }
+      GRAPHQL
+
+      res = SchemaArgumentTest::Schema.execute(query_str)
+      assert_equal '{:required_with_default_arg=>1}', res["data"]["field"]
+    end
+
+    it 'uses provided input value' do
+      query_str = <<-GRAPHQL
+      { field(requiredWithDefaultArg: 2) }
+      GRAPHQL
+
+      res = SchemaArgumentTest::Schema.execute(query_str)
+      assert_equal '{:required_with_default_arg=>2}', res["data"]["field"]
+    end
+
+    it 'respects non-null type' do
+      query_str = <<-GRAPHQL
+      { field(requiredWithDefaultArg: null) }
+      GRAPHQL
+
+      res = SchemaArgumentTest::Schema.execute(query_str)
+      assert_equal "Argument 'requiredWithDefaultArg' on Field 'field' has an invalid value. Expected type 'Int!'.", res['errors'][0]['message']
+    end
+
+  end
 end

spec/graphql/schema/validation_spec.rb

@@ -297,9 +297,9 @@ def assert_validation_warns(object, warning)
       end
     }
 
-    let(:invalid_default_argument_for_non_null_argument) {
+    let(:default_argument_for_non_null_argument) {
       GraphQL::Argument.define do
-        name "InvalidDefault"
+        name "ValidDefault"
         type !GraphQL::INT_TYPE
         default_value 1
       end
@@ -324,8 +324,8 @@ def assert_validation_warns(object, warning)
       assert_error_includes untyped_argument, "must be a valid input type (Scalar or InputObject), not Symbol"
     end
 
-    it "does not allow default values for non-null argument" do
-      assert_error_includes invalid_default_argument_for_non_null_argument, 'Variable InvalidDefault of type "Int!" is required and will not use the default value. Perhaps you meant to use type "Int".'
+    it "allows default values for non-null argument" do
+      assert_nil GraphQL::Schema::Validation.validate(default_argument_for_non_null_argument)
     end
 
     it "cannot use reserved name" do