Merge pull request #2302 from edx/db/django-command-migrate-to-split

Django command for migrating courses to split-mongo (and deleting)

.gitignore

@@ -49,6 +49,7 @@ coverage.xml
 cover/
 cover_html/
 reports/
+jscover.log
 jscover.log.*
 
 ### Installation artifacts

cms/djangoapps/contentstore/management/commands/migrate_to_split.py

+"""
+Django management command to migrate a course from the old Mongo modulestore
+to the new split-Mongo modulestore.
+"""
+from django.core.management.base import BaseCommand, CommandError
+from django.contrib.auth.models import User
+from xmodule.modulestore import Location
+from xmodule.modulestore.django import modulestore
+from xmodule.modulestore.split_migrator import SplitMigrator
+from xmodule.modulestore import InvalidLocationError
+from xmodule.modulestore.django import loc_mapper
+
+
+def user_from_str(identifier):
+    """
+    Return a user identified by the given string. The string could be an email
+    address, or a stringified integer corresponding to the ID of the user in
+    the database. If no user could be found, a User.DoesNotExist exception
+    will be raised.
+    """
+    try:
+        user_id = int(identifier)
+    except ValueError:
+        return User.objects.get(email=identifier)
+    else:
+        return User.objects.get(id=user_id)
+
+
+class Command(BaseCommand):
+    "Migrate a course from old-Mongo to split-Mongo"
+
+    help = "Migrate a course from old-Mongo to split-Mongo"
+    args = "location email <locator>"
+
+    def parse_args(self, *args):
+        """
+        Return a three-tuple of (location, user, locator_string).
+        If the user didn't specify a locator string, the third return value
+        will be None.
+        """
+        if len(args) < 2:
+            raise CommandError(
+                "migrate_to_split requires at least two arguments: "
+                "a location and a user identifier (email or ID)"
+            )
+
+        try:
+            location = Location(args[0])
+        except InvalidLocationError:
+            raise CommandError("Invalid location string {}".format(args[0]))
+
+        try:
+            user = user_from_str(args[1])
+        except User.DoesNotExist:
+            raise CommandError("No user found identified by {}".format(args[1]))
+
+        try:
+            package_id = args[2]
+        except IndexError:
+            package_id = None
+
+        return location, user, package_id
+
+    def handle(self, *args, **options):
+        location, user, package_id = self.parse_args(*args)
+
+        migrator = SplitMigrator(
+            draft_modulestore=modulestore('default'),
+            direct_modulestore=modulestore('direct'),
+            split_modulestore=modulestore('split'),
+            loc_mapper=loc_mapper(),
+        )
+
+        migrator.migrate_mongo_course(location, user, package_id)

cms/djangoapps/contentstore/management/commands/rollback_split_course.py

+"""
+Django management command to rollback a migration to split. The way to do this
+is to delete the course from the split mongo datastore.
+"""
+from django.core.management.base import BaseCommand, CommandError
+from xmodule.modulestore.django import modulestore, loc_mapper
+from xmodule.modulestore.exceptions import ItemNotFoundError, InsufficientSpecificationError
+from xmodule.modulestore.locator import CourseLocator
+
+
+class Command(BaseCommand):
+    "Rollback a course that was migrated to the split Mongo datastore"
+
+    help = "Rollback a course that was migrated to the split Mongo datastore"
+    args = "locator"
+
+    def handle(self, *args, **options):
+        if len(args) < 1:
+            raise CommandError(
+                "rollback_split_course requires at least one argument (locator)"
+            )
+
+        try:
+            locator = CourseLocator(url=args[0])
+        except ValueError:
+            raise CommandError("Invalid locator string {}".format(args[0]))
+
+        location = loc_mapper().translate_locator_to_location(locator, get_course=True)
+        if not location:
+            raise CommandError(
+                "This course does not exist in the old Mongo store. "
+                "This command is designed to rollback a course, not delete "
+                "it entirely."
+            )
+        old_mongo_course = modulestore('direct').get_item(location)
+        if not old_mongo_course:
+            raise CommandError(
+                "This course does not exist in the old Mongo store. "
+                "This command is designed to rollback a course, not delete "
+                "it entirely."
+            )
+
+        try:
+            modulestore('split').delete_course(locator.package_id)
+        except ItemNotFoundError:
+            raise CommandError("No course found with locator {}".format(locator))
+
+        print(
+            'Course rolled back successfully. To delete this course entirely, '
+            'call the "delete_course" management command.'
+        )

