Remove redundant field from GetDecommissionStateResponse (opensearch-…

…project#4751) * Add attribute name to query param and simplify GetDecommissionStateResponse Signed-off-by: Rishab Nahata <rnnahata@amazon.com>
imRishN · Oct 28, 2022 · cb20899 · cb20899
1 parent 91123ba
commit cb20899
Show file tree

Hide file tree

Showing 11 changed files with 158 additions and 93 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -79,6 +79,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - [Bug]: Fixed invalid location of JDK dependency for arm64 architecture([#4613](https://github.com/opensearch-project/OpenSearch/pull/4613))
 - [Bug]: Alias filter lost after rollover ([#4499](https://github.com/opensearch-project/OpenSearch/pull/4499))
 - 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))
 - Fix recovery path for searchable snapshots ([4813](https://github.com/opensearch-project/OpenSearch/pull/4813))
 - Fix a bug on handling an invalid array value for point type field #4900([#4900](https://github.com/opensearch-project/OpenSearch/pull/4900))

diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/cluster.get_decommission_awareness.json b/rest-api-spec/src/main/resources/rest-api-spec/api/cluster.get_decommission_awareness.json
@@ -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"
+            }
+          }
         }
       ]
     }

diff --git a/...ensearch/action/admin/cluster/decommission/awareness/get/GetDecommissionStateRequest.java b/...ensearch/action/admin/cluster/decommission/awareness/get/GetDecommissionStateRequest.java
@@ -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)) {
+            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;
     }
 }
diff --git a/...h/action/admin/cluster/decommission/awareness/get/GetDecommissionStateRequestBuilder.java b/...h/action/admin/cluster/decommission/awareness/get/GetDecommissionStateRequestBuilder.java
@@ -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;
+    }
 }
diff --git a/...nsearch/action/admin/cluster/decommission/awareness/get/GetDecommissionStateResponse.java b/...nsearch/action/admin/cluster/decommission/awareness/get/GetDecommissionStateResponse.java
@@ -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;
@@ -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() {
@@ -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);
     }
 }
diff --git a/.../action/admin/cluster/decommission/awareness/get/TransportGetDecommissionStateAction.java b/.../action/admin/cluster/decommission/awareness/get/TransportGetDecommissionStateAction.java
@@ -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()
                 )
             );

diff --git a/server/src/main/java/org/opensearch/client/ClusterAdminClient.java b/server/src/main/java/org/opensearch/client/ClusterAdminClient.java
@@ -819,17 +819,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.

diff --git a/server/src/main/java/org/opensearch/client/support/AbstractClient.java b/server/src/main/java/org/opensearch/client/support/AbstractClient.java
@@ -1351,17 +1351,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);
         }
 

diff --git a/...rc/main/java/org/opensearch/rest/action/admin/cluster/RestGetDecommissionStateAction.java b/...rc/main/java/org/opensearch/rest/action/admin/cluster/RestGetDecommissionStateAction.java
@@ -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
@@ -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));
     }
 }