Introduce RestUtils#getMasterNodeTimeout (#107986)

Many APIs accept a `?master_timeout` parameter, but reading this
parameter requires a little unnecessary boilerplate to specify the
literal parameter name and default value. Moreover, today's convention
is to construct a `MasterNodeRequest` and then read the default master
timeout from the freshly-created request. In practice this results in a
default of 30s, but we specify in the docs that this default is _always_
30s, and in principle one could create a transport request with a
different initial value which would deviate from the documented
behaviour.

This commit introduces a utility method for reading this parameter in a
fashion which is completely consistent with the documented behaviour.

Relates #107984

Author: DaveCTurner

modules/data-streams/src/main/java/org/elasticsearch/datastreams/lifecycle/rest/RestDataStreamLifecycleStatsAction.java

@@ -19,6 +19,7 @@
 import java.util.List;
 
 import static org.elasticsearch.rest.RestRequest.Method.GET;
+import static org.elasticsearch.rest.RestUtils.getMasterNodeTimeout;
 
 @ServerlessScope(Scope.PUBLIC)
 public class RestDataStreamLifecycleStatsAction extends BaseRestHandler {
@@ -36,7 +37,7 @@ public List<Route> routes() {
     @Override
     protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) {
         GetDataStreamLifecycleStatsAction.Request request = new GetDataStreamLifecycleStatsAction.Request();
-        request.masterNodeTimeout(restRequest.paramAsTime("master_timeout", request.masterNodeTimeout()));
+        request.masterNodeTimeout(getMasterNodeTimeout(restRequest));
         return channel -> client.execute(
             GetDataStreamLifecycleStatsAction.INSTANCE,
             request,

modules/data-streams/src/main/java/org/elasticsearch/datastreams/lifecycle/rest/RestExplainDataStreamLifecycleAction.java

@@ -21,6 +21,7 @@
 import java.util.List;
 
 import static org.elasticsearch.rest.RestRequest.Method.GET;
+import static org.elasticsearch.rest.RestUtils.getMasterNodeTimeout;
 
 @ServerlessScope(Scope.PUBLIC)
 public class RestExplainDataStreamLifecycleAction extends BaseRestHandler {
@@ -41,7 +42,7 @@ protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient
         ExplainDataStreamLifecycleAction.Request explainRequest = new ExplainDataStreamLifecycleAction.Request(indices);
         explainRequest.includeDefaults(restRequest.paramAsBoolean("include_defaults", false));
         explainRequest.indicesOptions(IndicesOptions.fromRequest(restRequest, IndicesOptions.strictExpandOpen()));
-        explainRequest.masterNodeTimeout(restRequest.paramAsTime("master_timeout", explainRequest.masterNodeTimeout()));
+        explainRequest.masterNodeTimeout(getMasterNodeTimeout(restRequest));
         return channel -> client.execute(
             ExplainDataStreamLifecycleAction.INSTANCE,
             explainRequest,

modules/data-streams/src/main/java/org/elasticsearch/datastreams/lifecycle/rest/RestPutDataStreamLifecycleAction.java

@@ -22,6 +22,7 @@
 import java.util.List;
 
 import static org.elasticsearch.rest.RestRequest.Method.PUT;
+import static org.elasticsearch.rest.RestUtils.getMasterNodeTimeout;
 
 @ServerlessScope(Scope.PUBLIC)
 public class RestPutDataStreamLifecycleAction extends BaseRestHandler {
@@ -41,7 +42,7 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli
         try (XContentParser parser = request.contentParser()) {
             PutDataStreamLifecycleAction.Request putLifecycleRequest = PutDataStreamLifecycleAction.Request.parseRequest(parser);
             putLifecycleRequest.indices(Strings.splitStringByCommaToArray(request.param("name")));
-            putLifecycleRequest.masterNodeTimeout(request.paramAsTime("master_timeout", putLifecycleRequest.masterNodeTimeout()));
+            putLifecycleRequest.masterNodeTimeout(getMasterNodeTimeout(request));
             putLifecycleRequest.ackTimeout(request.paramAsTime("timeout", putLifecycleRequest.ackTimeout()));
             putLifecycleRequest.indicesOptions(IndicesOptions.fromRequest(request, putLifecycleRequest.indicesOptions()));
             return channel -> client.execute(

modules/data-streams/src/main/java/org/elasticsearch/datastreams/rest/RestModifyDataStreamsAction.java

@@ -20,6 +20,7 @@
 import java.util.List;
 
 import static org.elasticsearch.rest.RestRequest.Method.POST;
+import static org.elasticsearch.rest.RestUtils.getMasterNodeTimeout;
 
 @ServerlessScope(Scope.PUBLIC)
 public class RestModifyDataStreamsAction extends BaseRestHandler {
@@ -43,7 +44,7 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli
         if (modifyDsRequest.getActions() == null || modifyDsRequest.getActions().isEmpty()) {
             throw new IllegalArgumentException("no data stream actions specified, at least one must be specified");
         }
-        modifyDsRequest.masterNodeTimeout(request.paramAsTime("master_timeout", modifyDsRequest.masterNodeTimeout()));
+        modifyDsRequest.masterNodeTimeout(getMasterNodeTimeout(request));
         modifyDsRequest.ackTimeout(request.paramAsTime("timeout", modifyDsRequest.ackTimeout()));
         return channel -> client.execute(ModifyDataStreamsAction.INSTANCE, modifyDsRequest, new RestToXContentListener<>(channel));
     }

qa/smoke-test-http/src/javaRestTest/java/org/elasticsearch/http/DanglingIndicesRestIT.java

@@ -33,6 +33,7 @@
 import static org.elasticsearch.cluster.metadata.IndexGraveyard.SETTING_MAX_TOMBSTONES;
 import static org.elasticsearch.indices.IndicesService.WRITE_DANGLING_INDICES_INFO_SETTING;
 import static org.elasticsearch.rest.RestStatus.ACCEPTED;
+import static org.elasticsearch.rest.RestUtils.REST_MASTER_TIMEOUT_PARAM;
 import static org.elasticsearch.test.XContentTestUtils.createJsonMapView;
 import static org.hamcrest.Matchers.empty;
 import static org.hamcrest.Matchers.equalTo;
@@ -111,7 +112,7 @@ public void testDanglingIndicesCanBeImported() throws Exception {
         importRequest.addParameter("accept_data_loss", "true");
         // Ensure this parameter is accepted
         importRequest.addParameter("timeout", "20s");
-        importRequest.addParameter("master_timeout", "20s");
+        importRequest.addParameter(REST_MASTER_TIMEOUT_PARAM, "20s");
         final Response importResponse = restClient.performRequest(importRequest);
         assertThat(importResponse.getStatusLine().getStatusCode(), equalTo(ACCEPTED.getStatus()));
 
@@ -147,7 +148,7 @@ public void testDanglingIndicesCanBeDeleted() throws Exception {
         deleteRequest.addParameter("accept_data_loss", "true");
         // Ensure these parameters is accepted
         deleteRequest.addParameter("timeout", "20s");
-        deleteRequest.addParameter("master_timeout", "20s");
+        deleteRequest.addParameter(REST_MASTER_TIMEOUT_PARAM, "20s");
         final Response deleteResponse = restClient.performRequest(deleteRequest);
         assertThat(deleteResponse.getStatusLine().getStatusCode(), equalTo(ACCEPTED.getStatus()));
 

server/src/internalClusterTest/java/org/elasticsearch/rest/action/admin/cluster/RestClusterStateActionIT.java

@@ -13,6 +13,8 @@
 
 import java.io.IOException;
 
+import static org.elasticsearch.rest.RestUtils.REST_MASTER_TIMEOUT_PARAM;
+
 public class RestClusterStateActionIT extends ESIntegTestCase {
 
     @Override
@@ -22,7 +24,7 @@ protected boolean addMockHttpTransport() {
 
     public void testInfiniteTimeOut() throws IOException {
         final var request = new Request("GET", "/_cluster/state/none");
-        request.addParameter("master_timeout", "-1");
+        request.addParameter(REST_MASTER_TIMEOUT_PARAM, "-1");
         getRestClient().performRequest(request);
     }
 }

server/src/main/java/org/elasticsearch/rest/RestUtils.java

@@ -10,6 +10,7 @@
 
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.core.Booleans;
+import org.elasticsearch.core.TimeValue;
 
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
@@ -256,4 +257,27 @@ public static Optional<String> extractTraceId(String traceparent) {
         return traceparent != null && traceparent.length() >= 55 ? Optional.of(traceparent.substring(3, 35)) : Optional.empty();
     }
 
+    /**
+     * The name of the common {@code ?master_timeout} query parameter.
+     */
+    public static final String REST_MASTER_TIMEOUT_PARAM = "master_timeout";
+
+    /**
+     * The default value for the common {@code ?master_timeout} query parameter.
+     */
+    public static final TimeValue REST_MASTER_TIMEOUT_DEFAULT = TimeValue.timeValueSeconds(30);
+
+    /**
+     * Extract the {@code ?master_timeout} parameter from the request, imposing the common default of {@code 30s} in case the parameter is
+     * missing.
+     *
+     * @param restRequest The request from which to extract the {@code ?master_timeout} parameter
+     * @return the timeout from the request, with a default of {@link #REST_MASTER_TIMEOUT_DEFAULT} ({@code 30s}) if the request does not
+     *         specify the parameter
+     */
+    public static TimeValue getMasterNodeTimeout(RestRequest restRequest) {
+        assert restRequest != null;
+        return restRequest.paramAsTime(REST_MASTER_TIMEOUT_PARAM, REST_MASTER_TIMEOUT_DEFAULT);
+    }
+
 }

server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestAddVotingConfigExclusionAction.java

@@ -22,6 +22,7 @@
 import java.util.List;
 
 import static org.elasticsearch.rest.RestRequest.Method.POST;
+import static org.elasticsearch.rest.RestUtils.getMasterNodeTimeout;
 
 public class RestAddVotingConfigExclusionAction extends BaseRestHandler {
     private static final TimeValue DEFAULT_TIMEOUT = TimeValue.timeValueSeconds(30L);
@@ -82,7 +83,7 @@ static AddVotingConfigExclusionsRequest resolveVotingConfigExclusionsRequest(fin
             request.paramAsTime("timeout", DEFAULT_TIMEOUT)
         );
 
-        return resolvedRequest.masterNodeTimeout(request.paramAsTime("master_timeout", resolvedRequest.masterNodeTimeout()));
+        return resolvedRequest.masterNodeTimeout(getMasterNodeTimeout(request));
     }
 
 }

server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestCleanupRepositoryAction.java

@@ -20,6 +20,7 @@
 import java.util.List;
 
 import static org.elasticsearch.rest.RestRequest.Method.POST;
+import static org.elasticsearch.rest.RestUtils.getMasterNodeTimeout;
 
 /**
  * Cleans up a repository
@@ -42,7 +43,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
         String name = request.param("repository");
         CleanupRepositoryRequest cleanupRepositoryRequest = new CleanupRepositoryRequest(name);
         cleanupRepositoryRequest.ackTimeout(request.paramAsTime("timeout", cleanupRepositoryRequest.ackTimeout()));
-        cleanupRepositoryRequest.masterNodeTimeout(request.paramAsTime("master_timeout", cleanupRepositoryRequest.masterNodeTimeout()));
+        cleanupRepositoryRequest.masterNodeTimeout(getMasterNodeTimeout(request));
         return channel -> client.admin().cluster().cleanupRepository(cleanupRepositoryRequest, new RestToXContentListener<>(channel));
     }
 }

server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestClearVotingConfigExclusionsAction.java

@@ -19,6 +19,7 @@
 import java.util.List;
 
 import static org.elasticsearch.rest.RestRequest.Method.DELETE;
+import static org.elasticsearch.rest.RestUtils.getMasterNodeTimeout;
 
 public class RestClearVotingConfigExclusionsAction extends BaseRestHandler {
 
@@ -45,7 +46,7 @@ protected RestChannelConsumer prepareRequest(final RestRequest request, final No
 
     static ClearVotingConfigExclusionsRequest resolveVotingConfigExclusionsRequest(final RestRequest request) {
         final var resolvedRequest = new ClearVotingConfigExclusionsRequest();
-        resolvedRequest.masterNodeTimeout(request.paramAsTime("master_timeout", resolvedRequest.masterNodeTimeout()));
+        resolvedRequest.masterNodeTimeout(getMasterNodeTimeout(request));
         resolvedRequest.setTimeout(resolvedRequest.masterNodeTimeout());
         resolvedRequest.setWaitForRemoval(request.paramAsBoolean("wait_for_removal", resolvedRequest.getWaitForRemoval()));
         return resolvedRequest;

server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestCloneSnapshotAction.java

@@ -24,6 +24,7 @@
 import java.util.Map;
 
 import static org.elasticsearch.rest.RestRequest.Method.PUT;
+import static org.elasticsearch.rest.RestUtils.getMasterNodeTimeout;
 
 /**
  * Clones indices from one snapshot into another snapshot in the same repository
@@ -51,7 +52,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
                 request.param("target_snapshot"),
                 XContentMapValues.nodeStringArrayValue(source.getOrDefault("indices", Collections.emptyList()))
             );
-            cloneSnapshotRequest.masterNodeTimeout(request.paramAsTime("master_timeout", cloneSnapshotRequest.masterNodeTimeout()));
+            cloneSnapshotRequest.masterNodeTimeout(getMasterNodeTimeout(request));
             cloneSnapshotRequest.indicesOptions(IndicesOptions.fromMap(source, cloneSnapshotRequest.indicesOptions()));
             return channel -> client.admin().cluster().cloneSnapshot(cloneSnapshotRequest, new RestToXContentListener<>(channel));
         }

server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestClusterGetSettingsAction.java

@@ -29,6 +29,7 @@
 import java.util.function.Predicate;
 
 import static org.elasticsearch.rest.RestRequest.Method.GET;
+import static org.elasticsearch.rest.RestUtils.getMasterNodeTimeout;
 
 @ServerlessScope(Scope.INTERNAL)
 public class RestClusterGetSettingsAction extends BaseRestHandler {
@@ -64,7 +65,7 @@ public String getName() {
 
     private static void setUpRequestParams(MasterNodeReadRequest<?> clusterRequest, RestRequest request) {
         clusterRequest.local(request.paramAsBoolean("local", clusterRequest.local()));
-        clusterRequest.masterNodeTimeout(request.paramAsTime("master_timeout", clusterRequest.masterNodeTimeout()));
+        clusterRequest.masterNodeTimeout(getMasterNodeTimeout(request));
     }
 
     @Override

server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestClusterHealthAction.java

@@ -31,6 +31,7 @@
 import java.util.Set;
 
 import static org.elasticsearch.rest.RestRequest.Method.GET;
+import static org.elasticsearch.rest.RestUtils.getMasterNodeTimeout;
 
 @ServerlessScope(Scope.INTERNAL)
 public class RestClusterHealthAction extends BaseRestHandler {
@@ -63,7 +64,7 @@ public static ClusterHealthRequest fromRequest(final RestRequest request) {
         final ClusterHealthRequest clusterHealthRequest = new ClusterHealthRequest(indices);
         clusterHealthRequest.indicesOptions(IndicesOptions.fromRequest(request, clusterHealthRequest.indicesOptions()));
         clusterHealthRequest.local(request.paramAsBoolean("local", clusterHealthRequest.local()));
-        clusterHealthRequest.masterNodeTimeout(request.paramAsTime("master_timeout", clusterHealthRequest.masterNodeTimeout()));
+        clusterHealthRequest.masterNodeTimeout(getMasterNodeTimeout(request));
         clusterHealthRequest.timeout(request.paramAsTime("timeout", clusterHealthRequest.timeout()));
         String waitForStatus = request.param("wait_for_status");
         if (waitForStatus != null) {

server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestClusterRerouteAction.java

@@ -31,6 +31,7 @@
 
 import static org.elasticsearch.common.util.set.Sets.addToCopy;
 import static org.elasticsearch.rest.RestRequest.Method.POST;
+import static org.elasticsearch.rest.RestUtils.getMasterNodeTimeout;
 
 @ServerlessScope(Scope.INTERNAL)
 public class RestClusterRerouteAction extends BaseRestHandler {
@@ -98,7 +99,7 @@ public static ClusterRerouteRequest createRequest(RestRequest request) throws IO
         clusterRerouteRequest.explain(request.paramAsBoolean("explain", clusterRerouteRequest.explain()));
         clusterRerouteRequest.ackTimeout(request.paramAsTime("timeout", clusterRerouteRequest.ackTimeout()));
         clusterRerouteRequest.setRetryFailed(request.paramAsBoolean("retry_failed", clusterRerouteRequest.isRetryFailed()));
-        clusterRerouteRequest.masterNodeTimeout(request.paramAsTime("master_timeout", clusterRerouteRequest.masterNodeTimeout()));
+        clusterRerouteRequest.masterNodeTimeout(getMasterNodeTimeout(request));
         request.applyContentParser(parser -> PARSER.parse(parser, clusterRerouteRequest, null));
         return clusterRerouteRequest;
     }

server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestClusterStateAction.java

@@ -42,6 +42,7 @@
 import static java.util.Collections.singletonMap;
 import static org.elasticsearch.common.util.set.Sets.addToCopy;
 import static org.elasticsearch.rest.RestRequest.Method.GET;
+import static org.elasticsearch.rest.RestUtils.getMasterNodeTimeout;
 
 @ServerlessScope(Scope.INTERNAL)
 public class RestClusterStateAction extends BaseRestHandler {
@@ -81,7 +82,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
         final ClusterStateRequest clusterStateRequest = new ClusterStateRequest();
         clusterStateRequest.indicesOptions(IndicesOptions.fromRequest(request, clusterStateRequest.indicesOptions()));
         clusterStateRequest.local(request.paramAsBoolean("local", clusterStateRequest.local()));
-        clusterStateRequest.masterNodeTimeout(request.paramAsTime("master_timeout", clusterStateRequest.masterNodeTimeout()));
+        clusterStateRequest.masterNodeTimeout(getMasterNodeTimeout(request));
         if (request.hasParam("wait_for_metadata_version")) {
             clusterStateRequest.waitForMetadataVersion(request.paramAsLong("wait_for_metadata_version", 0));
         }

server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestClusterUpdateSettingsAction.java

@@ -24,6 +24,7 @@
 import java.util.Set;
 
 import static org.elasticsearch.rest.RestRequest.Method.PUT;
+import static org.elasticsearch.rest.RestUtils.getMasterNodeTimeout;
 
 @ServerlessScope(Scope.INTERNAL)
 public class RestClusterUpdateSettingsAction extends BaseRestHandler {
@@ -45,9 +46,7 @@ public String getName() {
     public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
         final ClusterUpdateSettingsRequest clusterUpdateSettingsRequest = new ClusterUpdateSettingsRequest();
         clusterUpdateSettingsRequest.ackTimeout(request.paramAsTime("timeout", clusterUpdateSettingsRequest.ackTimeout()));
-        clusterUpdateSettingsRequest.masterNodeTimeout(
-            request.paramAsTime("master_timeout", clusterUpdateSettingsRequest.masterNodeTimeout())
-        );
+        clusterUpdateSettingsRequest.masterNodeTimeout(getMasterNodeTimeout(request));
         Map<String, Object> source;
         try (XContentParser parser = request.contentParser()) {
             source = parser.map();

server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestCreateSnapshotAction.java

@@ -21,6 +21,7 @@
 
 import static org.elasticsearch.rest.RestRequest.Method.POST;
 import static org.elasticsearch.rest.RestRequest.Method.PUT;
+import static org.elasticsearch.rest.RestUtils.getMasterNodeTimeout;
 
 /**
  * Creates a new snapshot
@@ -44,7 +45,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
         String snapshot = request.param("snapshot");
         CreateSnapshotRequest createSnapshotRequest = new CreateSnapshotRequest(repository, snapshot);
         request.applyContentParser(p -> createSnapshotRequest.source(p.mapOrdered()));
-        createSnapshotRequest.masterNodeTimeout(request.paramAsTime("master_timeout", createSnapshotRequest.masterNodeTimeout()));
+        createSnapshotRequest.masterNodeTimeout(getMasterNodeTimeout(request));
         createSnapshotRequest.waitForCompletion(request.paramAsBoolean("wait_for_completion", false));
         return channel -> client.admin().cluster().createSnapshot(createSnapshotRequest, new RestToXContentListener<>(channel));
     }

server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestDeleteDesiredNodesAction.java

@@ -18,6 +18,8 @@
 import java.io.IOException;
 import java.util.List;
 
+import static org.elasticsearch.rest.RestUtils.getMasterNodeTimeout;
+
 public class RestDeleteDesiredNodesAction extends BaseRestHandler {
     @Override
     public String getName() {
@@ -32,7 +34,7 @@ public List<Route> routes() {
     @Override
     protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException {
         final AcknowledgedRequest.Plain deleteDesiredNodesRequest = new AcknowledgedRequest.Plain();
-        deleteDesiredNodesRequest.masterNodeTimeout(request.paramAsTime("master_timeout", deleteDesiredNodesRequest.masterNodeTimeout()));
+        deleteDesiredNodesRequest.masterNodeTimeout(getMasterNodeTimeout(request));
         return restChannel -> client.execute(
             TransportDeleteDesiredNodesAction.TYPE,
             deleteDesiredNodesRequest,

server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestDeleteRepositoryAction.java

@@ -23,6 +23,7 @@
 import java.util.List;
 
 import static org.elasticsearch.rest.RestRequest.Method.DELETE;
+import static org.elasticsearch.rest.RestUtils.getMasterNodeTimeout;
 
 /**
  * Unregisters a repository
@@ -45,7 +46,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
         String name = request.param("repository");
         DeleteRepositoryRequest deleteRepositoryRequest = new DeleteRepositoryRequest(name);
         deleteRepositoryRequest.ackTimeout(request.paramAsTime("timeout", deleteRepositoryRequest.ackTimeout()));
-        deleteRepositoryRequest.masterNodeTimeout(request.paramAsTime("master_timeout", deleteRepositoryRequest.masterNodeTimeout()));
+        deleteRepositoryRequest.masterNodeTimeout(getMasterNodeTimeout(request));
         return channel -> client.admin()
             .cluster()
             .deleteRepository(