From 5f4dc5433e6ca3d4b2e0902d758d402b112f88f5 Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Fri, 9 Aug 2013 19:54:06 +0200 Subject: [PATCH] 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 #3474, Closes #3476 --- .../metadata/MetaDataMappingService.java | 26 +++++--- .../indices/mapping/UpdateMappingTests.java | 62 ++++++++++++++++++- 2 files changed, 79 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/elasticsearch/cluster/metadata/MetaDataMappingService.java b/src/main/java/org/elasticsearch/cluster/metadata/MetaDataMappingService.java index a0ad9d3b039..f0d74a4a86f 100644 --- a/src/main/java/org/elasticsearch/cluster/metadata/MetaDataMappingService.java +++ b/src/main/java/org/elasticsearch/cluster/metadata/MetaDataMappingService.java @@ -387,18 +387,28 @@ public class MetaDataMappingService extends AbstractComponent { 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 index 803dc43f4e2..fbf7ae5b0ce 100644 --- 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.indices.mapping.get.GetMappingsResponse; 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,57 @@ public class UpdateMappingTests extends AbstractSharedClusterTest { //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(); + + GetMappingsResponse response = client().admin().indices().prepareGetMappings("test").addTypes(MapperService.DEFAULT_MAPPING).get(); + Map defaultMapping = response.getMappings().get("test").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().indices().prepareGetMappings("test").addTypes(MapperService.DEFAULT_MAPPING).get(); + defaultMapping = response.getMappings().get("test").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().indices().prepareGetMappings("test").addTypes(MapperService.DEFAULT_MAPPING).get(); + defaultMapping = response.getMappings().get("test").get(MapperService.DEFAULT_MAPPING).sourceAsMap(); + Map fieldSettings = (Map) ((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); + + + } }