Put Filter back together

wagtail/wagtailimages/image_processor.py

@@ -1,23 +0,0 @@
-from wagtail.wagtailimages.backends import get_image_backend
-from wagtail.wagtailimages.utils import parse_filter_spec
-
-
-def process_image(input_file, output_file, filter_spec, backend_name='default'):
-    # Get the image backend
-    backend = get_image_backend(backend_name)
-
-    # Parse the filter spec
-    method_name, method_arg = parse_filter_spec(filter_spec)
-
-    # Load image
-    image = backend.open_image(input_file)
-    file_format = image.format
-
-    # Call method
-    method = getattr(backend, method_name)
-    image = method(image, method_arg)
-
-    # Save image
-    backend.save_image(image, output_file, file_format) 
-
-    return output_file

wagtail/wagtailimages/models.py

@@ -1,4 +1,5 @@
 import os.path
+import re
 
 from six import BytesIO
 
@@ -14,6 +15,7 @@ from django.utils.html import escape, format_html_join
 from django.conf import settings
 from django.utils.translation import ugettext_lazy  as _
 from django.utils.encoding import python_2_unicode_compatible
+from django.utils.functional import cached_property
 
 from unidecode import unidecode
 
@@ -21,7 +23,6 @@ from wagtail.wagtailadmin.taggable import TagSearchable
 from wagtail.wagtailimages.backends import get_image_backend
 from wagtail.wagtailsearch import indexed
 from wagtail.wagtailimages.utils import validate_image_format
-from wagtail.wagtailimages import image_processor
 
 
 @python_2_unicode_compatible
@@ -70,7 +71,16 @@ class AbstractImage(models.Model, TagSearchable):
             # If we have a backend attribute then pass it to process
             # image - else pass 'default'
             backend_name = getattr(self, 'backend', 'default')
-            generated_image_file = filter.process_image(file_field.file, backend_name=backend_name)
+            generated_image = filter.process_image(file_field.file, backend_name=backend_name)
+
+            # generate new filename derived from old one, inserting the filter spec string before the extension
+            input_filename_parts = os.path.basename(file_field.file.name).split('.')
+            filename_without_extension = '.'.join(input_filename_parts[:-1])
+            filename_without_extension = filename_without_extension[:60]  # trim filename base so that we're well under 100 chars
+            output_filename_parts = [filename_without_extension, filter.spec] + input_filename_parts[-1:]
+            output_filename = '.'.join(output_filename_parts)
+
+            generated_image_file = File(generated_image, name=output_filename)
 
             rendition, created = self.renditions.get_or_create(
                 filter=filter, defaults={'file': generated_image_file})
