HLRC: close job refactor (#33031)

* HLRC: close job refactor

* Changing refactor to make job_id a string

* Changing set entity methodology

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

@@ -78,11 +78,11 @@ final class MLRequestConverters {
                 .addPathPartAsIs("_open")
                 .build();
         Request request = new Request(HttpPost.METHOD_NAME, endpoint);
-        request.setJsonEntity(openJobRequest.toString());
+        request.setEntity(createEntity(openJobRequest, REQUEST_BODY_CONTENT_TYPE));
         return request;
     }
 
-    static Request closeJob(CloseJobRequest closeJobRequest) {
+    static Request closeJob(CloseJobRequest closeJobRequest) throws IOException {
         String endpoint = new EndpointBuilder()
             .addPathPartAsIs("_xpack")
             .addPathPartAsIs("ml")
@@ -91,18 +91,7 @@ final class MLRequestConverters {
             .addPathPartAsIs("_close")
             .build();
         Request request = new Request(HttpPost.METHOD_NAME, endpoint);
-
+        request.setEntity(createEntity(closeJobRequest, REQUEST_BODY_CONTENT_TYPE));
-        RequestConverters.Params params = new RequestConverters.Params(request);
-        if (closeJobRequest.isForce() != null) {
-            params.putParam("force", Boolean.toString(closeJobRequest.isForce()));
-        }
-        if (closeJobRequest.isAllowNoJobs() != null) {
-            params.putParam("allow_no_jobs", Boolean.toString(closeJobRequest.isAllowNoJobs()));
-        }
-        if (closeJobRequest.getTimeout() != null) {
-            params.putParam("timeout", closeJobRequest.getTimeout().getStringRep());
-        }
-
         return request;
     }
 

client/rest-high-level/src/test/java/org/elasticsearch/client/MLRequestConvertersTests.java

@@ -81,21 +81,17 @@ public class MLRequestConvertersTests extends ESTestCase {
         Request request = MLRequestConverters.openJob(openJobRequest);
         assertEquals(HttpPost.METHOD_NAME, request.getMethod());
         assertEquals("/_xpack/ml/anomaly_detectors/" + jobId + "/_open", request.getEndpoint());
-        ByteArrayOutputStream bos = new ByteArrayOutputStream();
+        assertEquals(requestEntityToString(request), "{\"job_id\":\""+ jobId +"\",\"timeout\":\"10m\"}");
-        request.getEntity().writeTo(bos);
-        assertEquals(bos.toString("UTF-8"), "{\"job_id\":\""+ jobId +"\",\"timeout\":\"10m\"}");
     }
 
-    public void testCloseJob() {
+    public void testCloseJob() throws Exception {
         String jobId = "somejobid";
         CloseJobRequest closeJobRequest = new CloseJobRequest(jobId);
 
         Request request = MLRequestConverters.closeJob(closeJobRequest);
         assertEquals(HttpPost.METHOD_NAME, request.getMethod());
         assertEquals("/_xpack/ml/anomaly_detectors/" + jobId + "/_close", request.getEndpoint());
-        assertFalse(request.getParameters().containsKey("force"));
+        assertEquals("{\"job_id\":\"somejobid\"}", requestEntityToString(request));
-        assertFalse(request.getParameters().containsKey("allow_no_jobs"));
-        assertFalse(request.getParameters().containsKey("timeout"));
 
         closeJobRequest = new CloseJobRequest(jobId, "otherjobs*");
         closeJobRequest.setForce(true);
@@ -104,9 +100,8 @@ public class MLRequestConvertersTests extends ESTestCase {
         request = MLRequestConverters.closeJob(closeJobRequest);
 
         assertEquals("/_xpack/ml/anomaly_detectors/" + jobId + ",otherjobs*/_close", request.getEndpoint());
-        assertEquals(Boolean.toString(true), request.getParameters().get("force"));
+        assertEquals("{\"job_id\":\"somejobid,otherjobs*\",\"timeout\":\"10m\",\"force\":true,\"allow_no_jobs\":false}",
-        assertEquals(Boolean.toString(false), request.getParameters().get("allow_no_jobs"));
+            requestEntityToString(request));
-        assertEquals("10m", request.getParameters().get("timeout"));
     }
 
     public void testDeleteJob() {
@@ -130,4 +125,10 @@ public class MLRequestConvertersTests extends ESTestCase {
         jobBuilder.setAnalysisConfig(analysisConfig);
         return jobBuilder.build();
     }
+
+    private static String requestEntityToString(Request request) throws Exception {
+        ByteArrayOutputStream bos = new ByteArrayOutputStream();
+        request.getEntity().writeTo(bos);
+        return bos.toString("UTF-8");
+    }
 }

x-pack/protocol/src/main/java/org/elasticsearch/protocol/xpack/ml/CloseJobRequest.java

@@ -21,8 +21,10 @@ package org.elasticsearch.protocol.xpack.ml;
 import org.elasticsearch.action.ActionRequest;
 import org.elasticsearch.action.ActionRequestValidationException;
 import org.elasticsearch.common.ParseField;
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.common.xcontent.ConstructingObjectParser;
+import org.elasticsearch.common.xcontent.ObjectParser;
 import org.elasticsearch.common.xcontent.ToXContentObject;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 
@@ -35,7 +37,7 @@ import java.util.Objects;
 
 public class CloseJobRequest extends ActionRequest implements ToXContentObject {
 
-    public static final ParseField JOB_IDS = new ParseField("job_ids");
+    public static final ParseField JOB_ID = new ParseField("job_id");
     public static final ParseField TIMEOUT = new ParseField("timeout");
     public static final ParseField FORCE = new ParseField("force");
     public static final ParseField ALLOW_NO_JOBS = new ParseField("allow_no_jobs");
@@ -46,7 +48,9 @@ public class CloseJobRequest extends ActionRequest implements ToXContentObject {
         true, a -> new CloseJobRequest((List<String>) a[0]));
 
     static {
-        PARSER.declareStringArray(ConstructingObjectParser.constructorArg(), JOB_IDS);
+        PARSER.declareField(ConstructingObjectParser.constructorArg(),
+            p -> Arrays.asList(Strings.commaDelimitedListToStringArray(p.text())),
+            JOB_ID, ObjectParser.ValueType.STRING_ARRAY);
         PARSER.declareString((obj, val) -> obj.setTimeout(TimeValue.parseTimeValue(val, TIMEOUT.getPreferredName())), TIMEOUT);
         PARSER.declareBoolean(CloseJobRequest::setForce, FORCE);
         PARSER.declareBoolean(CloseJobRequest::setAllowNoJobs, ALLOW_NO_JOBS);
@@ -132,7 +136,7 @@ public class CloseJobRequest extends ActionRequest implements ToXContentObject {
      * This includes `_all` string or when no jobs have been specified
      */
     public Boolean isAllowNoJobs() {
-        return allowNoJobs;
+        return this.allowNoJobs;
     }
 
     /**
@@ -149,7 +153,7 @@ public class CloseJobRequest extends ActionRequest implements ToXContentObject {
 
     @Override
     public int hashCode() {
-        return Objects.hash(jobIds, timeout, allowNoJobs, force);
+        return Objects.hash(jobIds, timeout, force, allowNoJobs);
     }
 
     @Override
@@ -165,16 +169,14 @@ public class CloseJobRequest extends ActionRequest implements ToXContentObject {
         CloseJobRequest that = (CloseJobRequest) other;
         return Objects.equals(jobIds, that.jobIds) &&
             Objects.equals(timeout, that.timeout) &&
-            Objects.equals(allowNoJobs, that.allowNoJobs) &&
+            Objects.equals(force, that.force) &&
-            Objects.equals(force, that.force);
+            Objects.equals(allowNoJobs, that.allowNoJobs);
     }
 
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
         builder.startObject();
-
+        builder.field(JOB_ID.getPreferredName(), Strings.collectionToCommaDelimitedString(jobIds));
-        builder.field(JOB_IDS.getPreferredName(), jobIds);
-
         if (timeout != null) {
             builder.field(TIMEOUT.getPreferredName(), timeout.getStringRep());
         }
@@ -184,8 +186,12 @@ public class CloseJobRequest extends ActionRequest implements ToXContentObject {
         if (allowNoJobs != null) {
             builder.field(ALLOW_NO_JOBS.getPreferredName(), allowNoJobs);
         }
-
         builder.endObject();
         return builder;
     }
+
+    @Override
+    public String toString() {
+        return Strings.toString(this);
+    }
 }