diff --git a/src/main/java/org/elasticsearch/index/mapper/core/AbstractFieldMapper.java b/src/main/java/org/elasticsearch/index/mapper/core/AbstractFieldMapper.java index c18e7c06563..654e203d14d 100644 --- a/src/main/java/org/elasticsearch/index/mapper/core/AbstractFieldMapper.java +++ b/src/main/java/org/elasticsearch/index/mapper/core/AbstractFieldMapper.java @@ -837,7 +837,7 @@ public abstract class AbstractFieldMapper implements FieldMapper { public static class MultiFields { public static MultiFields empty() { - return new MultiFields(Defaults.PATH_TYPE, ImmutableOpenMap.of()); + return new MultiFields(Defaults.PATH_TYPE, ImmutableOpenMap.of()); } public static class Builder { @@ -860,7 +860,7 @@ public abstract class AbstractFieldMapper implements FieldMapper { if (pathType == Defaults.PATH_TYPE && mapperBuilders.isEmpty()) { return empty(); } else if (mapperBuilders.isEmpty()) { - return new MultiFields(pathType, ImmutableOpenMap.of()); + return new MultiFields(pathType, ImmutableOpenMap.of()); } else { ContentPath.Type origPathType = context.path().pathType(); context.path().pathType(pathType); @@ -869,26 +869,27 @@ public abstract class AbstractFieldMapper implements FieldMapper { for (ObjectObjectCursor cursor : this.mapperBuilders) { String key = cursor.key; Mapper.Builder value = cursor.value; - mapperBuilders.put(key, value.build(context)); + Mapper mapper = value.build(context); + assert mapper instanceof FieldMapper; + mapperBuilders.put(key, mapper); } context.path().remove(); context.path().pathType(origPathType); - ImmutableOpenMap.Builder mappers = mapperBuilders.cast(); + ImmutableOpenMap.Builder mappers = mapperBuilders.cast(); return new MultiFields(pathType, mappers.build()); } } - } private final ContentPath.Type pathType; - private volatile ImmutableOpenMap mappers; + private volatile ImmutableOpenMap mappers; - public MultiFields(ContentPath.Type pathType, ImmutableOpenMap mappers) { + public MultiFields(ContentPath.Type pathType, ImmutableOpenMap mappers) { this.pathType = pathType; this.mappers = mappers; // we disable the all in multi-field mappers - for (ObjectCursor cursor : mappers.values()) { - Mapper mapper = cursor.value; + for (ObjectCursor cursor : mappers.values()) { + FieldMapper mapper = cursor.value; if (mapper instanceof AllFieldMapper.IncludeInAll) { ((AllFieldMapper.IncludeInAll) mapper).unsetIncludeInAll(); } @@ -906,7 +907,7 @@ public abstract class AbstractFieldMapper implements FieldMapper { context.path().pathType(pathType); context.path().add(mainField.name()); - for (ObjectCursor cursor : mappers.values()) { + for (ObjectCursor cursor : mappers.values()) { cursor.value.parse(context); } context.path().remove(); @@ -918,10 +919,10 @@ public abstract class AbstractFieldMapper implements FieldMapper { AbstractFieldMapper mergeWithMultiField = (AbstractFieldMapper) mergeWith; List> newFieldMappers = null; - ImmutableOpenMap.Builder newMappersBuilder = null; + ImmutableOpenMap.Builder newMappersBuilder = null; - for (ObjectCursor cursor : mergeWithMultiField.multiFields.mappers.values()) { - Mapper mergeWithMapper = cursor.value; + for (ObjectCursor cursor : mergeWithMultiField.multiFields.mappers.values()) { + FieldMapper mergeWithMapper = cursor.value; Mapper mergeIntoMapper = mappers.get(mergeWithMapper.name()); if (mergeIntoMapper == null) { // no mapping, simply add it if not simulating @@ -938,7 +939,7 @@ public abstract class AbstractFieldMapper implements FieldMapper { if (newFieldMappers == null) { newFieldMappers = new ArrayList<>(2); } - newFieldMappers.add((FieldMapper) mergeWithMapper); + newFieldMappers.add(mergeWithMapper); } } } else { @@ -957,13 +958,13 @@ public abstract class AbstractFieldMapper implements FieldMapper { } public void traverse(FieldMapperListener fieldMapperListener) { - for (ObjectCursor cursor : mappers.values()) { + for (ObjectCursor cursor : mappers.values()) { cursor.value.traverse(fieldMapperListener); } } public void close() { - for (ObjectCursor cursor : mappers.values()) { + for (ObjectCursor cursor : mappers.values()) { cursor.value.close(); } } diff --git a/src/main/java/org/elasticsearch/index/mapper/core/TypeParsers.java b/src/main/java/org/elasticsearch/index/mapper/core/TypeParsers.java index cfed59be20f..20f9eda9b26 100644 --- a/src/main/java/org/elasticsearch/index/mapper/core/TypeParsers.java +++ b/src/main/java/org/elasticsearch/index/mapper/core/TypeParsers.java @@ -34,6 +34,7 @@ import org.elasticsearch.index.mapper.DocumentMapperParser; import org.elasticsearch.index.mapper.FieldMapper.Loading; import org.elasticsearch.index.mapper.Mapper; import org.elasticsearch.index.mapper.MapperParsingException; +import org.elasticsearch.index.mapper.object.ObjectMapper; import java.util.ArrayList; import java.util.Collections; @@ -332,6 +333,9 @@ public class TypeParsers { } else { throw new MapperParsingException("No type specified for property [" + multiFieldName + "]"); } + if (type.equals(ObjectMapper.CONTENT_TYPE) || type.equals(ObjectMapper.NESTED_CONTENT_TYPE)) { + throw new MapperParsingException("Type [" + type + "] cannot be used in multi field"); + } Mapper.TypeParser typeParser = parserContext.typeParser(type); if (typeParser == null) { diff --git a/src/test/java/org/elasticsearch/index/mapper/multifield/MultiFieldTests.java b/src/test/java/org/elasticsearch/index/mapper/multifield/MultiFieldTests.java index 8a54985f0ce..75c58a7bf8f 100644 --- a/src/test/java/org/elasticsearch/index/mapper/multifield/MultiFieldTests.java +++ b/src/test/java/org/elasticsearch/index/mapper/multifield/MultiFieldTests.java @@ -30,6 +30,7 @@ import org.elasticsearch.common.xcontent.support.XContentMapValues; import org.elasticsearch.index.mapper.DocumentMapper; import org.elasticsearch.index.mapper.DocumentMapperParser; import org.elasticsearch.index.mapper.FieldMapper; +import org.elasticsearch.index.mapper.MapperParsingException; import org.elasticsearch.index.mapper.ParseContext.Document; import org.elasticsearch.index.mapper.core.*; import org.elasticsearch.index.mapper.geo.GeoPointFieldMapper; @@ -481,4 +482,30 @@ public class MultiFieldTests extends ElasticsearchSingleNodeTest { DocumentMapper docMapper2 = parser.parse(docMapper.mappingSource().string()); assertThat(docMapper.mappingSource(), equalTo(docMapper2.mappingSource())); } + + public void testObjectFieldNotAllowed() throws Exception { + String mapping = jsonBuilder().startObject().startObject("type").startObject("properties").startObject("my_field") + .field("type", "string").startObject("fields").startObject("multi").field("type", "object").endObject().endObject() + .endObject().endObject().endObject().endObject().string(); + final DocumentMapperParser parser = createIndex("test").mapperService().documentMapperParser(); + try { + parser.parse(mapping); + fail("expected mapping parse failure"); + } catch (MapperParsingException e) { + assertTrue(e.getMessage().contains("cannot be used in multi field")); + } + } + + public void testNestedFieldNotAllowed() throws Exception { + String mapping = jsonBuilder().startObject().startObject("type").startObject("properties").startObject("my_field") + .field("type", "string").startObject("fields").startObject("multi").field("type", "nested").endObject().endObject() + .endObject().endObject().endObject().endObject().string(); + final DocumentMapperParser parser = createIndex("test").mapperService().documentMapperParser(); + try { + parser.parse(mapping); + fail("expected mapping parse failure"); + } catch (MapperParsingException e) { + assertTrue(e.getMessage().contains("cannot be used in multi field")); + } + } }