diff --git a/ee/app/graphql/types/epic_type.rb b/ee/app/graphql/types/epic_type.rb index 62ef6865914f03e5b52a34c6180bcc773763b259..d8f18662e1be4a986e96cdb2ba9d93c1d5e1f07a 100644 --- a/ee/app/graphql/types/epic_type.rb +++ b/ee/app/graphql/types/epic_type.rb @@ -232,7 +232,9 @@ def blocked_by_count end def blocking_count - ::Epic::RelatedEpicLink.blocking_issuables_count_for(object) + ::Gitlab::Graphql::Aggregations::Epics::LazyBlockAggregate.new(context, object.id, link_type: :blocking) do |count| + count || 0 + end end def blocked_by_epics diff --git a/ee/lib/gitlab/graphql/aggregations/issuables/lazy_block_aggregate.rb b/ee/lib/gitlab/graphql/aggregations/issuables/lazy_block_aggregate.rb index b4273a22119dae115f992a52254c790b378af6f1..d280112038425019220abef290b5326f69895d95 100644 --- a/ee/lib/gitlab/graphql/aggregations/issuables/lazy_block_aggregate.rb +++ b/ee/lib/gitlab/graphql/aggregations/issuables/lazy_block_aggregate.rb @@ -7,30 +7,32 @@ module Issuables class LazyBlockAggregate include ::Gitlab::Graphql::Deferred - attr_reader :issuable_id, :lazy_state + attr_reader :issuable_id, :lazy_state, :link_type - def initialize(query_ctx, issuable_id, &block) + def initialize(query_ctx, issuable_id, link_type: :blocked, &block) @issuable_id = issuable_id + @link_type = link_type.to_s @block = block # Initialize the loading state for this query, # or get the previously-initiated state @lazy_state = query_ctx[:lazy_block_aggregate] ||= { - pending_ids: Set.new, - loaded_objects: {} + pending_ids: { 'blocked' => Set.new, 'blocking' => Set.new }, + loaded_objects: { 'blocked' => {}, 'blocking' => {} } } + # Register this ID to be loaded later: - @lazy_state[:pending_ids] << issuable_id + @lazy_state[:pending_ids][@link_type] << issuable_id end # Return the loaded record, hitting the database if needed def block_aggregate # Check if the record was already loaded - if @lazy_state[:pending_ids].present? + if @lazy_state[:pending_ids][link_type].present? load_records_into_loaded_objects end - result = @lazy_state[:loaded_objects][@issuable_id] + result = @lazy_state[:loaded_objects][link_type][@issuable_id] return @block.call(result) if @block @@ -52,16 +54,17 @@ def issuable_type def load_records_into_loaded_objects # The record hasn't been loaded yet, so # hit the database with all pending IDs to prevent N+1 - grouped_ids_row = "blocked_#{issuable_type}_id" - pending_ids = @lazy_state[:pending_ids].to_a - blocked_data = link_class.blocked_issuables_for_collection(pending_ids).compact.flatten + grouped_ids_row = "#{link_type}_#{issuable_type}_id" + pending_ids = @lazy_state[:pending_ids][link_type].to_a + builder = link_class.method("#{link_type}_issuables_for_collection") + data = builder.call(pending_ids).compact.flatten - blocked_data.each do |blocked| - issuable_id = blocked[grouped_ids_row] - @lazy_state[:loaded_objects][issuable_id] = blocked.count + data.each do |row| + issuable_id = row[grouped_ids_row] + @lazy_state[:loaded_objects][link_type][issuable_id] = row.count end - @lazy_state[:pending_ids].clear + @lazy_state[:pending_ids][link_type].clear end end end diff --git a/ee/spec/lib/gitlab/graphql/aggregations/epics/lazy_block_aggregate_spec.rb b/ee/spec/lib/gitlab/graphql/aggregations/epics/lazy_block_aggregate_spec.rb index 6955ba1e6c5dc2035950535e5f4a2614bf09d670..9d1bbfb54d59ecfb18697087003cda1b34db8d42 100644 --- a/ee/spec/lib/gitlab/graphql/aggregations/epics/lazy_block_aggregate_spec.rb +++ b/ee/spec/lib/gitlab/graphql/aggregations/epics/lazy_block_aggregate_spec.rb @@ -8,11 +8,18 @@ let(:issuable_link_class) { Epic::RelatedEpicLink } - let(:fake_data) do + let(:fake_blocked_data) do [ { blocked_epic_id: 135, count: 1.0 }, nil # nil for unblocked issuables ] end + + let(:fake_blocking_data) do + [ + { blocking_epic_id: 135, count: 1.0 }, + nil # nil for unblocked issuables + ] + end end end diff --git a/ee/spec/lib/gitlab/graphql/aggregations/issues/lazy_block_aggregate_spec.rb b/ee/spec/lib/gitlab/graphql/aggregations/issues/lazy_block_aggregate_spec.rb index 5f893bb57ab939a8186b82ef76b6e226afb6ffc7..3d89cb7f4926a9ac66192ba01e197cb772fe1610 100644 --- a/ee/spec/lib/gitlab/graphql/aggregations/issues/lazy_block_aggregate_spec.rb +++ b/ee/spec/lib/gitlab/graphql/aggregations/issues/lazy_block_aggregate_spec.rb @@ -8,11 +8,18 @@ let(:issuable_link_class) { IssueLink } - let(:fake_data) do + let(:fake_blocked_data) do [ { blocked_issue_id: 1745, count: 1.0 }, nil # nil for unblocked issuables ] end + + let(:fake_blocking_data) do + [ + { blocking_issue_id: 1745, count: 1.0 }, + nil # nil for unblocked issuables + ] + end end end diff --git a/ee/spec/requests/api/graphql/group/epics_spec.rb b/ee/spec/requests/api/graphql/group/epics_spec.rb index a2e385d33f6cee731d7ecec38db675bb22d0a22a..aa4deeda295a8e0173deb11c59cb9beaf28dd223 100644 --- a/ee/spec/requests/api/graphql/group/epics_spec.rb +++ b/ee/spec/requests/api/graphql/group/epics_spec.rb @@ -392,6 +392,12 @@ def global_ids(*epics) include_examples 'N+1 query check' end + + context 'when requesting blocking_count' do + let(:requested_fields) { [:blocking_count] } + + include_examples 'N+1 query check' + end end end diff --git a/ee/spec/support/shared_examples/lib/gitlab/graphql/issuables_lazy_block_aggregate_shared_examples.rb b/ee/spec/support/shared_examples/lib/gitlab/graphql/issuables_lazy_block_aggregate_shared_examples.rb index f7ea42d6a83620f95b31e297b73111734a05a51f..4e5c85d5d054169969058bc7aa5405e2591e0ada 100644 --- a/ee/spec/support/shared_examples/lib/gitlab/graphql/issuables_lazy_block_aggregate_shared_examples.rb +++ b/ee/spec/support/shared_examples/lib/gitlab/graphql/issuables_lazy_block_aggregate_shared_examples.rb @@ -6,64 +6,115 @@ end describe '#initialize' do - it 'adds the issuable_id to the lazy state' do + it 'adds the issuable_id to the `blocked` lazy state by default' do subject = described_class.new(query_ctx, issuable_id) - expect(subject.lazy_state[:pending_ids]).to match_array [issuable_id] + expect(subject.lazy_state[:pending_ids]['blocked']).to match_array [issuable_id] expect(subject.issuable_id).to match issuable_id end end describe '#block_aggregate' do - subject { described_class.new(query_ctx, issuable_id) } + subject { described_class.new(query_ctx, issuable_id, link_type: link_type) } let(:fake_state) do - { pending_ids: Set.new, loaded_objects: {} } + { + pending_ids: { 'blocked' => Set.new, 'blocking' => Set.new }, + loaded_objects: { 'blocked' => {}, 'blocking' => {} } + } end before do subject.instance_variable_set(:@lazy_state, fake_state) end - context 'when there is a block provided' do + shared_examples 'block provided' do subject do - described_class.new(query_ctx, issuable_id) do |result| + described_class.new(query_ctx, issuable_id, link_type: link_type) do |result| result.do_thing end end it 'calls the block' do - expect(fake_state[:loaded_objects][issuable_id]).to receive(:do_thing) + expect(fake_state[:loaded_objects][link_type][issuable_id]).to receive(:do_thing) subject.block_aggregate end end - context 'if the record has already been loaded' do - let(:fake_state) do - { pending_ids: Set.new, loaded_objects: { issuable_id => class_double(issuable_link_class, count: 10) } } - end - + shared_examples 'the record has already been loaded' do it 'does not make the query again' do - expect(issuable_link_class).not_to receive(:blocked_issuables_for_collection) + expect(issuable_link_class).not_to receive(:"#{link_type}_issuables_for_collection") subject.block_aggregate end end - context 'if the record has not been loaded' do - let(:fake_state) do - { pending_ids: Set.new([issuable_id]), loaded_objects: {} } - end + shared_examples 'the record has not been loaded' do + let(:fake_data) { link_type == 'blocked' ? fake_blocked_data : fake_blocking_data } before do - expect(issuable_link_class).to receive(:blocked_issuables_for_collection).and_return(fake_data) + expect(issuable_link_class).to receive(:"#{link_type}_issuables_for_collection").and_return(fake_data) end it 'clears the pending IDs' do subject.block_aggregate - expect(subject.lazy_state[:pending_ids]).to be_empty + expect(subject.lazy_state[:pending_ids][link_type]).to be_empty + end + end + + context 'when link_type is `blocked`' do + let(:link_type) { 'blocked' } + + it_behaves_like 'block provided' + + it_behaves_like 'the record has already been loaded' do + let(:fake_state) do + { + pending_ids: { 'blocked' => Set.new, 'blocking' => Set.new }, + loaded_objects: { + 'blocked' => { issuable_id => class_double(issuable_link_class, count: 10) }, + 'blocking' => {} + } + } + end + end + + it_behaves_like 'the record has not been loaded' do + let(:fake_state) do + { + pending_ids: { 'blocked' => Set.new([issuable_id]), 'blocking' => Set.new }, + loaded_objects: { 'blocked' => {}, 'blocking' => {} } + } + end + end + end + + context 'when link_type is `blocking`' do + let(:link_type) { 'blocking' } + + it_behaves_like 'block provided' + + it_behaves_like 'the record has already been loaded' do + let(:fake_state) do + { + pending_ids: { 'blocked' => Set.new, 'blocking' => Set.new }, + loaded_objects: { + 'blocked' => {}, + 'blocking' => { issuable_id => class_double(issuable_link_class, count: 5) } + } + } + end + end + + it_behaves_like 'the record has not been loaded' do + let(:fake_state) do + { + pending_ids: { 'blocked' => Set.new, 'blocking' => Set.new([issuable_id]) }, + loaded_objects: { 'blocked' => {}, 'blocking' => {} } + } + end end end end