Code

Ticket #6095: 6095-beta-02.diff

File 6095-beta-02.diff, 22.8 KB (added by floguy, 7 years ago)

Updated patch which disallows .add() and .remove() and has some improved test cases.

Line 
1Index: django/db/models/fields/related.py
2===================================================================
3--- django/db/models/fields/related.py  (revision 7003)
4+++ django/db/models/fields/related.py  (working copy)
5@@ -54,6 +54,21 @@
6     except klass.DoesNotExist:
7         raise validators.ValidationError, _("Please enter a valid %s.") % f.verbose_name
8 
9+def get_reverse_rel_field(from_model, to_model, related_name):
10+    "Gets the related field which points from one model to another."
11+    for field in from_model._meta.fields:
12+        if field.__class__ == ForeignKey:
13+            if field.rel.to == to_model:
14+                return field
15+    return None
16+
17+def get_model_for_db_table(db_table):
18+    "Gets a model class from a db_table string."
19+    for model in get_models():
20+        if model._meta.db_table == db_table:
21+            return model
22+    return None
23+
24 #HACK
25 class RelatedField(object):
26     def contribute_to_class(self, cls, name):
27@@ -267,7 +282,8 @@
28     and adds behavior for many-to-many related objects."""
29     class ManyRelatedManager(superclass):
30         def __init__(self, model=None, core_filters=None, instance=None, symmetrical=None,
31-                join_table=None, source_col_name=None, target_col_name=None):
32+                join_table=None, source_col_name=None, target_col_name=None,
33+                through=None):
34             super(ManyRelatedManager, self).__init__()
35             self.core_filters = core_filters
36             self.model = model
37@@ -276,6 +292,7 @@
38             self.join_table = join_table
39             self.source_col_name = source_col_name
40             self.target_col_name = target_col_name
41+            self.through = through
42             self._pk_val = self.instance._get_pk_val()
43             if self._pk_val is None:
44                 raise ValueError("%r instance needs to have a primary key value before a many-to-many relationship can be used." % model)
45@@ -284,6 +301,8 @@
46             return superclass.get_query_set(self).filter(**(self.core_filters))
47 
48         def add(self, *objs):
49+            if self.through:
50+                raise NotImplementedError, "Add not possible for ManyToManyFields which specify a through model.  Try %s.objects.create(...) instead." % self.through
51             self._add_items(self.source_col_name, self.target_col_name, *objs)
52 
53             # If this is a symmetrical m2m relation to self, add the mirror entry in the m2m table
54@@ -292,8 +311,10 @@
55         add.alters_data = True
56 
57         def remove(self, *objs):
58+            if self.through:
59+                raise NotImplementedError, "Remove not possible for ManyToManyFields which specify a through model."
60             self._remove_items(self.source_col_name, self.target_col_name, *objs)
61-
62+           
63             # If this is a symmetrical m2m relation to self, remove the mirror entry in the m2m table
64             if self.symmetrical:
65                 self._remove_items(self.target_col_name, self.source_col_name, *objs)
66@@ -405,7 +426,8 @@
67             symmetrical=False,
68             join_table=qn(self.related.field.m2m_db_table()),
69             source_col_name=qn(self.related.field.m2m_reverse_name()),
70-            target_col_name=qn(self.related.field.m2m_column_name())
71+            target_col_name=qn(self.related.field.m2m_column_name()),
72+            through=getattr(self.related.field.rel, 'through', None)
73         )
74 
75         return manager
76@@ -414,6 +436,10 @@
77         if instance is None:
78             raise AttributeError, "Manager must be accessed via instance"
79 
80+        through = getattr(self.related.field.rel, 'through', None)
81+        if through:
82+            raise NotImplementedError, "Cannot set values on a ManyToManyFields which specify a through model.  Use %s's Manager instead." % through
83+       
84         manager = self.__get__(instance)
85         manager.clear()
86         manager.add(*value)
87@@ -446,7 +472,8 @@
88             symmetrical=(self.field.rel.symmetrical and instance.__class__ == rel_model),
89             join_table=qn(self.field.m2m_db_table()),
90             source_col_name=qn(self.field.m2m_column_name()),
91-            target_col_name=qn(self.field.m2m_reverse_name())
92+            target_col_name=qn(self.field.m2m_reverse_name()),
93+            through=getattr(self.field.rel, 'through', None)
94         )
95 
96         return manager
97@@ -455,6 +482,10 @@
98         if instance is None:
99             raise AttributeError, "Manager must be accessed via instance"
100 
101+        through = getattr(self.field.rel, 'through', None)
102+        if through:
103+            raise NotImplementedError, "Cannot set values on a ManyToManyFields which specify a through model.  Use %s's Manager instead." % through
104+
105         manager = self.__get__(instance)
106         manager.clear()
107         manager.add(*value)
108@@ -648,8 +679,11 @@
109             filter_interface=kwargs.pop('filter_interface', None),
110             limit_choices_to=kwargs.pop('limit_choices_to', None),
111             raw_id_admin=kwargs.pop('raw_id_admin', False),
112-            symmetrical=kwargs.pop('symmetrical', True))
113+            symmetrical=kwargs.pop('symmetrical', True),
114+            through=kwargs.pop('through', None))
115         self.db_table = kwargs.pop('db_table', None)
116+        if kwargs['rel'].through:
117+            assert not self.db_table, "Cannot specify a db_table if an intermediary model is used."
118         if kwargs["rel"].raw_id_admin:
119             kwargs.setdefault("validator_list", []).append(self.isValidIDList)
120         Field.__init__(self, **kwargs)
121@@ -672,7 +706,9 @@
122 
123     def _get_m2m_db_table(self, opts):
124         "Function that can be curried to provide the m2m table name for this relation"
125-        if self.db_table:
126+        if self.rel.through != None:
127+            return get_model(opts.app_label, self.rel.through)._meta.db_table
128+        elif self.db_table:
129             return self.db_table
130         else:
131             return '%s_%s' % (opts.db_table, self.name)
132@@ -680,7 +716,12 @@
133     def _get_m2m_column_name(self, related):
134         "Function that can be curried to provide the source column name for the m2m table"
135         # If this is an m2m relation to self, avoid the inevitable name clash
136-        if related.model == related.parent_model:
137+        if self.rel.through != None:
138+            through = get_model(related.opts.app_label, self.rel.through)
139+            field = get_reverse_rel_field(through, related.model, self.rel.related_name)
140+            attname, column = field.get_attname_column()
141+            return column
142+        elif related.model == related.parent_model:
143             return 'from_' + related.model._meta.object_name.lower() + '_id'
144         else:
145             return related.model._meta.object_name.lower() + '_id'
146@@ -688,7 +729,12 @@
147     def _get_m2m_reverse_name(self, related):
148         "Function that can be curried to provide the related column name for the m2m table"
149         # If this is an m2m relation to self, avoid the inevitable name clash
150-        if related.model == related.parent_model:
151+        if self.rel.through != None:
152+            through = get_model(related.opts.app_label, self.rel.through)
153+            field = get_reverse_rel_field(through, related.parent_model, self.rel.related_name)
154+            attname, column = field.get_attname_column()
155+            return column
156+        elif related.model == related.parent_model:
157             return 'to_' + related.parent_model._meta.object_name.lower() + '_id'
158         else:
159             return related.parent_model._meta.object_name.lower() + '_id'
160@@ -809,7 +855,8 @@
161 
162 class ManyToManyRel(object):
163     def __init__(self, to, num_in_admin=0, related_name=None,
164-        filter_interface=None, limit_choices_to=None, raw_id_admin=False, symmetrical=True):
165+        filter_interface=None, limit_choices_to=None, raw_id_admin=False, symmetrical=True,
166+        through = None):
167         self.to = to
168         self.num_in_admin = num_in_admin
169         self.related_name = related_name
170@@ -821,5 +868,6 @@
171         self.raw_id_admin = raw_id_admin
172         self.symmetrical = symmetrical
173         self.multiple = True
174+        self.through = through
175 
176         assert not (self.raw_id_admin and self.filter_interface), "ManyToManyRels may not use both raw_id_admin and filter_interface"
177Index: django/core/management/validation.py
178===================================================================
179--- django/core/management/validation.py        (revision 7003)
180+++ django/core/management/validation.py        (working copy)
181@@ -104,6 +104,8 @@
182                         if r.get_accessor_name() == rel_query_name:
183                             e.add(opts, "Reverse query name for field '%s' clashes with related field '%s.%s'. Add a related_name argument to the definition for '%s'." % (f.name, rel_opts.object_name, r.get_accessor_name(), f.name))
184 
185+        seen_intermediary_signatures = []
186+
187         for i, f in enumerate(opts.many_to_many):
188             # Check to see if the related m2m field will clash with any
189             # existing fields, m2m fields, m2m related objects or related objects
190@@ -113,6 +115,28 @@
191                 # so skip the next section
192                 if isinstance(f.rel.to, (str, unicode)):
193                     continue
194+            if hasattr(f.rel, 'through') and f.rel.through != None:
195+                intermediary_model = None
196+                for model in models.get_models():
197+                    if model._meta.module_name == f.rel.through.lower():
198+                        intermediary_model = model
199+                if intermediary_model == None:
200+                    e.add(opts, "%s has a manually-defined m2m relationship through a model (%s) which does not exist." % (f.name, f.rel.through))
201+                else:
202+                    signature = (f.rel.to, cls, intermediary_model)
203+                    if signature in seen_intermediary_signatures:
204+                        e.add(opts, "%s has two manually defined m2m relationships through the same model (%s), which is not possible.  Please use a field on your intermediary model instead." % (cls._meta.object_name, intermediary_model._meta.object_name))
205+                    else:
206+                        seen_intermediary_signatures.append(signature)
207+                    seen_related_fk, seen_this_fk = False, False
208+                    for field in intermediary_model._meta.fields:
209+                        if field.rel:
210+                            if field.rel.to == f.rel.to:
211+                                seen_related_fk = True
212+                            elif field.rel.to == cls:
213+                                seen_this_fk = True
214+                    if not seen_related_fk or not seen_this_fk:
215+                        e.add(opts, "%s has a manualy-defined m2m relationship through a model (%s) which does not have foreign keys to %s and %s" % (f.name, f.rel.through, f.rel.to._meta.object_name, cls._meta.object_name))
216 
217             rel_opts = f.rel.to._meta
218             rel_name = RelatedObject(f.rel.to, cls, f).get_accessor_name()
219Index: django/core/management/sql.py
220===================================================================
221--- django/core/management/sql.py       (revision 7003)
222+++ django/core/management/sql.py       (working copy)
223@@ -352,7 +352,7 @@
224     qn = connection.ops.quote_name
225     inline_references = connection.features.inline_fk_references
226     for f in opts.many_to_many:
227-        if not isinstance(f.rel, generic.GenericRel):
228+        if not isinstance(f.rel, generic.GenericRel) and getattr(f.rel, 'through', None) == None:
229             tablespace = f.db_tablespace or opts.db_tablespace
230             if tablespace and connection.features.supports_tablespaces and connection.features.autoindexes_primary_keys:
231                 tablespace_sql = ' ' + connection.ops.tablespace_sql(tablespace, inline=True)
232Index: tests/modeltests/invalid_models/models.py
233===================================================================
234--- tests/modeltests/invalid_models/models.py   (revision 7003)
235+++ tests/modeltests/invalid_models/models.py   (working copy)
236@@ -111,7 +111,32 @@
237 class MissingRelations(models.Model):
238     rel1 = models.ForeignKey("Rel1")
239     rel2 = models.ManyToManyField("Rel2")
240+   
241+class MissingManualM2MModel(models.Model):
242+    name = models.CharField(max_length=5)
243+    missing_m2m = models.ManyToManyField(Model, through="MissingM2MModel")
244+   
245+class Person(models.Model):
246+    name = models.CharField(max_length=5)
247 
248+class Group(models.Model):
249+    name = models.CharField(max_length=5)
250+    primary = models.ManyToManyField(Person, through="Membership", related_name="primary")
251+    secondary = models.ManyToManyField(Person, through="Membership", related_name="secondary")
252+
253+class GroupTwo(models.Model):
254+    name = models.CharField(max_length=5)
255+    primary = models.ManyToManyField(Person, through="Membership")
256+    secondary = models.ManyToManyField(Group, through="MembershipMissingFK")
257+
258+class Membership(models.Model):
259+    person = models.ForeignKey(Person)
260+    group = models.ForeignKey(Group)
261+    not_default_or_null = models.CharField(max_length=5)
262+
263+class MembershipMissingFK(models.Model):
264+    person = models.ForeignKey(Person)
265+
266 model_errors = """invalid_models.fielderrors: "charfield": CharFields require a "max_length" attribute.
267 invalid_models.fielderrors: "decimalfield": DecimalFields require a "decimal_places" attribute.
268 invalid_models.fielderrors: "decimalfield": DecimalFields require a "max_digits" attribute.
269@@ -197,4 +222,8 @@
270 invalid_models.selfclashm2m: Reverse query name for m2m field 'm2m_4' clashes with field 'SelfClashM2M.selfclashm2m'. Add a related_name argument to the definition for 'm2m_4'.
271 invalid_models.missingrelations: 'rel2' has m2m relation with model Rel2, which has not been installed
272 invalid_models.missingrelations: 'rel1' has relation with model Rel1, which has not been installed
273+invalid_models.group: Group has two manually defined m2m relationships through the same model (Membership), which is not possible.  Please use a field on your intermediary model instead.
274+invalid_models.missingmanualm2mmodel: missing_m2m has a manually-defined m2m relationship through a model (MissingM2MModel) which does not exist.
275+invalid_models.grouptwo: primary has a manualy-defined m2m relationship through a model (Membership) which does not have foreign keys to Person and GroupTwo
276+invalid_models.grouptwo: secondary has a manualy-defined m2m relationship through a model (MembershipMissingFK) which does not have foreign keys to Group and GroupTwo
277 """
278Index: tests/modeltests/m2m_manual/__init__.py
279===================================================================
280Index: tests/modeltests/m2m_manual/models.py
281===================================================================
282--- tests/modeltests/m2m_manual/models.py       (revision 0)
283+++ tests/modeltests/m2m_manual/models.py       (revision 0)
284@@ -0,0 +1,227 @@
285+from django.db import models
286+from datetime import datetime
287+
288+# M2M described on one of the models
289+class Person(models.Model):
290+    name = models.CharField(max_length=128)
291+
292+    def __unicode__(self):
293+        return self.name
294+
295+class Group(models.Model):
296+    name = models.CharField(max_length=128)
297+    members = models.ManyToManyField(Person, through='Membership')
298+    custom_members = models.ManyToManyField(Person, through='CustomMembership', related_name="custom")
299+    nodefaultsnonulls = models.ManyToManyField(Person, through='TestNoDefaultsOrNulls', related_name="testnodefaultsnonulls")
300+   
301+    def __unicode__(self):
302+        return self.name
303+
304+class Membership(models.Model):
305+    person = models.ForeignKey(Person)
306+    group = models.ForeignKey(Group)
307+    date_joined = models.DateTimeField(default=datetime.now)
308+    invite_reason = models.CharField(max_length=64, null=True)
309+   
310+    def __unicode__(self):
311+        return "%s is a member of %s" % (self.person.name, self.group.name)
312+
313+class CustomMembership(models.Model):
314+    person = models.ForeignKey(Person, db_column="custom_person_column", related_name="custom_person_related_name")
315+    group = models.ForeignKey(Group)
316+    weird_fk = models.ForeignKey(Membership, null=True)
317+    date_joined = models.DateTimeField(default=datetime.now)
318+   
319+    def __unicode__(self):
320+        return "%s is a member of %s" % (self.person.name, self.group.name)
321+   
322+    class Meta:
323+        db_table = "test_table"
324+
325+class TestNoDefaultsOrNulls(models.Model):
326+    person = models.ForeignKey(Person)
327+    group = models.ForeignKey(Group)
328+    nodefaultnonull = models.CharField(max_length=5)
329+
330+__test__ = {'API_TESTS':"""
331+>>> from datetime import datetime
332+
333+### Creation and Saving Tests ###
334+>>> bob = Person.objects.create(name = 'Bob')
335+>>> jim = Person.objects.create(name = 'Jim')
336+>>> jane = Person.objects.create(name = 'Jane')
337+>>> rock = Group.objects.create(name = 'Rock')
338+>>> roll = Group.objects.create(name = 'Roll')
339+
340+>>> rock.members.all()
341+[]
342+
343+>>> m1 = Membership.objects.create(person = jim, group = rock)
344+>>> m2 = Membership.objects.create(person = jane, group = rock)
345+
346+>>> rock.members.all()
347+[<Person: Jim>, <Person: Jane>]
348+
349+>>> m3 = Membership.objects.create(person = bob, group = roll)
350+>>> m4 = Membership.objects.create(person = jim, group = roll)
351+>>> m5 = Membership.objects.create(person = jane, group = roll)
352+
353+>>> jim.group_set.all()
354+[<Group: Rock>, <Group: Roll>]
355+
356+# Check to make sure that querying via intermediary model works as normal
357+>>> m = Membership.objects.get(person = jane, group = rock)
358+>>> m
359+<Membership: Jane is a member of Rock>
360+
361+# Setting some date_joined dates
362+>>> m2.invite_reason = "She was just awesome."
363+>>> m2.date_joined = datetime(2006, 1, 1)
364+>>> m2.save()
365+
366+>>> m5.date_joined = datetime(2004, 1, 1)
367+>>> m5.save()
368+
369+>>> m3.date_joined = datetime(2004, 1, 1)
370+>>> m3.save()
371+
372+>>> Membership.objects.filter(person = jim)
373+[<Membership: Jim is a member of Rock>, <Membership: Jim is a member of Roll>]
374+
375+
376+### Forward Descriptors Tests ###
377+# Ensure that using the add or remove function errors, due to using a through model.
378+>>> rock.members.add(bob)
379+Traceback (most recent call last):
380+...
381+NotImplementedError: Add not possible for ManyToManyFields which specify a through model.  Try Membership.objects.create(...) instead.
382+
383+>>> rock.members.remove(jim)
384+Traceback (most recent call last):
385+...
386+NotImplementedError: Remove not possible for ManyToManyFields which specify a through model.
387+
388+>>> backup = list(rock.members.all())
389+>>> backup
390+[<Person: Jim>, <Person: Jane>]
391+
392+# The clear function should still work.
393+>>> rock.members.clear()
394+>>> rock.members.all()
395+[]
396+
397+# Assignment should not work with models specifying a through model.
398+>>> rock.members = backup
399+Traceback (most recent call last):
400+...
401+NotImplementedError: Cannot set values on a ManyToManyFields which specify a through model.  Use Membership's Manager instead.
402+
403+# Let's re-add those instances that we've cleared.
404+>>> m1.save()
405+>>> m2.save()
406+
407+>>> rock.members.all()
408+[<Person: Jim>, <Person: Jane>]
409+
410+
411+### Reverse Descriptors Tests ###
412+# Ensure that using the add or remove function errors, due to using a through model.
413+>>> bob.group_set.add(rock)
414+Traceback (most recent call last):
415+...
416+NotImplementedError: Add not possible for ManyToManyFields which specify a through model.  Try Membership.objects.create(...) instead.
417+
418+>>> jim.group_set.remove(rock)
419+Traceback (most recent call last):
420+...
421+NotImplementedError: Remove not possible for ManyToManyFields which specify a through model.
422+
423+>>> backup = list(jim.group_set.all())
424+>>> backup
425+[<Group: Rock>, <Group: Roll>]
426+
427+# The clear function should still work.
428+>>> jim.group_set.clear()
429+>>> jim.group_set.all()
430+[]
431+
432+# Assignment should not work with models specifying a through model.
433+>>> jim.group_set = backup
434+Traceback (most recent call last):
435+...
436+NotImplementedError: Cannot set values on a ManyToManyFields which specify a through model.  Use Membership's Manager instead.
437+
438+# Let's re-add those instances that we've cleared.
439+>>> m1.save()
440+>>> m4.save()
441+
442+>>> jim.group_set.all()
443+[<Group: Rock>, <Group: Roll>]
444+
445+### Custom Tests ###
446+
447+>>> rock.custom_members.all()
448+[]
449+>>> bob.custom.all()
450+[]
451+>>> cm1 = CustomMembership.objects.create(person = bob, group = rock)
452+>>> cm2 = CustomMembership.objects.create(person = jim, group = rock)
453+
454+>>> rock.custom_members.all()
455+[<Person: Bob>, <Person: Jim>]
456+>>> bob.custom.all()
457+[<Group: Rock>]
458+
459+# Let's make sure our new descriptors don't conflict with the FK related_name.
460+>>> bob.custom_person_related_name.all()
461+[<CustomMembership: Bob is a member of Rock>]
462+
463+###QUERY TESTS###
464+# Queries involving the related model (Person, in the case of Group) use its
465+# attname
466+>>> Group.objects.filter(members__name='Bob')
467+[<Group: Roll>]
468+
469+# Queries involving the relationship model (Membership, in the case of Group)
470+# use its model name
471+>>> Group.objects.filter(membership__invite_reason = "She was just awesome.")
472+[<Group: Rock>]
473+
474+# Queries involving the reverse related model (Group, in the case of Person)
475+# use its model name
476+>>> Person.objects.filter(group__name="Rock")
477+[<Person: Jim>, <Person: Jane>]
478+
479+# If the m2m field has specified a related_name, using that will work.
480+>>> Person.objects.filter(custom__name="Rock")
481+[<Person: Bob>, <Person: Jim>]
482+
483+# Queries involving the relationship model (Membership, in the case of Group)
484+# use its model name
485+>>> Person.objects.filter(membership__invite_reason = "She was just awesome.")
486+[<Person: Jane>]
487+
488+# Let's see all of the groups that Jane joined after 1 Jan 2005:
489+>>> Group.objects.filter(membership__date_joined__gt = datetime(2005, 1, 1),
490+... membership__person = jane)
491+[<Group: Rock>]
492+
493+# Now let's see all of the people that have joined Rock since 1 Jan 2005:
494+>>> Person.objects.filter(membership__date_joined__gt = datetime(2005, 1, 1),
495+... membership__group = rock)
496+[<Person: Jim>, <Person: Jane>]
497+
498+# Conceivably, queries through membership could return non-unique querysets.
499+# To demonstrate this, query for all people who have joined a group after 2004:
500+>>> Person.objects.filter(membership__date_joined__gt = datetime(2004, 1, 1))
501+[<Person: Jim>, <Person: Jim>, <Person: Jane>]
502+
503+# Jim showed up twice, because he joined two groups ('Rock', and 'Roll'):
504+>>> [(m.person.name, m.group.name) for m in
505+... Membership.objects.filter(date_joined__gt = datetime(2004, 1, 1))]
506+[(u'Jim', u'Rock'), (u'Jane', u'Rock'), (u'Jim', u'Roll')]
507+
508+# QuerySet's distinct() method can correct this problem.
509+>>> Person.objects.filter(membership__date_joined__gt = datetime(2004, 1, 1)).distinct()
510+[<Person: Jim>, <Person: Jane>]
511+"""}
512\ No newline at end of file
513Index: AUTHORS
514===================================================================
515--- AUTHORS     (revision 7003)
516+++ AUTHORS     (working copy)
517@@ -129,6 +129,7 @@
518     Afonso Fernández Nogueira <fonzzo.django@gmail.com>
519     Matthew Flanagan <http://wadofstuff.blogspot.com>
520     Eric Floehr <eric@intellovations.com>
521+    Eric Florenzano <floguy@gmail.com>
522     Vincent Foley <vfoleybourgon@yahoo.ca>
523     Rudolph Froger <rfroger@estrate.nl>
524     Jorge Gajon <gajon@gajon.org>