Skip to content
Snippets Groups Projects
Verified Commit e3262298 authored by Siddharth Dungarwal's avatar Siddharth Dungarwal :two: Committed by GitLab
Browse files

Add elastic_reference and es_parent methods in epic model

parent 1b869f48
No related branches found
No related tags found
1 merge request!180361Add elastic_reference and es_parent methods in epic model
# frozen_string_literal: true
module Search
module Elastic
module EpicsSearch
extend ActiveSupport::Concern
include ::Elastic::ApplicationVersionedSearch
included do
extend ::Gitlab::Utils::Override
override :maintain_elasticsearch_create
def maintain_elasticsearch_create
::Elastic::ProcessBookkeepingService.track!(*get_indexing_data)
end
override :maintain_elasticsearch_update
def maintain_elasticsearch_update(updated_attributes: previous_changes.keys)
::Elastic::ProcessBookkeepingService.track!(*get_indexing_data)
associations_to_update = associations_needing_elasticsearch_update(updated_attributes)
return unless associations_to_update.present?
ElasticAssociationIndexerWorker.perform_async(self.class.name, id, associations_to_update)
end
override :maintain_elasticsearch_destroy
def maintain_elasticsearch_destroy
::Elastic::ProcessBookkeepingService.track!(*get_indexing_data)
end
end
private
def get_indexing_data
[Search::Elastic::References::WorkItem.new(
issue_id, "group_#{group.root_ancestor.id}")]
end
end
end
end
...@@ -3,6 +3,7 @@ ...@@ -3,6 +3,7 @@
module EE module EE
module Epic module Epic
extend ActiveSupport::Concern extend ActiveSupport::Concern
extend ::Gitlab::Utils::Override
prepended do prepended do
include AtomicInternalId include AtomicInternalId
...@@ -23,7 +24,7 @@ module Epic ...@@ -23,7 +24,7 @@ module Epic
include EachBatch include EachBatch
include ::Exportable include ::Exportable
include Epics::MetadataCacheUpdate include Epics::MetadataCacheUpdate
include ::Search::Elastic::EpicsSearch include ::Elastic::ApplicationVersionedSearch
include Elastic::UpdateAssociatedEpicsOnDateChange include Elastic::UpdateAssociatedEpicsOnDateChange
# we'd need to make sure these override the existing associations so we prepend this. # we'd need to make sure these override the existing associations so we prepend this.
...@@ -562,6 +563,15 @@ def use_elasticsearch? ...@@ -562,6 +563,15 @@ def use_elasticsearch?
group.use_elasticsearch? group.use_elasticsearch?
end end
def es_parent
"group_#{group.root_ancestor.id}"
end
override :elastic_reference
def elastic_reference
::Search::Elastic::References::WorkItem.new(issue_id, es_parent).serialize
end
def issues_readable_by(current_user, preload: nil) def issues_readable_by(current_user, preload: nil)
related_issues = self.class.related_issues(ids: id, preload: preload) related_issues = self.class.related_issues(ids: id, preload: preload)
......
...@@ -44,6 +44,7 @@ module WorkItem ...@@ -44,6 +44,7 @@ module WorkItem
project: :project_feature project: :project_feature
) )
preloaded_data.each(&:lazy_labels)
::Namespaces::Preloaders::NamespaceRootAncestorPreloader.new(preloaded_data.map(&:namespace)).execute ::Namespaces::Preloaders::NamespaceRootAncestorPreloader.new(preloaded_data.map(&:namespace)).execute
if ::Feature.enabled?(:search_work_items_index_notes, ::Feature.current_request) if ::Feature.enabled?(:search_work_items_index_notes, ::Feature.current_request)
......
...@@ -28,11 +28,11 @@ ...@@ -28,11 +28,11 @@
context 'when an epic is created' do context 'when an epic is created' do
let(:epic) { build(:epic, group: group) } let(:epic) { build(:epic, group: group) }
it 'tracks the work item' do it 'tracks the epic and work item' do
expect(::Elastic::ProcessBookkeepingService).to receive(:track!).once do |*tracked_refs| expect(::Elastic::ProcessBookkeepingService).to receive(:track!).once do |*tracked_refs|
expect(tracked_refs.count).to eq(1) expect(tracked_refs.count).to eq(1)
expect(tracked_refs[0].class).to eq(Search::Elastic::References::WorkItem) expect(tracked_refs[0].class).to eq(Epic)
expect(tracked_refs[0].identifier).to eq(epic.issue_id) expect(tracked_refs[0].id).to eq(epic.id)
end end
expect(::Elastic::ProcessBookkeepingService).to receive(:track!).once do |*tracked_refs| expect(::Elastic::ProcessBookkeepingService).to receive(:track!).once do |*tracked_refs|
expect(tracked_refs.count).to eq(1) expect(tracked_refs.count).to eq(1)
...@@ -48,23 +48,23 @@ ...@@ -48,23 +48,23 @@
end end
context 'when an epic is updated' do context 'when an epic is updated' do
it 'tracks the work item' do it 'tracks the epic' do
expect(::Elastic::ProcessBookkeepingService).to receive(:track!).once do |*tracked_refs| expect(::Elastic::ProcessBookkeepingService).to receive(:track!).once do |*tracked_refs|
expect(tracked_refs.count).to eq(1) expect(tracked_refs.count).to eq(1)
expect(tracked_refs[0]).to be_a_kind_of(Search::Elastic::References::WorkItem) expect(tracked_refs[0].class).to eq(Epic)
expect(tracked_refs[0].identifier).to eq(epic.issue_id) expect(tracked_refs[0].id).to eq(epic.id)
end end
epic.update!(title: 'A new title') epic.update!(title: 'A new title')
end end
end end
context 'when an epic is deleted' do context 'when an epic is deleted' do
it 'tracks the work item' do it 'tracks the work item and epic' do
expect(::Elastic::ProcessBookkeepingService).to receive(:track!).with(*[WorkItem.find(epic.issue_id)]).once expect(::Elastic::ProcessBookkeepingService).to receive(:track!).with(*[WorkItem.find(epic.issue_id)]).once
expect(::Elastic::ProcessBookkeepingService).to receive(:track!).once do |*tracked_refs| expect(::Elastic::ProcessBookkeepingService).to receive(:track!).once do |*tracked_refs|
expect(tracked_refs.count).to eq(1) expect(tracked_refs.count).to eq(1)
expect(tracked_refs[0]).to be_a_kind_of(Search::Elastic::References::WorkItem) expect(tracked_refs[0].class).to eq(Epic)
expect(tracked_refs[0].identifier).to eq(epic.issue_id) expect(tracked_refs[0].id).to eq(epic.id)
end end
epic.destroy! epic.destroy!
end end
...@@ -145,8 +145,8 @@ ...@@ -145,8 +145,8 @@
it 'tracks the epic' do it 'tracks the epic' do
expect(::Elastic::ProcessBookkeepingService).to receive(:track!).once do |*tracked_refs| expect(::Elastic::ProcessBookkeepingService).to receive(:track!).once do |*tracked_refs|
expect(tracked_refs.count).to eq(1) expect(tracked_refs.count).to eq(1)
expect(tracked_refs[0]).to be_a_kind_of(Search::Elastic::References::WorkItem) expect(tracked_refs[0]).to be_a_kind_of(Epic)
expect(tracked_refs[0].identifier).to eq(epic.issue_id) expect(tracked_refs[0].id).to eq(epic.id)
end end
epic.update!(group: parent_group) epic.update!(group: parent_group)
end end
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe ::Search::Elastic::EpicsSearch, :elastic, feature_category: :global_search do
let_it_be(:epic) { create(:epic) }
describe '#maintain_elasticsearch_create' do
it 'calls track! for epic' do
expect(::Elastic::ProcessBookkeepingService).to receive(:track!).once do |*tracked_refs|
expect(tracked_refs.count).to eq(1)
expect(tracked_refs[0]).to be_a_kind_of(Search::Elastic::References::WorkItem)
expect(tracked_refs[0].identifier).to eq(epic.issue_id)
end
epic.maintain_elasticsearch_create
end
end
describe '#maintain_elasticsearch_destory' do
it 'calls track! for epic' do
expect(::Elastic::ProcessBookkeepingService).to receive(:track!).once do |*tracked_refs|
expect(tracked_refs.count).to eq(1)
expect(tracked_refs[0]).to be_a_kind_of(Search::Elastic::References::WorkItem)
expect(tracked_refs[0].identifier).to eq(epic.issue_id)
end
epic.maintain_elasticsearch_destroy
end
end
describe '#maintain_elasticsearch_update' do
it 'calls track! for epic' do
expect(::Elastic::ProcessBookkeepingService).to receive(:track!).once do |*tracked_refs|
expect(tracked_refs.count).to eq(1)
expect(tracked_refs[0]).to be_a_kind_of(Search::Elastic::References::WorkItem)
expect(tracked_refs[0].identifier).to eq(epic.issue_id)
end
epic.maintain_elasticsearch_update
end
context 'when we have associations to update' do
before do
allow(Epic).to receive(:elastic_index_dependants).and_return([{ association_name: :issues,
on_change: 'title' }])
end
it 'calls track! for epic and updates the associations' do
expect(::Elastic::ProcessBookkeepingService).to receive(:track!).once do |*tracked_refs|
expect(tracked_refs.count).to eq(1)
expect(tracked_refs[0]).to be_a_kind_of(Search::Elastic::References::WorkItem)
expect(tracked_refs[0].identifier).to eq(epic.issue_id)
end
expect(::ElasticAssociationIndexerWorker).to receive(:perform_async).with('Epic', epic.id, ['issues']).once
epic.maintain_elasticsearch_update
end
end
end
end
...@@ -1364,8 +1364,8 @@ def as_item(item) ...@@ -1364,8 +1364,8 @@ def as_item(item)
it 'calls ::Elastic::ProcessBookkeepingService.track! when the epic is updated' do it 'calls ::Elastic::ProcessBookkeepingService.track! when the epic is updated' do
expect(::Elastic::ProcessBookkeepingService).to receive(:track!).once do |*tracked_refs| expect(::Elastic::ProcessBookkeepingService).to receive(:track!).once do |*tracked_refs|
expect(tracked_refs.count).to eq(1) expect(tracked_refs.count).to eq(1)
expect(tracked_refs[0]).to be_a_kind_of(Search::Elastic::References::WorkItem) expect(tracked_refs[0]).to be_a_kind_of(Epic)
expect(tracked_refs[0].identifier).to eq(epic.issue_id) expect(tracked_refs[0].id).to eq(epic.id)
end end
epic.update!(title: 'A new title') epic.update!(title: 'A new title')
end end
...@@ -1833,4 +1833,20 @@ def as_item(item) ...@@ -1833,4 +1833,20 @@ def as_item(item)
end end
end end
end end
describe '#elastic_reference' do
let_it_be(:epic) { create(:epic, group: group) }
it 'returns the string representation for the elasticsearch' do
expect(epic.elastic_reference).to eq("WorkItem|#{epic.issue_id}|#{epic.es_parent}")
end
end
describe '#es_parent' do
let_it_be(:epic) { create(:epic, group: group) }
it 'returns to correct routing id' do
expect(epic.es_parent).to eq("group_#{group.root_ancestor.id}")
end
end
end end
...@@ -493,6 +493,70 @@ ...@@ -493,6 +493,70 @@
expect { described_class.new.execute }.not_to exceed_all_query_limit(control) expect { described_class.new.execute }.not_to exceed_all_query_limit(control)
end end
it 'does not have N+1 queries for epics' do
epics = create_list(:epic, 2, :use_fixed_dates)
described_class.track!(*epics)
control = ActiveRecord::QueryRecorder.new(skip_cached: false) { described_class.new.execute }
epics += create_list(:epic, 3, :use_fixed_dates)
described_class.track!(*epics)
expect do
described_class.new.execute
end.not_to exceed_all_query_limit(control)
end
it 'does not have N+1 queries for epics in a group with multiple parents' do
parent_group = create(:group)
group = create(:group, parent: parent_group)
epics = create_list(:epic, 2, group: group)
described_class.track!(*epics)
control = ActiveRecord::QueryRecorder.new(skip_cached: false) { described_class.new.execute }
epics += create_list(:epic, 3, group: create(:group, parent: parent_group))
described_class.track!(*epics)
expect do
described_class.new.execute
end.not_to exceed_all_query_limit(control)
end
it 'does not have N+1 queries for epics with inherited dates' do
child_epic = create(:epic, :use_fixed_dates)
milestone = create(:milestone, :with_dates)
epics = create_list(:epic, 2)
epics.each do |epic|
epic.start_date_sourcing_epic = child_epic
epic.due_date_sourcing_milestone = milestone
epic.save!
end
described_class.track!(*epics)
control = ActiveRecord::QueryRecorder.new(skip_cached: false) { described_class.new.execute }
epics += create_list(:epic, 3)
epics.each do |epic|
epic.start_date_sourcing_epic = child_epic
epic.due_date_sourcing_milestone = milestone
epic.save!
end
described_class.track!(*epics)
expect do
described_class.new.execute
end.not_to exceed_all_query_limit(control)
end
end end
it 'does not have N+1 queries for notes' do it 'does not have N+1 queries for notes' do
...@@ -611,64 +675,6 @@ ...@@ -611,64 +675,6 @@
expect { described_class.new.execute }.not_to exceed_all_query_limit(control) expect { described_class.new.execute }.not_to exceed_all_query_limit(control)
end end
end end
it 'does not have N+1 queries for epics' do
epics = create_list(:epic, 2, :use_fixed_dates)
described_class.track!(*epics)
control = ActiveRecord::QueryRecorder.new(skip_cached: false) { described_class.new.execute }
epics += create_list(:epic, 3, :use_fixed_dates)
described_class.track!(*epics)
expect { described_class.new.execute }.not_to exceed_all_query_limit(control)
end
it 'does not have N+1 queries for epics with inherited dates' do
child_epic = create(:epic, :use_fixed_dates)
milestone = create(:milestone, :with_dates)
epics = create_list(:epic, 2)
epics.each do |epic|
epic.start_date_sourcing_epic = child_epic
epic.due_date_sourcing_milestone = milestone
epic.save!
end
described_class.track!(*epics)
control = ActiveRecord::QueryRecorder.new(skip_cached: false) { described_class.new.execute }
epics += create_list(:epic, 3)
epics.each do |epic|
epic.start_date_sourcing_epic = child_epic
epic.due_date_sourcing_milestone = milestone
epic.save!
end
described_class.track!(*epics)
expect { described_class.new.execute }.not_to exceed_all_query_limit(control)
end
it 'does not have N+1 queries for epics in a group with multiple parents' do
parent_group = create(:group)
group = create(:group, parent: parent_group)
epics = create_list(:epic, 2, group: group)
described_class.track!(*epics)
control = ActiveRecord::QueryRecorder.new(skip_cached: false) { described_class.new.execute }
epics += create_list(:epic, 3, group: create(:group, parent: parent_group))
described_class.track!(*epics)
expect { described_class.new.execute }.not_to exceed_all_query_limit(control)
end
end end
def expect_processing(*refs, failures: []) def expect_processing(*refs, failures: [])
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment