Merge pull request rmosolgo#4318 from rmosolgo/reduce-is-a

Reduce runtime .is_a? calls in schema objects

Author: rmosolgo

lib/graphql/schema/directive.rb

@@ -8,6 +8,7 @@ class Schema
     # - {.resolve}: Wraps field resolution (so it should call `yield` to continue)
     class Directive < GraphQL::Schema::Member
       extend GraphQL::Schema::Member::HasArguments
+      extend GraphQL::Schema::Member::HasArguments::HasDirectiveArguments
 
       class << self
         # Directives aren't types, they don't have kinds.

lib/graphql/schema/field.rb

@@ -6,6 +6,7 @@ module GraphQL
   class Schema
     class Field
       include GraphQL::Schema::Member::HasArguments
+      include GraphQL::Schema::Member::HasArguments::FieldConfigured
       include GraphQL::Schema::Member::HasAstNode
       include GraphQL::Schema::Member::HasPath
       include GraphQL::Schema::Member::HasValidators

lib/graphql/schema/member/has_arguments.rb

@@ -11,6 +11,7 @@ def self.included(cls)
         def self.extended(cls)
           cls.extend(ArgumentClassAccessor)
           cls.include(ArgumentObjectLoader)
+          cls.extend(ClassConfigured)
         end
 
         # @see {GraphQL::Schema::Argument#initialize} for parameters
@@ -109,14 +110,6 @@ def remove_argument(arg_defn)
 
         # @return [Hash<String => GraphQL::Schema::Argument] Arguments defined on this thing, keyed by name. Includes inherited definitions
         def arguments(context = GraphQL::Query::NullContext)
-          inherited_arguments = if self.is_a?(Class) && superclass.respond_to?(:arguments)
-            superclass.arguments(context)
-          elsif defined?(@resolver_class) && @resolver_class
-            @resolver_class.field_arguments(context)
-          else
-            nil
-          end
-          # Local definitions override inherited ones
           if own_arguments.any?
             own_arguments_that_apply = {}
             own_arguments.each do |name, args_entry|
@@ -125,70 +118,119 @@ def arguments(context = GraphQL::Query::NullContext)
               end
             end
           end
+          # might be nil if there are actually no arguments
+          own_arguments_that_apply || own_arguments
+        end
 
-          if inherited_arguments
-            if own_arguments_that_apply
-              inherited_arguments.merge(own_arguments_that_apply)
-            else
-              inherited_arguments
+        module ClassConfigured
+          def inherited(child_class)
+            super
+            child_class.extend(InheritedArguments)
+          end
+
+          module InheritedArguments
+            def arguments(context = GraphQL::Query::NullContext)
+              own_arguments = super
+              inherited_arguments = superclass.arguments(context)
+
+              if own_arguments.any?
+                if inherited_arguments.any?
+                  # Local definitions override inherited ones
+                  inherited_arguments.merge(own_arguments)
+                else
+                  own_arguments
+                end
+              else
+                inherited_arguments
+              end
+            end
+
+            def all_argument_definitions
+              all_defns = {}
+              ancestors.reverse_each do |ancestor|
+                if ancestor.respond_to?(:own_arguments)
+                  all_defns.merge!(ancestor.own_arguments)
+                end
+              end
+              all_defns = all_defns.values
+              all_defns.flatten!
+              all_defns
+            end
+
+
+            def get_argument(argument_name, context = GraphQL::Query::NullContext)
+              warden = Warden.from_context(context)
+              for ancestor in ancestors
+                if ancestor.respond_to?(:own_arguments) &&
+                  (a = ancestor.own_arguments[argument_name]) &&
+                  (a = Warden.visible_entry?(:visible_argument?, a, context, warden))
+                  return a
+                end
+              end
+              nil
             end
-          else
-            # might be nil if there are actually no arguments
-            own_arguments_that_apply || own_arguments
           end
         end
 
-        def all_argument_definitions
-          if self.is_a?(Class)
-            all_defns = {}
-            ancestors.reverse_each do |ancestor|
-              if ancestor.respond_to?(:own_arguments)
-                all_defns.merge!(ancestor.own_arguments)
+        module FieldConfigured
+          def arguments(context = GraphQL::Query::NullContext)
+            own_arguments = super
+            if defined?(@resolver_class) && @resolver_class
+              inherited_arguments = @resolver_class.field_arguments(context)
+              if own_arguments.any?
+                if inherited_arguments.any?
+                  inherited_arguments.merge(own_arguments)
+                else
+                  own_arguments
+                end
+              else
+                inherited_arguments
               end
