[ML] Put ML filter API response should contain the filter (#31362)

elastic · Jun 15, 2018 · c6a5a6d · c6a5a6d
1 parent dbc9d60
commit c6a5a6d
Show file tree

Hide file tree

Showing 7 changed files with 100 additions and 15 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
@@ -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 filter) {
+            this.filter = filter;
+        }
+
+        @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/.../src/test/java/org/elasticsearch/xpack/core/ml/action/PutCalendarActionResponseTests.java b/.../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<PutCalendarAction.Response> {
+import java.io.IOException;
+
+public class PutCalendarActionResponseTests extends AbstractStreamableXContentTestCase<PutCalendarAction.Response> {
 
     @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/...re/src/test/java/org/elasticsearch/xpack/core/ml/action/PutFilterActionResponseTests.java b/...re/src/test/java/org/elasticsearch/xpack/core/ml/action/PutFilterActionResponseTests.java
@@ -0,0 +1,31 @@
+/*
+ * 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.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;
+
+import java.io.IOException;
+
+public class PutFilterActionResponseTests extends AbstractStreamableXContentTestCase<PutFilterAction.Response> {
+
+    @Override
+    protected PutFilterAction.Response createTestInstance() {
+        return new PutFilterAction.Response(MlFilterTests.createRandom());
+    }
+
+    @Override
+    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());
+    }
+}
diff --git a/...k/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportPutFilterAction.java b/...k/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportPutFilterAction.java
@@ -69,7 +69,7 @@ protected void doExecute(PutFilterAction.Request request, ActionListener<PutFilt
                     @Override
                     public void onResponse(BulkResponse indexResponse) {
                         jobManager.updateProcessOnFilterChanged(filter);
-                        listener.onResponse(new PutFilterAction.Response());
+                        listener.onResponse(new PutFilterAction.Response(filter));
                     }
 
                     @Override

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
@@ -115,10 +115,13 @@ setup:
         filter_id: filter-foo2
         body:  >
           {
+            "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/...l-native-tests/src/test/java/org/elasticsearch/xpack/ml/integration/DetectionRulesIT.java b/...l-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<String> 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/...src/test/java/org/elasticsearch/xpack/ml/integration/MlNativeAutodetectIntegTestCase.java b/...src/test/java/org/elasticsearch/xpack/ml/integration/MlNativeAutodetectIntegTestCase.java
@@ -419,9 +419,8 @@ protected List<Forecast> 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<String> jobIds, String description) {