Merge branch '48745-project-exports-fail-when-uploads-have-been-migrated-to-object-storage' into 'master'

Resolve "Project exports fail when uploads have been migrated to object storage"

Closes #48745

See merge request gitlab-org/gitlab-ce!20484

app/services/upload_service.rb

@@ -1,12 +1,12 @@
 class UploadService
-  def initialize(model, file, uploader_class = FileUploader)
-    @model, @file, @uploader_class = model, file, uploader_class
+  def initialize(model, file, uploader_class = FileUploader, **uploader_context)
+    @model, @file, @uploader_class, @uploader_context = model, file, uploader_class, uploader_context
   end
 
   def execute
     return nil unless @file && @file.size <= max_attachment_size
 
-    uploader = @uploader_class.new(@model)
+    uploader = @uploader_class.new(@model, nil, @uploader_context)
     uploader.store!(@file)
 
     uploader.to_h

app/uploaders/file_uploader.rb

@@ -15,7 +15,7 @@ class FileUploader < GitlabUploader
   prepend ObjectStorage::Extension::RecordsUploads
 
   MARKDOWN_PATTERN = %r{\!?\[.*?\]\(/uploads/(?<secret>[0-9a-f]{32})/(?<file>.*?)\)}
-  DYNAMIC_PATH_PATTERN = %r{(?<secret>\h{32})/(?<identifier>.*)}
+  DYNAMIC_PATH_PATTERN = %r{.*(?<secret>\h{32})/(?<identifier>.*)}
 
   after :remove, :prune_store_dir
 
@@ -67,6 +67,10 @@ class FileUploader < GitlabUploader
     SecureRandom.hex
   end
 
