From 4b9c17f196bab6075563f62d01f9db65c1a0515c Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Wed, 17 Oct 2018 13:33:54 +0200 Subject: Move Project#rename_repo to a service class This moves the logic of Project#rename_repo and all methods _only_ used by this method into a new service class: Projects::AfterRenameService. By moving this code into a separate service class we can more easily refactor it, and we also get rid of some RuboCop "disable" statements automatically. During the refactoring of this code, I removed most of the explicit logging using Gitlab::AppLogger. The data that was logged would not be useful when debugging renaming issues, as it does not add any value on top of data provided by users. I also removed a variety of comments that either mentioned something the code does in literal form, or contained various grammatical errors. Instead we now resort to more clearly named methods, removing the need for code comments. This method was chosen based on analysis in https://gitlab.com/gitlab-org/release/framework/issues/28. In this issue we determined this method has seen a total of 293 lines being changed in it. We also noticed that RuboCop determined the ABC size (https://www.softwarerenovation.com/ABCMetric.pdf) was too great. --- spec/migrations/rename_more_reserved_project_names_spec.rb | 11 ++++++++++- spec/migrations/rename_reserved_project_names_spec.rb | 11 ++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) (limited to 'spec/migrations') diff --git a/spec/migrations/rename_more_reserved_project_names_spec.rb b/spec/migrations/rename_more_reserved_project_names_spec.rb index 034e8a6a4e5..baf16c2ce53 100644 --- a/spec/migrations/rename_more_reserved_project_names_spec.rb +++ b/spec/migrations/rename_more_reserved_project_names_spec.rb @@ -31,7 +31,16 @@ describe RenameMoreReservedProjectNames, :delete do context 'when exception is raised during rename' do before do - allow(project).to receive(:rename_repo).and_raise(StandardError) + service = instance_double('service') + + allow(service) + .to receive(:execute) + .and_raise(Projects::AfterRenameService::RenameFailedError) + + allow(Projects::AfterRenameService) + .to receive(:new) + .with(project) + .and_return(service) end it 'captures exception from project rename' do diff --git a/spec/migrations/rename_reserved_project_names_spec.rb b/spec/migrations/rename_reserved_project_names_spec.rb index 592ac2b5fb9..7818aa0d560 100644 --- a/spec/migrations/rename_reserved_project_names_spec.rb +++ b/spec/migrations/rename_reserved_project_names_spec.rb @@ -35,7 +35,16 @@ describe RenameReservedProjectNames, :migration, schema: :latest do context 'when exception is raised during rename' do before do - allow(project).to receive(:rename_repo).and_raise(StandardError) + service = instance_double('service') + + allow(service) + .to receive(:execute) + .and_raise(Projects::AfterRenameService::RenameFailedError) + + allow(Projects::AfterRenameService) + .to receive(:new) + .with(project) + .and_return(service) end it 'captures exception from project rename' do -- cgit v1.2.1