Merge pull request rmosolgo#1437 from rmosolgo/field-methods-super

Support `super` in field methods

Author: Robert Mosolgo

lib/graphql/introspection/base_object.rb

@@ -9,6 +9,7 @@ def self.field(*args, **kwargs, &block)
 
       def self.inherited(child_class)
         child_class.introspection(true)
+        super
       end
     end
   end

lib/graphql/schema/field.rb

@@ -232,39 +232,49 @@ def resolve_field(obj, args, ctx)
           inner_obj = obj && obj.object
           prev_resolve.call(inner_obj, args, ctx)
         else
-          resolve_field_dynamic(obj, args, ctx)
+          public_send_field(obj, @method_sym, args, ctx)
         end
       end
 
-      private
-
-      # Try a few ways to resolve the field
-      # @api private
-      def resolve_field_dynamic(obj, args, ctx)
-        if obj.respond_to?(@method_sym)
-          public_send_field(obj, @method_sym, args, ctx)
-        elsif obj.object.is_a?(Hash)
+      # Find a way to resolve this field, checking:
+      #
+      # - Hash keys, if the wrapped object is a hash;
+      # - A method on the wrapped object;
+      # - Or, raise not implemented.
+      #
+      # This can be overridden by defining a method on the object type.
+      # @param obj [GraphQL::Schema::Object]
+      # @param ruby_kwargs [Hash<Symbol => Object>]
+      # @param ctx [GraphQL::Query::Context]
+      def resolve_field_method(obj, ruby_kwargs, ctx)
+        if obj.object.is_a?(Hash)
           inner_object = obj.object
           if inner_object.key?(@method_sym)
             inner_object[@method_sym]
           else
             inner_object[@method_str]
           end
         elsif obj.object.respond_to?(@method_sym)
-          public_send_field(obj.object, @method_sym, args, ctx)
+          if ruby_kwargs.any?
+            obj.object.public_send(@method_sym, **ruby_kwargs)
+          else
+            obj.object.public_send(@method_sym)
+          end
         else
           raise <<-ERR
-Failed to implement #{ctx.irep_node.owner_type.name}.#{ctx.field.name}, tried:
+        Failed to implement #{@owner.graphql_name}.#{@name}, tried:
 
-- `#{obj.class}##{@method_sym}`, which did not exist
-- `#{obj.object.class}##{@method_sym}`, which did not exist
-- Looking up hash key `#{@method_sym.inspect}` or `#{@method_str.inspect}` on `#{obj.object}`, but it wasn't a Hash
+        - `#{obj.class}##{@method_sym}`, which did not exist
+        - `#{obj.object.class}##{@method_sym}`, which did not exist
+        - Looking up hash key `#{@method_sym.inspect}` or `#{@method_str.inspect}` on `#{obj.object}`, but it wasn't a Hash
 
-To implement this field, define one of the methods above (and check for typos)
-ERR
+        To implement this field, define one of the methods above (and check for typos)
+        ERR
         end
       end
 
+      private
+
       NO_ARGS = {}.freeze
 
       def public_send_field(obj, method_name, graphql_args, field_ctx)

lib/graphql/schema/member/has_fields.rb

@@ -4,6 +4,47 @@ class Schema
     class Member
       # Shared code for Object and Interface
       module HasFields
+        class << self
+          # When this module is added to a class,
+          # add a place for that class's default behaviors
+          def self.extended(child_class)
+            add_default_resolve_module(child_class)
+            super
+          end
+
+          # Create a module which will have instance methods for implementing fields.
+          # These will be `super` methods for fields in interfaces, objects and mutations.
+          # Use an instance variable on the class instead of a constant
+          # so that module namespaces won't be an issue. (If we used constants,
+          # `child_class::DefaultResolve` might find a constant from an included module.)
+          def add_default_resolve_module(child_class)
+            if child_class.instance_variable_get(:@_default_resolve)
+              # This can happen when an object implements an interface,
+              # since that interface has the `included` hook above.
+              return
+            end
+
+            default_resolve_module = Module.new
+            child_class.instance_variable_set(:@_default_resolve, default_resolve_module)
+            child_class.include(default_resolve_module)
+          end
+        end
+
+        # When this is included into interfaces,
+        # add a place for default field behaviors
+        def included(child_class)
+          HasFields.add_default_resolve_module(child_class)
+          # Also, prepare a place for default field implementations
+          super
+        end
+
+        # When a subclass of objects are created,
+        # add a place for that subclass's default field behaviors
+        def inherited(child_class)
+          HasFields.add_default_resolve_module(child_class)
+          super
+        end
+
         # Add a field to this object or interface with the given definition
         # @see {GraphQL::Schema::Field#initialize} for method signature
         # @return [void]
