summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSean McGivern <sean@mcgivern.me.uk>2018-07-16 10:28:21 +0000
committerSean McGivern <sean@mcgivern.me.uk>2018-07-16 10:28:21 +0000
commit01ad732f8d5d5bef3ee6faf20dfa2110feb344c0 (patch)
tree961cf73e29fbc1dcfea39facd3c97447e34b7e53
parent6c237350b3967881cf9da50ec13a536e1a7e6755 (diff)
parent97ce0607d57d9a03fc6348a7ce3f9b069ced6c90 (diff)
downloadgitlab-ce-01ad732f8d5d5bef3ee6faf20dfa2110feb344c0.tar.gz
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
-rw-r--r--app/services/upload_service.rb6
-rw-r--r--app/uploaders/file_uploader.rb8
-rw-r--r--changelogs/unreleased/48745-project-exports-fail-when-uploads-have-been-migrated-to-object-storage.yml5
-rw-r--r--lib/gitlab/import_export/avatar_saver.rb11
-rw-r--r--lib/gitlab/import_export/uploads_manager.rb101
-rw-r--r--lib/gitlab/import_export/uploads_restorer.rb21
-rw-r--r--lib/gitlab/import_export/uploads_saver.rb15
-rw-r--r--spec/factories/uploads.rb7
-rw-r--r--spec/lib/gitlab/import_export/avatar_saver_spec.rb1
-rw-r--r--spec/lib/gitlab/import_export/uploads_manager_spec.rb80
-rw-r--r--spec/lib/gitlab/import_export/uploads_saver_spec.rb5
-rw-r--r--spec/uploaders/file_uploader_spec.rb9
12 files changed, 244 insertions, 25 deletions
diff --git a/app/services/upload_service.rb b/app/services/upload_service.rb
index d5a9b344905..8e20512cd61 100644
--- a/app/services/upload_service.rb
+++ b/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
diff --git a/app/uploaders/file_uploader.rb b/app/uploaders/file_uploader.rb
index 21292ddcf44..83f7b99d2a5 100644
--- a/app/uploaders/file_uploader.rb
+++ b/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
diff --git a/changelogs/unreleased/48745-project-exports-fail-when-uploads-have-been-migrated-to-object-storage.yml b/changelogs/unreleased/48745-project-exports-fail-when-uploads-have-been-migrated-to-object-storage.yml
new file mode 100644
index 00000000000..7552e0d3878
--- /dev/null
+++ b/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
diff --git a/lib/gitlab/import_export/avatar_saver.rb b/lib/gitlab/import_export/avatar_saver.rb
index 998c21e2586..31ef0490cb3 100644
--- a/lib/gitlab/import_export/avatar_saver.rb
+++ b/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
diff --git a/lib/gitlab/import_export/uploads_manager.rb b/lib/gitlab/import_export/uploads_manager.rb
new file mode 100644
index 00000000000..1110149712d
--- /dev/null
+++ b/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
diff --git a/lib/gitlab/import_export/uploads_restorer.rb b/lib/gitlab/import_export/uploads_restorer.rb
index df19354b76e..25f85936227 100644
--- a/lib/gitlab/import_export/uploads_restorer.rb
+++ b/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
diff --git a/lib/gitlab/import_export/uploads_saver.rb b/lib/gitlab/import_export/uploads_saver.rb
index 2f08dda55fd..b3f17af5661 100644
--- a/lib/gitlab/import_export/uploads_saver.rb
+++ b/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
diff --git a/spec/factories/uploads.rb b/spec/factories/uploads.rb
index b45f6f30e40..a81b2169b89 100644
--- a/spec/factories/uploads.rb
+++ b/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
diff --git a/spec/lib/gitlab/import_export/avatar_saver_spec.rb b/spec/lib/gitlab/import_export/avatar_saver_spec.rb
index 2223f163177..90e6d653d34 100644
--- a/spec/lib/gitlab/import_export/avatar_saver_spec.rb
+++ b/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
diff --git a/spec/lib/gitlab/import_export/uploads_manager_spec.rb b/spec/lib/gitlab/import_export/uploads_manager_spec.rb
new file mode 100644
index 00000000000..9c3870a0af8
--- /dev/null
+++ b/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
diff --git a/spec/lib/gitlab/import_export/uploads_saver_spec.rb b/spec/lib/gitlab/import_export/uploads_saver_spec.rb
index 095687fa89d..c716edd9397 100644
--- a/spec/lib/gitlab/import_export/uploads_saver_spec.rb
+++ b/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
diff --git a/spec/uploaders/file_uploader_spec.rb b/spec/uploaders/file_uploader_spec.rb
index 7ba28b4fc1f..3efe512a59c 100644
--- a/spec/uploaders/file_uploader_spec.rb
+++ b/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')