Suggestion: Add validators to Model Meta options

61 views
Skip to first unread message

Mathias Weirsøe Klitgaard

unread,
Dec 16, 2019, 10:41:40 AM12/16/19
to Django developers (Contributions to Django itself)
Hi Django Developers,

I'm new to contributing and from what I gathered, I should ask here before opening a ticket.

Right now the way to validate a model as a whole is to override Model.clean(). I have 2 problems with this:

1) If you do multiple checks that raise ValidationErrors, it is tedious to accumulate them.
2) Having different models that checks almost the same, e.g. start_date should come before end_date, it should be easy to reuse validators.

I suggest to add a 'validators' option to Model Meta options.
This should be very similar to the 'validators' option a Field has, a list of callables that raises ValidationErrors, except these should take an object 'instance' instead of a 'value'.
It should then be Model.full_clean() responsibility to call these model validators in addition to what it already does.

A concern I have is the confusion that may rise between these new 'model validators' and regular 'field validators'. Maybe clever naming could help distinguish them from eachother, and maybe it's not a huge deal since they are so closely related.

It would be nice if some common model validators got built-in as well. For example a ComparisonModelValidator could take the names of two fields and validate if they are in the right order, it could look like

class ComparisonModelValidator(low, high, message=None, code=None)
* low - field name of the lesser field
* high - field name of the greater field
* message - if not None, overrides message
* code - if not None, overrides code

Then it could be used in the model as

class Event(models.Model):
    start_date = models.DateField()
    end_date = models.DateField()
    ...

    class Meta:
        validators = [ComparisonModelValidator('start_date', 'end_date')]


All feedback is appreciated.

Mathias

Adam Johnson

unread,
Dec 16, 2019, 7:09:12 PM12/16/19
to django-d...@googlegroups.com
Hi Mathias

I feel like this has been suggested on the mailing list or ticket tracker before, a few years ago. However after a quick check I can't find the discussion/ticket in mind. Perhaps if you hunt a bit better than me you can find it.

I think it's definitely a whole in the validation API. I think it'd be better to see a composable Form.clean before another model option. Although Django makes things fast I do always feel a bit comfortable how closely coupled models and forms are.

Have you seen the database constraints added to Django and improved over the recent versions? https://docs.djangoproject.com/en/3.0/ref/models/constraints/ . I've been using them on a few projects with some success. I prefer this kind of validation since it's done in the database and thus it's impossible to insert or update rows that are bad. There's definitely a (substantial) project there to escalate check constraints into form validation errors too.

As for your composability approach - it's possible with today's Django if you use a bit of Python magic. clean() should work as a callable class rather than a function. For example (a sketch, untested):

class MultiCleaner:
    def __init__(self, model_validators):
        self.model_validators = model_validators
   
    def __call__(self, instance):
        for model_validator in model_validators:
            model_validator(instance)


class Model:
    ...
   
    clean = MultiCleaner([
        ComparisonModelValidator('start_date', 'end_date'),
    ])

If you want to expand this idea and test it I'd be interested to see the results.

Thanks,

Adam

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/57166b63-56e6-485a-ba99-23369e7c6b1b%40googlegroups.com.


--
Adam
Reply all
Reply to author
Forward
0 new messages