Merge pull request #19642 from cpennington/fbe-improve-course-api-performance

Allow courses api to return data incrementally

common/djangoapps/course_modes/models.py

@@ -312,7 +312,7 @@ class CourseMode(models.Model):
 
     @classmethod
     @request_cached(CACHE_NAMESPACE)
-    def modes_for_course(cls, course_id, include_expired=False, only_selectable=True):
+    def modes_for_course(cls, course_id=None, include_expired=False, only_selectable=True, course=None):
         """
         Returns a list of the non-expired modes for a given course id
 
@@ -330,11 +330,25 @@ class CourseMode(models.Model):
                 aren't available to users until they complete the course, so
                 they are hidden in track selection.)
 
+            course (CourseOverview): The course to select course modes from.
+
         Returns:
             list of `Mode` tuples
 
         """
-        found_course_modes = cls.objects.filter(course_id=course_id)
+        if course_id is None and course is None:
+            raise ValueError("One of course_id or course must not be None.")
+
+        if course is not None and not isinstance(course, CourseOverview):
+            # CourseModules don't have the data needed to pull related modes,
+            # so we'll fall back on course_id-based lookup instead
+            course_id = course.id
+            course = None
+
+        if course_id is not None:
+            found_course_modes = cls.objects.filter(course_id=course_id)
+        else:
+            found_course_modes = course.modes
 
         # Filter out expired course modes if include_expired is not set
         if not include_expired:
@@ -347,7 +361,10 @@ class CourseMode(models.Model):
         # we exclude them from the list if we're only looking for selectable modes
         # (e.g. on the track selection page or in the payment/verification flows).
         if only_selectable:
-            found_course_modes = found_course_modes.exclude(mode_slug__in=cls.CREDIT_MODES)
+            if course is not None and hasattr(course, 'selectable_modes'):
+                found_course_modes = course.selectable_modes
+            else:
+                found_course_modes = found_course_modes.exclude(mode_slug__in=cls.CREDIT_MODES)
 
         modes = ([mode.to_tuple() for mode in found_course_modes])
         if not modes:
@@ -356,7 +373,7 @@ class CourseMode(models.Model):
         return modes
 
     @classmethod
-    def modes_for_course_dict(cls, course_id, modes=None, **kwargs):
+    def modes_for_course_dict(cls, course_id=None, modes=None, **kwargs):
         """Returns the non-expired modes for a particular course.
 
         Arguments:
@@ -418,7 +435,7 @@ class CourseMode(models.Model):
             return None
 
     @classmethod
-    def verified_mode_for_course(cls, course_id, modes=None, include_expired=False):
+    def verified_mode_for_course(cls, course_id=None, modes=None, include_expired=False, course=None):
         """Find a verified mode for a particular course.
 
         Since we have multiple modes that can go through the verify flow,
