Update issues in Elasticsearch when project visibility settings change

Ensure changes to project visibility settings also update the associated
issue documents for that project. This is required to keep the issue
documents updated once the visibility permissions are stored within
the issue document in preparation for global search joins being
removed from the Elasticsearch issue query.

config/sidekiq_queues.yml

@@ -92,6 +92,8 @@
   - 1
 - - disallow_two_factor_for_subgroups
   - 1
+- - elastic_association_indexer
+  - 1
 - - elastic_commit_indexer
   - 1
 - - elastic_delete_project

ee/app/models/concerns/elastic/projects_search.rb

@@ -7,14 +7,18 @@ module Elastic
     include ApplicationVersionedSearch
 
     included do
+      extend ::Gitlab::Utils::Override
+
       def use_elasticsearch?
         ::Gitlab::CurrentSettings.elasticsearch_indexes_project?(self)
       end
 
+      override :maintain_elasticsearch_create
       def maintain_elasticsearch_create
         ::Elastic::ProcessInitialBookkeepingService.track!(self)
       end
 
+      override :maintain_elasticsearch_destroy
       def maintain_elasticsearch_destroy
         ElasticDeleteProjectWorker.perform_async(self.id, self.es_id)
       end

ee/app/models/ee/project_feature.rb

@@ -9,12 +9,22 @@ module EE
     prepended do
       set_available_features(EE_FEATURES)
 
-      # Ensure changes to project visibility settings go to elasticsearch
+      # Ensure changes to project visibility settings go to elasticsearch if the tracked field(s) change
       after_commit on: :update do
-        project.maintain_elasticsearch_update if project.maintaining_elasticsearch?
+        if project.maintaining_elasticsearch?
+          project.maintain_elasticsearch_update
+
+          ElasticAssociationIndexerWorker.perform_async(self.project.class.name, project_id, ['issues']) if elasticsearch_project_associations_need_updating?
+        end
       end
 
       default_value_for :requirements_access_level, value: Featurable::ENABLED, allows_nil: false
+
+      private
+
+      def elasticsearch_project_associations_need_updating?
+        self.previous_changes.key?(:issues_access_level)
+      end
     end
   end
 end

ee/app/services/elastic/process_bookkeeping_service.rb

@@ -53,6 +53,30 @@ module Elastic
       def with_redis(&blk)
         Gitlab::Redis::SharedState.with(&blk) # rubocop:disable CodeReuse/ActiveRecord
       end
+
+      def maintain_indexed_associations(object, associations)
+        each_indexed_association(object, associations) do |_, association|
+          association.find_in_batches do |group|
+            track!(*group)
+          end
+        end
+      end
+
+      private
+
+      def each_indexed_association(object, associations)
+        associations.each do |association_name|
+          association = object.association(association_name)
+          scope = association.scope
+          klass = association.klass
+
+          if klass == Note
+            scope = scope.searchable
+          end
+
+          yield klass, scope
+        end
+      end
     end
 
     def execute

ee/app/services/elastic/process_initial_bookkeeping_service.rb

@@ -4,8 +4,7 @@ module Elastic
   class ProcessInitialBookkeepingService < Elastic::ProcessBookkeepingService
     REDIS_SET_KEY = 'elastic:bulk:initial:0:zset'
     REDIS_SCORE_KEY = 'elastic:bulk:initial:0:score'
