mappings: keep parameters in mapping for _timestamp, _index and _size…

… even if disabled

Settings that are not default for _size, _index and _timestamp were only build in
toXContent if these fields were actually enabled.
_timestamp, _index and _size can be dynamically enabled or disabled.
Therfore the settings must be kept, even if the field is disabled.
(Dynamic enabling/disabling was intended, see TimestampFieldMapper.merge(..)
and SizeMappingTests#testThatDisablingWorksWhenMerging
but actually never worked, see below).

To avoid that _timestamp is overwritten by a default mapping
this commit also adds a check to mapping merging if the type is already
in the mapping. In this case the default is not applied anymore.
(see
SimpleTimestampTests#testThatUpdatingMappingShouldNotRemoveTimestampConfiguration)

As a side effect, this fixes
- overwriting of paramters from the _source field by default mappings
  (see DefaultSourceMappingTests).
- dynamic enabling and disabling of _timestamp and _size ()
  (see SimpleTimestampTests#testThatTimestampCanBeSwitchedOnAndOff and
  SizeMappingIntegrationTests#testThatTimestampCanBeSwitchedOnAndOff )

Tests:

Enable UpdateMappingOnClusterTests#test_doc_valuesInvalidMappingOnUpdate again
The missing settings in the mapping for _timestamp, _index and _size caused a the
failure: When creating a mapping which has settings other than default and the
field disabled, still empty field mappings were built from the type mappers.
When creating such a mapping, the mapping source on master and the rest of the cluster
can be out of sync for some time:

1. Master creates the index with source _timestamp:{_store:true}
   mapper classes are in a correct state but source is _timestamp:{}
2. Nodes update mapping and refresh source which then completely misses _timestamp
3. After a while source is refreshed again also on master and the _timestamp:{}
   vanishes there also.

The test UpdateMappingOnCusterTests#test_doc_valuesInvalidMappingOnUpdate failed
because the cluster state was sampled from master between 1. and 3. because the
randomized testing injected a default mapping with disabled _size and _timestamp
fields that have settings which are not default.

The test
TimestampMappingTests#testThatDisablingFieldMapperDoesNotReturnAnyUselessInfo
must be removed because it actualy expected the timestamp to remove
parameters when it was disabled.

closes #7137

src/main/java/org/elasticsearch/cluster/metadata/MetaDataMappingService.java

@@ -502,7 +502,7 @@ public ClusterState execute(final ClusterState currentState) throws Exception {
                             // _default_ types do not go through merging, but we do test the new settings. Also don't apply the old default
                             newMapper = indexService.mapperService().parse(request.type(), new CompressedString(request.source()), false);
                         } else {
-                            newMapper = indexService.mapperService().parse(request.type(), new CompressedString(request.source()));
+                            newMapper = indexService.mapperService().parse(request.type(), new CompressedString(request.source()), existingMapper == null);
                             if (existingMapper != null) {
                                 // first, simulate
                                 DocumentMapper.MergeResult mergeResult = existingMapper.merge(newMapper, mergeFlags().simulate(true));

src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java

@@ -394,6 +394,10 @@ public <T extends RootMapper> T rootMapper(Class<T> type) {
         return (T) rootMappers.get(type);
     }
 
+    public IndexFieldMapper indexMapper() {
+        return rootMapper(IndexFieldMapper.class);
+    }
+
     public TypeFieldMapper typeMapper() {
         return rootMapper(TypeFieldMapper.class);
     }
@@ -422,6 +426,10 @@ public ParentFieldMapper parentFieldMapper() {
         return rootMapper(ParentFieldMapper.class);
     }
 
+    public SizeFieldMapper sizeFieldMapper() {
+        return rootMapper(SizeFieldMapper.class);
+    }
+
     public TimestampFieldMapper timestampFieldMapper() {
         return rootMapper(TimestampFieldMapper.class);
     }

src/main/java/org/elasticsearch/index/mapper/MapperService.java

@@ -434,13 +434,6 @@ private void removeObjectAndFieldMappers(DocumentMapper docMapper) {
         }
     }
 
-    /**
-     * Just parses and returns the mapper without adding it, while still applying default mapping.
-     */
-    public DocumentMapper parse(String mappingType, CompressedString mappingSource) throws MapperParsingException {
-        return parse(mappingType, mappingSource, true);
-    }
-
     public DocumentMapper parse(String mappingType, CompressedString mappingSource, boolean applyDefault) throws MapperParsingException {
         String defaultMappingSource;
         if (PercolatorService.TYPE_NAME.equals(mappingType)) {

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

@@ -194,14 +194,14 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
         boolean includeDefaults = params.paramAsBoolean("include_defaults", false);
 
         // if all defaults, no need to write it at all
-        if (!includeDefaults && fieldType().stored() == Defaults.FIELD_TYPE.stored() && enabledState == Defaults.ENABLED_STATE) {
+        if (!includeDefaults && fieldType().stored() == Defaults.FIELD_TYPE.stored() && enabledState == Defaults.ENABLED_STATE && customFieldDataSettings == null) {
             return builder;
         }
         builder.startObject(CONTENT_TYPE);
-        if (includeDefaults || fieldType().stored() != Defaults.FIELD_TYPE.stored() && enabledState.enabled) {
+        if (includeDefaults || fieldType().stored() != Defaults.FIELD_TYPE.stored()) {
             builder.field("store", fieldType().stored());
         }
-        if (includeDefaults || enabledState.enabled != Defaults.ENABLED_STATE.enabled) {
+        if (includeDefaults || enabledState != Defaults.ENABLED_STATE) {
             builder.field("enabled", enabledState.enabled);
         }
 

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

@@ -161,10 +161,10 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
             return builder;
         }
         builder.startObject(contentType());
-        if (includeDefaults || enabledState.enabled != Defaults.ENABLED_STATE.enabled) {
+        if (includeDefaults || enabledState != Defaults.ENABLED_STATE) {
             builder.field("enabled", enabledState.enabled);
         }
-        if (includeDefaults || fieldType().stored() != Defaults.SIZE_FIELD_TYPE.stored() && enabledState.enabled) {
+        if (includeDefaults || fieldType().stored() != Defaults.SIZE_FIELD_TYPE.stored()) {
             builder.field("store", fieldType().stored());
         }
         builder.endObject();

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

@@ -246,32 +246,30 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
             return builder;
         }
         builder.startObject(CONTENT_TYPE);
-        if (includeDefaults || enabledState.enabled != Defaults.ENABLED.enabled) {
+        if (includeDefaults || enabledState != Defaults.ENABLED) {
             builder.field("enabled", enabledState.enabled);
         }
-        if (enabledState.enabled) {
-            if (includeDefaults || fieldType.indexed() != Defaults.FIELD_TYPE.indexed()) {
-                builder.field("index", indexTokenizeOptionToString(fieldType.indexed(), fieldType.tokenized()));
-            }
-            if (includeDefaults || fieldType.stored() != Defaults.FIELD_TYPE.stored()) {
-                builder.field("store", fieldType.stored());
-            }
-            if (includeDefaults || path != Defaults.PATH) {
-                builder.field("path", path);
-            }
-            if (includeDefaults || !dateTimeFormatter.format().equals(Defaults.DATE_TIME_FORMATTER.format())) {
-                builder.field("format", dateTimeFormatter.format());
-            }
-            if (includeDefaults || !Defaults.DEFAULT_TIMESTAMP.equals(defaultTimestamp)) {
-                builder.field("default", defaultTimestamp);
-            }
-            if (customFieldDataSettings != null) {
-                builder.field("fielddata", (Map) customFieldDataSettings.getAsMap());
-            } else if (includeDefaults) {
-                builder.field("fielddata", (Map) fieldDataType.getSettings().getAsMap());
-            }
-
+        if (includeDefaults || fieldType.indexed() != Defaults.FIELD_TYPE.indexed()) {
+            builder.field("index", indexTokenizeOptionToString(fieldType.indexed(), fieldType.tokenized()));
+        }
+        if (includeDefaults || fieldType.stored() != Defaults.FIELD_TYPE.stored()) {
+            builder.field("store", fieldType.stored());
+        }
+        if (includeDefaults || path != Defaults.PATH) {
+            builder.field("path", path);
+        }
+        if (includeDefaults || !dateTimeFormatter.format().equals(Defaults.DATE_TIME_FORMATTER.format())) {
+            builder.field("format", dateTimeFormatter.format());
         }
+        if (includeDefaults || !Defaults.DEFAULT_TIMESTAMP.equals(defaultTimestamp)) {
+            builder.field("default", defaultTimestamp);
+        }
+        if (customFieldDataSettings != null) {
+            builder.field("fielddata", (Map) customFieldDataSettings.getAsMap());
+        } else if (includeDefaults) {
+            builder.field("fielddata", (Map) fieldDataType.getSettings().getAsMap());
+        }
+
         builder.endObject();
         return builder;
     }

src/test/java/org/elasticsearch/index/mapper/index/IndexTypeMapperTests.java

@@ -111,4 +111,21 @@ public void testThatMergingFieldMappingAllowsDisabling() throws Exception {
         mapperEnabled.merge(mapperDisabled, DocumentMapper.MergeFlags.mergeFlags().simulate(false));
         assertThat(mapperEnabled.IndexFieldMapper().enabled(), is(false));
     }
+
+    @Test
+    public void testThatDisablingWorksWhenMerging() throws Exception {
+        String enabledMapping = XContentFactory.jsonBuilder().startObject().startObject("type")
+                .startObject("_index").field("enabled", true).endObject()
+                .endObject().endObject().string();
+        DocumentMapperParser parser = createIndex("test").mapperService().documentMapperParser();
+        DocumentMapper enabledMapper = parser.parse(enabledMapping);
+
+        String disabledMapping = XContentFactory.jsonBuilder().startObject().startObject("type")
+                .startObject("_index").field("enabled", false).endObject()
+                .endObject().endObject().string();
+        DocumentMapper disabledMapper = parser.parse(disabledMapping);
+
+        enabledMapper.merge(disabledMapper, DocumentMapper.MergeFlags.mergeFlags().simulate(false));
+        assertThat(enabledMapper.indexMapper().enabled(), is(false));
+    }
 }

src/test/java/org/elasticsearch/index/mapper/size/SizeMappingIntegrationTests.java

@@ -43,25 +43,44 @@ public void testThatUpdatingMappingShouldNotRemoveSizeMappingConfiguration() thr
         assertAcked(client().admin().indices().prepareCreate(index).addMapping(type, builder));
 
         // check mapping again
-        assertSizeMappingEnabled(index, type);
+        assertSizeMappingEnabled(index, type, true);
 
         // update some field in the mapping
         XContentBuilder updateMappingBuilder = jsonBuilder().startObject().startObject("properties").startObject("otherField").field("type", "string").endObject().endObject();
         PutMappingResponse putMappingResponse = client().admin().indices().preparePutMapping(index).setType(type).setSource(updateMappingBuilder).get();
         assertAcked(putMappingResponse);
 
-        // make sure timestamp field is still in mapping
-        assertSizeMappingEnabled(index, type);
+        // make sure size field is still in mapping
+        assertSizeMappingEnabled(index, type, true);
     }
 
-    private void assertSizeMappingEnabled(String index, String type) throws IOException {
-        String errMsg = String.format(Locale.ROOT, "Expected size field mapping to be enabled for %s/%s", index, type);
+    @Test
+    public void testThatSizeCanBeSwitchedOnAndOff() throws Exception {
+        String index = "foo";
+        String type = "mytype";
+
+        XContentBuilder builder = jsonBuilder().startObject().startObject("_size").field("enabled", true).endObject().endObject();
+        assertAcked(client().admin().indices().prepareCreate(index).addMapping(type, builder));
+
+        // check mapping again
+        assertSizeMappingEnabled(index, type, true);
+
+        // update some field in the mapping
+        XContentBuilder updateMappingBuilder = jsonBuilder().startObject().startObject("_size").field("enabled", false).endObject().endObject();
+        PutMappingResponse putMappingResponse = client().admin().indices().preparePutMapping(index).setType(type).setSource(updateMappingBuilder).get();
+        assertAcked(putMappingResponse);
+
+        // make sure size field is still in mapping
+        assertSizeMappingEnabled(index, type, false);
+    }
 
+    private void assertSizeMappingEnabled(String index, String type, boolean enabled) throws IOException {
+        String errMsg = String.format(Locale.ROOT, "Expected size field mapping to be " + (enabled ? "enabled" : "disabled") + " for %s/%s", index, type);
         GetMappingsResponse getMappingsResponse = client().admin().indices().prepareGetMappings(index).addTypes(type).get();
         Map<String, Object> mappingSource = getMappingsResponse.getMappings().get(index).get(type).getSourceAsMap();
         assertThat(errMsg, mappingSource, hasKey("_size"));
-        String ttlAsString = mappingSource.get("_size").toString();
-        assertThat(ttlAsString, is(notNullValue()));
-        assertThat(errMsg, ttlAsString, is("{enabled=true}"));
+        String sizeAsString = mappingSource.get("_size").toString();
+        assertThat(sizeAsString, is(notNullValue()));
+        assertThat(errMsg, sizeAsString, is("{enabled=" + (enabled) + "}"));
     }
 }

src/test/java/org/elasticsearch/index/mapper/source/DefaultSourceMappingTests.java

@@ -23,15 +23,20 @@
 import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.compress.CompressedString;
 import org.elasticsearch.common.compress.CompressorFactory;
+import org.elasticsearch.common.settings.ImmutableSettings;
+import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.common.xcontent.XContentType;
 import org.elasticsearch.index.mapper.*;
+import org.elasticsearch.index.service.IndexService;
 import org.elasticsearch.test.ElasticsearchSingleNodeTest;
 import org.junit.Test;
 
+import java.util.Arrays;
+import java.util.List;
 import java.util.Map;
 
-import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.*;
 
 /**
  *
@@ -203,4 +208,66 @@ public void testDefaultMappingAndWithMappingOverrideWithMapperService() throws E
         assertThat(mapper.type(), equalTo("my_type"));
         assertThat(mapper.sourceMapper().enabled(), equalTo(true));
     }
+
+    @Test
+    public void testParsingWithDefaultAppliedAndNotApplied() throws Exception {
+        String defaultMapping = XContentFactory.jsonBuilder().startObject().startObject(MapperService.DEFAULT_MAPPING)
+                .startObject("_source").array("includes", "default_field_path.").endObject()
+                .endObject().endObject().string();
+
+        MapperService mapperService = createIndex("test").mapperService();
+        mapperService.merge(MapperService.DEFAULT_MAPPING, new CompressedString(defaultMapping), true);
+
+        String mapping = XContentFactory.jsonBuilder().startObject().startObject("my_type")
+                .startObject("_source").array("includes", "custom_field_path.").endObject()
+                .endObject().endObject().string();
+        mapperService.merge("my_type", new CompressedString(mapping), true);
+        DocumentMapper mapper = mapperService.documentMapper("my_type");
+        assertThat(mapper.type(), equalTo("my_type"));
+        assertThat(mapper.sourceMapper().includes().length, equalTo(2));
+        assertThat(mapper.sourceMapper().includes(), hasItemInArray("default_field_path."));
+        assertThat(mapper.sourceMapper().includes(), hasItemInArray("custom_field_path."));
+
+        mapping = XContentFactory.jsonBuilder().startObject().startObject("my_type")
+                .startObject("properties").startObject("text").field("type", "string").endObject().endObject()
+                .endObject().endObject().string();
+        mapperService.merge("my_type", new CompressedString(mapping), false);
+        mapper = mapperService.documentMapper("my_type");
+        assertThat(mapper.type(), equalTo("my_type"));
+        assertThat(mapper.sourceMapper().includes(), hasItemInArray("default_field_path."));
+        assertThat(mapper.sourceMapper().includes(), hasItemInArray("custom_field_path."));
+        assertThat(mapper.sourceMapper().includes().length, equalTo(2));
+    }
+
+    public void testDefaultNotAppliedOnUpdate() throws Exception {
+        XContentBuilder defaultMapping = XContentFactory.jsonBuilder().startObject().startObject(MapperService.DEFAULT_MAPPING)
+                .startObject("_source").array("includes", "default_field_path.").endObject()
+                .endObject().endObject();
+
+        IndexService indexService = createIndex("test", ImmutableSettings.EMPTY, MapperService.DEFAULT_MAPPING, defaultMapping);
+
+        String mapping = XContentFactory.jsonBuilder().startObject().startObject("my_type")
+                .startObject("_source").array("includes", "custom_field_path.").endObject()
+                .endObject().endObject().string();
+        client().admin().indices().preparePutMapping("test").setType("my_type").setSource(mapping).get();
+
+        DocumentMapper mapper = indexService.mapperService().documentMapper("my_type");
+        assertThat(mapper.type(), equalTo("my_type"));
+        assertThat(mapper.sourceMapper().includes().length, equalTo(2));
+        List<String> includes = Arrays.asList(mapper.sourceMapper().includes());
+        assertThat("default_field_path.", isIn(includes));
+        assertThat("custom_field_path.", isIn(includes));
+
+        mapping = XContentFactory.jsonBuilder().startObject().startObject("my_type")
+                .startObject("properties").startObject("text").field("type", "string").endObject().endObject()
+                .endObject().endObject().string();
+        client().admin().indices().preparePutMapping("test").setType("my_type").setSource(mapping).get();
+
+        mapper = indexService.mapperService().documentMapper("my_type");
+        assertThat(mapper.type(), equalTo("my_type"));
+        includes = Arrays.asList(mapper.sourceMapper().includes());
+        assertThat("default_field_path.", isIn(includes));
+        assertThat("custom_field_path.", isIn(includes));
+        assertThat(mapper.sourceMapper().includes().length, equalTo(2));
+    }
 }

src/test/java/org/elasticsearch/index/mapper/timestamp/TimestampMappingTests.java

@@ -128,22 +128,6 @@ public void testThatDisablingDuringMergeIsWorking() throws Exception {
         assertThat(enabledMapper.timestampFieldMapper().enabled(), is(false));
     }
 
-    @Test
-    public void testThatDisablingFieldMapperDoesNotReturnAnyUselessInfo() throws Exception {
-        boolean inversedStoreSetting = !TimestampFieldMapper.Defaults.FIELD_TYPE.stored();
-        String mapping = XContentFactory.jsonBuilder().startObject().startObject("type")
-                .startObject("_timestamp").field("enabled", false).field("store", inversedStoreSetting).endObject()
-                .endObject().endObject().string();
-
-        DocumentMapper mapper = createIndex("test").mapperService().documentMapperParser().parse(mapping);
-
-        XContentBuilder builder = XContentFactory.jsonBuilder().startObject();
-        mapper.timestampFieldMapper().toXContent(builder, ToXContent.EMPTY_PARAMS);
-        builder.endObject();
-
-        assertThat(builder.string(), is(String.format(Locale.ROOT, "{\"%s\":{}}", TimestampFieldMapper.NAME)));
-    }
-
     @Test // issue 3174
     public void testThatSerializationWorksCorrectlyForIndexField() throws Exception {
         String enabledMapping = XContentFactory.jsonBuilder().startObject().startObject("type")

src/test/java/org/elasticsearch/index/mapper/update/UpdateMappingOnCusterTests.java

@@ -19,7 +19,6 @@
 
 package org.elasticsearch.index.mapper.update;
 
-import org.apache.lucene.util.LuceneTestCase;
 import org.elasticsearch.action.admin.indices.mapping.get.GetMappingsResponse;
 import org.elasticsearch.client.Client;
 import org.elasticsearch.common.xcontent.XContentBuilder;
@@ -77,7 +76,6 @@ public void test_doc_valuesInvalidMapping() throws Exception {
         }
     }
 
-    @LuceneTestCase.AwaitsFix(bugUrl = "")
     @Test
     public void test_doc_valuesInvalidMappingOnUpdate() throws Exception {
         String mapping = jsonBuilder().startObject().startObject(TYPE).startObject("properties").startObject("text").field("type", "string").endObject().endObject().endObject().string();
@@ -94,6 +92,17 @@ public void test_doc_valuesInvalidMappingOnUpdate() throws Exception {
         compareMappingOnNodes(mappingsBeforeUpdateResponse);
     }
 
+    // checks if the setting for timestamp and size are kept even if disabled
+    @Test
+    public void testDisabledSizeTimestampIndexDoNotLooseMappings() throws Exception {
+        String mapping = copyToStringFromClasspath("/org/elasticsearch/index/mapper/update/default_mapping_with_disabled_root_types.json");
+        prepareCreate(INDEX).addMapping(TYPE, mapping).get();
+        GetMappingsResponse mappingsBeforeGreen = client().admin().indices().prepareGetMappings(INDEX).addTypes(TYPE).get();
+        ensureGreen(INDEX);
+        // make sure all nodes have same cluster state
+        compareMappingOnNodes(mappingsBeforeGreen);
+    }
+
     protected void testConflict(String mapping, String mappingUpdate, String... errorMessages) throws InterruptedException {
         assertAcked(prepareCreate(INDEX).setSource(mapping).get());
         ensureGreen(INDEX);
@@ -110,11 +119,11 @@ protected void testConflict(String mapping, String mappingUpdate, String... erro
 
     }
 
-    private void compareMappingOnNodes(GetMappingsResponse mappingsBeforeUpdateResponse) {
+    private void compareMappingOnNodes(GetMappingsResponse previousMapping) {
         // make sure all nodes have same cluster state
         for (Client client : cluster()) {
-            GetMappingsResponse mappingsAfterUpdateResponse = client.admin().indices().prepareGetMappings(INDEX).addTypes(TYPE).setLocal(true).get();
-            assertThat(mappingsBeforeUpdateResponse.getMappings().get(INDEX).get(TYPE).source(), equalTo(mappingsAfterUpdateResponse.getMappings().get(INDEX).get(TYPE).source()));
+            GetMappingsResponse currentMapping = client.admin().indices().prepareGetMappings(INDEX).addTypes(TYPE).setLocal(true).get();
+            assertThat(previousMapping.getMappings().get(INDEX).get(TYPE).source(), equalTo(currentMapping.getMappings().get(INDEX).get(TYPE).source()));
         }
     }
 }