Skip to content

Commit

Permalink
Fixed #30396 -- Added system checks for uniqueness of indexes and con…
Browse files Browse the repository at this point in the history
…straints names.

Co-Authored-By: Mariusz Felisiak <[email protected]>
  • Loading branch information
can and felixxm committed May 2, 2019
1 parent 6485a5f commit bceadd2
Show file tree
Hide file tree
Showing 4 changed files with 204 additions and 1 deletion.
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ answer newbie questions, and generally made Django that much better:
Cameron Curry
Cameron Knight (ckknight)
Can Burak Çilingir <[email protected]>
Can Sarıgöl <[email protected]>
Carl Meyer <[email protected]>
Carles Pina i Estany <[email protected]>
Carlos Eduardo de Paula <[email protected]>
Expand Down
32 changes: 32 additions & 0 deletions django/core/checks/model_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
@register(Tags.models)
def check_all_models(app_configs=None, **kwargs):
db_table_models = defaultdict(list)
indexes = defaultdict(list)
constraints = defaultdict(list)
errors = []
if app_configs is None:
models = apps.get_models()
Expand All @@ -29,6 +31,10 @@ def check_all_models(app_configs=None, **kwargs):
)
else:
errors.extend(model.check(**kwargs))
for model_index in model._meta.indexes:
indexes[model_index.name].append(model._meta.label)
for model_constraint in model._meta.constraints:
constraints[model_constraint.name].append(model._meta.label)
for db_table, model_labels in db_table_models.items():
if len(model_labels) != 1:
errors.append(
Expand All @@ -39,6 +45,32 @@ def check_all_models(app_configs=None, **kwargs):
id='models.E028',
)
)
for index_name, model_labels in indexes.items():
if len(model_labels) > 1:
model_labels = set(model_labels)
errors.append(
Error(
"index name '%s' is not unique %s %s." % (
index_name,
'for model' if len(model_labels) == 1 else 'amongst models:',
', '.join(sorted(model_labels)),
),
id='models.E029' if len(model_labels) == 1 else 'models.E030',
),
)
for constraint_name, model_labels in constraints.items():
if len(model_labels) > 1:
model_labels = set(model_labels)
errors.append(
Error(
"constraint name '%s' is not unique %s %s." % (
constraint_name,
'for model' if len(model_labels) == 1 else 'amongst models:',
', '.join(sorted(model_labels)),
),
id='models.E031' if len(model_labels) == 1 else 'models.E032',
),
)
return errors


Expand Down
7 changes: 7 additions & 0 deletions docs/ref/checks.txt
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,13 @@ Models
* **models.W027**: ``<database>`` does not support check constraints.
* **models.E028**: ``db_table`` ``<db_table>`` is used by multiple models:
``<model list>``.
* **models.E029**: index name ``<index>`` is not unique for model ``<model>``.
* **models.E030**: index name ``<index>`` is not unique amongst models:
``<model list>``.
* **models.E031**: constraint name ``<constraint>`` is not unique for model
``<model>``.
* **models.E032**: constraint name ``<constraint>`` is not unique amongst
models: ``<model list>``.

Security
--------
Expand Down
165 changes: 164 additions & 1 deletion tests/check_framework/test_model_checks.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from django.core import checks
from django.core.checks import Error
from django.db import models
from django.test import SimpleTestCase
from django.test import SimpleTestCase, TestCase, skipUnlessDBFeature
from django.test.utils import (
isolate_apps, modify_settings, override_system_checks,
)
Expand Down Expand Up @@ -73,3 +73,166 @@ class Meta:

self.assertEqual(Model._meta.db_table, ProxyModel._meta.db_table)
self.assertEqual(checks.run_checks(app_configs=self.apps.get_app_configs()), [])


@isolate_apps('check_framework', attr_name='apps')
@override_system_checks([checks.model_checks.check_all_models])
class IndexNameTests(SimpleTestCase):
def test_collision_in_same_model(self):
index = models.Index(fields=['id'], name='foo')

class Model(models.Model):
class Meta:
indexes = [index, index]

