Skip to content

Update Security::FindingsFinder to use keyset pagination and filter by partition number

Per #378084 (closed), it was determined that implementing keyset pagination and filtering security findings by their partition number results in more efficient querying.

Implementation Plan

  • Update the Security::FindingsFinder query to use keyset pagination and query by the first findings partition number.
Testing Diff
diff --git a/ee/app/finders/security/findings_finder.rb b/ee/app/finders/security/findings_finder.rb
index a666f4f09c33..e878ddf538fe 100644
--- a/ee/app/finders/security/findings_finder.rb
+++ b/ee/app/finders/security/findings_finder.rb
@@ -84,11 +84,16 @@ def builds
     end
 
     def security_findings
-      @security_findings ||= include_dismissed? ? all_security_findings : all_security_findings.undismissed
+      @security_findings ||= (include_dismissed? ? all_security_findings : all_security_findings.undismissed).keyset_paginate
+    end
+
+    def partition_number
+      pipeline.security_findings.first.partition_number
     end
 
     def all_security_findings
       pipeline.security_findings
+              .by_partion_number(partition_number)
               .with_pipeline_entities
               .with_scan
               .with_scanner
diff --git a/ee/app/models/security/finding.rb b/ee/app/models/security/finding.rb
index 16df0e572552..5d1450d49069 100644
--- a/ee/app/models/security/finding.rb
+++ b/ee/app/models/security/finding.rb
@@ -54,10 +54,12 @@ class Finding < ApplicationRecord
                 .where('vulnerability_feedback.finding_uuid = security_findings.uuid'))
     end
     scope :latest, -> { joins(:scan).merge(Security::Scan.latest_successful) }
-    scope :ordered, -> { order(severity: :desc, confidence: :desc, id: :asc) }
+    # scope :ordered, -> { order(severity: :desc, confidence: :desc, id: :asc) }
+    scope :ordered, -> { order(Security::Finding.keyset_order) }
     scope :with_pipeline_entities, -> { preload(build: [:job_artifacts, :pipeline]) }
     scope :with_scan, -> { includes(:scan) }
     scope :with_scanner, -> { includes(:scanner) }
+    scope :by_partion_number, -> (partition_number) { where(partition_number: partition_number) }
     scope :deduplicated, -> { where(deduplicated: true) }
     scope :grouped_by_scan_type, -> { joins(:scan).group('security_scans.scan_type') }
 
@@ -87,6 +89,32 @@ def active_partition_number
         active_partition&.value || column_defaults['partition_number']
       end
 
+      def keyset_order
+        Gitlab::Pagination::Keyset::Order.build([
+                                                  # The attributes are documented in the `lib/gitlab/pagination/keyset/column_order_definition.rb` file
+                                                  Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(
+                                                    attribute_name: 'severity',
+                                                    order_expression: Security::Finding.arel_table[:severity].desc,
+                                                    # reversed_order_expression: Issue.arel_table[:relative_position].asc.nulls_first,
+                                                    # nullable: :nulls_last,
+                                                    order_direction: :desc,
+                                                    distinct: false
+                                                  ),
+                                                  Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(
+                                                    attribute_name: 'confidence',
+                                                    order_expression: Security::Finding.arel_table[:confidence].desc,
+                                                    # nullable: :not_nullable,
+                                                    distinct: true
+                                                  ),
+                                                  Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(
+                                                    attribute_name: 'id',
+                                                    order_expression: Security::Finding.arel_table[:id].asc,
+                                                    # nullable: :not_nullable,
+                                                    distinct: true
+                                                  )
+                                                ])
+      end
+
       private
 
       delegate :active_partition, to: :partitioning_strategy, private: true