From ff0c6db3223339f98893907be219c7b635f92943 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Thu, 13 Nov 2014 15:15:25 +0000 Subject: [PATCH 1/4] Add unit test for rendering image tag with a missing image --- wagtail/tests/fixtures/test.json | 14 +++++++++++++- wagtail/tests/templates/tests/event_page.html | 5 ++++- wagtail/wagtailimages/tests/tests.py | 14 ++++++++++++++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/wagtail/tests/fixtures/test.json b/wagtail/tests/fixtures/test.json index d16affe480..b2e0d6f4f5 100644 --- a/wagtail/tests/fixtures/test.json +++ b/wagtail/tests/fixtures/test.json @@ -81,7 +81,8 @@ "audience": "public", "location": "The North Pole", "body": "

Chestnuts roasting on an open fire

", - "cost": "Free" + "cost": "Free", + "feed_image": 1 } }, @@ -625,5 +626,16 @@ "page": 11, "password": "swordfish" } +}, +{ + "pk": 1, + "model": "wagtailimages.image", + "fields": { + "title": "A missing image", + "file": "original_images/missing.jpg", + "width": 1000, + "height": 1000, + "created_at": "2014-01-01T12:00:00.000Z" + } } ] diff --git a/wagtail/tests/templates/tests/event_page.html b/wagtail/tests/templates/tests/event_page.html index 795b7bf0a7..fd8f9bd757 100644 --- a/wagtail/tests/templates/tests/event_page.html +++ b/wagtail/tests/templates/tests/event_page.html @@ -1,4 +1,4 @@ -{% load wagtailcore_tags %} +{% load wagtailcore_tags wagtailimages_tags %} @@ -8,6 +8,9 @@

{{ self.title }}

Event

+ {% if self.feed_image %} + {% image self.feed_image width-200 class="feed-image" %} + {% endif %}

Back to events index

diff --git a/wagtail/wagtailimages/tests/tests.py b/wagtail/wagtailimages/tests/tests.py index 1030a3b7ef..2c75b0611f 100644 --- a/wagtail/wagtailimages/tests/tests.py +++ b/wagtail/wagtailimages/tests/tests.py @@ -63,6 +63,20 @@ class TestImageTag(TestCase): self.assertTrue('title="my wonderful title"' in result) +class TestMissingImage(TestCase): + """ + Missing image files in media/original_images should be handled gracefully, to cope with + pulling live databases to a development instance without copying the corresponding image files. + In this case, it's acceptable to render broken images, but not to fail rendering the page outright. + """ + fixtures = ['test.json'] + + def test_image_tag_with_missing_image(self): + # the page /events/christmas/ has a missing image as the feed image + response = self.client.get('/events/christmas/') + self.assertContains(response, 'A missing image', html=True) + + class TestFormat(TestCase): def setUp(self): # test format From d4a9f14e415a9dd05335f7a8639851134530ea16 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Thu, 13 Nov 2014 15:25:06 +0000 Subject: [PATCH 2/4] add failing test for #801 --- wagtail/tests/fixtures/test.json | 2 +- wagtail/tests/templates/tests/event_page.html | 1 + wagtail/wagtailimages/tests/tests.py | 5 +++++ 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/wagtail/tests/fixtures/test.json b/wagtail/tests/fixtures/test.json index b2e0d6f4f5..de8b1e27e8 100644 --- a/wagtail/tests/fixtures/test.json +++ b/wagtail/tests/fixtures/test.json @@ -224,7 +224,7 @@ "date_from": "2015-04-22", "audience": "public", "location": "Ameristralia", - "body": "

come celebrate the independence of Ameristralia

", + "body": "

come celebrate the independence of Ameristralia

", "cost": "Free" } }, diff --git a/wagtail/tests/templates/tests/event_page.html b/wagtail/tests/templates/tests/event_page.html index fd8f9bd757..a3ccaa7181 100644 --- a/wagtail/tests/templates/tests/event_page.html +++ b/wagtail/tests/templates/tests/event_page.html @@ -11,6 +11,7 @@ {% if self.feed_image %} {% image self.feed_image width-200 class="feed-image" %} {% endif %} + {{ self.body|richtext }}

Back to events index

diff --git a/wagtail/wagtailimages/tests/tests.py b/wagtail/wagtailimages/tests/tests.py index 2c75b0611f..6df1dba56b 100644 --- a/wagtail/wagtailimages/tests/tests.py +++ b/wagtail/wagtailimages/tests/tests.py @@ -76,6 +76,11 @@ class TestMissingImage(TestCase): response = self.client.get('/events/christmas/') self.assertContains(response, 'A missing image', html=True) + def test_rich_text_with_missing_image(self): + # the page /events/final-event/ has a missing image in the rich text body + response = self.client.get('/events/final-event/') + self.assertContains(response, 'where did my image go?', html=True) + class TestFormat(TestCase): def setUp(self): From e3dab2af35d134c81272758159eccdca96fb1b66 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Thu, 13 Nov 2014 16:02:36 +0000 Subject: [PATCH 3/4] Define a custom exception type for IO errors thrown when opening source images, and catch that in InageNode.render so that we're not masking unrelated IOErrors elsewhere in the get_rendition process --- wagtail/wagtailimages/models.py | 19 +++++++++++++++++-- .../templatetags/wagtailimages_tags.py | 6 +++--- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/wagtail/wagtailimages/models.py b/wagtail/wagtailimages/models.py index 163dfd6767..9e19a03f46 100644 --- a/wagtail/wagtailimages/models.py +++ b/wagtail/wagtailimages/models.py @@ -1,7 +1,7 @@ import os.path import re -from six import BytesIO +from six import BytesIO, text_type from taggit.managers import TaggableManager @@ -28,6 +28,13 @@ from wagtail.wagtailimages.rect import Rect from wagtail.wagtailadmin.utils import get_object_usage +class SourceImageIOError(IOError): + """ + Custom exception to distinguish IOErrors that were thrown while opening the source image + """ + pass + + def get_upload_to(instance, filename): folder_name = 'original_images' filename = instance.file.field.storage.get_valid_name(filename) @@ -178,7 +185,15 @@ 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 = filter.process_image(file_field.file, backend_name=backend_name, focal_point=self.get_focal_point()) + + try: + image_file = file_field.file # triggers a call to self.storage.open, so IOErrors from missing files will be raised at this point + except IOError as e: + # re-throw this as a SourceImageIOError so that calling code can distinguish + # these from IOErrors elsewhere in the process + raise SourceImageIOError(text_type(e)) + + generated_image = filter.process_image(image_file, backend_name=backend_name, focal_point=self.get_focal_point()) # generate new filename derived from old one, inserting the filter spec and focal point key before the extension if self.has_focal_point(): diff --git a/wagtail/wagtailimages/templatetags/wagtailimages_tags.py b/wagtail/wagtailimages/templatetags/wagtailimages_tags.py index 5c72734177..c8f609c6b9 100644 --- a/wagtail/wagtailimages/templatetags/wagtailimages_tags.py +++ b/wagtail/wagtailimages/templatetags/wagtailimages_tags.py @@ -1,6 +1,6 @@ from django import template -from wagtail.wagtailimages.models import Filter +from wagtail.wagtailimages.models import Filter, SourceImageIOError register = template.Library() @@ -53,10 +53,10 @@ class ImageNode(template.Node): try: rendition = image.get_rendition(self.filter) - except IOError: + 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 an IOError at the point where we try to + # 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. From 222b548d9e57ebc14a0a73262dd2a135a5837615 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Thu, 13 Nov 2014 16:11:30 +0000 Subject: [PATCH 4/4] catch SourceImageIOError when rendering image formats - fixes #801 --- wagtail/wagtailimages/formats.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/wagtail/wagtailimages/formats.py b/wagtail/wagtailimages/formats.py index 3c5644a078..1ad99e1188 100644 --- a/wagtail/wagtailimages/formats.py +++ b/wagtail/wagtailimages/formats.py @@ -1,6 +1,7 @@ from django.utils.html import escape from wagtail.utils.apps import get_app_submodules +from wagtail.wagtailimages.models import SourceImageIOError class Format(object): @@ -25,7 +26,15 @@ class Format(object): ) def image_to_html(self, image, alt_text, extra_attributes=''): - rendition = image.get_rendition(self.filter_spec) + 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' if self.classnames: class_attr = 'class="%s" ' % escape(self.classnames)