Skip to content

Commit f9de429

Browse files
[ML] Add ML filter update API (#31437)
This adds an api to allow updating a filter: POST _xpack/ml/filters/{filter_id}/_update The request body may have: - description: setting a new description - add_items: a list of the items to add - remove_items: a list of the items to remove This commit also changes the PUT filter api to error when the filter_id is already used. As now there is an api for updating filters, the put api should only be used to create new ones. Also, updating a filter results into a notification message auditing the change for every job that is using that filter.
1 parent e9789ce commit f9de429

37 files changed

+801
-72
lines changed

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackClientPlugin.java

+2
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@
8484
import org.elasticsearch.xpack.core.ml.action.StopDatafeedAction;
8585
import org.elasticsearch.xpack.core.ml.action.UpdateCalendarJobAction;
8686
import org.elasticsearch.xpack.core.ml.action.UpdateDatafeedAction;
87+
import org.elasticsearch.xpack.core.ml.action.UpdateFilterAction;
8788
import org.elasticsearch.xpack.core.ml.action.UpdateJobAction;
8889
import org.elasticsearch.xpack.core.ml.action.UpdateModelSnapshotAction;
8990
import org.elasticsearch.xpack.core.ml.action.UpdateProcessAction;
@@ -220,6 +221,7 @@ public List<GenericAction> getClientActions() {
220221
OpenJobAction.INSTANCE,
221222
GetFiltersAction.INSTANCE,
222223
PutFilterAction.INSTANCE,
224+
UpdateFilterAction.INSTANCE,
223225
DeleteFilterAction.INSTANCE,
224226
KillProcessAction.INSTANCE,
225227
GetBucketsAction.INSTANCE,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,192 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
package org.elasticsearch.xpack.core.ml.action;
7+
8+
import org.elasticsearch.action.Action;
9+
import org.elasticsearch.action.ActionRequest;
10+
import org.elasticsearch.action.ActionRequestBuilder;
11+
import org.elasticsearch.action.ActionRequestValidationException;
12+
import org.elasticsearch.client.ElasticsearchClient;
13+
import org.elasticsearch.common.Nullable;
14+
import org.elasticsearch.common.ParseField;
15+
import org.elasticsearch.common.Strings;
16+
import org.elasticsearch.common.io.stream.StreamInput;
17+
import org.elasticsearch.common.io.stream.StreamOutput;
18+
import org.elasticsearch.common.xcontent.ObjectParser;
19+
import org.elasticsearch.common.xcontent.ToXContentObject;
20+
import org.elasticsearch.common.xcontent.XContentBuilder;
21+
import org.elasticsearch.common.xcontent.XContentParser;
22+
import org.elasticsearch.xpack.core.ml.job.config.MlFilter;
23+
import org.elasticsearch.xpack.core.ml.job.messages.Messages;
24+
import org.elasticsearch.xpack.core.ml.utils.ExceptionsHelper;
25+
26+
import java.io.IOException;
27+
import java.util.Arrays;
28+
import java.util.Collection;
29+
import java.util.Collections;
30+
import java.util.Objects;
31+
import java.util.SortedSet;
32+
import java.util.TreeSet;
33+
34+
35+
public class UpdateFilterAction extends Action<UpdateFilterAction.Request, PutFilterAction.Response, UpdateFilterAction.RequestBuilder> {
36+
37+
public static final UpdateFilterAction INSTANCE = new UpdateFilterAction();
38+
public static final String NAME = "cluster:admin/xpack/ml/filters/update";
39+
40+
private UpdateFilterAction() {
41+
super(NAME);
42+
}
43+
44+
@Override
45+
public RequestBuilder newRequestBuilder(ElasticsearchClient client) {
46+
return new RequestBuilder(client);
47+
}
48+
49+
@Override
50+
public PutFilterAction.Response newResponse() {
51+
return new PutFilterAction.Response();
52+
}
53+
54+
public static class Request extends ActionRequest implements ToXContentObject {
55+
56+
public static final ParseField ADD_ITEMS = new ParseField("add_items");
57+
public static final ParseField REMOVE_ITEMS = new ParseField("remove_items");
58+
59+
private static final ObjectParser<Request, Void> PARSER = new ObjectParser<>(NAME, Request::new);
60+
61+
static {
62+
PARSER.declareString((request, filterId) -> request.filterId = filterId, MlFilter.ID);
63+
PARSER.declareStringOrNull(Request::setDescription, MlFilter.DESCRIPTION);
64+
PARSER.declareStringArray(Request::setAddItems, ADD_ITEMS);
65+
PARSER.declareStringArray(Request::setRemoveItems, REMOVE_ITEMS);
66+
}
67+
68+
public static Request parseRequest(String filterId, XContentParser parser) {
69+
Request request = PARSER.apply(parser, null);
70+
if (request.filterId == null) {
71+
request.filterId = filterId;
72+
} else if (!Strings.isNullOrEmpty(filterId) && !filterId.equals(request.filterId)) {
73+
// If we have both URI and body filter ID, they must be identical
74+
throw new IllegalArgumentException(Messages.getMessage(Messages.INCONSISTENT_ID, MlFilter.ID.getPreferredName(),
75+
request.filterId, filterId));
76+
}
77+
return request;
78+
}
79+
80+
private String filterId;
81+
@Nullable
82+
private String description;
83+
private SortedSet<String> addItems = Collections.emptySortedSet();
84+
private SortedSet<String> removeItems = Collections.emptySortedSet();
85+
86+
public Request() {
87+
}
88+
89+
public Request(String filterId) {
90+
this.filterId = ExceptionsHelper.requireNonNull(filterId, MlFilter.ID.getPreferredName());
91+
}
92+
93+
public String getFilterId() {
94+
return filterId;
95+
}
96+
97+
public String getDescription() {
98+
return description;
99+
}
100+
101+
public void setDescription(String description) {
102+
this.description = description;
103+
}
104+
105+
public SortedSet<String> getAddItems() {
106+
return addItems;
107+
}
108+
109+
public void setAddItems(Collection<String> addItems) {
110+
this.addItems = new TreeSet<>(ExceptionsHelper.requireNonNull(addItems, ADD_ITEMS.getPreferredName()));
111+
}
112+
113+
public SortedSet<String> getRemoveItems() {
114+
return removeItems;
115+
}
116+
117+
public void setRemoveItems(Collection<String> removeItems) {
118+
this.removeItems = new TreeSet<>(ExceptionsHelper.requireNonNull(removeItems, REMOVE_ITEMS.getPreferredName()));
119+
}
120+
121+
public boolean isNoop() {
122+
return description == null && addItems.isEmpty() && removeItems.isEmpty();
123+
}
124+
125+
@Override
126+
public ActionRequestValidationException validate() {
127+
return null;
128+
}
129+
130+
@Override
131+
public void readFrom(StreamInput in) throws IOException {
132+
super.readFrom(in);
133+
filterId = in.readString();
134+
description = in.readOptionalString();
135+
addItems = new TreeSet<>(Arrays.asList(in.readStringArray()));
136+
removeItems = new TreeSet<>(Arrays.asList(in.readStringArray()));
137+
}
138+
139+
@Override
140+
public void writeTo(StreamOutput out) throws IOException {
141+
super.writeTo(out);
142+
out.writeString(filterId);
143+
out.writeOptionalString(description);
144+
out.writeStringArray(addItems.toArray(new String[addItems.size()]));
145+
out.writeStringArray(removeItems.toArray(new String[removeItems.size()]));
146+
}
147+
148+
@Override
149+
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
150+
builder.startObject();
151+
builder.field(MlFilter.ID.getPreferredName(), filterId);
152+
if (description != null) {
153+
builder.field(MlFilter.DESCRIPTION.getPreferredName(), description);
154+
}
155+
if (addItems.isEmpty() == false) {
156+
builder.field(ADD_ITEMS.getPreferredName(), addItems);
157+
}
158+
if (removeItems.isEmpty() == false) {
159+
builder.field(REMOVE_ITEMS.getPreferredName(), removeItems);
160+
}
161+
builder.endObject();
162+
return builder;
163+
}
164+
165+
@Override
166+
public int hashCode() {
167+
return Objects.hash(filterId, description, addItems, removeItems);
168+
}
169+
170+
@Override
171+
public boolean equals(Object obj) {
172+
if (obj == null) {
173+
return false;
174+
}
175+
if (getClass() != obj.getClass()) {
176+
return false;
177+
}
178+
Request other = (Request) obj;
179+
return Objects.equals(filterId, other.filterId)
180+
&& Objects.equals(description, other.description)
181+
&& Objects.equals(addItems, other.addItems)
182+
&& Objects.equals(removeItems, other.removeItems);
183+
}
184+
}
185+
186+
public static class RequestBuilder extends ActionRequestBuilder<Request, PutFilterAction.Response, RequestBuilder> {
187+
188+
public RequestBuilder(ElasticsearchClient client) {
189+
super(client, INSTANCE, new Request());
190+
}
191+
}
192+
}

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

+8-5
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ private static ObjectParser<Builder, Void> createParser(boolean ignoreUnknownFie
5656
private final String description;
5757
private final SortedSet<String> items;
5858

59-
public MlFilter(String id, String description, SortedSet<String> items) {
59+
private MlFilter(String id, String description, SortedSet<String> items) {
6060
this.id = Objects.requireNonNull(id, ID.getPreferredName() + " must not be null");
6161
this.description = description;
6262
this.items = Objects.requireNonNull(items, ITEMS.getPreferredName() + " must not be null");
@@ -69,8 +69,7 @@ public MlFilter(StreamInput in) throws IOException {
6969
} else {
7070
description = null;
7171
}
72-
items = new TreeSet<>();
73-
items.addAll(Arrays.asList(in.readStringArray()));
72+
items = new TreeSet<>(Arrays.asList(in.readStringArray()));
7473
}
7574

7675
@Override
@@ -163,9 +162,13 @@ public Builder setDescription(String description) {
163162
return this;
164163
}
165164

165+
public Builder setItems(SortedSet<String> items) {
166+
this.items = items;
167+
return this;
168+
}
169+
166170
public Builder setItems(List<String> items) {
167-
this.items = new TreeSet<>();
168-
this.items.addAll(items);
171+
this.items = new TreeSet<>(items);
169172
return this;
170173
}
171174

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/messages/Messages.java

+2
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ public final class Messages {
4242
public static final String DATAFEED_FREQUENCY_MUST_BE_MULTIPLE_OF_AGGREGATIONS_INTERVAL =
4343
"Datafeed frequency [{0}] must be a multiple of the aggregation interval [{1}]";
4444

45+
public static final String FILTER_NOT_FOUND = "No filter with id [{0}] exists";
46+
4547
public static final String INCONSISTENT_ID =
4648
"Inconsistent {0}; ''{1}'' specified in the body differs from ''{2}'' specified as a URL argument";
4749
public static final String INVALID_ID = "Invalid {0}; ''{1}'' can contain lowercase alphanumeric (a-z and 0-9), hyphens or " +

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/process/autodetect/state/ModelSnapshot.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@
1818
import org.elasticsearch.common.xcontent.ToXContentObject;
1919
import org.elasticsearch.common.xcontent.XContentBuilder;
2020
import org.elasticsearch.common.xcontent.XContentFactory;
21-
import org.elasticsearch.common.xcontent.XContentHelper;
2221
import org.elasticsearch.common.xcontent.XContentParser;
2322
import org.elasticsearch.common.xcontent.XContentParser.Token;
23+
import org.elasticsearch.common.xcontent.XContentType;
2424
import org.elasticsearch.xpack.core.ml.job.config.Job;
2525
import org.elasticsearch.xpack.core.ml.utils.time.TimeUtils;
2626

@@ -318,7 +318,7 @@ public static String v54DocumentId(String jobId, String snapshotId) {
318318

319319
public static ModelSnapshot fromJson(BytesReference bytesReference) {
320320
try (InputStream stream = bytesReference.streamInput();
321-
XContentParser parser = XContentFactory.xContent(XContentHelper.xContentType(bytesReference))
321+
XContentParser parser = XContentFactory.xContent(XContentType.JSON)
322322
.createParser(NamedXContentRegistry.EMPTY, LoggingDeprecationHandler.INSTANCE, stream)) {
323323
return LENIENT_PARSER.apply(parser, null).build();
324324
} catch (IOException e) {

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/utils/ExceptionsHelper.java

+4
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ public static ElasticsearchException serverError(String msg, Throwable cause) {
3838
return new ElasticsearchException(msg, cause);
3939
}
4040

41+
public static ElasticsearchStatusException conflictStatusException(String msg, Throwable cause, Object... args) {
42+
return new ElasticsearchStatusException(msg, RestStatus.CONFLICT, cause, args);
43+
}
44+
4145
public static ElasticsearchStatusException conflictStatusException(String msg, Object... args) {
4246
return new ElasticsearchStatusException(msg, RestStatus.CONFLICT, args);
4347
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
package org.elasticsearch.xpack.core.ml.action;
7+
8+
import org.elasticsearch.common.xcontent.XContentParser;
9+
import org.elasticsearch.test.AbstractStreamableXContentTestCase;
10+
import org.elasticsearch.xpack.core.ml.action.UpdateFilterAction.Request;
11+
12+
import java.util.ArrayList;
13+
import java.util.Collection;
14+
import java.util.List;
15+
16+
public class UpdateFilterActionRequestTests extends AbstractStreamableXContentTestCase<Request> {
17+
18+
private String filterId = randomAlphaOfLength(20);
19+
20+
@Override
21+
protected Request createTestInstance() {
22+
UpdateFilterAction.Request request = new UpdateFilterAction.Request(filterId);
23+
if (randomBoolean()) {
24+
request.setDescription(randomAlphaOfLength(20));
25+
}
26+
if (randomBoolean()) {
27+
request.setAddItems(generateRandomStrings());
28+
}
29+
if (randomBoolean()) {
30+
request.setRemoveItems(generateRandomStrings());
31+
}
32+
return request;
33+
}
34+
35+
private static Collection<String> generateRandomStrings() {
36+
int size = randomIntBetween(0, 10);
37+
List<String> strings = new ArrayList<>(size);
38+
for (int i = 0; i < size; ++i) {
39+
strings.add(randomAlphaOfLength(20));
40+
}
41+
return strings;
42+
}
43+
44+
@Override
45+
protected boolean supportsUnknownFields() {
46+
return false;
47+
}
48+
49+
@Override
50+
protected Request createBlankInstance() {
51+
return new Request();
52+
}
53+
54+
@Override
55+
protected Request doParseInstance(XContentParser parser) {
56+
return Request.parseRequest(filterId, parser);
57+
}
58+
}

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

+5-4
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import org.elasticsearch.test.AbstractSerializingTestCase;
1212

1313
import java.io.IOException;
14+
import java.util.SortedSet;
1415
import java.util.TreeSet;
1516

1617
import static org.hamcrest.Matchers.contains;
@@ -43,7 +44,7 @@ public static MlFilter createRandom(String filterId) {
4344
for (int i = 0; i < size; i++) {
4445
items.add(randomAlphaOfLengthBetween(1, 20));
4546
}
46-
return new MlFilter(filterId, description, items);
47+
return MlFilter.builder(filterId).setDescription(description).setItems(items).build();
4748
}
4849

4950
@Override
@@ -57,13 +58,13 @@ protected MlFilter doParseInstance(XContentParser parser) {
5758
}
5859

5960
public void testNullId() {
60-
NullPointerException ex = expectThrows(NullPointerException.class, () -> new MlFilter(null, "", new TreeSet<>()));
61+
NullPointerException ex = expectThrows(NullPointerException.class, () -> MlFilter.builder(null).build());
6162
assertEquals(MlFilter.ID.getPreferredName() + " must not be null", ex.getMessage());
6263
}
6364

6465
public void testNullItems() {
65-
NullPointerException ex =
66-
expectThrows(NullPointerException.class, () -> new MlFilter(randomAlphaOfLengthBetween(1, 20), "", null));
66+
NullPointerException ex = expectThrows(NullPointerException.class,
67+
() -> MlFilter.builder(randomAlphaOfLength(20)).setItems((SortedSet<String>) null).build());
6768
assertEquals(MlFilter.ITEMS.getPreferredName() + " must not be null", ex.getMessage());
6869
}
6970

0 commit comments

Comments
 (0)