From b2106ed8f0aebca43221ac6337a9e4ff3486edef Mon Sep 17 00:00:00 2001
From: David Kyle <david.kyle@elastic.co>
Date: Wed, 24 Oct 2018 15:16:07 +0100
Subject: [PATCH] Prevent default job values overwriting nulled fields

---
 .../xpack/core/ml/job/config/Job.java            | 16 ++++++++++++++--
 .../xpack/core/ml/job/config/JobTests.java       |  2 ++
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/Job.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/Job.java
index 5a352ab26657c..66516c8f88c45 100644
--- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/Job.java
+++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/Job.java
@@ -90,7 +90,7 @@ public class Job extends AbstractDiffable<Job> implements Writeable, ToXContentO
     public static final long DEFAULT_MODEL_SNAPSHOT_RETENTION_DAYS = 1;
 
     private static ObjectParser<Builder, Void> createParser(boolean ignoreUnknownFields) {
-        ObjectParser<Builder, Void> parser = new ObjectParser<>("job_details", ignoreUnknownFields, Builder::new);
+        ObjectParser<Builder, Void> parser = new ObjectParser<>("job_details", ignoreUnknownFields, () -> new Builder(true));
 
         parser.declareString(Builder::setId, ID);
         parser.declareString(Builder::setJobType, JOB_TYPE);
@@ -641,7 +641,7 @@ public static class Builder implements Writeable, ToXContentObject {
         private ModelPlotConfig modelPlotConfig;
         private Long renormalizationWindowDays;
         private TimeValue backgroundPersistInterval;
-        private Long modelSnapshotRetentionDays = DEFAULT_MODEL_SNAPSHOT_RETENTION_DAYS;
+        private Long modelSnapshotRetentionDays;
         private Long resultsRetentionDays;
         private Map<String, Object> customSettings;
         private String modelSnapshotId;
@@ -649,10 +649,22 @@ public static class Builder implements Writeable, ToXContentObject {
         private String resultsIndexName;
         private boolean deleting;
 
+        private Builder(boolean ignoreDefaults) {
+            // Private constructor called by the parser to prevent default
+            // values being set. If a field with a default has explicitly
+            // been set to null then it isn't written in toXContent, hence
+            // won't be read by the parser and so won't be set back to null.
+            //
+            // The parameter isn't used it purely serves as a marker
+            // to differentiate between the different constructors
+        }
+
         public Builder() {
+            modelSnapshotRetentionDays = DEFAULT_MODEL_SNAPSHOT_RETENTION_DAYS;
         }
 
         public Builder(String id) {
+            this();
             this.id = id;
         }
 
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 4fa6617f045f6..862b8dae88645 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
@@ -632,6 +632,8 @@ public static Job createRandomizedJob() {
         }
         if (randomBoolean()) {
             builder.setModelSnapshotRetentionDays(randomNonNegativeLong());
+        } else {
+            builder.setModelSnapshotRetentionDays(null);
         }
         if (randomBoolean()) {
             builder.setResultsRetentionDays(randomNonNegativeLong());