Commit 844edac4 by Dennis Jen

Optimize OpenID Course Claims for users.

- Course overviews will cache courses upon publish.
- Added management command to warm up cache.
- OAuth2 handler returns courses via course overviews.
parent 6454ffc3
...@@ -488,21 +488,18 @@ class DashboardTest(ModuleStoreTestCase): ...@@ -488,21 +488,18 @@ class DashboardTest(ModuleStoreTestCase):
self.assertContains(response, expected_url) self.assertContains(response, expected_url)
@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
@ddt.data((ModuleStoreEnum.Type.mongo, 1), (ModuleStoreEnum.Type.split, 3)) @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
@ddt.unpack def test_dashboard_metadata_caching(self, modulestore_type):
def test_dashboard_metadata_caching(self, modulestore_type, expected_mongo_calls):
""" """
Check that the student dashboard makes use of course metadata caching. Check that the student dashboard makes use of course metadata caching.
After enrolling a student in a course, that course's metadata should be After creating a course, that course's metadata should be cached as a
cached as a CourseOverview. The student dashboard should never have to make CourseOverview. The student dashboard should never have to make calls to
calls to the modulestore. the modulestore.
Arguments: Arguments:
modulestore_type (ModuleStoreEnum.Type): Type of modulestore to create modulestore_type (ModuleStoreEnum.Type): Type of modulestore to create
test course in. test course in.
expected_mongo_calls (int >=0): Number of MongoDB queries expected for
a single call to the module store.
Note to future developers: Note to future developers:
If you break this test so that the "check_mongo_calls(0)" fails, If you break this test so that the "check_mongo_calls(0)" fails,
...@@ -512,11 +509,11 @@ class DashboardTest(ModuleStoreTestCase): ...@@ -512,11 +509,11 @@ class DashboardTest(ModuleStoreTestCase):
CourseDescriptor isn't necessary. CourseDescriptor isn't necessary.
""" """
# Create a course and log in the user. # Create a course and log in the user.
test_course = CourseFactory.create(default_store=modulestore_type) # Creating a new course will trigger a publish event and the course will be cached
test_course = CourseFactory.create(default_store=modulestore_type, emit_signals=True)
self.client.login(username="jack", password="test") self.client.login(username="jack", password="test")
# Enrolling the user in the course will result in a modulestore query. with check_mongo_calls(0):
with check_mongo_calls(expected_mongo_calls):
CourseEnrollment.enroll(self.user, test_course.id) CourseEnrollment.enroll(self.user, test_course.id)
# Subsequent requests will only result in SQL queries to load the # Subsequent requests will only result in SQL queries to load the
......
...@@ -114,7 +114,7 @@ class CourseFactory(XModuleFactory): ...@@ -114,7 +114,7 @@ class CourseFactory(XModuleFactory):
name = kwargs.get('name', kwargs.get('run', Location.clean(kwargs.get('display_name')))) name = kwargs.get('name', kwargs.get('run', Location.clean(kwargs.get('display_name'))))
run = kwargs.pop('run', name) run = kwargs.pop('run', name)
user_id = kwargs.pop('user_id', ModuleStoreEnum.UserID.test) user_id = kwargs.pop('user_id', ModuleStoreEnum.UserID.test)
emit_signals = kwargs.get('emit_signals', False) emit_signals = kwargs.pop('emit_signals', False)
# Pass the metadata just as field=value pairs # Pass the metadata just as field=value pairs
kwargs.update(kwargs.pop('metadata', {})) kwargs.update(kwargs.pop('metadata', {}))
......
...@@ -94,17 +94,14 @@ class TestSendCCXCoursePublished(ModuleStoreTestCase): ...@@ -94,17 +94,14 @@ class TestSendCCXCoursePublished(ModuleStoreTestCase):
structure = CourseStructure.objects.get(course_id=course_key) structure = CourseStructure.objects.get(course_id=course_key)
self.assertEqual(structure.structure, ccx_structure) self.assertEqual(structure.structure, ccx_structure)
def test_course_overview_deleted(self): def test_course_overview_cached(self):
"""Check that course overview is deleted after course published signal is sent """Check that course overview is cached after course published signal is sent
""" """
course_key = CCXLocator.from_course_locator(self.course.id, self.ccx.id) course_key = CCXLocator.from_course_locator(self.course.id, self.ccx.id)
overview = CourseOverview(id=course_key)
overview.version = 1
overview.save()
overview = CourseOverview.objects.filter(id=course_key) overview = CourseOverview.objects.filter(id=course_key)
self.assertEqual(len(overview), 1) self.assertEqual(len(overview), 0)
with mock_signal_receiver(SignalHandler.course_published) as receiver: with mock_signal_receiver(SignalHandler.course_published) as receiver:
self.call_fut(self.course.id) self.call_fut(self.course.id)
self.assertEqual(receiver.call_count, 3) self.assertEqual(receiver.call_count, 3)
overview = CourseOverview.objects.filter(id=course_key) overview = CourseOverview.objects.filter(id=course_key)
self.assertEqual(len(overview), 0) self.assertEqual(len(overview), 1)
...@@ -2,9 +2,9 @@ ...@@ -2,9 +2,9 @@
from django.conf import settings from django.conf import settings
from django.core.cache import cache from django.core.cache import cache
from xmodule.modulestore.django import modulestore
from courseware.access import has_access from courseware.access import has_access
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from openedx.core.djangoapps.user_api.models import UserPreference from openedx.core.djangoapps.user_api.models import UserPreference
from student.models import anonymous_id_for_user from student.models import anonymous_id_for_user
from student.models import UserProfile from student.models import UserProfile
...@@ -200,13 +200,13 @@ class CourseAccessHandler(object): ...@@ -200,13 +200,13 @@ class CourseAccessHandler(object):
course_ids = cache.get(key) course_ids = cache.get(key)
if not course_ids: if not course_ids:
courses = _get_all_courses() course_keys = CourseOverview.get_all_course_keys()
# Global staff have access to all courses. Filter courses for non-global staff. # Global staff have access to all courses. Filter courses for non-global staff.
if not GlobalStaff().has_user(user): if not GlobalStaff().has_user(user):
courses = [course for course in courses if has_access(user, access_type, course)] course_keys = [course_key for course_key in course_keys if has_access(user, access_type, course_key)]
course_ids = [unicode(course.id) for course in courses] course_ids = [unicode(course_key) for course_key in course_keys]
cache.set(key, course_ids, self.COURSE_CACHE_TIMEOUT) cache.set(key, course_ids, self.COURSE_CACHE_TIMEOUT)
...@@ -234,12 +234,3 @@ class IDTokenHandler(OpenIDHandler, ProfileHandler, CourseAccessHandler, Permiss ...@@ -234,12 +234,3 @@ class IDTokenHandler(OpenIDHandler, ProfileHandler, CourseAccessHandler, Permiss
class UserInfoHandler(OpenIDHandler, ProfileHandler, CourseAccessHandler, PermissionsHandler): class UserInfoHandler(OpenIDHandler, ProfileHandler, CourseAccessHandler, PermissionsHandler):
""" Configure the UserInfo handler for the LMS. """ """ Configure the UserInfo handler for the LMS. """
pass pass
def _get_all_courses():
""" Utility function to list all available courses. """
ms_courses = modulestore().get_courses()
courses = [course for course in ms_courses if course.scope_ids.block_type == 'course']
return courses
...@@ -2,16 +2,17 @@ ...@@ -2,16 +2,17 @@
from django.core.cache import cache from django.core.cache import cache
from django.test.utils import override_settings from django.test.utils import override_settings
from lang_pref import LANGUAGE_KEY from lang_pref import LANGUAGE_KEY
from opaque_keys.edx.locations import SlashSeparatedCourseKey
from xmodule.modulestore.tests.django_utils import TEST_DATA_MIXED_TOY_MODULESTORE from xmodule.modulestore.tests.factories import (check_mongo_calls, CourseFactory)
from student.models import anonymous_id_for_user from student.models import anonymous_id_for_user
from student.models import UserProfile from student.models import UserProfile
from student.roles import CourseStaffRole, CourseInstructorRole from student.roles import (CourseInstructorRole, CourseStaffRole, GlobalStaff,
OrgInstructorRole, OrgStaffRole)
from student.tests.factories import UserFactory, UserProfileFactory from student.tests.factories import UserFactory, UserProfileFactory
from openedx.core.djangoapps.user_api.preferences.api import set_user_preference from openedx.core.djangoapps.user_api.preferences.api import set_user_preference
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
# Will also run default tests for IDTokens and UserInfo # Will also run default tests for IDTokens and UserInfo
from oauth2_provider.tests import IDTokenTestCase, UserInfoTestCase from oauth2_provider.tests import IDTokenTestCase, UserInfoTestCase
...@@ -19,14 +20,10 @@ from oauth2_provider.tests import IDTokenTestCase, UserInfoTestCase ...@@ -19,14 +20,10 @@ from oauth2_provider.tests import IDTokenTestCase, UserInfoTestCase
class BaseTestMixin(ModuleStoreTestCase): class BaseTestMixin(ModuleStoreTestCase):
profile = None profile = None
MODULESTORE = TEST_DATA_MIXED_TOY_MODULESTORE
def setUp(self): def setUp(self):
super(BaseTestMixin, self).setUp() super(BaseTestMixin, self).setUp()
self.course_key = CourseFactory.create(emit_signals=True).id
self.course_key = SlashSeparatedCourseKey('edX', 'toy', '2012_Fall')
self.course_id = unicode(self.course_key) self.course_id = unicode(self.course_key)
self.user_factory = UserFactory self.user_factory = UserFactory
self.set_user(self.make_user()) self.set_user(self.make_user())
...@@ -77,7 +74,8 @@ class IDTokenTest(BaseTestMixin, IDTokenTestCase): ...@@ -77,7 +74,8 @@ class IDTokenTest(BaseTestMixin, IDTokenTestCase):
self.assertEqual(language, locale) self.assertEqual(language, locale)
def test_no_special_course_access(self): def test_no_special_course_access(self):
scopes, claims = self.get_id_token_values('openid course_instructor course_staff') with check_mongo_calls(0):
scopes, claims = self.get_id_token_values('openid course_instructor course_staff')
self.assertNotIn('course_staff', scopes) self.assertNotIn('course_staff', scopes)
self.assertNotIn('staff_courses', claims) self.assertNotIn('staff_courses', claims)
...@@ -86,14 +84,15 @@ class IDTokenTest(BaseTestMixin, IDTokenTestCase): ...@@ -86,14 +84,15 @@ class IDTokenTest(BaseTestMixin, IDTokenTestCase):
def test_course_staff_courses(self): def test_course_staff_courses(self):
CourseStaffRole(self.course_key).add_users(self.user) CourseStaffRole(self.course_key).add_users(self.user)
with check_mongo_calls(0):
scopes, claims = self.get_id_token_values('openid course_staff') scopes, claims = self.get_id_token_values('openid course_staff')
self.assertIn('course_staff', scopes) self.assertIn('course_staff', scopes)
self.assertNotIn('staff_courses', claims) # should not return courses in id_token self.assertNotIn('staff_courses', claims) # should not return courses in id_token
def test_course_instructor_courses(self): def test_course_instructor_courses(self):
CourseInstructorRole(self.course_key).add_users(self.user) with check_mongo_calls(0):
CourseInstructorRole(self.course_key).add_users(self.user)
scopes, claims = self.get_id_token_values('openid course_instructor') scopes, claims = self.get_id_token_values('openid course_instructor')
...@@ -104,6 +103,7 @@ class IDTokenTest(BaseTestMixin, IDTokenTestCase): ...@@ -104,6 +103,7 @@ class IDTokenTest(BaseTestMixin, IDTokenTestCase):
CourseStaffRole(self.course_key).add_users(self.user) CourseStaffRole(self.course_key).add_users(self.user)
course_id = unicode(self.course_key) course_id = unicode(self.course_key)
nonexistent_course_id = 'some/other/course' nonexistent_course_id = 'some/other/course'
claims = { claims = {
...@@ -113,7 +113,8 @@ class IDTokenTest(BaseTestMixin, IDTokenTestCase): ...@@ -113,7 +113,8 @@ class IDTokenTest(BaseTestMixin, IDTokenTestCase):
} }
} }
scopes, claims = self.get_id_token_values(scope='openid course_staff', claims=claims) with check_mongo_calls(0):
scopes, claims = self.get_id_token_values(scope='openid course_staff', claims=claims)
self.assertIn('course_staff', scopes) self.assertIn('course_staff', scopes)
self.assertIn('staff_courses', claims) self.assertIn('staff_courses', claims)
...@@ -133,6 +134,11 @@ class IDTokenTest(BaseTestMixin, IDTokenTestCase): ...@@ -133,6 +134,11 @@ class IDTokenTest(BaseTestMixin, IDTokenTestCase):
class UserInfoTest(BaseTestMixin, UserInfoTestCase): class UserInfoTest(BaseTestMixin, UserInfoTestCase):
def setUp(self):
super(UserInfoTest, self).setUp()
# create another course in the DB that only global staff have access to
CourseFactory.create(emit_signals=True)
def token_for_scope(self, scope): def token_for_scope(self, scope):
full_scope = 'openid %s' % scope full_scope = 'openid %s' % scope
self.set_access_token_scope(full_scope) self.set_access_token_scope(full_scope)
...@@ -158,43 +164,64 @@ class UserInfoTest(BaseTestMixin, UserInfoTestCase): ...@@ -158,43 +164,64 @@ class UserInfoTest(BaseTestMixin, UserInfoTestCase):
self.assertEqual(result.status_code, 200) self.assertEqual(result.status_code, 200)
return claims return claims
def test_request_staff_courses_using_scope(self): def _assert_role_using_scope(self, scope, claim, assert_one_course=True):
CourseStaffRole(self.course_key).add_users(self.user) with check_mongo_calls(0):
claims = self.get_with_scope('course_staff') claims = self.get_with_scope(scope)
self.assertEqual(len(claims), 2)
courses = claims['staff_courses'] courses = claims[claim]
self.assertIn(self.course_id, courses) self.assertIn(self.course_id, courses)
self.assertEqual(len(courses), 1) if assert_one_course:
self.assertEqual(len(courses), 1)
def test_request_instructor_courses_using_scope(self): def test_request_global_staff_courses_using_scope(self):
CourseInstructorRole(self.course_key).add_users(self.user) GlobalStaff().add_users(self.user)
claims = self.get_with_scope('course_instructor') self._assert_role_using_scope('course_staff', 'staff_courses', assert_one_course=False)
courses = claims['instructor_courses'] def test_request_org_staff_courses_using_scope(self):
self.assertIn(self.course_id, courses) OrgStaffRole(self.course_key.org).add_users(self.user)
self.assertEqual(len(courses), 1) self._assert_role_using_scope('course_staff', 'staff_courses')
def test_request_staff_courses_with_claims(self): def test_request_org_instructor_courses_using_scope(self):
OrgInstructorRole(self.course_key.org).add_users(self.user)
self._assert_role_using_scope('course_instructor', 'instructor_courses')
def test_request_staff_courses_using_scope(self):
CourseStaffRole(self.course_key).add_users(self.user) CourseStaffRole(self.course_key).add_users(self.user)
self._assert_role_using_scope('course_staff', 'staff_courses')
def test_request_instructor_courses_using_scope(self):
CourseInstructorRole(self.course_key).add_users(self.user)
self._assert_role_using_scope('course_instructor', 'instructor_courses')
def _assert_role_using_claim(self, scope, claim):
values = [self.course_id, 'some_invalid_course'] values = [self.course_id, 'some_invalid_course']
claims = self.get_with_claim_value('course_staff', 'staff_courses', values) with check_mongo_calls(0):
claims = self.get_with_claim_value(scope, claim, values)
self.assertEqual(len(claims), 2) self.assertEqual(len(claims), 2)
courses = claims['staff_courses'] courses = claims[claim]
self.assertIn(self.course_id, courses) self.assertIn(self.course_id, courses)
self.assertEqual(len(courses), 1) self.assertEqual(len(courses), 1)
def test_request_instructor_courses_with_claims(self): def test_request_global_staff_courses_with_claims(self):
CourseInstructorRole(self.course_key).add_users(self.user) GlobalStaff().add_users(self.user)
self._assert_role_using_claim('course_staff', 'staff_courses')
values = ['edX/toy/TT_2012_Fall', self.course_id, 'invalid_course_id'] def test_request_org_staff_courses_with_claims(self):
claims = self.get_with_claim_value('course_instructor', 'instructor_courses', values) OrgStaffRole(self.course_key.org).add_users(self.user)
self.assertEqual(len(claims), 2) self._assert_role_using_claim('course_staff', 'staff_courses')
courses = claims['instructor_courses'] def test_request_org_instructor_courses_with_claims(self):
self.assertIn(self.course_id, courses) OrgInstructorRole(self.course_key.org).add_users(self.user)
self.assertEqual(len(courses), 1) self._assert_role_using_claim('course_instructor', 'instructor_courses')
def test_request_staff_courses_with_claims(self):
CourseStaffRole(self.course_key).add_users(self.user)
self._assert_role_using_claim('course_staff', 'staff_courses')
def test_request_instructor_courses_with_claims(self):
CourseInstructorRole(self.course_key).add_users(self.user)
self._assert_role_using_claim('course_instructor', 'instructor_courses')
def test_permissions_scope(self): def test_permissions_scope(self):
claims = self.get_with_scope('permissions') claims = self.get_with_scope('permissions')
......
...@@ -269,6 +269,8 @@ OPENID_PROVIDER_TRUSTED_ROOTS = ['*'] ...@@ -269,6 +269,8 @@ OPENID_PROVIDER_TRUSTED_ROOTS = ['*']
############################## OAUTH2 Provider ################################ ############################## OAUTH2 Provider ################################
FEATURES['ENABLE_OAUTH2_PROVIDER'] = True FEATURES['ENABLE_OAUTH2_PROVIDER'] = True
# don't cache courses for testing
OIDC_COURSE_HANDLER_CACHE_TIMEOUT = 0
########################### External REST APIs ################################# ########################### External REST APIs #################################
FEATURES['ENABLE_MOBILE_REST_API'] = True FEATURES['ENABLE_MOBILE_REST_API'] = True
......
"""
Command to load course overviews.
"""
import logging
from optparse import make_option
from django.core.management.base import BaseCommand, CommandError
from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import CourseKey
from xmodule.modulestore.django import modulestore
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
log = logging.getLogger(__name__)
class Command(BaseCommand):
"""
Example usage:
$ ./manage.py lms generate_course_overview --all --settings=devstack
$ ./manage.py lms generate_course_overview 'edX/DemoX/Demo_Course' --settings=devstack
"""
args = '<course_id course_id ...>'
help = 'Generates and stores course overview for one or more courses.'
option_list = BaseCommand.option_list + (
make_option('--all',
action='store_true',
default=False,
help='Generate course overview for all courses.'),
)
def handle(self, *args, **options):
course_keys = []
if options['all']:
course_keys = [course.id for course in modulestore().get_courses()]
else:
if len(args) < 1:
raise CommandError('At least one course or --all must be specified.')
try:
course_keys = [CourseKey.from_string(arg) for arg in args]
except InvalidKeyError:
log.fatal('Invalid key specified.')
if not course_keys:
log.fatal('No courses specified.')
log.info('Generating course overview for %d courses.', len(course_keys))
log.debug('Generating course overview(s) for the following courses: %s', course_keys)
for course_key in course_keys:
try:
CourseOverview.get_from_id(course_key)
except Exception as ex: # pylint: disable=broad-except
log.exception('An error occurred while generating course overview for %s: %s', unicode(
course_key), ex.message)
log.info('Finished generating course overviews.')
# pylint: disable=missing-docstring
from django.core.management.base import CommandError
from mock import patch
from openedx.core.djangoapps.content.course_overviews.management.commands import generate_course_overview
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory
class TestGenerateCourseOverview(ModuleStoreTestCase):
"""
Tests course overview management command.
"""
def setUp(self):
"""
Create courses in modulestore.
"""
super(TestGenerateCourseOverview, self).setUp()
self.course_key_1 = CourseFactory.create().id
self.course_key_2 = CourseFactory.create().id
self.command = generate_course_overview.Command()
def _assert_courses_not_in_overview(self, *courses):
"""
Assert that courses doesn't exist in the course overviews.
"""
course_keys = CourseOverview.get_all_course_keys()
for expected_course_key in courses:
self.assertNotIn(expected_course_key, course_keys)
def _assert_courses_in_overview(self, *courses):
"""
Assert courses exists in course overviews.
"""
course_keys = CourseOverview.get_all_course_keys()
for expected_course_key in courses:
self.assertIn(expected_course_key, course_keys)
def test_generate_all(self):
"""
Test that all courses in the modulestore are loaded into course overviews.
"""
# ensure that the newly created courses aren't in course overviews
self._assert_courses_not_in_overview(self.course_key_1, self.course_key_2)
self.command.handle(all=True)
# CourseOverview will be populated with all courses in the modulestore
self._assert_courses_in_overview(self.course_key_1, self.course_key_2)
def test_generate_one(self):
"""
Test that a specified course is loaded into course overviews.
"""
self._assert_courses_not_in_overview(self.course_key_1, self.course_key_2)
self.command.handle(unicode(self.course_key_1), all=False)
self._assert_courses_in_overview(self.course_key_1)
self._assert_courses_not_in_overview(self.course_key_2)
@patch('openedx.core.djangoapps.content.course_overviews.management.commands.generate_course_overview.log')
def test_invalid_key(self, mock_log):
"""
Test that invalid key errors are logged.
"""
self.command.handle('not/found', all=False)
self.assertTrue(mock_log.fatal.called)
@patch('openedx.core.djangoapps.content.course_overviews.management.commands.generate_course_overview.log')
def test_not_found_key(self, mock_log):
"""
Test keys not found are logged.
"""
self.command.handle('fake/course/id', all=False)
self.assertTrue(mock_log.exception.called)
def test_no_params(self):
"""
Test exception raised when no parameters are specified.
"""
with self.assertRaises(CommandError):
self.command.handle(all=False)
...@@ -9,6 +9,7 @@ from django.db.utils import IntegrityError ...@@ -9,6 +9,7 @@ from django.db.utils import IntegrityError
from django.utils.translation import ugettext from django.utils.translation import ugettext
from model_utils.models import TimeStampedModel from model_utils.models import TimeStampedModel
from opaque_keys.edx.keys import CourseKey
from util.date_utils import strftime_localized from util.date_utils import strftime_localized
from xmodule import course_metadata_utils from xmodule import course_metadata_utils
from xmodule.course_module import CourseDescriptor from xmodule.course_module import CourseDescriptor
...@@ -151,7 +152,7 @@ class CourseOverview(TimeStampedModel): ...@@ -151,7 +152,7 @@ class CourseOverview(TimeStampedModel):
) )
@classmethod @classmethod
def _load_from_module_store(cls, course_id): def load_from_module_store(cls, course_id):
""" """
Load a CourseDescriptor, create a new CourseOverview from it, cache the Load a CourseDescriptor, create a new CourseOverview from it, cache the
overview, and return it. overview, and return it.
...@@ -225,7 +226,7 @@ class CourseOverview(TimeStampedModel): ...@@ -225,7 +226,7 @@ class CourseOverview(TimeStampedModel):
course_overview = None course_overview = None
except cls.DoesNotExist: except cls.DoesNotExist:
course_overview = None course_overview = None
return course_overview or cls._load_from_module_store(course_id) return course_overview or cls.load_from_module_store(course_id)
def clean_id(self, padding_char='='): def clean_id(self, padding_char='='):
""" """
...@@ -340,3 +341,13 @@ class CourseOverview(TimeStampedModel): ...@@ -340,3 +341,13 @@ class CourseOverview(TimeStampedModel):
Returns a list of ID strings for this course's prerequisite courses. Returns a list of ID strings for this course's prerequisite courses.
""" """
return json.loads(self._pre_requisite_courses_json) return json.loads(self._pre_requisite_courses_json)
@classmethod
def get_all_course_keys(cls):
"""
Returns all course keys from course overviews.
"""
return [
CourseKey.from_string(course_overview['id'])
for course_overview in CourseOverview.objects.values('id')
]
...@@ -11,9 +11,10 @@ from xmodule.modulestore.django import SignalHandler ...@@ -11,9 +11,10 @@ from xmodule.modulestore.django import SignalHandler
def _listen_for_course_publish(sender, course_key, **kwargs): # pylint: disable=unused-argument def _listen_for_course_publish(sender, course_key, **kwargs): # pylint: disable=unused-argument
""" """
Catches the signal that a course has been published in Studio and Catches the signal that a course has been published in Studio and
invalidates the corresponding CourseOverview cache entry if one exists. updates the corresponding CourseOverview cache entry.
""" """
CourseOverview.objects.filter(id=course_key).delete() CourseOverview.objects.filter(id=course_key).delete()
CourseOverview.load_from_module_store(course_key)
@receiver(SignalHandler.course_deleted) @receiver(SignalHandler.course_deleted)
......
...@@ -258,31 +258,22 @@ class CourseOverviewTestCase(ModuleStoreTestCase): ...@@ -258,31 +258,22 @@ class CourseOverviewTestCase(ModuleStoreTestCase):
self.store.delete_course(course.id, ModuleStoreEnum.UserID.test) self.store.delete_course(course.id, ModuleStoreEnum.UserID.test)
CourseOverview.get_from_id(course.id) CourseOverview.get_from_id(course.id)
@ddt.data((ModuleStoreEnum.Type.mongo, 1, 1), (ModuleStoreEnum.Type.split, 3, 4)) @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
@ddt.unpack def test_course_overview_caching(self, modulestore_type):
def test_course_overview_caching(self, modulestore_type, min_mongo_calls, max_mongo_calls):
""" """
Tests that CourseOverview structures are actually getting cached. Tests that CourseOverview structures are actually getting cached.
Arguments: Arguments:
modulestore_type (ModuleStoreEnum.Type): type of store to create the modulestore_type (ModuleStoreEnum.Type): type of store to create the
course in. course in.
min_mongo_calls (int): minimum number of MongoDB queries we expect
to be made.
max_mongo_calls (int): maximum number of MongoDB queries we expect
to be made.
""" """
course = CourseFactory.create(default_store=modulestore_type)
# The first time we load a CourseOverview, it will be a cache miss, so # Creating a new course will trigger a publish event and the course will be cached
# we expect the modulestore to be queried. course = CourseFactory.create(default_store=modulestore_type, emit_signals=True)
with check_mongo_calls_range(max_finds=max_mongo_calls, min_finds=min_mongo_calls):
_course_overview_1 = CourseOverview.get_from_id(course.id)
# The second time we load a CourseOverview, it will be a cache hit, so # The cache will be hit and mongo will not be queried
# we expect no modulestore queries to be made.
with check_mongo_calls(0): with check_mongo_calls(0):
_course_overview_2 = CourseOverview.get_from_id(course.id) CourseOverview.get_from_id(course.id)
@ddt.data(ModuleStoreEnum.Type.split, ModuleStoreEnum.Type.mongo) @ddt.data(ModuleStoreEnum.Type.split, ModuleStoreEnum.Type.mongo)
def test_get_non_existent_course(self, modulestore_type): def test_get_non_existent_course(self, modulestore_type):
...@@ -298,24 +289,18 @@ class CourseOverviewTestCase(ModuleStoreTestCase): ...@@ -298,24 +289,18 @@ class CourseOverviewTestCase(ModuleStoreTestCase):
with self.assertRaises(CourseOverview.DoesNotExist): with self.assertRaises(CourseOverview.DoesNotExist):
CourseOverview.get_from_id(store.make_course_key('Non', 'Existent', 'Course')) CourseOverview.get_from_id(store.make_course_key('Non', 'Existent', 'Course'))
@ddt.data(ModuleStoreEnum.Type.split, ModuleStoreEnum.Type.mongo) def test_get_errored_course(self):
def test_get_errored_course(self, modulestore_type):
""" """
Test that getting an ErrorDescriptor back from the module store causes Test that getting an ErrorDescriptor back from the module store causes
get_from_id to raise an IOError. load_from_module_store to raise an IOError.
Arguments:
modulestore_type (ModuleStoreEnum.Type): type of store to create the
course in.
""" """
course = CourseFactory.create(default_store=modulestore_type)
mock_get_course = mock.Mock(return_value=ErrorDescriptor) mock_get_course = mock.Mock(return_value=ErrorDescriptor)
with mock.patch('xmodule.modulestore.mixed.MixedModuleStore.get_course', mock_get_course): with mock.patch('xmodule.modulestore.mixed.MixedModuleStore.get_course', mock_get_course):
# This mock makes it so when the module store tries to load course data, # This mock makes it so when the module store tries to load course data,
# an exception is thrown, which causes get_course to return an ErrorDescriptor, # an exception is thrown, which causes get_course to return an ErrorDescriptor,
# which causes get_from_id to raise an IOError. # which causes get_from_id to raise an IOError.
with self.assertRaises(IOError): with self.assertRaises(IOError):
CourseOverview.get_from_id(course.id) CourseOverview.load_from_module_store(self.store.make_course_key('Non', 'Existent', 'Course'))
def test_malformed_grading_policy(self): def test_malformed_grading_policy(self):
""" """
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment