Skip to content

Prevent duplicate fields when mixing parent and root nested includes #27072

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.

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 3 commits into from
Oct 31, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,38 @@ public Builder dynamicTemplates(Collection<DynamicTemplate> templates) {
return this;
}

@Override
public RootObjectMapper build(BuilderContext context) {
fixRedundantIncludes(this, true);
return super.build(context);
}

/**
* Removes redundant root includes in {@link ObjectMapper.Nested} trees to avoid duplicate
* fields on the root mapper when {@code isIncludeInRoot} is {@code true} for a node that is
* itself included into a parent node, for which either {@code isIncludeInRoot} is
* {@code true} or which is transitively included in root by a chain of nodes with
* {@code isIncludeInParent} returning {@code true}.
* @param omb Builder whose children to check.
* @param parentIncluded True iff node is a child of root or a node that is included in
* root
*/
private static void fixRedundantIncludes(ObjectMapper.Builder omb, boolean parentIncluded) {
for (Object mapper : omb.mappersBuilders) {
if (mapper instanceof ObjectMapper.Builder) {
ObjectMapper.Builder child = (ObjectMapper.Builder) mapper;
Nested nested = child.nested;
boolean isNested = nested.isNested();
boolean includeInRootViaParent = parentIncluded && isNested && nested.isIncludeInParent();
boolean includedInRoot = isNested && nested.isIncludeInRoot();
if (includeInRootViaParent && includedInRoot) {
child.nested = Nested.newNested(true, false);
}
fixRedundantIncludes(child, includeInRootViaParent || includedInRoot);
}
}
}

@Override
protected ObjectMapper createMapper(String name, String fullPath, boolean enabled, Nested nested, Dynamic dynamic,
Map<String, Mapper> mappers, @Nullable Settings settings) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@

package org.elasticsearch.index.mapper;

import java.util.HashMap;
import java.util.HashSet;
import org.apache.lucene.index.IndexableField;
import org.elasticsearch.Version;
import org.elasticsearch.common.compress.CompressedXContent;
import org.elasticsearch.common.settings.Settings;
Expand Down Expand Up @@ -333,6 +336,67 @@ public void testMultiRootAndNested1() throws Exception {
assertThat(doc.docs().get(6).getFields("nested1.nested2.field2").length, equalTo(4));
}

/**
* Checks that multiple levels of nested includes where a node is both directly and transitively
* included in root by {@code include_in_root} and a chain of {@code include_in_parent} does not
* lead to duplicate fields on the root document.
*/
public void testMultipleLevelsIncludeRoot1() throws Exception {
String mapping = XContentFactory.jsonBuilder()
.startObject().startObject("type").startObject("properties")
.startObject("nested1").field("type", "nested").field("include_in_root", true).field("include_in_parent", true).startObject("properties")
.startObject("nested2").field("type", "nested").field("include_in_root", true).field("include_in_parent", true)
.endObject().endObject().endObject()
.endObject().endObject().endObject().string();

DocumentMapper docMapper = createIndex("test").mapperService().documentMapperParser().parse("type", new CompressedXContent(mapping));

ParsedDocument doc = docMapper.parse(SourceToParse.source("test", "type", "1", XContentFactory.jsonBuilder()
.startObject().startArray("nested1")
.startObject().startArray("nested2").startObject().field("foo", "bar")
.endObject().endArray().endObject().endArray()
.endObject()
.bytes(),
XContentType.JSON));

final Collection<IndexableField> fields = doc.rootDoc().getFields();
assertThat(fields.size(), equalTo(new HashSet<>(fields).size()));
}

/**
* Same as {@link NestedObjectMapperTests#testMultipleLevelsIncludeRoot1()} but tests for the
* case where the transitive {@code include_in_parent} and redundant {@code include_in_root}
* happen on a chain of nodes that starts from a parent node that is not directly connected to
* root by a chain of {@code include_in_parent}, i.e. that has {@code include_in_parent} set to
* {@code false} and {@code include_in_root} set to {@code true}.
*/
public void testMultipleLevelsIncludeRoot2() throws Exception {
String mapping = XContentFactory.jsonBuilder()
.startObject().startObject("type").startObject("properties")
.startObject("nested1").field("type", "nested")
.field("include_in_root", true).field("include_in_parent", true).startObject("properties")
.startObject("nested2").field("type", "nested")
.field("include_in_root", true).field("include_in_parent", false).startObject("properties")
.startObject("nested3").field("type", "nested")
.field("include_in_root", true).field("include_in_parent", true)
.endObject().endObject().endObject().endObject().endObject()
.endObject().endObject().endObject().string();

DocumentMapper docMapper = createIndex("test").mapperService().documentMapperParser().parse("type", new CompressedXContent(mapping));

ParsedDocument doc = docMapper.parse(SourceToParse.source("test", "type", "1", XContentFactory.jsonBuilder()
.startObject().startArray("nested1")
.startObject().startArray("nested2")
.startObject().startArray("nested3").startObject().field("foo", "bar")
.endObject().endArray().endObject().endArray().endObject().endArray()
.endObject()
.bytes(),
XContentType.JSON));

final Collection<IndexableField> fields = doc.rootDoc().getFields();
assertThat(fields.size(), equalTo(new HashSet<>(fields).size()));
}

public void testNestedArrayStrict() throws Exception {
String mapping = XContentFactory.jsonBuilder().startObject().startObject("type").startObject("properties")
.startObject("nested1").field("type", "nested").field("dynamic", "strict").startObject("properties")
Expand Down