Merge branch '7125-refactor-code-quality-similar-to-junit-tests-ce' into 'master'

CE Resolve "Refactor code quality similar to JUnit tests"

See merge request gitlab-org/gitlab-ce!21932

app/controllers/projects/artifacts_controller.rb

@@ -14,6 +14,8 @@ class Projects::ArtifactsController < Projects::ApplicationController
   before_action :entry, only: [:file]
 
   def download
+    return render_404 unless artifacts_file
+
     send_upload(artifacts_file, attachment: artifacts_file.filename)
   end
 
@@ -100,7 +102,7 @@ class Projects::ArtifactsController < Projects::ApplicationController
   # rubocop: enable CodeReuse/ActiveRecord
 
   def artifacts_file
-    @artifacts_file ||= build.artifacts_file
+    @artifacts_file ||= build.artifacts_file_for_type(params[:file_type] || :archive)
   end
 
   def entry

app/models/ci/build.rb

@@ -522,6 +522,13 @@ module Ci
       self.job_artifacts.update_all(expire_at: nil)
     end
 
+    def artifacts_file_for_type(type)
+      file = job_artifacts.find_by(file_type: Ci::JobArtifact.file_types[type])&.file
+      # TODO: to be removed once legacy artifacts is removed
+      file ||= legacy_artifacts_file if type == :archive
+      file
+    end
+
     def coverage_regex
       super || project.try(:build_coverage_regex)
     end

app/models/ci/job_artifact.rb

@@ -15,6 +15,7 @@ module Ci
       metadata: nil,
       trace: nil,
       junit: 'junit.xml',
+      codequality: 'codequality.json',
       sast: 'gl-sast-report.json',
       dependency_scanning: 'gl-dependency-scanning-report.json',
       container_scanning: 'gl-container-scanning-report.json',
@@ -26,6 +27,7 @@ module Ci
       metadata: :gzip,
       trace: :raw,
       junit: :gzip,
+      codequality: :gzip,
       sast: :gzip,
       dependency_scanning: :gzip,
       container_scanning: :gzip,
@@ -73,7 +75,8 @@ module Ci
       sast: 5, ## EE-specific
       dependency_scanning: 6, ## EE-specific
       container_scanning: 7, ## EE-specific
-      dast: 8 ## EE-specific
+      dast: 8, ## EE-specific
+      codequality: 9 ## EE-specific
     }
 
     enum file_format: {

lib/gitlab/ci/config/entry/reports.rb

@@ -11,7 +11,7 @@ module Gitlab
           include Validatable
           include Attributable
 
-          ALLOWED_KEYS = %i[junit sast dependency_scanning container_scanning dast].freeze
+          ALLOWED_KEYS = %i[junit codequality sast dependency_scanning container_scanning dast].freeze
 
           attributes ALLOWED_KEYS
 
@@ -21,6 +21,7 @@ module Gitlab
 
             with_options allow_nil: true do
               validates :junit, array_of_strings_or_string: true
+              validates :codequality, array_of_strings_or_string: true
               validates :sast, array_of_strings_or_string: true
               validates :dependency_scanning, array_of_strings_or_string: true
               validates :container_scanning, array_of_strings_or_string: true

spec/controllers/projects/artifacts_controller_spec.rb

@@ -19,10 +19,42 @@ describe Projects::ArtifactsController do
   end
 
   describe 'GET download' do
+    subject { get :download, namespace_id: project.namespace, project_id: project, job_id: job, file_type: file_type }
+
+    context 'when no file type is supplied' do
+      let(:file_type) { nil }
+
       it 'sends the artifacts file' do
         expect(controller).to receive(:send_file).with(job.artifacts_file.path, hash_including(disposition: 'attachment')).and_call_original
 
-      get :download, namespace_id: project.namespace, project_id: project, job_id: job
+        subject
+      end
+    end
+
+    context 'when a file type is supplied' do
+      context 'when an invalid file type is supplied' do
+        let(:file_type) { 'invalid' }
+
+        it 'returns 404' do
+          subject
+
+          expect(response).to have_gitlab_http_status(404)
+        end
+      end
+
+      context 'when codequality file type is supplied' do
+        let(:file_type) { 'codequality' }
+
+        before do
+          create(:ci_job_artifact, :codequality, job: job)
+        end
+
+        it 'sends the codequality report' do
+          expect(controller).to receive(:send_file).with(job.job_artifacts_codequality.file.path, hash_including(disposition: 'attachment')).and_call_original
+
+          subject
+        end
+      end
     end
   end
 

spec/factories/ci/job_artifacts.rb

@@ -117,6 +117,16 @@ FactoryBot.define do
       end
     end
 
+    trait :codequality do
+      file_type :codequality
+      file_format :gzip
+
+      after(:build) do |artifact, evaluator|
+        artifact.file = fixture_file_upload(
+          Rails.root.join('spec/fixtures/codequality/codequality.json.gz'), 'application/x-gzip')
+      end
+    end
+
     trait :correct_checksum do
       after(:build) do |artifact, evaluator|
         artifact.file_sha256 = Digest::SHA256.file(artifact.file.path).hexdigest

spec/lib/gitlab/ci/config/entry/reports_spec.rb

@@ -33,6 +33,7 @@ describe Gitlab::Ci::Config::Entry::Reports do
 
       where(:keyword, :file) do
         :junit | 'junit.xml'
+        :codequality | 'codequality.json'
         :sast | 'gl-sast-report.json'
         :dependency_scanning | 'gl-dependency-scanning-report.json'
         :container_scanning | 'gl-container-scanning-report.json'

spec/models/ci/build_spec.rb

@@ -1278,6 +1278,19 @@ describe Ci::Build do
     end
   end
 
+  describe '#artifacts_file_for_type' do
+    let(:build) { create(:ci_build, :artifacts) }
+    let(:file_type) { :archive }
+
+    subject { build.artifacts_file_for_type(file_type) }
+
+    it 'queries artifacts for type' do
+      expect(build).to receive_message_chain(:job_artifacts, :find_by).with(file_type: Ci::JobArtifact.file_types[file_type])
+
+      subject
+    end
+  end
+
   describe '#merge_request' do
     def create_mr(build, pipeline, factory: :merge_request, created_at: Time.now)
       create(factory, source_project: pipeline.project,

spec/models/ci/job_artifact_spec.rb

@@ -34,7 +34,7 @@ describe Ci::JobArtifact do
   describe '.erasable' do
     subject { described_class.erasable }
 
-    context 'when there is am erasable artifact' do
+    context 'when there is an erasable artifact' do
       let!(:artifact) { create(:ci_job_artifact, :junit) }
 
       it { is_expected.to eq([artifact]) }

spec/services/ci/retry_build_service_spec.rb

@@ -26,7 +26,8 @@ describe Ci::RetryBuildService do
        erased_at auto_canceled_by job_artifacts job_artifacts_archive
        job_artifacts_metadata job_artifacts_trace job_artifacts_junit
        job_artifacts_sast job_artifacts_dependency_scanning
-       job_artifacts_container_scanning job_artifacts_dast].freeze
+       job_artifacts_container_scanning job_artifacts_dast
+       job_artifacts_codequality].freeze
 
   IGNORE_ACCESSORS =
     %i[type lock_version target_url base_tags trace_sections