Skip to content

Commit dbefc05

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 f36371e commit dbefc05

File tree

6 files changed

+61
-39
lines changed

6 files changed

+61
-39
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
@@ -1113,7 +1113,8 @@ private void duelRun(PercolateQuery.QueryStore queryStore, MemoryIndex memoryInd
11131113
}
11141114

11151115
private void addQuery(Query query, List<ParseContext.Document> docs) {
1116-
ParseContext.InternalParseContext parseContext = new ParseContext.InternalParseContext(documentMapper, null, null, null, null);
1116+
ParseContext.InternalParseContext parseContext = new ParseContext.InternalParseContext(documentMapper.mapping(),
1117+
documentMapper.mappers(), mapperService.getIndexSettings(), mapperService.getIndexAnalyzers(), null, null, null, null);
11171118
fieldMapper.processQuery(query, parseContext);
11181119
ParseContext.Document queryDocument = parseContext.doc();
11191120
// 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();
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();
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();
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();
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
@@ -42,7 +42,6 @@
4242
import java.util.Objects;
4343
import java.util.stream.Stream;
4444

45-
4645
public class DocumentMapper implements ToXContentFragment {
4746

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

136-
IndexSettings indexSettings() {
137-
return indexSettings;
138-
}
139-
140-
IndexAnalyzers indexAnalyzers() {
141-
return indexAnalyzers;
142-
}
143-
144135
public Mapping mapping() {
145136
return mapping;
146137
}
@@ -194,18 +185,20 @@ public MappingLookup mappers() {
194185
}
195186

196187
public ParsedDocument parse(SourceToParse source) throws MapperParsingException {
197-
return documentParser.parseDocument(source, mapping.metadataMappers, this);
188+
return documentParser.parseDocument(source, mapping, fieldMappers, indexSettings, indexAnalyzers);
198189
}
199190

200191
public ParsedDocument createDeleteTombstoneDoc(String index, String id) throws MapperParsingException {
201192
final SourceToParse emptySource = new SourceToParse(index, id, new BytesArray("{}"), XContentType.JSON);
202-
return documentParser.parseDocument(emptySource, deleteTombstoneMetadataFieldMappers, this).toTombstone();
193+
return documentParser.parseDocument(emptySource, mapping, deleteTombstoneMetadataFieldMappers, fieldMappers,
194+
indexSettings, indexAnalyzers).toTombstone();
203195
}
204196

205197
public ParsedDocument createNoopTombstoneDoc(String index, String reason) throws MapperParsingException {
206198
final String id = ""; // _id won't be used.
207199
final SourceToParse sourceToParse = new SourceToParse(index, id, new BytesArray("{}"), XContentType.JSON);
208-
final ParsedDocument parsedDoc = documentParser.parseDocument(sourceToParse, noopTombstoneMetadataFieldMappers, this).toTombstone();
200+
final ParsedDocument parsedDoc = documentParser.parseDocument(sourceToParse, mapping, noopTombstoneMetadataFieldMappers,
201+
fieldMappers, indexSettings, indexAnalyzers).toTombstone();
209202
// Store the reason of a noop as a raw string in the _source field
210203
final BytesRef byteRef = new BytesRef(reason);
211204
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: 30 additions & 13 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,18 +60,33 @@ final class DocumentParser {
5860
}
5961

6062
ParsedDocument parseDocument(SourceToParse source,
61-
MetadataFieldMapper[] metadataFieldsMappers,
62-
DocumentMapper docMapper) throws MapperParsingException {
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+
}
6369

64-
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 {
6576
final ParseContext.InternalParseContext context;
6677
final XContentType xContentType = source.getXContentType();
67-
6878
try (XContentParser parser = XContentHelper.createParser(xContentRegistry,
6979
LoggingDeprecationHandler.INSTANCE, source.source(), xContentType)) {
70-
context = new ParseContext.InternalParseContext(docMapper, dateParserContext, dynamicRuntimeFieldsBuilder, source, parser);
80+
context = new ParseContext.InternalParseContext(mapping,
81+
mappingLookup,
82+
indexSettings,
83+
indexAnalyzers,
84+
dateParserContext,
85+
dynamicRuntimeFieldsBuilder,
86+
source,
87+
parser);
7188
validateStart(parser);
72-
internalParseDocument(mapping, metadataFieldsMappers, context, parser);
89+
internalParseDocument(mapping.root(), metadataFieldsMappers, context, parser);
7390
validateEnd(parser);
7491
} catch (Exception e) {
7592
throw wrapInMapperParsingException(source, e);
@@ -81,7 +98,7 @@ ParsedDocument parseDocument(SourceToParse source,
8198

8299
context.postParse();
83100

84-
return parsedDocument(source, context, createDynamicUpdate(mapping, docMapper.mappers(),
101+
return parsedDocument(source, context, createDynamicUpdate(mapping, mappingLookup,
85102
context.getDynamicMappers(), context.getDynamicRuntimeFields()));
86103
}
87104

@@ -99,19 +116,19 @@ private static boolean containsDisabledObjectMapper(ObjectMapper objectMapper, S
99116
return false;
100117
}
101118

102-
private static void internalParseDocument(Mapping mapping, MetadataFieldMapper[] metadataFieldsMappers,
119+
private static void internalParseDocument(RootObjectMapper root, MetadataFieldMapper[] metadataFieldsMappers,
103120
ParseContext context, XContentParser parser) throws IOException {
104-
final boolean emptyDoc = isEmptyDoc(mapping, parser);
121+
final boolean emptyDoc = isEmptyDoc(root, parser);
105122

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

110-
if (mapping.root.isEnabled() == false) {
127+
if (root.isEnabled() == false) {
111128
// entire type is disabled
112129
parser.skipChildren();
113130
} else if (emptyDoc == false) {
114-
parseObjectOrNested(context, mapping.root);
131+
parseObjectOrNested(context, root);
115132
}
116133

117134
for (MetadataFieldMapper metadataMapper : metadataFieldsMappers) {
@@ -137,8 +154,8 @@ private static void validateEnd(XContentParser parser) throws IOException {
137154
}
138155
}
139156

140-
private static boolean isEmptyDoc(Mapping mapping, XContentParser parser) throws IOException {
141-
if (mapping.root.isEnabled()) {
157+
private static boolean isEmptyDoc(RootObjectMapper root, XContentParser parser) throws IOException {
158+
if (root.isEnabled()) {
142159
final XContentParser.Token token = parser.nextToken();
143160
if (token == XContentParser.Token.END_OBJECT) {
144161
// 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
@@ -336,23 +336,26 @@ public static class InternalParseContext extends ParseContext {
336336
private long numNestedDocs;
337337
private boolean docsReversed = false;
338338

339-
public InternalParseContext(DocumentMapper docMapper,
339+
public InternalParseContext(Mapping mapping,
340+
MappingLookup mappingLookup,
341+
IndexSettings indexSettings,
342+
IndexAnalyzers indexAnalyzers,
340343
Function<DateFormatter, Mapper.TypeParser.ParserContext> parserContextFunction,
341344
DynamicRuntimeFieldsBuilder dynamicRuntimeFieldsBuilder,
342345
SourceToParse source,
343346
XContentParser parser) {
344-
this.mapping = docMapper.mapping();
345-
this.mappingLookup = docMapper.mappers();
346-
this.indexSettings = docMapper.indexSettings();
347-
this.indexAnalyzers = docMapper.indexAnalyzers();
347+
this.mapping = mapping;
348+
this.mappingLookup = mappingLookup;
349+
this.indexSettings = indexSettings;
350+
this.indexAnalyzers = indexAnalyzers;
348351
this.parserContextFunction = parserContextFunction;
349352
this.dynamicRuntimeFieldsBuilder = dynamicRuntimeFieldsBuilder;
350353
this.parser = parser;
351354
this.document = new Document();
352355
this.documents.add(document);
353356
this.version = null;
354357
this.sourceToParse = source;
355-
this.maxAllowedNumNestedDocs = docMapper.indexSettings().getMappingNestedDocsLimit();
358+
this.maxAllowedNumNestedDocs = indexSettings.getMappingNestedDocsLimit();
356359
this.numNestedDocs = 0L;
357360
}
358361

‎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)