Exec-next: make connections, scoping, and field extensions work

guides/execution/batching.md

@@ -233,29 +233,25 @@ One schema can run _both_ legacy execution and batching execution. This enable a
 
 Performance improvements in batching execution come at the cost of removing support for many "nice-to-have" features in GraphQL-Ruby by default. Those features are addressed here.
 
-### Query Analyzers, including complexity 🌕
+### Query Analyzers, including complexity ✅
 
 Support is identical; this runs before execution using the exact same code.
 
 TODO: accessing loaded arguments inside analzyers may turn out to be slightly different; it still calls legacy code.
 
-### Authorization, Scoping 🌕
+### Authorization, Scoping ✅
 
-Full compatibility, but the internal code which determines _when_ it should be called is still slow and clunky.
-
-- [x] Objects
-- [x] Fields
-- [x] Arguments
-- [x] Resolvers
-- [ ] TODO: improve detection/opt in for this feature
+Full compatibility. `def (self.)authorized?` and `def self.scope_items` will be called as needed during execution.
 
 ### Visibility, including Changesets ✅
 
-Visibility works exactly as before; both runtime modules call the same method to get type information from the schema.
+Visibility works exactly as before; both runtime modules call the same methods to get type information from the schema.
+
+### Dataloader ✅
 
-### Dataloader 🌕
+Dataloader runs with new execution, but batching
 
-Dataloader _works_ but batching behavior is different in some cases. TODO document those cases, consider better future compatibility.
+TODO document those cases, consider better future compatibility.
 
 ### Tracing ✅
 
@@ -273,13 +269,15 @@ Right now, lazy result from `resolve_type` is tied up in authorization compat sh
 
 ### `current_path` ❌
 
-TODO: not supported yet because the new runtime module doesn't actually product `current_path` while it's running. I think it's possible to support it though.
+This is not supported because the new runtime doesn't actually produce `current_path`.
+
+It is theoretically possible to support this but it will be a ton of work. If you use this for core runtime functions, please share your use case in a GitHub issue and we can investigate future options.
 
 ### `@defer` and `@stream` ❌
 
 This depends on `current_path` so isn't possible yet.
 
-### Caching ❌
+### ObjectCache ❌
 
 Actually this probably works but I haven't tested it.
 
@@ -299,17 +297,22 @@ Possible but not implemented. Legacy support is implemented I believe.
 
 Partial support is possible, `obj` will not be given to `validates:` anymore maybe?
 
-### Field Extensions ❌
+### Field Extensions ✅
+
+Field extensions _are_ called, but it uses new methods:
 
-Maybe this will be possible to support but with `objects` instead of `object` given to the hook. Change the hook name to `resolve_batch`?
+- `def resolve_batching(objects:, arguments:, context:, &block)` receives `objects:` instead of `object:` and should yield them to the given block to continue execution
+- `def after_resolve_batching(objects:, arguments:, context:, values:, memo:)` receives `objects:, values:, ...` instead of `object:, value:, ...` and should return an Array of results (isntead of a single result value).
+
+Because of their close integration with the runtime, `ConnectionExtension` and `ScopeExtension` don't actually use `after_resolve_batching`. Instead, support is hard-coded inside the runtime. This might be a smell that field extensions aren't worth supporting.
 
 ### Resolver classes (including Mutations and Subscriptions) ❌
 
 This should be supported somehow; legacy support is present now
 
-### Field `extras:`, including `lookahead` ❌
+### Field `extras:`, including `lookahead` ✅
 
-TODO support here is possible but not implemented. Legacy support is implemented but should be extracted to an opt-in thing.
+`:ast_node` and `:lookahead` are already implemented. Others are possible -- please raise an issue if you need one. `extras: [:current_path]` is not possible.
 
 ### `raw_value` ❌
 
@@ -323,13 +326,9 @@ TODO: support is possible here but not tested
 - raising GraphQL::ExecutionError
 - Schema class error handling hooks
 
