Override an existing file, using Model.save_FIELD_file method,

Undo what I presume is an accidental title change, since it makes no sense.

Maybe the thing should be called "overwrite" not "override" Also, documentation about that should be available somewhere ;)

Umm... I'm not sure this is an automatic "accepted", since we've had hesitation in the past about automatically overriding files (it has security and annoyance implications, for a start). Moving to "design decision needed" until we've had time to think about it.

I marked as accepted because this patch does not **automatically** overwrite a file, you need to override a method on your model, in which case you are supposed to know what you are doing and pass the right parameter to the super() method.

On the other side, not having this means that anybody that wants to overwrite files needs to write his/her own method checking for file existence, etc. Which doesn't seem very DRY.

Just my 0.02, well 0.002 :P

Now I can use

class UserProfile(models.Model):
    avatar = models.ImageField(blank=True, upload_to='users/avatars/', overwrite=True)

the other use is to set explicitly overwrite to True,

userprofile.save_avatar_file(filename, content, overwrite=True)

When is this patch going to be acccepted into the django development version? This is a very useful feature, there is no automatic overwriting of files, it simply gives more flexibility to the programmer. What is the problem with this exactly?

when some of core developers have time to decide whether it's a good idea or not (see Malcolm's comments above). If you want to hurry it up, then feel free to post a message to django-developers pointing out the good points of this.

Replying to Seamus:

When is this patch going to be acccepted into the django development version? This is a very useful feature, there is no automatic overwriting of files, it simply gives more flexibility to the programmer. What is the problem with this exactly?

There are currently over three hundred tickets at the stage of "design decision needed", and that stage, by its nature, requires a fair amount of attention from multiple developers. As a result, it can take quite some time for a more or less "normal" process of discussion to get around to any particular ticket unless someone steps up -- as Chris suggested -- and gets the ball rolling. The place to do that is the django-developers list (and please note that a short "this ticket needs discussion" or "I need this feature, why don't you add it already" message is generally frowned upon -- to get discussion going, mention the ticket and provide an argument for or against the proposed solution).

I was really surprised when discovered that the is no simple way to overwrite an uploaded file in Django. So I'm already using this patch and hope it will be merged soon.

I agree that this patch should be merged (and I started a topic in django-devel), but I disagree that there is no simple way to do it in Django.

Check this:

def _save_FIELD_file(self, field, filename, raw_contents, save=True):
        fullpath_filename = os.path.join(settings.MEDIA_ROOT, field.get_filename(filename))
        if os.path.exists(fullpath_filename):
                os.remove(fullpath_filename)
        super(MyModel, self)._save_FIELD_file(field, filename, raw_contents, save)

Patch which deletes a file when a new one is upload, but only if we're the only record which references that file.

I added a patch which takes a different approach on this subject. The reasoning is simple. The current code defines the following behavior: when an instance of a model is deleted, the file will also be deleted if the deleted model instance was the only instance which referenced this file. Hence, the curent code is already destructive. To me, it follows logically that the code should do the same thing when a new file is uploaded. If the new file upload is named the same as the old one, it'll be an effective overwrite. If not, it's just remove a file from the filesystem that would otherwise be unreferenceable and not manageable from the admin. Additionally, if there are other records that reference the file, the original one will still be preserved.

Since this is not regarded a bug, there is no reason to give this to milestone 1.0 beta.

Bugs aren't the only things planned for 1.0 beta. This is being addressed as part of #5361, which is planned to make it into 1.0 beta. Even though it won't be fixed directly, the behavior requested in this ticket will be quite possible once that goes in. Per Jacob, all such related tickets should be placed in the 1.0 beta milestone.

File storage is in as of [8244], so I'm marking this as wontfix -- as Marty says, it's very easy to write a custom file storage backend that works exactly how you like it.

I'm re-opening this because I respectfully disagree with Marty. Don't get me wrong, I much prefer the new FileStorage? backend interface, but according to the current docs - 1 the file storage object is never given any context with which to decide whether or not to overwrite the file.

Here is my dilemna, I have an Image model, that allows customers to upload images. Inevitably, the customers want to replace a specific image with a new one, so they re-upload a new version of the file and it gets a new name. So things break, like CSS background image urls. etc. In the old monkey-patch, I could query the models to find out if the current model-instance was the only row that referred to the image, and if so it would overwrite it. If not, it would rename the file as normal.

With the current system, my custom storage instance has no access to the necessary request specific context, like model-instance in order to decide whether or not to overwrite the file. So it must generally make a decision of "always overwrite files even when they have the same name" or "always generate a new name, even if the old filename will be orphaned (no more references to it in the database.)"

Actually, with the default backend you've got a DoS entry if you allow your users to upload a profile picture with an ImageField (even if you check the size of the stuff they upload) - since it will leave the orphaned images behind. The attacker just needs to reupload files to fill up the available disk space which may be scarce on shared hosting.

In any case, the current behaviour doesn't really make a lot of sense when you override upload_to to set a filename (e.g. using the db id) instead of relying on the name from the browser.

I think it should work this way: when you reupload a file, it should be the same as first deleting the old file and then writing the new one (maybe in reverse order, with a bit of code to handle the case where the names are identical). What do you think?

BTW, there's a snippet here with a custom backend that always overwrites:

http://www.djangosnippets.org/snippets/976/

With Django's new file access API there is more flexibity to deal with such problems.

I think it's not a good idea to override get_available_name since it's a public methode that shouldn't have a side effect (I suppose) due to his name

{{{class OverwriteStorage?(FileSystemStorage?):

def _save(self, name, content):

if self.exists(name):

self.delete(name)

return super(OverwriteStorage?, self)._save(name, content)

}}}

Thank you

Without having tried it, I don't think the code given my elaatifi (overriding _save) will work, since at this point get_available_name

Apparently I screwed up that last comment pretty badly - sorry. Not sure why the status changed.

Overriding both _save and get_available_name seems to do the trick for me. It *seems* that it should be unnecessary to override both, but overwriting _save wasn't enough, and only overriding get_available_name resulted in an infinite loop since _save demands that the file not exist (i.e. it won't do an actual overwrite -- but rather a delete, then a write).

from django.core.files.storage import FileSystemStorage
class OverwriteStorage(FileSystemStorage):
    def _save(self, name, content):
        if self.exists(name):
            self.delete(name)
        return super(OverwriteStorage, self)._save(name, content)
    def get_available_name(self, name):
        return name

I disclaim any copyright to the above code. (I'm guessing it would be considered too trivial to be copyrighted anyway, but just in case)

Also, while this is maybe a bit more likely to cause race conditions, nothing particularly harmful happens as a result. The thread will just have to try again until it succeeds.

Just a quick thought from a UI standpoint in the Admin, it sure would be nice when you add an image for there to be a pop-up that showed you the image and proposed three options. A similar pop-up for an image could post some meta-data.

Would you like to: Overwrite the existing image with the new one? (sitewide) Save the new image under a different name? (affects only this instance) Use the existing image? (No need to create a new one with an _ in the name)

rizumu: This bug is not particular to the admin. Also what you're talking about is the situation where the user is supposed to control the filename. This bug is about the case where the programmer is supposed to control the filename (where currently there's no simple knob in Django to say "this filename, and I mean it, no underscores").

Since the original issue is very out of date after filesystem refactor, I'll close this as a duplicate of the new ticket #11663