[PERF-224] Support to serve static assets from a CDN.

A base URL can now be configured which is, potentially, prepended to an
asset URL.  This allows a CDN, or caching server, to front static asset
requests, taking load off of the contentstore and speeding up page load
times.

Asset URL generation respects locked vs unlocked assets, and will not
generate links to locked assets that would traverse a CDN (even though
the authorization component of the contentserver middleware wouldn't
allow those links to work anyways).

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):
         """
-        Returns a path to a piece of static content when we are provided with a filepath and
-        a course_id
+        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
         """
-        # 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:
+
+        # 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))
+
+        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,
             ),