HonzaKral / django forked from django/django

Unofficial mirror of the Unofficial mirror of the Subversion repository intended to be used for work on #6845

Home | Edit | New

Model validation

High-level overview of the changes

The overview moves step by step from models to forms, discussing models, model forms and forms.

Models (django.db.models.Model)

  • clean and validate methods were added – validation should never alter data, clean is the hook for that
    • clean calls clean on individual fields and then calls validate, ValidationError with message_dict containing the errors is raised when errors occur
    • not implemented validate calls validate_unique and is a hook to add custom instance-wide validation; raise ValidationError from it in case of errors
    • validate is run even if some errors occur in clean
  • not implemented validate_unique has been moved here from ModelForm
  • not implemented implement validation for unqiue_for_date

Changes to the initial implementation:

  • no validate_FIELD or clean_FIELD hooks on models

Model fields (django.db.models.fields)

  • clean and validate methods were added
    • clean calls to_python and then validate
  • field classes accept a validator_list in constructor for hooking custom validation

Model forms (django.forms.models.BaseModelForm)

  • validate_unique has been moved to Model
  • save_instance remains, but its functionality has been split in two:
    • make_instance(form, instance, fields=None, exclude=None) which returns the unsaved instance with populated fields
      • file fields are saved as well, which is not ideal
    • save_made_instance(form, instance, fields=None, commit=True, fail_message=‘saved’) which saves the populated instance (and it’s m2m data)
    • save_instance is stilll around for backwards compatibility, it calls make_instance, then save_made_instance
  • clean constructs the instance with make_instance, then runs validate on it
  • save only calls save_made_instance

Model formsets (django.forms.models.BaseModelFormset)

  • call form.save in save_new and save_existing
  • not implemented: validate has been added for “cross-instance validation”, i.e. the model instances in the model formset may break any of the uniqueness constraints (unique, unique_together, unique_for_date,week,month) in between themselves (ticket #9493)

Validators

  • validators (replacements for many, but not all of the old validators) have been put in django.core.validators
  • There are two forms of validators — simple function-based arguments, and more complex class-based ones.
  • Simple validators look like:
def is_integer(value, all_values={}, instance=None):
  try:
    int(value)
  except (TypeError, ValueError):
    raise ValidationError("Not an integer", code=SOME_CODE)

error_code can be used later in forms to look up custom error messages.

  • Complex validators:
  class IsGreaterThan(object):
    def __init__(self, minval):
      self.minval = minval
    def __call__(self, value, all_values, instance=None):
      if value < self.minval:
        raise ValidationError(...)

Note that the naming is deliberate. Simple validators should be functions, complex ones classes, both named according to PEP8. Whenever possible, prefer imperative names to objective ones (e.g something like IsShorterThan(length) instead of LengthValidator).

The ValidationError class has moved from forms.utils to django.core.exceptions now that it is used for more than just forms. ErrorList has been kept in forms.utils.

Tasks

general:

  1. (Honza – first working draft has landed) Alter ValidationError to have .code property that will be:
    1. used to override error messages as currently seen in forms
    2. one of some constants (so create a few ;) )
    3. add message params to the VE as well to allow for messages with variables
  2. move duplicit code converting dates/times etc to django.util.dates from django.forms.fields and django.db.models.fields.init

Validators:

  1. (Honza – forst working draft done) add validators to FormField and have it called in the field’s clean() method
    1. port as much logic from form fields to independent validators
  2. (Alex) port as many validators from oldforms

Unique

  1. (Honza – done) move validate_unique from form to model

Tests

  1. (Eric) Write tests for is_lowercase simple validators
  2. Write tests for IsUpperCase complex validators
  3. Write tests for Fields that implement validation
  4. Write models that implement complex validation

Issues

  • custom messages – we have all the information in ValidationError (message, code and params for the message), we need to figure out what to do with it and, more importantly, where.
  • I am not sure FileFields should be saved prior to validation (make_instance) Gulopine is working on that

Open design decisions

  • which validators should be removed from core.validators? Should something be added?

The schism of Form and Model

How much of the validation do we want to push on form, and how much do we keep in the model and rely on form calling model.clean() ?

If we push too little (even less then now), the only way how to do so is stripping DbField.formfield() to a bare minimum, some code would break for people not using ModelForms but DbField.formfield() directly.

If we push too much, validation is going to be run twice and could result in delays and double error messages.

The problem of empty values in validators

Who should be responsible for handling None and other “empty” values (e.g. the empty string)?


So far we have decided to make validators handle empty values on their own – that means that most validators will contain line

 if value in EMPTY_VALUES: return

update on Jan 12 by mrts: In 0.96 the problem described below by Alex is solved by testing the validator attribute ‘always_run’ (hasattr(validator, ‘always_run’)_), see the corresponding line in 0.96 oldforms_init__.py#L347. I suggest we use similar logic (see below).


Alex is in favor of having empty checking delegated to the validators because:

  1. there are certain complex validators that need to know if a value is None when the field is null=True, examples such as required_if_other_field_is_empty needs to still be run if the value is None.

mrts is in favor of lifting that to fields for the following reasons:

  1. as the field has the information to decide whether empty value is indeed allowed (by examining self.null and self.blank), this should be field’s responsibility. If the value is empty, the validators shouldn’t be called at all.
  2. removes quite a lot of duplication in validators (duplication is smelly :) ).
  3. that’s how it was done in 0.96 — to deal with the problem Alex describes, a special validator attribute always_run was tested for.

mrts proposes the following change to db.models.Field.validate:

Current:


class Field(…):

def validate(…):

if value is None and not self.null:
raise exceptions.ValidationError(
ugettext_lazy(“This field cannot be null.”))

  1. cannot do if not value because of 0 passed to integer fields
    if not self.blank and value in ( None, ’’ ):
    raise exceptions.ValidationError(
    ugettext_lazy(“This field cannot be blank.”))
for validator in self.validators: validator(value, instance=model_instance)

Proposed (updated on Jan 12):


class Field(…):

def validate(…):

if value is None and not self.null:
raise exceptions.ValidationError(
ugettext_lazy(“This field cannot be null.”))

  1. cannot do if not value because of 0 passed to integer fields
  2. -- CHANGES START HERE
    value_is_empty = value in EMPTY_VALUES
if not self.blank and value_is_empty: raise exceptions.ValidationError( ugettext_lazy(“This field cannot be blank.”)) for validator in self.validators: if value_is_empty and not hasattr(validator, ‘run_on_empty_value’): continue validator(value, instance=model_instance)

Translating validation errors

Document that any custom errors that are not directly instantiated while raising ValidationError s have to use ugettext_lazy .

Ordinary ugettext translation should be used while raising errors (as translation occurs when error is instantiated):


raise ValidationError(_(“Invalid”))

But gettext_lazy should be used when passing error messages as field arguments (as plain ugettext would translate the string when the field is instantiated, which usually occurs before locale switching):


foo = RegexField(…, error_messages={ … : ugettext_lazy(“Invalid”) })

Last edited by mrts, Mon Feb 02 13:09:25 -0800 2009
Home | Edit | New
Versions: