Merge branch '214749-backfill-projects-in-elasticsearch-when-moved-between-groups' into 'master'

Backfill projects in Elasticsearch when transferred

See merge request gitlab-org/gitlab!51717

ee/app/services/ee/groups/transfer_service.rb

@@ -31,20 +31,21 @@ module EE
       end
 
       def update_elasticsearch_hooks(updated_project_ids)
-        project_ids = updated_project_ids
-
         # Handle when group is moved to a new group. There is no way to know
-        # whether the group was using Elasticsearch before the transfer, so the ES cache is invalidated
-        # for each associated project. Otherwise, it is assumed all projects are indexed
-        # and only those with visibility changes have their ES cache entry invalidated
+        # whether the group was using Elasticsearch before the transfer. If Elasticsearch limit indexing is
+        # enabled, each associated project has the ES cache is invalidated all associated data is indexed.
+        # If Elasticsearch limit indexing is disabled, all projects are indexed and only those with visibility
+        # changes have their ES cache entry invalidated.
         if ::Gitlab::CurrentSettings.elasticsearch_limit_indexing?
-          project_ids = group.all_projects.select(:id)
-        end
+          ::Project.id_in(group.all_projects.select(:id)).find_each do |project|
+            project.invalidate_elasticsearch_indexes_cache!
 
-        ::Project.id_in(project_ids).find_each do |project|
-          project.invalidate_elasticsearch_indexes_cache!
-
-          project.maintain_elasticsearch_update(updated_attributes: [:visibility_level]) if project.maintaining_elasticsearch?
+            ::Elastic::ProcessInitialBookkeepingService.backfill_projects!(project) if project.maintaining_elasticsearch?
+          end
+        else
+          ::Project.id_in(updated_project_ids).find_each do |project|
+            project.maintain_elasticsearch_update(updated_attributes: [:visibility_level]) if project.maintaining_elasticsearch?
+          end
         end
       end
     end

ee/app/services/ee/projects/transfer_service.rb

@@ -42,7 +42,7 @@ module EE
         if old_namespace.use_elasticsearch? != new_namespace.use_elasticsearch?
           project.invalidate_elasticsearch_indexes_cache!
 
-          project.maintain_elasticsearch_update(updated_attributes: [:visibility_level]) if project.maintaining_elasticsearch?
+          ::Elastic::ProcessInitialBookkeepingService.backfill_projects!(project) if project.maintaining_elasticsearch?
         end
       end
     end

ee/changelogs/unreleased/214749-backfill-projects-in-elasticsearch-when-moved-between-groups.yml

+---
+title: Backfill projects in Elasticsearch when moved between groups
+merge_request: 51717
+author:
+type: fixed

ee/spec/services/groups/transfer_service_spec.rb

@@ -14,7 +14,7 @@ RSpec.describe Groups::TransferService, '#execute' do
     new_group&.add_owner(user)
   end
 
-  describe 'elasticsearch indexing', :elastic, :aggregate_failures do
+  describe 'elasticsearch indexing', :aggregate_failures do
     before do
       stub_ee_application_setting(elasticsearch_indexing: true)
     end
@@ -24,16 +24,15 @@ RSpec.describe Groups::TransferService, '#execute' do
         stub_ee_application_setting(elasticsearch_limit_indexing: true)
       end
 
-      let(:project) { create(:project, :repository, :public, namespace: group) }
-
       context 'when moving from a non-indexed namespace to an indexed namespace' do
         before do
           create(:elasticsearch_indexed_namespace, namespace: new_group)
         end
 
-        it 'invalidates the cache and indexes the project and associated issues' do
-          expect(Elastic::ProcessBookkeepingService).to receive(:track!).with(project)
-          expect(ElasticAssociationIndexerWorker).to receive(:perform_async).with('Project', project.id, ['issues'])
+        it 'invalidates the cache and indexes the project and all associated data' do
+          expect(project).not_to receive(:maintain_elasticsearch_update)
+          expect(project).not_to receive(:maintain_elasticsearch_destroy)
+          expect(::Elastic::ProcessInitialBookkeepingService).to receive(:backfill_projects!).with(project)
           expect(::Gitlab::CurrentSettings).to receive(:invalidate_elasticsearch_indexes_cache_for_project!).with(project.id).and_call_original
 
           transfer_service.execute(new_group)
