From a5b0f71108f778b88eecb29b33713f9a829f685f Mon Sep 17 00:00:00 2001
From: "Dave St.Germain" <dstgermain@edx.org>
Date: Thu, 16 Jan 2020 13:56:32 -0500
Subject: [PATCH] Several optimizations for improving vertical rendering
 performance.

---
 common/djangoapps/xblock_django/api.py        |  5 ++
 common/djangoapps/xblock_django/models.py     |  4 --
 lms/djangoapps/courseware/tests/test_views.py | 12 ++--
 lms/djangoapps/courseware/views/views.py      |  2 +
 .../grades/tests/test_course_grade_factory.py |  6 +-
 .../tests/test_tasks_helper.py                |  4 +-
 .../courseware/courseware-chromeless.html     |  8 +--
 openedx/core/djangoapps/bookmarks/api.py      |  2 +-
 .../content/course_overviews/models.py        | 16 ++++++
 openedx/core/djangoapps/user_api/models.py    |  2 +
 openedx/core/lib/cache_utils.py               | 55 ++++++++++++++++++-
 .../tests/views/test_course_home.py           |  2 +-
 .../tests/views/test_course_updates.py        |  2 +-
 13 files changed, 97 insertions(+), 23 deletions(-)

diff --git a/common/djangoapps/xblock_django/api.py b/common/djangoapps/xblock_django/api.py
index 88a2b47e9d..b9cb2f3a53 100644
--- a/common/djangoapps/xblock_django/api.py
+++ b/common/djangoapps/xblock_django/api.py
@@ -4,8 +4,12 @@ API methods related to xblock state.
 
 
 from xblock_django.models import XBlockConfiguration, XBlockStudioConfiguration
+from openedx.core.lib.cache_utils import CacheInvalidationManager
+
+cacher = CacheInvalidationManager(model=XBlockConfiguration)
 
 
+@cacher
 def deprecated_xblocks():
     """
     Return the QuerySet of deprecated XBlock types. Note that this method is independent of
@@ -14,6 +18,7 @@ def deprecated_xblocks():
     return XBlockConfiguration.objects.current_set().filter(deprecated=True)
 
 
+@cacher
 def disabled_xblocks():
     """
     Return the QuerySet of disabled XBlock types (which should not render in the LMS).
diff --git a/common/djangoapps/xblock_django/models.py b/common/djangoapps/xblock_django/models.py
index a3d8b3764b..5df96dcb58 100644
--- a/common/djangoapps/xblock_django/models.py
+++ b/common/djangoapps/xblock_django/models.py
@@ -6,10 +6,8 @@ Models.
 from config_models.models import ConfigurationModel
 from django.db import models
 from django.utils.translation import ugettext_lazy as _
-from django.utils.encoding import python_2_unicode_compatible
 
 
-@python_2_unicode_compatible
 class XBlockConfiguration(ConfigurationModel):
     """
     XBlock configuration used by both LMS and Studio, and not specific to a particular template.
@@ -35,7 +33,6 @@ class XBlockConfiguration(ConfigurationModel):
         ).format(self.name, self.enabled, self.deprecated)
 
 
-@python_2_unicode_compatible
 class XBlockStudioConfigurationFlag(ConfigurationModel):
     """
     Enables site-wide Studio configuration for XBlocks.
@@ -52,7 +49,6 @@ class XBlockStudioConfigurationFlag(ConfigurationModel):
         return "XBlockStudioConfigurationFlag(enabled={})".format(self.enabled)
 
 
