Refactor HLRC RequestConverters parameters to be more explicit (#42128)

The existing `RequestConverters.Params` is confusing, because it wraps
an underlying request object and mutations of the `Params` object
actually mutate the `Request` that was used in the construction of the
`Params`.

This leads to a situation where we create a `RequestConverter.Params`
object, mutate it, and then it appears nothing happens to it - it
appears to be unused. What happens behind the scenes is that the Request
object is mutated when methods on `Params` are invoked. This results in
unclear, confusing code where mutating one object changes another with
no obvious connection.

This commit refactors `RequestConverters.Params` to be a simple helper
class to produce a `Map` which must be passed explicitly to a Request
object. This makes it apparent that the `Params` are actually used, and
that they have an effect on the `request` object explicit and easier to
understand.

Co-authored-by: Ojas Gulati <[email protected]>

Author: ojasgulati

client/rest-high-level/src/main/java/org/elasticsearch/client/CcrRequestConverters.java

@@ -48,9 +48,10 @@ static Request putFollow(PutFollowRequest putFollowRequest) throws IOException {
             .addPathPartAsIs("_ccr", "follow")
             .build();
         Request request = new Request(HttpPut.METHOD_NAME, endpoint);
-        RequestConverters.Params parameters = new RequestConverters.Params(request);
+        RequestConverters.Params parameters = new RequestConverters.Params();
         parameters.withWaitForActiveShards(putFollowRequest.waitForActiveShards());
         request.setEntity(createEntity(putFollowRequest, REQUEST_BODY_CONTENT_TYPE));
+        request.addParameters(parameters.asMap());
         return request;
     }
 

client/rest-high-level/src/main/java/org/elasticsearch/client/ClusterRequestConverters.java

@@ -36,22 +36,21 @@ private ClusterRequestConverters() {}
     static Request clusterPutSettings(ClusterUpdateSettingsRequest clusterUpdateSettingsRequest) throws IOException {
         Request request = new Request(HttpPut.METHOD_NAME, "/_cluster/settings");
 
-        RequestConverters.Params parameters = new RequestConverters.Params(request);
+        RequestConverters.Params parameters = new RequestConverters.Params();
         parameters.withTimeout(clusterUpdateSettingsRequest.timeout());
         parameters.withMasterTimeout(clusterUpdateSettingsRequest.masterNodeTimeout());
-
+        request.addParameters(parameters.asMap());
         request.setEntity(RequestConverters.createEntity(clusterUpdateSettingsRequest, RequestConverters.REQUEST_BODY_CONTENT_TYPE));
         return request;
     }
 
     static Request clusterGetSettings(ClusterGetSettingsRequest clusterGetSettingsRequest) throws IOException {
         Request request = new Request(HttpGet.METHOD_NAME, "/_cluster/settings");
-
-        RequestConverters.Params parameters = new RequestConverters.Params(request);
+        RequestConverters.Params parameters = new RequestConverters.Params();
         parameters.withLocal(clusterGetSettingsRequest.local());
         parameters.withIncludeDefaults(clusterGetSettingsRequest.includeDefaults());
         parameters.withMasterTimeout(clusterGetSettingsRequest.masterNodeTimeout());
-
+        request.addParameters(parameters.asMap());
         return request;
     }
 
@@ -63,7 +62,7 @@ static Request clusterHealth(ClusterHealthRequest healthRequest) {
             .build();
         Request request = new Request(HttpGet.METHOD_NAME, endpoint);
 
-        new RequestConverters.Params(request)
+        RequestConverters.Params params = new RequestConverters.Params()
             .withWaitForStatus(healthRequest.waitForStatus())
             .withWaitForNoRelocatingShards(healthRequest.waitForNoRelocatingShards())
             .withWaitForNoInitializingShards(healthRequest.waitForNoInitializingShards())
@@ -74,6 +73,7 @@ static Request clusterHealth(ClusterHealthRequest healthRequest) {
             .withMasterTimeout(healthRequest.masterNodeTimeout())
             .withLocal(healthRequest.local())
             .withLevel(healthRequest.level());
+        request.addParameters(params.asMap());
         return request;
     }
 }

client/rest-high-level/src/main/java/org/elasticsearch/client/DataFrameRequestConverters.java

@@ -82,10 +82,11 @@ static Request startDataFrameTransform(StartDataFrameTransformRequest startReque
                 .addPathPartAsIs("_start")
                 .build();
         Request request = new Request(HttpPost.METHOD_NAME, endpoint);
-        RequestConverters.Params params = new RequestConverters.Params(request);
+        RequestConverters.Params params = new RequestConverters.Params();
         if (startRequest.getTimeout() != null) {
             params.withTimeout(startRequest.getTimeout());
         }
+        request.addParameters(params.asMap());
         return request;
     }
 
@@ -96,13 +97,14 @@ static Request stopDataFrameTransform(StopDataFrameTransformRequest stopRequest)
                     .addPathPartAsIs("_stop")
                     .build();
             Request request = new Request(HttpPost.METHOD_NAME, endpoint);
-            RequestConverters.Params params = new RequestConverters.Params(request);
+            RequestConverters.Params params = new RequestConverters.Params();
             if (stopRequest.getWaitForCompletion() != null) {
                 params.withWaitForCompletion(stopRequest.getWaitForCompletion());
             }
             if (stopRequest.getTimeout() != null) {
                 params.withTimeout(stopRequest.getTimeout());
             }
+            request.addParameters(params.asMap());
             return request;
     }
 

client/rest-high-level/src/main/java/org/elasticsearch/client/IndexLifecycleRequestConverters.java

