schema change to better normalize asset->thumbnail relationships

cms/djangoapps/contentstore/views.py

@@ -635,23 +635,8 @@ def upload_asset(request, org, course, coursename):
     mime_type = request.FILES['file'].content_type
     filedata = request.FILES['file'].read()
 
-    file_location = StaticContent.compute_location(org, course, name)
-
-    content = StaticContent(file_location, name, mime_type, filedata)
-
-    # first commit to the DB
-    contentstore().save(content)
-
-    # then remove the cache so we're not serving up stale content
-    # NOTE: we're not re-populating the cache here as the DB owns the last-modified timestamp
-    # which is used when serving up static content. This integrity is needed for
-    # browser-side caching support. We *could* re-fetch the saved content so that we have the
-    # timestamp populated, but we might as well wait for the first real request to come in
-    # to re-populate the cache.
-    del_cached_content(content.location)
+    thumbnail_file_location = None
 
-    # 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 mime_type.split('/')[0] == 'image':
         try:
             # not sure if this is necessary, but let's rewind the stream just in case
@@ -673,11 +658,11 @@ def upload_asset(request, org, course, coursename):
             thumbnail_file.seek(0)
         
             # use a naming convention to associate originals with the thumbnail
-            thumbnail_name = content.generate_thumbnail_name()
+            thumbnail_name = StaticContent.generate_thumbnail_name(name)
 
             # then just store this thumbnail as any other piece of content
             thumbnail_file_location = StaticContent.compute_location(org, course, 
-                                                                              thumbnail_name)
+                                                                              thumbnail_name, is_thumbnail=True)
             thumbnail_content = StaticContent(thumbnail_file_location, thumbnail_name, 
                                               'image/jpeg', thumbnail_file)
             contentstore().save(thumbnail_content)
@@ -685,9 +670,37 @@ def upload_asset(request, org, course, coursename):
             # remove any cached content at this location, as thumbnails are treated just like any
             # other bit of static content
             del_cached_content(thumbnail_content.location)
+
+            # not sure if this is necessary, but let's rewind the stream just in case
+            request.FILES['file'].seek(0)
         except:
             # catch, log, and continue as thumbnails are not a hard requirement
             logging.error('Failed to generate thumbnail for {0}. Continuing...'.format(name))
+            thumbnail_file_location = None
+            raise
+
+
+    file_location = StaticContent.compute_location(org, course, name)
+
+    # if we're uploading an asset for which we can generate a thumbnail, let's generate it first so that we have
+    # the location to point to
+
+    content = StaticContent(file_location, name, mime_type, filedata, thumbnail_location = thumbnail_file_location)
+
+    # first commit to the DB
+    contentstore().save(content)
+
+    # then remove the cache so we're not serving up stale content
+    # NOTE: we're not re-populating the cache here as the DB owns the last-modified timestamp
+    # which is used when serving up static content. This integrity is needed for
+    # browser-side caching support. We *could* re-fetch the saved content so that we have the
+    # timestamp populated, but we might as well wait for the first real request to come in
+    # to re-populate the cache.
+    del_cached_content(content.location)
+
+    # 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
+ 
 
 
     return HttpResponse('Upload completed')
@@ -816,6 +829,7 @@ def asset_index(request, org, course, name):
     
     course_reference = StaticContent.compute_location(org, course, name)
     assets = contentstore().get_all_content_for_course(course_reference)
+    thumbnails = contentstore().get_all_content_thumbnails_for_course(course_reference)
     asset_display = []
     for asset in assets:
         id = asset['_id']
@@ -826,11 +840,11 @@ def asset_index(request, org, course, name):
         asset_location = StaticContent.compute_location(id['org'], id['course'], id['name'])
         display_info['url'] = StaticContent.get_url_path_from_location(asset_location)
         
-        thumbnail_name = contentstore().find(asset_location).generate_thumbnail_name()
-        thumbnail_location = StaticContent.compute_location(id['org'], id['course'], thumbnail_name)
+        # note, due to the schema change we may not have a 'thumbnail_location' in the result set
+        thumbnail_location = Location(asset.get('thumbnail_location', None))
+
         display_info['thumb_url'] = StaticContent.get_url_path_from_location(thumbnail_location)
         
-
         asset_display.append(display_info)
 
     return render_to_response('asset_index.html', {

common/lib/xmodule/xmodule/contentstore/content.py

 XASSET_LOCATION_TAG = 'c4x'
 XASSET_SRCREF_PREFIX = 'xasset:'
 
-XASSET_THUMBNAIL_TAIL_NAME = '.thumbnail.jpg'
+XASSET_THUMBNAIL_TAIL_NAME = '.jpg'
 
 import os
 import logging
 from xmodule.modulestore import Location
 
 class StaticContent(object):
-    def __init__(self, loc, name, content_type, data, last_modified_at=None):
+    def __init__(self, loc, name, content_type, data, last_modified_at=None, thumbnail_location=None):
         self.location = loc
         self.name = name #a display string which can be edited, and thus not part of the location which needs to be fixed
         self.content_type = content_type
         self.data = data
         self.last_modified_at = last_modified_at
+        self.thumbnail_location = thumbnail_location
 
     @property
     def is_thumbnail(self):
-        return self.name.endswith(XASSET_THUMBNAIL_TAIL_NAME)
+        return self.location.category == 'thumbnail'
 
-    def generate_thumbnail_name(self):
-        return ('{0}'+XASSET_THUMBNAIL_TAIL_NAME).format(os.path.splitext(self.name)[0])
+    @staticmethod
+    def generate_thumbnail_name(original_name):
+        return ('{0}'+XASSET_THUMBNAIL_TAIL_NAME).format(os.path.splitext(original_name)[0])
 
     @staticmethod
-    def compute_location(org, course, name, revision=None):
-        return Location([XASSET_LOCATION_TAG, org, course, 'asset', Location.clean(name), revision])
+    def compute_location(org, course, name, revision=None, is_thumbnail=False):
+        return Location([XASSET_LOCATION_TAG, org, course, 'asset' if not is_thumbnail else 'thumbnail', Location.clean(name), revision])
 
     def get_id(self):
         return StaticContent.get_id_from_location(self.location)
@@ -34,7 +36,10 @@ class StaticContent(object):
     
     @staticmethod
     def get_url_path_from_location(location):
-        return "/{tag}/{org}/{course}/{category}/{name}".format(**location.dict())
+        if location is not None:
+            return "/{tag}/{org}/{course}/{category}/{name}".format(**location.dict())
+        else:
+            return None
 
     @staticmethod
     def get_id_from_location(location):

common/lib/xmodule/xmodule/contentstore/mongo.py

@@ -28,7 +28,9 @@ class MongoContentStore(ContentStore):
         if self.fs.exists({"_id" : id}):
             self.fs.delete(id)
 
-        with self.fs.new_file(_id = id, filename=content.get_url_path(), content_type=content.content_type, displayname=content.name) as fp:
+        with self.fs.new_file(_id = id, filename=content.get_url_path(), content_type=content.content_type, 
+            displayname=content.name, thumbnail_location=content.thumbnail_location) as fp:
+
             fp.write(content.data)
         
         return content
@@ -38,11 +40,18 @@ class MongoContentStore(ContentStore):
         id = StaticContent.get_id_from_location(location)
         try:
             with self.fs.get(id) as fp:
-                return StaticContent(location, fp.displayname, fp.content_type, fp.read(), fp.uploadDate)
+                return StaticContent(location, fp.displayname, fp.content_type, fp.read(), 
+                    fp.uploadDate, thumbnail_location = fp.thumbnail_location if 'thumbnail_location' in fp else None)
         except NoFile:
             raise NotFoundError()
 
+    def get_all_content_thumbnails_for_course(self, location):
+        return self._get_all_content_for_course(location, get_thumbnails = True)
+
     def get_all_content_for_course(self, location):
+        return self._get_all_content_for_course(location, get_thumbnails = False)
+
+    def _get_all_content_for_course(self, location, get_thumbnails = False):
         '''
         Returns a list of all static assets for a course. The return format is a list of dictionary elements. Example:
 
@@ -62,7 +71,8 @@ class MongoContentStore(ContentStore):
 
             ]
         '''
-        course_filter = Location(XASSET_LOCATION_TAG, category="asset",course=location.course,org=location.org)
+        course_filter = Location(XASSET_LOCATION_TAG, category="asset" if not get_thumbnails else "thumbnail",
+            course=location.course,org=location.org)
         # 'borrow' the function 'location_to_query' from the Mongo modulestore implementation
         items = self.fs_files.find(location_to_query(course_filter))
         return list(items)