Skip to content

Commit

Permalink
Add bwc for parsing mappings from dynamic templates (#67763) (#67971)
Browse files Browse the repository at this point in the history
During refactoring of the mapper parsing code, the part that checks for unknown
mapper parameters and throws an error when one is found was moved to
FieldMapper.Builder#parse which gets also executed when applying matching
dynamic templates. However, dynamic templates introduced during the 7.x versions
may still contain arbitrary parameters, although we have deprecation warnings
for that in place on latest 7.x now. When using these templates, indexing a new
document with a new field that triggers one of these mappings to be parsed can
create an error as demonstrated in #66765. Instead we need to be lenient in
these cases and simply ignore the unknown parameter while issuing a deprecation
warning here as well.

Closes #66765
  • Loading branch information
Christoph Büscher committed Jan 26, 2021
1 parent 93f6518 commit 0c30a7d
Show file tree
Hide file tree
Showing 5 changed files with 132 additions and 8 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

package org.elasticsearch.indices.mapping;

import org.elasticsearch.Version;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.test.ESIntegTestCase;
import org.elasticsearch.test.VersionUtils;

import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures;

public class MalformedDynamicTemplateIT extends ESIntegTestCase {

@Override
protected boolean forbidPrivateIndexSettings() {
return false;
}

/**
* Check that we can index a document into an 7.x index with a matching dynamic template that
* contains unknown parameters. We were able to create those templates in 7.x still, so we need
* to be able to index new documents into them. Indexing should issue a deprecation warning though.
*/
public void testBWCMalformedDynamicTemplate() {
String mapping = "{ \"dynamic_templates\": [\n"
+ " {\n"
+ " \"my_template\": {\n"
+ " \"mapping\": {\n"
+ " \"ignore_malformed\": true,\n" // this parameter is not supported by "keyword" field type
+ " \"type\": \"keyword\"\n"
+ " },\n"
+ " \"path_match\": \"*\"\n"
+ " }\n"
+ " }\n"
+ " ]\n"
+ " }\n"
+ "}}";
String indexName = "malformed_dynamic_template";
assertAcked(prepareCreate(indexName).setSettings(Settings.builder().put(indexSettings())
.put("number_of_shards", 1)
.put("index.version.created", VersionUtils.randomCompatibleVersion(random(), Version.CURRENT))
).addMapping("_doc", mapping, XContentType.JSON).get());
client().prepareIndex(indexName, "_doc").setSource("{\"foo\" : \"bar\"}", XContentType.JSON).get();
assertNoFailures((client().admin().indices().prepareRefresh(indexName)).get());
assertHitCount(client().prepareSearch(indexName).get(), 1);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ private static boolean applyMatchingTemplate(ParseContext context,
RuntimeFieldType runtimeFieldType = parser.parse(fullName, mapping, parserContext);
Runtime.createDynamicField(runtimeFieldType, context);
} else {
Mapper.Builder builder = parseMapping(name, mappingType, mapping, dateFormatter, context);
Mapper.Builder builder = parseDynamicTemplateMapping(name, mappingType, mapping, dateFormatter, context);
CONCRETE.createDynamicField(builder, context);
}
return true;
Expand All @@ -227,15 +227,16 @@ private static Mapper.Builder findTemplateBuilderForObject(ParseContext context,
String dynamicType = matchType.defaultMappingType();
String mappingType = dynamicTemplate.mappingType(dynamicType);
Map<String, Object> mapping = dynamicTemplate.mappingForName(name, dynamicType);
return parseMapping(name, mappingType, mapping, null, context);
return parseDynamicTemplateMapping(name, mappingType, mapping, null, context);
}

private static Mapper.Builder parseMapping(String name,
private static Mapper.Builder parseDynamicTemplateMapping(String name,
String mappingType,
Map<String, Object> mapping,
DateFormatter dateFormatter,
ParseContext context) {
Mapper.TypeParser.ParserContext parserContext = context.parserContext(dateFormatter);
parserContext = parserContext.createDynamicTemplateFieldContext(parserContext);
Mapper.TypeParser typeParser = parserContext.typeParser(mappingType);
if (typeParser == null) {
throw new MapperParsingException("failed to find type parsed [" + mappingType + "] for [" + name + "]");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -978,8 +978,21 @@ public final void parse(String name, ParserContext parserContext, Map<String, Ob
iterator.remove();
continue;
}
throw new MapperParsingException("unknown parameter [" + propName
+ "] on mapper [" + name + "] of type [" + type + "]");
if (parserContext.isFromDynamicTemplate()) {
// The parameter is unknown, but this mapping is from a dynamic template.
// Until 7.x it was possible to use unknown parameters there, so for bwc we need to ignore it
deprecationLogger.deprecate(propName,
"Parameter [{}] is used in a dynamic template mapping and has no effect on type [{}]. "
+ "Usage will result in an error in future major versions and should be removed.",
propName,
type
);
iterator.remove();
continue;
}
throw new MapperParsingException(
"unknown parameter [" + propName + "] on mapper [" + name + "] of type [" + type + "]"
);
}
if (Objects.equals("boost", propName)) {
deprecationLogger.deprecate(
Expand Down
24 changes: 22 additions & 2 deletions server/src/main/java/org/elasticsearch/index/mapper/Mapper.java
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,11 @@ public DateFormatter getDateFormatter() {

public boolean isWithinMultiField() { return false; }

/**
* true if this pars context is coming from parsing dynamic template mappings
*/
public boolean isFromDynamicTemplate() { return false; }

protected Function<String, SimilarityProvider> similarityLookupService() { return similarityLookupService; }

/**
Expand All @@ -153,11 +158,15 @@ public ScriptService scriptService() {
return scriptService;
}

public ParserContext createMultiFieldContext(ParserContext in) {
ParserContext createMultiFieldContext(ParserContext in) {
return new MultiFieldParserContext(in);
}

static class MultiFieldParserContext extends ParserContext {
ParserContext createDynamicTemplateFieldContext(ParserContext in) {
return new DynamicTemplateParserContext(in);
}

private static class MultiFieldParserContext extends ParserContext {
MultiFieldParserContext(ParserContext in) {
super(in.similarityLookupService, in.typeParsers, in.runtimeTypeParsers, in.indexVersionCreated,
in.queryShardContextSupplier, in.dateFormatter, in.scriptService, in.indexAnalyzers, in.indexSettings,
Expand All @@ -167,6 +176,17 @@ static class MultiFieldParserContext extends ParserContext {
@Override
public boolean isWithinMultiField() { return true; }
}

private static class DynamicTemplateParserContext extends ParserContext {
DynamicTemplateParserContext(ParserContext in) {
super(in.similarityLookupService, in.typeParsers, in.runtimeTypeParsers, in.indexVersionCreated,
in.queryShardContextSupplier, in.dateFormatter, in.scriptService, in.indexAnalyzers, in.indexSettings,
in.idFieldDataEnabled, in.supportsDynamicRuntimeMappings);
}

@Override
public boolean isFromDynamicTemplate() { return true; }
}
}

Mapper.Builder parse(String name, Map<String, Object> node, ParserContext parserContext) throws MapperParsingException;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import org.elasticsearch.index.mapper.FieldMapper.Parameter;
import org.elasticsearch.plugins.MapperPlugin;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.test.VersionUtils;

import java.io.IOException;
import java.util.Arrays;
Expand Down Expand Up @@ -197,7 +198,7 @@ protected String contentType() {
}
}

private static TestMapper fromMapping(String mapping, Version version) {
private static TestMapper fromMapping(String mapping, Version version, boolean fromDynamicTemplate) {
MapperService mapperService = mock(MapperService.class);
IndexAnalyzers indexAnalyzers = new IndexAnalyzers(
org.elasticsearch.common.collect.Map.of(
Expand All @@ -218,11 +219,18 @@ private static TestMapper fromMapping(String mapping, Version version) {
mapperService.getIndexAnalyzers(), mapperService.getIndexSettings(), () -> {
throw new UnsupportedOperationException();
}, false);
if (fromDynamicTemplate) {
pc = pc.createDynamicTemplateFieldContext(pc);
}
return (TestMapper) new TypeParser()
.parse("field", XContentHelper.convertToMap(JsonXContent.jsonXContent, mapping, true), pc)
.build(new ContentPath());
}

private static TestMapper fromMapping(String mapping, Version version) {
return fromMapping(mapping, version, false);
}

private static TestMapper fromMapping(String mapping) {
return fromMapping(mapping, Version.CURRENT);
}
Expand Down Expand Up @@ -381,6 +389,20 @@ public void testDeprecatedParameterName() {
assertEquals("{\"field\":{\"type\":\"test_mapper\",\"fixed2\":true,\"required\":\"value\"}}", Strings.toString(mapper));
}

/**
* test parsing mapping from dynamic templates, should ignore unknown parameters for bwc and log deprecation warning before 8.0.0
*/
public void testBWCunknownParametersfromDynamicTemplates() {
String mapping = "{\"type\":\"test_mapper\",\"some_unknown_parameter\":true,\"required\":\"value\"}";
TestMapper mapper = fromMapping(mapping, VersionUtils.randomCompatibleVersion(random(), Version.CURRENT), true);
assertNotNull(mapper);
assertWarnings(
"Parameter [some_unknown_parameter] is used in a dynamic template mapping and has no effect on type [test_mapper]. "
+ "Usage will result in an error in future major versions and should be removed."
);
assertEquals("{\"field\":{\"type\":\"test_mapper\",\"required\":\"value\"}}", Strings.toString(mapper));
}

public void testAnalyzers() {
String mapping = "{\"type\":\"test_mapper\",\"analyzer\":\"_standard\",\"required\":\"value\"}";
TestMapper mapper = fromMapping(mapping);
Expand Down

0 comments on commit 0c30a7d

Please sign in to comment.