From 6de1137f67efb25dc3836c1375fdf71cddc49777 Mon Sep 17 00:00:00 2001 From: John Eskew Date: Sat, 3 Jan 2015 18:59:09 -0500 Subject: [PATCH] Optimize large amounts of asset metadata in the Split modulestore. Use binary search instead of linear search. Add tests for saving asset metadata lists. Refactor to use common code across modulestores. --- .../xmodule/xmodule/modulestore/__init__.py | 93 ++++++-- .../xmodule/xmodule/modulestore/mongo/base.py | 32 +-- .../xmodule/modulestore/split_mongo/split.py | 81 ++----- .../modulestore/split_mongo/split_draft.py | 15 +- .../modulestore/tests/test_assetstore.py | 198 ++++++++++++++---- 5 files changed, 276 insertions(+), 143 deletions(-) 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(