[ML] Hold ML filter items in sorted set (#31338)
Filter items should be unique. They should also be sorted to make them easier to read plus save sorting in the autodetect process.
This commit is contained in:
parent
fec7860edc
commit
da5bfda5f3
|
@ -17,11 +17,12 @@ import org.elasticsearch.common.xcontent.XContentBuilder;
|
|||
import org.elasticsearch.xpack.core.ml.MlMetaIndex;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Arrays;
|
||||
import java.util.Collections;
|
||||
import java.util.List;
|
||||
import java.util.Objects;
|
||||
import java.util.SortedSet;
|
||||
import java.util.TreeSet;
|
||||
|
||||
public class MlFilter implements ToXContentObject, Writeable {
|
||||
|
||||
|
@ -53,9 +54,9 @@ public class MlFilter implements ToXContentObject, Writeable {
|
|||
|
||||
private final String id;
|
||||
private final String description;
|
||||
private final List<String> items;
|
||||
private final SortedSet<String> items;
|
||||
|
||||
public MlFilter(String id, String description, List<String> items) {
|
||||
public MlFilter(String id, String description, SortedSet<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");
|
||||
|
@ -68,7 +69,8 @@ public class MlFilter implements ToXContentObject, Writeable {
|
|||
} else {
|
||||
description = null;
|
||||
}
|
||||
items = Arrays.asList(in.readStringArray());
|
||||
items = new TreeSet<>();
|
||||
items.addAll(Arrays.asList(in.readStringArray()));
|
||||
}
|
||||
|
||||
@Override
|
||||
|
@ -103,8 +105,8 @@ public class MlFilter implements ToXContentObject, Writeable {
|
|||
return description;
|
||||
}
|
||||
|
||||
public List<String> getItems() {
|
||||
return new ArrayList<>(items);
|
||||
public SortedSet<String> getItems() {
|
||||
return Collections.unmodifiableSortedSet(items);
|
||||
}
|
||||
|
||||
@Override
|
||||
|
@ -142,7 +144,7 @@ public class MlFilter implements ToXContentObject, Writeable {
|
|||
|
||||
private String id;
|
||||
private String description;
|
||||
private List<String> items = Collections.emptyList();
|
||||
private SortedSet<String> items = new TreeSet<>();
|
||||
|
||||
private Builder() {}
|
||||
|
||||
|
@ -162,12 +164,13 @@ public class MlFilter implements ToXContentObject, Writeable {
|
|||
}
|
||||
|
||||
public Builder setItems(List<String> items) {
|
||||
this.items = items;
|
||||
this.items = new TreeSet<>();
|
||||
this.items.addAll(items);
|
||||
return this;
|
||||
}
|
||||
|
||||
public Builder setItems(String... items) {
|
||||
this.items = Arrays.asList(items);
|
||||
setItems(Arrays.asList(items));
|
||||
return this;
|
||||
}
|
||||
|
||||
|
|
|
@ -11,10 +11,9 @@ import org.elasticsearch.common.xcontent.json.JsonXContent;
|
|||
import org.elasticsearch.test.AbstractSerializingTestCase;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collections;
|
||||
import java.util.List;
|
||||
import java.util.TreeSet;
|
||||
|
||||
import static org.hamcrest.Matchers.contains;
|
||||
import static org.hamcrest.Matchers.containsString;
|
||||
import static org.hamcrest.Matchers.equalTo;
|
||||
|
||||
|
@ -40,7 +39,7 @@ public class MlFilterTests extends AbstractSerializingTestCase<MlFilter> {
|
|||
}
|
||||
|
||||
int size = randomInt(10);
|
||||
List<String> items = new ArrayList<>(size);
|
||||
TreeSet<String> items = new TreeSet<>();
|
||||
for (int i = 0; i < size; i++) {
|
||||
items.add(randomAlphaOfLengthBetween(1, 20));
|
||||
}
|
||||
|
@ -58,7 +57,7 @@ public class MlFilterTests extends AbstractSerializingTestCase<MlFilter> {
|
|||
}
|
||||
|
||||
public void testNullId() {
|
||||
NullPointerException ex = expectThrows(NullPointerException.class, () -> new MlFilter(null, "", Collections.emptyList()));
|
||||
NullPointerException ex = expectThrows(NullPointerException.class, () -> new MlFilter(null, "", new TreeSet<>()));
|
||||
assertEquals(MlFilter.ID.getPreferredName() + " must not be null", ex.getMessage());
|
||||
}
|
||||
|
||||
|
@ -88,4 +87,14 @@ public class MlFilterTests extends AbstractSerializingTestCase<MlFilter> {
|
|||
MlFilter.LENIENT_PARSER.apply(parser, null);
|
||||
}
|
||||
}
|
||||
|
||||
public void testItemsAreSorted() {
|
||||
MlFilter filter = MlFilter.builder("foo").setItems("c", "b", "a").build();
|
||||
assertThat(filter.getItems(), contains("a", "b", "c"));
|
||||
}
|
||||
|
||||
public void testGetItemsReturnsUnmodifiable() {
|
||||
MlFilter filter = MlFilter.builder("foo").setItems("c", "b", "a").build();
|
||||
expectThrows(UnsupportedOperationException.class, () -> filter.getItems().add("x"));
|
||||
}
|
||||
}
|
||||
|
|
|
@ -22,7 +22,7 @@ setup:
|
|||
filter_id: filter-foo
|
||||
body: >
|
||||
{
|
||||
"items": ["abc", "xyz"]
|
||||
"items": ["xyz", "abc"]
|
||||
}
|
||||
|
||||
- do:
|
||||
|
|
Loading…
Reference in New Issue