prevent duplicate fields when mixing parent and root nested includes (#…

…27072) Closes #26990
elastic · Oct 31, 2017 · a4c159e · a4c159e
1 parent 3812d3c
commit a4c159e
Show file tree

Hide file tree

Showing 2 changed files with 96 additions and 0 deletions.
diff --git a/core/src/main/java/org/elasticsearch/index/mapper/RootObjectMapper.java b/core/src/main/java/org/elasticsearch/index/mapper/RootObjectMapper.java
@@ -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) {

diff --git a/core/src/test/java/org/elasticsearch/index/mapper/NestedObjectMapperTests.java b/core/src/test/java/org/elasticsearch/index/mapper/NestedObjectMapperTests.java
@@ -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;
@@ -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")