Browse Source

Remove direct dependency between ParserContext and MapperService (#63741)

ParserContext only needs some small portions of MapperService, and certainly does not need to expose MapperService through its current getter method.

With this change we address this by keeping references to the needed components rather than the whole MapperService
Luca Cavanna 5 years ago
parent
commit
d126afb2c2

+ 1 - 1
modules/parent-join/src/main/java/org/elasticsearch/join/mapper/ParentJoinFieldMapper.java

@@ -178,7 +178,7 @@ public final class ParentJoinFieldMapper extends FieldMapper {
     public static class TypeParser implements Mapper.TypeParser {
         @Override
         public Mapper.Builder parse(String name, Map<String, Object> node, ParserContext parserContext) throws MapperParsingException {
-            final IndexSettings indexSettings = parserContext.mapperService().getIndexSettings();
+            final IndexSettings indexSettings = parserContext.getIndexSettings();
             checkIndexCompatibility(indexSettings, name);
 
             Builder builder = new Builder(name);

+ 10 - 10
server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java

@@ -27,10 +27,8 @@ import org.apache.lucene.search.Scorer;
 import org.apache.lucene.search.Weight;
 import org.apache.lucene.util.BytesRef;
 import org.elasticsearch.ElasticsearchGenerationException;
-import org.elasticsearch.Version;
 import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.compress.CompressedXContent;
-import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.text.Text;
 import org.elasticsearch.common.xcontent.ToXContent;
 import org.elasticsearch.common.xcontent.ToXContentFragment;
@@ -56,22 +54,24 @@ public class DocumentMapper implements ToXContentFragment {
     public static class Builder {
 
         private final Map<Class<? extends MetadataFieldMapper>, MetadataFieldMapper> metadataMappers = new LinkedHashMap<>();
-
         private final RootObjectMapper rootObjectMapper;
+        private final Mapper.BuilderContext builderContext;
+        private final IndexSettings indexSettings;
+        private final IndexAnalyzers indexAnalyzers;
+        private final DocumentMapperParser documentMapperParser;
 
         private Map<String, Object> meta;
 
-        private final Mapper.BuilderContext builderContext;
-
         public Builder(RootObjectMapper.Builder builder, MapperService mapperService) {
-            final Settings indexSettings = mapperService.getIndexSettings().getSettings();
-            this.builderContext = new Mapper.BuilderContext(indexSettings, new ContentPath(1));
+            this.indexSettings = mapperService.getIndexSettings();
+            this.indexAnalyzers = mapperService.getIndexAnalyzers();
+            this.documentMapperParser = mapperService.documentMapperParser();
+            this.builderContext = new Mapper.BuilderContext(indexSettings.getSettings(), new ContentPath(1));
             this.rootObjectMapper = builder.build(builderContext);
 
             final DocumentMapper existingMapper = mapperService.documentMapper();
-            final Version indexCreatedVersion = mapperService.getIndexSettings().getIndexVersionCreated();
             final Map<String, TypeParser> metadataMapperParsers =
-                mapperService.mapperRegistry.getMetadataMapperParsers(indexCreatedVersion);
+                mapperService.mapperRegistry.getMetadataMapperParsers(indexSettings.getIndexVersionCreated());
             for (Map.Entry<String, MetadataFieldMapper.TypeParser> entry : metadataMapperParsers.entrySet()) {
                 final String name = entry.getKey();
                 final MetadataFieldMapper existingMetadataMapper = existingMapper == null
@@ -99,7 +99,7 @@ public class DocumentMapper implements ToXContentFragment {
             return this;
         }
 
-        public DocumentMapper build(IndexSettings indexSettings, DocumentMapperParser documentMapperParser, IndexAnalyzers indexAnalyzers) {
+        public DocumentMapper build() {
             Objects.requireNonNull(rootObjectMapper, "Mapper builder must have the root object mapper set");
             Mapping mapping = new Mapping(
                     indexSettings.getIndexVersionCreated(),

+ 7 - 5
server/src/main/java/org/elasticsearch/index/mapper/DocumentMapperParser.java

@@ -67,13 +67,15 @@ public class DocumentMapperParser {
     }
 
     public Mapper.TypeParser.ParserContext parserContext() {
-        return new Mapper.TypeParser.ParserContext(similarityService::getSimilarity, mapperService,
-                typeParsers::get, indexVersionCreated, queryShardContextSupplier, null, scriptService);
+        return new Mapper.TypeParser.ParserContext(similarityService::getSimilarity, typeParsers::get, indexVersionCreated,
+            queryShardContextSupplier, null, scriptService, mapperService.getIndexAnalyzers(), mapperService.getIndexSettings(),
+            mapperService::isIdFieldDataEnabled);
     }
 
     public Mapper.TypeParser.ParserContext parserContext(DateFormatter dateFormatter) {
-        return new Mapper.TypeParser.ParserContext(similarityService::getSimilarity, mapperService,
-            typeParsers::get, indexVersionCreated, queryShardContextSupplier, dateFormatter, scriptService);
+        return new Mapper.TypeParser.ParserContext(similarityService::getSimilarity, typeParsers::get, indexVersionCreated,
+            queryShardContextSupplier, dateFormatter, scriptService, mapperService.getIndexAnalyzers(), mapperService.getIndexSettings(),
+            mapperService::isIdFieldDataEnabled);
     }
 
     @SuppressWarnings("unchecked")
@@ -150,7 +152,7 @@ public class DocumentMapperParser {
 
         checkNoRemainingFields(mapping, parserContext.indexVersionCreated(), "Root mapping definition has unsupported parameters: ");
 
-        return docBuilder.build(mapperService.getIndexSettings(), mapperService.documentMapperParser(), mapperService.getIndexAnalyzers());
+        return docBuilder.build();
     }
 
     public static void checkNoRemainingFields(String fieldName, Map<?, ?> fieldNodeMap, Version indexVersionCreated) {

+ 1 - 1
server/src/main/java/org/elasticsearch/index/mapper/IdFieldMapper.java

@@ -95,7 +95,7 @@ public class IdFieldMapper extends MetadataFieldMapper {
         }
     }
 
-    public static final TypeParser PARSER = new FixedTypeParser(c -> new IdFieldMapper(() -> c.mapperService().isIdFieldDataEnabled()));
+    public static final TypeParser PARSER = new FixedTypeParser(c -> new IdFieldMapper(c::isIdFieldDataEnabled));
 
     static final class IdFieldType extends TermBasedFieldType {
 

+ 28 - 20
server/src/main/java/org/elasticsearch/index/mapper/Mapper.java

@@ -23,6 +23,7 @@ import org.elasticsearch.Version;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.time.DateFormatter;
 import org.elasticsearch.common.xcontent.ToXContentFragment;
+import org.elasticsearch.index.IndexSettings;
 import org.elasticsearch.index.analysis.IndexAnalyzers;
 import org.elasticsearch.index.query.QueryShardContext;
 import org.elasticsearch.index.similarity.SimilarityProvider;
@@ -30,6 +31,7 @@ import org.elasticsearch.script.ScriptService;
 
 import java.util.Map;
 import java.util.Objects;
+import java.util.function.BooleanSupplier;
 import java.util.function.Function;
 import java.util.function.Supplier;
 
@@ -79,48 +81,55 @@ public abstract class Mapper implements ToXContentFragment, Iterable<Mapper> {
         class ParserContext {
 
             private final Function<String, SimilarityProvider> similarityLookupService;
-
-            private final MapperService mapperService;
-
             private final Function<String, TypeParser> typeParsers;
-
             private final Version indexVersionCreated;
-
             private final Supplier<QueryShardContext> queryShardContextSupplier;
-
             private final DateFormatter dateFormatter;
-
             private final ScriptService scriptService;
+            private final IndexAnalyzers indexAnalyzers;
+            private final IndexSettings indexSettings;
+            private final BooleanSupplier idFieldDataEnabled;
 
             public ParserContext(Function<String, SimilarityProvider> similarityLookupService,
-                                 MapperService mapperService, Function<String, TypeParser> typeParsers,
-                                 Version indexVersionCreated, Supplier<QueryShardContext> queryShardContextSupplier,
-                                 DateFormatter dateFormatter, ScriptService scriptService) {
+                                 Function<String, TypeParser> typeParsers,
+                                 Version indexVersionCreated,
+                                 Supplier<QueryShardContext> queryShardContextSupplier,
+                                 DateFormatter dateFormatter,
+                                 ScriptService scriptService,
+                                 IndexAnalyzers indexAnalyzers,
+                                 IndexSettings indexSettings,
+                                 BooleanSupplier idFieldDataEnabled) {
                 this.similarityLookupService = similarityLookupService;
-                this.mapperService = mapperService;
                 this.typeParsers = typeParsers;
                 this.indexVersionCreated = indexVersionCreated;
                 this.queryShardContextSupplier = queryShardContextSupplier;
                 this.dateFormatter = dateFormatter;
                 this.scriptService = scriptService;
+                this.indexAnalyzers = indexAnalyzers;
+                this.indexSettings = indexSettings;
+                this.idFieldDataEnabled = idFieldDataEnabled;
             }
 
             public IndexAnalyzers getIndexAnalyzers() {
-                return mapperService.getIndexAnalyzers();
+                return indexAnalyzers;
+            }
+
+            public IndexSettings getIndexSettings() {
+                return indexSettings;
+            }
+
+            public boolean isIdFieldDataEnabled() {
+                return idFieldDataEnabled.getAsBoolean();
             }
 
             public Settings getSettings() {
-                return mapperService.getIndexSettings().getSettings();
+                return indexSettings.getSettings();
             }
 
             public SimilarityProvider getSimilarity(String name) {
                 return similarityLookupService.apply(name);
             }
 
-            public MapperService mapperService() {
-                return mapperService;
-            }
-
             public TypeParser typeParser(String type) {
                 return typeParsers.apply(type);
             }
@@ -161,14 +170,13 @@ public abstract class Mapper implements ToXContentFragment, Iterable<Mapper> {
 
             static class MultiFieldParserContext extends ParserContext {
                 MultiFieldParserContext(ParserContext in) {
-                    super(in.similarityLookupService(), in.mapperService(), in.typeParsers(),
-                            in.indexVersionCreated(), in.queryShardContextSupplier(), in.getDateFormatter(), in.scriptService());
+                    super(in.similarityLookupService, in.typeParsers, in.indexVersionCreated, in.queryShardContextSupplier,
+                        in.dateFormatter, in.scriptService, in.indexAnalyzers, in.indexSettings, in.idFieldDataEnabled);
                 }
 
                 @Override
                 public boolean isWithinMultiField() { return true; }
             }
-
         }
 
         Mapper.Builder parse(String name, Map<String, Object> node, ParserContext parserContext) throws MapperParsingException;

+ 1 - 1
server/src/main/java/org/elasticsearch/index/mapper/NestedPathFieldMapper.java

@@ -68,7 +68,7 @@ public class NestedPathFieldMapper extends MetadataFieldMapper {
     }
 
     public static final TypeParser PARSER = new FixedTypeParser(c -> {
-        final IndexSettings indexSettings = c.mapperService().getIndexSettings();
+        final IndexSettings indexSettings = c.getIndexSettings();
         return new NestedPathFieldMapper(indexSettings.getSettings());
     });
 

+ 1 - 3
server/src/main/java/org/elasticsearch/index/mapper/RootObjectMapper.java

@@ -131,9 +131,7 @@ public class RootObjectMapper extends ObjectMapper {
     public static class TypeParser extends ObjectMapper.TypeParser {
 
         @Override
-        @SuppressWarnings("rawtypes")
         public Mapper.Builder parse(String name, Map<String, Object> node, ParserContext parserContext) throws MapperParsingException {
-
             RootObjectMapper.Builder builder = new Builder(name);
             Iterator<Map.Entry<String, Object>> iterator = node.entrySet().iterator();
             while (iterator.hasNext()) {
@@ -399,7 +397,7 @@ public class RootObjectMapper extends ObjectMapper {
                 Mapper.Builder dummyBuilder = typeParser.parse(templateName, fieldTypeConfig, parserContext);
                 fieldTypeConfig.remove("type");
                 if (fieldTypeConfig.isEmpty()) {
-                    Settings indexSettings = parserContext.mapperService().getIndexSettings().getSettings();
+                    Settings indexSettings = parserContext.getSettings();
                     BuilderContext builderContext = new BuilderContext(indexSettings, new ContentPath(1));
                     dummyBuilder.build(builderContext);
                     dynamicTemplateInvalid = false;

+ 2 - 3
server/src/main/java/org/elasticsearch/index/shard/IndexShard.java

@@ -3349,7 +3349,7 @@ public class IndexShard extends AbstractIndexShardComponent implements IndicesCl
         }
 
         @Override
-        public void afterRefresh(boolean didRefresh) throws IOException {
+        public void afterRefresh(boolean didRefresh) {
             if (Assertions.ENABLED) {
                 assert callingThread != null : "afterRefresh called but not beforeRefresh";
                 assert callingThread == Thread.currentThread() : "beforeRefreshed called by a different thread. current ["
@@ -3363,8 +3363,7 @@ public class IndexShard extends AbstractIndexShardComponent implements IndicesCl
     private EngineConfig.TombstoneDocSupplier tombstoneDocSupplier() {
         final RootObjectMapper.Builder noopRootMapper = new RootObjectMapper.Builder("__noop");
         final DocumentMapper noopDocumentMapper = mapperService != null ?
-            new DocumentMapper.Builder(noopRootMapper, mapperService).build(mapperService.getIndexSettings(),
-                mapperService.documentMapperParser(), mapperService.getIndexAnalyzers()) :
+            new DocumentMapper.Builder(noopRootMapper, mapperService).build() :
             null;
         return new EngineConfig.TombstoneDocSupplier() {
             @Override

+ 1 - 3
server/src/test/java/org/elasticsearch/index/mapper/MultiFieldTests.java

@@ -131,13 +131,11 @@ public class MultiFieldTests extends ESSingleNodeTestCase {
     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(mapperService.getIndexSettings(), mapperService.documentMapperParser(),
-            mapperService.getIndexAnalyzers());
+        ), indexService.mapperService()).build();
 
         String builtMapping = builderDocMapper.mappingSource().string();
         // reparse it

+ 3 - 2
server/src/test/java/org/elasticsearch/index/mapper/ParametrizedMapperTests.java

@@ -205,7 +205,7 @@ public class ParametrizedMapperTests extends MapperServiceTestCase {
                 "default", new NamedAnalyzer("default", AnalyzerScope.INDEX, new StandardAnalyzer())),
             Collections.emptyMap(), Collections.emptyMap());
         when(mapperService.getIndexAnalyzers()).thenReturn(indexAnalyzers);
-        Mapper.TypeParser.ParserContext pc = new Mapper.TypeParser.ParserContext(s -> null, mapperService, s -> {
+        Mapper.TypeParser.ParserContext pc = new Mapper.TypeParser.ParserContext(s -> null, s -> {
             if (Objects.equals("keyword", s)) {
                 return KeywordFieldMapper.PARSER;
             }
@@ -213,7 +213,8 @@ public class ParametrizedMapperTests extends MapperServiceTestCase {
                 return BinaryFieldMapper.PARSER;
             }
             return null;
-        }, version, () -> null, null, null);
+        }, version, () -> null, null, null,
+            mapperService.getIndexAnalyzers(), mapperService.getIndexSettings(), mapperService::isIdFieldDataEnabled);
         return (TestMapper) new TypeParser()
             .parse("field", XContentHelper.convertToMap(JsonXContent.jsonXContent, mapping, true), pc)
             .build(new Mapper.BuilderContext(Settings.EMPTY, new ContentPath(0)));

+ 4 - 4
server/src/test/java/org/elasticsearch/index/mapper/TypeParsersTests.java

@@ -83,8 +83,8 @@ public class TypeParsersTests extends ESTestCase {
         MapperService mapperService = mock(MapperService.class);
         when(mapperService.getIndexAnalyzers()).thenReturn(indexAnalyzers);
         Version olderVersion = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0);
-        Mapper.TypeParser.ParserContext olderContext = new Mapper.TypeParser.ParserContext(
-            null, mapperService, type -> typeParser, olderVersion, null, null, null);
+        Mapper.TypeParser.ParserContext olderContext = new Mapper.TypeParser.ParserContext(null, type -> typeParser, olderVersion, null,
+            null, null, mapperService.getIndexAnalyzers(), mapperService.getIndexSettings(), mapperService::isIdFieldDataEnabled);
 
         builder.parse("some-field", olderContext, fieldNode);
         assertWarnings("At least one multi-field, [sub-field], " +
@@ -98,8 +98,8 @@ public class TypeParsersTests extends ESTestCase {
             BytesReference.bytes(mapping), true, mapping.contentType()).v2();
 
         Version version = VersionUtils.randomVersionBetween(random(), Version.V_8_0_0, Version.CURRENT);
-        Mapper.TypeParser.ParserContext context = new Mapper.TypeParser.ParserContext(
-            null, mapperService, type -> typeParser, version, null, null, null);
+        Mapper.TypeParser.ParserContext context = new Mapper.TypeParser.ParserContext(null, type -> typeParser, version, null, null,
+            null, mapperService.getIndexAnalyzers(), mapperService.getIndexSettings(), mapperService::isIdFieldDataEnabled);
 
         IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> {
             TextFieldMapper.Builder bad = new TextFieldMapper.Builder("textField", () -> Lucene.STANDARD_ANALYZER);

+ 3 - 2
server/src/test/java/org/elasticsearch/index/query/QueryShardContextTests.java

@@ -314,8 +314,9 @@ public class QueryShardContextTests extends ESTestCase {
         when(mapperService.getIndexAnalyzers()).thenReturn(indexAnalyzers);
         DocumentMapperParser documentMapperParser = mock(DocumentMapperParser.class);
         Map<String, Mapper.TypeParser> typeParserMap = IndicesModule.getMappers(Collections.emptyList());
-        Mapper.TypeParser.ParserContext parserContext = new Mapper.TypeParser.ParserContext(name -> null, mapperService,
-            typeParserMap::get, Version.CURRENT, () -> null, null, null);
+        Mapper.TypeParser.ParserContext parserContext = new Mapper.TypeParser.ParserContext(name -> null, typeParserMap::get,
+            Version.CURRENT, () -> null, null, null, mapperService.getIndexAnalyzers(), mapperService.getIndexSettings(),
+            mapperService::isIdFieldDataEnabled);
         when(documentMapperParser.parserContext()).thenReturn(parserContext);
         when(mapperService.documentMapperParser()).thenReturn(documentMapperParser);
         if (runtimeDocValues != null) {

+ 1 - 2
test/framework/src/main/java/org/elasticsearch/index/engine/TranslogHandler.java

@@ -71,8 +71,7 @@ public class TranslogHandler implements Engine.TranslogRecoveryRunner {
     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.getIndexSettings(), mapperService.documentMapperParser(),
-            mapperService.getIndexAnalyzers()), null);
+        return new DocumentMapperForType(b.build(), null);
     }
 
     private void applyOperation(Engine engine, Engine.Operation operation) throws IOException {

+ 2 - 2
test/framework/src/main/java/org/elasticsearch/search/aggregations/AggregatorTestCase.java

@@ -823,9 +823,9 @@ public abstract class AggregatorTestCase extends ESTestCase {
         iw.addDocument(doc);
     }
 
-    private class MockParserContext extends Mapper.TypeParser.ParserContext {
+    private static class MockParserContext extends Mapper.TypeParser.ParserContext {
         MockParserContext() {
-            super(null, null, null, null, null, null, null);
+            super(null, null, null, null, null, null, null, null, null);
         }
 
         @Override