Async Delete: Handle exceptions for duplicate requests (#12867)

* Async Delete: Handle exceptions for duplicate requests

* correct ruff errors

* Apply suggestions from code review

Co-authored-by: Blake Owens <76979297+blakeaowens@users.noreply.github.com>

---------

Co-authored-by: Blake Owens <76979297+blakeaowens@users.noreply.github.com>

Author: Maffooch

dojo/engagement/signals.py

@@ -1,3 +1,5 @@
+import contextlib
+
 from auditlog.models import LogEntry
 from django.conf import settings
 from django.contrib.contenttypes.models import ContentType
@@ -6,7 +8,7 @@
 from django.urls import reverse
 from django.utils.translation import gettext as _
 
-from dojo.models import Engagement
+from dojo.models import Engagement, Product
 from dojo.notes.helper import delete_related_notes
 from dojo.notifications.helper import create_notification
 
@@ -38,25 +40,28 @@ def engagement_pre_save(sender, instance, **kwargs):
 
 @receiver(post_delete, sender=Engagement)
 def engagement_post_delete(sender, instance, using, origin, **kwargs):
-    if instance == origin:
-        description = _('The engagement "%(name)s" was deleted') % {"name": instance.name}
-        if settings.ENABLE_AUDITLOG:
-            if le := LogEntry.objects.filter(
-                action=LogEntry.Action.DELETE,
-                content_type=ContentType.objects.get(app_label="dojo", model="engagement"),
-                object_id=instance.id,
-            ).order_by("-id").first():
-                description = _('The engagement "%(name)s" was deleted by %(user)s') % {
-                                "name": instance.name, "user": le.actor}
-        create_notification(event="engagement_deleted",  # template does not exists, it will default to "other" but this event name needs to stay because of unit testing
-                            title=_("Deletion of %(name)s") % {"name": instance.name},
-                            description=description,
-                            product=instance.product,
-                            url=reverse("view_product", args=(instance.product.id, )),
-                            recipients=[instance.lead],
-                            icon="exclamation-triangle")
+    # Catch instances in async delete where a single object is deleted more than once
+    with contextlib.suppress(sender.DoesNotExist, Product.DoesNotExist):
+        if instance == origin:
+            description = _('The engagement "%(name)s" was deleted') % {"name": instance.name}
+            if settings.ENABLE_AUDITLOG:
+                if le := LogEntry.objects.filter(
+                    action=LogEntry.Action.DELETE,
+                    content_type=ContentType.objects.get(app_label="dojo", model="engagement"),
+                    object_id=instance.id,
+                ).order_by("-id").first():
+                    description = _('The engagement "%(name)s" was deleted by %(user)s') % {
+                                    "name": instance.name, "user": le.actor}
+            create_notification(event="engagement_deleted",  # template does not exists, it will default to "other" but this event name needs to stay because of unit testing
+                                title=_("Deletion of %(name)s") % {"name": instance.name},
+                                description=description,
+                                product=instance.product,
+                                url=reverse("view_product", args=(instance.product.id, )),
+                                recipients=[instance.lead],
+                                icon="exclamation-triangle")
 
 
 @receiver(pre_delete, sender=Engagement)
 def engagement_pre_delete(sender, instance, **kwargs):
-    delete_related_notes(instance)
+    with contextlib.suppress(sender.DoesNotExist):
+        delete_related_notes(instance)

dojo/finding/helper.py

@@ -1,4 +1,5 @@
 import logging
+from contextlib import suppress
 from time import strftime
 
 from django.conf import settings
@@ -446,8 +447,10 @@ def finding_delete(instance, **kwargs):
 
 @receiver(post_delete, sender=Finding)
 def finding_post_delete(sender, instance, **kwargs):
-    logger.debug("finding post_delete, sender: %s instance: %s", to_str_typed(sender), to_str_typed(instance))
-    # calculate_grade(instance.test.engagement.product)
+    # Catch instances in async delete where a single object is deleted more than once
+    with suppress(Finding.DoesNotExist):
+        logger.debug("finding post_delete, sender: %s instance: %s", to_str_typed(sender), to_str_typed(instance))
+        # calculate_grade(instance.test.engagement.product)
 
 
 def reset_duplicate_before_delete(dupe):

dojo/models.py

@@ -1631,7 +1631,7 @@ def delete(self, *args, **kwargs):
         from dojo.finding import helper
         helper.prepare_duplicates_for_delete(engagement=self)
         super().delete(*args, **kwargs)
-        with suppress(Product.DoesNotExist):
+        with suppress(Engagement.DoesNotExist, Product.DoesNotExist):
             # Suppressing a potential issue created from async delete removing
             # related objects in a separate task
             calculate_grade(self.product)
@@ -2239,7 +2239,7 @@ def hash_code_allows_null_cwe(self):
     def delete(self, *args, **kwargs):
         logger.debug("%d test delete", self.id)
         super().delete(*args, **kwargs)
-        with suppress(Engagement.DoesNotExist, Product.DoesNotExist):
+        with suppress(Test.DoesNotExist, Engagement.DoesNotExist, Product.DoesNotExist):
             # Suppressing a potential issue created from async delete removing
             # related objects in a separate task
             calculate_grade(self.engagement.product)
@@ -2821,7 +2821,7 @@ def delete(self, *args, **kwargs):
         from dojo.finding import helper
         helper.finding_delete(self)
         super().delete(*args, **kwargs)
-        with suppress(Test.DoesNotExist, Engagement.DoesNotExist, Product.DoesNotExist):
+        with suppress(Finding.DoesNotExist, Test.DoesNotExist, Engagement.DoesNotExist, Product.DoesNotExist):
             # Suppressing a potential issue created from async delete removing
             # related objects in a separate task
             calculate_grade(self.test.engagement.product)

dojo/product/signals.py

@@ -1,3 +1,5 @@
+import contextlib
+
 from auditlog.models import LogEntry
 from django.conf import settings
 from django.contrib.contenttypes.models import ContentType
@@ -23,17 +25,19 @@ def product_post_save(sender, instance, created, **kwargs):
 
 @receiver(post_delete, sender=Product)
 def product_post_delete(sender, instance, **kwargs):
-    description = _('The product "%(name)s" was deleted') % {"name": instance.name}
-    if settings.ENABLE_AUDITLOG:
-        if le := LogEntry.objects.filter(
-            action=LogEntry.Action.DELETE,
-            content_type=ContentType.objects.get(app_label="dojo", model="product"),
-            object_id=instance.id,
-        ).order_by("-id").first():
-            description = _('The product "%(name)s" was deleted by %(user)s') % {
-                            "name": instance.name, "user": le.actor}
-    create_notification(event="product_deleted",  # template does not exists, it will default to "other" but this event name needs to stay because of unit testing
-                        title=_("Deletion of %(name)s") % {"name": instance.name},
-                        description=description,
-                        url=reverse("product"),
-                        icon="exclamation-triangle")
+    # Catch instances in async delete where a single object is deleted more than once
+    with contextlib.suppress(sender.DoesNotExist):
+        description = _('The product "%(name)s" was deleted') % {"name": instance.name}
+        if settings.ENABLE_AUDITLOG:
+            if le := LogEntry.objects.filter(
+                action=LogEntry.Action.DELETE,
+                content_type=ContentType.objects.get(app_label="dojo", model="product"),
+                object_id=instance.id,
+            ).order_by("-id").first():
+                description = _('The product "%(name)s" was deleted by %(user)s') % {
+                                "name": instance.name, "user": le.actor}
+        create_notification(event="product_deleted",  # template does not exists, it will default to "other" but this event name needs to stay because of unit testing
+                            title=_("Deletion of %(name)s") % {"name": instance.name},
+                            description=description,
+                            url=reverse("product"),
+                            icon="exclamation-triangle")

dojo/product_type/signals.py

@@ -1,3 +1,5 @@
+import contextlib
+
 from auditlog.models import LogEntry
 from django.conf import settings
 from django.contrib.contenttypes.models import ContentType
@@ -23,18 +25,20 @@ def product_type_post_save(sender, instance, created, **kwargs):
 
 @receiver(post_delete, sender=Product_Type)
 def product_type_post_delete(sender, instance, **kwargs):
-    description = _('The product type "%(name)s" was deleted') % {"name": instance.name}
-    if settings.ENABLE_AUDITLOG:
-        if le := LogEntry.objects.filter(
-            action=LogEntry.Action.DELETE,
-            content_type=ContentType.objects.get(app_label="dojo", model="product_type"),
-            object_id=instance.id,
-        ).order_by("-id").first():
-            description = _('The product type "%(name)s" was deleted by %(user)s') % {
-                            "name": instance.name, "user": le.actor}
-    create_notification(event="product_type_deleted",  # template does not exists, it will default to "other" but this event name needs to stay because of unit testing
-                        title=_("Deletion of %(name)s") % {"name": instance.name},
-                        description=description,
-                        no_users=True,
-                        url=reverse("product_type"),
-                        icon="exclamation-triangle")
+    # Catch instances in async delete where a single object is deleted more than once
+    with contextlib.suppress(sender.DoesNotExist):
+        description = _('The product type "%(name)s" was deleted') % {"name": instance.name}
+        if settings.ENABLE_AUDITLOG:
+            if le := LogEntry.objects.filter(
+                action=LogEntry.Action.DELETE,
+                content_type=ContentType.objects.get(app_label="dojo", model="product_type"),
+                object_id=instance.id,
+            ).order_by("-id").first():
+                description = _('The product type "%(name)s" was deleted by %(user)s') % {
+                                "name": instance.name, "user": le.actor}
+        create_notification(event="product_type_deleted",  # template does not exists, it will default to "other" but this event name needs to stay because of unit testing
+                            title=_("Deletion of %(name)s") % {"name": instance.name},
+                            description=description,
+                            no_users=True,
+                            url=reverse("product_type"),
+                            icon="exclamation-triangle")

dojo/test/signals.py

@@ -8,30 +8,32 @@
 from django.urls import reverse
 from django.utils.translation import gettext as _
 
-from dojo.models import Finding, Test
+from dojo.models import Engagement, Finding, Product, Test
 from dojo.notes.helper import delete_related_notes
 from dojo.notifications.helper import create_notification
 
 
 @receiver(post_delete, sender=Test)
 def test_post_delete(sender, instance, using, origin, **kwargs):
-    if instance == origin:
-        description = _('The test "%(name)s" was deleted') % {"name": str(instance)}
-        if settings.ENABLE_AUDITLOG:
-            if le := LogEntry.objects.filter(
-                action=LogEntry.Action.DELETE,
-                content_type=ContentType.objects.get(app_label="dojo", model="test"),
-                object_id=instance.id,
-            ).order_by("-id").first():
-                description = _('The test "%(name)s" was deleted by %(user)s') % {
-                                "name": str(instance), "user": le.actor}
-        create_notification(event="test_deleted",  # template does not exists, it will default to "other" but this event name needs to stay because of unit testing
-                            title=_("Deletion of %(name)s") % {"name": str(instance)},
-                            description=description,
-                            product=instance.engagement.product,
-                            url=reverse("view_engagement", args=(instance.engagement.id, )),
-                            recipients=[instance.engagement.lead],
-                            icon="exclamation-triangle")
+    # Catch instances in async delete where a single object is deleted more than once
+    with contextlib.suppress(sender.DoesNotExist, Engagement.DoesNotExist, Product.DoesNotExist):
+        if instance == origin:
+            description = _('The test "%(name)s" was deleted') % {"name": str(instance)}
+            if settings.ENABLE_AUDITLOG:
+                if le := LogEntry.objects.filter(
+                    action=LogEntry.Action.DELETE,
+                    content_type=ContentType.objects.get(app_label="dojo", model="test"),
+                    object_id=instance.id,
+                ).order_by("-id").first():
+                    description = _('The test "%(name)s" was deleted by %(user)s') % {
+                                    "name": str(instance), "user": le.actor}
+            create_notification(event="test_deleted",  # Template does not exist, it will default to "other" but this event name needs to stay because of unit testing
+                                title=_("Deletion of %(name)s") % {"name": str(instance)},
+                                description=description,
+                                product=instance.engagement.product,
+                                url=reverse("view_engagement", args=(instance.engagement.id, )),
+                                recipients=[instance.engagement.lead],
+                                icon="exclamation-triangle")
 
 
 @receiver(pre_save, sender=Test)
@@ -53,4 +55,5 @@ def update_found_by_for_findings(sender, instance, **kwargs):
 
 @receiver(pre_delete, sender=Test)
 def test_pre_delete(sender, instance, **kwargs):
-    delete_related_notes(instance)
+    with contextlib.suppress(sender.DoesNotExist):
+        delete_related_notes(instance)

dojo/utils.py

@@ -2367,7 +2367,8 @@ def crawl(self, obj, model_list, **kwargs):
             model = model_info[0]
             model_query = model_info[1]
             filter_dict = {model_query: obj}
-            objects_to_delete = model.objects.filter(**filter_dict)
+            # Only fetch the IDs since we will make a list of IDs in the following function call
+            objects_to_delete = model.objects.only("id").filter(**filter_dict)
             logger.debug("ASYNC_DELETE: Deleting " + str(len(objects_to_delete)) + " " + self.get_object_name(model) + "s in chunks")
             chunks = self.chunk_list(model, objects_to_delete)
             for chunk in chunks: