From fc60b91f450907c65eee339961fd40195f95005c Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Thu, 12 Nov 2020 15:03:08 +0100 Subject: [PATCH] Improve error message in case of invalid dynamic templates. Backporting #60870 to 7.x branch. Include the attempted 'match_mapping_type' into the message, so that it is clearer that multiple validation attempts have occurred. Dynamic template validation was recently added via #51233 and there was some confusion over the deprecation message itself. (in 7.x only deprecation warning will be omitted and from 8.0 an error will be returned) --- docs/reference/mapping/dynamic/templates.asciidoc | 3 ++- .../elasticsearch/index/mapper/RootObjectMapper.java | 7 +++++-- .../index/mapper/RootObjectMapperTests.java | 11 +++++++++-- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/docs/reference/mapping/dynamic/templates.asciidoc b/docs/reference/mapping/dynamic/templates.asciidoc index 83b7dd65376e0..dcf10f8791624 100644 --- a/docs/reference/mapping/dynamic/templates.asciidoc +++ b/docs/reference/mapping/dynamic/templates.asciidoc @@ -46,7 +46,8 @@ snippet may cause the update or validation of a dynamic template to fail under c the mapping snippet is considered valid. However, a validation error is returned at index time if a field matching the template is indexed as a different type. For example, configuring a dynamic template with no `match_mapping_type` is considered valid as string type, but if a field matching the dynamic template is indexed as a long, a validation - error is returned at index time. + error is returned at index time. It is recommended to configure the `match_mapping_type` to the expected JSON type or + configure the desired `type` in the mapping snippet. * If the `{name}` placeholder is used in the mapping snippet, validation is skipped when updating the dynamic template. This is because the field name is unknown at that time. Instead, validation occurs when the template is applied diff --git a/server/src/main/java/org/elasticsearch/index/mapper/RootObjectMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/RootObjectMapper.java index 7fb307dfe2236..5181352945604 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/RootObjectMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/RootObjectMapper.java @@ -31,6 +31,7 @@ import java.io.IOException; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.Iterator; @@ -405,8 +406,10 @@ private static void validateDynamicTemplate(Mapper.TypeParser.ParserContext pars final boolean shouldEmitDeprecationWarning = parserContext.indexVersionCreated().onOrAfter(Version.V_7_7_0); if (dynamicTemplateInvalid && shouldEmitDeprecationWarning) { - String message = String.format(Locale.ROOT, "dynamic template [%s] has invalid content [%s]", - dynamicTemplate.getName(), Strings.toString(dynamicTemplate)); + String format = "dynamic template [%s] has invalid content [%s], " + + "attempted to validate it with the following match_mapping_type: [%s]"; + String message = String.format(Locale.ROOT, format, dynamicTemplate.getName(), Strings.toString(dynamicTemplate), + Arrays.toString(types)); final String deprecationMessage; if (lastError != null) { diff --git a/server/src/test/java/org/elasticsearch/index/mapper/RootObjectMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/RootObjectMapperTests.java index 5452deda585bc..9e80227773c33 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/RootObjectMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/RootObjectMapperTests.java @@ -311,7 +311,8 @@ public void testIllegalDynamicTemplateUnknownFieldType() throws Exception { DocumentMapper mapper = mapperService.merge("type", new CompressedXContent(Strings.toString(mapping)), MergeReason.MAPPING_UPDATE); assertThat(mapper.mappingSource().toString(), containsString("\"type\":\"string\"")); assertWarnings("dynamic template [my_template] has invalid content [{\"match_mapping_type\":\"string\",\"mapping\":{\"type\":" + - "\"string\"}}], caused by [No mapper found for type [string]]"); + "\"string\"}}], attempted to validate it with the following match_mapping_type: [[string]], " + + "caused by [No mapper found for type [string]]"); } public void testIllegalDynamicTemplateUnknownAttribute() throws Exception { @@ -340,6 +341,7 @@ public void testIllegalDynamicTemplateUnknownAttribute() throws Exception { assertThat(mapper.mappingSource().toString(), containsString("\"foo\":\"bar\"")); assertWarnings("dynamic template [my_template] has invalid content [{\"match_mapping_type\":\"string\",\"mapping\":{" + "\"foo\":\"bar\",\"type\":\"keyword\"}}], " + + "attempted to validate it with the following match_mapping_type: [[string]], " + "caused by [unknown parameter [foo] on mapper [__dynamic__my_template] of type [keyword]]"); } @@ -368,7 +370,8 @@ public void testIllegalDynamicTemplateInvalidAttribute() throws Exception { DocumentMapper mapper = mapperService.merge("type", new CompressedXContent(Strings.toString(mapping)), MergeReason.MAPPING_UPDATE); assertThat(mapper.mappingSource().toString(), containsString("\"analyzer\":\"foobar\"")); assertWarnings("dynamic template [my_template] has invalid content [{\"match_mapping_type\":\"string\",\"mapping\":{" + - "\"analyzer\":\"foobar\",\"type\":\"text\"}}], caused by [analyzer [foobar] has not been configured in mappings]"); + "\"analyzer\":\"foobar\",\"type\":\"text\"}}], attempted to validate it with the following match_mapping_type: [[string]], " + + "caused by [analyzer [foobar] has not been configured in mappings]"); } public void testIllegalDynamicTemplateNoMappingType() throws Exception { @@ -437,10 +440,14 @@ public void testIllegalDynamicTemplateNoMappingType() throws Exception { if (useMatchMappingType) { assertWarnings("dynamic template [my_template] has invalid content [{\"match_mapping_type\":\"*\",\"mapping\":{" + "\"foo\":\"bar\",\"type\":\"{dynamic_type}\"}}], " + + "attempted to validate it with the following match_mapping_type: " + + "[[object, string, long, double, boolean, date, binary]], " + "caused by [unknown parameter [foo] on mapper [__dynamic__my_template] of type [binary]]"); } else { assertWarnings("dynamic template [my_template] has invalid content [{\"match\":\"string_*\",\"mapping\":{" + "\"foo\":\"bar\",\"type\":\"{dynamic_type}\"}}], " + + "attempted to validate it with the following match_mapping_type: " + + "[[object, string, long, double, boolean, date, binary]], " + "caused by [unknown parameter [foo] on mapper [__dynamic__my_template] of type [binary]]"); } }