[CCR] Removed custom follow and unfollow api's reponse classes with AcknowledgedResponse (#33260)

martijnvg · martijnvg · commit 2f0a3bb97942 · 2018-08-31T21:40:01.000+07:00
These response classes did not add any value and in that case just AcknowledgedResponse should be used.

I also changed the formatting of methods to take one line per parameter in
FollowIndexAction.java and UnfollowIndexAction.java files to make
reviewing diffs in the future easier.
diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/FollowIndexAction.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/FollowIndexAction.java
@@ -65,7 +65,7 @@
 import java.util.stream.Collectors;
 
 public class FollowIndexAction extends Action<FollowIndexAction.Request,
-        FollowIndexAction.Response, FollowIndexAction.RequestBuilder> {
+    AcknowledgedResponse, FollowIndexAction.RequestBuilder> {
 
     public static final FollowIndexAction INSTANCE = new FollowIndexAction();
     public static final String NAME = "cluster:admin/xpack/ccr/follow_index";
@@ -80,8 +80,8 @@ public RequestBuilder newRequestBuilder(ElasticsearchClient client) {
     }
 
     @Override
-    public Response newResponse() {
-        return new Response();
+    public AcknowledgedResponse newResponse() {
+        return new AcknowledgedResponse();
     }
 
     public static class Request extends ActionRequest implements ToXContentObject {
@@ -137,9 +137,17 @@ public static Request fromXContent(XContentParser parser, String followerIndex)
         private TimeValue retryTimeout;
         private TimeValue idleShardRetryDelay;
 
-        public Request(String leaderIndex, String followerIndex, Integer maxBatchOperationCount, Integer maxConcurrentReadBatches,
-                       Long maxOperationSizeInBytes, Integer maxConcurrentWriteBatches, Integer maxWriteBufferSize,
-                       TimeValue retryTimeout, TimeValue idleShardRetryDelay) {
+        public Request(
+            String leaderIndex,
+            String followerIndex,
+            Integer maxBatchOperationCount,
+            Integer maxConcurrentReadBatches,
+            Long maxOperationSizeInBytes,
+            Integer maxConcurrentWriteBatches,
+            Integer maxWriteBufferSize,
+            TimeValue retryTimeout,
+            TimeValue idleShardRetryDelay) {
+
             if (leaderIndex == null) {
                 throw new IllegalArgumentException("leader_index is missing");
             }
@@ -279,8 +287,17 @@ public boolean equals(Object o) {
 
         @Override
         public int hashCode() {
-            return Objects.hash(leaderIndex, followerIndex, maxBatchOperationCount, maxConcurrentReadBatches, maxOperationSizeInBytes,
-                maxConcurrentWriteBatches, maxWriteBufferSize, retryTimeout, idleShardRetryDelay);
+            return Objects.hash(
+                leaderIndex,
+                followerIndex,
+                maxBatchOperationCount,
+                maxConcurrentReadBatches,
+                maxOperationSizeInBytes,
+                maxConcurrentWriteBatches,
+                maxWriteBufferSize,
+                retryTimeout,
+                idleShardRetryDelay
+            );
         }
     }
 
@@ -291,17 +308,7 @@ public static class RequestBuilder extends ActionRequestBuilder<Request, Respons
         }
     }
 
-    public static class Response extends AcknowledgedResponse {
-
-        Response() {
-        }
-
-        Response(boolean acknowledged) {
-            super(acknowledged);
-        }
-    }
-
-    public static class TransportAction extends HandledTransportAction<Request, Response> {
+    public static class TransportAction extends HandledTransportAction<Request, AcknowledgedResponse> {
 
         private final Client client;
         private final ClusterService clusterService;
@@ -332,7 +339,7 @@ public TransportAction(
         }
 
         @Override
-        protected void doExecute(final Request request, final ActionListener<Response> listener) {
+        protected void doExecute(final  Request request, final ActionListener<AcknowledgedResponse> listener) {
             if (ccrLicenseChecker.isCcrAllowed()) {
                 final String[] indices = new String[]{request.leaderIndex};
                 final Map<String, List<String>> remoteClusterIndices = remoteClusterService.groupClusterIndices(indices, s -> false);
@@ -351,7 +358,8 @@ protected void doExecute(final Request request, final ActionListener<Response> l
             }
         }
 
-        private void followLocalIndex(final Request request, final ActionListener<Response> listener) {
+        private void followLocalIndex(final Request request,
+                                      final ActionListener<AcknowledgedResponse> listener) {
             final ClusterState state = clusterService.state();
             final IndexMetaData followerIndexMetadata = state.getMetaData().index(request.getFollowerIndex());
             // following an index in local cluster, so use local cluster state to fetch leader index metadata
@@ -367,7 +375,7 @@ private void followRemoteIndex(
                 final Request request,
                 final String clusterAlias,
                 final String leaderIndex,
-                final ActionListener<Response> listener) {
+                final ActionListener<AcknowledgedResponse> listener) {
             final ClusterState state = clusterService.state();
             final IndexMetaData followerIndexMetadata = state.getMetaData().index(request.getFollowerIndex());
             ccrLicenseChecker.checkRemoteClusterLicenseAndFetchLeaderIndexMetadata(
@@ -394,8 +402,13 @@ private void followRemoteIndex(
          *     <li>The leader index and follow index need to have the same number of primary shards</li>
          * </ul>
          */
-        void start(Request request, String clusterNameAlias, IndexMetaData leaderIndexMetadata, IndexMetaData followIndexMetadata,
-                   ActionListener<Response> handler) throws IOException {
+        void start(
+            Request request,
+            String clusterNameAlias,
+            IndexMetaData leaderIndexMetadata,
+            IndexMetaData followIndexMetadata,
+            ActionListener<AcknowledgedResponse> handler) throws IOException {
+
             MapperService mapperService = followIndexMetadata != null ? indicesService.createIndexMapperService(followIndexMetadata) : null;
             validate(request, leaderIndexMetadata, followIndexMetadata, mapperService);
             final int numShards = followIndexMetadata.getNumberOfShards();
@@ -443,7 +456,7 @@ void finalizeResponse() {
 
                                 if (error == null) {
                                     // include task ids?
-                                    handler.onResponse(new Response(true));
+                                    handler.onResponse(new AcknowledgedResponse(true));
                                 } else {
                                     // TODO: cancel all started tasks
                                     handler.onFailure(error);
@@ -506,7 +519,9 @@ void finalizeResponse() {
         WHITELISTED_SETTINGS = Collections.unmodifiableSet(whiteListedSettings);
     }
 
-    static void validate(Request request, IndexMetaData leaderIndex, IndexMetaData followIndex, MapperService followerMapperService) {
+    static void validate(Request request,
+                         IndexMetaData leaderIndex,
+                         IndexMetaData followIndex, MapperService followerMapperService) {
         if (leaderIndex == null) {
             throw new IllegalArgumentException("leader index [" + request.leaderIndex + "] does not exist");
         }
diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/UnfollowIndexAction.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/action/UnfollowIndexAction.java
@@ -31,7 +31,7 @@
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReferenceArray;
 
-public class UnfollowIndexAction extends Action<UnfollowIndexAction.Request, UnfollowIndexAction.Response,
+public class UnfollowIndexAction extends Action<UnfollowIndexAction.Request, AcknowledgedResponse,
         UnfollowIndexAction.RequestBuilder> {
 
     public static final UnfollowIndexAction INSTANCE = new UnfollowIndexAction();
@@ -47,8 +47,8 @@ public RequestBuilder newRequestBuilder(ElasticsearchClient client) {
     }
 
     @Override
-    public Response newResponse() {
-        return new Response();
+    public AcknowledgedResponse newResponse() {
+        return new AcknowledgedResponse();
     }
 
     public static class Request extends ActionRequest {
@@ -81,16 +81,6 @@ public void writeTo(StreamOutput out) throws IOException {
         }
     }
 
-    public static class Response extends AcknowledgedResponse {
-
-        Response(boolean acknowledged) {
-            super(acknowledged);
-        }
-
-        Response() {
-        }
-    }
-
     public static class RequestBuilder extends ActionRequestBuilder<Request, Response, RequestBuilder> {
 
         RequestBuilder(ElasticsearchClient client) {
@@ -99,22 +89,27 @@ public static class RequestBuilder extends ActionRequestBuilder<Request, Respons
     }
 
 
-    public static class TransportAction extends HandledTransportAction<Request, Response> {
+    public static class TransportAction extends HandledTransportAction<Request, AcknowledgedResponse> {
 
         private final Client client;
         private final PersistentTasksService persistentTasksService;
 
         @Inject
-        public TransportAction(Settings settings, ThreadPool threadPool, TransportService transportService, ActionFilters actionFilters,
-                               IndexNameExpressionResolver indexNameExpressionResolver, Client client,
+        public TransportAction(Settings settings,
+                               ThreadPool threadPool,
+                               TransportService transportService,
+                               ActionFilters actionFilters,
+                               IndexNameExpressionResolver indexNameExpressionResolver,
+                               Client client,
                                PersistentTasksService persistentTasksService) {
-            super(settings, NAME, threadPool, transportService, actionFilters, indexNameExpressionResolver, Request::new);
+            super(settings, NAME, threadPool,transportService, actionFilters, indexNameExpressionResolver,Request::new);
             this.client = client;
             this.persistentTasksService = persistentTasksService;
         }
 
         @Override
-        protected void doExecute(Request request, ActionListener<Response> listener) {
+        protected void doExecute(Request request,
+                                 ActionListener<AcknowledgedResponse> listener) {
             client.admin().cluster().state(new ClusterStateRequest(), ActionListener.wrap(r -> {
                 IndexMetaData followIndexMetadata = r.getState().getMetaData().index(request.followIndex);
                 if (followIndexMetadata == null) {
@@ -158,7 +153,7 @@ void finalizeResponse() {
 
                                 if (error == null) {
                                     // include task ids?
-                                    listener.onResponse(new Response(true));
+                                    listener.onResponse(new AcknowledgedResponse(true));
                                 } else {
                                     // TODO: cancel all started tasks
                                     listener.onFailure(error);
diff --git a/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/CcrLicenseIT.java b/x-pack/plugin/ccr/src/test/java/org/elasticsearch/xpack/ccr/CcrLicenseIT.java
@@ -8,6 +8,7 @@
 
 import org.elasticsearch.ElasticsearchSecurityException;
 import org.elasticsearch.action.ActionListener;
+import org.elasticsearch.action.support.master.AcknowledgedResponse;
 import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.test.ESSingleNodeTestCase;
@@ -36,9 +37,9 @@ public void testThatFollowingIndexIsUnavailableWithIncompatibleLicense() throws
         client().execute(
                 FollowIndexAction.INSTANCE,
                 followRequest,
-                new ActionListener<FollowIndexAction.Response>() {
+                new ActionListener<AcknowledgedResponse>() {
                     @Override
-                    public void onResponse(final FollowIndexAction.Response response) {
+                    public void onResponse(final AcknowledgedResponse response) {
                         fail();
                     }
 