self.assertEqual(checks.run_checks(app_configs=self.apps.get_app_configs()), [
Error(
"index name 'foo' is not unique for model check_framework.Model.",
id='models.E029',
),
])

def test_collision_in_different_models(self):
index = models.Index(fields=['id'], name='foo')

class Model1(models.Model):
class Meta:
indexes = [index]

class Model2(models.Model):
class Meta:
indexes = [index]

self.assertEqual(checks.run_checks(app_configs=self.apps.get_app_configs()), [
Error(
"index name 'foo' is not unique amongst models: "
"check_framework.Model1, check_framework.Model2.",
id='models.E030',
),
])

def test_collision_abstract_model(self):
class AbstractModel(models.Model):
class Meta:
indexes = [models.Index(fields=['id'], name='foo')]
abstract = True

class Model1(AbstractModel):
pass

class Model2(AbstractModel):
pass

self.assertEqual(checks.run_checks(app_configs=self.apps.get_app_configs()), [
Error(
"index name 'foo' is not unique amongst models: "
"check_framework.Model1, check_framework.Model2.",
id='models.E030',
),
])

@modify_settings(INSTALLED_APPS={'append': 'basic'})
@isolate_apps('basic', 'check_framework', kwarg_name='apps')
def test_collision_across_apps(self, apps):
index = models.Index(fields=['id'], name='foo')

class Model1(models.Model):
class Meta:
app_label = 'basic'
indexes = [index]

class Model2(models.Model):
class Meta:
app_label = 'check_framework'
indexes = [index]

self.assertEqual(checks.run_checks(app_configs=apps.get_app_configs()), [
Error(
"index name 'foo' is not unique amongst models: basic.Model1, "
"check_framework.Model2.",
id='models.E030',
),
])


@isolate_apps('check_framework', attr_name='apps')
@override_system_checks([checks.model_checks.check_all_models])
@skipUnlessDBFeature('supports_table_check_constraints')
class ConstraintNameTests(TestCase):
def test_collision_in_same_model(self):
class Model(models.Model):
class Meta:
constraints = [
models.CheckConstraint(check=models.Q(id__gt=0), name='foo'),
models.CheckConstraint(check=models.Q(id__lt=100), name='foo'),
]

self.assertEqual(checks.run_checks(app_configs=self.apps.get_app_configs()), [
Error(
"constraint name 'foo' is not unique for model "
"check_framework.Model.",
id='models.E031',
),
])

def test_collision_in_different_models(self):
constraint = models.CheckConstraint(check=models.Q(id__gt=0), name='foo')

class Model1(models.Model):
class Meta:
constraints = [constraint]

class Model2(models.Model):
class Meta:
constraints = [constraint]

self.assertEqual(checks.run_checks(app_configs=self.apps.get_app_configs()), [
Error(
"constraint name 'foo' is not unique amongst models: "
"check_framework.Model1, check_framework.Model2.",
id='models.E032',
),
])

def test_collision_abstract_model(self):
class AbstractModel(models.Model):
class Meta:
constraints = [models.CheckConstraint(check=models.Q(id__gt=0), name='foo')]
abstract = True

class Model1(AbstractModel):
pass

class Model2(AbstractModel):
pass

self.assertEqual(checks.run_checks(app_configs=self.apps.get_app_configs()), [
Error(
"constraint name 'foo' is not unique amongst models: "
"check_framework.Model1, check_framework.Model2.",
id='models.E032',
),
])

@modify_settings(INSTALLED_APPS={'append': 'basic'})
@isolate_apps('basic', 'check_framework', kwarg_name='apps')
def test_collision_across_apps(self, apps):
constraint = models.CheckConstraint(check=models.Q(id__gt=0), name='foo')

class Model1(models.Model):
class Meta:
app_label = 'basic'
constraints = [constraint]

class Model2(models.Model):
class Meta:
app_label = 'check_framework'
constraints = [constraint]

self.assertEqual(checks.run_checks(app_configs=apps.get_app_configs()), [
Error(
"constraint name 'foo' is not unique amongst models: "
"basic.Model1, check_framework.Model2.",
id='models.E032',
),
])

0 comments on commit bceadd2

Please sign in to comment.