Merge pull request #12959 from edx/PERF-346

[PERF-346] Add a second version component to versioned course asset URLs

common/djangoapps/contentserver/__init__.py

 """
 Serves course assets to end users.
 """
-
-CONTENTSERVER_VERSION = 1

common/djangoapps/contentserver/caching.py

@@ -4,7 +4,7 @@ Helper functions for caching course assets.
 from django.core.cache import caches
 from django.core.cache.backends.base import InvalidCacheBackendError
 from opaque_keys import InvalidKeyError
-from . import CONTENTSERVER_VERSION
+from xmodule.contentstore.content import STATIC_CONTENT_VERSION
 
 # See if there's a "course_assets" cache configured, and if not, fallback to the default cache.
 CONTENT_CACHE = caches['default']
@@ -18,14 +18,14 @@ def set_cached_content(content):
     """
     Stores the given piece of content in the cache, using its location as the key.
     """
-    CONTENT_CACHE.set(unicode(content.location).encode("utf-8"), content, version=CONTENTSERVER_VERSION)
+    CONTENT_CACHE.set(unicode(content.location).encode("utf-8"), content, version=STATIC_CONTENT_VERSION)
 
 
 def get_cached_content(location):
     """
     Retrieves the given piece of content by its location if cached.
     """
-    return CONTENT_CACHE.get(unicode(location).encode("utf-8"), version=CONTENTSERVER_VERSION)
+    return CONTENT_CACHE.get(unicode(location).encode("utf-8"), version=STATIC_CONTENT_VERSION)
 
 
 def del_cached_content(location):
@@ -46,4 +46,4 @@ def del_cached_content(location):
         # although deprecated keys allowed run=None, new keys don't if there is no version.
         pass
 
-    CONTENT_CACHE.delete_many(locations, version=CONTENTSERVER_VERSION)
+    CONTENT_CACHE.delete_many(locations, version=STATIC_CONTENT_VERSION)

common/djangoapps/contentserver/test/test_contentserver.py

@@ -16,7 +16,7 @@ from django.test.utils import override_settings
 from mock import patch
 
 from xmodule.contentstore.django import contentstore
-from xmodule.contentstore.content import StaticContent
+from xmodule.contentstore.content import StaticContent, VERSIONED_ASSETS_PREFIX
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
 from xmodule.modulestore.xml_importer import import_course_from_xml
@@ -51,6 +51,20 @@ def get_versioned_asset_url(asset_path):
     return asset_path
 
 
+def get_old_style_versioned_asset_url(asset_path):
+    """
+    Creates an old-style versioned asset URL.
+    """
+    try:
+        locator = StaticContent.get_location_from_path(asset_path)
+        content = AssetManager.find(locator, as_stream=True)
+        return u'{}/{}{}'.format(VERSIONED_ASSETS_PREFIX, content.content_digest, asset_path)
+    except (InvalidKeyError, ItemNotFoundError):
+        pass
+
+    return asset_path
+
+
 @ddt.ddt
 @override_settings(CONTENTSTORE=TEST_DATA_CONTENTSTORE)
 class ContentStoreToyCourseTest(SharedModuleStoreTestCase):
@@ -76,12 +90,14 @@ class ContentStoreToyCourseTest(SharedModuleStoreTestCase):
         cls.locked_asset = cls.course_key.make_asset_key('asset', 'sample_static.html')
         cls.url_locked = unicode(cls.locked_asset)
         cls.url_locked_versioned = get_versioned_asset_url(cls.url_locked)
+        cls.url_locked_versioned_old_style = get_old_style_versioned_asset_url(cls.url_locked)
         cls.contentstore.set_attr(cls.locked_asset, 'locked', True)
 
         # An unlocked asset
         cls.unlocked_asset = cls.course_key.make_asset_key('asset', 'another_static.txt')
         cls.url_unlocked = unicode(cls.unlocked_asset)
         cls.url_unlocked_versioned = get_versioned_asset_url(cls.url_unlocked)
+        cls.url_unlocked_versioned_old_style = get_old_style_versioned_asset_url(cls.url_unlocked)
         cls.length_unlocked = cls.contentstore.get_attr(cls.unlocked_asset, 'length')
 
     def setUp(self):
@@ -110,6 +126,14 @@ class ContentStoreToyCourseTest(SharedModuleStoreTestCase):
         resp = self.client.get(self.url_unlocked_versioned)
         self.assertEqual(resp.status_code, 200)
 
+    def test_unlocked_versioned_asset_old_style(self):
+        """
+        Test that unlocked assets that are versioned (old-style) are being served.
+        """
+        self.client.logout()
+        resp = self.client.get(self.url_unlocked_versioned_old_style)
+        self.assertEqual(resp.status_code, 200)
+
     def test_unlocked_versioned_asset_with_nonexistent_version(self):
         """
         Test that unlocked assets that are versioned, but have a nonexistent version,
@@ -133,6 +157,17 @@ class ContentStoreToyCourseTest(SharedModuleStoreTestCase):
         resp = self.client.get(self.url_locked_versioned)
         self.assertEqual(resp.status_code, 200)
 
+    def test_locked_versioned_old_styleasset(self):
+        """
+        Test that locked assets that are versioned (old-style) are being served.
+        """
+        CourseEnrollment.enroll(self.non_staff_usr, self.course_key)
+        self.assertTrue(CourseEnrollment.is_enrolled(self.non_staff_usr, self.course_key))
+
+        self.client.login(username=self.non_staff_usr, password='test')
+        resp = self.client.get(self.url_locked_versioned_old_style)
+        self.assertEqual(resp.status_code, 200)
+
     def test_locked_asset_not_logged_in(self):
         """
         Test that locked assets behave appropriately in case the user is not