-### Connection fields ❌
-
-TODO -- make this better.
-
-Currently, argument definitions _are_ added to the field when a connection type is used as a return type.
+### Connection fields ✅
 
-But arguments are not automatically hidden from the resolver and Connection wrappers are not automatically applied. Should they be?
+Connection arguments are automatically handled and connection wrapper objects are automatically applied to arrays and relations.
 
 ### Custom Introspection ✅
 

lib/graphql/execution/batching/field_compatibility.rb

@@ -57,12 +57,10 @@ def resolve_batch(frs, objects, context, kwargs)
                 if dynamic_introspection
                   obj_inst = @owner.wrap(obj_inst, context)
                 end
-                results << with_extensions(obj_inst, kwargs, context) do |obj, ruby_kwargs|
-                  if ruby_kwargs.empty?
-                    obj.public_send(@resolver_method)
-                  else
-                    obj.public_send(@resolver_method, **ruby_kwargs)
-                  end
+                results << if kwargs.empty?
+                  obj_inst.public_send(@resolver_method)
+                else
+                  obj_inst.public_send(@resolver_method, **kwargs)
                 end
               end
             end
@@ -75,23 +73,20 @@ def resolve_batch(frs, objects, context, kwargs)
               if maybe_err
                 next maybe_err
               end
-              resolver_inst_kwargs = if @resolver_class < Schema::HasSingleInputArgument
+              ruby_kwargs = if @resolver_class < Schema::HasSingleInputArgument
                 resolver_inst_kwargs[:input]
               else
                 resolver_inst_kwargs
               end
-              with_extensions(o, resolver_inst_kwargs, context) do |obj, ruby_kwargs|
-                resolver_inst.object = obj
-                resolver_inst.prepared_arguments = ruby_kwargs
-                is_authed, new_return_value = resolver_inst.authorized?(**ruby_kwargs)
-                if frs.runner.resolves_lazies && frs.runner.schema.lazy?(is_authed)
-                  is_authed, new_return_value = frs.runner.schema.sync_lazy(is_authed)
-                end
-                if is_authed
-                  resolver_inst.call_resolve(ruby_kwargs)
-                else
-                  new_return_value
-                end
+              resolver_inst.prepared_arguments = ruby_kwargs
+              is_authed, new_return_value = resolver_inst.authorized?(**ruby_kwargs)
+              if frs.runner.resolves_lazies && frs.runner.schema.lazy?(is_authed)
+                is_authed, new_return_value = frs.runner.schema.sync_lazy(is_authed)
+              end
+              if is_authed
+                resolver_inst.call_resolve(ruby_kwargs)
+              else
+                new_return_value
               end
             rescue RuntimeError => err
               err
@@ -107,24 +102,7 @@ def resolve_batch(frs, objects, context, kwargs)
           elsif objects.first.is_a?(Interpreter::RawValue)
             objects
           else
-            # need to use connection extension if present, and extensions expect object type instances
-            if extensions.empty?
-              objects.map { |o| o.public_send(@method_sym)}
-            else
-              results = []
-              frs.selections_step.graphql_objects.each_with_index do |obj_inst, idx|
-                if frs.object_is_authorized[idx]
-                  results << with_extensions(obj_inst, EmptyObjects::EMPTY_HASH, context) do |obj, arguments|
-                    if arguments.empty?
-                      obj.object.public_send(@method_sym)
-                    else
-                      obj.object.public_send(@method_sym, **arguments)
-                    end
-                  end
-                end
-              end
-              results
-            end
+            objects.map { |o| o.public_send(@method_sym)}
           end
         end
       end

lib/graphql/execution/batching/field_resolve_step.rb

@@ -20,9 +20,12 @@ def initialize(parent_type:, runner:, key:, selections_step:)
           @static_type = nil
           @next_selections = nil
           @object_is_authorized = nil
+          @finish_extension_idx = nil
+          @was_scoped = nil
         end
 
