From 5c9c0bbc2a2b05b3d3ee10ac31c4ada127ef665b Mon Sep 17 00:00:00 2001 From: Yohann D'ANELLO Date: Thu, 2 Apr 2020 00:30:22 +0200 Subject: [PATCH] Optimize permissions, use memoization --- apps/logs/signals.py | 4 +- apps/permission/backends.py | 50 ++++++++++++++++++------ apps/permission/decorators.py | 56 +++++++++++++++++++++++++++ apps/permission/models.py | 2 +- apps/permission/signals.py | 4 +- apps/permission/templatetags/perms.py | 10 +---- 6 files changed, 101 insertions(+), 25 deletions(-) create mode 100644 apps/permission/decorators.py diff --git a/apps/logs/signals.py b/apps/logs/signals.py index 37e9ba79..68bf95c0 100644 --- a/apps/logs/signals.py +++ b/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 diff --git a/apps/permission/backends.py b/apps/permission/backends.py index b95f8203..ef58403b 100644 --- a/apps/permission/backends.py +++ b/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 diff --git a/apps/permission/decorators.py b/apps/permission/decorators.py new file mode 100644 index 00000000..b6863f23 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/apps/permission/models.py b/apps/permission/models.py index b1012159..8aaf416c 100644 --- a/apps/permission/models.py +++ b/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) diff --git a/apps/permission/signals.py b/apps/permission/signals.py index 3bb79897..4ee63ae3 100644 --- a/apps/permission/signals.py +++ b/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() diff --git a/apps/permission/templatetags/perms.py b/apps/permission/templatetags/perms.py index b06fdd9a..4f61fcfe 100644 --- a/apps/permission/templatetags/perms.py +++ b/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