Merge pull request #4294 from edx/split_mongo_assets_cherry

Split mongo assets cherry

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

@@ -2,10 +2,8 @@
 Script for cloning a course
 """
 from django.core.management.base import BaseCommand, CommandError
-from xmodule.modulestore.store_utilities import clone_course
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.mixed import store_bulk_write_operations_on_course
-from xmodule.contentstore.django import contentstore
 from student.roles import CourseInstructorRole, CourseStaffRole
 from opaque_keys.edx.keys import CourseKey
 from opaque_keys import InvalidKeyError
@@ -37,12 +35,11 @@ class Command(BaseCommand):
         dest_course_id = self.course_key_from_arg(args[1])
 
         mstore = modulestore()
-        cstore = contentstore()
 
         print("Cloning course {0} to {1}".format(source_course_id, dest_course_id))
 
         with store_bulk_write_operations_on_course(mstore, dest_course_id):
-            if clone_course(mstore, cstore, source_course_id, dest_course_id, None):
+            if mstore.clone_course(source_course_id, dest_course_id, None):
                 print("copying User permissions...")
                 # purposely avoids auth.add_user b/c it doesn't have a caller to authorize
                 CourseInstructorRole(dest_course_id).add_users(

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

+# pylint: disable=W0212
+
 """
 Django management command to migrate a course from the old Mongo modulestore
 to the new split-Mongo modulestore.
@@ -10,6 +12,7 @@ from xmodule.modulestore.django import loc_mapper
 from opaque_keys.edx.keys import CourseKey
 from opaque_keys import InvalidKeyError
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
+from xmodule.modulestore import ModuleStoreEnum
 
 
 def user_from_str(identifier):
@@ -66,9 +69,8 @@ class Command(BaseCommand):
         course_key, user, org, offering = self.parse_args(*args)
 
         migrator = SplitMigrator(
-            draft_modulestore=modulestore('default'),
-            direct_modulestore=modulestore('direct'),
-            split_modulestore=modulestore('split'),
+            draft_modulestore=modulestore()._get_modulestore_by_type(ModuleStoreEnum.Type.mongo),
+            split_modulestore=modulestore()._get_modulestore_by_type(ModuleStoreEnum.Type.split),
             loc_mapper=loc_mapper(),
         )
 

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

@@ -14,7 +14,9 @@ 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
+from xmodule.modulestore import ModuleStoreEnum
 # pylint: disable=E1101
+# pylint: disable=W0212
 
 
 @unittest.skip("Not fixing split mongo until we land opaque-keys 0.9")
@@ -87,9 +89,8 @@ class TestRollbackSplitCourse(ModuleStoreTestCase):
 
         # migrate old course to split
         migrator = SplitMigrator(
-            draft_modulestore=modulestore('default'),
-            direct_modulestore=modulestore('direct'),
-            split_modulestore=modulestore('split'),
+            draft_modulestore=modulestore()._get_modulestore_by_type(ModuleStoreEnum.Type.mongo),
+            split_modulestore=modulestore()._get_modulestore_by_type(ModuleStoreEnum.Type.split),
             loc_mapper=loc_mapper(),
         )
         migrator.migrate_mongo_course(self.old_course.location, self.user)

cms/djangoapps/contentstore/tests/test_clone_course.py

+"""
+Unit tests for cloning a course between the same and different module stores.
+"""
+from django.utils.unittest.case import skipIf
+from opaque_keys.edx.locations import SlashSeparatedCourseKey
+from opaque_keys.edx.locator import CourseLocator
+from xmodule.modulestore import ModuleStoreEnum
+from contentstore.tests.utils import CourseTestCase
+
+
+@skipIf(
+    not 'run' in CourseLocator.KEY_FIELDS,
+    "Pending integration with latest opaque-keys library - need removal of offering, make_asset_key on CourseLocator, etc."
+)
+class CloneCourseTest(CourseTestCase):
+    """
+    Unit tests for cloning a course
+    """
+    def test_clone_course(self):
+        """Tests cloning of a course as follows: XML -> Mongo (+ data) -> Mongo -> Split -> Split"""
+        # 1. import and populate test toy course
+        mongo_course1_id = self.import_and_populate_course()
+        self.check_populated_course(mongo_course1_id)
+
+        # 2. clone course (mongo -> mongo)
+        # TODO - This is currently failing since clone_course doesn't handle Private content - fails on Publish
+        mongo_course2_id = SlashSeparatedCourseKey('edX2', 'toy2', '2013_Fall')
+        self.store.clone_course(mongo_course1_id, mongo_course2_id, self.user.id)
+        self.assertCoursesEqual(mongo_course1_id, mongo_course2_id)
+
+        # 3. clone course (mongo -> split)
+        with self.store.set_default_store(ModuleStoreEnum.Type.split):
+            split_course3_id = CourseLocator(
+                org="edx3", course="split3", run="2013_Fall", branch=ModuleStoreEnum.BranchName.draft
+            )
+            self.store.clone_course(mongo_course2_id, split_course3_id, self.user.id)
+            self.assertCoursesEqual(mongo_course2_id, split_course3_id)
+
+            # 4. clone course (split -> split)
+            split_course4_id = CourseLocator(
+                org="edx4", course="split4", run="2013_Fall", branch=ModuleStoreEnum.BranchName.draft
+            )
+            self.store.clone_course(split_course3_id, split_course4_id, self.user.id)
+            self.assertCoursesEqual(split_course3_id, split_course4_id)

cms/djangoapps/contentstore/tests/test_core_caching.py

 from cache_toolbox.core import get_cached_content, set_cached_content, del_cached_content
 from opaque_keys.edx.locations import Location
-from xmodule.contentstore.content import StaticContent
 from django.test import TestCase
 
 

cms/djangoapps/contentstore/tests/test_export_git.py

@@ -10,7 +10,6 @@ from uuid import uuid4
 
 from django.conf import settings
 from django.test.utils import override_settings
-from pymongo import MongoClient
 
 from .utils import CourseTestCase
 import contentstore.git_export_utils as git_export_utils
@@ -37,7 +36,7 @@ class TestExportGit(CourseTestCase):
         self.test_url = reverse_course_url('export_git', self.course.id)
 
     def tearDown(self):
-        MongoClient().drop_database(TEST_DATA_CONTENTSTORE['DOC_STORE_CONFIG']['db'])
+        modulestore().contentstore.drop_database()
         _CONTENTSTORE.clear()
 
     def test_giturl_missing(self):

cms/djangoapps/contentstore/tests/test_import.py

@@ -7,7 +7,6 @@ Tests for import_from_xml using the mongo modulestore.
 from django.test.client import Client
 from django.test.utils import override_settings
 from django.conf import settings
-from path import path
 import copy
 
 from django.contrib.auth.models import User
@@ -22,7 +21,6 @@ from xmodule.contentstore.django import _CONTENTSTORE
 
 from xmodule.exceptions import NotFoundError
 from uuid import uuid4
-from pymongo import MongoClient
 
 TEST_DATA_CONTENTSTORE = copy.deepcopy(settings.CONTENTSTORE)
 TEST_DATA_CONTENTSTORE['DOC_STORE_CONFIG']['db'] = 'test_xcontent_%s' % uuid4().hex
@@ -56,7 +54,7 @@ class ContentStoreImportTest(ModuleStoreTestCase):
         self.client.login(username=uname, password=password)
 
     def tearDown(self):
