diff --git a/common/djangoapps/course_modes/views.py b/common/djangoapps/course_modes/views.py index 4848e7a440..ccbed449f9 100644 --- a/common/djangoapps/course_modes/views.py +++ b/common/djangoapps/course_modes/views.py @@ -36,7 +36,6 @@ from openedx.core.djangoapps.embargo import api as embargo_api from openedx.core.djangoapps.enrollments.permissions import ENROLL_IN_COURSE from openedx.features.content_type_gating.models import ContentTypeGatingConfig from openedx.features.course_duration_limits.models import CourseDurationLimitConfig -from openedx.features.discounts.utils import get_first_purchase_offer_banner_fragment from openedx.features.discounts.applicability import discount_percentage from student.models import CourseEnrollment from util.db import outer_atomic diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 6df0f65647..ea11664594 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -209,9 +209,17 @@ def user_by_anonymous_id(uid): if uid is None: return None + request_cache = RequestCache('user_by_anonymous_id') + cache_response = request_cache.get_cached_response(uid) + if cache_response.is_found: + return cache_response.value + try: - return User.objects.get(anonymoususerid__anonymous_user_id=uid) + user = User.objects.get(anonymoususerid__anonymous_user_id=uid) + request_cache.set(uid, user) + return user except ObjectDoesNotExist: + request_cache.set(uid, None) return None @@ -1207,13 +1215,24 @@ class CourseEnrollment(models.Model): if user.is_anonymous: return None try: + request_cache = RequestCache('get_enrollment') + if select_related: + cache_key = (user.id, course_key, ','.join(select_related)) + else: + cache_key = (user.id, course_key) + cache_response = request_cache.get_cached_response(cache_key) + if cache_response.is_found: + return cache_response.value + query = cls.objects if select_related is not None: query = query.select_related(*select_related) - return query.get( + enrollment = query.get( user=user, course_id=course_key ) + request_cache.set(cache_key, enrollment) + return enrollment except cls.DoesNotExist: return None @@ -1257,6 +1276,8 @@ class CourseEnrollment(models.Model): This saves immediately. """ + RequestCache('get_enrollment').clear() + activation_changed = False # if is_active is None, then the call to update_enrollment didn't specify # any value, so just leave is_active as it is @@ -1488,6 +1509,8 @@ class CourseEnrollment(models.Model): `skip_refund` can be set to True to avoid the refund process. """ + RequestCache('get_enrollment').clear() + try: record = cls.objects.get(user=user, course_id=course_id) record.update_enrollment(is_active=False, skip_refund=skip_refund) @@ -1509,6 +1532,8 @@ class CourseEnrollment(models.Model): `course_id` is our usual course_id string (e.g. "edX/Test101/2013_Fall) """ + RequestCache('get_enrollment').clear() + try: user = User.objects.get(email=email) return cls.unenroll(user, course_id) diff --git a/common/lib/xmodule/xmodule/video_module/video_module.py b/common/lib/xmodule/xmodule/video_module/video_module.py index 1ad75f14b8..9ca7624555 100644 --- a/common/lib/xmodule/xmodule/video_module/video_module.py +++ b/common/lib/xmodule/xmodule/video_module/video_module.py @@ -22,6 +22,7 @@ from operator import itemgetter import six from django.conf import settings +from edx_django_utils.cache import RequestCache from lxml import etree from opaque_keys.edx.locator import AssetLocator from web_fragments.fragment import Fragment @@ -202,10 +203,14 @@ class VideoBlock( def youtube_disabled_for_course(self): if not self.location.context_key.is_course: return False # Only courses have this flag - if CourseYoutubeBlockedFlag.feature_enabled(self.location.course_key): - return True - else: - return False + request_cache = RequestCache('youtube_disabled_for_course') + cache_response = request_cache.get_cached_response(self.location.context_key) + if cache_response.is_found: + return cache_response.value + + youtube_is_disabled = CourseYoutubeBlockedFlag.feature_enabled(self.location.course_key) + request_cache.set(self.location.context_key, youtube_is_disabled) + return youtube_is_disabled def prioritize_hls(self, youtube_streams, html5_sources): """ diff --git a/lms/djangoapps/ccx/tests/test_field_override_performance.py b/lms/djangoapps/ccx/tests/test_field_override_performance.py index 0b60ddc345..4cd9db66a9 100644 --- a/lms/djangoapps/ccx/tests/test_field_override_performance.py +++ b/lms/djangoapps/ccx/tests/test_field_override_performance.py @@ -244,7 +244,7 @@ class TestFieldOverrideMongoPerformance(FieldOverridePerformanceTestCase): __test__ = True # TODO: decrease query count as part of REVO-28 - QUERY_COUNT = 32 + QUERY_COUNT = 29 TEST_DATA = { # (providers, course_width, enable_ccx, view_as_ccx): ( # # of sql queries to default, @@ -273,7 +273,7 @@ class TestFieldOverrideSplitPerformance(FieldOverridePerformanceTestCase): __test__ = True # TODO: decrease query count as part of REVO-28 - QUERY_COUNT = 32 + QUERY_COUNT = 29 TEST_DATA = { ('no_overrides', 1, True, False): (QUERY_COUNT, 3), diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index 6f05a576f1..c62882bde9 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -260,8 +260,8 @@ class IndexQueryTestCase(ModuleStoreTestCase): NUM_PROBLEMS = 20 @ddt.data( - (ModuleStoreEnum.Type.mongo, 10, 182), - (ModuleStoreEnum.Type.split, 4, 178), + (ModuleStoreEnum.Type.mongo, 10, 176), + (ModuleStoreEnum.Type.split, 4, 174), ) @ddt.unpack def test_index_query_counts(self, store_type, expected_mongo_query_count, expected_mysql_query_count): @@ -1494,8 +1494,8 @@ class ProgressPageTests(ProgressPageBaseTests): self.assertContains(resp, u"Download Your Certificate") @ddt.data( - (True, 52), - (False, 51) + (True, 49), + (False, 48) ) @ddt.unpack def test_progress_queries_paced_courses(self, self_paced, query_count): @@ -1508,8 +1508,8 @@ class ProgressPageTests(ProgressPageBaseTests): @patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False}) @ddt.data( - (False, 60, 40), - (True, 51, 35) + (False, 57, 37), + (True, 48, 32) ) @ddt.unpack def test_progress_queries(self, enable_waffle, initial, subsequent): diff --git a/lms/djangoapps/discussion/tests/test_views.py b/lms/djangoapps/discussion/tests/test_views.py index 7019c629b6..dd57f67d44 100644 --- a/lms/djangoapps/discussion/tests/test_views.py +++ b/lms/djangoapps/discussion/tests/test_views.py @@ -464,18 +464,18 @@ class SingleThreadQueryCountTestCase(ForumsEnableMixin, ModuleStoreTestCase): # course is outside the context manager that is verifying the number of queries, # and with split mongo, that method ends up querying disabled_xblocks (which is then # cached and hence not queried as part of call_single_thread). - (ModuleStoreEnum.Type.mongo, False, 1, 5, 2, 23, 8), - (ModuleStoreEnum.Type.mongo, False, 50, 5, 2, 23, 8), + (ModuleStoreEnum.Type.mongo, False, 1, 5, 2, 23, 7), + (ModuleStoreEnum.Type.mongo, False, 50, 5, 2, 23, 7), # split mongo: 3 queries, regardless of thread response size. - (ModuleStoreEnum.Type.split, False, 1, 3, 3, 23, 8), - (ModuleStoreEnum.Type.split, False, 50, 3, 3, 23, 8), + (ModuleStoreEnum.Type.split, False, 1, 3, 3, 23, 7), + (ModuleStoreEnum.Type.split, False, 50, 3, 3, 23, 7), # Enabling Enterprise integration should have no effect on the number of mongo queries made. - (ModuleStoreEnum.Type.mongo, True, 1, 5, 2, 23, 8), - (ModuleStoreEnum.Type.mongo, True, 50, 5, 2, 23, 8), + (ModuleStoreEnum.Type.mongo, True, 1, 5, 2, 23, 7), + (ModuleStoreEnum.Type.mongo, True, 50, 5, 2, 23, 7), # split mongo: 3 queries, regardless of thread response size. - (ModuleStoreEnum.Type.split, True, 1, 3, 3, 23, 8), - (ModuleStoreEnum.Type.split, True, 50, 3, 3, 23, 8), + (ModuleStoreEnum.Type.split, True, 1, 3, 3, 23, 7), + (ModuleStoreEnum.Type.split, True, 50, 3, 3, 23, 7), ) @ddt.unpack def test_number_of_mongo_queries( diff --git a/lms/djangoapps/grades/tests/test_course_grade_factory.py b/lms/djangoapps/grades/tests/test_course_grade_factory.py index 0565bd31e7..cf267b4788 100644 --- a/lms/djangoapps/grades/tests/test_course_grade_factory.py +++ b/lms/djangoapps/grades/tests/test_course_grade_factory.py @@ -100,32 +100,32 @@ class TestCourseGradeFactory(GradeTestBase): with self.assertNumQueries(3), mock_get_score(1, 2): _assert_read(expected_pass=False, expected_percent=0) # start off with grade of 0 - num_queries = 46 + num_queries = 45 with self.assertNumQueries(num_queries), mock_get_score(1, 2): grade_factory.update(self.request.user, self.course, force_update_subsections=True) - with self.assertNumQueries(4): + with self.assertNumQueries(3): _assert_read(expected_pass=True, expected_percent=0.5) # updated to grade of .5 - num_queries = 8 + num_queries = 7 with self.assertNumQueries(num_queries), mock_get_score(1, 4): grade_factory.update(self.request.user, self.course, force_update_subsections=False) - with self.assertNumQueries(4): + with self.assertNumQueries(3): _assert_read(expected_pass=True, expected_percent=0.5) # NOT updated to grade of .25 - num_queries = 25 + num_queries = 24 with self.assertNumQueries(num_queries), mock_get_score(2, 2): grade_factory.update(self.request.user, self.course, force_update_subsections=True) - with self.assertNumQueries(4): + with self.assertNumQueries(3): _assert_read(expected_pass=True, expected_percent=1.0) # updated to grade of 1.0 - num_queries = 29 + num_queries = 28 with self.assertNumQueries(num_queries), mock_get_score(0, 0): # the subsection now is worth zero grade_factory.update(self.request.user, self.course, force_update_subsections=True) - with self.assertNumQueries(4): + with self.assertNumQueries(3): _assert_read(expected_pass=False, expected_percent=0.0) # updated to grade of 0.0 @patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False}) diff --git a/openedx/features/course_duration_limits/access.py b/openedx/features/course_duration_limits/access.py index 9a86b0d9ef..6a0f30e8c2 100644 --- a/openedx/features/course_duration_limits/access.py +++ b/openedx/features/course_duration_limits/access.py @@ -11,6 +11,7 @@ import six from django.utils import timezone from django.utils.translation import get_language from django.utils.translation import ugettext as _ +from edx_django_utils.cache import RequestCache from web_fragments.fragment import Fragment from course_modes.models import CourseMode @@ -229,11 +230,42 @@ def generate_course_expired_message(user, course): def generate_course_expired_fragment(user, course): message = generate_course_expired_message(user, course) if message: - return Fragment(HTML(u"""\ + return generate_fragment_from_message(message) + + +def generate_fragment_from_message(message): + return Fragment(HTML(u"""\
""").format(message)) +def generate_course_expired_fragment_from_key(user, course_key): + """ + Like `generate_course_expired_fragment`, but using a CourseKey instead of + a CourseOverview and using request-level caching. + + Either returns WebFragment to inject XBlock content into, or None if we + shouldn't show a course expired message for this user. + """ + request_cache = RequestCache('generate_course_expired_fragment_from_key') + cache_key = u'message:{},{}'.format(user.id, course_key) + cache_response = request_cache.get_cached_response(cache_key) + if cache_response.is_found: + cached_message = cache_response.value + # In this case, there is no message to display. + if cached_message is None: + return None + return generate_fragment_from_message(cached_message) + + course = CourseOverview.get_from_id(course_key) + message = generate_course_expired_message(user, course) + request_cache.set(cache_key, message) + if message is None: + return None + + return generate_fragment_from_message(message) + + def course_expiration_wrapper(user, block, view, frag, context): # pylint: disable=W0613 """ An XBlock wrapper that prepends a message to the beginning of a vertical if @@ -242,9 +274,9 @@ def course_expiration_wrapper(user, block, view, frag, context): # pylint: disa if block.category != "vertical": return frag - course = CourseOverview.get_from_id(block.course_id) - course_expiration_fragment = generate_course_expired_fragment(user, course) - + course_expiration_fragment = generate_course_expired_fragment_from_key( + user, block.course_id + ) if not course_expiration_fragment: return frag diff --git a/openedx/features/course_experience/tests/views/test_course_home.py b/openedx/features/course_experience/tests/views/test_course_home.py index 151f726344..c2e7141a22 100644 --- a/openedx/features/course_experience/tests/views/test_course_home.py +++ b/openedx/features/course_experience/tests/views/test_course_home.py @@ -219,7 +219,7 @@ class TestCourseHomePage(CourseHomePageTestCase): # Fetch the view and verify the query counts # TODO: decrease query count as part of REVO-28 - with self.assertNumQueries(90, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): + with self.assertNumQueries(74, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): with check_mongo_calls(4): url = course_home_url(self.course) self.client.get(url) diff --git a/openedx/features/course_experience/tests/views/test_course_updates.py b/openedx/features/course_experience/tests/views/test_course_updates.py index 0f184f3860..a0b6b4150d 100644 --- a/openedx/features/course_experience/tests/views/test_course_updates.py +++ b/openedx/features/course_experience/tests/views/test_course_updates.py @@ -134,7 +134,7 @@ class TestCourseUpdatesPage(SharedModuleStoreTestCase): # Fetch the view and verify that the query counts haven't changed # TODO: decrease query count as part of REVO-28 - with self.assertNumQueries(53, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): + with self.assertNumQueries(51, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST): with check_mongo_calls(4): url = course_updates_url(self.course) self.client.get(url) diff --git a/openedx/features/discounts/utils.py b/openedx/features/discounts/utils.py index 8f5576ea5a..bf6640ac92 100644 --- a/openedx/features/discounts/utils.py +++ b/openedx/features/discounts/utils.py @@ -6,6 +6,7 @@ from datetime import datetime import six from django.utils.translation import ugettext as _ +from edx_django_utils.cache import RequestCache import pytz from course_modes.models import get_course_prices, format_course_price @@ -32,8 +33,9 @@ def offer_banner_wrapper(user, block, view, frag, context): # pylint: disable=W if block.category != "vertical": return frag - course = CourseOverview.get_from_id(block.course_id) - offer_banner_fragment = get_first_purchase_offer_banner_fragment(user, course) + offer_banner_fragment = get_first_purchase_offer_banner_fragment_from_key( + user, block.course_id + ) if not offer_banner_fragment: return frag @@ -97,11 +99,12 @@ def format_strikeout_price(user, course, base_price=None, check_for_discount=Tru return (HTML(u"{}").format(original_price), False) -def get_first_purchase_offer_banner_fragment(user, course): +def generate_offer_html(user, course): """ - Return an HTML Fragment with First Purcahse Discount message, - which has the discount_expiration_date, price, - discount percentage and a link to upgrade. + Create the actual HTML object with the offer text in it. + + Returns a openedx.core.djangolib.markup.HTML object, or None if the user + should not be shown an offer message. """ if user and not user.is_anonymous and course: now = datetime.now(tz=pytz.UTC).strftime(u"%Y-%m-%d %H:%M:%S%z") @@ -119,7 +122,8 @@ def get_first_purchase_offer_banner_fragment(user, course): offer_message = _(u'{banner_open} Upgrade by {discount_expiration_date} and save {percentage}% ' u'[{strikeout_price}]{span_close}{br}Discount will be automatically applied at checkout. ' u'{a_open}Upgrade Now{a_close}{div_close}') - return Fragment(HTML(offer_message).format( + + message_html = HTML(offer_message).format( a_open=HTML(u'').format( upgrade_link=verified_upgrade_deadline_link(user=user, course=course) ), @@ -134,5 +138,42 @@ def get_first_purchase_offer_banner_fragment(user, course): span_close=HTML(''), div_close=HTML(''), strikeout_price=HTML(format_strikeout_price(user, course, check_for_discount=False)[0]) - )) + ) + return message_html return None + + +def get_first_purchase_offer_banner_fragment(user, course): + """ + Return an HTML Fragment with First Purcahse Discount message, + which has the discount_expiration_date, price, + discount percentage and a link to upgrade. + """ + offer_html = generate_offer_html(user, course) + if offer_html is None: + return None + return Fragment(offer_html) + + +def get_first_purchase_offer_banner_fragment_from_key(user, course_key): + """ + Like `get_first_purchase_offer_banner_fragment`, but using a CourseKey + instead of a CourseOverview and using request-level caching. + + Either returns WebFragment to inject XBlock content into, or None if we + shouldn't show a first purchase offer message for this user. + """ + request_cache = RequestCache('get_first_purchase_offer_banner_fragment_from_key') + cache_key = u'html:{},{}'.format(user.id, course_key) + cache_response = request_cache.get_cached_response(cache_key) + if cache_response.is_found: + cached_html = cache_response.value + if cached_html is None: + return None + return Fragment(cached_html) + + course = CourseOverview.get_from_id(course_key) + offer_html = generate_offer_html(user, course) + request_cache.set(cache_key, offer_html) + + return Fragment(offer_html)