-
-    INDEXED_ASSOCIATIONS = [
+    INDEXED_PROJECT_ASSOCIATIONS = [
       :issues,
       :merge_requests,
       :snippets,
@@ -20,36 +19,12 @@ module Elastic
         projects.each do |project|
           raise ArgumentError, 'This method only accepts Projects' unless project.is_a?(Project)
 
-          maintain_indexed_associations(project)
+          maintain_indexed_associations(project, INDEXED_PROJECT_ASSOCIATIONS)
 
           ElasticCommitIndexerWorker.perform_async(project.id)
           ElasticCommitIndexerWorker.perform_async(project.id, nil, nil, true)
         end
       end
-
-      def each_indexed_association(project)
-        INDEXED_ASSOCIATIONS.each do |association_name|
-          association = project.association(association_name)
-          scope = association.scope
-          klass = association.klass
-
-          if klass == Note
-            scope = scope.searchable
-          end
-
-          yield klass, scope
-        end
-      end
-
-      private
-
-      def maintain_indexed_associations(project)
-        each_indexed_association(project) do |_, association|
-          association.find_in_batches do |group|
-            track!(*group)
-          end
-        end
-      end
     end
   end
 end

ee/app/workers/all_queues.yml

@@ -605,6 +605,14 @@
   :weight: 1
   :idempotent: true
   :tags: []
+- :name: elastic_association_indexer
+  :feature_category: :global_search
+  :has_external_dependencies:
+  :urgency: :low
+  :resource_boundary: :unknown
+  :weight: 1
+  :idempotent:
+  :tags: []
 - :name: elastic_commit_indexer
   :feature_category: :global_search
   :has_external_dependencies:

ee/app/workers/elastic_association_indexer_worker.rb

+# frozen_string_literal: true
+
+class ElasticAssociationIndexerWorker # rubocop:disable Scalability/IdempotentWorker
+  include ApplicationWorker
+
+  feature_category :global_search
+  loggable_arguments 0, 2
+
+  def perform(class_name, id, indexed_associations)
+    return unless Gitlab::CurrentSettings.elasticsearch_indexing?
+
+    klass = class_name.constantize
+    object = klass.find(id)
+    return unless object.use_elasticsearch?
+
+    Elastic::ProcessBookkeepingService.maintain_indexed_associations(object, indexed_associations)
+  end
+end

ee/spec/models/project_feature_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 RSpec.describe ProjectFeature do
-  let(:project) { create(:project) }
+  let(:project) { create(:project, :public) }
   let(:user) { create(:user) }
 
   describe '#feature_available?' do
@@ -20,4 +20,35 @@ RSpec.describe ProjectFeature do
       end
     end
   end
+
+  describe 'project visibility changes' do
+    using RSpec::Parameterized::TableSyntax
+
+    before do
+      allow(project).to receive(:maintaining_elasticsearch?).and_return(true)
+    end
+
+    where(:feature, :worker_expected) do
+      'issues'          | true
+      'wiki'            | false
+      'builds'          | false
+      'merge_requests'  | false
+      'repository'      | false
+      'pages'           | false
+    end
+
+    with_them do
+      it 're-indexes project and project associations on update' do
+        expect(project).to receive(:maintain_elasticsearch_update)
+
+        if worker_expected
+          expect(ElasticAssociationIndexerWorker).to receive(:perform_async).with('Project', project.id, ['issues'])
+        else
+          expect(ElasticAssociationIndexerWorker).not_to receive(:perform_async)
+        end
+
+        project.project_feature.update_attribute("#{feature}_access_level".to_sym, ProjectFeature::PRIVATE)
+      end
+    end
+  end
 end

ee/spec/models/project_spec.rb

@@ -2543,7 +2543,7 @@ RSpec.describe Project do
     end
   end
 
-  describe 'caculate template repositories' do
+  describe 'calculate template repositories' do
     let(:group1) { create(:group) }
     let(:group2) { create(:group) }
     let(:group2_sub1) { create(:group, parent: group2) }

ee/spec/services/elastic/process_bookkeeping_service_spec.rb

@@ -76,6 +76,28 @@ RSpec.describe Elastic::ProcessBookkeepingService, :clean_gitlab_redis_shared_st
     end
   end
 
+  describe '.maintain_indexed_associations' do
+    let(:project) { create(:project) }
+
+    it 'calls track! for each associated object' do
+      issue_1 = create(:issue, project: project)
+      issue_2 = create(:issue, project: project)
+
+      expect(described_class).to receive(:track!).with(issue_1, issue_2)
+
+      described_class.maintain_indexed_associations(project, ['issues'])
+    end
+
+    it 'correctly scopes associated note objects to not include system notes' do
+      note_searchable = create(:note, :on_issue, project: project)
+      create(:note, :on_issue, :system, project: project)
+
+      expect(described_class).to receive(:track!).with(note_searchable)
+
+      described_class.maintain_indexed_associations(project, ['notes'])
+    end
+  end
+
   describe '#execute' do
     let(:limit) { 5 }
 

ee/spec/services/elastic/process_initial_bookkeeping_service_spec.rb

@@ -8,7 +8,7 @@ RSpec.describe Elastic::ProcessInitialBookkeepingService do
 
   describe '.backfill_projects!' do
     it 'calls initial project indexing' do
-      expect(described_class).to receive(:maintain_indexed_associations)
+      expect(described_class).to receive(:maintain_indexed_associations).with(project, Elastic::ProcessInitialBookkeepingService::INDEXED_PROJECT_ASSOCIATIONS)
       expect(ElasticCommitIndexerWorker).to receive(:perform_async).with(project.id)
       expect(ElasticCommitIndexerWorker).to receive(:perform_async).with(project.id, nil, nil, true)
 

ee/spec/workers/elastic_association_indexer_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ElasticAssociationIndexerWorker do
+  subject { described_class.new }
+
+  let(:indexed_associations) { [:issues] }
+
+  before do
+    stub_ee_application_setting(elasticsearch_indexing: true)
+  end
+
+  context 'when elasticsearch_indexing is disabled' do
+    it 'does nothing' do
+      stub_ee_application_setting(elasticsearch_indexing: false)
+      expect(Elastic::ProcessBookkeepingService).not_to receive(:maintain_indexed_associations)
+
+      subject.perform('Project', 1, indexed_associations)
+    end
+  end
+
+  context 'when elasticsearch_indexing is enabled' do
+    let!(:project) { create(:project) }
+
+    context 'but object is not setup to use elasticsearch' do
+      it 'does nothing' do
+        expect_next_found_instance_of(Project) do |p|
+          expect(p).to receive(:use_elasticsearch?).and_return(false)
+        end
+        expect(Elastic::ProcessBookkeepingService).not_to receive(:maintain_indexed_associations)
+
+        subject.perform(project.class.name, project.id, indexed_associations)
+      end
+    end
+
+    it 'updates associations for the object' do
+      expect(Elastic::ProcessBookkeepingService).to receive(:maintain_indexed_associations).with(project, indexed_associations)
+
+      subject.perform(project.class.name, project.id, indexed_associations)
+    end
+  end
+end