Skip to content

Commit

Permalink
[1.9.x] Fixed #25535 -- Made ForeignObject checks less strict.
Browse files Browse the repository at this point in the history
Check that the foreign object `from_fields` are a subset of any unique
constraints on the foreign model.

Backport of 80dac8c and
c7aff31 from master
  • Loading branch information
acatton authored and timgraham committed Oct 14, 2015
1 parent aa0a3b6 commit 38d6e1e
Show file tree
Hide file tree
Showing 4 changed files with 154 additions and 12 deletions.
25 changes: 19 additions & 6 deletions django/db/models/fields/related.py
Original file line number Diff line number Diff line change
Expand Up @@ -473,22 +473,35 @@ def _check_unique_target(self):
if not self.foreign_related_fields:
return []

has_unique_field = any(rel_field.unique
for rel_field in self.foreign_related_fields)
if not has_unique_field and len(self.foreign_related_fields) > 1:
unique_foreign_fields = {
frozenset([f.name])
for f in self.remote_field.model._meta.get_fields()
if getattr(f, 'unique', False)
}
unique_foreign_fields.update({
frozenset(ut)
for ut in self.remote_field.model._meta.unique_together
})
foreign_fields = {f.name for f in self.foreign_related_fields}
has_unique_constraint = any(u <= foreign_fields for u in unique_foreign_fields)

