From 201dd19f6de266b3ae5663bc026cb763fc4f77ae Mon Sep 17 00:00:00 2001
From: Maxime Orefice <morefice@gitlab.com>
Date: Wed, 5 Aug 2020 16:13:19 -0400
Subject: [PATCH] Add pipeline create artifact service
This new service will be used to persist generated
coverage report once a pipeline has been completed.
---
app/models/ci/pipeline.rb | 10 +++
app/models/ci/pipeline_artifact.rb | 10 ++-
.../ci/pipelines/create_artifact_service.rb | 33 +++++++++
app/workers/all_queues.yml | 8 +++
.../ci/pipelines/create_artifact_worker.rb | 18 +++++
lib/carrier_wave_string_file.rb | 8 +++
lib/gitlab/ci/features.rb | 4 ++
spec/factories/ci/pipeline_artifacts.rb | 11 +++
spec/models/ci/pipeline_artifact_spec.rb | 46 +++++++------
spec/models/ci/pipeline_spec.rb | 32 +++++++++
.../pipelines/create_artifact_service_spec.rb | 67 +++++++++++++++++++
.../pipelines/create_artifact_worker_spec.rb | 32 +++++++++
12 files changed, 257 insertions(+), 22 deletions(-)
create mode 100644 app/services/ci/pipelines/create_artifact_service.rb
create mode 100644 app/workers/ci/pipelines/create_artifact_worker.rb
create mode 100644 spec/services/ci/pipelines/create_artifact_service_spec.rb
create mode 100644 spec/workers/ci/pipelines/create_artifact_worker_spec.rb
diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 27cc72dc1b3d..289417327e8c 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -228,6 +228,12 @@ class Pipeline < ApplicationRecord
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 @@ def has_reports?(reports_scope)
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
diff --git a/app/models/ci/pipeline_artifact.rb b/app/models/ci/pipeline_artifact.rb
index e7f51977ccd4..5a3bf52a43da 100644
--- a/app/models/ci/pipeline_artifact.rb
+++ b/app/models/ci/pipeline_artifact.rb
@@ -14,6 +14,11 @@ class PipelineArtifact < ApplicationRecord
].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 @@ class PipelineArtifact < ApplicationRecord
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
diff --git a/app/services/ci/pipelines/create_artifact_service.rb b/app/services/ci/pipelines/create_artifact_service.rb
new file mode 100644
index 000000000000..179e18f22e8c
--- /dev/null
+++ b/app/services/ci/pipelines/create_artifact_service.rb
@@ -0,0 +1,33 @@
+# 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
diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml
index 2c871c55f0ac..e16032cdc50f 100644
--- a/app/workers/all_queues.yml
+++ b/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:
diff --git a/app/workers/ci/pipelines/create_artifact_worker.rb b/app/workers/ci/pipelines/create_artifact_worker.rb
new file mode 100644
index 000000000000..220df975503a
--- /dev/null
+++ b/app/workers/ci/pipelines/create_artifact_worker.rb
@@ -0,0 +1,18 @@
+# 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
diff --git a/lib/carrier_wave_string_file.rb b/lib/carrier_wave_string_file.rb
index c9a64d9e6318..b6bc3d986ca9 100644
--- a/lib/carrier_wave_string_file.rb
+++ b/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
diff --git a/lib/gitlab/ci/features.rb b/lib/gitlab/ci/features.rb
index 934d1a4c9f10..895daf65d0d6 100644
--- a/lib/gitlab/ci/features.rb
+++ b/lib/gitlab/ci/features.rb
@@ -83,6 +83,10 @@ def self.expand_names_for_cross_pipeline_artifacts?(project)
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
diff --git a/spec/factories/ci/pipeline_artifacts.rb b/spec/factories/ci/pipeline_artifacts.rb
index ecfd1e79e789..e601b0bbf0e3 100644
--- a/spec/factories/ci/pipeline_artifacts.rb
+++ b/spec/factories/ci/pipeline_artifacts.rb
@@ -13,5 +13,16 @@
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
diff --git a/spec/models/ci/pipeline_artifact_spec.rb b/spec/models/ci/pipeline_artifact_spec.rb
index 9d63d74a6cc5..9d2172d75721 100644
--- a/spec/models/ci/pipeline_artifact_spec.rb
+++ b/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 @@
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 @@
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
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index b4e80fa75881..54dad2e18400 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -2948,6 +2948,38 @@ def create_build(name, stage_idx)
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 }
diff --git a/spec/services/ci/pipelines/create_artifact_service_spec.rb b/spec/services/ci/pipelines/create_artifact_service_spec.rb
new file mode 100644
index 000000000000..d5e9cf83a6d0
--- /dev/null
+++ b/spec/services/ci/pipelines/create_artifact_service_spec.rb
@@ -0,0 +1,67 @@
+# 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
diff --git a/spec/workers/ci/pipelines/create_artifact_worker_spec.rb b/spec/workers/ci/pipelines/create_artifact_worker_spec.rb
new file mode 100644
index 000000000000..31d2c4e9559b
--- /dev/null
+++ b/spec/workers/ci/pipelines/create_artifact_worker_spec.rb
@@ -0,0 +1,32 @@
+# 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
--
GitLab