-        MongoClient().drop_database(TEST_DATA_CONTENTSTORE['DOC_STORE_CONFIG']['db'])
+        contentstore().drop_database()
         _CONTENTSTORE.clear()
 
     def load_test_import_course(self):

cms/djangoapps/contentstore/tests/test_transcripts_utils.py

@@ -151,7 +151,7 @@ class TestSaveSubsToStore(ModuleStoreTestCase):
 
     def tearDown(self):
         self.clear_subs_content()
-        MongoClient().drop_database(TEST_DATA_CONTENTSTORE['DOC_STORE_CONFIG']['db'])
+        contentstore().drop_database()
         _CONTENTSTORE.clear()
 
 
@@ -190,7 +190,7 @@ class TestDownloadYoutubeSubs(ModuleStoreTestCase):
             org=self.org, number=self.number, display_name=self.display_name)
 
     def tearDown(self):
-        MongoClient().drop_database(TEST_DATA_CONTENTSTORE['DOC_STORE_CONFIG']['db'])
+        contentstore().drop_database()
         _CONTENTSTORE.clear()
 
     def test_success_downloading_subs(self):

cms/djangoapps/contentstore/views/assets.py

@@ -121,7 +121,7 @@ def _assets_json(request, course_key):
 
     asset_json = []
     for asset in assets:
-        asset_id = asset['_id']
+        asset_id = asset.get('content_son', asset['_id'])
         asset_location = StaticContent.compute_location(course_key, asset_id['name'])
         # note, due to the schema change we may not have a 'thumbnail_location' in the result set
         thumbnail_location = asset.get('thumbnail_location', None)

cms/djangoapps/contentstore/views/tests/test_import_export.py

@@ -17,12 +17,12 @@ from django.conf import settings
 from contentstore.utils import reverse_course_url
 
 from xmodule.contentstore.django import _CONTENTSTORE
-from xmodule.modulestore.django import loc_mapper
 from xmodule.modulestore.tests.factories import ItemFactory
 
 from contentstore.tests.utils import CourseTestCase
 from student import auth
 from student.roles import CourseInstructorRole, CourseStaffRole
+from xmodule.modulestore.django import modulestore
 
 TEST_DATA_CONTENTSTORE = copy.deepcopy(settings.CONTENTSTORE)
 TEST_DATA_CONTENTSTORE['DOC_STORE_CONFIG']['db'] = 'test_xcontent_%s' % uuid4().hex
@@ -70,7 +70,7 @@ class ImportTestCase(CourseTestCase):
 
     def tearDown(self):
         shutil.rmtree(self.content_dir)
-        MongoClient().drop_database(TEST_DATA_CONTENTSTORE['DOC_STORE_CONFIG']['db'])
+        modulestore().contentstore.drop_database()
         _CONTENTSTORE.clear()
 
     def test_no_coursexml(self):

cms/djangoapps/contentstore/views/tests/test_transcripts.py

@@ -81,7 +81,7 @@ class Basetranscripts(CourseTestCase):
         }
 
     def tearDown(self):
-        MongoClient().drop_database(TEST_DATA_CONTENTSTORE['DOC_STORE_CONFIG']['db'])
+        contentstore().drop_database()
         _CONTENTSTORE.clear()
 
 

common/djangoapps/contentserver/middleware.py

@@ -50,10 +50,15 @@ class StaticContentServer(object):
             if getattr(content, "locked", False):
                 if not hasattr(request, "user") or not request.user.is_authenticated():
                     return HttpResponseForbidden('Unauthorized')
-                if not request.user.is_staff and not CourseEnrollment.is_enrolled_by_partial(
+                if not request.user.is_staff:
+                    if getattr(loc, 'deprecated', False) and not CourseEnrollment.is_enrolled_by_partial(
                         request.user, loc.course_key
-                ):
-                    return HttpResponseForbidden('Unauthorized')
+                    ):
+                        return HttpResponseForbidden('Unauthorized')
+                    if not getattr(loc, 'deprecated', False) and not CourseEnrollment.is_enrolled(
+                        request.user, loc.course_key
+                    ):
+                        return HttpResponseForbidden('Unauthorized')
 
             # convert over the DB persistent last modified timestamp to a HTTP compatible
             # timestamp, so we can simply compare the strings

common/djangoapps/contentserver/tests/test.py

@@ -4,8 +4,6 @@ Tests for StaticContentServer
 import copy
 import logging
 from uuid import uuid4
-from path import path
-from pymongo import MongoClient
 
 from django.contrib.auth.models import User
 from django.conf import settings
@@ -74,7 +72,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
 
     def tearDown(self):
 
-        MongoClient().drop_database(TEST_DATA_CONTENTSTORE['DOC_STORE_CONFIG']['db'])
+        contentstore().drop_database()
         _CONTENTSTORE.clear()
 
     def test_unlocked_asset(self):

common/djangoapps/terrain/browser.py

