honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Mappings: Explicitly disallow multi fields from using object or nested fields 

Multi fields currently parse any field type passed in.  However, they
were only intended to support copying simple values from the outter
field. This change adds validation to ensure object and nested
fields are not used within multi fields.

closes #10745
This commit is contained in:
Ryan Ernst 2015-04-22 16:38:07 -07:00
parent 3daf460acb
commit f6d8b12796
3 changed files with 48 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
src/main/java/org/elasticsearch/index/mapper/core/AbstractFieldMapper.java
View File

@ -837,7 +837,7 @@ public abstract class AbstractFieldMapper<T> implements FieldMapper<T> {
public static class MultiFields {
public static MultiFields empty() {
return new MultiFields(Defaults.PATH_TYPE, ImmutableOpenMap.<String, Mapper>of());
return new MultiFields(Defaults.PATH_TYPE, ImmutableOpenMap.<String, FieldMapper>of());
}
public static class Builder {
@ -860,7 +860,7 @@ public abstract class AbstractFieldMapper<T> implements FieldMapper<T> {
if (pathType == Defaults.PATH_TYPE && mapperBuilders.isEmpty()) {
return empty();
} else if (mapperBuilders.isEmpty()) {
return new MultiFields(pathType, ImmutableOpenMap.<String, Mapper>of());
return new MultiFields(pathType, ImmutableOpenMap.<String, FieldMapper>of());
} else {
ContentPath.Type origPathType = context.path().pathType();
context.path().pathType(pathType);
@ -869,26 +869,27 @@ public abstract class AbstractFieldMapper<T> implements FieldMapper<T> {
for (ObjectObjectCursor<String, Mapper.Builder> 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<String, Mapper> mappers = mapperBuilders.cast();
ImmutableOpenMap.Builder<String, FieldMapper> mappers = mapperBuilders.cast();
return new MultiFields(pathType, mappers.build());
}
}
}
private final ContentPath.Type pathType;
private volatile ImmutableOpenMap<String, Mapper> mappers;
private volatile ImmutableOpenMap<String, FieldMapper> mappers;
public MultiFields(ContentPath.Type pathType, ImmutableOpenMap<String, Mapper> mappers) {
public MultiFields(ContentPath.Type pathType, ImmutableOpenMap<String, FieldMapper> mappers) {
this.pathType = pathType;
this.mappers = mappers;
// we disable the all in multi-field mappers
for (ObjectCursor<Mapper> cursor : mappers.values()) {
Mapper mapper = cursor.value;
for (ObjectCursor<FieldMapper> cursor : mappers.values()) {
FieldMapper mapper = cursor.value;
if (mapper instanceof AllFieldMapper.IncludeInAll) {
((AllFieldMapper.IncludeInAll) mapper).unsetIncludeInAll();
}
@ -906,7 +907,7 @@ public abstract class AbstractFieldMapper<T> implements FieldMapper<T> {
context.path().pathType(pathType);
context.path().add(mainField.name());
for (ObjectCursor<Mapper> cursor : mappers.values()) {
for (ObjectCursor<FieldMapper> cursor : mappers.values()) {
cursor.value.parse(context);
}
context.path().remove();
@ -918,10 +919,10 @@ public abstract class AbstractFieldMapper<T> implements FieldMapper<T> {
AbstractFieldMapper mergeWithMultiField = (AbstractFieldMapper) mergeWith;
List<FieldMapper<?>> newFieldMappers = null;
ImmutableOpenMap.Builder<String, Mapper> newMappersBuilder = null;
ImmutableOpenMap.Builder<String, FieldMapper> newMappersBuilder = null;
for (ObjectCursor<Mapper> cursor : mergeWithMultiField.multiFields.mappers.values()) {
Mapper mergeWithMapper = cursor.value;
for (ObjectCursor<FieldMapper> 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<T> implements FieldMapper<T> {
if (newFieldMappers == null) {
newFieldMappers = new ArrayList<>(2);
}
newFieldMappers.add((FieldMapper) mergeWithMapper);
newFieldMappers.add(mergeWithMapper);
}
}
} else {
@ -957,13 +958,13 @@ public abstract class AbstractFieldMapper<T> implements FieldMapper<T> {
}
public void traverse(FieldMapperListener fieldMapperListener) {
for (ObjectCursor<Mapper> cursor : mappers.values()) {
for (ObjectCursor<FieldMapper> cursor : mappers.values()) {
cursor.value.traverse(fieldMapperListener);
}
}
public void close() {
for (ObjectCursor<Mapper> cursor : mappers.values()) {
for (ObjectCursor<FieldMapper> cursor : mappers.values()) {
cursor.value.close();
}
}

4
src/main/java/org/elasticsearch/index/mapper/core/TypeParsers.java
View File

@ -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) {

27
src/test/java/org/elasticsearch/index/mapper/multifield/MultiFieldTests.java
View File

@ -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"));
}
}
}