Skip to content

Commit 9b29327

Browse files
dimitris-athanasioudimitris-athanasiou
authored
[ML] Add description to ML filters (#31330)
This adds a `description` to ML filters in order to allow users to describe their filters in a human readable form which is also editable (filter updates to be added shortly).
1 parent f7a0caf commit 9b29327

File tree

11 files changed

+75
-37
lines changed

11 files changed

+75
-37
lines changed

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/MlFilter.java

+41-6
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
*/
66
package org.elasticsearch.xpack.core.ml.job.config;
77

8+
import org.elasticsearch.Version;
89
import org.elasticsearch.common.Nullable;
910
import org.elasticsearch.common.ParseField;
1011
import org.elasticsearch.common.io.stream.StreamInput;
@@ -30,6 +31,7 @@ public class MlFilter implements ToXContentObject, Writeable {
3031

3132
public static final ParseField TYPE = new ParseField("type");
3233
public static final ParseField ID = new ParseField("filter_id");
34+
public static final ParseField DESCRIPTION = new ParseField("description");
3335
public static final ParseField ITEMS = new ParseField("items");
3436

3537
// For QueryPage
@@ -43,34 +45,48 @@ private static ObjectParser<Builder, Void> createParser(boolean ignoreUnknownFie
4345

4446
parser.declareString((builder, s) -> {}, TYPE);
4547
parser.declareString(Builder::setId, ID);
48+
parser.declareStringOrNull(Builder::setDescription, DESCRIPTION);
4649
parser.declareStringArray(Builder::setItems, ITEMS);
4750

4851
return parser;
4952
}
5053

5154
private final String id;
55+
private final String description;
5256
private final List<String> items;
5357

54-
public MlFilter(String id, List<String> items) {
58+
public MlFilter(String id, String description, List<String> items) {
5559
this.id = Objects.requireNonNull(id, ID.getPreferredName() + " must not be null");
60+
this.description = description;
5661
this.items = Objects.requireNonNull(items, ITEMS.getPreferredName() + " must not be null");
5762
}
5863

5964
public MlFilter(StreamInput in) throws IOException {
6065
id = in.readString();
66+
if (in.getVersion().onOrAfter(Version.V_6_4_0)) {
67+
description = in.readOptionalString();
68+
} else {
69+
description = null;
70+
}
6171
items = Arrays.asList(in.readStringArray());
6272
}
6373

6474
@Override
6575
public void writeTo(StreamOutput out) throws IOException {
6676
out.writeString(id);
77+
if (out.getVersion().onOrAfter(Version.V_6_4_0)) {
78+
out.writeOptionalString(description);
79+
}
6780
out.writeStringArray(items.toArray(new String[items.size()]));
6881
}
6982

7083
@Override
7184
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
7285
builder.startObject();
7386
builder.field(ID.getPreferredName(), id);
87+
if (description != null) {
88+
builder.field(DESCRIPTION.getPreferredName(), description);
89+
}
7490
builder.field(ITEMS.getPreferredName(), items);
7591
if (params.paramAsBoolean(MlMetaIndex.INCLUDE_TYPE_KEY, false)) {
7692
builder.field(TYPE.getPreferredName(), FILTER_TYPE);
@@ -83,6 +99,10 @@ public String getId() {
8399
return id;
84100
}
85101

102+
public String getDescription() {
103+
return description;
104+
}
105+
86106
public List<String> getItems() {
87107
return new ArrayList<>(items);
88108
}
@@ -98,12 +118,12 @@ public boolean equals(Object obj) {
98118
}
99119

100120
MlFilter other = (MlFilter) obj;
101-
return id.equals(other.id) && items.equals(other.items);
121+
return id.equals(other.id) && Objects.equals(description, other.description) && items.equals(other.items);
102122
}
103123

104124
@Override
105125
public int hashCode() {
106-
return Objects.hash(id, items);
126+
return Objects.hash(id, description, items);
107127
}
108128

109129
public String documentId() {
@@ -114,30 +134,45 @@ public static String documentId(String filterId) {
114134
return DOCUMENT_ID_PREFIX + filterId;
115135
}
116136

137+
public static Builder builder(String filterId) {
138+
return new Builder().setId(filterId);
139+
}
140+
117141
public static class Builder {
118142

119143
private String id;
144+
private String description;
120145
private List<String> items = Collections.emptyList();
121146

147+
private Builder() {}
148+
122149
public Builder setId(String id) {
123150
this.id = id;
124151
return this;
125152
}
126153

127-
private Builder() {}
128-
129154
@Nullable
130155
public String getId() {
131156
return id;
132157
}
133158

159+
public Builder setDescription(String description) {
160+
this.description = description;
161+
return this;
162+
}
163+
134164
public Builder setItems(List<String> items) {
135165
this.items = items;
136166
return this;
137167
}
138168

169+
public Builder setItems(String... items) {
170+
this.items = Arrays.asList(items);
171+
return this;
172+
}
173+
139174
public MlFilter build() {
140-
return new MlFilter(id, items);
175+
return new MlFilter(id, description, items);
141176
}
142177
}
143178
}

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/GetFiltersActionResponseTests.java

+2-3
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import org.elasticsearch.xpack.core.ml.action.GetFiltersAction.Response;
1010
import org.elasticsearch.xpack.core.ml.action.util.QueryPage;
1111
import org.elasticsearch.xpack.core.ml.job.config.MlFilter;
12+
import org.elasticsearch.xpack.core.ml.job.config.MlFilterTests;
1213

1314
import java.util.Collections;
1415

@@ -17,9 +18,7 @@ public class GetFiltersActionResponseTests extends AbstractStreamableTestCase<Ge
1718
@Override
1819
protected Response createTestInstance() {
1920
final QueryPage<MlFilter> result;
20-
21-
MlFilter doc = new MlFilter(
22-
randomAlphaOfLengthBetween(1, 20), Collections.singletonList(randomAlphaOfLengthBetween(1, 20)));
21+
MlFilter doc = MlFilterTests.createRandom();
2322
result = new QueryPage<>(Collections.singletonList(doc), 1, MlFilter.RESULTS_FIELD);
2423
return new Response(result);
2524
}

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/PutFilterActionRequestTests.java

+2-12
Original file line numberDiff line numberDiff line change
@@ -8,24 +8,15 @@
88
import org.elasticsearch.common.xcontent.XContentParser;
99
import org.elasticsearch.test.AbstractStreamableXContentTestCase;
1010
import org.elasticsearch.xpack.core.ml.action.PutFilterAction.Request;
11-
import org.elasticsearch.xpack.core.ml.job.config.MlFilter;
12-
13-
import java.util.ArrayList;
14-
import java.util.List;
11+
import org.elasticsearch.xpack.core.ml.job.config.MlFilterTests;
1512

1613
public class PutFilterActionRequestTests extends AbstractStreamableXContentTestCase<Request> {
1714

1815
private final String filterId = randomAlphaOfLengthBetween(1, 20);
1916

2017
@Override
2118
protected Request createTestInstance() {
22-
int size = randomInt(10);
23-
List<String> items = new ArrayList<>(size);
24-
for (int i = 0; i < size; i++) {
25-
items.add(randomAlphaOfLengthBetween(1, 20));
26-
}
27-
MlFilter filter = new MlFilter(filterId, items);
28-
return new PutFilterAction.Request(filter);
19+
return new PutFilterAction.Request(MlFilterTests.createRandom(filterId));
2920
}
3021

3122
@Override
@@ -42,5 +33,4 @@ protected Request createBlankInstance() {
4233
protected Request doParseInstance(XContentParser parser) {
4334
return PutFilterAction.Request.parseRequest(filterId, parser);
4435
}
45-
4636
}

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/MlFilterTests.java

+16-3
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,25 @@ public static MlFilter createTestFilter() {
2626

2727
@Override
2828
protected MlFilter createTestInstance() {
29+
return createRandom();
30+
}
31+
32+
public static MlFilter createRandom() {
33+
return createRandom(randomAlphaOfLengthBetween(1, 20));
34+
}
35+
36+
public static MlFilter createRandom(String filterId) {
37+
String description = null;
38+
if (randomBoolean()) {
39+
description = randomAlphaOfLength(20);
40+
}
41+
2942
int size = randomInt(10);
3043
List<String> items = new ArrayList<>(size);
3144
for (int i = 0; i < size; i++) {
3245
items.add(randomAlphaOfLengthBetween(1, 20));
3346
}
34-
return new MlFilter(randomAlphaOfLengthBetween(1, 20), items);
47+
return new MlFilter(filterId, description, items);
3548
}
3649

3750
@Override
@@ -45,13 +58,13 @@ protected MlFilter doParseInstance(XContentParser parser) {
4558
}
4659

4760
public void testNullId() {
48-
NullPointerException ex = expectThrows(NullPointerException.class, () -> new MlFilter(null, Collections.emptyList()));
61+
NullPointerException ex = expectThrows(NullPointerException.class, () -> new MlFilter(null, "", Collections.emptyList()));
4962
assertEquals(MlFilter.ID.getPreferredName() + " must not be null", ex.getMessage());
5063
}
5164

5265
public void testNullItems() {
5366
NullPointerException ex =
54-
expectThrows(NullPointerException.class, () -> new MlFilter(randomAlphaOfLengthBetween(1, 20), null));
67+
expectThrows(NullPointerException.class, () -> new MlFilter(randomAlphaOfLengthBetween(1, 20), "", null));
5568
assertEquals(MlFilter.ITEMS.getPreferredName() + " must not be null", ex.getMessage());
5669
}
5770

‎x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/JobProviderIT.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -385,8 +385,8 @@ public void testGetAutodetectParams() throws Exception {
385385
indexScheduledEvents(events);
386386

387387
List<MlFilter> filters = new ArrayList<>();
388-
filters.add(new MlFilter("fruit", Arrays.asList("apple", "pear")));
389-
filters.add(new MlFilter("tea", Arrays.asList("green", "builders")));
388+
filters.add(MlFilter.builder("fruit").setItems("apple", "pear").build());
389+
filters.add(MlFilter.builder("tea").setItems("green", "builders").build());
390390
indexFilters(filters);
391391

392392
DataCounts earliestCounts = DataCountsTests.createTestInstance(jobId);

‎x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/JobManagerTests.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ public void testUpdateProcessOnFilterChanged() {
210210

211211
JobManager jobManager = createJobManager();
212212

213-
MlFilter filter = new MlFilter("foo_filter", Arrays.asList("a", "b"));
213+
MlFilter filter = MlFilter.builder("foo_filter").setItems("a", "b").build();
214214

215215
jobManager.updateProcessOnFilterChanged(filter);
216216

‎x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/writer/ControlMsgToProcessWriterTests.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -207,8 +207,8 @@ public void testWriteUpdateDetectorRulesMessage() throws IOException {
207207
public void testWriteUpdateFiltersMessage() throws IOException {
208208
ControlMsgToProcessWriter writer = new ControlMsgToProcessWriter(lengthEncodedWriter, 2);
209209

210-
MlFilter filter1 = new MlFilter("filter_1", Arrays.asList("a"));
211-
MlFilter filter2 = new MlFilter("filter_2", Arrays.asList("b", "c"));
210+
MlFilter filter1 = MlFilter.builder("filter_1").setItems("a").build();
211+
MlFilter filter2 = MlFilter.builder("filter_2").setItems("b", "c").build();
212212

213213
writer.writeUpdateFiltersMessage(Arrays.asList(filter1, filter2));
214214

‎x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/writer/FieldConfigWriterTests.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -220,8 +220,8 @@ public void testWrite_GivenFilters() throws IOException {
220220
AnalysisConfig.Builder builder = new AnalysisConfig.Builder(Collections.singletonList(d));
221221
analysisConfig = builder.build();
222222

223-
filters.add(new MlFilter("filter_1", Arrays.asList("a", "b")));
224-
filters.add(new MlFilter("filter_2", Arrays.asList("c", "d")));
223+
filters.add(MlFilter.builder("filter_1").setItems("a", "b").build());
224+
filters.add(MlFilter.builder("filter_2").setItems("c", "d").build());
225225
writer = mock(OutputStreamWriter.class);
226226

227227
createFieldConfigWriter().write();

‎x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/writer/MlFilterWriterTests.java

+2-3
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010

1111
import java.io.IOException;
1212
import java.util.ArrayList;
13-
import java.util.Arrays;
1413
import java.util.Collections;
1514
import java.util.List;
1615

@@ -28,8 +27,8 @@ public void testWrite_GivenEmpty() throws IOException {
2827

2928
public void testWrite() throws IOException {
3029
List<MlFilter> filters = new ArrayList<>();
31-
filters.add(new MlFilter("filter_1", Arrays.asList("a", "b")));
32-
filters.add(new MlFilter("filter_2", Arrays.asList("c", "d")));
30+
filters.add(MlFilter.builder("filter_1").setItems("a", "b").build());
31+
filters.add(MlFilter.builder("filter_2").setItems("c", "d").build());
3332

3433
StringBuilder buffer = new StringBuilder();
3534
new MlFilterWriter(filters, buffer).write();

‎x-pack/plugin/src/test/resources/rest-api-spec/test/ml/filter_crud.yml

+2
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ setup:
3232
filter_id: filter-foo2
3333
body: >
3434
{
35+
"description": "This filter has a description",
3536
"items": ["123", "lmnop"]
3637
}
3738
@@ -76,6 +77,7 @@ setup:
7677
- match:
7778
filters.1:
7879
filter_id: "filter-foo2"
80+
description: "This filter has a description"
7981
items: ["123", "lmnop"]
8082

8183
- do:

‎x-pack/qa/ml-native-tests/src/test/java/org/elasticsearch/xpack/ml/integration/DetectionRulesIT.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ public void testCondition() throws Exception {
120120
}
121121

122122
public void testScope() throws Exception {
123-
MlFilter safeIps = new MlFilter("safe_ips", Arrays.asList("111.111.111.111", "222.222.222.222"));
123+
MlFilter safeIps = MlFilter.builder("safe_ips").setItems("111.111.111.111", "222.222.222.222").build();
124124
assertThat(putMlFilter(safeIps), is(true));
125125

126126
DetectionRule rule = new DetectionRule.Builder(RuleScope.builder().include("ip", "safe_ips")).build();
@@ -178,7 +178,7 @@ public void testScope() throws Exception {
178178
assertThat(records.get(0).getOverFieldValue(), equalTo("333.333.333.333"));
179179

180180
// Now let's update the filter
181-
MlFilter updatedFilter = new MlFilter(safeIps.getId(), Collections.singletonList("333.333.333.333"));
181+
MlFilter updatedFilter = MlFilter.builder(safeIps.getId()).setItems("333.333.333.333").build();
182182
assertThat(putMlFilter(updatedFilter), is(true));
183183

184184
// Wait until the notification that the process was updated is indexed
@@ -229,7 +229,7 @@ public void testScope() throws Exception {
229229
public void testScopeAndCondition() throws IOException {
230230
// We have 2 IPs and they're both safe-listed.
231231
List<String> ips = Arrays.asList("111.111.111.111", "222.222.222.222");
232-
MlFilter safeIps = new MlFilter("safe_ips", ips);
232+
MlFilter safeIps = MlFilter.builder("safe_ips").setItems(ips).build();
233233
assertThat(putMlFilter(safeIps), is(true));
234234

235235
// Ignore if ip in safe list AND actual < 10.

0 commit comments

Comments
 (0)