From a30e3fca48be23c856cda778ec3f0e0eec75fd91 Mon Sep 17 00:00:00 2001 From: Jacob Kaplan-Moss Date: Mon, 26 Feb 2007 17:17:11 +0000 Subject: [PATCH] Objects with FileFields no longer get save() called multiple times from 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 --- django/db/models/base.py | 7 +++-- django/db/models/fields/__init__.py | 14 ++++----- django/db/models/manipulators.py | 10 ++++--- tests/regressiontests/bug639/__init__.py | 0 tests/regressiontests/bug639/models.py | 16 +++++++++++ tests/regressiontests/bug639/test.jpg | Bin 0 -> 1780 bytes tests/regressiontests/bug639/tests.py | 35 +++++++++++++++++++++++ 7 files changed, 68 insertions(+), 14 deletions(-) create mode 100644 tests/regressiontests/bug639/__init__.py create mode 100644 tests/regressiontests/bug639/models.py create mode 100644 tests/regressiontests/bug639/test.jpg create mode 100644 tests/regressiontests/bug639/tests.py diff --git a/django/db/models/base.py b/django/db/models/base.py index ff99fd8b46..b70e6fd99a 100644 --- a/django/db/models/base.py +++ b/django/db/models/base.py @@ -356,7 +356,7 @@ class Model(object): def _get_FIELD_size(self, field): return os.path.getsize(self._get_FIELD_filename(field)) - def _save_FIELD_file(self, field, filename, raw_contents): + def _save_FIELD_file(self, field, filename, raw_contents, save=True): directory = field.get_directory_name() try: # Create the date-based directory if it doesn't exist. os.makedirs(os.path.join(settings.MEDIA_ROOT, directory)) @@ -391,8 +391,9 @@ class Model(object): if field.height_field: setattr(self, field.height_field, height) - # Save the object, because it has changed. - self.save() + # Save the object because it has changed unless save is False + if save: + self.save() _save_FIELD_file.alters_data = True diff --git a/django/db/models/fields/__init__.py b/django/db/models/fields/__init__.py index 6b38b2229e..e8a2c231c5 100644 --- a/django/db/models/fields/__init__.py +++ b/django/db/models/fields/__init__.py @@ -635,7 +635,7 @@ class FileField(Field): setattr(cls, 'get_%s_filename' % self.name, curry(cls._get_FIELD_filename, field=self)) setattr(cls, 'get_%s_url' % self.name, curry(cls._get_FIELD_url, field=self)) setattr(cls, 'get_%s_size' % self.name, curry(cls._get_FIELD_size, field=self)) - setattr(cls, 'save_%s_file' % self.name, lambda instance, filename, raw_contents: instance._save_FIELD_file(self, filename, raw_contents)) + setattr(cls, 'save_%s_file' % self.name, lambda instance, filename, raw_contents, save=True: instance._save_FIELD_file(self, filename, raw_contents, save)) dispatcher.connect(self.delete_file, signal=signals.post_delete, sender=cls) def delete_file(self, instance): @@ -653,14 +653,14 @@ class FileField(Field): def get_manipulator_field_names(self, name_prefix): return [name_prefix + self.name + '_file', name_prefix + self.name] - def save_file(self, new_data, new_object, original_object, change, rel): + def save_file(self, new_data, new_object, original_object, change, rel, save=True): upload_field_name = self.get_manipulator_field_names('')[0] if new_data.get(upload_field_name, False): func = getattr(new_object, 'save_%s_file' % self.name) if rel: - func(new_data[upload_field_name][0]["filename"], new_data[upload_field_name][0]["content"]) + func(new_data[upload_field_name][0]["filename"], new_data[upload_field_name][0]["content"], save) else: - func(new_data[upload_field_name]["filename"], new_data[upload_field_name]["content"]) + func(new_data[upload_field_name]["filename"], new_data[upload_field_name]["content"], save) def get_directory_name(self): return os.path.normpath(datetime.datetime.now().strftime(self.upload_to)) @@ -704,12 +704,12 @@ class ImageField(FileField): if not self.height_field: setattr(cls, 'get_%s_height' % self.name, curry(cls._get_FIELD_height, field=self)) - def save_file(self, new_data, new_object, original_object, change, rel): - FileField.save_file(self, new_data, new_object, original_object, change, rel) + def save_file(self, new_data, new_object, original_object, change, rel, save=True): + FileField.save_file(self, new_data, new_object, original_object, change, rel, save) # If the image has height and/or width field(s) and they haven't # changed, set the width and/or height field(s) back to their original # values. - if change and (self.width_field or self.height_field): + if change and (self.width_field or self.height_field) and save: if self.width_field: setattr(new_object, self.width_field, getattr(original_object, self.width_field)) if self.height_field: diff --git a/django/db/models/manipulators.py b/django/db/models/manipulators.py index aea3aa70a7..c624cb698b 100644 --- a/django/db/models/manipulators.py +++ b/django/db/models/manipulators.py @@ -96,14 +96,16 @@ class AutomaticManipulator(oldforms.Manipulator): if self.change: params[self.opts.pk.attname] = self.obj_key - # First, save the basic object itself. + # First, create the basic object itself. new_object = self.model(**params) - new_object.save() - # Now that the object's been saved, save any uploaded files. + # Now that the object's been created, save any uploaded files. for f in self.opts.fields: if isinstance(f, FileField): - f.save_file(new_data, new_object, self.change and self.original_object or None, self.change, rel=False) + f.save_file(new_data, new_object, self.change and self.original_object or None, self.change, rel=False, save=False) + + # Now save the object + new_object.save() # Calculate which primary fields have changed. if self.change: diff --git a/tests/regressiontests/bug639/__init__.py b/tests/regressiontests/bug639/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/regressiontests/bug639/models.py b/tests/regressiontests/bug639/models.py new file mode 100644 index 0000000000..30ca7e6326 --- /dev/null +++ b/tests/regressiontests/bug639/models.py @@ -0,0 +1,16 @@ +import tempfile +from django.db import models + +class Photo(models.Model): + title = models.CharField(maxlength=30) + image = models.ImageField(upload_to=tempfile.gettempdir()) + + # Support code for the tests; this keeps track of how many times save() gets + # called on each instance. + def __init__(self, *args, **kwargs): + super(Photo, self).__init__(*args, **kwargs) + self._savecount = 0 + + def save(self): + super(Photo, self).save() + self._savecount +=1 \ No newline at end of file diff --git a/tests/regressiontests/bug639/test.jpg b/tests/regressiontests/bug639/test.jpg new file mode 100644 index 0000000000000000000000000000000000000000..391b57a0f32e26fb7090a63e1d7f922ad0f7e5b4 GIT binary patch literal 1780 zcmbV}c~H|=5Xavy2MITUL?EPKAO;i48-FfqN-`m-@pM8oKiq}BLgW=8q z5Cj1?r2)klAoy?vNrFVKAZ{sTwH2UkboWLez@ikrfa{XRiv+;i8>~dXlxsoCcNgQoc4eq|1)Kp!LsMH*9iy$K zt*48@;w=ey0|UG**}}xqfx2;nJ=Kox!sPh7xNq~I+XW=Ew?#$^;snmDox=1TnVc8_ zf1wDZtEY$8$5RLd3g6YvmH(fmxCJn1Wvjt($P~aZ5F7(3nt+M2W2ACw3xoS62nI(W zRZwVEHFae|oeqFOa5xMBM~EHrsgVHqP(q zIEfOLAXHUbqHkb`Uq&{!SiWMd?K&!L{g2L@To|ry?j9`v%>nGdEkT^{h{z}|FFG+P zIVCkMeP`C5z1caUeYxTz1%*eC9Y0ZY=Ipuh3W@Z5<>i{%y84F3D_8H_ZMt{=LG#0w zj%V`D=Up#ecK7!64-5_skBq*Vn4FsaIP>ZA?A!tu1mJ&SeJA?|7e>hiLm=P?)B+a- zOI0p727x46tLSd|wNxw?S{98!KB90MFdPpe@?)M+gcV(hkooZKtTHX5J6 z8I5w&1BNP3IRr|aI}=;+Q2PNLbXjg(FyvV)KOm~6^2)_&VKlJ|Ys7m~vbs}TRPfV~ zH8GV9UG0HOc?q6LzVgnx=(`iEsvhYwvxv0NvA6yW7?UrgyXIdF`oF%}_i-?tS{n|m zYi&b}?YZ_h-ZD3NKN0ck8*i~$zqd=TMlXD6M53)YefL8znj@zWP;F@@^hwJ(7+MXJd>Tq^=6E z>d~(2l%S$ik(YQS+s)QE8F_wr zEnQPMxa>prbQNMSj*-DOo0-Wew3}&-;m%2Bgf?#1BA8{V*o0byemCi-o1P(NvE>;7 zq@y*jybi78a6_91*u3X8$p*OBq>1{yWMPwc1mcZ}VH=0JpP5ICaIts6XP|s@Pf4uS zd!O-|pInjrBGT#Np(>5%0cX|Dy)i=>$j&ij#W_I_C%?2h1DJkwa5(vZvqshhfdc$o zbghE-tS9)aLCbv!>7wkqMK=xm+Pb4ydas2{anDWtwV;@9nujY*44KB&^bV@;G|bcL zfPNpp;N%Rm{!KQT;C6Wos-F?OFB%cI?@5{Gn{B;*&=dm~Gqva}SzUAQ9<=-+kuU?g z=tIQ?D~5-SZ@jhD-M@r-5HYa4#c?#q6X8cz03xAe_C8;q>C;NyurkZh<7i(>L*P@w z+(~>4zf8vMVXiVG>R4j#RQo)OnvC_DBxK21XMO3ao=Y4G8M6w|VaVD|qn)uVqts^m zm*9?43PSV!Vq{dN+`Q&6k|g<*fgg>j*Gu@Z5dn zxrb?d`Yx;3SpC_~E+RYRHD|X)j_8=Nu<+)1_10?`MAf9u`o1a#gBrC`+qByAd5t+K zI4*v=GOz5BhtZfDC+zfRT6KEuc3XEXY-VBGuIR@NLqm?Aa;9Xmc5$HIVQoX{5iTa* z_~cCe81*>E;>yxI&BOx(H;3uRPkxkj1uMYe9i<+9#KGB0N%e~pcIFuxox9Z<2&vMt uc?CGQ#WC%x<~p>Pej+5U`BnTV8~?VA{_>?!qe_j*=jhOm^3$b?mwy0^7T=}- literal 0 HcmV?d00001 diff --git a/tests/regressiontests/bug639/tests.py b/tests/regressiontests/bug639/tests.py new file mode 100644 index 0000000000..c05ef120e1 --- /dev/null +++ b/tests/regressiontests/bug639/tests.py @@ -0,0 +1,35 @@ +""" +Tests for file field behavior, and specifically #639, in which Model.save() gets +called *again* for each FileField. This test will fail if calling an +auto-manipulator's save() method causes Model.save() to be called more than once. +""" + +import os +import unittest +from regressiontests.bug639.models import Photo +from django.http import QueryDict +from django.utils.datastructures import MultiValueDict + +class Bug639Test(unittest.TestCase): + + def testBug639(self): + """ + Simulate a file upload and check how many times Model.save() gets called. + """ + # Grab an image for testing + img = open(os.path.join(os.path.dirname(__file__), "test.jpg"), "rb").read() + + # Fake a request query dict with the file + qd = QueryDict("title=Testing&image=", mutable=True) + qd["image_file"] = { + "filename" : "test.jpg", + "content-type" : "image/jpeg", + "content" : img + } + + manip = Photo.AddManipulator() + manip.do_html2python(qd) + p = manip.save(qd) + + # Check the savecount stored on the object (see the model) + self.assertEqual(p._savecount, 1) \ No newline at end of file