Skip to content

Commit cc0b834

Browse files
romseygeekromseygeek
committed
Add explicit tests for dynamic template validation on bad mapping parameters (#74177)
When a dynamic template is applied to a 7.x index, we do some validation checks against the generated mappings and issue a deprecation warning if the validation fails. We had some tests that were checking this at a low level, but nothing that exercised the full logic. When dynamic runtimes were added, we expected this behaviour to carry over; however, a bug in building the ParserContext to pass to the runtime Builder meant that we would instead always throw an error. In fact, erroring here is a good result, as the only reason we issue a deprecation warning for non-runtime fields is to preserve backward compatibility; given that runtime fields are new, it has never been possible to add a bad dynamic runtime template. This commit adds specific tests for both situations. It ensures that the parser context passed to a runtime builder will know that it is in a dynamic context, but it also removes the version check and deprecation warning, as they were never actually triggered and we can be stricter here.
1 parent 20ed4fe commit cc0b834

File tree

6 files changed

+62
-27
lines changed

6 files changed

+62
-27
lines changed

‎server/src/main/java/org/elasticsearch/index/mapper/DynamicFieldsBuilder.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ private static boolean applyMatchingTemplate(ParseContext context,
193193
String mappingType = dynamicTemplate.mappingType(dynamicType);
194194
Map<String, Object> mapping = dynamicTemplate.mappingForName(name, dynamicType);
195195
if (dynamicTemplate.isRuntimeMapping()) {
196-
Mapper.TypeParser.ParserContext parserContext = context.parserContext(dateFormatter);
196+
Mapper.TypeParser.ParserContext parserContext = context.dynamicTemplateParserContext(dateFormatter);
197197
RuntimeField.Parser parser = parserContext.runtimeFieldParser(mappingType);
198198
String fullName = context.path().pathAsText(name);
199199
if (parser == null) {
@@ -225,8 +225,7 @@ private static Mapper.Builder parseDynamicTemplateMapping(String name,
225225
Map<String, Object> mapping,
226226
DateFormatter dateFormatter,
227227
ParseContext context) {
228-
Mapper.TypeParser.ParserContext parserContext = context.parserContext(dateFormatter);
229-
parserContext = parserContext.createDynamicTemplateFieldContext(parserContext);
228+
Mapper.TypeParser.ParserContext parserContext = context.dynamicTemplateParserContext(dateFormatter);
230229
Mapper.TypeParser typeParser = parserContext.typeParser(mappingType);
231230
if (typeParser == null) {
232231
throw new MapperParsingException("failed to find type parsed [" + mappingType + "] for [" + name + "]");

‎server/src/main/java/org/elasticsearch/index/mapper/Mapper.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -144,8 +144,8 @@ ParserContext createMultiFieldContext(ParserContext in) {
144144
return new MultiFieldParserContext(in);
145145
}
146146

147-
ParserContext createDynamicTemplateFieldContext(ParserContext in) {
148-
return new DynamicTemplateParserContext(in);
147+
ParserContext createDynamicTemplateFieldContext() {
148+
return new DynamicTemplateParserContext(this);
149149
}
150150

151151
private static class MultiFieldParserContext extends ParserContext {

‎server/src/main/java/org/elasticsearch/index/mapper/ParseContext.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -165,8 +165,8 @@ public Iterable<Document> nonRootDocuments() {
165165
}
166166

167167
@Override
168-
public Mapper.TypeParser.ParserContext parserContext(DateFormatter dateFormatter) {
169-
return in.parserContext(dateFormatter);
168+
public Mapper.TypeParser.ParserContext dynamicTemplateParserContext(DateFormatter dateFormatter) {
169+
return in.dynamicTemplateParserContext(dateFormatter);
170170
}
171171

172172
@Override
@@ -342,8 +342,8 @@ public InternalParseContext(MappingLookup mappingLookup,
342342
}
343343

344344
@Override
345-
public Mapper.TypeParser.ParserContext parserContext(DateFormatter dateFormatter) {
346-
return parserContextFunction.apply(dateFormatter);
345+
public Mapper.TypeParser.ParserContext dynamicTemplateParserContext(DateFormatter dateFormatter) {
346+
return parserContextFunction.apply(dateFormatter).createDynamicTemplateFieldContext();
347347
}
348348

349349
@Override
@@ -563,7 +563,7 @@ public Collection<String> getFieldNames() {
563563
*/
564564
public abstract Collection<String> getFieldNames();
565565

566-
public abstract Mapper.TypeParser.ParserContext parserContext(DateFormatter dateFormatter);
566+
public abstract Mapper.TypeParser.ParserContext dynamicTemplateParserContext(DateFormatter dateFormatter);
567567

568568
/**
569569
* Return a new context that will be within a copy-to operation.

‎server/src/main/java/org/elasticsearch/index/mapper/RuntimeField.java‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@
88

99
package org.elasticsearch.index.mapper;
1010

11-
import org.elasticsearch.common.logging.DeprecationCategory;
12-
import org.elasticsearch.common.logging.DeprecationLogger;
1311
import org.elasticsearch.common.xcontent.ToXContent;
1412
import org.elasticsearch.common.xcontent.ToXContentFragment;
1513
import org.elasticsearch.common.xcontent.XContentBuilder;
@@ -66,8 +64,6 @@ abstract class Builder implements ToXContent {
6664
final String name;
6765
final Parameter<Map<String, String>> meta = Parameter.metaParam();
6866

69-
private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RuntimeField.class);
70-
7167
protected Builder(String name) {
7268
this.name = name;
7369
}
@@ -103,18 +99,6 @@ public final void parse(String name, Mapper.TypeParser.ParserContext parserConte
10399
final Object propNode = entry.getValue();
104100
Parameter<?> parameter = paramsMap.get(propName);
105101
if (parameter == null) {
106-
if (parserContext.isFromDynamicTemplate()) {
107-
// The parameter is unknown, but this mapping is from a dynamic template.
108-
// Until 7.x it was possible to use unknown parameters there, so for bwc we need to ignore it
109-
deprecationLogger.deprecate(DeprecationCategory.API, propName,
110-
"Parameter [{}] is used in a dynamic template mapping and has no effect on type [{}]. "
111-
+ "Usage will result in an error in future major versions and should be removed.",
112-
propName,
113-
type
114-
);
115-
iterator.remove();
116-
continue;
117-
}
118102
throw new MapperParsingException(
119103
"unknown parameter [" + propName + "] on runtime field [" + name + "] of type [" + type + "]"
120104
);

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
import org.apache.lucene.util.BytesRef;
1414
import org.elasticsearch.index.mapper.ParseContext.Document;
1515

16+
import java.io.IOException;
17+
1618
import static org.elasticsearch.test.StreamsUtils.copyToStringFromClasspath;
1719
import static org.hamcrest.Matchers.equalTo;
1820
import static org.hamcrest.Matchers.notNullValue;
@@ -159,4 +161,54 @@ public void testSimpleWithXContentTraverse() throws Exception {
159161
fieldMapper = mapperService.documentMapper().mappers().getMapper("multi2.org");
160162
assertNotNull(fieldMapper);
161163
}
164+
165+
public void testDynamicMapperWithBadMapping() throws IOException {
166+
DocumentMapper mapper = createDocumentMapper(topMapping(b -> {
167+
b.startArray("dynamic_templates");
168+
{
169+
b.startObject();
170+
{
171+
b.startObject("test");
172+
{
173+
b.field("match_mapping_type", "string");
174+
b.startObject("mapping").field("badparam", false).endObject();
175+
}
176+
b.endObject();
177+
}
178+
b.endObject();
179+
}
180+
b.endArray();
181+
}));
182+
assertWarnings(
183+
"dynamic template [test] has invalid content [{\"match_mapping_type\":\"string\",\"mapping\":{\"badparam\":false}}], " +
184+
"attempted to validate it with the following match_mapping_type: [string], caused by " +
185+
"[unknown parameter [badparam] on mapper [__dynamic__test] of type [null]]");
186+
187+
mapper.parse(source(b -> b.field("field", "foo")));
188+
assertWarnings("Parameter [badparam] is used in a dynamic template mapping and has no effect on type [null]. " +
189+
"Usage will result in an error in future major versions and should be removed.");
190+
}
191+
192+
public void testDynamicRuntimeWithBadMapping() throws IOException {
193+
createMapperService(topMapping(b -> {
194+
b.startArray("dynamic_templates");
195+
{
196+
b.startObject();
197+
{
198+
b.startObject("test");
199+
{
200+
b.field("match_mapping_type", "string");
201+
b.startObject("runtime").field("badparam", false).endObject();
202+
}
203+
b.endObject();
204+
}
205+
b.endObject();
206+
}
207+
b.endArray();
208+
}));
209+
assertWarnings("dynamic template [test] has invalid content " +
210+
"[{\"match_mapping_type\":\"string\",\"runtime\":{\"badparam\":false}}], " +
211+
"attempted to validate it with the following match_mapping_type: [string], " +
212+
"caused by [unknown parameter [badparam] on runtime field [__dynamic__test] of type [null]]");
213+
}
162214
}

‎server/src/test/java/org/elasticsearch/index/mapper/ParametrizedMapperTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ private static TestMapper fromMapping(String mapping, Version version, boolean f
210210
throw new UnsupportedOperationException();
211211
});
212212
if (fromDynamicTemplate) {
213-
pc = pc.createDynamicTemplateFieldContext(pc);
213+
pc = pc.createDynamicTemplateFieldContext();
214214
}
215215
return (TestMapper) new TypeParser()
216216
.parse("field", XContentHelper.convertToMap(JsonXContent.jsonXContent, mapping, true), pc)

0 commit comments

Comments
 (0)