Improve auto flattening exception message

felixbarny · Feb 21, 2024 · 2030a4b · 2030a4b
1 parent c58e245
commit 2030a4b
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 18 deletions.
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java
@@ -649,28 +649,35 @@ private void asFlattenedFieldMappers(MapperBuilderContext context, List<FieldMap
 
     private void ensureFlattenable(MapperBuilderContext context, ContentPath path) {
         if (dynamic != null && context.getDynamic() != dynamic) {
-            throw new IllegalArgumentException(
-                "cannot flatten object ["
-                    + path.pathAsText(simpleName())
-                    + "] because the value of [dynamic] ("
+            throwAutoFlatteningException(
+                path,
+                "the value of [dynamic] ("
                     + dynamic
                     + ") is not compatible with the value from its parent context ("
                     + context.getDynamic()
                     + ")"
             );
         }
         if (isEnabled() == false) {
-            throw new IllegalArgumentException(
-                "cannot flatten object [" + path.pathAsText(simpleName()) + "] because the value of [enabled] is [false]"
-            );
+            throwAutoFlatteningException(path, "the value of [enabled] is [false]");
         }
         if (subobjects.explicit() && subobjects()) {
-            throw new IllegalArgumentException(
-                "cannot flatten object [" + path.pathAsText(simpleName()) + "] because the value of [subobjects] is [true]"
-            );
+            throwAutoFlatteningException(path, "the value of [subobjects] is [true]");
         }
     }
 
+    private void throwAutoFlatteningException(ContentPath path, String reason) {
+        throw new IllegalArgumentException(
+            "Object mapper ["
+                + path.pathAsText(simpleName())
+                + "] was found in a context where subobjects is set to false. "
+                + "Auto-flattening ["
+                + path.pathAsText(simpleName())
+                + "] failed because "
+                + reason
+        );
+    }
+
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
         toXContent(builder, params, null);

diff --git a/server/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java b/server/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java
@@ -1589,8 +1589,9 @@ public void testAutoFlattenObjectsSubobjectsMergeConflictingMappingParameter() t
         assertThat(
             e.getMessage(),
             containsString(
-                "Failed to parse mapping: cannot flatten object [parent] because the value of [dynamic] (FALSE) is not compatible "
-                    + "with the value from its parent context (TRUE)"
+                "Failed to parse mapping: Object mapper [parent] was found in a context where subobjects is set to false. " +
+                "Auto-flattening [parent] failed because the value of [dynamic] (FALSE) is not compatible " +
+                "with the value from its parent context (TRUE)"
             )
         );
     }
@@ -1629,8 +1630,9 @@ public void testAutoFlattenObjectsSubobjectsMergeConflictingMappingParameterRoot
         assertThat(
             e.getMessage(),
             containsString(
-                "Failed to parse mapping: cannot flatten object [parent] because the value of [dynamic] (TRUE) is not compatible "
-                    + "with the value from its parent context (FALSE)"
+                "Failed to parse mapping: Object mapper [parent] was found in a context where subobjects is set to false. " +
+                "Auto-flattening [parent] failed because the value of [dynamic] (TRUE) is not compatible " +
+                "with the value from its parent context (FALSE)"
             )
         );
     }

diff --git a/server/src/test/java/org/elasticsearch/index/mapper/ObjectMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/ObjectMapperTests.java
@@ -595,8 +595,9 @@ public void testFlattenDynamicIncompatible() {
             () -> objectMapper.asFlattenedFieldMappers(rootContext)
         );
         assertEquals(
-            "cannot flatten object [parent.child] because the value of [dynamic] (FALSE) is not compatible with the value from its "
-                + "parent context (TRUE)",
+            "Object mapper [parent.child] was found in a context where subobjects is set to false. " +
+            "Auto-flattening [parent.child] failed because the value of [dynamic] (FALSE) is not compatible with " +
+            "the value from its parent context (TRUE)",
             exception.getMessage()
         );
     }
@@ -609,7 +610,11 @@ public void testFlattenEnabledFalse() {
             IllegalArgumentException.class,
             () -> objectMapper.asFlattenedFieldMappers(rootContext)
         );
-        assertEquals("cannot flatten object [parent] because the value of [enabled] is [false]", exception.getMessage());
+        assertEquals(
+            "Object mapper [parent] was found in a context where subobjects is set to false. "
+                + "Auto-flattening [parent] failed because the value of [enabled] is [false]",
+            exception.getMessage()
+        );
     }
 
     public void testFlattenExplicitSubobjectsTrue() {
@@ -620,6 +625,10 @@ public void testFlattenExplicitSubobjectsTrue() {
             IllegalArgumentException.class,
             () -> objectMapper.asFlattenedFieldMappers(rootContext)
         );
-        assertEquals("cannot flatten object [parent] because the value of [subobjects] is [true]", exception.getMessage());
+        assertEquals(
+            "Object mapper [parent] was found in a context where subobjects is set to false. "
+                + "Auto-flattening [parent] failed because the value of [subobjects] is [true]",
+            exception.getMessage()
+        );
     }
 }