Merge branch '335825-use-namespace_ancestry-for-searches' into 'master'

Advanced Search: Use namespace_ancestry for issues

See merge request gitlab-org/gitlab!69741

ee/app/models/ee/namespace.rb

@@ -404,6 +404,11 @@ module EE
       ::Gitlab::CurrentSettings.invalidate_elasticsearch_indexes_cache_for_namespace!(self.id)
     end
 
+    def elastic_namespace_ancestry
+      separator = '-'
+      self_and_ancestor_ids(hierarchy_order: :desc).join(separator) + separator
+    end
+
     def enable_temporary_storage_increase!
       update(temporary_storage_increase_ends_on: TEMPORARY_STORAGE_INCREASE_DAYS.days.from_now)
     end

ee/config/feature_flags/development/elasticsearch_use_group_level_optimization.yml

+---
+name: elasticsearch_use_group_level_optimization
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/69741
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/340596
+milestone: '14.4'
+type: development
+group: group::global search
+default_enabled: false

ee/lib/elastic/latest/application_class_proxy.rb

@@ -381,6 +381,37 @@ module Elastic
 
         authorized_project_ids.to_a
       end
+
+      def authorized_namespace_ids(current_user, options = {})
+        return [] unless current_user && options[:group_ids].present?
+
+        authorized_ids = current_user.authorized_groups.pluck_primary_key.to_set
+        authorized_ids.intersection(options[:group_ids].to_set).to_a
+      end
+
+      def ancestry_filter(current_user, namespace_ancestry)
+        return {} unless current_user
+        return {} if namespace_ancestry.blank?
+
+        context.name(:ancestry_filter) do
+          filters = namespace_ancestry.map do |namespace_ids|
+            {
+              prefix: {
+                namespace_ancestry_ids: {
+                  _name: context.name(:descendants),
+                  value: namespace_ids
+                }
+              }
+            }
+          end
+
+          {
+            bool: {
+              should: filters
+            }
+          }
+        end
+      end
     end
   end
 end

ee/lib/elastic/latest/application_instance_proxy.rb

@@ -5,8 +5,6 @@ module Elastic
     class ApplicationInstanceProxy < Elasticsearch::Model::Proxy::InstanceMethodsProxy
       include InstanceProxyUtil
 
-      NAMESPACE_ANCESTRY_SEPARATOR = '-'
-
       def es_parent
         "project_#{target.project_id}" unless target.is_a?(Project) || target&.project_id.nil?
       end
@@ -21,8 +19,7 @@ module Elastic
 
       def namespace_ancestry
         project = target.is_a?(Project) ? target : target.project
-        namespace = project.namespace
-        namespace.self_and_ancestor_ids(hierarchy_order: :desc).join(NAMESPACE_ANCESTRY_SEPARATOR) + NAMESPACE_ANCESTRY_SEPARATOR
+        project.namespace.elastic_namespace_ancestry
       end
 
       private

ee/lib/elastic/latest/issue_class_proxy.rb

@@ -19,7 +19,7 @@ module Elastic
         options[:features] = 'issues'
         options[:no_join_project] = true
         context.name(:issue) do
-          query_hash = context.name(:authorized) { project_ids_filter(query_hash, options) }
+          query_hash = context.name(:authorized) { authorization_filter(query_hash, options) }
           query_hash = context.name(:confidentiality) { confidentiality_filter(query_hash, options) }
           query_hash = context.name(:match) { state_filter(query_hash, options) }
         end
@@ -56,6 +56,30 @@ module Elastic
         end
       end
 
+      def should_use_project_ids_filter?(options)
+        options[:project_ids] == :any ||
+        options[:group_ids].blank? ||
+        Feature.disabled?(:elasticsearch_use_group_level_optimization) ||
+        !Elastic::DataMigrationService.migration_has_finished?(:redo_backfill_namespace_ancestry_ids_for_issues)
+      end
+
+      def authorization_filter(query_hash, options)
+        return project_ids_filter(query_hash, options) if should_use_project_ids_filter?(options)
+
+        current_user = options[:current_user]
+        namespace_ancestry = Namespace.find(authorized_namespace_ids(current_user, options))
+                                      .map(&:elastic_namespace_ancestry)
+
+        return project_ids_filter(query_hash, options) if namespace_ancestry.blank?
+
+        context.name(:namespace) do
+          query_hash[:query][:bool][:filter] ||= []
+          query_hash[:query][:bool][:filter] << ancestry_filter(current_user, namespace_ancestry)
+        end
+
+        query_hash
+      end
+
       # Builds an elasticsearch query that will select documents from a
       # set of projects for Group and Project searches, taking user access
       # rules for issues into account. Relies upon super for Global searches

ee/lib/gitlab/elastic/group_search_results.rb

@@ -6,6 +6,8 @@ module Gitlab
     # superclass inside a module, because autoloading can occur in a
     # different order between execution environments.
     class GroupSearchResults < Gitlab::Elastic::SearchResults
+      extend Gitlab::Utils::Override
+
       attr_reader :group, :default_project_filter, :filters
 
       # rubocop:disable Metrics/ParameterLists
@@ -17,6 +19,16 @@ module Gitlab
         super(current_user, query, limit_project_ids, public_and_internal_projects: public_and_internal_projects, order_by: order_by, sort: sort, filters: filters)
       end
       # rubocop:enable Metrics/ParameterLists
+
+      override :scope_options
+      def scope_options(scope)
+        case scope
+        when :issues
+          super.merge(group_ids: [group.id])
+        else
+          super
+        end
+      end
     end
   end
 end

ee/spec/services/search/group_service_spec.rb

@@ -206,8 +206,24 @@ RSpec.describe Search::GroupService do
         permission_table_for_guest_feature_access
       end
 
-      with_them do
-        it_behaves_like 'search respects visibility'
+      context 'elasticsearch_use_group_level_optimization is enabled' do
+        before do
+          stub_feature_flags(elasticsearch_use_group_level_optimization: true)
+        end
+
+        with_them do
+          it_behaves_like 'search respects visibility'
+        end
+      end
+
+      context 'elasticsearch_use_group_level_optimization is disabled' do
+        before do
+          stub_feature_flags(elasticsearch_use_group_level_optimization: false)
+        end
+
+        with_them do
+          it_behaves_like 'search respects visibility'
+        end
       end
     end
 
@@ -296,13 +312,27 @@ RSpec.describe Search::GroupService do
       let!(:new_updated) { create(:issue, project: project, title: 'updated recent', updated_at: 1.day.ago) }
       let!(:very_old_updated) { create(:issue, project: project, title: 'updated very old', updated_at: 1.year.ago) }
 
+      let(:results_created) { described_class.new(nil, group, search: 'sorted', sort: sort).execute }
+      let(:results_updated) { described_class.new(nil, group, search: 'updated', sort: sort).execute }
+
       before do
         ensure_elasticsearch_index!
       end
 
-      include_examples 'search results sorted' do
-        let(:results_created) { described_class.new(nil, group, search: 'sorted', sort: sort).execute }
-        let(:results_updated) { described_class.new(nil, group, search: 'updated', sort: sort).execute }
+      context 'elasticsearch_use_group_level_optimization is enabled' do
+        before do
+          stub_feature_flags(elasticsearch_use_group_level_optimization: true)
+        end
+
+        include_examples 'search results sorted'
+      end
+
+      context 'elasticsearch_use_group_level_optimization is disabled' do
+        before do
+          stub_feature_flags(elasticsearch_use_group_level_optimization: false)
+        end
+
+        include_examples 'search results sorted'
       end
     end