Code

Ticket #16128: 16128.6.diff

File 16128.6.diff, 13.4 KB (added by akaariai, 2 years ago)
Line 
1diff --git a/django/contrib/contenttypes/models.py b/django/contrib/contenttypes/models.py
2index 1bb07e0..bc9de56 100644
3--- a/django/contrib/contenttypes/models.py
4+++ b/django/contrib/contenttypes/models.py
5@@ -16,11 +16,10 @@ class ContentTypeManager(models.Manager):
6         return ct
7 
8     def _get_opts(self, model):
9-        opts = model._meta
10-        while opts.proxy:
11-            model = opts.proxy_for_model
12-            opts = model._meta
13-        return opts
14+        """
15+        Returns the options for the concrete parent of the model.
16+        """
17+        return model._meta.concrete_class._meta
18 
19     def _get_from_cache(self, opts):
20         key = (opts.app_label, opts.object_name.lower())
21diff --git a/django/db/models/base.py b/django/db/models/base.py
22index b5ce39e..033bf37 100644
23--- a/django/db/models/base.py
24+++ b/django/db/models/base.py
25@@ -75,6 +75,8 @@ class ModelBase(type):
26                     new_class._meta.get_latest_by = base_meta.get_latest_by
27 
28         is_proxy = new_class._meta.proxy
29+        if not is_proxy and not abstract:
30+            new_class._meta.concrete_class = new_class
31 
32         if getattr(new_class, '_default_manager', None):
33             if not is_proxy:
34@@ -122,9 +124,12 @@ class ModelBase(type):
35             if (new_class._meta.local_fields or
36                     new_class._meta.local_many_to_many):
37                 raise FieldError("Proxy model '%s' contains model fields." % name)
38-            while base._meta.proxy:
39-                base = base._meta.proxy_for_model
40             new_class._meta.setup_proxy(base)
41+            # Add this class to 'proxying_models' for every proxied parent of
42+            # this class.
43+            while base:
44+                base._meta.proxying_models.append(new_class)
45+                base = base._meta.proxy_for_model
46 
47         # Do the appropriate setup for any model parents.
48         o2o_map = dict([(f.rel.to, f) for f in new_class._meta.local_fields
49@@ -149,9 +154,7 @@ class ModelBase(type):
50                                         (field.name, name, base.__name__))
51             if not base._meta.abstract:
52                 # Concrete classes...
53-                while base._meta.proxy:
54-                    # Skip over a proxy class to the "real" base it proxies.
55-                    base = base._meta.proxy_for_model
56+                base = base._meta.concrete_class
57                 if base in o2o_map:
58                     field = o2o_map[base]
59                 elif not is_proxy:
60diff --git a/django/db/models/deletion.py b/django/db/models/deletion.py
61index 9a6d499..d119534 100644
62--- a/django/db/models/deletion.py
63+++ b/django/db/models/deletion.py
64@@ -145,6 +145,11 @@ class Collector(object):
65         if not new_objs:
66             return
67         model = new_objs[0].__class__
68+        # Always use the concrete model class for deletion. We will handle
69+        # all the proxy-child classes later on. We do this because when
70+        # we are deleting a model, we are in fact deleting the whole proxy
71+        # equivalence class.
72+        model = model._meta.concrete_class
73 
74         # Recursively collect parent models, but not their related objects.
75         # These will be found by meta.get_all_related_objects()
76@@ -157,27 +162,47 @@ class Collector(object):
77                              reverse_dependency=True)
78 
79         if collect_related:
80-            for related in model._meta.get_all_related_objects(include_hidden=True):
81-                field = related.field
82-                if related.model._meta.auto_created:
83-                    self.add_batch(related.model, field, new_objs)
84-                else:
85-                    sub_objs = self.related_objects(related, new_objs)
86-                    if not sub_objs:
87+            # Collect all related objects to this model and for models proxying
88+            # this model. Note that at this point the model is always the concrete
89+            # parent in the proxy chain.
90+            # When deleting a model we are also deleting every model that
91+            # is in proxy-relation to this model. Would this be better handled by
92+            # get_all_related_objects kwarg include_proxy? Are we continuously
93+            # finding the same relations (proxy has all the same relations out as
94+            # the parent).
95+            proxy_equivalance = [model] + model._meta.proxying_models
96+
97+            # keep track of duplicate related models. These can be seen because of
98+            # proxy models.
99+            seen_related = set()
100+            for base in proxy_equivalance:
101+                for related in base._meta.get_all_related_objects(include_hidden=True):
102+                    if related in seen_related:
103                         continue
104-                    field.rel.on_delete(self, field, sub_objs, self.using)
105-
106-            # TODO This entire block is only needed as a special case to
107-            # support cascade-deletes for GenericRelation. It should be
108-            # removed/fixed when the ORM gains a proper abstraction for virtual
109-            # or composite fields, and GFKs are reworked to fit into that.
110-            for relation in model._meta.many_to_many:
111-                if not relation.rel.through:
112-                    sub_objs = relation.bulk_related_objects(new_objs, self.using)
113-                    self.collect(sub_objs,
114-                                 source=model,
115-                                 source_attr=relation.rel.related_name,
116-                                 nullable=True)
117+                    seen_related.add(related)
118+                    field = related.field
119+                    if related.model._meta.auto_created:
120+                        self.add_batch(related.model, field, new_objs)
121+                    else:
122+                        sub_objs = self.related_objects(related, new_objs)
123+                        if not sub_objs:
124+                            continue
125+                        field.rel.on_delete(self, field, sub_objs, self.using)
126+
127+                # TODO This entire block is only needed as a special case to
128+                # support cascade-deletes for GenericRelation. It should be
129+                # removed/fixed when the ORM gains a proper abstraction for virtual
130+                # or composite fields, and GFKs are reworked to fit into that.
131+                for relation in base._meta.many_to_many:
132+                    if not relation.rel.through:
133+                        if relation in seen_related:
134+                            continue
135+                        seen_related.add(relation)
136+                        sub_objs = relation.bulk_related_objects(new_objs, self.using)
137+                        self.collect(sub_objs,
138+                                     source=model,
139+                                     source_attr=relation.rel.related_name,
140+                                     nullable=True)
141 
142     def related_objects(self, related, objs):
143         """
144diff --git a/django/db/models/options.py b/django/db/models/options.py
145index 0cd52a3..853a616 100644
146--- a/django/db/models/options.py
147+++ b/django/db/models/options.py
148@@ -40,7 +40,14 @@ class Options(object):
149         self.abstract = False
150         self.managed = True
151         self.proxy = False
152+        # None for abstract classes, own class for concrete models
153+        # and the first concrete parent for proxy models. This does
154+        # not track multitable parents.
155+        self.concrete_class = None
156         self.proxy_for_model = None
157+        # We need to keep track of which models are proxying this model
158+        # so that we can fetch all references to this model when deleting.
159+        self.proxying_models = []
160         self.parents = SortedDict()
161         self.duplicate_targets = {}
162         self.auto_created = False
163@@ -183,6 +190,7 @@ class Options(object):
164         """
165         self.pk = target._meta.pk
166         self.proxy_for_model = target
167+        self.concrete_class = target._meta.concrete_class
168         self.db_table = target._meta.db_table
169 
170     def __repr__(self):
171diff --git a/django/db/models/sql/query.py b/django/db/models/sql/query.py
172index 61fd2be..b40c4c8 100644
173--- a/django/db/models/sql/query.py
174+++ b/django/db/models/sql/query.py
175@@ -569,10 +569,7 @@ class Query(object):
176             return
177         orig_opts = self.model._meta
178         seen = {}
179-        if orig_opts.proxy:
180-            must_include = {orig_opts.proxy_for_model: set([orig_opts.pk])}
181-        else:
182-            must_include = {self.model: set([orig_opts.pk])}
183+        must_include = {orig_opts.concrete_class: set([orig_opts.pk])}
184         for field_name in field_names:
185             parts = field_name.split(LOOKUP_SEP)
186             cur_model = self.model
187@@ -1949,9 +1946,4 @@ def add_to_dict(data, key, value):
188         data[key] = set([value])
189 
190 def get_proxied_model(opts):
191-    int_opts = opts
192-    proxied_model = None
193-    while int_opts.proxy:
194-        proxied_model = int_opts.proxy_for_model
195-        int_opts = proxied_model._meta
196-    return proxied_model
197+    return opts.concrete_class
198diff --git a/tests/regressiontests/proxy_bug/__init__.py b/tests/regressiontests/proxy_bug/__init__.py
199new file mode 100644
200index 0000000..e69de29
201diff --git a/tests/regressiontests/proxy_bug/models.py b/tests/regressiontests/proxy_bug/models.py
202new file mode 100644
203index 0000000..7389ffe
204--- /dev/null
205+++ b/tests/regressiontests/proxy_bug/models.py
206@@ -0,0 +1,22 @@
207+from django.db import models
208+
209+class File(models.Model):
210+    pass
211+
212+class Image(File):
213+    class Meta:
214+        proxy = True
215+
216+class Photo(Image):
217+    class Meta:
218+        proxy = True
219+
220+class FooImage(models.Model):
221+    my_image = models.ForeignKey(Image)
222+   
223+class FooFile(models.Model):
224+    my_file = models.ForeignKey(File)
225+
226+class FooPhoto(models.Model):
227+    my_photo = models.ForeignKey(Photo)
228+
229diff --git a/tests/regressiontests/proxy_bug/tests.py b/tests/regressiontests/proxy_bug/tests.py
230new file mode 100644
231index 0000000..5704d35
232--- /dev/null
233+++ b/tests/regressiontests/proxy_bug/tests.py
234@@ -0,0 +1,91 @@
235+from django.test import TestCase
236+#from django.test.utils import override_settings
237+from .models import Image, File, Photo, FooFile, FooImage, FooPhoto
238+
239+
240+class ProxyDeleteImageTest(TestCase):
241+    '''
242+    Tests on_delete behaviour for proxy models. Deleting the *proxy*
243+    instance bubbles through to its non-proxy and *all* referring objects
244+    are deleted.
245+    '''
246+
247+    def setUp(self):
248+        # Create an Image
249+        self.test_image = Image()
250+        self.test_image.save()
251+        foo_image = FooImage(my_image=self.test_image)
252+        foo_image.save()
253+
254+        # Get the Image instance as a File
255+        test_file = File.objects.get(pk=self.test_image.pk)
256+        foo_file = FooFile(my_file=test_file)
257+        foo_file.save()
258+
259+    #@override_settings(DEBUG=True)
260+    def test_delete(self):
261+        self.assertTrue(Image.objects.all().delete() is None)
262+        # An Image deletion == File deletion
263+        self.assertEqual(len(Image.objects.all()), 0)
264+        self.assertEqual(len(File.objects.all()), 0)
265+        # The Image deletion cascaded and *all* references to it are deleted.
266+        self.assertEqual(len(FooImage.objects.all()), 0)
267+        self.assertEqual(len(FooFile.objects.all()), 0)
268+        #from django.db import connection
269+        #for q in connection.queries:
270+        #    print q
271+
272+
273+class ProxyDeletePhotoTest(ProxyDeleteImageTest):
274+    '''
275+    Tests on_delete behaviour for proxy-of-proxy models. Deleting the *proxy*
276+    instance should bubble through to its proxy and non-proxy variants.
277+    Deleting *all* referring objects. For some reason it seems that the
278+    intermediate proxy model isn't cleaned up.
279+    '''
280+
281+    def setUp(self):
282+        # Create the Image, FooImage and FooFile instances
283+        super(ProxyDeletePhotoTest, self).setUp()
284+        # Get the Image as a Photo
285+        test_photo = Photo.objects.get(pk=self.test_image.pk)
286+        foo_photo = FooPhoto(my_photo=test_photo)
287+        foo_photo.save()
288+
289+
290+    #@override_settings(DEBUG=True)
291+    def test_delete(self):
292+        self.assertTrue(Photo.objects.all().delete() is None)
293+        # A Photo deletion == Image deletion == File deletion
294+        self.assertEqual(len(Photo.objects.all()), 0)
295+        self.assertEqual(len(Image.objects.all()), 0)
296+        self.assertEqual(len(File.objects.all()), 0)
297+        # The Photo deletion should have cascaded and deleted *all*
298+        # references to it.
299+        self.assertEqual(len(FooPhoto.objects.all()), 0)
300+        self.assertEqual(len(FooFile.objects.all()), 0)
301+        self.assertEqual(len(FooImage.objects.all()), 0)
302+        #from django.db import connection
303+        #for q in connection.queries:
304+        #    print q
305+        #print len(connection.queries)
306+
307+
308+class ProxyDeleteFileTest(ProxyDeleteImageTest):
309+    '''
310+    Tests on_delete cascade behaviour for proxy models. Deleting the
311+    *non-proxy* instance of a model should somehow notify it's proxy.
312+    Currently it doesn't, making this test fail.
313+    '''
314+
315+    def test_delete(self):
316+        # This should *not* raise an IntegrityError on databases that support
317+        # FK constraints.
318+        self.assertTrue(File.objects.all().delete() is None)
319+        # A File deletion == Image deletion
320+        self.assertEqual(len(File.objects.all()), 0)
321+        self.assertEqual(len(Image.objects.all()), 0)
322+        # The File deletion should have cascaded and deleted *all* references
323+        # to it.
324+        self.assertEqual(len(FooFile.objects.all()), 0)
325+        self.assertEqual(len(FooImage.objects.all()), 0)