From 2dd3f42e257981f2052613331bbc9e6dc497bde0 Mon Sep 17 00:00:00 2001 From: shredding Date: Fri, 23 Oct 2015 13:11:44 +0200 Subject: [PATCH] catch errors on non existing images --- CHANGELOG.txt | 1 + docs/releases/1.3.rst | 1 + wagtail/wagtailimages/blocks.py | 3 +- wagtail/wagtailimages/formats.py | 12 +--- wagtail/wagtailimages/jinja2tags.py | 15 +---- wagtail/wagtailimages/shortcuts.py | 22 +++++++ .../templatetags/wagtailimages_tags.py | 16 +----- wagtail/wagtailimages/tests/test_blocks.py | 57 +++++++++++++++++++ wagtail/wagtailimages/tests/test_jinja2.py | 20 +++++++ wagtail/wagtailimages/tests/test_shortcuts.py | 22 +++++++ 10 files changed, 132 insertions(+), 37 deletions(-) create mode 100644 wagtail/wagtailimages/shortcuts.py create mode 100644 wagtail/wagtailimages/tests/test_blocks.py create mode 100644 wagtail/wagtailimages/tests/test_shortcuts.py diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 8c5d03e86c..943acd5db3 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -38,6 +38,7 @@ Changelog * Fix: `MenuItem` `url` parameter can now take a lazy URL (Adon Metcalfe, rayrayndwiga) * Fix: Added missing translation tag to InlinePanel 'Add' button (jnns) * Fix: Restored correct highlighting behaviour of rich text toolbar buttons + * Fix: Rendering a missing image through ImageChooserBlock no longer breaks the whole page (Christian Peters) 1.2 (12.11.2015) ~~~~~~~~~~~~~~~~ diff --git a/docs/releases/1.3.rst b/docs/releases/1.3.rst index 4a1bcb5baa..ee19dd7bd2 100644 --- a/docs/releases/1.3.rst +++ b/docs/releases/1.3.rst @@ -88,6 +88,7 @@ Bug fixes * ``MenuItem`` ``url`` parameter can now take a lazy URL (Adon Metcalfe, rayrayndwiga) * Added missing translation tag to InlinePanel 'Add' button (jnns) * Restored correct highlighting behaviour of rich text toolbar buttons + * Rendering a missing image through ImageChooserBlock no longer breaks the whole page (Christian Peters) Upgrade considerations diff --git a/wagtail/wagtailimages/blocks.py b/wagtail/wagtailimages/blocks.py index b76cbac853..b87e8abc10 100644 --- a/wagtail/wagtailimages/blocks.py +++ b/wagtail/wagtailimages/blocks.py @@ -1,6 +1,7 @@ from django.utils.functional import cached_property from wagtail.wagtailcore.blocks import ChooserBlock +from .shortcuts import get_rendition_or_not_found class ImageChooserBlock(ChooserBlock): @@ -16,6 +17,6 @@ class ImageChooserBlock(ChooserBlock): def render_basic(self, value): if value: - return value.get_rendition('original').img_tag() + return get_rendition_or_not_found(value, 'original').img_tag() else: return '' diff --git a/wagtail/wagtailimages/formats.py b/wagtail/wagtailimages/formats.py index 1ad99e1188..0e968d182c 100644 --- a/wagtail/wagtailimages/formats.py +++ b/wagtail/wagtailimages/formats.py @@ -1,7 +1,7 @@ from django.utils.html import escape from wagtail.utils.apps import get_app_submodules -from wagtail.wagtailimages.models import SourceImageIOError +from .shortcuts import get_rendition_or_not_found class Format(object): @@ -26,15 +26,7 @@ class Format(object): ) def image_to_html(self, image, alt_text, extra_attributes=''): - try: - rendition = image.get_rendition(self.filter_spec) - except SourceImageIOError: - # Image file is (probably) missing from /media/original_images - generate a dummy - # rendition so that we just output a broken image, rather than crashing out completely - # during rendering - Rendition = image.renditions.model # pick up any custom Image / Rendition classes that may be in use - rendition = Rendition(image=image, width=0, height=0) - rendition.file.name = 'not-found' + rendition = get_rendition_or_not_found(image, self.filter_spec) if self.classnames: class_attr = 'class="%s" ' % escape(self.classnames) diff --git a/wagtail/wagtailimages/jinja2tags.py b/wagtail/wagtailimages/jinja2tags.py index 9a132e88af..c16ba4ed9e 100644 --- a/wagtail/wagtailimages/jinja2tags.py +++ b/wagtail/wagtailimages/jinja2tags.py @@ -2,25 +2,14 @@ from __future__ import absolute_import from jinja2.ext import Extension -from wagtail.wagtailimages.models import SourceImageIOError +from .shortcuts import get_rendition_or_not_found def image(image, filterspec, **attrs): if not image: return '' - try: - rendition = image.get_rendition(filterspec) - except SourceImageIOError: - # It's fairly routine for people to pull down remote databases to their - # local dev versions without retrieving the corresponding image files. - # In such a case, we would get a SourceImageIOError at the point where we try to - # create the resized version of a non-existent image. Since this is a - # bit catastrophic for a missing image, we'll substitute a dummy - # Rendition object so that we just output a broken link instead. - Rendition = image.renditions.model # pick up any custom Image / Rendition classes that may be in use - rendition = Rendition(image=image, width=0, height=0) - rendition.file.name = 'not-found' + rendition = get_rendition_or_not_found(image, filterspec) if attrs: return rendition.img_tag(attrs) diff --git a/wagtail/wagtailimages/shortcuts.py b/wagtail/wagtailimages/shortcuts.py new file mode 100644 index 0000000000..f78f05dde9 --- /dev/null +++ b/wagtail/wagtailimages/shortcuts.py @@ -0,0 +1,22 @@ +# coding=utf-8 +from wagtail.wagtailimages.models import SourceImageIOError + + +def get_rendition_or_not_found(image, specs): + """ + Tries to get / create the rendition for the image or renders a not-found image if it does not exist. + + :param image: AbstractImage + :param specs: str or Filter + :return: Rendition + """ + try: + return image.get_rendition(specs) + except SourceImageIOError: + # Image file is (probably) missing from /media/original_images - generate a dummy + # rendition so that we just output a broken image, rather than crashing out completely + # during rendering. + Rendition = image.renditions.model # pick up any custom Image / Rendition classes that may be in use + rendition = Rendition(image=image, width=0, height=0) + rendition.file.name = 'not-found' + return rendition diff --git a/wagtail/wagtailimages/templatetags/wagtailimages_tags.py b/wagtail/wagtailimages/templatetags/wagtailimages_tags.py index a932b5058a..86c401b5eb 100644 --- a/wagtail/wagtailimages/templatetags/wagtailimages_tags.py +++ b/wagtail/wagtailimages/templatetags/wagtailimages_tags.py @@ -1,7 +1,8 @@ from django import template from django.utils.functional import cached_property -from wagtail.wagtailimages.models import Filter, SourceImageIOError +from wagtail.wagtailimages.models import Filter +from wagtail.wagtailimages.shortcuts import get_rendition_or_not_found register = template.Library() @@ -54,18 +55,7 @@ class ImageNode(template.Node): if not image: return '' - try: - rendition = image.get_rendition(self.filter) - except SourceImageIOError: - # It's fairly routine for people to pull down remote databases to their - # local dev versions without retrieving the corresponding image files. - # In such a case, we would get a SourceImageIOError at the point where we try to - # create the resized version of a non-existent image. Since this is a - # bit catastrophic for a missing image, we'll substitute a dummy - # Rendition object so that we just output a broken link instead. - Rendition = image.renditions.model # pick up any custom Image / Rendition classes that may be in use - rendition = Rendition(image=image, width=0, height=0) - rendition.file.name = 'not-found' + rendition = get_rendition_or_not_found(image, self.filter) if self.output_var_name: # return the rendition object in the given variable diff --git a/wagtail/wagtailimages/tests/test_blocks.py b/wagtail/wagtailimages/tests/test_blocks.py new file mode 100644 index 0000000000..785536b680 --- /dev/null +++ b/wagtail/wagtailimages/tests/test_blocks.py @@ -0,0 +1,57 @@ +# -*- coding: utf-8 -* +from __future__ import unicode_literals + +import os + +from django.test import TestCase +from django.core import serializers +from django.conf import settings + +from wagtail.wagtailimages.blocks import ImageChooserBlock + +from .utils import get_test_image_file, Image + + +class TestImageChooserBlock(TestCase): + def setUp(self): + self.image = Image.objects.create( + title="Test image", + file=get_test_image_file(), + ) + + # Create an image with a missing file, by deserializing fom a python object + # (which bypasses FileField's attempt to read the file) + self.bad_image = list(serializers.deserialize('python', [{ + 'fields': { + 'title': 'missing image', + 'height': 100, + 'file': 'original_images/missing-image.jpg', + 'width': 100, + }, + 'model': 'wagtailimages.image' + }]))[0].object + self.bad_image.save() + + def get_image_filename(self, image, filterspec): + """ + Get the generated filename for a resized image + """ + name, ext = os.path.splitext(os.path.basename(image.file.name)) + return '{}images/{}.{}{}'.format( + settings.MEDIA_URL, name, filterspec, ext) + + def test_render(self): + block = ImageChooserBlock() + html = block.render(self.image) + expected_html = 'Test image'.format( + self.get_image_filename(self.image, "original") + ) + + self.assertHTMLEqual(html, expected_html) + + def test_render_missing(self): + block = ImageChooserBlock() + html = block.render(self.bad_image) + expected_html = 'missing image' + + self.assertHTMLEqual(html, expected_html) diff --git a/wagtail/wagtailimages/tests/test_jinja2.py b/wagtail/wagtailimages/tests/test_jinja2.py index fcddb20e02..139a11ba0c 100644 --- a/wagtail/wagtailimages/tests/test_jinja2.py +++ b/wagtail/wagtailimages/tests/test_jinja2.py @@ -5,6 +5,7 @@ import unittest import django from django.conf import settings +from django.core import serializers from django.test import TestCase from wagtail.wagtailcore.models import Site @@ -25,6 +26,19 @@ class TestImagesJinja(TestCase): file=get_test_image_file(), ) + # Create an image with a missing file, by deserializing fom a python object + # (which bypasses FileField's attempt to read the file) + self.bad_image = list(serializers.deserialize('python', [{ + 'fields': { + 'title': 'missing image', + 'height': 100, + 'file': 'original_images/missing-image.jpg', + 'width': 100, + }, + 'model': 'wagtailimages.image' + }]))[0].object + self.bad_image.save() + def render(self, string, context=None, request_context=True): if context is None: context = {} @@ -64,3 +78,9 @@ class TestImagesJinja(TestCase): 'width: {{ background.width }}, url: {{ background.url }}') output = ('width: 200, url: ' + self.get_image_filename(self.image, "width-200")) self.assertHTMLEqual(self.render(template, {'myimage': self.image}), output) + + def test_missing_image(self): + self.assertHTMLEqual( + self.render('{{ image(myimage, "width-200") }}', {'myimage': self.bad_image}), + 'missing image' + ) diff --git a/wagtail/wagtailimages/tests/test_shortcuts.py b/wagtail/wagtailimages/tests/test_shortcuts.py new file mode 100644 index 0000000000..0de864a9d1 --- /dev/null +++ b/wagtail/wagtailimages/tests/test_shortcuts.py @@ -0,0 +1,22 @@ +# coding=utf-8 +from django.test import TestCase +from wagtail.wagtailimages.shortcuts import get_rendition_or_not_found +from .utils import Image, get_test_image_file + + +class TestShortcuts(TestCase): + + fixtures = ['test.json'] + + def test_fallback_to_not_found(self): + bad_image = Image.objects.get(id=1) + good_image = Image.objects.create( + title="Test image", + file=get_test_image_file(), + ) + + rendition = get_rendition_or_not_found(good_image, 'width-400') + self.assertEqual(rendition.width, 400) + + rendition = get_rendition_or_not_found(bad_image, 'width-400') + self.assertEqual(rendition.file.name, 'not-found')