GraphQL::Analysis::AST::FieldUsage deprecated argument detections for mutations raises an error when the InputType value is prepared

[jrichardlai]

Describe the bug

GraphQL::Analysis::AST::FieldUsage deprecated argument detections for mutations is raising an error when the InputType value is prepared.

Versions

graphql version: 2.2.11

Steps to reproduce

diff --git a/spec/graphql/analysis/ast/field_usage_spec.rb b/spec/graphql/analysis/ast/field_usage_spec.rb
index 6e54ac38d..75e5dd635 100644
--- a/spec/graphql/analysis/ast/field_usage_spec.rb
+++ b/spec/graphql/analysis/ast/field_usage_spec.rb
@@ -254,6 +254,17 @@ describe GraphQL::Analysis::AST::FieldUsage do
     end
   end

+  describe "mutation with deprecated arguments with prepared values" do
+    let(:query_string) {%|
+      mutation {
+        pushValue(preparedTestInput: { deprecatedDate: "2020-10-10" })
+      }
+    |}
+
+    it "keeps track of nested deprecated arguments" do
+      assert_equal ['PreparedDateInput.deprecatedDate'], result[:used_deprecated_arguments]
+    end
+  end

   describe "when an argument prepare raises a GraphQL::ExecutionError" do
     class ArgumentErrorFieldUsageSchema < GraphQL::Schema
diff --git a/spec/support/dummy/schema.rb b/spec/support/dummy/schema.rb
index 206142cdb..d88b2d2ab 100644
--- a/spec/support/dummy/schema.rb
+++ b/spec/support/dummy/schema.rb
@@ -265,6 +265,18 @@ module Dummy
     argument :old_source, String, required: false, deprecation_reason: "No longer supported"
   end

+  class PreparedDateInput < BaseInputObject
+    description "Input with prepared value"
+    argument :date, String, description: "date as a string", required: false
+    argument :deprecated_date, String, description: "date as a string", required: false, deprecation_reason: "Use date"
+
+    def prepare
+      return nil unless date || deprecated_date
+
+      Date.parse(date || deprecated_date)
+    end
+  end
+
   class DeepNonNull < BaseObject
     field :non_null_int, Integer, null: false do
       argument :returning, Integer, required: false
@@ -490,6 +502,7 @@ module Dummy
     field :push_value, [Integer], null: false, description: "Push a value onto a global array :D" do
       argument :value, Integer, as: :val
       argument :deprecated_test_input, DairyProductInput, required: false
+      argument :prepared_test_input, PreparedDateInput, required: false
     end
     def push_value(val:)
       GLOBAL_VALUES << val

Expected behavior

GraphQL::Analysis::AST::FieldUsage can detect deprecated arguments within input objects in a mutation when there is a prepared field.

Actual behavior

Error is returned when an InputType argument is prepared

Click to view exception backtrace

Error:
GraphQL::Analysis::AST::FieldUsage::mutation with deprecated arguments with prepared values#test_0001_keeps track of nested deprecated arguments:
NoMethodError: undefined method `arguments' for #<Date: 2020-10-10 ((2459133j,0s,0n),+0s,2299161j)>
    lib/graphql/analysis/ast/field_usage.rb:52:in `block in extract_deprecated_arguments'
    lib/graphql/analysis/ast/field_usage.rb:39:in `each_pair'
    lib/graphql/analysis/ast/field_usage.rb:39:in `extract_deprecated_arguments'
    lib/graphql/analysis/ast/field_usage.rb:23:in `on_leave_field'
    lib/graphql/analysis/ast/visitor.rb:85:in `block in call_on_leave_field'

Additional context

I was looking on how to fix it, however I was not able to find a way to get the unprepared arguments values from GraphQL::Execution::Interpreter::ArgumentValue. Is there a way to do so?

