introduce ToXContentObject interface

`ToXContentObject` extends `ToXContent` without adding new methods to it, while allowing to mark classes that output complete xcontent objects to distinguish them from classes that require starting and ending an anonymous object externally.

Ideally ToXContent would be renamed to ToXContentFragment, but that would be a huge change in our codebase, hence we simply document the fact that toXContent outputs fragments with no guarantees that the output is valid per se without an external ancestor.

Relates to #16347

core/src/main/java/org/elasticsearch/action/admin/cluster/health/ClusterHealthResponse.java

@@ -28,6 +28,7 @@ import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.common.xcontent.StatusToXContent;
+import org.elasticsearch.common.xcontent.ToXContentObject;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.rest.RestStatus;
@@ -36,7 +37,7 @@ import java.io.IOException;
 import java.util.Locale;
 import java.util.Map;
 
-public class ClusterHealthResponse extends ActionResponse implements StatusToXContent {
+public class ClusterHealthResponse extends ActionResponse implements StatusToXContent, ToXContentObject {
     private String clusterName;
     private int numberOfPendingTasks = 0;
     private int numberOfInFlightFetch = 0;
@@ -240,6 +241,7 @@ public class ClusterHealthResponse extends ActionResponse implements StatusToXCo
 
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
+        builder.startObject();
         builder.field(CLUSTER_NAME, getClusterName());
         builder.field(STATUS, getStatus().name().toLowerCase(Locale.ROOT));
         builder.field(TIMED_OUT, isTimedOut());
@@ -268,6 +270,7 @@ public class ClusterHealthResponse extends ActionResponse implements StatusToXCo
             }
             builder.endObject();
         }
+        builder.endObject();
         return builder;
     }
 }

core/src/main/java/org/elasticsearch/action/admin/cluster/node/tasks/list/ListTasksResponse.java

@@ -214,6 +214,6 @@ public class ListTasksResponse extends BaseTasksResponse implements ToXContent {
 
     @Override
     public String toString() {
-        return Strings.toString(this, true);
+        return Strings.toString(this);
     }
 }

core/src/main/java/org/elasticsearch/action/bulk/BulkItemResponse.java

@@ -179,7 +179,7 @@ public class BulkItemResponse implements Streamable, StatusToXContent {
 
         @Override
         public String toString() {
-            return Strings.toString(this, true);
+            return Strings.toString(this);
         }
     }
 

core/src/main/java/org/elasticsearch/action/get/GetResponse.java

@@ -194,6 +194,6 @@ public class GetResponse extends ActionResponse implements Iterable<GetField>, T
 
     @Override
     public String toString() {
-        return Strings.toString(this, true);
+        return Strings.toString(this);
     }
 }

core/src/main/java/org/elasticsearch/action/index/IndexResponse.java

@@ -57,7 +57,7 @@ public class IndexResponse extends DocWriteResponse {
         builder.append(",version=").append(getVersion());
         builder.append(",result=").append(getResult().getLowercase());
         builder.append(",seqNo=").append(getSeqNo());
-        builder.append(",shards=").append(Strings.toString(getShardInfo(), true));
+        builder.append(",shards=").append(Strings.toString(getShardInfo()));
         return builder.append("]").toString();
     }
 

core/src/main/java/org/elasticsearch/action/ingest/GetPipelineResponse.java

@@ -23,6 +23,7 @@ import org.elasticsearch.action.ActionResponse;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.xcontent.StatusToXContent;
+import org.elasticsearch.common.xcontent.ToXContentObject;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.ingest.PipelineConfiguration;
 import org.elasticsearch.rest.RestStatus;
@@ -31,7 +32,7 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
 
-public class GetPipelineResponse extends ActionResponse implements StatusToXContent {
+public class GetPipelineResponse extends ActionResponse implements StatusToXContent, ToXContentObject {
 
     private List<PipelineConfiguration> pipelines;
 
@@ -76,9 +77,11 @@ public class GetPipelineResponse extends ActionResponse implements StatusToXCont
 
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
+        builder.startObject();
         for (PipelineConfiguration pipeline : pipelines) {
             builder.field(pipeline.getId(), pipeline.getConfigAsMap());
         }
+        builder.endObject();
         return builder;
     }
 }

core/src/main/java/org/elasticsearch/action/search/ClearScrollResponse.java

