Remove DocumentMapperForType (#72616)

DocumentMapperForType is used to create a document mapper when no mappings exists for an index and we are indexing the first document in it. This is only to cover for the edge case of empty docs, without any fields to dynamically map, being indexed, as we need to ensure that any index with at least one document in it has some mappings.

We can replace using DocumentMapperForType with the same logic that MapperService#documentMapperWithAutoCreate includes. This also helps clean up the only case where we create a DocumentMapper from its public constructor, which can be removed and replaced by a more targeted static method.

Author: javanna

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

@@ -12,20 +12,21 @@
 import org.elasticsearch.index.IndexSettings;
 import org.elasticsearch.index.analysis.IndexAnalyzers;
 
-import java.util.Collections;
-
 public class DocumentMapper {
     private final String type;
     private final CompressedXContent mappingSource;
     private final MappingLookup mappingLookup;
 
-    public DocumentMapper(RootObjectMapper.Builder rootBuilder, MapperService mapperService) {
-        this(mapperService.getIndexSettings(), mapperService.getIndexAnalyzers(), mapperService.documentParser(),
-            new Mapping(
-                rootBuilder.build(new ContentPath(1)),
-                mapperService.getMetadataMappers(rootBuilder.name()).values().toArray(new MetadataFieldMapper[0]),
-                Collections.emptyMap()
-            ));
+    /**
+     * Create a new {@link DocumentMapper} that holds empty mappings.
+     * @param type the type of the mappings to create
+     * @param mapperService the mapper service that holds the needed components
+     * @return the newly created document mapper
+     */
+    public static DocumentMapper createEmpty(String type, MapperService mapperService) {
+        Mapping mapping = mapperService.parseMapping(type, null, true);
+        return new DocumentMapper(
+            mapperService.getIndexSettings(), mapperService.getIndexAnalyzers(), mapperService.documentParser(), mapping);
     }
 
     DocumentMapper(IndexSettings indexSettings,

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

@@ -582,20 +582,6 @@ public String resolveDocumentType(String type) {
         return type;
     }
 
-    /**
-     * Returns the document mapper created, including a mapping update if the
-     * type has been dynamically created.
-     */
-    public DocumentMapperForType documentMapperWithAutoCreate(String type) {
-        DocumentMapper mapper = documentMapper(type);
-        if (mapper != null) {
-            return new DocumentMapperForType(mapper, null);
-        }
-        Mapping mapping = parseMapping(type, null, true);
-        mapper = new DocumentMapper(indexSettings, indexAnalyzers, documentParser, mapping);
-        return new DocumentMapperForType(mapper, mapper.mapping());
-    }
-
     /**
      * Given the full name of a field, returns its {@link MappedFieldType}.
      */

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

@@ -99,7 +99,7 @@
 import org.elasticsearch.index.get.GetStats;
 import org.elasticsearch.index.get.ShardGetService;
 import org.elasticsearch.index.mapper.DateFieldMapper;
-import org.elasticsearch.index.mapper.DocumentMapperForType;
+import org.elasticsearch.index.mapper.DocumentMapper;
 import org.elasticsearch.index.mapper.IdFieldMapper;
 import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.index.mapper.MapperParsingException;
@@ -830,7 +830,7 @@ private Engine.IndexResult applyIndexOperation(Engine engine, long seqNo, long o
                 sourceWithResolvedType = new SourceToParse(sourceToParse.index(), resolvedType, sourceToParse.id(),
                     sourceToParse.source(), sourceToParse.getXContentType(), sourceToParse.routing(), sourceToParse.dynamicTemplates());
             }
-            operation = prepareIndex(docMapper(resolvedType), sourceWithResolvedType,
+            operation = prepareIndex(mapperService, resolvedType, sourceWithResolvedType,
                 seqNo, opPrimaryTerm, version, versionType, origin, autoGeneratedTimeStamp, isRetry, ifSeqNo, ifPrimaryTerm);
             Mapping update = operation.parsedDoc().dynamicMappingsUpdate();
             if (update != null) {
@@ -848,16 +848,24 @@ private Engine.IndexResult applyIndexOperation(Engine engine, long seqNo, long o
         return index(engine, operation);
     }
 
-    public static Engine.Index prepareIndex(DocumentMapperForType docMapper, SourceToParse source, long seqNo,
+    public static Engine.Index prepareIndex(MapperService mapperService, String type, SourceToParse source, long seqNo,
                                             long primaryTerm, long version, VersionType versionType, Engine.Operation.Origin origin,
                                             long autoGeneratedIdTimestamp, boolean isRetry,
                                             long ifSeqNo, long ifPrimaryTerm) {
         long startTime = System.nanoTime();
         assert source.dynamicTemplates().isEmpty() || origin == Engine.Operation.Origin.PRIMARY :
             "dynamic_templates parameter can only be associated with primary operations";
-        ParsedDocument doc = docMapper.getDocumentMapper().parse(source);
-        if (docMapper.getMapping() != null) {
-            doc.addDynamicMappingsUpdate(docMapper.getMapping());
+        DocumentMapper documentMapper = mapperService.documentMapper(type);
+        Mapping mapping = null;
+        if (documentMapper == null) {
+            documentMapper = DocumentMapper.createEmpty(type, mapperService);
+            mapping = documentMapper.mapping();
+        }
+        ParsedDocument doc = documentMapper.parse(source);
+        if (mapping != null) {
+            //If we are indexing but there is no mapping we create one. This is to ensure that whenever at least a document is indexed
+            // some mappings do exist. It covers for the case of indexing an empty doc (`{}`).
+            doc.addDynamicMappingsUpdate(mapping);
         }
         Term uid = new Term(IdFieldMapper.NAME, Uid.encodeId(doc.id()));
         return new Engine.Index(uid, doc, seqNo, primaryTerm, version, versionType, origin, startTime, autoGeneratedIdTimestamp, isRetry,
@@ -955,20 +963,21 @@ private Engine.DeleteResult applyDeleteOperation(Engine engine, long seqNo, long
         // In order to work around this issue, we make deletions create types. This way, we
         // fail if index and delete operations do not use the same type.
         // TODO: clean this up when types are gone
+        String resolvedType = mapperService.resolveDocumentType(type);
         try {
-            Mapping update = docMapper(type).getMapping();
-            if (update != null) {
-                return new Engine.DeleteResult(update);
+            DocumentMapper documentMapper = mapperService.documentMapper(resolvedType);
+            if (documentMapper == null) {
+                documentMapper = DocumentMapper.createEmpty(resolvedType, mapperService);
+                return new Engine.DeleteResult(documentMapper.mapping());
             }
         } catch (MapperParsingException | IllegalArgumentException | TypeMissingException e) {
             return new Engine.DeleteResult(e, version, getOperationPrimaryTerm(), seqNo, false);
         }
-        if (mapperService.resolveDocumentType(type).equals(mapperService.mappingLookup().getType()) == false) {
+        if (resolvedType.equals(mapperService.mappingLookup().getType()) == false) {
             // We should never get there due to the fact that we generate mapping updates on deletes,
             // but we still prefer to have a hard exception here as we would otherwise delete a
             // document in the wrong type.
-            throw new IllegalStateException("Deleting document from type [" +
-                    mapperService.resolveDocumentType(type) + "] while current type is [" +
+            throw new IllegalStateException("Deleting document from type [" + resolvedType + "] while current type is [" +
                     mapperService.mappingLookup().getType() + "]");
         }
         final Term uid = new Term(IdFieldMapper.NAME, Uid.encodeId(id));
@@ -978,11 +987,9 @@ private Engine.DeleteResult applyDeleteOperation(Engine engine, long seqNo, long
     }
 
     private Engine.Delete prepareDelete(String type, String id, Term uid, long seqNo, long primaryTerm, long version,
-                                               VersionType versionType, Engine.Operation.Origin origin,
-                                               long ifSeqNo, long ifPrimaryTerm) {
+                                        VersionType versionType, Engine.Operation.Origin origin, long ifSeqNo, long ifPrimaryTerm) {
         long startTime = System.nanoTime();
-        return new Engine.Delete(mapperService.resolveDocumentType(type), id, uid, seqNo, primaryTerm, version, versionType,
-            origin, startTime, ifSeqNo, ifPrimaryTerm);
+        return new Engine.Delete(type, id, uid, seqNo, primaryTerm, version, versionType, origin, startTime, ifSeqNo, ifPrimaryTerm);
     }
 
     private Engine.DeleteResult delete(Engine engine, Engine.Delete delete) throws IOException {
@@ -2920,11 +2927,6 @@ protected Analyzer getWrappedAnalyzer(String fieldName) {
         };
     }
 
-    private DocumentMapperForType docMapper(String type) {
-        return mapperService.documentMapperWithAutoCreate(
-            mapperService.resolveDocumentType(type));
-    }
-
     private EngineConfig newEngineConfig(LongSupplier globalCheckpointSupplier) {
         final Sort indexSort = indexSortSupplier.get();
         final Engine.Warmer warmer = reader -> {

server/src/main/java/org/elasticsearch/index/shard/IndexShard.java

@@ -31,7 +31,7 @@
 import org.elasticsearch.common.xcontent.support.XContentMapValues;
 import org.elasticsearch.index.engine.Engine;
 import org.elasticsearch.index.get.GetResult;
-import org.elasticsearch.index.mapper.DocumentMapperForType;
+import org.elasticsearch.index.mapper.DocumentMapper;
 import org.elasticsearch.index.mapper.IdFieldMapper;
 import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.index.mapper.MapperService;
@@ -329,13 +329,9 @@ public static String[] getValues(IndexableField[] fields) {
     private static ParsedDocument parseDocument(IndexShard indexShard, String index, String type, BytesReference doc,
                                                 XContentType xContentType, String routing) {
         MapperService mapperService = indexShard.mapperService();
-        DocumentMapperForType docMapper = mapperService.documentMapperWithAutoCreate(type);
-        ParsedDocument parsedDocument = docMapper.getDocumentMapper().parse(
-            new SourceToParse(index, type, "_id_for_tv_api", doc, xContentType, routing, Collections.emptyMap()));
-        if (docMapper.getMapping() != null) {
-            parsedDocument.addDynamicMappingsUpdate(docMapper.getMapping());
-        }
-        return parsedDocument;
+        DocumentMapper documentMapper = mapperService.documentMapper(mapperService.resolveDocumentType(type));
+        //TODO this throws NPE if there are no mappings yet, we have the same problem in PainlessExecutionAction
+        return documentMapper.parse(new SourceToParse(index, type, "_id_for_tv_api", doc, xContentType, routing, Collections.emptyMap()));
     }
 
     private static Fields mergeFields(Fields fields1, Fields fields2) throws IOException {

server/src/main/java/org/elasticsearch/index/termvectors/TermVectorsService.java

@@ -9,7 +9,6 @@
 package org.elasticsearch.index.engine;
 
 import org.apache.lucene.analysis.standard.StandardAnalyzer;
-import org.elasticsearch.Version;
 import org.elasticsearch.common.xcontent.NamedXContentRegistry;
 import org.elasticsearch.common.xcontent.XContentHelper;
 import org.elasticsearch.index.IndexSettings;
@@ -18,12 +17,9 @@
 import org.elasticsearch.index.analysis.AnalyzerScope;
 import org.elasticsearch.index.analysis.IndexAnalyzers;
 import org.elasticsearch.index.analysis.NamedAnalyzer;
-import org.elasticsearch.index.mapper.DocumentMapper;
-import org.elasticsearch.index.mapper.DocumentMapperForType;
 import org.elasticsearch.index.mapper.MapperRegistry;
 import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.index.mapper.Mapping;
-import org.elasticsearch.index.mapper.RootObjectMapper;
 import org.elasticsearch.index.mapper.SourceToParse;
 import org.elasticsearch.index.seqno.SequenceNumbers;
 import org.elasticsearch.index.shard.IndexShard;
@@ -62,11 +58,6 @@ public TranslogHandler(NamedXContentRegistry xContentRegistry, IndexSettings ind
                 () -> null, () -> false, null);
     }
 
-    private DocumentMapperForType docMapper(String type) {
-        RootObjectMapper.Builder rootBuilder = new RootObjectMapper.Builder(type, Version.CURRENT);
-        return new DocumentMapperForType(new DocumentMapper(rootBuilder, mapperService), mappingUpdate);
-    }
-
     private void applyOperation(Engine engine, Engine.Operation operation) throws IOException {
         switch (operation.operationType()) {
             case INDEX:
@@ -116,7 +107,7 @@ public Engine.Operation convertToEngineOp(Translog.Operation operation, Engine.O
             case INDEX:
                 final Translog.Index index = (Translog.Index) operation;
                 final String indexName = mapperService.index().getName();
-                final Engine.Index engineIndex = IndexShard.prepareIndex(docMapper(index.type()),
+                final Engine.Index engineIndex = IndexShard.prepareIndex(mapperService, index.type(),
                     new SourceToParse(indexName, index.type(), index.id(), index.source(), XContentHelper.xContentType(index.source()),
                         index.routing(), Collections.emptyMap()), index.seqNo(), index.primaryTerm(), index.version(), versionType, origin,
                     index.getAutoGeneratedIdTimestamp(), true, SequenceNumbers.UNASSIGNED_SEQ_NO, SequenceNumbers.UNASSIGNED_PRIMARY_TERM);