Skip to content

Commit 507d6d4

Browse files
imRishNashking94
imRishN
authored and
ashking94
committed
Remove redundant field from GetDecommissionStateResponse (opensearch-project#4751)
* Add attribute name to query param and simplify GetDecommissionStateResponse Signed-off-by: Rishab Nahata <[email protected]>
1 parent bfdb3f7 commit 507d6d4

File tree

11 files changed

+158
-93
lines changed

11 files changed

+158
-93
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
136136
- Fix new race condition in DecommissionControllerTests ([4688](https://github.com/opensearch-project/OpenSearch/pull/4688))
137137
- Fix SearchStats (de)serialization (caused by https://github.com/opensearch-project/OpenSearch/pull/4616) ([#4697](https://github.com/opensearch-project/OpenSearch/pull/4697))
138138
- Fixing Gradle warnings associated with publishPluginZipPublicationToXxx tasks ([#4696](https://github.com/opensearch-project/OpenSearch/pull/4696))
139+
- [BUG]: Remove redundant field from GetDecommissionStateResponse ([#4751](https://github.com/opensearch-project/OpenSearch/pull/4751))
139140
- Fixed randomly failing test ([4774](https://github.com/opensearch-project/OpenSearch/pull/4774))
140141
- Update version check after backport ([4786](https://github.com/opensearch-project/OpenSearch/pull/4786))
141142
- Fix decommission status update to non leader nodes ([4800](https://github.com/opensearch-project/OpenSearch/pull/4800))

‎rest-api-spec/src/main/resources/rest-api-spec/api/cluster.get_decommission_awareness.json

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,16 @@
88
"url": {
99
"paths": [
1010
{
11-
"path": "/_cluster/decommission/awareness/_status",
12-
"methods": [
11+
"path":"/_cluster/decommission/awareness/{awareness_attribute_name}/_status",
12+
"methods":[
1313
"GET"
14-
]
14+
],
15+
"parts":{
16+
"awareness_attribute_name":{
17+
"type":"string",
18+
"description":"Awareness attribute name"
19+
}
20+
}
1521
}
1622
]
1723
}

‎server/src/main/java/org/opensearch/action/admin/cluster/decommission/awareness/get/GetDecommissionStateRequest.java

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,31 +10,71 @@
1010

1111
import org.opensearch.action.ActionRequestValidationException;
1212
import org.opensearch.action.support.clustermanager.ClusterManagerNodeReadRequest;
13+
import org.opensearch.common.Strings;
1314
import org.opensearch.common.io.stream.StreamInput;
1415
import org.opensearch.common.io.stream.StreamOutput;
1516

1617
import java.io.IOException;
1718

19+
import static org.opensearch.action.ValidateActions.addValidationError;
20+
1821
/**
1922
* Get Decommissioned attribute request
2023
*
2124
* @opensearch.internal
2225
*/
2326
public class GetDecommissionStateRequest extends ClusterManagerNodeReadRequest<GetDecommissionStateRequest> {
2427

28+
private String attributeName;
29+
2530
public GetDecommissionStateRequest() {}
2631

32+
/**
33+
* Constructs a new get decommission state request with given attribute name
34+
*
35+
* @param attributeName name of the attribute
36+
*/
37+
public GetDecommissionStateRequest(String attributeName) {
38+
this.attributeName = attributeName;
39+
}
40+
2741
public GetDecommissionStateRequest(StreamInput in) throws IOException {
2842
super(in);
43+
attributeName = in.readString();
2944
}
3045

3146
@Override
3247
public void writeTo(StreamOutput out) throws IOException {
3348
super.writeTo(out);
49+
out.writeString(attributeName);
3450
}
3551

3652
@Override
3753
public ActionRequestValidationException validate() {
38-
return null;
54+
ActionRequestValidationException validationException = null;
55+
if (attributeName == null || Strings.isEmpty(attributeName)) {
56+
validationException = addValidationError("attribute name is missing", validationException);
57+
}
58+
return validationException;
59+
}
60+
61+
/**
62+
* Sets attribute name
63+
*
64+
* @param attributeName attribute name
65+
* @return this request
66+
*/
67+
public GetDecommissionStateRequest attributeName(String attributeName) {
68+
this.attributeName = attributeName;
69+
return this;
70+
}
71+
72+
/**
73+
* Returns attribute name
74+
*
75+
* @return attributeName name of attribute
76+
*/
77+
public String attributeName() {
78+
return this.attributeName;
3979
}
4080
}

‎server/src/main/java/org/opensearch/action/admin/cluster/decommission/awareness/get/GetDecommissionStateRequestBuilder.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,4 +27,13 @@ public class GetDecommissionStateRequestBuilder extends ClusterManagerNodeReadOp
2727
public GetDecommissionStateRequestBuilder(OpenSearchClient client, GetDecommissionStateAction action) {
2828
super(client, action, new GetDecommissionStateRequest());
2929
}
30+
31+
/**
32+
* @param attributeName name of attribute
33+
* @return current object
34+
*/
35+
public GetDecommissionStateRequestBuilder setAttributeName(String attributeName) {
36+
request.attributeName(attributeName);
37+
return this;
38+
}
3039
}

‎server/src/main/java/org/opensearch/action/admin/cluster/decommission/awareness/get/GetDecommissionStateResponse.java

Lines changed: 28 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010

1111
import org.opensearch.OpenSearchParseException;
1212
import org.opensearch.action.ActionResponse;
13-
import org.opensearch.cluster.decommission.DecommissionAttribute;
1413
import org.opensearch.cluster.decommission.DecommissionStatus;
1514
import org.opensearch.common.io.stream.StreamInput;
1615
import org.opensearch.common.io.stream.StreamOutput;
@@ -31,49 +30,40 @@
3130
*/
3231
public class GetDecommissionStateResponse extends ActionResponse implements ToXContentObject {
3332

34-
private DecommissionAttribute decommissionedAttribute;
33+
private String attributeValue;
3534
private DecommissionStatus status;
3635

3736
GetDecommissionStateResponse() {
3837
this(null, null);
3938
}
4039

41-
GetDecommissionStateResponse(DecommissionAttribute decommissionedAttribute, DecommissionStatus status) {
42-
this.decommissionedAttribute = decommissionedAttribute;
40+
GetDecommissionStateResponse(String attributeValue, DecommissionStatus status) {
41+
this.attributeValue = attributeValue;
4342
this.status = status;
4443
}
4544

4645
GetDecommissionStateResponse(StreamInput in) throws IOException {
4746
// read decommissioned attribute and status only if it is present
4847
if (in.readBoolean()) {
49-
this.decommissionedAttribute = new DecommissionAttribute(in);
50-
}
51-
if (in.readBoolean()) {
48+
this.attributeValue = in.readString();
5249
this.status = DecommissionStatus.fromString(in.readString());
5350
}
5451
}
5552

5653
@Override
5754
public void writeTo(StreamOutput out) throws IOException {
58-
// if decommissioned attribute is null, mark absence of decommissioned attribute
59-
if (decommissionedAttribute == null) {
60-
out.writeBoolean(false);
61-
} else {
62-
out.writeBoolean(true);
63-
decommissionedAttribute.writeTo(out);
64-
}
65-
66-
// if status is null, mark absence of status
67-
if (status == null) {
55+
// if decommissioned attribute value is null or status is null then mark its absence
56+
if (attributeValue == null || status == null) {
6857
out.writeBoolean(false);
6958
} else {
7059
out.writeBoolean(true);
60+
out.writeString(attributeValue);
7161
out.writeString(status.status());
7262
}
7363
}
7464

75-
public DecommissionAttribute getDecommissionedAttribute() {
76-
return decommissionedAttribute;
65+
public String getAttributeValue() {
66+
return attributeValue;
7767
}
7868

7969
public DecommissionStatus getDecommissionStatus() {
@@ -83,84 +73,49 @@ public DecommissionStatus getDecommissionStatus() {
8373
@Override
8474
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
8575
builder.startObject();
86-
builder.startObject("awareness");
87-
if (decommissionedAttribute != null) {
88-
builder.field(decommissionedAttribute.attributeName(), decommissionedAttribute.attributeValue());
89-
}
90-
builder.endObject();
91-
if (status != null) {
92-
builder.field("status", status);
76+
if (attributeValue != null && status != null) {
77+
builder.field(attributeValue, status);
9378
}
9479
builder.endObject();
9580
return builder;
9681
}
9782

9883
public static GetDecommissionStateResponse fromXContent(XContentParser parser) throws IOException {
9984
ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser);
100-
String attributeType = "awareness";
10185
XContentParser.Token token;
102-
DecommissionAttribute decommissionAttribute = null;
86+
String attributeValue = null;
10387
DecommissionStatus status = null;
10488
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
10589
if (token == XContentParser.Token.FIELD_NAME) {
106-
String currentFieldName = parser.currentName();
107-
if (attributeType.equals(currentFieldName)) {
108-
if (parser.nextToken() != XContentParser.Token.START_OBJECT) {
109-
throw new OpenSearchParseException(
110-
"failed to parse decommission attribute type [{}], expected object",
111-
attributeType
112-
);
113-
}
114-
token = parser.nextToken();
115-
if (token != XContentParser.Token.END_OBJECT) {
116-
if (token == XContentParser.Token.FIELD_NAME) {
117-
String fieldName = parser.currentName();
118-
String value;
119-
token = parser.nextToken();
120-
if (token == XContentParser.Token.VALUE_STRING) {
121-
value = parser.text();
122-
} else {
123-
throw new OpenSearchParseException(
124-
"failed to parse attribute [{}], expected string for attribute value",
125-
fieldName
126-
);
127-
}
128-
decommissionAttribute = new DecommissionAttribute(fieldName, value);
129-
parser.nextToken();
130-
} else {
131-
throw new OpenSearchParseException("failed to parse attribute type [{}], unexpected type", attributeType);
132-
}
133-
} else {
134-
throw new OpenSearchParseException("failed to parse attribute type [{}]", attributeType);
135-
}
136-
} else if ("status".equals(currentFieldName)) {
137-
if (parser.nextToken() != XContentParser.Token.VALUE_STRING) {
138-
throw new OpenSearchParseException(
139-
"failed to parse status of decommissioning, expected string but found unknown type"
140-
);
141-
}
142-
status = DecommissionStatus.fromString(parser.text().toLowerCase(Locale.ROOT));
143-
} else {
144-
throw new OpenSearchParseException(
145-
"unknown field found [{}], failed to parse the decommission attribute",
146-
currentFieldName
147-
);
90+
attributeValue = parser.currentName();
91+
if (parser.nextToken() != XContentParser.Token.VALUE_STRING) {
92+
throw new OpenSearchParseException("failed to parse status of decommissioning, expected string but found unknown type");
14893
}
94+
status = DecommissionStatus.fromString(parser.text().toLowerCase(Locale.ROOT));
95+
} else {
96+
throw new OpenSearchParseException(
97+
"failed to parse decommission state, expected [{}] but found [{}]",
98+
XContentParser.Token.FIELD_NAME,
99+
token
100+
);
149101
}
150102
}
151-
return new GetDecommissionStateResponse(decommissionAttribute, status);
103+
return new GetDecommissionStateResponse(attributeValue, status);
152104
}
153105

154106
@Override
155107
public boolean equals(Object o) {
156108
if (this == o) return true;
157109
if (o == null || getClass() != o.getClass()) return false;
158110
GetDecommissionStateResponse that = (GetDecommissionStateResponse) o;
159-
return decommissionedAttribute.equals(that.decommissionedAttribute) && status == that.status;
111+
if (!Objects.equals(attributeValue, that.attributeValue)) {
112+
return false;
113+
}
114+
return Objects.equals(status, that.status);
160115
}
161116

162117
@Override
163118
public int hashCode() {
164-
return Objects.hash(decommissionedAttribute, status);
119+
return Objects.hash(attributeValue, status);
165120
}
166121
}

‎server/src/main/java/org/opensearch/action/admin/cluster/decommission/awareness/get/TransportGetDecommissionStateAction.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,10 +69,11 @@ protected void clusterManagerOperation(
6969
ActionListener<GetDecommissionStateResponse> listener
7070
) throws Exception {
7171
DecommissionAttributeMetadata decommissionAttributeMetadata = state.metadata().decommissionAttributeMetadata();
72-
if (decommissionAttributeMetadata != null) {
72+
if (decommissionAttributeMetadata != null
73+
&& request.attributeName().equals(decommissionAttributeMetadata.decommissionAttribute().attributeName())) {
7374
listener.onResponse(
7475
new GetDecommissionStateResponse(
75-
decommissionAttributeMetadata.decommissionAttribute(),
76+
decommissionAttributeMetadata.decommissionAttribute().attributeValue(),
7677
decommissionAttributeMetadata.status()
7778
)
7879
);

‎server/src/main/java/org/opensearch/client/ClusterAdminClient.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -873,17 +873,17 @@ public interface ClusterAdminClient extends OpenSearchClient {
873873
/**
874874
* Get Decommissioned attribute
875875
*/
876-
ActionFuture<GetDecommissionStateResponse> getDecommission(GetDecommissionStateRequest request);
876+
ActionFuture<GetDecommissionStateResponse> getDecommissionState(GetDecommissionStateRequest request);
877877

878878
/**
879879
* Get Decommissioned attribute
880880
*/
881-
void getDecommission(GetDecommissionStateRequest request, ActionListener<GetDecommissionStateResponse> listener);
881+
void getDecommissionState(GetDecommissionStateRequest request, ActionListener<GetDecommissionStateResponse> listener);
882882

883883
/**
884884
* Get Decommissioned attribute
885885
*/
886-
GetDecommissionStateRequestBuilder prepareGetDecommission();
886+
GetDecommissionStateRequestBuilder prepareGetDecommissionState();
887887

888888
/**
889889
* Deletes the decommission metadata.

‎server/src/main/java/org/opensearch/client/support/AbstractClient.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1417,17 +1417,17 @@ public DecommissionRequestBuilder prepareDecommission(DecommissionRequest reques
14171417
}
14181418

14191419
@Override
1420-
public ActionFuture<GetDecommissionStateResponse> getDecommission(GetDecommissionStateRequest request) {
1420+
public ActionFuture<GetDecommissionStateResponse> getDecommissionState(GetDecommissionStateRequest request) {
14211421
return execute(GetDecommissionStateAction.INSTANCE, request);
14221422
}
14231423

14241424
@Override
1425-
public void getDecommission(GetDecommissionStateRequest request, ActionListener<GetDecommissionStateResponse> listener) {
1425+
public void getDecommissionState(GetDecommissionStateRequest request, ActionListener<GetDecommissionStateResponse> listener) {
14261426
execute(GetDecommissionStateAction.INSTANCE, request, listener);
14271427
}
14281428

14291429
@Override
1430-
public GetDecommissionStateRequestBuilder prepareGetDecommission() {
1430+
public GetDecommissionStateRequestBuilder prepareGetDecommissionState() {
14311431
return new GetDecommissionStateRequestBuilder(this, GetDecommissionStateAction.INSTANCE);
14321432
}
14331433

‎server/src/main/java/org/opensearch/rest/action/admin/cluster/RestGetDecommissionStateAction.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public class RestGetDecommissionStateAction extends BaseRestHandler {
3030

3131
@Override
3232
public List<Route> routes() {
33-
return singletonList(new Route(GET, "/_cluster/decommission/awareness/_status"));
33+
return singletonList(new Route(GET, "/_cluster/decommission/awareness/{awareness_attribute_name}/_status"));
3434
}
3535

3636
@Override
@@ -41,6 +41,8 @@ public String getName() {
4141
@Override
4242
public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
4343
GetDecommissionStateRequest getDecommissionStateRequest = Requests.getDecommissionStateRequest();
44-
return channel -> client.admin().cluster().getDecommission(getDecommissionStateRequest, new RestToXContentListener<>(channel));
44+
String attributeName = request.param("awareness_attribute_name");
45+
getDecommissionStateRequest.attributeName(attributeName);
46+
return channel -> client.admin().cluster().getDecommissionState(getDecommissionStateRequest, new RestToXContentListener<>(channel));
4547
}
4648
}

0 commit comments

Comments
 (0)