@@ -23,6 +23,7 @@ import org.elasticsearch.action.ActionResponse;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.xcontent.StatusToXContent;
+import org.elasticsearch.common.xcontent.ToXContentObject;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.rest.RestStatus;
 
@@ -31,7 +32,7 @@ import java.io.IOException;
 import static org.elasticsearch.rest.RestStatus.NOT_FOUND;
 import static org.elasticsearch.rest.RestStatus.OK;
 
-public class ClearScrollResponse extends ActionResponse implements StatusToXContent {
+public class ClearScrollResponse extends ActionResponse implements StatusToXContent, ToXContentObject {
 
     private boolean succeeded;
     private int numFreed;
@@ -66,8 +67,10 @@ public class ClearScrollResponse extends ActionResponse implements StatusToXCont
 
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
+        builder.startObject();
         builder.field(Fields.SUCCEEDED, succeeded);
         builder.field(Fields.NUMFREED, numFreed);
+        builder.endObject();
         return builder;
     }
 

core/src/main/java/org/elasticsearch/action/search/SearchResponse.java

@@ -231,6 +231,6 @@ public class SearchResponse extends ActionResponse implements StatusToXContent {
 
     @Override
     public String toString() {
-        return Strings.toString(this, true);
+        return Strings.toString(this);
     }
 }

core/src/main/java/org/elasticsearch/action/support/replication/ReplicationResponse.java

@@ -29,6 +29,7 @@ import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.io.stream.Streamable;
 import org.elasticsearch.common.xcontent.ToXContent;
+import org.elasticsearch.common.xcontent.ToXContentObject;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.index.shard.ShardId;
@@ -250,7 +251,7 @@ public class ReplicationResponse extends ActionResponse {
             return shardInfo;
         }
 
-        public static class Failure implements ShardOperationFailedException, ToXContent {
+        public static class Failure implements ShardOperationFailedException, ToXContentObject {
 
             private static final String _INDEX = "_index";
             private static final String _SHARD = "_shard";

core/src/main/java/org/elasticsearch/common/Strings.java

@@ -25,6 +25,7 @@ import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.io.FastStringReader;
 import org.elasticsearch.common.util.CollectionUtils;
 import org.elasticsearch.common.xcontent.ToXContent;
+import org.elasticsearch.common.xcontent.ToXContentObject;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.json.JsonXContent;
 
@@ -857,26 +858,17 @@ public class Strings {
     }
 
     /**
-     * Return a {@link String} that is the json representation of the provided
+     * Return a {@link String} that is the json representation of the provided {@link ToXContent}.
-     * {@link ToXContent}.
+     * Wraps the output into an anonymous object.
      */
     public static String toString(ToXContent toXContent) {
-        return toString(toXContent, false);
-    }
-
-    /**
-     * Return a {@link String} that is the json representation of the provided
-     * {@link ToXContent}.
-     * @param wrapInObject set this to true if the ToXContent instance expects to be inside an object
-     */
-    public static String toString(ToXContent toXContent, boolean wrapInObject) {
         try {
             XContentBuilder builder = JsonXContent.contentBuilder();
-            if (wrapInObject) {
+            if (toXContent.isFragment()) {
                 builder.startObject();
             }
             toXContent.toXContent(builder, ToXContent.EMPTY_PARAMS);
-            if (wrapInObject) {
+            if (toXContent.isFragment()) {
                 builder.endObject();
             }
             return builder.string();

core/src/main/java/org/elasticsearch/common/xcontent/ToXContent.java

@@ -26,6 +26,8 @@ import java.util.Map;
 
 /**
  * An interface allowing to transfer an object to "XContent" using an {@link XContentBuilder}.
+ * The output may or may not be a value object. Objects implementing {@link ToXContentObject} output a valid value
+ * but those that don't may or may not require emitting a startObject and an endObject.
  */
 public interface ToXContent {
 
@@ -126,4 +128,8 @@ public interface ToXContent {
     }
 
     XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException;
+
+    default boolean isFragment() {
+        return true;
+    }
 }

core/src/main/java/org/elasticsearch/common/xcontent/ToXContentObject.java

@@ -0,0 +1,34 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.common.xcontent;
+
+/**
+ * An interface allowing to transfer an object to "XContent" using an {@link XContentBuilder}.
+ * The difference between {@link ToXContent} and {@link ToXContentObject} is that the former may output a fragment that
+ * requires to start and end a new anonymous object externally, while the latter guarantees that what gets printed
+ * out is fully valid syntax without any external addition.
+ */
+public interface ToXContentObject extends ToXContent {
+
+    @Override
+    default boolean isFragment() {
+        return false;
+    }
+}

core/src/main/java/org/elasticsearch/common/xcontent/XContentHelper.java

@@ -378,19 +378,18 @@ public class XContentHelper {
 
     /**
      * Returns the bytes that represent the XContent output of the provided {@link ToXContent} object, using the provided
-     * {@link XContentType}. Wraps the output into a new anonymous object depending on the value of the wrapInObject argument.
+     * {@link XContentType}. Wraps the output into a new anonymous object.
      */
-    public static BytesReference toXContent(ToXContent toXContent, XContentType xContentType, boolean wrapInObject) throws IOException {
+    public static BytesReference toXContent(ToXContent toXContent, XContentType xContentType) throws IOException {
         try (XContentBuilder builder = XContentBuilder.builder(xContentType.xContent())) {
-            if (wrapInObject) {
+            if (toXContent.isFragment()) {
                 builder.startObject();
             }
             toXContent.toXContent(builder, ToXContent.EMPTY_PARAMS);
-            if (wrapInObject) {
+            if (toXContent.isFragment()) {
                 builder.endObject();
             }
             return builder.bytes();
         }
     }
-
 }

core/src/main/java/org/elasticsearch/rest/action/RestStatusToXContentListener.java

@@ -30,7 +30,7 @@ import java.util.function.Function;
 /**
  * Content listener that extracts that {@link RestStatus} from the response.
  */
-public class RestStatusToXContentListener<Response extends StatusToXContent> extends RestResponseListener<Response> {
+public class RestStatusToXContentListener<Response extends StatusToXContent> extends RestToXContentListener<Response> {
     private final Function<Response, String> extractLocation;
 
     /**
@@ -52,15 +52,9 @@ public class RestStatusToXContentListener<Response extends StatusToXContent> ext
     }
 
     @Override
-    public final RestResponse buildResponse(Response response) throws Exception {
+    public RestResponse buildResponse(Response response, XContentBuilder builder) throws Exception {
-        return buildResponse(response, channel.newBuilder());
+        toXContent(response, builder);
-    }
+        RestResponse restResponse = new BytesRestResponse(response.status(), builder);
-
-    public final RestResponse buildResponse(Response response, XContentBuilder builder) throws Exception {
-        builder.startObject();
-        response.toXContent(builder, channel.request());
-        builder.endObject();
-        BytesRestResponse restResponse = new BytesRestResponse(response.status(), builder);
         if (RestStatus.CREATED == restResponse.status()) {
             String location = extractLocation.apply(response);
             if (location != null) {

core/src/main/java/org/elasticsearch/rest/action/RestToXContentListener.java

@@ -26,6 +26,8 @@ import org.elasticsearch.rest.RestChannel;
 import org.elasticsearch.rest.RestResponse;
 import org.elasticsearch.rest.RestStatus;
 
+import java.io.IOException;
+
 /**
  * A REST based action listener that assumes the response is of type {@link ToXContent} and automatically
  * builds an XContent based response (wrapping the toXContent in startObject/endObject).
@@ -41,22 +43,20 @@ public class RestToXContentListener<Response extends ToXContent> extends RestRes
         return buildResponse(response, channel.newBuilder());
     }
 
-    public final RestResponse buildResponse(Response response, XContentBuilder builder) throws Exception {
+    protected final void toXContent(Response response, XContentBuilder builder) throws IOException {
-        if (wrapInObject()) {
+        final boolean needsNewObject = response.isFragment();
+        if (needsNewObject) {
             builder.startObject();
         }
         response.toXContent(builder, channel.request());
-        if (wrapInObject()) {
+        if (needsNewObject) {
             builder.endObject();
         }
-        return new BytesRestResponse(getStatus(response), builder);
     }
 
-    protected boolean wrapInObject() {
+    public RestResponse buildResponse(Response response, XContentBuilder builder) throws Exception {
-        //Ideally, the toXContent method starts with startObject and ends with endObject.
+        toXContent(response, builder);
-        //In practice, we have many places where toXContent produces a json fragment that's not valid by itself. We will
+        return new BytesRestResponse(getStatus(response), builder);
-        //migrate those step by step, so that we never have to start objects here, and we can remove this method.
-        return true;
     }
 
     protected RestStatus getStatus(Response response) {

core/src/main/java/org/elasticsearch/rest/action/document/RestGetAction.java

@@ -75,11 +75,6 @@ public class RestGetAction extends BaseRestHandler {
         getRequest.fetchSourceContext(FetchSourceContext.parseFromRestRequest(request));
 
         return channel -> client.get(getRequest, new RestToXContentListener<GetResponse>(channel) {
-            @Override
-            protected boolean wrapInObject() {
-                return false;
-            }
-
             @Override
             protected RestStatus getStatus(GetResponse response) {
                 return response.isExists() ? OK : NOT_FOUND;

core/src/test/java/org/elasticsearch/action/get/GetResponseTests.java

@@ -46,7 +46,7 @@ public class GetResponseTests extends ESTestCase {
         Tuple<GetResult, GetResult> tuple = randomGetResult(xContentType);
         GetResponse getResponse = new GetResponse(tuple.v1());
         GetResponse expectedGetResponse = new GetResponse(tuple.v2());
-        BytesReference originalBytes = toXContent(getResponse, xContentType, false);
+        BytesReference originalBytes = toXContent(getResponse, xContentType);
         //test that we can parse what we print out
         GetResponse parsedGetResponse;
         try (XContentParser parser = createParser(xContentType.xContent(), originalBytes)) {
@@ -55,11 +55,10 @@ public class GetResponseTests extends ESTestCase {
         }
         assertEquals(expectedGetResponse, parsedGetResponse);
         //print the parsed object out and test that the output is the same as the original output
-        BytesReference finalBytes = toXContent(parsedGetResponse, xContentType, false);
+        BytesReference finalBytes = toXContent(parsedGetResponse, xContentType);
         assertToXContentEquivalent(originalBytes, finalBytes, xContentType);
         //check that the source stays unchanged, no shuffling of keys nor anything like that
         assertEquals(expectedGetResponse.getSourceAsString(), parsedGetResponse.getSourceAsString());
-
     }
 
     public void testToXContent() throws IOException {

core/src/test/java/org/elasticsearch/action/support/replication/ReplicationResponseTests.java

@@ -134,7 +134,7 @@ public class ReplicationResponseTests extends ESTestCase {
         final XContentType xContentType = randomFrom(XContentType.values());
 
         final ReplicationResponse.ShardInfo shardInfo = new ReplicationResponse.ShardInfo(5, 3);
-        final BytesReference shardInfoBytes = XContentHelper.toXContent(shardInfo, xContentType, true);
+        final BytesReference shardInfoBytes = XContentHelper.toXContent(shardInfo, xContentType);
 
         // Expected JSON is {"_shards":{"total":5,"successful":3,"failed":0}}
         try (XContentParser parser = createParser(xContentType.xContent(), shardInfoBytes)) {
@@ -164,7 +164,7 @@ public class ReplicationResponseTests extends ESTestCase {
         final XContentType xContentType = randomFrom(XContentType.values());
 
         final ReplicationResponse.ShardInfo shardInfo = new ReplicationResponse.ShardInfo(randomIntBetween(1, 5), randomIntBetween(1, 5));
-        final BytesReference shardInfoBytes = XContentHelper.toXContent(shardInfo, xContentType, true);
+        final BytesReference shardInfoBytes = XContentHelper.toXContent(shardInfo, xContentType);
 
         ReplicationResponse.ShardInfo parsedShardInfo;
         try (XContentParser parser = createParser(xContentType.xContent(), shardInfoBytes)) {
@@ -177,7 +177,7 @@ public class ReplicationResponseTests extends ESTestCase {
         // We can use assertEquals because the shardInfo doesn't have a failure (and exceptions)
         assertEquals(shardInfo, parsedShardInfo);
 
-        BytesReference parsedShardInfoBytes = XContentHelper.toXContent(parsedShardInfo, xContentType, true);
+        BytesReference parsedShardInfoBytes = XContentHelper.toXContent(parsedShardInfo, xContentType);
         assertEquals(shardInfoBytes, parsedShardInfoBytes);
     }
 
@@ -185,7 +185,7 @@ public class ReplicationResponseTests extends ESTestCase {
         final XContentType xContentType = randomFrom(XContentType.values());
 
         final ReplicationResponse.ShardInfo shardInfo = randomShardInfo();
-        final BytesReference shardInfoBytes = XContentHelper.toXContent(shardInfo, xContentType, true);
+        final BytesReference shardInfoBytes = XContentHelper.toXContent(shardInfo, xContentType);
 
         try (XContentParser parser = createParser(xContentType.xContent(), shardInfoBytes)) {
             assertEquals(XContentParser.Token.START_OBJECT, parser.nextToken());
@@ -226,7 +226,7 @@ public class ReplicationResponseTests extends ESTestCase {
         final XContentType xContentType = randomFrom(XContentType.values());
 
         final ReplicationResponse.ShardInfo shardInfo = randomShardInfo();
-        final BytesReference shardInfoBytes = XContentHelper.toXContent(shardInfo, xContentType, true);
+        final BytesReference shardInfoBytes = XContentHelper.toXContent(shardInfo, xContentType);
 
         ReplicationResponse.ShardInfo parsedShardInfo;
         try (XContentParser parser = createParser(xContentType.xContent(), shardInfoBytes)) {
@@ -267,7 +267,7 @@ public class ReplicationResponseTests extends ESTestCase {
         final XContentType xContentType = randomFrom(XContentType.values());
 
         final ReplicationResponse.ShardInfo.Failure shardInfoFailure = randomFailure();
-        final BytesReference shardInfoBytes = XContentHelper.toXContent(shardInfoFailure, xContentType, false);
+        final BytesReference shardInfoBytes = XContentHelper.toXContent(shardInfoFailure, xContentType);
 
         try (XContentParser parser = createParser(xContentType.xContent(), shardInfoBytes)) {
             assertFailure(parser, shardInfoFailure);
@@ -278,7 +278,7 @@ public class ReplicationResponseTests extends ESTestCase {
         final XContentType xContentType = randomFrom(XContentType.values());
 
         final ReplicationResponse.ShardInfo.Failure shardInfoFailure = randomFailure();
-        final BytesReference shardInfoBytes = XContentHelper.toXContent(shardInfoFailure, xContentType, false);
+        final BytesReference shardInfoBytes = XContentHelper.toXContent(shardInfoFailure, xContentType);
 
         ReplicationResponse.ShardInfo.Failure parsedFailure;
         try (XContentParser parser = createParser(xContentType.xContent(), shardInfoBytes)) {

core/src/test/java/org/elasticsearch/cluster/serialization/ClusterStateToStringTests.java

@@ -56,7 +56,7 @@ public class ClusterStateToStringTests extends ESAllocationTestCase {
         AllocationService strategy = createAllocationService();
         clusterState = ClusterState.builder(clusterState).routingTable(strategy.reroute(clusterState, "reroute").routingTable()).build();
 
-        String clusterStateString = Strings.toString(clusterState, true);
+        String clusterStateString = Strings.toString(clusterState);
         assertNotNull(clusterStateString);
 
         assertThat(clusterStateString, containsString("test_idx"));

core/src/test/java/org/elasticsearch/common/StringsTests.java

@@ -21,11 +21,9 @@ package org.elasticsearch.common;
 
 import org.elasticsearch.common.util.set.Sets;
 import org.elasticsearch.common.xcontent.ToXContent;
-import org.elasticsearch.common.xcontent.XContentBuilder;
+import org.elasticsearch.common.xcontent.ToXContentObject;
 import org.elasticsearch.test.ESTestCase;
 
-import java.io.IOException;
-
 import static org.hamcrest.Matchers.containsString;
 
 public class StringsTests extends ESTestCase {
@@ -58,21 +56,36 @@ public class StringsTests extends ESTestCase {
         assertEquals("", Strings.cleanTruncate("foo", 0));
     }
 
-    public void testEvilToString() {
+    public void testToStringToXContent() {
-        ToXContent needsEnclosingObject = new ToXContent() {
+        final ToXContent toXContent;
-            @Override
+        final boolean error;
-            public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
+        if (randomBoolean()) {
-                return builder.field("ok", "here").field("catastrophe", "");
+            if (randomBoolean()) {
+                error = false;
+                toXContent  = (builder, params) -> builder.field("ok", "here").field("catastrophe", "");
+            } else {
+                error = true;
+                toXContent  = (builder, params) ->
+                        builder.startObject().field("ok", "here").field("catastrophe", "").endObject();
             }
-        };
+        } else {
-        String toString = Strings.toString(needsEnclosingObject);
+            if (randomBoolean()) {
-        assertThat(toString, containsString("Error building toString out of XContent"));
+                error = false;
-        assertThat(toString, containsString("Can not write a field name, expecting a value"));
+                toXContent = (ToXContentObject) (builder, params) ->
+                        builder.startObject().field("ok", "here").field("catastrophe", "").endObject();
+            } else {
+                error = true;
+                toXContent = (ToXContentObject) (builder, params) -> builder.field("ok", "here").field("catastrophe", "");
+            }
+        }
 
-        // We can salvage it!
+        String toString = Strings.toString(toXContent);
-        toString = Strings.toString(needsEnclosingObject, true);
+        if (error) {
-        assertThat(toString, containsString("\"ok\":\"here\""));
+            assertThat(toString, containsString("Error building toString out of XContent"));
-        assertThat(toString, containsString("\"catastrophe\":\"\""));
+        } else {
+            assertThat(toString, containsString("\"ok\":\"here\""));
+            assertThat(toString, containsString("\"catastrophe\":\"\""));
+        }
     }
 
     public void testSplitStringToSet() {

core/src/test/java/org/elasticsearch/common/xcontent/support/XContentHelperTests.java

@@ -22,6 +22,7 @@ package org.elasticsearch.common.xcontent.support;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.xcontent.NamedXContentRegistry;
 import org.elasticsearch.common.xcontent.ToXContent;
+import org.elasticsearch.common.xcontent.ToXContentObject;
 import org.elasticsearch.common.xcontent.XContentHelper;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.XContentType;
@@ -67,27 +68,39 @@ public class XContentHelperTests extends ESTestCase {
         assertThat(content, Matchers.equalTo(expected));
     }
 
-    public void testToXContentWrapInObject() throws IOException {
+    public void testToXContent() throws IOException {
-        boolean wrapInObject = randomBoolean();
+        final XContentType xContentType = randomFrom(XContentType.values());
-        XContentType xContentType = randomFrom(XContentType.values());
+        final ToXContent toXContent;
-        ToXContent toXContent = (builder, params) -> {
+        final boolean error;
-            if (wrapInObject == false) {
+        if (randomBoolean()) {
-                builder.startObject();
+            if (randomBoolean()) {
+                error = false;
+                toXContent = (builder, params) -> builder.field("field", "value");
+            } else {
+                error = true;
+                toXContent = (builder, params) -> builder.startObject().field("field", "value").endObject();
             }
-            builder.field("field", "value");
+        } else {
-            if (wrapInObject == false) {
+            if (randomBoolean()) {
-                builder.endObject();
+                error = false;
+                toXContent = (ToXContentObject) (builder, params) -> builder.startObject().field("field", "value").endObject();
+            } else {
+                error = true;
+                toXContent = (ToXContentObject) (builder, params) -> builder.field("field", "value");
+            }
+        }
+        if (error) {
+            expectThrows(IOException.class, () -> XContentHelper.toXContent(toXContent, xContentType));
+        } else {
+            BytesReference bytes = XContentHelper.toXContent(toXContent, xContentType);
+            try (XContentParser parser = xContentType.xContent().createParser(NamedXContentRegistry.EMPTY, bytes)) {
+                assertEquals(XContentParser.Token.START_OBJECT, parser.nextToken());
+                assertEquals(XContentParser.Token.FIELD_NAME, parser.nextToken());
+                assertTrue(parser.nextToken().isValue());
+                assertEquals("value", parser.text());
+                assertEquals(XContentParser.Token.END_OBJECT, parser.nextToken());
+                assertNull(parser.nextToken());
             }
-            return builder;
-        };
-        BytesReference bytes = XContentHelper.toXContent(toXContent, xContentType, wrapInObject);
-        try (XContentParser parser = xContentType.xContent().createParser(NamedXContentRegistry.EMPTY, bytes)) {
-            assertEquals(XContentParser.Token.START_OBJECT, parser.nextToken());
-            assertEquals(XContentParser.Token.FIELD_NAME, parser.nextToken());
-            assertTrue(parser.nextToken().isValue());
-            assertEquals("value", parser.text());
-            assertEquals(XContentParser.Token.END_OBJECT, parser.nextToken());
-            assertNull(parser.nextToken());
         }
     }
 }

core/src/test/java/org/elasticsearch/index/get/GetFieldTests.java

@@ -45,7 +45,7 @@ public class GetFieldTests extends ESTestCase {
 
     public void testToXContent() throws IOException {
         GetField getField = new GetField("field", Arrays.asList("value1", "value2"));
-        String output = Strings.toString(getField, true);
+        String output = Strings.toString(getField);
         assertEquals("{\"field\":[\"value1\",\"value2\"]}", output);
     }
 
@@ -58,7 +58,7 @@ public class GetFieldTests extends ESTestCase {
         Tuple<GetField, GetField> tuple = randomGetField(xContentType);
         GetField getField = tuple.v1();
         GetField expectedGetField = tuple.v2();
-        BytesReference originalBytes = toXContent(getField, xContentType, true);
+        BytesReference originalBytes = toXContent(getField, xContentType);
         //test that we can parse what we print out
         GetField parsedGetField;
         try (XContentParser parser = createParser(xContentType.xContent(), originalBytes)) {
@@ -71,7 +71,7 @@ public class GetFieldTests extends ESTestCase {
             assertNull(parser.nextToken());
         }
         assertEquals(expectedGetField, parsedGetField);
-        BytesReference finalBytes = toXContent(parsedGetField, xContentType, true);
+        BytesReference finalBytes = toXContent(parsedGetField, xContentType);
         assertToXContentEquivalent(originalBytes, finalBytes, xContentType);
     }
 

core/src/test/java/org/elasticsearch/index/get/GetResultTests.java

@@ -48,7 +48,7 @@ public class GetResultTests extends ESTestCase {
         Tuple<GetResult, GetResult> tuple = randomGetResult(xContentType);
         GetResult getResult = tuple.v1();
         GetResult expectedGetResult = tuple.v2();
-        BytesReference originalBytes = toXContent(getResult, xContentType, false);
+        BytesReference originalBytes = toXContent(getResult, xContentType);
         //test that we can parse what we print out
         GetResult parsedGetResult;
         try (XContentParser parser = createParser(xContentType.xContent(), originalBytes)) {
@@ -57,7 +57,7 @@ public class GetResultTests extends ESTestCase {
         }
         assertEquals(expectedGetResult, parsedGetResult);
         //print the parsed object out and test that the output is the same as the original output
-        BytesReference finalBytes = toXContent(parsedGetResult, xContentType, false);
+        BytesReference finalBytes = toXContent(parsedGetResult, xContentType);
         assertToXContentEquivalent(originalBytes, finalBytes, xContentType);
         //check that the source stays unchanged, no shuffling of keys nor anything like that
         assertEquals(expectedGetResult.sourceAsString(), parsedGetResult.sourceAsString());

core/src/test/java/org/elasticsearch/search/internal/SearchSortValuesTests.java

@@ -79,7 +79,7 @@ public class SearchSortValuesTests extends ESTestCase {
         parser.nextToken();
         if (sortValues.sortValues().length > 0) {
             SearchSortValues parsed = SearchSortValues.fromXContent(parser);
-            assertToXContentEquivalent(builder.bytes(), toXContent(parsed, xcontentType, true), xcontentType);
+            assertToXContentEquivalent(builder.bytes(), toXContent(parsed, xcontentType), xcontentType);
             parser.nextToken();
         }
         assertEquals(XContentParser.Token.END_OBJECT, parser.currentToken());

qa/backwards-5.0/src/test/java/org/elasticsearch/backwards/IndexingIT.java

@@ -70,7 +70,7 @@ public class IndexingIT extends ESRestTestCase {
 
     private void createIndex(String name, Settings settings) throws IOException {
         assertOK(client().performRequest("PUT", name, Collections.emptyMap(),
-            new StringEntity("{ \"settings\": " + Strings.toString(settings, true) + " }")));
+            new StringEntity("{ \"settings\": " + Strings.toString(settings) + " }")));
     }
 
     private void updateIndexSetting(String name, Settings.Builder settings) throws IOException {
@@ -78,7 +78,7 @@ public class IndexingIT extends ESRestTestCase {
     }
     private void updateIndexSetting(String name, Settings settings) throws IOException {
         assertOK(client().performRequest("PUT", name + "/_settings", Collections.emptyMap(),
-            new StringEntity(Strings.toString(settings, true))));
+            new StringEntity(Strings.toString(settings))));
     }
 
     protected int indexDocs(String index, final int idStart, final int numDocs) throws IOException {