refactor(importers): track reimport finding buckets by id, not instance

dojo/finding/helper.py

@@ -184,22 +184,17 @@ def update_finding_status(new_state_finding, user, changed_fields=None):
     new_state_finding.last_status_update = now
 
 
-def filter_findings_by_existence(findings):
+def filter_finding_ids_by_existence(finding_ids):
     """
-    Return only findings that still exist in the database (by id).
+    Return the subset of the given ids that still exist in the database.
 
     Centralized helper used by importers to avoid FK violations during
-    bulk_create.
+    bulk_create -- e.g. a background async_dupe_delete task removing a finding
+    between when an importer collected it and when it writes a history record.
     """
-    if not findings:
-        return []
-    candidate_ids = [finding.id for finding in findings if getattr(finding, "id", None)]
-    if not candidate_ids:
-        return []
-    existing_ids = set(
-        Finding.objects.filter(id__in=candidate_ids).values_list("id", flat=True),
-    )
-    return [finding for finding in findings if finding.id in existing_ids]
+    if not finding_ids:
+        return set()
+    return set(Finding.objects.filter(id__in=finding_ids).values_list("id", flat=True))
 
 
 def can_edit_mitigated_data(user):

dojo/importers/base_importer.py

@@ -475,10 +475,10 @@ def apply_import_tags_for_batch(self, findings: list[Finding]) -> None:
 
     def update_import_history(
         self,
-        new_findings: list[Finding] | None = None,
-        closed_findings: list[Finding] | None = None,
-        reactivated_findings: list[Finding] | None = None,
-        untouched_findings: list[Finding] | None = None,
+        new_findings: list[int] | None = None,
+        closed_findings: list[int] | None = None,
+        reactivated_findings: list[int] | None = None,
+        untouched_findings: list[int] | None = None,
     ) -> Test_Import:
         """Creates a record of the import or reimport operation that has occurred."""
         # Quick fail check to determine if we even wanted this
