Convert RangeFieldMapper to parametrized form (#62058)

This also adds the ability to define a serialization check on Parameters, used
in this case to only serialize format and locale parameters if the mapper is a
date range.
This commit is contained in:
Alan Woodward 2020-09-08 18:39:17 +01:00 committed by Alan Woodward
parent 5f05eef7e3
commit 28fd4a2ae8
8 changed files with 136 additions and 142 deletions

View File

@ -165,7 +165,7 @@ public class PercolatorFieldMapper extends FieldMapper {
}
static RangeFieldMapper createExtractedRangeFieldBuilder(String name, RangeType rangeType, BuilderContext context) {
RangeFieldMapper.Builder builder = new RangeFieldMapper.Builder(name, rangeType);
RangeFieldMapper.Builder builder = new RangeFieldMapper.Builder(name, rangeType, true);
// For now no doc values, because in processQuery(...) only the Lucene range fields get added:
builder.docValues(false);
return builder.build(context);

View File

@ -43,6 +43,7 @@ import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.function.BiFunction;
import java.util.function.BooleanSupplier;
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.function.Supplier;
@ -146,6 +147,7 @@ public abstract class ParametrizedFieldMapper extends FieldMapper {
private boolean acceptsNull = false;
private Consumer<T> validator = null;
private Serializer<T> serializer = XContentBuilder::field;
private BooleanSupplier serializerPredicate = () -> true;
private Function<T, String> conflictSerializer = Object::toString;
private T value;
private boolean isSet;
@ -230,6 +232,15 @@ public abstract class ParametrizedFieldMapper extends FieldMapper {
return this;
}
/**
* Sets an additional check on whether or not this parameter should be serialized,
* after the existing 'set' and 'include_defaults' checks.
*/
public Parameter<T> setShouldSerialize(BooleanSupplier shouldSerialize) {
this.serializerPredicate = shouldSerialize;
return this;
}
private void validate() {
if (validator != null) {
validator.accept(getValue());
@ -255,7 +266,7 @@ public abstract class ParametrizedFieldMapper extends FieldMapper {
}
private void toXContent(XContentBuilder builder, boolean includeDefaults) throws IOException {
if (includeDefaults || isConfigured()) {
if ((includeDefaults || isConfigured()) && serializerPredicate.getAsBoolean()) {
serializer.serialize(builder, name, getValue());
}
}

View File

@ -19,8 +19,6 @@
package org.elasticsearch.index.mapper;
import org.apache.lucene.document.FieldType;
import org.apache.lucene.index.IndexOptions;
import org.apache.lucene.index.Term;
import org.apache.lucene.search.BoostQuery;
import org.apache.lucene.search.DocValuesFieldExistsQuery;
@ -35,12 +33,11 @@ import org.elasticsearch.common.joda.Joda;
import org.elasticsearch.common.lucene.Lucene;
import org.elasticsearch.common.network.InetAddresses;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.time.DateFormatter;
import org.elasticsearch.common.time.DateMathParser;
import org.elasticsearch.common.util.LocaleUtils;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.support.XContentMapValues;
import org.elasticsearch.index.fielddata.IndexFieldData;
import org.elasticsearch.index.fielddata.plain.BinaryIndexFieldData;
import org.elasticsearch.index.query.QueryShardContext;
@ -53,10 +50,10 @@ import java.net.InetAddress;
import java.net.UnknownHostException;
import java.time.ZoneId;
import java.time.ZoneOffset;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Locale;
import java.util.Map;
@ -70,18 +67,12 @@ import static org.elasticsearch.index.query.RangeQueryBuilder.LTE_FIELD;
import static org.elasticsearch.index.query.RangeQueryBuilder.LT_FIELD;
/** A {@link FieldMapper} for indexing numeric and date ranges, and creating queries */
public class RangeFieldMapper extends FieldMapper {
public class RangeFieldMapper extends ParametrizedFieldMapper {
public static final boolean DEFAULT_INCLUDE_UPPER = true;
public static final boolean DEFAULT_INCLUDE_LOWER = true;
public static class Defaults {
public static final Explicit<Boolean> COERCE = new Explicit<>(true, false);
public static final FieldType FIELD_TYPE = new FieldType();
static {
FIELD_TYPE.setStored(false);
FIELD_TYPE.setIndexOptions(IndexOptions.DOCS);
FIELD_TYPE.freeze();
}
public static final DateFormatter DATE_FORMATTER = DateFieldMapper.DEFAULT_DATE_TIME_FORMATTER;
}
@ -89,104 +80,80 @@ public class RangeFieldMapper extends FieldMapper {
static final Setting<Boolean> COERCE_SETTING =
Setting.boolSetting("index.mapping.coerce", true, Setting.Property.IndexScope);
public static class Builder extends FieldMapper.Builder<Builder> {
private Boolean coerce;
private Locale locale = Locale.ROOT;
private String pattern;
private static RangeFieldMapper toType(FieldMapper in) {
return (RangeFieldMapper) in;
}
public static class Builder extends ParametrizedFieldMapper.Builder {
private final Parameter<Boolean> index = Parameter.indexParam(m -> toType(m).index, true);
private final Parameter<Boolean> hasDocValues = Parameter.docValuesParam(m -> toType(m).hasDocValues, true);
private final Parameter<Boolean> store = Parameter.storeParam(m -> toType(m).store, false);
private final Parameter<Explicit<Boolean>> coerce;
private final Parameter<String> format
= Parameter.stringParam("format", false, m -> toType(m).format, Defaults.DATE_FORMATTER.pattern());
private final Parameter<Locale> locale = new Parameter<>("locale", false, () -> Locale.ROOT,
(n, c, o) -> LocaleUtils.parse(o.toString()), m -> toType(m).locale);
private final Parameter<Float> boost = Parameter.boostParam();
private final Parameter<Map<String, String>> meta = Parameter.metaParam();
private final RangeType type;
public Builder(String name, RangeType type) {
super(name, Defaults.FIELD_TYPE);
public Builder(String name, RangeType type, Settings settings) {
this(name, type, COERCE_SETTING.get(settings));
}
public Builder(String name, RangeType type, boolean coerceByDefault) {
super(name);
this.type = type;
builder = this;
}
public Builder coerce(boolean coerce) {
this.coerce = coerce;
return builder;
}
protected Explicit<Boolean> coerce(BuilderContext context) {
if (coerce != null) {
return new Explicit<>(coerce, true);
this.coerce = Parameter.explicitBoolParam("coerce", true, m -> toType(m).coerce, coerceByDefault);
if (this.type != RangeType.DATE) {
format.setShouldSerialize(() -> false);
locale.setShouldSerialize(() -> false);
}
if (context.indexSettings() != null) {
return new Explicit<>(COERCE_SETTING.get(context.indexSettings()), false);
}
return Defaults.COERCE;
}
public Builder format(String format) {
this.pattern = format;
public void docValues(boolean hasDocValues) {
this.hasDocValues.setValue(hasDocValues);
}
Builder format(String format) {
this.format.setValue(format);
return this;
}
public void locale(Locale locale) {
this.locale = locale;
@Override
protected List<Parameter<?>> getParameters() {
return Arrays.asList(index, hasDocValues, store, coerce, format, locale, boost, meta);
}
protected RangeFieldType setupFieldType(BuilderContext context) {
if (pattern != null) {
if (format.isConfigured()) {
if (type != RangeType.DATE) {
throw new IllegalArgumentException("field [" + name() + "] of type [range]"
+ " should not define a dateTimeFormatter unless it is a " + RangeType.DATE + " type");
}
DateFormatter dateTimeFormatter;
if (Joda.isJodaPattern(context.indexCreatedVersion(), pattern)) {
dateTimeFormatter = Joda.forPattern(pattern).withLocale(locale);
if (Joda.isJodaPattern(context.indexCreatedVersion(), format.getValue())) {
dateTimeFormatter = Joda.forPattern(format.getValue()).withLocale(locale.getValue());
} else {
dateTimeFormatter = DateFormatter.forPattern(pattern).withLocale(locale);
dateTimeFormatter = DateFormatter.forPattern(format.getValue()).withLocale(locale.getValue());
}
return new RangeFieldType(buildFullName(context), indexed, hasDocValues,
dateTimeFormatter, meta);
return new RangeFieldType(buildFullName(context), index.getValue(), hasDocValues.getValue(),
dateTimeFormatter, meta.getValue());
}
if (type == RangeType.DATE) {
return new RangeFieldType(buildFullName(context), indexed, hasDocValues, Defaults.DATE_FORMATTER, meta);
return new RangeFieldType(buildFullName(context), index.getValue(), hasDocValues.getValue(),
Defaults.DATE_FORMATTER, meta.getValue());
}
return new RangeFieldType(buildFullName(context), type, indexed, hasDocValues, meta);
return new RangeFieldType(buildFullName(context), type, index.getValue(), hasDocValues.getValue(), meta.getValue());
}
@Override
public RangeFieldMapper build(BuilderContext context) {
setupFieldType(context);
return new RangeFieldMapper(name, fieldType, setupFieldType(context), coerce(context),
multiFieldsBuilder.build(this, context), copyTo);
}
}
public static class TypeParser implements Mapper.TypeParser {
final RangeType type;
public TypeParser(RangeType type) {
this.type = type;
}
@Override
public Mapper.Builder<?> parse(String name, Map<String, Object> node,
ParserContext parserContext) throws MapperParsingException {
Builder builder = new Builder(name, type);
TypeParsers.parseField(builder, name, node, parserContext);
for (Iterator<Map.Entry<String, Object>> iterator = node.entrySet().iterator(); iterator.hasNext();) {
Map.Entry<String, Object> entry = iterator.next();
String propName = entry.getKey();
Object propNode = entry.getValue();
if (propName.equals("null_value")) {
throw new MapperParsingException("Property [null_value] is not supported for [" + this.type.name
+ "] field types.");
} else if (propName.equals("coerce")) {
builder.coerce(XContentMapValues.nodeBooleanValue(propNode, name + ".coerce"));
iterator.remove();
} else if (propName.equals("locale")) {
builder.locale(LocaleUtils.parse(propNode.toString()));
iterator.remove();
} else if (propName.equals("format")) {
builder.format(propNode.toString());
iterator.remove();
} else if (TypeParsers.parseMultiField(builder::addMultiField, name, parserContext, propName, propNode)) {
iterator.remove();
}
}
return builder;
RangeFieldType ft = setupFieldType(context);
ft.setBoost(boost.getValue());
return new RangeFieldMapper(name, ft, multiFieldsBuilder.build(this, context), copyTo.build(), type, this);
}
}
@ -288,17 +255,37 @@ public class RangeFieldMapper extends FieldMapper {
}
}
private Explicit<Boolean> coerce;
private final RangeType type;
private final boolean index;
private final boolean hasDocValues;
private final boolean store;
private final Explicit<Boolean> coerce;
private final String format;
private final Locale locale;
private final boolean coerceByDefault;
private RangeFieldMapper(
String simpleName,
FieldType fieldType,
MappedFieldType mappedFieldType,
Explicit<Boolean> coerce,
MultiFields multiFields,
CopyTo copyTo) {
super(simpleName, fieldType, mappedFieldType, multiFields, copyTo);
this.coerce = coerce;
CopyTo copyTo,
RangeType type,
Builder builder) {
super(simpleName, mappedFieldType, multiFields, copyTo);
this.type = type;
this.index = builder.index.getValue();
this.hasDocValues = builder.hasDocValues.getValue();
this.store = builder.store.getValue();
this.coerce = builder.coerce.getValue();
this.format = builder.format.getValue();
this.locale = builder.locale.getValue();
this.coerceByDefault = builder.coerce.getDefaultValue().value();
}
@Override
public ParametrizedFieldMapper.Builder getMergeBuilder() {
return new Builder(simpleName(), type, coerceByDefault).init(this);
}
@Override
@ -373,12 +360,9 @@ public class RangeFieldMapper extends FieldMapper {
+ name() + "], expected an object but got " + parser.currentName());
}
}
boolean docValued = fieldType().hasDocValues();
boolean indexed = fieldType().isSearchable();
boolean stored = fieldType.stored();
context.doc().addAll(fieldType().rangeType.createFields(context, name(), range, indexed, docValued, stored));
context.doc().addAll(fieldType().rangeType.createFields(context, name(), range, index, hasDocValues, store));
if (docValued == false && (indexed || stored)) {
if (hasDocValues == false && (index || store)) {
createFieldNamesField(context);
}
}
@ -414,33 +398,6 @@ public class RangeFieldMapper extends FieldMapper {
};
}
@Override
protected void mergeOptions(FieldMapper other, List<String> conflicts) {
RangeFieldMapper mergeWith = (RangeFieldMapper) other;
if (mergeWith.coerce.explicit()) {
this.coerce = mergeWith.coerce;
}
}
@Override
protected void doXContentBody(XContentBuilder builder, boolean includeDefaults, Params params) throws IOException {
super.doXContentBody(builder, includeDefaults, params);
if (fieldType().rangeType == RangeType.DATE
&& (includeDefaults || (fieldType().dateTimeFormatter() != null
&& fieldType().dateTimeFormatter().pattern().equals(DateFieldMapper.DEFAULT_DATE_TIME_FORMATTER.pattern()) == false))) {
builder.field("format", fieldType().dateTimeFormatter().pattern());
}
if (fieldType().rangeType == RangeType.DATE
&& (includeDefaults || (fieldType().dateTimeFormatter() != null
&& fieldType().dateTimeFormatter().locale() != Locale.ROOT))) {
builder.field("locale", fieldType().dateTimeFormatter().locale());
}
if (includeDefaults || coerce.explicit()) {
builder.field("coerce", coerce.value());
}
}
private static Range parseIpRangeFromCidr(final XContentParser parser) throws IOException {
final Tuple<InetAddress, Integer> cidr = InetAddresses.parseCidr(parser.text());
// create the lower value by zeroing out the host portion, upper value by filling it with all ones.
@ -463,8 +420,8 @@ public class RangeFieldMapper extends FieldMapper {
RangeType type;
Object from;
Object to;
private boolean includeFrom;
private boolean includeTo;
private final boolean includeFrom;
private final boolean includeTo;
public Range(RangeType type, Object from, Object to, boolean includeFrom, boolean includeTo) {
this.type = type;

View File

@ -706,6 +706,10 @@ public enum RangeType {
public abstract Query dvRangeQuery(String field, BinaryDocValuesRangeQuery.QueryType queryType, Object from, Object to,
boolean includeFrom, boolean includeTo);
public final Mapper.TypeParser parser() {
return new ParametrizedFieldMapper.TypeParser((n, c) -> new RangeFieldMapper.Builder(n, this, c.getSettings()));
}
public final String name;
private final NumberFieldMapper.NumberType numberType;
public final LengthType lengthType;

View File

@ -45,7 +45,6 @@ import org.elasticsearch.index.mapper.Mapper;
import org.elasticsearch.index.mapper.MetadataFieldMapper;
import org.elasticsearch.index.mapper.NumberFieldMapper;
import org.elasticsearch.index.mapper.ObjectMapper;
import org.elasticsearch.index.mapper.RangeFieldMapper;
import org.elasticsearch.index.mapper.RangeType;
import org.elasticsearch.index.mapper.RoutingFieldMapper;
import org.elasticsearch.index.mapper.SeqNoFieldMapper;
@ -117,7 +116,7 @@ public class IndicesModule extends AbstractModule {
mappers.put(type.typeName(), type.parser());
}
for (RangeType type : RangeType.values()) {
mappers.put(type.typeName(), new RangeFieldMapper.TypeParser(type));
mappers.put(type.typeName(), type.parser());
}
mappers.put(BooleanFieldMapper.CONTENT_TYPE, BooleanFieldMapper.PARSER);
mappers.put(BinaryFieldMapper.CONTENT_TYPE, BinaryFieldMapper.PARSER);

View File

@ -90,7 +90,7 @@ public class IpRangeFieldMapperTests extends ESSingleNodeTestCase {
Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT.id).build();
Mapper.BuilderContext context = new Mapper.BuilderContext(settings, new ContentPath());
RangeFieldMapper mapper = new RangeFieldMapper.Builder("field", RangeType.IP).build(context);
RangeFieldMapper mapper = new RangeFieldMapper.Builder("field", RangeType.IP, true).build(context);
Map<String, Object> range = org.elasticsearch.common.collect.Map.of("gte", "2001:db8:0:0:0:0:2:1");
assertEquals(List.of(org.elasticsearch.common.collect.Map.of("gte", "2001:db8::2:1")), fetchSourceValue(mapper, range));
assertEquals(List.of("2001:db8::2:1/32"), fetchSourceValue(mapper, "2001:db8:0:0:0:0:2:1/32"));

View File

@ -127,6 +127,9 @@ public class ParametrizedMapperTests extends ESSingleNodeTestCase {
protected Builder(String name) {
super(name);
// only output search analyzer if different to analyzer
searchAnalyzer.setShouldSerialize(
() -> Objects.equals(analyzer.getValue().name(), searchAnalyzer.getValue().name()) == false);
}
@Override
@ -227,6 +230,16 @@ public class ParametrizedMapperTests extends ESSingleNodeTestCase {
return fromMapping(mapping, Version.CURRENT);
}
private String toStringWithDefaults(ToXContent value) throws IOException {
XContentBuilder builder = JsonXContent.contentBuilder();
ToXContent.Params params = new ToXContent.MapParams(Collections.singletonMap("include_defaults", "true"));
builder.startObject();
value.toXContent(builder, params);
builder.endObject();
return Strings.toString(builder);
}
// defaults - create empty builder config, and serialize with and without defaults
public void testDefaults() throws IOException {
String mapping = "{\"type\":\"test_mapper\",\"required\":\"value\"}";
@ -236,17 +249,11 @@ public class ParametrizedMapperTests extends ESSingleNodeTestCase {
assertEquals("default", mapper.variable);
assertEquals("{\"field\":" + mapping + "}", Strings.toString(mapper));
XContentBuilder builder = JsonXContent.contentBuilder();
ToXContent.Params params = new ToXContent.MapParams(org.elasticsearch.common.collect.Map.of("include_defaults", "true"));
builder.startObject();
mapper.toXContent(builder, params);
builder.endObject();
assertEquals("{\"field\":{\"type\":\"test_mapper\",\"fixed\":true," +
"\"fixed2\":false,\"variable\":\"default\",\"index\":true," +
"\"wrapper\":\"default\",\"int_value\":5,\"analyzer\":\"_keyword\"," +
"\"search_analyzer\":\"_keyword\",\"required\":\"value\",\"restricted\":\"foo\"}}",
Strings.toString(builder));
"\"required\":\"value\",\"restricted\":\"foo\"}}",
toStringWithDefaults(mapper));
}
// merging - try updating 'fixed' and 'fixed2' should get an error, try updating 'variable' and verify update
@ -410,7 +417,7 @@ public class ParametrizedMapperTests extends ESSingleNodeTestCase {
assertEquals("{\"field\":{\"type\":\"test_mapper\",\"index\":false,\"required\":\"value\"}}", Strings.toString(mapper));
}
public void testLinkedAnalyzers() {
public void testLinkedAnalyzers() throws IOException {
String mapping = "{\"type\":\"test_mapper\",\"analyzer\":\"_standard\",\"required\":\"value\"}";
TestMapper mapper = fromMapping(mapping);
assertEquals("_standard", mapper.analyzer.name());
@ -429,6 +436,21 @@ public class ParametrizedMapperTests extends ESSingleNodeTestCase {
assertEquals("default", mapper.analyzer.name());
assertEquals("_standard", mapper.searchAnalyzer.name());
assertEquals("{\"field\":" + mappingWithBoth + "}", Strings.toString(mapper));
// we've configured things so that search_analyzer is only output when different from
// analyzer, no matter what the value of `include_defaults` is
String mappingWithSame = "{\"type\":\"test_mapper\",\"analyzer\":\"default\"," +
"\"search_analyzer\":\"default\",\"required\":\"value\"}";
mapper = fromMapping(mappingWithSame);
assertEquals("default", mapper.analyzer.name());
assertEquals("default", mapper.searchAnalyzer.name());
assertEquals("{\"field\":{\"type\":\"test_mapper\",\"analyzer\":\"default\",\"required\":\"value\"}}", Strings.toString(mapper));
assertEquals("{\"field\":{\"type\":\"test_mapper\",\"fixed\":true," +
"\"fixed2\":false,\"variable\":\"default\",\"index\":true," +
"\"wrapper\":\"default\",\"int_value\":5,\"analyzer\":\"default\"," +
"\"required\":\"value\",\"restricted\":\"foo\"}}",
toStringWithDefaults(mapper));
}
public void testRequiredField() {
@ -461,4 +483,5 @@ public class ParametrizedMapperTests extends ESSingleNodeTestCase {
assertEquals("foo", mapper.restricted);
}
}
}

View File

@ -343,12 +343,12 @@ public class RangeFieldMapperTests extends AbstractNumericFieldMapperTestCase {
Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT.id).build();
Mapper.BuilderContext context = new Mapper.BuilderContext(settings, new ContentPath());
RangeFieldMapper longMapper = new RangeFieldMapper.Builder("field", RangeType.LONG).build(context);
RangeFieldMapper longMapper = new RangeFieldMapper.Builder("field", RangeType.LONG, true).build(context);
Map<String, Object> longRange = org.elasticsearch.common.collect.Map.of("gte", 3.14, "lt", "42.9");
assertEquals(List.of(org.elasticsearch.common.collect.Map.of("gte", 3L, "lt", 42L)),
fetchSourceValue(longMapper, longRange));
RangeFieldMapper dateMapper = new RangeFieldMapper.Builder("field", RangeType.DATE)
RangeFieldMapper dateMapper = new RangeFieldMapper.Builder("field", RangeType.DATE, true)
.format("yyyy/MM/dd||epoch_millis")
.build(context);
Map<String, Object> dateRange = org.elasticsearch.common.collect.Map.of("lt", "1990/12/29", "gte", 597429487111L);
@ -360,12 +360,12 @@ public class RangeFieldMapperTests extends AbstractNumericFieldMapperTestCase {
Settings settings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT.id).build();
Mapper.BuilderContext context = new Mapper.BuilderContext(settings, new ContentPath());
RangeFieldMapper longMapper = new RangeFieldMapper.Builder("field", RangeType.LONG).build(context);
RangeFieldMapper longMapper = new RangeFieldMapper.Builder("field", RangeType.LONG, true).build(context);
Map<String, Object> longRange = org.elasticsearch.common.collect.Map.of("gte", 3.14, "lt", "42.9");
assertEquals(List.of(org.elasticsearch.common.collect.Map.of("gte", 3L, "lt", 42L)),
fetchSourceValue(longMapper, longRange));
RangeFieldMapper dateMapper = new RangeFieldMapper.Builder("field", RangeType.DATE)
RangeFieldMapper dateMapper = new RangeFieldMapper.Builder("field", RangeType.DATE, true)
.format("strict_date_time")
.build(context);
Map<String, Object> dateRange = org.elasticsearch.common.collect.Map.of("lt", "1990-12-29T00:00:00.000Z");