From f7caf632c05c75d6fd7ac0db991207049adef61e Mon Sep 17 00:00:00 2001 From: Hendrik Muhs Date: Fri, 10 Aug 2018 13:13:48 +0200 Subject: [PATCH 1/6] enable unit tests --- .../ml-feature-index-builder/build.gradle | 10 ++++++--- ...tureIndexBuilderJobActionRequestTests.java | 11 ++++++++++ ...tartFeatureIndexBuilderJobActionTests.java | 21 +++++++++++++++++++ 3 files changed, 39 insertions(+), 3 deletions(-) create mode 100644 x-pack/plugin/ml-feature-index-builder/src/test/java/org/elasticsearch/xpack/ml/featureindexbuilder/action/PutFeatureIndexBuilderJobActionRequestTests.java create mode 100644 x-pack/plugin/ml-feature-index-builder/src/test/java/org/elasticsearch/xpack/ml/featureindexbuilder/action/StartFeatureIndexBuilderJobActionTests.java diff --git a/x-pack/plugin/ml-feature-index-builder/build.gradle b/x-pack/plugin/ml-feature-index-builder/build.gradle index f92709b528643..30d4cd8a60262 100644 --- a/x-pack/plugin/ml-feature-index-builder/build.gradle +++ b/x-pack/plugin/ml-feature-index-builder/build.gradle @@ -3,7 +3,6 @@ import org.elasticsearch.gradle.BuildPlugin evaluationDependsOn(xpackModule('core')) apply plugin: 'elasticsearch.esplugin' - esplugin { name 'ml-feature-index-builder' description 'A plugin to build feature indexes' @@ -11,13 +10,18 @@ esplugin { extendedPlugins = ['x-pack-core'] } +compileJava.options.compilerArgs << "-Xlint:-rawtypes" +compileTestJava.options.compilerArgs << "-Xlint:-rawtypes" + dependencies { compileOnly "org.elasticsearch:elasticsearch:${version}" - compileOnly "org.elasticsearch.plugin:x-pack-core:${version}" + compileOnly project(path: xpackModule('core'), configuration: 'shadow') testCompile project(path: xpackModule('core'), configuration: 'testArtifacts') } - +run { + plugin xpackModule('core') +} integTest.enabled = false diff --git a/x-pack/plugin/ml-feature-index-builder/src/test/java/org/elasticsearch/xpack/ml/featureindexbuilder/action/PutFeatureIndexBuilderJobActionRequestTests.java b/x-pack/plugin/ml-feature-index-builder/src/test/java/org/elasticsearch/xpack/ml/featureindexbuilder/action/PutFeatureIndexBuilderJobActionRequestTests.java new file mode 100644 index 0000000000000..6e3e612176179 --- /dev/null +++ b/x-pack/plugin/ml-feature-index-builder/src/test/java/org/elasticsearch/xpack/ml/featureindexbuilder/action/PutFeatureIndexBuilderJobActionRequestTests.java @@ -0,0 +1,11 @@ +/* + * 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.ml.featureindexbuilder.action; + +public class PutFeatureIndexBuilderJobActionRequestTests { + +} diff --git a/x-pack/plugin/ml-feature-index-builder/src/test/java/org/elasticsearch/xpack/ml/featureindexbuilder/action/StartFeatureIndexBuilderJobActionTests.java b/x-pack/plugin/ml-feature-index-builder/src/test/java/org/elasticsearch/xpack/ml/featureindexbuilder/action/StartFeatureIndexBuilderJobActionTests.java new file mode 100644 index 0000000000000..cec3458e8445a --- /dev/null +++ b/x-pack/plugin/ml-feature-index-builder/src/test/java/org/elasticsearch/xpack/ml/featureindexbuilder/action/StartFeatureIndexBuilderJobActionTests.java @@ -0,0 +1,21 @@ +package org.elasticsearch.xpack.ml.featureindexbuilder.action; +/* + * 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. + */ + +import org.elasticsearch.test.AbstractStreamableTestCase; +import org.elasticsearch.xpack.core.rollup.action.StartRollupJobAction.Request; + +public class StartFeatureIndexBuilderJobActionTests extends AbstractStreamableTestCase { + @Override + protected Request createTestInstance() { + return new Request(randomAlphaOfLengthBetween(1, 20)); + } + + @Override + protected Request createBlankInstance() { + return new Request(); + } +} From 5db1432bb758e39e823440497808df800aced012 Mon Sep 17 00:00:00 2001 From: Hendrik Muhs Date: Fri, 10 Aug 2018 13:29:35 +0200 Subject: [PATCH 2/6] rename FeatureIndexBuilderJobState --- .../FeatureIndexBuilder.java | 2 - ...nsportPutFeatureIndexBuilderJobAction.java | 1 - .../job/FeatureIndexBuilderJobState.java | 101 ++++++++++++++++++ .../job/FeatureIndexBuilderJobStatus.java | 73 ------------- .../job/FeatureIndexBuilderJobTask.java | 5 +- 5 files changed, 103 insertions(+), 79 deletions(-) create mode 100644 x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJobState.java delete mode 100644 x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJobStatus.java diff --git a/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/FeatureIndexBuilder.java b/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/FeatureIndexBuilder.java index 197a2f9f29350..a216465bc0935 100644 --- a/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/FeatureIndexBuilder.java +++ b/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/FeatureIndexBuilder.java @@ -30,8 +30,6 @@ import org.elasticsearch.threadpool.FixedExecutorBuilder; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.xpack.core.XPackPlugin; -import org.elasticsearch.xpack.core.rollup.RollupField; -import org.elasticsearch.xpack.core.rollup.job.RollupJob; import org.elasticsearch.xpack.core.scheduler.SchedulerEngine; import org.elasticsearch.xpack.ml.featureindexbuilder.action.PutFeatureIndexBuilderJobAction; import org.elasticsearch.xpack.ml.featureindexbuilder.action.StartFeatureIndexBuilderJobAction; diff --git a/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/action/TransportPutFeatureIndexBuilderJobAction.java b/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/action/TransportPutFeatureIndexBuilderJobAction.java index e4e17987d4049..16c541db7a01d 100644 --- a/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/action/TransportPutFeatureIndexBuilderJobAction.java +++ b/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/action/TransportPutFeatureIndexBuilderJobAction.java @@ -69,7 +69,6 @@ protected void masterOperation(Request request, ClusterState clusterState, Actio FeatureIndexBuilderJob job = createFeatureIndexBuilderJob(request.getConfig(), threadPool); startPersistentTask(job, listener, persistentTasksService); - } private static FeatureIndexBuilderJob createFeatureIndexBuilderJob(FeatureIndexBuilderJobConfig config, ThreadPool threadPool) { diff --git a/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJobState.java b/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJobState.java new file mode 100644 index 0000000000000..7d47ff6ea79bc --- /dev/null +++ b/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJobState.java @@ -0,0 +1,101 @@ +/* + * 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.ml.featureindexbuilder.job; + +import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.xcontent.ConstructingObjectParser; +import org.elasticsearch.common.xcontent.ObjectParser; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.persistent.PersistentTaskState; +import org.elasticsearch.tasks.Task; +import org.elasticsearch.xpack.core.indexing.IndexerState; + +import java.io.IOException; +import java.util.Objects; +import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg; + +public class FeatureIndexBuilderJobState implements Task.Status, PersistentTaskState { + public static final String NAME = "xpack/feature_index_builder/job"; + + private final IndexerState state; + + private static final ParseField STATE = new ParseField("job_state"); + + public static final ConstructingObjectParser PARSER = + new ConstructingObjectParser<>(NAME, + args -> new FeatureIndexBuilderJobState((IndexerState) args[0])); + + static { + PARSER.declareField(constructorArg(), p -> { + if (p.currentToken() == XContentParser.Token.VALUE_STRING) { + return IndexerState.fromString(p.text()); + } + throw new IllegalArgumentException("Unsupported token [" + p.currentToken() + "]"); + }, STATE, ObjectParser.ValueType.STRING); + } + + public FeatureIndexBuilderJobState(IndexerState state) { + this.state = state; + } + + public FeatureIndexBuilderJobState(StreamInput in) throws IOException { + state = IndexerState.fromStream(in); + } + + public IndexerState getJobState() { + return state; + } + + public static FeatureIndexBuilderJobState fromXContent(XContentParser parser) { + try { + return PARSER.parse(parser, null); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.startObject(); + builder.field(STATE.getPreferredName(), state.value()); + builder.endObject(); + return builder; + } + + @Override + public String getWriteableName() { + return NAME; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + state.writeTo(out); + } + + @Override + public boolean equals(Object other) { + if (this == other) { + return true; + } + + if (other == null || getClass() != other.getClass()) { + return false; + } + + FeatureIndexBuilderJobState that = (FeatureIndexBuilderJobState) other; + + return Objects.equals(this.state, that.state); + } + + @Override + public int hashCode() { + return Objects.hash(state); + } +} \ No newline at end of file diff --git a/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJobStatus.java b/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJobStatus.java deleted file mode 100644 index 85debce5339e0..0000000000000 --- a/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJobStatus.java +++ /dev/null @@ -1,73 +0,0 @@ -/* - * 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.ml.featureindexbuilder.job; - -import java.util.Collection; -import java.util.Map; -import java.util.Set; - -public class FeatureIndexBuilderJobStatus implements Map { - - @Override - public void clear() { - } - - @Override - public boolean containsKey(Object arg0) { - return false; - } - - @Override - public boolean containsValue(Object arg0) { - return false; - } - - @Override - public Set> entrySet() { - return null; - } - - @Override - public String get(Object arg0) { - return null; - } - - @Override - public boolean isEmpty() { - return false; - } - - @Override - public Set keySet() { - return null; - } - - @Override - public String put(String arg0, String arg1) { - return null; - } - - @Override - public void putAll(Map arg0) { - } - - @Override - public String remove(Object arg0) { - return null; - } - - @Override - public int size() { - return 0; - } - - @Override - public Collection values() { - return null; - } - -} diff --git a/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJobTask.java b/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJobTask.java index ca546529414d5..90a190419579a 100644 --- a/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJobTask.java +++ b/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJobTask.java @@ -17,7 +17,6 @@ import org.elasticsearch.persistent.PersistentTasksExecutor; import org.elasticsearch.tasks.TaskId; import org.elasticsearch.threadpool.ThreadPool; -import org.elasticsearch.xpack.core.rollup.action.StartRollupJobAction; import org.elasticsearch.xpack.core.scheduler.SchedulerEngine; import org.elasticsearch.xpack.core.scheduler.SchedulerEngine.Event; import org.elasticsearch.xpack.ml.featureindexbuilder.FeatureIndexBuilder; @@ -70,14 +69,14 @@ protected AllocatedPersistentTask createTask(long id, String type, String action PersistentTasksCustomMetaData.PersistentTask persistentTask, Map headers) { return new FeatureIndexBuilderJobTask(id, type, action, parentTaskId, persistentTask.getParams(), - (FeatureIndexBuilderJobStatus) persistentTask.getState(), client, schedulerEngine, threadPool, headers); + (FeatureIndexBuilderJobState) persistentTask.getState(), client, schedulerEngine, threadPool, headers); } } private final FeatureIndexBuilderJob job; public FeatureIndexBuilderJobTask(long id, String type, String action, TaskId parentTask, FeatureIndexBuilderJob job, - FeatureIndexBuilderJobStatus state, Client client, SchedulerEngine schedulerEngine, ThreadPool threadPool, + FeatureIndexBuilderJobState state, Client client, SchedulerEngine schedulerEngine, ThreadPool threadPool, Map headers) { super(id, type, action, "" + "_" + job.getConfig().getId(), parentTask, headers); this.job = job; From 3bcea05673971106962d7d8f92a20cbc669053db Mon Sep 17 00:00:00 2001 From: Hendrik Muhs Date: Fri, 10 Aug 2018 16:53:34 +0200 Subject: [PATCH 3/6] add test for PutFeatureIndexBuilderJob --- .../PutFeatureIndexBuilderJobAction.java | 7 +- .../job/FeatureIndexBuilderJob.java | 5 +- .../job/FeatureIndexBuilderJobConfig.java | 86 +++++-------------- .../RestPutFeatureIndexBuilderJobAction.java | 2 +- ...tureIndexBuilderJobActionRequestTests.java | 38 +++++++- 5 files changed, 66 insertions(+), 72 deletions(-) diff --git a/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/action/PutFeatureIndexBuilderJobAction.java b/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/action/PutFeatureIndexBuilderJobAction.java index 22e3b2a6ba2c7..a05b526483bec 100644 --- a/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/action/PutFeatureIndexBuilderJobAction.java +++ b/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/action/PutFeatureIndexBuilderJobAction.java @@ -48,9 +48,8 @@ public Request() { } - public static Request parseRequest(String id, XContentParser parser) { - FeatureIndexBuilderJobConfig.Builder config = FeatureIndexBuilderJobConfig.Builder.fromXContent(id, parser); - return new Request(config.build()); + public static Request fromXContent(final XContentParser parser, final String id) throws IOException { + return new Request(FeatureIndexBuilderJobConfig.fromXContent(parser, id)); } @Override @@ -107,7 +106,7 @@ protected RequestBuilder(ElasticsearchClient client, PutFeatureIndexBuilderJobAc super(client, action, new Request()); } } - + public static class Response extends AcknowledgedResponse { public Response() { super(); diff --git a/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJob.java b/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJob.java index 4dd037d0d8d92..5819f0243ad52 100644 --- a/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJob.java +++ b/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJob.java @@ -14,6 +14,8 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.xpack.core.XPackPlugin; +import org.elasticsearch.xpack.core.rollup.job.RollupJobConfig; + import java.io.IOException; import java.util.Objects; @@ -25,12 +27,11 @@ public class FeatureIndexBuilderJob implements XPackPlugin.XPackPersistentTaskPa private static final ParseField CONFIG = new ParseField("config"); - @SuppressWarnings("unchecked") public static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>(NAME, a -> new FeatureIndexBuilderJob((FeatureIndexBuilderJobConfig) a[0])); static { - PARSER.declareObject(ConstructingObjectParser.constructorArg(), (p, c) -> FeatureIndexBuilderJobConfig.PARSER.apply(p,c).build(), CONFIG); + PARSER.declareObject(ConstructingObjectParser.constructorArg(), (p, c) -> FeatureIndexBuilderJobConfig.fromXContent(p, null), CONFIG); } diff --git a/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJobConfig.java b/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJobConfig.java index 19e10044e5c21..94f41b7d9e667 100644 --- a/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJobConfig.java +++ b/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJobConfig.java @@ -6,48 +6,48 @@ package org.elasticsearch.xpack.ml.featureindexbuilder.job; +import org.elasticsearch.common.Nullable; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.NamedWriteable; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.common.io.stream.Writeable; -import org.elasticsearch.common.xcontent.ObjectParser; +import org.elasticsearch.common.xcontent.ConstructingObjectParser; import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; - import java.io.IOException; import java.util.Objects; +import static org.elasticsearch.common.xcontent.ConstructingObjectParser.optionalConstructorArg; + /** * This class holds the configuration details of a feature index builder job */ public class FeatureIndexBuilderJobConfig implements NamedWriteable, ToXContentObject { - private static final String NAME = "xpack/feature_index_builder/jobconfig"; - public static final ParseField ID = new ParseField("id"); - - private String id; + private static final String NAME = "xpack/feature_index_builder/jobconfig"; + private static final String ID = "id"; - public static final ObjectParser PARSER = new ObjectParser<>(NAME, false, - FeatureIndexBuilderJobConfig.Builder::new); + private final String id; + private static final ConstructingObjectParser PARSER; static { - PARSER.declareString(FeatureIndexBuilderJobConfig.Builder::setId, ID); + PARSER = new ConstructingObjectParser<>(NAME, false, (args, optionalId) -> { + String id = args[0] != null ? (String) args[0] : optionalId; + return new FeatureIndexBuilderJobConfig(id); + }); + PARSER.declareString(optionalConstructorArg(), new ParseField(ID)); } - FeatureIndexBuilderJobConfig(String id) { + public FeatureIndexBuilderJobConfig(final String id) { this.id = id; } - public FeatureIndexBuilderJobConfig(StreamInput in) throws IOException { + public FeatureIndexBuilderJobConfig(final StreamInput in) throws IOException { id = in.readString(); } - public FeatureIndexBuilderJobConfig() { - } - public String getId() { return id; } @@ -56,15 +56,14 @@ public String getCron() { return "*"; } - public void writeTo(StreamOutput out) throws IOException { + public void writeTo(final StreamOutput out) throws IOException { out.writeString(id); } - public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + public XContentBuilder toXContent(final XContentBuilder builder, final Params params) throws IOException { builder.startObject(); - if (id != null) { - // to be replace by constant - builder.field("id", id); + { + builder.field(ID, id); } builder.endObject(); return builder; @@ -85,7 +84,7 @@ public boolean equals(Object other) { return false; } - FeatureIndexBuilderJobConfig that = (FeatureIndexBuilderJobConfig) other; + final FeatureIndexBuilderJobConfig that = (FeatureIndexBuilderJobConfig) other; return Objects.equals(this.id, that.id); } @@ -100,48 +99,7 @@ public String toString() { return Strings.toString(this, true, true); } - public static class Builder implements Writeable, ToXContentObject { - private String id; - - public Builder() {} - - public Builder(FeatureIndexBuilderJobConfig job) { - this.setId(job.getId()); - } - - public static FeatureIndexBuilderJobConfig.Builder fromXContent(String id, XContentParser parser) { - FeatureIndexBuilderJobConfig.Builder config = FeatureIndexBuilderJobConfig.PARSER.apply(parser, null); - if (id != null) { - config.setId(id); - } - return config; - } - - @Override - public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - builder.startObject(); - if (id != null) { - builder.field(ID.getPreferredName(), id); - } - builder.endObject(); - return builder; - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - out.writeString(id); - } - - public String getId() { - return id; - } - - public void setId(String id) { - this.id = id; - } - - public FeatureIndexBuilderJobConfig build() { - return new FeatureIndexBuilderJobConfig(id); - } + public static FeatureIndexBuilderJobConfig fromXContent(final XContentParser parser, @Nullable final String optionalJobId) throws IOException { + return PARSER.parse(parser, optionalJobId); } } diff --git a/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/rest/action/RestPutFeatureIndexBuilderJobAction.java b/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/rest/action/RestPutFeatureIndexBuilderJobAction.java index 8f08fbd2b5b05..afcda67b241d2 100644 --- a/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/rest/action/RestPutFeatureIndexBuilderJobAction.java +++ b/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/rest/action/RestPutFeatureIndexBuilderJobAction.java @@ -37,7 +37,7 @@ protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient String id = restRequest.param(ID.getPreferredName()); XContentParser parser = restRequest.contentParser(); - PutFeatureIndexBuilderJobAction.Request request = PutFeatureIndexBuilderJobAction.Request.parseRequest(id, parser); + PutFeatureIndexBuilderJobAction.Request request = PutFeatureIndexBuilderJobAction.Request.fromXContent(parser, id); return channel -> client.execute(PutFeatureIndexBuilderJobAction.INSTANCE, request, new RestToXContentListener<>(channel)); } diff --git a/x-pack/plugin/ml-feature-index-builder/src/test/java/org/elasticsearch/xpack/ml/featureindexbuilder/action/PutFeatureIndexBuilderJobActionRequestTests.java b/x-pack/plugin/ml-feature-index-builder/src/test/java/org/elasticsearch/xpack/ml/featureindexbuilder/action/PutFeatureIndexBuilderJobActionRequestTests.java index 6e3e612176179..14bedcbe909b1 100644 --- a/x-pack/plugin/ml-feature-index-builder/src/test/java/org/elasticsearch/xpack/ml/featureindexbuilder/action/PutFeatureIndexBuilderJobActionRequestTests.java +++ b/x-pack/plugin/ml-feature-index-builder/src/test/java/org/elasticsearch/xpack/ml/featureindexbuilder/action/PutFeatureIndexBuilderJobActionRequestTests.java @@ -6,6 +6,42 @@ package org.elasticsearch.xpack.ml.featureindexbuilder.action; -public class PutFeatureIndexBuilderJobActionRequestTests { +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.test.AbstractStreamableXContentTestCase; +import org.elasticsearch.xpack.ml.featureindexbuilder.action.PutFeatureIndexBuilderJobAction.Request; +import org.elasticsearch.xpack.ml.featureindexbuilder.job.FeatureIndexBuilderJobConfig; +import org.junit.Before; + +import java.io.IOException; + +public class PutFeatureIndexBuilderJobActionRequestTests extends AbstractStreamableXContentTestCase { + + private String jobId; + + @Before + public void setupJobID() { + jobId = randomAlphaOfLengthBetween(1,10); + } + + @Override + protected Request doParseInstance(XContentParser parser) throws IOException { + return Request.fromXContent(parser, jobId); + } + + @Override + protected Request createBlankInstance() { + return new Request(); + } + + @Override + protected boolean supportsUnknownFields() { + return false; + } + + @Override + protected Request createTestInstance() { + FeatureIndexBuilderJobConfig config = new FeatureIndexBuilderJobConfig(randomAlphaOfLengthBetween(1,10)); + return new Request(config); + } } From 683f04fc3bed13d3066ca2b73054132221ddb384 Mon Sep 17 00:00:00 2001 From: Hendrik Muhs Date: Mon, 13 Aug 2018 10:29:08 +0200 Subject: [PATCH 4/6] address review comments and fix style --- ...portStartFeatureIndexBuilderJobAction.java | 28 ++++++++++--------- .../job/FeatureIndexBuilderJob.java | 23 ++++++++------- .../job/FeatureIndexBuilderJobConfig.java | 3 +- .../job/FeatureIndexBuilderJobState.java | 7 ++--- .../job/FeatureIndexBuilderJobTask.java | 18 ++++++------ 5 files changed, 40 insertions(+), 39 deletions(-) diff --git a/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/action/TransportStartFeatureIndexBuilderJobAction.java b/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/action/TransportStartFeatureIndexBuilderJobAction.java index b6aacc98daf1a..aaaf695e346d9 100644 --- a/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/action/TransportStartFeatureIndexBuilderJobAction.java +++ b/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/action/TransportStartFeatureIndexBuilderJobAction.java @@ -29,28 +29,30 @@ import java.util.function.Consumer; public class TransportStartFeatureIndexBuilderJobAction extends - TransportTasksAction { + TransportTasksAction { private final XPackLicenseState licenseState; -@Inject -public TransportStartFeatureIndexBuilderJobAction(Settings settings, TransportService transportService, - ActionFilters actionFilters, ClusterService clusterService, XPackLicenseState licenseState) { -super(settings, StartFeatureIndexBuilderJobAction.NAME, clusterService, transportService, actionFilters, - StartFeatureIndexBuilderJobAction.Request::new, StartFeatureIndexBuilderJobAction.Response::new, ThreadPool.Names.SAME); -this.licenseState = licenseState; -} + @Inject + public TransportStartFeatureIndexBuilderJobAction(Settings settings, TransportService transportService, ActionFilters actionFilters, + ClusterService clusterService, XPackLicenseState licenseState) { + super(settings, StartFeatureIndexBuilderJobAction.NAME, clusterService, transportService, actionFilters, + StartFeatureIndexBuilderJobAction.Request::new, StartFeatureIndexBuilderJobAction.Response::new, ThreadPool.Names.SAME); + this.licenseState = licenseState; + } @Override protected void processTasks(StartFeatureIndexBuilderJobAction.Request request, Consumer operation) { FeatureIndexBuilderJobTask matchingTask = null; - + // todo: re-factor, see rollup TransportTaskHelper for (Task task : taskManager.getTasks().values()) { - if (task instanceof FeatureIndexBuilderJobTask && ((FeatureIndexBuilderJobTask)task).getConfig().getId().equals(request.getId())) { + if (task instanceof FeatureIndexBuilderJobTask + && ((FeatureIndexBuilderJobTask) task).getConfig().getId().equals(request.getId())) { if (matchingTask != null) { - throw new IllegalArgumentException("Found more than one matching task for feature index builder job [" + request.getId() + "] when " + - "there should only be one."); + throw new IllegalArgumentException("Found more than one matching task for feature index builder job [" + request.getId() + + "] when " + "there should only be one."); } matchingTask = (FeatureIndexBuilderJobTask) task; } @@ -69,7 +71,7 @@ protected void doExecute(Task task, StartFeatureIndexBuilderJobAction.Request re listener.onFailure(LicenseUtils.newComplianceException(XPackField.FIB)); return; } - + super.doExecute(task, request, listener); } diff --git a/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJob.java b/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJob.java index 5819f0243ad52..16a4163e8135a 100644 --- a/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJob.java +++ b/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJob.java @@ -14,7 +14,6 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.xpack.core.XPackPlugin; -import org.elasticsearch.xpack.core.rollup.job.RollupJobConfig; import java.io.IOException; import java.util.Objects; @@ -22,19 +21,19 @@ public class FeatureIndexBuilderJob implements XPackPlugin.XPackPersistentTaskParams { public static final String NAME = "xpack/feature_index_builder/job"; - + private FeatureIndexBuilderJobConfig config; - + private static final ParseField CONFIG = new ParseField("config"); - - public static final ConstructingObjectParser PARSER - = new ConstructingObjectParser<>(NAME, a -> new FeatureIndexBuilderJob((FeatureIndexBuilderJobConfig) a[0])); + + public static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>(NAME, + a -> new FeatureIndexBuilderJob((FeatureIndexBuilderJobConfig) a[0])); static { - PARSER.declareObject(ConstructingObjectParser.constructorArg(), (p, c) -> FeatureIndexBuilderJobConfig.fromXContent(p, null), CONFIG); + PARSER.declareObject(ConstructingObjectParser.constructorArg(), (p, c) -> FeatureIndexBuilderJobConfig.fromXContent(p, null), + CONFIG); } - public FeatureIndexBuilderJob(FeatureIndexBuilderJobConfig config) { this.config = Objects.requireNonNull(config); } @@ -42,7 +41,7 @@ public FeatureIndexBuilderJob(FeatureIndexBuilderJobConfig config) { public FeatureIndexBuilderJob(StreamInput in) throws IOException { this.config = new FeatureIndexBuilderJobConfig(in); } - + @Override public String getWriteableName() { return NAME; @@ -69,11 +68,11 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws public FeatureIndexBuilderJobConfig getConfig() { return config; } - + public static FeatureIndexBuilderJob fromXContent(XContentParser parser) throws IOException { return PARSER.parse(parser, null); } - + @Override public boolean equals(Object other) { if (this == other) { @@ -88,7 +87,7 @@ public boolean equals(Object other) { return Objects.equals(this.config, that.config); } - + @Override public int hashCode() { return Objects.hash(config); diff --git a/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJobConfig.java b/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJobConfig.java index 94f41b7d9e667..f3dbb1ac36910 100644 --- a/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJobConfig.java +++ b/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJobConfig.java @@ -99,7 +99,8 @@ public String toString() { return Strings.toString(this, true, true); } - public static FeatureIndexBuilderJobConfig fromXContent(final XContentParser parser, @Nullable final String optionalJobId) throws IOException { + public static FeatureIndexBuilderJobConfig fromXContent(final XContentParser parser, @Nullable final String optionalJobId) + throws IOException { return PARSER.parse(parser, optionalJobId); } } diff --git a/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJobState.java b/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJobState.java index 7d47ff6ea79bc..5634910e6c529 100644 --- a/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJobState.java +++ b/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJobState.java @@ -28,9 +28,8 @@ public class FeatureIndexBuilderJobState implements Task.Status, PersistentTaskS private static final ParseField STATE = new ParseField("job_state"); - public static final ConstructingObjectParser PARSER = - new ConstructingObjectParser<>(NAME, - args -> new FeatureIndexBuilderJobState((IndexerState) args[0])); + public static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>(NAME, + args -> new FeatureIndexBuilderJobState((IndexerState) args[0])); static { PARSER.declareField(constructorArg(), p -> { @@ -96,6 +95,6 @@ public boolean equals(Object other) { @Override public int hashCode() { - return Objects.hash(state); + return Objects.hash(state); } } \ No newline at end of file diff --git a/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJobTask.java b/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJobTask.java index 90a190419579a..a4de3927e5bc1 100644 --- a/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJobTask.java +++ b/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJobTask.java @@ -31,13 +31,14 @@ public class FeatureIndexBuilderJobTask extends AllocatedPersistentTask implemen private final FeatureIndexBuilderIndexer indexer; static final String SCHEDULE_NAME = "xpack/feature_index_builder/job" + "/schedule"; - + public static class FeatureIndexBuilderJobPersistentTasksExecutor extends PersistentTasksExecutor { private final Client client; private final SchedulerEngine schedulerEngine; private final ThreadPool threadPool; - public FeatureIndexBuilderJobPersistentTasksExecutor(Settings settings, Client client, SchedulerEngine schedulerEngine, ThreadPool threadPool) { + public FeatureIndexBuilderJobPersistentTasksExecutor(Settings settings, Client client, SchedulerEngine schedulerEngine, + ThreadPool threadPool) { super(settings, "xpack/feature_index_builder/job", FeatureIndexBuilder.TASK_THREAD_POOL_NAME); this.client = client; this.schedulerEngine = schedulerEngine; @@ -47,17 +48,17 @@ public FeatureIndexBuilderJobPersistentTasksExecutor(Settings settings, Client c @Override protected void nodeOperation(AllocatedPersistentTask task, @Nullable FeatureIndexBuilderJob params, PersistentTaskState state) { FeatureIndexBuilderJobTask buildTask = (FeatureIndexBuilderJobTask) task; - SchedulerEngine.Job schedulerJob = new SchedulerEngine.Job(SCHEDULE_NAME + "_" + params.getConfig().getId(), - next()); + SchedulerEngine.Job schedulerJob = new SchedulerEngine.Job(SCHEDULE_NAME + "_" + params.getConfig().getId(), next()); - // Note that while the task is added to the scheduler here, the internal state will prevent + // Note that while the task is added to the scheduler here, the internal state + // will prevent // it from doing any work until the task is "started" via the StartJob api schedulerEngine.register(buildTask); schedulerEngine.add(schedulerJob); logger.info("FeatureIndexBuilder job [" + params.getConfig().getId() + "] created."); } - + static SchedulerEngine.Schedule next() { return (startTime, now) -> { return now + 1000; // to be fixed, hardcode something @@ -66,15 +67,14 @@ static SchedulerEngine.Schedule next() { @Override protected AllocatedPersistentTask createTask(long id, String type, String action, TaskId parentTaskId, - PersistentTasksCustomMetaData.PersistentTask persistentTask, - Map headers) { + PersistentTasksCustomMetaData.PersistentTask persistentTask, Map headers) { return new FeatureIndexBuilderJobTask(id, type, action, parentTaskId, persistentTask.getParams(), (FeatureIndexBuilderJobState) persistentTask.getState(), client, schedulerEngine, threadPool, headers); } } private final FeatureIndexBuilderJob job; - + public FeatureIndexBuilderJobTask(long id, String type, String action, TaskId parentTask, FeatureIndexBuilderJob job, FeatureIndexBuilderJobState state, Client client, SchedulerEngine schedulerEngine, ThreadPool threadPool, Map headers) { From 845b74b5ef82de79cac708c4042a79c1d215b8b9 Mon Sep 17 00:00:00 2001 From: Hendrik Muhs Date: Mon, 13 Aug 2018 13:01:08 +0200 Subject: [PATCH 5/6] fix rest tests --- docs/reference/rest-api/info.asciidoc | 5 +++++ .../xpack/ml/featureindexbuilder/FeatureIndexBuilder.java | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/reference/rest-api/info.asciidoc b/docs/reference/rest-api/info.asciidoc index 1cf4ab563b185..643b7007a2af4 100644 --- a/docs/reference/rest-api/info.asciidoc +++ b/docs/reference/rest-api/info.asciidoc @@ -62,6 +62,11 @@ Example response: "status" : "active" }, "features" : { + "fib" : { + "description" : "Time series feature index creation", + "available" : false, + "enabled" : true + }, "graph" : { "description" : "Graph Data Exploration for the Elastic Stack", "available" : false, diff --git a/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/FeatureIndexBuilder.java b/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/FeatureIndexBuilder.java index a216465bc0935..e209bd5e34038 100644 --- a/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/FeatureIndexBuilder.java +++ b/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/FeatureIndexBuilder.java @@ -56,7 +56,7 @@ public class FeatureIndexBuilder extends Plugin implements ActionPlugin, Persist public static final String NAME = "feature_index_builder"; public static final String BASE_PATH = "/_xpack/feature_index_builder/"; - public static final String TASK_THREAD_POOL_NAME = "feature_index_builder_indexing"; + public static final String TASK_THREAD_POOL_NAME = "ml_feature_index_builder_indexing"; // list of headers that will be stored when a job is created public static final Set HEADER_FILTERS = new HashSet<>( From 80d36ddb4e9522a0a091d8197508e5025c9a0743 Mon Sep 17 00:00:00 2001 From: Hendrik Muhs Date: Tue, 14 Aug 2018 10:18:44 +0200 Subject: [PATCH 6/6] address review comments --- .../job/FeatureIndexBuilderJobConfig.java | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJobConfig.java b/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJobConfig.java index f3dbb1ac36910..645a97ab8d928 100644 --- a/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJobConfig.java +++ b/x-pack/plugin/ml-feature-index-builder/src/main/java/org/elasticsearch/xpack/ml/featureindexbuilder/job/FeatureIndexBuilderJobConfig.java @@ -27,17 +27,18 @@ public class FeatureIndexBuilderJobConfig implements NamedWriteable, ToXContentObject { private static final String NAME = "xpack/feature_index_builder/jobconfig"; - private static final String ID = "id"; + private static final ParseField ID = new ParseField("id"); private final String id; - private static final ConstructingObjectParser PARSER; + private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>(NAME, false, + (args, optionalId) -> { + String id = args[0] != null ? (String) args[0] : optionalId; + return new FeatureIndexBuilderJobConfig(id); + }); + static { - PARSER = new ConstructingObjectParser<>(NAME, false, (args, optionalId) -> { - String id = args[0] != null ? (String) args[0] : optionalId; - return new FeatureIndexBuilderJobConfig(id); - }); - PARSER.declareString(optionalConstructorArg(), new ParseField(ID)); + PARSER.declareString(optionalConstructorArg(), ID); } public FeatureIndexBuilderJobConfig(final String id) { @@ -62,9 +63,7 @@ public void writeTo(final StreamOutput out) throws IOException { public XContentBuilder toXContent(final XContentBuilder builder, final Params params) throws IOException { builder.startObject(); - { - builder.field(ID, id); - } + builder.field(ID.getPreferredName(), id); builder.endObject(); return builder; }