Ticket #16128: 16128.diff

django/contrib/contenttypes/models.py

@@ -17 +17 @@
         return ct
 
     def _get_opts(self, model):
-        opts = model._meta
-        while opts.proxy:
-            model = opts.proxy_for_model
-            opts = model._meta
-        return opts
+        return model._meta.concrete_class._meta
 
     def _get_from_cache(self, opts):
         key = (opts.app_label, opts.object_name.lower())

django/db/models/base.py

@@ -122 +122 @@
             if (new_class._meta.local_fields or
                     new_class._meta.local_many_to_many):
                 raise FieldError("Proxy model '%s' contains model fields." % name)
-            while base._meta.proxy:
-                base = base._meta.proxy_for_model
             new_class._meta.setup_proxy(base)
+            new_class._meta.concrete_class = base._meta.concrete_class
+        else:
+            new_class._meta.concrete_class = new_class
 
         # Do the appropriate setup for any model parents.
         o2o_map = dict([(f.rel.to, f) for f in new_class._meta.local_fields
@@ -149 +150 @@
                                         (field.name, name, base.__name__))
             if not base._meta.abstract:
                 # Concrete classes...
-                while base._meta.proxy:
-                    # Skip over a proxy class to the "real" base it proxies.
-                    base = base._meta.proxy_for_model
+                base = base._meta.concrete_class
                 if base in o2o_map:
                     field = o2o_map[base]
                 elif not is_proxy:

django/db/models/deletion.py

@@ -144 +144 @@
                             reverse_dependency=reverse_dependency)
         if not new_objs:
             return
+        
         model = new_objs[0].__class__
 
         # Recursively collect parent models, but not their related objects.
@@ -157 +158 @@
                              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

@@ -40 +40 @@
         self.abstract = False
         self.managed = True
         self.proxy = False
+        # For any class which is a proxy (including automatically created
+        # classes for deferred object loading) the proxy_for_model tells
+        # which class this model is proxying. Note that proxy_for_model
+        # can create a chain of proxy models. For non-proxy models the
+        # variable is always None.
         self.proxy_for_model = None
+        # For any non-abstract class the concrete class is the model
+        # in the end of the proxy_for_model chain. In particular, for
+        # concrete models the concrete_class is always the class itself.
+        self.concrete_class = None
         self.parents = SortedDict()
         self.duplicate_targets = {}
         self.auto_created = False
@@ -350 +359 @@
     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().
@@ -369 +381 @@
             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()
@@ -383 +396 @@
                     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_class == f.rel.to._meta.concrete_class:
+                        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:

django/db/models/sql/query.py

@@ -575 +575 @@
             return
         orig_opts = self.model._meta
         seen = {}
-        if orig_opts.proxy:
-            must_include = {orig_opts.proxy_for_model: set([orig_opts.pk])}
-        else:
-            must_include = {self.model: set([orig_opts.pk])}
+        # NOTE: This does what the previous code did. Is this correct? The
+        # code below is a bit hard to grasp...
+        must_include = {orig_opts.concrete_class: set([orig_opts.pk])}
         for field_name in field_names:
             parts = field_name.split(LOOKUP_SEP)
             cur_model = self.model
@@ -586 +585 @@
             for name in parts[:-1]:
                 old_model = cur_model
                 source = opts.get_field_by_name(name)[0]
-                cur_model = opts.get_field_by_name(name)[0].rel.to
+                cur_model = source.rel.to
                 opts = cur_model._meta
                 # Even if we're "just passing through" this model, we must add
                 # both the current model's pk and the related reference field
@@ -1992 +1991 @@
         data[key] = set([value])
 
 def get_proxied_model(opts):
-    int_opts = opts
-    proxied_model = None
-    while int_opts.proxy:
-        proxied_model = int_opts.proxy_for_model
-        int_opts = proxied_model._meta
-    return proxied_model
+    return opts.concrete_class

tests/modeltests/proxy_models/tests.py

@@ -232 +232 @@
         resp = [u.name for u in UserProxyProxy.objects.all()]
         self.assertEqual(resp, ['Bruce'])
 
+    def test_proxy_for_model(self):
+        self.assertEquals(UserProxy, UserProxyProxy._meta.proxy_for_model)
+
     def test_proxy_delete(self):
         """
         Proxy objects can be deleted

tests/regressiontests/delete_regress/models.py

@@ -67 +67 @@
 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 +8 @@
 
 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 +147 @@
             track = Book.objects.create(pagecount=x+100)
         Book.objects.all().delete()
         self.assertEqual(Book.objects.count(), 0)
+
+
+class ProxyDeleteImageTest(TestCase):
+    '''
+    Tests on_delete behaviour for proxy models. Deleting the *proxy*
+    instance bubbles through to its non-proxy and *all* referring objects
+    are deleted.
+    '''
+
+    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()
+
+    #@override_settings(DEBUG=True)
+    def test_delete(self):
+        self.assertTrue(Image.objects.all().delete() is None)
+        # 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)
+        #from django.db import connection
+        #for q in connection.queries:
+        #    print q
+
+
+class ProxyDeletePhotoTest(ProxyDeleteImageTest):
+    '''
+    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. For some reason it seems that the 
+    intermediate proxy model isn't cleaned up.
+    '''
+
+    def setUp(self):
+        # Create the Image, FooImage and FooFile instances
+        super(ProxyDeletePhotoTest, 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()
+
+
+    #@override_settings(DEBUG=True)
+    def test_delete(self):
+        self.assertTrue(Photo.objects.all().delete() is None)
+        # 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)
+        #from django.db import connection
+        #for q in connection.queries:
+        #    print q
+        #print len(connection.queries)
+
+
+class ProxyDeleteFileTest(ProxyDeleteImageTest):
+    '''
+    Tests on_delete cascade behaviour for proxy models. Deleting the
+    *non-proxy* instance of a model should somehow notify it's proxy.
+    Currently it doesn't, making this test fail.
+    '''
+
+    def test_delete(self):
+        # This should *not* raise an IntegrityError on databases that support 
+        # FK constraints.
+        self.assertTrue(File.objects.all().delete() is None)
+        # 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)