diff --git a/cms/djangoapps/contentstore/course_info_model.py b/cms/djangoapps/contentstore/course_info_model.py index 4d0006d10c..0ff15e94bb 100644 --- a/cms/djangoapps/contentstore/course_info_model.py +++ b/cms/djangoapps/contentstore/course_info_model.py @@ -36,7 +36,7 @@ def get_course_updates(location, provided_id, user_id): try: course_updates = modulestore().get_item(location) except ItemNotFoundError: - course_updates = modulestore().create_and_save_xmodule(location, user_id) + course_updates = modulestore().create_item(user_id, location) course_update_items = get_course_update_items(course_updates, provided_id) return _get_visible_update(course_update_items) @@ -51,7 +51,7 @@ def update_course_updates(location, update, passed_id=None, user=None): try: course_updates = modulestore().get_item(location) except ItemNotFoundError: - course_updates = modulestore().create_and_save_xmodule(location, user.id) + course_updates = modulestore().create_item(user.id, location) course_update_items = list(reversed(get_course_update_items(course_updates))) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 4321166e08..74b7dddb88 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -575,7 +575,7 @@ class ContentStoreToyCourseTest(ContentStoreTestCase): location = course.id.make_usage_key('chapter', 'neuvo') # Ensure draft mongo store does not create drafts for things that shouldn't be draft - newobject = draft_store.create_and_save_xmodule(location, self.user.id) + newobject = draft_store.create_item(self.user.id, location) self.assertFalse(getattr(newobject, 'is_draft', False)) with self.assertRaises(InvalidVersionError): draft_store.convert_to_draft(location, self.user.id) @@ -1395,7 +1395,7 @@ class ContentStoreTest(ContentStoreTestCase): new_component_location = course.id.make_usage_key('discussion', 'new_component') # crate a new module and add it as a child to a vertical - self.store.create_and_save_xmodule(new_component_location, self.user.id) + self.store.create_item(self.user.id, new_component_location) new_discussion_item = self.store.get_item(new_component_location) diff --git a/cms/djangoapps/contentstore/tests/test_crud.py b/cms/djangoapps/contentstore/tests/test_crud.py index 34e578bdb7..f49cdb7f79 100644 --- a/cms/djangoapps/contentstore/tests/test_crud.py +++ b/cms/djangoapps/contentstore/tests/test_crud.py @@ -191,12 +191,12 @@ class TemplateTests(unittest.TestCase): self.assertIsNotNone(updated_problem.previous_version) self.assertEqual(updated_problem.previous_version, first_problem.update_version) self.assertNotEqual(updated_problem.update_version, first_problem.update_version) - updated_loc = self.split_store.delete_item(updated_problem.location, ModuleStoreEnum.UserID.test, 'testbot') + self.split_store.delete_item(updated_problem.location, ModuleStoreEnum.UserID.test, 'testbot') second_problem = persistent_factories.ItemFactory.create( display_name='problem 2', parent_location=BlockUsageLocator.make_relative( - updated_loc, block_type='problem', block_id=sub.location.block_id + test_course.location.version_agnostic(), block_type='problem', block_id=sub.location.block_id ), user_id='testbot', category='problem', data="" diff --git a/cms/djangoapps/contentstore/tests/test_orphan.py b/cms/djangoapps/contentstore/tests/test_orphan.py index 3b080aa080..be9916235a 100644 --- a/cms/djangoapps/contentstore/tests/test_orphan.py +++ b/cms/djangoapps/contentstore/tests/test_orphan.py @@ -33,8 +33,8 @@ class TestOrphan(CourseTestCase): def _create_item(self, category, name, data, metadata, parent_category, parent_name, runtime): location = self.course.location.replace(category=category, name=name) store = modulestore() - store.create_and_save_xmodule( - location, self.user.id, definition_data=data, metadata=metadata, runtime=runtime + store.create_item( + self.user.id, location, definition_data=data, metadata=metadata, runtime=runtime ) if parent_name: # add child to parent in mongo diff --git a/cms/djangoapps/contentstore/tests/utils.py b/cms/djangoapps/contentstore/tests/utils.py index bd029fe99b..9e04ddbe71 100644 --- a/cms/djangoapps/contentstore/tests/utils.py +++ b/cms/djangoapps/contentstore/tests/utils.py @@ -151,11 +151,11 @@ class CourseTestCase(ModuleStoreTestCase): self.assertEqual(self.store.compute_publish_state(draft_vertical), PublishState.draft) # create a Private (draft only) vertical - private_vertical = self.store.create_and_save_xmodule(course_id.make_usage_key('vertical', self.PRIVATE_VERTICAL), self.user.id) + private_vertical = self.store.create_item(self.user.id, course_id.make_usage_key('vertical', self.PRIVATE_VERTICAL)) self.assertEqual(self.store.compute_publish_state(private_vertical), PublishState.private) # create a Published (no draft) vertical - public_vertical = self.store.create_and_save_xmodule(course_id.make_usage_key('vertical', self.PUBLISHED_VERTICAL), self.user.id) + public_vertical = self.store.create_item(self.user.id, course_id.make_usage_key('vertical', self.PUBLISHED_VERTICAL)) public_vertical = self.store.publish(public_vertical.location, self.user.id) self.assertEqual(self.store.compute_publish_state(public_vertical), PublishState.public) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index e4f300e325..00f49f48bf 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -287,7 +287,7 @@ def _save_item(user, usage_key, data=None, children=None, metadata=None, nullout if usage_key.category in CREATE_IF_NOT_FOUND: # New module at this location, for pages that are not pre-created. # Used for course info handouts. - existing_item = store.create_and_save_xmodule(usage_key, user.id) + existing_item = store.create_item(user.id, usage_key) else: raise except InvalidLocationError: @@ -416,9 +416,9 @@ def _create_item(request): if display_name is not None: metadata['display_name'] = display_name - created_block = store.create_and_save_xmodule( - dest_usage_key, + created_block = store.create_item( request.user.id, + dest_usage_key, definition_data=data, metadata=metadata, runtime=parent.runtime, @@ -465,11 +465,9 @@ def _duplicate_item(parent_usage_key, duplicate_source_usage_key, user, display_ else: duplicate_metadata['display_name'] = _("Duplicate of '{0}'").format(source_item.display_name) - dest_module = store.create_and_save_xmodule( - dest_usage_key, - - + dest_module = store.create_item( user.id, + dest_usage_key, definition_data=source_item.get_explicitly_set_fields_by_scope(Scope.content), metadata=duplicate_metadata, runtime=source_item.runtime, @@ -539,10 +537,8 @@ def orphan_handler(request, course_key_string): store = modulestore() items = store.get_orphans(course_usage_key) for itemloc in items: - # get_orphans returns the deprecated string format w/o revision - usage_key = course_usage_key.make_usage_key_from_deprecated_string(itemloc) # need to delete all versions - store.delete_item(usage_key, request.user.id, revision=ModuleStoreEnum.RevisionOption.all) + store.delete_item(itemloc, request.user.id, revision=ModuleStoreEnum.RevisionOption.all) return JsonResponse({'deleted': items}) else: raise PermissionDenied() @@ -559,7 +555,7 @@ def _get_module_info(usage_key, user, rewrite_static_links=True): except ItemNotFoundError: if usage_key.category in CREATE_IF_NOT_FOUND: # Create a new one for certain categories only. Used for course info handouts. - module = store.create_and_save_xmodule(usage_key, user.id) + module = store.create_item(user.id, usage_key) else: raise diff --git a/cms/djangoapps/contentstore/views/tests/test_course_updates.py b/cms/djangoapps/contentstore/views/tests/test_course_updates.py index dd345426f4..4558ab5733 100644 --- a/cms/djangoapps/contentstore/views/tests/test_course_updates.py +++ b/cms/djangoapps/contentstore/views/tests/test_course_updates.py @@ -130,7 +130,7 @@ class CourseUpdateTest(CourseTestCase): ''' # get the updates and populate 'data' field with some data. location = self.course.id.make_usage_key('course_info', 'updates') - course_updates = modulestore().create_and_save_xmodule(location, self.user.id) + course_updates = modulestore().create_item(self.user.id, location) update_date = u"January 23, 2014" update_content = u"Hello world!" update_data = u"

  1. " + update_date + "

    " + update_content + "
