Merge branch 'mo-add-pipeline-artifact-service' into 'master'

Add CreateArtifactService to persist code_coverage report

See merge request gitlab-org/gitlab!39236

app/models/ci/pipeline.rb

@@ -228,6 +228,12 @@ module Ci
         end
       end
 
+      after_transition any => ::Ci::Pipeline.completed_statuses do |pipeline|
+        pipeline.run_after_commit do
+          ::Ci::Pipelines::CreateArtifactWorker.perform_async(pipeline.id)
+        end
+      end
+
       after_transition any => ::Ci::Pipeline.completed_statuses do |pipeline|
         next unless pipeline.bridge_triggered?
         next unless pipeline.bridge_waiting?
@@ -856,6 +862,10 @@ module Ci
       complete? && latest_report_builds(reports_scope).exists?
     end
 
+    def has_coverage_reports?
+      self.has_reports?(Ci::JobArtifact.coverage_reports)
+    end
+
     def test_report_summary
       Gitlab::Ci::Reports::TestReportSummary.new(latest_builds_report_results)
     end

app/models/ci/pipeline_artifact.rb

@@ -14,6 +14,11 @@ module Ci
     ].freeze
 
     FILE_SIZE_LIMIT = 10.megabytes.freeze
+    EXPIRATION_DATE = 1.week.freeze
+
+    DEFAULT_FILE_NAMES = {
+      code_coverage: 'code_coverage.json'
+    }.freeze
 
     belongs_to :project, class_name: "Project", inverse_of: :pipeline_artifacts
     belongs_to :pipeline, class_name: "Ci::Pipeline", inverse_of: :pipeline_artifacts
@@ -24,14 +29,13 @@ module Ci
     validates :file_type, presence: true
 
     mount_file_store_uploader Ci::PipelineArtifactUploader
-    before_save :set_size, if: :file_changed?
 
     enum file_type: {
       code_coverage: 1
     }
 
-    def set_size
-      self.size = file.size
+    def self.has_code_coverage?
+      where(file_type: :code_coverage).exists?
     end
   end
 end

app/services/ci/pipelines/create_artifact_service.rb

+# frozen_string_literal: true
+module Ci
+  module Pipelines
+    class CreateArtifactService
+      def execute(pipeline)
+        return unless ::Gitlab::Ci::Features.coverage_report_view?(pipeline.project)
+        return unless pipeline.has_coverage_reports?
+        return if pipeline.pipeline_artifacts.has_code_coverage?
+
+        file = build_carrierwave_file(pipeline)
+
+        pipeline.pipeline_artifacts.create!(
+          project_id: pipeline.project_id,
+          file_type: :code_coverage,
+          file_format: :raw,
+          size: file["tempfile"].size,
+          file: file,
+          expire_at: Ci::PipelineArtifact::EXPIRATION_DATE.from_now
+        )
+      end
+
+      private
+
+      def build_carrierwave_file(pipeline)
+        CarrierWaveStringFile.new_file(
+          file_content: pipeline.coverage_reports.to_json,
+          filename: Ci::PipelineArtifact::DEFAULT_FILE_NAMES.fetch(:code_coverage),
+          content_type: 'application/json'
+        )
+      end
+    end
+  end
+end

app/workers/all_queues.yml

@@ -899,6 +899,14 @@
   :weight: 1
   :idempotent: true
   :tags: []
+- :name: pipeline_background:ci_pipelines_create_artifact
+  :feature_category: :continuous_integration
+  :has_external_dependencies:
+  :urgency: :low
+  :resource_boundary: :unknown
+  :weight: 1
+  :idempotent: true
+  :tags: []
 - :name: pipeline_background:ci_ref_delete_unlock_artifacts
   :feature_category: :continuous_integration
   :has_external_dependencies:

app/workers/ci/pipelines/create_artifact_worker.rb

