Merge pull request #4670 from rmosolgo/inmplements-definition-methods

Fix definition_methods with implements

guides/type_definitions/interfaces.md

@@ -177,6 +177,8 @@ end
 
 The type definition DSL uses this mechanism, too, so you can override those methods here also.
 
+Note: Under the hood, `definition_methods` causes a module to be `extend`ed by the Inteface. Any calls to `extend` or `implement` may override methods from `definition_methods`.
+
 ### Resolve Type
 
 When a field's return type is an interface, GraphQL has to figure out what _specific_ object type to use for the return value. In the example above, each `customer` must be categorized as an `Individual` or `Company`. You can do this by:

lib/graphql/schema/interface.rb

@@ -20,6 +20,15 @@ module DefinitionMethods
         # - Added as class methods to this interface
         # - Added as class methods to all child interfaces
         def definition_methods(&block)
+          # Use an instance variable to tell whether it's been included previously or not;
+          # You can't use constant detection because constants are brought into scope
+          # by `include`, which has already happened at this point.
+          if !defined?(@_definition_methods)
+            defn_methods_module = Module.new
+            @_definition_methods = defn_methods_module
+            const_set(:DefinitionMethods, defn_methods_module)
+            extend(self::DefinitionMethods)
+          end
           self::DefinitionMethods.module_eval(&block)
         end
 
@@ -47,20 +56,11 @@ def included(child_class)
 
             child_class.type_membership_class(self.type_membership_class)
             child_class.ancestors.reverse_each do |ancestor|
-              if ancestor.const_defined?(:DefinitionMethods)
+              if ancestor.const_defined?(:DefinitionMethods) && ancestor != child_class
                 child_class.extend(ancestor::DefinitionMethods)
               end
             end
 
-            # Use an instance variable to tell whether it's been included previously or not;
-            # You can't use constant detection because constants are brought into scope
-            # by `include`, which has already happened at this point.
-            if !child_class.instance_variable_defined?(:@_definition_methods)
-              defn_methods_module = Module.new
-              child_class.instance_variable_set(:@_definition_methods, defn_methods_module)
-              child_class.const_set(:DefinitionMethods, defn_methods_module)
-              child_class.extend(child_class::DefinitionMethods)
-            end
             child_class.introspection(introspection)
             child_class.description(description)
             # If interfaces are mixed into each other, only define this class once

spec/graphql/schema/interface_spec.rb

@@ -124,6 +124,8 @@ module InterfaceB
 
     module InterfaceC
       include GraphQL::Schema::Interface
+      definition_methods do
+      end
     end
 
     module InterfaceD
@@ -603,6 +605,61 @@ def nil_fallback
         }
         assert_equal expected_result, result["data"]
       end
+
+      describe "in definition_methods when implementing another interface" do
+        class InterfaceInheritanceSchema < GraphQL::Schema
+          module Node
+            include GraphQL::Schema::Interface
+            definition_methods do
+              def resolve_type(obj, ctx)
+                raise "This should never be called -- it's overriden"
+              end
+            end
+          end
+          module Pet
+            include GraphQL::Schema::Interface
+            implements Node
+
+            definition_methods do
+              def resolve_type(obj, ctx)
+                if obj[:name] == "Fifi"
+                  Dog
+                else
+                  Cat
+                end
+              end
+            end
+          end
+          class Cat < GraphQL::Schema::Object
+            implements Pet
+          end
+
+          class Dog < GraphQL::Schema::Object
+            implements Pet
+          end
+
+          class Query < GraphQL::Schema::Object
+            field :pet, Pet do
+              argument :name, String
+            end
+
+            def pet(name:)
+              { name: name }
+            end
+          end
+
+          query(Query)
+          orphan_types(Cat, Dog)
+        end
+
+        it "calls the local definition, not the inherited one" do
+          res = InterfaceInheritanceSchema.execute("{ pet(name: \"Fifi\") { __typename } }")
+          assert_equal "Dog", res["data"]["pet"]["__typename"]
+
+          res = InterfaceInheritanceSchema.execute("{ pet(name: \"Pepper\") { __typename } }")
+          assert_equal "Cat", res["data"]["pet"]["__typename"]
+        end
+      end
     end
   end
 end