Merge pull request #8989 from edx/utkjad/fixtemp

PLAT-734 Fix slow transaction due to multiple calls to unpickling

cms/djangoapps/contentstore/views/tests/test_assets.py

@@ -6,13 +6,12 @@ from io import BytesIO
 from pytz import UTC
 from PIL import Image
 import json
-
+from mock import patch
 from django.conf import settings
 
 from contentstore.tests.utils import CourseTestCase
 from contentstore.views import assets
 from contentstore.utils import reverse_course_url
-from xmodule.assetstore.assetmgr import AssetMetadataFoundTemporary
 from xmodule.assetstore import AssetMetadata
 from xmodule.contentstore.content import StaticContent
 from xmodule.contentstore.django import contentstore
@@ -331,23 +330,13 @@ class DownloadTestCase(AssetsTestCase):
         resp = self.client.get(url, HTTP_ACCEPT='text/html')
         self.assertEquals(resp.status_code, 404)
 
-    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.GENERAL_ASSET_TYPE, 'pic1.jpg')
-        asset_md = AssetMetadata(asset_key, {
-            'internal_name': 'EKMND332DDBK',
-            'basename': 'pix/archive',
-            'locked': False,
-            'curr_version': '14',
-            'prev_version': '13'
-        })
-        modulestore().save_asset_metadata(asset_md, 15)
-        # Get the asset metadata and have it be found in the modulestore.
-        # Currently, no asset metadata should be found in the modulestore. The code is not yet storing it there.
-        # If asset metadata *is* found there, an exception is raised. This test ensures the exception is indeed raised.
-        # THIS IS TEMPORARY. Soon, asset metadata *will* be stored in the modulestore.
-        with self.assertRaises((AssetMetadataFoundTemporary, NameError)):
-            self.client.get(unicode(asset_key), HTTP_ACCEPT='text/html')
+    @patch('xmodule.modulestore.mixed.MixedModuleStore.find_asset_metadata')
+    def test_pickling_calls(self, patched_find_asset_metadata):
+        """ Tests if assets are not calling find_asset_metadata
+        """
+        patched_find_asset_metadata.return_value = None
+        self.client.get(self.uploaded_url, HTTP_ACCEPT='text/html')
+        self.assertFalse(patched_find_asset_metadata.called)
 
 
 class AssetToJsonTestCase(AssetsTestCase):

common/lib/xmodule/xmodule/assetstore/assetmgr.py

@@ -9,11 +9,12 @@ Handles:
 Phase 1: Checks to see if an asset's metadata can be found in the course's modulestore.
     If not found, fails over to access the asset from the contentstore.
     At first, the asset metadata will never be found, since saving isn't implemented yet.
+Note: Hotfix (PLAT-734) No asset calls find_asset_metadata, and directly accesses from contentstore.
+
 """
 
 from contracts import contract, new_contract
 from opaque_keys.edx.keys import AssetKey
-from xmodule.modulestore.django import modulestore
 from xmodule.contentstore.django import contentstore
 
 
@@ -49,14 +50,11 @@ class AssetManager(object):
     @contract(asset_key='AssetKey', throw_on_not_found='bool', as_stream='bool')
     def find(asset_key, throw_on_not_found=True, as_stream=False):
         """
-        Finds a course asset either in the assetstore -or- in the deprecated contentstore.
+        Finds course asset in the deprecated contentstore.
+        This method was previously searching for the course asset in the assetstore first, then in the deprecated
+        contentstore. However, the asset was never found in the assetstore since an asset's metadata is
+        not yet stored there.(removed calls to modulestore().find_asset_metadata(asset_key))
+        The assetstore search was removed due to performance issues caused by each call unpickling the pickled and
+        compressed course structure from the structure cache.
         """
-        content_md = modulestore().find_asset_metadata(asset_key)
-
-        # If found, raise an exception.
-        if content_md:
-            # For now, no asset metadata should be found in the modulestore.
-            raise AssetMetadataFoundTemporary()
-        else:
-            # If not found, load the asset via the contentstore.
-            return contentstore().find(asset_key, throw_on_not_found, as_stream)
+        return contentstore().find(asset_key, throw_on_not_found, as_stream)