@@ -25,11 +66,29 @@ def fields
           all_fields
         end
 
-        # Register this field with the class, overriding a previous one if needed
+        # Register this field with the class, overriding a previous one if needed.
+        # Also, add a parent method for resolving this field.
         # @param field_defn [GraphQL::Schema::Field]
         # @return [void]
         def add_field(field_defn)
           own_fields[field_defn.name] = field_defn
+          # Add a `super` method for implementing this field:
+          field_key = field_defn.name.inspect
+          default_resolve_module = self.instance_variable_get(:@_default_resolve)
+          if default_resolve_module.nil?
+            # This should have been set up in one of the inherited or included hooks above,
+            # if it wasn't, it's because those hooks weren't called because `super` wasn't present.
+            raise <<-ERR
+Uh oh! #{self} doesn't have a default resolve module. This probably means that an `inherited` hook didn't call super.
+Check `inherited` on #{self}'s superclasses.
+ERR
+          end
+          default_resolve_module.module_eval <<-RUBY, __FILE__, __LINE__ + 1
+            def #{field_defn.method_sym}(**args)
+              field_inst = self.class.fields[#{field_key}] || raise(%|Failed to find field #{field_key} for \#{self.class} among \#{self.class.fields.keys}|)
+              field_inst.resolve_field_method(self, args, context)
+            end
+          RUBY
           nil
         end
 

lib/graphql/schema/mutation.rb

@@ -85,6 +85,7 @@ def resolve(**args)
       class << self
         def inherited(base)
           base.null(null)
+          super
         end
 
         # Override the method from HasFields to support `field: Mutation.field`, for backwards compat.

lib/graphql/schema/relay_classic_mutation.rb

@@ -26,6 +26,7 @@ class RelayClassicMutation < GraphQL::Schema::Mutation
       class << self
         def inherited(base)
           base.null(true)
+          super
         end
 
         # The base class for generated input object types

spec/graphql/schema/member/has_fields_spec.rb

@@ -0,0 +1,126 @@
+# frozen_string_literal: true
+require "spec_helper"
+
+describe GraphQL::Schema::Member::HasFields do
+  module SuperTest
+    class BaseObject < GraphQL::Schema::Object
+    end
+
+    module BaseInterface
+      include GraphQL::Schema::Interface
+    end
+
+    module InterfaceWithFloatField
+      include BaseInterface
+      field :float, Float, null: false
+      def float
+        # This should call the default implementation
+        super * 0.5
+      end
+    end
+
+    module SubInterfaceWithFloatField
+      include InterfaceWithFloatField
+      def float
+        # This should call `InterfaceWithFloatField#float`
+        super * 0.1
+      end
+    end
+
+    class ObjectWithFloatField < BaseObject
+      implements InterfaceWithFloatField
+    end
+
+    class ObjectWithSubFloatField < BaseObject
+      implements SubInterfaceWithFloatField
+    end
+
+    module InterfaceWithStringField
+      include BaseInterface
+      field :string, String, null: false
+      def string
+        # Return a literal value to ensure this method was called
+        "here's a string"
+      end
+    end
+
+    class ObjectWithStringField < BaseObject
+      implements InterfaceWithStringField
+      def string
+        # This should call to `InterfaceWithStringField#string`
+        super.upcase
+      end
+    end
+
+    class SubObjectWithStringField < ObjectWithStringField
+      def string
+        # This should call to `ObjectWithStringField#string`
+        super.reverse
+      end
+    end
+
+    class Query < BaseObject
+      field :int, Integer, null: false
+      def int
+        # This should call default resolution
+        super * 2
+      end
+
+      field :string1, ObjectWithStringField, null: false
+      alias :string1 :object
+
+      field :string2, SubObjectWithStringField, null: false
+      alias :string2 :object
+
+      field :float1, ObjectWithFloatField, null: false
+      alias :float1 :object
+
+      field :float2, ObjectWithSubFloatField, null: false
+      alias :float2 :object
+    end
+
+    class Schema < GraphQL::Schema
+      query(Query)
+    end
+  end
+
+  describe "Calling super in field methods" do
+    # Test that calling `super` in field methods "works", which means:
+    # - If there is a super method in the user-created hierarchy (either a class or module), it is called
+    #   This is tested by putting random transformations in method bodies,
+    #   then asserting that they are called.
+    # - If there's no user-defined super method, it calls the built-in default behavior
+    #   This is tested by putting values in the `root_value` hash.
+    #   The default behavior is to fetch hash values by key, so we assert that
+    #   those values are subject to the specified transformations.
+
+    describe "Object methods" do
+      it "may call super to default implementation" do
+        res = SuperTest::Schema.execute("{ int }", root_value: { int: 4 })
+        assert_equal 8, res["data"]["int"]
+      end
+
+      it "may call super to interface method" do
+        res = SuperTest::Schema.execute(" { string1 { string } }", root_value: {})
+        assert_equal "HERE'S A STRING", res["data"]["string1"]["string"]
+      end
+
+      it "may call super to superclass method" do
+        res = SuperTest::Schema.execute(" { string2 { string } }", root_value: {})
+        assert_equal "GNIRTS A S'EREH", res["data"]["string2"]["string"]
+      end
+    end
+
+    describe "Interface methods" do
+      it "may call super to interface method" do
+        res = SuperTest::Schema.execute(" { float1 { float } }", root_value: { float: 6.0 })
+        assert_equal 3.0, res["data"]["float1"]["float"]
+      end
+
+      it "may call super to superclass method" do
+        res = SuperTest::Schema.execute(" { float2 { float } }", root_value: { float: 6.0 })
+        assert_in_delta 0.001, 0.3, res["data"]["float2"]["float"]
+      end
+    end
+  end
+end

spec/graphql/schema/object_spec.rb

@@ -8,7 +8,7 @@
       assert_equal "Ensemble", object_class.graphql_name
       assert_equal "A group of musicians playing together", object_class.description
       assert_equal 6, object_class.fields.size
-      assert_equal 2, object_class.interfaces.size
+      assert_equal 3, object_class.interfaces.size
       # Compatibility methods are delegated to the underlying BaseType
       assert object_class.respond_to?(:connection_type)
     end
@@ -22,7 +22,7 @@
       # one more than the parent class
       assert_equal 7, new_object_class.fields.size
       # inherited interfaces are present
-      assert_equal 2, new_object_class.interfaces.size
+      assert_equal 3, new_object_class.interfaces.size
       # The new field is present
       assert new_object_class.fields.key?("newField")
       # The overridden field is present:

spec/support/jazz.rb

@@ -65,15 +65,13 @@ def initialize(*args, **options, &block)
       super(*args, **options, &block)
     end
 
-    def to_graphql
-      field_defn = super
-      if @upcase
-        inner_resolve = field_defn.resolve_proc
-        field_defn.resolve = ->(obj, args, ctx) {
-          inner_resolve.call(obj, args, ctx).upcase
-        }
+    def resolve_field(*)
+      result = super
+      if @upcase && result
+        result.upcase
+      else
+        result
       end
-      field_defn
     end
   end
 
@@ -168,15 +166,20 @@ def upcase_name
     end
   end
 
+  module HasMusicians
+    include BaseInterface
+    field :musicians, "[Jazz::Musician]", null: false
+  end
+
+
   # Here's a new-style GraphQL type definition
   class Ensemble < ObjectWithUpcasedName
     # Test string type names
     # This method should override inherited one
     field :name, "String", null: false, method: :overridden_name
-    implements GloballyIdentifiableType, NamedEntity
+    implements GloballyIdentifiableType, NamedEntity, HasMusicians
     description "A group of musicians playing together"
     config :config, :configged
-    field :musicians, "[Jazz::Musician]", null: false
     field :formed_at, String, null: true, hash_key: "formedAtDate"
 
     def overridden_name