@@ -439,7 +456,12 @@ class CourseMode(models.Model):
             Mode or None
 
         """
-        modes_dict = cls.modes_for_course_dict(course_id, modes=modes, include_expired=include_expired)
+        modes_dict = cls.modes_for_course_dict(
+            course_id=course_id,
+            modes=modes,
+            include_expired=include_expired,
+            course=course
+        )
         verified_mode = modes_dict.get('verified', None)
         professional_mode = modes_dict.get('professional', None)
         # we prefer professional over verify
@@ -703,7 +725,7 @@ class CourseMode(models.Model):
         Takes a mode model and turns it into a model named tuple.
 
         Returns:
-            A 'Model' namedtuple with all the same attributes as the model.
+            A 'Mode' namedtuple with all the same attributes as the model.
 
         """
         return Mode(

lms/djangoapps/branding/__init__.py

@@ -15,7 +15,7 @@ from openedx.core.djangoapps.site_configuration import helpers as configuration_
 
 def get_visible_courses(org=None, filter_=None):
     """
-    Return the set of CourseOverviews that should be visible in this branded
+    Yield the CourseOverviews that should be visible in this branded
     instance.
 
     Arguments:
@@ -27,9 +27,10 @@ def get_visible_courses(org=None, filter_=None):
     # Import is placed here to avoid model import at project startup.
     from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
 
-    courses = []
     current_site_orgs = configuration_helpers.get_current_site_orgs()
 
+    courses = CourseOverview.objects.none()
+
     if org:
         # Check the current site's orgs to make sure the org's courses should be displayed
         if not current_site_orgs or org in current_site_orgs:
@@ -40,7 +41,7 @@ def get_visible_courses(org=None, filter_=None):
     else:
         courses = CourseOverview.get_all_courses(filter_=filter_)
 
-    courses = sorted(courses, key=lambda course: course.number)
+    courses = courses.order_by('id')
 
     # Filtering can stop here.
     if current_site_orgs:
@@ -57,11 +58,11 @@ def get_visible_courses(org=None, filter_=None):
         )
 
     if filtered_visible_ids:
-        return [course for course in courses if course.id in filtered_visible_ids]
+        return courses.filter(id__in=filtered_visible_ids)
     else:
         # Filter out any courses based on current org, to avoid leaking these.
         orgs = configuration_helpers.get_all_orgs()
-        return [course for course in courses if course.location.org not in orgs]
+        return courses.exclude(org__in=orgs)
 
 
 def get_university_for_request():

lms/djangoapps/course_api/api.py

@@ -57,7 +57,7 @@ def course_detail(request, username, course_key):
 
 def list_courses(request, username, org=None, filter_=None):
     """
-    Return a list of available courses.
+    Yield all available courses.
 
     The courses returned are all be visible to the user identified by
     `username` and the logged in user should have permission to view courses
@@ -81,7 +81,7 @@ def list_courses(request, username, org=None, filter_=None):
             by the given key-value pairs.
 
     Return value:
-        List of `CourseOverview` objects representing the collection of courses.
+        Yield `CourseOverview` objects representing the collection of courses.
     """
     user = get_effective_user(request.user, username)
     return get_courses(user, org=org, filter_=filter_)

lms/djangoapps/course_api/tests/test_api.py

@@ -231,12 +231,12 @@ class TestGetCourseListExtras(CourseListTestMixin, ModuleStoreTestCase):
 
     def test_no_courses(self):
         courses = self._make_api_call(self.honor_user, self.honor_user)
-        self.assertEqual(len(courses), 0)
+        self.assertEqual(len(list(courses)), 0)
 
     def test_hidden_course_for_honor(self):
         self.create_course(visible_to_staff_only=True)
         courses = self._make_api_call(self.honor_user, self.honor_user)
-        self.assertEqual(len(courses), 0)
+        self.assertEqual(len(list(courses)), 0)
 
     def test_hidden_course_for_staff(self):
         self.create_course(visible_to_staff_only=True)

lms/djangoapps/course_api/tests/test_views.py

 """
 Tests for Course API views.
 """
+from datetime import datetime
 import ddt
 from hashlib import md5
 
@@ -12,8 +13,14 @@ from search.tests.test_course_discovery import DemoCourse
 from search.tests.tests import TEST_INDEX_NAME
 from search.tests.utils import SearcherMixin
 
+from course_modes.models import CourseMode
+from course_modes.tests.factories import CourseModeFactory
+from edx_django_utils.cache import RequestCache
 from openedx.core.lib.tests import attr
+from openedx.features.content_type_gating.models import ContentTypeGatingConfig
+from openedx.features.course_duration_limits.models import CourseDurationLimitConfig
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, SharedModuleStoreTestCase
+from xmodule.modulestore.tests.factories import CourseFactory
 from waffle.testutils import override_switch
 
 from ..views import CourseDetailView, CourseListUserThrottle
@@ -283,6 +290,7 @@ class CourseListSearchViewTest(CourseApiTestViewMixin, ModuleStoreTestCase, Sear
     """
 
     ENABLED_SIGNALS = ['course_published']
+    ENABLED_CACHES = ModuleStoreTestCase.ENABLED_CACHES + ['configuration']
 
     def setUp(self):
         super(CourseListSearchViewTest, self).setUp()
@@ -298,6 +306,7 @@ class CourseListSearchViewTest(CourseApiTestViewMixin, ModuleStoreTestCase, Sear
         self.url = reverse('course-list')
         self.staff_user = self.create_user(username='staff', is_staff=True)
         self.honor_user = self.create_user(username='honor', is_staff=False)
+        self.audit_user = self.create_user(username='audit', is_staff=False)
 
     def create_and_index_course(self, org_code, short_description):
         """
@@ -347,4 +356,56 @@ class CourseListSearchViewTest(CourseApiTestViewMixin, ModuleStoreTestCase, Sear
         res = self.verify_response(params={'search_term': 'unique search term'})
         self.assertIn('results', res.data)
         self.assertNotEqual(res.data['results'], [])
-        self.assertEqual(res.data['pagination']['count'], 1)  # Should list a single course
+        # Returns a count of 3 courses because that's the estimate before filtering
+        self.assertEqual(res.data['pagination']['count'], 3)
+        self.assertEqual(len(res.data['results']), 1)  # Should return a single course
+
+    def test_too_many_courses(self):
+        """
+        Test that search results are limited to 100 courses, and that they don't
+        blow up the database.
+        """
+
+        ContentTypeGatingConfig.objects.create(
+            enabled=True,
+            enabled_as_of=datetime(2018, 1, 1),
+        )
+
+        CourseDurationLimitConfig.objects.create(
+            enabled=True,
+            enabled_as_of=datetime(2018, 1, 1),
+        )
+
+        course_ids = []
+
+        # Create 300 courses across 30 organizations
+        for org_num in range(10):
+            org_id = 'org{}'.format(org_num)
+            for course_num in range(30):
+                course_name = 'course{}.{}'.format(org_num, course_num)
+                course_run_name = 'run{}.{}'.format(org_num, course_num)
+                course = CourseFactory.create(org=org_id, number=course_name, run=course_run_name, emit_signals=True)
+                CourseModeFactory.create(course_id=course.id, mode_slug=CourseMode.AUDIT)
+                CourseModeFactory.create(course_id=course.id, mode_slug=CourseMode.VERIFIED)
+                course_ids.append(course.id)
+
+        self.setup_user(self.audit_user)
+
+        # These query counts were found empirically
+        query_counts = [65, 52, 52, 52, 52, 52, 52, 52, 52, 52, 22]
+        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})
+
+                self.assertIn('results', response.data)
+                self.assertEqual(response.data['pagination']['count'], 303)
+                self.assertEqual(len(response.data['results']), 30 if page < 11 else 3)
+                self.assertEqual(
+                    [c['id'] for c in response.data['results']],
+                    ordered_course_ids[(page - 1) * 30:page * 30]
+                )

lms/djangoapps/course_api/views.py

@@ -10,6 +10,7 @@ from rest_framework.throttling import UserRateThrottle
 
 from edx_rest_framework_extensions.paginators import NamespacedPageNumberPagination
 from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, view_auth_classes
+from openedx.core.lib.api.view_utils import LazySequence
 
 from . import USE_RATE_LIMIT_2_FOR_COURSE_LIST_API, USE_RATE_LIMIT_10_FOR_COURSE_LIST_API
 from .api import course_detail, list_courses
@@ -243,7 +244,7 @@ class CourseListView(DeveloperErrorViewMixin, ListAPIView):
 
     def get_queryset(self):
         """
-        Return a list of courses visible to the user.
+        Yield courses visible to the user.
         """
         form = CourseListGetForm(self.request.query_params, initial={'requesting_user': self.request.user})
         if not form.is_valid():
@@ -264,9 +265,12 @@ class CourseListView(DeveloperErrorViewMixin, ListAPIView):
             size=self.results_size_infinity,
         )
 
-        search_courses_ids = {course['data']['id']: True for course in search_courses['results']}
+        search_courses_ids = {course['data']['id'] for course in search_courses['results']}
 
-        return [
-            course for course in db_courses
-            if unicode(course.id) in search_courses_ids
-        ]
+        return LazySequence(
+            (
+                course for course in db_courses
+                if unicode(course.id) in search_courses_ids
+            ),
+            est_len=len(db_courses)
+        )

lms/djangoapps/courseware/courses.py

@@ -23,7 +23,9 @@ from courseware.date_summary import (
 from courseware.masquerade import check_content_start_date_for_masquerade_user
 from courseware.model_data import FieldDataCache
 from courseware.module_render import get_module
+from course_modes.models import CourseMode
 from django.conf import settings
+from django.db.models import Prefetch
 from django.urls import reverse
 from django.http import Http404, QueryDict
 from enrollment.api import get_course_enrollment_details
@@ -35,6 +37,7 @@ from lms.djangoapps.courseware.exceptions import CourseAccessRedirect
 from opaque_keys.edx.keys import UsageKey
 from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
 from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
+from openedx.core.lib.api.view_utils import LazySequence
 from path import Path as path
 from six import text_type
 from static_replace import replace_static_urls
@@ -451,19 +454,29 @@ def get_course_syllabus_section(course, section_key):
 
 def get_courses(user, org=None, filter_=None):
     """
-    Returns a list of courses available, sorted by course.number and optionally
-    filtered by org code (case-insensitive).
-    """
-    courses = branding.get_visible_courses(org=org, filter_=filter_)
+    Return a LazySequence of courses available, optionally filtered by org code (case-insensitive).
+    """
+    courses = branding.get_visible_courses(
+        org=org,
+        filter_=filter_,
+    ).prefetch_related(
+        Prefetch(
+            'modes',
+            queryset=CourseMode.objects.exclude(mode_slug__in=CourseMode.CREDIT_MODES),
+            to_attr='selectable_modes',
+        ),
+        'image_set',
+    )
 
     permission_name = configuration_helpers.get_value(
         'COURSE_CATALOG_VISIBILITY_PERMISSION',
         settings.COURSE_CATALOG_VISIBILITY_PERMISSION
     )
 
-    courses = [c for c in courses if has_access(user, permission_name, c)]
-
-    return courses
+    return LazySequence(
+        (c for c in courses if has_access(user, permission_name, c)),
+        est_len=courses.count()
+    )
 
 
 def get_permission_for_course_about():

lms/djangoapps/courseware/tests/test_courses.py

@@ -141,7 +141,7 @@ class CoursesTest(ModuleStoreTestCase):
 
             # Request filtering for an org distinct from the designated org.
             no_courses = get_courses(user, org=primary)
-            self.assertEqual(no_courses, [])
+            self.assertEqual(list(no_courses), [])
 
             # Request filtering for an org matching the designated org.
             site_courses = get_courses(user, org=alternate)

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:

openedx/core/djangoapps/content/course_overviews/models.py

@@ -409,7 +409,8 @@ class CourseOverview(TimeStampedModel):
         migrate and test switching to display_name_with_default, which is no
         longer escaped.
         """
-        return block_metadata_utils.display_name_with_default_escaped(self)
+        # pylint: disable=line-too-long
+        return block_metadata_utils.display_name_with_default_escaped(self)  # xss-lint: disable=python-deprecated-display-name
 
     @property
     def dashboard_start_display(self):
@@ -560,7 +561,7 @@ class CourseOverview(TimeStampedModel):
     @classmethod
     def get_all_courses(cls, orgs=None, filter_=None):
         """
-        Returns all CourseOverview objects in the database.
+        Return a queryset containing all CourseOverview objects in the database.
 
         Arguments:
             orgs (list[string]): Optional parameter that allows case-insensitive

openedx/core/djangolib/testing/utils.py

@@ -73,6 +73,9 @@ class CacheIsolationMixin(object):
                     'BACKEND': 'django.core.cache.backends.locmem.LocMemCache',
                     'LOCATION': cache_name,
                     'KEY_FUNCTION': 'util.memcache.safe_key',
+                    'OPTIONS': {
+                        'MAX_ENTRIES': 1000,
+                    },
                 } for cache_name in cls.ENABLED_CACHES
             })
 

