Skip to content

Commit

Permalink
Fixes breaking change introduced in #95558. (#95711)
Browse files Browse the repository at this point in the history
* Fixes breaking change introduced in #95558.

Match and unmatch fields in dynamic_templates (match, unmatch, path_match, path_unmatch)
can either be single-valued or a list of values. If a list of values is provided, the
values must all be strings. Numbers, boolean, other lists, etc. are not allowed and an
error will be returned to the user in that case.

However, previously (before #95558) we allowed any JSON type for the match/unmatch
fields (not just string), so changing that would be a breaking change. Thus, this commit
no longer enforces string-only types for single-valued match/unmatch fields in dynamic_templates.
  • Loading branch information
quux00 authored May 2, 2023
1 parent e908fad commit 7d243a4
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -237,22 +237,20 @@ private static boolean matchPatternsAreDefined(List<String> match, List<String>
}

private static void addEntriesToPatternList(List<String> matchList, String propName, Map.Entry<String, Object> entry) {
if (entry.getValue() instanceof String s) {
matchList.add(s);
} else if (entry.getValue() instanceof List<?> ls) {
if (entry.getValue() instanceof List<?> ls) {
for (Object o : ls) {
if (o instanceof String s) {
matchList.add(s);
} else {
// for array-based matches, we enforce that only strings are allowed as un/match values in the array
throw new MapperParsingException(
Strings.format("[%s] values must either be a string or list of strings, but was [%s]", propName, entry.getValue())
);
}
}
} else {
throw new MapperParsingException(
Strings.format("[%s] values must either be a string or list of strings, but was [%s]", propName, entry.getValue())
);
// to preserve backwards compatability for single-valued matches, we convert whatever the user provided to a string
matchList.add(entry.getValue().toString());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,26 @@ public void testMatchTemplateName() throws Exception {
assertTrue(template.match("my_template", "foo.bar", "not_match_name", randomFrom(XContentFieldType.values())));
assertFalse(template.match(null, "foo.bar", "not_match_name", randomFrom(XContentFieldType.values())));
}
// match name with number rather than string (is allowed)
{
Map<String, Object> templateDef = new HashMap<>();
templateDef.put("match", 12);
templateDef.put("mapping", Map.of());
DynamicTemplate template = DynamicTemplate.parse("my_template", templateDef);
assertTrue(template.match("my_template", "foo.bar", "foo", randomFrom(XContentFieldType.values())));
assertTrue(template.match(null, null, "12", randomFrom(XContentFieldType.values())));
assertFalse(template.match("not_template_name", null, "12", randomFrom(XContentFieldType.values())));
assertTrue(template.match("my_template", "foo.bar", "not_match_name", randomFrom(XContentFieldType.values())));
assertFalse(template.match(null, "foo.bar", "not_match_name", randomFrom(XContentFieldType.values())));
}
// match name with array of patterns with non-strings (not allowed)
{
Map<String, Object> templateDef = new HashMap<>();
templateDef.put("match", List.of("baz*", "*quux", 12));
templateDef.put("mapping", Map.of());
MapperParsingException e = expectThrows(MapperParsingException.class, () -> DynamicTemplate.parse("my_template", templateDef));
assertEquals("[match] values must either be a string or list of strings, but was [[baz*, *quux, 12]]", e.getMessage());
}
// no match condition
{
Map<String, Object> templateDef = new HashMap<>();
Expand Down

0 comments on commit 7d243a4

Please sign in to comment.