Merge branch...

Merge branch '300843-change-project-group-notes-es-searches-to-use-new-denormalized-fields' into 'master'

Change notes ES searches to use new denormalized fields

See merge request gitlab-org/gitlab!53294

ee/lib/elastic/latest/note_class_proxy.rb

@@ -13,6 +13,7 @@ module Elastic
         options[:in] = ['note']
         query_hash = basic_query_hash(%w[note], query, count_only: options[:count_only])
 
+        options[:no_join_project] = Elastic::DataMigrationService.migration_has_finished?(:add_permissions_data_to_notes_documents)
         context.name(:note) do
           query_hash = context.name(:authorized) { project_ids_filter(query_hash, options) }
           query_hash = context.name(:confidentiality) { confidentiality_filter(query_hash, options) }
@@ -100,6 +101,9 @@ module Elastic
 
       override :project_ids_filter
       def project_ids_filter(query_hash, options)
+        # support for not using project joins in the query
+        no_join_project = options[:no_join_project]
+
         query_hash[:query][:bool][:filter] ||= []
 
         project_query = context.name(:project) do
@@ -107,7 +111,8 @@ module Elastic
             options[:current_user],
             options[:project_ids],
             options[:public_and_internal_projects],
-            options[:features]
+            options[:features],
+            no_join_project
           )
         end
 
@@ -124,23 +129,30 @@ module Elastic
         project_query[:should].flatten.each do |condition|
           noteable_type = condition.delete(:noteable_type).to_s
 
-          filters[:bool][:should] << {
+          should_filter = {
             bool: {
               must: [
-                {
-                  has_parent: {
-                    parent_type: "project",
-                    query: {
-                      bool: {
-                        should: condition
-                      }
-                    }
-                  }
-                },
                 { term: { noteable_type: { _name: context.name(:noteable, :is_a, noteable_type), value: noteable_type } } }
               ]
             }
           }
+
+          should_filter[:bool][:must] << if no_join_project
+                                           condition
+                                         else
+                                           {
+                                             has_parent: {
+                                               parent_type: "project",
+                                               query: {
+                                                 bool: {
+                                                   should: condition
+                                                 }
+                                               }
+                                             }
+                                           }
+                                         end
+
+          filters[:bool][:should] << should_filter
         end
 
         query_hash[:query][:bool][:filter] << filters
@@ -150,18 +162,18 @@ module Elastic
       # Query notes based on the various feature permission of the noteable_type.
       # Appends `noteable_type` (which will be removed in project_ids_filter)
       # for base model filtering.
-      # We do not implement `no_join_project` argument for notes class yet
-      # as this is not supported. This will need to be fixed when we move
-      # notes to a new index.
       override :pick_projects_by_membership
-      def pick_projects_by_membership(project_ids, user, _, _ = nil)
+      def pick_projects_by_membership(project_ids, user, no_join_project, _ = nil)
+        # support for not using project joins in the query
+        project_id_key = no_join_project ? :project_id : :id
+
         noteable_type_to_feature.map do |noteable_type, feature|
           context.name(feature) do
             condition =
               if project_ids == :any
                 { term: { visibility_level: { _name: context.name(:any), value: Project::PRIVATE } } }
               else
-                { terms: { _name: context.name(:membership, :id), id: filter_ids_by_feature(project_ids, user, feature) } }
+                { terms: { _name: context.name(:membership, :id), project_id_key => filter_ids_by_feature(project_ids, user, feature) } }
               end
 
             limit =

ee/spec/support/shared_examples/services/search_notes_shared_examples.rb

@@ -32,7 +32,7 @@ RSpec.shared_examples 'search notes shared examples' do
     end
 
     context 'when user can read confidential notes' do
-      it 'does not filter confidental notes' do
+      it 'does not filter confidential notes' do
         noteable.project.add_reporter(user)
 
         expect_search_results(user, 'notes', expected_objects: [not_confidential_note, nil_confidential_note, confidential_note]) do |user|

ee/spec/support/shared_examples/services/search_query_applies_joins_based_on_migrations_shared_examples.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples 'search query applies joins based on migrations shared examples' do |migration_name|
+  context 'using joins for global permission checks', :elastic do
+    let(:es_host) { Gitlab::CurrentSettings.elasticsearch_url[0] }
+    let(:search_url) { Addressable::Template.new("#{es_host}/{index}/doc/_search{?params*}") }
+
+    context "when #{migration_name} migration is finished" do
+      before do
+        allow(Elastic::DataMigrationService).to receive(:migration_has_finished?)
+                                                  .with(migration_name)
+                                                  .and_return(true)
+      end
+
+      it 'does not use joins to apply permissions' do
+        request = a_request(:get, search_url).with do |req|
+          expect(req.body).not_to include("has_parent")
+        end
+
+        results
+
+        expect(request).to have_been_made
+      end
+    end
+
+    context "when #{migration_name} migration is not finished" do
+      before do
+        allow(Elastic::DataMigrationService).to receive(:migration_has_finished?)
+                                                  .with(migration_name)
+                                                  .and_return(false)
+      end
+
+      it 'uses joins to apply permissions' do
+        request = a_request(:get, search_url).with do |req|
+          expect(req.body).to include("has_parent")
+        end
+
+        results
+
+        expect(request).to have_been_made
+      end
+    end
+  end
+end

spec/support/shared_contexts/policies/project_policy_table_shared_context.rb

@@ -356,7 +356,7 @@ RSpec.shared_context 'ProjectPolicyTable context' do
     :private  | :anonymous  | 0
   end
 
-  # :snippet_level, :project_level, :feature_access_level, :membership, :expected_count
+  # :snippet_level, :project_level, :feature_access_level, :membership, :admin_mode, :expected_count
   def permission_table_for_project_snippet_access
     :public   | :public   | :enabled  | :admin      | true  | 1
     :public   | :public   | :enabled  | :admin      | false | 1