diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index b3c28f726d..816ccab091 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -760,14 +760,16 @@ def upload_asset(request, org, course, coursename): content_loc = StaticContent.compute_location(org, course, filename) content = StaticContent(content_loc, filename, mime_type, filedata) - # first let's save a thumbnail so we can get back a thumbnail location - thumbnail_content = contentstore().generate_thumbnail(content) + # first let's see if a thumbnail can be created + (thumbnail_content, thumbnail_location) = contentstore().generate_thumbnail(content) + # delete cached thumbnail even if one couldn't be created this time (else the old thumbnail will continue to show) + del_cached_content(thumbnail_location) + # now store thumbnail location only if we could create it if thumbnail_content is not None: - content.thumbnail_location = thumbnail_content.location - del_cached_content(thumbnail_content.location) + content.thumbnail_location = thumbnail_location - #then commit the content + #then commit the content contentstore().save(content) del_cached_content(content.location) @@ -777,7 +779,7 @@ def upload_asset(request, org, course, coursename): response_payload = {'displayname' : content.name, 'uploadDate' : get_date_display(readback.last_modified_at), 'url' : StaticContent.get_url_path_from_location(content.location), - 'thumb_url' : StaticContent.get_url_path_from_location(thumbnail_content.location) if thumbnail_content is not None else None, + 'thumb_url' : StaticContent.get_url_path_from_location(thumbnail_location) if thumbnail_content is not None else None, 'msg' : 'Upload completed' } diff --git a/common/lib/xmodule/xmodule/contentstore/content.py b/common/lib/xmodule/xmodule/contentstore/content.py index b84aa01c54..5b10acc0ef 100644 --- a/common/lib/xmodule/xmodule/contentstore/content.py +++ b/common/lib/xmodule/xmodule/contentstore/content.py @@ -18,7 +18,7 @@ class StaticContent(object): self.content_type = content_type self.data = data self.last_modified_at = last_modified_at - self.thumbnail_location = Location(thumbnail_location) + self.thumbnail_location = Location(thumbnail_location) if thumbnail_location is not None else None # optional information about where this file was imported from. This is needed to support import/export # cycles self.import_path = import_path @@ -113,6 +113,12 @@ class ContentStore(object): def generate_thumbnail(self, content): thumbnail_content = None + # use a naming convention to associate originals with the thumbnail + thumbnail_name = StaticContent.generate_thumbnail_name(content.location.name) + + thumbnail_file_location = StaticContent.compute_location(content.location.org, content.location.course, + thumbnail_name, is_thumbnail = True) + # if we're uploading an image, then let's generate a thumbnail so that we can # serve it up when needed without having to rescale on the fly if content.content_type is not None and content.content_type.split('/')[0] == 'image': @@ -131,13 +137,8 @@ class ContentStore(object): thumbnail_file = StringIO.StringIO() im.save(thumbnail_file, 'JPEG') thumbnail_file.seek(0) - - # use a naming convention to associate originals with the thumbnail - thumbnail_name = StaticContent.generate_thumbnail_name(content.location.name) - # then just store this thumbnail as any other piece of content - thumbnail_file_location = StaticContent.compute_location(content.location.org, content.location.course, - thumbnail_name, is_thumbnail = True) + # store this thumbnail as any other piece of content thumbnail_content = StaticContent(thumbnail_file_location, thumbnail_name, 'image/jpeg', thumbnail_file) @@ -147,7 +148,7 @@ class ContentStore(object): # log and continue as thumbnails are generally considered as optional logging.exception("Failed to generate thumbnail for {0}. Exception: {1}".format(content.location, str(e))) - return thumbnail_content + return thumbnail_content, thumbnail_file_location diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index 2fd70f68cd..331ba786b3 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -35,10 +35,10 @@ def import_static_content(modules, course_loc, course_data_path, static_content_ content = StaticContent(content_loc, filename, mime_type, data, import_path = fullname_with_subpath) # first let's save a thumbnail so we can get back a thumbnail location - thumbnail_content = static_content_store.generate_thumbnail(content) + (thumbnail_content, thumbnail_location) = static_content_store.generate_thumbnail(content) if thumbnail_content is not None: - content.thumbnail_location = thumbnail_content.location + content.thumbnail_location = thumbnail_location #then commit the content static_content_store.save(content) @@ -69,10 +69,10 @@ def verify_content_links(module, base_dir, static_content_store, link, remap_dic content = StaticContent(content_loc, filename, mime_type, data, import_path = path) # first let's save a thumbnail so we can get back a thumbnail location - thumbnail_content = static_content_store.generate_thumbnail(content) + (thumbnail_content, thumbnail_location) = static_content_store.generate_thumbnail(content) if thumbnail_content is not None: - content.thumbnail_location = thumbnail_content.location + content.thumbnail_location = thumbnail_location #then commit the content static_content_store.save(content) diff --git a/common/lib/xmodule/xmodule/tests/test_content.py b/common/lib/xmodule/xmodule/tests/test_content.py new file mode 100644 index 0000000000..6bd10f22f7 --- /dev/null +++ b/common/lib/xmodule/xmodule/tests/test_content.py @@ -0,0 +1,25 @@ +import unittest +from xmodule.contentstore.content import StaticContent +from xmodule.contentstore.content import ContentStore +from xmodule.modulestore import Location + +class Content: + def __init__(self, location, content_type): + self.location = location + self.content_type = content_type + +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 + # all elements being None. It is important that the location be just None for rendering. + content = StaticContent('loc', 'name', 'content_type', 'data', None, None, None) + self.assertIsNone(content.thumbnail_location) + + content = StaticContent('loc', 'name', 'content_type', 'data') + self.assertIsNone(content.thumbnail_location) + def test_generate_thumbnail_nonimage(self): + contentStore = ContentStore() + 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) \ No newline at end of file