Commit cebdd267 authored by Micaël Bergeron's avatar Micaël Bergeron Committed by Stan Hu

add support for file copy on object storage

parent e61f66b3
...@@ -81,6 +81,13 @@ class FileUploader < GitlabUploader ...@@ -81,6 +81,13 @@ class FileUploader < GitlabUploader
apply_context!(uploader_context) apply_context!(uploader_context)
end end
def initialize_copy(from)
super
@secret = self.class.generate_secret
@upload = nil # calling record_upload would delete the old upload if set
end
# enforce the usage of Hashed storage when storing to # enforce the usage of Hashed storage when storing to
# remote store as the FileMover doesn't support OS # remote store as the FileMover doesn't support OS
def base_dir(store = nil) def base_dir(store = nil)
...@@ -144,6 +151,25 @@ class FileUploader < GitlabUploader ...@@ -144,6 +151,25 @@ class FileUploader < GitlabUploader
@secret ||= self.class.generate_secret @secret ||= self.class.generate_secret
end end
# return a new uploader with a file copy on another project
def self.copy_to(uploader, to_project)
moved = uploader.dup.tap do |u|
u.model = to_project
end
moved.copy_file(uploader.file)
moved
end
def copy_file(file)
if file_storage?
store!(file)
else
self.file = file.copy_to(store_path)
record_upload # after_store is not triggered
end
end
private private
def apply_context!(uploader_context) def apply_context!(uploader_context)
......
...@@ -23,11 +23,8 @@ module Gitlab ...@@ -23,11 +23,8 @@ module Gitlab
file = find_file(@source_project, $~[:secret], $~[:file]) file = find_file(@source_project, $~[:secret], $~[:file])
break markdown unless file.try(:exists?) break markdown unless file.try(:exists?)
new_uploader = FileUploader.new(target_project) moved = FileUploader.copy_to(file, target_project)
with_link_in_tmp_dir(file.file) do |open_tmp_file| moved.markdown_link
new_uploader.store!(open_tmp_file)
end
new_uploader.markdown_link
end end
end end
...@@ -48,20 +45,7 @@ module Gitlab ...@@ -48,20 +45,7 @@ module Gitlab
def find_file(project, secret, file) def find_file(project, secret, file)
uploader = FileUploader.new(project, secret: secret) uploader = FileUploader.new(project, secret: secret)
uploader.retrieve_from_store!(file) uploader.retrieve_from_store!(file)
uploader.file if uploader.object_store == ObjectStorage::Store::LOCAL uploader
end
# Because the uploaders use 'move_to_store' we must have a temporary
# file that is allowed to be (re)moved.
def with_link_in_tmp_dir(file)
dir = Dir.mktmpdir('UploadsRewriter', File.dirname(file))
# The filename matters to Carrierwave so we make sure to preserve it
tmp_file = File.join(dir, File.basename(file))
File.link(file, tmp_file)
# Open the file to placate Carrierwave
File.open(tmp_file) { |open_file| yield open_file }
ensure
FileUtils.rm_rf(dir)
end end
end end
end end
......
...@@ -20,10 +20,11 @@ describe Gitlab::Gfm::UploadsRewriter do ...@@ -20,10 +20,11 @@ describe Gitlab::Gfm::UploadsRewriter do
"Text and #{image_uploader.markdown_link} and #{zip_uploader.markdown_link}" "Text and #{image_uploader.markdown_link} and #{zip_uploader.markdown_link}"
end end
shared_examples "files are accessible" do
describe '#rewrite' do describe '#rewrite' do
let!(:new_text) { rewriter.rewrite(new_project) } let!(:new_text) { rewriter.rewrite(new_project) }
let(:old_files) { [image_uploader, zip_uploader].map(&:file) } let(:old_files) { [image_uploader, zip_uploader] }
let(:new_files) do let(:new_files) do
described_class.new(new_text, new_project, user).files described_class.new(new_text, new_project, user).files
end end
...@@ -52,6 +53,23 @@ describe Gitlab::Gfm::UploadsRewriter do ...@@ -52,6 +53,23 @@ describe Gitlab::Gfm::UploadsRewriter do
expect(new_paths).not_to include zip_uploader.secret expect(new_paths).not_to include zip_uploader.secret
end end
end end
end
context "file are stored locally" do
include_examples "files are accessible"
end
context "files are store remotely" do
before do
stub_uploads_object_storage(FileUploader)
old_files.each do |file|
file.migrate!(ObjectStorage::Store::REMOTE)
end
end
include_examples "files are accessible"
end
describe '#needs_rewrite?' do describe '#needs_rewrite?' do
subject { rewriter.needs_rewrite? } subject { rewriter.needs_rewrite? }
......
...@@ -80,6 +80,50 @@ describe FileUploader do ...@@ -80,6 +80,50 @@ describe FileUploader do
end end
end end
describe 'copy_to' do
shared_examples 'returns a valid uploader' do
describe 'returned uploader' do
let(:new_project) { create(:project) }
let(:moved) { described_class.copy_to(subject, new_project) }
it 'generates a new secret' do
expect(subject).to be
expect(described_class).to receive(:generate_secret).once.and_call_original
expect(moved).to be
end
it 'create new upload' do
expect(moved.upload).not_to eq(subject.upload)
end
it 'copies the file' do
expect(subject.file).to exist
expect(moved.file).to exist
expect(subject.file).not_to eq(moved.file)
expect(subject.object_store).to eq(moved.object_store)
end
end
end
context 'files are store locally' do
include_examples 'returns a valid uploader'
before do
subject.store!(fixture_file_upload('spec/fixtures/dk.png'))
end
end
context 'files are stored remotely' do
before do
stub_uploads_object_storage
subject.store!(fixture_file_upload('spec/fixtures/dk.png'))
subject.migrate!(ObjectStorage::Store::REMOTE)
end
include_examples 'returns a valid uploader'
end
end
describe '#secret' do describe '#secret' do
it 'generates a secret if none is provided' do it 'generates a secret if none is provided' do
expect(described_class).to receive(:generate_secret).and_return('secret') expect(described_class).to receive(:generate_secret).and_return('secret')
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment