Add parameter update and conflict tests to MapperTestCase (#62828) (#…

…62902)

This commit adds a mechanism to MapperTestCase that allows implementing
test classes to check that their parameters can be updated, or throw conflict
errors as advertised. Child classes override the registerParameters method
and tell the passed-in UpdateChecker class about their parameters. Simple
conflicts can be checked, using the existing minimal mappings as a base to
compare against, or alternatively a particular initial mapping can be provided
to check edge cases (eg, norms can be updated from true to false, but not
vice versa). Updates are registered with a predicate that checks that the update
has in fact been applied to the resulting FieldMapper.

Fixes #61631

modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/ScaledFloatFieldMapper.java

@@ -282,6 +282,14 @@ private ScaledFloatFieldMapper(
         this.coerceByDefault = builder.coerce.getDefaultValue().value();
     }
 
+    boolean coerce() {
+        return coerce.value();
+    }
+
+    boolean ignoreMalformed() {
+        return ignoreMalformed.value();
+    }
+
     @Override
     public ScaledFloatFieldType fieldType() {
         return (ScaledFloatFieldType) super.fieldType();

modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/RankFeatureFieldMapperTests.java

@@ -33,7 +33,6 @@
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.plugins.Plugin;
 import org.hamcrest.Matchers;
-import org.junit.Before;
 
 import java.io.IOException;
 import java.util.Arrays;
@@ -54,12 +53,9 @@ protected Set<String> unsupportedProperties() {
         return org.elasticsearch.common.collect.Set.of("analyzer", "similarity", "store", "doc_values", "index");
     }
 
-    @Before
-    public void setup() {
-        addModifier("positive_score_impact", false, (a, b) -> {
-            a.positiveScoreImpact(true);
-            b.positiveScoreImpact(false);
-        });
+    @Override
+    protected void registerParameters(ParameterChecker checker) throws IOException {
+        checker.registerConflictCheck("positive_score_impact", b -> b.field("positive_score_impact", false));
     }
 
     @Override

modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/RankFeaturesFieldMapperTests.java

@@ -59,6 +59,11 @@ protected void minimalMapping(XContentBuilder b) throws IOException {
         b.field("type", "rank_features");
     }
 
+    @Override
+    protected void registerParameters(ParameterChecker checker) {
+        // no parameters to configure
+    }
+
     @Override
     protected boolean supportsMeta() {
         return false;

modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/ScaledFloatFieldMapperTests.java

@@ -56,6 +56,25 @@ protected void minimalMapping(XContentBuilder b) throws IOException {
         b.field("type", "scaled_float").field("scaling_factor", 10.0);
     }
 
+    @Override
+    protected void registerParameters(ParameterChecker checker) throws IOException {
+        checker.registerConflictCheck(
+            "scaling_factor",
+            fieldMapping(this::minimalMapping),
+            fieldMapping(b -> {
+                b.field("type", "scaled_float");
+                b.field("scaling_factor", 5.0);
+            }));
+        checker.registerConflictCheck("doc_values", b -> b.field("doc_values", false));
+        checker.registerConflictCheck("index", b -> b.field("index", false));
+        checker.registerConflictCheck("store", b -> b.field("store", true));
+        checker.registerConflictCheck("null_value", b -> b.field("null_value", 1));
+        checker.registerUpdateCheck(b -> b.field("coerce", false),
+            m -> assertFalse(((ScaledFloatFieldMapper) m).coerce()));
+        checker.registerUpdateCheck(b -> b.field("ignore_malformed", true),
+            m -> assertTrue(((ScaledFloatFieldMapper) m).ignoreMalformed()));
+    }
+
     public void testExistsQueryDocValuesDisabled() throws IOException {
         MapperService mapperService = createMapperService(fieldMapping(b -> {
             minimalMapping(b);

modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/SearchAsYouTypeFieldMapperTests.java

@@ -34,8 +34,6 @@
 import org.apache.lucene.search.Query;
 import org.apache.lucene.search.SynonymQuery;
 import org.apache.lucene.search.TermQuery;
-import org.apache.lucene.search.similarities.BM25Similarity;
-import org.apache.lucene.search.similarities.BooleanSimilarity;
 import org.apache.lucene.search.spans.FieldMaskingSpanQuery;
 import org.apache.lucene.search.spans.SpanNearQuery;
 import org.apache.lucene.search.spans.SpanTermQuery;
@@ -55,9 +53,7 @@
 import org.elasticsearch.index.query.MatchPhraseQueryBuilder;
 import org.elasticsearch.index.query.MultiMatchQueryBuilder;
 import org.elasticsearch.index.query.QueryShardContext;
-import org.elasticsearch.index.similarity.SimilarityProvider;
 import org.elasticsearch.plugins.Plugin;
-import org.junit.Before;
 
 import java.io.IOException;
 import java.util.ArrayList;
@@ -80,20 +76,53 @@
 public class SearchAsYouTypeFieldMapperTests extends FieldMapperTestCase2<SearchAsYouTypeFieldMapper.Builder> {
 
     @Override
-    protected void writeFieldValue(XContentBuilder builder) throws IOException {
-        builder.value("new york city");
+    protected void registerParameters(ParameterChecker checker) throws IOException {
+        checker.registerConflictCheck("max_shingle_size", b -> b.field("max_shingle_size", 4));
+        checker.registerConflictCheck("similarity", b -> b.field("similarity", "boolean"));
+        checker.registerConflictCheck("index", b -> b.field("index", false));
+        checker.registerConflictCheck("store", b -> b.field("store", true));
+        checker.registerConflictCheck("analyzer", b -> b.field("analyzer", "keyword"));
+        checker.registerConflictCheck("index_options", b -> b.field("index_options", "docs"));
+        checker.registerConflictCheck("term_vector", b -> b.field("term_vector", "yes"));
+
+        // norms can be set from true to false, but not vice versa
+        checker.registerConflictCheck("norms",
+            fieldMapping(b -> {
+                b.field("type", "text");
+                b.field("norms", false);
+            }),
+            fieldMapping(b -> {
+                b.field("type", "text");
+                b.field("norms", true);
+            }));
+        checker.registerUpdateCheck(
+            b -> {
+                b.field("type", "search_as_you_type");
+                b.field("norms", true);
+            },
+            b -> {
+                b.field("type", "search_as_you_type");
+                b.field("norms", false);
+            },
+            m -> assertFalse(m.fieldType().getTextSearchInfo().hasNorms())
+        );
+
+        checker.registerUpdateCheck(b -> {
+                b.field("analyzer", "default");
+                b.field("search_analyzer", "keyword");
+            },
+            m -> assertEquals("keyword", m.fieldType().getTextSearchInfo().getSearchAnalyzer().name()));
+        checker.registerUpdateCheck(b -> {
+                b.field("analyzer", "default");
+                b.field("search_analyzer", "keyword");
+                b.field("search_quote_analyzer", "keyword");
+            },
+            m -> assertEquals("keyword", m.fieldType().getTextSearchInfo().getSearchQuoteAnalyzer().name()));
+
     }
 
-    @Before
-    public void addModifiers() {
-        addModifier("max_shingle_size", false, (a, b) -> {
-            a.maxShingleSize(3);
-            b.maxShingleSize(2);
-        });
-        addModifier("similarity", false, (a, b) -> {
-            a.similarity(new SimilarityProvider("BM25", new BM25Similarity()));
-            b.similarity(new SimilarityProvider("boolean", new BooleanSimilarity()));
-        });
+    protected void writeFieldValue(XContentBuilder builder) throws IOException {
+        builder.value("new york city");
     }
 
     @Override

plugins/analysis-icu/src/main/java/org/elasticsearch/index/mapper/ICUCollationKeywordFieldMapper.java

@@ -578,51 +578,51 @@ protected void mergeOptions(FieldMapper other, List<String> conflicts) {
             conflicts.add("mapper [" + name() + "] has different [collator]");
         }
         if (!Objects.equals(rules, icuMergeWith.rules)) {
-            conflicts.add("Cannot update rules setting for [" + CONTENT_TYPE + "]");
+            conflicts.add("Cannot update parameter [rules] for [" + CONTENT_TYPE + "]");
         }
 
         if (!Objects.equals(language, icuMergeWith.language)) {
-            conflicts.add("Cannot update language setting for [" + CONTENT_TYPE + "]");
+            conflicts.add("Cannot update parameter [language] for [" + CONTENT_TYPE + "]");
         }
 
         if (!Objects.equals(country, icuMergeWith.country)) {
-            conflicts.add("Cannot update country setting for [" + CONTENT_TYPE + "]");
+            conflicts.add("Cannot update parameter [country] for [" + CONTENT_TYPE + "]");
         }
 
         if (!Objects.equals(variant, icuMergeWith.variant)) {
-            conflicts.add("Cannot update variant setting for [" + CONTENT_TYPE + "]");
+            conflicts.add("Cannot update parameter [variant] for [" + CONTENT_TYPE + "]");
         }
 
         if (!Objects.equals(strength, icuMergeWith.strength)) {
-            conflicts.add("Cannot update strength setting for [" + CONTENT_TYPE + "]");
+            conflicts.add("Cannot update parameter [strength] for [" + CONTENT_TYPE + "]");
         }
 
         if (!Objects.equals(decomposition, icuMergeWith.decomposition)) {
-            conflicts.add("Cannot update decomposition setting for [" + CONTENT_TYPE + "]");
+            conflicts.add("Cannot update parameter [decomposition] for [" + CONTENT_TYPE + "]");
         }
 
         if (!Objects.equals(alternate, icuMergeWith.alternate)) {
-            conflicts.add("Cannot update alternate setting for [" + CONTENT_TYPE + "]");
+            conflicts.add("Cannot update parameter [alternate] for [" + CONTENT_TYPE + "]");
         }
 
         if (caseLevel != icuMergeWith.caseLevel) {
-            conflicts.add("Cannot update case_level setting for [" + CONTENT_TYPE + "]");
+            conflicts.add("Cannot update parameter [case_level] for [" + CONTENT_TYPE + "]");
         }
 
         if (!Objects.equals(caseFirst, icuMergeWith.caseFirst)) {
-            conflicts.add("Cannot update case_first setting for [" + CONTENT_TYPE + "]");
+            conflicts.add("Cannot update parameter [case_first] for [" + CONTENT_TYPE + "]");
         }
 
         if (numeric != icuMergeWith.numeric) {
-            conflicts.add("Cannot update numeric setting for [" + CONTENT_TYPE + "]");
+            conflicts.add("Cannot update parameter [numeric] for [" + CONTENT_TYPE + "]");
         }
 
         if (!Objects.equals(variableTop, icuMergeWith.variableTop)) {
-            conflicts.add("Cannot update variable_top setting for [" + CONTENT_TYPE + "]");
+            conflicts.add("Cannot update parameter [variable_top] for [" + CONTENT_TYPE + "]");
         }
 
         if (hiraganaQuaternaryMode != icuMergeWith.hiraganaQuaternaryMode) {
-            conflicts.add("Cannot update hiragana_quaternary_mode setting for [" + CONTENT_TYPE + "]");
+            conflicts.add("Cannot update parameter [hiragana_quaternary_mode] for [" + CONTENT_TYPE + "]");
         }
 
         this.ignoreAbove = icuMergeWith.ignoreAbove;

plugins/analysis-icu/src/test/java/org/elasticsearch/index/mapper/ICUCollationKeywordFieldMapperTests.java

@@ -34,7 +34,6 @@
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.plugin.analysis.icu.AnalysisICUPlugin;
 import org.elasticsearch.plugins.Plugin;
-import org.junit.Before;
 
 import java.io.IOException;
 import java.util.Arrays;
@@ -63,31 +62,16 @@ protected Set<String> unsupportedProperties() {
         return org.elasticsearch.common.collect.Set.of("analyzer", "similarity");
     }
 
-    @Before
-    public void setup() {
-        addModifier("strength", false, (a, b) -> {
-            a.strength("primary");
-            b.strength("secondary");
-        });
-        addModifier("decomposition", false, (a, b) -> {
-            a.decomposition("no");
-            b.decomposition("canonical");
-        });
-        addModifier("alternate", false, (a, b) -> {
-            a.alternate("shifted");
-            b.alternate("non-ignorable");
-        });
-        addBooleanModifier("case_level", false, ICUCollationKeywordFieldMapper.Builder::caseLevel);
-        addModifier("case_first", false, (a, b) -> {
-            a.caseFirst("upper");
-            a.caseFirst("lower");
-        });
-        addBooleanModifier("numeric", false, ICUCollationKeywordFieldMapper.Builder::numeric);
-        addModifier("variable_top", false, (a, b) -> {
-            a.variableTop(";");
-            b.variableTop(":");
-        });
-        addBooleanModifier("hiragana_quaternary_mode", false, ICUCollationKeywordFieldMapper.Builder::hiraganaQuaternaryMode);
+    @Override
+    protected void registerParameters(ParameterChecker checker) throws IOException {
+        checker.registerConflictCheck("strength", b -> b.field("strength", "secondary"));
+        checker.registerConflictCheck("decomposition", b -> b.field("decomposition", "canonical"));
+        checker.registerConflictCheck("alternate", b -> b.field("alternate", "non-ignorable"));
+        checker.registerConflictCheck("case_level", b -> b.field("case_level", true));
+        checker.registerConflictCheck("case_first", b -> b.field("case_first", "lower"));
+        checker.registerConflictCheck("numeric", b -> b.field("numeric", true));
+        checker.registerConflictCheck("variable_top", b -> b.field("variable_top", ":"));
+        checker.registerConflictCheck("hiragana_quaternary_mode", b -> b.field("hiragana_quaternary_mode", true));
     }
 
     @Override

plugins/mapper-murmur3/src/test/java/org/elasticsearch/index/mapper/murmur3/Murmur3FieldMapperTests.java

@@ -59,6 +59,11 @@ protected void minimalMapping(XContentBuilder b) throws IOException {
         b.field("type", "murmur3");
     }
 
+    @Override
+    protected void registerParameters(ParameterChecker checker) {
+        // no parameters to configure
+    }
+
     public void testDefaults() throws Exception {
         DocumentMapper mapper = createDocumentMapper(fieldMapping(this::minimalMapping));
         ParsedDocument parsedDoc = mapper.parse(source(b -> b.field("field", "value")));

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

@@ -134,6 +134,7 @@ public final boolean parsesArrayValue() {
 
     @Override
     protected void mergeOptions(FieldMapper other, List<String> conflicts) {
+        super.mergeOptions(other, conflicts);
         AbstractPointGeometryFieldMapper gpfm = (AbstractPointGeometryFieldMapper)other;
         // TODO make this un-updateable
         if (gpfm.nullValue != null) {

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

@@ -188,6 +188,7 @@ public final boolean parsesArrayValue() {
 
     @Override
     protected final void mergeOptions(FieldMapper other, List<String> conflicts) {
+        super.mergeOptions(other, conflicts);
         AbstractShapeGeometryFieldMapper gsfm = (AbstractShapeGeometryFieldMapper)other;
         if (gsfm.coerce.explicit()) {
             this.coerce = gsfm.coerce;

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

@@ -137,7 +137,7 @@ public static class Builder extends ParametrizedFieldMapper.Builder {
                 b.startArray(n);
                 c.toXContent(b, ToXContent.EMPTY_PARAMS);
                 b.endArray();
-            }, ContextMappings::toString);
+            }, Objects::toString);
         private final Parameter<Integer> maxInputLength = Parameter.intParam("max_input_length", true,
             m -> toType(m).maxInputLength, Defaults.DEFAULT_MAX_INPUT_LENGTH)
             .addDeprecatedName("max_input_len")
@@ -351,6 +351,10 @@ public boolean parsesArrayValue() {
         return true;
     }
 
+    int getMaxInputLength() {
+        return maxInputLength;
+    }
+
     /**
      * Parses and indexes inputs
      *

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

@@ -400,7 +400,7 @@ private void mergeSharedOptions(FieldMapper mergeWith, List<String> conflicts) {
             conflicts.add("mapper [" + name() + "] has different [norms] values, cannot change from disable to enabled");
         }
         if (fieldType.storeTermVectors() != other.storeTermVectors()) {
-            conflicts.add("mapper [" + name() + "] has different [store_term_vector] values");
+            conflicts.add("mapper [" + name() + "] has different [term_vector] values");
         }
         if (fieldType.storeTermVectorOffsets() != other.storeTermVectorOffsets()) {
             conflicts.add("mapper [" + name() + "] has different [store_term_vector_offsets] values");

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

@@ -345,6 +345,10 @@ private IpFieldMapper(
         this.indexCreatedVersion = builder.indexCreatedVersion;
     }
 
+    boolean ignoreMalformed() {
+        return ignoreMalformed;
+    }
+
     @Override
     public IpFieldType fieldType() {
         return (IpFieldType) super.fieldType();

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

@@ -92,7 +92,8 @@ public static class Builder extends ParametrizedFieldMapper.Builder {
         private final Parameter<String> indexOptions
             = Parameter.restrictedStringParam("index_options", false, m -> toType(m).indexOptions, "docs", "freqs");
         private final Parameter<Boolean> hasNorms
-            = Parameter.boolParam("norms", false, m -> toType(m).fieldType.omitNorms() == false, false);
+            = Parameter.boolParam("norms", true, m -> toType(m).fieldType.omitNorms() == false, false)
+            .setMergeValidator((o, n) -> o == n || (o && n == false));  // norms can be updated from 'true' to 'false' but not vv
         private final Parameter<SimilarityProvider> similarity = new Parameter<>("similarity", false, () -> null,
             (n, c, o) -> TypeParsers.resolveSimilarity(c, n, o), m -> toType(m).similarity)
             .setSerializer((b, f, v) -> b.field(f, v == null ? null : v.name()), v -> v == null ? null : v.name())

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

@@ -1014,6 +1014,14 @@ private NumberFieldMapper(
         this.coerceByDefault = builder.coerce.getDefaultValue().value();
     }
 
+    boolean coerce() {
+        return coerce.value();
+    }
+
+    boolean ignoreMalformed() {
+        return ignoreMalformed.value();
+    }
+
     @Override
     public NumberFieldType fieldType() {
         return (NumberFieldType) super.fieldType();

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

@@ -144,7 +144,6 @@ public static final class Parameter<T> {
         private final Supplier<T> defaultValue;
         private final TriFunction<String, ParserContext, Object, T> parser;
         private final Function<FieldMapper, T> initializer;
-        private final boolean updateable;
         private boolean acceptsNull = false;
         private Consumer<T> validator = null;
         private Serializer<T> serializer = XContentBuilder::field;
@@ -169,7 +168,6 @@ public Parameter(String name, boolean updateable, Supplier<T> defaultValue,
             this.value = null;
             this.parser = parser;
             this.initializer = initializer;
-            this.updateable = updateable;
             this.mergeValidator = (previous, toMerge) -> updateable || Objects.equals(previous, toMerge);
         }
 

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

@@ -274,6 +274,10 @@ private RangeFieldMapper(
         this.coerceByDefault = builder.coerce.getDefaultValue().value();
     }
 
+    boolean coerce() {
+        return coerce.value();
+    }
+
     @Override
     public ParametrizedFieldMapper.Builder getMergeBuilder() {
         return new Builder(simpleName(), type, coerceByDefault).init(this);