Refactor: extract duplicate steps to a service class

app/services/projects/update_statistics_service.rb

+# frozen_string_literal: true
+
+module Projects
+  class UpdateStatisticsService < BaseService
+    def execute
+      return unless project && project.repository.exists?
+
+      Rails.logger.info("Updating statistics for project #{project.id}")
+
+      project.statistics.refresh!(only: statistics.map(&:to_sym))
+    end
+
+    private
+
+    def statistics
+      params[:statistics]
+    end
+  end
+end

app/workers/project_cache_worker.rb

@@ -18,7 +18,7 @@ class ProjectCacheWorker
 
     return unless project && project.repository.exists?
 
-    update_statistics(project, statistics.map(&:to_sym))
+    update_statistics(project, statistics)
 
     project.repository.refresh_method_caches(files.map(&:to_sym))
 
@@ -34,9 +34,8 @@ class ProjectCacheWorker
     return if Gitlab::Database.read_only?
     return unless try_obtain_lease_for(project.id, statistics)
 
-    Rails.logger.info("Updating statistics for project #{project.id}")
+    Projects::UpdateStatisticsService.new(project, nil, statistics: statistics).execute
 
-    project.statistics.refresh!(only: statistics)
     UpdateProjectStatisticsWorker.perform_in(LEASE_TIMEOUT, project.id, statistics)
   end
 

app/workers/update_project_statistics_worker.rb

@@ -12,11 +12,7 @@ class UpdateProjectStatisticsWorker
   def perform(project_id, statistics = [])
     project = Project.find_by(id: project_id)
 
-    return unless project && project.repository.exists?
-
-    Rails.logger.info("Updating statistics for project #{project.id}")
-
-    project.statistics.refresh!(only: statistics.map(&:to_sym))
+    Projects::UpdateStatisticsService.new(project, nil, statistics: statistics).execute
   end
   # rubocop: enable CodeReuse/ActiveRecord
 end

spec/services/projects/update_statistics_service_spec.rb

+require 'spec_helper'
+
+describe Projects::UpdateStatisticsService do
+  let(:service) { described_class.new(project, nil, statistics: statistics)}
+  let(:statistics) { %w(repository_size) }
+
+  describe '#execute' do
+    context 'with a non-existing project' do
+      let(:project) { nil }
+
+      it 'does nothing' do
+        expect_any_instance_of(ProjectStatistics).not_to receive(:refresh!)
+
+        service.execute
+      end
+    end
+
+    context 'with an existing project without a repository' do
+      let(:project) { create(:project) }
+
+      it 'does nothing' do
+        expect_any_instance_of(ProjectStatistics).not_to receive(:refresh!)
+
+        service.execute
+      end
+    end
+
+    context 'with an existing project with a repository' do
+      let(:project) { create(:project, :repository) }
+
+      it 'refreshes the project statistics' do
+        expect_any_instance_of(ProjectStatistics).to receive(:refresh!)
+          .with(only: statistics.map(&:to_sym))
+          .and_call_original
+
+        service.execute
+      end
+    end
+  end
+end

spec/workers/project_cache_worker_spec.rb

@@ -48,7 +48,7 @@ describe ProjectCacheWorker do
 
         it 'updates the project statistics' do
           expect(worker).to receive(:update_statistics)
-            .with(kind_of(Project), %i(repository_size))
+            .with(kind_of(Project), statistics)
             .and_call_original
 
           worker.perform(project.id, [], statistics)
@@ -73,28 +73,31 @@ describe ProjectCacheWorker do
     let(:statistics) { %w(repository_size) }
 
     context 'when a lease could not be obtained' do
-      it 'does not update the repository size' do
+      it 'does not update the project statistics' do
         stub_exclusive_lease_taken(lease_key, timeout: lease_timeout)
 
+        expect(Projects::UpdateStatisticsService).not_to receive(:new)
+
         expect(UpdateProjectStatisticsWorker).not_to receive(:perform_in)
 
-        worker.update_statistics(project, statistics.map(&:to_sym))
+        worker.update_statistics(project, statistics)
       end
     end
 
     context 'when a lease could be obtained' do
-      it 'updates the project statistics' do
+      it 'updates the project statistics twice' do
         stub_exclusive_lease(lease_key, timeout: lease_timeout)
 
-        expect(project.statistics).to receive(:refresh!)
-          .with(only: statistics.map(&:to_sym))
+        expect(Projects::UpdateStatisticsService).to receive(:new)
+          .with(project, nil, statistics: statistics)
           .and_call_original
+          .twice
 
         expect(UpdateProjectStatisticsWorker).to receive(:perform_in)
-          .with(lease_timeout, project.id, statistics.map(&:to_sym))
+          .with(lease_timeout, project.id, statistics)
           .and_call_original
 
-        worker.update_statistics(project, statistics.map(&:to_sym))
+        worker.update_statistics(project, statistics)
       end
     end
   end

spec/workers/update_project_statistics_worker_spec.rb

@@ -3,46 +3,15 @@ require 'spec_helper'
 describe UpdateProjectStatisticsWorker do
   let(:worker) { described_class.new }
   let(:project) { create(:project, :repository) }
+  let(:statistics) { %w(repository_size) }
 
   describe '#perform' do
-    context 'with a non-existing project' do
-      it 'does nothing' do
-        expect_any_instance_of(ProjectStatistics).not_to receive(:refresh!)
-
-        worker.perform(-1)
-      end
-    end
-
-    context 'with an existing project without a repository' do
-      it 'does nothing' do
-        allow_any_instance_of(Repository).to receive(:exists?).and_return(false)
-
-        expect_any_instance_of(ProjectStatistics).not_to receive(:refresh!)
-
-        worker.perform(project.id)
-      end
-    end
-
-    context 'with an existing project' do
-      it 'refreshes the project statistics' do
-        expect_any_instance_of(ProjectStatistics).to receive(:refresh!)
-          .with(only: [])
-          .and_call_original
-
-        worker.perform(project.id)
-      end
-
-      context 'with a specific statistics target' do
-        it 'refreshes the project repository size' do
-          statistics_target = %w(repository_size)
-
-          expect_any_instance_of(ProjectStatistics).to receive(:refresh!)
-            .with(only: statistics_target.map(&:to_sym))
+    it 'updates the project statistics' do
+      expect(Projects::UpdateStatisticsService).to receive(:new)
+        .with(project, nil, statistics: statistics)
         .and_call_original
 
-          worker.perform(project.id, statistics_target)
-        end
-      end
+      worker.perform(project.id, statistics)
     end
   end
 end