cms/djangoapps/contentstore/management/commands/tests/test_migrate_to_split.py

+"""
+Unittests for migrating a course to split mongo
+"""
+import unittest
+
+from django.contrib.auth.models import User
+from django.core.management import CommandError, call_command
+from django.test.utils import override_settings
+from contentstore.management.commands.migrate_to_split import Command
+from contentstore.tests.modulestore_config import TEST_MODULESTORE
+from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
+from xmodule.modulestore.tests.factories import CourseFactory
+from xmodule.modulestore.django import modulestore, loc_mapper
+from xmodule.modulestore.locator import CourseLocator
+# pylint: disable=E1101
+
+
+class TestArgParsing(unittest.TestCase):
+    """
+    Tests for parsing arguments for the `migrate_to_split` management command
+    """
+    def setUp(self):
+        self.command = Command()
+
+    def test_no_args(self):
+        errstring = "migrate_to_split requires at least two arguments"
+        with self.assertRaisesRegexp(CommandError, errstring):
+            self.command.handle()
+
+    def test_invalid_location(self):
+        errstring = "Invalid location string"
+        with self.assertRaisesRegexp(CommandError, errstring):
+            self.command.handle("foo", "bar")
+
+    def test_nonexistant_user_id(self):
+        errstring = "No user found identified by 99"
+        with self.assertRaisesRegexp(CommandError, errstring):
+            self.command.handle("i4x://org/course/category/name", "99")
+
+    def test_nonexistant_user_email(self):
+        errstring = "No user found identified by fake@example.com"
+        with self.assertRaisesRegexp(CommandError, errstring):
+            self.command.handle("i4x://org/course/category/name", "fake@example.com")
+
+
+@override_settings(MODULESTORE=TEST_MODULESTORE)
+class TestMigrateToSplit(ModuleStoreTestCase):
+    """
+    Unit tests for migrating a course from old mongo to split mongo
+    """
+
+    def setUp(self):
+        super(TestMigrateToSplit, self).setUp()
+        uname = 'testuser'
+        email = 'test+courses@edx.org'
+        password = 'foo'
+        self.user = User.objects.create_user(uname, email, password)
+        self.course = CourseFactory()
+
+    def test_user_email(self):
+        call_command(
+            "migrate_to_split",
+            str(self.course.location),
+            str(self.user.email),
+        )
+        locator = loc_mapper().translate_location(self.course.id, self.course.location)
+        course_from_split = modulestore('split').get_course(locator)
+        self.assertIsNotNone(course_from_split)
+
+    def test_user_id(self):
+        call_command(
+            "migrate_to_split",
+            str(self.course.location),
+            str(self.user.id),
+        )
+        locator = loc_mapper().translate_location(self.course.id, self.course.location)
+        course_from_split = modulestore('split').get_course(locator)
+        self.assertIsNotNone(course_from_split)
+
+    def test_locator_string(self):
+        call_command(
+            "migrate_to_split",
+            str(self.course.location),
+            str(self.user.id),
+            "org.dept.name.run",
+        )
+        locator = CourseLocator(package_id="org.dept.name.run", branch="published")
+        course_from_split = modulestore('split').get_course(locator)
+        self.assertIsNotNone(course_from_split)

cms/djangoapps/contentstore/management/commands/tests/test_rollback_split_course.py

