Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #19658 from edx/revert-19642-fbe-improve-course-api-performance

Browse Source 
Revert "Allow courses api to return data incrementally"
This commit is contained in:
Calen Pennington
2019-01-25 13:01:20 -05:00
committed by GitHub
parent c9ed838991 805f32a357
commit 2f0cd83ed2
19 changed files with 67 additions and 366 deletions
Download Patch File Download Diff File Expand all files Collapse all files

36
common/djangoapps/course_modes/models.py
View File

@@ -312,7 +312,7 @@ class CourseMode(models.Model):
@classmethod
@request_cached(CACHE_NAMESPACE)
def modes_for_course(cls, course_id=None, include_expired=False, only_selectable=True, course=None):
def modes_for_course(cls, course_id, include_expired=False, only_selectable=True):
"""
Returns a list of the non-expired modes for a given course id
@@ -330,25 +330,11 @@ 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
"""
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
found_course_modes = cls.objects.filter(course_id=course_id)
# Filter out expired course modes if include_expired is not set
if not include_expired:
@@ -361,10 +347,7 @@ 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:
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)
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:
@@ -373,7 +356,7 @@ class CourseMode(models.Model):
return modes
@classmethod
def modes_for_course_dict(cls, course_id=None, modes=None, **kwargs):
def modes_for_course_dict(cls, course_id, modes=None, **kwargs):
"""Returns the non-expired modes for a particular course.
Arguments:
@@ -435,7 +418,7 @@ class CourseMode(models.Model):
return None
@classmethod
def verified_mode_for_course(cls, course_id=None, modes=None, include_expired=False, course=None):
def verified_mode_for_course(cls, course_id, modes=None, include_expired=False):
"""Find a verified mode for a particular course.
Since we have multiple modes that can go through the verify flow,
@@ -456,12 +439,7 @@ class CourseMode(models.Model):
Mode or None
"""
modes_dict = cls.modes_for_course_dict(
course_id=course_id,
modes=modes,
include_expired=include_expired,
course=course
)
modes_dict = cls.modes_for_course_dict(course_id, modes=modes, include_expired=include_expired)
verified_mode = modes_dict.get('verified', None)
professional_mode = modes_dict.get('professional', None)
# we prefer professional over verify
@@ -725,7 +703,7 @@ class CourseMode(models.Model):
Takes a mode model and turns it into a model named tuple.
Returns:
A 'Mode' namedtuple with all the same attributes as the model.
A 'Model' namedtuple with all the same attributes as the model.
"""
return Mode(

11
lms/djangoapps/branding/__init__.py
View File

@@ -15,7 +15,7 @@ from openedx.core.djangoapps.site_configuration import helpers as configuration_
def get_visible_courses(org=None, filter_=None):
"""
Yield the CourseOverviews that should be visible in this branded
Return the set of CourseOverviews that should be visible in this branded
instance.
Arguments:
@@ -27,10 +27,9 @@ 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:
@@ -41,7 +40,7 @@ def get_visible_courses(org=None, filter_=None):
else:
courses = CourseOverview.get_all_courses(filter_=filter_)
courses = courses.order_by('id')
courses = sorted(courses, key=lambda course: course.number)
# Filtering can stop here.
if current_site_orgs:
@@ -58,11 +57,11 @@ def get_visible_courses(org=None, filter_=None):
)
if filtered_visible_ids:
return courses.filter(id__in=filtered_visible_ids)
return [course for course in courses if course.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 courses.exclude(org__in=orgs)
return [course for course in courses if course.location.org not in orgs]
def get_university_for_request():

4
lms/djangoapps/course_api/api.py
View File

@@ -57,7 +57,7 @@ def course_detail(request, username, course_key):
def list_courses(request, username, org=None, filter_=None):
"""
Yield all available courses.
Return a list of 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:
Yield `CourseOverview` objects representing the collection of courses.
List of `CourseOverview` objects representing the collection of courses.
"""
user = get_effective_user(request.user, username)
return get_courses(user, org=org, filter_=filter_)

4
lms/djangoapps/course_api/tests/test_api.py
View File

@@ -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(list(courses)), 0)
self.assertEqual(len(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(list(courses)), 0)
self.assertEqual(len(courses), 0)
def test_hidden_course_for_staff(self):
self.create_course(visible_to_staff_only=True)

63
lms/djangoapps/course_api/tests/test_views.py
View File

@@ -1,7 +1,6 @@
"""
Tests for Course API views.
"""
from datetime import datetime
import ddt
from hashlib import md5
@@ -13,14 +12,8 @@ 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
@@ -290,7 +283,6 @@ class CourseListSearchViewTest(CourseApiTestViewMixin, ModuleStoreTestCase, Sear
"""
ENABLED_SIGNALS = ['course_published']
ENABLED_CACHES = ModuleStoreTestCase.ENABLED_CACHES + ['configuration']
def setUp(self):
super(CourseListSearchViewTest, self).setUp()
@@ -306,7 +298,6 @@ 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):
"""
@@ -356,56 +347,4 @@ 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'], [])
# 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]
)
self.assertEqual(res.data['pagination']['count'], 1) # Should list a single course

16
lms/djangoapps/course_api/views.py
View File

@@ -10,7 +10,6 @@ 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
@@ -244,7 +243,7 @@ class CourseListView(DeveloperErrorViewMixin, ListAPIView):
def get_queryset(self):
"""
Yield courses visible to the user.
Return a list of courses visible to the user.
"""
form = CourseListGetForm(self.request.query_params, initial={'requesting_user': self.request.user})
if not form.is_valid():
@@ -265,12 +264,9 @@ class CourseListView(DeveloperErrorViewMixin, ListAPIView):
size=self.results_size_infinity,
)
search_courses_ids = {course['data']['id'] for course in search_courses['results']}
search_courses_ids = {course['data']['id']: True for course in search_courses['results']}
return LazySequence(
(
course for course in db_courses
if unicode(course.id) in search_courses_ids
),
est_len=len(db_courses)
)
return [
course for course in db_courses
if unicode(course.id) in search_courses_ids
]

25
lms/djangoapps/courseware/courses.py
View File

@@ -23,9 +23,7 @@ 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
@@ -37,7 +35,6 @@ 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
@@ -454,29 +451,19 @@ def get_course_syllabus_section(course, section_key):
def get_courses(user, org=None, filter_=None):
"""
Return a LazySequence of courses available, optionally filtered by org code (case-insensitive).
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_,
).prefetch_related(
Prefetch(
'modes',
queryset=CourseMode.objects.exclude(mode_slug__in=CourseMode.CREDIT_MODES),
to_attr='selectable_modes',
),
'image_set',
)
courses = branding.get_visible_courses(org=org, filter_=filter_)
permission_name = configuration_helpers.get_value(
'COURSE_CATALOG_VISIBILITY_PERMISSION',
settings.COURSE_CATALOG_VISIBILITY_PERMISSION
)
return LazySequence(
(c for c in courses if has_access(user, permission_name, c)),
est_len=courses.count()
)
courses = [c for c in courses if has_access(user, permission_name, c)]
return courses
def get_permission_for_course_about():

