json: do not treat an empty body as an error

[etix]

Sending an empty json body when no fields are mandatory should not result in a "Bad request".

[mholt]

Thanks @etix!

I have been really busy lately. Could you do me a favor and write a test case to assert this does what we expect? My initial inspection makes me think it's good, but I just want to be sure. For example, are we sure that this only affects structs where no fields are mandatory? Even if it does, is it necessary? (In other words, on the very next line, the validation middleware iterates the struct and will map errors for every required field that is a zero value. Since the body was empty, the struct is all zero values, so the required ones cause an error.)

My final thought is: is a request that's expected to have a JSON body, that actually doesn't have a body, an error?

The tweak looks good otherwise! I just want to make sure we nail down the expected behavior of the library as we go. These are great issues to figure out.

[etix]

Hello @mholt and thanks for the review!

First a bit of context, I'm using martini (and its binding module) to develop an API were some endpoints only contain optional fields. For example imagine a GET /something with a count field which is defined on the spec to have a default value, a payload is therefore not necessary. Without the patch sending no payload results in a Bad request (which is misleading in this case). Furthermore because it ends with an error 400 you don't get the list of errors encountered by the validator and thus you don't know which fields were mandatory! Not very user-friendly for an API, you'll agree. Finally it's worth noting that this behavior is already accepted by binding.Form but wasn't for binding.Json.

Doing associated tests were not as easy as adding few testCase elements because of the current structure which was not designed to handle empty payloads at all but still I gave a quick try (which is probably not very clean I admit). Suggestions on this matter are welcome since I'm quite new to martini :)

For example, are we sure that this only affects structs where no fields are mandatory?

Yes, mandatory fields will break with an error similar to: binding_test.go:128: {method:GET path: payload: contentType: ok:true ref:0x9c1de0} should be OK (0 errors), but had errors: {Overall:map[] Fields:map[Title:Required]}

Even if it does, is it necessary?

Indeed, that's an interesting question. Some people may want to completely bypass all requirements because an empty payload can be a special case for a getter maybe, but since I don't have this need we'd better wait for someone who badly needs it.

My final thought is: is a request that's expected to have a JSON body, that actually doesn't have a body, an error?

IMHO it's not, at least for the specific case of an API that you might want to hit with curl, sending no payload can still give you welcomed insight about which fields you missed.

[mholt]

I'm convinced. An empty payload still won't have values for the required fields, which will map errors to the context, so I think that's covered. Thanks for your contribution!