Fixed #23615 -- Validate that a Model instance's "check" attribute is a method.
The "check" name is a reserved word used by Django's check framework, and cannot be redefined as something else other than a method, or the check framework will raise an error. This change amends the django.core.checks.model_check.check_all_models() function, so that it verifies that a model instance's attribute "check" is actually a method. This new check is assigned the id "models.E020".
This commit is contained in:
parent
157f9cf240
commit
a5c77417a6
|
@ -1,28 +1,43 @@
|
||||||
# -*- coding: utf-8 -*-
|
# -*- coding: utf-8 -*-
|
||||||
from __future__ import unicode_literals
|
from __future__ import unicode_literals
|
||||||
|
|
||||||
from itertools import chain
|
import inspect
|
||||||
import types
|
import types
|
||||||
|
|
||||||
from django.apps import apps
|
from django.apps import apps
|
||||||
|
from django.core.checks import Error, Tags, register
|
||||||
from . import Error, Tags, register
|
|
||||||
|
|
||||||
|
|
||||||
@register(Tags.models)
|
@register(Tags.models)
|
||||||
def check_all_models(app_configs=None, **kwargs):
|
def check_all_models(app_configs=None, **kwargs):
|
||||||
errors = [model.check(**kwargs)
|
errors = []
|
||||||
for model in apps.get_models()
|
for model in apps.get_models():
|
||||||
if app_configs is None or model._meta.app_config in app_configs]
|
if app_configs is None or model._meta.app_config in app_configs:
|
||||||
return list(chain(*errors))
|
if not inspect.ismethod(model.check):
|
||||||
|
errors.append(
|
||||||
|
Error(
|
||||||
|
"The '%s.check()' class method is "
|
||||||
|
"currently overridden by %r." % (
|
||||||
|
model.__name__, model.check),
|
||||||
|
hint=None,
|
||||||
|
obj=model,
|
||||||
|
id='models.E020'
|
||||||
|
)
|
||||||
|
)
|
||||||
|
else:
|
||||||
|
errors.extend(model.check(**kwargs))
|
||||||
|
return errors
|
||||||
|
|
||||||
|
|
||||||
@register(Tags.models, Tags.signals)
|
@register(Tags.models, Tags.signals)
|
||||||
def check_model_signals(app_configs=None, **kwargs):
|
def check_model_signals(app_configs=None, **kwargs):
|
||||||
"""Ensure lazily referenced model signals senders are installed."""
|
"""
|
||||||
|
Ensure lazily referenced model signals senders are installed.
|
||||||
|
"""
|
||||||
|
# Avoid circular import
|
||||||
from django.db import models
|
from django.db import models
|
||||||
errors = []
|
|
||||||
|
|
||||||
|
errors = []
|
||||||
for name in dir(models.signals):
|
for name in dir(models.signals):
|
||||||
obj = getattr(models.signals, name)
|
obj = getattr(models.signals, name)
|
||||||
if isinstance(obj, models.signals.ModelSignal):
|
if isinstance(obj, models.signals.ModelSignal):
|
||||||
|
|
|
@ -64,6 +64,7 @@ Models
|
||||||
* **models.E019**: Autogenerated column name too long for M2M field
|
* **models.E019**: Autogenerated column name too long for M2M field
|
||||||
``<M2M field>``. Maximum length is ``<maximum length>`` for database
|
``<M2M field>``. Maximum length is ``<maximum length>`` for database
|
||||||
``<alias>``.
|
``<alias>``.
|
||||||
|
* **models.E020**: The ``<model>.check()`` class method is currently overridden.
|
||||||
|
|
||||||
Fields
|
Fields
|
||||||
~~~~~~
|
~~~~~~
|
||||||
|
@ -94,7 +95,6 @@ Fields
|
||||||
are mutually exclusive. Only one of these options may be present.
|
are mutually exclusive. Only one of these options may be present.
|
||||||
* **fields.W161**: Fixed default value provided.
|
* **fields.W161**: Fixed default value provided.
|
||||||
|
|
||||||
|
|
||||||
File Fields
|
File Fields
|
||||||
~~~~~~~~~~~
|
~~~~~~~~~~~
|
||||||
|
|
||||||
|
|
|
@ -120,3 +120,6 @@ Bugfixes
|
||||||
|
|
||||||
* Fixed a crash while parsing cookies containing invalid content
|
* Fixed a crash while parsing cookies containing invalid content
|
||||||
(:ticket:`23638`).
|
(:ticket:`23638`).
|
||||||
|
|
||||||
|
* The system check framework now raises error **models.E020** when the
|
||||||
|
class method ``Model.check()`` is unreachable (:ticket:`23615`).
|
||||||
|
|
|
@ -8,11 +8,13 @@ from django.apps import apps
|
||||||
from django.conf import settings
|
from django.conf import settings
|
||||||
from django.core import checks
|
from django.core import checks
|
||||||
from django.core.checks import Error, Warning
|
from django.core.checks import Error, Warning
|
||||||
|
from django.core.checks.model_checks import check_all_models
|
||||||
from django.core.checks.registry import CheckRegistry
|
from django.core.checks.registry import CheckRegistry
|
||||||
from django.core.checks.compatibility.django_1_6_0 import check_1_6_compatibility
|
from django.core.checks.compatibility.django_1_6_0 import check_1_6_compatibility
|
||||||
from django.core.checks.compatibility.django_1_7_0 import check_1_7_compatibility
|
from django.core.checks.compatibility.django_1_7_0 import check_1_7_compatibility
|
||||||
from django.core.management.base import CommandError
|
from django.core.management.base import CommandError
|
||||||
from django.core.management import call_command
|
from django.core.management import call_command
|
||||||
|
from django.db import models
|
||||||
from django.db.models.fields import NOT_PROVIDED
|
from django.db.models.fields import NOT_PROVIDED
|
||||||
from django.test import TestCase
|
from django.test import TestCase
|
||||||
from django.test.utils import override_settings, override_system_checks
|
from django.test.utils import override_settings, override_system_checks
|
||||||
|
@ -327,3 +329,56 @@ class SilencingCheckTests(TestCase):
|
||||||
|
|
||||||
self.assertEqual(out.getvalue(), 'System check identified no issues (1 silenced).\n')
|
self.assertEqual(out.getvalue(), 'System check identified no issues (1 silenced).\n')
|
||||||
self.assertEqual(err.getvalue(), '')
|
self.assertEqual(err.getvalue(), '')
|
||||||
|
|
||||||
|
|
||||||
|
class CheckFrameworkReservedNamesTests(TestCase):
|
||||||
|
|
||||||
|
def setUp(self):
|
||||||
|
self.current_models = apps.all_models[__package__]
|
||||||
|
self.saved_models = set(self.current_models)
|
||||||
|
|
||||||
|
def tearDown(self):
|
||||||
|
for model in (set(self.current_models) - self.saved_models):
|
||||||
|
del self.current_models[model]
|
||||||
|
apps.clear_cache()
|
||||||
|
|
||||||
|
@override_settings(SILENCED_SYSTEM_CHECKS=['models.E020'])
|
||||||
|
def test_model_check_method_not_shadowed(self):
|
||||||
|
class ModelWithAttributeCalledCheck(models.Model):
|
||||||
|
check = 42
|
||||||
|
|
||||||
|
class ModelWithFieldCalledCheck(models.Model):
|
||||||
|
check = models.IntegerField()
|
||||||
|
|
||||||
|
class ModelWithRelatedManagerCalledCheck(models.Model):
|
||||||
|
pass
|
||||||
|
|
||||||
|
class ModelWithDescriptorCalledCheck(models.Model):
|
||||||
|
check = models.ForeignKey(ModelWithRelatedManagerCalledCheck)
|
||||||
|
article = models.ForeignKey(ModelWithRelatedManagerCalledCheck, related_name='check')
|
||||||
|
|
||||||
|
expected = [
|
||||||
|
Error(
|
||||||
|
"The 'ModelWithAttributeCalledCheck.check()' class method is "
|
||||||
|
"currently overridden by 42.",
|
||||||
|
hint=None,
|
||||||
|
obj=ModelWithAttributeCalledCheck,
|
||||||
|
id='models.E020'
|
||||||
|
),
|
||||||
|
Error(
|
||||||
|
"The 'ModelWithRelatedManagerCalledCheck.check()' class method is "
|
||||||
|
"currently overridden by %r." % ModelWithRelatedManagerCalledCheck.check,
|
||||||
|
hint=None,
|
||||||
|
obj=ModelWithRelatedManagerCalledCheck,
|
||||||
|
id='models.E020'
|
||||||
|
),
|
||||||
|
Error(
|
||||||
|
"The 'ModelWithDescriptorCalledCheck.check()' class method is "
|
||||||
|
"currently overridden by %r." % ModelWithDescriptorCalledCheck.check,
|
||||||
|
hint=None,
|
||||||
|
obj=ModelWithDescriptorCalledCheck,
|
||||||
|
id='models.E020'
|
||||||
|
),
|
||||||
|
]
|
||||||
|
|
||||||
|
self.assertEqual(check_all_models(), expected)
|
||||||
|
|
Loading…
Reference in New Issue