DocumentMapperParser to no longer depend directly on MapperService (elastic#63850)

This change was mainly triggered by the need for `MapperService` to pass `this` during its constructor when creating `DocumentMapperParser`. Also, `MapperService` is carried around in some places where only a subset of it is needed.

With this change we rather carry around the components that are strictly needed, in a couple of cases functions that `MapperService` provides, which helps clarifying the dependency between `DocumentMapperParser`, `DocumentMapper` and `MapperService`, as well as removing the need for MapperService to pass `this` to `DocumentMapperParser`

Author: javanna

modules/percolator/src/test/java/org/elasticsearch/percolator/CandidateQueryTests.java

@@ -78,7 +78,6 @@
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.util.BytesRef;
 import org.elasticsearch.Version;
-import org.elasticsearch.cluster.metadata.IndexMetadata;
 import org.elasticsearch.common.CheckedFunction;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.bytes.BytesArray;
@@ -89,7 +88,6 @@
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.index.IndexService;
-import org.elasticsearch.index.IndexSettings;
 import org.elasticsearch.index.mapper.DocumentMapper;
 import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.index.mapper.MapperService;
@@ -1121,11 +1119,7 @@ private void duelRun(PercolateQuery.QueryStore queryStore, MemoryIndex memoryInd
     }
 
     private void addQuery(Query query, List<ParseContext.Document> docs) {
-        IndexMetadata build = IndexMetadata.builder("")
-            .settings(Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT))
-            .numberOfShards(1).numberOfReplicas(0).build();
-        IndexSettings settings = new IndexSettings(build, Settings.EMPTY);
-        ParseContext.InternalParseContext parseContext = new ParseContext.InternalParseContext(documentMapper, null, null);
+        ParseContext.InternalParseContext parseContext = new ParseContext.InternalParseContext(documentMapper, null, null, null);
         fieldMapper.processQuery(query, parseContext);
         ParseContext.Document queryDocument = parseContext.doc();
         // Add to string representation of the query to make debugging easier:

modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorFieldMapperTests.java

@@ -42,7 +42,6 @@
 import org.apache.lucene.util.BytesRef;
 import org.elasticsearch.Version;
 import org.elasticsearch.action.support.PlainActionFuture;
-import org.elasticsearch.cluster.metadata.IndexMetadata;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.bytes.BytesReference;
@@ -59,7 +58,6 @@
 import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.common.xcontent.XContentType;
 import org.elasticsearch.index.IndexService;
-import org.elasticsearch.index.IndexSettings;
 import org.elasticsearch.index.mapper.DocumentMapper;
 import org.elasticsearch.index.mapper.DocumentMapperParser;
 import org.elasticsearch.index.mapper.MapperParsingException;
@@ -183,11 +181,7 @@ public void testExtractTerms() throws Exception {
 
         DocumentMapper documentMapper = mapperService.documentMapper("doc");
         PercolatorFieldMapper fieldMapper = (PercolatorFieldMapper) documentMapper.mappers().getMapper(fieldName);
-        IndexMetadata build = IndexMetadata.builder("")
-            .settings(Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT))
-            .numberOfShards(1).numberOfReplicas(0).build();
-        IndexSettings settings = new IndexSettings(build, Settings.EMPTY);
-        ParseContext.InternalParseContext parseContext = new ParseContext.InternalParseContext(documentMapper, null, null);
+        ParseContext.InternalParseContext parseContext = new ParseContext.InternalParseContext(documentMapper, null, null, null);
         fieldMapper.processQuery(bq.build(), parseContext);
         ParseContext.Document document = parseContext.doc();
 
@@ -208,7 +202,7 @@ public void testExtractTerms() throws Exception {
         bq.add(termQuery1, Occur.MUST);
         bq.add(termQuery2, Occur.MUST);
 
-        parseContext = new ParseContext.InternalParseContext(documentMapper, null, null);
+        parseContext = new ParseContext.InternalParseContext(documentMapper, null, null, null);
         fieldMapper.processQuery(bq.build(), parseContext);
         document = parseContext.doc();
 
@@ -236,12 +230,8 @@ public void testExtractRanges() throws Exception {
         bq.add(rangeQuery2, Occur.MUST);
 
         DocumentMapper documentMapper = mapperService.documentMapper("doc");
-        IndexMetadata build = IndexMetadata.builder("")
-            .settings(Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT))
-            .numberOfShards(1).numberOfReplicas(0).build();
-        IndexSettings settings = new IndexSettings(build, Settings.EMPTY);
         PercolatorFieldMapper fieldMapper = (PercolatorFieldMapper) documentMapper.mappers().getMapper(fieldName);
-        ParseContext.InternalParseContext parseContext = new ParseContext.InternalParseContext(documentMapper, null, null);
+        ParseContext.InternalParseContext parseContext = new ParseContext.InternalParseContext(documentMapper, null, null, null);
         fieldMapper.processQuery(bq.build(), parseContext);
         ParseContext.Document document = parseContext.doc();
 
@@ -266,7 +256,7 @@ public void testExtractRanges() throws Exception {
             .rangeQuery(15, 20, true, true, null, null, null, context);
         bq.add(rangeQuery2, Occur.MUST);
 
-        parseContext = new ParseContext.InternalParseContext(documentMapper, null, null);
+        parseContext = new ParseContext.InternalParseContext(documentMapper, null, null, null);
         fieldMapper.processQuery(bq.build(), parseContext);
         document = parseContext.doc();
 
@@ -289,11 +279,7 @@ public void testExtractTermsAndRanges_failed() throws Exception {
         TermRangeQuery query = new TermRangeQuery("field1", new BytesRef("a"), new BytesRef("z"), true, true);
         DocumentMapper documentMapper = mapperService.documentMapper("doc");
         PercolatorFieldMapper fieldMapper = (PercolatorFieldMapper) documentMapper.mappers().getMapper(fieldName);
-        IndexMetadata build = IndexMetadata.builder("")
-            .settings(Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT))
-            .numberOfShards(1).numberOfReplicas(0).build();
-        IndexSettings settings = new IndexSettings(build, Settings.EMPTY);
-        ParseContext.InternalParseContext parseContext = new ParseContext.InternalParseContext(documentMapper, null, null);
+        ParseContext.InternalParseContext parseContext = new ParseContext.InternalParseContext(documentMapper, null, null, null);
         fieldMapper.processQuery(query, parseContext);
         ParseContext.Document document = parseContext.doc();
 
@@ -307,11 +293,7 @@ public void testExtractTermsAndRanges_partial() throws Exception {
         PhraseQuery phraseQuery = new PhraseQuery("field", "term");
         DocumentMapper documentMapper = mapperService.documentMapper("doc");
         PercolatorFieldMapper fieldMapper = (PercolatorFieldMapper) documentMapper.mappers().getMapper(fieldName);
-        IndexMetadata build = IndexMetadata.builder("")
-            .settings(Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT))
-            .numberOfShards(1).numberOfReplicas(0).build();
-        IndexSettings settings = new IndexSettings(build, Settings.EMPTY);
-        ParseContext.InternalParseContext parseContext = new ParseContext.InternalParseContext(documentMapper, null, null);
+        ParseContext.InternalParseContext parseContext = new ParseContext.InternalParseContext(documentMapper, null, null, null);
         fieldMapper.processQuery(phraseQuery, parseContext);
         ParseContext.Document document = parseContext.doc();
 

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

@@ -37,58 +37,45 @@
 import org.elasticsearch.index.IndexSettings;
 import org.elasticsearch.index.analysis.IndexAnalyzers;
 import org.elasticsearch.index.mapper.MapperService.MergeReason;
-import org.elasticsearch.index.mapper.MetadataFieldMapper.TypeParser;
 import org.elasticsearch.search.internal.SearchContext;
 
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.Collection;
-import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Objects;
+import java.util.function.Function;
 import java.util.stream.Stream;
 
 
 public class DocumentMapper implements ToXContentFragment {
 
     public static class Builder {
-
-        private final Map<Class<? extends MetadataFieldMapper>, MetadataFieldMapper> metadataMappers = new LinkedHashMap<>();
+        private final Map<Class<? extends MetadataFieldMapper>, MetadataFieldMapper> metadataMappers;
         private final RootObjectMapper rootObjectMapper;
         private final Mapper.BuilderContext builderContext;
         private final IndexSettings indexSettings;
         private final IndexAnalyzers indexAnalyzers;
-        private final DocumentMapperParser documentMapperParser;
         private final DocumentParser documentParser;
 
         private Map<String, Object> meta;
 
         public Builder(RootObjectMapper.Builder builder, MapperService mapperService) {
-            this.indexSettings = mapperService.getIndexSettings();
-            this.indexAnalyzers = mapperService.getIndexAnalyzers();
-            this.documentMapperParser = mapperService.documentMapperParser();
-            this.documentParser = mapperService.documentParser();
+            this(builder, mapperService.getIndexSettings(), mapperService.getIndexAnalyzers(), mapperService.documentParser(),
+                mapperService::getMetadataMappers);
+        }
+
+        Builder(RootObjectMapper.Builder builder,
+                IndexSettings indexSettings,
+                IndexAnalyzers indexAnalyzers,
+                DocumentParser documentParser,
+                Function<String, Map<Class<? extends MetadataFieldMapper>, MetadataFieldMapper>> metadataMappersFunction) {
+            this.indexSettings = indexSettings;
+            this.indexAnalyzers = indexAnalyzers;
+            this.documentParser = documentParser;
             this.builderContext = new Mapper.BuilderContext(indexSettings.getSettings(), new ContentPath(1));
             this.rootObjectMapper = builder.build(builderContext);
-            final String type = rootObjectMapper.name();
-            final DocumentMapper existingMapper = mapperService.documentMapper(type);
-            final Map<String, TypeParser> metadataMapperParsers =
-                mapperService.mapperRegistry.getMetadataMapperParsers(indexSettings.getIndexVersionCreated());
-            for (Map.Entry<String, MetadataFieldMapper.TypeParser> entry : metadataMapperParsers.entrySet()) {
-                final String name = entry.getKey();
-                final MetadataFieldMapper existingMetadataMapper = existingMapper == null
-                        ? null
-                        : (MetadataFieldMapper) existingMapper.mappers().getMapper(name);
-                final MetadataFieldMapper metadataMapper;
-                if (existingMetadataMapper == null) {
-                    final TypeParser parser = entry.getValue();
-                    metadataMapper = parser.getDefault(mapperService.fieldType(name),
-                            mapperService.documentMapperParser().parserContext());
-                } else {
-                    metadataMapper = existingMetadataMapper;
-                }
-                metadataMappers.put(metadataMapper.getClass(), metadataMapper);
-            }
+            this.metadataMappers = metadataMappersFunction.apply(rootObjectMapper.name());
         }
 
         public Builder meta(Map<String, Object> meta) {
@@ -109,7 +96,7 @@ public DocumentMapper build() {
                     rootObjectMapper,
                     metadataMappers.values().toArray(new MetadataFieldMapper[0]),
                     meta);
-            return new DocumentMapper(indexSettings, documentMapperParser, indexAnalyzers, documentParser, mapping);
+            return new DocumentMapper(indexSettings, indexAnalyzers, documentParser, mapping);
         }
     }
 
@@ -121,19 +108,16 @@ public DocumentMapper build() {
     private final MappingLookup fieldMappers;
     private final IndexSettings indexSettings;
     private final IndexAnalyzers indexAnalyzers;
-    private final DocumentMapperParser documentMapperParser;
     private final MetadataFieldMapper[] deleteTombstoneMetadataFieldMappers;
     private final MetadataFieldMapper[] noopTombstoneMetadataFieldMappers;
 
     private DocumentMapper(IndexSettings indexSettings,
-                           DocumentMapperParser documentMapperParser,
                            IndexAnalyzers indexAnalyzers,
                            DocumentParser documentParser,
                            Mapping mapping) {
         this.type = mapping.root().name();
         this.typeText = new Text(this.type);
         this.mapping = mapping;
-        this.documentMapperParser = documentMapperParser;
         this.documentParser = documentParser;
         this.indexSettings = indexSettings;
         this.indexAnalyzers = indexAnalyzers;
@@ -159,10 +143,6 @@ IndexSettings indexSettings() {
         return indexSettings;
     }
 
-    DocumentMapperParser documentMapperParser() {
-        return documentMapperParser;
-    }
-
     IndexAnalyzers indexAnalyzers() {
         return indexAnalyzers;
     }
@@ -291,7 +271,7 @@ public ObjectMapper findNestedObjectMapper(int nestedDocId, SearchContext sc, Le
 
     public DocumentMapper merge(Mapping mapping, MergeReason reason) {
         Mapping merged = this.mapping.merge(mapping, reason);
-        return new DocumentMapper(this.indexSettings, this.documentMapperParser, this.indexAnalyzers, this.documentParser, merged);
+        return new DocumentMapper(this.indexSettings, this.indexAnalyzers, this.documentParser, merged);
     }
 
     public void validate(IndexSettings settings, boolean checkLimits) {

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

@@ -23,64 +23,49 @@
 import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.collect.Tuple;
 import org.elasticsearch.common.compress.CompressedXContent;
-import org.elasticsearch.common.time.DateFormatter;
 import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
 import org.elasticsearch.common.xcontent.NamedXContentRegistry;
 import org.elasticsearch.common.xcontent.XContentHelper;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.XContentType;
 import org.elasticsearch.index.IndexSettings;
-import org.elasticsearch.index.query.QueryShardContext;
-import org.elasticsearch.index.similarity.SimilarityService;
-import org.elasticsearch.indices.mapper.MapperRegistry;
-import org.elasticsearch.script.ScriptService;
+import org.elasticsearch.index.analysis.IndexAnalyzers;
 
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
+import java.util.function.Function;
 import java.util.function.Supplier;
 
 import static java.util.Collections.unmodifiableMap;
 
 public class DocumentMapperParser {
-
-    final MapperService mapperService;
-    private final NamedXContentRegistry xContentRegistry;
-    private final SimilarityService similarityService;
-    private final Supplier<QueryShardContext> queryShardContextSupplier;
+    private final IndexSettings indexSettings;
+    private final IndexAnalyzers indexAnalyzers;
+    private final Function<String, String> documentTypeResolver;
+    private final DocumentParser documentParser;
+    private final Function<String, Map<Class<? extends MetadataFieldMapper>, MetadataFieldMapper>> metadataMappersFunction;
+    private final Supplier<Mapper.TypeParser.ParserContext> parserContextSupplier;
     private final RootObjectMapper.TypeParser rootObjectTypeParser = new RootObjectMapper.TypeParser();
-    private final Version indexVersionCreated;
-    private final Map<String, Mapper.TypeParser> typeParsers;
     private final Map<String, MetadataFieldMapper.TypeParser> rootTypeParsers;
-    private final ScriptService scriptService;
-
-    public DocumentMapperParser(IndexSettings indexSettings,
-                                MapperService mapperService,
-                                NamedXContentRegistry xContentRegistry,
-                                SimilarityService similarityService,
-                                MapperRegistry mapperRegistry,
-                                Supplier<QueryShardContext> queryShardContextSupplier,
-                                ScriptService scriptService) {
-        this.mapperService = mapperService;
-        this.xContentRegistry = xContentRegistry;
-        this.similarityService = similarityService;
-        this.queryShardContextSupplier = queryShardContextSupplier;
-        this.scriptService = scriptService;
-        this.typeParsers = mapperRegistry.getMapperParsers();
-        this.indexVersionCreated = indexSettings.getIndexVersionCreated();
-        this.rootTypeParsers = mapperRegistry.getMetadataMapperParsers(indexVersionCreated);
-    }
-
-    public Mapper.TypeParser.ParserContext parserContext() {
-        return new Mapper.TypeParser.ParserContext(similarityService::getSimilarity, typeParsers::get, indexVersionCreated,
-            queryShardContextSupplier, null, scriptService, mapperService.getIndexAnalyzers(), mapperService.getIndexSettings(),
-            mapperService::isIdFieldDataEnabled);
-    }
+    private final NamedXContentRegistry xContentRegistry;
 
-    public Mapper.TypeParser.ParserContext parserContext(DateFormatter dateFormatter) {
-        return new Mapper.TypeParser.ParserContext(similarityService::getSimilarity, typeParsers::get, indexVersionCreated,
-            queryShardContextSupplier, dateFormatter, scriptService, mapperService.getIndexAnalyzers(), mapperService.getIndexSettings(),
-            mapperService::isIdFieldDataEnabled);
+    DocumentMapperParser(IndexSettings indexSettings,
+                         IndexAnalyzers indexAnalyzers,
+                         Function<String, String> documentTypeResolver,
+                         DocumentParser documentParser,
+                         Function<String, Map<Class<? extends MetadataFieldMapper>, MetadataFieldMapper>> metadataMappersFunction,
+                         Supplier<Mapper.TypeParser.ParserContext> parserContextSupplier,
+                         Map<String, MetadataFieldMapper.TypeParser> metadataMapperParsers,
+                         NamedXContentRegistry xContentRegistry) {
+        this.indexSettings = indexSettings;
+        this.indexAnalyzers = indexAnalyzers;
+        this.documentTypeResolver = documentTypeResolver;
+        this.documentParser = documentParser;
+        this.metadataMappersFunction = metadataMappersFunction;
+        this.parserContextSupplier = parserContextSupplier;
+        this.rootTypeParsers = metadataMapperParsers;
+        this.xContentRegistry = xContentRegistry;
     }
 
     public DocumentMapper parse(@Nullable String type, CompressedXContent source) throws MapperParsingException {
@@ -114,11 +99,11 @@ private DocumentMapper parse(String type, Map<String, Object> mapping, String de
             }
         }
 
-
-        Mapper.TypeParser.ParserContext parserContext = parserContext();
+        Mapper.TypeParser.ParserContext parserContext = parserContextSupplier.get();
         // parse RootObjectMapper
-        DocumentMapper.Builder docBuilder = new DocumentMapper.Builder(
-                (RootObjectMapper.Builder) rootObjectTypeParser.parse(type, mapping, parserContext), mapperService);
+        RootObjectMapper.Builder root = (RootObjectMapper.Builder) rootObjectTypeParser.parse(type, mapping, parserContext);
+        DocumentMapper.Builder docBuilder = new DocumentMapper.Builder(root, indexSettings, indexAnalyzers, documentParser,
+            metadataMappersFunction);
         Iterator<Map.Entry<String, Object>> iterator = mapping.entrySet().iterator();
         // parse DocumentMapper
         while(iterator.hasNext()) {
@@ -206,7 +191,7 @@ private Tuple<String, Map<String, Object>> extractMapping(String type, Map<Strin
 
         String rootName = root.keySet().iterator().next();
         Tuple<String, Map<String, Object>> mapping;
-        if (type == null || type.equals(rootName) || mapperService.resolveDocumentType(type).equals(rootName)) {
+        if (type == null || type.equals(rootName) || documentTypeResolver.apply(type).equals(rootName)) {
             mapping = new Tuple<>(rootName, (Map<String, Object>) root.get(rootName));
         } else {
             mapping = new Tuple<>(type, root);