Django full_clean on save

388 views
Skip to first unread message

Tal Einat

unread,
Oct 19, 2021, 3:44:13 AM10/19/21
to pywe...@googlegroups.com
Hi folks,

For those who've worked with Django a lot: What are your experiences, or thoughts, with making Django models call full_clean() on save(), to validate things that can't be validated at the DB level? (By overriding the save() method or registering a pre_save signal handler.)

If also using DRF, are there cases where you'd prefer validations to be on the model itself rather than its DRF serializer?

- Tal Einat

Yuval Adam

unread,
Oct 19, 2021, 3:55:34 AM10/19/21
to pywe...@googlegroups.com
I generally like this idea (implemented usually by overriding save()) , as a failsafe to ensure I'm not saving any bad data.

However, I would expect to catch any data validation errors higher up the stack, like in the DRF serializer, where error handling can be more granular and relevant.

--
You received this message because you are subscribed to the Google Groups "PyWeb-IL" group.
To unsubscribe from this group and stop receiving emails from it, send an email to pyweb-il+u...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/pyweb-il/CALWZvp755-eoGLNnTdtyRhE_51ytRCMzsv%2BWqB_yNTisGy2EMw%40mail.gmail.com.


--
Yuval Adam

אורי

unread,
Oct 19, 2021, 4:00:10 AM10/19/21
to pywe...@googlegroups.com
Hi Tal,

In Speedy Net all the models run full_clean() in save() since its launch (2019). All the models inherit from TimeStampedModel which inherits from  BaseModel.

class ValidateModelMixin(object):
    def save(self, *args, **kwargs):
        """
        Call `full_clean` before saving.
        """
        self.full_clean()
        return super().save(*args, **kwargs)


class BaseModel(ValidateModelMixin, models.Model):
    objects = BaseManager()

    def save(self, *args, **kwargs):
        try:
            field = self._meta.get_field('id')
            if ((not (self.id)) and (hasattr(field, 'id_generator'))):
                self.id = field.id_generator()
                while (self._meta.model.objects.filter(id=self.id).exists()):
                    self.id = field.id_generator()
        except FieldDoesNotExist:
            pass
        return super().save(*args, **kwargs)

    class Meta:
        abstract = True


class TimeStampedModel(BaseModel):
    date_created = models.DateTimeField(auto_now_add=True, db_index=True)
    date_updated = models.DateTimeField(auto_now=True, db_index=True)

    class Meta:
        abstract = True




On Tue, Oct 19, 2021 at 10:44 AM Tal Einat <tale...@gmail.com> wrote:
--

Shai Berger

unread,
Oct 19, 2021, 4:51:22 AM10/19/21
to pywe...@googlegroups.com
On Tue, 19 Oct 2021 10:44:01 +0300
Tal Einat <tale...@gmail.com> wrote:

> Hi folks,
>
> For those who've worked with Django a lot: What are your experiences,
> or thoughts, with making Django models call full_clean() on save(), to
> validate things that can't be validated at the DB level? (By
> overriding the save() method or registering a pre_save signal
> handler.)
>

Model validation was added in the 1.2 or 1.3 version, and if I recall
correctly, the assumption already back then was that it's a good idea
to add it when you write new code -- I don't remember exactly the
reasons for not doing it by default; I'm guessing somewhere between
backwards-compatibility and the fact that, since most of the same
validations are performed by model-forms anyway, and some of them can be
non-trivial (e.g. including database access for uniqueness checks), it
might cause undue inefficiency (back then, a very large majority of the
interaction with models was done through forms).

> If also using DRF, are there cases where you'd prefer validations to
> be on the model itself rather than its DRF serializer?
>

I'd say it depends on how much the models get saved outside of
serializers.

HTH,
Shai.

alonn

unread,
Oct 20, 2021, 3:13:35 AM10/20/21
to PyWeb-IL
Agreed with Yuval, it's not a bad Idea, but should be a last line of defense, not the first
Reply all
Reply to author
Forward
0 new messages