Add spec for better loads: dataloading, prototype new loads code

lib/graphql.rb

@@ -72,7 +72,14 @@ def self.scan_with_ruby(graphql_string)
     GraphQL::Language::Lexer.tokenize(graphql_string)
   end
 
-  NOT_CONFIGURED = Object.new.freeze
+  NOT_CONFIGURED = Object.new
+  def NOT_CONFIGURED.to_s
+    "<GraphQL::NOT_CONFIGURED>"
+  end
+  def NOT_CONFIGURED.inspect
+    "<GraphQL::NOT_CONFIGURED>"
+  end
+  NOT_CONFIGURED.freeze
   private_constant :NOT_CONFIGURED
   module EmptyObjects
     EMPTY_HASH = {}.freeze

lib/graphql/dataloader/null_dataloader.rb

@@ -48,8 +48,10 @@ def run_isolated
 
       def clear_cache; end
 
-      def yield(_source)
-        raise GraphQL::Error, "GraphQL::Dataloader is not running -- add `use GraphQL::Dataloader` to your schema to use Dataloader sources."
+      def yield(_source = nil)
+        run
+        # TODO solve this problem another way
+        # raise GraphQL::Error, "GraphQL::Dataloader is not running -- add `use GraphQL::Dataloader` to your schema to use Dataloader sources."
       end
 
       def append_job(callable = nil)

lib/graphql/execution/interpreter/argument_value.rb

@@ -6,15 +6,93 @@ class Interpreter
       # 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:, original_value:, default_used:)
+        def initialize(definition:, value:, original_value:, default_used:, ast_node:)
+          @arguments = nil
           @definition = definition
           @value = value
           @original_value = original_value
           @default_used = default_used
+          @ast_node = ast_node
+          @state = :initialized
         end
 
-        # @return [Object] The Ruby-ready value for this Argument
-        attr_reader :value
+        attr_writer :arguments
+
+        # Lazy api tODO conflicts with old attr_reader :value
+        def value
+          if @arguments.context&.schema&.lazy?(@value) # TODO InputObject still calls coerce_arguments which doesn't initialize this
+            @value = @arguments.context.schema.sync_lazy(@value)
+            @arguments.context.dataloader.append_job(self)
+          end
+          @value
+        rescue GraphQL::UnauthorizedError => err
+          @state = :finished
+          context = @arguments.context
+          @value = context.schema.unauthorized_object(err)
+        rescue GraphQL::ExecutionError => exec_err
+          @state = :finished
+          context = @arguments.context
+          exec_err.path ||= context.current_path
+          exec_err.ast_node ||= @ast_node
+          @value = exec_err
+        rescue StandardError => err
+          @state = :finished
+          context = @arguments.context
+          @value = context.schema.handle_or_reraise(context, err)
+        end
+
+        # @private implements Dataloader API
+        def call
+          context = @arguments.context
+          case @state
+          when :initialized
+            @value = definition.type.coerce_input(@original_value, context)
+            @state = :coerced
+          when :coerced
+            @value = definition.prepare_value(@arguments.parent_object, @value, context: context)
+            @state = :prepared
+          when :prepared
+            if definition.loads && !definition.from_resolver?
+              @value = definition.load_and_authorize_value(definition.owner, @value, context)
+              while @value.is_a?(Array) && @value.any? { |v| NOT_CONFIGURED.equal?(v) }
+                @arguments.context.dataloader.yield # TODO hack to wait for other work to finish
+              end
+            end
+            @state = :finished
+          end
+
+          if context.schema.lazy?(@value) # TODO use runtime cached version
+            context.dataloader.lazy_at_depth(context[:current_result].depth, self)
+          elsif @state != :finished
+            # TODO non-recursive
+            call
+          end
+        rescue GraphQL::UnauthorizedError => err
+          @state = :finished
+          context = @arguments.context
+          @value = context.schema.unauthorized_object(err)
+        rescue GraphQL::ExecutionError => exec_err
+          @state = :finished
+          exec_err.path ||= context.current_path
+          exec_err.ast_node ||= @ast_node
+          @value = exec_err
+        rescue StandardError => err
+          @state = :finished
+          context = @arguments.context
+          @value = context.schema.handle_or_reraise(context, err)
+        end
+
+        def completed?
+          @state == :finished
+        end
+
+        def errored?
+          @value.is_a?(StandardError)
+        end
+
+        attr_reader :state
+        # # @return [Object] The Ruby-ready value for this Argument
+        # attr_reader :value
 
         # @return [Object] The value of this argument _before_ `prepare` is applied.
         attr_reader :original_value

lib/graphql/execution/interpreter/arguments.rb

@@ -13,18 +13,20 @@ class Arguments
         extend Forwardable
         include GraphQL::Dig
 
-        # The Ruby-style arguments hash, ready for a resolver.
-        # This hash is the one used at runtime.
-        #
-        # @return [Hash<Symbol, Object>]
-        attr_reader :keyword_arguments
+        # @return [GraphQL::Query::Context]
+        attr_reader :context
+
+        # @return [Object, nil]
+        attr_reader :parent_object
 
         # @param argument_values [nil, Hash{Symbol => ArgumentValue}]
         # @param keyword_arguments [nil, Hash{Symbol => Object}]
