Remove cyclic dependency between DocumentMapper and MapperService (el…

…astic#63639)

`DocumentMapper` holds on to a reference of `MapperService` so that its `merge` method, which creates and return a new `DocumentMapper` instance, can provide it to its own constructor. On the other hand `MapperService` has a volatile reference to `DocumentMapper`, which gets updated by calling `merge`.

Given that only three components out of `MapperService` are needed, `DocumentMapper` can instead hold on to those specific objects: `IndexSettings`, `DocumentMapperParser` and `IndexAnalyzers`.

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

@@ -101,42 +101,40 @@ public Builder put(MetadataFieldMapper.Builder mapper) {
             return this;
         }
 
-        public DocumentMapper build(MapperService mapperService) {
+        public DocumentMapper build(IndexSettings indexSettings, DocumentMapperParser documentMapperParser, IndexAnalyzers indexAnalyzers) {
             Objects.requireNonNull(rootObjectMapper, "Mapper builder must have the root object mapper set");
             Mapping mapping = new Mapping(
-                    mapperService.getIndexSettings().getIndexVersionCreated(),
+                    indexSettings.getIndexVersionCreated(),
                     rootObjectMapper,
                     metadataMappers.values().toArray(new MetadataFieldMapper[0]),
                     meta);
-            return new DocumentMapper(mapperService, mapping);
+            return new DocumentMapper(indexSettings, documentMapperParser, indexAnalyzers, mapping);
         }
     }
 
-    private final MapperService mapperService;
-
     private final String type;
     private final Text typeText;
-
     private final CompressedXContent mappingSource;
-
     private final Mapping mapping;
-
     private final DocumentParser documentParser;
-
     private final MappingLookup fieldMappers;
-
+    private final IndexSettings indexSettings;
+    private final IndexAnalyzers indexAnalyzers;
+    private final DocumentMapperParser documentMapperParser;
     private final MetadataFieldMapper[] deleteTombstoneMetadataFieldMappers;
     private final MetadataFieldMapper[] noopTombstoneMetadataFieldMappers;
 
-    public DocumentMapper(MapperService mapperService, Mapping mapping) {
-        this.mapperService = mapperService;
+    private DocumentMapper(IndexSettings indexSettings,
+                           DocumentMapperParser documentMapperParser,
+                           IndexAnalyzers indexAnalyzers,
+                           Mapping mapping) {
         this.type = mapping.root().name();
         this.typeText = new Text(this.type);
-        final IndexSettings indexSettings = mapperService.getIndexSettings();
         this.mapping = mapping;
-        this.documentParser = new DocumentParser(indexSettings, mapperService.documentMapperParser(), this);
-
-        final IndexAnalyzers indexAnalyzers = mapperService.getIndexAnalyzers();
+        this.documentMapperParser = documentMapperParser;
+        this.indexSettings = indexSettings;
+        this.indexAnalyzers = indexAnalyzers;
+        this.documentParser = new DocumentParser(indexSettings, documentMapperParser, this);
         this.fieldMappers = MappingLookup.fromMapping(this.mapping, indexAnalyzers.getDefaultIndexAnalyzer());
 
         try {
@@ -279,7 +277,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(mapperService, merged);
+        return new DocumentMapper(this.indexSettings, this.documentMapperParser, this.indexAnalyzers, merged);
     }
 
     public void validate(IndexSettings settings, boolean checkLimits) {
@@ -306,8 +304,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
     @Override
     public String toString() {
         return "DocumentMapper{" +
-            "mapperService=" + mapperService +
-            ", type='" + type + '\'' +
+            "type='" + type + '\'' +
             ", typeText=" + typeText +
             ", mappingSource=" + mappingSource +
             ", mapping=" + mapping +

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

@@ -145,7 +145,7 @@ private DocumentMapper parse(String type, Map<String, Object> mapping, String de
 
         checkNoRemainingFields(mapping, parserContext.indexVersionCreated(), "Root mapping definition has unsupported parameters: ");
 
-        return docBuilder.build(mapperService);
+        return docBuilder.build(mapperService.getIndexSettings(), mapperService.documentMapperParser(), mapperService.getIndexAnalyzers());
     }
 
     public static void checkNoRemainingFields(String fieldName, Map<?, ?> fieldNodeMap, Version indexVersionCreated) {

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

@@ -3430,7 +3430,8 @@ public void afterRefresh(boolean didRefresh) throws IOException {
     private EngineConfig.TombstoneDocSupplier tombstoneDocSupplier() {
         final RootObjectMapper.Builder noopRootMapper = new RootObjectMapper.Builder("__noop");
         final DocumentMapper noopDocumentMapper = mapperService != null ?
-            new DocumentMapper.Builder(noopRootMapper, mapperService).build(mapperService) :
+            new DocumentMapper.Builder(noopRootMapper, mapperService).build(mapperService.getIndexSettings(),
+                mapperService.documentMapperParser(), mapperService.getIndexAnalyzers()) :
             null;
         return new EngineConfig.TombstoneDocSupplier() {
             @Override

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

@@ -45,9 +45,7 @@
 public class DocumentMapperTests extends MapperServiceTestCase {
 
     public void testAddFields() throws Exception {
-        DocumentMapper stage1
-            = createDocumentMapper(mapping(b -> b.startObject("name").field("type", "text").endObject()));
-
+        DocumentMapper stage1 = createDocumentMapper(mapping(b -> b.startObject("name").field("type", "text").endObject()));
         DocumentMapper stage2 = createDocumentMapper(mapping(b -> {
             b.startObject("name").field("type", "text").endObject();
             b.startObject("age").field("type", "integer").endObject();
@@ -74,7 +72,7 @@ public void testAddFields() throws Exception {
 }
 
     public void testMergeObjectDynamic() throws Exception {
-        DocumentMapper mapper = createDocumentMapper(mapping(b -> {}));
+        DocumentMapper mapper = createDocumentMapper(mapping(b -> { }));
         assertNull(mapper.root().dynamic());
 
         DocumentMapper withDynamicMapper = createDocumentMapper(topMapping(b -> b.field("dynamic", "false")));
@@ -85,10 +83,8 @@ public void testMergeObjectDynamic() throws Exception {
     }
 
     public void testMergeObjectAndNested() throws Exception {
-        DocumentMapper objectMapper
-            = createDocumentMapper(mapping(b -> b.startObject("obj").field("type", "object").endObject()));
-        DocumentMapper nestedMapper
-            = createDocumentMapper(mapping(b -> b.startObject("obj").field("type", "nested").endObject()));
+        DocumentMapper objectMapper = createDocumentMapper(mapping(b -> b.startObject("obj").field("type", "object").endObject()));
+        DocumentMapper nestedMapper = createDocumentMapper((mapping(b -> b.startObject("obj").field("type", "nested").endObject())));
         MergeReason reason = randomFrom(MergeReason.MAPPING_UPDATE, MergeReason.INDEX_TEMPLATE);
 
         {
@@ -225,10 +221,7 @@ public void testMergeMetadataFieldsForIndexTemplates() throws IOException {
     }
 
     public void testMergeMeta() throws IOException {
-
-        DocumentMapper initMapper
-            = createDocumentMapper(topMapping(b -> b.startObject("_meta").field("foo", "bar").endObject()));
-
+        DocumentMapper initMapper = createDocumentMapper(topMapping(b -> b.startObject("_meta").field("foo", "bar").endObject()));
         assertThat(initMapper.meta().get("foo"), equalTo("bar"));
 
         DocumentMapper updatedMapper = createDocumentMapper(fieldMapping(b -> b.field("type", "text")));
@@ -238,13 +231,11 @@ public void testMergeMeta() throws IOException {
 
         updatedMapper
             = createDocumentMapper(topMapping(b -> b.startObject("_meta").field("foo", "new_bar").endObject()));
-
         mergedMapper = initMapper.merge(updatedMapper.mapping(), MergeReason.MAPPING_UPDATE);
         assertThat(mergedMapper.meta().get("foo"), equalTo("new_bar"));
     }
 
     public void testMergeMetaForIndexTemplate() throws IOException {
-
         DocumentMapper initMapper = createDocumentMapper(topMapping(b -> {
             b.startObject("_meta");
             {

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

@@ -131,12 +131,13 @@ private void testMultiField(String mapping) throws Exception {
     public void testBuildThenParse() throws Exception {
         IndexService indexService = createIndex("test");
         Supplier<NamedAnalyzer> a = () -> Lucene.STANDARD_ANALYZER;
-
+        MapperService mapperService = indexService.mapperService();
         DocumentMapper builderDocMapper = new DocumentMapper.Builder(new RootObjectMapper.Builder("person").add(
                 new TextFieldMapper.Builder("name", a).store(true)
                         .addMultiField(new TextFieldMapper.Builder("indexed", a).index(true))
                         .addMultiField(new TextFieldMapper.Builder("not_indexed", a).index(false).store(true))
-        ), indexService.mapperService()).build(indexService.mapperService());
+        ), indexService.mapperService()).build(mapperService.getIndexSettings(), mapperService.documentMapperParser(),
+            mapperService.getIndexAnalyzers());
 
         String builtMapping = builderDocMapper.mappingSource().string();
         // reparse it

test/framework/src/main/java/org/elasticsearch/index/MapperTestUtils.java

@@ -78,8 +78,8 @@ public static MapperService newMapperService(NamedXContentRegistry xContentRegis
 
     public static void assertConflicts(String mapping1,
                                        String mapping2,
-                                       DocumentMapperParser
-                                           parser, String... conflicts) throws IOException {
+                                       DocumentMapperParser parser,
+                                       String... conflicts) throws IOException {
         DocumentMapper docMapper = parser.parse("type", new CompressedXContent(mapping1));
         if (conflicts.length == 0) {
             docMapper.merge(parser.parse("type", new CompressedXContent(mapping2)).mapping(), MergeReason.MAPPING_UPDATE);

test/framework/src/main/java/org/elasticsearch/index/engine/TranslogHandler.java

@@ -74,7 +74,8 @@ public TranslogHandler(NamedXContentRegistry xContentRegistry, IndexSettings ind
     private DocumentMapperForType docMapper(String type) {
         RootObjectMapper.Builder rootBuilder = new RootObjectMapper.Builder(type);
         DocumentMapper.Builder b = new DocumentMapper.Builder(rootBuilder, mapperService);
-        return new DocumentMapperForType(b.build(mapperService), mappingUpdate);
+        return new DocumentMapperForType(b.build(mapperService.getIndexSettings(), mapperService.documentMapperParser(),
+            mapperService.getIndexAnalyzers()), mappingUpdate);
     }
 
     private void applyOperation(Engine engine, Engine.Operation operation) throws IOException {

x-pack/plugin/data-streams/src/test/java/org/elasticsearch/xpack/datastreams/mapper/DataStreamTimestampFieldMapperTests.java

@@ -294,5 +294,4 @@ public void testCannotUpdateTimestampField() throws IOException {
             + "{\"@timestamp2\": {\"type\": \"date\"},\"@timestamp\": {\"type\": \"date\"}}}})";
         assertConflicts(mapping1, mapping2, parser, "Mapper for [_data_stream_timestamp]", "[enabled] from [false] to [true]");
     }
-
 }

x-pack/plugin/mapper-flattened/src/internalClusterTest/java/org/elasticsearch/xpack/flattened/mapper/FlatObjectFieldMapperTests.java

@@ -366,10 +366,8 @@ public void testDepthLimit() throws IOException {
                 .endObject()
             .endObject()
         .endObject());
-
         DocumentMapper newMapper = mapper.merge(
-            parser.parse("type", new CompressedXContent(newMapping)).mapping(),
-            MergeReason.MAPPING_UPDATE);
+            parser.parse("type", new CompressedXContent(newMapping)).mapping(), MergeReason.MAPPING_UPDATE);
 
         expectThrows(MapperParsingException.class, () ->
             newMapper.parse(new SourceToParse("test", "type", "1", doc, XContentType.JSON)));
@@ -430,10 +428,8 @@ public void testIgnoreAbove() throws IOException {
                 .endObject()
             .endObject()
         .endObject());
-
         DocumentMapper newMapper = mapper.merge(
-            parser.parse("type", new CompressedXContent(newMapping)).mapping(),
-            MergeReason.MAPPING_UPDATE);
+            parser.parse("type", new CompressedXContent(newMapping)).mapping(), MergeReason.MAPPING_UPDATE);
 
         ParsedDocument newParsedDoc = newMapper.parse(new SourceToParse("test", "type", "1", doc, XContentType.JSON));
         IndexableField[] newFields = newParsedDoc.rootDoc().getFields("field");