Fix GetRepositoriesResponse streamable and get rid of rid of readFrom

method in GetSnapshotsResponse

Author: Andrey Ershov

server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/get/GetRepositoriesAction.java

@@ -20,6 +20,7 @@
 package org.elasticsearch.action.admin.cluster.repositories.get;
 
 import org.elasticsearch.action.Action;
+import org.elasticsearch.common.io.stream.Writeable;
 
 /**
  * Get repositories action
@@ -35,7 +36,12 @@ private GetRepositoriesAction() {
 
     @Override
     public GetRepositoriesResponse newResponse() {
-        return new GetRepositoriesResponse();
+        throw new UnsupportedOperationException("usage of Streamable is to be replaced by Writeable");
+    }
+
+    @Override
+    public Writeable.Reader<GetRepositoriesResponse> getResponseReader() {
+        return GetRepositoriesResponse::new;
     }
 }
 

server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/get/GetRepositoriesResponse.java

@@ -29,7 +29,6 @@
 import org.elasticsearch.common.xcontent.XContentParser;
 
 import java.io.IOException;
-import java.util.Collections;
 import java.util.List;
 
 import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureExpectedToken;
@@ -41,10 +40,6 @@ public class GetRepositoriesResponse extends ActionResponse implements ToXConten
 
     private final RepositoriesMetaData repositories;
 
-    GetRepositoriesResponse() {
-        repositories = new RepositoriesMetaData(Collections.emptyList());
-    }
-
     GetRepositoriesResponse(RepositoriesMetaData repositories) {
         this.repositories = repositories;
     }

server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/get/TransportGetRepositoriesAction.java

@@ -31,11 +31,13 @@
 import org.elasticsearch.cluster.metadata.RepositoryMetaData;
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.inject.Inject;
+import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.regex.Regex;
 import org.elasticsearch.repositories.RepositoryMissingException;
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.transport.TransportService;
 
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.LinkedHashSet;
@@ -62,7 +64,12 @@ protected String executor() {
 
     @Override
     protected GetRepositoriesResponse newResponse() {
-        return new GetRepositoriesResponse();
+        throw new UnsupportedOperationException("usage of Streamable is to be replaced by Writeable");
+    }
+
+    @Override
+    protected GetRepositoriesResponse read(StreamInput in) throws IOException {
+        return new GetRepositoriesResponse(in);
     }
 
     @Override

server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsAction.java

@@ -20,6 +20,7 @@
 package org.elasticsearch.action.admin.cluster.snapshots.get;
 
 import org.elasticsearch.action.Action;
+import org.elasticsearch.common.io.stream.Writeable;
 
 /**
  * Get snapshots action
@@ -35,7 +36,12 @@ private GetSnapshotsAction() {
 
     @Override
     public GetSnapshotsResponse newResponse() {
-        return new GetSnapshotsResponse();
+        throw new UnsupportedOperationException("usage of Streamable is to be replaced by Writeable");
+    }
+
+    @Override
+    public Writeable.Reader<GetSnapshotsResponse> getResponseReader() {
+        return GetSnapshotsResponse::new;
     }
 }
 

server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsResponse.java

@@ -55,6 +55,40 @@ public class GetSnapshotsResponse extends ActionResponse implements ToXContentOb
                 (p, c) -> Response.fromXContent(p), new ParseField("responses"));
     }
 
+    public GetSnapshotsResponse(StreamInput in) throws IOException {
+        if (in.getVersion().onOrAfter(GetSnapshotsRequest.MULTIPLE_REPOSITORIES_SUPPORT_ADDED)) {
+            int successfulSize = in.readVInt();
+            Map<String, List<SnapshotInfo>> successfulResponses = new HashMap<>(successfulSize);
+            for (int i = 0; i < successfulSize; i++) {
+                String repository = in.readString();
+                int size = in.readVInt();
+                List<SnapshotInfo> snapshotInfos = new ArrayList<>(size);
+                for (int j = 0; j < size; j++) {
+                    snapshotInfos.add(new SnapshotInfo(in));
+                }
+                successfulResponses.put(repository, snapshotInfos);
+            }
+
+            int failedSize = in.readVInt();
+            Map<String, ElasticsearchException> failedResponses = new HashMap<>(failedSize);
+            for (int i = 0; i < failedSize; i++) {
+                String repository = in.readString();
+                ElasticsearchException error = in.readException();
+                failedResponses.put(repository, error);
+            }
+            this.successfulResponses = Collections.unmodifiableMap(successfulResponses);
+            this.failedResponses = Collections.unmodifiableMap(failedResponses);
+        } else {
+            int size = in.readVInt();
+            List<SnapshotInfo> snapshots = new ArrayList<>(size);
+            for (int i = 0; i < size; i++) {
+                snapshots.add(new SnapshotInfo(in));
+            }
+            this.successfulResponses = Collections.singletonMap("unknown", snapshots);
+            this.failedResponses = Collections.emptyMap();
+        }
+    }
+
 
     public static class Response {
         private String repository;
@@ -93,26 +127,23 @@ private static Response fromXContent(XContentParser parser) throws IOException {
         }
     }
 
-    private Map<String, List<SnapshotInfo>> successfulResponses = Collections.emptyMap();
-    private Map<String, ElasticsearchException> failedResponses = Collections.emptyMap();
+    private final Map<String, List<SnapshotInfo>> successfulResponses;
+    private final Map<String, ElasticsearchException> failedResponses;
 
     public GetSnapshotsResponse(Collection<Response> responses) {
-        this.successfulResponses = new HashMap<>();
-        this.failedResponses = new HashMap<>();
+        Map<String, List<SnapshotInfo>> successfulResponses = new HashMap<>();
+        Map<String, ElasticsearchException> failedResponses = new HashMap<>();
         for (Response response : responses) {
             if (response.snapshots != null) {
                 assert response.error == null;
-                this.successfulResponses.put(response.repository, response.snapshots);
+                successfulResponses.put(response.repository, response.snapshots);
             } else {
                 assert response.snapshots == null;
-                this.failedResponses.put(response.repository, response.error);
+                failedResponses.put(response.repository, response.error);
             }
         }
-        successfulResponses = Collections.unmodifiableMap(successfulResponses);
-        failedResponses = Collections.unmodifiableMap(failedResponses);
-    }
-
-    public GetSnapshotsResponse() {
+        this.successfulResponses = Collections.unmodifiableMap(successfulResponses);
+        this.failedResponses = Collections.unmodifiableMap(failedResponses);
     }
 
     /**
@@ -229,37 +260,7 @@ public void writeTo(StreamOutput out) throws IOException {
 
     @Override
     public void readFrom(StreamInput in) throws IOException {
-        super.readFrom(in);
-        if (in.getVersion().onOrAfter(GetSnapshotsRequest.MULTIPLE_REPOSITORIES_SUPPORT_ADDED)) {
-            int successfulSize = in.readVInt();
-            successfulResponses = new HashMap<>(successfulSize);
-            for (int i = 0; i < successfulSize; i++) {
-                String repository = in.readString();
-                int size = in.readVInt();
-                List<SnapshotInfo> snapshotInfos = new ArrayList<>(size);
-                for (int j = 0; j < size; j++) {
-                    snapshotInfos.add(new SnapshotInfo(in));
-                }
-                successfulResponses.put(repository, snapshotInfos);
-            }
-
-            int failedSize = in.readVInt();
-            failedResponses = new HashMap<>(failedSize);
-            for (int i = 0; i < failedSize; i++) {
-                String repository = in.readString();
-                ElasticsearchException error = in.readException();
-                failedResponses.put(repository, error);
-            }
-            successfulResponses = Collections.unmodifiableMap(successfulResponses);
-            failedResponses = Collections.unmodifiableMap(failedResponses);
-        } else {
-            int size = in.readVInt();
-            List<SnapshotInfo> snapshots = new ArrayList<>(size);
-            for (int i = 0; i < size; i++) {
-                snapshots.add(new SnapshotInfo(in));
-            }
-            successfulResponses = Collections.singletonMap("unknown", snapshots);
-        }
+        throw new UnsupportedOperationException("usage of Streamable is to be replaced by Writeable");
     }
 
     public static GetSnapshotsResponse fromXContent(XContentParser parser) throws IOException {

server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java

@@ -37,6 +37,7 @@
 import org.elasticsearch.cluster.metadata.RepositoryMetaData;
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.inject.Inject;
+import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.regex.Regex;
 import org.elasticsearch.repositories.IndexId;
 import org.elasticsearch.repositories.RepositoryData;
@@ -47,6 +48,7 @@
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.transport.TransportService;
 
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
@@ -78,7 +80,12 @@ protected String executor() {
 
     @Override
     protected GetSnapshotsResponse newResponse() {
-        return new GetSnapshotsResponse();
+        throw new UnsupportedOperationException("usage of Streamable is to be replaced by Writeable");
+    }
+
+    @Override
+    protected GetSnapshotsResponse read(StreamInput in) throws IOException {
+        return new GetSnapshotsResponse(in);
     }
 
     @Override

server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsResponseTests.java

@@ -59,7 +59,9 @@ private GetSnapshotsResponse doParseInstance(XContentParser parser) throws IOExc
     }
 
     private GetSnapshotsResponse copyInstance(GetSnapshotsResponse instance, Version version) throws IOException {
-        return copyStreamable(instance, new NamedWriteableRegistry(Collections.emptyList()), () -> new GetSnapshotsResponse(), version);
+        return copyInstance(instance, new NamedWriteableRegistry(Collections.emptyList()), (out, value) -> value.writeTo(out),
+                in -> new GetSnapshotsResponse(in), version);
+
     }
 
     private void assertEqualInstances(GetSnapshotsResponse expectedInstance, GetSnapshotsResponse newInstance) {