+"""
+Unittests for deleting a split mongo course
+"""
+import unittest
+from StringIO import StringIO
+from mock import patch
+
+from django.contrib.auth.models import User
+from django.core.management import CommandError, call_command
+from django.test.utils import override_settings
+from contentstore.management.commands.rollback_split_course import Command
+from contentstore.tests.modulestore_config import TEST_MODULESTORE
+from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
+from xmodule.modulestore.tests.persistent_factories import PersistentCourseFactory
+from xmodule.modulestore.tests.factories import CourseFactory
+from xmodule.modulestore.django import modulestore, loc_mapper
+from xmodule.modulestore.exceptions import ItemNotFoundError
+from xmodule.modulestore.split_migrator import SplitMigrator
+# pylint: disable=E1101
+
+
+class TestArgParsing(unittest.TestCase):
+    """
+    Tests for parsing arguments for the `rollback_split_course` management command
+    """
+    def setUp(self):
+        self.command = Command()
+
+    def test_no_args(self):
+        errstring = "rollback_split_course requires at least one argument"
+        with self.assertRaisesRegexp(CommandError, errstring):
+            self.command.handle()
+
+    def test_invalid_locator(self):
+        errstring = "Invalid locator string !?!"
+        with self.assertRaisesRegexp(CommandError, errstring):
+            self.command.handle("!?!")
+
+
+@override_settings(MODULESTORE=TEST_MODULESTORE)
+class TestRollbackSplitCourseNoOldMongo(ModuleStoreTestCase):
+    """
+    Unit tests for rolling back a split-mongo course from command line,
+    where the course doesn't exist in the old mongo store
+    """
+
+    def setUp(self):
+        super(TestRollbackSplitCourseNoOldMongo, self).setUp()
+        self.course = PersistentCourseFactory()
+
+    def test_no_old_course(self):
+        locator = self.course.location
+        errstring = "course does not exist in the old Mongo store"
+        with self.assertRaisesRegexp(CommandError, errstring):
+            Command().handle(str(locator))
+
+@override_settings(MODULESTORE=TEST_MODULESTORE)
+class TestRollbackSplitCourseNoSplitMongo(ModuleStoreTestCase):
+    """
+    Unit tests for rolling back a split-mongo course from command line,
+    where the course doesn't exist in the split mongo store
+    """
+
+    def setUp(self):
+        super(TestRollbackSplitCourseNoSplitMongo, self).setUp()
+        self.old_course = CourseFactory()
+
+    def test_nonexistent_locator(self):
+        locator = loc_mapper().translate_location(self.old_course.id, self.old_course.location)
+        errstring = "No course found with locator"
+        with self.assertRaisesRegexp(CommandError, errstring):
+            Command().handle(str(locator))
+
+
+@override_settings(MODULESTORE=TEST_MODULESTORE)
+class TestRollbackSplitCourse(ModuleStoreTestCase):
+    """
+    Unit tests for rolling back a split-mongo course from command line
+    """
+    def setUp(self):
+        super(TestRollbackSplitCourse, self).setUp()
+        self.old_course = CourseFactory()
+        uname = 'testuser'
+        email = 'test+courses@edx.org'
+        password = 'foo'
+        self.user = User.objects.create_user(uname, email, password)
+
+        # migrate old course to split
+        migrator = SplitMigrator(
+            draft_modulestore=modulestore('default'),
+            direct_modulestore=modulestore('direct'),
+            split_modulestore=modulestore('split'),
+            loc_mapper=loc_mapper(),
+        )
+        migrator.migrate_mongo_course(self.old_course.location, self.user)
+        locator = loc_mapper().translate_location(self.old_course.id, self.old_course.location)
+        self.course = modulestore('split').get_course(locator)
+
+    @patch("sys.stdout", new_callable=StringIO)
+    def test_happy_path(self, mock_stdout):
+        locator = self.course.location
+        call_command(
+            "rollback_split_course",
+            str(locator),
+        )
+        with self.assertRaises(ItemNotFoundError):
+            modulestore('split').get_course(locator)
+
+        self.assertIn("Course rolled back successfully", mock_stdout.getvalue())
+

cms/djangoapps/contentstore/tests/test_contentstore.py

@@ -133,7 +133,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
 
         # just pick one vertical
         descriptor = store.get_items(Location('i4x', 'edX', 'simple', 'vertical', None, None))[0]
-        locator = loc_mapper().translate_location(course.location.course_id, descriptor.location, False, True)
+        locator = loc_mapper().translate_location(course.location.course_id, descriptor.location, True, True)
         resp = self.client.get_html(locator.url_reverse('unit'))
         self.assertEqual(resp.status_code, 200)
         _test_no_locations(self, resp)
@@ -144,12 +144,11 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
     def test_advanced_components_in_edit_unit(self):
         # This could be made better, but for now let's just assert that we see the advanced modules mentioned in the page
         # response HTML
