diff --git a/django/db/models/deletion.py b/django/db/models/deletion.py index 9a6d49955a..7d6594afc0 100644 --- a/django/db/models/deletion.py +++ b/django/db/models/deletion.py @@ -144,6 +144,7 @@ class Collector(object): reverse_dependency=reverse_dependency) if not new_objs: return + model = new_objs[0].__class__ # Recursively collect parent models, but not their related objects. @@ -157,7 +158,8 @@ class Collector(object): reverse_dependency=True) if collect_related: - for related in model._meta.get_all_related_objects(include_hidden=True): + for related in model._meta.get_all_related_objects( + include_hidden=True, include_proxy_eq=True): field = related.field if related.model._meta.auto_created: self.add_batch(related.model, field, new_objs) diff --git a/django/db/models/options.py b/django/db/models/options.py index e13a4967cf..44f8891942 100644 --- a/django/db/models/options.py +++ b/django/db/models/options.py @@ -359,12 +359,15 @@ class Options(object): def get_delete_permission(self): return 'delete_%s' % self.object_name.lower() - def get_all_related_objects(self, local_only=False, include_hidden=False): + def get_all_related_objects(self, local_only=False, include_hidden=False, + include_proxy_eq=False): return [k for k, v in self.get_all_related_objects_with_model( - local_only=local_only, include_hidden=include_hidden)] + local_only=local_only, include_hidden=include_hidden, + include_proxy_eq=include_proxy_eq)] def get_all_related_objects_with_model(self, local_only=False, - include_hidden=False): + include_hidden=False, + include_proxy_eq=False): """ Returns a list of (related-object, model) pairs. Similar to get_fields_with_model(). @@ -378,8 +381,9 @@ class Options(object): predicates.append(lambda k, v: not v) if not include_hidden: predicates.append(lambda k, v: not k.field.rel.is_hidden()) - return filter(lambda t: all([p(*t) for p in predicates]), - self._related_objects_cache.items()) + cache = (self._related_objects_proxy_cache if include_proxy_eq + else self._related_objects_cache) + return filter(lambda t: all([p(*t) for p in predicates]), cache.items()) def _fill_related_objects_cache(self): cache = SortedDict() @@ -392,11 +396,18 @@ class Options(object): cache[obj] = parent else: cache[obj] = model + # Collect also objects which are in relation to some proxy child/parent of self. + proxy_cache = cache.copy() for klass in get_models(include_auto_created=True, only_installed=False): for f in klass._meta.local_fields: - if f.rel and not isinstance(f.rel.to, basestring) and self == f.rel.to._meta: - cache[RelatedObject(f.rel.to, klass, f)] = None + if f.rel and not isinstance(f.rel.to, basestring): + if self == f.rel.to._meta: + cache[RelatedObject(f.rel.to, klass, f)] = None + proxy_cache[RelatedObject(f.rel.to, klass, f)] = None + elif self.concrete_model == f.rel.to._meta.concrete_model: + proxy_cache[RelatedObject(f.rel.to, klass, f)] = None self._related_objects_cache = cache + self._related_objects_proxy_cache = proxy_cache def get_all_related_many_to_many_objects(self, local_only=False): try: diff --git a/tests/regressiontests/delete_regress/models.py b/tests/regressiontests/delete_regress/models.py index 622fbc8104..214452c97b 100644 --- a/tests/regressiontests/delete_regress/models.py +++ b/tests/regressiontests/delete_regress/models.py @@ -67,3 +67,27 @@ class Location(models.Model): class Item(models.Model): version = models.ForeignKey(Version) location = models.ForeignKey(Location, blank=True, null=True) + +# Models for #16128 + +class File(models.Model): + pass + +class Image(File): + class Meta: + proxy = True + +class Photo(Image): + class Meta: + proxy = True + +class FooImage(models.Model): + my_image = models.ForeignKey(Image) + +class FooFile(models.Model): + my_file = models.ForeignKey(File) + +class FooPhoto(models.Model): + my_photo = models.ForeignKey(Photo) + + diff --git a/tests/regressiontests/delete_regress/tests.py b/tests/regressiontests/delete_regress/tests.py index c2a2171d89..8ef00e92a5 100644 --- a/tests/regressiontests/delete_regress/tests.py +++ b/tests/regressiontests/delete_regress/tests.py @@ -8,7 +8,7 @@ from django.test import TestCase, TransactionTestCase, skipUnlessDBFeature from .models import (Book, Award, AwardNote, Person, Child, Toy, PlayedWith, PlayedWithNote, Email, Researcher, Food, Eaten, Policy, Version, Location, - Item) + Item, Image, File, Photo, FooFile, FooImage, FooPhoto) # Can't run this test under SQLite, because you can't @@ -147,3 +147,99 @@ class LargeDeleteTests(TestCase): track = Book.objects.create(pagecount=x+100) Book.objects.all().delete() self.assertEqual(Book.objects.count(), 0) + + + +class ProxyDeleteTest(TestCase): + """ + Tests on_delete behaviour for proxy models. Deleting the *proxy* + instance bubbles through to its non-proxy and *all* referring objects + are deleted. + + See #16128. + + """ + + def setUp(self): + # Create an Image + self.test_image = Image() + self.test_image.save() + foo_image = FooImage(my_image=self.test_image) + foo_image.save() + + # Get the Image instance as a File + test_file = File.objects.get(pk=self.test_image.pk) + foo_file = FooFile(my_file=test_file) + foo_file.save() + + + def test_delete(self): + Image.objects.all().delete() + + # An Image deletion == File deletion + self.assertEqual(len(Image.objects.all()), 0) + self.assertEqual(len(File.objects.all()), 0) + + # The Image deletion cascaded and *all* references to it are deleted. + self.assertEqual(len(FooImage.objects.all()), 0) + self.assertEqual(len(FooFile.objects.all()), 0) + + + +class ProxyOfProxyDeleteTest(ProxyDeleteTest): + """ + Tests on_delete behaviour for proxy-of-proxy models. Deleting the *proxy* + instance should bubble through to its proxy and non-proxy variants. + Deleting *all* referring objects. + + See #16128. + + """ + + def setUp(self): + # Create the Image, FooImage and FooFile instances + super(ProxyOfProxyDeleteTest, self).setUp() + + # Get the Image as a Photo + test_photo = Photo.objects.get(pk=self.test_image.pk) + foo_photo = FooPhoto(my_photo=test_photo) + foo_photo.save() + + + def test_delete(self): + Photo.objects.all().delete() + + # A Photo deletion == Image deletion == File deletion + self.assertEqual(len(Photo.objects.all()), 0) + self.assertEqual(len(Image.objects.all()), 0) + self.assertEqual(len(File.objects.all()), 0) + + # The Photo deletion should have cascaded and deleted *all* + # references to it. + self.assertEqual(len(FooPhoto.objects.all()), 0) + self.assertEqual(len(FooFile.objects.all()), 0) + self.assertEqual(len(FooImage.objects.all()), 0) + + + +class ProxyParentDeleteTest(ProxyDeleteTest): + """ + Tests on_delete cascade behaviour for proxy models. Deleting the + *non-proxy* instance of a model should delete objects referencing the + proxy. + + See #16128. + + """ + + def test_delete(self): + File.objects.all().delete() + + # A File deletion == Image deletion + self.assertEqual(len(File.objects.all()), 0) + self.assertEqual(len(Image.objects.all()), 0) + + # The File deletion should have cascaded and deleted *all* references + # to it. + self.assertEqual(len(FooFile.objects.all()), 0) + self.assertEqual(len(FooImage.objects.all()), 0)