From 7a55f1207d7f03f7ce594c8482f5848a5026653f Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 7 Jul 2014 15:32:21 -0400 Subject: [PATCH] Add tests of cross-modulestore import/export [LMS-2945] --- .../lib/xmodule/xmodule/modulestore/django.py | 1 + .../lib/xmodule/xmodule/modulestore/mixed.py | 6 +- .../xmodule/modulestore/mongo/draft.py | 3 +- .../test_cross_modulestore_import_export.py | 303 ++++++++++++++++++ .../tests/test_mixed_modulestore.py | 8 +- .../xmodule/modulestore/xml_importer.py | 1 - common/lib/xmodule/xmodule/tests/__init__.py | 143 ++++++++- 7 files changed, 453 insertions(+), 12 deletions(-) create mode 100644 common/lib/xmodule/xmodule/modulestore/tests/test_cross_modulestore_import_export.py diff --git a/common/lib/xmodule/xmodule/modulestore/django.py b/common/lib/xmodule/xmodule/modulestore/django.py index 973833f4f8..fbcc491ac7 100644 --- a/common/lib/xmodule/xmodule/modulestore/django.py +++ b/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 ) diff --git a/common/lib/xmodule/xmodule/modulestore/mixed.py b/common/lib/xmodule/xmodule/modulestore/mixed.py index 4811163429..34a19dd2c0 100644 --- a/common/lib/xmodule/xmodule/modulestore/mixed.py +++ b/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 = {} diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py index 9e26c211dc..98cdc9fad7 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py +++ b/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) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_cross_modulestore_import_export.py b/common/lib/xmodule/xmodule/modulestore/tests/test_cross_modulestore_import_export.py new file mode 100644 index 0000000000..6763838fde --- /dev/null +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_cross_modulestore_import_export.py @@ -0,0 +1,303 @@ +""" +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, + ) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py index 7dc251b4e1..1c4f94d102 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/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 diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index 2de6a9b01a..158c9001d8 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/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) diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index d79bd4366c..e7c8e06912 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/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)