Fix and add specs for testing metadata entry

app/uploaders/gitlab_uploader.rb

@@ -71,6 +71,10 @@ class GitlabUploader < CarrierWave::Uploader::Base
     File.join('/', self.class.base_dir, dynamic_segment, filename)
   end
 
+  def cached_size
+    size
+  end
+
   def open
     stream = if file_storage?
       File.open(path, "rb") if path

spec/controllers/projects/jobs_controller_spec.rb

@@ -225,8 +225,11 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do
       end
 
       context 'when there are no network issues' do
+        let(:url) { 'http://object-storage/trace' }
+        let(:file) { expand_fixture_path('trace/sample_trace') }
+
         before do
-          stub_remote_trace_206
+          stub_remote_url_206(url, file)
 
           get_trace
         end
@@ -241,7 +244,7 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do
 
       context 'when there is a network issue' do
         before do
-          stub_remote_trace_500
+          stub_remote_url_500(url)
         end
 
         it 'returns a trace' do

spec/lib/gitlab/http_io_spec.rb

@@ -4,8 +4,11 @@ describe Gitlab::HttpIO do
   include HttpIOHelpers
 
   let(:http_io) { described_class.new(url, size) }
-  let(:url) { remote_trace_url }
-  let(:size) { remote_trace_size }
+
+  let(:url) { 'http://object-storage/trace' }
+  let(:file_path) { expand_fixture_path('trace/sample_trace') }
+  let(:file_body) { File.read(file_path).force_encoding(Encoding::BINARY) }
+  let(:size) { File.size(file_path) }
 
   describe '#close' do
     subject { http_io.close }
@@ -86,10 +89,10 @@ describe Gitlab::HttpIO do
   describe '#each_line' do
     subject { http_io.each_line }
 
-    let(:string_io) { StringIO.new(remote_trace_body) }
+    let(:string_io) { StringIO.new(file_body) }
 
     before do
-      stub_remote_trace_206
+      stub_remote_url_206(url, file_path)
     end
 
     it 'yields lines' do
@@ -99,7 +102,7 @@ describe Gitlab::HttpIO do
     context 'when buckets on GCS' do
       context 'when BUFFER_SIZE is larger than file size' do
         before do
-          stub_remote_trace_200
+          stub_remote_url_200(url, file_path)
           set_larger_buffer_size_than(size)
         end
 
@@ -117,7 +120,7 @@ describe Gitlab::HttpIO do
 
     context 'when there are no network issue' do
       before do
-        stub_remote_trace_206
+        stub_remote_url_206(url, file_path)
       end
 
       context 'when read whole size' do
@@ -129,7 +132,7 @@ describe Gitlab::HttpIO do
           end
 
           it 'reads a trace' do
-            is_expected.to eq(remote_trace_body)
+            is_expected.to eq(file_body)
           end
         end
 
@@ -139,7 +142,7 @@ describe Gitlab::HttpIO do
           end
 
           it 'reads a trace' do
-            is_expected.to eq(remote_trace_body)
+            is_expected.to eq(file_body)
           end
         end
       end
@@ -153,7 +156,7 @@ describe Gitlab::HttpIO do
           end
 
           it 'reads a trace' do
-            is_expected.to eq(remote_trace_body[0, length])
+            is_expected.to eq(file_body[0, length])
           end
         end
 
@@ -163,7 +166,7 @@ describe Gitlab::HttpIO do
           end
 
           it 'reads a trace' do
-            is_expected.to eq(remote_trace_body[0, length])
+            is_expected.to eq(file_body[0, length])
           end
         end
       end
@@ -177,7 +180,7 @@ describe Gitlab::HttpIO do
           end
 
           it 'reads a trace' do
-            is_expected.to eq(remote_trace_body)
+            is_expected.to eq(file_body)
           end
         end
 
@@ -187,7 +190,7 @@ describe Gitlab::HttpIO do
           end
 
           it 'reads a trace' do
-            is_expected.to eq(remote_trace_body)
+            is_expected.to eq(file_body)
           end
         end
       end
@@ -221,11 +224,11 @@ describe Gitlab::HttpIO do
       let(:length) { nil }
 
       before do
-        stub_remote_trace_500
+        stub_remote_url_500(url)
       end
 
       it 'reads a trace' do
-        expect { subject }.to raise_error(Gitlab::Ci::Trace::HttpIO::FailedToGetChunkError)
+        expect { subject }.to raise_error(Gitlab::HttpIO::FailedToGetChunkError)
       end
     end
   end
@@ -233,15 +236,15 @@ describe Gitlab::HttpIO do
   describe '#readline' do
     subject { http_io.readline }
 
