Fixed #16128 - Correctly cascade-delete proxy models as if they were the…

Fixed #16128 - Correctly cascade-delete proxy models as if they were the concrete model class. Thanks xkennyx for the report, and Aymeric Augustin, Claude Paroz, Adam Nelson, jaap3, and Anssi Kääriäinen for work on the patch.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@17664 bcc190cf-cafb-0310-a4f2-bffc1f526a37

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)

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:

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)
+
+

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)