Skip to content

Commit 004c409

Browse files
tlrxtlrx
committed
Don't detect source's XContentType in DocumentParser.parseDocument() (#26880)
DocumentParser.parseDocument() auto detects the XContentType of the document to parse, but this information is already provided by SourceToParse.
1 parent 1d3776f commit 004c409

File tree

4 files changed

+26
-28
lines changed

4 files changed

+26
-28
lines changed

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

Lines changed: 21 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import org.elasticsearch.common.joda.FormatDateTimeFormatter;
2828
import org.elasticsearch.common.xcontent.XContentHelper;
2929
import org.elasticsearch.common.xcontent.XContentParser;
30+
import org.elasticsearch.common.xcontent.XContentType;
3031
import org.elasticsearch.index.IndexSettings;
3132
import org.elasticsearch.index.mapper.DynamicTemplate.XContentFieldType;
3233
import org.elasticsearch.index.mapper.KeywordFieldMapper.KeywordFieldType;
@@ -58,9 +59,10 @@ ParsedDocument parseDocument(SourceToParse source) throws MapperParsingException
5859

5960
final Mapping mapping = docMapper.mapping();
6061
final ParseContext.InternalParseContext context;
61-
try (XContentParser parser = XContentHelper.createParser(docMapperParser.getXContentRegistry(), source.source())) {
62-
context = new ParseContext.InternalParseContext(indexSettings.getSettings(),
63-
docMapperParser, docMapper, source, parser);
62+
final XContentType xContentType = source.getXContentType();
63+
64+
try (XContentParser parser = XContentHelper.createParser(docMapperParser.getXContentRegistry(), source.source(), xContentType)) {
65+
context = new ParseContext.InternalParseContext(indexSettings.getSettings(), docMapperParser, docMapper, source, parser);
6466
validateStart(parser);
6567
internalParseDocument(mapping, context, parser);
6668
validateEnd(parser);
@@ -74,8 +76,7 @@ ParsedDocument parseDocument(SourceToParse source) throws MapperParsingException
7476

7577
reverseOrder(context);
7678

77-
ParsedDocument doc = parsedDocument(source, context, createDynamicUpdate(mapping, docMapper, context.getDynamicMappers()));
78-
return doc;
79+
return parsedDocument(source, context, createDynamicUpdate(mapping, docMapper, context.getDynamicMappers()));
7980
}
8081

8182
private static void internalParseDocument(Mapping mapping, ParseContext.InternalParseContext context, XContentParser parser) throws IOException {
@@ -89,7 +90,7 @@ private static void internalParseDocument(Mapping mapping, ParseContext.Internal
8990
// entire type is disabled
9091
parser.skipChildren();
9192
} else if (emptyDoc == false) {
92-
parseObjectOrNested(context, mapping.root, true);
93+
parseObjectOrNested(context, mapping.root);
9394
}
9495

9596
for (MetadataFieldMapper metadataMapper : mapping.metadataMappers) {
@@ -331,7 +332,7 @@ private static ObjectMapper createUpdate(ObjectMapper parent, String[] nameParts
331332
return parent.mappingUpdate(mapper);
332333
}
333334

334-
static void parseObjectOrNested(ParseContext context, ObjectMapper mapper, boolean atRoot) throws IOException {
335+
static void parseObjectOrNested(ParseContext context, ObjectMapper mapper) throws IOException {
335336
if (mapper.isEnabled() == false) {
336337
context.parser().skipChildren();
337338
return;
@@ -466,7 +467,7 @@ private static ParseContext nestedContext(ParseContext context, ObjectMapper map
466467

467468
private static void parseObjectOrField(ParseContext context, Mapper mapper) throws IOException {
468469
if (mapper instanceof ObjectMapper) {
469-
parseObjectOrNested(context, (ObjectMapper) mapper, false);
470+
parseObjectOrNested(context, (ObjectMapper) mapper);
470471
} else {
471472
FieldMapper fieldMapper = (FieldMapper)mapper;
472473
Mapper update = fieldMapper.parse(context);
@@ -482,14 +483,13 @@ private static void parseObjectOrField(ParseContext context, Mapper mapper) thro
482483
private static void parseObject(final ParseContext context, ObjectMapper mapper, String currentFieldName) throws IOException {
483484
assert currentFieldName != null;
484485

485-
Mapper objectMapper = getMapper(mapper, currentFieldName);
486+
final String[] paths = splitAndValidatePath(currentFieldName);
487+
Mapper objectMapper = getMapper(mapper, currentFieldName, paths);
486488
if (objectMapper != null) {
487489
context.path().add(currentFieldName);
488490
parseObjectOrField(context, objectMapper);
489491
context.path().remove();
490492
} else {
491-
492-
final String[] paths = splitAndValidatePath(currentFieldName);
493493
currentFieldName = paths[paths.length - 1];
494494
Tuple<Integer, ObjectMapper> parentMapperTuple = getDynamicParentMapper(context, paths, mapper);
495495
ObjectMapper parentMapper = parentMapperTuple.v2();
@@ -519,7 +519,9 @@ private static void parseObject(final ParseContext context, ObjectMapper mapper,
519519

520520
private static void parseArray(ParseContext context, ObjectMapper parentMapper, String lastFieldName) throws IOException {
521521
String arrayFieldName = lastFieldName;
522-
Mapper mapper = getMapper(parentMapper, lastFieldName);
522+
523+
final String[] paths = splitAndValidatePath(arrayFieldName);
524+
Mapper mapper = getMapper(parentMapper, lastFieldName, paths);
523525
if (mapper != null) {
524526
// There is a concrete mapper for this field already. Need to check if the mapper
525527
// expects an array, if so we pass the context straight to the mapper and if not
@@ -530,8 +532,6 @@ private static void parseArray(ParseContext context, ObjectMapper parentMapper,
530532
parseNonDynamicArray(context, parentMapper, lastFieldName, arrayFieldName);
531533
}
532534
} else {
533-
534-
final String[] paths = splitAndValidatePath(arrayFieldName);
535535
arrayFieldName = paths[paths.length - 1];
536536
lastFieldName = arrayFieldName;
537537
Tuple<Integer, ObjectMapper> parentMapperTuple = getDynamicParentMapper(context, paths, parentMapper);
@@ -590,12 +590,12 @@ private static void parseValue(final ParseContext context, ObjectMapper parentMa
590590
if (currentFieldName == null) {
591591
throw new MapperParsingException("object mapping [" + parentMapper.name() + "] trying to serialize a value with no field associated with it, current value [" + context.parser().textOrNull() + "]");
592592
}
593-
Mapper mapper = getMapper(parentMapper, currentFieldName);
593+
594+
final String[] paths = splitAndValidatePath(currentFieldName);
595+
Mapper mapper = getMapper(parentMapper, currentFieldName, paths);
594596
if (mapper != null) {
595597
parseObjectOrField(context, mapper);
596598
} else {
597-
598-
final String[] paths = splitAndValidatePath(currentFieldName);
599599
currentFieldName = paths[paths.length - 1];
600600
Tuple<Integer, ObjectMapper> parentMapperTuple = getDynamicParentMapper(context, paths, parentMapper);
601601
parentMapper = parentMapperTuple.v2();
@@ -608,7 +608,7 @@ private static void parseValue(final ParseContext context, ObjectMapper parentMa
608608

609609
private static void parseNullValue(ParseContext context, ObjectMapper parentMapper, String lastFieldName) throws IOException {
610610
// we can only handle null values if we have mappings for them
611-
Mapper mapper = getMapper(parentMapper, lastFieldName);
611+
Mapper mapper = getMapper(parentMapper, lastFieldName, splitAndValidatePath(lastFieldName));
612612
if (mapper != null) {
613613
// TODO: passing null to an object seems bogus?
614614
parseObjectOrField(context, mapper);
@@ -894,15 +894,15 @@ private static Tuple<Integer, ObjectMapper> getDynamicParentMapper(ParseContext
894894
break;
895895
case FALSE:
896896
// Should not dynamically create any more mappers so return the last mapper
897-
return new Tuple<Integer, ObjectMapper>(pathsAdded, parent);
897+
return new Tuple<>(pathsAdded, parent);
898898

899899
}
900900
}
901901
context.path().add(paths[i]);
902902
pathsAdded++;
903903
parent = mapper;
904904
}
905-
return new Tuple<Integer, ObjectMapper>(pathsAdded, mapper);
905+
return new Tuple<>(pathsAdded, mapper);
906906
}
907907

908908
// find what the dynamic setting is given the current parse context and parent
@@ -930,8 +930,7 @@ private static ObjectMapper.Dynamic dynamicOrDefault(ObjectMapper parentMapper,
930930
}
931931

932932
// looks up a child mapper, but takes into account field names that expand to objects
933-
static Mapper getMapper(ObjectMapper objectMapper, String fieldName) {
934-
String[] subfields = splitAndValidatePath(fieldName);
933+
private static Mapper getMapper(ObjectMapper objectMapper, String fieldName, String[] subfields) {
935934
for (int i = 0; i < subfields.length - 1; ++i) {
936935
Mapper mapper = objectMapper.getMapper(subfields[i]);
937936
if (mapper == null || (mapper instanceof ObjectMapper) == false) {

‎core/src/test/java/org/elasticsearch/index/mapper/DynamicMappingTests.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ private Mapper parse(DocumentMapper mapper, DocumentMapperParser parser, XConten
217217
ParseContext.InternalParseContext ctx = new ParseContext.InternalParseContext(settings, parser, mapper, source, xContentParser);
218218
assertEquals(XContentParser.Token.START_OBJECT, ctx.parser().nextToken());
219219
ctx.parser().nextToken();
220-
DocumentParser.parseObjectOrNested(ctx, mapper.root(), true);
220+
DocumentParser.parseObjectOrNested(ctx, mapper.root());
221221
Mapping mapping = DocumentParser.createDynamicUpdate(mapper.mapping(), mapper, ctx.getDynamicMappers());
222222
return mapping == null ? null : mapping.root();
223223
}
@@ -639,8 +639,7 @@ private void doTestDefaultFloatingPointMappings(DocumentMapper mapper, XContentB
639639
.field("baz", (double) 3.2f) // double that can be accurately represented as a float
640640
.field("quux", "3.2") // float detected through numeric detection
641641
.endObject().bytes();
642-
ParsedDocument parsedDocument = mapper.parse(SourceToParse.source("index", "type", "id", source,
643-
XContentType.JSON));
642+
ParsedDocument parsedDocument = mapper.parse(SourceToParse.source("index", "type", "id", source, builder.contentType()));
644643
Mapping update = parsedDocument.dynamicMappingsUpdate();
645644
assertNotNull(update);
646645
assertThat(((FieldMapper) update.root().getMapper("foo")).fieldType().typeName(), equalTo("float"));

‎core/src/test/java/org/elasticsearch/index/mapper/SourceFieldMapperTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ public void testNoFormat() throws Exception {
6565
doc = documentMapper.parse(SourceToParse.source("test", "type", "1", XContentFactory.smileBuilder().startObject()
6666
.field("field", "value")
6767
.endObject().bytes(),
68-
XContentType.JSON));
68+
XContentType.SMILE));
6969

7070
assertThat(XContentFactory.xContentType(doc.source()), equalTo(XContentType.SMILE));
7171
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -606,10 +606,10 @@ public void testPercolatorQueryViaMultiSearch() throws Exception {
606606
jsonBuilder().startObject().field("field1", "b").endObject().bytes(), XContentType.JSON)))
607607
.add(client().prepareSearch("test")
608608
.setQuery(new PercolateQueryBuilder("query",
609-
yamlBuilder().startObject().field("field1", "c").endObject().bytes(), XContentType.JSON)))
609+
yamlBuilder().startObject().field("field1", "c").endObject().bytes(), XContentType.YAML)))
610610
.add(client().prepareSearch("test")
611611
.setQuery(new PercolateQueryBuilder("query",
612-
smileBuilder().startObject().field("field1", "b c").endObject().bytes(), XContentType.JSON)))
612+
smileBuilder().startObject().field("field1", "b c").endObject().bytes(), XContentType.SMILE)))
613613
.add(client().prepareSearch("test")
614614
.setQuery(new PercolateQueryBuilder("query",
615615
jsonBuilder().startObject().field("field1", "d").endObject().bytes(), XContentType.JSON)))

0 commit comments

Comments
 (0)