Add tests of cross-modulestore import/export

[LMS-2945]

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

@@ -71,6 +71,7 @@ def create_modulestore_instance(engine, content_store, doc_store_config, options
         doc_store_config=doc_store_config,
         i18n_service=i18n_service or ModuleI18nService(),
         branch_setting_func=_get_modulestore_branch_setting,
+        create_modulestore_instance=create_modulestore_instance,
         **_options
     )
 

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

@@ -12,7 +12,6 @@ from opaque_keys import InvalidKeyError
 
 from . import ModuleStoreWriteBase
 from xmodule.modulestore import ModuleStoreEnum
-from xmodule.modulestore.django import create_modulestore_instance
 from opaque_keys.edx.locator import CourseLocator, BlockUsageLocator
 from xmodule.modulestore.exceptions import ItemNotFoundError
 from opaque_keys.edx.keys import CourseKey, UsageKey
@@ -30,13 +29,16 @@ class MixedModuleStore(ModuleStoreWriteBase):
     """
     ModuleStore knows how to route requests to the right persistence ms
     """
-    def __init__(self, contentstore, mappings, stores, i18n_service=None, **kwargs):
+    def __init__(self, contentstore, mappings, stores, i18n_service=None, create_modulestore_instance=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__(contentstore, **kwargs)
 
+        if create_modulestore_instance is None:
+            raise ValueError('MixedModuleStore constructor must be passed a create_modulestore_instance function')
+
         self.modulestores = []
         self.mappings = {}
 

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

@@ -50,7 +50,8 @@ class DraftModuleStore(MongoModuleStore):
     def __init__(self, *args, **kwargs):
         """
         Args:
-            branch_setting_func: a function that returns the branch setting to use for this store's operations
+            branch_setting_func: a function that returns the branch setting to use for this store's operations.
+                This should be an attribute from ModuleStoreEnum.Branch
         """
         super(DraftModuleStore, self).__init__(*args, **kwargs)
         self.branch_setting_func = kwargs.pop('branch_setting_func', lambda: ModuleStoreEnum.Branch.published_only)

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

+"""
+This suite of tests verifies that courses exported from one modulestore can be imported into
+another modulestore and the result will be identical (ignoring changes to identifiers that are
+the result of being imported into a course with a different course id).
+
+It does this by providing facilities for creating and cleaning up each of the modulestore types,
+and then for each combination of modulestores, performing the sequence:
+    1) use xml_importer to read a course from xml from disk into the first modulestore (called the source)
+    2) use xml_exporter to dump the course from the source modulestore to disk
+    3) use xml_importer to read the dumped course into a second modulestore (called the destination)
+    4) Compare all modules in the source and destination modulestores to make sure that they line up
+
+"""
+
+import ddt
+import itertools
+import random
+from contextlib import contextmanager, nested
+from functools import partial
+from unittest import TestCase
+from shutil import rmtree
+from tempfile import mkdtemp
+from opaque_keys.edx.locations import SlashSeparatedCourseKey
+
+from xmodule.tests import CourseComparisonTest
+
+from xmodule.modulestore.split_mongo.split import SplitMongoModuleStore
+from xmodule.modulestore.mongo.base import ModuleStoreEnum
+from xmodule.modulestore.mongo.draft import DraftModuleStore
+from xmodule.modulestore.mixed import MixedModuleStore
+from xmodule.contentstore.mongo import MongoContentStore
+from xmodule.modulestore.xml_importer import import_from_xml
+from xmodule.modulestore.xml_exporter import export_to_xml
+
+COMMON_DOCSTORE_CONFIG = {
+    'host': 'localhost'
+}
+
+
+class MemoryCache(object):
+    """
+    This fits the metadata_inheritance_cache_subsystem interface used by
+    the modulestore, and stores the data in a dictionary in memory.
+    """
+    def __init__(self):
+        self._data = {}
+
+    def get(self, key, default=None):
+        return self._data.get(key, default)
+
+    def set(self, key, value):
+        self._data[key] = value
+
+
+class MongoModulestoreBuilder(object):
+    """
+    A builder class for a DraftModuleStore.
+    """
+    @contextmanager
+    def build(self, contentstore):
+        """
+        A contextmanager that returns an isolated mongo modulestore, and then deletes
+        all of its data at the end of the context.
+
+        Args:
+            contentstore: The contentstore that this modulestore should use to store
+                all of its assets.
+        """
+        doc_store_config = dict(
+            db='modulestore{}'.format(random.randint(0, 10000)),
+            collection='xmodule',
+            **COMMON_DOCSTORE_CONFIG
+        )
+
+        # Set up a temp directory for storing filesystem content created during import
+        fs_root = mkdtemp()
+
+        modulestore = DraftModuleStore(
+            contentstore,
+            doc_store_config,
+            fs_root,
+            render_template=repr,
+            branch_setting_func=lambda: ModuleStoreEnum.Branch.draft_preferred,
+            metadata_inheritance_cache_subsystem=MemoryCache(),
+        )
+
+        try:
+            yield modulestore
+        finally:
+            # Delete the created database
+            db = modulestore.database
+            db.connection.drop_database(db)
+            db.connection.close()
+
+            # Delete the created directory on the filesystem
+            rmtree(fs_root)
+
+    def __repr__(self):
+        return 'MongoModulestoreBuilder()'
+
+
+class VersioningModulestoreBuilder(object):
+    """
+    A builder class for a VersioningModuleStore.
+    """
+    @contextmanager
+    def build(self, contentstore):
+        """
+        A contextmanager that returns an isolated versioning modulestore, and then deletes
+        all of its data at the end of the context.
+
+        Args:
+            contentstore: The contentstore that this modulestore should use to store
+                all of its assets.
+        """
+        doc_store_config = dict(
+            db='modulestore{}'.format(random.randint(0, 10000)),
+            collection='split_module',
+            **COMMON_DOCSTORE_CONFIG
+        )
+        # Set up a temp directory for storing filesystem content created during import
+        fs_root = mkdtemp()
+
+        modulestore = SplitMongoModuleStore(
+            contentstore,
+            doc_store_config,
+            fs_root,
+            render_template=repr,
+        )
+
+        try:
+            yield modulestore
+        finally:
+            # Delete the created database
+            db = modulestore.db
+            db.connection.drop_database(db)
+            db.connection.close()
+
+            # Delete the created directory on the filesystem
+            rmtree(fs_root)
+
+    def __repr__(self):
+        return 'SplitModulestoreBuilder()'
+
+class MixedModulestoreBuilder(object):
+    """
+    A builder class for a MixedModuleStore.
+    """
+    def __init__(self, store_builders, mappings=None):
+        """
+        Args:
+            store_builders: A list of modulestore builder objects. These will be instantiated, in order,
+                as the backing stores for the MixedModuleStore.
+            mappings: Any course mappings to pass to the MixedModuleStore on instantiation.
+        """
+        self.store_builders = store_builders
+        self.mappings = mappings or {}
+
+    @contextmanager
+    def build(self, contentstore):
+        """
+        A contextmanager that returns a mixed modulestore built on top of modulestores
+        generated by other builder classes.
+
+        Args:
+            contentstore: The contentstore that this modulestore should use to store
+                all of its assets.
+        """
+        names, generators = zip(*self.store_builders)
+
+        with nested(*(gen.build(contentstore) for gen in generators)) as modulestores:
+            # Make the modulestore creation function just return the already-created modulestores
+            store_iterator = iter(modulestores)
+            create_modulestore_instance = lambda *args, **kwargs: store_iterator.next()
+
+            # Generate a fake list of stores to give the already generated stores appropriate names
+            stores = [{'NAME': name, 'ENGINE': 'This space deliberately left blank'} for name in names]
+
+            modulestore = MixedModuleStore(contentstore, self.mappings, stores, create_modulestore_instance=create_modulestore_instance)
+
+            yield modulestore
+
+    def __repr__(self):
+        return 'MixedModulestoreBuilder({!r}, {!r})'.format(self.store_builders, self.mappings)
+
+
+class MongoContentstoreBuilder(object):
+    """
+    A builder class for a MongoContentStore.
+    """
+    @contextmanager
+    def build(self):
+        """
+        A contextmanager that returns a MongoContentStore, and deletes its contents
+        when the context closes.
+        """
+        contentstore = MongoContentStore(
+            db='contentstore{}'.format(random.randint(0, 10000)),
+            collection='content',
+            **COMMON_DOCSTORE_CONFIG
+        )
+
+        try:
+            yield contentstore
+        finally:
+            # Delete the created database
+            db = contentstore.fs_files.database
+            db.connection.drop_database(db)
+            db.connection.close()
+
+    def __repr__(self):
+        return 'MongoContentstoreBuilder()'
+
+
+MODULESTORE_SETUPS = (
+    MongoModulestoreBuilder(),
+    VersioningModulestoreBuilder(),
+    MixedModulestoreBuilder([('draft', MongoModulestoreBuilder())]),
+    MixedModulestoreBuilder([('split', VersioningModulestoreBuilder())]),
+)
+CONTENTSTORE_SETUPS = (MongoContentstoreBuilder(),)
+COURSE_DATA_NAMES = ('toy', 'test_unicode')
+
+@ddt.ddt
+class CrossStoreXMLRoundtrip(CourseComparisonTest):
+    """
+    This class exists to test XML import and export between different modulestore
+    classes.
+    """
+
+    def setUp(self):
+        super(CrossStoreXMLRoundtrip, self).setUp()
+        self.export_dir = mkdtemp()
+        self.addCleanup(rmtree, self.export_dir)
+
+    @ddt.data(*itertools.product(
+        MODULESTORE_SETUPS,
+        MODULESTORE_SETUPS,
+        CONTENTSTORE_SETUPS,
+        CONTENTSTORE_SETUPS,
+        COURSE_DATA_NAMES,
+    ))
+    @ddt.unpack
+    def test_round_trip(self, source_builder, dest_builder, source_content_builder, dest_content_builder, course_data_name):
+        self.maxDiff = None
+        source_course_key = SlashSeparatedCourseKey('source', 'course', 'key')
+        dest_course_key = SlashSeparatedCourseKey('dest', 'course', 'key')
+
+        # Construct the contentstore for storing the first import
+        with source_content_builder.build() as source_content:
+            # Construct the modulestore for storing the first import (using the previously created contentstore)
+            with source_builder.build(source_content) as source_store:
+                # Construct the contentstore for storing the second import
+                with dest_content_builder.build() as dest_content:
+                    # Construct the modulestore for storing the second import (using the second contentstore)
+                    with dest_builder.build(dest_content) as dest_store:
+                        import_from_xml(
+                            source_store,
+                            'test_user',
+                            'common/test/data',
+                            course_dirs=[course_data_name],
+                            static_content_store=source_content,
+                            target_course_id=source_course_key,
+                            create_new_course_if_not_present=True,
+                        )
+
+                        export_to_xml(
+                            source_store,
+                            source_content,
+                            source_course_key,
+                            self.export_dir,
+                            'exported_course',
+                        )
+
+                        import_from_xml(
+                            dest_store,
+                            'test_user',
+                            self.export_dir,
+                            static_content_store=dest_content,
+                            target_course_id=dest_course_key,
+                            create_new_course_if_not_present=True,
+                        )
+
+                        self.exclude_field(source_course_key.make_usage_key('course', 'key'), 'wiki_slug')
+                        self.exclude_field(None, 'xml_attributes')
+                        self.ignore_asset_key('_id')
+                        self.ignore_asset_key('uploadDate')
+                        self.ignore_asset_key('content_son')
+                        self.ignore_asset_key('thumbnail_location')
+
+                        self.assertCoursesEqual(
+                            source_store,
+                            source_course_key,
+                            dest_store,
+                            dest_course_key,
+                        )
+
+                        self.assertAssetsEqual(
+                            source_content,
+                            source_course_key,
+                            dest_content,
+                            dest_course_key,
+                        )

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

@@ -104,12 +104,6 @@ class TestMixedModuleStore(unittest.TestCase):
         self.addCleanup(self.connection.close)
         super(TestMixedModuleStore, self).setUp()
 
-        patcher = patch.multiple(
-            'xmodule.modulestore.mixed',
-            create_modulestore_instance=create_modulestore_instance,
-        )
-        patcher.start()
-        self.addCleanup(patcher.stop)
         self.addTypeEqualityFunc(BlockUsageLocator, '_compareIgnoreVersion')
         self.addTypeEqualityFunc(CourseLocator, '_compareIgnoreVersion')
         # define attrs which get set in initdb to quell pylint
@@ -207,7 +201,7 @@ class TestMixedModuleStore(unittest.TestCase):
                 if index > 0:
                     store_configs[index], store_configs[0] = store_configs[0], store_configs[index]
                 break
-        self.store = MixedModuleStore(None, **self.options)
+        self.store = MixedModuleStore(None, create_modulestore_instance=create_modulestore_instance, **self.options)
         self.addCleanup(self.store.close_all_connections)
 
         # convert to CourseKeys

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

@@ -151,7 +151,6 @@ def import_from_xml(
 
     # If we're going to remap the course_id, then we can only do that with
     # a single course
-
     if target_course_id:
         assert(len(xml_module_store.modules) == 1)
 

common/lib/xmodule/xmodule/tests/__init__.py

@@ -19,7 +19,7 @@ from xblock.field_data import DictFieldData
 from xblock.fields import ScopeIds
 
 from xmodule.x_module import ModuleSystem, XModuleDescriptor, XModuleMixin
-from xmodule.modulestore.inheritance import InheritanceMixin
+from xmodule.modulestore.inheritance import InheritanceMixin, own_metadata
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
 from xmodule.mako_module import MakoDescriptorSystem
 from xmodule.error_module import ErrorDescriptor
@@ -154,3 +154,144 @@ class LogicTest(unittest.TestCase):
     def ajax_request(self, dispatch, data):
         """Call Xmodule.handle_ajax."""
         return json.loads(self.xmodule.handle_ajax(dispatch, data))
+
+
+class CourseComparisonTest(unittest.TestCase):
+    """
+    Mixin that has methods for comparing courses for equality.
+    """
+
+    def setUp(self):
+        self.field_exclusions = set()
+        self.ignored_asset_keys = set()
+
+    def exclude_field(self, usage_id, field_name):
+        """
+        Mark field ``field_name`` of expected block usage ``usage_id`` as ignored
+
+        Args:
+            usage_id (:class:`opaque_keys.edx.UsageKey` or ``None``). If ``None``, skip, this field in all blocks
+            field_name (string): The name of the field to skip
+        """
+        self.field_exclusions.add((usage_id, field_name))
+
+    def ignore_asset_key(self, key_name):
+        self.ignored_asset_keys.add(key_name)
+
+    def assertCoursesEqual(self, expected_store, expected_course_key, actual_store, actual_course_key):
+        """
+        Assert that the courses identified by ``expected_course_key`` in ``expected_store`` and
+        ``actual_course_key`` in ``actual_store`` are identical (ignore differences related
+        owing to the course_keys being different).
+
+        Any field value mentioned in ``self.field_exclusions`` by the key (usage_id, field_name)
+        will be ignored for the purpose of equality checking.
+        """
+        expected_items = expected_store.get_items(expected_course_key)
+        actual_items = actual_store.get_items(actual_course_key)
+        self.assertGreater(len(expected_items), 0)
+        self.assertEqual(len(expected_items), len(actual_items))
+
+        actual_item_map = {item.location: item for item in actual_items}
+
+        for expected_item in expected_items:
+            actual_item_location = expected_item.location.map_into_course(actual_course_key)
+            if expected_item.location.category == 'course':
+                actual_item_location = actual_item_location.replace(name=actual_item_location.run)
+            actual_item = actual_item_map.get(actual_item_location)
+
+            # compare published state
+            exp_pub_state = expected_store.compute_publish_state(expected_item)
+            act_pub_state = actual_store.compute_publish_state(actual_item)
+            self.assertEqual(
+                exp_pub_state,
+                act_pub_state,
+                'Published states for usages {} and {} differ: {!r} != {!r}'.format(
+                    expected_item.location,
+                    actual_item.location,
+                    exp_pub_state,
+                    act_pub_state
+                )
+            )
+
+            # compare fields
+            self.assertEqual(expected_item.fields, actual_item.fields)
+
+            for field_name in expected_item.fields:
+                if (expected_item.scope_ids.usage_id, field_name) in self.field_exclusions:
+                    continue
+
+                if (None, field_name) in self.field_exclusions:
+                    continue
+
+                # Children are handled specially
+                if field_name == 'children':
+                    continue
+
+                exp_value = getattr(expected_item, field_name)
+                actual_value = getattr(actual_item, field_name)
+                self.assertEqual(
+                    exp_value,
+                    actual_value,
+                    "Field {} doesn't match between usages {} and {}: {!r} != {!r}".format(
+                        field_name,
+                        expected_item.scope_ids.usage_id,
+                        actual_item.scope_ids.usage_id,
+                        exp_value,
+                        actual_value,
+                    )
+                )
+
+            # compare children
+            self.assertEqual(expected_item.has_children, actual_item.has_children)
+            if expected_item.has_children:
+                expected_children = []
+                for course1_item_child in expected_item.children:
+                    expected_children.append(
+                        course1_item_child.map_into_course(actual_course_key)
+                    )
+                self.assertEqual(expected_children, actual_item.children)
+
+    def assertAssetEqual(self, expected_course_key, expected_asset, actual_course_key, actual_asset):
+        for key in self.ignored_asset_keys:
+            if key in expected_asset:
+                del expected_asset[key]
+            if key in actual_asset:
+                del actual_asset[key]
+
+        expected_key = expected_asset.pop('asset_key')
+        actual_key = actual_asset.pop('asset_key')
+        self.assertEqual(expected_key.map_into_course(actual_course_key), actual_key)
+        self.assertEqual(expected_key, actual_key.map_into_course(expected_course_key))
+
+        expected_filename = expected_asset.pop('filename')
+        actual_filename = actual_asset.pop('filename')
+        self.assertEqual(expected_key.to_deprecated_string(), expected_filename)
+        self.assertEqual(actual_key.to_deprecated_string(), actual_filename)
+        self.assertEqual(expected_asset, actual_asset)
+
+    def _assertAssetsEqual(self, expected_course_key, expected_assets, actual_course_key, actual_assets):
+        self.assertEqual(len(expected_assets), len(actual_assets))
+
+        actual_assets_map = {asset['asset_key']: asset for asset in actual_assets}
+        for expected_item in expected_assets:
+            actual_item = actual_assets_map[expected_item['asset_key'].map_into_course(actual_course_key)]
+            self.assertAssetEqual(expected_course_key, expected_item, actual_course_key, actual_item)
+
+    def assertAssetsEqual(self, expected_store, expected_course_key, actual_store, actual_course_key):
+        """
+        Assert that the course assets identified by ``expected_course_key`` in ``expected_store`` and
+        ``actual_course_key`` in ``actual_store`` are identical, allowing for differences related
+        to their being from different course keys.
+        """
+
+        expected_content, expected_count = expected_store.get_all_content_for_course(expected_course_key)
+        actual_content, actual_count = actual_store.get_all_content_for_course(actual_course_key)
+
+        self.assertEqual(expected_count, actual_count)
+        self._assertAssetsEqual(expected_course_key, expected_content, actual_course_key, actual_content)
+
+        expected_thumbs = expected_store.get_all_content_thumbnails_for_course(expected_course_key)
+        actual_thumbs = actual_store.get_all_content_thumbnails_for_course(actual_course_key)
+
+        self._assertAssetsEqual(expected_course_key, expected_thumbs, actual_course_key, actual_thumbs)