Skip to content

Commit 67a27e2

Browse files
romseygeekromseygeek
committed
Add declarative parameters to FieldMappers (#58663)
The FieldMapper infrastructure currently has a bunch of shared parameters, many of which are only applicable to a subset of the 41 mapper implementations we ship with. Merging, parsing and serialization of these parameters are spread around the class hierarchy, with much repetitive boilerplate code required. It would be much easier to reason about these things if we could declare the parameter set of each FieldMapper directly in the implementing class, and share the parsing, merging and serialization logic instead. This commit is a first effort at introducing a declarative parameter style. It adds a new FieldMapper subclass, ParametrizedFieldMapper, and refactors two mappers, Boolean and Binary, to use it. Parameters are declared on Builder classes, with the declaration including the parameter name, whether or not it is updateable, a default value, how to parse it from mappings, and how to extract it from another mapper at merge time. Builders have a getParameters method, which returns a list of the declared parameters; this is then used for parsing, merging and serialization. Merging is achieved by constructing a new Builder from the existing Mapper, and merging in values from the merging Mapper; conflicts are all caught at this point, and if none exist then a new, merged, Mapper can be built from the Builder. This allows all values on the Mapper to be final. Other mappers can be gradually migrated to this new style, and once they have all been refactored we can merge ParametrizedFieldMapper and FieldMapper entirely.
1 parent daa4832 commit 67a27e2

File tree

20 files changed

+795
-159
lines changed

20 files changed

+795
-159
lines changed

‎modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -159,10 +159,7 @@ static KeywordFieldMapper createExtractQueryFieldBuilder(String name, BuilderCon
159159
}
160160

161161
static BinaryFieldMapper createQueryBuilderFieldBuilder(BuilderContext context) {
162-
BinaryFieldMapper.Builder builder = new BinaryFieldMapper.Builder(QUERY_BUILDER_FIELD_NAME);
163-
builder.docValues(true);
164-
builder.indexOptions(IndexOptions.NONE);
165-
builder.store(false);
162+
BinaryFieldMapper.Builder builder = new BinaryFieldMapper.Builder(QUERY_BUILDER_FIELD_NAME, true);
166163
return builder.build(context);
167164
}
168165

‎server/src/main/java/org/elasticsearch/common/xcontent/support/XContentMapValues.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,16 @@ public static String nodeStringValue(Object node, String defaultValue) {
332332
return node.toString();
333333
}
334334

335+
/**
336+
* Returns the {@link Object#toString} value of its input, or {@code null} if the input is null
337+
*/
338+
public static String nodeStringValue(Object node) {
339+
if (node == null) {
340+
return null;
341+
}
342+
return node.toString();
343+
}
344+
335345
public static float nodeFloatValue(Object node, float defaultValue) {
336346
if (node == null) {
337347
return defaultValue;

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

Lines changed: 35 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,7 @@
2020
package org.elasticsearch.index.mapper;
2121

2222
import com.carrotsearch.hppc.ObjectArrayList;
23-
import org.apache.lucene.document.FieldType;
2423
import org.apache.lucene.document.StoredField;
25-
import org.apache.lucene.index.IndexOptions;
2624
import org.apache.lucene.index.Term;
2725
import org.apache.lucene.search.DocValuesFieldExistsQuery;
2826
import org.apache.lucene.search.Query;
@@ -43,47 +41,45 @@
4341

4442
import java.io.IOException;
4543
import java.time.ZoneId;
44+
import java.util.Arrays;
4645
import java.util.Base64;
4746
import java.util.Collections;
4847
import java.util.List;
4948
import java.util.Map;
5049

51-
import static org.elasticsearch.index.mapper.TypeParsers.parseField;
52-
53-
public class BinaryFieldMapper extends FieldMapper {
50+
public class BinaryFieldMapper extends ParametrizedFieldMapper {
5451

5552
public static final String CONTENT_TYPE = "binary";
5653

57-
public static class Defaults {
58-
public static final FieldType FIELD_TYPE = new FieldType();
59-
60-
static {
61-
FIELD_TYPE.setIndexOptions(IndexOptions.NONE);
62-
FIELD_TYPE.setOmitNorms(true);
63-
FIELD_TYPE.freeze();
64-
}
54+
private static BinaryFieldMapper toType(FieldMapper in) {
55+
return (BinaryFieldMapper) in;
6556
}
6657

67-
public static class Builder extends FieldMapper.Builder<Builder> {
58+
public static class Builder extends ParametrizedFieldMapper.Builder {
59+
60+
private final Parameter<Boolean> stored = Parameter.boolParam("store", false, m -> toType(m).stored, false);
61+
private final Parameter<Boolean> hasDocValues = Parameter.boolParam("doc_values", false, m -> toType(m).hasDocValues, false);
62+
private final Parameter<Map<String, String>> meta
63+
= new Parameter<>("meta", true, Collections.emptyMap(), TypeParsers::parseMeta, m -> m.fieldType().meta());
6864

6965
public Builder(String name) {
70-
super(name, Defaults.FIELD_TYPE);
71-
hasDocValues = false;
72-
builder = this;
66+
this(name, false);
67+
}
68+
69+
public Builder(String name, boolean hasDocValues) {
70+
super(name);
71+
this.hasDocValues.setValue(hasDocValues);
7372
}
7473

7574
@Override
76-
public BinaryFieldMapper build(BuilderContext context) {
77-
return new BinaryFieldMapper(name, fieldType, new BinaryFieldType(buildFullName(context), hasDocValues, meta),
78-
multiFieldsBuilder.build(this, context), copyTo);
75+
public List<Parameter<?>> getParameters() {
76+
return Arrays.asList(meta, stored, hasDocValues);
7977
}
8078

8179
@Override
82-
public Builder index(boolean index) {
83-
if (index) {
84-
throw new MapperParsingException("Binary field [" + name() + "] cannot be indexed");
85-
}
86-
return builder;
80+
public BinaryFieldMapper build(BuilderContext context) {
81+
return new BinaryFieldMapper(name, new BinaryFieldType(buildFullName(context), hasDocValues.getValue(), meta.getValue()),
82+
multiFieldsBuilder.build(this, context), copyTo.build(), this);
8783
}
8884
}
8985

@@ -92,7 +88,7 @@ public static class TypeParser implements Mapper.TypeParser {
9288
public BinaryFieldMapper.Builder parse(String name, Map<String, Object> node, ParserContext parserContext)
9389
throws MapperParsingException {
9490
BinaryFieldMapper.Builder builder = new BinaryFieldMapper.Builder(name);
95-
parseField(builder, name, node, parserContext);
91+
builder.parse(name, parserContext, node);
9692
return builder;
9793
}
9894
}
@@ -167,14 +163,19 @@ public Query termQuery(Object value, QueryShardContext context) {
167163
}
168164
}
169165

170-
protected BinaryFieldMapper(String simpleName, FieldType fieldType, MappedFieldType mappedFieldType,
171-
MultiFields multiFields, CopyTo copyTo) {
172-
super(simpleName, fieldType, mappedFieldType, multiFields, copyTo);
166+
private final boolean stored;
167+
private final boolean hasDocValues;
168+
169+
protected BinaryFieldMapper(String simpleName, MappedFieldType mappedFieldType,
170+
MultiFields multiFields, CopyTo copyTo, Builder builder) {
171+
super(simpleName, mappedFieldType, multiFields, copyTo);
172+
this.stored = builder.stored.getValue();
173+
this.hasDocValues = builder.hasDocValues.getValue();
173174
}
174175

175176
@Override
176177
protected void parseCreateField(ParseContext context) throws IOException {
177-
if (!fieldType.stored() && !fieldType().hasDocValues()) {
178+
if (stored == false && hasDocValues == false) {
178179
return;
179180
}
180181
byte[] value = context.parseExternalValue(byte[].class);
@@ -188,11 +189,11 @@ protected void parseCreateField(ParseContext context) throws IOException {
188189
if (value == null) {
189190
return;
190191
}
191-
if (fieldType.stored()) {
192+
if (stored) {
192193
context.doc().add(new StoredField(fieldType().name(), value));
193194
}
194195

195-
if (fieldType().hasDocValues()) {
196+
if (hasDocValues) {
196197
CustomBinaryDocValuesField field = (CustomBinaryDocValuesField) context.doc().getByKey(fieldType().name());
197198
if (field == null) {
198199
field = new CustomBinaryDocValuesField(fieldType().name(), value);
@@ -210,18 +211,8 @@ protected void parseCreateField(ParseContext context) throws IOException {
210211
}
211212

212213
@Override
213-
protected void mergeOptions(FieldMapper other, List<String> conflicts) {
214-
215-
}
216-
217-
@Override
218-
protected boolean docValuesByDefault() {
219-
return false;
220-
}
221-
222-
@Override
223-
protected boolean indexedByDefault() {
224-
return false;
214+
public ParametrizedFieldMapper.Builder getMergeBuilder() {
215+
return new BinaryFieldMapper.Builder(simpleName()).init(this);
225216
}
226217

227218
@Override

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

Lines changed: 46 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import org.apache.lucene.document.Field;
2323
import org.apache.lucene.document.FieldType;
2424
import org.apache.lucene.document.SortedNumericDocValuesField;
25+
import org.apache.lucene.document.StoredField;
2526
import org.apache.lucene.index.IndexOptions;
2627
import org.apache.lucene.index.Term;
2728
import org.apache.lucene.search.DocValuesFieldExistsQuery;
@@ -30,7 +31,6 @@
3031
import org.apache.lucene.search.TermRangeQuery;
3132
import org.apache.lucene.util.BytesRef;
3233
import org.elasticsearch.common.Nullable;
33-
import org.elasticsearch.common.xcontent.XContentBuilder;
3434
import org.elasticsearch.common.xcontent.XContentParser;
3535
import org.elasticsearch.common.xcontent.support.XContentMapValues;
3636
import org.elasticsearch.index.fielddata.IndexFieldData;
@@ -41,17 +41,15 @@
4141

4242
import java.io.IOException;
4343
import java.time.ZoneId;
44+
import java.util.Arrays;
4445
import java.util.Collections;
45-
import java.util.Iterator;
4646
import java.util.List;
4747
import java.util.Map;
4848

49-
import static org.elasticsearch.index.mapper.TypeParsers.parseField;
50-
5149
/**
5250
* A field mapper for boolean fields.
5351
*/
54-
public class BooleanFieldMapper extends FieldMapper {
52+
public class BooleanFieldMapper extends ParametrizedFieldMapper {
5553

5654
public static final String CONTENT_TYPE = "boolean";
5755

@@ -71,25 +69,37 @@ public static class Values {
7169
public static final BytesRef FALSE = new BytesRef("F");
7270
}
7371

74-
public static class Builder extends FieldMapper.Builder<Builder> {
72+
private static BooleanFieldMapper toType(FieldMapper in) {
73+
return (BooleanFieldMapper) in;
74+
}
75+
76+
public static class Builder extends ParametrizedFieldMapper.Builder {
77+
78+
private final Parameter<Boolean> docValues = Parameter.boolParam("doc_values", false, m -> toType(m).hasDocValues, true);
79+
private final Parameter<Boolean> indexed = Parameter.boolParam("index", false, m -> toType(m).indexed, true);
80+
private final Parameter<Boolean> stored = Parameter.boolParam("store", false, m -> toType(m).stored, false);
7581

76-
private Boolean nullValue;
82+
private final Parameter<Boolean> nullValue
83+
= new Parameter<>("null_value", false, null, (n, o) -> XContentMapValues.nodeBooleanValue(o), m -> toType(m).nullValue);
84+
85+
private final Parameter<Float> boost = Parameter.floatParam("boost", true, m -> m.fieldType().boost(), 1.0f);
86+
private final Parameter<Map<String, String>> meta
87+
= new Parameter<>("meta", true, Collections.emptyMap(), TypeParsers::parseMeta, m -> m.fieldType().meta());
7788

7889
public Builder(String name) {
79-
super(name, Defaults.FIELD_TYPE);
80-
this.builder = this;
90+
super(name);
8191
}
8292

83-
public Builder nullValue(Boolean nullValue) {
84-
this.nullValue = nullValue;
85-
return builder;
93+
@Override
94+
protected List<Parameter<?>> getParameters() {
95+
return Arrays.asList(meta, boost, docValues, indexed, nullValue, stored);
8696
}
8797

8898
@Override
8999
public BooleanFieldMapper build(BuilderContext context) {
90-
return new BooleanFieldMapper(name, fieldType,
91-
new BooleanFieldType(buildFullName(context), indexed, hasDocValues, meta),
92-
multiFieldsBuilder.build(this, context), copyTo, nullValue);
100+
MappedFieldType ft = new BooleanFieldType(buildFullName(context), indexed.getValue(), docValues.getValue(), meta.getValue());
101+
ft.setBoost(boost.getValue());
102+
return new BooleanFieldMapper(name, ft, multiFieldsBuilder.build(this, context), copyTo.build(), this);
93103
}
94104
}
95105

@@ -98,19 +108,7 @@ public static class TypeParser implements Mapper.TypeParser {
98108
public BooleanFieldMapper.Builder parse(String name, Map<String, Object> node, ParserContext parserContext)
99109
throws MapperParsingException {
100110
BooleanFieldMapper.Builder builder = new BooleanFieldMapper.Builder(name);
101-
parseField(builder, name, node, parserContext);
102-
for (Iterator<Map.Entry<String, Object>> iterator = node.entrySet().iterator(); iterator.hasNext();) {
103-
Map.Entry<String, Object> entry = iterator.next();
104-
String propName = entry.getKey();
105-
Object propNode = entry.getValue();
106-
if (propName.equals("null_value")) {
107-
if (propNode == null) {
108-
throw new MapperParsingException("Property [null_value] cannot be null.");
109-
}
110-
builder.nullValue(XContentMapValues.nodeBooleanValue(propNode, name + ".null_value"));
111-
iterator.remove();
112-
}
113-
}
111+
builder.parse(name, parserContext, node);
114112
return builder;
115113
}
116114
}
@@ -217,11 +215,17 @@ public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower
217215
}
218216

219217
private final Boolean nullValue;
220-
221-
protected BooleanFieldMapper(String simpleName, FieldType fieldType, MappedFieldType mappedFieldType,
222-
MultiFields multiFields, CopyTo copyTo, Boolean nullValue) {
223-
super(simpleName, fieldType, mappedFieldType, multiFields, copyTo);
224-
this.nullValue = nullValue;
218+
private final boolean indexed;
219+
private final boolean hasDocValues;
220+
private final boolean stored;
221+
222+
protected BooleanFieldMapper(String simpleName, MappedFieldType mappedFieldType,
223+
MultiFields multiFields, CopyTo copyTo, Builder builder) {
224+
super(simpleName, mappedFieldType, multiFields, copyTo);
225+
this.nullValue = builder.nullValue.getValue();
226+
this.stored = builder.stored.getValue();
227+
this.indexed = builder.indexed.getValue();
228+
this.hasDocValues = builder.docValues.getValue();
225229
}
226230

227231
@Override
@@ -231,7 +235,7 @@ public BooleanFieldType fieldType() {
231235

232236
@Override
233237
protected void parseCreateField(ParseContext context) throws IOException {
234-
if (fieldType().isSearchable() == false && !fieldType.stored() && !fieldType().hasDocValues()) {
238+
if (indexed == false && stored == false && hasDocValues == false) {
235239
return;
236240
}
237241

@@ -250,31 +254,27 @@ protected void parseCreateField(ParseContext context) throws IOException {
250254
if (value == null) {
251255
return;
252256
}
253-
if (fieldType().isSearchable() || fieldType.stored()) {
254-
context.doc().add(new Field(fieldType().name(), value ? "T" : "F", fieldType));
257+
if (indexed) {
258+
context.doc().add(new Field(fieldType().name(), value ? "T" : "F", Defaults.FIELD_TYPE));
259+
}
260+
if (stored) {
261+
context.doc().add(new StoredField(fieldType().name(), value ? "T" : "F"));
255262
}
256-
if (fieldType().hasDocValues()) {
263+
if (hasDocValues) {
257264
context.doc().add(new SortedNumericDocValuesField(fieldType().name(), value ? 1 : 0));
258265
} else {
259266
createFieldNamesField(context);
260267
}
261268
}
262269

263270
@Override
264-
protected void mergeOptions(FieldMapper other, List<String> conflicts) {
265-
// TODO ban updating null values
271+
public ParametrizedFieldMapper.Builder getMergeBuilder() {
272+
return new Builder(simpleName()).init(this);
266273
}
267274

268275
@Override
269276
protected String contentType() {
270277
return CONTENT_TYPE;
271278
}
272279

273-
@Override
274-
protected void doXContentBody(XContentBuilder builder, boolean includeDefaults, Params params) throws IOException {
275-
super.doXContentBody(builder, includeDefaults, params);
276-
if (includeDefaults || nullValue != null) {
277-
builder.field("null_value", nullValue);
278-
}
279-
}
280280
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ public Mapper.Builder<?> parse(String name, Map<String, Object> node, ParserCont
159159
} else if (Fields.CONTEXTS.match(fieldName, LoggingDeprecationHandler.INSTANCE)) {
160160
builder.contextMappings(ContextMappings.load(fieldNode, parserContext.indexVersionCreated()));
161161
iterator.remove();
162-
} else if (parseMultiField(builder, name, parserContext, fieldName, fieldNode)) {
162+
} else if (parseMultiField(builder::addMultiField, name, parserContext, fieldName, fieldNode)) {
163163
iterator.remove();
164164
}
165165
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,13 @@ public ContentPath(int offset) {
4545
this.index = 0;
4646
}
4747

48+
public ContentPath(String path) {
49+
this.sb = new StringBuilder();
50+
this.offset = 0;
51+
this.index = 0;
52+
add(path);
53+
}
54+
4855
public void add(String name) {
4956
path[index++] = name;
5057
if (index == path.length) { // expand if needed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,7 @@ public Mapper.Builder<?> parse(String name, Map<String, Object> node, ParserCont
292292
} else if (propName.equals("format")) {
293293
builder.format(propNode.toString());
294294
iterator.remove();
295-
} else if (TypeParsers.parseMultiField(builder, name, parserContext, propName, propNode)) {
295+
} else if (TypeParsers.parseMultiField(builder::addMultiField, name, parserContext, propName, propNode)) {
296296
iterator.remove();
297297
}
298298
}

0 commit comments

Comments
 (0)