diff options
author | Brett Walker <bwalker@gitlab.com> | 2019-06-04 17:52:06 -0500 |
---|---|---|
committer | Brett Walker <bwalker@gitlab.com> | 2019-06-05 10:17:54 -0500 |
commit | 95b3fe286349176be7f117211dcc0ffa9dfcb8bb (patch) | |
tree | ac0ef2ca1dc6b09e192093f6398abe4a8805c035 | |
parent | 75e11922026db90ccb81b0125b18b0bd2ffbb7fb (diff) | |
download | gitlab-ce-95b3fe286349176be7f117211dcc0ffa9dfcb8bb.tar.gz |
Use :complexity_multiplier only with connections
This helps reduce complexity for non-connections
-rw-r--r-- | app/graphql/types/base_field.rb | 13 | ||||
-rw-r--r-- | spec/graphql/resolvers/base_resolver_spec.rb | 22 | ||||
-rw-r--r-- | spec/graphql/resolvers/issues_resolver_spec.rb | 2 | ||||
-rw-r--r-- | spec/graphql/resolvers/namespace_projects_resolver_spec.rb | 2 | ||||
-rw-r--r-- | spec/graphql/types/base_field_spec.rb | 29 |
5 files changed, 41 insertions, 27 deletions
diff --git a/app/graphql/types/base_field.rb b/app/graphql/types/base_field.rb index a374851e835..35763be685a 100644 --- a/app/graphql/types/base_field.rb +++ b/app/graphql/types/base_field.rb @@ -29,15 +29,16 @@ module Types # proc because we set complexity depending on arguments and number of # items which can be loaded. proc do |ctx, args, child_complexity| - page_size = @max_page_size || ctx.schema.default_max_page_size - limit_value = [args[:first], args[:last], page_size].compact.min - # Resolvers may add extra complexity depending on used arguments complexity = child_complexity + self.resolver&.try(:resolver_complexity, args, child_complexity: child_complexity).to_i - # Resolvers may add extra complexity depending on number of items being loaded. - multiplier = self.resolver&.try(:complexity_multiplier, args).to_f - complexity += complexity * limit_value * multiplier + if @connection + # Resolvers may add extra complexity depending on number of items being loaded. + page_size = @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 diff --git a/spec/graphql/resolvers/base_resolver_spec.rb b/spec/graphql/resolvers/base_resolver_spec.rb index 9982288e206..c162fdbbb47 100644 --- a/spec/graphql/resolvers/base_resolver_spec.rb +++ b/spec/graphql/resolvers/base_resolver_spec.rb @@ -29,18 +29,20 @@ describe Resolvers::BaseResolver do end end - it 'increases complexity based on arguments' do - field = Types::BaseField.new(name: 'test', type: GraphQL::STRING_TYPE, resolver_class: described_class, null: false, max_page_size: 1) + context 'when field is a connection' do + it 'increases complexity based on arguments' do + field = Types::BaseField.new(name: 'test', type: GraphQL::STRING_TYPE.connection_type, resolver_class: described_class, null: false, max_page_size: 1) - expect(field.to_graphql.complexity.call({}, { sort: 'foo' }, 1)).to eq 3 - expect(field.to_graphql.complexity.call({}, { search: 'foo' }, 1)).to eq 7 - end + expect(field.to_graphql.complexity.call({}, { sort: 'foo' }, 1)).to eq 3 + expect(field.to_graphql.complexity.call({}, { search: 'foo' }, 1)).to eq 7 + end - it 'does not increase complexity when filtering by iids' do - field = Types::BaseField.new(name: 'test', type: GraphQL::STRING_TYPE, resolver_class: described_class, null: false, max_page_size: 100) + it 'does not increase complexity when filtering by iids' do + field = Types::BaseField.new(name: 'test', type: GraphQL::STRING_TYPE.connection_type, resolver_class: described_class, null: false, max_page_size: 100) - expect(field.to_graphql.complexity.call({}, { sort: 'foo' }, 1)).to eq 6 - expect(field.to_graphql.complexity.call({}, { sort: 'foo', iid: 1 }, 1)).to eq 3 - expect(field.to_graphql.complexity.call({}, { sort: 'foo', iids: [1, 2, 3] }, 1)).to eq 3 + expect(field.to_graphql.complexity.call({}, { sort: 'foo' }, 1)).to eq 6 + expect(field.to_graphql.complexity.call({}, { sort: 'foo', iid: 1 }, 1)).to eq 3 + expect(field.to_graphql.complexity.call({}, { sort: 'foo', iids: [1, 2, 3] }, 1)).to eq 3 + end end end diff --git a/spec/graphql/resolvers/issues_resolver_spec.rb b/spec/graphql/resolvers/issues_resolver_spec.rb index bffcdbfe915..798fe00de97 100644 --- a/spec/graphql/resolvers/issues_resolver_spec.rb +++ b/spec/graphql/resolvers/issues_resolver_spec.rb @@ -121,7 +121,7 @@ describe Resolvers::IssuesResolver do end it 'increases field complexity based on arguments' do - field = Types::BaseField.new(name: 'test', type: GraphQL::STRING_TYPE, resolver_class: described_class, null: false, max_page_size: 100) + field = Types::BaseField.new(name: 'test', type: GraphQL::STRING_TYPE.connection_type, resolver_class: described_class, null: false, max_page_size: 100) expect(field.to_graphql.complexity.call({}, {}, 1)).to eq 4 expect(field.to_graphql.complexity.call({}, { labelName: 'foo' }, 1)).to eq 8 diff --git a/spec/graphql/resolvers/namespace_projects_resolver_spec.rb b/spec/graphql/resolvers/namespace_projects_resolver_spec.rb index 395e08081d3..20e197e9f73 100644 --- a/spec/graphql/resolvers/namespace_projects_resolver_spec.rb +++ b/spec/graphql/resolvers/namespace_projects_resolver_spec.rb @@ -57,7 +57,7 @@ describe Resolvers::NamespaceProjectsResolver, :nested_groups do end it 'has an high complexity regardless of arguments' do - field = Types::BaseField.new(name: 'test', type: GraphQL::STRING_TYPE, resolver_class: described_class, null: false, max_page_size: 100) + field = Types::BaseField.new(name: 'test', type: GraphQL::STRING_TYPE.connection_type, resolver_class: described_class, null: false, max_page_size: 100) expect(field.to_graphql.complexity.call({}, {}, 1)).to eq 24 expect(field.to_graphql.complexity.call({}, { include_subgroups: true }, 1)).to eq 24 diff --git a/spec/graphql/types/base_field_spec.rb b/spec/graphql/types/base_field_spec.rb index a7fb156d9a8..0d3c3e37daf 100644 --- a/spec/graphql/types/base_field_spec.rb +++ b/spec/graphql/types/base_field_spec.rb @@ -28,18 +28,29 @@ describe Types::BaseField do expect(field.to_graphql.complexity).to eq 12 end - it 'sets complexity depending on arguments for resolvers' do - field = described_class.new(name: 'test', type: GraphQL::STRING_TYPE, resolver_class: resolver, max_page_size: 100, null: true) + context 'when field has a resolver proc' do + 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) } - expect(field.to_graphql.complexity.call({}, {}, 2)).to eq 4 - expect(field.to_graphql.complexity.call({}, { first: 50 }, 2)).to eq 3 - end + it 'sets complexity depending on arguments for resolvers' do + expect(field.to_graphql.complexity.call({}, {}, 2)).to eq 4 + expect(field.to_graphql.complexity.call({}, { first: 50 }, 2)).to eq 3 + end - it 'sets complexity depending on number load limits for resolvers' do - field = described_class.new(name: 'test', type: GraphQL::STRING_TYPE, resolver_class: resolver, max_page_size: 100, null: true) + it 'sets complexity depending on number load limits for resolvers' 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 + end - 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 + context 'and is not a connection' do + it 'sets complexity as normal' do + field = described_class.new(name: 'test', type: GraphQL::STRING_TYPE, resolver_class: resolver, max_page_size: 100, null: true) + + expect(field.to_graphql.complexity.call({}, {}, 2)).to eq 2 + expect(field.to_graphql.complexity.call({}, { first: 50 }, 2)).to eq 2 + end + end end end end |