FileField deletes files even when model instance is deleted in a transaction that is rolled back

[durdinator]

I've got a model with a few FileFields. I've got code resembling this:

class SomeObject(models.Model):
    ...

class RelatedFile(models.Model):
    filename = models.FileField(...)
    owner = models.ForeignKey(SomeObject, related_name='related_files')

@commit_on_success
def update_files(request, id):
    obj = get_object_or_404(SomeObject, pk=id)
    obj.related_files.all().delete()
    ... more code - which depending on external conditions can raise an exception ...

Since the FileField hooks the RelatedFile's post_delete signal, it deletes the original files when the delete() method is called. Then if the transaction is rolled back, the rows in the database, not being deleted after all, point to nonexistent files.

[Philippe Raoult]

right now there's no signal for commit/rollback, and I guess we'd need that to make sure the files aren't deleted yet. See #5415 for the implementation of something similar.

[Flo Ledermann]

This is a severe dataloss bug that has bitten me badly recently. Suggesting raising the priority of this, although it's too late for 1.3 now. I'll try to at least provide a test case and a doc patch as soon as i find some time.

[Russell Keith-Magee]

It's not too late at all. 1.3 final won't be released until mid January at the earliest.

I think this adds weight to Malcolm's argument at DjangoCon that FileField shouldn't *ever* delete a file, and the cleanup task should be left to an external process.

[Jannis Leidel]

Couldn't agree more with Malcolm's argument.

[Flo Ledermann]

For what it's worth, as someone who has been bitten by this bug, I absolutely agree that FileField should never delete files in the filesystem, especially if we cannot guarantee transactional integrity (which we probably cannot given the mechanisms we currently have).

[Carl Meyer]

(In [15321]) Fixed #6456 - Excised FileField file deletion to avoid data loss. Thanks to durdinator for the report.

[Carl Meyer]

(In [15322]) [1.2.X] Fixed #6456 - Excised FileField file deletion to avoid data loss. Thanks to durdinator for the report.

Backport of r15321 from trunk. Backported despite slight backwards-incompatibility due to potential for data loss.

[Jacob]

Milestone 1.3 deleted