summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorCarl Meyer <carl@oddbird.net>2012-03-05 03:41:01 +0000
committerCarl Meyer <carl@oddbird.net>2012-03-05 03:41:01 +0000
commit7e92ad8506e6f312b4feb69e74ef17c42678bc05 (patch)
tree784b3f3b8db5bd063e10014e434a1b860288951f
parent5ccc6f17b8452ef1377755d21148036dc0c88ba0 (diff)
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
-rw-r--r--django/db/models/deletion.py4
-rw-r--r--django/db/models/options.py25
-rw-r--r--tests/regressiontests/delete_regress/models.py24
-rw-r--r--tests/regressiontests/delete_regress/tests.py98
4 files changed, 142 insertions, 9 deletions
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)