Skip to content

Commit

Permalink
[ML] Add ML filter update API (#31437)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
dimitris-athanasiou committed Jun 22, 2018
1 parent f22f91c commit c6cbc99
Show file tree
Hide file tree
Showing 37 changed files with 794 additions and 72 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@
import org.elasticsearch.xpack.core.ml.action.StopDatafeedAction;
import org.elasticsearch.xpack.core.ml.action.UpdateCalendarJobAction;
import org.elasticsearch.xpack.core.ml.action.UpdateDatafeedAction;
import org.elasticsearch.xpack.core.ml.action.UpdateFilterAction;
import org.elasticsearch.xpack.core.ml.action.UpdateJobAction;
import org.elasticsearch.xpack.core.ml.action.UpdateModelSnapshotAction;
import org.elasticsearch.xpack.core.ml.action.UpdateProcessAction;
Expand Down Expand Up @@ -220,6 +221,7 @@ public List<Action> getClientActions() {
OpenJobAction.INSTANCE,
GetFiltersAction.INSTANCE,
PutFilterAction.INSTANCE,
UpdateFilterAction.INSTANCE,
DeleteFilterAction.INSTANCE,
KillProcessAction.INSTANCE,
GetBucketsAction.INSTANCE,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,187 @@
/*
* 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.action.Action;
import org.elasticsearch.action.ActionRequest;
import org.elasticsearch.action.ActionRequestBuilder;
import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.client.ElasticsearchClient;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.xcontent.ObjectParser;
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.xpack.core.ml.job.config.MlFilter;
import org.elasticsearch.xpack.core.ml.job.messages.Messages;
import org.elasticsearch.xpack.core.ml.utils.ExceptionsHelper;

import java.io.IOException;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.Objects;
import java.util.SortedSet;
import java.util.TreeSet;


public class UpdateFilterAction extends Action<PutFilterAction.Response> {

public static final UpdateFilterAction INSTANCE = new UpdateFilterAction();
public static final String NAME = "cluster:admin/xpack/ml/filters/update";

private UpdateFilterAction() {
super(NAME);
}

@Override
public PutFilterAction.Response newResponse() {
return new PutFilterAction.Response();
}

public static class Request extends ActionRequest implements ToXContentObject {

public static final ParseField ADD_ITEMS = new ParseField("add_items");
public static final ParseField REMOVE_ITEMS = new ParseField("remove_items");

private static final ObjectParser<Request, Void> PARSER = new ObjectParser<>(NAME, Request::new);

static {
PARSER.declareString((request, filterId) -> request.filterId = filterId, MlFilter.ID);
PARSER.declareStringOrNull(Request::setDescription, MlFilter.DESCRIPTION);
PARSER.declareStringArray(Request::setAddItems, ADD_ITEMS);
PARSER.declareStringArray(Request::setRemoveItems, REMOVE_ITEMS);
}

public static Request parseRequest(String filterId, XContentParser parser) {
Request request = PARSER.apply(parser, null);
if (request.filterId == null) {
request.filterId = filterId;
} else if (!Strings.isNullOrEmpty(filterId) && !filterId.equals(request.filterId)) {
// If we have both URI and body filter ID, they must be identical
throw new IllegalArgumentException(Messages.getMessage(Messages.INCONSISTENT_ID, MlFilter.ID.getPreferredName(),
request.filterId, filterId));
}
return request;
}

private String filterId;
@Nullable
private String description;
private SortedSet<String> addItems = Collections.emptySortedSet();
private SortedSet<String> removeItems = Collections.emptySortedSet();

public Request() {
}

public Request(String filterId) {
this.filterId = ExceptionsHelper.requireNonNull(filterId, MlFilter.ID.getPreferredName());
}

public String getFilterId() {
return filterId;
}

public String getDescription() {
return description;
}

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

public SortedSet<String> getAddItems() {
return addItems;
}

public void setAddItems(Collection<String> addItems) {
this.addItems = new TreeSet<>(ExceptionsHelper.requireNonNull(addItems, ADD_ITEMS.getPreferredName()));
}

public SortedSet<String> getRemoveItems() {
return removeItems;
}

public void setRemoveItems(Collection<String> removeItems) {
this.removeItems = new TreeSet<>(ExceptionsHelper.requireNonNull(removeItems, REMOVE_ITEMS.getPreferredName()));
}

public boolean isNoop() {
return description == null && addItems.isEmpty() && removeItems.isEmpty();
}

@Override
public ActionRequestValidationException validate() {
return null;
}

@Override
public void readFrom(StreamInput in) throws IOException {
super.readFrom(in);
filterId = in.readString();
description = in.readOptionalString();
addItems = new TreeSet<>(Arrays.asList(in.readStringArray()));
removeItems = new TreeSet<>(Arrays.asList(in.readStringArray()));
}

@Override
public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);
out.writeString(filterId);
out.writeOptionalString(description);
out.writeStringArray(addItems.toArray(new String[addItems.size()]));
out.writeStringArray(removeItems.toArray(new String[removeItems.size()]));
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();
builder.field(MlFilter.ID.getPreferredName(), filterId);
if (description != null) {
builder.field(MlFilter.DESCRIPTION.getPreferredName(), description);
}
if (addItems.isEmpty() == false) {
builder.field(ADD_ITEMS.getPreferredName(), addItems);
}
if (removeItems.isEmpty() == false) {
builder.field(REMOVE_ITEMS.getPreferredName(), removeItems);
}
builder.endObject();
return builder;
}

@Override
public int hashCode() {
return Objects.hash(filterId, description, addItems, removeItems);
}

@Override
public boolean equals(Object obj) {
if (obj == null) {
return false;
}
if (getClass() != obj.getClass()) {
return false;
}
Request other = (Request) obj;
return Objects.equals(filterId, other.filterId)
&& Objects.equals(description, other.description)
&& Objects.equals(addItems, other.addItems)
&& Objects.equals(removeItems, other.removeItems);
}
}

public static class RequestBuilder extends ActionRequestBuilder<Request, PutFilterAction.Response> {

public RequestBuilder(ElasticsearchClient client) {
super(client, INSTANCE, new Request());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ private static ObjectParser<Builder, Void> createParser(boolean ignoreUnknownFie
private final String description;
private final SortedSet<String> items;

public MlFilter(String id, String description, SortedSet<String> items) {
private 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");
Expand All @@ -69,8 +69,7 @@ public MlFilter(StreamInput in) throws IOException {
} else {
description = null;
}
items = new TreeSet<>();
items.addAll(Arrays.asList(in.readStringArray()));
items = new TreeSet<>(Arrays.asList(in.readStringArray()));
}

@Override
Expand Down Expand Up @@ -163,9 +162,13 @@ public Builder setDescription(String description) {
return this;
}

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

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ public final class Messages {
public static final String DATAFEED_FREQUENCY_MUST_BE_MULTIPLE_OF_AGGREGATIONS_INTERVAL =
"Datafeed frequency [{0}] must be a multiple of the aggregation interval [{1}]";

public static final String FILTER_NOT_FOUND = "No filter with id [{0}] exists";

public static final String INCONSISTENT_ID =
"Inconsistent {0}; ''{1}'' specified in the body differs from ''{2}'' specified as a URL argument";
public static final String INVALID_ID = "Invalid {0}; ''{1}'' can contain lowercase alphanumeric (a-z and 0-9), hyphens or " +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.XContentParser.Token;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.xpack.core.ml.job.config.Job;
import org.elasticsearch.xpack.core.ml.utils.time.TimeUtils;

Expand Down Expand Up @@ -345,7 +345,7 @@ public static String v54DocumentId(String jobId, String snapshotId) {

public static ModelSnapshot fromJson(BytesReference bytesReference) {
try (InputStream stream = bytesReference.streamInput();
XContentParser parser = XContentFactory.xContent(XContentHelper.xContentType(bytesReference))
XContentParser parser = XContentFactory.xContent(XContentType.JSON)
.createParser(NamedXContentRegistry.EMPTY, LoggingDeprecationHandler.INSTANCE, stream)) {
return LENIENT_PARSER.apply(parser, null).build();
} catch (IOException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ public static ElasticsearchException serverError(String msg, Throwable cause) {
return new ElasticsearchException(msg, cause);
}

public static ElasticsearchStatusException conflictStatusException(String msg, Throwable cause, Object... args) {
return new ElasticsearchStatusException(msg, RestStatus.CONFLICT, cause, args);
}

public static ElasticsearchStatusException conflictStatusException(String msg, Object... args) {
return new ElasticsearchStatusException(msg, RestStatus.CONFLICT, args);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
* 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.action.UpdateFilterAction.Request;

import java.util.ArrayList;
import java.util.Collection;
import java.util.List;

public class UpdateFilterActionRequestTests extends AbstractStreamableXContentTestCase<Request> {

private String filterId = randomAlphaOfLength(20);

@Override
protected Request createTestInstance() {
UpdateFilterAction.Request request = new UpdateFilterAction.Request(filterId);
if (randomBoolean()) {
request.setDescription(randomAlphaOfLength(20));
}
if (randomBoolean()) {
request.setAddItems(generateRandomStrings());
}
if (randomBoolean()) {
request.setRemoveItems(generateRandomStrings());
}
return request;
}

private static Collection<String> generateRandomStrings() {
int size = randomIntBetween(0, 10);
List<String> strings = new ArrayList<>(size);
for (int i = 0; i < size; ++i) {
strings.add(randomAlphaOfLength(20));
}
return strings;
}

@Override
protected boolean supportsUnknownFields() {
return false;
}

@Override
protected Request createBlankInstance() {
return new Request();
}

@Override
protected Request doParseInstance(XContentParser parser) {
return Request.parseRequest(filterId, parser);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import org.elasticsearch.test.AbstractSerializingTestCase;

import java.io.IOException;
import java.util.SortedSet;
import java.util.TreeSet;

import static org.hamcrest.Matchers.contains;
Expand Down Expand Up @@ -43,7 +44,7 @@ public static MlFilter createRandom(String filterId) {
for (int i = 0; i < size; i++) {
items.add(randomAlphaOfLengthBetween(1, 20));
}
return new MlFilter(filterId, description, items);
return MlFilter.builder(filterId).setDescription(description).setItems(items).build();
}

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

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

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

Expand Down
Loading

0 comments on commit c6cbc99

Please sign in to comment.