From b386ed33a163210ed87a0e753df3aec11ac3e53f Mon Sep 17 00:00:00 2001 From: Zachary Tong Date: Tue, 13 Dec 2016 11:38:11 -0500 Subject: [PATCH] Add ability to get all lists, ensure params are mutually exclusive (elastic/elasticsearch#514) Also removes some unnecessary stuff in validate() Related elastic/elasticsearch#291 Original commit: elastic/x-pack-elasticsearch@d9af2d3a30840ec9382865248c0c2dad29880bb3 --- .../xpack/prelert/action/GetListAction.java | 104 ++++++++++++++++-- .../prelert/rest/list/RestGetListAction.java | 15 ++- .../action/GetListActionRequestTests.java | 14 ++- .../api/xpack.prelert.get_lists.json | 13 ++- .../rest-api-spec/test/list_crud.yaml | 54 +++++++++ 5 files changed, 186 insertions(+), 14 deletions(-) diff --git a/elasticsearch/src/main/java/org/elasticsearch/xpack/prelert/action/GetListAction.java b/elasticsearch/src/main/java/org/elasticsearch/xpack/prelert/action/GetListAction.java index 8e79d064382..af19e8db463 100644 --- a/elasticsearch/src/main/java/org/elasticsearch/xpack/prelert/action/GetListAction.java +++ b/elasticsearch/src/main/java/org/elasticsearch/xpack/prelert/action/GetListAction.java @@ -12,6 +12,9 @@ import org.elasticsearch.action.ActionResponse; import org.elasticsearch.action.get.GetRequest; import org.elasticsearch.action.get.GetResponse; import org.elasticsearch.action.get.TransportGetAction; +import org.elasticsearch.action.search.SearchRequest; +import org.elasticsearch.action.search.SearchResponse; +import org.elasticsearch.action.search.TransportSearchAction; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.master.MasterNodeReadOperationRequestBuilder; import org.elasticsearch.action.support.master.MasterNodeReadRequest; @@ -22,6 +25,7 @@ import org.elasticsearch.cluster.block.ClusterBlockException; import org.elasticsearch.cluster.block.ClusterBlockLevel; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.io.stream.StreamInput; @@ -32,13 +36,18 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.rest.RestStatus; +import org.elasticsearch.search.SearchHit; +import org.elasticsearch.search.builder.SearchSourceBuilder; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; import org.elasticsearch.xpack.prelert.job.persistence.QueryPage; +import org.elasticsearch.xpack.prelert.job.results.PageParams; import org.elasticsearch.xpack.prelert.lists.ListDocument; import java.io.IOException; +import java.util.ArrayList; import java.util.Collections; +import java.util.List; import java.util.Objects; import static org.elasticsearch.action.ValidateActions.addValidationError; @@ -66,11 +75,16 @@ public class GetListAction extends Action { private String listId; + private PageParams pageParams; - Request() { + public Request() { } - public Request(String listId) { + public void setListId(String listId) { + if (pageParams != null) { + throw new IllegalArgumentException("Param [" + ListDocument.ID.getPreferredName() + "] is incompatible with [" + + PageParams.FROM.getPreferredName()+ ", " + PageParams.SIZE.getPreferredName() + "]."); + } this.listId = listId; } @@ -78,11 +92,26 @@ public class GetListAction extends Action { private final TransportGetAction transportGetAction; + private final TransportSearchAction transportSearchAction; // TODO these need to be moved to a settings object later // See #20 @@ -203,12 +233,13 @@ public class GetListAction extends Action listener) throws Exception { final String listId = request.getListId(); + if (!Strings.isNullOrEmpty(listId)) { + getList(listId, listener); + } else if (request.getPageParams() != null) { + getLists(request.getPageParams(), listener); + } else { + throw new IllegalStateException("Both listId and pageParams are null"); + } + } + + @Override + protected ClusterBlockException checkBlock(Request request, ClusterState state) { + return state.blocks().globalBlockedException(ClusterBlockLevel.METADATA_READ); + } + + private void getList(String listId, ActionListener listener) { GetRequest getRequest = new GetRequest(PRELERT_INFO_INDEX, ListDocument.TYPE.getPreferredName(), listId); transportGetAction.execute(getRequest, new ActionListener() { @Override @@ -255,9 +301,47 @@ public class GetListAction extends Action listener) { + SearchSourceBuilder sourceBuilder = new SearchSourceBuilder() + .from(pageParams.getFrom()) + .size(pageParams.getSize()); + + SearchRequest searchRequest = new SearchRequest(new String[]{PRELERT_INFO_INDEX}, sourceBuilder) + .types(ListDocument.TYPE.getPreferredName()); + + transportSearchAction.execute(searchRequest, new ActionListener() { + @Override + public void onResponse(SearchResponse response) { + + try { + QueryPage responseBody; + if (response.getHits().hits().length > 0) { + List docs = new ArrayList<>(response.getHits().hits().length); + for (SearchHit hit : response.getHits().getHits()) { + BytesReference docSource = hit.sourceRef(); + XContentParser parser = XContentFactory.xContent(docSource).createParser(docSource); + docs.add(ListDocument.PARSER.apply(parser, () -> parseFieldMatcher)); + } + + responseBody = new QueryPage<>(docs, docs.size(), ListDocument.RESULTS_FIELD); + + Response listResponse = new Response(responseBody); + listener.onResponse(listResponse); + } else { + this.onFailure(QueryPage.emptyQueryPage(ListDocument.RESULTS_FIELD)); + } + + } catch (Exception e) { + this.onFailure(e); + } + } + + + @Override + public void onFailure(Exception e) { + listener.onFailure(e); + } + }); } } diff --git a/elasticsearch/src/main/java/org/elasticsearch/xpack/prelert/rest/list/RestGetListAction.java b/elasticsearch/src/main/java/org/elasticsearch/xpack/prelert/rest/list/RestGetListAction.java index bf2ce3e9360..efa25926a4c 100644 --- a/elasticsearch/src/main/java/org/elasticsearch/xpack/prelert/rest/list/RestGetListAction.java +++ b/elasticsearch/src/main/java/org/elasticsearch/xpack/prelert/rest/list/RestGetListAction.java @@ -6,6 +6,7 @@ package org.elasticsearch.xpack.prelert.rest.list; import org.elasticsearch.client.node.NodeClient; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.rest.BaseRestHandler; @@ -14,6 +15,7 @@ import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.action.RestStatusToXContentListener; import org.elasticsearch.xpack.prelert.PrelertPlugin; import org.elasticsearch.xpack.prelert.action.GetListAction; +import org.elasticsearch.xpack.prelert.job.results.PageParams; import org.elasticsearch.xpack.prelert.lists.ListDocument; import java.io.IOException; @@ -28,11 +30,22 @@ public class RestGetListAction extends BaseRestHandler { this.transportGetListAction = transportGetListAction; controller.registerHandler(RestRequest.Method.GET, PrelertPlugin.BASE_PATH + "lists/{" + ListDocument.ID.getPreferredName() + "}", this); + controller.registerHandler(RestRequest.Method.GET, PrelertPlugin.BASE_PATH + "lists/", this); } @Override protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) throws IOException { - GetListAction.Request getListRequest = new GetListAction.Request(restRequest.param(ListDocument.ID.getPreferredName())); + GetListAction.Request getListRequest = new GetListAction.Request(); + String listId = restRequest.param(ListDocument.ID.getPreferredName()); + if (!Strings.isNullOrEmpty(listId)) { + getListRequest.setListId(listId); + } + if (restRequest.hasParam(PageParams.FROM.getPreferredName()) + || restRequest.hasParam(PageParams.SIZE.getPreferredName()) + || Strings.isNullOrEmpty(listId)) { + getListRequest.setPageParams(new PageParams(restRequest.paramAsInt(PageParams.FROM.getPreferredName(), PageParams.DEFAULT_FROM), + restRequest.paramAsInt(PageParams.SIZE.getPreferredName(), PageParams.DEFAULT_SIZE))); + } return channel -> transportGetListAction.execute(getListRequest, new RestStatusToXContentListener<>(channel)); } diff --git a/elasticsearch/src/test/java/org/elasticsearch/xpack/prelert/action/GetListActionRequestTests.java b/elasticsearch/src/test/java/org/elasticsearch/xpack/prelert/action/GetListActionRequestTests.java index 4a94489b55f..d038120b77a 100644 --- a/elasticsearch/src/test/java/org/elasticsearch/xpack/prelert/action/GetListActionRequestTests.java +++ b/elasticsearch/src/test/java/org/elasticsearch/xpack/prelert/action/GetListActionRequestTests.java @@ -6,6 +6,7 @@ package org.elasticsearch.xpack.prelert.action; import org.elasticsearch.xpack.prelert.action.GetListAction.Request; +import org.elasticsearch.xpack.prelert.job.results.PageParams; import org.elasticsearch.xpack.prelert.support.AbstractStreamableTestCase; public class GetListActionRequestTests extends AbstractStreamableTestCase { @@ -13,7 +14,18 @@ public class GetListActionRequestTests extends AbstractStreamableTestCase + { + "id": "foo2", + "items": ["123", "lmnop"] + } + - do: indices.refresh: {} @@ -33,6 +41,51 @@ setup: items: ["abc", "xyz"] --- +"Test get lists API": + + - do: + xpack.prelert.get_lists: {} + + - match: { count: 2 } + - match: + lists.0: + id: "foo" + items: ["abc", "xyz"] + + - match: + lists.1: + id: "foo2" + items: ["123", "lmnop"] + + - do: + xpack.prelert.get_lists: + from: 1 + size: 1 + + - match: { count: 1 } + +--- +"Test invalid param combinations": + + - do: + catch: request + xpack.prelert.get_lists: + list_id: "foo" + from: 0 + + - do: + catch: request + xpack.prelert.get_lists: + list_id: "foo" + size: 1 + + - do: + catch: request + xpack.prelert.get_lists: + list_id: "foo" + from: 0 + size: 1 +--- "Test create list api": - do: xpack.prelert.put_list: @@ -41,6 +94,7 @@ setup: "id": "foo2", "items": ["abc", "xyz"] } + - match: { acknowledged: true } - do: