From 432249e9cfa37d81847064d8dbb56e3c385bb863 Mon Sep 17 00:00:00 2001
From: Don Mitchell <dmitchell@edx.org>
Date: Wed, 19 Feb 2014 12:06:44 -0500
Subject: [PATCH] making mixed ms capable of front ending all modulestores for
 most operations including CRUD STUD-600

---
 .../contentstore/tests/test_contentstore.py   |   9 +-
 .../contentstore/views/component.py           |   4 +-
 cms/djangoapps/contentstore/views/item.py     |   4 +-
 cms/envs/common.py                            |   5 +-
 cms/envs/test.py                              |   2 +-
 .../xmodule/xmodule/modulestore/__init__.py   |  72 ++-
 .../xmodule/modulestore/loc_mapper_store.py   |  38 +-
 .../lib/xmodule/xmodule/modulestore/mixed.py  | 321 +++++++++--
 .../xmodule/xmodule/modulestore/mongo/base.py |  53 +-
 .../xmodule/modulestore/mongo/draft.py        |  21 +-
 .../xmodule/modulestore/split_mongo/split.py  |  48 +-
 .../xmodule/modulestore/tests/django_utils.py |   7 +-
 .../xmodule/modulestore/tests/factories.py    |   7 +-
 .../modulestore/tests/persistent_factories.py |  42 +-
 .../modulestore/tests/test_location_mapper.py |  19 +-
 .../tests/test_mixed_modulestore.py           | 503 ++++++++++--------
 .../xmodule/modulestore/tests/test_mongo.py   |   1 +
 .../lib/xmodule/xmodule/tests/test_import.py  |   4 +-
 .../xmodule/xmodule/tests/xml/factories.py    |   2 +-
 common/lib/xmodule/xmodule/x_module.py        |  16 -
 lms/djangoapps/branding/tests.py              |   2 +-
 lms/envs/acceptance.py                        |   2 +-
 lms/envs/common.py                            |   3 +-
 23 files changed, 779 insertions(+), 406 deletions(-)

diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py
index f41a5fe046..300e338cbe 100644
--- a/cms/djangoapps/contentstore/tests/test_contentstore.py
+++ b/cms/djangoapps/contentstore/tests/test_contentstore.py
@@ -1003,7 +1003,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         # We had a bug where orphaned draft nodes caused export to fail. This is here to cover that case.
         vertical.location = mongo.draft.as_draft(vertical.location.replace(name='no_references'))
 
-        draft_store.save_xmodule(vertical)
+        draft_store.update_item(vertical, allow_not_found=True)
         orphan_vertical = draft_store.get_item(vertical.location)
         self.assertEqual(orphan_vertical.location.name, 'no_references')
 
@@ -1020,13 +1020,14 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
 
         # now create a new/different private (draft only) vertical
         vertical.location = mongo.draft.as_draft(Location(['i4x', 'edX', 'toy', 'vertical', 'a_private_vertical', None]))
-        draft_store.save_xmodule(vertical)
+        draft_store.update_item(vertical, allow_not_found=True)
         private_vertical = draft_store.get_item(vertical.location)
         vertical = None  # blank out b/c i destructively manipulated its location 2 lines above
 
         # add the new private to list of children
-        sequential = module_store.get_item(Location(['i4x', 'edX', 'toy',
-                                           'sequential', 'vertical_sequential', None]))
+        sequential = module_store.get_item(
+            Location('i4x', 'edX', 'toy', 'sequential', 'vertical_sequential', None)
+        )
         private_location_no_draft = private_vertical.location.replace(revision=None)
         sequential.children.append(private_location_no_draft.url())
         module_store.update_item(sequential, self.user.id)
diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py
index c1338d6230..4efc388d08 100644
--- a/cms/djangoapps/contentstore/views/component.py
+++ b/cms/djangoapps/contentstore/views/component.py
@@ -23,7 +23,7 @@ from xblock.exceptions import NoSuchHandlerError
 from xblock.fields import Scope
 from xblock.plugin import PluginMissingError
 from xblock.runtime import Mixologist
-from xmodule.x_module import prefer_xmodules
+from xmodule.modulestore import prefer_xmodules
 
 from lms.lib.xblock.runtime import unquote_slashes
 
@@ -370,6 +370,6 @@ def component_handler(request, usage_id, handler, suffix=''):
         log.info("XBlock %s attempted to access missing handler %r", descriptor, handler, exc_info=True)
         raise Http404
 
-    modulestore().save_xmodule(descriptor)
+    modulestore().update_item(descriptor)
 
     return webob_to_django_response(resp)
diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py
index d9734773b0..a0e1bfb164 100644
--- a/cms/djangoapps/contentstore/views/item.py
+++ b/cms/djangoapps/contentstore/views/item.py
@@ -521,8 +521,8 @@ def orphan_handler(request, tag=None, package_id=None, branch=None, version_guid
     if request.method == 'DELETE':
         if request.user.is_staff:
             items = modulestore().get_orphans(old_location, 'draft')
-            for item in items:
-                modulestore('draft').delete_item(item, delete_all_versions=True)
+            for itemloc in items:
+                modulestore('draft').delete_item(itemloc, delete_all_versions=True)
             return JsonResponse({'deleted': items})
         else:
             raise PermissionDenied()
diff --git a/cms/envs/common.py b/cms/envs/common.py
index 902bd59c1e..0f8d5c9e0d 100644
--- a/cms/envs/common.py
+++ b/cms/envs/common.py
@@ -34,7 +34,8 @@ from path import path
 from lms.lib.xblock.mixin import LmsBlockMixin
 from cms.lib.xblock.mixin import CmsBlockMixin
 from xmodule.modulestore.inheritance import InheritanceMixin
-from xmodule.x_module import XModuleMixin, only_xmodules
+from xmodule.modulestore import only_xmodules
+from xmodule.x_module import XModuleMixin
 from dealer.git import git
 
 ############################ FEATURE CONFIGURATION #############################
@@ -220,7 +221,7 @@ XBLOCK_SELECT_FUNCTION = only_xmodules
 # either by uncommenting them here, or adding them to your private.py
 # You should also enable the ALLOW_ALL_ADVANCED_COMPONENTS feature flag, so that
 # xblocks can be added via advanced settings
-# from xmodule.x_module import prefer_xmodules
+# from xmodule.modulestore import prefer_xmodules
 # XBLOCK_SELECT_FUNCTION = prefer_xmodules
 
 ############################ SIGNAL HANDLERS ################################
diff --git a/cms/envs/test.py b/cms/envs/test.py
index 1e1adf49e4..2f329d60fe 100644
--- a/cms/envs/test.py
+++ b/cms/envs/test.py
@@ -16,6 +16,7 @@ from .common import *
 import os
 from path import path
 from warnings import filterwarnings
+from xmodule.modulestore import prefer_xmodules
 
 # Nose Test Runner
 TEST_RUNNER = 'django_nose.NoseTestSuiteRunner'
@@ -158,7 +159,6 @@ filterwarnings('ignore', message='No request passed to the backend, unable to ra
 
 
 ################################# XBLOCK ######################################
-from xmodule.x_module import prefer_xmodules
 XBLOCK_SELECT_FUNCTION = prefer_xmodules
 
 
diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py
index 8663d6d65b..d752b6dcae 100644
--- a/common/lib/xmodule/xmodule/modulestore/__init__.py
+++ b/common/lib/xmodule/xmodule/modulestore/__init__.py
@@ -7,11 +7,15 @@ import logging
 import re
 
 from collections import namedtuple
+import collections
 
 from abc import ABCMeta, abstractmethod
+from xblock.plugin import default_select
 
 from .exceptions import InvalidLocationError, InsufficientSpecificationError
 from xmodule.errortracker import make_error_tracker
+from xblock.runtime import Mixologist
+from xblock.core import XBlock
 
 log = logging.getLogger('edx.modulestore')
 
@@ -529,9 +533,75 @@ class ModuleStoreReadBase(ModuleStoreRead):
                 return c
         return None
 
+    def update_item(self, xblock, user_id=None, allow_not_found=False, force=False):
+        """
+        Update the given xblock's persisted repr. Pass the user's unique id which the persistent store
+        should save with the update if it has that ability.
+
+        :param allow_not_found: whether this method should raise an exception if the given xblock
+        has not been persisted before.
+        :param force: fork the structure and don't update the course draftVersion if there's a version
+        conflict (only applicable to version tracking and conflict detecting persistence stores)
+
+        :raises VersionConflictError: if package_id and version_guid given and the current
+        version head != version_guid and force is not True. (only applicable to version tracking stores)
+        """
+        raise NotImplementedError
+
+    def delete_item(self, location, user_id=None, delete_all_versions=False, delete_children=False, force=False):
+        """
+        Delete an item from persistence. Pass the user's unique id which the persistent store
+        should save with the update if it has that ability.
+
+        :param delete_all_versions: removes both the draft and published version of this item from
+        the course if using draft and old mongo. Split may or may not implement this.
+        :param force: fork the structure and don't update the course draftVersion if there's a version
+        conflict (only applicable to version tracking and conflict detecting persistence stores)
+
+        :raises VersionConflictError: if package_id and version_guid given and the current
+        version head != version_guid and force is not True. (only applicable to version tracking stores)
+        """
+        raise NotImplementedError
 
 class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite):
     '''
     Implement interface functionality that can be shared.
     '''
-    pass
+    def __init__(self, **kwargs):
+        super(ModuleStoreWriteBase, self).__init__(**kwargs)
+        # TODO: Don't have a runtime just to generate the appropriate mixin classes (cpennington)
+        # This is only used by partition_fields_by_scope, which is only needed because
+        # the split mongo store is used for item creation as well as item persistence
+        self.mixologist = Mixologist(self.xblock_mixins)
+
+    def partition_fields_by_scope(self, category, fields):
+        """
+        Return dictionary of {scope: {field1: val, ..}..} for the fields of this potential xblock
+
+        :param category: the xblock category
+        :param fields: the dictionary of {fieldname: value}
+        """
+        if fields is None:
+            return {}
+        cls = self.mixologist.mix(XBlock.load_class(category, select=prefer_xmodules))
+        result = collections.defaultdict(dict)
+        for field_name, value in fields.iteritems():
+            field = getattr(cls, field_name)
+            result[field.scope][field_name] = value
+        return result
+
+
+def only_xmodules(identifier, entry_points):
+    """Only use entry_points that are supplied by the xmodule package"""
+    from_xmodule = [entry_point for entry_point in entry_points if entry_point.dist.key == 'xmodule']
+
+    return default_select(identifier, from_xmodule)
+
+
+def prefer_xmodules(identifier, entry_points):
+    """Prefer entry_points from the xmodule package"""
+    from_xmodule = [entry_point for entry_point in entry_points if entry_point.dist.key == 'xmodule']
+    if from_xmodule:
+        return default_select(identifier, from_xmodule)
+    else:
+        return default_select(identifier, entry_points)
diff --git a/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py b/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py
index 1f2325e811..b67454865e 100644
--- a/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py
+++ b/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py
@@ -119,7 +119,8 @@ class LocMapperStore(object):
 
         return package_id
 
-    def translate_location(self, old_style_course_id, location, published=True, add_entry_if_missing=True):
+    def translate_location(self, old_style_course_id, location, published=True,
+                           add_entry_if_missing=True, passed_block_id=None):
         """
         Translate the given module location to a Locator. If the mapping has the run id in it, then you
         should provide old_style_course_id with that run id in it to disambiguate the mapping if there exists more
@@ -137,6 +138,8 @@ class LocMapperStore(object):
         :param add_entry_if_missing: a boolean as to whether to raise ItemNotFoundError or to create an entry if
         the course
         or block is not found in the map.
+        :param passed_block_id: what block_id to assign and save if none is found
+        (only if add_entry_if_missing)
 
         NOTE: unlike old mongo, draft branches contain the whole course; so, it applies to all category
         of locations including course.
@@ -158,7 +161,7 @@ class LocMapperStore(object):
                 self.create_map_entry(course_location)
                 entry = self.location_map.find_one(location_id)
             else:
-                raise ItemNotFoundError()
+                raise ItemNotFoundError(location)
         elif len(maps) == 1:
             entry = maps[0]
         else:
@@ -172,7 +175,9 @@ class LocMapperStore(object):
         block_id = entry['block_map'].get(self.encode_key_for_mongo(location.name))
         if block_id is None:
             if add_entry_if_missing:
-                block_id = self._add_to_block_map(location, location_id, entry['block_map'])
+                block_id = self._add_to_block_map(
+                    location, location_id, entry['block_map'], passed_block_id
+                )
             else:
                 raise ItemNotFoundError(location)
         elif isinstance(block_id, dict):
@@ -188,7 +193,7 @@ class LocMapperStore(object):
             elif add_entry_if_missing:
                 block_id = self._add_to_block_map(location, location_id, entry['block_map'])
             else:
-                raise ItemNotFoundError()
+                raise ItemNotFoundError(location)
         else:
             raise InvalidLocationError()
 
@@ -297,7 +302,7 @@ class LocMapperStore(object):
         maps = self.location_map.find(location_id)
         maps = list(maps)
         if len(maps) == 0:
-            raise ItemNotFoundError()
+            raise ItemNotFoundError(location)
         elif len(maps) == 1:
             entry = maps[0]
         else:
@@ -315,18 +320,19 @@ class LocMapperStore(object):
         else:
             return draft_course_locator
 
-    def _add_to_block_map(self, location, location_id, block_map):
+    def _add_to_block_map(self, location, location_id, block_map, block_id=None):
         '''add the given location to the block_map and persist it'''
-        if self._block_id_is_guid(location.name):
-            # This makes the ids more meaningful with a small probability of name collision.
-            # The downside is that if there's more than one course mapped to from the same org/course root
-            # the block ids will likely be out of sync and collide from an id perspective. HOWEVER,
-            # if there are few == org/course roots or their content is unrelated, this will work well.
-            block_id = self._verify_uniqueness(location.category + location.name[:3], block_map)
-        else:
-            # if 2 different category locations had same name, then they'll collide. Make the later
-            # mapped ones unique
-            block_id = self._verify_uniqueness(location.name, block_map)
+        if block_id is None:
+            if self._block_id_is_guid(location.name):
+                # This makes the ids more meaningful with a small probability of name collision.
+                # The downside is that if there's more than one course mapped to from the same org/course root
+                # the block ids will likely be out of sync and collide from an id perspective. HOWEVER,
+                # if there are few == org/course roots or their content is unrelated, this will work well.
+                block_id = self._verify_uniqueness(location.category + location.name[:3], block_map)
+            else:
+                # if 2 different category locations had same name, then they'll collide. Make the later
+                # mapped ones unique
+                block_id = self._verify_uniqueness(location.name, block_map)
         encoded_location_name = self.encode_key_for_mongo(location.name)
         block_map.setdefault(encoded_location_name, {})[location.category] = block_id
         self.location_map.update(location_id, {'$set': {'block_map': block_map}})
diff --git a/common/lib/xmodule/xmodule/modulestore/mixed.py b/common/lib/xmodule/xmodule/modulestore/mixed.py
index 6005638796..e95dcee5aa 100644
--- a/common/lib/xmodule/xmodule/modulestore/mixed.py
+++ b/common/lib/xmodule/xmodule/modulestore/mixed.py
@@ -5,15 +5,18 @@ In this way, courses can be served up both - say - XMLModuleStore or MongoModule
 
 """
 
+import re
+from importlib import import_module
+import logging
+from xblock.fields import Reference, ReferenceList, String
+
 from . import ModuleStoreWriteBase
 from xmodule.modulestore.django import create_modulestore_instance, loc_mapper
-import logging
-from xmodule.modulestore import Location
-from xblock.fields import Reference, ReferenceList, String
-from xmodule.modulestore.locator import CourseLocator, Locator, BlockUsageLocator
+from xmodule.modulestore import Location, SPLIT_MONGO_MODULESTORE_TYPE
+from xmodule.modulestore.locator import CourseLocator, BlockUsageLocator
 from xmodule.modulestore.exceptions import InsufficientSpecificationError, ItemNotFoundError
 from xmodule.modulestore.parsers import ALLOWED_ID_CHARS
-import re
+from uuid import uuid4
 
 log = logging.getLogger(__name__)
 
@@ -28,7 +31,8 @@ class MixedModuleStore(ModuleStoreWriteBase):
         Initialize a MixedModuleStore. Here we look into our passed in kwargs which should be a
         collection of other modulestore configuration informations
 
-        :param reference_type: either Location or Locator to indicate what type of reference this app
+        :param reference_type: either a class object such as Locator or Location or the fully
+        qualified dot-path to that class def to indicate what type of reference the app
         uses.
         """
         super(MixedModuleStore, self).__init__(**kwargs)
@@ -39,7 +43,14 @@ class MixedModuleStore(ModuleStoreWriteBase):
         if reference_type is None:
             log.warn("reference_type not specified in MixedModuleStore settings. %s",
                 "Will default temporarily to the to-be-deprecated Location.")
-        self.use_locations = (reference_type != 'Locator')
+            self.reference_type = Location
+        elif isinstance(reference_type, basestring):
+            module_path, _, class_name = reference_type.rpartition('.')
+            class_ = getattr(import_module(module_path), class_name)
+            self.reference_type = class_
+        else:
+            self.reference_type = reference_type
+
         if 'default' not in stores:
             raise Exception('Missing a default modulestore in the MixedModuleStore __init__ method.')
 
@@ -58,15 +69,24 @@ class MixedModuleStore(ModuleStoreWriteBase):
                 store['OPTIONS'],
                 i18n_service=i18n_service,
             )
+            # it would be better to have a notion of read-only rather than hardcode
+            # key name
+            if is_xml:
+                self.ensure_loc_maps_exist(key)
 
     def _get_modulestore_for_courseid(self, course_id):
         """
         For a given course_id, look in the mapping table and see if it has been pinned
         to a particular modulestore
         """
+        # TODO when this becomes a router capable of handling more than one r/w backend
+        # we'll need to generalize this to handle mappings from old Locations w/o full
+        # course_id in much the same way as loc_mapper().translate_location does.
         mapping = self.mappings.get(course_id, 'default')
         return self.modulestores[mapping]
 
+    # TODO move the location converters to a helper class which returns a converter object w/ 2
+    # methods: convert_xblock and convert_reference. Then have mixed get the converter and use it.
     def _locator_to_location(self, reference):
         """
         Convert the referenced locator to a location casting to and from a string as necessary
@@ -84,14 +104,16 @@ class MixedModuleStore(ModuleStoreWriteBase):
         stringify = isinstance(reference, basestring)
         if stringify:
             reference = Location(reference)
-        locator = loc_mapper().translate_location(course_id, reference, reference.revision == 'draft', True)
+        locator = loc_mapper().translate_location(course_id, reference, reference.revision == None, True)
         return unicode(locator) if stringify else locator
 
     def _incoming_reference_adaptor(self, store, course_id, reference):
         """
         Convert the reference to the type the persistence layer wants
         """
-        if issubclass(store.reference_type, Location if self.use_locations else Locator):
+        if reference is None:
+            return None
+        if issubclass(store.reference_type, self.reference_type):
             return reference
         if store.reference_type == Location:
             return self._locator_to_location(reference)
@@ -101,7 +123,9 @@ class MixedModuleStore(ModuleStoreWriteBase):
         """
         Convert the reference to the type the application wants
         """
-        if issubclass(store.reference_type, Location if self.use_locations else Locator):
+        if reference is None:
+            return None
+        if issubclass(store.reference_type, self.reference_type):
             return reference
         if store.reference_type == Location:
             return self._location_to_locator(course_id, reference)
@@ -111,6 +135,7 @@ class MixedModuleStore(ModuleStoreWriteBase):
         """
         Change all reference fields in this xblock to the type expected by the receiving layer
         """
+        xblock.location = adaptor(store, course_id, xblock.location)
         for field in xblock.fields.itervalues():
             if field.is_set_on(xblock):
                 if isinstance(field, Reference):
@@ -136,7 +161,7 @@ class MixedModuleStore(ModuleStoreWriteBase):
         Change all reference fields in this xblock to the type expected by the persistence layer
         """
         string_converter = self._get_string_converter(
-            course_id, store.reference_type, xblock.location
+            course_id, store.reference_type, xblock.scope_ids.usage_id
         )
         return self._xblock_adaptor_iterator(
             self._incoming_reference_adaptor, string_converter, store, course_id, xblock
@@ -147,7 +172,7 @@ class MixedModuleStore(ModuleStoreWriteBase):
         Change all reference fields in this xblock to the type expected by the persistence layer
         """
         string_converter = self._get_string_converter(
-            course_id, xblock.location.__class__, xblock.location
+            course_id, xblock.scope_ids.usage_id.__class__, xblock.scope_ids.usage_id
         )
         return self._xblock_adaptor_iterator(
             self._outgoing_reference_adaptor, string_converter, store, course_id, xblock
@@ -160,9 +185,7 @@ class MixedModuleStore(ModuleStoreWriteBase):
         Return a closure which finds and replaces all embedded links in a string field
         with the correct rewritten link for the target type
         """
-        if self.use_locations and reference_type == Location:
-            return lambda field, xblock: None
-        if not self.use_locations and issubclass(reference_type, Locator):
+        if issubclass(self.reference_type, reference_type):
             return lambda field, xblock: None
         if isinstance(from_base_addr, Location):
             def mapper(found_id):
@@ -189,6 +212,21 @@ class MixedModuleStore(ModuleStoreWriteBase):
             field.write_to(xblock, value)
         return converter
 
+    def ensure_loc_maps_exist(self, store_name):
+        """
+        Ensure location maps exist for every course in the modulestore whose
+        name is the given name (mostly used for 'xml'). It creates maps for any
+        missing ones.
+
+        NOTE: will only work if the given store is Location based. If it's not,
+        it raises NotImplementedError
+        """
+        store = self.modulestores[store_name]
+        if store.reference_type != Location:
+            raise NotImplementedError(u"Cannot create maps from %s", store.reference_type)
+        for course in store.get_courses():
+            loc_mapper().translate_location(course.location.course_id, course.location)
+
     def has_item(self, course_id, reference):
         """
         Does the course include the xblock who's id is reference?
@@ -232,9 +270,8 @@ class MixedModuleStore(ModuleStoreWriteBase):
             raise Exception("Must pass in a course_id when calling get_items()")
 
         store = self._get_modulestore_for_courseid(course_id or getattr(location, 'package_id'))
-        # translate won't work w/ missing fields so work around it
-        if store.reference_type == Location:
-            if not self.use_locations:
+        if not issubclass(self.reference_type, store.reference_type):
+            if store.reference_type == Location:
                 if getattr(location, 'block_id', False):
                     location = self._incoming_reference_adaptor(store, course_id, location)
                 else:
@@ -245,26 +282,31 @@ class MixedModuleStore(ModuleStoreWriteBase):
                         category=qualifiers.get('category', None),
                         name=None
                     )
-        else:
-            if self.use_locations:
+            else:
                 if not isinstance(location, Location):
                     location = Location(location)
                 try:
                     location.ensure_fully_specified()
                     location = loc_mapper().translate_location(
-                        course_id, location, location.revision == 'published', True
+                        course_id, location, location.revision == None, True
                     )
                 except InsufficientSpecificationError:
                     # construct the Locator by hand
                     if location.category is not None and qualifiers.get('category', False):
                         qualifiers['category'] = location.category
                     location = loc_mapper().translate_location_to_course_locator(
-                        course_id, location, location.revision == 'published'
+                        course_id, location, location.revision == None
                     )
         xblocks = store.get_items(location, course_id, depth, qualifiers)
         xblocks = [self._outgoing_xblock_adaptor(store, course_id, xblock) for xblock in xblocks]
         return xblocks
 
+    def _get_course_id_from_course_location(self, course_location):
+        """
+        Get the proper course_id based on the type of course_location
+        """
+        return getattr(course_location, 'course_id', None) or getattr(course_location, 'package_id', None)
+
     def get_courses(self):
         '''
         Returns a list containing the top level XModuleDescriptors of the courses
@@ -272,20 +314,31 @@ class MixedModuleStore(ModuleStoreWriteBase):
         '''
         courses = []
         for key in self.modulestores:
-            store_courses = self.modulestores[key].get_courses()
+            store = self.modulestores[key]
+            store_courses = store.get_courses()
             # If the store has not been labeled as 'default' then we should
             # only surface courses that have a mapping entry, for example the XMLModuleStore will
             # slurp up anything that is on disk, however, we don't want to surface those to
             # consumers *unless* there is an explicit mapping in the configuration
+            # TODO obviously this filtering only applies to filebased stores
             if key != 'default':
                 for course in store_courses:
+                    course_id = self._get_course_id_from_course_location(course.location)
                     # make sure that the courseId is mapped to the store in question
-                    if key == self.mappings.get(course.location.course_id, 'default'):
-                        courses = courses + ([course])
+                    if key == self.mappings.get(course_id, 'default'):
+                        courses.append(
+                            self._outgoing_reference_adaptor(store, course_id, course.location)
+                        )
             else:
                 # if we're the 'default' store provider, then we surface all courses hosted in
                 # that store provider
-                courses = courses + (store_courses)
+                store_courses = [
+                    self._outgoing_reference_adaptor(
+                        store, self._get_course_id_from_course_location(course.location), course.location
+                    )
+                    for course in store_courses
+                ]
+                courses = courses + store_courses
 
         return courses
 
@@ -302,7 +355,7 @@ class MixedModuleStore(ModuleStoreWriteBase):
             # translate won't work w/ missing fields so work around it
             if store.reference_type == Location:
                 # takes the course_id: figure out if this is old or new style
-                if not self.use_locations:
+                if not issubclass(store.reference_type, self.reference_type):
                     if isinstance(course_id, basestring):
                         course_id = CourseLocator(package_id=course_id, branch='published')
                     course_location = loc_mapper().translate_locator_to_location(course_id, get_course=True)
@@ -333,8 +386,10 @@ class MixedModuleStore(ModuleStoreWriteBase):
         store = self._get_modulestore_for_courseid(course_id)
         decoded_ref = self._incoming_reference_adaptor(store, course_id, location)
         parents = store.get_parent_locations(decoded_ref, course_id)
-        return [self._outgoing_reference_adaptor(store, course_id, reference)
-                for reference in parents]
+        return [
+            self._outgoing_reference_adaptor(store, course_id, reference)
+            for reference in parents
+        ]
 
     def get_modulestore_type(self, course_id):
         """
@@ -367,16 +422,187 @@ class MixedModuleStore(ModuleStoreWriteBase):
             errs.update(store.get_errored_courses())
         return errs
 
+    def _get_course_id_from_block(self, block, store):
+        """
+        Get the course_id from the block or from asking its store. Expensive.
+        """
+        if block.course_id is not None:
+            return block.course_id
+        try:
+            course = store._get_course_for_item(block.scope_ids.usage_id)
+            if course:
+                return course.scope_ids.usage_id.course_id
+        except:  # sorry, that method just raises vanilla Exception
+            pass
+
+    def _infer_course_id_try(self, location):
+        """
+        Create, Update, Delete operations don't require a fully-specified course_id, but
+        there's no complete & sound general way to compute the course_id except via the
+        proper modulestore. This method attempts several sound but not complete methods.
+        :param location: an old style Location
+        """
+        if location.category == 'course':  # easiest case
+            return location.course_id
+        # try finding in loc_mapper
+        try:
+            locator = loc_mapper().translate_location_to_course_locator(None, location)
+            location = loc_mapper().translate_locator_to_location(locator, get_course=True)
+            return location.course_id
+        except ItemNotFoundError:
+            pass
+        # expensive query against all location-based modulestores to look for location.
+        for store in self.modulestores.itervalues():
+            if isinstance(location, store.reference_type):
+                try:
+                    block = store.get_item(location)
+                    course_id = self._get_course_id_from_block(block, store)
+                    if course_id is not None:
+                        return course_id
+                except NotImplementedError:
+                    blocks = store.get_items(location)
+                    if len(blocks) == 1:
+                        block = blocks[0]
+                        if block.course_id is not None:
+                            return block.course_id
+                except ItemNotFoundError:
+                    pass
+        # if we get here, it must be in a Locator based store, but we won't be able to find
+        # it.
+        return None
+
+    def create_course(self, course_location, user_id=None, store_name='default', **kwargs):
+        """
+        Creates and returns the course. It creates a loc map from the course_location to
+        the new one (if provided as id_root).
+
+        NOTE: course_location must be a Location not
+        a Locator until we no longer need to do loc mapping.
+
+        NOTE: unlike the other mixed modulestore methods, this does not adapt its argument
+        to the persistence store but requires its caller to know what the persistence store
+        wants for args. It does not translate any references on the way in; so, don't
+        pass children or other reference fields here.
+        It does, however, adapt the xblock on the way out to the app's
+        reference_type
+
+        :returns: course xblock
+        """
+        store = self.modulestores[store_name]
+        if not hasattr(store, 'create_course'):
+            raise NotImplementedError(u"Cannot create a course on store %s", store_name)
+        if store.get_modulestore_type(course_location.course_id) == SPLIT_MONGO_MODULESTORE_TYPE:
+            org = kwargs.pop('org', course_location.org)
+            pretty_id = kwargs.pop('pretty_id', None)
+            # TODO rename id_root to package_id for consistency. It's too confusing
+            id_root = kwargs.pop('id_root', u"{0.org}.{0.course}.{0.name}".format(course_location))
+            course = store.create_course(
+                org, pretty_id, user_id, id_root=id_root, master_branch=course_location.revision or 'published',
+                **kwargs
+            )
+            block_map = {course_location.name: {'course': course.location.block_id}}
+            # NOTE: course.location will be a Locator not == course_location
+            loc_mapper().create_map_entry(
+                course_location, course.location.package_id, block_map=block_map
+            )
+        else:  # assume mongo
+            course = store.create_course(course_location, **kwargs)
+            loc_mapper().translate_location(course_location.course_id, course_location)
+
+        return self._outgoing_xblock_adaptor(store, course_location.course_id, course)
+
+    def create_item(self, course_or_parent_loc, category, user_id=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).
+
+        Adds an entry to the loc map using the kwarg location if provided (must be a
+        Location if provided) or block_id and category if provided.
+
+        :param course_or_parent_loc: will be translated appropriately to the course's store.
+        Can be a course_id (org/course/run), CourseLocator, Location, or BlockUsageLocator.
+        """
+        # find the store for the course
+        if self.reference_type == Location:
+            if hasattr(course_or_parent_loc, 'tag'):
+                course_id = self._infer_course_id_try(course_or_parent_loc)
+            else:
+                course_id = course_or_parent_loc
+        else:
+            course_id = course_or_parent_loc.package_id
+        store = self._get_modulestore_for_courseid(course_id)
+
+        location = kwargs.pop('location', None)
+        # invoke its create_item
+        if store.reference_type == Location:
+            # convert parent loc if it's legit
+            block_id = kwargs.pop('block_id', uuid4().hex)
+            if isinstance(course_or_parent_loc, basestring):
+                parent_loc = None
+                if location is None:
+                    locn_dict = Location.parse_course_id(course_id)
+                    locn_dict['category'] = category
+                    locn_dict['name'] = block_id
+                    location = Location(locn_dict)
+            else:
+                parent_loc = self._incoming_reference_adaptor(store, course_id, course_or_parent_loc)
+                # must have a legitimate location, compute if appropriate
+                if location is None:
+                    location = parent_loc.replace(category=category, name=block_id)
+            # do the actual creation
+            xblock = store.create_and_save_xmodule(location, **kwargs)
+            # add the loc mapping
+            loc_mapper().translate_location(course_id, location)
+            # 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.url())
+                store.update_item(parent)
+        else:
+            if isinstance(course_or_parent_loc, basestring): # course_id
+                old_course_id = course_or_parent_loc
+                course_or_parent_loc = loc_mapper().translate_location_to_course_locator(
+                    course_or_parent_loc, None
+                )
+            elif isinstance(course_or_parent_loc, CourseLocator):
+                old_course_loc = loc_mapper().translate_locator_to_location(
+                    course_or_parent_loc, get_course=True
+                )
+                old_course_id = old_course_loc.course_id
+            else:  # it's a Location
+                old_course_id = course_id
+                course_or_parent_loc = self._location_to_locator(course_id, course_or_parent_loc)
+            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)
+            if location is None:
+                locn_dict = Location.parse_course_id(old_course_id)
+                locn_dict['category'] = category
+                locn_dict['name'] = xblock.location.block_id
+                location = Location(locn_dict)
+            # map location.name to xblock.location.block_id
+            loc_mapper().translate_location(
+                old_course_id, location, passed_block_id=xblock.location.block_id
+            )
+        return xblock
+
     def update_item(self, xblock, user_id, allow_not_found=False):
         """
         Update the xblock persisted to be the same as the given for all types of fields
         (content, children, and metadata) attribute the change to the given user.
         """
