From 6f11b98b4fb823874fbafd42ba96debfacd55c9b Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Mon, 22 Jul 2013 10:26:12 -0400 Subject: [PATCH 01/27] initial commit for a mixed module store which can interoperate with both XML and Mongo module stores --- .../xmodule/xmodule/modulestore/__init__.py | 2 +- .../lib/xmodule/xmodule/modulestore/django.py | 41 ++++--- .../lib/xmodule/xmodule/modulestore/mixed.py | 106 ++++++++++++++++++ .../xmodule/xmodule/modulestore/mongo/base.py | 2 +- .../lib/xmodule/xmodule/modulestore/search.py | 2 +- .../xmodule/modulestore/split_mongo/split.py | 9 +- .../xmodule/modulestore/store_utilities.py | 6 +- .../tests/test_split_modulestore.py | 44 ++++---- common/lib/xmodule/xmodule/modulestore/xml.py | 4 +- lms/envs/cms/mixed_dev.py | 37 ++++++ 10 files changed, 206 insertions(+), 47 deletions(-) create mode 100644 common/lib/xmodule/xmodule/modulestore/mixed.py create mode 100644 lms/envs/cms/mixed_dev.py diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index d616f21efa..17741225e5 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -258,7 +258,7 @@ class ModuleStore(object): An abstract interface for a database backend that stores XModuleDescriptor instances """ - def has_item(self, location): + def has_item(self, course_id, location): """ Returns True if location exists in this ModuleStore. """ diff --git a/common/lib/xmodule/xmodule/modulestore/django.py b/common/lib/xmodule/xmodule/modulestore/django.py index c98e6cadef..7b1ce37d07 100644 --- a/common/lib/xmodule/xmodule/modulestore/django.py +++ b/common/lib/xmodule/xmodule/modulestore/django.py @@ -25,24 +25,31 @@ def load_function(path): return getattr(import_module(module_path), name) +def create_modulestore_instance(engine, options): + """ + This will return a new instance of a modulestore given an engine and options + """ + class_ = load_function(engine) + + _options = {} + _options.update(options) + + for key in FUNCTION_KEYS: + if key in _options: + _options[key] = load_function(_options[key]) + + return class_( + **_options + ) + + def modulestore(name='default'): + """ + This returns an instance of a modulestore of given name. This will wither return an existing + modulestore or create a new one + """ if name not in _MODULESTORES: - class_ = load_function(settings.MODULESTORE[name]['ENGINE']) - - options = {} - - options.update(settings.MODULESTORE[name]['OPTIONS']) - for key in FUNCTION_KEYS: - if key in options: - options[key] = load_function(options[key]) - - _MODULESTORES[name] = class_( - **options - ) + _MODULESTORES[name] = create_modulestore_instance(settings.MODULESTORE[name]['ENGINE'], + settings.MODULESTORE[name]['OPTIONS']) return _MODULESTORES[name] - -# if 'DJANGO_SETTINGS_MODULE' in environ: -# # Initialize the modulestores immediately -# for store_name in settings.MODULESTORE: -# modulestore(store_name) diff --git a/common/lib/xmodule/xmodule/modulestore/mixed.py b/common/lib/xmodule/xmodule/modulestore/mixed.py new file mode 100644 index 0000000000..1ecb12f858 --- /dev/null +++ b/common/lib/xmodule/xmodule/modulestore/mixed.py @@ -0,0 +1,106 @@ +""" +MixedModuleStore allows for aggregation between multiple modulestores. + +In this way, courses can be served up both - say - XMLModuleStore or MongoModuleStore + +IMPORTANT: This modulestore is experimental AND INCOMPLETE. Therefore this should only be used cautiously +""" + +from . import ModuleStoreBase +from django import create_modulestore_instance + + +class MixedModuleStore(ModuleStoreBase): + """ + ModuleStore that can be backed by either XML or Mongo + """ + def __init__(self, mappings, stores): + """ + Initialize a MixedModuleStore. Here we look into our passed in kwargs which should be a + collection of other modulestore configuration informations + """ + super(MixedModuleStore, self).__init__() + + self.modulestores = {} + self.mappings = mappings + for key in stores: + self.modulestores[key] = create_modulestore_instance(stores[key]['ENGINE'], + stores[key]['OPTIONS']) + + def _get_modulestore_for_courseid(self, course_id): + """ + For a given course_id, look in the mapping table and see if it has been pinned + to a particular modulestore + """ + return self.mappings.get(course_id, self.mappings['default']) + + def has_item(self, course_id, location): + return self._get_modulestore_for_courseid(course_id).has_item(course_id, location) + + def get_item(self, location, depth=0): + """ + This method is explicitly not implemented as we need a course_id to disambiguate + We should be able to fix this when the data-model rearchitecting is done + """ + raise NotImplementedError + + def get_instance(self, course_id, location, depth=0): + return self._get_modulestore_for_courseid(course_id).get_instance(course_id, location, depth) + + def get_items(self, location, course_id=None, depth=0): + """ + Returns a list of XModuleDescriptor instances for the items + that match location. Any element of location that is None is treated + as a wildcard that matches any value + + location: Something that can be passed to Location + + depth: An argument that some module stores may use to prefetch + descendents of the queried modules for more efficient results later + in the request. The depth is counted in the number of calls to + get_children() to cache. None indicates to cache all descendents + """ + if not course_id: + raise Exception("Must pass in a course_id when calling get_items() with MixedModuleStore") + + return self._get_modulestore_for_courseid(course_id).get_items(location, course_id, depth) + + def update_item(self, location, data, allow_not_found=False): + """ + MixedModuleStore is for read-only (aka LMS) + """ + raise NotImplementedError + + def update_children(self, location, children): + """ + MixedModuleStore is for read-only (aka LMS) + """ + raise NotImplementedError + + def update_metadata(self, location, metadata): + """ + MixedModuleStore is for read-only (aka LMS) + """ + raise NotImplementedError + + def delete_item(self, location): + """ + MixedModuleStore is for read-only (aka LMS) + """ + raise NotImplementedError + + def get_courses(self): + ''' + Returns a list containing the top level XModuleDescriptors of the courses + in this modulestore. + ''' + courses = [] + for key in self.modulestores: + courses.append(self.modulestores[key].get_courses) + return courses + + def get_course(self, course_id): + return self._get_modulestore_for_courseid(course_id).get_course(course_id) + + def get_parent_locations(self, location, course_id): + return self._get_modulestore_for_courseid(course_id).get_parent_locations(location, course_id) diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index 21daff1875..8b4ce23ba7 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -547,7 +547,7 @@ class MongoModuleStore(ModuleStoreBase): raise ItemNotFoundError(location) return item - def has_item(self, location): + def has_item(self, course_id, location): """ Returns True if location exists in this ModuleStore. """ diff --git a/common/lib/xmodule/xmodule/modulestore/search.py b/common/lib/xmodule/xmodule/modulestore/search.py index 25ebc7e89c..804cdb0194 100644 --- a/common/lib/xmodule/xmodule/modulestore/search.py +++ b/common/lib/xmodule/xmodule/modulestore/search.py @@ -81,7 +81,7 @@ def path_to_location(modulestore, course_id, location): # If we're here, there is no path return None - if not modulestore.has_item(location): + if not modulestore.has_item(course_id, location): raise ItemNotFoundError path = find_path_to_course() diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index 74c7e7241a..52f5539bae 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -275,7 +275,14 @@ class SplitMongoModuleStore(ModuleStoreBase): result = self._load_items(course_entry, [root], 0, lazy=True) return result[0] - def has_item(self, block_location): + def get_course_for_item(self, location): + ''' + Provided for backward compatibility. Is equivalent to calling get_course + :param location: + ''' + return self.get_course(location) + + def has_item(self, course_id, block_location): """ Returns True if location exists in its course. Returns false if the course or the block w/in the course do not exist for the given version. diff --git a/common/lib/xmodule/xmodule/modulestore/store_utilities.py b/common/lib/xmodule/xmodule/modulestore/store_utilities.py index e0f3db6810..19d1cac988 100644 --- a/common/lib/xmodule/xmodule/modulestore/store_utilities.py +++ b/common/lib/xmodule/xmodule/modulestore/store_utilities.py @@ -152,7 +152,7 @@ def clone_course(modulestore, contentstore, source_location, dest_location, dele # check to see if the dest_location exists as an empty course # we need an empty course because the app layers manage the permissions and users - if not modulestore.has_item(dest_location): + if not modulestore.has_item(dest_location.course_id, dest_location): raise Exception("An empty course at {0} must have already been created. Aborting...".format(dest_location)) # verify that the dest_location really is an empty course, which means only one with an optional 'overview' @@ -171,7 +171,7 @@ def clone_course(modulestore, contentstore, source_location, dest_location, dele raise Exception("Course at destination {0} is not an empty course. You can only clone into an empty course. Aborting...".format(dest_location)) # check to see if the source course is actually there - if not modulestore.has_item(source_location): + if not modulestore.has_item(source_location.course_id, source_location): raise Exception("Cannot find a course at {0}. Aborting".format(source_location)) # Get all modules under this namespace which is (tag, org, course) tuple @@ -250,7 +250,7 @@ def delete_course(modulestore, contentstore, source_location, commit=False): """ # check to see if the source course is actually there - if not modulestore.has_item(source_location): + if not modulestore.has_item(source_location.course_id, source_location): raise Exception("Cannot find a course at {0}. Aborting".format(source_location)) # first delete all of the thumbnails diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py index 9976a33a00..ca5eb72a26 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py @@ -257,18 +257,19 @@ class SplitModuleItemTests(SplitModuleTest): ''' has_item(BlockUsageLocator) ''' + course_id = 'GreekHero' # positive tests of various forms locator = BlockUsageLocator(version_guid=self.GUID_D1, usage_id='head12345') - self.assertTrue(modulestore().has_item(locator), + self.assertTrue(modulestore().has_item(course_id, locator), "couldn't find in %s" % self.GUID_D1) locator = BlockUsageLocator(course_id='GreekHero', usage_id='head12345', branch='draft') self.assertTrue( - modulestore().has_item(locator), + modulestore().has_item(course_id, locator), "couldn't find in 12345" ) self.assertTrue( - modulestore().has_item(BlockUsageLocator( + modulestore().has_item(course_id, BlockUsageLocator( course_id=locator.course_id, branch='draft', usage_id=locator.usage_id @@ -276,7 +277,7 @@ class SplitModuleItemTests(SplitModuleTest): "couldn't find in draft 12345" ) self.assertFalse( - modulestore().has_item(BlockUsageLocator( + modulestore().has_item(course_id, BlockUsageLocator( course_id=locator.course_id, branch='published', usage_id=locator.usage_id)), @@ -284,40 +285,41 @@ class SplitModuleItemTests(SplitModuleTest): ) locator.branch = 'draft' self.assertTrue( - modulestore().has_item(locator), + modulestore().has_item(course_id, locator), "not found in draft 12345" ) # not a course obj locator = BlockUsageLocator(course_id='GreekHero', usage_id='chapter1', branch='draft') self.assertTrue( - modulestore().has_item(locator), + modulestore().has_item(course_id, locator), "couldn't find chapter1" ) # in published course - locator = BlockUsageLocator(course_id="wonderful", usage_id="head23456", branch='draft') - self.assertTrue(modulestore().has_item(BlockUsageLocator(course_id=locator.course_id, - usage_id=locator.usage_id, - branch='published')), + locator = BlockUsageLocator(course_id="wonderful", usage_id="head23456", revision='draft') + self.assertTrue(modulestore().has_item(course_id, BlockUsageLocator(course_id=locator.course_id, + usage_id=locator.usage_id, + revision='published')), "couldn't find in 23456") locator.branch = 'published' - self.assertTrue(modulestore().has_item(locator), "couldn't find in 23456") + self.assertTrue(modulestore().has_item(course_id, locator), "couldn't find in 23456") def test_negative_has_item(self): # negative tests--not found # no such course or block + course_id = 'GreekHero' locator = BlockUsageLocator(course_id="doesnotexist", usage_id="head23456", branch='draft') - self.assertFalse(modulestore().has_item(locator)) + self.assertFalse(modulestore().has_item(course_id, locator)) locator = BlockUsageLocator(course_id="wonderful", usage_id="doesnotexist", branch='draft') - self.assertFalse(modulestore().has_item(locator)) + self.assertFalse(modulestore().has_item(course_id, locator)) # negative tests--insufficient specification self.assertRaises(InsufficientSpecificationError, BlockUsageLocator) self.assertRaises(InsufficientSpecificationError, - modulestore().has_item, BlockUsageLocator(version_guid=self.GUID_D1)) + modulestore().has_item, None, BlockUsageLocator(version_guid=self.GUID_D1)) self.assertRaises(InsufficientSpecificationError, - modulestore().has_item, BlockUsageLocator(course_id='GreekHero')) + modulestore().has_item, None, BlockUsageLocator(course_id='GreekHero')) def test_get_item(self): ''' @@ -737,13 +739,13 @@ class TestItemCrud(SplitModuleTest): deleted = BlockUsageLocator(course_id=reusable_location.course_id, branch=reusable_location.branch, usage_id=locn_to_del.usage_id) - self.assertFalse(modulestore().has_item(deleted)) - self.assertRaises(VersionConflictError, modulestore().has_item, locn_to_del) + self.assertFalse(modulestore().has_item(reusable_location.course_id, deleted)) + self.assertRaises(VersionConflictError, modulestore().has_item, reusable_location.course_id, locn_to_del) locator = BlockUsageLocator( version_guid=locn_to_del.version_guid, usage_id=locn_to_del.usage_id ) - self.assertTrue(modulestore().has_item(locator)) + self.assertTrue(modulestore().has_item(reusable_location.course_id, locator)) self.assertNotEqual(new_course_loc.version_guid, course.location.version_guid) # delete a subtree @@ -754,7 +756,7 @@ class TestItemCrud(SplitModuleTest): def check_subtree(node): if node: node_loc = node.location - self.assertFalse(modulestore().has_item( + self.assertFalse(modulestore().has_item(reusable_location.course_id, BlockUsageLocator( course_id=node_loc.course_id, branch=node_loc.branch, @@ -762,7 +764,7 @@ class TestItemCrud(SplitModuleTest): locator = BlockUsageLocator( version_guid=node.location.version_guid, usage_id=node.location.usage_id) - self.assertTrue(modulestore().has_item(locator)) + self.assertTrue(modulestore().has_item(reusable_location.course_id, locator)) if node.has_children: for sub in node.get_children(): check_subtree(sub) @@ -873,7 +875,7 @@ class TestCourseCreation(SplitModuleTest): original_course = modulestore().get_course(original_locator) self.assertEqual(original_course.location.version_guid, original_index['versions']['draft']) self.assertFalse( - modulestore().has_item(BlockUsageLocator( + modulestore().has_item(new_draft_locator.course_id, BlockUsageLocator( original_locator, usage_id=new_item.location.usage_id )) diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 8bc3142c77..ae726041ab 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -505,12 +505,12 @@ class XMLModuleStore(ModuleStoreBase): except KeyError: raise ItemNotFoundError(location) - def has_item(self, location): + def has_item(self, course_id, location): """ Returns True if location exists in this ModuleStore. """ location = Location(location) - return any(location in course_modules for course_modules in self.modules.values()) + return location in self.modules[course_id] def get_item(self, location, depth=0): """ diff --git a/lms/envs/cms/mixed_dev.py b/lms/envs/cms/mixed_dev.py new file mode 100644 index 0000000000..22fd5eeb0e --- /dev/null +++ b/lms/envs/cms/mixed_dev.py @@ -0,0 +1,37 @@ +""" +This configuration is to run the MixedModuleStore on a localdev environment +""" + +from .dev import * + +MODULESTORE = { + 'default': { + 'ENGINE': 'xmodule.modulestore.mixed.MixedModuleStore', + 'OPTIONS': { + 'mappings': { + '6.002/a/a': 'xml', + '6.002/b/b': 'xml' + }, + 'stores': { + 'xml': { + 'ENGINE': 'xmodule.modulestore.xml.XMLModuleStore', + 'OPTIONS': { + 'data_dir': DATA_DIR, + 'default_class': 'xmodule.hidden_module.HiddenDescriptor', + } + }, + 'default': { + 'ENGINE': 'xmodule.modulestore.mongo.MongoModuleStore', + 'OPTIONS': { + 'default_class': 'xmodule.raw_module.RawDescriptor', + 'host': 'localhost', + 'db': 'xmodule', + 'collection': 'modulestore', + 'fs_root': DATA_DIR, + 'render_template': 'mitxmako.shortcuts.render_to_string', + } + } + }, + } + } +} From b5253b52b693f231d534600ad6eb3681e89f5614 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Tue, 23 Jul 2013 21:25:18 -0400 Subject: [PATCH 02/27] add a set_modulestore_configuration to the modulestore interface. This can be used to pass in settings run the Django tier --- cms/one_time_startup.py | 7 +++++-- .../lib/xmodule/xmodule/modulestore/__init__.py | 16 ++++++++++++++++ common/lib/xmodule/xmodule/modulestore/mixed.py | 14 ++++++++++++++ lms/one_time_startup.py | 6 ++++-- 4 files changed, 39 insertions(+), 4 deletions(-) diff --git a/cms/one_time_startup.py b/cms/one_time_startup.py index cbd8775d97..4198cf2637 100644 --- a/cms/one_time_startup.py +++ b/cms/one_time_startup.py @@ -9,8 +9,11 @@ from django.core.cache import get_cache CACHE = get_cache('mongo_metadata_inheritance') for store_name in settings.MODULESTORE: store = modulestore(store_name) - store.metadata_inheritance_cache_subsystem = CACHE - store.request_cache = RequestCache.get_request_cache() + + store.set_modulestore_configuration({ + 'metadata_inheritance_cache_subsystem': CACHE, + 'request_cache': RequestCache.get_request_cache() + }) modulestore_update_signal = Signal(providing_args=['modulestore', 'course_id', 'location']) store.modulestore_update_signal = modulestore_update_signal diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index 17741225e5..a9848d6c05 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -384,6 +384,13 @@ class ModuleStore(object): """ raise NotImplementedError + def set_modulestore_configuration(self, config_dict): + ''' + Allows for runtime configuration of the modulestore. In particular this is how the + application (LMS/CMS) can pass down Django related configuration information, e.g. caches, etc. + ''' + raise NotImplementedError + class ModuleStoreBase(ModuleStore): ''' @@ -395,6 +402,7 @@ class ModuleStoreBase(ModuleStore): ''' self._location_errors = {} # location -> ErrorLog self.metadata_inheritance_cache = None + self.request_cache = None self.modulestore_update_signal = None # can be set by runtime to route notifications of datastore changes def _get_errorlog(self, location): @@ -439,6 +447,14 @@ class ModuleStoreBase(ModuleStore): return c return None + def set_modulestore_configuration(self, config_dict): + """ + This is the base implementation of the interface, all we need to do is store + two possible configurations as attributes on the class + """ + self.metadata_inheritance_cache = config_dict.get('metadata_inheritance_cache_subsystem', None) + self.request_cache = config_dict.get('request_cache', None) + def namedtuple_to_son(namedtuple, prefix=''): """ diff --git a/common/lib/xmodule/xmodule/modulestore/mixed.py b/common/lib/xmodule/xmodule/modulestore/mixed.py index 1ecb12f858..fe4d4d63be 100644 --- a/common/lib/xmodule/xmodule/modulestore/mixed.py +++ b/common/lib/xmodule/xmodule/modulestore/mixed.py @@ -100,7 +100,21 @@ class MixedModuleStore(ModuleStoreBase): return courses def get_course(self, course_id): + """ + returns the course module associated with the course_id + """ return self._get_modulestore_for_courseid(course_id).get_course(course_id) def get_parent_locations(self, location, course_id): + """ + returns the parent locations for a given lcoation and course_id + """ return self._get_modulestore_for_courseid(course_id).get_parent_locations(location, course_id) + + def set_modulestore_configuration(self, config_dict): + """ + This implementation of the interface method will pass along the configuration to all ModuleStore + instances + """ + for store in self.modulestores.values(): + store.set_modulestore_configuration(config_dict) diff --git a/lms/one_time_startup.py b/lms/one_time_startup.py index e10ec06685..2cd2077c4e 100644 --- a/lms/one_time_startup.py +++ b/lms/one_time_startup.py @@ -8,8 +8,10 @@ from django.core.cache import get_cache cache = get_cache('mongo_metadata_inheritance') for store_name in settings.MODULESTORE: store = modulestore(store_name) - store.metadata_inheritance_cache_subsystem = cache - store.request_cache = RequestCache.get_request_cache() + store.set_modulestore_configuration({ + 'metadata_inheritance_cache_subsystem': cache, + 'request_cache': RequestCache.get_request_cache() + }) if hasattr(settings, 'DATADOG_API'): dog_http_api.api_key = settings.DATADOG_API From 82988972ade7452ca59dbeab63ce974e80afa972 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Wed, 24 Jul 2013 08:40:04 -0400 Subject: [PATCH 03/27] WIP: added a get_modulestore_type. Added some unit tests. changed over the isinstance() with respect to modulestores to use this get_modulestore_type() --- common/djangoapps/static_replace/__init__.py | 10 +++++----- .../lib/xmodule/xmodule/modulestore/__init__.py | 9 +++++++++ common/lib/xmodule/xmodule/modulestore/mixed.py | 7 +++++++ .../lib/xmodule/xmodule/modulestore/mongo/base.py | 15 +++++++++++---- .../xmodule/modulestore/tests/test_mongo.py | 8 ++++++-- .../xmodule/xmodule/modulestore/tests/test_xml.py | 11 ++++++++--- common/lib/xmodule/xmodule/modulestore/xml.py | 9 ++++++++- common/lib/xmodule/xmodule/video_module.py | 7 +------ lms/djangoapps/courseware/courses.py | 4 ++-- 9 files changed, 57 insertions(+), 23 deletions(-) diff --git a/common/djangoapps/static_replace/__init__.py b/common/djangoapps/static_replace/__init__.py index 9e50d73b26..c08db34349 100644 --- a/common/djangoapps/static_replace/__init__.py +++ b/common/djangoapps/static_replace/__init__.py @@ -6,7 +6,7 @@ from staticfiles import finders from django.conf import settings from xmodule.modulestore.django import modulestore -from xmodule.modulestore.xml import XMLModuleStore +from xmodule.modulestore import XML_MODULESTORE_TYPE from xmodule.contentstore.content import StaticContent log = logging.getLogger(__name__) @@ -90,7 +90,7 @@ def replace_course_urls(text, course_id): return re.sub(_url_replace_regex('/course/'), replace_course_url, text) -def replace_static_urls(text, data_directory, course_namespace=None): +def replace_static_urls(text, data_directory, course_id=None): """ Replace /static/$stuff urls either with their correct url as generated by collectstatic, (/static/$md5_hashed_stuff) or by the course-specific content static url @@ -99,7 +99,7 @@ def replace_static_urls(text, data_directory, course_namespace=None): text: The source text to do the substitution in data_directory: The directory in which course data is stored - course_namespace: The course identifier used to distinguish static content for this course in studio + course_id: The course identifier used to distinguish static content for this course in studio """ def replace_static_url(match): @@ -116,7 +116,7 @@ def replace_static_urls(text, data_directory, course_namespace=None): if settings.DEBUG and finders.find(rest, True): return original # if we're running with a MongoBacked store course_namespace is not None, then use studio style urls - elif course_namespace is not None and not isinstance(modulestore(), XMLModuleStore): + elif course_id and modulestore().get_modulestore_type(course_id) != XML_MODULESTORE_TYPE: # first look in the static file pipeline and see if we are trying to reference # a piece of static content which is in the mitx repo (e.g. JS associated with an xmodule) if staticfiles_storage.exists(rest): @@ -124,7 +124,7 @@ def replace_static_urls(text, data_directory, course_namespace=None): else: # if not, then assume it's courseware specific content and then look in the # Mongo-backed database - url = StaticContent.convert_legacy_static_url(rest, course_namespace) + url = StaticContent.convert_legacy_static_url(rest, course_id) # Otherwise, look the file up in staticfiles_storage, and append the data directory if needed else: course_path = "/".join((data_directory, rest)) diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index a9848d6c05..ee61998931 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -14,6 +14,8 @@ from bson.son import SON log = logging.getLogger('mitx.' + 'modulestore') +MONGO_MODULESTORE_TYPE = 'mongo' +XML_MODULESTORE_TYPE = 'xml' URL_RE = re.compile(""" (?P[^:]+)://? @@ -391,6 +393,13 @@ class ModuleStore(object): ''' raise NotImplementedError + def get_modulestore_type(self, course_id): + """ + Returns a type which identifies which modulestore is servicing the given + course_id. The return can be either "xml" (for XML based courses) or "mongo" for MongoDB backed courses + """ + raise NotImplementedError + class ModuleStoreBase(ModuleStore): ''' diff --git a/common/lib/xmodule/xmodule/modulestore/mixed.py b/common/lib/xmodule/xmodule/modulestore/mixed.py index fe4d4d63be..253852d1db 100644 --- a/common/lib/xmodule/xmodule/modulestore/mixed.py +++ b/common/lib/xmodule/xmodule/modulestore/mixed.py @@ -118,3 +118,10 @@ class MixedModuleStore(ModuleStoreBase): """ for store in self.modulestores.values(): store.set_modulestore_configuration(config_dict) + + def get_modulestore_type(self, course_id): + """ + Returns a type which identifies which modulestore is servicing the given + course_id. The return can be either "xml" (for XML based courses) or "mongo" for MongoDB backed courses + """ + return self._get_modulestore_for_courseid(course_id).get_modulestore_type(course_id) diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index 8b4ce23ba7..de946f76e1 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -32,7 +32,7 @@ from xmodule.error_module import ErrorDescriptor from xblock.runtime import DbModel, KeyValueStore, InvalidScopeError from xblock.core import Scope -from xmodule.modulestore import ModuleStoreBase, Location, namedtuple_to_son +from xmodule.modulestore import ModuleStoreBase, Location, namedtuple_to_son, MONGO_MODULESTORE_TYPE from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore.inheritance import own_metadata, INHERITABLE_METADATA, inherit_metadata @@ -841,6 +841,13 @@ class MongoModuleStore(ModuleStoreBase): {'_id': True}) return [i['_id'] for i in items] + def get_modulestore_type(self, course_id): + """ + Returns a type which identifies which modulestore is servicing the given + course_id. The return can be either "xml" (for XML based courses) or "mongo" for MongoDB backed courses + """ + return MONGO_MODULESTORE_TYPE + def _create_new_model_data(self, category, location, definition_data, metadata): """ To instantiate a new xmodule which will be saved latter, set up the dbModel and kvs @@ -854,9 +861,9 @@ class MongoModuleStore(ModuleStoreBase): ) class_ = XModuleDescriptor.load_class( - category, - self.default_class - ) + category, + self.default_class + ) model_data = DbModel(kvs, class_, None, MongoUsage(None, location)) model_data['category'] = category model_data['location'] = location diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py index 69ba9ad94b..58c48f673e 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py @@ -45,8 +45,7 @@ class TestMongoModuleStore(object): @staticmethod def initdb(): # connect to the db - store = MongoModuleStore(HOST, DB, COLLECTION, FS_ROOT, RENDER_TEMPLATE, - default_class=DEFAULT_CLASS) + store = MongoModuleStore(HOST, DB, COLLECTION, FS_ROOT, RENDER_TEMPLATE, default_class=DEFAULT_CLASS) # Explicitly list the courses to load (don't want the big one) courses = ['toy', 'simple'] import_from_xml(store, DATA_DIR, courses) @@ -71,6 +70,10 @@ class TestMongoModuleStore(object): pprint([Location(i['_id']).url() for i in ids]) + def test_mongo_modulestore_type(self): + store = MongoModuleStore(HOST, DB, COLLECTION, FS_ROOT, RENDER_TEMPLATE, default_class=DEFAULT_CLASS) + assert_equals(store.get_modulestore_type('foo/bar/baz'), 'mongo') + def test_get_courses(self): '''Make sure the course objects loaded properly''' courses = self.store.get_courses() @@ -117,6 +120,7 @@ class TestMongoModuleStore(object): '{0} is a template course'.format(course) ) + class TestMongoKeyValueStore(object): def setUp(self): diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py b/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py index 1819850614..676d928bb7 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py @@ -1,12 +1,13 @@ import os.path -from nose.tools import assert_raises +from nose.tools import assert_raises, assert_equals from xmodule.course_module import CourseDescriptor from xmodule.modulestore.xml import XMLModuleStore +from xmodule.modulestore import XML_MODULESTORE_TYPE -from xmodule.tests import DATA_DIR -from xmodule.modulestore.tests.test_modulestore import check_path_to_location +from .test_modulestore import check_path_to_location +from . import DATA_DIR class TestXMLModuleStore(object): @@ -19,6 +20,10 @@ class TestXMLModuleStore(object): check_path_to_location(modulestore) + def test_xml_modulestore_type(self): + store = XMLModuleStore(DATA_DIR, course_dirs=['toy', 'simple']) + assert_equals(store.get_modulestore_type('foo/bar/baz'), XML_MODULESTORE_TYPE) + def test_unicode_chars_in_xml_content(self): # edX/full/6.002_Spring_2012 has non-ASCII chars, and during # uniquification of names, would raise a UnicodeError. It no longer does. diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index ae726041ab..fd04982f81 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -21,7 +21,7 @@ from xmodule.x_module import XModuleDescriptor, XMLParsingSystem from xmodule.html_module import HtmlDescriptor -from . import ModuleStoreBase, Location +from . import ModuleStoreBase, Location, XML_MODULESTORE_TYPE from .exceptions import ItemNotFoundError from .inheritance import compute_inherited_metadata @@ -601,3 +601,10 @@ class XMLModuleStore(ModuleStoreBase): raise ItemNotFoundError("{0} not in {1}".format(location, course_id)) return self.parent_trackers[course_id].parents(location) + + def get_modulestore_type(self, course_id): + """ + Returns a type which identifies which modulestore is servicing the given + course_id. The return can be either "xml" (for XML based courses) or "mongo" for MongoDB backed courses + """ + return XML_MODULESTORE_TYPE diff --git a/common/lib/xmodule/xmodule/video_module.py b/common/lib/xmodule/xmodule/video_module.py index dd408a6f74..e847638af1 100644 --- a/common/lib/xmodule/xmodule/video_module.py +++ b/common/lib/xmodule/xmodule/video_module.py @@ -161,12 +161,7 @@ class VideoModule(VideoFields, XModule): return json.dumps({'position': self.position}) def get_html(self): - if isinstance(modulestore(), MongoModuleStore): - caption_asset_path = StaticContent.get_base_url_path_for_course_assets(self.location) + '/subs_' - else: - # VS[compat] - # cdodge: filesystem static content support. - caption_asset_path = "/static/subs/" + caption_asset_path = "/static/subs/" get_ext = lambda filename: filename.rpartition('.')[-1] sources = {get_ext(src): src for src in self.html5_sources} diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 086f92a123..34de2eb958 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -8,7 +8,7 @@ from django.http import Http404 from .module_render import get_module from xmodule.course_module import CourseDescriptor -from xmodule.modulestore import Location +from xmodule.modulestore import Location, MONGO_MODULESTORE_TYPE from xmodule.modulestore.django import modulestore from xmodule.contentstore.content import StaticContent from xmodule.modulestore.xml import XMLModuleStore @@ -82,7 +82,7 @@ def get_opt_course_with_access(user, course_id, action): def course_image_url(course): """Try to look up the image url for the course. If it's not found, log an error and return the dead link""" - if isinstance(modulestore(), XMLModuleStore): + if modulestore().get_modulestore_type(course.course_id) == MONGO_MODULESTORE_TYPE: return '/static/' + course.data_dir + "/images/course_image.jpg" else: loc = course.location._replace(tag='c4x', category='asset', name='images_course_image.jpg') From 2616d8f7a2819e68f87932db4de357227b939faf Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Wed, 24 Jul 2013 08:52:11 -0400 Subject: [PATCH 04/27] remove unneeded type check on the modulestore --- common/lib/xmodule/xmodule/modulestore/store_utilities.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/store_utilities.py b/common/lib/xmodule/xmodule/modulestore/store_utilities.py index 19d1cac988..725d4aebb7 100644 --- a/common/lib/xmodule/xmodule/modulestore/store_utilities.py +++ b/common/lib/xmodule/xmodule/modulestore/store_utilities.py @@ -146,10 +146,6 @@ def _clone_modules(modulestore, modules, source_location, dest_location): def clone_course(modulestore, contentstore, source_location, dest_location, delete_original=False): - # first check to see if the modulestore is Mongo backed - if not isinstance(modulestore, MongoModuleStore): - raise Exception("Expected a MongoModuleStore in the runtime. Aborting....") - # check to see if the dest_location exists as an empty course # we need an empty course because the app layers manage the permissions and users if not modulestore.has_item(dest_location.course_id, dest_location): From cff93d324bc932cd5f6ee3448f6dcfef29d11251 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 25 Jul 2013 00:50:42 -0400 Subject: [PATCH 05/27] WIP --- cms/djangoapps/contentstore/views/preview.py | 4 ++-- common/djangoapps/contentserver/middleware.py | 5 +++++ common/djangoapps/static_replace/__init__.py | 2 +- common/djangoapps/student/views.py | 1 + common/djangoapps/xmodule_modifiers.py | 4 ++-- .../xmodule/xmodule/contentstore/content.py | 6 ++++++ .../lib/xmodule/xmodule/modulestore/mixed.py | 21 +++++++++++++++++-- lms/djangoapps/branding/__init__.py | 5 +++-- lms/djangoapps/courseware/courses.py | 11 +++++----- lms/djangoapps/courseware/module_render.py | 8 ++++++- lms/djangoapps/staticbook/views.py | 2 +- lms/envs/cms/mixed_dev.py | 3 +-- 12 files changed, 53 insertions(+), 19 deletions(-) diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 0591ff0dc4..bbbf3d5f0a 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -116,7 +116,7 @@ def preview_module_system(request, preview_id, descriptor): get_module=partial(load_preview_module, request, preview_id), render_template=render_from_lms, debug=True, - replace_urls=partial(static_replace.replace_static_urls, data_directory=None, course_namespace=descriptor.location), + replace_urls=partial(static_replace.replace_static_urls, data_directory=None, course_id=course_id), user=request.user, xblock_model_data=preview_model_data, can_execute_unsafe_code=(lambda: can_execute_unsafe_code(course_id)), @@ -158,7 +158,7 @@ def load_preview_module(request, preview_id, descriptor): module.get_html = replace_static_urls( module.get_html, getattr(module, 'data_dir', module.location.course), - course_namespace=Location([module.location.tag, module.location.org, module.location.course, None, None]) + course_id=module.location.org+'/'+module.location.course+'/REPLACE_WITH_RUN' ) module.get_html = save_module( diff --git a/common/djangoapps/contentserver/middleware.py b/common/djangoapps/contentserver/middleware.py index d89e3fdd23..733e1c30e3 100644 --- a/common/djangoapps/contentserver/middleware.py +++ b/common/djangoapps/contentserver/middleware.py @@ -6,11 +6,14 @@ from xmodule.modulestore import InvalidLocationError from cache_toolbox.core import get_cached_content, set_cached_content from xmodule.exceptions import NotFoundError +import logging + class StaticContentServer(object): def process_request(self, request): # look to see if the request is prefixed with 'c4x' tag if request.path.startswith('/' + XASSET_LOCATION_TAG + '/'): + logging.debug('**** path = {0}'.format(request.path)) try: loc = StaticContent.get_location_from_path(request.path) except InvalidLocationError: @@ -24,8 +27,10 @@ class StaticContentServer(object): if content is None: # nope, not in cache, let's fetch from DB try: + logging.debug('!!!! loc = {0}'.format(loc)) content = contentstore().find(loc, as_stream=True) except NotFoundError: + logging.debug('**** NOT FOUND') response = HttpResponse() response.status_code = 404 return response diff --git a/common/djangoapps/static_replace/__init__.py b/common/djangoapps/static_replace/__init__.py index c08db34349..63c576cdd2 100644 --- a/common/djangoapps/static_replace/__init__.py +++ b/common/djangoapps/static_replace/__init__.py @@ -124,7 +124,7 @@ def replace_static_urls(text, data_directory, course_id=None): else: # if not, then assume it's courseware specific content and then look in the # Mongo-backed database - url = StaticContent.convert_legacy_static_url(rest, course_id) + url = StaticContent.convert_legacy_static_url_with_course_id(rest, course_id) # Otherwise, look the file up in staticfiles_storage, and append the data directory if needed else: course_path = "/".join((data_directory, rest)) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 7795a13c47..4d59b5cc66 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -95,6 +95,7 @@ def index(request, extra_context={}, user=None): courses = sort_by_announcement(courses) context = {'courses': courses} + context.update(extra_context) return render_to_response('index.html', context) diff --git a/common/djangoapps/xmodule_modifiers.py b/common/djangoapps/xmodule_modifiers.py index dd40b5139d..6b73395599 100644 --- a/common/djangoapps/xmodule_modifiers.py +++ b/common/djangoapps/xmodule_modifiers.py @@ -76,7 +76,7 @@ def replace_course_urls(get_html, course_id): return _get_html -def replace_static_urls(get_html, data_dir, course_namespace=None): +def replace_static_urls(get_html, data_dir, course_id=None): """ Updates the supplied module with a new get_html function that wraps the old get_html function and substitutes urls of the form /static/... @@ -85,7 +85,7 @@ def replace_static_urls(get_html, data_dir, course_namespace=None): @wraps(get_html) def _get_html(): - return static_replace.replace_static_urls(get_html(), data_dir, course_namespace) + return static_replace.replace_static_urls(get_html(), data_dir, course_id) return _get_html diff --git a/common/lib/xmodule/xmodule/contentstore/content.py b/common/lib/xmodule/xmodule/contentstore/content.py index eaf3f48dea..cdecfd1af5 100644 --- a/common/lib/xmodule/xmodule/contentstore/content.py +++ b/common/lib/xmodule/xmodule/contentstore/content.py @@ -100,6 +100,12 @@ class StaticContent(object): loc = StaticContent.compute_location(course_namespace.org, course_namespace.course, path) return StaticContent.get_url_path_from_location(loc) + @staticmethod + def convert_legacy_static_url_with_course_id(path, course_id): + org, course_num, run = course_id.split("/") + loc = StaticContent.compute_location(org, course_num, path) + return StaticContent.get_url_path_from_location(loc) + def stream_data(self): yield self._data diff --git a/common/lib/xmodule/xmodule/modulestore/mixed.py b/common/lib/xmodule/xmodule/modulestore/mixed.py index 253852d1db..85ba95af48 100644 --- a/common/lib/xmodule/xmodule/modulestore/mixed.py +++ b/common/lib/xmodule/xmodule/modulestore/mixed.py @@ -8,6 +8,9 @@ IMPORTANT: This modulestore is experimental AND INCOMPLETE. Therefore this shoul from . import ModuleStoreBase from django import create_modulestore_instance +import logging + +log = logging.getLogger(__name__) class MixedModuleStore(ModuleStoreBase): @@ -23,6 +26,9 @@ class MixedModuleStore(ModuleStoreBase): self.modulestores = {} self.mappings = mappings + if 'default' not in stores: + raise Exception('Missing a default modulestore in the MixedModuleStore __init__ method.') + for key in stores: self.modulestores[key] = create_modulestore_instance(stores[key]['ENGINE'], stores[key]['OPTIONS']) @@ -32,7 +38,8 @@ class MixedModuleStore(ModuleStoreBase): For a given course_id, look in the mapping table and see if it has been pinned to a particular modulestore """ - return self.mappings.get(course_id, self.mappings['default']) + mapping = self.mappings.get(course_id, 'default') + return self.modulestores[mapping] def has_item(self, course_id, location): return self._get_modulestore_for_courseid(course_id).has_item(course_id, location) @@ -96,7 +103,8 @@ class MixedModuleStore(ModuleStoreBase): ''' courses = [] for key in self.modulestores: - courses.append(self.modulestores[key].get_courses) + courses = courses + (self.modulestores[key].get_courses()) + return courses def get_course(self, course_id): @@ -125,3 +133,12 @@ class MixedModuleStore(ModuleStoreBase): course_id. The return can be either "xml" (for XML based courses) or "mongo" for MongoDB backed courses """ return self._get_modulestore_for_courseid(course_id).get_modulestore_type(course_id) + + def get_errored_courses(self): + """ + Return a dictionary of course_dir -> [(msg, exception_str)], for each + course_dir where course loading failed. + """ + errs = {} + for store in self.modulestores.values(): + errs.update(store.get_errored_courses()) diff --git a/lms/djangoapps/branding/__init__.py b/lms/djangoapps/branding/__init__.py index b2ac874020..0bdc69bd0d 100644 --- a/lms/djangoapps/branding/__init__.py +++ b/lms/djangoapps/branding/__init__.py @@ -1,4 +1,3 @@ - from xmodule.modulestore.django import modulestore from xmodule.course_module import CourseDescriptor from django.conf import settings @@ -15,7 +14,9 @@ def get_visible_courses(domain=None): """ Return the set of CourseDescriptors that should be visible in this branded instance """ - courses = [c for c in modulestore().get_courses() + _courses = modulestore().get_courses() + + courses = [c for c in _courses if isinstance(c, CourseDescriptor)] courses = sorted(courses, key=lambda course: course.number) diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 34de2eb958..5f31658cf2 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -8,10 +8,9 @@ from django.http import Http404 from .module_render import get_module from xmodule.course_module import CourseDescriptor -from xmodule.modulestore import Location, MONGO_MODULESTORE_TYPE +from xmodule.modulestore import Location, XML_MODULESTORE_TYPE from xmodule.modulestore.django import modulestore from xmodule.contentstore.content import StaticContent -from xmodule.modulestore.xml import XMLModuleStore from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError from courseware.model_data import ModelDataCache from static_replace import replace_static_urls @@ -82,12 +81,12 @@ def get_opt_course_with_access(user, course_id, action): def course_image_url(course): """Try to look up the image url for the course. If it's not found, log an error and return the dead link""" - if modulestore().get_modulestore_type(course.course_id) == MONGO_MODULESTORE_TYPE: + if modulestore().get_modulestore_type(course.location.course_id) == XML_MODULESTORE_TYPE: return '/static/' + course.data_dir + "/images/course_image.jpg" else: loc = course.location._replace(tag='c4x', category='asset', name='images_course_image.jpg') - path = StaticContent.get_url_path_from_location(loc) - return path + _path = StaticContent.get_url_path_from_location(loc) + return _path def find_file(fs, dirs, filename): @@ -243,7 +242,7 @@ def get_course_syllabus_section(course, section_key): return replace_static_urls( htmlFile.read().decode('utf-8'), getattr(course, 'data_dir', None), - course_namespace=course.location + course_id=course.location.course_id ) except ResourceNotFoundError: log.exception("Missing syllabus section {key} in course {url}".format( diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 0a48c56f87..a851a6d8f4 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -332,6 +332,7 @@ def get_module_for_descriptor_internal(user, descriptor, model_data_cache, cours # TODO (cpennington): When modules are shared between courses, the static # prefix is going to have to be specific to the module, not the directory # that the xml was loaded from + system = ModuleSystem( track_function=track_function, render_template=render_to_string, @@ -347,7 +348,7 @@ def get_module_for_descriptor_internal(user, descriptor, model_data_cache, cours replace_urls=partial( static_replace.replace_static_urls, data_directory=getattr(descriptor, 'data_dir', None), - course_namespace=descriptor.location._replace(category=None, name=None), + course_id=course_id, ), replace_course_urls=partial( static_replace.replace_course_urls, @@ -368,6 +369,7 @@ def get_module_for_descriptor_internal(user, descriptor, model_data_cache, cours cache=cache, can_execute_unsafe_code=(lambda: can_execute_unsafe_code(course_id)), ) + # pass position specified in URL to module through ModuleSystem system.set('position', position) system.set('DEBUG', settings.DEBUG) @@ -405,8 +407,12 @@ def get_module_for_descriptor_internal(user, descriptor, model_data_cache, cours module.get_html = replace_static_urls( _get_html, getattr(descriptor, 'data_dir', None), +<<<<<<< HEAD course_namespace=module.location._replace(category=None, name=None) ) +======= + course_id=course_id) +>>>>>>> WIP # Allow URLs of the form '/course/' refer to the root of multicourse directory # hierarchy of this course diff --git a/lms/djangoapps/staticbook/views.py b/lms/djangoapps/staticbook/views.py index 9ed14bfb6c..25475eb36a 100644 --- a/lms/djangoapps/staticbook/views.py +++ b/lms/djangoapps/staticbook/views.py @@ -50,7 +50,7 @@ def remap_static_url(original_url, course): output_url = replace_static_urls( input_url, getattr(course, 'data_dir', None), - course_namespace=course.location, + coures_id=course.location.course_id, ) # strip off the quotes again... return output_url[1:-1] diff --git a/lms/envs/cms/mixed_dev.py b/lms/envs/cms/mixed_dev.py index 22fd5eeb0e..9a1a3a4cb8 100644 --- a/lms/envs/cms/mixed_dev.py +++ b/lms/envs/cms/mixed_dev.py @@ -9,8 +9,7 @@ MODULESTORE = { 'ENGINE': 'xmodule.modulestore.mixed.MixedModuleStore', 'OPTIONS': { 'mappings': { - '6.002/a/a': 'xml', - '6.002/b/b': 'xml' + 'MITx/2.01x/2013_Spring': 'xml' }, 'stores': { 'xml': { From 5298f54f4c7edb45a7148e1e655badb991fe35b5 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 25 Jul 2013 10:56:57 -0400 Subject: [PATCH 06/27] fix tests --- cms/djangoapps/contentstore/module_info_model.py | 9 +-------- .../static_replace/test/test_static_replace.py | 9 ++++----- lms/djangoapps/courseware/tests/test_video_mongo.py | 9 ++++++++- lms/djangoapps/staticbook/views.py | 2 +- 4 files changed, 14 insertions(+), 15 deletions(-) diff --git a/cms/djangoapps/contentstore/module_info_model.py b/cms/djangoapps/contentstore/module_info_model.py index bce4b0326c..049f95c221 100644 --- a/cms/djangoapps/contentstore/module_info_model.py +++ b/cms/djangoapps/contentstore/module_info_model.py @@ -1,6 +1,5 @@ from static_replace import replace_static_urls from xmodule.modulestore.exceptions import ItemNotFoundError -from xmodule.modulestore import Location def get_module_info(store, location, rewrite_static_links=False): @@ -16,13 +15,7 @@ def get_module_info(store, location, rewrite_static_links=False): data = replace_static_urls( module.data, None, - course_namespace=Location([ - module.location.tag, - module.location.org, - module.location.course, - None, - None - ]) + course_id=module.location.org + '/' + module.location.course + '/REPLACE_WITH_RUN_WHEN_IMPLEMENTED' ) return { diff --git a/common/djangoapps/static_replace/test/test_static_replace.py b/common/djangoapps/static_replace/test/test_static_replace.py index f23610e1bd..b1bc05b895 100644 --- a/common/djangoapps/static_replace/test/test_static_replace.py +++ b/common/djangoapps/static_replace/test/test_static_replace.py @@ -10,7 +10,6 @@ from xmodule.modulestore.xml import XMLModuleStore DATA_DIRECTORY = 'data_dir' COURSE_ID = 'org/course/run' -NAMESPACE = Location('org', 'course', 'run', None, None) STATIC_SOURCE = '"/static/file.png"' @@ -52,18 +51,18 @@ def test_storage_url_not_exists(mock_storage): def test_mongo_filestore(mock_modulestore, mock_static_content): mock_modulestore.return_value = Mock(MongoModuleStore) - mock_static_content.convert_legacy_static_url.return_value = "c4x://mock_url" + mock_static_content.convert_legacy_static_url_with_course_id.return_value = "c4x://mock_url" # No namespace => no change to path assert_equals('"/static/data_dir/file.png"', replace_static_urls(STATIC_SOURCE, DATA_DIRECTORY)) # Namespace => content url assert_equals( - '"' + mock_static_content.convert_legacy_static_url.return_value + '"', - replace_static_urls(STATIC_SOURCE, DATA_DIRECTORY, NAMESPACE) + '"' + mock_static_content.convert_legacy_static_url_with_course_id.return_value + '"', + replace_static_urls(STATIC_SOURCE, DATA_DIRECTORY, course_id=COURSE_ID) ) - mock_static_content.convert_legacy_static_url.assert_called_once_with('file.png', NAMESPACE) + mock_static_content.convert_legacy_static_url_with_course_id.assert_called_once_with('file.png', COURSE_ID) @patch('static_replace.settings') diff --git a/lms/djangoapps/courseware/tests/test_video_mongo.py b/lms/djangoapps/courseware/tests/test_video_mongo.py index 65da586812..af71e15be7 100644 --- a/lms/djangoapps/courseware/tests/test_video_mongo.py +++ b/lms/djangoapps/courseware/tests/test_video_mongo.py @@ -54,7 +54,7 @@ class TestVideo(BaseTestXmodule): expected_context = { 'data_dir': getattr(self, 'data_dir', None), - 'caption_asset_path': '/c4x/MITx/999/asset/subs_', + 'caption_asset_path': '/static/subs/', 'show_captions': 'true', 'display_name': 'A Name', 'end': 3610.0, @@ -104,10 +104,17 @@ class TestVideoNonYouTube(TestVideo): expected_context = { 'data_dir': getattr(self, 'data_dir', None), +<<<<<<< HEAD 'caption_asset_path': '/c4x/MITx/999/asset/subs_', 'show_captions': 'true', 'display_name': 'A Name', 'end': 3610.0, +======= + 'caption_asset_path': '/static/subs/', + 'show_captions': self.item_module.show_captions, + 'display_name': self.item_module.display_name_with_default, + 'end': self.item_module.end_time, +>>>>>>> fix tests 'id': self.item_module.location.html_id(), 'sources': sources, 'start': 3603.0, diff --git a/lms/djangoapps/staticbook/views.py b/lms/djangoapps/staticbook/views.py index 25475eb36a..f73cfb6e5b 100644 --- a/lms/djangoapps/staticbook/views.py +++ b/lms/djangoapps/staticbook/views.py @@ -50,7 +50,7 @@ def remap_static_url(original_url, course): output_url = replace_static_urls( input_url, getattr(course, 'data_dir', None), - coures_id=course.location.course_id, + course_id=course.location.course_id, ) # strip off the quotes again... return output_url[1:-1] From fa61bdf69f7647ea401e03625a755d3297b08435 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 25 Jul 2013 11:25:38 -0400 Subject: [PATCH 07/27] remove debugging logging --- common/djangoapps/contentserver/middleware.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/common/djangoapps/contentserver/middleware.py b/common/djangoapps/contentserver/middleware.py index 733e1c30e3..d89e3fdd23 100644 --- a/common/djangoapps/contentserver/middleware.py +++ b/common/djangoapps/contentserver/middleware.py @@ -6,14 +6,11 @@ from xmodule.modulestore import InvalidLocationError from cache_toolbox.core import get_cached_content, set_cached_content from xmodule.exceptions import NotFoundError -import logging - class StaticContentServer(object): def process_request(self, request): # look to see if the request is prefixed with 'c4x' tag if request.path.startswith('/' + XASSET_LOCATION_TAG + '/'): - logging.debug('**** path = {0}'.format(request.path)) try: loc = StaticContent.get_location_from_path(request.path) except InvalidLocationError: @@ -27,10 +24,8 @@ class StaticContentServer(object): if content is None: # nope, not in cache, let's fetch from DB try: - logging.debug('!!!! loc = {0}'.format(loc)) content = contentstore().find(loc, as_stream=True) except NotFoundError: - logging.debug('**** NOT FOUND') response = HttpResponse() response.status_code = 404 return response From 54bd3170bc5eb7cc2c81d45bc18a6ae18efd0e43 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 25 Jul 2013 11:27:44 -0400 Subject: [PATCH 08/27] remove another debug logging message --- common/djangoapps/student/views.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 4d59b5cc66..7b24f80917 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -96,6 +96,8 @@ def index(request, extra_context={}, user=None): context = {'courses': courses} + context = {'courses': courses, 'news': top_news} + context.update(extra_context) return render_to_response('index.html', context) From 9f14f1ee62e30912239f59cbc746602a8d5459ca Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 25 Jul 2013 11:28:40 -0400 Subject: [PATCH 09/27] update file comment on MixedModuleStore --- common/lib/xmodule/xmodule/modulestore/mixed.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/modulestore/mixed.py b/common/lib/xmodule/xmodule/modulestore/mixed.py index 85ba95af48..34effafc14 100644 --- a/common/lib/xmodule/xmodule/modulestore/mixed.py +++ b/common/lib/xmodule/xmodule/modulestore/mixed.py @@ -3,7 +3,7 @@ MixedModuleStore allows for aggregation between multiple modulestores. In this way, courses can be served up both - say - XMLModuleStore or MongoModuleStore -IMPORTANT: This modulestore is experimental AND INCOMPLETE. Therefore this should only be used cautiously +IMPORTANT: This modulestore only supports READONLY applications, e.g. LMS """ from . import ModuleStoreBase From aa8b0545907d8844d813538d9deb88936a9b45d5 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 25 Jul 2013 13:38:18 -0400 Subject: [PATCH 10/27] fix missed conflict resolution --- lms/djangoapps/courseware/module_render.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index a851a6d8f4..4f05d660be 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -407,12 +407,8 @@ def get_module_for_descriptor_internal(user, descriptor, model_data_cache, cours module.get_html = replace_static_urls( _get_html, getattr(descriptor, 'data_dir', None), -<<<<<<< HEAD - course_namespace=module.location._replace(category=None, name=None) + course_id=course_id ) -======= - course_id=course_id) ->>>>>>> WIP # Allow URLs of the form '/course/' refer to the root of multicourse directory # hierarchy of this course From e4eea6cc4f9a689881890da869ea2c74bf2defc9 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Mon, 29 Jul 2013 21:15:39 -0400 Subject: [PATCH 11/27] some additional comments to clarify the partially bogus course_id. --- cms/djangoapps/contentstore/module_info_model.py | 4 +++- cms/djangoapps/contentstore/views/preview.py | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/cms/djangoapps/contentstore/module_info_model.py b/cms/djangoapps/contentstore/module_info_model.py index 049f95c221..c0e1ff7207 100644 --- a/cms/djangoapps/contentstore/module_info_model.py +++ b/cms/djangoapps/contentstore/module_info_model.py @@ -12,10 +12,12 @@ def get_module_info(store, location, rewrite_static_links=False): data = module.data if rewrite_static_links: + # we pass a partially bogus course_id as we don't have the RUN information passed yet + # through the CMS. Also the contentstore is also not RUN-aware at this point in time. data = replace_static_urls( module.data, None, - course_id=module.location.org + '/' + module.location.course + '/REPLACE_WITH_RUN_WHEN_IMPLEMENTED' + course_id=module.location.org + '/' + module.location.course + '/BOGUS_RUN_REPLACE_WHEN_AVAILABLE' ) return { diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index bbbf3d5f0a..75ee59dd2d 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -155,10 +155,12 @@ def load_preview_module(request, preview_id, descriptor): "xmodule_display.html", ) + # we pass a partially bogus course_id as we don't have the RUN information passed yet + # through the CMS. Also the contentstore is also not RUN-aware at this point in time. module.get_html = replace_static_urls( module.get_html, getattr(module, 'data_dir', module.location.course), - course_id=module.location.org+'/'+module.location.course+'/REPLACE_WITH_RUN' + course_id=module.location.org+'/'+module.location.course+'/BOGUS_RUN_REPLACE_WHEN_AVAILABLE' ) module.get_html = save_module( From 52928d165a6f69decb5f682abd08e212fa2392b1 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Mon, 29 Jul 2013 21:30:07 -0400 Subject: [PATCH 12/27] update SplitModuleStore tests to pull the course_id from the locator --- .../modulestore/tests/test_split_modulestore.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py index ca5eb72a26..1f297add36 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py @@ -265,11 +265,11 @@ class SplitModuleItemTests(SplitModuleTest): locator = BlockUsageLocator(course_id='GreekHero', usage_id='head12345', branch='draft') self.assertTrue( - modulestore().has_item(course_id, locator), + modulestore().has_item(locator.course_id, locator), "couldn't find in 12345" ) self.assertTrue( - modulestore().has_item(course_id, BlockUsageLocator( + modulestore().has_item(locator.course_id, BlockUsageLocator( course_id=locator.course_id, branch='draft', usage_id=locator.usage_id @@ -277,7 +277,7 @@ class SplitModuleItemTests(SplitModuleTest): "couldn't find in draft 12345" ) self.assertFalse( - modulestore().has_item(course_id, BlockUsageLocator( + modulestore().has_item(locator.course_id, BlockUsageLocator( course_id=locator.course_id, branch='published', usage_id=locator.usage_id)), @@ -285,26 +285,27 @@ class SplitModuleItemTests(SplitModuleTest): ) locator.branch = 'draft' self.assertTrue( - modulestore().has_item(course_id, locator), + modulestore().has_item(locator.course_id, locator), "not found in draft 12345" ) # not a course obj locator = BlockUsageLocator(course_id='GreekHero', usage_id='chapter1', branch='draft') self.assertTrue( - modulestore().has_item(course_id, locator), + modulestore().has_item(locator.course_id, locator), "couldn't find chapter1" ) # in published course locator = BlockUsageLocator(course_id="wonderful", usage_id="head23456", revision='draft') - self.assertTrue(modulestore().has_item(course_id, BlockUsageLocator(course_id=locator.course_id, + self.assertTrue(modulestore().has_item(locator.course_id, BlockUsageLocator(course_id=locator.course_id, usage_id=locator.usage_id, revision='published')), "couldn't find in 23456") locator.branch = 'published' self.assertTrue(modulestore().has_item(course_id, locator), "couldn't find in 23456") + def test_negative_has_item(self): # negative tests--not found # no such course or block From 7bdc4c51340e83dbeaad23e2fcb9d80934e91247 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Mon, 5 Aug 2013 13:40:26 -0400 Subject: [PATCH 13/27] fix errant conflict resolution --- common/djangoapps/student/views.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 7b24f80917..4d59b5cc66 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -96,8 +96,6 @@ def index(request, extra_context={}, user=None): context = {'courses': courses} - context = {'courses': courses, 'news': top_news} - context.update(extra_context) return render_to_response('index.html', context) From 7a80a9c02aefc47b159a541e69d5c51683d77afa Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 8 Aug 2013 15:40:28 -0400 Subject: [PATCH 14/27] fix broken test after rebase --- .../xmodule/xmodule/modulestore/tests/test_split_modulestore.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py index 1f297add36..530a173d23 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py @@ -297,7 +297,7 @@ class SplitModuleItemTests(SplitModuleTest): ) # in published course - locator = BlockUsageLocator(course_id="wonderful", usage_id="head23456", revision='draft') + locator = BlockUsageLocator(course_id="wonderful", usage_id="head23456", branch='draft') self.assertTrue(modulestore().has_item(locator.course_id, BlockUsageLocator(course_id=locator.course_id, usage_id=locator.usage_id, revision='published')), From 9cc796df833f7abfbb68f63f58c729cf07848b58 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 8 Aug 2013 15:52:11 -0400 Subject: [PATCH 15/27] fix one more 'revision' -> 'branch' argument name change --- .../xmodule/xmodule/modulestore/tests/test_split_modulestore.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py index 530a173d23..c217646725 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py @@ -300,7 +300,7 @@ class SplitModuleItemTests(SplitModuleTest): locator = BlockUsageLocator(course_id="wonderful", usage_id="head23456", branch='draft') self.assertTrue(modulestore().has_item(locator.course_id, BlockUsageLocator(course_id=locator.course_id, usage_id=locator.usage_id, - revision='published')), + branch='published')), "couldn't find in 23456") locator.branch = 'published' self.assertTrue(modulestore().has_item(course_id, locator), "couldn't find in 23456") From 5ee5beafbef101d6c6a705dd9051a6e722547dc7 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Tue, 13 Aug 2013 12:42:24 -0400 Subject: [PATCH 16/27] fix bad merge conflict resolution --- lms/djangoapps/courseware/tests/test_video_mongo.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/lms/djangoapps/courseware/tests/test_video_mongo.py b/lms/djangoapps/courseware/tests/test_video_mongo.py index af71e15be7..3436938cc0 100644 --- a/lms/djangoapps/courseware/tests/test_video_mongo.py +++ b/lms/djangoapps/courseware/tests/test_video_mongo.py @@ -104,17 +104,10 @@ class TestVideoNonYouTube(TestVideo): expected_context = { 'data_dir': getattr(self, 'data_dir', None), -<<<<<<< HEAD - 'caption_asset_path': '/c4x/MITx/999/asset/subs_', + 'caption_asset_path': '/static/subs/', 'show_captions': 'true', 'display_name': 'A Name', 'end': 3610.0, -======= - 'caption_asset_path': '/static/subs/', - 'show_captions': self.item_module.show_captions, - 'display_name': self.item_module.display_name_with_default, - 'end': self.item_module.end_time, ->>>>>>> fix tests 'id': self.item_module.location.html_id(), 'sources': sources, 'start': 3603.0, From 0b8866ef29fe82449138675586c87b49c611fe03 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Tue, 13 Aug 2013 17:01:09 -0400 Subject: [PATCH 17/27] forgot to return dictionary on get_errored_courses --- common/lib/xmodule/xmodule/modulestore/mixed.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/modulestore/mixed.py b/common/lib/xmodule/xmodule/modulestore/mixed.py index 34effafc14..ec2b563cac 100644 --- a/common/lib/xmodule/xmodule/modulestore/mixed.py +++ b/common/lib/xmodule/xmodule/modulestore/mixed.py @@ -129,7 +129,7 @@ class MixedModuleStore(ModuleStoreBase): def get_modulestore_type(self, course_id): """ - Returns a type which identifies which modulestore is servicing the given + Returns a type which identifies which modulestore is servicing the given course_id. The return can be either "xml" (for XML based courses) or "mongo" for MongoDB backed courses """ return self._get_modulestore_for_courseid(course_id).get_modulestore_type(course_id) @@ -142,3 +142,4 @@ class MixedModuleStore(ModuleStoreBase): errs = {} for store in self.modulestores.values(): errs.update(store.get_errored_courses()) + return errs From 61219169d43f9c2af634b81f0ab26af68b9e6479 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Tue, 13 Aug 2013 19:56:00 -0400 Subject: [PATCH 18/27] change over the modulestore configuration to be a dict and use a property accessor. Also update some django-admin configs to use this means to set the runtime configuration --- .../management/commands/check_course.py | 8 +++++++- .../management/commands/clone_course.py | 11 +++++------ .../management/commands/delete_course.py | 7 +++++-- common/lib/xmodule/xmodule/modulestore/__init__.py | 14 ++++++++++---- .../lib/xmodule/xmodule/modulestore/mongo/base.py | 5 +---- lms/djangoapps/courseware/tests/__init__.py | 3 +-- .../instructor/tests/test_legacy_gradebook.py | 3 ++- 7 files changed, 31 insertions(+), 20 deletions(-) diff --git a/cms/djangoapps/contentstore/management/commands/check_course.py b/cms/djangoapps/contentstore/management/commands/check_course.py index 215bb8add8..2f0b0b2a2c 100644 --- a/cms/djangoapps/contentstore/management/commands/check_course.py +++ b/cms/djangoapps/contentstore/management/commands/check_course.py @@ -5,6 +5,9 @@ from xmodule.course_module import CourseDescriptor from request_cache.middleware import RequestCache +from django.core.cache import get_cache + +CACHE = get_cache('mongo_metadata_inheritance') class Command(BaseCommand): help = '''Enumerates through the course and find common errors''' @@ -19,7 +22,10 @@ class Command(BaseCommand): store = modulestore() # setup a request cache so we don't throttle the DB with all the metadata inheritance requests - store.request_cache = RequestCache.get_request_cache() + store.set_modulestore_configuration({ + 'metadata_inheritance_cache_subsystem': CACHE, + 'request_cache': RequestCache.get_request_cache() + }) course = store.get_item(loc, depth=3) diff --git a/cms/djangoapps/contentstore/management/commands/clone_course.py b/cms/djangoapps/contentstore/management/commands/clone_course.py index 5fffe29543..aa0e076f08 100644 --- a/cms/djangoapps/contentstore/management/commands/clone_course.py +++ b/cms/djangoapps/contentstore/management/commands/clone_course.py @@ -15,10 +15,6 @@ from auth.authz import _copy_course_group from request_cache.middleware import RequestCache from django.core.cache import get_cache -# -# To run from command line: rake cms:delete_course LOC=MITx/111/Foo1 -# - CACHE = get_cache('mongo_metadata_inheritance') class Command(BaseCommand): @@ -36,8 +32,11 @@ class Command(BaseCommand): mstore = modulestore('direct') cstore = contentstore() - mstore.metadata_inheritance_cache_subsystem = CACHE - mstore.request_cache = RequestCache.get_request_cache() + mstore.set_modulestore_configuration({ + 'metadata_inheritance_cache_subsystem': CACHE, + 'request_cache': RequestCache.get_request_cache() + }) + org, course_num, run = dest_course_id.split("/") mstore.ignore_write_events_on_courses.append('{0}/{1}'.format(org, course_num)) diff --git a/cms/djangoapps/contentstore/management/commands/delete_course.py b/cms/djangoapps/contentstore/management/commands/delete_course.py index 3e0ecfb8d9..b0901ccfc9 100644 --- a/cms/djangoapps/contentstore/management/commands/delete_course.py +++ b/cms/djangoapps/contentstore/management/commands/delete_course.py @@ -36,8 +36,11 @@ class Command(BaseCommand): ms = modulestore('direct') cs = contentstore() - ms.metadata_inheritance_cache_subsystem = CACHE - ms.request_cache = RequestCache.get_request_cache() + ms.set_modulestore_configuration({ + 'metadata_inheritance_cache_subsystem': CACHE, + 'request_cache': RequestCache.get_request_cache() + }) + org, course_num, run = course_id.split("/") ms.ignore_write_events_on_courses.append('{0}/{1}'.format(org, course_num)) diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index ee61998931..a7e29ed105 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -410,8 +410,7 @@ class ModuleStoreBase(ModuleStore): Set up the error-tracking logic. ''' self._location_errors = {} # location -> ErrorLog - self.metadata_inheritance_cache = None - self.request_cache = None + self.modulestore_configuration = {} self.modulestore_update_signal = None # can be set by runtime to route notifications of datastore changes def _get_errorlog(self, location): @@ -456,13 +455,20 @@ class ModuleStoreBase(ModuleStore): return c return None + @property + def metadata_inheritance_cache_subsystem(self): + return self.modulestore_configuration.get('metadata_inheritance_cache_subsystem', None) + + @property + def request_cache(self): + return self.modulestore_configuration.get('request_cache', None) + def set_modulestore_configuration(self, config_dict): """ This is the base implementation of the interface, all we need to do is store two possible configurations as attributes on the class """ - self.metadata_inheritance_cache = config_dict.get('metadata_inheritance_cache_subsystem', None) - self.request_cache = config_dict.get('request_cache', None) + self.modulestore_configuration = config_dict def namedtuple_to_son(namedtuple, prefix=''): diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index de946f76e1..ad2732409d 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -270,8 +270,7 @@ class MongoModuleStore(ModuleStoreBase): def __init__(self, host, db, collection, fs_root, render_template, port=27017, default_class=None, error_tracker=null_error_tracker, - user=None, password=None, request_cache=None, - metadata_inheritance_cache_subsystem=None, **kwargs): + user=None, password=None, **kwargs): super(MongoModuleStore, self).__init__() @@ -303,8 +302,6 @@ class MongoModuleStore(ModuleStoreBase): self.error_tracker = error_tracker self.render_template = render_template self.ignore_write_events_on_courses = [] - self.request_cache = request_cache - self.metadata_inheritance_cache_subsystem = metadata_inheritance_cache_subsystem def compute_metadata_inheritance_tree(self, location): ''' diff --git a/lms/djangoapps/courseware/tests/__init__.py b/lms/djangoapps/courseware/tests/__init__.py index 40f1df0fbc..9d1b549b9f 100644 --- a/lms/djangoapps/courseware/tests/__init__.py +++ b/lms/djangoapps/courseware/tests/__init__.py @@ -50,8 +50,7 @@ class BaseTestXmodule(ModuleStoreTestCase): self.course = CourseFactory.create(data=self.COURSE_DATA) # Turn off cache. - modulestore().request_cache = None - modulestore().metadata_inheritance_cache_subsystem = None + modulestore().set_modulestore_configuration({}) chapter = ItemFactory.create( parent_location=self.course.location, diff --git a/lms/djangoapps/instructor/tests/test_legacy_gradebook.py b/lms/djangoapps/instructor/tests/test_legacy_gradebook.py index 8be648a930..aaf03deb8c 100644 --- a/lms/djangoapps/instructor/tests/test_legacy_gradebook.py +++ b/lms/djangoapps/instructor/tests/test_legacy_gradebook.py @@ -25,7 +25,8 @@ class TestGradebook(ModuleStoreTestCase): instructor = AdminFactory.create() self.client.login(username=instructor.username, password='test') - modulestore().request_cache = modulestore().metadata_inheritance_cache_subsystem = None + # remove the caches + modulestore().set_modulestore_configuration({}) kwargs = {} if self.grading_policy is not None: From c6cde6a72204a9e688ea0d6dfe9550f2cb39a0fc Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Tue, 13 Aug 2013 20:12:37 -0400 Subject: [PATCH 19/27] resolve incorrect merge conflict resolution --- common/lib/xmodule/xmodule/modulestore/tests/test_xml.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py b/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py index 676d928bb7..ffbce40874 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py @@ -7,7 +7,7 @@ from xmodule.modulestore.xml import XMLModuleStore from xmodule.modulestore import XML_MODULESTORE_TYPE from .test_modulestore import check_path_to_location -from . import DATA_DIR +from xmodule.tests import DATA_DIR class TestXMLModuleStore(object): From ed584a9abb11644ccf67857045343cec338a6701 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 15 Aug 2013 12:41:30 -0400 Subject: [PATCH 20/27] fix pep8 violations --- cms/djangoapps/contentstore/views/preview.py | 2 +- common/lib/xmodule/xmodule/modulestore/__init__.py | 2 +- .../modulestore/tests/test_split_modulestore.py | 11 ++++++----- common/lib/xmodule/xmodule/modulestore/xml.py | 2 +- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 75ee59dd2d..7a3a224d86 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -160,7 +160,7 @@ def load_preview_module(request, preview_id, descriptor): module.get_html = replace_static_urls( module.get_html, getattr(module, 'data_dir', module.location.course), - course_id=module.location.org+'/'+module.location.course+'/BOGUS_RUN_REPLACE_WHEN_AVAILABLE' + course_id=module.location.org + '/' + module.location.course + '/BOGUS_RUN_REPLACE_WHEN_AVAILABLE' ) module.get_html = save_module( diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index a7e29ed105..eb9c44a441 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -395,7 +395,7 @@ class ModuleStore(object): def get_modulestore_type(self, course_id): """ - Returns a type which identifies which modulestore is servicing the given + Returns a type which identifies which modulestore is servicing the given course_id. The return can be either "xml" (for XML based courses) or "mongo" for MongoDB backed courses """ raise NotImplementedError diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py index c217646725..d548dcc838 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py @@ -298,14 +298,15 @@ class SplitModuleItemTests(SplitModuleTest): # in published course locator = BlockUsageLocator(course_id="wonderful", usage_id="head23456", branch='draft') - self.assertTrue(modulestore().has_item(locator.course_id, BlockUsageLocator(course_id=locator.course_id, - usage_id=locator.usage_id, - branch='published')), - "couldn't find in 23456") + self.assertTrue( + modulestore().has_item( + locator.course_id, + BlockUsageLocator(course_id=locator.course_id, usage_id=locator.usage_id, branch='published') + ), "couldn't find in 23456" + ) locator.branch = 'published' self.assertTrue(modulestore().has_item(course_id, locator), "couldn't find in 23456") - def test_negative_has_item(self): # negative tests--not found # no such course or block diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index fd04982f81..89c3299394 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -604,7 +604,7 @@ class XMLModuleStore(ModuleStoreBase): def get_modulestore_type(self, course_id): """ - Returns a type which identifies which modulestore is servicing the given + Returns a type which identifies which modulestore is servicing the given course_id. The return can be either "xml" (for XML based courses) or "mongo" for MongoDB backed courses """ return XML_MODULESTORE_TYPE From cf715cb7278405bd8294c541b41951919d1d1d46 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Thu, 15 Aug 2013 12:48:01 -0400 Subject: [PATCH 21/27] fix pylint violations --- common/lib/xmodule/xmodule/contentstore/content.py | 6 +++++- common/lib/xmodule/xmodule/modulestore/__init__.py | 6 ++++++ lms/envs/cms/mixed_dev.py | 6 +++++- 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/contentstore/content.py b/common/lib/xmodule/xmodule/contentstore/content.py index cdecfd1af5..9d767482d5 100644 --- a/common/lib/xmodule/xmodule/contentstore/content.py +++ b/common/lib/xmodule/xmodule/contentstore/content.py @@ -102,7 +102,11 @@ class StaticContent(object): @staticmethod def convert_legacy_static_url_with_course_id(path, course_id): - org, course_num, run = course_id.split("/") + """ + Returns a path to a piece of static content when we are provided with a filepath and + a course_id + """ + org, course_num, __ = course_id.split("/") loc = StaticContent.compute_location(org, course_num, path) return StaticContent.get_url_path_from_location(loc) diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index eb9c44a441..707390d759 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -457,10 +457,16 @@ class ModuleStoreBase(ModuleStore): @property def metadata_inheritance_cache_subsystem(self): + """ + Exposes an accessor to the runtime configuration for the metadata inheritance cache + """ return self.modulestore_configuration.get('metadata_inheritance_cache_subsystem', None) @property def request_cache(self): + """ + Exposes an accessor to the runtime configuration for the request cache + """ return self.modulestore_configuration.get('request_cache', None) def set_modulestore_configuration(self, config_dict): diff --git a/lms/envs/cms/mixed_dev.py b/lms/envs/cms/mixed_dev.py index 9a1a3a4cb8..88e1f3d1f8 100644 --- a/lms/envs/cms/mixed_dev.py +++ b/lms/envs/cms/mixed_dev.py @@ -2,7 +2,11 @@ This configuration is to run the MixedModuleStore on a localdev environment """ -from .dev import * +# We intentionally define lots of variables that aren't used, and +# want to import all variables from base settings files +# pylint: disable=W0401, W0614 + +from .dev import *, DATA_DIR MODULESTORE = { 'default': { From bca2018ab5eb8defdd8ecca769d3fa8a3d852067 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 16 Aug 2013 00:53:15 -0400 Subject: [PATCH 22/27] add unit tests for all methods in MixedModuleStore --- .../lib/xmodule/xmodule/modulestore/django.py | 2 +- .../tests/test_mixed_modulestore.py | 242 ++++++++++++++++++ 2 files changed, 243 insertions(+), 1 deletion(-) create mode 100644 common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py diff --git a/common/lib/xmodule/xmodule/modulestore/django.py b/common/lib/xmodule/xmodule/modulestore/django.py index 7b1ce37d07..2f0cd126f9 100644 --- a/common/lib/xmodule/xmodule/modulestore/django.py +++ b/common/lib/xmodule/xmodule/modulestore/django.py @@ -35,7 +35,7 @@ def create_modulestore_instance(engine, options): _options.update(options) for key in FUNCTION_KEYS: - if key in _options: + if key in _options and isinstance(_options[key], basestring): _options[key] = load_function(_options[key]) return class_( diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py new file mode 100644 index 0000000000..c065b14de9 --- /dev/null +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -0,0 +1,242 @@ +from nose.tools import assert_equals, assert_raises, assert_false, assert_true, assert_not_equals +import pymongo +from uuid import uuid4 + +from xmodule.tests import DATA_DIR +from xmodule.modulestore import Location, MONGO_MODULESTORE_TYPE, XML_MODULESTORE_TYPE +from xmodule.modulestore.exceptions import ItemNotFoundError +from xmodule.modulestore.mixed import MixedModuleStore +from xmodule.modulestore.xml_importer import import_from_xml + + +HOST = 'localhost' +PORT = 27017 +DB = 'test_mongo_%s' % uuid4().hex +COLLECTION = 'modulestore' +FS_ROOT = DATA_DIR # TODO (vshnayder): will need a real fs_root for testing load_item +DEFAULT_CLASS = 'xmodule.raw_module.RawDescriptor' +RENDER_TEMPLATE = lambda t_n, d, ctx = None, nsp = 'main': '' + +IMPORT_COURSEID = 'MITx/999/2013_Spring' +XML_COURSEID1 = 'edX/toy/2012_Fall' +XML_COURSEID2 = 'edX/simple/2012_Fall' + +OPTIONS = { + 'mappings': { + XML_COURSEID1: 'xml', + XML_COURSEID2: 'xml', + IMPORT_COURSEID: 'default' + }, + 'stores': { + 'xml': { + 'ENGINE': 'xmodule.modulestore.xml.XMLModuleStore', + 'OPTIONS': { + 'data_dir': DATA_DIR, + 'default_class': 'xmodule.hidden_module.HiddenDescriptor', + } + }, + 'default': { + 'ENGINE': 'xmodule.modulestore.mongo.MongoModuleStore', + 'OPTIONS': { + 'default_class': DEFAULT_CLASS, + 'host': HOST, + 'db': DB, + 'collection': COLLECTION, + 'fs_root': DATA_DIR, + 'render_template': RENDER_TEMPLATE, + } + } + } +} + + +class TestMixedModuleStore(object): + '''Tests!''' + @classmethod + def setupClass(cls): + cls.connection = pymongo.connection.Connection(HOST, PORT) + cls.connection.drop_database(DB) + cls.fake_location = Location(['i4x', 'foo', 'bar', 'vertical', 'baz']) + cls.import_org, cls.import_course, cls.import_run = IMPORT_COURSEID.split('/') + # NOTE: Creating a single db for all the tests to save time. This + # is ok only as long as none of the tests modify the db. + # If (when!) that changes, need to either reload the db, or load + # once and copy over to a tmp db for each test. + cls.store = cls.initdb() + + @classmethod + def teardownClass(cls): + cls.connection = pymongo.connection.Connection(HOST, PORT) + cls.connection.drop_database(DB) + + @staticmethod + def initdb(): + # connect to the db + _options = {} + _options.update(OPTIONS) + store = MixedModuleStore(**_options) + + import_from_xml( + store._get_modulestore_for_courseid(IMPORT_COURSEID), + DATA_DIR, + ['toy'], + target_location_namespace=Location( + 'i4x', + TestMixedModuleStore.import_org, + TestMixedModuleStore.import_course, + 'course', + TestMixedModuleStore.import_run + ) + ) + + return store + + @staticmethod + def destroy_db(connection): + # Destroy the test db. + connection.drop_database(DB) + + def setUp(self): + # make a copy for convenience + self.connection = TestMixedModuleStore.connection + + def tearDown(self): + pass + + def test_get_modulestore_type(self): + """ + Make sure we get back the store type we expect for given mappings + """ + assert_equals(self.store.get_modulestore_type(XML_COURSEID1), XML_MODULESTORE_TYPE) + assert_equals(self.store.get_modulestore_type(XML_COURSEID2), XML_MODULESTORE_TYPE) + assert_equals(self.store.get_modulestore_type(IMPORT_COURSEID), MONGO_MODULESTORE_TYPE) + # try an unknown mapping, it should be the 'default' store + assert_equals(self.store.get_modulestore_type('foo/bar/2012_Fall'), MONGO_MODULESTORE_TYPE) + + def test_has_item(self): + assert_true(self.store.has_item( + IMPORT_COURSEID, Location(['i4x', self.import_org, self.import_course, 'course', self.import_run]) + )) + assert_true(self.store.has_item( + XML_COURSEID1, Location(['i4x', 'edX', 'toy', 'course', '2012_Fall']) + )) + + # try negative cases + assert_false(self.store.has_item( + XML_COURSEID1, Location(['i4x', self.import_org, self.import_course, 'course', self.import_run]) + )) + assert_false(self.store.has_item( + IMPORT_COURSEID, Location(['i4x', 'edX', 'toy', 'course', '2012_Fall']) + )) + + def test_get_item(self): + with assert_raises(NotImplementedError): + self.store.get_item(self.fake_location) + + def test_get_instance(self): + module = self.store.get_instance( + IMPORT_COURSEID, Location(['i4x', self.import_org, self.import_course, 'course', self.import_run]) + ) + assert_not_equals(module, None) + + module = self.store.get_instance( + XML_COURSEID1, Location(['i4x', 'edX', 'toy', 'course', '2012_Fall']) + ) + assert_not_equals(module, None) + + # try negative cases + with assert_raises(ItemNotFoundError): + self.store.get_instance( + XML_COURSEID1, Location(['i4x', self.import_org, self.import_course, 'course', self.import_run]) + ) + + with assert_raises(ItemNotFoundError): + self.store.get_instance( + IMPORT_COURSEID, Location(['i4x', 'edX', 'toy', 'course', '2012_Fall']) + ) + + def test_get_items(self): + modules = self.store.get_items(['i4x', None, None, 'course', None], IMPORT_COURSEID) + assert_equals(len(modules), 1) + assert_equals(modules[0].location.course, self.import_course) + + modules = self.store.get_items(['i4x', None, None, 'course', None], XML_COURSEID1) + assert_equals(len(modules), 1) + assert_equals(modules[0].location.course, 'toy') + + modules = self.store.get_items(['i4x', None, None, 'course', None], XML_COURSEID2) + assert_equals(len(modules), 1) + assert_equals(modules[0].location.course, 'simple') + + def test_update_item(self): + with assert_raises(NotImplementedError): + self.store.update_item(self.fake_location, None) + + def test_update_children(self): + with assert_raises(NotImplementedError): + self.store.update_children(self.fake_location, None) + + def test_update_metadata(self): + with assert_raises(NotImplementedError): + self.store.update_metadata(self.fake_location, None) + + def test_delete_item(self): + with assert_raises(NotImplementedError): + self.store.delete_item(self.fake_location) + + def test_get_courses(self): + # we should have 3 total courses aggregated + courses = self.store.get_courses() + course_ids = [] + for course in courses: + course_ids.append(course.location.course_id) + assert_true(IMPORT_COURSEID in course_ids) + assert_true(XML_COURSEID1 in course_ids) + assert_true(XML_COURSEID2 in course_ids) + + def test_get_course(self): + module = self.store.get_course(IMPORT_COURSEID) + assert_equals(module.location.course, self.import_course) + + module = self.store.get_course(XML_COURSEID1) + assert_equals(module.location.course, 'toy') + + module = self.store.get_course(XML_COURSEID2) + assert_equals(module.location.course, 'simple') + + def test_get_parent_locations(self): + parents = self.store.get_parent_locations( + Location(['i4x', self.import_org, self.import_course, 'chapter', 'Overview']), + IMPORT_COURSEID + ) + assert_equals(len(parents), 1) + assert_equals(Location(parents[0]).org, self.import_org) + assert_equals(Location(parents[0]).course, self.import_course) + assert_equals(Location(parents[0]).name, self.import_run) + + parents = self.store.get_parent_locations( + Location(['i4x', 'edX', 'toy', 'chapter', 'Overview']), + XML_COURSEID1 + ) + assert_equals(len(parents), 1) + assert_equals(Location(parents[0]).org, 'edX') + assert_equals(Location(parents[0]).course, 'toy') + assert_equals(Location(parents[0]).name, '2012_Fall') + + def test_set_modulestore_configuration(self): + config = {'foo': 'bar'} + self.store.set_modulestore_configuration(config) + assert_equals( + config, + self.store._get_modulestore_for_courseid(IMPORT_COURSEID).modulestore_configuration + ) + + assert_equals( + config, + self.store._get_modulestore_for_courseid(XML_COURSEID1).modulestore_configuration + ) + + assert_equals( + config, + self.store._get_modulestore_for_courseid(XML_COURSEID2).modulestore_configuration + ) From ae6f97a3683b61d5d4e21948a66c6238b985df4e Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 16 Aug 2013 09:51:57 -0400 Subject: [PATCH 23/27] add test for test_static_url_generation_from_courseid --- common/lib/xmodule/xmodule/tests/test_content.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/common/lib/xmodule/xmodule/tests/test_content.py b/common/lib/xmodule/xmodule/tests/test_content.py index e73c33197c..6f5cf8ab8c 100644 --- a/common/lib/xmodule/xmodule/tests/test_content.py +++ b/common/lib/xmodule/xmodule/tests/test_content.py @@ -19,12 +19,18 @@ class ContentTest(unittest.TestCase): content = StaticContent('loc', 'name', 'content_type', 'data') self.assertIsNone(content.thumbnail_location) + + def test_static_url_generation_from_courseid(self): + url = StaticContent.convert_legacy_static_url_with_course_id('images_course_image.jpg', 'foo/bar/bz') + self.assertEqual(url, '/c4x/foo/bar/asset/images_course_image.jpg') + def test_generate_thumbnail_image(self): contentStore = ContentStore() content = Content(Location(u'c4x', u'mitX', u'800', u'asset', u'monsters__.jpg'), None) (thumbnail_content, thumbnail_file_location) = contentStore.generate_thumbnail(content) self.assertIsNone(thumbnail_content) self.assertEqual(Location(u'c4x', u'mitX', u'800', u'thumbnail', u'monsters__.jpg'), thumbnail_file_location) + def test_compute_location(self): # We had a bug that __ got converted into a single _. Make sure that substitution of INVALID_CHARS (like space) # still happen. From 13ff461461f13beb2482ef07a973d73349545e08 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 16 Aug 2013 10:30:47 -0400 Subject: [PATCH 24/27] add a filter to get_courses to not surface any courses that haven't been mapped, unless the store provider has been labeled as 'default' --- common/lib/xmodule/xmodule/modulestore/mixed.py | 15 ++++++++++++++- .../modulestore/tests/test_mixed_modulestore.py | 1 + 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/modulestore/mixed.py b/common/lib/xmodule/xmodule/modulestore/mixed.py index ec2b563cac..9548323115 100644 --- a/common/lib/xmodule/xmodule/modulestore/mixed.py +++ b/common/lib/xmodule/xmodule/modulestore/mixed.py @@ -103,7 +103,20 @@ class MixedModuleStore(ModuleStoreBase): ''' courses = [] for key in self.modulestores: - courses = courses + (self.modulestores[key].get_courses()) + store_courses = self.modulestores[key].get_courses() + # If the store has not been labeled as 'default' then we should + # only surface courses that have a mapping entry, for example the XMLModuleStore will + # slurp up anything that is on disk, however, we don't want to surface those to + # consumers *unless* there is an explicit mapping in the configuration + if key != 'default': + for course in store_courses: + # make sure that the courseId is mapped to the store in question + if key == self.mappings.get(course.location.course_id, 'default'): + courses = courses + ([course]) + else: + # if we're the 'default' store provider, then we surface all courses hosted in + # that store provider + courses = courses + (store_courses) return courses diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py index c065b14de9..70e4d1a5d3 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -187,6 +187,7 @@ class TestMixedModuleStore(object): def test_get_courses(self): # we should have 3 total courses aggregated courses = self.store.get_courses() + assert_equals(len(courses), 3) course_ids = [] for course in courses: course_ids.append(course.location.course_id) From d1ce55f65c2a9019691c8184cb14c0a2a7353107 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 16 Aug 2013 12:02:58 -0400 Subject: [PATCH 25/27] have the test teardown explicitly call destroy_db --- .../xmodule/modulestore/tests/test_mixed_modulestore.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py index 70e4d1a5d3..e04ef9c363 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -66,8 +66,7 @@ class TestMixedModuleStore(object): @classmethod def teardownClass(cls): - cls.connection = pymongo.connection.Connection(HOST, PORT) - cls.connection.drop_database(DB) + cls.destroy_db(cls.connection) @staticmethod def initdb(): From 1128f369f7261d8a6dfaae7bb5dc0e708f5b6e79 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 16 Aug 2013 15:24:20 -0400 Subject: [PATCH 26/27] add changelog entry --- CHANGELOG.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index e108a31594..1338cacd3b 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -96,6 +96,8 @@ LMS: Removed press releases Common: Updated Sass and Bourbon libraries, added Neat library +LMS: Add a MixedModuleStore to aggregate the XMLModuleStore and MongoMonduleStore + LMS: Users are no longer auto-activated if they click "reset password" This is now done when they click on the link in the reset password email they receive (along with usual path through activation email). From bd71a2ceb3a03d283db6e6a255ac409b80419b20 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 16 Aug 2013 15:24:43 -0400 Subject: [PATCH 27/27] add unit test for video_caption asset path --- .../contentstore/tests/test_contentstore.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index e70df4164a..96b0b84e36 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -400,6 +400,20 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): resp = self.client.get(url) self.assertEqual(resp.status_code, 200) + def test_video_module_caption_asset_path(self): + ''' + This verifies that a video caption url is as we expect it to be + ''' + direct_store = modulestore('direct') + import_from_xml(direct_store, 'common/test/data/', ['toy']) + + # also try a custom response which will trigger the 'is this course in whitelist' logic + video_module_location = Location(['i4x', 'edX', 'toy', 'video', 'sample_video', None]) + url = reverse('preview_component', kwargs={'location': video_module_location.url()}) + resp = self.client.get(url) + self.assertEqual(resp.status_code, 200) + self.assertContains(resp, 'data-caption-asset-path="/c4x/edX/toy/asset/subs_"') + def test_delete(self): direct_store = modulestore('direct') CourseFactory.create(org='edX', course='999', display_name='Robot Super Course')