-        def initialize(keyword_arguments: nil, argument_values:)
+        def initialize(keyword_arguments: nil, parent_object: nil, context: nil, argument_values:)
           @empty = argument_values.nil? || argument_values.empty?
+          @context = context
+          @parent_object = parent_object
           # This is only present when `extras` have been merged in:
-          if keyword_arguments
+          @keyword_arguments = if keyword_arguments
             # This is a little crazy. We expect the `:argument_details` extra to _include extras_,
             # but the object isn't created until _after_ extras are put together.
             # So, we have to use a special flag here to say, "at the last minute, add yourself to the keyword args."
@@ -38,18 +40,15 @@ def initialize(keyword_arguments: nil, argument_values:)
             if keyword_arguments[:argument_details] == :__arguments_add_self
               keyword_arguments[:argument_details] = self
             end
-            @keyword_arguments = keyword_arguments.freeze
-          elsif !@empty
-            @keyword_arguments = {}
-            argument_values.each do |name, arg_val|
-              @keyword_arguments[name] = arg_val.value
-            end
-            @keyword_arguments.freeze
+            keyword_arguments.freeze
+          elsif @empty
+            NO_ARGS
           else
-            @keyword_arguments = NO_ARGS
+            nil
           end
           @argument_values = argument_values ? argument_values.freeze : NO_ARGS
-          freeze
+          @argument_values.each_value { |v| v.arguments = self }
+          # freeze TODO put this call elsewhere?
         end
 
         # @return [Hash{Symbol => ArgumentValue}]
@@ -59,6 +58,22 @@ def empty?
           @empty
         end
 
+        # The Ruby-style arguments hash, ready for a resolver.
+        # This hash is the one used at runtime.
+        #
+        # @return [Hash<Symbol, Object>]
+        def keyword_arguments
+          kws = {}
+          argument_values.each do |name, arg_val|
+            kws[name] = arg_val.value
+          end
+          if @keyword_arguments
+            @keyword_arguments.merge(kws)
+          else
+            kws
+          end
+        end
+
         def_delegators :keyword_arguments, :key?, :[], :fetch, :keys, :each, :values, :size, :to_h
         def_delegators :argument_values, :each_value
 

lib/graphql/execution/interpreter/runtime.rb

@@ -362,14 +362,36 @@ def evaluate_selection(result_name, field_ast_nodes_or_ast_node, selections_resu
               evaluate_selection_with_args(resolved_arguments, field_defn, ast_node, field_ast_nodes, owner_object, result_name, selections_result, runtime_state)
             end
           else
-            @query.arguments_cache.dataload_for(ast_node, field_defn, owner_object) do |resolved_arguments|
+            args_hash = GraphQL::Execution::Interpreter::ArgumentsCache.prepare_args_hash(query, ast_node)
+            arguments = field_defn.create_runtime_arguments(owner_object, args_hash, context, ast_node)
+            arguments.argument_values.each_value do |arg_value|
+              @dataloader.append_job(arg_value)
+            end
+
+            @dataloader.append_job {
+              while !arguments.argument_values.each_value.all?(&:completed?)
+                @dataloader.yield # TODO this is a hack to let those finish first
+              end
+
+              if (first_error = arguments.argument_values.each_value.find(&:errored?))
+                arguments = first_error.value
+              end
               runtime_state = get_current_runtime_state # This might be in a different fiber
               runtime_state.current_field = field_defn
-              runtime_state.current_arguments = resolved_arguments
+              runtime_state.current_arguments = arguments
               runtime_state.current_result_name = result_name
               runtime_state.current_result = selections_result
-              evaluate_selection_with_args(resolved_arguments, field_defn, ast_node, field_ast_nodes, owner_object, result_name, selections_result, runtime_state)
-            end
+              evaluate_selection_with_args(arguments, field_defn, ast_node, field_ast_nodes, owner_object, result_name, selections_result, runtime_state)
+            }
+
+            # @query.arguments_cache.dataload_for(ast_node, field_defn, owner_object) do |resolved_arguments|
+            #   runtime_state = get_current_runtime_state # This might be in a different fiber
+            #   runtime_state.current_field = field_defn
+            #   runtime_state.current_arguments = resolved_arguments
+            #   runtime_state.current_result_name = result_name
+            #   runtime_state.current_result = selections_result
+            #   evaluate_selection_with_args(resolved_arguments, field_defn, ast_node, field_ast_nodes, owner_object, result_name, selections_result, runtime_state)
+            # end
           end
         end
 

lib/graphql/query/context.rb

@@ -84,7 +84,7 @@ def types
 
       attr_writer :types
 
-      RUNTIME_METADATA_KEYS = Set.new([:current_object, :current_arguments, :current_field, :current_path]).freeze
+      RUNTIME_METADATA_KEYS = Set.new([:current_object, :current_arguments, :current_field, :current_path, :current_result]).freeze
       # @!method []=(key, value)
       #   Reassign `key` to the hash passed to {Schema#execute} as `context:`
 

lib/graphql/schema/argument.rb

@@ -315,6 +315,7 @@ def coerce_into_values(parent_object, values, context, argument_values)
               original_value: resolved_coerced_value,
               definition: self,
               default_used: default_used,
+              ast_node: nil,
             )
           end
         end
