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