Fix copy_to when the target is a dynamic object field.

Fixes #11237
This commit is contained in:
Jim Ferenczi 2015-12-03 15:31:52 +01:00
parent 059a675aa5
commit 8558a40894
3 changed files with 215 additions and 37 deletions

View File

@ -28,8 +28,6 @@ import org.elasticsearch.common.Strings;
import org.elasticsearch.common.joda.FormatDateTimeFormatter; import org.elasticsearch.common.joda.FormatDateTimeFormatter;
import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.ReleasableLock; import org.elasticsearch.common.util.concurrent.ReleasableLock;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.mapper.core.DateFieldMapper.DateFieldType; import org.elasticsearch.index.mapper.core.DateFieldMapper.DateFieldType;
@ -47,7 +45,6 @@ import java.io.IOException;
import java.util.Collections; import java.util.Collections;
import java.util.HashSet; import java.util.HashSet;
import java.util.List; import java.util.List;
import java.util.Map;
import java.util.Set; import java.util.Set;
/** A parser for documents, given mappings from a DocumentMapper */ /** A parser for documents, given mappings from a DocumentMapper */
@ -712,37 +709,64 @@ class DocumentParser implements Closeable {
// The path of the dest field might be completely different from the current one so we need to reset it // The path of the dest field might be completely different from the current one so we need to reset it
context = context.overridePath(new ContentPath(0)); context = context.overridePath(new ContentPath(0));
String[] paths = Strings.splitStringToArray(field, '.');
String fieldName = paths[paths.length-1];
ObjectMapper mapper = context.root(); ObjectMapper mapper = context.root();
String objectPath = ""; ObjectMapper[] mappers = new ObjectMapper[paths.length-1];
String fieldPath = field; if (paths.length > 1) {
int posDot = field.lastIndexOf('.'); ObjectMapper parent = context.root();
if (posDot > 0) { for (int i = 0; i < paths.length-1; i++) {
objectPath = field.substring(0, posDot); mapper = context.docMapper().objectMappers().get(context.path().fullPathAsText(paths[i]));
context.path().add(objectPath);
mapper = context.docMapper().objectMappers().get(objectPath);
fieldPath = field.substring(posDot + 1);
}
if (mapper == null) { if (mapper == null) {
//TODO: Create an object dynamically? // One mapping is missing, check if we are allowed to create a dynamic one.
throw new MapperParsingException("attempt to copy value to non-existing object [" + field + "]"); ObjectMapper.Dynamic dynamic = parent.dynamic();
if (dynamic == null) {
dynamic = dynamicOrDefault(context.root().dynamic());
} }
ObjectMapper update = parseDynamicValue(context, mapper, fieldPath, context.parser().currentToken());
switch (dynamic) {
case STRICT:
throw new StrictDynamicMappingException(parent.fullPath(), paths[i]);
case TRUE:
Mapper.Builder builder = context.root().findTemplateBuilder(context, paths[i], "object");
if (builder == null) {
// if this is a non root object, then explicitly set the dynamic behavior if set
if (!(parent instanceof RootObjectMapper) && parent.dynamic() != ObjectMapper.Defaults.DYNAMIC) {
((ObjectMapper.Builder) builder).dynamic(parent.dynamic());
}
builder = MapperBuilders.object(paths[i]).enabled(true).pathType(parent.pathType());
}
Mapper.BuilderContext builderContext = new Mapper.BuilderContext(context.indexSettings(), context.path());
mapper = (ObjectMapper) builder.build(builderContext);
if (mapper.nested() != ObjectMapper.Nested.NO) {
throw new MapperParsingException("It is forbidden to create dynamic nested objects ([" + context.path().fullPathAsText(paths[i]) + "]) through `copy_to`");
}
break;
case FALSE:
// Maybe we should log something to tell the user that the copy_to is ignored in this case.
break;
default:
throw new AssertionError("Unexpected dynamic type " + dynamic);
}
}
context.path().add(paths[i]);
mappers[i] = mapper;
parent = mapper;
}
}
ObjectMapper update = parseDynamicValue(context, mapper, fieldName, context.parser().currentToken());
assert update != null; // we are parsing a dynamic value so we necessarily created a new mapping assert update != null; // we are parsing a dynamic value so we necessarily created a new mapping
// propagate the update to the root if (paths.length > 1) {
while (objectPath.length() > 0) { for (int i = paths.length - 2; i >= 0; i--) {
String parentPath = "";
ObjectMapper parent = context.root(); ObjectMapper parent = context.root();
posDot = objectPath.lastIndexOf('.'); if (i > 0) {
if (posDot > 0) { parent = mappers[i-1];
parentPath = objectPath.substring(0, posDot);
parent = context.docMapper().objectMappers().get(parentPath);
}
if (parent == null) {
throw new IllegalStateException("[" + objectPath + "] has no parent for path [" + parentPath + "]");
} }
assert parent != null;
update = parent.mappingUpdate(update); update = parent.mappingUpdate(update);
objectPath = parentPath; }
} }
context.addDynamicMappingsUpdate(update); context.addDynamicMappingsUpdate(update);
} }

