From 272de9eb6baad45abec029aae92c2b7d9478c841 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Fri, 11 Jan 2013 14:12:22 -0800 Subject: [PATCH 1/5] Send post_delete signals immediately In a normal relational construct, if you're listening for an event that signals a child was deleted, you dont expect that the parent was deleted already. This change ensures that post_delete signals are fired immediately after objects are deleted in the graph. --- django/db/models/deletion.py | 22 ++++++++++++---------- tests/modeltests/delete/tests.py | 12 ++++++++++++ 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/django/db/models/deletion.py b/django/db/models/deletion.py index 1c3a582fc5..e4cd89233c 100644 --- a/django/db/models/deletion.py +++ b/django/db/models/deletion.py @@ -75,17 +75,17 @@ class Collector(object): self.using = using # Initially, {model: set([instances])}, later values become lists. self.data = {} - self.field_updates = {} # {model: {(field, value): set([instances])}} + self.field_updates = {} # {model: {(field, value): set([instances])}} # fast_deletes is a list of queryset-likes that can be deleted without # fetching the objects into memory. - self.fast_deletes = [] + self.fast_deletes = [] # Tracks deletion-order dependency for databases without transactions # or ability to defer constraint checks. Only concrete model classes # should be included, as the dependencies exist only between actual # database tables; proxy models are represented here by their concrete # parent. - self.dependencies = {} # {model: set([models])} + self.dependencies = {} # {model: set([models])} def add(self, objs, source=None, nullable=False, reverse_dependency=False): """ @@ -262,6 +262,14 @@ class Collector(object): self.data = SortedDict([(model, self.data[model]) for model in sorted_models]) + def send_post_delete_signals(self, model, instances): + if model._meta.auto_created: + return + for obj in instances: + signals.post_delete.send( + sender=model, instance=obj, using=self.using + ) + @force_managed def delete(self): # sort instance collections @@ -300,13 +308,7 @@ class Collector(object): query = sql.DeleteQuery(model) pk_list = [obj.pk for obj in instances] query.delete_batch(pk_list, self.using) - - # send post_delete signals - for model, obj in self.instances_with_model(): - if not model._meta.auto_created: - signals.post_delete.send( - sender=model, instance=obj, using=self.using - ) + self.send_post_delete_signals(model, instances) # update collected instances for model, instances_for_fieldvalues in six.iteritems(self.field_updates): diff --git a/tests/modeltests/delete/tests.py b/tests/modeltests/delete/tests.py index 20b815c33d..5d7b7a0b33 100644 --- a/tests/modeltests/delete/tests.py +++ b/tests/modeltests/delete/tests.py @@ -229,6 +229,18 @@ class DeletionTests(TestCase): models.signals.post_delete.disconnect(log_post_delete) models.signals.post_delete.disconnect(log_pre_delete) + def test_relational_post_delete_signals_happen_before_parent_object(self): + def log_post_delete(instance, **kwargs): + self.assertTrue(R.objects.filter(pk=instance.r_id)) + + models.signals.post_delete.connect(log_post_delete, sender=S) + + r = R.objects.create(pk=1) + S.objects.create(pk=1, r=r) + r.delete() + + models.signals.post_delete.disconnect(log_post_delete) + @skipUnlessDBFeature("can_defer_constraint_checks") def test_can_defer_constraint_checks(self): u = User.objects.create( From d53e3b15eee6cb89e3a4651b0f89b01fd30360d7 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Fri, 11 Jan 2013 14:43:53 -0800 Subject: [PATCH 2/5] Add David Cramer to AUTHORS --- AUTHORS | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS b/AUTHORS index 4b6636314d..3659d8e0df 100644 --- a/AUTHORS +++ b/AUTHORS @@ -142,6 +142,7 @@ answer newbie questions, and generally made Django that much better: crankycoder@gmail.com Paul Collier Robert Coup + David Cramer Pete Crosier Matt Croydon Jure Cuhalev From 6045efa029dcae58ec6aab20bdcb0dc325851048 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Fri, 11 Jan 2013 14:58:36 -0800 Subject: [PATCH 3/5] Move signal disconnect into finally block --- tests/modeltests/delete/tests.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/modeltests/delete/tests.py b/tests/modeltests/delete/tests.py index 5d7b7a0b33..99981df018 100644 --- a/tests/modeltests/delete/tests.py +++ b/tests/modeltests/delete/tests.py @@ -233,13 +233,15 @@ class DeletionTests(TestCase): def log_post_delete(instance, **kwargs): self.assertTrue(R.objects.filter(pk=instance.r_id)) - models.signals.post_delete.connect(log_post_delete, sender=S) - r = R.objects.create(pk=1) S.objects.create(pk=1, r=r) - r.delete() - models.signals.post_delete.disconnect(log_post_delete) + models.signals.post_delete.connect(log_post_delete, sender=S) + + try: + r.delete() + finally: + models.signals.post_delete.disconnect(log_post_delete) @skipUnlessDBFeature("can_defer_constraint_checks") def test_can_defer_constraint_checks(self): From abbb88886bb1cd6013432c70586fbc118d378e27 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Mon, 14 Jan 2013 13:17:01 -0800 Subject: [PATCH 4/5] Move logic seperation as its not longer repetitive --- django/db/models/deletion.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/django/db/models/deletion.py b/django/db/models/deletion.py index e4cd89233c..81f74923c2 100644 --- a/django/db/models/deletion.py +++ b/django/db/models/deletion.py @@ -262,14 +262,6 @@ class Collector(object): self.data = SortedDict([(model, self.data[model]) for model in sorted_models]) - def send_post_delete_signals(self, model, instances): - if model._meta.auto_created: - return - for obj in instances: - signals.post_delete.send( - sender=model, instance=obj, using=self.using - ) - @force_managed def delete(self): # sort instance collections @@ -308,7 +300,12 @@ class Collector(object): query = sql.DeleteQuery(model) pk_list = [obj.pk for obj in instances] query.delete_batch(pk_list, self.using) - self.send_post_delete_signals(model, instances) + + if not model._meta.auto_created: + for obj in instances: + signals.post_delete.send( + sender=model, instance=obj, using=self.using + ) # update collected instances for model, instances_for_fieldvalues in six.iteritems(self.field_updates): From a7ed09d13d9532089bd2380edab1df5df96082a6 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Mon, 14 Jan 2013 13:18:24 -0800 Subject: [PATCH 5/5] Improve test to ensure that post_delete was actually called --- tests/modeltests/delete/tests.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/modeltests/delete/tests.py b/tests/modeltests/delete/tests.py index 99981df018..0a3ddcfc2e 100644 --- a/tests/modeltests/delete/tests.py +++ b/tests/modeltests/delete/tests.py @@ -230,8 +230,12 @@ class DeletionTests(TestCase): models.signals.post_delete.disconnect(log_pre_delete) def test_relational_post_delete_signals_happen_before_parent_object(self): + deletions = [] + def log_post_delete(instance, **kwargs): self.assertTrue(R.objects.filter(pk=instance.r_id)) + self.assertEquals(type(instance), S) + deletions.append(instance.id) r = R.objects.create(pk=1) S.objects.create(pk=1, r=r) @@ -243,6 +247,9 @@ class DeletionTests(TestCase): finally: models.signals.post_delete.disconnect(log_post_delete) + self.assertEquals(len(deletions), 1) + self.assertEquals(deletions[0], 1) + @skipUnlessDBFeature("can_defer_constraint_checks") def test_can_defer_constraint_checks(self): u = User.objects.create(