Skip to content

Commit

Permalink
ParametrizedFieldMapper to run validators against default value (#60042)
Browse files Browse the repository at this point in the history 
Sometimes there is the need to make a field required in the mappings, and validate that a value has been provided for it. This can be done through a validator when using ParametrizedFieldMapper, but validators need to run also when a value for a field has not been specified.

Relates to #59332
  • Loading branch information
@javanna
javanna committed Jul 22, 2020
1 parent 7e652ca commit 702c997
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 39 deletions.
4 changes: 2 additions & 2 deletions server/src/main/java/org/elasticsearch/index/mapper/ParametrizedFieldMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,8 @@ public Parameter<T> setSerializer(Serializer<T> serializer) {
}

private void validate() {
if (validator != null && isSet) {
validator.accept(value);
if (validator != null) {
validator.accept(getValue());
}
}

Expand Down
98 changes: 61 additions & 37 deletions server/src/test/java/org/elasticsearch/index/mapper/ParametrizedMapperTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,16 +115,21 @@ public static class Builder extends ParametrizedFieldMapper.Builder {
= Parameter.analyzerParam("analyzer", true, m -> toType(m).analyzer, () -> Lucene.KEYWORD_ANALYZER);
final Parameter<NamedAnalyzer> searchAnalyzer
= Parameter.analyzerParam("search_analyzer", true, m -> toType(m).searchAnalyzer, analyzer::getValue);

final Parameter<Boolean> index = Parameter.boolParam("index", false, m -> toType(m).index, true);
final Parameter<String> required = Parameter.stringParam("required", true, m -> toType(m).required, null)
.setValidator(value -> {
if (value == null) {
throw new IllegalArgumentException("field [required] must be specified");
}
});

protected Builder(String name) {
super(name);
}

@Override
protected List<Parameter<?>> getParameters() {
return Arrays.asList(fixed, fixed2, variable, index, wrapper, intValue, analyzer, searchAnalyzer);
return Arrays.asList(fixed, fixed2, variable, index, wrapper, intValue, analyzer, searchAnalyzer, required);
}

@Override
Expand Down Expand Up @@ -154,6 +159,7 @@ public static class TestMapper extends ParametrizedFieldMapper {
private final NamedAnalyzer analyzer;
private final NamedAnalyzer searchAnalyzer;
private final boolean index;
private final String required;

protected TestMapper(String simpleName, String fullName, MultiFields multiFields, CopyTo copyTo,
ParametrizedMapperTests.Builder builder) {
Expand All @@ -166,6 +172,7 @@ protected TestMapper(String simpleName, String fullName, MultiFields multiFields
this.analyzer = builder.analyzer.getValue();
this.searchAnalyzer = builder.searchAnalyzer.getValue();
this.index = builder.index.getValue();
this.required = builder.required.getValue();
}

@Override
Expand All @@ -174,7 +181,7 @@ public Builder getMergeBuilder() {
}

@Override
protected void parseCreateField(ParseContext context) throws IOException {
protected void parseCreateField(ParseContext context) {

}

Expand Down Expand Up @@ -213,7 +220,7 @@ private static TestMapper fromMapping(String mapping) {

// defaults - create empty builder config, and serialize with and without defaults
public void testDefaults() throws IOException {
String mapping = "{\"type\":\"test_mapper\"}";
String mapping = "{\"type\":\"test_mapper\",\"required\":\"value\"}";
TestMapper mapper = fromMapping(mapping);

assertTrue(mapper.fixed);
Expand All @@ -228,17 +235,18 @@ public void testDefaults() throws IOException {
builder.endObject();
assertEquals("{\"field\":{\"type\":\"test_mapper\",\"fixed\":true," +
"\"fixed2\":false,\"variable\":\"default\",\"index\":true," +
"\"wrapper\":\"default\",\"int_value\":5,\"analyzer\":\"_keyword\",\"search_analyzer\":\"_keyword\"}}",
"\"wrapper\":\"default\",\"int_value\":5,\"analyzer\":\"_keyword\"," +
"\"search_analyzer\":\"_keyword\",\"required\":\"value\"}}",
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}";
String mapping = "{\"type\":\"test_mapper\",\"fixed\":false,\"required\":\"value\"}";
TestMapper mapper = fromMapping(mapping);
assertEquals("{\"field\":" + mapping + "}", Strings.toString(mapper));

TestMapper badMerge = fromMapping("{\"type\":\"test_mapper\",\"fixed\":true,\"fixed2\":true}");
TestMapper badMerge = fromMapping("{\"type\":\"test_mapper\",\"fixed\":true,\"fixed2\":true,\"required\":\"value\"}");
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" +
Expand All @@ -248,56 +256,61 @@ public void testMerging() {
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 goodMerge = fromMapping("{\"type\":\"test_mapper\",\"fixed\":false,\"variable\":\"updated\",\"required\":\"value\"}");
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));

assertEquals("{\"field\":{\"type\":\"test_mapper\",\"fixed\":false,\"variable\":\"updated\",\"required\":\"value\"}}",
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\"}}}";
String mapping = "{\"type\":\"test_mapper\",\"variable\":\"foo\",\"required\":\"value\"," +
"\"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\"}}}";
String addSubField = "{\"type\":\"test_mapper\",\"variable\":\"foo\",\"required\":\"value\"" +
",\"fields\":{\"sub2\":{\"type\":\"keyword\"}}}";
TestMapper toMerge = fromMapping(addSubField);
TestMapper merged = (TestMapper) mapper.merge(toMerge);
assertEquals("{\"field\":{\"type\":\"test_mapper\",\"variable\":\"foo\"," +
assertEquals("{\"field\":{\"type\":\"test_mapper\",\"variable\":\"foo\",\"required\":\"value\"," +
"\"fields\":{\"sub\":{\"type\":\"keyword\"},\"sub2\":{\"type\":\"keyword\"}}}}", Strings.toString(merged));

String badSubField = "{\"type\":\"test_mapper\",\"variable\":\"foo\",\"fields\":{\"sub2\":{\"type\":\"binary\"}}}";
String badSubField = "{\"type\":\"test_mapper\",\"variable\":\"foo\",\"required\":\"value\"," +
"\"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\"]}";
String mapping = "{\"type\":\"test_mapper\",\"variable\":\"foo\",\"required\":\"value\",\"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 toMerge = fromMapping("{\"type\":\"test_mapper\",\"variable\":\"updated\",\"required\":\"value\"," +
"\"copy_to\":[\"foo\",\"bar\"]}");
TestMapper merged = (TestMapper) mapper.merge(toMerge);
assertEquals("{\"field\":{\"type\":\"test_mapper\",\"variable\":\"updated\",\"copy_to\":[\"foo\",\"bar\"]}}",
assertEquals("{\"field\":{\"type\":\"test_mapper\",\"variable\":\"updated\",\"required\":\"value\"," +
"\"copy_to\":[\"foo\",\"bar\"]}}",
Strings.toString(merged));

TestMapper removeCopyTo = fromMapping("{\"type\":\"test_mapper\",\"variable\":\"updated\"}");
TestMapper removeCopyTo = fromMapping("{\"type\":\"test_mapper\",\"variable\":\"updated\",\"required\":\"value\"}");
TestMapper noCopyTo = (TestMapper) merged.merge(removeCopyTo);
assertEquals("{\"field\":{\"type\":\"test_mapper\",\"variable\":\"updated\"}}", Strings.toString(noCopyTo));
assertEquals("{\"field\":{\"type\":\"test_mapper\",\"variable\":\"updated\",\"required\":\"value\"}}", Strings.toString(noCopyTo));
}

public void testNullables() {
String mapping = "{\"type\":\"test_mapper\",\"fixed\":null}";
String mapping = "{\"type\":\"test_mapper\",\"fixed\":null,\"required\":\"value\"}";
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}";
String fine = "{\"type\":\"test_mapper\",\"variable\":null,\"required\":\"value\"}";
TestMapper mapper = fromMapping(fine);
assertEquals("{\"field\":" + fine + "}", Strings.toString(mapper));
}
Expand All @@ -323,79 +336,90 @@ public void testObjectSerialization() throws IOException {

// test custom serializer
public void testCustomSerialization() {
String mapping = "{\"type\":\"test_mapper\",\"wrapper\":\"wrapped value\"}";
String mapping = "{\"type\":\"test_mapper\",\"wrapper\":\"wrapped value\",\"required\":\"value\"}";
TestMapper mapper = fromMapping(mapping);
assertEquals("wrapped value", mapper.wrapper.name);
assertEquals("{\"field\":" + mapping + "}", Strings.toString(mapper));
}

// test validator
public void testParameterValidation() {
String mapping = "{\"type\":\"test_mapper\",\"int_value\":10}";
String mapping = "{\"type\":\"test_mapper\",\"int_value\":10,\"required\":\"value\"}";
TestMapper mapper = fromMapping(mapping);
assertEquals(10, mapper.intValue);
assertEquals("{\"field\":" + mapping + "}", Strings.toString(mapper));

IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
() -> fromMapping("{\"type\":\"test_mapper\",\"int_value\":60}"));
() -> fromMapping("{\"type\":\"test_mapper\",\"int_value\":60,\"required\":\"value\"}"));
assertEquals("Value of [n] cannot be greater than 50", e.getMessage());

}

// test deprecations
public void testDeprecatedParameterName() {
String mapping = "{\"type\":\"test_mapper\",\"fixed2_old\":true}";
String mapping = "{\"type\":\"test_mapper\",\"fixed2_old\":true,\"required\":\"value\"}";
TestMapper mapper = fromMapping(mapping);
assertTrue(mapper.fixed2);
assertWarnings("Parameter [fixed2_old] on mapper [field] is deprecated, use [fixed2]");
assertEquals("{\"field\":{\"type\":\"test_mapper\",\"fixed2\":true}}", Strings.toString(mapper));
assertEquals("{\"field\":{\"type\":\"test_mapper\",\"fixed2\":true,\"required\":\"value\"}}", Strings.toString(mapper));
}

public void testAnalyzers() {
String mapping = "{\"type\":\"test_mapper\",\"analyzer\":\"_standard\"}";
String mapping = "{\"type\":\"test_mapper\",\"analyzer\":\"_standard\",\"required\":\"value\"}";
TestMapper mapper = fromMapping(mapping);
assertEquals(mapper.analyzer, Lucene.STANDARD_ANALYZER);
assertEquals("{\"field\":" + mapping + "}", Strings.toString(mapper));

String withDef = "{\"type\":\"test_mapper\",\"analyzer\":\"default\"}";
String withDef = "{\"type\":\"test_mapper\",\"analyzer\":\"default\",\"required\":\"value\"}";
mapper = fromMapping(withDef);
assertEquals(mapper.analyzer.name(), "default");
assertThat(mapper.analyzer.analyzer(), instanceOf(StandardAnalyzer.class));
assertEquals("{\"field\":" + withDef + "}", Strings.toString(mapper));

String badAnalyzer = "{\"type\":\"test_mapper\",\"analyzer\":\"wibble\"}";
String badAnalyzer = "{\"type\":\"test_mapper\",\"analyzer\":\"wibble\",\"required\":\"value\"}";
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> fromMapping(badAnalyzer));
assertEquals("analyzer [wibble] has not been configured in mappings", e.getMessage());
}

public void testDeprecatedParameters() throws IOException {
public void testDeprecatedParameters() {
// 'index' is declared explicitly, 'store' is not, but is one of the previously always-accepted params
String mapping = "{\"type\":\"test_mapper\",\"index\":false,\"store\":true}";
String mapping = "{\"type\":\"test_mapper\",\"index\":false,\"store\":true,\"required\":\"value\"}";
TestMapper mapper = fromMapping(mapping, Version.V_7_8_0);
assertWarnings("Parameter [store] has no effect on type [test_mapper] and will be removed in future");
assertFalse(mapper.index);
assertEquals("{\"field\":{\"type\":\"test_mapper\",\"index\":false}}", Strings.toString(mapper));
assertEquals("{\"field\":{\"type\":\"test_mapper\",\"index\":false,\"required\":\"value\"}}", Strings.toString(mapper));
}

public void testLinkedAnalyzers() {
String mapping = "{\"type\":\"test_mapper\",\"analyzer\":\"_standard\"}";
String mapping = "{\"type\":\"test_mapper\",\"analyzer\":\"_standard\",\"required\":\"value\"}";
TestMapper mapper = fromMapping(mapping);
assertEquals("_standard", mapper.analyzer.name());
assertEquals("_standard", mapper.searchAnalyzer.name());
assertEquals("{\"field\":" + mapping + "}", Strings.toString(mapper));

String mappingWithSA = "{\"type\":\"test_mapper\",\"search_analyzer\":\"_standard\"}";
String mappingWithSA = "{\"type\":\"test_mapper\",\"search_analyzer\":\"_standard\",\"required\":\"value\"}";
mapper = fromMapping(mappingWithSA);
assertEquals("_keyword", mapper.analyzer.name());
assertEquals("_standard", mapper.searchAnalyzer.name());
assertEquals("{\"field\":" + mappingWithSA + "}", Strings.toString(mapper));

String mappingWithBoth = "{\"type\":\"test_mapper\",\"analyzer\":\"default\",\"search_analyzer\":\"_standard\"}";
String mappingWithBoth = "{\"type\":\"test_mapper\",\"analyzer\":\"default\"," +
"\"search_analyzer\":\"_standard\",\"required\":\"value\"}";
mapper = fromMapping(mappingWithBoth);
assertEquals("default", mapper.analyzer.name());
assertEquals("_standard", mapper.searchAnalyzer.name());
assertEquals("{\"field\":" + mappingWithBoth + "}", Strings.toString(mapper));

}

public void testRequiredField() {
{
String mapping = "{\"type\":\"test_mapper\"}";
IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, () -> fromMapping(mapping));
assertEquals("field [required] must be specified", iae.getMessage());
}
{
String mapping = "{\"type\":\"test_mapper\",\"required\":null}";
MapperParsingException exc = expectThrows(MapperParsingException.class, () -> fromMapping(mapping));
assertEquals("[required] on mapper [field] of type [test_mapper] must not have a [null] value", exc.getMessage());
}
}
}

0 comments on commit 702c997

Please sign in to comment.