diff --git a/plugin/src/main/java/org/elasticsearch/xpack/ml/job/results/ModelPlot.java b/plugin/src/main/java/org/elasticsearch/xpack/ml/job/results/ModelPlot.java index 1a34194de7b..7027c550717 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/ml/job/results/ModelPlot.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/ml/job/results/ModelPlot.java @@ -87,7 +87,10 @@ public class ModelPlot implements ToXContentObject, Writeable { private double modelLower; private double modelUpper; private double modelMedian; - private double actual; + /** + * This can be null because buckets where no values were observed will still have a model, but no actual + */ + private Double actual; public ModelPlot(String jobId, Date timestamp, long bucketSpan) { this.jobId = jobId; @@ -121,7 +124,11 @@ public class ModelPlot implements ToXContentObject, Writeable { modelLower = in.readDouble(); modelUpper = in.readDouble(); modelMedian = in.readDouble(); - actual = in.readDouble(); + if (in.getVersion().before(Version.V_6_0_0_rc1)) { + actual = in.readDouble(); + } else { + actual = in.readOptionalDouble(); + } if (in.getVersion().onOrAfter(Version.V_5_5_0)) { bucketSpan = in.readLong(); } else { @@ -156,7 +163,17 @@ public class ModelPlot implements ToXContentObject, Writeable { out.writeDouble(modelLower); out.writeDouble(modelUpper); out.writeDouble(modelMedian); - out.writeDouble(actual); + if (out.getVersion().before(Version.V_6_0_0_rc1)) { + if (actual == null) { + // older versions cannot accommodate null, so we have no choice but to propagate the bug of + // https://github.com/elastic/x-pack-elasticsearch/issues/2528 + out.writeDouble(0.0); + } else { + out.writeDouble(actual); + } + } else { + out.writeOptionalDouble(actual); + } if (out.getVersion().onOrAfter(Version.V_5_5_0)) { out.writeLong(bucketSpan); } @@ -196,7 +213,9 @@ public class ModelPlot implements ToXContentObject, Writeable { builder.field(MODEL_LOWER.getPreferredName(), modelLower); builder.field(MODEL_UPPER.getPreferredName(), modelUpper); builder.field(MODEL_MEDIAN.getPreferredName(), modelMedian); - builder.field(ACTUAL.getPreferredName(), actual); + if (actual != null) { + builder.field(ACTUAL.getPreferredName(), actual); + } builder.endObject(); return builder; } @@ -302,11 +321,11 @@ public class ModelPlot implements ToXContentObject, Writeable { this.modelMedian = modelMedian; } - public double getActual() { + public Double getActual() { return actual; } - public void setActual(double actual) { + public void setActual(Double actual) { this.actual = actual; } @@ -331,7 +350,7 @@ public class ModelPlot implements ToXContentObject, Writeable { this.modelLower == that.modelLower && this.modelUpper == that.modelUpper && this.modelMedian == that.modelMedian && - this.actual == that.actual && + Objects.equals(this.actual, that.actual) && this.bucketSpan == that.bucketSpan; } diff --git a/plugin/src/test/java/org/elasticsearch/xpack/ml/job/results/ModelPlotTests.java b/plugin/src/test/java/org/elasticsearch/xpack/ml/job/results/ModelPlotTests.java index 0354a5933e4..c6e316ed11f 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/ml/job/results/ModelPlotTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/ml/job/results/ModelPlotTests.java @@ -6,12 +6,19 @@ package org.elasticsearch.xpack.ml.job.results; import org.elasticsearch.common.io.stream.Writeable.Reader; +import org.elasticsearch.common.xcontent.ToXContent; +import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.test.AbstractSerializingTestCase; +import java.io.IOException; import java.util.Date; import java.util.Objects; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.not; + public class ModelPlotTests extends AbstractSerializingTestCase { @Override @@ -172,6 +179,15 @@ public class ModelPlotTests extends AbstractSerializingTestCase { assertFalse(modelPlot2.equals(modelPlot1)); } + public void testEquals_GivenDifferentOneNullActual() { + ModelPlot modelPlot1 = createFullyPopulated(); + ModelPlot modelPlot2 = createFullyPopulated(); + modelPlot2.setActual(null); + + assertFalse(modelPlot1.equals(modelPlot2)); + assertFalse(modelPlot2.equals(modelPlot1)); + } + public void testEquals_GivenEqualmodelPlots() { ModelPlot modelPlot1 = createFullyPopulated(); ModelPlot modelPlot2 = createFullyPopulated(); @@ -181,6 +197,17 @@ public class ModelPlotTests extends AbstractSerializingTestCase { assertEquals(modelPlot1.hashCode(), modelPlot2.hashCode()); } + public void testToXContent_GivenNullActual() throws IOException { + XContentBuilder builder = JsonXContent.contentBuilder(); + + ModelPlot modelPlot = createFullyPopulated(); + modelPlot.setActual(null); + modelPlot.toXContent(builder, ToXContent.EMPTY_PARAMS); + + String json = builder.string(); + assertThat(json, not(containsString("actual"))); + } + public void testId() { ModelPlot plot = new ModelPlot("job-foo", new Date(100L), 60L);