@@ -186,11 +186,10 @@ def reset_databases(scenario):
     whereas modulestore data is in unique collection names.  This data is created implicitly during the scenarios.
     If no data is created during the test, these lines equivilently do nothing.
     '''
-    mongo = MongoClient()
-    mongo.drop_database(settings.CONTENTSTORE['DOC_STORE_CONFIG']['db'])
+    modulestore = xmodule.modulestore.django.modulestore()._get_modulestore_by_type(ModuleStoreEnum.Type.mongo)
+    modulestore.contentstore.drop_database()
     _CONTENTSTORE.clear()
 
-    modulestore = xmodule.modulestore.django.modulestore()._get_modulestore_by_type(ModuleStoreEnum.Type.mongo)
     modulestore.collection.drop()
     xmodule.modulestore.django.clear_existing_modulestores()
 

common/lib/xmodule/xmodule/contentstore/content.py

@@ -10,8 +10,8 @@ import StringIO
 from urlparse import urlparse, urlunparse, parse_qsl
 from urllib import urlencode
 
-from opaque_keys.edx.locations import AssetLocation, SlashSeparatedCourseKey
-from .django import contentstore
+from opaque_keys.edx.locations import AssetLocation
+from opaque_keys.edx.keys import CourseKey
 from PIL import Image
 
 
@@ -103,7 +103,7 @@ class StaticContent(object):
         if course_key is None:
             return None
 
-        assert(isinstance(course_key, SlashSeparatedCourseKey))
+        assert(isinstance(course_key, CourseKey))
         return course_key.make_asset_key('asset', '').to_deprecated_string()
 
     @staticmethod
@@ -111,6 +111,8 @@ class StaticContent(object):
         """
         Generate an AssetKey for the given path (old c4x/org/course/asset/name syntax)
         """
+        # TODO OpaqueKey - change to from_string once opaque keys lands
+        # return AssetLocation.from_string(path)
         return AssetLocation.from_deprecated_string(path)
 
     @staticmethod
@@ -213,6 +215,12 @@ class ContentStore(object):
         """
         raise NotImplementedError
 
+    def copy_all_course_assets(self, source_course_key, dest_course_key):
+        """
+        Copy all the course assets from source_course_key to dest_course_key
+        """
+        raise NotImplementedError
+
     def generate_thumbnail(self, content, tempfile_path=None):
         thumbnail_content = None
         # use a naming convention to associate originals with the thumbnail
@@ -248,7 +256,7 @@ class ContentStore(object):
                 thumbnail_content = StaticContent(thumbnail_file_location, thumbnail_name,
                                                   'image/jpeg', thumbnail_file)
 
-                contentstore().save(thumbnail_content)
+                self.save(thumbnail_content)
 
             except Exception, e:
                 # log and continue as thumbnails are generally considered as optional

common/lib/xmodule/xmodule/modulestore/__init__.py

@@ -330,6 +330,23 @@ class ModuleStoreWrite(ModuleStoreRead):
         pass
 
     @abstractmethod
+    def clone_course(self, source_course_id, dest_course_id, user_id):
+        """
+        Sets up source_course_id to point a course with the same content as the desct_course_id. This
+        operation may be cheap or expensive. It may have to copy all assets and all xblock content or
+        merely setup new pointers.
+
+        Backward compatibility: this method used to require in some modulestores that dest_course_id
+        pointed to an empty but already created course. Implementers should support this or should
+        enable creating the course from scratch.
+
+        Raises:
+            ItemNotFoundError: if the source course doesn't exist (or any of its xblocks aren't found)
+            DuplicateItemError: if the destination course already exists (with content in some cases)
+        """
+        pass
+
+    @abstractmethod
     def delete_course(self, course_key, user_id=None):
         """
         Deletes the course. It may be a soft or hard delete. It may or may not remove the xblock definitions
@@ -434,8 +451,10 @@ class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite):
     '''
     Implement interface functionality that can be shared.
     '''
-    def __init__(self, **kwargs):
+    def __init__(self, contentstore, **kwargs):
         super(ModuleStoreWriteBase, self).__init__(**kwargs)
+
+        self.contentstore = contentstore
         # 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
@@ -501,6 +520,16 @@ class ModuleStoreWriteBase(ModuleStoreReadBase, ModuleStoreWrite):
         self.update_item(new_object, user_id, allow_not_found=True)
         return new_object
 
+    def clone_course(self, source_course_id, dest_course_id, user_id):
+        """
+        This base method just copies the assets. The lower level impls must do the actual cloning of
+        content.
+        """
+        # copy the assets
+        self.contentstore.copy_all_course_assets(source_course_id, dest_course_id)
+        super(ModuleStoreWriteBase, self).clone_course(source_course_id, dest_course_id, user_id)
+        return dest_course_id
+
 
 def only_xmodules(identifier, entry_points):
     """Only use entry_points that are supplied by the xmodule package"""

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

@@ -17,6 +17,7 @@ import threading
 from xmodule.modulestore.loc_mapper_store import LocMapperStore
 from xmodule.util.django import get_current_request_hostname
 import xmodule.modulestore  # pylint: disable=unused-import
+from xmodule.contentstore.django import contentstore
 
 # We may not always have the request_cache module available
 try:
@@ -37,7 +38,7 @@ def load_function(path):
     return getattr(import_module(module_path), name)
 
 
-def create_modulestore_instance(engine, doc_store_config, options, i18n_service=None):
+def create_modulestore_instance(engine, content_store, doc_store_config, options, i18n_service=None):
     """
     This will return a new instance of a modulestore given an engine and options
     """
@@ -62,6 +63,7 @@ def create_modulestore_instance(engine, doc_store_config, options, i18n_service=
         metadata_inheritance_cache = get_cache('default')
 
     return class_(
+        contentstore=content_store,
         metadata_inheritance_cache_subsystem=metadata_inheritance_cache,
         request_cache=request_cache,
         xblock_mixins=getattr(settings, 'XBLOCK_MIXINS', ()),
@@ -85,6 +87,7 @@ def modulestore():
     if _MIXED_MODULESTORE is None:
         _MIXED_MODULESTORE = create_modulestore_instance(
             settings.MODULESTORE['default']['ENGINE'],
+            contentstore(),
             settings.MODULESTORE['default'].get('DOC_STORE_CONFIG', {}),
             settings.MODULESTORE['default'].get('OPTIONS', {})
         )

common/lib/xmodule/xmodule/modulestore/mixed.py

@@ -11,7 +11,7 @@ from contextlib import contextmanager
 from opaque_keys import InvalidKeyError
 
 from . import ModuleStoreWriteBase
-from xmodule.modulestore import PublishState
+from xmodule.modulestore import PublishState, ModuleStoreEnum, split_migrator
 from xmodule.modulestore.django import create_modulestore_instance, loc_mapper
 from opaque_keys.edx.locator import CourseLocator, BlockUsageLocator
 from xmodule.modulestore.exceptions import ItemNotFoundError
@@ -29,12 +29,12 @@ class MixedModuleStore(ModuleStoreWriteBase):
     """
     ModuleStore knows how to route requests to the right persistence ms
     """
-    def __init__(self, mappings, stores, i18n_service=None, **kwargs):
+    def __init__(self, contentstore, mappings, stores, i18n_service=None, **kwargs):
         """
         Initialize a MixedModuleStore. Here we look into our passed in kwargs which should be a
         collection of other modulestore configuration information
         """
-        super(MixedModuleStore, self).__init__(**kwargs)
+        super(MixedModuleStore, self).__init__(contentstore, **kwargs)
 
         self.modulestores = []
         self.mappings = {}
@@ -61,6 +61,7 @@ class MixedModuleStore(ModuleStoreWriteBase):
                 ]
             store = create_modulestore_instance(
                 store_settings['ENGINE'],
+                self.contentstore,
                 store_settings.get('DOC_STORE_CONFIG', {}),
                 store_settings.get('OPTIONS', {}),
                 i18n_service=i18n_service,
@@ -295,6 +296,36 @@ class MixedModuleStore(ModuleStoreWriteBase):
 
         return store.create_course(org, offering, user_id, fields, **kwargs)
 
+    def clone_course(self, source_course_id, dest_course_id, user_id):
+        """
+        See the superclass for the general documentation.
+
+        If cloning w/in a store, delegates to that store's clone_course which, in order to be self-
+        sufficient, should handle the asset copying (call the same method as this one does)
+        If cloning between stores,
+            * copy the assets
+            * migrate the courseware
+        """
+        source_modulestore = self._get_modulestore_for_courseid(source_course_id)
+        # for a temporary period of time, we may want to hardcode dest_modulestore as split if there's a split
+        # to have only course re-runs go to split. This code, however, uses the config'd priority
+        dest_modulestore = self._get_modulestore_for_courseid(dest_course_id)
+        if source_modulestore == dest_modulestore:
+            return source_modulestore.clone_course(source_course_id, dest_course_id, user_id)
+
+        # ensure super's only called once. The delegation above probably calls it; so, don't move
+        # the invocation above the delegation call
+        super(MixedModuleStore, self).clone_course(source_course_id, dest_course_id, user_id)
+
+        if dest_modulestore.get_modulestore_type() == ModuleStoreEnum.Type.split:
+            if not hasattr(self, 'split_migrator'):
+                self.split_migrator = split_migrator.SplitMigrator(
+                    dest_modulestore, source_modulestore, loc_mapper()
+                )
+            self.split_migrator.migrate_mongo_course(
+                source_course_id, user_id, dest_course_id.org, dest_course_id.offering
+            )
+
     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,
@@ -460,6 +491,24 @@ class MixedModuleStore(ModuleStoreWriteBase):
         else:
             raise NotImplementedError(u"Cannot call {} on store {}".format(method, store))
 
+    @contextmanager
+    def set_default_store(self, store_type):
+        """
+        A context manager for temporarily changing the default store in the Mixed modulestore
+        """
+        previous_store_list = self.modulestores
+        found = False
+        try:
+            for i, store in enumerate(self.modulestores):
+                if store.get_modulestore_type() == store_type:
+                    self.modulestores.insert(0, self.modulestores.pop(i))
+                    found = True
+                    yield
+            if not found:
+                raise Exception(u"Cannot find store of type {}".format(store_type))
+        finally:
+            self.modulestores = previous_store_list
+
 
 @contextmanager
 def store_branch_setting(store, branch_setting):

common/lib/xmodule/xmodule/modulestore/mongo/base.py

@@ -332,12 +332,12 @@ class MongoModuleStore(ModuleStoreWriteBase):
     """
     A Mongodb backed ModuleStore
     """
-    reference_type = Location
+    reference_type = SlashSeparatedCourseKey
 
     # TODO (cpennington): Enable non-filesystem filestores
     # pylint: disable=C0103
     # pylint: disable=W0201
-    def __init__(self, doc_store_config, fs_root, render_template,
+    def __init__(self, contentstore, doc_store_config, fs_root, render_template,
                  default_class=None,
                  error_tracker=null_error_tracker,
                  i18n_service=None,
@@ -346,7 +346,7 @@ class MongoModuleStore(ModuleStoreWriteBase):
         :param doc_store_config: must have a host, db, and collection entries. Other common entries: port, tz_aware.
         """
 
-        super(MongoModuleStore, self).__init__(**kwargs)
+        super(MongoModuleStore, self).__init__(contentstore, **kwargs)
 
         def do_connection(
             db, collection, host, port=27017, tz_aware=True, user=None, password=None, **kwargs
@@ -857,7 +857,6 @@ class MongoModuleStore(ModuleStoreWriteBase):
         Raises:
             InvalidLocationError: If a course with the same org and offering already exists
         """
-
         course, _, run = offering.partition('/')
         course_id = SlashSeparatedCourseKey(org, course, run)
 

common/lib/xmodule/xmodule/modulestore/mongo/draft.py

@@ -7,15 +7,21 @@ and otherwise returns i4x://org/course/cat/name).
 """
 
 import pymongo
+import logging
 
+from opaque_keys.edx.locations import Location
 from xmodule.exceptions import InvalidVersionError
 from xmodule.modulestore import PublishState, ModuleStoreEnum
-from xmodule.modulestore.exceptions import ItemNotFoundError, DuplicateItemError, InvalidBranchSetting
+from xmodule.modulestore.exceptions import (
+    ItemNotFoundError, DuplicateItemError, InvalidBranchSetting, DuplicateCourseError
+)
 from xmodule.modulestore.mongo.base import (
     MongoModuleStore, MongoRevisionKey, as_draft, as_published,
     DIRECT_ONLY_CATEGORIES, SORT_REVISION_FAVOR_DRAFT
 )
-from opaque_keys.edx.locations import Location
+from xmodule.modulestore.store_utilities import rewrite_nonportable_content_links
+
+log = logging.getLogger(__name__)
 
 
 def wrap_draft(item):
@@ -138,6 +144,73 @@ class DraftModuleStore(MongoModuleStore):
             del key['_id.revision']
             return self.collection.find(key).count() > 0
 
+    def clone_course(self, source_course_id, dest_course_id, user_id):
+        """
+        Only called if cloning within this store or if env doesn't set up mixed.
+        * copy the courseware
+        """
+        # check to see if the source course is actually there
+        if not self.has_course(source_course_id):
+            raise ItemNotFoundError("Cannot find a course at {0}. Aborting".format(source_course_id))
+
+        # verify that the dest_location really is an empty course
+        # b/c we don't want the payload, I'm copying the guts of get_items here
+        query = self._course_key_to_son(dest_course_id)
+        query['_id.category'] = {'$nin': ['course', 'about']}
+        if self.collection.find(query).limit(1).count() > 0:
+            raise DuplicateCourseError(
+                dest_course_id,
+                "Course at destination {0} is not an empty course. You can only clone into an empty course. Aborting...".format(
+                    dest_course_id
+                )
+            )
+
+        # clone the assets
+        super(DraftModuleStore, self).clone_course(source_course_id, dest_course_id, user_id)
+
+        # get the whole old course
+        new_course = self.get_course(dest_course_id)
+        if new_course is None:
+            # create_course creates the about overview
+            new_course = self.create_course(dest_course_id.org, dest_course_id.offering, user_id)
+
+        # Get all modules under this namespace which is (tag, org, course) tuple
+        modules = self.get_items(source_course_id, revision=ModuleStoreEnum.RevisionOption.published_only)
+        self._clone_modules(modules, dest_course_id, user_id)
+        course_location = dest_course_id.make_usage_key('course', dest_course_id.run)
+        self.publish(course_location, user_id)
+
+        modules = self.get_items(source_course_id, revision=ModuleStoreEnum.RevisionOption.draft_only)
+        self._clone_modules(modules, dest_course_id, user_id)
+
+        return True
+
+    def _clone_modules(self, modules, dest_course_id, user_id):
+        """Clones each module into the given course"""
+        for module in modules:
+            original_loc = module.location
+            module.location = module.location.map_into_course(dest_course_id)
+            if module.location.category == 'course':
+                module.location = module.location.replace(name=module.location.run)
+
+            log.info("Cloning module %s to %s....", original_loc, module.location)
+
+            if 'data' in module.fields and module.fields['data'].is_set_on(module) and isinstance(module.data, basestring):
+                module.data = rewrite_nonportable_content_links(
+                    original_loc.course_key, dest_course_id, module.data
+                )
+
+            # repoint children
+            if module.has_children:
+                new_children = []
+                for child_loc in module.children:
+                    child_loc = child_loc.map_into_course(dest_course_id)
+                    new_children.append(child_loc)
+
+                module.children = new_children
+
+            self.update_item(module, user_id, allow_not_found=True)
+
     def _get_raw_parent_locations(self, location, key_revision):
         """
         Get the parents but don't unset the revision in their locations.

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

@@ -15,10 +15,9 @@ class SplitMigrator(object):
     Copies courses from old mongo to split mongo and sets up location mapping so any references to the old
     name will be able to find the new elements.
     """
-    def __init__(self, split_modulestore, direct_modulestore, draft_modulestore, loc_mapper):
+    def __init__(self, split_modulestore, draft_modulestore, loc_mapper):
         super(SplitMigrator, self).__init__()
         self.split_modulestore = split_modulestore
-        self.direct_modulestore = direct_modulestore
         self.draft_modulestore = draft_modulestore
         self.loc_mapper = loc_mapper
 
@@ -43,7 +42,7 @@ class SplitMigrator(object):
         # locations are in location, children, conditionals, course.tab
 
         # create the course: set fields to explicitly_set for each scope, id_root = new_course_locator, master_branch = 'production'
-        original_course = self.direct_modulestore.get_course(course_key)
+        original_course = self.draft_modulestore.get_course(course_key)
         new_course_root_locator = self.loc_mapper.translate_location(original_course.location)
         new_course = self.split_modulestore.create_course(
             new_course_root_locator.org, new_course_root_locator.offering, user.id,
@@ -65,7 +64,7 @@ class SplitMigrator(object):
 
         # iterate over published course elements. Wildcarding rather than descending b/c some elements are orphaned (e.g.,
         # course about pages, conditionals)
-        for module in self.direct_modulestore.get_items(course_key):
+        for module in self.draft_modulestore.get_items(course_key, revision=ModuleStoreEnum.RevisionOption.published_only):
             # don't copy the course again. No drafts should get here
             if module.location != old_course_loc:
                 # create split_xblock using split.create_item

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

@@ -105,7 +105,8 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
 
     SCHEMA_VERSION = 1
     reference_type = Locator
-    def __init__(self, doc_store_config, fs_root, render_template,
+
+    def __init__(self, contentstore, doc_store_config, fs_root, render_template,
                  default_class=None,
                  error_tracker=null_error_tracker,
                  loc_mapper=None,
@@ -115,7 +116,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
         :param doc_store_config: must have a host, db, and collection entries. Other common entries: port, tz_aware.
         """
 
-        super(SplitMongoModuleStore, self).__init__(**kwargs)
+        super(SplitMongoModuleStore, self).__init__(contentstore, **kwargs)
         self.loc_mapper = loc_mapper
 
         self.db_connection = MongoConnection(**doc_store_config)
@@ -870,6 +871,20 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
         # reconstruct the new_item from the cache
         return self.get_item(item_loc)
 
+    def clone_course(self, source_course_id, dest_course_id, user_id):
+        """
+        See :meth: `.ModuleStoreWrite.clone_course` for documentation.
+
+        In split, other than copying the assets, this is cheap as it merely creates a new version of the
+        existing course.
+        """
+        super(SplitMongoModuleStore, self).clone_course(source_course_id, dest_course_id, user_id)
+        source_index = self.get_course_index_info(source_course_id)
+        return self.create_course(
+            dest_course_id.org, dest_course_id.offering, user_id, fields=None,  # override start_date?
+            versions_dict=source_index['versions']
+        )
+
     def create_course(
         self, org, offering, user_id, fields=None,
         master_branch=ModuleStoreEnum.BranchName.draft, versions_dict=None, root_category='course',

common/lib/xmodule/xmodule/modulestore/store_utilities.py

@@ -2,7 +2,6 @@ import re
 import logging
 
 from xmodule.contentstore.content import StaticContent
-from xmodule.modulestore import ModuleStoreEnum
 
 
 def _prefix_only_url_replace_regex(prefix):
@@ -88,91 +87,6 @@ def rewrite_nonportable_content_links(source_course_id, dest_course_id, text):
     return text
 
 
-def _clone_modules(modulestore, modules, source_course_id, dest_course_id, user_id):
-    for module in modules:
-        original_loc = module.location
-        module.location = module.location.map_into_course(dest_course_id)
-        if module.location.category == 'course':
-            module.location = module.location.replace(name=module.location.run)
-
-        print "Cloning module {0} to {1}....".format(original_loc, module.location)
-
-        if 'data' in module.fields and module.fields['data'].is_set_on(module) and isinstance(module.data, basestring):
-            module.data = rewrite_nonportable_content_links(
-                source_course_id, dest_course_id, module.data
-            )
-
-        # repoint children
-        if module.has_children:
-            new_children = []
-            for child_loc in module.children:
-                child_loc = child_loc.map_into_course(dest_course_id)
-                new_children.append(child_loc)
-
-            module.children = new_children
-
-        modulestore.update_item(module, user_id, allow_not_found=True)
-
-
-def clone_course(modulestore, contentstore, source_course_id, dest_course_id, user_id):
-    # check to see if the dest_location exists as an empty course
-    # we need an empty course because the app layers manage the permissions and users
-    if not modulestore.has_course(dest_course_id):
-        raise Exception(u"An empty course at {0} must have already been created. Aborting...".format(dest_course_id))
-
-    # verify that the dest_location really is an empty course, which means only one with an optional 'overview'
-    dest_modules = modulestore.get_items(dest_course_id)
-
-    for module in dest_modules:
-        if module.location.category == 'course' or (
-                module.location.category == 'about' and module.location.name == 'overview'
-        ):
-            continue
-        # only course and about overview allowed
-        raise Exception("Course at destination {0} is not an empty course. You can only clone into an empty course. Aborting...".format(dest_course_id))
-
-    # check to see if the source course is actually there
-    if not modulestore.has_course(source_course_id):
-        raise Exception("Cannot find a course at {0}. Aborting".format(source_course_id))
-
-    # Get all modules under this namespace which is (tag, org, course) tuple
-    modules = modulestore.get_items(source_course_id, revision=ModuleStoreEnum.RevisionOption.published_only)
-    _clone_modules(modulestore, modules, source_course_id, dest_course_id, user_id)
-    course_location = dest_course_id.make_usage_key('course', dest_course_id.run)
-    modulestore.publish(course_location, user_id)
-
-    modules = modulestore.get_items(source_course_id, revision=ModuleStoreEnum.RevisionOption.draft_only)
-    _clone_modules(modulestore, modules, source_course_id, dest_course_id, user_id)
-
-    # now iterate through all of the assets and clone them
-    # first the thumbnails
-    thumb_keys = contentstore.get_all_content_thumbnails_for_course(source_course_id)
-    for thumb_key in thumb_keys:
-        content = contentstore.find(thumb_key)
-        content.location = content.location.map_into_course(dest_course_id)
-
-        print "Cloning thumbnail {0} to {1}".format(thumb_key, content.location)
-
-        contentstore.save(content)
-
-    # now iterate through all of the assets, also updating the thumbnail pointer
-
-    asset_keys, __ = contentstore.get_all_content_for_course(source_course_id)
-    for asset_key in asset_keys:
-        content = contentstore.find(asset_key)
-        content.location = content.location.map_into_course(dest_course_id)
-
-        # be sure to update the pointer to the thumbnail
-        if content.thumbnail_location is not None:
-            content.thumbnail_location = content.thumbnail_location.map_into_course(dest_course_id)
-
-        print "Cloning asset {0} to {1}".format(asset_key, content.location)
-
-        contentstore.save(content)
-
-    return True
-
-
 def delete_course(modulestore, contentstore, course_key, commit=False):
     """
     This method will actually do the work to delete all content in a course in a MongoDB backed

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

@@ -7,7 +7,6 @@ from django.test import TestCase
 from xmodule.modulestore.django import (
     modulestore, clear_existing_modulestores, loc_mapper)
 from xmodule.modulestore import ModuleStoreEnum
-from xmodule.contentstore.django import contentstore
 
 
 def mixed_store_config(data_dir, mappings):
@@ -160,10 +159,8 @@ class ModuleStoreTestCase(TestCase):
             connection.drop_database(store.db.name)
             connection.close()
 
-        if contentstore().fs_files:
-            db = contentstore().fs_files.database
-            db.connection.drop_database(db)
-            db.connection.close()
+        if hasattr(store, 'contentstore'):
+            store.contentstore.drop_database()
 
         location_mapper = loc_mapper()
         if location_mapper.db:

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

+"""
+ Test contentstore.mongo functionality
+"""
+import logging
+from uuid import uuid4
+import unittest
+import mimetypes
+from tempfile import mkdtemp
+import path
+import shutil
+
+from opaque_keys.edx.locations import SlashSeparatedCourseKey, AssetLocation
+from xmodule.tests import DATA_DIR
+from xmodule.contentstore.mongo import MongoContentStore
+from xmodule.contentstore.content import StaticContent
+from xmodule.exceptions import NotFoundError
+import ddt
+from __builtin__ import delattr
+
+
+log = logging.getLogger(__name__)
+
+HOST = 'localhost'
+PORT = 27017
+DB = 'test_mongo_%s' % uuid4().hex[:5]
+
+
+@ddt.ddt
+class TestContentstore(unittest.TestCase):
+    """
+    Test the methods in contentstore.mongo using deprecated and non-deprecated keys
+    """
+
+    # don't use these 2 class vars as they restore behavior once the tests are done
+    asset_deprecated = None
+    ssck_deprecated = None
+
+    @classmethod
+    def tearDownClass(cls):
+        """
+        Restores deprecated values
+        """
+        if cls.asset_deprecated is not None:
+            setattr(AssetLocation, 'deprecated', cls.asset_deprecated)
+        else:
+            delattr(AssetLocation, 'deprecated')
+        if cls.ssck_deprecated is not None:
+            setattr(SlashSeparatedCourseKey, 'deprecated', cls.ssck_deprecated)
+        else:
+            delattr(SlashSeparatedCourseKey, 'deprecated')
+        return super(TestContentstore, cls).tearDownClass()
+
+    def set_up_assets(self, deprecated):
+        """
+        Setup contentstore w/ proper overriding of deprecated.
+        """
+        # since MongoModuleStore and MongoContentStore are basically assumed to be together, create this class
+        # as well
+        self.contentstore = MongoContentStore(HOST, DB, port=PORT)
+        self.addCleanup(self.contentstore.drop_database)
+
+        setattr(AssetLocation, 'deprecated', deprecated)
+        setattr(SlashSeparatedCourseKey, 'deprecated', deprecated)
+
+        self.course1_key = SlashSeparatedCourseKey('test', 'asset_test', '2014_07')
+        self.course2_key = SlashSeparatedCourseKey('test', 'asset_test2', '2014_07')
+
+        self.course1_files = ['contains.sh', 'picture1.jpg', 'picture2.jpg']
+        self.course2_files = ['picture1.jpg', 'picture3.jpg', 'door_2.ogg']
+
+        def load_assets(course_key, files):
+            locked = False
+            for filename in files:
+                asset_key = course_key.make_asset_key('asset', filename)
+                self.save_asset(filename, asset_key, filename, locked)
+                locked = not locked
+
+        load_assets(self.course1_key, self.course1_files)
+        load_assets(self.course2_key, self.course2_files)
+
+    def save_asset(self, filename, asset_key, displayname, locked):
+        """
+        Load and save the given file.
+        """
+        with open("{}/static/{}".format(DATA_DIR, filename), "rb") as f:
+            content = StaticContent(
+                asset_key, displayname, mimetypes.guess_type(filename)[0], f.read(),
+                locked=locked
+            )
+            self.contentstore.save(content)
+
+    @ddt.data(True, False)
+    def test_delete(self, deprecated):
+        """
+        Test that deleting assets works
+        """
+        self.set_up_assets(deprecated)
+        asset_key = self.course1_key.make_asset_key('asset', self.course1_files[0])
+        self.contentstore.delete(asset_key)
+
+        with self.assertRaises(NotFoundError):
+            self.contentstore.find(asset_key)
+
+        # ensure deleting a non-existent file is a noop
+        self.contentstore.delete(asset_key)
+
+    @ddt.data(True, False)
+    def test_find(self, deprecated):
+        """
+        Test using find
+        """
+        self.set_up_assets(deprecated)
+        asset_key = self.course1_key.make_asset_key('asset', self.course1_files[0])
+        self.assertIsNotNone(self.contentstore.find(asset_key), "Could not find {}".format(asset_key))
+
+        self.assertIsNotNone(self.contentstore.find(asset_key, as_stream=True), "Could not find {}".format(asset_key))
+
+        unknown_asset = self.course1_key.make_asset_key('asset', 'no_such_file.gif')
+        with self.assertRaises(NotFoundError):
+            self.contentstore.find(unknown_asset)
+        self.assertIsNone(
+            self.contentstore.find(unknown_asset, throw_on_not_found=False),
+            "Found unknown asset {}".format(unknown_asset)
+        )
+
+    @ddt.data(True, False)
+    def test_export_for_course(self, deprecated):
+        """
+        Test export
+        """
+        self.set_up_assets(deprecated)
+        root_dir = path.path(mkdtemp())
+        try:
+            self.contentstore.export_all_for_course(
+                self.course1_key, root_dir,
+                path.path(root_dir / "policy.json"),
+            )
+            for filename in self.course1_files:
+                filepath = path.path(root_dir / filename)
+                self.assertTrue(filepath.isfile(), "{} is not a file".format(filepath))
+            for filename in self.course2_files:
+                if filename not in self.course1_files:
+                    filepath = path.path(root_dir / filename)
+                    self.assertFalse(filepath.isfile(), "{} is unexpected exported a file".format(filepath))
+        finally:
+            shutil.rmtree(root_dir)
+
+    @ddt.data(True, False)
+    def test_get_all_content(self, deprecated):
+        """
+        Test get_all_content_for_course
+        """
+        self.set_up_assets(deprecated)
+        course1_assets, count = self.contentstore.get_all_content_for_course(self.course1_key)
+        self.assertEqual(count, len(self.course1_files), course1_assets)
+        for asset in course1_assets:
+            parsed = AssetLocation.from_deprecated_string(asset['filename'])
+            self.assertIn(parsed.name, self.course1_files)
+
+        course1_assets, __ = self.contentstore.get_all_content_for_course(self.course1_key, 1, 1)
+        self.assertEqual(len(course1_assets), 1, course1_assets)
+
+        fake_course = SlashSeparatedCourseKey('test', 'fake', 'non')
+        course_assets, count = self.contentstore.get_all_content_for_course(fake_course)
+        self.assertEqual(count, 0)
+        self.assertEqual(course_assets, [])
+
+    @ddt.data(True, False)
+    def test_attrs(self, deprecated):
+        """
+        Test setting and getting attrs
+        """
+        self.set_up_assets(deprecated)
+        for filename in self.course1_files:
+            asset_key = self.course1_key.make_asset_key('asset', filename)
+            prelocked = self.contentstore.get_attr(asset_key, 'locked', False)
+            self.contentstore.set_attr(asset_key, 'locked', not prelocked)
+            self.assertEqual(self.contentstore.get_attr(asset_key, 'locked', False), not prelocked)
+
+    @ddt.data(True, False)
+    def test_copy_assets(self, deprecated):
+        """
+        copy_all_course_assets
+        """
+        self.set_up_assets(deprecated)
+        dest_course = SlashSeparatedCourseKey('test', 'destination', 'copy')
+        self.contentstore.copy_all_course_assets(self.course1_key, dest_course)
+        for filename in self.course1_files:
+            asset_key = self.course1_key.make_asset_key('asset', filename)
+            dest_key = dest_course.make_asset_key('asset', filename)
+            source = self.contentstore.find(asset_key)
+            copied = self.contentstore.find(dest_key)
+            for propname in ['name', 'content_type', 'length', 'locked']:
+                self.assertEqual(getattr(source, propname), getattr(copied, propname))
+
+        __, count = self.contentstore.get_all_content_for_course(dest_course)
+        self.assertEqual(count, len(self.course1_files))
+
+    @ddt.data(True, False)
+    def test_delete_assets(self, deprecated):
+        """
+        delete_all_course_assets
+        """
+        self.set_up_assets(deprecated)
+        self.contentstore.delete_all_course_assets(self.course1_key)
+        __, count = self.contentstore.get_all_content_for_course(self.course1_key)
+        self.assertEqual(count, 0)
+        # ensure it didn't remove any from other course
+        __, count = self.contentstore.get_all_content_for_course(self.course2_key)
+        self.assertEqual(count, len(self.course2_files))

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

@@ -210,7 +210,7 @@ class TestMixedModuleStore(LocMapperSetupSansDjango):
                 if index > 0:
                     store_configs[index], store_configs[0] = store_configs[0], store_configs[index]
                 break
-        self.store = MixedModuleStore(**self.options)
+        self.store = MixedModuleStore(None, **self.options)
         self.addCleanup(self.store.close_all_connections)
 
         # convert to CourseKeys
@@ -518,7 +518,7 @@ def load_function(path):
 
 
 # pylint: disable=unused-argument
-def create_modulestore_instance(engine, doc_store_config, options, i18n_service=None):
+def create_modulestore_instance(engine, contentstore, doc_store_config, options, i18n_service=None):
     """
     This will return a new instance of a modulestore given an engine and options
     """
@@ -526,6 +526,7 @@ def create_modulestore_instance(engine, doc_store_config, options, i18n_service=
 
     return class_(
         doc_store_config=doc_store_config,
+        contentstore=contentstore,
         branch_setting_func=lambda: ModuleStoreEnum.Branch.draft_preferred,
         **options
     )

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

+# pylint: disable=E1101
+# pylint: disable=W0212
 # pylint: disable=E0611
 from nose.tools import assert_equals, assert_raises, \
     assert_not_equals, assert_false, assert_true, assert_greater, assert_is_instance, assert_is_none
@@ -86,8 +88,8 @@ class TestMongoModuleStore(unittest.TestCase):
             cls.connection.drop_database(DB)
             cls.connection.close()
 
-    @staticmethod
-    def initdb():
+    @classmethod
+    def initdb(cls):
         # connect to the db
         doc_store_config = {
             'host': HOST,
@@ -101,6 +103,7 @@ class TestMongoModuleStore(unittest.TestCase):
         # Also test draft store imports
         #
         draft_store = DraftModuleStore(
+            content_store,
             doc_store_config, FS_ROOT, RENDER_TEMPLATE,
             default_class=DEFAULT_CLASS,
             branch_setting_func=lambda: ModuleStoreEnum.Branch.draft_preferred
@@ -109,7 +112,7 @@ class TestMongoModuleStore(unittest.TestCase):
             draft_store,
             999,
             DATA_DIR,
-            TestMongoModuleStore.courses,
+            cls.courses,
             static_content_store=content_store
         )
 
@@ -131,13 +134,6 @@ class TestMongoModuleStore(unittest.TestCase):
         # Destroy the test db.
         connection.drop_database(DB)
 
-    def setUp(self):
-        # make a copy for convenience
-        self.connection = TestMongoModuleStore.connection
-
-    def tearDown(self):
-        pass
-
     def test_init(self):
         '''Make sure the db loads'''
         ids = list(self.connection[DB][COLLECTION].find({}, {'_id': True}))
@@ -145,6 +141,7 @@ class TestMongoModuleStore(unittest.TestCase):
 
     def test_mongo_modulestore_type(self):
         store = MongoModuleStore(
+            None,
             {'host': HOST, 'db': DB, 'collection': COLLECTION},
             FS_ROOT, RENDER_TEMPLATE, default_class=DEFAULT_CLASS
         )
@@ -284,54 +281,57 @@ class TestMongoModuleStore(unittest.TestCase):
         Test getting, setting, and defaulting the locked attr and arbitrary attrs.
         """
         location = Location('edX', 'toy', '2012_Fall', 'course', '2012_Fall')
-        course_content, __ = TestMongoModuleStore.content_store.get_all_content_for_course(location.course_key)
+        course_content, __ = self.content_store.get_all_content_for_course(location.course_key)
         assert_true(len(course_content) > 0)
         # a bit overkill, could just do for content[0]
         for content in course_content:
             assert not content.get('locked', False)
-            asset_key = AssetLocation._from_deprecated_son(content['_id'], location.run)
-            assert not TestMongoModuleStore.content_store.get_attr(asset_key, 'locked', False)
-            attrs = TestMongoModuleStore.content_store.get_attrs(asset_key)
+            asset_key = AssetLocation._from_deprecated_son(content.get('content_son', content['_id']), location.run)
+            assert not self.content_store.get_attr(asset_key, 'locked', False)
+            attrs = self.content_store.get_attrs(asset_key)
             assert_in('uploadDate', attrs)
             assert not attrs.get('locked', False)
-            TestMongoModuleStore.content_store.set_attr(asset_key, 'locked', True)
-            assert TestMongoModuleStore.content_store.get_attr(asset_key, 'locked', False)
-            attrs = TestMongoModuleStore.content_store.get_attrs(asset_key)
+            self.content_store.set_attr(asset_key, 'locked', True)
+            assert self.content_store.get_attr(asset_key, 'locked', False)
+            attrs = self.content_store.get_attrs(asset_key)
             assert_in('locked', attrs)
             assert attrs['locked'] is True
-            TestMongoModuleStore.content_store.set_attrs(asset_key, {'miscel': 99})
-            assert_equals(TestMongoModuleStore.content_store.get_attr(asset_key, 'miscel'), 99)
+            self.content_store.set_attrs(asset_key, {'miscel': 99})
+            assert_equals(self.content_store.get_attr(asset_key, 'miscel'), 99)
 
-        asset_key = AssetLocation._from_deprecated_son(course_content[0]['_id'], location.run)
+        asset_key = AssetLocation._from_deprecated_son(
+            course_content[0].get('content_son', course_content[0]['_id']),
+            location.run
+        )
         assert_raises(
-            AttributeError, TestMongoModuleStore.content_store.set_attr, asset_key,
+            AttributeError, self.content_store.set_attr, asset_key,
             'md5', 'ff1532598830e3feac91c2449eaa60d6'
         )
         assert_raises(
-            AttributeError, TestMongoModuleStore.content_store.set_attrs, asset_key,
+            AttributeError, self.content_store.set_attrs, asset_key,
             {'foo': 9, 'md5': 'ff1532598830e3feac91c2449eaa60d6'}
         )
         assert_raises(
-            NotFoundError, TestMongoModuleStore.content_store.get_attr,
+            NotFoundError, self.content_store.get_attr,
             Location('bogus', 'bogus', 'bogus', 'asset', 'bogus'),
             'displayname'
         )
         assert_raises(
-            NotFoundError, TestMongoModuleStore.content_store.set_attr,
+            NotFoundError, self.content_store.set_attr,
             Location('bogus', 'bogus', 'bogus', 'asset', 'bogus'),
             'displayname', 'hello'
         )
         assert_raises(
-            NotFoundError, TestMongoModuleStore.content_store.get_attrs,
+            NotFoundError, self.content_store.get_attrs,
             Location('bogus', 'bogus', 'bogus', 'asset', 'bogus')
         )
         assert_raises(
-            NotFoundError, TestMongoModuleStore.content_store.set_attrs,
+            NotFoundError, self.content_store.set_attrs,
             Location('bogus', 'bogus', 'bogus', 'asset', 'bogus'),
             {'displayname': 'hello'}
         )
         assert_raises(
-            NotFoundError, TestMongoModuleStore.content_store.set_attrs,
+            NotFoundError, self.content_store.set_attrs,
             Location('bogus', 'bogus', 'bogus', 'asset', None),
             {'displayname': 'hello'}
         )

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

@@ -23,7 +23,7 @@ class TestMigration(SplitWMongoCourseBoostrapper):
         # pylint: disable=W0142
         self.loc_mapper = LocMapperStore(test_location_mapper.TrivialCache(), **self.db_config)
         self.split_mongo.loc_mapper = self.loc_mapper
-        self.migrator = SplitMigrator(self.split_mongo, self.old_mongo, self.draft_mongo, self.loc_mapper)
+        self.migrator = SplitMigrator(self.split_mongo, self.draft_mongo, self.loc_mapper)
 
     def tearDown(self):
         dbref = self.loc_mapper.db

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

@@ -1759,6 +1759,7 @@ def modulestore():
 
         # pylint: disable=W0142
         SplitModuleTest.modulestore = class_(
+            None,  # contentstore
             SplitModuleTest.MODULESTORE['DOC_STORE_CONFIG'],
             **options
         )

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

@@ -49,14 +49,15 @@ class SplitWMongoCourseBoostrapper(unittest.TestCase):
         self.userid = random.getrandbits(32)
         super(SplitWMongoCourseBoostrapper, self).setUp()
         self.split_mongo = SplitMongoModuleStore(
+            None,
             self.db_config,
             **self.modulestore_options
         )
         self.addCleanup(self.split_mongo.db.connection.close)
         self.addCleanup(self.tear_down_split)
-        self.old_mongo = MongoModuleStore(self.db_config, **self.modulestore_options)
+        self.old_mongo = MongoModuleStore(None, self.db_config, **self.modulestore_options)
         self.draft_mongo = DraftMongoModuleStore(
-            self.db_config, branch_setting_func=lambda: ModuleStoreEnum.Branch.draft_preferred, **self.modulestore_options
+            None, self.db_config, branch_setting_func=lambda: ModuleStoreEnum.Branch.draft_preferred, **self.modulestore_options
         )
         self.addCleanup(self.tear_down_mongo)
         self.old_course_key = None

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

@@ -85,6 +85,7 @@ def modulestore():
 
         # pylint: disable=W0142
         ModuleStoreNoSettings.modulestore = class_(
+            None,  # contentstore
             ModuleStoreNoSettings.MODULESTORE['DOC_STORE_CONFIG'],
             **options
         )

common/lib/xmodule/xmodule/modulestore/xml.py

@@ -19,15 +19,14 @@ from xmodule.errortracker import make_error_tracker, exc_info_to_str
 from xmodule.mako_module import MakoDescriptorSystem
 from xmodule.x_module import XMLParsingSystem, policy_key
 from xmodule.modulestore.xml_exporter import DEFAULT_CONTENT_FIELDS
-from xmodule.modulestore import ModuleStoreEnum
+from xmodule.modulestore import ModuleStoreEnum, ModuleStoreReadBase
 from xmodule.tabs import CourseTabList
 from opaque_keys.edx.keys import UsageKey
-from opaque_keys.edx.locations import SlashSeparatedCourseKey
+from opaque_keys.edx.locations import SlashSeparatedCourseKey, Location
 
 from xblock.field_data import DictFieldData
 from xblock.runtime import DictKeyValueStore, IdGenerator
 
-from . import ModuleStoreReadBase, Location, ModuleStoreEnum
 
 from .exceptions import ItemNotFoundError
 from .inheritance import compute_inherited_metadata, inheriting_field_data
@@ -720,7 +719,7 @@ class XMLModuleStore(ModuleStoreReadBase):
         except KeyError:
             raise ItemNotFoundError(usage_key)
 
-    def get_items(self, course_id, settings=None, content=None, **kwargs):
+    def get_items(self, course_id, settings=None, content=None, revision=None, **kwargs):
         """
         Returns:
             list of XModuleDescriptor instances for the matching items within the course with
@@ -745,6 +744,9 @@ class XMLModuleStore(ModuleStoreReadBase):
                 you can search dates by providing either a datetime for == (probably
                 useless) or a tuple (">"|"<" datetime) for after or before, etc.
         """
+        if revision == ModuleStoreEnum.RevisionOption.draft_only:
+            return []
+
         items = []
 
         category = kwargs.pop('category', None)

common/lib/xmodule/xmodule/modulestore/xml_importer.py

@@ -97,7 +97,7 @@ def import_static_content(
             try:
                 static_content_store.save(content)
             except Exception as err:
-                log.exception('Error importing {0}, error={1}'.format(
+                log.exception(u'Error importing {0}, error={1}'.format(
                     fullname_with_subpath, err
                 ))
 

common/lib/xmodule/xmodule/video_module/transcripts_utils.py

@@ -500,8 +500,7 @@ class Transcript(object):
         Delete asset by location and filename.
         """
         try:
-            content = Transcript.get_asset(location, filename)
-            contentstore().delete(content.get_id())
+            contentstore().delete(Transcript.asset_location(location, filename))
             log.info("Transcript asset %s was removed from store.", filename)
         except NotFoundError:
             pass

common/test/data/static/contains.sh

+#!/usr/bin/env zsh
+git log --all ^opaque-keys-merge-base --format=%H $1 | while read f; do git branch --contains $f; done | sort -u

lms/djangoapps/courseware/tests/test_video_handlers.py

@@ -65,7 +65,10 @@ def _clear_assets(location):
 
     assets, __ = store.get_all_content_for_course(location.course_key)
     for asset in assets:
-        asset_location = AssetLocation._from_deprecated_son(asset["_id"], location.course_key.run)
+        asset_location = AssetLocation._from_deprecated_son(
+            asset.get('content_son', asset["_id"]),
+            location.course_key.run
+        )
         del_cached_content(asset_location)
         store.delete(asset_location)
 

mongo_indexes.md

@@ -26,10 +26,16 @@ fs.files:
 Index needed thru 'category' by `_get_all_content_for_course` and others. That query also takes a sort
 which can be `uploadDate`, `display_name`, 
 
+Replace existing index which leaves out `run` with this one:
 ```
-ensureIndex({'_id.tag': 1, '_id.org': 1, '_id.course': 1, '_id.category': 1})
+ensureIndex({'_id.tag': 1, '_id.org': 1, '_id.course': 1, '_id.category': 1, '_id.run': 1})
+ensureIndex({'content_son.tag': 1, 'content_son.org': 1, 'content_son.course': 1, 'content_son.category': 1, 'content_son.run': 1})
 ```
 
+Note: I'm not advocating adding one which leaves out `category` for now because that would only be
+used for `delete_all_course_assets` which in the future should not actually delete the assets except
+when doing garbage collection.
+
 Remove index on `displayname`
 
 modulestore: