Fixed #11149 -- Don't call save_form_data on file-type fields multiple times when saving a model form.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@10826 bcc190cf-cafb-0310-a4f2-bffc1f526a37

3 files changed, 27 insertions, 5 deletions

diff --git a/django/forms/models.py b/django/forms/models.py
index aab870b6a0..a0b217860d 100644
--- a/django/forms/models.py
+++ b/django/forms/models.py
@@ -50,14 +50,14 @@ def save_instance(form, instance, fields=None, fail_message='saved',
             continue
         if exclude and f.name in exclude:
             continue
+        # OneToOneField doesn't allow assignment of None. Guard against that
+        # instead of allowing it and throwing an error.
+        if isinstance(f, models.OneToOneField) and cleaned_data[f.name] is None:
+            continue
         # Defer saving file-type fields until after the other fields, so a
         # callable upload_to can use the values from other fields.
         if isinstance(f, models.FileField):
             file_field_list.append(f)
-        # OneToOneField doesn't allow assignment of None. Guard against that
-        # instead of allowing it and throwing an error.
-        if isinstance(f, models.OneToOneField) and cleaned_data[f.name] is None:
-            pass
         else:
             f.save_form_data(instance, cleaned_data[f.name])
 
diff --git a/tests/regressiontests/model_forms_regress/models.py b/tests/regressiontests/model_forms_regress/models.py
index 488e2bca40..376586e94f 100644
--- a/tests/regressiontests/model_forms_regress/models.py
+++ b/tests/regressiontests/model_forms_regress/models.py
@@ -28,3 +28,12 @@ class Article(models.Model):
 
     def __unicode__(self):
         return self.headline
+
+class CustomFileField(models.FileField):
+    def save_form_data(self, instance, data):
+        been_here = getattr(self, 'been_saved', False)
+        assert not been_here, "save_form_data called more than once"
+        setattr(self, 'been_saved', True)
+
+class CustomFF(models.Model):
+    f = CustomFileField(upload_to='unused', blank=True)
diff --git a/tests/regressiontests/model_forms_regress/tests.py b/tests/regressiontests/model_forms_regress/tests.py
index ccc272eae6..85e284b639 100644
--- a/tests/regressiontests/model_forms_regress/tests.py
+++ b/tests/regressiontests/model_forms_regress/tests.py
@@ -6,7 +6,7 @@ from django.forms.models import modelform_factory
 from django.conf import settings
 from django.test import TestCase
 
-from models import Person, Triple, FilePathModel, Article, Publication
+from models import Person, Triple, FilePathModel, Article, Publication, CustomFF
 
 class ModelMultipleChoiceFieldTests(TestCase):
 
@@ -88,3 +88,16 @@ class ManyToManyCallableInitialTests(TestCase):
 <option value="2" selected="selected">Second Book</option>
 <option value="3">Third Book</option>
 </select>  Hold down "Control", or "Command" on a Mac, to select more than one.</li>""")
+
+class CFFForm(forms.ModelForm):
+    class Meta:
+        model = CustomFF
+
+class CustomFieldSaveTests(TestCase):
+    def test_save(self):
+        "Regression for #11149: save_form_data should be called only once"
+        
+        # It's enough that the form saves without error -- the custom save routine will
+        # generate an AssertionError if it is called more than once during save.
+        form = CFFForm(data = {'f': None})
+        form.save()
\ No newline at end of file