Merge branch '341796_drop_ci_table_join' into 'master'

Remove cross database join on Security::Scan model

See merge request gitlab-org/gitlab!72611

ee/app/models/security/finding.rb

@@ -34,7 +34,7 @@ module Security
                 .where('security_scans.id = security_findings.scan_id')
                 .where('vulnerability_feedback.project_fingerprint = security_findings.project_fingerprint'))
     end
-    scope :latest, -> { joins(:scan).merge(Security::Scan.latest_successful_by_build).allow_cross_joins_across_databases(url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/341796') }
+    scope :latest, -> { joins(:scan).merge(Security::Scan.latest_successful) }
     scope :ordered, -> { order(severity: :desc, confidence: :desc, id: :asc) }
     scope :with_pipeline_entities, -> { preload(build: [:job_artifacts, :pipeline]) }
     scope :with_scan, -> { includes(:scan) }

ee/app/models/security/scan.rb

@@ -43,9 +43,7 @@ module Security
     end
 
     scope :latest, -> { where(latest: true) }
-    # We are going to deprecate the following scope soon as this requires join between ci and non-ci table
-    # which will not be possible after database decomposition (https://gitlab.com/groups/gitlab-org/-/epics/6373)
-    scope :latest_successful_by_build, -> { joins(:build).where(ci_builds: { retried: [nil, false], status: 'success' }) }
+    scope :latest_successful, -> { latest.succeeded }
     scope :by_build_ids, ->(build_ids) { where(build_id: build_ids) }
     scope :without_errors, -> { where("jsonb_array_length(COALESCE(info->'errors', '[]'::jsonb)) = 0") }
 

ee/spec/factories/security_scans.rb

@@ -10,5 +10,10 @@ FactoryBot.define do
     trait :with_error do
       info { { errors: [{ type: 'ParsingError', message: 'Unknown error happened' }] } }
     end
+
+    trait :latest_successful do
+      latest { true }
+      status { :succeeded }
+    end
   end
 end

ee/spec/finders/security/findings_finder_spec.rb

@@ -48,7 +48,7 @@ RSpec.describe Security::FindingsFinder do
         report_sast.merge!(report_sast)
 
         { artifact_ds => report_ds, artifact_sast => report_sast }.each do |artifact, report|
-          scan = create(:security_scan, scan_type: artifact.job.name, build: artifact.job)
+          scan = create(:security_scan, :latest_successful, scan_type: artifact.job.name, build: artifact.job)
 
           report.findings.each_with_index do |finding, index|
             create(:security_finding,
@@ -321,7 +321,7 @@ RSpec.describe Security::FindingsFinder do
             Gitlab::Ci::Parsers::Security::DependencyScanning.parse!(retried_content, report)
             report.merge!(report)
 
-            scan = create(:security_scan, scan_type: retried_build.name, build: retried_build)
+            scan = create(:security_scan, scan_type: retried_build.name, build: retried_build, latest: false)
 
             report.findings.each_with_index do |finding, index|
               create(:security_finding,
@@ -344,7 +344,7 @@ RSpec.describe Security::FindingsFinder do
           let(:expected_fingerprints) { secret_detection_report.findings.map(&:project_fingerprint) }
 
           before do
-            scan = create(:security_scan, scan_type: :secret_detection, build: build_2)
+            scan = create(:security_scan, :latest_successful, scan_type: :secret_detection, build: build_2)
             artifact = create(:ee_ci_job_artifact, :secret_detection, job: build_2)
             report_content = File.read(artifact.file.path)
 

ee/spec/models/security/finding_spec.rb

@@ -3,8 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe Security::Finding do
-  let_it_be(:scan_1) { create(:security_scan, scan_type: :sast) }
-  let_it_be(:scan_2) { create(:security_scan, scan_type: :dast) }
+  let_it_be(:scan_1) { create(:security_scan, :latest_successful, scan_type: :sast) }
+  let_it_be(:scan_2) { create(:security_scan, :latest_successful, scan_type: :dast) }
   let_it_be(:finding_1) { create(:security_finding, scan: scan_1) }
   let_it_be(:finding_2) { create(:security_finding, scan: scan_2) }
 
@@ -150,7 +150,7 @@ RSpec.describe Security::Finding do
     let(:expected_findings) { [finding_2] }
 
     before do
-      finding_1.build.update!(retried: true)
+      scan_1.update!(latest: false)
     end
 
     it { is_expected.to eq(expected_findings) }

ee/spec/models/security/scan_spec.rb

@@ -86,12 +86,12 @@ RSpec.describe Security::Scan do
     end
   end
 
-  describe '.latest_successful_by_build' do
-    let!(:first_successful_scan) { create(:security_scan, build: create(:ci_build, :success, :retried)) }
-    let!(:second_successful_scan) { create(:security_scan, build: create(:ci_build, :success)) }
-    let!(:failed_scan) { create(:security_scan, build: create(:ci_build, :failed)) }
+  describe '.latest_successful' do
+    let!(:first_successful_scan) { create(:security_scan, latest: false, status: :succeeded) }
+    let!(:second_successful_scan) { create(:security_scan, latest: true, status: :succeeded) }
+    let!(:failed_scan) { create(:security_scan, latest: true, status: :failed) }
 
-    subject { described_class.latest_successful_by_build }
+    subject { described_class.latest_successful }
 
     it { is_expected.to match_array([second_successful_scan]) }
   end