openedx/core/lib/api/view_utils.py

 """
 Utilities related to API views
 """
+from collections import Sequence
 from django.core.exceptions import NON_FIELD_ERRORS, ObjectDoesNotExist, ValidationError
 from django.http import Http404
 from django.utils.translation import ugettext as _
@@ -205,3 +206,100 @@ class RetrievePatchAPIView(RetrieveModelMixin, UpdateModelMixin, GenericAPIView)
                 # PATCH requests where the object does not exist should still
                 # return a 404 response.
                 raise
+
+
+class LazySequence(Sequence):
+    """
+    This class provides an immutable Sequence interface on top of an existing
+    iterable.
+
+    It is immutable, and accepts an estimated length in order to support __len__
+    without exhausting the underlying sequence
+    """
+    def __init__(self, iterable, est_len=None):  # pylint: disable=super-init-not-called
+        self.iterable = iterable
+        self.est_len = est_len
+        self._data = []
+        self._exhausted = False
+
+    def __len__(self):
+        # Return the actual data length if we know it exactly (because
+        # the underlying sequence is exhausted), or it's greater than
+        # the initial estimated length
+        if len(self._data) > self.est_len or self._exhausted:
+            return len(self._data)
+        else:
+            return self.est_len
+
+    def __iter__(self):
+        # Yield all the known data first
+        for item in self._data:
+            yield item
+
+        # Capture and yield data from the underlying iterator
+        # until it is exhausted
+        while True:
+            try:
+                item = next(self.iterable)
+                self._data.append(item)
+                yield item
+            except StopIteration:
+                self._exhausted = True
+                return
+
+    def __getitem__(self, index):
+        if isinstance(index, int):
+            # For a single index, if we haven't already loaded enough
+            # data, we can load data until we have enough, and then
+            # return the value from the loaded data
+            if index < 0:
+                raise IndexError("Negative indexes aren't supported")
+
+            while len(self._data) <= index:
+                try:
+                    self._data.append(next(self.iterable))
+                except StopIteration:
+                    self._exhausted = True
+                    raise IndexError("Underlying sequence exhausted")
+
+            return self._data[index]
+        elif isinstance(index, slice):
+            # For a slice, we can load data until we reach 'stop'.
+            # Once we have data including 'stop', then we can use
+            # the underlying list to actually understand the mechanics
+            # of the slicing operation.
+            if index.start is not None and index.start < 0:
+                raise IndexError("Negative indexes aren't supported")
+            if index.stop is not None and index.stop < 0:
+                raise IndexError("Negative indexes aren't supported")
+
+            if index.step is not None and index.step < 0:
+                largest_value = index.start + 1
+            else:
+                largest_value = index.stop
+
+            if largest_value is not None:
+                while len(self._data) <= largest_value:
+                    try:
+                        self._data.append(next(self.iterable))
+                    except StopIteration:
+                        self._exhausted = True
+                        break
+            else:
+                self._data.extend(self.iterable)
+            return self._data[index]
+        else:
+            raise TypeError("Unsupported index type")
+
+    def __repr__(self):
+        if self._exhausted:
+            return "LazySequence({!r}, {!r})".format(
+                self._data,
+                self.est_len,
+            )
+        else:
+            return "LazySequence(itertools.chain({!r}, {!r}), {!r})".format(
+                self._data,
+                self.iterable,
+                self.est_len,
+            )

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)
-
-            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
-
-            result = f(*args, **kwargs)
-
-            if request_cache:
-                request_cache.set(cache_key, result)
-
-            return result
-
-        return _decorator
+        # 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(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 = wrapped(*args, **kwargs)
+
+        if request_cache:
+            request_cache.set(cache_key, result)
+
+        return result
+
     return decorator
 
 

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)

