From febe136d4c3310ec8901abecca3ea5ba2be3952c Mon Sep 17 00:00:00 2001 From: can Date: Fri, 5 Jul 2019 15:15:41 +0300 Subject: [PATCH] Fixed #30397 -- Added app_label/class interpolation for names of indexes and constraints. --- django/db/models/options.py | 18 ++++++ docs/ref/models/constraints.txt | 15 ++++- docs/ref/models/indexes.txt | 10 +++- docs/releases/3.0.txt | 5 ++ tests/check_framework/test_model_checks.py | 66 ++++++++++++++++++++++ tests/constraints/models.py | 21 +++++++ tests/constraints/tests.py | 15 ++++- tests/model_indexes/models.py | 8 ++- tests/model_indexes/tests.py | 19 ++++++- 9 files changed, 166 insertions(+), 11 deletions(-) diff --git a/django/db/models/options.py b/django/db/models/options.py index fea65f7626..1f11e26d87 100644 --- a/django/db/models/options.py +++ b/django/db/models/options.py @@ -180,6 +180,12 @@ class Options: self.unique_together = normalize_together(self.unique_together) self.index_together = normalize_together(self.index_together) + # App label/class name interpolation for names of constraints and + # indexes. + if not getattr(cls._meta, 'abstract', False): + for attr_name in {'constraints', 'indexes'}: + objs = getattr(self, attr_name, []) + setattr(self, attr_name, self._format_names_with_class(cls, objs)) # verbose_name_plural is a special case because it uses a 's' # by default. @@ -201,6 +207,18 @@ class Options: self.db_table = "%s_%s" % (self.app_label, self.model_name) self.db_table = truncate_name(self.db_table, connection.ops.max_name_length()) + def _format_names_with_class(self, cls, objs): + """App label/class name interpolation for object names.""" + new_objs = [] + for obj in objs: + obj = obj.clone() + obj.name = obj.name % { + 'app_label': cls._meta.app_label.lower(), + 'class': cls.__name__.lower(), + } + new_objs.append(obj) + return new_objs + def _prepare(self, model): if self.order_with_respect_to: # The app registry will not be ready at this point, so we cannot diff --git a/docs/ref/models/constraints.txt b/docs/ref/models/constraints.txt index d172353326..b5b77b8654 100644 --- a/docs/ref/models/constraints.txt +++ b/docs/ref/models/constraints.txt @@ -25,8 +25,11 @@ option. cannot normally specify a constraint on an abstract base class, since the :attr:`Meta.constraints ` option is inherited by subclasses, with exactly the same values for the attributes - (including ``name``) each time. Instead, specify the ``constraints`` option - on subclasses directly, providing a unique name for each constraint. + (including ``name``) each time. To work around name collisions, part of the + name may contain ``'%(app_label)s'`` and ``'%(class)s'``, which are + replaced, respectively, by the lowercased app label and class name of the + concrete model. For example ``CheckConstraint(check=Q(age__gte=18), + name='%(app_label)s_%(class)s_is_adult')``. .. admonition:: Validation of Constraints @@ -63,6 +66,10 @@ ensures the age field is never less than 18. The name of the constraint. +.. versionchanged:: 3.0 + + Interpolation of ``'%(app_label)s'`` and ``'%(class)s'`` was added. + ``UniqueConstraint`` ==================== @@ -89,6 +96,10 @@ date. The name of the constraint. +.. versionchanged:: 3.0 + + Interpolation of ``'%(app_label)s'`` and ``'%(class)s'`` was added. + ``condition`` ------------- diff --git a/docs/ref/models/indexes.txt b/docs/ref/models/indexes.txt index 13e37f78c7..5909ee9c64 100644 --- a/docs/ref/models/indexes.txt +++ b/docs/ref/models/indexes.txt @@ -55,9 +55,15 @@ than 30 characters and shouldn't start with a number (0-9) or underscore (_). cannot normally specify a partial index on an abstract base class, since the :attr:`Meta.indexes ` option is inherited by subclasses, with exactly the same values for the attributes - (including ``name``) each time. Instead, specify the ``indexes`` option - on subclasses directly, providing a unique name for each index. + (including ``name``) each time. To work around name collisions, part of the + name may contain ``'%(app_label)s'`` and ``'%(class)s'``, which are + replaced, respectively, by the lowercased app label and class name of the + concrete model. For example ``Index(fields=['title'], + name='%(app_label)s_%(class)s_title_index')``. +.. versionchanged:: 3.0 + + Interpolation of ``'%(app_label)s'`` and ``'%(class)s'`` was added. ``db_tablespace`` ----------------- diff --git a/docs/releases/3.0.txt b/docs/releases/3.0.txt index 6c25f79d7b..c4cc480031 100644 --- a/docs/releases/3.0.txt +++ b/docs/releases/3.0.txt @@ -263,6 +263,11 @@ Models * Allowed symmetrical intermediate table for self-referential :class:`~django.db.models.ManyToManyField`. +* The ``name`` attributes of :class:`~django.db.models.CheckConstraint`, + :class:`~django.db.models.UniqueConstraint`, and + :class:`~django.db.models.Index` now support app label and class + interpolation using the ``'%(app_label)s'`` and ``'%(class)s'`` placeholders. + Requests and Responses ~~~~~~~~~~~~~~~~~~~~~~ diff --git a/tests/check_framework/test_model_checks.py b/tests/check_framework/test_model_checks.py index 0cbc0aff44..79177e38f7 100644 --- a/tests/check_framework/test_model_checks.py +++ b/tests/check_framework/test_model_checks.py @@ -131,6 +131,22 @@ class IndexNameTests(SimpleTestCase): ), ]) + def test_no_collision_abstract_model_interpolation(self): + class AbstractModel(models.Model): + name = models.CharField(max_length=20) + + class Meta: + indexes = [models.Index(fields=['name'], name='%(app_label)s_%(class)s_foo')] + abstract = True + + class Model1(AbstractModel): + pass + + class Model2(AbstractModel): + pass + + self.assertEqual(checks.run_checks(app_configs=self.apps.get_app_configs()), []) + @modify_settings(INSTALLED_APPS={'append': 'basic'}) @isolate_apps('basic', 'check_framework', kwarg_name='apps') def test_collision_across_apps(self, apps): @@ -154,6 +170,23 @@ class IndexNameTests(SimpleTestCase): ), ]) + @modify_settings(INSTALLED_APPS={'append': 'basic'}) + @isolate_apps('basic', 'check_framework', kwarg_name='apps') + def test_no_collision_across_apps_interpolation(self, apps): + index = models.Index(fields=['id'], name='%(app_label)s_%(class)s_foo') + + class Model1(models.Model): + class Meta: + app_label = 'basic' + constraints = [index] + + class Model2(models.Model): + class Meta: + app_label = 'check_framework' + constraints = [index] + + self.assertEqual(checks.run_checks(app_configs=apps.get_app_configs()), []) + @isolate_apps('check_framework', attr_name='apps') @override_system_checks([checks.model_checks.check_all_models]) @@ -214,6 +247,22 @@ class ConstraintNameTests(TestCase): ), ]) + def test_no_collision_abstract_model_interpolation(self): + class AbstractModel(models.Model): + class Meta: + constraints = [ + models.CheckConstraint(check=models.Q(id__gt=0), name='%(app_label)s_%(class)s_foo'), + ] + abstract = True + + class Model1(AbstractModel): + pass + + class Model2(AbstractModel): + pass + + self.assertEqual(checks.run_checks(app_configs=self.apps.get_app_configs()), []) + @modify_settings(INSTALLED_APPS={'append': 'basic'}) @isolate_apps('basic', 'check_framework', kwarg_name='apps') def test_collision_across_apps(self, apps): @@ -236,3 +285,20 @@ class ConstraintNameTests(TestCase): id='models.E032', ), ]) + + @modify_settings(INSTALLED_APPS={'append': 'basic'}) + @isolate_apps('basic', 'check_framework', kwarg_name='apps') + def test_no_collision_across_apps_interpolation(self, apps): + constraint = models.CheckConstraint(check=models.Q(id__gt=0), name='%(app_label)s_%(class)s_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()), []) diff --git a/tests/constraints/models.py b/tests/constraints/models.py index f316b95161..fb6ef620f0 100644 --- a/tests/constraints/models.py +++ b/tests/constraints/models.py @@ -13,6 +13,10 @@ class Product(models.Model): check=models.Q(price__gt=models.F('discounted_price')), name='price_gt_discounted_price', ), + models.CheckConstraint( + check=models.Q(price__gt=0), + name='%(app_label)s_%(class)s_price_gt_0', + ), models.UniqueConstraint(fields=['name', 'color'], name='name_color_uniq'), models.UniqueConstraint( fields=['name'], @@ -20,3 +24,20 @@ class Product(models.Model): condition=models.Q(color__isnull=True), ), ] + + +class AbstractModel(models.Model): + age = models.IntegerField() + + class Meta: + abstract = True + constraints = [ + models.CheckConstraint( + check=models.Q(age__gte=18), + name='%(app_label)s_%(class)s_adult', + ), + ] + + +class ChildModel(AbstractModel): + pass diff --git a/tests/constraints/tests.py b/tests/constraints/tests.py index b7aeb1e7f0..c2f99c565d 100644 --- a/tests/constraints/tests.py +++ b/tests/constraints/tests.py @@ -3,7 +3,7 @@ from django.db import IntegrityError, connection, models from django.db.models.constraints import BaseConstraint from django.test import SimpleTestCase, TestCase, skipUnlessDBFeature -from .models import Product +from .models import ChildModel, Product def get_constraints(table): @@ -76,8 +76,17 @@ class CheckConstraintTests(TestCase): @skipUnlessDBFeature('supports_table_check_constraints') def test_name(self): constraints = get_constraints(Product._meta.db_table) - expected_name = 'price_gt_discounted_price' - self.assertIn(expected_name, constraints) + for expected_name in ( + 'price_gt_discounted_price', + 'constraints_product_price_gt_0', + ): + with self.subTest(expected_name): + self.assertIn(expected_name, constraints) + + @skipUnlessDBFeature('supports_table_check_constraints') + def test_abstract_name(self): + constraints = get_constraints(ChildModel._meta.db_table) + self.assertIn('constraints_childmodel_adult', constraints) class UniqueConstraintTests(TestCase): diff --git a/tests/model_indexes/models.py b/tests/model_indexes/models.py index 6f7548261f..42651cd868 100644 --- a/tests/model_indexes/models.py +++ b/tests/model_indexes/models.py @@ -7,20 +7,26 @@ class Book(models.Model): pages = models.IntegerField(db_column='page_count') shortcut = models.CharField(max_length=50, db_tablespace='idx_tbls') isbn = models.CharField(max_length=50, db_tablespace='idx_tbls') + barcode = models.CharField(max_length=31) class Meta: indexes = [ models.Index(fields=['title']), models.Index(fields=['isbn', 'id']), + models.Index(fields=['barcode'], name='%(app_label)s_%(class)s_barcode_idx'), ] class AbstractModel(models.Model): name = models.CharField(max_length=50) + shortcut = models.CharField(max_length=3) class Meta: abstract = True - indexes = [models.Index(fields=['name'])] + indexes = [ + models.Index(fields=['name']), + models.Index(fields=['shortcut'], name='%(app_label)s_%(class)s_idx'), + ] class ChildModel1(AbstractModel): diff --git a/tests/model_indexes/tests.py b/tests/model_indexes/tests.py index 5180d8cee9..ade27e1a4b 100644 --- a/tests/model_indexes/tests.py +++ b/tests/model_indexes/tests.py @@ -134,13 +134,26 @@ class SimpleIndexesTests(SimpleTestCase): def test_name_set(self): index_names = [index.name for index in Book._meta.indexes] - self.assertCountEqual(index_names, ['model_index_title_196f42_idx', 'model_index_isbn_34f975_idx']) + self.assertCountEqual( + index_names, + [ + 'model_index_title_196f42_idx', + 'model_index_isbn_34f975_idx', + 'model_indexes_book_barcode_idx', + ], + ) def test_abstract_children(self): index_names = [index.name for index in ChildModel1._meta.indexes] - self.assertEqual(index_names, ['model_index_name_440998_idx']) + self.assertEqual( + index_names, + ['model_index_name_440998_idx', 'model_indexes_childmodel1_idx'], + ) index_names = [index.name for index in ChildModel2._meta.indexes] - self.assertEqual(index_names, ['model_index_name_b6c374_idx']) + self.assertEqual( + index_names, + ['model_index_name_b6c374_idx', 'model_indexes_childmodel2_idx'], + ) class IndexesTests(TestCase):