[Django] #30735: Testing client encode_multipart may also support dict format
Django
---------------------------------------------+------------------------
Reporter: ychab | Owner: nobody
Type: New feature | Status: new
Component: Testing framework | Version: 2.2
Severity: Normal | Keywords: test
Triage Stage: Unreviewed | Has patch: 1
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
---------------------------------------------+------------------------
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
--
Ticket URL: <https://code.djangoproject.com/ticket/30735>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: ychab | Owner: ychab
Type: New feature | Status: assigned
Component: Testing framework | Version: 2.2
Keywords: test | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by ychab):
* status: new => assigned
* owner: nobody => ychab
--
Ticket URL: <https://code.djangoproject.com/ticket/30735#comment:1>
Django
Reporter: ychab | Owner: ychab
Type: New feature | Status: assigned
Keywords: test | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Changes (by ychab):
* needs_tests: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/30735#comment:2>
Django
Reporter: ychab | Owner: ychab
Type: New feature | Status: assigned
Keywords: test | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by ychab):
Here is the merge request, awaiting tests I can done if everything is ok:
https://github.com/django/django/pull/11720/files
Thanks
--
Ticket URL: <https://code.djangoproject.com/ticket/30735#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Yannick Chabbert | Owner: Yannick
| Chabbert
Type: New feature | Status: closed
Component: Testing framework | Version: master
Severity: Normal | Resolution: wontfix
Keywords: test | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Changes (by felixxm):
* status: assigned => closed
* version: 2.2 => master
* resolution: => wontfix
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/30735#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Yannick Chabbert | Owner: Yannick
| Chabbert
Type: New feature | Status: closed
Component: Testing framework | Version: master
Severity: Normal | Resolution: wontfix
| Unreviewed
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by 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).
--
Ticket URL: <https://code.djangoproject.com/ticket/30735#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Yannick Chabbert | Owner: Yannick
| Chabbert
Type: New feature | Status: closed
Component: Testing framework | Version: master
Severity: Normal | Resolution: wontfix
| Unreviewed
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by Yannick Chabbert):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/30735#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Yannick Chabbert | Owner: Yannick
| Chabbert
Type: New feature | Status: closed
Component: Testing framework | Version: master
Severity: Normal | Resolution: wontfix
| Unreviewed
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by Yannick Chabbert):
Replying to [comment:5 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 !
--
Ticket URL: <https://code.djangoproject.com/ticket/30735#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Yannick Chabbert | Owner: Yannick
| Chabbert
Type: New feature | Status: closed
Component: Testing framework | Version: master
Severity: Normal | Resolution: wontfix
| Unreviewed
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by Yannick Chabbert):
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!
--
Ticket URL: <https://code.djangoproject.com/ticket/30735#comment:8>