+# frozen_string_literal: true
+
+module Ci
+  module Pipelines
+    class CreateArtifactWorker
+      include ApplicationWorker
+      include PipelineBackgroundQueue
+
+      idempotent!
+
+      def perform(pipeline_id)
+        Ci::Pipeline.find_by_id(pipeline_id).try do |pipeline|
+          Ci::Pipelines::CreateArtifactService.new.execute(pipeline)
+        end
+      end
+    end
+  end
+end

lib/carrier_wave_string_file.rb

@@ -4,4 +4,12 @@ class CarrierWaveStringFile < StringIO
   def original_filename
     ""
   end
+
+  def self.new_file(file_content:, filename:, content_type: "application/octet-stream")
+    {
+      "tempfile" => StringIO.new(file_content),
+      "filename" => filename,
+      "content_type" => content_type
+    }
+  end
 end

lib/gitlab/ci/features.rb

@@ -83,6 +83,10 @@ module Gitlab
       def self.project_transactionless_destroy?(project)
         Feature.enabled?(:project_transactionless_destroy, project, default_enabled: false)
       end
+
+      def self.coverage_report_view?(project)
+        ::Feature.enabled?(:coverage_report_view, project)
+      end
     end
   end
 end

spec/factories/ci/pipeline_artifacts.rb

@@ -13,5 +13,16 @@ FactoryBot.define do
       artifact.file = fixture_file_upload(
         Rails.root.join('spec/fixtures/pipeline_artifacts/code_coverage.json'), 'application/json')
     end
+
+    trait :with_multibyte_characters do
+      size { { "utf8" => "✓" }.to_json.size }
+      after(:build) do |artifact, _evaluator|
+        artifact.file = CarrierWaveStringFile.new_file(
+          file_content: { "utf8" => "✓" }.to_json,
+          filename: 'filename',
+          content_type: 'application/json'
+        )
+      end
+    end
   end
 end

spec/models/ci/pipeline_artifact_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 RSpec.describe Ci::PipelineArtifact, type: :model do
-  let_it_be(:coverage_report) { create(:ci_pipeline_artifact) }
+  let(:coverage_report) { create(:ci_pipeline_artifact) }
 
   describe 'associations' do
     it { is_expected.to belong_to(:pipeline) }
@@ -44,24 +44,6 @@ RSpec.describe Ci::PipelineArtifact, type: :model do
     end
   end
 
-  describe '#set_size' do
-    subject { create(:ci_pipeline_artifact) }
-
-    context 'when file is being created' do
-      it 'sets the size' do
-        expect(subject.size).to eq(85)
-      end
-    end
-
-    context 'when file is being updated' do
-      it 'updates the size' do
-        subject.update!(file: fixture_file_upload('spec/fixtures/dk.png'))
-
-        expect(subject.size).to eq(1062)
-      end
-    end
-  end
-
   describe 'file is being stored' do
     subject { create(:ci_pipeline_artifact) }
 
@@ -78,5 +60,31 @@ RSpec.describe Ci::PipelineArtifact, type: :model do
         it_behaves_like 'mounted file in object store'
       end
     end
+
+    context 'when file contains multi-byte characters' do
+      let(:coverage_report_multibyte) { create(:ci_pipeline_artifact, :with_multibyte_characters) }
+
+      it 'sets the size in bytesize' do
+        expect(coverage_report_multibyte.size).to eq(12)
+      end
+    end
+  end
+
+  describe '.has_code_coverage?' do
+    subject { Ci::PipelineArtifact.has_code_coverage? }
+
+    context 'when pipeline artifact has a code coverage' do
+      let!(:pipeline_artifact) { create(:ci_pipeline_artifact) }
+
+      it 'returns true' do
+        expect(subject).to be_truthy
+      end
+    end
+
+    context 'when pipeline artifact does not have a code coverage' do
+      it 'returns false' do
+        expect(subject).to be_falsey
+      end
+    end
   end
 end

spec/models/ci/pipeline_spec.rb

