Put codes of parsing master_timeout into a method

Signed-off-by: Tianli Feng <ftianli@amazon.com>
opensearch-project · Mar 15, 2022 · dec2597 · dec2597
1 parent 8e0e074
commit dec2597
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 21 deletions.
diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/cat.nodes.json b/rest-api-spec/src/main/resources/rest-api-spec/api/cat.nodes.json
@@ -59,7 +59,7 @@
       },
       "cluster_manager_timeout":{
         "type":"time",
-        "description":"Explicit operation timeout for connection to master node"
+        "description":"Explicit operation timeout for connection to cluster-manager node"
       },
       "h":{
         "type":"list",

diff --git a/server/src/main/java/org/opensearch/rest/action/cat/AbstractCatAction.java b/server/src/main/java/org/opensearch/rest/action/cat/AbstractCatAction.java
@@ -58,10 +58,6 @@ public abstract class AbstractCatAction extends BaseRestHandler {
 
     protected abstract Table getTableWithHeader(RestRequest request);
 
-    // TODO: Remove this line after removing MASTER_ROLE. It's used for the REST CAT actions that accepts parameter 'master_timeout'
-    protected static final String DEPRECATED_MESSAGE_MASTER_TIMEOUT =
-        "Deprecated parameter [master_timeout] used. To promote inclusive language, please use [cluster_manager_timeout] instead. It will be unsupported in a future major version.";
-
     @Override
     public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
         boolean helpWanted = request.paramAsBoolean("help", false);

diff --git a/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java b/server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java
@@ -110,14 +110,8 @@ public RestChannelConsumer doCatRequest(final RestRequest request, final NodeCli
             deprecationLogger.deprecate("cat_nodes_local_parameter", LOCAL_DEPRECATED_MESSAGE);
         }
         clusterStateRequest.local(request.paramAsBoolean("local", clusterStateRequest.local()));
-        // TODO: Remove below lines about 'master_timeout', after removing MASTER_ROLE.
-        clusterStateRequest.masterNodeTimeout(request.paramAsTime("master_timeout", clusterStateRequest.masterNodeTimeout()));
-        // Add "cluster_manager_timeout" as the alternative to "master_timeout", for promoting inclusive language.
-        if (request.hasParam("master_timeout")) {
-            deprecationLogger.deprecate("cat_nodes_master_timeout_parameter", DEPRECATED_MESSAGE_MASTER_TIMEOUT);
-        }
-        request.validateParamValuesAreEqual("master_timeout", "cluster_manager_timeout");
         clusterStateRequest.masterNodeTimeout(request.paramAsTime("cluster_manager_timeout", clusterStateRequest.masterNodeTimeout()));
+        parseDeprecatedMasterTimeoutParam(clusterStateRequest, request);
         final boolean fullId = request.paramAsBoolean("full_id", false);
         return channel -> client.admin().cluster().state(clusterStateRequest, new RestActionListener<ClusterStateResponse>(channel) {
             @Override
@@ -532,4 +526,27 @@ Table buildTable(
     private short calculatePercentage(long used, long max) {
         return max <= 0 ? 0 : (short) ((100d * used) / max);
     }
+
+    /**
+     * Parse the deprecated request parameter 'master_timeout', and add deprecated log if the parameter is used.
+     * It also validates whether the value of 'master_timeout' is the same with 'cluster_manager_timeout'.
+     * Remove the method along with MASTER_ROLE.
+     * @deprecated As of 2.0, because promoting inclusive language.
+     */
+    @Deprecated
+    private void parseDeprecatedMasterTimeoutParam(ClusterStateRequest clusterStateRequest, RestRequest request) {
+        final String deprecatedTimeoutParam = "master_timeout";
+        final String clusterManagerTimeoutParam = "cluster_manager_timeout";
+        final String deprecatedMessage = String.format(
+            Locale.US,
+            "Deprecated parameter [%s] used. To promote inclusive language, please use [%s] instead. It will be unsupported in a future major version.",
+            deprecatedTimeoutParam,
+            clusterManagerTimeoutParam
+        );
+        clusterStateRequest.masterNodeTimeout(request.paramAsTime(deprecatedTimeoutParam, clusterStateRequest.masterNodeTimeout()));
+        if (request.hasParam(deprecatedTimeoutParam)) {
+            deprecationLogger.deprecate("cat_nodes_master_timeout_parameter", deprecatedMessage);
+            request.validateParamValuesAreEqual(deprecatedTimeoutParam, clusterManagerTimeoutParam);
+        }
+    }
 }
diff --git a/server/src/main/java/org/opensearch/rest/action/cat/RestPluginsAction.java b/server/src/main/java/org/opensearch/rest/action/cat/RestPluginsAction.java
@@ -42,7 +42,6 @@
 import org.opensearch.cluster.node.DiscoveryNode;
 import org.opensearch.cluster.node.DiscoveryNodes;
 import org.opensearch.common.Table;
-import org.opensearch.common.logging.DeprecationLogger;
 import org.opensearch.plugins.PluginInfo;
 import org.opensearch.rest.RestRequest;
 import org.opensearch.rest.RestResponse;
@@ -55,7 +54,6 @@
 import static org.opensearch.rest.RestRequest.Method.GET;
 
 public class RestPluginsAction extends AbstractCatAction {
-    private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestPluginsAction.class);
 
     @Override
     public List<Route> routes() {
@@ -77,14 +75,7 @@ public RestChannelConsumer doCatRequest(final RestRequest request, final NodeCli
         final ClusterStateRequest clusterStateRequest = new ClusterStateRequest();
         clusterStateRequest.clear().nodes(true);
         clusterStateRequest.local(request.paramAsBoolean("local", clusterStateRequest.local()));
-        // TODO: Remove below lines about 'master_timeout', after removing MASTER_ROLE.
         clusterStateRequest.masterNodeTimeout(request.paramAsTime("master_timeout", clusterStateRequest.masterNodeTimeout()));
-        // Add "cluster_manager_timeout" as the alternative to "master_timeout", for promoting inclusive language.
-        if (request.hasParam("master_timeout")) {
-            deprecationLogger.deprecate("cat_nodes_master_timeout_parameter", DEPRECATED_MESSAGE_MASTER_TIMEOUT);
-        }
-        request.validateParamValuesAreEqual("master_timeout", "cluster_manager_timeout");
-        clusterStateRequest.masterNodeTimeout(request.paramAsTime("cluster_manager_timeout", clusterStateRequest.masterNodeTimeout()));
 
         return channel -> client.admin().cluster().state(clusterStateRequest, new RestActionListener<ClusterStateResponse>(channel) {
             @Override