refactor: remove ASYNC_DELETE_MAPPING, use FINDING_SCOPE_FILTERS

Replace the model_list-based mapping with a simple scope filter dict.
prepare_duplicates_for_delete now accepts a single object and derives
the scope via FINDING_SCOPE_FILTERS. Removes the redundant non-Finding
model deletion loop — cascade_delete on the top-level object handles
all remaining children. Cleans up async_delete class.

Author: valentijnscholten

dojo/finding/helper.py

@@ -615,33 +615,28 @@ def reconfigure_duplicate_cluster(original, cluster_outside):
         cluster_outside.exclude(id=new_original.id).update(duplicate_finding=new_original)
 
 
-def prepare_duplicates_for_delete(test=None, engagement=None, product=None, product_type=None):
-    logger.debug(
-        "prepare duplicates for delete, test: %s, engagement: %s, product: %s, product_type: %s",
-        test.id if test else None,
-        engagement.id if engagement else None,
-        product.id if product else None,
-        product_type.id if product_type else None,
-    )
-    if test is None and engagement is None and product is None and product_type is None:
-        logger.warning("nothing to prepare as no scope object provided")
+def prepare_duplicates_for_delete(obj):
+    """Prepare duplicate clusters before deleting a Test, Engagement, Product, or Product_Type.
+
+    Resets inside-scope duplicate FKs and reconfigures outside-scope clusters
+    so that cascade_delete won't hit FK violations on the self-referential
+    duplicate_finding field.
+    """
+    from dojo.utils import FINDING_SCOPE_FILTERS  # noqa: PLC0415 circular import
+
+    scope_field = FINDING_SCOPE_FILTERS.get(type(obj))
+    if scope_field is None:
+        logger.warning("prepare_duplicates_for_delete: unsupported object type %s", type(obj).__name__)
         return
 
+    logger.debug("prepare_duplicates_for_delete: %s %d", type(obj).__name__, obj.id)
+
     # should not be needed in normal healthy instances.
     # but in that case it's a cheap count query and we might as well run it to be safe
     fix_loop_duplicates()
 
     # Build scope as a subquery — never materialized into Python memory
-    if product_type:
-        scope_filter = {"test__engagement__product__prod_type": product_type}
-    elif product:
-        scope_filter = {"test__engagement__product": product}
-    elif engagement:
-        scope_filter = {"test__engagement": engagement}
-    else:
-        scope_filter = {"test": test}
-
-    scope_ids_subquery = Finding.objects.filter(**scope_filter).values_list("id", flat=True)
+    scope_ids_subquery = Finding.objects.filter(**{scope_field: obj}).values_list("id", flat=True)
 
     if not scope_ids_subquery.exists():
         logger.debug("no findings in scope, nothing to prepare")
