Convert KeywordFieldMapper to parametrized form

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

@@ -149,8 +149,6 @@ private static boolean getMapUnmappedFieldAsText(Settings indexSettings) {
         static KeywordFieldMapper createExtractQueryFieldBuilder(String name, BuilderContext context) {
             KeywordFieldMapper.Builder queryMetadataFieldBuilder = new KeywordFieldMapper.Builder(name);
             queryMetadataFieldBuilder.docValues(false);
-            queryMetadataFieldBuilder.store(false);
-            queryMetadataFieldBuilder.indexOptions(IndexOptions.DOCS);
             return queryMetadataFieldBuilder.build(context);
         }
 

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

@@ -31,9 +31,11 @@
 
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Iterator;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -314,6 +316,28 @@ public static Parameter<List<String>> stringArrayParam(String name, boolean upda
                 }, initializer);
         }
 
+        /**
+         * Defines a parameter that takes one of a restricted set of string values
+         * @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 values        the set of values that the parameter can take.  The first value in the list
+         *                      is the default value, to be used if the parameter is undefined in a mapping
+         */
+        public static Parameter<String> restrictedStringParam(String name, boolean updateable,
+                                                              Function<FieldMapper, String> initializer, String... values) {
+            assert values.length > 0;
+            Set<String> acceptedValues = new LinkedHashSet<>(Arrays.asList(values));
+            return stringParam(name, updateable, initializer, values[0])
+                .setValidator(v -> {
+                   if (acceptedValues.contains(v)) {
+                       return;
+                   }
+                   throw new MapperParsingException("Unknown value [" + v + "] for field [" + name +
+                       "] - accepted values are " + acceptedValues.toString());
+                });
+        }
+
         /**
          * Defines a parameter that takes an analyzer name
          * @param name              the parameter name

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

@@ -388,10 +388,11 @@ private static void validateDynamicTemplate(Mapper.TypeParser.ParserContext pars
                 continue;
             }
 
-            Map<String, Object> fieldTypeConfig = dynamicTemplate.mappingForName("__dummy__", defaultDynamicType);
-            fieldTypeConfig.remove("type");
+            String templateName = "__dynamic__" + dynamicTemplate.name();
+            Map<String, Object> fieldTypeConfig = dynamicTemplate.mappingForName(templateName, defaultDynamicType);
             try {
-                Mapper.Builder<?> dummyBuilder = typeParser.parse("__dummy__", fieldTypeConfig, parserContext);
+                Mapper.Builder<?> dummyBuilder = typeParser.parse(templateName, fieldTypeConfig, parserContext);
+                fieldTypeConfig.remove("type");
                 if (fieldTypeConfig.isEmpty()) {
                     Settings indexSettings = parserContext.mapperService().getIndexSettings().getSettings();
                     BuilderContext builderContext = new BuilderContext(indexSettings, new ContentPath(1));

server/src/main/java/org/elasticsearch/indices/IndicesModule.java

@@ -118,7 +118,7 @@ public static Map<String, Mapper.TypeParser> getMappers(List<MapperPlugin> mappe
         mappers.put(nanoseconds.type(), DateFieldMapper.NANOS_PARSER);
         mappers.put(IpFieldMapper.CONTENT_TYPE, IpFieldMapper.PARSER);
         mappers.put(TextFieldMapper.CONTENT_TYPE, new TextFieldMapper.TypeParser());
-        mappers.put(KeywordFieldMapper.CONTENT_TYPE, new KeywordFieldMapper.TypeParser());
+        mappers.put(KeywordFieldMapper.CONTENT_TYPE, KeywordFieldMapper.PARSER);
         mappers.put(ObjectMapper.CONTENT_TYPE, new ObjectMapper.TypeParser());
         mappers.put(ObjectMapper.NESTED_CONTENT_TYPE, new ObjectMapper.TypeParser());
         mappers.put(CompletionFieldMapper.CONTENT_TYPE, CompletionFieldMapper.PARSER);

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

@@ -113,7 +113,7 @@ public void testExternalValuesWithMultifield() throws Exception {
         Map<String, Mapper.TypeParser> mapperParsers = new HashMap<>();
         mapperParsers.put(ExternalMapperPlugin.EXTERNAL, new ExternalMapper.TypeParser(ExternalMapperPlugin.EXTERNAL, "foo"));
         mapperParsers.put(TextFieldMapper.CONTENT_TYPE, new TextFieldMapper.TypeParser());
-        mapperParsers.put(KeywordFieldMapper.CONTENT_TYPE, new KeywordFieldMapper.TypeParser());
+        mapperParsers.put(KeywordFieldMapper.CONTENT_TYPE, KeywordFieldMapper.PARSER);
         MapperRegistry mapperRegistry = new MapperRegistry(mapperParsers, Collections.emptyMap(), MapperPlugin.NOOP_FIELD_FILTER);
 
         Supplier<QueryShardContext> queryShardContext = () -> {

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

@@ -26,8 +26,6 @@
 import org.apache.lucene.index.IndexOptions;
 import org.apache.lucene.index.IndexableField;
 import org.apache.lucene.index.IndexableFieldType;
-import org.apache.lucene.search.similarities.BM25Similarity;
-import org.apache.lucene.search.similarities.BooleanSimilarity;
 import org.apache.lucene.util.BytesRef;
 import org.elasticsearch.Version;
 import org.elasticsearch.cluster.metadata.IndexMetadata;
@@ -41,12 +39,12 @@
 import org.elasticsearch.index.analysis.PreConfiguredTokenFilter;
 import org.elasticsearch.index.analysis.TokenizerFactory;
 import org.elasticsearch.index.mapper.MapperService.MergeReason;
-import org.elasticsearch.index.similarity.SimilarityProvider;
 import org.elasticsearch.index.termvectors.TermVectorsService;
 import org.elasticsearch.indices.analysis.AnalysisModule;
 import org.elasticsearch.plugins.AnalysisPlugin;
 import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.search.lookup.SourceLookup;
+import org.elasticsearch.test.ESSingleNodeTestCase;
 import org.elasticsearch.test.InternalSettingsPlugin;
 import org.junit.Before;
 
@@ -56,7 +54,6 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 
 import static java.util.Collections.singletonList;
 import static java.util.Collections.singletonMap;
@@ -65,17 +62,7 @@
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.instanceOf;
 
-public class KeywordFieldMapperTests extends FieldMapperTestCase<KeywordFieldMapper.Builder> {
-
-    @Override
-    protected KeywordFieldMapper.Builder newBuilder() {
-        return new KeywordFieldMapper.Builder("keyword");
-    }
-
-    @Override
-    protected Set<String> unsupportedProperties() {
-        return Set.of("analyzer");
-    }
+public class KeywordFieldMapperTests extends ESSingleNodeTestCase {
 
     /**
      * Creates a copy of the lowercase token filter which we use for testing merge errors.
@@ -99,11 +86,6 @@ protected Collection<Class<? extends Plugin>> getPlugins() {
         return pluginList(InternalSettingsPlugin.class, MockAnalysisPlugin.class);
     }
 
-    @Override
-    protected Settings getIndexMapperSettings() {
-        return mapperSettings;
-    }
-
     private static final Settings mapperSettings = Settings.builder()
         .put("index.analysis.normalizer.my_lowercase.type", "custom")
         .putList("index.analysis.normalizer.my_lowercase.filter", "lowercase")
@@ -117,18 +99,6 @@ protected Settings getIndexMapperSettings() {
     public void setup() {
         indexService = createIndex("test", mapperSettings);
         parser = indexService.mapperService().documentMapperParser();
-        addModifier("normalizer", false, (a, b) -> {
-            a.normalizer(indexService.getIndexAnalyzers(), "my_lowercase");
-        });
-        addBooleanModifier("split_queries_on_whitespace", true, KeywordFieldMapper.Builder::splitQueriesOnWhitespace);
-        addModifier("index_options", false, (a, b) -> {
-            a.indexOptions(IndexOptions.DOCS);
-            b.indexOptions(IndexOptions.DOCS_AND_FREQS);
-        });
-        addModifier("similarity", false, (a, b) -> {
-            a.similarity(new SimilarityProvider("BM25", new BM25Similarity()));
-            b.similarity(new SimilarityProvider("boolean", new BooleanSimilarity()));
-        });
     }
 
     public void testDefaults() throws Exception {
@@ -339,16 +309,17 @@ public void testIndexOptions() throws IOException {
                     .startObject("properties").startObject("field").field("type", "keyword")
                     .field("index_options", indexOptions).endObject().endObject()
                     .endObject().endObject());
-            IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
+            MapperParsingException e = expectThrows(MapperParsingException.class,
                     () -> parser.parse("type", new CompressedXContent(mapping2)));
-            assertEquals("The [keyword] field does not support positions, got [index_options]=" + indexOptions, e.getMessage());
+            assertEquals("Unknown value [" + indexOptions + "] for field [index_options] - accepted values are [docs, freqs]",
+                e.getMessage());
         }
     }
 
     public void testBoost() throws IOException {
         String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type")
-                .startObject("properties").startObject("field").field("type", "keyword").field("boost", 2f).endObject().endObject()
-                .endObject().endObject());
+            .startObject("properties").startObject("field").field("type", "keyword").field("boost", 2f).endObject().endObject()
+            .endObject().endObject());
 
         DocumentMapper mapper = parser.parse("type", new CompressedXContent(mapping));
 
@@ -524,8 +495,8 @@ public void testUpdateNormalizer() throws IOException {
                 () -> indexService.mapperService().merge("type",
                         new CompressedXContent(mapping2), MergeReason.MAPPING_UPDATE));
         assertEquals(
-                "Mapper for [field] conflicts with existing mapping:\n" +
-                    "[mapper [field] has different [analyzer], mapper [field] has different [normalizer]]",
+                "Mapper for [field] conflicts with existing mapper:\n" +
+                    "\tCannot update parameter [normalizer] from [my_lowercase] to [my_other_lowercase]",
                 e.getMessage());
     }
 
@@ -650,8 +621,8 @@ public void testParseSourceValue() {
         assertEquals("42", ignoreAboveMapper.parseSourceValue(42L, null));
         assertEquals("true", ignoreAboveMapper.parseSourceValue(true, null));
 
-        KeywordFieldMapper normalizerMapper = new KeywordFieldMapper.Builder("field")
-            .normalizer(indexService.getIndexAnalyzers(), "lowercase")
+        KeywordFieldMapper normalizerMapper = new KeywordFieldMapper.Builder("field", indexService.getIndexAnalyzers())
+            .normalizer("lowercase")
             .build(context);
         assertEquals("value", normalizerMapper.parseSourceValue("VALUE", null));
         assertEquals("42", normalizerMapper.parseSourceValue(42L, null));

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

@@ -121,14 +121,16 @@ public static class Builder extends ParametrizedFieldMapper.Builder {
                     throw new IllegalArgumentException("field [required] must be specified");
                 }
             });
+        final Parameter<String> restricted
+            = Parameter.restrictedStringParam("restricted", true, m -> toType(m).restricted, "foo", "bar");
 
         protected Builder(String name) {
             super(name);
         }
 
         @Override
         protected List<Parameter<?>> getParameters() {
-            return List.of(fixed, fixed2, variable, index, wrapper, intValue, analyzer, searchAnalyzer, required);
+            return List.of(fixed, fixed2, variable, index, wrapper, intValue, analyzer, searchAnalyzer, required, restricted);
         }
 
         @Override
@@ -159,6 +161,7 @@ public static class TestMapper extends ParametrizedFieldMapper {
         private final NamedAnalyzer searchAnalyzer;
         private final boolean index;
         private final String required;
+        private final String restricted;
 
         protected TestMapper(String simpleName, String fullName, MultiFields multiFields, CopyTo copyTo,
                              ParametrizedMapperTests.Builder builder) {
@@ -172,6 +175,7 @@ protected TestMapper(String simpleName, String fullName, MultiFields multiFields
             this.searchAnalyzer = builder.searchAnalyzer.getValue();
             this.index = builder.index.getValue();
             this.required = builder.required.getValue();
+            this.restricted = builder.restricted.getValue();
         }
 
         @Override
@@ -205,7 +209,7 @@ private static TestMapper fromMapping(String mapping, Version version) {
         when(mapperService.getIndexAnalyzers()).thenReturn(indexAnalyzers);
         Mapper.TypeParser.ParserContext pc = new Mapper.TypeParser.ParserContext(s -> null, mapperService, s -> {
             if (Objects.equals("keyword", s)) {
-                return new KeywordFieldMapper.TypeParser();
+                return KeywordFieldMapper.PARSER;
             }
             if (Objects.equals("binary", s)) {
                 return BinaryFieldMapper.PARSER;
@@ -239,7 +243,7 @@ public void testDefaults() throws IOException {
         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\"}}",
+                "\"search_analyzer\":\"_keyword\",\"required\":\"value\",\"restricted\":\"foo\"}}",
             Strings.toString(builder));
     }
 
@@ -440,4 +444,22 @@ public void testRequiredField() {
             assertEquals("[required] on mapper [field] of type [test_mapper] must not have a [null] value", exc.getMessage());
         }
     }
+
+    public void testRestrictedField() {
+        {
+            String mapping = "{\"type\":\"test_mapper\",\"required\":\"a\",\"restricted\":\"baz\"}";
+            MapperParsingException e = expectThrows(MapperParsingException.class, () -> fromMapping(mapping));
+            assertEquals("Unknown value [baz] for field [restricted] - accepted values are [foo, bar]", e.getMessage());
+        }
+        {
+            String mapping = "{\"type\":\"test_mapper\",\"required\":\"a\",\"restricted\":\"bar\"}";
+            TestMapper mapper = fromMapping(mapping);
+            assertEquals("bar", mapper.restricted);
+        }
+        {
+            String mapping = "{\"type\":\"test_mapper\",\"required\":\"a\"}";
+            TestMapper mapper = fromMapping(mapping);
+            assertEquals("foo", mapper.restricted);
+        }
+    }
 }

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

@@ -340,8 +340,9 @@ public void testIllegalDynamicTemplateUnknownAttribute() throws Exception {
         MapperService mapperService = createIndex("test").mapperService();
         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 [__dynamic__my_template] of type [keyword]"));
     }
 
     public void testIllegalDynamicTemplateInvalidAttribute() throws Exception {
@@ -369,7 +370,7 @@ public void testIllegalDynamicTemplateInvalidAttribute() throws Exception {
         MapperParsingException e = expectThrows(MapperParsingException.class,
             () -> mapperService.merge("type", new CompressedXContent(Strings.toString(mapping)), MergeReason.MAPPING_UPDATE));
         assertThat(e.getRootCause(), instanceOf(MapperParsingException.class));
-        assertThat(e.getRootCause().getMessage(), equalTo("analyzer [foobar] not found for field [__dummy__]"));
+        assertThat(e.getRootCause().getMessage(), equalTo("analyzer [foobar] not found for field [__dynamic__my_template]"));
     }
 
     public void testIllegalDynamicTemplateNoMappingType() throws Exception {
@@ -433,7 +434,8 @@ public void testIllegalDynamicTemplateNoMappingType() throws Exception {
             MapperParsingException e = expectThrows(MapperParsingException.class,
                 () -> mapperService.merge("type", new CompressedXContent(Strings.toString(mapping)), MergeReason.MAPPING_UPDATE));
             assertThat(e.getRootCause(), instanceOf(MapperParsingException.class));
-            assertThat(e.getRootCause().getMessage(), equalTo("unknown parameter [foo] on mapper [__dummy__] of type [null]"));
+            assertThat(e.getRootCause().getMessage(),
+                equalTo("unknown parameter [foo] on mapper [__dynamic__my_template] of type [binary]"));
         }
     }