@@ -144,29 +154,84 @@ class Filter(models.Model):
     """
     spec = models.CharField(max_length=255, db_index=True)
 
-    def process_image(self, input_file, backend_name='default'):
+    OPERATION_NAMES = {
+        'max': 'resize_to_max',
+        'min': 'resize_to_min',
+        'width': 'resize_to_width',
+        'height': 'resize_to_height',
+        'fill': 'resize_to_fill',
+        'original': 'no_operation',
+    }
+
+    class InvalidFilterSpecError(ValueError):
+        pass
+
+    def _parse_spec_string(self):
+        # parse the spec string and return the method name and method arg.
+        # There are various possible formats to match against:
+        # 'original'
+        # 'width-200'
+        # 'max-320x200'
+
+        if self.spec == 'original':
+            return Filter.OPERATION_NAMES['original'], None
+
+        match = re.match(r'(width|height)-(\d+)$', self.spec)
+        if match:
+            return Filter.OPERATION_NAMES[match.group(1)], int(match.group(2))
+
+        match = re.match(r'(max|min|fill)-(\d+)x(\d+)$', self.spec)
+        if match:
+            width = int(match.group(2))
+            height = int(match.group(3))
+            return Filter.OPERATION_NAMES[match.group(1)], (width, height)
+
+        # Spec is not one of our recognised patterns
+        raise Filter.InvalidFilterSpecError("Invalid image filter spec: %r" % self.spec)
+
+    @cached_property
+    def _method(self):
+        return self._parse_spec_string()
+
+    def is_valid(self):
+        try:
+            self._parse_spec_string()
+            return True
+        except Filter.InvalidFilterSpecError:
+            return False
+
+    def process_image(self, input_file, output_file=None, backend_name='default'):
         """
         Given an input image file as a django.core.files.File object,
         generate an output image with this filter applied, returning it
         as another django.core.files.File object
         """
-        # If file is closed, open it
+        # Get backend
+        backend = get_image_backend(backend_name)
+
+        # Parse spec string
+        method_name, method_arg = self._method
+
+        # Open image
         input_file.open('rb')
+        image = backend.open_image(input_file)
+        file_format = image.format
 
-        # Process the image
+        # Process image
-        output = image_processor.process_image(input_file, BytesIO(), self.spec, backend_name=backend_name)
+        method = getattr(backend, method_name)
+        image = method(image, method_arg)
 
-        # and then close the input file
+        # Make sure we have an output file
+        if output_file is None:
+            output_file = BytesIO()
+
+        # Write output
+        backend.save_image(image, output_file, file_format)
+
+        # Close the input file
         input_file.close()
 
-        # generate new filename derived from old one, inserting the filter spec string before the extension
+        return output_file
-        input_filename_parts = os.path.basename(input_file.name).split('.')
-        filename_without_extension = '.'.join(input_filename_parts[:-1])
-        filename_without_extension = filename_without_extension[:60]  # trim filename base so that we're well under 100 chars
-        output_filename_parts = [filename_without_extension, self.spec] + input_filename_parts[-1:]
-        output_filename = '.'.join(output_filename_parts)
-
-        return File(output, name=output_filename)
 
 
 class AbstractRendition(models.Model):

wagtail/wagtailimages/tests.py

@@ -23,7 +23,7 @@ from wagtail.wagtailimages.formats import (
 
 from wagtail.wagtailimages.backends import get_image_backend
 from wagtail.wagtailimages.backends.pillow import PillowBackend
-from wagtail.wagtailimages.utils import parse_filter_spec, InvalidFilterSpecError, generate_signature, verify_signature
+from wagtail.wagtailimages.utils import generate_signature, verify_signature
 
 
 def get_test_image_file():
@@ -480,31 +480,6 @@ class TestFormat(TestCase):
         self.assertEqual(result, self.format)
 
 
-class TestFilterSpecParsing(TestCase):
-    good = {
-        'original': ('no_operation', None),
-        'min-800x600': ('resize_to_min', (800, 600)),
-        'max-800x600': ('resize_to_max', (800, 600)),
-        'fill-800x600': ('resize_to_fill', (800, 600)),
-        'width-800': ('resize_to_width', 800),
-        'height-600': ('resize_to_height', 600),
-    }
-
-    bad = [
-        'original-800x600', # Shouldn't have parameters
-        'abcdefg', # Filter doesn't exist
-        'min', 'max', 'fill', 'width', 'height' , # Should have parameters
-    ]
-
-    def test_good(self):
-        for filter_spec, expected_result in self.good.items():
-            self.assertEqual(parse_filter_spec(filter_spec), expected_result)
-
-    def test_bad(self):
-        for filter_spec in self.bad:
-            self.assertRaises(InvalidFilterSpecError, parse_filter_spec, filter_spec)
-
-
 class TestSignatureGeneration(TestCase):
     def test_signature_generation(self):
         self.assertEqual(generate_signature(100, 'fill-800x600'), b'xnZOzQyUg6pkfciqcfRJRosOrGg=')

wagtail/wagtailimages/utils.py

@@ -34,47 +34,6 @@ def validate_image_format(f):
             raise ValidationError(_("Not a valid %s image. Please use a gif, jpeg or png file with the correct file extension.") % (extension.upper()))
 
 
-class InvalidFilterSpecError(RuntimeError):
-    pass
-
-
-# TODO: Cache results from this method in something like Python 3.2s LRU cache (available in Django 1.7 as django.utils.lru_cache)
-def parse_filter_spec(filter_spec):
-    # parse the spec string and save the results to
-    # self.method_name and self.method_arg. There are various possible
-    # formats to match against:
-    # 'original'
-    # 'width-200'
-    # 'max-320x200'
-
-    OPERATION_NAMES = {
-        'max': 'resize_to_max',
-        'min': 'resize_to_min',
-        'width': 'resize_to_width',
-        'height': 'resize_to_height',
-        'fill': 'resize_to_fill',
-        'original': 'no_operation',
-    }
-
-    # original
-    if filter_spec == 'original':
-        return OPERATION_NAMES['original'], None
-
-    # width/height
-    match = re.match(r'(width|height)-(\d+)$', filter_spec)
-    if match:
-        return OPERATION_NAMES[match.group(1)], int(match.group(2))
-
-    # max/min/fill
-    match = re.match(r'(max|min|fill)-(\d+)x(\d+)$', filter_spec)
-    if match:
-        width = int(match.group(2))
-        height = int(match.group(3))
-        return OPERATION_NAMES[match.group(1)], (width, height)
-
-    raise InvalidFilterSpecError(filter_spec)
-
-
 def generate_signature(image_id, filter_spec):
     # Based on libthumbor hmac generation
     # https://github.com/thumbor/libthumbor/blob/b19dc58cf84787e08c8e397ab322e86268bb4345/libthumbor/crypto.py#L50

wagtail/wagtailimages/views/frontend.py

@@ -3,9 +3,8 @@ from django.http import HttpResponse
 from django.core.exceptions import PermissionDenied
 from django.views.decorators.cache import cache_page
 
-from wagtail.wagtailimages.models import get_image_model
+from wagtail.wagtailimages.models import get_image_model, Filter
-from wagtail.wagtailimages.utils import InvalidFilterSpecError, verify_signature
+from wagtail.wagtailimages.utils import verify_signature
-from wagtail.wagtailimages import image_processor
 
 
 @cache_page(60 * 60 * 24 * 60) # Cache for 60 days
@@ -16,6 +15,6 @@ def serve(request, signature, image_id, filter_spec):
         raise PermissionDenied
 
     try:
-        return image_processor.process_image(image.file.file, HttpResponse(content_type='image/jpeg'), filter_spec)
+        return Filter(spec=filter_spec).process_image(image.file.file, HttpResponse(content_type='image/jpeg'))
-    except InvalidFilterSpecError:
+    except Filter.InvalidFilterSpecError:
         return HttpResponse("Invalid filter spec: " + filter_spec, content_type='text/plain', status=400)

wagtail/wagtailimages/views/images.py

@@ -13,9 +13,9 @@ from django.http import HttpResponse
 from wagtail.wagtailcore.models import Site
 from wagtail.wagtailadmin.forms import SearchForm
 
-from wagtail.wagtailimages.models import get_image_model
+from wagtail.wagtailimages.models import get_image_model, Filter
 from wagtail.wagtailimages.forms import get_image_form, URLGeneratorForm
-from wagtail.wagtailimages.utils import parse_filter_spec, InvalidFilterSpecError, generate_signature
+from wagtail.wagtailimages.utils import generate_signature
 
 
 @permission_required('wagtailimages.add_image')
@@ -159,9 +159,7 @@ def generate_url(request, image_id, filter_spec):
         }, status=403)
 
     # Parse the filter spec to make sure its valid
-    try:
+    if not Filter(spec=filter_spec).is_valid():
-        parse_filter_spec(filter_spec)
-    except InvalidFilterSpecError:
         return json_response({
             'error': "Invalid filter spec."
         }, status=400)