-    let(:string_io) { StringIO.new(remote_trace_body) }
+    let(:string_io) { StringIO.new(file_body) }
 
     before do
-      stub_remote_trace_206
+      stub_remote_url_206(url, file_path)
     end
 
     shared_examples 'all line matching' do
       it 'reads a line' do
-        (0...remote_trace_body.lines.count).each do
+        (0...file_body.lines.count).each do
           expect(http_io.readline).to eq(string_io.readline)
         end
       end
@@ -251,11 +254,11 @@ describe Gitlab::HttpIO do
       let(:length) { nil }
 
       before do
-        stub_remote_trace_500
+        stub_remote_url_500(url)
       end
 
       it 'reads a trace' do
-        expect { subject }.to raise_error(Gitlab::Ci::Trace::HttpIO::FailedToGetChunkError)
+        expect { subject }.to raise_error(Gitlab::HttpIO::FailedToGetChunkError)
       end
     end
 

spec/models/ci/build_spec.rb

@@ -2687,4 +2687,58 @@ describe Ci::Build do
       end
     end
   end
+
+  describe '#artifacts_metadata_entry' do
+    set(:build) { create(:ci_build, project: project) }
+    let(:path) { 'other_artifacts_0.1.2/another-subdirectory/banana_sample.gif' }
+
+    before do
+      stub_artifacts_object_storage
+    end
+
+    subject { build.artifacts_metadata_entry(path) }
+
+    context 'when using local storage' do
+      let!(:metadata) { create(:ci_job_artifact, :metadata, job: build) }
+
+      context 'for existing file' do
+        it 'does exist' do
+          is_expected.to be_exists
+        end
+      end
+
+      context 'for non-existing file' do
+        let(:path) { 'invalid-file' }
+
+        it 'does not exist' do
+          is_expected.not_to be_exists
+        end
+      end
+    end
+
+    context 'when using remote storage' do
+      include HttpIOHelpers
+
+      let!(:metadata) { create(:ci_job_artifact, :remote_store, :metadata, job: build) }
+      let(:file_path) { expand_fixture_path('ci_build_artifacts_metadata.gz') }
+
+      before do
+        stub_remote_url_206(metadata.file.url, file_path)
+      end
+
+      context 'for existing file' do
+        it 'does exist' do
+          is_expected.to be_exists
+        end
+      end
+
+      context 'for non-existing file' do
+        let(:path) { 'invalid-file' }
+
+        it 'does not exist' do
+          is_expected.not_to be_exists
+        end
+      end
+    end
+  end
 end

spec/requests/api/jobs_spec.rb

@@ -535,12 +535,14 @@ describe API::Jobs do
     context 'authorized user' do
       context 'when trace is in ObjectStorage' do
         let!(:job) { create(:ci_build, :trace_artifact, pipeline: pipeline) }
+        let(:url) { 'http://object-storage/trace' }
+        let(:file) { expand_fixture_path('trace/sample_trace') }
 
         before do
-          stub_remote_trace_206
+          stub_remote_url_206(url, file_path)
           allow_any_instance_of(JobArtifactUploader).to receive(:file_storage?) { false }
-          allow_any_instance_of(JobArtifactUploader).to receive(:url) { remote_trace_url }
-          allow_any_instance_of(JobArtifactUploader).to receive(:size) { remote_trace_size }
+          allow_any_instance_of(JobArtifactUploader).to receive(:url) { url }
+          allow_any_instance_of(JobArtifactUploader).to receive(:size) { File.size(file) }
         end
 
         it 'returns specific job trace' do

spec/support/http_io/http_io_helpers.rb

 module HttpIOHelpers
-  def stub_remote_trace_206
-    WebMock.stub_request(:get, remote_trace_url)
-      .to_return { |request| remote_trace_response(request, 206) }
+  def stub_remote_url_206(url, file_path)
+    WebMock.stub_request(:get, url)
+      .to_return { |request| remote_url_response(file_path, request, 206) }
   end
 
-  def stub_remote_trace_200
-    WebMock.stub_request(:get, remote_trace_url)
-      .to_return { |request| remote_trace_response(request, 200) }
+  def stub_remote_url_200(url, file_path)
+    WebMock.stub_request(:get, url)
+      .to_return { |request| remote_url_response(file_path, request, 200) }
   end
 
-  def stub_remote_trace_500
-    WebMock.stub_request(:get, remote_trace_url)
+  def stub_remote_url_500(url)
+    WebMock.stub_request(:get, url)
       .to_return(status: [500, "Internal Server Error"])
   end
 
-  def remote_trace_url
-    "http://trace.com/trace"
-  end
-
-  def remote_trace_response(request, responce_status)
+  def remote_url_response(file_path, request, response_status)
     range = request.headers['Range'].match(/bytes=(\d+)-(\d+)/)
 
