converge Split APIs with Mixed and Draft

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)))
 

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)
 

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="<problem></problem>"

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

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)
 

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
 

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"<ol><li><h2>" + update_date + "</h2>" + update_content + "</li></ol>"
@@ -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)

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',

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

common/lib/xmodule/xmodule/modulestore/draft_and_published.py

+"""
+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

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
-        """
-        # 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)
+        Returns the newly created item.
 
-        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
+        :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)
+        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):
         """

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):
         """

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.

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

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!
         """
+        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(course_or_parent_locator, force, continue_version)
-        structure = self._lookup_course(course_or_parent_locator)['structure']
+        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

common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py

+"""
+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

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)
 

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

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))
 

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)

common/lib/xmodule/xmodule/modulestore/tests/test_split_draft_modulestore.py

+"""
+    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))

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 = "<problem>empty</problem>"
         new_module = modulestore().create_item(
-            locator, category, 'anotheruser',
+            'anotheruser', parent_location=locator, category=category,
             fields={'display_name': 'problem 1', 'data': new_payload},
         )
         another_payload = "<problem>not empty</problem>"
         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 = "<problem>empty</problem>"
         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 = "<problem>empty</problem>"
         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 = "<problem>not empty</problem>"
         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
 

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
         )

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
         )
 

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,