+  def self.extract_dynamic_path(path)
+    DYNAMIC_PATH_PATTERN.match(path)
+  end
+
   def upload_paths(identifier)
     [
       File.join(secret, identifier),
@@ -143,7 +147,7 @@ class FileUploader < GitlabUploader
     return if apply_context!(value.uploader_context)
 
     # fallback to the regex based extraction
-    if matches = DYNAMIC_PATH_PATTERN.match(value.path)
+    if matches = self.class.extract_dynamic_path(value.path)
       @secret = matches[:secret]
       @identifier = matches[:identifier]
     end

changelogs/unreleased/48745-project-exports-fail-when-uploads-have-been-migrated-to-object-storage.yml

@@ -0,0 +1,5 @@
+---
+title: Add uploader support to Import/Export uploads
+merge_request: 20484
+author:
+type: added

lib/gitlab/import_export/avatar_saver.rb

@@ -11,7 +11,12 @@ module Gitlab
       def save
         return true unless @project.avatar.exists?
 
-        copy_files(avatar_path, avatar_export_path)
+        Gitlab::ImportExport::UploadsManager.new(
+          project: @project,
+          shared: @shared,
+          relative_export_path: 'avatar',
+          from: avatar_path
+        ).save
       rescue => e
         @shared.error(e)
         false
@@ -19,10 +24,6 @@ module Gitlab
 
       private
 
-      def avatar_export_path
-        File.join(@shared.export_path, 'avatar', @project.avatar_identifier)
-      end
-
       def avatar_path
         @project.avatar.path
       end

lib/gitlab/import_export/uploads_manager.rb

@@ -0,0 +1,101 @@
+module Gitlab
+  module ImportExport
+    class UploadsManager
+      include Gitlab::ImportExport::CommandLineUtil
+
+      UPLOADS_BATCH_SIZE = 100
+
+      def initialize(project:, shared:, relative_export_path: 'uploads', from: nil)
+        @project = project
+        @shared = shared
+        @relative_export_path = relative_export_path
+        @from = from || default_uploads_path
+      end
+
+      def save
+        copy_files(@from, uploads_export_path) if File.directory?(@from)
+
+        if File.file?(@from) && @relative_export_path == 'avatar'
+          copy_files(@from, File.join(uploads_export_path, @project.avatar.filename))
+        end
+
+        copy_from_object_storage
+
+        true
+      rescue => e
+        @shared.error(e)
+        false
+      end
+
+      def restore
+        Dir["#{uploads_export_path}/**/*"].each do |upload|
+          next if File.directory?(upload)
+
+          add_upload(upload)
+        end
+
+        true
+      rescue => e
+        @shared.error(e)
+        false
+      end
+
+      private
+
+      def add_upload(upload)
+        uploader_context = FileUploader.extract_dynamic_path(upload).named_captures.symbolize_keys
+
+        UploadService.new(@project, File.open(upload, 'r'), FileUploader, uploader_context).execute
+      end
+
+      def copy_from_object_storage
+        return unless Gitlab::ImportExport.object_storage?
+
+        each_uploader do |uploader|
+          next unless uploader.file
+          next if uploader.upload.local? # Already copied, using  the old  method
+
+          download_and_copy(uploader)
+        end
+      end
+
+      def default_uploads_path
+        FileUploader.absolute_base_dir(@project)
+      end
+
+      def uploads_export_path
+        @uploads_export_path ||= File.join(@shared.export_path, @relative_export_path)
+      end
+
+      def each_uploader
+        avatar_path = @project.avatar&.upload&.path
+
+        if @relative_export_path == 'avatar'
+          yield(@project.avatar)
+        else
+          project_uploads_except_avatar(avatar_path).find_each(batch_size: UPLOADS_BATCH_SIZE) do |upload|
+            yield(upload.build_uploader)
+          end
+        end
+      end
+
+      def project_uploads_except_avatar(avatar_path)
+        return @project.uploads unless avatar_path
+
+        @project.uploads.where("path != ?", avatar_path)
+      end
+
+      def download_and_copy(upload)
+        secret = upload.try(:secret) || ''
+        upload_path = File.join(uploads_export_path, secret, upload.filename)
+
+        mkdir_p(File.join(uploads_export_path, secret))
+
+        File.open(upload_path, 'w') do |file|
+          # Download (stream) file from the uploader's location
+          IO.copy_stream(URI.parse(upload.file.url).open, file)
+        end
+      end
+    end
+  end
+end

lib/gitlab/import_export/uploads_restorer.rb

@@ -2,13 +2,30 @@ module Gitlab
   module ImportExport
     class UploadsRestorer < UploadsSaver
       def restore
-        return true unless File.directory?(uploads_export_path)
+        if Gitlab::ImportExport.object_storage?
+          Gitlab::ImportExport::UploadsManager.new(
+            project: @project,
+            shared: @shared
+          ).restore
+        elsif File.directory?(uploads_export_path)
+          copy_files(uploads_export_path, uploads_path)
 
-        copy_files(uploads_export_path, uploads_path)
+          true
+        else
+          true # Proceed without uploads
+        end
       rescue => e
         @shared.error(e)
         false
       end
+
+      def uploads_path
+        FileUploader.absolute_base_dir(@project)
+      end
+
+      def uploads_export_path
+        @uploads_export_path ||= File.join(@shared.export_path, 'uploads')
+      end
     end
   end
 end

lib/gitlab/import_export/uploads_saver.rb

@@ -9,21 +9,14 @@ module Gitlab
       end
 
       def save
-        return true unless File.directory?(uploads_path)
-
-        copy_files(uploads_path, uploads_export_path)
+        Gitlab::ImportExport::UploadsManager.new(
+          project: @project,
+          shared: @shared
+        ).save
       rescue => e
         @shared.error(e)
         false
       end
-
-      def uploads_path
-        FileUploader.absolute_base_dir(@project)
-      end
-
-      def uploads_export_path
-        File.join(@shared.export_path, 'uploads')
-      end
     end
   end
 end

spec/factories/uploads.rb

@@ -28,6 +28,13 @@ FactoryBot.define do
       secret SecureRandom.hex
     end
 
+    trait :with_file do
+      after(:create) do |upload|
+        FileUtils.mkdir_p(File.dirname(upload.absolute_path))
+        FileUtils.touch(upload.absolute_path)
+      end
+    end
+
     trait :object_storage do
       store ObjectStorage::Store::REMOTE
     end

spec/lib/gitlab/import_export/avatar_saver_spec.rb

@@ -9,6 +9,7 @@ describe Gitlab::ImportExport::AvatarSaver do
   before do
     FileUtils.mkdir_p("#{shared.export_path}/avatar/")
     allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path)
+    stub_feature_flags(import_export_object_storage: false)
   end
 
   after do

spec/lib/gitlab/import_export/uploads_manager_spec.rb

