From 53209f78302a639032afabf5326d28d4ddd9d03c Mon Sep 17 00:00:00 2001 From: can Date: Thu, 4 Jul 2019 19:21:50 +0300 Subject: [PATCH] Fixed #30613 -- Moved index name validation to system checks. --- django/db/models/base.py | 27 +++++++++++++++++-- django/db/models/indexes.py | 21 ++------------- docs/ref/checks.txt | 4 +++ tests/invalid_models_tests/test_models.py | 33 +++++++++++++++++++++++ tests/model_indexes/tests.py | 14 ---------- 5 files changed, 64 insertions(+), 35 deletions(-) diff --git a/django/db/models/base.py b/django/db/models/base.py index 71d04244e0..1ddbde9393 100644 --- a/django/db/models/base.py +++ b/django/db/models/base.py @@ -1567,9 +1567,32 @@ class Model(metaclass=ModelBase): @classmethod def _check_indexes(cls): - """Check the fields of indexes.""" + """Check the fields and names of indexes.""" + errors = [] + for index in cls._meta.indexes: + # Index name can't start with an underscore or a number, restricted + # for cross-database compatibility with Oracle. + if index.name[0] == '_' or index.name[0].isdigit(): + errors.append( + checks.Error( + "The index name '%s' cannot start with an underscore " + "or a number." % index.name, + obj=cls, + id='models.E033', + ), + ) + if len(index.name) > index.max_name_length: + errors.append( + checks.Error( + "The index name '%s' cannot be longer than %d " + "characters." % (index.name, index.max_name_length), + obj=cls, + id='models.E034', + ), + ) fields = [field for index in cls._meta.indexes for field, _ in index.fields_orders] - return cls._check_local_fields(fields, 'indexes') + errors.extend(cls._check_local_fields(fields, 'indexes')) + return errors @classmethod def _check_local_fields(cls, fields, option): diff --git a/django/db/models/indexes.py b/django/db/models/indexes.py index a8bf089ba1..c093b45619 100644 --- a/django/db/models/indexes.py +++ b/django/db/models/indexes.py @@ -33,28 +33,10 @@ class Index: for field_name in self.fields ] self.name = name or '' - if self.name: - errors = self.check_name() - if len(self.name) > self.max_name_length: - errors.append('Index names cannot be longer than %s characters.' % self.max_name_length) - if errors: - raise ValueError(errors) self.db_tablespace = db_tablespace self.opclasses = opclasses self.condition = condition - def check_name(self): - errors = [] - # Name can't start with an underscore on Oracle; prepend D if needed. - if self.name[0] == '_': - errors.append('Index names cannot start with an underscore (_).') - self.name = 'D%s' % self.name[1:] - # Name can't start with a number on Oracle; prepend D if needed. - elif self.name[0].isdigit(): - errors.append('Index names cannot start with a number (0-9).') - self.name = 'D%s' % self.name[1:] - return errors - def _get_condition_sql(self, model, schema_editor): if self.condition is None: return None @@ -122,7 +104,8 @@ class Index: 'Index too long for multiple database support. Is self.suffix ' 'longer than 3 characters?' ) - self.check_name() + if self.name[0] == '_' or self.name[0].isdigit(): + self.name = 'D%s' % self.name[1:] def __repr__(self): return "<%s: fields='%s'%s>" % ( diff --git a/docs/ref/checks.txt b/docs/ref/checks.txt index 20fb50e131..b1f9e085b4 100644 --- a/docs/ref/checks.txt +++ b/docs/ref/checks.txt @@ -313,6 +313,10 @@ Models ````. * **models.E032**: constraint name ```` is not unique amongst models: ````. +* **models.E033**: The index name ```` cannot start with an underscore + or a number. +* **models.E034**: The index name ```` cannot be longer than + ```` characters. Security -------- diff --git a/tests/invalid_models_tests/test_models.py b/tests/invalid_models_tests/test_models.py index 32617555a2..18a59c407d 100644 --- a/tests/invalid_models_tests/test_models.py +++ b/tests/invalid_models_tests/test_models.py @@ -296,6 +296,39 @@ class IndexesTests(SimpleTestCase): self.assertEqual(Bar.check(), []) + def test_name_constraints(self): + class Model(models.Model): + class Meta: + indexes = [ + models.Index(fields=['id'], name='_index_name'), + models.Index(fields=['id'], name='5index_name'), + ] + + self.assertEqual(Model.check(), [ + Error( + "The index name '%sindex_name' cannot start with an " + "underscore or a number." % prefix, + obj=Model, + id='models.E033', + ) for prefix in ('_', '5') + ]) + + def test_max_name_length(self): + index_name = 'x' * 31 + + class Model(models.Model): + class Meta: + indexes = [models.Index(fields=['id'], name=index_name)] + + self.assertEqual(Model.check(), [ + Error( + "The index name '%s' cannot be longer than 30 characters." + % index_name, + obj=Model, + id='models.E034', + ), + ]) + @isolate_apps('invalid_models_tests') class FieldNamesTests(SimpleTestCase): diff --git a/tests/model_indexes/tests.py b/tests/model_indexes/tests.py index 60fc0560e4..5180d8cee9 100644 --- a/tests/model_indexes/tests.py +++ b/tests/model_indexes/tests.py @@ -63,20 +63,6 @@ class SimpleIndexesTests(SimpleTestCase): with self.assertRaisesMessage(ValueError, 'Index.condition must be a Q instance.'): models.Index(condition='invalid', name='long_book_idx') - def test_max_name_length(self): - msg = 'Index names cannot be longer than 30 characters.' - with self.assertRaisesMessage(ValueError, msg): - models.Index(fields=['title'], name='looooooooooooong_index_name_idx') - - def test_name_constraints(self): - msg = 'Index names cannot start with an underscore (_).' - with self.assertRaisesMessage(ValueError, msg): - models.Index(fields=['title'], name='_name_starting_with_underscore') - - msg = 'Index names cannot start with a number (0-9).' - with self.assertRaisesMessage(ValueError, msg): - models.Index(fields=['title'], name='5name_starting_with_number') - def test_name_auto_generation(self): index = models.Index(fields=['author']) index.set_name_with_model(Book)