@@ -44,9 +44,10 @@ static Request getLifecyclePolicy(GetLifecyclePolicyRequest getLifecyclePolicyRe
         String endpoint = new RequestConverters.EndpointBuilder().addPathPartAsIs("_ilm/policy")
                 .addCommaSeparatedPathParts(getLifecyclePolicyRequest.getPolicyNames()).build();
         Request request = new Request(HttpGet.METHOD_NAME, endpoint);
-        RequestConverters.Params params = new RequestConverters.Params(request);
+        RequestConverters.Params params = new RequestConverters.Params();
         params.withMasterTimeout(getLifecyclePolicyRequest.masterNodeTimeout());
         params.withTimeout(getLifecyclePolicyRequest.timeout());
+        request.addParameters(params.asMap());
         return request;
     }
 
@@ -56,9 +57,10 @@ static Request putLifecyclePolicy(PutLifecyclePolicyRequest putLifecycleRequest)
             .addPathPartAsIs(putLifecycleRequest.getName())
             .build();
         Request request = new Request(HttpPut.METHOD_NAME, endpoint);
-        RequestConverters.Params params = new RequestConverters.Params(request);
+        RequestConverters.Params params = new RequestConverters.Params();
         params.withMasterTimeout(putLifecycleRequest.masterNodeTimeout());
         params.withTimeout(putLifecycleRequest.timeout());
+        request.addParameters(params.asMap());
         request.setEntity(RequestConverters.createEntity(putLifecycleRequest, RequestConverters.REQUEST_BODY_CONTENT_TYPE));
         return request;
     }
@@ -69,9 +71,10 @@ static Request deleteLifecyclePolicy(DeleteLifecyclePolicyRequest deleteLifecycl
                 .addPathPartAsIs("_ilm/policy")
                 .addPathPartAsIs(deleteLifecyclePolicyRequest.getLifecyclePolicy())
                 .build());
-        RequestConverters.Params params = new RequestConverters.Params(request);
+        RequestConverters.Params params = new RequestConverters.Params();
         params.withMasterTimeout(deleteLifecyclePolicyRequest.masterNodeTimeout());
         params.withTimeout(deleteLifecyclePolicyRequest.timeout());
+        request.addParameters(params.asMap());
         return request;
     }
 
@@ -83,9 +86,10 @@ static Request removeIndexLifecyclePolicy(RemoveIndexLifecyclePolicyRequest remo
                         .addCommaSeparatedPathParts(indices)
                         .addPathPartAsIs("_ilm", "remove")
                         .build());
-        RequestConverters.Params params = new RequestConverters.Params(request);
+        RequestConverters.Params params = new RequestConverters.Params();
         params.withIndicesOptions(removePolicyRequest.indicesOptions());
         params.withMasterTimeout(removePolicyRequest.masterNodeTimeout());
+        request.addParameters(params.asMap());
         return request;
     }
 
@@ -95,9 +99,10 @@ static Request startILM(StartILMRequest startILMRequest) {
                 .addPathPartAsIs("_ilm")
                 .addPathPartAsIs("start")
             .build());
-        RequestConverters.Params params = new RequestConverters.Params(request);
+        RequestConverters.Params params = new RequestConverters.Params();
         params.withMasterTimeout(startILMRequest.masterNodeTimeout());
         params.withTimeout(startILMRequest.timeout());
+        request.addParameters(params.asMap());
         return request;
     }
 
@@ -107,9 +112,10 @@ static Request stopILM(StopILMRequest stopILMRequest) {
                 .addPathPartAsIs("_ilm")
                 .addPathPartAsIs("stop")
             .build());
-        RequestConverters.Params params = new RequestConverters.Params(request);
+        RequestConverters.Params params = new RequestConverters.Params();
         params.withMasterTimeout(stopILMRequest.masterNodeTimeout());
         params.withTimeout(stopILMRequest.timeout());
+        request.addParameters(params.asMap());
         return request;
     }
 
@@ -119,9 +125,10 @@ static Request lifecycleManagementStatus(LifecycleManagementStatusRequest lifecy
                 .addPathPartAsIs("_ilm")
                 .addPathPartAsIs("status")
             .build());
-        RequestConverters.Params params = new RequestConverters.Params(request);
+        RequestConverters.Params params = new RequestConverters.Params();
         params.withMasterTimeout(lifecycleManagementStatusRequest.masterNodeTimeout());
         params.withTimeout(lifecycleManagementStatusRequest.timeout());
+        request.addParameters(params.asMap());
         return request;
     }
 
@@ -132,9 +139,10 @@ static Request explainLifecycle(ExplainLifecycleRequest explainLifecycleRequest)
                 .addPathPartAsIs("_ilm")
                 .addPathPartAsIs("explain")
             .build());
-        RequestConverters.Params params = new RequestConverters.Params(request);
+        RequestConverters.Params params = new RequestConverters.Params();
         params.withIndicesOptions(explainLifecycleRequest.indicesOptions());
         params.withMasterTimeout(explainLifecycleRequest.masterNodeTimeout());
+        request.addParameters(params.asMap());
         return request;
     }
 
@@ -145,9 +153,10 @@ static Request retryLifecycle(RetryLifecyclePolicyRequest retryLifecyclePolicyRe
                 .addPathPartAsIs("_ilm")
                 .addPathPartAsIs("retry")
                 .build());
-        RequestConverters.Params params = new RequestConverters.Params(request);
+        RequestConverters.Params params = new RequestConverters.Params();
         params.withMasterTimeout(retryLifecyclePolicyRequest.masterNodeTimeout());
         params.withTimeout(retryLifecyclePolicyRequest.timeout());
+        request.addParameters(params.asMap());
         return request;
     }
 }