+    body = File.read(file_path).force_encoding(Encoding::BINARY)
+    size = body.bytesize
+
     {
-      status: responce_status,
-      headers: remote_trace_response_headers(responce_status, range[1].to_i, range[2].to_i),
-      body: range_trace_body(range[1].to_i, range[2].to_i)
+      status: response_status,
+      headers: remote_url_response_headers(response_status, range[1].to_i, range[2].to_i, size),
+      body: body[range[1].to_i..range[2].to_i]
     }
   end
 
-  def remote_trace_response_headers(responce_status, from, to)
-    headers = { 'Content-Type' => 'text/plain' }
-
-    if responce_status == 206
-      headers.merge('Content-Range' => "bytes #{from}-#{to}/#{remote_trace_size}")
-    end
-
-    headers
+  def remote_url_response_headers(response_status, from, to, size)
+    { 'Content-Type' => 'text/plain' }.tap do |headers|
+      if response_status == 206
+        headers.merge('Content-Range' => "bytes #{from}-#{to}/#{size}")
       end
-
-  def range_trace_body(from, to)
-    remote_trace_body[from..to]
     end
-
-  def remote_trace_body
-    @remote_trace_body ||= File.read(expand_fixture_path('trace/sample_trace'))
-      .force_encoding(Encoding::BINARY)
-  end
-
-  def remote_trace_size
-    remote_trace_body.bytesize
   end
 
   def set_smaller_buffer_size_than(file_size)

spec/uploaders/gitlab_uploader_spec.rb

@@ -68,4 +68,66 @@ describe GitlabUploader do
       expect(subject.file.path).to match(/#{subject.cache_dir}/)
     end
   end
+
+  describe '#open' do
+    context 'when trace is stored in File storage' do
+      context 'when file exists' do
+        let(:file) do
+          fixture_file_upload('spec/fixtures/trace/sample_trace', 'text/plain')
+        end
+
+        before do
+          subject.store!(file)
+        end
+
+        it 'returns io stream' do
+          expect(subject.open).to be_a(IO)
+        end
+
+        it 'when passing block it yields' do
+          expect { |b| subject.open(&b) }.to yield_control
+        end
+      end
+
+      context 'when file does not exist' do
+        it 'returns nil' do
+          expect(subject.open).to be_nil
+        end
+
+        it 'when passing block it does not yield' do
+          expect { |b| subject.open(&b) }.not_to yield_control
+        end
+      end
+    end
+
+    context 'when trace is stored in Object storage' do
+      before do
+        allow(subject).to receive(:file_storage?) { false }
+      end
+
+      context 'when file exists' do
+        before do
+          allow(subject).to receive(:url) { 'http://object_storage.com/trace' }
+        end
+
+        it 'returns http io stream' do
+          expect(subject.open).to be_a(Gitlab::HttpIO)
+        end
+
+        it 'when passing block it yields' do
+          expect { |b| subject.open(&b) }.to yield_control.once
+        end
+      end
+
+      context 'when file does not exist' do
+        it 'returns nil' do
+          expect(subject.open).to be_nil
+        end
+
+        it 'when passing block it does not yield' do
+          expect { |b| subject.open(&b) }.not_to yield_control
+        end
+      end
+    end
+  end
 end

spec/uploaders/job_artifact_uploader_spec.rb

@@ -23,43 +23,6 @@ describe JobArtifactUploader do
                     store_dir: %r[\h{2}/\h{2}/\h{64}/\d{4}_\d{1,2}_\d{1,2}/\d+/\d+\z]
   end
 
-  describe '#open' do
-    subject { uploader.open }
-
-    context 'when trace is stored in File storage' do
-      context 'when file exists' do
-        let(:file) do
-          fixture_file_upload('spec/fixtures/trace/sample_trace', 'text/plain')
-        end
-
-        before do
-          uploader.store!(file)
-        end
-
-        it 'returns io stream' do
-          is_expected.to be_a(IO)
-        end
-      end
-
-      context 'when file does not exist' do
-        it 'returns nil' do
-          is_expected.to be_nil
-        end
-      end
-    end
-
-    context 'when trace is stored in Object storage' do
-      before do
-        allow(uploader).to receive(:file_storage?) { false }
-        allow(uploader).to receive(:url) { 'http://object_storage.com/trace' }
-      end
-
-      it 'returns http io stream' do
-        is_expected.to be_a(Gitlab::HttpIO)
-      end
-    end
-  end
-
   context 'file is stored in valid local_path' do
     let(:file) do
       fixture_file_upload('spec/fixtures/ci_build_artifacts.zip', 'application/zip')