-        attr_reader :ast_node, :key, :parent_type, :selections_step, :runner, :field_definition, :object_is_authorized, :arguments
+        attr_reader :ast_node, :key, :parent_type, :selections_step, :runner,
+          :field_definition, :object_is_authorized, :arguments, :was_scoped
 
         def path
           @path ||= [*@selections_step.path, @key].freeze
@@ -147,11 +150,20 @@ def sync(lazy)
         def call
           if @enqueued_authorization && @pending_authorize_steps_count == 0
             enqueue_next_steps
+          elsif @finish_extension_idx
+            finish_extensions
           elsif @field_results
             build_results
           else
             execute_field
           end
+        rescue StandardError => err
+          if @field_definition && !err.message.start_with?("Resolving ")
+            # TODO remove this check ^^^^^^ when NullDataloader isn't recursive
+            raise err, "Resolving #{@field_definition.path}: #{err.message}", err.backtrace
+          else
+            raise
+          end
         end
 
         def add_graphql_error(err)
@@ -181,6 +193,7 @@ def execute_field
           end
 
           if @field_definition.dynamic_introspection
+            # TODO break this backwards compat somehow?
             objects = @selections_step.graphql_objects
           end
 
@@ -202,7 +215,7 @@ def execute_field
               end
               @arguments[:ast_node] = ast_node
             else
-              raise ArgumentError, "This extra isn't supported yet: #{extra.inspect}. Open an issue on GraphQL-Ruby to add compatibility for it."
+              raise ArgumentError, "This `extra` isn't supported yet: #{extra.inspect}. Open an issue on GraphQL-Ruby to add compatibility for it."
             end
           end
 
@@ -222,12 +235,40 @@ def execute_field
             @object_is_authorized = AlwaysAuthorized
           end
 
+          if @parent_type.default_relay? && authorized_objects.all? { |o| o.respond_to?(:was_authorized_by_scope_items?) && o.was_authorized_by_scope_items? }
+            @was_scoped = true
+          end
+
           query.current_trace.begin_execute_field(@field_definition, @arguments, authorized_objects, query)
-          @field_results = @field_definition.resolve_batch(self, authorized_objects, ctx, @arguments)
+          has_extensions = @field_definition.extensions.size > 0
+          if has_extensions
+            @extended = GraphQL::Schema::Field::ExtendedState.new(@arguments, authorized_objects)
+            @field_results = @field_definition.run_batching_extensions_before_resolve(authorized_objects, @arguments, ctx, @extended) do |objs, args|
+              if (added_extras = @extended.added_extras)
+                args = args.dup
+                added_extras.each { |e| args.delete(e) }
+              end
+              @field_definition.resolve_batch(self, objs, ctx, args)
+            end
+            @finish_extension_idx = 0
+          else
+            @field_results = @field_definition.resolve_batch(self, authorized_objects, ctx, @arguments)
+          end
+
           query.current_trace.end_execute_field(@field_definition, @arguments, authorized_objects, query, @field_results)
 
+          if any_lazy_results?
+            @runner.dataloader.lazy_at_depth(path.size, self)
+          elsif has_extensions
+            finish_extensions
+          else
+            build_results
+          end
+        end
+
+        def any_lazy_results?
+          lazies = false
           if @runner.resolves_lazies # TODO extract this
-            lazies = false
             # TODO add a per-query cache of `.lazy?`
             @field_results.each do |field_result|
               if @runner.schema.lazy?(field_result)
@@ -244,15 +285,49 @@ def execute_field
                 end
               end
             end
+          end
+          lazies
+        end
 
