From 94ea35d3beae1beb9b045776f7e23cd2123ae383 Mon Sep 17 00:00:00 2001 From: John Eskew Date: Mon, 8 Dec 2014 12:53:22 -0500 Subject: [PATCH] Export modulestore-stored asset metadata as XML to exported course. Import asset metadata XML into modulestore. Optimize importing many items of asset metadata by avoiding multiple round-trips to MongoDB. --- .../management/commands/import.py | 2 +- .../contentstore/tests/test_import.py | 2 +- .../contentstore/views/tests/test_assets.py | 2 +- .../xmodule/xmodule/assetstore/__init__.py | 44 ++++- .../xmodule/assetstore/tests/assets.xsd | 3 +- .../assetstore/tests/test_asset_xml.py | 5 +- .../xmodule/xmodule/modulestore/__init__.py | 89 ++++++---- .../lib/xmodule/xmodule/modulestore/mixed.py | 41 +++-- .../xmodule/xmodule/modulestore/mongo/base.py | 110 ++++++++---- .../xmodule/modulestore/split_mongo/split.py | 44 ++++- .../modulestore/split_mongo/split_draft.py | 11 ++ .../modulestore/tests/test_assetstore.py | 114 +++++++++++- .../test_cross_modulestore_import_export.py | 13 +- .../tests/test_mixed_modulestore.py | 4 +- .../xmodule/modulestore/xml_exporter.py | 21 ++- .../xmodule/modulestore/xml_importer.py | 70 +++++++- common/lib/xmodule/xmodule/tests/__init__.py | 20 +++ .../manual-testing-complete/assets/assets.xml | 164 ++++++++++++++++++ 18 files changed, 647 insertions(+), 112 deletions(-) create mode 100644 common/test/data/manual-testing-complete/assets/assets.xml diff --git a/cms/djangoapps/contentstore/management/commands/import.py b/cms/djangoapps/contentstore/management/commands/import.py index d742895f0e..08dae9f20d 100644 --- a/cms/djangoapps/contentstore/management/commands/import.py +++ b/cms/djangoapps/contentstore/management/commands/import.py @@ -44,7 +44,7 @@ class Command(BaseCommand): mstore, ModuleStoreEnum.UserID.mgmt_command, data_dir, course_dirs, load_error_modules=False, static_content_store=contentstore(), verbose=True, do_import_static=do_import_static, - create_new_course_if_not_present=True, + create_course_if_not_present=True, ) for course in course_items: diff --git a/cms/djangoapps/contentstore/tests/test_import.py b/cms/djangoapps/contentstore/tests/test_import.py index daf48cea87..4297df6d2c 100644 --- a/cms/djangoapps/contentstore/tests/test_import.py +++ b/cms/djangoapps/contentstore/tests/test_import.py @@ -56,7 +56,7 @@ class ContentStoreImportTest(ModuleStoreTestCase): do_import_static=False, verbose=True, target_course_id=target_course_id, - create_new_course_if_not_present=create_new_course_if_not_present, + create_course_if_not_present=create_new_course_if_not_present, ) course_id = module_store.make_course_key('edX', 'test_import_course', '2012_Fall') course = module_store.get_course(course_id) diff --git a/cms/djangoapps/contentstore/views/tests/test_assets.py b/cms/djangoapps/contentstore/views/tests/test_assets.py index bf891ecf9a..a340a4d24f 100644 --- a/cms/djangoapps/contentstore/views/tests/test_assets.py +++ b/cms/djangoapps/contentstore/views/tests/test_assets.py @@ -204,7 +204,7 @@ class DownloadTestCase(AssetsTestCase): def test_metadata_found_in_modulestore(self): # Insert asset metadata into the modulestore (with no accompanying asset). - asset_key = self.course.id.make_asset_key(AssetMetadata.ASSET_TYPE, 'pic1.jpg') + asset_key = self.course.id.make_asset_key(AssetMetadata.GENERAL_ASSET_TYPE, 'pic1.jpg') asset_md = AssetMetadata(asset_key, { 'internal_name': 'EKMND332DDBK', 'basename': 'pix/archive', diff --git a/common/lib/xmodule/xmodule/assetstore/__init__.py b/common/lib/xmodule/xmodule/assetstore/__init__.py index fcab2d3c2a..be2c9fc723 100644 --- a/common/lib/xmodule/xmodule/assetstore/__init__.py +++ b/common/lib/xmodule/xmodule/assetstore/__init__.py @@ -28,10 +28,25 @@ class AssetMetadata(object): EDIT_INFO_ATTRS = ['curr_version', 'prev_version', 'edited_by', 'edited_by_email', 'edited_on'] CREATE_INFO_ATTRS = ['created_by', 'created_by_email', 'created_on'] ATTRS_ALLOWED_TO_UPDATE = TOP_LEVEL_ATTRS + EDIT_INFO_ATTRS - ALL_ATTRS = ['asset_id'] + ATTRS_ALLOWED_TO_UPDATE + CREATE_INFO_ATTRS + ASSET_TYPE_ATTR = 'type' + ASSET_BASENAME_ATTR = 'filename' + XML_ONLY_ATTRS = [ASSET_TYPE_ATTR, ASSET_BASENAME_ATTR] + XML_ATTRS = XML_ONLY_ATTRS + ATTRS_ALLOWED_TO_UPDATE + CREATE_INFO_ATTRS - # Default type for AssetMetadata objects. A constant for convenience. - ASSET_TYPE = 'asset' + # Type for assets uploaded by a course author in Studio. + GENERAL_ASSET_TYPE = 'asset' + + # Asset section XML tag for asset metadata as XML. + ALL_ASSETS_XML_TAG = 'assets' + + # Individual asset XML tag for asset metadata as XML. + ASSET_XML_TAG = 'asset' + + # Top-level directory name in exported course XML which holds asset metadata. + EXPORTED_ASSET_DIR = 'assets' + + # Filename of all asset metadata exported as XML. + EXPORTED_ASSET_FILENAME = 'assets.xml' @contract(asset_id='AssetKey', pathname='basestring|None', internal_name='basestring|None', @@ -118,6 +133,7 @@ class AssetMetadata(object): """ return { 'filename': self.asset_id.path, + 'asset_type': self.asset_id.asset_type, 'pathname': self.pathname, 'internal_name': self.internal_name, 'locked': self.locked, @@ -169,11 +185,11 @@ class AssetMetadata(object): for child in node: qname = etree.QName(child) tag = qname.localname - if tag in self.ALL_ATTRS: + if tag in self.XML_ATTRS: value = child.text - if tag == 'asset_id': - # Locator. - value = AssetKey.from_string(value) + if tag in self.XML_ONLY_ATTRS: + # An AssetLocator is constructed separately from these parts. + continue elif tag == 'locked': # Boolean. value = True if value == "true" else False @@ -197,13 +213,23 @@ class AssetMetadata(object): Add the asset data as XML to the passed-in node. The node should already be created as a top-level "asset" element. """ - for attr in self.ALL_ATTRS: + for attr in self.XML_ATTRS: child = etree.SubElement(node, attr) - value = getattr(self, attr) + # Get the value. + if attr == self.ASSET_TYPE_ATTR: + value = self.asset_id.asset_type + elif attr == self.ASSET_BASENAME_ATTR: + value = self.asset_id.path + else: + value = getattr(self, attr) + + # Format the value. if isinstance(value, bool): value = "true" if value else "false" elif isinstance(value, datetime): value = value.isoformat() + elif isinstance(value, dict): + value = json.dumps(value) else: value = unicode(value) child.text = value diff --git a/common/lib/xmodule/xmodule/assetstore/tests/assets.xsd b/common/lib/xmodule/xmodule/assetstore/tests/assets.xsd index ab7be765b8..9500a505c9 100644 --- a/common/lib/xmodule/xmodule/assetstore/tests/assets.xsd +++ b/common/lib/xmodule/xmodule/assetstore/tests/assets.xsd @@ -27,7 +27,8 @@ - + + diff --git a/common/lib/xmodule/xmodule/assetstore/tests/test_asset_xml.py b/common/lib/xmodule/xmodule/assetstore/tests/test_asset_xml.py index d01187904f..703fa103c7 100644 --- a/common/lib/xmodule/xmodule/assetstore/tests/test_asset_xml.py +++ b/common/lib/xmodule/xmodule/assetstore/tests/test_asset_xml.py @@ -30,7 +30,6 @@ class TestAssetXml(unittest.TestCase): self.course_assets.append(asset_md) # Read in the XML schema definition and make a validator. - #xsd_path = path(__file__).abspath().dirname() / xsd_filename xsd_path = path(__file__).realpath().parent / xsd_filename with open(xsd_path, 'r') as f: schema_root = etree.XML(f.read()) @@ -51,7 +50,9 @@ class TestAssetXml(unittest.TestCase): new_asset_md = AssetMetadata(new_asset_key) new_asset_md.from_xml(asset) # Compare asset_md to new_asset_md. - for attr in AssetMetadata.ALL_ATTRS: + for attr in AssetMetadata.XML_ATTRS: + if attr in AssetMetadata.XML_ONLY_ATTRS: + continue orig_value = getattr(asset_md, attr) new_value = getattr(new_asset_md, attr) self.assertEqual(orig_value, new_value) diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index e149d47bd3..386015b518 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -36,6 +36,7 @@ log = logging.getLogger('edx.modulestore') new_contract('CourseKey', CourseKey) new_contract('AssetKey', AssetKey) new_contract('AssetMetadata', AssetMetadata) +new_contract('SortedListWithKey', SortedListWithKey) class ModuleStoreEnum(object): @@ -279,14 +280,22 @@ class ModuleStoreAssetInterface(object): """ The methods for accessing assets and their metadata """ - def _find_course_assets(self, course_key): + @contract(asset_list='SortedListWithKey', asset_id='AssetKey') + def _find_asset_in_list(self, asset_list, asset_id): """ - Finds the persisted repr of the asset metadata not converted to AssetMetadata yet. - Returns the container holding a dict indexed by asset block_type whose values are a list - of raw metadata documents + Given a asset list that's a SortedListWithKey, find the index of a particular asset. + Returns: Index of asset, if found. None if not found. """ - log.warning("_find_course_assets request of ModuleStoreAssetInterface - not implemented.") - return None + # See if this asset already exists by checking the external_filename. + # Studio doesn't currently support using multiple course assets with the same filename. + # So use the filename as the unique identifier. + idx = None + idx_left = asset_list.bisect_left({'filename': asset_id.path}) + idx_right = asset_list.bisect_right({'filename': asset_id.path}) + if idx_left != idx_right: + # Asset was found in the list. + idx = idx_left + return idx def _find_course_asset(self, asset_key): """ @@ -297,26 +306,16 @@ class ModuleStoreAssetInterface(object): asset_key (AssetKey): what to look for Returns: - AssetMetadata[] for all assets of the given asset_key's type, & the index of asset in list - (None if asset does not exist) + Tuple of: + - AssetMetadata[] for all assets of the given asset_key's type + - the index of asset in list (None if asset does not exist) """ course_assets = self._find_course_assets(asset_key.course_key) - if course_assets is None: - return None, None - all_assets = SortedListWithKey([], key=itemgetter('filename')) # Assets should be pre-sorted, so add them efficiently without sorting. # extend() will raise a ValueError if the passed-in list is not sorted. all_assets.extend(course_assets.setdefault(asset_key.block_type, [])) - # See if this asset already exists by checking the external_filename. - # Studio doesn't currently support using multiple course assets with the same filename. - # So use the filename as the unique identifier. - idx = None - idx_left = all_assets.bisect_left({'filename': asset_key.block_id}) - idx_right = all_assets.bisect_right({'filename': asset_key.block_id}) - if idx_left != idx_right: - # Asset was found in the list. - idx = idx_left + idx = self._find_asset_in_list(all_assets, asset_key) return course_assets, idx @@ -341,14 +340,17 @@ class ModuleStoreAssetInterface(object): mdata.from_storable(all_assets[asset_idx]) return mdata - @contract(course_key='CourseKey', start='int | None', maxresults='int | None', sort='tuple(str,(int,>=1,<=2))|None',) + @contract( + course_key='CourseKey', asset_type='None | basestring', + start='int | None', maxresults='int | None', sort='tuple(str,(int,>=1,<=2))|None' + ) def get_all_asset_metadata(self, course_key, asset_type, start=0, maxresults=-1, sort=None, **kwargs): """ Returns a list of asset metadata for all assets of the given asset_type in the course. Args: course_key (CourseKey): course identifier - asset_type (str): the block_type of the assets to return + asset_type (str): the block_type of the assets to return. If None, return assets of all types. start (int): optional - start at this asset number. Zero-based! maxresults (int): optional - return at most this many, -1 means no limit sort (array): optional - None means no sort @@ -360,10 +362,6 @@ class ModuleStoreAssetInterface(object): List of AssetMetadata objects. """ course_assets = self._find_course_assets(course_key) - if course_assets is None: - # If no course assets are found, return None instead of empty list - # to distinguish zero assets from "not able to retrieve assets". - return None # Determine the proper sort - with defaults of ('displayname', SortOrder.ascending). key_func = itemgetter('filename') @@ -374,7 +372,17 @@ class ModuleStoreAssetInterface(object): if sort[1] == ModuleStoreEnum.SortOrder.descending: sort_order = ModuleStoreEnum.SortOrder.descending - all_assets = SortedListWithKey(course_assets.get(asset_type, []), key=key_func) + if asset_type is None: + # Add assets of all types to the sorted list. + all_assets = SortedListWithKey([], key=key_func) + for asset_type, val in course_assets.iteritems(): + # '_id' is sometimes added to the course_assets for CRUD purposes + # (depending on the modulestore). If it's present, skip it. + if asset_type != '_id': + all_assets.update(val) + else: + # Add assets of a single type to the sorted list. + all_assets = SortedListWithKey(course_assets.get(asset_type, []), key=key_func) num_assets = len(all_assets) start_idx = start @@ -393,7 +401,8 @@ class ModuleStoreAssetInterface(object): ret_assets = [] for idx in xrange(start_idx, end_idx, step_incr): raw_asset = all_assets[idx] - new_asset = AssetMetadata(course_key.make_asset_key(asset_type, raw_asset['filename'])) + asset_key = course_key.make_asset_key(raw_asset['asset_type'], raw_asset['filename']) + new_asset = AssetMetadata(asset_key) new_asset.from_storable(raw_asset) ret_assets.append(new_asset) return ret_assets @@ -404,13 +413,29 @@ class ModuleStoreAssetWriteInterface(ModuleStoreAssetInterface): The write operations for assets and asset metadata """ @contract(asset_metadata='AssetMetadata') - def save_asset_metadata(self, asset_metadata, user_id): + def save_asset_metadata(self, asset_metadata, user_id, import_only): """ Saves the asset metadata for a particular course's asset. Arguments: - asset_metadata (AssetMetadata): data about the course asset data (must have asset_id - set) + asset_metadata (AssetMetadata): data about the course asset data + user_id (int): user ID saving the asset metadata + import_only (bool): True if importing without editing, False if editing + + Returns: + True if metadata save was successful, else False + """ + raise NotImplementedError() + + @contract(asset_metadata_list='list(AssetMetadata)') + def save_asset_metadata_list(self, asset_metadata_list, user_id, import_only): + """ + Saves a list of asset metadata for a particular course's asset. + + Arguments: + asset_metadata (AssetMetadata): data about the course asset data + user_id (int): user ID saving the asset metadata + import_only (bool): True if importing without editing, False if editing Returns: True if metadata save was successful, else False @@ -438,6 +463,7 @@ class ModuleStoreAssetWriteInterface(ModuleStoreAssetInterface): asset_key (AssetKey): asset identifier attr (str): which attribute to set value: the value to set it to (any type pymongo accepts such as datetime, number, string) + user_id (int): user ID saving the asset metadata Raises: ItemNotFoundError if no such item exists @@ -456,6 +482,7 @@ class ModuleStoreAssetWriteInterface(ModuleStoreAssetInterface): Arguments: source_course_key (CourseKey): identifier of course to copy from dest_course_key (CourseKey): identifier of course to copy to + user_id (int): user ID copying the asset metadata """ pass diff --git a/common/lib/xmodule/xmodule/modulestore/mixed.py b/common/lib/xmodule/xmodule/modulestore/mixed.py index 7d76d31efd..0ce363c483 100644 --- a/common/lib/xmodule/xmodule/modulestore/mixed.py +++ b/common/lib/xmodule/xmodule/modulestore/mixed.py @@ -351,17 +351,40 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase): store = self._get_modulestore_for_courseid(course_key) return store.delete_course(course_key, user_id) - @contract(asset_metadata='AssetMetadata') - def save_asset_metadata(self, asset_metadata, user_id): + @contract(asset_metadata='AssetMetadata', user_id=int, import_only=bool) + def save_asset_metadata(self, asset_metadata, user_id, import_only=False): """ Saves the asset metadata for a particular course's asset. Args: - course_key (CourseKey): course identifier asset_metadata (AssetMetadata): data about the course asset data + user_id (int): user ID saving the asset metadata + import_only (bool): True if importing without editing, False if editing + + Returns: + True if info save was successful, else False """ store = self._get_modulestore_for_courseid(asset_metadata.asset_id.course_key) - return store.save_asset_metadata(asset_metadata, user_id) + return store.save_asset_metadata(asset_metadata, user_id, import_only) + + @contract(asset_metadata_list='list(AssetMetadata)', user_id=int, import_only=bool) + def save_asset_metadata_list(self, asset_metadata_list, user_id, import_only=False): + """ + Saves the asset metadata for each asset in a list of asset metadata. + Optimizes the saving of many assets. + + Args: + asset_metadata_list (list(AssetMetadata)): list of data about several course assets + user_id (int): user ID saving the asset metadata + import_only (bool): True if importing without editing, False if editing + + Returns: + True if info save was successful, else False + """ + if len(asset_metadata_list) == 0: + return True + store = self._get_modulestore_for_courseid(asset_metadata_list[0].asset_id.course_key) + return store.save_asset_metadata_list(asset_metadata_list, user_id, import_only) @strip_key @contract(asset_key='AssetKey') @@ -379,7 +402,7 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase): return store.find_asset_metadata(asset_key, **kwargs) @strip_key - @contract(course_key='CourseKey', start=int, maxresults=int, sort='tuple|None') + @contract(course_key='CourseKey', asset_type='None | basestring', start=int, maxresults=int, sort='tuple|None') def get_all_asset_metadata(self, course_key, asset_type, start=0, maxresults=-1, sort=None, **kwargs): """ Returns a list of static assets for a course. @@ -387,6 +410,7 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase): Args: course_key (CourseKey): course identifier + asset_type (str): type of asset, such as 'asset', 'video', etc. If None, return assets of all types. start (int): optional - start at this asset number maxresults (int): optional - return at most this many, -1 means no limit sort (array): optional - None means no sort @@ -395,12 +419,7 @@ class MixedModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase): sort_order - one of 'ascending' or 'descending' Returns: - List of asset data dictionaries, which have the following keys: - asset_key (AssetKey): asset identifier - displayname: The human-readable name of the asset - uploadDate (datetime.datetime): The date and time that the file was uploaded - contentType: The mimetype string of the asset - md5: An md5 hash of the asset content + List of AssetMetadata objects. """ store = self._get_modulestore_for_courseid(course_key) return store.get_all_asset_metadata(course_key, asset_type, start, maxresults, sort, **kwargs) diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index 1d8dc531d6..2246441635 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -1473,7 +1473,8 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo course_key (CourseKey): course identifier Returns: - Asset info for the course + Dict with (at least) an '_id' key, identifying the relevant Mongo doc. If asset metadata + exists, other keys will be the other asset types with values as lists of asset metadata. """ # Using the course_key, find or insert the course asset metadata document. # A single document exists per course to store the course asset metadata. @@ -1482,11 +1483,15 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo {'course_id': unicode(course_key)}, ) - # Pass back 'assets' dict but add the '_id' key to it for document update purposes. if course_assets is None: - # Not found, so create. - course_assets = {'course_id': unicode(course_key), 'assets': {}} - course_assets['assets']['_id'] = self.asset_collection.insert(course_assets) + # Check to see if the course is created in the course collection. + if self.get_course(course_key) is None: + raise ItemNotFoundError(course_key) + else: + # Course exists, so create matching assets document. + course_assets = {'course_id': unicode(course_key), 'assets': {}} + # Pass back 'assets' dict but add the '_id' key to it for document update purposes. + course_assets['assets']['_id'] = self.asset_collection.insert(course_assets) elif isinstance(course_assets['assets'], list): # This record is in the old course assets format. # Ensure that no data exists before updating the format. @@ -1508,40 +1513,83 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo """ return 'assets.{}'.format(asset_type) - @contract(asset_metadata='AssetMetadata') - def save_asset_metadata(self, asset_metadata, user_id): + @contract(asset_metadata_list='list(AssetMetadata)', user_id=int) + def _save_asset_metadata_list(self, asset_metadata_list, user_id, import_only): """ - Saves the info for a particular course's asset. + Internal; saves the info for a particular course's asset. Arguments: - asset_metadata (AssetMetadata): data about the course asset - - Returns: - True if info save was successful, else False + asset_metadata_list (list(AssetMetadata)): list of data about several course assets + user_id (int): user ID saving the asset metadata + import_only (bool): True if edited_on/by data should remain unchanged. """ - course_assets, asset_idx = self._find_course_asset(asset_metadata.asset_id) - all_assets = SortedListWithKey([], key=itemgetter('filename')) - # Assets should be pre-sorted, so add them efficiently without sorting. - # extend() will raise a ValueError if the passed-in list is not sorted. - all_assets.extend(course_assets[asset_metadata.asset_id.block_type]) - asset_metadata.update({'edited_by': user_id, 'edited_on': datetime.now(UTC)}) + course_assets = self._find_course_assets(asset_metadata_list[0].asset_id.course_key) - # Translate metadata to Mongo format. - metadata_to_insert = asset_metadata.to_storable() - if asset_idx is None: - # Add new metadata sorted into the list. - all_assets.add(metadata_to_insert) - else: - # Replace existing metadata. - all_assets[asset_idx] = metadata_to_insert + changed_asset_types = set() + assets_by_type = {} + for asset_md in asset_metadata_list: + asset_type = asset_md.asset_id.asset_type + changed_asset_types.add(asset_type) + # Lazily create a sorted list if not already created. + if asset_type not in assets_by_type: + assets_by_type[asset_type] = SortedListWithKey(course_assets.get(asset_type, []), key=itemgetter('filename')) + all_assets = assets_by_type[asset_type] + asset_idx = self._find_asset_in_list(assets_by_type[asset_type], asset_md.asset_id) + if not import_only: + asset_md.update({'edited_by': user_id, 'edited_on': datetime.now(UTC)}) + + # Translate metadata to Mongo format. + metadata_to_insert = asset_md.to_storable() + if asset_idx is None: + # Add new metadata sorted into the list. + all_assets.add(metadata_to_insert) + else: + # Replace existing metadata. + all_assets[asset_idx] = metadata_to_insert + + # Build an update set with potentially multiple embedded fields. + updates_by_type = {} + for asset_type in changed_asset_types: + updates_by_type[self._make_mongo_asset_key(asset_type)] = assets_by_type[asset_type].as_list() # Update the document. self.asset_collection.update( {'_id': course_assets['_id']}, - {'$set': {self._make_mongo_asset_key(asset_metadata.asset_id.block_type): all_assets.as_list()}} + {'$set': updates_by_type} ) return True + @contract(asset_metadata='AssetMetadata', user_id=int) + def save_asset_metadata(self, asset_metadata, user_id, import_only=False): + """ + Saves the info for a particular course's asset. + + Arguments: + asset_metadata (AssetMetadata): data about the course asset data + user_id (int): user ID saving the asset metadata + import_only (bool): True if importing without editing, False if editing + + Returns: + True if info save was successful, else False + """ + return self._save_asset_metadata_list([asset_metadata, ], user_id, import_only) + + @contract(asset_metadata_list='list(AssetMetadata)', user_id=int) + def save_asset_metadata_list(self, asset_metadata_list, user_id, import_only=False): + """ + Saves the asset metadata for each asset in a list of asset metadata. + Optimizes the saving of many assets. + + Args: + asset_metadata (AssetMetadata): data about the course asset data + user_id (int): user ID saving the asset metadata + import_only (bool): True if importing without editing, False if editing + + Returns: + True if info save was successful, else False + """ + return self._save_asset_metadata_list(asset_metadata_list, user_id, import_only) + @contract(source_course_key='CourseKey', dest_course_key='CourseKey') def copy_all_asset_metadata(self, source_course_key, dest_course_key, user_id): """ @@ -1630,8 +1678,12 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo """ # Using the course_id, find the course asset metadata document. # A single document exists per course to store the course asset metadata. - course_assets = self._find_course_assets(course_key) - self.asset_collection.remove(course_assets['_id']) + try: + course_assets = self._find_course_assets(course_key) + self.asset_collection.remove(course_assets['_id']) + except ItemNotFoundError: + # When deleting asset metadata, if a course's asset metadata is not present, no big deal. + pass def heartbeat(self): """ diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index d269806e3f..aa0dc8eb74 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -2225,7 +2225,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): # So use the filename as the unique identifier. accessor = asset_key.block_type for idx, asset in enumerate(structure.setdefault(accessor, [])): - if asset['filename'] == asset_key.block_id: + if asset['filename'] == asset_key.path: return idx return None @@ -2256,7 +2256,45 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): # update the index entry if appropriate self._update_head(asset_key.course_key, index_entry, asset_key.branch, new_structure['_id']) - def save_asset_metadata(self, asset_metadata, user_id): + def save_asset_metadata_list(self, asset_metadata_list, user_id, import_only=False): + """ + A wrapper for functions wanting to manipulate assets. Gets and versions the structure, + passes the mutable array for all asset types as well as the idx to the function for it to + update, then persists the changed data back into the course. + + The update function can raise an exception if it doesn't want to actually do the commit. The + surrounding method probably should catch that exception. + """ + asset_key = asset_metadata_list[0].asset_id + course_key = asset_key.course_key + + with self.bulk_operations(course_key): + original_structure = self._lookup_course(course_key).structure + index_entry = self._get_index_if_valid(course_key) + new_structure = self.version_structure(course_key, original_structure, user_id) + + # Add all asset metadata to the structure at once. + for asset_metadata in asset_metadata_list: + metadata_to_insert = asset_metadata.to_storable() + asset_md_key = asset_metadata.asset_id + + asset_idx = self._lookup_course_asset(new_structure.setdefault('assets', {}), asset_md_key) + + all_assets = new_structure['assets'][asset_md_key.asset_type] + if asset_idx is None: + all_assets.append(metadata_to_insert) + else: + all_assets[asset_idx] = metadata_to_insert + new_structure['assets'][asset_md_key.asset_type] = all_assets + + # update index if appropriate and structures + self.update_structure(course_key, new_structure) + + if index_entry is not None: + # update the index entry if appropriate + self._update_head(course_key, index_entry, asset_key.branch, new_structure['_id']) + + def save_asset_metadata(self, asset_metadata, user_id, import_only=False): """ The guts of saving a new or updated asset """ @@ -2347,7 +2385,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): index_entry = self._get_index_if_valid(dest_course_key) new_structure = self.version_structure(dest_course_key, original_structure, user_id) - new_structure['assets'] = source_structure.get('assets', []) + new_structure['assets'] = source_structure.get('assets', {}) new_structure['thumbnails'] = source_structure.get('thumbnails', []) # update index if appropriate and structures diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py index d44d2c0baf..bb59da2a7d 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py @@ -477,6 +477,17 @@ class DraftVersioningModuleStore(SplitMongoModuleStore, ModuleStoreDraftAndPubli update_function ) + def save_asset_metadata_list(self, asset_metadata_list, user_id, import_only=False): + """ + Updates both the published and draft branches + """ + asset_key = asset_metadata_list[0].asset_id + asset_metadata_list[0].asset_id = self._map_revision_to_branch(asset_key, ModuleStoreEnum.RevisionOption.published_only) + # if one call gets an exception, don't do the other call but pass on the exception + super(DraftVersioningModuleStore, self).save_asset_metadata_list(asset_metadata_list, user_id, import_only) + asset_metadata_list[0].asset_id = self._map_revision_to_branch(asset_key, ModuleStoreEnum.RevisionOption.draft_only) + super(DraftVersioningModuleStore, self).save_asset_metadata_list(asset_metadata_list, user_id, import_only) + def _find_course_asset(self, asset_key): return super(DraftVersioningModuleStore, self)._find_course_asset( self._map_revision_to_branch(asset_key) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_assetstore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_assetstore.py index 2a8e6fd064..7e9bfc747f 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_assetstore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_assetstore.py @@ -8,8 +8,10 @@ from nose.plugins.attrib import attr import pytz import unittest +from opaque_keys.edx.keys import CourseKey from xmodule.assetstore import AssetMetadata from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.tests.test_cross_modulestore_import_export import ( MIXED_MODULESTORE_BOTH_SETUP, MODULESTORE_SETUPS, MongoContentstoreBuilder, @@ -26,7 +28,7 @@ class AssetStoreTestData(object): user_email = "me@example.com" asset_fields = ( - 'filename', 'internal_name', 'pathname', 'locked', + AssetMetadata.ASSET_BASENAME_ATTR, 'internal_name', 'pathname', 'locked', 'edited_by', 'edited_by_email', 'edited_on', 'created_by', 'created_by_email', 'created_on', 'curr_version', 'prev_version' ) @@ -117,11 +119,8 @@ class TestMongoAssetMetadataStorage(unittest.TestCase): with MongoContentstoreBuilder().build() as contentstore: with storebuilder.build(contentstore) as store: course = CourseFactory.create(modulestore=store) - asset_filename = 'burnside.jpg' new_asset_loc = course.id.make_asset_key('asset', asset_filename) - # Confirm that the asset's metadata is not present. - self.assertIsNone(store.find_asset_metadata(new_asset_loc)) # Save the asset's metadata. new_asset_md = self._make_asset_metadata(new_asset_loc) store.save_asset_metadata(new_asset_md, ModuleStoreEnum.UserID.test) @@ -134,7 +133,7 @@ class TestMongoAssetMetadataStorage(unittest.TestCase): @ddt.data(*MODULESTORE_SETUPS) def test_delete(self, storebuilder): """ - Delete non_existent and existent metadata + Delete non-existent and existent metadata """ with MongoContentstoreBuilder().build() as contentstore: with storebuilder.build(contentstore) as store: @@ -152,7 +151,7 @@ class TestMongoAssetMetadataStorage(unittest.TestCase): @ddt.data(*MODULESTORE_SETUPS) def test_find_non_existing_assets(self, storebuilder): """ - Save multiple metadata in each store and retrieve it singularly, as all assets, and after deleting all. + Find a non-existent asset in an existing course. """ with MongoContentstoreBuilder().build() as contentstore: with storebuilder.build(contentstore) as store: @@ -162,10 +161,40 @@ class TestMongoAssetMetadataStorage(unittest.TestCase): asset_md = store.find_asset_metadata(new_asset_loc) self.assertIsNone(asset_md) + @ddt.data(*MODULESTORE_SETUPS) + def test_get_all_non_existing_assets(self, storebuilder): + """ + Get all assets in an existing course when no assets exist. + """ + with MongoContentstoreBuilder().build() as contentstore: + with storebuilder.build(contentstore) as store: + course = CourseFactory.create(modulestore=store) + # Find existing asset metadata. + asset_md = store.get_all_asset_metadata(course.id, 'asset') + self.assertEquals(asset_md, []) + + @ddt.data(*MODULESTORE_SETUPS) + def test_find_assets_in_non_existent_course(self, storebuilder): + """ + Find asset metadata from a non-existent course. + """ + with MongoContentstoreBuilder().build() as contentstore: + with storebuilder.build(contentstore) as store: + course = CourseFactory.create(modulestore=store) + fake_course_id = CourseKey.from_string("{}nothere/{}nothere/{}nothere".format( + course.id.org, course.id.course, course.id.run + )) + new_asset_loc = fake_course_id.make_asset_key('asset', 'burnside.jpg') + # Find asset metadata from non-existent course. + with self.assertRaises(ItemNotFoundError): + store.find_asset_metadata(new_asset_loc) + with self.assertRaises(ItemNotFoundError): + store.get_all_asset_metadata(fake_course_id, 'asset') + @ddt.data(*MODULESTORE_SETUPS) def test_add_same_asset_twice(self, storebuilder): """ - Save multiple metadata in each store and retrieve it singularly, as all assets, and after deleting all. + Add an asset's metadata, then add it again. """ with MongoContentstoreBuilder().build() as contentstore: with storebuilder.build(contentstore) as store: @@ -359,6 +388,58 @@ class TestMongoAssetMetadataStorage(unittest.TestCase): unknown_asset_key = course.id.make_asset_key('different', 'nosuchfile.jpg') self.assertIsNone(store.find_asset_metadata(unknown_asset_key)) + @ddt.data(*MODULESTORE_SETUPS) + def test_get_multiple_types(self, storebuilder): + """ + getting all things which are of type other than 'asset' + """ + def check_asset_values(assets, orig): + """ + Check asset values. + """ + for idx, asset in enumerate(orig): + self.assertEquals(assets[idx].asset_id.asset_type, asset[0]) + self.assertEquals(assets[idx].asset_id.path, asset[1]) + + with MongoContentstoreBuilder().build() as contentstore: + with storebuilder.build(contentstore) as store: + course = CourseFactory.create(modulestore=store) + differents = (('different', 'burn.jpg'),) + vrmls = ( + ('vrml', 'olympus_mons.vrml'), + ('vrml', 'ponte_vecchio.vrml'), + ) + regular_assets = (('asset', 'zippy.png'),) + alls = differents + vrmls + regular_assets + + # Save 'em. + for asset_type, filename in alls: + asset_key = course.id.make_asset_key(asset_type, filename) + new_asset = self._make_asset_thumbnail_metadata( + self._make_asset_metadata(asset_key) + ) + store.save_asset_metadata(new_asset, ModuleStoreEnum.UserID.test) + + # Check 'em. + for asset_type, asset_list in ( + ('different', differents), + ('vrml', vrmls), + ('asset', regular_assets), + ): + assets = store.get_all_asset_metadata(course.id, asset_type) + self.assertEquals(len(assets), len(asset_list)) + check_asset_values(assets, asset_list) + + self.assertEquals(len(store.get_all_asset_metadata(course.id, 'not_here')), 0) + self.assertEquals(len(store.get_all_asset_metadata(course.id, None)), 4) + + assets = store.get_all_asset_metadata( + course.id, None, start=0, maxresults=-1, + sort=('displayname', ModuleStoreEnum.SortOrder.ascending) + ) + self.assertEquals(len(assets), len(alls)) + check_asset_values(assets, alls) + @ddt.data(*MODULESTORE_SETUPS) def test_delete_all_different_type(self, storebuilder): """ @@ -456,8 +537,6 @@ class TestMongoAssetMetadataStorage(unittest.TestCase): course_key = store.make_course_key("org", "course", "run") asset_key = course_key.make_asset_key('asset', 'foo.jpg') self.assertEquals(store.find_asset_metadata(asset_key), None) - # pylint: disable=protected-access - self.assertEquals(store._find_course_asset(asset_key), (None, None)) self.assertEquals(store.get_all_asset_metadata(course_key, 'asset'), []) @ddt.data(*MODULESTORE_SETUPS) @@ -481,6 +560,23 @@ class TestMongoAssetMetadataStorage(unittest.TestCase): self.assertEquals(all_assets[0].asset_id.path, 'pic1.jpg') self.assertEquals(all_assets[1].asset_id.path, 'shout.ogg') + @ddt.data(*MODULESTORE_SETUPS) + def test_copy_all_assets_from_course_with_no_assets(self, storebuilder): + """ + Create a course with *no* assets, and try copy them all to another course in the same modulestore. + """ + with MongoContentstoreBuilder().build() as contentstore: + with storebuilder.build(contentstore) as store: + course1 = CourseFactory.create(modulestore=store) + course2 = CourseFactory.create(modulestore=store) + store.copy_all_asset_metadata(course1.id, course2.id, ModuleStoreEnum.UserID.test * 101) + self.assertEquals(len(store.get_all_asset_metadata(course1.id, 'asset')), 0) + self.assertEquals(len(store.get_all_asset_metadata(course2.id, 'asset')), 0) + all_assets = store.get_all_asset_metadata( + course2.id, 'asset', sort=('displayname', ModuleStoreEnum.SortOrder.ascending) + ) + self.assertEquals(len(all_assets), 0) + @ddt.data( ('mongo', 'split'), ('split', 'mongo'), 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 index 82d469612d..dd684c6826 100644 --- 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 @@ -331,7 +331,8 @@ class CrossStoreXMLRoundtrip(CourseComparisonTest, PartitionTestCase): course_dirs=[course_data_name], static_content_store=source_content, target_course_id=source_course_key, - create_new_course_if_not_present=True, + create_course_if_not_present=True, + raise_on_failure=True, ) export_to_xml( @@ -349,7 +350,8 @@ class CrossStoreXMLRoundtrip(CourseComparisonTest, PartitionTestCase): course_dirs=['exported_source_course'], static_content_store=dest_content, target_course_id=dest_course_key, - create_new_course_if_not_present=True, + create_course_if_not_present=True, + raise_on_failure=True, ) # NOT CURRENTLY USED @@ -382,3 +384,10 @@ class CrossStoreXMLRoundtrip(CourseComparisonTest, PartitionTestCase): dest_content, dest_course_key, ) + + self.assertAssetsMetadataEqual( + source_store, + source_course_key, + dest_store, + 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 959c47b737..0bdef107b5 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -1932,7 +1932,7 @@ class TestMixedModuleStore(CourseComparisonTest): self.store, self.user_id, DATA_DIR, ['toy'], load_error_modules=False, static_content_store=contentstore, target_course_id=dest_course_key, - create_new_course_if_not_present=True, + create_course_if_not_present=True, ) course_id = courses[0].id # no need to verify course content here as test_cross_modulestore_import_export does that @@ -1980,7 +1980,7 @@ class TestMixedModuleStore(CourseComparisonTest): self.store, self.user_id, DATA_DIR, ['toy'], load_error_modules=False, static_content_store=contentstore, target_course_id=dest_course_key, - create_new_course_if_not_present=True, + create_course_if_not_present=True, ) course_id = courses[0].id # no need to verify course content here as test_cross_modulestore_import_export does that diff --git a/common/lib/xmodule/xmodule/modulestore/xml_exporter.py b/common/lib/xmodule/xmodule/modulestore/xml_exporter.py index 3cc691153e..a0a97a47ca 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_exporter.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_exporter.py @@ -7,6 +7,7 @@ import lxml.etree from xblock.fields import Scope, Reference, ReferenceList, ReferenceValueDict from xmodule.contentstore.content import StaticContent from xmodule.exceptions import NotFoundError +from xmodule.assetstore import AssetMetadata from xmodule.modulestore import EdxJSONEncoder, ModuleStoreEnum from xmodule.modulestore.inheritance import own_metadata from xmodule.modulestore.store_utilities import draft_node_constructor, get_draft_subtree_roots @@ -43,6 +44,7 @@ def export_to_xml(modulestore, contentstore, course_key, root_dir, course_dir): course = modulestore.get_course(course_key, depth=None) # None means infinite fsm = OSFS(root_dir) export_fs = course.runtime.export_fs = fsm.makeopendir(course_dir) + root_course_dir = root_dir + '/' + course_dir root = lxml.etree.Element('unknown') @@ -57,13 +59,26 @@ def export_to_xml(modulestore, contentstore, course_key, root_dir, course_dir): with export_fs.open('course.xml', 'w') as course_xml: lxml.etree.ElementTree(root).write(course_xml) + # Export the modulestore's asset metadata. + asset_dir = root_course_dir + '/' + AssetMetadata.EXPORTED_ASSET_DIR + '/' + if not os.path.isdir(asset_dir): + os.makedirs(asset_dir) + asset_root = lxml.etree.Element(AssetMetadata.ALL_ASSETS_XML_TAG) + course_assets = modulestore.get_all_asset_metadata(course_key, None) + for asset_md in course_assets: + # All asset types are exported using the "asset" tag - but their asset type is specified in each asset key. + asset = lxml.etree.SubElement(asset_root, AssetMetadata.ASSET_XML_TAG) + asset_md.to_xml(asset) + with OSFS(asset_dir).open(AssetMetadata.EXPORTED_ASSET_FILENAME, 'w') as asset_xml_file: + lxml.etree.ElementTree(asset_root).write(asset_xml_file) + # export the static assets policies_dir = export_fs.makeopendir('policies') if contentstore: contentstore.export_all_for_course( course_key, - root_dir + '/' + course_dir + '/static/', - root_dir + '/' + course_dir + '/policies/assets.json', + root_course_dir + '/static/', + root_course_dir + '/policies/assets.json', ) # If we are using the default course image, export it to the @@ -79,7 +94,7 @@ def export_to_xml(modulestore, contentstore, course_key, root_dir, course_dir): except NotFoundError: pass else: - output_dir = root_dir + '/' + course_dir + '/static/images/' + output_dir = root_course_dir + '/static/images/' if not os.path.isdir(output_dir): os.makedirs(output_dir) with OSFS(output_dir).open('course_image.jpg', 'wb') as course_image_file: diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index 4f89249415..39a61bd342 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -26,6 +26,7 @@ import mimetypes from path import path import json import re +from lxml import etree from .xml import XMLModuleStore, ImportSystem, ParentTracker from xblock.runtime import KvsFieldData, DictKeyValueStore @@ -38,6 +39,7 @@ from xmodule.errortracker import make_error_tracker from .store_utilities import rewrite_nonportable_content_links import xblock from xmodule.tabs import CourseTabList +from xmodule.assetstore import AssetMetadata from xmodule.modulestore.django import ASSET_IGNORE_REGEX from xmodule.modulestore.exceptions import DuplicateCourseError from xmodule.modulestore.mongo.base import MongoRevisionKey @@ -139,7 +141,8 @@ def import_from_xml( default_class='xmodule.raw_module.RawDescriptor', load_error_modules=True, static_content_store=None, target_course_id=None, verbose=False, - do_import_static=True, create_new_course_if_not_present=False): + do_import_static=True, create_course_if_not_present=False, + raise_on_failure=False): """ Import xml-based courses from data_dir into modulestore. @@ -167,7 +170,7 @@ def import_from_xml( time the course is loaded. Static content for some courses may also be served directly by nginx, instead of going through django. - create_new_course_if_not_present: If True, then a new course is created if it doesn't already exist. + create_course_if_not_present: If True, then a new course is created if it doesn't already exist. Otherwise, it throws an InvalidLocationError if the course does not exist. default_class, load_error_modules: are arguments for constructing the XMLModuleStore (see its doc) @@ -196,7 +199,7 @@ def import_from_xml( runtime = None # Creates a new course if it doesn't already exist - if create_new_course_if_not_present and not store.has_course(dest_course_id, ignore_case=True): + if create_course_if_not_present and not store.has_course(dest_course_id, ignore_case=True): try: new_course = store.create_course(dest_course_id.org, dest_course_id.course, dest_course_id.run, user_id) runtime = new_course.runtime @@ -223,6 +226,9 @@ def import_from_xml( static_content_store, do_import_static, course_data_path, dest_course_id, verbose ) + # Import asset metadata stored in XML. + _import_course_asset_metadata(store, course_data_path, dest_course_id, raise_on_failure) + # STEP 3: import PUBLISHED items # now loop through all the modules depth first and then orphans with store.branch_setting(ModuleStoreEnum.Branch.published_only, dest_course_id): @@ -285,10 +291,60 @@ def import_from_xml( return new_courses +def _import_course_asset_metadata(store, data_dir, course_id, raise_on_failure): + """ + Read in assets XML file, parse it, and add all asset metadata to the modulestore. + """ + asset_dir = path(data_dir) / AssetMetadata.EXPORTED_ASSET_DIR + assets_filename = AssetMetadata.EXPORTED_ASSET_FILENAME + asset_xml_file = asset_dir / assets_filename + + def make_asset_id(course_id, asset_xml): + """ + Construct an asset ID out of a complete asset XML section. + """ + asset_type = None + asset_name = None + for child in asset_xml.iterchildren(): + if child.tag == AssetMetadata.ASSET_TYPE_ATTR: + asset_type = child.text + elif child.tag == AssetMetadata.ASSET_BASENAME_ATTR: + asset_name = child.text + return course_id.make_asset_key(asset_type, asset_name) + + all_assets = [] + try: + xml_data = etree.parse(asset_xml_file).getroot() + assert(xml_data.tag == AssetMetadata.ALL_ASSETS_XML_TAG) + for asset in xml_data.iterchildren(): + if asset.tag == AssetMetadata.ASSET_XML_TAG: + # Construct the asset key. + asset_key = make_asset_id(course_id, asset) + asset_md = AssetMetadata(asset_key) + asset_md.from_xml(asset) + all_assets.append(asset_md) + except IOError: + logging.info('No {} file is present with asset metadata.'.format(assets_filename)) + return + except Exception: # pylint: disable=W0703 + logging.exception('Error while parsing asset xml.') + if raise_on_failure: + raise + else: + return + + # Now add all asset metadata to the modulestore. + if len(all_assets) > 0: + store.save_asset_metadata_list(all_assets, all_assets[0].edited_by, import_only=True) + + def _import_course_module( store, runtime, user_id, data_dir, course_key, dest_course_id, source_course, do_import_static, verbose, ): + """ + Import a course module. + """ if verbose: log.debug("Scanning {0} for course module...".format(course_key)) @@ -534,11 +590,11 @@ def _import_course_draft( for child in module.get_children(): _import_module(child) - # now walk the /vertical directory where each file in there - # will be a draft copy of the Vertical + # Now walk the /vertical directory. + # Each file in the directory will be a draft copy of the vertical. - # First it is necessary to order the draft items by their desired index in the child list - # (order os.walk returns them in is not guaranteed). + # First it is necessary to order the draft items by their desired index in the child list, + # since the order in which os.walk() returns the files is not guaranteed. drafts = [] for dirname, _dirnames, filenames in os.walk(draft_dir): for filename in filenames: diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index eac771de45..3fba95b2ab 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -26,6 +26,7 @@ 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 +from xmodule.assetstore import AssetMetadata from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.mongo.draft import DraftModuleStore from xmodule.modulestore.xml import CourseLocationManager @@ -498,3 +499,22 @@ class CourseComparisonTest(BulkAssertionTest): 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) + + def assertAssetsMetadataEqual(self, expected_modulestore, expected_course_key, actual_modulestore, actual_course_key): + """ + Assert that the modulestore asset metdata for the ``expected_course_key`` and the ``actual_course_key`` + are equivalent. + """ + expected_course_assets = expected_modulestore.get_all_asset_metadata( + expected_course_key, None, sort=('displayname', ModuleStoreEnum.SortOrder.descending) + ) + actual_course_assets = actual_modulestore.get_all_asset_metadata( + actual_course_key, None, sort=('displayname', ModuleStoreEnum.SortOrder.descending) + ) + self.assertEquals(len(expected_course_assets), len(actual_course_assets)) + for idx, __ in enumerate(expected_course_assets): + for attr in AssetMetadata.ATTRS_ALLOWED_TO_UPDATE: + if attr in ('edited_on',): + # edited_on is updated upon import. + continue + self.assertEquals(getattr(expected_course_assets[idx], attr), getattr(actual_course_assets[idx], attr)) diff --git a/common/test/data/manual-testing-complete/assets/assets.xml b/common/test/data/manual-testing-complete/assets/assets.xml new file mode 100644 index 0000000000..4472a7c837 --- /dev/null +++ b/common/test/data/manual-testing-complete/assets/assets.xml @@ -0,0 +1,164 @@ + + + asset + pic1.jpg + pix/archive + EKMND332DDBK + false + None + None + {"copyrighted": true} + 14 + 13 + 144 + me@example.com + 2014-12-02T23:05:05.196505+00:00 + 144 + me@example.com + 2014-12-02T23:05:05.196505+00:00 + + + asset + shout.ogg + sounds + KFMDONSKF39K + true + None + None + {} + 1 + None + 144 + me@example.com + 2014-12-02T23:05:05.196505+00:00 + 144 + me@example.com + 2014-12-02T23:05:05.196505+00:00 + + + asset + code.tgz + exercises/14 + ZZB2333YBDMW + false + None + None + {"filesize": 123456} + AB + AA + 288 + me@example.com + 2014-12-02T23:05:05.196505+00:00 + 288 + me@example.com + 2014-12-02T23:05:05.196505+00:00 + + + asset + dog.png + pictures/animals + PUPY4242X + true + None + None + {} + 5 + 4 + 432 + me@example.com + 2014-12-02T23:05:05.196505+00:00 + 432 + me@example.com + 2014-12-02T23:05:05.196505+00:00 + + + asset + not_here.txt + /dev/null + JJJCCC747 + false + None + None + {} + 50 + 49 + 576 + me@example.com + 2014-12-02T23:05:05.196505+00:00 + 576 + me@example.com + 2014-12-02T23:05:05.196505+00:00 + + + asset + asset.txt + /dev/null + JJJCCC747858 + false + None + None + {} + 50 + 49 + 576 + me@example.com + 2014-12-02T23:05:05.196505+00:00 + 576 + me@example.com + 2014-12-02T23:05:05.196505+00:00 + + + asset + roman_history.pdf + texts/italy + JASDUNSADK + true + None + None + {"complicated": true, "thing_list": [14, true, "blue", {"nest": "but no eggs"}]} + 1.1 + 1.01 + 1008 + me@example.com + 2014-12-02T23:05:05.196505+00:00 + 1008 + me@example.com + 2014-12-02T23:05:05.196505+00:00 + + + asset + weather_patterns.bmp + science + 928SJXX2EB + false + None + None + {"forecast": "horrible - wear many layers"} + 52 + 51 + 1152 + me@example.com + 2014-12-02T23:05:05.196505+00:00 + 1152 + me@example.com + 2014-12-02T23:05:05.196505+00:00 + + + video + demo.swf + demos/easy + DFDFGGGG14 + false + None + None + {} + 5 + 4 + 1296 + me@example.com + 2014-12-02T23:05:05.196505+00:00 + 1296 + me@example.com + 2014-12-02T23:05:05.196505+00:00 + +