8000 [1.7.x] Fixed #23615 -- Validate that a Model instance's "check" attr… · alex-python/django@e8262b5 · GitHub
[go: up one dir, main page]
Skip to content

Commit e8262b5

Browse files
Rigel Di Scalaloic
Rigel Di Scala
authored and
loic
committed
[1.7.x] 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". Conflicts: docs/ref/checks.txt Backport of a5c7741 from master
1 parent b729ef0 commit e8262b5

File tree

4 files changed

+83
-9
lines changed

4 files changed

+83
-9
lines changed

‎django/core/checks/model_checks.py

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,43 @@
11
# -*- coding: utf-8 -*-
22
from __future__ import unicode_literals
33

4-
from itertools import chain
4+
import inspect
55
import types
66

77
from django.apps import apps
8-
9-
from . import Error, Tags, register
8+
from django.core.checks import Error, Tags, register
109

1110

1211
@register(Tags.models)
1312
def check_all_models(app_configs=None, **kwargs):
14-
errors = [model.check(**kwargs)
15-
for model in apps.get_models()
16-
if app_configs is None or model._meta.app_config in app_configs]
17-
return list(chain(*errors))
13+
errors = []
14+
for model in apps.get_models():
15+
if app_configs is None or model._meta.app_config in app_configs:
16+
if not inspect.ismethod(model.check):
17+
errors.append(
18+
Error(
19+
"The '%s.check()' class method is "
20+
"currently overridden by %r." % (
21+
model.__name__, model.check),
22+
hint=None,
23+
obj=model,
24+
id='models.E020'
25+
)
26+
)
27+
else:
28+
errors.extend(model.check(**kwargs))
29+
return errors
1830

1931

2032
@register(Tags.models, Tags.signals)
2133
def check_model_signals(app_configs=None, **kwargs):
22-
"""Ensure lazily referenced model signals senders are installed."""
34+
"""
35+
Ensure lazily referenced model signals senders are installed.
36+
"""
37+
# Avoid circular import
2338
from django.db import models
24-
errors = []
2539

40+
errors = []
2641
for name in dir(models.signals):
2742
obj = getattr(models.signals, name)
2843
if isinstance(obj, models.signals.ModelSignal):

‎docs/ref/checks.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ Models
5656
* **models.E015**: ``ordering`` refers to the non-existent field
5757
``<field name>``.
5858
* **models.E017**: Proxy model ``<model>`` contains model fields.
59+
* **models.E020**: The ``<model>.check()`` class method is currently overridden.
5960

6061
Fields
6162
~~~~~~

‎docs/releases/1.7.1.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,3 +119,6 @@ Bugfixes
119119

120120
* Fixed a crash while parsing cookies containing invalid content
121121
(:ticket:`23638`).
122+
123+
* The system check framework now raises error **models.E020** when the
124+
class method ``Model.check()`` is unreachable (:ticket:`23615`).

‎tests/check_framework/tests.py

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,13 @@
88
from django.conf import settings
99
from django.core import checks
1010
from django.core.checks import Error, Warning
11+
from django.core.checks.model_checks import check_all_models
1112
from django.core.checks.registry import CheckRegistry
1213
from django.core.checks.compatibility.django_1_6_0 import check_1_6_compatibility
1314
from django.core.checks.compatibility.django_1_7_0 import check_1_7_compatibility
1415
from django.core.management.base import CommandError
1516
from django.core.management import call_command
17+
from django.db import models
1618
from django.db.models.fields import NOT_PROVIDED
1719
from django.test import TestCase
1820
from django.test.utils import override_settings, override_system_checks
@@ -300,3 +302,56 @@ def test_silenced_warning(self):
300302

301303
self.assertEqual(out.getvalue(), 'System check identified no issues (1 silenced).\n')
302304
self.assertEqual(err.getvalue(), '')
305+
306+
307+
class CheckFrameworkReservedNamesTests(TestCase):
308+
309+
def setUp(self):
310+
self.current_models = apps.all_models[__package__]
311+
self.saved_models = set(self.current_models)
312+
313+
def tearDown(self):
314+
for model in (set(self.current_models) - self.saved_models):
315+
del self.current_models[model]
316+
apps.clear_cache()
317+
318+
@override_settings(SILENCED_SYSTEM_CHECKS=['models.E020'])
319+
def test_model_check_method_not_shadowed(self):
320+
class ModelWithAttributeCalledCheck(models.Model):
321+
check = 42
322+
323+
class ModelWithFieldCalledCheck(models.Model):
324+
check = models.IntegerField()
325+
326+
class ModelWithRelatedManagerCalledCheck(models.Model):
327+
pass
328+
329+
class ModelWithDescriptorCalledCheck(models.Model):
330+
check = models.ForeignKey(ModelWithRelatedManagerCalledCheck)
331+
article = models.ForeignKey(ModelWithRelatedManagerCalledCheck, related_name='check')
332+
333+
expected = [
334+
Error(
335+
"The 'ModelWithAttributeCalledCheck.check()' class method is "
336+
"currently overridden by 42.",
337+
hint=None,
338+
obj=ModelWithAttributeCalledCheck,
339+
id='models.E020'
340+
),
341+
Error(
342+
"The 'ModelWithRelatedManagerCalledCheck.check()' class method is "
343+
"currently overridden by %r." % ModelWithRelatedManagerCalledCheck.check,
344+
hint=None,
345+
obj=ModelWithRelatedManagerCalledCheck,
346+
id='models.E020'
347+
),
348+
Error(
349+
"The 'ModelWithDescriptorCalledCheck.check()' class method is "
350+
"currently overridden by %r." % ModelWithDescriptorCalledCheck.check,
351+
hint=None,
352+
obj=ModelWithDescriptorCalledCheck,
353+
id='models.E020'
354+
),
355+
]
356+
357+
self.assertEqual(check_all_models(), expected)

0 commit comments

Comments
 (0)
0