diff --git a/common/djangoapps/course_modes/models.py b/common/djangoapps/course_modes/models.py
index c2645fdbee3c7669af3ea134f3682715d94b80b1..9f80c4e733a0222b5f4648ec525f4fae3263ed9f 100644
--- a/common/djangoapps/course_modes/models.py
+++ b/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(
diff --git a/lms/djangoapps/branding/__init__.py b/lms/djangoapps/branding/__init__.py
index 534c0c4faffa332b8220a535794a889bf3da2306..e467d96ef4060ec79a43836d1b0a491808353acd 100644
--- a/lms/djangoapps/branding/__init__.py
+++ b/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():
diff --git a/lms/djangoapps/course_api/api.py b/lms/djangoapps/course_api/api.py
index 79a3b387963a7b9159cf7f6dc28f48573dddbe56..ef25e61399b3e97c821b512c47c2e19ff6f288e1 100644
--- a/lms/djangoapps/course_api/api.py
+++ b/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_)
diff --git a/lms/djangoapps/course_api/tests/test_api.py b/lms/djangoapps/course_api/tests/test_api.py
index cef5d4c73978d3d02192d9398e4a286b982a39c6..7670acbfa1d4775cffc42105441fd5ef6686cedf 100644
--- a/lms/djangoapps/course_api/tests/test_api.py
+++ b/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)
diff --git a/lms/djangoapps/course_api/tests/test_views.py b/lms/djangoapps/course_api/tests/test_views.py
index 974752be37a5d22bd2b620f05ff8e2ccb4a0591c..641a7c915d4899c24fd2091cbf2386c9f6322216 100644
--- a/lms/djangoapps/course_api/tests/test_views.py
+++ b/lms/djangoapps/course_api/tests/test_views.py
@@ -1,6 +1,7 @@
"""
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]
+ )
diff --git a/lms/djangoapps/course_api/views.py b/lms/djangoapps/course_api/views.py
index 66743ea0798ccbddae70a3cd599f66f45549d60c..26d798634243b7eee9ec058ada9bbd4462696473 100644
--- a/lms/djangoapps/course_api/views.py
+++ b/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)
+ )
diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py
index caccff13132dcadd1c272e1924d820ca151e3c57..30461cbf1f0df94b0a0e0adcc0c370623d7e5330 100644
--- a/lms/djangoapps/courseware/courses.py
+++ b/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():
diff --git a/lms/djangoapps/courseware/tests/test_courses.py b/lms/djangoapps/courseware/tests/test_courses.py
index c54d98e4e204ecaadbf48157496057fb67a86c70..12d694524c993368acc4d73ac1c1801603b86914 100644
--- a/lms/djangoapps/courseware/tests/test_courses.py
+++ b/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)
diff --git a/openedx/core/djangoapps/config_model_utils/models.py b/openedx/core/djangoapps/config_model_utils/models.py
index 7e47db7d1b3b500ef622b4af873fcab391b9746d..2d29ac360557b1987f4ee20684d21d95030c0c14 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/djangoapps/content/course_overviews/models.py b/openedx/core/djangoapps/content/course_overviews/models.py
index e852cce1b05343bbd815231794e2b6dd41e3075d..6d1998c92f06a29d052b97fa0a22dd641d264d0e 100644
--- a/openedx/core/djangoapps/content/course_overviews/models.py
+++ b/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
diff --git a/openedx/core/djangolib/testing/utils.py b/openedx/core/djangolib/testing/utils.py
index 710f5a35ba75780efd098cbbd83b1ed7a536b266..3d620385e8d496ec333a4757d0c916e44d0c55b3 100644
--- a/openedx/core/djangolib/testing/utils.py
+++ b/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
})
diff --git a/openedx/core/lib/api/view_utils.py b/openedx/core/lib/api/view_utils.py
index ef67c4eb82819d0b2f9e2d4d5756aacfcdf9e7fc..61c8dbd319270bfc8d998097ae653c7f78aa9e61 100644
--- a/openedx/core/lib/api/view_utils.py
+++ b/openedx/core/lib/api/view_utils.py
@@ -1,6 +1,7 @@
"""
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,
+ )
diff --git a/openedx/core/lib/cache_utils.py b/openedx/core/lib/cache_utils.py
index ba4324143d106d9657c751a489c1f4e1c6e30801..3910d33f7c09bb2af8e043f4d7a37f0c6549af2c 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)
-
- 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
diff --git a/openedx/features/content_type_gating/tests/test_models.py b/openedx/features/content_type_gating/tests/test_models.py
index e51a16b5298685afacd42f387e8cb343403685e5..aca44f17bcf62e589cb3228f26197fdca7781843 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/access.py b/openedx/features/course_duration_limits/access.py
index a024475a23cd43843e52c9567d108546855e2338..7d4f1c3abed066ff51c9d22a02b7420db3402fb3 100644
--- a/openedx/features/course_duration_limits/access.py
+++ b/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)
diff --git a/openedx/features/course_duration_limits/tests/test_course_expiration.py b/openedx/features/course_duration_limits/tests/test_course_expiration.py
index a87abe0825f7d70bb071692e23f2f0d69430a3c2..fb957ab6974faadd733d259c2b34f5740206867a 100644
--- a/openedx/features/course_duration_limits/tests/test_course_expiration.py
+++ b/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)
diff --git a/openedx/features/course_duration_limits/tests/test_models.py b/openedx/features/course_duration_limits/tests/test_models.py
index 6a6db087cdbf1d8cb7c0b4ea5a46b51bf0b93af1..584a4b210b9de909f0d424cbd4df1cc3202d421a 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)
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 048ebcf8c4ebcf7161d4c8008f691e244a3bbc9a..9975da81f9f91ab5ed3f017de93729e1ca30e571 100644
--- a/openedx/features/course_experience/tests/views/test_course_home.py
+++ b/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)
diff --git a/openedx/features/course_experience/views/course_home.py b/openedx/features/course_experience/views/course_home.py
index 5c1449c3f87fb96531eca6099dc5a82e8561f069..d3fe698adeb00222f369d9655a79b8454fd3bcaf 100644
--- a/openedx/features/course_experience/views/course_home.py
+++ b/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