Moved dynamic field handling in doc parsing to end of parsing

core/src/main/java/org/elasticsearch/index/mapper/Mapper.java

@@ -76,6 +76,7 @@ public String name() {
             return this.name;
         }
 
+        /** Returns a newly built mapper. */
         public abstract Y build(BuilderContext context);
     }
 

core/src/main/java/org/elasticsearch/index/mapper/ParseContext.java

@@ -331,13 +331,13 @@ public StringBuilder stringBuilder() {
         }
 
         @Override
-        public void addDynamicMappingsUpdate(Mapper update) {
-            in.addDynamicMappingsUpdate(update);
+        public void addDynamicMapper(Mapper update) {
+            in.addDynamicMapper(update);
         }
 
         @Override
-        public Mapper dynamicMappingsUpdate() {
-            return in.dynamicMappingsUpdate();
+        public List<Mapper> getDynamicMappers() {
+            return in.getDynamicMappers();
         }
     }
 
@@ -369,7 +369,7 @@ public static class InternalParseContext extends ParseContext {
 
         private AllEntries allEntries = new AllEntries();
 
-        private Mapper dynamicMappingsUpdate = null;
+        private List<Mapper> dynamicMappers = new ArrayList<>();
 
         public InternalParseContext(@Nullable Settings indexSettings, DocumentMapperParser docMapperParser, DocumentMapper docMapper, ContentPath path) {
             this.indexSettings = indexSettings;
@@ -394,7 +394,7 @@ public void reset(XContentParser parser, Document document, SourceToParse source
             this.source = source == null ? null : sourceToParse.source();
             this.path.reset();
             this.allEntries = new AllEntries();
-            this.dynamicMappingsUpdate = null;
+            this.dynamicMappers = new ArrayList<>();
         }
 
         @Override
@@ -536,18 +536,13 @@ public StringBuilder stringBuilder() {
         }
 
         @Override
-        public void addDynamicMappingsUpdate(Mapper mapper) {
-            assert mapper instanceof RootObjectMapper : mapper;
-            if (dynamicMappingsUpdate == null) {
-                dynamicMappingsUpdate = mapper;
-            } else {
-                dynamicMappingsUpdate = dynamicMappingsUpdate.merge(mapper, false);
-            }
+        public void addDynamicMapper(Mapper mapper) {
+            dynamicMappers.add(mapper);
         }
 
         @Override
-        public Mapper dynamicMappingsUpdate() {
-            return dynamicMappingsUpdate;
+        public List<Mapper> getDynamicMappers() {
+            return dynamicMappers;
         }
     }
 
@@ -747,12 +742,12 @@ public final <T> T parseExternalValue(Class<T> clazz) {
     public abstract StringBuilder stringBuilder();
 
     /**
-     * Add a dynamic update to the root object mapper.
+     * Add a new mapper dynamically created while parsing.
      */
-    public abstract void addDynamicMappingsUpdate(Mapper update);
+    public abstract void addDynamicMapper(Mapper update);
 
     /**
-     * Get dynamic updates to the root object mapper.
+     * Get dynamic mappers created while parsing.
      */
-    public abstract Mapper dynamicMappingsUpdate();
+    public abstract List<Mapper> getDynamicMappers();
 }

core/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java

@@ -19,12 +19,20 @@
 
 package org.elasticsearch.index.mapper;
 
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.compress.CompressedXContent;
+import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.index.mapper.internal.UidFieldMapper;
+import org.elasticsearch.index.mapper.object.ObjectMapper;
 import org.elasticsearch.test.ESSingleNodeTestCase;
 
+import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
+
 // TODO: make this a real unit test
 public class DocumentParserTests extends ESSingleNodeTestCase {
 
@@ -61,4 +69,97 @@ public void testFieldDisabled() throws Exception {
         assertNotNull(doc.rootDoc().getField("bar"));
         assertNotNull(doc.rootDoc().getField(UidFieldMapper.NAME));
     }
+
+    DocumentMapper createDummyMapping(MapperService mapperService) throws Exception {
+        String mapping = jsonBuilder().startObject().startObject("type").startObject("properties")
+            .startObject("a").startObject("properties")
+            .startObject("b").field("type", "object").startObject("properties")
+            .startObject("c").field("type", "object")
+            .endObject().endObject().endObject().endObject().endObject().endObject().endObject().endObject().string();
+
+        DocumentMapper defaultMapper = mapperService.documentMapperParser().parse("type", new CompressedXContent(mapping));
+        return defaultMapper;
+    }
+
+    // creates an object mapper, which is about 100x harder than it should be....
+    ObjectMapper createObjectMapper(MapperService mapperService, String name) throws Exception {
+        String[] nameParts = name.split("\\.");
+        ContentPath path = new ContentPath();
+        for (int i = 0; i < nameParts.length - 1; ++i) {
+            path.add(nameParts[i]);
+        }
+        ParseContext context = new ParseContext.InternalParseContext(Settings.EMPTY,
+            mapperService.documentMapperParser(), mapperService.documentMapper("type"), path);
+        Mapper.Builder builder = new ObjectMapper.Builder(nameParts[nameParts.length - 1]).enabled(true);
+        Mapper.BuilderContext builderContext = new Mapper.BuilderContext(context.indexSettings(), context.path());
+        return (ObjectMapper)builder.build(builderContext);
+    }
+
+    public void testEmptyMappingUpdate() throws Exception {
+        DocumentMapper docMapper = createDummyMapping(createIndex("test").mapperService());
+        assertNull(DocumentParser.createDynamicUpdate(docMapper.mapping(), docMapper, Collections.emptyList()));
+    }
+
+    public void testSingleMappingUpdate() throws Exception {
+        DocumentMapper docMapper = createDummyMapping(createIndex("test").mapperService());
+        List<Mapper> updates = Collections.singletonList(new MockFieldMapper("foo"));
+        Mapping mapping = DocumentParser.createDynamicUpdate(docMapper.mapping(), docMapper, updates);
+        assertNotNull(mapping.root().getMapper("foo"));
+    }
+
+    public void testSubfieldMappingUpdate() throws Exception {
+        DocumentMapper docMapper = createDummyMapping(createIndex("test").mapperService());
+        List<Mapper> updates = Collections.singletonList(new MockFieldMapper("a.foo"));
+        Mapping mapping = DocumentParser.createDynamicUpdate(docMapper.mapping(), docMapper, updates);
+        Mapper aMapper = mapping.root().getMapper("a");
+        assertNotNull(aMapper);
+        assertTrue(aMapper instanceof ObjectMapper);
+        assertNotNull(((ObjectMapper)aMapper).getMapper("foo"));
+        assertNull(((ObjectMapper)aMapper).getMapper("b"));
+    }
+
+    public void testMultipleSubfieldMappingUpdate() throws Exception {
+        DocumentMapper docMapper = createDummyMapping(createIndex("test").mapperService());
+        List<Mapper> updates = new ArrayList<>();
+        updates.add(new MockFieldMapper("a.foo"));
+        updates.add(new MockFieldMapper("a.bar"));
+        Mapping mapping = DocumentParser.createDynamicUpdate(docMapper.mapping(), docMapper, updates);
+        Mapper aMapper = mapping.root().getMapper("a");
+        assertNotNull(aMapper);
+        assertTrue(aMapper instanceof ObjectMapper);
+        assertNotNull(((ObjectMapper)aMapper).getMapper("foo"));
+        assertNotNull(((ObjectMapper)aMapper).getMapper("bar"));
+        assertNull(((ObjectMapper)aMapper).getMapper("b"));
+    }
+
+    public void testDeepSubfieldMappingUpdate() throws Exception {
+        DocumentMapper docMapper = createDummyMapping(createIndex("test").mapperService());
+        List<Mapper> updates = Collections.singletonList(new MockFieldMapper("a.b.foo"));
+        Mapping mapping = DocumentParser.createDynamicUpdate(docMapper.mapping(), docMapper, updates);
+        Mapper aMapper = mapping.root().getMapper("a");
+        assertNotNull(aMapper);
+        assertTrue(aMapper instanceof ObjectMapper);
+        Mapper bMapper = ((ObjectMapper)aMapper).getMapper("b");
+        assertTrue(bMapper instanceof ObjectMapper);
+        assertNotNull(((ObjectMapper)bMapper).getMapper("foo"));
+        assertNull(((ObjectMapper)bMapper).getMapper("c"));
+    }
+
+    public void testObjectMappingUpdate() throws Exception {
+        MapperService mapperService = createIndex("test").mapperService();
+        DocumentMapper docMapper = createDummyMapping(mapperService);
+        List<Mapper> updates = new ArrayList<>();
+        updates.add(createObjectMapper(mapperService, "foo"));
+        updates.add(createObjectMapper(mapperService, "foo.bar"));
+        updates.add(new MockFieldMapper("foo.bar.baz"));
+        updates.add(new MockFieldMapper("foo.field"));
+        Mapping mapping = DocumentParser.createDynamicUpdate(docMapper.mapping(), docMapper, updates);
+        Mapper fooMapper = mapping.root().getMapper("foo");
+        assertNotNull(fooMapper);
+        assertTrue(fooMapper instanceof ObjectMapper);
+        assertNotNull(((ObjectMapper)fooMapper).getMapper("field"));
+        Mapper barMapper = ((ObjectMapper)fooMapper).getMapper("bar");
+        assertTrue(barMapper instanceof ObjectMapper);
+        assertNotNull(((ObjectMapper)barMapper).getMapper("baz"));
+    }
 }

core/src/test/java/org/elasticsearch/index/mapper/DynamicMappingTests.java

@@ -42,6 +42,7 @@
 import org.elasticsearch.test.ESSingleNodeTestCase;
 
 import java.io.IOException;
+import java.util.List;
 
 import static java.util.Collections.emptyMap;
 import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
@@ -211,7 +212,9 @@ private Mapper parse(DocumentMapper mapper, DocumentMapperParser parser, XConten
         ctx.reset(XContentHelper.createParser(source.source()), new ParseContext.Document(), source);
         assertEquals(XContentParser.Token.START_OBJECT, ctx.parser().nextToken());
         ctx.parser().nextToken();
-        return DocumentParser.parseObject(ctx, mapper.root(), true);
+        DocumentParser.parseObjectOrNested(ctx, mapper.root(), true);
+        Mapping mapping = DocumentParser.createDynamicUpdate(mapper.mapping(), mapper, ctx.getDynamicMappers());
+        return mapping == null ? null : mapping.root();
     }
 
     public void testDynamicMappingsNotNeeded() throws Exception {