Merge pull request #6166 from edx/jeskew/modulestore_asset_md_format_changes

Make changes to the Mongo modulestore format for asset metadata.

https://openedx.atlassian.net/browse/PLAT-301

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']

common/lib/xmodule/xmodule/assetstore/tests/assets.xsd

@@ -29,7 +29,7 @@
   <xs:all>
     <xs:element name="asset_id" type="stringType"/>
     <xs:element name="contenttype" type="stringType"/>
-    <xs:element name="basename" type="stringType"/>
+    <xs:element name="pathname" type="stringType"/>
     <xs:element name="internal_name" type="stringType"/>
     <xs:element name="locked" type="boolType"/>
     <xs:element name="thumbnail" type="stringType" minOccurs="0"/>

common/lib/xmodule/xmodule/modulestore/mongo/base.py

@@ -1478,12 +1478,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):
@@ -1515,7 +1524,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
 
@@ -1532,8 +1541,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.
@@ -1565,7 +1575,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):
@@ -1588,7 +1601,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
 

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,
@@ -181,6 +181,38 @@ class TestMongoAssetMetadataStorage(unittest.TestCase):
                 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):
         """
         Save multiple metadata in each store and retrieve it singularly, as all assets, and after deleting all.
@@ -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'),