From d087b4dac466fc7da6dbdc8668aea943af4546e8 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Tue, 21 Nov 2017 18:34:00 +0100 Subject: FileUploader should check for hashed_storage?(:attachments) to use disk_path --- spec/uploaders/file_uploader_spec.rb | 50 +++++++++++++++++++++++++++--------- 1 file changed, 38 insertions(+), 12 deletions(-) (limited to 'spec/uploaders') diff --git a/spec/uploaders/file_uploader_spec.rb b/spec/uploaders/file_uploader_spec.rb index f52b2bab05b..fd195d6f9b8 100644 --- a/spec/uploaders/file_uploader_spec.rb +++ b/spec/uploaders/file_uploader_spec.rb @@ -28,25 +28,51 @@ describe FileUploader do end context 'hashed storage' do - let(:project) { build_stubbed(:project, :hashed) } + context 'when rolled out attachments' do + 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') + 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 + dynamic_segment = project.disk_path - expect(described_class.absolute_path(upload)) - .to end_with("#{dynamic_segment}/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.disk_path) + expect(uploader.store_dir).not_to include("system") + end end end - describe "#store_dir" do - it "stores in the namespace path" do - uploader = described_class.new(project) + context 'when only repositories are rolled out' do + let(:project) { build_stubbed(:project, storage_version: Project::HASHED_STORAGE_FEATURES[:repository]) } - expect(uploader.store_dir).to include(project.disk_path) - expect(uploader.store_dir).not_to include("system") + 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 + + 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 -- cgit v1.2.3 From 61864a5a5bb523953589c9398a431c4369fbfc76 Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Thu, 21 Sep 2017 10:34:12 +0200 Subject: Rename Artifact to JobArtifact, split metadata out Two things at ones, as there was no clean way to seperate the commit and give me feedback from the tests. But the model Artifact is now JobArtifact, and the table does not have a type anymore, but the metadata is now its own model: Ci::JobArtifactMetadata. --- spec/uploaders/artifact_uploader_spec.rb | 4 ++-- spec/uploaders/job_artifact_uploader_spec.rb | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) create mode 100644 spec/uploaders/job_artifact_uploader_spec.rb (limited to 'spec/uploaders') diff --git a/spec/uploaders/artifact_uploader_spec.rb b/spec/uploaders/artifact_uploader_spec.rb index 2a3bd0e3bb2..9cb2c090b43 100644 --- a/spec/uploaders/artifact_uploader_spec.rb +++ b/spec/uploaders/artifact_uploader_spec.rb @@ -1,7 +1,7 @@ require 'rails_helper' describe ArtifactUploader do - let(:job) { create(:ci_build) } + set(:job) { create(:ci_build) } let(:uploader) { described_class.new(job, :artifacts_file) } let(:path) { Gitlab.config.artifacts.path } @@ -26,7 +26,7 @@ describe ArtifactUploader do subject { uploader.store_dir } it { is_expected.to start_with(path) } - it { is_expected.to end_with("#{job.project_id}/#{job.id}") } + it { is_expected.to end_with("#{job.project_id}/#{job.created_at.utc.strftime('%Y_%m')}/#{job.id}") } end describe '#cache_dir' do diff --git a/spec/uploaders/job_artifact_uploader_spec.rb b/spec/uploaders/job_artifact_uploader_spec.rb new file mode 100644 index 00000000000..d045acf9089 --- /dev/null +++ b/spec/uploaders/job_artifact_uploader_spec.rb @@ -0,0 +1,15 @@ +require 'spec_helper' + +describe JobArtifactUploader do + set(:job_artifact) { create(:ci_job_artifact) } + let(:job) { job_artifact.job } + let(:uploader) { described_class.new(job_artifact, :file) } + + describe '#store_dir' do + subject { uploader.store_dir } + + it { is_expected.to start_with(Gitlab.config.artifacts.path) } + it { is_expected.not_to end_with("#{job.project_id}/#{job.created_at.utc.strftime('%Y_%m')}/#{job.id}") } + it { is_expected.to match(/\h{2}\/\h{2}\/\h{64}\/\d{4}_\d{1,2}_\d{1,2}\/\d+\/\d+\z/) } + end +end -- cgit v1.2.3 From ba5697fd809563cb2fd619d7c50362303ab86434 Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Wed, 8 Nov 2017 10:46:47 +0100 Subject: Fix legacy migration test --- spec/uploaders/artifact_uploader_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/uploaders') diff --git a/spec/uploaders/artifact_uploader_spec.rb b/spec/uploaders/artifact_uploader_spec.rb index 9cb2c090b43..0a07a7337b5 100644 --- a/spec/uploaders/artifact_uploader_spec.rb +++ b/spec/uploaders/artifact_uploader_spec.rb @@ -26,7 +26,7 @@ describe ArtifactUploader do subject { uploader.store_dir } it { is_expected.to start_with(path) } - it { is_expected.to end_with("#{job.project_id}/#{job.created_at.utc.strftime('%Y_%m')}/#{job.id}") } + it { is_expected.to end_with("#{job.project_id}/#{job.id}") } end describe '#cache_dir' do -- cgit v1.2.3 From 871de0f18581bb03fed5c0d800f8183598a0195f Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 23 Nov 2017 16:57:27 +0100 Subject: Rename artifacts_* to legacy_artifacts_* --- spec/uploaders/artifact_uploader_spec.rb | 61 ------------------------- spec/uploaders/legacy_artifact_uploader_spec.rb | 61 +++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 61 deletions(-) delete mode 100644 spec/uploaders/artifact_uploader_spec.rb create mode 100644 spec/uploaders/legacy_artifact_uploader_spec.rb (limited to 'spec/uploaders') diff --git a/spec/uploaders/artifact_uploader_spec.rb b/spec/uploaders/artifact_uploader_spec.rb deleted file mode 100644 index 0a07a7337b5..00000000000 --- a/spec/uploaders/artifact_uploader_spec.rb +++ /dev/null @@ -1,61 +0,0 @@ -require 'rails_helper' - -describe ArtifactUploader do - set(:job) { create(:ci_build) } - let(:uploader) { described_class.new(job, :artifacts_file) } - let(:path) { Gitlab.config.artifacts.path } - - describe '.local_artifacts_store' do - subject { described_class.local_artifacts_store } - - 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(path) } - it { is_expected.to end_with('tmp/uploads/') } - end - - describe '#store_dir' do - subject { uploader.store_dir } - - it { is_expected.to start_with(path) } - it { is_expected.to end_with("#{job.project_id}/#{job.id}") } - 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 - - describe '#filename' do - # we need to use uploader, as this makes to use mounter - # which initialises uploader.file object - let(:uploader) { job.artifacts_file } - - subject { uploader.filename } - - it { is_expected.to be_nil } - - context 'with artifacts' do - let(:job) { create(:ci_build, :artifacts) } - - it { is_expected.not_to be_nil } - end - end -end diff --git a/spec/uploaders/legacy_artifact_uploader_spec.rb b/spec/uploaders/legacy_artifact_uploader_spec.rb new file mode 100644 index 00000000000..1d9adaccd8d --- /dev/null +++ b/spec/uploaders/legacy_artifact_uploader_spec.rb @@ -0,0 +1,61 @@ +require 'rails_helper' + +describe LegacyArtifactUploader do + set(:job) { create(:ci_build) } + let(:uploader) { described_class.new(job, :artifacts_file) } + let(:path) { Gitlab.config.artifacts.path } + + describe '.local_artifacts_store' do + subject { described_class.local_artifacts_store } + + 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(path) } + it { is_expected.to end_with('tmp/uploads/') } + end + + describe '#store_dir' do + subject { uploader.store_dir } + + it { is_expected.to start_with(path) } + it { is_expected.to end_with("#{job.project_id}/#{job.id}") } + 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 + + describe '#filename' do + # we need to use uploader, as this makes to use mounter + # which initialises uploader.file object + let(:uploader) { job.artifacts_file } + + subject { uploader.filename } + + it { is_expected.to be_nil } + + context 'with artifacts' do + let(:job) { create(:ci_build, :artifacts) } + + it { is_expected.not_to be_nil } + end + end +end -- cgit v1.2.3 From 38c61ab6df15fbd1eab22a8dff8da01b17c075f3 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 23 Nov 2017 18:51:20 +0100 Subject: Fix specs failures, and use factory with `:ci_job_artifact, :archive` --- spec/uploaders/job_artifact_uploader_spec.rb | 38 +++++++++++++++++++++++-- spec/uploaders/legacy_artifact_uploader_spec.rb | 22 ++++++++++++-- 2 files changed, 55 insertions(+), 5 deletions(-) (limited to 'spec/uploaders') diff --git a/spec/uploaders/job_artifact_uploader_spec.rb b/spec/uploaders/job_artifact_uploader_spec.rb index d045acf9089..bb2cc52381d 100644 --- a/spec/uploaders/job_artifact_uploader_spec.rb +++ b/spec/uploaders/job_artifact_uploader_spec.rb @@ -2,14 +2,46 @@ require 'spec_helper' describe JobArtifactUploader do set(:job_artifact) { create(:ci_job_artifact) } - let(:job) { job_artifact.job } let(:uploader) { described_class.new(job_artifact, :file) } + let(:path) { Gitlab.config.artifacts.path } describe '#store_dir' do subject { uploader.store_dir } - it { is_expected.to start_with(Gitlab.config.artifacts.path) } - it { is_expected.not_to end_with("#{job.project_id}/#{job.created_at.utc.strftime('%Y_%m')}/#{job.id}") } + it { is_expected.to start_with(path) } + it { is_expected.not_to end_with("#{job_artifact.project_id}/#{job_artifact.created_at.utc.strftime('%Y_%m')}/#{job_artifact.id}") } it { is_expected.to match(/\h{2}\/\h{2}\/\h{64}\/\d{4}_\d{1,2}_\d{1,2}\/\d+\/\d+\z/) } 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 + + context 'file is stored in valid path' do + let(:file) do + fixture_file_upload(Rails.root.join( + 'spec/fixtures/ci_build_artifacts.zip'), 'application/zip') + end + + before do + uploader.store!(file) + end + + subject { uploader.file.path } + + it { is_expected.to start_with(path) } + it { is_expected.to include("/#{job_artifact.created_at.utc.strftime('%Y_%m_%d')}/") } + it { is_expected.to include("/#{job_artifact.project_id.to_s}/") } + it { is_expected.to end_with("ci_build_artifacts.zip") } + end end diff --git a/spec/uploaders/legacy_artifact_uploader_spec.rb b/spec/uploaders/legacy_artifact_uploader_spec.rb index 1d9adaccd8d..203630de91c 100644 --- a/spec/uploaders/legacy_artifact_uploader_spec.rb +++ b/spec/uploaders/legacy_artifact_uploader_spec.rb @@ -5,8 +5,8 @@ describe LegacyArtifactUploader do let(:uploader) { described_class.new(job, :artifacts_file) } let(:path) { Gitlab.config.artifacts.path } - describe '.local_artifacts_store' do - subject { described_class.local_artifacts_store } + describe '.local_store_path' do + subject { described_class.local_store_path } it "delegate to artifacts path" do expect(Gitlab.config.artifacts).to receive(:path) @@ -58,4 +58,22 @@ describe LegacyArtifactUploader do it { is_expected.not_to be_nil } end end + + context 'file is stored in valid path' do + let(:file) do + fixture_file_upload(Rails.root.join( + 'spec/fixtures/ci_build_artifacts.zip'), 'application/zip') + end + + before do + uploader.store!(file) + end + + subject { uploader.file.path } + + it { is_expected.to start_with(path) } + it { is_expected.to include("/#{job.created_at.utc.strftime('%Y_%m')}/") } + it { is_expected.to include("/#{job.project_id.to_s}/") } + it { is_expected.to end_with("ci_build_artifacts.zip") } + end end -- cgit v1.2.3 From ee8efb3d67ba8b8b2ece9962cd2aa79063fffaa0 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 23 Nov 2017 19:41:49 +0100 Subject: Sync ArtifactUploader specs with EE --- spec/uploaders/job_artifact_uploader_spec.rb | 22 +++++++++++-------- spec/uploaders/legacy_artifact_uploader_spec.rb | 28 ++++++++++++------------- 2 files changed, 26 insertions(+), 24 deletions(-) (limited to 'spec/uploaders') diff --git a/spec/uploaders/job_artifact_uploader_spec.rb b/spec/uploaders/job_artifact_uploader_spec.rb index bb2cc52381d..e80d5272a4a 100644 --- a/spec/uploaders/job_artifact_uploader_spec.rb +++ b/spec/uploaders/job_artifact_uploader_spec.rb @@ -1,33 +1,37 @@ require 'spec_helper' describe JobArtifactUploader do - set(:job_artifact) { create(:ci_job_artifact) } + let(:job_artifact) { create(:ci_job_artifact) } let(:uploader) { described_class.new(job_artifact, :file) } - let(:path) { Gitlab.config.artifacts.path } + let(:local_path) { Gitlab.config.artifacts.path } describe '#store_dir' do subject { uploader.store_dir } - it { is_expected.to start_with(path) } - it { is_expected.not_to end_with("#{job_artifact.project_id}/#{job_artifact.created_at.utc.strftime('%Y_%m')}/#{job_artifact.id}") } - it { is_expected.to match(/\h{2}\/\h{2}\/\h{64}\/\d{4}_\d{1,2}_\d{1,2}\/\d+\/\d+\z/) } + let(:path) { "#{job_artifact.created_at.utc.strftime('%Y_%m_%d')}/#{job_artifact.project_id}/#{job_artifact.id}" } + + context 'when using local storage' do + it { is_expected.to start_with(local_path) } + it { is_expected.to match(/\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(path) } + 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(path) } + it { is_expected.to start_with(local_path) } it { is_expected.to end_with('/tmp/work') } end - context 'file is stored in valid path' do + context 'file is stored in valid local_path' do let(:file) do fixture_file_upload(Rails.root.join( 'spec/fixtures/ci_build_artifacts.zip'), 'application/zip') @@ -39,7 +43,7 @@ describe JobArtifactUploader do subject { uploader.file.path } - it { is_expected.to start_with(path) } + 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.project_id.to_s}/") } 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 203630de91c..714976b92c1 100644 --- a/spec/uploaders/legacy_artifact_uploader_spec.rb +++ b/spec/uploaders/legacy_artifact_uploader_spec.rb @@ -1,9 +1,9 @@ require 'rails_helper' describe LegacyArtifactUploader do - set(:job) { create(:ci_build) } - let(:uploader) { described_class.new(job, :artifacts_file) } - let(:path) { Gitlab.config.artifacts.path } + let(:job) { create(:ci_build) } + let(:uploader) { described_class.new(job, :legacy_artifacts_file) } + let(:local_path) { Gitlab.config.artifacts.path } describe '.local_store_path' do subject { described_class.local_store_path } @@ -18,28 +18,32 @@ describe LegacyArtifactUploader do describe '.artifacts_upload_path' do subject { described_class.artifacts_upload_path } - it { is_expected.to start_with(path) } + it { is_expected.to start_with(local_path) } it { is_expected.to end_with('tmp/uploads/') } end describe '#store_dir' do subject { uploader.store_dir } - it { is_expected.to start_with(path) } - it { is_expected.to end_with("#{job.project_id}/#{job.id}") } + 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 describe '#cache_dir' do subject { uploader.cache_dir } - it { is_expected.to start_with(path) } + 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(path) } + it { is_expected.to start_with(local_path) } it { is_expected.to end_with('/tmp/work') } end @@ -51,12 +55,6 @@ describe LegacyArtifactUploader do subject { uploader.filename } it { is_expected.to be_nil } - - context 'with artifacts' do - let(:job) { create(:ci_build, :artifacts) } - - it { is_expected.not_to be_nil } - end end context 'file is stored in valid path' do @@ -71,7 +69,7 @@ describe LegacyArtifactUploader do subject { uploader.file.path } - it { is_expected.to start_with(path) } + 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.to_s}/") } it { is_expected.to end_with("ci_build_artifacts.zip") } -- cgit v1.2.3 From 6881d0917458f9b62542ee2908e2d258c75a8537 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 23 Nov 2017 19:53:31 +0100 Subject: Fix rubocop --- spec/uploaders/job_artifact_uploader_spec.rb | 6 +++--- spec/uploaders/legacy_artifact_uploader_spec.rb | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) (limited to 'spec/uploaders') diff --git a/spec/uploaders/job_artifact_uploader_spec.rb b/spec/uploaders/job_artifact_uploader_spec.rb index e80d5272a4a..14fd5f3600f 100644 --- a/spec/uploaders/job_artifact_uploader_spec.rb +++ b/spec/uploaders/job_artifact_uploader_spec.rb @@ -33,8 +33,8 @@ describe JobArtifactUploader do context 'file is stored in valid local_path' do let(:file) do - fixture_file_upload(Rails.root.join( - 'spec/fixtures/ci_build_artifacts.zip'), 'application/zip') + fixture_file_upload( + Rails.root.join('spec/fixtures/ci_build_artifacts.zip'), 'application/zip') end before do @@ -45,7 +45,7 @@ describe JobArtifactUploader do 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.project_id.to_s}/") } + it { is_expected.to include("/#{job_artifact.project_id}/") } it { is_expected.to end_with("ci_build_artifacts.zip") } end end diff --git a/spec/uploaders/legacy_artifact_uploader_spec.rb b/spec/uploaders/legacy_artifact_uploader_spec.rb index 714976b92c1..efeffb78772 100644 --- a/spec/uploaders/legacy_artifact_uploader_spec.rb +++ b/spec/uploaders/legacy_artifact_uploader_spec.rb @@ -59,8 +59,8 @@ describe LegacyArtifactUploader do context 'file is stored in valid path' do let(:file) do - fixture_file_upload(Rails.root.join( - 'spec/fixtures/ci_build_artifacts.zip'), 'application/zip') + fixture_file_upload( + Rails.root.join('spec/fixtures/ci_build_artifacts.zip'), 'application/zip') end before do @@ -71,7 +71,7 @@ describe LegacyArtifactUploader do 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.to_s}/") } + it { is_expected.to include("/#{job.project_id}/") } it { is_expected.to end_with("ci_build_artifacts.zip") } end end -- cgit v1.2.3 From f7c18ca31469b199c1a898cef583c9aae99f1375 Mon Sep 17 00:00:00 2001 From: Jarka Kadlecova Date: Wed, 6 Dec 2017 12:36:11 +0100 Subject: Support uploads for groups --- spec/uploaders/namespace_file_uploader_spec.rb | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 spec/uploaders/namespace_file_uploader_spec.rb (limited to 'spec/uploaders') diff --git a/spec/uploaders/namespace_file_uploader_spec.rb b/spec/uploaders/namespace_file_uploader_spec.rb new file mode 100644 index 00000000000..c6c4500c179 --- /dev/null +++ b/spec/uploaders/namespace_file_uploader_spec.rb @@ -0,0 +1,21 @@ +require 'spec_helper' + +describe NamespaceFileUploader do + let(:group) { build_stubbed(:group) } + let(:uploader) { described_class.new(group) } + + 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) + + expect(described_class.absolute_path(upload_record)) + .to include("-/system/namespace/#{group.id}") + end + end +end -- cgit v1.2.3