Merge branch 'complexity-fix' into 'master'

Fix graphql resolver complexity

See merge request gitlab-org/gitlab!24983

app/graphql/resolvers/base_resolver.rb

@@ -28,6 +28,10 @@ module Resolvers
       end
     end
 
+    def self.complexity
+      0
+    end
+
     def self.resolver_complexity(args, child_complexity:)
       complexity = 1
       complexity += 1 if args[:sort]

app/graphql/types/base_field.rb

@@ -9,7 +9,7 @@ module Types
     def initialize(*args, **kwargs, &block)
       @calls_gitaly = !!kwargs.delete(:calls_gitaly)
       @constant_complexity = !!kwargs[:complexity]
-      kwargs[:complexity] ||= field_complexity(kwargs[:resolver_class])
+      kwargs[:complexity] = field_complexity(kwargs[:resolver_class], kwargs[:complexity])
       @feature_flag = kwargs[:feature_flag]
       kwargs = check_feature_flag(kwargs)
 
@@ -51,7 +51,9 @@ module Types
       args
     end
 
-    def field_complexity(resolver_class)
+    def field_complexity(resolver_class, current)
+      return current if current.present? && current > 0
+
       if resolver_class
         field_resolver_complexity
       else
@@ -66,22 +68,30 @@ module Types
       # proc because we set complexity depending on arguments and number of
       # items which can be loaded.
       proc do |ctx, args, child_complexity|
+        next base_complexity unless resolver_complexity_enabled?(ctx)
+
         # Resolvers may add extra complexity depending on used arguments
         complexity = child_complexity + self.resolver&.try(:resolver_complexity, args, child_complexity: child_complexity).to_i
         complexity += 1 if calls_gitaly?
+        complexity += complexity * connection_complexity_multiplier(ctx, args)
 
-        field_defn = to_graphql
+        complexity.to_i
+      end
+    end
+
+    def resolver_complexity_enabled?(ctx)
+      ctx.fetch(:graphql_resolver_complexity_flag) { |key| ctx[key] = Feature.enabled?(:graphql_resolver_complexity) }
+    end
 
-        if field_defn.connection?
+    def connection_complexity_multiplier(ctx, args)
       # Resolvers may add extra complexity depending on number of items being loaded.
+      field_defn = to_graphql
+      return 0 unless field_defn.connection?
+
       page_size   = field_defn.connection_max_page_size || ctx.schema.default_max_page_size
       limit_value = [args[:first], args[:last], page_size].compact.min
       multiplier  = self.resolver&.try(:complexity_multiplier, args).to_f
-          complexity += complexity * limit_value * multiplier
-        end
-
-        complexity.to_i
-      end
+      limit_value * multiplier
     end
   end
 end

ee/app/graphql/resolvers/epic_resolver.rb

@@ -92,5 +92,10 @@ module Resolvers
 
       complexity
     end
+
+    # https://gitlab.com/gitlab-org/gitlab/issues/205312
+    def self.complexity_multiplier(args)
+      0.001
+    end
   end
 end

ee/app/graphql/types/design_management/design_collection_type.rb

@@ -17,7 +17,8 @@ module Types
             Types::DesignManagement::DesignType.connection_type,
             null: false,
             resolver: Resolvers::DesignManagement::DesignsResolver,
-            description: 'All designs for the design collection'
+            description: 'All designs for the design collection',
+            complexity: 5
 
       field :versions,
             Types::DesignManagement::VersionType.connection_type,

spec/graphql/types/base_field_spec.rb

@@ -46,7 +46,15 @@ describe Types::BaseField do
       expect(field.to_graphql.complexity).to eq 12
     end
 
-    context 'when field has a resolver proc' do
+    context 'when field has a resolver' do
+      context 'when a valid complexity is already set' do
+        let(:field) { described_class.new(name: 'test', type: GraphQL::STRING_TYPE.connection_type, resolver_class: resolver, complexity: 2, max_page_size: 100, null: true) }
+
+        it 'uses this complexity' do
+          expect(field.to_graphql.complexity).to eq 2
+        end
+      end
+
       context 'and is a connection' do
         let(:field) { described_class.new(name: 'test', type: GraphQL::STRING_TYPE.connection_type, resolver_class: resolver, max_page_size: 100, null: true) }
 
@@ -59,6 +67,17 @@ describe Types::BaseField do
           expect(field.to_graphql.complexity.call({}, { first: 1 }, 2)).to eq 2
           expect(field.to_graphql.complexity.call({}, { first: 1, foo: true }, 2)).to eq 4
         end
+
+        context 'when graphql_resolver_complexity is disabled' do
+          before do
+            stub_feature_flags(graphql_resolver_complexity: false)
+          end
+
+          it 'sets default field complexity' do
+            expect(field.to_graphql.complexity.call({}, {}, 2)).to eq 1
+            expect(field.to_graphql.complexity.call({}, { first: 50 }, 2)).to eq 1
+          end
+        end
       end
 
       context 'and is not a connection' do

spec/requests/api/graphql_spec.rb

@@ -152,4 +152,52 @@ describe 'GraphQL' do
       end
     end
   end
+
+  describe 'resolver complexity' do
+    let_it_be(:project) { create(:project, :public) }
+    let(:query) do
+      graphql_query_for(
+        'project',
+        { 'fullPath' => project.full_path },
+        query_graphql_field(resource, {}, 'edges { node { iid } }')
+      )
+    end
+
+    before do
+      stub_const('GitlabSchema::DEFAULT_MAX_COMPLEXITY', 6)
+      stub_feature_flags(graphql_resolver_complexity: true)
+    end
+
+    context 'when fetching single resource' do
+      let(:resource) { 'issues(first: 1)' }
+
+      it 'processes the query' do
+        post_graphql(query)
+
+        expect(graphql_errors).to be_nil
+      end
+    end
+
+    context 'when fetching too many resources' do
+      let(:resource) { 'issues(first: 100)' }
+
+      it 'returns an error' do
+        post_graphql(query)
+
+        expect_graphql_errors_to_include(/which exceeds max complexity/)
+      end
+
+      context 'when graphql_resolver_complexity is disabled' do
+        before do
+          stub_feature_flags(graphql_resolver_complexity: false)
+        end
+
+        it 'processes the query' do
+          post_graphql(query)
+
+          expect(graphql_errors).to be_nil
+        end
+      end
+    end
+  end
 end