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@d9af2d3a30
This commit is contained in:
Zachary Tong 2016-12-13 11:38:11 -05:00 committed by GitHub
parent 6ef954e3f0
commit b386ed33a1
5 changed files with 186 additions and 14 deletions

View File

@ -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<GetListAction.Request, GetListAction.R
public static class Request extends MasterNodeReadRequest<Request> {
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<GetListAction.Request, GetListAction.R
return listId;
}
public PageParams getPageParams() {
return pageParams;
}
public void setPageParams(PageParams pageParams) {
if (listId != null) {
throw new IllegalArgumentException("Param [" + PageParams.FROM.getPreferredName()
+ ", " + PageParams.SIZE.getPreferredName() + "] is incompatible with ["
+ ListDocument.ID.getPreferredName() + "].");
}
this.pageParams = pageParams;
}
@Override
public ActionRequestValidationException validate() {
ActionRequestValidationException validationException = null;
if (listId == null) {
validationException = addValidationError("List ID is required for GetList API.", validationException);
if (pageParams == null && listId == null) {
validationException = addValidationError("Both [" + ListDocument.ID.getPreferredName() + "] and ["
+ PageParams.FROM.getPreferredName() + ", " + PageParams.SIZE.getPreferredName() + "] "
+ "cannot be null" , validationException);
}
return validationException;
}
@ -196,6 +225,7 @@ public class GetListAction extends Action<GetListAction.Request, GetListAction.R
public static class TransportAction extends TransportMasterNodeReadAction<Request, Response> {
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<GetListAction.Request, GetListAction.R
@Inject
public TransportAction(Settings settings, TransportService transportService, ClusterService clusterService,
ThreadPool threadPool, ActionFilters actionFilters,
IndexNameExpressionResolver indexNameExpressionResolver,
TransportGetAction transportGetAction) {
ThreadPool threadPool, ActionFilters actionFilters,
IndexNameExpressionResolver indexNameExpressionResolver,
TransportGetAction transportGetAction, TransportSearchAction transportSearchAction) {
super(settings, GetListAction.NAME, transportService, clusterService, threadPool, actionFilters,
indexNameExpressionResolver, Request::new);
this.transportGetAction = transportGetAction;
this.transportSearchAction = transportSearchAction;
}
@Override
@ -224,6 +255,21 @@ public class GetListAction extends Action<GetListAction.Request, GetListAction.R
@Override
protected void masterOperation(Request request, ClusterState state, ActionListener<Response> 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<Response> listener) {
GetRequest getRequest = new GetRequest(PRELERT_INFO_INDEX, ListDocument.TYPE.getPreferredName(), listId);
transportGetAction.execute(getRequest, new ActionListener<GetResponse>() {
@Override
@ -255,9 +301,47 @@ public class GetListAction extends Action<GetListAction.Request, GetListAction.R
});
}
@Override
protected ClusterBlockException checkBlock(Request request, ClusterState state) {
return state.blocks().globalBlockedException(ClusterBlockLevel.METADATA_READ);
private void getLists(PageParams pageParams, ActionListener<Response> 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<SearchResponse>() {
@Override
public void onResponse(SearchResponse response) {
try {
QueryPage<ListDocument> responseBody;
if (response.getHits().hits().length > 0) {
List<ListDocument> 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);
}
});
}
}

View File

@ -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));
}

View File

@ -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<GetListAction.Request> {
@ -13,7 +14,18 @@ public class GetListActionRequestTests extends AbstractStreamableTestCase<GetLis
@Override
protected Request createTestInstance() {
return new Request(randomAsciiOfLengthBetween(1, 20));
Request request = new Request();
if (randomBoolean()) {
request.setListId(randomAsciiOfLengthBetween(1, 20));
} else {
if (randomBoolean()) {
int from = randomInt(PageParams.MAX_FROM_SIZE_SUM);
int maxSize = PageParams.MAX_FROM_SIZE_SUM - from;
int size = randomInt(maxSize);
request.setPageParams(new PageParams(from, size));
}
}
return request;
}
@Override

View File

@ -3,13 +3,22 @@
"methods": [ "GET" ],
"url": {
"path": "/_xpack/prelert/lists/{list_id}",
"paths": [ "/_xpack/prelert/lists/{list_id}" ],
"paths": [ "/_xpack/prelert/lists/", "/_xpack/prelert/lists/{list_id}" ],
"parts": {
"list_id": {
"type" : "string",
"required" : true,
"description" : "The ID of the list to fetch"
}
},
"params": {
"from": {
"type": "int",
"description": "skips a number of lists"
},
"size": {
"type": "int",
"description": "specifies a max number of lists to get"
}
}
},
"body": null

View File

@ -8,6 +8,14 @@ setup:
"items": ["abc", "xyz"]
}
- do:
xpack.prelert.put_list:
body: >
{
"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: