Skip to content

Fix copy_to when the target is a dynamic object field. #15385

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 11, 2015
Merged

Fix copy_to when the target is a dynamic object field. #15385

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 54 additions & 27 deletions core/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -717,37 +717,64 @@ private static void parseCopy(String field, ParseContext context) throws IOExcep
// 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));

String[] paths = Strings.splitStringToArray(field, '.');
String fieldName = paths[paths.length-1];
ObjectMapper mapper = context.root();
String objectPath = "";
String fieldPath = field;
int posDot = field.lastIndexOf('.');
if (posDot > 0) {
objectPath = field.substring(0, posDot);
context.path().add(objectPath);
mapper = context.docMapper().objectMappers().get(objectPath);
fieldPath = field.substring(posDot + 1);
}
if (mapper == null) {
//TODO: Create an object dynamically?
throw new MapperParsingException("attempt to copy value to non-existing object [" + field + "]");
}
ObjectMapper update = parseDynamicValue(context, mapper, fieldPath, context.parser().currentToken());
assert update != null; // we are parsing a dynamic value so we necessarily created a new mapping

// propagate the update to the root
while (objectPath.length() > 0) {
String parentPath = "";
ObjectMapper[] mappers = new ObjectMapper[paths.length-1];
if (paths.length > 1) {
ObjectMapper parent = context.root();
posDot = objectPath.lastIndexOf('.');
if (posDot > 0) {
parentPath = objectPath.substring(0, posDot);
parent = context.docMapper().objectMappers().get(parentPath);
for (int i = 0; i < paths.length-1; i++) {
mapper = context.docMapper().objectMappers().get(context.path().fullPathAsText(paths[i]));
if (mapper == null) {
// One mapping is missing, check if we are allowed to create a dynamic one.
ObjectMapper.Dynamic dynamic = parent.dynamic();
if (dynamic == null) {
dynamic = dynamicOrDefault(context.root().dynamic());
}

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;
}
if (parent == null) {
throw new IllegalStateException("[" + objectPath + "] has no parent for path [" + parentPath + "]");
}
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

if (paths.length > 1) {
for (int i = paths.length - 2; i >= 0; i--) {
ObjectMapper parent = context.root();
if (i > 0) {
parent = mappers[i-1];
}
assert parent != null;
update = parent.mappingUpdate(update);
}
update = parent.mappingUpdate(update);
objectPath = parentPath;
}
context.addDynamicMappingsUpdate(update);
}
Expand Down
20 changes: 20 additions & 0 deletions core/src/test/java/org/elasticsearch/index/mapper/copyto/CopyToMapperIntegrationIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@

import java.io.IOException;

import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.hamcrest.Matchers.equalTo;

Expand Down Expand Up @@ -72,6 +73,25 @@ public void testDynamicTemplateCopyTo() throws Exception {

}

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 {
return XContentFactory.jsonBuilder().startObject().startObject("doc")
Expand Down
150 changes: 142 additions & 8 deletions core/src/test/java/org/elasticsearch/index/mapper/copyto/CopyToMapperTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,27 +174,126 @@ public void testCopyToFieldsInnerObjectParsing() throws Exception {

@SuppressWarnings("unchecked")
@Test
public void testCopyToFieldsNonExistingInnerObjectParsing() throws Exception {
String mapping = jsonBuilder().startObject().startObject("type1").startObject("properties")

public void testCopyToDynamicInnerObjectParsing() throws Exception {
String mapping = jsonBuilder().startObject().startObject("type1")
.startObject("properties")
.startObject("copy_test")
.field("type", "string")
.field("copy_to", "very.inner.field")
.field("type", "string")
.field("copy_to", "very.inner.field")
.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);

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 [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("attempt to copy value to non-existing object"));
assertThat(ex.getMessage(), startsWith("mapping set to strict, dynamic introduction of [field] within [very.far] is not allowed"));
}
}

Expand Down Expand Up @@ -346,6 +445,41 @@ public void testCopyToNestedField() throws Exception {
}
}

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) {
IndexableField[] values = doc.getFields(field);
if (values == null) {
Expand Down