Changes to Attachments Migration for EE and Geo compatibility

3 files changed, 22 insertions, 8 deletions

diff --git a/app/services/projects/hashed_storage/migrate_attachments_service.rb b/app/services/projects/hashed_storage/migrate_attachments_service.rb
index 8cac6221a96..f8aaec8a9c0 100644
--- a/app/services/projects/hashed_storage/migrate_attachments_service.rb
+++ b/app/services/projects/hashed_storage/migrate_attachments_service.rb
@@ -3,7 +3,7 @@ module Projects
     AttachmentMigrationError = Class.new(StandardError)
 
     class MigrateAttachmentsService < BaseService
-      attr_reader :logger
+      attr_reader :logger, :old_path, :new_path
 
       def initialize(project, logger = nil)
         @project = project
@@ -11,16 +11,21 @@ module Projects
       end
 
       def execute
-        old_path = FileUploader.dynamic_path_segment(project)
+        @old_path = project.full_path
+        @new_path = project.disk_path
+
+        origin = FileUploader.dynamic_path_segment(project)
         project.storage_version = ::Project::HASHED_STORAGE_FEATURES[:attachments]
-        new_path = FileUploader.dynamic_path_segment(project)
+        target = FileUploader.dynamic_path_segment(project)
 
-        move_folder!(old_path, new_path)
+        result = move_folder!(origin, target)
         project.save!
 
-        if block_given?
+        if result && block_given?
           yield
         end
+
+        result
       end
 
       private
@@ -41,6 +46,8 @@ module Projects
 
         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
diff --git a/app/uploaders/file_uploader.rb b/app/uploaders/file_uploader.rb
index f4a5cf75018..71658df5b41 100644
--- a/app/uploaders/file_uploader.rb
+++ b/app/uploaders/file_uploader.rb
@@ -31,12 +31,19 @@ class FileUploader < GitlabUploader
   # Returns a String without a trailing slash
   def self.dynamic_path_segment(project)
     if project.hashed_storage?(:attachments)
-      File.join(CarrierWave.root, base_dir, project.disk_path)
+      dynamic_path_builder(project.disk_path)
     else
-      File.join(CarrierWave.root, base_dir, project.full_path)
+      dynamic_path_builder(project.full_path)
     end
   end
 
+  # Auxiliary method to build dynamic path segment when not using a project model
+  #
+  # Prefer to use the `.dynamic_path_segment` as it includes Hashed Storage specific logic
+  def self.dynamic_path_builder(path)
+    File.join(CarrierWave.root, base_dir, path)
+  end
+
   attr_accessor :model
   attr_reader :secret
 
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 de2abfc1985..50e59954f73 100644
--- a/spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb
+++ b/spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb
@@ -58,6 +58,6 @@ describe Projects::HashedStorage::MigrateAttachmentsService do
   end
 
   def base_path(storage)
-    File.join(CarrierWave.root, FileUploader.base_dir, storage.disk_path)
+    FileUploader.dynamic_path_builder(storage.disk_path)
   end
 end