Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

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

Browse Source 
This version component reflects the "version" of the StaticContent
objects which we cache server-side.  If the layout of those objects
changes between releases, errors occur when loading them from cache.

By using a separate version value, which can be incremented on its own
after a change has been made to the StaticContent class, we can avoid
loading older cached content and in turn take advantage of these changes
faster, without needing to intervene operationally.
This commit is contained in:
Toby Lawrence
2016-07-07 11:26:27 -04:00
parent 07eb59c331
commit f15533dc8f
5 changed files with 54 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
common/djangoapps/contentserver/__init__.py
View File

@@ -1,5 +1,3 @@
"""
Serves course assets to end users.
"""
CONTENTSERVER_VERSION = 1

8
common/djangoapps/contentserver/caching.py
View File

@@ -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)

37
common/djangoapps/contentserver/test/test_contentserver.py
View File

@@ -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

14
common/djangoapps/static_replace/test/test_static_replace.py
View File

@@ -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):

9
common/lib/xmodule/xmodule/contentstore/content.py
View File

@@ -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):