Skip to content

Commit 6658ff0

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

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) {
@@ -338,7 +339,7 @@ private static ObjectMapper createUpdate(ObjectMapper parent, String[] nameParts
338339
return parent.mappingUpdate(mapper);
339340
}
340341

341-
static void parseObjectOrNested(ParseContext context, ObjectMapper mapper, boolean atRoot) throws IOException {
342+
static void parseObjectOrNested(ParseContext context, ObjectMapper mapper) throws IOException {
342343
if (mapper.isEnabled() == false) {
343344
context.parser().skipChildren();
344345
return;
@@ -467,7 +468,7 @@ private static ParseContext nestedContext(ParseContext context, ObjectMapper map
467468

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

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

519519
private static void parseArray(ParseContext context, ObjectMapper parentMapper, String lastFieldName) throws IOException {
520520
String arrayFieldName = lastFieldName;
521-
Mapper mapper = getMapper(parentMapper, lastFieldName);
521+
522+
final String[] paths = splitAndValidatePath(arrayFieldName);
523+
Mapper mapper = getMapper(parentMapper, lastFieldName, paths);
522524
if (mapper != null) {
523525
// There is a concrete mapper for this field already. Need to check if the mapper
524526
// expects an array, if so we pass the context straight to the mapper and if not
@@ -529,8 +531,6 @@ private static void parseArray(ParseContext context, ObjectMapper parentMapper,
529531
parseNonDynamicArray(context, parentMapper, lastFieldName, arrayFieldName);
530532
}
531533
} else {
532-
533-
final String[] paths = splitAndValidatePath(arrayFieldName);
534534
arrayFieldName = paths[paths.length - 1];
535535
lastFieldName = arrayFieldName;
536536
Tuple<Integer, ObjectMapper> parentMapperTuple = getDynamicParentMapper(context, paths, parentMapper);
@@ -589,12 +589,12 @@ private static void parseValue(final ParseContext context, ObjectMapper parentMa
589589
if (currentFieldName == null) {
590590
throw new MapperParsingException("object mapping [" + parentMapper.name() + "] trying to serialize a value with no field associated with it, current value [" + context.parser().textOrNull() + "]");
591591
}
592-
Mapper mapper = getMapper(parentMapper, currentFieldName);
592+
593+
final String[] paths = splitAndValidatePath(currentFieldName);
594+
Mapper mapper = getMapper(parentMapper, currentFieldName, paths);
593595
if (mapper != null) {
594596
parseObjectOrField(context, mapper);
595597
} else {
596-
597-
final String[] paths = splitAndValidatePath(currentFieldName);
598598
currentFieldName = paths[paths.length - 1];
599599
Tuple<Integer, ObjectMapper> parentMapperTuple = getDynamicParentMapper(context, paths, parentMapper);
600600
parentMapper = parentMapperTuple.v2();
@@ -607,7 +607,7 @@ private static void parseValue(final ParseContext context, ObjectMapper parentMa
607607

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

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

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

931931
// looks up a child mapper, but takes into account field names that expand to objects
932-
static Mapper getMapper(ObjectMapper objectMapper, String fieldName) {
933-
String[] subfields = splitAndValidatePath(fieldName);
932+
private static Mapper getMapper(ObjectMapper objectMapper, String fieldName, String[] subfields) {
934933
for (int i = 0; i < subfields.length - 1; ++i) {
935934
Mapper mapper = objectMapper.getMapper(subfields[i]);
936935
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
@@ -805,10 +805,10 @@ public void testPercolatorQueryViaMultiSearch() throws Exception {
805805
jsonBuilder().startObject().field("field1", "b").endObject().bytes(), XContentType.JSON)))
806806
.add(client().prepareSearch("test")
807807
.setQuery(new PercolateQueryBuilder("query",
808-
yamlBuilder().startObject().field("field1", "c").endObject().bytes(), XContentType.JSON)))
808+
yamlBuilder().startObject().field("field1", "c").endObject().bytes(), XContentType.YAML)))
809809
.add(client().prepareSearch("test")
810810
.setQuery(new PercolateQueryBuilder("query",
811-
smileBuilder().startObject().field("field1", "b c").endObject().bytes(), XContentType.JSON)))
811+
smileBuilder().startObject().field("field1", "b c").endObject().bytes(), XContentType.SMILE)))
812812
.add(client().prepareSearch("test")
813813
.setQuery(new PercolateQueryBuilder("query",
814814
jsonBuilder().startObject().field("field1", "d").endObject().bytes(), XContentType.JSON)))

0 commit comments

Comments
 (0)