Clarify ToXContent contract

[nik9000]

ElasticsearchException implements ToXContent but it doens't look like it works with XContentBuilder#writeValue because ElasticsearchException doesn't output its start and end object markers. Is this a bug? If so we should probably fix it and add more comments around toXContent's contract and probably make some simple way to test that implementers comply with the contract.

[bleskes]

This is a good one. The reason why some ToXContent implementations don't call start and end object is that sometime you want to output within another context, where you for example have added some information for context. For an example look at org.elasticsearch.rest.BytesRestResponse#convert where we enrich the top level error. I'm not saying this is the only solution, but this is why it's like that...

[nik9000]

Thanks for clarifying. I think it'd be more clear if we had two interfaces for the different contracts. We frequently use "innerToXContent" when we want to support this. I get that we have a bazillion interfaces already but it'd be nice to know more about the things that implement ToXContent. We could even have consistent tests for them!

[javanna]

Would be great to clarify and clean up this contract for sure, in this case using a class that extends ToXContent doesn't say enough, you have to look at the implementation to see if you have to open a new object yourself or not.

[javanna]

We have been encountering this inconsistency while writing parsing code for the High Level REST client. We could fix it as we go through all the toXContent methods (in separate PRs would be wise). I see that most of the objects don't open a start object, and output essentially fragments that are not valid without an ancestor. On the other hand, our ActionResponses should always print out a complete object without the need to start and end the object externally, which some REST actions do and some others do not.

How about having ToXContent for complete objects and ToXContentFragment for fragments?