Fixed #29408 -- Added validation of related fields and lookups in model Meta.ordering.

This commit is contained in:
Hasan Ramezani 2019-03-01 17:09:33 +01:00 committed by Mariusz Felisiak
parent 4dcbe6eb2d
commit 440505cb2c
4 changed files with 125 additions and 6 deletions

View File

@ -1643,9 +1643,35 @@ class Model(metaclass=ModelBase):
# Convert "-field" to "field". # Convert "-field" to "field".
fields = ((f[1:] if f.startswith('-') else f) for f in fields) fields = ((f[1:] if f.startswith('-') else f) for f in fields)
# Skip ordering in the format field1__field2 (FIXME: checking # Separate related field and non related fields.
# this format would be nice, but it's a little fiddly). _fields = []
fields = (f for f in fields if LOOKUP_SEP not in f) related_fields = []
for f in fields:
if LOOKUP_SEP in f:
related_fields.append(f)
else:
_fields.append(f)
fields = _fields
# Check related fields.
for field in related_fields:
_cls = cls
fld = None
for part in field.split(LOOKUP_SEP):
try:
fld = _cls._meta.get_field(part)
if fld.is_relation:
_cls = fld.get_path_info()[-1].to_opts.model
except (FieldDoesNotExist, AttributeError):
if fld is None or fld.get_transform(part) is None:
errors.append(
checks.Error(
"'ordering' refers to the nonexistent field, "
"related field or lookup '%s'." % field,
obj=cls,
id='models.E015',
)
)
# Skip ordering on pk. This is always a valid order_by field # Skip ordering on pk. This is always a valid order_by field
# but is an alias and therefore won't be found by opts.get_field. # but is an alias and therefore won't be found by opts.get_field.

View File

@ -277,8 +277,8 @@ Models
supported for that option. supported for that option.
* **models.E014**: ``ordering`` must be a tuple or list (even if you want to * **models.E014**: ``ordering`` must be a tuple or list (even if you want to
order by only one field). order by only one field).
* **models.E015**: ``ordering`` refers to the nonexistent field * **models.E015**: ``ordering`` refers to the nonexistent field, related field
``<field name>``. or lookup ``<field name>``.
* **models.E016**: ``indexes/index_together/unique_together`` refers to field * **models.E016**: ``indexes/index_together/unique_together`` refers to field
``<field_name>`` which is not local to model ``<model>``. ``<field_name>`` which is not local to model ``<model>``.
* **models.E017**: Proxy model ``<model>`` contains model fields. * **models.E017**: Proxy model ``<model>`` contains model fields.

View File

@ -5,9 +5,10 @@ from django.core.checks import Error, Warning
from django.core.checks.model_checks import _check_lazy_references from django.core.checks.model_checks import _check_lazy_references
from django.core.exceptions import ImproperlyConfigured from django.core.exceptions import ImproperlyConfigured
from django.db import connection, connections, models from django.db import connection, connections, models
from django.db.models.functions import Lower
from django.db.models.signals import post_init from django.db.models.signals import post_init
from django.test import SimpleTestCase from django.test import SimpleTestCase
from django.test.utils import isolate_apps, override_settings from django.test.utils import isolate_apps, override_settings, register_lookup
def get_max_column_name_length(): def get_max_column_name_length():
@ -665,6 +666,89 @@ class OtherModelTests(SimpleTestCase):
) )
]) ])
def test_ordering_pointing_to_missing_related_field(self):
class Model(models.Model):
test = models.IntegerField()
class Meta:
ordering = ('missing_related__id',)
self.assertEqual(Model.check(), [
Error(
"'ordering' refers to the nonexistent field, related field "
"or lookup 'missing_related__id'.",
obj=Model,
id='models.E015',
)
])
def test_ordering_pointing_to_missing_related_model_field(self):
class Parent(models.Model):
pass
class Child(models.Model):
parent = models.ForeignKey(Parent, models.CASCADE)
class Meta:
ordering = ('parent__missing_field',)
self.assertEqual(Child.check(), [
Error(
"'ordering' refers to the nonexistent field, related field "
"or lookup 'parent__missing_field'.",
obj=Child,
id='models.E015',
)
])
def test_ordering_pointing_to_non_related_field(self):
class Child(models.Model):
parent = models.IntegerField()
class Meta:
ordering = ('parent__missing_field',)
self.assertEqual(Child.check(), [
Error(
"'ordering' refers to the nonexistent field, related field "
"or lookup 'parent__missing_field'.",
obj=Child,
id='models.E015',
)
])
def test_ordering_pointing_to_two_related_model_field(self):
class Parent2(models.Model):
pass
class Parent1(models.Model):
parent2 = models.ForeignKey(Parent2, models.CASCADE)
class Child(models.Model):
parent1 = models.ForeignKey(Parent1, models.CASCADE)
class Meta:
ordering = ('parent1__parent2__missing_field',)
self.assertEqual(Child.check(), [
Error(
"'ordering' refers to the nonexistent field, related field "
"or lookup 'parent1__parent2__missing_field'.",
obj=Child,
id='models.E015',
)
])
def test_ordering_allows_registered_lookups(self):
class Model(models.Model):
test = models.CharField(max_length=100)
class Meta:
ordering = ('test__lower',)
with register_lookup(models.CharField, Lower):
self.assertEqual(Model.check(), [])
def test_ordering_pointing_to_foreignkey_field(self): def test_ordering_pointing_to_foreignkey_field(self):
class Parent(models.Model): class Parent(models.Model):
pass pass

View File

@ -19,6 +19,15 @@ except ImportError:
pass pass
class TestModelMetaOrdering(PostgreSQLTestCase):
def test_ordering_by_json_field_value(self):
class TestJSONModel(JSONModel):
class Meta:
ordering = ['field__value']
self.assertEqual(TestJSONModel.check(), [])
class TestSaveLoad(PostgreSQLTestCase): class TestSaveLoad(PostgreSQLTestCase):
def test_null(self): def test_null(self):
instance = JSONModel() instance = JSONModel()