" @@ -204,7 +204,7 @@ class CourseUpdateTest(CourseTestCase): '''Test trying to add to a saved course_update which is not an ol.''' # get the updates and set to something wrong location = self.course.id.make_usage_key('course_info', 'updates') - modulestore().create_and_save_xmodule(location, self.user.id) + modulestore().create_item(self.user.id, location) course_updates = modulestore().get_item(location) course_updates.data = 'bad news' modulestore().update_item(course_updates, self.user.id) diff --git a/cms/envs/test.py b/cms/envs/test.py index 46239478b6..068bd39c43 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -63,7 +63,7 @@ STATICFILES_DIRS += [ MODULESTORE['default']['OPTIONS']['stores'].append( { 'NAME': 'split', - 'ENGINE': 'xmodule.modulestore.split_mongo.split.SplitMongoModuleStore', + 'ENGINE': 'xmodule.modulestore.split_mongo.split_draft.DraftVersioningModuleStore', 'DOC_STORE_CONFIG': DOC_STORE_CONFIG, 'OPTIONS': { 'render_template': 'edxmako.shortcuts.render_to_string', diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index 574fa5fb64..220fca229a 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -7,6 +7,7 @@ import logging import re import json import datetime +from uuid import uuid4 from collections import namedtuple, defaultdict import collections @@ -560,6 +561,25 @@ class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite): result[field.scope][field_name] = value return result + def create_item(self, user_id, location, parent_location=None, category=None, **kwargs): + """ + Creates and saves a new item. + Either location or (category, parent_location) or both must be provided. + If parent_location is provided, a new item of the given category is added as a child. + If location is not provided, a new item with the given category and given block_id + is added to the parent_location. If the block_id is not provided, a unique name + is automatically generated. + + Returns the newly created item. + + :param user_id: ID of the user creating and saving the xmodule + :param location: a Location--must have a category + :param parent_location: optional parameter, specifying the Location of the parent item + :param category: optional parameter for the category of the new item + :param block_id: a unique identifier for the new item + """ + raise NotImplementedError + def update_item(self, xblock, user_id, allow_not_found=False, force=False): """ Update the given xblock's persisted repr. Pass the user's unique id which the persistent store @@ -589,21 +609,6 @@ class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite): """ raise NotImplementedError - def create_and_save_xmodule(self, location, user_id, definition_data=None, metadata=None, runtime=None, fields={}): - """ - Create the new xmodule and save it. - - :param location: a Location--must have a category - :param user_id: ID of the user creating and saving the xmodule - :param definition_data: can be empty. The initial definition_data for the kvs - :param metadata: can be empty, the initial metadata for the kvs - :param runtime: if you already have an xblock from the course, the xblock.runtime value - :param fields: a dictionary of field names and values for the new xmodule - """ - new_object = self.create_xmodule(location, definition_data, metadata, runtime, fields) - self.update_item(new_object, user_id, allow_not_found=True) - return new_object - def clone_course(self, source_course_id, dest_course_id, user_id): """ This base method just copies the assets. The lower level impls must do the actual cloning of @@ -694,3 +699,18 @@ class EdxJSONEncoder(json.JSONEncoder): return obj.isoformat() else: return super(EdxJSONEncoder, self).default(obj) + + +def compute_location_from_args(location=None, parent_location=None, **kwargs): + """ + Returns a Location object that is generated from the given arguments, as follows: + If location is provided, returns it. + If location is not provided, returns a location with the given category and given block_id. + If the block_id is not provided, a unique name is automatically generated. + """ + if location is None: + block_id = kwargs.get('block_id', kwargs.get('name', kwargs.get('display_name', uuid4().hex))) + category = kwargs.pop('category') + course_key = getattr(parent_location, 'course_key', parent_location) + location = course_key.make_usage_key(category, Location.clean(block_id)) + return location diff --git a/common/lib/xmodule/xmodule/modulestore/draft_and_published.py b/common/lib/xmodule/xmodule/modulestore/draft_and_published.py new file mode 100644 index 0000000000..77a245bc75 --- /dev/null +++ b/common/lib/xmodule/xmodule/modulestore/draft_and_published.py @@ -0,0 +1,49 @@ +""" +This module provides an abstraction for Module Stores that support Draft and Published branches. +""" + +from abc import ABCMeta, abstractmethod +from . import ModuleStoreEnum + +class ModuleStoreDraftAndPublished(object): + """ + A mixin for a read-write database backend that supports two branches, Draft and Published, with + options to prefer Draft and fallback to Published. + """ + __metaclass__ = ABCMeta + + def __init__(self, **kwargs): + super(ModuleStoreDraftAndPublished, self).__init__(**kwargs) + self.branch_setting_func = kwargs.pop('branch_setting_func', lambda: ModuleStoreEnum.Branch.published_only) + + @abstractmethod + def delete_item(self, location, user_id, revision=None, **kwargs): + raise NotImplementedError + + @abstractmethod + def get_parent_location(self, location, revision=None, **kwargs): + raise NotImplementedError + + @abstractmethod + def has_changes(self, usage_key): + raise NotImplementedError + + @abstractmethod + def publish(self, location, user_id): + raise NotImplementedError + + @abstractmethod + def unpublish(self, location, user_id): + raise NotImplementedError + + @abstractmethod + def revert_to_published(self, location, user_id): + raise NotImplementedError + + @abstractmethod + def compute_publish_state(self, xblock): + raise NotImplementedError + + @abstractmethod + def convert_to_draft(self, location, user_id): + raise NotImplementedError diff --git a/common/lib/xmodule/xmodule/modulestore/mixed.py b/common/lib/xmodule/xmodule/modulestore/mixed.py index 34a19dd2c0..3cbf893463 100644 --- a/common/lib/xmodule/xmodule/modulestore/mixed.py +++ b/common/lib/xmodule/xmodule/modulestore/mixed.py @@ -6,17 +6,13 @@ In this way, courses can be served up both - say - XMLModuleStore or MongoModule """ import logging -from uuid import uuid4 from contextlib import contextmanager from opaque_keys import InvalidKeyError from . import ModuleStoreWriteBase from xmodule.modulestore import ModuleStoreEnum -from opaque_keys.edx.locator import CourseLocator, BlockUsageLocator from xmodule.modulestore.exceptions import ItemNotFoundError -from opaque_keys.edx.keys import CourseKey, UsageKey -from xmodule.modulestore.mongo.base import MongoModuleStore -from xmodule.modulestore.split_mongo.split import SplitMongoModuleStore +from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locations import SlashSeparatedCourseKey import itertools from xmodule.modulestore.split_migrator import SplitMigrator @@ -172,7 +168,7 @@ class MixedModuleStore(ModuleStoreWriteBase): raise Exception("Must pass in a course_key when calling get_items()") store = self._get_modulestore_for_courseid(course_key) - return store.get_items(course_key, settings, content, **kwargs) + return store.get_items(course_key, settings=settings, content=content, **kwargs) def get_courses(self): ''' @@ -272,7 +268,7 @@ class MixedModuleStore(ModuleStoreWriteBase): errs.update(store.get_errored_courses()) return errs - def create_course(self, org, course, run, user_id, fields=None, **kwargs): + def create_course(self, org, course, run, user_id, **kwargs): """ Creates and returns the course. @@ -287,10 +283,7 @@ class MixedModuleStore(ModuleStoreWriteBase): Returns: a CourseDescriptor """ store = self._get_modulestore_for_courseid(None) - if not hasattr(store, 'create_course'): - raise NotImplementedError(u"Cannot create a course on store {}".format(store)) - - return store.create_course(org, course, run, user_id, fields, **kwargs) + return store.create_course(org, course, run, user_id, **kwargs) def clone_course(self, source_course_id, dest_course_id, user_id): """ @@ -319,49 +312,26 @@ class MixedModuleStore(ModuleStoreWriteBase): source_course_id, user_id, dest_course_id.org, dest_course_id.course, dest_course_id.run ) - def create_item(self, course_or_parent_loc, category, user_id, **kwargs): + def create_item(self, user_id, location=None, parent_location=None, **kwargs): """ - Create and return the item. If parent_loc is a specific location v a course id, - it installs the new item as a child of the parent (if the parent_loc is a specific - xblock reference). + Creates and saves a new item. + Either location or (category, parent_location) or both must be provided. + If parent_location is provided, a new item of the given category is added as a child. + If location is not provided, a new item with the given category and given block_id + is added to the parent_location. If the block_id is not provided, a unique name + is automatically generated. - :param course_or_parent_loc: Can be a CourseKey or UsageKey - :param category (str): The block_type of the item we are creating + Returns the newly created item. + + :param user_id: ID of the user creating and saving the xmodule + :param location: a Location--must have a category + :param parent_location: optional parameter, specifying the Location of the parent item + :param category: optional parameter for the category of the new item + :param block_id: a unique identifier for the new item """ - # find the store for the course - course_id = getattr(course_or_parent_loc, 'course_key', course_or_parent_loc) - store = self._get_modulestore_for_courseid(course_id) - - location = kwargs.pop('location', None) - # invoke its create_item - if isinstance(store, MongoModuleStore): - block_id = kwargs.pop('block_id', getattr(location, 'name', uuid4().hex)) - parent_loc = course_or_parent_loc if isinstance(course_or_parent_loc, UsageKey) else None - # must have a legitimate location, compute if appropriate - if location is None: - location = course_id.make_usage_key(category, block_id) - # do the actual creation - xblock = self.create_and_save_xmodule(location, user_id, **kwargs) - # don't forget to attach to parent - if parent_loc is not None and not 'detached' in xblock._class_tags: - parent = store.get_item(parent_loc) - parent.children.append(location) - store.update_item(parent, user_id) - elif isinstance(store, SplitMongoModuleStore): - if not isinstance(course_or_parent_loc, (CourseLocator, BlockUsageLocator)): - raise ValueError(u"Cannot create a child of {} in split. Wrong repr.".format(course_or_parent_loc)) - - # split handles all the fields in one dict not separated by scope - fields = kwargs.get('fields', {}) - fields.update(kwargs.pop('metadata', {})) - fields.update(kwargs.pop('definition_data', {})) - kwargs['fields'] = fields - - xblock = store.create_item(course_or_parent_loc, category, user_id, **kwargs) - else: - raise NotImplementedError(u"Cannot create an item on store %s" % store) - - return xblock + location = compute_location_from_args(location, parent_location, **kwargs) + modulestore = self._verify_modulestore_support(location, 'create_item') + return modulestore.create_item(user_id, location, parent_location, **kwargs) def update_item(self, xblock, user_id, allow_not_found=False): """ @@ -378,7 +348,7 @@ class MixedModuleStore(ModuleStoreWriteBase): store = self._verify_modulestore_support(location, 'delete_item') store.delete_item(location, user_id=user_id, **kwargs) - def revert_to_published(self, location, user_id=None): + def revert_to_published(self, location, user_id): """ Reverts an item to its last published version (recursively traversing all of its descendants). If no published version exists, a VersionConflictError is thrown. @@ -389,7 +359,7 @@ class MixedModuleStore(ModuleStoreWriteBase): :raises InvalidVersionError: if no published version exists for the location specified """ store = self._verify_modulestore_support(location, 'revert_to_published') - return store.revert_to_published(location, user_id=user_id) + return store.revert_to_published(location, user_id) def close_all_connections(self): """ @@ -408,7 +378,7 @@ class MixedModuleStore(ModuleStoreWriteBase): if hasattr(modulestore, '_drop_database'): modulestore._drop_database() # pylint: disable=protected-access - def create_xmodule(self, location, definition_data=None, metadata=None, runtime=None, fields={}): + def create_xmodule(self, location, definition_data=None, metadata=None, runtime=None, fields={}, **kwargs): """ Create the new xmodule but don't save it. Returns the new module. @@ -419,7 +389,7 @@ class MixedModuleStore(ModuleStoreWriteBase): :param fields: a dictionary of field names and values for the new xmodule """ store = self._verify_modulestore_support(location, 'create_xmodule') - return store.create_xmodule(location, definition_data, metadata, runtime, fields) + return store.create_xmodule(location, definition_data, metadata, runtime, fields, **kwargs) def get_courses_for_wiki(self, wiki_slug): """ diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index 52865e1993..f24b39cca2 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -34,6 +34,7 @@ from xblock.exceptions import InvalidScopeError from xblock.fields import Scope, ScopeIds, Reference, ReferenceList, ReferenceValueDict from xmodule.modulestore import ModuleStoreWriteBase, ModuleStoreEnum +from xmodule.modulestore.draft_and_published import ModuleStoreDraftAndPublished from opaque_keys.edx.locations import Location from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError, ReferentialIntegrityError from xmodule.modulestore.inheritance import own_metadata, InheritanceMixin, inherit_metadata, InheritanceKeyValueStore @@ -335,7 +336,7 @@ def as_published(location): return location.replace(revision=MongoRevisionKey.published) -class MongoModuleStore(ModuleStoreWriteBase): +class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase): """ A Mongodb backed ModuleStore """ @@ -353,7 +354,7 @@ class MongoModuleStore(ModuleStoreWriteBase): :param doc_store_config: must have a host, db, and collection entries. Other common entries: port, tz_aware. """ - super(MongoModuleStore, self).__init__(contentstore, **kwargs) + super(MongoModuleStore, self).__init__(contentstore=contentstore, **kwargs) def do_connection( db, collection, host, port=27017, tz_aware=True, user=None, password=None, **kwargs @@ -903,7 +904,7 @@ class MongoModuleStore(ModuleStoreWriteBase): ])) location = course_id.make_usage_key('course', course_id.run) - course = self.create_and_save_xmodule(location, user_id, fields=fields, **kwargs) + course = self.create_item(user_id, location, fields=fields, **kwargs) # clone a default 'about' overview module as well about_location = location.replace( @@ -911,16 +912,16 @@ class MongoModuleStore(ModuleStoreWriteBase): name='overview' ) overview_template = AboutDescriptor.get_template('overview.yaml') - self.create_and_save_xmodule( - about_location, + self.create_item( user_id, + about_location, definition_data=overview_template.get('data'), runtime=course.system ) return course - def create_xmodule(self, location, definition_data=None, metadata=None, runtime=None, fields={}): + def create_xmodule(self, location, definition_data=None, metadata=None, runtime=None, fields={}, **kwargs): """ Create the new xmodule but don't save it. Returns the new module. @@ -1161,7 +1162,7 @@ class MongoModuleStore(ModuleStoreWriteBase): def get_orphans(self, course_key): """ - Return an array of all of the locations (deprecated string format) for orphans in the course. + Return an array of all of the locations for orphans in the course. """ course_key = self.fill_in_run(course_key) detached_categories = [name for name, __ in XBlock.load_tagged_classes("detached")] @@ -1178,7 +1179,7 @@ class MongoModuleStore(ModuleStoreWriteBase): ) all_reachable = all_reachable.union(item.get('definition', {}).get('children', [])) item_locs -= all_reachable - return list(item_locs) + return [course_key.make_usage_key_from_deprecated_string(item_loc) for item_loc in item_locs] def get_courses_for_wiki(self, wiki_slug): """ diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py index 8c243faf2f..d70342cc48 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py @@ -11,7 +11,7 @@ import logging from opaque_keys.edx.locations import Location from xmodule.exceptions import InvalidVersionError -from xmodule.modulestore import PublishState, ModuleStoreEnum +from xmodule.modulestore import PublishState, ModuleStoreEnum, compute_location_from_args from xmodule.modulestore.exceptions import ( ItemNotFoundError, DuplicateItemError, InvalidBranchSetting, DuplicateCourseError ) @@ -54,7 +54,6 @@ class DraftModuleStore(MongoModuleStore): This should be an attribute from ModuleStoreEnum.Branch """ super(DraftModuleStore, self).__init__(*args, **kwargs) - self.branch_setting_func = kwargs.pop('branch_setting_func', lambda: ModuleStoreEnum.Branch.published_only) def get_item(self, usage_key, depth=0, revision=None): """ @@ -287,7 +286,7 @@ class DraftModuleStore(MongoModuleStore): else ModuleStoreEnum.RevisionOption.draft_preferred return super(DraftModuleStore, self).get_parent_location(location, revision, **kwargs) - def create_xmodule(self, location, definition_data=None, metadata=None, runtime=None, fields={}): + def create_xmodule(self, location, definition_data=None, metadata=None, runtime=None, fields={}, **kwargs): """ Create the new xmodule but don't save it. Returns the new module with a draft locator if the category allows drafts. If the category does not allow drafts, just creates a published module. @@ -306,6 +305,35 @@ class DraftModuleStore(MongoModuleStore): super(DraftModuleStore, self).create_xmodule(location, definition_data, metadata, runtime, fields) ) + def create_item(self, user_id, location=None, parent_location=None, **kwargs): + """ + Creates and saves a new item. + Either location or (category, parent_location) or both must be provided. + If parent_location is provided, a new item of the given category is added as a child. + If location is not provided, a new item with the given category and given block_id + is added to the parent_location. If the block_id is not provided, a unique name + is automatically generated. + + Returns the newly created item. + + :param user_id: ID of the user creating and saving the xmodule + :param location: a Location--must have a category + :param parent_location: optional parameter, specifying the Location of the parent item + :param category: optional parameter for the category of the new item + :param block_id: a unique identifier for the new item + """ + location = compute_location_from_args(location, parent_location, **kwargs) + xblock = self.create_xmodule(location, **kwargs) + self.update_item(xblock, user_id, allow_not_found=True) + + # attach to parent if given + if parent_location is not None and not 'detached' in xblock._class_tags: + parent = self.get_item(parent_location) + parent.children.append(location) + self.update_item(parent, user_id) + + return xblock + def get_items(self, course_key, settings=None, content=None, revision=None, **kwargs): """ Performance Note: This is generally a costly operation, but useful for wildcard searches. diff --git a/common/lib/xmodule/xmodule/modulestore/split_migrator.py b/common/lib/xmodule/xmodule/modulestore/split_migrator.py index bf8929d760..6a841df398 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_migrator.py +++ b/common/lib/xmodule/xmodule/modulestore/split_migrator.py @@ -84,12 +84,11 @@ class SplitMigrator(object): # it doesn't need the parent as the first arg. That is, it translates and populates # the 'children' field as it goes. _new_module = self.split_modulestore.create_item( - course_version_locator, module.category, user_id, + user.id, new_locator, parent_location=course_version_locator, block_id=module.location.block_id, fields=self._get_json_fields_translate_references( module, course_version_locator, new_course.location.block_id ), - # TODO remove continue_version when bulk write is impl'd continue_version=True ) # after done w/ published items, add version for DRAFT pointing to the published structure @@ -130,7 +129,11 @@ class SplitMigrator(object): else: # only a draft version (aka, 'private'). _new_module = self.split_modulestore.create_item( +<<<<<<< HEAD new_draft_course_loc, module.category, user_id, +======= + user.id, new_locator, parent_location=new_draft_course_loc, +>>>>>>> converge create_item. block_id=new_locator.block_id, fields=self._get_json_fields_translate_references( module, new_draft_course_loc, published_course_usage_key.block_id diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index 015a915f48..a743906418 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -64,7 +64,9 @@ from opaque_keys.edx.locator import ( ) from xmodule.modulestore.exceptions import InsufficientSpecificationError, VersionConflictError, DuplicateItemError, \ DuplicateCourseError -from xmodule.modulestore import inheritance, ModuleStoreWriteBase, ModuleStoreEnum, PublishState +from xmodule.modulestore import ( + inheritance, ModuleStoreWriteBase, ModuleStoreEnum, compute_location_from_args +) from ..exceptions import ItemNotFoundError from .definition_lazy_loader import DefinitionLazyLoader @@ -325,17 +327,17 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): } return envelope - def get_courses(self, branch=ModuleStoreEnum.BranchName.draft, qualifiers=None): + def get_courses(self, branch, qualifiers=None): ''' 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 - (e.g., ModuleStoreEnum.BranchName.draft). To get specific versions via guid use get_course. + 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. Default value is ModuleStoreEnum.BranchName.draft. + :param branch: the branch for which to return courses. :param qualifiers: an optional dict restricting which elements should match ''' if qualifiers is None: @@ -415,20 +417,6 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): return self._get_block_from_structure(course_structure, usage_key.block_id) is not None - def has_changes(self, usage_key): - """ - Checks if the given block has unpublished changes - :param usage_key: the block to check - :return: True if the draft and published versions differ - """ - draft = self.get_item(usage_key.for_branch(ModuleStoreEnum.BranchName.draft)) - try: - published = self.get_item(usage_key.for_branch(ModuleStoreEnum.BranchName.published)) - except ItemNotFoundError: - return True - - return draft.update_version != published.update_version - def get_item(self, usage_key, depth=0): """ depth (int): An argument that some module stores may use to prefetch @@ -543,7 +531,9 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): if block_data['category'] in detached_categories: items.discard(decode_key_from_mongo(block_id)) return [ - BlockUsageLocator(course_key=course_key, block_type=blocks[block_id]['category'], block_id=block_id) + BlockUsageLocator( + course_key=course_key, block_type=blocks[block_id]['category'], block_id=block_id + ).version_agnostic() for block_id in items ] @@ -776,10 +766,8 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): # DHM: Should I rewrite this to take a new xblock instance rather than to construct it? That is, require the # caller to use XModuleDescriptor.load_from_json thus reducing similar code and making the object creation and # validation behavior a responsibility of the model layer rather than the persistence layer. - def create_item( - self, course_or_parent_locator, category, user_id, - block_id=None, definition_locator=None, fields=None, - force=False, continue_version=False + def create_item(self, user_id, location=None, parent_location=None, + definition_locator=None, force=False, continue_version=False, **kwargs ): """ Add a descriptor to persistence as the last child of the optional parent_location or just as an element @@ -827,10 +815,22 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): the course id'd by version_guid but instead in one w/ a new version_guid. Ensure in this case that you get the new version_guid from the locator in the returned object! """ - # find course_index entry if applicable and structures entry - index_entry = self._get_index_if_valid(course_or_parent_locator, force, continue_version) - structure = self._lookup_course(course_or_parent_locator)['structure'] + location = compute_location_from_args(location, parent_location, **kwargs) + if not isinstance(location, (CourseLocator, BlockUsageLocator)): + raise ValueError(u"Cannot create item {} in split. Wrong repr.".format(location)) + + # convert fields into a single dict if separated by scope + fields = kwargs.get('fields', {}) + fields.update(kwargs.pop('metadata', {})) + fields.update(kwargs.pop('definition_data', {})) + kwargs['fields'] = fields + + # find course_index entry if applicable and structures entry + index_entry = self._get_index_if_valid(location, force, continue_version) + structure = self._lookup_course(location)['structure'] + + category = location.block_type partitioned_fields = self.partition_fields_by_scope(category, fields) new_def_data = partitioned_fields.get(Scope.content, {}) # persist the definition if persisted != passed @@ -848,6 +848,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): new_id = new_structure['_id'] # generate usage id + block_id = kwargs.pop('block_id', location.block_id) if block_id is not None: if encode_key_for_mongo(block_id) in new_structure['blocks']: raise DuplicateItemError(block_id, self, 'structures') @@ -873,8 +874,8 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): # if given parent, add new block as child and update parent's version parent = None - if isinstance(course_or_parent_locator, BlockUsageLocator) and course_or_parent_locator.block_id is not None: - encoded_block_id = encode_key_for_mongo(course_or_parent_locator.block_id) + if isinstance(parent_location, BlockUsageLocator) and parent_location.block_id is not None: + encoded_block_id = encode_key_for_mongo(parent_location.block_id) parent = new_structure['blocks'][encoded_block_id] parent['fields'].setdefault('children', []).append(new_block_id) if not continue_version or parent['edit_info']['update_version'] != structure['_id']: @@ -893,9 +894,9 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): # update the index entry if appropriate if index_entry is not None: if not continue_version: - self._update_head(index_entry, course_or_parent_locator.branch, new_id) + self._update_head(index_entry, location.branch, new_id) item_loc = BlockUsageLocator( - course_or_parent_locator.version_agnostic(), + location.version_agnostic(), block_type=category, block_id=new_block_id, ) @@ -924,8 +925,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): ) def create_course( - self, org, course, run, user_id, fields=None, - master_branch=ModuleStoreEnum.BranchName.draft, + self, org, course, run, user_id, master_branch=None, fields=None, versions_dict=None, root_category='course', root_block_id='course', **kwargs ): @@ -1271,15 +1271,13 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): if key not in new_keys or original_fields[key] != settings[key]: return True - def xblock_publish(self, user_id, source_course, destination_course, subtree_list, blacklist): + def copy(self, user_id, source_course, destination_course, subtree_list=None, blacklist=None): """ - Publishes each xblock in subtree_list and those blocks descendants excluding blacklist + Copies each xblock in subtree_list and those blocks descendants excluding blacklist from source_course to destination_course. - To delete a block, publish its parent. You can blacklist the other sibs to keep them from - being refreshed. You can also just call delete_item on the destination. - - To unpublish a block, call delete_item on the destination. + To delete a block in the destination_course, copy its parent and blacklist the other + sibs to keep them from being copies. You can also just call delete_item on the destination. Ensures that each subtree occurs in the same place in destination as it does in source. If any of the source's subtree parents are missing from destination, it raises ItemNotFound([parent_ids]). @@ -1355,10 +1353,6 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): self.db_connection.insert_structure(destination_structure) self._update_head(index_entry, destination_course.branch, destination_structure['_id']) - def unpublish(self, location, user_id): - published_location = location.replace(branch=ModuleStoreEnum.BranchName.published) - self.delete_item(published_location, user_id) - def update_course_index(self, updated_index_entry): """ Change the given course's index entry. @@ -1445,18 +1439,6 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): # in case the course is later restored. # super(SplitMongoModuleStore, self).delete_course(course_key, user_id) - def revert_to_published(self, location, user_id=None): - """ - Reverts an item to its last published version (recursively traversing all of its descendants). - If no published version exists, a VersionConflictError is thrown. - - If a published version exists but there is no draft version of this item or any of its descendants, this - method is a no-op. - - :raises InvalidVersionError: if no published version exists for the location specified - """ - raise NotImplementedError() - def inherit_settings(self, block_map, block_json, inheriting_settings=None): """ Updates block_json with any inheritable setting set by an ancestor and recurses to children. @@ -1863,47 +1845,3 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): Check that the db is reachable. """ return {ModuleStoreEnum.Type.split: self.db_connection.heartbeat()} - - - def compute_publish_state(self, xblock): - """ - Returns whether this xblock is draft, public, or private. - - Returns: - PublishState.draft - published exists and is different from draft - PublishState.public - published exists and is the same as draft - PublishState.private - no published version exists - """ - # TODO figure out what to say if xblock is not from the HEAD of its branch - def get_head(branch): - course_structure = self._lookup_course(xblock.location.course_key.for_branch(branch))['structure'] - return self._get_block_from_structure(course_structure, xblock.location.block_id) - - if xblock.location.branch is None: - raise ValueError(u'{} is not in a branch; so, this is nonsensical'.format(xblock.location)) - if xblock.location.branch == ModuleStoreEnum.BranchName.draft: - other = get_head(ModuleStoreEnum.BranchName.published) - elif xblock.location.branch == ModuleStoreEnum.BranchName.published: - other = get_head(ModuleStoreEnum.BranchName.draft) - else: - raise ValueError(u'{} is not in a branch other than draft or published; so, this is nonsensical'.format(xblock.location)) - - if not other: - if xblock.location.branch == ModuleStoreEnum.BranchName.draft: - return PublishState.private - else: - return PublishState.public # a bit nonsensical - elif xblock.update_version != other['edit_info']['update_version']: - return PublishState.draft - else: - return PublishState.public - - -def convert_to_draft(self, location, user_id): - """ - Create a copy of the source and mark its revision as draft. - - :param source: the location of the source (its revision must be None) - """ - # This is a no-op in Split since a draft version of the data always remains - pass diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py new file mode 100644 index 0000000000..f4862154c2 --- /dev/null +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py @@ -0,0 +1,135 @@ +""" +Module for the dual-branch fall-back Draft->Published Versioning ModuleStore +""" + +from ..exceptions import ItemNotFoundError +from split import SplitMongoModuleStore +from xmodule.modulestore import ModuleStoreEnum, PublishState +from xmodule.modulestore.draft_and_published import ModuleStoreDraftAndPublished + + +class DraftVersioningModuleStore(ModuleStoreDraftAndPublished, SplitMongoModuleStore): + """ + A subclass of Split that supports a dual-branch fall-back versioning framework + with a Draft branch that falls back to a Published branch. + """ + def __init__(self, **kwargs): + super(DraftVersioningModuleStore, self).__init__(**kwargs) + + def create_course(self, org, course, run, user_id, **kwargs): + master_branch = kwargs.pop('master_branch', ModuleStoreEnum.BranchName.draft) + return super(DraftVersioningModuleStore, self).create_course( + org, course, run, user_id, master_branch, **kwargs + ) + + def get_courses(self): + """ + Returns all the courses on the Draft branch (which is a superset of the courses on the Published branch). + """ + return super(DraftVersioningModuleStore, self).get_courses(ModuleStoreEnum.BranchName.draft) + + def delete_item(self, location, user_id, revision=None, **kwargs): + """ + Delete the given item from persistence. kwargs allow modulestore specific parameters. + """ + if 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] + else: + branches_to_delete = [ModuleStoreEnum.BranchName.draft] + for branch in branches_to_delete: + SplitMongoModuleStore.delete_item(self, location.for_branch(branch), user_id, **kwargs) + + def get_parent_location(self, location, revision=None, **kwargs): + # NAATODO - support draft_preferred + return SplitMongoModuleStore.get_parent_location(self, location, **kwargs) + + def has_changes(self, usage_key): + """ + Checks if the given block has unpublished changes + :param usage_key: the block to check + :return: True if the draft and published versions differ + """ + draft = self.get_item(usage_key.for_branch(ModuleStoreEnum.BranchName.draft)) + try: + published = self.get_item(usage_key.for_branch(ModuleStoreEnum.BranchName.published)) + except ItemNotFoundError: + return True + + return draft.update_version != published.update_version + + def publish(self, location, user_id, **kwargs): + """ + Save a current draft to the underlying modulestore. + Returns the newly published item. + """ + SplitMongoModuleStore.copy( + self, + user_id, + location.course_key.for_branch(ModuleStoreEnum.BranchName.draft), + location.course_key.for_branch(ModuleStoreEnum.BranchName.published), + [location], + ) + + def unpublish(self, location, user_id): + """ + Deletes the published version of the item. + Returns the newly unpublished item. + """ + self.delete_item(location.for_branch(ModuleStoreEnum.BranchName.published), user_id) + return self.get_item(location.for_branch(ModuleStoreEnum.BranchName.draft)) + + def revert_to_published(self, location, user_id): + """ + Reverts an item to its last published version (recursively traversing all of its descendants). + If no published version exists, a VersionConflictError is thrown. + + If a published version exists but there is no draft version of this item or any of its descendants, this + method is a no-op. + + :raises InvalidVersionError: if no published version exists for the location specified + """ + raise NotImplementedError() + + def compute_publish_state(self, xblock): + """ + Returns whether this xblock is draft, public, or private. + + Returns: + PublishState.draft - published exists and is different from draft + PublishState.public - published exists and is the same as draft + PublishState.private - no published version exists + """ + # TODO figure out what to say if xblock is not from the HEAD of its branch + def get_head(branch): + course_structure = self._lookup_course(xblock.location.course_key.for_branch(branch))['structure'] + return self._get_block_from_structure(course_structure, xblock.location.block_id) + + if xblock.location.branch is None: + raise ValueError(u'{} is not in a branch; so, this is nonsensical'.format(xblock.location)) + if xblock.location.branch == ModuleStoreEnum.BranchName.draft: + other = get_head(ModuleStoreEnum.BranchName.published) + elif xblock.location.branch == ModuleStoreEnum.BranchName.published: + other = get_head(ModuleStoreEnum.BranchName.draft) + else: + raise ValueError(u'{} is not in a branch other than draft or published; so, this is nonsensical'.format(xblock.location)) + + if not other: + if xblock.location.branch == ModuleStoreEnum.BranchName.draft: + return PublishState.private + else: + return PublishState.public # a bit nonsensical + elif xblock.update_version != other['edit_info']['update_version']: + return PublishState.draft + else: + return PublishState.public + + def convert_to_draft(self, location, user_id): + """ + Create a copy of the source and mark its revision as draft. + + :param source: the location of the source (its revision must be None) + """ + # This is a no-op in Split since a draft version of the data always remains + pass diff --git a/common/lib/xmodule/xmodule/modulestore/tests/factories.py b/common/lib/xmodule/xmodule/modulestore/tests/factories.py index 1d28d1efd5..885182fd75 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/factories.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/factories.py @@ -174,7 +174,7 @@ class ItemFactory(XModuleFactory): if display_name is not None: metadata['display_name'] = display_name runtime = parent.runtime if parent else None - store.create_and_save_xmodule(location, user_id, metadata=metadata, definition_data=data, runtime=runtime) + store.create_item(user_id, location, metadata=metadata, definition_data=data, runtime=runtime) module = store.get_item(location) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/persistent_factories.py b/common/lib/xmodule/xmodule/modulestore/tests/persistent_factories.py index 7a71a02389..d678724da0 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/persistent_factories.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/persistent_factories.py @@ -61,7 +61,7 @@ class ItemFactory(SplitFactory): # pylint: disable=W0613 @classmethod def _create(cls, target_class, parent_location, category='chapter', - user_id=ModuleStoreEnum.UserID.test, block_id=None, definition_locator=None, force=False, + user_id=ModuleStoreEnum.UserID.test, definition_locator=None, force=False, continue_version=False, **kwargs): """ passes *kwargs* as the new item's field values: @@ -74,8 +74,8 @@ class ItemFactory(SplitFactory): """ modulestore = kwargs.pop('modulestore') return modulestore.create_item( - parent_location, category, user_id, definition_locator=definition_locator, - block_id=block_id, force=force, continue_version=continue_version, fields=kwargs + user_id, category=category, parent_location=parent_location, defintion_locator=definition_locator, + force=force, continue_version=continue_version, **kwargs ) @classmethod 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 644aca7d66..ad84818839 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -67,7 +67,7 @@ class TestMixedModuleStore(unittest.TestCase): }, { 'NAME': 'split', - 'ENGINE': 'xmodule.modulestore.split_mongo.split.SplitMongoModuleStore', + 'ENGINE': 'xmodule.modulestore.split_mongo.split_draft.DraftVersioningModuleStore', 'DOC_STORE_CONFIG': DOC_STORE_CONFIG, 'OPTIONS': modulestore_options }, @@ -118,11 +118,11 @@ class TestMixedModuleStore(unittest.TestCase): Create a course w/ one item in the persistence store using the given course & item location. """ course = self.store.create_course(course_key.org, course_key.course, course_key.run, self.user_id) - category = self.writable_chapter_location.category block_id = self.writable_chapter_location.name chapter = self.store.create_item( # don't use course_location as it may not be the repr - course.location, category, self.user_id, location=self.writable_chapter_location, block_id=block_id + self.user_id, self.writable_chapter_location, + parent_location=course.location, block_id=block_id ) if isinstance(course.id, CourseLocator): self.course_locations[self.MONGO_COURSEID] = course.location.version_agnostic() @@ -174,7 +174,10 @@ class TestMixedModuleStore(unittest.TestCase): ] def create_sub_tree(parent, block_info): - block = self.store.create_item(parent.location, block_info.category, self.user_id, block_id=block_info.display_name) + block = self.store.create_item( + self.user_id, parent_location=parent.location, + category=block_info.category, block_id=block_info.display_name + ) for tree in block_info.sub_tree: create_sub_tree(block, tree) # reload the block to update its children field @@ -226,8 +229,6 @@ class TestMixedModuleStore(unittest.TestCase): self.xml_chapter_location = self.course_locations[self.XML_COURSEID1].replace( category='chapter', name='Overview' ) - - self._create_course(default, self.course_locations[self.MONGO_COURSEID].course_key) @ddt.data('draft', 'split') @@ -325,11 +326,12 @@ class TestMixedModuleStore(unittest.TestCase): # create and delete a private vertical with private children private_vert = self.store.create_item( # don't use course_location as it may not be the repr - self.course_locations[self.MONGO_COURSEID], 'vertical', user_id=self.user_id, block_id='private' + self.user_id, parent_location=self.course_locations[self.MONGO_COURSEID], + category='vertical', block_id='private' ) private_leaf = self.store.create_item( # don't use course_location as it may not be the repr - private_vert.location, 'html', user_id=self.user_id, block_id='private_leaf' + self.user_id, parent_location=private_vert.location, category='html', block_id='private_leaf' ) # verify pre delete state (just to verify that the test is valid) @@ -577,7 +579,7 @@ class TestMixedModuleStore(unittest.TestCase): self.initdb(default_ms) self._create_block_hierarchy() with self.assertRaises(InvalidVersionError): - self.store.revert_to_published(self.vertical_x1a.location) + self.store.revert_to_published(self.vertical_x1a.location, self.user_id) @ddt.data('draft') def test_revert_to_published_direct_only(self, default_ms): @@ -586,7 +588,7 @@ class TestMixedModuleStore(unittest.TestCase): """ self.initdb(default_ms) self._create_block_hierarchy() - self.store.revert_to_published(self.sequential_x1.location) + self.store.revert_to_published(self.sequential_x1.location, self.user_id) reverted_parent = self.store.get_item(self.sequential_x1.location) # It does not discard the child vertical, even though that child is a draft (with no published version) self.assertEqual(1, len(reverted_parent.children)) @@ -596,12 +598,10 @@ class TestMixedModuleStore(unittest.TestCase): self.initdb(default_ms) # create an orphan course_id = self.course_locations[self.MONGO_COURSEID].course_key - orphan = self.store.create_item(course_id, 'problem', self.user_id, block_id='orphan') + orphan_location = course_id.make_usage_key('problem', 'orphan') + orphan = self.store.create_item(self.user_id, orphan_location) found_orphans = self.store.get_orphans(self.course_locations[self.MONGO_COURSEID].course_key) - if default_ms == 'split': - self.assertEqual(found_orphans, [orphan.location.version_agnostic()]) - else: - self.assertEqual(found_orphans, [orphan.location.to_deprecated_string()]) + self.assertEqual(found_orphans, [orphan_location]) @ddt.data('draft') def test_create_item_from_parent_location(self, default_ms): @@ -610,7 +610,10 @@ class TestMixedModuleStore(unittest.TestCase): new location for the child """ self.initdb(default_ms) - self.store.create_item(self.course_locations[self.MONGO_COURSEID], 'problem', self.user_id, block_id='orphan') + self.store.create_item( + self.user_id, parent_location=self.course_locations[self.MONGO_COURSEID], + category='problem', block_id='orphan' + ) orphans = self.store.get_orphans(self.course_locations[self.MONGO_COURSEID].course_key) self.assertEqual(len(orphans), 0, "unexpected orphans: {}".format(orphans)) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py index c78c508a6f..69a5285195 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py @@ -23,7 +23,7 @@ from xblock.plugin import Plugin from xmodule.tests import DATA_DIR from opaque_keys.edx.locations import Location from xmodule.modulestore import ModuleStoreEnum -from xmodule.modulestore.mongo import MongoModuleStore, MongoKeyValueStore +from xmodule.modulestore.mongo import MongoKeyValueStore from xmodule.modulestore.draft import DraftModuleStore from opaque_keys.edx.locations import SlashSeparatedCourseKey, AssetLocation from opaque_keys.edx.keys import UsageKey @@ -148,7 +148,7 @@ class TestMongoModuleStore(unittest.TestCase): assert_greater(len(ids), 12) def test_mongo_modulestore_type(self): - store = MongoModuleStore( + store = DraftModuleStore( None, {'host': HOST, 'db': DB, 'collection': COLLECTION}, FS_ROOT, RENDER_TEMPLATE, default_class=DEFAULT_CLASS @@ -390,13 +390,13 @@ class TestMongoModuleStore(unittest.TestCase): def setup_test(): course = self.draft_store.get_course(course_key) # can't use item factory as it depends on django settings - p1ele = self.draft_store.create_and_save_xmodule( - course.id.make_usage_key('problem', 'p1'), 99, runtime=course.runtime) - p2ele = self.draft_store.create_and_save_xmodule( - course.id.make_usage_key('problem', 'p2'), 99, runtime=course.runtime) + p1ele = self.draft_store.create_item( + 99, course.id.make_usage_key('problem', 'p1'), runtime=course.runtime) + p2ele = self.draft_store.create_item( + 99, course.id.make_usage_key('problem', 'p2'), runtime=course.runtime) self.refloc = course.id.make_usage_key('ref_test', 'ref_test') - self.draft_store.create_and_save_xmodule( - self.refloc, 99, runtime=course.runtime, fields={ + self.draft_store.create_item( + 99,self.refloc, runtime=course.runtime, fields={ 'reference_link': p1ele.location, 'reference_list': [p1ele.location, p2ele.location], 'reference_dict': {'p1': p1ele.location, 'p2': p2ele.location}, @@ -501,8 +501,8 @@ class TestMongoModuleStore(unittest.TestCase): chapter_location = Location('edx', 'direct', '2012_Fall', 'chapter', 'test_chapter') # Create dummy direct only xblocks - self.draft_store.create_and_save_xmodule(course_location, user_id=self.dummy_user) - self.draft_store.create_and_save_xmodule(chapter_location, user_id=self.dummy_user) + self.draft_store.create_item(self.dummy_user, course_location) + self.draft_store.create_item(self.dummy_user, chapter_location) # Check that neither xblock has changes self.assertFalse(self.draft_store.has_changes(course_location)) @@ -515,7 +515,7 @@ class TestMongoModuleStore(unittest.TestCase): location = Location('edX', 'changes', '2012_Fall', 'vertical', 'test_vertical') # Create a dummy component to test against - self.draft_store.create_and_save_xmodule(location, user_id=self.dummy_user) + self.draft_store.create_item(self.dummy_user, location) # Not yet published, so changes are present self.assertTrue(self.draft_store.has_changes(location)) @@ -570,7 +570,7 @@ class TestMongoModuleStore(unittest.TestCase): } for key in locations: - self.draft_store.create_and_save_xmodule(locations[key], user_id=user_id) + self.draft_store.create_item(user_id, locations[key]) grandparent = self.draft_store.get_item(locations['grandparent']) grandparent.children += [locations['parent_sibling'], locations['parent']] @@ -663,7 +663,7 @@ class TestMongoModuleStore(unittest.TestCase): # Create a new child and attach it to parent new_child_location = Location('edX', 'tree', 'has_changes_add_remove_child', 'vertical', 'new_child') - self.draft_store.create_and_save_xmodule(new_child_location, user_id=self.dummy_user) + self.draft_store.create_item(self.dummy_user, new_child_location) parent = self.draft_store.get_item(locations['parent']) parent.children += [new_child_location] self.draft_store.update_item(parent, user_id=self.dummy_user) @@ -688,8 +688,8 @@ class TestMongoModuleStore(unittest.TestCase): parent_location = Location('edX', 'test', 'non_direct_only_children', 'vertical', 'parent') child_location = Location('edX', 'test', 'non_direct_only_children', 'html', 'child') - parent = self.draft_store.create_and_save_xmodule(parent_location, user_id=self.dummy_user) - child = self.draft_store.create_and_save_xmodule(child_location, user_id=self.dummy_user) + parent = self.draft_store.create_item(self.dummy_user, parent_location) + child = self.draft_store.create_item(self.dummy_user, child_location) parent.children += [child_location] self.draft_store.update_item(parent, user_id=self.dummy_user) self.draft_store.publish(parent_location, self.dummy_user) @@ -760,7 +760,7 @@ class TestMongoModuleStore(unittest.TestCase): location = Location('edX', 'editInfoTest', '2012_Fall', 'html', 'test_html') # Create a dummy component to test against - self.draft_store.create_and_save_xmodule(location, user_id=self.dummy_user) + self.draft_store.create_item(self.dummy_user, location) # Store the current edit time and verify that dummy_user created the component component = self.draft_store.get_item(location) @@ -785,7 +785,7 @@ class TestMongoModuleStore(unittest.TestCase): publish_user = 456 # Create a dummy component to test against - self.draft_store.create_and_save_xmodule(location, user_id=create_user) + self.draft_store.create_item(create_user, location) # Store the current time, then publish old_time = datetime.now(UTC) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_draft_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_draft_modulestore.py new file mode 100644 index 0000000000..d73b2d9b1c --- /dev/null +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_draft_modulestore.py @@ -0,0 +1,63 @@ +""" + Test split_draft modulestore +""" +import unittest +import uuid +from xmodule.modulestore.split_mongo.split_draft import DraftVersioningModuleStore +from xmodule.modulestore import ModuleStoreEnum +from opaque_keys.edx.locator import CourseLocator +from xmodule.modulestore.inheritance import InheritanceMixin +from xmodule.x_module import XModuleMixin +from xmodule.modulestore.tests.test_split_modulestore import SplitModuleTest + +# pylint: disable=W0613 +def render_to_template_mock(*args): + pass + + +class TestDraftVersioningModuleStore(unittest.TestCase): + def setUp(self): + super(TestDraftVersioningModuleStore, self).setUp() + self.module_store = DraftVersioningModuleStore( + contentstore=None, + doc_store_config={ + 'host': 'localhost', + 'db': 'test_xmodule', + 'collection': 'modulestore{0}'.format(uuid.uuid4().hex[:5]), + }, + fs_root='', + default_class='xmodule.raw_module.RawDescriptor', + render_template=render_to_template_mock, + xblock_mixins=(InheritanceMixin, XModuleMixin), + ) + # NAATODO - uncomment once merged with drop_database PR + # self.addCleanup(module_store._drop_database) + + SplitModuleTest.bootstrapDB(self.module_store) + + def test_has_changes(self): + """ + Tests that has_changes() only returns true when changes are present + """ + draft_course = CourseLocator( + org='testx', course='GreekHero', run='run', branch=ModuleStoreEnum.BranchName.draft + ) + head = draft_course.make_usage_key('course', 'head12345') + dummy_user = ModuleStoreEnum.UserID.test + + # Not yet published, so changes are present + self.assertTrue(self.module_store.has_changes(head)) + + # Publish and verify that there are no unpublished changes + self.module_store.publish(head, dummy_user) + self.assertFalse(self.module_store.has_changes(head)) + + # Change the course, then check that there now are changes + course = self.module_store.get_item(head) + course.show_calculator = not course.show_calculator + self.module_store.update_item(course, dummy_user) + self.assertTrue(self.module_store.has_changes(head)) + + # Publish and verify again + self.module_store.publish(head, dummy_user) + self.assertFalse(self.module_store.has_changes(head)) 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 d25c0c3226..7ff8c5f492 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py @@ -446,17 +446,17 @@ class SplitModuleTest(unittest.TestCase): } @staticmethod - def bootstrapDB(): + def bootstrapDB(split_store): ''' Sets up the initial data into the db ''' - split_store = modulestore() for _course_id, course_spec in SplitModuleTest.COURSE_CONTENT.iteritems(): course = split_store.create_course( course_spec['org'], course_spec['course'], course_spec['run'], course_spec['user_id'], + master_branch=BRANCH_NAME_DRAFT, fields=course_spec['fields'], root_block_id=course_spec['root_block_id'] ) @@ -494,7 +494,7 @@ class SplitModuleTest(unittest.TestCase): block_id="head23456" ) destination = CourseLocator(org="testx", course="wonderful", run="run", branch=BRANCH_NAME_PUBLISHED) - split_store.xblock_publish("test@edx.org", to_publish, destination, [to_publish], None) + split_store.copy("test@edx.org", to_publish, destination, [to_publish], None) def setUp(self): self.user_id = random.getrandbits(32) @@ -823,32 +823,6 @@ class SplitModuleItemTests(SplitModuleTest): with self.assertRaises(ItemNotFoundError): modulestore().get_item(course.location.for_branch(BRANCH_NAME_PUBLISHED)) - def test_has_changes(self): - """ - Tests that has_changes() only returns true when changes are present - """ - draft_course = CourseLocator(org='testx', course='GreekHero', run="run", branch=BRANCH_NAME_DRAFT) - published_course = CourseLocator(org='testx', course='GreekHero', run="run", branch=BRANCH_NAME_PUBLISHED) - head = draft_course.make_usage_key('course', 'head12345') - dummy_user = ModuleStoreEnum.UserID.test - - # Not yet published, so changes are present - self.assertTrue(modulestore().has_changes(head)) - - # Publish and verify that there are no unpublished changes - modulestore().xblock_publish(dummy_user, draft_course, published_course, [head], None) - self.assertFalse(modulestore().has_changes(head)) - - # Change the course, then check that there now are changes - course = modulestore().get_item(head) - course.show_calculator = not course.show_calculator - modulestore().update_item(course, dummy_user) - self.assertTrue(modulestore().has_changes(head)) - - # Publish and verify again - modulestore().xblock_publish(dummy_user, draft_course, published_course, [head], None) - self.assertFalse(modulestore().has_changes(head)) - def test_get_non_root(self): # not a course obj locator = BlockUsageLocator( @@ -1002,7 +976,7 @@ class TestItemCrud(SplitModuleTest): def test_create_minimal_item(self): """ - create_item(course_or_parent_locator, category, user, definition_locator=None, fields): new_desciptor + create_item(user, location, category, definition_locator=None, fields): new_desciptor """ # grab link to course to ensure new versioning works locator = CourseLocator(org='testx', course='GreekHero', run="run", branch=BRANCH_NAME_DRAFT) @@ -1011,7 +985,7 @@ class TestItemCrud(SplitModuleTest): # add minimal one w/o a parent category = 'sequential' new_module = modulestore().create_item( - locator, category, 'user123', + 'user123', parent_location=locator, category=category, fields={'display_name': 'new sequential'} ) # check that course version changed and course's previous is the other one @@ -1052,7 +1026,7 @@ class TestItemCrud(SplitModuleTest): premod_course = modulestore().get_course(locator.course_key) category = 'chapter' new_module = modulestore().create_item( - locator, category, 'user123', + 'user123', parent_location=locator, category=category, fields={'display_name': 'new chapter'}, definition_locator=original.definition_locator ) @@ -1081,12 +1055,12 @@ class TestItemCrud(SplitModuleTest): category = 'problem' new_payload = "empty" new_module = modulestore().create_item( - locator, category, 'anotheruser', + 'anotheruser', parent_location=locator, category=category, fields={'display_name': 'problem 1', 'data': new_payload}, ) another_payload = "not empty" another_module = modulestore().create_item( - locator, category, 'anotheruser', + 'anotheruser', parent_location=locator, category=category, fields={'display_name': 'problem 2', 'data': another_payload}, definition_locator=original.definition_locator, ) @@ -1113,7 +1087,7 @@ class TestItemCrud(SplitModuleTest): parent_locator = BlockUsageLocator(course_key, 'course', block_id="head345679") chapter_locator = BlockUsageLocator(course_key, 'chapter', block_id="foo.bar_-~:0") modulestore().create_item( - parent_locator, 'chapter', 'anotheruser', + 'anotheruser', parent_location=parent_locator, category='chapter', block_id=chapter_locator.block_id, fields={'display_name': 'chapter 99'}, ) @@ -1124,7 +1098,7 @@ class TestItemCrud(SplitModuleTest): new_payload = "empty" problem_locator = BlockUsageLocator(course_key, 'problem', block_id="prob.bar_-~:99a") modulestore().create_item( - chapter_locator, 'problem', 'anotheruser', + 'anotheruser', parent_location=chapter_locator, category='problem', block_id=problem_locator.block_id, fields={'display_name': 'chapter 99', 'data': new_payload}, ) @@ -1140,7 +1114,7 @@ class TestItemCrud(SplitModuleTest): """ # start transaction w/ simple creation user = random.getrandbits(32) - new_course = modulestore().create_course('test_org', 'test_transaction', 'test_run', user) + new_course = modulestore().create_course('test_org', 'test_transaction', 'test_run', user, BRANCH_NAME_DRAFT) new_course_locator = new_course.id index_history_info = modulestore().get_course_history_info(new_course.location) course_block_prev_version = new_course.previous_version @@ -1150,7 +1124,7 @@ class TestItemCrud(SplitModuleTest): # positive simple case: no force, add chapter new_ele = modulestore().create_item( - new_course.location, 'chapter', user, + user, parent_location=new_course.location, category='chapter', fields={'display_name': 'chapter 1'}, continue_version=True ) @@ -1168,7 +1142,7 @@ class TestItemCrud(SplitModuleTest): # try to create existing item with self.assertRaises(DuplicateItemError): _fail = modulestore().create_item( - new_course.location, 'chapter', user, + user, parent_location=new_course.location, category='chapter', block_id=new_ele.location.block_id, fields={'display_name': 'chapter 2'}, continue_version=True @@ -1176,7 +1150,7 @@ class TestItemCrud(SplitModuleTest): # start a new transaction new_ele = modulestore().create_item( - new_course.location, 'chapter', user, + user, parent_location=new_course.location, category='chapter', fields={'display_name': 'chapter 2'}, continue_version=False ) @@ -1184,7 +1158,7 @@ class TestItemCrud(SplitModuleTest): # ensure force w/ continue gives exception with self.assertRaises(VersionConflictError): _fail = modulestore().create_item( - new_course.location, 'chapter', user, + user, parent_location=new_course.location, category='chapter', fields={'display_name': 'chapter 2'}, force=True, continue_version=True ) @@ -1192,7 +1166,7 @@ class TestItemCrud(SplitModuleTest): # ensure trying to continue the old one gives exception with self.assertRaises(VersionConflictError): _fail = modulestore().create_item( - new_course.location, 'chapter', user, + user, parent_location=new_course.location, category='chapter', fields={'display_name': 'chapter 3'}, continue_version=True ) @@ -1200,7 +1174,7 @@ class TestItemCrud(SplitModuleTest): # add new child to old parent in continued (leave off version_guid) course_module_locator = new_course.location.version_agnostic() new_ele = modulestore().create_item( - course_module_locator, 'chapter', user, + user, parent_location=course_module_locator, category='chapter', fields={'display_name': 'chapter 4'}, continue_version=True ) @@ -1310,12 +1284,12 @@ class TestItemCrud(SplitModuleTest): category = 'problem' new_payload = "empty" modulestore().create_item( - locator, category, 'test_update_manifold', + 'test_update_manifold', parent_location=locator, category=category, fields={'display_name': 'problem 1', 'data': new_payload}, ) another_payload = "not empty" modulestore().create_item( - locator, category, 'test_update_manifold', + 'test_update_manifold', parent_location=locator, category=category, fields={'display_name': 'problem 2', 'data': another_payload}, definition_locator=original.definition_locator, ) @@ -1382,7 +1356,7 @@ class TestItemCrud(SplitModuleTest): """ Create a course we can delete """ - course = modulestore().create_course('nihilx', 'deletion', 'run', 'deleting_user') + course = modulestore().create_course('nihilx', 'deletion', 'run', 'deleting_user', BRANCH_NAME_DRAFT) root = course.location.version_agnostic().for_branch(BRANCH_NAME_DRAFT) for _ in range(4): self.create_subtree_for_deletion(root, ['chapter', 'vertical', 'problem']) @@ -1394,7 +1368,9 @@ class TestItemCrud(SplitModuleTest): """ if not category_queue: return - node = modulestore().create_item(parent.version_agnostic(), category_queue[0], 'deleting_user') + node = modulestore().create_item( + 'deleting_user', parent_location=parent.version_agnostic(), category=category_queue[0] + ) node_loc = node.location.map_into_course(parent.course_key) for _ in range(4): self.create_subtree_for_deletion(node_loc, category_queue[1:]) @@ -1409,7 +1385,9 @@ class TestCourseCreation(SplitModuleTest): The simplest case but probing all expected results from it. """ # Oddly getting differences of 200nsec - new_course = modulestore().create_course('test_org', 'test_course', 'test_run', 'create_user') + new_course = modulestore().create_course( + 'test_org', 'test_course', 'test_run', 'create_user', BRANCH_NAME_DRAFT + ) new_locator = new_course.location # check index entry index_info = modulestore().get_course_index_info(new_locator) @@ -1437,7 +1415,7 @@ class TestCourseCreation(SplitModuleTest): original_locator = CourseLocator(org='testx', course='wonderful', run="run", branch=BRANCH_NAME_DRAFT) original_index = modulestore().get_course_index_info(original_locator) new_draft = modulestore().create_course( - 'best', 'leech', 'leech_run', 'leech_master', + 'best', 'leech', 'leech_run', 'leech_master', BRANCH_NAME_DRAFT, versions_dict=original_index['versions']) new_draft_locator = new_draft.location self.assertRegexpMatches(new_draft_locator.org, 'best') @@ -1456,7 +1434,7 @@ class TestCourseCreation(SplitModuleTest): # changing this course will not change the original course # using new_draft.location will insert the chapter under the course root new_item = modulestore().create_item( - new_draft.location, 'chapter', 'leech_master', + 'leech_master', parent_location=new_draft.location, category='chapter', fields={'display_name': 'new chapter'} ) new_draft_locator = new_draft_locator.course_key.version_agnostic() @@ -1493,7 +1471,7 @@ class TestCourseCreation(SplitModuleTest): fields['grading_policy']['GRADE_CUTOFFS'] = {'A': .9, 'B': .8, 'C': .65} fields['display_name'] = 'Derivative' new_draft = modulestore().create_course( - 'counter', 'leech', 'leech_run', 'leech_master', + 'counter', 'leech', 'leech_run', 'leech_master', BRANCH_NAME_DRAFT, versions_dict={BRANCH_NAME_DRAFT: original_index['versions'][BRANCH_NAME_DRAFT]}, fields=fields ) @@ -1540,7 +1518,7 @@ class TestCourseCreation(SplitModuleTest): """ user = random.getrandbits(32) new_course = modulestore().create_course( - 'test_org', 'test_transaction', 'test_run', user, + 'test_org', 'test_transaction', 'test_run', user, BRANCH_NAME_DRAFT, root_block_id='top', root_category='chapter' ) self.assertEqual(new_course.location.block_id, 'top') @@ -1559,10 +1537,12 @@ class TestCourseCreation(SplitModuleTest): Test create_course rejects duplicate id """ user = random.getrandbits(32) - courses = modulestore().get_courses() + courses = modulestore().get_courses(BRANCH_NAME_DRAFT) with self.assertRaises(DuplicateCourseError): dupe_course_key = courses[0].location.course_key - modulestore().create_course(dupe_course_key.org, dupe_course_key.course, dupe_course_key.run, user) + modulestore().create_course( + dupe_course_key.org, dupe_course_key.course, dupe_course_key.run, user, BRANCH_NAME_DRAFT + ) class TestInheritance(SplitModuleTest): @@ -1609,14 +1589,14 @@ class TestPublish(SplitModuleTest): chapter1 = source_course.make_usage_key('chapter', 'chapter1') chapter2 = source_course.make_usage_key('chapter', 'chapter2') chapter3 = source_course.make_usage_key('chapter', 'chapter3') - modulestore().xblock_publish(self.user_id, source_course, dest_course, [head], [chapter2, chapter3]) + modulestore().copy(self.user_id, source_course, dest_course, [head], [chapter2, chapter3]) expected = [head.block_id, chapter1.block_id] self._check_course( source_course, dest_course, expected, [chapter2.block_id, chapter3.block_id, "problem1", "problem3_2"] ) # add a child under chapter1 new_module = modulestore().create_item( - chapter1, "sequential", self.user_id, + self.user_id, parent_location=chapter1, category="sequential", fields={'display_name': 'new sequential'}, ) # remove chapter1 from expected b/c its pub'd version != the source anymore since source changed @@ -1625,7 +1605,7 @@ class TestPublish(SplitModuleTest): with self.assertRaises(ItemNotFoundError): modulestore().get_item(new_module.location.map_into_course(dest_course)) # publish it - modulestore().xblock_publish(self.user_id, source_course, dest_course, [new_module.location], None) + modulestore().copy(self.user_id, source_course, dest_course, [new_module.location], None) expected.append(new_module.location.block_id) # check that it is in the published course and that its parent is the chapter pub_module = modulestore().get_item(new_module.location.map_into_course(dest_course)) @@ -1634,10 +1614,10 @@ class TestPublish(SplitModuleTest): ) # ensure intentionally orphaned blocks work (e.g., course_info) new_module = modulestore().create_item( - source_course, "course_info", self.user_id, block_id="handouts" + self.user_id, parent_location=source_course, category="course_info", block_id="handouts" ) # publish it - modulestore().xblock_publish(self.user_id, source_course, dest_course, [new_module.location], None) + modulestore().copy(self.user_id, source_course, dest_course, [new_module.location], None) expected.append(new_module.location.block_id) # check that it is in the published course (no error means it worked) pub_module = modulestore().get_item(new_module.location.map_into_course(dest_course)) @@ -1656,15 +1636,15 @@ class TestPublish(SplitModuleTest): chapter3 = source_course.make_usage_key('chapter', 'chapter3') problem1 = source_course.make_usage_key('problem', 'problem1') with self.assertRaises(ItemNotFoundError): - modulestore().xblock_publish(self.user_id, source_course, destination_course, [chapter3], None) + modulestore().copy(self.user_id, source_course, destination_course, [chapter3], None) # publishing into a new branch w/o publishing the root destination_course = CourseLocator(org='testx', course='GreekHero', run='run', branch=BRANCH_NAME_PUBLISHED) with self.assertRaises(ItemNotFoundError): - modulestore().xblock_publish(self.user_id, source_course, destination_course, [chapter3], None) + modulestore().copy(self.user_id, source_course, destination_course, [chapter3], None) # publishing a subdag w/o the parent already in course - modulestore().xblock_publish(self.user_id, source_course, destination_course, [head], [chapter3]) + modulestore().copy(self.user_id, source_course, destination_course, [head], [chapter3]) with self.assertRaises(ItemNotFoundError): - modulestore().xblock_publish(self.user_id, source_course, destination_course, [problem1], []) + modulestore().copy(self.user_id, source_course, destination_course, [problem1], []) def test_move_delete(self): """ @@ -1675,7 +1655,7 @@ class TestPublish(SplitModuleTest): head = source_course.make_usage_key('course', "head12345") chapter2 = source_course.make_usage_key('chapter', 'chapter2') problem1 = source_course.make_usage_key('problem', 'problem1') - modulestore().xblock_publish(self.user_id, source_course, dest_course, [head], [chapter2]) + modulestore().copy(self.user_id, source_course, dest_course, [head], [chapter2]) expected = ["head12345", "chapter1", "chapter3", "problem1", "problem3_2"] self._check_course(source_course, dest_course, expected, ["chapter2"]) # now move problem1 and delete problem3_2 @@ -1684,7 +1664,7 @@ class TestPublish(SplitModuleTest): chapter1.children.append(problem1) chapter3.children.remove(problem1.map_into_course(chapter3.location.course_key)) modulestore().delete_item(source_course.make_usage_key("problem", "problem3_2"), self.user_id) - modulestore().xblock_publish(self.user_id, source_course, dest_course, [head], [chapter2]) + modulestore().copy(self.user_id, source_course, dest_course, [head], [chapter2]) expected = ["head12345", "chapter1", "chapter3", "problem1"] self._check_course(source_course, dest_course, expected, ["chapter2", "problem3_2"]) @@ -1776,7 +1756,7 @@ def modulestore(): **options ) - SplitModuleTest.bootstrapDB() + SplitModuleTest.bootstrapDB(SplitModuleTest.modulestore) return SplitModuleTest.modulestore diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_w_old_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_w_old_mongo.py index d990271652..dc5c7d18f8 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_w_old_mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_w_old_mongo.py @@ -86,7 +86,6 @@ class SplitWMongoCourseBoostrapper(unittest.TestCase): existing draft for both the new item and the parent """ location = self.old_course_key.make_usage_key(category, name) - self.draft_mongo.create_and_save_xmodule( location, self.user_id, definition_data=data, metadata=metadata, runtime=self.runtime ) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_xml_importer.py b/common/lib/xmodule/xmodule/modulestore/tests/test_xml_importer.py index 3317757c15..cf4a2f971e 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_xml_importer.py @@ -7,6 +7,7 @@ from xblock.fields import String, Scope, ScopeIds from xblock.runtime import Runtime, KvsFieldData, DictKeyValueStore from xmodule.x_module import XModuleMixin from opaque_keys.edx.locations import Location +from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.inheritance import InheritanceMixin from xmodule.modulestore.xml_importer import _import_module_and_update_references from opaque_keys.edx.locations import SlashSeparatedCourseKey @@ -39,7 +40,7 @@ class ModuleStoreNoSettings(unittest.TestCase): 'collection': COLLECTION, } MODULESTORE = { - 'ENGINE': 'xmodule.modulestore.mongo.MongoModuleStore', + 'ENGINE': 'xmodule.modulestore.mongo.DraftMongoModuleStore', 'DOC_STORE_CONFIG': DOC_STORE_CONFIG, 'OPTIONS': modulestore_options } @@ -85,6 +86,7 @@ def modulestore(): ModuleStoreNoSettings.modulestore = class_( None, # contentstore ModuleStoreNoSettings.MODULESTORE['DOC_STORE_CONFIG'], + branch_setting_func = lambda: ModuleStoreEnum.Branch.draft_preferred, **options ) diff --git a/common/lib/xmodule/xmodule/split_test_module.py b/common/lib/xmodule/xmodule/split_test_module.py index 9bd5810efe..061e32ba78 100644 --- a/common/lib/xmodule/xmodule/split_test_module.py +++ b/common/lib/xmodule/xmodule/split_test_module.py @@ -571,14 +571,14 @@ class SplitTestDescriptor(SplitTestFields, SequenceDescriptor, StudioEditableDes A mutable modulestore is needed to call this method (will need to update after mixed modulestore work, currently relies on mongo's create_and_save_xmodule method). """ - assert hasattr(self.system, 'modulestore') and hasattr(self.system.modulestore, 'create_and_save_xmodule'), \ + assert hasattr(self.system, 'modulestore') and hasattr(self.system.modulestore, 'create_item'), \ "editor_saved should only be called when a mutable modulestore is available" modulestore = self.system.modulestore dest_usage_key = self.location.replace(category="vertical", name=uuid4().hex) metadata = {'display_name': group.name} - modulestore.create_and_save_xmodule( - dest_usage_key, + modulestore.create_item( user_id, + dest_usage_key, definition_data=None, metadata=metadata, runtime=self.system,