Skip to content

Commit f5ed679

Browse files
author
Stephen Cefali
authored
feat(auth): adds request_cache decorator to cache org member (#26165)
1 parent 318323d commit f5ed679

File tree

3 files changed

+128
-2
lines changed

3 files changed

+128
-2
lines changed

src/sentry/auth/access.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,12 @@
1919
Team,
2020
UserPermission,
2121
)
22+
from sentry.utils.request_cache import request_cache
23+
24+
25+
@request_cache
26+
def get_cached_organization_member(user_id, organization_id):
27+
return OrganizationMember.objects.get(user_id=user_id, organization_id=organization_id)
2228

2329

2430
def _sso_params(member):
@@ -279,7 +285,7 @@ def from_request(request, organization=None, scopes=None):
279285
# we special case superuser so that if they're a member of the org
280286
# they must still follow SSO checks, but they gain global access
281287
try:
282-
member = OrganizationMember.objects.get(user=request.user, organization=organization)
288+
member = get_cached_organization_member(request.user.id, organization.id)
283289
except OrganizationMember.DoesNotExist:
284290
requires_sso, sso_is_valid = False, True
285291
else:
@@ -345,7 +351,7 @@ def from_user(user, organization=None, scopes=None):
345351
return OrganizationlessAccess(permissions=UserPermission.for_user(user.id))
346352

347353
try:
348-
om = OrganizationMember.objects.get(user=user, organization=organization)
354+
om = get_cached_organization_member(user.id, organization.id)
349355
except OrganizationMember.DoesNotExist:
350356
return OrganizationlessAccess(permissions=UserPermission.for_user(user.id))
351357

src/sentry/utils/request_cache.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
import threading
2+
3+
from celery.signals import task_failure, task_success
4+
from django.core.signals import request_finished
5+
6+
from sentry import app
7+
8+
_cache = threading.local()
9+
10+
11+
def request_cache(func):
12+
"""
13+
A decorator to memoize functions on a per-request basis.
14+
Arguments to the memoized function should NOT be objects
15+
Use primitive types as arguments
16+
"""
17+
18+
def wrapped(*args, **kwargs):
19+
# if no request, skip cache
20+
if app.env.request is None:
21+
return func(*args, **kwargs)
22+
23+
if not hasattr(_cache, "items"):
24+
_cache.items = {}
25+
cache_key = (func, repr(args), repr(kwargs))
26+
if cache_key in _cache.items:
27+
rv = _cache.items[cache_key]
28+
else:
29+
rv = func(*args, **kwargs)
30+
_cache.items[cache_key] = rv
31+
return rv
32+
33+
return wrapped
34+
35+
36+
def clear_cache(**kwargs):
37+
_cache.items = {}
38+
39+
40+
request_finished.connect(clear_cache)
41+
task_failure.connect(clear_cache)
42+
task_success.connect(clear_cache)
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
from django.core.handlers.wsgi import WSGIHandler
2+
from django.core.signals import request_finished
3+
from django.http import HttpRequest
4+
from django.utils import timezone
5+
6+
from sentry import app
7+
from sentry.testutils import TestCase
8+
from sentry.utils.compat.mock import patch
9+
from sentry.utils.request_cache import clear_cache, request_cache
10+
11+
12+
@request_cache
13+
def cached_fn(arg1, arg2=None):
14+
# call this so we can mock it and see how often it's called
15+
timezone.now()
16+
if arg2:
17+
return arg1 + arg2
18+
return arg1
19+
20+
21+
class RequestCacheTest(TestCase):
22+
def setUp(self):
23+
self.original_receivers = request_finished.receivers
24+
request_finished.receivers = []
25+
request_finished.connect(clear_cache)
26+
super().setUp()
27+
28+
def tearDown(self):
29+
# remove the request, trigger the signal to clear the cache, restore receivers
30+
app.env.request = None
31+
request_finished.send(sender=WSGIHandler)
32+
request_finished.receivers = self.original_receivers
33+
super().tearDown()
34+
35+
@patch("django.utils.timezone.now")
36+
def test_basic_cache(self, mock_now):
37+
app.env.request = HttpRequest()
38+
assert cached_fn("cat", arg2="dog") == "catdog"
39+
assert cached_fn("cat", arg2="dog") == "catdog"
40+
assert mock_now.call_count == 1
41+
42+
@patch("django.utils.timezone.now")
43+
def test_cache_none(self, mock_now):
44+
app.env.request = HttpRequest()
45+
assert cached_fn(None) is None
46+
assert cached_fn(None) is None
47+
assert mock_now.call_count == 1
48+
49+
@patch("django.utils.timezone.now")
50+
def test_different_args(self, mock_now):
51+
app.env.request = HttpRequest()
52+
assert cached_fn("cat", arg2="dog") == "catdog"
53+
assert cached_fn("hey", arg2="dog") == "heydog"
54+
assert mock_now.call_count == 2
55+
56+
@patch("django.utils.timezone.now")
57+
def test_different_kwargs(self, mock_now):
58+
app.env.request = HttpRequest()
59+
assert cached_fn("cat", arg2="dog") == "catdog"
60+
assert cached_fn("cat", arg2="hat") == "cathat"
61+
assert mock_now.call_count == 2
62+
63+
@patch("django.utils.timezone.now")
64+
def test_different_request(self, mock_now):
65+
app.env.request = HttpRequest()
66+
assert cached_fn("cat") == "cat"
67+
request_finished.send(sender=WSGIHandler)
68+
app.env.request = HttpRequest()
69+
assert cached_fn("cat") == "cat"
70+
assert mock_now.call_count == 2
71+
72+
@patch("django.utils.timezone.now")
73+
def test_request_over(self, mock_now):
74+
app.env.request = HttpRequest()
75+
assert cached_fn("cat") == "cat"
76+
app.env.request = None
77+
assert cached_fn("cat") == "cat"
78+
assert mock_now.call_count == 2

0 commit comments

Comments
 (0)