From d8738792c7139dba3826f09d5196f6a731119816 Mon Sep 17 00:00:00 2001 From: Dimitris Athanasiou Date: Fri, 15 Jun 2018 16:27:43 +0100 Subject: [PATCH 1/2] [ML] Put ML filter API response should contain the filter --- .../xpack/core/ml/action/PutFilterAction.java | 51 +++++++++++++++++-- .../action/PutFilterActionResponseTests.java | 22 ++++++++ .../ml/action/TransportPutFilterAction.java | 2 +- .../rest-api-spec/test/ml/filter_crud.yml | 6 ++- .../ml/integration/DetectionRulesIT.java | 7 ++- .../MlNativeAutodetectIntegTestCase.java | 5 +- 6 files changed, 80 insertions(+), 13 deletions(-) create mode 100644 x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/PutFilterActionResponseTests.java diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/PutFilterAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/PutFilterAction.java index 2f7606795f001..278a0af6083d4 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/PutFilterAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/PutFilterAction.java @@ -9,7 +9,7 @@ import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionRequestBuilder; import org.elasticsearch.action.ActionRequestValidationException; -import org.elasticsearch.action.support.master.AcknowledgedResponse; +import org.elasticsearch.action.ActionResponse; import org.elasticsearch.client.ElasticsearchClient; import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; @@ -115,10 +115,53 @@ public RequestBuilder(ElasticsearchClient client) { } } - public static class Response extends AcknowledgedResponse { + public static class Response extends ActionResponse implements ToXContentObject { - public Response() { - super(true); + private MlFilter filter; + + Response() { + } + + public Response(MlFilter calendar) { + this.filter = calendar; + } + + @Override + public void readFrom(StreamInput in) throws IOException { + super.readFrom(in); + filter = new MlFilter(in); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + super.writeTo(out); + filter.writeTo(out); + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + return filter.toXContent(builder, params); + } + + public MlFilter getFilter() { + return filter; + } + + @Override + public int hashCode() { + return Objects.hash(filter); + } + + @Override + public boolean equals(Object obj) { + if (obj == null) { + return false; + } + if (getClass() != obj.getClass()) { + return false; + } + Response other = (Response) obj; + return Objects.equals(filter, other.filter); } } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/PutFilterActionResponseTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/PutFilterActionResponseTests.java new file mode 100644 index 0000000000000..9030890cf4021 --- /dev/null +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/PutFilterActionResponseTests.java @@ -0,0 +1,22 @@ +/* + * 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.core.ml.action; + +import org.elasticsearch.test.AbstractStreamableTestCase; +import org.elasticsearch.xpack.core.ml.job.config.MlFilterTests; + +public class PutFilterActionResponseTests extends AbstractStreamableTestCase { + + @Override + protected PutFilterAction.Response createTestInstance() { + return new PutFilterAction.Response(MlFilterTests.createRandom()); + } + + @Override + protected PutFilterAction.Response createBlankInstance() { + return new PutFilterAction.Response(); + } +} diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportPutFilterAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportPutFilterAction.java index f7ac11e2d1aec..fc14ef085dd33 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportPutFilterAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportPutFilterAction.java @@ -69,7 +69,7 @@ protected void doExecute(PutFilterAction.Request request, ActionListener { + "description": "A newly created filter", "items": ["abc", "xyz"] } - - match: { acknowledged: true } + - match: { filter_id: filter-foo2 } + - match: { description: "A newly created filter" } + - match: { items: ["abc", "xyz"]} - do: xpack.ml.get_filters: @@ -128,6 +131,7 @@ setup: - match: filters.0: filter_id: "filter-foo2" + description: "A newly created filter" items: ["abc", "xyz"] --- diff --git a/x-pack/qa/ml-native-tests/src/test/java/org/elasticsearch/xpack/ml/integration/DetectionRulesIT.java b/x-pack/qa/ml-native-tests/src/test/java/org/elasticsearch/xpack/ml/integration/DetectionRulesIT.java index b99170546df3b..fbda8ad716b2c 100644 --- a/x-pack/qa/ml-native-tests/src/test/java/org/elasticsearch/xpack/ml/integration/DetectionRulesIT.java +++ b/x-pack/qa/ml-native-tests/src/test/java/org/elasticsearch/xpack/ml/integration/DetectionRulesIT.java @@ -35,7 +35,6 @@ import java.util.Set; import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.isOneOf; /** @@ -121,7 +120,7 @@ public void testCondition() throws Exception { public void testScope() throws Exception { MlFilter safeIps = MlFilter.builder("safe_ips").setItems("111.111.111.111", "222.222.222.222").build(); - assertThat(putMlFilter(safeIps), is(true)); + assertThat(putMlFilter(safeIps).getFilter(), equalTo(safeIps)); DetectionRule rule = new DetectionRule.Builder(RuleScope.builder().include("ip", "safe_ips")).build(); @@ -179,7 +178,7 @@ public void testScope() throws Exception { // Now let's update the filter MlFilter updatedFilter = MlFilter.builder(safeIps.getId()).setItems("333.333.333.333").build(); - assertThat(putMlFilter(updatedFilter), is(true)); + assertThat(putMlFilter(updatedFilter).getFilter(), equalTo(updatedFilter)); // Wait until the notification that the process was updated is indexed assertBusy(() -> { @@ -230,7 +229,7 @@ public void testScopeAndCondition() throws IOException { // We have 2 IPs and they're both safe-listed. List ips = Arrays.asList("111.111.111.111", "222.222.222.222"); MlFilter safeIps = MlFilter.builder("safe_ips").setItems(ips).build(); - assertThat(putMlFilter(safeIps), is(true)); + assertThat(putMlFilter(safeIps).getFilter(), equalTo(safeIps)); // Ignore if ip in safe list AND actual < 10. DetectionRule rule = new DetectionRule.Builder(RuleScope.builder().include("ip", "safe_ips")) diff --git a/x-pack/qa/ml-native-tests/src/test/java/org/elasticsearch/xpack/ml/integration/MlNativeAutodetectIntegTestCase.java b/x-pack/qa/ml-native-tests/src/test/java/org/elasticsearch/xpack/ml/integration/MlNativeAutodetectIntegTestCase.java index 9057db476ad77..4e6fb03497e6a 100644 --- a/x-pack/qa/ml-native-tests/src/test/java/org/elasticsearch/xpack/ml/integration/MlNativeAutodetectIntegTestCase.java +++ b/x-pack/qa/ml-native-tests/src/test/java/org/elasticsearch/xpack/ml/integration/MlNativeAutodetectIntegTestCase.java @@ -419,9 +419,8 @@ protected List getForecasts(String jobId, ForecastRequestStats forecas return forecasts; } - protected boolean putMlFilter(MlFilter filter) { - PutFilterAction.Response response = client().execute(PutFilterAction.INSTANCE, new PutFilterAction.Request(filter)).actionGet(); - return response.isAcknowledged(); + 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) { From 8c9ba845771c856a8d1fb500fafeb6c07bee6d5c Mon Sep 17 00:00:00 2001 From: Dimitris Athanasiou Date: Fri, 15 Jun 2018 17:18:58 +0100 Subject: [PATCH 2/2] Address review feedback --- .../xpack/core/ml/action/PutFilterAction.java | 4 ++-- .../ml/action/PutCalendarActionResponseTests.java | 13 +++++++++++-- .../ml/action/PutFilterActionResponseTests.java | 13 +++++++++++-- 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/PutFilterAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/PutFilterAction.java index 278a0af6083d4..8269a105b6463 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/PutFilterAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/PutFilterAction.java @@ -122,8 +122,8 @@ public static class Response extends ActionResponse implements ToXContentObject Response() { } - public Response(MlFilter calendar) { - this.filter = calendar; + public Response(MlFilter filter) { + this.filter = filter; } @Override diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/PutCalendarActionResponseTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/PutCalendarActionResponseTests.java index 941de884554bf..77d4d788db620 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/PutCalendarActionResponseTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/PutCalendarActionResponseTests.java @@ -5,10 +5,14 @@ */ package org.elasticsearch.xpack.core.ml.action; -import org.elasticsearch.test.AbstractStreamableTestCase; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.test.AbstractStreamableXContentTestCase; +import org.elasticsearch.xpack.core.ml.calendars.Calendar; import org.elasticsearch.xpack.core.ml.calendars.CalendarTests; -public class PutCalendarActionResponseTests extends AbstractStreamableTestCase { +import java.io.IOException; + +public class PutCalendarActionResponseTests extends AbstractStreamableXContentTestCase { @Override protected PutCalendarAction.Response createTestInstance() { @@ -19,4 +23,9 @@ protected PutCalendarAction.Response createTestInstance() { protected PutCalendarAction.Response createBlankInstance() { return new PutCalendarAction.Response(); } + + @Override + protected PutCalendarAction.Response doParseInstance(XContentParser parser) throws IOException { + return new PutCalendarAction.Response(Calendar.LENIENT_PARSER.parse(parser, null).build()); + } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/PutFilterActionResponseTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/PutFilterActionResponseTests.java index 9030890cf4021..1e697f5172a4a 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/PutFilterActionResponseTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/PutFilterActionResponseTests.java @@ -5,10 +5,14 @@ */ package org.elasticsearch.xpack.core.ml.action; -import org.elasticsearch.test.AbstractStreamableTestCase; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.test.AbstractStreamableXContentTestCase; +import org.elasticsearch.xpack.core.ml.job.config.MlFilter; import org.elasticsearch.xpack.core.ml.job.config.MlFilterTests; -public class PutFilterActionResponseTests extends AbstractStreamableTestCase { +import java.io.IOException; + +public class PutFilterActionResponseTests extends AbstractStreamableXContentTestCase { @Override protected PutFilterAction.Response createTestInstance() { @@ -19,4 +23,9 @@ protected PutFilterAction.Response createTestInstance() { protected PutFilterAction.Response createBlankInstance() { return new PutFilterAction.Response(); } + + @Override + protected PutFilterAction.Response doParseInstance(XContentParser parser) throws IOException { + return new PutFilterAction.Response(MlFilter.LENIENT_PARSER.parse(parser, null).build()); + } }