From b02150a5ed8bd4f2dc393bfd3b6de0518149c468 Mon Sep 17 00:00:00 2001 From: Benjamin Trent Date: Wed, 22 Aug 2018 06:54:08 -0500 Subject: [PATCH] HLRC: close job refactor (#33031) * HLRC: close job refactor * Changing refactor to make job_id a string * Changing set entity methodology --- .../client/MLRequestConverters.java | 17 +++--------- .../client/MLRequestConvertersTests.java | 21 ++++++++------- .../protocol/xpack/ml/CloseJobRequest.java | 26 ++++++++++++------- 3 files changed, 30 insertions(+), 34 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/MLRequestConverters.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/MLRequestConverters.java index 9a4e825be7c..1c2b6f79eaf 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/MLRequestConverters.java +++ b/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); - - 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()); - } - + request.setEntity(createEntity(closeJobRequest, REQUEST_BODY_CONTENT_TYPE)); return request; } diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/MLRequestConvertersTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/MLRequestConvertersTests.java index 9ed09d06b72..0d95c359658 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/MLRequestConvertersTests.java +++ b/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(); - request.getEntity().writeTo(bos); - assertEquals(bos.toString("UTF-8"), "{\"job_id\":\""+ jobId +"\",\"timeout\":\"10m\"}"); + assertEquals(requestEntityToString(request), "{\"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")); - assertFalse(request.getParameters().containsKey("allow_no_jobs")); - assertFalse(request.getParameters().containsKey("timeout")); + assertEquals("{\"job_id\":\"somejobid\"}", requestEntityToString(request)); 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(Boolean.toString(false), request.getParameters().get("allow_no_jobs")); - assertEquals("10m", request.getParameters().get("timeout")); + assertEquals("{\"job_id\":\"somejobid,otherjobs*\",\"timeout\":\"10m\",\"force\":true,\"allow_no_jobs\":false}", + requestEntityToString(request)); } 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"); + } } diff --git a/x-pack/protocol/src/main/java/org/elasticsearch/protocol/xpack/ml/CloseJobRequest.java b/x-pack/protocol/src/main/java/org/elasticsearch/protocol/xpack/ml/CloseJobRequest.java index 1df5a02889e..3d54bfb9488 100644 --- a/x-pack/protocol/src/main/java/org/elasticsearch/protocol/xpack/ml/CloseJobRequest.java +++ b/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) 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_IDS.getPreferredName(), jobIds); - + builder.field(JOB_ID.getPreferredName(), Strings.collectionToCommaDelimitedString(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); + } }