Re: [Django] #15590: FileField path isn't settable
Django
-------------------------------------+-------------------------------------
Reporter: simon29 | Owner: simon29
Type: New | Status: new
feature | Component: File
Milestone: 1.4 | uploads/storage
Version: SVN | Severity: Normal
Resolution: | Keywords: filefield
Triage Stage: Design | imagefield path filename
decision needed | Has patch: 1
Needs documentation: 1 | Needs tests: 1
Patch needs improvement: 1 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Changes (by floledermann):
* cc: ledermann@… (added)
* ui_ux: => 0
* easy: => 0
Comment:
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?
--
Ticket URL: <https://code.djangoproject.com/ticket/15590#comment:5>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: simon29 | Owner: simon29
Component: File | Version: SVN
uploads/storage | Resolution:
Severity: Normal | Triage Stage: Design
Keywords: filefield | decision needed
imagefield path filename | Needs documentation: 1
Has patch: 1 | Patch needs improvement: 1
Needs tests: 1 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by kitsunde):
* cc: kitsunde@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/15590#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: simon29 | Owner: simon29
Type: New feature | Status: new
uploads/storage | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: filefield | Needs documentation: 1
imagefield path filename | Patch needs improvement: 1
Has patch: 1 | UI/UX: 0
Needs tests: 1 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by jacob):
* stage: Design decision needed => Accepted
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/15590#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: simon29 | Owner: simon29
Type: New feature | Status: new
Component: File | Version: master
uploads/storage | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: filefield | Needs documentation: 1
imagefield path filename | Patch needs improvement: 1
Has patch: 1 | UI/UX: 0
Needs tests: 1 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by simon29):
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).
--
Ticket URL: <https://code.djangoproject.com/ticket/15590#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: simon29 | Owner: simon29
Type: New feature | Status: new
Component: File | Version: master
uploads/storage | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: filefield | Needs documentation: 1
imagefield path filename | Patch needs improvement: 1
Has patch: 1 | UI/UX: 0
Needs tests: 1 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by claudep):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/15590#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: simon29 | Owner: simon29
Type: New feature | Status: new
Severity: Normal | Resolution:
Keywords: filefield | Triage Stage: Accepted
imagefield path filename |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by collinanderson):
* cc: cmawebsite@… (added)
* needs_better_patch: 1 => 0
* component: File uploads/storage => Documentation
* needs_tests: 1 => 0
* easy: 0 => 1
* needs_docs: 1 => 0
* has_patch: 1 => 0
Comment:
I agree setting `.name` is the way to go. Just needs to be better
documented.
--
Ticket URL: <https://code.djangoproject.com/ticket/15590#comment:11>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: assigned
Component: Documentation | Version: master
Severity: Normal | Resolution:
Keywords: filefield | Triage Stage: Accepted
imagefield path filename |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: simon29 => timgraham
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/15590#comment:12>
Django
-------------------------------------+-------------------------------------
Reporter: simon29 | Owner:
Severity: Normal | Resolution:
Keywords: filefield | Triage Stage: Accepted
imagefield path filename |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by timgraham):
* status: assigned => new
* owner: timgraham =>
--
Ticket URL: <https://code.djangoproject.com/ticket/15590#comment:13>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: filefield | Triage Stage: Accepted
imagefield path filename |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: => jorgebg
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/15590#comment:14>
Django
-------------------------------------+-------------------------------------
Reporter: simon29 | Owner: jorgebg
Component: Documentation | Version: master
Severity: Normal | Resolution: fixed
Keywords: filefield | Triage Stage: Accepted
imagefield path filename |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"931a340f1feca05b7a9f95efb9a3ba62b93b37f9" 931a340]:
{{{
#!CommitTicketReference repository=""
revision="931a340f1feca05b7a9f95efb9a3ba62b93b37f9"
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.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/15590#comment:15>
Django
-------------------------------------+-------------------------------------
Reporter: simon29 | Owner: jorgebg
Type: New feature | Status: closed
Component: Documentation | Version: master
Severity: Normal | Resolution: fixed
Keywords: filefield | Triage Stage: Accepted
imagefield path filename |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"18f11072b8892064d002f3b873c576778082f748" 18f11072]:
{{{
#!CommitTicketReference repository=""
revision="18f11072b8892064d002f3b873c576778082f748"
[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
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/15590#comment:16>
Django
-------------------------------------+-------------------------------------
Reporter: simon29 | Owner: jorgebg
Type: New feature | Status: closed
Component: Documentation | Version: master
Severity: Normal | Resolution: fixed
Keywords: filefield | Triage Stage: Accepted
imagefield path filename |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"2bddc74b42dd427711385e8a3e0b72c42f2d7b36" 2bddc74]:
{{{
#!CommitTicketReference repository=""
revision="2bddc74b42dd427711385e8a3e0b72c42f2d7b36"
[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
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/15590#comment:17>