Merge pull request #11025 from edx/PERF-224

[PERF-224] Serve course assets from a CDN

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)

common/djangoapps/static_replace/admin.py

+"""
+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)

common/djangoapps/static_replace/models.py

+"""
+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 '<AssetBaseUrlConfig(base_url={})>'.format(self.get_base_url())
+
+    def __unicode__(self):
+        return unicode(repr(self))

common/djangoapps/static_replace/test/test_static_replace.py

+"""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)

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

 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):
+        """
+        Parses a path, extracting an asset key or creating one.
+
+        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 path to a piece of static content when we are provided with a filepath and
-        a course_id
+        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
         """
-        # 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)
-
-        # 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:
+
+        # 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))
+
+        serialized_asset_key = StaticContent.serialize_asset_key_with_slash(asset_key)
+        base_url = base_url if serve_from_cdn else ''
 
-        # Reconstruct with new path
-        return urlunparse((scheme, netloc, loc_url, params, urlencode(new_query_list), fragment))
+        return urlunparse((None, base_url, serialized_asset_key, params, urlencode(updated_query_params), fragment))
 
     def stream_data(self):
         yield self._data

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
         )
 

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