2
lms/djangoapps/courseware/tests/test_courses.py
View File

@@ -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(list(no_courses), [])
self.assertEqual(no_courses, [])
# Request filtering for an org matching the designated org.
site_courses = get_courses(user, org=alternate)

3
openedx/core/djangoapps/config_model_utils/models.py
View File

@@ -22,7 +22,6 @@ 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):
@@ -253,9 +252,7 @@ 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:

5
openedx/core/djangoapps/content/course_overviews/models.py
View File

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

3
openedx/core/djangolib/testing/utils.py
View File

@@ -73,9 +73,6 @@ 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
})

98
openedx/core/lib/api/view_utils.py
View File

@@ -1,7 +1,6 @@
"""
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 _
@@ -206,100 +205,3 @@ 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,
)

43
openedx/core/lib/cache_utils.py
View File

@@ -9,7 +9,6 @@ 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):
@@ -48,32 +47,38 @@ 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.
"""
@wrapt.decorator
def decorator(wrapped, instance, args, kwargs):
def decorator(f):
"""
Arguments:
args, kwargs: values passed into the wrapped function
f (func): the function to wrap
"""
# 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)
@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(wrapped, arg_map_function, *args, **kwargs)
cached_response = request_cache.get_cached_response(cache_key)
if cached_response.is_found:
return cached_response.value
if request_cache:
cache_key = _func_call_cache_key(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 = wrapped(*args, **kwargs)
result = f(*args, **kwargs)
if request_cache:
request_cache.set(cache_key, result)
if request_cache:
request_cache.set(cache_key, result)
return result
return result
return _decorator
return decorator

37
openedx/features/content_type_gating/tests/test_models.py
View File

@@ -6,7 +6,6 @@ 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
@@ -234,14 +233,10 @@ 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)
@@ -249,8 +244,6 @@ 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)
@@ -260,14 +253,10 @@ 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)
@@ -275,8 +264,6 @@ 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)
@@ -284,8 +271,6 @@ 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)
@@ -296,14 +281,10 @@ 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)
@@ -311,8 +292,6 @@ 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)
@@ -320,8 +299,6 @@ 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)
@@ -329,8 +306,6 @@ 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)
@@ -341,14 +316,10 @@ 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)
@@ -356,8 +327,6 @@ 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)
@@ -365,8 +334,6 @@ 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)
@@ -374,8 +341,6 @@ 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)
@@ -383,8 +348,6 @@ 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)

