Merge pull request rmosolgo#679 from rmosolgo/connection-instrumentation

refactor(Relay) wrap returned objects with instrumentation

Author: rmosolgo

lib/graphql/compatibility/lazy_execution_specification.rb

@@ -167,6 +167,7 @@ def test_it_calls_lazy_resolve_instrumentation
               "PUSH",
               "Query.push: 1",
               "Query.push: 2",
+              "Query.pushes: [1, 2, 3]",
               "PUSH",
               "LazyPush.push: 3",
               "LazyPushEdge.node: 1",

lib/graphql/compatibility/lazy_execution_specification/lazy_schema.rb

@@ -37,6 +37,10 @@ def initialize(ctx, values)
           def push
             @values.map { |v| LazyPush.new(@ctx, v) }
           end
+
+          def value
+            @values
+          end
         end
 
         module LazyInstrumentation

lib/graphql/define/assign_connection.rb

@@ -4,8 +4,9 @@ module Define
     module AssignConnection
       def self.call(type_defn, *field_args, max_page_size: nil, **field_kwargs, &field_block)
         underlying_field = GraphQL::Define::AssignObjectField.call(type_defn, *field_args, **field_kwargs, &field_block)
-        connection_field = GraphQL::Relay::ConnectionField.create(underlying_field, max_page_size: max_page_size)
-        type_defn.fields[underlying_field.name] = connection_field
+        underlying_field.connection_max_page_size = max_page_size
+        underlying_field.connection = true
+        type_defn.fields[underlying_field.name] = underlying_field
       end
     end
   end

lib/graphql/field.rb

@@ -177,13 +177,25 @@ class Field
     # @return [Object, GraphQL::Function] The function used to derive this field
     attr_accessor :function
 
+    attr_writer :connection
+
+    # @return [Boolean]
+    def connection?
+      @connection
+    end
+
+    # @return [nil, Integer]
+    attr_accessor :connection_max_page_size
+
     def initialize
       @complexity = 1
       @arguments = {}
       @resolve_proc = build_default_resolver
       @lazy_resolve_proc = DefaultLazyResolve
       @relay_node_field = false
       @default_arguments = nil
+      @connection = false
+      @connection_max_page_size = nil
     end
 
     def initialize_copy(other)

lib/graphql/relay.rb

@@ -11,6 +11,6 @@
 require 'graphql/relay/global_id_resolve'
 require 'graphql/relay/mutation'
 require 'graphql/relay/node'
-require 'graphql/relay/connection_field'
+require 'graphql/relay/connection_instrumentation'
 require 'graphql/relay/connection_resolve'
 require 'graphql/relay/connection_type'

lib/graphql/relay/connection_field.rb renamed to lib/graphql/relay/connection_instrumentation.rb

@@ -2,12 +2,11 @@
 module GraphQL
   module Relay
     # Provided a GraphQL field which returns a collection of nodes,
-    # `ConnectionField.create` modifies that field to expose those nodes
-    # as a collection.
+    # wrap that field to expose those nodes as a connection.
     #
     # The original resolve proc is used to fetch nodes,
     # then a connection implementation is fetched with {BaseConnection.connection_for_nodes}.