-        if self.use_locations:
-            raise NotImplementedError
-
-        locator = xblock.location
-        course_id = locator.package_id
+        if self.reference_type == Location:
+            course_id = xblock.course_id
+            if course_id is None:
+                course_id = self._infer_course_id_try(xblock.scope_ids.usage_id)
+                if course_id is None:
+                    raise ItemNotFoundError(u"Cannot find modulestore for %s", xblock.scope_ids.usage_id)
+        else:
+            locator = xblock.scope_ids.usage_id
+            course_id = locator.package_id
         store = self._get_modulestore_for_courseid(course_id)
 
         # if an xblock, convert its contents to correct addr scheme
@@ -385,13 +611,28 @@ class MixedModuleStore(ModuleStoreWriteBase):
 
         return self._outgoing_xblock_adaptor(store, course_id, xblock)
 
-    def delete_item(self, location, **kwargs):
+    def delete_item(self, location, user_id=None):
         """
         Delete the given item from persistence.
         """
-        if self.use_locations:
-            raise NotImplementedError
+        if self.reference_type == Location:
+            course_id = self._infer_course_id_try(location)
+            if course_id is None:
+                raise ItemNotFoundError(u"Cannot find modulestore for %s", location)
+        else:
+            course_id = location.package_id
+        store = self._get_modulestore_for_courseid(course_id)
+
+        decoded_ref = self._incoming_reference_adaptor(store, course_id, location)
+        return store.delete_item(decoded_ref, user_id=user_id)
+
+    def close_all_connections(self):
+        """
+        Close all db connections
+        """
+        for mstore in self.modulestores.itervalues():
+            if hasattr(mstore, 'database'):
+                mstore.database.connection.close()
+            elif hasattr(mstore, 'db'):
+                mstore.db.connection.close()
 
