Fix DefaultShardOperationFailedException subclass xcontent serialization (#43435)

The current toXContent implementation can fail when the superclasses toXContent
is called (see #43423). This change makes sure that 
DefaultShardOperationFailedException#toXContent is final and implementations
need to add special fields in #innerToXContent. All implementations should write
to self-contained xContent objects. Also adding a test for xContent deserialization 
to CloseIndexResponseTests.

Closes #43423

server/src/main/java/org/elasticsearch/action/admin/indices/close/CloseIndexResponse.java

@@ -78,6 +78,7 @@ public class CloseIndexResponse extends ShardsAcknowledgedResponse {
         }
     }
 
+    @Override
     protected void addCustomFields(final XContentBuilder builder, final Params params) throws IOException {
         super.addCustomFields(builder, params);
         builder.startObject("indices");
@@ -190,7 +191,7 @@ public class CloseIndexResponse extends ShardsAcknowledgedResponse {
     public static class ShardResult implements Writeable, ToXContentFragment {
 
         private final int id;
-        private final ShardResult.Failure[] failures;
+        private final Failure[] failures;
 
         public ShardResult(final int id, final Failure[] failures) {
             this.id = id;
@@ -199,7 +200,7 @@ public class CloseIndexResponse extends ShardsAcknowledgedResponse {
 
         ShardResult(final StreamInput in) throws IOException {
             this.id = in.readVInt();
-            this.failures = in.readOptionalArray(Failure::readFailure, ShardResult.Failure[]::new);
+            this.failures = in.readOptionalArray(Failure::readFailure, Failure[]::new);
         }
 
         @Override
@@ -227,9 +228,7 @@ public class CloseIndexResponse extends ShardsAcknowledgedResponse {
                 builder.startArray("failures");
                 if (failures != null) {
                     for (Failure failure : failures) {
-                        builder.startObject();
                         failure.toXContent(builder, params);
-                        builder.endObject();
                     }
                 }
                 builder.endArray();
@@ -242,7 +241,7 @@ public class CloseIndexResponse extends ShardsAcknowledgedResponse {
             return Strings.toString(this);
         }
 
-        public static class Failure extends DefaultShardOperationFailedException implements Writeable {
+        public static class Failure extends DefaultShardOperationFailedException {
 
             private @Nullable String nodeId;
 
@@ -275,11 +274,11 @@ public class CloseIndexResponse extends ShardsAcknowledgedResponse {
             }
 
             @Override
-            public XContentBuilder toXContent(final XContentBuilder builder, final Params params) throws IOException {
+            public XContentBuilder innerToXContent(final XContentBuilder builder, final Params params) throws IOException {
                 if (nodeId != null) {
                     builder.field("node", nodeId);
                 }
-                return super.toXContent(builder, params);
+                return super.innerToXContent(builder, params);
             }
 
             @Override

server/src/main/java/org/elasticsearch/action/admin/indices/shards/IndicesShardStoresResponse.java

@@ -267,12 +267,9 @@ public class IndicesShardStoresResponse extends ActionResponse implements ToXCon
         }
 
         @Override
-        public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
+        public XContentBuilder innerToXContent(XContentBuilder builder, Params params) throws IOException {
-            builder.startObject();
             builder.field("node", nodeId());
-            super.innerToXContent(builder, params);
+            return super.innerToXContent(builder, params);
-            builder.endObject();
-            return builder;
         }
     }
 

server/src/main/java/org/elasticsearch/action/support/DefaultShardOperationFailedException.java

@@ -25,6 +25,7 @@ import org.elasticsearch.action.ShardOperationFailedException;
 import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
+import org.elasticsearch.common.io.stream.Writeable;
 import org.elasticsearch.common.xcontent.ConstructingObjectParser;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
@@ -35,7 +36,7 @@ import java.io.IOException;
 import static org.elasticsearch.ExceptionsHelper.detailedMessage;
 import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;
 
-public class DefaultShardOperationFailedException extends ShardOperationFailedException {
+public class DefaultShardOperationFailedException extends ShardOperationFailedException implements Writeable {
 
     private static final String INDEX = "index";
     private static final String SHARD_ID = "shard";
@@ -90,13 +91,13 @@ public class DefaultShardOperationFailedException extends ShardOperationFailedEx
     }
 
     @Override
-    public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
+    public final XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
         builder.startObject();
         innerToXContent(builder, params);
         builder.endObject();
         return builder;
     }
-    
+
     protected XContentBuilder innerToXContent(XContentBuilder builder, Params params) throws IOException {
         builder.field("shard", shardId());
         builder.field("index", index());

server/src/test/java/org/elasticsearch/action/admin/indices/close/CloseIndexResponseTests.java

@@ -22,9 +22,14 @@ package org.elasticsearch.action.admin.indices.close;
 import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.Version;
 import org.elasticsearch.action.NoShardAvailableActionException;
+import org.elasticsearch.action.admin.indices.close.CloseIndexResponse.IndexResult;
 import org.elasticsearch.action.support.master.AcknowledgedResponse;
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.io.stream.BytesStreamOutput;
 import org.elasticsearch.common.io.stream.StreamInput;
+import org.elasticsearch.common.xcontent.ToXContent;
+import org.elasticsearch.common.xcontent.XContentBuilder;
+import org.elasticsearch.common.xcontent.XContentType;
 import org.elasticsearch.index.Index;
 import org.elasticsearch.index.IndexNotFoundException;
 import org.elasticsearch.index.shard.ShardId;
@@ -32,8 +37,10 @@ import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.test.VersionUtils;
 import org.elasticsearch.transport.ActionNotFoundTransportException;
 
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 
 import static org.elasticsearch.test.VersionUtils.randomVersionBetween;
@@ -46,6 +53,38 @@ import static org.hamcrest.Matchers.nullValue;
 
 public class CloseIndexResponseTests extends ESTestCase {
 
+    /**
+     * Test that random responses can be written to xcontent without errors.
+     * Also check some specific simple cases for output.
+     */
+    public void testToXContent() throws IOException {
+        CloseIndexResponse response = randomResponse();
+        XContentType xContentType = randomFrom(XContentType.values());
+        try (XContentBuilder builder = XContentBuilder.builder(xContentType.xContent())) {
+            response.toXContent(builder, ToXContent.EMPTY_PARAMS);
+        }
+
+        Index index = new Index("test", "uuid");
+        IndexResult indexResult = new CloseIndexResponse.IndexResult(index);
+        CloseIndexResponse closeIndexResponse = new CloseIndexResponse(true, true,
+                Collections.singletonList(indexResult));
+        assertEquals("{\"acknowledged\":true,\"shards_acknowledged\":true,\"indices\":{\"test\":{\"closed\":true}}}",
+                Strings.toString(closeIndexResponse));
+
+        CloseIndexResponse.ShardResult[] shards = new CloseIndexResponse.ShardResult[1];
+        shards[0] = new CloseIndexResponse.ShardResult(0, new CloseIndexResponse.ShardResult.Failure[] {
+                new CloseIndexResponse.ShardResult.Failure("test", 0, new ActionNotFoundTransportException("test"), "nodeId") });
+        indexResult = new CloseIndexResponse.IndexResult(index, shards);
+        closeIndexResponse = new CloseIndexResponse(true, true,
+                Collections.singletonList(indexResult));
+        assertEquals("{\"acknowledged\":true,\"shards_acknowledged\":true,"
+                + "\"indices\":{\"test\":{\"closed\":false,\"failedShards\":{\"0\":{"
+                + "\"failures\":[{\"node\":\"nodeId\",\"shard\":0,\"index\":\"test\",\"status\":\"INTERNAL_SERVER_ERROR\","
+                + "\"reason\":{\"type\":\"action_not_found_transport_exception\","
+                + "\"reason\":\"No handler for action [test]\"}}]}}}}}",
+                Strings.toString(closeIndexResponse));
+    }
+
     public void testSerialization() throws Exception {
         final CloseIndexResponse response = randomResponse();
         try (BytesStreamOutput out = new BytesStreamOutput()) {
@@ -131,7 +170,10 @@ public class CloseIndexResponseTests extends ESTestCase {
                             acknowledged = false;
                             failures = new CloseIndexResponse.ShardResult.Failure[randomIntBetween(1, 3)];
                             for (int j = 0; j < failures.length; j++) {
-                                String nodeId = randomAlphaOfLength(5);
+                                String nodeId = null;
+                                if (frequently()) {
+                                    nodeId = randomAlphaOfLength(5);
+                                }
                                 failures[j] = new CloseIndexResponse.ShardResult.Failure(indexName, i, randomException(index, i), nodeId);
                             }
                         }