diff --git a/cms/envs/aws.py b/cms/envs/aws.py index a24e25b74e..a781576455 100644 --- a/cms/envs/aws.py +++ b/cms/envs/aws.py @@ -122,6 +122,13 @@ SITE_NAME = ENV_TOKENS['SITE_NAME'] LOG_DIR = ENV_TOKENS['LOG_DIR'] CACHES = ENV_TOKENS['CACHES'] +# Cache used for location mapping -- called many times with the same key/value +# in a given request. +if 'loc_cache' not in CACHES: + CACHES['loc_cache'] = { + 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', + 'LOCATION': 'edx_location_mem_cache', + } SESSION_COOKIE_DOMAIN = ENV_TOKENS.get('SESSION_COOKIE_DOMAIN') SESSION_ENGINE = ENV_TOKENS.get('SESSION_ENGINE', SESSION_ENGINE) diff --git a/common/djangoapps/student/roles.py b/common/djangoapps/student/roles.py index 13b9b38a15..7020bcd5d3 100644 --- a/common/djangoapps/student/roles.py +++ b/common/djangoapps/student/roles.py @@ -239,4 +239,4 @@ class CourseCreatorRole(GroupBasedRole): """ ROLE = "course_creator_group" def __init__(self, *args, **kwargs): - super(CourseCreatorRole, self).__init__(self.ROLE, *args, **kwargs) + super(CourseCreatorRole, self).__init__([self.ROLE], *args, **kwargs) diff --git a/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py b/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py index d05810b473..9d5c29e6ae 100644 --- a/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py +++ b/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py @@ -256,7 +256,10 @@ class LocMapperStore(object): :param course_id: old style course id """ - # doesn't use caching as cache requires full location w/o wildcards + cached = self._get_course_locator_from_cache(old_style_course_id, published) + if cached: + return cached + location_id = self._interpret_location_course_id(old_style_course_id, location) if old_style_course_id is None: old_style_course_id = self._generate_location_course_id(location_id) @@ -274,11 +277,13 @@ class LocMapperStore(object): if 'name' not in item['_id']: entry = item break + published_course_locator = CourseLocator(package_id=entry['course_id'], branch=entry['prod_branch']) + draft_course_locator = CourseLocator(package_id=entry['course_id'], branch=entry['draft_branch']) + self._cache_course_locator(old_style_course_id, published_course_locator, draft_course_locator) if published: - branch = entry['prod_branch'] + return published_course_locator else: - branch = entry['draft_branch'] - return CourseLocator(package_id=entry['course_id'], branch=branch) + return draft_course_locator def _add_to_block_map(self, location, location_id, block_map): '''add the given location to the block_map and persist it''' @@ -392,6 +397,17 @@ class LocMapperStore(object): return entry[1] return None + def _get_course_locator_from_cache(self, old_course_id, published): + """ + Get the course Locator for this old course id + """ + entry = self.cache.get(old_course_id) + if entry is not None: + if published: + return entry[0].as_course_locator() + else: + return entry[1].as_course_locator() + def _get_location_from_cache(self, locator): """ See if the locator is in the cache. If so, return the mapped location. @@ -405,6 +421,12 @@ class LocMapperStore(object): """ return self.cache.get('courseId+{}'.format(locator_package_id)) + def _cache_course_locator(self, old_course_id, published_course_locator, draft_course_locator): + """ + For quick lookup of courses + """ + self.cache.set(old_course_id, (published_course_locator, draft_course_locator)) + def _cache_location_map_entry(self, old_course_id, location, published_usage, draft_usage): """ Cache the mapping from location to the draft and published Locators in entry. @@ -417,4 +439,5 @@ class LocMapperStore(object): setmany[unicode(published_usage)] = location setmany[unicode(draft_usage)] = location setmany['{}+{}'.format(old_course_id, location.url())] = (published_usage, draft_usage) + setmany[old_course_id] = (published_usage, draft_usage) self.cache.set_many(setmany) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py b/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py index 73fa287b4c..90da230e76 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py @@ -397,14 +397,20 @@ class TrivialCache(object): def __init__(self): self.cache = {} - def get(self, key): + def get(self, key, default=None): """ Mock the .get """ - return self.cache.get(key) + return self.cache.get(key, default) def set_many(self, entries): """ mock set_many """ self.cache.update(entries) + + def set(self, key, entry): + """ + mock set + """ + self.cache[key] = entry diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py index c444aa32e1..98c64dcac5 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py @@ -20,6 +20,7 @@ from xmodule.modulestore.split_mongo.split import SplitMongoModuleStore from xmodule.modulestore.mongo.base import MongoModuleStore from xmodule.modulestore.split_migrator import SplitMigrator from xmodule.modulestore.mongo import draft +from xmodule.modulestore.tests import test_location_mapper class TestMigration(unittest.TestCase): @@ -43,10 +44,8 @@ class TestMigration(unittest.TestCase): def setUp(self): super(TestMigration, self).setUp() - noop_cache = mock.Mock(spec=['get', 'set_many']) - noop_cache.configure_mock(**{'get.return_value': None}) # pylint: disable=W0142 - self.loc_mapper = LocMapperStore(noop_cache, **self.db_config) + self.loc_mapper = LocMapperStore(test_location_mapper.TrivialCache(), **self.db_config) self.old_mongo = MongoModuleStore(self.db_config, **self.modulestore_options) self.draft_mongo = DraftModuleStore(self.db_config, **self.modulestore_options) self.split_mongo = SplitMongoModuleStore( diff --git a/lms/envs/aws.py b/lms/envs/aws.py index 3e43a35c29..0f1873e688 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -155,6 +155,13 @@ MEDIA_URL = ENV_TOKENS['MEDIA_URL'] LOG_DIR = ENV_TOKENS['LOG_DIR'] CACHES = ENV_TOKENS['CACHES'] +# Cache used for location mapping -- called many times with the same key/value +# in a given request. +if 'loc_cache' not in CACHES: + CACHES['loc_cache'] = { + 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', + 'LOCATION': 'edx_location_mem_cache', + } # Email overrides DEFAULT_FROM_EMAIL = ENV_TOKENS.get('DEFAULT_FROM_EMAIL', DEFAULT_FROM_EMAIL)