search

encode / django-rest-framework

Web APIs for Django. 🎸
https://www.django-rest-framework.org
Other
27.83k stars 6.76k forks source link

Add `__hash__` method for `permissions.OperandHolder` class #9417

Closed vanya909 closed 3 weeks ago

vanya909 commented 1 month ago

OperandHolder is not hashable, so need to add __hash__ method

Description

OperandHolder is not hashable after #8710, need to add __hash__ method to it to provide convenient syntax to filter unique permissions using set or dict.fromkeys

Ways to represent

1. from rest_framework import permissions
2. set([permissions.IsAuthenticated & permissions.IsAdminUser])

image

Fixes

Second way is provided in this PR

auvipy commented 3 weeks ago

thanks anyway. merged as it was small, after cross checking

adamchainz commented 3 weeks ago

We should really have tests for this. @vanya909 can you make a follow-up PR with a test that hashes an instance?

tomchristie commented 3 weeks ago

Can you explain why using set(...) on these classes is useful?

vanya909 commented 3 weeks ago

@tomchristie

Can you explain why using set(...) on these classes is useful?

It's useful in cases when we want to filter out unique permissions in order to avoid calculating of some complex permissions two or more times

Of course it can be done by just iterating over permissions list and appending in a result list those permissions which haven't been added yet, but using set() is more simple

In case when we want to keep original order of permissions dict.fromkeys() can be used, I would say dict.fromkeys() is preferable, but it also requires all permissions to be hashable

peterthomassen commented 2 weeks ago

Also, this fixes the regression introduced by #8710, so don't think it requires justification beyond that