diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index 8191989c74..8d92a3579f 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -9,6 +9,7 @@ import json import datetime from uuid import uuid4 +from pytz import UTC from collections import namedtuple, defaultdict import collections from contextlib import contextmanager @@ -36,7 +37,6 @@ log = logging.getLogger('edx.modulestore') new_contract('CourseKey', CourseKey) new_contract('AssetKey', AssetKey) new_contract('AssetMetadata', AssetMetadata) -new_contract('SortedListWithKey', SortedListWithKey) class ModuleStoreEnum(object): @@ -276,27 +276,65 @@ class BulkOperationsMixin(object): return self._get_bulk_ops_record(course_key, ignore_case).active -class ModuleStoreAssetInterface(object): +class IncorrectlySortedList(Exception): """ - The methods for accessing assets and their metadata + Thrown when calling find() on a SortedAssetList not sorted by filename. """ - @contract(asset_list='SortedListWithKey', asset_id='AssetKey') - def _find_asset_in_list(self, asset_list, asset_id): + pass + + +class SortedAssetList(SortedListWithKey): + """ + List of assets that is sorted based on an asset attribute. + """ + def __init__(self, **kwargs): + self.filename_sort = False + key_func = kwargs.get('key', None) + if key_func is None: + kwargs['key'] = itemgetter('filename') + self.filename_sort = True + super(SortedAssetList, self).__init__(**kwargs) + + @contract(asset_id=AssetKey) + def find(self, asset_id): """ - Given a asset list that's a SortedListWithKey, find the index of a particular asset. + Find the index of a particular asset in the list. This method is only functional for lists + sorted by filename. If the list is sorted on any other key, find() raises a Returns: Index of asset, if found. None if not found. """ + # Don't attempt to find an asset by filename in a list that's not sorted by filename. + if not self.filename_sort: + raise IncorrectlySortedList() # 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}) + idx_left = self.bisect_left({'filename': asset_id.path}) + idx_right = self.bisect_right({'filename': asset_id.path}) if idx_left != idx_right: # Asset was found in the list. idx = idx_left return idx + @contract(asset_md=AssetMetadata) + def insert_or_update(self, asset_md): + """ + Insert asset metadata if asset is not present. Update asset metadata if asset is already present. + """ + metadata_to_insert = asset_md.to_storable() + asset_idx = self.find(asset_md.asset_id) + if asset_idx is None: + # Add new metadata sorted into the list. + self.add(metadata_to_insert) + else: + # Replace existing metadata. + self[asset_idx] = metadata_to_insert + + +class ModuleStoreAssetBase(object): + """ + The methods for accessing assets and their metadata + """ def _find_course_asset(self, asset_key): """ Returns same as _find_course_assets plus the index to the given asset or None. Does not convert @@ -311,11 +349,11 @@ class ModuleStoreAssetInterface(object): - the index of asset in list (None if asset does not exist) """ course_assets = self._find_course_assets(asset_key.course_key) - all_assets = SortedListWithKey([], key=itemgetter('filename')) + all_assets = SortedAssetList(iterable=[]) # 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, [])) - idx = self._find_asset_in_list(all_assets, asset_key) + idx = all_assets.find(asset_key) return course_assets, idx @@ -334,9 +372,8 @@ class ModuleStoreAssetInterface(object): if asset_idx is None: return None - info = asset_key.block_type mdata = AssetMetadata(asset_key, asset_key.path, **kwargs) - all_assets = course_assets[info] + all_assets = course_assets[asset_key.asset_type] mdata.from_storable(all_assets[asset_idx]) return mdata @@ -364,7 +401,7 @@ class ModuleStoreAssetInterface(object): course_assets = self._find_course_assets(course_key) # Determine the proper sort - with defaults of ('displayname', SortOrder.ascending). - key_func = itemgetter('filename') + key_func = None sort_order = ModuleStoreEnum.SortOrder.ascending if sort: if sort[0] == 'uploadDate': @@ -374,12 +411,12 @@ class ModuleStoreAssetInterface(object): if asset_type is None: # Add assets of all types to the sorted list. - all_assets = SortedListWithKey([], key=key_func) + all_assets = SortedAssetList(iterable=[], key=key_func) for asset_type, val in course_assets.iteritems(): 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) + all_assets = SortedAssetList(iterable=course_assets.get(asset_type, []), key=key_func) num_assets = len(all_assets) start_idx = start @@ -405,10 +442,32 @@ class ModuleStoreAssetInterface(object): return ret_assets -class ModuleStoreAssetWriteInterface(ModuleStoreAssetInterface): +class ModuleStoreAssetWriteInterface(ModuleStoreAssetBase): """ The write operations for assets and asset metadata """ + def _save_assets_by_type(self, course_key, asset_metadata_list, course_assets, user_id, import_only): + """ + Common private method that saves/updates asset metadata items in the internal modulestore + structure used to store asset metadata items. + """ + # Lazily create a sorted list if not already created. + assets_by_type = defaultdict(lambda: SortedAssetList(iterable=course_assets.get(asset_type, []))) + + for asset_md in asset_metadata_list: + if asset_md.asset_id.course_key != course_key: + # pylint: disable=logging-format-interpolation + log.warning("Asset's course {} does not match other assets for course {} - not saved.".format( + asset_md.asset_id.course_key, course_key + )) + continue + if not import_only: + asset_md.update({'edited_by': user_id, 'edited_on': datetime.datetime.now(UTC)}) + asset_type = asset_md.asset_id.asset_type + all_assets = assets_by_type[asset_type] + all_assets.insert_or_update(asset_md) + return assets_by_type + @contract(asset_metadata='AssetMetadata') def save_asset_metadata(self, asset_metadata, user_id, import_only): """ @@ -485,7 +544,7 @@ class ModuleStoreAssetWriteInterface(ModuleStoreAssetInterface): # pylint: disable=abstract-method -class ModuleStoreRead(ModuleStoreAssetInterface): +class ModuleStoreRead(ModuleStoreAssetBase): """ An abstract interface for a database backend that stores XModuleDescriptor instances and extends read-only functionality diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index 4a1c107c82..51227e4b2d 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -26,8 +26,6 @@ from mongodb_proxy import MongoProxy, autoretry_read from path import path from pytz import UTC from contracts import contract, new_contract -from operator import itemgetter -from sortedcontainers import SortedListWithKey from importlib import import_module from opaque_keys.edx.keys import UsageKey, CourseKey, AssetKey @@ -1535,34 +1533,14 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo user_id (int|long): user ID saving the asset metadata import_only (bool): True if edited_on/by data should remain unchanged. """ - course_assets = self._find_course_assets(asset_metadata_list[0].asset_id.course_key) - - 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 + course_key = asset_metadata_list[0].asset_id.course_key + course_assets = self._find_course_assets(course_key) + assets_by_type = self._save_assets_by_type(course_key, asset_metadata_list, course_assets, user_id, import_only) # 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() + for asset_type, assets in assets_by_type.iteritems(): + updates_by_type[self._make_mongo_asset_key(asset_type)] = assets.as_list() # Update the document. self.asset_collection.update( diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index 4fd11dfe34..9e9b09ebd2 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -73,7 +73,7 @@ from opaque_keys.edx.locator import ( from xmodule.modulestore.exceptions import InsufficientSpecificationError, VersionConflictError, DuplicateItemError, \ DuplicateCourseError from xmodule.modulestore import ( - inheritance, ModuleStoreWriteBase, ModuleStoreEnum, BulkOpsRecord, BulkOperationsMixin + inheritance, ModuleStoreWriteBase, ModuleStoreEnum, BulkOpsRecord, BulkOperationsMixin, SortedAssetList ) from ..exceptions import ItemNotFoundError @@ -2416,27 +2416,6 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): """ return self._lookup_course(course_key).structure.get('assets', {}) - def _find_course_asset(self, asset_key): - """ - Return the raw dict of assets type as well as the index to the one being sought from w/in - it's subvalue (or None) - """ - assets = self._lookup_course(asset_key.course_key).structure.get('assets', {}) - return assets, self._lookup_course_asset(assets, asset_key) - - def _lookup_course_asset(self, structure, asset_key): - """ - Find the course asset in the structure or return None if it does not exist - """ - # 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. - accessor = asset_key.block_type - for idx, asset in enumerate(structure.setdefault(accessor, [])): - if asset['filename'] == asset_key.path: - return idx - return None - def _update_course_assets(self, user_id, asset_key, update_function): """ A wrapper for functions wanting to manipulate assets. Gets and versions the structure, @@ -2450,12 +2429,17 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): original_structure = self._lookup_course(asset_key.course_key).structure index_entry = self._get_index_if_valid(asset_key.course_key) new_structure = self.version_structure(asset_key.course_key, original_structure, user_id) + course_assets = new_structure.setdefault('assets', {}) - asset_idx = self._lookup_course_asset(new_structure.setdefault('assets', {}), asset_key) + asset_type = asset_key.asset_type + all_assets = SortedAssetList(iterable=[]) + # 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_type, [])) + asset_idx = all_assets.find(asset_key) - new_structure['assets'][asset_key.block_type] = update_function( - new_structure['assets'][asset_key.block_type], asset_idx - ) + all_assets_updated = update_function(all_assets, asset_idx) + new_structure['assets'][asset_type] = all_assets_updated.as_list() # update index if appropriate and structures self.update_structure(asset_key.course_key, new_structure) @@ -2466,13 +2450,12 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): 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. + Saves a list of AssetMetadata to the modulestore. The list can be composed of multiple + asset types. This method is optimized for multiple inserts at once - it only re-saves the structure + at the end of all saves/updates. """ + # Determine course key to use in bulk operation. Use the first asset assuming that + # all assets will be for the same course. asset_key = asset_metadata_list[0].asset_id course_key = asset_key.course_key @@ -2480,20 +2463,14 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): 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) + course_assets = new_structure.setdefault('assets', {}) - # 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 + assets_by_type = self._save_assets_by_type( + course_key, asset_metadata_list, course_assets, user_id, import_only + ) - 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 + for asset_type, assets in assets_by_type.iteritems(): + new_structure['assets'][asset_type] = assets.as_list() # update index if appropriate and structures self.update_structure(course_key, new_structure) @@ -2504,21 +2481,9 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): def save_asset_metadata(self, asset_metadata, user_id, import_only=False): """ - The guts of saving a new or updated asset + Saves or updates a single asset. Simply makes it a list and calls the list save above. """ - metadata_to_insert = asset_metadata.to_storable() - - def _internal_method(all_assets, asset_idx): - """ - Either replace the existing entry or add a new one - """ - if asset_idx is None: - all_assets.append(metadata_to_insert) - else: - all_assets[asset_idx] = metadata_to_insert - return all_assets - - return self._update_course_assets(user_id, asset_metadata.asset_id, _internal_method) + return self.save_asset_metadata_list([asset_metadata, ], user_id, import_only) @contract(asset_key='AssetKey', attr_dict=dict) def set_asset_metadata_attrs(self, asset_key, attr_dict, user_id): 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 2b43280434..7ebb9d94e5 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py @@ -528,12 +528,19 @@ class DraftVersioningModuleStore(SplitMongoModuleStore, ModuleStoreDraftAndPubli """ 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 + # Convert each asset key to the proper branch before saving. + asset_keys = [asset_md.asset_id for asset_md in asset_metadata_list] + for asset_md in asset_metadata_list: + asset_key = asset_md.asset_id + asset_md.asset_id = self._map_revision_to_branch(asset_key, ModuleStoreEnum.RevisionOption.published_only) 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) + for asset_md in asset_metadata_list: + asset_key = asset_md.asset_id + asset_md.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) + # Change each asset key back to its original state. + for k in asset_keys: + asset_md.asset_id = k def _find_course_asset(self, asset_key): return super(DraftVersioningModuleStore, self)._find_course_asset( diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_assetstore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_assetstore.py index 6152cd59f9..cf5517b529 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_assetstore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_assetstore.py @@ -9,8 +9,9 @@ import pytz import unittest from opaque_keys.edx.keys import CourseKey +from opaque_keys.edx.locator import CourseLocator from xmodule.assetstore import AssetMetadata -from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore import ModuleStoreEnum, SortedAssetList, IncorrectlySortedList from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.tests.test_cross_modulestore_import_export import ( @@ -33,19 +34,53 @@ class AssetStoreTestData(object): 'edited_by', 'edited_by_email', 'edited_on', 'created_by', 'created_by_email', 'created_on', 'curr_version', 'prev_version' ) + # pylint: disable=bad-continuation all_asset_data = ( - ('pic1.jpg', 'EKMND332DDBK', 'pix/archive', False, user_id_long, user_email, now, user_id_long, user_email, now, '14', '13'), - ('shout.ogg', 'KFMDONSKF39K', 'sounds', True, user_id, user_email, now, user_id, user_email, now, '1', None), - ('code.tgz', 'ZZB2333YBDMW', 'exercises/14', False, user_id * 2, user_email, now, user_id * 2, user_email, now, 'AB', 'AA'), - ('dog.png', 'PUPY4242X', 'pictures/animals', True, user_id_long * 3, user_email, now, user_id_long * 3, user_email, now, '5', '4'), - ('not_here.txt', 'JJJCCC747', '/dev/null', False, user_id * 4, user_email, now, user_id * 4, user_email, now, '50', '49'), - ('asset.txt', 'JJJCCC747858', '/dev/null', False, user_id * 4, user_email, now, user_id * 4, user_email, now, '50', '49'), - ('roman_history.pdf', 'JASDUNSADK', 'texts/italy', True, user_id * 7, user_email, now, user_id * 7, user_email, now, '1.1', '1.01'), - ('weather_patterns.bmp', '928SJXX2EB', 'science', False, user_id * 8, user_email, now, user_id * 8, user_email, now, '52', '51'), - ('demo.swf', 'DFDFGGGG14', 'demos/easy', False, user_id * 9, user_email, now, user_id * 9, user_email, now, '5', '4'), + ('pic1.jpg', 'EKMND332DDBK', 'pix/archive', False, + user_id_long, user_email, now + timedelta(seconds=10 * 1), user_id_long, user_email, now, '14', '13'), + ('shout.ogg', 'KFMDONSKF39K', 'sounds', True, + user_id, user_email, now + timedelta(seconds=10 * 2), user_id, user_email, now, '1', None), + ('code.tgz', 'ZZB2333YBDMW', 'exercises/14', False, + user_id * 2, user_email, now + timedelta(seconds=10 * 3), user_id * 2, user_email, now, 'AB', 'AA'), + ('dog.png', 'PUPY4242X', 'pictures/animals', True, + user_id_long * 3, user_email, now + timedelta(seconds=10 * 4), user_id_long * 3, user_email, now, '5', '4'), + ('not_here.txt', 'JJJCCC747', '/dev/null', False, + user_id * 4, user_email, now + timedelta(seconds=10 * 5), user_id * 4, user_email, now, '50', '49'), + ('asset.txt', 'JJJCCC747858', '/dev/null', False, + user_id * 4, user_email, now + timedelta(seconds=10 * 6), user_id * 4, user_email, now, '50', '49'), + ('roman_history.pdf', 'JASDUNSADK', 'texts/italy', True, + user_id * 7, user_email, now + timedelta(seconds=10 * 7), user_id * 7, user_email, now, '1.1', '1.01'), + ('weather_patterns.bmp', '928SJXX2EB', 'science', False, + user_id * 8, user_email, now + timedelta(seconds=10 * 8), user_id * 8, user_email, now, '52', '51'), + ('demo.swf', 'DFDFGGGG14', 'demos/easy', False, + user_id * 9, user_email, now + timedelta(seconds=10 * 9), user_id * 9, user_email, now, '5', '4'), ) +class TestSortedAssetList(unittest.TestCase): + """ + Tests the SortedAssetList class. + """ + def setUp(self): + asset_list = [dict(zip(AssetStoreTestData.asset_fields, asset)) for asset in AssetStoreTestData.all_asset_data] + self.sorted_asset_list_by_filename = SortedAssetList(iterable=asset_list) + self.sorted_asset_list_by_last_edit = SortedAssetList(iterable=asset_list, key=lambda x: x['edited_on']) + self.course_key = CourseLocator('org', 'course', 'run') + + def test_exception_on_bad_sort(self): + asset_key = self.course_key.make_asset_key('asset', 'pic1.jpg') + with self.assertRaises(IncorrectlySortedList): + __ = self.sorted_asset_list_by_last_edit.find(asset_key) + + def test_find(self): + asset_key = self.course_key.make_asset_key('asset', 'asset.txt') + self.assertEquals(self.sorted_asset_list_by_filename.find(asset_key), 0) + asset_key_last = self.course_key.make_asset_key('asset', 'weather_patterns.bmp') + self.assertEquals( + self.sorted_asset_list_by_filename.find(asset_key_last), len(AssetStoreTestData.all_asset_data) - 1 + ) + + @attr('mongo') @ddt.ddt class TestMongoAssetMetadataStorage(unittest.TestCase): @@ -57,6 +92,14 @@ class TestMongoAssetMetadataStorage(unittest.TestCase): self.addTypeEqualityFunc(datetime, self._compare_datetimes) self.addTypeEqualityFunc(AssetMetadata, self._compare_metadata) + self.differents = (('different', 'burn.jpg'),) + self.vrmls = ( + ('vrml', 'olympus_mons.vrml'), + ('vrml', 'ponte_vecchio.vrml'), + ) + self.regular_assets = (('asset', 'zippy.png'),) + self.alls = self.differents + self.vrmls + self.regular_assets + def _compare_metadata(self, mdata1, mdata2, msg=None): """ So we can use the below date comparison @@ -389,32 +432,26 @@ class TestMongoAssetMetadataStorage(unittest.TestCase): unknown_asset_key = course.id.make_asset_key('different', 'nosuchfile.jpg') self.assertIsNone(store.find_asset_metadata(unknown_asset_key)) + def _check_asset_values(self, assets, orig): + """ + Check asset type/path 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]) + @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]) - + # pylint: disable=bad-continuation 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: + for asset_type, filename in self.alls: asset_key = course.id.make_asset_key(asset_type, filename) new_asset = self._make_asset_thumbnail_metadata( self._make_asset_metadata(asset_key) @@ -423,13 +460,13 @@ class TestMongoAssetMetadataStorage(unittest.TestCase): # Check 'em. for asset_type, asset_list in ( - ('different', differents), - ('vrml', vrmls), - ('asset', regular_assets), + ('different', self.differents), + ('vrml', self.vrmls), + ('asset', self.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._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) @@ -438,8 +475,93 @@ class TestMongoAssetMetadataStorage(unittest.TestCase): course.id, None, start=0, maxresults=-1, sort=('displayname', ModuleStoreEnum.SortOrder.ascending) ) - self.assertEquals(len(assets), len(alls)) - check_asset_values(assets, alls) + self.assertEquals(len(assets), len(self.alls)) + self._check_asset_values(assets, self.alls) + + @ddt.data(*MODULESTORE_SETUPS) + def test_save_metadata_list(self, storebuilder): + """ + Save a list of asset metadata all at once. + """ + # pylint: disable=bad-continuation + with MongoContentstoreBuilder().build() as contentstore: + with storebuilder.build(contentstore) as store: + course = CourseFactory.create(modulestore=store) + + # Make a list of AssetMetadata objects. + md_list = [] + for asset_type, filename in self.alls: + asset_key = course.id.make_asset_key(asset_type, filename) + md_list.append(self._make_asset_thumbnail_metadata( + self._make_asset_metadata(asset_key) + )) + + # Save 'em. + store.save_asset_metadata_list(md_list, ModuleStoreEnum.UserID.test) + + # Check 'em. + for asset_type, asset_list in ( + ('different', self.differents), + ('vrml', self.vrmls), + ('asset', self.regular_assets), + ): + assets = store.get_all_asset_metadata(course.id, asset_type) + self.assertEquals(len(assets), len(asset_list)) + self._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(self.alls)) + self._check_asset_values(assets, self.alls) + + @ddt.data(*MODULESTORE_SETUPS) + def test_save_metadata_list_with_mismatched_asset(self, storebuilder): + """ + Save a list of asset metadata all at once - but with one asset's metadata from a different course. + """ + # pylint: disable=bad-continuation + with MongoContentstoreBuilder().build() as contentstore: + with storebuilder.build(contentstore) as store: + course1 = CourseFactory.create(modulestore=store) + course2 = CourseFactory.create(modulestore=store) + + # Make a list of AssetMetadata objects. + md_list = [] + for asset_type, filename in self.alls: + if asset_type == 'asset': + asset_key = course2.id.make_asset_key(asset_type, filename) + else: + asset_key = course1.id.make_asset_key(asset_type, filename) + md_list.append(self._make_asset_thumbnail_metadata( + self._make_asset_metadata(asset_key) + )) + + # Save 'em. + store.save_asset_metadata_list(md_list, ModuleStoreEnum.UserID.test) + + # Check 'em. + for asset_type, asset_list in ( + ('different', self.differents), + ('vrml', self.vrmls), + ): + assets = store.get_all_asset_metadata(course1.id, asset_type) + self.assertEquals(len(assets), len(asset_list)) + self._check_asset_values(assets, asset_list) + + self.assertEquals(len(store.get_all_asset_metadata(course1.id, 'asset')), 0) + self.assertEquals(len(store.get_all_asset_metadata(course1.id, None)), 3) + + assets = store.get_all_asset_metadata( + course1.id, None, start=0, maxresults=-1, + sort=('displayname', ModuleStoreEnum.SortOrder.ascending) + ) + self.assertEquals(len(assets), len(self.differents + self.vrmls)) + self._check_asset_values(assets, self.differents + self.vrmls) @ddt.data(*MODULESTORE_SETUPS) def test_delete_all_different_type(self, storebuilder): @@ -462,7 +584,6 @@ class TestMongoAssetMetadataStorage(unittest.TestCase): """ Save multiple metadata in each store and retrieve it singularly, as all assets, and after deleting all. """ - # Temporarily only perform this test for Old Mongo - not Split. with MongoContentstoreBuilder().build() as contentstore: with storebuilder.build(contentstore) as store: course1 = CourseFactory.create(modulestore=store) @@ -497,10 +618,13 @@ class TestMongoAssetMetadataStorage(unittest.TestCase): asset_page = store.get_all_asset_metadata( course2.id, 'asset', start=2 * i, maxresults=2, sort=sort_test[0] ) - self.assertEquals(len(asset_page), sort_test[2][i]) - self.assertEquals(asset_page[0].asset_id.path, sort_test[1][2 * i]) - if sort_test[2][i] == 2: - self.assertEquals(asset_page[1].asset_id.path, sort_test[1][(2 * i) + 1]) + num_expected_results = sort_test[2][i] + expected_filename = sort_test[1][2 * i] + self.assertEquals(len(asset_page), num_expected_results) + self.assertEquals(asset_page[0].asset_id.path, expected_filename) + if num_expected_results == 2: + expected_filename = sort_test[1][(2 * i) + 1] + self.assertEquals(asset_page[1].asset_id.path, expected_filename) # Now fetch everything. asset_page = store.get_all_asset_metadata(