Check earlier for nested type with disabled subobjects while parsing doc (#87644)

With #87218 we have expanded testing and made sure that we check that the nested type is not used within objects that have subobjects disabled.
This commit moves the check needed while applying dynamic mappings to an earlier point, where it is possible to distinguish between nested and object, so we can expose a different error in the two scenarios.

Also, this commit rewords the error message thrown so that it is more user-friendly.

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

@@ -483,6 +483,26 @@ public final class DocumentParser {
                 dynamicObjectMapper = DynamicFieldsBuilder.createDynamicObjectMapper(context, currentFieldName);
                 context.addDynamicMapper(dynamicObjectMapper);
             }
+            if (mapper.subobjects() == false) {
+                if (dynamicObjectMapper instanceof NestedObjectMapper) {
+                    throw new MapperParsingException(
+                        "Tried to add nested object ["
+                            + dynamicObjectMapper.simpleName()
+                            + "] to object ["
+                            + mapper.name()
+                            + "] which does not support subobjects"
+                    );
+                }
+                if (dynamicObjectMapper instanceof ObjectMapper) {
+                    throw new MapperParsingException(
+                        "Tried to add subobject ["
+                            + dynamicObjectMapper.simpleName()
+                            + "] to object ["
+                            + mapper.name()
+                            + "] which does not support subobjects"
+                    );
+                }
+            }
             if (dynamicObjectMapper instanceof NestedObjectMapper && context.isWithinCopyTo()) {
                 throwOnCreateDynamicNestedViaCopyTo(dynamicObjectMapper);
             }

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

@@ -148,19 +148,7 @@ public class ObjectMapper extends Mapper implements Cloneable {
             Map<String, Mapper> mappers = new HashMap<>();
             for (Mapper.Builder builder : mappersBuilders) {
                 Mapper mapper = builder.build(mapperBuilderContext);
-                if (subobjects.value() == false && mapper instanceof ObjectMapper) {
-                    // we already check this at parse time (parseProperties) but we need to check again
-                    // here as dynamic mapping updates don't go through parsing.
-                    // Nested can only be dynamically mapped through dynamic templates, which are parsed and validated earlier.
-                    assert mapper instanceof NestedObjectMapper == false;
-                    throw new IllegalArgumentException(
-                        "Object ["
-                            + context.buildFullName(name)
-                            + "] has subobjects set to false hence it does not support inner object ["
-                            + mapper.simpleName()
-                            + "]"
-                    );
-                }
+                assert mapper instanceof ObjectMapper == false || subobjects.value() : "unexpected object while subobjects are disabled";
                 Mapper existing = mappers.get(mapper.simpleName());
                 if (existing != null) {
                     mapper = existing.merge(mapper, mapperBuilderContext);
@@ -285,20 +273,20 @@ public class ObjectMapper extends Mapper implements Cloneable {
 
                     if (objBuilder.subobjects.value() == false && type.equals(ObjectMapper.CONTENT_TYPE)) {
                         throw new MapperParsingException(
-                            "Object ["
-                                + objBuilder.name()
-                                + "] has subobjects set to false hence it does not support inner object ["
+                            "Tried to add subobject ["
                                 + fieldName
-                                + "]"
+                                + "] to object ["
+                                + objBuilder.name()
+                                + "] which does not support subobjects"
                         );
                     }
                     if (objBuilder.subobjects.value() == false && type.equals(NestedObjectMapper.CONTENT_TYPE)) {
                         throw new MapperParsingException(
-                            "Object ["
-                                + objBuilder.name()
-                                + "] has subobjects set to false hence it does not support nested object ["
+                            "Tried to add nested object ["
                                 + fieldName
-                                + "]"
+                                + "] to object ["
+                                + objBuilder.name()
+                                + "] which does not support subobjects"
                         );
                     }
                     Mapper.TypeParser typeParser = parserContext.typeParser(type);

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

@@ -1846,7 +1846,7 @@ public class DocumentParserTests extends MapperServiceTestCase {
         DocumentMapper mapper = createDocumentMapper(
             mapping(b -> b.startObject("metrics.service").field("type", "object").field("subobjects", false).endObject())
         );
-        IllegalArgumentException err = expectThrows(IllegalArgumentException.class, () -> mapper.parse(source("""
+        MapperParsingException err = expectThrows(MapperParsingException.class, () -> mapper.parse(source("""
             {
               "metrics": {
                 "service": {
@@ -1857,17 +1857,14 @@ public class DocumentParserTests extends MapperServiceTestCase {
               }
             }
             """)));
-        assertEquals(
-            "Object [metrics.service] has subobjects set to false hence it does not support inner object [time]",
-            err.getMessage()
-        );
+        assertEquals("Tried to add subobject [time] to object [metrics.service] which does not support subobjects", err.getMessage());
     }
 
     public void testSubobjectsFalseWithInnerDottedObject() throws Exception {
         DocumentMapper mapper = createDocumentMapper(
             mapping(b -> b.startObject("metrics.service").field("type", "object").field("subobjects", false).endObject())
         );
-        IllegalArgumentException err = expectThrows(IllegalArgumentException.class, () -> mapper.parse(source("""
+        MapperParsingException err = expectThrows(MapperParsingException.class, () -> mapper.parse(source("""
             {
               "metrics": {
                 "service": {
@@ -1879,14 +1876,14 @@ public class DocumentParserTests extends MapperServiceTestCase {
             }
             """)));
         assertEquals(
-            "Object [metrics.service] has subobjects set to false hence it does not support inner object [test.with.dots]",
+            "Tried to add subobject [test.with.dots] to object [metrics.service] which does not support subobjects",
             err.getMessage()
         );
     }
 
     public void testSubobjectsFalseRootWithInnerObject() throws Exception {
         DocumentMapper mapper = createDocumentMapper(topMapping(b -> b.field("subobjects", false)));
-        IllegalArgumentException err = expectThrows(IllegalArgumentException.class, () -> mapper.parse(source("""
+        MapperParsingException err = expectThrows(MapperParsingException.class, () -> mapper.parse(source("""
             {
               "metrics": {
                 "service": {
@@ -1895,7 +1892,7 @@ public class DocumentParserTests extends MapperServiceTestCase {
               }
             }
             """)));
-        assertEquals("Object [_doc] has subobjects set to false hence it does not support inner object [metrics]", err.getMessage());
+        assertEquals("Tried to add subobject [metrics] to object [_doc] which does not support subobjects", err.getMessage());
     }
 
     public void testSubobjectsFalseRoot() throws Exception {
@@ -2035,7 +2032,7 @@ public class DocumentParserTests extends MapperServiceTestCase {
         DocumentMapper mapper = createDocumentMapper(
             mapping(b -> b.startObject("metrics").field("type", "object").field("subobjects", false).endObject())
         );
-        IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, () -> mapper.parse(source("""
+        MapperParsingException err = expectThrows(MapperParsingException.class, () -> mapper.parse(source("""
             {
               "metrics.service.time": [
                 {
@@ -2045,8 +2042,8 @@ public class DocumentParserTests extends MapperServiceTestCase {
             }
             """)));
         assertEquals(
-            "Object [metrics] has subobjects set to false hence it does not support inner object [service.time]",
-            iae.getMessage()
+            "Tried to add subobject [service.time] to object [metrics] which does not support subobjects",
+            err.getRootCause().getMessage()
         );
     }
 

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

@@ -1175,6 +1175,73 @@ public class DynamicTemplatesTests extends MapperServiceTestCase {
         assertNoSubobjects(mapperService);
     }
 
+    public void testSubobjectFalseDynamicNestedNotAllowed() throws IOException {
+        DocumentMapper mapper = createDocumentMapper(topMapping(b -> {
+            b.startArray("dynamic_templates");
+            {
+                b.startObject();
+                b.startObject("nested");
+                {
+                    b.field("match", "object");
+                    b.startObject("mapping");
+                    {
+                        b.field("type", "nested");
+                    }
+                    b.endObject();
+                }
+                b.endObject();
+                b.endObject();
+            }
+            b.endArray();
+            b.startObject("properties");
+            b.startObject("metrics").field("type", "object").field("subobjects", false).endObject();
+            b.endObject();
+        }));
+
+        MapperParsingException err = expectThrows(MapperParsingException.class, () -> mapper.parse(source("""
+            {
+              "metrics.object" : [
+                {}
+              ]
+            }
+            """)));
+        assertEquals(
+            "Tried to add nested object [object] to object [metrics] which does not support subobjects",
+            err.getRootCause().getMessage()
+        );
+    }
+
+    public void testRootSubobjectFalseDynamicNestedNotAllowed() throws IOException {
+        DocumentMapper mapper = createDocumentMapper(topMapping(b -> {
+            b.startArray("dynamic_templates");
+            {
+                b.startObject();
+                b.startObject("nested");
+                {
+                    b.field("match", "object");
+                    b.startObject("mapping");
+                    {
+                        b.field("type", "nested");
+                    }
+                    b.endObject();
+                }
+                b.endObject();
+                b.endObject();
+            }
+            b.endArray();
+            b.field("subobjects", false);
+        }));
+
+        MapperParsingException err = expectThrows(MapperParsingException.class, () -> mapper.parse(source("""
+            {
+              "object" : [
+                {}
+              ]
+            }
+            """)));
+        assertEquals("Tried to add nested object [object] to object [_doc] which does not support subobjects", err.getMessage());
+    }
+
     public void testSubobjectsFalseDocsWithGeoPointFromDynamicTemplate() throws Exception {
         DocumentMapper mapper = createDocumentMapper(topMapping(b -> {
             b.startArray("dynamic_templates");
@@ -1276,7 +1343,7 @@ public class DynamicTemplatesTests extends MapperServiceTestCase {
         assertNotNull(service.getMapper("time"));
     }
 
-    public void testSubobjectsFalseWithInnerNestedFromDynamicTemplate() throws IOException {
+    public void testSubobjectsFalseWithInnerNestedFromDynamicTemplate() {
         MapperParsingException exception = expectThrows(MapperParsingException.class, () -> createMapperService(topMapping(b -> {
             b.startArray("dynamic_templates");
             {
@@ -1312,7 +1379,7 @@ public class DynamicTemplatesTests extends MapperServiceTestCase {
         );
         assertThat(exception.getRootCause(), instanceOf(MapperParsingException.class));
         assertEquals(
-            "Object [__dynamic__test] has subobjects set to false hence it does not support nested object [time]",
+            "Tried to add nested object [time] to object [__dynamic__test] which does not support subobjects",
             exception.getRootCause().getMessage()
         );
     }

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

@@ -396,7 +396,7 @@ public class ObjectMapperTests extends MapperServiceTestCase {
             b.endObject();
         })));
         assertEquals(
-            "Failed to parse mapping: Object [service] has subobjects set to false hence it does not support inner object [time]",
+            "Failed to parse mapping: Tried to add subobject [time] to object [service] which does not support subobjects",
             exception.getMessage()
         );
     }
@@ -417,7 +417,7 @@ public class ObjectMapperTests extends MapperServiceTestCase {
             b.endObject();
         })));
         assertEquals(
-            "Failed to parse mapping: Object [service] has subobjects set to false hence it does not support nested object [time]",
+            "Failed to parse mapping: Tried to add nested object [time] to object [service] which does not support subobjects",
             exception.getMessage()
         );
     }
@@ -465,8 +465,7 @@ public class ObjectMapperTests extends MapperServiceTestCase {
             b.endObject();
         })));
         assertEquals(
-            "Failed to parse mapping: Object [_doc] has subobjects set to false hence it does not support inner object "
-                + "[metrics.service.time]",
+            "Failed to parse mapping: Tried to add subobject [metrics.service.time] to object [_doc] which does not support subobjects",
             exception.getMessage()
         );
     }
@@ -483,8 +482,7 @@ public class ObjectMapperTests extends MapperServiceTestCase {
             b.endObject();
         })));
         assertEquals(
-            "Failed to parse mapping: Object [_doc] has subobjects set to false hence it does not support nested object "
-                + "[metrics.service]",
+            "Failed to parse mapping: Tried to add nested object [metrics.service] to object [_doc] which does not support subobjects",
             exception.getMessage()
         );
     }