Commit 29689737 authored by charlie ablett's avatar charlie ablett Committed by Dmytro Zaporozhets (DZ)

Refresh cache on user for assigned open issues count after cache invalidation...

Refresh cache on user for assigned open issues count after cache invalidation [RUN ALL RSPEC] [RUN AS-IF-FOSS]
parent d6a09e6d
......@@ -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
......
# 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
......@@ -2432,6 +2432,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
......
# 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
---
title: Recalculate assigned open issues count after cache invalidation
merge_request: 59961
author:
type: performance
---
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
......@@ -398,6 +398,8 @@
- 1
- - upload_checksum
- 1
- - users_update_open_issue_count
- 1
- - vulnerabilities_statistics_adjustment
- 1
- - vulnerability_exports_export
......
......@@ -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) }
subject do
user.invalidate_issue_cache_counts
user.save!
end
shared_examples 'invalidates the cached value' do
it 'invalidates cache for issue counter' do
cache_mock = double
expect(Rails.cache).to receive(:delete).with(['users', user.id, 'assigned_open_issues_count'])
expect(cache_mock).to receive(:delete).with(['users', user.id, 'assigned_open_issues_count'])
subject
end
end
allow(Rails).to receive(:cache).and_return(cache_mock)
it_behaves_like 'invalidates the cached value'
user.invalidate_issue_cache_counts
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
......
# 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
# 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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment