Skip to content

Commit e0dbf6e

Browse files
gwbrownojasgulati
gwbrown
and
ojasgulati
authored
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]>
1 parent 5e02dc6 commit e0dbf6e

16 files changed

+259
-158
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,10 @@ static Request putFollow(PutFollowRequest putFollowRequest) throws IOException {
4848
.addPathPartAsIs("_ccr", "follow")
4949
.build();
5050
Request request = new Request(HttpPut.METHOD_NAME, endpoint);
51-
RequestConverters.Params parameters = new RequestConverters.Params(request);
51+
RequestConverters.Params parameters = new RequestConverters.Params();
5252
parameters.withWaitForActiveShards(putFollowRequest.waitForActiveShards());
5353
request.setEntity(createEntity(putFollowRequest, REQUEST_BODY_CONTENT_TYPE));
54+
request.addParameters(parameters.asMap());
5455
return request;
5556
}
5657

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,22 +36,21 @@ private ClusterRequestConverters() {}
3636
static Request clusterPutSettings(ClusterUpdateSettingsRequest clusterUpdateSettingsRequest) throws IOException {
3737
Request request = new Request(HttpPut.METHOD_NAME, "/_cluster/settings");
3838

39-
RequestConverters.Params parameters = new RequestConverters.Params(request);
39+
RequestConverters.Params parameters = new RequestConverters.Params();
4040
parameters.withTimeout(clusterUpdateSettingsRequest.timeout());
4141
parameters.withMasterTimeout(clusterUpdateSettingsRequest.masterNodeTimeout());
42-
42+
request.addParameters(parameters.asMap());
4343
request.setEntity(RequestConverters.createEntity(clusterUpdateSettingsRequest, RequestConverters.REQUEST_BODY_CONTENT_TYPE));
4444
return request;
4545
}
4646

4747
static Request clusterGetSettings(ClusterGetSettingsRequest clusterGetSettingsRequest) throws IOException {
4848
Request request = new Request(HttpGet.METHOD_NAME, "/_cluster/settings");
49-
50-
RequestConverters.Params parameters = new RequestConverters.Params(request);
49+
RequestConverters.Params parameters = new RequestConverters.Params();
5150
parameters.withLocal(clusterGetSettingsRequest.local());
5251
parameters.withIncludeDefaults(clusterGetSettingsRequest.includeDefaults());
5352
parameters.withMasterTimeout(clusterGetSettingsRequest.masterNodeTimeout());
54-
53+
request.addParameters(parameters.asMap());
5554
return request;
5655
}
5756

@@ -63,7 +62,7 @@ static Request clusterHealth(ClusterHealthRequest healthRequest) {
6362
.build();
6463
Request request = new Request(HttpGet.METHOD_NAME, endpoint);
6564

66-
new RequestConverters.Params(request)
65+
RequestConverters.Params params = new RequestConverters.Params()
6766
.withWaitForStatus(healthRequest.waitForStatus())
6867
.withWaitForNoRelocatingShards(healthRequest.waitForNoRelocatingShards())
6968
.withWaitForNoInitializingShards(healthRequest.waitForNoInitializingShards())
@@ -74,6 +73,7 @@ static Request clusterHealth(ClusterHealthRequest healthRequest) {
7473
.withMasterTimeout(healthRequest.masterNodeTimeout())
7574
.withLocal(healthRequest.local())
7675
.withLevel(healthRequest.level());
76+
request.addParameters(params.asMap());
7777
return request;
7878
}
7979
}

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,10 +82,11 @@ static Request startDataFrameTransform(StartDataFrameTransformRequest startReque
8282
.addPathPartAsIs("_start")
8383
.build();
8484
Request request = new Request(HttpPost.METHOD_NAME, endpoint);
85-
RequestConverters.Params params = new RequestConverters.Params(request);
85+
RequestConverters.Params params = new RequestConverters.Params();
8686
if (startRequest.getTimeout() != null) {
8787
params.withTimeout(startRequest.getTimeout());
8888
}
89+
request.addParameters(params.asMap());
8990
return request;
9091
}
9192

@@ -96,13 +97,14 @@ static Request stopDataFrameTransform(StopDataFrameTransformRequest stopRequest)
9697
.addPathPartAsIs("_stop")
9798
.build();
9899
Request request = new Request(HttpPost.METHOD_NAME, endpoint);
99-
RequestConverters.Params params = new RequestConverters.Params(request);
100+
RequestConverters.Params params = new RequestConverters.Params();
100101
if (stopRequest.getWaitForCompletion() != null) {
101102
params.withWaitForCompletion(stopRequest.getWaitForCompletion());
102103
}
103104
if (stopRequest.getTimeout() != null) {
104105
params.withTimeout(stopRequest.getTimeout());
105106
}
107+
request.addParameters(params.asMap());
106108
return request;
107109
}
108110

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

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,10 @@ static Request getLifecyclePolicy(GetLifecyclePolicyRequest getLifecyclePolicyRe
4444
String endpoint = new RequestConverters.EndpointBuilder().addPathPartAsIs("_ilm/policy")
4545
.addCommaSeparatedPathParts(getLifecyclePolicyRequest.getPolicyNames()).build();
4646
Request request = new Request(HttpGet.METHOD_NAME, endpoint);
47-
RequestConverters.Params params = new RequestConverters.Params(request);
47+
RequestConverters.Params params = new RequestConverters.Params();
4848
params.withMasterTimeout(getLifecyclePolicyRequest.masterNodeTimeout());
4949
params.withTimeout(getLifecyclePolicyRequest.timeout());
50+
request.addParameters(params.asMap());
5051
return request;
5152
}
5253

