From 90b2b74e7691fa81489e76b6840e2187debfe29d Mon Sep 17 00:00:00 2001 From: David Roberts Date: Fri, 29 Sep 2017 09:17:36 +0100 Subject: [PATCH] [ML] Tolerate a body without timestamp for get_buckets with a timestamp (elastic/x-pack-elasticsearch#2640) When getting a single bucket, the get_buckets API can take a timestamp either in the body or in the URL. Prior to this change, if a timestamp was specified in the URL but a body not containing a timestamp was specified (either empty or containing other parameters like exclude_interim or sort) then it would cause a bad_request exception. This in turn causes problems for clients that cannot send a body when GETting and always send a body when POSTing. This change fixes get_buckets to always read any timestamp in the URL, even when a body is sent. relates elastic/x-pack-elasticsearch#2515 Original commit: elastic/x-pack-elasticsearch@5c23dd972edb6f95aa7ccd868306f60abcf330b1 --- .../ml/rest/results/RestGetBucketsAction.java | 16 ++++++++++------ .../test/ml/jobs_get_result_buckets.yml | 12 ++++++++++++ 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/plugin/src/main/java/org/elasticsearch/xpack/ml/rest/results/RestGetBucketsAction.java b/plugin/src/main/java/org/elasticsearch/xpack/ml/rest/results/RestGetBucketsAction.java index a653f272089..b73a8bd0755 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/ml/rest/results/RestGetBucketsAction.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/ml/rest/results/RestGetBucketsAction.java @@ -6,6 +6,7 @@ package org.elasticsearch.xpack.ml.rest.results; import org.elasticsearch.client.node.NodeClient; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.rest.BaseRestHandler; @@ -45,20 +46,23 @@ public class RestGetBucketsAction extends BaseRestHandler { @Override protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) throws IOException { String jobId = restRequest.param(Job.ID.getPreferredName()); + String timestamp = restRequest.param(GetBucketsAction.Request.TIMESTAMP.getPreferredName()); final GetBucketsAction.Request request; if (restRequest.hasContentOrSourceParam()) { XContentParser parser = restRequest.contentOrSourceParamParser(); request = GetBucketsAction.Request.parseRequest(jobId, parser); + + // A timestamp in the URL overrides any timestamp that may also have been set in the body + if (!Strings.isNullOrEmpty(timestamp)) { + request.setTimestamp(timestamp); + } } else { request = new GetBucketsAction.Request(jobId); // Check if the REST param is set first so mutually exclusive - // options will only cause an error if set - if (restRequest.hasParam(GetBucketsAction.Request.TIMESTAMP.getPreferredName())) { - String timestamp = restRequest.param(GetBucketsAction.Request.TIMESTAMP.getPreferredName()); - if (timestamp != null && !timestamp.isEmpty()) { - request.setTimestamp(timestamp); - } + // options will cause an error if set + if (!Strings.isNullOrEmpty(timestamp)) { + request.setTimestamp(timestamp); } // multiple bucket options if (restRequest.hasParam(PageParams.FROM.getPreferredName()) || restRequest.hasParam(PageParams.SIZE.getPreferredName())) { diff --git a/plugin/src/test/resources/rest-api-spec/test/ml/jobs_get_result_buckets.yml b/plugin/src/test/resources/rest-api-spec/test/ml/jobs_get_result_buckets.yml index 02bebcf33a5..2a7a7970e5d 100644 --- a/plugin/src/test/resources/rest-api-spec/test/ml/jobs_get_result_buckets.yml +++ b/plugin/src/test/resources/rest-api-spec/test/ml/jobs_get_result_buckets.yml @@ -170,6 +170,18 @@ setup: - match: { buckets.0.job_id: jobs-get-result-buckets } - match: { buckets.0.result_type: bucket} +--- +"Test result single bucket api with empty body": + - do: + xpack.ml.get_buckets: + job_id: "jobs-get-result-buckets" + timestamp: "2016-06-01T00:00:00Z" + body: {} + + - match: { buckets.0.timestamp: 1464739200000} + - match: { buckets.0.job_id: jobs-get-result-buckets } + - match: { buckets.0.result_type: bucket} + --- "Test mutually-exclusive params": - do: