[Comp-754] Fix table latency for staff_users and sync with My toggle (#653)

Author: tolkamps1

compliance-api/src/compliance_api/models/staff_user.py

@@ -8,7 +8,7 @@
 from typing import Optional
 
 from sqlalchemy import Boolean, Column, ForeignKey, Index, Integer, String
-from sqlalchemy.orm import relationship
+from sqlalchemy.orm import joinedload, relationship
 
 from .base_model import BaseModelVersioned
 from .utils import with_session
@@ -52,10 +52,16 @@ class StaffUser(BaseModelVersioned):
     )
     position = relationship("Position", foreign_keys=[position_id], lazy="joined")
     deputy_director = relationship(
-        "StaffUser", remote_side=[id], foreign_keys=[deputy_director_id]
+        "StaffUser",
+        remote_side=[id],
+        foreign_keys=[deputy_director_id],
+        lazy="selectin"
     )
     supervisor = relationship(
-        "StaffUser", remote_side=[id], foreign_keys=[supervisor_id]
+        "StaffUser",
+        remote_side=[id],
+        foreign_keys=[supervisor_id],
+        lazy="selectin"
     )
     is_deleted = Column(Boolean, default=False, server_default="f", nullable=False)
     __table_args__ = (
@@ -107,3 +113,31 @@ def delete_staff_user(cls, staff_user_id, session=None):
         user.is_active = False
         session.flush()
         return user
+
+    @classmethod
+    def get_all_with_relationships(cls, default_filters=True, sort_by=None):
+        """
+        Fetch all staff users with eager loading of relationships.
+
+        This prevents DetachedInstanceError when accessing relationships
+        after objects are cached or detached from the session.
+        """
+        query = cls.query
+
+        # Apply filters
+        if default_filters:
+            query = query.filter_by(is_active=True)
+        query = query.filter_by(is_deleted=False)
+
+        # Eager load all relationships to prevent lazy loading issues
+        query = query.options(
+            joinedload(cls.position),
+            joinedload(cls.deputy_director),
+            joinedload(cls.supervisor)
+        )
+
+        # Apply sorting
+        if sort_by and hasattr(cls, sort_by):
+            query = query.order_by(getattr(cls, sort_by))
+
+        return query.all()

compliance-api/src/compliance_api/resources/staff_user.py

@@ -20,6 +20,7 @@
 from compliance_api.auth import auth
 from compliance_api.exceptions import ResourceNotFoundError
 from compliance_api.schemas import KeyValueSchema, StaffUserCreateSchema, StaffUserSchema, StaffUserUpdateSchema
+from compliance_api.services.cached_staff_user import CachedStaffUserService
 from compliance_api.services import StaffUserService
 from compliance_api.utils.enum import PermissionEnum
 from compliance_api.utils.util import cors_preflight
@@ -54,9 +55,8 @@ class StaffUsers(Resource):
     @auth.require
     def get():
         """Fetch all users."""
-        users = StaffUserService.get_all_staff_users()
-        user_list_schema = StaffUserSchema(many=True)
-        return user_list_schema.dump(users), HTTPStatus.OK
+        users = CachedStaffUserService.get_all_staff_users_with_auth()
+        return users, HTTPStatus.OK
 
     @staticmethod
     @auth.require

compliance-api/src/compliance_api/services/authorize_service/auth_service.py

@@ -4,54 +4,115 @@
 from flask import current_app, g
 
 from compliance_api.exceptions import BusinessError
+from compliance_api.utils.cache import cache
 from compliance_api.utils.constant import AUTH_APP
 from compliance_api.utils.enum import HttpMethod
 
 from .constant import API_REQUEST_TIMEOUT
 
 
 class AuthService:
-    """Handle service request for epic.authorize."""
+    """Handle service request for epic.authorize with integrated cache management."""
 
     @staticmethod
     def get_epic_user_by_guid(auth_user_guid: str):
-        """Return the user representation from epic.authorize."""
+        """
+        Return the user representation from epic.authorize (cached).
+
+        This is called frequently for individual user lookups,
+        so we cache it with the user's token hash for security.
+        """
+        from compliance_api.services.cached_staff_user import CachedStaffUserService
+
+        # Include token hash in cache key for security
+        token_hash = CachedStaffUserService._get_token_hash()
+        cache_key = f"auth_user:{auth_user_guid}:{token_hash}"
+
+        cached_result = cache.get(cache_key)
+        if cached_result is not None:
+            current_app.logger.debug(f"Cache hit for auth user {auth_user_guid}")
+            return cached_result
+
+        current_app.logger.debug(f"Cache miss for auth user {auth_user_guid}")
+
         auth_user_response = _request_auth_service(f"users/{auth_user_guid}")
         if auth_user_response.status_code != 200:
             raise BusinessError(
                 f"Error finding user with ID {auth_user_guid} from auth server"
             )
-        return auth_user_response.json()
+
+        result = auth_user_response.json()
+        cache.set(cache_key, result, timeout=180)  # 3 minutes
+        return result
 
     @staticmethod
     def get_epic_users_by_app():
-        """Return the users belong to COMPLIANCE app in identity server."""
+        """
+        Return all users belonging to COMPLIANCE app with caching.
+
+        This caches per-user to prevent data leakage.
+        """
+        from compliance_api.services.cached_staff_user import CachedStaffUserService
+        from compliance_api.utils.constant import AUTH_APP
+        from compliance_api.exceptions import BusinessError
+
+        # Include token hash in cache key for security
+        token_hash = CachedStaffUserService._get_token_hash()
+        cache_key = f"auth_users_app:{AUTH_APP}:{token_hash}"
+
+        cached_result = cache.get(cache_key)
+        if cached_result is not None:
+            current_app.logger.debug("Cache hit for auth users by app")
+            return cached_result
+
+        current_app.logger.debug("Cache miss for auth users by app")
+
+        # Fetch from auth service
         auth_users_response = _request_auth_service(f"users?app_name={AUTH_APP}")
         if auth_users_response.status_code != 200:
             raise BusinessError(f"Error fetching users for the app {AUTH_APP}")
-        return auth_users_response.json()
+
+        result = auth_users_response.json()
+        cache.set(cache_key, result, timeout=180)  # 3 minutes
+        return result
 
     @staticmethod
     def update_user_group(auth_user_guid: str, payload: dict):
-        """Update the group of the user in the identity server."""
+        """Update user group and invalidate all related caches."""
+        from compliance_api.services.cached_staff_user import CachedStaffUserService
+        from compliance_api.utils.enum import HttpMethod
+        from compliance_api.exceptions import BusinessError
+
         update_group_response = _request_auth_service(
             f"users/{auth_user_guid}/groups", HttpMethod.PUT, payload
         )
         if update_group_response.status_code != 204:
             raise BusinessError(
                 f"Update group in the auth server failed for user : {auth_user_guid}"
             )
+
+        # Invalidate ALL staff caches since permissions changed
+        CachedStaffUserService.invalidate_staff_cache(auth_user_guid)
+
         return update_group_response
 
     @staticmethod
     def delete_user_group(auth_user_guid: str, group: str, del_sub_group_mappings=True):
-        """Delete the user-group mapping in identity server."""
+        """Delete user group and invalidate all related caches."""
+        from compliance_api.services.cached_staff_user import CachedStaffUserService
+        from compliance_api.utils.enum import HttpMethod
+        from compliance_api.exceptions import BusinessError
+
         delete_response = _request_auth_service(
             f"users/{auth_user_guid}/groups/{group}?del_sub_group_mappings={del_sub_group_mappings}",
             HttpMethod.DELETE,
         )
         if delete_response.status_code != 204:
             raise BusinessError("Delete group mapping failed")
+
+        # Invalidate ALL staff caches since permissions changed
+        CachedStaffUserService.invalidate_staff_cache(auth_user_guid)
+
         return delete_response
 
 

compliance-api/src/compliance_api/services/cached_staff_user.py

@@ -1,8 +1,11 @@
-"""Cached service for staff user validation during token authentication."""
+"""Integrated caching strategy for staff users."""
 
-from flask import current_app
+import hashlib
+from flask import current_app, g
 
 from compliance_api.models.staff_user import StaffUser as StaffUserModel
+from compliance_api.schemas.staff_user import StaffUserSchema
+from compliance_api.services.authorize_service.auth_service import AuthService
 from compliance_api.utils.cache import cache
 
 
@@ -12,6 +15,7 @@ class CachedStaffUserService:
     CACHE_TIMEOUT = 3600  # 1 hour
     STAFF_CACHE_KEY_PREFIX = "staff_user:"
     ALL_STAFF_CACHE_KEY = "all_staff_users"
+    ALL_STAFF_WITH_AUTH_PREFIX = "all_staff_with_auth:"
 
     @classmethod
     def exists_staff_by_auth_guid(cls, auth_guid: str) -> bool:
@@ -47,21 +51,84 @@ def exists_staff_by_auth_guid(cls, auth_guid: str) -> bool:
 
         return cache_value
 
-    @classmethod
-    def invalidate_staff_cache(cls, auth_guid: str = None):
+    @staticmethod
+    def get_all_staff_users_with_auth():
+        """
+        Get all staff users with merged auth data (cached as serialized data).
+
+        Returns:
+            List of serialized staff user dictionaries
+        """
+        # pylint: disable=import-outside-toplevel
+        from compliance_api.services.staff_user import _set_permission_level_in_compliance_user_obj
+
+        # Create cache key that includes token hash for security
+        token_hash = CachedStaffUserService._get_token_hash()
+
+        version = cache.get("all_staff_with_auth_version")
+        if version is None:
+            version = 0
+
+        cache_key = (
+            f"{CachedStaffUserService.ALL_STAFF_WITH_AUTH_PREFIX}"
+            f"{token_hash}:v{version}"
+        )
+
+        cached_result = cache.get(cache_key)
+        if cached_result is not None:
+            current_app.logger.debug("Cache hit for all staff users with auth data")
+            return cached_result
+
+        current_app.logger.debug("Cache miss for all staff users with auth data")
+
+        # Get users from compliance database with eager loading
+        users = StaffUserModel.get_all_with_relationships(default_filters=False)
+
+        # Get compliance users from epic system
+        auth_users = AuthService.get_epic_users_by_app()
+
+        # Merge the two sets of users to set the permission in the result
+        index_auth_users = {user["username"]: user for user in auth_users}
+        for user in users:
+            auth_user = index_auth_users.get(user.auth_user_guid, None)
+            user = _set_permission_level_in_compliance_user_obj(user, auth_user)
+
+        # Serialize the data BEFORE caching to avoid detached instance issues
+        user_schema = StaffUserSchema(many=True)
+        serialized_users = user_schema.dump(users)
+
+        # Cache the serialized data
+        cache.set(cache_key, serialized_users, timeout=300)  # 5 minutes
+
+        return serialized_users
+
+    @staticmethod
+    def invalidate_staff_cache(auth_guid: str = None):
         """
         Invalidate cached staff user data.
 
         Args:
             auth_guid: Specific auth_guid to invalidate, or None to clear all staff cache
         """
         if auth_guid:
-            cache_key = f"{cls.STAFF_CACHE_KEY_PREFIX}{auth_guid}"
+            cache_key = f"{CachedStaffUserService.STAFF_CACHE_KEY_PREFIX}{auth_guid}"
             cache.delete(cache_key)
             current_app.logger.info(f"Invalidated cache for staff user: {auth_guid}")
-        else:
-            cls._clear_all_staff_cache()
-            current_app.logger.info("Invalidated all staff user cache")
+
+        # Always invalidate the aggregate cache when any staff user changes
+        CachedStaffUserService._invalidate_all_staff_with_auth_cache()
+
+    @staticmethod
+    def _invalidate_all_staff_with_auth_cache():
+        try:
+            current_version = cache.get("all_staff_with_auth_version")
+            if current_version is None:
+                current_version = 0
+
+            cache.set("all_staff_with_auth_version", current_version + 1)
+            current_app.logger.info("Bumped staff+auth cache version")
+        except (AttributeError, RuntimeError) as e:
+            current_app.logger.error(f"Error invalidating staff+auth cache: {e}")
 
     @classmethod
     def _clear_all_staff_cache(cls):
@@ -71,3 +138,19 @@ def _clear_all_staff_cache(cls):
             current_app.logger.info("Cleared all cache (simple cache type)")
         except (AttributeError, RuntimeError) as e:
             current_app.logger.error(f"Error clearing cache: {str(e)}")
+
+    @staticmethod
+    def _get_token_hash():
+        """
+        Generate a cache key suffix based on the current user's token.
+
+        This ensures different users get different cached data while
+        not exposing the actual token in cache keys.
+        """
+        token = getattr(g, "access_token", None)
+        if not token:
+            return "no_token"
+
+        # Use first 16 chars of SHA256 hash
+        token_hash = hashlib.sha256(token.encode()).hexdigest()[:16]
+        return token_hash