diff options
Diffstat (limited to 'spec/uploaders/object_storage_spec.rb')
| -rw-r--r-- | spec/uploaders/object_storage_spec.rb | 191 |
1 files changed, 155 insertions, 36 deletions
diff --git a/spec/uploaders/object_storage_spec.rb b/spec/uploaders/object_storage_spec.rb index 2dd0925a8e6..7e673681c31 100644 --- a/spec/uploaders/object_storage_spec.rb +++ b/spec/uploaders/object_storage_spec.rb @@ -191,6 +191,18 @@ describe ObjectStorage do it "calls a cache path" do expect { |b| uploader.use_file(&b) }.to yield_with_args(%r[tmp/cache]) end + + it "cleans up the cached file" do + cached_path = '' + + uploader.use_file do |path| + cached_path = path + + expect(File.exist?(cached_path)).to be_truthy + end + + expect(File.exist?(cached_path)).to be_falsey + end end end @@ -321,7 +333,7 @@ describe ObjectStorage do when_file_is_in_use do expect(uploader).not_to receive(:unsafe_migrate!) - expect { uploader.migrate!(described_class::Store::REMOTE) }.to raise_error('exclusive lease already taken') + expect { uploader.migrate!(described_class::Store::REMOTE) }.to raise_error(ObjectStorage::ExclusiveLeaseTaken) end end @@ -329,7 +341,19 @@ describe ObjectStorage do when_file_is_in_use do expect(uploader).not_to receive(:unsafe_use_file) - expect { uploader.use_file }.to raise_error('exclusive lease already taken') + expect { uploader.use_file }.to raise_error(ObjectStorage::ExclusiveLeaseTaken) + end + end + + it 'can still migrate other files of the same model' do + uploader2 = uploader_class.new(object, :file) + uploader2.upload = create(:upload) + uploader.upload = create(:upload) + + when_file_is_in_use do + expect(uploader2).to receive(:unsafe_migrate!) + + uploader2.migrate!(described_class::Store::REMOTE) end end end @@ -355,14 +379,10 @@ describe ObjectStorage do end describe '.workhorse_authorize' do - subject { uploader_class.workhorse_authorize } + let(:has_length) { true } + let(:maximum_size) { nil } - before do - # ensure that we use regular Fog libraries - # other tests might call `Fog.mock!` and - # it will make tests to fail - Fog.unmock! - end + subject { uploader_class.workhorse_authorize(has_length: has_length, maximum_size: maximum_size) } shared_examples 'uses local storage' do it "returns temporary path" do @@ -371,10 +391,6 @@ describe ObjectStorage do expect(subject[:TempPath]).to start_with(uploader_class.root) expect(subject[:TempPath]).to include(described_class::TMP_UPLOAD_PATH) end - - it "does not return remote store" do - is_expected.not_to have_key('RemoteObject') - end end shared_examples 'uses remote storage' do @@ -383,7 +399,7 @@ describe ObjectStorage do expect(subject[:RemoteObject]).to have_key(:ID) expect(subject[:RemoteObject]).to include(Timeout: a_kind_of(Integer)) - expect(subject[:RemoteObject][:Timeout]).to be(ObjectStorage::DIRECT_UPLOAD_TIMEOUT) + expect(subject[:RemoteObject][:Timeout]).to be(ObjectStorage::DirectUpload::TIMEOUT) expect(subject[:RemoteObject]).to have_key(:GetURL) expect(subject[:RemoteObject]).to have_key(:DeleteURL) expect(subject[:RemoteObject]).to have_key(:StoreURL) @@ -391,9 +407,31 @@ describe ObjectStorage do expect(subject[:RemoteObject][:DeleteURL]).to include(described_class::TMP_UPLOAD_PATH) expect(subject[:RemoteObject][:StoreURL]).to include(described_class::TMP_UPLOAD_PATH) end + end + + shared_examples 'uses remote storage with multipart uploads' do + it_behaves_like 'uses remote storage' do + it "returns multipart upload" do + is_expected.to have_key(:RemoteObject) + + expect(subject[:RemoteObject]).to have_key(:MultipartUpload) + expect(subject[:RemoteObject][:MultipartUpload]).to have_key(:PartSize) + expect(subject[:RemoteObject][:MultipartUpload]).to have_key(:PartURLs) + expect(subject[:RemoteObject][:MultipartUpload]).to have_key(:CompleteURL) + expect(subject[:RemoteObject][:MultipartUpload]).to have_key(:AbortURL) + expect(subject[:RemoteObject][:MultipartUpload][:PartURLs]).to all(include(described_class::TMP_UPLOAD_PATH)) + expect(subject[:RemoteObject][:MultipartUpload][:CompleteURL]).to include(described_class::TMP_UPLOAD_PATH) + expect(subject[:RemoteObject][:MultipartUpload][:AbortURL]).to include(described_class::TMP_UPLOAD_PATH) + end + end + end - it "does not return local store" do - is_expected.not_to have_key('TempPath') + shared_examples 'uses remote storage without multipart uploads' do + it_behaves_like 'uses remote storage' do + it "does not return multipart upload" do + is_expected.to have_key(:RemoteObject) + expect(subject[:RemoteObject]).not_to have_key(:MultipartUpload) + end end end @@ -416,6 +454,8 @@ describe ObjectStorage do end context 'uses AWS' do + let(:storage_url) { "https://uploads.s3-eu-central-1.amazonaws.com/" } + before do expect(uploader_class).to receive(:object_store_credentials) do { provider: "AWS", @@ -425,18 +465,40 @@ describe ObjectStorage do end end - it_behaves_like 'uses remote storage' do - let(:storage_url) { "https://uploads.s3-eu-central-1.amazonaws.com/" } + context 'for known length' do + it_behaves_like 'uses remote storage without multipart uploads' do + it 'returns links for S3' do + expect(subject[:RemoteObject][:GetURL]).to start_with(storage_url) + expect(subject[:RemoteObject][:DeleteURL]).to start_with(storage_url) + expect(subject[:RemoteObject][:StoreURL]).to start_with(storage_url) + end + end + end + + context 'for unknown length' do + let(:has_length) { false } + let(:maximum_size) { 1.gigabyte } - it 'returns links for S3' do - expect(subject[:RemoteObject][:GetURL]).to start_with(storage_url) - expect(subject[:RemoteObject][:DeleteURL]).to start_with(storage_url) - expect(subject[:RemoteObject][:StoreURL]).to start_with(storage_url) + before do + stub_object_storage_multipart_init(storage_url) + end + + it_behaves_like 'uses remote storage with multipart uploads' do + it 'returns links for S3' do + expect(subject[:RemoteObject][:GetURL]).to start_with(storage_url) + expect(subject[:RemoteObject][:DeleteURL]).to start_with(storage_url) + expect(subject[:RemoteObject][:StoreURL]).to start_with(storage_url) + expect(subject[:RemoteObject][:MultipartUpload][:PartURLs]).to all(start_with(storage_url)) + expect(subject[:RemoteObject][:MultipartUpload][:CompleteURL]).to start_with(storage_url) + expect(subject[:RemoteObject][:MultipartUpload][:AbortURL]).to start_with(storage_url) + end end end end context 'uses Google' do + let(:storage_url) { "https://storage.googleapis.com/uploads/" } + before do expect(uploader_class).to receive(:object_store_credentials) do { provider: "Google", @@ -445,36 +507,71 @@ describe ObjectStorage do end end - it_behaves_like 'uses remote storage' do - let(:storage_url) { "https://storage.googleapis.com/uploads/" } + context 'for known length' do + it_behaves_like 'uses remote storage without multipart uploads' do + it 'returns links for Google Cloud' do + expect(subject[:RemoteObject][:GetURL]).to start_with(storage_url) + expect(subject[:RemoteObject][:DeleteURL]).to start_with(storage_url) + expect(subject[:RemoteObject][:StoreURL]).to start_with(storage_url) + end + end + end + + context 'for unknown length' do + let(:has_length) { false } + let(:maximum_size) { 1.gigabyte } - it 'returns links for Google Cloud' do - expect(subject[:RemoteObject][:GetURL]).to start_with(storage_url) - expect(subject[:RemoteObject][:DeleteURL]).to start_with(storage_url) - expect(subject[:RemoteObject][:StoreURL]).to start_with(storage_url) + it_behaves_like 'uses remote storage without multipart uploads' do + it 'returns links for Google Cloud' do + expect(subject[:RemoteObject][:GetURL]).to start_with(storage_url) + expect(subject[:RemoteObject][:DeleteURL]).to start_with(storage_url) + expect(subject[:RemoteObject][:StoreURL]).to start_with(storage_url) + end end end end context 'uses GDK/minio' do + let(:storage_url) { "http://minio:9000/uploads/" } + before do expect(uploader_class).to receive(:object_store_credentials) do { provider: "AWS", aws_access_key_id: "AWS_ACCESS_KEY_ID", aws_secret_access_key: "AWS_SECRET_ACCESS_KEY", - endpoint: 'http://127.0.0.1:9000', + endpoint: 'http://minio:9000', path_style: true, region: "gdk" } end end - it_behaves_like 'uses remote storage' do - let(:storage_url) { "http://127.0.0.1:9000/uploads/" } + context 'for known length' do + it_behaves_like 'uses remote storage without multipart uploads' do + it 'returns links for S3' do + expect(subject[:RemoteObject][:GetURL]).to start_with(storage_url) + expect(subject[:RemoteObject][:DeleteURL]).to start_with(storage_url) + expect(subject[:RemoteObject][:StoreURL]).to start_with(storage_url) + end + end + end + + context 'for unknown length' do + let(:has_length) { false } + let(:maximum_size) { 1.gigabyte } - it 'returns links for S3' do - expect(subject[:RemoteObject][:GetURL]).to start_with(storage_url) - expect(subject[:RemoteObject][:DeleteURL]).to start_with(storage_url) - expect(subject[:RemoteObject][:StoreURL]).to start_with(storage_url) + before do + stub_object_storage_multipart_init(storage_url) + end + + it_behaves_like 'uses remote storage with multipart uploads' do + it 'returns links for S3' do + expect(subject[:RemoteObject][:GetURL]).to start_with(storage_url) + expect(subject[:RemoteObject][:DeleteURL]).to start_with(storage_url) + expect(subject[:RemoteObject][:StoreURL]).to start_with(storage_url) + expect(subject[:RemoteObject][:MultipartUpload][:PartURLs]).to all(start_with(storage_url)) + expect(subject[:RemoteObject][:MultipartUpload][:CompleteURL]).to start_with(storage_url) + expect(subject[:RemoteObject][:MultipartUpload][:AbortURL]).to start_with(storage_url) + end end end end @@ -498,7 +595,7 @@ describe ObjectStorage do context 'when local file is used' do context 'when valid file is used' do let(:uploaded_file) do - fixture_file_upload(Rails.root + 'spec/fixtures/rails_sample.jpg', 'image/jpg') + fixture_file_upload('spec/fixtures/rails_sample.jpg', 'image/jpg') end it "properly caches the file" do @@ -659,4 +756,26 @@ describe ObjectStorage do end end end + + describe '#retrieve_from_store!' do + [:group, :project, :user].each do |model| + context "for #{model}s" do + let(:models) { create_list(model, 3, :with_avatar).map(&:reload) } + let(:avatars) { models.map(&:avatar) } + + it 'batches fetching uploads from the database' do + # Ensure that these are all created and fully loaded before we start + # running queries for avatars + models + + expect { avatars }.not_to exceed_query_limit(1) + end + + it 'fetches a unique upload for each model' do + expect(avatars.map(&:url).uniq).to eq(avatars.map(&:url)) + expect(avatars.map(&:upload).uniq).to eq(avatars.map(&:upload)) + end + end + end + end end |