From 219b7dbd12fbd9e7438ca86d6ea7cd0bfdba866b Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Thu, 9 Jul 2020 10:56:25 +0100 Subject: [PATCH] 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. --- .../percolator/PercolatorFieldMapper.java | 5 +- .../xcontent/support/XContentMapValues.java | 10 + .../index/mapper/BinaryFieldMapper.java | 78 ++-- .../index/mapper/BooleanFieldMapper.java | 91 +++-- .../index/mapper/CompletionFieldMapper.java | 2 +- .../index/mapper/ContentPath.java | 7 + .../index/mapper/DateFieldMapper.java | 2 +- .../index/mapper/FieldMapper.java | 34 +- .../index/mapper/IpFieldMapper.java | 2 +- .../index/mapper/ParametrizedFieldMapper.java | 345 ++++++++++++++++++ .../index/mapper/RangeFieldMapper.java | 2 +- .../index/mapper/TypeParsers.java | 42 ++- .../fielddata/AbstractFieldDataTestCase.java | 2 +- .../index/mapper/BinaryFieldMapperTests.java | 14 +- .../index/mapper/BooleanFieldMapperTests.java | 23 +- .../index/mapper/NullValueTests.java | 20 +- .../index/mapper/ParametrizedMapperTests.java | 250 +++++++++++++ .../index/mapper/RootObjectMapperTests.java | 4 +- .../mapper/HistogramFieldMapper.java | 5 +- .../mapper/ConstantKeywordFieldMapper.java | 8 +- 20 files changed, 787 insertions(+), 159 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/index/mapper/ParametrizedFieldMapper.java create mode 100644 server/src/test/java/org/elasticsearch/index/mapper/ParametrizedMapperTests.java diff --git a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java index 13836c993f464..638ce7efd7a3d 100644 --- a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java +++ b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java @@ -155,10 +155,7 @@ static KeywordFieldMapper createExtractQueryFieldBuilder(String name, BuilderCon } static BinaryFieldMapper createQueryBuilderFieldBuilder(BuilderContext context) { - BinaryFieldMapper.Builder builder = new BinaryFieldMapper.Builder(QUERY_BUILDER_FIELD_NAME); - builder.docValues(true); - builder.indexOptions(IndexOptions.NONE); - builder.store(false); + BinaryFieldMapper.Builder builder = new BinaryFieldMapper.Builder(QUERY_BUILDER_FIELD_NAME, true); return builder.build(context); } diff --git a/server/src/main/java/org/elasticsearch/common/xcontent/support/XContentMapValues.java b/server/src/main/java/org/elasticsearch/common/xcontent/support/XContentMapValues.java index 05f8cb3a2fc45..dfbb507365fa9 100644 --- a/server/src/main/java/org/elasticsearch/common/xcontent/support/XContentMapValues.java +++ b/server/src/main/java/org/elasticsearch/common/xcontent/support/XContentMapValues.java @@ -332,6 +332,16 @@ public static String nodeStringValue(Object node, String defaultValue) { return node.toString(); } + /** + * Returns the {@link Object#toString} value of its input, or {@code null} if the input is null + */ + public static String nodeStringValue(Object node) { + if (node == null) { + return null; + } + return node.toString(); + } + public static float nodeFloatValue(Object node, float defaultValue) { if (node == null) { return defaultValue; diff --git a/server/src/main/java/org/elasticsearch/index/mapper/BinaryFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/BinaryFieldMapper.java index 503703ec3f863..de9b80031d266 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/BinaryFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/BinaryFieldMapper.java @@ -20,9 +20,7 @@ package org.elasticsearch.index.mapper; import com.carrotsearch.hppc.ObjectArrayList; -import org.apache.lucene.document.FieldType; import org.apache.lucene.document.StoredField; -import org.apache.lucene.index.IndexOptions; import org.apache.lucene.index.Term; import org.apache.lucene.search.DocValuesFieldExistsQuery; import org.apache.lucene.search.Query; @@ -48,42 +46,39 @@ import java.util.List; import java.util.Map; -import static org.elasticsearch.index.mapper.TypeParsers.parseField; - -public class BinaryFieldMapper extends FieldMapper { +public class BinaryFieldMapper extends ParametrizedFieldMapper { public static final String CONTENT_TYPE = "binary"; - public static class Defaults { - public static final FieldType FIELD_TYPE = new FieldType(); - - static { - FIELD_TYPE.setIndexOptions(IndexOptions.NONE); - FIELD_TYPE.setOmitNorms(true); - FIELD_TYPE.freeze(); - } + private static BinaryFieldMapper toType(FieldMapper in) { + return (BinaryFieldMapper) in; } - public static class Builder extends FieldMapper.Builder { + public static class Builder extends ParametrizedFieldMapper.Builder { + + private final Parameter stored = Parameter.boolParam("store", false, m -> toType(m).stored, false); + private final Parameter hasDocValues = Parameter.boolParam("doc_values", false, m -> toType(m).hasDocValues, false); + private final Parameter> meta + = new Parameter<>("meta", true, Collections.emptyMap(), TypeParsers::parseMeta, m -> m.fieldType().meta()); public Builder(String name) { - super(name, Defaults.FIELD_TYPE); - hasDocValues = false; - builder = this; + this(name, false); + } + + public Builder(String name, boolean hasDocValues) { + super(name); + this.hasDocValues.setValue(hasDocValues); } @Override - public BinaryFieldMapper build(BuilderContext context) { - return new BinaryFieldMapper(name, fieldType, new BinaryFieldType(buildFullName(context), hasDocValues, meta), - multiFieldsBuilder.build(this, context), copyTo); + public List> getParameters() { + return List.of(meta, stored, hasDocValues); } @Override - public Builder index(boolean index) { - if (index) { - throw new MapperParsingException("Binary field [" + name() + "] cannot be indexed"); - } - return builder; + public BinaryFieldMapper build(BuilderContext context) { + return new BinaryFieldMapper(name, new BinaryFieldType(buildFullName(context), hasDocValues.getValue(), meta.getValue()), + multiFieldsBuilder.build(this, context), copyTo.build(), this); } } @@ -92,7 +87,7 @@ public static class TypeParser implements Mapper.TypeParser { public BinaryFieldMapper.Builder parse(String name, Map node, ParserContext parserContext) throws MapperParsingException { BinaryFieldMapper.Builder builder = new BinaryFieldMapper.Builder(name); - parseField(builder, name, node, parserContext); + builder.parse(name, parserContext, node); return builder; } } @@ -167,14 +162,19 @@ public Query termQuery(Object value, QueryShardContext context) { } } - protected BinaryFieldMapper(String simpleName, FieldType fieldType, MappedFieldType mappedFieldType, - MultiFields multiFields, CopyTo copyTo) { - super(simpleName, fieldType, mappedFieldType, multiFields, copyTo); + private final boolean stored; + private final boolean hasDocValues; + + protected BinaryFieldMapper(String simpleName, MappedFieldType mappedFieldType, + MultiFields multiFields, CopyTo copyTo, Builder builder) { + super(simpleName, mappedFieldType, multiFields, copyTo); + this.stored = builder.stored.getValue(); + this.hasDocValues = builder.hasDocValues.getValue(); } @Override protected void parseCreateField(ParseContext context) throws IOException { - if (!fieldType.stored() && !fieldType().hasDocValues()) { + if (stored == false && hasDocValues == false) { return; } byte[] value = context.parseExternalValue(byte[].class); @@ -188,11 +188,11 @@ protected void parseCreateField(ParseContext context) throws IOException { if (value == null) { return; } - if (fieldType.stored()) { + if (stored) { context.doc().add(new StoredField(fieldType().name(), value)); } - if (fieldType().hasDocValues()) { + if (hasDocValues) { CustomBinaryDocValuesField field = (CustomBinaryDocValuesField) context.doc().getByKey(fieldType().name()); if (field == null) { field = new CustomBinaryDocValuesField(fieldType().name(), value); @@ -210,18 +210,8 @@ protected void parseCreateField(ParseContext context) throws IOException { } @Override - protected boolean indexedByDefault() { - return false; - } - - @Override - protected boolean docValuesByDefault() { - return false; - } - - @Override - protected void mergeOptions(FieldMapper other, List conflicts) { - + public ParametrizedFieldMapper.Builder getMergeBuilder() { + return new BinaryFieldMapper.Builder(simpleName()).init(this); } @Override diff --git a/server/src/main/java/org/elasticsearch/index/mapper/BooleanFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/BooleanFieldMapper.java index 9a3894bea5e74..5912e5107774b 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/BooleanFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/BooleanFieldMapper.java @@ -22,6 +22,7 @@ import org.apache.lucene.document.Field; import org.apache.lucene.document.FieldType; import org.apache.lucene.document.SortedNumericDocValuesField; +import org.apache.lucene.document.StoredField; import org.apache.lucene.index.IndexOptions; import org.apache.lucene.index.Term; import org.apache.lucene.search.DocValuesFieldExistsQuery; @@ -30,7 +31,6 @@ import org.apache.lucene.search.TermRangeQuery; import org.apache.lucene.util.BytesRef; import org.elasticsearch.common.Nullable; -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; @@ -42,16 +42,13 @@ import java.io.IOException; import java.time.ZoneId; import java.util.Collections; -import java.util.Iterator; import java.util.List; import java.util.Map; -import static org.elasticsearch.index.mapper.TypeParsers.parseField; - /** * A field mapper for boolean fields. */ -public class BooleanFieldMapper extends FieldMapper { +public class BooleanFieldMapper extends ParametrizedFieldMapper { public static final String CONTENT_TYPE = "boolean"; @@ -71,25 +68,37 @@ public static class Values { public static final BytesRef FALSE = new BytesRef("F"); } - public static class Builder extends FieldMapper.Builder { + private static BooleanFieldMapper toType(FieldMapper in) { + return (BooleanFieldMapper) in; + } + + public static class Builder extends ParametrizedFieldMapper.Builder { + + private final Parameter docValues = Parameter.boolParam("doc_values", false, m -> toType(m).hasDocValues, true); + private final Parameter indexed = Parameter.boolParam("index", false, m -> toType(m).indexed, true); + private final Parameter stored = Parameter.boolParam("store", false, m -> toType(m).stored, false); - private Boolean nullValue; + private final Parameter nullValue + = new Parameter<>("null_value", false, null, (n, o) -> XContentMapValues.nodeBooleanValue(o), m -> toType(m).nullValue); + + private final Parameter boost = Parameter.floatParam("boost", true, m -> m.fieldType().boost(), 1.0f); + private final Parameter> meta + = new Parameter<>("meta", true, Collections.emptyMap(), TypeParsers::parseMeta, m -> m.fieldType().meta()); public Builder(String name) { - super(name, Defaults.FIELD_TYPE); - this.builder = this; + super(name); } - public Builder nullValue(Boolean nullValue) { - this.nullValue = nullValue; - return builder; + @Override + protected List> getParameters() { + return List.of(meta, boost, docValues, indexed, nullValue, stored); } @Override public BooleanFieldMapper build(BuilderContext context) { - return new BooleanFieldMapper(name, fieldType, - new BooleanFieldType(buildFullName(context), indexed, hasDocValues, meta), - multiFieldsBuilder.build(this, context), copyTo, nullValue); + MappedFieldType ft = new BooleanFieldType(buildFullName(context), indexed.getValue(), docValues.getValue(), meta.getValue()); + ft.setBoost(boost.getValue()); + return new BooleanFieldMapper(name, ft, multiFieldsBuilder.build(this, context), copyTo.build(), this); } } @@ -98,19 +107,7 @@ public static class TypeParser implements Mapper.TypeParser { public BooleanFieldMapper.Builder parse(String name, Map node, ParserContext parserContext) throws MapperParsingException { BooleanFieldMapper.Builder builder = new BooleanFieldMapper.Builder(name); - parseField(builder, name, node, parserContext); - for (Iterator> iterator = node.entrySet().iterator(); iterator.hasNext();) { - Map.Entry entry = iterator.next(); - String propName = entry.getKey(); - Object propNode = entry.getValue(); - if (propName.equals("null_value")) { - if (propNode == null) { - throw new MapperParsingException("Property [null_value] cannot be null."); - } - builder.nullValue(XContentMapValues.nodeBooleanValue(propNode, name + ".null_value")); - iterator.remove(); - } - } + builder.parse(name, parserContext, node); return builder; } } @@ -217,11 +214,17 @@ public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower } private final Boolean nullValue; - - protected BooleanFieldMapper(String simpleName, FieldType fieldType, MappedFieldType mappedFieldType, - MultiFields multiFields, CopyTo copyTo, Boolean nullValue) { - super(simpleName, fieldType, mappedFieldType, multiFields, copyTo); - this.nullValue = nullValue; + private final boolean indexed; + private final boolean hasDocValues; + private final boolean stored; + + protected BooleanFieldMapper(String simpleName, MappedFieldType mappedFieldType, + MultiFields multiFields, CopyTo copyTo, Builder builder) { + super(simpleName, mappedFieldType, multiFields, copyTo); + this.nullValue = builder.nullValue.getValue(); + this.stored = builder.stored.getValue(); + this.indexed = builder.indexed.getValue(); + this.hasDocValues = builder.docValues.getValue(); } @Override @@ -231,7 +234,7 @@ public BooleanFieldType fieldType() { @Override protected void parseCreateField(ParseContext context) throws IOException { - if (fieldType().isSearchable() == false && !fieldType.stored() && !fieldType().hasDocValues()) { + if (indexed == false && stored == false && hasDocValues == false) { return; } @@ -250,10 +253,13 @@ protected void parseCreateField(ParseContext context) throws IOException { if (value == null) { return; } - if (fieldType().isSearchable() || fieldType.stored()) { - context.doc().add(new Field(fieldType().name(), value ? "T" : "F", fieldType)); + if (indexed) { + context.doc().add(new Field(fieldType().name(), value ? "T" : "F", Defaults.FIELD_TYPE)); + } + if (stored) { + context.doc().add(new StoredField(fieldType().name(), value ? "T" : "F")); } - if (fieldType().hasDocValues()) { + if (hasDocValues) { context.doc().add(new SortedNumericDocValuesField(fieldType().name(), value ? 1 : 0)); } else { createFieldNamesField(context); @@ -261,8 +267,8 @@ protected void parseCreateField(ParseContext context) throws IOException { } @Override - protected void mergeOptions(FieldMapper other, List conflicts) { - // TODO ban updating null values + public ParametrizedFieldMapper.Builder getMergeBuilder() { + return new Builder(simpleName()).init(this); } @Override @@ -270,11 +276,4 @@ protected String contentType() { return CONTENT_TYPE; } - @Override - protected void doXContentBody(XContentBuilder builder, boolean includeDefaults, Params params) throws IOException { - super.doXContentBody(builder, includeDefaults, params); - if (includeDefaults || nullValue != null) { - builder.field("null_value", nullValue); - } - } } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java index 1b863c3051965..00f503a64632c 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/CompletionFieldMapper.java @@ -158,7 +158,7 @@ public Mapper.Builder parse(String name, Map node, ParserCont } else if (Fields.CONTEXTS.match(fieldName, LoggingDeprecationHandler.INSTANCE)) { builder.contextMappings(ContextMappings.load(fieldNode, parserContext.indexVersionCreated())); iterator.remove(); - } else if (parseMultiField(builder, name, parserContext, fieldName, fieldNode)) { + } else if (parseMultiField(builder::addMultiField, name, parserContext, fieldName, fieldNode)) { iterator.remove(); } } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/ContentPath.java b/server/src/main/java/org/elasticsearch/index/mapper/ContentPath.java index 7f3e26312ad87..015293864e55b 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/ContentPath.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/ContentPath.java @@ -45,6 +45,13 @@ public ContentPath(int offset) { this.index = 0; } + public ContentPath(String path) { + this.sb = new StringBuilder(); + this.offset = 0; + this.index = 0; + add(path); + } + public void add(String name) { path[index++] = name; if (index == path.length) { // expand if needed diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java index 69de862ac54ab..642d137dbf3cf 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java @@ -286,7 +286,7 @@ public Mapper.Builder parse(String name, Map node, ParserCont } else if (propName.equals("format")) { builder.format(propNode.toString()); iterator.remove(); - } else if (TypeParsers.parseMultiField(builder, name, parserContext, propName, propNode)) { + } else if (TypeParsers.parseMultiField(builder::addMultiField, name, parserContext, propName, propNode)) { iterator.remove(); } } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java index 070a192edacda..9d21c4a76b766 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java @@ -27,6 +27,7 @@ import org.elasticsearch.common.collect.ImmutableOpenMap; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.support.AbstractXContentParser; @@ -292,7 +293,7 @@ protected FieldMapper clone() { } @Override - public final FieldMapper merge(Mapper mergeWith) { + public FieldMapper merge(Mapper mergeWith) { FieldMapper merged = clone(); List conflicts = new ArrayList<>(); if (mergeWith instanceof FieldMapper == false) { @@ -487,7 +488,7 @@ public static String termVectorOptionsToString(FieldType fieldType) { protected abstract String contentType(); - public static class MultiFields { + public static class MultiFields implements Iterable { public static MultiFields empty() { return new MultiFields(ImmutableOpenMap.of()); @@ -502,8 +503,29 @@ public Builder add(Mapper.Builder builder) { return this; } + public Builder add(Mapper mapper) { + mapperBuilders.put(mapper.simpleName(), new Mapper.Builder(mapper.simpleName()) { + @Override + public Mapper build(BuilderContext context) { + return mapper; + } + }); + return this; + } + + public Builder update(Mapper toMerge, ContentPath contentPath) { + if (mapperBuilders.containsKey(toMerge.simpleName()) == false) { + add(toMerge); + } else { + Mapper.Builder builder = mapperBuilders.get(toMerge.simpleName()); + Mapper existing = builder.build(new BuilderContext(Settings.EMPTY, contentPath)); + add(existing.merge(toMerge)); + } + return this; + } + @SuppressWarnings("unchecked") - public MultiFields build(FieldMapper.Builder mainFieldBuilder, BuilderContext context) { + public MultiFields build(Mapper.Builder mainFieldBuilder, BuilderContext context) { if (mapperBuilders.isEmpty()) { return empty(); } else { @@ -568,6 +590,7 @@ public MultiFields merge(MultiFields mergeWith) { return new MultiFields(mappers); } + @Override public Iterator iterator() { return StreamSupport.stream(mappers.values().spliterator(), false).map((p) -> (Mapper)p.value).iterator(); } @@ -634,6 +657,11 @@ public CopyTo build() { } return new CopyTo(Collections.unmodifiableList(copyToBuilders)); } + + public void reset(CopyTo copyTo) { + copyToBuilders.clear(); + copyToBuilders.addAll(copyTo.copyToFields); + } } public List copyToFields() { diff --git a/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java index d836bc0284eb3..42dd60cff421c 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java @@ -130,7 +130,7 @@ public Mapper.Builder parse(String name, Map node, ParserCont } else if (propName.equals("ignore_malformed")) { builder.ignoreMalformed(XContentMapValues.nodeBooleanValue(propNode, name + ".ignore_malformed")); iterator.remove(); - } else if (TypeParsers.parseMultiField(builder, name, parserContext, propName, propNode)) { + } else if (TypeParsers.parseMultiField(builder::addMultiField, name, parserContext, propName, propNode)) { iterator.remove(); } } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/ParametrizedFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/ParametrizedFieldMapper.java new file mode 100644 index 0000000000000..f0fe1906d05e1 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/index/mapper/ParametrizedFieldMapper.java @@ -0,0 +1,345 @@ +/* + * 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.index.mapper; + +import org.apache.lucene.document.FieldType; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.support.XContentMapValues; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.function.BiFunction; +import java.util.function.Function; + +/** + * Defines how a particular field should be indexed and searched + * + * Configuration {@link Parameter}s for the mapper are defined on a {@link Builder} subclass, + * and returned by its {@link Builder#getParameters()} method. Merging, serialization + * and parsing of the mapper are all mediated through this set of parameters. + * + * Subclasses should implement a {@link Builder} that is returned from the + * {@link #getMergeBuilder()} method, initialised with the existing builder. + */ +public abstract class ParametrizedFieldMapper extends FieldMapper { + + /** + * Creates a new ParametrizedFieldMapper + */ + protected ParametrizedFieldMapper(String simpleName, MappedFieldType mappedFieldType, MultiFields multiFields, CopyTo copyTo) { + super(simpleName, new FieldType(), mappedFieldType, multiFields, copyTo); + } + + /** + * Returns a {@link Builder} to be used for merging and serialization + * + * Implement as follows: + * {@code return new MyBuilder(simpleName()).init(this); } + */ + public abstract ParametrizedFieldMapper.Builder getMergeBuilder(); + + @Override + public final ParametrizedFieldMapper merge(Mapper mergeWith) { + + if (mergeWith instanceof FieldMapper == false) { + throw new IllegalArgumentException("mapper [" + name() + "] cannot be changed from type [" + + contentType() + "] to [" + mergeWith.getClass().getSimpleName() + "]"); + } + if (Objects.equals(this.getClass(), mergeWith.getClass()) == false) { + throw new IllegalArgumentException("mapper [" + name() + "] cannot be changed from type [" + + contentType() + "] to [" + ((FieldMapper) mergeWith).contentType() + "]"); + } + + ParametrizedFieldMapper.Builder builder = getMergeBuilder(); + Conflicts conflicts = new Conflicts(name()); + builder.merge((FieldMapper) mergeWith, conflicts); + conflicts.check(); + return builder.build(new BuilderContext(Settings.EMPTY, parentPath(name()))); + } + + private static ContentPath parentPath(String name) { + int endPos = name.lastIndexOf("."); + if (endPos == -1) { + return new ContentPath(0); + } + return new ContentPath(name.substring(0, endPos)); + } + + @Override + protected final void mergeOptions(FieldMapper other, List conflicts) { + // TODO remove when everything is parametrized + } + + @Override + public final XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.startObject(simpleName()); + builder.field("type", contentType()); + boolean includeDefaults = params.paramAsBoolean("include_defaults", false); + getMergeBuilder().toXContent(builder, includeDefaults); + multiFields.toXContent(builder, params); + copyTo.toXContent(builder, params); + return builder.endObject(); + } + + /** + * A configurable parameter for a field mapper + * @param the type of the value the parameter holds + */ + public static final class Parameter { + + public final String name; + private final T defaultValue; + private final BiFunction parser; + private final Function initializer; + private final boolean updateable; + private boolean acceptsNull = false; + private T value; + + /** + * Creates a new Parameter + * @param name the parameter name, used in parsing and serialization + * @param updateable whether the parameter can be updated with a new value during a mapping update + * @param defaultValue the default value for the parameter, used if unspecified in mappings + * @param parser a function that converts an object to a parameter value + * @param initializer a function that reads a parameter value from an existing mapper + */ + public Parameter(String name, boolean updateable, T defaultValue, + BiFunction parser, Function initializer) { + this.name = name; + this.defaultValue = defaultValue; + this.value = defaultValue; + this.parser = parser; + this.initializer = initializer; + this.updateable = updateable; + } + + /** + * Returns the current value of the parameter + */ + public T getValue() { + return value; + } + + /** + * Sets the current value of the parameter + */ + public void setValue(T value) { + this.value = value; + } + + /** + * Allows the parameter to accept a {@code null} value + */ + public Parameter acceptsNull() { + this.acceptsNull = true; + return this; + } + + private void init(FieldMapper toInit) { + this.value = initializer.apply(toInit); + } + + private void parse(String field, Object in) { + this.value = parser.apply(field, in); + } + + private void merge(FieldMapper toMerge, Conflicts conflicts) { + T value = initializer.apply(toMerge); + if (updateable == false && Objects.equals(this.value, value) == false) { + conflicts.addConflict(name, this.value.toString(), value.toString()); + } else { + this.value = value; + } + } + + private void toXContent(XContentBuilder builder, boolean includeDefaults) throws IOException { + if (includeDefaults || (Objects.equals(defaultValue, value) == false)) { + builder.field(name, value); + } + } + + /** + * Defines a parameter that takes the values {@code true} or {@code false} + * @param name the parameter name + * @param updateable whether the parameter can be changed by a mapping update + * @param initializer a function that reads the parameter value from an existing mapper + * @param defaultValue the default value, to be used if the parameter is undefined in a mapping + */ + public static Parameter boolParam(String name, boolean updateable, + Function initializer, boolean defaultValue) { + return new Parameter<>(name, updateable, defaultValue, (n, o) -> XContentMapValues.nodeBooleanValue(o), initializer); + } + + /** + * Defines a parameter that takes a float value + * @param name the parameter name + * @param updateable whether the parameter can be changed by a mapping update + * @param initializer a function that reads the parameter value from an existing mapper + * @param defaultValue the default value, to be used if the parameter is undefined in a mapping + */ + public static Parameter floatParam(String name, boolean updateable, + Function initializer, float defaultValue) { + return new Parameter<>(name, updateable, defaultValue, (n, o) -> XContentMapValues.nodeFloatValue(o), initializer); + } + + /** + * Defines a parameter that takes a string value + * @param name the parameter name + * @param updateable whether the parameter can be changed by a mapping update + * @param initializer a function that reads the parameter value from an existing mapper + * @param defaultValue the default value, to be used if the parameter is undefined in a mapping + */ + public static Parameter stringParam(String name, boolean updateable, + Function initializer, String defaultValue) { + return new Parameter<>(name, updateable, defaultValue, + (n, o) -> XContentMapValues.nodeStringValue(o), initializer); + } + } + + private static final class Conflicts { + + private final String mapperName; + private final List conflicts = new ArrayList<>(); + + Conflicts(String mapperName) { + this.mapperName = mapperName; + } + + void addConflict(String parameter, String existing, String toMerge) { + conflicts.add("Cannot update parameter [" + parameter + "] from [" + existing + "] to [" + toMerge + "]"); + } + + void check() { + if (conflicts.isEmpty()) { + return; + } + String message = "Mapper for [" + mapperName + "] conflicts with existing mapper:\n\t" + + String.join("\n\t", conflicts); + throw new IllegalArgumentException(message); + } + + } + + /** + * A Builder for a ParametrizedFieldMapper + */ + public abstract static class Builder extends Mapper.Builder { + + protected final MultiFields.Builder multiFieldsBuilder = new MultiFields.Builder(); + protected final CopyTo.Builder copyTo = new CopyTo.Builder(); + + /** + * Creates a new Builder with a field name + */ + protected Builder(String name) { + super(name); + } + + /** + * Initialises all parameters from an existing mapper + */ + public Builder init(FieldMapper initializer) { + for (Parameter param : getParameters()) { + param.init(initializer); + } + for (Mapper subField : initializer.multiFields) { + multiFieldsBuilder.add(subField); + } + return this; + } + + private void merge(FieldMapper in, Conflicts conflicts) { + for (Parameter param : getParameters()) { + param.merge(in, conflicts); + } + for (Mapper newSubField : in.multiFields) { + multiFieldsBuilder.update(newSubField, parentPath(newSubField.name())); + } + this.copyTo.reset(in.copyTo); + } + + /** + * @return the list of parameters defined for this mapper + */ + protected abstract List> getParameters(); + + @Override + public abstract ParametrizedFieldMapper build(BuilderContext context); + + /** + * Builds the full name of the field, taking into account parent objects + */ + protected String buildFullName(BuilderContext context) { + return context.path().pathAsText(name); + } + + private void toXContent(XContentBuilder builder, boolean includeDefaults) throws IOException { + for (Parameter parameter : getParameters()) { + parameter.toXContent(builder, includeDefaults); + } + } + + /** + * Parse mapping parameters from a map of mappings + * @param name the field mapper name + * @param parserContext the parser context + * @param fieldNode the root node of the map of mappings for this field + */ + public final void parse(String name, TypeParser.ParserContext parserContext, Map fieldNode) { + Map> paramsMap = new HashMap<>(); + for (Parameter param : getParameters()) { + paramsMap.put(param.name, param); + } + String type = (String) fieldNode.remove("type"); + for (Iterator> iterator = fieldNode.entrySet().iterator(); iterator.hasNext();) { + Map.Entry entry = iterator.next(); + final String propName = entry.getKey(); + final Object propNode = entry.getValue(); + if (Objects.equals("fields", propName)) { + TypeParsers.parseMultiField(multiFieldsBuilder::add, name, parserContext, propName, propNode); + iterator.remove(); + continue; + } + if (Objects.equals("copy_to", propName)) { + TypeParsers.parseCopyFields(propNode).forEach(copyTo::add); + iterator.remove(); + continue; + } + Parameter parameter = paramsMap.get(propName); + if (parameter == null) { + throw new MapperParsingException("unknown parameter [" + propName + + "] on mapper [" + name + "] of type [" + type + "]"); + } + if (propNode == null && parameter.acceptsNull == false) { + throw new MapperParsingException("[" + propName + "] on mapper [" + name + + "] of type [" + type + "] must not have a [null] value"); + } + parameter.parse(name, propNode); + iterator.remove(); + } + } + } +} diff --git a/server/src/main/java/org/elasticsearch/index/mapper/RangeFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/RangeFieldMapper.java index 8d887a57a1547..66f00cd8fdf7d 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/RangeFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/RangeFieldMapper.java @@ -172,7 +172,7 @@ public Mapper.Builder parse(String name, Map node, } else if (propName.equals("format")) { builder.format(propNode.toString()); iterator.remove(); - } else if (TypeParsers.parseMultiField(builder, name, parserContext, propName, propNode)) { + } else if (TypeParsers.parseMultiField(builder::addMultiField, name, parserContext, propName, propNode)) { iterator.remove(); } } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/TypeParsers.java b/server/src/main/java/org/elasticsearch/index/mapper/TypeParsers.java index 7fc98ec2f5229..8754bdd4ce821 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/TypeParsers.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/TypeParsers.java @@ -29,10 +29,12 @@ import org.elasticsearch.index.analysis.NamedAnalyzer; import org.elasticsearch.index.similarity.SimilarityProvider; +import java.util.ArrayList; import java.util.Collections; import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.function.Consumer; import java.util.function.Function; import java.util.stream.Collectors; @@ -181,12 +183,7 @@ public static void checkNull(String propName, Object propNode) { /** * Parse the {@code meta} key of the mapping. */ - public static void parseMeta(FieldMapper.Builder builder, String name, Map fieldNode) { - Object metaObject = fieldNode.remove("meta"); - if (metaObject == null) { - // no meta - return; - } + public static Map parseMeta(String name, Object metaObject) { if (metaObject instanceof Map == false) { throw new MapperParsingException("[meta] must be an object, got " + metaObject.getClass().getSimpleName() + "[" + metaObject + "] for field [" + name +"]"); @@ -219,17 +216,17 @@ public static void parseMeta(FieldMapper.Builder builder, String name, Map, Object> entryValueFunction = Map.Entry::getValue; final Function stringCast = String.class::cast; - Map checkedMeta = meta.entrySet().stream() + return meta.entrySet().stream() .collect(Collectors.toUnmodifiableMap(Map.Entry::getKey, entryValueFunction.andThen(stringCast))); - builder.meta(checkedMeta); } + + /** * Parse common field attributes such as {@code doc_values} or {@code store}. */ public static void parseField(FieldMapper.Builder builder, String name, Map fieldNode, Mapper.TypeParser.ParserContext parserContext) { - parseMeta(builder, name, fieldNode); for (Iterator> iterator = fieldNode.entrySet().iterator(); iterator.hasNext();) { Map.Entry entry = iterator.next(); final String propName = entry.getKey(); @@ -238,6 +235,9 @@ public static void parseField(FieldMapper.Builder builder, String name, Map builder, String name, Map copyFields = parseCopyFields(propNode); + FieldMapper.CopyTo.Builder cpBuilder = new FieldMapper.CopyTo.Builder(); + copyFields.forEach(cpBuilder::add); + builder.copyTo(cpBuilder.build()); } iterator.remove(); } @@ -269,8 +272,8 @@ public static void parseField(FieldMapper.Builder builder, String name, Map multiFieldsBuilder, String name, + Mapper.TypeParser.ParserContext parserContext, String propName, Object propNode) { if (propName.equals("fields")) { if (parserContext.isWithinMultiField()) { // For indices created prior to 8.0, we only emit a deprecation warning and do not fail type parsing. This is to @@ -327,7 +330,7 @@ public static boolean parseMultiField(FieldMapper.Builder builder, String name, if (typeParser == null) { throw new MapperParsingException("no handler for type [" + type + "] declared on field [" + multiFieldName + "]"); } - builder.addMultiField(typeParser.parse(multiFieldName, multiFieldNodes, parserContext)); + multiFieldsBuilder.accept(typeParser.parse(multiFieldName, multiFieldNodes, parserContext)); multiFieldNodes.remove("type"); DocumentMapperParser.checkNoRemainingFields(propName, multiFieldNodes, parserContext.indexVersionCreated()); } @@ -383,17 +386,16 @@ public static void parseTermVector(String fieldName, String termVector, FieldMap } } - @SuppressWarnings({"unchecked", "rawtypes"}) - public static void parseCopyFields(Object propNode, FieldMapper.Builder builder) { - FieldMapper.CopyTo.Builder copyToBuilder = new FieldMapper.CopyTo.Builder(); + public static List parseCopyFields(Object propNode) { + List copyFields = new ArrayList<>(); if (isArray(propNode)) { for (Object node : (List) propNode) { - copyToBuilder.add(nodeStringValue(node, null)); + copyFields.add(nodeStringValue(node, null)); } } else { - copyToBuilder.add(nodeStringValue(propNode, null)); + copyFields.add(nodeStringValue(propNode, null)); } - builder.copyTo(copyToBuilder.build()); + return copyFields; } public static SimilarityProvider resolveSimilarity(Mapper.TypeParser.ParserContext parserContext, String name, String value) { diff --git a/server/src/test/java/org/elasticsearch/index/fielddata/AbstractFieldDataTestCase.java b/server/src/test/java/org/elasticsearch/index/fielddata/AbstractFieldDataTestCase.java index c7883c8cd7603..a6bdf6cfacef9 100644 --- a/server/src/test/java/org/elasticsearch/index/fielddata/AbstractFieldDataTestCase.java +++ b/server/src/test/java/org/elasticsearch/index/fielddata/AbstractFieldDataTestCase.java @@ -122,7 +122,7 @@ public > IFD getForField(String type, String field } else if (type.equals("geo_point")) { fieldType = new GeoPointFieldMapper.Builder(fieldName).docValues(docValues).build(context).fieldType(); } else if (type.equals("binary")) { - fieldType = new BinaryFieldMapper.Builder(fieldName).docValues(docValues).build(context).fieldType(); + fieldType = new BinaryFieldMapper.Builder(fieldName, docValues).build(context).fieldType(); } else { throw new UnsupportedOperationException(type); } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/BinaryFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/BinaryFieldMapperTests.java index dad6ac927a667..2de5ab8f75d79 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/BinaryFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/BinaryFieldMapperTests.java @@ -32,28 +32,18 @@ import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.test.ESSingleNodeTestCase; import org.elasticsearch.test.InternalSettingsPlugin; import java.io.IOException; import java.util.Arrays; import java.util.Collection; -import java.util.Set; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; -public class BinaryFieldMapperTests extends FieldMapperTestCase { - - @Override - protected Set unsupportedProperties() { - return Set.of("analyzer", "eager_global_ordinals", "norms", "similarity", "index"); - } - - @Override - protected BinaryFieldMapper.Builder newBuilder() { - return new BinaryFieldMapper.Builder("binary"); - } +public class BinaryFieldMapperTests extends ESSingleNodeTestCase { @Override protected Collection> getPlugins() { diff --git a/server/src/test/java/org/elasticsearch/index/mapper/BooleanFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/BooleanFieldMapperTests.java index 564be1efdca85..0d5ed6d24803a 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/BooleanFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/BooleanFieldMapperTests.java @@ -27,6 +27,9 @@ import org.apache.lucene.index.IndexableField; import org.apache.lucene.index.LeafReader; import org.apache.lucene.index.SortedNumericDocValues; +import org.apache.lucene.index.Term; +import org.apache.lucene.search.BoostQuery; +import org.apache.lucene.search.TermQuery; import org.apache.lucene.store.ByteBuffersDirectory; import org.apache.lucene.store.Directory; import org.apache.lucene.util.BytesRef; @@ -41,25 +44,21 @@ import org.elasticsearch.index.mapper.MapperService.MergeReason; import org.elasticsearch.index.mapper.ParseContext.Document; import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.test.ESSingleNodeTestCase; import org.elasticsearch.test.InternalSettingsPlugin; import org.junit.Before; import java.io.IOException; import java.util.Collection; import java.util.Collections; -import java.util.Set; import static org.hamcrest.Matchers.containsString; -public class BooleanFieldMapperTests extends FieldMapperTestCase { +public class BooleanFieldMapperTests extends ESSingleNodeTestCase { + private IndexService indexService; private DocumentMapperParser parser; - @Override - protected Set unsupportedProperties() { - return Set.of("analyzer", "similarity"); - } - @Before public void setup() { indexService = createIndex("test"); @@ -285,9 +284,13 @@ public void testMeta() throws Exception { assertEquals(mapping3, mapper.mappingSource().toString()); } - @Override - protected BooleanFieldMapper.Builder newBuilder() { - return new BooleanFieldMapper.Builder("boolean"); + public void testBoosts() throws Exception { + String mapping = "{\"_doc\":{\"properties\":{\"field\":{\"type\":\"boolean\",\"boost\":2.0}}}}"; + DocumentMapper mapper = indexService.mapperService().merge("_doc", new CompressedXContent(mapping), MergeReason.MAPPING_UPDATE); + assertEquals(mapping, mapper.mappingSource().toString()); + + MappedFieldType ft = indexService.mapperService().fieldType("field"); + assertEquals(new BoostQuery(new TermQuery(new Term("field", "T")), 2.0f), ft.termQuery("true", null)); } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/NullValueTests.java b/server/src/test/java/org/elasticsearch/index/mapper/NullValueTests.java index f38f83b6e418f..fce744553456a 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/NullValueTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/NullValueTests.java @@ -2,6 +2,14 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.compress.CompressedXContent; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.index.IndexService; +import org.elasticsearch.test.ESSingleNodeTestCase; + +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.either; +import static org.hamcrest.Matchers.equalTo; /* * Licensed to Elasticsearch under one or more contributor @@ -22,14 +30,6 @@ * under the License. */ - -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.xcontent.XContentFactory; -import org.elasticsearch.index.IndexService; -import org.elasticsearch.test.ESSingleNodeTestCase; - -import static org.hamcrest.Matchers.equalTo; - public class NullValueTests extends ESSingleNodeTestCase { public void testNullNullValue() throws Exception { IndexService indexService = createIndex("test", Settings.builder().build()); @@ -52,7 +52,9 @@ public void testNullNullValue() throws Exception { indexService.mapperService().documentMapperParser().parse("type", new CompressedXContent(mapping)); fail("Test should have failed because [null_value] was null."); } catch (MapperParsingException e) { - assertThat(e.getMessage(), equalTo("Property [null_value] cannot be null.")); + assertThat(e.getMessage(), + either(equalTo("Property [null_value] cannot be null.")) + .or(containsString("must not have a [null] value"))); } } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/ParametrizedMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/ParametrizedMapperTests.java new file mode 100644 index 0000000000000..ea236871cd380 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/index/mapper/ParametrizedMapperTests.java @@ -0,0 +1,250 @@ +/* + * 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.index.mapper; + +import org.elasticsearch.Version; +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.compress.CompressedXContent; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.xcontent.ToXContent; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentHelper; +import org.elasticsearch.common.xcontent.json.JsonXContent; +import org.elasticsearch.index.IndexService; +import org.elasticsearch.index.mapper.ParametrizedFieldMapper.Parameter; +import org.elasticsearch.plugins.MapperPlugin; +import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.test.ESSingleNodeTestCase; + +import java.io.IOException; +import java.util.Collection; +import java.util.List; +import java.util.Map; +import java.util.Objects; + +public class ParametrizedMapperTests extends ESSingleNodeTestCase { + + public static class TestPlugin extends Plugin implements MapperPlugin { + @Override + public Map getMappers() { + return Map.of("test_mapper", new TypeParser()); + } + } + + @Override + protected Collection> getPlugins() { + return List.of(TestPlugin.class); + } + + private static TestMapper toType(Mapper in) { + return (TestMapper) in; + } + + public static class Builder extends ParametrizedFieldMapper.Builder { + + final Parameter fixed + = Parameter.boolParam("fixed", false, m -> toType(m).fixed, true); + final Parameter fixed2 + = Parameter.boolParam("fixed2", false, m -> toType(m).fixed2, false); + final Parameter variable + = Parameter.stringParam("variable", true, m -> toType(m).variable, "default").acceptsNull(); + + protected Builder(String name) { + super(name); + } + + @Override + protected List> getParameters() { + return List.of(fixed, fixed2, variable); + } + + @Override + public ParametrizedFieldMapper build(Mapper.BuilderContext context) { + return new TestMapper(name(), buildFullName(context), + multiFieldsBuilder.build(this, context), copyTo.build(), this); + } + } + + public static class TypeParser implements Mapper.TypeParser { + + @Override + public Mapper.Builder parse(String name, Map node, ParserContext parserContext) throws MapperParsingException { + Builder builder = new Builder(name); + builder.parse(name, parserContext, node); + return builder; + } + } + + public static class TestMapper extends ParametrizedFieldMapper { + + private final boolean fixed; + private final boolean fixed2; + private final String variable; + + protected TestMapper(String simpleName, String fullName, MultiFields multiFields, CopyTo copyTo, + ParametrizedMapperTests.Builder builder) { + super(simpleName, new KeywordFieldMapper.KeywordFieldType(fullName), multiFields, copyTo); + this.fixed = builder.fixed.getValue(); + this.fixed2 = builder.fixed2.getValue(); + this.variable = builder.variable.getValue(); + } + + @Override + public Builder getMergeBuilder() { + return new ParametrizedMapperTests.Builder(simpleName()).init(this); + } + + @Override + protected void parseCreateField(ParseContext context) throws IOException { + + } + + @Override + protected String contentType() { + return "test_mapper"; + } + } + + private static TestMapper fromMapping(String mapping) { + Mapper.TypeParser.ParserContext pc = new Mapper.TypeParser.ParserContext(s -> null, null, s -> { + if (Objects.equals("keyword", s)) { + return new KeywordFieldMapper.TypeParser(); + } + if (Objects.equals("binary", s)) { + return new BinaryFieldMapper.TypeParser(); + } + return null; + }, Version.CURRENT, () -> null); + return (TestMapper) new TypeParser() + .parse("field", XContentHelper.convertToMap(JsonXContent.jsonXContent, mapping, true), pc) + .build(new Mapper.BuilderContext(Settings.EMPTY, new ContentPath(0))); + } + + // defaults - create empty builder config, and serialize with and without defaults + public void testDefaults() throws IOException { + String mapping = "{\"type\":\"test_mapper\"}"; + TestMapper mapper = fromMapping(mapping); + + assertTrue(mapper.fixed); + assertEquals("default", mapper.variable); + + assertEquals("{\"field\":" + mapping + "}", Strings.toString(mapper)); + + XContentBuilder builder = JsonXContent.contentBuilder(); + ToXContent.Params params = new ToXContent.MapParams(Map.of("include_defaults", "true")); + builder.startObject(); + mapper.toXContent(builder, params); + builder.endObject(); + assertEquals("{\"field\":{\"type\":\"test_mapper\",\"fixed\":true,\"fixed2\":false,\"variable\":\"default\"}}", + Strings.toString(builder)); + } + + // merging - try updating 'fixed' and 'fixed2' should get an error, try updating 'variable' and verify update + public void testMerging() { + String mapping = "{\"type\":\"test_mapper\",\"fixed\":false}"; + TestMapper mapper = fromMapping(mapping); + assertEquals("{\"field\":" + mapping + "}", Strings.toString(mapper)); + + TestMapper badMerge = fromMapping("{\"type\":\"test_mapper\",\"fixed\":true,\"fixed2\":true}"); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> mapper.merge(badMerge)); + String expectedError = "Mapper for [field] conflicts with existing mapper:\n" + + "\tCannot update parameter [fixed] from [false] to [true]\n" + + "\tCannot update parameter [fixed2] from [false] to [true]"; + assertEquals(expectedError, e.getMessage()); + + assertEquals("{\"field\":" + mapping + "}", Strings.toString(mapper)); // original mapping is unaffected + + // TODO: should we have to include 'fixed' here? Or should updates take as 'defaults' the existing values? + TestMapper goodMerge = fromMapping("{\"type\":\"test_mapper\",\"fixed\":false,\"variable\":\"updated\"}"); + TestMapper merged = (TestMapper) mapper.merge(goodMerge); + + assertEquals("{\"field\":" + mapping + "}", Strings.toString(mapper)); // original mapping is unaffected + assertEquals("{\"field\":{\"type\":\"test_mapper\",\"fixed\":false,\"variable\":\"updated\"}}", Strings.toString(merged)); + + } + + // add multifield, verify, add second multifield, verify, overwrite second multifield + public void testMultifields() { + String mapping = "{\"type\":\"test_mapper\",\"variable\":\"foo\",\"fields\":{\"sub\":{\"type\":\"keyword\"}}}"; + TestMapper mapper = fromMapping(mapping); + assertEquals("{\"field\":" + mapping + "}", Strings.toString(mapper)); + + String addSubField = "{\"type\":\"test_mapper\",\"variable\":\"foo\",\"fields\":{\"sub2\":{\"type\":\"keyword\"}}}"; + TestMapper toMerge = fromMapping(addSubField); + TestMapper merged = (TestMapper) mapper.merge(toMerge); + assertEquals("{\"field\":{\"type\":\"test_mapper\",\"variable\":\"foo\"," + + "\"fields\":{\"sub\":{\"type\":\"keyword\"},\"sub2\":{\"type\":\"keyword\"}}}}", Strings.toString(merged)); + + String badSubField = "{\"type\":\"test_mapper\",\"variable\":\"foo\",\"fields\":{\"sub2\":{\"type\":\"binary\"}}}"; + TestMapper badToMerge = fromMapping(badSubField); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> merged.merge(badToMerge)); + assertEquals("mapper [field.sub2] cannot be changed from type [keyword] to [binary]", e.getMessage()); + } + + // add copy_to, verify + public void testCopyTo() { + String mapping = "{\"type\":\"test_mapper\",\"variable\":\"foo\",\"copy_to\":[\"other\"]}"; + TestMapper mapper = fromMapping(mapping); + assertEquals("{\"field\":" + mapping + "}", Strings.toString(mapper)); + + // On update, copy_to is completely replaced + + TestMapper toMerge = fromMapping("{\"type\":\"test_mapper\",\"variable\":\"updated\",\"copy_to\":[\"foo\",\"bar\"]}"); + TestMapper merged = (TestMapper) mapper.merge(toMerge); + assertEquals("{\"field\":{\"type\":\"test_mapper\",\"variable\":\"updated\",\"copy_to\":[\"foo\",\"bar\"]}}", + Strings.toString(merged)); + + TestMapper removeCopyTo = fromMapping("{\"type\":\"test_mapper\",\"variable\":\"updated\"}"); + TestMapper noCopyTo = (TestMapper) merged.merge(removeCopyTo); + assertEquals("{\"field\":{\"type\":\"test_mapper\",\"variable\":\"updated\"}}", Strings.toString(noCopyTo)); + } + + public void testNullables() { + String mapping = "{\"type\":\"test_mapper\",\"fixed\":null}"; + MapperParsingException e = expectThrows(MapperParsingException.class, () -> fromMapping(mapping)); + assertEquals("[fixed] on mapper [field] of type [test_mapper] must not have a [null] value", e.getMessage()); + + String fine = "{\"type\":\"test_mapper\",\"variable\":null}"; + TestMapper mapper = fromMapping(fine); + assertEquals("{\"field\":" + fine + "}", Strings.toString(mapper)); + } + + public void testObjectSerialization() throws IOException { + + IndexService indexService = createIndex("test"); + + String mapping = "{\"_doc\":{" + + "\"properties\":{" + + "\"actual\":{\"type\":\"double\"}," + + "\"bucket_count\":{\"type\":\"long\"}," + + "\"bucket_influencers\":{\"type\":\"nested\",\"properties\":{" + + "\"anomaly_score\":{\"type\":\"double\"}," + + "\"bucket_span\":{\"type\":\"long\"}," + + "\"is_interim\":{\"type\":\"boolean\"}}}}}}"; + indexService.mapperService().merge("_doc", new CompressedXContent(mapping), MapperService.MergeReason.MAPPING_UPDATE); + assertEquals(mapping, Strings.toString(indexService.mapperService().documentMapper())); + + indexService.mapperService().merge("_doc", new CompressedXContent(mapping), MapperService.MergeReason.MAPPING_UPDATE); + assertEquals(mapping, Strings.toString(indexService.mapperService().documentMapper())); + + + } + +} diff --git a/server/src/test/java/org/elasticsearch/index/mapper/RootObjectMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/RootObjectMapperTests.java index f5d7459f517d7..c714615fe29eb 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/RootObjectMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/RootObjectMapperTests.java @@ -432,8 +432,8 @@ public void testIllegalDynamicTemplateNoMappingType() throws Exception { mapping.endObject(); MapperParsingException e = expectThrows(MapperParsingException.class, () -> mapperService.merge("type", new CompressedXContent(Strings.toString(mapping)), MergeReason.MAPPING_UPDATE)); - assertThat(e.getRootCause(), instanceOf(IllegalArgumentException.class)); - assertThat(e.getRootCause().getMessage(), equalTo("Unused mapping attributes [{foo=bar}]")); + assertThat(e.getRootCause(), instanceOf(MapperParsingException.class)); + assertThat(e.getRootCause().getMessage(), equalTo("unknown parameter [foo] on mapper [__dummy__] of type [null]")); } } diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java index 6d07c70974e21..67832936cfa3e 100644 --- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java +++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java @@ -121,7 +121,6 @@ public static class TypeParser implements Mapper.TypeParser { public Mapper.Builder parse(String name, Map node, ParserContext parserContext) throws MapperParsingException { Builder builder = new HistogramFieldMapper.Builder(name); - TypeParsers.parseMeta(builder, name, node); for (Iterator> iterator = node.entrySet().iterator(); iterator.hasNext();) { Map.Entry entry = iterator.next(); String propName = entry.getKey(); @@ -130,6 +129,10 @@ public Mapper.Builder parse(String name, Map node, Pars builder.ignoreMalformed(XContentMapValues.nodeBooleanValue(propNode, name + "." + Names.IGNORE_MALFORMED)); iterator.remove(); } + if (propName.equals("meta")) { + builder.meta(TypeParsers.parseMeta(propName, propNode)); + iterator.remove(); + } } return builder; } diff --git a/x-pack/plugin/mapper-constant-keyword/src/main/java/org/elasticsearch/xpack/constantkeyword/mapper/ConstantKeywordFieldMapper.java b/x-pack/plugin/mapper-constant-keyword/src/main/java/org/elasticsearch/xpack/constantkeyword/mapper/ConstantKeywordFieldMapper.java index 24067a70e5c50..f8dc530e26a5f 100644 --- a/x-pack/plugin/mapper-constant-keyword/src/main/java/org/elasticsearch/xpack/constantkeyword/mapper/ConstantKeywordFieldMapper.java +++ b/x-pack/plugin/mapper-constant-keyword/src/main/java/org/elasticsearch/xpack/constantkeyword/mapper/ConstantKeywordFieldMapper.java @@ -102,7 +102,9 @@ public Mapper.Builder parse(String name, Map node, ParserCont if (value != null) { builder.setValue(value.toString()); } - TypeParsers.parseMeta(builder, name, node); + if (node.containsKey("meta")) { + builder.meta(TypeParsers.parseMeta(name, node.remove("meta"))); + } return builder; } } @@ -152,11 +154,11 @@ public String value() { public String typeName() { return CONTENT_TYPE; } - + @Override public String familyTypeName() { return KeywordFieldMapper.CONTENT_TYPE; - } + } @Override public IndexFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName) {