Replace Streamable w/ Writable in AcknowledgedResponse and subclasses (…

…#43414)

This commit replaces usages of Streamable with Writeable for the
AcknowledgedResponse and its subclasses, plus associated actions.

Note that where possible response fields were made final and default
constructors were removed.

This is a large PR, but the change is mostly mechanical.

Relates to #34389

client/rest-high-level/src/test/java/org/elasticsearch/client/AbstractHlrcWriteableXContentTestCase.java

@@ -0,0 +1,70 @@
+/*
+ * 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.client;
+
+import org.elasticsearch.common.io.stream.Writeable;
+import org.elasticsearch.common.xcontent.ToXContent;
+import org.elasticsearch.common.xcontent.XContentParser;
+import org.elasticsearch.test.AbstractSerializingTestCase;
+
+import java.io.IOException;
+
+import static org.elasticsearch.test.AbstractXContentTestCase.xContentTester;
+
+/**
+ * @deprecated Use {@link AbstractResponseTestCase} instead of this class.
+ */
+// TODO: Remove and change subclasses to use AbstractResponseTestCase instead
+@Deprecated
+public abstract class AbstractHlrcWriteableXContentTestCase<T extends ToXContent & Writeable, H>
+        extends AbstractSerializingTestCase<T> {
+
+    /**
+     * Generic test that creates new instance of HLRC request/response from the test instance and checks
+     * both for equality and asserts equality on the two queries.
+     */
+    public final void testHlrcFromXContent() throws IOException {
+        xContentTester(this::createParser, this::createTestInstance, getToXContentParams(),
+            p -> convertHlrcToInternal(doHlrcParseInstance(p)))
+            .numberOfTestRuns(NUMBER_OF_TEST_RUNS)
+            .supportsUnknownFields(supportsUnknownFields())
+            .shuffleFieldsExceptions(getShuffleFieldsExceptions())
+            .randomFieldsExcludeFilter(getRandomFieldsExcludeFilter())
+            .assertEqualsConsumer(this::assertEqualInstances)
+            .assertToXContentEquivalence(true)
+            .test();
+    }
+
+    /**
+     * Parses to a new HLRC instance using the provided {@link XContentParser}
+     */
+    public abstract H doHlrcParseInstance(XContentParser parser) throws IOException;
+
+    /**
+     * Converts a HLRC instance to a XPack instance
+     */
+    public abstract T convertHlrcToInternal(H instance);
+
+    //TODO this would be final ideally: why do both responses need to parse from xcontent, only one (H) should? I think that T#fromXContent
+    //are only there for testing and could go away? Then the additional testHlrcFromXContent is also no longer needed.
+    @Override
+    protected T doParseInstance(XContentParser parser) throws IOException {
+        return convertHlrcToInternal(doHlrcParseInstance(parser));
+    }
+}

client/rest-high-level/src/test/java/org/elasticsearch/client/license/PutLicenseResponseTests.java

@@ -18,8 +18,9 @@
  */
 package org.elasticsearch.client.license;
 
+import org.elasticsearch.client.AbstractHlrcWriteableXContentTestCase;
+import org.elasticsearch.common.io.stream.Writeable;
 import org.elasticsearch.common.xcontent.XContentParser;
-import org.elasticsearch.client.AbstractHlrcStreamableXContentTestCase;
 import org.elasticsearch.protocol.xpack.license.LicensesStatus;
 
 import java.io.IOException;
@@ -29,8 +30,8 @@
 import java.util.function.Function;
 import java.util.function.Predicate;
 
-public class PutLicenseResponseTests extends AbstractHlrcStreamableXContentTestCase<
-    org.elasticsearch.protocol.xpack.license.PutLicenseResponse, org.elasticsearch.client.license.PutLicenseResponse> {
+public class PutLicenseResponseTests extends AbstractHlrcWriteableXContentTestCase<
+    org.elasticsearch.protocol.xpack.license.PutLicenseResponse, PutLicenseResponse> {
 
     @Override
     public org.elasticsearch.client.license.PutLicenseResponse doHlrcParseInstance(XContentParser parser) throws IOException {
@@ -96,8 +97,8 @@ private static Map<String, String[]> randomAckMessages() {
     }
 
     @Override
-    protected org.elasticsearch.protocol.xpack.license.PutLicenseResponse createBlankInstance() {
-        return new org.elasticsearch.protocol.xpack.license.PutLicenseResponse();
+    protected Writeable.Reader<org.elasticsearch.protocol.xpack.license.PutLicenseResponse> instanceReader() {
+        return org.elasticsearch.protocol.xpack.license.PutLicenseResponse::new;
     }
 
     @Override

client/rest-high-level/src/test/java/org/elasticsearch/client/license/StartBasicResponseTests.java

@@ -18,19 +18,20 @@
  */
 package org.elasticsearch.client.license;
 
+import org.elasticsearch.client.AbstractHlrcWriteableXContentTestCase;
 import org.elasticsearch.common.Strings;
+import org.elasticsearch.common.io.stream.Writeable;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.license.PostStartBasicResponse;
-import org.elasticsearch.client.AbstractHlrcStreamableXContentTestCase;
 
 import java.io.IOException;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.function.Predicate;
 
-public class StartBasicResponseTests extends
-        AbstractHlrcStreamableXContentTestCase<PostStartBasicResponse, org.elasticsearch.client.license.StartBasicResponse> {
+public class StartBasicResponseTests extends AbstractHlrcWriteableXContentTestCase<
+    PostStartBasicResponse, StartBasicResponse> {
 
     @Override
     public org.elasticsearch.client.license.StartBasicResponse doHlrcParseInstance(XContentParser parser) throws IOException {
@@ -44,8 +45,8 @@ public PostStartBasicResponse convertHlrcToInternal(org.elasticsearch.client.lic
     }
 
     @Override
-    protected PostStartBasicResponse createBlankInstance() {
-        return new PostStartBasicResponse();
+    protected Writeable.Reader<PostStartBasicResponse> instanceReader() {
+        return PostStartBasicResponse::new;
     }
 
     @Override

server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/DeleteRepositoryAction.java

@@ -21,6 +21,7 @@
 
 import org.elasticsearch.action.Action;
 import org.elasticsearch.action.support.master.AcknowledgedResponse;
+import org.elasticsearch.common.io.stream.Writeable;
 
 /**
  * Unregister repository action
@@ -36,7 +37,12 @@ private DeleteRepositoryAction() {
 
     @Override
     public AcknowledgedResponse newResponse() {
-        return new AcknowledgedResponse();
+        throw new UnsupportedOperationException("usage of Streamable is to be replaced by Writeable");
+    }
+
+    @Override
+    public Writeable.Reader<AcknowledgedResponse> getResponseReader() {
+        return AcknowledgedResponse::new;
     }
 }
 

server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/delete/TransportDeleteRepositoryAction.java

@@ -29,10 +29,13 @@
 import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.inject.Inject;
+import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.repositories.RepositoriesService;
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.transport.TransportService;
 
+import java.io.IOException;
+
 /**
  * Transport action for unregister repository operation
  */
@@ -54,9 +57,14 @@ protected String executor() {
         return ThreadPool.Names.SAME;
     }
 
+    @Override
+    protected AcknowledgedResponse read(StreamInput in) throws IOException {
+        return new AcknowledgedResponse(in);
+    }
+
     @Override
     protected AcknowledgedResponse newResponse() {
-        return new AcknowledgedResponse();
+        throw new UnsupportedOperationException("usage of Streamable is to be replaced by Writeable");
     }
 
     @Override

server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/PutRepositoryAction.java

@@ -21,6 +21,7 @@
 
 import org.elasticsearch.action.Action;
 import org.elasticsearch.action.support.master.AcknowledgedResponse;
+import org.elasticsearch.common.io.stream.Writeable;
 
 /**
  * Register repository action
@@ -36,7 +37,12 @@ private PutRepositoryAction() {
 
     @Override
     public AcknowledgedResponse newResponse() {
-        return new AcknowledgedResponse();
+        throw new UnsupportedOperationException("usage of Streamable is to be replaced by Writeable");
+    }
+
+    @Override
+    public Writeable.Reader<AcknowledgedResponse> getResponseReader() {
+        return AcknowledgedResponse::new;
     }
 }
 

server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/put/TransportPutRepositoryAction.java

@@ -29,10 +29,13 @@
 import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.inject.Inject;
+import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.repositories.RepositoriesService;
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.transport.TransportService;
 
+import java.io.IOException;
+
 /**
  * Transport action for register repository operation
  */
@@ -54,9 +57,14 @@ protected String executor() {
         return ThreadPool.Names.SAME;
     }
 
+    @Override
+    protected AcknowledgedResponse read(StreamInput in) throws IOException {
+        return new AcknowledgedResponse(in);
+    }
+
     @Override
     protected AcknowledgedResponse newResponse() {
-        return new AcknowledgedResponse();
+        throw new UnsupportedOperationException("usage of Streamable is to be replaced by Writeable");
     }
 
     @Override

server/src/main/java/org/elasticsearch/action/admin/cluster/reroute/ClusterRerouteAction.java

@@ -20,6 +20,7 @@
 package org.elasticsearch.action.admin.cluster.reroute;
 
 import org.elasticsearch.action.Action;
+import org.elasticsearch.common.io.stream.Writeable;
 
 public class ClusterRerouteAction extends Action<ClusterRerouteResponse> {
 
@@ -32,6 +33,11 @@ private ClusterRerouteAction() {
 
     @Override
     public ClusterRerouteResponse newResponse() {
-        return new ClusterRerouteResponse();
+        throw new UnsupportedOperationException("usage of Streamable is to be replaced by Writeable");
+    }
+
+    @Override
+    public Writeable.Reader<ClusterRerouteResponse> getResponseReader() {
+        return ClusterRerouteResponse::new;
     }
 }

server/src/main/java/org/elasticsearch/action/admin/cluster/reroute/ClusterRerouteResponse.java

@@ -35,11 +35,13 @@
  */
 public class ClusterRerouteResponse extends AcknowledgedResponse implements ToXContentObject {
 
-    private ClusterState state;
-    private RoutingExplanations explanations;
-
-    ClusterRerouteResponse() {
+    private final ClusterState state;
+    private final RoutingExplanations explanations;
 
+    ClusterRerouteResponse(StreamInput in) throws IOException {
+        super(in);
+        state = ClusterState.readFrom(in, null);
+        explanations = RoutingExplanations.readFrom(in);
     }
 
     ClusterRerouteResponse(boolean acknowledged, ClusterState state, RoutingExplanations explanations) {
@@ -59,13 +61,6 @@ public RoutingExplanations getExplanations() {
         return this.explanations;
     }
 
-    @Override
-    public void readFrom(StreamInput in) throws IOException {
-        super.readFrom(in);
-        state = ClusterState.readFrom(in, null);
-        explanations = RoutingExplanations.readFrom(in);
-    }
-
     @Override
     public void writeTo(StreamOutput out) throws IOException {
         super.writeTo(out);

server/src/main/java/org/elasticsearch/action/admin/cluster/reroute/TransportClusterRerouteAction.java

@@ -46,9 +46,11 @@
 import org.elasticsearch.common.collect.ImmutableOpenIntMap;
 import org.elasticsearch.common.collect.ImmutableOpenMap;
 import org.elasticsearch.common.inject.Inject;
+import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.transport.TransportService;
 
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
@@ -80,7 +82,12 @@ protected ClusterBlockException checkBlock(ClusterRerouteRequest request, Cluste
 
     @Override
     protected ClusterRerouteResponse newResponse() {
-        return new ClusterRerouteResponse();
+        throw new UnsupportedOperationException("usage of Streamable is to be replaced by Writeable");
+    }
+
+    @Override
+    protected ClusterRerouteResponse read(StreamInput in) throws IOException {
+        return new ClusterRerouteResponse(in);
     }
 
     @Override

server/src/main/java/org/elasticsearch/action/admin/cluster/settings/ClusterUpdateSettingsAction.java

@@ -20,6 +20,7 @@
 package org.elasticsearch.action.admin.cluster.settings;
 
 import org.elasticsearch.action.Action;
+import org.elasticsearch.common.io.stream.Writeable;
 
 public class ClusterUpdateSettingsAction extends Action<ClusterUpdateSettingsResponse> {
 
@@ -32,6 +33,11 @@ private ClusterUpdateSettingsAction() {
 
     @Override
     public ClusterUpdateSettingsResponse newResponse() {
-        return new ClusterUpdateSettingsResponse();
+        throw new UnsupportedOperationException("usage of Streamable is to be replaced by Writeable");
+    }
+
+    @Override
+    public Writeable.Reader<ClusterUpdateSettingsResponse> getResponseReader() {
+        return ClusterUpdateSettingsResponse::new;
     }
 }