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

Cache personal projects count.

Closes #37462

See merge request gitlab-org/gitlab-ce!18197

9 files changed, 58 insertions, 8 deletions

diff --git a/app/models/user.rb b/app/models/user.rb
index ce56b39b1c8..2b95be3f888 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -700,10 +700,6 @@ class User < ActiveRecord::Base
     projects_limit - personal_projects_count
   end
 
-  def personal_projects_count
-    @personal_projects_count ||= personal_projects.count
-  end
-
   def recent_push(project = nil)
     service = Users::LastPushEventService.new(self)
 
@@ -1046,6 +1042,12 @@ class User < ActiveRecord::Base
     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
     todos_done_count(force: true)
     todos_pending_count(force: true)
@@ -1056,6 +1058,7 @@ class User < ActiveRecord::Base
     invalidate_merge_request_cache_counts
     invalidate_todos_done_count
     invalidate_todos_pending_count
+    invalidate_personal_projects_count
   end
 
   def invalidate_issue_cache_counts
@@ -1074,6 +1077,10 @@ class User < ActiveRecord::Base
     Rails.cache.delete(['users', id, 'todos_pending_count'])
   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
   # flow means we don't call that automatically (and can't conveniently do so).
   #
diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb
index 633e2c8236c..d361d070993 100644
--- a/app/services/projects/create_service.rb
+++ b/app/services/projects/create_service.rb
@@ -96,6 +96,8 @@ module Projects
       system_hook_service.execute_hooks_for(@project, :create)
 
       setup_authorizations
+
+      current_user.invalidate_personal_projects_count
     end
 
     # Refresh the current user's authorizations inline (so they can access the
diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb
index 4b8f955ae69..114762c208e 100644
--- a/app/services/projects/destroy_service.rb
+++ b/app/services/projects/destroy_service.rb
@@ -34,6 +34,8 @@ module Projects
       system_hook_service.execute_hooks_for(project, :destroy)
       log_info("Project \"#{project.full_path}\" was removed")
 
+      current_user.invalidate_personal_projects_count
+
       true
     rescue => error
       attempt_rollback(project, error.message)
diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb
index 26765e5c3f3..5a23f0f0a62 100644
--- a/app/services/projects/transfer_service.rb
+++ b/app/services/projects/transfer_service.rb
@@ -24,6 +24,8 @@ module Projects
 
       transfer(project)
 
+      current_user.invalidate_personal_projects_count
+
       true
     rescue Projects::TransferService::TransferError => ex
       project.reload
diff --git a/changelogs/unreleased/ab-37462-cache-personal-projects-count.yml b/changelogs/unreleased/ab-37462-cache-personal-projects-count.yml
new file mode 100644
index 00000000000..55069b1f2d2
--- /dev/null
+++ b/changelogs/unreleased/ab-37462-cache-personal-projects-count.yml
@@ -0,0 +1,5 @@
+---
+title: Cache personal projects count.
+merge_request: 18197
+author:
+type: performance
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index a600987d0bf..73266c0085f 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -2234,6 +2234,20 @@ describe User do
     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
     context 'regular user' do
       let(:user) { build(:user) }
@@ -2283,11 +2297,9 @@ describe User do
       user = build(:user)
       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)
-      end
+      expect(user.personal_projects_count).to eq(1)
     end
   end
 
diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb
index 2cacb97a293..e35f0f6337a 100644
--- a/spec/services/projects/create_service_spec.rb
+++ b/spec/services/projects/create_service_spec.rb
@@ -28,6 +28,14 @@ describe Projects::CreateService, '#execute' do
     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
     it 'sets the correct permissions' do
       admin = create(:admin)
diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb
index 0bec2054f50..9bb1cda565c 100644
--- a/spec/services/projects/destroy_service_spec.rb
+++ b/spec/services/projects/destroy_service_spec.rb
@@ -66,6 +66,12 @@ describe Projects::DestroyService do
     end
 
     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
 
   context 'Sidekiq fake' do
diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb
index 95a6771c59d..ff9b2372a35 100644
--- a/spec/services/projects/transfer_service_spec.rb
+++ b/spec/services/projects/transfer_service_spec.rb
@@ -37,6 +37,12 @@ describe Projects::TransferService do
       transfer_project(project, user, group)
     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
       transfer_project(project, user, group) do |service|
         expect(service).to receive(:execute_system_hooks)