#30735 closed New feature (wontfix)
Testing client encode_multipart may also support dict format.
Reported by: | Yannick Chabbert | Owned by: | Yannick Chabbert |
---|---|---|---|
Component: | Testing framework | Version: | dev |
Severity: | Normal | Keywords: | test |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | yes | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description
When using implicitly multipart-form data with the testing client (post / put methods), we can't unfortunetly provide data like dict, whereas we could provide file, list and even list of files...
Of course, the actual workaround is to do it manually (cast dict to json) but it should be really great if it could be done automatically!
Furthermore, some third party libraries like Django Rest Framework are limited by this, see https://github.com/encode/django-rest-framework/blob/3.10.2/rest_framework/renderers.py#L907
Concretly with DRF, we would be able to test file upload with nested data on an endpoint at the same time. I highlight test because otherwise in real life, it works if the HTTP client send the dict data as a json string.
Here is just a basic example, with the native Django test client where we can't do this:
with open(filepath, 'rb') as fp: response = self.client.post('/post/', data={ 'nested_data': { 'firstname': 'foo', 'lastname': 'foo', }, 'testfile': fp, })
Indeed instead, we have to cast it manually:
import json with open(filepath, 'rb') as fp: response = self.client.post('/post/', data={ 'nested_data': json.dumps({ 'firstname': 'foo', 'lastname': 'foo', }), 'testfile': fp, })
In that case, it would be ok and nested data would be properly parsed.
Finally, the feature request should not be so hard and I have already a working patch. However, I need first to know if it is an acceptable feature request? Am I missing something else?
Thanks for the review
Change History (8)
comment:1 by , 5 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:2 by , 5 years ago
Needs tests: | set |
---|
comment:3 by , 5 years ago
follow-up: 6 comment:4 by , 5 years ago
Resolution: | → wontfix |
---|---|
Status: | assigned → closed |
Summary: | Testing client encode_multipart may also support dict format → Testing client encode_multipart may also support dict format. |
Version: | 2.2 → master |
Thanks for this report, however you cannot provide nested dicts because as far as I'm concerned it is not a RFC compliant. I don't think that Django should assume that users want to test JSON data. Moreover I don't see how DRF is limited by Django in this area, assertion that you pointed is rather a conscious choice.
follow-up: 7 comment:5 by , 5 years ago
Aside: if you were to propose the adjustment here to DRF, we'd say something like, "subclass MultiPartRenderer to automatically convert dicts to JSON, if that's what you know you want for your project". I'd suggest that as a way forward, although, I'd probably advise just keeping the json.dumps()
inline, since it's nicely visible there, and not much of a hardship... (HTH).
comment:6 by , 5 years ago
Thanks for the quick answer. I agree with you and it clearly needs more search before suggesting an helper like this.
What I'm still not sure and I still don't understand yet is why it works in real life (sending data dict as json string along with file)!!?
- It is the DRF parser which magically detect that the string is a json format and thus, deserialize it?
- Or it happen elsewhere, at a lower level in Django itself?
- This parsing behaviour is wanted or just a side effect?
- Afterall, even if it work, is it RFC compliant to send json data as string with multipart content type?
The big deal to me is not really to have this helper just like for list or file (because finally, we could do it manually) but to understand why this exception is raised and if there is a good reason to do this? If true, it means that we should't do this and parser should be fixed to failed instead. Otherwise, exception shouldn't be raised and give an helper to dumps data as json at a lowest level in Django (that's why I open this issue in the Django project and not in DRF)...
Indeed at the begining, I was thinking: "ok, test failed because an exception is raised to tell me it is not possible". Sad story but ok, this is it. But after discovering that it works in real life, I'm saying: "Oooh good catch! In fact, it works!! So this exception is maybe wrong??"
I will try to investigate more deeply on it and give some feedbacks.
comment:7 by , 5 years ago
Replying to Carlton Gibson:
Aside: if you were to propose the adjustment here to DRF, we'd say something like, "subclass MultiPartRenderer to automatically convert dicts to JSON, if that's what you know you want for your project". I'd suggest that as a way forward, although, I'd probably advise just keeping the
json.dumps()
inline, since it's nicely visible there, and not much of a hardship... (HTH).
Didn't see your comment, good suggestion thanks !
comment:8 by , 5 years ago
Ok, I can confirm that it work because this is an expected behavior in DRF.
Request is first parsed by the Django MutiPartParser
which just extract the raw json string: https://github.com/django/django/blob/master/django/http/multipartparser.py#L196
It is then handled by the corresponding serializer field (DRF) by implementing the {{to_internal_value}} method.
RFC compliant or not (I still didn't know but finally, a string is a string, no matter what it contains!), this is not an issue with Django and sorry for the bad report!
Here is the merge request, awaiting tests I can done if everything is ok: https://github.com/django/django/pull/11720/files
Thanks