Skip to content

Commit

Permalink
Fixed #31410 -- Added check for fields of UniqueConstraints and Check…
Browse files Browse the repository at this point in the history
…Constraint.
  • Loading branch information
hramezani committed May 4, 2020
1 parent 0ebabe3 commit 69f28cd
Show file tree
Hide file tree
Showing 3 changed files with 146 additions and 1 deletion.
28 changes: 27 additions & 1 deletion django/db/models/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,12 @@
connections, router, transaction,
)
from django.db.models import (
NOT_PROVIDED, ExpressionWrapper, IntegerField, Max, Value,
NOT_PROVIDED, ExpressionWrapper, F, IntegerField, Max, Value,
)
from django.db.models.constants import LOOKUP_SEP
from django.db.models.constraints import CheckConstraint, UniqueConstraint
from django.db.models.deletion import CASCADE, Collector
from django.db.models.expressions import RawSQL
from django.db.models.fields.related import (
ForeignObjectRel, OneToOneField, lazy_related_operation, resolve_relation,
)
Expand Down Expand Up @@ -1923,6 +1924,31 @@ def _check_constraints(cls, databases):
id='models.W038',
)
)

def fetch_check_constraint_check_fields(check):
fields = set()
if isinstance(check, (ExpressionWrapper, RawSQL)):
return fields
for child in check.children:
if isinstance(child, Q):
fields.update(fetch_check_constraint_check_fields(child))
else:
fields.add(child[0].split(LOOKUP_SEP)[0])
if isinstance(child[1], F):
fields.add(child[1].name)
return fields

for constraint in cls._meta.constraints:
if isinstance(constraint, UniqueConstraint):
errors.extend(cls._check_local_fields(constraint.fields, "unique_constraint"))
elif isinstance(constraint, CheckConstraint):
errors.extend(
cls._check_local_fields(
fetch_check_constraint_check_fields(constraint.check),
"check_constraint"
)
)

return errors


Expand Down
3 changes: 3 additions & 0 deletions docs/releases/3.1.txt
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,9 @@ Models
* The new :attr:`.UniqueConstraint.deferrable` attribute allows creating
deferrable unique constraints.

* Added check for fields of :class:`.UniqueConstraint` and
:class:`.CheckConstraint`.

Pagination
~~~~~~~~~~

Expand Down
116 changes: 116 additions & 0 deletions tests/invalid_models_tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1371,6 +1371,122 @@ class Meta:
constraints = [models.CheckConstraint(check=models.Q(age__gte=18), name='is_adult')]
self.assertEqual(Model.check(databases=self.databases), [])

def test_constraint_pointing_to_missing_field(self):
class Model(models.Model):
class Meta:
constraints = [
models.UniqueConstraint(fields=['missing_field'], name='name1'),
models.CheckConstraint(check=models.Q(missing_field__gte=1), name='name2'),
]

self.assertEqual(Model.check(databases=self.databases), [
Error(
"'unique_constraint' refers to the nonexistent field 'missing_field'.",
obj=Model,
id='models.E012',
),
Error(
"'check_constraint' refers to the nonexistent field 'missing_field'.",
obj=Model,
id='models.E012',
),

])

def test_constraint_pointing_to_m2m_field(self):
class Model(models.Model):
m2m = models.ManyToManyField('self')

class Meta:
constraints = [
models.UniqueConstraint(fields=['m2m'], name='name1'),
models.CheckConstraint(check=models.Q(m2m=1), name='name2'),
]

self.assertEqual(Model.check(databases=self.databases), [
Error(
"'unique_constraint' refers to a ManyToManyField 'm2m', but "
"ManyToManyFields are not permitted in 'unique_constraint'.",
obj=Model,
id='models.E013',
),
Error(
"'check_constraint' refers to a ManyToManyField 'm2m', but "
"ManyToManyFields are not permitted in 'check_constraint'.",
obj=Model,
id='models.E013',
),
])

def test_pointing_to_non_local_field(self):
class Foo(models.Model):
field1 = models.IntegerField()

class Bar(Foo):
field2 = models.IntegerField()

class Meta:
constraints = [
models.UniqueConstraint(fields=['field2', 'field1'], name='name1'),
models.CheckConstraint(
check=models.Q(models.Q(models.Q(field2=1) | models.Q(field2=2)) & models.Q(field1=1)),
name='name2'
),
]

self.assertEqual(Bar.check(databases=self.databases), [
Error(
"'unique_constraint' refers to field 'field1' which is not local to "
"model 'Bar'.",
hint='This issue may be caused by multi-table inheritance.',
obj=Bar,
id='models.E016',
),
Error(
"'check_constraint' refers to field 'field1' which is not local to "
"model 'Bar'.",
hint='This issue may be caused by multi-table inheritance.',
obj=Bar,
id='models.E016',
),
])

def test_pointing_to_fk(self):
class Foo(models.Model):
pass

class Bar(models.Model):
foo_1 = models.ForeignKey(Foo, on_delete=models.CASCADE, related_name='bar_1')
foo_2 = models.ForeignKey(Foo, on_delete=models.CASCADE, related_name='bar_2')

class Meta:
constraints = [
models.UniqueConstraint(fields=['foo_1_id', 'foo_2'], name='name1'),
models.CheckConstraint(check=models.Q(foo_1_id__gte=0), name='name2'),
]

self.assertEqual(Bar.check(databases=self.databases), [])

def test_constraint_pointing_to_missing_f_expression(self):
class Model(models.Model):
field1 = models.IntegerField()

class Meta:
constraints = [
models.CheckConstraint(
check=models.Q(field1__gte=models.F('missing_f')), name='name'
),
]

self.assertEqual(Model.check(databases=self.databases), [
Error(
"'check_constraint' refers to the nonexistent field 'missing_f'.",
obj=Model,
id='models.E012',
),

])

def test_unique_constraint_with_condition(self):
class Model(models.Model):
age = models.IntegerField()
Expand Down

0 comments on commit 69f28cd

Please sign in to comment.