Optimize permissions, use memoization

apps/logs/signals.py

@@ -50,7 +50,7 @@ def save_object(sender, instance, **kwargs):
     if instance._meta.label_lower in EXCLUDED:
         return
 
-    if hasattr(instance, "_force_save"):
+    if hasattr(instance, "_no_log"):
         return
 
     # noinspection PyProtectedMember
@@ -109,7 +109,7 @@ def delete_object(sender, instance, **kwargs):
     if instance._meta.label_lower in EXCLUDED:
         return
 
-    if hasattr(instance, "_force_delete"):
+    if hasattr(instance, "_no_log"):
         return
 
     # Si un utilisateur est connecté, on récupère l'utilisateur courant ainsi que son adresse IP

apps/permission/backends.py

@@ -10,6 +10,7 @@ from django.db.models import Q, F
 from note.models import Note, NoteUser, NoteClub, NoteSpecial
 from note_kfet.middlewares import get_current_session
 from member.models import Membership, Club
+from .decorators import memoize
 
 from .models import Permission
 
@@ -22,6 +23,28 @@ class PermissionBackend(ModelBackend):
     supports_anonymous_user = False
     supports_inactive_user = False
 
+    @staticmethod
+    @memoize
+    def get_raw_permissions(user, t):
+        """
+        Query permissions of a certain type for a user, then memoize it.
+        :param user: The owner of the permissions
+        :param t: The type of the permissions: view, change, add or delete
+        :return: The queryset of the permissions of the user (memoized) grouped by clubs
+        """
+        if isinstance(user, AnonymousUser):
+            # Unauthenticated users have no permissions
+            return Permission.objects.none()
+
+        return Permission.objects.annotate(club=F("rolepermissions__role__membership__club")) \
+            .filter(
+                rolepermissions__role__membership__user=user,
+                rolepermissions__role__membership__date_start__lte=datetime.date.today(),
+                rolepermissions__role__membership__date_end__gte=datetime.date.today(),
+                type=t,
+                mask__rank__lte=get_current_session().get("permission_mask", 0),
+            ).distinct('club', 'pk')
+
     @staticmethod
     def permissions(user, model, type):
         """
@@ -31,18 +54,16 @@ class PermissionBackend(ModelBackend):
         :param type: The type of the permissions: view, change, add or delete
         :return: A generator of the requested permissions
         """
-        for permission in Permission.objects.annotate(club=F("rolepermissions__role__membership__club")) \
-                .filter(
-            rolepermissions__role__membership__user=user,
-            rolepermissions__role__membership__date_start__lte=datetime.date.today(),
-            rolepermissions__role__membership__date_end__gte=datetime.date.today(),
-            model__app_label=model.app_label,  # For polymorphic models, we don't filter on model type
-            type=type,
-        ).all():
-            if not isinstance(model, permission.model.__class__) or not permission.club:
+        clubs = {}
+
+        for permission in PermissionBackend.get_raw_permissions(user, type):
+            if not isinstance(model.model_class()(), permission.model.model_class()) or not permission.club:
                 continue
 
-            club = Club.objects.get(pk=permission.club)
+            if permission.club not in clubs:
+                clubs[permission.club] = club = Club.objects.get(pk=permission.club)
+            else:
+                club = clubs[permission.club]
             permission = permission.about(
                 user=user,
                 club=club,
@@ -56,10 +77,10 @@ class PermissionBackend(ModelBackend):
                 F=F,
                 Q=Q
             )
-            if permission.mask.rank <= get_current_session().get("permission_mask", 0):
-                yield permission
+            yield permission
 
     @staticmethod
