Failing tests for #1397

Testing the following with an external image backend:
 - Uploading through image chooser (fails)
 - Visiting the edit page of an image with a null file_size (fails)
 - Editing an image, changing the image file
 - Adding a new image

The latter two do not fail, I thought I'd add them to reduce risk of regressions in the future

wagtail/tests/dummy_external_storage.py

@@ -0,0 +1,72 @@
+# This file contains a file storage backend that imitates behaviours of
+# common external storage backends (S3 boto, libcloud, etc).
+
+# The following behaviours have been added to this backend:
+#  - Calling .path on the storage or image file raises NotImplementedError
+#  - File.open() after the file has been closed raises an error
+
+from django.core.files.storage import Storage, FileSystemStorage
+from django.core.files.base import File
+from django.utils.deconstruct import deconstructible
+
+
+@deconstructible
+class DummyExternalStorage(Storage):
+    def __init__(self, *args, **kwargs):
+        self.wrapped = FileSystemStorage(*args, **kwargs)
+
+    def path(self, name):
+        # Overridden to give it the behaviour of the base Storage class
+        # This is what an external storage backend would have
+        raise NotImplementedError("This backend doesn't support absolute paths.")
+
+    def _open(self, name, mode='rb'):
+        # Overridden to return a DummyExternalStorageFile instead of a normal
+        # File object
+        return DummyExternalStorageFile(open(self.wrapped.path(name), mode))
+
+
+    # Wrap all other functions
+
+    def _save(self, name, content):
+        return self.wrapped._save(name, content)
+
+    def delete(self, name):
+        self.wrapped.delete(name)
+
+    def exists(self, name):
+        return self.wrapped.exists(name)
+
+    def listdir(self, path):
+        return self.wrapped.listdir(path)
+
+    def size(self, name):
+        return self.wrapped.size(name)
+
+    def url(self, name):
+        return self.wrapped.url(name)
+
+    def accessed_time(self, name):
+        return self.wrapped.accessed_time(name)
+
+    def created_time(self, name):
+        return self.wrapped.created_time(name)
+
+    def modified_time(self, name):
+        return self.wrapped.modified_time(name)
+
+
+class DummyExternalStorageFile(File):
+    def open(self, mode=None):
+        # Based on: https://github.com/django/django/blob/2c39f282b8389f47fee4b24e785a58567c6c3629/django/core/files/base.py#L135-L141
+
+        # I've commented out two lines of this function which stops it checking
+        # the filesystem for the file. Making it behave as if it is using an
+        # external file storage.
+
+        if not self.closed:
+            self.seek(0)
+        # elif self.name and os.path.exists(self.name):
+        #    self.file = open(self.name, mode or self.mode)
+        else:
+            raise ValueError("The file cannot be reopened.")

wagtail/wagtailimages/tests/test_admin_views.py

@@ -1,6 +1,7 @@
 from __future__ import unicode_literals
 
 import json
+import unittest
 
 from django.test import TestCase, override_settings
 from django.utils.http import urlquote
@@ -89,6 +90,19 @@ class TestImageAddView(TestCase, WagtailTestUtils):
         # Test that the file_size field was set
         self.assertTrue(image.file_size)
 
+    @override_settings(DEFAULT_FILE_STORAGE='wagtail.tests.dummy_external_storage.DummyExternalStorage')
+    def test_add_with_external_file_storage(self):
+        response = self.post({
+            'title': "Test image",
+            'file': SimpleUploadedFile('test.png', get_test_image_file().file.getvalue()),
+        })
+
+        # Should redirect back to index
+        self.assertRedirects(response, reverse('wagtailimages:index'))
+
+        # Check that the image was created
+        self.assertTrue(Image.objects.filter(title="Test image").exists())
+
     def test_add_no_file_selected(self):
         response = self.post({
             'title': "Test image",
@@ -142,6 +156,20 @@ class TestImageEditView(TestCase, WagtailTestUtils):
         self.assertEqual(response.status_code, 200)
         self.assertTemplateUsed(response, 'wagtailimages/images/edit.html')
 
+    @unittest.expectedFailure
+    @override_settings(DEFAULT_FILE_STORAGE='wagtail.tests.dummy_external_storage.DummyExternalStorage')
+    def test_simple_with_external_storage(self):
+        # The view calls get_file_size on the image that closes the file if
+        # file_size wasn't prevously populated.
+
+        # The view then attempts to reopen the file when rendering the template
+        # which caused crashes when certian storage backends were in use.
+        # See #1397
+
+        response = self.get()
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtailimages/images/edit.html')
+
     def test_edit(self):
         response = self.post({
             'title': "Edited",
@@ -173,6 +201,26 @@ class TestImageEditView(TestCase, WagtailTestUtils):
         image = Image.objects.get(id=self.image.id)
         self.assertNotEqual(image.file_size, 100000)
 
+    @override_settings(DEFAULT_FILE_STORAGE='wagtail.tests.dummy_external_storage.DummyExternalStorage')
+    def test_edit_with_new_image_file_and_external_storage(self):
+        file_content = get_test_image_file().file.getvalue()
+
+        # Change the file size of the image
+        self.image.file_size = 100000
+        self.image.save()
+
+        response = self.post({
+            'title': "Edited",
+            'file': SimpleUploadedFile('new.png', file_content),
+        })
+
+        # Should redirect back to index
+        self.assertRedirects(response, reverse('wagtailimages:index'))
+
+        # Check that the image file size changed (assume it changed to the correct value)
+        image = Image.objects.get(id=self.image.id)
+        self.assertNotEqual(image.file_size, 100000)
+
     def test_with_missing_image_file(self):
         self.image.file.delete(False)
 
@@ -304,6 +352,20 @@ class TestImageChooserUploadView(TestCase, WagtailTestUtils):
         # The form should have an error
         self.assertFormError(response, 'uploadform', 'file', "This field is required.")
 
+    @unittest.expectedFailure
+    @override_settings(DEFAULT_FILE_STORAGE='wagtail.tests.dummy_external_storage.DummyExternalStorage')
+    def test_upload_with_external_storage(self):
+        response = self.client.post(reverse('wagtailimages:chooser_upload'), {
+            'title': "Test image",
+            'file': SimpleUploadedFile('test.png', get_test_image_file().file.getvalue()),
+        })
+
+        # Check response
+        self.assertEqual(response.status_code, 200)
+
+        # Check that the image was created
+        self.assertTrue(Image.objects.filter(title="Test image").exists())
+
 
 class TestMultipleImageUploader(TestCase, WagtailTestUtils):
     """