Skip to content

Commit 27ca0a8

Browse files
romseygeekromseygeek
authored
Convert ConstantKeywordFieldMapper to parametrized form (elastic#62688)
As part of the conversion, adds the ability to customize merge validation - in this case, we allow an update to the constant value if it is currently set to null, but refuse further updates once it has been set once. This commit also converts ParametrizedMapperTests to use MapperServiceTestCase.
1 parent 352b2c3 commit 27ca0a8

File tree

5 files changed

+126
-125
lines changed

5 files changed

+126
-125
lines changed

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

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
import java.util.Objects;
4343
import java.util.Set;
4444
import java.util.function.BiFunction;
45+
import java.util.function.BiPredicate;
4546
import java.util.function.BooleanSupplier;
4647
import java.util.function.Consumer;
4748
import java.util.function.Function;
@@ -147,7 +148,8 @@ public static final class Parameter<T> {
147148
private Consumer<T> validator = null;
148149
private Serializer<T> serializer = XContentBuilder::field;
149150
private BooleanSupplier serializerPredicate = () -> true;
150-
private Function<T, String> conflictSerializer = Object::toString;
151+
private Function<T, String> conflictSerializer = Objects::toString;
152+
private BiPredicate<T, T> mergeValidator;
151153
private T value;
152154
private boolean isSet;
153155

@@ -167,6 +169,7 @@ public Parameter(String name, boolean updateable, Supplier<T> defaultValue,
167169
this.parser = parser;
168170
this.initializer = initializer;
169171
this.updateable = updateable;
172+
this.mergeValidator = (previous, toMerge) -> updateable || Objects.equals(previous, toMerge);
170173
}
171174

172175
/**
@@ -240,6 +243,15 @@ public Parameter<T> setShouldSerialize(BooleanSupplier shouldSerialize) {
240243
return this;
241244
}
242245

246+
/**
247+
* Sets a custom merge validator. By default, merges are accepted if the
248+
* parameter is updateable, or if the previous and new values are equal
249+
*/
250+
public Parameter<T> setMergeValidator(BiPredicate<T, T> mergeValidator) {
251+
this.mergeValidator = mergeValidator;
252+
return this;
253+
}
254+
243255
private void validate() {
244256
if (validator != null) {
245257
validator.accept(getValue());
@@ -257,10 +269,10 @@ private void parse(String field, ParserContext context, Object in) {
257269
private void merge(FieldMapper toMerge, Conflicts conflicts) {
258270
T value = initializer.apply(toMerge);
259271
T current = getValue();
260-
if (updateable == false && Objects.equals(current, value) == false) {
261-
conflicts.addConflict(name, conflictSerializer.apply(current), conflictSerializer.apply(value));
262-
} else {
272+
if (mergeValidator.test(current, value)) {
263273
setValue(value);
274+
} else {
275+
conflicts.addConflict(name, conflictSerializer.apply(current), conflictSerializer.apply(value));
264276
}
265277
}
266278

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

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,15 +29,13 @@
2929
import org.elasticsearch.common.xcontent.XContentBuilder;
3030
import org.elasticsearch.common.xcontent.XContentHelper;
3131
import org.elasticsearch.common.xcontent.json.JsonXContent;
32-
import org.elasticsearch.index.IndexService;
3332
import org.elasticsearch.index.analysis.AnalyzerScope;
3433
import org.elasticsearch.index.analysis.IndexAnalyzers;
3534
import org.elasticsearch.index.analysis.NamedAnalyzer;
3635
import org.elasticsearch.index.mapper.ParametrizedFieldMapper.Parameter;
3736
import org.elasticsearch.plugins.MapperPlugin;
3837
import org.elasticsearch.plugins.Plugin;
3938
import org.elasticsearch.search.lookup.SearchLookup;
40-
import org.elasticsearch.test.ESSingleNodeTestCase;
4139

4240
import java.io.IOException;
4341
import java.util.Collection;
@@ -46,11 +44,12 @@
4644
import java.util.Map;
4745
import java.util.Objects;
4846

47+
import static org.hamcrest.Matchers.containsString;
4948
import static org.hamcrest.Matchers.instanceOf;
5049
import static org.mockito.Mockito.mock;
5150
import static org.mockito.Mockito.when;
5251

53-
public class ParametrizedMapperTests extends ESSingleNodeTestCase {
52+
public class ParametrizedMapperTests extends MapperServiceTestCase {
5453

5554
public static class TestPlugin extends Plugin implements MapperPlugin {
5655
@Override
@@ -60,8 +59,8 @@ public Map<String, Mapper.TypeParser> getMappers() {
6059
}
6160

6261
@Override
63-
protected Collection<Class<? extends Plugin>> getPlugins() {
64-
return List.of(TestPlugin.class);
62+
protected Collection<Plugin> getPlugins() {
63+
return List.of(new TestPlugin());
6564
}
6665

6766
private static class StringWrapper {
@@ -110,7 +109,8 @@ public static class Builder extends ParametrizedFieldMapper.Builder {
110109
if (n > 50) {
111110
throw new IllegalArgumentException("Value of [n] cannot be greater than 50");
112111
}
113-
});
112+
})
113+
.setMergeValidator((o, n) -> n >= o);
114114
final Parameter<NamedAnalyzer> analyzer
115115
= Parameter.analyzerParam("analyzer", false, m -> toType(m).analyzer, () -> Lucene.KEYWORD_ANALYZER);
116116
final Parameter<NamedAnalyzer> searchAnalyzer
@@ -333,8 +333,6 @@ public void testNullables() {
333333

334334
public void testObjectSerialization() throws IOException {
335335

336-
IndexService indexService = createIndex("test");
337-
338336
String mapping = "{\"_doc\":{" +
339337
"\"properties\":{" +
340338
"\"actual\":{\"type\":\"double\"}," +
@@ -343,11 +341,12 @@ public void testObjectSerialization() throws IOException {
343341
"\"anomaly_score\":{\"type\":\"double\"}," +
344342
"\"bucket_span\":{\"type\":\"long\"}," +
345343
"\"is_interim\":{\"type\":\"boolean\"}}}}}}";
346-
indexService.mapperService().merge("_doc", new CompressedXContent(mapping), MapperService.MergeReason.MAPPING_UPDATE);
347-
assertEquals(mapping, Strings.toString(indexService.mapperService().documentMapper()));
348344

349-
indexService.mapperService().merge("_doc", new CompressedXContent(mapping), MapperService.MergeReason.MAPPING_UPDATE);
350-
assertEquals(mapping, Strings.toString(indexService.mapperService().documentMapper()));
345+
MapperService mapperService = createMapperService(mapping);
346+
assertEquals(mapping, Strings.toString(mapperService.documentMapper()));
347+
348+
mapperService.merge("_doc", new CompressedXContent(mapping), MapperService.MergeReason.MAPPING_UPDATE);
349+
assertEquals(mapping, Strings.toString(mapperService.documentMapper()));
351350
}
352351

353352
// test custom serializer
@@ -487,4 +486,19 @@ public void testRestrictedField() {
487486
}
488487
}
489488

489+
public void testCustomMergeValidation() throws IOException {
490+
MapperService mapperService = createMapperService(fieldMapping(b -> {
491+
b.field("type", "test_mapper");
492+
b.field("required", "a");
493+
b.field("int_value", 10);
494+
}));
495+
496+
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> merge(mapperService, fieldMapping(b -> {
497+
b.field("type", "test_mapper");
498+
b.field("required", "a");
499+
b.field("int_value", 5); // custom merge validator says that int_value can only increase
500+
})));
501+
assertThat(e.getMessage(), containsString("int_value"));
502+
}
503+
490504
}

‎x-pack/plugin/mapper-constant-keyword/src/internalClusterTest/java/org/elasticsearch/xpack/constantkeyword/mapper/ConstantKeywordFieldMapperTests.java

Lines changed: 51 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -7,78 +7,46 @@
77
package org.elasticsearch.xpack.constantkeyword.mapper;
88

99
import org.elasticsearch.common.Strings;
10-
import org.elasticsearch.common.bytes.BytesReference;
1110
import org.elasticsearch.common.compress.CompressedXContent;
1211
import org.elasticsearch.common.xcontent.XContentBuilder;
13-
import org.elasticsearch.common.xcontent.XContentFactory;
14-
import org.elasticsearch.common.xcontent.XContentType;
1512
import org.elasticsearch.index.mapper.DocumentMapper;
1613
import org.elasticsearch.index.mapper.FieldMapper;
17-
import org.elasticsearch.index.mapper.FieldMapperTestCase2;
1814
import org.elasticsearch.index.mapper.MapperParsingException;
1915
import org.elasticsearch.index.mapper.MapperService;
2016
import org.elasticsearch.index.mapper.MapperService.MergeReason;
17+
import org.elasticsearch.index.mapper.MapperTestCase;
2118
import org.elasticsearch.index.mapper.ParsedDocument;
22-
import org.elasticsearch.index.mapper.SourceToParse;
2319
import org.elasticsearch.index.mapper.ValueFetcher;
2420
import org.elasticsearch.plugins.Plugin;
2521
import org.elasticsearch.search.lookup.SourceLookup;
2622
import org.elasticsearch.xpack.constantkeyword.ConstantKeywordMapperPlugin;
27-
import org.junit.Before;
2823

2924
import java.io.IOException;
3025
import java.util.Collection;
3126
import java.util.Collections;
3227
import java.util.List;
33-
import java.util.Set;
3428

35-
public class ConstantKeywordFieldMapperTests extends FieldMapperTestCase2<ConstantKeywordFieldMapper.Builder> {
29+
public class ConstantKeywordFieldMapperTests extends MapperTestCase {
3630

3731
@Override
3832
protected Collection<Plugin> getPlugins() {
3933
return List.of(new ConstantKeywordMapperPlugin());
4034
}
4135

42-
@Override
43-
protected Set<String> unsupportedProperties() {
44-
return Set.of("analyzer", "similarity", "store", "doc_values", "index");
45-
}
46-
47-
@Override
48-
protected ConstantKeywordFieldMapper.Builder newBuilder() {
49-
return new ConstantKeywordFieldMapper.Builder("constant");
50-
}
51-
52-
@Before
53-
public void addModifiers() {
54-
addModifier("value", false, (a, b) -> {
55-
a.setValue("foo");
56-
b.setValue("bar");
57-
});
58-
addModifier("unset", false, (a, b) -> {
59-
a.setValue("foo");
60-
;
61-
});
62-
addModifier("value-from-null", true, (a, b) -> { b.setValue("bar"); });
63-
}
64-
6536
public void testDefaults() throws Exception {
6637
XContentBuilder mapping = fieldMapping(b -> b.field("type", "constant_keyword").field("value", "foo"));
6738
DocumentMapper mapper = createDocumentMapper(mapping);
6839
assertEquals(Strings.toString(mapping), mapper.mappingSource().toString());
6940

70-
BytesReference source = BytesReference.bytes(XContentFactory.jsonBuilder().startObject().endObject());
71-
ParsedDocument doc = mapper.parse(new SourceToParse("test", "1", source, XContentType.JSON));
41+
ParsedDocument doc = mapper.parse(source(b -> {}));
7242
assertNull(doc.rootDoc().getField("field"));
7343

74-
source = BytesReference.bytes(XContentFactory.jsonBuilder().startObject().field("field", "foo").endObject());
75-
doc = mapper.parse(new SourceToParse("test", "1", source, XContentType.JSON));
44+
doc = mapper.parse(source(b -> b.field("field", "foo")));
7645
assertNull(doc.rootDoc().getField("field"));
7746

78-
BytesReference illegalSource = BytesReference.bytes(XContentFactory.jsonBuilder().startObject().field("field", "bar").endObject());
7947
MapperParsingException e = expectThrows(
8048
MapperParsingException.class,
81-
() -> mapper.parse(new SourceToParse("test", "1", illegalSource, XContentType.JSON))
49+
() -> mapper.parse(source(b -> b.field("field", "bar")))
8250
);
8351
assertEquals(
8452
"[constant_keyword] field [field] only accepts values that are equal to the value defined in the mappings [foo], "
@@ -90,8 +58,7 @@ public void testDefaults() throws Exception {
9058
public void testDynamicValue() throws Exception {
9159
MapperService mapperService = createMapperService(fieldMapping(b -> b.field("type", "constant_keyword")));
9260

93-
BytesReference source = BytesReference.bytes(XContentFactory.jsonBuilder().startObject().field("field", "foo").endObject());
94-
ParsedDocument doc = mapperService.documentMapper().parse(new SourceToParse("test", "1", source, XContentType.JSON));
61+
ParsedDocument doc = mapperService.documentMapper().parse(source(b -> b.field("field", "foo")));
9562
assertNull(doc.rootDoc().getField("field"));
9663
assertNotNull(doc.dynamicMappingsUpdate());
9764

@@ -100,11 +67,55 @@ public void testDynamicValue() throws Exception {
10067
String expectedMapping = Strings.toString(fieldMapping(b -> b.field("type", "constant_keyword").field("value", "foo")));
10168
assertEquals(expectedMapping, updatedMapper.mappingSource().toString());
10269

103-
doc = updatedMapper.parse(new SourceToParse("test", "1", source, XContentType.JSON));
70+
doc = updatedMapper.parse(source(b -> b.field("field", "foo")));
10471
assertNull(doc.rootDoc().getField("field"));
10572
assertNull(doc.dynamicMappingsUpdate());
10673
}
10774

75+
public void testBadValues() {
76+
{
77+
MapperParsingException e = expectThrows(MapperParsingException.class, () -> createMapperService(fieldMapping(b -> {
78+
b.field("type", "constant_keyword");
79+
b.nullField("value");
80+
})));
81+
assertEquals(e.getMessage(),
82+
"Failed to parse mapping: [value] on mapper [field] of type [constant_keyword] must not have a [null] value");
83+
}
84+
{
85+
MapperParsingException e = expectThrows(MapperParsingException.class, () -> createMapperService(fieldMapping(b -> {
86+
b.field("type", "constant_keyword");
87+
b.startObject("value").field("foo", "bar").endObject();
88+
})));
89+
assertEquals(e.getMessage(),
90+
"Failed to parse mapping: Property [value] on field [field] must be a number or a string, but got [{foo=bar}]");
91+
}
92+
}
93+
94+
public void testNumericValue() throws IOException {
95+
MapperService mapperService = createMapperService(fieldMapping(b -> {
96+
b.field("type", "constant_keyword");
97+
b.field("value", 74);
98+
}));
99+
ConstantKeywordFieldMapper.ConstantKeywordFieldType ft
100+
= (ConstantKeywordFieldMapper.ConstantKeywordFieldType) mapperService.fieldType("field");
101+
assertEquals("74", ft.value());
102+
}
103+
104+
public void testUpdate() throws IOException {
105+
MapperService mapperService = createMapperService(fieldMapping(b -> {
106+
b.field("type", "constant_keyword");
107+
b.field("value", "foo");
108+
}));
109+
110+
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> merge(mapperService, fieldMapping(b -> {
111+
b.field("type", "constant_keyword");
112+
b.field("value", "bar");
113+
})));
114+
assertEquals(e.getMessage(),
115+
"Mapper for [field] conflicts with existing mapper:\n" +
116+
"\tCannot update parameter [value] from [foo] to [bar]");
117+
}
118+
108119
@Override
109120
protected void minimalMapping(XContentBuilder b) throws IOException {
110121
b.field("type", "constant_keyword");

‎x-pack/plugin/mapper-constant-keyword/src/main/java/org/elasticsearch/xpack/constantkeyword/ConstantKeywordMapperPlugin.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
public class ConstantKeywordMapperPlugin extends Plugin implements MapperPlugin {
1919
@Override
2020
public Map<String, Mapper.TypeParser> getMappers() {
21-
return singletonMap(ConstantKeywordFieldMapper.CONTENT_TYPE, new ConstantKeywordFieldMapper.TypeParser());
21+
return singletonMap(ConstantKeywordFieldMapper.CONTENT_TYPE, ConstantKeywordFieldMapper.PARSER);
2222
}
2323

2424
}

0 commit comments

Comments
 (0)