@@ -56,9 +57,10 @@ static Request putLifecyclePolicy(PutLifecyclePolicyRequest putLifecycleRequest)
5657
.addPathPartAsIs(putLifecycleRequest.getName())
5758
.build();
5859
Request request = new Request(HttpPut.METHOD_NAME, endpoint);
59-
RequestConverters.Params params = new RequestConverters.Params(request);
60+
RequestConverters.Params params = new RequestConverters.Params();
6061
params.withMasterTimeout(putLifecycleRequest.masterNodeTimeout());
6162
params.withTimeout(putLifecycleRequest.timeout());
63+
request.addParameters(params.asMap());
6264
request.setEntity(RequestConverters.createEntity(putLifecycleRequest, RequestConverters.REQUEST_BODY_CONTENT_TYPE));
6365
return request;
6466
}
@@ -69,9 +71,10 @@ static Request deleteLifecyclePolicy(DeleteLifecyclePolicyRequest deleteLifecycl
6971
.addPathPartAsIs("_ilm/policy")
7072
.addPathPartAsIs(deleteLifecyclePolicyRequest.getLifecyclePolicy())
7173
.build());
72-
RequestConverters.Params params = new RequestConverters.Params(request);
74+
RequestConverters.Params params = new RequestConverters.Params();
7375
params.withMasterTimeout(deleteLifecyclePolicyRequest.masterNodeTimeout());
7476
params.withTimeout(deleteLifecyclePolicyRequest.timeout());
77+
request.addParameters(params.asMap());
7578
return request;
7679
}
7780

@@ -83,9 +86,10 @@ static Request removeIndexLifecyclePolicy(RemoveIndexLifecyclePolicyRequest remo
8386
.addCommaSeparatedPathParts(indices)
8487
.addPathPartAsIs("_ilm", "remove")
8588
.build());
86-
RequestConverters.Params params = new RequestConverters.Params(request);
89+
RequestConverters.Params params = new RequestConverters.Params();
8790
params.withIndicesOptions(removePolicyRequest.indicesOptions());
8891
params.withMasterTimeout(removePolicyRequest.masterNodeTimeout());
92+
request.addParameters(params.asMap());
8993
return request;
9094
}
9195

@@ -95,9 +99,10 @@ static Request startILM(StartILMRequest startILMRequest) {
9599
.addPathPartAsIs("_ilm")
96100
.addPathPartAsIs("start")
97101
.build());
98-
RequestConverters.Params params = new RequestConverters.Params(request);
102+
RequestConverters.Params params = new RequestConverters.Params();
99103
params.withMasterTimeout(startILMRequest.masterNodeTimeout());
100104
params.withTimeout(startILMRequest.timeout());
105+
request.addParameters(params.asMap());
101106
return request;
102107
}
103108

@@ -107,9 +112,10 @@ static Request stopILM(StopILMRequest stopILMRequest) {
107112
.addPathPartAsIs("_ilm")
108113
.addPathPartAsIs("stop")
109114
.build());
110-
RequestConverters.Params params = new RequestConverters.Params(request);
115+
RequestConverters.Params params = new RequestConverters.Params();
111116
params.withMasterTimeout(stopILMRequest.masterNodeTimeout());
112117
params.withTimeout(stopILMRequest.timeout());
118+
request.addParameters(params.asMap());
113119
return request;
114120
}
115121

@@ -119,9 +125,10 @@ static Request lifecycleManagementStatus(LifecycleManagementStatusRequest lifecy
119125
.addPathPartAsIs("_ilm")
120126
.addPathPartAsIs("status")
121127
.build());
122-
RequestConverters.Params params = new RequestConverters.Params(request);
128+
RequestConverters.Params params = new RequestConverters.Params();
123129
params.withMasterTimeout(lifecycleManagementStatusRequest.masterNodeTimeout());
124130
params.withTimeout(lifecycleManagementStatusRequest.timeout());
131+
request.addParameters(params.asMap());
125132
return request;
126133
}
127134

@@ -132,9 +139,10 @@ static Request explainLifecycle(ExplainLifecycleRequest explainLifecycleRequest)
132139
.addPathPartAsIs("_ilm")
133140
.addPathPartAsIs("explain")
134141
.build());
135-
RequestConverters.Params params = new RequestConverters.Params(request);
142+
RequestConverters.Params params = new RequestConverters.Params();
136143
params.withIndicesOptions(explainLifecycleRequest.indicesOptions());
137144
params.withMasterTimeout(explainLifecycleRequest.masterNodeTimeout());
145+
request.addParameters(params.asMap());
138146
return request;
139147
}
140148

@@ -145,9 +153,10 @@ static Request retryLifecycle(RetryLifecyclePolicyRequest retryLifecyclePolicyRe
145153
.addPathPartAsIs("_ilm")
146154
.addPathPartAsIs("retry")
147155
.build());
148-
RequestConverters.Params params = new RequestConverters.Params(request);
156+
RequestConverters.Params params = new RequestConverters.Params();
149157
params.withMasterTimeout(retryLifecyclePolicyRequest.masterNodeTimeout());
150158
params.withTimeout(retryLifecyclePolicyRequest.timeout());
159+
request.addParameters(params.asMap());
151160
return request;
152161
}
153162
}

0 commit comments

Comments
 (0)