Skip to content

Commit 2ef5902

Browse files
romseygeekromseygeek
authored
Convert KeywordFieldMapper to parametrized form (#60645)
This makes KeywordFieldMapper extend ParametrizedFieldMapper, with explicitly defined parameters. In addition, we add a new option to Parameter, restrictedStringParam, which accepts a restricted set of string options.
1 parent 9264a0f commit 2ef5902

File tree

11 files changed

+201
-251
lines changed

11 files changed

+201
-251
lines changed

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,8 +149,6 @@ private static boolean getMapUnmappedFieldAsText(Settings indexSettings) {
149149
static KeywordFieldMapper createExtractQueryFieldBuilder(String name, BuilderContext context) {
150150
KeywordFieldMapper.Builder queryMetadataFieldBuilder = new KeywordFieldMapper.Builder(name);
151151
queryMetadataFieldBuilder.docValues(false);
152-
queryMetadataFieldBuilder.store(false);
153-
queryMetadataFieldBuilder.indexOptions(IndexOptions.DOCS);
154152
return queryMetadataFieldBuilder.build(context);
155153
}
156154

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

Lines changed: 110 additions & 175 deletions
Large diffs are not rendered by default.

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,11 @@
3131

3232
import java.io.IOException;
3333
import java.util.ArrayList;
34+
import java.util.Arrays;
3435
import java.util.Collections;
3536
import java.util.HashMap;
3637
import java.util.Iterator;
38+
import java.util.LinkedHashSet;
3739
import java.util.List;
3840
import java.util.Map;
3941
import java.util.Objects;
@@ -314,6 +316,28 @@ public static Parameter<List<String>> stringArrayParam(String name, boolean upda
314316
}, initializer);
315317
}
316318

