Merge branch '325470-cablett-user-assigned-issue-counts-2' into 'master'

Refresh cache on user for assigned open issues count after cache invalidation [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!59961

app/models/user.rb

@@ -1677,6 +1677,12 @@ class User < ApplicationRecord
 
   def invalidate_issue_cache_counts
     Rails.cache.delete(['users', id, 'assigned_open_issues_count'])
+
+    if Feature.enabled?(:assigned_open_issues_cache, default_enabled: :yaml)
+      run_after_commit do
+        Users::UpdateOpenIssueCountWorker.perform_async(self.id)
+      end
+    end
   end
 
   def invalidate_merge_request_cache_counts

app/services/users/update_assigned_open_issue_count_service.rb

+# frozen_string_literal: true
+
+module Users
+  # Service class for calculating and caching the number of assigned open issues for a user.
+  class UpdateAssignedOpenIssueCountService
+    attr_accessor :target_user
+
+    def initialize(target_user:)
+      @target_user = target_user
+
+      raise ArgumentError.new("Please provide a target user") unless target_user.is_a?(User)
+    end
+
+    def execute
+      value = calculate_count
+      Rails.cache.write(cache_key, value, expires_in: User::COUNT_CACHE_VALIDITY_PERIOD)
+
+      ServiceResponse.success(payload: { count: value })
+    rescue StandardError => e
+      ServiceResponse.error(message: e.message)
+    end
+
+    private
+
+    def cache_key
+      ['users', target_user.id, 'assigned_open_issues_count']
+    end
+
+    def calculate_count
+      IssuesFinder.new(target_user, assignee_id: target_user.id, state: 'opened', non_archived: true).execute.count
+    end
+  end
+end

app/workers/all_queues.yml

@@ -2440,6 +2440,14 @@
   :weight: 1
   :idempotent:
   :tags: []
+- :name: users_update_open_issue_count
+  :feature_category: :users
+  :has_external_dependencies:
+  :urgency: :low
+  :resource_boundary: :unknown
+  :weight: 1
+  :idempotent: true
+  :tags: []
 - :name: web_hook
   :feature_category: :integrations
   :has_external_dependencies: true

app/workers/users/update_open_issue_count_worker.rb

+# frozen_string_literal: true
+
+module Users
+  class UpdateOpenIssueCountWorker
+    include ApplicationWorker
+
+    feature_category :users
+    idempotent!
+
+    def perform(target_user_ids)
+      target_user_ids = Array.wrap(target_user_ids)
+
+      raise ArgumentError.new('No target user ID provided') if target_user_ids.empty?
+
+      target_users = User.id_in(target_user_ids)
+      raise ArgumentError.new('No valid target user ID provided') if target_users.empty?
+
+      target_users.each do |user|
+        Users::UpdateAssignedOpenIssueCountService.new(target_user: user).execute
+      end
+    rescue StandardError => exception
+      Gitlab::ErrorTracking.track_and_raise_for_dev_exception(exception)
+    end
+  end
+end

changelogs/unreleased/325470-cablett-user-assigned-issue-counts-2.yml

+---
+title: Recalculate assigned open issues count after cache invalidation
+merge_request: 59961
+author:
+type: performance

config/feature_flags/development/assigned_open_issues_cache.yml

+---
+name: assigned_open_issues_cache
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/59961
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/325470
+group: group::product planning
+type: development
+default_enabled: false

config/sidekiq_queues.yml

@@ -398,6 +398,8 @@
   - 1
 - - upload_checksum
   - 1
+- - users_update_open_issue_count
+  - 1
 - - vulnerabilities_statistics_adjustment
   - 1
 - - vulnerability_exports_export

spec/models/user_spec.rb

@@ -4220,16 +4220,45 @@ RSpec.describe User do
   end
 
   describe '#invalidate_issue_cache_counts' do
-    let(:user) { build_stubbed(:user) }
+    let_it_be(:user) { create(:user) }
 
-    it 'invalidates cache for issue counter' do
-      cache_mock = double
+    subject do
+      user.invalidate_issue_cache_counts
+      user.save!
+    end
 
-      expect(cache_mock).to receive(:delete).with(['users', user.id, 'assigned_open_issues_count'])
+    shared_examples 'invalidates the cached value' do
+      it 'invalidates cache for issue counter' do
+        expect(Rails.cache).to receive(:delete).with(['users', user.id, 'assigned_open_issues_count'])
 
-      allow(Rails).to receive(:cache).and_return(cache_mock)
+        subject
+      end
+    end
 
-      user.invalidate_issue_cache_counts
+    it_behaves_like 'invalidates the cached value'
+
+    context 'if feature flag assigned_open_issues_cache is enabled' do
+      it 'calls the recalculate worker' do
+        expect(Users::UpdateOpenIssueCountWorker).to receive(:perform_async).with(user.id)
+
+        subject
+      end
+
+      it_behaves_like 'invalidates the cached value'
+    end
+
+    context 'if feature flag assigned_open_issues_cache is disabled' do
+      before do
+        stub_feature_flags(assigned_open_issues_cache: false)
+      end
+
+      it 'does not call the recalculate worker' do
+        expect(Users::UpdateOpenIssueCountWorker).not_to receive(:perform_async).with(user.id)
+
+        subject
+      end
+
+      it_behaves_like 'invalidates the cached value'
     end
   end
 

spec/services/users/update_assigned_open_issue_count_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Users::UpdateAssignedOpenIssueCountService do
+  let_it_be(:user) { create(:user) }
+
+  describe '#initialize' do
+    context 'incorrect arguments provided' do
+      it 'raises an error if there are no target user' do
+        expect { described_class.new(target_user: nil) }.to raise_error(ArgumentError, /Please provide a target user/)
+        expect { described_class.new(target_user: "nonsense") }.to raise_error(ArgumentError, /Please provide a target user/)
+      end
+    end
+
+    context 'when correct arguments provided' do
+      it 'is successful' do
+        expect { described_class.new(target_user: user) }.not_to raise_error
+      end
+    end
+  end
+
+  describe "#execute", :clean_gitlab_redis_cache do
+    let(:fake_update_service) { double }
+    let(:fake_issue_count_service) { double }
+    let(:provided_value) { nil }
+
+    subject { described_class.new(target_user: user).execute }
+
+    context 'successful' do
+      it 'returns a success response' do
+        expect(subject).to be_success
+      end
+
+      it 'writes the cache with the new value' do
+        expect(Rails.cache).to receive(:write).with(['users', user.id, 'assigned_open_issues_count'], 0, expires_in: User::COUNT_CACHE_VALIDITY_PERIOD)
+
+        subject
+      end
+
+      it 'calls the issues finder to get the latest value' do
+        expect(IssuesFinder).to receive(:new).with(user, assignee_id: user.id, state: 'opened', non_archived: true).and_return(fake_issue_count_service)
+        expect(fake_issue_count_service).to receive(:execute)
+
+        subject
+      end
+    end
+  end
+end

spec/workers/users/update_open_issue_count_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Users::UpdateOpenIssueCountWorker do
+  let_it_be(:first_user) { create(:user) }
+  let_it_be(:second_user) { create(:user) }
+
+  describe '#perform' do
+    let(:target_user_ids) { [first_user.id, second_user.id] }
+
+    subject { described_class.new.perform(target_user_ids) }
+
+    context 'when arguments are missing' do
+      context 'when target_user_ids are missing' do
+        context 'when nil' do
+          let(:target_user_ids) { nil }
+
+          it 'raises an error' do
+            expect { subject }.to raise_error(ArgumentError, /No target user ID provided/)
+          end
+        end
+
+        context 'when empty array' do
+          let(:target_user_ids) { [] }
+
+          it 'raises an error' do
+            expect { subject }.to raise_error(ArgumentError, /No target user ID provided/)
+          end
+        end
+
+        context 'when not an ID' do
+          let(:target_user_ids) { "nonsense" }
+
+          it 'raises an error' do
+            expect { subject }.to raise_error(ArgumentError, /No valid target user ID provided/)
+          end
+        end
+      end
+    end
+
+    context 'when successful' do
+      let(:job_args) { [target_user_ids] }
+      let(:fake_service1) { double }
+      let(:fake_service2) { double }
+
+      it 'calls the user update service' do
+        expect(Users::UpdateAssignedOpenIssueCountService).to receive(:new).with(target_user: first_user).and_return(fake_service1)
+        expect(Users::UpdateAssignedOpenIssueCountService).to receive(:new).with(target_user: second_user).and_return(fake_service2)
+        expect(fake_service1).to receive(:execute)
+        expect(fake_service2).to receive(:execute)
+
+        subject
+      end
+
+      it_behaves_like 'an idempotent worker' do
+        it 'recalculates' do
+          subject
+
+          expect(first_user.assigned_open_issues_count).to eq(0)
+        end
+      end
+    end
+  end
+end