Merge pull request #1273 from kaedroho/issue-1256

A couple of fixes to get_willow_image

wagtail/wagtailimages/models.py

@@ -88,20 +88,25 @@ class AbstractImage(models.Model, TagSearchable):
 
     @contextmanager
     def get_willow_image(self):
+        # Open file if it is closed
+        close_file = False
         try:
-            image_file = self.file.file  # triggers a call to self.storage.open, so IOErrors from missing files will be raised at this point
+            if self.file.closed:
+                self.file.open('rb')
+                close_file = True
         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))
 
-        image_file.open('rb')
-        image_file.seek(0)
+        # Seek to beginning
+        self.file.seek(0)
 
         try:
-            yield WillowImage.open(image_file)
+            yield WillowImage.open(self.file)
         finally:
-            image_file.close()
+            if close_file:
+                self.file.close()
 
     def get_rect(self):
         return Rect(0, 0, self.width, self.height)

wagtail/wagtailimages/tests/test_models.py

@@ -231,6 +231,35 @@ class TestGetWillowImage(TestCase):
             with bad_image.get_willow_image():
                 self.fail() # Shouldn't get here
 
+    def test_closes_image(self):
+        # This tests that willow closes images after use
+        with self.image.get_willow_image():
+            self.assertFalse(self.image.file.closed)
+
+        self.assertTrue(self.image.file.closed)
+
+    def test_closes_image_on_exception(self):
+        # This tests that willow closes images when the with is exited with an exception
+        try:
+            with self.image.get_willow_image():
+                self.assertFalse(self.image.file.closed)
+                raise ValueError("Something went wrong!")
+        except ValueError:
+            pass
+
+        self.assertTrue(self.image.file.closed)
+
+    def test_doesnt_close_open_image(self):
+        # This tests that when the image file is already open, get_willow_image doesn't close it (#1256)
+        self.image.file.open('rb')
+
+        with self.image.get_willow_image():
+            pass
+
+        self.assertFalse(self.image.file.closed)
+
+        self.image.file.close()
+
 
 class TestIssue573(TestCase):
     """