4
openedx/features/course_duration_limits/access.py
View File

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

34
openedx/features/course_duration_limits/tests/test_course_expiration.py
View File

@@ -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, CourseOverview.get_from_id(self.course.id))
result = get_user_course_expiration_date(self.user, self.course)
self.assertEqual(result, None)
@mock.patch("openedx.features.course_duration_limits.access.get_course_run_details")
@@ -94,10 +94,7 @@ 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,
CourseOverview.get_from_id(self.course.id),
)
result = get_user_course_expiration_date(self.user, self.course)
self.assertEqual(result, enrollment.created + access_duration)
@mock.patch("openedx.features.course_duration_limits.access.get_course_run_details")
@@ -112,17 +109,11 @@ 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,
CourseOverview.get_from_id(past_course.id),
)
result = get_user_course_expiration_date(self.user, past_course)
self.assertEqual(result, None)
add_course_mode(past_course, upgrade_deadline_expired=False)
result = get_user_course_expiration_date(
self.user,
CourseOverview.get_from_id(past_course.id),
)
result = get_user_course_expiration_date(self.user, past_course)
content_availability_date = enrollment.created
self.assertEqual(result, content_availability_date + access_duration)
@@ -130,18 +121,12 @@ 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,
CourseOverview.get_from_id(future_course.id),
)
result = get_user_course_expiration_date(self.user, future_course)
self.assertEqual(result, None)
add_course_mode(future_course, upgrade_deadline_expired=False)
result = get_user_course_expiration_date(
self.user,
CourseOverview.get_from_id(future_course.id),
)
content_availability_date = start_date.replace(microsecond=0)
result = get_user_course_expiration_date(self.user, future_course)
content_availability_date = start_date
self.assertEqual(result, content_availability_date + access_duration)
@mock.patch("openedx.features.course_duration_limits.access.get_course_run_details")
@@ -156,10 +141,7 @@ 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,
CourseOverview.get_from_id(course.id),
)
result = get_user_course_expiration_date(self.user, course)
content_availability_date = enrollment.created
self.assertEqual(result, content_availability_date + access_duration)

37
openedx/features/course_duration_limits/tests/test_models.py
View File

@@ -10,7 +10,6 @@ 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
@@ -267,14 +266,10 @@ 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)
@@ -282,8 +277,6 @@ 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)
@@ -293,14 +286,10 @@ 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)
@@ -308,8 +297,6 @@ 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)
@@ -317,8 +304,6 @@ 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)
@@ -329,14 +314,10 @@ 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)
@@ -344,8 +325,6 @@ 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)
@@ -353,8 +332,6 @@ 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)
@@ -362,8 +339,6 @@ 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)
@@ -374,14 +349,10 @@ 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)
@@ -389,8 +360,6 @@ 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)
@@ -398,8 +367,6 @@ 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)
@@ -407,8 +374,6 @@ 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)
@@ -416,8 +381,6 @@ 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)

2
openedx/features/course_experience/tests/views/test_course_home.py
View File

@@ -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(89, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST):
with self.assertNumQueries(87, table_blacklist=QUERY_COUNT_TABLE_BLACKLIST):
with check_mongo_calls(4):
url = course_home_url(self.course)
self.client.get(url)

6
openedx/features/course_experience/views/course_home.py
View File

@@ -25,7 +25,6 @@ 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
@@ -154,10 +153,7 @@ 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,
CourseOverview.get_from_id(course.id)
)
course_expiration_fragment = generate_course_expired_fragment(request.user, course)
elif allow_public_outline or allow_public:
outline_fragment = CourseOutlineFragmentView().render_to_fragment(
request, course_id=course_id, user_is_enrolled=False, **kwargs