Model validation is just about ready to go. I have one small issue
with it, namely with ComplexValidator, which I'll describe below, but
I think we can resolve it fairly easily. Here's a bit of background.
Sorry if you're already familiar with the branch.
Validators are functions that are tied to fields. They take the
field's value, and raise a ValidationError if the value doesn't match
the validator's criteria. For example:
def validate_integer(value):
try:
int(value)
except (ValueError, TypeError), e:
raise ValidationError('')
Regular validators only have access to their field's own value, but
there's a second type of validator that is *also* tied to a field, but
has access to *all* of the cleaned form data, or the model instance,
depending on the context. When it raises ValidationError, the errors
are tied to that particular field instead of going into
non-field-errors like the form.clean() hook. It's this second type of
validator that I have a problem with, or rather, its implementation.
Right now, ComplexValidator's __call__ method perfoms the validation.
It takes obj, and all_values as arguments. Only one of them should be
provided. obj should be a model instance, all_values should be a
dictionary of cleaned data from a form. Here's the current
implementation:
class ComplexValidator(object):
def get_value(self, name, all_values, obj):
assert all_values or obj, "Either all_values or obj must
be supplied"
if all_values:
return all_values.get(name, None)
if obj:
return getattr(obj, name, None)
def __call__(self, value, all_values=None, obj=None):
raise NotImplementedError()
class RequiredIfOtherFieldBlank(ComplexValidator):
def __init__(self, other_field):
self.other_field = other_field
def __call__(self, value, all_values=None, obj=None):
if all_values is None:
all_values = {}
if self.get_value(self.other_field, all_values, obj) in
EMPTY_VALUES:
if value in EMPTY_VALUES:
raise ValidationError('This field is required if
%s is blank.' % self.other_field)
I have two reservations about this implementation:
The ComplexValidator doesn't know in advance if it's going to be used
for model validation or form validation. ComplexValidator's get_value
method is meant to help with this situation, but it needs to take
*both* the dict and obj values to get the value, and as such, it's a
little clunky.
The other problem I have with ComplexValidator is that your validators
must subclass it since we use isinstance checks to find them. It's
mostly a knee-jerk reaction. I can live with it, but it smells.
I know there was some discussion at EuroDjangoCon, and people seemed
OK with the current situation, but I'm -0 on leaving ComplexValidator
as-is. Here are a few options:
1) Drop ComplexValidator support for 1.2.
I think we could come up with a more elegant solution given some
usage and time. The branch is still incredibly useful even without
ComplexValidator.
2) Convert a model to a dict before passing it to ComplexValidator
so they always just deal with dicts.
This wouldn't address my discomfort with isinstance checks, but it
would make writing ComplexValidators a lot less clunky.
3) Leave ComplexValidator in as-is.
I don't need a whole lot of convincing that this is the way to go.
If we come up with a better solution later, ComplexValidator itself
would be fairly easy to deprecate.
4) You're awesome idea that has escaped me so far.
This will probably start with option 1, then we'd add the feature
after the branch is merged, or in the 1.3 timeline.
Does anyone have strong feelings about the way to go here?
Thank you Honza for all of your work on this code. It's a pretty
tricky problem, but we're almost there.
Joseph
Here are a few relevant parts of the code in case anyone wants to dig
in further.
ComplexValidator implementation:
http://code.djangoproject.com/browser/django/branches/soc2009/model-validation/django/core/validators.py#L139
ComplexValidator usage in model validation:
http://code.djangoproject.com/browser/django/branches/soc2009/model-validation/django/db/models/base.py#L810
ComplexValidator usage in form validation:
http://code.djangoproject.com/browser/django/branches/soc2009/model-validation/django/forms/forms.py#L293
--
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en.