@@ -2948,6 +2948,38 @@ RSpec.describe Ci::Pipeline, :mailer do
     end
   end
 
+  describe '#has_coverage_reports?' do
+    subject { pipeline.has_coverage_reports? }
+
+    context 'when pipeline has builds with coverage reports' do
+      before do
+        create(:ci_build, :coverage_reports, pipeline: pipeline, project: project)
+      end
+
+      context 'when pipeline status is running' do
+        let(:pipeline) { create(:ci_pipeline, :running, project: project) }
+
+        it { expect(subject).to be_falsey }
+      end
+
+      context 'when pipeline status is success' do
+        let(:pipeline) { create(:ci_pipeline, :success, project: project) }
+
+        it { expect(subject).to be_truthy }
+      end
+    end
+
+    context 'when pipeline does not have builds with coverage reports' do
+      before do
+        create(:ci_build, :artifacts, pipeline: pipeline, project: project)
+      end
+
+      let(:pipeline) { create(:ci_pipeline, :success, project: project) }
+
+      it { expect(subject).to be_falsey }
+    end
+  end
+
   describe '#test_report_summary' do
     subject { pipeline.test_report_summary }
 

spec/services/ci/pipelines/create_artifact_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ::Ci::Pipelines::CreateArtifactService do
+  describe '#execute' do
+    subject { described_class.new.execute(pipeline) }
+
+    context 'when pipeline has coverage reports' do
+      let(:pipeline) { create(:ci_pipeline, :with_coverage_reports) }
+
+      context 'when pipeline is finished' do
+        it 'creates a pipeline artifact' do
+          subject
+
+          expect(Ci::PipelineArtifact.count).to eq(1)
+        end
+
+        it 'persists the default file name' do
+          subject
+
+          file = Ci::PipelineArtifact.first.file
+
+          expect(file.filename).to eq('code_coverage.json')
+        end
+
+        it 'sets expire_at to 1 week' do
+          freeze_time do
+            subject
+
+            pipeline_artifact = Ci::PipelineArtifact.first
+
+            expect(pipeline_artifact.expire_at).to eq(1.week.from_now)
+          end
+        end
+      end
+
+      context 'when feature is disabled' do
+        it 'does not create a pipeline artifact' do
+          stub_feature_flags(coverage_report_view: false)
+
+          subject
+
+          expect(Ci::PipelineArtifact.count).to eq(0)
+        end
+      end
+
+      context 'when pipeline artifact has already been created' do
+        it 'do not raise an error and do not persist the same artifact twice' do
+          expect { 2.times { described_class.new.execute(pipeline) } }.not_to raise_error(ActiveRecord::RecordNotUnique)
+
+          expect(Ci::PipelineArtifact.count).to eq(1)
+        end
+      end
+    end
+
+    context 'when pipeline is running and coverage report does not exist' do
+      let(:pipeline) { create(:ci_pipeline, :running) }
+
+      it 'does not persist data' do
+        subject
+
+        expect(Ci::PipelineArtifact.count).to eq(0)
+      end
+    end
+  end
+end

spec/workers/ci/pipelines/create_artifact_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ::Ci::Pipelines::CreateArtifactWorker do
+  describe '#perform' do
+    subject { described_class.new.perform(pipeline_id) }
+
+    context 'when pipeline exists' do
+      let(:pipeline) { create(:ci_pipeline) }
+      let(:pipeline_id) { pipeline.id }
+
+      it 'calls pipeline report result service' do
+        expect_next_instance_of(::Ci::Pipelines::CreateArtifactService) do |create_artifact_service|
+          expect(create_artifact_service).to receive(:execute)
+        end
+
+        subject
+      end
+    end
+
+    context 'when pipeline does not exist' do
+      let(:pipeline_id) { non_existing_record_id }
+
+      it 'does not call pipeline create artifact service' do
+        expect(Ci::Pipelines::CreateArtifactService).not_to receive(:execute)
+
+        subject
+      end
+    end
+  end
+end