diff --git a/common/lib/xmodule/xmodule/contentstore/content.py b/common/lib/xmodule/xmodule/contentstore/content.py index 830d754deb..6652a4e5a6 100644 --- a/common/lib/xmodule/xmodule/contentstore/content.py +++ b/common/lib/xmodule/xmodule/contentstore/content.py @@ -40,8 +40,11 @@ class StaticContent(object): @staticmethod def generate_thumbnail_name(original_name): + name_root, ext = os.path.splitext(original_name) + if not ext == XASSET_THUMBNAIL_TAIL_NAME: + name_root = name_root + ext.replace(u'.', u'-') return u"{name_root}{extension}".format( - name_root=os.path.splitext(original_name)[0], + name_root=name_root, extension=XASSET_THUMBNAIL_TAIL_NAME,) @staticmethod diff --git a/common/lib/xmodule/xmodule/tests/test_content.py b/common/lib/xmodule/xmodule/tests/test_content.py index f4e97f092f..457a4a22b2 100644 --- a/common/lib/xmodule/xmodule/tests/test_content.py +++ b/common/lib/xmodule/xmodule/tests/test_content.py @@ -1,4 +1,5 @@ import unittest +import ddt from xmodule.contentstore.content import StaticContent, StaticContentStream from xmodule.contentstore.content import ContentStore from opaque_keys.edx.locations import SlashSeparatedCourseKey, AssetLocation @@ -73,6 +74,7 @@ class FakeGridFsItem: return chunk +@ddt.ddt class ContentTest(unittest.TestCase): def test_thumbnail_none(self): # We had a bug where a thumbnail location of None was getting transformed into a Location tuple, with @@ -88,12 +90,19 @@ class ContentTest(unittest.TestCase): 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') - def test_generate_thumbnail_image(self): + @ddt.data( + (u"monsters__.jpg", u"monsters__.jpg"), + (u"monsters__.png", u"monsters__-png.jpg"), + (u"dots.in.name.jpg", u"dots.in.name.jpg"), + (u"dots.in.name.png", u"dots.in.name-png.jpg"), + ) + @ddt.unpack + def test_generate_thumbnail_image(self, original_filename, thumbnail_filename): contentStore = ContentStore() - content = Content(AssetLocation(u'mitX', u'800', u'ignore_run', u'asset', u'monsters__.jpg'), None) + content = Content(AssetLocation(u'mitX', u'800', u'ignore_run', u'asset', original_filename), None) (thumbnail_content, thumbnail_file_location) = contentStore.generate_thumbnail(content) self.assertIsNone(thumbnail_content) - self.assertEqual(AssetLocation(u'mitX', u'800', u'ignore_run', u'thumbnail', u'monsters__.jpg'), thumbnail_file_location) + self.assertEqual(AssetLocation(u'mitX', u'800', u'ignore_run', u'thumbnail', thumbnail_filename), 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)