Clear cache for all keys

- Worker should clear public and total counts
- Add specs for method clear_all_cache_keys

app/services/groups/count_service.rb

@@ -22,11 +22,12 @@ module Groups
       refresh_cache_over_threshold
     end
 
-    def refresh_cache_over_threshold
+    def refresh_cache_over_threshold(key = nil)
+      key ||= cache_key
       new_count = uncached_count
 
       if new_count > CACHED_COUNT_THRESHOLD
-        update_cache_for_key(cache_key) { new_count }
+        update_cache_for_key(key) { new_count }
       else
         delete_cache
       end
@@ -34,8 +35,10 @@ module Groups
       new_count
     end
 
-    def cache_key
-      ['groups', "#{issuable_key}_count_service", VERSION, group.id, cache_key_name]
+    def cache_key(key_name = nil)
+      key_name ||= cache_key_name
+
+      ['groups', "#{issuable_key}_count_service", VERSION, group.id, key_name]
     end
 
     private

app/services/groups/open_issues_count_service.rb

@@ -6,6 +6,12 @@ module Groups
     PUBLIC_COUNT_KEY = 'group_public_open_issues_count'
     TOTAL_COUNT_KEY = 'group_total_open_issues_count'
 
+    def clear_all_cache_keys
+      [cache_key(PUBLIC_COUNT_KEY), cache_key(TOTAL_COUNT_KEY)].each do |key|
+        Rails.cache.delete(key)
+      end
+    end
+
     private
 
     def cache_key_name
@@ -23,7 +29,14 @@ module Groups
     end
 
     def relation_for_count
-      IssuesFinder.new(user, group_id: group.id, state: 'opened', non_archived: true, include_subgroups: true, public_only: public_only?).execute
+      IssuesFinder.new(
+        user,
+        group_id: group.id,
+        state: 'opened',
+        non_archived: true,
+        include_subgroups: true,
+        public_only: public_only?
+      ).execute
     end
 
     def issuable_key

app/services/issuable/bulk_update_service.rb

@@ -17,8 +17,8 @@ module Issuable
       set_update_params(type)
       updated_issuables = update_issuables(type, ids)
 
-      if updated_issuables.present? && requires_count_cache_refresh?(type)
-        schedule_group_issues_count_refresh(updated_issuables)
+      if updated_issuables.present? && requires_count_cache_reset?(type)
+        schedule_group_issues_count_reset(updated_issuables)
       end
 
       response_success(payload: { count: updated_issuables.size })
@@ -86,15 +86,15 @@ module Issuable
       ServiceResponse.error(message: message, http_status: http_status)
     end
 
-    def requires_count_cache_refresh?(type)
+    def requires_count_cache_reset?(type)
       type.to_sym == :issue && params.include?(:state_event)
     end
 
-    def schedule_group_issues_count_refresh(updated_issuables)
+    def schedule_group_issues_count_reset(updated_issuables)
       group_ids = updated_issuables.map(&:project).map(&:namespace_id)
       return if group_ids.empty?
 
-      Issuables::RefreshGroupsIssueCounterWorker.perform_async(group_ids)
+      Issuables::ClearGroupsIssueCounterWorker.perform_async(group_ids)
     end
   end
 end

app/workers/all_queues.yml

@@ -2110,7 +2110,8 @@
   :weight: 1
   :idempotent:
   :tags: []
-- :name: issuables_refresh_groups_issue_counter
+- :name: issuables_clear_groups_issue_counter
+  :worker_name: Issuables::ClearGroupsIssueCounterWorker
   :feature_category: :issue_tracking
   :has_external_dependencies:
   :urgency: :low

app/workers/issuables/refresh_groups_issue_counter_worker.rb → app/workers/issuables/clear_groups_issue_counter_worker.rb

 # frozen_string_literal: true
 
 module Issuables
-  class RefreshGroupsIssueCounterWorker
+  class ClearGroupsIssueCounterWorker
     include ApplicationWorker
 
     idempotent!
@@ -15,14 +15,14 @@ module Issuables
         .new(Group.by_id(group_ids))
         .base_and_ancestors
 
-      refresh_cached_count(groups_with_ancestors)
+      clear_cached_count(groups_with_ancestors)
     end
 
     private
 
-    def refresh_cached_count(groups)
+    def clear_cached_count(groups)
       groups.each do |group|
-        Groups::OpenIssuesCountService.new(group).refresh_cache_over_threshold
+        Groups::OpenIssuesCountService.new(group).clear_all_cache_keys
       end
     end
   end

changelogs/unreleased/299178-implement-refresh_cache-for-issues-count-in-group-sidebar.yml

 ---
-title: Refresh group open issues count cache when bulk updating issue state
+title: Clear group open issues count cache when bulk updating issues state
 merge_request: 56386
 author:
 type: added

config/sidekiq_queues.yml

@@ -190,7 +190,7 @@
   - 1
 - - issuable_export_csv
   - 1
-- - issuables_refresh_groups_issue_counter
+- - issuables_clear_groups_issue_counter
   - 1
 - - issue_placement
   - 2

spec/models/group_spec.rb

@@ -2376,4 +2376,18 @@ RSpec.describe Group do
 
     it_behaves_like 'model with Debian distributions'
   end
+
+  describe '.ids_with_disabled_email' do
+    let!(:parent_1) { create(:group, emails_disabled: true) }
+    let!(:child_1) { create(:group, parent: parent_1) }
+
+    let!(:parent_2) { create(:group, emails_disabled: false) }
+    let!(:child_2) { create(:group, parent: parent_2) }
+
+    let!(:other_group) { create(:group, emails_disabled: false) }
+
+    subject(:group_ids_where_email_is_disabled) { described_class.ids_with_disabled_email([child_1, child_2, other_group]) }
+
+    it { is_expected.to eq(Set.new([child_1.id])) }
+  end
 end

spec/services/groups/open_issues_count_service_spec.rb

@@ -57,4 +57,15 @@ RSpec.describe Groups::OpenIssuesCountService, :use_clean_rails_memory_store_cac
       it_behaves_like 'a counter caching service with threshold'
     end
   end
+
+  describe '#clear_all_cache_keys' do
+    it 'calls `Rails.cache.delete` with the correct keys' do
+      expect(Rails.cache).to receive(:delete)
+        .with(['groups', 'open_issues_count_service', 1, group.id, described_class::PUBLIC_COUNT_KEY])
+      expect(Rails.cache).to receive(:delete)
+        .with(['groups', 'open_issues_count_service', 1, group.id, described_class::TOTAL_COUNT_KEY])
+
+      subject.clear_all_cache_keys
+    end
+  end
 end

spec/services/issuable/bulk_update_service_spec.rb

@@ -101,17 +101,17 @@ RSpec.describe Issuable::BulkUpdateService do
     end
   end
 
-  shared_examples 'scheduling cached group count refresh' do
+  shared_examples 'scheduling cached group count clear' do
     it 'schedules worker' do
-      expect(Issuables::RefreshGroupsIssueCounterWorker).to receive(:perform_async)
+      expect(Issuables::ClearGroupsIssueCounterWorker).to receive(:perform_async)
 
       bulk_update(issuables, params)
     end
   end
 
-  shared_examples 'not scheduling cached group count refresh' do
+  shared_examples 'not scheduling cached group count clear' do
     it 'does not schedule worker' do
-      expect(Issuables::RefreshGroupsIssueCounterWorker).not_to receive(:perform_async)
+      expect(Issuables::ClearGroupsIssueCounterWorker).not_to receive(:perform_async)
 
       bulk_update(issuables, params)
     end
@@ -148,7 +148,7 @@ RSpec.describe Issuable::BulkUpdateService do
         expect(project.issues.closed).not_to be_empty
       end
 
-      it_behaves_like 'scheduling cached group count refresh' do
+      it_behaves_like 'scheduling cached group count clear' do
         let(:issuables) { issues }
         let(:params) { { state_event: 'close' } }
       end
@@ -171,7 +171,7 @@ RSpec.describe Issuable::BulkUpdateService do
         expect(project.issues.opened).not_to be_empty
       end
 
-      it_behaves_like 'scheduling cached group count refresh' do
+      it_behaves_like 'scheduling cached group count clear' do
         let(:issuables) { issues }
         let(:params) { { state_event: 'reopen' } }
       end
@@ -258,7 +258,7 @@ RSpec.describe Issuable::BulkUpdateService do
 
       it_behaves_like 'updates milestones'
 
-      it_behaves_like 'not scheduling cached group count refresh' do
+      it_behaves_like 'not scheduling cached group count clear' do
         let(:params) { { milestone_id: milestone.id } }
       end
     end

spec/workers/issuables/clear_groups_issue_counter_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Issuables::ClearGroupsIssueCounterWorker do
+  describe '#perform' do
+    let_it_be(:user) { create(:user) }
+    let_it_be(:parent_group) { create(:group) }
+    let_it_be(:root_group) { create(:group, parent: parent_group) }
+    let_it_be(:subgroup) { create(:group, parent: root_group) }
+
+    let(:count_service) { Groups::OpenIssuesCountService }
+    let(:instance1) { instance_double(count_service) }
+    let(:instance2) { instance_double(count_service) }
+
+    it_behaves_like 'an idempotent worker' do
+      let(:job_args) { [[root_group.id]] }
+      let(:exec_times) { IdempotentWorkerHelper::WORKER_EXEC_TIMES }
+
+      it 'clears the cached issue count in given groups and ancestors' do
+        expect(count_service).to receive(:new)
+          .exactly(exec_times).times.with(root_group).and_return(instance1)
+        expect(count_service).to receive(:new)
+          .exactly(exec_times).times.with(parent_group).and_return(instance2)
+        expect(count_service).not_to receive(:new).with(subgroup)
+
+        [instance1, instance2].all? do |instance|
+          expect(instance).to receive(:clear_all_cache_keys).exactly(exec_times).times
+        end
+
+        subject
+      end
+    end
+
+    it 'does not call count service or rise error when group_ids is empty' do
+      expect(count_service).not_to receive(:new)
+      expect(Gitlab::ErrorTracking).not_to receive(:log_exception)
+
+      described_class.new.perform([])
+    end
+  end
+end

spec/workers/issuables/refresh_groups_issue_counter_worker_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Issuables::RefreshGroupsIssueCounterWorker do
-  describe '#perform' do
-    let_it_be(:user) { create(:user) }
-    let_it_be(:parent_group) { create(:group) }
-    let_it_be(:root_group) { create(:group, parent: parent_group) }
-    let_it_be(:subgroup) { create(:group, parent: root_group) }
-
-    let(:count_service) { Groups::OpenIssuesCountService }
-    let(:group_ids) { [root_group.id] }
-
-    context 'when group_ids is empty' do
-      let(:group_ids) { [] }
-
-      it 'does not call count service or rise error' do
-        expect(count_service).not_to receive(:new)
-        expect(Gitlab::ErrorTracking).not_to receive(:log_exception)
-
-        described_class.new.perform(group_ids)
-      end
-    end
-
-    context 'when updating cache' do
-      let(:instance1) { instance_double(count_service) }
-      let(:instance2) { instance_double(count_service) }
-
-      it_behaves_like 'an idempotent worker' do
-        let(:job_args) { [group_ids] }
-        let(:exec_times) { IdempotentWorkerHelper::WORKER_EXEC_TIMES }
-
-        it 'refreshes the issue count in given groups and ancestors' do
-          expect(count_service).to receive(:new)
-            .exactly(exec_times).times.with(root_group).and_return(instance1)
-          expect(count_service).to receive(:new)
-            .exactly(exec_times).times.with(parent_group).and_return(instance2)
-          expect(count_service).not_to receive(:new).with(subgroup)
-
-          [instance1, instance2].all? do |instance|
-            expect(instance).to receive(:refresh_cache_over_threshold).exactly(exec_times).times
-          end
-
-          subject
-        end
-      end
-    end
-  end
-end