@@ -541,23 +541,22 @@ def update_import_history(
 
         # In longer running imports it can happen that the async_dupe_delete task removes a finding before the history record is created
         # We filter out these findings here to avoid FK violations (IntegrityError)
-        all_findings = []
+        all_finding_ids = []
         for list_, _ in finding_action_mappings:
-            all_findings.extend(list_)
-        existing_findings = finding_helper.filter_findings_by_existence(all_findings) if all_findings else []
-        existing_ids = {f.id for f in existing_findings}
+            all_finding_ids.extend(list_)
+        existing_ids = finding_helper.filter_finding_ids_by_existence(all_finding_ids) if all_finding_ids else set()
 
         # Collect all import history records using the validated IDs
         import_history_records = []
-        for findings, action in finding_action_mappings:
+        for finding_ids, action in finding_action_mappings:
             import_history_records.extend(
                 Test_Import_Finding_Action(
                     test_import=test_import,
-                    finding_id=finding.id,
+                    finding_id=finding_id,
                     action=action,
                 )
-                for finding in findings
-                if finding.id in existing_ids
+                for finding_id in finding_ids
+                if finding_id in existing_ids
             )
 
         # Bulk create all at once and let Django handle batching internally.
@@ -1201,6 +1200,13 @@ def notify_scan_added(
         findings_reactivated=None,
         findings_untouched=None,
     ):
+        """
+        new_findings/findings_mitigated/findings_reactivated/findings_untouched are
+        ids, not instances (M1) -- nothing here needs a live Finding until the
+        notification is actually built, and then only a capped, ordered slice of it
+        (a reimport that touches thousands of findings should not template all of
+        them into an email/webhook body).
+        """
         if findings_untouched is None:
             findings_untouched = []
         if findings_reactivated is None:
@@ -1212,48 +1218,55 @@ def notify_scan_added(
         logger.debug("Scan added notifications")
 
         # When deduplication has finished (synchronous mode, or async_wait after the
-        # join), the in-memory findings still carry their pre-dedup duplicate=False
-        # flag because deduplication runs on separately-fetched instances. Refresh the
-        # flag from the database and split each list into "real" and duplicate findings
-        # so the notification reflects post-dedup reality instead of counting/listing
-        # deduplicated findings as brand new. In plain async mode dedup has not run yet,
-        # so we leave the lists untouched (best-effort, historical behavior).
-        findings_new_duplicate: list[Finding] = []
-        findings_reactivated_duplicate: list[Finding] = []
-        findings_untouched_duplicate: list[Finding] = []
+        # join), the ids collected during matching still reflect pre-dedup reality
+        # because deduplication runs on separately-fetched instances. Split each list
+        # of ids into "real" and duplicate ids from a fresh query so the notification
+        # reflects post-dedup reality instead of counting/listing deduplicated
+        # findings as brand new. In plain async mode dedup has not run yet, so we
+        # leave the lists untouched (best-effort, historical behavior).
+        findings_new_duplicate_ids: list[int] = []
+        findings_reactivated_duplicate_ids: list[int] = []
+        findings_untouched_duplicate_ids: list[int] = []
         if getattr(self, "deduplication_complete", False):
-            all_ids = [f.id for f in (*new_findings, *findings_reactivated, *findings_untouched)]
+            all_ids = [*new_findings, *findings_reactivated, *findings_untouched]
             duplicate_ids = set()
             if all_ids:
                 duplicate_ids = set(
                     Finding.objects.filter(id__in=all_ids, duplicate=True).values_list("id", flat=True),
                 )
 
-            def _split(findings):
-                kept, duplicates = [], []
-                for finding in findings:
-                    if finding.id in duplicate_ids:
-                        # refresh the in-memory flag so any template logic is correct
-                        finding.duplicate = True
-                        duplicates.append(finding)
-                    else:
-                        kept.append(finding)
+            def _split(ids):
+                kept = [i for i in ids if i not in duplicate_ids]
+                duplicates = [i for i in ids if i in duplicate_ids]
                 return kept, duplicates
 
-            new_findings, findings_new_duplicate = _split(new_findings)
-            findings_reactivated, findings_reactivated_duplicate = _split(findings_reactivated)
-            findings_untouched, findings_untouched_duplicate = _split(findings_untouched)
+            new_findings, findings_new_duplicate_ids = _split(new_findings)
+            findings_reactivated, findings_reactivated_duplicate_ids = _split(findings_reactivated)
+            findings_untouched, findings_untouched_duplicate_ids = _split(findings_untouched)
             # Recompute the headline count to exclude findings that turned out to be
             # duplicates of an existing finding (they are not genuinely new activity).
             updated_count = len(new_findings) + len(findings_reactivated) + len(findings_mitigated)
 
-        new_findings = sorted(new_findings, key=lambda x: x.numerical_severity)
-        findings_mitigated = sorted(findings_mitigated, key=lambda x: x.numerical_severity)
-        findings_reactivated = sorted(findings_reactivated, key=lambda x: x.numerical_severity)
-        findings_untouched = sorted(findings_untouched, key=lambda x: x.numerical_severity)
-        findings_new_duplicate = sorted(findings_new_duplicate, key=lambda x: x.numerical_severity)
-        findings_reactivated_duplicate = sorted(findings_reactivated_duplicate, key=lambda x: x.numerical_severity)
-        findings_untouched_duplicate = sorted(findings_untouched_duplicate, key=lambda x: x.numerical_severity)
+        max_findings = settings.NOTIFICATION_SCAN_ADDED_MAX_FINDINGS
+
+        def _hydrate(ids):
+            # duplicate is re-read fresh here, so unlike the old in-memory instances
+            # there is no separate write-back needed to keep template logic correct.
+            if not ids:
+                return []
+            return list(
+                Finding.objects.filter(id__in=ids)
+                .only("id", "title", "severity", "numerical_severity", "duplicate")
+                .order_by("numerical_severity")[:max_findings],
+            )
+
+        new_findings = _hydrate(new_findings)
+        findings_mitigated = _hydrate(findings_mitigated)
+        findings_reactivated = _hydrate(findings_reactivated)
+        findings_untouched = _hydrate(findings_untouched)
+        findings_new_duplicate = _hydrate(findings_new_duplicate_ids)
+        findings_reactivated_duplicate = _hydrate(findings_reactivated_duplicate_ids)
+        findings_untouched_duplicate = _hydrate(findings_untouched_duplicate_ids)
 
         title = (
             f"Created/Updated {updated_count} findings for {test.engagement.product}: {test.engagement.name}: {test}"

dojo/importers/default_importer.py

@@ -134,12 +134,17 @@ def process_scan(
         self.save_without_resurrecting(self.test)
         if self.engagement_target_end_updated:
             self.save_without_resurrecting(self.test.engagement)
+        # update_import_history()/notify_scan_added() take ids (M1); this path still
+        # builds new_findings/closed_findings as instances internally, so adapt at
+        # the boundary rather than reshape process_findings()/close_old_findings().
+        new_finding_ids = [f.id for f in new_findings]
+        closed_finding_ids = [f.id for f in closed_findings]
         # Create a test import history object to record the flags sent to the importer
         # This operation will return None if the user does not have the import history
         # feature enabled
         test_import_history = self.update_import_history(
-            new_findings=new_findings,
-            closed_findings=closed_findings,
+            new_findings=new_finding_ids,
+            closed_findings=closed_finding_ids,
         )
         # In 'async_wait' mode, block until background deduplication has finished
         # so notifications and statistics reflect the deduplicated state.
@@ -157,12 +162,12 @@ def process_scan(
             url=reverse("view_test", args=(self.test.id,)),
             url_api=reverse("test-detail", args=(self.test.id,)),
         )
-        updated_count = len(new_findings) + len(closed_findings)
+        updated_count = len(new_finding_ids) + len(closed_finding_ids)
         self.notify_scan_added(
             self.test,
             updated_count,
-            new_findings=new_findings,
-            findings_mitigated=closed_findings,
+            new_findings=new_finding_ids,
+            findings_mitigated=closed_finding_ids,
         )
         # Update the test progress to reflect that the import has completed
         logger.debug("IMPORT_SCAN: Updating Test progress")