openedx/features/course_duration_limits/access.py

@@ -66,7 +66,9 @@ def get_user_course_expiration_date(user, course):
 
     access_duration = MIN_DURATION
 
-    if not CourseMode.verified_mode_for_course(course.id, include_expired=True):
+    verified_mode = CourseMode.verified_mode_for_course(course=course, include_expired=True)
+
+    if not verified_mode:
         return None
 
     enrollment = CourseEnrollment.get_enrollment(user, course.id)

openedx/features/course_duration_limits/tests/test_course_expiration.py

@@ -63,7 +63,7 @@ class CourseExpirationTestCase(ModuleStoreTestCase):
     def test_enrollment_mode(self):
         """Tests that verified enrollments do not have an expiration"""
         CourseEnrollment.enroll(self.user, self.course.id, CourseMode.VERIFIED)
-        result = get_user_course_expiration_date(self.user, self.course)
+        result = get_user_course_expiration_date(self.user, CourseOverview.get_from_id(self.course.id))
         self.assertEqual(result, None)
 
     @mock.patch("openedx.features.course_duration_limits.access.get_course_run_details")
@@ -94,7 +94,10 @@ class CourseExpirationTestCase(ModuleStoreTestCase):
             self.course.self_paced = True
         mock_get_course_run_details.return_value = {'weeks_to_complete': weeks_to_complete}
         enrollment = CourseEnrollment.enroll(self.user, self.course.id, CourseMode.AUDIT)
