diff options
author | Gabriel Mazetto <brodock@gmail.com> | 2019-01-22 04:33:32 +0100 |
---|---|---|
committer | Gabriel Mazetto <brodock@gmail.com> | 2019-01-22 13:55:18 +0100 |
commit | d391dfb4acd1c75857b1578c449b0e508fc8a0ed (patch) | |
tree | 8899b80cb615dd518efe2b8a614c177ab0ca0b89 /app | |
parent | 7a7948e64b2d5bd9f00f9f58862b0a0599e78c83 (diff) | |
download | gitlab-ce-d391dfb4acd1c75857b1578c449b0e508fc8a0ed.tar.gz |
Refactored AfterRenameService to reduce coupling
We still rely on the Dirty API for project rename (before/after) values,
but we don't access the dirty api from the service class anymore.
The previous value is now part of the initialization, which makes it
easier to test and the behavior is clearer.
The same was done with the `rename_repo` on the Storage classes, we now
provide before and after values as part of the method signature.
Diffstat (limited to 'app')
-rw-r--r-- | app/models/storage/hashed_project.rb | 2 | ||||
-rw-r--r-- | app/models/storage/legacy_project.rb | 11 | ||||
-rw-r--r-- | app/services/projects/after_rename_service.rb | 29 | ||||
-rw-r--r-- | app/services/projects/update_service.rb | 5 |
4 files changed, 28 insertions, 19 deletions
diff --git a/app/models/storage/hashed_project.rb b/app/models/storage/hashed_project.rb index 911fb7e9ce9..f5d0d6fab3b 100644 --- a/app/models/storage/hashed_project.rb +++ b/app/models/storage/hashed_project.rb @@ -31,7 +31,7 @@ module Storage gitlab_shell.add_namespace(repository_storage, base_dir) end - def rename_repo + def rename_repo(old_full_path: nil, new_full_path: nil) true end diff --git a/app/models/storage/legacy_project.rb b/app/models/storage/legacy_project.rb index 9f6f19acb41..76ac5c13c18 100644 --- a/app/models/storage/legacy_project.rb +++ b/app/models/storage/legacy_project.rb @@ -29,18 +29,19 @@ module Storage gitlab_shell.add_namespace(repository_storage, base_dir) end - def rename_repo - new_full_path = project.build_full_path + def rename_repo(old_full_path: nil, new_full_path: nil) + old_full_path ||= project.full_path_was + new_full_path ||= project.build_full_path - if gitlab_shell.mv_repository(repository_storage, project.full_path_was, new_full_path) + if gitlab_shell.mv_repository(repository_storage, old_full_path, new_full_path) # If repository moved successfully we need to send update instructions to users. # However we cannot allow rollback since we moved repository # So we basically we mute exceptions in next actions begin - gitlab_shell.mv_repository(repository_storage, "#{project.full_path_was}.wiki", "#{new_full_path}.wiki") + gitlab_shell.mv_repository(repository_storage, "#{old_full_path}.wiki", "#{new_full_path}.wiki") return true rescue => e - Rails.logger.error "Exception renaming #{project.full_path_was} -> #{new_full_path}: #{e}" + Rails.logger.error "Exception renaming #{old_full_path} -> #{new_full_path}: #{e}" # Returning false does not rollback after_* transaction but gives # us information about failing some of tasks return false diff --git a/app/services/projects/after_rename_service.rb b/app/services/projects/after_rename_service.rb index 9ace1f85336..c3cd9d1ea4a 100644 --- a/app/services/projects/after_rename_service.rb +++ b/app/services/projects/after_rename_service.rb @@ -12,22 +12,27 @@ module Projects # # Projects::AfterRenameService.new(project).execute class AfterRenameService - attr_reader :project, :full_path_before, :full_path_after, :path_before + # @return [String] The Project being renamed. + attr_reader :project - RenameFailedError = Class.new(StandardError) + # @return [String] The path slug the project was using, before the rename took place. + attr_reader :path_before - # @param [Project] project The Project of the repository to rename. - def initialize(project) - @project = project + # @return [String] The full path of the namespace + project, before the rename took place. + attr_reader :full_path_before - # The full path of the namespace + project, before the rename took place. - @full_path_before = project.full_path_was + # @return [String] The full path of the namespace + project, after the rename took place. + attr_reader :full_path_after - # The full path of the namespace + project, after the rename took place. - @full_path_after = project.build_full_path + RenameFailedError = Class.new(StandardError) - # The path of just the project, before the rename took place. - @path_before = project.previous_changes['path'].first + # @param [Project] project The Project being renamed. + # @param [String] path_before The path slug the project was using, before the rename took place. + def initialize(project, path_before:, full_path_before:) + @project = project + @path_before = path_before + @full_path_before = full_path_before + @full_path_after = project.full_path end def execute @@ -61,7 +66,7 @@ module Projects .new(project, full_path_before) .execute else - project.storage.rename_repo + project.storage.rename_repo(old_full_path: full_path_before, new_full_path: full_path_after) end rename_failed! unless success diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index d5a3a4043f3..6856009b395 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -76,7 +76,10 @@ module Projects end def after_rename_service(project) - AfterRenameService.new(project) + # The path slug the project was using, before the rename took place. + path_before = project.previous_changes['path'].first + + AfterRenameService.new(project, path_before: path_before, full_path_before: project.full_path_was) end def changing_pages_related_config? |