Support pluggable backends for FileField

FileField refactor to support backends, as well as a FileSystemBackend to implement existing functionality

I don't have any documentation written for this patch yet, but the patch is here to look over. It's also untested in newforms-admin, but it doesn't change any of the form-handling code, so it should work properly. I'll try to have some documentation ready sometime this week. In the meantime, I'll make some additional notes about it on django-developers, to help explain things a bit.

Moved backend code into django.core.filestorage, added documentation and a few minor tweaks

Fixed a tyop and added missing documentation in the last patch

There is wrong import in last patch.

In django/db/models/fields/init.py on line 750 should be "from django.core.filestorage import FileSystemBackend?"

Eh close ... it should be:

from django.core.filestorage.filesystem import FileSystemBackend

One more idea ... the default backend should be in settings. Then you could (for example) use different backend for development and other on production server.

Fixed few details, so it works for me with current trunk (hope didn't break it for you :-) ). Also default backend is loaded from settings.

Seems my last patch is wrong, ignore (or delete) it please ...

While I'm not opposed to finding a way to specify the filestorage backend in settings.py, specifying just a path alone won't generally suffice. You see, alternate backends don't always share the same arguments, so it's difficult to expect a single function call to work for all available backends.

The only way I can really think of doing it is to instantiate the backend object directly in the settings. For FileStorageBackend?, this is okay as long as the media_root and media_url arguments are supplied. If they're not, it'll try loading them from django.conf.settings, which will obviously break fairly severely.

Of course, you could manually supply the MEDIA_ROOT and MEDIA_URL values as arguments, but then that disallows using it in global_settings.py, since MEDIA_ROOT and MEDIA_URL aren't set properly when the default backend would get instantiated.

So, all in all, I think it's worth considering, but I'm not sure the framework in its current condition can support it. I'll see what it'll take to do so, but it may be a documentation issue. For instance, one way to do it would be to import the backend from some module that's different in your development and production environments. Then the application code could just use the backend protocol, while the environment-specific code determine which backend is most appropriate.

Will likely solve #641, #1642 and #5306.

Will solve #3588 when fixed.

Actually, I don't see how this patch would have any impact whatsoever on #3588. This doesn't change what field is used to store the path+filename, it only affects how and where the file itself is stored.

Working patch with documentation, including get_absolute_path()

Fixed a reference that didn't get updated to get_absolute_path()

Fixed a bug when saving the uploaded file

Much more complete patch, including docs and tests

This latest patch includes tests and updated documentation, and some changes that attempt to address many various issues that have come up regarding FileField?. I've added "fs-rf-fixed" and "fs-rf-docs" keywords to tickets that are addressed, at least in part, by this new patch. The tickets with "fs-rf-fixed" are legitimately fixed by this patch, and be closed immediately when committed. The "fs-rf-docs" tickets aren't fixed directly by the patch, but their fixes are enabled by the new features, and the documentation covers what hooks will be needed to solve the problems described.

Also, the patch doesn't come up in the web viewer, but it should be intact. TortoiseSVN tends to screw up my patches sometimes for some reason, though, so if it doesn't work, let me know and I'll try again.

hej Gulopine i cant seem to be able to apply filestorage.7.diff using

patch -p0 filestorage.7.diff

to me it looks like the .diff is broken could you try again?

Fixed a few things, added tests and hopefully the patch will work this time

Finally implemented File.eq properly

It dawned on me that I had some temporary kludge in File.__eq__, that I had put in just to get the serializer tests to work. Now it's implemented properly, and the serializer tests still work!

The most recent diff had some problems I didn't catch before uploading, so it's not to be used at the moment. I'll get a new patch up this week.

Fixed merge problems, updated to r7091, passed all tests, and removed a few uses of the now-deprecated get_FOO_*() methods

My awesome contribution: s/desribed/described/ line 1167 of filestorage.10.diff

In FileSystemBackend.delete_file() I suggest that we add a test:

 if filename and os.path.exists(file_name):

because otherwise it raises an IOError if filename is empty.

An import of settings is missing in db.models.fields.files, used line 156, patch:

from django.conf import settings

It appears that this patch is supposed to provide dynamic file naming capability but I have reviewed the docs and have not seen how that capability is exercised

Fixed a few issues raised by David Larlet, added documentation for upload_to callables, updated to patch cleanly against r7133

George, this new patch includes documentation on providing a function as the upload_to argument, which provides the funcitonality you require.

Also, I mistyped in the attacment comment. That last attachment is valid up to r7141.

David Larlet informed me that the new upload_to handling prevents model instances from being pickled (and thus cached) in many cases. This is now a known issue, I've got a fix for it, and I'll update the patch this weekend to resolve the issue.

Fixed caching problem, added FileField subclassing docs, updated to r7156

Fixed a problem when deleting models, updated to r7261

Can't see the latest patches, trac issue again ? By the way will this solve #6456 ? From filestorage.6.diff I'd say no but maybe this has been added since.

No, I don't expect this will have any impact on #6456, which is why I didn't mark it with any fs-rf keywords. This patch doesn't really deal with the interactions between the database and file behaviors, it just manages how files are stored and accessed. #6456 deals more with the "when" which is also important, but beyond what this ticket is for.

Some reorganization, several method renames and doc rewordings, and an update to r7321

This most recent patch includes several changes suggested by Jacob, and should be a final patch except for some possible documentation changes. Anyone who's already been using this patch, be aware that most of the method names have changed, and **your code will break**. It won't break anybody who's running trunk, but those of you using the patch have been warned.

Many changes, many questions:

• why don't you use django.core.urlresolvers.get_callable in get_storage?
• how can I pass arguments to my custom Storage? (let's say I want to pass Amazon credentials for S3, see #6390)
• why get_absolute_url() had been renamed to url()? I just want to understand because the previous version looks more coherent with models for instance.

I confirm that my code will break :) With your answers, I will try to integrate this new patch in my project before it's merged to the trunk.

Okay, I figured you'd be the first to react, and I'm glad you're following closely enough to ask these questions.

• I didn't know about django.core.urlresolvers.get_callable() when I wrote it. Jacob thought there was some kind of a function to resolve import paths to objects, but we just didn't run across it. I'll look into it.
• Just write your __init__() in the same way as the one in FileSystemStorage. That is, it can take arguments, just make sure they're optional, and their defaults should be pulled from settings.
• Not to pass the buck, but those method renamings were requested by both Jacob and Adrian. It's a little inconsistent, but it's not something I'm willing to fight over.

I do apologize for the breakage, but that can happen even on our fairly-stable trunk, and is especially likely when running uncommitted patches.

No problem for the breakage, I know that this game has rules and I'm happy to contribute. Thanks for your answers, I will pass all default arguments as settings now.

It seems that there is still a s/get_absolute_url/url/ to do in db.models.fields.files.File.url() (and maybe a test to add to verify this point?).

Okay, I confirm that it works perfectly with both filesystem and S3 storages if you use the file attached in #6390.

There is still a s/get_absolute_path/path/ to do in db.models.fields.files.File.path() line 38, let me know if you want a diff of your diff.

Takes david's corrections into account and updates to r7397

Okay, I like david's suggestion of using the existing get_callable, but it feels wrong to reach into urlresolver code for this, and refactoring that would be outside the scope of this ticket. I've left it as is for now.

What about my proposition to do not delete default image? For the moment I'd hardcoded it in storage.delete but that's not really reusable/safe. It'll be probably more appropriated to do it in FileField.delete_file because this way you have access to self.default

Indeed, I missed that one, thanks for the reminder. I'll definitely put up a new patch with that included.

Gulopine, It looks like django.core.filestorage.init is missing an import of the exceptions module.

You need a: from django.core import exceptions

I only hit this bug because my settings file didn't have a DEFAULT_FILE_STORAGE setting

Thanks

Thanks, that's a good catch. I've added it to my local copy, and I'll get a new patch up soon. I'm just holding off to see if there's anything else that needs to be changed/added.

Some security enhancements, more tests, and an update to r7520

Sometimes self._dimensions_cache doesn't exist in db.models.fields.files.ImageFile.delete(), I suggest to check with if hasattr(self, '_dimensions_cache'): before.