From c3e3bf2a736dd6e9597e8e217bb0870d282d2e39 Mon Sep 17 00:00:00 2001
From: Dmitry Gruzd <dgruzd@gitlab.com>
Date: Wed, 13 Dec 2023 18:16:43 +0000
Subject: [PATCH] Refactor some of our elastic helpers and specs
---
.../advanced_search_migration_styleguide.md | 24 +++++++------------
.../elastic/migration_backfill_helper.rb | 2 ++
.../migration_update_mappings_helper.rb | 2 ++
...kfill_hashed_root_namespace_id_on_notes.rb | 4 ----
...hed_root_namespace_id_to_merge_requests.rb | 6 ++---
..._add_hashed_root_namespace_id_to_issues.rb | 6 ++---
...fill_hashed_root_namespace_id_on_issues.rb | 4 ----
...hed_root_namespace_id_on_merge_requests.rb | 4 ----
...0426195404_add_hidden_to_merge_requests.rb | 6 ++---
...55555_backfill_hidden_on_merge_requests.rb | 4 ----
...0230519142363_add_ci_catalog_to_project.rb | 6 ++---
...tone_permissions_to_milestone_documents.rb | 4 ----
.../20230628094243_add_archived_to_issues.rb | 6 ++---
...30628094700_backfill_archived_on_issues.rb | 4 ----
...10142700_add_archived_to_merge_requests.rb | 6 ++---
...500_backfill_archived_on_merge_requests.rb | 4 ----
...7132609_backfill_archived_on_work_items.rb | 4 ----
.../20230719142200_add_archived_to_notes.rb | 6 ++---
...230722212041_backfill_archived_on_notes.rb | 4 ----
...049_add_schema_version_to_merge_request.rb | 6 ++---
...230824114205_add_schema_version_to_note.rb | 6 ++---
...1152648_backfill_archived_on_milestones.rb | 4 ----
.../elastic/latest/project_instance_proxy.rb | 2 +-
.../latest/project_instance_proxy_spec.rb | 17 ++++++-------
.../models/concerns/elastic/project_spec.rb | 6 +++--
25 files changed, 45 insertions(+), 102 deletions(-)
diff --git a/doc/development/search/advanced_search_migration_styleguide.md b/doc/development/search/advanced_search_migration_styleguide.md
index 0ad3b5dc7c9d..87083d1a36f7 100644
--- a/doc/development/search/advanced_search_migration_styleguide.md
+++ b/doc/development/search/advanced_search_migration_styleguide.md
@@ -66,17 +66,15 @@ The following migration helpers are available in `ee/app/workers/concerns/elasti
Backfills a specific field in an index. In most cases, the mapping for the field should already be added.
-Requires the `index_name` and `field_name` methods to backfill a single field.
+Requires the `field_name` method and `DOCUMENT_TYPE` constant to backfill a single field.
```ruby
class MigrationName < Elastic::Migration
include Elastic::MigrationBackfillHelper
- private
+ DOCUMENT_TYPE = Issue
- def index_name
- Issue.__elasticsearch__.index_name
- end
+ private
def field_name
:schema_version
@@ -84,17 +82,15 @@ class MigrationName < Elastic::Migration
end
```
-Requires the `index_name` and `field_names` methods to backfill multiple fields if any field is null.
+Requires the `field_names` method and `DOCUMENT_TYPE` constant to backfill multiple fields if any field is null.
```ruby
class MigrationName < Elastic::Migration
include Elastic::MigrationBackfillHelper
- private
+ DOCUMENT_TYPE = Issue
- def index_name
- Issue.__elasticsearch__.index_name
- end
+ private
def field_names
%w[schema_version visibility_level]
@@ -106,17 +102,15 @@ end
Updates a mapping in an index by calling `put_mapping` with the mapping specified.
-Requires the `index_name` and `new_mappings` methods.
+Requires the `new_mappings` method and `DOCUMENT_TYPE` constant.
```ruby
class MigrationName < Elastic::Migration
include Elastic::MigrationUpdateMappingsHelper
- private
+ DOCUMENT_TYPE = Issue
- def index_name
- Issue.__elasticsearch__.index_name
- end
+ private
def new_mappings
{
diff --git a/ee/app/workers/concerns/elastic/migration_backfill_helper.rb b/ee/app/workers/concerns/elastic/migration_backfill_helper.rb
index 9a1fcdee5a61..8bca7aa6e550 100644
--- a/ee/app/workers/concerns/elastic/migration_backfill_helper.rb
+++ b/ee/app/workers/concerns/elastic/migration_backfill_helper.rb
@@ -31,6 +31,8 @@ def completed?
private
def index_name
+ return self.class::DOCUMENT_TYPE.__elasticsearch__.index_name if self.class.const_defined?(:DOCUMENT_TYPE)
+
raise NotImplementedError
end
diff --git a/ee/app/workers/concerns/elastic/migration_update_mappings_helper.rb b/ee/app/workers/concerns/elastic/migration_update_mappings_helper.rb
index c72eda44ad7b..f8f669182dac 100644
--- a/ee/app/workers/concerns/elastic/migration_update_mappings_helper.rb
+++ b/ee/app/workers/concerns/elastic/migration_update_mappings_helper.rb
@@ -24,6 +24,8 @@ def completed?
private
def index_name
+ return self.class::DOCUMENT_TYPE.__elasticsearch__.index_name if self.class.const_defined?(:DOCUMENT_TYPE)
+
raise NotImplementedError
end
diff --git a/ee/elastic/migrate/20230307102400_backfill_hashed_root_namespace_id_on_notes.rb b/ee/elastic/migrate/20230307102400_backfill_hashed_root_namespace_id_on_notes.rb
index 98d9b3b3937d..9ebfd4e648f4 100644
--- a/ee/elastic/migrate/20230307102400_backfill_hashed_root_namespace_id_on_notes.rb
+++ b/ee/elastic/migrate/20230307102400_backfill_hashed_root_namespace_id_on_notes.rb
@@ -12,10 +12,6 @@ class BackfillHashedRootNamespaceIdOnNotes < Elastic::Migration
private
- def index_name
- DOCUMENT_TYPE.__elasticsearch__.index_name
- end
-
def field_name
'hashed_root_namespace_id'
end
diff --git a/ee/elastic/migrate/20230316150000_add_hashed_root_namespace_id_to_merge_requests.rb b/ee/elastic/migrate/20230316150000_add_hashed_root_namespace_id_to_merge_requests.rb
index f3b5ee3edf77..8a11197321b7 100644
--- a/ee/elastic/migrate/20230316150000_add_hashed_root_namespace_id_to_merge_requests.rb
+++ b/ee/elastic/migrate/20230316150000_add_hashed_root_namespace_id_to_merge_requests.rb
@@ -3,11 +3,9 @@
class AddHashedRootNamespaceIdToMergeRequests < Elastic::Migration
include Elastic::MigrationUpdateMappingsHelper
- private
+ DOCUMENT_TYPE = MergeRequest
- def index_name
- MergeRequest.__elasticsearch__.index_name
- end
+ private
def new_mappings
{
diff --git a/ee/elastic/migrate/20230317120500_add_hashed_root_namespace_id_to_issues.rb b/ee/elastic/migrate/20230317120500_add_hashed_root_namespace_id_to_issues.rb
index 17625bf62699..7033701ecca9 100644
--- a/ee/elastic/migrate/20230317120500_add_hashed_root_namespace_id_to_issues.rb
+++ b/ee/elastic/migrate/20230317120500_add_hashed_root_namespace_id_to_issues.rb
@@ -3,11 +3,9 @@
class AddHashedRootNamespaceIdToIssues < Elastic::Migration
include Elastic::MigrationUpdateMappingsHelper
- private
+ DOCUMENT_TYPE = Issue
- def index_name
- Issue.__elasticsearch__.index_name
- end
+ private
def new_mappings
{
diff --git a/ee/elastic/migrate/20230321091100_backfill_hashed_root_namespace_id_on_issues.rb b/ee/elastic/migrate/20230321091100_backfill_hashed_root_namespace_id_on_issues.rb
index 627d0181a1cf..ed04d42f43b0 100644
--- a/ee/elastic/migrate/20230321091100_backfill_hashed_root_namespace_id_on_issues.rb
+++ b/ee/elastic/migrate/20230321091100_backfill_hashed_root_namespace_id_on_issues.rb
@@ -12,10 +12,6 @@ class BackfillHashedRootNamespaceIdOnIssues < Elastic::Migration
private
- def index_name
- DOCUMENT_TYPE.__elasticsearch__.index_name
- end
-
def field_name
'hashed_root_namespace_id'
end
diff --git a/ee/elastic/migrate/20230321202400_backfill_hashed_root_namespace_id_on_merge_requests.rb b/ee/elastic/migrate/20230321202400_backfill_hashed_root_namespace_id_on_merge_requests.rb
index 1c98d12be0db..0fcfca4d25f7 100644
--- a/ee/elastic/migrate/20230321202400_backfill_hashed_root_namespace_id_on_merge_requests.rb
+++ b/ee/elastic/migrate/20230321202400_backfill_hashed_root_namespace_id_on_merge_requests.rb
@@ -12,10 +12,6 @@ class BackfillHashedRootNamespaceIdOnMergeRequests < Elastic::Migration
private
- def index_name
- DOCUMENT_TYPE.__elasticsearch__.index_name
- end
-
def field_name
'hashed_root_namespace_id'
end
diff --git a/ee/elastic/migrate/20230426195404_add_hidden_to_merge_requests.rb b/ee/elastic/migrate/20230426195404_add_hidden_to_merge_requests.rb
index 4ad1fa077ed3..729c11c3b008 100644
--- a/ee/elastic/migrate/20230426195404_add_hidden_to_merge_requests.rb
+++ b/ee/elastic/migrate/20230426195404_add_hidden_to_merge_requests.rb
@@ -3,11 +3,9 @@
class AddHiddenToMergeRequests < Elastic::Migration
include Elastic::MigrationUpdateMappingsHelper
- private
+ DOCUMENT_TYPE = MergeRequest
- def index_name
- MergeRequest.__elasticsearch__.index_name
- end
+ private
def new_mappings
{
diff --git a/ee/elastic/migrate/20230427555555_backfill_hidden_on_merge_requests.rb b/ee/elastic/migrate/20230427555555_backfill_hidden_on_merge_requests.rb
index dd4acc911dca..657c6d2ac713 100644
--- a/ee/elastic/migrate/20230427555555_backfill_hidden_on_merge_requests.rb
+++ b/ee/elastic/migrate/20230427555555_backfill_hidden_on_merge_requests.rb
@@ -12,10 +12,6 @@ class BackfillHiddenOnMergeRequests < Elastic::Migration
private
- def index_name
- DOCUMENT_TYPE.__elasticsearch__.index_name
- end
-
def field_name
:hidden
end
diff --git a/ee/elastic/migrate/20230519142363_add_ci_catalog_to_project.rb b/ee/elastic/migrate/20230519142363_add_ci_catalog_to_project.rb
index 0b533bc5ab95..4cda3c6f9119 100644
--- a/ee/elastic/migrate/20230519142363_add_ci_catalog_to_project.rb
+++ b/ee/elastic/migrate/20230519142363_add_ci_catalog_to_project.rb
@@ -3,11 +3,9 @@
class AddCiCatalogToProject < Elastic::Migration
include Elastic::MigrationUpdateMappingsHelper
- private
+ DOCUMENT_TYPE = Project
- def index_name
- Project.__elasticsearch__.index_name
- end
+ private
def new_mappings
{
diff --git a/ee/elastic/migrate/20230607500000_backfill_milestone_permissions_to_milestone_documents.rb b/ee/elastic/migrate/20230607500000_backfill_milestone_permissions_to_milestone_documents.rb
index dd400b2ce5e0..6e48d27ebf62 100644
--- a/ee/elastic/migrate/20230607500000_backfill_milestone_permissions_to_milestone_documents.rb
+++ b/ee/elastic/migrate/20230607500000_backfill_milestone_permissions_to_milestone_documents.rb
@@ -11,10 +11,6 @@ class BackfillMilestonePermissionsToMilestoneDocuments < Elastic::Migration
private
- def index_name
- DOCUMENT_TYPE.__elasticsearch__.index_name
- end
-
def field_names
%w[merge_requests_access_level issues_access_level visibility_level]
end
diff --git a/ee/elastic/migrate/20230628094243_add_archived_to_issues.rb b/ee/elastic/migrate/20230628094243_add_archived_to_issues.rb
index ec1c9d0047f2..1f339c6858e7 100644
--- a/ee/elastic/migrate/20230628094243_add_archived_to_issues.rb
+++ b/ee/elastic/migrate/20230628094243_add_archived_to_issues.rb
@@ -3,11 +3,9 @@
class AddArchivedToIssues < Elastic::Migration
include Elastic::MigrationUpdateMappingsHelper
- private
+ DOCUMENT_TYPE = Issue
- def index_name
- Issue.__elasticsearch__.index_name
- end
+ private
def new_mappings
{
diff --git a/ee/elastic/migrate/20230628094700_backfill_archived_on_issues.rb b/ee/elastic/migrate/20230628094700_backfill_archived_on_issues.rb
index dc867f606aef..deffa770dff8 100644
--- a/ee/elastic/migrate/20230628094700_backfill_archived_on_issues.rb
+++ b/ee/elastic/migrate/20230628094700_backfill_archived_on_issues.rb
@@ -12,10 +12,6 @@ class BackfillArchivedOnIssues < Elastic::Migration
private
- def index_name
- DOCUMENT_TYPE.__elasticsearch__.index_name
- end
-
def field_name
'archived'
end
diff --git a/ee/elastic/migrate/20230710142700_add_archived_to_merge_requests.rb b/ee/elastic/migrate/20230710142700_add_archived_to_merge_requests.rb
index f79e4aff69de..edee1bbc63f2 100644
--- a/ee/elastic/migrate/20230710142700_add_archived_to_merge_requests.rb
+++ b/ee/elastic/migrate/20230710142700_add_archived_to_merge_requests.rb
@@ -3,11 +3,9 @@
class AddArchivedToMergeRequests < Elastic::Migration
include Elastic::MigrationUpdateMappingsHelper
- private
+ DOCUMENT_TYPE = MergeRequest
- def index_name
- MergeRequest.__elasticsearch__.index_name
- end
+ private
def new_mappings
{
diff --git a/ee/elastic/migrate/20230711140500_backfill_archived_on_merge_requests.rb b/ee/elastic/migrate/20230711140500_backfill_archived_on_merge_requests.rb
index 93643cdd0f5e..f6e5d50b1f18 100644
--- a/ee/elastic/migrate/20230711140500_backfill_archived_on_merge_requests.rb
+++ b/ee/elastic/migrate/20230711140500_backfill_archived_on_merge_requests.rb
@@ -12,10 +12,6 @@ class BackfillArchivedOnMergeRequests < Elastic::Migration
private
- def index_name
- DOCUMENT_TYPE.__elasticsearch__.index_name
- end
-
def field_name
'archived'
end
diff --git a/ee/elastic/migrate/20230717132609_backfill_archived_on_work_items.rb b/ee/elastic/migrate/20230717132609_backfill_archived_on_work_items.rb
index 8f97bd5120db..f02bddc7443b 100644
--- a/ee/elastic/migrate/20230717132609_backfill_archived_on_work_items.rb
+++ b/ee/elastic/migrate/20230717132609_backfill_archived_on_work_items.rb
@@ -12,10 +12,6 @@ class BackfillArchivedOnWorkItems < Elastic::Migration
private
- def index_name
- DOCUMENT_TYPE.__elasticsearch__.index_name
- end
-
def field_name
'archived'
end
diff --git a/ee/elastic/migrate/20230719142200_add_archived_to_notes.rb b/ee/elastic/migrate/20230719142200_add_archived_to_notes.rb
index 21509e4c0f5e..40a8af68395e 100644
--- a/ee/elastic/migrate/20230719142200_add_archived_to_notes.rb
+++ b/ee/elastic/migrate/20230719142200_add_archived_to_notes.rb
@@ -3,11 +3,9 @@
class AddArchivedToNotes < Elastic::Migration
include Elastic::MigrationUpdateMappingsHelper
- private
+ DOCUMENT_TYPE = Note
- def index_name
- ::Elastic::Latest::NoteConfig.index_name
- end
+ private
def new_mappings
{ archived: { type: 'boolean' } }
diff --git a/ee/elastic/migrate/20230722212041_backfill_archived_on_notes.rb b/ee/elastic/migrate/20230722212041_backfill_archived_on_notes.rb
index 514877cee3fa..91443669408c 100644
--- a/ee/elastic/migrate/20230722212041_backfill_archived_on_notes.rb
+++ b/ee/elastic/migrate/20230722212041_backfill_archived_on_notes.rb
@@ -11,10 +11,6 @@ class BackfillArchivedOnNotes < Elastic::Migration
private
- def index_name
- DOCUMENT_TYPE.__elasticsearch__.index_name
- end
-
def field_name
'archived'
end
diff --git a/ee/elastic/migrate/20230823154049_add_schema_version_to_merge_request.rb b/ee/elastic/migrate/20230823154049_add_schema_version_to_merge_request.rb
index 8bf7f5ecf5e1..0d54655b70fe 100644
--- a/ee/elastic/migrate/20230823154049_add_schema_version_to_merge_request.rb
+++ b/ee/elastic/migrate/20230823154049_add_schema_version_to_merge_request.rb
@@ -3,11 +3,9 @@
class AddSchemaVersionToMergeRequest < Elastic::Migration
include Elastic::MigrationUpdateMappingsHelper
- private
+ DOCUMENT_TYPE = MergeRequest
- def index_name
- ::Elastic::Latest::MergeRequestConfig.index_name
- end
+ private
def new_mappings
{
diff --git a/ee/elastic/migrate/20230824114205_add_schema_version_to_note.rb b/ee/elastic/migrate/20230824114205_add_schema_version_to_note.rb
index 9b00e2027d0e..efc0b4fbe75e 100644
--- a/ee/elastic/migrate/20230824114205_add_schema_version_to_note.rb
+++ b/ee/elastic/migrate/20230824114205_add_schema_version_to_note.rb
@@ -3,11 +3,9 @@
class AddSchemaVersionToNote < Elastic::Migration
include Elastic::MigrationUpdateMappingsHelper
- private
+ DOCUMENT_TYPE = Note
- def index_name
- ::Elastic::Latest::NoteConfig.index_name
- end
+ private
def new_mappings
{
diff --git a/ee/elastic/migrate/20230831152648_backfill_archived_on_milestones.rb b/ee/elastic/migrate/20230831152648_backfill_archived_on_milestones.rb
index a3d0569925b2..c712078d5fe7 100644
--- a/ee/elastic/migrate/20230831152648_backfill_archived_on_milestones.rb
+++ b/ee/elastic/migrate/20230831152648_backfill_archived_on_milestones.rb
@@ -11,10 +11,6 @@ class BackfillArchivedOnMilestones < Elastic::Migration
private
- def index_name
- DOCUMENT_TYPE.__elasticsearch__.index_name
- end
-
def field_name
'archived'
end
diff --git a/ee/lib/elastic/latest/project_instance_proxy.rb b/ee/lib/elastic/latest/project_instance_proxy.rb
index 7598ca0880ee..7d3e4121ebfe 100644
--- a/ee/lib/elastic/latest/project_instance_proxy.rb
+++ b/ee/lib/elastic/latest/project_instance_proxy.rb
@@ -11,7 +11,7 @@ class ProjectInstanceProxy < ApplicationInstanceProxy
repository_access_level
].freeze
- def as_indexed_json(options = {})
+ def as_indexed_json(_options = {})
# We don't use as_json(only: ...) because it calls all virtual and serialized attributes
# https://gitlab.com/gitlab-org/gitlab/issues/349
data = {}
diff --git a/ee/spec/lib/elastic/latest/project_instance_proxy_spec.rb b/ee/spec/lib/elastic/latest/project_instance_proxy_spec.rb
index f9b8440c2985..5c680e319ef1 100644
--- a/ee/spec/lib/elastic/latest/project_instance_proxy_spec.rb
+++ b/ee/spec/lib/elastic/latest/project_instance_proxy_spec.rb
@@ -2,11 +2,12 @@
require 'spec_helper'
-RSpec.describe Elastic::Latest::ProjectInstanceProxy, feature_category: :global_search do
- include ElasticsearchHelpers
+RSpec.describe Elastic::Latest::ProjectInstanceProxy, :elastic_helpers, feature_category: :global_search do
let_it_be(:project) { create(:project) }
- subject { described_class.new(project) }
+ let(:schema_version) { 2306 }
+
+ subject(:proxy) { described_class.new(project) }
describe 'when migrate_projects_to_separate_index migration is not completed' do
before do
@@ -17,7 +18,7 @@
describe '#as_indexed_json' do
it 'serializes project as hash' do
- result = subject.as_indexed_json.with_indifferent_access
+ result = proxy.as_indexed_json.with_indifferent_access
expect(result).to include(
id: project.id,
@@ -44,7 +45,7 @@
end
it 'sets all tracked feature access levels to PRIVATE' do
- result = subject.as_indexed_json.with_indifferent_access
+ result = proxy.as_indexed_json.with_indifferent_access
Elastic::Latest::ProjectInstanceProxy::TRACKED_FEATURE_SETTINGS.each do |feature|
expect(result).to include(feature => ProjectFeature::PRIVATE) # rubocop:disable GitlabSecurity/PublicSend
@@ -63,7 +64,7 @@
describe '#as_indexed_json' do
it 'serializes project as hash' do
- result = subject.as_indexed_json.with_indifferent_access
+ result = proxy.as_indexed_json.with_indifferent_access
expect(result).to include(
id: project.id,
@@ -80,13 +81,13 @@
traversal_ids: project.elastic_namespace_ancestry,
type: 'project',
visibility_level: project.visibility_level,
- schema_version: 23_06,
+ schema_version: schema_version,
ci_catalog: project.catalog_resource.present?
)
end
it 'contains the expected mappings' do
- result = subject.as_indexed_json.with_indifferent_access.keys
+ result = proxy.as_indexed_json.with_indifferent_access.keys
project_proxy = Elastic::Latest::ApplicationClassProxy.new(Project, use_separate_indices: true)
# readme_content is not populated by as_indexed_json
expected_keys = project_proxy.mappings.to_hash[:properties].keys.map(&:to_s) - ['readme_content']
diff --git a/ee/spec/models/concerns/elastic/project_spec.rb b/ee/spec/models/concerns/elastic/project_spec.rb
index 00308ca2729c..32a580656a2f 100644
--- a/ee/spec/models/concerns/elastic/project_spec.rb
+++ b/ee/spec/models/concerns/elastic/project_spec.rb
@@ -7,6 +7,8 @@
stub_ee_application_setting(elasticsearch_search: true, elasticsearch_indexing: true)
end
+ let(:schema_version) { 2306 }
+
context 'when limited indexing is on' do
let_it_be(:project) { create(:project, :empty_repo, name: 'main_project') }
@@ -275,7 +277,7 @@
'ci_catalog' => project.catalog_resource.present?,
'join_field' => project.es_type,
'type' => project.es_type,
- 'schema_version' => 2306,
+ 'schema_version' => schema_version,
'traversal_ids' => project.elastic_namespace_ancestry,
'name_with_namespace' => project.full_name,
'path_with_namespace' => project.full_path
@@ -316,7 +318,7 @@
).merge({
'ci_catalog' => project.catalog_resource.present?,
'type' => project.es_type,
- 'schema_version' => 2306,
+ 'schema_version' => schema_version,
'traversal_ids' => project.elastic_namespace_ancestry,
'name_with_namespace' => project.full_name,
'path_with_namespace' => project.full_path
--
GitLab