-        self.check_components_on_page(ADVANCED_COMPONENT_TYPES, ['Word cloud',
-                                                                 'Annotation',
-                                                                 'Text Annotation',
-                                                                 'Video Annotation',
-                                                                 'Open Response Assessment',
-                                                                 'Peer Grading Interface'])
+        self.check_components_on_page(
+            ADVANCED_COMPONENT_TYPES,
+            ['Word cloud', 'Annotation', 'Text Annotation', 'Video Annotation',
+             'Open Response Assessment', 'Peer Grading Interface'],
+        )
 
     def test_advanced_components_require_two_clicks(self):
         self.check_components_on_page(['word_cloud'], ['Word cloud'])
@@ -161,7 +160,8 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         # just pick one vertical
         descriptor = store.get_items(Location('i4x', 'edX', 'simple', 'vertical', None, None))[0]
         location = descriptor.location.replace(name='.' + descriptor.location.name)
-        locator = loc_mapper().translate_location(course_items[0].location.course_id, location, False, True)
+        locator = loc_mapper().translate_location(
+            course_items[0].location.course_id, location, add_entry_if_missing=True)
 
         resp = self.client.get_html(locator.url_reverse('unit'))
         self.assertEqual(resp.status_code, 400)
@@ -449,7 +449,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         """ Returns the locator for a given tab. """
         tab_location = 'i4x://edX/999/static_tab/{0}'.format(tab['url_slug'])
         return loc_mapper().translate_location(
-            course.location.course_id, Location(tab_location), False, True
+            course.location.course_id, Location(tab_location), True, True
         )
 
     def _create_static_tabs(self):
@@ -457,7 +457,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         module_store = modulestore('direct')
         CourseFactory.create(org='edX', course='999', display_name='Robot Super Course')
         course_location = Location('i4x', 'edX', '999', 'course', 'Robot_Super_Course', None)
-        new_location = loc_mapper().translate_location(course_location.course_id, course_location, False, True)
+        new_location = loc_mapper().translate_location(course_location.course_id, course_location, True, True)
 
         ItemFactory.create(
             parent_location=course_location,
@@ -512,7 +512,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
 
         # also try a custom response which will trigger the 'is this course in whitelist' logic
         locator = loc_mapper().translate_location(
-            course_items[0].location.course_id, location, False, True
+            course_items[0].location.course_id, location, True, True
         )
         resp = self.client.get_html(locator.url_reverse('xblock'))
         self.assertEqual(resp.status_code, 200)
@@ -534,7 +534,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         # make sure the parent points to the child object which is to be deleted
         self.assertTrue(sequential.location.url() in chapter.children)
 
-        location = loc_mapper().translate_location(course_location.course_id, sequential.location, False, True)
+        location = loc_mapper().translate_location(course_location.course_id, sequential.location, True, True)
         self.client.delete(location.url_reverse('xblock'), {'recurse': True, 'all_versions': True})
 
         found = False
@@ -685,7 +685,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
 
         # go through the website to do the delete, since the soft-delete logic is in the view
         course = course_items[0]
-        location = loc_mapper().translate_location(course.location.course_id, course.location, False, True)
+        location = loc_mapper().translate_location(course.location.course_id, course.location, True, True)
         url = location.url_reverse('assets/', '/c4x/edX/toy/asset/sample_static.txt')
         resp = self.client.delete(url)
         self.assertEqual(resp.status_code, 204)
@@ -1062,7 +1062,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         )
 
         # Unit test fails in Jenkins without this.
-        loc_mapper().translate_location(course_location.course_id, course_location, False, True)
+        loc_mapper().translate_location(course_location.course_id, course_location, True, True)
 
         items = module_store.get_items(stub_location.replace(category='vertical', name=None))
         self._check_verticals(items, course_location.course_id)
@@ -1353,7 +1353,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         # Assert is here to make sure that the course being tested actually has verticals (units) to check.
         self.assertGreater(len(items), 0)
         for descriptor in items:
-            unit_locator = loc_mapper().translate_location(course_id, descriptor.location, False, True)
+            unit_locator = loc_mapper().translate_location(course_id, descriptor.location, True, True)
             resp = self.client.get_html(unit_locator.url_reverse('unit'))
             self.assertEqual(resp.status_code, 200)
             _test_no_locations(self, resp)
@@ -1645,7 +1645,7 @@ class ContentStoreTest(ModuleStoreTestCase):
 
         import_from_xml(modulestore('direct'), 'common/test/data/', ['simple'])
         loc = Location(['i4x', 'edX', 'simple', 'course', '2012_Fall', None])
