From 0f6a1c373980b53ae36178fc596acd38df4fb821 Mon Sep 17 00:00:00 2001
From: Arturo Herrero <arturo.herrero@gmail.com>
Date: Mon, 3 Mar 2025 13:22:34 +0100
Subject: [PATCH] Fix Rubocop offenses for migration update mappings helper

EE: true
---
 .rubocop_todo/gitlab/bounded_contexts.yml     |  1 -
 .rubocop_todo/search/namespaced_class.yml     |  1 -
 .../advanced_search_migration_styleguide.md   | 16 +++++--
 .../migration_update_mappings_helper.rb       | 40 ------------------
 .../migration_update_mappings_helper.rb       | 42 +++++++++++++++++++
 ...0152447_add_work_item_type_id_to_issues.rb |  2 +-
 ...0130154724_add_fields_to_projects_index.rb |  2 +-
 ...0208160152_add_count_fields_to_projects.rb |  2 +-
 .../20240410193847_add_embedding_to_issues.rb |  2 +-
 .../20240517092224_add_routing_to_issues.rb   |  2 +-
 ...5458_add_root_namespace_id_to_work_item.rb |  2 +-
 ...04125425_add_label_ids_to_merge_request.rb |  2 +-
 ..._issues_access_level_in_work_item_index.rb |  2 +-
 ...41003142503_add_embedding_to_work_items.rb |  2 +-
 .../20241003151804_add_notes_to_work_items.rb |  2 +-
 ...105111645_add_work_item_type_correct_id.rb |  2 +-
 ...942_add_traversal_ids_to_merge_requests.rb |  2 +-
 ...4115350_add_assignees_to_merge_requests.rb |  2 +-
 .../elastic/data_migration_service_spec.rb    |  6 +--
 .../elasticsearch/elasticsearch_migration.txt |  2 +-
 20 files changed, 71 insertions(+), 63 deletions(-)
 delete mode 100644 ee/app/workers/concerns/elastic/migration_update_mappings_helper.rb
 create mode 100644 ee/app/workers/concerns/search/elastic/migration_update_mappings_helper.rb

diff --git a/.rubocop_todo/gitlab/bounded_contexts.yml b/.rubocop_todo/gitlab/bounded_contexts.yml
index affb6b5d2e617..ca3017dff6711 100644
--- a/.rubocop_todo/gitlab/bounded_contexts.yml
+++ b/.rubocop_todo/gitlab/bounded_contexts.yml
@@ -3431,7 +3431,6 @@ Gitlab/BoundedContexts:
     - 'ee/app/workers/concerns/elastic/migration_obsolete.rb'
     - 'ee/app/workers/concerns/elastic/migration_options.rb'
     - 'ee/app/workers/concerns/elastic/migration_state.rb'
-    - 'ee/app/workers/concerns/elastic/migration_update_mappings_helper.rb'
     - 'ee/app/workers/concerns/geo_backoff_delay.rb'
     - 'ee/app/workers/concerns/geo_queue.rb'
     - 'ee/app/workers/concerns/update_orchestration_policy_configuration.rb'
diff --git a/.rubocop_todo/search/namespaced_class.yml b/.rubocop_todo/search/namespaced_class.yml
index 3d4dee458e4cd..fe367321fefd2 100644
--- a/.rubocop_todo/search/namespaced_class.yml
+++ b/.rubocop_todo/search/namespaced_class.yml
@@ -47,7 +47,6 @@ Search/NamespacedClass:
     - 'ee/app/workers/concerns/elastic/migration_obsolete.rb'
     - 'ee/app/workers/concerns/elastic/migration_options.rb'
     - 'ee/app/workers/concerns/elastic/migration_state.rb'
-    - 'ee/app/workers/concerns/elastic/migration_update_mappings_helper.rb'
     - 'ee/app/workers/elastic/migration_worker.rb'
     - 'ee/app/workers/elastic/namespace_update_worker.rb'
     - 'ee/app/workers/elastic/project_transfer_worker.rb'