@@ -679,7 +674,7 @@ def prepare_duplicates_for_delete(test=None, engagement=None, product=None, prod
 @receiver(pre_delete, sender=Test)
 def test_pre_delete(sender, instance, **kwargs):
     logger.debug("test pre_delete, sender: %s instance: %s", to_str_typed(sender), to_str_typed(instance))
-    prepare_duplicates_for_delete(test=instance)
+    prepare_duplicates_for_delete(instance)
 
 
 @receiver(post_delete, sender=Test)
@@ -690,7 +685,7 @@ def test_post_delete(sender, instance, **kwargs):
 @receiver(pre_delete, sender=Engagement)
 def engagement_pre_delete(sender, instance, **kwargs):
     logger.debug("engagement pre_delete, sender: %s instance: %s", to_str_typed(sender), to_str_typed(instance))
-    prepare_duplicates_for_delete(engagement=instance)
+    prepare_duplicates_for_delete(instance)
 
 
 @receiver(post_delete, sender=Engagement)

dojo/models.py

@@ -1647,7 +1647,7 @@ def is_ci_cd(self):
     def delete(self, *args, **kwargs):
         logger.debug("%d engagement delete", self.id)
         from dojo.finding import helper as finding_helper  # noqa: PLC0415 circular import
-        finding_helper.prepare_duplicates_for_delete(engagement=self)
+        finding_helper.prepare_duplicates_for_delete(self)
         super().delete(*args, **kwargs)
         with suppress(Engagement.DoesNotExist, Product.DoesNotExist):
             # Suppressing a potential issue created from async delete removing

dojo/utils.py

@@ -2026,23 +2026,16 @@ def is_finding_groups_enabled():
     return get_system_setting("enable_finding_groups")
 
 
-# Mapping of object types to their related models for cascading deletes
-ASYNC_DELETE_MAPPING = {
-    "Product_Type": [
-        (Endpoint, "product__prod_type__id"),
-        (Finding, "test__engagement__product__prod_type__id"),
-        (Test, "engagement__product__prod_type__id"),
-        (Engagement, "product__prod_type__id"),
-        (Product, "prod_type__id")],
-    "Product": [
-        (Endpoint, "product__id"),
-        (Finding, "test__engagement__product__id"),
-        (Test, "engagement__product__id"),
-        (Engagement, "product__id")],
-    "Engagement": [
-        (Finding, "test__engagement__id"),
-        (Test, "engagement__id")],
-    "Test": [(Finding, "test__id")],
+# Supported object types for async cascade deletion
+ASYNC_DELETE_SUPPORTED_TYPES = (Product_Type, Product, Engagement, Test)
+
+# Finding scope filters per model type — used to build the finding queryset
+# for bulk deletion before cascade_delete handles the rest.
+FINDING_SCOPE_FILTERS = {
+    Product_Type: "test__engagement__product__prod_type",
+    Product: "test__engagement__product",
+    Engagement: "test__engagement",
+    Test: "test",
 }
 
 
@@ -2054,7 +2047,7 @@ def _get_object_name(obj):
 
 
 @app.task
-def async_delete_crawl_task(obj, model_list, **kwargs):
+def async_delete_crawl_task(obj, **kwargs):
     """
     Delete an object and all its related objects using the SQL cascade walker.
 
@@ -2083,26 +2076,14 @@ def async_delete_crawl_task(obj, model_list, **kwargs):
             product = obj.engagement.product
 
     # Step 1: Determine finding scope
-    finding_filter = None
-    for model, query_path in model_list:
-        if model is Finding:
-            finding_filter = {query_path: obj.id}
-            break
-
-    if finding_filter:
-        finding_qs = Finding.objects.filter(**finding_filter)
+    scope_field = FINDING_SCOPE_FILTERS.get(type(obj))
+    if scope_field:
+        finding_qs = Finding.objects.filter(**{scope_field: obj})
 
         # Step 2: Prepare duplicate clusters (must happen before any deletion)
-        # When CASCADE_DELETE=True, reconfigure_duplicate_cluster skips deletion —
+        # When CASCADE_DELETE=True, reconfigure_duplicate_cluster skips reconfiguration —
         # we handle that below by expanding scope to include outside duplicates.
-        if isinstance(obj, Product_Type):
-            prepare_duplicates_for_delete(product_type=obj)
-        elif isinstance(obj, Product):
-            prepare_duplicates_for_delete(product=obj)
-        elif isinstance(obj, Engagement):
-            prepare_duplicates_for_delete(engagement=obj)
-        elif isinstance(obj, Test):
-            prepare_duplicates_for_delete(test=obj)
+        prepare_duplicates_for_delete(obj)
 
         # Step 3: Delete outside-scope duplicates first — these point to findings
         # in the main scope via duplicate_finding FK, so they must be removed before
@@ -2119,22 +2100,14 @@ def async_delete_crawl_task(obj, model_list, **kwargs):
         # Step 4: Delete the main scope findings
         bulk_delete_findings(finding_qs, chunk_size=chunk_size)
 
-        # Delete remaining non-Finding children (Tests, Engagements, Endpoints)
-        # These are now lightweight since their Findings are gone
-        for model, query_path in model_list:
-            if model is not Finding:
-                filter_dict = {query_path: obj.id}
-                qs = model.objects.filter(**filter_dict)
-                if qs.exists():
-                    with transaction.atomic():
-                        cascade_delete(model, qs, skip_relations={Finding})
-
-    # Step 6: Delete the top-level object itself
+    # Step 5: Delete the top-level object and all remaining children (Tests,
+    # Engagements, Endpoints, etc.) via cascade_delete. Findings are already
+    # gone, so skip_relations={Finding} avoids walking empty relations.
     pk_query = type(obj).objects.filter(pk=obj.pk)
     with transaction.atomic():
         cascade_delete(type(obj), pk_query, skip_relations={Finding})
 
-    # Step 7: Recalculate product grade once (not per-object)
+    # Step 6: Recalculate product grade once (not per-object)
     # The custom delete() methods on Finding/Test/Engagement each call
     # perform_product_grading — cascade_delete bypasses custom delete().
     if product:
@@ -2153,14 +2126,11 @@ def async_delete_task(obj, **kwargs):
     """
     from dojo.celery_dispatch import dojo_dispatch_task  # noqa: PLC0415 circular import
 
-    logger.debug("ASYNC_DELETE: Deleting " + _get_object_name(obj) + ": " + str(obj))
-    model_list = ASYNC_DELETE_MAPPING.get(_get_object_name(obj))
-    if model_list:
-        # The object to be deleted was found in the object list
-        dojo_dispatch_task(async_delete_crawl_task, obj, model_list)
+    logger.debug("ASYNC_DELETE: Deleting %s: %s", _get_object_name(obj), obj)
+    if isinstance(obj, ASYNC_DELETE_SUPPORTED_TYPES):
+        dojo_dispatch_task(async_delete_crawl_task, obj)
     else:
-        # The object is not supported in async delete, delete normally
-        logger.debug("ASYNC_DELETE: " + _get_object_name(obj) + " async delete not supported. Deleteing normally: " + str(obj))
+        logger.debug("ASYNC_DELETE: %s async delete not supported. Deleting normally: %s", _get_object_name(obj), obj)
         obj.delete()
 
 
@@ -2177,10 +2147,6 @@ class async_delete:
     which properly handles user context injection and pghistory context.
     """
 
-    def __init__(self, *args, **kwargs):
-        # Keep mapping reference for backwards compatibility
-        self.mapping = ASYNC_DELETE_MAPPING
-
     def delete(self, obj, **kwargs):
         """
         Entry point to delete an object asynchronously.
@@ -2192,18 +2158,10 @@ def delete(self, obj, **kwargs):
 
         dojo_dispatch_task(async_delete_task, obj, **kwargs)
 
-    # Keep helper methods for backwards compatibility and potential direct use
     @staticmethod
     def get_object_name(obj):
         return _get_object_name(obj)
 
-    @staticmethod
-    def chunk_list(model, full_list):
-        chunk_size = get_setting("ASYNC_OBEJECT_DELETE_CHUNK_SIZE")
-        chunk_list = [full_list[i:i + chunk_size] for i in range(0, len(full_list), chunk_size)]
-        logger.debug("ASYNC_DELETE: Split " + _get_object_name(model) + " into " + str(len(chunk_list)) + " chunks of " + str(chunk_size))
-        return chunk_list
-
 
 @receiver(user_logged_in)
 def log_user_login(sender, request, user, **kwargs):

unittests/test_prepare_duplicates_for_delete.py

@@ -104,7 +104,7 @@ def test_no_duplicates(self):
         f2 = self._create_finding(self.test1, "F2")
 
         with impersonate(self.testuser):
-            prepare_duplicates_for_delete(test=self.test1)
+            prepare_duplicates_for_delete(self.test1)
 
         f1.refresh_from_db()
         f2.refresh_from_db()
@@ -120,7 +120,7 @@ def test_inside_scope_duplicates_reset(self):
         self._make_duplicate(dupe, original)
 
         with impersonate(self.testuser):
-            prepare_duplicates_for_delete(test=self.test1)
+            prepare_duplicates_for_delete(self.test1)
 
         dupe.refresh_from_db()
         self.assertIsNone(dupe.duplicate_finding)
@@ -137,7 +137,7 @@ def test_outside_scope_duplicates_get_new_original(self):
         self._make_duplicate(outside_dupe, original)
 
         with impersonate(self.testuser):
-            prepare_duplicates_for_delete(test=self.test1)
+            prepare_duplicates_for_delete(self.test1)
 
         outside_dupe.refresh_from_db()
         # Outside dupe becomes the new original
@@ -158,7 +158,7 @@ def test_outside_scope_cluster_repointed(self):
         self._make_duplicate(dupe_d, original)
 
         with impersonate(self.testuser):
-            prepare_duplicates_for_delete(test=self.test1)
+            prepare_duplicates_for_delete(self.test1)
 
         dupe_b.refresh_from_db()
         dupe_c.refresh_from_db()
@@ -182,7 +182,7 @@ def test_engagement_scope_inside_reset(self):
         self._make_duplicate(dupe, original)
 
         with impersonate(self.testuser):
-            prepare_duplicates_for_delete(engagement=self.engagement1)
+            prepare_duplicates_for_delete(self.engagement1)
 
         dupe.refresh_from_db()
         self.assertIsNone(dupe.duplicate_finding)
@@ -195,7 +195,7 @@ def test_engagement_scope_outside_reconfigure(self):
         self._make_duplicate(outside_dupe, original)
 
         with impersonate(self.testuser):
-            prepare_duplicates_for_delete(engagement=self.engagement1)
+            prepare_duplicates_for_delete(self.engagement1)
 
         outside_dupe.refresh_from_db()
         self.assertFalse(outside_dupe.duplicate)
@@ -210,7 +210,7 @@ def test_mixed_inside_and_outside_duplicates(self):
         self._make_duplicate(outside_dupe, original)
 
         with impersonate(self.testuser):
-            prepare_duplicates_for_delete(test=self.test1)
+            prepare_duplicates_for_delete(self.test1)
 
         inside_dupe.refresh_from_db()
         outside_dupe.refresh_from_db()
@@ -235,7 +235,7 @@ def test_cascade_delete_skips_outside_reconfigure(self):
         self._make_duplicate(outside_dupe, original)
 
         with impersonate(self.testuser):
-            prepare_duplicates_for_delete(test=self.test1)
+            prepare_duplicates_for_delete(self.test1)
 
         outside_dupe.refresh_from_db()
         # Outside dupe is still a duplicate — not reconfigured or deleted
@@ -252,7 +252,7 @@ def test_multiple_originals(self):
         self._make_duplicate(dupe_of_b, original_b)
 
         with impersonate(self.testuser):
-            prepare_duplicates_for_delete(test=self.test1)
+            prepare_duplicates_for_delete(self.test1)
 
         dupe_of_a.refresh_from_db()
         dupe_of_b.refresh_from_db()
@@ -274,7 +274,7 @@ def test_original_status_copied_to_new_original(self):
         self._make_duplicate(outside_dupe, original)
 
         with impersonate(self.testuser):
-            prepare_duplicates_for_delete(test=self.test1)
+            prepare_duplicates_for_delete(self.test1)
 
         outside_dupe.refresh_from_db()
         self.assertFalse(outside_dupe.duplicate)
@@ -292,7 +292,7 @@ def test_found_by_copied_to_new_original(self):
         self._make_duplicate(outside_dupe, original)
 
         with impersonate(self.testuser):
-            prepare_duplicates_for_delete(test=self.test1)
+            prepare_duplicates_for_delete(self.test1)
 
         outside_dupe.refresh_from_db()
         found_by_ids = set(outside_dupe.found_by.values_list("id", flat=True))
@@ -337,7 +337,7 @@ def test_delete_product_with_cross_engagement_duplicates(self):
         Action: delete the entire product via async_delete_crawl_task.
         Expected: product and all findings are deleted without errors.
         """
-        from dojo.utils import ASYNC_DELETE_MAPPING, async_delete_crawl_task
+        from dojo.utils import async_delete_crawl_task
 
         finding_a = self._create_finding(self.test1, "Original A")
         finding_a.active = True
@@ -351,9 +351,8 @@ def test_delete_product_with_cross_engagement_duplicates(self):
         finding_a_id = finding_a.id
         finding_b_id = finding_b.id
 
-        model_list = ASYNC_DELETE_MAPPING["Product"]
         with impersonate(self.testuser):
-            async_delete_crawl_task(self.product, model_list)
+            async_delete_crawl_task(self.product)
 
         # Everything should be gone
         self.assertFalse(Product.objects.filter(id=product_id).exists())