Skip to content

Commit b29c09b

Browse files
javannajavanna
authored
Don't require DocumentMapper as an argument when parsing a document (#66780)
Currently, an incoming document is parsed through `DocumentMapper#parse`, which in turns calls `DocumentParser#parseDocument` providing `this` among other arguments. As part of the effort to reduce usages of `DocumentMapper` when possible, as it represents the mutable side of mappings (through mappings updates) and involves complexity, we can carry around only the needed components. This does add some required arguments to `DocumentParser#parseDocument` , though it makes dependencies clearer. This change does not affect end consumers as they all go through DocumentMapper anyways, but by not needed to provide DocumentMapper to parseDocument, we may be able to unblock further improvements down the line. Relates to #66295
1 parent dad3396 commit b29c09b

File tree

6 files changed

+66
-45
lines changed

6 files changed

+66
-45
lines changed

‎modules/percolator/src/test/java/org/elasticsearch/percolator/CandidateQueryTests.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1119,7 +1119,8 @@ private void duelRun(PercolateQuery.QueryStore queryStore, MemoryIndex memoryInd
11191119
}
11201120

11211121
private void addQuery(Query query, List<ParseContext.Document> docs) {
1122-
ParseContext.InternalParseContext parseContext = new ParseContext.InternalParseContext(documentMapper, null, null, null, null);
1122+
ParseContext.InternalParseContext parseContext = new ParseContext.InternalParseContext(documentMapper.mapping(),
1123+
documentMapper.mappers(), mapperService.getIndexSettings(), mapperService.getIndexAnalyzers(), null, null, null, null);
11231124
fieldMapper.processQuery(query, parseContext);
11241125
ParseContext.Document queryDocument = parseContext.doc();
11251126
// Add to string representation of the query to make debugging easier:

‎modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorFieldMapperTests.java

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,8 @@ public void testExtractTerms() throws Exception {
181181

182182
DocumentMapper documentMapper = mapperService.documentMapper("doc");
183183
PercolatorFieldMapper fieldMapper = (PercolatorFieldMapper) documentMapper.mappers().getMapper(fieldName);
184-
ParseContext.InternalParseContext parseContext = new ParseContext.InternalParseContext(documentMapper, null, null, null, null);
184+
ParseContext.InternalParseContext parseContext = new ParseContext.InternalParseContext(documentMapper.mapping(),
185+
documentMapper.mappers(), mapperService.getIndexSettings(), mapperService.getIndexAnalyzers(), null, null, null, null);
185186
fieldMapper.processQuery(bq.build(), parseContext);
186187
ParseContext.Document document = parseContext.doc();
187188

@@ -202,7 +203,8 @@ public void testExtractTerms() throws Exception {
202203
bq.add(termQuery1, Occur.MUST);
203204
bq.add(termQuery2, Occur.MUST);
204205

205-
parseContext = new ParseContext.InternalParseContext(documentMapper, null, null, null, null);
206+
parseContext = new ParseContext.InternalParseContext(documentMapper.mapping(),
207+
documentMapper.mappers(), mapperService.getIndexSettings(), mapperService.getIndexAnalyzers(), null, null, null, null);
206208
fieldMapper.processQuery(bq.build(), parseContext);
207209
document = parseContext.doc();
208210

@@ -231,7 +233,8 @@ public void testExtractRanges() throws Exception {
231233

232234
DocumentMapper documentMapper = mapperService.documentMapper("doc");
233235
PercolatorFieldMapper fieldMapper = (PercolatorFieldMapper) documentMapper.mappers().getMapper(fieldName);
234-
ParseContext.InternalParseContext parseContext = new ParseContext.InternalParseContext(documentMapper, null, null, null, null);
236+
ParseContext.InternalParseContext parseContext = new ParseContext.InternalParseContext(documentMapper.mapping(),
237+
documentMapper.mappers(), mapperService.getIndexSettings(), mapperService.getIndexAnalyzers(), null, null, null, null);
235238
fieldMapper.processQuery(bq.build(), parseContext);
236239
ParseContext.Document document = parseContext.doc();
237240

@@ -256,7 +259,8 @@ public void testExtractRanges() throws Exception {
256259
.rangeQuery(15, 20, true, true, null, null, null, context);
257260
bq.add(rangeQuery2, Occur.MUST);
258261

259-
parseContext = new ParseContext.InternalParseContext(documentMapper, null, null, null, null);
262+
parseContext = new ParseContext.InternalParseContext(documentMapper.mapping(),
263+
documentMapper.mappers(), mapperService.getIndexSettings(), mapperService.getIndexAnalyzers(), null, null, null, null);
260264
fieldMapper.processQuery(bq.build(), parseContext);
261265
document = parseContext.doc();
262266

@@ -279,7 +283,8 @@ public void testExtractTermsAndRanges_failed() throws Exception {
279283
TermRangeQuery query = new TermRangeQuery("field1", new BytesRef("a"), new BytesRef("z"), true, true);
280284
DocumentMapper documentMapper = mapperService.documentMapper("doc");
281285
PercolatorFieldMapper fieldMapper = (PercolatorFieldMapper) documentMapper.mappers().getMapper(fieldName);
282-
ParseContext.InternalParseContext parseContext = new ParseContext.InternalParseContext(documentMapper, null, null, null, null);
286+
ParseContext.InternalParseContext parseContext = new ParseContext.InternalParseContext(documentMapper.mapping(),
287+
documentMapper.mappers(), mapperService.getIndexSettings(), mapperService.getIndexAnalyzers(), null, null, null, null);
283288
fieldMapper.processQuery(query, parseContext);
284289
ParseContext.Document document = parseContext.doc();
285290

@@ -293,7 +298,8 @@ public void testExtractTermsAndRanges_partial() throws Exception {
293298
PhraseQuery phraseQuery = new PhraseQuery("field", "term");
294299
DocumentMapper documentMapper = mapperService.documentMapper("doc");
295300
PercolatorFieldMapper fieldMapper = (PercolatorFieldMapper) documentMapper.mappers().getMapper(fieldName);
296-
ParseContext.InternalParseContext parseContext = new ParseContext.InternalParseContext(documentMapper, null, null, null, null);
301+
ParseContext.InternalParseContext parseContext = new ParseContext.InternalParseContext(documentMapper.mapping(),
302+
documentMapper.mappers(), mapperService.getIndexSettings(), mapperService.getIndexAnalyzers(), null, null, null, null);
297303
fieldMapper.processQuery(phraseQuery, parseContext);
298304
ParseContext.Document document = parseContext.doc();
299305

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

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@
4343
import java.util.function.Function;
4444
import java.util.stream.Stream;
4545

46-
4746
public class DocumentMapper implements ToXContentFragment {
4847

4948
public static class Builder {
@@ -134,14 +133,6 @@ private DocumentMapper(IndexSettings indexSettings,
134133
.filter(field -> noopTombstoneMetadataFields.contains(field.name())).toArray(MetadataFieldMapper[]::new);
135134
}
136135

137-
IndexSettings indexSettings() {
138-
return indexSettings;
139-
}
140-
141-
IndexAnalyzers indexAnalyzers() {
142-
return indexAnalyzers;
143-
}
144-
145136
public Mapping mapping() {
146137
return mapping;
147138
}
@@ -203,18 +194,20 @@ public MappingLookup mappers() {
203194
}
204195

205196
public ParsedDocument parse(SourceToParse source) throws MapperParsingException {
206-
return documentParser.parseDocument(source, mapping.metadataMappers, this);
197+
return documentParser.parseDocument(source, mapping, fieldMappers, indexSettings, indexAnalyzers);
207198
}
208199

209200
public ParsedDocument createDeleteTombstoneDoc(String index, String type, String id) throws MapperParsingException {
210201
final SourceToParse emptySource = new SourceToParse(index, type, id, new BytesArray("{}"), XContentType.JSON);
211-
return documentParser.parseDocument(emptySource, deleteTombstoneMetadataFieldMappers, this).toTombstone();
202+
return documentParser.parseDocument(emptySource, mapping, deleteTombstoneMetadataFieldMappers, fieldMappers,
203+
indexSettings, indexAnalyzers).toTombstone();
212204
}
213205

214206
public ParsedDocument createNoopTombstoneDoc(String index, String reason) throws MapperParsingException {
215207
final String id = ""; // _id won't be used.
216208
final SourceToParse sourceToParse = new SourceToParse(index, type, id, new BytesArray("{}"), XContentType.JSON);
217-
final ParsedDocument parsedDoc = documentParser.parseDocument(sourceToParse, noopTombstoneMetadataFieldMappers, this).toTombstone();
209+
final ParsedDocument parsedDoc = documentParser.parseDocument(sourceToParse, mapping, noopTombstoneMetadataFieldMappers,
210+
fieldMappers, indexSettings, indexAnalyzers).toTombstone();
218211
// Store the reason of a noop as a raw string in the _source field
219212
final BytesRef byteRef = new BytesRef(reason);
220213
parsedDoc.rootDoc().add(new StoredField(SourceFieldMapper.NAME, byteRef.bytes, byteRef.offset, byteRef.length));

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

Lines changed: 35 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@
4040
import org.elasticsearch.common.xcontent.XContentHelper;
4141
import org.elasticsearch.common.xcontent.XContentParser;
4242
import org.elasticsearch.common.xcontent.XContentType;
43+
import org.elasticsearch.index.IndexSettings;
44+
import org.elasticsearch.index.analysis.IndexAnalyzers;
4345
import org.elasticsearch.index.query.QueryShardContext;
4446

4547
/** A parser for documents, given mappings from a DocumentMapper */
@@ -58,19 +60,34 @@ final class DocumentParser {
5860
}
5961

6062
ParsedDocument parseDocument(SourceToParse source,
61-
MetadataFieldMapper[] metadataFieldsMappers,
62-
DocumentMapper docMapper) throws MapperParsingException {
63-
validateType(source, docMapper);
63+
Mapping mapping,
64+
MappingLookup mappingLookup,
65+
IndexSettings indexSettings,
66+
IndexAnalyzers indexAnalyzers) throws MapperParsingException {
67+
return parseDocument(source, mapping, mapping.metadataMappers, mappingLookup, indexSettings, indexAnalyzers);
68+
}
6469

65-
final Mapping mapping = docMapper.mapping();
70+
ParsedDocument parseDocument(SourceToParse source,
71+
Mapping mapping,
72+
MetadataFieldMapper[] metadataFieldsMappers,
73+
MappingLookup mappingLookup,
74+
IndexSettings indexSettings,
75+
IndexAnalyzers indexAnalyzers) throws MapperParsingException {
76+
validateType(source, mappingLookup.getType());
6677
final ParseContext.InternalParseContext context;
6778
final XContentType xContentType = source.getXContentType();
68-
6979
try (XContentParser parser = XContentHelper.createParser(xContentRegistry,
7080
LoggingDeprecationHandler.INSTANCE, source.source(), xContentType)) {
71-
context = new ParseContext.InternalParseContext(docMapper, dateParserContext, dynamicRuntimeFieldsBuilder, source, parser);
81+
context = new ParseContext.InternalParseContext(mapping,
82+
mappingLookup,
83+
indexSettings,
84+
indexAnalyzers,
85+
dateParserContext,
86+
dynamicRuntimeFieldsBuilder,
87+
source,
88+
parser);
7289
validateStart(parser);
73-
internalParseDocument(mapping, metadataFieldsMappers, context, parser);
90+
internalParseDocument(mapping.root(), metadataFieldsMappers, context, parser);
7491
validateEnd(parser);
7592
} catch (Exception e) {
7693
throw wrapInMapperParsingException(source, e);
@@ -82,7 +99,7 @@ ParsedDocument parseDocument(SourceToParse source,
8299

83100
context.postParse();
84101

85-
return parsedDocument(source, context, createDynamicUpdate(mapping, docMapper.mappers(),
102+
return parsedDocument(source, context, createDynamicUpdate(mapping, mappingLookup,
86103
context.getDynamicMappers(), context.getDynamicRuntimeFields()));
87104
}
88105

@@ -100,35 +117,34 @@ private static boolean containsDisabledObjectMapper(ObjectMapper objectMapper, S
100117
return false;
101118
}
102119

103-
private static void internalParseDocument(Mapping mapping, MetadataFieldMapper[] metadataFieldsMappers,
120+
private static void internalParseDocument(RootObjectMapper root, MetadataFieldMapper[] metadataFieldsMappers,
104121
ParseContext context, XContentParser parser) throws IOException {
105-
final boolean emptyDoc = isEmptyDoc(mapping, parser);
122+
final boolean emptyDoc = isEmptyDoc(root, parser);
106123

107124
for (MetadataFieldMapper metadataMapper : metadataFieldsMappers) {
108125
metadataMapper.preParse(context);
109126
}
110127

111-
if (mapping.root.isEnabled() == false) {
128+
if (root.isEnabled() == false) {
112129
// entire type is disabled
113130
parser.skipChildren();
114131
} else if (emptyDoc == false) {
115-
parseObjectOrNested(context, mapping.root);
132+
parseObjectOrNested(context, root);
116133
}
117134

118135
for (MetadataFieldMapper metadataMapper : metadataFieldsMappers) {
119136
metadataMapper.postParse(context);
120137
}
121138
}
122139

123-
private void validateType(SourceToParse source, DocumentMapper docMapper) {
124-
if (docMapper.type().equals(MapperService.DEFAULT_MAPPING)) {
140+
private void validateType(SourceToParse source, String type) {
141+
if (type.equals(MapperService.DEFAULT_MAPPING)) {
125142
throw new IllegalArgumentException("It is forbidden to index into the default mapping [" + MapperService.DEFAULT_MAPPING + "]");
126143
}
127144

128-
if (Objects.equals(source.type(), docMapper.type()) == false &&
145+
if (Objects.equals(source.type(), type) == false &&
129146
MapperService.SINGLE_MAPPING_NAME.equals(source.type()) == false) { // used by typeless APIs
130-
throw new MapperParsingException("Type mismatch, provide type [" + source.type() + "] but mapper is of type ["
131-
+ docMapper.type() + "]");
147+
throw new MapperParsingException("Type mismatch, provide type [" + source.type() + "] but mapper is of type [" + type + "]");
132148
}
133149
}
134150

@@ -150,8 +166,8 @@ private static void validateEnd(XContentParser parser) throws IOException {
150166
}
151167
}
152168

153-
private static boolean isEmptyDoc(Mapping mapping, XContentParser parser) throws IOException {
154-
if (mapping.root.isEnabled()) {
169+
private static boolean isEmptyDoc(RootObjectMapper root, XContentParser parser) throws IOException {
170+
if (root.isEnabled()) {
155171
final XContentParser.Token token = parser.nextToken();
156172
if (token == XContentParser.Token.END_OBJECT) {
157173
// empty doc, we can handle it...

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

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -342,23 +342,26 @@ public static class InternalParseContext extends ParseContext {
342342
private long numNestedDocs;
343343
private boolean docsReversed = false;
344344

345-
public InternalParseContext(DocumentMapper docMapper,
345+
public InternalParseContext(Mapping mapping,
346+
MappingLookup mappingLookup,
347+
IndexSettings indexSettings,
348+
IndexAnalyzers indexAnalyzers,
346349
Function<DateFormatter, Mapper.TypeParser.ParserContext> parserContextFunction,
347350
DynamicRuntimeFieldsBuilder dynamicRuntimeFieldsBuilder,
348351
SourceToParse source,
349352
XContentParser parser) {
350-
this.mapping = docMapper.mapping();
351-
this.mappingLookup = docMapper.mappers();
352-
this.indexSettings = docMapper.indexSettings();
353-
this.indexAnalyzers = docMapper.indexAnalyzers();
353+
this.mapping = mapping;
354+
this.mappingLookup = mappingLookup;
355+
this.indexSettings = indexSettings;
356+
this.indexAnalyzers = indexAnalyzers;
354357
this.parserContextFunction = parserContextFunction;
355358
this.dynamicRuntimeFieldsBuilder = dynamicRuntimeFieldsBuilder;
356359
this.parser = parser;
357360
this.document = new Document();
358361
this.documents.add(document);
359362
this.version = null;
360363
this.sourceToParse = source;
361-
this.maxAllowedNumNestedDocs = docMapper.indexSettings().getMappingNestedDocsLimit();
364+
this.maxAllowedNumNestedDocs = indexSettings.getMappingNestedDocsLimit();
362365
this.numNestedDocs = 0L;
363366
}
364367

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -617,7 +617,9 @@ MapperService createMapperService() throws Exception {
617617

618618
// creates an object mapper, which is about 100x harder than it should be....
619619
ObjectMapper createObjectMapper(MapperService mapperService, String name) {
620-
ParseContext context = new ParseContext.InternalParseContext(mapperService.documentMapper(), null, null, null, null);
620+
DocumentMapper docMapper = mapperService.documentMapper();
621+
ParseContext context = new ParseContext.InternalParseContext(docMapper.mapping(), docMapper.mappers(),
622+
mapperService.getIndexSettings(), mapperService.getIndexAnalyzers(), null, null, null, null);
621623
String[] nameParts = name.split("\\.");
622624
for (int i = 0; i < nameParts.length - 1; ++i) {
623625
context.path().add(nameParts[i]);

0 commit comments

Comments
 (0)