Merge branch '350915_mark_vulnerabilities_as_resolved_in_batches' into 'master'

Update `resolved_on_default_branch` attribute in batches

See merge request gitlab-org/gitlab!78960

app/models/vulnerability.rb

@@ -2,6 +2,7 @@
 
 # Placeholder class for model that is implemented in EE
 class Vulnerability < ApplicationRecord
+  include EachBatch
   include IgnorableColumns
 
   def self.link_reference_pattern

ee/app/services/security/ingestion/ingest_reports_service.rb

@@ -39,14 +39,8 @@ module Security
         IngestReportService.execute(security_scan)
       end
 
-      # This can cause issues if we have lots of existing ids
-      # or, if we try to update lots of records at once.
-      # Maybe we can extract this into a different service class
-      # and update the records iteratively.
       def mark_resolved_vulnerabilities(existing_ids)
-        project.vulnerabilities
-               .id_not_in(existing_ids)
-               .update_all(resolved_on_default_branch: true)
+        MarkAsResolvedService.execute(project, existing_ids)
       end
 
       def mark_project_as_vulnerable!

ee/app/services/security/ingestion/mark_as_resolved_service.rb

+# frozen_string_literal: true
+
+module Security
+  module Ingestion
+    # This service class takes the IDs of recently ingested
+    # vulnerabilities for a project and marks the missing
+    # vulnerabilities as resolved on default branch.
+    class MarkAsResolvedService
+      def self.execute(project, ingested_ids)
+        new(project, ingested_ids).execute
+      end
+
+      def initialize(project, ingested_ids)
+        @project = project
+        @ingested_ids = ingested_ids
+      end
+
+      def execute
+        vulnerabilities.each_batch { |batch| process_batch(batch) }
+      end
+
+      private
+
+      attr_reader :project, :ingested_ids
+
+      delegate :vulnerabilities, to: :project, private: true
+
+      def process_batch(batch)
+        (batch.pluck_primary_key - ingested_ids).then { |missing_ids| mark_as_resolved(missing_ids) }
+      end
+
+      def mark_as_resolved(missing_ids)
+        return if missing_ids.blank?
+
+        Vulnerability.id_in(missing_ids)
+                     .with_resolution(false)
+                     .update_all(resolved_on_default_branch: true)
+      end
+    end
+  end
+end

ee/spec/services/security/ingestion/ingest_reports_service_spec.rb

@@ -22,6 +22,7 @@ RSpec.describe Security::Ingestion::IngestReportsService do
 
     before do
       allow(Security::Ingestion::IngestReportService).to receive(:execute).and_return(ids_1, ids_2)
+      allow(Security::Ingestion::MarkAsResolvedService).to receive(:execute)
     end
 
     it 'calls IngestReportService for each succeeded security scan' do
@@ -35,8 +36,12 @@ RSpec.describe Security::Ingestion::IngestReportsService do
     it 'sets the resolved vulnerabilities, latest pipeline ID and has_vulnerabilities flag' do
       expect { ingest_reports }.to change { project.reload.project_setting&.has_vulnerabilities }.to(true)
                                .and change { project.reload.vulnerability_statistic&.latest_pipeline_id }.to(pipeline.id)
-                               .and change { vulnerability_2.reload.resolved_on_default_branch }.from(false).to(true)
-                               .and not_change { vulnerability_1.reload.resolved_on_default_branch }.from(false)
+    end
+
+    it 'calls MarkAsResolvedService with the recently ingested vulnerability IDs' do
+      ingest_reports
+
+      expect(Security::Ingestion::MarkAsResolvedService).to have_received(:execute).with(project, ids_1)
     end
 
     describe 'scheduling the AutoFix background job' do

ee/spec/services/security/ingestion/mark_as_resolved_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Security::Ingestion::MarkAsResolvedService do
+  let!(:project) { create(:project) }
+  let!(:vulnerability_1) { create(:vulnerability, project: project) }
+  let!(:vulnerability_2) { create(:vulnerability, project: project) }
+
+  let(:ingested_ids) { [vulnerability_2.id] }
+  let(:service_object) { described_class.new(project, ingested_ids) }
+
+  describe '#execute' do
+    subject(:mark_as_resolved) { service_object.execute }
+
+    it 'marks the missing vulnerabilities as resolved on default branch' do
+      expect { mark_as_resolved }.to change { vulnerability_1.reload.resolved_on_default_branch }.from(false).to(true)
+                                 .and not_change { vulnerability_2.reload.resolved_on_default_branch }.from(false)
+    end
+  end
+end