-            if lazies
-              @runner.dataloader.lazy_at_depth(path.size, self)
+        def finish_extensions
+          ctx = @selections_step.query.context
+          memos = @extended.memos || EmptyObjects::EMPTY_HASH
+          while ext = @field_definition.extensions[@finish_extension_idx]
+            # These two are hardcoded here because of how they need to interact with runtime metadata.
+            # It would probably be better
+            case ext
+            when Schema::Field::ConnectionExtension
+              conns = ctx.schema.connections
+              @field_results = @field_results.map.each_with_index do |value, idx|
+                object = @extended.object[idx]
+                conn = conns.populate_connection(@field_definition, object, value, @arguments, ctx)
+                if conn
+                  conn.was_authorized_by_scope_items = @was_scoped
+                end
+                conn
+              end
+            when Schema::Field::ScopeExtension
+              if @was_scoped.nil?
+                if (rt = @field_definition.type.unwrap).respond_to?(:scope_items)
+                  @was_scoped = true
+                  @field_results = @field_results.map { |v| v.nil? ? v : rt.scope_items(v, ctx) }
+                else
+                  @was_scoped = false
+                end
+              end
             else
-              build_results
+              memo = memos[@finish_extension_idx]
+              @field_results = ext.after_resolve_batching(objects: @extended.object, arguments: @extended.arguments, context: ctx, values: @field_results, memo: memo) # rubocop:disable Development/ContextIsPassedCop
+            end
+            @finish_extension_idx += 1
+            if any_lazy_results?
+              @runner.dataloader.lazy_at_depth(path.size, self)
+              return
             end
-          else
-            build_results
           end
+
+          @finish_extension_idx = nil
+          build_results
         end
 
         def build_results
@@ -298,8 +373,8 @@ def build_results
               # Do nothing -- it will enqueue itself later
             end
           else
-            results = @selections_step.results
             ctx = @selections_step.query.context
+            results = @selections_step.results
             field_result_idx = 0
             i = 0
             s = results.size

lib/graphql/execution/batching/prepare_object_step.rb

@@ -62,6 +62,12 @@ def call
         end
 
         def authorize
+          if @field_resolve_step.was_scoped && !@resolved_type.reauthorize_scoped_objects
+            @authorized_value = @object
+            create_result
+            return
+          end
+
           query = @field_resolve_step.selections_step.query
           begin
             query.current_trace.begin_authorized(@resolved_type, @object, query.context)

lib/graphql/pagination/connection.rb

@@ -94,6 +94,8 @@ def initialize(items, parent: nil, field: nil, context: nil, first: nil, after:
         @was_authorized_by_scope_items = detect_was_authorized_by_scope_items
       end
 
+      attr_writer :was_authorized_by_scope_items
+
       def was_authorized_by_scope_items?
         @was_authorized_by_scope_items
       end

lib/graphql/pagination/connections.rb

@@ -83,6 +83,38 @@ def wrap(field, parent, items, arguments, context)
         end
       end
 
+      def populate_connection(field, object, value, original_arguments, context)
+        if value.is_a? GraphQL::ExecutionError
+          # This isn't even going to work because context doesn't have ast_node anymore
+          context.add_error(value)
+          nil
+        elsif value.nil?
+          nil
+        elsif value.is_a?(GraphQL::Pagination::Connection)
+          # update the connection with some things that may not have been provided
+          value.context ||= context
+          value.parent ||= object
+          value.first_value ||= original_arguments[:first]
+          value.after_value ||= original_arguments[:after]
+          value.last_value ||= original_arguments[:last]
+          value.before_value ||= original_arguments[:before]
+          value.arguments ||= original_arguments # rubocop:disable Development/ContextIsPassedCop -- unrelated .arguments method
+          value.field ||= field
+          if field.has_max_page_size? && !value.has_max_page_size_override?
+            value.max_page_size = field.max_page_size
+          end
+          if field.has_default_page_size? && !value.has_default_page_size_override?
+            value.default_page_size = field.default_page_size
+          end
+          if (custom_t = context.schema.connections.edge_class_for_field(field))
+            value.edge_class = custom_t
+          end
+          value
+        else
+          context.namespace(:connections)[:all_wrappers] ||= context.schema.connections.all_wrappers
+          context.schema.connections.wrap(field, object, value, original_arguments, context)
+        end
+      end
       # use an override if there is one
       # @api private
       def edge_class_for_field(field)