Merge branch 'mo-move-each-blob-to-artifactable' into 'master'

Extract each_blob to Artifactable

See merge request gitlab-org/gitlab!40392

app/models/ci/job_artifact.rb

@@ -12,8 +12,6 @@ module Ci
     include FileStoreMounter
     extend Gitlab::Ci::Model
 
-    NotSupportedAdapterError = Class.new(StandardError)
-
     ignore_columns :locked, remove_after: '2020-07-22', remove_with: '13.4'
 
     TEST_REPORT_FILE_TYPES = %w[junit].freeze
@@ -271,16 +269,6 @@ module Ci
         end
     end
 
-    def each_blob(&blk)
-      unless file_format_adapter_class
-        raise NotSupportedAdapterError, 'This file format requires a dedicated adapter'
-      end
-
-      file.open do |stream|
-        file_format_adapter_class.new(stream).each_blob(&blk)
-      end
-    end
-
     def self.archived_trace_exists_for?(job_id)
       where(job_id: job_id).trace.take&.file&.file&.exists?
     end
@@ -298,10 +286,6 @@ module Ci
 
     private
 
-    def file_format_adapter_class
-      FILE_FORMAT_ADAPTERS[file_format.to_sym]
-    end
-
     def set_size
       self.size = file.size
     end

app/models/concerns/ci/artifactable.rb

@@ -4,6 +4,8 @@ module Ci
   module Artifactable
     extend ActiveSupport::Concern
 
+    NotSupportedAdapterError = Class.new(StandardError)
+
     FILE_FORMAT_ADAPTERS = {
       gzip: Gitlab::Ci::Build::Artifacts::Adapters::GzipStream,
       raw: Gitlab::Ci::Build::Artifacts::Adapters::RawStream
@@ -16,5 +18,21 @@ module Ci
         gzip: 3
       }, _suffix: true
     end
+
+    def each_blob(&blk)
+      unless file_format_adapter_class
+        raise NotSupportedAdapterError, 'This file format requires a dedicated adapter'
+      end
+
+      file.open do |stream|
+        file_format_adapter_class.new(stream).each_blob(&blk)
+      end
+    end
+
+    private
+
+    def file_format_adapter_class
+      FILE_FORMAT_ADAPTERS[file_format.to_sym]
+    end
   end
 end

lib/gitlab/ci/pipeline/artifact/code_coverage.rb

@@ -5,6 +5,8 @@ module Gitlab
     module Pipeline
       module Artifact
         class CodeCoverage
+          include Gitlab::Utils::StrongMemoize
+
           def initialize(pipeline_artifact)
             @pipeline_artifact = pipeline_artifact
           end
@@ -18,7 +20,11 @@ module Gitlab
           private
 
           def raw_report
-            @raw_report ||= Gitlab::Json.parse(@pipeline_artifact.file.read)
+            strong_memoize(:raw_report) do
+              @pipeline_artifact.each_blob do |blob|
+                Gitlab::Json.parse(blob)
+              end
+            end
           end
         end
       end

spec/models/ci/job_artifact_spec.rb

@@ -343,42 +343,6 @@ RSpec.describe Ci::JobArtifact do
     end
   end
 
-  describe '#each_blob' do
-    context 'when file format is gzip' do
-      context 'when gzip file contains one file' do
-        let(:artifact) { build(:ci_job_artifact, :junit) }
-
-        it 'iterates blob once' do
-          expect { |b| artifact.each_blob(&b) }.to yield_control.once
-        end
-      end
-
-      context 'when gzip file contains three files' do
-        let(:artifact) { build(:ci_job_artifact, :junit_with_three_testsuites) }
-
-        it 'iterates blob three times' do
-          expect { |b| artifact.each_blob(&b) }.to yield_control.exactly(3).times
-        end
-      end
-    end
-
-    context 'when file format is raw' do
-      let(:artifact) { build(:ci_job_artifact, :codequality, file_format: :raw) }
-
-      it 'iterates blob once' do
-        expect { |b| artifact.each_blob(&b) }.to yield_control.once
-      end
-    end
-
-    context 'when there are no adapters for the file format' do
-      let(:artifact) { build(:ci_job_artifact, :junit, file_format: :zip) }
-
-      it 'raises an error' do
-        expect { |b| artifact.each_blob(&b) }.to raise_error(described_class::NotSupportedAdapterError)
-      end
-    end
-  end
-
   describe 'expired?' do
     subject { artifact.expired? }
 

spec/models/concerns/ci/artifactable_spec.rb

@@ -18,4 +18,40 @@ RSpec.describe Ci::Artifactable do
       it { is_expected.to be_const_defined(:FILE_FORMAT_ADAPTERS) }
     end
   end
+
+  describe '#each_blob' do
+    context 'when file format is gzip' do
+      context 'when gzip file contains one file' do
+        let(:artifact) { build(:ci_job_artifact, :junit) }
+
+        it 'iterates blob once' do
+          expect { |b| artifact.each_blob(&b) }.to yield_control.once
+        end
+      end
+
+      context 'when gzip file contains three files' do
+        let(:artifact) { build(:ci_job_artifact, :junit_with_three_testsuites) }
+
+        it 'iterates blob three times' do
+          expect { |b| artifact.each_blob(&b) }.to yield_control.exactly(3).times
+        end
+      end
+    end
+
+    context 'when file format is raw' do
+      let(:artifact) { build(:ci_job_artifact, :codequality, file_format: :raw) }
+
+      it 'iterates blob once' do
+        expect { |b| artifact.each_blob(&b) }.to yield_control.once
+      end
+    end
+
+    context 'when there are no adapters for the file format' do
+      let(:artifact) { build(:ci_job_artifact, :junit, file_format: :zip) }
+
+      it 'raises an error' do
+        expect { |b| artifact.each_blob(&b) }.to raise_error(described_class::NotSupportedAdapterError)
+      end
+    end
+  end
 end