Remove typename checks in mapping updates (#47347)

This commit removes types validation during mapping updates. This will make
further work on types removal easier, as it will prevent test failures due to type-name
clashes when we remove type information from PutMapping and CreateIndex requests

Part of #41059

Author: romseygeek

rest-api-spec/src/main/resources/rest-api-spec/test/delete/70_mix_typeless_typeful.yml

@@ -19,15 +19,6 @@
           id:     1
           body:   { foo: bar }
 
- - do:
-      catch:   bad_request
-      delete:
-          index:  index
-          type:   some_random_type
-          id:     1
-
- - match:   { error.root_cause.0.reason:   "/Rejecting.mapping.update.to.\\[index\\].as.the.final.mapping.would.have.more.than.1.type.*/" }
-
  - do:
       delete:
           index:  index

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

@@ -43,17 +43,14 @@
 import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.index.mapper.MapperService.MergeReason;
 import org.elasticsearch.indices.IndicesService;
-import org.elasticsearch.indices.InvalidTypeNameException;
 
 import java.io.IOException;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
-import static org.elasticsearch.index.mapper.MapperService.isMappingSourceTyped;
 import static org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason.NO_LONGER_ASSIGNED;
 
 /**
@@ -245,7 +242,7 @@ class PutMappingExecutor implements ClusterStateTaskExecutor<PutMappingClusterSt
 
         private ClusterState applyRequest(ClusterState currentState, PutMappingClusterStateUpdateRequest request,
                                           Map<Index, MapperService> indexMapperServices) throws IOException {
-            String mappingType = request.type();
+
             CompressedXContent mappingUpdateSource = new CompressedXContent(request.source());
             final MetaData metaData = currentState.metaData();
             final List<IndexMetaData> updateList = new ArrayList<>();
@@ -260,32 +257,11 @@ private ClusterState applyRequest(ClusterState currentState, PutMappingClusterSt
                 updateList.add(indexMetaData);
                 // try and parse it (no need to add it here) so we can bail early in case of parsing exception
                 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));
-                }
-
                 DocumentMapper newMapper = mapperService.parse(request.type(), mappingUpdateSource);
                 if (existingMapper != null) {
                     // first, simulate: just call merge and ignore the result
                     existingMapper.merge(newMapper.mapping());
                 }
-
-
-                if (mappingType == null) {
-                    mappingType = newMapper.type();
-                } else if (mappingType.equals(newMapper.type()) == false
-                        && (isMappingSourceTyped(request.type(), mappingUpdateSource)
-                                || mapperService.resolveDocumentType(mappingType).equals(newMapper.type()) == false)) {
-                    throw new InvalidTypeNameException("Type name provided does not match type name within mapping definition.");
-                }
-            }
-            assert mappingType != null;
-
-            if (MapperService.SINGLE_MAPPING_NAME.equals(mappingType) == false
-                    && mappingType.charAt(0) == '_') {
-                throw new InvalidTypeNameException("Document mapping type name can't start with '_', found: [" + mappingType + "]");
             }
             MetaData.Builder builder = MetaData.builder(metaData);
             boolean updated = false;
@@ -296,13 +272,12 @@ private ClusterState applyRequest(ClusterState currentState, PutMappingClusterSt
                 final Index index = indexMetaData.getIndex();
                 final MapperService mapperService = indexMapperServices.get(index);
 
-                String typeForUpdate = mapperService.getTypeForUpdate(mappingType, mappingUpdateSource);
                 CompressedXContent existingSource = null;
-                DocumentMapper existingMapper = mapperService.documentMapper(typeForUpdate);
+                DocumentMapper existingMapper = mapperService.documentMapper();
                 if (existingMapper != null) {
                     existingSource = existingMapper.mappingSource();
                 }
-                DocumentMapper mergedMapper = mapperService.merge(typeForUpdate, mappingUpdateSource, MergeReason.MAPPING_UPDATE);
+                DocumentMapper mergedMapper = mapperService.merge(request.type(), mappingUpdateSource, MergeReason.MAPPING_UPDATE);
                 CompressedXContent updatedSource = mergedMapper.mappingSource();
 
                 if (existingSource != null) {
@@ -321,9 +296,9 @@ private ClusterState applyRequest(ClusterState currentState, PutMappingClusterSt
                 } else {
                     updatedMapping = true;
                     if (logger.isDebugEnabled()) {
-                        logger.debug("{} create_mapping [{}] with source [{}]", index, mappingType, updatedSource);
+                        logger.debug("{} create_mapping with source [{}]", index, updatedSource);
                     } else if (logger.isInfoEnabled()) {
-                        logger.info("{} create_mapping [{}]", index, mappingType);
+                        logger.info("{} create_mapping", index);
                     }
                 }
 

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

@@ -70,7 +70,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;
@@ -303,7 +302,8 @@ public void merge(IndexMetaData indexMetaData, MergeReason reason) {
     }
 
     public DocumentMapper merge(String type, CompressedXContent mappingSource, MergeReason reason) {
-        return internalMerge(Collections.singletonMap(type, mappingSource), reason).get(type);
+        // TODO change internalMerge() to return a single DocumentMapper rather than a Map
+        return internalMerge(Collections.singletonMap(type, mappingSource), reason).values().iterator().next();
     }
 
     private synchronized Map<String, DocumentMapper> internalMerge(IndexMetaData indexMetaData,
@@ -373,14 +373,6 @@ private synchronized Map<String, DocumentMapper> internalMerge(DocumentMapper ma
 
         Map<String, DocumentMapper> results = new LinkedHashMap<>(2);
 
-        {
-            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
@@ -619,15 +611,6 @@ 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.

server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataMappingServiceTests.java

@@ -19,15 +19,11 @@
 
 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;
@@ -38,7 +34,6 @@
 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;
 
@@ -140,76 +135,4 @@ public void testMappingUpdateAccepts_docAsType() throws Exception {
                         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<PutMappingClusterStateUpdateRequest> 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<PutMappingClusterStateUpdateRequest> 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: "));
-    }
 }

server/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java

@@ -54,7 +54,6 @@
 import java.util.Map;
 
 import static org.hamcrest.CoreMatchers.containsString;
-import static org.hamcrest.CoreMatchers.startsWith;
 import static org.hamcrest.Matchers.instanceOf;
 
 public class MapperServiceTests extends ESSingleNodeTestCase {
@@ -267,36 +266,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 testFieldNameLengthLimit() throws Throwable {
         int maxFieldNameLength = randomIntBetween(15, 20);
         String testString = new String(new char[maxFieldNameLength + 1]).replace("\0", "a");