Merge branch 'create-use_separate_indices-method' into 'master'

Remove hack to set index_name for wikis

See merge request !121442



Merged-by: Dmitry Gruzd <dgruzd@gitlab.com>
Approved-by: Siddharth Dungarwal <sdungarwal@gitlab.com>
Approved-by: Dmitry Gruzd <dgruzd@gitlab.com>
Co-authored-by: rkumar555 <rkumar@gitlab.com>

ee/app/models/ee/project_wiki.rb

@@ -17,5 +17,15 @@ def after_wiki_activity
 
       project.repository_state&.touch(:last_wiki_updated_at)
     end
+
+    # TODO: This method may be removed once we implement Group Wikis
+    class_methods do
+      extend ::Gitlab::Utils::Override
+
+      override :use_separate_indices?
+      def use_separate_indices?
+        ::Elastic::DataMigrationService.migration_has_finished?(:migrate_wikis_to_separate_index)
+      end
+    end
   end
 end

ee/app/models/ee/wiki.rb

@@ -9,5 +9,21 @@ module Wiki
     def kerberos_url_to_repo
       [::Gitlab.config.build_gitlab_kerberos_url, '/', full_path, '.git'].join('')
     end
+
+    class_methods do
+      extend ::Gitlab::Utils::Override
+      def base_class
+        ::Wiki
+      end
+
+      def use_separate_indices?
+        ::Elastic::DataMigrationService.migration_has_finished?(:migrate_wikis_to_separate_index)
+      end
+
+      # This method might be removed once the feature_flag use_base_class_in_proxy_util is fully rolled out
+      def abstract_class?
+        false
+      end
+    end
   end
 end

ee/config/feature_flags/development/use_base_class_in_proxy_util.yml

+---
+name: use_base_class_in_proxy_util
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/121442
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/412706
+milestone: '16.1'
+type: development
+group: group::global search
+default_enabled: false

ee/lib/elastic/class_proxy_util.rb

@@ -11,16 +11,20 @@ module ClassProxyUtil
     def initialize(target, use_separate_indices: false)
       super(target)
 
-      const_name = if use_separate_indices
-                     # This check is needed to load custom mappings for AR and non-AR classes
-                     # For example, Issue and Commit
-                     if target.superclass == Object || target.superclass.abstract_class?
-                       "#{target.name}Config"
+      # use_separate_indices check is needed to load custom mappings for AR and non-AR classes
+      # For example, Issue and Commit
+      const_name = if Feature.disabled?(:use_base_class_in_proxy_util)
+                     if use_separate_indices
+                       if target.superclass == Object || target.superclass.abstract_class?
+                         "#{target.name}Config"
+                       else
+                         "#{target.superclass.name}Config"
+                       end
                      else
-                       "#{target.superclass.name}Config"
+                       'Config'
                      end
                    else
-                     'Config'
+                     use_separate_indices ? "#{target.base_class}Config" : 'Config'
                    end
 
       config = version_namespace.const_get(const_name, false)

ee/lib/elastic/instance_proxy_util.rb

@@ -9,14 +9,18 @@ module InstanceProxyUtil
     def initialize(target, use_separate_indices: false)
       super(target)
 
-      const_name = if use_separate_indices
-                     if target.class.superclass.abstract_class?
-                       "#{target.class.name}Config"
+      const_name = if Feature.disabled?(:use_base_class_in_proxy_util)
+                     if use_separate_indices
+                       if target.class.superclass.abstract_class?
+                         "#{target.class.name}Config"
+                       else
+                         "#{target.class.superclass.name}Config"
+                       end
                      else
-                       "#{target.class.superclass.name}Config"
+                       'Config'
                      end
                    else
-                     'Config'
+                     use_separate_indices ? "#{target.class.base_class}Config" : 'Config'
                    end
 
       config = version_namespace.const_get(const_name, false)

ee/lib/elastic/latest/git_class_proxy.rb

@@ -280,9 +280,9 @@ def blob_query(query, type: 'blob', page: 1, per: 20, options: {})
           options[:no_join_project] = Elastic::DataMigrationService.migration_has_finished?(:backfill_wiki_permissions_in_main_index)
         end
 
-        if use_separate_wiki_index? options[:features]
+        if options[:features].eql?('wiki') && use_separate_wiki_index?
           fields = %w[content file_name path]
-          options[:index_name] = Elastic::Latest::WikiConfig.index_name
+          options[:index_name] = Elastic::Latest::WikiConfig.index_name if Feature.disabled?(:use_base_class_in_proxy_util)
         else
           fields = %w[blob.content blob.file_name blob.path]
         end
@@ -319,7 +319,7 @@ def blob_query(query, type: 'blob', page: 1, per: 20, options: {})
         bool_expr[:must_not] += query_filter_context[:must_not] if query_filter_context[:must_not].any?
 
         # add filters extracted from the `options`
