diff --git a/common/djangoapps/static_replace/__init__.py b/common/djangoapps/static_replace/__init__.py index b22a1058e1..64145302ff 100644 --- a/common/djangoapps/static_replace/__init__.py +++ b/common/djangoapps/static_replace/__init__.py @@ -5,6 +5,7 @@ from django.contrib.staticfiles.storage import staticfiles_storage from django.contrib.staticfiles import finders from django.conf import settings +from static_replace.models import AssetBaseUrlConfig from xmodule.modulestore.django import modulestore from xmodule.modulestore import ModuleStoreEnum from xmodule.contentstore.content import StaticContent @@ -180,7 +181,8 @@ def replace_static_urls(text, data_directory=None, course_id=None, static_asset_ else: # if not, then assume it's courseware specific content and then look in the # Mongo-backed database - url = StaticContent.convert_legacy_static_url_with_course_id(rest, course_id) + base_url = AssetBaseUrlConfig.get_base_url() + url = StaticContent.get_canonicalized_asset_path(course_id, rest, base_url) if AssetLocator.CANONICAL_NAMESPACE in url: url = url.replace('block@', 'block/', 1) diff --git a/common/djangoapps/static_replace/admin.py b/common/djangoapps/static_replace/admin.py new file mode 100644 index 0000000000..d475d61b2f --- /dev/null +++ b/common/djangoapps/static_replace/admin.py @@ -0,0 +1,30 @@ +""" +Django admin page for AssetBaseUrlConfig, which allows you to set the base URL +that gets prepended to asset URLs in order to serve them from, say, a CDN. +""" +from django.contrib import admin + +from config_models.admin import ConfigurationModelAdmin +from .models import AssetBaseUrlConfig + + +class AssetBaseUrlConfigAdmin(ConfigurationModelAdmin): + """ + Basic configuration for asset base URL. + """ + list_display = [ + 'base_url' + ] + + def get_list_display(self, request): + """ + Restore default list_display behavior. + + ConfigurationModelAdmin overrides this, but in a way that doesn't + respect the ordering. This lets us customize it the usual Django admin + way. + """ + return self.list_display + + +admin.site.register(AssetBaseUrlConfig, AssetBaseUrlConfigAdmin) diff --git a/common/djangoapps/static_replace/models.py b/common/djangoapps/static_replace/models.py new file mode 100644 index 0000000000..7521d885bb --- /dev/null +++ b/common/djangoapps/static_replace/models.py @@ -0,0 +1,29 @@ +""" +Models for static_replace +""" + +from django.db.models.fields import TextField +from config_models.models import ConfigurationModel + + +class AssetBaseUrlConfig(ConfigurationModel): + """Configuration for the base URL used for static assets.""" + + class Meta(object): + app_label = 'static_replace' + + base_url = TextField( + blank=True, + help_text="The alternative hostname to serve static assets from. Should be in the form of hostname[:port]." + ) + + @classmethod + def get_base_url(cls): + """Gets the base URL to use for serving static assets, if present""" + return cls.current().base_url + + def __repr__(self): + return ''.format(self.get_base_url()) + + def __unicode__(self): + return unicode(repr(self)) diff --git a/common/djangoapps/static_replace/test/test_static_replace.py b/common/djangoapps/static_replace/test/test_static_replace.py index 0d86acfdc7..56f90bd0be 100644 --- a/common/djangoapps/static_replace/test/test_static_replace.py +++ b/common/djangoapps/static_replace/test/test_static_replace.py @@ -1,4 +1,11 @@ +"""Tests for static_replace""" + +from urllib import quote_plus + +import ddt import re +from PIL import Image +from cStringIO import StringIO from nose.tools import assert_equals, assert_true, assert_false # pylint: disable=no-name-in-module from static_replace import ( @@ -11,7 +18,12 @@ from static_replace import ( from mock import patch, Mock from opaque_keys.edx.locations import SlashSeparatedCourseKey +from xmodule.contentstore.content import StaticContent +from xmodule.contentstore.django import contentstore +from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.mongo import MongoModuleStore +from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory, check_mongo_calls from xmodule.modulestore.xml import XMLModuleStore DATA_DIRECTORY = 'data_dir' @@ -85,21 +97,23 @@ def test_storage_url_not_exists(mock_storage): @patch('static_replace.StaticContent', autospec=True) @patch('static_replace.modulestore', autospec=True) -def test_mongo_filestore(mock_modulestore, mock_static_content): +@patch('static_replace.AssetBaseUrlConfig.get_base_url') +def test_mongo_filestore(mock_get_base_url, mock_modulestore, mock_static_content): mock_modulestore.return_value = Mock(MongoModuleStore) - mock_static_content.convert_legacy_static_url_with_course_id.return_value = "c4x://mock_url" + mock_static_content.get_canonicalized_asset_path.return_value = "c4x://mock_url" + mock_get_base_url.return_value = u'' # No namespace => no change to path assert_equals('"/static/data_dir/file.png"', replace_static_urls(STATIC_SOURCE, DATA_DIRECTORY)) # Namespace => content url assert_equals( - '"' + mock_static_content.convert_legacy_static_url_with_course_id.return_value + '"', + '"' + mock_static_content.get_canonicalized_asset_path.return_value + '"', replace_static_urls(STATIC_SOURCE, DATA_DIRECTORY, course_id=COURSE_KEY) ) - mock_static_content.convert_legacy_static_url_with_course_id.assert_called_once_with('file.png', COURSE_KEY) + mock_static_content.get_canonicalized_asset_path.assert_called_once_with(COURSE_KEY, 'file.png', u'') @patch('static_replace.settings', autospec=True) @@ -161,3 +175,274 @@ def test_regex(): for s in no: print 'Should not match: {0!r}'.format(s) assert_false(re.match(regex, s)) + + +@ddt.ddt +class CanonicalContentTest(SharedModuleStoreTestCase): + """ + Tests the generation of canonical asset URLs for different types + of assets: c4x-style, opaque key style, locked, unlocked, CDN + set, CDN not set, etc. + """ + + def setUp(self): + super(CanonicalContentTest, self).setUp() + + @classmethod + def setUpClass(cls): + cls.courses = {} + + super(CanonicalContentTest, cls).setUpClass() + + names_and_prefixes = [(ModuleStoreEnum.Type.split, 'split'), (ModuleStoreEnum.Type.mongo, 'old')] + for store, prefix in names_and_prefixes: + with cls.store.default_store(store): + cls.courses[prefix] = CourseFactory.create(org='a', course='b', run=prefix) + + # Create an unlocked image. + unlock_content = cls.create_image(prefix, (32, 32), 'blue', '{}_unlock.png') + + # Create a locked image. + lock_content = cls.create_image(prefix, (32, 32), 'green', '{}_lock.png', locked=True) + + # Create a thumbnail of the images. + contentstore().generate_thumbnail(unlock_content, dimensions=(16, 16)) + contentstore().generate_thumbnail(lock_content, dimensions=(16, 16)) + + # Create an unlocked image in a subdirectory. + cls.create_image(prefix, (1, 1), 'red', 'special/{}_unlock.png') + + # Create a locked image in a subdirectory. + cls.create_image(prefix, (1, 1), 'yellow', 'special/{}_lock.png', locked=True) + + # Create an unlocked image with funky characters in the name. + cls.create_image(prefix, (1, 1), 'black', 'weird {}_unlock.png') + + @classmethod + def create_image(cls, prefix, dimensions, color, name, locked=False): + """ + Creates an image. + + Args: + prefix: the prefix to use e.g. split vs mongo + dimensions: tuple of (width, height) + color: the background color of the image + name: the name of the image; can be a format string + locked: whether or not the asset should be locked + + Returns: + StaticContent: the StaticContent object for the created image + """ + new_image = Image.new('RGB', dimensions, color) + new_buf = StringIO() + new_image.save(new_buf, format='png') + new_buf.seek(0) + new_name = name.format(prefix) + new_key = StaticContent.compute_location(cls.courses[prefix].id, new_name) + new_content = StaticContent(new_key, new_name, 'image/png', new_buf.getvalue(), locked=locked) + contentstore().save(new_content) + + return new_content + + @ddt.data( + # No leading slash. + (u'', u'{prefix}_unlock.png', u'/{asset_key}@{prefix}_unlock.png', 1), + (u'', u'{prefix}_lock.png', u'/{asset_key}@{prefix}_lock.png', 1), + (u'', u'weird {prefix}_unlock.png', u'/{asset_key}@weird_{prefix}_unlock.png', 1), + (u'dev', u'{prefix}_unlock.png', u'//dev/{asset_key}@{prefix}_unlock.png', 1), + (u'dev', u'{prefix}_lock.png', u'/{asset_key}@{prefix}_lock.png', 1), + (u'dev', u'weird {prefix}_unlock.png', u'//dev/{asset_key}@weird_{prefix}_unlock.png', 1), + # No leading slash with subdirectory. This ensures we properly substitute slashes. + (u'', u'special/{prefix}_unlock.png', u'/{asset_key}@special_{prefix}_unlock.png', 1), + (u'', u'special/{prefix}_lock.png', u'/{asset_key}@special_{prefix}_lock.png', 1), + (u'dev', u'special/{prefix}_unlock.png', u'//dev/{asset_key}@special_{prefix}_unlock.png', 1), + (u'dev', u'special/{prefix}_lock.png', u'/{asset_key}@special_{prefix}_lock.png', 1), + # Leading slash. + (u'', u'/{prefix}_unlock.png', u'/{asset_key}@{prefix}_unlock.png', 1), + (u'', u'/{prefix}_lock.png', u'/{asset_key}@{prefix}_lock.png', 1), + (u'dev', u'/{prefix}_unlock.png', u'//dev/{asset_key}@{prefix}_unlock.png', 1), + (u'dev', u'/{prefix}_lock.png', u'/{asset_key}@{prefix}_lock.png', 1), + # Leading slash with subdirectory. This ensures we properly substitute slashes. + (u'', u'/special/{prefix}_unlock.png', u'/{asset_key}@special_{prefix}_unlock.png', 1), + (u'', u'/special/{prefix}_lock.png', u'/{asset_key}@special_{prefix}_lock.png', 1), + (u'dev', u'/special/{prefix}_unlock.png', u'//dev/{asset_key}@special_{prefix}_unlock.png', 1), + (u'dev', u'/special/{prefix}_lock.png', u'/{asset_key}@special_{prefix}_lock.png', 1), + # Static path. + (u'', u'/static/{prefix}_unlock.png', u'/{asset_key}@{prefix}_unlock.png', 1), + (u'', u'/static/{prefix}_lock.png', u'/{asset_key}@{prefix}_lock.png', 1), + (u'', u'/static/weird {prefix}_unlock.png', u'/{asset_key}@weird_{prefix}_unlock.png', 1), + (u'dev', u'/static/{prefix}_unlock.png', u'//dev/{asset_key}@{prefix}_unlock.png', 1), + (u'dev', u'/static/{prefix}_lock.png', u'/{asset_key}@{prefix}_lock.png', 1), + (u'dev', u'/static/weird {prefix}_unlock.png', u'//dev/{asset_key}@weird_{prefix}_unlock.png', 1), + # Static path with subdirectory. This ensures we properly substitute slashes. + (u'', u'/static/special/{prefix}_unlock.png', u'/{asset_key}@special_{prefix}_unlock.png', 1), + (u'', u'/static/special/{prefix}_lock.png', u'/{asset_key}@special_{prefix}_lock.png', 1), + (u'dev', u'/static/special/{prefix}_unlock.png', u'//dev/{asset_key}@special_{prefix}_unlock.png', 1), + (u'dev', u'/static/special/{prefix}_lock.png', u'/{asset_key}@special_{prefix}_lock.png', 1), + # Static path with query parameter. + ( + u'', + u'/static/{prefix}_unlock.png?foo=/static/{prefix}_lock.png', + u'/{asset_key}@{prefix}_unlock.png?foo={encoded_asset_key}{prefix}_lock.png', + 2 + ), + ( + u'', + u'/static/{prefix}_lock.png?foo=/static/{prefix}_unlock.png', + u'/{asset_key}@{prefix}_lock.png?foo={encoded_asset_key}{prefix}_unlock.png', + 2 + ), + ( + u'dev', + u'/static/{prefix}_unlock.png?foo=/static/{prefix}_lock.png', + u'//dev/{asset_key}@{prefix}_unlock.png?foo={encoded_asset_key}{prefix}_lock.png', + 2 + ), + ( + u'dev', + u'/static/{prefix}_lock.png?foo=/static/{prefix}_unlock.png', + u'/{asset_key}@{prefix}_lock.png?foo={encoded_base_url}{encoded_asset_key}{prefix}_unlock.png', + 2 + ), + # Already asset key. + (u'', u'/{asset_key}@{prefix}_unlock.png', u'/{asset_key}@{prefix}_unlock.png', 1), + (u'', u'/{asset_key}@{prefix}_lock.png', u'/{asset_key}@{prefix}_lock.png', 1), + (u'dev', u'/{asset_key}@{prefix}_unlock.png', u'//dev/{asset_key}@{prefix}_unlock.png', 1), + (u'dev', u'/{asset_key}@{prefix}_lock.png', u'/{asset_key}@{prefix}_lock.png', 1), + # Old, c4x-style path. + (u'', u'/{c4x}/{prefix}_unlock.png', u'/{c4x}/{prefix}_unlock.png', 1), + (u'', u'/{c4x}/{prefix}_lock.png', u'/{c4x}/{prefix}_lock.png', 1), + (u'', u'/{c4x}/weird_{prefix}_lock.png', u'/{c4x}/weird_{prefix}_lock.png', 1), + (u'dev', u'/{c4x}/{prefix}_unlock.png', u'/{c4x}/{prefix}_unlock.png', 1), + (u'dev', u'/{c4x}/{prefix}_lock.png', u'/{c4x}/{prefix}_lock.png', 1), + (u'dev', u'/{c4x}/weird_{prefix}_unlock.png', u'/{c4x}/weird_{prefix}_unlock.png', 1), + # Thumbnails. + (u'', u'/{th_key}@{prefix}_unlock-{th_ext}', u'/{th_key}@{prefix}_unlock-{th_ext}', 1), + (u'', u'/{th_key}@{prefix}_lock-{th_ext}', u'/{th_key}@{prefix}_lock-{th_ext}', 1), + (u'dev', u'/{th_key}@{prefix}_unlock-{th_ext}', u'//dev/{th_key}@{prefix}_unlock-{th_ext}', 1), + (u'dev', u'/{th_key}@{prefix}_lock-{th_ext}', u'//dev/{th_key}@{prefix}_lock-{th_ext}', 1), + ) + @ddt.unpack + def test_canonical_asset_path_with_new_style_assets(self, base_url, start, expected, mongo_calls): + prefix = 'split' + encoded_base_url = quote_plus('//' + base_url) + c4x = 'c4x/a/b/asset' + asset_key = 'asset-v1:a+b+{}+type@asset+block'.format(prefix) + encoded_asset_key = quote_plus('/asset-v1:a+b+{}+type@asset+block@'.format(prefix)) + th_key = 'asset-v1:a+b+{}+type@thumbnail+block'.format(prefix) + th_ext = 'png-16x16.jpg' + + start = start.format( + prefix=prefix, + c4x=c4x, + asset_key=asset_key, + encoded_base_url=encoded_base_url, + encoded_asset_key=encoded_asset_key, + th_key=th_key, + th_ext=th_ext + ) + expected = expected.format( + prefix=prefix, + c4x=c4x, + asset_key=asset_key, + encoded_base_url=encoded_base_url, + encoded_asset_key=encoded_asset_key, + th_key=th_key, + th_ext=th_ext + ) + + with check_mongo_calls(mongo_calls): + asset_path = StaticContent.get_canonicalized_asset_path(self.courses[prefix].id, start, base_url) + self.assertEqual(asset_path, expected) + + @ddt.data( + # No leading slash. + (u'', u'{prefix}_unlock.png', u'/{c4x}/{prefix}_unlock.png', 1), + (u'', u'{prefix}_lock.png', u'/{c4x}/{prefix}_lock.png', 1), + (u'', u'weird {prefix}_unlock.png', u'/{c4x}/weird_{prefix}_unlock.png', 1), + (u'dev', u'{prefix}_unlock.png', u'//dev/{c4x}/{prefix}_unlock.png', 1), + (u'dev', u'{prefix}_lock.png', u'/{c4x}/{prefix}_lock.png', 1), + (u'dev', u'weird {prefix}_unlock.png', u'//dev/{c4x}/weird_{prefix}_unlock.png', 1), + # No leading slash with subdirectory. This ensures we probably substitute slashes. + (u'', u'special/{prefix}_unlock.png', u'/{c4x}/special_{prefix}_unlock.png', 1), + (u'', u'special/{prefix}_lock.png', u'/{c4x}/special_{prefix}_lock.png', 1), + (u'dev', u'special/{prefix}_unlock.png', u'//dev/{c4x}/special_{prefix}_unlock.png', 1), + (u'dev', u'special/{prefix}_lock.png', u'/{c4x}/special_{prefix}_lock.png', 1), + # Leading slash. + (u'', u'/{prefix}_unlock.png', u'/{c4x}/{prefix}_unlock.png', 1), + (u'', u'/{prefix}_lock.png', u'/{c4x}/{prefix}_lock.png', 1), + (u'dev', u'/{prefix}_unlock.png', u'//dev/{c4x}/{prefix}_unlock.png', 1), + (u'dev', u'/{prefix}_lock.png', u'/{c4x}/{prefix}_lock.png', 1), + # Leading slash with subdirectory. This ensures we properly substitute slashes. + (u'', u'/special/{prefix}_unlock.png', u'/{c4x}/special_{prefix}_unlock.png', 1), + (u'', u'/special/{prefix}_lock.png', u'/{c4x}/special_{prefix}_lock.png', 1), + (u'dev', u'/special/{prefix}_unlock.png', u'//dev/{c4x}/special_{prefix}_unlock.png', 1), + (u'dev', u'/special/{prefix}_lock.png', u'/{c4x}/special_{prefix}_lock.png', 1), + # Static path. + (u'', u'/static/{prefix}_unlock.png', u'/{c4x}/{prefix}_unlock.png', 1), + (u'', u'/static/{prefix}_lock.png', u'/{c4x}/{prefix}_lock.png', 1), + (u'', u'/static/weird {prefix}_unlock.png', u'/{c4x}/weird_{prefix}_unlock.png', 1), + (u'dev', u'/static/{prefix}_unlock.png', u'//dev/{c4x}/{prefix}_unlock.png', 1), + (u'dev', u'/static/{prefix}_lock.png', u'/{c4x}/{prefix}_lock.png', 1), + (u'dev', u'/static/weird {prefix}_unlock.png', u'//dev/{c4x}/weird_{prefix}_unlock.png', 1), + # Static path with subdirectory. This ensures we properly substitute slashes. + (u'', u'/static/special/{prefix}_unlock.png', u'/{c4x}/special_{prefix}_unlock.png', 1), + (u'', u'/static/special/{prefix}_lock.png', u'/{c4x}/special_{prefix}_lock.png', 1), + (u'dev', u'/static/special/{prefix}_unlock.png', u'//dev/{c4x}/special_{prefix}_unlock.png', 1), + (u'dev', u'/static/special/{prefix}_lock.png', u'/{c4x}/special_{prefix}_lock.png', 1), + # Static path with query parameter. + ( + u'', + u'/static/{prefix}_unlock.png?foo=/static/{prefix}_lock.png', + u'/{c4x}/{prefix}_unlock.png?foo={encoded_c4x}{prefix}_lock.png', + 2 + ), + ( + u'', + u'/static/{prefix}_lock.png?foo=/static/{prefix}_unlock.png', + u'/{c4x}/{prefix}_lock.png?foo={encoded_c4x}{prefix}_unlock.png', + 2 + ), + ( + u'dev', + u'/static/{prefix}_unlock.png?foo=/static/{prefix}_lock.png', + u'//dev/{c4x}/{prefix}_unlock.png?foo={encoded_c4x}{prefix}_lock.png', + 2 + ), + ( + u'dev', + u'/static/{prefix}_lock.png?foo=/static/{prefix}_unlock.png', + u'/{c4x}/{prefix}_lock.png?foo={encoded_base_url}{encoded_c4x}{prefix}_unlock.png', + 2 + ), + # Old, c4x-style path. + (u'', u'/{c4x}/{prefix}_unlock.png', u'/{c4x}/{prefix}_unlock.png', 1), + (u'', u'/{c4x}/{prefix}_lock.png', u'/{c4x}/{prefix}_lock.png', 1), + (u'', u'/{c4x}/weird_{prefix}_unlock.png', u'/{c4x}/weird_{prefix}_unlock.png', 1), + (u'dev', u'/{c4x}/{prefix}_unlock.png', u'//dev/{c4x}/{prefix}_unlock.png', 1), + (u'dev', u'/{c4x}/{prefix}_lock.png', u'/{c4x}/{prefix}_lock.png', 1), + (u'dev', u'/{c4x}/weird_{prefix}_unlock.png', u'//dev/{c4x}/weird_{prefix}_unlock.png', 1), + ) + @ddt.unpack + def test_canonical_asset_path_with_c4x_style_assets(self, base_url, start, expected, mongo_calls): + prefix = 'old' + c4x_block = 'c4x/a/b/asset' + encoded_c4x_block = quote_plus('/' + c4x_block + '/') + encoded_base_url = quote_plus('//' + base_url) + + start = start.format( + prefix=prefix, + encoded_base_url=encoded_base_url, + c4x=c4x_block, + encoded_c4x=encoded_c4x_block + ) + expected = expected.format( + prefix=prefix, + encoded_base_url=encoded_base_url, + c4x=c4x_block, + encoded_c4x=encoded_c4x_block + ) + + with check_mongo_calls(mongo_calls): + asset_path = StaticContent.get_canonicalized_asset_path(self.courses[prefix].id, start, base_url) + self.assertEqual(asset_path, expected) diff --git a/common/lib/xmodule/xmodule/contentstore/content.py b/common/lib/xmodule/xmodule/contentstore/content.py index 9337b176e8..ddf92eea35 100644 --- a/common/lib/xmodule/xmodule/contentstore/content.py +++ b/common/lib/xmodule/xmodule/contentstore/content.py @@ -1,5 +1,8 @@ import re import uuid + +from xmodule.assetstore.assetmgr import AssetManager + XASSET_LOCATION_TAG = 'c4x' XASSET_SRCREF_PREFIX = 'xasset:' @@ -16,6 +19,8 @@ from urllib import urlencode from opaque_keys.edx.locator import AssetLocator from opaque_keys.edx.keys import CourseKey, AssetKey from opaque_keys import InvalidKeyError +from xmodule.modulestore.exceptions import ItemNotFoundError +from xmodule.exceptions import NotFoundError from PIL import Image @@ -137,32 +142,79 @@ class StaticContent(object): return AssetKey.from_string(path[1:]) @staticmethod - def convert_legacy_static_url_with_course_id(path, course_id): + def get_asset_key_from_path(course_key, path): """ - Returns a path to a piece of static content when we are provided with a filepath and - a course_id - """ - # Generate url of urlparse.path component - scheme, netloc, orig_path, params, query, fragment = urlparse(path) - loc = StaticContent.compute_location(course_id, orig_path) - loc_url = StaticContent.serialize_asset_key_with_slash(loc) + Parses a path, extracting an asset key or creating one. - # parse the query params for "^/static/" and replace with the location url - orig_query = parse_qsl(query) - new_query_list = [] - for query_name, query_value in orig_query: + Args: + course_key: key to the course which owns this asset + path: the path to said content + + Returns: + AssetKey: the asset key that represents the path + """ + + # Clean up the path, removing any static prefix and any leading slash. + if path.startswith('/static/'): + path = path[len('/static/'):] + + path = path.lstrip('/') + + try: + return AssetKey.from_string(path) + except InvalidKeyError: + # If we couldn't parse the path, just let compute_location figure it out. + # It's most likely a path like /image.png or something. + return StaticContent.compute_location(course_key, path) + + @staticmethod + def get_canonicalized_asset_path(course_key, path, base_url): + """ + Returns a fully-qualified path to a piece of static content. + + If a static asset CDN is configured, this path will include it. + Otherwise, the path will simply be relative. + + Args: + course_key: key to the course which owns this asset + path: the path to said content + + Returns: + string: fully-qualified path to asset + """ + + # Break down the input path. + _, _, relative_path, params, query_string, fragment = urlparse(path) + + # Convert our path to an asset key if it isn't one already. + asset_key = StaticContent.get_asset_key_from_path(course_key, relative_path) + + # Check the status of the asset to see if this can be served via CDN aka publicly. + serve_from_cdn = False + try: + content = AssetManager.find(asset_key, as_stream=True) + is_locked = getattr(content, "locked", True) + serve_from_cdn = not is_locked + except (ItemNotFoundError, NotFoundError): + # If we can't find the item, just treat it as if it's locked. + serve_from_cdn = False + + # Update any query parameter values that have asset paths in them. This is for assets that + # require their own after-the-fact values, like a Flash file that needs the path of a config + # file passed to it e.g. /static/visualization.swf?configFile=/static/visualization.xml + query_params = parse_qsl(query_string) + updated_query_params = [] + for query_name, query_value in query_params: if query_value.startswith("/static/"): - new_query = StaticContent.compute_location( - course_id, - query_value[len('/static/'):], - ) - new_query_url = StaticContent.serialize_asset_key_with_slash(new_query) - new_query_list.append((query_name, new_query_url)) + new_query_value = StaticContent.get_canonicalized_asset_path(course_key, query_value, base_url) + updated_query_params.append((query_name, new_query_value)) else: - new_query_list.append((query_name, query_value)) + updated_query_params.append((query_name, query_value)) - # Reconstruct with new path - return urlunparse((scheme, netloc, loc_url, params, urlencode(new_query_list), fragment)) + serialized_asset_key = StaticContent.serialize_asset_key_with_slash(asset_key) + base_url = base_url if serve_from_cdn else '' + + return urlunparse((None, base_url, serialized_asset_key, params, urlencode(updated_query_params), fragment)) def stream_data(self): yield self._data diff --git a/common/lib/xmodule/xmodule/tests/test_content.py b/common/lib/xmodule/xmodule/tests/test_content.py index 637bb86d23..4d151e20e3 100644 --- a/common/lib/xmodule/xmodule/tests/test_content.py +++ b/common/lib/xmodule/xmodule/tests/test_content.py @@ -4,6 +4,7 @@ import os import unittest import ddt from path import Path as path + from xmodule.contentstore.content import StaticContent, StaticContentStream from xmodule.contentstore.content import ContentStore from opaque_keys.edx.locations import SlashSeparatedCourseKey, AssetLocation @@ -94,11 +95,6 @@ class ContentTest(unittest.TestCase): content = StaticContent('loc', 'name', 'content_type', 'data') self.assertIsNone(content.thumbnail_location) - def test_static_url_generation_from_courseid(self): - course_key = SlashSeparatedCourseKey('foo', 'bar', 'bz') - url = StaticContent.convert_legacy_static_url_with_course_id('images_course_image.jpg', course_key) - self.assertEqual(url, '/c4x/foo/bar/asset/images_course_image.jpg') - @ddt.data( (u"monsters__.jpg", u"monsters__.jpg"), (u"monsters__.png", u"monsters__-png.jpg"), @@ -122,9 +118,9 @@ class ContentTest(unittest.TestCase): self.assertEqual(AssetLocation(u'mitX', u'400', u'ignore', u'asset', u'subs__1eo_jXvZnE_.srt.sjson', None), asset_location) def test_get_location_from_path(self): - asset_location = StaticContent.get_location_from_path(u'/c4x/foo/bar/asset/images_course_image.jpg') + asset_location = StaticContent.get_location_from_path(u'/c4x/a/b/asset/images_course_image.jpg') self.assertEqual( - AssetLocation(u'foo', u'bar', None, u'asset', u'images_course_image.jpg', None), + AssetLocation(u'a', u'b', None, u'asset', u'images_course_image.jpg', None), asset_location ) diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index 91e96a58d3..e5c2f1a953 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -1104,7 +1104,7 @@ class TestHtmlModifiers(ModuleStoreTestCase): result_fragment = module.render(STUDENT_VIEW) self.assertIn( - '/c4x/{org}/{course}/asset/_file.jpg'.format( + '/c4x/{org}/{course}/asset/file.jpg'.format( org=self.course.location.org, course=self.course.location.course, ),