From cb78ddc491a147c3a78fe1218f68198396e49709 Mon Sep 17 00:00:00 2001 From: Tim Abbott Date: Sun, 17 Mar 2019 14:19:53 -0700 Subject: [PATCH] models: Fix performance of supported_auth_backends with caching. See the comment, but this is a significant performance optimization for all of our pages using common_context, because this code path is called more than a dozen times (recursively) by common_context. --- zerver/lib/test_classes.py | 2 ++ zerver/models.py | 18 +++++++++++++++++- zerver/tests/test_auth_backends.py | 7 +++++-- 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/zerver/lib/test_classes.py b/zerver/lib/test_classes.py index f470379778..df203b44c2 100644 --- a/zerver/lib/test_classes.py +++ b/zerver/lib/test_classes.py @@ -38,6 +38,7 @@ from zerver.lib.test_helpers import ( ) from zerver.models import ( + clear_supported_auth_backends_cache, get_stream, get_client, get_display_recipient, @@ -94,6 +95,7 @@ class ZulipTestCase(TestCase): super().tearDown() # Important: we need to clear event queues to avoid leaking data to future tests. clear_client_event_queues_for_testing() + clear_supported_auth_backends_cache() ''' WRAPPER_COMMENT: diff --git a/zerver/models.py b/zerver/models.py index 5a9c610983..1314dace21 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -133,6 +133,22 @@ def get_realm_emoji_cache_key(realm: 'Realm') -> str: def get_active_realm_emoji_cache_key(realm: 'Realm') -> str: return u'active_realm_emoji:%s' % (realm.id,) +# This simple call-once caching saves ~500us in auth_enabled_helper, +# which is a significant optimization for common_context. Note that +# these values cannot change in a running production system, but do +# regularly change within unit tests; we address the latter by calling +# clear_supported_auth_backends_cache in our standard tearDown code. +supported_backends = None # type: Optional[Set[type]] +def supported_auth_backends() -> Set[type]: + global supported_backends + if supported_backends is None: + supported_backends = django.contrib.auth.get_backends() + return supported_backends + +def clear_supported_auth_backends_cache() -> None: + global supported_backends + supported_backends = None + class Realm(models.Model): MAX_REALM_NAME_LENGTH = 40 MAX_REALM_SUBDOMAIN_LENGTH = 40 @@ -339,7 +355,7 @@ class Realm(models.Model): from zproject.backends import AUTH_BACKEND_NAME_MAP ret = {} # type: Dict[str, bool] - supported_backends = {backend.__class__ for backend in django.contrib.auth.get_backends()} + supported_backends = [backend.__class__ for backend in supported_auth_backends()] for k, v in self.authentication_methods.iteritems(): backend = AUTH_BACKEND_NAME_MAP[k] if backend in supported_backends: diff --git a/zerver/tests/test_auth_backends.py b/zerver/tests/test_auth_backends.py index 01d568fb06..e216aba21f 100644 --- a/zerver/tests/test_auth_backends.py +++ b/zerver/tests/test_auth_backends.py @@ -43,7 +43,8 @@ from zerver.lib.test_classes import ( ) from zerver.models import \ get_realm, email_to_username, CustomProfileField, CustomProfileFieldValue, \ - UserProfile, PreregistrationUser, Realm, RealmDomain, get_user, MultiuseInvite + UserProfile, PreregistrationUser, Realm, RealmDomain, get_user, MultiuseInvite, \ + clear_supported_auth_backends_cache from zerver.signals import JUST_CREATED_THRESHOLD from confirmation.models import Confirmation, create_confirmation_link @@ -78,7 +79,7 @@ class AuthBackendTest(ZulipTestCase): return username def verify_backend(self, backend: Any, good_kwargs: Optional[Dict[str, Any]]=None, bad_kwargs: Optional[Dict[str, Any]]=None) -> None: - + clear_supported_auth_backends_cache() user_profile = self.example_user('hamlet') assert good_kwargs is not None @@ -116,7 +117,9 @@ class AuthBackendTest(ZulipTestCase): # Verify auth fails if the auth backend is disabled on server with self.settings(AUTHENTICATION_BACKENDS=('zproject.backends.ZulipDummyBackend',)): + clear_supported_auth_backends_cache() self.assertIsNone(backend.authenticate(**good_kwargs)) + clear_supported_auth_backends_cache() # Verify auth fails if the auth backend is disabled for the realm for backend_name in AUTH_BACKEND_NAME_MAP.keys():