Skip to content

Commit a13dff2

Browse files
perf(importers): batch vulnerability_id inserts
Replace per-row Vulnerability_Id saves with bulk_create in two layers: - fix sanitize_vulnerability_ids to return filtered list (was a no-op bug — reassigned local variable, caller never saw the result) - save_vulnerability_ids now uses bulk_create per finding instead of one INSERT per ID; fixes all callers including the reimporter path - DefaultImporter.store_vulnerability_ids accumulates Vulnerability_Id objects across all findings in a batch; flush_vulnerability_ids() does a single bulk_create at each batch boundary (alongside location_handler.persist()) For a scan with 1000 findings × 5 CVEs each: 5000 INSERT queries reduced to O(batches) bulk_create calls.
1 parent c08db32 commit a13dff2

4 files changed

Lines changed: 42 additions & 25 deletions

File tree

dojo/finding/helper.py

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -986,27 +986,26 @@ def add_locations(finding, form, *, replace=False):
986986
return set(locations_to_associate)
987987

988988

989-
def sanitize_vulnerability_ids(vulnerability_ids) -> None:
989+
def sanitize_vulnerability_ids(vulnerability_ids):
990990
"""Remove undisired vulnerability id values"""
991-
vulnerability_ids = [x for x in vulnerability_ids if x.strip()]
991+
return [x for x in vulnerability_ids if x.strip()]
992992

993993

994994
def save_vulnerability_ids(finding, vulnerability_ids, *, delete_existing: bool = True):
995-
# Remove duplicates
995+
# Remove duplicates and empty/whitespace IDs
996996
vulnerability_ids = list(dict.fromkeys(vulnerability_ids))
997+
vulnerability_ids = sanitize_vulnerability_ids(vulnerability_ids)
997998

998999
# Remove old vulnerability ids if requested
9991000
# Callers can set delete_existing=False when they know there are no existing IDs
10001001
# to avoid an unnecessary delete query (e.g., for new findings)
10011002
if delete_existing:
10021003
Vulnerability_Id.objects.filter(finding=finding).delete()
10031004

1004-
# Remove undisired vulnerability ids
1005-
sanitize_vulnerability_ids(vulnerability_ids)
1006-
# Save new vulnerability ids
1007-
# Using bulk create throws Django 50 warnings about unsaved models...
1008-
for vulnerability_id in vulnerability_ids:
1009-
Vulnerability_Id(finding=finding, vulnerability_id=vulnerability_id).save()
1005+
Vulnerability_Id.objects.bulk_create([
1006+
Vulnerability_Id(finding=finding, vulnerability_id=vid)
1007+
for vid in vulnerability_ids
1008+
])
10101009

10111010
# Set CVE
10121011
if vulnerability_ids:

