Merge pull request #1600 from MITx/bug/christina/misc

Don't collapse multiple underscores for asset names.

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)

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

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)