-        new_location = loc_mapper().translate_location(loc.course_id, loc, False, True)
+        new_location = loc_mapper().translate_location(loc.course_id, loc, True, True)
 
         resp = self._show_course_overview(loc)
         self.assertEqual(resp.status_code, 200)
@@ -1666,14 +1666,14 @@ class ContentStoreTest(ModuleStoreTestCase):
 
         # go look at a subsection page
         subsection_location = loc.replace(category='sequential', name='test_sequence')
-        subsection_locator = loc_mapper().translate_location(loc.course_id, subsection_location, False, True)
+        subsection_locator = loc_mapper().translate_location(loc.course_id, subsection_location, True, True)
         resp = self.client.get_html(subsection_locator.url_reverse('subsection'))
         self.assertEqual(resp.status_code, 200)
         _test_no_locations(self, resp)
 
         # go look at the Edit page
         unit_location = loc.replace(category='vertical', name='test_vertical')
-        unit_locator = loc_mapper().translate_location(loc.course_id, unit_location, False, True)
+        unit_locator = loc_mapper().translate_location(loc.course_id, unit_location, True, True)
         resp = self.client.get_html(unit_locator.url_reverse('unit'))
         self.assertEqual(resp.status_code, 200)
         _test_no_locations(self, resp)
@@ -1681,7 +1681,7 @@ class ContentStoreTest(ModuleStoreTestCase):
         def delete_item(category, name):
             """ Helper method for testing the deletion of an xblock item. """
             del_loc = loc.replace(category=category, name=name)
-            del_location = loc_mapper().translate_location(loc.course_id, del_loc, False, True)
+            del_location = loc_mapper().translate_location(loc.course_id, del_loc, True, True)
             resp = self.client.delete(del_location.url_reverse('xblock'))
             self.assertEqual(resp.status_code, 204)
             _test_no_locations(self, resp, status_code=204, html=False)
@@ -1883,7 +1883,7 @@ class ContentStoreTest(ModuleStoreTestCase):
         """
         Show the course overview page.
         """
-        new_location = loc_mapper().translate_location(location.course_id, location, False, True)
+        new_location = loc_mapper().translate_location(location.course_id, location, True, True)
         resp = self.client.get_html(new_location.url_reverse('course/', ''))
         _test_no_locations(self, resp)
         return resp
@@ -1998,7 +1998,7 @@ def _course_factory_create_course():
     Creates a course via the CourseFactory and returns the locator for it.
     """
     course = CourseFactory.create(org='MITx', course='999', display_name='Robot Super Course')
-    return loc_mapper().translate_location(course.location.course_id, course.location, False, True)
+    return loc_mapper().translate_location(course.location.course_id, course.location, True, True)
 
 
 def _get_course_id(test_course_data):

common/lib/xmodule/xmodule/modulestore/django.py

@@ -152,6 +152,9 @@ def clear_existing_modulestores():
     _MODULESTORES.clear()
     # pylint: disable=W0603
     global _loc_singleton
+    cache = getattr(_loc_singleton, "cache", None)
+    if cache:
+        cache.clear()
     _loc_singleton = None
 
 

common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py

@@ -93,7 +93,7 @@ class LocMapperStore(object):
                 package_id = "{0.org}.{0.course}".format(course_location)
         # very like _interpret_location_id but w/o the _id
         location_id = self._construct_location_son(
-            course_location.org, course_location.course, 
+            course_location.org, course_location.course,
             course_location.name if course_location.category == 'course' else None
         )
 
@@ -219,6 +219,11 @@ class LocMapperStore(object):
             return None
         result = None
         for candidate in maps:
+            if get_course and 'name' in candidate['_id']:
+                candidate_id = candidate['_id']
+                return Location(
+                    'i4x', candidate_id['org'], candidate_id['course'], 'course', candidate_id['name']
+                )
             old_course_id = self._generate_location_course_id(candidate['_id'])
             for old_name, cat_to_usage in candidate['block_map'].iteritems():
                 for category, block_id in cat_to_usage.iteritems():
@@ -240,7 +245,7 @@ class LocMapperStore(object):
                         candidate['course_id'], branch=candidate['draft_branch'], block_id=block_id
                     )
                     self._cache_location_map_entry(old_course_id, location, published_locator, draft_locator)
-                    
+
                     if get_course and category == 'course':
                         result = location
                     elif not get_course and block_id == locator.block_id:
@@ -261,8 +266,6 @@ class LocMapperStore(object):
             return cached
 
         location_id = self._interpret_location_course_id(old_style_course_id, location)
-        if old_style_course_id is None:
-            old_style_course_id = self._generate_location_course_id(location_id)
 
         maps = self.location_map.find(location_id)
         maps = list(maps)
@@ -320,10 +323,10 @@ class LocMapperStore(object):
             return {'_id': self._construct_location_son(location.org, location.course, location.name)}
         else:
             return bson.son.SON([('_id.org', location.org), ('_id.course', location.course)])
-    
+
     def _generate_location_course_id(self, entry_id):
         """
