Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ML] Add description to ML filters #31330

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/
package org.elasticsearch.xpack.core.ml.job.config;

import org.elasticsearch.Version;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.io.stream.StreamInput;
Expand All @@ -30,6 +31,7 @@ public class MlFilter implements ToXContentObject, Writeable {

public static final ParseField TYPE = new ParseField("type");
public static final ParseField ID = new ParseField("filter_id");
public static final ParseField DESCRIPTION = new ParseField("description");
public static final ParseField ITEMS = new ParseField("items");

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

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

return parser;
}

private final String id;
private final String description;
private final List<String> items;

public MlFilter(String id, List<String> items) {
public MlFilter(String id, String description, List<String> items) {
this.id = Objects.requireNonNull(id, ID.getPreferredName() + " must not be null");
this.description = description;
this.items = Objects.requireNonNull(items, ITEMS.getPreferredName() + " must not be null");
}

public MlFilter(StreamInput in) throws IOException {
id = in.readString();
if (in.getVersion().onOrAfter(Version.V_6_4_0)) {
description = in.readOptionalString();
} else {
description = null;
}
items = Arrays.asList(in.readStringArray());
}

@Override
public void writeTo(StreamOutput out) throws IOException {
out.writeString(id);
if (out.getVersion().onOrAfter(Version.V_6_4_0)) {
out.writeOptionalString(description);
}
out.writeStringArray(items.toArray(new String[items.size()]));
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();
builder.field(ID.getPreferredName(), id);
if (description != null) {
builder.field(DESCRIPTION.getPreferredName(), description);
}
builder.field(ITEMS.getPreferredName(), items);
if (params.paramAsBoolean(MlMetaIndex.INCLUDE_TYPE_KEY, false)) {
builder.field(TYPE.getPreferredName(), FILTER_TYPE);
Expand All @@ -83,6 +99,10 @@ public String getId() {
return id;
}

public String getDescription() {
return description;
}

public List<String> getItems() {
return new ArrayList<>(items);
}
Expand All @@ -98,12 +118,12 @@ public boolean equals(Object obj) {
}

MlFilter other = (MlFilter) obj;
return id.equals(other.id) && items.equals(other.items);
return id.equals(other.id) && Objects.equals(description, other.description) && items.equals(other.items);
}

@Override
public int hashCode() {
return Objects.hash(id, items);
return Objects.hash(id, description, items);
}

public String documentId() {
Expand All @@ -114,30 +134,45 @@ public static String documentId(String filterId) {
return DOCUMENT_ID_PREFIX + filterId;
}

public static Builder builder(String filterId) {
return new Builder().setId(filterId);
}

public static class Builder {

private String id;
private String description;
private List<String> items = Collections.emptyList();

private Builder() {}

public Builder setId(String id) {
this.id = id;
return this;
}

private Builder() {}

@Nullable
public String getId() {
return id;
}

public Builder setDescription(String description) {
this.description = description;
return this;
}

public Builder setItems(List<String> items) {
this.items = items;
return this;
}

public Builder setItems(String... items) {
this.items = Arrays.asList(items);
return this;
}

public MlFilter build() {
return new MlFilter(id, items);
return new MlFilter(id, description, items);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import org.elasticsearch.xpack.core.ml.action.GetFiltersAction.Response;
import org.elasticsearch.xpack.core.ml.action.util.QueryPage;
import org.elasticsearch.xpack.core.ml.job.config.MlFilter;
import org.elasticsearch.xpack.core.ml.job.config.MlFilterTests;

import java.util.Collections;

Expand All @@ -17,9 +18,7 @@ public class GetFiltersActionResponseTests extends AbstractStreamableTestCase<Ge
@Override
protected Response createTestInstance() {
final QueryPage<MlFilter> result;

MlFilter doc = new MlFilter(
randomAlphaOfLengthBetween(1, 20), Collections.singletonList(randomAlphaOfLengthBetween(1, 20)));
MlFilter doc = MlFilterTests.createRandom();
result = new QueryPage<>(Collections.singletonList(doc), 1, MlFilter.RESULTS_FIELD);
return new Response(result);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,15 @@
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.test.AbstractStreamableXContentTestCase;
import org.elasticsearch.xpack.core.ml.action.PutFilterAction.Request;
import org.elasticsearch.xpack.core.ml.job.config.MlFilter;

import java.util.ArrayList;
import java.util.List;
import org.elasticsearch.xpack.core.ml.job.config.MlFilterTests;

public class PutFilterActionRequestTests extends AbstractStreamableXContentTestCase<Request> {

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

@Override
protected Request createTestInstance() {
int size = randomInt(10);
List<String> items = new ArrayList<>(size);
for (int i = 0; i < size; i++) {
items.add(randomAlphaOfLengthBetween(1, 20));
}
MlFilter filter = new MlFilter(filterId, items);
return new PutFilterAction.Request(filter);
return new PutFilterAction.Request(MlFilterTests.createRandom(filterId));
}

@Override
Expand All @@ -42,5 +33,4 @@ protected Request createBlankInstance() {
protected Request doParseInstance(XContentParser parser) {
return PutFilterAction.Request.parseRequest(filterId, parser);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,25 @@ public static MlFilter createTestFilter() {

@Override
protected MlFilter createTestInstance() {
return createRandom();
}

public static MlFilter createRandom() {
return createRandom(randomAlphaOfLengthBetween(1, 20));
}

public static MlFilter createRandom(String filterId) {
String description = null;
if (randomBoolean()) {
description = randomAlphaOfLength(20);
}

int size = randomInt(10);
List<String> items = new ArrayList<>(size);
for (int i = 0; i < size; i++) {
items.add(randomAlphaOfLengthBetween(1, 20));
}
return new MlFilter(randomAlphaOfLengthBetween(1, 20), items);
return new MlFilter(filterId, description, items);
}

@Override
Expand All @@ -45,13 +58,13 @@ protected MlFilter doParseInstance(XContentParser parser) {
}

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

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -385,8 +385,8 @@ public void testGetAutodetectParams() throws Exception {
indexScheduledEvents(events);

List<MlFilter> filters = new ArrayList<>();
filters.add(new MlFilter("fruit", Arrays.asList("apple", "pear")));
filters.add(new MlFilter("tea", Arrays.asList("green", "builders")));
filters.add(MlFilter.builder("fruit").setItems("apple", "pear").build());
filters.add(MlFilter.builder("tea").setItems("green", "builders").build());
indexFilters(filters);

DataCounts earliestCounts = DataCountsTests.createTestInstance(jobId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ public void testUpdateProcessOnFilterChanged() {

JobManager jobManager = createJobManager();

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

jobManager.updateProcessOnFilterChanged(filter);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,8 +207,8 @@ public void testWriteUpdateDetectorRulesMessage() throws IOException {
public void testWriteUpdateFiltersMessage() throws IOException {
ControlMsgToProcessWriter writer = new ControlMsgToProcessWriter(lengthEncodedWriter, 2);

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

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,8 @@ public void testWrite_GivenFilters() throws IOException {
AnalysisConfig.Builder builder = new AnalysisConfig.Builder(Collections.singletonList(d));
analysisConfig = builder.build();

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

createFieldConfigWriter().write();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;

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

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

StringBuilder buffer = new StringBuilder();
new MlFilterWriter(filters, buffer).write();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ setup:
filter_id: filter-foo2
body: >
{
"description": "This filter has a description",
"items": ["123", "lmnop"]
}

Expand Down Expand Up @@ -76,6 +77,7 @@ setup:
- match:
filters.1:
filter_id: "filter-foo2"
description: "This filter has a description"
items: ["123", "lmnop"]

- do:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ public void testCondition() throws Exception {
}

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

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

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

// Wait until the notification that the process was updated is indexed
Expand Down Expand Up @@ -229,7 +229,7 @@ public void testScope() throws Exception {
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 = new MlFilter("safe_ips", ips);
MlFilter safeIps = MlFilter.builder("safe_ips").setItems(ips).build();
assertThat(putMlFilter(safeIps), is(true));

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