LMS-11179 Fix Assets with Split

cms/djangoapps/contentstore/views/assets.py

@@ -277,7 +277,7 @@ def _get_asset_json(display_name, date, location, thumbnail_location, locked):
     """
     Helper method for formatting the asset information to send to client.
     """
-    asset_url = location.to_deprecated_string()
+    asset_url = _add_slash(location.to_deprecated_string())
     external_url = settings.LMS_BASE + asset_url
     return {
         'display_name': display_name,
@@ -285,8 +285,14 @@ def _get_asset_json(display_name, date, location, thumbnail_location, locked):
         'url': asset_url,
         'external_url': external_url,
         'portable_url': StaticContent.get_static_path_from_location(location),
-        'thumbnail': thumbnail_location.to_deprecated_string() if thumbnail_location is not None else None,
+        'thumbnail': _add_slash(unicode(thumbnail_location)) if thumbnail_location else None,
         'locked': locked,
         # Needed for Backbone delete/update.
         'id': unicode(location)
     }
+
+
+def _add_slash(url):
+    if not url.startswith('/'):
+        url = '/' + url  # TODO NAATODO - is there a better way to do this?
+    return url

common/djangoapps/cache_toolbox/core.py

@@ -10,6 +10,7 @@ Core methods
 
 from django.core.cache import cache
 from django.db import DEFAULT_DB_ALIAS
+from opaque_keys import InvalidKeyError
 
 from . import app_settings
 
@@ -121,6 +122,10 @@ def del_cached_content(location):
     # delete content for the given location, as well as for content with run=None.
     # it's possible that the content could have been cached without knowing the
     # course_key - and so without having the run.
-    cache.delete_many(
-        [unicode(loc).encode("utf-8") for loc in [location, location.replace(run=None)]]
-    )
+    try:
+        cache.delete_many(
+            [unicode(loc).encode("utf-8") for loc in [location, location.replace(run=None)]]
+        )
+    except InvalidKeyError:
+        # although deprecated keys allowed run=None, new keys don't if there is no version.
+        pass

common/djangoapps/contentserver/middleware.py

@@ -6,6 +6,7 @@ from xmodule.contentstore.django import contentstore
 from xmodule.contentstore.content import StaticContent, XASSET_LOCATION_TAG
 from xmodule.modulestore import InvalidLocationError
 from opaque_keys import InvalidKeyError
+from opaque_keys.edx.locator import AssetLocator
 from cache_toolbox.core import get_cached_content, set_cached_content
 from xmodule.exceptions import NotFoundError
 
@@ -14,8 +15,11 @@ from xmodule.exceptions import NotFoundError
 
 class StaticContentServer(object):
     def process_request(self, request):
-        # look to see if the request is prefixed with 'c4x' tag
-        if request.path.startswith('/' + XASSET_LOCATION_TAG + '/'):
+        # look to see if the request is prefixed with an asset prefix tag
+        if (
+            request.path.startswith('/' + XASSET_LOCATION_TAG + '/') or
+            request.path.startswith('/' + AssetLocator.CANONICAL_NAMESPACE)
+        ):
             try:
                 loc = StaticContent.get_location_from_path(request.path)
             except (InvalidLocationError, InvalidKeyError):

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

@@ -10,8 +10,9 @@ import StringIO
 from urlparse import urlparse, urlunparse, parse_qsl
 from urllib import urlencode
 
-from opaque_keys.edx.locations import AssetLocation
-from opaque_keys.edx.keys import CourseKey
+from opaque_keys.edx.locator import AssetLocator
+from opaque_keys.edx.keys import CourseKey, AssetKey
+from opaque_keys import InvalidKeyError
 from PIL import Image
 
 
