From 8362b45c8e94d5bf3585f27b2e821388d9b9fb14 Mon Sep 17 00:00:00 2001 From: Robert Sparks Date: Tue, 17 Sep 2024 13:05:56 -0500 Subject: [PATCH] fix: optimize and debug has_role and can_manage_some_groups (#7949) * fix: optimize can_manage_some_groups * fix: improve cache key * refactor: extra_role_qs to kwargs and bugfix to cache key * fix: restrict groupman_role matches to active states * chore: styling, decommenting, black --- ietf/group/utils.py | 21 ++++-- ietf/ietfauth/utils.py | 162 +++++++++++++++++++++++++++++------------ 2 files changed, 131 insertions(+), 52 deletions(-) diff --git a/ietf/group/utils.py b/ietf/group/utils.py index 68b618120b..dcf9d83e6f 100644 --- a/ietf/group/utils.py +++ b/ietf/group/utils.py @@ -2,6 +2,7 @@ # -*- coding: utf-8 -*- import datetime +from itertools import chain from pathlib import Path from django.db.models import Q @@ -153,17 +154,23 @@ def can_manage_materials(user, group): def can_manage_session_materials(user, group, session): return has_role(user, 'Secretariat') or (group.has_role(user, group.features.matman_roles) and not session.is_material_submission_cutoff()) -# Maybe this should be cached... def can_manage_some_groups(user): if not user.is_authenticated: return False + authroles = set( + chain.from_iterable( + GroupFeatures.objects.values_list("groupman_authroles", flat=True) + ) + ) + extra_role_qs = dict() for gf in GroupFeatures.objects.all(): - for authrole in gf.groupman_authroles: - if has_role(user, authrole): - return True - if Role.objects.filter(name__in=gf.groupman_roles, group__type_id=gf.type_id, person__user=user).exists(): - return True - return False + extra_role_qs[f"{gf.type_id} groupman roles"] = Q( + name__in=gf.groupman_roles, + group__type_id=gf.type_id, + group__state__in=["active", "bof", "proposed"], + ) + return has_role(user, authroles, extra_role_qs=extra_role_qs) + def can_provide_status_update(user, group): if not group.features.acts_like_wg: diff --git a/ietf/ietfauth/utils.py b/ietf/ietfauth/utils.py index 6fa9cddbcb..d8bd67a4e0 100644 --- a/ietf/ietfauth/utils.py +++ b/ietf/ietfauth/utils.py @@ -38,9 +38,10 @@ def has_role(user, role_names, *args, **kwargs): """Determines whether user has any of the given standard roles given. Role names must be a list or, in case of a single value, a string.""" - if not isinstance(role_names, (list, tuple)): - role_names = [ role_names ] - + extra_role_qs = kwargs.get("extra_role_qs", None) + if not isinstance(role_names, (list, tuple, set)): + role_names = [role_names] + if not user or not user.is_authenticated: return False @@ -48,7 +49,13 @@ def has_role(user, role_names, *args, **kwargs): if not hasattr(user, "roles_check_cache"): user.roles_check_cache = {} - key = frozenset(role_names) + keynames = set(role_names) + if extra_role_qs: + keynames.update(set(extra_role_qs.keys())) + year = kwargs.get("year", None) + if year is not None: + keynames.add(f"nomcomyear{year}") + key = frozenset(keynames) if key not in user.roles_check_cache: try: person = user.person @@ -56,54 +63,119 @@ def has_role(user, role_names, *args, **kwargs): return False role_qs = { - "Area Director": Q(person=person, name__in=("pre-ad", "ad"), group__type="area", group__state="active"), - "Secretariat": Q(person=person, name="secr", group__acronym="secretariat"), - "IAB" : Q(person=person, name="member", group__acronym="iab"), - "IANA": Q(person=person, name="auth", group__acronym="iana"), - "RFC Editor": Q(person=person, name="auth", group__acronym="rpc"), - "ISE" : Q(person=person, name="chair", group__acronym="ise"), - "IAD": Q(person=person, name="admdir", group__acronym="ietf"), - "IETF Chair": Q(person=person, name="chair", group__acronym="ietf"), - "IETF Trust Chair": Q(person=person, name="chair", group__acronym="ietf-trust"), - "IRTF Chair": Q(person=person, name="chair", group__acronym="irtf"), - "RSAB Chair": Q(person=person, name="chair", group__acronym="rsab"), - "IAB Chair": Q(person=person, name="chair", group__acronym="iab"), - "IAB Executive Director": Q(person=person, name="execdir", group__acronym="iab"), - "IAB Group Chair": Q(person=person, name="chair", group__type="iab", group__state="active"), - "IAOC Chair": Q(person=person, name="chair", group__acronym="iaoc"), - "WG Chair": Q(person=person,name="chair", group__type="wg", group__state__in=["active","bof", "proposed"]), - "WG Secretary": Q(person=person,name="secr", group__type="wg", group__state__in=["active","bof", "proposed"]), - "RG Chair": Q(person=person,name="chair", group__type="rg", group__state__in=["active","proposed"]), - "RG Secretary": Q(person=person,name="secr", group__type="rg", group__state__in=["active","proposed"]), - "AG Secretary": Q(person=person,name="secr", group__type="ag", group__state__in=["active"]), - "RAG Secretary": Q(person=person,name="secr", group__type="rag", group__state__in=["active"]), - "Team Chair": Q(person=person,name="chair", group__type="team", group__state="active"), - "Program Lead": Q(person=person,name="lead", group__type="program", group__state="active"), - "Program Secretary": Q(person=person,name="secr", group__type="program", group__state="active"), - "Program Chair": Q(person=person,name="chair", group__type="program", group__state="active"), - "EDWG Chair": Q(person=person, name="chair", group__type="edwg", group__state="active"), - "Nomcom Chair": Q(person=person, name="chair", group__type="nomcom", group__acronym__icontains=kwargs.get('year', '0000')), - "Nomcom Advisor": Q(person=person, name="advisor", group__type="nomcom", group__acronym__icontains=kwargs.get('year', '0000')), - "Nomcom": Q(person=person, group__type="nomcom", group__acronym__icontains=kwargs.get('year', '0000')), - "Liaison Manager": Q(person=person,name="liaiman",group__type="sdo",group__state="active", ), - "Authorized Individual": Q(person=person,name="auth",group__type="sdo",group__state="active", ), - "Recording Manager": Q(person=person,name="recman",group__type="ietf",group__state="active", ), - "Reviewer": Q(person=person, name="reviewer", group__state="active"), - "Review Team Secretary": Q(person=person, name="secr", group__reviewteamsettings__isnull=False,group__state="active", ), - "IRSG Member": (Q(person=person, name="member", group__acronym="irsg") | Q(person=person, name="chair", group__acronym="irtf") | Q(person=person, name="atlarge", group__acronym="irsg")), - "RSAB Member": Q(person=person, name="member", group__acronym="rsab"), - "Robot": Q(person=person, name="robot", group__acronym="secretariat"), - } - - filter_expr = Q(pk__in=[]) # ensure empty set is returned if no other terms are added + "Area Director": Q( + name__in=("pre-ad", "ad"), group__type="area", group__state="active" + ), + "Secretariat": Q(name="secr", group__acronym="secretariat"), + "IAB": Q(name="member", group__acronym="iab"), + "IANA": Q(name="auth", group__acronym="iana"), + "RFC Editor": Q(name="auth", group__acronym="rpc"), + "ISE": Q(name="chair", group__acronym="ise"), + "IAD": Q(name="admdir", group__acronym="ietf"), + "IETF Chair": Q(name="chair", group__acronym="ietf"), + "IETF Trust Chair": Q(name="chair", group__acronym="ietf-trust"), + "IRTF Chair": Q(name="chair", group__acronym="irtf"), + "RSAB Chair": Q(name="chair", group__acronym="rsab"), + "IAB Chair": Q(name="chair", group__acronym="iab"), + "IAB Executive Director": Q(name="execdir", group__acronym="iab"), + "IAB Group Chair": Q( + name="chair", group__type="iab", group__state="active" + ), + "IAOC Chair": Q(name="chair", group__acronym="iaoc"), + "WG Chair": Q( + name="chair", + group__type="wg", + group__state__in=["active", "bof", "proposed"], + ), + "WG Secretary": Q( + name="secr", + group__type="wg", + group__state__in=["active", "bof", "proposed"], + ), + "RG Chair": Q( + name="chair", group__type="rg", group__state__in=["active", "proposed"] + ), + "RG Secretary": Q( + name="secr", group__type="rg", group__state__in=["active", "proposed"] + ), + "AG Secretary": Q( + name="secr", group__type="ag", group__state__in=["active"] + ), + "RAG Secretary": Q( + name="secr", group__type="rag", group__state__in=["active"] + ), + "Team Chair": Q(name="chair", group__type="team", group__state="active"), + "Program Lead": Q( + name="lead", group__type="program", group__state="active" + ), + "Program Secretary": Q( + name="secr", group__type="program", group__state="active" + ), + "Program Chair": Q( + name="chair", group__type="program", group__state="active" + ), + "EDWG Chair": Q(name="chair", group__type="edwg", group__state="active"), + "Nomcom Chair": Q( + name="chair", + group__type="nomcom", + group__acronym__icontains=kwargs.get("year", "0000"), + ), + "Nomcom Advisor": Q( + name="advisor", + group__type="nomcom", + group__acronym__icontains=kwargs.get("year", "0000"), + ), + "Nomcom": Q( + group__type="nomcom", + group__acronym__icontains=kwargs.get("year", "0000"), + ), + "Liaison Manager": Q( + name="liaiman", + group__type="sdo", + group__state="active", + ), + "Authorized Individual": Q( + name="auth", + group__type="sdo", + group__state="active", + ), + "Recording Manager": Q( + name="recman", + group__type="ietf", + group__state="active", + ), + "Reviewer": Q(name="reviewer", group__state="active"), + "Review Team Secretary": Q( + name="secr", + group__reviewteamsettings__isnull=False, + group__state="active", + ), + "IRSG Member": ( + Q(name="member", group__acronym="irsg") + | Q(name="chair", group__acronym="irtf") + | Q(name="atlarge", group__acronym="irsg") + ), + "RSAB Member": Q(name="member", group__acronym="rsab"), + "Robot": Q(name="robot", group__acronym="secretariat"), + } + + filter_expr = Q( + pk__in=[] + ) # ensure empty set is returned if no other terms are added for r in role_names: filter_expr |= role_qs[r] + if extra_role_qs: + for r in extra_role_qs: + filter_expr |= extra_role_qs[r] - user.roles_check_cache[key] = bool(Role.objects.filter(filter_expr).exists()) + user.roles_check_cache[key] = bool( + Role.objects.filter(person=person).filter(filter_expr).exists() + ) return user.roles_check_cache[key] + # convenient decorator def passes_test_decorator(test_func, message):