@@ -354,15 +355,13 @@ def load_and_authorize_value(load_method_owner, coerced_value, context)
         elsif loads
           if type.list?
             loaded_values = []
-            # We want to run these list items all together,
-            # but we also need to wait for the result so we can return it :S
-            context.dataloader.run_isolated do
-              coerced_value.each_with_index { |val, idx|
-                context.dataloader.append_job do
-                  loaded_values[idx] = load_method_owner.load_and_authorize_application_object(self, val, context)
-                end
-              }
-            end
+            coerced_value.each_with_index { |val, idx|
+              loaded_values[idx] = NOT_CONFIGURED
+              context.dataloader.append_job do
+                loaded_values[idx] = load_method_owner.load_and_authorize_application_object(self, val, context)
+              end
+            }
+
             context.schema.after_any_lazies(loaded_values, &:itself)
           else
             load_method_owner.load_and_authorize_application_object(self, coerced_value, context)

lib/graphql/schema/member/has_arguments.rb

@@ -247,6 +247,47 @@ def argument_class(new_arg_class = nil)
           self.class.argument_class(new_arg_class)
         end
 
+        def create_runtime_arguments(parent_object, values, context, ast_node)
+          argument_values = nil
+          arg_defns = context.types.arguments(self)
+          arg_defns.each do |argument_defn|
+            default_used = false
+            value = if values.key?(argument_defn.graphql_name)
+              values[argument_defn.graphql_name]
+            # elsif values.key?(argument_defn.keyword) TODO can I not need this?
+            elsif argument_defn.default_value?
+              default_used = true
+              argument_defn.default_value
+            else
+              next
+            end
+
+            if value.nil? && argument_defn.replace_null_with_default?
+              value = argument_defn.default_value
+              default_used = true
+            end
+
+            argument_values ||= {}
+            argument_values[argument_defn.keyword] = GraphQL::Execution::Interpreter::ArgumentValue.new(
+              value: NOT_CONFIGURED,
+              original_value: value,
+              definition: argument_defn,
+              default_used: default_used,
+              ast_node: ast_node,
+            )
+          end
+
+          if argument_values
+            Execution::Interpreter::Arguments.new(
+              context: context,
+              parent_object: parent_object,
+              argument_values: argument_values
+            )
+          else
+            Execution::Interpreter::Arguments::EMPTY
+          end
+        end
+
         # @api private
         # If given a block, it will eventually yield the loaded args to the block.
         #

spec/graphql/dataloader_spec.rb

@@ -934,6 +934,17 @@ def self.included(child_class)
           context = { batched_calls_counter: BatchedCallsCounter.new }
           schema.execute(query_str, context: context)
           assert_equal 1, context[:batched_calls_counter].count
+
+          database_log.clear
+          query_str = "{ r1: recipe(id: 5) { name } recipesById(ids: [6]) { name } }"
+          context = { batched_calls_counter: BatchedCallsCounter.new }
+          result = schema.execute(query_str, context: context)
+          pp result.to_h
+          pp database_log
+          assert_graphql_equal({ "r1" => {"name" => "Cornbread" }, "recipesById" => [ { "name" => "Grits"}]}, result["data"])
+          assert_equal 1, context[:batched_calls_counter].count
+          expected_log = [[:mget, ["5", "6"]]]
+          assert_equal expected_log, database_log
         end
 
         it "batches nested object calls in .authorized? after using lazy_resolve" do

spec/graphql/schema/argument_spec.rb

@@ -226,7 +226,7 @@ def self.resolve_type(type, obj, ctx)
       GRAPHQL
 
       res = SchemaArgumentTest::Schema.execute(query_str, context: {multiply_by: 3})
-      assert_equal({ 'f1' => {arg: "echo", required_with_default_arg: 1}.inspect, 'f2' => nil }, res['data'])
+      assert_equal({ 'f1' => {arg: "echo", required_with_default_arg: 1}.inspect, 'f2' => {required_with_default_arg: 1, unauthorized_prepared_arg: nil}.inspect }, res['data'])
       assert_nil(res['errors'])
     end
   end
@@ -340,7 +340,7 @@ def self.resolve_type(type, obj, ctx)
 
       res = SchemaArgumentTest::Schema.execute(query_str)
       assert_nil res["errors"]
-      assert_nil res["data"].fetch("field")
+      assert_equal({required_with_default_arg: 1, unauthorized_instrument: nil}.inspect, res["data"].fetch("field"))
     end
 
     it "handles applies authorization even when a custom load method is provided" do