Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

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
  • Loading branch information...
commit 7e92ad8506e6f312b4feb69e74ef17c42678bc05 1 parent 5ccc6f1
@carljm carljm authored
View
4 django/db/models/deletion.py
@@ -144,6 +144,7 @@ def collect(self, objs, source=None, nullable=False, collect_related=True,
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 @@ def collect(self, objs, source=None, nullable=False, collect_related=True,
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)
View
25 django/db/models/options.py
@@ -359,12 +359,15 @@ def get_change_permission(self):
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 @@ def get_all_related_objects_with_model(self, local_only=False,
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 @@ def _fill_related_objects_cache(self):
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:
View
24 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)
+
+
View
98 tests/regressiontests/delete_regress/tests.py
@@ -8,7 +8,7 @@
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 @@ def test_large_deletes(self):
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)
Please sign in to comment.
Something went wrong with that request. Please try again.