pry(#<GraphQL::Analysis::AST::FieldUsage>)> argument
=> #<GraphQL::Execution::Interpreter::ArgumentValue:0x000000010651f750 @default_used=false, @definition=#<GraphQL::Schema::Argument Mutation.pushValue.preparedTestInput: PreparedDateInput>, @value=#<Date: 2020-10-10 ((2459133j,0s,0n),+0s,2299161j)>>

[rmosolgo]

Hey, thanks for reporting this and sorry for the trouble. I don't know of a better fix than the guideline-tech#1, because as you noticed, the un-prepared value isn't available.

It might be possible to add an original_value method to ArgumentValue. We could retain a reference to it from these two places that prepare_value is called:

graphql-ruby/lib/graphql/schema/input_object.rb

Lines 29 to 34 in 1c4b1dc

	# Weirdly, procs are applied during coercion, but not methods.
	# Probably because these methods require a `self`.
	if arg_defn.prepare.is_a?(Symbol) || context.nil?
	prepared_value = arg_defn.prepare_value(self, @ruby_style_hash[ruby_kwargs_key])
	overwrite_argument(ruby_kwargs_key, prepared_value)
	end

graphql-ruby/lib/graphql/schema/argument.rb

Lines 274 to 279 in 1c4b1dc

	prepared_value = begin
	prepare_value(parent_object, resolved_coerced_value, context: context)
	rescue StandardError => err
	context.schema.handle_or_reraise(context, err)
	end

[jrichardlai]

Hello, I probably misunderstood what you meant here, but when I tried getting the value from prepare_value it seems the value was already prepared ( converted to the Date ) and I was not able to use it as an argument, were you thinking of using the Hash value as original_value and convert it to ArgumentValue in FieldUsage?

Otherwise the value is already prepared as Date from

graphql-ruby/lib/graphql/schema/input_object.rb

Lines 218 to 219 in 66b5720

	input_obj_instance = self.new(resolved_arguments, ruby_kwargs: resolved_arguments.keyword_arguments, context: ctx, defaults_used: nil)
	input_obj_instance.prepare

Which is called in

graphql-ruby/lib/graphql/schema/argument.rb

Lines 264 to 265 in 1c4b1dc

	coerced_value = begin
	type.coerce_input(value, context)

I was able to get it working, but the solution seems dirty, not sure if I should add a method to get the unprepared value from the raw not-coerced value.

guideline-tech@29bf573

diff --git a/lib/graphql/analysis/ast/field_usage.rb b/lib/graphql/analysis/ast/field_usage.rb
index dc83577dbd..185b9a8f9b 100644
--- a/lib/graphql/analysis/ast/field_usage.rb
+++ b/lib/graphql/analysis/ast/field_usage.rb
@@ -48,8 +48,8 @@ module GraphQL
               argument_type = argument_type.of_type
             end

-            if argument_type.kind.input_object? && argument.value.respond_to?(:arguments) # Skip if value is not a GraphQL::Schema::InputObject
-              extract_deprecated_arguments(argument.value.arguments.argument_values) # rubocop:disable Development/ContextIsPassedCop -- runtime args instance
+            if argument_type.kind.input_object?
+              extract_deprecated_arguments(argument.original_value.arguments.argument_values) # rubocop:disable Development/ContextIsPassedCop -- runtime args instance
             elsif argument_type.kind.enum?
               extract_deprecated_enum_value(argument_type, argument.value)
             elsif argument_type.list?
diff --git a/lib/graphql/execution/interpreter/argument_value.rb b/lib/graphql/execution/interpreter/argument_value.rb
index 4ca37977c0..82d31f3804 100644
--- a/lib/graphql/execution/interpreter/argument_value.rb
+++ b/lib/graphql/execution/interpreter/argument_value.rb
@@ -6,15 +6,19 @@ module GraphQL
       # A container for metadata regarding arguments present in a GraphQL query.
       # @see Interpreter::Arguments#argument_values for a hash of these objects.
       class ArgumentValue
-        def initialize(definition:, value:, default_used:)
+        def initialize(definition:, value:, original_value:, default_used:)
           @definition = definition
           @value = value
+          @original_value = original_value
           @default_used = default_used
         end

         # @return [Object] The Ruby-ready value for this Argument
         attr_reader :value

+        # @return [Object] The original Ruby-ready value for this Argument
+        attr_reader :original_value
+
         # @return [GraphQL::Schema::Argument] The definition instance for this argument
         attr_reader :definition

diff --git a/lib/graphql/schema/argument.rb b/lib/graphql/schema/argument.rb
index 633f329b65..dfa85a69a2 100644
--- a/lib/graphql/schema/argument.rb
+++ b/lib/graphql/schema/argument.rb
@@ -261,8 +261,11 @@ module GraphQL
         end

         loaded_value = nil
+        original_value = nil
         coerced_value = begin
-          type.coerce_input(value, context)
+          type.coerce_input(value, context) do |input_obj_instance|
+            original_value = input_obj_instance
+          end
         rescue StandardError => err
           context.schema.handle_or_reraise(context, err)
         end
@@ -290,6 +293,7 @@ module GraphQL
             # TODO code smell to access such a deeply-nested constant in a distant module
             argument_values[arg_key] = GraphQL::Execution::Interpreter::ArgumentValue.new(
               value: resolved_loaded_value,
+              original_value: original_value,
               definition: self,
               default_used: default_used,
             )
diff --git a/lib/graphql/schema/input_object.rb b/lib/graphql/schema/input_object.rb
index 46c69c1826..856eae5ef5 100644
--- a/lib/graphql/schema/input_object.rb
+++ b/lib/graphql/schema/input_object.rb
@@ -216,6 +216,7 @@ module GraphQL
               raise resolved_arguments
             else
               input_obj_instance = self.new(resolved_arguments, ruby_kwargs: resolved_arguments.keyword_arguments, context: ctx, defaults_used: nil)
+              yield input_obj_instance if block_given?
               input_obj_instance.prepare
             end
           end
diff --git a/spec/graphql/analysis/ast/field_usage_spec.rb b/spec/graphql/analysis/ast/field_usage_spec.rb
index 53cafb4851..56924b3904 100644
--- a/spec/graphql/analysis/ast/field_usage_spec.rb
+++ b/spec/graphql/analysis/ast/field_usage_spec.rb
@@ -254,18 +254,30 @@ describe GraphQL::Analysis::AST::FieldUsage do
     end
   end

-  describe "mutation with deprecated arguments with prepared values does not break" do
+  describe "mutation with deprecated arguments with prepared values" do
     let(:query_string) {%|
       mutation {
-        pushValue(preparedTestInput: { deprecatedDate: "2020-10-10" })
+        pushValue(preparedTestInput: { date: "2020-10-10" })
       }
     |}

-    it "does not keeps track of nested deprecated arguments" do
+    it "does not return non-deprecated arguments" do
       assert_equal [], result[:used_deprecated_arguments]
     end
   end

+  describe "mutation with deprecated arguments with prepared values" do
+    let(:query_string) {%|
+      mutation {
+        pushValue(preparedTestInput: { deprecatedDate: "2020-10-10" })
+      }
+    |}
+
+    it "keeps track of nested deprecated arguments" do
+      assert_equal ['PreparedDateInput.deprecatedDate'], result[:used_deprecated_arguments]
+    end
+  end
+
   describe "when an argument prepare raises a GraphQL::ExecutionError" do
     class ArgumentErrorFieldUsageSchema < GraphQL::Schema
       class FieldUsage < GraphQL::Analysis::AST::FieldUsage

[rmosolgo]

Hey! Sorry it took me a while to come back to this. Thanks for sharing what you found when you investigated that fix. I agree, it didn't turn out as nice as I thought it might...

I reviewed this part of the gem and noticed that input_object.prepare was actually being called twice. First, when incoming GraphQL arguments are being turned into an InputObject instance:

graphql-ruby/lib/graphql/schema/input_object.rb

Lines 218 to 219 in ec1ae00

	input_obj_instance = self.new(resolved_arguments, ruby_kwargs: resolved_arguments.keyword_arguments, context: ctx, defaults_used: nil)
	input_obj_instance.prepare

Then, later, the Argument code calls .prepare again:

graphql-ruby/lib/graphql/schema/argument.rb

Lines 208 to 211 in ec1ae00

	def prepare_value(obj, value, context: nil)
	if value.is_a?(GraphQL::Schema::InputObject)
	value = value.prepare
	end

For normal uses of .prepare, that second call would never happen because the value would no longer be an InputObject instance. But the default implementation of InputObject#prepare is to return itself, and in that case, it was being called twice.

I found that if I removed that first call to .prepare, then it was much easier to capture original_value and pass it along to the ArgumentValue. I took that approach in #4902.