Allow validators to short-circuit in form field validation

[Alexey Rogachev]

I have a couple of validators for files, FileSizeValidator and FileTypeValidator.

I attach it to form field like this:

class ChartForm(forms.ModelForm):
    import_file = forms.FileField(required=False, label='File for import', validators = [
        FileSizeValidator(max_size='500 kb'),
        FileTypeValidator(extensions=('xlsx',))
    ])

Validators are extended from object and implement __init__ and __call__ methods (latter raises ValidationError).

The problem is, when I load the bigger file and with different extension, both validators execute and both error messages are shown for field (about exceeding max limit and wrong extension).

What I want - if first validator failed, don't even apply the rest of validators. For this specific scenario is not crucial, but in case of bigger amount of validators and more complex checks it can be a problem. For example if we need to read the file and verify that something exists there, why whe should do that if a size limit already exceeded.

Another option can be customizing each validator to not execute in case of existing errors.

I tried to add additional field argument to validator:

class FileSizeValidator(object):
    def __init__(self, max_size, field):
        self.max_size = max_size
        self.field = field

and pass it in form's __init __ method:

class ChartForm(forms.ModelForm):
    def __init__(self, *args, **kwargs):
        super(ChartForm, self).__init__(*args, **kwargs)        
        for validator in self.fields['import_file'].validators:
            validator.field = self.fields['import_file']

but I don't know how to properly call self.field.clean() which requires data. Besides that, this is not good approach, because some other validators (for example from Django core) can be attached to field.

Also this will require creating some additional mixin that must be attached along with validators.

I think there should be much simpler and elegant solution.

Can't find related questions or info in docs.

[Tim Graham]

I think the idea makes sense but I'm not sure about a suitable API. If you have a design proposal, please share it on the DevelopersMailingList to get feedback.

[Alexey Rogachev]

What about adding additional kwarg to Field in django.forms.fields called for example show_circuit_validation:

def __init__(self, required=True, widget=None, label=None, initial=None, help_text='', error_messages=None, show_hidden_initial=False, validators=(), show_circuit_validation=False, localize=False, disabled=False, label_suffix=None):

And apply according changes in run_validators method:

def run_validators(self, value):
    if value in self.empty_values:
        return
    errors = []
    for v in self.validators:
        try:
            v(value)
        except ValidationError as e:
            if hasattr(e, 'code') and e.code in self.error_messages:
                e.message = self.error_messages[e.code]
            # Change start
            if self.short_circuit_validation:
                raise ValidationError(e.error_list)
            else:
                errors.extend(e.error_list)
            # Change end
        if errors:
            raise ValidationError(errors)

Not sure about default value for that argument, in my opinion it should be True.

[Alexey Rogachev]

Another option can be customizing each validator to not execute in case of existing errors.

Usually we need either to apply all validators or just first one, so probably this is redundant. But it can be implemented by adding additional argument to validator class and adding check with getattr for example in case it not exist. But validator can be also defined as simple function, so it brings additional complexity.

[Aymeric Augustin]

short_circuit_validation seems a bit narrow. It covers the specific use case where you can write a sequence of validator that should pass exactly in order, and if one fails, it isn't worth checking the next ones. I'm not convinced that will be sufficient in general "in case of bigger amount of validators and more complex checks".

I think writing a validator that wraps other validators and delegates to the appropriate ones, according to your business logic, is the most flexible approach. It doesn't require changes in Django.

[Alexey Rogachev]

The name of parameter (show_circuit_validation) is approximate, can be discussed and changed. I disagree that it covers specific use case. It's needed quite often. In "clean" methods it's possible and easy to achieve by just conditionally and sequentially raising ValidationError. Why not to have same possibility when using common validators? I'm not sure that creating a wrapper validator will be a good solution because validators in set can vary. Also this will require to change the way it's defined or subclass fields to override this logic, because this is defined in field.

As a workaround I can just do a set of calls in form's clean_import_file method like that:

def clean_import_file(self):
    import_file = self.cleaned_data.get('import_file')
    if import_file:
        FileSizeValidator(max_size='500 kb')(import_file)
        FileTypeValidator(extensions=('xlsx',))(import_file)
    return import_file

But I don't like this approach, even code is DRY, validators is not a part of configuration anymore.

[Alexey Rogachev]

Based on idea of Aymeric Augustin I've come up with this workaround:

class MultiValidator(object):
    def __init__(self, validators):
        self.validators = validators

    def __call__(self, value):
        for validator in self.validators:
            validator(value)

Then we can group multiple validators using this MultiValidator:

class ChartForm(forms.ModelForm):
    import_file = forms.FileField(required=False, label='File for import', validators=[MultiValidator([
        FileSizeValidator(max_size='500 kb'),
        FileTypeValidator(extensions=('xlsx',)),
    ])])

I don't like the additional nesting and mess with brackets, but besides that I think it's OK.

We can improve the look by subclassing and creating specific class:

class ChartFileMultiValidator(MultiValidator):
    def __init__(self):
        validators = [
            FileSizeValidator(max_size='500 kb'),
            FileTypeValidator(extensions=('xlsx',)),
        ]
        super(ChartFileMultiValidator, self).__init__(validators)

Then the code reduces to:

class ChartForm(forms.ModelForm):
    import_file = forms.FileField(required=False, label='File for import', validators=[ChartFileMultiValidator])

I personally prefer first approach.

In this case maybe add info about default strategy of applying validators for form field and possible option for short circuit validation to documentation?

[Tim Graham]

There isn't any consensus on the ​django-developers thread or here that changes should be made in Django.