-        Generate a Location course_id for the given entry's id
+        Generate a Location course_id for the given entry's id.
         """
         # strip id envelope if any
         entry_id = entry_id.get('_id', entry_id)
@@ -334,7 +337,7 @@ class LocMapperStore(object):
             return '{0[_id.org]}/{0[_id.course]}'.format(entry_id)
         else:
             return '{0[org]}/{0[course]}'.format(entry_id)
-    
+
     def _construct_location_son(self, org, course, name=None):
         """
         Construct the SON needed to repr the location for either a query or an insertion
@@ -401,6 +404,8 @@ class LocMapperStore(object):
         """
         Get the course Locator for this old course id
         """
+        if not old_course_id:
+            return None
         entry = self.cache.get(old_course_id)
         if entry is not None:
             if published:
@@ -425,6 +430,8 @@ class LocMapperStore(object):
         """
         For quick lookup of courses
         """
+        if not old_course_id:
+            return
         self.cache.set(old_course_id, (published_course_locator, draft_course_locator))
 
     def _cache_location_map_entry(self, old_course_id, location, published_usage, draft_usage):

common/lib/xmodule/xmodule/modulestore/split_migrator.py

@@ -23,7 +23,7 @@ class SplitMigrator(object):
         self.draft_modulestore = draft_modulestore
         self.loc_mapper = loc_mapper
 
-    def migrate_mongo_course(self, course_location, user_id, new_package_id=None):
+    def migrate_mongo_course(self, course_location, user, new_package_id=None):
         """
         Create a new course in split_mongo representing the published and draft versions of the course from the
         original mongo store. And return the new_package_id (which the caller can also get by calling
@@ -32,7 +32,7 @@ class SplitMigrator(object):
         If the new course already exists, this raises DuplicateItemError
 
         :param course_location: a Location whose category is 'course' and points to the course
-        :param user_id: the user whose action is causing this migration
+        :param user: the user whose action is causing this migration
         :param new_package_id: (optional) the Locator.package_id for the new course. Defaults to
         whatever translate_location_to_locator returns
         """
@@ -48,18 +48,18 @@ class SplitMigrator(object):
         new_course_root_locator = self.loc_mapper.translate_location(old_course_id, course_location)
         new_course = self.split_modulestore.create_course(
             course_location.org, original_course.display_name,
-            user_id, id_root=new_package_id,
+            user.id, id_root=new_package_id,
             fields=self._get_json_fields_translate_children(original_course, old_course_id, True),
             root_block_id=new_course_root_locator.block_id,
             master_branch=new_course_root_locator.branch
         )
 
-        self._copy_published_modules_to_course(new_course, course_location, old_course_id, user_id)
-        self._add_draft_modules_to_course(new_package_id, old_course_id, course_location, user_id)
+        self._copy_published_modules_to_course(new_course, course_location, old_course_id, user)
+        self._add_draft_modules_to_course(new_package_id, old_course_id, course_location, user)
 
         return new_package_id
 
-    def _copy_published_modules_to_course(self, new_course, old_course_loc, old_course_id, user_id):
+    def _copy_published_modules_to_course(self, new_course, old_course_loc, old_course_id, user):
         """
         Copy all of the modules from the 'direct' version of the course to the new split course.
         """