+            else
+              own_arguments
             end
-          elsif defined?(@resolver_class) && @resolver_class
-            all_defns = {}
-            @resolver_class.all_field_argument_definitions.each do |arg_defn|
-              key = arg_defn.graphql_name
-              case (current_value = all_defns[key])
-              when nil
-                all_defns[key] = arg_defn
-              when Array
-                current_value << arg_defn
-              when GraphQL::Schema::Argument
-                all_defns[key] = [current_value, arg_defn]
-              else
-                raise "Invariant: Unexpected argument definition, #{current_value.class}: #{current_value.inspect}"
+          end
+
+          def all_argument_definitions
+            if defined?(@resolver_class) && @resolver_class
+              all_defns = {}
+              @resolver_class.all_field_argument_definitions.each do |arg_defn|
+                key = arg_defn.graphql_name
+                case (current_value = all_defns[key])
+                when nil
+                  all_defns[key] = arg_defn
+                when Array
+                  current_value << arg_defn
+                when GraphQL::Schema::Argument
+                  all_defns[key] = [current_value, arg_defn]
+                else
+                  raise "Invariant: Unexpected argument definition, #{current_value.class}: #{current_value.inspect}"
+                end
               end
+              all_defns.merge!(own_arguments)
+              all_defns = all_defns.values
+              all_defns.flatten!
+              all_defns
+            else
+              super
             end
-            all_defns.merge!(own_arguments)
-          else
-            all_defns = own_arguments
           end
-          all_defns = all_defns.values
+        end
+
+        def all_argument_definitions
+          all_defns = own_arguments.values
           all_defns.flatten!
           all_defns
         end
 
         # @return [GraphQL::Schema::Argument, nil] Argument defined on this thing, fetched by name.
         def get_argument(argument_name, context = GraphQL::Query::NullContext)
           warden = Warden.from_context(context)
-          if !self.is_a?(Class)
-            if (arg_config = own_arguments[argument_name]) && (visible_arg = Warden.visible_entry?(:visible_argument?, arg_config, context, warden))
-              visible_arg
-            elsif defined?(@resolver_class) && @resolver_class
-              @resolver_class.get_field_argument(argument_name, context)
-            else
-              nil
-            end
+          if (arg_config = own_arguments[argument_name]) && (visible_arg = Warden.visible_entry?(:visible_argument?, arg_config, context, warden))
+            visible_arg
+          elsif defined?(@resolver_class) && @resolver_class
+            @resolver_class.get_field_argument(argument_name, context)
           else
-            for ancestor in ancestors
-              if ancestor.respond_to?(:own_arguments) &&
-                (a = ancestor.own_arguments[argument_name]) &&
-                (a = Warden.visible_entry?(:visible_argument?, a, context, warden))
-                return a
-              end
-            end
             nil
           end
         end
@@ -265,7 +307,12 @@ def coerce_arguments(parent_object, values, context, &block)
         # but not for directives.
         # TODO apply static validations on schema definitions?
         def validate_directive_argument(arg_defn, value)
-          if arg_defn.owner.is_a?(Class) && arg_defn.owner < GraphQL::Schema::Directive
+          # this is only implemented on directives.
+          nil
+        end
+
+        module HasDirectiveArguments
+          def validate_directive_argument(arg_defn, value)
             if value.nil? && arg_defn.type.non_null?
               raise ArgumentError, "#{arg_defn.path} is required, but no value was given"
             end

lib/graphql/schema/member/has_interfaces.rb

@@ -55,16 +55,37 @@ def own_interface_type_memberships
         end
 
         def interface_type_memberships
-          own_tms = own_interface_type_memberships
-          if (self.is_a?(Class) && superclass.respond_to?(:interface_type_memberships))
-            inherited_tms = superclass.interface_type_memberships
-            if inherited_tms.size > 0
-              own_tms + inherited_tms
-            else
-              own_tms
+          own_interface_type_memberships
+        end
+
+        module ClassConfigured
+          # This combination of extended -> inherited -> extended
+          # means that the base class (`Schema::Object`) *won't*
+          # have the superclass-related code in `InheritedInterfaces`,
+          # but child classes of `Schema::Object` will have it.
+          # That way, we don't need a `superclass.respond_to?(...)` check.
+          def inherited(child_class)
+            super
+            child_class.extend(InheritedInterfaces)
+          end
+
+          module InheritedInterfaces
+            def interfaces(context = GraphQL::Query::NullContext)
+              visible_interfaces = super
+              visible_interfaces.concat(superclass.interfaces(context))
+              visible_interfaces.uniq!
+              visible_interfaces
+            end
+
+            def interface_type_memberships
+              own_tms = super
+              inherited_tms = superclass.interface_type_memberships
+              if inherited_tms.size > 0
+                own_tms + inherited_tms
+              else
+                own_tms
+              end
             end
