diff --git a/cms/djangoapps/contentstore/views/assets.py b/cms/djangoapps/contentstore/views/assets.py index fd77774dae..ceef41bda2 100644 --- a/cms/djangoapps/contentstore/views/assets.py +++ b/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 diff --git a/common/djangoapps/cache_toolbox/core.py b/common/djangoapps/cache_toolbox/core.py index a0a512ca68..421940321b 100644 --- a/common/djangoapps/cache_toolbox/core.py +++ b/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 diff --git a/common/djangoapps/contentserver/middleware.py b/common/djangoapps/contentserver/middleware.py index 7606c324fb..e711fd2b66 100644 --- a/common/djangoapps/contentserver/middleware.py +++ b/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): diff --git a/common/lib/xmodule/xmodule/contentstore/content.py b/common/lib/xmodule/xmodule/contentstore/content.py index a6c092d900..f096290bc7 100644 --- a/common/lib/xmodule/xmodule/contentstore/content.py +++ b/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): diff --git a/common/lib/xmodule/xmodule/contentstore/mongo.py b/common/lib/xmodule/xmodule/contentstore/mongo.py index cf8e5e03b5..91eebdbdcc 100644 --- a/common/lib/xmodule/xmodule/contentstore/mongo.py +++ b/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): diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_contentstore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_contentstore.py index 1ce5265a17..cda981e329 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_contentstore.py +++ b/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)