Merge branch 'fix_housekeeping_trigger' into 'master'

Only trigger housekeeping once per push

See merge request gitlab-org/gitlab!75581

app/services/git/branch_push_service.rb

@@ -26,7 +26,6 @@ module Git
       enqueue_detect_repository_languages
 
       execute_related_hooks
-      perform_housekeeping
 
       stop_environments
       unlock_artifacts
@@ -71,13 +70,6 @@ module Git
       BranchHooksService.new(project, current_user, params).execute
     end
 
-    def perform_housekeeping
-      housekeeping = Repositories::HousekeepingService.new(project)
-      housekeeping.increment!
-      housekeeping.execute if housekeeping.needed?
-    rescue Repositories::HousekeepingService::LeaseTaken
-    end
-
     def removing_branch?
       Gitlab::Git.blank_ref?(newrev)
     end

app/services/git/process_ref_changes_service.rb

@@ -9,6 +9,8 @@ module Git
 
       process_changes_by_action(:branch, changes.branch_changes)
       process_changes_by_action(:tag, changes.tag_changes)
+
+      perform_housekeeping
     end
 
     private
@@ -83,5 +85,12 @@ module Git
 
       MergeRequests::PushedBranchesService.new(project: project, current_user: current_user, params: { changes: changes }).execute
     end
+
+    def perform_housekeeping
+      housekeeping = Repositories::HousekeepingService.new(project)
+      housekeeping.increment!
+      housekeeping.execute if housekeeping.needed?
+    rescue Repositories::HousekeepingService::LeaseTaken
+    end
   end
 end

spec/services/git/branch_push_service_spec.rb

@@ -554,44 +554,6 @@ RSpec.describe Git::BranchPushService, services: true do
     end
   end
 
-  describe "housekeeping", :clean_gitlab_redis_cache, :clean_gitlab_redis_queues, :clean_gitlab_redis_shared_state do
-    let(:housekeeping) { Repositories::HousekeepingService.new(project) }
-
-    before do
-      allow(Repositories::HousekeepingService).to receive(:new).and_return(housekeeping)
-    end
-
-    it 'does not perform housekeeping when not needed' do
-      expect(housekeeping).not_to receive(:execute)
-
-      execute_service(project, user, oldrev: oldrev, newrev: newrev, ref: ref)
-    end
-
-    context 'when housekeeping is needed' do
-      before do
-        allow(housekeeping).to receive(:needed?).and_return(true)
-      end
-
-      it 'performs housekeeping' do
-        expect(housekeeping).to receive(:execute)
-
-        execute_service(project, user, oldrev: oldrev, newrev: newrev, ref: ref)
-      end
-
-      it 'does not raise an exception' do
-        allow(housekeeping).to receive(:try_obtain_lease).and_return(false)
-
-        execute_service(project, user, oldrev: oldrev, newrev: newrev, ref: ref)
-      end
-    end
-
-    it 'increments the push counter' do
-      expect(housekeeping).to receive(:increment!)
-
-      execute_service(project, user, oldrev: oldrev, newrev: newrev, ref: ref)
-    end
-  end
-
   describe "CI environments" do
     context 'create branch' do
       let(:oldrev) { blankrev }

spec/services/git/process_ref_changes_service_spec.rb

@@ -161,6 +161,50 @@ RSpec.describe Git::ProcessRefChangesService do
         end
       end
     end
+
+    describe "housekeeping", :clean_gitlab_redis_cache, :clean_gitlab_redis_queues, :clean_gitlab_redis_shared_state do
+      let(:housekeeping) { Repositories::HousekeepingService.new(project) }
+
+      before do
+        allow(Repositories::HousekeepingService).to receive(:new).and_return(housekeeping)
+
+        allow(push_service_class)
+          .to receive(:new)
+          .with(project, project.owner, hash_including(execute_project_hooks: true, create_push_event: true))
+          .exactly(changes.count).times
+          .and_return(service)
+      end
+
+      it 'does not perform housekeeping when not needed' do
+        expect(housekeeping).not_to receive(:execute)
+
+        subject.execute
+      end
+
+      context 'when housekeeping is needed' do
+        before do
+          allow(housekeeping).to receive(:needed?).and_return(true)
+        end
+
+        it 'performs housekeeping' do
+          expect(housekeeping).to receive(:execute)
+
+          subject.execute
+        end
+
+        it 'does not raise an exception' do
+          allow(housekeeping).to receive(:try_obtain_lease).and_return(false)
+
+          subject.execute
+        end
+      end
+
+      it 'increments the push counter' do
+        expect(housekeeping).to receive(:increment!)
+
+        subject.execute
+      end
+    end
   end
 
   context 'branch changes' do