Merge pull request #107 from JoelEager/the-great-refactor-round-2

Cache cleanup and general refactoring

Author: JoelEager

packet/__init__.py

@@ -31,6 +31,8 @@
 logging.getLogger().setLevel(app.config["LOG_LEVEL"])
 app.logger.info("Launching packet v" + app.config["VERSION"])
 
+app.logger.info("Using the {} realm".format(app.config["REALM"]))
+
 # Initialize the extensions
 db = SQLAlchemy(app)
 migrate = Migrate(app, db)

packet/commands.py

@@ -1,5 +1,5 @@
 """
-Defines command-line utilities for use with the app
+Defines command-line utilities for use with packet
 """
 
 from secrets import token_hex

packet/context_processors.py

@@ -11,7 +11,6 @@
 
 
 # pylint: disable=bare-except
-@lru_cache(maxsize=128)
 def get_csh_name(username):
     try:
         member = ldap_get_member(username)

packet/ldap.py

@@ -1,9 +1,12 @@
+"""
+Helper functions for working with the csh_ldap library
+"""
+
 from functools import lru_cache
 
 from packet import _ldap
 
 
-@lru_cache(maxsize=1024)
 def _ldap_get_group_members(group):
     return _ldap.get_group(group).get_members()
 
@@ -16,48 +19,21 @@ def _ldap_is_member_of_group(member, group):
     return False
 
 
-@lru_cache(maxsize=1024)
-def _ldap_is_member_of_directorship(account, directorship):
-    directors = _ldap.get_directorship_heads(directorship)
-    for director in directors:
-        if director.uid == account.uid:
-            return True
-    return False
-
-
 # Getters
 
-@lru_cache(maxsize=4096)
+@lru_cache(maxsize=256)
 def ldap_get_member(username):
     return _ldap.get_member(username, uid=True)
 
 
-@lru_cache(maxsize=1024)
 def ldap_get_active_members():
     return _ldap_get_group_members("active")
 
 
-@lru_cache(maxsize=1024)
 def ldap_get_intro_members():
     return _ldap_get_group_members("intromembers")
 
 
-@lru_cache(maxsize=1024)
-def ldap_get_onfloor_members():
-    return _ldap_get_group_members("onfloor")
-
-
-@lru_cache(maxsize=1024)
-def ldap_get_groups(account):
-    group_list = account.get("memberOf")
-    groups = []
-    for group_dn in group_list:
-        if "cn=groups,cn=accounts" in group_dn:
-            groups.append(group_dn.split(",")[0][3:])
-    return groups
-
-
-@lru_cache(maxsize=1024)
 def ldap_get_eboard():
     members = _ldap_get_group_members("eboard-chairman") + _ldap_get_group_members("eboard-evaluations"
         ) + _ldap_get_group_members("eboard-financial") + _ldap_get_group_members("eboard-history"
@@ -68,63 +44,28 @@ def ldap_get_eboard():
     return members
 
 
-@lru_cache(maxsize=2048)
 def ldap_get_live_onfloor():
     """
     :return: All upperclassmen who live on floor and are not eboard
     """
     members = []
-    onfloor = ldap_get_onfloor_members()
+    onfloor = _ldap_get_group_members("onfloor")
     for member in onfloor:
         if ldap_get_roomnumber(member) and not ldap_is_eboard(member):
             members.append(member)
+
     return members
 
 
 # Status checkers
 
-@lru_cache(maxsize=1024)
-def ldap_is_eval_director(account):
-    return _ldap_is_member_of_directorship(account, 'evaluations')
-
-
-@lru_cache(maxsize=1024)
-def ldap_is_active(account):
-    return _ldap_is_member_of_group(account, 'active')
-
-
-@lru_cache(maxsize=1024)
-def ldap_is_alumni(account):
-    # If the user is not active, they are an alumni.
-    return not _ldap_is_member_of_group(account, 'active')
-
-
-@lru_cache(maxsize=1024)
 def ldap_is_eboard(account):
     return _ldap_is_member_of_group(account, 'eboard')
 
 
-@lru_cache(maxsize=1024)
-def ldap_is_rtp(account):
-    return _ldap_is_member_of_group(account, 'rtp')
-
-
-@lru_cache(maxsize=1024)
 def ldap_is_intromember(account):
     return _ldap_is_member_of_group(account, 'intromembers')
 
-
-@lru_cache(maxsize=1024)
-def ldap_is_onfloor(account):
-    return _ldap_is_member_of_group(account, 'onfloor')
-
-
-@lru_cache(maxsize=1024)
-def ldap_is_current_student(account):
-    return _ldap_is_member_of_group(account, 'current_student')
-
-
-@lru_cache(maxsize=1024)
 def ldap_get_roomnumber(account):
     try:
         return account.roomNumber

packet/log_utils.py

@@ -5,7 +5,10 @@
 from functools import wraps
 from datetime import datetime
 
-from packet import context_processors, app
+from packet import app
+from packet.context_processors import get_rit_name
+from packet.ldap import ldap_get_member
+from packet.utils import is_freshman_on_floor
 
 
 def log_time(func):
@@ -26,16 +29,29 @@ def wrapped_function(*args, **kwargs):
     return wrapped_function
 
 
+def _format_cache(func):
+    """
+    :return: The output of func.cache_info() as a compactly formatted string
+    """
+    info = func.cache_info()
+    return "{}[hits={}, misses={}, size={}/{}]".format(func.__name__, info.hits, info.misses, info.currsize,
+                                                       info.maxsize)
+
+
+# Tuple of lru_cache functions to log stats from
+_caches = (get_rit_name, ldap_get_member, is_freshman_on_floor)
+
+
 def log_cache(func):
     """
     Decorator for logging cache info
     """
+
     @wraps(func)
     def wrapped_function(*args, **kwargs):
         result = func(*args, **kwargs)
 
-        app.logger.info("get_csh_name(): {}, get_rit_name(): {}".format(context_processors.get_csh_name.cache_info(),
-                                                                        context_processors.get_rit_name.cache_info()))
+        app.logger.info("Cache stats: " + ", ".join(map(_format_cache, _caches)))
 
         return result
 

packet/models.py

@@ -1,5 +1,5 @@
 """
-Defines the application's database models.
+Defines the application's database models
 """
 
 from datetime import datetime
@@ -107,7 +107,7 @@ def is_100(self):
         """
         Checks if this packet has reached 100%
         """
-        return self.signatures_required().total == self.signatures_recieved().total
+        return self.signatures_required().total == self.signatures_received().total
 
     @classmethod
     def open_packets(cls):

packet/routes/api.py

@@ -1,3 +1,7 @@
+"""
+Shared API endpoints
+"""
+
 from packet import app, db
 from packet.utils import before_request, packet_auth, notify_slack
 from packet.models import Packet, MiscSignature

packet/routes/shared.py

@@ -45,6 +45,13 @@ def freshman_packet(packet_id, info=None):
                                upper=filter(lambda sig: not sig.eboard, packet.upper_signatures))
 
 