@@ -79,7 +79,7 @@ class SplitMigrator(object):
                     old_course_id, module.location, True, add_entry_if_missing=True
                 )
                 _new_module = self.split_modulestore.create_item(
-                    course_version_locator, module.category, user_id,
+                    course_version_locator, module.category, user.id,
                     block_id=new_locator.block_id,
                     fields=self._get_json_fields_translate_children(module, old_course_id, True),
                     continue_version=True
@@ -94,7 +94,7 @@ class SplitMigrator(object):
         # children which meant some pointers were to non-existent locations in 'direct'
         self.split_modulestore.internal_clean_children(course_version_locator)
 
-    def _add_draft_modules_to_course(self, new_package_id, old_course_id, old_course_loc, user_id):
+    def _add_draft_modules_to_course(self, new_package_id, old_course_id, old_course_loc, user):
         """
         update each draft. Create any which don't exist in published and attach to their parents.
         """
@@ -124,12 +124,12 @@ class SplitMigrator(object):
                         if name != 'children' and field.is_set_on(module):
                             field.write_to(split_module, field.read_from(module))
 
-                    _new_module = self.split_modulestore.update_item(split_module, user_id)
+                    _new_module = self.split_modulestore.update_item(split_module, user.id)
                 else:
                     # only a draft version (aka, 'private'). parent needs updated too.
                     # create a new course version just in case the current head is also the prod head
                     _new_module = self.split_modulestore.create_item(
-                        new_draft_course_loc, module.category, user_id,
+                        new_draft_course_loc, module.category, user.id,
                         block_id=new_locator.block_id,
                         fields=self._get_json_fields_translate_children(module, old_course_id, True)
                     )
@@ -156,7 +156,7 @@ class SplitMigrator(object):
                             new_parent_cursor = idx + 1
                             break
                 new_parent.children.insert(new_parent_cursor, new_block_id)
-                new_parent = self.split_modulestore.update_item(new_parent, user_id)
+                new_parent = self.split_modulestore.update_item(new_parent, user.id)
 
     def _get_json_fields_translate_children(self, xblock, old_course_id, published):
         """

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

@@ -1284,6 +1284,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)
         self.db_connection.delete_course_index(index['_id'])
 
     def get_errored_courses(self):

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

@@ -4,8 +4,8 @@ Modulestore configuration for test cases.
 
 from uuid import uuid4
 from django.test import TestCase
-from xmodule.modulestore.django import editable_modulestore, \
-    clear_existing_modulestores
+from xmodule.modulestore.django import (
+    editable_modulestore, clear_existing_modulestores, loc_mapper)
 from xmodule.contentstore.django import contentstore
 
 
@@ -225,6 +225,9 @@ class ModuleStoreTestCase(TestCase):
         if contentstore().fs_files:
             db = contentstore().fs_files.database
             db.connection.drop_database(db)
+        location_mapper = loc_mapper()
+        if location_mapper.db:
+            location_mapper.location_map.drop()
 
     @classmethod
     def setUpClass(cls):

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

@@ -80,8 +80,8 @@ class TestLocationMapper(unittest.TestCase):
         Request translation, check package_id, block_id, and branch
         """
         prob_locator = loc_mapper().translate_location(
-            old_style_course_id, 
-            location, 
+            old_style_course_id,
+            location,
             published= (branch=='published'),
             add_entry_if_missing=add_entry
         )
@@ -114,7 +114,7 @@ class TestLocationMapper(unittest.TestCase):
 
         new_style_package_id = '{}.geek_dept.{}.baz_run'.format(org, course)
         block_map = {
-            'abc123': {'problem': 'problem2'}, 
+            'abc123': {'problem': 'problem2'},
             'def456': {'problem': 'problem4'},
             'ghi789': {'problem': 'problem7'},
         }
@@ -139,7 +139,7 @@ class TestLocationMapper(unittest.TestCase):
 
         # add a distractor course (note that abc123 has a different translation in this one)
         distractor_block_map = {
-            'abc123': {'problem': 'problem3'}, 
+            'abc123': {'problem': 'problem3'},
             'def456': {'problem': 'problem4'},
             'ghi789': {'problem': 'problem7'},
         }

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

@@ -271,7 +271,8 @@ class TestMigration(unittest.TestCase):
                 self.compare_dags(presplit, pre_child, split_child, published)
 
     def test_migrator(self):
-        self.migrator.migrate_mongo_course(self.course_location, random.getrandbits(32))
+        user = mock.Mock(id=1)
+        self.migrator.migrate_mongo_course(self.course_location, user)
         # now compare the migrated to the original course
         self.compare_courses(self.old_mongo, True)
         self.compare_courses(self.draft_mongo, False)