Merge branch 'dz-refactor-specific-build-artifacts' into 'master'

Refactor specific build artifacts code

See merge request gitlab-org/gitlab-ee!3689

ee/app/models/ee/ci/build.rb

@@ -7,6 +7,10 @@ module EE
     module Build
       extend ActiveSupport::Concern
 
+      CODEQUALITY_FILE = 'codeclimate.json'.freeze
+      SAST_FILE = 'gl-sast-report.json'.freeze
+      PERFORMANCE_FILE = 'performance.json'.freeze
+
       included do
         scope :codequality, ->() { where(name: %w[codequality codeclimate]) }
         scope :performance, ->() { where(name: %w[performance deploy]) }
@@ -27,17 +31,21 @@ module EE
       end
 
       def has_codeclimate_json?
-        options.dig(:artifacts, :paths) == ['codeclimate.json'] &&
-          artifacts_metadata?
+        has_artifact?(CODEQUALITY_FILE)
       end
 
       def has_performance_json?
-        options.dig(:artifacts, :paths) == ['performance.json'] &&
-          artifacts_metadata?
+        has_artifact?(PERFORMANCE_FILE)
       end
 
       def has_sast_json?
-        options.dig(:artifacts, :paths) == ['gl-sast-report.json'] &&
+        has_artifact?(SAST_FILE)
+      end
+
+      private
+
+      def has_artifact?(name)
+        options.dig(:artifacts, :paths) == [name] &&
           artifacts_metadata?
       end
     end

ee/app/serializers/ee/merge_request_entity.rb

@@ -7,7 +7,7 @@ module EE
         expose :head_path, if: -> (mr, _) { can?(current_user, :read_build, mr.head_codeclimate_artifact) } do |merge_request|
           raw_project_build_artifacts_url(merge_request.source_project,
                                           merge_request.head_codeclimate_artifact,
-                                          path: 'codeclimate.json')
+                                          path: Ci::Build::CODEQUALITY_FILE)
         end
 
         expose :head_blob_path, if: -> (mr, _) { mr.head_pipeline_sha } do |merge_request|
@@ -17,7 +17,7 @@ module EE
         expose :base_path, if: -> (mr, _) { can?(current_user, :read_build, mr.base_codeclimate_artifact) } do |merge_request|
           raw_project_build_artifacts_url(merge_request.target_project,
                                           merge_request.base_codeclimate_artifact,
-                                          path: 'codeclimate.json')
+                                          path: Ci::Build::CODEQUALITY_FILE)
         end
 
         expose :base_blob_path, if: -> (mr, _) { mr.base_pipeline_sha } do |merge_request|
@@ -29,13 +29,13 @@ module EE
         expose :head_path, if: -> (mr, _) { can?(current_user, :read_build, mr.head_performance_artifact) } do |merge_request|
           raw_project_build_artifacts_url(merge_request.source_project,
                                           merge_request.head_performance_artifact,
-                                          path: 'performance.json')
+                                          path: Ci::Build::PERFORMANCE_FILE)
         end
 
         expose :base_path, if: -> (mr, _) { can?(current_user, :read_build, mr.base_performance_artifact) } do |merge_request|
           raw_project_build_artifacts_url(merge_request.target_project,
                                           merge_request.base_performance_artifact,
-                                          path: 'performance.json')
+                                          path: Ci::Build::PERFORMANCE_FILE)
         end
       end
 
@@ -43,7 +43,7 @@ module EE
         expose :path do |merge_request|
           raw_project_build_artifacts_url(merge_request.source_project,
                                           merge_request.sast_artifact,
-                                          path: 'gl-sast-report.json')
+                                          path: Ci::Build::SAST_FILE)
         end
 
         expose :blob_path, if: -> (mr, _) { mr.head_pipeline_sha } do |merge_request|

spec/ee/spec/models/ee/ci/build_spec.rb

@@ -128,105 +128,43 @@ describe Ci::Build do
     end
   end
 
-  describe '#has_codeclimate_json?' do
-    context 'valid build' do
-      let!(:build) do
-        create(
-          :ci_build,
-          :artifacts,
-          name: 'codequality',
-          pipeline: pipeline,
-          options: {
-            artifacts: {
-              paths: ['codeclimate.json']
+  ARTIFACTS_METHODS = {
+    has_codeclimate_json?: Ci::Build::CODEQUALITY_FILE,
+    has_performance_json?: Ci::Build::PERFORMANCE_FILE,
+    has_sast_json?: Ci::Build::SAST_FILE
+  }.freeze
+
+  ARTIFACTS_METHODS.each do |method, filename|
+    describe "##{method}" do
+      context 'valid build' do
+        let!(:build) do
+          create(
+            :ci_build,
+            :artifacts,
+            pipeline: pipeline,
+            options: {
+              artifacts: {
+                paths: [filename]
+              }
             }
-          }
-        )
-      end
-
-      it { expect(build.has_codeclimate_json?).to be_truthy }
-    end
-
-    context 'invalid build' do
-      let!(:build) do
-        create(
-          :ci_build,
-          :artifacts,
-          name: 'codequality',
-          pipeline: pipeline,
-          options: {}
-        )
-      end
-
-      it { expect(build.has_codeclimate_json?).to be_falsey }
-    end
-  end
-
-  describe '#has_performance_json?' do
-    context 'valid build' do
-      let!(:build) do
-        create(
-          :ci_build,
-          :artifacts,
-          name: 'performance',
-          pipeline: pipeline,
-          options: {
-            artifacts: {
-              paths: ['performance.json']
-            }
-          }
-        )
-      end
-
-      it { expect(build.has_performance_json?).to be_truthy }
-    end
-
-    context 'invalid build' do
-      let!(:build) do
-        create(
-          :ci_build,
-          :artifacts,
-          name: 'performance',
-          pipeline: pipeline,
-          options: {}
-        )
-      end
-
-      it { expect(build.has_performance_json?).to be_falsey }
-    end
-  end
+          )
+        end
 
-  describe '#has_sast_json?' do
-    context 'valid build' do
-      let!(:build) do
-        create(
-          :ci_build,
-          :artifacts,
-          name: 'sast',
-          pipeline: pipeline,
-          options: {
-            artifacts: {
-              paths: ['gl-sast-report.json']
-            }
-          }
-        )
+        it { expect(build.send(method)).to be_truthy }
       end
 
-      it { expect(build.has_sast_json?).to be_truthy }
-    end
+      context 'invalid build' do
+        let!(:build) do
+          create(
+            :ci_build,
+            :artifacts,
+            pipeline: pipeline,
+            options: {}
+          )
+        end
 
-    context 'invalid build' do
-      let!(:build) do
-        create(
-          :ci_build,
-          :artifacts,
-          name: 'sast',
-          pipeline: pipeline,
-          options: {}
-        )
+        it { expect(build.send(method)).to be_falsey }
       end
-
-      it { expect(build.has_sast_json?).to be_falsey }
     end
   end
 end