if not has_unique_constraint and len(self.foreign_related_fields) > 1:
field_combination = ', '.join("'%s'" % rel_field.name
for rel_field in self.foreign_related_fields)
model_name = self.remote_field.model.__name__
return [
checks.Error(
"None of the fields %s on model '%s' have a unique=True constraint."
"No subset of the fields %s on model '%s' is unique."
% (field_combination, model_name),
hint=None,
hint=(
"Add unique=True on any of those fields or add at "
"least a subset of them to a unique_together constraint."
),
obj=self,
id='fields.E310',
)
]
elif not has_unique_field:
elif not has_unique_constraint:
field_name = self.foreign_related_fields[0].name
model_name = self.remote_field.model.__name__
return [
Expand Down
5 changes: 3 additions & 2 deletions docs/ref/checks.txt
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,9 @@ Related Fields
for ``<field name>``.
* **fields.E306**: Related name must be a valid Python identifier or end with
a ``'+'``.
* **fields.E310**: None of the fields ``<field1>``, ``<field2>``, ... on model
``<model>`` have a ``unique=True`` constraint.
* **fields.E310**: No subset of the fields ``<field1>``, ``<field2>``, ... on
model ``<model>`` is unique. Add ``unique=True`` on any of those fields or
add at least a subset of them to a unique_together constraint.
* **fields.E311**: ``<model>`` must set ``unique=True`` because it is
referenced by a ``ForeignKey``.
* **fields.E320**: Field specifies ``on_delete=SET_NULL``, but cannot be null.
Expand Down
53 changes: 52 additions & 1 deletion tests/foreign_object/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
from operator import attrgetter

from django.core.exceptions import FieldError
from django.test import TestCase, skipUnlessDBFeature
from django.db import models
from django.db.models.fields.related import ForeignObject
from django.test import SimpleTestCase, TestCase, skipUnlessDBFeature
from django.utils import translation

from .models import (
Expand Down Expand Up @@ -391,3 +393,52 @@ def test_batch_create_foreign_object(self):
""" See: https://code.djangoproject.com/ticket/21566 """
objs = [Person(name="abcd_%s" % i, person_country=self.usa) for i in range(0, 5)]
Person.objects.bulk_create(objs, 10)


class TestModelCheckTests(SimpleTestCase):

def test_check_composite_foreign_object(self):
class Parent(models.Model):
a = models.PositiveIntegerField()
b = models.PositiveIntegerField()

class Meta:
unique_together = (('a', 'b'),)

class Child(models.Model):
a = models.PositiveIntegerField()
b = models.PositiveIntegerField()
value = models.CharField(max_length=255)
parent = ForeignObject(
Parent,
on_delete=models.SET_NULL,
from_fields=('a', 'b'),
to_fields=('a', 'b'),
related_name='children',
)

self.assertEqual(Child._meta.get_field('parent').check(from_model=Child), [])

def test_check_subset_composite_foreign_object(self):
class Parent(models.Model):
a = models.PositiveIntegerField()
b = models.PositiveIntegerField()
c = models.PositiveIntegerField()

class Meta:
unique_together = (('a', 'b'),)

class Child(models.Model):
a = models.PositiveIntegerField()
b = models.PositiveIntegerField()
c = models.PositiveIntegerField()
d = models.CharField(max_length=255)
parent = ForeignObject(
Parent,
on_delete=models.SET_NULL,
from_fields=('a', 'b', 'c'),
to_fields=('a', 'b', 'c'),
related_name='children',
)

self.assertEqual(Child._meta.get_field('parent').check(from_model=Child), [])
83 changes: 80 additions & 3 deletions tests/invalid_models_tests/test_relative_fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

from django.core.checks import Error, Warning as DjangoWarning
from django.db import models
from django.db.models.fields.related import ForeignObject
from django.test import ignore_warnings
from django.test.testcases import skipIfDBFeature
from django.test.utils import override_settings
Expand Down Expand Up @@ -507,9 +508,11 @@ class MMembership(models.Model):
errors = field.check()
expected = [
Error(
"None of the fields 'country_id', 'city_id' on model 'Person' "
"have a unique=True constraint.",
hint=None,
"No subset of the fields 'country_id', 'city_id' on model 'Person' is unique.",
hint=(
"Add unique=True on any of those fields or add at least "
"a subset of them to a unique_together constraint."
),
obj=field,
id='fields.E310',
)
Expand Down Expand Up @@ -1395,3 +1398,77 @@ class Invitation(models.Model):
obj=field,
id='fields.E337')]
self.assertEqual(expected, errors)

def test_superset_foreign_object(self):
class Parent(models.Model):
a = models.PositiveIntegerField()
b = models.PositiveIntegerField()
c = models.PositiveIntegerField()

class Meta:
unique_together = (('a', 'b', 'c'),)

class Child(models.Model):
a = models.PositiveIntegerField()
b = models.PositiveIntegerField()
value = models.CharField(max_length=255)
parent = ForeignObject(
Parent,
on_delete=models.SET_NULL,
from_fields=('a', 'b'),
to_fields=('a', 'b'),
related_name='children',
)

field = Child._meta.get_field('parent')
errors = field.check(from_model=Child)
expected = [
Error(
"No subset of the fields 'a', 'b' on model 'Parent' is unique.",
hint=(
"Add unique=True on any of those fields or add at least "
"a subset of them to a unique_together constraint."
),
obj=field,
id='fields.E310',
),
]
self.assertEqual(expected, errors)

def test_insersection_foreign_object(self):
class Parent(models.Model):
a = models.PositiveIntegerField()
b = models.PositiveIntegerField()
c = models.PositiveIntegerField()
d = models.PositiveIntegerField()

class Meta:
unique_together = (('a', 'b', 'c'),)

class Child(models.Model):
a = models.PositiveIntegerField()
b = models.PositiveIntegerField()
d = models.PositiveIntegerField()
value = models.CharField(max_length=255)
parent = ForeignObject(
Parent,
on_delete=models.SET_NULL,
from_fields=('a', 'b', 'd'),
to_fields=('a', 'b', 'd'),
related_name='children',
)

field = Child._meta.get_field('parent')
errors = field.check(from_model=Child)
expected = [
Error(
"No subset of the fields 'a', 'b', 'd' on model 'Parent' is unique.",
hint=(
"Add unique=True on any of those fields or add at least "
"a subset of them to a unique_together constraint."
),
obj=field,
id='fields.E310',
),
]
self.assertEqual(expected, errors)

0 comments on commit 38d6e1e

Please sign in to comment.