Add support for serializing top-level arrays to JSON

[jeffwidman]

Major discussion of the issue is in #248.

I'm tired of working around it, so just like to get it fixed.

This solution is similar to #1402 & #1209 with a few changes:

• I found it unexpected that in Add support for toplevel arrays #1402 to convert a python list to a top level array you had to pass jsonify(*l). I changed it so that both jsonify(1,2,3) and jsonify([1,2,3]) serialize to [1,2,3] as I think that's the more intuitive behavior.
• Per the discussion in Handle a single unusual argument in jsonify #1209 & Review JSON Security #248, I didn't create a separate config variable to modify this behavior.

I know it'd be nice to incorporate a fix for #1443 in here as well, but I don't fully understand the issue there. So let's get this merged, and then tackle that issue in a followup PR.

Fix #170, #248, #510, #673, #1177 and check one of the boxes on #1182

[ThiefMaster]

Shouldn't jsonify with both args and kwargs fail?

[jeffwidman]

I don't know.

I was just going off of the current implementation (data = dict(*args, **kwargs)) which doesn't look like it will complain if passed both args and kwargs.

Happy to change that if it should.

[ThiefMaster]

I wonder if the starargs-like syntax is actually useful.. If you have a fixed/low number of list entries, chances are good that you should be using a dict instead.

[jeffwidman]

If you have a fixed/low number of list entries, chances are good that you should be using a dict instead.

Why is that?

I've got API endpoints like /categories/ and /users/ that I want to return lists of items. I don't understand why a dict would make more sense than an array even if I've only got 4-5 categories.

[ThiefMaster]

No, I meant the cases where you would do jsonify(1,2,3) - if it's a list of users, you pass a list there, but you won't do jsonify(u1, u2, u3)

[jeffwidman]

Agreed.

json.dumps() only accepts the first item and appears to silently drop the others, so I considered copying that. It might be more natural as most people are used to the dumps() API.

However, jsonify() currently accepts **kwargs, so I decided handling *args this way was more internally consistent.

Overall, I think the divergence in accepted input between json.dumps() and jsonify() creates more confusion than convenience. Right now I sometimes forget which one supports kwargs and which supports arrays. And I'm not the only one who has trouble remembering.

Splitting behavior so jsonify() rejects *args but happily handles **kwargs just adds to the confusion, so we should pick either of the following:

1. Make jsonify() internally consistent so it accepts both *args and **kwargs.
2. Make jsonify() consistent with dumps(), meaning they both only accept a single argument. This implies a long term plan to deprecate jsonify(**kwargs) so that it throws an exception just like dumps(**kwargs).

I don't care strongly, but if I had to pick I'd choose the latter because it's simpler to think of jsonify() as a json.dumps() that adds Flask niceties like the response class and serializing datetimes/UUIDs.

[justanr]

👍 on being consistent with json.dumps, especially since it's a widely copied API.

[ThiefMaster]

BTW, I think the reason why jsonify uses *args instead of just a single obj argument is to allow an argument with that name name to be used as a kwarg.

[mitsuhiko]

Maybe the solution to this mess is to introduce a new function that just invokes json.dumps and makes a response. Same behavior as json.dumps but with different name and makes a response object.

[jeffwidman]

Maybe the solution to this mess is to introduce a new function that just invokes json.dumps and makes a response. Same behavior as json.dumps but with different name and makes a response object.

Is there any reason to create a new function vs just modifying the existing jsonify() function?

It breaks backwards compatibility, but for a 1.0 release that should be fine. Also, I know in my own projects, I've never intentionally used jsonify(**kwargs), I've just always passed it a dict, and I suspect most people use it similarly.

To me, the path of least surprise is to stay with the current jsonify() function name and remove the **kwargs support to be consistent with dumps().

The other nicety that jsonify() currently has is serializing datetimes & UUIDs. If they weren't already in there, I wouldn't add them, but since they are in there, I'd just leave it in.

[ThiefMaster]

Breaking kwargs in jsonify would be a terrible idea. It's the kind of breakage that provides pretty much no real value and just annoys people (including myself ;)).

While passing **kwargs literally is indeed rate, jsonify(some=thing, other=thing) is pretty common.

[jeffwidman]

I hear you. :-)

My vote is to stick with my original implementation where both jsonify(1,2,3) and jsonify([1,2,3]) serialize to [1,2,3]. That behavior is a bit more loose than dumps(), but I think it's more convenient for the user and unlikely to trip up anyone unexpectedly. If anything, copying dumps(1,2,3) by serializing jsonify(1,2,3) to 1 is more likely to trip up someone.

Shouldn't jsonify with both args and kwargs fail?

Still a very valid question, but I don't want it to delay merging this PR. For now, let's just stick with Flask's current behavior which is to accept both args and kwargs and serialize them to a dict. We can discuss changing that behavior in a separate issue or PR.

[jeffwidman]

Any other feedback @mitsuhiko @davidism ?

Would like to see this merged so I don't have to keep working around it.

Supporting top-level arrays was already discussed ad nauseum in the issues I linked to in the OP, so IMHO this PR should be relatively non-controversial since it's just implementing behavior that already has been decided on.

[ThiefMaster]

What about a test case for all the important cases on how to call jsonify? It'd be a shame to break any of them (kwargs / dict) without noticing it at some point

[jeffwidman]

Good idea. Let me see what I can do.

[davidism]

This needs a test case and docs to demonstrate exactly what is accepted by the function now. The docs for the jsonify function can be improved.

Note that as a side effect, simple types are accepted as well (True, 3.14, etc.), so there should be a test/doc for that too. Test both a single and multi character string.

Not sure if I like that *args will be treated as an array, if it was me I'd make it always explicit, but it's not a huge deal.

[jeffwidman]

Some updates:

1. Added tests.
2. Moved around the existing jsonify() tests so they're located next to each other--before they were a little scattered. Other than relocating, I made minimal changes to existing tests.
3. Expanded the docs for jsonify(). I'm not very familiar with RST/sphinx, so there might be some formatting issues I missed.
4. Restructured the code handling the *args/**kwargs to be more explicit and linear. Previously, it wasn't always clear which combinations of *args/**kwargs would throw an exception and cause the try/except block to switch from dict to array.
5. As part of this restructuring, made it so passing args + kwargs at the same time to jsonify() will fail as suggested by @ThiefMaster. After thinking about it more, I agree that this is the least surprising outcome rather than silently dropping either the args or kwargs.

Feedback welcome.

[jeffwidman]

@davidism any chance you can review this weekend?

[davidism]

I'll get to this on Sunday.

[davidism]

OK, Monday morning. 😉 Thanks for your work on this. I might clean up the wording a bit at some point, but other than that this looks good.

[jeffwidman]

Thanks for merging @davidism, really nice to see this finally land. Also, improvements to my wording are always appreciated.