-@python_2_unicode_compatible
 class XBlockStudioConfiguration(ConfigurationModel):
     """
     Studio editing configuration for a specific XBlock/template combination.
diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py
index 92108ef0cc..732ac62483 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, 176),
-        (ModuleStoreEnum.Type.split, 4, 174),
+        (ModuleStoreEnum.Type.mongo, 10, 172),
+        (ModuleStoreEnum.Type.split, 4, 170),
     )
     @ddt.unpack
     def test_index_query_counts(self, store_type, expected_mongo_query_count, expected_mysql_query_count):
@@ -1492,8 +1492,8 @@ class ProgressPageTests(ProgressPageBaseTests):
             self.assertContains(resp, u"Download Your Certificate")
 
     @ddt.data(
-        (True, 54),
-        (False, 53)
+        (True, 53),
+        (False, 52)
     )
     @ddt.unpack
     def test_progress_queries_paced_courses(self, self_paced, query_count):
@@ -1506,8 +1506,8 @@ class ProgressPageTests(ProgressPageBaseTests):
 
     @patch.dict(settings.FEATURES, {'ASSUME_ZERO_GRADE_IF_ABSENT_FOR_ALL_TESTS': False})
     @ddt.data(
-        (False, 62, 41),
-        (True, 53, 36)
+        (False, 61, 40),
+        (True, 52, 35)
     )
     @ddt.unpack
     def test_progress_queries(self, enable_waffle, initial, subsequent):
diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py
index 0af852d6b5..8a6ed95bbf 100644
--- a/lms/djangoapps/courseware/views/views.py
+++ b/lms/djangoapps/courseware/views/views.py
@@ -35,6 +35,7 @@ from django.views.decorators.csrf import ensure_csrf_cookie
 from django.views.decorators.http import require_GET, require_http_methods, require_POST
 from django.views.generic import View
 from edx_django_utils.monitoring import set_custom_metrics_for_course_key
+from edxnotes.helpers import is_feature_enabled
 from ipware.ip import get_ip
 from markupsafe import escape
 from opaque_keys import InvalidKeyError
@@ -1607,6 +1608,7 @@ def render_xblock(request, usage_key_string, check_if_enrolled=True):
             'disable_footer': True,
             'disable_window_wrap': True,
             'enable_completion_on_view_service': enable_completion_on_view_service,
+            'edx_notes_enabled': is_feature_enabled(course, request.user),
             'staff_access': bool(request.user.has_perm(VIEW_XQA_INTERFACE, course)),
             'xqa_server': settings.FEATURES.get('XQA_SERVER', 'http://your_xqa_server.com'),
         }
diff --git a/lms/djangoapps/grades/tests/test_course_grade_factory.py b/lms/djangoapps/grades/tests/test_course_grade_factory.py
index cf267b4788..dc41c26978 100644
--- a/lms/djangoapps/grades/tests/test_course_grade_factory.py
+++ b/lms/djangoapps/grades/tests/test_course_grade_factory.py
@@ -100,21 +100,21 @@ 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 = 45
+        num_queries = 44
         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(3):
             _assert_read(expected_pass=True, expected_percent=0.5)  # updated to grade of .5
 
-        num_queries = 7
+        num_queries = 6
         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(3):
             _assert_read(expected_pass=True, expected_percent=0.5)  # NOT updated to grade of .25
 
-        num_queries = 24
+        num_queries = 23
         with self.assertNumQueries(num_queries), mock_get_score(2, 2):
             grade_factory.update(self.request.user, self.course, force_update_subsections=True)
 
diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py
index 7199277edc..1ca037c00e 100644
--- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py
+++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py
@@ -420,7 +420,7 @@ class TestInstructorGradeReport(InstructorGradeReportTestCase):
 
         RequestCache.clear_all_namespaces()
 
-        expected_query_count = 50
+        expected_query_count = 45
         with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'):
             with check_mongo_calls(mongo_count):
                 with self.assertNumQueries(expected_query_count):
@@ -2260,7 +2260,7 @@ class TestCertificateGeneration(InstructorTaskModuleTestCase):
             'failed': 3,
             'skipped': 2
         }
-        with self.assertNumQueries(146):
+        with self.assertNumQueries(141):
             self.assertCertificatesGenerated(task_input, expected_results)
 
         expected_results = {
diff --git a/lms/templates/courseware/courseware-chromeless.html b/lms/templates/courseware/courseware-chromeless.html
index 0abee7697e..a9bf9414b2 100644
--- a/lms/templates/courseware/courseware-chromeless.html
+++ b/lms/templates/courseware/courseware-chromeless.html
@@ -3,10 +3,10 @@
 <%namespace name='static' file='/static_content.html'/>
 <%!
 from django.utils.translation import ugettext as _
-from edxnotes.helpers import is_feature_enabled as is_edxnotes_enabled
 
 from openedx.core.djangolib.markup import HTML
 from openedx.core.djangolib.js_utils import js_escaped_string
+
 %>
 <%def name="course_name()">
  <% return _("{course_number} Courseware").format(course_number=course.display_number_with_default) %>
@@ -36,7 +36,7 @@ ${static.get_page_title_breadcrumbs(course_name())}
 <%static:css group='style-course-vendor'/>
 <%static:css group='style-course'/>
 ## Utility: Notes
-% if is_edxnotes_enabled(course, request.user):
+% if edx_notes_enabled:
 <%static:css group='style-student-notes'/>
 % endif
 
@@ -92,10 +92,10 @@ ${HTML(fragment.foot_html())}
       </main>
   </section>
 </div>
-% if course.show_calculator or is_edxnotes_enabled(course, request.user):
+% if course.show_calculator or edx_notes_enabled:
     <nav class="nav-utilities ${"has-utility-calculator" if course.show_calculator else ""}" aria-label="${_('Course Utilities')}">
       ## Utility: Notes
-      % if is_edxnotes_enabled(course, request.user):
+      % if edx_notes_enabled:
         <%include file="/edxnotes/toggle_notes.html" args="course=course"/>
       % endif
 
diff --git a/openedx/core/djangoapps/bookmarks/api.py b/openedx/core/djangoapps/bookmarks/api.py
index 9cba6320a7..67b1c701de 100644
--- a/openedx/core/djangoapps/bookmarks/api.py
+++ b/openedx/core/djangoapps/bookmarks/api.py
@@ -74,7 +74,7 @@ def get_bookmarks(user, course_key=None, fields=None, serialized=True):
         else:
             bookmarks_queryset = bookmarks_queryset.select_related('user')
 
-        bookmarks_queryset = bookmarks_queryset.order_by('-created')
+        bookmarks_queryset = bookmarks_queryset.order_by('-id')
     else:
         bookmarks_queryset = Bookmark.objects.none()
 
diff --git a/openedx/core/djangoapps/content/course_overviews/models.py b/openedx/core/djangoapps/content/course_overviews/models.py
index bcb7d42609..578fc5e7c0 100644
--- a/openedx/core/djangoapps/content/course_overviews/models.py
+++ b/openedx/core/djangoapps/content/course_overviews/models.py
@@ -13,6 +13,7 @@ from django.conf import settings
 from django.db import models, transaction
 from django.db.models import Q
 from django.db.models.fields import BooleanField, DateTimeField, DecimalField, FloatField, IntegerField, TextField
+from django.db.models.signals import post_save, post_delete
 from django.db.utils import IntegrityError
 from django.template import defaultfilters
 from django.utils.encoding import python_2_unicode_compatible
@@ -27,6 +28,7 @@ from lms.djangoapps.discussion import django_comment_client
 from openedx.core.djangoapps.catalog.models import CatalogIntegration
 from openedx.core.djangoapps.lang_pref.api import get_closest_released_language
 from openedx.core.djangoapps.models.course_details import CourseDetails
+from openedx.core.lib.cache_utils import request_cached, RequestCache
 from static_replace.models import AssetBaseUrlConfig
 from xmodule import block_metadata_utils, course_metadata_utils
 from xmodule.course_module import DEFAULT_START_DATE, CourseDescriptor
@@ -319,6 +321,7 @@ class CourseOverview(TimeStampedModel):
         return modulestore().has_course(course_id)
 
     @classmethod
+    @request_cached('course_overview')
     def get_from_id(cls, course_id):
         """
         Load a CourseOverview object for a given course ID.
