Document how to change the path of a FileField

[Simon Litchfield]

Currently we can't set filefield/imagefield's underlying filename, without loading the file and feeding it's contents into save(). Obviously this is grossly inefficient.

This is a simple patch that allows you to set the path like so --

instance.myfile.path = 'uploads/new-path.avi'

Previously, instance.myfile.path is read only. If you set instance.myfile then instance.myfile.path etc will raise a ValueError, The 'myfile' attribute has no file associated with it.

Works with FileField, ImageField, and anything else that uses a File object that is subclassed from FieldFile.

Check stackoverflow, google, etc- lots of people banging their heads trying to figure out why they can't simply set the filename.

[Russell Keith-Magee]

I might be missing something here, but this doesn't seem like a very good idea to me -- at least, not in the way it's proposed.

The file path is readonly because it points to an actual file. You can't just change the path value, because then it will point to a file location that won't exist. And changing the file path as a column operation isn't something that (to me) is an obvious interface for a 'file move' operation. There's also the problem of whether the file backends support atomic move operations (which may be problematic on some platforms).

However, if there's mass confusion, there is something that needs to be looked at here -- even if it's just a documentation fix or a tightening of error messages. Further discussion required.

[Simon Litchfield]

I agree, "moving" a file should be explicit, something like myfield.move(src, target).

The issue here, is cases where other processes or whatever will place a file in a certain location, and you just need to "set" that location in your database. Whether or not a "move" is involved. Currently you can't do that. Allowing 'path' to be set makes sense in these situations.

Remember, django can't take the stance that it guarantees a file will be there just because it says so in the database. It's up to the developer to make sure they don't set .path to some non-existant rubbish, in the same way they need to make sure the file is there.

People are doing crazy stuff like opening the whole file and saving it back using the save() method (since save and delete are the only methods provided in docs).

The only other workaround is ugly, I don't think we wanna be adding this to the docs --

model_instance.myfile = model_instance.myfile.field.attr_class(instance, model_instance.myfile.field, 'my-filename.jpg')

[Flo Ledermann]

This may be a slightly different usecase or a subset of the cases covered by this issue, but my approach to setting a FileField to contain a server-processed file is using a helper class copied together from django.core.files.uploadedfile.UploadedFile and django.core.files.uploadedfile.TemporaryUploadedFile :

import os
from django.conf import settings
from django.core.files.base import File
from django.core.files import temp as tempfile


class DjangoTempFile(File):

    def __init__(self, name=None, suffix='.temp', content_type=None, size=None, charset=None):
        if settings.FILE_UPLOAD_TEMP_DIR:
            file = tempfile.NamedTemporaryFile(suffix=suffix,
                dir=settings.FILE_UPLOAD_TEMP_DIR)
        else:
            file = tempfile.NamedTemporaryFile(suffix=suffix)

        super(DjangoTempFile, self).__init__(file, name)

        self.content_type = content_type

        # TODO check if these could be removed
        self.size = size
        self.charset = charset

    def __repr__(self):
        return "<%s: %s (%s)>" % (
            self.__class__.__name__, smart_str(self.name), self.content_type)

    def temporary_file_path(self):
        """
        Returns the full path of this file.
        """
        return self.file.name

    def close(self):
        try:
            return self.file.close()
        except OSError, e:
            if e.errno != 2:
                # Means the file was moved or deleted before the tempfile
                # could unlink it.  Still sets self.file.close_called and
                # calls self.file.file.close() before the exception
                raise

    def _get_name(self):
        return self._name

    def _set_name(self, name):
        # Sanitize the file name so that it can't be dangerous.
        if name is not None:
            # Just use the basename of the file -- anything else is dangerous.
            name = os.path.basename(name)

            # File names longer than 255 characters can cause problems on older OSes.
            if len(name) > 255:
                name, ext = os.path.splitext(name)
                name = name[:255 - len(ext)] + ext

        self._name = name

    name = property(_get_name, _set_name)

You can use this file to write to from whatever code you want, and then call model.image_field.save(name, temp_file_instance) which will pick up the temporary_file_path method and copy the file over instead of re-reading it. The advantage is that you benefit from Django's name-collision resolving, and external processes should probably not write to the MEDIA_ROOT anyway but use a temporary file instead.

Maybe Django should provide such a class as part of the official API?

[Jacob]

Milestone 1.4 deleted

[Jacob]

I'm unconvinced of the specifics (see Russ's comments; allowing the path to be settable seems like a foot-gun), but something needs to be done here. Marking accepted; hopefully someone can come up with a better API.

[Simon Litchfield]

Just to clarify, all that's wanted here is a way to set a value in the database. Currently it's not possible unless you resort to raw SQL or the 'ugly hack' I posted above. Every other field is "settable". Django is assuming control of my files<->database; and I don't always necessarily want that, particularly if I go as far as deliberately using the assignment operator.

That said I agree with Russ, we could certainly use a move() type method (though I'm not sure if that's what people have been asking for).

[Claude Paroz]

I think that the OP use case is solved by setting the name instead of the path property (name being relative to the field storage base location), as demonstrated by the following test case:

diff --git a/tests/file_storage/tests.py b/tests/file_storage/tests.py
index c3800cb..573914b 100644
--- a/tests/file_storage/tests.py
+++ b/tests/file_storage/tests.py
@@ -560,6 +560,26 @@ class FileFieldStorageTests(unittest.TestCase):
         with temp_storage.open('tests/stringio') as f:
             self.assertEqual(f.read(), b'content')
 
+    def test_filefield_name_updatable(self):
+        """
+        Test that the name attribute of a FileField can be changed to an existing file.
+        """
+        obj1 = Storage()
+        obj1.normal.save("django_test.txt", ContentFile("content"))
+
+        initial_path = obj1.normal.path
+        initial_name = obj1.normal.name
+        new_path = initial_path.replace("test.txt", "test2.txt")
+        new_name = initial_name.replace("test.txt", "test2.txt")
+        # Rename the underlying file object
+        os.rename(initial_path, new_path)
+        obj1.normal.name = new_name
+        obj1.save()
+
+        obj1 = Storage.objects.get(pk=obj1.pk)
+        self.assertEqual(obj1.normal.name, new_name)
+        self.assertEqual(obj1.normal.path, new_path)
+
 
 # Tests for a race condition on file saving (#4948).
 # This is written in such a way that it'll always pass on platforms

If confirmed, this should be of course made clearer in the documentation.

[Collin Anderson]

I agree setting .name is the way to go. Just needs to be better documented.

[Tim Graham <timograham@…>]

In 931a340:

Fixed #15590 -- Documented how the path of a FileField can be changed.

Thanks simon29 for report, and freakboy3742, floledermann,
jacob, claudep and collinanderson for discussing the task.

[Tim Graham <timograham@…>]

In 18f11072:

[1.7.x] Fixed #15590 -- Documented how the path of a FileField can be changed.

Thanks simon29 for report, and freakboy3742, floledermann,
jacob, claudep and collinanderson for discussing the task.

Backport of 931a340f1feca05b7a9f95efb9a3ba62b93b37f9 from master

[Tim Graham <timograham@…>]

In 2bddc74:

[1.8.x] Fixed #15590 -- Documented how the path of a FileField can be changed.

Thanks simon29 for report, and freakboy3742, floledermann,
jacob, claudep and collinanderson for discussing the task.

Backport of 931a340f1feca05b7a9f95efb9a3ba62b93b37f9 from master