Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

has_permission gets called twice #8987

Closed
aminiun opened this issue May 20, 2023 · 5 comments
Closed

has_permission gets called twice #8987

aminiun opened this issue May 20, 2023 · 5 comments
Labels

Comments

@aminiun
Copy link
Contributor

aminiun commented May 20, 2023

Hi!
In drf==3.14, the has_object_permission code has changed in OR class in 4aea8dd.

The problem is, we are calling has_permission twice here, and I have some queries in my custom permission class, in has_permission method. So for each request, my queries are running twice and it makes my API slow.

I'd love to fix it myself, but I couldn't achieve any good solution for preventing calling has_permission again.
Any idea ?

@Ehsan200
Copy link
Contributor

Hello there!

First of all, it will run the has_permission function for every class in your permissions, so this behavior is normal.

To optimize performance, consider caching your query based on the request and using it repeatedly instead of running a query on the database each time.

Furthermore, please provide us with the details of your custom permission classes here.

@aminiun
Copy link
Contributor Author

aminiun commented May 21, 2023

Hello there!

First of all, it will run the has_permission function for every class in your permissions, so this behavior is normal.

To optimize performance, consider caching your query based on the request and using it repeatedly instead of running a query on the database each time.

Furthermore, please provide us with the details of your custom permission classes here.

Yes. In my case, I can use cache to prevent extra DB hit (actually I already have it).

My point is, assume a has_permission function that has some complexity. For example, I was in a project that had a rich ACL management. So the has_permission method was a heavy method with a lot of calculations and scenarios. Then calling has_permission twice would be a pain.

I'm not trying to solve it in my project, as the solution for me simply can be a cache. I'm just looking for enhancement and having a better optimization.

@Ehsan200
Copy link
Contributor

Ehsan200 commented May 22, 2023

Hello there!
First of all, it will run the has_permission function for every class in your permissions, so this behavior is normal.
To optimize performance, consider caching your query based on the request and using it repeatedly instead of running a query on the database each time.
Furthermore, please provide us with the details of your custom permission classes here.

Yes. In my case, I can use cache to prevent extra DB hit (actually I already have it).

My point is, assume a has_permission function that has some complexity. For example, I was in a project that had a rich ACL management. So the has_permission method was a heavy method with a lot of calculations and scenarios. Then calling has_permission twice would be a pain.

I'm not trying to solve it in my project, as the solution for me simply can be a cache. I'm just looking for enhancement and having a better optimization.

I think you might have misunderstood the class "OR."

class OperationHolderMixin:
    def __and__(self, other):
        return OperandHolder(AND, self, other)

    def __or__(self, other):
        return OperandHolder(OR, self, other)

    def __rand__(self, other):
        return OperandHolder(AND, other, self)

    def __ror__(self, other):
        return OperandHolder(OR, other, self)

    def __invert__(self):
        return SingleOperandHolder(NOT, self)

The BasePermission class is using this mixin.

In the __or__ method, it will perform a logical "or" operation on two permissions. This means that it will execute the has_permission method for both of the permission classes.
see this example:

composed_permissions = IsAuthenticatedUserOwner | permissions.IsAdminUser
has_perm = composed_permissions().has_object_permission(request, None, None)

It will execute has_permission for both IsAuthenticatedUserOwner and permissions.IsAdminUser, and this is normal. If I haven't understood your issue yet, please provide an example for clarification.

This is the edited version:

I understand what is happening here.
Yes, you are right. It will run has_permission twice if you call has_obj_permission because every view will run has_permission by default.
You can override check_permissions in your view to do nothing based on your API actions. If your action needs to call get_object, don't run the super method, but if not, run it.
This is a simple approach for you. You can also create a CustomMixin to handle that for you.

@aminiun
Copy link
Contributor Author

aminiun commented Jun 10, 2023

This is the edited version:

I understand what is happening here. Yes, you are right. It will run has_permission twice if you call has_obj_permission because every view will run has_permission by default. You can override check_permissions in your view to do nothing based on your API actions. If your action needs to call get_object, don't run the super method, but if not, run it. This is a simple approach for you. You can also create a CustomMixin to handle that for you.

Thanks for your suggestion. But I was more looking for a solution to fix it in DRF, not in my code.

@stale
Copy link

stale bot commented Aug 13, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Aug 13, 2023
@encode encode locked and limited conversation to collaborators Aug 15, 2023
@auvipy auvipy converted this issue into discussion #9072 Aug 15, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
Projects
None yet
Development

No branches or pull requests

2 participants