Avoid unnecessary file data overwrite when editing files (#513)

* Tweak file edit to avoid unnecessary file data saves

* Added tests for changes is FileUploadTempStore

kotti/tests/test_file.py

@@ -6,6 +6,7 @@
 import pytest
 from colander import null
 from mock import MagicMock
+from mock import patch
 from pyramid.httpexceptions import HTTPMovedPermanently
 
 from kotti.filedepot import StoredFileResponse
@@ -81,16 +82,18 @@ def test_edit_without_file(self, filedepot):
         view.context.filename = u'myfile.png'
         view.context.mimetype = u'image/png'
         view.context.size = 777
-        view.edit(title=u'A title',
-                  description=u'A description',
-                  tags=[],
-                  file=null)
-        assert view.context.title == u'A title'
-        assert view.context.description == u'A description'
-        assert view.context.data.file.read() == 'filecontents'
-        assert view.context.filename == u'myfile.png'
-        assert view.context.mimetype == u'image/png'
-        assert view.context.size == 777
+        with patch("kotti.views.edit.content._to_fieldstorage") as tfs:
+            view.edit(title=u'A title',
+                      description=u'A description',
+                      tags=[],
+                      file=null)
+            assert tfs.call_count == 0
+            assert view.context.title == u'A title'
+            assert view.context.description == u'A description'
+            assert view.context.data.file.read() == 'filecontents'
+            assert view.context.filename == u'myfile.png'
+            assert view.context.mimetype == u'image/png'
+            assert view.context.size == 777
 
 
 class TestFileAddForm:
@@ -139,6 +142,21 @@ def test_delitem(self):
         del tmpstore['important']
         assert 'important' not in tmpstore.session
 
+    def test_setitem_with_stream(self):
+        ts = self.make_one()
+        ts['a'] = {'fp': StringIO('test'), 'marker': 'yes'}
+        assert ts.session['a'] == {'file_contents': 'test', 'marker': 'yes'}
+        v = ts['a']
+        assert 'fp' in v.keys()
+        assert v['marker'] == 'yes'
+        assert v['fp'].read() == 'test'
+
+    def test_setitem_with_empty(self):
+        ts = self.make_one()
+        ts['a'] = {'fp': None, 'marker': 'yes'}
+        assert ts.session['a'] == {'file_contents': '', 'marker': 'yes'}
+        assert ts['a'] == {'fp': None, 'marker': 'yes'}
+
 
 class TestDepotStore:
 

kotti/views/edit/content.py

@@ -95,13 +95,16 @@ def before(self, form):
         form.appstruct = get_appstruct(self.context, self.schema)
         if self.context.data is not None:
             form.appstruct.update({'file': {
-                'fp': StringIO(self.context.data.file.read()),
-                'filename': self.context.name,
+                'fp': None,
+                'filename': self.context.data['filename'],  # self.context.name
                 'mimetype': self.context.mimetype,
                 'uid': str(random.randint(1000000000, 9999999999)),
             }})
 
     def schema_factory(self):
+        # File uploads are stored in the session so that you don't need
+        # to upload your file again if validation of another schema node
+        # fails.
         tmpstore = FileUploadTempStore(self.request)
         return FileSchema(tmpstore)
 
@@ -110,7 +113,7 @@ def edit(self, **appstruct):
         self.context.title = title
         self.context.description = appstruct['description']
         self.context.tags = appstruct['tags']
-        if appstruct['file']:
+        if appstruct['file'] and appstruct['file']['fp']:
             self.context.data = _to_fieldstorage(**appstruct['file'])
 
 

kotti/views/form.py

@@ -244,13 +244,20 @@ def keys(self):
     def __setitem__(self, name, value):
         value = value.copy()
         fp = value.pop('fp')
-        value['file_contents'] = fp.read()
-        fp.seek(0)
+        if fp is not None:
+            value['file_contents'] = fp.read()
+            fp.seek(0)
+        else:
+            value['file_contents'] = ''
         self.session[name] = value
 
     def __getitem__(self, name):
         value = self.session[name].copy()
-        value['fp'] = StringIO(value.pop('file_contents'))
+        content = value.pop('file_contents')
+        if content:
+            value['fp'] = StringIO(content)
+        else:
+            value['fp'] = None
         return value
 
     def __delitem__(self, name):
@@ -268,6 +275,8 @@ def validate_file_size_limit(node, value):
     You can configure the maximum size by setting the kotti.max_file_size
     option to the maximum number of bytes that you want to allow.
     """
+    if not value['fp']:
+        return
 
     value['fp'].seek(0, 2)
     size = value['fp'].tell()