[ML] Get Filters should use executeAsyncWithOrigin (elastic/x-pack-elasticsearch#3295)

* Get Filters should use executeAsyncWithOrigin

Original commit: elastic/x-pack-elasticsearch@786c7bfd06
This commit is contained in:
David Kyle 2017-12-12 11:15:54 +00:00 committed by GitHub
parent 6113b86bdb
commit 0d46e9035c
7 changed files with 44 additions and 57 deletions

View File

@ -202,8 +202,7 @@ public class DeleteFilterAction extends Action<DeleteFilterAction.Request, Delet
@Override @Override
public void onFailure(Exception e) { public void onFailure(Exception e) {
logger.error("Could not delete filter with ID [" + filterId + "]", e); listener.onFailure(ExceptionsHelper.serverError("Could not delete filter with ID [" + filterId + "]", e));
listener.onFailure(new IllegalStateException("Could not delete filter with ID [" + filterId + "]", e));
} }
}); });
} }

View File

@ -11,14 +11,14 @@ import org.elasticsearch.action.ActionRequest;
import org.elasticsearch.action.ActionRequestBuilder; import org.elasticsearch.action.ActionRequestBuilder;
import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.action.ActionResponse; import org.elasticsearch.action.ActionResponse;
import org.elasticsearch.action.get.GetAction;
import org.elasticsearch.action.get.GetRequest; import org.elasticsearch.action.get.GetRequest;
import org.elasticsearch.action.get.GetResponse; import org.elasticsearch.action.get.GetResponse;
import org.elasticsearch.action.get.TransportGetAction;
import org.elasticsearch.action.search.SearchRequest; import org.elasticsearch.action.search.SearchRequest;
import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.action.search.TransportSearchAction;
import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.ActionFilters;
import org.elasticsearch.action.support.HandledTransportAction; import org.elasticsearch.action.support.HandledTransportAction;
import org.elasticsearch.client.Client;
import org.elasticsearch.client.ElasticsearchClient; import org.elasticsearch.client.ElasticsearchClient;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.common.Strings; import org.elasticsearch.common.Strings;
@ -51,6 +51,8 @@ import java.util.List;
import java.util.Objects; import java.util.Objects;
import static org.elasticsearch.action.ValidateActions.addValidationError; import static org.elasticsearch.action.ValidateActions.addValidationError;
import static org.elasticsearch.xpack.ClientHelper.ML_ORIGIN;
import static org.elasticsearch.xpack.ClientHelper.executeAsyncWithOrigin;
public class GetFiltersAction extends Action<GetFiltersAction.Request, GetFiltersAction.Response, GetFiltersAction.RequestBuilder> { public class GetFiltersAction extends Action<GetFiltersAction.Request, GetFiltersAction.Response, GetFiltersAction.RequestBuilder> {
@ -81,10 +83,6 @@ public class GetFiltersAction extends Action<GetFiltersAction.Request, GetFilter
} }
public void setFilterId(String filterId) { public void setFilterId(String filterId) {
if (pageParams != null) {
throw new IllegalArgumentException("Param [" + MlFilter.ID.getPreferredName() + "] is incompatible with ["
+ PageParams.FROM.getPreferredName()+ ", " + PageParams.SIZE.getPreferredName() + "].");
}
this.filterId = filterId; this.filterId = filterId;
} }
@ -97,21 +95,16 @@ public class GetFiltersAction extends Action<GetFiltersAction.Request, GetFilter
} }
public void setPageParams(PageParams pageParams) { public void setPageParams(PageParams pageParams) {
if (filterId != null) {
throw new IllegalArgumentException("Param [" + PageParams.FROM.getPreferredName()
+ ", " + PageParams.SIZE.getPreferredName() + "] is incompatible with ["
+ MlFilter.ID.getPreferredName() + "].");
}
this.pageParams = pageParams; this.pageParams = pageParams;
} }
@Override @Override
public ActionRequestValidationException validate() { public ActionRequestValidationException validate() {
ActionRequestValidationException validationException = null; ActionRequestValidationException validationException = null;
if (pageParams == null && filterId == null) { if (pageParams != null && filterId != null) {
validationException = addValidationError("Both [" + MlFilter.ID.getPreferredName() + "] and [" validationException = addValidationError("Params [" + PageParams.FROM.getPreferredName() +
+ PageParams.FROM.getPreferredName() + ", " + PageParams.SIZE.getPreferredName() + "] " ", " + PageParams.SIZE.getPreferredName() + "] are incompatible with ["
+ "cannot be null" , validationException); + MlFilter.ID.getPreferredName() + "]", validationException);
} }
return validationException; return validationException;
} }
@ -218,18 +211,16 @@ public class GetFiltersAction extends Action<GetFiltersAction.Request, GetFilter
public static class TransportAction extends HandledTransportAction<Request, Response> { public static class TransportAction extends HandledTransportAction<Request, Response> {
private final TransportGetAction transportGetAction; private final Client client;
private final TransportSearchAction transportSearchAction;
@Inject @Inject
public TransportAction(Settings settings, ThreadPool threadPool, public TransportAction(Settings settings, ThreadPool threadPool,
TransportService transportService, ActionFilters actionFilters, TransportService transportService, ActionFilters actionFilters,
IndexNameExpressionResolver indexNameExpressionResolver, IndexNameExpressionResolver indexNameExpressionResolver,
TransportGetAction transportGetAction, TransportSearchAction transportSearchAction) { Client client) {
super(settings, NAME, threadPool, transportService, actionFilters, super(settings, NAME, threadPool, transportService, actionFilters,
indexNameExpressionResolver, Request::new); indexNameExpressionResolver, Request::new);
this.transportGetAction = transportGetAction; this.client = client;
this.transportSearchAction = transportSearchAction;
} }
@Override @Override
@ -237,16 +228,18 @@ public class GetFiltersAction extends Action<GetFiltersAction.Request, GetFilter
final String filterId = request.getFilterId(); final String filterId = request.getFilterId();
if (!Strings.isNullOrEmpty(filterId)) { if (!Strings.isNullOrEmpty(filterId)) {
getFilter(filterId, listener); getFilter(filterId, listener);
} else if (request.getPageParams() != null) { } else {
getFilters(request.getPageParams(), listener); PageParams pageParams = request.getPageParams();
} else { if (pageParams == null) {
throw new IllegalStateException("Both filterId and pageParams are null"); pageParams = PageParams.defaultParams();
}
getFilters(pageParams, listener);
} }
} }
private void getFilter(String filterId, ActionListener<Response> listener) { private void getFilter(String filterId, ActionListener<Response> listener) {
GetRequest getRequest = new GetRequest(MlMetaIndex.INDEX_NAME, MlMetaIndex.TYPE, MlFilter.documentId(filterId)); GetRequest getRequest = new GetRequest(MlMetaIndex.INDEX_NAME, MlMetaIndex.TYPE, MlFilter.documentId(filterId));
transportGetAction.execute(getRequest, new ActionListener<GetResponse>() { executeAsyncWithOrigin(client, ML_ORIGIN, GetAction.INSTANCE, getRequest, new ActionListener<GetResponse>() {
@Override @Override
public void onResponse(GetResponse getDocResponse) { public void onResponse(GetResponse getDocResponse) {
@ -287,7 +280,7 @@ public class GetFiltersAction extends Action<GetFiltersAction.Request, GetFilter
.indicesOptions(JobProvider.addIgnoreUnavailable(SearchRequest.DEFAULT_INDICES_OPTIONS)) .indicesOptions(JobProvider.addIgnoreUnavailable(SearchRequest.DEFAULT_INDICES_OPTIONS))
.source(sourceBuilder); .source(sourceBuilder);
transportSearchAction.execute(searchRequest, new ActionListener<SearchResponse>() { executeAsyncWithOrigin(client.threadPool().getThreadContext(), ML_ORIGIN, searchRequest, new ActionListener<SearchResponse>() {
@Override @Override
public void onResponse(SearchResponse response) { public void onResponse(SearchResponse response) {
List<MlFilter> docs = new ArrayList<>(); List<MlFilter> docs = new ArrayList<>();
@ -310,7 +303,8 @@ public class GetFiltersAction extends Action<GetFiltersAction.Request, GetFilter
public void onFailure(Exception e) { public void onFailure(Exception e) {
listener.onFailure(e); listener.onFailure(e);
} }
}); },
client::search);
} }
} }
} }

View File

@ -213,7 +213,8 @@ public class PutCalendarAction extends Action<PutCalendarAction.Request, PutCale
@Override @Override
public void onFailure(Exception e) { public void onFailure(Exception e) {
listener.onFailure(e); listener.onFailure(
ExceptionsHelper.serverError("Error putting calendar with id [" + calendar.getId() + "]", e));
} }
}); });
} }

