Skip to content

Commit

Permalink
Don't detect source's XContentType in DocumentParser.parseDocument()
Browse files Browse the repository at this point in the history 
DocumentParser.parseDocument() auto detects the XContentType of the
document to parse, but this information is already provided by SourceToParse.
  • Loading branch information
@tlrx
tlrx committed Oct 10, 2017
1 parent e22844b commit 11d71d1
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 25 deletions.
41 changes: 20 additions & 21 deletions core/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.elasticsearch.common.joda.FormatDateTimeFormatter;
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.index.mapper.DynamicTemplate.XContentFieldType;
import org.elasticsearch.index.mapper.KeywordFieldMapper.KeywordFieldType;
Expand Down Expand Up @@ -58,9 +59,10 @@ ParsedDocument parseDocument(SourceToParse source) throws MapperParsingException

final Mapping mapping = docMapper.mapping();
final ParseContext.InternalParseContext context;
try (XContentParser parser = XContentHelper.createParser(docMapperParser.getXContentRegistry(), source.source())) {
context = new ParseContext.InternalParseContext(indexSettings.getSettings(),
docMapperParser, docMapper, source, parser);
final XContentType xContentType = source.getXContentType();

try (XContentParser parser = XContentHelper.createParser(docMapperParser.getXContentRegistry(), source.source(), xContentType)) {
context = new ParseContext.InternalParseContext(indexSettings.getSettings(), docMapperParser, docMapper, source, parser);
validateStart(parser);
internalParseDocument(mapping, context, parser);
validateEnd(parser);
Expand All @@ -74,8 +76,7 @@ ParsedDocument parseDocument(SourceToParse source) throws MapperParsingException

reverseOrder(context);

ParsedDocument doc = parsedDocument(source, context, createDynamicUpdate(mapping, docMapper, context.getDynamicMappers()));
return doc;
return parsedDocument(source, context, createDynamicUpdate(mapping, docMapper, context.getDynamicMappers()));
}

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

for (MetadataFieldMapper metadataMapper : mapping.metadataMappers) {
Expand Down Expand Up @@ -338,7 +339,7 @@ private static ObjectMapper createUpdate(ObjectMapper parent, String[] nameParts
return parent.mappingUpdate(mapper);
}

static void parseObjectOrNested(ParseContext context, ObjectMapper mapper, boolean atRoot) throws IOException {
static void parseObjectOrNested(ParseContext context, ObjectMapper mapper) throws IOException {
if (mapper.isEnabled() == false) {
context.parser().skipChildren();
return;
Expand Down Expand Up @@ -467,7 +468,7 @@ private static ParseContext nestedContext(ParseContext context, ObjectMapper map

private static void parseObjectOrField(ParseContext context, Mapper mapper) throws IOException {
if (mapper instanceof ObjectMapper) {
parseObjectOrNested(context, (ObjectMapper) mapper, false);
parseObjectOrNested(context, (ObjectMapper) mapper);
} else {
FieldMapper fieldMapper = (FieldMapper)mapper;
Mapper update = fieldMapper.parse(context);
Expand All @@ -481,14 +482,13 @@ private static void parseObjectOrField(ParseContext context, Mapper mapper) thro
private static void parseObject(final ParseContext context, ObjectMapper mapper, String currentFieldName) throws IOException {
assert currentFieldName != null;

Mapper objectMapper = getMapper(mapper, currentFieldName);
final String[] paths = splitAndValidatePath(currentFieldName);
Mapper objectMapper = getMapper(mapper, currentFieldName, paths);
if (objectMapper != null) {
context.path().add(currentFieldName);
parseObjectOrField(context, objectMapper);
context.path().remove();
} else {

final String[] paths = splitAndValidatePath(currentFieldName);
currentFieldName = paths[paths.length - 1];
Tuple<Integer, ObjectMapper> parentMapperTuple = getDynamicParentMapper(context, paths, mapper);
ObjectMapper parentMapper = parentMapperTuple.v2();
Expand Down Expand Up @@ -518,7 +518,9 @@ private static void parseObject(final ParseContext context, ObjectMapper mapper,

private static void parseArray(ParseContext context, ObjectMapper parentMapper, String lastFieldName) throws IOException {
String arrayFieldName = lastFieldName;
Mapper mapper = getMapper(parentMapper, lastFieldName);

final String[] paths = splitAndValidatePath(arrayFieldName);
Mapper mapper = getMapper(parentMapper, lastFieldName, paths);
if (mapper != null) {
// There is a concrete mapper for this field already. Need to check if the mapper
// expects an array, if so we pass the context straight to the mapper and if not
Expand All @@ -529,8 +531,6 @@ private static void parseArray(ParseContext context, ObjectMapper parentMapper,
parseNonDynamicArray(context, parentMapper, lastFieldName, arrayFieldName);
}
} else {

final String[] paths = splitAndValidatePath(arrayFieldName);
arrayFieldName = paths[paths.length - 1];
lastFieldName = arrayFieldName;
Tuple<Integer, ObjectMapper> parentMapperTuple = getDynamicParentMapper(context, paths, parentMapper);
Expand Down Expand Up @@ -589,12 +589,12 @@ private static void parseValue(final ParseContext context, ObjectMapper parentMa
if (currentFieldName == null) {
throw new MapperParsingException("object mapping [" + parentMapper.name() + "] trying to serialize a value with no field associated with it, current value [" + context.parser().textOrNull() + "]");
}
Mapper mapper = getMapper(parentMapper, currentFieldName);

final String[] paths = splitAndValidatePath(currentFieldName);
Mapper mapper = getMapper(parentMapper, currentFieldName, paths);
if (mapper != null) {
parseObjectOrField(context, mapper);
} else {

final String[] paths = splitAndValidatePath(currentFieldName);
currentFieldName = paths[paths.length - 1];
Tuple<Integer, ObjectMapper> parentMapperTuple = getDynamicParentMapper(context, paths, parentMapper);
parentMapper = parentMapperTuple.v2();
Expand All @@ -607,7 +607,7 @@ private static void parseValue(final ParseContext context, ObjectMapper parentMa

private static void parseNullValue(ParseContext context, ObjectMapper parentMapper, String lastFieldName) throws IOException {
// we can only handle null values if we have mappings for them
Mapper mapper = getMapper(parentMapper, lastFieldName);
Mapper mapper = getMapper(parentMapper, lastFieldName, splitAndValidatePath(lastFieldName));
if (mapper != null) {
// TODO: passing null to an object seems bogus?
parseObjectOrField(context, mapper);
Expand Down Expand Up @@ -901,7 +901,7 @@ private static Tuple<Integer, ObjectMapper> getDynamicParentMapper(ParseContext
pathsAdded++;
parent = mapper;
}
return new Tuple<Integer, ObjectMapper>(pathsAdded, mapper);
return new Tuple<>(pathsAdded, mapper);
}

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

// looks up a child mapper, but takes into account field names that expand to objects
static Mapper getMapper(ObjectMapper objectMapper, String fieldName) {
String[] subfields = splitAndValidatePath(fieldName);
static Mapper getMapper(ObjectMapper objectMapper, String fieldName, String[] subfields) {
for (int i = 0; i < subfields.length - 1; ++i) {
Mapper mapper = objectMapper.getMapper(subfields[i]);
if (mapper == null || (mapper instanceof ObjectMapper) == false) {
Expand Down
5 changes: 2 additions & 3 deletions core/src/test/java/org/elasticsearch/index/mapper/DynamicMappingTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ private Mapper parse(DocumentMapper mapper, DocumentMapperParser parser, XConten
ParseContext.InternalParseContext ctx = new ParseContext.InternalParseContext(settings, parser, mapper, source, xContentParser);
assertEquals(XContentParser.Token.START_OBJECT, ctx.parser().nextToken());
ctx.parser().nextToken();
DocumentParser.parseObjectOrNested(ctx, mapper.root(), true);
DocumentParser.parseObjectOrNested(ctx, mapper.root());
Mapping mapping = DocumentParser.createDynamicUpdate(mapper.mapping(), mapper, ctx.getDynamicMappers());
return mapping == null ? null : mapping.root();
}
Expand Down Expand Up @@ -639,8 +639,7 @@ private void doTestDefaultFloatingPointMappings(DocumentMapper mapper, XContentB
.field("baz", (double) 3.2f) // double that can be accurately represented as a float
.field("quux", "3.2") // float detected through numeric detection
.endObject().bytes();
ParsedDocument parsedDocument = mapper.parse(SourceToParse.source("index", "type", "id", source,
XContentType.JSON));
ParsedDocument parsedDocument = mapper.parse(SourceToParse.source("index", "type", "id", source, builder.contentType()));
Mapping update = parsedDocument.dynamicMappingsUpdate();
assertNotNull(update);
assertThat(((FieldMapper) update.root().getMapper("foo")).fieldType().typeName(), equalTo("float"));
Expand Down
2 changes: 1 addition & 1 deletion core/src/test/java/org/elasticsearch/index/mapper/SourceFieldMapperTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public void testNoFormat() throws Exception {
doc = documentMapper.parse(SourceToParse.source("test", "type", "1", XContentFactory.smileBuilder().startObject()
.field("field", "value")
.endObject().bytes(),
XContentType.JSON));
XContentType.SMILE));

assertThat(XContentFactory.xContentType(doc.source()), equalTo(XContentType.SMILE));
}
Expand Down

0 comments on commit 11d71d1

Please sign in to comment.