@@ -0,0 +1,80 @@
+require 'spec_helper'
+
+describe Gitlab::ImportExport::UploadsManager do
+  let(:shared) { project.import_export_shared }
+  let(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec" }
+  let(:project) { create(:project) }
+  let(:exported_file_path) { "#{shared.export_path}/uploads/#{upload.secret}/#{File.basename(upload.path)}" }
+
+  subject(:manager) { described_class.new(project: project, shared: shared) }
+
+  before do
+    allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path)
+    FileUtils.mkdir_p(shared.export_path)
+  end
+
+  after do
+    FileUtils.rm_rf(shared.export_path)
+  end
+
+  describe '#save' do
+    context 'when the project has uploads locally stored' do
+      let(:upload) { create(:upload, :issuable_upload, :with_file, model: project) }
+
+      before do
+        project.uploads << upload
+      end
+
+      it 'does not cause errors' do
+        manager.save
+
+        expect(shared.errors).to be_empty
+      end
+
+      it 'copies the file in the correct location when there is an upload' do
+        manager.save
+
+        expect(File).to exist(exported_file_path)
+      end
+    end
+
+    context 'using object storage' do
+      let!(:upload) { create(:upload, :issuable_upload, :object_storage, model: project) }
+
+      before do
+        stub_feature_flags(import_export_object_storage: true)
+        stub_uploads_object_storage(FileUploader)
+      end
+
+      it 'saves the file' do
+        fake_uri = double
+
+        expect(fake_uri).to receive(:open).and_return(StringIO.new('File content'))
+        expect(URI).to receive(:parse).and_return(fake_uri)
+
+        manager.save
+
+        expect(File.read(exported_file_path)).to eq('File content')
+      end
+    end
+
+    describe '#restore' do
+      context 'using object storage' do
+        before do
+          stub_feature_flags(import_export_object_storage: true)
+          stub_uploads_object_storage(FileUploader)
+
+          FileUtils.mkdir_p(File.join(shared.export_path, 'uploads/72a497a02fe3ee09edae2ed06d390038'))
+          FileUtils.touch(File.join(shared.export_path, 'uploads/72a497a02fe3ee09edae2ed06d390038', "dummy.txt"))
+        end
+
+        it 'restores the file' do
+          manager.restore
+
+          expect(project.uploads.size).to eq(1)
+          expect(project.uploads.first.build_uploader.filename).to eq('dummy.txt')
+        end
+      end
+    end
+  end
+end

spec/lib/gitlab/import_export/uploads_saver_spec.rb

@@ -7,6 +7,7 @@ describe Gitlab::ImportExport::UploadsSaver do
     let(:shared) { project.import_export_shared }
 
     before do
+      stub_feature_flags(import_export_object_storage: false)
       allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path)
     end
 
@@ -30,7 +31,7 @@ describe Gitlab::ImportExport::UploadsSaver do
       it 'copies the uploads to the export path' do
         saver.save
 
-        uploads = Dir.glob(File.join(saver.uploads_export_path, '**/*')).map { |file| File.basename(file) }
+        uploads = Dir.glob(File.join(shared.export_path, 'uploads/**/*')).map { |file| File.basename(file) }
 
         expect(uploads).to include('banana_sample.gif')
       end
@@ -52,7 +53,7 @@ describe Gitlab::ImportExport::UploadsSaver do
       it 'copies the uploads to the export path' do
         saver.save
 
-        uploads = Dir.glob(File.join(saver.uploads_export_path, '**/*')).map { |file| File.basename(file) }
+        uploads = Dir.glob(File.join(shared.export_path, 'uploads/**/*')).map { |file| File.basename(file) }
 
         expect(uploads).to include('banana_sample.gif')
       end

spec/uploaders/file_uploader_spec.rb

@@ -124,6 +124,15 @@ describe FileUploader do
     end
   end
 
+  describe '.extract_dynamic_path' do
+    it 'works with hashed storage' do
+      path = 'export/4b227777d4dd1fc61c6f884f48641d02b4d121d3fd328cb08b5531fcacdabf8a/test/uploads/72a497a02fe3ee09edae2ed06d390038/dummy.txt'
+
+      expect(described_class.extract_dynamic_path(path)[:identifier]).to eq('dummy.txt')
+      expect(described_class.extract_dynamic_path(path)[:secret]).to eq('72a497a02fe3ee09edae2ed06d390038')
+    end
+  end
+
   describe '#secret' do
     it 'generates a secret if none is provided' do
       expect(described_class).to receive(:generate_secret).and_return('secret')