+def packet_sort_key(packet):
+    """
+    Utility function for generating keys for sorting packets
+    """
+    return packet.signatures_received_result.total, packet.did_sign_result
+
+
 @app.route("/packets/")
 @log_cache
 @packet_auth
@@ -59,7 +66,6 @@ def packets(info=None):
         packet.signatures_received_result = packet.signatures_received()
         packet.signatures_required_result = packet.signatures_required()
 
-    open_packets.sort(key=lambda packet: packet.signatures_received_result.total, reverse=True)
-    open_packets.sort(key=lambda packet: packet.did_sign_result, reverse=True)
+    open_packets.sort(key=packet_sort_key, reverse=True)
 
     return render_template("active_packets.html", info=info, packets=open_packets)

packet/utils.py

@@ -1,23 +1,23 @@
-# Credit to Liam Middlebrook and Ram Zallan
-# https://github.com/liam-middlebrook/gallery
+"""
+General utilities and decorators for supporting the Python logic
+"""
+
 from functools import wraps, lru_cache
 
 import requests
 from flask import session
 
-from packet import _ldap, auth, app
+from packet import auth, app
 from packet.models import Freshman
-from packet.ldap import (ldap_get_member,
-                         ldap_is_active,
-                         ldap_is_onfloor,
-                         ldap_get_roomnumber,
-                         ldap_get_groups,
-                         ldap_is_intromember)
+from packet.ldap import ldap_get_member, ldap_is_intromember
 
 INTRO_REALM = "https://sso.csh.rit.edu/auth/realms/intro"
 
-
 def before_request(func):
+    """
+    Credit to Liam Middlebrook and Ram Zallan
+    https://github.com/liam-middlebrook/gallery
+    """
     @wraps(func)
     def wrapped_function(*args, **kwargs):
         uid = str(session["userinfo"].get("preferred_username", ""))
@@ -26,18 +26,12 @@ def wrapped_function(*args, **kwargs):
             info = {
                 "realm": "intro",
                 "uid": uid,
-                "onfloor": is_on_floor(uid)
+                "onfloor": is_freshman_on_floor(uid)
             }
         else:
-            uuid = str(session["userinfo"].get("sub", ""))
-            user_obj = _ldap.get_member(uid, uid=True)
             info = {
                 "realm": "csh",
-                "uuid": uuid,
-                "uid": uid,
-                "user_obj": user_obj,
-                "member_info": get_member_info(uid),
-                "color": requests.get('https://themeswitcher.csh.rit.edu/api/colour').content
+                "uid": uid
             }
 
         kwargs["info"] = info
@@ -46,25 +40,12 @@ def wrapped_function(*args, **kwargs):
     return wrapped_function
 
 
-@lru_cache(maxsize=2048)
-def get_member_info(uid):
-    account = ldap_get_member(uid)
-
-    member_info = {
-        "user_obj": account,
-        "group_list": ldap_get_groups(account),
-        "uid": account.uid,
-        "name": account.cn,
-        "active": ldap_is_active(account),
-        "onfloor": ldap_is_onfloor(account),
-        "room": ldap_get_roomnumber(account),
-    }
-    return member_info
-
-
-@lru_cache(maxsize=2048)
-def is_on_floor(uid):
-    freshman = Freshman.query.filter_by(rit_username=uid).first()
+@lru_cache(maxsize=128)
+def is_freshman_on_floor(rit_username):
+    """
+    Checks if a freshman is on floor
+    """
+    freshman = Freshman.query.filter_by(rit_username=rit_username).first()
     if freshman is not None:
         return freshman.onfloor
     else:
@@ -88,6 +69,7 @@ def wrapped_function(*args, **kwargs):
 
     return wrapped_function
 
+
 def notify_slack(name: str):
     """
     Sends a congratulate on sight decree to Slack