From b3eb3c7efdd0fe513f3cdfe8155bc2ae6be31d12 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Mon, 10 Aug 2020 10:07:32 +0200 Subject: [PATCH] Improve error message in case of invalid dynamic templates 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 | 15 ++++++++------- .../index/mapper/RootObjectMapper.java | 8 +++++--- .../index/mapper/RootObjectMapperTests.java | 3 ++- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/docs/reference/mapping/dynamic/templates.asciidoc b/docs/reference/mapping/dynamic/templates.asciidoc index 911320cc9aa39..b95daff04ad64 100644 --- a/docs/reference/mapping/dynamic/templates.asciidoc +++ b/docs/reference/mapping/dynamic/templates.asciidoc @@ -38,15 +38,16 @@ Dynamic templates are specified as an array of named objects: <3> The mapping that the matched field should use. If a provided mapping contains an invalid mapping snippet, a validation error -is returned. Validation occurs when applying the dynamic template at index time, -and, in most cases, when the dynamic template is updated. Providing an invalid mapping +is returned. Validation occurs when applying the dynamic template at index time, +and, in most cases, when the dynamic template is updated. Providing an invalid mapping snippet may cause the update or validation of a dynamic template to fail under certain conditions: -* If no `match_mapping_type` has been specified but the template is valid for at least one predefined mapping type, - 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. +* If no `match_mapping_type` has been specified but the template is valid for at least one predefined mapping type, + 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. 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 c2e2cf2f74ec2..18b767540a84f 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/RootObjectMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/RootObjectMapper.java @@ -33,6 +33,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; @@ -408,14 +409,15 @@ private static void validateDynamicTemplate(Mapper.TypeParser.ParserContext pars final boolean failInvalidDynamicTemplates = parserContext.indexVersionCreated().onOrAfter(Version.V_8_0_0); if (dynamicTemplateInvalid) { - 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)); if (failInvalidDynamicTemplates) { throw new IllegalArgumentException(message, lastError); } else { final String deprecationMessage; if (lastError != null) { - deprecationMessage = String.format(Locale.ROOT, "%s, caused by [%s]", message, lastError.getMessage()); + deprecationMessage = String.format(Locale.ROOT, "%s, last error: [%s]", message, lastError.getMessage()); } else { deprecationMessage = message; } 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 c714615fe29eb..aef69ddf9d96d 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/RootObjectMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/RootObjectMapperTests.java @@ -470,6 +470,7 @@ public void testIllegalDynamicTemplate7DotXIndex() 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]], " + + "last error: [No mapper found for type [string]]"); } }