Skip to content

Commit 8ad476a

Browse files
authored
Split Mapping parsing from DocumentMapper construction (#68593)
DocumentMapperParser parses xcontent mappings into a DocumentMapper, through DocumentMapper.Builder. Most of the work is done to construct a Mapping instance, that then gets provided to the DocumentMapper. Moving forward, we would like to rely more on Mapping and less on the entire DocumentMapper, but currently it is cumbersome to create Mapping without creating an instance of DocumentMapper. This commit removes DocumentMapperParser and DocumentMapper.Builder in favor of a new class called MappingParser that given xcontent mappings returns a Mapping instance, which can be used on its own or provided to DocumentMapper at its construction time. This will help with further refactorings as well as to possibly have more tests that don't rely on the entire MapperService/DocumentMapper but rather only on the needed components.
1 parent 273508f commit 8ad476a

File tree

15 files changed

+253
-228
lines changed

15 files changed

+253
-228
lines changed

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

Lines changed: 16 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -24,72 +24,33 @@
2424
import java.io.IOException;
2525
import java.util.Arrays;
2626
import java.util.Collection;
27+
import java.util.Collections;
2728
import java.util.Map;
28-
import java.util.Objects;
2929
import java.util.stream.Stream;
3030

3131
public class DocumentMapper implements ToXContentFragment {
32-
33-
public static final class Builder {
34-
private final Map<Class<? extends MetadataFieldMapper>, MetadataFieldMapper> metadataMappers;
35-
private final RootObjectMapper rootObjectMapper;
36-
private final ContentPath contentPath;
37-
private final IndexSettings indexSettings;
38-
private final IndexAnalyzers indexAnalyzers;
39-
private final DocumentParser documentParser;
40-
41-
private Map<String, Object> meta;
42-
43-
public Builder(RootObjectMapper.Builder builder, MapperService mapperService) {
44-
this(builder, mapperService.getIndexSettings(), mapperService.getIndexAnalyzers(), mapperService.documentParser(),
45-
mapperService.getMetadataMappers());
46-
}
47-
48-
Builder(RootObjectMapper.Builder builder,
49-
IndexSettings indexSettings,
50-
IndexAnalyzers indexAnalyzers,
51-
DocumentParser documentParser,
52-
Map<Class<? extends MetadataFieldMapper>, MetadataFieldMapper> metadataMappers) {
53-
this.indexSettings = indexSettings;
54-
this.indexAnalyzers = indexAnalyzers;
55-
this.documentParser = documentParser;
56-
this.contentPath = new ContentPath(1);
57-
this.rootObjectMapper = builder.build(contentPath);
58-
this.metadataMappers = metadataMappers;
59-
}
60-
61-
public Builder meta(Map<String, Object> meta) {
62-
this.meta = meta;
63-
return this;
64-
}
65-
66-
public Builder put(MetadataFieldMapper.Builder mapper) {
67-
MetadataFieldMapper metadataMapper = mapper.build(contentPath);
68-
metadataMappers.put(metadataMapper.getClass(), metadataMapper);
69-
return this;
70-
}
71-
72-
public DocumentMapper build() {
73-
Objects.requireNonNull(rootObjectMapper, "Mapper builder must have the root object mapper set");
74-
Mapping mapping = new Mapping(
75-
rootObjectMapper,
76-
metadataMappers.values().toArray(new MetadataFieldMapper[0]),
77-
meta);
78-
return new DocumentMapper(indexSettings, indexAnalyzers, documentParser, mapping);
79-
}
80-
}
81-
8232
private final String type;
8333
private final CompressedXContent mappingSource;
8434
private final DocumentParser documentParser;
8535
private final MappingLookup mappingLookup;
8636
private final MetadataFieldMapper[] deleteTombstoneMetadataFieldMappers;
8737
private final MetadataFieldMapper[] noopTombstoneMetadataFieldMappers;
8838

89-
private DocumentMapper(IndexSettings indexSettings,
90-
IndexAnalyzers indexAnalyzers,
91-
DocumentParser documentParser,
92-
Mapping mapping) {
39+
public DocumentMapper(RootObjectMapper.Builder rootBuilder, MapperService mapperService) {
40+
this(
41+
mapperService.getIndexSettings(),
42+
mapperService.getIndexAnalyzers(),
43+
mapperService.documentParser(),
44+
new Mapping(
45+
rootBuilder.build(new ContentPath(1)),
46+
mapperService.getMetadataMappers().values().toArray(new MetadataFieldMapper[0]),
47+
Collections.emptyMap()));
48+
}
49+
50+
DocumentMapper(IndexSettings indexSettings,
51+
IndexAnalyzers indexAnalyzers,
52+
DocumentParser documentParser,
53+
Mapping mapping) {
9354
this.type = mapping.root().name();
9455
this.documentParser = documentParser;
9556
this.mappingLookup = MappingLookup.fromMapping(mapping, documentParser, indexSettings, indexAnalyzers);

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

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ public enum MergeReason {
9191
Setting.longSetting("index.mapping.field_name_length.limit", Long.MAX_VALUE, 1L, Property.Dynamic, Property.IndexScope);
9292

9393
private final IndexAnalyzers indexAnalyzers;
94-
private final DocumentMapperParser documentMapperParser;
94+
private final MappingParser mappingParser;
9595
private final DocumentParser documentParser;
9696
private final Version indexVersionCreated;
9797
private final MapperRegistry mapperRegistry;
@@ -115,8 +115,8 @@ public MapperService(IndexSettings indexSettings, IndexAnalyzers indexAnalyzers,
115115
Map<String, MetadataFieldMapper.TypeParser> metadataMapperParsers =
116116
mapperRegistry.getMetadataMapperParsers(indexSettings.getIndexVersionCreated());
117117
this.parserContextSupplier = () -> parserContextFunction.apply(null);
118-
this.documentMapperParser = new DocumentMapperParser(indexSettings, indexAnalyzers, this::resolveDocumentType, documentParser,
119-
this::getMetadataMappers, parserContextSupplier, metadataMapperParsers);
118+
this.mappingParser = new MappingParser(parserContextSupplier, metadataMapperParsers,
119+
this::getMetadataMappers, this::resolveDocumentType);
120120
}
121121

122122
public boolean hasNested() {
@@ -287,7 +287,7 @@ private synchronized DocumentMapper internalMerge(String type, CompressedXConten
287287
DocumentMapper documentMapper;
288288

289289
try {
290-
documentMapper = documentMapperParser.parse(type, mappings);
290+
documentMapper = parse(type, mappings);
291291
} catch (Exception e) {
292292
throw new MapperParsingException("Failed to parse mapping: {}", e, e.getMessage());
293293
}
@@ -335,7 +335,8 @@ private boolean assertSerialization(DocumentMapper mapper) {
335335
}
336336

337337
public DocumentMapper parse(String mappingType, CompressedXContent mappingSource) throws MapperParsingException {
338-
return documentMapperParser.parse(mappingType, mappingSource);
338+
Mapping mapping = mappingParser.parse(mappingType, mappingSource);
339+
return new DocumentMapper(indexSettings, indexAnalyzers, documentParser, mapping);
339340
}
340341

341342
/**

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,14 @@
88

99
package org.elasticsearch.index.mapper;
1010

11-
import static java.util.Collections.unmodifiableMap;
11+
import org.elasticsearch.Version;
12+
import org.elasticsearch.common.Strings;
13+
import org.elasticsearch.common.xcontent.ToXContent;
14+
import org.elasticsearch.common.xcontent.ToXContentFragment;
15+
import org.elasticsearch.common.xcontent.XContentBuilder;
16+
import org.elasticsearch.common.xcontent.XContentFactory;
17+
import org.elasticsearch.common.xcontent.XContentHelper;
18+
import org.elasticsearch.index.mapper.MapperService.MergeReason;
1219

1320
import java.io.IOException;
1421
import java.io.UncheckedIOException;
@@ -18,14 +25,7 @@
1825
import java.util.HashMap;
1926
import java.util.Map;
2027

21-
import org.elasticsearch.Version;
22-
import org.elasticsearch.common.Strings;
23-
import org.elasticsearch.common.xcontent.ToXContent;
24-
import org.elasticsearch.common.xcontent.ToXContentFragment;
25-
import org.elasticsearch.common.xcontent.XContentBuilder;
26-
import org.elasticsearch.common.xcontent.XContentFactory;
27-
import org.elasticsearch.common.xcontent.XContentHelper;
28-
import org.elasticsearch.index.mapper.MapperService.MergeReason;
28+
import static java.util.Collections.unmodifiableMap;
2929

3030
/**
3131
* Wrapper around everything that defines a mapping, without references to

server/src/main/java/org/elasticsearch/index/mapper/DocumentMapperParser.java renamed to server/src/main/java/org/elasticsearch/index/mapper/MappingParser.java

Lines changed: 73 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@
1212
import org.elasticsearch.common.compress.CompressedXContent;
1313
import org.elasticsearch.common.xcontent.XContentHelper;
1414
import org.elasticsearch.common.xcontent.XContentType;
15-
import org.elasticsearch.index.IndexSettings;
16-
import org.elasticsearch.index.analysis.IndexAnalyzers;
1715

1816
import java.util.Collections;
1917
import java.util.HashMap;
@@ -22,34 +20,58 @@
2220
import java.util.function.Function;
2321
import java.util.function.Supplier;
2422

25-
public class DocumentMapperParser {
26-
private final IndexSettings indexSettings;
27-
private final IndexAnalyzers indexAnalyzers;
28-
private final Function<String, String> documentTypeResolver;
29-
private final DocumentParser documentParser;
30-
private final Supplier<Map<Class<? extends MetadataFieldMapper>, MetadataFieldMapper>> metadataMappersSupplier;
23+
/**
24+
* Parser for {@link Mapping} provided in {@link CompressedXContent} format
25+
*/
26+
public final class MappingParser {
3127
private final Supplier<Mapper.TypeParser.ParserContext> parserContextSupplier;
3228
private final RootObjectMapper.TypeParser rootObjectTypeParser = new RootObjectMapper.TypeParser();
33-
private final Map<String, MetadataFieldMapper.TypeParser> rootTypeParsers;
29+
private final Supplier<Map<Class<? extends MetadataFieldMapper>, MetadataFieldMapper>> metadataMappersSupplier;
30+
private final Map<String, MetadataFieldMapper.TypeParser> metadataMapperParsers;
31+
private final Function<String, String> documentTypeResolver;
3432

35-
DocumentMapperParser(IndexSettings indexSettings,
36-
IndexAnalyzers indexAnalyzers,
37-
Function<String, String> documentTypeResolver,
38-
DocumentParser documentParser,
39-
Supplier<Map<Class<? extends MetadataFieldMapper>, MetadataFieldMapper>> metadataMappersSupplier,
40-
Supplier<Mapper.TypeParser.ParserContext> parserContextSupplier,
41-
Map<String, MetadataFieldMapper.TypeParser> metadataMapperParsers) {
42-
this.indexSettings = indexSettings;
43-
this.indexAnalyzers = indexAnalyzers;
44-
this.documentTypeResolver = documentTypeResolver;
45-
this.documentParser = documentParser;
46-
this.metadataMappersSupplier = metadataMappersSupplier;
33+
MappingParser(Supplier<Mapper.TypeParser.ParserContext> parserContextSupplier,
34+
Map<String, MetadataFieldMapper.TypeParser> metadataMapperParsers,
35+
Supplier<Map<Class<? extends MetadataFieldMapper>, MetadataFieldMapper>> metadataMappersSupplier,
36+
Function<String, String> documentTypeResolver) {
4737
this.parserContextSupplier = parserContextSupplier;
48-
this.rootTypeParsers = metadataMapperParsers;
38+
this.metadataMapperParsers = metadataMapperParsers;
39+
this.metadataMappersSupplier = metadataMappersSupplier;
40+
this.documentTypeResolver = documentTypeResolver;
41+
}
42+
43+
/**
44+
* Verify that there are no remaining fields in the provided map that contained mapped fields
45+
*
46+
* @param fieldName the name of the field that is being parsed
47+
* @param fieldNodeMap the map of fields
48+
*/
49+
public static void checkNoRemainingFields(String fieldName, Map<?, ?> fieldNodeMap) {
50+
checkNoRemainingFields(fieldNodeMap, "Mapping definition for [" + fieldName + "] has unsupported parameters: ");
51+
}
52+
53+
/**
54+
* Verify that there are no remaining fields in the provided map that contained mapped fields
55+
*
56+
* @param fieldNodeMap the map of fields
57+
* @param message the error message to be returned in case the provided map contains one or more fields
58+
*/
59+
public static void checkNoRemainingFields(Map<?, ?> fieldNodeMap, String message) {
60+
if (fieldNodeMap.isEmpty() == false) {
61+
throw new MapperParsingException(message + getRemainingFields(fieldNodeMap));
62+
}
63+
}
64+
65+
private static String getRemainingFields(Map<?, ?> map) {
66+
StringBuilder remainingFields = new StringBuilder();
67+
for (Object key : map.keySet()) {
68+
remainingFields.append(" [").append(key).append(" : ").append(map.get(key)).append("]");
69+
}
70+
return remainingFields.toString();
4971
}
5072

5173
@SuppressWarnings("unchecked")
52-
DocumentMapper parse(@Nullable String type, CompressedXContent source) throws MapperParsingException {
74+
Mapping parse(@Nullable String type, CompressedXContent source) throws MapperParsingException {
5375
Map<String, Object> mapping = null;
5476
if (source != null) {
5577
mapping = XContentHelper.convertToMap(source.compressedReference(), true, XContentType.JSON).v2();
@@ -65,82 +87,68 @@ DocumentMapper parse(@Nullable String type, CompressedXContent source) throws Ma
6587
}
6688
}
6789
}
90+
if (type == null) {
91+
throw new MapperParsingException("Failed to derive type");
92+
}
6893
if (mapping == null) {
6994
mapping = new HashMap<>();
7095
}
7196
return parse(type, mapping);
7297
}
7398

74-
@SuppressWarnings({"unchecked"})
75-
private DocumentMapper parse(String type, Map<String, Object> mapping) throws MapperParsingException {
76-
if (type == null) {
77-
throw new MapperParsingException("Failed to derive type");
78-
}
79-
99+
private Mapping parse(String type, Map<String, Object> mapping) throws MapperParsingException {
100+
ContentPath contentPath = new ContentPath(1);
80101
Mapper.TypeParser.ParserContext parserContext = parserContextSupplier.get();
81-
// parse RootObjectMapper
82-
RootObjectMapper.Builder root = (RootObjectMapper.Builder) rootObjectTypeParser.parse(type, mapping, parserContext);
83-
DocumentMapper.Builder docBuilder = new DocumentMapper.Builder(root, indexSettings, indexAnalyzers, documentParser,
84-
metadataMappersSupplier.get());
102+
RootObjectMapper rootObjectMapper = rootObjectTypeParser.parse(type, mapping, parserContext).build(contentPath);
103+
104+
Map<Class<? extends MetadataFieldMapper>, MetadataFieldMapper> metadataMappers = metadataMappersSupplier.get();
105+
Map<String, Object> meta = null;
106+
85107
Iterator<Map.Entry<String, Object>> iterator = mapping.entrySet().iterator();
86-
// parse DocumentMapper
87-
while(iterator.hasNext()) {
108+
while (iterator.hasNext()) {
88109
Map.Entry<String, Object> entry = iterator.next();
89110
String fieldName = entry.getKey();
90111
Object fieldNode = entry.getValue();
91112

92-
MetadataFieldMapper.TypeParser typeParser = rootTypeParsers.get(fieldName);
113+
MetadataFieldMapper.TypeParser typeParser = metadataMapperParsers.get(fieldName);
93114
if (typeParser != null) {
94115
iterator.remove();
95116
if (false == fieldNode instanceof Map) {
96117
throw new IllegalArgumentException("[_parent] must be an object containing [type]");
97118
}
119+
@SuppressWarnings("unchecked")
98120
Map<String, Object> fieldNodeMap = (Map<String, Object>) fieldNode;
99-
docBuilder.put(typeParser.parse(fieldName, fieldNodeMap, parserContext));
121+
MetadataFieldMapper metadataFieldMapper = typeParser.parse(fieldName, fieldNodeMap, parserContext).build(contentPath);
122+
metadataMappers.put(metadataFieldMapper.getClass(), metadataFieldMapper);
100123
fieldNodeMap.remove("type");
101124
checkNoRemainingFields(fieldName, fieldNodeMap);
102125
}
103126
}
104127

105-
Map<String, Object> meta = (Map<String, Object>) mapping.remove("_meta");
106-
if (meta != null) {
128+
@SuppressWarnings("unchecked")
129+
Map<String, Object> removed = (Map<String, Object>) mapping.remove("_meta");
130+
if (removed != null) {
107131
/*
108132
* It may not be required to copy meta here to maintain immutability but the cost is pretty low here.
109133
*
110-
* Note: this copy can not be replaced by Map#copyOf because we rely on consistent serialization order since we do byte-level
111-
* checks on the mapping between what we receive from the master and what we have locally. As Map#copyOf is not necessarily
112-
* the same underlying map implementation, we could end up with a different iteration order. For reference, see
113-
* MapperService#assertSerializtion and GitHub issues #10302 and #10318.
134+
* Note: this copy can not be replaced by Map#copyOf because we rely on consistent serialization order since we do
135+
* byte-level checks on the mapping between what we receive from the master and what we have locally. As Map#copyOf
136+
* is not necessarily the same underlying map implementation, we could end up with a different iteration order.
137+
* For reference, see MapperService#assertSerializtion and GitHub issues #10302 and #10318.
114138
*
115139
* Do not change this to Map#copyOf or any other method of copying meta that could change the iteration order.
116140
*
117141
* TODO:
118142
* - this should almost surely be a copy as a LinkedHashMap to have the ordering guarantees that we are relying on
119-
* - investigate the above note about whether or not we really need to be copying here, the ideal outcome would be to not
143+
* - investigate the above note about whether or not we need to be copying here, the ideal outcome would be to not
120144
*/
121-
docBuilder.meta(Collections.unmodifiableMap(new HashMap<>(meta)));
145+
meta = Collections.unmodifiableMap(new HashMap<>(removed));
122146
}
123-
124147
checkNoRemainingFields(mapping, "Root mapping definition has unsupported parameters: ");
125148

126-
return docBuilder.build();
127-
}
128-
129-
public static void checkNoRemainingFields(String fieldName, Map<?, ?> fieldNodeMap) {
130-
checkNoRemainingFields(fieldNodeMap, "Mapping definition for [" + fieldName + "] has unsupported parameters: ");
131-
}
132-
133-
public static void checkNoRemainingFields(Map<?, ?> fieldNodeMap, String message) {
134-
if (fieldNodeMap.isEmpty() == false) {
135-
throw new MapperParsingException(message + getRemainingFields(fieldNodeMap));
136-
}
137-
}
138-
139-
private static String getRemainingFields(Map<?, ?> map) {
140-
StringBuilder remainingFields = new StringBuilder();
141-
for (Object key : map.keySet()) {
142-
remainingFields.append(" [").append(key).append(" : ").append(map.get(key)).append("]");
143-
}
144-
return remainingFields.toString();
149+
return new Mapping(
150+
rootObjectMapper,
151+
metadataMappers.values().toArray(new MetadataFieldMapper[0]),
152+
meta);
145153
}
146154
}

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,7 @@ protected static void parseProperties(ObjectMapper.Builder objBuilder, Map<Strin
328328
}
329329
objBuilder.add(fieldBuilder);
330330
propNode.remove("type");
331-
DocumentMapperParser.checkNoRemainingFields(fieldName, propNode);
331+
MappingParser.checkNoRemainingFields(fieldName, propNode);
332332
iterator.remove();
333333
} else if (isEmptyList) {
334334
iterator.remove();
@@ -338,10 +338,8 @@ protected static void parseProperties(ObjectMapper.Builder objBuilder, Map<Strin
338338
}
339339
}
340340

341-
DocumentMapperParser.checkNoRemainingFields(propsNode, "DocType mapping definition has unsupported parameters: ");
342-
341+
MappingParser.checkNoRemainingFields(propsNode, "DocType mapping definition has unsupported parameters: ");
343342
}
344-
345343
}
346344

347345
private final String fullPath;

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,8 @@ private static void fixRedundantIncludes(ObjectMapper objectMapper, boolean pare
131131
static final class TypeParser extends ObjectMapper.TypeParser {
132132

133133
@Override
134-
public Mapper.Builder parse(String name, Map<String, Object> node, ParserContext parserContext) throws MapperParsingException {
134+
public RootObjectMapper.Builder parse(String name, Map<String, Object> node, ParserContext parserContext)
135+
throws MapperParsingException {
135136
RootObjectMapper.Builder builder = new Builder(name, parserContext.indexVersionCreated());
136137
Iterator<Map.Entry<String, Object>> iterator = node.entrySet().iterator();
137138
while (iterator.hasNext()) {

0 commit comments

Comments
 (0)