Feat/add cache to permissions

[Ehsan200]

Description

Add new base class for permissions to cache result of has_permission and 'has_object_permission' methods to prevent multi call in some senarios.

refs #8987

[Ehsan200]

no more progress in this PR

[auvipy]

@encode/django-rest-framework Should we consider the caching of permissions by principle?

[sevdog]

Caching with a simple lru_cache without any limits would just waste a lot of memory, since one element in the cache would lead to a memory-DoS quite easy.

If a cache should be used it should avoid this kind of problems.

This implementation however does not resolve the real issue since the permission classes are instantiated twice due to how APIView.get_permission is implemented:

django-rest-framework/rest_framework/views.py

Lines 274 to 278 in 4f7e9ed

	def get_permissions(self):
	"""
	Instantiates and returns the list of permissions that this view requires.
	"""
	return [permission() for permission in self.permission_classes]

django-rest-framework/rest_framework/views.py

Lines 326 to 350 in 4f7e9ed

	def check_permissions(self, request):
	"""
	Check if the request should be permitted.
	Raises an appropriate exception if the request is not permitted.
	"""
	for permission in self.get_permissions():
	if not permission.has_permission(request, self):
	self.permission_denied(
	request,
	message=getattr(permission, 'message', None),
	code=getattr(permission, 'code', None)
	)
	def check_object_permissions(self, request, obj):
	"""
	Check if the request should be permitted for a given object.
	Raises an appropriate exception if the request is not permitted.
	"""
	for permission in self.get_permissions():
	if not permission.has_object_permission(request, self, obj):
	self.permission_denied(
	request,
	message=getattr(permission, 'message', None),
	code=getattr(permission, 'code', None)
	)

Thus, this kind of LRU is only going to waste memory and slow down the program.

I agree that #7522 may cause some problems, also because the behaviour is not so intuitive and is only documented in the changelog and not on the permissions API reference.

[Ehsan200]

Caching with a simple lru_cache without any limits would just waste a lot of memory, since one element in the cache would lead to a memory-DoS quite easy.

If a cache should be used it should avoid this kind of problems.

This implementation however does not resolve the real issue since the permission classes are instantiated twice due to how APIView.get_permission is implemented:

django-rest-framework/rest_framework/views.py

Lines 274 to 278 in 4f7e9ed

	def get_permissions(self):
	"""
	Instantiates and returns the list of permissions that this view requires.
	"""
	return [permission() for permission in self.permission_classes]

django-rest-framework/rest_framework/views.py

Lines 326 to 350 in 4f7e9ed

	def check_permissions(self, request):
	"""
	Check if the request should be permitted.
	Raises an appropriate exception if the request is not permitted.
	"""
	for permission in self.get_permissions():
	if not permission.has_permission(request, self):
	self.permission_denied(
	request,
	message=getattr(permission, 'message', None),
	code=getattr(permission, 'code', None)
	)
	def check_object_permissions(self, request, obj):
	"""
	Check if the request should be permitted for a given object.
	Raises an appropriate exception if the request is not permitted.
	"""
	for permission in self.get_permissions():
	if not permission.has_object_permission(request, self, obj):
	self.permission_denied(
	request,
	message=getattr(permission, 'message', None),
	code=getattr(permission, 'code', None)
	)

Thus, this kind of LRU is only going to waste memory and slow down the program.

I agree that #7522 may cause some problems, also because the behaviour is not so intuitive and is only documented in the changelog and not on the permissions API reference.

Hello there!
You are right about lru_cache and I will change that.
but about the main problem, it will solve the issue because view classes will use the cached_permissions property to use the same permissions in that class.

[tomchristie]

Should we consider the caching of permissions by principle?

Short: We shouldn't be making functional changes to REST framework now, other than tracking new Django releases.

[auvipy]

I am going to close this for now. i think this can be done as per projects need/customised.