+ Staff Member #1
+Biography of instructor/staff member #1
+diff --git a/cms/djangoapps/contentstore/views/assets.py b/cms/djangoapps/contentstore/views/assets.py
index b1da60bbbc..fd77774dae 100644
--- a/cms/djangoapps/contentstore/views/assets.py
+++ b/cms/djangoapps/contentstore/views/assets.py
@@ -121,8 +121,7 @@ def _assets_json(request, course_key):
asset_json = []
for asset in assets:
- asset_id = asset.get('content_son', asset['_id'])
- asset_location = StaticContent.compute_location(course_key, asset_id['name'])
+ asset_location = asset['asset_key']
# note, due to the schema change we may not have a 'thumbnail_location' in the result set
thumbnail_location = asset.get('thumbnail_location', None)
if thumbnail_location:
diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py
index 84bc8b768f..b8dc24cf68 100644
--- a/common/lib/xmodule/xmodule/capa_module.py
+++ b/common/lib/xmodule/xmodule/capa_module.py
@@ -125,11 +125,6 @@ class CapaDescriptor(CapaFields, RawDescriptor):
]
}
- # Capa modules have some additional metadata:
- # TODO (vshnayder): do problems have any other metadata? Do they
- # actually use type and points?
- metadata_attributes = RawDescriptor.metadata_attributes + ('type', 'points')
-
# The capa format specifies that what we call max_attempts in the code
# is the attribute `attempts`. This will do that conversion
metadata_translations = dict(RawDescriptor.metadata_translations)
diff --git a/common/lib/xmodule/xmodule/combined_open_ended_module.py b/common/lib/xmodule/xmodule/combined_open_ended_module.py
index ee48c8cbe7..11eece3e1d 100644
--- a/common/lib/xmodule/xmodule/combined_open_ended_module.py
+++ b/common/lib/xmodule/xmodule/combined_open_ended_module.py
@@ -497,8 +497,6 @@ class CombinedOpenEndedDescriptor(CombinedOpenEndedFields, RawDescriptor):
#Specify whether or not to pass in open ended interface
needs_open_ended_interface = True
- metadata_attributes = RawDescriptor.metadata_attributes
-
js = {'coffee': [resource_string(__name__, 'js/src/combinedopenended/edit.coffee')]}
js_module_name = "OpenEndedMarkdownEditingDescriptor"
css = {'scss': [resource_string(__name__, 'css/editor/edit.scss'), resource_string(__name__, 'css/combinedopenended/edit.scss')]}
diff --git a/common/lib/xmodule/xmodule/contentstore/content.py b/common/lib/xmodule/xmodule/contentstore/content.py
index 3bb0a5e5d5..a6c092d900 100644
--- a/common/lib/xmodule/xmodule/contentstore/content.py
+++ b/common/lib/xmodule/xmodule/contentstore/content.py
@@ -188,23 +188,13 @@ class ContentStore(object):
Returns a list of static assets for a course, followed by the total number of assets.
By default all assets are returned, but start and maxresults can be provided to limit the query.
- The return format is a list of dictionary elements. Example:
-
- [
-
- {u'displayname': u'profile.jpg', u'chunkSize': 262144, u'length': 85374,
- u'uploadDate': datetime.datetime(2012, 10, 3, 5, 41, 54, 183000), u'contentType': u'image/jpeg',
- u'_id': {u'category': u'asset', u'name': u'profile.jpg', u'course': u'6.002x', u'tag': u'c4x',
- u'org': u'MITx', u'revision': None}, u'md5': u'36dc53519d4b735eb6beba51cd686a0e'},
-
- {u'displayname': u'profile.thumbnail.jpg', u'chunkSize': 262144, u'length': 4073,
- u'uploadDate': datetime.datetime(2012, 10, 3, 5, 41, 54, 196000), u'contentType': u'image/jpeg',
- u'_id': {u'category': u'asset', u'name': u'profile.thumbnail.jpg', u'course': u'6.002x', u'tag': u'c4x',
- u'org': u'MITx', u'revision': None}, u'md5': u'ff1532598830e3feac91c2449eaa60d6'},
-
- ....
-
- ]
+ The return format is a list of asset data dictionaries.
+ The asset data dictionaries have the following keys:
+ asset_key (:class:`opaque_keys.edx.AssetKey`): The key of the asset
+ displayname: The human-readable name of the asset
+ uploadDate (datetime.datetime): The date and time that the file was uploadDate
+ contentType: The mimetype string of the asset
+ md5: An md5 hash of the asset content
'''
raise NotImplementedError
diff --git a/common/lib/xmodule/xmodule/contentstore/mongo.py b/common/lib/xmodule/xmodule/contentstore/mongo.py
index 7ae258c421..c3e7571efa 100644
--- a/common/lib/xmodule/xmodule/contentstore/mongo.py
+++ b/common/lib/xmodule/xmodule/contentstore/mongo.py
@@ -146,9 +146,6 @@ class MongoContentStore(ContentStore):
assets, __ = self.get_all_content_for_course(course_key)
for asset in assets:
- asset_id = asset.get('content_son', asset['_id'])
- # assuming course_key's deprecated flag is controlling rather than presence or absence of 'run' in _id
- asset_location = course_key.make_asset_key(asset_id['category'], asset_id['name'])
# TODO: On 6/19/14, I had to put a try/except around this
# to export a course. The course failed on JSON files in
# the /static/ directory placed in it with an import.
@@ -157,10 +154,10 @@ class MongoContentStore(ContentStore):
#
# When debugging course exports, this might be a good place
# to look. -- pmitros
- self.export(asset_location, output_directory)
+ self.export(asset['asset_key'], output_directory)
for attr, value in asset.iteritems():
- if attr not in ['_id', 'md5', 'uploadDate', 'length', 'chunkSize']:
- policy.setdefault(asset_location.name, {})[attr] = value
+ if attr not in ['_id', 'md5', 'uploadDate', 'length', 'chunkSize', 'asset_key']:
+ policy.setdefault(asset['asset_key'].name, {})[attr] = value
with open(assets_policy_file, 'w') as f:
json.dump(policy, f)
@@ -175,23 +172,14 @@ class MongoContentStore(ContentStore):
def _get_all_content_for_course(self, course_key, get_thumbnails=False, start=0, maxresults=-1, sort=None):
'''
- Returns a list of all static assets for a course. The return format is a list of dictionary elements. Example:
+ Returns a list of all static assets for a course. The return format is a list of asset data dictionary elements.
- [
-
- {u'displayname': u'profile.jpg', u'chunkSize': 262144, u'length': 85374,
- u'uploadDate': datetime.datetime(2012, 10, 3, 5, 41, 54, 183000), u'contentType': u'image/jpeg',
- u'_id': {u'category': u'asset', u'name': u'profile.jpg', u'course': u'6.002x', u'tag': u'c4x',
- u'org': u'MITx', u'revision': None}, u'md5': u'36dc53519d4b735eb6beba51cd686a0e'},
-
- {u'displayname': u'profile.thumbnail.jpg', u'chunkSize': 262144, u'length': 4073,
- u'uploadDate': datetime.datetime(2012, 10, 3, 5, 41, 54, 196000), u'contentType': u'image/jpeg',
- u'_id': {u'category': u'asset', u'name': u'profile.thumbnail.jpg', u'course': u'6.002x', u'tag': u'c4x',
- u'org': u'MITx', u'revision': None}, u'md5': u'ff1532598830e3feac91c2449eaa60d6'},
-
- ....
-
- ]
+ The asset data dictionaries have the following keys:
+ asset_key (:class:`opaque_keys.edx.AssetKey`): The key of the asset
+ displayname: The human-readable name of the asset
+ uploadDate (datetime.datetime): The date and time that the file was uploadDate
+ contentType: The mimetype string of the asset
+ md5: An md5 hash of the asset content
'''
if maxresults > 0:
items = self.fs_files.find(
@@ -203,7 +191,14 @@ class MongoContentStore(ContentStore):
query_for_course(course_key, "asset" if not get_thumbnails else "thumbnail"), sort=sort
)
count = items.count()
- return list(items), count
+ assets = list(items)
+
+ # We're constructing the asset key immediately after retrieval from the database so that
+ # callers are insulated from knowing how our identifiers are stored.
+ for asset in assets:
+ asset_id = asset.get('content_son', asset['_id'])
+ asset['asset_key'] = course_key.make_asset_key(asset_id['category'], asset_id['name'])
+ return assets, count
def set_attr(self, asset_key, attr, value=True):
"""
diff --git a/common/lib/xmodule/xmodule/modulestore/django.py b/common/lib/xmodule/xmodule/modulestore/django.py
index cd69fb1c7a..58e1a7acf4 100644
--- a/common/lib/xmodule/xmodule/modulestore/django.py
+++ b/common/lib/xmodule/xmodule/modulestore/django.py
@@ -70,6 +70,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/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py
index 01c67a3821..52865e1993 100644
--- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py
+++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py
@@ -54,6 +54,9 @@ SORT_REVISION_FAVOR_DRAFT = ('_id.revision', pymongo.DESCENDING)
# sort order that returns PUBLISHED items first
SORT_REVISION_FAVOR_PUBLISHED = ('_id.revision', pymongo.ASCENDING)
+BLOCK_TYPES_WITH_CHILDREN = list(set(
+ name for name, class_ in XBlock.load_classes() if getattr(class_, 'has_children', False)
+))
class MongoRevisionKey(object):
"""
@@ -460,14 +463,11 @@ class MongoModuleStore(ModuleStoreWriteBase):
# note this is a bit ugly as when we add new categories of containers, we have to add it here
course_id = self.fill_in_run(course_id)
- block_types_with_children = set(
- name for name, class_ in XBlock.load_classes() if getattr(class_, 'has_children', False)
- )
query = SON([
('_id.tag', 'i4x'),
('_id.org', course_id.org),
('_id.course', course_id.course),
- ('_id.category', {'$in': list(block_types_with_children)})
+ ('_id.category', {'$in': BLOCK_TYPES_WITH_CHILDREN})
])
# we just want the Location, children, and inheritable metadata
record_filter = {'_id': 1, 'definition.children': 1}
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/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py
index 82c0f83bda..015a915f48 100644
--- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py
+++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py
@@ -374,7 +374,10 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
'''
Gets the course descriptor for the course identified by the locator
'''
- assert(isinstance(course_id, CourseLocator))
+ if not isinstance(course_id, CourseLocator):
+ # The supplied CourseKey is of the wrong type, so it can't possibly be stored in this modulestore.
+ raise ItemNotFoundError(course_id)
+
course_entry = self._lookup_course(course_id)
root = course_entry['structure']['root']
result = self._load_items(course_entry, [root], 0, lazy=True)
@@ -389,7 +392,10 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
Note: we return the course_id instead of a boolean here since the found course may have
a different id than the given course_id when ignore_case is True.
'''
- assert(isinstance(course_id, CourseLocator))
+ if not isinstance(course_id, CourseLocator):
+ # The supplied CourseKey is of the wrong type, so it can't possibly be stored in this modulestore.
+ return False
+
course_index = self.db_connection.get_course_index(course_id, ignore_case)
return CourseLocator(course_index['org'], course_index['course'], course_index['run'], course_id.branch) if course_index else None
@@ -432,7 +438,10 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
descendants.
raises InsufficientSpecificationError or ItemNotFoundError
"""
- assert isinstance(usage_key, BlockUsageLocator)
+ if not isinstance(usage_key, BlockUsageLocator):
+ # The supplied UsageKey is of the wrong type, so it can't possibly be stored in this modulestore.
+ raise ItemNotFoundError(usage_key)
+
course = self._lookup_course(usage_key)
items = self._load_items(course, [usage_key.block_id], depth, lazy=True)
if len(items) == 0:
@@ -1375,7 +1384,10 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
change to this item, it raises a VersionConflictError unless force is True. In the force case, it forks
the course but leaves the head pointer where it is (this change will not be in the course head).
"""
- assert isinstance(usage_locator, BlockUsageLocator)
+ if not isinstance(usage_locator, BlockUsageLocator):
+ # The supplied UsageKey is of the wrong type, so it can't possibly be stored in this modulestore.
+ raise ItemNotFoundError(usage_locator)
+
original_structure = self._lookup_course(usage_locator.course_key)['structure']
if original_structure['root'] == usage_locator.block_id:
raise ValueError("Cannot delete the root of a course")
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..0ea2fae369
--- /dev/null
+++ b/common/lib/xmodule/xmodule/modulestore/tests/test_cross_modulestore_import_export.py
@@ -0,0 +1,319 @@
+"""
+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 unittest import SkipTest
+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):
+ """
+ Get a key from the cache.
+
+ Args:
+ key: The key to update.
+ default: The value to return if the key hasn't been set previously.
+ """
+ return self._data.get(key, default)
+
+ def set(self, key, value):
+ """
+ Set a key in the cache.
+
+ Args:
+ key: The key to update.
+ value: The value change the key to.
+ """
+ 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.
+ """
+ # pylint: disable=unreachable
+ raise SkipTest("DraftVersioningModuleStore doesn't yet support the same interface as the rest of the modulestores")
+ 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', 'manual-testing-complete')
+
+
+@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):
+ 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..824652fcf1 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,156 @@ 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):
+ """
+ Add an asset key to the list of keys to be ignored when comparing assets.
+
+ Args:
+ key_name: The name of the key to ignore.
+ """
+ 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 {!r} 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):
+ """
+ Assert that two assets are equal, allowing for differences related to their being from different courses.
+ """
+ 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): # pylint: disable=invalid-name
+ """
+ Private helper method for assertAssetsEqual
+ """
+ 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)
diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py
index 1092765821..4676b14159 100644
--- a/common/lib/xmodule/xmodule/xml_module.py
+++ b/common/lib/xmodule/xmodule/xml_module.py
@@ -123,17 +123,6 @@ class XmlDescriptor(XModuleDescriptor):
# Note -- url_name isn't in this list because it's handled specially on
# import and export.
- # TODO (vshnayder): Do we need a list of metadata we actually
- # understand? And if we do, is this the place?
- # Related: What's the right behavior for clean_metadata?
- metadata_attributes = ('format', 'graceperiod', 'showanswer', 'rerandomize',
- 'start', 'due', 'graded', 'display_name', 'url_name', 'hide_from_toc',
- 'ispublic', # if True, then course is listed for all users; see
- 'xqa_key', # for xqaa server access
- 'giturl', # url of git server for origin of file
- # VS[compat] Remove once unused.
- 'name', 'slug')
-
metadata_to_strip = ('data_dir',
'tabs', 'grading_policy', 'published_by', 'published_date',
'discussion_blackouts',
@@ -157,12 +146,12 @@ class XmlDescriptor(XModuleDescriptor):
@classmethod
def clean_metadata_from_xml(cls, xml_object):
"""
- Remove any attribute named in cls.metadata_attributes from the supplied
+ Remove any attribute named for a field with scope Scope.settings from the supplied
xml_object
"""
- for attr in cls.metadata_attributes:
- if xml_object.get(attr) is not None:
- del xml_object.attrib[attr]
+ for field_name, field in cls.fields.items():
+ if field.scope == Scope.settings and xml_object.get(field_name) is not None:
+ del xml_object.attrib[field_name]
@classmethod
def file_to_xml(cls, file_object):
@@ -220,6 +209,9 @@ class XmlDescriptor(XModuleDescriptor):
definition_xml = cls.load_file(filepath, system.resources_fs, def_id)
+ # Add the attributes from the pointer node
+ definition_xml.attrib.update(xml_object.attrib)
+
definition_metadata = get_metadata_from_xml(definition_xml)
cls.clean_metadata_from_xml(definition_xml)
definition, children = cls.definition_from_xml(definition_xml, system)
diff --git a/common/test/data/manual-testing-complete/about/overview.html b/common/test/data/manual-testing-complete/about/overview.html
new file mode 100644
index 0000000000..33911ae1ee
--- /dev/null
+++ b/common/test/data/manual-testing-complete/about/overview.html
@@ -0,0 +1,47 @@
+ Include your long course description here. The long course description should contain 150-400 words. This is paragraph 2 of the long course description. Add more paragraphs as needed. Make sure to enclose them in paragraph tags. Add information about course prerequisites here. Biography of instructor/staff member #1 Biography of instructor/staff member #2 No, a free online version of Chemistry: Principles, Patterns, and Applications, First Edition by Bruce Averill and Patricia Eldredge will be available, though you can purchase a printed version (published by FlatWorld Knowledge) if you’d like. Your answer would be displayed here. Enter your (optional) instructions for the exercise in HTML format. Annotations are specified by an Add your HTML with annotation spans here. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nulla facilisi. Enter your (optional) instructions for the exercise in HTML format. Annotations are specified by an Add your HTML with annotation spans here. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nulla facilisi. This is an example annotation. This page is required to post a comment Check this. Enter your (optional) instructions for the exercise in HTML format. Annotations are specified by an Add your HTML with annotation spans here.
+
+ Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nulla facilisi.
+ Hour 1 has four separate (two-part) annotation questions. To complete an Annotation Exercise in this course, hover your mouse over each highlighted portion of the focus text. When the Instructor prompt appears, read the question and click "Reply to Annotation." This will take you to a two-part task: an open-ended response question, and a question that will be answered by choosing one of multiple semantic tags.
+ Each of these exercises, one per hour, is meant to improve your understanding of the given text (in this case, Hour 1 Text C) by helping you analyze the context after you finish your slow reading of the text. But these exercises of annotation after slow reading do much more than that. They will build up your ability to understand not only the context but also [[1]] all other texts that contain similar contexts and [[2]] all the texts you will be reading in general. I can make this big promise because the texts that you are analyzing are part of a system, and the systematic thinking that went into the original texts can be decoded by analyzing the building blocks of the system. The way you analyze these building blocks is by trying to figure out how they are connected to the other building blocks in the system. That is what these annotation exercises are all about: they help you figure out the connections. And the more things you figure out, the more mental connections you can make on your own. But the exercise of making such connections through your annotations is a gradual process, and you need to be patient with yourself. You will get better and better at it, I am sure. The more connections you are able to make, the more powerful will be your reading ability. That is the beauty of analyzing something that was created by way of systematic thinking in an ancient tradition that took many centuries to develop (I estimate at least ten centuries). The tradition actually helps you think about the tradition. It will help you figure things out. By now you have seen that I really like the expression figure out: it puts the emphasis on reading out of the text, not into the text. When you read into the text, you lose sight of the system that had built that text. In the next exercise, I will switch metaphors in describing the system that had built the text. In the present exercise, I have been using the metaphor of building a structure with building blocks. In the next exercise I will start using the metaphor of weaving a fabric. (Here is a working definition of metaphor: it is an expression of meaning where one thing is substituted for another thing. For example, when we use the metaphor “thread of thought,” the idea of a thread, as used by someone who is weaving or sewing, is substituted for the idea of a way of thinking. For another example: when Nietzsche speaks about reading with delicate fingers, the idea of a goldsmith touching gold is substituted for the idea of reading a text with the eyes.) The main goal of this first exercise is to start practicing the art of slow reading. The best way to make a start, I think, is to read a story within a story. The inner story in this exercise is the story of the hero Hēraklēs, as retold by Agamemnon and as quoted by “Homer” in lines 78-138 of Iliad XIX. The outer story is the story of the Iliad itself, as retold by “Homer.” So the story within a story is 60 lines long, while the story of the Iliad itself is over 15,000 lines long. Your task in this exercise is to do a close reading of the short story of 60 lines, which is embedded in the long story of the Iliad. You can perform this task even if you haven’t started reading the Iliad, since I can summarize for you all 15,000+ lines of this huge epic in just three sentences here:
+ About This Course
+ Prerequisites
+ Course Staff
+
+ Staff Member #1
+
+ Staff Member #2
+ Frequently Asked Questions
+ Do I need to buy a textbook?
+ Question #2
+ <annotation> tag which may may have the following attributes:
+
+ title (optional). Title of the annotation. Defaults to Commentary if omitted.body (required). Text of the annotation.problem (optional). Numeric index of the problem associated with this annotation. This is a zero-based index, so the first problem on the page would have problem="0".highlight (optional). Possible values: yellow, red, orange, green, blue, or purple. Defaults to yellow if this attribute is omitted.<annotation> tag which may may have the following attributes:
+
+ title (optional). Title of the annotation. Defaults to Commentary if omitted.body (required). Text of the annotation.problem (optional). Numeric index of the problem associated with this annotation. This is a zero-based index, so the first problem on the page would have problem="0".highlight (optional). Possible values: yellow, red, orange, green, blue, or purple. Defaults to yellow if this attribute is omitted.<annotation> tag which may may have the following attributes:
+
+ title (optional). Title of the annotation. Defaults to Commentary if omitted.body (required). Text of the annotation.problem (optional). Numeric index of the problem associated with this annotation. This is a zero-based index, so the first problem on the page would have problem="0".highlight (optional). Possible values: yellow, red, orange, green, blue, or purple. Defaults to yellow if this attribute is omitted.
+
+
+
+
+
+
Now that you have the “big picture” of the Iliad as the outer story, you can have a very interesting experience as you do your slow reading of the inner story, contained in the 60 lines spoken by Agamemnon and telling the story of Hēraklēs.
+Here is something to keep in mind. The research of the great literary critic I. A. Richards, who was once a professor at Harvard University (he retired in 1963 and died in 1979), shows that the relationship of any kind of an outer story to any kind of an inner story is not necessarily a “one-way street,” as it were, leading from the outer into the inner story: there can also be a “two-way street,” with the inner story communicating with the outer story as well as the other way around. Where I use the expressions “outer story” and “inner story,” Richards used the terms “tenor” and “vehicle.” I have always found those terms rather forbidding, but, as you see, they stand for things that are fairly easy to explain.
+ + |76 Then Agamemnon, the king of men, spoke up at their meeting, |77 right there from the place where he was sitting, not even standing up in the middle of the assembly. |78 “Near and dear ones,” said he, “Danaan [= Achaean] heroes, attendants [therapontes] of Arēs! |79 It is a good thing to listen when a man stands up to speak, and it is not seemly |80 to speak in relay after him. It would be hard for someone to do that, even if he is a practiced speaker. |81 For how could any man in an assembly either hear anything when there is an uproar |82 or say anything? Even a public speaker who speaks clearly will be disconcerted by it. |83 What I will do is to make a declaration addressed to [Achilles] the son of Peleus. As for the rest of you |84 Argives [= Achaeans], you should understand and know well, each one of you, the words [mūthos] that I say for the record. |85 By now the Achaeans have been saying these words [mūthos] to me many times, |86 and they have been blaming me. But I am not responsible [aitios]. |87 No, those who are really responsible are Zeus and Fate [Moira] and the Fury [Erinys] who roams in the mist.
Iliad XIX 76-138
Enter your (optional) instructions for the exercise in HTML format.
+Annotations are specified by an <annotation> tag which may may have the following attributes:
title (optional). Title of the annotation. Defaults to Commentary if omitted.body (required). Text of the annotation.problem (optional). Numeric index of the problem associated with this annotation. This is a zero-based index, so the first problem on the page would have problem="0".highlight (optional). Possible values: yellow, red, orange, green, blue, or purple. Defaults to yellow if this attribute is omitted.Add your HTML with annotation spans here.
+Lorem ipsum dolor sit amet, consectetur adipiscing elit.
Nulla facilisi.
Enter your (optional) instructions for the exercise in HTML format.
+Annotations are specified by an <annotation> tag which may may have the following attributes:
title (optional). Title of the annotation. Defaults to Commentary if omitted.body (required). Text of the annotation.problem (optional). Numeric index of the problem associated with this annotation. This is a zero-based index, so the first problem on the page would have problem="0".highlight (optional). Possible values: yellow, red, orange, green, blue, or purple. Defaults to yellow if this attribute is omitted.Add your HTML with annotation spans here.
+Lorem ipsum dolor sit amet, consectetur adipiscing elit.
Nulla facilisi.
'All of us can think of a book that we hope none of our children or any other children have taken off the shelf. But if I have the right to remove that book from the shelf -- that work I abhor -- then you also have exactly the same right and so does everyone else. And then we have no books left on the shelf for any of us.' --Katherine Paterson, Author +
++ Write a persuasive essay to a newspaper reflecting your views on censorship in libraries. Do you believe that certain materials, such as books, music, movies, magazines, etc., should be removed from the shelves if they are found offensive? Support your position with convincing arguments from your own experience, observations, and/or reading. +
+'All of us can think of a book that we hope none of our children or any other children have taken off the shelf. But if I have the right to remove that book from the shelf -- that work I abhor -- then you also have exactly the same right and so does everyone else. And then we have no books left on the shelf for any of us.' --Katherine Paterson, Author +
++ Write a persuasive essay to a newspaper reflecting your views on censorship in libraries. Do you believe that certain materials, such as books, music, movies, magazines, etc., should be removed from the shelves if they are found offensive? Support your position with convincing arguments from your own experience, observations, and/or reading. +
+'All of us can think of a book that we hope none of our children or any other children have taken off the shelf. But if I have the right to remove that book from the shelf -- that work I abhor -- then you also have exactly the same right and so does everyone else. And then we have no books left on the shelf for any of us.' --Katherine Paterson, Author +
++ Write a persuasive essay to a newspaper reflecting your views on censorship in libraries. Do you believe that certain materials, such as books, music, movies, magazines, etc., should be removed from the shelves if they are found offensive? Support your position with convincing arguments from your own experience, observations, and/or reading. +
+'All of us can think of a book that we hope none of our children or any other children have taken off the shelf. But if I have the right to remove that book from the shelf -- that work I abhor -- then you also have exactly the same right and so does everyone else. And then we have no books left on the shelf for any of us.' --Katherine Paterson, Author +
++ Write a persuasive essay to a newspaper reflecting your views on censorship in libraries. Do you believe that certain materials, such as books, music, movies, magazines, etc., should be removed from the shelves if they are found offensive? Support your position with convincing arguments from your own experience, observations, and/or reading. +
+What is edX?
An organization established by MIT and Harvard University that will develop an open-source technology platform to deliver online courses. EdX will support Harvard and MIT faculty in conducting research on teaching and learning on campus through tools that enrich classroom and laboratory experiences. At the same time, edX will also reach learners around the world through online course materials. The edX website will begin by hosting MITx and Harvardx content, with the goal of adding content from other universities interested in joining the platform. edX will also support the Harvard and MIT faculty in conducting research on teaching and learning.