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 = '