+    @memoize
     def filter_queryset(user, model, t, field=None):
         """
         Filter a queryset by considering the permissions of a given user.
@@ -93,10 +114,15 @@ class PermissionBackend(ModelBackend):
             query = query | perm.query
         return query
 
+    @memoize
     def has_perm(self, user_obj, perm, obj=None):
         if user_obj is None or isinstance(user_obj, AnonymousUser):
             return False
 
+        sess = get_current_session()
+        if sess is not None and sess.session_key is None:
+            return Permission.objects.none()
+
         if user_obj.is_superuser and get_current_session().get("permission_mask", 0) >= 42:
             return True
 

apps/permission/decorators.py

@@ -0,0 +1,56 @@
+# Copyright (C) 2018-2020 by BDE ENS Paris-Saclay
+# SPDX-License-Identifier: GPL-3.0-or-later
+
+from functools import lru_cache
+from time import time
+
+from django.contrib.sessions.models import Session
+from note_kfet.middlewares import get_current_session
+
+
+def memoize(f):
+    """
+    Memoize results and store in sessions
+
+    This decorator is useful for permissions: they are loaded once needed, then stored for next calls.
+    The storage is contained with sessions since it depends on the selected mask.
+    """
+    sess_funs = {}
+    last_collect = time()
+
+    def collect():
+        """
+        Clear cache of results when sessions are invalid, to flush useless data.
+        This function is called every minute.
+        """
+        nonlocal sess_funs
+
+        new_sess_funs = {}
+        for sess_key in sess_funs:
+            if Session.objects.filter(session_key=sess_key).exists():
+                new_sess_funs[sess_key] = sess_funs[sess_key]
+        sess_funs = new_sess_funs
+
+    def func(*args, **kwargs):
+        nonlocal last_collect
+
+        if time() - last_collect > 60:
+            # Clear cache
+            collect()
+            last_collect = time()
+
+        # If there is no session, then we don't memoize anything.
+        sess = get_current_session()
+        if sess is None or sess.session_key is None:
+            return f(*args, **kwargs)
+
+        sess_key = sess.session_key
+        if sess_key not in sess_funs:
+            # lru_cache makes the job of memoization
+            # We store only the 512 latest data per session. It has to be enough.
+            sess_funs[sess_key] = lru_cache(512)(f)
+        return sess_funs[sess_key](*args, **kwargs)
+
+    func.func_name = f.__name__
+
+    return func

apps/permission/models.py

@@ -45,7 +45,7 @@ class InstancedPermission:
                 else:
                     oldpk = obj.pk
                 # Ensure previous models are deleted
-                self.model.model_class().objects.filter(pk=obj.pk).annotate(_force_delete=F("pk") + 1).delete()
+                self.model.model_class().objects.filter(pk=obj.pk).annotate(_force_delete=F("pk")).delete()
                 # Force insertion, no data verification, no trigger
                 obj._force_save = True
                 Model.save(obj, force_insert=True)

apps/permission/signals.py

@@ -27,7 +27,7 @@ def pre_save_object(sender, instance, **kwargs):
     if instance._meta.label_lower in EXCLUDED:
         return
 
-    if hasattr(instance, "_no_log"):
+    if hasattr(instance, "_force_save"):
         return
 
     user = get_current_authenticated_user()
@@ -74,7 +74,7 @@ def pre_delete_object(instance, **kwargs):
     if instance._meta.label_lower in EXCLUDED:
         return
 
-    if hasattr(instance, "_no_log"):
+    if hasattr(instance, "_force_delete"):
         return
 
     user = get_current_authenticated_user()

apps/permission/templatetags/perms.py

@@ -20,11 +20,8 @@ def not_empty_model_list(model_name):
         return False
     elif user.is_superuser and session.get("permission_mask", 0) >= 42:
         return True
-    if session.get("not_empty_model_list_" + model_name, None):
-        return session.get("not_empty_model_list_" + model_name, None) == 1
     qs = model_list(model_name)
-    session["not_empty_model_list_" + model_name] = 1 if qs.exists() else 2
-    return session.get("not_empty_model_list_" + model_name) == 1
+    return qs.exists()
 
 
 @stringfilter
@@ -38,11 +35,8 @@ def not_empty_model_change_list(model_name):
         return False
     elif user.is_superuser and session.get("permission_mask", 0) >= 42:
         return True
-    if session.get("not_empty_model_change_list_" + model_name, None):
-        return session.get("not_empty_model_change_list_" + model_name, None) == 1
     qs = model_list(model_name, "change")
-    session["not_empty_model_change_list_" + model_name] = 1 if qs.exists() else 2
-    return session.get("not_empty_model_change_list_" + model_name) == 1
+    return qs.exists()
 
 
 @stringfilter