@@ -1024,3 +1027,16 @@ class SimulateCoursePublishConfig(ConfigurationModel):
 
     def __str__(self):
         return six.text_type(self.arguments)
+
+
+def _invalidate_overview_cache(**kwargs):  # pylint: disable=unused-argument
+    """
+    Invalidate the course overview request cache.
+    """
+    RequestCache('course_overview').clear()
+
+
+post_save.connect(_invalidate_overview_cache, sender=CourseOverview)
+post_save.connect(_invalidate_overview_cache, sender=CourseOverviewImageConfig)
+post_delete.connect(_invalidate_overview_cache, sender=CourseOverview)
+post_delete.connect(_invalidate_overview_cache, sender=CourseOverviewImageConfig)
diff --git a/openedx/core/djangoapps/user_api/models.py b/openedx/core/djangoapps/user_api/models.py
index 11a44f85ec..915814125e 100644
--- a/openedx/core/djangoapps/user_api/models.py
+++ b/openedx/core/djangoapps/user_api/models.py
@@ -22,6 +22,7 @@ from opaque_keys.edx.django.models import CourseKeyField
 # create an alias in "user_api".
 
 from openedx.core.djangolib.model_mixins import DeletableByUserValue
+from openedx.core.lib.cache_utils import request_cached
 # pylint: disable=unused-import
 from student.models import (
     PendingEmailChange,
@@ -54,6 +55,7 @@ class UserPreference(models.Model):
         unique_together = ("user", "key")
 
     @staticmethod
+    @request_cached()
     def get_all_preferences(user):
         """
         Gets all preferences for a given user
diff --git a/openedx/core/lib/cache_utils.py b/openedx/core/lib/cache_utils.py
index 10029cfc1a..9a71acc791 100644
--- a/openedx/core/lib/cache_utils.py
+++ b/openedx/core/lib/cache_utils.py
@@ -10,8 +10,10 @@ import zlib
 
 import six
 import wrapt
+from django.db.models.signals import post_save, post_delete
 from django.utils.encoding import force_text
-from edx_django_utils.cache import RequestCache
+
+from edx_django_utils.cache import RequestCache, TieredCache
 from six import iteritems
 from six.moves import cPickle as pickle
 from six.moves import map
@@ -151,6 +153,57 @@ class process_cached(object):  # pylint: disable=invalid-name
         return functools.partial(self.__call__, obj)
 
 
+class CacheInvalidationManager:
+    """
+    This class provides a decorator for simple functions, which can handle invalidation.
+
+    To use, instantiate with a namespace or django model class:
+    `manager = CacheInvalidationManager(model=User)`
+
+    Then use it as a decorator on functions with no arguments
+    `@manager
+    def get_system_user():
+         ...
+    `
+    When the User model is saved or deleted, all cache keys used by
+    the decorator will be cleared.
+    """
+
+    def __init__(self, namespace=None, model=None, cache_time=86400):
+        if model:
+            post_save.connect(self.invalidate, sender=model)
+            post_delete.connect(self.invalidate, sender=model)
+            namespace = str(model)
+        self.namespace = namespace
+        self.cache_time = cache_time
+        self.keys = set()
+
+    # pylint: disable=unused-argument
+    def invalidate(self, **kwargs):
+        """
+        Invalidate all keys tracked by the manager.
+        """
+        for key in self.keys:
+            TieredCache.delete_all_tiers(key)
+
+    def __call__(self, func):
+        """
+        Decorator for functions with no arguments.
+        """
+        cache_key = '{}.{}.{}'.format(self.namespace, func.__module__, func.__name__)
+        self.keys.add(cache_key)
+
+        @functools.wraps(func)
+        def decorator(*args, **kwargs):  # pylint: disable=unused-argument,missing-docstring
+            result = TieredCache.get_cached_response(cache_key)
+            if result.is_found:
+                return result.value
+            result = func()
+            TieredCache.set_all_tiers(cache_key, result, self.cache_time)
+            return result
+        return decorator
+
+
 def zpickle(data):
     """Given any data structure, returns a zlib compressed pickled serialization."""
     return zlib.compress(pickle.dumps(data, 4))  # Keep this constant as we upgrade from python 2 to 3.
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 8b88c99381..82169369dd 100644
--- a/openedx/features/course_experience/tests/views/test_course_home.py
+++ b/openedx/features/course_experience/tests/views/test_course_home.py
@@ -218,7 +218,7 @@ class TestCourseHomePage(CourseHomePageTestCase):
 
         # Fetch the view and verify the query counts
         # TODO: decrease query count as part of REVO-28
-        with self.assertNumQueries(74, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST):
+        with self.assertNumQueries(73, 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 a0b6b4150d..69ad5b78d1 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(51, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST):
+        with self.assertNumQueries(50, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST):
             with check_mongo_calls(4):
                 url = course_updates_url(self.course)
                 self.client.get(url)