@@ -52,12 +53,10 @@ class StaticContent(object):
             asset
         """
         path = path.replace('/', '_')
-        return AssetLocation(
-            course_key.org, course_key.course, course_key.run,
+        return course_key.make_asset_key(
             'asset' if not is_thumbnail else 'thumbnail',
-            AssetLocation.clean_keeping_underscores(path),
-            revision
-        )
+            AssetLocator.clean_keeping_underscores(path)
+        ).for_branch(None)
 
     def get_id(self):
         return self.location
@@ -104,16 +103,21 @@ class StaticContent(object):
             return None
 
         assert(isinstance(course_key, CourseKey))
-        return course_key.make_asset_key('asset', '').to_deprecated_string()
+        # create a dummy asset location and then strip off the last character: 'a'
+        return course_key.make_asset_key('asset', 'a').for_branch(None).to_deprecated_string()[:-1]
 
     @staticmethod
     def get_location_from_path(path):
         """
         Generate an AssetKey for the given path (old c4x/org/course/asset/name syntax)
         """
-        # TODO OpaqueKeys after opaque keys deprecation is working
-        # return AssetLocation.from_string(path)
-        return AssetLocation.from_deprecated_string(path)
+        try:
+            return AssetKey.from_string(path)
+        except InvalidKeyError:
+            # TODO - re-address this once LMS-11198 is tackled.
+            if path.startswith('/'):
+                # try stripping off the leading slash and try again
+                return AssetKey.from_string(path[1:])
 
     @staticmethod
     def convert_legacy_static_url_with_course_id(path, course_id):

common/lib/xmodule/xmodule/contentstore/mongo.py

@@ -94,7 +94,9 @@ class MongoContentStore(ContentStore):
                 fp = self.fs.get(content_id)
                 thumbnail_location = getattr(fp, 'thumbnail_location', None)
                 if thumbnail_location:
-                    thumbnail_location = location.course_key.make_asset_key('thumbnail', thumbnail_location[4])
+                    thumbnail_location = location.course_key.make_asset_key(
+                        'thumbnail', thumbnail_location[4]
+                    )
                 return StaticContentStream(
                     location, fp.displayname, fp.content_type, fp, last_modified_at=fp.uploadDate,
                     thumbnail_location=thumbnail_location,
@@ -105,7 +107,9 @@ class MongoContentStore(ContentStore):
                 with self.fs.get(content_id) as fp:
                     thumbnail_location = getattr(fp, 'thumbnail_location', None)
                     if thumbnail_location:
-                        thumbnail_location = location.course_key.make_asset_key('thumbnail', thumbnail_location[4])
+                        thumbnail_location = location.course_key.make_asset_key(
+                            'thumbnail', thumbnail_location[4]
+                        )
                     return StaticContent(
                         location, fp.displayname, fp.content_type, fp.read(), last_modified_at=fp.uploadDate,
                         thumbnail_location=thumbnail_location,
@@ -304,7 +308,9 @@ class MongoContentStore(ContentStore):
                 asset_id = asset_key
             else:  # add the run, since it's the last field, we're golden
                 asset_key['run'] = dest_course_key.run
-                asset_id = unicode(dest_course_key.make_asset_key(asset_key['category'], asset_key['name']))
+                asset_id = unicode(
+                    dest_course_key.make_asset_key(asset_key['category'], asset_key['name']).for_branch(None)
+                )
 
             self.fs.put(
                 source_content.read(),
@@ -347,7 +353,7 @@ class MongoContentStore(ContentStore):
             # NOTE, there's no need to state that run doesn't exist in the negative case b/c access via
             # SON requires equivalence (same keys and values in exact same order)
             dbkey['run'] = location.run
-            content_id = unicode(location)
+            content_id = unicode(location.for_branch(None))
         return content_id, dbkey
 
     def make_id_son(self, fs_entry):

common/lib/xmodule/xmodule/modulestore/tests/test_contentstore.py

@@ -9,7 +9,8 @@ from tempfile import mkdtemp
 import path
 import shutil
 
-from opaque_keys.edx.locations import SlashSeparatedCourseKey, AssetLocation
+from opaque_keys.edx.locator import CourseLocator, AssetLocator
+from opaque_keys.edx.keys import AssetKey
 from xmodule.tests import DATA_DIR
 from xmodule.contentstore.mongo import MongoContentStore
 from xmodule.contentstore.content import StaticContent
@@ -41,13 +42,13 @@ class TestContentstore(unittest.TestCase):
         Restores deprecated values
         """
         if cls.asset_deprecated is not None:
-            setattr(AssetLocation, 'deprecated', cls.asset_deprecated)
+            setattr(AssetLocator, 'deprecated', cls.asset_deprecated)
         else:
-            delattr(AssetLocation, 'deprecated')
+            delattr(AssetLocator, 'deprecated')
         if cls.ssck_deprecated is not None:
-            setattr(SlashSeparatedCourseKey, 'deprecated', cls.ssck_deprecated)
+            setattr(CourseLocator, 'deprecated', cls.ssck_deprecated)
         else:
-            delattr(SlashSeparatedCourseKey, 'deprecated')
+            delattr(CourseLocator, 'deprecated')
         return super(TestContentstore, cls).tearDownClass()
 
     def set_up_assets(self, deprecated):
@@ -59,11 +60,11 @@ class TestContentstore(unittest.TestCase):
         self.contentstore = MongoContentStore(HOST, DB, port=PORT)
         self.addCleanup(self.contentstore._drop_database)  # pylint: disable=protected-access
 
-        setattr(AssetLocation, 'deprecated', deprecated)
-        setattr(SlashSeparatedCourseKey, 'deprecated', deprecated)
+        setattr(AssetLocator, 'deprecated', deprecated)
+        setattr(CourseLocator, 'deprecated', deprecated)
 
-        self.course1_key = SlashSeparatedCourseKey('test', 'asset_test', '2014_07')
-        self.course2_key = SlashSeparatedCourseKey('test', 'asset_test2', '2014_07')
+        self.course1_key = CourseLocator('test', 'asset_test', '2014_07')
+        self.course2_key = CourseLocator('test', 'asset_test2', '2014_07')
 
         self.course1_files = ['contains.sh', 'picture1.jpg', 'picture2.jpg']
         self.course2_files = ['picture1.jpg', 'picture3.jpg', 'door_2.ogg']
@@ -154,13 +155,13 @@ class TestContentstore(unittest.TestCase):
         course1_assets, count = self.contentstore.get_all_content_for_course(self.course1_key)
         self.assertEqual(count, len(self.course1_files), course1_assets)
         for asset in course1_assets:
-            parsed = AssetLocation.from_deprecated_string(asset['filename'])
+            parsed = AssetKey.from_string(asset['filename'])
             self.assertIn(parsed.name, self.course1_files)
 
         course1_assets, __ = self.contentstore.get_all_content_for_course(self.course1_key, 1, 1)
         self.assertEqual(len(course1_assets), 1, course1_assets)
 
-        fake_course = SlashSeparatedCourseKey('test', 'fake', 'non')
+        fake_course = CourseLocator('test', 'fake', 'non')
         course_assets, count = self.contentstore.get_all_content_for_course(fake_course)
         self.assertEqual(count, 0)
         self.assertEqual(course_assets, [])
@@ -183,7 +184,7 @@ class TestContentstore(unittest.TestCase):
         copy_all_course_assets
         """
         self.set_up_assets(deprecated)
-        dest_course = SlashSeparatedCourseKey('test', 'destination', 'copy')
+        dest_course = CourseLocator('test', 'destination', 'copy')
         self.contentstore.copy_all_course_assets(self.course1_key, dest_course)
         for filename in self.course1_files:
             asset_key = self.course1_key.make_asset_key('asset', filename)