From a842921e1fe756e84f5525db56225556c2b404ef Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Tue, 22 Jan 2019 21:21:27 -0500 Subject: [PATCH] Cache org-site lookups in the RequestCache --- lms/djangoapps/course_api/tests/test_views.py | 3 +- .../djangoapps/config_model_utils/models.py | 3 ++ openedx/core/lib/cache_utils.py | 43 ++++++++----------- .../content_type_gating/tests/test_models.py | 37 ++++++++++++++++ .../tests/test_models.py | 37 ++++++++++++++++ 5 files changed, 98 insertions(+), 25 deletions(-) diff --git a/lms/djangoapps/course_api/tests/test_views.py b/lms/djangoapps/course_api/tests/test_views.py index 5541e511ca..0acba1b764 100644 --- a/lms/djangoapps/course_api/tests/test_views.py +++ b/lms/djangoapps/course_api/tests/test_views.py @@ -392,12 +392,13 @@ class CourseListSearchViewTest(CourseApiTestViewMixin, ModuleStoreTestCase, Sear self.setup_user(self.audit_user) # These query counts were found empirically - query_counts = [174, 196, 226, 256, 286, 316, 346, 376, 406, 436, 331] + query_counts = [122, 140, 170, 200, 230, 260, 290, 320, 350, 380, 327] ordered_course_ids = sorted([str(cid) for cid in (course_ids + [c.id for c in self.courses])]) self.clear_caches() for page in range(1, 12): + RequestCache.clear_all_namespaces() with self.assertNumQueries(query_counts[page - 1]): response = self.verify_response(params={'page': page, 'page_size': 30}) diff --git a/openedx/core/djangoapps/config_model_utils/models.py b/openedx/core/djangoapps/config_model_utils/models.py index 7e47db7d1b..2d29ac3605 100644 --- a/openedx/core/djangoapps/config_model_utils/models.py +++ b/openedx/core/djangoapps/config_model_utils/models.py @@ -22,6 +22,7 @@ import crum from config_models.models import ConfigurationModel, cache from openedx.core.djangoapps.site_configuration.models import SiteConfiguration from openedx.core.djangoapps.content.course_overviews.models import CourseOverview +from openedx.core.lib.cache_utils import request_cached class Provenance(Enum): @@ -252,7 +253,9 @@ class StackedConfigurationModel(ConfigurationModel): return course_key.org @classmethod + @request_cached() def _site_from_org(cls, org): + configuration = SiteConfiguration.get_configuration_for_org(org, select_related=['site']) if configuration is None: try: diff --git a/openedx/core/lib/cache_utils.py b/openedx/core/lib/cache_utils.py index ba4324143d..3910d33f7c 100644 --- a/openedx/core/lib/cache_utils.py +++ b/openedx/core/lib/cache_utils.py @@ -9,6 +9,7 @@ import zlib from django.utils.encoding import force_text from edx_django_utils.cache import RequestCache +import wrapt def request_cached(namespace=None, arg_map_function=None, request_cache_getter=None): @@ -47,38 +48,32 @@ def request_cached(namespace=None, arg_map_function=None, request_cache_getter=N cache the value it returns, and return that cached value for subsequent calls with the same args/kwargs within a single request. """ - def decorator(f): + @wrapt.decorator + def decorator(wrapped, instance, args, kwargs): """ Arguments: - f (func): the function to wrap + args, kwargs: values passed into the wrapped function """ - @functools.wraps(f) - def _decorator(*args, **kwargs): - """ - Arguments: - args, kwargs: values passed into the wrapped function - """ - # Check to see if we have a result in cache. If not, invoke our wrapped - # function. Cache and return the result to the caller. - if request_cache_getter: - request_cache = request_cache_getter(args, kwargs) - else: - request_cache = RequestCache(namespace) + # Check to see if we have a result in cache. If not, invoke our wrapped + # function. Cache and return the result to the caller. + if request_cache_getter: + request_cache = request_cache_getter(args if instance is None else (instance,) + args, kwargs) + else: + request_cache = RequestCache(namespace) - if request_cache: - cache_key = _func_call_cache_key(f, arg_map_function, *args, **kwargs) - cached_response = request_cache.get_cached_response(cache_key) - if cached_response.is_found: - return cached_response.value + if request_cache: + cache_key = _func_call_cache_key(wrapped, arg_map_function, *args, **kwargs) + cached_response = request_cache.get_cached_response(cache_key) + if cached_response.is_found: + return cached_response.value - result = f(*args, **kwargs) + result = wrapped(*args, **kwargs) - if request_cache: - request_cache.set(cache_key, result) + if request_cache: + request_cache.set(cache_key, result) - return result + return result - return _decorator return decorator diff --git a/openedx/features/content_type_gating/tests/test_models.py b/openedx/features/content_type_gating/tests/test_models.py index e51a16b529..aca44f17bc 100644 --- a/openedx/features/content_type_gating/tests/test_models.py +++ b/openedx/features/content_type_gating/tests/test_models.py @@ -6,6 +6,7 @@ from django.utils import timezone from mock import Mock import pytz +from edx_django_utils.cache import RequestCache from opaque_keys.edx.locator import CourseLocator from openedx.core.djangoapps.config_model_utils.models import Provenance from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory @@ -233,10 +234,14 @@ class TestContentTypeGatingConfig(CacheIsolationTestCase): global_config = ContentTypeGatingConfig(enabled=True, enabled_as_of=datetime(2018, 1, 1)) global_config.save() + RequestCache.clear_all_namespaces() + # Check that the global value is not retrieved from cache after save with self.assertNumQueries(1): self.assertTrue(ContentTypeGatingConfig.current().enabled) + RequestCache.clear_all_namespaces() + # Check that the global value can be retrieved from cache after read with self.assertNumQueries(0): self.assertTrue(ContentTypeGatingConfig.current().enabled) @@ -244,6 +249,8 @@ class TestContentTypeGatingConfig(CacheIsolationTestCase): global_config.enabled = False global_config.save() + RequestCache.clear_all_namespaces() + # Check that the global value in cache was deleted on save with self.assertNumQueries(1): self.assertFalse(ContentTypeGatingConfig.current().enabled) @@ -253,10 +260,14 @@ class TestContentTypeGatingConfig(CacheIsolationTestCase): site_config = ContentTypeGatingConfig(site=site_cfg.site, enabled=True, enabled_as_of=datetime(2018, 1, 1)) site_config.save() + RequestCache.clear_all_namespaces() + # Check that the site value is not retrieved from cache after save with self.assertNumQueries(1): self.assertTrue(ContentTypeGatingConfig.current(site=site_cfg.site).enabled) + RequestCache.clear_all_namespaces() + # Check that the site value can be retrieved from cache after read with self.assertNumQueries(0): self.assertTrue(ContentTypeGatingConfig.current(site=site_cfg.site).enabled) @@ -264,6 +275,8 @@ class TestContentTypeGatingConfig(CacheIsolationTestCase): site_config.enabled = False site_config.save() + RequestCache.clear_all_namespaces() + # Check that the site value in cache was deleted on save with self.assertNumQueries(1): self.assertFalse(ContentTypeGatingConfig.current(site=site_cfg.site).enabled) @@ -271,6 +284,8 @@ class TestContentTypeGatingConfig(CacheIsolationTestCase): global_config = ContentTypeGatingConfig(enabled=True, enabled_as_of=datetime(2018, 1, 1)) global_config.save() + RequestCache.clear_all_namespaces() + # Check that the site value is not updated in cache by changing the global value with self.assertNumQueries(0): self.assertFalse(ContentTypeGatingConfig.current(site=site_cfg.site).enabled) @@ -281,10 +296,14 @@ class TestContentTypeGatingConfig(CacheIsolationTestCase): org_config = ContentTypeGatingConfig(org=course.org, enabled=True, enabled_as_of=datetime(2018, 1, 1)) org_config.save() + RequestCache.clear_all_namespaces() + # Check that the org value is not retrieved from cache after save with self.assertNumQueries(2): self.assertTrue(ContentTypeGatingConfig.current(org=course.org).enabled) + RequestCache.clear_all_namespaces() + # Check that the org value can be retrieved from cache after read with self.assertNumQueries(0): self.assertTrue(ContentTypeGatingConfig.current(org=course.org).enabled) @@ -292,6 +311,8 @@ class TestContentTypeGatingConfig(CacheIsolationTestCase): org_config.enabled = False org_config.save() + RequestCache.clear_all_namespaces() + # Check that the org value in cache was deleted on save with self.assertNumQueries(2): self.assertFalse(ContentTypeGatingConfig.current(org=course.org).enabled) @@ -299,6 +320,8 @@ class TestContentTypeGatingConfig(CacheIsolationTestCase): global_config = ContentTypeGatingConfig(enabled=True, enabled_as_of=datetime(2018, 1, 1)) global_config.save() + RequestCache.clear_all_namespaces() + # Check that the org value is not updated in cache by changing the global value with self.assertNumQueries(0): self.assertFalse(ContentTypeGatingConfig.current(org=course.org).enabled) @@ -306,6 +329,8 @@ class TestContentTypeGatingConfig(CacheIsolationTestCase): site_config = ContentTypeGatingConfig(site=site_cfg.site, enabled=True, enabled_as_of=datetime(2018, 1, 1)) site_config.save() + RequestCache.clear_all_namespaces() + # Check that the org value is not updated in cache by changing the site value with self.assertNumQueries(0): self.assertFalse(ContentTypeGatingConfig.current(org=course.org).enabled) @@ -316,10 +341,14 @@ class TestContentTypeGatingConfig(CacheIsolationTestCase): course_config = ContentTypeGatingConfig(course=course, enabled=True, enabled_as_of=datetime(2018, 1, 1)) course_config.save() + RequestCache.clear_all_namespaces() + # Check that the org value is not retrieved from cache after save with self.assertNumQueries(2): self.assertTrue(ContentTypeGatingConfig.current(course_key=course.id).enabled) + RequestCache.clear_all_namespaces() + # Check that the org value can be retrieved from cache after read with self.assertNumQueries(0): self.assertTrue(ContentTypeGatingConfig.current(course_key=course.id).enabled) @@ -327,6 +356,8 @@ class TestContentTypeGatingConfig(CacheIsolationTestCase): course_config.enabled = False course_config.save() + RequestCache.clear_all_namespaces() + # Check that the org value in cache was deleted on save with self.assertNumQueries(2): self.assertFalse(ContentTypeGatingConfig.current(course_key=course.id).enabled) @@ -334,6 +365,8 @@ class TestContentTypeGatingConfig(CacheIsolationTestCase): global_config = ContentTypeGatingConfig(enabled=True, enabled_as_of=datetime(2018, 1, 1)) global_config.save() + RequestCache.clear_all_namespaces() + # Check that the org value is not updated in cache by changing the global value with self.assertNumQueries(0): self.assertFalse(ContentTypeGatingConfig.current(course_key=course.id).enabled) @@ -341,6 +374,8 @@ class TestContentTypeGatingConfig(CacheIsolationTestCase): site_config = ContentTypeGatingConfig(site=site_cfg.site, enabled=True, enabled_as_of=datetime(2018, 1, 1)) site_config.save() + RequestCache.clear_all_namespaces() + # Check that the org value is not updated in cache by changing the site value with self.assertNumQueries(0): self.assertFalse(ContentTypeGatingConfig.current(course_key=course.id).enabled) @@ -348,6 +383,8 @@ class TestContentTypeGatingConfig(CacheIsolationTestCase): org_config = ContentTypeGatingConfig(org=course.org, enabled=True, enabled_as_of=datetime(2018, 1, 1)) org_config.save() + RequestCache.clear_all_namespaces() + # Check that the org value is not updated in cache by changing the site value with self.assertNumQueries(0): self.assertFalse(ContentTypeGatingConfig.current(course_key=course.id).enabled) diff --git a/openedx/features/course_duration_limits/tests/test_models.py b/openedx/features/course_duration_limits/tests/test_models.py index 6a6db087cd..584a4b210b 100644 --- a/openedx/features/course_duration_limits/tests/test_models.py +++ b/openedx/features/course_duration_limits/tests/test_models.py @@ -10,6 +10,7 @@ from django.utils import timezone from mock import Mock import pytz +from edx_django_utils.cache import RequestCache from opaque_keys.edx.locator import CourseLocator from openedx.core.djangoapps.config_model_utils.models import Provenance from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory @@ -266,10 +267,14 @@ class TestCourseDurationLimitConfig(CacheIsolationTestCase): global_config = CourseDurationLimitConfig(enabled=True, enabled_as_of=datetime(2018, 1, 1)) global_config.save() + RequestCache.clear_all_namespaces() + # Check that the global value is not retrieved from cache after save with self.assertNumQueries(1): self.assertTrue(CourseDurationLimitConfig.current().enabled) + RequestCache.clear_all_namespaces() + # Check that the global value can be retrieved from cache after read with self.assertNumQueries(0): self.assertTrue(CourseDurationLimitConfig.current().enabled) @@ -277,6 +282,8 @@ class TestCourseDurationLimitConfig(CacheIsolationTestCase): global_config.enabled = False global_config.save() + RequestCache.clear_all_namespaces() + # Check that the global value in cache was deleted on save with self.assertNumQueries(1): self.assertFalse(CourseDurationLimitConfig.current().enabled) @@ -286,10 +293,14 @@ class TestCourseDurationLimitConfig(CacheIsolationTestCase): site_config = CourseDurationLimitConfig(site=site_cfg.site, enabled=True, enabled_as_of=datetime(2018, 1, 1)) site_config.save() + RequestCache.clear_all_namespaces() + # Check that the site value is not retrieved from cache after save with self.assertNumQueries(1): self.assertTrue(CourseDurationLimitConfig.current(site=site_cfg.site).enabled) + RequestCache.clear_all_namespaces() + # Check that the site value can be retrieved from cache after read with self.assertNumQueries(0): self.assertTrue(CourseDurationLimitConfig.current(site=site_cfg.site).enabled) @@ -297,6 +308,8 @@ class TestCourseDurationLimitConfig(CacheIsolationTestCase): site_config.enabled = False site_config.save() + RequestCache.clear_all_namespaces() + # Check that the site value in cache was deleted on save with self.assertNumQueries(1): self.assertFalse(CourseDurationLimitConfig.current(site=site_cfg.site).enabled) @@ -304,6 +317,8 @@ class TestCourseDurationLimitConfig(CacheIsolationTestCase): global_config = CourseDurationLimitConfig(enabled=True, enabled_as_of=datetime(2018, 1, 1)) global_config.save() + RequestCache.clear_all_namespaces() + # Check that the site value is not updated in cache by changing the global value with self.assertNumQueries(0): self.assertFalse(CourseDurationLimitConfig.current(site=site_cfg.site).enabled) @@ -314,10 +329,14 @@ class TestCourseDurationLimitConfig(CacheIsolationTestCase): org_config = CourseDurationLimitConfig(org=course.org, enabled=True, enabled_as_of=datetime(2018, 1, 1)) org_config.save() + RequestCache.clear_all_namespaces() + # Check that the org value is not retrieved from cache after save with self.assertNumQueries(2): self.assertTrue(CourseDurationLimitConfig.current(org=course.org).enabled) + RequestCache.clear_all_namespaces() + # Check that the org value can be retrieved from cache after read with self.assertNumQueries(0): self.assertTrue(CourseDurationLimitConfig.current(org=course.org).enabled) @@ -325,6 +344,8 @@ class TestCourseDurationLimitConfig(CacheIsolationTestCase): org_config.enabled = False org_config.save() + RequestCache.clear_all_namespaces() + # Check that the org value in cache was deleted on save with self.assertNumQueries(2): self.assertFalse(CourseDurationLimitConfig.current(org=course.org).enabled) @@ -332,6 +353,8 @@ class TestCourseDurationLimitConfig(CacheIsolationTestCase): global_config = CourseDurationLimitConfig(enabled=True, enabled_as_of=datetime(2018, 1, 1)) global_config.save() + RequestCache.clear_all_namespaces() + # Check that the org value is not updated in cache by changing the global value with self.assertNumQueries(0): self.assertFalse(CourseDurationLimitConfig.current(org=course.org).enabled) @@ -339,6 +362,8 @@ class TestCourseDurationLimitConfig(CacheIsolationTestCase): site_config = CourseDurationLimitConfig(site=site_cfg.site, enabled=True, enabled_as_of=datetime(2018, 1, 1)) site_config.save() + RequestCache.clear_all_namespaces() + # Check that the org value is not updated in cache by changing the site value with self.assertNumQueries(0): self.assertFalse(CourseDurationLimitConfig.current(org=course.org).enabled) @@ -349,10 +374,14 @@ class TestCourseDurationLimitConfig(CacheIsolationTestCase): course_config = CourseDurationLimitConfig(course=course, enabled=True, enabled_as_of=datetime(2018, 1, 1)) course_config.save() + RequestCache.clear_all_namespaces() + # Check that the org value is not retrieved from cache after save with self.assertNumQueries(2): self.assertTrue(CourseDurationLimitConfig.current(course_key=course.id).enabled) + RequestCache.clear_all_namespaces() + # Check that the org value can be retrieved from cache after read with self.assertNumQueries(0): self.assertTrue(CourseDurationLimitConfig.current(course_key=course.id).enabled) @@ -360,6 +389,8 @@ class TestCourseDurationLimitConfig(CacheIsolationTestCase): course_config.enabled = False course_config.save() + RequestCache.clear_all_namespaces() + # Check that the org value in cache was deleted on save with self.assertNumQueries(2): self.assertFalse(CourseDurationLimitConfig.current(course_key=course.id).enabled) @@ -367,6 +398,8 @@ class TestCourseDurationLimitConfig(CacheIsolationTestCase): global_config = CourseDurationLimitConfig(enabled=True, enabled_as_of=datetime(2018, 1, 1)) global_config.save() + RequestCache.clear_all_namespaces() + # Check that the org value is not updated in cache by changing the global value with self.assertNumQueries(0): self.assertFalse(CourseDurationLimitConfig.current(course_key=course.id).enabled) @@ -374,6 +407,8 @@ class TestCourseDurationLimitConfig(CacheIsolationTestCase): site_config = CourseDurationLimitConfig(site=site_cfg.site, enabled=True, enabled_as_of=datetime(2018, 1, 1)) site_config.save() + RequestCache.clear_all_namespaces() + # Check that the org value is not updated in cache by changing the site value with self.assertNumQueries(0): self.assertFalse(CourseDurationLimitConfig.current(course_key=course.id).enabled) @@ -381,6 +416,8 @@ class TestCourseDurationLimitConfig(CacheIsolationTestCase): org_config = CourseDurationLimitConfig(org=course.org, enabled=True, enabled_as_of=datetime(2018, 1, 1)) org_config.save() + RequestCache.clear_all_namespaces() + # Check that the org value is not updated in cache by changing the site value with self.assertNumQueries(0): self.assertFalse(CourseDurationLimitConfig.current(course_key=course.id).enabled)