-          else
-            own_tms
           end
         end
 
@@ -73,28 +94,29 @@ def interfaces(context = GraphQL::Query::NullContext)
           warden = Warden.from_context(context)
           visible_interfaces = []
           own_interface_type_memberships.each do |type_membership|
-            # During initialization, `type_memberships` can hold late-bound types
             case type_membership
-            when String, Schema::LateBoundType
-              visible_interfaces << type_membership
             when Schema::TypeMembership
               if warden.visible_type_membership?(type_membership, context)
                 visible_interfaces << type_membership.abstract_type
               end
+            when String, Schema::LateBoundType
+              # During initialization, `type_memberships` can hold late-bound types
+              visible_interfaces << type_membership
             else
               raise "Invariant: Unexpected type_membership #{type_membership.class}: #{type_membership.inspect}"
             end
           end
+          visible_interfaces.uniq!
 
-          if self.is_a?(Class) && superclass <= GraphQL::Schema::Object
-            visible_interfaces.concat(superclass.interfaces(context))
-          end
-
-          visible_interfaces.uniq
+          visible_interfaces
         end
 
         private
 
+        def self.extended(child_class)
+          child_class.extend(ClassConfigured)
+        end
+
         def inherited(subclass)
           super
           subclass.class_eval do

lib/graphql/schema/member/has_validators.rb

@@ -18,12 +18,38 @@ def validates(validation_config)
 
         # @return [Array<GraphQL::Schema::Validator>]
         def validators
-          own_validators = @own_validators || EMPTY_ARRAY
-          if self.is_a?(Class) && superclass.respond_to?(:validators) && (inherited_validators = superclass.validators).any?
-            inherited_validators + own_validators
-          else
-            own_validators
+          @own_validators || EMPTY_ARRAY
+        end
+
+        module ClassConfigured
+          def inherited(child_cls)
+            super
+            child_cls.extend(ClassValidators)
           end
+
+          module ClassValidators
+            include Schema::FindInheritedValue::EmptyObjects
+
+            def validators
+              inherited_validators = superclass.validators
+              if inherited_validators.any?
+                if @own_validators.nil?
+                  inherited_validators
+                else
+                  inherited_validators + @own_validators
+                end
+              elsif @own_validators.nil?
+                EMPTY_ARRAY
+              else
+                @own_validators
+              end
+            end
+          end
+        end
+
+        def self.extended(child_cls)
+          super
+          child_cls.extend(ClassConfigured)
         end
       end
     end

spec/graphql/schema/interface_spec.rb

@@ -336,7 +336,7 @@ def thing
       end
       interfaces_names = thing_type["interfaces"].map { |i| i["name"] }.sort
 
-      assert_equal interfaces_names, ["Named", "Node", "Timestamped"]
+      assert_equal ["Named", "Node", "Timestamped"], interfaces_names
     end
   end
 

spec/graphql/schema/object_spec.rb

@@ -40,13 +40,15 @@
       # one more than the parent class
       assert_equal 10, new_object_class.fields.size
       # inherited interfaces are present
-      assert_equal [
-          "GloballyIdentifiable",
-          "HasMusicians",
-          "InvisibleNameEntity",
-          "NamedEntity",
-          "PrivateNameEntity",
-        ], new_object_class.interfaces.map(&:graphql_name).sort
+      expected_interface_names = [
+        "GloballyIdentifiable",
+        "HasMusicians",
+        "InvisibleNameEntity",
+        "NamedEntity",
+        "PrivateNameEntity",
+      ]
+      assert_equal expected_interface_names, object_class.interfaces.map(&:graphql_name).sort
+      assert_equal expected_interface_names, new_object_class.interfaces.map(&:graphql_name).sort
       # The new field is present
       assert new_object_class.fields.key?("newField")
       # The overridden field is present: