Make the test client automatically encode JSON data

[Nick Sarbicki]

I stumbled on this while testing some code which expects a JSON body.

When creating a RequestFactory post request with a dict for the data and content_type='application/json' the resultant body is invalid JSON.

This is because force_bytes simply detects it is not a string and forces it to a string before encoding.

Code to reproduce is here:

In [1]: from django import test

In [2]: factory = test.RequestFactory()

In [3]: req = factory.post('/', content_type='application/json', data={"test": "data"})

In [4]: req.body
Out[4]: b"{'test': 'data'}"

And the offending code is here:

​https://github.com/django/django/blob/master/django/utils/encoding.py#L103

Assuming this is not an intended "feature" I will happily submit a patch adding _another_ isinstance line checking for dicts and encoding with json.dumps.

[Tom Forbes]

force_bytes is working as intended in this case, and we should definitely not coerce objects to json there. If this is accepted you should add any changes to the _encode_data() method in RequestFactory: ​https://github.com/django/django/blob/86de930f413e0ad902e11d78ac988e6743202ea6/django/test/client.py#L303

[Nick Sarbicki]

I can agree with moving it up a level. Although in that case maybe it is worth moving it even further upstream to post:

​https://github.com/django/django/blob/86de930f413e0ad902e11d78ac988e6743202ea6/django/test/client.py#L334

We already instantiate a dict here if there is no data - even if it is empty - so we are effectively expecting a json body by default.

encode_data is not used anywhere else at this point and as it is mostly concerned with encoding the data from a detected charset instead of mutating it's type you could argue that it is also working as expected.

However I would argue that .post is not working as expected.

[Tom Forbes]

Hmm good point. put, patch and delete would also need this functionality, not just post, so there might be a bit more to this. For example what encoder class should it use? Should we use the DjangoJSONEncoder from the serializations framework? How would (or should?) we let people customize this?

[Tim Graham]

In the absence of a fix, ​Stack Overflow (and other things I found on the web) suggest using data=json.dumps(python_dict) It may make sense to that conversion automatically as long as there's minimal chance of breaking existing code.

[Nick Sarbicki]

That is the current solution I am using.

Which leads me to say that for me I'd be happy using the default python json encoder - which is already imported.

Maybe we could add an _encode_json method akin to the _parse_json method in Client which takes kwargs from the calling method although I'm loathe to add a **kwargs argument just to pass it into a json encoder. Maybe a data_kwargs argument which takes a dict?

I'm of the opinion that not much customisation is required except maybe a custom serializer for some fields which you can do via json.dumps.

Assuming we are only converting dictionaries code breakage should be null unless people are intentionally ingesting broken JSON only.

[Adam Johnson]

Maybe we could add an _encode_json method akin to the _parse_json method in Client which takes kwargs from the calling method although I'm loathe to add a **kwargs argument just to pass it into a json encoder. Maybe a data_kwargs argument which takes a dict?

I think it's fine not worrying about the customization; _encode_json should be simple enough that users can subclass and replace if it doesn't work the way they want.

[Tom Forbes]

It would be nice if we could use the Django encoder by default, it does some pretty useful things around encoding datetimes, decimals and uuid's that I find pretty useful:

​https://github.com/django/django/blob/3e9aa298719f19d5f09dbe0df29b6bb8d2136229/django/core/serializers/json.py#L76

[Nick Sarbicki]

Fine by me, I have no particular attachment to the encoders and you are right, it is nice to have those extras.

With this accepted I'll try write up a patch tonight/tomorrow.

[Jonas Haag]

How about adding a new json parameter as in python-requests? From their docs:

>>> url = 'https://api.github.com/some/endpoint'
>>> payload = {'some': 'data'}

>>> r = requests.post(url, json=payload)

Dealing with requests regularly (and I'm sure I'm not the only one in the Python web world who does that) this has become the natural way to specify a JSON request body for me. On multiple occasions I just assumed there was a json parameter on the test client just to be disappointed :)

[Nick Sarbicki]

I like the idea but they're two different systems with different APIs, I don't want to add more to the interface just to make it feel like requests.

Especially as it would duplicate the keywords functionality with json and dicts.

Maybe it should be a separate ticket?

[Nick Sarbicki]

PR for the ticket ​https://github.com/django/django/pull/9636

[Nick Sarbicki]

PR updated for testing all routes.

[Tim Graham]

At first glance, I think adding a json_encoder argument to every HTTP method in the test client looks like more clutter than its worth. After all, a user can use data=json.dumps({...}, encoder=CustomClass) for that case, correct? That could be documented.

[Carlton Gibson]

Even if using a custom encoder, it's not something I want to pass every time, either calling the request method, or encoding the data explicitly.

Can we make it an attribute on RequestFactory (and so Client)?

_encode_json will use this directly, rather than take a parameter: ... json.dumps(data, cls=self.json_encoder_class)

When testing I set my custom encoder at the point of use (or subclass if I prefer):

factory = test.RequestFactory()
factory.json_encoder_class = MyCustomEncoder
req = factory.post('/', content_type='application/json', ...)

If we do this then we just need a test that the user can set a custom encoder.

[Nick Sarbicki]

I agree, and I like the property idea a lot more - good idea.

However, I'm not a fan of changing properties after instantiation.

So I'm going to put it as an __init__ kwarg. Still have the option of changing it post instantiation but you can do it in one line if you want.

factory = test.RequestFactory(json_encoder=MyCustomEncoder)
req = factory.post('/', content_type='application/json', ...)

[Nick Sarbicki]

I was also pondering putting the if content_type == JSON_CONTENT statement within the _encode_json method.

But I don't think it would be clear enough that we won't actually encode JSON if the content_type is something else.

[Nick Sarbicki]

PR updated to reflect the above.

[Carlton Gibson]

All good. I think add the json_encoder argument to __init__ works well.

I'm going to mark as RFC. Good one!

[Nick Sarbicki]

Git went a bit crazy so moved to a new PR here: ​https://github.com/django/django/pull/9645

[Tim Graham <timograham@…>]

In 47268242:

Fixed #29082 -- Allowed the test client to encode JSON request data.