diff options
| -rw-r--r-- | app/models/project.rb | 79 | ||||
| -rw-r--r-- | app/services/projects/after_rename_service.rb | 135 | ||||
| -rw-r--r-- | app/services/projects/update_service.rb | 2 | ||||
| -rw-r--r-- | db/post_migrate/20161221153951_rename_reserved_project_names.rb | 6 | ||||
| -rw-r--r-- | db/post_migrate/20170313133418_rename_more_reserved_project_names.rb | 6 | ||||
| -rw-r--r-- | spec/migrations/rename_more_reserved_project_names_spec.rb | 11 | ||||
| -rw-r--r-- | spec/migrations/rename_reserved_project_names_spec.rb | 11 | ||||
| -rw-r--r-- | spec/models/project_spec.rb | 167 | ||||
| -rw-r--r-- | spec/services/projects/after_rename_service_spec.rb | 198 |
9 files changed, 368 insertions, 247 deletions
diff --git a/app/models/project.rb b/app/models/project.rb index b80e41e4a96..731272b3bf8 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1640,34 +1640,6 @@ class Project < ActiveRecord::Base end # rubocop: enable CodeReuse/ServiceClass - def rename_repo - path_before = previous_changes['path'].first - full_path_before = full_path_was - full_path_after = build_full_path - - Gitlab::AppLogger.info("Attempting to rename #{full_path_was} -> #{full_path_after}") - - if has_container_registry_tags? - Gitlab::AppLogger.info("Project #{full_path_was} cannot be renamed because container registry tags are present!") - - # we currently don't support renaming repository if it contains images in container registry - raise StandardError.new('Project cannot be renamed, because images are present in its container registry') - end - - expire_caches_before_rename(full_path_before) - - if rename_or_migrate_repository! - Gitlab::AppLogger.info("Project was renamed: #{full_path_before} -> #{full_path_after}") - after_rename_repository(full_path_before, path_before) - else - Gitlab::AppLogger.info("Repository could not be renamed: #{full_path_before} -> #{full_path_after}") - - # if we cannot move namespace directory we should rollback - # db changes in order to prevent out of sync between db and fs - raise StandardError.new('Repository cannot be renamed') - end - end - def write_repository_config(gl_full_path: full_path) # We'd need to keep track of project full path otherwise directory tree # created with hashed storage enabled cannot be usefully imported using @@ -2096,51 +2068,6 @@ class Project < ActiveRecord::Base auto_cancel_pending_pipelines == 'enabled' end - private - - # rubocop: disable CodeReuse/ServiceClass - def rename_or_migrate_repository! - if Gitlab::CurrentSettings.hashed_storage_enabled? && - storage_upgradable? && - Feature.disabled?(:skip_hashed_storage_upgrade) # kill switch in case we need to disable upgrade behavior - ::Projects::HashedStorageMigrationService.new(self, full_path_was).execute - else - storage.rename_repo - end - end - # rubocop: enable CodeReuse/ServiceClass - - def storage_upgradable? - storage_version != LATEST_STORAGE_VERSION - end - - def after_rename_repository(full_path_before, path_before) - execute_rename_repository_hooks!(full_path_before) - - write_repository_config - - # We need to check if project had been rolled out to move resource to hashed storage or not and decide - # if we need execute any take action or no-op. - unless hashed_storage?(:attachments) - Gitlab::UploadsTransfer.new.rename_project(path_before, self.path, namespace.full_path) - end - - Gitlab::PagesTransfer.new.rename_project(path_before, self.path, namespace.full_path) - end - - # rubocop: disable CodeReuse/ServiceClass - def execute_rename_repository_hooks!(full_path_before) - # When we import a project overwriting the original project, there - # is a move operation. In that case we don't want to send the instructions. - send_move_instructions(full_path_before) unless import_started? - - self.old_path_with_namespace = full_path_before - SystemHooksService.new.execute_hooks_for(self, :rename) - - reload_repository! - end - # rubocop: enable CodeReuse/ServiceClass - def storage @storage ||= if hashed_storage?(:repository) @@ -2150,6 +2077,12 @@ class Project < ActiveRecord::Base end end + def storage_upgradable? + storage_version != LATEST_STORAGE_VERSION + end + + private + def use_hashed_storage if self.new_record? && Gitlab::CurrentSettings.hashed_storage_enabled self.storage_version = LATEST_STORAGE_VERSION diff --git a/app/services/projects/after_rename_service.rb b/app/services/projects/after_rename_service.rb new file mode 100644 index 00000000000..4131da44f5a --- /dev/null +++ b/app/services/projects/after_rename_service.rb @@ -0,0 +1,135 @@ +# frozen_string_literal: true + +module Projects + # Service class for performing operations that should take place after a + # project has been renamed. + # + # Example usage: + # + # project = Project.find(42) + # + # project.update(...) + # + # Projects::AfterRenameService.new(project).execute + class AfterRenameService + attr_reader :project, :full_path_before, :full_path_after, :path_before + + RenameFailedError = Class.new(StandardError) + + # @param [Project] project The Project of the repository to rename. + def initialize(project) + @project = project + + # The full path of the namespace + project, before the rename took place. + @full_path_before = project.full_path_was + + # The full path of the namespace + project, after the rename took place. + @full_path_after = project.build_full_path + + # The path of just the project, before the rename took place. + @path_before = project.path_was + end + + def execute + first_ensure_no_registry_tags_are_present + expire_caches_before_rename + rename_or_migrate_repository! + send_move_instructions + execute_system_hooks + update_repository_configuration + rename_transferred_documents + log_completion + end + + def first_ensure_no_registry_tags_are_present + return unless project.has_container_registry_tags? + + raise RenameFailedError.new( + "Project #{full_path_before} cannot be renamed because images are " \ + "present in its container registry" + ) + end + + def expire_caches_before_rename + project.expire_caches_before_rename(full_path_before) + end + + def rename_or_migrate_repository! + success = + if migrate_to_hashed_storage? + ::Projects::HashedStorageMigrationService + .new(project, full_path_before) + .execute + else + project.storage.rename_repo + end + + rename_failed! unless success + end + + def send_move_instructions + return unless send_move_instructions? + + project.send_move_instructions(full_path_before) + end + + def execute_system_hooks + project.old_path_with_namespace = full_path_before + SystemHooksService.new.execute_hooks_for(project, :rename) + end + + def update_repository_configuration + project.reload_repository! + project.write_repository_config + end + + def rename_transferred_documents + if rename_uploads? + Gitlab::UploadsTransfer + .new + .rename_project(path_before, project_path, namespace_full_path) + end + + Gitlab::PagesTransfer + .new + .rename_project(path_before, project_path, namespace_full_path) + end + + def log_completion + Gitlab::AppLogger.info( + "Project #{project.id} has been renamed from " \ + "#{full_path_before} to #{full_path_after}" + ) + end + + def migrate_to_hashed_storage? + Gitlab::CurrentSettings.hashed_storage_enabled? && + project.storage_upgradable? && + Feature.disabled?(:skip_hashed_storage_upgrade) + end + + def send_move_instructions? + !project.import_started? + end + + def rename_uploads? + !project.hashed_storage?(:attachments) + end + + def project_path + project.path + end + + def namespace_full_path + project.namespace.full_path + end + + def rename_failed! + error = "Repository #{full_path_before} could not be renamed to #{full_path_after}" + + Gitlab::AppLogger.error(error) + + raise RenameFailedError.new(error) + end + end +end diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index f25a4e30938..93e48fc0199 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -67,7 +67,7 @@ module Projects end if project.previous_changes.include?('path') - project.rename_repo + AfterRenameService.new(project).execute else system_hook_service.execute_hooks_for(project, :update) end diff --git a/db/post_migrate/20161221153951_rename_reserved_project_names.rb b/db/post_migrate/20161221153951_rename_reserved_project_names.rb index 08d7f499eec..678876e886c 100644 --- a/db/post_migrate/20161221153951_rename_reserved_project_names.rb +++ b/db/post_migrate/20161221153951_rename_reserved_project_names.rb @@ -113,7 +113,9 @@ class RenameReservedProjectNames < ActiveRecord::Migration begin # Because project path update is quite complex operation we can't safely # copy-paste all code from GitLab. As exception we use Rails code here - project.rename_repo if rename_project_row(project, path) + if rename_project_row(project, path) + Projects::AfterRenameService.new(project).execute + end rescue Exception => e # rubocop: disable Lint/RescueException Rails.logger.error "Exception when renaming project #{id}: #{e.message}" end @@ -123,6 +125,6 @@ class RenameReservedProjectNames < ActiveRecord::Migration def rename_project_row(project, path) project.respond_to?(:update_attributes) && project.update(path: path) && - project.respond_to?(:rename_repo) + defined?(Projects::AfterRenameService) end end diff --git a/db/post_migrate/20170313133418_rename_more_reserved_project_names.rb b/db/post_migrate/20170313133418_rename_more_reserved_project_names.rb index 43a37667250..26a67b0f814 100644 --- a/db/post_migrate/20170313133418_rename_more_reserved_project_names.rb +++ b/db/post_migrate/20170313133418_rename_more_reserved_project_names.rb @@ -55,7 +55,9 @@ class RenameMoreReservedProjectNames < ActiveRecord::Migration begin # Because project path update is quite complex operation we can't safely # copy-paste all code from GitLab. As exception we use Rails code here - project.rename_repo if rename_project_row(project, path) + if rename_project_row(project, path) + Projects::AfterRenameService.new(project).execute + end rescue Exception => e # rubocop: disable Lint/RescueException Rails.logger.error "Exception when renaming project #{id}: #{e.message}" end @@ -65,6 +67,6 @@ class RenameMoreReservedProjectNames < ActiveRecord::Migration def rename_project_row(project, path) project.respond_to?(:update_attributes) && project.update(path: path) && - project.respond_to?(:rename_repo) + defined?(Projects::AfterRenameService) end end 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 diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index a807c336165..fd6b8e62b53 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2956,88 +2956,6 @@ describe Project do end end - describe '#rename_repo' do - before do - # Project#gitlab_shell returns a new instance of Gitlab::Shell on every - # call. This makes testing a bit easier. - allow(project).to receive(:gitlab_shell).and_return(gitlab_shell) - allow(project).to receive(:previous_changes).and_return('path' => ['foo']) - stub_feature_flags(skip_hashed_storage_upgrade: false) - end - - it 'renames a repository' do - stub_container_registry_config(enabled: false) - - expect(gitlab_shell).to receive(:mv_repository) - .ordered - .with(project.repository_storage, "#{project.namespace.full_path}/foo", "#{project.full_path}") - .and_return(true) - - expect(gitlab_shell).to receive(:mv_repository) - .ordered - .with(project.repository_storage, "#{project.namespace.full_path}/foo.wiki", "#{project.full_path}.wiki") - .and_return(true) - - expect_any_instance_of(SystemHooksService) - .to receive(:execute_hooks_for) - .with(project, :rename) - - expect_any_instance_of(Gitlab::UploadsTransfer) - .to receive(:rename_project) - .with('foo', project.path, project.namespace.full_path) - - expect(project).to receive(:expire_caches_before_rename) - - project.rename_repo - end - - context 'container registry with images' do - let(:container_repository) { create(:container_repository) } - - before do - stub_container_registry_config(enabled: true) - stub_container_registry_tags(repository: :any, tags: ['tag']) - project.container_repositories << container_repository - end - - subject { project.rename_repo } - - it { expect { subject }.to raise_error(StandardError) } - end - - context 'gitlab pages' do - before do - expect(project_storage).to receive(:rename_repo) { true } - end - - it 'moves pages folder to new location' do - expect_any_instance_of(Gitlab::PagesTransfer).to receive(:rename_project) - - project.rename_repo - end - end - - context 'attachments' do - before do - expect(project_storage).to receive(:rename_repo) { true } - end - - it 'moves uploads folder to new location' do - expect_any_instance_of(Gitlab::UploadsTransfer).to receive(:rename_project) - - project.rename_repo - end - end - - it 'updates project full path in .git/config' do - allow(project_storage).to receive(:rename_repo).and_return(true) - - project.rename_repo - - expect(rugged_config['gitlab.fullpath']).to eq(project.full_path) - end - end - describe '#pages_path' do it 'returns a path where pages are stored' do expect(project.pages_path).to eq(File.join(Settings.pages.path, project.namespace.full_path, project.path)) @@ -3128,91 +3046,6 @@ describe Project do end end - describe '#rename_repo' do - before do - # Project#gitlab_shell returns a new instance of Gitlab::Shell on every - # call. This makes testing a bit easier. - allow(project).to receive(:gitlab_shell).and_return(gitlab_shell) - allow(project).to receive(:previous_changes).and_return('path' => ['foo']) - stub_feature_flags(skip_hashed_storage_upgrade: false) - end - - context 'migration to hashed storage' do - it 'calls HashedStorageMigrationService with correct options' do - project = create(:project, :repository, :legacy_storage) - allow(project).to receive(:previous_changes).and_return('path' => ['foo']) - - expect_next_instance_of(::Projects::HashedStorageMigrationService) do |service| - expect(service).to receive(:execute).and_return(true) - end - - project.rename_repo - end - end - - it 'renames a repository' do - stub_container_registry_config(enabled: false) - - expect(gitlab_shell).not_to receive(:mv_repository) - - expect_any_instance_of(SystemHooksService) - .to receive(:execute_hooks_for) - .with(project, :rename) - - expect(project).to receive(:expire_caches_before_rename) - - project.rename_repo - end - - context 'container registry with images' do - let(:container_repository) { create(:container_repository) } - - before do - stub_container_registry_config(enabled: true) - stub_container_registry_tags(repository: :any, tags: ['tag']) - project.container_repositories << container_repository - end - - subject { project.rename_repo } - - it { expect { subject }.to raise_error(StandardError) } - end - - context 'gitlab pages' do - it 'moves pages folder to new location' do - expect_any_instance_of(Gitlab::PagesTransfer).to receive(:rename_project) - - project.rename_repo - end - end - - context 'attachments' do - it 'keeps uploads folder location unchanged' do - expect_any_instance_of(Gitlab::UploadsTransfer).not_to receive(:rename_project) - - project.rename_repo - end - - context 'when not rolled out' do - let(:project) { create(:project, :repository, storage_version: 1, skip_disk_validation: true) } - - it 'moves pages folder to hashed storage' do - expect_next_instance_of(Projects::HashedStorage::MigrateAttachmentsService) do |service| - expect(service).to receive(:execute) - end - - project.rename_repo - end - end - end - - it 'updates project full path in .git/config' do - project.rename_repo - - expect(rugged_config['gitlab.fullpath']).to eq(project.full_path) - end - end - describe '#pages_path' do it 'returns a path where pages are stored' do expect(project.pages_path).to eq(File.join(Settings.pages.path, project.namespace.full_path, project.path)) diff --git a/spec/services/projects/after_rename_service_spec.rb b/spec/services/projects/after_rename_service_spec.rb new file mode 100644 index 00000000000..b4718a07204 --- /dev/null +++ b/spec/services/projects/after_rename_service_spec.rb @@ -0,0 +1,198 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Projects::AfterRenameService do + let(:rugged_config) { rugged_repo(project.repository).config } + + describe '#execute' do + context 'using legacy storage' do + let(:project) { create(:project, :repository, :legacy_storage) } + let(:gitlab_shell) { Gitlab::Shell.new } + let(:project_storage) { project.send(:storage) } + + before do + # Project#gitlab_shell returns a new instance of Gitlab::Shell on every + # call. This makes testing a bit easier. + allow(project).to receive(:gitlab_shell).and_return(gitlab_shell) + + allow(project) + .to receive(:previous_changes) + .and_return('path' => ['foo']) + + allow(project) + .to receive(:path_was) + .and_return('foo') + + stub_feature_flags(skip_hashed_storage_upgrade: false) + end + + it 'renames a repository' do + stub_container_registry_config(enabled: false) + + expect(gitlab_shell).to receive(:mv_repository) + .ordered + .with(project.repository_storage, "#{project.namespace.full_path}/foo", "#{project.full_path}") + .and_return(true) + + expect(gitlab_shell).to receive(:mv_repository) + .ordered + .with(project.repository_storage, "#{project.namespace.full_path}/foo.wiki", "#{project.full_path}.wiki") + .and_return(true) + + expect_any_instance_of(SystemHooksService) + .to receive(:execute_hooks_for) + .with(project, :rename) + + expect_any_instance_of(Gitlab::UploadsTransfer) + .to receive(:rename_project) + .with('foo', project.path, project.namespace.full_path) + + expect(project).to receive(:expire_caches_before_rename) + + described_class.new(project).execute + end + + context 'container registry with images' do + let(:container_repository) { create(:container_repository) } + + before do + stub_container_registry_config(enabled: true) + stub_container_registry_tags(repository: :any, tags: ['tag']) + project.container_repositories << container_repository + end + + it 'raises a RenameFailedError' do + expect { described_class.new(project).execute } + .to raise_error(described_class::RenameFailedError) + end + end + + context 'gitlab pages' do + before do + expect(project_storage).to receive(:rename_repo) { true } + end + + it 'moves pages folder to new location' do + expect_any_instance_of(Gitlab::PagesTransfer).to receive(:rename_project) + + described_class.new(project).execute + end + end + + context 'attachments' do + before do + expect(project_storage).to receive(:rename_repo) { true } + end + + it 'moves uploads folder to new location' do + expect_any_instance_of(Gitlab::UploadsTransfer).to receive(:rename_project) + + described_class.new(project).execute + end + end + + it 'updates project full path in .git/config' do + allow(project_storage).to receive(:rename_repo).and_return(true) + + described_class.new(project).execute + + expect(rugged_config['gitlab.fullpath']).to eq(project.full_path) + end + end + + context 'using hashed storage' do + let(:project) { create(:project, :repository, skip_disk_validation: true) } + let(:gitlab_shell) { Gitlab::Shell.new } + let(:hash) { Digest::SHA2.hexdigest(project.id.to_s) } + let(:hashed_prefix) { File.join('@hashed', hash[0..1], hash[2..3]) } + let(:hashed_path) { File.join(hashed_prefix, hash) } + + before do + # Project#gitlab_shell returns a new instance of Gitlab::Shell on every + # call. This makes testing a bit easier. + allow(project).to receive(:gitlab_shell).and_return(gitlab_shell) + allow(project).to receive(:previous_changes).and_return('path' => ['foo']) + + stub_feature_flags(skip_hashed_storage_upgrade: false) + stub_application_setting(hashed_storage_enabled: true) + end + + context 'migration to hashed storage' do + it 'calls HashedStorageMigrationService with correct options' do + project = create(:project, :repository, :legacy_storage) + allow(project).to receive(:previous_changes).and_return('path' => ['foo']) + + expect_next_instance_of(::Projects::HashedStorageMigrationService) do |service| + expect(service).to receive(:execute).and_return(true) + end + + described_class.new(project).execute + end + end + + it 'renames a repository' do + stub_container_registry_config(enabled: false) + + expect(gitlab_shell).not_to receive(:mv_repository) + + expect_any_instance_of(SystemHooksService) + .to receive(:execute_hooks_for) + .with(project, :rename) + + expect(project).to receive(:expire_caches_before_rename) + + described_class.new(project).execute + end + + context 'container registry with images' do + let(:container_repository) { create(:container_repository) } + + before do + stub_container_registry_config(enabled: true) + stub_container_registry_tags(repository: :any, tags: ['tag']) + project.container_repositories << container_repository + end + + it 'raises a RenameFailedError' do + expect { described_class.new(project).execute } + .to raise_error(described_class::RenameFailedError) + end + end + + context 'gitlab pages' do + it 'moves pages folder to new location' do + expect_any_instance_of(Gitlab::PagesTransfer).to receive(:rename_project) + + described_class.new(project).execute + end + end + + context 'attachments' do + it 'keeps uploads folder location unchanged' do + expect_any_instance_of(Gitlab::UploadsTransfer).not_to receive(:rename_project) + + described_class.new(project).execute + end + + context 'when not rolled out' do + let(:project) { create(:project, :repository, storage_version: 1, skip_disk_validation: true) } + + it 'moves pages folder to hashed storage' do + expect_next_instance_of(Projects::HashedStorage::MigrateAttachmentsService) do |service| + expect(service).to receive(:execute) + end + + described_class.new(project).execute + end + end + end + + it 'updates project full path in .git/config' do + described_class.new(project).execute + + expect(rugged_config['gitlab.fullpath']).to eq(project.full_path) + end + end + end +end |