319+
/**
320+
* Defines a parameter that takes one of a restricted set of string values
321+
* @param name the parameter name
322+
* @param updateable whether the parameter can be changed by a mapping update
323+
* @param initializer a function that reads the parameter value from an existing mapper
324+
* @param values the set of values that the parameter can take. The first value in the list
325+
* is the default value, to be used if the parameter is undefined in a mapping
326+
*/
327+
public static Parameter<String> restrictedStringParam(String name, boolean updateable,
328+
Function<FieldMapper, String> initializer, String... values) {
329+
assert values.length > 0;
330+
Set<String> acceptedValues = new LinkedHashSet<>(Arrays.asList(values));
331+
return stringParam(name, updateable, initializer, values[0])
332+
.setValidator(v -> {
333+
if (acceptedValues.contains(v)) {
334+
return;
335+
}
336+
throw new MapperParsingException("Unknown value [" + v + "] for field [" + name +
337+
"] - accepted values are " + acceptedValues.toString());
338+
});
339+
}
340+
317341
/**
318342
* Defines a parameter that takes an analyzer name
319343
* @param name the parameter name

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -388,10 +388,11 @@ private static void validateDynamicTemplate(Mapper.TypeParser.ParserContext pars
388388
continue;
389389
}
390390

391-
Map<String, Object> fieldTypeConfig = dynamicTemplate.mappingForName("__dummy__", defaultDynamicType);
392-
fieldTypeConfig.remove("type");
391+
String templateName = "__dynamic__" + dynamicTemplate.name();
392+
Map<String, Object> fieldTypeConfig = dynamicTemplate.mappingForName(templateName, defaultDynamicType);
393393
try {
394-
Mapper.Builder<?> dummyBuilder = typeParser.parse("__dummy__", fieldTypeConfig, parserContext);
394+
Mapper.Builder<?> dummyBuilder = typeParser.parse(templateName, fieldTypeConfig, parserContext);
395+
fieldTypeConfig.remove("type");
395396
if (fieldTypeConfig.isEmpty()) {
396397
Settings indexSettings = parserContext.mapperService().getIndexSettings().getSettings();
397398
BuilderContext builderContext = new BuilderContext(indexSettings, new ContentPath(1));

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ public static Map<String, Mapper.TypeParser> getMappers(List<MapperPlugin> mappe
118118
mappers.put(nanoseconds.type(), DateFieldMapper.NANOS_PARSER);
119119
mappers.put(IpFieldMapper.CONTENT_TYPE, IpFieldMapper.PARSER);
120120
mappers.put(TextFieldMapper.CONTENT_TYPE, new TextFieldMapper.TypeParser());
121-
mappers.put(KeywordFieldMapper.CONTENT_TYPE, new KeywordFieldMapper.TypeParser());
121+
mappers.put(KeywordFieldMapper.CONTENT_TYPE, KeywordFieldMapper.PARSER);
122122
mappers.put(ObjectMapper.CONTENT_TYPE, new ObjectMapper.TypeParser());
123123
mappers.put(ObjectMapper.NESTED_CONTENT_TYPE, new ObjectMapper.TypeParser());
124124
mappers.put(CompletionFieldMapper.CONTENT_TYPE, CompletionFieldMapper.PARSER);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ public void testExternalValuesWithMultifield() throws Exception {
113113
Map<String, Mapper.TypeParser> mapperParsers = new HashMap<>();
114114
mapperParsers.put(ExternalMapperPlugin.EXTERNAL, new ExternalMapper.TypeParser(ExternalMapperPlugin.EXTERNAL, "foo"));
115115
mapperParsers.put(TextFieldMapper.CONTENT_TYPE, new TextFieldMapper.TypeParser());
116-
mapperParsers.put(KeywordFieldMapper.CONTENT_TYPE, new KeywordFieldMapper.TypeParser());
116+
mapperParsers.put(KeywordFieldMapper.CONTENT_TYPE, KeywordFieldMapper.PARSER);
117117
MapperRegistry mapperRegistry = new MapperRegistry(mapperParsers, Collections.emptyMap(), MapperPlugin.NOOP_FIELD_FILTER);
118118

119119
Supplier<QueryShardContext> queryShardContext = () -> {

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

Lines changed: 11 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,6 @@
2626
import org.apache.lucene.index.IndexOptions;
2727
import org.apache.lucene.index.IndexableField;
2828
import org.apache.lucene.index.IndexableFieldType;
29-
import org.apache.lucene.search.similarities.BM25Similarity;
30-
import org.apache.lucene.search.similarities.BooleanSimilarity;
3129
import org.apache.lucene.util.BytesRef;
3230
import org.elasticsearch.Version;
3331
import org.elasticsearch.cluster.metadata.IndexMetadata;
@@ -41,12 +39,12 @@
4139
import org.elasticsearch.index.analysis.PreConfiguredTokenFilter;
4240
import org.elasticsearch.index.analysis.TokenizerFactory;
4341
import org.elasticsearch.index.mapper.MapperService.MergeReason;
44-
import org.elasticsearch.index.similarity.SimilarityProvider;
4542
import org.elasticsearch.index.termvectors.TermVectorsService;
4643
import org.elasticsearch.indices.analysis.AnalysisModule;
4744
import org.elasticsearch.plugins.AnalysisPlugin;
4845
import org.elasticsearch.plugins.Plugin;
4946
import org.elasticsearch.search.lookup.SourceLookup;
47+
import org.elasticsearch.test.ESSingleNodeTestCase;
5048
import org.elasticsearch.test.InternalSettingsPlugin;
5149
import org.junit.Before;
5250

@@ -56,7 +54,6 @@
5654
import java.util.Collections;
5755
import java.util.List;
5856
import java.util.Map;
59-
import java.util.Set;
6057

6158
import static java.util.Collections.singletonList;
6259
import static java.util.Collections.singletonMap;
@@ -65,17 +62,7 @@
6562
import static org.hamcrest.Matchers.equalTo;
6663
import static org.hamcrest.Matchers.instanceOf;
6764

68-
public class KeywordFieldMapperTests extends FieldMapperTestCase<KeywordFieldMapper.Builder> {
69-
70-
@Override
71-
protected KeywordFieldMapper.Builder newBuilder() {
72-
return new KeywordFieldMapper.Builder("keyword");
73-
}
74-
75-
@Override
76-
protected Set<String> unsupportedProperties() {
77-
return Set.of("analyzer");
78-
}
65+
public class KeywordFieldMapperTests extends ESSingleNodeTestCase {
7966

8067
/**
8168
* 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() {
9986
return pluginList(InternalSettingsPlugin.class, MockAnalysisPlugin.class);
10087
}
10188

102-
@Override
103-
protected Settings getIndexMapperSettings() {
104-
return mapperSettings;
105-
}
106-
10789
private static final Settings mapperSettings = Settings.builder()
10890
.put("index.analysis.normalizer.my_lowercase.type", "custom")
10991
.putList("index.analysis.normalizer.my_lowercase.filter", "lowercase")
@@ -117,18 +99,6 @@ protected Settings getIndexMapperSettings() {
11799
public void setup() {
118100
indexService = createIndex("test", mapperSettings);
119101
parser = indexService.mapperService().documentMapperParser();
120-
addModifier("normalizer", false, (a, b) -> {
121-
a.normalizer(indexService.getIndexAnalyzers(), "my_lowercase");
122-
});
123-
addBooleanModifier("split_queries_on_whitespace", true, KeywordFieldMapper.Builder::splitQueriesOnWhitespace);
124-
addModifier("index_options", false, (a, b) -> {
125-
a.indexOptions(IndexOptions.DOCS);
126-
b.indexOptions(IndexOptions.DOCS_AND_FREQS);
127-
});
128-
addModifier("similarity", false, (a, b) -> {
129-
a.similarity(new SimilarityProvider("BM25", new BM25Similarity()));
130-
b.similarity(new SimilarityProvider("boolean", new BooleanSimilarity()));
131-
});
132102
}
133103

134104
public void testDefaults() throws Exception {
@@ -339,16 +309,17 @@ public void testIndexOptions() throws IOException {
339309
.startObject("properties").startObject("field").field("type", "keyword")
340310
.field("index_options", indexOptions).endObject().endObject()
341311
.endObject().endObject());
342-
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
312+
MapperParsingException e = expectThrows(MapperParsingException.class,
343313
() -> parser.parse("type", new CompressedXContent(mapping2)));
344-
assertEquals("The [keyword] field does not support positions, got [index_options]=" + indexOptions, e.getMessage());
314+
assertEquals("Unknown value [" + indexOptions + "] for field [index_options] - accepted values are [docs, freqs]",
315+
e.getMessage());
345316
}
346317
}
347318

348319
public void testBoost() throws IOException {
349320
String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type")
350-
.startObject("properties").startObject("field").field("type", "keyword").field("boost", 2f).endObject().endObject()
351-
.endObject().endObject());
321+
.startObject("properties").startObject("field").field("type", "keyword").field("boost", 2f).endObject().endObject()
322+
.endObject().endObject());
352323

353324
DocumentMapper mapper = parser.parse("type", new CompressedXContent(mapping));
354325

@@ -524,8 +495,8 @@ public void testUpdateNormalizer() throws IOException {
524495
() -> indexService.mapperService().merge("type",
525496
new CompressedXContent(mapping2), MergeReason.MAPPING_UPDATE));
526497
assertEquals(
527-
"Mapper for [field] conflicts with existing mapping:\n" +
528-
"[mapper [field] has different [analyzer], mapper [field] has different [normalizer]]",
498+
"Mapper for [field] conflicts with existing mapper:\n" +
499+
"\tCannot update parameter [normalizer] from [my_lowercase] to [my_other_lowercase]",
529500
e.getMessage());
530501
}
531502

@@ -650,8 +621,8 @@ public void testParseSourceValue() {
650621
assertEquals("42", ignoreAboveMapper.parseSourceValue(42L, null));
651622
assertEquals("true", ignoreAboveMapper.parseSourceValue(true, null));
652623

653-
KeywordFieldMapper normalizerMapper = new KeywordFieldMapper.Builder("field")
654-
.normalizer(indexService.getIndexAnalyzers(), "lowercase")
624+
KeywordFieldMapper normalizerMapper = new KeywordFieldMapper.Builder("field", indexService.getIndexAnalyzers())
625+
.normalizer("lowercase")
655626
.build(context);
656627
assertEquals("value", normalizerMapper.parseSourceValue("VALUE", null));
657628
assertEquals("42", normalizerMapper.parseSourceValue(42L, null));

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

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -121,14 +121,16 @@ public static class Builder extends ParametrizedFieldMapper.Builder {
121121
throw new IllegalArgumentException("field [required] must be specified");
122122
}
123123
});
124+
final Parameter<String> restricted
125+
= Parameter.restrictedStringParam("restricted", true, m -> toType(m).restricted, "foo", "bar");
124126

125127
protected Builder(String name) {
126128
super(name);
127129
}
128130

129131
@Override
130132
protected List<Parameter<?>> getParameters() {
131-
return List.of(fixed, fixed2, variable, index, wrapper, intValue, analyzer, searchAnalyzer, required);
133+
return List.of(fixed, fixed2, variable, index, wrapper, intValue, analyzer, searchAnalyzer, required, restricted);
132134
}
133135

134136
@Override
@@ -159,6 +161,7 @@ public static class TestMapper extends ParametrizedFieldMapper {
159161
private final NamedAnalyzer searchAnalyzer;
160162
private final boolean index;
161163
private final String required;
164+
private final String restricted;
162165

163166
protected TestMapper(String simpleName, String fullName, MultiFields multiFields, CopyTo copyTo,
164167
ParametrizedMapperTests.Builder builder) {
@@ -172,6 +175,7 @@ protected TestMapper(String simpleName, String fullName, MultiFields multiFields
172175
this.searchAnalyzer = builder.searchAnalyzer.getValue();
173176
this.index = builder.index.getValue();
174177
this.required = builder.required.getValue();
178+
this.restricted = builder.restricted.getValue();
175179
}
176180

177181
@Override
@@ -205,7 +209,7 @@ private static TestMapper fromMapping(String mapping, Version version) {
205209
when(mapperService.getIndexAnalyzers()).thenReturn(indexAnalyzers);
206210
Mapper.TypeParser.ParserContext pc = new Mapper.TypeParser.ParserContext(s -> null, mapperService, s -> {
207211
if (Objects.equals("keyword", s)) {
208-
return new KeywordFieldMapper.TypeParser();
212+
return KeywordFieldMapper.PARSER;
209213
}
210214
if (Objects.equals("binary", s)) {
211215
return BinaryFieldMapper.PARSER;
@@ -239,7 +243,7 @@ public void testDefaults() throws IOException {
239243
assertEquals("{\"field\":{\"type\":\"test_mapper\",\"fixed\":true," +
240244
"\"fixed2\":false,\"variable\":\"default\",\"index\":true," +
241245
"\"wrapper\":\"default\",\"int_value\":5,\"analyzer\":\"_keyword\"," +
242-
"\"search_analyzer\":\"_keyword\",\"required\":\"value\"}}",
246+
"\"search_analyzer\":\"_keyword\",\"required\":\"value\",\"restricted\":\"foo\"}}",
243247
Strings.toString(builder));
244248
}
245249

@@ -440,4 +444,22 @@ public void testRequiredField() {
440444
assertEquals("[required] on mapper [field] of type [test_mapper] must not have a [null] value", exc.getMessage());
441445
}
442446
}
447+
448+
public void testRestrictedField() {
449+
{
450+
String mapping = "{\"type\":\"test_mapper\",\"required\":\"a\",\"restricted\":\"baz\"}";
451+
MapperParsingException e = expectThrows(MapperParsingException.class, () -> fromMapping(mapping));
452+
assertEquals("Unknown value [baz] for field [restricted] - accepted values are [foo, bar]", e.getMessage());
453+
}
454+
{
455+
String mapping = "{\"type\":\"test_mapper\",\"required\":\"a\",\"restricted\":\"bar\"}";
456+
TestMapper mapper = fromMapping(mapping);
457+
assertEquals("bar", mapper.restricted);
458+
}
459+
{
460+
String mapping = "{\"type\":\"test_mapper\",\"required\":\"a\"}";
461+
TestMapper mapper = fromMapping(mapping);
462+
assertEquals("foo", mapper.restricted);
463+
}
464+
}
443465
}

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -340,8 +340,9 @@ public void testIllegalDynamicTemplateUnknownAttribute() throws Exception {
340340
MapperService mapperService = createIndex("test").mapperService();
341341
MapperParsingException e = expectThrows(MapperParsingException.class,
342342
() -> mapperService.merge("type", new CompressedXContent(Strings.toString(mapping)), MergeReason.MAPPING_UPDATE));
343-
assertThat(e.getRootCause(), instanceOf(IllegalArgumentException.class));
344-
assertThat(e.getRootCause().getMessage(), equalTo("Unused mapping attributes [{foo=bar}]"));
343+
assertThat(e.getRootCause(), instanceOf(MapperParsingException.class));
344+
assertThat(e.getRootCause().getMessage(),
345+
equalTo("unknown parameter [foo] on mapper [__dynamic__my_template] of type [keyword]"));
345346
}
346347

347348
public void testIllegalDynamicTemplateInvalidAttribute() throws Exception {
@@ -369,7 +370,7 @@ public void testIllegalDynamicTemplateInvalidAttribute() throws Exception {
369370
MapperParsingException e = expectThrows(MapperParsingException.class,
370371
() -> mapperService.merge("type", new CompressedXContent(Strings.toString(mapping)), MergeReason.MAPPING_UPDATE));
371372
assertThat(e.getRootCause(), instanceOf(MapperParsingException.class));
372-
assertThat(e.getRootCause().getMessage(), equalTo("analyzer [foobar] not found for field [__dummy__]"));
373+
assertThat(e.getRootCause().getMessage(), equalTo("analyzer [foobar] not found for field [__dynamic__my_template]"));
373374
}
374375

375376
public void testIllegalDynamicTemplateNoMappingType() throws Exception {
@@ -433,7 +434,8 @@ public void testIllegalDynamicTemplateNoMappingType() throws Exception {
433434
MapperParsingException e = expectThrows(MapperParsingException.class,
434435
() -> mapperService.merge("type", new CompressedXContent(Strings.toString(mapping)), MergeReason.MAPPING_UPDATE));
435436
assertThat(e.getRootCause(), instanceOf(MapperParsingException.class));
436-
assertThat(e.getRootCause().getMessage(), equalTo("unknown parameter [foo] on mapper [__dummy__] of type [null]"));
437+
assertThat(e.getRootCause().getMessage(),
438+
equalTo("unknown parameter [foo] on mapper [__dynamic__my_template] of type [binary]"));
437439
}
438440
}
439441

0 commit comments

Comments
 (0)