Fix missing intermediate object error when applying dynamic template

server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java

@@ -403,8 +403,8 @@ private static DocumentParserContext nestedContext(DocumentParserContext context
     }
 
     static void parseObjectOrField(DocumentParserContext context, Mapper mapper) throws IOException {
-        if (mapper instanceof ObjectMapper) {
-            parseObjectOrNested(context, (ObjectMapper) mapper);
+        if (mapper instanceof ObjectMapper objectMapper) {
+            parseObjectOrNested(context, objectMapper);
         } else if (mapper instanceof FieldMapper fieldMapper) {
             fieldMapper.parse(context);
             if (context.isWithinCopyTo() == false) {
@@ -447,9 +447,10 @@ private static void throwOnCopyToOnObject(Mapper mapper, List<String> copyToFiel
         );
     }
 
-    private static void parseObject(final DocumentParserContext context, ObjectMapper mapper, String currentFieldName) throws IOException {
+    private static void parseObject(final DocumentParserContext context, ObjectMapper parentObjectMapper, String currentFieldName)
+        throws IOException {
         assert currentFieldName != null;
-        Mapper objectMapper = getMapper(context, mapper, currentFieldName);
+        Mapper objectMapper = getMapper(context, parentObjectMapper, currentFieldName);
         if (objectMapper != null) {
             context.path().add(currentFieldName);
             if (objectMapper instanceof ObjectMapper objMapper) {
@@ -461,16 +462,17 @@ private static void parseObject(final DocumentParserContext context, ObjectMappe
             context.path().setWithinLeafObject(false);
             context.path().remove();
         } else {
-            parseObjectDynamic(context, mapper, currentFieldName);
+            parseObjectDynamic(context, parentObjectMapper, currentFieldName);
         }
     }
 
-    private static void parseObjectDynamic(DocumentParserContext context, ObjectMapper mapper, String currentFieldName) throws IOException {
-        ObjectMapper.Dynamic dynamic = dynamicOrDefault(mapper, context);
+    private static void parseObjectDynamic(DocumentParserContext context, ObjectMapper parentObjectMapper, String currentFieldName)
+        throws IOException {
+        ObjectMapper.Dynamic dynamic = dynamicOrDefault(parentObjectMapper, context);
         if (dynamic == ObjectMapper.Dynamic.STRICT) {
-            throw new StrictDynamicMappingException(mapper.fullPath(), currentFieldName);
+            throw new StrictDynamicMappingException(parentObjectMapper.fullPath(), currentFieldName);
         } else if (dynamic == ObjectMapper.Dynamic.FALSE) {
-            failIfMatchesRoutingPath(context, mapper, currentFieldName);
+            failIfMatchesRoutingPath(context, parentObjectMapper, currentFieldName);
             // not dynamic, read everything up to end object
             context.parser().skipChildren();
         } else {
@@ -483,13 +485,13 @@ private static void parseObjectDynamic(DocumentParserContext context, ObjectMapp
                 dynamicObjectMapper = DynamicFieldsBuilder.createDynamicObjectMapper(context, currentFieldName);
                 context.addDynamicMapper(dynamicObjectMapper);
             }
-            if (mapper.subobjects() == false) {
+            if (parentObjectMapper.subobjects() == false) {
                 if (dynamicObjectMapper instanceof NestedObjectMapper) {
                     throw new MapperParsingException(
                         "Tried to add nested object ["
                             + dynamicObjectMapper.simpleName()
                             + "] to object ["
-                            + mapper.name()
+                            + parentObjectMapper.name()
                             + "] which does not support subobjects"
                     );
                 }
@@ -498,7 +500,7 @@ private static void parseObjectDynamic(DocumentParserContext context, ObjectMapp
                         "Tried to add subobject ["
                             + dynamicObjectMapper.simpleName()
                             + "] to object ["
-                            + mapper.name()
+                            + parentObjectMapper.name()
                             + "] which does not support subobjects"
                     );
                 }

server/src/main/java/org/elasticsearch/index/mapper/DocumentParserContext.java

@@ -234,20 +234,44 @@ public final void addDynamicMapper(Mapper mapper) {
             && newFieldsSeen.add(mapper.name())) {
             mappingLookup.checkFieldLimit(indexSettings().getMappingTotalFieldsLimit(), newFieldsSeen.size());
         }
-        if (mapper instanceof ObjectMapper) {
-            dynamicObjectMappers.put(mapper.name(), (ObjectMapper) mapper);
+        if (mapper instanceof ObjectMapper objectMapper) {
+            dynamicObjectMappers.put(objectMapper.name(), objectMapper);
+            // dynamic object mappers may have been obtained from applying a dynamic template, in which case their definition may contain
+            // sub-fields as well as sub-objects that need to be added to the mappings
+            for (Mapper submapper : objectMapper.mappers.values()) {
+                // we could potentially skip the step of adding these to the dynamic mappers, because their parent is already added to
+                // that list, and what is important is that all of the intermediate objects are added to the dynamic object mappers so that
+                // they can be looked up once sub-fields need to be added to them. For simplicity, we treat these like any other object
+                addDynamicMapper(submapper);
+            }
         }
+        // TODO we may want to stop adding object mappers to the dynamic mappers list: most times they will be mapped when parsing their
+        // sub-fields (see ObjectMapper.Builder#addDynamic), which causes extra work as the two variants of the same object field
+        // will be merged together when creating the final dynamic update. The only cases where object fields need extra treatment are
+        // dynamically mapped objects when the incoming document defines no sub-fields in them:
+        // 1) by default, they would be empty containers in the mappings, is it then important to map them?
+        // 2) they can be the result of applying a dynamic template which may define sub-fields or set dynamic, enabled or subobjects.
         dynamicMappers.add(mapper);
     }
 
     /**
-     * Get dynamic mappers created while parsing.
+     * Get dynamic mappers created as a result of parsing an incoming document. Responsible for exposing all the newly created
+     * fields that need to be merged into the existing mappings. Used to create the required mapping update at the end of document parsing.
+     * Consists of a flat set of {@link Mapper}s that will need to be added to their respective parent {@link ObjectMapper}s in order
+     * to become part of the resulting dynamic mapping update.
      */
     public final List<Mapper> getDynamicMappers() {
         return dynamicMappers;
     }
 
-    public final ObjectMapper getDynamicObjectMapper(String name) {
+    /**
+     * Get a dynamic object mapper by name. Allows consumers to lookup objects that have been dynamically added as a result
+     * of parsing an incoming document. Used to find the parent object for new fields that are being dynamically mapped whose parent is
+     * also not mapped yet. Such new fields will need to be dynamically added to their parent according to its dynamic behaviour.
+     * Holds a flat set of object mappers, meaning that an object field named <code>foo.bar</code> can be looked up directly with its
+     * dotted name.
+     */
+    final ObjectMapper getDynamicObjectMapper(String name) {
         return dynamicObjectMappers.get(name);
     }
 
@@ -259,7 +283,9 @@ public final void addDynamicRuntimeField(RuntimeField runtimeField) {
     }
 
     /**
-     * Get dynamic runtime fields created while parsing.
+     * Get dynamic runtime fields created while parsing. Holds a flat set of {@link RuntimeField}s.
+     * Runtime fields get dynamically mapped when {@link org.elasticsearch.index.mapper.ObjectMapper.Dynamic#RUNTIME} is used,
+     * or when dynamic templates specify a <code>runtime</code> section.
      */
     public final List<RuntimeField> getDynamicRuntimeFields() {
         return Collections.unmodifiableList(dynamicRuntimeFields);

server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java

@@ -102,14 +102,14 @@ Builder addMappers(Map<String, Mapper> mappers) {
         }
 
         /**
-         * Adds a dynamically created Mapper to this builder.
+         * Adds a dynamically created {@link Mapper} to this builder.
          *
          * @param name      the name of the Mapper, including object prefixes
          * @param prefix    the object prefix of this mapper
          * @param mapper    the mapper to add
          * @param context   the DocumentParserContext in which the mapper has been built
          */
-        public void addDynamic(String name, String prefix, Mapper mapper, DocumentParserContext context) {
+        public final void addDynamic(String name, String prefix, Mapper mapper, DocumentParserContext context) {
             // If the mapper to add has no dots, or the current object mapper has subobjects set to false,
             // we just add it as it is for sure a leaf mapper
             if (name.contains(".") == false || subobjects.value() == false) {
@@ -118,29 +118,29 @@ public void addDynamic(String name, String prefix, Mapper mapper, DocumentParser
             // otherwise we strip off the first object path of the mapper name, load or create
             // the relevant object mapper, and then recurse down into it, passing the remainder
             // of the mapper name. So for a mapper 'foo.bar.baz', we locate 'foo' and then
-            // call addDynamic on it with the name 'bar.baz'.
+            // call addDynamic on it with the name 'bar.baz', and next call addDynamic on 'bar' with the name 'baz'.
             else {
                 int firstDotIndex = name.indexOf(".");
-                String childName = name.substring(0, firstDotIndex);
-                String fullChildName = prefix == null ? childName : prefix + "." + childName;
-                ObjectMapper.Builder childBuilder = findChild(fullChildName, context);
-                childBuilder.addDynamic(name.substring(firstDotIndex + 1), fullChildName, mapper, context);
-                add(childBuilder);
+                String immediateChild = name.substring(0, firstDotIndex);
+                String immediateChildFullName = prefix == null ? immediateChild : prefix + "." + immediateChild;
+                ObjectMapper.Builder parentBuilder = findObjectBuilder(immediateChildFullName, context);
+                parentBuilder.addDynamic(name.substring(firstDotIndex + 1), immediateChildFullName, mapper, context);
+                add(parentBuilder);
             }
         }
 
-        private static ObjectMapper.Builder findChild(String fullChildName, DocumentParserContext context) {
-            // does the child mapper already exist? if so, use that
-            ObjectMapper child = context.mappingLookup().objectMappers().get(fullChildName);
-            if (child != null) {
-                return child.newBuilder(context.indexSettings().getIndexVersionCreated());
+        private static ObjectMapper.Builder findObjectBuilder(String fullName, DocumentParserContext context) {
+            // does the object mapper already exist? if so, use that
+            ObjectMapper objectMapper = context.mappingLookup().objectMappers().get(fullName);
+            if (objectMapper != null) {
+                return objectMapper.newBuilder(context.indexSettings().getIndexVersionCreated());
             }
-            // has the child mapper been added as a dynamic update already?
-            child = context.getDynamicObjectMapper(fullChildName);
-            if (child != null) {
-                return child.newBuilder(context.indexSettings().getIndexVersionCreated());
+            // has the object mapper been added as a dynamic update already?
+            objectMapper = context.getDynamicObjectMapper(fullName);
+            if (objectMapper != null) {
+                return objectMapper.newBuilder(context.indexSettings().getIndexVersionCreated());
             }
-            throw new IllegalArgumentException("Missing intermediate object " + fullChildName);
+            throw new IllegalStateException("Missing intermediate object " + fullName);
         }
 
         protected final Map<String, Mapper> buildMappers(boolean root, MapperBuilderContext context) {

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

@@ -734,6 +734,20 @@ public void testObjectMappingUpdate() throws Exception {
         assertNotNull(((ObjectMapper) barMapper).getMapper("baz"));
     }
 
+    public void testEmptyObjectGetsMapped() throws Exception {
+        MapperService mapperService = createMapperService();
+        DocumentMapper docMapper = mapperService.documentMapper();
+        ParsedDocument doc = docMapper.parse(source(b -> {
+            b.startObject("foo");
+            b.endObject();
+        }));
+        Mapping mapping = doc.dynamicMappingsUpdate();
+        assertNotNull(mapping);
+        Mapper foo = mapping.getRoot().getMapper("foo");
+        assertThat(foo, instanceOf(ObjectMapper.class));
+        assertEquals(0, ((ObjectMapper) foo).mappers.size());
+    }
+
     public void testDynamicGeoPointArrayWithTemplate() throws Exception {
         DocumentMapper mapper = createDocumentMapper(topMapping(b -> {
             b.startArray("dynamic_templates");