-        store = self._get_modulestore_for_courseid(location.package_id)
-        decoded_ref = self._incoming_reference_adaptor(store, location.package_id, location)
-        return store.delete_item(decoded_ref, **kwargs)
diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py
index 6ee52a8b01..4dcf3b6739 100644
--- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py
+++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py
@@ -625,7 +625,16 @@ class MongoModuleStore(ModuleStoreWriteBase):
         modules = self._load_items(list(items), depth)
         return modules
 
-    def create_xmodule(self, location, definition_data=None, metadata=None, system=None):
+    def create_course(self, location, definition_data=None, metadata=None, runtime=None):
+        """
+        Create a course with the given location. The location category must be 'course'.
+        """
+        if location.category != 'course':
+            raise ValueError(u"Course roots must be of category 'course': {}".format(unicode(location)))
+        return self.create_and_save_xmodule(location, definition_data, metadata, runtime)
+
+    def create_xmodule(self, location, definition_data=None, metadata=None, system=None,
+                       fields={}):
         """
         Create the new xmodule but don't save it. Returns the new module.
 
@@ -672,36 +681,18 @@ class MongoModuleStore(ModuleStoreWriteBase):
             ScopeIds(None, location.category, location, location),
             dbmodel,
         )
+        for key, value in fields.iteritems():
+            xmodule[key] = value
         # decache any pending field settings from init
         xmodule.save()
         return xmodule
 
-    def save_xmodule(self, xmodule):
-        """
-        Save the given xmodule (will either create or update based on whether id already exists).
-        Pulls out the data definition v metadata v children locally but saves it all.
-
-        :param xmodule:
-        """
-        # Save any changes to the xmodule to the MongoKeyValueStore
-        xmodule.save()
-        self.collection.save({
-            '_id': namedtuple_to_son(xmodule.location),
-            'metadata': own_metadata(xmodule),
-            'definition': {
-                'data': xmodule.get_explicitly_set_fields_by_scope(Scope.content),
-                'children': xmodule.children if xmodule.has_children else []
-            }
-        })
-        # recompute (and update) the metadata inheritance tree which is cached
-        self.refresh_cached_metadata_inheritance_tree(xmodule.location)
-        self.fire_updated_modulestore_signal(get_course_id_no_run(xmodule.location), xmodule.location)
-
-    def create_and_save_xmodule(self, location, definition_data=None, metadata=None, system=None):
+    def create_and_save_xmodule(self, location, definition_data=None, metadata=None, system=None,
+                                fields={}):
         """
         Create the new xmodule and save it. Does not return the new module because if the caller
         will insert it as a child, it's inherited metadata will completely change. The difference
-        between this and just doing create_xmodule and save_xmodule is this ensures static_tabs get
+        between this and just doing create_xmodule and update_item is this ensures static_tabs get
         pointed to by the course.
 
         :param location: a Location--must have a category
@@ -711,9 +702,9 @@ class MongoModuleStore(ModuleStoreWriteBase):
         """
         # differs from split mongo in that I believe most of this logic should be above the persistence
         # layer but added it here to enable quick conversion. I'll need to reconcile these.
-        new_object = self.create_xmodule(location, definition_data, metadata, system)
+        new_object = self.create_xmodule(location, definition_data, metadata, system, fields)
         location = new_object.location
-        self.save_xmodule(new_object)
+        self.update_item(new_object, allow_not_found=True)
 
         # VS[compat] cdodge: This is a hack because static_tabs also have references from the course module, so
         # if we add one then we need to also add it to the policy information (i.e. metadata)
@@ -728,9 +719,9 @@ class MongoModuleStore(ModuleStoreWriteBase):
                 'url_slug': new_object.location.name
             })
             course.tabs = existing_tabs
-            # Save any changes to the course to the MongoKeyValueStore
-            course.save()
-            self.update_item(course, '**replace_user**')
+            self.update_item(course)
+
+        return new_object
 
     def fire_updated_modulestore_signal(self, course_id, location):
         """
@@ -787,7 +778,7 @@ class MongoModuleStore(ModuleStoreWriteBase):
         if result['n'] == 0:
             raise ItemNotFoundError(location)
 
-    def update_item(self, xblock, user, allow_not_found=False):
+    def update_item(self, xblock, user=None, allow_not_found=False):
         """
         Update the persisted version of xblock to reflect its current values.
 
@@ -861,7 +852,7 @@ class MongoModuleStore(ModuleStoreWriteBase):
         location = Location.ensure_fully_specified(location)
         items = self.collection.find({'definition.children': location.url()},
                                      {'_id': True})
-        return [i['_id'] for i in items]
+        return [Location(i['_id']) for i in items]
 
     def get_modulestore_type(self, course_id):
         """
diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py
index 5956b00513..99a10c6e4a 100644
--- a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py
+++ b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py
@@ -106,21 +106,6 @@ class DraftModuleStore(MongoModuleStore):
             raise InvalidVersionError(location)
         return super(DraftModuleStore, self).create_xmodule(draft_loc, definition_data, metadata, system)
 
-    def save_xmodule(self, xmodule):
-        """
-        Save the given xmodule (will either create or update based on whether id already exists).
-        Pulls out the data definition v metadata v children locally but saves it all.
-
-        :param xmodule:
-        """
-        orig_location = xmodule.location
-
-        xmodule.location = as_draft(orig_location)
-        try:
-            super(DraftModuleStore, self).save_xmodule(xmodule)
-        finally:
-            xmodule.location = orig_location
-
     def get_items(self, location, course_id=None, depth=0, qualifiers=None):
         """
         Returns a list of XModuleDescriptor instances for the items
@@ -159,7 +144,7 @@ class DraftModuleStore(MongoModuleStore):
         if draft_location.category in DIRECT_ONLY_CATEGORIES:
             raise InvalidVersionError(source_location)
         if not original:
-            raise ItemNotFoundError
+            raise ItemNotFoundError(source_location)
         original['_id'] = namedtuple_to_son(draft_location)
         try:
             self.collection.insert(original)
@@ -171,7 +156,7 @@ class DraftModuleStore(MongoModuleStore):
 
         return self._load_items([original])[0]
 
-    def update_item(self, xblock, user, allow_not_found=False):
+    def update_item(self, xblock, user=None, allow_not_found=False):
         """
         Save the current values to persisted version of the xblock
 
@@ -187,7 +172,7 @@ class DraftModuleStore(MongoModuleStore):
                 raise
 
         xblock.location = draft_loc
-        super(DraftModuleStore, self).update_item(xblock, user)
+        super(DraftModuleStore, self).update_item(xblock, user, allow_not_found)
         # don't allow locations to truly represent themselves as draft outside of this file
         xblock.location = as_published(xblock.location)
 
diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py
index 2d717edd45..9ac0533eb9 100644
--- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py
+++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py
@@ -51,14 +51,13 @@ import logging
 import re
 from importlib import import_module
 from path import path
-import collections
 import copy
 from pytz import UTC
 
 from xmodule.errortracker import null_error_tracker
-from xmodule.x_module import prefer_xmodules
 from xmodule.modulestore.locator import (
-    BlockUsageLocator, DefinitionLocator, CourseLocator, VersionTree, LocalId, Locator
+    BlockUsageLocator, DefinitionLocator, CourseLocator, VersionTree,
+    LocalId, Locator
 )
 from xmodule.modulestore.exceptions import InsufficientSpecificationError, VersionConflictError, DuplicateItemError
 from xmodule.modulestore import inheritance, ModuleStoreWriteBase, Location, SPLIT_MONGO_MODULESTORE_TYPE
@@ -67,7 +66,6 @@ from ..exceptions import ItemNotFoundError
 from .definition_lazy_loader import DefinitionLazyLoader
 from .caching_descriptor_system import CachingDescriptorSystem
 from xblock.fields import Scope
-from xblock.runtime import Mixologist
 from bson.objectid import ObjectId
 from xmodule.modulestore.split_mongo.mongo_connection import MongoConnection
 from xblock.core import XBlock
@@ -132,11 +130,6 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
         self.render_template = render_template
         self.i18n_service = i18n_service
 
-        # TODO: Don't have a runtime just to generate the appropriate mixin classes (cpennington)
-        # This is only used by _partition_fields_by_scope, which is only needed because
-        # the split mongo store is used for item creation as well as item persistence
-        self.mixologist = Mixologist(self.xblock_mixins)
-
     def cache_items(self, system, base_block_ids, depth=0, lazy=True):
         '''
         Handles caching of items once inheritance and any other one time
@@ -759,7 +752,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
         index_entry = self._get_index_if_valid(course_or_parent_locator, force, continue_version)
         structure = self._lookup_course(course_or_parent_locator)['structure']
 
-        partitioned_fields = self._partition_fields_by_scope(category, fields)
+        partitioned_fields = self.partition_fields_by_scope(category, fields)
         new_def_data = partitioned_fields.get(Scope.content, {})
         # persist the definition if persisted != passed
         if (definition_locator is None or isinstance(definition_locator.definition_id, LocalId)):
@@ -822,14 +815,19 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
         if index_entry is not None:
             if not continue_version:
                 self._update_head(index_entry, course_or_parent_locator.branch, new_id)
-            course_parent = course_or_parent_locator.as_course_locator()
+            item_loc = BlockUsageLocator(
+                package_id=course_or_parent_locator.package_id,
+                branch=course_or_parent_locator.branch,
+                block_id=new_block_id,
+            )
         else:
-            course_parent = None
+            item_loc = BlockUsageLocator(
+                block_id=new_block_id,
+                version_guid=new_id,
+            )
 
         # reconstruct the new_item from the cache
-        return self.get_item(BlockUsageLocator(package_id=course_parent,
-                                               block_id=new_block_id,
-                                               version_guid=new_id))
+        return self.get_item(item_loc)
 
     def create_course(
         self, org, prettyid, user_id, id_root=None, fields=None,
@@ -867,7 +865,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
         provide any fields overrides, see above). if not provided, will create a mostly empty course
         structure with just a category course root xblock.
         """
-        partitioned_fields = self._partition_fields_by_scope(root_category, fields)
+        partitioned_fields = self.partition_fields_by_scope(root_category, fields)
         block_fields = partitioned_fields.setdefault(Scope.settings, {})
         if Scope.children in partitioned_fields:
             block_fields.update(partitioned_fields[Scope.children])
@@ -1287,7 +1285,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
         if index is None:
             raise ItemNotFoundError(package_id)
         # this is the only real delete in the system. should it do something else?
-        log.info("deleting course from split-mongo: %s", package_id)
+        log.info(u"deleting course from split-mongo: %s", package_id)
         self.db_connection.delete_course_index(index['_id'])
 
     def get_errored_courses(self):
@@ -1494,22 +1492,6 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
         index_entry['versions'][branch] = new_id
         self.db_connection.update_course_index(index_entry)
 
-    def _partition_fields_by_scope(self, category, fields):
-        """
-        Return dictionary of {scope: {field1: val, ..}..} for the fields of this potential xblock
-
-        :param category: the xblock category
-        :param fields: the dictionary of {fieldname: value}
-        """
-        if fields is None:
-            return {}
-        cls = self.mixologist.mix(XBlock.load_class(category, select=self.xblock_select))
-        result = collections.defaultdict(dict)
-        for field_name, value in fields.iteritems():
-            field = getattr(cls, field_name)
-            result[field.scope][field_name] = value
-        return result
-
     def _filter_special_fields(self, fields):
         """
         Remove any fields which split or its kvs computes or adds but does not want persisted.
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py
index 71887629c4..ef30e96add 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py
@@ -33,7 +33,7 @@ def mixed_store_config(data_dir, mappings):
             'ENGINE': 'xmodule.modulestore.mixed.MixedModuleStore',
             'OPTIONS': {
                 'mappings': mappings,
-                'reference_type': 'Location',
+                'reference_type': 'xmodule.modulestore.Location',
                 'stores': {
                     'default': mongo_config['default'],
                     'xml': xml_config['default']
@@ -220,12 +220,17 @@ class ModuleStoreTestCase(TestCase):
         store = editable_modulestore()
         if hasattr(store, 'collection'):
             store.collection.drop()
+            store.db.connection.close()
+        elif hasattr(store, 'close_all_connections'):
+            store.close_all_connections()
         if contentstore().fs_files:
             db = contentstore().fs_files.database
             db.connection.drop_database(db)
+            db.connection.close()
         location_mapper = loc_mapper()
         if location_mapper.db:
             location_mapper.location_map.drop()
+            location_mapper.db.connection.close()
 
     @classmethod
     def setUpClass(cls):
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/factories.py b/common/lib/xmodule/xmodule/modulestore/tests/factories.py
index 4788c8a1d1..6fe138e05c 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/factories.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/factories.py
@@ -2,8 +2,7 @@ from factory import Factory, lazy_attribute_sequence, lazy_attribute
 from factory.containers import CyclicDefinitionError
 from uuid import uuid4
 
-from xmodule.modulestore import Location
-from xmodule.x_module import prefer_xmodules
+from xmodule.modulestore import Location, prefer_xmodules
 from xblock.core import XBlock
 
 
@@ -58,7 +57,7 @@ class CourseFactory(XModuleFactory):
             setattr(new_course, k, v)
 
         # Update the data in the mongo datastore
-        store.save_xmodule(new_course)
+        store.update_item(new_course)
         return new_course
 
 
@@ -159,7 +158,7 @@ class ItemFactory(XModuleFactory):
             setattr(module, attr, val)
         module.save()
 
-        store.save_xmodule(module)
+        store.update_item(module)
 
         if 'detached' not in module._class_tags:
             parent.children.append(location.url())
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/persistent_factories.py b/common/lib/xmodule/xmodule/modulestore/tests/persistent_factories.py
index 3031990974..b5cce6be6c 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/persistent_factories.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/persistent_factories.py
@@ -1,13 +1,23 @@
-from xmodule.modulestore.django import modulestore
 from xmodule.course_module import CourseDescriptor
 from xmodule.x_module import XModuleDescriptor
 import factory
+from factory.helpers import lazy_attribute
 
 
-# [dhm] I'm not sure why we're using factory_boy if we're not following its pattern. If anyone
-# assumes they can call build, it will completely fail, for example.
-# pylint: disable=W0232
-class PersistentCourseFactory(factory.Factory):
+class SplitFactory(factory.Factory):
+    """
+    Abstracted superclass which defines modulestore so that there's no dependency on django
+    if the caller passes modulestore in kwargs
+    """
+    @lazy_attribute
+    def modulestore(self):
+        # Delayed import so that we only depend on django if the caller
+        # hasn't provided their own modulestore
+        from xmodule.modulestore.django import modulestore
+        return modulestore('split')
+
+
+class PersistentCourseFactory(SplitFactory):
     """
     Create a new course (not a new version of a course, but a whole new index entry).
 
@@ -23,12 +33,15 @@ class PersistentCourseFactory(factory.Factory):
 
     # pylint: disable=W0613
     @classmethod
-    def _create(cls, target_class, org='testX', prettyid='999', user_id='test_user', master_branch='draft', **kwargs):
+    def _create(cls, target_class, org='testX', prettyid='999', user_id='test_user',
+                master_branch='draft', id_root=None, **kwargs):
 
+        modulestore = kwargs.pop('modulestore')
+        root_block_id = kwargs.pop('root_block_id', 'course')
         # Write the data to the mongo datastore
-        new_course = modulestore('split').create_course(
-            org, prettyid, user_id, fields=kwargs, id_root=prettyid,
-            master_branch=master_branch)
+        new_course = modulestore.create_course(
+            org, prettyid, user_id, fields=kwargs, id_root=id_root or prettyid,
+            master_branch=master_branch, root_block_id=root_block_id)
 
         return new_course
 
@@ -37,7 +50,7 @@ class PersistentCourseFactory(factory.Factory):
         raise NotImplementedError()
 
 
-class ItemFactory(factory.Factory):
+class ItemFactory(SplitFactory):
     FACTORY_FOR = XModuleDescriptor
 
     display_name = factory.LazyAttributeSequence(lambda o, n: "{} {}".format(o.category, n))
@@ -45,7 +58,8 @@ class ItemFactory(factory.Factory):
     # pylint: disable=W0613
     @classmethod
     def _create(cls, target_class, parent_location, category='chapter',
-        user_id='test_user', definition_locator=None, **kwargs):
+        user_id='test_user', block_id=None, definition_locator=None, force=False,
+        continue_version=False, **kwargs):
         """
         passes *kwargs* as the new item's field values:
 
@@ -55,8 +69,10 @@ class ItemFactory(factory.Factory):
 
         :param definition_locator (optional): the DescriptorLocator for the definition this uses or branches
         """
-        return modulestore('split').create_item(
-            parent_location, category, user_id, definition_locator, fields=kwargs
+        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
         )
 
     @classmethod
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py b/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py
index 0848e5c707..7a878cc9f1 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py
@@ -12,11 +12,11 @@ from xmodule.modulestore.loc_mapper_store import LocMapperStore
 from mock import Mock
 
 
-class TestLocationMapper(unittest.TestCase):
+class LocMapperSetupSansDjango(unittest.TestCase):
     """
-    Test the location to locator mapper
+    Create and destroy a loc mapper for each test
     """
-
+    loc_store = None
     def setUp(self):
         modulestore_options = {
             'host': 'localhost',
@@ -27,14 +27,19 @@ class TestLocationMapper(unittest.TestCase):
         cache_standin = TrivialCache()
         self.instrumented_cache = Mock(spec=cache_standin, wraps=cache_standin)
         # pylint: disable=W0142
-        TestLocationMapper.loc_store = LocMapperStore(self.instrumented_cache, **modulestore_options)
+        LocMapperSetupSansDjango.loc_store = LocMapperStore(self.instrumented_cache, **modulestore_options)
 
     def tearDown(self):
         dbref = TestLocationMapper.loc_store.db
         dbref.drop_collection(TestLocationMapper.loc_store.location_map)
         dbref.connection.close()
-        TestLocationMapper.loc_store = None
+        self.loc_store = None
 
+
+class TestLocationMapper(LocMapperSetupSansDjango):
+    """
+    Test the location to locator mapper
+    """
     def test_create_map(self):
         org = 'foo_org'
         course = 'bar_course'
@@ -125,7 +130,7 @@ class TestLocationMapper(unittest.TestCase):
         )
         test_problem_locn = Location('i4x', org, course, 'problem', 'abc123')
         # only one course matches
-        self.translate_n_check(test_problem_locn, old_style_course_id, new_style_package_id, 'problem2', 'published')
+
         # look for w/ only the Location (works b/c there's only one possible course match). Will force
         # cache as default translation for this problemid
         self.translate_n_check(test_problem_locn, None, new_style_package_id, 'problem2', 'published')
@@ -389,7 +394,7 @@ def loc_mapper():
     """
     Mocks the global location mapper.
     """
-    return TestLocationMapper.loc_store
+    return LocMapperSetupSansDjango.loc_store
 
 
 def render_to_template_mock(*_args):
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 1b7cbb443b..81ebd70a33 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py
@@ -1,286 +1,371 @@
-# pylint: disable=E0611
-from nose.tools import assert_equals, assert_raises, assert_false, \
-    assert_true, assert_not_equals, assert_in, assert_not_in
-# pylint: enable=E0611
 import pymongo
 from uuid import uuid4
+import copy
+import ddt
+from mock import patch
 
 from xmodule.tests import DATA_DIR
-from xmodule.modulestore import Location, MONGO_MODULESTORE_TYPE, XML_MODULESTORE_TYPE
+from xmodule.modulestore import Location, MONGO_MODULESTORE_TYPE, SPLIT_MONGO_MODULESTORE_TYPE, \
+    XML_MODULESTORE_TYPE
 from xmodule.modulestore.exceptions import ItemNotFoundError
-from xmodule.modulestore.xml_importer import import_from_xml
 
-# Mixed modulestore depends on django, so we'll manually configure some django settings
-# before importing the module
+from xmodule.modulestore.locator import BlockUsageLocator
+from xmodule.modulestore.tests.test_location_mapper import LocMapperSetupSansDjango, loc_mapper
+
+# FIXME remove settings
 from django.conf import settings
-import unittest
-import copy
 if not settings.configured:
     settings.configure()
 
 from xmodule.modulestore.mixed import MixedModuleStore
 
+@ddt.ddt
+class TestMixedModuleStore(LocMapperSetupSansDjango):
+    """
+    Quasi-superclass which tests Location based apps against both split and mongo dbs (Locator and
+    Location-based dbs)
+    """
+    HOST = 'localhost'
+    PORT = 27017
+    DB = 'test_mongo_%s' % uuid4().hex[:5]
+    COLLECTION = 'modulestore'
+    FS_ROOT = DATA_DIR
+    DEFAULT_CLASS = 'xmodule.raw_module.RawDescriptor'
+    RENDER_TEMPLATE = lambda t_n, d, ctx = None, nsp = 'main': ''
+    REFERENCE_TYPE = 'xmodule.modulestore.Location'
 
-HOST = 'localhost'
-PORT = 27017
-DB = 'test_mongo_%s' % uuid4().hex[:5]
-COLLECTION = 'modulestore'
-FS_ROOT = DATA_DIR
-DEFAULT_CLASS = 'xmodule.raw_module.RawDescriptor'
-RENDER_TEMPLATE = lambda t_n, d, ctx = None, nsp = 'main': ''
+    IMPORT_COURSEID = 'MITx/999/2013_Spring'
+    XML_COURSEID1 = 'edX/toy/2012_Fall'
+    XML_COURSEID2 = 'edX/simple/2012_Fall'
 
-IMPORT_COURSEID = 'MITx/999/2013_Spring'
-XML_COURSEID1 = 'edX/toy/2012_Fall'
-XML_COURSEID2 = 'edX/simple/2012_Fall'
-
-OPTIONS = {
-    'mappings': {
-        XML_COURSEID1: 'xml',
-        XML_COURSEID2: 'xml',
-        IMPORT_COURSEID: 'default'
-    },
-    'reference_type': 'Location',
-    'stores': {
-        'xml': {
-            'ENGINE': 'xmodule.modulestore.xml.XMLModuleStore',
-            'OPTIONS': {
-                'data_dir': DATA_DIR,
-                'default_class': 'xmodule.hidden_module.HiddenDescriptor',
-            }
+    modulestore_options = {
+        'default_class': DEFAULT_CLASS,
+        'fs_root': DATA_DIR,
+        'render_template': RENDER_TEMPLATE,
+    }
+    DOC_STORE_CONFIG = {
+        'host': HOST,
+        'db': DB,
+        'collection': COLLECTION,
+    }
+    OPTIONS = {
+        'mappings': {
+            XML_COURSEID1: 'xml',
+            XML_COURSEID2: 'xml',
+            IMPORT_COURSEID: 'default'
         },
-        'default': {
-            'ENGINE': 'xmodule.modulestore.mongo.MongoModuleStore',
-            'DOC_STORE_CONFIG': {
-                'host': HOST,
-                'db': DB,
-                'collection': COLLECTION,
+        'reference_type': REFERENCE_TYPE,
+        'stores': {
+            'xml': {
+                'ENGINE': 'xmodule.modulestore.xml.XMLModuleStore',
+                'OPTIONS': {
+                    'data_dir': DATA_DIR,
+                    'default_class': 'xmodule.hidden_module.HiddenDescriptor',
+                }
             },
-            'OPTIONS': {
-                'default_class': DEFAULT_CLASS,
-                'fs_root': DATA_DIR,
-                'render_template': RENDER_TEMPLATE,
+            'direct': {
+                'ENGINE': 'xmodule.modulestore.mongo.MongoModuleStore',
+                'DOC_STORE_CONFIG': DOC_STORE_CONFIG,
+                'OPTIONS': modulestore_options
+            },
+            'draft': {
+                'ENGINE': 'xmodule.modulestore.mongo.MongoModuleStore',
+                'DOC_STORE_CONFIG': DOC_STORE_CONFIG,
+                'OPTIONS': modulestore_options
+            },
+            'split': {
+                'ENGINE': 'xmodule.modulestore.split_mongo.SplitMongoModuleStore',
+                'DOC_STORE_CONFIG': DOC_STORE_CONFIG,
+                'OPTIONS': modulestore_options
             }
         }
     }
-}
 
+    def _compareIgnoreVersion(self, loc1, loc2, msg=None):
+        """
+        AssertEqual replacement for CourseLocator
+        """
+        if not (loc1.package_id == loc2.package_id and loc1.branch == loc2.branch and loc1.block_id == loc2.block_id):
+            self.fail(self._formatMessage(msg, u"{} != {}".format(unicode(loc1), unicode(loc2))))
 
-class TestMixedModuleStore(object):
-    '''Tests!'''
-    @classmethod
-    def setupClass(cls):
+    def setUp(self):
         """
         Set up the database for testing
         """
-        cls.connection = pymongo.MongoClient(
-            host=HOST,
-            port=PORT,
+        self.options = getattr(self, 'options', self.OPTIONS)
+        self.connection = pymongo.MongoClient(
+            host=self.HOST,
+            port=self.PORT,
             tz_aware=True,
         )
-        cls.connection.drop_database(DB)
-        cls.fake_location = Location('i4x', 'foo', 'bar', 'vertical', 'baz')
-        import_course_dict = Location.parse_course_id(IMPORT_COURSEID)
-        cls.import_org = import_course_dict['org']
-        cls.import_course = import_course_dict['course']
-        cls.import_run = import_course_dict['name']
-        # NOTE: Creating a single db for all the tests to save time.  This
-        # is ok only as long as none of the tests modify the db.
-        # If (when!) that changes, need to either reload the db, or load
-        # once and copy over to a tmp db for each test.
-        cls.store = cls.initdb()
+        self.connection.drop_database(self.DB)
+        self.addCleanup(self.connection.drop_database, self.DB)
+        self.addCleanup(self.connection.close)
+        
+        super(TestMixedModuleStore, self).setUp()
 
-    @classmethod
-    def teardownClass(cls):
-        """
-        Clear out database after test has completed
-        """
-        cls.destroy_db(cls.connection)
+        patcher = patch('xmodule.modulestore.mixed.loc_mapper', return_value=LocMapperSetupSansDjango.loc_store)
+        patcher.start()
+        self.addCleanup(patcher.stop)
+        self.addTypeEqualityFunc(BlockUsageLocator, '_compareIgnoreVersion')
 
-    @staticmethod
-    def initdb():
+    def _create_course(self, default, course_location, item_location):
         """
-        Initialize the database and import one test course into it
+        Create a course w/ one item in the persistence store using the given course & item location.
+        NOTE: course_location and item_location must be Location regardless of the app reference type in order
+        to cause the right mapping to be created.
         """
-        # connect to the db
-        _options = {}
-        _options.update(OPTIONS)
-        store = MixedModuleStore(**_options)
-
-        import_from_xml(
-            store._get_modulestore_for_courseid(IMPORT_COURSEID),
-            DATA_DIR,
-            ['toy'],
-            target_location_namespace=Location(
-                'i4x',
-                TestMixedModuleStore.import_org,
-                TestMixedModuleStore.import_course,
-                'course',
-                TestMixedModuleStore.import_run
+        if default == 'split':
+            course = self.store.create_course(course_location, store_name=default)
+            chapter = self.store.create_item(
+                # don't use course_location as it may not be the repr
+                course.location, item_location.category, location=item_location, block_id=item_location.name
             )
+        else:
+            course = self.store.create_course(
+                course_location, store_name=default, metadata={'display_name': course_location.name}
+            )
+            chapter = self.store.create_item(course_location, item_location.category, location=item_location)
+        if self.REFERENCE_TYPE == 'xmodule.modulestore.locator.CourseLocator':
+            # add_entry is false b/c this is a test that the right thing happened w/o
+            # wanting any additional side effects
+            lookup_map = loc_mapper().translate_location(
+                course_location.course_id, course_location, add_entry_if_missing=False
+            )
+            self.assertEqual(lookup_map, course.location)
+            lookup_map = loc_mapper().translate_location(
+                course_location.course_id, item_location, add_entry_if_missing=False
+            )
+            self.assertEqual(lookup_map, chapter.location)
+        else:
+            self.assertEqual(course.location, course_location)
+            self.assertEqual(chapter.location, item_location)
+
+    def initdb(self, default):
+        """
+        Initialize the database and create one test course in it
+        """
+        # set the default modulestore
+        self.options['stores']['default'] = self.options['stores'][default]
+        self.store = MixedModuleStore(**self.options)
+        self.addCleanup(self.store.close_all_connections)
+
+        def generate_location(course_id):
+            """
+            Generate the locations for the given ids
+            """
+            org, course, run = course_id.split('/')
+            return Location('i4x', org, course, 'course', run)
+
+        self.course_locations = {
+             course_id: generate_location(course_id)
+             for course_id in [self.IMPORT_COURSEID, self.XML_COURSEID1, self.XML_COURSEID2]
+        }
+        self.fake_location = Location('i4x', 'foo', 'bar', 'vertical', 'baz')
+        self.import_chapter_location = self.course_locations[self.IMPORT_COURSEID].replace(
+            category='chapter', name='Overview'
         )
+        self.xml_chapter_location = self.course_locations[self.XML_COURSEID1].replace(
+            category='chapter', name='Overview'
+        )
+        # grab old style location b4 possibly converted
+        import_location = self.course_locations[self.IMPORT_COURSEID]
+        # get Locators and set up the loc mapper if app is Locator based
+        if self.REFERENCE_TYPE == 'xmodule.modulestore.locator.CourseLocator':
+            self.fake_location = loc_mapper().translate_location('foo/bar/2012_Fall', self.fake_location)
+            self.import_chapter_location = loc_mapper().translate_location(
+                self.IMPORT_COURSEID, self.import_chapter_location
+            )
+            self.xml_chapter_location = loc_mapper().translate_location(
+                self.XML_COURSEID1, self.xml_chapter_location
+            )
+            self.course_locations = {
+                course_id: loc_mapper().translate_location(course_id, locn)
+                for course_id, locn in self.course_locations.iteritems()
+            }
 
-        return store
+        self._create_course(default, import_location, self.import_chapter_location)
 
-    @staticmethod
-    def destroy_db(connection):
-        """
-        Destroy the test db.
-        """
-        connection.drop_database(DB)
-
-    def setUp(self):
-        # make a copy for convenience
-        self.connection = TestMixedModuleStore.connection
-
-    def tearDown(self):
-        pass
-
-    def test_get_modulestore_type(self):
+    @ddt.data('direct', 'split')
+    def test_get_modulestore_type(self, default_ms):
         """
         Make sure we get back the store type we expect for given mappings
         """
-        assert_equals(self.store.get_modulestore_type(XML_COURSEID1), XML_MODULESTORE_TYPE)
-        assert_equals(self.store.get_modulestore_type(XML_COURSEID2), XML_MODULESTORE_TYPE)
-        assert_equals(self.store.get_modulestore_type(IMPORT_COURSEID), MONGO_MODULESTORE_TYPE)
+        self.initdb(default_ms)
+        self.assertEqual(self.store.get_modulestore_type(self.XML_COURSEID1), XML_MODULESTORE_TYPE)
+        self.assertEqual(self.store.get_modulestore_type(self.XML_COURSEID2), XML_MODULESTORE_TYPE)
+        mongo_ms_type = MONGO_MODULESTORE_TYPE if default_ms == 'direct' else SPLIT_MONGO_MODULESTORE_TYPE
+        self.assertEqual(self.store.get_modulestore_type(self.IMPORT_COURSEID), mongo_ms_type)
         # try an unknown mapping, it should be the 'default' store
-        assert_equals(self.store.get_modulestore_type('foo/bar/2012_Fall'), MONGO_MODULESTORE_TYPE)
+        self.assertEqual(self.store.get_modulestore_type('foo/bar/2012_Fall'), mongo_ms_type)
 
-    def test_has_item(self):
-        assert_true(self.store.has_item(
-            IMPORT_COURSEID, Location(['i4x', self.import_org, self.import_course, 'course', self.import_run])
-        ))
-        assert_true(self.store.has_item(
-            XML_COURSEID1, Location(['i4x', 'edX', 'toy', 'course', '2012_Fall'])
-        ))
+    @ddt.data('direct', 'split')
+    def test_has_item(self, default_ms):
+        self.initdb(default_ms)
+        for course_id, course_locn in self.course_locations.iteritems():
+            self.assertTrue(self.store.has_item(course_id, course_locn))
 
         # try negative cases
-        assert_false(self.store.has_item(
-            XML_COURSEID1, Location(['i4x', self.import_org, self.import_course, 'course', self.import_run])
-        ))
-        assert_false(self.store.has_item(
-            IMPORT_COURSEID, Location(['i4x', 'edX', 'toy', 'course', '2012_Fall'])
-        ))
+        self.assertFalse(self.store.has_item(self.XML_COURSEID1, self.course_locations[self.IMPORT_COURSEID]))
+        self.assertFalse(self.store.has_item(self.IMPORT_COURSEID, self.course_locations[self.XML_COURSEID1]))
 
-    def test_get_item(self):
-        with assert_raises(NotImplementedError):
+    @ddt.data('direct', 'split')
+    def test_get_item(self, default_ms):
+        self.initdb(default_ms)
+        with self.assertRaises(NotImplementedError):
             self.store.get_item(self.fake_location)
 
-    def test_get_instance(self):
-        module = self.store.get_instance(
-            IMPORT_COURSEID, Location(['i4x', self.import_org, self.import_course, 'course', self.import_run])
-        )
-        assert_not_equals(module, None)
-
-        module = self.store.get_instance(
-            XML_COURSEID1, Location(['i4x', 'edX', 'toy', 'course', '2012_Fall'])
-        )
-        assert_not_equals(module, None)
+    @ddt.data('direct', 'split')
+    def test_get_instance(self, default_ms):
+        self.initdb(default_ms)
+        for course_id, course_locn in self.course_locations.iteritems():
+            self.assertIsNotNone(self.store.get_instance(course_id, course_locn))
 
         # try negative cases
-        with assert_raises(ItemNotFoundError):
-            self.store.get_instance(
-                XML_COURSEID1, Location(['i4x', self.import_org, self.import_course, 'course', self.import_run])
-            )
+        with self.assertRaises(ItemNotFoundError):
+            self.store.get_instance(self.XML_COURSEID1, self.course_locations[self.IMPORT_COURSEID])
+        with self.assertRaises(ItemNotFoundError):
+            self.store.get_instance(self.IMPORT_COURSEID, self.course_locations[self.XML_COURSEID1])
 
-        with assert_raises(ItemNotFoundError):
-            self.store.get_instance(
-                IMPORT_COURSEID, Location(['i4x', 'edX', 'toy', 'course', '2012_Fall'])
-            )
+    @ddt.data('direct', 'split')
+    def test_get_items(self, default_ms):
+        self.initdb(default_ms)
+        for course_id, course_locn in self.course_locations.iteritems():
+            if hasattr(course_locn, 'as_course_locator'):
+                locn = course_locn.as_course_locator()
+            else:
+                locn = course_locn.replace(org=None, course=None, name=None)
+            # NOTE: use get_course if you just want the course. get_items is expensive
+            modules = self.store.get_items(locn, course_id, qualifiers={'category': 'course'})
+            self.assertEqual(len(modules), 1)
+            self.assertEqual(modules[0].location, course_locn)
 
-    def test_get_items(self):
-        # NOTE: use get_course if you just want the course. get_items only allows wildcarding of category and name
-        modules = self.store.get_items(Location('i4x', None, None, 'course', None), IMPORT_COURSEID)
-        assert_equals(len(modules), 1)
-        assert_equals(modules[0].location.course, self.import_course)
+    @ddt.data('direct', 'split')
+    def test_update_item(self, default_ms):
+        """
+        Update should fail for r/o dbs and succeed for r/w ones
+        """
+        self.initdb(default_ms)
+        # try a r/o db
+        if self.REFERENCE_TYPE == 'xmodule.modulestore.locator.CourseLocator':
+            course_id = self.course_locations[self.XML_COURSEID1]
+        else:
+            course_id = self.XML_COURSEID1
+        course = self.store.get_course(course_id)
+        # if following raised, then the test is really a noop, change it
+        self.assertFalse(course.show_calculator, "Default changed making test meaningless")
+        course.show_calculator = True
+        with self.assertRaises(NotImplementedError):
+            self.store.update_item(course, None)
+        # now do it for a r/w db
+        # get_course api's are inconsistent: one takes Locators the other an old style course id
+        if hasattr(self.course_locations[self.IMPORT_COURSEID], 'as_course_locator'):
+            locn = self.course_locations[self.IMPORT_COURSEID]
+        else:
+            locn = self.IMPORT_COURSEID
+        course = self.store.get_course(locn)
+        # if following raised, then the test is really a noop, change it
+        self.assertFalse(course.show_calculator, "Default changed making test meaningless")
+        course.show_calculator = True
+        self.store.update_item(course, None)
+        course = self.store.get_course(locn)
+        self.assertTrue(course.show_calculator)
 
-        modules = self.store.get_items(Location('i4x', None, None, 'course', None), XML_COURSEID1)
-        assert_equals(len(modules), 1)
-        assert_equals(modules[0].location.course, 'toy')
+    @ddt.data('direct', 'split')
+    def test_delete_item(self, default_ms):
+        """
+        Delete should reject on r/o db and work on r/w one
+        """
+        self.initdb(default_ms)
+        # r/o try deleting the course
+        with self.assertRaises(NotImplementedError):
+            self.store.delete_item(self.xml_chapter_location)
+        self.store.delete_item(self.import_chapter_location, '**replace_user**')
+        # verify it's gone
+        with self.assertRaises(ItemNotFoundError):
+            self.store.get_instance(self.IMPORT_COURSEID, self.import_chapter_location)
 
-        modules = self.store.get_items(Location('i4x', 'edX', 'simple', 'course', None), XML_COURSEID2)
-        assert_equals(len(modules), 1)
-        assert_equals(modules[0].location.course, 'simple')
-
-    def test_update_item(self):
-        # FIXME update
-        with assert_raises(NotImplementedError):
-            self.store.update_item(self.fake_location, '**replace_user**')
-
-    def test_delete_item(self):
-        with assert_raises(NotImplementedError):
-            self.store.delete_item(self.fake_location)
-
-    def test_get_courses(self):
+    @ddt.data('direct', 'split')
+    def test_get_courses(self, default_ms):
+        self.initdb(default_ms)
         # we should have 3 total courses aggregated
         courses = self.store.get_courses()
-        assert_equals(len(courses), 3)
-        course_ids = []
-        for course in courses:
-            course_ids.append(course.location.course_id)
-        assert_true(IMPORT_COURSEID in course_ids)
-        assert_true(XML_COURSEID1 in course_ids)
-        assert_true(XML_COURSEID2 in course_ids)
+        self.assertEqual(len(courses), 3)
+        course_ids = [course.location for course in courses]
+        self.assertIn(self.course_locations[self.IMPORT_COURSEID], course_ids)
+        self.assertIn(self.course_locations[self.XML_COURSEID1], course_ids)
+        self.assertIn(self.course_locations[self.XML_COURSEID2], course_ids)
 
     def test_xml_get_courses(self):
         """
         Test that the xml modulestore only loaded the courses from the maps.
         """
         courses = self.store.modulestores['xml'].get_courses()
-        assert_equals(len(courses), 2)
+        self.assertEqual(len(courses), 2)
         course_ids = [course.location.course_id for course in courses]
-        assert_in(XML_COURSEID1, course_ids)
-        assert_in(XML_COURSEID2, course_ids)
+        self.assertIn(self.XML_COURSEID1, course_ids)
+        self.assertIn(self.XML_COURSEID2, course_ids)
         # this course is in the directory from which we loaded courses but not in the map
-        assert_not_in("edX/toy/TT_2012_Fall", course_ids)
+        self.assertNotIn("edX/toy/TT_2012_Fall", course_ids)
 
-    def test_get_course(self):
-        module = self.store.get_course(IMPORT_COURSEID)
-        assert_equals(module.location.course, self.import_course)
-
-        module = self.store.get_course(XML_COURSEID1)
-        assert_equals(module.location.course, 'toy')
-
-        module = self.store.get_course(XML_COURSEID2)
-        assert_equals(module.location.course, 'simple')
+    @ddt.data('direct', 'split')
+    def test_get_course(self, default_ms):
+        self.initdb(default_ms)
+        for course_locn in self.course_locations.itervalues():
+            if hasattr(course_locn, 'as_course_locator'):
+                locn = course_locn.as_course_locator()
+            else:
+                locn = course_locn.course_id
+            # NOTE: use get_course if you just want the course. get_items is expensive
+            course = self.store.get_course(locn)
+            self.assertIsNotNone(course)
+            self.assertEqual(course.location, course_locn)
 
     # pylint: disable=E1101
-    def test_get_parent_locations(self):
+    @ddt.data('direct', 'split')
+    def test_get_parent_locations(self, default_ms):
+        self.initdb(default_ms)
         parents = self.store.get_parent_locations(
-            Location(['i4x', self.import_org, self.import_course, 'chapter', 'Overview']),
-            IMPORT_COURSEID
+            self.import_chapter_location,
+            self.IMPORT_COURSEID
         )
-        assert_equals(len(parents), 1)
-        assert_equals(Location(parents[0]).org, self.import_org)
-        assert_equals(Location(parents[0]).course, self.import_course)
-        assert_equals(Location(parents[0]).name, self.import_run)
+        self.assertEqual(len(parents), 1)
+        self.assertEqual(parents[0], self.course_locations[self.IMPORT_COURSEID])
 
         parents = self.store.get_parent_locations(
-            Location(['i4x', 'edX', 'toy', 'chapter', 'Overview']),
-            XML_COURSEID1
+            self.xml_chapter_location,
+            self.XML_COURSEID1
         )
-        assert_equals(len(parents), 1)
-        assert_equals(Location(parents[0]).org, 'edX')
-        assert_equals(Location(parents[0]).course, 'toy')
-        assert_equals(Location(parents[0]).name, '2012_Fall')
+        self.assertEqual(len(parents), 1)
+        self.assertEqual(parents[0], self.course_locations[self.XML_COURSEID1])
 
-class TestMixedMSInit(unittest.TestCase):
+@ddt.ddt
+class TestMixedUseLocator(TestMixedModuleStore):
+    """
+    Tests a mixed ms which uses Locators instead of Locations
+    """
+    REFERENCE_TYPE = 'xmodule.modulestore.locator.CourseLocator'
+
+    def setUp(self):
+        self.options = copy.copy(self.OPTIONS)
+        self.options['reference_type'] = self.REFERENCE_TYPE
+        super(TestMixedUseLocator, self).setUp()
+
+@ddt.ddt
+class TestMixedMSInit(TestMixedModuleStore):
     """
     Test initializing w/o a reference_type
     """
+    REFERENCE_TYPE = None
     def setUp(self):
-        unittest.TestCase.setUp(self)
-        options = copy.copy(OPTIONS)
-        del options['reference_type']
-        self.connection = pymongo.MongoClient(
-            host=HOST,
-            port=PORT,
-            tz_aware=True,
-        )
-        self.store = MixedModuleStore(**options)
+        self.options = copy.copy(self.OPTIONS)
+        del self.options['reference_type']
+        super(TestMixedMSInit, self).setUp()
 
-    def test_use_locations(self):
+    @ddt.data('direct', 'split')
+    def test_use_locations(self, default_ms):
         """
         Test that use_locations defaulted correctly
         """
-        self.assertTrue(self.store.use_locations)
-
+        self.initdb(default_ms)
+        self.assertEqual(self.store.reference_type, Location)
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py
index f492c321d2..33b6eaa2a4 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py
@@ -56,6 +56,7 @@ class TestMongoModuleStore(object):
     def teardownClass(cls):
         if cls.connection:
             cls.connection.drop_database(DB)
+            cls.connection.close()
 
     @staticmethod
     def initdb():
diff --git a/common/lib/xmodule/xmodule/tests/test_import.py b/common/lib/xmodule/xmodule/tests/test_import.py
index 0f0aa80145..0a185ded87 100644
--- a/common/lib/xmodule/xmodule/tests/test_import.py
+++ b/common/lib/xmodule/xmodule/tests/test_import.py
@@ -11,10 +11,10 @@ from mock import Mock, patch
 from django.utils.timezone import UTC
 
 from xmodule.xml_module import is_pointer_tag
-from xmodule.modulestore import Location
+from xmodule.modulestore import Location, only_xmodules
 from xmodule.modulestore.xml import ImportSystem, XMLModuleStore, LocationReader
 from xmodule.modulestore.inheritance import compute_inherited_metadata
-from xmodule.x_module import XModuleMixin, only_xmodules
+from xmodule.x_module import XModuleMixin
 from xmodule.fields import Date
 from xmodule.tests import DATA_DIR
 from xmodule.modulestore.inheritance import InheritanceMixin
diff --git a/common/lib/xmodule/xmodule/tests/xml/factories.py b/common/lib/xmodule/xmodule/tests/xml/factories.py
index a66fdba2e1..e498645c2d 100644
--- a/common/lib/xmodule/xmodule/tests/xml/factories.py
+++ b/common/lib/xmodule/xmodule/tests/xml/factories.py
@@ -9,7 +9,7 @@ from factory import Factory, lazy_attribute, post_generation, Sequence
 from lxml import etree
 
 from xmodule.modulestore.inheritance import InheritanceMixin
-from xmodule.x_module import only_xmodules
+from xmodule.modulestore import only_xmodules
 
 
 class XmlImportData(object):
diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py
index 6d9ada7314..489a2c8032 100644
--- a/common/lib/xmodule/xmodule/x_module.py
+++ b/common/lib/xmodule/xmodule/x_module.py
@@ -580,22 +580,6 @@ class ResourceTemplates(object):
                     return template
 
 
-def prefer_xmodules(identifier, entry_points):
-    """Prefer entry_points from the xmodule package"""
-    from_xmodule = [entry_point for entry_point in entry_points if entry_point.dist.key == 'xmodule']
-    if from_xmodule:
-        return default_select(identifier, from_xmodule)
-    else:
-        return default_select(identifier, entry_points)
-
-
-def only_xmodules(identifier, entry_points):
-    """Only use entry_points that are supplied by the xmodule package"""
-    from_xmodule = [entry_point for entry_point in entry_points if entry_point.dist.key == 'xmodule']
-
-    return default_select(identifier, from_xmodule)
-
-
 @XBlock.needs("i18n")
 class XModuleDescriptor(XModuleMixin, HTMLSnippet, ResourceTemplates, XBlock):
     """
diff --git a/lms/djangoapps/branding/tests.py b/lms/djangoapps/branding/tests.py
index 3849e54161..c25e768567 100644
--- a/lms/djangoapps/branding/tests.py
+++ b/lms/djangoapps/branding/tests.py
@@ -31,7 +31,7 @@ class AnonymousIndexPageTest(ModuleStoreTestCase):
         self.course = CourseFactory.create()
         self.course.days_early_for_beta = 5
         self.course.enrollment_start = datetime.datetime.now(UTC) + datetime.timedelta(days=3)
-        self.store.save_xmodule(self.course)
+        self.store.update_item(self.course)
 
     @override_settings(FEATURES=FEATURES_WITH_STARTDATE)
     def test_none_user_index_access_with_startdate_fails(self):
diff --git a/lms/envs/acceptance.py b/lms/envs/acceptance.py
index 5f059782ad..eeafd783ea 100644
--- a/lms/envs/acceptance.py
+++ b/lms/envs/acceptance.py
@@ -45,7 +45,7 @@ MODULESTORE = {
         'ENGINE': 'xmodule.modulestore.mixed.MixedModuleStore',
         'OPTIONS': {
             'mappings': {},
-            'reference_type': 'Location',
+            'reference_type': 'xmodule.modulestore.Location',
             'stores': {
                 'default': {
                     'ENGINE': 'xmodule.modulestore.mongo.MongoModuleStore',
diff --git a/lms/envs/common.py b/lms/envs/common.py
index a9d2f75726..1bf49cd9ff 100644
--- a/lms/envs/common.py
+++ b/lms/envs/common.py
@@ -34,7 +34,8 @@ from .discussionsettings import *
 
 from lms.lib.xblock.mixin import LmsBlockMixin
 from xmodule.modulestore.inheritance import InheritanceMixin
-from xmodule.x_module import XModuleMixin, only_xmodules
+from xmodule.x_module import XModuleMixin
+from xmodule.modulestore import only_xmodules
 
 ################################### FEATURES ###################################
 # The display name of the platform to be used in templates/emails/etc.