Skip to content

Commit 78c8092

Browse files
authored
[Comp-792] fix caching versions for multiple pods (#712)
1 parent 9e809fc commit 78c8092

File tree

4 files changed

+93
-71
lines changed

4 files changed

+93
-71
lines changed

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

Lines changed: 41 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,21 @@
1414
class AuthService:
1515
"""Handle service request for epic.authorize with integrated cache management."""
1616

17+
AUTH_CACHE_VERSION_KEY = "auth_cache_version"
18+
19+
@classmethod
20+
def _get_auth_cache_version(cls):
21+
"""
22+
Get current auth cache version from shared cache.
23+
24+
This ensures all pods see the same version number.
25+
"""
26+
version = cache.get(cls.AUTH_CACHE_VERSION_KEY)
27+
if version is None:
28+
version = 0
29+
cache.set(cls.AUTH_CACHE_VERSION_KEY, version)
30+
return version
31+
1732
@staticmethod
1833
def get_epic_user_by_guid(auth_user_guid: str):
1934
"""
@@ -24,9 +39,10 @@ def get_epic_user_by_guid(auth_user_guid: str):
2439
"""
2540
from compliance_api.services.cached_staff_user import CachedStaffUserService
2641

27-
# Include token hash in cache key for security
42+
# Include version AND token hash in cache key
43+
version = AuthService._get_auth_cache_version()
2844
token_hash = CachedStaffUserService._get_token_hash()
29-
cache_key = f"auth_user:{auth_user_guid}:{token_hash}"
45+
cache_key = f"auth_user:{auth_user_guid}:{token_hash}:v{version}"
3046

3147
cached_result = cache.get(cache_key)
3248
if cached_result is not None:
@@ -56,9 +72,10 @@ def get_epic_users_by_app():
5672
from compliance_api.utils.constant import AUTH_APP
5773
from compliance_api.exceptions import BusinessError
5874

59-
# Include token hash in cache key for security
75+
# Include version AND token hash in cache key
76+
version = AuthService._get_auth_cache_version()
6077
token_hash = CachedStaffUserService._get_token_hash()
61-
cache_key = f"auth_users_app:{AUTH_APP}:{token_hash}"
78+
cache_key = f"auth_users_app:{AUTH_APP}:{token_hash}:v{version}"
6279

6380
cached_result = cache.get(cache_key)
6481
if cached_result is not None:
@@ -91,11 +108,8 @@ def update_user_group(auth_user_guid: str, payload: dict):
91108
f"Update group in the auth server failed for user : {auth_user_guid}"
92109
)
93110

94-
# Invalidate auth caches for this specific user
95-
AuthService._invalidate_auth_user_cache(auth_user_guid)
96-
97-
# Invalidate the "all users by app" cache since this user's groups changed
98-
AuthService._invalidate_auth_users_by_app_cache()
111+
# Invalidate all auth caches by bumping version
112+
AuthService._invalidate_all_auth_cache()
99113

