Skip to content

Commit d4fceb5

Browse files
committed
RuntimeField.Builder should not extend FieldMapper.Builder (#73840)
RuntimeField.Builder currently extends FieldMapper.Builder so that it can share some parsing code and re-use the Parameter infrastructure. However, this also means that we have to have a number of no-op method implementations, and in addition this will make it complicated to add a fields parameter within multi-keyed object field types. This commit removes the class relationship between these two classes.
1 parent 84956bf commit d4fceb5

File tree

4 files changed

+69
-36
lines changed

4 files changed

+69
-36
lines changed

server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -654,6 +654,10 @@ public Parameter<T> acceptsNull() {
654654
return this;
655655
}
656656

657+
public boolean canAcceptNull() {
658+
return acceptsNull;
659+
}
660+
657661
/**
658662
* Adds a deprecated parameter name.
659663
*
@@ -758,7 +762,13 @@ private void init(FieldMapper toInit) {
758762
setValue(initializer.apply(toInit));
759763
}
760764

761-
private void parse(String field, ParserContext context, Object in) {
765+
/**
766+
* Parse the field value from an Object
767+
* @param field the field name
768+
* @param context the parser context
769+
* @param in the object
770+
*/
771+
public void parse(String field, ParserContext context, Object in) {
762772
setValue(parser.apply(field, context, in));
763773
}
764774

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

Lines changed: 51 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,12 @@
88

99
package org.elasticsearch.index.mapper;
1010

11+
import org.elasticsearch.common.logging.DeprecationCategory;
12+
import org.elasticsearch.common.logging.DeprecationLogger;
13+
import org.elasticsearch.common.xcontent.ToXContent;
1114
import org.elasticsearch.common.xcontent.ToXContentFragment;
1215
import org.elasticsearch.common.xcontent.XContentBuilder;
16+
import org.elasticsearch.index.mapper.FieldMapper.Parameter;
1317

1418
import java.io.IOException;
1519
import java.util.Collection;
@@ -58,51 +62,69 @@ default XContentBuilder toXContent(XContentBuilder builder, Params params) throw
5862
*/
5963
Collection<MappedFieldType> asMappedFieldTypes();
6064

61-
/**
62-
* For runtime fields the {@link RuntimeField.Parser} returns directly the {@link MappedFieldType}.
63-
* Internally we still create a {@link RuntimeField.Builder} so we reuse the {@link FieldMapper.Parameter} infrastructure,
64-
* but {@link RuntimeField.Builder#init(FieldMapper)} and {@link RuntimeField.Builder#build(ContentPath)} are never called as
65-
* {@link RuntimeField.Parser#parse(String, Map, Mapper.TypeParser.ParserContext)} calls
66-
* {@link RuntimeField.Builder#parse(String, Mapper.TypeParser.ParserContext, Map)} and returns the corresponding
67-
* {@link MappedFieldType}.
68-
*/
69-
abstract class Builder extends FieldMapper.Builder {
70-
final FieldMapper.Parameter<Map<String, String>> meta = FieldMapper.Parameter.metaParam();
65+
abstract class Builder implements ToXContent {
66+
final String name;
67+
final Parameter<Map<String, String>> meta = Parameter.metaParam();
68+
69+
private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RuntimeField.class);
7170

7271
protected Builder(String name) {
73-
super(name);
72+
this.name = name;
7473
}
7574

7675
public Map<String, String> meta() {
7776
return meta.getValue();
7877
}
7978

80-
@Override
81-
protected List<FieldMapper.Parameter<?>> getParameters() {
79+
protected List<Parameter<?>> getParameters() {
8280
return Collections.singletonList(meta);
8381
}
8482

85-
@Override
86-
public FieldMapper.Builder init(FieldMapper initializer) {
87-
throw new UnsupportedOperationException();
88-
}
83+
protected abstract RuntimeField createRuntimeField(Mapper.TypeParser.ParserContext parserContext);
8984

9085
@Override
91-
public final FieldMapper build(ContentPath context) {
92-
throw new UnsupportedOperationException();
86+
public final XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
87+
boolean includeDefaults = params.paramAsBoolean("include_defaults", false);
88+
for (Parameter<?> parameter : getParameters()) {
89+
parameter.toXContent(builder, includeDefaults);
90+
}
91+
return builder;
9392
}
9493

95-
protected abstract RuntimeField createRuntimeField(Mapper.TypeParser.ParserContext parserContext);
96-
97-
private void validate() {
98-
ContentPath contentPath = parentPath(name());
99-
FieldMapper.MultiFields multiFields = multiFieldsBuilder.build(this, contentPath);
100-
if (multiFields.iterator().hasNext()) {
101-
throw new IllegalArgumentException("runtime field [" + name + "] does not support [fields]");
94+
public final void parse(String name, Mapper.TypeParser.ParserContext parserContext, Map<String, Object> fieldNode) {
95+
Map<String, Parameter<?>> paramsMap = new HashMap<>();
96+
for (Parameter<?> param : getParameters()) {
97+
paramsMap.put(param.name, param);
10298
}
103-
FieldMapper.CopyTo copyTo = this.copyTo.build();
104-
if (copyTo.copyToFields().isEmpty() == false) {
105-
throw new IllegalArgumentException("runtime field [" + name + "] does not support [copy_to]");
99+
String type = (String) fieldNode.remove("type");
100+
for (Iterator<Map.Entry<String, Object>> iterator = fieldNode.entrySet().iterator(); iterator.hasNext();) {
101+
Map.Entry<String, Object> entry = iterator.next();
102+
final String propName = entry.getKey();
103+
final Object propNode = entry.getValue();
104+
Parameter<?> parameter = paramsMap.get(propName);
105+
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+
}
118+
throw new MapperParsingException(
119+
"unknown parameter [" + propName + "] on runtime field [" + name + "] of type [" + type + "]"
120+
);
121+
}
122+
if (propNode == null && parameter.canAcceptNull() == false) {
123+
throw new MapperParsingException("[" + propName + "] on runtime field [" + name
124+
+ "] of type [" + type + "] must not have a [null] value");
125+
}
126+
parameter.parse(name, parserContext, propNode);
127+
iterator.remove();
106128
}
107129
}
108130
}
@@ -123,7 +145,6 @@ RuntimeField parse(String name, Map<String, Object> node, Mapper.TypeParser.Pars
123145

124146
RuntimeField.Builder builder = builderFunction.apply(name);
125147
builder.parse(name, parserContext, node);
126-
builder.validate();
127148
return builder.createRuntimeField(parserContext);
128149
}
129150
}

server/src/test/java/org/elasticsearch/index/mapper/AbstractScriptFieldTypeTestCase.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import java.util.Collections;
3636
import java.util.function.BiConsumer;
3737

38+
import static org.hamcrest.Matchers.containsString;
3839
import static org.hamcrest.Matchers.equalTo;
3940
import static org.mockito.Matchers.anyString;
4041
import static org.mockito.Mockito.mock;
@@ -107,7 +108,7 @@ public void testCopyToIsNotSupported() throws IOException {
107108
b.field("copy_to", "target");
108109
});
109110
MapperParsingException exception = expectThrows(MapperParsingException.class, () -> createMapperService(mapping));
110-
assertEquals("Failed to parse mapping [_doc]: runtime field [field] does not support [copy_to]", exception.getMessage());
111+
assertThat(exception.getMessage(), containsString("unknown parameter [copy_to] on runtime field"));
111112
}
112113

