Fixed #22064 -- Add check for related_name
Validates that related_name is a valid Python id or ends with a '+' and it's not a keyword. Without a check it passed silently leading to unpredictable problems. Thanks Konrad Świat for the initial work.
This commit is contained in:
parent
8c581ff394
commit
1e5e2a4707
|
@ -97,11 +97,32 @@ signals.class_prepared.connect(do_pending_lookups)
|
||||||
class RelatedField(Field):
|
class RelatedField(Field):
|
||||||
def check(self, **kwargs):
|
def check(self, **kwargs):
|
||||||
errors = super(RelatedField, self).check(**kwargs)
|
errors = super(RelatedField, self).check(**kwargs)
|
||||||
|
errors.extend(self._check_related_name_is_valid())
|
||||||
errors.extend(self._check_relation_model_exists())
|
errors.extend(self._check_relation_model_exists())
|
||||||
errors.extend(self._check_referencing_to_swapped_model())
|
errors.extend(self._check_referencing_to_swapped_model())
|
||||||
errors.extend(self._check_clashes())
|
errors.extend(self._check_clashes())
|
||||||
return errors
|
return errors
|
||||||
|
|
||||||
|
def _check_related_name_is_valid(self):
|
||||||
|
import re
|
||||||
|
import keyword
|
||||||
|
related_name = self.rel.related_name
|
||||||
|
|
||||||
|
is_valid_id = (related_name and re.match('^[a-zA-Z_][a-zA-Z0-9_]*$', related_name)
|
||||||
|
and not keyword.iskeyword(related_name))
|
||||||
|
if related_name and not (is_valid_id or related_name.endswith('+')):
|
||||||
|
return [
|
||||||
|
checks.Error(
|
||||||
|
"The name '%s' is invalid related_name for field %s.%s" %
|
||||||
|
(self.rel.related_name, self.model._meta.object_name,
|
||||||
|
self.name),
|
||||||
|
hint="Related name must be a valid Python identifier or end with a '+'",
|
||||||
|
obj=self,
|
||||||
|
id='fields.E306',
|
||||||
|
)
|
||||||
|
]
|
||||||
|
return []
|
||||||
|
|
||||||
def _check_relation_model_exists(self):
|
def _check_relation_model_exists(self):
|
||||||
rel_is_missing = self.rel.to not in apps.get_models()
|
rel_is_missing = self.rel.to not in apps.get_models()
|
||||||
rel_is_string = isinstance(self.rel.to, six.string_types)
|
rel_is_string = isinstance(self.rel.to, six.string_types)
|
||||||
|
|
|
@ -117,6 +117,8 @@ Related Fields
|
||||||
``<field name>``.
|
``<field name>``.
|
||||||
* **fields.E305**: Field name ``<field name>`` clashes with reverse query name
|
* **fields.E305**: Field name ``<field name>`` clashes with reverse query name
|
||||||
for ``<field name>``.
|
for ``<field name>``.
|
||||||
|
* **fields.E306**: Related name must be a valid Python identifier or end with
|
||||||
|
a ``'+'``.
|
||||||
* **fields.E310**: None of the fields ``<field1>``, ``<field2>``, ... on model
|
* **fields.E310**: None of the fields ``<field1>``, ``<field2>``, ... on model
|
||||||
``<model>`` have a ``unique=True`` constraint.
|
``<model>`` have a ``unique=True`` constraint.
|
||||||
* **fields.E311**: ``<model>`` must set ``unique=True`` because it is
|
* **fields.E311**: ``<model>`` must set ``unique=True`` because it is
|
||||||
|
|
|
@ -546,6 +546,73 @@ class RelativeFieldTests(IsolatedModelsTestCase):
|
||||||
errors = field.check(from_model=Model)
|
errors = field.check(from_model=Model)
|
||||||
self.assertEqual(errors, [expected_error])
|
self.assertEqual(errors, [expected_error])
|
||||||
|
|
||||||
|
def test_related_field_has_invalid_related_name(self):
|
||||||
|
digit = 0
|
||||||
|
illegal_non_alphanumeric = '!'
|
||||||
|
whitespace = '\t'
|
||||||
|
|
||||||
|
invalid_related_names = [
|
||||||
|
'%s_begins_with_digit' % digit,
|
||||||
|
'%s_begins_with_illegal_non_alphanumeric' % illegal_non_alphanumeric,
|
||||||
|
'%s_begins_with_whitespace' % whitespace,
|
||||||
|
'contains_%s_illegal_non_alphanumeric' % illegal_non_alphanumeric,
|
||||||
|
'contains_%s_whitespace' % whitespace,
|
||||||
|
'ends_with_with_illegal_non_alphanumeric_%s' % illegal_non_alphanumeric,
|
||||||
|
'ends_with_whitespace_%s' % whitespace,
|
||||||
|
# Python's keyword
|
||||||
|
'with',
|
||||||
|
]
|
||||||
|
|
||||||
|
class Parent(models.Model):
|
||||||
|
pass
|
||||||
|
|
||||||
|
for invalid_related_name in invalid_related_names:
|
||||||
|
Child = type(str('Child_%s') % str(invalid_related_name), (models.Model,), {
|
||||||
|
'parent': models.ForeignKey('Parent', related_name=invalid_related_name),
|
||||||
|
'__module__': Parent.__module__,
|
||||||
|
})
|
||||||
|
|
||||||
|
field = Child._meta.get_field('parent')
|
||||||
|
errors = Child.check()
|
||||||
|
expected = [
|
||||||
|
Error(
|
||||||
|
"The name '%s' is invalid related_name for field Child_%s.parent"
|
||||||
|
% (invalid_related_name, invalid_related_name),
|
||||||
|
hint="Related name must be a valid Python identifier or end with a '+'",
|
||||||
|
obj=field,
|
||||||
|
id='fields.E306',
|
||||||
|
),
|
||||||
|
]
|
||||||
|
self.assertEqual(errors, expected)
|
||||||
|
|
||||||
|
def test_related_field_has_valid_related_name(self):
|
||||||
|
lowercase = 'a'
|
||||||
|
uppercase = 'A'
|
||||||
|
digit = 0
|
||||||
|
|
||||||
|
related_names = [
|
||||||
|
'%s_starts_with_lowercase' % lowercase,
|
||||||
|
'%s_tarts_with_uppercase' % uppercase,
|
||||||
|
'_starts_with_underscore',
|
||||||
|
'contains_%s_digit' % digit,
|
||||||
|
'ends_with_plus+',
|
||||||
|
'_',
|
||||||
|
'_+',
|
||||||
|
'+',
|
||||||
|
]
|
||||||
|
|
||||||
|
class Parent(models.Model):
|
||||||
|
pass
|
||||||
|
|
||||||
|
for related_name in related_names:
|
||||||
|
Child = type(str('Child_%s') % str(related_name), (models.Model,), {
|
||||||
|
'parent': models.ForeignKey('Parent', related_name=related_name),
|
||||||
|
'__module__': Parent.__module__,
|
||||||
|
})
|
||||||
|
|
||||||
|
errors = Child.check()
|
||||||
|
self.assertFalse(errors)
|
||||||
|
|
||||||
|
|
||||||
class AccessorClashTests(IsolatedModelsTestCase):
|
class AccessorClashTests(IsolatedModelsTestCase):
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue