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

Better validation of `copy_to`. (#25983) 

We are currently quite lenient about the targets of `copy_to`. However in a
number of cases we can detect illegal use of `copy_to` at mapping update time.
For instance, it does not make sense to use object fields as targets of
`copy_to`, or fields that would end up in a different nested document.
This commit is contained in:
Adrien Grand 2017-08-01 16:23:28 +02:00 committed by GitHub
parent 53c829b6bc
commit e9669b3762
2 changed files with 160 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

54
core/src/main/java/org/elasticsearch/index/mapper/MapperService.java
View File

@ -423,6 +423,10 @@ public class MapperService extends AbstractIndexComponent implements Closeable {
}
}
if (indexSettings.getIndexVersionCreated().onOrAfter(Version.V_6_0_0_beta1)) {
validateCopyTo(fieldMappers, fullPathObjectMappers);
}
if (reason == MergeReason.MAPPING_UPDATE) {
// this check will only be performed on the master node when there is
// a call to the update mapping API. For all other cases like
@ -648,12 +652,60 @@ public class MapperService extends AbstractIndexComponent implements Closeable {
}
}
private void checkIndexSortCompatibility(IndexSortConfig sortConfig, boolean hasNested) {
private static void checkIndexSortCompatibility(IndexSortConfig sortConfig, boolean hasNested) {
if (sortConfig.hasIndexSort() && hasNested) {
throw new IllegalArgumentException("cannot have nested fields when index sort is activated");
}
}
private static void validateCopyTo(List<FieldMapper> fieldMappers, Map<String, ObjectMapper> fullPathObjectMappers) {
for (FieldMapper mapper : fieldMappers) {
if (mapper.copyTo() != null && mapper.copyTo().copyToFields().isEmpty() == false) {
final String sourceScope = getNestedScope(mapper.name(), fullPathObjectMappers);
for (String copyTo : mapper.copyTo().copyToFields()) {
if (fullPathObjectMappers.containsKey(copyTo)) {
throw new IllegalArgumentException("Cannot copy to field [" + copyTo + "] since it is mapped as an object");
}
final String targetScope = getNestedScope(copyTo, fullPathObjectMappers);
checkNestedScopeCompatibility(sourceScope, targetScope);
}
}
}
}
private static String getNestedScope(String path, Map<String, ObjectMapper> fullPathObjectMappers) {
for (String parentPath = parentObject(path); parentPath != null; parentPath = parentObject(parentPath)) {
ObjectMapper objectMapper = fullPathObjectMappers.get(parentPath);
if (objectMapper != null && objectMapper.nested().isNested()) {
return parentPath;
}
}
return null;
}
private static void checkNestedScopeCompatibility(String source, String target) {
boolean targetIsParentOfSource;
if (source == null || target == null) {
targetIsParentOfSource = target == null;
} else {
targetIsParentOfSource = source.startsWith(target + ".");
}
if (targetIsParentOfSource == false) {
throw new IllegalArgumentException(
"Illegal combination of [copy_to] and [nested] mappings: [copy_to] may only copy data to the current nested " +
"document or any of its parents, however one [copy_to] directive is trying to copy data from nested object [" +
source + "] to [" + target + "]");
}
}
private static String parentObject(String field) {
int lastDot = field.lastIndexOf('.');
if (lastDot == -1) {
return null;
}
return field.substring(0, lastDot);
}
public DocumentMapper parse(String mappingType, CompressedXContent mappingSource, boolean applyDefault) throws MapperParsingException {
return documentParser.parse(mappingType, mappingSource, applyDefault ? defaultMappingSource : null);
}

107
core/src/test/java/org/elasticsearch/index/mapper/CopyToMapperTests.java
View File

@ -29,8 +29,10 @@ import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.common.xcontent.json.JsonXContent;
import org.elasticsearch.index.IndexService;
import org.elasticsearch.index.mapper.MapperService.MergeReason;
import org.elasticsearch.index.mapper.ParseContext.Document;
import org.elasticsearch.test.ESSingleNodeTestCase;
import org.hamcrest.Matchers;
import java.util.Arrays;
import java.util.List;
@ -414,6 +416,111 @@ public class CopyToMapperTests extends ESSingleNodeTestCase {
assertFieldValue(root, "n1.n2.target");
}
public void testCopyToChildNested() throws Exception {
IndexService indexService = createIndex("test");
XContentBuilder rootToNestedMapping = jsonBuilder().startObject()
.startObject("doc")
.startObject("properties")
.startObject("source")
.field("type", "long")
.field("copy_to", "n1.target")
.endObject()
.startObject("n1")
.field("type", "nested")
.startObject("properties")
.startObject("target")
.field("type", "long")
.endObject()
.endObject()
.endObject()
.endObject()
.endObject()
.endObject();
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
() -> indexService.mapperService().merge("doc", new CompressedXContent(rootToNestedMapping.bytes()),
MergeReason.MAPPING_UPDATE, false));
assertThat(e.getMessage(), Matchers.startsWith("Illegal combination of [copy_to] and [nested] mappings"));
XContentBuilder nestedToNestedMapping = jsonBuilder().startObject()
.startObject("doc")
.startObject("properties")
.startObject("n1")
.field("type", "nested")
.startObject("properties")
.startObject("source")
.field("type", "long")
.field("copy_to", "n1.n2.target")
.endObject()
.startObject("n2")
.field("type", "nested")
.startObject("properties")
.startObject("target")
.field("type", "long")
.endObject()
.endObject()
.endObject()
.endObject()
.endObject()
.endObject()
.endObject()
.endObject();
e = expectThrows(IllegalArgumentException.class,
() -> indexService.mapperService().merge("doc", new CompressedXContent(nestedToNestedMapping.bytes()),
MergeReason.MAPPING_UPDATE, false));
}
public void testCopyToSiblingNested() throws Exception {
IndexService indexService = createIndex("test");
XContentBuilder rootToNestedMapping = jsonBuilder().startObject()
.startObject("doc")
.startObject("properties")
.startObject("n1")
.field("type", "nested")
.startObject("properties")
.startObject("source")
.field("type", "long")
.field("copy_to", "n2.target")
.endObject()
.endObject()
.endObject()
.startObject("n2")
.field("type", "nested")
.startObject("properties")
.startObject("target")
.field("type", "long")
.endObject()
.endObject()
.endObject()
.endObject()
.endObject()
.endObject();
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
() -> indexService.mapperService().merge("doc", new CompressedXContent(rootToNestedMapping.bytes()),
MergeReason.MAPPING_UPDATE, false));
assertThat(e.getMessage(), Matchers.startsWith("Illegal combination of [copy_to] and [nested] mappings"));
}
public void testCopyToObject() throws Exception {
IndexService indexService = createIndex("test");
XContentBuilder rootToNestedMapping = jsonBuilder().startObject()
.startObject("doc")
.startObject("properties")
.startObject("source")
.field("type", "long")
.field("copy_to", "target")
.endObject()
.startObject("target")
.field("type", "object")
.endObject()
.endObject()
.endObject()
.endObject();
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
() -> indexService.mapperService().merge("doc", new CompressedXContent(rootToNestedMapping.bytes()),
MergeReason.MAPPING_UPDATE, false));
assertThat(e.getMessage(), Matchers.startsWith("Cannot copy to field [target] since it is mapped as an object"));
}
public void testCopyToDynamicNestedObjectParsing() throws Exception {
String mapping = jsonBuilder().startObject().startObject("type1")
.startArray("dynamic_templates")