100114
# Invalidate ALL staff caches since permissions changed
101115
CachedStaffUserService.invalidate_staff_cache(auth_user_guid)
@@ -116,50 +130,30 @@ def delete_user_group(auth_user_guid: str, group: str, del_sub_group_mappings=Tr
116130
if delete_response.status_code != 204:
117131
raise BusinessError("Delete group mapping failed")
118132

119-
# Invalidate auth caches for this specific user
120-
AuthService._invalidate_auth_user_cache(auth_user_guid)
121-
122-
# Invalidate the "all users by app" cache since this user's groups changed
123-
AuthService._invalidate_auth_users_by_app_cache()
133+
# Invalidate all auth caches by bumping version
134+
AuthService._invalidate_all_auth_cache()
124135

125-
# Invalidate ALL staff caches since permissions changed
136+
# Invalidate all staff caches since permissions changed
126137
CachedStaffUserService.invalidate_staff_cache(auth_user_guid)
127138

128139
return delete_response
129140

130141
@staticmethod
131-
def _invalidate_auth_user_cache(auth_user_guid: str):
132-
"""
133-
Invalidate the individual auth user cache.
134-
135-
Since cache keys include token hashes, we can't delete all variations.
136-
Instead, we use a pattern-based approach or accept that cache will expire naturally.
137-
"""
138-
from compliance_api.services.cached_staff_user import CachedStaffUserService
139-
140-
# Get current token hash
141-
token_hash = CachedStaffUserService._get_token_hash()
142-
cache_key = f"auth_user:{auth_user_guid}:{token_hash}"
143-
144-
cache.delete(cache_key)
145-
current_app.logger.info(f"Invalidated auth user cache for {auth_user_guid}")
146-
147-
@staticmethod
148-
def _invalidate_auth_users_by_app_cache():
149-
"""
150-
Invalidate the "all users by app" cache.
151-
152-
Since cache keys include token hashes, we can't delete all variations.
153-
Instead, we delete for the current token.
154-
"""
155-
from compliance_api.services.cached_staff_user import CachedStaffUserService
156-
157-
# Get current token hash
158-
token_hash = CachedStaffUserService._get_token_hash()
159-
cache_key = f"auth_users_app:{AUTH_APP}:{token_hash}"
160-
161-
cache.delete(cache_key)
162-
current_app.logger.info("Invalidated auth users by app cache")
142+
def _invalidate_all_auth_cache():
143+
"""Invalidate all auth cache by bumping version number."""
144+
try:
145+
current_version = cache.get(AuthService.AUTH_CACHE_VERSION_KEY)
146+
if current_version is None:
147+
current_version = 0
148+
149+
new_version = current_version + 1
150+
cache.set(AuthService.AUTH_CACHE_VERSION_KEY, new_version)
151+
152+
current_app.logger.debug(
153+
f"Bumped auth cache version from {current_version} to {new_version}"
154+
)
155+
except (AttributeError, RuntimeError) as e:
156+
current_app.logger.error(f"Error invalidating auth cache: {e}")
163157

164158

165159
def _request_auth_service(

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

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,20 @@ class CachedStaffUserService:
1616
STAFF_CACHE_KEY_PREFIX = "staff_user:"
1717
ALL_STAFF_CACHE_KEY = "all_staff_users"
1818
ALL_STAFF_WITH_AUTH_PREFIX = "all_staff_with_auth:"
19-
CACHE_VERSION_KEY = "all_staff_with_auth_version"
19+
CACHE_VERSION_KEY = "staff_cache_version"
20+
21+
@classmethod
22+
def _get_cache_version(cls):
23+
"""
24+
Get current cache version from shared cache.
25+
26+
This ensures all pods see the same version number.
27+
"""
28+
version = cache.get(cls.CACHE_VERSION_KEY)
29+
if version is None:
30+
version = 0
31+
cache.set(cls.CACHE_VERSION_KEY, version)
32+
return version
2033

2134
@classmethod
2235
def exists_staff_by_auth_guid(cls, auth_guid: str) -> bool:
@@ -35,7 +48,9 @@ def exists_staff_by_auth_guid(cls, auth_guid: str) -> bool:
3548
if not auth_guid:
3649
return False
3750

38-
cache_key = f"{cls.STAFF_CACHE_KEY_PREFIX}{auth_guid}"
51+
# Include version in cache key
52+
version = cls._get_cache_version()
53+
cache_key = f"{cls.STAFF_CACHE_KEY_PREFIX}{auth_guid}:v{version}"
3954

4055
cached_staff_exists = cache.get(cache_key)
4156
if cached_staff_exists is not None:
@@ -63,10 +78,8 @@ def get_all_staff_users_with_auth():
6378
# pylint: disable=import-outside-toplevel
6479
from compliance_api.services.staff_user import _set_permission_level_in_compliance_user_obj
6580

66-
# Get current version
67-
version = cache.get(CachedStaffUserService.CACHE_VERSION_KEY)
68-
if version is None:
69-
version = 0
81+
# Get current version from shared cache
82+
version = CachedStaffUserService._get_cache_version()
7083

7184
# Create cache key that includes token hash AND version
7285
token_hash = CachedStaffUserService._get_token_hash()
@@ -111,20 +124,18 @@ def invalidate_staff_cache(auth_guid: str = None):
111124
Args:
112125
auth_guid: Specific auth_guid to invalidate, or None to clear all staff cache
113126
"""
127+
# Bump version to invalidate all caches across all pods
128+
CachedStaffUserService._invalidate_all_staff_cache()
129+
114130
if auth_guid:
115-
cache_key = f"{CachedStaffUserService.STAFF_CACHE_KEY_PREFIX}{auth_guid}"
116-
cache.delete(cache_key)
117131
current_app.logger.info(f"Invalidated cache for staff user: {auth_guid}")
118132

119-
# Always invalidate the aggregate cache when any staff user changes
120-
CachedStaffUserService._invalidate_all_staff_with_auth_cache()
121-
122133
@staticmethod
123-
def _invalidate_all_staff_with_auth_cache():
134+
def _invalidate_all_staff_cache():
124135
"""
125-
Invalidate all staff+auth cache by bumping version number.
136+
Invalidate all staff cache by bumping version number.
126137
127-
This invalidates cache for all tokens/users since the version
138+
This invalidates cache for across pods since the version
128139
is part of every cache key.
129140
"""
130141
try:
@@ -135,10 +146,10 @@ def _invalidate_all_staff_with_auth_cache():
135146
new_version = current_version + 1
136147
cache.set(CachedStaffUserService.CACHE_VERSION_KEY, new_version)
137148
current_app.logger.info(
138-
f"Bumped staff+auth cache version from {current_version} to {new_version}"
149+
f"Bumped staff cache version from {current_version} to {new_version}"
139150
)
140151
except (AttributeError, RuntimeError) as e:
141-
current_app.logger.error(f"Error invalidating staff+auth cache: {e}")
152+
current_app.logger.error(f"Error invalidating staff cache: {e}")
142153

143154
@classmethod
144155
def _clear_all_staff_cache(cls):

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,10 @@ def update_user(user_id, user_data):
146146
if new_auth_guid and new_auth_guid != existing_user.auth_user_guid:
147147
CachedStaffUserService.invalidate_staff_cache(new_auth_guid)
148148

149+
# Return the latest user object
150+
final_auth_guid = new_auth_guid if new_auth_guid else existing_user.auth_user_guid
151+
return StaffUserService.get_user_by_auth_guid(final_auth_guid)
152+
149153
return updated_user
150154

151155
@staticmethod

compliance-api/tests/unit/services/test_cached_staff_user.py

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,8 @@ def test_exists_staff_by_auth_guid_cache_miss(self):
3939
assert exists is True
4040

4141
# Verify cache was populated
42-
cache_key = f"{CachedStaffUserService.STAFF_CACHE_KEY_PREFIX}{auth_guid}"
42+
version = CachedStaffUserService._get_cache_version()
43+
cache_key = f"{CachedStaffUserService.STAFF_CACHE_KEY_PREFIX}{auth_guid}:v{version}"
4344
cached_value = cache.get(cache_key)
4445
assert cached_value is True
4546

@@ -86,20 +87,32 @@ def test_invalidate_staff_cache_specific_user(self):
8687
exists1 = CachedStaffUserService.exists_staff_by_auth_guid(auth_guid)
8788
assert exists1 is True
8889

89-
# Verify cache exists
90-
cache_key = f"{CachedStaffUserService.STAFF_CACHE_KEY_PREFIX}{auth_guid}"
91-
assert cache.get(cache_key) is True
90+
# Get the version and verify cache exists
91+
version_before = CachedStaffUserService._get_cache_version()
92+
cache_key_before = f"{CachedStaffUserService.STAFF_CACHE_KEY_PREFIX}{auth_guid}:v{version_before}"
93+
assert cache.get(cache_key_before) is True
9294

93-
# Invalidate specific user cache
95+
# Bumps the version
9496
CachedStaffUserService.invalidate_staff_cache(auth_guid)
9597

96-
# Verify cache was cleared
97-
assert cache.get(cache_key) is None
98+
# Verify version was bumped
99+
version_after = CachedStaffUserService._get_cache_version()
100+
assert version_after == version_before + 1
98101

99-
# Should still work (will fetch from database)
102+
# Verify old cache key still exists (not deleted, just obsolete)
103+
assert cache.get(cache_key_before) is True
104+
105+
# Verify new cache key doesn't exist yet
106+
cache_key_after = f"{CachedStaffUserService.STAFF_CACHE_KEY_PREFIX}{auth_guid}:v{version_after}"
107+
assert cache.get(cache_key_after) is None
108+
109+
# Should still fetch from database with new version
100110
exists2 = CachedStaffUserService.exists_staff_by_auth_guid(auth_guid)
101111
assert exists2 is True
102112

113+
# Verify new cache was populated
114+
assert cache.get(cache_key_after) is True
115+
103116
def test_invalidate_staff_cache_all_users(self):
104117
"""Test invalidating all staff cache."""
105118
# Populate cache for both users

0 commit comments

Comments
 (0)