View File

@ -30,6 +30,7 @@ import org.elasticsearch.test.ESIntegTestCase;
import java.io.IOException; import java.io.IOException;
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.equalTo;
@ -68,6 +69,25 @@ public class CopyToMapperIntegrationIT extends ESIntegTestCase {
} }
public void testDynamicObjectCopyTo() throws Exception {
String mapping = jsonBuilder().startObject().startObject("doc").startObject("properties")
.startObject("foo")
.field("type", "string")
.field("copy_to", "root.top.child")
.endObject()
.endObject().endObject().endObject().string();
assertAcked(
client().admin().indices().prepareCreate("test-idx")
.addMapping("doc", mapping)
);
client().prepareIndex("test-idx", "doc", "1")
.setSource("foo", "bar")
.get();
client().admin().indices().prepareRefresh("test-idx").execute().actionGet();
SearchResponse response = client().prepareSearch("test-idx")
.setQuery(QueryBuilders.termQuery("root.top.child", "bar")).get();
assertThat(response.getHits().totalHits(), equalTo(1L));
}
private XContentBuilder createDynamicTemplateMapping() throws IOException { private XContentBuilder createDynamicTemplateMapping() throws IOException {
return XContentFactory.jsonBuilder().startObject().startObject("doc") return XContentFactory.jsonBuilder().startObject().startObject("doc")

View File

@ -167,15 +167,80 @@ public class CopyToMapperTests extends ESSingleNodeTestCase {
} }
public void testCopyToFieldsNonExistingInnerObjectParsing() throws Exception { public void testCopyToDynamicInnerObjectParsing() throws Exception {
String mapping = jsonBuilder().startObject().startObject("type1").startObject("properties") String mapping = jsonBuilder().startObject().startObject("type1")
.startObject("properties")
.startObject("copy_test") .startObject("copy_test")
.field("type", "string") .field("type", "string")
.field("copy_to", "very.inner.field") .field("copy_to", "very.inner.field")
.endObject() .endObject()
.endObject()
.endObject().endObject().string();
.endObject().endObject().endObject().string(); DocumentMapper docMapper = createIndex("test").mapperService().documentMapperParser().parse(mapping);
BytesReference json = jsonBuilder().startObject()
.field("copy_test", "foo")
.field("new_field", "bar")
.endObject().bytes();
ParseContext.Document doc = docMapper.parse("test", "type1", "1", json).rootDoc();
assertThat(doc.getFields("copy_test").length, equalTo(1));
assertThat(doc.getFields("copy_test")[0].stringValue(), equalTo("foo"));
assertThat(doc.getFields("very.inner.field").length, equalTo(1));
assertThat(doc.getFields("very.inner.field")[0].stringValue(), equalTo("foo"));
assertThat(doc.getFields("new_field").length, equalTo(1));
assertThat(doc.getFields("new_field")[0].stringValue(), equalTo("bar"));
}
public void testCopyToDynamicInnerInnerObjectParsing() throws Exception {
String mapping = jsonBuilder().startObject().startObject("type1")
.startObject("properties")
.startObject("copy_test")
.field("type", "string")
.field("copy_to", "very.far.inner.field")
.endObject()
.startObject("very")
.field("type", "object")
.startObject("properties")
.startObject("far")
.field("type", "object")
.endObject()
.endObject()
.endObject()
.endObject()
.endObject().endObject().string();
DocumentMapper docMapper = createIndex("test").mapperService().documentMapperParser().parse(mapping);
BytesReference json = jsonBuilder().startObject()
.field("copy_test", "foo")
.field("new_field", "bar")
.endObject().bytes();
ParseContext.Document doc = docMapper.parse("test", "type1", "1", json).rootDoc();
assertThat(doc.getFields("copy_test").length, equalTo(1));
assertThat(doc.getFields("copy_test")[0].stringValue(), equalTo("foo"));
assertThat(doc.getFields("very.far.inner.field").length, equalTo(1));
assertThat(doc.getFields("very.far.inner.field")[0].stringValue(), equalTo("foo"));
assertThat(doc.getFields("new_field").length, equalTo(1));
assertThat(doc.getFields("new_field")[0].stringValue(), equalTo("bar"));
}
public void testCopyToStrictDynamicInnerObjectParsing() throws Exception {
String mapping = jsonBuilder().startObject().startObject("type1")
.field("dynamic", "strict")
.startObject("properties")
.startObject("copy_test")
.field("type", "string")
.field("copy_to", "very.inner.field")
.endObject()
.endObject()
.endObject().endObject().string();
DocumentMapper docMapper = createIndex("test").mapperService().documentMapperParser().parse(mapping); DocumentMapper docMapper = createIndex("test").mapperService().documentMapperParser().parse(mapping);
@ -187,7 +252,41 @@ public class CopyToMapperTests extends ESSingleNodeTestCase {
docMapper.parse("test", "type1", "1", json).rootDoc(); docMapper.parse("test", "type1", "1", json).rootDoc();
fail(); fail();
} catch (MapperParsingException ex) { } catch (MapperParsingException ex) {
assertThat(ex.getMessage(), startsWith("attempt to copy value to non-existing object")); assertThat(ex.getMessage(), startsWith("mapping set to strict, dynamic introduction of [very] within [type1] is not allowed"));
}
}
public void testCopyToInnerStrictDynamicInnerObjectParsing() throws Exception {
String mapping = jsonBuilder().startObject().startObject("type1")
.startObject("properties")
.startObject("copy_test")
.field("type", "string")
.field("copy_to", "very.far.field")
.endObject()
.startObject("very")
.field("type", "object")
.startObject("properties")
.startObject("far")
.field("type", "object")
.field("dynamic", "strict")
.endObject()
.endObject()
.endObject()
.endObject()
.endObject().endObject().string();
DocumentMapper docMapper = createIndex("test").mapperService().documentMapperParser().parse(mapping);
BytesReference json = jsonBuilder().startObject()
.field("copy_test", "foo")
.endObject().bytes();
try {
docMapper.parse("test", "type1", "1", json).rootDoc();
fail();
} catch (MapperParsingException ex) {
assertThat(ex.getMessage(), startsWith("mapping set to strict, dynamic introduction of [field] within [very.far] is not allowed"));
} }
} }
@ -337,6 +436,41 @@ public class CopyToMapperTests extends ESSingleNodeTestCase {
} }
} }
public void testCopyToDynamicNestedObjectParsing() throws Exception {
String mapping = jsonBuilder().startObject().startObject("type1")
.startArray("dynamic_templates")
.startObject()
.startObject("objects")
.field("match_mapping_type", "object")
.startObject("mapping")
.field("type", "nested")
.endObject()
.endObject()
.endObject()
.endArray()
.startObject("properties")
.startObject("copy_test")
.field("type", "string")
.field("copy_to", "very.inner.field")
.endObject()
.endObject()
.endObject().endObject().string();
DocumentMapper docMapper = createIndex("test").mapperService().documentMapperParser().parse(mapping);
BytesReference json = jsonBuilder().startObject()
.field("copy_test", "foo")
.field("new_field", "bar")
.endObject().bytes();
try {
docMapper.parse("test", "type1", "1", json).rootDoc();
fail();
} catch (MapperParsingException ex) {
assertThat(ex.getMessage(), startsWith("It is forbidden to create dynamic nested objects ([very]) through `copy_to`"));
}
}
private void assertFieldValue(Document doc, String field, Number... expected) { private void assertFieldValue(Document doc, String field, Number... expected) {
IndexableField[] values = doc.getFields(field); IndexableField[] values = doc.getFields(field);
if (values == null) { if (values == null) {