-        result = get_user_course_expiration_date(self.user, self.course)
+        result = get_user_course_expiration_date(
+            self.user,
+            CourseOverview.get_from_id(self.course.id),
+        )
         self.assertEqual(result, enrollment.created + access_duration)
 
     @mock.patch("openedx.features.course_duration_limits.access.get_course_run_details")
@@ -109,11 +112,17 @@ class CourseExpirationTestCase(ModuleStoreTestCase):
         start_date = now() - timedelta(weeks=10)
         past_course = CourseFactory(start=start_date)
         enrollment = CourseEnrollment.enroll(self.user, past_course.id, CourseMode.AUDIT)
-        result = get_user_course_expiration_date(self.user, past_course)
+        result = get_user_course_expiration_date(
+            self.user,
+            CourseOverview.get_from_id(past_course.id),
+        )
         self.assertEqual(result, None)
 
         add_course_mode(past_course, upgrade_deadline_expired=False)
-        result = get_user_course_expiration_date(self.user, past_course)
+        result = get_user_course_expiration_date(
+            self.user,
+            CourseOverview.get_from_id(past_course.id),
+        )
         content_availability_date = enrollment.created
         self.assertEqual(result, content_availability_date + access_duration)
 
@@ -121,12 +130,18 @@ class CourseExpirationTestCase(ModuleStoreTestCase):
         start_date = now() + timedelta(weeks=10)
         future_course = CourseFactory(start=start_date)
         enrollment = CourseEnrollment.enroll(self.user, future_course.id, CourseMode.AUDIT)
