From 4900f0447e2cac622c7578f4fff886c82dffe7f0 Mon Sep 17 00:00:00 2001
From: Klaas van Schelven <klaas@vanschelven.com>
Date: Thu, 3 Jul 2025 22:04:51 +0200
Subject: [PATCH] Project-deletion: slight optimization

Removes the following 2 redundant queries from the deletion process:

```
SELECT "tags_tagkey"."id" FROM "tags_tagkey" WHERE "tags_tagkey"."project_id" IN (1) ORDER BY "tags_tagkey"."project_id" ASC, "tags_tagkey"."id" ASC LIMIT 498
UPDATE "projects_project" SET "stored_event_count" = ("projects_project"."stored_event_count" - 1) WHERE "projects_project"."id" = 1
```
---
 bugsink/utils.py  | 15 ++++++++++++---
 issues/tasks.py   |  1 +
 projects/tasks.py |  1 +
 projects/tests.py |  2 +-
 4 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/bugsink/utils.py b/bugsink/utils.py
index 7395733..8583bd4 100644
--- a/bugsink/utils.py
+++ b/bugsink/utils.py
@@ -231,7 +231,7 @@ def prune_orphans(model, d_ids_to_check):
     # vacuuming once in a while" is a much better fit for that.
 
 
-def do_pre_delete(project_id, model, pks_to_delete):
+def do_pre_delete(project_id, model, pks_to_delete, is_for_project):
     "More model-specific cleanup, if needed; only for Event model at the moment."
 
     if model.__name__ != "Event":
@@ -246,11 +246,15 @@ def do_pre_delete(project_id, model, pks_to_delete):
         .values_list("id", "storage_backend")
     )
 
+    if is_for_project:
+        # no need to update the stored_event_count for the project, because the project is being deleted
+        return
+
     # note: don't bother to do the same thing for Issue.stored_event_count, since we're in the process of deleting Issue
     Project.objects.filter(id=project_id).update(stored_event_count=F('stored_event_count') - len(pks_to_delete))
 
 
-def delete_deps_with_budget(project_id, referring_model, fk_name, referred_ids, budget, dep_graph):
+def delete_deps_with_budget(project_id, referring_model, fk_name, referred_ids, budget, dep_graph, is_for_project):
     r"""
     Deletes all objects of type referring_model that refer to any of the referred_ids via fk_name.
     Returns the number of deleted objects.
@@ -291,6 +295,7 @@ def delete_deps_with_budget(project_id, referring_model, fk_name, referred_ids,
             [d["pk"] for d in relevant_ids],
             budget - num_deleted,
             dep_graph,
+            is_for_project,
         )
 
         if num_deleted >= budget:
@@ -300,12 +305,16 @@ def delete_deps_with_budget(project_id, referring_model, fk_name, referred_ids,
     # left. We can now delete the referring objects themselves (limited by budget).
     relevant_ids_after_rec = relevant_ids[:budget - num_deleted]
 
-    do_pre_delete(project_id, referring_model, [d['pk'] for d in relevant_ids_after_rec])
+    do_pre_delete(project_id, referring_model, [d['pk'] for d in relevant_ids_after_rec], is_for_project)
 
     my_num_deleted, del_d = referring_model.objects.filter(pk__in=[d['pk'] for d in relevant_ids_after_rec]).delete()
     num_deleted += my_num_deleted
     assert set(del_d.keys()) == {referring_model._meta.label}  # assert no-cascading (we do that ourselves)
 
+    if is_for_project:
+        # short-circuit: project-deletion implies "no orphans" because the project kill everything with it.
+        return num_deleted
+
     # Note that prune_orphans doesn't respect the budget. Reason: it's not easy to do, b/c the order is reversed (we
     # would need to predict somehow at the previous step how much budget to leave unused) and we don't care _that much_
     # about a precise budget "at the edges of our algo", as long as we don't have a "single huge blocking thing".
diff --git a/issues/tasks.py b/issues/tasks.py
index af95743..ba6fe9b 100644
--- a/issues/tasks.py
+++ b/issues/tasks.py
@@ -64,6 +64,7 @@ def delete_issue_deps(project_id, issue_id):
                 [issue_id],
                 budget - num_deleted,
                 dep_graph,
+                is_for_project=False,
             )
 
             num_deleted += this_num_deleted
diff --git a/projects/tasks.py b/projects/tasks.py
index e060ea2..5b51b28 100644
--- a/projects/tasks.py
+++ b/projects/tasks.py
@@ -123,6 +123,7 @@ def delete_project_deps(project_id):
                 [project_id],
                 budget - num_deleted,
                 dep_graph,
+                is_for_project=True,
             )
 
             num_deleted += this_num_deleted
diff --git a/projects/tests.py b/projects/tests.py
index 182a0a0..986209a 100644
--- a/projects/tests.py
+++ b/projects/tests.py
@@ -66,7 +66,7 @@ class ProjectDeletionTestCase(TransactionTestCase):
         # correct for bugsink/transaction.py's select_for_update for non-sqlite databases
         correct_for_select_for_update = 1 if 'sqlite' not in settings.DATABASES['default']['ENGINE'] else 0
 
-        with self.assertNumQueries(29 + correct_for_select_for_update):
+        with self.assertNumQueries(27 + correct_for_select_for_update):
             self.project.delete_deferred()
 
         # tests run w/ TASK_ALWAYS_EAGER, so in the below we can just check the database directly