Commit c32f54c2 authored by Oswaldo Ferreira's avatar Oswaldo Ferreira

Support object storage at FileMover class

parent c56ca67e
# frozen_string_literal: true
class FileMover
include Gitlab::Utils::StrongMemoize
attr_reader :secret, :file_name, :from_model, :to_model, :update_field
def initialize(file_path, update_field = :description, from_model:, to_model:)
......@@ -12,8 +14,12 @@ class FileMover
end
def execute
temp_file_uploader.retrieve_from_store!(file_name)
return unless valid?
uploader.retrieve_from_store!(file_name)
move
if update_markdown
......@@ -25,14 +31,23 @@ class FileMover
private
def valid?
Pathname.new(temp_file_path).realpath.to_path.start_with?(
(Pathname(temp_file_uploader.root) + temp_file_uploader.base_dir).to_path
)
if temp_file_uploader.file_storage?
Pathname.new(temp_file_path).realpath.to_path.start_with?(
(Pathname(temp_file_uploader.root) + temp_file_uploader.base_dir).to_path
)
else
temp_file_uploader.exists?
end
end
def move
FileUtils.mkdir_p(File.dirname(file_path))
FileUtils.move(temp_file_path, file_path)
if temp_file_uploader.file_storage?
FileUtils.mkdir_p(File.dirname(file_path))
FileUtils.move(temp_file_path, file_path)
else
uploader.copy_file(temp_file_uploader.file)
temp_file_uploader.upload.destroy!
end
end
def update_markdown
......@@ -46,28 +61,36 @@ class FileMover
def update_upload_model
return unless upload = temp_file_uploader.upload
return if upload.destroyed?
upload.update!(model_id: to_model.id, model_type: to_model.type)
upload.update!(model: to_model)
end
def temp_file_path
return @temp_file_path if @temp_file_path
temp_file_uploader.retrieve_from_store!(file_name)
@temp_file_path = temp_file_uploader.file.path
strong_memoize(:temp_file_path) do
temp_file_uploader.file.path
end
end
def file_path
return @file_path if @file_path
uploader.retrieve_from_store!(file_name)
@file_path = uploader.file.path
strong_memoize(:file_path) do
uploader.file.path
end
end
def uploader
@uploader ||= PersonalFileUploader.new(to_model, secret: secret)
@uploader ||=
begin
uploader = PersonalFileUploader.new(to_model, secret: secret)
# Enforcing a REMOTE object storage given FileUploader#retrieve_from_store! won't do it
# (there's no upload at the target yet).
if uploader.class.object_store_enabled?
uploader.object_store = ::ObjectStorage::Store::REMOTE
end
uploader
end
end
def temp_file_uploader
......@@ -77,6 +100,8 @@ class FileMover
def revert
Rails.logger.warn("Markdown not updated, file move reverted for #{to_model}")
FileUtils.move(file_path, temp_file_path)
if temp_file_uploader.file_storage?
FileUtils.move(file_path, temp_file_path)
end
end
end
......@@ -23,63 +23,110 @@ describe FileMover do
subject { described_class.new(temp_file_path, from_model: user, to_model: snippet).execute }
describe '#execute' do
let(:tmp_upload) { tmp_uploader.upload }
before do
tmp_uploader.store!(file)
end
expect(FileUtils).to receive(:mkdir_p).with(a_string_including(File.dirname(file_path)))
expect(FileUtils).to receive(:move).with(a_string_including(temp_file_path), a_string_including(file_path))
allow_any_instance_of(CarrierWave::SanitizedFile).to receive(:exists?).and_return(true)
allow_any_instance_of(CarrierWave::SanitizedFile).to receive(:size).and_return(10)
context 'local storage' do
before do
allow(FileUtils).to receive(:mkdir_p).with(a_string_including(File.dirname(file_path)))
allow(FileUtils).to receive(:move).with(a_string_including(temp_file_path), a_string_including(file_path))
allow_any_instance_of(CarrierWave::SanitizedFile).to receive(:exists?).and_return(true)
allow_any_instance_of(CarrierWave::SanitizedFile).to receive(:size).and_return(10)
stub_file_mover(temp_file_path)
end
stub_file_mover(temp_file_path)
end
let(:tmp_upload) { tmp_uploader.upload }
context 'when move and field update successful' do
it 'updates the description correctly' do
subject
context 'when move and field update successful' do
it 'updates the description correctly' do
subject
expect(snippet.reload.description)
.to eq("test ![banana_sample](/uploads/-/system/personal_snippet/#{snippet.id}/secret55/banana_sample.gif) "\
"same ![banana_sample](/uploads/-/system/personal_snippet/#{snippet.id}/secret55/banana_sample.gif) ")
end
expect(snippet.reload.description)
.to eq(
"test ![banana_sample](/uploads/-/system/personal_snippet/#{snippet.id}/secret55/banana_sample.gif) "\
"same ![banana_sample](/uploads/-/system/personal_snippet/#{snippet.id}/secret55/banana_sample.gif) "
)
end
it 'updates existing upload record' do
expect { subject }
.to change { tmp_upload.reload.attributes.values_at('model_id', 'model_type') }
.from([user.id, 'User']).to([snippet.id, 'Snippet'])
end
it 'updates existing upload record' do
expect { subject }
.to change { tmp_upload.reload.attributes.values_at('model_id', 'model_type') }
.from([user.id, 'User']).to([snippet.id, 'PersonalSnippet'])
it 'schedules a background migration' do
expect_any_instance_of(PersonalFileUploader).to receive(:schedule_background_upload).once
subject
end
end
it 'schedules a background migration' do
expect_any_instance_of(PersonalFileUploader).to receive(:schedule_background_upload).once
context 'when update_markdown fails' do
before do
expect(FileUtils).to receive(:move).with(a_string_including(file_path), a_string_including(temp_file_path))
end
subject
subject { described_class.new(file_path, :non_existing_field, from_model: user, to_model: snippet).execute }
it 'does not update the description' do
subject
expect(snippet.reload.description)
.to eq("test ![banana_sample](/uploads/-/system/user/#{user.id}/secret55/banana_sample.gif) "\
"same ![banana_sample](/uploads/-/system/user/#{user.id}/secret55/banana_sample.gif) ")
end
it 'does not change the upload record' do
expect { subject }
.not_to change { tmp_upload.reload.attributes.values_at('model_id', 'model_type') }
end
end
end
context 'when update_markdown fails' do
context 'when tmp uploader is not local storage' do
before do
expect(FileUtils).to receive(:move).with(a_string_including(file_path), a_string_including(temp_file_path))
allow(PersonalFileUploader).to receive(:object_store_enabled?) { true }
tmp_uploader.object_store = ObjectStorage::Store::REMOTE
allow_any_instance_of(PersonalFileUploader).to receive(:file_storage?) { false }
end
subject { described_class.new(file_path, :non_existing_field, from_model: user, to_model: snippet).execute }
after do
FileUtils.rm_f(File.join('personal_snippet', snippet.id.to_s, secret, filename))
end
it 'does not update the description' do
subject
context 'when move and field update successful' do
it 'updates the description correctly' do
subject
expect(snippet.reload.description)
.to eq(
"test ![banana_sample](/uploads/-/system/user/#{user.id}/secret55/banana_sample.gif) "\
"same ![banana_sample](/uploads/-/system/user/#{user.id}/secret55/banana_sample.gif) "
)
expect(snippet.reload.description)
.to eq("test ![banana_sample](/uploads/-/system/personal_snippet/#{snippet.id}/secret55/banana_sample.gif) "\
"same ![banana_sample](/uploads/-/system/personal_snippet/#{snippet.id}/secret55/banana_sample.gif) ")
end
it 'creates new target upload record an delete the old upload' do
expect { subject }
.to change { Upload.last.attributes.values_at('model_id', 'model_type') }
.from([user.id, 'User']).to([snippet.id, 'Snippet'])
expect(Upload.count).to eq(1)
end
end
it 'does not change the upload record' do
expect { subject }
.not_to change { tmp_upload.reload.attributes.values_at('model_id', 'model_type') }
context 'when update_markdown fails' do
subject { described_class.new(file_path, :non_existing_field, from_model: user, to_model: snippet).execute }
it 'does not update the description' do
subject
expect(snippet.reload.description)
.to eq("test ![banana_sample](/uploads/-/system/user/#{user.id}/secret55/banana_sample.gif) "\
"same ![banana_sample](/uploads/-/system/user/#{user.id}/secret55/banana_sample.gif) ")
end
it 'does not change the upload record' do
expect { subject }
.to change { Upload.last.attributes.values_at('model_id', 'model_type') }.from([user.id, 'User'])
end
end
end
end
......
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