diff --git a/common/lib/xmodule/xmodule/assetstore/__init__.py b/common/lib/xmodule/xmodule/assetstore/__init__.py index c7bbe68f33..fcab2d3c2a 100644 --- a/common/lib/xmodule/xmodule/assetstore/__init__.py +++ b/common/lib/xmodule/xmodule/assetstore/__init__.py @@ -24,7 +24,7 @@ class AssetMetadata(object): in the modulestore. """ - TOP_LEVEL_ATTRS = ['basename', 'internal_name', 'locked', 'contenttype', 'thumbnail', 'fields'] + TOP_LEVEL_ATTRS = ['pathname', 'internal_name', 'locked', 'contenttype', 'thumbnail', 'fields'] 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 @@ -34,14 +34,14 @@ class AssetMetadata(object): ASSET_TYPE = 'asset' @contract(asset_id='AssetKey', - basename='basestring|None', internal_name='basestring|None', + pathname='basestring|None', internal_name='basestring|None', locked='bool|None', contenttype='basestring|None', thumbnail='basestring|None', fields='dict|None', curr_version='basestring|None', prev_version='basestring|None', created_by='int|None', created_by_email='basestring|None', created_on='datetime|None', edited_by='int|None', edited_by_email='basestring|None', edited_on='datetime|None') def __init__(self, asset_id, - basename=None, internal_name=None, + pathname=None, internal_name=None, locked=None, contenttype=None, thumbnail=None, fields=None, curr_version=None, prev_version=None, @@ -53,7 +53,7 @@ class AssetMetadata(object): Arguments: asset_id (AssetKey): Key identifying this particular asset. - basename (str): Original path to file at asset upload time. + pathname (str): Original path to file at asset upload time. internal_name (str): Name, url, or handle for the storage system to access the file. locked (bool): If True, only course participants can access the asset. contenttype (str): MIME type of the asset. @@ -71,7 +71,7 @@ class AssetMetadata(object): Not saved. """ self.asset_id = asset_id if field_decorator is None else field_decorator(asset_id) - self.basename = basename # Path w/o filename. + self.pathname = pathname # Path w/o filename. self.internal_name = internal_name self.locked = locked self.contenttype = contenttype @@ -91,7 +91,7 @@ class AssetMetadata(object): def __repr__(self): return """AssetMetadata{!r}""".format(( self.asset_id, - self.basename, self.internal_name, + self.pathname, self.internal_name, self.locked, self.contenttype, self.fields, self.curr_version, self.prev_version, self.created_by, self.created_by_email, self.created_on, @@ -118,7 +118,7 @@ class AssetMetadata(object): """ return { 'filename': self.asset_id.path, - 'basename': self.basename, + 'pathname': self.pathname, 'internal_name': self.internal_name, 'locked': self.locked, 'contenttype': self.contenttype, @@ -145,7 +145,7 @@ class AssetMetadata(object): """ if asset_doc is None: return - self.basename = asset_doc['basename'] + self.pathname = asset_doc['pathname'] self.internal_name = asset_doc['internal_name'] self.locked = asset_doc['locked'] self.contenttype = asset_doc['contenttype'] diff --git a/common/lib/xmodule/xmodule/assetstore/tests/assets.xsd b/common/lib/xmodule/xmodule/assetstore/tests/assets.xsd index 8a05d353c8..ab7be765b8 100644 --- a/common/lib/xmodule/xmodule/assetstore/tests/assets.xsd +++ b/common/lib/xmodule/xmodule/assetstore/tests/assets.xsd @@ -29,7 +29,7 @@ - + diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index 9f5a09aee3..93558e83b1 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -1476,12 +1476,21 @@ 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), 'storage': 'FILLMEIN-TMP', 'assets': []} - course_assets['_id'] = self.asset_collection.insert(course_assets) + course_assets = {'course_id': unicode(course_key), 'assets': {}} + course_assets['assets']['_id'] = self.asset_collection.insert(course_assets) + else: + course_assets['assets']['_id'] = course_assets['_id'] - return course_assets + return course_assets['assets'] + + def _make_mongo_asset_key(self, asset_type): + """ + Given a asset type, form a key needed to update the proper embedded field in the Mongo doc. + """ + return 'assets.{}'.format(asset_type) @contract(asset_metadata='AssetMetadata') def save_asset_metadata(self, asset_metadata, user_id): @@ -1513,7 +1522,7 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo # Update the document. self.asset_collection.update( {'_id': course_assets['_id']}, - {'$set': {asset_metadata.asset_id.block_type: all_assets.as_list()}} + {'$set': {self._make_mongo_asset_key(asset_metadata.asset_id.block_type): all_assets.as_list()}} ) return True @@ -1530,8 +1539,9 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo """ source_assets = self._find_course_assets(source_course_key) dest_assets = source_assets.copy() - dest_assets['course_id'] = unicode(dest_course_key) del dest_assets['_id'] + dest_assets = {'assets': dest_assets} + dest_assets['course_id'] = unicode(dest_course_key) self.asset_collection.remove({'course_id': unicode(dest_course_key)}) # Update the document. @@ -1563,7 +1573,10 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo # Generate a Mongo doc from the metadata and update the course asset info. all_assets[asset_idx] = md.to_storable() - self.asset_collection.update({'_id': course_assets['_id']}, {"$set": {asset_key.block_type: all_assets}}) + self.asset_collection.update( + {'_id': course_assets['_id']}, + {"$set": {self._make_mongo_asset_key(asset_key.block_type): all_assets}} + ) @contract(asset_key='AssetKey') def delete_asset_metadata(self, asset_key, user_id): @@ -1586,7 +1599,7 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo # Update the document. self.asset_collection.update( {'_id': course_assets['_id']}, - {'$set': {asset_key.block_type: all_asset_info}} + {'$set': {self._make_mongo_asset_key(asset_key.block_type): all_asset_info}} ) return 1 diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_assetstore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_assetstore.py index 4d10ac87f3..87a9c5d7c5 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_assetstore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_assetstore.py @@ -26,7 +26,7 @@ class AssetStoreTestData(object): user_email = "me@example.com" asset_fields = ( - 'filename', 'internal_name', 'basename', 'locked', + 'filename', 'internal_name', 'pathname', 'locked', 'edited_by', 'edited_by_email', 'edited_on', 'created_by', 'created_by_email', 'created_on', 'curr_version', 'prev_version' ) @@ -77,7 +77,7 @@ class TestMongoAssetMetadataStorage(unittest.TestCase): now = datetime.now(pytz.utc) return AssetMetadata( asset_loc, internal_name='EKMND332DDBK', - basename='pictures/historical', contenttype='image/jpeg', + pathname='pictures/historical', contenttype='image/jpeg', locked=False, fields={'md5': '77631ca4f0e08419b70726a447333ab6'}, edited_by=ModuleStoreEnum.UserID.test, edited_on=now, created_by=ModuleStoreEnum.UserID.test, created_on=now, @@ -180,6 +180,38 @@ class TestMongoAssetMetadataStorage(unittest.TestCase): # Still one here? self.assertEquals(len(store.get_all_asset_metadata(course.id, 'asset')), 1) + @ddt.data(*MODULESTORE_SETUPS) + def test_different_asset_types(self, storebuilder): + """ + Test saving assets with other asset types. + """ + with MongoContentstoreBuilder().build() as contentstore: + with storebuilder.build(contentstore) as store: + course = CourseFactory.create(modulestore=store) + new_asset_loc = course.id.make_asset_key('vrml', 'pyramid.vrml') + new_asset_md = self._make_asset_metadata(new_asset_loc) + # Add asset metadata. + store.save_asset_metadata(new_asset_md, ModuleStoreEnum.UserID.test) + self.assertEquals(len(store.get_all_asset_metadata(course.id, 'vrml')), 1) + self.assertEquals(len(store.get_all_asset_metadata(course.id, 'asset')), 0) + + @ddt.data(*MODULESTORE_SETUPS) + def test_asset_types_with_other_field_names(self, storebuilder): + """ + Test saving assets using an asset type of 'course_id'. + """ + with MongoContentstoreBuilder().build() as contentstore: + with storebuilder.build(contentstore) as store: + course = CourseFactory.create(modulestore=store) + new_asset_loc = course.id.make_asset_key('course_id', 'just_to_see_if_it_still_works.jpg') + new_asset_md = self._make_asset_metadata(new_asset_loc) + # Add asset metadata. + store.save_asset_metadata(new_asset_md, ModuleStoreEnum.UserID.test) + self.assertEquals(len(store.get_all_asset_metadata(course.id, 'course_id')), 1) + self.assertEquals(len(store.get_all_asset_metadata(course.id, 'asset')), 0) + all_assets = store.get_all_asset_metadata(course.id, 'course_id') + self.assertEquals(all_assets[0].asset_id.path, new_asset_loc.path) + @ddt.data(*MODULESTORE_SETUPS) def test_lock_unlock_assets(self, storebuilder): """ @@ -206,7 +238,7 @@ class TestMongoAssetMetadataStorage(unittest.TestCase): self.assertEquals(reupdated_asset_md.locked, locked_state) ALLOWED_ATTRS = ( - ('basename', '/new/path'), + ('pathname', '/new/path'), ('internal_name', 'new_filename.txt'), ('locked', True), ('contenttype', 'image/png'),