Merge branch '351522_use_uuid_as_primary_key_for_deduplication_logic' into 'master'

Deduplicate findings by comparing the UUIDs

See merge request gitlab-org/gitlab!79680

ee/spec/lib/gitlab/ci/reports/security/finding_spec.rb

@@ -361,6 +361,7 @@ RSpec.describe Gitlab::Ci::Reports::Security::Finding do
     let(:finding) { build(:ci_reports_security_finding, identifiers: [identifier_1, identifier_2], location: location, vulnerability_finding_signatures_enabled: true, signatures: [signature]) }
     let(:expected_keys) do
       [
+        finding.uuid,
         build(:ci_reports_security_finding_key, location_fingerprint: location.fingerprint, identifier_fingerprint: identifier_1.fingerprint),
         build(:ci_reports_security_finding_key, location_fingerprint: location.fingerprint, identifier_fingerprint: identifier_2.fingerprint),
         build(:ci_reports_security_finding_key, location_fingerprint: signature.signature_hex, identifier_fingerprint: identifier_1.fingerprint),

lib/gitlab/ci/reports/security/finding.rb

@@ -126,7 +126,7 @@ module Gitlab
               location_fingerprints.map do |location_fingerprint|
                 FindingKey.new(location_fingerprint: location_fingerprint, identifier_fingerprint: identifier.fingerprint)
               end
-            end
+            end.push(uuid)
           end
 
           def primary_identifier_fingerprint

lib/gitlab/ci/reports/security/finding_key.rb

@@ -11,6 +11,8 @@ module Gitlab
           end
 
           def ==(other)
+            return false unless other.is_a?(self.class)
+
             has_fingerprints? && other.has_fingerprints? &&
               location_fingerprint == other.location_fingerprint &&
                 identifier_fingerprint == other.identifier_fingerprint

spec/lib/gitlab/ci/reports/security/finding_key_spec.rb

@@ -6,36 +6,47 @@ RSpec.describe Gitlab::Ci::Reports::Security::FindingKey do
   using RSpec::Parameterized::TableSyntax
 
   describe '#==' do
-    where(:location_fp_1, :location_fp_2, :identifier_fp_1, :identifier_fp_2, :equals?) do
-      nil           | 'different location fp' | 'identifier fp' | 'different identifier fp' | false
-      'location fp' | nil                     | 'identifier fp' | 'different identifier fp' | false
-      'location fp' | 'different location fp' | nil             | 'different identifier fp' | false
-      'location fp' | 'different location fp' | 'identifier fp' | nil                       | false
-      nil           | nil                     | 'identifier fp' | 'identifier fp'           | false
-      'location fp' | 'location fp'           | nil             | nil                       | false
-      nil           | nil                     | nil             | nil                       | false
-      'location fp' | 'different location fp' | 'identifier fp' | 'different identifier fp' | false
-      'location fp' | 'different location fp' | 'identifier fp' | 'identifier fp'           | false
-      'location fp' | 'location fp'           | 'identifier fp' | 'different identifier fp' | false
-      'location fp' | 'location fp'           | 'identifier fp' | 'identifier fp'           | true
-    end
-
-    with_them do
-      let(:finding_key_1) do
-        build(:ci_reports_security_finding_key,
-              location_fingerprint: location_fp_1,
-              identifier_fingerprint: identifier_fp_1)
+    context 'when the comparison is done between FindingKey instances' do
+      where(:location_fp_1, :location_fp_2, :identifier_fp_1, :identifier_fp_2, :equals?) do
+        nil           | 'different location fp' | 'identifier fp' | 'different identifier fp' | false
+        'location fp' | nil                     | 'identifier fp' | 'different identifier fp' | false
+        'location fp' | 'different location fp' | nil             | 'different identifier fp' | false
+        'location fp' | 'different location fp' | 'identifier fp' | nil                       | false
+        nil           | nil                     | 'identifier fp' | 'identifier fp'           | false
+        'location fp' | 'location fp'           | nil             | nil                       | false
+        nil           | nil                     | nil             | nil                       | false
+        'location fp' | 'different location fp' | 'identifier fp' | 'different identifier fp' | false
+        'location fp' | 'different location fp' | 'identifier fp' | 'identifier fp'           | false
+        'location fp' | 'location fp'           | 'identifier fp' | 'different identifier fp' | false
+        'location fp' | 'location fp'           | 'identifier fp' | 'identifier fp'           | true
       end
 
-      let(:finding_key_2) do
-        build(:ci_reports_security_finding_key,
-              location_fingerprint: location_fp_2,
-              identifier_fingerprint: identifier_fp_2)
+      with_them do
+        let(:finding_key_1) do
+          build(:ci_reports_security_finding_key,
+                location_fingerprint: location_fp_1,
+                identifier_fingerprint: identifier_fp_1)
+        end
+
+        let(:finding_key_2) do
+          build(:ci_reports_security_finding_key,
+                location_fingerprint: location_fp_2,
+                identifier_fingerprint: identifier_fp_2)
+        end
+
+        subject { finding_key_1 == finding_key_2 }
+
+        it { is_expected.to be(equals?) }
       end
+    end
+
+    context 'when the comparison is not done between FindingKey instances' do
+      let(:finding_key) { build(:ci_reports_security_finding_key) }
+      let(:uuid) { SecureRandom.uuid }
 
-      subject { finding_key_1 == finding_key_2 }
+      subject { finding_key == uuid }
 
-      it { is_expected.to be(equals?) }
+      it { is_expected.to be_falsey }
     end
   end
 end