diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/GetFiltersAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/GetFiltersAction.java index de22800832dfc..a304704914470 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/GetFiltersAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/GetFiltersAction.java @@ -6,7 +6,6 @@ package org.elasticsearch.xpack.core.ml.action; import org.elasticsearch.action.ActionRequestBuilder; -import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.ActionType; import org.elasticsearch.client.ElasticsearchClient; import org.elasticsearch.common.io.stream.StreamInput; @@ -14,14 +13,11 @@ import org.elasticsearch.rest.RestStatus; import org.elasticsearch.xpack.core.action.AbstractGetResourcesRequest; import org.elasticsearch.xpack.core.action.AbstractGetResourcesResponse; -import org.elasticsearch.xpack.core.action.util.PageParams; import org.elasticsearch.xpack.core.action.util.QueryPage; import org.elasticsearch.xpack.core.ml.job.config.MlFilter; import java.io.IOException; -import static org.elasticsearch.action.ValidateActions.addValidationError; - public class GetFiltersAction extends ActionType { @@ -35,31 +31,16 @@ private GetFiltersAction() { public static class Request extends AbstractGetResourcesRequest { public Request() { - // Put our own defaults for backwards compatibility - super(null, null, true); - } - - public Request(StreamInput in) throws IOException { - super(in); + setAllowNoResources(true); } - public void setFilterId(String filterId) { + public Request(String filterId) { setResourceId(filterId); + setAllowNoResources(true); } - public String getFilterId() { - return getResourceId(); - } - - @Override - public ActionRequestValidationException validate() { - ActionRequestValidationException validationException = null; - if (getPageParams() != null && getResourceId() != null) { - validationException = addValidationError("Params [" + PageParams.FROM.getPreferredName() + - ", " + PageParams.SIZE.getPreferredName() + "] are incompatible with [" - + MlFilter.ID.getPreferredName() + "]", validationException); - } - return validationException; + public Request(StreamInput in) throws IOException { + super(in); } @Override diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/GetFiltersActionRequestTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/GetFiltersActionRequestTests.java index a4419a60fca20..71602af1539dd 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/GetFiltersActionRequestTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/GetFiltersActionRequestTests.java @@ -7,23 +7,22 @@ import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.test.AbstractWireSerializingTestCase; -import org.elasticsearch.xpack.core.ml.action.GetFiltersAction.Request; import org.elasticsearch.xpack.core.action.util.PageParams; +import org.elasticsearch.xpack.core.ml.action.GetFiltersAction.Request; public class GetFiltersActionRequestTests extends AbstractWireSerializingTestCase { @Override protected Request createTestInstance() { + if (randomBoolean()) { + return new Request(randomAlphaOfLength(10)); + } Request request = new Request(); if (randomBoolean()) { - request.setFilterId(randomAlphaOfLengthBetween(1, 20)); - } else { - if (randomBoolean()) { - int from = randomInt(10000); - int size = randomInt(10000); - request.setPageParams(new PageParams(from, size)); - } + int from = randomInt(10000); + int size = randomInt(10000); + request.setPageParams(new PageParams(from, size)); } return request; } diff --git a/x-pack/plugin/ml/qa/ml-with-security/build.gradle b/x-pack/plugin/ml/qa/ml-with-security/build.gradle index 1146db74510bb..bdbe35ed8b885 100644 --- a/x-pack/plugin/ml/qa/ml-with-security/build.gradle +++ b/x-pack/plugin/ml/qa/ml-with-security/build.gradle @@ -127,7 +127,6 @@ integTest.runner { 'ml/filter_crud/Test create filter api with mismatching body ID', 'ml/filter_crud/Test create filter given invalid filter_id', 'ml/filter_crud/Test get filter API with bad ID', - 'ml/filter_crud/Test invalid param combinations', 'ml/filter_crud/Test non-existing filter', 'ml/filter_crud/Test update filter given remove item is not present', 'ml/filter_crud/Test get all filter given index exists but no mapping for filter_id', diff --git a/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/MlNativeAutodetectIntegTestCase.java b/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/MlNativeAutodetectIntegTestCase.java index 2d41cf6409700..8a3c4236b7950 100644 --- a/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/MlNativeAutodetectIntegTestCase.java +++ b/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/MlNativeAutodetectIntegTestCase.java @@ -39,7 +39,6 @@ import org.elasticsearch.xpack.core.ml.action.PostDataAction; import org.elasticsearch.xpack.core.ml.action.PutCalendarAction; import org.elasticsearch.xpack.core.ml.action.PutDatafeedAction; -import org.elasticsearch.xpack.core.ml.action.PutFilterAction; import org.elasticsearch.xpack.core.ml.action.PutJobAction; import org.elasticsearch.xpack.core.ml.action.RevertModelSnapshotAction; import org.elasticsearch.xpack.core.ml.action.StartDatafeedAction; @@ -53,7 +52,6 @@ import org.elasticsearch.xpack.core.ml.job.config.Job; import org.elasticsearch.xpack.core.ml.job.config.JobState; import org.elasticsearch.xpack.core.ml.job.config.JobUpdate; -import org.elasticsearch.xpack.core.ml.job.config.MlFilter; import org.elasticsearch.xpack.core.ml.job.persistence.AnomalyDetectorsIndex; import org.elasticsearch.xpack.core.ml.job.process.autodetect.state.DataCounts; import org.elasticsearch.xpack.core.ml.job.process.autodetect.state.ModelSnapshot; @@ -353,10 +351,6 @@ protected List getForecasts(String jobId, ForecastRequestStats forecas return forecasts; } - protected PutFilterAction.Response putMlFilter(MlFilter filter) { - return client().execute(PutFilterAction.INSTANCE, new PutFilterAction.Request(filter)).actionGet(); - } - protected PutCalendarAction.Response putCalendar(String calendarId, List jobIds, String description) { PutCalendarAction.Request request = new PutCalendarAction.Request(new Calendar(calendarId, jobIds, description)); return client().execute(PutCalendarAction.INSTANCE, request).actionGet(); diff --git a/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/MlNativeIntegTestCase.java b/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/MlNativeIntegTestCase.java index f91afb7ac6e34..901268ef0cae9 100644 --- a/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/MlNativeIntegTestCase.java +++ b/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/MlNativeIntegTestCase.java @@ -32,12 +32,15 @@ import org.elasticsearch.xpack.core.ml.MlMetadata; import org.elasticsearch.xpack.core.ml.MlTasks; import org.elasticsearch.xpack.core.ml.action.DeleteExpiredDataAction; +import org.elasticsearch.xpack.core.ml.action.GetFiltersAction; import org.elasticsearch.xpack.core.ml.action.OpenJobAction; +import org.elasticsearch.xpack.core.ml.action.PutFilterAction; import org.elasticsearch.xpack.core.ml.action.StartDataFrameAnalyticsAction; import org.elasticsearch.xpack.core.ml.action.StartDatafeedAction; import org.elasticsearch.xpack.core.ml.datafeed.DatafeedState; import org.elasticsearch.xpack.core.ml.dataframe.DataFrameAnalyticsTaskState; import org.elasticsearch.xpack.core.ml.job.config.JobTaskState; +import org.elasticsearch.xpack.core.ml.job.config.MlFilter; import org.elasticsearch.xpack.core.security.SecurityField; import org.elasticsearch.xpack.core.security.authc.TokenMetaData; import org.elasticsearch.xpack.ilm.IndexLifecycle; @@ -161,6 +164,14 @@ protected DeleteExpiredDataAction.Response deleteExpiredData() throws Exception return response; } + protected PutFilterAction.Response putMlFilter(MlFilter filter) { + return client().execute(PutFilterAction.INSTANCE, new PutFilterAction.Request(filter)).actionGet(); + } + + protected GetFiltersAction.Response getMlFilters() { + return client().execute(GetFiltersAction.INSTANCE, new GetFiltersAction.Request()).actionGet(); + } + @Override protected void ensureClusterStateConsistency() throws IOException { if (cluster() != null && cluster().size() > 0) { diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/process/autodetect/AutodetectProcessManager.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/process/autodetect/AutodetectProcessManager.java index 4845b67bae3f2..205a4292194a7 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/process/autodetect/AutodetectProcessManager.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/process/autodetect/AutodetectProcessManager.java @@ -357,21 +357,11 @@ public void onFailure(Exception e) { if (updateParams.getFilter() == null) { filterListener.onResponse(null); } else { - GetFiltersAction.Request getFilterRequest = new GetFiltersAction.Request(); - getFilterRequest.setFilterId(updateParams.getFilter().getId()); - executeAsyncWithOrigin(client, ML_ORIGIN, GetFiltersAction.INSTANCE, getFilterRequest, - new ActionListener() { - - @Override - public void onResponse(GetFiltersAction.Response response) { - filterListener.onResponse(response.getFilters().results().get(0)); - } - - @Override - public void onFailure(Exception e) { - handler.accept(e); - } - }); + GetFiltersAction.Request getFilterRequest = new GetFiltersAction.Request(updateParams.getFilter().getId()); + executeAsyncWithOrigin(client, ML_ORIGIN, GetFiltersAction.INSTANCE, getFilterRequest, ActionListener.wrap( + getFilterResponse -> filterListener.onResponse(getFilterResponse.getFilters().results().get(0)), + handler::accept + )); } } diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/filter/RestGetFiltersAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/filter/RestGetFiltersAction.java index 0da9de686029f..267b37a589fed 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/filter/RestGetFiltersAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/filter/RestGetFiltersAction.java @@ -46,17 +46,17 @@ public String getName() { @Override protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) throws IOException { - GetFiltersAction.Request getListRequest = new GetFiltersAction.Request(); + GetFiltersAction.Request request = new GetFiltersAction.Request(); String filterId = restRequest.param(MlFilter.ID.getPreferredName()); if (!Strings.isNullOrEmpty(filterId)) { - getListRequest.setFilterId(filterId); + request.setResourceId(filterId); } if (restRequest.hasParam(PageParams.FROM.getPreferredName()) || restRequest.hasParam(PageParams.SIZE.getPreferredName())) { - getListRequest.setPageParams( + request.setPageParams( new PageParams(restRequest.paramAsInt(PageParams.FROM.getPreferredName(), PageParams.DEFAULT_FROM), restRequest.paramAsInt(PageParams.SIZE.getPreferredName(), PageParams.DEFAULT_SIZE))); } - return channel -> client.execute(GetFiltersAction.INSTANCE, getListRequest, new RestStatusToXContentListener<>(channel)); + return channel -> client.execute(GetFiltersAction.INSTANCE, request, new RestStatusToXContentListener<>(channel)); } } diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/MlFiltersIT.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/MlFiltersIT.java new file mode 100644 index 0000000000000..9cbfbddb851e7 --- /dev/null +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/MlFiltersIT.java @@ -0,0 +1,36 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.ml.integration; + +import org.elasticsearch.xpack.core.ml.action.GetFiltersAction; +import org.elasticsearch.xpack.core.ml.action.PutFilterAction; +import org.elasticsearch.xpack.core.ml.job.config.MlFilter; +import org.elasticsearch.xpack.ml.MlSingleNodeTestCase; +import org.junit.Before; + +import static org.hamcrest.Matchers.equalTo; + +public class MlFiltersIT extends MlSingleNodeTestCase { + + @Before + public void beforeTests() throws Exception { + waitForMlTemplates(); + } + + public void testGetFilters_ShouldReturnUpTo100ByDefault() { + int filtersCount = randomIntBetween(11, 100); + for (int i = 0; i < filtersCount; i++) { + PutFilterAction.Request putFilterRequest = new PutFilterAction.Request( + MlFilter.builder("filter-" + i).setItems("item-" + i).build()); + client().execute(PutFilterAction.INSTANCE, putFilterRequest).actionGet(); + } + + GetFiltersAction.Response filters = client().execute(GetFiltersAction.INSTANCE, new GetFiltersAction.Request()).actionGet(); + assertThat((int) filters.getFilters().count(), equalTo(filtersCount)); + assertThat(filters.getFilters().results().size(), equalTo(filtersCount)); + } +} diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/filter_crud.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/filter_crud.yml index d886ec1fb7ff6..3ff3fe5b34bfd 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/filter_crud.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/filter_crud.yml @@ -129,28 +129,6 @@ setup: description: "This filter has a description" items: ["123", "lmnop"] ---- -"Test invalid param combinations": - - - do: - catch: bad_request - ml.get_filters: - filter_id: "filter-foo" - from: 0 - - - do: - catch: bad_request - ml.get_filters: - filter_id: "filter-foo" - size: 1 - - - do: - catch: bad_request - ml.get_filters: - filter_id: "filter-foo" - from: 0 - size: 1 - --- "Test create filter given invalid filter_id": - do: