Skip to content

Commit a18b1cc

Browse files
authored
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 a81fbb9 commit a18b1cc

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
@@ -653,6 +653,10 @@ public Parameter<T> acceptsNull() {
653653
return this;
654654
}
655655

656+
public boolean canAcceptNull() {
657+
return acceptsNull;
658+
}
659+
656660
/**
657661
* Adds a deprecated parameter name.
658662
*
@@ -757,7 +761,13 @@ private void init(FieldMapper toInit) {
757761
setValue(initializer.apply(toInit));
758762
}
759763

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

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

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

99
package org.elasticsearch.index.mapper;
1010

11+
import org.elasticsearch.Version;
12+
import org.elasticsearch.common.logging.DeprecationCategory;
13+
import org.elasticsearch.common.logging.DeprecationLogger;
14+
import org.elasticsearch.common.xcontent.ToXContent;
1115
import org.elasticsearch.common.xcontent.ToXContentFragment;
1216
import org.elasticsearch.common.xcontent.XContentBuilder;
17+
import org.elasticsearch.index.mapper.FieldMapper.Parameter;
1318

1419
import java.io.IOException;
1520
import java.util.Collection;
@@ -58,51 +63,69 @@ default XContentBuilder toXContent(XContentBuilder builder, Params params) throw
5863
*/
5964
Collection<MappedFieldType> asMappedFieldTypes();
6065

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();
66+
abstract class Builder implements ToXContent {
67+
final String name;
68+
final Parameter<Map<String, String>> meta = Parameter.metaParam();
69+
70+
private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RuntimeField.class);
7171

7272
protected Builder(String name) {
73-
super(name);
73+
this.name = name;
7474
}
7575

7676
public Map<String, String> meta() {
7777
return meta.getValue();
7878
}
7979

80-
@Override
81-
protected List<FieldMapper.Parameter<?>> getParameters() {
80+
protected List<Parameter<?>> getParameters() {
8281
return Collections.singletonList(meta);
8382
}
8483

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

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

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]");
95+
public final void parse(String name, Mapper.TypeParser.ParserContext parserContext, Map<String, Object> fieldNode) {
96+
Map<String, Parameter<?>> paramsMap = new HashMap<>();
97+
for (Parameter<?> param : getParameters()) {
98+
paramsMap.put(param.name, param);
10299
}
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]");
100+
String type = (String) fieldNode.remove("type");
101+
for (Iterator<Map.Entry<String, Object>> iterator = fieldNode.entrySet().iterator(); iterator.hasNext();) {
102+
Map.Entry<String, Object> entry = iterator.next();
103+
final String propName = entry.getKey();
104+
final Object propNode = entry.getValue();
105+
Parameter<?> parameter = paramsMap.get(propName);
106+
if (parameter == null) {
107+
if (parserContext.isFromDynamicTemplate() && parserContext.indexVersionCreated().before(Version.V_8_0_0)) {
108+
// The parameter is unknown, but this mapping is from a dynamic template.
109+
// Until 7.x it was possible to use unknown parameters there, so for bwc we need to ignore it
110+
deprecationLogger.deprecate(DeprecationCategory.API, propName,
111+
"Parameter [{}] is used in a dynamic template mapping and has no effect on type [{}]. "
112+
+ "Usage will result in an error in future major versions and should be removed.",
113+
propName,
114+
type
115+
);
116+
iterator.remove();
117+
continue;
118+
}
119+
throw new MapperParsingException(
120+
"unknown parameter [" + propName + "] on runtime field [" + name + "] of type [" + type + "]"
121+
);
122+
}
123+
if (propNode == null && parameter.canAcceptNull() == false) {
124+
throw new MapperParsingException("[" + propName + "] on runtime field [" + name
125+
+ "] of type [" + type + "] must not have a [null] value");
126+
}
127+
parameter.parse(name, parserContext, propNode);
128+
iterator.remove();
106129
}
107130
}
108131
}
@@ -123,7 +146,6 @@ RuntimeField parse(String name, Map<String, Object> node, Mapper.TypeParser.Pars
123146

124147
RuntimeField.Builder builder = builderFunction.apply(name);
125148
builder.parse(name, parserContext, node);
126-
builder.validate();
127149
return builder.createRuntimeField(parserContext);
128150
}
129151
}

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

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

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

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

121122
public void testStoredScriptsAreNotSupported() throws Exception {

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,7 @@ public void testIllegalDynamicTemplateUnknownAttributeRuntime() throws Exception
376376
assertEquals("Failed to parse mapping: dynamic template [my_template] has invalid content [" +
377377
"{\"match_mapping_type\":\"string\",\"runtime\":{\"foo\":\"bar\",\"type\":\"keyword\"}}], " +
378378
"attempted to validate it with the following match_mapping_type: [string]", e.getMessage());
379-
assertEquals("unknown parameter [foo] on mapper [__dynamic__my_template] of type [keyword]", e.getRootCause().getMessage());
379+
assertEquals("unknown parameter [foo] on runtime field [__dynamic__my_template] of type [keyword]", e.getRootCause().getMessage());
380380
}
381381

382382
public void testIllegalDynamicTemplateInvalidAttribute() throws Exception {
@@ -497,7 +497,7 @@ public void testIllegalDynamicTemplateNoMappingTypeRuntime() throws Exception {
497497
assertThat(e.getMessage(), containsString("Failed to parse mapping: dynamic template [my_template] has invalid content ["));
498498
assertThat(e.getMessage(), containsString("attempted to validate it with the following match_mapping_type: " +
499499
"[string, long, double, boolean, date]"));
500-
assertEquals("unknown parameter [foo] on mapper [__dynamic__my_template] of type [date]", e.getRootCause().getMessage());
500+
assertEquals("unknown parameter [foo] on runtime field [__dynamic__my_template] of type [date]", e.getRootCause().getMessage());
501501
}
502502

503503
public void testIllegalDynamicTemplate7DotXIndex() throws Exception {
@@ -680,7 +680,7 @@ public void testRuntimeSectionWrongFormat() throws IOException {
680680
public void testRuntimeSectionRemainingField() throws IOException {
681681
XContentBuilder mapping = runtimeFieldMapping(builder -> builder.field("type", "keyword").field("unsupported", "value"));
682682
MapperParsingException e = expectThrows(MapperParsingException.class, () -> createMapperService(mapping));
683-
assertEquals("Failed to parse mapping: unknown parameter [unsupported] on mapper [field] of type [keyword]", e.getMessage());
683+
assertEquals("Failed to parse mapping: unknown parameter [unsupported] on runtime field [field] of type [keyword]", e.getMessage());
684684
}
685685

686686
public void testTemplateWithoutMatchPredicates() throws Exception {

0 commit comments

Comments
 (0)