-        result = get_user_course_expiration_date(self.user, future_course)
+        result = get_user_course_expiration_date(
+            self.user,
+            CourseOverview.get_from_id(future_course.id),
+        )
         self.assertEqual(result, None)
 
         add_course_mode(future_course, upgrade_deadline_expired=False)
-        result = get_user_course_expiration_date(self.user, future_course)
-        content_availability_date = start_date
+        result = get_user_course_expiration_date(
+            self.user,
+            CourseOverview.get_from_id(future_course.id),
+        )
+        content_availability_date = start_date.replace(microsecond=0)
         self.assertEqual(result, content_availability_date + access_duration)
 
     @mock.patch("openedx.features.course_duration_limits.access.get_course_run_details")
@@ -141,7 +156,10 @@ class CourseExpirationTestCase(ModuleStoreTestCase):
         course = CourseFactory(start=start_date)
         enrollment = CourseEnrollment.enroll(self.user, course.id, CourseMode.AUDIT)
         add_course_mode(course, upgrade_deadline_expired=True)
-        result = get_user_course_expiration_date(self.user, course)
+        result = get_user_course_expiration_date(
+            self.user,
+            CourseOverview.get_from_id(course.id),
+        )
         content_availability_date = enrollment.created
         self.assertEqual(result, content_availability_date + access_duration)
 

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)

openedx/features/course_experience/tests/views/test_course_home.py

@@ -204,7 +204,7 @@ class TestCourseHomePage(CourseHomePageTestCase):
 
         # Fetch the view and verify the query counts
         # TODO: decrease query count as part of REVO-28
-        with self.assertNumQueries(87, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST):
+        with self.assertNumQueries(89, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST):
             with check_mongo_calls(4):
                 url = course_home_url(self.course)
                 self.client.get(url)

openedx/features/course_experience/views/course_home.py

@@ -25,6 +25,7 @@ from lms.djangoapps.courseware.exceptions import CourseAccessRedirect
 from lms.djangoapps.courseware.views.views import CourseTabView
 from openedx.core.djangoapps.plugin_api.views import EdxFragmentView
 from openedx.core.djangoapps.util.maintenance_banner import add_maintenance_banner
+from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
 from openedx.features.course_experience.course_tools import CourseToolsPluginManager
 from openedx.features.course_duration_limits.access import generate_course_expired_fragment
 from student.models import CourseEnrollment
@@ -153,7 +154,10 @@ class CourseHomeFragmentView(EdxFragmentView):
             course_sock_fragment = CourseSockFragmentView().render_to_fragment(request, course=course, **kwargs)
             has_visited_course, resume_course_url = self._get_resume_course_info(request, course_id)
             handouts_html = self._get_course_handouts(request, course)
-            course_expiration_fragment = generate_course_expired_fragment(request.user, course)
+            course_expiration_fragment = generate_course_expired_fragment(
+                request.user,
+                CourseOverview.get_from_id(course.id)
+            )
         elif allow_public_outline or allow_public:
             outline_fragment = CourseOutlineFragmentView().render_to_fragment(
                 request, course_id=course_id, user_is_enrolled=False, **kwargs