Regression: FileField calls validate during form validation; breaks auto_now_add date fields

[Russell Keith-Magee]

As of dcd43831 (fixing #9893), a FileField will call generate_filename() as part of the validation step for a FileField on a form. This was then updated in 05d333ba to address #18515.

Unfortunately, this means that the filename function provided as an argument to upload_to can no longer reference any field with a pre-save behavior.

The common use case for this is to organize files on disk according to upload date. For example:

def user_filename(instance, filename):
    return os.path.join('user_files', instance.uploaded_timestamp.strftime('%Y-%m-%d'), filename)

class UserFile(models.Model):
    uploaded_timestamp = models.DateTimeField(auto_now_add=True)
    data = models.FileField(upload_to=user_filename)

Under Django 1.5, attempting to call is_valid() on a Modelform for this model will raise a "'NoneType' object has no attribute 'strftime'" exception, because instance.uploaded_timestamp hasn't been instantiated yet. This is despite the fact that the uploaded data has been provided, the generated filename would be valid, and the upload timestamp can be computed.

In Django 1.4 and earlier, this works because no validation was performed for FileFields filenames; the uploaded_timestamp was evaluated as part of the model pre-save, and the persistence of the file to disk occurred after the model was saved.

To my reading, ​the documentation is ambiguous on whether this is expected behavior or not. It says that the model may not be saved to the database yet, but points at AutoField as the cause for problems. However, it also explicitly talks about using strftime as part of file paths. A file datetimes of 'now' would seem to be an obvious usage of this feature.

[Russell Keith-Magee]

For the record, I discovered this by upgrading a commercial project to Django 1.5, so there is at least one project in the wild that will be affected by this change. Although I've discovered it with a auto_now_add FileField, it's not hard to see that this change also affects any field with a pre_save behaviour.

It also has the potential to lead to incorrect validation. Consider the case of a field with a pre_save behavior that updates the field (auto_now is one example, but any denormalization/summary field would be an example). The call to validate occurs *before* the call to pre_save is made, which means that you're going to get the pre_save value used as part of your validation. If you then save the model, the pre_save() will be called, and the actual filename that is used for saving the file will be different to the one used for validation.

Some initial thoughts about possible solutions:

• Document that you can't use a field with pre-save behaviour. Not ideal IMHO, since it rules out an obvious use case for upload_to.
• Roll back the fix. Also less than ideal; #9893 is an edge case bug, but it's something that has been seen in the wild, and isn't *too* hard to generate.
• Invoke pre_save on all model fields prior to validation. Given that most validation doesn't need this, this approach seems a little excessive.

[Aymeric Augustin]

Would this work if the DateTimeField was declared with default=timezone.now rather than auto_now_add=True?

[Russell Keith-Magee]

@aaugustin: I've just checked with my production code, and yes, default=timezone.now works for the auto_now_add case. However, it won't address auto_now, or the pre_save conflict problem.

[Aymeric Augustin]

Since we're extremely close to 1.5 RC1, the most reasonable course of action in the short term is to revert the two commits.

[Aymeric Augustin <aymeric.augustin@…>]

In db278c3bf9177043c42a9ed8b529a5c117938460:

Fixed #19525 -- Reverted dcd4383107 and 05d333ba3b.

Refs #9893, #18515.

Thanks Russell for the report.

[Aymeric Augustin <aymeric.augustin@…>]

In 3cb87ec605fb9e7785aa0580bd8220797b622e0c:

[1.5.x] Fixed #19525 -- Reverted dcd4383107 and 05d333ba3b.

Refs #9893, #18515.

Thanks Russell for the report.

Backport of db278c3 from master.

[Preston Holmes <preston@…>]

In 0e431e5dd7ed19aa2119ceba9ebed050c2988844:

[1.5.x] Fixed #19525 -- Reverted dcd4383107 and 05d333ba3b.

Refs #9893, #18515.

Thanks Russell for the report.

Backport of db278c3 from master.