Smart, semantic exception handling for image resizing. refs #3379

Author: whatisjasongoldstein

ckeditor/fields.py

@@ -1,8 +1,10 @@
 from django.db import models
 from django import forms
+from django.core.exceptions import ValidationError
+from ckeditor.utils.image_resize import ImageResizeError
 
-import ckeditor.signals as signals
 from ckeditor.widgets import CKEditorWidget
+from ckeditor.utils.image_resize import resize_images
 
 
 class RichTextField(models.TextField):
@@ -11,6 +13,13 @@ def __init__(self, *args, **kwargs):
         self.dynamic_resize = kwargs.pop("dynamic_resize",False)
         super(RichTextField, self).__init__(*args, **kwargs)
 
+    def validate(self, value, model_instance):
+        super(RichTextField, self).validate(value, model_instance)
+        try:
+            resize_images(value)
+        except ImageResizeError as e:
+            raise ValidationError(unicode(e))
+
     def formfield(self, **kwargs):
         defaults = {
             'form_class': RichTextFormField,
@@ -19,10 +28,6 @@ def formfield(self, **kwargs):
         defaults.update(kwargs)
         return super(RichTextField, self).formfield(**defaults)
 
-    def contribute_to_class(self, cls, name):
-        super(RichTextField, self).contribute_to_class(cls, name)
-        if self.dynamic_resize:
-            signals.add_dynamic_resize(cls, name)
 
 class RichTextFormField(forms.fields.Field):
     def __init__(self, config_name='default', *args, **kwargs):

ckeditor/signals.py

@@ -1,9 +1,9 @@
-import uuid
 import os
-import shutil
+import sys
 import urlparse
 import re
 import hashlib
+import logging
 
 from lxml import html
 from PIL import Image, ImageFile
@@ -16,6 +16,11 @@
 
 ImageFile.MAXBLOCKS = 10000000
 
+logger = logging.getLogger(__name__)
+
+class ImageResizeError(Exception):
+    pass
+
 
 def match_or_none(string, rx):
     """
@@ -174,6 +179,9 @@ def transcode_to_jpeg(image, path, width, height):
     new_image.save(new_path, quality=80, optimize=1)
     return new_path
 
+def _get_resize_error_message(path):
+    return 'There was a problem resizing this image: {0}'.format(os.path.split(path)[1])
+
 def re_render(path, width, height):
     """
     Given an original image, width, and height, creates a thumbnailed image
@@ -193,6 +201,7 @@ def re_render(path, width, height):
     @return: Path to the 'rendered' image.
     @rtype:  "/path/to/image"
     """
+
     try:
         image = Image.open(path)
     except IOError:
@@ -230,15 +239,21 @@ def re_render(path, width, height):
         return new_path
 
     # Re-render the image, optimizing for filesize
-    new_image = image.resize((width, height), Image.ANTIALIAS)
+    try:
+        new_image = image.resize((width, height), Image.ANTIALIAS)
+    except TypeError:
+        raise ImageResizeError(_get_resize_error_message(path)), None, sys.exc_info()[2]
 
     image_params = {}
     if image.format != "GIF":
         image_params = dict(
             quality = 80,
             optimize = 1,
         )
-    new_image.save(new_path, **image_params)
+    try:
+        new_image.save(new_path, **image_params)
+    except IOError:
+        raise ImageResizeError(_get_resize_error_message(path)), None, sys.exc_info()[2]
     return new_path
 
 def get_html_tree(content):
@@ -258,17 +273,27 @@ def resize_images(post_content):
     @return: Modified contents.
     @rtype:  basestring
     """
+
     # Get tree
     tree = get_html_tree(post_content)
-
     # Get images
     imgs = tree.xpath('//img[starts-with(@src, "%s")]' % settings.STATIC_URL)
     for img in imgs:
         orig_url = img.attrib['src']
         orig_path = get_local_path(orig_url)
 
         width, height = get_dimensions(img)
-        rendered_path = re_render(orig_path, width, height)
+        try:
+            rendered_path = re_render(orig_path, width, height)
+        except ImageResizeError:
+            raise
+        except Exception as e:
+            # If something goes wrong, just use the original path so as not to
+            # interrupt the user. However, log it, because it's still a real problem.
+            logger.error(e, exc_info=True, extra={
+                'stack': True,
+            })
+            rendered_path = orig_path
 
         # If we haven't changed the image, move along.
         if rendered_path == orig_path: