When get filters is called without setting the `size` paramter only up to 10 filters are returned. However, 100 filters should be returned. This commit fixes this and adds an integ test to guard it. It seems this was accidentally broken in #39976. Closes #54206 Backport of #54207
This commit is contained in:
parent
f48e8f31b9
commit
cc981fa377
|
@ -6,7 +6,6 @@
|
||||||
package org.elasticsearch.xpack.core.ml.action;
|
package org.elasticsearch.xpack.core.ml.action;
|
||||||
|
|
||||||
import org.elasticsearch.action.ActionRequestBuilder;
|
import org.elasticsearch.action.ActionRequestBuilder;
|
||||||
import org.elasticsearch.action.ActionRequestValidationException;
|
|
||||||
import org.elasticsearch.action.ActionType;
|
import org.elasticsearch.action.ActionType;
|
||||||
import org.elasticsearch.client.ElasticsearchClient;
|
import org.elasticsearch.client.ElasticsearchClient;
|
||||||
import org.elasticsearch.common.io.stream.StreamInput;
|
import org.elasticsearch.common.io.stream.StreamInput;
|
||||||
|
@ -14,14 +13,11 @@ import org.elasticsearch.common.xcontent.StatusToXContentObject;
|
||||||
import org.elasticsearch.rest.RestStatus;
|
import org.elasticsearch.rest.RestStatus;
|
||||||
import org.elasticsearch.xpack.core.action.AbstractGetResourcesRequest;
|
import org.elasticsearch.xpack.core.action.AbstractGetResourcesRequest;
|
||||||
import org.elasticsearch.xpack.core.action.AbstractGetResourcesResponse;
|
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.action.util.QueryPage;
|
||||||
import org.elasticsearch.xpack.core.ml.job.config.MlFilter;
|
import org.elasticsearch.xpack.core.ml.job.config.MlFilter;
|
||||||
|
|
||||||
import java.io.IOException;
|
import java.io.IOException;
|
||||||
|
|
||||||
import static org.elasticsearch.action.ValidateActions.addValidationError;
|
|
||||||
|
|
||||||
|
|
||||||
public class GetFiltersAction extends ActionType<GetFiltersAction.Response> {
|
public class GetFiltersAction extends ActionType<GetFiltersAction.Response> {
|
||||||
|
|
||||||
|
@ -35,33 +31,18 @@ public class GetFiltersAction extends ActionType<GetFiltersAction.Response> {
|
||||||
public static class Request extends AbstractGetResourcesRequest {
|
public static class Request extends AbstractGetResourcesRequest {
|
||||||
|
|
||||||
public Request() {
|
public Request() {
|
||||||
// Put our own defaults for backwards compatibility
|
setAllowNoResources(true);
|
||||||
super(null, null, true);
|
}
|
||||||
|
|
||||||
|
public Request(String filterId) {
|
||||||
|
setResourceId(filterId);
|
||||||
|
setAllowNoResources(true);
|
||||||
}
|
}
|
||||||
|
|
||||||
public Request(StreamInput in) throws IOException {
|
public Request(StreamInput in) throws IOException {
|
||||||
super(in);
|
super(in);
|
||||||
}
|
}
|
||||||
|
|
||||||
public void setFilterId(String filterId) {
|
|
||||||
setResourceId(filterId);
|
|
||||||
}
|
|
||||||
|
|
||||||
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;
|
|
||||||
}
|
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public String getResourceIdField() {
|
public String getResourceIdField() {
|
||||||
return MlFilter.ID.getPreferredName();
|
return MlFilter.ID.getPreferredName();
|
||||||
|
|
|
@ -7,24 +7,23 @@ package org.elasticsearch.xpack.core.ml.action;
|
||||||
|
|
||||||
import org.elasticsearch.common.io.stream.Writeable;
|
import org.elasticsearch.common.io.stream.Writeable;
|
||||||
import org.elasticsearch.test.AbstractWireSerializingTestCase;
|
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.action.util.PageParams;
|
||||||
|
import org.elasticsearch.xpack.core.ml.action.GetFiltersAction.Request;
|
||||||
|
|
||||||
public class GetFiltersActionRequestTests extends AbstractWireSerializingTestCase<Request> {
|
public class GetFiltersActionRequestTests extends AbstractWireSerializingTestCase<Request> {
|
||||||
|
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
protected Request createTestInstance() {
|
protected Request createTestInstance() {
|
||||||
Request request = new Request();
|
|
||||||
if (randomBoolean()) {
|
if (randomBoolean()) {
|
||||||
request.setFilterId(randomAlphaOfLengthBetween(1, 20));
|
return new Request(randomAlphaOfLength(10));
|
||||||
} else {
|
}
|
||||||
|
Request request = new Request();
|
||||||
if (randomBoolean()) {
|
if (randomBoolean()) {
|
||||||
int from = randomInt(10000);
|
int from = randomInt(10000);
|
||||||
int size = randomInt(10000);
|
int size = randomInt(10000);
|
||||||
request.setPageParams(new PageParams(from, size));
|
request.setPageParams(new PageParams(from, size));
|
||||||
}
|
}
|
||||||
}
|
|
||||||
return request;
|
return request;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -127,7 +127,6 @@ integTest.runner {
|
||||||
'ml/filter_crud/Test create filter api with mismatching body ID',
|
'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 create filter given invalid filter_id',
|
||||||
'ml/filter_crud/Test get filter API with bad 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 non-existing filter',
|
||||||
'ml/filter_crud/Test update filter given remove item is not present',
|
'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',
|
'ml/filter_crud/Test get all filter given index exists but no mapping for filter_id',
|
||||||
|
|
|
@ -39,7 +39,6 @@ import org.elasticsearch.xpack.core.ml.action.PostCalendarEventsAction;
|
||||||
import org.elasticsearch.xpack.core.ml.action.PostDataAction;
|
import org.elasticsearch.xpack.core.ml.action.PostDataAction;
|
||||||
import org.elasticsearch.xpack.core.ml.action.PutCalendarAction;
|
import org.elasticsearch.xpack.core.ml.action.PutCalendarAction;
|
||||||
import org.elasticsearch.xpack.core.ml.action.PutDatafeedAction;
|
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.PutJobAction;
|
||||||
import org.elasticsearch.xpack.core.ml.action.RevertModelSnapshotAction;
|
import org.elasticsearch.xpack.core.ml.action.RevertModelSnapshotAction;
|
||||||
import org.elasticsearch.xpack.core.ml.action.StartDatafeedAction;
|
import org.elasticsearch.xpack.core.ml.action.StartDatafeedAction;
|
||||||
|
@ -53,7 +52,6 @@ import org.elasticsearch.xpack.core.ml.datafeed.DatafeedUpdate;
|
||||||
import org.elasticsearch.xpack.core.ml.job.config.Job;
|
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.JobState;
|
||||||
import org.elasticsearch.xpack.core.ml.job.config.JobUpdate;
|
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.persistence.AnomalyDetectorsIndex;
|
||||||
import org.elasticsearch.xpack.core.ml.job.process.autodetect.state.DataCounts;
|
import org.elasticsearch.xpack.core.ml.job.process.autodetect.state.DataCounts;
|
||||||
import org.elasticsearch.xpack.core.ml.job.process.autodetect.state.ModelSnapshot;
|
import org.elasticsearch.xpack.core.ml.job.process.autodetect.state.ModelSnapshot;
|
||||||
|
@ -353,10 +351,6 @@ abstract class MlNativeAutodetectIntegTestCase extends MlNativeIntegTestCase {
|
||||||
return forecasts;
|
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<String> jobIds, String description) {
|
protected PutCalendarAction.Response putCalendar(String calendarId, List<String> jobIds, String description) {
|
||||||
PutCalendarAction.Request request = new PutCalendarAction.Request(new Calendar(calendarId, jobIds, description));
|
PutCalendarAction.Request request = new PutCalendarAction.Request(new Calendar(calendarId, jobIds, description));
|
||||||
return client().execute(PutCalendarAction.INSTANCE, request).actionGet();
|
return client().execute(PutCalendarAction.INSTANCE, request).actionGet();
|
||||||
|
|
|
@ -31,12 +31,15 @@ import org.elasticsearch.xpack.core.ilm.LifecycleSettings;
|
||||||
import org.elasticsearch.xpack.core.ml.MlMetadata;
|
import org.elasticsearch.xpack.core.ml.MlMetadata;
|
||||||
import org.elasticsearch.xpack.core.ml.MlTasks;
|
import org.elasticsearch.xpack.core.ml.MlTasks;
|
||||||
import org.elasticsearch.xpack.core.ml.action.DeleteExpiredDataAction;
|
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.OpenJobAction;
|
||||||
|
import org.elasticsearch.xpack.core.ml.action.PutFilterAction;
|
||||||
import org.elasticsearch.xpack.core.ml.action.StartDataFrameAnalyticsAction;
|
import org.elasticsearch.xpack.core.ml.action.StartDataFrameAnalyticsAction;
|
||||||
import org.elasticsearch.xpack.core.ml.action.StartDatafeedAction;
|
import org.elasticsearch.xpack.core.ml.action.StartDatafeedAction;
|
||||||
import org.elasticsearch.xpack.core.ml.datafeed.DatafeedState;
|
import org.elasticsearch.xpack.core.ml.datafeed.DatafeedState;
|
||||||
import org.elasticsearch.xpack.core.ml.dataframe.DataFrameAnalyticsTaskState;
|
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.JobTaskState;
|
||||||
|
import org.elasticsearch.xpack.core.ml.job.config.MlFilter;
|
||||||
import org.elasticsearch.xpack.core.security.SecurityField;
|
import org.elasticsearch.xpack.core.security.SecurityField;
|
||||||
import org.elasticsearch.xpack.core.security.authc.TokenMetaData;
|
import org.elasticsearch.xpack.core.security.authc.TokenMetaData;
|
||||||
import org.elasticsearch.xpack.ilm.IndexLifecycle;
|
import org.elasticsearch.xpack.ilm.IndexLifecycle;
|
||||||
|
@ -136,6 +139,14 @@ abstract class MlNativeIntegTestCase extends ESIntegTestCase {
|
||||||
return response;
|
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
|
@Override
|
||||||
protected void ensureClusterStateConsistency() throws IOException {
|
protected void ensureClusterStateConsistency() throws IOException {
|
||||||
if (cluster() != null && cluster().size() > 0) {
|
if (cluster() != null && cluster().size() > 0) {
|
||||||
|
|
|
@ -357,21 +357,11 @@ public class AutodetectProcessManager implements ClusterStateListener {
|
||||||
if (updateParams.getFilter() == null) {
|
if (updateParams.getFilter() == null) {
|
||||||
filterListener.onResponse(null);
|
filterListener.onResponse(null);
|
||||||
} else {
|
} else {
|
||||||
GetFiltersAction.Request getFilterRequest = new GetFiltersAction.Request();
|
GetFiltersAction.Request getFilterRequest = new GetFiltersAction.Request(updateParams.getFilter().getId());
|
||||||
getFilterRequest.setFilterId(updateParams.getFilter().getId());
|
executeAsyncWithOrigin(client, ML_ORIGIN, GetFiltersAction.INSTANCE, getFilterRequest, ActionListener.wrap(
|
||||||
executeAsyncWithOrigin(client, ML_ORIGIN, GetFiltersAction.INSTANCE, getFilterRequest,
|
getFilterResponse -> filterListener.onResponse(getFilterResponse.getFilters().results().get(0)),
|
||||||
new ActionListener<GetFiltersAction.Response>() {
|
handler::accept
|
||||||
|
));
|
||||||
@Override
|
|
||||||
public void onResponse(GetFiltersAction.Response response) {
|
|
||||||
filterListener.onResponse(response.getFilters().results().get(0));
|
|
||||||
}
|
|
||||||
|
|
||||||
@Override
|
|
||||||
public void onFailure(Exception e) {
|
|
||||||
handler.accept(e);
|
|
||||||
}
|
|
||||||
});
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -47,17 +47,17 @@ public class RestGetFiltersAction extends BaseRestHandler {
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) throws IOException {
|
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());
|
String filterId = restRequest.param(MlFilter.ID.getPreferredName());
|
||||||
if (!Strings.isNullOrEmpty(filterId)) {
|
if (!Strings.isNullOrEmpty(filterId)) {
|
||||||
getListRequest.setFilterId(filterId);
|
request.setResourceId(filterId);
|
||||||
}
|
}
|
||||||
if (restRequest.hasParam(PageParams.FROM.getPreferredName()) || restRequest.hasParam(PageParams.SIZE.getPreferredName())) {
|
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),
|
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, request, new RestStatusToXContentListener<>(channel));
|
||||||
}
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -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));
|
||||||
|
}
|
||||||
|
}
|
|
@ -129,28 +129,6 @@ setup:
|
||||||
description: "This filter has a description"
|
description: "This filter has a description"
|
||||||
items: ["123", "lmnop"]
|
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":
|
"Test create filter given invalid filter_id":
|
||||||
- do:
|
- do:
|
||||||
|
|
Loading…
Reference in New Issue