diff --git a/common/lib/xmodule/xmodule/contentstore/content.py b/common/lib/xmodule/xmodule/contentstore/content.py index be33401bc8..9dc4b1367b 100644 --- a/common/lib/xmodule/xmodule/contentstore/content.py +++ b/common/lib/xmodule/xmodule/contentstore/content.py @@ -35,7 +35,8 @@ class StaticContent(object): @staticmethod def compute_location(org, course, name, revision=None, is_thumbnail=False): name = name.replace('/', '_') - return Location([XASSET_LOCATION_TAG, org, course, 'asset' if not is_thumbnail else 'thumbnail', Location.clean(name), revision]) + return Location([XASSET_LOCATION_TAG, org, course, 'asset' if not is_thumbnail else 'thumbnail', + Location.clean_keeping_underscores(name), revision]) def get_id(self): return StaticContent.get_id_from_location(self.location) diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index 0ba7e36540..525527c93f 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -71,6 +71,17 @@ class Location(_LocationBase): """ return Location._clean(value, INVALID_CHARS) + + @staticmethod + def clean_keeping_underscores(value): + """ + Return value, replacing INVALID_CHARS, but not collapsing multiple '_' chars. + This for cleaning asset names, as the YouTube ID's may have underscores in them, and we need the + transcript asset name to match. In the future we may want to change the behavior of _clean. + """ + return INVALID_CHARS.sub('_', value) + + @staticmethod def clean_for_url_name(value): """ diff --git a/common/lib/xmodule/xmodule/tests/test_content.py b/common/lib/xmodule/xmodule/tests/test_content.py index 1bcd2f4ebe..e73c33197c 100644 --- a/common/lib/xmodule/xmodule/tests/test_content.py +++ b/common/lib/xmodule/xmodule/tests/test_content.py @@ -19,9 +19,14 @@ class ContentTest(unittest.TestCase): content = StaticContent('loc', 'name', 'content_type', 'data') self.assertIsNone(content.thumbnail_location) - def test_generate_thumbnail_nonimage(self): + def test_generate_thumbnail_image(self): contentStore = ContentStore() - content = Content(Location(u'c4x', u'mitX', u'800', u'asset', u'monsters.jpg'), None) + content = Content(Location(u'c4x', u'mitX', u'800', u'asset', u'monsters__.jpg'), None) (thumbnail_content, thumbnail_file_location) = contentStore.generate_thumbnail(content) self.assertIsNone(thumbnail_content) - self.assertEqual(Location(u'c4x', u'mitX', u'800', u'thumbnail', u'monsters.jpg'), thumbnail_file_location) + self.assertEqual(Location(u'c4x', u'mitX', u'800', u'thumbnail', u'monsters__.jpg'), thumbnail_file_location) + def test_compute_location(self): + # We had a bug that __ got converted into a single _. Make sure that substitution of INVALID_CHARS (like space) + # still happen. + asset_location = StaticContent.compute_location('mitX', '400', 'subs__1eo_jXvZnE .srt.sjson') + self.assertEqual(Location(u'c4x', u'mitX', u'400', u'asset', u'subs__1eo_jXvZnE_.srt.sjson', None), asset_location)