Skip to content

Performance cost of using rules.contrib.rest_framework.AutoPermissionViewSetMixin #202

@ticosax

Description

@ticosax

Hi,
I've noticed that the AutoPermissionViewSetMixin.initial() method invokes get_object(), which is called again later to resolve the view by the handler method.
While in most if the cases it can have close to insignificant impact, as we are probably fetching a instance from an indexed property (pk, uuid, ...)
It is nonetheless not optimal to hit the DB more than necessary.

I have a PoC alternative implementation that hooks the permission check within DRF API and reuse the obj from the handler. (no additional cost of hitting the DB).
That alternative implementation requires to install DRF though.
I understand it's a no go to install DRF for everyone. It could be installed as part of extras though.
Would you be willing to consider it? If yes I could make a proper PR.

# settings
#...
REST_FRAMEWORK = {

    "DEFAULT_PERMISSION_CLASSES": [
        "rest_framework.permissions.IsAuthenticated",
        "djangoapp.mixins.permissions.AutoRulesPermissions",
    ],
...
}
from rest_framework import permissions

class AutoRulesPermissions(permissions.BasePermission):
    def _get_rule(self, view) -> str | None:
        if not (action := getattr(view, "action", None)) or not hasattr(
            view, "permission_type_map"
        ):
            return None
        try:
            return view.permission_type_map[action]
        except KeyError as exc:
            raise ImproperlyConfigured(
                f"AutoPermissionViewSetMixin tried to authorize a request with the "
                f"{view.action!r} action, but permission_type_map only contains: {view.permission_type_map!r}"
            ) from exc

    def has_permission(self, request, view) -> bool:
        if request.method.lower() not in view.http_method_names:
            return False
        try:
            getattr(view, request.method.lower())
        except AttributeError:
            # method not supported will be denied with 405 anyway
            return True
        if (perm_type := self._get_rule(view)) is None:
            # Skip permission checking for this action
            return True
        perm = view.get_queryset().model.get_perm(perm_type)
        return rules_permissions.test_rule(perm, request.user)

    def has_object_permission(self, request, view, obj) -> bool:
        if (perm_type := self._get_rule(view)) is None:
            # Skip permission checking for this action
            return True
        perm = obj.__class__.get_perm(perm_type)
        return rules_permissions.test_rule(perm, request.user, obj)

View Mixin would only carry the permission_type_map

class AutoPermissionViewSetMixin:
    """
    Enforces object-level permissions in ``rest_framework.viewsets.ViewSet``,
    deriving the permission type from the particular action to be performed..

    As with ``rules.contrib.views.AutoPermissionRequiredMixin``, this only works when
    model permissions are registered using ``rules.contrib.models.RulesModelMixin``.
    """

    # Maps API actions to model permission types. None as value skips permission
    # checks for the particular action.
    # This map needs to be extended when custom actions are implemented
    # using the @action decorator.
    # Extend or replace it in subclasses like so:
    # permission_type_map = {
    #     **AutoPermissionViewSetMixin.permission_type_map,
    #     "close": "change",
    #     "reopen": "change",
    # }
    permission_type_map = {
        "create": "add",
        "destroy": "delete",
        "list": None,
        "partial_update": "change",
        "retrieve": "view",
        "update": "change",
    }

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions