From 46316150ff711898708f5888ca66fdc151046775 Mon Sep 17 00:00:00 2001 From: Colin Goodheart-Smithe Date: Wed, 6 Aug 2014 13:20:51 +0100 Subject: [PATCH] Mapping: fixes dynamic mapping of geo_point fields If a dynamic mapping for a geo_point field is defined and the first document specifies the value of the field as a geo_point array, the dynamic mapping throws an error as the array is broken into individual number before consulting the dynamic mapping configuration. This change adds a check of the dynamic mapping before the array is split into individual numbers. Closes #6939 --- .../index/mapper/object/ObjectMapper.java | 132 ++++++++++++------ .../geo/LatLonMappingGeoPointTests.java | 21 +++ 2 files changed, 110 insertions(+), 43 deletions(-) diff --git a/src/main/java/org/elasticsearch/index/mapper/object/ObjectMapper.java b/src/main/java/org/elasticsearch/index/mapper/object/ObjectMapper.java index 44504a3593de1..a184ccc78a74b 100644 --- a/src/main/java/org/elasticsearch/index/mapper/object/ObjectMapper.java +++ b/src/main/java/org/elasticsearch/index/mapper/object/ObjectMapper.java @@ -575,34 +575,7 @@ private void serializeObject(final ParseContext context, String currentFieldName } BuilderContext builderContext = new BuilderContext(context.indexSettings(), context.path()); objectMapper = builder.build(builderContext); - // ...now re add it - context.path().add(currentFieldName); - context.setMappingsModified(); - - if (context.isWithinNewMapper()) { - // within a new mapper, no need to traverse, just parse - objectMapper.parse(context); - } else { - // create a context of new mapper, so we batch aggregate all the changes within - // this object mapper once, and traverse all of them to add them in a single go - context.setWithinNewMapper(); - try { - objectMapper.parse(context); - FieldMapperListener.Aggregator newFields = new FieldMapperListener.Aggregator(); - ObjectMapperListener.Aggregator newObjects = new ObjectMapperListener.Aggregator(); - objectMapper.traverse(newFields); - objectMapper.traverse(newObjects); - // callback on adding those fields! - context.docMapper().addFieldMappers(newFields.mappers); - context.docMapper().addObjectMappers(newObjects.mappers); - } finally { - context.clearWithinNewMapper(); - } - } - - // only put after we traversed and did the callbacks, so other parsing won't see it only after we - // properly traversed it and adding the mappers - putMapper(objectMapper); + putDynamicMapper(context, currentFieldName, objectMapper); } else { objectMapper.parse(context); } @@ -622,22 +595,95 @@ private void serializeArray(ParseContext context, String lastFieldName) throws I if (mapper != null && mapper instanceof ArrayValueMapperParser) { mapper.parse(context); } else { - XContentParser parser = context.parser(); - XContentParser.Token token; - while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) { - if (token == XContentParser.Token.START_OBJECT) { - serializeObject(context, lastFieldName); - } else if (token == XContentParser.Token.START_ARRAY) { - serializeArray(context, lastFieldName); - } else if (token == XContentParser.Token.FIELD_NAME) { - lastFieldName = parser.currentName(); - } else if (token == XContentParser.Token.VALUE_NULL) { - serializeNullValue(context, lastFieldName); - } else if (token == null) { - throw new MapperParsingException("object mapping for [" + name + "] with array for [" + arrayFieldName + "] tried to parse as array, but got EOF, is there a mismatch in types for the same field?"); - } else { - serializeValue(context, lastFieldName, token); + + Dynamic dynamic = this.dynamic; + if (dynamic == null) { + dynamic = context.root().dynamic(); + } + if (dynamic == Dynamic.STRICT) { + throw new StrictDynamicMappingException(fullPath, arrayFieldName); + } else if (dynamic == Dynamic.TRUE) { + // we sync here just so we won't add it twice. Its not the end of the world + // to sync here since next operations will get it before + synchronized (mutex) { + mapper = mappers.get(arrayFieldName); + if (mapper == null) { + Mapper.Builder builder = context.root().findTemplateBuilder(context, arrayFieldName, "object"); + if (builder == null) { + serializeNonDynamicArray(context, lastFieldName, arrayFieldName); + return; + } + BuilderContext builderContext = new BuilderContext(context.indexSettings(), context.path()); + mapper = builder.build(builderContext); + if (mapper != null && mapper instanceof ArrayValueMapperParser) { + putDynamicMapper(context, arrayFieldName, mapper); + } else { + serializeNonDynamicArray(context, lastFieldName, arrayFieldName); + } + } else { + + serializeNonDynamicArray(context, lastFieldName, arrayFieldName); + } } + } else { + + serializeNonDynamicArray(context, lastFieldName, arrayFieldName); + } + } + } + + private void putDynamicMapper(ParseContext context, String arrayFieldName, Mapper mapper) throws IOException { + // ...now re add it + context.path().add(arrayFieldName); + context.setMappingsModified(); + + if (context.isWithinNewMapper()) { + // within a new mapper, no need to traverse, + // just parse + mapper.parse(context); + } else { + // create a context of new mapper, so we batch + // aggregate all the changes within + // this object mapper once, and traverse all of + // them to add them in a single go + context.setWithinNewMapper(); + try { + mapper.parse(context); + FieldMapperListener.Aggregator newFields = new FieldMapperListener.Aggregator(); + ObjectMapperListener.Aggregator newObjects = new ObjectMapperListener.Aggregator(); + mapper.traverse(newFields); + mapper.traverse(newObjects); + // callback on adding those fields! + context.docMapper().addFieldMappers(newFields.mappers); + context.docMapper().addObjectMappers(newObjects.mappers); + } finally { + context.clearWithinNewMapper(); + } + } + + // only put after we traversed and did the + // callbacks, so other parsing won't see it only + // after we + // properly traversed it and adding the mappers + putMapper(mapper); + } + + private void serializeNonDynamicArray(ParseContext context, String lastFieldName, String arrayFieldName) throws IOException { + XContentParser parser = context.parser(); + XContentParser.Token token; + while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) { + if (token == XContentParser.Token.START_OBJECT) { + serializeObject(context, lastFieldName); + } else if (token == XContentParser.Token.START_ARRAY) { + serializeArray(context, lastFieldName); + } else if (token == XContentParser.Token.FIELD_NAME) { + lastFieldName = parser.currentName(); + } else if (token == XContentParser.Token.VALUE_NULL) { + serializeNullValue(context, lastFieldName); + } else if (token == null) { + throw new MapperParsingException("object mapping for [" + name + "] with array for [" + arrayFieldName + "] tried to parse as array, but got EOF, is there a mismatch in types for the same field?"); + } else { + serializeValue(context, lastFieldName, token); } } } diff --git a/src/test/java/org/elasticsearch/index/mapper/geo/LatLonMappingGeoPointTests.java b/src/test/java/org/elasticsearch/index/mapper/geo/LatLonMappingGeoPointTests.java index 92054575011b2..a8026702f6dfb 100644 --- a/src/test/java/org/elasticsearch/index/mapper/geo/LatLonMappingGeoPointTests.java +++ b/src/test/java/org/elasticsearch/index/mapper/geo/LatLonMappingGeoPointTests.java @@ -345,6 +345,27 @@ public void testLonLatArray() throws Exception { assertThat(doc.rootDoc().get("point"), equalTo("1.2,1.3")); } + @Test + public void testLonLatArrayDynamic() throws Exception { + String mapping = XContentFactory.jsonBuilder().startObject().startObject("type") + .startArray("dynamic_templates").startObject() + .startObject("point").field("match", "point*").startObject("mapping").field("type", "geo_point").field("lat_lon", true).endObject().endObject() + .endObject().endArray() + .endObject().endObject().string(); + + DocumentMapper defaultMapper = createIndex("test").mapperService().documentMapperParser().parse(mapping); + + ParsedDocument doc = defaultMapper.parse("type", "1", XContentFactory.jsonBuilder() + .startObject() + .startArray("point").value(1.3).value(1.2).endArray() + .endObject() + .bytes()); + + assertThat(doc.rootDoc().getField("point.lat"), notNullValue()); + assertThat(doc.rootDoc().getField("point.lon"), notNullValue()); + assertThat(doc.rootDoc().get("point"), equalTo("1.2,1.3")); + } + @Test public void testLonLatArrayStored() throws Exception { String mapping = XContentFactory.jsonBuilder().startObject().startObject("type")