KCDC
KCDC

Reputation: 734

Mypy: properly typing a Django mixin class when accessing a method on super()

Django has a quirk where it doesn't validate models by default before writing to the database. A non-ideal situation that devs try to work around by creating a Mixin class for example: https://www.xormedia.com/django-model-validation-on-save/

The idea of that workaround is that you can inherit this mixin to your custom defined Django models whenever you want to add validation-on-save to it.

That approach works, but I'm trying to upgrade those old examples to a properly typed example. Below is my current code:

from typing import Iterable, TypeVar

from django.db import models

DjangoModel = TypeVar("DjangoModel", bound=models.Model)

class ValidateModelMixin:
    """Use this mixing to make model.save() call model.full_clean()

    Django's model.save() doesn't call full_clean() by default. More info:
    * "Why doesn't django's model.save() call full clean?"
        http://stackoverflow.com/questions/4441539/
    * "Model docs imply that ModelForm will call Model.full_clean(),
        but it won't."
        https://code.djangoproject.com/ticket/13100
    """

    def save(
        self: DjangoModel,
        force_insert: bool = False,
        force_update: bool = False,
        using: str | None = "default",  # DEFAULT_DB_ALIAS
        update_fields: Iterable[str] | None = None,
    ) -> None:
        """Override the save method to call full_clean before saving the model.

        Takes into account the force_insert and force_update flags, as they
        are passed to the save method when trying to skip the validation.
        Also passes on any positional and keyword arguments that were passed
        at the original call-site of the method.
        """
        # Only validate the model if the force-flags are not enabled
        if not (force_insert or force_update):
            self.full_clean()

        # Then save the model, passing in the original arguments
        super().save(force_insert, force_update, using, update_fields)

Mypy produces the following error for the code above:

error: Unsupported argument 2 for "super" [misc]

I think this is Mypy not liking the arguments I'm passing to super().save(). But those arguments do seem to match the arguments of Django's models.Model.save: https://docs.djangoproject.com/en/5.0/ref/models/instances/#django.db.models.Model.save https://github.com/typeddjango/django-stubs/blob/1546e5c78aae6974f93d1c2c29ba50c177187f3a/django-stubs/db/models/base.pyi#L72

I believe I might not be setting the right type for the self argument, but I'm not sure how I should be typing this code instead.

Upvotes: 0

Views: 435

Answers (1)

KCDC
KCDC

Reputation: 734

I the end this other question helped me out. Specifically:

mypy recommends to implement mixins through a Protocol

Mypy docs

So I implemented a protocol to define the type for a DjangoModel and that made the type checks pass. The full code that I ended up using:

from typing import Iterable, Protocol, Self


class DjangoModel(Protocol):
    def save(
        self: Self,
        force_insert: bool = False,
        force_update: bool = False,
        using: str | None = "default",
        update_fields: Iterable[str] | None = None,
    ) -> None: ...

    def full_clean(
        self,
        exclude: Iterable[str] | None = None,
        validate_unique: bool = True,
        validate_constraints: bool = True,
    ) -> None: ...


class ValidateModelMixin:
    """Make model.save() call model.full_clean()

    Django's model.save() doesn't call full_clean() by default. More info:
    * "Why doesn't django's model.save() call full clean?"
        http://stackoverflow.com/questions/4441539/

    * "Model docs imply that ModelForm will call Model.full_clean(),
        but it won't."
        https://code.djangoproject.com/ticket/13100
    """

    def save(
        self: DjangoModel,
        force_insert: bool = False,
        force_update: bool = False,
        using: str | None = "default",  # DEFAULT_DB_ALIAS
        update_fields: Iterable[str] | None = None,
    ) -> None:
        """Override the save method to call full_clean before saving the model.

        Takes into account the force_insert and force_update flags, as they
        are passed to the save method when trying to skip the validation.
        Also passes on any positional and keyword arguments that were passed
        at the original call-site of the method.
        """
        # Only validate the model if the force-flags are not enabled
        if not (force_insert or force_update):
            self.full_clean()

        # Type ignore below, because even though mypy is correct that calling
        # save on super() here directly is not safe, we don't do that
        # for this usecase. Instead it's called on the instance inheriting from
        # both the Mixin and Django's models.Model. Ideally we'd inherit Django's
        # models.Model directly, instead of using a mixin, so we can guarantee 
        # super.save exists. But Django's tabel queries rely on the first 
        # class inheriting models.Model, which would then be the mixin. 
        # In other words: direct inheritance would break Django's queries.
        super().save(force_insert, force_update, using, update_fields)  # type: ignore[safe-super]

Edit: Unfortunately, I still had to use a type-ignore[safe-super] in the latest version of Mypy. As with this solution you could call ValidateModelMixin.save directly, which would crash at runtime because the mixin itself doesn't inherit from Django's models.Model (which is the super().save we're referring to).

I couldn't find a way to fix that because:

  1. You cannot let the mixin class inherit from models.Model because Django's table queries rely on the first class inheriting models.Model, so queries will try to query <app_name>_validatemixinmodel. Therefore direct inheritance seems to not be an option.
  2. Making the mixin an AbstractBaseClass (to avoid instances being created and ValidateModelMixin.save() being called on those instances) results in the following error in Django: TypeError: metaclass conflict: the metaclass of a derived class must be a (non-strict) subclass of the metaclasses of all its bases

Conclusion: the code above provides more type-safety than not providing types at all, but you do have to remember not to call .save on the ValidateModelMixin or its instances directly, because with the type-ignore mypy will not warn you against doing so. For me that's okay as it doesn't make sense to do that anyway, but if someone comes up with a better approach then I can change the accepted answer on this question.

Upvotes: 1

Related Questions