Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove redundant field from GetDecommissionStateResponse #4751

Merged
merged 12 commits into from
Oct 18, 2022
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- Fix new race condition in DecommissionControllerTests ([4688](https://github.com/opensearch-project/OpenSearch/pull/4688))
- Fix SearchStats (de)serialization (caused by https://github.com/opensearch-project/OpenSearch/pull/4616) ([#4697](https://github.com/opensearch-project/OpenSearch/pull/4697))
- Fixing Gradle warnings associated with publishPluginZipPublicationToXxx tasks ([#4696](https://github.com/opensearch-project/OpenSearch/pull/4696))
- [BUG]: Remove redundant field from GetDecommissionStateResponse ([#4751](https://github.com/opensearch-project/OpenSearch/pull/4751))
- Fixed randomly failing test ([4774](https://github.com/opensearch-project/OpenSearch/pull/4774))
- Update version check after backport ([4786](https://github.com/opensearch-project/OpenSearch/pull/4786))
### Security
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,16 @@
"url": {
"paths": [
{
"path": "/_cluster/decommission/awareness/_status",
"methods": [
"path":"/_cluster/decommission/awareness/{awareness_attribute_name}/_status",
"methods":[
"GET"
]
],
"parts":{
"awareness_attribute_name":{
"type":"string",
"description":"Awareness attribute name"
}
}
}
]
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,31 +10,71 @@

import org.opensearch.action.ActionRequestValidationException;
import org.opensearch.action.support.clustermanager.ClusterManagerNodeReadRequest;
import org.opensearch.common.Strings;
import org.opensearch.common.io.stream.StreamInput;
import org.opensearch.common.io.stream.StreamOutput;

import java.io.IOException;

import static org.opensearch.action.ValidateActions.addValidationError;

/**
* Get Decommissioned attribute request
*
* @opensearch.internal
*/
public class GetDecommissionStateRequest extends ClusterManagerNodeReadRequest<GetDecommissionStateRequest> {

private String attributeName;

public GetDecommissionStateRequest() {}

/**
* Constructs a new get decommission state request with given attribute name
*
* @param attributeName name of the attribute
*/
public GetDecommissionStateRequest(String attributeName) {
this.attributeName = attributeName;
}

public GetDecommissionStateRequest(StreamInput in) throws IOException {
super(in);
attributeName = in.readString();
}

@Override
public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);
out.writeString(attributeName);
}

@Override
public ActionRequestValidationException validate() {
return null;
ActionRequestValidationException validationException = null;
if (attributeName == null || Strings.isEmpty(attributeName)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Strings.isEmpty

validationException = addValidationError("attribute name is missing", validationException);
}
return validationException;
}

/**
* Sets attribute name
*
* @param attributeName attribute name
* @return this request
*/
public GetDecommissionStateRequest attributeName(String attributeName) {
this.attributeName = attributeName;
return this;
}

/**
* Returns attribute name
*
* @return attributeName name of attribute
*/
public String attributeName() {
return this.attributeName;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,13 @@ public class GetDecommissionStateRequestBuilder extends ClusterManagerNodeReadOp
public GetDecommissionStateRequestBuilder(OpenSearchClient client, GetDecommissionStateAction action) {
super(client, action, new GetDecommissionStateRequest());
}

/**
* @param attributeName name of attribute
* @return current object
*/
public GetDecommissionStateRequestBuilder setAttributeName(String attributeName) {
request.attributeName(attributeName);
return this;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

import org.opensearch.OpenSearchParseException;
import org.opensearch.action.ActionResponse;
import org.opensearch.cluster.decommission.DecommissionAttribute;
import org.opensearch.cluster.decommission.DecommissionStatus;
import org.opensearch.common.io.stream.StreamInput;
import org.opensearch.common.io.stream.StreamOutput;
Expand All @@ -31,49 +30,40 @@
*/
public class GetDecommissionStateResponse extends ActionResponse implements ToXContentObject {

private DecommissionAttribute decommissionedAttribute;
private String attributeValue;
private DecommissionStatus status;

GetDecommissionStateResponse() {
this(null, null);
}

GetDecommissionStateResponse(DecommissionAttribute decommissionedAttribute, DecommissionStatus status) {
this.decommissionedAttribute = decommissionedAttribute;
GetDecommissionStateResponse(String attributeValue, DecommissionStatus status) {
this.attributeValue = attributeValue;
this.status = status;
}

GetDecommissionStateResponse(StreamInput in) throws IOException {
// read decommissioned attribute and status only if it is present
if (in.readBoolean()) {
this.decommissionedAttribute = new DecommissionAttribute(in);
}
if (in.readBoolean()) {
this.attributeValue = in.readString();
this.status = DecommissionStatus.fromString(in.readString());
}
}

@Override
public void writeTo(StreamOutput out) throws IOException {
// if decommissioned attribute is null, mark absence of decommissioned attribute
if (decommissionedAttribute == null) {
out.writeBoolean(false);
} else {
out.writeBoolean(true);
decommissionedAttribute.writeTo(out);
}

// if status is null, mark absence of status
if (status == null) {
// if decommissioned attribute value is null or status is null then mark its absence
if (attributeValue == null || status == null) {
out.writeBoolean(false);
} else {
out.writeBoolean(true);
out.writeString(attributeValue);
out.writeString(status.status());
}
}

public DecommissionAttribute getDecommissionedAttribute() {
return decommissionedAttribute;
public String getAttributeValue() {
return attributeValue;
}

public DecommissionStatus getDecommissionStatus() {
Expand All @@ -83,84 +73,49 @@ public DecommissionStatus getDecommissionStatus() {
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();
builder.startObject("awareness");
if (decommissionedAttribute != null) {
builder.field(decommissionedAttribute.attributeName(), decommissionedAttribute.attributeValue());
}
builder.endObject();
if (status != null) {
builder.field("status", status);
if (attributeValue != null && status != null) {
builder.field(attributeValue, status);
}
builder.endObject();
return builder;
}

public static GetDecommissionStateResponse fromXContent(XContentParser parser) throws IOException {
ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser);
String attributeType = "awareness";
XContentParser.Token token;
DecommissionAttribute decommissionAttribute = null;
String attributeValue = null;
DecommissionStatus status = null;
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
if (token == XContentParser.Token.FIELD_NAME) {
String currentFieldName = parser.currentName();
if (attributeType.equals(currentFieldName)) {
if (parser.nextToken() != XContentParser.Token.START_OBJECT) {
throw new OpenSearchParseException(
"failed to parse decommission attribute type [{}], expected object",
attributeType
);
}
token = parser.nextToken();
if (token != XContentParser.Token.END_OBJECT) {
if (token == XContentParser.Token.FIELD_NAME) {
String fieldName = parser.currentName();
String value;
token = parser.nextToken();
if (token == XContentParser.Token.VALUE_STRING) {
value = parser.text();
} else {
throw new OpenSearchParseException(
"failed to parse attribute [{}], expected string for attribute value",
fieldName
);
}
decommissionAttribute = new DecommissionAttribute(fieldName, value);
parser.nextToken();
} else {
throw new OpenSearchParseException("failed to parse attribute type [{}], unexpected type", attributeType);
}
} else {
throw new OpenSearchParseException("failed to parse attribute type [{}]", attributeType);
}
} else if ("status".equals(currentFieldName)) {
if (parser.nextToken() != XContentParser.Token.VALUE_STRING) {
throw new OpenSearchParseException(
"failed to parse status of decommissioning, expected string but found unknown type"
);
}
status = DecommissionStatus.fromString(parser.text().toLowerCase(Locale.ROOT));
} else {
throw new OpenSearchParseException(
"unknown field found [{}], failed to parse the decommission attribute",
currentFieldName
);
attributeValue = parser.currentName();
if (parser.nextToken() != XContentParser.Token.VALUE_STRING) {
throw new OpenSearchParseException("failed to parse status of decommissioning, expected string but found unknown type");
}
status = DecommissionStatus.fromString(parser.text().toLowerCase(Locale.ROOT));
} else {
throw new OpenSearchParseException(
"failed to parse decommission state, expected [{}] but found [{}]",
XContentParser.Token.FIELD_NAME,
token
);
}
}
return new GetDecommissionStateResponse(decommissionAttribute, status);
return new GetDecommissionStateResponse(attributeValue, status);
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
GetDecommissionStateResponse that = (GetDecommissionStateResponse) o;
return decommissionedAttribute.equals(that.decommissionedAttribute) && status == that.status;
if (!Objects.equals(attributeValue, that.attributeValue)) {
return false;
}
return Objects.equals(status, that.status);
}

@Override
public int hashCode() {
return Objects.hash(decommissionedAttribute, status);
return Objects.hash(attributeValue, status);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,11 @@ protected void clusterManagerOperation(
ActionListener<GetDecommissionStateResponse> listener
) throws Exception {
DecommissionAttributeMetadata decommissionAttributeMetadata = state.metadata().decommissionAttributeMetadata();
if (decommissionAttributeMetadata != null) {
if (decommissionAttributeMetadata != null
&& request.attributeName().equals(decommissionAttributeMetadata.decommissionAttribute().attributeName())) {
listener.onResponse(
new GetDecommissionStateResponse(
decommissionAttributeMetadata.decommissionAttribute(),
decommissionAttributeMetadata.decommissionAttribute().attributeValue(),
decommissionAttributeMetadata.status()
)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -873,17 +873,17 @@ public interface ClusterAdminClient extends OpenSearchClient {
/**
* Get Decommissioned attribute
*/
ActionFuture<GetDecommissionStateResponse> getDecommission(GetDecommissionStateRequest request);
ActionFuture<GetDecommissionStateResponse> getDecommissionState(GetDecommissionStateRequest request);

/**
* Get Decommissioned attribute
*/
void getDecommission(GetDecommissionStateRequest request, ActionListener<GetDecommissionStateResponse> listener);
void getDecommissionState(GetDecommissionStateRequest request, ActionListener<GetDecommissionStateResponse> listener);

/**
* Get Decommissioned attribute
*/
GetDecommissionStateRequestBuilder prepareGetDecommission();
GetDecommissionStateRequestBuilder prepareGetDecommissionState();

/**
* Deletes the decommission metadata.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1417,17 +1417,17 @@ public DecommissionRequestBuilder prepareDecommission(DecommissionRequest reques
}

@Override
public ActionFuture<GetDecommissionStateResponse> getDecommission(GetDecommissionStateRequest request) {
public ActionFuture<GetDecommissionStateResponse> getDecommissionState(GetDecommissionStateRequest request) {
return execute(GetDecommissionStateAction.INSTANCE, request);
}

@Override
public void getDecommission(GetDecommissionStateRequest request, ActionListener<GetDecommissionStateResponse> listener) {
public void getDecommissionState(GetDecommissionStateRequest request, ActionListener<GetDecommissionStateResponse> listener) {
execute(GetDecommissionStateAction.INSTANCE, request, listener);
}

@Override
public GetDecommissionStateRequestBuilder prepareGetDecommission() {
public GetDecommissionStateRequestBuilder prepareGetDecommissionState() {
return new GetDecommissionStateRequestBuilder(this, GetDecommissionStateAction.INSTANCE);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public class RestGetDecommissionStateAction extends BaseRestHandler {

@Override
public List<Route> routes() {
return singletonList(new Route(GET, "/_cluster/decommission/awareness/_status"));
return singletonList(new Route(GET, "/_cluster/decommission/awareness/{awareness_attribute_name}/_status"));
}

@Override
Expand All @@ -41,6 +41,8 @@ public String getName() {
@Override
public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
GetDecommissionStateRequest getDecommissionStateRequest = Requests.getDecommissionStateRequest();
return channel -> client.admin().cluster().getDecommission(getDecommissionStateRequest, new RestToXContentListener<>(channel));
String attributeName = request.param("awareness_attribute_name");
getDecommissionStateRequest.attributeName(attributeName);
return channel -> client.admin().cluster().getDecommissionState(getDecommissionStateRequest, new RestToXContentListener<>(channel));
}
}
Loading