From b91a005162515cfe398d90772ed66ecd92e17531 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Fri, 1 Mar 2019 07:58:05 +0100 Subject: Extract duplicated code into BaseAttachmentService Exceptions were also simplified from 2 to 1. --- .../hashed_storage/base_attachment_service.rb | 51 ++++++++++++++++++++++ .../hashed_storage/migrate_attachments_service.rb | 50 ++------------------- .../hashed_storage/rollback_attachments_service.rb | 47 +------------------- .../migrate_attachments_service_spec.rb | 4 +- .../rollback_attachments_service_spec.rb | 4 +- 5 files changed, 59 insertions(+), 97 deletions(-) create mode 100644 app/services/projects/hashed_storage/base_attachment_service.rb diff --git a/app/services/projects/hashed_storage/base_attachment_service.rb b/app/services/projects/hashed_storage/base_attachment_service.rb new file mode 100644 index 00000000000..828ab616bab --- /dev/null +++ b/app/services/projects/hashed_storage/base_attachment_service.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +module Projects + module HashedStorage + AttachmentMigrationError = Class.new(StandardError) + + AttachmentCannotMoveError = Class.new(StandardError) + + class BaseAttachmentService < BaseService + # Returns the disk_path value before the execution + attr_reader :old_disk_path + + # Returns the disk_path value after the execution + attr_reader :new_disk_path + + # Returns the logger currently in use + attr_reader :logger + + # Return whether this operation was skipped or not + # + # @return [Boolean] true if skipped of false otherwise + def skipped? + @skipped + end + + protected + + def move_folder!(old_path, new_path) + unless File.directory?(old_path) + logger.info("Skipped attachments move from '#{old_path}' to '#{new_path}', source path doesn't exist or is not a directory (PROJECT_ID=#{project.id})") + @skipped = true + + return true + end + + if File.exist?(new_path) + logger.error("Cannot move attachments from '#{old_path}' to '#{new_path}', target path already exist (PROJECT_ID=#{project.id})") + raise AttachmentCannotMoveError, "Target path '#{new_path}' already exists" + end + + # Create base path folder on the new storage layout + FileUtils.mkdir_p(File.dirname(new_path)) + + FileUtils.mv(old_path, new_path) + logger.info("Project attachments moved from '#{old_path}' to '#{new_path}' (PROJECT_ID=#{project.id})") + + true + end + end + end +end diff --git a/app/services/projects/hashed_storage/migrate_attachments_service.rb b/app/services/projects/hashed_storage/migrate_attachments_service.rb index 0df1e4ee130..2b8b7e459e0 100644 --- a/app/services/projects/hashed_storage/migrate_attachments_service.rb +++ b/app/services/projects/hashed_storage/migrate_attachments_service.rb @@ -2,20 +2,7 @@ module Projects module HashedStorage - AttachmentMigrationError = Class.new(StandardError) - - class MigrateAttachmentsService < BaseService - # Returns the disk_path value before the execution - # This is used in EE for Geo - attr_reader :old_disk_path - - # Returns the diks_path value after the execution - # This is used in EE for Geo - attr_reader :new_disk_path - - # Returns the logger currently in use - attr_reader :logger - + class MigrateAttachmentsService < BaseAttachmentService def initialize(project, old_disk_path, logger: nil) @project = project @logger = logger || Rails.logger @@ -25,7 +12,8 @@ module Projects def execute origin = FileUploader.absolute_base_dir(project) - # It's possible that old_disk_path does not match project.disk_path. For example, that happens when we rename a project + # It's possible that old_disk_path does not match project.disk_path. + # For example, that happens when we rename a project origin.sub!(/#{Regexp.escape(project.full_path)}\z/, old_disk_path) project.storage_version = ::Project::HASHED_STORAGE_FEATURES[:attachments] @@ -42,38 +30,6 @@ module Projects result end - - # Return whether this operation was skipped or not - # This is used in EE for Geo to decide if an event will be triggered or not - # - # @return [Boolean] true if skipped of false otherwise - def skipped? - @skipped - end - - private - - def move_folder!(old_path, new_path) - unless File.directory?(old_path) - logger.info("Skipped attachments migration from '#{old_path}' to '#{new_path}', source path doesn't exist or is not a directory (PROJECT_ID=#{project.id})") - @skipped = true - - return true - end - - if File.exist?(new_path) - logger.error("Cannot migrate attachments from '#{old_path}' to '#{new_path}', target path already exist (PROJECT_ID=#{project.id})") - raise AttachmentMigrationError, "Target path '#{new_path}' already exists" - end - - # Create hashed storage base path folder - FileUtils.mkdir_p(File.dirname(new_path)) - - FileUtils.mv(old_path, new_path) - logger.info("Migrated project attachments from '#{old_path}' to '#{new_path}' (PROJECT_ID=#{project.id})") - - true - end end end end diff --git a/app/services/projects/hashed_storage/rollback_attachments_service.rb b/app/services/projects/hashed_storage/rollback_attachments_service.rb index 8d0de27d30b..c35b8787dee 100644 --- a/app/services/projects/hashed_storage/rollback_attachments_service.rb +++ b/app/services/projects/hashed_storage/rollback_attachments_service.rb @@ -2,20 +2,7 @@ module Projects module HashedStorage - AttachmentRollbackError = Class.new(StandardError) - - class RollbackAttachmentsService < BaseService - # Returns the disk_path value before the execution - # This is used in EE for Geo - attr_reader :old_disk_path - - # Returns the diks_path value after the execution - # This is used in EE for Geo - attr_reader :new_disk_path - - # Returns the logger currently in use - attr_reader :logger - + class RollbackAttachmentsService < BaseAttachmentService def initialize(project, logger: nil) @project = project @logger = logger || Rails.logger @@ -38,38 +25,6 @@ module Projects result end - - # Return whether this operation was skipped or not - # This is used in EE for Geo to decide if an event will be triggered or not - # - # @return [Boolean] true if skipped of false otherwise - def skipped? - @skipped - end - - private - - def move_folder!(old_path, new_path) - unless File.directory?(old_path) - logger.info("Skipped attachments rollback from '#{old_path}' to '#{new_path}', source path doesn't exist or is not a directory (PROJECT_ID=#{project.id})") - @skipped = true - - return true - end - - if File.exist?(new_path) - logger.error("Cannot rollback attachments from '#{old_path}' to '#{new_path}', target path already exist (PROJECT_ID=#{project.id})") - raise AttachmentRollbackError, "Target path '#{new_path}' already exists" - end - - # Create hashed storage base path folder - FileUtils.mkdir_p(File.dirname(new_path)) - - FileUtils.mv(old_path, new_path) - logger.info("Rolled project attachments back from '#{old_path}' to '#{new_path}' (PROJECT_ID=#{project.id})") - - true - end end end end diff --git a/spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb b/spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb index d51988c6f42..639dd930618 100644 --- a/spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb +++ b/spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb @@ -70,10 +70,10 @@ describe Projects::HashedStorage::MigrateAttachmentsService do FileUtils.mkdir_p(base_path(hashed_storage)) end - it 'raises AttachmentMigrationError' do + it 'raises AttachmentCannotMoveError' do expect(FileUtils).not_to receive(:mv).with(base_path(legacy_storage), base_path(hashed_storage)) - expect { service.execute }.to raise_error(Projects::HashedStorage::AttachmentMigrationError) + expect { service.execute }.to raise_error(Projects::HashedStorage::AttachmentCannotMoveError) end end end diff --git a/spec/services/projects/hashed_storage/rollback_attachments_service_spec.rb b/spec/services/projects/hashed_storage/rollback_attachments_service_spec.rb index 9e18428f412..6f4154d6011 100644 --- a/spec/services/projects/hashed_storage/rollback_attachments_service_spec.rb +++ b/spec/services/projects/hashed_storage/rollback_attachments_service_spec.rb @@ -72,10 +72,10 @@ describe Projects::HashedStorage::RollbackAttachmentsService do FileUtils.mkdir_p(base_path(legacy_storage)) end - it 'raises AttachmentRollbackError' do + it 'raises AttachmentCannotMoveError' do expect(FileUtils).not_to receive(:mv).with(base_path(legacy_storage), base_path(hashed_storage)) - expect { service.execute }.to raise_error(Projects::HashedStorage::AttachmentRollbackError) + expect { service.execute }.to raise_error(Projects::HashedStorage::AttachmentCannotMoveError) end end end -- cgit v1.2.1