From 7f64f37c4657c708c00dff8542ef2b96ae6c0a60 Mon Sep 17 00:00:00 2001 From: Dimitris Athanasiou Date: Tue, 25 Apr 2017 14:44:10 +0100 Subject: [PATCH] [ML] Fix POST version of get categories API (elastic/x-pack-elasticsearch#1184) Also improves PageParams parsing to fill in defaults. relates elastic/x-pack-elasticsearch#1180 Original commit: elastic/x-pack-elasticsearch@fccd7795cadeaafbdb3c8c773dc7c0c8aa6334d9 --- .../xpack/ml/action/util/PageParams.java | 8 +-- .../rest/results/RestGetCategoriesAction.java | 5 +- .../test/ml/jobs_get_result_categories.yaml | 49 ++++++++++++++++++- 3 files changed, 55 insertions(+), 7 deletions(-) diff --git a/plugin/src/main/java/org/elasticsearch/xpack/ml/action/util/PageParams.java b/plugin/src/main/java/org/elasticsearch/xpack/ml/action/util/PageParams.java index 102d8e8ad3c..f351322aa64 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/ml/action/util/PageParams.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/ml/action/util/PageParams.java @@ -26,14 +26,14 @@ public class PageParams extends ToXContentToBytes implements Writeable { public static final int DEFAULT_SIZE = 100; - public static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( - PAGE.getPreferredName(), a -> new PageParams((int) a[0], (int) a[1])); + public static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>(PAGE.getPreferredName(), + a -> new PageParams(a[0] == null ? DEFAULT_FROM : (int) a[0], a[1] == null ? DEFAULT_SIZE : (int) a[1])); public static final int MAX_FROM_SIZE_SUM = 10000; static { - PARSER.declareInt(ConstructingObjectParser.constructorArg(), FROM); - PARSER.declareInt(ConstructingObjectParser.constructorArg(), SIZE); + PARSER.declareInt(ConstructingObjectParser.optionalConstructorArg(), FROM); + PARSER.declareInt(ConstructingObjectParser.optionalConstructorArg(), SIZE); } private final int from; diff --git a/plugin/src/main/java/org/elasticsearch/xpack/ml/rest/results/RestGetCategoriesAction.java b/plugin/src/main/java/org/elasticsearch/xpack/ml/rest/results/RestGetCategoriesAction.java index a9b72ec4d9a..204453792ba 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/ml/rest/results/RestGetCategoriesAction.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/ml/rest/results/RestGetCategoriesAction.java @@ -49,9 +49,10 @@ public class RestGetCategoriesAction extends BaseRestHandler { if (bodyBytes != null && bodyBytes.length() > 0) { XContentParser parser = restRequest.contentParser(); request = GetCategoriesAction.Request.parseRequest(jobId, parser); - request.setCategoryId(categoryId); + if (!Strings.isNullOrEmpty(categoryId)) { + request.setCategoryId(categoryId); + } } else { - request = new Request(jobId); if (!Strings.isNullOrEmpty(categoryId)) { request.setCategoryId(categoryId); diff --git a/plugin/src/test/resources/rest-api-spec/test/ml/jobs_get_result_categories.yaml b/plugin/src/test/resources/rest-api-spec/test/ml/jobs_get_result_categories.yaml index ee01b418dd0..39ddcbdd5da 100644 --- a/plugin/src/test/resources/rest-api-spec/test/ml/jobs_get_result_categories.yaml +++ b/plugin/src/test/resources/rest-api-spec/test/ml/jobs_get_result_categories.yaml @@ -49,7 +49,54 @@ setup: - match: { categories.1.category_id: 2 } --- -"Test result category api": +"Test get categories with pagination": + - do: + xpack.ml.get_categories: + job_id: "farequote" + size: 1 + + - length: { categories: 1 } + - match: { categories.0.job_id: farequote } + - match: { categories.0.category_id: 1 } + + - do: + xpack.ml.get_categories: + job_id: "farequote" + from: 1 + size: 2 + + - length: { categories: 1 } + - match: { categories.0.job_id: farequote } + - match: { categories.0.category_id: 2 } + +--- +"Test post get categories with pagination": + - do: + xpack.ml.get_categories: + job_id: "farequote" + body: > + { + "page": { "size": 1} + } + + - length: { categories: 1 } + - match: { categories.0.job_id: farequote } + - match: { categories.0.category_id: 1 } + + - do: + xpack.ml.get_categories: + job_id: "farequote" + body: > + { + "page": { "from":1, "size": 1} + } + + - length: { categories: 1 } + - match: { categories.0.job_id: farequote } + - match: { categories.0.category_id: 2 } + +--- +"Test get category by id": - do: xpack.ml.get_categories: job_id: "farequote"