Model FileFields empty on first pass through save()

[django@…]

For models with file fields (FileField/ImageField etc.) save() is called twice on manipulator form submission - the first time the _pre_save() and _post_save() hooks are called the file fields are empty meaning that you must explicitly check that the field value is not empty string before taking any related action. This behaviour seems confusing and I think it explains this problem:

​http://groups.google.com/group/django-users/browse_frm/thread/99a9aa63b343d27e?tvc=1&q=filefield+_post_save

[rjwittams]

This is absolutely right. I'd not looked into how file fields worked in detail yet... an eye opener.

So - in meta/__init__.py:

in manipulator_save,

save is called on the object, ( calling pre and post).

then

save_file from FileField in meta/fields.py is called for each filefield.

This leads back to meta/__init__.py, method_save_file.

In here the file is written to disk, and save is called on the object again *once for each file field*.

(calling pre and post again).

So this is probably unexpected and unwanted behaviour. The ideal is to be able

I think what we need is

1 . A general way for fields to register extra methods on the object instance rather than special casing for each one in

ModelBase.__new__ . Eg the incarnations of method_get_next_or_previous would be registered in a method of DateField.

2 . A way for some of those methods to be called during save() on the object ( after pre_save? not sure. )

So in this case method_save_file would be split into

method_set_file_data # Sets the file data to be written later
method_set_file_name # Sets the file name
method_file_presave # Actually saves the file and throws an exception aborting save if there is a problem.

the names are just examples, not set in stone. In reality, maybe they may be methods of the FileField class.

In this scheme _pre and _post save only ever get called once per save, and there is no magic in the manipulator, all the logic related to particular fields is kept with that field, and it opens up the possibility of arbitrary logic in user defined fields.

I would be happy to do this in the post new-admin "sorting out core fields and subclassing and class MANIPULATOR/MODULE and model hooking for security" branch...

[django@…]

I think I've encountered further problems caused by this one, though I'm still not keen to play in meta/__init__.py myself (there be devils)

This old and conspicuously unanswered message describes what happens:
​http://groups.google.com/group/django-users/browse_frm/thread/eac1719e472d500?tvc=1&q=errors+uploading+to+image+field

So when there is a datefield in the same model as a file field, for example, the datefield's get_db_prep_save(value) is getting called twice, first time with a datetime instance, as expected, but second time with the value already stringified, causing breakage.

Pinging rjwittams, devil slayer...

[rjwittams]

I am tackling this in the core-fields removal

[Christopher Lenz <cmlenz@…>]

Seeing a probably related error here on the magic-removal branch that's very similar to the one described ​here, however I have an auto_add_now field that isn't being set (or being set to NULL, not sure) in the second call to save() caused by the FileField.

Probably also related is the fact that the object is still inserted in the database, and the file is uploaded, but the object is not actually being updated to reflect the file name.

[jim-django@…]

I'm experiencing the same problem with trunk. Upon saving a new instance of the SwitchPlate class, I get the following traceback:

OperationalError at /admin/shop/switchplates/add/
SQL logic error or missing database
Request Method: 	POST
Request URL: 	http://localhost:8000/admin/shop/switchplates/add/
Exception Type: 	OperationalError
Exception Value: 	SQL logic error or missing database
Exception Location: 	c:\my documents\jim\projects\django-svn\django\core\db\backends\sqlite3.py in execute, line 71

With the last frame in the traceback looking like this:

c:\my documents\jim\projects\django-svn\django\core\db\backends\sqlite3.py in execute

  64. Django uses "format" style placeholders, but pysqlite2 uses "qmark" style.
  65. This fixes it -- but note that if you want to use a literal "%s" in a query,
  66. you'll need to use "%%s" (which I belive is true of other wrappers as well).
  67. """
  68.
  69. def execute(self, query, params=[]):
  70. query = self.convert_query(query, len(params))

  71. return Database.Cursor.execute(self, query, params) ...

  72.
  73. def executemany(self, query, params=[]):
  74. query = self.convert_query(query, len(params[0]))
  75. return Database.Cursor.executemany(self, query, params)
  76.
  77. def convert_query(self, query, num_params):

▼ Local vars 
Variable  	Value
params 	
['random title', 'random-title', '1', None, '1', None, True, 'uploaded\\images\\2006\\week08\\eyewings___.JPG', 236, 239, 3]
query 	
'UPDATE "shop_switchplates" SET "title"=?,"slug"=?,"category_id"=?,"series_id"=?,"type"=?,"pub_date"=?,"visible"=?,"image_file"=?,"image_height"=?,"image_width"=? WHERE "id"=?'
self 	
<django.core.db.backends.sqlite3.SQLiteCursorWrapper object at 0x01AD1858>

The model looks like this, stripped of it's META class and methods:

class SwitchPlate(meta.Model):
    title = meta.CharField('Title', maxlength=50, help_text = "What would you call this plate if you were asking for it over the phone?")
    slug = meta.SlugField("Web name ('slug')", 
                          prepopulate_from=('title',), unique_for_date='pub_date',
                          help_text='Machine-readable version of the title, used in the web address for this particular switch plate. Leave as-is unless very ugly or confusing.')
    category = meta.ForeignKey(Category, verbose_name="Category", 
                                    help_text="Which store category this plate should be filed under")
    series = meta.ForeignKey(Series, verbose_name="Series", blank=True, null=True,
                                  help_text="If several plates are strongly related, create a Series to file them under as well.")
    type = meta.CharField("Plate Type", choices = PLATE_TYPE_CHOICES, maxlength = 1, radio_admin=True,
                          help_text = "Create separate switch plate records for different types of switches, even if the design is the same. They can be linked together by series if appropriate")
        
    pub_date = meta.DateTimeField('Publication Date', auto_now_add = True,
                                  help_text='Date this switch plate begins showing on the site. If set to a future date, the plate will begin appearing on the site then.')
    visible = meta.BooleanField('Visibility', default=True,
                                help_text='Uncheck this to stop this switch plate from appearing on the site. Check again to let it show. Even if checked, plates will not show before their publication dates above.')

    image_file = meta.ImageField("Image File", height_field='image_height', width_field='image_width',
                                 upload_to='uploaded/images/%Y/week%U/')
    image_height = meta.IntegerField("Height", default=None, null=True, blank=True,
                                     help_text = "Image height in pixels (automatically filled in for you)")
    image_width = meta.IntegerField("Width", default=None, null=True, blank=True,
                                    help_text = "Image width in pixels (automatically filled in for you)")

[jim-django@…]

I should add that this is with pysql3 backend.

The problem goes away when the pub_date field is supplied with a default; I set it to meta.LazyDate() which provides a useful though slightly inaccurate preview of what the final data will be.

[anonymous]

is this still a problem? i just ran into an issue where i have a Media model and a MediaVersion model which is identical plus an object_id fk back to Media. i've overridden Media.save() to copy all the date post-save into a new MediaVersion object. BUT, when i update an image or file field in Media, it seems to call Media.save() twice, and thus i get two new MediaVersion objects. one has all the changes to non-file/image fields, and the second has just the changed file/image field. what can i do to work-around this? or even fix it so save() is called only once?

[Jacob]

(In [4609]) 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.