From 36c9cf4d72bd78e3301700c321f8951c1d69478b Mon Sep 17 00:00:00 2001 From: Przemyslaw Witek Date: Tue, 2 Jun 2020 15:59:58 +0200 Subject: [PATCH 1/2] Introduce ModelPlotConfig.annotationsEnabled setting --- .../client/ml/job/config/ModelPlotConfig.java | 20 +++++++++---- .../MlClientDocumentationIT.java | 2 +- .../client/ml/job/config/JobTests.java | 2 +- .../client/ml/job/config/JobUpdateTests.java | 2 +- .../ml/job/config/ModelPlotConfigTests.java | 6 +++- .../anomaly-detection/apis/put-job.asciidoc | 4 +++ .../apis/update-job.asciidoc | 4 +++ docs/reference/ml/ml-shared.asciidoc | 5 ++++ .../core/ml/job/config/ModelPlotConfig.java | 30 +++++++++++++++---- .../ml/job/results/ReservedFieldNames.java | 1 + .../xpack/core/ml/config_index_mappings.json | 3 ++ .../UpdateProcessActionRequestTests.java | 3 +- .../xpack/core/ml/job/config/JobTests.java | 2 +- .../core/ml/job/config/JobUpdateTests.java | 6 ++-- .../ml/job/config/ModelPlotConfigTests.java | 12 ++++++-- .../xpack/ml/integration/ModelPlotsIT.java | 4 +-- .../writer/ModelPlotConfigWriter.java | 5 ++++ .../xpack/ml/job/config/JobBuilderTests.java | 5 ++-- .../AutodetectControlMsgWriterTests.java | 5 ++-- .../writer/ModelPlotConfigWriterTests.java | 28 +++++++++++++---- .../rest-api-spec/test/ml/jobs_crud.yml | 1 + 21 files changed, 115 insertions(+), 35 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/ml/job/config/ModelPlotConfig.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/ml/job/config/ModelPlotConfig.java index b39db054b308b..9674908fccc20 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/ml/job/config/ModelPlotConfig.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/ml/job/config/ModelPlotConfig.java @@ -30,22 +30,27 @@ public class ModelPlotConfig implements ToXContentObject { private static final ParseField TYPE_FIELD = new ParseField("model_plot_config"); private static final ParseField ENABLED_FIELD = new ParseField("enabled"); - public static final ParseField TERMS_FIELD = new ParseField("terms"); + private static final ParseField TERMS_FIELD = new ParseField("terms"); + private static final ParseField ANNOTATIONS_ENABLED_FIELD = new ParseField("annotations_enabled"); public static final ConstructingObjectParser PARSER = - new ConstructingObjectParser<>(TYPE_FIELD.getPreferredName(), true, a -> new ModelPlotConfig((boolean) a[0], (String) a[1])); + new ConstructingObjectParser<>( + TYPE_FIELD.getPreferredName(), true, a -> new ModelPlotConfig((boolean) a[0], (String) a[1], (Boolean) a[2])); static { PARSER.declareBoolean(ConstructingObjectParser.constructorArg(), ENABLED_FIELD); PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), TERMS_FIELD); + PARSER.declareBoolean(ConstructingObjectParser.optionalConstructorArg(), ANNOTATIONS_ENABLED_FIELD); } private final boolean enabled; private final String terms; + private final boolean annotationsEnabled; - public ModelPlotConfig(boolean enabled, String terms) { + public ModelPlotConfig(boolean enabled, String terms, Boolean annotationsEnabled) { this.enabled = enabled; this.terms = terms; + this.annotationsEnabled = Boolean.TRUE.equals(annotationsEnabled); } @Override @@ -55,6 +60,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws if (terms != null) { builder.field(TERMS_FIELD.getPreferredName(), terms); } + builder.field(ANNOTATIONS_ENABLED_FIELD.getPreferredName(), annotationsEnabled); builder.endObject(); return builder; } @@ -67,6 +73,10 @@ public String getTerms() { return this.terms; } + public boolean annotationsEnabled() { + return annotationsEnabled; + } + @Override public boolean equals(Object other) { if (this == other) { @@ -78,11 +88,11 @@ public boolean equals(Object other) { } ModelPlotConfig that = (ModelPlotConfig) other; - return this.enabled == that.enabled && Objects.equals(this.terms, that.terms); + return this.enabled == that.enabled && Objects.equals(this.terms, that.terms) && this.annotationsEnabled == that.annotationsEnabled; } @Override public int hashCode() { - return Objects.hash(enabled, terms); + return Objects.hash(enabled, terms, annotationsEnabled); } } diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/MlClientDocumentationIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/MlClientDocumentationIT.java index 27c4bd66c8825..96d993f3556dc 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/MlClientDocumentationIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/MlClientDocumentationIT.java @@ -590,7 +590,7 @@ public void testUpdateJob() throws Exception { .setDetectorUpdates(Arrays.asList(detectorUpdate)) // <6> .setGroups(Arrays.asList("job-group-1")) // <7> .setResultsRetentionDays(10L) // <8> - .setModelPlotConfig(new ModelPlotConfig(true, null)) // <9> + .setModelPlotConfig(new ModelPlotConfig(true, null, true)) // <9> .setModelSnapshotRetentionDays(7L) // <10> .setCustomSettings(customSettings) // <11> .setRenormalizationWindowDays(3L) // <12> diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/ml/job/config/JobTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/ml/job/config/JobTests.java index dcd7d4005d891..8f116be30df55 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/ml/job/config/JobTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/ml/job/config/JobTests.java @@ -133,7 +133,7 @@ public static Job.Builder createRandomizedJobBuilder() { builder.setDataDescription(dataDescription); if (randomBoolean()) { - builder.setModelPlotConfig(new ModelPlotConfig(randomBoolean(), randomAlphaOfLength(10))); + builder.setModelPlotConfig(ModelPlotConfigTests.createRandomized()); } if (randomBoolean()) { builder.setRenormalizationWindowDays(randomNonNegativeLong()); diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/ml/job/config/JobUpdateTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/ml/job/config/JobUpdateTests.java index dbe81550a6821..9780f1c82d21d 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/ml/job/config/JobUpdateTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/ml/job/config/JobUpdateTests.java @@ -56,7 +56,7 @@ public static JobUpdate createRandom(String jobId) { update.setDetectorUpdates(createRandomDetectorUpdates()); } if (randomBoolean()) { - update.setModelPlotConfig(new ModelPlotConfig(randomBoolean(), randomAlphaOfLength(10))); + update.setModelPlotConfig(ModelPlotConfigTests.createRandomized()); } if (randomBoolean()) { update.setAnalysisLimits(AnalysisLimitsTests.createRandomized()); diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/ml/job/config/ModelPlotConfigTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/ml/job/config/ModelPlotConfigTests.java index 50f1b49f41443..4e398ac5bc09c 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/ml/job/config/ModelPlotConfigTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/ml/job/config/ModelPlotConfigTests.java @@ -25,7 +25,11 @@ public class ModelPlotConfigTests extends AbstractXContentTestCase LENIENT_PARSER = createParser(true); @@ -28,35 +30,46 @@ public class ModelPlotConfig implements ToXContentObject, Writeable { private static ConstructingObjectParser createParser(boolean ignoreUnknownFields) { ConstructingObjectParser parser = new ConstructingObjectParser<>(TYPE_FIELD.getPreferredName(), - ignoreUnknownFields, a -> new ModelPlotConfig((boolean) a[0], (String) a[1])); + ignoreUnknownFields, a -> new ModelPlotConfig((boolean) a[0], (String) a[1], (Boolean) a[2])); parser.declareBoolean(ConstructingObjectParser.constructorArg(), ENABLED_FIELD); parser.declareString(ConstructingObjectParser.optionalConstructorArg(), TERMS_FIELD); + parser.declareBoolean(ConstructingObjectParser.optionalConstructorArg(), ANNOTATIONS_ENABLED_FIELD); return parser; } private final boolean enabled; private final String terms; + private final boolean annotationsEnabled; public ModelPlotConfig() { - this(true, null); + this(true, null, true); } - public ModelPlotConfig(boolean enabled, String terms) { + public ModelPlotConfig(boolean enabled, String terms, Boolean annotationsEnabled) { this.enabled = enabled; this.terms = terms; + this.annotationsEnabled = Boolean.TRUE.equals(annotationsEnabled); } public ModelPlotConfig(StreamInput in) throws IOException { enabled = in.readBoolean(); terms = in.readOptionalString(); + if (in.getVersion().onOrAfter(Version.V_8_0_0)) { + annotationsEnabled = in.readBoolean(); + } else { + annotationsEnabled = enabled; + } } @Override public void writeTo(StreamOutput out) throws IOException { out.writeBoolean(enabled); out.writeOptionalString(terms); + if (out.getVersion().onOrAfter(Version.V_8_0_0)) { + out.writeBoolean(annotationsEnabled); + } } @Override @@ -66,6 +79,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws if (terms != null) { builder.field(TERMS_FIELD.getPreferredName(), terms); } + builder.field(ANNOTATIONS_ENABLED_FIELD.getPreferredName(), annotationsEnabled); builder.endObject(); return builder; } @@ -75,7 +89,11 @@ public boolean isEnabled() { } public String getTerms() { - return this.terms; + return terms; + } + + public boolean annotationsEnabled() { + return annotationsEnabled; } @Override @@ -89,11 +107,11 @@ public boolean equals(Object other) { } ModelPlotConfig that = (ModelPlotConfig) other; - return this.enabled == that.enabled && Objects.equals(this.terms, that.terms); + return this.enabled == that.enabled && Objects.equals(this.terms, that.terms) && this.annotationsEnabled == that.annotationsEnabled; } @Override public int hashCode() { - return Objects.hash(enabled, terms); + return Objects.hash(enabled, terms, annotationsEnabled); } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/ReservedFieldNames.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/ReservedFieldNames.java index a91ea0dbdde19..17880b76dfe76 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/ReservedFieldNames.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/results/ReservedFieldNames.java @@ -277,6 +277,7 @@ public final class ReservedFieldNames { ModelPlotConfig.ENABLED_FIELD.getPreferredName(), ModelPlotConfig.TERMS_FIELD.getPreferredName(), + ModelPlotConfig.ANNOTATIONS_ENABLED_FIELD.getPreferredName(), DatafeedConfig.ID.getPreferredName(), DatafeedConfig.QUERY_DELAY.getPreferredName(), diff --git a/x-pack/plugin/core/src/main/resources/org/elasticsearch/xpack/core/ml/config_index_mappings.json b/x-pack/plugin/core/src/main/resources/org/elasticsearch/xpack/core/ml/config_index_mappings.json index b88bce4c9c02e..4d05d4e1915b4 100644 --- a/x-pack/plugin/core/src/main/resources/org/elasticsearch/xpack/core/ml/config_index_mappings.json +++ b/x-pack/plugin/core/src/main/resources/org/elasticsearch/xpack/core/ml/config_index_mappings.json @@ -325,6 +325,9 @@ }, "terms" : { "type" : "keyword" + }, + "annotations_enabled" : { + "type" : "boolean" } } }, diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/UpdateProcessActionRequestTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/UpdateProcessActionRequestTests.java index 0895086c83d1d..c8a331e408185 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/UpdateProcessActionRequestTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/action/UpdateProcessActionRequestTests.java @@ -11,6 +11,7 @@ import org.elasticsearch.xpack.core.ml.job.config.MlFilter; import org.elasticsearch.xpack.core.ml.job.config.MlFilterTests; import org.elasticsearch.xpack.core.ml.job.config.ModelPlotConfig; +import org.elasticsearch.xpack.core.ml.job.config.ModelPlotConfigTests; import java.util.ArrayList; import java.util.List; @@ -21,7 +22,7 @@ public class UpdateProcessActionRequestTests extends AbstractWireSerializingTest protected UpdateProcessAction.Request createTestInstance() { ModelPlotConfig config = null; if (randomBoolean()) { - config = new ModelPlotConfig(randomBoolean(), randomAlphaOfLength(10)); + config = ModelPlotConfigTests.createRandomized(); } List updates = null; if (randomBoolean()) { diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/JobTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/JobTests.java index b02e559f5fec2..94e3cae09e6d0 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/JobTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/JobTests.java @@ -640,7 +640,7 @@ public static Job createRandomizedJob() { builder.setDataDescription(dataDescription); if (randomBoolean()) { - builder.setModelPlotConfig(new ModelPlotConfig(randomBoolean(), randomAlphaOfLength(10))); + builder.setModelPlotConfig(ModelPlotConfigTests.createRandomized()); } if (randomBoolean()) { builder.setRenormalizationWindowDays(randomNonNegativeLong()); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/JobUpdateTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/JobUpdateTests.java index fdcb990957f7c..67ca39aca8e0c 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/JobUpdateTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/JobUpdateTests.java @@ -61,7 +61,7 @@ public JobUpdate createRandom(String jobId, @Nullable Job job) { update.setDetectorUpdates(detectorUpdates); } if (randomBoolean()) { - update.setModelPlotConfig(new ModelPlotConfig(randomBoolean(), randomAlphaOfLength(10))); + update.setModelPlotConfig(ModelPlotConfigTests.createRandomized()); } if (randomBoolean()) { update.setAnalysisLimits(AnalysisLimits.validateAndSetDefaults(AnalysisLimitsTests.createRandomized(), null, @@ -222,7 +222,7 @@ public void testMergeWithJob() { new RuleCondition(RuleCondition.AppliesTo.ACTUAL, Operator.GT, 5))).build()); detectorUpdates.add(new JobUpdate.DetectorUpdate(1, "description-2", detectionRules2)); - ModelPlotConfig modelPlotConfig = new ModelPlotConfig(randomBoolean(), randomAlphaOfLength(10)); + ModelPlotConfig modelPlotConfig = ModelPlotConfigTests.createRandomized(); AnalysisLimits analysisLimits = new AnalysisLimits(randomNonNegativeLong(), randomNonNegativeLong()); List categorizationFilters = Arrays.asList(generateRandomStringArray(10, 10, false)); Map customSettings = Collections.singletonMap(randomAlphaOfLength(10), randomAlphaOfLength(10)); @@ -300,7 +300,7 @@ public void testMergeWithJob_GivenRandomUpdates_AssertImmutability() { public void testIsAutodetectProcessUpdate() { JobUpdate update = new JobUpdate.Builder("foo").build(); assertFalse(update.isAutodetectProcessUpdate()); - update = new JobUpdate.Builder("foo").setModelPlotConfig(new ModelPlotConfig(true, "ff")).build(); + update = new JobUpdate.Builder("foo").setModelPlotConfig(new ModelPlotConfig(true, "ff", false)).build(); assertTrue(update.isAutodetectProcessUpdate()); update = new JobUpdate.Builder("foo").setDetectorUpdates(Collections.singletonList(mock(JobUpdate.DetectorUpdate.class))).build(); assertTrue(update.isAutodetectProcessUpdate()); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/ModelPlotConfigTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/ModelPlotConfigTests.java index c57f637d57225..4a9da2ef5ae51 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/ModelPlotConfigTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/ModelPlotConfigTests.java @@ -15,13 +15,19 @@ public class ModelPlotConfigTests extends AbstractSerializingTestCase { public void testConstructorDefaults() { - assertThat(new ModelPlotConfig().isEnabled(), is(true)); - assertThat(new ModelPlotConfig().getTerms(), is(nullValue())); + ModelPlotConfig modelPlotConfig = new ModelPlotConfig(); + assertThat(modelPlotConfig.isEnabled(), is(true)); + assertThat(modelPlotConfig.getTerms(), is(nullValue())); + assertThat(modelPlotConfig.annotationsEnabled(), is(true)); } @Override protected ModelPlotConfig createTestInstance() { - return new ModelPlotConfig(randomBoolean(), randomAlphaOfLengthBetween(1, 30)); + return createRandomized(); + } + + public static ModelPlotConfig createRandomized() { + return new ModelPlotConfig(randomBoolean(), randomAlphaOfLengthBetween(1, 30), randomBoolean()); } @Override diff --git a/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/ModelPlotsIT.java b/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/ModelPlotsIT.java index 5c80ce9034c33..7ecf4e528a0c1 100644 --- a/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/ModelPlotsIT.java +++ b/x-pack/plugin/ml/qa/native-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/ModelPlotsIT.java @@ -94,7 +94,7 @@ public void testPartitionFieldWithoutTerms() throws Exception { public void testPartitionFieldWithTerms() throws Exception { Job.Builder job = jobWithPartitionUser("model-plots-it-test-partition-field-with-terms"); - job.setModelPlotConfig(new ModelPlotConfig(true, "user_2,user_3")); + job.setModelPlotConfig(new ModelPlotConfig(true, "user_2,user_3", false)); registerJob(job); putJob(job); String datafeedId = job.getId() + "-feed"; @@ -116,7 +116,7 @@ public void testPartitionFieldWithTerms() throws Exception { public void testByFieldWithTerms() throws Exception { Job.Builder job = jobWithByUser("model-plots-it-test-by-field-with-terms"); - job.setModelPlotConfig(new ModelPlotConfig(true, "user_2,user_3")); + job.setModelPlotConfig(new ModelPlotConfig(true, "user_2,user_3", false)); registerJob(job); putJob(job); String datafeedId = job.getId() + "-feed"; diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/process/autodetect/writer/ModelPlotConfigWriter.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/process/autodetect/writer/ModelPlotConfigWriter.java index 7b7e51f3bb26d..8744bb789a591 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/process/autodetect/writer/ModelPlotConfigWriter.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/process/autodetect/writer/ModelPlotConfigWriter.java @@ -42,6 +42,11 @@ public void write() throws IOException { .append(terms == null ? "" : terms) .append(NEW_LINE); + contents.append(ModelPlotConfig.ANNOTATIONS_ENABLED_FIELD.getPreferredName()) + .append(EQUALS) + .append(modelPlotConfig.annotationsEnabled()) + .append(NEW_LINE); + writer.write(contents.toString()); } } diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/config/JobBuilderTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/config/JobBuilderTests.java index a728a7b1122f4..4b479d7871f4b 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/config/JobBuilderTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/config/JobBuilderTests.java @@ -13,7 +13,7 @@ import org.elasticsearch.xpack.core.ml.job.config.AnalysisLimitsTests; import org.elasticsearch.xpack.core.ml.job.config.DataDescription; import org.elasticsearch.xpack.core.ml.job.config.Job; -import org.elasticsearch.xpack.core.ml.job.config.ModelPlotConfig; +import org.elasticsearch.xpack.core.ml.job.config.ModelPlotConfigTests; import java.util.Collections; import java.util.Date; @@ -48,8 +48,7 @@ protected Job.Builder createTestInstance() { builder.setDataDescription(dataDescription); } if (randomBoolean()) { - builder.setModelPlotConfig(new ModelPlotConfig(randomBoolean(), - randomAlphaOfLength(10))); + builder.setModelPlotConfig(ModelPlotConfigTests.createRandomized()); } if (randomBoolean()) { builder.setRenormalizationWindowDays(randomNonNegativeLong()); diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/writer/AutodetectControlMsgWriterTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/writer/AutodetectControlMsgWriterTests.java index 0f9e1e859b5b8..221ae9dde7bea 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/writer/AutodetectControlMsgWriterTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/writer/AutodetectControlMsgWriterTests.java @@ -178,12 +178,13 @@ public void testWriteResetBucketsMessage() throws IOException { public void testWriteUpdateModelPlotMessage() throws IOException { AutodetectControlMsgWriter writer = new AutodetectControlMsgWriter(lengthEncodedWriter, 4); - writer.writeUpdateModelPlotMessage(new ModelPlotConfig(true, "foo,bar")); + writer.writeUpdateModelPlotMessage(new ModelPlotConfig(true, "foo,bar", false)); InOrder inOrder = inOrder(lengthEncodedWriter); inOrder.verify(lengthEncodedWriter).writeNumFields(4); inOrder.verify(lengthEncodedWriter, times(3)).writeField(""); - inOrder.verify(lengthEncodedWriter).writeField("u[modelPlotConfig]\nboundspercentile = 95.0\nterms = foo,bar\n"); + inOrder.verify(lengthEncodedWriter) + .writeField("u[modelPlotConfig]\nboundspercentile = 95.0\nterms = foo,bar\nannotations_enabled = false\n"); verifyNoMoreInteractions(lengthEncodedWriter); } diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/writer/ModelPlotConfigWriterTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/writer/ModelPlotConfigWriterTests.java index 4440c06b67381..4682658b43df6 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/writer/ModelPlotConfigWriterTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/writer/ModelPlotConfigWriterTests.java @@ -36,24 +36,42 @@ public void testWrite_GivenEnabledConfigWithoutTerms() throws IOException { writer.write(); - verify(this.writer).write("boundspercentile = 95.0\nterms = \n"); + verify(this.writer).write("boundspercentile = 95.0\nterms = \nannotations_enabled = true\n"); } public void testWrite_GivenEnabledConfigWithTerms() throws IOException { - ModelPlotConfig modelPlotConfig = new ModelPlotConfig(true, "foo,bar"); + ModelPlotConfig modelPlotConfig = new ModelPlotConfig(true, "foo,bar", false); ModelPlotConfigWriter writer = new ModelPlotConfigWriter(modelPlotConfig, this.writer); writer.write(); - verify(this.writer).write("boundspercentile = 95.0\nterms = foo,bar\n"); + verify(this.writer).write("boundspercentile = 95.0\nterms = foo,bar\nannotations_enabled = false\n"); } public void testWrite_GivenDisabledConfigWithTerms() throws IOException { - ModelPlotConfig modelPlotConfig = new ModelPlotConfig(false, "foo,bar"); + ModelPlotConfig modelPlotConfig = new ModelPlotConfig(false, "foo,bar", false); ModelPlotConfigWriter writer = new ModelPlotConfigWriter(modelPlotConfig, this.writer); writer.write(); - verify(this.writer).write("boundspercentile = -1.0\nterms = foo,bar\n"); + verify(this.writer).write("boundspercentile = -1.0\nterms = foo,bar\nannotations_enabled = false\n"); + } + + public void testWrite_GivenEnabledConfigWithEnabledAnnotations() throws IOException { + ModelPlotConfig modelPlotConfig = new ModelPlotConfig(true, null, true); + ModelPlotConfigWriter writer = new ModelPlotConfigWriter(modelPlotConfig, this.writer); + + writer.write(); + + verify(this.writer).write("boundspercentile = 95.0\nterms = \nannotations_enabled = true\n"); + } + + public void testWrite_GivenDisabledConfigWithEnabledAnnotations() throws IOException { + ModelPlotConfig modelPlotConfig = new ModelPlotConfig(false, null, true); + ModelPlotConfigWriter writer = new ModelPlotConfigWriter(modelPlotConfig, this.writer); + + writer.write(); + + verify(this.writer).write("boundspercentile = -1.0\nterms = \nannotations_enabled = true\n"); } } diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/jobs_crud.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/jobs_crud.yml index b5cfa75a5c2c0..fe2ced7bd7895 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/jobs_crud.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/jobs_crud.yml @@ -393,6 +393,7 @@ - match: { description: "Post update description" } - match: { model_plot_config.enabled: false } - match: { model_plot_config.terms: "foobar" } + - match: { model_plot_config.annotations_enabled: false } - match: { analysis_config.categorization_filters: ["cat3.*"] } - match: { analysis_config.detectors.0.custom_rules.0.actions: ["skip_result"] } - length: { analysis_config.detectors.0.custom_rules.0.conditions: 1 } From f0c6280646d3b971cbf54b2262df621983ac1ad0 Mon Sep 17 00:00:00 2001 From: Przemyslaw Witek Date: Wed, 3 Jun 2020 14:33:08 +0200 Subject: [PATCH 2/2] Apply review comments --- .../client/ml/job/config/ModelPlotConfigTests.java | 2 +- .../xpack/core/ml/job/config/ModelPlotConfigTests.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/ml/job/config/ModelPlotConfigTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/ml/job/config/ModelPlotConfigTests.java index 4e398ac5bc09c..5e1e601983871 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/ml/job/config/ModelPlotConfigTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/ml/job/config/ModelPlotConfigTests.java @@ -29,7 +29,7 @@ protected ModelPlotConfig createTestInstance() { } public static ModelPlotConfig createRandomized() { - return new ModelPlotConfig(randomBoolean(), randomAlphaOfLengthBetween(1, 30), randomBoolean()); + return new ModelPlotConfig(randomBoolean(), randomAlphaOfLengthBetween(1, 30), randomBoolean() ? randomBoolean() : null); } @Override diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/ModelPlotConfigTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/ModelPlotConfigTests.java index 4a9da2ef5ae51..9009dfedd1719 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/ModelPlotConfigTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/ModelPlotConfigTests.java @@ -27,7 +27,7 @@ protected ModelPlotConfig createTestInstance() { } public static ModelPlotConfig createRandomized() { - return new ModelPlotConfig(randomBoolean(), randomAlphaOfLengthBetween(1, 30), randomBoolean()); + return new ModelPlotConfig(randomBoolean(), randomAlphaOfLengthBetween(1, 30), randomBoolean() ? randomBoolean() : null); } @Override