perf(reimporter): batch vulnerability_id reconciliation

Extend the cross-finding accumulation pattern to DefaultReImporter:

- reconcile_vulnerability_ids now accumulates changed findings into
  pending_vuln_id_deletes / pending_vulnerability_ids instead of
  issuing per-finding DELETE + INSERT immediately
- flush_vulnerability_ids (BaseImporter) runs one bulk DELETE WHERE
  finding_id IN (...) followed by one bulk_create for all new IDs
- flush called at both dedupe batch boundaries (alongside
  location_handler.persist()) and after the mitigation loop

Early-exit path (unchanged IDs) never touches either buffer, so the
common case pays zero extra cost.

Add two unit tests: cross-finding batch (3 findings, 2 changed + 1
unchanged, verify buffer contents before flush and DB state after) and
unchanged-IDs early-exit (verify buffers stay empty).

Author: valentijnscholten

dojo/importers/base_importer.py

@@ -79,6 +79,7 @@ def __init__(
         """
         ImporterOptions.__init__(self, *args, **kwargs)
         self.pending_vulnerability_ids: list[Vulnerability_Id] = []
+        self.pending_vuln_id_deletes: list[int] = []
 
     def check_child_implementation_exception(self):
         """
@@ -797,7 +798,10 @@ def store_vulnerability_ids(
         return finding
 
     def flush_vulnerability_ids(self) -> None:
-        """Bulk-insert all accumulated Vulnerability_Id objects and clear the buffer."""
+        """Delete stale and bulk-insert accumulated Vulnerability_Id objects, then clear buffers."""
+        if self.pending_vuln_id_deletes:
+            Vulnerability_Id.objects.filter(finding_id__in=self.pending_vuln_id_deletes).delete()
+            self.pending_vuln_id_deletes.clear()
         if self.pending_vulnerability_ids:
             Vulnerability_Id.objects.bulk_create(self.pending_vulnerability_ids, batch_size=1000)
             self.pending_vulnerability_ids.clear()

dojo/importers/default_reimporter.py

@@ -23,6 +23,7 @@
     Notes,
     Test,
     Test_Import,
+    Vulnerability_Id,
 )
 from dojo.tags import inheritance as tag_inheritance
 from dojo.tags.inheritance import apply_inherited_tags_for_findings
@@ -438,6 +439,7 @@ def _process_findings_internal(
                     # They don't need to be aligned since they optimize different operations.
                     if len(batch_finding_ids) >= dedupe_batch_max_size or is_final:
                         self.location_handler.persist()
+                        self.flush_vulnerability_ids()
                         # Apply parser-supplied tags for this batch before post-processing starts,
                         # so rules/deduplication tasks see the tags already on the findings.
                         bulk_apply_parser_tags(findings_with_parser_tags)
@@ -561,6 +563,7 @@ def close_old_findings(
                 mitigated_findings.append(finding)
         # Persist any accumulated location/endpoint status changes
         self.location_handler.persist()
+        self.flush_vulnerability_ids()
         # push finding groups to jira since we only only want to push whole groups
         # We dont check if the finding jira sync is applicable quite yet until we can get in the loop
         # but this is a way to at least make it that far
@@ -944,33 +947,34 @@ def reconcile_vulnerability_ids(
     ) -> Finding:
         """
         Reconcile vulnerability IDs for an existing finding.
-        Checks if IDs have changed before updating to avoid unnecessary database operations.
-        Uses prefetched data if available, otherwise fetches efficiently.
-
-        Args:
-            finding: The existing finding to reconcile vulnerability IDs for.
-                Must have unsaved_vulnerability_ids set.
-
-        Returns:
-            The finding object
-
+        Accumulates changes into pending_vuln_id_deletes / pending_vulnerability_ids
+        for batch flush at the batch boundary via flush_vulnerability_ids().
         """
-        vulnerability_ids_to_process = finding.unsaved_vulnerability_ids or []
+        vulnerability_ids_to_process = list(dict.fromkeys(finding.unsaved_vulnerability_ids or []))
+        vulnerability_ids_to_process = [x for x in vulnerability_ids_to_process if x.strip()]
 
         # Use prefetched data directly without triggering queries
         existing_vuln_ids = {v.vulnerability_id for v in finding.vulnerability_id_set.all()}
         new_vuln_ids = set(vulnerability_ids_to_process)
 
-        # Early exit if unchanged
+        # Early exit if unchanged — no DB work needed
         if existing_vuln_ids == new_vuln_ids:
             logger.debug(
                 f"Skipping vulnerability_ids update for finding {finding.id} - "
                 f"vulnerability_ids unchanged: {sorted(existing_vuln_ids)}",
             )
             return finding
 
-        # Update if changed
-        finding_helper.save_vulnerability_ids(finding, vulnerability_ids_to_process, delete_existing=True)
+        # Accumulate delete + insert for batch flush
+        self.pending_vuln_id_deletes.append(finding.id)
+        self.pending_vulnerability_ids.extend([
+            Vulnerability_Id(finding=finding, vulnerability_id=vid)
+            for vid in vulnerability_ids_to_process
+        ])
+        if vulnerability_ids_to_process:
+            finding.cve = vulnerability_ids_to_process[0]
+        else:
+            finding.cve = None
         return finding
 
     def finding_post_processing(

unittests/test_importers_importer.py

@@ -887,7 +887,9 @@ def test_clear_vulnerability_ids_on_empty_list(self):
 
         # Process with empty list - should clear all IDs
         finding.unsaved_vulnerability_ids = []
-        DefaultReImporter(test=self.test, environment=self.importer_data["environment"], scan_type=self.importer_data["scan_type"]).reconcile_vulnerability_ids(finding)
+        reimporter = DefaultReImporter(test=self.test, environment=self.importer_data["environment"], scan_type=self.importer_data["scan_type"])
+        reimporter.reconcile_vulnerability_ids(finding)
+        reimporter.flush_vulnerability_ids()
         # Save the finding to persist the cve=None change
         finding.save()
 
@@ -924,7 +926,9 @@ def test_change_vulnerability_ids_on_reimport(self):
         # Process with different IDs - should replace old IDs
         new_vulnerability_ids = ["CVE-2021-9999", "GHSA-xxxx-yyyy"]
         finding.unsaved_vulnerability_ids = new_vulnerability_ids
-        DefaultReImporter(test=self.test, environment=self.importer_data["environment"], scan_type=self.importer_data["scan_type"]).reconcile_vulnerability_ids(finding)
+        reimporter = DefaultReImporter(test=self.test, environment=self.importer_data["environment"], scan_type=self.importer_data["scan_type"])
+        reimporter.reconcile_vulnerability_ids(finding)
+        reimporter.flush_vulnerability_ids()
         # Save the finding to persist the cve change
         finding.save()
 
@@ -940,3 +944,90 @@ def test_change_vulnerability_ids_on_reimport(self):
         vuln_ids = list(Vulnerability_Id.objects.filter(finding=finding).values_list("vulnerability_id", flat=True))
         self.assertEqual(set(new_vulnerability_ids), set(vuln_ids))
         finding.delete()
+
+    def test_reconcile_vulnerability_ids_cross_finding_batch(self):
+        """Multiple findings accumulated before flush — one delete+insert pair per changed finding."""
+        reimporter = DefaultReImporter(test=self.test, environment=self.importer_data["environment"], scan_type=self.importer_data["scan_type"])
+
+        # finding_a: IDs change (CVE-A → CVE-B)
+        finding_a = Finding(test=self.test, reporter=self.testuser)
+        finding_a.save()
+        Vulnerability_Id.objects.create(finding=finding_a, vulnerability_id="CVE-A-OLD")
+        finding_a.cve = "CVE-A-OLD"
+        finding_a.save()
+
+        # finding_b: IDs change (CVE-B1, CVE-B2 → CVE-B-NEW)
+        finding_b = Finding(test=self.test, reporter=self.testuser)
+        finding_b.save()
+        Vulnerability_Id.objects.create(finding=finding_b, vulnerability_id="CVE-B1")
+        Vulnerability_Id.objects.create(finding=finding_b, vulnerability_id="CVE-B2")
+        finding_b.cve = "CVE-B1"
+        finding_b.save()
+
+        # finding_c: IDs unchanged — should not appear in delete/insert buffers
+        finding_c = Finding(test=self.test, reporter=self.testuser)
+        finding_c.save()
+        Vulnerability_Id.objects.create(finding=finding_c, vulnerability_id="CVE-C-SAME")
+        finding_c.cve = "CVE-C-SAME"
+        finding_c.save()
+
+        finding_a.unsaved_vulnerability_ids = ["CVE-A-NEW"]
+        finding_b.unsaved_vulnerability_ids = ["CVE-B-NEW"]
+        finding_c.unsaved_vulnerability_ids = ["CVE-C-SAME"]
+
+        # Accumulate all three before any flush
+        reimporter.reconcile_vulnerability_ids(finding_a)
+        reimporter.reconcile_vulnerability_ids(finding_b)
+        reimporter.reconcile_vulnerability_ids(finding_c)
+
+        # pending_vuln_id_deletes only contains changed findings, not finding_c
+        self.assertIn(finding_a.id, reimporter.pending_vuln_id_deletes)
+        self.assertIn(finding_b.id, reimporter.pending_vuln_id_deletes)
+        self.assertNotIn(finding_c.id, reimporter.pending_vuln_id_deletes)
+        self.assertEqual(2, len(reimporter.pending_vulnerability_ids))
+
+        # Old IDs still in DB (not yet deleted)
+        self.assertEqual(1, Vulnerability_Id.objects.filter(finding=finding_a).count())
+        self.assertEqual(2, Vulnerability_Id.objects.filter(finding=finding_b).count())
+
+        reimporter.flush_vulnerability_ids()
+
+        # Buffers cleared
+        self.assertEqual([], reimporter.pending_vuln_id_deletes)
+        self.assertEqual([], reimporter.pending_vulnerability_ids)
+
+        # finding_a: old deleted, new inserted
+        vuln_ids_a = list(Vulnerability_Id.objects.filter(finding=finding_a).values_list("vulnerability_id", flat=True))
+        self.assertEqual(["CVE-A-NEW"], vuln_ids_a)
+        self.assertEqual("CVE-A-NEW", finding_a.cve)
+
+        # finding_b: both old deleted, new inserted
+        vuln_ids_b = list(Vulnerability_Id.objects.filter(finding=finding_b).values_list("vulnerability_id", flat=True))
+        self.assertEqual(["CVE-B-NEW"], vuln_ids_b)
+        self.assertEqual("CVE-B-NEW", finding_b.cve)
+
+        # finding_c: unchanged — IDs untouched
+        vuln_ids_c = list(Vulnerability_Id.objects.filter(finding=finding_c).values_list("vulnerability_id", flat=True))
+        self.assertEqual(["CVE-C-SAME"], vuln_ids_c)
+
+        finding_a.delete()
+        finding_b.delete()
+        finding_c.delete()
+
+    def test_reconcile_vulnerability_ids_unchanged_no_db_write(self):
+        """Early-exit path: unchanged IDs never touch pending buffers."""
+        reimporter = DefaultReImporter(test=self.test, environment=self.importer_data["environment"], scan_type=self.importer_data["scan_type"])
+
+        finding = Finding(test=self.test, reporter=self.testuser)
+        finding.save()
+        Vulnerability_Id.objects.create(finding=finding, vulnerability_id="CVE-2020-1234")
+        finding.cve = "CVE-2020-1234"
+        finding.save()
+
+        finding.unsaved_vulnerability_ids = ["CVE-2020-1234"]
+        reimporter.reconcile_vulnerability_ids(finding)
+
+        self.assertEqual([], reimporter.pending_vuln_id_deletes)
+        self.assertEqual([], reimporter.pending_vulnerability_ids)
+
+        finding.delete()