Merge commit '7fabc892f251740dbd9a4755baede662e6854870' into object-storage-ee-to-ce-backport

4 files changed, 131 insertions, 32 deletions

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
diff --git a/spec/uploaders/job_artifact_uploader_spec.rb b/spec/uploaders/job_artifact_uploader_spec.rb
new file mode 100644
index 00000000000..14fd5f3600f
--- /dev/null
+++ b/spec/uploaders/job_artifact_uploader_spec.rb
@@ -0,0 +1,51 @@
+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 }
+
+  describe '#store_dir' do
+    subject { uploader.store_dir }
+
+    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(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
+      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(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}/") }
+    it { is_expected.to end_with("ci_build_artifacts.zip") }
+  end
+end
diff --git a/spec/uploaders/artifact_uploader_spec.rb b/spec/uploaders/legacy_artifact_uploader_spec.rb
index 88f394b2938..efeffb78772 100644
--- a/spec/uploaders/artifact_uploader_spec.rb
+++ b/spec/uploaders/legacy_artifact_uploader_spec.rb
@@ -1,9 +1,8 @@
 require 'rails_helper'
 
-describe ArtifactUploader do
-  let(:store) { described_class::LOCAL_STORE }
-  let(:job) { create(:ci_build, artifacts_file_store: store) }
-  let(:uploader) { described_class.new(job, :artifacts_file) }
+describe LegacyArtifactUploader do
+  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
@@ -18,7 +17,7 @@ describe ArtifactUploader do
 
   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/') }
   end
@@ -32,29 +31,19 @@ describe ArtifactUploader do
       it { is_expected.to start_with(local_path) }
       it { is_expected.to end_with(path) }
     end
-
-    context 'when using remote storage' do
-      let(:store) { described_class::REMOTE_STORE }
-      
-      before do
-        stub_artifacts_object_storage
-      end
-
-      it { is_expected.to eq(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
 
@@ -66,11 +55,23 @@ describe ArtifactUploader do
     subject { uploader.filename }
 
     it { is_expected.to be_nil }
+  end
 
-    context 'with artifacts' do
-      let(:job) { create(:ci_build, :artifacts) }
+  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
 
-      it { is_expected.not_to be_nil }
+    before do
+      uploader.store!(file)
     end
+
+    subject { uploader.file.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}/") }
+    it { is_expected.to end_with("ci_build_artifacts.zip") }
   end
 end
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