Convert ShardOperationFailedException to Writeable (#44532) (#44580)

This commit converts subclasses of ShardOperationFailedException to
implement ctors with StreamInput instead of readFrom. It also simplifies
IndicesShardStoresResponse.Failure to serialize its shardId after the
super data.

relates #34389

server/src/main/java/org/elasticsearch/action/ShardOperationFailedException.java

@@ -20,7 +20,7 @@
 package org.elasticsearch.action;
 
 import org.elasticsearch.common.Nullable;
-import org.elasticsearch.common.io.stream.Streamable;
+import org.elasticsearch.common.io.stream.Writeable;
 import org.elasticsearch.common.xcontent.ToXContentObject;
 import org.elasticsearch.rest.RestStatus;
 
@@ -30,7 +30,7 @@ import java.util.Objects;
  * An exception indicating that a failure occurred performing an operation on the shard.
  *
  */
-public abstract class ShardOperationFailedException implements Streamable, ToXContentObject {
+public abstract class ShardOperationFailedException implements Writeable, ToXContentObject {
 
     protected String index;
     protected int shardId = -1;

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

@@ -238,7 +238,9 @@ public class CloseIndexResponse extends ShardsAcknowledgedResponse {
 
             private @Nullable String nodeId;
 
-            private Failure() {
+            private Failure(StreamInput in) throws IOException {
+                super(in);
+                nodeId = in.readOptionalString();
             }
 
             public Failure(final String index, final int shardId, final Throwable reason) {
@@ -254,12 +256,6 @@ public class CloseIndexResponse extends ShardsAcknowledgedResponse {
                 return nodeId;
             }
 
-            @Override
-            public void readFrom(final StreamInput in) throws IOException {
-                super.readFrom(in);
-                nodeId = in.readOptionalString();
-            }
-
             @Override
             public void writeTo(final StreamOutput out) throws IOException {
                 super.writeTo(out);
@@ -280,9 +276,7 @@ public class CloseIndexResponse extends ShardsAcknowledgedResponse {
             }
 
             static Failure readFailure(final StreamInput in) throws IOException {
-                final Failure failure = new Failure();
+                return new Failure(in);
-                failure.readFrom(in);
-                return failure;
             }
         }
     }

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

@@ -241,7 +241,14 @@ public class IndicesShardStoresResponse extends ActionResponse implements ToXCon
             this.nodeId = nodeId;
         }
 
-        private Failure() {
+        private Failure(StreamInput in) throws IOException {
+            if (in.getVersion().before(Version.V_7_4_0)) {
+                nodeId = in.readString();
+            }
+            readFrom(in, this);
+            if (in.getVersion().onOrAfter(Version.V_7_4_0)) {
+                nodeId = in.readString();
+            }
         }
 
         public String nodeId() {
@@ -249,21 +256,18 @@ public class IndicesShardStoresResponse extends ActionResponse implements ToXCon
         }
 
         static Failure readFailure(StreamInput in) throws IOException {
-            Failure failure = new Failure();
+            return new Failure(in);
-            failure.readFrom(in);
-            return failure;
-        }
-
-        @Override
-        public void readFrom(StreamInput in) throws IOException {
-            nodeId = in.readString();
-            super.readFrom(in);
         }
 
         @Override
         public void writeTo(StreamOutput out) throws IOException {
-            out.writeString(nodeId);
+            if (out.getVersion().before(Version.V_7_4_0)) {
+                out.writeString(nodeId);
+            }
             super.writeTo(out);
+            if (out.getVersion().onOrAfter(Version.V_7_4_0)) {
+                out.writeString(nodeId);
+            }
         }
 
         @Override

server/src/main/java/org/elasticsearch/action/search/ShardSearchFailure.java

@@ -54,7 +54,15 @@ public class ShardSearchFailure extends ShardOperationFailedException {
 
     private SearchShardTarget shardTarget;
 
-    ShardSearchFailure() {
+    ShardSearchFailure(StreamInput in) throws IOException {
+        shardTarget = in.readOptionalWriteable(SearchShardTarget::new);
+        if (shardTarget != null) {
+            index = shardTarget.getFullyQualifiedIndexName();
+            shardId = shardTarget.getShardId().getId();
+        }
+        reason = in.readString();
+        status = RestStatus.readFrom(in);
+        cause = in.readException();
     }
 
     public ShardSearchFailure(Exception e) {
@@ -91,21 +99,8 @@ public class ShardSearchFailure extends ShardOperationFailedException {
     }
 
     public static ShardSearchFailure readShardSearchFailure(StreamInput in) throws IOException {
-        ShardSearchFailure shardSearchFailure = new ShardSearchFailure();
+        return new ShardSearchFailure(in);
-        shardSearchFailure.readFrom(in);
-        return shardSearchFailure;
-    }
 
-    @Override
-    public void readFrom(StreamInput in) throws IOException {
-        shardTarget = in.readOptionalWriteable(SearchShardTarget::new);
-        if (shardTarget != null) {
-            index = shardTarget.getFullyQualifiedIndexName();
-            shardId = shardTarget.getShardId().getId();
-        }
-        reason = in.readString();
-        status = RestStatus.readFrom(in);
-        cause = in.readException();
     }
 
     @Override

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

@@ -51,7 +51,10 @@ public class DefaultShardOperationFailedException extends ShardOperationFailedEx
         PARSER.declareObject(constructorArg(), (p, c) -> ElasticsearchException.fromXContent(p), new ParseField(REASON));
     }
 
-    protected DefaultShardOperationFailedException() {
+    protected DefaultShardOperationFailedException() {}
+
+    protected DefaultShardOperationFailedException(StreamInput in) throws IOException {
+        readFrom(in, this);
     }
 
     public DefaultShardOperationFailedException(ElasticsearchException e) {
@@ -64,17 +67,14 @@ public class DefaultShardOperationFailedException extends ShardOperationFailedEx
     }
 
     public static DefaultShardOperationFailedException readShardOperationFailed(StreamInput in) throws IOException {
-        DefaultShardOperationFailedException exp = new DefaultShardOperationFailedException();
+        return new DefaultShardOperationFailedException(in);
-        exp.readFrom(in);
-        return exp;
     }
 
-    @Override
+    public static void readFrom(StreamInput in, DefaultShardOperationFailedException f) throws IOException {
-    public void readFrom(StreamInput in) throws IOException {
+        f.index = in.readOptionalString();
-        index = in.readOptionalString();
+        f.shardId = in.readVInt();
-        shardId = in.readVInt();
+        f.cause = in.readException();
-        cause = in.readException();
+        f.status = RestStatus.readFrom(in);
-        status = RestStatus.readFrom(in);
     }
 
     @Override

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

@@ -143,9 +143,7 @@ public class ReplicationResponse extends ActionResponse {
             int size = in.readVInt();
             failures = new Failure[size];
             for (int i = 0; i < size; i++) {
-                Failure failure = new Failure();
+                failures[i] = new Failure(in);
-                failure.readFrom(in);
-                failures[i] = failure;
             }
         }
 
@@ -242,6 +240,16 @@ public class ReplicationResponse extends ActionResponse {
             private String nodeId;
             private boolean primary;
 
+            public Failure(StreamInput in) throws IOException {
+                shardId = new ShardId(in);
+                super.shardId = shardId.getId();
+                index = shardId.getIndexName();
+                nodeId = in.readOptionalString();
+                cause = in.readException();
+                status = RestStatus.readFrom(in);
+                primary = in.readBoolean();
+            }
+
             public Failure(ShardId  shardId, @Nullable String nodeId, Exception cause, RestStatus status, boolean primary) {
                 super(shardId.getIndexName(), shardId.getId(), ExceptionsHelper.detailedMessage(cause), status, cause);
                 this.shardId = shardId;
@@ -272,17 +280,6 @@ public class ReplicationResponse extends ActionResponse {
                 return primary;
             }
 
-            @Override
-            public void readFrom(StreamInput in) throws IOException {
-                shardId = new ShardId(in);
-                super.shardId = shardId.getId();
-                index = shardId.getIndexName();
-                nodeId = in.readOptionalString();
-                cause = in.readException();
-                status = RestStatus.readFrom(in);
-                primary = in.readBoolean();
-            }
-
             @Override
             public void writeTo(StreamOutput out) throws IOException {
                 shardId.writeTo(out);

server/src/main/java/org/elasticsearch/snapshots/SnapshotInfo.java

@@ -303,7 +303,7 @@ public final class SnapshotInfo implements Comparable<SnapshotInfo>, ToXContent,
         if (size > 0) {
             List<SnapshotShardFailure> failureBuilder = new ArrayList<>();
             for (int i = 0; i < size; i++) {
-                failureBuilder.add(SnapshotShardFailure.readSnapshotShardFailure(in));
+                failureBuilder.add(new SnapshotShardFailure(in));
             }
             shardFailures = Collections.unmodifiableList(failureBuilder);
         } else {

server/src/main/java/org/elasticsearch/snapshots/SnapshotShardFailure.java

@@ -45,8 +45,13 @@ public class SnapshotShardFailure extends ShardOperationFailedException {
     private String nodeId;
     private ShardId shardId;
 
-    private SnapshotShardFailure() {
+    SnapshotShardFailure(StreamInput in) throws IOException {
-
+        nodeId = in.readOptionalString();
+        shardId = new ShardId(in);
+        super.shardId = shardId.getId();
+        index = shardId.getIndexName();
+        reason = in.readString();
+        status = RestStatus.readFrom(in);
     }
 
     /**
@@ -84,28 +89,6 @@ public class SnapshotShardFailure extends ShardOperationFailedException {
         return nodeId;
     }
 
-    /**
-     * Reads shard failure information from stream input
-     *
-     * @param in stream input
-     * @return shard failure information
-     */
-    static SnapshotShardFailure readSnapshotShardFailure(StreamInput in) throws IOException {
-        SnapshotShardFailure exp = new SnapshotShardFailure();
-        exp.readFrom(in);
-        return exp;
-    }
-
-    @Override
-    public void readFrom(StreamInput in) throws IOException {
-        nodeId = in.readOptionalString();
-        shardId = new ShardId(in);
-        super.shardId = shardId.getId();
-        index = shardId.getIndexName();
-        reason = in.readString();
-        status = RestStatus.readFrom(in);
-    }
-
     @Override
     public void writeTo(StreamOutput out) throws IOException {
         out.writeOptionalString(nodeId);

server/src/test/java/org/elasticsearch/action/ShardOperationFailedExceptionTests.java

@@ -20,7 +20,6 @@
 package org.elasticsearch.action;
 
 import org.elasticsearch.common.Nullable;
-import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.rest.RestStatus;
@@ -58,11 +57,6 @@ public class ShardOperationFailedExceptionTests extends ESTestCase {
            super(index, shardId, reason, status, cause);
         }
 
-        @Override
-        public void readFrom(StreamInput in) throws IOException {
-
-        }
-
         @Override
         public void writeTo(StreamOutput out) throws IOException {
 

server/src/test/java/org/elasticsearch/action/search/ShardSearchFailureTests.java

@@ -138,7 +138,7 @@ public class ShardSearchFailureTests extends ESTestCase {
 
     public void testSerialization() throws IOException {
         ShardSearchFailure testItem = createTestItem(randomAlphaOfLength(12));
-        ShardSearchFailure deserializedInstance = copyStreamable(testItem, writableRegistry(),
+        ShardSearchFailure deserializedInstance = copyWriteable(testItem, writableRegistry(),
             ShardSearchFailure::new, VersionUtils.randomVersion(random()));
         assertEquals(testItem.index(), deserializedInstance.index());
         assertEquals(testItem.shard(), deserializedInstance.shard());