From 9eba00a6eea8a42052262b4aceb62089f280a4c9 Mon Sep 17 00:00:00 2001 From: Aghyad Albaghajati Date: Thu, 25 Jul 2024 02:49:22 +0300 Subject: [PATCH] Fix/post delete signal gets called twice for models in a foreign-key relation and cascade deletion constraint (#12) * Extended _has_signal_listeners in BulkTrackerCollector * Updates deletion logic to allow fast_delete * Updates testcases * Updates comments --------- Co-authored-by: hassaanalansary --- CHANGELOG.md | 3 + bulk_tracker/__init__.py | 2 +- bulk_tracker/collector.py | 6 + bulk_tracker/models.py | 10 +- tests/test_delete_signal.py | 268 +++++++++++++++++++++++++----------- 5 files changed, 202 insertions(+), 87 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a2962d8..8c3d55c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ All notable changes to this project will be documented in this file. +## 0.2.1 (2024-07-24) +- A fix where `post_delete_signal()` was called twice for a model in a foreign-key relationship gets deleted with a cascade deletion constraint. + ## 0.2.0 (2024-07-24) - `post_delete_signal()` is now being sent if a parent model was deleted and triggered a cascade delete. diff --git a/bulk_tracker/__init__.py b/bulk_tracker/__init__.py index d3ec452..3ced358 100644 --- a/bulk_tracker/__init__.py +++ b/bulk_tracker/__init__.py @@ -1 +1 @@ -__version__ = "0.2.0" +__version__ = "0.2.1" diff --git a/bulk_tracker/collector.py b/bulk_tracker/collector.py index 6e3664e..c1fc026 100644 --- a/bulk_tracker/collector.py +++ b/bulk_tracker/collector.py @@ -13,6 +13,7 @@ class BulkTrackerCollector(Collector): + def delete(self, *, tracking_info_: TrackingInfo | None = None): # sort instance collections for model, instances in self.data.items(): @@ -29,8 +30,13 @@ def delete(self, *, tracking_info_: TrackingInfo | None = None): if len(self.data) == 1 and len(instances) == 1: instance = list(instances)[0] if self.can_fast_delete(instance): + to_be_deleted = None + if post_delete_signal.has_listeners(model): + to_be_deleted = deepcopy(instance) with transaction.mark_for_rollback_on_error(self.using): count = sql.DeleteQuery(model).delete_batch([instance.pk], self.using) + if to_be_deleted: + send_post_delete_signal([to_be_deleted], model, tracking_info_) setattr(instance, model._meta.pk.attname, None) return count, {model._meta.label: count} diff --git a/bulk_tracker/models.py b/bulk_tracker/models.py index 2312e7a..87977c3 100644 --- a/bulk_tracker/models.py +++ b/bulk_tracker/models.py @@ -6,11 +6,7 @@ from bulk_tracker.collector import BulkTrackerCollector from bulk_tracker.helper_objects import TrackingInfo from bulk_tracker.managers import BulkTrackerManager -from bulk_tracker.signals import ( - send_post_create_signal, - send_post_delete_signal, - send_post_update_signal, -) +from bulk_tracker.signals import send_post_create_signal, send_post_update_signal class BulkTrackerModel(models.Model): @@ -53,6 +49,4 @@ def delete(self, using=None, keep_parents=False, tracking_info_: TrackingInfo | except TypeError: collector = BulkTrackerCollector(using=using) collector.collect([self], keep_parents=keep_parents) - ret = collector.delete(tracking_info_=tracking_info_) - send_post_delete_signal([self], model=self.__class__, tracking_info_=tracking_info_) - return ret + return collector.delete(tracking_info_=tracking_info_) diff --git a/tests/test_delete_signal.py b/tests/test_delete_signal.py index fe56612..01444ce 100644 --- a/tests/test_delete_signal.py +++ b/tests/test_delete_signal.py @@ -72,7 +72,7 @@ def post_delete_receiver( self.assertEqual(2, len(signal_called_with["objects"])) self.assertEqual(None, signal_called_with["tracking_info_"]) - modified_objects: list[ModifiedObject[Post]] = signal_called_with["objects"] + modified_objects: list[ModifiedObject[Post]] = sorted(signal_called_with["objects"], key=lambda o: o.instance.id) self.assertEqual("Sound of Winter", modified_objects[0].instance.title) self.assertEqual(datetime.strptime("1998-01-08", "%Y-%m-%d").date(), modified_objects[0].instance.publish_date) @@ -117,6 +117,15 @@ def post_delete_receiver( @patch("bulk_tracker.signals.post_delete_signal.send_robust") def test_should_use_robust_send_if_is_robust_is_true_in_tracking_info(self, mocked_signal_robust, mocked_signal): # Arrange + def post_delete_receiver( + sender, + objects: list[ModifiedObject[Post]], + tracking_info_: TrackingInfo | None = None, + **kwargs, + ): + pass + + post_delete_signal.connect(post_delete_receiver, sender=Author) # Act self.author_john.delete(tracking_info_=TrackingInfo(is_robust=True)) @@ -127,83 +136,162 @@ def test_should_use_robust_send_if_is_robust_is_true_in_tracking_info(self, mock def test_queryset_delete_should_send_post_delete_signal_for_foreign_keys_with_cascade_if_fast_delete_is_used(self): # Arrange - signal_called_with = {} - - def post_delete_receiver( - sender, - objects: list[ModifiedObject[Post]], - tracking_info_: TrackingInfo | None = None, - **kwargs, + signal_called_with_post = {} + signal_called_with_author = {} + + def post_delete_receiver_post( + sender, + objects: list[ModifiedObject[Post]], + tracking_info_: TrackingInfo | None = None, + **kwargs, ): - signal_called_with["sender"] = sender - signal_called_with["objects"] = objects - signal_called_with["tracking_info_"] = tracking_info_ + signal_called_with_post.setdefault("times_called", 0) + signal_called_with_post["times_called"] += 1 + signal_called_with_post["sender"] = sender + signal_called_with_post["objects"] = objects + signal_called_with_post["tracking_info_"] = tracking_info_ + + def post_delete_receiver_author( + sender, + objects: list[ModifiedObject[Author]], + tracking_info_: TrackingInfo | None = None, + **kwargs, + ): + signal_called_with_author.setdefault("times_called", 0) + signal_called_with_author["times_called"] += 1 + signal_called_with_author["sender"] = sender + signal_called_with_author["objects"] = objects + signal_called_with_author["tracking_info_"] = tracking_info_ - post_delete_signal.connect(post_delete_receiver, sender=Post) - post = Post.objects.create(title="Sound of Winter", publish_date="1998-01-08", author=self.author_john) + def pre_delete_receiver(**kwargs): + pass + + post_delete_signal.connect(post_delete_receiver_author, sender=Author) + post_delete_signal.connect(post_delete_receiver_post, sender=Post) + pre_delete.connect(pre_delete_receiver, sender=Post) # disable fast delete in `Collector` + posts = [Post.objects.create(title="Sound of Winter", publish_date="1998-01-08", author=self.author_john), + Post.objects.create(title="Sound of Summer", publish_date="1998-10-08", author=self.author_john)] # Act - Author.objects.filter(id=self.author_john.id).delete() + author_id = self.author_john.id + Author.objects.filter(id=author_id).delete(tracking_info_=TrackingInfo(comment="This is a comment")) # Assert - modified_objects: list[ModifiedObject[Post]] = signal_called_with["objects"] - self.assertEqual(signal_called_with["sender"], Post) - self.assertEqual(post.id, modified_objects[0].instance.id) - self.assertEqual("Sound of Winter", modified_objects[0].instance.title) - self.assertEqual(datetime.strptime("1998-01-08", "%Y-%m-%d").date(), modified_objects[0].instance.publish_date) - self.assertEqual(self.author_john.id, modified_objects[0].instance.author_id) + modified_objects: list[ModifiedObject[Post]] = sorted(signal_called_with_post["objects"], + key=lambda o: o.instance.id) + self.assertEqual(2, len(modified_objects)) + for i in range(len(posts)): + self.assertEqual(signal_called_with_post["sender"], Post) + self.assertEqual(posts[i].id, modified_objects[i].instance.id) + self.assertEqual(posts[i].title, modified_objects[i].instance.title) + self.assertEqual(datetime.strptime(posts[i].publish_date, "%Y-%m-%d").date(), + modified_objects[i].instance.publish_date) + self.assertEqual(author_id, modified_objects[i].instance.author_id) + self.assertEqual("This is a comment", signal_called_with_post["tracking_info_"].comment) + self.assertEqual(1, signal_called_with_post["times_called"]) + + modified_objects: list[ModifiedObject[Author]] = signal_called_with_author["objects"] + self.assertEqual(author_id, modified_objects[0].instance.id) + self.assertEqual(self.author_john.first_name, modified_objects[0].instance.first_name) + self.assertEqual(self.author_john.last_name, modified_objects[0].instance.last_name) + self.assertEqual("This is a comment", signal_called_with_author["tracking_info_"].comment) + self.assertEqual(1, signal_called_with_author["times_called"]) def test_queryset_delete_should_send_post_delete_signal_for_foreign_keys_with_cascade_if_fast_delete_is_not_used( self, ): # Arrange - signal_called_with = {} - - def pre_delete_receiver(**kwargs): - pass - - def post_delete_receiver( - sender, - objects: list[ModifiedObject[Post]], - tracking_info_: TrackingInfo | None = None, - **kwargs, + signal_called_with_post = {} + signal_called_with_author = {} + + def post_delete_receiver_post( + sender, + objects: list[ModifiedObject[Post]], + tracking_info_: TrackingInfo | None = None, + **kwargs, ): - signal_called_with["sender"] = sender - signal_called_with["objects"] = objects - signal_called_with["tracking_info_"] = tracking_info_ + signal_called_with_post.setdefault("times_called", 0) + signal_called_with_post["times_called"] += 1 + signal_called_with_post["sender"] = sender + signal_called_with_post["objects"] = objects + signal_called_with_post["tracking_info_"] = tracking_info_ + + def post_delete_receiver_author( + sender, + objects: list[ModifiedObject[Author]], + tracking_info_: TrackingInfo | None = None, + **kwargs, + ): + signal_called_with_author.setdefault("times_called", 0) + signal_called_with_author["times_called"] += 1 + signal_called_with_author["sender"] = sender + signal_called_with_author["objects"] = objects + signal_called_with_author["tracking_info_"] = tracking_info_ - pre_delete.connect(pre_delete_receiver, sender=Post) # disable fast delete in `Collector` - post_delete_signal.connect(post_delete_receiver, sender=Post) - post = Post.objects.create(title="Sound of Winter", publish_date="1998-01-08", author=self.author_john) + post_delete_signal.connect(post_delete_receiver_author, sender=Author) + post_delete_signal.connect(post_delete_receiver_post, sender=Post) + + posts = [Post.objects.create(title="Sound of Winter", publish_date="1998-01-08", author=self.author_john), + Post.objects.create(title="Sound of Summer", publish_date="1998-10-08", author=self.author_john)] # Act - Author.objects.filter(id=self.author_john.id).delete() + author_id = self.author_john.id + Author.objects.filter(id=author_id).delete(tracking_info_=TrackingInfo(comment="This is a comment")) # Assert - modified_objects: list[ModifiedObject[Post]] = signal_called_with["objects"] - self.assertEqual(signal_called_with["sender"], Post) - self.assertEqual(post.id, modified_objects[0].instance.id) - self.assertEqual("Sound of Winter", modified_objects[0].instance.title) - self.assertEqual(datetime.strptime("1998-01-08", "%Y-%m-%d").date(), modified_objects[0].instance.publish_date) - self.assertEqual(self.author_john.id, modified_objects[0].instance.author_id) - - def test_model_delete_should_send_post_delete_signal_for_foreign_keys_with_cascade_if_fast_delete_is_used(self): + modified_objects: list[ModifiedObject[Post]] = sorted(signal_called_with_post["objects"], key=lambda o: o.instance.id) + self.assertEqual(2, len(modified_objects)) + for i in range(len(posts)): + self.assertEqual(signal_called_with_post["sender"], Post) + self.assertEqual(posts[i].id, modified_objects[i].instance.id) + self.assertEqual(posts[i].title, modified_objects[i].instance.title) + self.assertEqual(datetime.strptime(posts[i].publish_date, "%Y-%m-%d").date(), modified_objects[i].instance.publish_date) + self.assertEqual(author_id, modified_objects[i].instance.author_id) + self.assertEqual("This is a comment", signal_called_with_post["tracking_info_"].comment) + self.assertEqual(1, signal_called_with_post["times_called"]) + + modified_objects: list[ModifiedObject[Author]] = signal_called_with_author["objects"] + self.assertEqual(author_id, modified_objects[0].instance.id) + self.assertEqual(self.author_john.first_name, modified_objects[0].instance.first_name) + self.assertEqual(self.author_john.last_name, modified_objects[0].instance.last_name) + self.assertEqual("This is a comment", signal_called_with_author["tracking_info_"].comment) + self.assertEqual(1, signal_called_with_author["times_called"]) + + def test_model_delete_should_send_post_delete_signal_for_foreign_keys_if_fast_delete_is_used(self): # Arrange - signal_called_with = {} - - def post_delete_receiver( - sender, - objects: list[ModifiedObject[Post]], - tracking_info_: TrackingInfo | None = None, - **kwargs, + signal_called_with_author = {} + signal_called_with_post = {} + + def post_delete_receiver_post( + sender, + objects: list[ModifiedObject[Post]], + tracking_info_: TrackingInfo | None = None, + **kwargs, ): - signal_called_with.setdefault("times_called", 0) - signal_called_with["times_called"] += 1 - signal_called_with["sender"] = sender - signal_called_with["objects"] = objects - signal_called_with["tracking_info_"] = tracking_info_ + signal_called_with_post.setdefault("times_called", 0) + signal_called_with_post["times_called"] += 1 + signal_called_with_post["sender"] = sender + signal_called_with_post["objects"] = objects + signal_called_with_post["tracking_info_"] = tracking_info_ + + def post_delete_receiver_author( + sender, + objects: list[ModifiedObject[Author]], + tracking_info_: TrackingInfo | None = None, + **kwargs, + ): + signal_called_with_author.setdefault("times_called", 0) + signal_called_with_author["times_called"] += 1 + signal_called_with_author["sender"] = sender + signal_called_with_author["objects"] = objects + signal_called_with_author["tracking_info_"] = tracking_info_ - post_delete_signal.connect(post_delete_receiver, sender=Post) + def pre_delete_receiver(**kwargs): + pass + + post_delete_signal.connect(post_delete_receiver_post, sender=Post) + post_delete_signal.connect(post_delete_receiver_author, sender=Author) + pre_delete.connect(pre_delete_receiver, sender=Post) # disable fast delete in `Collector` post = Post.objects.create(title="Sound of Winter", publish_date="1998-01-08", author=self.author_john) author_id = self.author_john.id @@ -211,36 +299,53 @@ def post_delete_receiver( self.author_john.delete(tracking_info_=TrackingInfo(comment="This is a comment")) # Assert - modified_objects: list[ModifiedObject[Post]] = signal_called_with["objects"] - self.assertEqual(signal_called_with["sender"], Post) + modified_objects: list[ModifiedObject[Post]] = signal_called_with_post["objects"] + self.assertEqual(signal_called_with_post["sender"], Post) self.assertEqual(post.id, modified_objects[0].instance.id) self.assertEqual("Sound of Winter", modified_objects[0].instance.title) self.assertEqual(datetime.strptime("1998-01-08", "%Y-%m-%d").date(), modified_objects[0].instance.publish_date) self.assertEqual(author_id, modified_objects[0].instance.author_id) - self.assertEqual("This is a comment", signal_called_with["tracking_info_"].comment) - self.assertEqual(1, signal_called_with["times_called"]) + self.assertEqual("This is a comment", signal_called_with_post["tracking_info_"].comment) + self.assertEqual(1, signal_called_with_post["times_called"]) + + modified_objects: list[ModifiedObject[Author]] = signal_called_with_author["objects"] + self.assertEqual(author_id, modified_objects[0].instance.id) + self.assertEqual(self.author_john.first_name, modified_objects[0].instance.first_name) + self.assertEqual(self.author_john.last_name, modified_objects[0].instance.last_name) + self.assertEqual("This is a comment", signal_called_with_author["tracking_info_"].comment) + self.assertEqual(1, signal_called_with_author["times_called"]) def test_model_delete_should_send_post_delete_signal_for_foreign_keys_with_cascade_if_fast_delete_is_not_used(self): # Arrange - signal_called_with = {} - - def pre_delete_receiver(**kwargs): - pass + signal_called_with_author = {} + signal_called_with_post = {} - def post_delete_receiver( + def post_delete_receiver_post( sender, objects: list[ModifiedObject[Post]], tracking_info_: TrackingInfo | None = None, **kwargs, ): - signal_called_with.setdefault("times_called", 0) - signal_called_with["times_called"] += 1 - signal_called_with["sender"] = sender - signal_called_with["objects"] = objects - signal_called_with["tracking_info_"] = tracking_info_ + signal_called_with_post.setdefault("times_called", 0) + signal_called_with_post["times_called"] += 1 + signal_called_with_post["sender"] = sender + signal_called_with_post["objects"] = objects + signal_called_with_post["tracking_info_"] = tracking_info_ - pre_delete.connect(pre_delete_receiver, sender=Post) # disable fast delete in `Collector` - post_delete_signal.connect(post_delete_receiver, sender=Post) + def post_delete_receiver_author( + sender, + objects: list[ModifiedObject[Author]], + tracking_info_: TrackingInfo | None = None, + **kwargs, + ): + signal_called_with_author.setdefault("times_called", 0) + signal_called_with_author["times_called"] += 1 + signal_called_with_author["sender"] = sender + signal_called_with_author["objects"] = objects + signal_called_with_author["tracking_info_"] = tracking_info_ + + post_delete_signal.connect(post_delete_receiver_post, sender=Post) + post_delete_signal.connect(post_delete_receiver_author, sender=Author) post = Post.objects.create(title="Sound of Winter", publish_date="1998-01-08", author=self.author_john) author_id = self.author_john.id @@ -248,11 +353,18 @@ def post_delete_receiver( self.author_john.delete(tracking_info_=TrackingInfo(comment="This is a comment")) # Assert - modified_objects: list[ModifiedObject[Post]] = signal_called_with["objects"] - self.assertEqual(signal_called_with["sender"], Post) + modified_objects: list[ModifiedObject[Post]] = signal_called_with_post["objects"] + self.assertEqual(signal_called_with_post["sender"], Post) self.assertEqual(post.id, modified_objects[0].instance.id) self.assertEqual("Sound of Winter", modified_objects[0].instance.title) self.assertEqual(datetime.strptime("1998-01-08", "%Y-%m-%d").date(), modified_objects[0].instance.publish_date) self.assertEqual(author_id, modified_objects[0].instance.author_id) - self.assertEqual("This is a comment", signal_called_with["tracking_info_"].comment) - self.assertEqual(1, signal_called_with["times_called"]) + self.assertEqual("This is a comment", signal_called_with_post["tracking_info_"].comment) + self.assertEqual(1, signal_called_with_post["times_called"]) + + modified_objects: list[ModifiedObject[Author]] = signal_called_with_author["objects"] + self.assertEqual(author_id, modified_objects[0].instance.id) + self.assertEqual(self.author_john.first_name, modified_objects[0].instance.first_name) + self.assertEqual(self.author_john.last_name, modified_objects[0].instance.last_name) + self.assertEqual("This is a comment", signal_called_with_author["tracking_info_"].comment) + self.assertEqual(1, signal_called_with_author["times_called"])