Simplify DocumentParser handling (#63800)

DocumentMapperParser holds a reference to the xcontent registry, only with the purpose of being retrieved in DocumentParser#parsedDocument .

This can be greatly simplified by making DocumentParser take the registry as a constructor argument, and removing the same reference from DocumentMapperParser. This may also allow for further refactorings around how DocumentMapperParser is used. With this change it is MapperService that creates DocumentParser and exposes it so that DocumentMapper can use it.

Author: javanna

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

@@ -59,13 +59,15 @@ public static class Builder {
         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.builderContext = new Mapper.BuilderContext(indexSettings.getSettings(), new ContentPath(1));
             this.rootObjectMapper = builder.build(builderContext);
 
@@ -106,7 +108,7 @@ public DocumentMapper build() {
                     rootObjectMapper,
                     metadataMappers.values().toArray(new MetadataFieldMapper[0]),
                     meta);
-            return new DocumentMapper(indexSettings, documentMapperParser, indexAnalyzers, mapping);
+            return new DocumentMapper(indexSettings, documentMapperParser, indexAnalyzers, documentParser, mapping);
         }
     }
 
@@ -125,14 +127,15 @@ public DocumentMapper build() {
     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;
-        this.documentParser = new DocumentParser();
         this.fieldMappers = MappingLookup.fromMapping(this.mapping, indexAnalyzers.getDefaultIndexAnalyzer());
 
         try {
@@ -279,7 +282,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, merged);
+        return new DocumentMapper(this.indexSettings, this.documentMapperParser, this.indexAnalyzers, this.documentParser, merged);
     }
 
     public void validate(IndexSettings settings, boolean checkLimits) {

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

@@ -23,7 +23,6 @@
 import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.compress.CompressedXContent;
 import org.elasticsearch.common.time.DateFormatter;
-import org.elasticsearch.common.xcontent.NamedXContentRegistry;
 import org.elasticsearch.common.xcontent.XContentHelper;
 import org.elasticsearch.common.xcontent.XContentType;
 import org.elasticsearch.index.IndexSettings;
@@ -41,23 +40,21 @@
 public class DocumentMapperParser {
 
     final MapperService mapperService;
-    private final NamedXContentRegistry xContentRegistry;
     private final SimilarityService similarityService;
     private final Supplier<QueryShardContext> queryShardContextSupplier;
-
     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) {
+    public DocumentMapperParser(IndexSettings indexSettings,
+                                MapperService mapperService,
+                                SimilarityService similarityService,
+                                MapperRegistry mapperRegistry,
+                                Supplier<QueryShardContext> queryShardContextSupplier,
+                                ScriptService scriptService) {
         this.mapperService = mapperService;
-        this.xContentRegistry = xContentRegistry;
         this.similarityService = similarityService;
         this.queryShardContextSupplier = queryShardContextSupplier;
         this.scriptService = scriptService;
@@ -173,8 +170,4 @@ private static String getRemainingFields(Map<?, ?> map) {
         }
         return remainingFields.toString();
     }
-
-    NamedXContentRegistry getXContentRegistry() {
-        return xContentRegistry;
-    }
 }

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

@@ -28,6 +28,7 @@
 import org.elasticsearch.common.settings.Settings;
 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;
@@ -45,6 +46,12 @@
 /** A parser for documents, given mappings from a DocumentMapper */
 final class DocumentParser {
 
+    private final NamedXContentRegistry xContentRegistry;
+
+    DocumentParser(NamedXContentRegistry xContentRegistry) {
+        this.xContentRegistry = xContentRegistry;
+    }
+
     ParsedDocument parseDocument(SourceToParse source,
                                  MetadataFieldMapper[] metadataFieldsMappers,
                                  DocumentMapper docMapper) throws MapperParsingException {
@@ -53,7 +60,7 @@ ParsedDocument parseDocument(SourceToParse source,
         final ParseContext.InternalParseContext context;
         final XContentType xContentType = source.getXContentType();
 
-        try (XContentParser parser = XContentHelper.createParser(docMapper.documentMapperParser().getXContentRegistry(),
+        try (XContentParser parser = XContentHelper.createParser(xContentRegistry,
             LoggingDeprecationHandler.INSTANCE, source.source(), xContentType)) {
             context = new ParseContext.InternalParseContext(docMapper, source, parser);
             validateStart(parser);

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

@@ -103,26 +103,24 @@ public enum MergeReason {
         Setting.longSetting("index.mapping.field_name_length.limit", Long.MAX_VALUE, 1L, Property.Dynamic, Property.IndexScope);
 
     private final IndexAnalyzers indexAnalyzers;
-
-    private volatile DocumentMapper mapper;
-
-    private final DocumentMapperParser documentParser;
+    private final DocumentMapperParser documentMapperParser;
+    private final DocumentParser documentParser;
     private final Version indexVersionCreated;
-
     private final MapperAnalyzerWrapper indexAnalyzer;
-
     final MapperRegistry mapperRegistry;
-
     private final BooleanSupplier idFieldDataEnabled;
 
+    private volatile DocumentMapper mapper;
+
     public MapperService(IndexSettings indexSettings, IndexAnalyzers indexAnalyzers, NamedXContentRegistry xContentRegistry,
                          SimilarityService similarityService, MapperRegistry mapperRegistry,
                          Supplier<QueryShardContext> queryShardContextSupplier, BooleanSupplier idFieldDataEnabled,
                          ScriptService scriptService) {
         super(indexSettings);
         this.indexVersionCreated = indexSettings.getIndexVersionCreated();
         this.indexAnalyzers = indexAnalyzers;
-        this.documentParser = new DocumentMapperParser(indexSettings, this, xContentRegistry, similarityService, mapperRegistry,
+        this.documentParser = new DocumentParser(xContentRegistry);
+        this.documentMapperParser = new DocumentMapperParser(indexSettings, this, similarityService, mapperRegistry,
                 queryShardContextSupplier, scriptService);
         this.indexAnalyzer = new MapperAnalyzerWrapper(indexAnalyzers.getDefaultIndexAnalyzer(), MappedFieldType::indexAnalyzer);
         this.mapperRegistry = mapperRegistry;
@@ -142,6 +140,10 @@ public NamedAnalyzer getNamedAnalyzer(String analyzerName) {
     }
 
     public DocumentMapperParser documentMapperParser() {
+        return this.documentMapperParser;
+    }
+
+    DocumentParser documentParser() {
         return this.documentParser;
     }
 
@@ -278,7 +280,7 @@ private synchronized DocumentMapper internalMerge(String type, CompressedXConten
         DocumentMapper documentMapper;
 
         try {
-            documentMapper = documentParser.parse(type, mappings);
+            documentMapper = documentMapperParser.parse(type, mappings);
         } catch (Exception e) {
             throw new MapperParsingException("Failed to parse mapping: {}", e, e.getMessage());
         }
@@ -326,7 +328,7 @@ private boolean assertSerialization(DocumentMapper mapper) {
     }
 
     public DocumentMapper parse(String mappingType, CompressedXContent mappingSource) throws MapperParsingException {
-        return documentParser.parse(mappingType, mappingSource);
+        return documentMapperParser.parse(mappingType, mappingSource);
     }
 
     /**