From a5c77417a651c93036cf963e6da518653115be7e Mon Sep 17 00:00:00 2001 From: Rigel Di Scala Date: Tue, 14 Oct 2014 14:10:27 +0100 Subject: [PATCH] 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". --- django/core/checks/model_checks.py | 33 +++++++++++++----- docs/ref/checks.txt | 2 +- docs/releases/1.7.1.txt | 3 ++ tests/check_framework/tests.py | 55 ++++++++++++++++++++++++++++++ 4 files changed, 83 insertions(+), 10 deletions(-) diff --git a/django/core/checks/model_checks.py b/django/core/checks/model_checks.py index 3f5a36b41b..3d0d56af58 100644 --- a/django/core/checks/model_checks.py +++ b/django/core/checks/model_checks.py @@ -1,28 +1,43 @@ # -*- coding: utf-8 -*- from __future__ import unicode_literals -from itertools import chain +import inspect import types from django.apps import apps - -from . import Error, Tags, register +from django.core.checks import Error, Tags, register @register(Tags.models) def check_all_models(app_configs=None, **kwargs): - errors = [model.check(**kwargs) - for model in apps.get_models() - if app_configs is None or model._meta.app_config in app_configs] - return list(chain(*errors)) + errors = [] + for model in apps.get_models(): + if app_configs is None or model._meta.app_config in app_configs: + 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) 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 - errors = [] + errors = [] for name in dir(models.signals): obj = getattr(models.signals, name) if isinstance(obj, models.signals.ModelSignal): diff --git a/docs/ref/checks.txt b/docs/ref/checks.txt index 841c8f4c48..fde5dfa210 100644 --- a/docs/ref/checks.txt +++ b/docs/ref/checks.txt @@ -64,6 +64,7 @@ Models * **models.E019**: Autogenerated column name too long for M2M field ````. Maximum length is ```` for database ````. +* **models.E020**: The ``.check()`` class method is currently overridden. Fields ~~~~~~ @@ -94,7 +95,6 @@ Fields are mutually exclusive. Only one of these options may be present. * **fields.W161**: Fixed default value provided. - File Fields ~~~~~~~~~~~ diff --git a/docs/releases/1.7.1.txt b/docs/releases/1.7.1.txt index 6c06857396..435c496230 100644 --- a/docs/releases/1.7.1.txt +++ b/docs/releases/1.7.1.txt @@ -120,3 +120,6 @@ Bugfixes * Fixed a crash while parsing cookies containing invalid content (:ticket:`23638`). + +* The system check framework now raises error **models.E020** when the + class method ``Model.check()`` is unreachable (:ticket:`23615`). diff --git a/tests/check_framework/tests.py b/tests/check_framework/tests.py index c1059de604..d603cf926e 100644 --- a/tests/check_framework/tests.py +++ b/tests/check_framework/tests.py @@ -8,11 +8,13 @@ from django.apps import apps from django.conf import settings from django.core import checks 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.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.management.base import CommandError from django.core.management import call_command +from django.db import models from django.db.models.fields import NOT_PROVIDED from django.test import TestCase 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(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)