Extract duplicated code into BaseAttachmentService

Exceptions were also simplified from 2 to 1.

app/services/projects/hashed_storage/base_attachment_service.rb

+# 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

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

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

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

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