From cf52488ccba8913027334c16892ee1fce1241169 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Tue, 15 Jan 2019 04:30:55 +0100 Subject: Move MigrationService to HashedStorage module This is part of the refactor to include a RollbackService into HashedStorage module --- app/services/projects/after_rename_service.rb | 2 +- .../projects/hashed_storage/migration_service.rb | 29 ++++++++++++ .../projects/hashed_storage_migration_service.rb | 27 ----------- .../project_migrate_hashed_storage_worker.rb | 2 +- .../services/projects/after_rename_service_spec.rb | 4 +- .../hashed_storage/migration_service_spec.rb | 52 ++++++++++++++++++++++ .../hashed_storage_migration_service_spec.rb | 52 ---------------------- .../project_migrate_hashed_storage_worker_spec.rb | 8 ++-- 8 files changed, 89 insertions(+), 87 deletions(-) create mode 100644 app/services/projects/hashed_storage/migration_service.rb delete mode 100644 app/services/projects/hashed_storage_migration_service.rb create mode 100644 spec/services/projects/hashed_storage/migration_service_spec.rb delete mode 100644 spec/services/projects/hashed_storage_migration_service_spec.rb diff --git a/app/services/projects/after_rename_service.rb b/app/services/projects/after_rename_service.rb index c3cd9d1ea4a..fafdecb3222 100644 --- a/app/services/projects/after_rename_service.rb +++ b/app/services/projects/after_rename_service.rb @@ -62,7 +62,7 @@ module Projects def rename_or_migrate_repository! success = if migrate_to_hashed_storage? - ::Projects::HashedStorageMigrationService + ::Projects::HashedStorage::MigrationService .new(project, full_path_before) .execute else diff --git a/app/services/projects/hashed_storage/migration_service.rb b/app/services/projects/hashed_storage/migration_service.rb new file mode 100644 index 00000000000..140c35bdd52 --- /dev/null +++ b/app/services/projects/hashed_storage/migration_service.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module Projects + module HashedStorage + class MigrationService < BaseService + attr_reader :logger, :old_disk_path + + def initialize(project, old_disk_path, logger: nil) + @project = project + @old_disk_path = old_disk_path + @logger = logger || Rails.logger + end + + def execute + # Migrate repository from Legacy to Hashed Storage + unless project.hashed_storage?(:repository) + return unless HashedStorage::MigrateRepositoryService.new(project, old_disk_path, logger: logger).execute + end + + # Migrate attachments from Legacy to Hashed Storage + unless project.hashed_storage?(:attachments) + HashedStorage::MigrateAttachmentsService.new(project, old_disk_path, logger: logger).execute + end + + true + end + end + end +end diff --git a/app/services/projects/hashed_storage_migration_service.rb b/app/services/projects/hashed_storage_migration_service.rb deleted file mode 100644 index a0e734005f8..00000000000 --- a/app/services/projects/hashed_storage_migration_service.rb +++ /dev/null @@ -1,27 +0,0 @@ -# frozen_string_literal: true - -module Projects - class HashedStorageMigrationService < BaseService - attr_reader :logger, :old_disk_path - - def initialize(project, old_disk_path, logger: nil) - @project = project - @old_disk_path = old_disk_path - @logger = logger || Rails.logger - end - - def execute - # Migrate repository from Legacy to Hashed Storage - unless project.hashed_storage?(:repository) - return unless HashedStorage::MigrateRepositoryService.new(project, old_disk_path, logger: logger).execute - end - - # Migrate attachments from Legacy to Hashed Storage - unless project.hashed_storage?(:attachments) - HashedStorage::MigrateAttachmentsService.new(project, old_disk_path, logger: logger).execute - end - - true - end - end -end diff --git a/app/workers/project_migrate_hashed_storage_worker.rb b/app/workers/project_migrate_hashed_storage_worker.rb index 4c6339f7701..c3cdf102382 100644 --- a/app/workers/project_migrate_hashed_storage_worker.rb +++ b/app/workers/project_migrate_hashed_storage_worker.rb @@ -12,7 +12,7 @@ class ProjectMigrateHashedStorageWorker uuid = lease_for(project_id).try_obtain if uuid - ::Projects::HashedStorageMigrationService.new(project, old_disk_path || project.full_path, logger: logger).execute + ::Projects::HashedStorage::MigrationService.new(project, old_disk_path || project.full_path, logger: logger).execute else false end diff --git a/spec/services/projects/after_rename_service_spec.rb b/spec/services/projects/after_rename_service_spec.rb index bc5366a3339..b8055a285f2 100644 --- a/spec/services/projects/after_rename_service_spec.rb +++ b/spec/services/projects/after_rename_service_spec.rb @@ -101,10 +101,10 @@ describe Projects::AfterRenameService do end context 'with hashed storage upgrade when renaming enabled' do - it 'calls HashedStorageMigrationService with correct options' do + it 'calls HashedStorage::MigrationService with correct options' do stub_application_setting(hashed_storage_enabled: true) - expect_next_instance_of(::Projects::HashedStorageMigrationService) do |service| + expect_next_instance_of(::Projects::HashedStorage::MigrationService) do |service| expect(service).to receive(:execute).and_return(true) end diff --git a/spec/services/projects/hashed_storage/migration_service_spec.rb b/spec/services/projects/hashed_storage/migration_service_spec.rb new file mode 100644 index 00000000000..b4647586363 --- /dev/null +++ b/spec/services/projects/hashed_storage/migration_service_spec.rb @@ -0,0 +1,52 @@ +require 'spec_helper' + +describe Projects::HashedStorage::MigrationService do + let(:project) { create(:project, :empty_repo, :wiki_repo, :legacy_storage) } + let(:logger) { double } + + subject(:service) { described_class.new(project, project.full_path, logger: logger) } + + describe '#execute' do + context 'repository migration' do + let(:repository_service) { Projects::HashedStorage::MigrateRepositoryService.new(project, project.full_path, logger: logger) } + + it 'delegates migration to Projects::HashedStorage::MigrateRepositoryService' do + expect(Projects::HashedStorage::MigrateRepositoryService) + .to receive(:new) + .with(project, project.full_path, logger: logger) + .and_return(repository_service) + expect(repository_service).to receive(:execute) + + service.execute + end + + it 'does not delegate migration if repository is already migrated' do + project.storage_version = ::Project::LATEST_STORAGE_VERSION + expect(Projects::HashedStorage::MigrateRepositoryService).not_to receive(:new) + + service.execute + end + end + + context 'attachments migration' do + let(:attachments_service) { Projects::HashedStorage::MigrateAttachmentsService.new(project, project.full_path, logger: logger) } + + it 'delegates migration to Projects::HashedStorage::MigrateRepositoryService' do + expect(Projects::HashedStorage::MigrateAttachmentsService) + .to receive(:new) + .with(project, project.full_path, logger: logger) + .and_return(attachments_service) + expect(attachments_service).to receive(:execute) + + service.execute + end + + it 'does not delegate migration if attachments are already migrated' do + project.storage_version = ::Project::LATEST_STORAGE_VERSION + expect(Projects::HashedStorage::MigrateAttachmentsService).not_to receive(:new) + + service.execute + end + end + end +end diff --git a/spec/services/projects/hashed_storage_migration_service_spec.rb b/spec/services/projects/hashed_storage_migration_service_spec.rb deleted file mode 100644 index 5368c3828dd..00000000000 --- a/spec/services/projects/hashed_storage_migration_service_spec.rb +++ /dev/null @@ -1,52 +0,0 @@ -require 'spec_helper' - -describe Projects::HashedStorageMigrationService do - let(:project) { create(:project, :empty_repo, :wiki_repo, :legacy_storage) } - let(:logger) { double } - - subject(:service) { described_class.new(project, project.full_path, logger: logger) } - - describe '#execute' do - context 'repository migration' do - let(:repository_service) { Projects::HashedStorage::MigrateRepositoryService.new(project, project.full_path, logger: logger) } - - it 'delegates migration to Projects::HashedStorage::MigrateRepositoryService' do - expect(Projects::HashedStorage::MigrateRepositoryService) - .to receive(:new) - .with(project, project.full_path, logger: logger) - .and_return(repository_service) - expect(repository_service).to receive(:execute) - - service.execute - end - - it 'does not delegate migration if repository is already migrated' do - project.storage_version = ::Project::LATEST_STORAGE_VERSION - expect(Projects::HashedStorage::MigrateRepositoryService).not_to receive(:new) - - service.execute - end - end - - context 'attachments migration' do - let(:attachments_service) { Projects::HashedStorage::MigrateAttachmentsService.new(project, project.full_path, logger: logger) } - - it 'delegates migration to Projects::HashedStorage::MigrateRepositoryService' do - expect(Projects::HashedStorage::MigrateAttachmentsService) - .to receive(:new) - .with(project, project.full_path, logger: logger) - .and_return(attachments_service) - expect(attachments_service).to receive(:execute) - - service.execute - end - - it 'does not delegate migration if attachments are already migrated' do - project.storage_version = ::Project::LATEST_STORAGE_VERSION - expect(Projects::HashedStorage::MigrateAttachmentsService).not_to receive(:new) - - service.execute - end - end - end -end diff --git a/spec/workers/project_migrate_hashed_storage_worker_spec.rb b/spec/workers/project_migrate_hashed_storage_worker_spec.rb index 3703320418b..18470923316 100644 --- a/spec/workers/project_migrate_hashed_storage_worker_spec.rb +++ b/spec/workers/project_migrate_hashed_storage_worker_spec.rb @@ -9,7 +9,7 @@ describe ProjectMigrateHashedStorageWorker, :clean_gitlab_redis_shared_state do let(:lease_timeout) { ProjectMigrateHashedStorageWorker::LEASE_TIMEOUT } it 'skips when project no longer exists' do - expect(::Projects::HashedStorageMigrationService).not_to receive(:new) + expect(::Projects::HashedStorage::MigrationService).not_to receive(:new) subject.perform(-1) end @@ -17,7 +17,7 @@ describe ProjectMigrateHashedStorageWorker, :clean_gitlab_redis_shared_state do it 'skips when project is pending delete' do pending_delete_project = create(:project, :empty_repo, pending_delete: true) - expect(::Projects::HashedStorageMigrationService).not_to receive(:new) + expect(::Projects::HashedStorage::MigrationService).not_to receive(:new) subject.perform(pending_delete_project.id) end @@ -27,7 +27,7 @@ describe ProjectMigrateHashedStorageWorker, :clean_gitlab_redis_shared_state do migration_service = spy - allow(::Projects::HashedStorageMigrationService) + allow(::Projects::HashedStorage::MigrationService) .to receive(:new).with(project, project.full_path, logger: subject.logger) .and_return(migration_service) @@ -39,7 +39,7 @@ describe ProjectMigrateHashedStorageWorker, :clean_gitlab_redis_shared_state do it 'skips when dont have lease when dont have exclusive lease' do stub_exclusive_lease_taken(lease_key, timeout: lease_timeout) - expect(::Projects::HashedStorageMigrationService).not_to receive(:new) + expect(::Projects::HashedStorage::MigrationService).not_to receive(:new) subject.perform(project.id) end -- cgit v1.2.1