Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Objects with FileFields no longer get save() called multiple times fr…

…om the AutomaticManipulator! This fixes #639, #572, and likely others I don't know of.

This may be slightly backwards-incompatible: if you've been relying on the multiple-save behavior (why?), then you'll no longer see that happen.


git-svn-id: http://code.djangoproject.com/svn/django/trunk@4609 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit a30e3fca48be23c856cda778ec3f0e0eec75fd91 1 parent f313e07
Jacob Kaplan-Moss authored February 26, 2007
7  django/db/models/base.py
@@ -356,7 +356,7 @@ def _get_FIELD_url(self, field):
356 356
     def _get_FIELD_size(self, field):
357 357
         return os.path.getsize(self._get_FIELD_filename(field))
358 358
 
359  
-    def _save_FIELD_file(self, field, filename, raw_contents):
  359
+    def _save_FIELD_file(self, field, filename, raw_contents, save=True):
360 360
         directory = field.get_directory_name()
361 361
         try: # Create the date-based directory if it doesn't exist.
362 362
             os.makedirs(os.path.join(settings.MEDIA_ROOT, directory))
@@ -391,8 +391,9 @@ def _save_FIELD_file(self, field, filename, raw_contents):
391 391
             if field.height_field:
392 392
                 setattr(self, field.height_field, height)
393 393
 
394  
-        # Save the object, because it has changed.
395  
-        self.save()
  394
+        # Save the object because it has changed unless save is False
  395
+        if save:
  396
+            self.save()
396 397
 
397 398
     _save_FIELD_file.alters_data = True
398 399
 
14  django/db/models/fields/__init__.py
@@ -635,7 +635,7 @@ def contribute_to_class(self, cls, name):
635 635
         setattr(cls, 'get_%s_filename' % self.name, curry(cls._get_FIELD_filename, field=self))
636 636
         setattr(cls, 'get_%s_url' % self.name, curry(cls._get_FIELD_url, field=self))
637 637
         setattr(cls, 'get_%s_size' % self.name, curry(cls._get_FIELD_size, field=self))
638  
-        setattr(cls, 'save_%s_file' % self.name, lambda instance, filename, raw_contents: instance._save_FIELD_file(self, filename, raw_contents))
  638
+        setattr(cls, 'save_%s_file' % self.name, lambda instance, filename, raw_contents, save=True: instance._save_FIELD_file(self, filename, raw_contents, save))
639 639
         dispatcher.connect(self.delete_file, signal=signals.post_delete, sender=cls)
640 640
 
641 641
     def delete_file(self, instance):
@@ -653,14 +653,14 @@ def get_manipulator_field_objs(self):
653 653
     def get_manipulator_field_names(self, name_prefix):
654 654
         return [name_prefix + self.name + '_file', name_prefix + self.name]
655 655
 
656  
-    def save_file(self, new_data, new_object, original_object, change, rel):
  656
+    def save_file(self, new_data, new_object, original_object, change, rel, save=True):
657 657
         upload_field_name = self.get_manipulator_field_names('')[0]
658 658
         if new_data.get(upload_field_name, False):
659 659
             func = getattr(new_object, 'save_%s_file' % self.name)
660 660
             if rel:
661  
-                func(new_data[upload_field_name][0]["filename"], new_data[upload_field_name][0]["content"])
  661
+                func(new_data[upload_field_name][0]["filename"], new_data[upload_field_name][0]["content"], save)
662 662
             else:
663  
-                func(new_data[upload_field_name]["filename"], new_data[upload_field_name]["content"])
  663
+                func(new_data[upload_field_name]["filename"], new_data[upload_field_name]["content"], save)
664 664
 
665 665
     def get_directory_name(self):
666 666
         return os.path.normpath(datetime.datetime.now().strftime(self.upload_to))
@@ -704,12 +704,12 @@ def contribute_to_class(self, cls, name):
704 704
         if not self.height_field:
705 705
             setattr(cls, 'get_%s_height' % self.name, curry(cls._get_FIELD_height, field=self))
706 706
 
707  
-    def save_file(self, new_data, new_object, original_object, change, rel):
708  
-        FileField.save_file(self, new_data, new_object, original_object, change, rel)
  707
