From d818e0c9b2b88276cc499974f9eee893170bf0a8 Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Mon, 20 Jan 2014 10:45:21 +0800 Subject: [PATCH] Fixed #16905 -- Added extensible checks (nee validation) framework MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is the result of Christopher Medrela's 2013 Summer of Code project. Thanks also to Preston Holmes, Tim Graham, Anssi Kääriäinen, Florian Apolloner, and Alex Gaynor for review notes along the way. Also: Fixes #8579, fixes #3055, fixes #19844. --- django/conf/__init__.py | 12 +- django/conf/global_settings.py | 10 + django/contrib/admin/__init__.py | 6 +- django/contrib/admin/checks.py | 932 +++++++++++++++ django/contrib/admin/options.py | 65 +- django/contrib/admin/sites.py | 2 +- django/contrib/auth/__init__.py | 6 + django/contrib/auth/checks.py | 69 ++ .../management/commands/changepassword.py | 2 +- django/contrib/auth/tests/test_management.py | 83 +- django/contrib/contenttypes/__init__.py | 8 + django/contrib/contenttypes/checks.py | 18 + django/contrib/contenttypes/generic.py | 167 ++- .../gis/management/commands/ogrinspect.py | 2 +- django/contrib/sites/managers.py | 76 +- .../management/commands/collectstatic.py | 2 +- django/core/checks/__init__.py | 18 + django/core/checks/compatibility/base.py | 39 - .../core/checks/compatibility/django_1_6_0.py | 137 ++- django/core/checks/messages.py | 84 ++ django/core/checks/model_checks.py | 49 + django/core/checks/registry.py | 63 + django/core/management/base.py | 126 +- django/core/management/commands/check.py | 36 +- .../management/commands/compilemessages.py | 2 +- .../management/commands/createcachetable.py | 2 +- django/core/management/commands/dbshell.py | 2 +- .../core/management/commands/diffsettings.py | 2 +- django/core/management/commands/inspectdb.py | 2 +- .../core/management/commands/makemessages.py | 2 +- django/core/management/commands/runserver.py | 4 +- django/core/management/commands/shell.py | 2 +- django/core/management/commands/test.py | 2 +- django/core/management/commands/testserver.py | 2 +- django/core/management/commands/validate.py | 17 +- django/core/management/templates.py | 2 +- django/core/management/validation.py | 411 ------- django/db/backends/__init__.py | 28 +- django/db/backends/mysql/validation.py | 33 +- django/db/models/base.py | 303 +++++ django/db/models/fields/__init__.py | 298 ++++- django/db/models/fields/files.py | 74 +- django/db/models/fields/related.py | 367 +++++- django/db/models/manager.py | 19 + django/db/models/options.py | 19 +- django/forms/models.py | 16 +- django/test/__init__.py | 3 +- django/test/testcases.py | 6 +- django/test/utils.py | 20 + django/utils/termcolors.py | 3 + docs/howto/custom-management-commands.txt | 32 +- docs/index.txt | 1 + docs/internals/deprecation.txt | 10 + docs/intro/tutorial01.txt | 6 +- docs/ref/checks.txt | 208 ++++ docs/ref/django-admin.txt | 41 +- docs/ref/index.txt | 1 + docs/ref/settings.txt | 17 + docs/releases/1.7.txt | 42 +- docs/topics/db/models.txt | 4 +- tests/{check => admin_checks}/__init__.py | 0 tests/admin_checks/models.py | 57 + tests/admin_checks/tests.py | 436 +++++++ .../app_raising_messages}/__init__.py | 0 .../app_raising_messages/models.py | 27 + .../app_raising_warning/__init__.py | 0 .../app_raising_warning/models.py | 16 + .../management/commands/app_command.py | 2 +- .../management/commands/base_command.py | 2 +- .../management/commands/label_command.py | 2 +- .../management/commands/noargs_command.py | 2 +- .../management/commands/validation_command.py | 11 + tests/admin_scripts/tests.py | 193 ++- tests/admin_validation/tests.py | 4 +- tests/admin_views/tests.py | 31 +- tests/check/tests.py | 128 -- tests/check_framework/__init__.py | 0 tests/{check => check_framework}/models.py | 8 + tests/check_framework/tests.py | 219 ++++ tests/contenttypes_tests/tests.py | 303 ++++- tests/fixtures_model_package/tests.py | 3 + tests/inline_formsets/tests.py | 9 +- tests/invalid_models_tests/base.py | 18 + .../invalid_models/models.py | 535 --------- .../test_backend_specific.py | 68 ++ tests/invalid_models_tests/test_models.py | 334 ++++++ .../test_ordinary_fields.py | 417 +++++++ .../test_relative_fields.py | 1037 +++++++++++++++++ tests/invalid_models_tests/tests.py | 46 - tests/logging_tests/tests.py | 2 +- tests/migrate_signals/tests.py | 6 +- tests/migrations/test_commands.py | 17 +- tests/model_fields/tests.py | 5 + tests/model_validation/tests.py | 36 +- tests/modeladmin/tests.py | 950 +++++++-------- tests/proxy_model_inheritance/tests.py | 4 + tests/sites_framework/models.py | 11 - tests/sites_framework/tests.py | 50 +- tests/test_runner/tests.py | 5 +- .../management/commands/dance.py | 2 +- tests/validation/test_error_messages.py | 5 - 101 files changed, 7058 insertions(+), 1958 deletions(-) create mode 100644 django/contrib/admin/checks.py create mode 100644 django/contrib/auth/checks.py create mode 100644 django/contrib/contenttypes/checks.py delete mode 100644 django/core/checks/compatibility/base.py create mode 100644 django/core/checks/messages.py create mode 100644 django/core/checks/model_checks.py create mode 100644 django/core/checks/registry.py delete mode 100644 django/core/management/validation.py create mode 100644 docs/ref/checks.txt rename tests/{check => admin_checks}/__init__.py (100%) create mode 100644 tests/admin_checks/models.py create mode 100644 tests/admin_checks/tests.py rename tests/{invalid_models_tests/invalid_models => admin_scripts/app_raising_messages}/__init__.py (100%) create mode 100644 tests/admin_scripts/app_raising_messages/models.py create mode 100644 tests/admin_scripts/app_raising_warning/__init__.py create mode 100644 tests/admin_scripts/app_raising_warning/models.py create mode 100644 tests/admin_scripts/management/commands/validation_command.py delete mode 100644 tests/check/tests.py create mode 100644 tests/check_framework/__init__.py rename tests/{check => check_framework}/models.py (62%) create mode 100644 tests/check_framework/tests.py create mode 100644 tests/invalid_models_tests/base.py delete mode 100644 tests/invalid_models_tests/invalid_models/models.py create mode 100644 tests/invalid_models_tests/test_backend_specific.py create mode 100644 tests/invalid_models_tests/test_models.py create mode 100644 tests/invalid_models_tests/test_ordinary_fields.py create mode 100644 tests/invalid_models_tests/test_relative_fields.py delete mode 100644 tests/invalid_models_tests/tests.py diff --git a/django/conf/__init__.py b/django/conf/__init__.py index e10d4b1afb..aad67bd7f5 100644 --- a/django/conf/__init__.py +++ b/django/conf/__init__.py @@ -99,7 +99,7 @@ class Settings(BaseSettings): ) tuple_settings = ("INSTALLED_APPS", "TEMPLATE_DIRS") - + self._explicit_settings = set() for setting in dir(mod): if setting.isupper(): setting_value = getattr(mod, setting) @@ -110,6 +110,7 @@ class Settings(BaseSettings): "Please fix your settings." % setting) setattr(self, setting, setting_value) + self._explicit_settings.add(setting) if not self.SECRET_KEY: raise ImproperlyConfigured("The SECRET_KEY setting must not be empty.") @@ -126,6 +127,9 @@ class Settings(BaseSettings): os.environ['TZ'] = self.TIME_ZONE time.tzset() + def is_overridden(self, setting): + return setting in self._explicit_settings + class UserSettingsHolder(BaseSettings): """ @@ -159,4 +163,10 @@ class UserSettingsHolder(BaseSettings): def __dir__(self): return list(self.__dict__) + dir(self.default_settings) + def is_overridden(self, setting): + if setting in self._deleted: + return False + else: + return self.default_settings.is_overridden(setting) + settings = LazySettings() diff --git a/django/conf/global_settings.py b/django/conf/global_settings.py index d86a4272cd..745f55edd0 100644 --- a/django/conf/global_settings.py +++ b/django/conf/global_settings.py @@ -618,3 +618,13 @@ STATICFILES_FINDERS = ( # Migration module overrides for apps, by app label. MIGRATION_MODULES = {} + +################# +# SYSTEM CHECKS # +################# + +# List of all issues generated by system checks that should be silenced. Light +# issues like warnings, infos or debugs will not generate a message. Silencing +# serious issues like errors and criticals does not result in hiding the +# message, but Django will not stop you from e.g. running server. +SILENCED_SYSTEM_CHECKS = [] diff --git a/django/contrib/admin/__init__.py b/django/contrib/admin/__init__.py index d8fc12e0a5..530badf1df 100644 --- a/django/contrib/admin/__init__.py +++ b/django/contrib/admin/__init__.py @@ -1,13 +1,15 @@ # ACTION_CHECKBOX_NAME is unused, but should stay since its import from here # has been referenced in documentation. +from django.contrib.admin.checks import check_admin_app from django.contrib.admin.decorators import register from django.contrib.admin.helpers import ACTION_CHECKBOX_NAME from django.contrib.admin.options import ModelAdmin, HORIZONTAL, VERTICAL from django.contrib.admin.options import StackedInline, TabularInline -from django.contrib.admin.sites import AdminSite, site from django.contrib.admin.filters import (ListFilter, SimpleListFilter, FieldListFilter, BooleanFieldListFilter, RelatedFieldListFilter, ChoicesFieldListFilter, DateFieldListFilter, AllValuesFieldListFilter) +from django.contrib.admin.sites import AdminSite, site +from django.core import checks from django.utils.module_loading import autodiscover_modules __all__ = [ @@ -21,3 +23,5 @@ __all__ = [ def autodiscover(): autodiscover_modules('admin', register_to=site) + +checks.register('admin')(check_admin_app) diff --git a/django/contrib/admin/checks.py b/django/contrib/admin/checks.py new file mode 100644 index 0000000000..9a39d9a901 --- /dev/null +++ b/django/contrib/admin/checks.py @@ -0,0 +1,932 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from itertools import chain + +from django.contrib.admin.util import get_fields_from_path, NotRelationField +from django.core import checks +from django.db import models +from django.db.models.fields import FieldDoesNotExist +from django.forms.models import BaseModelForm, _get_foreign_key, BaseModelFormSet + + +def check_admin_app(**kwargs): + from django.contrib.admin.sites import site + + return list(chain.from_iterable( + model_admin.check(model, **kwargs) + for model, model_admin in site._registry.items() + )) + + +class BaseModelAdminChecks(object): + + def check(self, cls, model, **kwargs): + errors = [] + errors.extend(self._check_raw_id_fields(cls, model)) + errors.extend(self._check_fields(cls, model)) + errors.extend(self._check_fieldsets(cls, model)) + errors.extend(self._check_exclude(cls, model)) + errors.extend(self._check_form(cls, model)) + errors.extend(self._check_filter_vertical(cls, model)) + errors.extend(self._check_filter_horizontal(cls, model)) + errors.extend(self._check_radio_fields(cls, model)) + errors.extend(self._check_prepopulated_fields(cls, model)) + errors.extend(self._check_view_on_site_url(cls, model)) + errors.extend(self._check_ordering(cls, model)) + errors.extend(self._check_readonly_fields(cls, model)) + return errors + + def _check_raw_id_fields(self, cls, model): + """ Check that `raw_id_fields` only contains field names that are listed + on the model. """ + + if not isinstance(cls.raw_id_fields, (list, tuple)): + return must_be('a list or tuple', option='raw_id_fields', obj=cls, id='admin.E001') + else: + return list(chain(*[ + self._check_raw_id_fields_item(cls, model, field_name, 'raw_id_fields[%d]' % index) + for index, field_name in enumerate(cls.raw_id_fields) + ])) + + def _check_raw_id_fields_item(self, cls, model, field_name, label): + """ Check an item of `raw_id_fields`, i.e. check that field named + `field_name` exists in model `model` and is a ForeignKey or a + ManyToManyField. """ + + try: + field = model._meta.get_field(field_name) + except models.FieldDoesNotExist: + return refer_to_missing_field(field=field_name, option=label, + model=model, obj=cls, id='admin.E002') + else: + if not isinstance(field, (models.ForeignKey, models.ManyToManyField)): + return must_be('a ForeignKey or ManyToManyField', + option=label, obj=cls, id='admin.E003') + else: + return [] + + def _check_fields(self, cls, model): + """ Check that `fields` only refer to existing fields, doesn't contain + duplicates. Check if at most one of `fields` and `fieldsets` is defined. + """ + + if cls.fields is None: + return [] + elif not isinstance(cls.fields, (list, tuple)): + return must_be('a list or tuple', option='fields', obj=cls, id='admin.E004') + elif cls.fieldsets: + return [ + checks.Error( + 'Both "fieldsets" and "fields" are specified.', + hint=None, + obj=cls, + id='admin.E005', + ) + ] + elif len(cls.fields) != len(set(cls.fields)): + return [ + checks.Error( + 'There are duplicate field(s) in "fields".', + hint=None, + obj=cls, + id='admin.E006', + ) + ] + else: + return list(chain(*[ + self._check_field_spec(cls, model, field_name, 'fields') + for field_name in cls.fields + ])) + + def _check_fieldsets(self, cls, model): + """ Check that fieldsets is properly formatted and doesn't contain + duplicates. """ + + if cls.fieldsets is None: + return [] + elif not isinstance(cls.fieldsets, (list, tuple)): + return must_be('a list or tuple', option='fieldsets', obj=cls, id='admin.E007') + else: + return list(chain(*[ + self._check_fieldsets_item(cls, model, fieldset, 'fieldsets[%d]' % index) + for index, fieldset in enumerate(cls.fieldsets) + ])) + + def _check_fieldsets_item(self, cls, model, fieldset, label): + """ Check an item of `fieldsets`, i.e. check that this is a pair of a + set name and a dictionary containing "fields" key. """ + + if not isinstance(fieldset, (list, tuple)): + return must_be('a list or tuple', option=label, obj=cls, id='admin.E008') + elif len(fieldset) != 2: + return must_be('a pair', option=label, obj=cls, id='admin.E009') + elif not isinstance(fieldset[1], dict): + return must_be('a dictionary', option='%s[1]' % label, obj=cls, id='admin.E010') + elif 'fields' not in fieldset[1]: + return [ + checks.Error( + '"%s[1]" must contain "fields" key.' % label, + hint=None, + obj=cls, + id='admin.E011', + ) + ] + elif len(fieldset[1]['fields']) != len(set(fieldset[1]['fields'])): + return [ + checks.Error( + 'There are duplicate field(s) in "%s[1]".' % label, + hint=None, + obj=cls, + id='admin.E012', + ) + ] + else: + return list(chain(*[ + self._check_field_spec(cls, model, fields, '%s[1][\'fields\']' % label) + for fields in fieldset[1]['fields'] + ])) + + def _check_field_spec(self, cls, model, fields, label): + """ `fields` should be an item of `fields` or an item of + fieldset[1]['fields'] for any `fieldset` in `fieldsets`. It should be a + field name or a tuple of field names. """ + + if isinstance(fields, tuple): + return list(chain(*[ + self._check_field_spec_item(cls, model, field_name, "%s[%d]" % (label, index)) + for index, field_name in enumerate(fields) + ])) + else: + return self._check_field_spec_item(cls, model, fields, label) + + def _check_field_spec_item(self, cls, model, field_name, label): + if field_name in cls.readonly_fields: + # Stuff can be put in fields that isn't actually a model field if + # it's in readonly_fields, readonly_fields will handle the + # validation of such things. + return [] + else: + try: + field = model._meta.get_field(field_name) + except models.FieldDoesNotExist: + # If we can't find a field on the model that matches, it could + # be an extra field on the form. + return [] + else: + if (isinstance(field, models.ManyToManyField) and + not field.rel.through._meta.auto_created): + return [ + checks.Error( + '"%s" cannot include the ManyToManyField "%s", ' + 'because "%s" manually specifies relationship model.' + % (label, field_name, field_name), + hint=None, + obj=cls, + id='admin.E013', + ) + ] + else: + return [] + + def _check_exclude(self, cls, model): + """ Check that exclude is a sequence without duplicates. """ + + if cls.exclude is None: # default value is None + return [] + elif not isinstance(cls.exclude, (list, tuple)): + return must_be('a list or tuple', option='exclude', obj=cls, id='admin.E014') + elif len(cls.exclude) > len(set(cls.exclude)): + return [ + checks.Error( + '"exclude" contains duplicate field(s).', + hint=None, + obj=cls, + id='admin.E015', + ) + ] + else: + return [] + + def _check_form(self, cls, model): + """ Check that form subclasses BaseModelForm. """ + + if hasattr(cls, 'form') and not issubclass(cls.form, BaseModelForm): + return must_inherit_from(parent='BaseModelForm', option='form', + obj=cls, id='admin.E016') + else: + return [] + + def _check_filter_vertical(self, cls, model): + """ Check that filter_vertical is a sequence of field names. """ + + if not hasattr(cls, 'filter_vertical'): + return [] + elif not isinstance(cls.filter_vertical, (list, tuple)): + return must_be('a list or tuple', option='filter_vertical', obj=cls, id='admin.E017') + else: + return list(chain(*[ + self._check_filter_item(cls, model, field_name, "filter_vertical[%d]" % index) + for index, field_name in enumerate(cls.filter_vertical) + ])) + + def _check_filter_horizontal(self, cls, model): + """ Check that filter_horizontal is a sequence of field names. """ + + if not hasattr(cls, 'filter_horizontal'): + return [] + elif not isinstance(cls.filter_horizontal, (list, tuple)): + return must_be('a list or tuple', option='filter_horizontal', obj=cls, id='admin.E018') + else: + return list(chain(*[ + self._check_filter_item(cls, model, field_name, "filter_horizontal[%d]" % index) + for index, field_name in enumerate(cls.filter_horizontal) + ])) + + def _check_filter_item(self, cls, model, field_name, label): + """ Check one item of `filter_vertical` or `filter_horizontal`, i.e. + check that given field exists and is a ManyToManyField. """ + + try: + field = model._meta.get_field(field_name) + except models.FieldDoesNotExist: + return refer_to_missing_field(field=field_name, option=label, + model=model, obj=cls, id='admin.E019') + else: + if not isinstance(field, models.ManyToManyField): + return must_be('a ManyToManyField', option=label, obj=cls, id='admin.E020') + else: + return [] + + def _check_radio_fields(self, cls, model): + """ Check that `radio_fields` is a dictionary. """ + + if not hasattr(cls, 'radio_fields'): + return [] + elif not isinstance(cls.radio_fields, dict): + return must_be('a dictionary', option='radio_fields', obj=cls, id='admin.E021') + else: + return list(chain(*[ + self._check_radio_fields_key(cls, model, field_name, 'radio_fields') + + self._check_radio_fields_value(cls, model, val, 'radio_fields[\'%s\']' % field_name) + for field_name, val in cls.radio_fields.items() + ])) + + def _check_radio_fields_key(self, cls, model, field_name, label): + """ Check that a key of `radio_fields` dictionary is name of existing + field and that the field is a ForeignKey or has `choices` defined. """ + + try: + field = model._meta.get_field(field_name) + except models.FieldDoesNotExist: + return refer_to_missing_field(field=field_name, option=label, + model=model, obj=cls, id='admin.E022') + else: + if not (isinstance(field, models.ForeignKey) or field.choices): + return [ + checks.Error( + '"%s" refers to "%s", which is neither an instance of ForeignKey nor does have choices set.' % ( + label, field_name + ), + hint=None, + obj=cls, + id='admin.E023', + ) + ] + else: + return [] + + def _check_radio_fields_value(self, cls, model, val, label): + """ Check type of a value of `radio_fields` dictionary. """ + + from django.contrib.admin.options import HORIZONTAL, VERTICAL + + if val not in (HORIZONTAL, VERTICAL): + return [ + checks.Error( + '"%s" is neither admin.HORIZONTAL nor admin.VERTICAL.' % label, + hint=None, + obj=cls, + id='admin.E024', + ) + ] + else: + return [] + + def _check_view_on_site_url(self, cls, model): + if hasattr(cls, 'view_on_site'): + if not callable(cls.view_on_site) and not isinstance(cls.view_on_site, bool): + return [ + checks.Error( + '"view_on_site" is not a callable or a boolean value.', + hint=None, + obj=cls, + id='admin.E025', + ) + ] + else: + return [] + else: + return [] + + def _check_prepopulated_fields(self, cls, model): + """ Check that `prepopulated_fields` is a dictionary containing allowed + field types. """ + + if not hasattr(cls, 'prepopulated_fields'): + return [] + elif not isinstance(cls.prepopulated_fields, dict): + return must_be('a dictionary', option='prepopulated_fields', obj=cls, id='admin.E026') + else: + return list(chain(*[ + self._check_prepopulated_fields_key(cls, model, field_name, 'prepopulated_fields') + + self._check_prepopulated_fields_value(cls, model, val, 'prepopulated_fields[\'%s\']' % field_name) + for field_name, val in cls.prepopulated_fields.items() + ])) + + def _check_prepopulated_fields_key(self, cls, model, field_name, label): + """ Check a key of `prepopulated_fields` dictionary, i.e. check that it + is a name of existing field and the field is one of the allowed types. + """ + + forbidden_field_types = ( + models.DateTimeField, + models.ForeignKey, + models.ManyToManyField + ) + + try: + field = model._meta.get_field(field_name) + except models.FieldDoesNotExist: + return refer_to_missing_field(field=field_name, option=label, + model=model, obj=cls, id='admin.E027') + else: + if isinstance(field, forbidden_field_types): + return [ + checks.Error( + '"%s" refers to "%s", which must not be a DateTimeField, ' + 'ForeignKey or ManyToManyField.' % ( + label, field_name + ), + hint=None, + obj=cls, + id='admin.E028', + ) + ] + else: + return [] + + def _check_prepopulated_fields_value(self, cls, model, val, label): + """ Check a value of `prepopulated_fields` dictionary, i.e. it's an + iterable of existing fields. """ + + if not isinstance(val, (list, tuple)): + return must_be('a list or tuple', option=label, obj=cls, id='admin.E029') + else: + return list(chain(*[ + self._check_prepopulated_fields_value_item(cls, model, subfield_name, "%s[%r]" % (label, index)) + for index, subfield_name in enumerate(val) + ])) + + def _check_prepopulated_fields_value_item(self, cls, model, field_name, label): + """ For `prepopulated_fields` equal to {"slug": ("title",)}, + `field_name` is "title". """ + + try: + model._meta.get_field(field_name) + except models.FieldDoesNotExist: + return refer_to_missing_field(field=field_name, option=label, + model=model, obj=cls, id='admin.E030') + else: + return [] + + def _check_ordering(self, cls, model): + """ Check that ordering refers to existing fields or is random. """ + + # ordering = None + if cls.ordering is None: # The default value is None + return [] + elif not isinstance(cls.ordering, (list, tuple)): + return must_be('a list or tuple', option='ordering', obj=cls, id='admin.E031') + else: + return list(chain(*[ + self._check_ordering_item(cls, model, field_name, 'ordering[%d]' % index) + for index, field_name in enumerate(cls.ordering) + ])) + + def _check_ordering_item(self, cls, model, field_name, label): + """ Check that `ordering` refers to existing fields. """ + + if field_name == '?' and len(cls.ordering) != 1: + return [ + checks.Error( + '"ordering" has the random ordering marker "?", ' + 'but contains other fields as well.', + hint='Either remove the "?", or remove the other fields.', + obj=cls, + id='admin.E032', + ) + ] + elif field_name == '?': + return [] + elif '__' in field_name: + # Skip ordering in the format field1__field2 (FIXME: checking + # this format would be nice, but it's a little fiddly). + return [] + else: + if field_name.startswith('-'): + field_name = field_name[1:] + + try: + model._meta.get_field(field_name) + except models.FieldDoesNotExist: + return refer_to_missing_field(field=field_name, option=label, + model=model, obj=cls, id='admin.E033') + else: + return [] + + def _check_readonly_fields(self, cls, model): + """ Check that readonly_fields refers to proper attribute or field. """ + + if cls.readonly_fields == (): + return [] + elif not isinstance(cls.readonly_fields, (list, tuple)): + return must_be('a list or tuple', option='readonly_fields', obj=cls, id='admin.E034') + else: + return list(chain(*[ + self._check_readonly_fields_item(cls, model, field_name, "readonly_fields[%d]" % index) + for index, field_name in enumerate(cls.readonly_fields) + ])) + + def _check_readonly_fields_item(self, cls, model, field_name, label): + if callable(field_name): + return [] + elif hasattr(cls, field_name): + return [] + elif hasattr(model, field_name): + return [] + else: + try: + model._meta.get_field(field_name) + except models.FieldDoesNotExist: + return [ + checks.Error( + '"%s" is neither a callable nor an attribute of "%s" nor found in the model %s.%s.' % ( + label, cls.__name__, model._meta.app_label, model._meta.object_name + ), + hint=None, + obj=cls, + id='admin.E035', + ) + ] + else: + return [] + + +class ModelAdminChecks(BaseModelAdminChecks): + + def check(self, cls, model, **kwargs): + errors = super(ModelAdminChecks, self).check(cls, model=model, **kwargs) + errors.extend(self._check_save_as(cls, model)) + errors.extend(self._check_save_on_top(cls, model)) + errors.extend(self._check_inlines(cls, model)) + errors.extend(self._check_list_display(cls, model)) + errors.extend(self._check_list_display_links(cls, model)) + errors.extend(self._check_list_filter(cls, model)) + errors.extend(self._check_list_select_related(cls, model)) + errors.extend(self._check_list_per_page(cls, model)) + errors.extend(self._check_list_max_show_all(cls, model)) + errors.extend(self._check_list_editable(cls, model)) + errors.extend(self._check_search_fields(cls, model)) + errors.extend(self._check_date_hierarchy(cls, model)) + return errors + + def _check_save_as(self, cls, model): + """ Check save_as is a boolean. """ + + if not isinstance(cls.save_as, bool): + return must_be('a boolean', option='save_as', + obj=cls, id='admin.E101') + else: + return [] + + def _check_save_on_top(self, cls, model): + """ Check save_on_top is a boolean. """ + + if not isinstance(cls.save_on_top, bool): + return must_be('a boolean', option='save_on_top', + obj=cls, id='admin.E102') + else: + return [] + + def _check_inlines(self, cls, model): + """ Check all inline model admin classes. """ + + if not isinstance(cls.inlines, (list, tuple)): + return must_be('a list or tuple', option='inlines', obj=cls, id='admin.E103') + else: + return list(chain(*[ + self._check_inlines_item(cls, model, item, "inlines[%d]" % index) + for index, item in enumerate(cls.inlines) + ])) + + def _check_inlines_item(self, cls, model, inline, label): + """ Check one inline model admin. """ + + from django.contrib.admin.options import BaseModelAdmin + + if not issubclass(inline, BaseModelAdmin): + return must_inherit_from(parent='BaseModelAdmin', option=label, + obj=cls, id='admin.E104') + elif not inline.model: + return [ + checks.Error( + '"model" is a required attribute of "%s".' % label, + hint=None, + obj=cls, + id='admin.E105', + ) + ] + elif not issubclass(inline.model, models.Model): + return must_be('a Model', option='%s.model' % label, + obj=cls, id='admin.E106') + else: + return inline.check(model) + + def _check_list_display(self, cls, model): + """ Check that list_display only contains fields or usable attributes. + """ + + if not isinstance(cls.list_display, (list, tuple)): + return must_be('a list or tuple', option='list_display', obj=cls, id='admin.E107') + else: + return list(chain(*[ + self._check_list_display_item(cls, model, item, "list_display[%d]" % index) + for index, item in enumerate(cls.list_display) + ])) + + def _check_list_display_item(self, cls, model, item, label): + if callable(item): + return [] + elif hasattr(cls, item): + return [] + elif hasattr(model, item): + # getattr(model, item) could be an X_RelatedObjectsDescriptor + try: + field = model._meta.get_field(item) + except models.FieldDoesNotExist: + try: + field = getattr(model, item) + except AttributeError: + field = None + + if field is None: + return [ + checks.Error( + '"%s" refers to "%s" that is neither a field, method nor a property of model %s.%s.' % ( + label, item, model._meta.app_label, model._meta.object_name + ), + hint=None, + obj=cls, + id='admin.E108', + ) + ] + elif isinstance(field, models.ManyToManyField): + return [ + checks.Error( + '"%s" must not be a ManyToManyField.' % label, + hint=None, + obj=cls, + id='admin.E109', + ) + ] + else: + return [] + else: + try: + model._meta.get_field(item) + except models.FieldDoesNotExist: + return [ + checks.Error( + '"%s" is neither a callable nor an attribute of "%s" nor found in model %s.%s.' % ( + label, cls.__name__, model._meta.app_label, model._meta.object_name + ), + hint=None, + obj=cls, + id='admin.E110', + ) + ] + else: + return [] + + def _check_list_display_links(self, cls, model): + """ Check that list_display_links is a unique subset of list_display. + """ + + if cls.list_display_links is None: + return [] + elif not isinstance(cls.list_display_links, (list, tuple)): + return must_be('a list or tuple or None', option='list_display_links', obj=cls, id='admin.E111') + else: + return list(chain(*[ + self._check_list_display_links_item(cls, model, field_name, "list_display_links[%d]" % index) + for index, field_name in enumerate(cls.list_display_links) + ])) + + def _check_list_display_links_item(self, cls, model, field_name, label): + if field_name not in cls.list_display: + return [ + checks.Error( + '"%s" refers to "%s", which is not defined in "list_display".' % ( + label, field_name + ), + hint=None, + obj=cls, + id='admin.E112', + ) + ] + else: + return [] + + def _check_list_filter(self, cls, model): + if not isinstance(cls.list_filter, (list, tuple)): + return must_be('a list or tuple', option='list_filter', obj=cls, id='admin.E113') + else: + return list(chain(*[ + self._check_list_filter_item(cls, model, item, "list_filter[%d]" % index) + for index, item in enumerate(cls.list_filter) + ])) + + def _check_list_filter_item(self, cls, model, item, label): + """ + Check one item of `list_filter`, i.e. check if it is one of three options: + 1. 'field' -- a basic field filter, possibly w/ relationships (e.g. + 'field__rel') + 2. ('field', SomeFieldListFilter) - a field-based list filter class + 3. SomeListFilter - a non-field list filter class + """ + + from django.contrib.admin import ListFilter, FieldListFilter + + if callable(item) and not isinstance(item, models.Field): + # If item is option 3, it should be a ListFilter... + if not issubclass(item, ListFilter): + return must_inherit_from(parent='ListFilter', option=label, + obj=cls, id='admin.E114') + # ... but not a FieldListFilter. + elif issubclass(item, FieldListFilter): + return [ + checks.Error( + '"%s" must not inherit from FieldListFilter.' % label, + hint=None, + obj=cls, + id='admin.E115', + ) + ] + else: + return [] + elif isinstance(item, (tuple, list)): + # item is option #2 + field, list_filter_class = item + if not issubclass(list_filter_class, FieldListFilter): + return must_inherit_from(parent='FieldListFilter', option='%s[1]' % label, + obj=cls, id='admin.E116') + else: + return [] + else: + # item is option #1 + field = item + + # Validate the field string + try: + get_fields_from_path(model, field) + except (NotRelationField, FieldDoesNotExist): + return [ + checks.Error( + '"%s" refers to "%s", which does not refer to a Field.' % (label, field), + hint=None, + obj=cls, + id='admin.E117', + ) + ] + else: + return [] + + def _check_list_select_related(self, cls, model): + """ Check that list_select_related is a boolean, a list or a tuple. """ + + if not isinstance(cls.list_select_related, (bool, list, tuple)): + return must_be('a boolean, tuple or list', option='list_select_related', + obj=cls, id='admin.E118') + else: + return [] + + def _check_list_per_page(self, cls, model): + """ Check that list_per_page is an integer. """ + + if not isinstance(cls.list_per_page, int): + return must_be('an integer', option='list_per_page', obj=cls, id='admin.E119') + else: + return [] + + def _check_list_max_show_all(self, cls, model): + """ Check that list_max_show_all is an integer. """ + + if not isinstance(cls.list_max_show_all, int): + return must_be('an integer', option='list_max_show_all', obj=cls, id='admin.E120') + else: + return [] + + def _check_list_editable(self, cls, model): + """ Check that list_editable is a sequence of editable fields from + list_display without first element. """ + + if not isinstance(cls.list_editable, (list, tuple)): + return must_be('a list or tuple', option='list_editable', obj=cls, id='admin.E121') + else: + return list(chain(*[ + self._check_list_editable_item(cls, model, item, "list_editable[%d]" % index) + for index, item in enumerate(cls.list_editable) + ])) + + def _check_list_editable_item(self, cls, model, field_name, label): + try: + field = model._meta.get_field_by_name(field_name)[0] + except models.FieldDoesNotExist: + return refer_to_missing_field(field=field_name, option=label, + model=model, obj=cls, id='admin.E122') + else: + if field_name not in cls.list_display: + return refer_to_missing_field(field=field_name, option=label, + model=model, obj=cls, id='admin.E123') + elif field_name in cls.list_display_links: + return [ + checks.Error( + '"%s" cannot be in both "list_editable" and "list_display_links".' % field_name, + hint=None, + obj=cls, + id='admin.E124', + ) + ] + elif not cls.list_display_links and cls.list_display[0] in cls.list_editable: + return [ + checks.Error( + '"%s" refers to the first field in list_display ("%s"), ' + 'which cannot be used unless list_display_links is set.' % ( + label, cls.list_display[0] + ), + hint=None, + obj=cls, + id='admin.E125', + ) + ] + elif not field.editable: + return [ + checks.Error( + '"%s" refers to field "%s", whih is not editable through the admin.' % ( + label, field_name + ), + hint=None, + obj=cls, + id='admin.E126', + ) + ] + + def _check_search_fields(self, cls, model): + """ Check search_fields is a sequence. """ + + if not isinstance(cls.search_fields, (list, tuple)): + return must_be('a list or tuple', option='search_fields', obj=cls, id='admin.E127') + else: + return [] + + def _check_date_hierarchy(self, cls, model): + """ Check that date_hierarchy refers to DateField or DateTimeField. """ + + if cls.date_hierarchy is None: + return [] + else: + try: + field = model._meta.get_field(cls.date_hierarchy) + except models.FieldDoesNotExist: + return refer_to_missing_field(option='date_hierarchy', + field=cls.date_hierarchy, + model=model, obj=cls, id='admin.E128') + else: + if not isinstance(field, (models.DateField, models.DateTimeField)): + return must_be('a DateField or DateTimeField', option='date_hierarchy', + obj=cls, id='admin.E129') + else: + return [] + + +class InlineModelAdminChecks(BaseModelAdminChecks): + + def check(self, cls, parent_model, **kwargs): + errors = super(InlineModelAdminChecks, self).check(cls, model=cls.model, **kwargs) + errors.extend(self._check_fk_name(cls, parent_model)) + errors.extend(self._check_exclude_of_parent_model(cls, parent_model)) + errors.extend(self._check_extra(cls)) + errors.extend(self._check_max_num(cls)) + errors.extend(self._check_formset(cls)) + return errors + + def _check_exclude_of_parent_model(self, cls, parent_model): + # Do not perform more specific checks if the base checks result in an + # error. + errors = super(InlineModelAdminChecks, self)._check_exclude(cls, parent_model) + if errors: + return [] + + # Skip if `fk_name` is invalid. + if self._check_fk_name(cls, parent_model): + return [] + + if cls.exclude is None: + return [] + + fk = _get_foreign_key(parent_model, cls.model, fk_name=cls.fk_name) + if fk.name in cls.exclude: + return [ + checks.Error( + 'Cannot exclude the field "%s", because it is the foreign key ' + 'to the parent model %s.%s.' % ( + fk.name, parent_model._meta.app_label, parent_model._meta.object_name + ), + hint=None, + obj=cls, + id='admin.E201', + ) + ] + else: + return [] + + def _check_fk_name(self, cls, parent_model): + try: + _get_foreign_key(parent_model, cls.model, fk_name=cls.fk_name) + except ValueError as e: + return [checks.Error(e.args[0], hint=None, obj=cls, id='admin.E202')] + else: + return [] + + def _check_extra(self, cls): + """ Check that extra is an integer. """ + + if not isinstance(cls.extra, int): + return must_be('an integer', option='extra', obj=cls, id='admin.E203') + else: + return [] + + def _check_max_num(self, cls): + """ Check that max_num is an integer. """ + + if cls.max_num is None: + return [] + elif not isinstance(cls.max_num, int): + return must_be('an integer', option='max_num', obj=cls, id='admin.E204') + else: + return [] + + def _check_formset(self, cls): + """ Check formset is a subclass of BaseModelFormSet. """ + + if not issubclass(cls.formset, BaseModelFormSet): + return must_inherit_from(parent='BaseModelFormSet', option='formset', + obj=cls, id='admin.E205') + else: + return [] + + +def must_be(type, option, obj, id): + return [ + checks.Error( + '"%s" must be %s.' % (option, type), + hint=None, + obj=obj, + id=id, + ), + ] + + +def must_inherit_from(parent, option, obj, id): + return [ + checks.Error( + '"%s" must inherit from %s.' % (option, parent), + hint=None, + obj=obj, + id=id, + ), + ] + + +def refer_to_missing_field(field, option, model, obj, id): + return [ + checks.Error( + '"%s" refers to field "%s", which is missing from model %s.%s.' % ( + option, field, model._meta.app_label, model._meta.object_name + ), + hint=None, + obj=obj, + id=id, + ), + ] diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index 0f2404bf40..50c59a3575 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -8,14 +8,18 @@ from django import forms from django.conf import settings from django.contrib import messages from django.contrib.admin import widgets, helpers -from django.contrib.admin.utils import (unquote, flatten_fieldsets, get_deleted_objects, - model_format_dict, NestedObjects, lookup_needs_distinct) from django.contrib.admin import validation +from django.contrib.admin.checks import (BaseModelAdminChecks, ModelAdminChecks, + InlineModelAdminChecks) +from django.contrib.admin.utils import (unquote, flatten_fieldsets, + get_deleted_objects, model_format_dict, NestedObjects, + lookup_needs_distinct) from django.contrib.admin.templatetags.admin_static import static from django.contrib.admin.templatetags.admin_urls import add_preserved_filters from django.contrib.auth import get_permission_codename from django.contrib.contenttypes.models import ContentType -from django.core.exceptions import PermissionDenied, ValidationError, FieldError +from django.core import checks +from django.core.exceptions import PermissionDenied, ValidationError, FieldError, ImproperlyConfigured from django.core.paginator import Paginator from django.core.urlresolvers import reverse from django.db import models, transaction, router @@ -30,16 +34,17 @@ from django.http import Http404, HttpResponseRedirect from django.http.response import HttpResponseBase from django.shortcuts import get_object_or_404 from django.template.response import SimpleTemplateResponse, TemplateResponse -from django.utils.decorators import method_decorator -from django.utils.html import escape, escapejs -from django.utils.safestring import mark_safe from django.utils import six +from django.utils.decorators import method_decorator from django.utils.deprecation import RenameMethodsBase +from django.utils.encoding import force_text +from django.utils.encoding import python_2_unicode_compatible +from django.utils.html import escape, escapejs from django.utils.http import urlencode from django.utils.text import capfirst, get_text_list from django.utils.translation import ugettext as _ from django.utils.translation import ungettext -from django.utils.encoding import force_text +from django.utils.safestring import mark_safe from django.views.decorators.csrf import csrf_protect @@ -103,14 +108,42 @@ class BaseModelAdmin(six.with_metaclass(RenameBaseModelAdminMethods)): ordering = None view_on_site = True - # validation - validator_class = validation.BaseValidator + # Validation of ModelAdmin definitions + # Old, deprecated style: + validator_class = None + default_validator_class = validation.BaseValidator + # New style: + checks_class = BaseModelAdminChecks @classmethod def validate(cls, model): - validator = cls.validator_class() + warnings.warn( + 'ModelAdmin.validate() is deprecated. Use "check()" instead.', + PendingDeprecationWarning) + if cls.validator_class: + validator = cls.validator_class() + else: + validator = cls.default_validator_class() validator.validate(cls, model) + @classmethod + def check(cls, model, **kwargs): + if cls.validator_class: + warnings.warn( + 'ModelAdmin.validator_class is deprecated. ' + 'ModeAdmin validators must be converted to use ' + 'the system check framework.', + PendingDeprecationWarning) + validator = cls.validator_class() + try: + validator.validate(cls, model) + except ImproperlyConfigured as e: + return [checks.Error(e.args[0], hint=None, obj=cls)] + else: + return [] + else: + return cls.checks_class().check(cls, model, **kwargs) + def __init__(self): self._orig_formfield_overrides = self.formfield_overrides overrides = FORMFIELD_FOR_DBFIELD_DEFAULTS.copy() @@ -435,6 +468,7 @@ class BaseModelAdmin(six.with_metaclass(RenameBaseModelAdminMethods)): return request.user.has_perm("%s.%s" % (opts.app_label, codename)) +@python_2_unicode_compatible class ModelAdmin(BaseModelAdmin): "Encapsulates all admin options and functionality for a given model." @@ -469,7 +503,10 @@ class ModelAdmin(BaseModelAdmin): actions_selection_counter = True # validation - validator_class = validation.ModelAdminValidator + # Old, deprecated style: + default_validator_class = validation.ModelAdminValidator + # New style: + checks_class = ModelAdminChecks def __init__(self, model, admin_site): self.model = model @@ -477,6 +514,9 @@ class ModelAdmin(BaseModelAdmin): self.admin_site = admin_site super(ModelAdmin, self).__init__() + def __str__(self): + return "%s.%s" % (self.model._meta.app_label, self.__class__.__name__) + def get_inline_instances(self, request, obj=None): inline_instances = [] for inline_class in self.inlines: @@ -1685,8 +1725,7 @@ class InlineModelAdmin(BaseModelAdmin): verbose_name_plural = None can_delete = True - # validation - validator_class = validation.InlineValidator + checks_class = InlineModelAdminChecks def __init__(self, parent_model, admin_site): self.admin_site = admin_site diff --git a/django/contrib/admin/sites.py b/django/contrib/admin/sites.py index 010574f136..ba7c38dcad 100644 --- a/django/contrib/admin/sites.py +++ b/django/contrib/admin/sites.py @@ -100,7 +100,7 @@ class AdminSite(object): admin_class = type("%sAdmin" % model.__name__, (admin_class,), options) if admin_class is not ModelAdmin and settings.DEBUG: - admin_class.validate(model) + admin_class.check(model) # Instantiate the admin class to save in the registry self._registry[model] = admin_class(model, self) diff --git a/django/contrib/auth/__init__.py b/django/contrib/auth/__init__.py index f2bcaba390..24c870dafe 100644 --- a/django/contrib/auth/__init__.py +++ b/django/contrib/auth/__init__.py @@ -2,6 +2,8 @@ import inspect import re from django.conf import settings +from django.contrib.auth.checks import check_user_model +from django.core import checks from django.core.exceptions import ImproperlyConfigured, PermissionDenied from django.utils.module_loading import import_by_path from django.middleware.csrf import rotate_token @@ -13,6 +15,10 @@ BACKEND_SESSION_KEY = '_auth_user_backend' REDIRECT_FIELD_NAME = 'next' +# Register the user model checks +checks.register('models')(check_user_model) + + def load_backend(path): return import_by_path(path)() diff --git a/django/contrib/auth/checks.py b/django/contrib/auth/checks.py new file mode 100644 index 0000000000..f16da4dd47 --- /dev/null +++ b/django/contrib/auth/checks.py @@ -0,0 +1,69 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.apps import apps +from django.core import checks + + +def check_user_model(**kwargs): + from django.conf import settings + + errors = [] + app_name, model_name = settings.AUTH_USER_MODEL.split('.') + + cls = apps.get_model(app_name, model_name) + + # Check that REQUIRED_FIELDS is a list + if not isinstance(cls.REQUIRED_FIELDS, (list, tuple)): + errors.append( + checks.Error( + 'The REQUIRED_FIELDS must be a list or tuple.', + hint=None, + obj=cls, + id='auth.E001', + ) + ) + + # Check that the USERNAME FIELD isn't included in REQUIRED_FIELDS. + if cls.USERNAME_FIELD in cls.REQUIRED_FIELDS: + errors.append( + checks.Error( + ('The field named as the USERNAME_FIELD ' + 'must not be included in REQUIRED_FIELDS ' + 'on a custom user model.'), + hint=None, + obj=cls, + id='auth.E002', + ) + ) + + # Check that the username field is unique + if not cls._meta.get_field(cls.USERNAME_FIELD).unique: + if (settings.AUTHENTICATION_BACKENDS == + ('django.contrib.auth.backends.ModelBackend',)): + errors.append( + checks.Error( + ('The %s.%s field must be unique because it is ' + 'pointed to by USERNAME_FIELD.') % ( + cls._meta.object_name, cls.USERNAME_FIELD + ), + hint=None, + obj=cls, + id='auth.E003', + ) + ) + else: + errors.append( + checks.Warning( + ('The %s.%s field is pointed to by USERNAME_FIELD, ' + 'but it is not unique.') % ( + cls._meta.object_name, cls.USERNAME_FIELD + ), + hint=('Ensure that your authentication backend can handle ' + 'non-unique usernames.'), + obj=cls, + id='auth.W004', + ) + ) + + return errors diff --git a/django/contrib/auth/management/commands/changepassword.py b/django/contrib/auth/management/commands/changepassword.py index e333cafb04..4d6ae60acd 100644 --- a/django/contrib/auth/management/commands/changepassword.py +++ b/django/contrib/auth/management/commands/changepassword.py @@ -15,7 +15,7 @@ class Command(BaseCommand): ) help = "Change a user's password for django.contrib.auth." - requires_model_validation = False + requires_system_checks = False def _get_pass(self, prompt="Password: "): p = getpass.getpass(prompt=prompt) diff --git a/django/contrib/auth/tests/test_management.py b/django/contrib/auth/tests/test_management.py index 8a591fc7a9..f8af6829b5 100644 --- a/django/contrib/auth/tests/test_management.py +++ b/django/contrib/auth/tests/test_management.py @@ -3,17 +3,18 @@ from datetime import date from django.apps import apps from django.contrib.auth import models, management +from django.contrib.auth.checks import check_user_model from django.contrib.auth.management import create_permissions from django.contrib.auth.management.commands import changepassword from django.contrib.auth.models import User from django.contrib.auth.tests.custom_user import CustomUser from django.contrib.auth.tests.utils import skipIfCustomUser from django.contrib.contenttypes.models import ContentType +from django.core import checks from django.core import exceptions from django.core.management import call_command from django.core.management.base import CommandError -from django.core.management.validation import get_validation_errors -from django.test import TestCase, override_settings +from django.test import TestCase, override_settings, override_system_checks from django.utils import six from django.utils.six import StringIO @@ -161,7 +162,7 @@ class CreatesuperuserManagementCommandTestCase(TestCase): email="joe@somewhere.org", date_of_birth="1976-04-01", stdout=new_io, - skip_validation=True + skip_checks=True ) command_output = new_io.getvalue().strip() self.assertEqual(command_output, 'Superuser created successfully.') @@ -185,7 +186,7 @@ class CreatesuperuserManagementCommandTestCase(TestCase): username="joe@somewhere.org", stdout=new_io, stderr=new_io, - skip_validation=True + skip_checks=True ) self.assertEqual(CustomUser._default_manager.count(), 0) @@ -193,25 +194,81 @@ class CreatesuperuserManagementCommandTestCase(TestCase): class CustomUserModelValidationTestCase(TestCase): @override_settings(AUTH_USER_MODEL='auth.CustomUserNonListRequiredFields') + @override_system_checks([check_user_model]) def test_required_fields_is_list(self): "REQUIRED_FIELDS should be a list." - new_io = StringIO() - get_validation_errors(new_io, apps.get_app_config('auth')) - self.assertIn("The REQUIRED_FIELDS must be a list or tuple.", new_io.getvalue()) + + from .custom_user import CustomUserNonListRequiredFields + errors = checks.run_checks() + expected = [ + checks.Error( + 'The REQUIRED_FIELDS must be a list or tuple.', + hint=None, + obj=CustomUserNonListRequiredFields, + id='auth.E001', + ), + ] + self.assertEqual(errors, expected) @override_settings(AUTH_USER_MODEL='auth.CustomUserBadRequiredFields') + @override_system_checks([check_user_model]) def test_username_not_in_required_fields(self): "USERNAME_FIELD should not appear in REQUIRED_FIELDS." - new_io = StringIO() - get_validation_errors(new_io, apps.get_app_config('auth')) - self.assertIn("The field named as the USERNAME_FIELD should not be included in REQUIRED_FIELDS on a swappable User model.", new_io.getvalue()) + + from .custom_user import CustomUserBadRequiredFields + errors = checks.run_checks() + expected = [ + checks.Error( + ('The field named as the USERNAME_FIELD must not be included ' + 'in REQUIRED_FIELDS on a custom user model.'), + hint=None, + obj=CustomUserBadRequiredFields, + id='auth.E002', + ), + ] + self.assertEqual(errors, expected) @override_settings(AUTH_USER_MODEL='auth.CustomUserNonUniqueUsername') + @override_system_checks([check_user_model]) def test_username_non_unique(self): "A non-unique USERNAME_FIELD should raise a model validation error." - new_io = StringIO() - get_validation_errors(new_io, apps.get_app_config('auth')) - self.assertIn("The USERNAME_FIELD must be unique. Add unique=True to the field parameters.", new_io.getvalue()) + + from .custom_user import CustomUserNonUniqueUsername + errors = checks.run_checks() + expected = [ + checks.Error( + ('The CustomUserNonUniqueUsername.username field must be ' + 'unique because it is pointed to by USERNAME_FIELD.'), + hint=None, + obj=CustomUserNonUniqueUsername, + id='auth.E003', + ), + ] + self.assertEqual(errors, expected) + + @override_settings(AUTH_USER_MODEL='auth.CustomUserNonUniqueUsername', + AUTHENTICATION_BACKENDS=[ + 'my.custom.backend', + ]) + @override_system_checks([check_user_model]) + def test_username_non_unique_with_custom_backend(self): + """ A non-unique USERNAME_FIELD should raise an error only if we use the + default authentication backend. Otherwise, an warning should be raised. + """ + + from .custom_user import CustomUserNonUniqueUsername + errors = checks.run_checks() + expected = [ + checks.Warning( + ('The CustomUserNonUniqueUsername.username field is pointed to ' + 'by USERNAME_FIELD, but it is not unique.'), + hint=('Ensure that your authentication backend can handle ' + 'non-unique usernames.'), + obj=CustomUserNonUniqueUsername, + id='auth.W004', + ) + ] + self.assertEqual(errors, expected) class PermissionTestCase(TestCase): diff --git a/django/contrib/contenttypes/__init__.py b/django/contrib/contenttypes/__init__.py index e69de29bb2..e061c232a1 100644 --- a/django/contrib/contenttypes/__init__.py +++ b/django/contrib/contenttypes/__init__.py @@ -0,0 +1,8 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.contrib.contenttypes.checks import check_generic_foreign_keys +from django.core import checks + + +checks.register('models')(check_generic_foreign_keys) diff --git a/django/contrib/contenttypes/checks.py b/django/contrib/contenttypes/checks.py new file mode 100644 index 0000000000..7b4ab81962 --- /dev/null +++ b/django/contrib/contenttypes/checks.py @@ -0,0 +1,18 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.utils import six + + +def check_generic_foreign_keys(**kwargs): + from .generic import GenericForeignKey + from django.db import models + + errors = [] + fields = (obj + for cls in models.get_models() + for obj in six.itervalues(vars(cls)) + if isinstance(obj, GenericForeignKey)) + for field in fields: + errors.extend(field.check()) + return errors diff --git a/django/contrib/contenttypes/generic.py b/django/contrib/contenttypes/generic.py index 7d130bec24..3f9e5046ec 100644 --- a/django/contrib/contenttypes/generic.py +++ b/django/contrib/contenttypes/generic.py @@ -6,10 +6,12 @@ from __future__ import unicode_literals from collections import defaultdict from functools import partial +from django.core import checks from django.core.exceptions import ObjectDoesNotExist from django.db import connection from django.db import models, router, transaction, DEFAULT_DB_ALIAS -from django.db.models import signals +from django.db.models import signals, FieldDoesNotExist +from django.db.models.base import ModelBase from django.db.models.fields.related import ForeignObject, ForeignObjectRel from django.db.models.related import PathInfo from django.db.models.sql.datastructures import Col @@ -20,7 +22,7 @@ from django.contrib.admin.options import InlineModelAdmin, flatten_fieldsets from django.contrib.contenttypes.models import ContentType from django.utils import six from django.utils.deprecation import RenameMethodsBase -from django.utils.encoding import smart_text +from django.utils.encoding import smart_text, python_2_unicode_compatible class RenameGenericForeignKeyMethods(RenameMethodsBase): @@ -29,6 +31,7 @@ class RenameGenericForeignKeyMethods(RenameMethodsBase): ) +@python_2_unicode_compatible class GenericForeignKey(six.with_metaclass(RenameGenericForeignKeyMethods)): """ Provides a generic relation to any object through content-type/object-id @@ -53,6 +56,52 @@ class GenericForeignKey(six.with_metaclass(RenameGenericForeignKeyMethods)): setattr(cls, name, self) + def __str__(self): + model = self.model + app = model._meta.app_label + return '%s.%s.%s' % (app, model._meta.object_name, self.name) + + def check(self, **kwargs): + errors = [] + errors.extend(self._check_content_type_field()) + errors.extend(self._check_object_id_field()) + errors.extend(self._check_field_name()) + return errors + + def _check_content_type_field(self): + return _check_content_type_field( + model=self.model, + field_name=self.ct_field, + checked_object=self) + + def _check_object_id_field(self): + try: + self.model._meta.get_field(self.fk_field) + except FieldDoesNotExist: + return [ + checks.Error( + 'The field refers to "%s" field which is missing.' % self.fk_field, + hint=None, + obj=self, + id='contenttypes.E001', + ) + ] + else: + return [] + + def _check_field_name(self): + if self.name.endswith("_"): + return [ + checks.Error( + 'Field names must not end with underscores.', + hint=None, + obj=self, + id='contenttypes.E002', + ) + ] + else: + return [] + def instance_pre_init(self, signal, sender, args, kwargs, **_kwargs): """ Handles initializing an object with the generic FK instead of @@ -185,6 +234,72 @@ class GenericRelation(ForeignObject): to, to_fields=[], from_fields=[self.object_id_field_name], **kwargs) + def check(self, **kwargs): + errors = super(GenericRelation, self).check(**kwargs) + errors.extend(self._check_content_type_field()) + errors.extend(self._check_object_id_field()) + errors.extend(self._check_generic_foreign_key_existence()) + return errors + + def _check_content_type_field(self): + target = self.rel.to + if isinstance(target, ModelBase): + return _check_content_type_field( + model=target, + field_name=self.content_type_field_name, + checked_object=self) + else: + return [] + + def _check_object_id_field(self): + target = self.rel.to + if isinstance(target, ModelBase): + opts = target._meta + try: + opts.get_field(self.object_id_field_name) + except FieldDoesNotExist: + return [ + checks.Error( + 'The field refers to %s.%s field which is missing.' % ( + opts.object_name, self.object_id_field_name + ), + hint=None, + obj=self, + id='contenttypes.E003', + ) + ] + else: + return [] + else: + return [] + + def _check_generic_foreign_key_existence(self): + target = self.rel.to + if isinstance(target, ModelBase): + # Using `vars` is very ugly approach, but there is no better one, + # because GenericForeignKeys are not considered as fields and, + # therefore, are not included in `target._meta.local_fields`. + fields = target._meta.virtual_fields + if any(isinstance(field, GenericForeignKey) and + field.ct_field == self.content_type_field_name and + field.fk_field == self.object_id_field_name + for field in fields): + return [] + else: + return [ + checks.Warning( + ('The field defines a generic relation with the model ' + '%s.%s, but the model lacks GenericForeignKey.') % ( + target._meta.app_label, target._meta.object_name + ), + hint=None, + obj=self, + id='contenttypes.E004', + ) + ] + else: + return [] + def resolve_related_fields(self): self.to_fields = [self.model._meta.pk.name] return [(self.rel.to._meta.get_field_by_name(self.object_id_field_name)[0], @@ -252,6 +367,54 @@ class GenericRelation(ForeignObject): }) +def _check_content_type_field(model, field_name, checked_object): + """ Check if field named `field_name` in model `model` exists and is + valid content_type field (is a ForeignKey to ContentType). """ + + try: + field = model._meta.get_field(field_name) + except FieldDoesNotExist: + return [ + checks.Error( + 'The field refers to %s.%s field which is missing.' % ( + model._meta.object_name, field_name + ), + hint=None, + obj=checked_object, + id='contenttypes.E005', + ) + ] + else: + if not isinstance(field, models.ForeignKey): + return [ + checks.Error( + ('"%s" field is used by a %s ' + 'as content type field and therefore it must be ' + 'a ForeignKey.') % ( + field_name, checked_object.__class__.__name__ + ), + hint=None, + obj=checked_object, + id='contenttypes.E006', + ) + ] + elif field.rel.to != ContentType: + return [ + checks.Error( + ('"%s" field is used by a %s ' + 'as content type field and therefore it must be ' + 'a ForeignKey to ContentType.') % ( + field_name, checked_object.__class__.__name__ + ), + hint=None, + obj=checked_object, + id='contenttypes.E007', + ) + ] + else: + return [] + + class ReverseGenericRelatedObjectsDescriptor(object): """ This class provides the functionality that makes the related-object diff --git a/django/contrib/gis/management/commands/ogrinspect.py b/django/contrib/gis/management/commands/ogrinspect.py index 78c610635a..8ea1a9e8e9 100644 --- a/django/contrib/gis/management/commands/ogrinspect.py +++ b/django/contrib/gis/management/commands/ogrinspect.py @@ -74,7 +74,7 @@ class Command(LabelCommand): help='Generate mapping dictionary for use with `LayerMapping`.') ) - requires_model_validation = False + requires_system_checks = False def handle(self, *args, **options): try: diff --git a/django/contrib/sites/managers.py b/django/contrib/sites/managers.py index 173f439775..28e3168017 100644 --- a/django/contrib/sites/managers.py +++ b/django/contrib/sites/managers.py @@ -1,42 +1,66 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + from django.conf import settings +from django.core import checks from django.db import models from django.db.models.fields import FieldDoesNotExist class CurrentSiteManager(models.Manager): "Use this to limit objects to those associated with the current site." + def __init__(self, field_name=None): super(CurrentSiteManager, self).__init__() self.__field_name = field_name - self.__is_validated = False - def _validate_field_name(self): - field_names = self.model._meta.get_all_field_names() + def check(self, **kwargs): + errors = super(CurrentSiteManager, self).check(**kwargs) + errors.extend(self._check_field_name()) + return errors - # If a custom name is provided, make sure the field exists on the model - if self.__field_name is not None and self.__field_name not in field_names: - raise ValueError("%s couldn't find a field named %s in %s." % - (self.__class__.__name__, self.__field_name, self.model._meta.object_name)) - - # Otherwise, see if there is a field called either 'site' or 'sites' - else: - for potential_name in ['site', 'sites']: - if potential_name in field_names: - self.__field_name = potential_name - self.__is_validated = True - break - - # Now do a type check on the field (FK or M2M only) + def _check_field_name(self): + field_name = self._get_field_name() try: - field = self.model._meta.get_field(self.__field_name) - if not isinstance(field, (models.ForeignKey, models.ManyToManyField)): - raise TypeError("%s must be a ForeignKey or ManyToManyField." % self.__field_name) + field = self.model._meta.get_field(field_name) except FieldDoesNotExist: - raise ValueError("%s couldn't find a field named %s in %s." % - (self.__class__.__name__, self.__field_name, self.model._meta.object_name)) - self.__is_validated = True + return [ + checks.Error( + "CurrentSiteManager could not find a field named '%s'." % field_name, + hint=('Ensure that you did not misspell the field name. ' + 'Does the field exist?'), + obj=self, + id='sites.E001', + ) + ] + + if not isinstance(field, (models.ForeignKey, models.ManyToManyField)): + return [ + checks.Error( + "CurrentSiteManager requires that '%s.%s' must be a " + "ForeignKey or ManyToManyField." % ( + self.model._meta.object_name, field_name + ), + hint=None, + obj=self, + id='sites.E002', + ) + ] + + return [] + + def _get_field_name(self): + """ Return self.__field_name or 'site' or 'sites'. """ + + if not self.__field_name: + try: + self.model._meta.get_field('site') + except FieldDoesNotExist: + self.__field_name = 'sites' + else: + self.__field_name = 'site' + return self.__field_name def get_queryset(self): - if not self.__is_validated: - self._validate_field_name() - return super(CurrentSiteManager, self).get_queryset().filter(**{self.__field_name + '__id': settings.SITE_ID}) + return super(CurrentSiteManager, self).get_queryset().filter( + **{self._get_field_name() + '__id': settings.SITE_ID}) diff --git a/django/contrib/staticfiles/management/commands/collectstatic.py b/django/contrib/staticfiles/management/commands/collectstatic.py index 9009b82c67..d16dab540c 100644 --- a/django/contrib/staticfiles/management/commands/collectstatic.py +++ b/django/contrib/staticfiles/management/commands/collectstatic.py @@ -45,7 +45,7 @@ class Command(NoArgsCommand): "'.*' and '*~'."), ) help = "Collect static files in a single location." - requires_model_validation = False + requires_system_checks = False def __init__(self, *args, **kwargs): super(NoArgsCommand, self).__init__(*args, **kwargs) diff --git a/django/core/checks/__init__.py b/django/core/checks/__init__.py index e69de29bb2..82a3acf337 100644 --- a/django/core/checks/__init__.py +++ b/django/core/checks/__init__.py @@ -0,0 +1,18 @@ +# -*- coding: utf8 -*- +from __future__ import unicode_literals + +from .messages import (CheckMessage, + Debug, Info, Warning, Error, Critical, + DEBUG, INFO, WARNING, ERROR, CRITICAL) +from .registry import register, run_checks, tag_exists + +# Import these to force registration of checks +import django.core.checks.compatibility.django_1_6_0 # NOQA +import django.core.checks.model_checks # NOQA + +__all__ = [ + 'CheckMessage', + 'Debug', 'Info', 'Warning', 'Error', 'Critical', + 'DEBUG', 'INFO', 'WARNING', 'ERROR', 'CRITICAL', + 'register', 'run_checks', 'tag_exists', +] diff --git a/django/core/checks/compatibility/base.py b/django/core/checks/compatibility/base.py deleted file mode 100644 index 7fe52d2af9..0000000000 --- a/django/core/checks/compatibility/base.py +++ /dev/null @@ -1,39 +0,0 @@ -from __future__ import unicode_literals -import warnings - -from django.core.checks.compatibility import django_1_6_0 - - -COMPAT_CHECKS = [ - # Add new modules at the top, so we keep things in descending order. - # After two-three minor releases, old versions should get dropped. - django_1_6_0, -] - - -def check_compatibility(): - """ - Runs through compatibility checks to warn the user with an existing install - about changes in an up-to-date Django. - - Modules should be located in ``django.core.compat_checks`` (typically one - per release of Django) & must have a ``run_checks`` function that runs - all the checks. - - Returns a list of informational messages about incompatibilities. - """ - messages = [] - - for check_module in COMPAT_CHECKS: - check = getattr(check_module, 'run_checks', None) - - if check is None: - warnings.warn( - "The '%s' module lacks a " % check_module.__name__ + - "'run_checks' method, which is needed to verify compatibility." - ) - continue - - messages.extend(check()) - - return messages diff --git a/django/core/checks/compatibility/django_1_6_0.py b/django/core/checks/compatibility/django_1_6_0.py index 1be727f70d..999059b681 100644 --- a/django/core/checks/compatibility/django_1_6_0.py +++ b/django/core/checks/compatibility/django_1_6_0.py @@ -1,10 +1,20 @@ +# -*- encoding: utf-8 -*- from __future__ import unicode_literals from django.apps import apps -from django.db import models + +from .. import Warning, register -def check_test_runner(): +@register('compatibility') +def check_1_6_compatibility(**kwargs): + errors = [] + errors.extend(_check_test_runner(**kwargs)) + errors.extend(_check_boolean_field_default_value(**kwargs)) + return errors + + +def _check_test_runner(app_configs=None, **kwargs): """ Checks if the user has *not* overridden the ``TEST_RUNNER`` setting & warns them about the default behavior changes. @@ -13,53 +23,94 @@ def check_test_runner(): doing & avoid generating a message. """ from django.conf import settings - new_default = 'django.test.runner.DiscoverRunner' - test_runner_setting = getattr(settings, 'TEST_RUNNER', new_default) - if test_runner_setting == new_default: - message = [ - "Django 1.6 introduced a new default test runner ('%s')" % new_default, - "You should ensure your tests are all running & behaving as expected. See", - "https://docs.djangoproject.com/en/dev/releases/1.6/#discovery-of-tests-in-any-test-module", - "for more information.", + # We need to establish if this is a project defined on the 1.5 project template, + # because if the project was generated on the 1.6 template, it will have be been + # developed with the new TEST_RUNNER behavior in mind. + + # There's no canonical way to do this; so we leverage off the fact that 1.6 + # also introduced a new project template, removing a bunch of settings from the + # default that won't be in common usage. + + # We make this determination on a balance of probabilities. Each of these factors + # contributes a weight; if enough of them trigger, we've got a likely 1.6 project. + weight = 0 + + # If TEST_RUNNER is explicitly set, it's all a moot point - if it's been explcitly set, + # the user has opted into a specific set of behaviors, which won't change as the + # default changes. + if not settings.is_overridden('TEST_RUNNER'): + # Strong markers: + # SITE_ID = 1 is in 1.5 template, not defined in 1.6 template + try: + settings.SITE_ID + weight += 2 + except AttributeError: + pass + + # BASE_DIR is not defined in 1.5 template, set in 1.6 template + try: + settings.BASE_DIR + except AttributeError: + weight += 2 + + # TEMPLATE_LOADERS defined in 1.5 template, not defined in 1.6 template + if settings.is_overridden('TEMPLATE_LOADERS'): + weight += 2 + + # MANAGERS defined in 1.5 template, not defined in 1.6 template + if settings.is_overridden('MANAGERS'): + weight += 2 + + # Weaker markers - These are more likely to have been added in common usage + # ADMINS defined in 1.5 template, not defined in 1.6 template + if settings.is_overridden('ADMINS'): + weight += 1 + + # Clickjacking enabled by default in 1.6 + if 'django.middleware.clickjacking.XFrameOptionsMiddleware' not in set(settings.MIDDLEWARE_CLASSES): + weight += 1 + + if weight >= 6: + return [ + Warning( + "Some project unittests may not execute as expected.", + hint=("Django 1.6 introduced a new default test runner. It looks like " + "this project was generated using Django 1.5 or earlier. You should " + "ensure your tests are all running & behaving as expected. See " + "https://docs.djangoproject.com/en/dev/releases/1.6/#discovery-of-tests-in-any-test-module " + "for more information."), + obj=None, + id='1_6.W001', + ) ] - return ' '.join(message) + else: + return [] -def check_boolean_field_default_value(): +def _check_boolean_field_default_value(app_configs=None, **kwargs): """ Checks if there are any BooleanFields without a default value, & - warns the user that the default has changed from False to Null. + warns the user that the default has changed from False to None. """ - fields = [] - for cls in apps.get_models(): - opts = cls._meta - for f in opts.local_fields: - if isinstance(f, models.BooleanField) and not f.has_default(): - fields.append( - '%s.%s: "%s"' % (opts.app_label, opts.object_name, f.name) - ) - if fields: - fieldnames = ", ".join(fields) - message = [ - "You have not set a default value for one or more BooleanFields:", - "%s." % fieldnames, - "In Django 1.6 the default value of BooleanField was changed from", - "False to Null when Field.default isn't defined. See", - "https://docs.djangoproject.com/en/1.6/ref/models/fields/#booleanfield" - "for more information." - ] - return ' '.join(message) + from django.db import models - -def run_checks(): - """ - Required by the ``check`` management command, this returns a list of - messages from all the relevant check functions for this version of Django. - """ - checks = [ - check_test_runner(), - check_boolean_field_default_value(), + problem_fields = [ + field + for model in apps.get_models(**kwargs) + if app_configs is None or model._meta.app_config in app_configs + for field in model._meta.local_fields + if isinstance(field, models.BooleanField) and not field.has_default() + ] + + return [ + Warning( + "BooleanField does not have a default value. ", + hint=("Django 1.6 changed the default value of BooleanField from False to None. " + "See https://docs.djangoproject.com/en/1.6/ref/models/fields/#booleanfield " + "for more information."), + obj=field, + id='1_6.W002', + ) + for field in problem_fields ] - # Filter out the ``None`` or empty strings. - return [output for output in checks if output] diff --git a/django/core/checks/messages.py b/django/core/checks/messages.py new file mode 100644 index 0000000000..bda3ff9a5c --- /dev/null +++ b/django/core/checks/messages.py @@ -0,0 +1,84 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.utils.encoding import python_2_unicode_compatible, force_str + + +# Levels +DEBUG = 10 +INFO = 20 +WARNING = 30 +ERROR = 40 +CRITICAL = 50 + + +@python_2_unicode_compatible +class CheckMessage(object): + + def __init__(self, level, msg, hint, obj=None, id=None): + assert isinstance(level, int), "The first argument should be level." + self.level = level + self.msg = msg + self.hint = hint + self.obj = obj + self.id = id + + def __eq__(self, other): + return all(getattr(self, attr) == getattr(other, attr) + for attr in ['level', 'msg', 'hint', 'obj', 'id']) + + def __ne__(self, other): + return not (self == other) + + def __str__(self): + from django.db import models + + if self.obj is None: + obj = "?" + elif isinstance(self.obj, models.base.ModelBase): + # We need to hardcode ModelBase and Field cases because its __str__ + # method doesn't return "applabel.modellabel" and cannot be changed. + model = self.obj + app = model._meta.app_label + obj = '%s.%s' % (app, model._meta.object_name) + else: + obj = force_str(self.obj) + id = "(%s) " % self.id if self.id else "" + hint = "\n\tHINT: %s" % self.hint if self.hint else '' + return "%s: %s%s%s" % (obj, id, self.msg, hint) + + def __repr__(self): + return "<%s: level=%r, msg=%r, hint=%r, obj=%r, id=%r>" % \ + (self.__class__.__name__, self.level, self.msg, self.hint, self.obj, self.id) + + def is_serious(self): + return self.level >= ERROR + + def is_silenced(self): + from django.conf import settings + return self.id in settings.SILENCED_SYSTEM_CHECKS + + +class Debug(CheckMessage): + def __init__(self, *args, **kwargs): + return super(Debug, self).__init__(DEBUG, *args, **kwargs) + + +class Info(CheckMessage): + def __init__(self, *args, **kwargs): + return super(Info, self).__init__(INFO, *args, **kwargs) + + +class Warning(CheckMessage): + def __init__(self, *args, **kwargs): + return super(Warning, self).__init__(WARNING, *args, **kwargs) + + +class Error(CheckMessage): + def __init__(self, *args, **kwargs): + return super(Error, self).__init__(ERROR, *args, **kwargs) + + +class Critical(CheckMessage): + def __init__(self, *args, **kwargs): + return super(Critical, self).__init__(CRITICAL, *args, **kwargs) diff --git a/django/core/checks/model_checks.py b/django/core/checks/model_checks.py new file mode 100644 index 0000000000..3110d7c491 --- /dev/null +++ b/django/core/checks/model_checks.py @@ -0,0 +1,49 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from itertools import chain +import types + +from django.apps import apps + +from . import Error, register + + +@register('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)) + + +@register('models', 'signals') +def check_model_signals(app_configs=None, **kwargs): + """Ensure lazily referenced model signals senders are installed.""" + from django.db import models + errors = [] + + for name in dir(models.signals): + obj = getattr(models.signals, name) + if isinstance(obj, models.signals.ModelSignal): + for reference, receivers in obj.unresolved_references.items(): + for receiver, _, _ in receivers: + # The receiver is either a function or an instance of class + # defining a `__call__` method. + if isinstance(receiver, types.FunctionType): + description = "The `%s` function" % receiver.__name__ + else: + description = "An instance of the `%s` class" % receiver.__class__.__name__ + errors.append( + Error( + "%s was connected to the `%s` signal " + "with a lazy reference to the '%s' sender, " + "which has not been installed." % ( + description, name, '.'.join(reference) + ), + obj=receiver.__module__, + hint=None, + id='E014' + ) + ) + return errors diff --git a/django/core/checks/registry.py b/django/core/checks/registry.py new file mode 100644 index 0000000000..efb5b4b892 --- /dev/null +++ b/django/core/checks/registry.py @@ -0,0 +1,63 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from itertools import chain + +from django.utils.itercompat import is_iterable + + +class CheckRegistry(object): + + def __init__(self): + self.registered_checks = [] + + def register(self, *tags): + """ + Decorator. Register given function `f` labeled with given `tags`. The + function should receive **kwargs and return list of Errors and + Warnings. + + Example:: + + registry = CheckRegistry() + @registry.register('mytag', 'anothertag') + def my_check(apps, **kwargs): + # ... perform checks and collect `errors` ... + return errors + + """ + + def inner(check): + check.tags = tags + self.registered_checks.append(check) + return check + + return inner + + def run_checks(self, app_configs=None, tags=None): + """ Run all registered checks and return list of Errors and Warnings. + """ + errors = [] + if tags is not None: + checks = [check for check in self.registered_checks + if hasattr(check, 'tags') and set(check.tags) & set(tags)] + else: + checks = self.registered_checks + + for check in checks: + new_errors = check(app_configs=app_configs) + assert is_iterable(new_errors), ( + "The function %r did not return a list. All functions registered " + "with the checks registry must return a list." % check) + errors.extend(new_errors) + return errors + + def tag_exists(self, tag): + tags = chain(*[check.tags for check in self.registered_checks if hasattr(check, 'tags')]) + return tag in tags + + +registry = CheckRegistry() +register = registry.register +run_checks = registry.run_checks +tag_exists = registry.tag_exists diff --git a/django/core/management/base.py b/django/core/management/base.py index e55f777b75..5ac1cdf38d 100644 --- a/django/core/management/base.py +++ b/django/core/management/base.py @@ -1,3 +1,6 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + """ Base classes for writing management commands (named commands which can be executed through ``django-admin.py`` or ``manage.py``). @@ -12,9 +15,10 @@ import warnings from optparse import make_option, OptionParser import django +from django.core import checks +from django.core.exceptions import ImproperlyConfigured from django.core.management.color import color_style, no_style from django.utils.encoding import force_str -from django.utils.six import StringIO class CommandError(Exception): @@ -136,7 +140,20 @@ class BaseCommand(object): wrapped with ``BEGIN;`` and ``COMMIT;``. Default value is ``False``. + ``requires_system_checks`` + A boolean; if ``True``, entire Django project will be checked for errors + prior to executing the command. Default value is ``True``. + To validate an individual application's models + rather than all applications' models, call + ``self.check(app_configs)`` from ``handle()``, where ``app_configs`` + is the list of application's configuration provided by the + app registry. + ``requires_model_validation`` + DEPRECATED - This value will only be used if requires_system_checks + has not been provided. Defining both ``requires_system_checks`` and + ``requires_model_validation`` will result in an error. + A boolean; if ``True``, validation of installed models will be performed prior to executing the command. Default value is ``True``. To validate an individual application's models @@ -181,13 +198,40 @@ class BaseCommand(object): # Configuration shortcuts that alter various logic. can_import_settings = True - requires_model_validation = True output_transaction = False # Whether to wrap the output in a "BEGIN; COMMIT;" leave_locale_alone = False + # Uncomment the following line of code after deprecation plan for + # requires_model_validation comes to completion: + # + # requires_system_checks = True + def __init__(self): self.style = color_style() + # `requires_model_validation` is deprecated in favour of + # `requires_system_checks`. If both options are present, an error is + # raised. Otherwise the present option is used. If none of them is + # defined, the default value (True) is used. + has_old_option = hasattr(self, 'requires_model_validation') + has_new_option = hasattr(self, 'requires_system_checks') + + if has_old_option: + warnings.warn( + '"requires_model_validation" is deprecated ' + 'in favour of "requires_system_checks".', + PendingDeprecationWarning) + if has_old_option and has_new_option: + raise ImproperlyConfigured( + 'Command %s defines both "requires_model_validation" ' + 'and "requires_system_checks", which is illegal. Use only ' + '"requires_system_checks".' % self.__class__.__name__) + + self.requires_system_checks = ( + self.requires_system_checks if has_new_option else + self.requires_model_validation if has_old_option else + True) + def get_version(self): """ Return the Django version, which should be correct for all @@ -253,8 +297,8 @@ class BaseCommand(object): def execute(self, *args, **options): """ - Try to execute this command, performing model validation if - needed (as controlled by the attribute + Try to execute this command, performing system checks if needed (as + controlled by attributes ``self.requires_system_checks`` and ``self.requires_model_validation``, except if force-skipped). """ self.stdout = OutputWrapper(options.get('stdout', sys.stdout)) @@ -286,8 +330,10 @@ class BaseCommand(object): translation.activate('en-us') try: - if self.requires_model_validation and not options.get('skip_validation'): - self.validate() + if (self.requires_system_checks and + not options.get('skip_validation') and # This will be removed at the end of deprecation proccess for `skip_validation`. + not options.get('skip_checks')): + self.check() output = self.handle(*args, **options) if output: if self.output_transaction: @@ -305,21 +351,67 @@ class BaseCommand(object): translation.activate(saved_locale) def validate(self, app_config=None, display_num_errors=False): - """ - Validates the given app, raising CommandError for any errors. + """ Deprecated. Delegates to ``check``.""" - If app_config is None, then this will validate all installed apps. + if app_config is None: + app_configs = None + else: + app_configs = [app_config] + + return self.check(app_configs=app_configs, display_num_errors=display_num_errors) + + def check(self, app_configs=None, tags=None, display_num_errors=False): + """ + Uses the system check framework to validate entire Django project. + Raises CommandError for any serious message (error or critical errors). + If there are only light messages (like warnings), they are printed to + stderr and no exception is raised. """ - from django.core.management.validation import get_validation_errors - s = StringIO() - num_errors = get_validation_errors(s, app_config) - if num_errors: - s.seek(0) - error_text = s.read() - raise CommandError("One or more models did not validate:\n%s" % error_text) + all_issues = checks.run_checks(app_configs=app_configs, tags=tags) + + msg = "" + if all_issues: + debugs = [e for e in all_issues if e.level < checks.INFO and not e.is_silenced()] + infos = [e for e in all_issues if checks.INFO <= e.level < checks.WARNING and not e.is_silenced()] + warnings = [e for e in all_issues if checks.WARNING <= e.level < checks.ERROR and not e.is_silenced()] + errors = [e for e in all_issues if checks.ERROR <= e.level < checks.CRITICAL] + criticals = [e for e in all_issues if checks.CRITICAL <= e.level] + sorted_issues = [ + (criticals, 'CRITICALS'), + (errors, 'ERRORS'), + (warnings, 'WARNINGS'), + (infos, 'INFOS'), + (debugs, 'DEBUGS'), + ] + + for issues, group_name in sorted_issues: + if issues: + formatted = ( + color_style().ERROR(force_str(e)) + if e.is_serious() + else color_style().WARNING(force_str(e)) + for e in issues) + formatted = "\n".join(sorted(formatted)) + msg += '\n%s:\n%s\n' % (group_name, formatted) + + msg = "System check identified some issues:\n%s" % msg + if display_num_errors: - self.stdout.write("%s error%s found" % (num_errors, '' if num_errors == 1 else 's')) + if msg: + msg += '\n' + msg += "System check identified %s." % ( + "no issues" if len(all_issues) == 0 else + "1 issue" if len(all_issues) == 1 else + "%s issues" % len(all_issues) + ) + + if any(e.is_serious() and not e.is_silenced() for e in all_issues): + raise CommandError(msg) + elif msg and all_issues: + self.stderr.write(msg) + elif msg: + self.stdout.write(msg) def handle(self, *args, **options): """ diff --git a/django/core/management/commands/check.py b/django/core/management/commands/check.py index 05f48c82bc..b6d2571ab0 100644 --- a/django/core/management/commands/check.py +++ b/django/core/management/commands/check.py @@ -1,14 +1,32 @@ +# -*- coding: utf-8 -*- from __future__ import unicode_literals -import warnings -from django.core.checks.compatibility.base import check_compatibility -from django.core.management.base import NoArgsCommand +from optparse import make_option + +from django.apps import apps +from django.core import checks +from django.core.management.base import BaseCommand, CommandError -class Command(NoArgsCommand): - help = "Checks your configuration's compatibility with this version " + \ - "of Django." +class Command(BaseCommand): + help = "Checks the entire Django project for potential problems." - def handle_noargs(self, **options): - for message in check_compatibility(): - warnings.warn(message) + requires_system_checks = False + + option_list = BaseCommand.option_list + ( + make_option('--tag', '-t', action='append', dest='tags', + help='Run only checks labeled with given tag.'), + ) + + def handle(self, *app_labels, **options): + if app_labels: + app_configs = [apps.get_app_config(app_label) for app_label in app_labels] + else: + app_configs = None + + tags = options.get('tags', None) + if tags and any(not checks.tag_exists(tag) for tag in tags): + invalid_tag = next(tag for tag in tags if not checks.tag_exists(tag)) + raise CommandError('There is no system check with the "%s" tag.' % invalid_tag) + + self.check(app_configs=app_configs, tags=tags, display_num_errors=True) diff --git a/django/core/management/commands/compilemessages.py b/django/core/management/commands/compilemessages.py index 915176e6a7..4367c31a9f 100644 --- a/django/core/management/commands/compilemessages.py +++ b/django/core/management/commands/compilemessages.py @@ -65,7 +65,7 @@ class Command(BaseCommand): ) help = 'Compiles .po files to .mo files for use with builtin gettext support.' - requires_model_validation = False + requires_system_checks = False leave_locale_alone = True def handle(self, **options): diff --git a/django/core/management/commands/createcachetable.py b/django/core/management/commands/createcachetable.py index a71ff945a9..10506525fc 100644 --- a/django/core/management/commands/createcachetable.py +++ b/django/core/management/commands/createcachetable.py @@ -19,7 +19,7 @@ class Command(BaseCommand): 'Defaults to the "default" database.'), ) - requires_model_validation = False + requires_system_checks = False def handle(self, *tablenames, **options): db = options.get('database') diff --git a/django/core/management/commands/dbshell.py b/django/core/management/commands/dbshell.py index 866dab0ed3..5cb0a411a5 100644 --- a/django/core/management/commands/dbshell.py +++ b/django/core/management/commands/dbshell.py @@ -14,7 +14,7 @@ class Command(BaseCommand): 'open a shell. Defaults to the "default" database.'), ) - requires_model_validation = False + requires_system_checks = False def handle(self, **options): connection = connections[options.get('database')] diff --git a/django/core/management/commands/diffsettings.py b/django/core/management/commands/diffsettings.py index 4d1f4cb5b3..c346b14fff 100644 --- a/django/core/management/commands/diffsettings.py +++ b/django/core/management/commands/diffsettings.py @@ -19,7 +19,7 @@ class Command(NoArgsCommand): 'Default values are prefixed by "###".'), ) - requires_model_validation = False + requires_system_checks = False def handle_noargs(self, **options): # Inspired by Postfix's "postconf -n". diff --git a/django/core/management/commands/inspectdb.py b/django/core/management/commands/inspectdb.py index b9ff34d53d..54fdad2001 100644 --- a/django/core/management/commands/inspectdb.py +++ b/django/core/management/commands/inspectdb.py @@ -18,7 +18,7 @@ class Command(NoArgsCommand): 'introspect. Defaults to using the "default" database.'), ) - requires_model_validation = False + requires_system_checks = False db_module = 'django.db' diff --git a/django/core/management/commands/makemessages.py b/django/core/management/commands/makemessages.py index 19df0bc43a..9c81bc8e20 100644 --- a/django/core/management/commands/makemessages.py +++ b/django/core/management/commands/makemessages.py @@ -203,7 +203,7 @@ class Command(NoArgsCommand): "applications) directory.\n\nYou must run this command with one of either the " "--locale or --all options.") - requires_model_validation = False + requires_system_checks = False leave_locale_alone = True def handle_noargs(self, *args, **options): diff --git a/django/core/management/commands/runserver.py b/django/core/management/commands/runserver.py index 327832ba43..e85aa88c3e 100644 --- a/django/core/management/commands/runserver.py +++ b/django/core/management/commands/runserver.py @@ -37,7 +37,7 @@ class Command(BaseCommand): args = '[optional port number, or ipaddr:port]' # Validation is called explicitly each time the server is reloaded. - requires_model_validation = False + requires_system_checks = False def get_handler(self, *args, **options): """ @@ -99,7 +99,7 @@ class Command(BaseCommand): shutdown_message = options.get('shutdown_message', '') quit_command = 'CTRL-BREAK' if sys.platform == 'win32' else 'CONTROL-C' - self.stdout.write("Validating models...\n\n") + self.stdout.write("Performing system checks...\n\n") self.validate(display_num_errors=True) self.check_migrations() now = datetime.now().strftime('%B %d, %Y - %X') diff --git a/django/core/management/commands/shell.py b/django/core/management/commands/shell.py index 5ef51db3f9..5e833a29e5 100644 --- a/django/core/management/commands/shell.py +++ b/django/core/management/commands/shell.py @@ -18,7 +18,7 @@ class Command(NoArgsCommand): ) help = "Runs a Python interactive interpreter. Tries to use IPython or bpython, if one of them is available." - requires_model_validation = False + requires_system_checks = False def _ipython_pre_011(self): """Start IPython pre-0.11""" diff --git a/django/core/management/commands/test.py b/django/core/management/commands/test.py index 5232c37646..36dca9c27b 100644 --- a/django/core/management/commands/test.py +++ b/django/core/management/commands/test.py @@ -30,7 +30,7 @@ class Command(BaseCommand): help = ('Discover and run tests in the specified modules or the current directory.') args = '[path.to.modulename|path.to.modulename.TestCase|path.to.modulename.TestCase.test_method]...' - requires_model_validation = False + requires_system_checks = False def __init__(self): self.test_runner = None diff --git a/django/core/management/commands/testserver.py b/django/core/management/commands/testserver.py index 79f0ff5501..0a9c99dcbf 100644 --- a/django/core/management/commands/testserver.py +++ b/django/core/management/commands/testserver.py @@ -16,7 +16,7 @@ class Command(BaseCommand): help = 'Runs a development server with data from the given fixture(s).' args = '[fixture ...]' - requires_model_validation = False + requires_system_checks = False def handle(self, *fixture_labels, **options): from django.core.management import call_command diff --git a/django/core/management/commands/validate.py b/django/core/management/commands/validate.py index 0dec3ea8b9..b032c33f94 100644 --- a/django/core/management/commands/validate.py +++ b/django/core/management/commands/validate.py @@ -1,10 +1,15 @@ -from django.core.management.base import NoArgsCommand +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +import warnings + +from django.core.management.commands.check import Command as CheckCommand -class Command(NoArgsCommand): - help = "Validates all installed models." - - requires_model_validation = False +class Command(CheckCommand): + help = 'Deprecated. Use "check" command instead. ' + CheckCommand.help def handle_noargs(self, **options): - self.validate(display_num_errors=True) + warnings.warn('"validate" has been deprecated in favour of "check".', + PendingDeprecationWarning) + super(Command, self).handle_noargs(**options) diff --git a/django/core/management/templates.py b/django/core/management/templates.py index 728b1af34c..c2a6476912 100644 --- a/django/core/management/templates.py +++ b/django/core/management/templates.py @@ -52,7 +52,7 @@ class TemplateCommand(BaseCommand): 'Separate multiple extensions with commas, or use ' '-n multiple times.') ) - requires_model_validation = False + requires_system_checks = False # Can't import settings during this command, because they haven't # necessarily been created. can_import_settings = False diff --git a/django/core/management/validation.py b/django/core/management/validation.py deleted file mode 100644 index e5eee65fef..0000000000 --- a/django/core/management/validation.py +++ /dev/null @@ -1,411 +0,0 @@ -import collections -import sys -import types - -from django.conf import settings -from django.core.management.color import color_style -from django.utils.encoding import force_str -from django.utils.itercompat import is_iterable -from django.utils import six - - -class ModelErrorCollection: - def __init__(self, outfile=sys.stdout): - self.errors = [] - self.outfile = outfile - self.style = color_style() - - def add(self, context, error): - self.errors.append((context, error)) - self.outfile.write(self.style.ERROR(force_str("%s: %s\n" % (context, error)))) - - -def get_validation_errors(outfile, app_config=None): - """ - Validates all models that are part of the specified app. If no app name is provided, - validates all models of all installed apps. Writes errors, if any, to outfile. - Returns number of errors. - """ - from django.apps import apps - from django.db import connection, models - from django.db.models.deletion import SET_NULL, SET_DEFAULT - - e = ModelErrorCollection(outfile) - - for cls in (app_config or apps).get_models(include_swapped=True): - opts = cls._meta - - # Check swappable attribute. - if opts.swapped: - try: - app_label, model_name = opts.swapped.split('.') - except ValueError: - e.add(opts, "%s is not of the form 'app_label.app_name'." % opts.swappable) - continue - try: - apps.get_model(app_label, model_name) - except LookupError: - e.add(opts, "Model has been swapped out for '%s' which has not been installed or is abstract." % opts.swapped) - # No need to perform any other validation checks on a swapped model. - continue - - # If this is the current User model, check known validation problems with User models - if settings.AUTH_USER_MODEL == '%s.%s' % (opts.app_label, opts.object_name): - # Check that REQUIRED_FIELDS is a list - if not isinstance(cls.REQUIRED_FIELDS, (list, tuple)): - e.add(opts, 'The REQUIRED_FIELDS must be a list or tuple.') - - # Check that the USERNAME FIELD isn't included in REQUIRED_FIELDS. - if cls.USERNAME_FIELD in cls.REQUIRED_FIELDS: - e.add(opts, 'The field named as the USERNAME_FIELD should not be included in REQUIRED_FIELDS on a swappable User model.') - - # Check that the username field is unique - if not opts.get_field(cls.USERNAME_FIELD).unique: - e.add(opts, 'The USERNAME_FIELD must be unique. Add unique=True to the field parameters.') - - # Store a list of column names which have already been used by other fields. - used_column_names = [] - - # Model isn't swapped; do field-specific validation. - for f in opts.local_fields: - if f.name == 'id' and not f.primary_key and opts.pk.name == 'id': - e.add(opts, '"%s": You can\'t use "id" as a field name, because each model automatically gets an "id" field if none of the fields have primary_key=True. You need to either remove/rename your "id" field or add primary_key=True to a field.' % f.name) - if f.name.endswith('_'): - e.add(opts, '"%s": Field names cannot end with underscores, because this would lead to ambiguous queryset filters.' % f.name) - if (f.primary_key and f.null and - not connection.features.interprets_empty_strings_as_nulls): - # We cannot reliably check this for backends like Oracle which - # consider NULL and '' to be equal (and thus set up - # character-based fields a little differently). - e.add(opts, '"%s": Primary key fields cannot have null=True.' % f.name) - - # Column name validation. - # Determine which column name this field wants to use. - _, column_name = f.get_attname_column() - - # Ensure the column name is not already in use. - if column_name and column_name in used_column_names: - e.add(opts, "Field '%s' has column name '%s' that is already used." % (f.name, column_name)) - else: - used_column_names.append(column_name) - - if isinstance(f, models.CharField): - try: - max_length = int(f.max_length) - if max_length <= 0: - e.add(opts, '"%s": CharFields require a "max_length" attribute that is a positive integer.' % f.name) - except (ValueError, TypeError): - e.add(opts, '"%s": CharFields require a "max_length" attribute that is a positive integer.' % f.name) - if isinstance(f, models.DecimalField): - decimalp_ok, mdigits_ok = False, False - decimalp_msg = '"%s": DecimalFields require a "decimal_places" attribute that is a non-negative integer.' - try: - decimal_places = int(f.decimal_places) - if decimal_places < 0: - e.add(opts, decimalp_msg % f.name) - else: - decimalp_ok = True - except (ValueError, TypeError): - e.add(opts, decimalp_msg % f.name) - mdigits_msg = '"%s": DecimalFields require a "max_digits" attribute that is a positive integer.' - try: - max_digits = int(f.max_digits) - if max_digits <= 0: - e.add(opts, mdigits_msg % f.name) - else: - mdigits_ok = True - except (ValueError, TypeError): - e.add(opts, mdigits_msg % f.name) - invalid_values_msg = '"%s": DecimalFields require a "max_digits" attribute value that is greater than or equal to the value of the "decimal_places" attribute.' - if decimalp_ok and mdigits_ok: - if decimal_places > max_digits: - e.add(opts, invalid_values_msg % f.name) - if isinstance(f, models.ImageField): - try: - from django.utils.image import Image # NOQA - except ImportError: - e.add(opts, '"%s": To use ImageFields, you need to install Pillow. Get it at https://pypi.python.org/pypi/Pillow.' % f.name) - if isinstance(f, models.BooleanField) and getattr(f, 'null', False): - e.add(opts, '"%s": BooleanFields do not accept null values. Use a NullBooleanField instead.' % f.name) - if isinstance(f, models.FilePathField) and not (f.allow_files or f.allow_folders): - e.add(opts, '"%s": FilePathFields must have either allow_files or allow_folders set to True.' % f.name) - if isinstance(f, models.GenericIPAddressField) and not getattr(f, 'null', False) and getattr(f, 'blank', False): - e.add(opts, '"%s": GenericIPAddressField can not accept blank values if null values are not allowed, as blank values are stored as null.' % f.name) - if f.choices: - if isinstance(f.choices, six.string_types) or not is_iterable(f.choices): - e.add(opts, '"%s": "choices" should be iterable (e.g., a tuple or list).' % f.name) - else: - for c in f.choices: - if isinstance(c, six.string_types) or not is_iterable(c) or len(c) != 2: - e.add(opts, '"%s": "choices" should be a sequence of two-item iterables (e.g. list of 2 item tuples).' % f.name) - if f.db_index not in (None, True, False): - e.add(opts, '"%s": "db_index" should be either None, True or False.' % f.name) - - # Perform any backend-specific field validation. - connection.validation.validate_field(e, opts, f) - - # Check if the on_delete behavior is sane - if f.rel and hasattr(f.rel, 'on_delete'): - if f.rel.on_delete == SET_NULL and not f.null: - e.add(opts, "'%s' specifies on_delete=SET_NULL, but cannot be null." % f.name) - elif f.rel.on_delete == SET_DEFAULT and not f.has_default(): - e.add(opts, "'%s' specifies on_delete=SET_DEFAULT, but has no default value." % f.name) - - # Check to see if the related field will clash with any existing - # fields, m2m fields, m2m related objects or related objects - if f.rel: - if f.rel.to not in apps.get_models(): - # If the related model is swapped, provide a hint; - # otherwise, the model just hasn't been installed. - if not isinstance(f.rel.to, six.string_types) and f.rel.to._meta.swapped: - e.add(opts, "'%s' defines a relation with the model '%s.%s', which has been swapped out. Update the relation to point at settings.%s." % (f.name, f.rel.to._meta.app_label, f.rel.to._meta.object_name, f.rel.to._meta.swappable)) - else: - e.add(opts, "'%s' has a relation with model %s, which has either not been installed or is abstract." % (f.name, f.rel.to)) - # it is a string and we could not find the model it refers to - # so skip the next section - if isinstance(f.rel.to, six.string_types): - continue - - # Make sure the related field specified by a ForeignKey is unique - if f.requires_unique_target: - if len(f.foreign_related_fields) > 1: - has_unique_field = False - for rel_field in f.foreign_related_fields: - has_unique_field = has_unique_field or rel_field.unique - if not has_unique_field: - e.add(opts, "Field combination '%s' under model '%s' must have a unique=True constraint" % (','.join(rel_field.name for rel_field in f.foreign_related_fields), f.rel.to.__name__)) - else: - if not f.foreign_related_fields[0].unique: - e.add(opts, "Field '%s' under model '%s' must have a unique=True constraint." % (f.foreign_related_fields[0].name, f.rel.to.__name__)) - - rel_opts = f.rel.to._meta - rel_name = f.related.get_accessor_name() - rel_query_name = f.related_query_name() - if not f.rel.is_hidden(): - for r in rel_opts.fields: - if r.name == rel_name: - e.add(opts, "Accessor for field '%s' clashes with field '%s.%s'. Add a related_name argument to the definition for '%s'." % (f.name, rel_opts.object_name, r.name, f.name)) - if r.name == rel_query_name: - e.add(opts, "Reverse query name for field '%s' clashes with field '%s.%s'. Add a related_name argument to the definition for '%s'." % (f.name, rel_opts.object_name, r.name, f.name)) - for r in rel_opts.local_many_to_many: - if r.name == rel_name: - e.add(opts, "Accessor for field '%s' clashes with m2m field '%s.%s'. Add a related_name argument to the definition for '%s'." % (f.name, rel_opts.object_name, r.name, f.name)) - if r.name == rel_query_name: - e.add(opts, "Reverse query name for field '%s' clashes with m2m field '%s.%s'. Add a related_name argument to the definition for '%s'." % (f.name, rel_opts.object_name, r.name, f.name)) - for r in rel_opts.get_all_related_many_to_many_objects(): - if r.get_accessor_name() == rel_name: - e.add(opts, "Accessor for field '%s' clashes with accessor for field '%s.%s'. Add a related_name argument to the definition for '%s'." % (f.name, r.model._meta.object_name, r.field.name, f.name)) - if r.get_accessor_name() == rel_query_name: - e.add(opts, "Reverse query name for field '%s' clashes with accessor for field '%s.%s'. Add a related_name argument to the definition for '%s'." % (f.name, r.model._meta.object_name, r.field.name, f.name)) - for r in rel_opts.get_all_related_objects(): - if r.field is not f: - if r.get_accessor_name() == rel_name: - e.add(opts, "Accessor for field '%s' clashes with accessor for field '%s.%s'. Add a related_name argument to the definition for '%s'." % (f.name, r.model._meta.object_name, r.field.name, f.name)) - if r.get_accessor_name() == rel_query_name: - e.add(opts, "Reverse query name for field '%s' clashes with accessor for field '%s.%s'. Add a related_name argument to the definition for '%s'." % (f.name, r.model._meta.object_name, r.field.name, f.name)) - - seen_intermediary_signatures = [] - for i, f in enumerate(opts.local_many_to_many): - # Check to see if the related m2m field will clash with any - # existing fields, m2m fields, m2m related objects or related - # objects - if f.rel.to not in apps.get_models(): - # If the related model is swapped, provide a hint; - # otherwise, the model just hasn't been installed. - if not isinstance(f.rel.to, six.string_types) and f.rel.to._meta.swapped: - e.add(opts, "'%s' defines a relation with the model '%s.%s', which has been swapped out. Update the relation to point at settings.%s." % (f.name, f.rel.to._meta.app_label, f.rel.to._meta.object_name, f.rel.to._meta.swappable)) - else: - e.add(opts, "'%s' has an m2m relation with model %s, which has either not been installed or is abstract." % (f.name, f.rel.to)) - - # it is a string and we could not find the model it refers to - # so skip the next section - if isinstance(f.rel.to, six.string_types): - continue - - # Check that the field is not set to unique. ManyToManyFields do not support unique. - if f.unique: - e.add(opts, "ManyToManyFields cannot be unique. Remove the unique argument on '%s'." % f.name) - - if f.rel.through is not None and not isinstance(f.rel.through, six.string_types): - from_model, to_model = cls, f.rel.to - if from_model == to_model and f.rel.symmetrical and not f.rel.through._meta.auto_created: - e.add(opts, "Many-to-many fields with intermediate tables cannot be symmetrical.") - seen_from, seen_to, seen_self = False, False, 0 - for inter_field in f.rel.through._meta.fields: - rel_to = getattr(inter_field.rel, 'to', None) - if from_model == to_model: # relation to self - if rel_to == from_model: - seen_self += 1 - if seen_self > 2: - e.add(opts, "Intermediary model %s has more than " - "two foreign keys to %s, which is ambiguous " - "and is not permitted." % ( - f.rel.through._meta.object_name, - from_model._meta.object_name - ) - ) - else: - if rel_to == from_model: - if seen_from: - e.add(opts, "Intermediary model %s has more " - "than one foreign key to %s, which is " - "ambiguous and is not permitted." % ( - f.rel.through._meta.object_name, - from_model._meta.object_name - ) - ) - else: - seen_from = True - elif rel_to == to_model: - if seen_to: - e.add(opts, "Intermediary model %s has more " - "than one foreign key to %s, which is " - "ambiguous and is not permitted." % ( - f.rel.through._meta.object_name, - rel_to._meta.object_name - ) - ) - else: - seen_to = True - if f.rel.through not in apps.get_models(include_auto_created=True): - e.add(opts, "'%s' specifies an m2m relation through model " - "%s, which has not been installed." % (f.name, f.rel.through)) - signature = (f.rel.to, cls, f.rel.through) - if signature in seen_intermediary_signatures: - e.add(opts, "The model %s has two manually-defined m2m " - "relations through the model %s, which is not " - "permitted. Please consider using an extra field on " - "your intermediary model instead." % ( - cls._meta.object_name, - f.rel.through._meta.object_name - ) - ) - else: - seen_intermediary_signatures.append(signature) - if not f.rel.through._meta.auto_created: - seen_related_fk, seen_this_fk = False, False - for field in f.rel.through._meta.fields: - if field.rel: - if not seen_related_fk and field.rel.to == f.rel.to: - seen_related_fk = True - elif field.rel.to == cls: - seen_this_fk = True - if not seen_related_fk or not seen_this_fk: - e.add(opts, "'%s' is a manually-defined m2m relation " - "through model %s, which does not have foreign keys " - "to %s and %s" % ( - f.name, f.rel.through._meta.object_name, - f.rel.to._meta.object_name, cls._meta.object_name - ) - ) - elif isinstance(f.rel.through, six.string_types): - e.add(opts, "'%s' specifies an m2m relation through model %s, " - "which has not been installed" % (f.name, f.rel.through)) - - rel_opts = f.rel.to._meta - rel_name = f.related.get_accessor_name() - rel_query_name = f.related_query_name() - # If rel_name is none, there is no reverse accessor (this only - # occurs for symmetrical m2m relations to self). If this is the - # case, there are no clashes to check for this field, as there are - # no reverse descriptors for this field. - if not f.rel.is_hidden(): - for r in rel_opts.fields: - if r.name == rel_name: - e.add(opts, "Accessor for m2m field '%s' clashes with field '%s.%s'. Add a related_name argument to the definition for '%s'." % (f.name, rel_opts.object_name, r.name, f.name)) - if r.name == rel_query_name: - e.add(opts, "Reverse query name for m2m field '%s' clashes with field '%s.%s'. Add a related_name argument to the definition for '%s'." % (f.name, rel_opts.object_name, r.name, f.name)) - for r in rel_opts.local_many_to_many: - if r.name == rel_name: - e.add(opts, "Accessor for m2m field '%s' clashes with m2m field '%s.%s'. Add a related_name argument to the definition for '%s'." % (f.name, rel_opts.object_name, r.name, f.name)) - if r.name == rel_query_name: - e.add(opts, "Reverse query name for m2m field '%s' clashes with m2m field '%s.%s'. Add a related_name argument to the definition for '%s'." % (f.name, rel_opts.object_name, r.name, f.name)) - for r in rel_opts.get_all_related_many_to_many_objects(): - if r.field is not f: - if r.get_accessor_name() == rel_name: - e.add(opts, "Accessor for m2m field '%s' clashes with accessor for m2m field '%s.%s'. Add a related_name argument to the definition for '%s'." % (f.name, r.model._meta.object_name, r.field.name, f.name)) - if r.get_accessor_name() == rel_query_name: - e.add(opts, "Reverse query name for m2m field '%s' clashes with accessor for m2m field '%s.%s'. Add a related_name argument to the definition for '%s'." % (f.name, r.model._meta.object_name, r.field.name, f.name)) - for r in rel_opts.get_all_related_objects(): - if r.get_accessor_name() == rel_name: - e.add(opts, "Accessor for m2m field '%s' clashes with accessor for field '%s.%s'. Add a related_name argument to the definition for '%s'." % (f.name, r.model._meta.object_name, r.field.name, f.name)) - if r.get_accessor_name() == rel_query_name: - e.add(opts, "Reverse query name for m2m field '%s' clashes with accessor for field '%s.%s'. Add a related_name argument to the definition for '%s'." % (f.name, r.model._meta.object_name, r.field.name, f.name)) - - # Check ordering attribute. - if opts.ordering: - for field_name in opts.ordering: - if field_name == '?': - continue - if field_name.startswith('-'): - field_name = field_name[1:] - if opts.order_with_respect_to and field_name == '_order': - continue - # Skip ordering in the format field1__field2 (FIXME: checking - # this format would be nice, but it's a little fiddly). - if '__' in field_name: - continue - # 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. - if field_name == 'pk': - continue - try: - opts.get_field(field_name, many_to_many=False) - except models.FieldDoesNotExist: - e.add(opts, '"ordering" refers to "%s", a field that doesn\'t exist.' % field_name) - - # Check unique_together. - for ut in opts.unique_together: - validate_local_fields(e, opts, "unique_together", ut) - if not isinstance(opts.index_together, collections.Sequence): - e.add(opts, '"index_together" must a sequence') - else: - for it in opts.index_together: - validate_local_fields(e, opts, "index_together", it) - - validate_model_signals(e) - - return len(e.errors) - - -def validate_local_fields(e, opts, field_name, fields): - from django.db import models - - if not isinstance(fields, collections.Sequence): - e.add(opts, 'all %s elements must be sequences' % field_name) - else: - for field in fields: - try: - f = opts.get_field(field, many_to_many=True) - except models.FieldDoesNotExist: - e.add(opts, '"%s" refers to %s, a field that doesn\'t exist.' % (field_name, field)) - else: - if isinstance(f.rel, models.ManyToManyRel): - e.add(opts, '"%s" refers to %s. ManyToManyFields are not supported in %s.' % (field_name, f.name, field_name)) - if f not in opts.local_fields: - e.add(opts, '"%s" refers to %s. This is not in the same model as the %s statement.' % (field_name, f.name, field_name)) - - -def validate_model_signals(e): - """Ensure lazily referenced model signals senders are installed.""" - from django.db import models - - for name in dir(models.signals): - obj = getattr(models.signals, name) - if isinstance(obj, models.signals.ModelSignal): - for reference, receivers in obj.unresolved_references.items(): - for receiver, _, _ in receivers: - # The receiver is either a function or an instance of class - # defining a `__call__` method. - if isinstance(receiver, types.FunctionType): - description = "The `%s` function" % receiver.__name__ - else: - description = "An instance of the `%s` class" % receiver.__class__.__name__ - e.add( - receiver.__module__, - "%s was connected to the `%s` signal " - "with a lazy reference to the '%s' sender, " - "which has not been installed." % ( - description, name, '.'.join(reference) - ) - ) diff --git a/django/db/backends/__init__.py b/django/db/backends/__init__.py index 99d68221e8..b96407056a 100644 --- a/django/db/backends/__init__.py +++ b/django/db/backends/__init__.py @@ -10,6 +10,7 @@ from contextlib import contextmanager from importlib import import_module from django.conf import settings +from django.core import checks from django.db import DEFAULT_DB_ALIAS from django.db.backends.signals import connection_created from django.db.backends import utils @@ -1400,5 +1401,30 @@ class BaseDatabaseValidation(object): self.connection = connection def validate_field(self, errors, opts, f): - "By default, there is no backend-specific validation" + """ + By default, there is no backend-specific validation. + + This method has been deprecated by the new checks framework. New + backends should implement check_field instead. + """ + # This is deliberately commented out. It exists as a marker to + # remind us to remove this method, and the check_field() shim, + # when the time comes. + # warnings.warn('"validate_field" has been deprecated", PendingDeprecationWarning) pass + + def check_field(self, field, **kwargs): + class ErrorList(list): + """A dummy list class that emulates API used by the older + validate_field() method. When validate_field() is fully + deprecated, this dummy can be removed too. + """ + def add(self, opts, error_message): + self.append(checks.Error(error_message, hint=None, obj=field)) + + errors = ErrorList() + # Some tests create fields in isolation -- the fields are not attached + # to any model, so they have no `model` attribute. + opts = field.model._meta if hasattr(field, 'model') else None + self.validate_field(errors, field, opts) + return list(errors) diff --git a/django/db/backends/mysql/validation.py b/django/db/backends/mysql/validation.py index 17b7cde756..cc8a7ac75b 100644 --- a/django/db/backends/mysql/validation.py +++ b/django/db/backends/mysql/validation.py @@ -1,17 +1,34 @@ +from django.core import checks from django.db.backends import BaseDatabaseValidation class DatabaseValidation(BaseDatabaseValidation): - def validate_field(self, errors, opts, f): + def check_field(self, field, **kwargs): """ MySQL has the following field length restriction: No character (varchar) fields can have a length exceeding 255 characters if they have a unique index on them. """ - from django.db import models - varchar_fields = (models.CharField, models.CommaSeparatedIntegerField, - models.SlugField) - if (isinstance(f, varchar_fields) and f.unique - and (f.max_length is None or int(f.max_length) > 255)): - msg = '"%(name)s": %(cls)s cannot have a "max_length" greater than 255 when using "unique=True".' - errors.add(opts, msg % {'name': f.name, 'cls': f.__class__.__name__}) + from django.db import connection + + errors = super(DatabaseValidation, self).check_field(field, **kwargs) + try: + field_type = field.db_type(connection) + except AttributeError: + # If the field is a relative field and the target model is + # missing, then field.rel.to is not a model and doesn't have + # `_meta` attribute. + field_type = '' + + if (field_type.startswith('varchar') and field.unique + and (field.max_length is None or int(field.max_length) > 255)): + errors.append( + checks.Error( + ('Under mysql backend, the field cannot have a "max_length" ' + 'greated than 255 when it is unique.'), + hint=None, + obj=field, + id='E047', + ) + ) + return errors diff --git a/django/db/models/base.py b/django/db/models/base.py index c9187c10dc..9e835d0e16 100644 --- a/django/db/models/base.py +++ b/django/db/models/base.py @@ -9,6 +9,7 @@ from django.apps import apps from django.apps.base import MODELS_MODULE_NAME import django.db.models.manager # NOQA: Imported to register signal handler. from django.conf import settings +from django.core import checks from django.core.exceptions import (ObjectDoesNotExist, MultipleObjectsReturned, FieldError, ValidationError, NON_FIELD_ERRORS) from django.db.models.fields import AutoField, FieldDoesNotExist @@ -1030,6 +1031,308 @@ class Model(six.with_metaclass(ModelBase)): if errors: raise ValidationError(errors) + @classmethod + def check(cls, **kwargs): + errors = [] + errors.extend(cls._check_swappable()) + errors.extend(cls._check_managers(**kwargs)) + if not cls._meta.swapped: + errors.extend(cls._check_fields(**kwargs)) + errors.extend(cls._check_m2m_through_same_relationship()) + errors.extend(cls._check_id_field()) + errors.extend(cls._check_column_name_clashes()) + errors.extend(cls._check_index_together()) + errors.extend(cls._check_unique_together()) + errors.extend(cls._check_ordering()) + + return errors + + @classmethod + def _check_swappable(cls): + """ Check if the swapped model exists. """ + + errors = [] + if cls._meta.swapped: + try: + app_label, model_name = cls._meta.swapped.split('.') + except ValueError: + errors.append( + checks.Error( + '"%s" is not of the form "app_label.app_name".' % cls._meta.swappable, + hint=None, + obj=cls, + id='E002', + ) + ) + else: + try: + apps.get_model(app_label, model_name) + except LookupError: + errors.append( + checks.Error( + ('The model has been swapped out for %s.%s ' + 'which has not been installed or is abstract.') % ( + app_label, model_name + ), + hint=('Ensure that you did not misspell the model ' + 'name and the app name as well as the model ' + 'is not abstract. Does your INSTALLED_APPS ' + 'setting contain the "%s" app?') % app_label, + obj=cls, + id='E003', + ) + ) + return errors + + @classmethod + def _check_managers(cls, **kwargs): + """ Perform all manager checks. """ + + errors = [] + managers = cls._meta.concrete_managers + cls._meta.abstract_managers + for (_, _, manager) in managers: + errors.extend(manager.check(**kwargs)) + return errors + + @classmethod + def _check_fields(cls, **kwargs): + """ Perform all field checks. """ + + errors = [] + for field in cls._meta.local_fields: + errors.extend(field.check(**kwargs)) + for field in cls._meta.local_many_to_many: + errors.extend(field.check(from_model=cls, **kwargs)) + return errors + + @classmethod + def _check_m2m_through_same_relationship(cls): + """ Check if no relationship model is used by more than one m2m field. + """ + + errors = [] + seen_intermediary_signatures = [] + + fields = cls._meta.local_many_to_many + + # Skip when the target model wasn't found. + fields = (f for f in fields if isinstance(f.rel.to, ModelBase)) + + # Skip when the relationship model wasn't found. + fields = (f for f in fields if isinstance(f.rel.through, ModelBase)) + + for f in fields: + signature = (f.rel.to, cls, f.rel.through) + if signature in seen_intermediary_signatures: + errors.append( + checks.Error( + ('The model has two many-to-many relations through ' + 'the intermediary %s model, which is not permitted.') % ( + f.rel.through._meta.object_name + ), + hint=None, + obj=cls, + id='E004', + ) + ) + else: + seen_intermediary_signatures.append(signature) + return errors + + @classmethod + def _check_id_field(cls): + """ Check if `id` field is a primary key. """ + + fields = list(f for f in cls._meta.local_fields + if f.name == 'id' and f != cls._meta.pk) + # fields is empty or consists of the invalid "id" field + if fields and not fields[0].primary_key and cls._meta.pk.name == 'id': + return [ + checks.Error( + ('You cannot use "id" as a field name, because each model ' + 'automatically gets an "id" field if none ' + 'of the fields have primary_key=True.'), + hint=('Remove or rename "id" field ' + 'or add primary_key=True to a field.'), + obj=cls, + id='E005', + ) + ] + else: + return [] + + @classmethod + def _check_column_name_clashes(cls): + # Store a list of column names which have already been used by other fields. + used_column_names = [] + errors = [] + + for f in cls._meta.local_fields: + _, column_name = f.get_attname_column() + + # Ensure the column name is not already in use. + if column_name and column_name in used_column_names: + errors.append( + checks.Error( + 'Field "%s" has column name "%s" that is already used.' % (f.name, column_name), + hint=None, + obj=cls, + ) + ) + else: + used_column_names.append(column_name) + + return errors + + @classmethod + def _check_index_together(cls): + """ Check the value of "index_together" option. """ + if not isinstance(cls._meta.index_together, (tuple, list)): + return [ + checks.Error( + '"index_together" must be a list or tuple.', + hint=None, + obj=cls, + id='E006', + ) + ] + + elif any(not isinstance(fields, (tuple, list)) + for fields in cls._meta.index_together): + return [ + checks.Error( + 'All "index_together" elements must be lists or tuples.', + hint=None, + obj=cls, + id='E007', + ) + ] + + else: + errors = [] + for fields in cls._meta.index_together: + errors.extend(cls._check_local_fields(fields, "index_together")) + return errors + + @classmethod + def _check_unique_together(cls): + """ Check the value of "unique_together" option. """ + if not isinstance(cls._meta.unique_together, (tuple, list)): + return [ + checks.Error( + '"unique_together" must be a list or tuple.', + hint=None, + obj=cls, + id='E008', + ) + ] + + elif any(not isinstance(fields, (tuple, list)) + for fields in cls._meta.unique_together): + return [ + checks.Error( + 'All "unique_together" elements must be lists or tuples.', + hint=None, + obj=cls, + id='E009', + ) + ] + + else: + errors = [] + for fields in cls._meta.unique_together: + errors.extend(cls._check_local_fields(fields, "unique_together")) + return errors + + @classmethod + def _check_local_fields(cls, fields, option): + from django.db import models + + errors = [] + for field_name in fields: + try: + field = cls._meta.get_field(field_name, + many_to_many=True) + except models.FieldDoesNotExist: + errors.append( + checks.Error( + '"%s" points to a missing field named "%s".' % (option, field_name), + hint='Ensure that you did not misspell the field name.', + obj=cls, + id='E010', + ) + ) + else: + if isinstance(field.rel, models.ManyToManyRel): + errors.append( + checks.Error( + ('"%s" refers to a m2m "%s" field, but ' + 'ManyToManyFields are not supported in "%s".') % ( + option, field_name, option + ), + hint=None, + obj=cls, + id='E011', + ) + ) + return errors + + @classmethod + def _check_ordering(cls): + """ Check "ordering" option -- is it a list of lists and do all fields + exist? """ + + from django.db.models import FieldDoesNotExist + + if not cls._meta.ordering: + return [] + + if not isinstance(cls._meta.ordering, (list, tuple)): + return [ + checks.Error( + ('"ordering" must be a tuple or list ' + '(even if you want to order by only one field).'), + hint=None, + obj=cls, + id='E012', + ) + ] + + errors = [] + + fields = cls._meta.ordering + + # Skip '?' fields. + fields = (f for f in fields if f != '?') + + # Convert "-field" to "field". + fields = ((f[1:] if f.startswith('-') else f) for f in fields) + + fields = (f for f in fields if + f != '_order' or not cls._meta.order_with_respect_to) + + # Skip ordering in the format field1__field2 (FIXME: checking + # this format would be nice, but it's a little fiddly). + fields = (f for f in fields if '__' not in f) + + # 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. + fields = (f for f in fields if f != 'pk') + + for field_name in fields: + try: + cls._meta.get_field(field_name, many_to_many=False) + except FieldDoesNotExist: + errors.append( + checks.Error( + '"ordering" pointing to a missing "%s" field.' % field_name, + hint='Ensure that you did not misspell the field name.', + obj=cls, + id='E013', + ) + ) + return errors + ############################################ # HELPER FUNCTIONS (CURRIED MODEL METHODS) # diff --git a/django/db/models/fields/__init__.py b/django/db/models/fields/__init__.py index 7ace8878aa..e61d079f0d 100644 --- a/django/db/models/fields/__init__.py +++ b/django/db/models/fields/__init__.py @@ -1,3 +1,4 @@ +# -*- coding: utf-8 -*- from __future__ import unicode_literals import collections @@ -15,16 +16,18 @@ from django.db.models.lookups import default_lookups, RegisterLookupMixin from django.db.models.query_utils import QueryWrapper from django.conf import settings from django import forms -from django.core import exceptions, validators +from django.core import exceptions, validators, checks from django.utils.datastructures import DictWrapper from django.utils.dateparse import parse_date, parse_datetime, parse_time from django.utils.functional import curry, total_ordering, Promise from django.utils.text import capfirst from django.utils import timezone from django.utils.translation import ugettext_lazy as _ -from django.utils.encoding import smart_text, force_text, force_bytes +from django.utils.encoding import (smart_text, force_text, force_bytes, + python_2_unicode_compatible) from django.utils.ipv6 import clean_ipv6_address from django.utils import six +from django.utils.itercompat import is_iterable # Avoid "TypeError: Item in ``from list'' not a string" -- unicode_literals # makes these strings unicode @@ -81,6 +84,7 @@ def _empty(of_cls): @total_ordering +@python_2_unicode_compatible class Field(RegisterLookupMixin): """Base class for all field types""" @@ -159,6 +163,111 @@ class Field(RegisterLookupMixin): self._error_messages = error_messages # Store for deconstruction later self.error_messages = messages + def __str__(self): + """ Return "app_label.model_label.field_name". """ + model = self.model + app = model._meta.app_label + return '%s.%s.%s' % (app, model._meta.object_name, self.name) + + def __repr__(self): + """ + Displays the module, class and name of the field. + """ + path = '%s.%s' % (self.__class__.__module__, self.__class__.__name__) + name = getattr(self, 'name', None) + if name is not None: + return '<%s: %s>' % (path, name) + return '<%s>' % path + + def check(self, **kwargs): + errors = [] + errors.extend(self._check_field_name()) + errors.extend(self._check_choices()) + errors.extend(self._check_db_index()) + errors.extend(self._check_null_allowed_for_primary_keys()) + errors.extend(self._check_backend_specific_checks(**kwargs)) + return errors + + def _check_field_name(self): + """ Check if field name is valid (i. e. not ending with an underscore). + """ + if self.name.endswith('_'): + return [ + checks.Error( + 'Field names must not end with underscores.', + hint=None, + obj=self, + id='E001', + ) + ] + else: + return [] + + def _check_choices(self): + if self.choices: + if (isinstance(self.choices, six.string_types) or + not is_iterable(self.choices)): + return [ + checks.Error( + '"choices" must be an iterable (e.g., a list or tuple).', + hint=None, + obj=self, + id='E033', + ) + ] + elif any(isinstance(choice, six.string_types) or + not is_iterable(choice) or len(choice) != 2 + for choice in self.choices): + return [ + checks.Error( + ('All "choices" elements must be a tuple of two ' + 'elements (the first one is the actual value ' + 'to be stored and the second element is ' + 'the human-readable name).'), + hint=None, + obj=self, + id='E034', + ) + ] + else: + return [] + else: + return [] + + def _check_db_index(self): + if self.db_index not in (None, True, False): + return [ + checks.Error( + '"db_index" must be either None, True or False.', + hint=None, + obj=self, + id='E035', + ) + ] + else: + return [] + + def _check_null_allowed_for_primary_keys(self): + if (self.primary_key and self.null and + not connection.features.interprets_empty_strings_as_nulls): + # We cannot reliably check this for backends like Oracle which + # consider NULL and '' to be equal (and thus set up + # character-based fields a little differently). + return [ + checks.Error( + 'Primary keys must not have null=True.', + hint=('Set null=False on the field or ' + 'remove primary_key=True argument.'), + obj=self, + id='E036', + ) + ] + else: + return [] + + def _check_backend_specific_checks(self, **kwargs): + return connection.validation.check_field(self, **kwargs) + def deconstruct(self): """ Returns enough information to recreate the field as a 4-tuple: @@ -708,16 +817,6 @@ class Field(RegisterLookupMixin): """ return getattr(obj, self.attname) - def __repr__(self): - """ - Displays the module, class and name of the field. - """ - path = '%s.%s' % (self.__class__.__module__, self.__class__.__name__) - name = getattr(self, 'name', None) - if name is not None: - return '<%s: %s>' % (path, name) - return '<%s>' % path - class AutoField(Field): description = _("Integer") @@ -728,11 +827,27 @@ class AutoField(Field): } def __init__(self, *args, **kwargs): - assert kwargs.get('primary_key', False) is True, \ - "%ss must have primary_key=True." % self.__class__.__name__ kwargs['blank'] = True super(AutoField, self).__init__(*args, **kwargs) + def check(self, **kwargs): + errors = super(AutoField, self).check(**kwargs) + errors.extend(self._check_primary_key()) + return errors + + def _check_primary_key(self): + if not self.primary_key: + return [ + checks.Error( + 'The field must have primary_key=True, because it is an AutoField.', + hint=None, + obj=self, + id='E048', + ), + ] + else: + return [] + def deconstruct(self): name, path, args, kwargs = super(AutoField, self).deconstruct() del kwargs['blank'] @@ -791,6 +906,24 @@ class BooleanField(Field): kwargs['blank'] = True super(BooleanField, self).__init__(*args, **kwargs) + def check(self, **kwargs): + errors = super(BooleanField, self).check(**kwargs) + errors.extend(self._check_null(**kwargs)) + return errors + + def _check_null(self, **kwargs): + if getattr(self, 'null', False): + return [ + checks.Error( + 'BooleanFields do not acceps null values.', + hint='Use a NullBooleanField instead.', + obj=self, + id='E037', + ) + ] + else: + return [] + def deconstruct(self): name, path, args, kwargs = super(BooleanField, self).deconstruct() del kwargs['blank'] @@ -849,6 +982,37 @@ class CharField(Field): super(CharField, self).__init__(*args, **kwargs) self.validators.append(validators.MaxLengthValidator(self.max_length)) + def check(self, **kwargs): + errors = super(CharField, self).check(**kwargs) + errors.extend(self._check_max_length_attibute(**kwargs)) + return errors + + def _check_max_length_attibute(self, **kwargs): + try: + max_length = int(self.max_length) + if max_length <= 0: + raise ValueError() + except TypeError: + return [ + checks.Error( + 'The field must have "max_length" attribute.', + hint=None, + obj=self, + id='E038', + ) + ] + except ValueError: + return [ + checks.Error( + '"max_length" must be a positive integer.', + hint=None, + obj=self, + id='E039', + ) + ] + else: + return [] + def get_internal_type(self): return "CharField" @@ -1114,6 +1278,77 @@ class DecimalField(Field): self.max_digits, self.decimal_places = max_digits, decimal_places super(DecimalField, self).__init__(verbose_name, name, **kwargs) + def check(self, **kwargs): + errors = super(DecimalField, self).check(**kwargs) + errors.extend(self._check_decimal_places_and_max_digits(**kwargs)) + return errors + + def _check_decimal_places_and_max_digits(self, **kwargs): + errors = self.__check_decimal_places() + errors += self.__check_max_digits() + if not errors and int(self.decimal_places) > int(self.max_digits): + errors.append( + checks.Error( + '"max_digits" must be greater or equal to "decimal_places".', + hint=None, + obj=self, + id='E040', + ) + ) + return errors + + def __check_decimal_places(self): + try: + decimal_places = int(self.decimal_places) + if decimal_places < 0: + raise ValueError() + except TypeError: + return [ + checks.Error( + 'The field requires a "decimal_places" attribute.', + hint=None, + obj=self, + id='E041', + ) + ] + except ValueError: + return [ + checks.Error( + '"decimal_places" attribute must be a non-negative integer.', + hint=None, + obj=self, + id='E042', + ) + ] + else: + return [] + + def __check_max_digits(self): + try: + max_digits = int(self.max_digits) + if max_digits <= 0: + raise ValueError() + except TypeError: + return [ + checks.Error( + 'The field requires a "max_digits" attribute.', + hint=None, + obj=self, + id='E043', + ) + ] + except ValueError: + return [ + checks.Error( + '"max_digits" attribute must be a positive integer.', + hint=None, + obj=self, + id='E044', + ) + ] + else: + return [] + def deconstruct(self): name, path, args, kwargs = super(DecimalField, self).deconstruct() if self.max_digits: @@ -1212,6 +1447,23 @@ class FilePathField(Field): kwargs['max_length'] = kwargs.get('max_length', 100) super(FilePathField, self).__init__(verbose_name, name, **kwargs) + def check(self, **kwargs): + errors = super(FilePathField, self).check(**kwargs) + errors.extend(self._check_allowing_files_or_folders(**kwargs)) + return errors + + def _check_allowing_files_or_folders(self, **kwargs): + if not self.allow_files and not self.allow_folders: + return [ + checks.Error( + 'The field must have either "allow_files" or "allow_folders" set to True.', + hint=None, + obj=self, + id='E045', + ) + ] + return [] + def deconstruct(self): name, path, args, kwargs = super(FilePathField, self).deconstruct() if self.path != '': @@ -1373,6 +1625,24 @@ class GenericIPAddressField(Field): super(GenericIPAddressField, self).__init__(verbose_name, name, *args, **kwargs) + def check(self, **kwargs): + errors = super(GenericIPAddressField, self).check(**kwargs) + errors.extend(self._check_blank_and_null_values(**kwargs)) + return errors + + def _check_blank_and_null_values(self, **kwargs): + if not getattr(self, 'null', False) and getattr(self, 'blank', False): + return [ + checks.Error( + ('The field cannot accept blank values if null values ' + 'are not allowed, as blank values are stored as null.'), + hint=None, + obj=self, + id='E046', + ) + ] + return [] + def deconstruct(self): name, path, args, kwargs = super(GenericIPAddressField, self).deconstruct() if self.unpack_ipv4 is not False: diff --git a/django/db/models/fields/files.py b/django/db/models/fields/files.py index 08ac4413a0..cee54c8783 100644 --- a/django/db/models/fields/files.py +++ b/django/db/models/fields/files.py @@ -3,6 +3,8 @@ import os from django import forms from django.db.models.fields import Field +from django.core import checks +from django.core.exceptions import ImproperlyConfigured from django.core.files.base import File from django.core.files.storage import default_storage from django.core.files.images import ImageFile @@ -223,9 +225,8 @@ class FileField(Field): description = _("File") def __init__(self, verbose_name=None, name=None, upload_to='', storage=None, **kwargs): - for arg in ('primary_key', 'unique'): - if arg in kwargs: - raise TypeError("'%s' is not a valid argument for %s." % (arg, self.__class__)) + self._primary_key_set_explicitly = 'primary_key' in kwargs + self._unique_set_explicitly = 'unique' in kwargs self.storage = storage or default_storage self.upload_to = upload_to @@ -235,6 +236,52 @@ class FileField(Field): kwargs['max_length'] = kwargs.get('max_length', 100) super(FileField, self).__init__(verbose_name, name, **kwargs) + def check(self, **kwargs): + errors = super(FileField, self).check(**kwargs) + #errors.extend(self._check_upload_to()) + errors.extend(self._check_unique()) + errors.extend(self._check_primary_key()) + return errors + + def _check_upload_to(self): + if not self.upload_to: + return [ + checks.Error( + 'The field requires an "upload_to" attribute.', + hint=None, + obj=self, + id='E031', + ) + ] + else: + return [] + + def _check_unique(self): + if self._unique_set_explicitly: + return [ + checks.Error( + '"unique" is not a valid argument for %s.' % self.__class__.__name__, + hint=None, + obj=self, + id='E049', + ) + ] + else: + return [] + + def _check_primary_key(self): + if self._primary_key_set_explicitly: + return [ + checks.Error( + '"primary_key" is not a valid argument for %s.' % self.__class__.__name__, + hint=None, + obj=self, + id='E050', + ) + ] + else: + return [] + def deconstruct(self): name, path, args, kwargs = super(FileField, self).deconstruct() if kwargs.get("max_length", None) != 100: @@ -348,6 +395,27 @@ class ImageField(FileField): self.width_field, self.height_field = width_field, height_field super(ImageField, self).__init__(verbose_name, name, **kwargs) + def check(self, **kwargs): + errors = super(ImageField, self).check(**kwargs) + errors.extend(self._check_image_library_installed()) + return errors + + def _check_image_library_installed(self): + try: + from django.utils.image import Image # NOQA + except ImproperlyConfigured: + return [ + checks.Error( + 'To use ImageFields, Pillow must be installed.', + hint=('Get Pillow at https://pypi.python.org/pypi/Pillow ' + 'or run command "pip install pillow".'), + obj=self, + id='E032', + ) + ] + else: + return [] + def deconstruct(self): name, path, args, kwargs = super(ImageField, self).deconstruct() if self.width_field: diff --git a/django/db/models/fields/related.py b/django/db/models/fields/related.py index 56a74da67e..570547cdcc 100644 --- a/django/db/models/fields/related.py +++ b/django/db/models/fields/related.py @@ -1,14 +1,16 @@ from operator import attrgetter +from django.apps import apps +from django.core import checks from django.db import connection, connections, router, transaction from django.db.backends import utils from django.db.models import signals, Q +from django.db.models.deletion import SET_NULL, SET_DEFAULT, CASCADE from django.db.models.fields import (AutoField, Field, IntegerField, PositiveIntegerField, PositiveSmallIntegerField, FieldDoesNotExist) from django.db.models.lookups import IsNull from django.db.models.related import RelatedObject, PathInfo from django.db.models.query import QuerySet -from django.db.models.deletion import CASCADE from django.db.models.sql.datastructures import Col from django.utils.encoding import smart_text from django.utils import six @@ -93,6 +95,156 @@ signals.class_prepared.connect(do_pending_lookups) class RelatedField(Field): + def check(self, **kwargs): + errors = super(RelatedField, self).check(**kwargs) + errors.extend(self._check_relation_model_exists()) + errors.extend(self._check_referencing_to_swapped_model()) + errors.extend(self._check_clashes()) + return errors + + def _check_relation_model_exists(self): + rel_is_missing = self.rel.to not in apps.get_models() + rel_is_string = isinstance(self.rel.to, six.string_types) + model_name = self.rel.to if rel_is_string else self.rel.to._meta.object_name + if rel_is_missing and (rel_is_string or not self.rel.to._meta.swapped): + return [ + checks.Error( + ('The field has a relation with model %s, which ' + 'has either not been installed or is abstract.') % model_name, + hint=('Ensure that you did not misspell the model name and ' + 'the model is not abstract. Does your INSTALLED_APPS ' + 'setting contain the app where %s is defined?') % model_name, + obj=self, + id='E030', + ) + ] + return [] + + def _check_referencing_to_swapped_model(self): + if (self.rel.to not in apps.get_models() and + not isinstance(self.rel.to, six.string_types) and + self.rel.to._meta.swapped): + model = "%s.%s" % ( + self.rel.to._meta.app_label, + self.rel.to._meta.object_name + ) + return [ + checks.Error( + ('The field defines a relation with the model %s, ' + 'which has been swapped out.') % model, + hint='Update the relation to point at settings.%s' % self.rel.to._meta.swappable, + obj=self, + id='E029', + ) + ] + return [] + + def _check_clashes(self): + """ Check accessor and reverse query name clashes. """ + + from django.db.models.base import ModelBase + + errors = [] + opts = self.model._meta + + # `f.rel.to` may be a string instead of a model. Skip if model name is + # not resolved. + if not isinstance(self.rel.to, ModelBase): + return [] + + # If the field doesn't install backward relation on the target model (so + # `is_hidden` returns True), then there are no clashes to check and we + # can skip these fields. + if self.rel.is_hidden(): + return [] + + try: + self.related + except AttributeError: + return [] + + # Consider that we are checking field `Model.foreign` and the models + # are: + # + # class Target(models.Model): + # model = models.IntegerField() + # model_set = models.IntegerField() + # + # class Model(models.Model): + # foreign = models.ForeignKey(Target) + # m2m = models.ManyToManyField(Target) + + rel_opts = self.rel.to._meta + # rel_opts.object_name == "Target" + rel_name = self.related.get_accessor_name() # i. e. "model_set" + rel_query_name = self.related_query_name() # i. e. "model" + field_name = "%s.%s" % (opts.object_name, + self.name) # i. e. "Model.field" + + # Check clashes between accessor or reverse query name of `field` + # and any other field name -- i. e. accessor for Model.foreign is + # model_set and it clashes with Target.model_set. + potential_clashes = rel_opts.fields + rel_opts.local_many_to_many + for clash_field in potential_clashes: + clash_name = "%s.%s" % (rel_opts.object_name, + clash_field.name) # i. e. "Target.model_set" + if clash_field.name == rel_name: + errors.append( + checks.Error( + 'Accessor for field %s clashes with field %s.' % (field_name, clash_name), + hint=('Rename field %s or add/change a related_name ' + 'argument to the definition for field %s.') % (clash_name, field_name), + obj=self, + id='E014', + ) + ) + + if clash_field.name == rel_query_name: + errors.append( + checks.Error( + 'Reverse query name for field %s clashes with field %s.' % (field_name, clash_name), + hint=('Rename field %s or add/change a related_name ' + 'argument to the definition for field %s.') % (clash_name, field_name), + obj=self, + id='E015', + ) + ) + + # Check clashes between accessors/reverse query names of `field` and + # any other field accessor -- i. e. Model.foreign accessor clashes with + # Model.m2m accessor. + potential_clashes = rel_opts.get_all_related_many_to_many_objects() + potential_clashes += rel_opts.get_all_related_objects() + potential_clashes = (r for r in potential_clashes + if r.field is not self) + for clash_field in potential_clashes: + clash_name = "%s.%s" % ( # i. e. "Model.m2m" + clash_field.model._meta.object_name, + clash_field.field.name) + if clash_field.get_accessor_name() == rel_name: + errors.append( + checks.Error( + 'Clash between accessors for %s and %s.' % (field_name, clash_name), + hint=('Add or change a related_name argument ' + 'to the definition for %s or %s.') % (field_name, clash_name), + obj=self, + id='E016', + ) + ) + + if clash_field.get_accessor_name() == rel_query_name: + errors.append( + checks.Error( + 'Clash between reverse query names for %s and %s.' % (field_name, clash_name), + hint=('Add or change a related_name argument ' + 'to the definition for %s or %s.') % (field_name, clash_name), + obj=self, + id='E017', + ) + ) + + return errors + def db_type(self, connection): '''By default related field will not have a column as it relates columns to another table''' @@ -1104,6 +1256,58 @@ class ForeignObject(RelatedField): super(ForeignObject, self).__init__(**kwargs) + def check(self, **kwargs): + errors = super(ForeignObject, self).check(**kwargs) + errors.extend(self._check_unique_target()) + return errors + + def _check_unique_target(self): + rel_is_string = isinstance(self.rel.to, six.string_types) + if rel_is_string or not self.requires_unique_target: + return [] + + # Skip if the + try: + self.foreign_related_fields + except FieldDoesNotExist: + return [] + + try: + self.related + except AttributeError: + return [] + + has_unique_field = any(rel_field.unique + for rel_field in self.foreign_related_fields) + if not has_unique_field and len(self.foreign_related_fields) > 1: + field_combination = ','.join(rel_field.name + for rel_field in self.foreign_related_fields) + model_name = self.rel.to.__name__ + return [ + checks.Error( + ('No unique=True constraint ' + 'on field combination "%s" under model %s.') % (field_combination, model_name), + hint=('Set unique=True argument on any of the fields ' + '"%s" under model %s.') % (field_combination, model_name), + obj=self, + id='E018', + ) + ] + elif not has_unique_field: + field_name = self.foreign_related_fields[0].name + model_name = self.rel.to.__name__ + return [ + checks.Error( + ('%s.%s must have unique=True ' + 'because it is referenced by a foreign key.') % (model_name, field_name), + hint=None, + obj=self, + id='E019', + ) + ] + else: + return [] + def deconstruct(self): name, path, args, kwargs = super(ForeignObject, self).deconstruct() kwargs['from_fields'] = self.from_fields @@ -1331,7 +1535,6 @@ class ForeignKey(ForeignObject): except AttributeError: # to._meta doesn't exist, so it must be RECURSIVE_RELATIONSHIP_CONSTANT assert isinstance(to, six.string_types), "%s(%r) is invalid. First parameter to ForeignKey must be either a model, a model name, or the string %r" % (self.__class__.__name__, to, RECURSIVE_RELATIONSHIP_CONSTANT) else: - assert not to._meta.abstract, "%s cannot define a relation with abstract class %s" % (self.__class__.__name__, to._meta.object_name) # For backwards compatibility purposes, we need to *try* and set # the to_field during FK construction. It won't be guaranteed to # be correct until contribute_to_class is called. Refs #12190. @@ -1352,6 +1555,34 @@ class ForeignKey(ForeignObject): ) super(ForeignKey, self).__init__(to, ['self'], [to_field], **kwargs) + def check(self, **kwargs): + errors = super(ForeignKey, self).check(**kwargs) + errors.extend(self._check_on_delete()) + return errors + + def _check_on_delete(self): + on_delete = getattr(self.rel, 'on_delete', None) + if on_delete == SET_NULL and not self.null: + return [ + checks.Error( + 'The field specifies on_delete=SET_NULL, but cannot be null.', + hint='Set null=True argument on the field.', + obj=self, + id='E020', + ) + ] + elif on_delete == SET_DEFAULT and not self.has_default(): + return [ + checks.Error( + 'The field specifies on_delete=SET_DEFAULT, but has no default value.', + hint=None, + obj=self, + id='E021', + ) + ] + else: + return [] + def deconstruct(self): name, path, args, kwargs = super(ForeignKey, self).deconstruct() del kwargs['to_fields'] @@ -1559,7 +1790,7 @@ class ManyToManyField(RelatedField): def __init__(self, to, db_constraint=True, swappable=True, **kwargs): try: - assert not to._meta.abstract, "%s cannot define a relation with abstract class %s" % (self.__class__.__name__, to._meta.object_name) + to._meta except AttributeError: # to._meta doesn't exist, so it must be RECURSIVE_RELATIONSHIP_CONSTANT assert isinstance(to, six.string_types), "%s(%r) is invalid. First parameter to ManyToManyField must be either a model, a model name, or the string %r" % (self.__class__.__name__, to, RECURSIVE_RELATIONSHIP_CONSTANT) # Class names must be ASCII in Python 2.x, so we forcibly coerce it here to break early if there's a problem. @@ -1582,6 +1813,136 @@ class ManyToManyField(RelatedField): super(ManyToManyField, self).__init__(**kwargs) + def check(self, **kwargs): + errors = super(ManyToManyField, self).check(**kwargs) + errors.extend(self._check_unique(**kwargs)) + errors.extend(self._check_relationship_model(**kwargs)) + return errors + + def _check_unique(self, **kwargs): + if self.unique: + return [ + checks.Error( + 'ManyToManyFields must not be unique.', + hint=None, + obj=self, + id='E022', + ) + ] + return [] + + def _check_relationship_model(self, from_model=None, **kwargs): + errors = [] + + if self.rel.through not in apps.get_models(include_auto_created=True): + # The relationship model is not installed. + errors.append( + checks.Error( + ('The field specifies a many-to-many relation through model ' + '%s, which has not been installed.') % self.rel.through, + hint=('Ensure that you did not misspell the model name and ' + 'the model is not abstract. Does your INSTALLED_APPS ' + 'setting contain the app where %s is defined?') % self.rel.through, + obj=self, + id='E023', + ) + ) + + elif not isinstance(self.rel.through, six.string_types): + + assert from_model is not None, \ + "ManyToManyField with intermediate " \ + "tables cannot be checked if you don't pass the model " \ + "where the field is attached to." + + # Set some useful local variables + to_model = self.rel.to + from_model_name = from_model._meta.object_name + if isinstance(to_model, six.string_types): + to_model_name = to_model + else: + to_model_name = to_model._meta.object_name + relationship_model_name = self.rel.through._meta.object_name + self_referential = from_model == to_model + + # Check symmetrical attribute. + if (self_referential and self.rel.symmetrical and + not self.rel.through._meta.auto_created): + errors.append( + checks.Error( + 'Many-to-many fields with intermediate tables must not be symmetrical.', + hint=None, + obj=self, + id='E024', + ) + ) + + # Count foreign keys in intermediate model + if self_referential: + seen_self = sum(from_model == getattr(field.rel, 'to', None) + for field in self.rel.through._meta.fields) + + if seen_self > 2: + errors.append( + checks.Error( + ('The model is used as an intermediary model by ' + '%s, but it has more than two foreign keys ' + 'to %s, which is ambiguous and is not permitted.') % (self, from_model_name), + hint=None, + obj=self.rel.through, + id='E025', + ) + ) + + else: + # Count foreign keys in relationship model + seen_from = sum(from_model == getattr(field.rel, 'to', None) + for field in self.rel.through._meta.fields) + seen_to = sum(to_model == getattr(field.rel, 'to', None) + for field in self.rel.through._meta.fields) + + if seen_from > 1: + errors.append( + checks.Error( + ('The model is used as an intermediary model by ' + '%s, but it has more than one foreign key ' + 'to %s, which is ambiguous and is not permitted.') % (self, from_model_name), + hint=('If you want to create a recursive relationship, ' + 'use ForeignKey("self", symmetrical=False, ' + 'through="%s").') % relationship_model_name, + obj=self, + id='E026', + ) + ) + + if seen_to > 1: + errors.append( + checks.Error( + ('The model is used as an intermediary model by ' + '%s, but it has more than one foreign key ' + 'to %s, which is ambiguous and is not permitted.') % (self, to_model_name), + hint=('If you want to create a recursive ' + 'relationship, use ForeignKey("self", ' + 'symmetrical=False, through="%s").') % relationship_model_name, + obj=self, + id='E027', + ) + ) + + if seen_from == 0 or seen_to == 0: + errors.append( + checks.Error( + ('The model is used as an intermediary model by ' + '%s, but it misses a foreign key to %s or %s.') % ( + self, from_model_name, to_model_name + ), + hint=None, + obj=self.rel.through, + id='E028', + ) + ) + return errors + def deconstruct(self): name, path, args, kwargs = super(ManyToManyField, self).deconstruct() # Handle the simpler arguments diff --git a/django/db/models/manager.py b/django/db/models/manager.py index 48ad3db9cc..299faa0917 100644 --- a/django/db/models/manager.py +++ b/django/db/models/manager.py @@ -7,6 +7,7 @@ from django.db.models import signals from django.db.models.fields import FieldDoesNotExist from django.utils import six from django.utils.deprecation import RenameMethodsBase +from django.utils.encoding import python_2_unicode_compatible def ensure_default_manager(sender, **kwargs): @@ -58,6 +59,7 @@ class RenameManagerMethods(RenameMethodsBase): ) +@python_2_unicode_compatible class BaseManager(six.with_metaclass(RenameManagerMethods)): # Tracks each time a Manager instance is created. Used to retain order. creation_counter = 0 @@ -70,6 +72,19 @@ class BaseManager(six.with_metaclass(RenameManagerMethods)): self._db = None self._hints = {} + def __str__(self): + """ Return "app_label.model_label.manager_name". """ + model = self.model + opts = model._meta + app = model._meta.app_label + manager_name = next(name for (_, name, manager) + in opts.concrete_managers + opts.abstract_managers + if manager == self) + return '%s.%s.%s' % (app, model._meta.object_name, manager_name) + + def check(self, **kwargs): + return [] + @classmethod def _get_queryset_methods(cls, queryset_class): def create_method(name, method): @@ -155,6 +170,10 @@ class BaseManager(six.with_metaclass(RenameManagerMethods)): def db(self): return self._db or router.db_for_read(self.model, **self._hints) + ####################### + # PROXIES TO QUERYSET # + ####################### + def get_queryset(self): """ Returns a new QuerySet object. Subclasses can override this method to diff --git a/django/db/models/options.py b/django/db/models/options.py index 414271a95e..ebbb19f479 100644 --- a/django/db/models/options.py +++ b/django/db/models/options.py @@ -30,13 +30,18 @@ def normalize_unique_together(unique_together): tuple of two strings. Normalize it to a tuple of tuples, so that calling code can uniformly expect that. """ - if not unique_together: - return () - first_element = next(iter(unique_together)) - if not isinstance(first_element, (tuple, list)): - unique_together = (unique_together,) - # Normalize everything to tuples - return tuple(tuple(ut) for ut in unique_together) + try: + if not unique_together: + return () + first_element = next(iter(unique_together)) + if not isinstance(first_element, (tuple, list)): + unique_together = (unique_together,) + # Normalize everything to tuples + return tuple(tuple(ut) for ut in unique_together) + except TypeError: + # If the value of unique_together isn't valid, return it + # verbatim; this will be picked up by the check framework later. + return unique_together @python_2_unicode_compatible diff --git a/django/forms/models.py b/django/forms/models.py index a8494360c2..7c4c92c140 100644 --- a/django/forms/models.py +++ b/django/forms/models.py @@ -927,9 +927,13 @@ def _get_foreign_key(parent_model, model, fk_name=None, can_fail=False): if not isinstance(fk, ForeignKey) or \ (fk.rel.to != parent_model and fk.rel.to not in parent_model._meta.get_parent_list()): - raise Exception("fk_name '%s' is not a ForeignKey to %s" % (fk_name, parent_model)) + raise ValueError( + "fk_name '%s' is not a ForeignKey to '%s.%'." + % (fk_name, parent_model._meta.app_label, parent_model._meta.object_name)) elif len(fks_to_parent) == 0: - raise Exception("%s has no field named '%s'" % (model, fk_name)) + raise ValueError( + "'%s.%s' has no field named '%s'." + % (model._meta.app_label, model._meta.object_name, fk_name)) else: # Try to discover what the ForeignKey from model to parent_model is fks_to_parent = [ @@ -943,9 +947,13 @@ def _get_foreign_key(parent_model, model, fk_name=None, can_fail=False): elif len(fks_to_parent) == 0: if can_fail: return - raise Exception("%s has no ForeignKey to %s" % (model, parent_model)) + raise ValueError( + "'%s.%s' has no ForeignKey to '%s.%s'." + % (model._meta.app_label, model._meta.object_name, parent_model._meta.app_label, parent_model._meta.object_name)) else: - raise Exception("%s has more than 1 ForeignKey to %s" % (model, parent_model)) + raise ValueError( + "'%s.%s' has more than one ForeignKey to '%s.%s'." + % (model._meta.app_label, model._meta.object_name, parent_model._meta.app_label, parent_model._meta.object_name)) return fk diff --git a/django/test/__init__.py b/django/test/__init__.py index c611ef6850..32608a94f2 100644 --- a/django/test/__init__.py +++ b/django/test/__init__.py @@ -8,10 +8,11 @@ from django.test.testcases import ( SimpleTestCase, LiveServerTestCase, skipIfDBFeature, skipUnlessDBFeature ) -from django.test.utils import modify_settings, override_settings +from django.test.utils import modify_settings, override_settings, override_system_checks __all__ = [ 'Client', 'RequestFactory', 'TestCase', 'TransactionTestCase', 'SimpleTestCase', 'LiveServerTestCase', 'skipIfDBFeature', 'skipUnlessDBFeature', 'modify_settings', 'override_settings', + 'override_system_checks' ] diff --git a/django/test/testcases.py b/django/test/testcases.py index 056eb5469f..4a44a171bc 100644 --- a/django/test/testcases.py +++ b/django/test/testcases.py @@ -787,7 +787,7 @@ class TransactionTestCase(SimpleTestCase): # We have to use this slightly awkward syntax due to the fact # that we're using *args and **kwargs together. call_command('loaddata', *self.fixtures, - **{'verbosity': 0, 'database': db_name, 'skip_validation': True}) + **{'verbosity': 0, 'database': db_name, 'skip_checks': True}) def _post_teardown(self): """Performs any post-test things. This includes: @@ -820,7 +820,7 @@ class TransactionTestCase(SimpleTestCase): # when flushing only a subset of the apps for db_name in self._databases_names(include_mirrors=False): call_command('flush', verbosity=0, interactive=False, - database=db_name, skip_validation=True, + database=db_name, skip_checks=True, reset_sequences=False, allow_cascade=self.available_apps is not None, inhibit_post_migrate=self.available_apps is not None) @@ -887,7 +887,7 @@ class TestCase(TransactionTestCase): 'verbosity': 0, 'commit': False, 'database': db_name, - 'skip_validation': True, + 'skip_checks': True, }) except Exception: self._fixture_teardown() diff --git a/django/test/utils.py b/django/test/utils.py index 42cb127129..d205f5c9fb 100644 --- a/django/test/utils.py +++ b/django/test/utils.py @@ -300,6 +300,26 @@ class modify_settings(override_settings): super(modify_settings, self).enable() +def override_system_checks(new_checks): + """ Acts as a decorator. Overrides list of registered system checks. + Useful when you override `INSTALLED_APPS`, e.g. if you exclude `auth` app, + you also need to exclude its system checks. """ + + from django.core.checks.registry import registry + + def outer(test_func): + @wraps(test_func) + def inner(*args, **kwargs): + old_checks = registry.registered_checks + registry.registered_checks = new_checks + try: + return test_func(*args, **kwargs) + finally: + registry.registered_checks = old_checks + return inner + return outer + + def compare_xml(want, got): """Tries to do a 'xml-comparison' of want and got. Plain string comparison doesn't always work because, for example, attribute diff --git a/django/utils/termcolors.py b/django/utils/termcolors.py index cb2a0f9748..aa2cdc97c3 100644 --- a/django/utils/termcolors.py +++ b/django/utils/termcolors.py @@ -76,6 +76,7 @@ LIGHT_PALETTE = 'light' PALETTES = { NOCOLOR_PALETTE: { 'ERROR': {}, + 'WARNING': {}, 'NOTICE': {}, 'SQL_FIELD': {}, 'SQL_COLTYPE': {}, @@ -95,6 +96,7 @@ PALETTES = { }, DARK_PALETTE: { 'ERROR': {'fg': 'red', 'opts': ('bold',)}, + 'WARNING': {'fg': 'yellow', 'opts': ('bold',)}, 'NOTICE': {'fg': 'red'}, 'SQL_FIELD': {'fg': 'green', 'opts': ('bold',)}, 'SQL_COLTYPE': {'fg': 'green'}, @@ -114,6 +116,7 @@ PALETTES = { }, LIGHT_PALETTE: { 'ERROR': {'fg': 'red', 'opts': ('bold',)}, + 'WARNING': {'fg': 'yellow', 'opts': ('bold',)}, 'NOTICE': {'fg': 'red'}, 'SQL_FIELD': {'fg': 'green', 'opts': ('bold',)}, 'SQL_COLTYPE': {'fg': 'green'}, diff --git a/docs/howto/custom-management-commands.txt b/docs/howto/custom-management-commands.txt index b5d461e2ce..42dd4bfc06 100644 --- a/docs/howto/custom-management-commands.txt +++ b/docs/howto/custom-management-commands.txt @@ -227,8 +227,23 @@ All attributes can be set in your derived class and can be used in wrapped with ``BEGIN;`` and ``COMMIT;``. Default value is ``False``. +.. attribute:: BaseCommand.requires_system_checks + +.. versionadded:: 1.7 + + A boolean; if ``True``, the entire Django project will be checked for + potential problems prior to executing the command. If + ``requires_system_checks`` is missing, the value of + ``requires_model_validation`` is used. If the latter flag is missing + as well, the default value (``True``) is used. Defining both + ``requires_system_checks`` and ``requires_model_validation`` will result + in an error. + .. attribute:: BaseCommand.requires_model_validation +.. deprecated:: 1.7 + Replaced by ``requires_system_checks`` + A boolean; if ``True``, validation of installed models will be performed prior to executing the command. Default value is ``True``. To validate an individual application's models @@ -299,12 +314,23 @@ the :meth:`~BaseCommand.handle` method must be implemented. The actual logic of the command. Subclasses must implement this method. +.. method:: BaseCommand.check(app_configs=None, tags=None, display_num_errors=False) + +.. versionadded:: 1.7 + + Uses the system check framework to inspect the entire Django project for + potential problems. Serious problems are raised as a :class:`CommandError`; + warnings are output to stderr; minor notifications are output to stdout. + + If ``apps`` and ``tags`` are both None, all system checks are performed. + ``tags`` can be a list of check tags, like ``compatibility`` or ``models``. + .. method:: BaseCommand.validate(app=None, display_num_errors=False) - Validates the given app, raising :class:`CommandError` for any errors. - - If ``app`` is None, then all installed apps are validated. +.. deprecated:: 1.7 + Replaced with the :djadmin:`check` command + If ``app`` is None, then all installed apps are checked for errors. .. _ref-basecommand-subclasses: diff --git a/docs/index.txt b/docs/index.txt index 1856beb65e..cf907df335 100644 --- a/docs/index.txt +++ b/docs/index.txt @@ -291,6 +291,7 @@ Learn about some other core functionalities of the Django framework: * :doc:`Flatpages ` * :doc:`Redirects ` * :doc:`Signals ` +* :doc:`System check framework ` * :doc:`The sites framework ` * :doc:`Unicode in Django ` diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index 34e974cea3..d8d0110a74 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -234,6 +234,16 @@ these changes. * Passing callable arguments to querysets will no longer be possible. +* ``BaseCommand.requires_model_validation`` will be removed in favor of + ``requires_system_checks``. Admin validators will be replaced by admin + checks. + +* ``ModelAdmin.validator`` will be removed in favor of the new ``checks`` + attribute. + +* ``django.db.backends.DatabaseValidation.validate_field`` will be removed in + favor of the ``check_field`` method. + 2.0 --- diff --git a/docs/intro/tutorial01.txt b/docs/intro/tutorial01.txt index e79535661c..1bf11e47ae 100644 --- a/docs/intro/tutorial01.txt +++ b/docs/intro/tutorial01.txt @@ -140,7 +140,7 @@ You'll see the following output on the command line: .. parsed-literal:: - Validating models... + Performing system checks... 0 errors found |today| - 15:50:53 @@ -545,8 +545,8 @@ Note the following: changes. If you're interested, you can also run -:djadmin:`python manage.py validate `; this checks for any errors in -your models without making migrations or touching the database. +:djadmin:`python manage.py check `; this checks for any problems in +your project without making migrations or touching the database. Now, run :djadmin:`migrate` again to create those model tables in your database: diff --git a/docs/ref/checks.txt b/docs/ref/checks.txt new file mode 100644 index 0000000000..0a10c753a3 --- /dev/null +++ b/docs/ref/checks.txt @@ -0,0 +1,208 @@ +====================== +System check framework +====================== + +.. versionadded:: 1.7 + +.. module:: django.core.checks + +The system check framework is a set of static checks for validating Django +projects. It detects common problems and provides hints for how to fix them. +The framework is extensible so you can easily add your own checks. + +Checks can be triggered explicitly via the :djadmin:`check` command. Checks are +triggered implicitly before most commands, including :djadmin:`runserver` and +:djadmin:`migrate`. For performance reasons, the checks are not performed if +:setting:`DEBUG` is set to ``False``. + +Serious errors will prevent Django commands (such as :djadmin:`runserver`) from +running at all. Minor problems are reported to the console. If you have inspected +the cause of a warning and are happy to ignore it, you can hide specific warnings +using the :setting:`SILENCED_SYSTEM_CHECKS` setting in your project settings file. + +Writing your own checks +======================= + +The framework is flexible and allows you to write functions that perform +any other kind of check you may require. The following is an example stub +check function:: + + from django.core.checks import register + + @register() + def example_check(app_configs, **kwargs): + errors = [] + # ... your check logic here + return errors + +The check function *must* accept an ``app_configs`` argument; this argument is +the list of applications that should be inspected. If None, the check must be +run on *all* installed apps in the project. The ``**kwargs`` argument is required +for future expansion. + +Messages +-------- + +The function must return a list of messages. If no problems are found as a result +of the check, the check function must return an empty list. + +.. class:: CheckMessage(level, msg, hint, obj=None, id=None) + +The warnings and errors raised by the check method must be instances of +:class:`~django.core.checks.CheckMessage`. An instance of +:class:`~django.core.checks.CheckMessage` encapsulates a single reportable +error or warning. It also provides context and hints applicable to the +message, and a unique identifier that is used for filtering purposes. + +The concept is very similar to messages from the :doc:`message +framework ` or the :doc:`logging framework +`. Messages are tagged with a ``level`` indicating the +severity of the message. + +Constructor arguments are: + +``level`` + The severity of the message. Use one of the + predefined values: ``DEBUG``, ``INFO``, ``WARNING``, ``ERROR``, + ``CRITICAL``. If the level is greater or equal to ``ERROR``, then Django + will prevent management commands from executing. Messages with + level lower than ``ERROR`` (i.e. warnings) are reported to the console, + but can be silenced. + +``msg`` + A short (less than 80 characters) string describing the problem. The string + should *not* contain newlines. + +``hint`` + A single-line string providing a hint for fixing the problem. If no hint + can be provided, or the hint is self-evident from the error message, a + value of ``None`` can be used:: + + Error('error message') # Will not work. + Error('error message', None) # Good + Error('error message', hint=None) # Better + +``obj`` + Optional. An object providing context for the message (for example, the + model where the problem was discovered). The object should be a model, field, + or manager or any other object that defines ``__str__`` method (on + Python 2 you need to define ``__unicode__`` method). The method is used while + reporting all messages and its result precedes the message. + +``id`` + Optional string. A unique identifier for the issue. Identifiers should + follow the pattern ``applabel.X001``, where ``X`` is one of the letters + ``CEWID``, indicating the message severity (``C`` for criticals, + ``E`` for errors and so). The number can be allocated by the application, + but should be unique within that application. + +There are also shortcuts to make creating messages with common levels easier. +When using these methods you can omit the ``level`` argument because it is +implied by the class name. + +.. class:: Debug(msg, hint, obj=None, id=None) +.. class:: Info(msg, hint, obj=None, id=None) +.. class:: Warning(msg, hint, obj=None, id=None) +.. class:: Error(msg, hint, obj=None, id=None) +.. class:: Critical(msg, hint, obj=None, id=None) + +Messages are comparable. That allows you to easily write tests:: + + from django.core.checks import Error + errors = checked_object.check() + expected_errors = [ + Error( + 'an error', + hint=None, + obj=checked_object, + id='myapp.E001', + ) + ] + self.assertEqual(errors, expected_errors) + +Registering and labeling checks +------------------------------- + +Lastly, your check function must be registered explicitly with system check +registry. + +.. function:: register(*tags)(function) + +You can pass as many tags to ``register`` as you want in order to label your +check. Tagging checks is useful since it allows you to run only a certain +group of checks. For example, to register a compatibility check, you would +make the following call:: + + from django.core.checks import register + + @register('compatibility') + def my_check(app_configs, **kwargs): + # ... perform compatibility checks and collect errors + return errors + +.. _field-checking: + +Field, Model, and Manager checks +-------------------------------- + +In some cases, you won't need to register your check function -- you can +piggyback on an existing registration. + +Fields, models, and model managers all implement a ``check()`` method that is +already registered with the check framework. If you want to add extra checks, +you can extend the implemenation on the base class, perform any extra +checks you need, and append any messages to those generated by the base class. +It's recommended the you delegate each check to a separate methods. + +Consider an example where you are implementing a custom field named +``RangedIntegerField``. This field adds ``min`` and ``max`` arguments to the +constructor of ``IntegerField``. You may want to add a check to ensure that users +provide a min value that is less than or equal to the max value. The following +code snippet shows how you can implement this check:: + + from django.core import checks + from django.db import models + + class RangedIntegerField(models.IntegerField): + def __init__(self, min=None, max=None, **kwargs): + super(RangedIntegerField, self).__init__(**kwargs) + self.min = min + self.max = max + + def check(self, **kwargs): + # Call the superclass + errors = super(RangedIntegerField, self).check(**kwargs) + + # Do some custom checks and add messages to `errors`: + errors.extend(self._check_min_max_values(**kwargs)) + + # Return all errors and warnings + return errors + + def _check_min_max_values(self, **kwargs): + if (self.min is not None and + self.max is not None and + self.min > self.max): + return [ + checks.Error( + 'min greated than max.', + hint='Decrease min or increase max.', + obj=self, + id='myapp.E001', + ) + ] + # When no error, return an empty list + return [] + +If you wanted to add checks to a model manager, you would take the same +approach on your sublass of :class:`~django.db.models.Manager`. + +If you want to add a check to a model class, the approach is *almost* the same: +the only difference is that the check is a classmethod, not an instance method:: + + class MyModel(models.Model): + @classmethod + def check(cls, **kwargs): + errors = super(MyModel, cls).check(**kwargs) + # ... your own checks ... + return errors diff --git a/docs/ref/django-admin.txt b/docs/ref/django-admin.txt index 06f589ee63..29c6bffb9e 100644 --- a/docs/ref/django-admin.txt +++ b/docs/ref/django-admin.txt @@ -95,18 +95,36 @@ documentation for the :djadminopt:`--verbosity` option. Available commands ================== -check ------ +check +--------------------------- .. django-admin:: check -.. versionadded:: 1.6 +.. versionchanged:: 1.7 -Performs a series of checks to verify a given setup (settings/application code) -is compatible with the current version of Django. +Uses the :doc:`system check framework ` to inspect +the entire Django project for common problems. -Upon finding things that are incompatible or require notifying the user, it -issues a series of warnings. +The system check framework will confirm that there aren't any problems with +your installed models or your admin registrations. It will also provide warnings +of common compatibility problems introduced by upgrading Django to a new version. +Custom checks may be introduced by other libraries and applications. + +By default, all apps will be checkd. You can check a subset of apps by providing +a list of app labels as arguments:: + + python manage.py auth admin myapp + +If you do not specify any app, all apps will be checked. + +.. django-admin-option:: --tag + +The :doc:`system check framework ` performs many different +types of checks. These check types are categorized with tags. You can use these tags +to restrict the checks performed to just those in a particular category. For example, +to perform only security and compatibility checks, you would run:: + + python manage.py --tag security -tag compatibility compilemessages --------------- @@ -810,9 +828,9 @@ reduction. ``pyinotify`` support was added. When you start the server, and each time you change Python code while the -server is running, the server will validate all of your installed models. (See -the :djadmin:`validate` command below.) If the validator finds errors, it will -print them to standard output, but it won't stop the server. +server is running, the server will check your entire Django project for errors (see +the :djadmin:`check` command). If any errors are found, they will be printed +to standard output, but it won't stop the server. You can run as many servers as you want, as long as they're on separate ports. Just execute ``django-admin.py runserver`` more than once. @@ -1310,6 +1328,9 @@ validate .. django-admin:: validate +.. deprecated:: 1.7 + Replaced by the :djadmin:`check` command. + Validates all installed models (according to the :setting:`INSTALLED_APPS` setting) and prints validation errors to standard output. diff --git a/docs/ref/index.txt b/docs/ref/index.txt index 6d7bfd4da4..0b2f3ab3eb 100644 --- a/docs/ref/index.txt +++ b/docs/ref/index.txt @@ -6,6 +6,7 @@ API Reference :maxdepth: 1 applications + checks class-based-views/index clickjacking contrib/index diff --git a/docs/ref/settings.txt b/docs/ref/settings.txt index e7ee4013c2..d3fc66b8e1 100644 --- a/docs/ref/settings.txt +++ b/docs/ref/settings.txt @@ -1766,6 +1766,22 @@ The backend used for signing cookies and other data. See also the :doc:`/topics/signing` documentation. +.. setting:: SILENCED_SYSTEM_CHECKS + +SILENCED_SYSTEM_CHECKS +---------------------- + +.. versionadded:: 1.7 + +Default: ``[]`` + +A list of identifers of messages generated by the system check framework +(i.e. ``["models.W001"]``) that you wish to permanently acknowledge and ignore. +Silenced warnings will no longer be output to the console; silenced errors +will still be printed, but will not prevent management commands from running. + +See also the :doc:`/ref/checks` documentation. + .. setting:: TEMPLATE_CONTEXT_PROCESSORS TEMPLATE_CONTEXT_PROCESSORS @@ -2737,6 +2753,7 @@ Error reporting * :setting:`IGNORABLE_404_URLS` * :setting:`MANAGERS` * :setting:`SEND_BROKEN_LINK_EMAILS` +* :setting:`SILENCED_SYSTEM_CHECKS` File uploads ------------ diff --git a/docs/releases/1.7.txt b/docs/releases/1.7.txt index 7d58f7a7c8..34b69e3e83 100644 --- a/docs/releases/1.7.txt +++ b/docs/releases/1.7.txt @@ -140,6 +140,17 @@ Using a custom manager when traversing reverse relations It is now possible to :ref:`specify a custom manager ` when traversing a reverse relationship. +New system check framework +~~~~~~~~~~~~~~~~~~~~~~~~~~ + +We've added a new :doc:`System check framework ` for +detecting common problems (like invalid models) and providing hints for +resolving those problems. The framework is extensible so you can add your +own checks for your own apps and libraries. + +To perform system checks, you use the :djadmin:`check` management command. +This command replaces the older :djadmin:`validate` management command. + New ``Prefetch`` object for advanced ``prefetch_related`` operations. ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -993,7 +1004,7 @@ considered a bug and has been addressed. ``syncdb`` ~~~~~~~~~~ -The ``syncdb`` command has been deprecated in favour of the new ``migrate`` +The :djadmin:`syncdb` command has been deprecated in favor of the new :djadmin:`migrate` command. ``migrate`` takes the same arguments as ``syncdb`` used to plus a few more, so it's safe to just change the name you're calling and nothing else. @@ -1103,3 +1114,32 @@ remove the setting from your configuration at your convenience. ``SplitDateTimeWidget`` support in :class:`~django.forms.DateTimeField` is deprecated, use ``SplitDateTimeWidget`` with :class:`~django.forms.SplitDateTimeField` instead. + +``validate`` +~~~~~~~~~~~~ + +:djadmin:`validate` command is deprecated in favor of :djadmin:`check` command. + +``django.core.management.BaseCommand`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +``requires_model_validation`` is deprecated in favor of a new +``requires_system_checks`` flag. If the latter flag is missing, then the +value of the former flag is used. Defining both ``requires_system_checks`` and +``requires_model_validation`` results in an error. + +The ``check()`` method has replaced the old ``validate()`` method. + +``ModelAdmin.validator`` +~~~~~~~~~~~~~~~~~~~~~~~~ + +``ModelAdmin.validator`` is deprecated in favor of new ``checks`` attribute. + +``django.db.backends.DatabaseValidation.validate_field`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +This method is deprecated in favor of a new ``check_field`` method. +The functionality required by ``check_field()`` is the same as that provided +by ``validate_field()``, but the output format is different. Third-party database +backends needing this functionality should modify their backends to provide an +implementation of ``check_field()``. diff --git a/docs/topics/db/models.txt b/docs/topics/db/models.txt index 1138f12fb2..51fa51194a 100644 --- a/docs/topics/db/models.txt +++ b/docs/topics/db/models.txt @@ -960,7 +960,7 @@ The reverse name of the ``common.ChildA.m2m`` field will be reverse name of the ``rare.ChildB.m2m`` field will be ``rare_childb_related``. It is up to you how you use the ``'%(class)s'`` and ``'%(app_label)s`` portion to construct your related name, but if you forget to use it, Django will raise -errors when you validate your models (or run :djadmin:`migrate`). +errors when you perform system checks (or run :djadmin:`migrate`). If you don't specify a :attr:`~django.db.models.ForeignKey.related_name` attribute for a field in an abstract base class, the default reverse name will @@ -1053,7 +1053,7 @@ are putting those types of relations on a subclass of another model, you **must** specify the :attr:`~django.db.models.ForeignKey.related_name` attribute on each such field. If you forget, Django will raise an error when you run -:djadmin:`validate` or :djadmin:`migrate`. +:djadmin:`check` or :djadmin:`migrate`. For example, using the above ``Place`` class again, let's create another subclass with a :class:`~django.db.models.ManyToManyField`:: diff --git a/tests/check/__init__.py b/tests/admin_checks/__init__.py similarity index 100% rename from tests/check/__init__.py rename to tests/admin_checks/__init__.py diff --git a/tests/admin_checks/models.py b/tests/admin_checks/models.py new file mode 100644 index 0000000000..5db1747a64 --- /dev/null +++ b/tests/admin_checks/models.py @@ -0,0 +1,57 @@ +""" +Tests of ModelAdmin system checks logic. +""" + +from django.db import models +from django.utils.encoding import python_2_unicode_compatible + + +class Album(models.Model): + title = models.CharField(max_length=150) + + +@python_2_unicode_compatible +class Song(models.Model): + title = models.CharField(max_length=150) + album = models.ForeignKey(Album) + original_release = models.DateField(editable=False) + + class Meta: + ordering = ('title',) + + def __str__(self): + return self.title + + def readonly_method_on_model(self): + # does nothing + pass + + +class TwoAlbumFKAndAnE(models.Model): + album1 = models.ForeignKey(Album, related_name="album1_set") + album2 = models.ForeignKey(Album, related_name="album2_set") + e = models.CharField(max_length=1) + + +class Author(models.Model): + name = models.CharField(max_length=100) + + +class Book(models.Model): + name = models.CharField(max_length=100) + subtitle = models.CharField(max_length=100) + price = models.FloatField() + authors = models.ManyToManyField(Author, through='AuthorsBooks') + + +class AuthorsBooks(models.Model): + author = models.ForeignKey(Author) + book = models.ForeignKey(Book) + + +class State(models.Model): + name = models.CharField(max_length=15) + + +class City(models.Model): + state = models.ForeignKey(State) diff --git a/tests/admin_checks/tests.py b/tests/admin_checks/tests.py new file mode 100644 index 0000000000..0f6320430e --- /dev/null +++ b/tests/admin_checks/tests.py @@ -0,0 +1,436 @@ +from __future__ import unicode_literals + +from django import forms +from django.contrib import admin +from django.core import checks +from django.core.exceptions import ImproperlyConfigured +from django.test import TestCase + +from .models import Song, Book, Album, TwoAlbumFKAndAnE, City, State + + +class SongForm(forms.ModelForm): + pass + + +class ValidFields(admin.ModelAdmin): + form = SongForm + fields = ['title'] + + +class ValidFormFieldsets(admin.ModelAdmin): + def get_form(self, request, obj=None, **kwargs): + class ExtraFieldForm(SongForm): + name = forms.CharField(max_length=50) + return ExtraFieldForm + + fieldsets = ( + (None, { + 'fields': ('name',), + }), + ) + + +class SystemChecksTestCase(TestCase): + + def test_checks_are_performed(self): + class MyAdmin(admin.ModelAdmin): + @classmethod + def check(self, model, **kwargs): + return ['error!'] + + admin.site.register(Song, MyAdmin) + try: + errors = checks.run_checks() + expected = ['error!'] + self.assertEqual(errors, expected) + finally: + admin.site.unregister(Song) + + def test_readonly_and_editable(self): + class SongAdmin(admin.ModelAdmin): + readonly_fields = ["original_release"] + fieldsets = [ + (None, { + "fields": ["title", "original_release"], + }), + ] + + errors = SongAdmin.check(model=Song) + self.assertEqual(errors, []) + + def test_custom_modelforms_with_fields_fieldsets(self): + """ + # Regression test for #8027: custom ModelForms with fields/fieldsets + """ + + errors = ValidFields.check(model=Song) + self.assertEqual(errors, []) + + def test_custom_get_form_with_fieldsets(self): + """ + Ensure that the fieldsets checks are skipped when the ModelAdmin.get_form() method + is overridden. + Refs #19445. + """ + + errors = ValidFormFieldsets.check(model=Song) + self.assertEqual(errors, []) + + def test_exclude_values(self): + """ + Tests for basic system checks of 'exclude' option values (#12689) + """ + + class ExcludedFields1(admin.ModelAdmin): + exclude = 'foo' + + errors = ExcludedFields1.check(model=Book) + expected = [ + checks.Error( + '"exclude" must be a list or tuple.', + hint=None, + obj=ExcludedFields1, + id='admin.E014', + ) + ] + self.assertEqual(errors, expected) + + def test_exclude_duplicate_values(self): + class ExcludedFields2(admin.ModelAdmin): + exclude = ('name', 'name') + + errors = ExcludedFields2.check(model=Book) + expected = [ + checks.Error( + '"exclude" contains duplicate field(s).', + hint=None, + obj=ExcludedFields2, + id='admin.E015', + ) + ] + self.assertEqual(errors, expected) + + def test_exclude_in_inline(self): + class ExcludedFieldsInline(admin.TabularInline): + model = Song + exclude = 'foo' + + class ExcludedFieldsAlbumAdmin(admin.ModelAdmin): + model = Album + inlines = [ExcludedFieldsInline] + + errors = ExcludedFieldsAlbumAdmin.check(model=Album) + expected = [ + checks.Error( + '"exclude" must be a list or tuple.', + hint=None, + obj=ExcludedFieldsInline, + id='admin.E014', + ) + ] + self.assertEqual(errors, expected) + + def test_exclude_inline_model_admin(self): + """ + Regression test for #9932 - exclude in InlineModelAdmin should not + contain the ForeignKey field used in ModelAdmin.model + """ + + class SongInline(admin.StackedInline): + model = Song + exclude = ['album'] + + class AlbumAdmin(admin.ModelAdmin): + model = Album + inlines = [SongInline] + + errors = AlbumAdmin.check(model=Album) + expected = [ + checks.Error( + ('Cannot exclude the field "album", because it is the foreign key ' + 'to the parent model admin_checks.Album.'), + hint=None, + obj=SongInline, + id='admin.E201', + ) + ] + self.assertEqual(errors, expected) + + def test_app_label_in_admin_checks(self): + """ + Regression test for #15669 - Include app label in admin system check messages + """ + + class RawIdNonexistingAdmin(admin.ModelAdmin): + raw_id_fields = ('nonexisting',) + + errors = RawIdNonexistingAdmin.check(model=Album) + expected = [ + checks.Error( + ('"raw_id_fields[0]" refers to field "nonexisting", which is ' + 'missing from model admin_checks.Album.'), + hint=None, + obj=RawIdNonexistingAdmin, + id='admin.E002', + ) + ] + self.assertEqual(errors, expected) + + def test_fk_exclusion(self): + """ + Regression test for #11709 - when testing for fk excluding (when exclude is + given) make sure fk_name is honored or things blow up when there is more + than one fk to the parent model. + """ + + class TwoAlbumFKAndAnEInline(admin.TabularInline): + model = TwoAlbumFKAndAnE + exclude = ("e",) + fk_name = "album1" + + class MyAdmin(admin.ModelAdmin): + inlines = [TwoAlbumFKAndAnEInline] + + errors = MyAdmin.check(model=Album) + self.assertEqual(errors, []) + + def test_inline_self_check(self): + class TwoAlbumFKAndAnEInline(admin.TabularInline): + model = TwoAlbumFKAndAnE + + class MyAdmin(admin.ModelAdmin): + inlines = [TwoAlbumFKAndAnEInline] + + errors = MyAdmin.check(model=Album) + expected = [ + checks.Error( + "'admin_checks.TwoAlbumFKAndAnE' has more than one ForeignKey to 'admin_checks.Album'.", + hint=None, + obj=TwoAlbumFKAndAnEInline, + id='admin.E202', + ) + ] + self.assertEqual(errors, expected) + + def test_inline_with_specified(self): + class TwoAlbumFKAndAnEInline(admin.TabularInline): + model = TwoAlbumFKAndAnE + fk_name = "album1" + + class MyAdmin(admin.ModelAdmin): + inlines = [TwoAlbumFKAndAnEInline] + + errors = MyAdmin.check(model=Album) + self.assertEqual(errors, []) + + def test_readonly(self): + class SongAdmin(admin.ModelAdmin): + readonly_fields = ("title",) + + errors = SongAdmin.check(model=Song) + self.assertEqual(errors, []) + + def test_readonly_on_method(self): + def my_function(obj): + pass + + class SongAdmin(admin.ModelAdmin): + readonly_fields = (my_function,) + + errors = SongAdmin.check(model=Song) + self.assertEqual(errors, []) + + def test_readonly_on_modeladmin(self): + class SongAdmin(admin.ModelAdmin): + readonly_fields = ("readonly_method_on_modeladmin",) + + def readonly_method_on_modeladmin(self, obj): + pass + + errors = SongAdmin.check(model=Song) + self.assertEqual(errors, []) + + def test_readonly_method_on_model(self): + class SongAdmin(admin.ModelAdmin): + readonly_fields = ("readonly_method_on_model",) + + errors = SongAdmin.check(model=Song) + self.assertEqual(errors, []) + + def test_nonexistant_field(self): + class SongAdmin(admin.ModelAdmin): + readonly_fields = ("title", "nonexistant") + + errors = SongAdmin.check(model=Song) + expected = [ + checks.Error( + ('"readonly_fields[1]" is neither a callable nor an attribute ' + 'of "SongAdmin" nor found in the model admin_checks.Song.'), + hint=None, + obj=SongAdmin, + id='admin.E035', + ) + ] + self.assertEqual(errors, expected) + + def test_nonexistant_field_on_inline(self): + class CityInline(admin.TabularInline): + model = City + readonly_fields = ['i_dont_exist'] # Missing attribute + + errors = CityInline.check(State) + expected = [ + checks.Error( + ('"readonly_fields[0]" is neither a callable nor an attribute ' + 'of "CityInline" nor found in the model admin_checks.City.'), + hint=None, + obj=CityInline, + id='admin.E035', + ) + ] + self.assertEqual(errors, expected) + + def test_extra(self): + class SongAdmin(admin.ModelAdmin): + def awesome_song(self, instance): + if instance.title == "Born to Run": + return "Best Ever!" + return "Status unknown." + + errors = SongAdmin.check(model=Song) + self.assertEqual(errors, []) + + def test_readonly_lambda(self): + class SongAdmin(admin.ModelAdmin): + readonly_fields = (lambda obj: "test",) + + errors = SongAdmin.check(model=Song) + self.assertEqual(errors, []) + + def test_graceful_m2m_fail(self): + """ + Regression test for #12203/#12237 - Fail more gracefully when a M2M field that + specifies the 'through' option is included in the 'fields' or the 'fieldsets' + ModelAdmin options. + """ + + class BookAdmin(admin.ModelAdmin): + fields = ['authors'] + + errors = BookAdmin.check(model=Book) + expected = [ + checks.Error( + ('"fields" cannot include the ManyToManyField "authors", ' + 'because "authors" manually specifies relationship model.'), + hint=None, + obj=BookAdmin, + id='admin.E013', + ) + ] + self.assertEqual(errors, expected) + + def test_cannot_include_through(self): + class FieldsetBookAdmin(admin.ModelAdmin): + fieldsets = ( + ('Header 1', {'fields': ('name',)}), + ('Header 2', {'fields': ('authors',)}), + ) + + errors = FieldsetBookAdmin.check(model=Book) + expected = [ + checks.Error( + ('"fieldsets[1][1][\'fields\']" cannot include the ManyToManyField ' + '"authors", because "authors" manually specifies relationship model.'), + hint=None, + obj=FieldsetBookAdmin, + id='admin.E013', + ) + ] + self.assertEqual(errors, expected) + + def test_nested_fields(self): + class NestedFieldsAdmin(admin.ModelAdmin): + fields = ('price', ('name', 'subtitle')) + + errors = NestedFieldsAdmin.check(model=Book) + self.assertEqual(errors, []) + + def test_nested_fieldsets(self): + class NestedFieldsetAdmin(admin.ModelAdmin): + fieldsets = ( + ('Main', {'fields': ('price', ('name', 'subtitle'))}), + ) + + errors = NestedFieldsetAdmin.check(model=Book) + self.assertEqual(errors, []) + + def test_explicit_through_override(self): + """ + Regression test for #12209 -- If the explicitly provided through model + is specified as a string, the admin should still be able use + Model.m2m_field.through + """ + + class AuthorsInline(admin.TabularInline): + model = Book.authors.through + + class BookAdmin(admin.ModelAdmin): + inlines = [AuthorsInline] + + errors = BookAdmin.check(model=Book) + self.assertEqual(errors, []) + + def test_non_model_fields(self): + """ + Regression for ensuring ModelAdmin.fields can contain non-model fields + that broke with r11737 + """ + + class SongForm(forms.ModelForm): + extra_data = forms.CharField() + + class FieldsOnFormOnlyAdmin(admin.ModelAdmin): + form = SongForm + fields = ['title', 'extra_data'] + + errors = FieldsOnFormOnlyAdmin.check(model=Song) + self.assertEqual(errors, []) + + def test_non_model_first_field(self): + """ + Regression for ensuring ModelAdmin.field can handle first elem being a + non-model field (test fix for UnboundLocalError introduced with r16225). + """ + + class SongForm(forms.ModelForm): + extra_data = forms.CharField() + + class Meta: + model = Song + fields = '__all__' + + class FieldsOnFormOnlyAdmin(admin.ModelAdmin): + form = SongForm + fields = ['extra_data', 'title'] + + errors = FieldsOnFormOnlyAdmin.check(model=Song) + self.assertEqual(errors, []) + + def test_validator_compatibility(self): + class MyValidator(object): + def validate(self, cls, model): + raise ImproperlyConfigured("error!") + + class MyModelAdmin(admin.ModelAdmin): + validator_class = MyValidator + + errors = MyModelAdmin.check(model=Song) + expected = [ + checks.Error( + 'error!', + hint=None, + obj=MyModelAdmin, + ) + ] + self.assertEqual(errors, expected) diff --git a/tests/invalid_models_tests/invalid_models/__init__.py b/tests/admin_scripts/app_raising_messages/__init__.py similarity index 100% rename from tests/invalid_models_tests/invalid_models/__init__.py rename to tests/admin_scripts/app_raising_messages/__init__.py diff --git a/tests/admin_scripts/app_raising_messages/models.py b/tests/admin_scripts/app_raising_messages/models.py new file mode 100644 index 0000000000..aece8a8176 --- /dev/null +++ b/tests/admin_scripts/app_raising_messages/models.py @@ -0,0 +1,27 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.core import checks +from django.db import models + + +class ModelRaisingMessages(models.Model): + @classmethod + def check(self, **kwargs): + return [ + checks.Warning( + 'First warning', + hint='Hint', + obj='obj' + ), + checks.Warning( + 'Second warning', + hint=None, + obj='a' + ), + checks.Error( + 'An error', + hint='Error hint', + obj=None, + ) + ] diff --git a/tests/admin_scripts/app_raising_warning/__init__.py b/tests/admin_scripts/app_raising_warning/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/admin_scripts/app_raising_warning/models.py b/tests/admin_scripts/app_raising_warning/models.py new file mode 100644 index 0000000000..8f58abe127 --- /dev/null +++ b/tests/admin_scripts/app_raising_warning/models.py @@ -0,0 +1,16 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.core import checks +from django.db import models + + +class ModelRaisingMessages(models.Model): + @classmethod + def check(self, **kwargs): + return [ + checks.Warning( + 'A warning', + hint=None, + ), + ] diff --git a/tests/admin_scripts/management/commands/app_command.py b/tests/admin_scripts/management/commands/app_command.py index 4706645484..f0981eba2d 100644 --- a/tests/admin_scripts/management/commands/app_command.py +++ b/tests/admin_scripts/management/commands/app_command.py @@ -3,7 +3,7 @@ from django.core.management.base import AppCommand class Command(AppCommand): help = 'Test Application-based commands' - requires_model_validation = False + requires_system_checks = False args = '[app_label ...]' def handle_app_config(self, app_config, **options): diff --git a/tests/admin_scripts/management/commands/base_command.py b/tests/admin_scripts/management/commands/base_command.py index 6e37ca238e..c313235ead 100644 --- a/tests/admin_scripts/management/commands/base_command.py +++ b/tests/admin_scripts/management/commands/base_command.py @@ -10,7 +10,7 @@ class Command(BaseCommand): make_option('--option_c', '-c', action='store', dest='option_c', default='3'), ) help = 'Test basic commands' - requires_model_validation = False + requires_system_checks = False args = '[labels ...]' def handle(self, *labels, **options): diff --git a/tests/admin_scripts/management/commands/label_command.py b/tests/admin_scripts/management/commands/label_command.py index 3bce1305bc..9bba413ff3 100644 --- a/tests/admin_scripts/management/commands/label_command.py +++ b/tests/admin_scripts/management/commands/label_command.py @@ -3,7 +3,7 @@ from django.core.management.base import LabelCommand class Command(LabelCommand): help = "Test Label-based commands" - requires_model_validation = False + requires_system_checks = False args = '