Skip to content

Commit 301d95f

Browse files
javannajavanna
committed
Remove direct dependency between ParserContext and MapperService (elastic#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
1 parent 293d1ad commit 301d95f

File tree

13 files changed

+62
-56
lines changed

13 files changed

+62
-56
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ public ParentJoinFieldMapper build(BuilderContext context) {
178178
public static class TypeParser implements Mapper.TypeParser {
179179
@Override
180180
public Mapper.Builder parse(String name, Map<String, Object> node, ParserContext parserContext) throws MapperParsingException {
181-
final IndexSettings indexSettings = parserContext.mapperService().getIndexSettings();
181+
final IndexSettings indexSettings = parserContext.getIndexSettings();
182182
checkIndexCompatibility(indexSettings, name);
183183

184184
Builder builder = new Builder(name);

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

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,8 @@
2727
import org.apache.lucene.search.Weight;
2828
import org.apache.lucene.util.BytesRef;
2929
import org.elasticsearch.ElasticsearchGenerationException;
30-
import org.elasticsearch.Version;
3130
import org.elasticsearch.common.bytes.BytesArray;
3231
import org.elasticsearch.common.compress.CompressedXContent;
33-
import org.elasticsearch.common.settings.Settings;
3432
import org.elasticsearch.common.text.Text;
3533
import org.elasticsearch.common.xcontent.ToXContent;
3634
import org.elasticsearch.common.xcontent.ToXContentFragment;
@@ -56,23 +54,24 @@ public class DocumentMapper implements ToXContentFragment {
5654
public static class Builder {
5755

5856
private final Map<Class<? extends MetadataFieldMapper>, MetadataFieldMapper> metadataMappers = new LinkedHashMap<>();
59-
6057
private final RootObjectMapper rootObjectMapper;
58+
private final Mapper.BuilderContext builderContext;
59+
private final IndexSettings indexSettings;
60+
private final IndexAnalyzers indexAnalyzers;
61+
private final DocumentMapperParser documentMapperParser;
6162

6263
private Map<String, Object> meta;
6364

64-
private final Mapper.BuilderContext builderContext;
65-
6665
public Builder(RootObjectMapper.Builder builder, MapperService mapperService) {
67-
final Settings indexSettings = mapperService.getIndexSettings().getSettings();
68-
this.builderContext = new Mapper.BuilderContext(indexSettings, new ContentPath(1));
66+
this.indexSettings = mapperService.getIndexSettings();
67+
this.indexAnalyzers = mapperService.getIndexAnalyzers();
68+
this.documentMapperParser = mapperService.documentMapperParser();
69+
this.builderContext = new Mapper.BuilderContext(indexSettings.getSettings(), new ContentPath(1));
6970
this.rootObjectMapper = builder.build(builderContext);
70-
7171
final String type = rootObjectMapper.name();
7272
final DocumentMapper existingMapper = mapperService.documentMapper(type);
73-
final Version indexCreatedVersion = mapperService.getIndexSettings().getIndexVersionCreated();
7473
final Map<String, TypeParser> metadataMapperParsers =
75-
mapperService.mapperRegistry.getMetadataMapperParsers(indexCreatedVersion);
74+
mapperService.mapperRegistry.getMetadataMapperParsers(indexSettings.getIndexVersionCreated());
7675
for (Map.Entry<String, MetadataFieldMapper.TypeParser> entry : metadataMapperParsers.entrySet()) {
7776
final String name = entry.getKey();
7877
final MetadataFieldMapper existingMetadataMapper = existingMapper == null
@@ -101,7 +100,7 @@ public Builder put(MetadataFieldMapper.Builder mapper) {
101100
return this;
102101
}
103102

104-
public DocumentMapper build(IndexSettings indexSettings, DocumentMapperParser documentMapperParser, IndexAnalyzers indexAnalyzers) {
103+
public DocumentMapper build() {
105104
Objects.requireNonNull(rootObjectMapper, "Mapper builder must have the root object mapper set");
106105
Mapping mapping = new Mapping(
107106
indexSettings.getIndexVersionCreated(),

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

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,13 +71,15 @@ public DocumentMapperParser(IndexSettings indexSettings, MapperService mapperSer
7171
}
7272

7373
public Mapper.TypeParser.ParserContext parserContext() {
74-
return new Mapper.TypeParser.ParserContext(similarityService::getSimilarity, mapperService,
75-
typeParsers::get, indexVersionCreated, queryShardContextSupplier, null, scriptService);
74+
return new Mapper.TypeParser.ParserContext(similarityService::getSimilarity, typeParsers::get, indexVersionCreated,
75+
queryShardContextSupplier, null, scriptService, mapperService.getIndexAnalyzers(), mapperService.getIndexSettings(),
76+
mapperService::isIdFieldDataEnabled);
7677
}
7778

7879
public Mapper.TypeParser.ParserContext parserContext(DateFormatter dateFormatter) {
79-
return new Mapper.TypeParser.ParserContext(similarityService::getSimilarity, mapperService,
80-
typeParsers::get, indexVersionCreated, queryShardContextSupplier, dateFormatter, scriptService);
80+
return new Mapper.TypeParser.ParserContext(similarityService::getSimilarity, typeParsers::get, indexVersionCreated,
81+
queryShardContextSupplier, dateFormatter, scriptService, mapperService.getIndexAnalyzers(), mapperService.getIndexSettings(),
82+
mapperService::isIdFieldDataEnabled);
8183
}
8284

8385
public DocumentMapper parse(@Nullable String type, CompressedXContent source) throws MapperParsingException {
@@ -145,7 +147,7 @@ private DocumentMapper parse(String type, Map<String, Object> mapping, String de
145147

146148
checkNoRemainingFields(mapping, parserContext.indexVersionCreated(), "Root mapping definition has unsupported parameters: ");
147149

148-
return docBuilder.build(mapperService.getIndexSettings(), mapperService.documentMapperParser(), mapperService.getIndexAnalyzers());
150+
return docBuilder.build();
149151
}
150152

151153
public static void checkNoRemainingFields(String fieldName, Map<?, ?> fieldNodeMap, Version indexVersionCreated) {

‎server/src/main/java/org/elasticsearch/index/mapper/IdFieldMapper.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ public static class Defaults {
9595
}
9696
}
9797

98-
public static final TypeParser PARSER = new FixedTypeParser(c -> new IdFieldMapper(() -> c.mapperService().isIdFieldDataEnabled()));
98+
public static final TypeParser PARSER = new FixedTypeParser(c -> new IdFieldMapper(c::isIdFieldDataEnabled));
9999

100100
static final class IdFieldType extends TermBasedFieldType {
101101

‎server/src/main/java/org/elasticsearch/index/mapper/Mapper.java

Lines changed: 28 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,15 @@
2323
import org.elasticsearch.common.settings.Settings;
2424
import org.elasticsearch.common.time.DateFormatter;
2525
import org.elasticsearch.common.xcontent.ToXContentFragment;
26+
import org.elasticsearch.index.IndexSettings;
2627
import org.elasticsearch.index.analysis.IndexAnalyzers;
2728
import org.elasticsearch.index.query.QueryShardContext;
2829
import org.elasticsearch.index.similarity.SimilarityProvider;
2930
import org.elasticsearch.script.ScriptService;
3031

3132
import java.util.Map;
3233
import java.util.Objects;
34+
import java.util.function.BooleanSupplier;
3335
import java.util.function.Function;
3436
import java.util.function.Supplier;
3537

@@ -79,48 +81,55 @@ public interface TypeParser {
7981
class ParserContext {
8082

8183
private final Function<String, SimilarityProvider> similarityLookupService;
82-
83-
private final MapperService mapperService;
84-
8584
private final Function<String, TypeParser> typeParsers;
86-
8785
private final Version indexVersionCreated;
88-
8986
private final Supplier<QueryShardContext> queryShardContextSupplier;
90-
9187
private final DateFormatter dateFormatter;
92-
9388
private final ScriptService scriptService;
89+
private final IndexAnalyzers indexAnalyzers;
90+
private final IndexSettings indexSettings;
91+
private final BooleanSupplier idFieldDataEnabled;
9492

9593
public ParserContext(Function<String, SimilarityProvider> similarityLookupService,
96-
MapperService mapperService, Function<String, TypeParser> typeParsers,
97-
Version indexVersionCreated, Supplier<QueryShardContext> queryShardContextSupplier,
98-
DateFormatter dateFormatter, ScriptService scriptService) {
94+
Function<String, TypeParser> typeParsers,
95+
Version indexVersionCreated,
96+
Supplier<QueryShardContext> queryShardContextSupplier,
97+
DateFormatter dateFormatter,
98+
ScriptService scriptService,
99+
IndexAnalyzers indexAnalyzers,
100+
IndexSettings indexSettings,
101+
BooleanSupplier idFieldDataEnabled) {
99102
this.similarityLookupService = similarityLookupService;
100-
this.mapperService = mapperService;
101103
this.typeParsers = typeParsers;
102104
this.indexVersionCreated = indexVersionCreated;
103105
this.queryShardContextSupplier = queryShardContextSupplier;
104106
this.dateFormatter = dateFormatter;
105107
this.scriptService = scriptService;
108+
this.indexAnalyzers = indexAnalyzers;
109+
this.indexSettings = indexSettings;
110+
this.idFieldDataEnabled = idFieldDataEnabled;
106111
}
107112

108113
public IndexAnalyzers getIndexAnalyzers() {
109-
return mapperService.getIndexAnalyzers();
114+
return indexAnalyzers;
115+
}
116+
117+
public IndexSettings getIndexSettings() {
118+
return indexSettings;
119+
}
120+
121+
public boolean isIdFieldDataEnabled() {
122+
return idFieldDataEnabled.getAsBoolean();
110123
}
111124

112125
public Settings getSettings() {
113-
return mapperService.getIndexSettings().getSettings();
126+
return indexSettings.getSettings();
114127
}
115128

116129
public SimilarityProvider getSimilarity(String name) {
117130
return similarityLookupService.apply(name);
118131
}
119132

120-
public MapperService mapperService() {
121-
return mapperService;
122-
}
123-
124133
public TypeParser typeParser(String type) {
125134
return typeParsers.apply(type);
126135
}
@@ -161,14 +170,13 @@ public ParserContext createMultiFieldContext(ParserContext in) {
161170

162171
static class MultiFieldParserContext extends ParserContext {
163172
MultiFieldParserContext(ParserContext in) {
164-
super(in.similarityLookupService(), in.mapperService(), in.typeParsers(),
165-
in.indexVersionCreated(), in.queryShardContextSupplier(), in.getDateFormatter(), in.scriptService());
173+
super(in.similarityLookupService, in.typeParsers, in.indexVersionCreated, in.queryShardContextSupplier,
174+
in.dateFormatter, in.scriptService, in.indexAnalyzers, in.indexSettings, in.idFieldDataEnabled);
166175
}
167176

168177
@Override
169178
public boolean isWithinMultiField() { return true; }
170179
}
171-
172180
}
173181

174182
Mapper.Builder parse(String name, Map<String, Object> node, ParserContext parserContext) throws MapperParsingException;

‎server/src/main/java/org/elasticsearch/index/mapper/RootObjectMapper.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,6 @@ public static class TypeParser extends ObjectMapper.TypeParser {
132132

133133
@Override
134134
public Mapper.Builder parse(String name, Map<String, Object> node, ParserContext parserContext) throws MapperParsingException {
135-
136135
RootObjectMapper.Builder builder = new Builder(name);
137136
Iterator<Map.Entry<String, Object>> iterator = node.entrySet().iterator();
138137
while (iterator.hasNext()) {
@@ -395,7 +394,7 @@ private static void validateDynamicTemplate(Mapper.TypeParser.ParserContext pars
395394
Mapper.Builder dummyBuilder = typeParser.parse(templateName, fieldTypeConfig, parserContext);
396395
fieldTypeConfig.remove("type");
397396
if (fieldTypeConfig.isEmpty()) {
398-
Settings indexSettings = parserContext.mapperService().getIndexSettings().getSettings();
397+
Settings indexSettings = parserContext.getSettings();
399398
BuilderContext builderContext = new BuilderContext(indexSettings, new ContentPath(1));
400399
dummyBuilder.build(builderContext);
401400
dynamicTemplateInvalid = false;

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3416,7 +3416,7 @@ public void beforeRefresh() throws IOException {
34163416
}
34173417

34183418
@Override
3419-
public void afterRefresh(boolean didRefresh) throws IOException {
3419+
public void afterRefresh(boolean didRefresh) {
34203420
if (Assertions.ENABLED) {
34213421
assert callingThread != null : "afterRefresh called but not beforeRefresh";
34223422
assert callingThread == Thread.currentThread() : "beforeRefreshed called by a different thread. current ["
@@ -3430,8 +3430,7 @@ public void afterRefresh(boolean didRefresh) throws IOException {
34303430
private EngineConfig.TombstoneDocSupplier tombstoneDocSupplier() {
34313431
final RootObjectMapper.Builder noopRootMapper = new RootObjectMapper.Builder("__noop");
34323432
final DocumentMapper noopDocumentMapper = mapperService != null ?
3433-
new DocumentMapper.Builder(noopRootMapper, mapperService).build(mapperService.getIndexSettings(),
3434-
mapperService.documentMapperParser(), mapperService.getIndexAnalyzers()) :
3433+
new DocumentMapper.Builder(noopRootMapper, mapperService).build() :
34353434
null;
34363435
return new EngineConfig.TombstoneDocSupplier() {
34373436
@Override

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -131,13 +131,11 @@ private void testMultiField(String mapping) throws Exception {
131131
public void testBuildThenParse() throws Exception {
132132
IndexService indexService = createIndex("test");
133133
Supplier<NamedAnalyzer> a = () -> Lucene.STANDARD_ANALYZER;
134-
MapperService mapperService = indexService.mapperService();
135134
DocumentMapper builderDocMapper = new DocumentMapper.Builder(new RootObjectMapper.Builder("person").add(
136135
new TextFieldMapper.Builder("name", a).store(true)
137136
.addMultiField(new TextFieldMapper.Builder("indexed", a).index(true))
138137
.addMultiField(new TextFieldMapper.Builder("not_indexed", a).index(false).store(true))
139-
), indexService.mapperService()).build(mapperService.getIndexSettings(), mapperService.documentMapperParser(),
140-
mapperService.getIndexAnalyzers());
138+
), indexService.mapperService()).build();
141139

142140
String builtMapping = builderDocMapper.mappingSource().string();
143141
// reparse it

‎server/src/test/java/org/elasticsearch/index/mapper/ParametrizedMapperTests.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -207,15 +207,16 @@ private static TestMapper fromMapping(String mapping, Version version) {
207207
"default", new NamedAnalyzer("default", AnalyzerScope.INDEX, new StandardAnalyzer())),
208208
Collections.emptyMap(), Collections.emptyMap());
209209
when(mapperService.getIndexAnalyzers()).thenReturn(indexAnalyzers);
210-
Mapper.TypeParser.ParserContext pc = new Mapper.TypeParser.ParserContext(s -> null, mapperService, s -> {
210+
Mapper.TypeParser.ParserContext pc = new Mapper.TypeParser.ParserContext(s -> null, s -> {
211211
if (Objects.equals("keyword", s)) {
212212
return KeywordFieldMapper.PARSER;
213213
}
214214
if (Objects.equals("binary", s)) {
215215
return BinaryFieldMapper.PARSER;
216216
}
217217
return null;
218-
}, version, () -> null, null, null);
218+
}, version, () -> null, null, null,
219+
mapperService.getIndexAnalyzers(), mapperService.getIndexSettings(), mapperService::isIdFieldDataEnabled);
219220
return (TestMapper) new TypeParser()
220221
.parse("field", XContentHelper.convertToMap(JsonXContent.jsonXContent, mapping, true), pc)
221222
.build(new Mapper.BuilderContext(Settings.EMPTY, new ContentPath(0)));

‎server/src/test/java/org/elasticsearch/index/mapper/TypeParsersTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,8 @@ public void testMultiFieldWithinMultiField() throws IOException {
7979
IndexAnalyzers indexAnalyzers = new IndexAnalyzers(defaultAnalyzers(), Collections.emptyMap(), Collections.emptyMap());
8080
MapperService mapperService = mock(MapperService.class);
8181
when(mapperService.getIndexAnalyzers()).thenReturn(indexAnalyzers);
82-
Mapper.TypeParser.ParserContext olderContext = new Mapper.TypeParser.ParserContext(
83-
null, mapperService, type -> typeParser, Version.CURRENT, null, null, null);
82+
Mapper.TypeParser.ParserContext olderContext = new Mapper.TypeParser.ParserContext(null, type -> typeParser, Version.CURRENT, null,
83+
null, null, mapperService.getIndexAnalyzers(), mapperService.getIndexSettings(), mapperService::isIdFieldDataEnabled);
8484

8585
builder.parse("some-field", olderContext, fieldNode);
8686
assertWarnings("At least one multi-field, [sub-field], " +

‎server/src/test/java/org/elasticsearch/index/query/QueryShardContextTests.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -315,8 +315,9 @@ private static QueryShardContext createQueryShardContext(String indexUuid, Strin
315315
when(mapperService.getIndexAnalyzers()).thenReturn(indexAnalyzers);
316316
DocumentMapperParser documentMapperParser = mock(DocumentMapperParser.class);
317317
Map<String, Mapper.TypeParser> typeParserMap = IndicesModule.getMappers(Collections.emptyList());
318-
Mapper.TypeParser.ParserContext parserContext = new Mapper.TypeParser.ParserContext(name -> null, mapperService,
319-
typeParserMap::get, Version.CURRENT, () -> null, null, null);
318+
Mapper.TypeParser.ParserContext parserContext = new Mapper.TypeParser.ParserContext(name -> null, typeParserMap::get,
319+
Version.CURRENT, () -> null, null, null, mapperService.getIndexAnalyzers(), mapperService.getIndexSettings(),
320+
mapperService::isIdFieldDataEnabled);
320321
when(documentMapperParser.parserContext()).thenReturn(parserContext);
321322
when(mapperService.documentMapperParser()).thenReturn(documentMapperParser);
322323
if (runtimeDocValues != null) {

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,7 @@ public TranslogHandler(NamedXContentRegistry xContentRegistry, IndexSettings ind
7474
private DocumentMapperForType docMapper(String type) {
7575
RootObjectMapper.Builder rootBuilder = new RootObjectMapper.Builder(type);
7676
DocumentMapper.Builder b = new DocumentMapper.Builder(rootBuilder, mapperService);
77-
return new DocumentMapperForType(b.build(mapperService.getIndexSettings(), mapperService.documentMapperParser(),
78-
mapperService.getIndexAnalyzers()), mappingUpdate);
77+
return new DocumentMapperForType(b.build(), mappingUpdate);
7978
}
8079

8180
private void applyOperation(Engine engine, Engine.Operation operation) throws IOException {

‎test/framework/src/main/java/org/elasticsearch/search/aggregations/AggregatorTestCase.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -827,9 +827,9 @@ private void writeTestDoc(MappedFieldType fieldType, String fieldName, RandomInd
827827
iw.addDocument(doc);
828828
}
829829

830-
private class MockParserContext extends Mapper.TypeParser.ParserContext {
830+
private static class MockParserContext extends Mapper.TypeParser.ParserContext {
831831
MockParserContext() {
832-
super(null, null, null, Version.CURRENT, null, null, null);
832+
super(null, null, Version.CURRENT, null, null, null, null, null, null);
833833
}
834834

835835
@Override

0 commit comments

Comments
 (0)