+    def save_file(self, new_data, new_object, original_object, change, rel, save=True):
  708
+        FileField.save_file(self, new_data, new_object, original_object, change, rel, save)
709 709
         # If the image has height and/or width field(s) and they haven't
710 710
         # changed, set the width and/or height field(s) back to their original
711 711
         # values.
712  
-        if change and (self.width_field or self.height_field):
  712
+        if change and (self.width_field or self.height_field) and save:
713 713
             if self.width_field:
714 714
                 setattr(new_object, self.width_field, getattr(original_object, self.width_field))
715 715
             if self.height_field:
10  django/db/models/manipulators.py
@@ -96,14 +96,16 @@ def save(self, new_data):
96 96
         if self.change:
97 97
             params[self.opts.pk.attname] = self.obj_key
98 98
 
99  
-        # First, save the basic object itself.
  99
+        # First, create the basic object itself.
100 100
         new_object = self.model(**params)
101  
-        new_object.save()
102 101
 
103  
-        # Now that the object's been saved, save any uploaded files.
  102
+        # Now that the object's been created, save any uploaded files.
104 103
         for f in self.opts.fields:
105 104
             if isinstance(f, FileField):
106  
-                f.save_file(new_data, new_object, self.change and self.original_object or None, self.change, rel=False)
  105
+                f.save_file(new_data, new_object, self.change and self.original_object or None, self.change, rel=False, save=False)
  106
+
  107
+        # Now save the object
  108
+        new_object.save()
107 109
 
108 110
         # Calculate which primary fields have changed.
109 111
         if self.change:
0  tests/regressiontests/bug639/__init__.py
No changes.
16  tests/regressiontests/bug639/models.py
... ...
@@ -0,0 +1,16 @@
  1
+import tempfile
  2
+from django.db import models
  3
+
  4
+class Photo(models.Model):
  5
+    title = models.CharField(maxlength=30)
  6
+    image = models.ImageField(upload_to=tempfile.gettempdir())
  7
+    
  8
+    # Support code for the tests; this keeps track of how many times save() gets
  9
+    # called on each instance.
  10
+    def __init__(self, *args, **kwargs):
  11
+       super(Photo, self).__init__(*args, **kwargs)
  12
+       self._savecount = 0
  13
+    
  14
+    def save(self):
  15
+        super(Photo, self).save()
  16
+        self._savecount +=1
BIN  tests/regressiontests/bug639/test.jpg
35  tests/regressiontests/bug639/tests.py
... ...
@@ -0,0 +1,35 @@
  1
+"""
  2
+Tests for file field behavior, and specifically #639, in which Model.save() gets
  3
+called *again* for each FileField. This test will fail if calling an
  4
+auto-manipulator's save() method causes Model.save() to be called more than once.
  5
+"""
  6
+
  7
+import os
  8
+import unittest
  9
+from regressiontests.bug639.models import Photo
  10
+from django.http import QueryDict
  11
+from django.utils.datastructures import MultiValueDict
  12
+
  13
+class Bug639Test(unittest.TestCase):
  14
+        
  15
+    def testBug639(self):
  16
+        """
  17
+        Simulate a file upload and check how many times Model.save() gets called.
  18
+        """
  19
+        # Grab an image for testing
  20
+        img = open(os.path.join(os.path.dirname(__file__), "test.jpg"), "rb").read()
  21
+        
  22
+        # Fake a request query dict with the file
  23
+        qd = QueryDict("title=Testing&image=", mutable=True)
  24
+        qd["image_file"] = {
  25
+            "filename" : "test.jpg",
  26
+            "content-type" : "image/jpeg",
  27
+            "content" : img
  28
+        }
  29
+        
  30
+        manip = Photo.AddManipulator()
  31
+        manip.do_html2python(qd)
  32
+        p = manip.save(qd)
  33
+        
  34
+        # Check the savecount stored on the object (see the model)
  35
+        self.assertEqual(p._savecount, 1)

0 notes on commit a30e3fc

Please sign in to comment.
Something went wrong with that request. Please try again.