common/djangoapps/static_replace/test/test_static_replace.py

@@ -525,9 +525,9 @@ class CanonicalContentTest(SharedModuleStoreTestCase):
         # - finally shove back in our regex patterns
         digest = CanonicalContentTest.get_content_digest_for_asset_path(prefix, start)
         if digest:
-            adjusted_asset_key = u'assets/courseware/MARK/asset-v1:a+b+{}+type@asset+block'.format(prefix)
-            adjusted_th_key = u'assets/courseware/MARK/asset-v1:a+b+{}+type@thumbnail+block'.format(prefix)
-            encoded_asset_key = u'/assets/courseware/MARK/asset-v1:a+b+{}+type@asset+block@'.format(prefix)
+            adjusted_asset_key = u'assets/courseware/VMARK/HMARK/asset-v1:a+b+{}+type@asset+block'.format(prefix)
+            adjusted_th_key = u'assets/courseware/VMARK/HMARK/asset-v1:a+b+{}+type@thumbnail+block'.format(prefix)
+            encoded_asset_key = u'/assets/courseware/VMARK/HMARK/asset-v1:a+b+{}+type@asset+block@'.format(prefix)
             encoded_asset_key = urlquote(encoded_asset_key)
 
         expected = expected.format(
@@ -544,7 +544,8 @@ class CanonicalContentTest(SharedModuleStoreTestCase):
         )
 
         expected = encode_unicode_characters_in_url(expected)
-        expected = expected.replace('MARK', '[a-f0-9]{32}')
+        expected = expected.replace('VMARK', r'v[\d]')
+        expected = expected.replace('HMARK', '[a-f0-9]{32}')
         expected = expected.replace('+', r'\+').replace('?', r'\?')
 
         with check_mongo_calls(mongo_calls):
@@ -728,7 +729,7 @@ class CanonicalContentTest(SharedModuleStoreTestCase):
         # - finally shove back in our regex patterns
         digest = CanonicalContentTest.get_content_digest_for_asset_path(prefix, start)
         if digest:
-            adjusted_c4x_block = 'assets/courseware/MARK/c4x/a/b/asset'
+            adjusted_c4x_block = 'assets/courseware/VMARK/HMARK/c4x/a/b/asset'
             encoded_c4x_block = urlquote('/' + adjusted_c4x_block + '/')
 
         expected = expected.format(
@@ -741,7 +742,8 @@ class CanonicalContentTest(SharedModuleStoreTestCase):
         )
 
         expected = encode_unicode_characters_in_url(expected)
-        expected = expected.replace('MARK', '[a-f0-9]{32}')
+        expected = expected.replace('VMARK', r'v[\d]')
+        expected = expected.replace('HMARK', '[a-f0-9]{32}')
         expected = expected.replace('+', r'\+').replace('?', r'\?')
 
         with check_mongo_calls(mongo_calls):

common/lib/xmodule/xmodule/contentstore/content.py

@@ -3,12 +3,13 @@ import uuid
 
 from xmodule.assetstore.assetmgr import AssetManager
 
+STATIC_CONTENT_VERSION = 1
 XASSET_LOCATION_TAG = 'c4x'
 XASSET_SRCREF_PREFIX = 'xasset:'
 XASSET_THUMBNAIL_TAIL_NAME = '.jpg'
 STREAM_DATA_CHUNK_SIZE = 1024
 VERSIONED_ASSETS_PREFIX = '/assets/courseware'
-VERSIONED_ASSETS_PATTERN = r'/assets/courseware/([a-f0-9]{32})'
+VERSIONED_ASSETS_PATTERN = r'/assets/courseware/(v[\d]/)?([a-f0-9]{32})'
 
 import os
 import logging
@@ -163,7 +164,7 @@ class StaticContent(object):
         if StaticContent.is_versioned_asset_path(asset_path):
             result = re.match(VERSIONED_ASSETS_PATTERN, asset_path)
             if result is not None:
-                asset_digest = result.groups()[0]
+                asset_digest = result.groups()[1]
             asset_path = re.sub(VERSIONED_ASSETS_PATTERN, '', asset_path)
 
         return (asset_digest, asset_path)
@@ -178,7 +179,9 @@ class StaticContent(object):
         if StaticContent.is_versioned_asset_path(path):
             return path
 
-        return VERSIONED_ASSETS_PREFIX + '/' + version + path
+        structure_version = 'v{}'.format(STATIC_CONTENT_VERSION)
+
+        return u'{}/{}/{}{}'.format(VERSIONED_ASSETS_PREFIX, structure_version, version, path)
 
     @staticmethod
     def get_asset_key_from_path(course_key, path):