-        options[:project_id_field] = use_separate_wiki_index?(options[:features]) ? 'rid' : 'blob.rid'
+        options[:project_id_field] = options[:features].eql?('wiki') && use_separate_wiki_index? ? 'rid' : 'blob.rid'
         options_filter_context = options_filter_context(:blob, options)
         bool_expr[:filter] += options_filter_context[:filter] if options_filter_context[:filter].any?
         options[:order] = :default if options[:order].blank? && !aggregation
@@ -356,8 +356,8 @@ def blob_query(query, type: 'blob', page: 1, per: 20, options: {})
       # rubocop:enable Metrics/PerceivedComplexity
       # rubocop:enable Metrics/CyclomaticComplexity
 
-      def use_separate_wiki_index?(feature)
-        feature.eql?('wiki') && Elastic::DataMigrationService.migration_has_finished?(:migrate_wikis_to_separate_index)
+      def use_separate_wiki_index?
+        Elastic::DataMigrationService.migration_has_finished?(:migrate_wikis_to_separate_index)
       end
     end
   end

ee/spec/lib/elastic/latest/git_class_proxy_add_suffix_project_in_wiki_rid_running_wiki_search_spec.rb

@@ -5,9 +5,9 @@
 
 RSpec.describe Elastic::Latest::GitClassProxy, :elastic, :sidekiq_inline, feature_category: :global_search do
   let_it_be(:wiki_project) { create(:project, :wiki_repo) }
-  let(:included_class) { Elastic::Latest::RepositoryClassProxy }
+  let(:included_class) { Elastic::Latest::ProjectWikiClassProxy }
 
-  subject { included_class.new(wiki_project.repository) }
+  subject { included_class.new(wiki_project.wiki.class, use_separate_indices: ProjectWiki.use_separate_indices?) }
 
   it 'fetches the results considering new and old format of rid' do
     stub_ee_application_setting(elasticsearch_search: true, elasticsearch_indexing: true)

ee/spec/lib/elastic/latest/git_class_proxy_spec.rb

@@ -13,11 +13,14 @@
     ensure_elasticsearch_index!
   end
 
-  subject { included_class.new(project.repository) }
+  subject { included_class.new(project.repository.class) }
 
   describe '#elastic_search' do
     context 'if type is wiki_blob' do
       let_it_be(:wiki_project) { create(:project, :wiki_repo, visibility_level: 0, wiki_access_level: 0) }
+      let(:included_class) { Elastic::Latest::ProjectWikiClassProxy }
+
+      subject { included_class.new(project.wiki.class, use_separate_indices: ProjectWiki.use_separate_indices?) }
 
       context 'if migrate_wikis_to_separate_index is not finished' do
         before do
@@ -221,6 +224,18 @@
                          'blob:match:search_terms')
   end
 
+  context 'when use_base_class_in_proxy_util is disabled' do
+    before do
+      stub_feature_flags(use_base_class_in_proxy_util: false)
+    end
+
+    it "names elasticsearch queries" do
+      subject.elastic_search_as_found_blob('*')
+
+      assert_named_queries('doc:is_a:blob', 'blob:match:search_terms')
+    end
+  end
+
   context 'when backfilling migration is complete' do
     let_it_be(:user) { create(:user) }
 

ee/spec/lib/elastic/latest/project_instance_proxy_spec.rb

@@ -42,6 +42,20 @@
           expect(result).to include(feature => ProjectFeature::PRIVATE) # rubocop:disable GitlabSecurity/PublicSend
         end
       end
+
+      context 'when feature_flag use_base_class_in_proxy_util is disabled' do
+        before do
+          stub_feature_flags(use_base_class_in_proxy_util: false)
+        end
+
+        it 'sets all tracked feature access levels to PRIVATE' do
+          result = subject.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
+          end
+        end
+      end
     end
   end
 end

ee/spec/lib/elastic/latest/project_wiki_class_proxy_spec.rb

@@ -2,10 +2,10 @@
 
 require 'spec_helper'
 
-RSpec.describe Elastic::Latest::ProjectWikiClassProxy, :elastic do
+RSpec.describe Elastic::Latest::ProjectWikiClassProxy, :elastic, feature_category: :global_search do
   let(:project) { create(:project, :wiki_repo) }
 
-  subject { described_class.new(project.wiki.repository) }
+  subject { described_class.new(project.wiki.class, use_separate_indices: ProjectWiki.use_separate_indices?) }
 
   describe '#elastic_search_as_wiki_page' do
     let!(:page) { create(:wiki_page, wiki: project.wiki) }
@@ -35,7 +35,18 @@
   it 'names elasticsearch queries' do
     subject.elastic_search_as_wiki_page('*')
 
-    assert_named_queries('doc:is_a:wiki_blob',
-                         'blob:match:search_terms')
+    assert_named_queries('doc:is_a:wiki_blob', 'blob:match:search_terms')
+  end
+
+  context 'when use_base_class_in_proxy_util is disabled' do
+    before do
+      stub_feature_flags(use_base_class_in_proxy_util: false)
+    end
+
+    it 'names elasticsearch queries' do
+      subject.elastic_search_as_wiki_page('*')
+
+      assert_named_queries('doc:is_a:wiki_blob', 'blob:match:search_terms')
+    end
   end
 end

