diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataMappingService.java b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataMappingService.java index feec1833443a2..4dc38c97a8e38 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataMappingService.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/MetaDataMappingService.java @@ -265,6 +265,13 @@ private ClusterState applyRequest(ClusterState currentState, PutMappingClusterSt // try and parse it (no need to add it here) so we can bail early in case of parsing exception DocumentMapper newMapper; DocumentMapper existingMapper = mapperService.documentMapper(); + + String typeForUpdate = mapperService.getTypeForUpdate(mappingType, mappingUpdateSource); + if (existingMapper != null && existingMapper.type().equals(typeForUpdate) == false) { + throw new IllegalArgumentException("Rejecting mapping update to [" + mapperService.index().getName() + + "] as the final mapping would have more than 1 type: " + Arrays.asList(existingMapper.type(), typeForUpdate)); + } + if (MapperService.DEFAULT_MAPPING.equals(request.type())) { // _default_ types do not go through merging, but we do test the new settings. Also don't apply the old default newMapper = mapperService.parse(request.type(), mappingUpdateSource, false); @@ -299,14 +306,7 @@ private ClusterState applyRequest(ClusterState currentState, PutMappingClusterSt final Index index = indexMetaData.getIndex(); final MapperService mapperService = indexMapperServices.get(index); - // If the _type name is _doc and there is no _doc top-level key then this means that we - // are handling a typeless call. In such a case, we override _doc with the actual type - // name in the mappings. This allows to use typeless APIs on typed indices. - String typeForUpdate = mappingType; // the type to use to apply the mapping update - if (isMappingSourceTyped(request.type(), mappingUpdateSource) == false) { - typeForUpdate = mapperService.resolveDocumentType(mappingType); - } - + String typeForUpdate = mapperService.getTypeForUpdate(mappingType, mappingUpdateSource); CompressedXContent existingSource = null; DocumentMapper existingMapper = mapperService.documentMapper(typeForUpdate); if (existingMapper != null) { diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java index 90cbf40f924e8..997a03d731c53 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java @@ -21,7 +21,6 @@ import com.carrotsearch.hppc.ObjectHashSet; import com.carrotsearch.hppc.cursors.ObjectCursor; - import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.message.ParameterizedMessage; import org.apache.lucene.analysis.Analyzer; @@ -72,7 +71,6 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.Set; import java.util.function.Function; import java.util.function.Supplier; @@ -450,14 +448,6 @@ private synchronized Map internalMerge(@Nullable Documen results.put(DEFAULT_MAPPING, defaultMapper); } - { - if (mapper != null && this.mapper != null && Objects.equals(this.mapper.type(), mapper.type()) == false) { - throw new IllegalArgumentException( - "Rejecting mapping update to [" + index().getName() + "] as the final mapping would have more than 1 type: " - + Arrays.asList(this.mapper.type(), mapper.type())); - } - } - DocumentMapper newMapper = null; if (mapper != null) { // check naming @@ -705,6 +695,15 @@ public static boolean isMappingSourceTyped(String type, CompressedXContent mappi return isMappingSourceTyped(type, root); } + /** + * If the _type name is _doc and there is no _doc top-level key then this means that we + * are handling a typeless call. In such a case, we override _doc with the actual type + * name in the mappings. This allows to use typeless APIs on typed indices. + */ + public String getTypeForUpdate(String type, CompressedXContent mappingSource) { + return isMappingSourceTyped(type, mappingSource) == false ? resolveDocumentType(type) : type; + } + /** * Resolves a type from a mapping-related request into the type that should be used when * merging and updating mappings. diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataMappingServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataMappingServiceTests.java index 81fd5ec7fc83d..14d640c4ca351 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataMappingServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataMappingServiceTests.java @@ -19,11 +19,15 @@ package org.elasticsearch.cluster.metadata; +import org.elasticsearch.action.admin.indices.create.CreateIndexRequestBuilder; import org.elasticsearch.action.admin.indices.mapping.put.PutMappingClusterStateUpdateRequest; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.ClusterStateTaskExecutor; import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.compress.CompressedXContent; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.index.Index; import org.elasticsearch.index.IndexService; import org.elasticsearch.index.mapper.MapperService; @@ -34,6 +38,7 @@ import java.util.Collection; import java.util.Collections; +import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.not; @@ -134,4 +139,77 @@ public void testMappingUpdateAccepts_docAsType() throws Exception { Collections.singletonMap("foo", Collections.singletonMap("type", "keyword"))), mappingMetaData.sourceAsMap()); } + + public void testForbidMultipleTypes() throws Exception { + CreateIndexRequestBuilder createIndexRequest = client().admin().indices() + .prepareCreate("test") + .addMapping(MapperService.SINGLE_MAPPING_NAME); + IndexService indexService = createIndex("test", createIndexRequest); + + MetaDataMappingService mappingService = getInstanceFromNode(MetaDataMappingService.class); + ClusterService clusterService = getInstanceFromNode(ClusterService.class); + + PutMappingClusterStateUpdateRequest request = new PutMappingClusterStateUpdateRequest() + .type("other_type") + .indices(new Index[] {indexService.index()}) + .source(Strings.toString(XContentFactory.jsonBuilder() + .startObject() + .startObject("other_type").endObject() + .endObject())); + ClusterStateTaskExecutor.ClusterTasksResult result = + mappingService.putMappingExecutor.execute(clusterService.state(), Collections.singletonList(request)); + assertThat(result.executionResults.size(), equalTo(1)); + + ClusterStateTaskExecutor.TaskResult taskResult = result.executionResults.values().iterator().next(); + assertFalse(taskResult.isSuccess()); + assertThat(taskResult.getFailure().getMessage(), containsString( + "Rejecting mapping update to [test] as the final mapping would have more than 1 type: ")); + } + + /** + * This test checks that the multi-type validation is done before we do any other kind of validation + * on the mapping that's added, see https://github.com/elastic/elasticsearch/issues/29313 + */ + public void testForbidMultipleTypesWithConflictingMappings() throws Exception { + XContentBuilder mapping = XContentFactory.jsonBuilder().startObject() + .startObject(MapperService.SINGLE_MAPPING_NAME) + .startObject("properties") + .startObject("field1") + .field("type", "text") + .endObject() + .endObject() + .endObject() + .endObject(); + + CreateIndexRequestBuilder createIndexRequest = client().admin().indices() + .prepareCreate("test") + .addMapping(MapperService.SINGLE_MAPPING_NAME, mapping); + IndexService indexService = createIndex("test", createIndexRequest); + + MetaDataMappingService mappingService = getInstanceFromNode(MetaDataMappingService.class); + ClusterService clusterService = getInstanceFromNode(ClusterService.class); + + String conflictingMapping = Strings.toString(XContentFactory.jsonBuilder().startObject() + .startObject("other_type") + .startObject("properties") + .startObject("field1") + .field("type", "keyword") + .endObject() + .endObject() + .endObject() + .endObject()); + + PutMappingClusterStateUpdateRequest request = new PutMappingClusterStateUpdateRequest() + .type("other_type") + .indices(new Index[] {indexService.index()}) + .source(conflictingMapping); + ClusterStateTaskExecutor.ClusterTasksResult result = + mappingService.putMappingExecutor.execute(clusterService.state(), Collections.singletonList(request)); + assertThat(result.executionResults.size(), equalTo(1)); + + ClusterStateTaskExecutor.TaskResult taskResult = result.executionResults.values().iterator().next(); + assertFalse(taskResult.isSuccess()); + assertThat(taskResult.getFailure().getMessage(), containsString( + "Rejecting mapping update to [test] as the final mapping would have more than 1 type: ")); + } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java b/server/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java index 6bdfc167dec8b..3e02e2a6be059 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java @@ -57,7 +57,6 @@ import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.Matchers.instanceOf; -import static org.hamcrest.Matchers.startsWith; public class MapperServiceTests extends ESSingleNodeTestCase { @@ -298,36 +297,6 @@ public void testTotalFieldsLimitWithFieldAlias() throws Throwable { assertEquals("Limit of total fields [" + numberOfNonAliasFields + "] in index [test2] has been exceeded", e.getMessage()); } - public void testForbidMultipleTypes() throws IOException { - String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type").endObject().endObject()); - MapperService mapperService = createIndex("test").mapperService(); - mapperService.merge("type", new CompressedXContent(mapping), MergeReason.MAPPING_UPDATE); - - String mapping2 = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type2").endObject().endObject()); - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, - () -> mapperService.merge("type2", new CompressedXContent(mapping2), MergeReason.MAPPING_UPDATE)); - assertThat(e.getMessage(), startsWith("Rejecting mapping update to [test] as the final mapping would have more than 1 type: ")); - } - - /** - * This test checks that the multi-type validation is done before we do any other kind of validation on the mapping that's added, - * see https://github.com/elastic/elasticsearch/issues/29313 - */ - public void testForbidMultipleTypesWithConflictingMappings() throws IOException { - String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type") - .startObject("properties").startObject("field1").field("type", "integer_range") - .endObject().endObject().endObject().endObject()); - MapperService mapperService = createIndex("test").mapperService(); - mapperService.merge("type", new CompressedXContent(mapping), MergeReason.MAPPING_UPDATE); - - String mapping2 = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type2") - .startObject("properties").startObject("field1").field("type", "integer") - .endObject().endObject().endObject().endObject()); - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, - () -> mapperService.merge("type2", new CompressedXContent(mapping2), MergeReason.MAPPING_UPDATE)); - assertThat(e.getMessage(), startsWith("Rejecting mapping update to [test] as the final mapping would have more than 1 type: ")); - } - public void testDefaultMappingIsRejectedOn7() throws IOException { String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("_default_").endObject().endObject()); MapperService mapperService = createIndex("test").mapperService();