dojo/importers/base_importer.py

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
Test_Import,
3232
Test_Import_Finding_Action,
3333
Test_Type,
34+
Vulnerability_Id,
3435
)
3536
from dojo.notifications.helper import create_notification
3637
from dojo.tags.utils import bulk_add_tags_to_instances
@@ -77,6 +78,7 @@ def __init__(
7778
and will raise a `NotImplemented` exception
7879
"""
7980
ImporterOptions.__init__(self, *args, **kwargs)
81+
self.pending_vulnerability_ids: list[Vulnerability_Id] = []
8082

8183
def check_child_implementation_exception(self):
8284
"""
@@ -778,21 +780,28 @@ def store_vulnerability_ids(
778780
finding: Finding,
779781
) -> Finding:
780782
"""
781-
Store vulnerability IDs for a finding.
782-
Reads from finding.unsaved_vulnerability_ids and saves them overwriting existing ones.
783-
784-
Args:
785-
finding: The finding to store vulnerability IDs for
786-
787-
Returns:
788-
The finding object
789-
783+
Accumulate Vulnerability_Id objects for bulk insert at the batch boundary.
784+
Call flush_vulnerability_ids() to persist.
790785
"""
791786
self.sanitize_vulnerability_ids(finding)
792-
vulnerability_ids_to_process = finding.unsaved_vulnerability_ids or []
793-
finding_helper.save_vulnerability_ids(finding, vulnerability_ids_to_process, delete_existing=False)
787+
vulnerability_ids_to_process = list(dict.fromkeys(finding.unsaved_vulnerability_ids or []))
788+
vulnerability_ids_to_process = [x for x in vulnerability_ids_to_process if x.strip()]
789+
self.pending_vulnerability_ids.extend([
790+
Vulnerability_Id(finding=finding, vulnerability_id=vid)
791+
for vid in vulnerability_ids_to_process
792+
])
793+
if vulnerability_ids_to_process:
794+
finding.cve = vulnerability_ids_to_process[0]
795+
else:
796+
finding.cve = None
794797
return finding
795798

799+
def flush_vulnerability_ids(self) -> None:
800+
"""Bulk-insert all accumulated Vulnerability_Id objects and clear the buffer."""
801+
if self.pending_vulnerability_ids:
802+
Vulnerability_Id.objects.bulk_create(self.pending_vulnerability_ids, batch_size=1000)
803+
self.pending_vulnerability_ids.clear()
804+
796805
def process_files(
797806
self,
798807
finding: Finding,

dojo/importers/default_importer.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,7 @@ def _process_findings_internal(
275275
# If batch is full or we're at the end, persist locations/endpoints and dispatch
276276
if len(batch_finding_ids) >= batch_max_size or is_final_finding:
277277
self.location_handler.persist()
278+
self.flush_vulnerability_ids()
278279
# Apply parser-supplied tags for this batch before post-processing starts,
279280
# so rules/deduplication tasks see the tags already on the findings.
280281
bulk_apply_parser_tags(findings_with_parser_tags)
@@ -415,6 +416,7 @@ def close_old_findings(
415416
)
416417
# Persist any accumulated location/endpoint status changes
417418
self.location_handler.persist()
419+
self.flush_vulnerability_ids()
418420
# push finding groups to jira since we only only want to push whole groups
419421
# We dont check if the finding jira sync is applicable quite yet until we can get in the loop
420422
# but this is a way to at least make it that far

unittests/test_importers_importer.py

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -803,14 +803,15 @@ def create_default_data(self):
803803
}
804804

805805
def test_handle_vulnerability_ids_references_and_cve(self):
806-
# Why doesn't this test use the test db and query for one?
807806
vulnerability_ids = ["CVE", "REF-1", "REF-2"]
808807
finding = Finding()
809808
finding.unsaved_vulnerability_ids = vulnerability_ids
810809
finding.test = self.test
811810
finding.reporter = self.testuser
812811
finding.save()
813-
DefaultImporter(**self.importer_data).store_vulnerability_ids(finding)
812+
importer = DefaultImporter(**self.importer_data)
813+
importer.store_vulnerability_ids(finding)
814+
importer.flush_vulnerability_ids()
814815

815816
self.assertEqual("CVE", finding.vulnerability_ids[0])
816817
self.assertEqual("CVE", finding.cve)
@@ -827,7 +828,9 @@ def test_handle_no_vulnerability_ids_references_and_cve(self):
827828
finding.save()
828829
finding.unsaved_vulnerability_ids = vulnerability_ids
829830

830-
DefaultImporter(**self.importer_data).store_vulnerability_ids(finding)
831+
importer = DefaultImporter(**self.importer_data)
832+
importer.store_vulnerability_ids(finding)
833+
importer.flush_vulnerability_ids()
831834

832835
self.assertEqual("CVE", finding.vulnerability_ids[0])
833836
self.assertEqual("CVE", finding.cve)
@@ -841,7 +844,9 @@ def test_handle_vulnerability_ids_references_and_no_cve(self):
841844
finding.reporter = self.testuser
842845
finding.save()
843846
finding.unsaved_vulnerability_ids = vulnerability_ids
844-
DefaultImporter(**self.importer_data).store_vulnerability_ids(finding)
847+
importer = DefaultImporter(**self.importer_data)
848+
importer.store_vulnerability_ids(finding)
849+
importer.flush_vulnerability_ids()
845850

846851
self.assertEqual("REF-1", finding.vulnerability_ids[0])
847852
self.assertEqual("REF-1", finding.cve)
@@ -854,7 +859,9 @@ def test_no_handle_vulnerability_ids_references_and_no_cve(self):
854859
finding.test = self.test
855860
finding.reporter = self.testuser
856861
finding.save()
857-
DefaultImporter(**self.importer_data).store_vulnerability_ids(finding)
862+
importer = DefaultImporter(**self.importer_data)
863+
importer.store_vulnerability_ids(finding)
864+
importer.flush_vulnerability_ids()
858865
self.assertEqual(finding.cve, None)
859866
self.assertEqual(finding.unsaved_vulnerability_ids, None)
860867
self.assertEqual(finding.vulnerability_ids, [])

0 commit comments

Comments
 (0)