From 0ded6699092cbeba2779c408b1e4b34f384f1942 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Tue, 28 Oct 2014 21:53:57 -0700 Subject: [PATCH] Split mongo support for libraries and Library XBlock --- common/djangoapps/xmodule_django/models.py | 4 +- common/lib/xmodule/setup.py | 5 +- .../xmodule/xmodule/library_root_xblock.py | 92 +++++++ .../xmodule/xmodule/modulestore/__init__.py | 1 + .../lib/xmodule/xmodule/modulestore/mixed.py | 65 +++++ .../xmodule/xmodule/modulestore/mongo/base.py | 4 +- .../split_mongo/caching_descriptor_system.py | 10 +- .../xmodule/modulestore/split_mongo/split.py | 258 ++++++++++++------ .../modulestore/split_mongo/split_draft.py | 25 +- .../xmodule/modulestore/tests/factories.py | 30 ++ .../modulestore/tests/test_libraries.py | 207 ++++++++++++++ .../tests/test_mixed_modulestore.py | 33 +-- .../xmodule/modulestore/tests/test_mongo.py | 11 + .../xmodule/modulestore/tests/utils.py | 110 ++++++++ requirements/edx/github.txt | 2 +- 15 files changed, 720 insertions(+), 137 deletions(-) create mode 100644 common/lib/xmodule/xmodule/library_root_xblock.py create mode 100644 common/lib/xmodule/xmodule/modulestore/tests/test_libraries.py create mode 100644 common/lib/xmodule/xmodule/modulestore/tests/utils.py diff --git a/common/djangoapps/xmodule_django/models.py b/common/djangoapps/xmodule_django/models.py index 5c522fa9a8..bcb287dc0b 100644 --- a/common/djangoapps/xmodule_django/models.py +++ b/common/djangoapps/xmodule_django/models.py @@ -68,7 +68,7 @@ def _strip_value(value, lookup='exact'): class CourseKeyField(models.CharField): - description = "A SlashSeparatedCourseKey object, saved to the DB in the form of a string" + description = "A CourseKey object, saved to the DB in the form of a string" __metaclass__ = models.SubfieldBase @@ -84,7 +84,7 @@ class CourseKeyField(models.CharField): return None if isinstance(value, basestring): - return SlashSeparatedCourseKey.from_deprecated_string(value) + return CourseKey.from_string(value) else: return value diff --git a/common/lib/xmodule/setup.py b/common/lib/xmodule/setup.py index 5182a8454c..f0721e91a4 100644 --- a/common/lib/xmodule/setup.py +++ b/common/lib/xmodule/setup.py @@ -44,6 +44,9 @@ XMODULES = [ "crowdsource_hinter = xmodule.crowdsource_hinter:CrowdsourceHinterDescriptor", "lti = xmodule.lti_module:LTIDescriptor", ] +XBLOCKS = [ + "library = xmodule.library_root_xblock:LibraryRoot", +] setup( name="XModule", @@ -64,7 +67,7 @@ setup( # See http://guide.python-distribute.org/creation.html#entry-points # for a description of entry_points entry_points={ - 'xblock.v1': XMODULES, + 'xblock.v1': XMODULES + XBLOCKS, 'xmodule.v1': XMODULES, 'console_scripts': [ 'xmodule_assets = xmodule.static_content:main', diff --git a/common/lib/xmodule/xmodule/library_root_xblock.py b/common/lib/xmodule/xmodule/library_root_xblock.py new file mode 100644 index 0000000000..dc00aaa97f --- /dev/null +++ b/common/lib/xmodule/xmodule/library_root_xblock.py @@ -0,0 +1,92 @@ +""" +'library' XBlock (LibraryRoot) +""" +import logging + +from .studio_editable import StudioEditableModule +from xblock.core import XBlock +from xblock.fields import Scope, String, List +from xblock.fragment import Fragment + +log = logging.getLogger(__name__) + +# Make '_' a no-op so we can scrape strings +_ = lambda text: text + + +class LibraryRoot(XBlock): + """ + The LibraryRoot is the root XBlock of a content library. All other blocks in + the library are its children. It contains metadata such as the library's + display_name. + """ + display_name = String( + help=_("Enter the name of the library as it should appear in Studio."), + default="Library", + display_name=_("Library Display Name"), + scope=Scope.settings + ) + advanced_modules = List( + display_name=_("Advanced Module List"), + help=_("Enter the names of the advanced components to use in your library."), + scope=Scope.settings + ) + has_children = True + has_author_view = True + + def __unicode__(self): + return u"Library: {}".format(self.display_name) + + def __str__(self): + return unicode(self).encode('utf-8') + + def author_view(self, context): + """ + Renders the Studio preview view, which supports drag and drop. + """ + fragment = Fragment() + contents = [] + + for child_key in self.children: # pylint: disable=E1101 + context['reorderable_items'].add(child_key) + child = self.runtime.get_block(child_key) + rendered_child = self.runtime.render_child(child, StudioEditableModule.get_preview_view_name(child), context) + fragment.add_frag_resources(rendered_child) + + contents.append({ + 'id': unicode(child_key), + 'content': rendered_child.content, + }) + + fragment.add_content(self.runtime.render_template("studio_render_children_view.html", { + 'items': contents, + 'xblock_context': context, + 'can_add': True, + 'can_reorder': True, + })) + return fragment + + @property + def display_org_with_default(self): + """ + Org display names are not implemented. This just provides API compatibility with CourseDescriptor. + Always returns the raw 'org' field from the key. + """ + return self.scope_ids.usage_id.course_key.org + + @property + def display_number_with_default(self): + """ + Display numbers are not implemented. This just provides API compatibility with CourseDescriptor. + Always returns the raw 'library' field from the key. + """ + return self.scope_ids.usage_id.course_key.library + + @classmethod + def parse_xml(cls, xml_data, system, id_generator, **kwargs): + """ XML support not yet implemented. """ + raise NotImplementedError + + def add_xml_to_node(self, resource_fs): + """ XML support not yet implemented. """ + raise NotImplementedError diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index 987df6eea2..085f3f7d5e 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -82,6 +82,7 @@ class ModuleStoreEnum(object): """ draft = 'draft-branch' published = 'published-branch' + library = 'library' class UserID(object): """ diff --git a/common/lib/xmodule/xmodule/modulestore/mixed.py b/common/lib/xmodule/xmodule/modulestore/mixed.py index b284c6bddd..7d76d31efd 100644 --- a/common/lib/xmodule/xmodule/modulestore/mixed.py +++ b/common/lib/xmodule/xmodule/modulestore/mixed.py @@ -13,6 +13,7 @@ from contracts import contract, new_contract from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey, AssetKey +from opaque_keys.edx.locator import LibraryLocator from opaque_keys.edx.locations import SlashSeparatedCourseKey from xmodule.assetstore import AssetMetadata @@ -25,6 +26,7 @@ from .split_migrator import SplitMigrator new_contract('CourseKey', CourseKey) new_contract('AssetKey', AssetKey) new_contract('AssetMetadata', AssetMetadata) +new_contract('LibraryLocator', LibraryLocator) log = logging.getLogger(__name__) @@ -259,6 +261,23 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase): courses[course_id] = course return courses.values() + @strip_key + def get_libraries(self, **kwargs): + """ + Returns a list containing the top level XBlock of the libraries (LibraryRoot) in this modulestore. + """ + libraries = {} + for store in self.modulestores: + if not hasattr(store, 'get_libraries'): + continue + # filter out ones which were fetched from earlier stores but locations may not be == + for course in store.get_libraries(**kwargs): + course_id = self._clean_course_id_for_mapping(course.location) + if course_id not in libraries: + # course is indeed unique. save it in result + libraries[course_id] = course + return libraries.values() + def make_course_key(self, org, course, run): """ Return a valid :class:`~opaque_keys.edx.keys.CourseKey` for this modulestore @@ -290,6 +309,24 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase): except ItemNotFoundError: return None + @strip_key + @contract(library_key='LibraryLocator') + def get_library(self, library_key, depth=0, **kwargs): + """ + returns the library block associated with the given key. If no such library exists, + it returns None + + :param library_key: must be a LibraryLocator + """ + try: + store = self._verify_modulestore_support(library_key, 'get_library') + return store.get_library(library_key, depth=depth, **kwargs) + except NotImplementedError: + log.exception("Modulestore configured for %s does not have get_library method", library_key) + return None + except ItemNotFoundError: + return None + @strip_key def has_course(self, course_id, ignore_case=False, **kwargs): """ @@ -507,6 +544,34 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase): return course + @strip_key + def create_library(self, org, library, user_id, fields, **kwargs): + """ + Creates and returns a new library. + + Args: + org (str): the organization that owns the course + library (str): the code/number/name of the library + user_id: id of the user creating the course + fields (dict): Fields to set on the course at initialization - e.g. display_name + kwargs: Any optional arguments understood by a subset of modulestores to customize instantiation + + Returns: a LibraryRoot + """ + # first make sure an existing course/lib doesn't already exist in the mapping + lib_key = LibraryLocator(org=org, library=library) + if lib_key in self.mappings: + raise DuplicateCourseError(lib_key, lib_key) + + # create the library + store = self._verify_modulestore_support(None, 'create_library') + library = store.create_library(org, library, user_id, fields, **kwargs) + + # add new library to the mapping + self.mappings[lib_key] = store + + return library + @strip_key def clone_course(self, source_course_id, dest_course_id, user_id, fields=None, **kwargs): """ diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index 9f5a09aee3..a9662b771f 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -33,7 +33,7 @@ from importlib import import_module from opaque_keys.edx.keys import UsageKey, CourseKey, AssetKey from opaque_keys.edx.locations import Location from opaque_keys.edx.locations import SlashSeparatedCourseKey -from opaque_keys.edx.locator import CourseLocator +from opaque_keys.edx.locator import CourseLocator, LibraryLocator from xblock.core import XBlock from xblock.exceptions import InvalidScopeError @@ -875,6 +875,8 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo otherwise, do a case sensitive search """ assert(isinstance(course_key, CourseKey)) + if isinstance(course_key, LibraryLocator): + return None # Libraries require split mongo course_key = self.fill_in_run(course_key) location = course_key.make_usage_key('course', course_key.run) if ignore_case: diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py index 73159bf221..7881be1c2f 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py @@ -4,7 +4,7 @@ from contracts import contract, new_contract from lazy import lazy from xblock.runtime import KvsFieldData from xblock.fields import ScopeIds -from opaque_keys.edx.locator import BlockUsageLocator, LocalId, CourseLocator, DefinitionLocator +from opaque_keys.edx.locator import BlockUsageLocator, LocalId, CourseLocator, LibraryLocator, DefinitionLocator from xmodule.mako_module import MakoDescriptorSystem from xmodule.error_module import ErrorDescriptor from xmodule.errortracker import exc_info_to_str @@ -19,6 +19,8 @@ from xmodule.modulestore.split_mongo import BlockKey, CourseEnvelope log = logging.getLogger(__name__) new_contract('BlockUsageLocator', BlockUsageLocator) +new_contract('CourseLocator', CourseLocator) +new_contract('LibraryLocator', LibraryLocator) new_contract('BlockKey', BlockKey) new_contract('CourseEnvelope', CourseEnvelope) @@ -115,7 +117,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin): self.modulestore.cache_block(course_key, version_guid, block_key, block) return block - @contract(block_key=BlockKey, course_key=CourseLocator) + @contract(block_key=BlockKey, course_key="CourseLocator | LibraryLocator") def get_module_data(self, block_key, course_key): """ Get block from module_data adding it to module_data if it's not already there but is in the structure @@ -178,8 +180,8 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin): if definition_id is None: definition_id = LocalId() - block_locator = BlockUsageLocator( - course_key, + # Construct the Block Usage Locator: + block_locator = course_key.make_usage_key( block_type=block_key.type, block_id=block_key.id, ) diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index f6fa21271f..d269806e3f 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -65,8 +65,7 @@ from xblock.core import XBlock from xblock.fields import Scope, Reference, ReferenceList, ReferenceValueDict from xmodule.errortracker import null_error_tracker from opaque_keys.edx.locator import ( - BlockUsageLocator, DefinitionLocator, CourseLocator, VersionTree, - LocalId, + BlockUsageLocator, DefinitionLocator, CourseLocator, LibraryLocator, VersionTree, LocalId, ) from xmodule.modulestore.exceptions import InsufficientSpecificationError, VersionConflictError, DuplicateItemError, \ DuplicateCourseError @@ -190,8 +189,8 @@ class SplitBulkWriteMixin(BulkOperationsMixin): if course_key is None: return self._bulk_ops_record_type() - if not isinstance(course_key, CourseLocator): - raise TypeError(u'{!r} is not a CourseLocator'.format(course_key)) + if not isinstance(course_key, (CourseLocator, LibraryLocator)): + raise TypeError(u'{!r} is not a CourseLocator or LibraryLocator'.format(course_key)) # handle version_guid based retrieval locally if course_key.org is None or course_key.course is None or course_key.run is None: return self._active_bulk_ops.records[ @@ -207,8 +206,8 @@ class SplitBulkWriteMixin(BulkOperationsMixin): """ Clear the record for this course """ - if not isinstance(course_key, CourseLocator): - raise TypeError('{!r} is not a CourseLocator'.format(course_key)) + if not isinstance(course_key, (CourseLocator, LibraryLocator)): + raise TypeError('{!r} is not a CourseLocator or LibraryLocator'.format(course_key)) if course_key.org and course_key.course and course_key.run: del self._active_bulk_ops.records[course_key.replace(branch=None, version_guid=None)] @@ -776,19 +775,10 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): # add it in the envelope for the structure. return CourseEnvelope(course_key.replace(version_guid=version_guid), entry) - @autoretry_read() - def get_courses(self, branch, **kwargs): - ''' - Returns a list of course descriptors matching any given qualifiers. - - qualifiers should be a dict of keywords matching the db fields or any - legal query for mongo to use against the active_versions collection. - - Note, this is to find the current head of the named branch type. - To get specific versions via guid use get_course. - - :param branch: the branch for which to return courses. - ''' + def _get_structures_for_branch(self, branch): + """ + Internal generator for fetching lists of courses, libraries, etc. + """ matching_indexes = self.find_matching_course_indexes(branch) # collect ids and then query for those @@ -800,29 +790,73 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): id_version_map[version_guid] = course_index if not version_guids: - return [] + return - matching_structures = self.find_structures_by_id(version_guids) + for entry in self.find_structures_by_id(version_guids): + yield entry, id_version_map[entry['_id']] - # get the blocks for each course index (s/b the root) + def _get_structures_for_branch_and_locator(self, branch, locator_factory, **kwargs): + + """ + Internal generator for fetching lists of courses, libraries, etc. + :param str branch: Branch to fetch structures from + :param type locator_factory: Factory to create locator from structure info and branch + """ result = [] - for entry in matching_structures: - course_info = id_version_map[entry['_id']] - envelope = CourseEnvelope( - CourseLocator( - org=course_info['org'], - course=course_info['course'], - run=course_info['run'], - branch=branch, - ), - entry - ) + for entry, structure_info in self._get_structures_for_branch(branch): + locator = locator_factory(structure_info, branch) + envelope = CourseEnvelope(locator, entry) root = entry['root'] - course_list = self._load_items(envelope, [root], 0, lazy=True, **kwargs) - if not isinstance(course_list[0], ErrorDescriptor): - result.append(course_list[0]) + structures_list = self._load_items(envelope, [root], 0, lazy=True, **kwargs) + if not isinstance(structures_list[0], ErrorDescriptor): + result.append(structures_list[0]) return result + def _create_course_locator(self, course_info, branch): + """ + Creates course locator using course_info dict and branch + """ + return CourseLocator( + org=course_info['org'], + course=course_info['course'], + run=course_info['run'], + branch=branch, + ) + + def _create_library_locator(self, library_info, branch): + """ + Creates library locator using library_info dict and branch + """ + return LibraryLocator( + org=library_info['org'], + library=library_info['course'], + branch=branch, + ) + + @autoretry_read() + def get_courses(self, branch, **kwargs): + """ + Returns a list of course descriptors matching any given qualifiers. + + qualifiers should be a dict of keywords matching the db fields or any + legal query for mongo to use against the active_versions collection. + + Note, this is to find the current head of the named branch type. + To get specific versions via guid use get_course. + + :param branch: the branch for which to return courses. + """ + # get the blocks for each course index (s/b the root) + return self._get_structures_for_branch_and_locator(branch, self._create_course_locator, **kwargs) + + def get_libraries(self, branch="library", **kwargs): + """ + Returns a list of "library" root blocks matching any given qualifiers. + + TODO: better way of identifying library index entry vs. course index entry. + """ + return self._get_structures_for_branch_and_locator(branch, self._create_library_locator, **kwargs) + def make_course_key(self, org, course, run): """ Return a valid :class:`~opaque_keys.edx.keys.CourseKey` for this modulestore @@ -832,6 +866,15 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): """ return CourseLocator(org, course, run) + def _get_structure(self, structure_id, depth, **kwargs): + """ + Gets Course or Library by locator + """ + structure_entry = self._lookup_course(structure_id) + root = structure_entry.structure['root'] + result = self._load_items(structure_entry, [root], depth, lazy=True, **kwargs) + return result[0] + def get_course(self, course_id, depth=0, **kwargs): ''' Gets the course descriptor for the course identified by the locator @@ -839,11 +882,16 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): if not isinstance(course_id, CourseLocator) or course_id.deprecated: # The supplied CourseKey is of the wrong type, so it can't possibly be stored in this modulestore. raise ItemNotFoundError(course_id) + return self._get_structure(course_id, depth, **kwargs) - course_entry = self._lookup_course(course_id) - root = course_entry.structure['root'] - result = self._load_items(course_entry, [root], depth, lazy=True, **kwargs) - return result[0] + def get_library(self, library_id, depth=0, **kwargs): + """ + Gets the 'library' root block for the library identified by the locator + """ + if not isinstance(library_id, LibraryLocator): + # The supplied CourseKey is of the wrong type, so it can't possibly be stored in this modulestore. + raise ItemNotFoundError(library_id) + return self._get_structure(library_id, depth, **kwargs) def has_course(self, course_id, ignore_case=False, **kwargs): ''' @@ -1227,20 +1275,28 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): new_def_data = self._serialize_fields(old_definition['block_type'], new_def_data) if needs_saved(): - # Do a deep copy so that we don't corrupt the cached version of the definition - new_definition = copy.deepcopy(old_definition) - new_definition['_id'] = ObjectId() - new_definition['fields'] = new_def_data - new_definition['edit_info']['edited_by'] = user_id - new_definition['edit_info']['edited_on'] = datetime.datetime.now(UTC) - # previous version id - new_definition['edit_info']['previous_version'] = definition_locator.definition_id - new_definition['schema_version'] = self.SCHEMA_VERSION - self.update_definition(course_key, new_definition) - return DefinitionLocator(new_definition['block_type'], new_definition['_id']), True + definition_locator = self._update_definition_from_data(course_key, old_definition, new_def_data, user_id) + return definition_locator, True else: return definition_locator, False + def _update_definition_from_data(self, course_key, old_definition, new_def_data, user_id): + """ + Update the persisted version of the given definition and return the + locator of the new definition. Does not check if data differs from the + previous version. + """ + new_definition = copy.deepcopy(old_definition) + new_definition['_id'] = ObjectId() + new_definition['fields'] = new_def_data + new_definition['edit_info']['edited_by'] = user_id + new_definition['edit_info']['edited_on'] = datetime.datetime.now(UTC) + # previous version id + new_definition['edit_info']['previous_version'] = old_definition['_id'] + new_definition['schema_version'] = self.SCHEMA_VERSION + self.update_definition(course_key, new_definition) + return DefinitionLocator(new_definition['block_type'], new_definition['_id']) + def _generate_block_key(self, course_blocks, category): """ Generate a somewhat readable block id unique w/in this course using the category @@ -1325,7 +1381,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): # persist the definition if persisted != passed if (definition_locator is None or isinstance(definition_locator.definition_id, LocalId)): definition_locator = self.create_definition_from_data(course_key, new_def_data, block_type, user_id) - elif new_def_data is not None: + elif new_def_data: definition_locator, _ = self.update_definition_from_data(course_key, definition_locator, new_def_data, user_id) # copy the structure and modify the new one @@ -1494,6 +1550,19 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): assert master_branch is not None # check course and run's uniqueness locator = CourseLocator(org=org, course=course, run=run, branch=master_branch) + return self._create_courselike( + locator, user_id, master_branch, fields, versions_dict, + search_targets, root_category, root_block_id, **kwargs + ) + + def _create_courselike( + self, locator, user_id, master_branch, fields=None, + versions_dict=None, search_targets=None, root_category='course', + root_block_id=None, **kwargs + ): + """ + Internal code for creating a course or library + """ index = self.get_course_index(locator) if index is not None: raise DuplicateCourseError(locator, index) @@ -1508,20 +1577,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): # if building a wholly new structure if versions_dict is None or master_branch not in versions_dict: # create new definition and structure - definition_id = ObjectId() - definition_entry = { - '_id': definition_id, - 'block_type': root_category, - 'fields': definition_fields, - 'edit_info': { - 'edited_by': user_id, - 'edited_on': datetime.datetime.now(UTC), - 'previous_version': None, - 'original_version': definition_id, - }, - 'schema_version': self.SCHEMA_VERSION, - } - self.update_definition(locator, definition_entry) + definition_id = self.create_definition_from_data(locator, definition_fields, root_category, user_id).definition_id draft_structure = self._new_structure( user_id, @@ -1549,19 +1605,17 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): if block_fields is not None: root_block['fields'].update(self._serialize_fields(root_category, block_fields)) if definition_fields is not None: - definition = copy.deepcopy(self.get_definition(locator, root_block['definition'])) - definition['fields'].update(definition_fields) - definition['edit_info']['previous_version'] = definition['_id'] - definition['edit_info']['edited_by'] = user_id - definition['edit_info']['edited_on'] = datetime.datetime.now(UTC) - definition['_id'] = ObjectId() - definition['schema_version'] = self.SCHEMA_VERSION - self.update_definition(locator, definition) - root_block['definition'] = definition['_id'] - root_block['edit_info']['edited_on'] = datetime.datetime.now(UTC) - root_block['edit_info']['edited_by'] = user_id - root_block['edit_info']['previous_version'] = root_block['edit_info'].get('update_version') - root_block['edit_info']['update_version'] = new_id + old_def = self.get_definition(locator, root_block['definition']) + new_fields = old_def['fields'] + new_fields.update(definition_fields) + definition_id = self._update_definition_from_data(locator, old_def, new_fields, user_id).definition_id + root_block['definition'] = definition_id + root_block['edit_info'].update({ + 'edited_on': datetime.datetime.now(UTC), + 'edited_by': user_id, + 'previous_version': root_block['edit_info'].get('update_version'), + 'update_version': new_id, + }) versions_dict[master_branch] = new_id else: # Pointing to an existing course structure @@ -1574,9 +1628,9 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): self.update_structure(locator, draft_structure) index_entry = { '_id': ObjectId(), - 'org': org, - 'course': course, - 'run': run, + 'org': locator.org, + 'course': locator.course, + 'run': locator.run, 'edited_by': user_id, 'edited_on': datetime.datetime.now(UTC), 'versions': versions_dict, @@ -1588,9 +1642,23 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): self.insert_course_index(locator, index_entry) # expensive hack to persist default field values set in __init__ method (e.g., wiki_slug) - course = self.get_course(locator, **kwargs) + if isinstance(locator, LibraryLocator): + course = self.get_library(locator, **kwargs) + else: + course = self.get_course(locator, **kwargs) return self.update_item(course, user_id, **kwargs) + def create_library(self, org, library, user_id, fields, **kwargs): + """ + Create a new library. Arguments are similar to create_course(). + """ + kwargs["fields"] = fields + kwargs["master_branch"] = kwargs.get("master_branch", ModuleStoreEnum.BranchName.library) + kwargs["root_category"] = kwargs.get("root_category", "library") + kwargs["root_block_id"] = kwargs.get("root_block_id", "library") + locator = LibraryLocator(org=org, library=library, branch=kwargs["master_branch"]) + return self._create_courselike(locator, user_id, **kwargs) + def update_item(self, descriptor, user_id, allow_not_found=False, force=False, **kwargs): """ Save the descriptor's fields. it doesn't descend the course dag to save the children. @@ -1680,14 +1748,24 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): if index_entry is not None: self._update_search_targets(index_entry, definition_fields) self._update_search_targets(index_entry, settings) - course_key = CourseLocator( - org=index_entry['org'], - course=index_entry['course'], - run=index_entry['run'], - branch=course_key.branch, - version_guid=new_id - ) + if isinstance(course_key, LibraryLocator): + course_key = LibraryLocator( + org=index_entry['org'], + library=index_entry['course'], + branch=course_key.branch, + version_guid=new_id + ) + else: + course_key = CourseLocator( + org=index_entry['org'], + course=index_entry['course'], + run=index_entry['run'], + branch=course_key.branch, + version_guid=new_id + ) self._update_head(course_key, index_entry, course_key.branch, new_id) + elif isinstance(course_key, LibraryLocator): + course_key = LibraryLocator(version_guid=new_id) else: course_key = CourseLocator(version_guid=new_id) diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py index 1ee64f3f05..d44d2c0baf 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py @@ -9,7 +9,7 @@ from xmodule.modulestore.exceptions import InsufficientSpecificationError from xmodule.modulestore.draft_and_published import ( ModuleStoreDraftAndPublished, DIRECT_ONLY_CATEGORIES, UnsupportedRevisionError ) -from opaque_keys.edx.locator import CourseLocator +from opaque_keys.edx.locator import CourseLocator, LibraryLocator, LibraryUsageLocator from xmodule.modulestore.split_mongo import BlockKey from contracts import contract @@ -57,6 +57,10 @@ class DraftVersioningModuleStore(SplitMongoModuleStore, ModuleStoreDraftAndPubli course_id = self._map_revision_to_branch(course_id) return super(DraftVersioningModuleStore, self).get_course(course_id, depth=depth, **kwargs) + def get_library(self, library_id, depth=0, **kwargs): + library_id = self._map_revision_to_branch(library_id) + return super(DraftVersioningModuleStore, self).get_library(library_id, depth=depth, **kwargs) + def clone_course(self, source_course_id, dest_course_id, user_id, fields=None, revision=None, **kwargs): """ See :py:meth: xmodule.modulestore.split_mongo.split.SplitMongoModuleStore.clone_course @@ -153,7 +157,9 @@ class DraftVersioningModuleStore(SplitMongoModuleStore, ModuleStoreDraftAndPubli Otherwise, raises a ValueError. """ with self.bulk_operations(location.course_key): - if revision == ModuleStoreEnum.RevisionOption.published_only: + if isinstance(location, LibraryUsageLocator): + branches_to_delete = [ModuleStoreEnum.BranchName.library] # Libraries don't yet have draft/publish support + elif revision == ModuleStoreEnum.RevisionOption.published_only: branches_to_delete = [ModuleStoreEnum.BranchName.published] elif revision == ModuleStoreEnum.RevisionOption.all: branches_to_delete = [ModuleStoreEnum.BranchName.published, ModuleStoreEnum.BranchName.draft] @@ -180,18 +186,25 @@ class DraftVersioningModuleStore(SplitMongoModuleStore, ModuleStoreDraftAndPubli """ Maps RevisionOptions to BranchNames, inserting them into the key """ + if isinstance(key, (LibraryLocator, LibraryUsageLocator)): + # Libraries don't yet have draft/publish support: + draft_branch = ModuleStoreEnum.BranchName.library + published_branch = ModuleStoreEnum.BranchName.library + else: + draft_branch = ModuleStoreEnum.BranchName.draft + published_branch = ModuleStoreEnum.BranchName.published if revision == ModuleStoreEnum.RevisionOption.published_only: - return key.for_branch(ModuleStoreEnum.BranchName.published) + return key.for_branch(published_branch) elif revision == ModuleStoreEnum.RevisionOption.draft_only: - return key.for_branch(ModuleStoreEnum.BranchName.draft) + return key.for_branch(draft_branch) elif revision is None: if key.branch is not None: return key elif self.get_branch_setting(key) == ModuleStoreEnum.Branch.draft_preferred: - return key.for_branch(ModuleStoreEnum.BranchName.draft) + return key.for_branch(draft_branch) else: - return key.for_branch(ModuleStoreEnum.BranchName.published) + return key.for_branch(published_branch) else: raise UnsupportedRevisionError() diff --git a/common/lib/xmodule/xmodule/modulestore/tests/factories.py b/common/lib/xmodule/xmodule/modulestore/tests/factories.py index cdbf312d41..f49f2d2b11 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/factories.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/factories.py @@ -78,6 +78,36 @@ class CourseFactory(XModuleFactory): return new_course +class LibraryFactory(XModuleFactory): + """ + Factory for creating a content library + """ + org = factory.Sequence('org{}'.format) + library = factory.Sequence('lib{}'.format) + display_name = factory.Sequence('Test Library {}'.format) + + # pylint: disable=unused-argument + @classmethod + def _create(cls, target_class, **kwargs): + """ + Create a library with a unique name and key. + All class attributes (from this class and base classes) are automagically + passed in via **kwargs. + """ + # some of the kwargst actual field values, so pop those off for use separately: + org = kwargs.pop('org') + library = kwargs.pop('library') + store = kwargs.pop('modulestore') + user_id = kwargs.pop('user_id', ModuleStoreEnum.UserID.test) + + # Pass the metadata just as field=value pairs + kwargs.update(kwargs.pop('metadata', {})) + default_store_override = kwargs.pop('default_store', ModuleStoreEnum.Type.split) + with store.default_store(default_store_override): + new_library = store.create_library(org, library, user_id, fields=kwargs) + return new_library + + class ItemFactory(XModuleFactory): """ Factory for XModule items. diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_libraries.py b/common/lib/xmodule/xmodule/modulestore/tests/test_libraries.py new file mode 100644 index 0000000000..c380da1e8b --- /dev/null +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_libraries.py @@ -0,0 +1,207 @@ +# -*- coding: utf-8 -*- +""" +Basic unit tests related to content libraries. + +Higher-level tests are in `cms/djangoapps/contentstore`. +""" +from bson.objectid import ObjectId +import ddt +from mock import patch +from opaque_keys.edx.locator import LibraryLocator +from xblock.fragment import Fragment +from xblock.runtime import Runtime as VanillaRuntime +from xmodule.modulestore.exceptions import DuplicateCourseError +from xmodule.modulestore.tests.factories import LibraryFactory, ItemFactory, check_mongo_calls +from xmodule.modulestore.tests.utils import MixedSplitTestCase +from xmodule.x_module import AUTHOR_VIEW + + +@ddt.ddt +class TestLibraries(MixedSplitTestCase): + """ + Test for libraries. + Mostly tests code found throughout split mongo, but also tests library_root_xblock.py + """ + def test_create_library(self): + """ + Test that we can create a library, and see how many mongo calls it uses to do so. + + Expected mongo calls, in order: + find_one({'org': '...', 'run': 'library', 'course': '...'}) + insert(definition: {'block_type': 'library', 'fields': {}}) + + insert_structure(bulk) + insert_course_index(bulk) + get_course_index(bulk) + """ + with check_mongo_calls(2, 3): + LibraryFactory.create(modulestore=self.store) + + def test_duplicate_library(self): + """ + Make sure we cannot create duplicate libraries + """ + org, lib_code = ('DuplicateX', "DUP") + LibraryFactory.create(org=org, library=lib_code, modulestore=self.store) + with self.assertRaises(DuplicateCourseError): + LibraryFactory.create(org=org, library=lib_code, modulestore=self.store) + + @ddt.data( + "This is a test library!", + u"Ωμέγα Βιβλιοθήκη", + ) + def test_str_repr(self, name): + """ + Test __unicode__() and __str__() methods of libraries + """ + library = LibraryFactory.create(metadata={"display_name": name}, modulestore=self.store) + self.assertIn(name, unicode(library)) + if not isinstance(name, unicode): + self.assertIn(name, str(library)) + + def test_display_with_default_methods(self): + """ + Check that the display_x_with_default methods have been implemented, for + compatibility with courses. + """ + org = 'TestOrgX' + lib_code = 'LC101' + library = LibraryFactory.create(org=org, library=lib_code, modulestore=self.store) + self.assertEqual(library.display_org_with_default, org) + self.assertEqual(library.display_number_with_default, lib_code) + + def test_block_with_children(self): + """ + Test that blocks used from a library can have children. + """ + library = LibraryFactory.create(modulestore=self.store) + + # In the library, create a vertical block with a child: + vert_block = ItemFactory.create( + category="vertical", + parent_location=library.location, + user_id=self.user_id, + publish_item=False, + modulestore=self.store, + ) + child_block = ItemFactory.create( + category="html", + parent_location=vert_block.location, + user_id=self.user_id, + publish_item=False, + metadata={"data": "Hello world", }, + modulestore=self.store, + ) + self.assertEqual(child_block.parent.replace(version_guid=None, branch=None), vert_block.location) + + def test_update_item(self): + """ + Test that update_item works for a block in a library + """ + library = LibraryFactory.create(modulestore=self.store) + + block = ItemFactory.create( + category="html", + parent_location=library.location, + user_id=self.user_id, + publish_item=False, + metadata={"data": "Hello world", }, + modulestore=self.store, + ) + block_key = block.location + block.data = "NEW" + old_version = self.store.get_item(block_key, remove_version=False, remove_branch=False).location.version_guid + self.store.update_item(block, self.user_id) + # Reload block from the modulestore + block = self.store.get_item(block_key) + self.assertEqual(block.data, "NEW") + self.assertEqual(block.location, block_key) + new_version = self.store.get_item(block_key, remove_version=False, remove_branch=False).location.version_guid + self.assertNotEqual(old_version, new_version) + + def test_delete_item(self): + """ + Test to make sure delete_item() works on blocks in a library + """ + library = LibraryFactory.create(modulestore=self.store) + lib_key = library.location.library_key + block = ItemFactory.create( + category="html", + parent_location=library.location, + user_id=self.user_id, + publish_item=False, + modulestore=self.store, + ) + library = self.store.get_library(lib_key) + self.assertEqual(len(library.children), 1) + self.store.delete_item(block.location, self.user_id) + library = self.store.get_library(lib_key) + self.assertEqual(len(library.children), 0) + + def test_get_library_non_existent(self): + """ Test get_library() with non-existent key """ + result = self.store.get_library(LibraryLocator("non", "existent")) + self.assertEqual(result, None) + + def test_get_libraries(self): + """ Test get_libraries() """ + libraries = [LibraryFactory.create(modulestore=self.store) for _ in range(0, 3)] + lib_dict = dict([(lib.location.library_key, lib) for lib in libraries]) + + lib_list = self.store.get_libraries() + + self.assertEqual(len(lib_list), len(libraries)) + for lib in lib_list: + self.assertIn(lib.location.library_key, lib_dict) + + def test_strip(self): + """ + Test that library keys coming out of MixedModuleStore are stripped of + branch and version info by default. + """ + # Create a library + lib_key = LibraryFactory.create(modulestore=self.store).location.library_key + # Re-load the library from the modulestore, explicitly including version information: + lib = self.store.get_library(lib_key) + self.assertEqual(lib.location.version_guid, None) + self.assertEqual(lib.location.branch, None) + self.assertEqual(lib.location.library_key.version_guid, None) + self.assertEqual(lib.location.library_key.branch, None) + + def test_get_lib_version(self): + """ + Test that we can get version data about a library from get_library() + """ + # Create a library + lib_key = LibraryFactory.create(modulestore=self.store).location.library_key + # Re-load the library from the modulestore, explicitly including version information: + lib = self.store.get_library(lib_key, remove_version=False, remove_branch=False) + version = lib.location.library_key.version_guid + self.assertIsInstance(version, ObjectId) + + @patch('xmodule.modulestore.split_mongo.caching_descriptor_system.CachingDescriptorSystem.render', VanillaRuntime.render) + def test_library_author_view(self): + """ + Test that LibraryRoot.author_view can run and includes content from its + children. + We have to patch the runtime (module system) in order to be able to + render blocks in our test environment. + """ + library = LibraryFactory.create(modulestore=self.store) + # Add one HTML block to the library: + ItemFactory.create( + category="html", + parent_location=library.location, + user_id=self.user_id, + publish_item=False, + modulestore=self.store, + ) + library = self.store.get_library(library.location.library_key) + + context = {'reorderable_items': set(), } + # Patch the HTML block to always render "Hello world" + message = u"Hello world" + hello_render = lambda _, context: Fragment(message) + with patch('xmodule.html_module.HtmlDescriptor.author_view', hello_render, create=True): + result = library.render(AUTHOR_VIEW, context) + self.assertIn(message, result.content) 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 19e8664955..2c534b9460 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -39,6 +39,7 @@ from xmodule.modulestore.mixed import MixedModuleStore from xmodule.modulestore.search import path_to_location from xmodule.modulestore.tests.factories import check_mongo_calls, check_exact_number_of_calls, \ mongo_uses_error_check +from xmodule.modulestore.tests.utils import create_modulestore_instance from xmodule.modulestore.tests.mongo_connection import MONGO_PORT_NUM, MONGO_HOST from xmodule.tests import DATA_DIR, CourseComparisonTest @@ -1967,35 +1968,3 @@ class TestMixedModuleStore(CourseComparisonTest): with self.store.branch_setting(ModuleStoreEnum.Branch.published_only, course_id): published_vertical = self.store.get_item(vertical_loc) self.assertEqual(draft_vertical.display_name, published_vertical.display_name) - -# ============================================================================================================ -# General utils for not using django settings -# ============================================================================================================ - - -def load_function(path): - """ - Load a function by name. - - path is a string of the form "path.to.module.function" - returns the imported python object `function` from `path.to.module` - """ - module_path, _, name = path.rpartition('.') - return getattr(import_module(module_path), name) - - -# pylint: disable=unused-argument -def create_modulestore_instance(engine, contentstore, doc_store_config, options, i18n_service=None, fs_service=None): - """ - This will return a new instance of a modulestore given an engine and options - """ - class_ = load_function(engine) - - if issubclass(class_, ModuleStoreDraftAndPublished): - options['branch_setting_func'] = lambda: ModuleStoreEnum.Branch.draft_preferred - - return class_( - doc_store_config=doc_store_config, - contentstore=contentstore, - **options - ) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py index b08031252a..270114a25f 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py @@ -29,6 +29,7 @@ from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.mongo import MongoKeyValueStore from xmodule.modulestore.draft import DraftModuleStore from opaque_keys.edx.locations import SlashSeparatedCourseKey, AssetLocation +from opaque_keys.edx.locator import LibraryLocator from opaque_keys.edx.keys import UsageKey from xmodule.modulestore.xml_exporter import export_to_xml from xmodule.modulestore.xml_importer import import_from_xml, perform_xlint @@ -236,6 +237,16 @@ class TestMongoModuleStore(TestMongoModuleStoreBase): assert_false(self.draft_store.has_course(mix_cased)) assert_false(self.draft_store.has_course(mix_cased, ignore_case=True)) + def test_has_course_with_library(self): + """ + Test that has_course() returns False when called with a LibraryLocator. + This is required because MixedModuleStore will use has_course() to check + where a given library are stored. + """ + lib_key = LibraryLocator("TestOrg", "TestLib") + result = self.draft_store.has_course(lib_key) + assert_false(result) + def test_loads(self): assert_not_none( self.draft_store.get_item(Location('edX', 'toy', '2012_Fall', 'course', '2012_Fall')) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/utils.py b/common/lib/xmodule/xmodule/modulestore/tests/utils.py new file mode 100644 index 0000000000..76c379ec0b --- /dev/null +++ b/common/lib/xmodule/xmodule/modulestore/tests/utils.py @@ -0,0 +1,110 @@ +""" +Helper classes and methods for running modulestore tests without Django. +""" +from importlib import import_module +from opaque_keys.edx.keys import UsageKey +from unittest import TestCase +from xblock.fields import XBlockMixin +from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.draft_and_published import ModuleStoreDraftAndPublished +from xmodule.modulestore.edit_info import EditInfoMixin +from xmodule.modulestore.inheritance import InheritanceMixin +from xmodule.modulestore.mixed import MixedModuleStore +from xmodule.modulestore.tests.mongo_connection import MONGO_PORT_NUM, MONGO_HOST +from xmodule.tests import DATA_DIR + + +def load_function(path): + """ + Load a function by name. + + path is a string of the form "path.to.module.function" + returns the imported python object `function` from `path.to.module` + """ + module_path, _, name = path.rpartition('.') + return getattr(import_module(module_path), name) + + +# pylint: disable=unused-argument +def create_modulestore_instance(engine, contentstore, doc_store_config, options, i18n_service=None, fs_service=None): + """ + This will return a new instance of a modulestore given an engine and options + """ + class_ = load_function(engine) + + if issubclass(class_, ModuleStoreDraftAndPublished): + options['branch_setting_func'] = lambda: ModuleStoreEnum.Branch.draft_preferred + + return class_( + doc_store_config=doc_store_config, + contentstore=contentstore, + **options + ) + + +class LocationMixin(XBlockMixin): + """ + Adds a `location` property to an :class:`XBlock` so it is more compatible + with old-style :class:`XModule` API. This is a simplified version of + :class:`XModuleMixin`. + """ + @property + def location(self): + """ Get the UsageKey of this block. """ + return self.scope_ids.usage_id + + @location.setter + def location(self, value): + """ Set the UsageKey of this block. """ + assert isinstance(value, UsageKey) + self.scope_ids = self.scope_ids._replace( # pylint: disable=attribute-defined-outside-init,protected-access + def_id=value, + usage_id=value, + ) + + +class MixedSplitTestCase(TestCase): + """ + Stripped-down version of ModuleStoreTestCase that can be used without Django + (i.e. for testing in common/lib/ ). Sets up MixedModuleStore and Split. + """ + RENDER_TEMPLATE = lambda t_n, d, ctx = None, nsp = 'main': u'{}: {}, {}'.format(t_n, repr(d), repr(ctx)) + modulestore_options = { + 'default_class': 'xmodule.raw_module.RawDescriptor', + 'fs_root': DATA_DIR, + 'render_template': RENDER_TEMPLATE, + 'xblock_mixins': (EditInfoMixin, InheritanceMixin, LocationMixin), + } + DOC_STORE_CONFIG = { + 'host': MONGO_HOST, + 'port': MONGO_PORT_NUM, + 'db': 'test_mongo_libs', + 'collection': 'modulestore', + 'asset_collection': 'assetstore', + } + MIXED_OPTIONS = { + 'stores': [ + { + 'NAME': 'split', + 'ENGINE': 'xmodule.modulestore.split_mongo.split_draft.DraftVersioningModuleStore', + 'DOC_STORE_CONFIG': DOC_STORE_CONFIG, + 'OPTIONS': modulestore_options + }, + ] + } + + def setUp(self): + """ + Set up requirements for testing: a user ID and a modulestore + """ + super(MixedSplitTestCase, self).setUp() + self.user_id = ModuleStoreEnum.UserID.test + + self.store = MixedModuleStore( + None, + create_modulestore_instance=create_modulestore_instance, + mappings={}, + **self.MIXED_OPTIONS + ) + self.addCleanup(self.store.close_all_connections) + self.addCleanup(self.store._drop_database) # pylint: disable=protected-access diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index 75a5baba56..8b11461739 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -30,7 +30,7 @@ -e git+https://github.com/edx-solutions/django-splash.git@7579d052afcf474ece1239153cffe1c89935bc4f#egg=django-splash -e git+https://github.com/edx/acid-block.git@df1a7f0cae46567c251d507b8c72168aed8ec042#egg=acid-xblock -e git+https://github.com/edx/edx-ora2.git@release-2014-10-27T19.33#egg=edx-ora2 --e git+https://github.com/edx/opaque-keys.git@0.1.2#egg=opaque-keys +-e git+https://github.com/edx/opaque-keys.git@b12401384921c075e5a4ed7aedc3bea57f56ec32#egg=opaque-keys -e git+https://github.com/edx/ease.git@97de68448e5495385ba043d3091f570a699d5b5f#egg=ease -e git+https://github.com/edx/i18n-tools.git@56f048af9b6868613c14aeae760548834c495011#egg=i18n-tools -e git+https://github.com/edx/edx-oauth2-provider.git@0.4.0#egg=oauth2-provider