113114
public void testMultiFieldsIsNotSupported() throws IOException {
@@ -116,7 +117,7 @@ public void testMultiFieldsIsNotSupported() throws IOException {
116117
b.startObject("fields").startObject("test").field("type", "keyword").endObject().endObject();
117118
});
118119
MapperParsingException exception = expectThrows(MapperParsingException.class, () -> createMapperService(mapping));
119-
assertEquals("Failed to parse mapping [_doc]: runtime field [field] does not support [fields]", exception.getMessage());
120+
assertThat(exception.getMessage(), containsString("unknown parameter [fields] on runtime field"));
120121
}
121122

122123
public void testStoredScriptsAreNotSupported() throws Exception {

server/src/test/java/org/elasticsearch/index/mapper/RootObjectMapperTests.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,7 @@ public void testIllegalDynamicTemplateUnknownAttributeRuntime() throws Exception
383383
assertWarnings("dynamic template [my_template] has invalid content [" +
384384
"{\"match_mapping_type\":\"string\",\"runtime\":{\"foo\":\"bar\",\"type\":\"keyword\"}}], " +
385385
"attempted to validate it with the following match_mapping_type: [string], " +
386-
"caused by [unknown parameter [foo] on mapper [__dynamic__my_template] of type [keyword]]");
386+
"caused by [unknown parameter [foo] on runtime field [__dynamic__my_template] of type [keyword]]");
387387
}
388388

389389
public void testIllegalDynamicTemplateInvalidAttribute() throws Exception {
@@ -523,7 +523,7 @@ public void testIllegalDynamicTemplateNoMappingTypeRuntime() throws Exception {
523523
String expected = "dynamic template [my_template] has invalid content [{" + matchError +
524524
",\"runtime\":{\"foo\":\"bar\",\"type\":\"{dynamic_type}\"}}], " +
525525
"attempted to validate it with the following match_mapping_type: [string, long, double, boolean, date], " +
526-
"caused by [unknown parameter [foo] on mapper [__dynamic__my_template] of type [date]]";
526+
"caused by [unknown parameter [foo] on runtime field [__dynamic__my_template] of type [date]]";
527527
assertWarnings(expected);
528528
}
529529

@@ -705,7 +705,8 @@ public void testRuntimeSectionWrongFormat() throws IOException {
705705
public void testRuntimeSectionRemainingField() throws IOException {
706706
XContentBuilder mapping = runtimeFieldMapping(builder -> builder.field("type", "keyword").field("unsupported", "value"));
707707
MapperParsingException e = expectThrows(MapperParsingException.class, () -> createMapperService(mapping));
708-
assertEquals("Failed to parse mapping [_doc]: unknown parameter [unsupported] on mapper [field] of type [keyword]", e.getMessage());
708+
assertEquals("Failed to parse mapping [_doc]: unknown parameter [unsupported] on runtime field [field] of type [keyword]",
709+
e.getMessage());
709710
}
710711

711712
public void testTemplateWithoutMatchPredicates() throws Exception {

0 commit comments

Comments
 (0)