#32243 closed Cleanup/optimization (fixed)
Clarify docs on manually setting and saving a FileField.
Reported by: | Gordon Wrigley | Owned by: | Joshua Massover |
---|---|---|---|
Component: | Documentation | Version: | 3.1 |
Severity: | Normal | Keywords: | |
Cc: | Joshua Massover | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
The documentation talks about how to set a filefield on a new object and how to set a filefield using a form but I couldn't find anything that talked about how to directly set a filefield on an existing object.
https://docs.djangoproject.com/en/3.1/topics/http/file-uploads/#handling-uploaded-files-with-a-model
Maybe those docs could use some extra detail on how you can set an existing file field by either assigning a ContentFile (with a name) to the field and saving the instance, or by calling save on the field and passing a ContentFile and a name. And particularly how the name is required in both cases and will be passed through upload_to.
Further figuring this out for myself was substantially complicated by the following behaviour:
In [24]: e=MyModel.objects.first() In [25]: e.my_file Out[25]: <FieldFile: None> In [26]: e.my_file=ContentFile(content=b"fred") In [27]: e.save() In [28]: e=MyModel.objects.first() In [29]: e.my_file Out[29]: <FieldFile: None> In [30]: e.my_file=ContentFile(content=b"bob", name="bob.txt") In [31]: e.save() In [32]: e=MyModel.objects.first() In [33]: e.my_file Out[33]: <FieldFile: files/5bc2fe4c-4262-4134-9397-c740de5a7edf/bob.txt> In [34]: e.my_file.open().read() Out[34]: b'bob'
Particularly 26-29 where setting the filefield to a ContentFile with no name and then saving is effectively just ignored with no error.
Is this expected behaviour?
Change History (18)
comment:1 by , 4 years ago
Component: | Uncategorized → Documentation |
---|---|
Type: | Uncategorized → Cleanup/optimization |
comment:2 by , 4 years ago
Triage Stage: | Unreviewed → Accepted |
---|
follow-up: 5 comment:3 by , 4 years ago
Summary: | Saving filefields. → Clarify docs on manually setting and saving a FileField. |
---|
comment:4 by , 4 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
follow-up: 6 comment:5 by , 4 years ago
Hi Carlton,
As the file is not being saved without the name
attribute of the file, shouldn't we generate the name
property randomly if not provided by the user?
If yes, I will update the relevant codes implementing random name generation, otherwise, I will update the documentation mentioning name
is required. However, I think we should not enforce the user to provide the name
attribute. I would prefer to update the code.
comment:6 by , 4 years ago
Replying to Hasanul Islam:
Hi Carlton,
As the file is not being saved without thename
attribute of the file, shouldn't we generate thename
property randomly if not provided by the user?
If yes, I will update the relevant codes implementing random name generation, otherwise, I will update the documentation mentioning
name
is required. However, I think we should not enforce the user to provide thename
attribute. I would prefer to update the code.
TL;DR: Allow Files (not just ContentFiles) with a blank/None name. In FileField.save()
, add a check after the generate_filename()
call to make sure the file actually has a name once we're ready to save it to disk.
When saving the model it seems to still use the upload_to
argument. If you pass something for upload_to
it will overwrite whatever was passed for name
.
Here's an example of my use case. A user can upload a picture through the admin panel. Whenever a new picture is uploaded I create a thumbnail for it.
def photo_upload_path(instance, original_filename): filename = hashlib.sha256(str(time.time()).encode("utf-8")).hexdigest() filename = filename[:16] return f"photos/{filename}.jpg" def thumbnail_upload_path(instance, original_filename): filename = os.path.basename(instance.photo.name) return f"photos/thumbnails/{filename}" class MyModel(models.Model): photo = models.ImageField(upload_to=photo_upload_path) # Hidden in Django admin. Managed by us. photo_thumbnail = models.ImageField(upload_to=thumbnail_upload_path) def save(self): img = Image.open(self.photo.path) img.thumbnail(settings.THUMBNAIL_SIZE) contents = io.BytesIO() img.save(contents, "JPEG") self.photo_thumbnail = ContentFile(contents.getvalue(), name="foo") super.save()
In this case, the photo gets renamed to photos/<hash>.jpg
, and I want thumbnails to go to photos/thumbnails/<hash>.jpg
.
I resize the image, save its contents to a BytesIO object, and create the ContentFile object. However I have to pass in a bogus name for the ContentFile even though it will use the upload_to
to overwrite it. If I pass an empty name Django seems to throw the file away and it doesn't get saved.
comment:7 by , 3 years ago
#33388 was closed as a duplicate to this one. I included an example but not in the Using files in models as Carlton suggested, I put it in [https://docs.djangoproject.com/en/3.1/topics/http/file-uploads/#handling-uploaded-files-with-a-model Handling uploaded files with a model]. Glad to move it where ever if this does not suffice.
comment:8 by , 3 years ago
Cc: | added |
---|---|
Has patch: | set |
Owner: | changed from | to
comment:13 by , 3 years ago
Patch needs improvement: | unset |
---|
comment:14 by , 3 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
Hi Gordon. Thanks for the report.
I agree this is an area that folks find confusing. Happy to review suggested changes.
I think the right place for a clarification would be in the Using files in models section of the Files topic documentation.
Perhaps a review of the tests in tests/files to ensure we've covered all the relevant cases would also be in order.