diff --git a/AUTHORS b/AUTHORS index 6983965cd1..e9c027167c 100644 --- a/AUTHORS +++ b/AUTHORS @@ -152,6 +152,7 @@ answer newbie questions, and generally made Django that much better: Cameron Curry Cameron Knight (ckknight) Can Burak Çilingir + Can Sarıgöl Carl Meyer Carles Pina i Estany Carlos Eduardo de Paula diff --git a/django/core/checks/model_checks.py b/django/core/checks/model_checks.py index 6c6ac2c7f4..5c2266ca1d 100644 --- a/django/core/checks/model_checks.py +++ b/django/core/checks/model_checks.py @@ -10,6 +10,8 @@ from django.core.checks import Error, Tags, register @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() @@ -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( @@ -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 diff --git a/docs/ref/checks.txt b/docs/ref/checks.txt index 973ccebe90..a15eb558fe 100644 --- a/docs/ref/checks.txt +++ b/docs/ref/checks.txt @@ -306,6 +306,13 @@ Models * **models.W027**: ```` does not support check constraints. * **models.E028**: ``db_table`` ```` is used by multiple models: ````. +* **models.E029**: index name ```` is not unique for model ````. +* **models.E030**: index name ```` is not unique amongst models: + ````. +* **models.E031**: constraint name ```` is not unique for model + ````. +* **models.E032**: constraint name ```` is not unique amongst + models: ````. Security -------- diff --git a/tests/check_framework/test_model_checks.py b/tests/check_framework/test_model_checks.py index 2e55ad637d..0cbc0aff44 100644 --- a/tests/check_framework/test_model_checks.py +++ b/tests/check_framework/test_model_checks.py @@ -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, ) @@ -73,3 +73,166 @@ class DuplicateDBTableTests(SimpleTestCase): 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', + ), + ])