Allow to update ttl field mapping after initial creation. Fixes #2136

Adding possibility to change TTL field mapper data without specifying enabled flag in mapping update

src/main/java/org/elasticsearch/index/mapper/internal/EnabledAttributeMapper.java

@@ -0,0 +1,33 @@
+/*
+ * Licensed to ElasticSearch and Shay Banon under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. ElasticSearch licenses this
+ * file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.elasticsearch.index.mapper.internal;
+
+public enum EnabledAttributeMapper {
+    ENABLED(true), UNSET_ENABLED(true), DISABLED(false), UNSET_DISABLED(false);
+
+    public final boolean enabled;
+
+    EnabledAttributeMapper(boolean enabled) {
+        this.enabled = enabled;
+    }
+
+    public boolean unset() {
+        return this == UNSET_DISABLED || this == UNSET_ENABLED;
+    }
+}

src/main/java/org/elasticsearch/index/mapper/internal/TTLFieldMapper.java

@@ -39,7 +39,6 @@ import java.io.IOException;
 import java.util.Date;
 import java.util.Map;
 
-import static org.elasticsearch.common.xcontent.support.XContentMapValues.nodeBooleanValue;
 import static org.elasticsearch.common.xcontent.support.XContentMapValues.nodeTimeValue;
 import static org.elasticsearch.index.mapper.core.TypeParsers.parseField;
 
@@ -60,21 +59,21 @@ public class TTLFieldMapper extends LongFieldMapper implements InternalMapper, R
             TTL_FIELD_TYPE.freeze();
         }
 
-        public static final boolean ENABLED = false;
+        public static final EnabledAttributeMapper ENABLED_STATE = EnabledAttributeMapper.DISABLED;
         public static final long DEFAULT = -1;
     }
 
     public static class Builder extends NumberFieldMapper.Builder<Builder, TTLFieldMapper> {
 
-        private boolean enabled = Defaults.ENABLED;
+        private EnabledAttributeMapper enabledState = EnabledAttributeMapper.UNSET_DISABLED;
         private long defaultTTL = Defaults.DEFAULT;
 
         public Builder() {
             super(Defaults.NAME, new FieldType(Defaults.TTL_FIELD_TYPE));
         }
 
-        public Builder enabled(boolean enabled) {
+        public Builder enabled(EnabledAttributeMapper enabled) {
-            this.enabled = enabled;
+            this.enabledState = enabled;
             return builder;
         }
 
@@ -85,7 +84,7 @@ public class TTLFieldMapper extends LongFieldMapper implements InternalMapper, R
 
         @Override
         public TTLFieldMapper build(BuilderContext context) {
-            return new TTLFieldMapper(fieldType, enabled, defaultTTL, ignoreMalformed(context), provider, fieldDataSettings);
+            return new TTLFieldMapper(fieldType, enabledState, defaultTTL, ignoreMalformed(context), provider, fieldDataSettings);
         }
     }
 
@@ -97,8 +96,8 @@ public class TTLFieldMapper extends LongFieldMapper implements InternalMapper, R
             for (Map.Entry<String, Object> entry : node.entrySet()) {
                 String fieldName = Strings.toUnderscoreCase(entry.getKey());
                 Object fieldNode = entry.getValue();
-                if (fieldName.equals("enabled")) {
+                if (fieldName.equals("enabled") && ("true".equals(fieldNode.toString()) || "yes".equals(fieldNode.toString()))) {
-                    builder.enabled(nodeBooleanValue(fieldNode));
+                    builder.enabled(EnabledAttributeMapper.ENABLED);
                 } else if (fieldName.equals("default")) {
                     TimeValue ttlTimeValue = nodeTimeValue(fieldNode, null);
                     if (ttlTimeValue != null) {
@@ -110,24 +109,24 @@ public class TTLFieldMapper extends LongFieldMapper implements InternalMapper, R
         }
     }
 
-    private boolean enabled;
+    private EnabledAttributeMapper enabledState;
     private long defaultTTL;
 
     public TTLFieldMapper() {
-        this(new FieldType(Defaults.TTL_FIELD_TYPE), Defaults.ENABLED, Defaults.DEFAULT, Defaults.IGNORE_MALFORMED, null, null);
+        this(new FieldType(Defaults.TTL_FIELD_TYPE), Defaults.ENABLED_STATE, Defaults.DEFAULT, Defaults.IGNORE_MALFORMED, null, null);
     }
 
-    protected TTLFieldMapper(FieldType fieldType, boolean enabled, long defaultTTL, Explicit<Boolean> ignoreMalformed,
+    protected TTLFieldMapper(FieldType fieldType, EnabledAttributeMapper enabled, long defaultTTL, Explicit<Boolean> ignoreMalformed,
                              PostingsFormatProvider provider, @Nullable Settings fieldDataSettings) {
         super(new Names(Defaults.NAME, Defaults.NAME, Defaults.NAME, Defaults.NAME), Defaults.PRECISION_STEP,
                 Defaults.BOOST, fieldType, Defaults.NULL_VALUE, ignoreMalformed,
                 provider, null, fieldDataSettings);
-        this.enabled = enabled;
+        this.enabledState = enabled;
         this.defaultTTL = defaultTTL;
     }
 
     public boolean enabled() {
-        return this.enabled;
+        return this.enabledState.enabled;
     }
 
     public long defaultTTL() {
@@ -189,7 +188,7 @@ public class TTLFieldMapper extends LongFieldMapper implements InternalMapper, R
 
     @Override
     protected Field innerParseCreateField(ParseContext context) throws IOException, AlreadyExpiredException {
-        if (enabled) {
+        if (enabledState.enabled) {
             long ttl = context.sourceToParse().ttl();
             if (ttl <= 0 && defaultTTL > 0) { // no ttl provided so we use the default value
                 ttl = defaultTTL;
@@ -213,12 +212,12 @@ public class TTLFieldMapper extends LongFieldMapper implements InternalMapper, R
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
         // if all are defaults, no sense to write it at all
-        if (enabled == Defaults.ENABLED && defaultTTL == Defaults.DEFAULT) {
+        if (enabledState == Defaults.ENABLED_STATE && defaultTTL == Defaults.DEFAULT) {
             return builder;
         }
         builder.startObject(CONTENT_TYPE);
-        if (enabled != Defaults.ENABLED) {
+        if (enabledState != Defaults.ENABLED_STATE) {
-            builder.field("enabled", enabled);
+            builder.field("enabled", enabledState.enabled);
         }
         if (defaultTTL != Defaults.DEFAULT) {
             builder.field("default", defaultTTL);
@@ -234,6 +233,9 @@ public class TTLFieldMapper extends LongFieldMapper implements InternalMapper, R
             if (ttlMergeWith.defaultTTL != -1) {
                 this.defaultTTL = ttlMergeWith.defaultTTL;
             }
+            if (ttlMergeWith.enabledState != enabledState && !ttlMergeWith.enabledState.unset()) {
+                this.enabledState = ttlMergeWith.enabledState;
+            }
         }
     }
 }

src/test/java/org/elasticsearch/test/unit/index/mapper/ttl/TTLMappingTests.java

@@ -70,7 +70,7 @@ public class TTLMappingTests {
     public void testDefaultValues() throws Exception {
         String mapping = XContentFactory.jsonBuilder().startObject().startObject("type").endObject().string();
         DocumentMapper docMapper = MapperTests.newParser().parse(mapping);
-        assertThat(docMapper.TTLFieldMapper().enabled(), equalTo(TTLFieldMapper.Defaults.ENABLED));
+        assertThat(docMapper.TTLFieldMapper().enabled(), equalTo(TTLFieldMapper.Defaults.ENABLED_STATE.enabled));
         assertThat(docMapper.TTLFieldMapper().fieldType().stored(), equalTo(TTLFieldMapper.Defaults.TTL_FIELD_TYPE.stored()));
         assertThat(docMapper.TTLFieldMapper().fieldType().indexed(), equalTo(TTLFieldMapper.Defaults.TTL_FIELD_TYPE.indexed()));
     }
@@ -88,4 +88,53 @@ public class TTLMappingTests {
         assertThat(docMapper.TTLFieldMapper().fieldType().stored(), equalTo(false));
         assertThat(docMapper.TTLFieldMapper().fieldType().indexed(), equalTo(false));
     }
+
+    @Test
+    public void testThatEnablingTTLFieldOnMergeWorks() throws Exception {
+        String mappingWithoutTtl = XContentFactory.jsonBuilder().startObject().startObject("type")
+                .startObject("properties").field("field").startObject().field("type", "string").endObject().endObject()
+                .endObject().endObject().string();
+
+        String mappingWithTtl = XContentFactory.jsonBuilder().startObject().startObject("type")
+                .startObject("_ttl")
+                .field("enabled", "yes").field("store", "no").field("index", "no")
+                .endObject()
+                .startObject("properties").field("field").startObject().field("type", "string").endObject().endObject()
+                .endObject().endObject().string();
+
+        DocumentMapper mapperWithoutTtl = MapperTests.newParser().parse(mappingWithoutTtl);
+        DocumentMapper mapperWithTtl = MapperTests.newParser().parse(mappingWithTtl);
+
+        DocumentMapper.MergeFlags mergeFlags = DocumentMapper.MergeFlags.mergeFlags().simulate(false);
+        DocumentMapper.MergeResult mergeResult = mapperWithoutTtl.merge(mapperWithTtl, mergeFlags);
+
+        assertThat(mergeResult.hasConflicts(), equalTo(false));
+        assertThat(mapperWithoutTtl.TTLFieldMapper().enabled(), equalTo(true));
+    }
+
+    @Test
+    public void testThatChangingTTLKeepsMapperEnabled() throws Exception {
+        String mappingWithTtl = XContentFactory.jsonBuilder().startObject().startObject("type")
+                .startObject("_ttl")
+                .field("enabled", "yes")
+                .endObject()
+                .startObject("properties").field("field").startObject().field("type", "string").endObject().endObject()
+                .endObject().endObject().string();
+
+        String updatedMapping = XContentFactory.jsonBuilder().startObject().startObject("type")
+                .startObject("_ttl")
+                .field("default", "1w")
+                .endObject()
+                .startObject("properties").field("field").startObject().field("type", "string").endObject().endObject()
+                .endObject().endObject().string();
+
+        DocumentMapper initialMapper = MapperTests.newParser().parse(mappingWithTtl);
+        DocumentMapper updatedMapper = MapperTests.newParser().parse(updatedMapping);
+
+        DocumentMapper.MergeFlags mergeFlags = DocumentMapper.MergeFlags.mergeFlags().simulate(false);
+        DocumentMapper.MergeResult mergeResult = initialMapper.merge(updatedMapper, mergeFlags);
+
+        assertThat(mergeResult.hasConflicts(), equalTo(false));
+        assertThat(initialMapper.TTLFieldMapper().enabled(), equalTo(true));
+    }
 }