Rename kwards as file_types & report_types

Using `file_types` in JobArtifact model and `report_type` in Pipeline as kwargs makes it easier to reason about.

Rename kwards as file_types & report_types
Using `file_types` in JobArtifact model and `report_type` in Pipeline as kwargs makes it easier to reason about.
814cb658 · Mehmet Emin INAC · Mayra Cabrera · d795ab07 · 814cb658 · 814cb658
--- a/ee/app/finders/security/pipeline_vulnerabilities_finder.rb
+++ b/ee/app/finders/security/pipeline_vulnerabilities_finder.rb
@@ -24,8 +24,6 @@ def initialize(pipeline:, params: {})
    end

    def execute
-      requested_reports = pipeline_reports.select { |report_type| requested_type?(report_type) }
-
      findings = requested_reports.each_with_object([]) do |(type, report), findings|
        raise ParseError, 'JSON parsing failed' if report.error.is_a?(Gitlab::Ci::Parsers::Security::Common::SecurityReportParserError)

@@ -54,8 +52,8 @@ def sort_findings(findings)
      Gitlab::Utils.stable_sort_by(findings) { |x| [-x.severity_value, -x.confidence_value] }
    end

-    def pipeline_reports
-      pipeline&.security_reports&.reports || {}
+    def requested_reports
+      @requested_reports ||= pipeline&.security_reports(report_types: report_types)&.reports || {}
    end

    def vulnerabilities_by_finding_fingerprint(report_type, report)
@@ -105,10 +103,6 @@ def filter(findings)
      end
    end

-    def requested_type?(type)
-      report_types.include?(type)
-    end
-
    def include_dismissed?
      params[:scope] == 'all'
    end

--- a/ee/app/models/ee/ci/job_artifact.rb
+++ b/ee/app/models/ee/ci/job_artifact.rb
@@ -26,8 +26,10 @@ module Ci::JobArtifact
      scope :project_id_in, ->(ids) { where(project_id: ids) }
      scope :with_files_stored_remotely, -> { where(file_store: ::JobArtifactUploader::Store::REMOTE) }

-      scope :security_reports, -> do
-        with_file_types(SECURITY_REPORT_FILE_TYPES)
+      scope :security_reports, -> (file_types: SECURITY_REPORT_FILE_TYPES) do
+        requested_file_types = *file_types
+
+        with_file_types(requested_file_types & SECURITY_REPORT_FILE_TYPES)
      end

      scope :license_scanning_reports, -> do

--- a/ee/app/models/ee/ci/pipeline.rb
+++ b/ee/app/models/ee/ci/pipeline.rb
@@ -101,9 +101,11 @@ def expose_license_scanning_data?
        batch_lookup_report_artifact_for_file_type(:license_scanning).present?
      end

-      def security_reports
+      def security_reports(report_types: [])
+        reports_scope = report_types.empty? ? ::Ci::JobArtifact.security_reports : ::Ci::JobArtifact.security_reports(file_types: report_types)
+
        ::Gitlab::Ci::Reports::Security::Reports.new(self).tap do |security_reports|
-          builds.latest.with_reports(::Ci::JobArtifact.security_reports).each do |build|
+          builds.latest.with_reports(reports_scope).each do |build|
            build.collect_security_reports!(security_reports)
          end
        end

--- a/ee/changelogs/unreleased/238156_short_term_solution_to_load_pipeline_security_reports.yml
+++ b/ee/changelogs/unreleased/238156_short_term_solution_to_load_pipeline_security_reports.yml
+---
+title: Load only the requested report artifacts into the memory for vulnerability_findings
+  endpoint
+merge_request: 39749
+author:
+type: performance
--- a/ee/spec/models/ci/pipeline_spec.rb
+++ b/ee/spec/models/ci/pipeline_spec.rb
@@ -169,6 +169,14 @@
          expect(subject.get_report('container_scanning', cs1_artifact).findings.size).to eq(8)
        end
      end
+
+      context 'when the `report_types` parameter is provided' do
+        subject(:filtered_report_types) { pipeline.security_reports(report_types: %w(sast)).reports.values.map(&:type).uniq }
+
+        it 'returns only the reports which are requested' do
+          expect(filtered_report_types).to eq(%w(sast))
+        end
+      end
    end

    context 'when pipeline does not have any builds with security reports' do

--- a/ee/spec/models/ee/ci/job_artifact_spec.rb
+++ b/ee/spec/models/ee/ci/job_artifact_spec.rb
@@ -54,19 +54,48 @@
  end

  describe '.security_reports' do
-    subject { Ci::JobArtifact.security_reports }
-
-    context 'when there is a security report' do
+    context 'when the `file_types` parameter is provided' do
      let!(:sast_artifact) { create(:ee_ci_job_artifact, :sast) }
-      let!(:secret_detection_artifact) { create(:ee_ci_job_artifact, :secret_detection) }

-      it { is_expected.to eq([sast_artifact, secret_detection_artifact]) }
+      subject { Ci::JobArtifact.security_reports(file_types: file_types) }
+
+      context 'when the provided file_types is array' do
+        let(:file_types) { %w(secret_detection) }
+
+        context 'when there is a security report with the given value' do
+          let!(:secret_detection_artifact) { create(:ee_ci_job_artifact, :secret_detection) }
+
+          it { is_expected.to eq([secret_detection_artifact]) }
+        end
+
+        context 'when there are no security reports with the given value' do
+          it { is_expected.to be_empty }
+        end
+      end
+
+      context 'when the provided file_types is string' do
+        let(:file_types) { 'secret_detection' }
+        let!(:secret_detection_artifact) { create(:ee_ci_job_artifact, :secret_detection) }
+
+        it { is_expected.to eq([secret_detection_artifact]) }
+      end
    end

-    context 'when there are no security reports' do
-      let!(:artifact) { create(:ci_job_artifact, :archive) }
+    context 'when the file_types parameter is not provided' do
+      subject { Ci::JobArtifact.security_reports }

-      it { is_expected.to be_empty }
+      context 'when there is a security report' do
+        let!(:sast_artifact) { create(:ee_ci_job_artifact, :sast) }
+        let!(:secret_detection_artifact) { create(:ee_ci_job_artifact, :secret_detection) }
+
+        it { is_expected.to match_array([sast_artifact, secret_detection_artifact]) }
+      end
+
+      context 'when there are no security reports' do
+        let!(:artifact) { create(:ci_job_artifact, :archive) }
+
+        it { is_expected.to be_empty }
+      end
    end
  end