diff options
Diffstat (limited to 'spec/uploaders')
-rw-r--r-- | spec/uploaders/attachment_uploader_spec.rb | 30 | ||||
-rw-r--r-- | spec/uploaders/avatar_uploader_spec.rb | 18 | ||||
-rw-r--r-- | spec/uploaders/file_mover_spec.rb | 18 | ||||
-rw-r--r-- | spec/uploaders/file_uploader_spec.rb | 121 | ||||
-rw-r--r-- | spec/uploaders/job_artifact_uploader_spec.rb | 32 | ||||
-rw-r--r-- | spec/uploaders/legacy_artifact_uploader_spec.rb | 49 | ||||
-rw-r--r-- | spec/uploaders/lfs_object_uploader_spec.rb | 38 | ||||
-rw-r--r-- | spec/uploaders/namespace_file_uploader_spec.rb | 21 | ||||
-rw-r--r-- | spec/uploaders/personal_file_uploader_spec.rb | 28 | ||||
-rw-r--r-- | spec/uploaders/records_uploads_spec.rb | 73 |
10 files changed, 294 insertions, 134 deletions
diff --git a/spec/uploaders/attachment_uploader_spec.rb b/spec/uploaders/attachment_uploader_spec.rb index 091ba824fc6..04ee6e9bfad 100644 --- a/spec/uploaders/attachment_uploader_spec.rb +++ b/spec/uploaders/attachment_uploader_spec.rb @@ -1,14 +1,28 @@ require 'spec_helper' describe AttachmentUploader do - let(:note) { create(:note, :with_attachment) } - let(:uploader) { note.attachment } - let(:upload) { create(:upload, :attachment_upload, model: uploader.model) } + let(:uploader) { described_class.new(build_stubbed(:user)) } - subject { uploader } + describe "#store_dir" do + it "stores in the system dir" do + expect(uploader.store_dir).to start_with("uploads/-/system/user") + end - it_behaves_like 'builds correct paths', - store_dir: %r[uploads/-/system/note/attachment/], - upload_path: %r[uploads/-/system/note/attachment/], - absolute_path: %r[#{CarrierWave.root}/uploads/-/system/note/attachment/] + it "uses the old path when using object storage" do + expect(described_class).to receive(:file_storage?).and_return(false) + expect(uploader.store_dir).to start_with("uploads/user") + end + end + + describe '#move_to_cache' do + it 'is true' do + expect(uploader.move_to_cache).to eq(true) + end + end + + describe '#move_to_store' do + it 'is true' do + expect(uploader.move_to_store).to eq(true) + end + end end diff --git a/spec/uploaders/avatar_uploader_spec.rb b/spec/uploaders/avatar_uploader_spec.rb index bf9028c9260..1dc574699d8 100644 --- a/spec/uploaders/avatar_uploader_spec.rb +++ b/spec/uploaders/avatar_uploader_spec.rb @@ -1,16 +1,18 @@ require 'spec_helper' describe AvatarUploader do - let(:model) { create(:user, :with_avatar) } - let(:uploader) { described_class.new(model, :avatar) } - let(:upload) { create(:upload, model: model) } + let(:uploader) { described_class.new(build_stubbed(:user)) } - subject { uploader } + describe "#store_dir" do + it "stores in the system dir" do + expect(uploader.store_dir).to start_with("uploads/-/system/user") + end - it_behaves_like 'builds correct paths', - store_dir: %r[uploads/-/system/user/avatar/], - upload_path: %r[uploads/-/system/user/avatar/], - absolute_path: %r[#{CarrierWave.root}/uploads/-/system/user/avatar/] + it "uses the old path when using object storage" do + expect(described_class).to receive(:file_storage?).and_return(false) + expect(uploader.store_dir).to start_with("uploads/user") + end + end describe '#move_to_cache' do it 'is false' do diff --git a/spec/uploaders/file_mover_spec.rb b/spec/uploaders/file_mover_spec.rb index bc024cd307c..0cf462e9553 100644 --- a/spec/uploaders/file_mover_spec.rb +++ b/spec/uploaders/file_mover_spec.rb @@ -3,13 +3,13 @@ require 'spec_helper' describe FileMover do let(:filename) { 'banana_sample.gif' } let(:file) { fixture_file_upload(Rails.root.join('spec', 'fixtures', filename)) } - let(:temp_file_path) { File.join('uploads/-/system/temp', 'secret55', filename) } - let(:temp_description) do - "test ![banana_sample](/#{temp_file_path}) "\ - "same ![banana_sample](/#{temp_file_path}) " + 'test ![banana_sample](/uploads/-/system/temp/secret55/banana_sample.gif) same ![banana_sample]'\ + '(/uploads/-/system/temp/secret55/banana_sample.gif)' end - let(:file_path) { File.join('uploads/-/system/personal_snippet', snippet.id.to_s, 'secret55', filename) } + let(:temp_file_path) { File.join('secret55', filename).to_s } + let(:file_path) { File.join('uploads', '-', 'system', 'personal_snippet', snippet.id.to_s, 'secret55', filename).to_s } + let(:snippet) { create(:personal_snippet, description: temp_description) } subject { described_class.new(file_path, snippet).execute } @@ -28,8 +28,8 @@ describe FileMover do 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) " + "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 @@ -50,8 +50,8 @@ describe FileMover do expect(snippet.reload.description) .to eq( - "test ![banana_sample](/uploads/-/system/temp/secret55/banana_sample.gif) "\ - "same ![banana_sample](/uploads/-/system/temp/secret55/banana_sample.gif) " + "test ![banana_sample](/uploads/-/system/temp/secret55/banana_sample.gif)"\ + " same ![banana_sample](/uploads/-/system/temp/secret55/banana_sample.gif)" ) end diff --git a/spec/uploaders/file_uploader_spec.rb b/spec/uploaders/file_uploader_spec.rb index a72f853df75..845516e5004 100644 --- a/spec/uploaders/file_uploader_spec.rb +++ b/spec/uploaders/file_uploader_spec.rb @@ -1,57 +1,118 @@ require 'spec_helper' describe FileUploader do - let(:group) { create(:group, name: 'awesome') } - let(:project) { create(:project, namespace: group, name: 'project') } - let(:uploader) { described_class.new(project) } - let(:upload) { double(model: project, path: 'secret/foo.jpg') } + let(:uploader) { described_class.new(build_stubbed(:project)) } - subject { uploader } + context 'legacy storage' do + let(:project) { build_stubbed(:project) } + + describe '.absolute_path' do + it 'returns the correct absolute path by building it dynamically' do + upload = double(model: project, path: 'secret/foo.jpg') + + dynamic_segment = project.full_path - shared_examples 'builds correct legacy storage paths' do - include_examples 'builds correct paths', - store_dir: %r{awesome/project/\h+}, - absolute_path: %r{#{described_class.root}/awesome/project/secret/foo.jpg} + expect(described_class.absolute_path(upload)) + .to end_with("#{dynamic_segment}/secret/foo.jpg") + end + end + + describe "#store_dir" do + it "stores in the namespace path" do + uploader = described_class.new(project) + + expect(uploader.store_dir).to include(project.full_path) + expect(uploader.store_dir).not_to include("system") + end + end end - shared_examples 'uses hashed storage' do + context 'hashed storage' do context 'when rolled out attachments' do - before do - allow(project).to receive(:disk_path).and_return('ca/fe/fe/ed') + let(:project) { build_stubbed(:project, :hashed) } + + describe '.absolute_path' do + it 'returns the correct absolute path by building it dynamically' do + upload = double(model: project, path: 'secret/foo.jpg') + + dynamic_segment = project.disk_path + + expect(described_class.absolute_path(upload)) + .to end_with("#{dynamic_segment}/secret/foo.jpg") + end end - let(:project) { build_stubbed(:project, :hashed, namespace: group, name: 'project') } + describe "#store_dir" do + it "stores in the namespace path" do + uploader = described_class.new(project) - it_behaves_like 'builds correct paths', - store_dir: %r{ca/fe/fe/ed/\h+}, - absolute_path: %r{#{described_class.root}/ca/fe/fe/ed/secret/foo.jpg} + expect(uploader.store_dir).to include(project.disk_path) + expect(uploader.store_dir).not_to include("system") + end + end end context 'when only repositories are rolled out' do - let(:project) { build_stubbed(:project, namespace: group, name: 'project', storage_version: Project::HASHED_STORAGE_FEATURES[:repository]) } + let(:project) { build_stubbed(:project, storage_version: Project::HASHED_STORAGE_FEATURES[:repository]) } - it_behaves_like 'builds correct legacy storage paths' - end - end + describe '.absolute_path' do + it 'returns the correct absolute path by building it dynamically' do + upload = double(model: project, path: 'secret/foo.jpg') - context 'legacy storage' do - it_behaves_like 'builds correct legacy storage paths' - include_examples 'uses hashed storage' + dynamic_segment = project.full_path + + expect(described_class.absolute_path(upload)) + .to end_with("#{dynamic_segment}/secret/foo.jpg") + end + end + + describe "#store_dir" do + it "stores in the namespace path" do + uploader = described_class.new(project) + + expect(uploader.store_dir).to include(project.full_path) + expect(uploader.store_dir).not_to include("system") + end + end + end end describe 'initialize' do - let(:uploader) { described_class.new(double, 'secret') } + it 'generates a secret if none is provided' do + expect(SecureRandom).to receive(:hex).and_return('secret') + + uploader = described_class.new(double) + + expect(uploader.secret).to eq 'secret' + end it 'accepts a secret parameter' do - expect(described_class).not_to receive(:generate_secret) - expect(uploader.secret).to eq('secret') + expect(SecureRandom).not_to receive(:hex) + + uploader = described_class.new(double, 'secret') + + expect(uploader.secret).to eq 'secret' end end - describe '#secret' do - it 'generates a secret if none is provided' do - expect(described_class).to receive(:generate_secret).and_return('secret') - expect(uploader.secret).to eq('secret') + describe '#move_to_cache' do + it 'is true' do + expect(uploader.move_to_cache).to eq(true) + end + end + + describe '#move_to_store' do + it 'is true' do + expect(uploader.move_to_store).to eq(true) + end + end + + describe '#relative_path' do + it 'removes the leading dynamic path segment' do + fixture = Rails.root.join('spec', 'fixtures', 'rails_sample.jpg') + uploader.store!(fixture_file_upload(fixture)) + + expect(uploader.relative_path).to match(%r{\A\h{32}/rails_sample.jpg\z}) end end end diff --git a/spec/uploaders/job_artifact_uploader_spec.rb b/spec/uploaders/job_artifact_uploader_spec.rb index d606404e95d..a067c3e75f4 100644 --- a/spec/uploaders/job_artifact_uploader_spec.rb +++ b/spec/uploaders/job_artifact_uploader_spec.rb @@ -3,13 +3,33 @@ require 'spec_helper' describe JobArtifactUploader do let(:job_artifact) { create(:ci_job_artifact) } let(:uploader) { described_class.new(job_artifact, :file) } + let(:local_path) { Gitlab.config.artifacts.path } - subject { uploader } + describe '#store_dir' do + subject { uploader.store_dir } - it_behaves_like "builds correct paths", - store_dir: %r[\h{2}/\h{2}/\h{64}/\d{4}_\d{1,2}_\d{1,2}/\d+/\d+\z], - cache_dir: %r[artifacts/tmp/cache], - work_dir: %r[artifacts/tmp/work] + let(:path) { "#{job_artifact.created_at.utc.strftime('%Y_%m_%d')}/#{job_artifact.job_id}/#{job_artifact.id}" } + + context 'when using local storage' do + it { is_expected.to start_with(local_path) } + it { is_expected.to match(%r{\h{2}/\h{2}/\h{64}/\d{4}_\d{1,2}_\d{1,2}/\d+/\d+\z}) } + it { is_expected.to end_with(path) } + end + end + + describe '#cache_dir' do + subject { uploader.cache_dir } + + it { is_expected.to start_with(local_path) } + it { is_expected.to end_with('/tmp/cache') } + end + + describe '#work_dir' do + subject { uploader.work_dir } + + it { is_expected.to start_with(local_path) } + it { is_expected.to end_with('/tmp/work') } + end context 'file is stored in valid local_path' do let(:file) do @@ -23,7 +43,7 @@ describe JobArtifactUploader do subject { uploader.file.path } - it { is_expected.to start_with("#{uploader.root}/#{uploader.class.base_dir}") } + it { is_expected.to start_with(local_path) } it { is_expected.to include("/#{job_artifact.created_at.utc.strftime('%Y_%m_%d')}/") } it { is_expected.to include("/#{job_artifact.job_id}/#{job_artifact.id}/") } it { is_expected.to end_with("ci_build_artifacts.zip") } diff --git a/spec/uploaders/legacy_artifact_uploader_spec.rb b/spec/uploaders/legacy_artifact_uploader_spec.rb index 54c6a8b869b..efeffb78772 100644 --- a/spec/uploaders/legacy_artifact_uploader_spec.rb +++ b/spec/uploaders/legacy_artifact_uploader_spec.rb @@ -3,22 +3,49 @@ require 'rails_helper' describe LegacyArtifactUploader do let(:job) { create(:ci_build) } let(:uploader) { described_class.new(job, :legacy_artifacts_file) } - let(:local_path) { described_class.root } + let(:local_path) { Gitlab.config.artifacts.path } - subject { uploader } + describe '.local_store_path' do + subject { described_class.local_store_path } - # TODO: move to Workhorse::UploadPath - describe '.workhorse_upload_path' do - subject { described_class.workhorse_upload_path } + it "delegate to artifacts path" do + expect(Gitlab.config.artifacts).to receive(:path) + + subject + end + end + + describe '.artifacts_upload_path' do + subject { described_class.artifacts_upload_path } it { is_expected.to start_with(local_path) } - it { is_expected.to end_with('tmp/uploads') } + it { is_expected.to end_with('tmp/uploads/') } + end + + describe '#store_dir' do + subject { uploader.store_dir } + + let(:path) { "#{job.created_at.utc.strftime('%Y_%m')}/#{job.project_id}/#{job.id}" } + + context 'when using local storage' do + it { is_expected.to start_with(local_path) } + it { is_expected.to end_with(path) } + end end - it_behaves_like "builds correct paths", - store_dir: %r[\d{4}_\d{1,2}/\d+/\d+\z], - cache_dir: %r[artifacts/tmp/cache], - work_dir: %r[artifacts/tmp/work] + describe '#cache_dir' do + subject { uploader.cache_dir } + + it { is_expected.to start_with(local_path) } + it { is_expected.to end_with('/tmp/cache') } + end + + describe '#work_dir' do + subject { uploader.work_dir } + + it { is_expected.to start_with(local_path) } + it { is_expected.to end_with('/tmp/work') } + end describe '#filename' do # we need to use uploader, as this makes to use mounter @@ -42,7 +69,7 @@ describe LegacyArtifactUploader do subject { uploader.file.path } - it { is_expected.to start_with("#{uploader.root}") } + it { is_expected.to start_with(local_path) } it { is_expected.to include("/#{job.created_at.utc.strftime('%Y_%m')}/") } it { is_expected.to include("/#{job.project_id}/") } it { is_expected.to end_with("ci_build_artifacts.zip") } diff --git a/spec/uploaders/lfs_object_uploader_spec.rb b/spec/uploaders/lfs_object_uploader_spec.rb index 6ebc885daa8..7088bc23334 100644 --- a/spec/uploaders/lfs_object_uploader_spec.rb +++ b/spec/uploaders/lfs_object_uploader_spec.rb @@ -2,13 +2,39 @@ require 'spec_helper' describe LfsObjectUploader do let(:lfs_object) { create(:lfs_object, :with_file) } - let(:uploader) { described_class.new(lfs_object, :file) } + let(:uploader) { described_class.new(lfs_object) } let(:path) { Gitlab.config.lfs.storage_path } - subject { uploader } + describe '#move_to_cache' do + it 'is true' do + expect(uploader.move_to_cache).to eq(true) + end + end - it_behaves_like "builds correct paths", - store_dir: %r[\h{2}/\h{2}], - cache_dir: %r[/lfs-objects/tmp/cache], - work_dir: %r[/lfs-objects/tmp/work] + describe '#move_to_store' do + it 'is true' do + expect(uploader.move_to_store).to eq(true) + end + end + + describe '#store_dir' do + subject { uploader.store_dir } + + it { is_expected.to start_with(path) } + it { is_expected.to end_with("#{lfs_object.oid[0, 2]}/#{lfs_object.oid[2, 2]}") } + end + + describe '#cache_dir' do + subject { uploader.cache_dir } + + it { is_expected.to start_with(path) } + it { is_expected.to end_with('/tmp/cache') } + end + + describe '#work_dir' do + subject { uploader.work_dir } + + it { is_expected.to start_with(path) } + it { is_expected.to end_with('/tmp/work') } + end end diff --git a/spec/uploaders/namespace_file_uploader_spec.rb b/spec/uploaders/namespace_file_uploader_spec.rb index 24a2fc0f72e..c6c4500c179 100644 --- a/spec/uploaders/namespace_file_uploader_spec.rb +++ b/spec/uploaders/namespace_file_uploader_spec.rb @@ -1,16 +1,21 @@ require 'spec_helper' -IDENTIFIER = %r{\h+/\S+} - describe NamespaceFileUploader do let(:group) { build_stubbed(:group) } let(:uploader) { described_class.new(group) } - let(:upload) { create(:upload, :namespace_upload, model: group) } - subject { uploader } + describe "#store_dir" do + it "stores in the namespace id directory" do + expect(uploader.store_dir).to include(group.id.to_s) + end + end + + describe ".absolute_path" do + it "stores in thecorrect directory" do + upload_record = create(:upload, :namespace_upload, model: group) - it_behaves_like 'builds correct paths', - store_dir: %r[uploads/-/system/namespace/\d+], - upload_path: IDENTIFIER, - absolute_path: %r[#{CarrierWave.root}/uploads/-/system/namespace/\d+/#{IDENTIFIER}] + expect(described_class.absolute_path(upload_record)) + .to include("-/system/namespace/#{group.id}") + end + end end diff --git a/spec/uploaders/personal_file_uploader_spec.rb b/spec/uploaders/personal_file_uploader_spec.rb index ed1fba6edda..cbafa9f478d 100644 --- a/spec/uploaders/personal_file_uploader_spec.rb +++ b/spec/uploaders/personal_file_uploader_spec.rb @@ -1,27 +1,25 @@ require 'spec_helper' -IDENTIFIER = %r{\h+/\S+} - describe PersonalFileUploader do - let(:model) { create(:personal_snippet) } - let(:uploader) { described_class.new(model) } - let(:upload) { create(:upload, :personal_snippet_upload) } + let(:uploader) { described_class.new(build_stubbed(:project)) } + let(:snippet) { create(:personal_snippet) } - subject { uploader } + describe '.absolute_path' do + it 'returns the correct absolute path by building it dynamically' do + upload = double(model: snippet, path: 'secret/foo.jpg') - it_behaves_like 'builds correct paths', - store_dir: %r[uploads/-/system/personal_snippet/\d+], - upload_path: IDENTIFIER, - absolute_path: %r[#{CarrierWave.root}/uploads/-/system/personal_snippet/\d+/#{IDENTIFIER}] + dynamic_segment = "personal_snippet/#{snippet.id}" - describe '#to_h' do - before do - subject.instance_variable_set(:@secret, 'secret') + expect(described_class.absolute_path(upload)).to end_with("/-/system/#{dynamic_segment}/secret/foo.jpg") end + end + + describe '#to_h' do + it 'returns the hass' do + uploader = described_class.new(snippet, 'secret') - it 'is correct' do allow(uploader).to receive(:file).and_return(double(extension: 'txt', filename: 'file_name')) - expected_url = "/uploads/-/system/personal_snippet/#{model.id}/secret/file_name" + expected_url = "/uploads/-/system/personal_snippet/#{snippet.id}/secret/file_name" expect(uploader.to_h).to eq( alt: 'file_name', diff --git a/spec/uploaders/records_uploads_spec.rb b/spec/uploaders/records_uploads_spec.rb index 9a3e5d83e01..7ef7fb7d758 100644 --- a/spec/uploaders/records_uploads_spec.rb +++ b/spec/uploaders/records_uploads_spec.rb @@ -3,16 +3,16 @@ require 'rails_helper' describe RecordsUploads do let!(:uploader) do class RecordsUploadsExampleUploader < GitlabUploader - include RecordsUploads::Concern + include RecordsUploads storage :file - def dynamic_segment - 'co/fe/ee' + def model + FactoryBot.build_stubbed(:user) end end - RecordsUploadsExampleUploader.new(build_stubbed(:user)) + RecordsUploadsExampleUploader.new end def upload_fixture(filename) @@ -20,55 +20,48 @@ describe RecordsUploads do end describe 'callbacks' do - let(:upload) { create(:upload) } - - before do - uploader.upload = upload - end - - it '#record_upload after `store`' do + it 'calls `record_upload` after `store`' do expect(uploader).to receive(:record_upload).once uploader.store!(upload_fixture('doc_sample.txt')) end - it '#destroy_upload after `remove`' do + it 'calls `destroy_upload` after `remove`' do + expect(uploader).to receive(:destroy_upload).once + uploader.store!(upload_fixture('doc_sample.txt')) - expect(uploader).to receive(:destroy_upload).once uploader.remove! end end describe '#record_upload callback' do - it 'creates an Upload record after store' do - expect { uploader.store!(upload_fixture('rails_sample.jpg')) }.to change { Upload.count }.by(1) - end + it 'returns early when not using file storage' do + allow(uploader).to receive(:file_storage?).and_return(false) + expect(Upload).not_to receive(:record) - it 'creates a new record and assigns size, path, model, and uploader' do uploader.store!(upload_fixture('rails_sample.jpg')) - - upload = uploader.upload - aggregate_failures do - expect(upload).to be_persisted - expect(upload.size).to eq uploader.file.size - expect(upload.path).to eq uploader.upload_path - expect(upload.model_id).to eq uploader.model.id - expect(upload.model_type).to eq uploader.model.class.to_s - expect(upload.uploader).to eq uploader.class.to_s - end end - it "does not create an Upload record when the file doesn't exist" do + it "returns early when the file doesn't exist" do allow(uploader).to receive(:file).and_return(double(exists?: false)) + expect(Upload).not_to receive(:record) - expect { uploader.store!(upload_fixture('rails_sample.jpg')) }.not_to change { Upload.count } + uploader.store!(upload_fixture('rails_sample.jpg')) + end + + it 'creates an Upload record after store' do + expect(Upload).to receive(:record) + .with(uploader) + + uploader.store!(upload_fixture('rails_sample.jpg')) end it 'does not create an Upload record if model is missing' do - allow_any_instance_of(RecordsUploadsExampleUploader).to receive(:model).and_return(nil) + expect_any_instance_of(RecordsUploadsExampleUploader).to receive(:model).and_return(nil) + expect(Upload).not_to receive(:record).with(uploader) - expect { uploader.store!(upload_fixture('rails_sample.jpg')) }.not_to change { Upload.count } + uploader.store!(upload_fixture('rails_sample.jpg')) end it 'it destroys Upload records at the same path before recording' do @@ -79,15 +72,29 @@ describe RecordsUploads do uploader: uploader.class.to_s ) - uploader.upload = existing uploader.store!(upload_fixture('rails_sample.jpg')) expect { existing.reload }.to raise_error(ActiveRecord::RecordNotFound) - expect(Upload.count).to eq(1) + expect(Upload.count).to eq 1 end end describe '#destroy_upload callback' do + it 'returns early when not using file storage' do + uploader.store!(upload_fixture('rails_sample.jpg')) + + allow(uploader).to receive(:file_storage?).and_return(false) + expect(Upload).not_to receive(:remove_path) + + uploader.remove! + end + + it 'returns early when file is nil' do + expect(Upload).not_to receive(:remove_path) + + uploader.remove! + end + it 'it destroys Upload records at the same path after removal' do uploader.store!(upload_fixture('rails_sample.jpg')) |