-    class ConnectionField
+    module ConnectionInstrumentation
       ARGUMENT_DEFINITIONS = [
           ["first", GraphQL::INT_TYPE, "Returns the first _n_ elements from the list."],
           ["after", GraphQL::STRING_TYPE, "Returns the elements in the list that come after the specified global ID."],
@@ -28,14 +27,21 @@ class ConnectionField
       # Build a connection field from a {GraphQL::Field} by:
       # - Merging in the default arguments
       # - Transforming its resolve function to return a connection object
-      # @param underlying_field [GraphQL::Field] A field which returns nodes to be wrapped as a connection
-      # @param max_page_size [Integer] The maximum number of nodes which may be requested (if a larger page is requested, it is limited to this number)
-      # @return [GraphQL::Field] A redefined field with connection behavior
-      def self.create(underlying_field, max_page_size: nil)
-        connection_arguments = DEFAULT_ARGUMENTS.merge(underlying_field.arguments)
-        original_resolve = underlying_field.resolve_proc
-        connection_resolve = GraphQL::Relay::ConnectionResolve.new(underlying_field, original_resolve, max_page_size: max_page_size)
-        underlying_field.redefine(resolve: connection_resolve, arguments: connection_arguments)
+      def self.instrument(type, field)
+        if field.connection?
+          connection_arguments = DEFAULT_ARGUMENTS.merge(field.arguments)
+          original_resolve = field.resolve_proc
+          original_lazy_resolve = field.lazy_resolve_proc
+          connection_resolve = GraphQL::Relay::ConnectionResolve.new(field, original_resolve)
+          connection_lazy_resolve = GraphQL::Relay::ConnectionResolve.new(field, original_lazy_resolve)
+          field.redefine(
+            resolve: connection_resolve,
+            lazy_resolve: connection_lazy_resolve,
+            arguments: connection_arguments,
+          )
+        else
+          field
+        end
       end
     end
   end

lib/graphql/relay/connection_resolve.rb

@@ -2,18 +2,16 @@
 module GraphQL
   module Relay
     class ConnectionResolve
-      def initialize(field, underlying_resolve, max_page_size: nil)
+      def initialize(field, underlying_resolve)
         @field = field
         @underlying_resolve = underlying_resolve
-        @max_page_size = max_page_size
+        @max_page_size = field.connection_max_page_size
       end
 
       def call(obj, args, ctx)
         nodes = @underlying_resolve.call(obj, args, ctx)
         if ctx.schema.lazy?(nodes)
-          @field.prepare_lazy(nodes, args, ctx).then { |resolved_nodes|
-            build_connection(resolved_nodes, args, obj, ctx)
-          }
+          nodes
         else
           build_connection(nodes, args, obj, ctx)
         end

lib/graphql/relay/mutation.rb

@@ -122,7 +122,7 @@ def has_generated_return_type?
       end
 
       def resolve=(new_resolve_proc)
-        @resolve_proc = MutationResolve.new(self, new_resolve_proc, wrap_result: has_generated_return_type?)
+        @resolve_proc = new_resolve_proc
       end
 
       def field
@@ -218,20 +218,34 @@ def self.define_subclass(mutation_defn)
         end
       end
 
+      module MutationInstrumentation
+        def self.instrument(type, field)
+          if field.mutation
+            new_resolve = MutationResolve.new(field.mutation, field.resolve_proc)
+            new_lazy_resolve = MutationResolve.new(field.mutation, field.lazy_resolve_proc)
+            field.redefine(resolve: new_resolve, lazy_resolve: new_lazy_resolve)
+          else
+            field
+          end
+        end
+      end
+
       class MutationResolve
-        def initialize(mutation, resolve, wrap_result:)
+        def initialize(mutation, resolve)
           @mutation = mutation
           @resolve = resolve
-          @wrap_result = wrap_result
+          @wrap_result = mutation.has_generated_return_type?
         end
 
         def call(obj, args, ctx)
-          mutation_result = @resolve.call(obj, args[:input], ctx)
+          begin
+            mutation_result = @resolve.call(obj, args[:input], ctx)
+          rescue GraphQL::ExecutionError => err
+            mutation_result = err
+          end
 
           if ctx.schema.lazy?(mutation_result)
-            @mutation.field.prepare_lazy(mutation_result, args, ctx).then { |inner_obj|
-              build_result(inner_obj, args, ctx)
-            }
+            mutation_result
           else
             build_result(mutation_result, args, ctx)
           end

lib/graphql/schema.rb

@@ -433,8 +433,15 @@ def rescue_middleware
 
     private
 
+    # Apply instrumentation to fields. Relay instrumentation is applied last
+    # so that user-provided instrumentation can wrap user-provided resolve functions,
+    # _then_ Relay helpers can wrap the returned objects.
     def build_instrumented_field_map
-      @instrumented_field_map = InstrumentedFieldMap.new(self, @instrumenters[:field])
+      all_instrumenters = @instrumenters[:field] + [
+        GraphQL::Relay::ConnectionInstrumentation,
+        GraphQL::Relay::Mutation::MutationInstrumentation,
+      ]
+      @instrumented_field_map = InstrumentedFieldMap.new(self, all_instrumenters)
     end
 
     def build_types_map

spec/graphql/relay/connection_field_spec.rb renamed to spec/graphql/relay/connection_instrumentation_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 require "spec_helper"
 
-describe GraphQL::Relay::ConnectionField do
+describe GraphQL::Relay::ConnectionInstrumentation do
   it "replaces the previous field definition" do
     test_type = GraphQL::ObjectType.define do
       name "Test"
@@ -16,7 +16,7 @@
     assert_instance_of StarWars::ShipsWithMaxPageSize, conn_field.function
   end
 
-  it "leaves the original field untouched" do
+  let(:build_schema) {
     test_type = nil
 
     test_field = GraphQL::Field.define do
@@ -29,7 +29,15 @@
       connection :tests, test_field
     end
 
-    conn_field = test_type.fields["tests"]
+    [
+      test_field,
+      GraphQL::Schema.define(query: test_type, raise_definition_error: true)
+    ]
+  }
+
+  it "leaves the original field untouched" do
+    test_field, test_schema = build_schema
+    conn_field = test_schema.get_field(test_schema.query, "tests")
 
     assert_equal 0, test_field.arguments.length
     assert_equal 4, conn_field.arguments.length
@@ -39,12 +47,8 @@
   end
 
   it "passes connection behaviors to redefinitions" do
-    test_type = GraphQL::ObjectType.define do
-      name "Test"
-      connection :tests, test_type.connection_type
-    end
-
-    connection_field = test_type.fields["tests"]
+    _test_field, test_schema = build_schema
+    connection_field = test_schema.get_field(test_schema.query, "tests")
     redefined_connection_field = connection_field.redefine { argument "name", types.String }
 
     assert_equal 4, connection_field.arguments.size