@@ -46,9 +45,10 @@ RSpec.describe Groups::TransferService, '#execute' do
           create(:elasticsearch_indexed_namespace, namespace: new_group)
         end
 
-        it 'invalidates the cache and indexes the project and associated issues' do
-          expect(Elastic::ProcessBookkeepingService).to receive(:track!).with(project)
-          expect(ElasticAssociationIndexerWorker).to receive(:perform_async).with('Project', project.id, ['issues'])
+        it 'invalidates the cache and indexes the project and associated issues only' do
+          expect(project).not_to receive(:maintain_elasticsearch_update)
+          expect(project).not_to receive(:maintain_elasticsearch_destroy)
+          expect(::Elastic::ProcessInitialBookkeepingService).to receive(:backfill_projects!).with(project)
           expect(::Gitlab::CurrentSettings).to receive(:invalidate_elasticsearch_indexes_cache_for_project!).with(project.id).and_call_original
 
           transfer_service.execute(new_group)
@@ -60,11 +60,12 @@ RSpec.describe Groups::TransferService, '#execute' do
       context 'when visibility changes' do
         let(:new_group) { create(:group, :private) }
 
-        it 'reindexes projects and associated issues' do
+        it 'does not invalidate the cache and reindexes projects and associated issues' do
           project1 = create(:project, :repository, :public, namespace: group)
           project2 = create(:project, :repository, :public, namespace: group)
           project3 = create(:project, :repository, :private, namespace: group)
 
+          expect(::Gitlab::CurrentSettings).not_to receive(:invalidate_elasticsearch_indexes_cache_for_project!)
           expect(Elastic::ProcessBookkeepingService).to receive(:track!).with(project1)
           expect(ElasticAssociationIndexerWorker).to receive(:perform_async).with('Project', project1.id, ['issues'])
           expect(Elastic::ProcessBookkeepingService).to receive(:track!).with(project2)

ee/spec/services/projects/transfer_service_spec.rb

@@ -90,9 +90,9 @@ RSpec.describe Projects::TransferService do
           create(:elasticsearch_indexed_namespace, namespace: group)
         end
 
-        it 'invalidates the cache and indexes the project and associated issues' do
-          expect(Elastic::ProcessBookkeepingService).to receive(:track!).with(project)
-          expect(ElasticAssociationIndexerWorker).to receive(:perform_async).with('Project', project.id, ['issues'])
+        it 'invalidates the cache and indexes the project and all associated data' do
+          expect(Elastic::ProcessInitialBookkeepingService).to receive(:backfill_projects!).with(project)
+          expect(project).not_to receive(:maintain_elasticsearch_destroy)
           expect(::Gitlab::CurrentSettings).to receive(:invalidate_elasticsearch_indexes_cache_for_project!).with(project.id).and_call_original
 
           subject.execute(group)
@@ -105,9 +105,9 @@ RSpec.describe Projects::TransferService do
           create(:elasticsearch_indexed_namespace, namespace: project.namespace)
         end
 
-        it 'does not invalidate the cache and reindexes the project only' do
-          expect(Elastic::ProcessBookkeepingService).to receive(:track!).with(project)
-          expect(ElasticAssociationIndexerWorker).not_to receive(:perform_async)
+        it 'does not invalidate the cache does not index or delete anything' do
+          expect(Elastic::ProcessInitialBookkeepingService).not_to receive(:backfill_projects!).with(project)
+          expect(project).not_to receive(:maintain_elasticsearch_destroy)
           expect(::Gitlab::CurrentSettings).not_to receive(:invalidate_elasticsearch_indexes_cache_for_project!)
 
           subject.execute(group)