ee/spec/lib/elastic/latest/project_wiki_instance_proxy_spec.rb

@@ -2,10 +2,10 @@
 
 require 'spec_helper'
 
-RSpec.describe Elastic::Latest::ProjectWikiInstanceProxy do
+RSpec.describe Elastic::Latest::ProjectWikiInstanceProxy, feature_category: :global_search do
   let_it_be(:project) { create(:project, :wiki_repo) }
 
-  subject { described_class.new(project.wiki) }
+  subject { described_class.new(project.wiki, use_separate_indices: true) }
 
   describe '#elastic_search_as_wiki_page' do
     let(:params) do
@@ -32,5 +32,19 @@
 
       subject.elastic_search_as_wiki_page('foo', **params)
     end
+
+    context 'when use_base_class_in_proxy_util is disabled' do
+      before do
+        stub_feature_flags(use_base_class_in_proxy_util: false)
+      end
+
+      it 'uses provided repository_id' do
+        params[:options][:repository_id] = "wiki_63"
+
+        expect(subject.class).to receive(:elastic_search_as_wiki_page).with('foo', params)
+
+        subject.elastic_search_as_wiki_page('foo', **params)
+      end
+    end
   end
 end

ee/spec/lib/elastic/latest/user_class_proxy_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 RSpec.describe Elastic::Latest::UserClassProxy, feature_category: :global_search do
-  subject { described_class.new(User) }
+  subject { described_class.new(User, use_separate_indices: true) }
 
   let(:query) { 'bob' }
   let(:options) { {} }
@@ -200,6 +200,20 @@
       expect(filter_query[:term]).to include({ in_forbidden_state: hash_including(value: false) })
     end
 
+    context 'when use_base_class_in_proxy_util is disabled' do
+      before do
+        stub_feature_flags(use_base_class_in_proxy_util: false)
+      end
+
+      it 'has a term with forbidden_state eq false' do
+        expect(query_hash.count).to eq(1)
+        filter_query = query_hash.first
+
+        expect(filter_query).to have_key(:term)
+        expect(filter_query[:term]).to include({ in_forbidden_state: hash_including(value: false) })
+      end
+    end
+
     context 'when the user is an admin' do
       let(:options) { { admin: true } }
 

ee/spec/lib/elastic/latest/user_instance_proxy_spec.rb

@@ -5,7 +5,7 @@
 RSpec.describe Elastic::Latest::UserInstanceProxy, feature_category: :global_search do
   let_it_be_with_reload(:user) { create(:user, :admin, :public_email) }
 
-  subject { described_class.new(user) }
+  subject { described_class.new(user, use_separate_indices: true) }
 
   describe '#as_indexed_json' do
     let(:result) { subject.as_indexed_json.with_indifferent_access }
@@ -109,6 +109,16 @@
   end
 
   describe '#es_parent' do
+    context 'when use_base_class_in_proxy_util is disabled' do
+      before do
+        stub_feature_flags(use_base_class_in_proxy_util: false)
+      end
+
+      it 'is nil so that elasticsearch routing is disabled' do
+        expect(subject.es_parent).to be_nil
+      end
+    end
+
     it 'is nil so that elasticsearch routing is disabled' do
       expect(subject.es_parent).to be_nil
     end

ee/spec/models/ee/project_wiki_spec.rb

@@ -24,4 +24,32 @@
       end
     end
   end
+
+  describe '#use_separate_indices?', :elastic do
+    context 'if migrate_wikis_to_separate_index is finished' do
+      before do
+        set_elasticsearch_migration_to(:migrate_wikis_to_separate_index, including: true)
+      end
+
+      it 'returns true' do
+        expect(described_class.use_separate_indices?).to be true
+      end
+    end
+
+    context 'if migrate_wikis_to_separate_index is not finished' do
+      before do
+        set_elasticsearch_migration_to(:migrate_wikis_to_separate_index, including: false)
+      end
+
+      it 'returns false' do
+        expect(described_class.use_separate_indices?).to be false
+      end
+    end
+  end
+
+  describe '#base_class' do
+    it 'returns Wiki' do
+      expect(described_class.base_class).to eq Wiki
+    end
+  end
 end

ee/spec/models/ee/wiki_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Wiki, feature_category: :wiki do
+  describe '#use_separate_indices?', :elastic do
+    context 'if migrate_wikis_to_separate_index is finished' do
+      before do
+        set_elasticsearch_migration_to(:migrate_wikis_to_separate_index, including: true)
+      end
+
+      it 'returns true' do
+        expect(described_class.use_separate_indices?).to be true
+      end
+    end
+
+    context 'if migrate_wikis_to_separate_index is not finished' do
+      before do
+        set_elasticsearch_migration_to(:migrate_wikis_to_separate_index, including: false)
+      end
+
+      it 'returns false' do
+        expect(described_class.use_separate_indices?).to be false
+      end
+    end
+  end
+
+  describe '#base_class' do
+    it 'returns Wiki' do
+      expect(described_class.base_class).to eq described_class
+    end
+  end
+end