Skip to content

Remove direct dependency between ParserContext and MapperService #63741

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 15, 2020
Merged

Remove direct dependency between ParserContext and MapperService #63741

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ad85712
Remove direct dependency between ParserContext and MapperService
javanna Oct 15, 2020
ce5e001
checkstyle
javanna Oct 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion modules/parent-join/src/main/java/org/elasticsearch/join/mapper/ParentJoinFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ public ParentJoinFieldMapper build(BuilderContext context) {
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);
Expand Down
20 changes: 10 additions & 10 deletions server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,8 @@
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;
Expand All @@ -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
Expand Down Expand Up @@ -99,7 +99,7 @@ public Builder put(MetadataFieldMapper.Builder mapper) {
return this;
}

public DocumentMapper build(IndexSettings indexSettings, DocumentMapperParser documentMapperParser, IndexAnalyzers indexAnalyzers) {
public DocumentMapper build() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hah, I'm getting whiplash now.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

haha I had a deja vu making this change, and I did not mean for any of this to happen just two days ago. but I am lucky, I guess :)

Objects.requireNonNull(rootObjectMapper, "Mapper builder must have the root object mapper set");
Mapping mapping = new Mapping(
indexSettings.getIndexVersionCreated(),
Expand Down
12 changes: 7 additions & 5 deletions server/src/main/java/org/elasticsearch/index/mapper/DocumentMapperParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,15 @@ public DocumentMapperParser(IndexSettings indexSettings, MapperService mapperSer
}

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")
Expand Down Expand Up @@ -150,7 +152,7 @@ private DocumentMapper parse(String type, Map<String, Object> mapping) throws Ma

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) {
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/org/elasticsearch/index/mapper/IdFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public static class Defaults {
}
}

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 {

Expand Down
48 changes: 28 additions & 20 deletions server/src/main/java/org/elasticsearch/index/mapper/Mapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,15 @@
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;
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;

Expand Down Expand Up @@ -79,48 +81,55 @@ public interface TypeParser {
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);
}
Expand Down Expand Up @@ -161,14 +170,13 @@ public ParserContext createMultiFieldContext(ParserContext in) {

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;
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/org/elasticsearch/index/mapper/NestedPathFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public static class Defaults {
}

public static final TypeParser PARSER = new FixedTypeParser(c -> {
final IndexSettings indexSettings = c.mapperService().getIndexSettings();
final IndexSettings indexSettings = c.getIndexSettings();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can probably change this to use the Version directly as a follow-up

return new NestedPathFieldMapper(indexSettings.getSettings());
});

Expand Down
4 changes: 1 addition & 3 deletions server/src/main/java/org/elasticsearch/index/mapper/RootObjectMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,7 @@ private static void fixRedundantIncludes(ObjectMapper objectMapper, boolean pare
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()) {
Expand Down Expand Up @@ -399,7 +397,7 @@ private static void validateDynamicTemplate(Mapper.TypeParser.ParserContext pars
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;
Expand Down
5 changes: 2 additions & 3 deletions server/src/main/java/org/elasticsearch/index/shard/IndexShard.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3349,7 +3349,7 @@ public void beforeRefresh() throws IOException {
}

@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 ["
Expand All @@ -3363,8 +3363,7 @@ 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.getIndexSettings(),
mapperService.documentMapperParser(), mapperService.getIndexAnalyzers()) :
new DocumentMapper.Builder(noopRootMapper, mapperService).build() :
null;
return new EngineConfig.TombstoneDocSupplier() {
@Override
Expand Down
4 changes: 1 addition & 3 deletions server/src/test/java/org/elasticsearch/index/mapper/MultiFieldTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,13 +131,11 @@ 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(mapperService.getIndexSettings(), mapperService.documentMapperParser(),
mapperService.getIndexAnalyzers());
), indexService.mapperService()).build();

String builtMapping = builderDocMapper.mappingSource().string();
// reparse it
Expand Down
5 changes: 3 additions & 2 deletions server/src/test/java/org/elasticsearch/index/mapper/ParametrizedMapperTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,15 +205,16 @@ private static TestMapper fromMapping(String mapping, Version version) {
"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;
}
if (Objects.equals("binary", s)) {
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)));
Expand Down
8 changes: 4 additions & 4 deletions server/src/test/java/org/elasticsearch/index/mapper/TypeParsersTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ public void testMultiFieldWithinMultiField() throws IOException {
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], " +
Expand All @@ -98,8 +98,8 @@ public void testMultiFieldWithinMultiField() throws IOException {
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);
Expand Down
5 changes: 3 additions & 2 deletions server/src/test/java/org/elasticsearch/index/query/QueryShardContextTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,8 +314,9 @@ private static QueryShardContext createQueryShardContext(String indexUuid, Strin
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) {
Expand Down
3 changes: 1 addition & 2 deletions test/framework/src/main/java/org/elasticsearch/index/engine/TranslogHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,7 @@ 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.getIndexSettings(), mapperService.documentMapperParser(),
mapperService.getIndexAnalyzers()), null);
return new DocumentMapperForType(b.build(), null);
}

private void applyOperation(Engine engine, Engine.Operation operation) throws IOException {
Expand Down
4 changes: 2 additions & 2 deletions test/framework/src/main/java/org/elasticsearch/search/aggregations/AggregatorTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -823,9 +823,9 @@ private void writeTestDoc(MappedFieldType fieldType, String fieldName, RandomInd
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
Expand Down