when changing the mapping of the _default_ mapping, do not apply the …

…old _default_ mapping to the new one and also do not validate the new version with a merge but parse is as a new type. Closes elastic#3474, Closes elastic#3476
mute · Aug 9, 2013 · 208e89e · 208e89e
1 parent b7ae046
commit 208e89e
Show file tree

Hide file tree

Showing 2 changed files with 80 additions and 9 deletions.
diff --git a/src/main/java/org/elasticsearch/cluster/metadata/MetaDataMappingService.java b/src/main/java/org/elasticsearch/cluster/metadata/MetaDataMappingService.java
@@ -386,18 +386,28 @@ public ClusterState execute(final ClusterState currentState) throws Exception {
                         IndexService indexService = indicesService.indexService(index);
                         if (indexService != null) {
                             // try and parse it (no need to add it here) so we can bail early in case of parsing exception
-                            DocumentMapper newMapper = indexService.mapperService().parse(request.mappingType, request.mappingSource);
-                            newMappers.put(index, newMapper);
+                            DocumentMapper newMapper;
                             DocumentMapper existingMapper = indexService.mapperService().documentMapper(request.mappingType);
-                            if (existingMapper != null) {
-                                // first, simulate
-                                DocumentMapper.MergeResult mergeResult = existingMapper.merge(newMapper, mergeFlags().simulate(true));
-                                // if we have conflicts, and we are not supposed to ignore them, throw an exception
-                                if (!request.ignoreConflicts && mergeResult.hasConflicts()) {
-                                    throw new MergeMappingException(mergeResult.conflicts());
+                            if (MapperService.DEFAULT_MAPPING.equals(request.mappingType)) {
+                                // _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.mappingType, request.mappingSource, false);
+                            } else {
+                                newMapper = indexService.mapperService().parse(request.mappingType, request.mappingSource);
+                                if (existingMapper != null) {
+                                    // first, simulate
+                                    DocumentMapper.MergeResult mergeResult = existingMapper.merge(newMapper, mergeFlags().simulate(true));
+                                    // if we have conflicts, and we are not supposed to ignore them, throw an exception
+                                    if (!request.ignoreConflicts && mergeResult.hasConflicts()) {
+                                        throw new MergeMappingException(mergeResult.conflicts());
+                                    }
                                 }
+                            }
+
+                            newMappers.put(index, newMapper);
+                            if (existingMapper != null) {
                                 existingMappers.put(index, existingMapper);
                             }
+
                         } else {
                             throw new IndexMissingException(new Index(index));
                         }

diff --git a/src/test/java/org/elasticsearch/test/integration/indices/mapping/UpdateMappingTests.java b/src/test/java/org/elasticsearch/test/integration/indices/mapping/UpdateMappingTests.java
@@ -1,15 +1,22 @@
 package org.elasticsearch.test.integration.indices.mapping;
 
+import org.elasticsearch.action.admin.cluster.state.ClusterStateResponse;
 import org.elasticsearch.action.admin.indices.mapping.put.PutMappingResponse;
 import org.elasticsearch.action.admin.indices.refresh.RefreshResponse;
 import org.elasticsearch.action.count.CountResponse;
 import org.elasticsearch.common.Priority;
 import org.elasticsearch.common.settings.ImmutableSettings;
+import org.elasticsearch.common.xcontent.json.JsonXContent;
+import org.elasticsearch.index.mapper.MapperParsingException;
+import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.index.mapper.MergeMappingException;
 import org.elasticsearch.test.integration.AbstractSharedClusterTest;
 import org.junit.Test;
 
-import static org.hamcrest.Matchers.equalTo;
+import java.util.Map;
+
+import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertThrows;
+import static org.hamcrest.Matchers.*;
 
 public class UpdateMappingTests extends AbstractSharedClusterTest {
 
@@ -100,4 +107,58 @@ public void updateMappingNoChanges() throws Exception {
         //no changes, we return
         assertThat(putMappingResponse.isAcknowledged(), equalTo(true));
     }
+
+
+    @SuppressWarnings("unchecked")
+    @Test
+    public void updateDefaultMappingSettings() throws Exception {
+        client().admin().indices().prepareCreate("test").addMapping(MapperService.DEFAULT_MAPPING,
+                JsonXContent.contentBuilder().startObject().startObject(MapperService.DEFAULT_MAPPING)
+                        .field("date_detection", false)
+                        .endObject().endObject()
+        ).get();
+
+        ClusterStateResponse response = client().admin().cluster().prepareState().get();
+
+        Map<String, Object> defaultMapping = response.getState().metaData().index("test").getMappings().get(MapperService.DEFAULT_MAPPING).sourceAsMap();
+        assertThat(defaultMapping, hasKey("date_detection"));
+
+
+        // now remove it
+        client().admin().indices().preparePutMapping("test").setType(MapperService.DEFAULT_MAPPING).setSource(
+                JsonXContent.contentBuilder().startObject().startObject(MapperService.DEFAULT_MAPPING)
+                        .endObject().endObject()
+        ).get();
+
+        response = client().admin().cluster().prepareState().get();
+        defaultMapping = response.getState().metaData().index("test").getMappings().get(MapperService.DEFAULT_MAPPING).sourceAsMap();
+        assertThat(defaultMapping, not(hasKey("date_detection")));
+
+        // now test you can change stuff that are normally unchangable
+        client().admin().indices().preparePutMapping("test").setType(MapperService.DEFAULT_MAPPING).setSource(
+                JsonXContent.contentBuilder().startObject().startObject(MapperService.DEFAULT_MAPPING)
+                        .startObject("properties").startObject("f").field("type", "string").field("index", "analyzed").endObject().endObject()
+                        .endObject().endObject()
+        ).get();
+
+
+        client().admin().indices().preparePutMapping("test").setType(MapperService.DEFAULT_MAPPING).setSource(
+                JsonXContent.contentBuilder().startObject().startObject(MapperService.DEFAULT_MAPPING)
+                        .startObject("properties").startObject("f").field("type", "string").field("index", "not_analyzed").endObject().endObject()
+                        .endObject().endObject()
+        ).get();
+        response = client().admin().cluster().prepareState().get();
+        defaultMapping = response.getState().metaData().index("test").getMappings().get(MapperService.DEFAULT_MAPPING).sourceAsMap();
+        Map<String, Object> fieldSettings = (Map<String, Object>) ((Map) defaultMapping.get("properties")).get("f");
+        assertThat(fieldSettings, hasEntry("index", (Object) "not_analyzed"));
+
+        // but we still validate the _default_ type
+        assertThrows(client().admin().indices().preparePutMapping("test").setType(MapperService.DEFAULT_MAPPING).setSource(
+                JsonXContent.contentBuilder().startObject().startObject(MapperService.DEFAULT_MAPPING)
+                        .startObject("properties").startObject("f").field("type", "DOESNT_EXIST").endObject().endObject()
+                        .endObject().endObject()
+        ), MapperParsingException.class);
+
+
+    }
 }