View File

@ -5,7 +5,6 @@
*/ */
package org.elasticsearch.xpack.ml.action; package org.elasticsearch.xpack.ml.action;
import org.elasticsearch.ResourceNotFoundException;
import org.elasticsearch.action.Action; import org.elasticsearch.action.Action;
import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionRequest;
@ -199,8 +198,7 @@ public class PutFilterAction extends Action<PutFilterAction.Request, PutFilterAc
@Override @Override
public void onFailure(Exception e) { public void onFailure(Exception e) {
listener.onFailure( listener.onFailure(ExceptionsHelper.serverError("Error putting filter with id [" + filter.getId() + "]", e));
new ResourceNotFoundException("Could not create filter with ID [" + filter.getId() + "]", e));
} }
}); });
} }

View File

@ -40,10 +40,9 @@ public class RestGetFiltersAction extends BaseRestHandler {
if (!Strings.isNullOrEmpty(filterId)) { if (!Strings.isNullOrEmpty(filterId)) {
getListRequest.setFilterId(filterId); getListRequest.setFilterId(filterId);
} }
if (restRequest.hasParam(PageParams.FROM.getPreferredName()) if (restRequest.hasParam(PageParams.FROM.getPreferredName()) || restRequest.hasParam(PageParams.SIZE.getPreferredName())) {
|| restRequest.hasParam(PageParams.SIZE.getPreferredName()) getListRequest.setPageParams(
|| Strings.isNullOrEmpty(filterId)) { new PageParams(restRequest.paramAsInt(PageParams.FROM.getPreferredName(), PageParams.DEFAULT_FROM),
getListRequest.setPageParams(new PageParams(restRequest.paramAsInt(PageParams.FROM.getPreferredName(), PageParams.DEFAULT_FROM),
restRequest.paramAsInt(PageParams.SIZE.getPreferredName(), PageParams.DEFAULT_SIZE))); restRequest.paramAsInt(PageParams.SIZE.getPreferredName(), PageParams.DEFAULT_SIZE)));
} }
return channel -> client.execute(GetFiltersAction.INSTANCE, getListRequest, new RestStatusToXContentListener<>(channel)); return channel -> client.execute(GetFiltersAction.INSTANCE, getListRequest, new RestStatusToXContentListener<>(channel));

View File

@ -202,3 +202,19 @@ setup:
catch: missing catch: missing
xpack.ml.get_filters: xpack.ml.get_filters:
filter_id: "filter-foo" filter_id: "filter-foo"
---
"Test get all filter given no filter exists":
- do:
xpack.ml.delete_filter:
filter_id: "filter-foo"
- do:
xpack.ml.delete_filter:
filter_id: "filter-foo2"
- do:
xpack.ml.get_filters: {}
- match: { count: 0 }
- match: { filters: [] }

View File

@ -1,20 +0,0 @@
---
"Test get all filter given no filter exists":
- do:
xpack.ml.put_filter:
filter_id: filter-foo
body: >
{
"filter_id": "filter-foo",
"items": ["abc", "xyz"]
}
- do:
xpack.ml.delete_filter:
filter_id: "filter-foo"
- do:
xpack.ml.get_filters: {}
- match: { count: 0 }
- match: { filters: [] }