diff --git a/doc/development/search/advanced_search_migration_styleguide.md b/doc/development/search/advanced_search_migration_styleguide.md
index a32d6fe8b84c1..778ae9f5b9cfe 100644
--- a/doc/development/search/advanced_search_migration_styleguide.md
+++ b/doc/development/search/advanced_search_migration_styleguide.md
@@ -101,7 +101,7 @@ To apply setting changes, for example adding an analyzer, either:
 To apply mapping changes, either:
 
 - Use a [zero-downtime reindexing migration](#zero-downtime-reindex-migration).
-- Use an [update mapping migration](#elasticmigrationupdatemappingshelper) to change the mapping for the existing index and optionally a follow-up [backfill migration](#searchelasticmigrationbackfillhelper) to ensure all documents in the index has this field populated.
+- Use an [update mapping migration](#searchelasticmigrationupdatemappingshelper) to change the mapping for the existing index and optionally a follow-up [backfill migration](#searchelasticmigrationbackfillhelper) to ensure all documents in the index has this field populated.
 
 #### Zero-downtime reindex migration
 
@@ -159,7 +159,7 @@ class MigrationName < Elastic::Migration
 end
 ```
 
-#### `Elastic::MigrationUpdateMappingsHelper`
+#### `Search::Elastic::MigrationUpdateMappingsHelper`
 
 Updates a mapping in an index by calling `put_mapping` with the mapping specified.
 
@@ -167,7 +167,7 @@ Requires the `new_mappings` method and `DOCUMENT_TYPE` constant.
 
 ```ruby
 class MigrationName < Elastic::Migration
-  include Elastic::MigrationUpdateMappingsHelper
+  include ::Search::Elastic::MigrationUpdateMappingsHelper
 
   DOCUMENT_TYPE = Issue
 
@@ -183,6 +183,14 @@ class MigrationName < Elastic::Migration
 end
 ```
 
+You can test this migration with the `'migration adds mapping'` shared examples.
+
+```ruby
+describe 'migration', :elastic, :sidekiq_inline do
+  include_examples 'migration adds mapping'
+end
+```
+
 #### `Search::Elastic::MigrationRemoveFieldsHelper`
 
 Removes specified fields from an index.
@@ -483,7 +491,7 @@ Use the following formula to calculate the runtime:
 Follow these best practices for best results:
 
 - Order all migrations for each document type so that any migrations that use
-  [`Elastic::MigrationUpdateMappingsHelper`](#elasticmigrationupdatemappingshelper)
+  [`Search::Elastic::MigrationUpdateMappingsHelper`](#searchelasticmigrationupdatemappingshelper)
   are executed before migrations that use the
   [`Search::Elastic::MigrationBackfillHelper`](#searchelasticmigrationbackfillhelper). This avoids
   reindexing the same documents multiple times if all of the migrations are unapplied
diff --git a/ee/app/workers/concerns/elastic/migration_update_mappings_helper.rb b/ee/app/workers/concerns/elastic/migration_update_mappings_helper.rb
deleted file mode 100644
index f8f669182dac5..0000000000000
--- a/ee/app/workers/concerns/elastic/migration_update_mappings_helper.rb
+++ /dev/null
@@ -1,40 +0,0 @@
-# frozen_string_literal: true
-
-module Elastic
-  module MigrationUpdateMappingsHelper
-    def migrate
-      if completed?
-        log "Skipping updating #{index_name} mapping migration since it is already applied"
-        return
-      end
-
-      log "Adding #{new_mappings.inspect} to #{index_name} mapping"
-      update_mapping!(index_name, { properties: new_mappings })
-    end
-
-    def completed?
-      helper.refresh_index(index_name: index_name)
-
-      mappings = helper.get_mapping(index_name: index_name)
-
-      # Check if mappings include all new_mappings
-      new_mappings.keys.map(&:to_s).to_set.subset?(mappings.keys.to_set)
-    end
-
-    private
-
-    def index_name
-      return self.class::DOCUMENT_TYPE.__elasticsearch__.index_name if self.class.const_defined?(:DOCUMENT_TYPE)
-
-      raise NotImplementedError
-    end
-
-    def new_mappings
-      raise NotImplementedError
-    end
-
-    def update_mapping!(index_name, mappings)
-      helper.update_mapping(index_name: index_name, mappings: mappings)
-    end
-  end
-end
diff --git a/ee/app/workers/concerns/search/elastic/migration_update_mappings_helper.rb b/ee/app/workers/concerns/search/elastic/migration_update_mappings_helper.rb
new file mode 100644
index 0000000000000..0239a29434893
--- /dev/null
+++ b/ee/app/workers/concerns/search/elastic/migration_update_mappings_helper.rb
@@ -0,0 +1,42 @@
+# frozen_string_literal: true
+
+module Search
+  module Elastic
+    module MigrationUpdateMappingsHelper
+      def migrate
+        if completed?
+          log "Skipping updating #{index_name} mapping migration since it is already applied"
+          return
+        end
+
+        log "Adding #{new_mappings.inspect} to #{index_name} mapping"
+        update_mapping!(index_name, { properties: new_mappings })
+      end
+
+      def completed?
+        helper.refresh_index(index_name: index_name)
+
+        mappings = helper.get_mapping(index_name: index_name)
+
+        # Check if mappings include all new_mappings
+        new_mappings.keys.map(&:to_s).to_set.subset?(mappings.keys.to_set)
+      end
+
+      private
+
+      def index_name
+        return self.class::DOCUMENT_TYPE.__elasticsearch__.index_name if self.class.const_defined?(:DOCUMENT_TYPE)
+
+        raise NotImplementedError
+      end
+
+      def new_mappings
+        raise NotImplementedError
+      end
+
+      def update_mapping!(index_name, mappings)
+        helper.update_mapping(index_name: index_name, mappings: mappings)
+      end
+    end
+  end
+end
diff --git a/ee/elastic/migrate/20231130152447_add_work_item_type_id_to_issues.rb b/ee/elastic/migrate/20231130152447_add_work_item_type_id_to_issues.rb
index da29ea56f5935..ba236f547a228 100644
--- a/ee/elastic/migrate/20231130152447_add_work_item_type_id_to_issues.rb
+++ b/ee/elastic/migrate/20231130152447_add_work_item_type_id_to_issues.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class AddWorkItemTypeIdToIssues < Elastic::Migration
-  include Elastic::MigrationUpdateMappingsHelper
+  include ::Search::Elastic::MigrationUpdateMappingsHelper
 
   private
 
diff --git a/ee/elastic/migrate/20240130154724_add_fields_to_projects_index.rb b/ee/elastic/migrate/20240130154724_add_fields_to_projects_index.rb
index 55518954756c7..35c968243a361 100644
--- a/ee/elastic/migrate/20240130154724_add_fields_to_projects_index.rb
+++ b/ee/elastic/migrate/20240130154724_add_fields_to_projects_index.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class AddFieldsToProjectsIndex < Elastic::Migration
-  include Elastic::MigrationUpdateMappingsHelper
+  include ::Search::Elastic::MigrationUpdateMappingsHelper
 
   DOCUMENT_TYPE = Project
 
diff --git a/ee/elastic/migrate/20240208160152_add_count_fields_to_projects.rb b/ee/elastic/migrate/20240208160152_add_count_fields_to_projects.rb
index 7d247ac4b22b6..e44e660a2988f 100644
--- a/ee/elastic/migrate/20240208160152_add_count_fields_to_projects.rb
+++ b/ee/elastic/migrate/20240208160152_add_count_fields_to_projects.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class AddCountFieldsToProjects < Elastic::Migration
-  include Elastic::MigrationUpdateMappingsHelper
+  include ::Search::Elastic::MigrationUpdateMappingsHelper
 
   DOCUMENT_TYPE = Project
 
diff --git a/ee/elastic/migrate/20240410193847_add_embedding_to_issues.rb b/ee/elastic/migrate/20240410193847_add_embedding_to_issues.rb
index 85222e5ac066f..b3af98b371559 100644
--- a/ee/elastic/migrate/20240410193847_add_embedding_to_issues.rb
+++ b/ee/elastic/migrate/20240410193847_add_embedding_to_issues.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class AddEmbeddingToIssues < Elastic::Migration
-  include Elastic::MigrationUpdateMappingsHelper
+  include ::Search::Elastic::MigrationUpdateMappingsHelper
 
   skip_if -> { !Gitlab::Elastic::Helper.default.vectors_supported?(:elasticsearch) }
 
diff --git a/ee/elastic/migrate/20240517092224_add_routing_to_issues.rb b/ee/elastic/migrate/20240517092224_add_routing_to_issues.rb
index ff8fe2dfe97ca..15fe1b20cee62 100644
--- a/ee/elastic/migrate/20240517092224_add_routing_to_issues.rb
+++ b/ee/elastic/migrate/20240517092224_add_routing_to_issues.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class AddRoutingToIssues < Elastic::Migration
-  include Elastic::MigrationUpdateMappingsHelper
+  include ::Search::Elastic::MigrationUpdateMappingsHelper
 
   DOCUMENT_TYPE = Issue
 
diff --git a/ee/elastic/migrate/20240626145458_add_root_namespace_id_to_work_item.rb b/ee/elastic/migrate/20240626145458_add_root_namespace_id_to_work_item.rb
index e17929a2b770e..29781a7797ff2 100644
--- a/ee/elastic/migrate/20240626145458_add_root_namespace_id_to_work_item.rb
+++ b/ee/elastic/migrate/20240626145458_add_root_namespace_id_to_work_item.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class AddRootNamespaceIdToWorkItem < Elastic::Migration
-  include Elastic::MigrationUpdateMappingsHelper
+  include ::Search::Elastic::MigrationUpdateMappingsHelper
 
   private
 
diff --git a/ee/elastic/migrate/20240704125425_add_label_ids_to_merge_request.rb b/ee/elastic/migrate/20240704125425_add_label_ids_to_merge_request.rb
index 9f8e3d8eddda1..03a862bc20158 100644
--- a/ee/elastic/migrate/20240704125425_add_label_ids_to_merge_request.rb
+++ b/ee/elastic/migrate/20240704125425_add_label_ids_to_merge_request.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class AddLabelIdsToMergeRequest < Elastic::Migration
-  include Elastic::MigrationUpdateMappingsHelper
+  include ::Search::Elastic::MigrationUpdateMappingsHelper
 
   DOCUMENT_TYPE = MergeRequest
 
diff --git a/ee/elastic/migrate/20240814223217_add_issues_access_level_in_work_item_index.rb b/ee/elastic/migrate/20240814223217_add_issues_access_level_in_work_item_index.rb
index 7596a4a0c92ff..9814103793b51 100644
--- a/ee/elastic/migrate/20240814223217_add_issues_access_level_in_work_item_index.rb
+++ b/ee/elastic/migrate/20240814223217_add_issues_access_level_in_work_item_index.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class AddIssuesAccessLevelInWorkItemIndex < Elastic::Migration
-  include Elastic::MigrationUpdateMappingsHelper
+  include ::Search::Elastic::MigrationUpdateMappingsHelper
 
   private
 
diff --git a/ee/elastic/migrate/20241003142503_add_embedding_to_work_items.rb b/ee/elastic/migrate/20241003142503_add_embedding_to_work_items.rb
index dffedeb5f937d..d70465229181d 100644
--- a/ee/elastic/migrate/20241003142503_add_embedding_to_work_items.rb
+++ b/ee/elastic/migrate/20241003142503_add_embedding_to_work_items.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class AddEmbeddingToWorkItems < Elastic::Migration
-  include Elastic::MigrationUpdateMappingsHelper
+  include ::Search::Elastic::MigrationUpdateMappingsHelper
 
   skip_if -> { !elasticsearch_8_plus? }
 
diff --git a/ee/elastic/migrate/20241003151804_add_notes_to_work_items.rb b/ee/elastic/migrate/20241003151804_add_notes_to_work_items.rb
index 42dbae37d0087..a84584c0c37ca 100644
--- a/ee/elastic/migrate/20241003151804_add_notes_to_work_items.rb
+++ b/ee/elastic/migrate/20241003151804_add_notes_to_work_items.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class AddNotesToWorkItems < Elastic::Migration
-  include Elastic::MigrationUpdateMappingsHelper
+  include ::Search::Elastic::MigrationUpdateMappingsHelper
 
   private
 
diff --git a/ee/elastic/migrate/20241105111645_add_work_item_type_correct_id.rb b/ee/elastic/migrate/20241105111645_add_work_item_type_correct_id.rb
index de0449ce0bfc1..5474e9c6a7c16 100644
--- a/ee/elastic/migrate/20241105111645_add_work_item_type_correct_id.rb
+++ b/ee/elastic/migrate/20241105111645_add_work_item_type_correct_id.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class AddWorkItemTypeCorrectId < Elastic::Migration
-  include Elastic::MigrationUpdateMappingsHelper
+  include ::Search::Elastic::MigrationUpdateMappingsHelper
 
   DOCUMENT_TYPE = WorkItem
 
diff --git a/ee/elastic/migrate/20241107131942_add_traversal_ids_to_merge_requests.rb b/ee/elastic/migrate/20241107131942_add_traversal_ids_to_merge_requests.rb
index 8b1e39e1d20ea..1da209bf4268e 100644
--- a/ee/elastic/migrate/20241107131942_add_traversal_ids_to_merge_requests.rb
+++ b/ee/elastic/migrate/20241107131942_add_traversal_ids_to_merge_requests.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class AddTraversalIdsToMergeRequests < Elastic::Migration
-  include Elastic::MigrationUpdateMappingsHelper
+  include ::Search::Elastic::MigrationUpdateMappingsHelper
 
   DOCUMENT_TYPE = MergeRequest
 
diff --git a/ee/elastic/migrate/20241204115350_add_assignees_to_merge_requests.rb b/ee/elastic/migrate/20241204115350_add_assignees_to_merge_requests.rb
index bc51bab7e6a52..97743d471aad0 100644
--- a/ee/elastic/migrate/20241204115350_add_assignees_to_merge_requests.rb
+++ b/ee/elastic/migrate/20241204115350_add_assignees_to_merge_requests.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class AddAssigneesToMergeRequests < Elastic::Migration
-  include Elastic::MigrationUpdateMappingsHelper
+  include ::Search::Elastic::MigrationUpdateMappingsHelper
 
   DOCUMENT_TYPE = MergeRequest
 
diff --git a/ee/spec/services/elastic/data_migration_service_spec.rb b/ee/spec/services/elastic/data_migration_service_spec.rb
index 867d0324140cd..cb263d6fe914f 100644
--- a/ee/spec/services/elastic/data_migration_service_spec.rb
+++ b/ee/spec/services/elastic/data_migration_service_spec.rb
@@ -67,7 +67,7 @@
     describe 'migrations order optimization' do
       it 'ensure all update migrations run before backfill migrations', :aggregate_failures do
         error_message = <<~DOC
-          Migrations should be ordered so all migrations that use ::Elastic::MigrationUpdateMappingsHelper
+          Migrations should be ordered so all migrations that use ::Search::Elastic::MigrationUpdateMappingsHelper
           run before any migrations that use ::Search::Elastic::MigrationBackfillHelper. If this spec fails, rename the
           `YYYYMMDDHHMMSS` part of the migration filename with a datetime before the last backfill migration for the
           index_name.
@@ -78,7 +78,7 @@
 
         filtered_migrations = non_obsolete_migrations.filter do |m|
           klass = m.class
-          klass.include?(::Elastic::MigrationUpdateMappingsHelper) ||
+          klass.include?(::Search::Elastic::MigrationUpdateMappingsHelper) ||
             klass.include?(::Search::Elastic::MigrationBackfillHelper)
         end
 
@@ -90,7 +90,7 @@
           end.map(&:version)
 
           mapping_versions = non_obsolete_migrations.filter do |m|
-            m.class.include?(::Elastic::MigrationUpdateMappingsHelper) && m.send(:index_name) == index_name
+            m.class.include?(::Search::Elastic::MigrationUpdateMappingsHelper) && m.send(:index_name) == index_name
           end.map(&:version)
 
           backfill_ranges = backfill_versions.each_cons(2).map { |a, b| a..b }
diff --git a/spec/fixtures/migrations/elasticsearch/elasticsearch_migration.txt b/spec/fixtures/migrations/elasticsearch/elasticsearch_migration.txt
index 122d3ef6d2b36..2418ee88ffd08 100644
--- a/spec/fixtures/migrations/elasticsearch/elasticsearch_migration.txt
+++ b/spec/fixtures/migrations/elasticsearch/elasticsearch_migration.txt
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class ElasticsearchMigration < Elastic::Migration
-    include Elastic::MigrationUpdateMappingsHelper
+    include ::Search::Elastic::MigrationUpdateMappingsHelper
 
     private
 
-- 
GitLab