Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fixed #14549 - Removed restriction of single FKs on intermediary tables

Thanks to Loic Bistuer for review. Minor changes to error messages
done by committer.
  • Loading branch information...
commit c627da0ccc12861163f28177aa7538b420a9d310 1 parent 95c74b9
@dfunckt dfunckt authored akaariai committed
View
76 django/db/models/fields/related.py
@@ -1250,9 +1250,12 @@ def __init__(self, field, to, field_name, related_name=None, limit_choices_to=No
class ManyToManyRel(object):
def __init__(self, to, related_name=None, limit_choices_to=None,
- symmetrical=True, through=None, db_constraint=True, related_query_name=None):
+ symmetrical=True, through=None, through_fields=None,
+ db_constraint=True, related_query_name=None):
if through and not db_constraint:
raise ValueError("Can't supply a through model and db_constraint=False")
+ if through_fields and not through:
+ raise ValueError("Cannot specify through_fields without a through model")
self.to = to
self.related_name = related_name
self.related_query_name = related_query_name
@@ -1262,6 +1265,7 @@ def __init__(self, to, related_name=None, limit_choices_to=None,
self.symmetrical = symmetrical
self.multiple = True
self.through = through
+ self.through_fields = through_fields
self.db_constraint = db_constraint
def is_hidden(self):
@@ -1849,6 +1853,7 @@ def __init__(self, to, db_constraint=True, swappable=True, **kwargs):
limit_choices_to=kwargs.pop('limit_choices_to', None),
symmetrical=kwargs.pop('symmetrical', to == RECURSIVE_RELATIONSHIP_CONSTANT),
through=kwargs.pop('through', None),
+ through_fields=kwargs.pop('through_fields', None),
db_constraint=db_constraint,
)
@@ -1878,6 +1883,12 @@ def _check_unique(self, **kwargs):
return []
def _check_relationship_model(self, from_model=None, **kwargs):
+ if hasattr(self.rel.through, '_meta'):
+ qualified_model_name = "%s.%s" % (
+ self.rel.through._meta.app_label, self.rel.through.__name__)
+ else:
+ qualified_model_name = self.rel.through
+
errors = []
if self.rel.through not in apps.get_models(include_auto_created=True):
@@ -1885,13 +1896,28 @@ def _check_relationship_model(self, from_model=None, **kwargs):
errors.append(
checks.Error(
("Field specifies a many-to-many relation through model "
- "'%s', which has not been installed.") % self.rel.through,
+ "'%s', which has not been installed.") %
+ qualified_model_name,
hint=None,
obj=self,
id='fields.E331',
)
)
+ elif self.rel.through_fields is not None:
+ if not len(self.rel.through_fields) >= 2 or not (self.rel.through_fields[0] and self.rel.through_fields[1]):
+ errors.append(
+ checks.Error(
+ ("The field is given an iterable for through_fields, "
+ "which does not provide the names for both link fields "
+ "that Django should use for the relation through model "
+ "'%s'.") % qualified_model_name,
+ hint=None,
+ obj=self,
+ id='fields.E337',
+ )
+ )
+
elif not isinstance(self.rel.through, six.string_types):
assert from_model is not None, \
@@ -1926,13 +1952,16 @@ def _check_relationship_model(self, from_model=None, **kwargs):
seen_self = sum(from_model == getattr(field.rel, 'to', None)
for field in self.rel.through._meta.fields)
- if seen_self > 2:
+ if seen_self > 2 and not self.rel.through_fields:
errors.append(
checks.Error(
("The model is used as an intermediate model by "
"'%s', but it has more than two foreign keys "
- "to '%s', which is ambiguous.") % (self, from_model_name),
- hint=None,
+ "to '%s', which is ambiguous. You must specify "
+ "which two foreign keys Django should use via the "
+ "through_fields keyword argument.") % (self, from_model_name),
+ hint=("Use through_fields to specify which two "
+ "foreign keys Django should use."),
obj=self.rel.through,
id='fields.E333',
)
@@ -1945,12 +1974,14 @@ def _check_relationship_model(self, from_model=None, **kwargs):
seen_to = sum(to_model == getattr(field.rel, 'to', None)
for field in self.rel.through._meta.fields)
- if seen_from > 1:
+ if seen_from > 1 and not self.rel.through_fields:
errors.append(
checks.Error(
("The model is used as an intermediate model by "
"'%s', but it has more than one foreign key "
- "from '%s', which is ambiguous.") % (self, from_model_name),
+ "from '%s', which is ambiguous. You must specify "
+ "which foreign key Django should use via the "
+ "through_fields keyword argument.") % (self, from_model_name),
hint=('If you want to create a recursive relationship, '
'use ForeignKey("self", symmetrical=False, '
'through="%s").') % relationship_model_name,
@@ -1959,12 +1990,14 @@ def _check_relationship_model(self, from_model=None, **kwargs):
)
)
- if seen_to > 1:
+ if seen_to > 1 and not self.rel.through_fields:
errors.append(
checks.Error(
("The model is used as an intermediate model by "
"'%s', but it has more than one foreign key "
- "to '%s', which is ambiguous.") % (self, to_model_name),
+ "to '%s', which is ambiguous. You must specify "
+ "which foreign key Django should use via the "
+ "through_fields keyword argument.") % (self, to_model_name),
hint=('If you want to create a recursive '
'relationship, use ForeignKey("self", '
'symmetrical=False, through="%s").') % relationship_model_name,
@@ -2057,10 +2090,18 @@ def _get_m2m_attr(self, related, attr):
cache_attr = '_m2m_%s_cache' % attr
if hasattr(self, cache_attr):
return getattr(self, cache_attr)
+ if self.rel.through_fields is not None:
+ link_field_name = self.rel.through_fields[0]
+ else:
+ link_field_name = None
for f in self.rel.through._meta.fields:
- if hasattr(f, 'rel') and f.rel and f.rel.to == related.model:
+ if hasattr(f, 'rel') and f.rel and f.rel.to == related.model and \
+ (link_field_name is None or link_field_name == f.name):
setattr(self, cache_attr, getattr(f, attr))
return getattr(self, cache_attr)
+ # We only reach here if we're given an invalid field name via the
+ # `through_fields` argument
+ raise FieldDoesNotExist(link_field_name)
def _get_m2m_reverse_attr(self, related, attr):
"Function that can be curried to provide the related accessor or DB column name for the m2m table"
@@ -2068,9 +2109,13 @@ def _get_m2m_reverse_attr(self, related, attr):
if hasattr(self, cache_attr):
return getattr(self, cache_attr)
found = False
+ if self.rel.through_fields is not None:
+ link_field_name = self.rel.through_fields[1]
+ else:
+ link_field_name = None
for f in self.rel.through._meta.fields:
if hasattr(f, 'rel') and f.rel and f.rel.to == related.parent_model:
- if related.model == related.parent_model:
+ if link_field_name is None and related.model == related.parent_model:
# If this is an m2m-intermediate to self,
# the first foreign key you find will be
# the source column. Keep searching for
@@ -2080,10 +2125,15 @@ def _get_m2m_reverse_attr(self, related, attr):
break
else:
found = True
- else:
+ elif link_field_name is None or link_field_name == f.name:
setattr(self, cache_attr, getattr(f, attr))
break
- return getattr(self, cache_attr)
+ try:
+ return getattr(self, cache_attr)
+ except AttributeError:
+ # We only reach here if we're given an invalid reverse field
+ # name via the `through_fields` argument
+ raise FieldDoesNotExist(link_field_name)
def value_to_string(self, obj):
data = ''
View
9 docs/ref/checks.txt
@@ -90,10 +90,11 @@ Related Fields
* **fields.E330**: ManyToManyFields cannot be unique.
* **fields.E331**: Field specifies a many-to-many relation through model ``%s``, which has not been installed.
* **fields.E332**: Many-to-many fields with intermediate tables must not be symmetrical.
-* **fields.E333**: The model is used as an intermediate model by ``<model>``, but it has more than two foreign keys to ``<model>``, which is ambiguous.
-* **fields.E334**: The model is used as an intermediate model by ``<model>``, but it has more than one foreign key from ``<model>``, which is ambiguous.
-* **fields.E335**: The model is used as an intermediate model by ``<model>``, but it has more than one foreign key to ``<model>``, which is ambiguous.
-* **fields.E336**: The model is used as an intermediary model by ``<model>``, but it does not have foreign key to ``<model>`` or ``<model>``."
+* **fields.E333**: The model is used as an intermediate model by ``<model>``, but it has more than two foreign keys to ``<model>``, which is ambiguous. You must specify which two foreign keys Django should use via the through_fields keyword argument.
+* **fields.E334**: The model is used as an intermediate model by ``<model>``, but it has more than one foreign key from ``<model>``, which is ambiguous. You must specify which foreign key Django should use via the through_fields keyword argument.
+* **fields.E335**: The model is used as an intermediate model by ``<model>``, but it has more than one foreign key to ``<model>``, which is ambiguous. You must specify which foreign key Django should use via the through_fields keyword argument.
+* **fields.E336**: The model is used as an intermediary model by ``<model>``, but it does not have foreign key to ``<model>`` or ``<model>``.
+* **fields.E337**: The field is given an iterable for through_fields, which does not provide the names for both link fields that Django should use for the relation through <model>.
Signals
~~~~~~~
View
49 docs/ref/models/fields.txt
@@ -1337,6 +1337,55 @@ that control how the relationship functions.
:ref:`extra data with a many-to-many relationship
<intermediary-manytomany>`.
+.. attribute:: ManyToManyField.through_fields
+
+ .. versionadded:: 1.7
+
+ Only used when a custom intermediary model is specified. Django will
+ normally determine which fields of the intermediary model to use in order
+ to establish a many-to-many relationship automatically. However,
+ consider the following models::
+
+ from django.db import models
+
+ class Person(models.Model):
+ name = models.CharField(max_length=50)
+
+ class Group(models.Model):
+ name = models.CharField(max_length=128)
+ members = models.ManyToManyField(Person, through='Membership', through_fields=('person', 'group'))
+
+ class Membership(models.Model):
+ person = models.ForeignKey(Person)
+ group = models.ForeignKey(Group)
+ inviter = models.ForeignKey(Person, related_name="membership_invites")
+ invite_reason = models.CharField(max_length=64)
+
+ ``Membership`` has *two* foreign keys to ``Person`` (``person`` and
+ ``inviter``), which makes the relationship ambiguous and Django can't know
+ which one to use. In this case, you must explicitly specify which
+ foreign keys Django should use using ``through_fields``, as in the example
+ above.
+
+ ``through_fields`` accepts a 2-tuple ``('field1', 'field2')``, where
+ ``field1`` is the name of the foreign key to the target model (``person``
+ in this case), and ``field2`` the name of the foreign key to the model the
+ :class:`ManyToManyField` is defined on (``group`` in this case).
+
+ When you have more than one foreign key on an intermediary model to any
+ (or even both) of the models participating in a many-to-many relationship,
+ you *must* specify ``through_fields``. This also applies to
+ :ref:`recursive relationships <recursive-relationships>`
+ when an intermediary model is used and there are more than two
+ foreign keys to the model, or you want to explicitly specify which two
+ Django should use.
+
+ Recursive relationships using an intermediary model are always defined as
+ non-symmetrical -- that is, with :attr:`symmetrical=False <ManyToManyField.symmetrical>`
+ -- therefore, there is the concept of a "source" and a "target". In that
+ case ``'field1'`` will be treated as the "source" of the relationship and
+ ``'field2'`` as the "target".
+
.. attribute:: ManyToManyField.db_table
The name of the table to create for storing the many-to-many data. If this
View
6 docs/releases/1.7.txt
@@ -658,6 +658,12 @@ Models
* You can use a single list for :attr:`~django.db.models.Options.index_together`
(rather than a list of lists) when specifying a single set of fields.
+* Custom intermediate models having more than one foreign key to any of the
+ models participating in a many-to-many relationship are now permitted,
+ provided you explicitly specify which foreign keys should be used by setting
+ the new :attr:`ManyToManyField.through_fields <django.db.models.ManyToManyField.through_fields>`
+ argument.
+
Signals
^^^^^^^
View
34 docs/topics/db/models.txt
@@ -434,30 +434,38 @@ something like this::
invite_reason = models.CharField(max_length=64)
When you set up the intermediary model, you explicitly specify foreign
-keys to the models that are involved in the ManyToMany relation. This
+keys to the models that are involved in the many-to-many relationship. This
explicit declaration defines how the two models are related.
There are a few restrictions on the intermediate model:
* Your intermediate model must contain one - and *only* one - foreign key
- to the target model (this would be ``Person`` in our example). If you
- have more than one foreign key, a validation error will be raised.
-
-* Your intermediate model must contain one - and *only* one - foreign key
- to the source model (this would be ``Group`` in our example). If you
- have more than one foreign key, a validation error will be raised.
-
-* The only exception to this is a model which has a many-to-many
- relationship to itself, through an intermediary model. In this
- case, two foreign keys to the same model are permitted, but they
- will be treated as the two (different) sides of the many-to-many
- relation.
+ to the target model (this would be ``Person`` in our example), or you must
+ explicitly specify the foreign keys Django should use for the relationship
+ using :attr:`ManyToManyField.through_fields <ManyToManyField.through_fields>`.
+ If you have more than one foreign key and ``through_fields`` is not
+ specified, a validation error will be raised. A similar restriction applies
+ to the foreign key to the source model (this would be ``Group`` in our
+ example).
+
+* For a model which has a many-to-many relationship to itself through an
+ intermediary model, two foreign keys to the same model are permitted, but
+ they will be treated as the two (different) sides of the many-to-many
+ relationship. If there are *more* than two foreign keys though, you
+ must also specify ``through_fields`` as above, or a validation error
+ will be raised.
* When defining a many-to-many relationship from a model to
itself, using an intermediary model, you *must* use
:attr:`symmetrical=False <ManyToManyField.symmetrical>` (see
:ref:`the model field reference <manytomany-arguments>`).
+.. versionchanged:: 1.7
+
+ In Django 1.6 and earlier, intermediate models containing more than one
+ foreign key to any of the models involved in the many-to-many relationship
+ used to be prohibited.
+
Now that you have set up your :class:`~django.db.models.ManyToManyField` to use
your intermediary model (``Membership``, in this case), you're ready to start
creating some many-to-many relationships. You do this by creating instances of
View
92 tests/invalid_models_tests/test_relative_fields.py
@@ -3,6 +3,7 @@
from django.core.checks import Error
from django.db import models
+from django.db.models.fields import FieldDoesNotExist
from django.test.utils import override_settings
from django.test.testcases import skipIfDBFeature
@@ -81,7 +82,9 @@ class AmbiguousRelationship(models.Model):
Error(
("The model is used as an intermediate model by "
"'invalid_models_tests.Group.field', but it has more than one "
- "foreign key to 'Person', which is ambiguous."),
+ "foreign key to 'Person', which is ambiguous. You must specify "
+ "which foreign key Django should use via the through_fields "
+ "keyword argument."),
hint=('If you want to create a recursive relationship, use '
'ForeignKey("self", symmetrical=False, '
'through="AmbiguousRelationship").'),
@@ -205,8 +208,10 @@ class InvalidRelationship(models.Model):
Error(
("The model is used as an intermediate model by "
"'invalid_models_tests.Person.friends', but it has more than two "
- "foreign keys to 'Person', which is ambiguous."),
- hint=None,
+ "foreign keys to 'Person', which is ambiguous. You must specify "
+ "which two foreign keys Django should use via the through_fields "
+ "keyword argument."),
+ hint='Use through_fields to specify which two foreign keys Django should use.',
obj=InvalidRelationship,
id='fields.E333',
),
@@ -1051,3 +1056,84 @@ class Model(models.Model):
),
]
self.assertEqual(errors, expected)
+
+
+class M2mThroughFieldsTests(IsolatedModelsTestCase):
+ def test_m2m_field_argument_validation(self):
+ """
+ Tests that ManyToManyField accepts the ``through_fields`` kwarg
+ only if an intermediary table is specified.
+ """
+ class Fan(models.Model):
+ pass
+
+ self.assertRaisesMessage(
+ ValueError, 'Cannot specify through_fields without a through model',
+ models.ManyToManyField, Fan, through_fields=('f1', 'f2'))
+
+ def test_invalid_m2m_field(self):
+ """
+ Tests that providing invalid source field name to ManyToManyField.through_fields
+ raises FieldDoesNotExist.
+ """
+ class Fan(models.Model):
+ pass
+
+ class Event(models.Model):
+ invitees = models.ManyToManyField(Fan, through='Invitation', through_fields=('invalid_field', 'invitee'))
+
+ class Invitation(models.Model):
+ event = models.ForeignKey(Event)
+ invitee = models.ForeignKey(Fan)
+ inviter = models.ForeignKey(Fan, related_name='+')
+
+ with self.assertRaisesMessage(FieldDoesNotExist, 'invalid_field'):
+ Event().invitees.all()
+
+ def test_invalid_m2m_reverse_field(self):
+ """
+ Tests that providing invalid reverse field name to ManyToManyField.through_fields
+ raises FieldDoesNotExist.
+ """
+ class Fan(models.Model):
+ pass
+
+ class Event(models.Model):
+ invitees = models.ManyToManyField(Fan, through='Invitation', through_fields=('event', 'invalid_field'))
+
+ class Invitation(models.Model):
+ event = models.ForeignKey(Event)
+ invitee = models.ForeignKey(Fan)
+ inviter = models.ForeignKey(Fan, related_name='+')
+
+ with self.assertRaisesMessage(FieldDoesNotExist, 'invalid_field'):
+ Event().invitees.all()
+
+ def test_explicit_field_names(self):
+ """
+ Tests that if ``through_fields`` kwarg is given, it must specify both
+ link fields of the intermediary table.
+ """
+ class Fan(models.Model):
+ pass
+
+ class Event(models.Model):
+ invitees = models.ManyToManyField(Fan, through='Invitation', through_fields=(None, 'invitee'))
+
+ class Invitation(models.Model):
+ event = models.ForeignKey(Event)
+ invitee = models.ForeignKey(Fan)
+ inviter = models.ForeignKey(Fan, related_name='+')
+
+ field = Event._meta.get_field('invitees')
+ errors = field.check(from_model=Event)
+ expected = [
+ Error(
+ ("The field is given an iterable for through_fields, "
+ "which does not provide the names for both link fields "
+ "that Django should use for the relation through model "
+ "'invalid_models_tests.Invitation'."),
+ hint=None,
+ obj=field,
+ id='fields.E337')]
+ self.assertEqual(expected, errors)
View
36 tests/m2m_through/models.py
@@ -77,3 +77,39 @@ class Friendship(models.Model):
first = models.ForeignKey(PersonSelfRefM2M, related_name="rel_from_set")
second = models.ForeignKey(PersonSelfRefM2M, related_name="rel_to_set")
date_friended = models.DateTimeField()
+
+
+# Custom through link fields
+@python_2_unicode_compatible
+class Event(models.Model):
+ title = models.CharField(max_length=50)
+ invitees = models.ManyToManyField(Person, through='Invitation', through_fields=('event', 'invitee'), related_name='events_invited')
+
+ def __str__(self):
+ return self.title
+
+
+class Invitation(models.Model):
+ event = models.ForeignKey(Event, related_name='invitations')
+ # field order is deliberately inverted. the target field is "invitee".
+ inviter = models.ForeignKey(Person, related_name='invitations_sent')
+ invitee = models.ForeignKey(Person, related_name='invitations')
+
+
+@python_2_unicode_compatible
+class Employee(models.Model):
+ name = models.CharField(max_length=5)
+ subordinates = models.ManyToManyField('self', through="Relationship", through_fields=('source', 'target'), symmetrical=False)
+
+ class Meta:
+ ordering = ('pk',)
+
+ def __str__(self):
+ return self.name
+
+
+class Relationship(models.Model):
+ # field order is deliberately inverted.
+ another = models.ForeignKey(Employee, related_name="rel_another_set", null=True)
+ target = models.ForeignKey(Employee, related_name="rel_target_set")
+ source = models.ForeignKey(Employee, related_name="rel_source_set")
View
29 tests/m2m_through/tests.py
@@ -6,7 +6,7 @@
from django.test import TestCase
from .models import (Person, Group, Membership, CustomMembership,
- PersonSelfRefM2M, Friendship)
+ PersonSelfRefM2M, Friendship, Event, Invitation, Employee, Relationship)
class M2mThroughTests(TestCase):
@@ -276,6 +276,33 @@ def test_self_referential_tests(self):
attrgetter("name")
)
+ def test_through_fields(self):
+ """
+ Tests that relations with intermediary tables with multiple FKs
+ to the M2M's ``to`` model are possible.
+ """
+ event = Event.objects.create(title='Rockwhale 2014')
+ Invitation.objects.create(event=event, inviter=self.bob, invitee=self.jim)
+ Invitation.objects.create(event=event, inviter=self.bob, invitee=self.jane)
+ self.assertQuerysetEqual(event.invitees.all(), [
+ 'Jane',
+ 'Jim',
+ ], attrgetter('name'))
+
+ def test_through_fields_self_referential(self):
+ john = Employee.objects.create(name='john')
+ peter = Employee.objects.create(name='peter')
+ mary = Employee.objects.create(name='mary')
+ harry = Employee.objects.create(name='harry')
+ Relationship.objects.create(source=john, target=peter, another=None)
+ Relationship.objects.create(source=john, target=mary, another=None)
+ Relationship.objects.create(source=john, target=harry, another=peter)
+ self.assertQuerysetEqual(john.subordinates.all(), [
+ 'peter',
+ 'mary',
+ 'harry',
+ ], attrgetter('name'))
+
def test_query_tests(self):
Membership.objects.create(person=self.jim, group=self.rock)
m2 = Membership.objects.create(person=self.jane, group=self.rock)
Please sign in to comment.
Something went wrong with that request. Please try again.