Commit 9dc276dd authored by Yorick Peterse's avatar Yorick Peterse

Merge branch 'ab-37462-cache-personal-projects-count' into 'master'

Cache personal projects count.

Closes #37462

See merge request gitlab-org/gitlab-ce!18197
parents 132de986 fa46b19d
...@@ -700,10 +700,6 @@ class User < ActiveRecord::Base ...@@ -700,10 +700,6 @@ class User < ActiveRecord::Base
projects_limit - personal_projects_count projects_limit - personal_projects_count
end end
def personal_projects_count
@personal_projects_count ||= personal_projects.count
end
def recent_push(project = nil) def recent_push(project = nil)
service = Users::LastPushEventService.new(self) service = Users::LastPushEventService.new(self)
...@@ -1046,6 +1042,12 @@ class User < ActiveRecord::Base ...@@ -1046,6 +1042,12 @@ class User < ActiveRecord::Base
end end
end end
def personal_projects_count(force: false)
Rails.cache.fetch(['users', id, 'personal_projects_count'], force: force, expires_in: 24.hours, raw: true) do
personal_projects.count
end.to_i
end
def update_todos_count_cache def update_todos_count_cache
todos_done_count(force: true) todos_done_count(force: true)
todos_pending_count(force: true) todos_pending_count(force: true)
...@@ -1056,6 +1058,7 @@ class User < ActiveRecord::Base ...@@ -1056,6 +1058,7 @@ class User < ActiveRecord::Base
invalidate_merge_request_cache_counts invalidate_merge_request_cache_counts
invalidate_todos_done_count invalidate_todos_done_count
invalidate_todos_pending_count invalidate_todos_pending_count
invalidate_personal_projects_count
end end
def invalidate_issue_cache_counts def invalidate_issue_cache_counts
...@@ -1074,6 +1077,10 @@ class User < ActiveRecord::Base ...@@ -1074,6 +1077,10 @@ class User < ActiveRecord::Base
Rails.cache.delete(['users', id, 'todos_pending_count']) Rails.cache.delete(['users', id, 'todos_pending_count'])
end end
def invalidate_personal_projects_count
Rails.cache.delete(['users', id, 'personal_projects_count'])
end
# This is copied from Devise::Models::Lockable#valid_for_authentication?, as our auth # This is copied from Devise::Models::Lockable#valid_for_authentication?, as our auth
# flow means we don't call that automatically (and can't conveniently do so). # flow means we don't call that automatically (and can't conveniently do so).
# #
......
...@@ -96,6 +96,8 @@ module Projects ...@@ -96,6 +96,8 @@ module Projects
system_hook_service.execute_hooks_for(@project, :create) system_hook_service.execute_hooks_for(@project, :create)
setup_authorizations setup_authorizations
current_user.invalidate_personal_projects_count
end end
# Refresh the current user's authorizations inline (so they can access the # Refresh the current user's authorizations inline (so they can access the
......
...@@ -34,6 +34,8 @@ module Projects ...@@ -34,6 +34,8 @@ module Projects
system_hook_service.execute_hooks_for(project, :destroy) system_hook_service.execute_hooks_for(project, :destroy)
log_info("Project \"#{project.full_path}\" was removed") log_info("Project \"#{project.full_path}\" was removed")
current_user.invalidate_personal_projects_count
true true
rescue => error rescue => error
attempt_rollback(project, error.message) attempt_rollback(project, error.message)
......
...@@ -24,6 +24,8 @@ module Projects ...@@ -24,6 +24,8 @@ module Projects
transfer(project) transfer(project)
current_user.invalidate_personal_projects_count
true true
rescue Projects::TransferService::TransferError => ex rescue Projects::TransferService::TransferError => ex
project.reload project.reload
......
---
title: Cache personal projects count.
merge_request: 18197
author:
type: performance
...@@ -2234,6 +2234,20 @@ describe User do ...@@ -2234,6 +2234,20 @@ describe User do
end end
end end
context '#invalidate_personal_projects_count' do
let(:user) { build_stubbed(:user) }
it 'invalidates cache for personal projects counter' do
cache_mock = double
expect(cache_mock).to receive(:delete).with(['users', user.id, 'personal_projects_count'])
allow(Rails).to receive(:cache).and_return(cache_mock)
user.invalidate_personal_projects_count
end
end
describe '#allow_password_authentication_for_web?' do describe '#allow_password_authentication_for_web?' do
context 'regular user' do context 'regular user' do
let(:user) { build(:user) } let(:user) { build(:user) }
...@@ -2283,11 +2297,9 @@ describe User do ...@@ -2283,11 +2297,9 @@ describe User do
user = build(:user) user = build(:user)
projects = double(:projects, count: 1) projects = double(:projects, count: 1)
expect(user).to receive(:personal_projects).once.and_return(projects) expect(user).to receive(:personal_projects).and_return(projects)
2.times do expect(user.personal_projects_count).to eq(1)
expect(user.personal_projects_count).to eq(1)
end
end end
end end
......
...@@ -28,6 +28,14 @@ describe Projects::CreateService, '#execute' do ...@@ -28,6 +28,14 @@ describe Projects::CreateService, '#execute' do
end end
end end
describe 'after create actions' do
it 'invalidate personal_projects_count caches' do
expect(user).to receive(:invalidate_personal_projects_count)
create_project(user, opts)
end
end
context "admin creates project with other user's namespace_id" do context "admin creates project with other user's namespace_id" do
it 'sets the correct permissions' do it 'sets the correct permissions' do
admin = create(:admin) admin = create(:admin)
......
...@@ -66,6 +66,12 @@ describe Projects::DestroyService do ...@@ -66,6 +66,12 @@ describe Projects::DestroyService do
end end
it_behaves_like 'deleting the project' it_behaves_like 'deleting the project'
it 'invalidates personal_project_count cache' do
expect(user).to receive(:invalidate_personal_projects_count)
destroy_project(project, user)
end
end end
context 'Sidekiq fake' do context 'Sidekiq fake' do
......
...@@ -37,6 +37,12 @@ describe Projects::TransferService do ...@@ -37,6 +37,12 @@ describe Projects::TransferService do
transfer_project(project, user, group) transfer_project(project, user, group)
end end
it 'invalidates the user\'s personal_project_count cache' do
expect(user).to receive(:invalidate_personal_projects_count)
transfer_project(project, user, group)
end
it 'executes system hooks' do it 'executes system hooks' do
transfer_project(project, user, group) do |service| transfer_project(project, user, group) do |service|
expect(service).to receive(:execute_system_hooks) expect(service).to receive(:execute_system_hooks)
......
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