Skip to content

Commit

Permalink
Add multiple validators to Parameters (#77073) (#77091)
Browse files Browse the repository at this point in the history
This PR implements support for multiple validators to a FieldMapper.Parameter.

The Parameter#setValidator method was replaced by Parameter#addValidator that can be called multipled times
to add validation to a parameter.

All validators of a parameter will be executed in the same order as they have been added and if any of them fails all validation will failed.
  • Loading branch information
csoulios authored Aug 31, 2021
1 parent 9fbc109 commit 6a88d84
Show file tree
Hide file tree
Showing 14 changed files with 44 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public static class Builder extends FieldMapper.Builder {

private final Parameter<Double> scalingFactor = new Parameter<>("scaling_factor", false, () -> null,
(n, c, o) -> XContentMapValues.nodeDoubleValue(o), m -> toType(m).scalingFactor)
.setValidator(v -> {
.addValidator(v -> {
if (v == null) {
throw new IllegalArgumentException("Field [scaling_factor] is required");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public static class Builder extends FieldMapper.Builder {
// `doc_values=false`, even though it cannot be set; and so we need to continue
// serializing it forever because of mapper assertions in mixed clusters.
private final Parameter<Boolean> docValues = Parameter.docValuesParam(m -> false, false)
.setValidator(v -> {
.addValidator(v -> {
if (v) {
throw new MapperParsingException("Cannot set [doc_values] on field of type [search_as_you_type]");
}
Expand All @@ -106,7 +106,7 @@ public static class Builder extends FieldMapper.Builder {

private final Parameter<Integer> maxShingleSize = Parameter.intParam("max_shingle_size", false,
m -> builder(m).maxShingleSize.get(), Defaults.MAX_SHINGLE_SIZE)
.setValidator(v -> {
.addValidator(v -> {
if (v < MAX_SHINGLE_SIZE_LOWER_BOUND || v > MAX_SHINGLE_SIZE_UPPER_BOUND) {
throw new MapperParsingException("[max_shingle_size] must be at least [" + MAX_SHINGLE_SIZE_LOWER_BOUND
+ "] and at most " + "[" + MAX_SHINGLE_SIZE_UPPER_BOUND + "], got [" + v + "]");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ public static class Builder extends FieldMapper.Builder {

final Parameter<Integer> ignoreAbove
= Parameter.intParam("ignore_above", true, m -> toType(m).ignoreAbove, Integer.MAX_VALUE)
.setValidator(v -> {
.addValidator(v -> {
if (v < 0) {
throw new IllegalArgumentException("[ignore_above] must be positive, got [" + v + "]");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ public static class Builder extends FieldMapper.Builder {
private final Parameter<Integer> maxInputLength = Parameter.intParam("max_input_length", true,
m -> builder(m).maxInputLength.get(), Defaults.DEFAULT_MAX_INPUT_LENGTH)
.addDeprecatedName("max_input_len")
.setValidator(Builder::validateInputLength)
.addValidator(Builder::validateInputLength)
.alwaysSerialize();
private final Parameter<Map<String, String>> meta = Parameter.metaParam();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public class CompositeRuntimeField implements RuntimeField {
() -> null,
RuntimeField::parseScript,
RuntimeField.initializerNotSupported()
).setValidator(s -> {
).addValidator(s -> {
if (s == null) {
throw new IllegalArgumentException("composite runtime field [" + name + "] must declare a [script]");
}
Expand All @@ -50,7 +50,7 @@ public class CompositeRuntimeField implements RuntimeField {
Collections::emptyMap,
(f, p, o) -> parseFields(f, o),
RuntimeField.initializerNotSupported()
).setValidator(objectMap -> {
).addValidator(objectMap -> {
if (objectMap == null || objectMap.isEmpty()) {
throw new IllegalArgumentException("composite runtime field [" + name + "] must declare its [fields]");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,7 @@ public static final class Parameter<T> implements Supplier<T> {
private final TriFunction<String, MappingParserContext, Object, T> parser;
private final Function<FieldMapper, T> initializer;
private boolean acceptsNull = false;
private Consumer<T> validator = null;
private List<Consumer<T>> validators = new ArrayList<>();
private Serializer<T> serializer = XContentBuilder::field;
private SerializerCheck<T> serializerCheck = (includeDefaults, isConfigured, value) -> includeDefaults || isConfigured;
private Function<T, String> conflictSerializer = Objects::toString;
Expand Down Expand Up @@ -682,10 +682,11 @@ public Parameter<T> deprecated() {
}

/**
* Adds validation to a parameter, called after parsing and merging
* Adds validation to a parameter, called after parsing and merging. Multiple
* validators can be added and all of them will be executed.
*/
public Parameter<T> setValidator(Consumer<T> validator) {
this.validator = validator;
public Parameter<T> addValidator(Consumer<T> validator) {
this.validators.add(validator);
return this;
}

Expand Down Expand Up @@ -742,8 +743,9 @@ public Parameter<T> precludesParameters(Parameter<?>... ps) {
}

void validate() {
if (validator != null) {
validator.accept(getValue());
// Iterate over the list of validators and execute them one by one.
for (Consumer<T> v : validators) {
v.accept(getValue());
}
if (this.isConfigured()) {
for (Parameter<?> p : requires) {
Expand Down Expand Up @@ -894,7 +896,7 @@ public static Parameter<String> restrictedStringParam(String name, boolean updat
assert values.length > 0;
Set<String> acceptedValues = new LinkedHashSet<>(Arrays.asList(values));
return stringParam(name, updateable, initializer, values[0])
.setValidator(v -> {
.addValidator(v -> {
if (acceptedValues.contains(v)) {
return;
}
Expand Down Expand Up @@ -1082,7 +1084,7 @@ protected void addScriptValidation(
Parameter<Boolean> indexParam,
Parameter<Boolean> docValuesParam
) {
scriptParam.setValidator(s -> {
scriptParam.addValidator(s -> {
if (s != null && indexParam.get() == false && docValuesParam.get() == false) {
throw new MapperParsingException("Cannot define script on field with index:false and doc_values:false");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -537,5 +537,4 @@ public FieldMapper.Builder getMergeBuilder() {
return new Builder(simpleName(), indexAnalyzers, scriptCompiler).init(this);
}


}
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ public Builder(String name, Version version, boolean ignoreMalformedByDefault, b
this.ignoreMalformed = ignoreMalformedParam(m -> builder(m).ignoreMalformed.get(), ignoreMalformedByDefault);
this.coerce = coerceParam(m -> builder(m).coerce.get(), coerceByDefault);

this.pointsOnly.setValidator(v -> {
this.pointsOnly.addValidator(v -> {
if (v == null) {
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public Analyzers(IndexAnalyzers indexAnalyzers,
m -> analyzerInitFunction.apply(m).indexAnalyzer.get(), indexAnalyzers::getDefaultIndexAnalyzer)
.setSerializerCheck((id, ic, a) -> id || ic ||
Objects.equals(a, getSearchAnalyzer()) == false || Objects.equals(a, getSearchQuoteAnalyzer()) == false)
.setValidator(a -> a.checkAllowedInMode(AnalysisMode.INDEX_TIME));
.addValidator(a -> a.checkAllowedInMode(AnalysisMode.INDEX_TIME));
this.searchAnalyzer
= Parameter.analyzerParam("search_analyzer", true,
m -> m.fieldType().getTextSearchInfo().getSearchAnalyzer(), () -> {
Expand All @@ -54,7 +54,7 @@ public Analyzers(IndexAnalyzers indexAnalyzers,
return indexAnalyzer.get();
})
.setSerializerCheck((id, ic, a) -> id || ic || Objects.equals(a, getSearchQuoteAnalyzer()) == false)
.setValidator(a -> a.checkAllowedInMode(AnalysisMode.SEARCH_TIME));
.addValidator(a -> a.checkAllowedInMode(AnalysisMode.SEARCH_TIME));
this.searchQuoteAnalyzer
= Parameter.analyzerParam("search_quote_analyzer", true,
m -> m.fieldType().getTextSearchInfo().getSearchQuoteAnalyzer(), () -> {
Expand All @@ -66,10 +66,10 @@ public Analyzers(IndexAnalyzers indexAnalyzers,
}
return searchAnalyzer.get();
})
.setValidator(a -> a.checkAllowedInMode(AnalysisMode.SEARCH_TIME));
.addValidator(a -> a.checkAllowedInMode(AnalysisMode.SEARCH_TIME));
this.positionIncrementGap = Parameter.intParam("position_increment_gap", false,
m -> analyzerInitFunction.apply(m).positionIncrementGap.get(), TextFieldMapper.Defaults.POSITION_INCREMENT_GAP)
.setValidator(v -> {
.addValidator(v -> {
if (v < 0) {
throw new MapperParsingException("[position_increment_gap] must be positive, got [" + v + "]");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ public static class Builder extends FieldMapper.Builder {

final Parameter<Integer> depthLimit
= Parameter.intParam("depth_limit", true, m -> builder(m).depthLimit.get(), Defaults.DEPTH_LIMIT)
.setValidator(v -> {
.addValidator(v -> {
if (v < 0) {
throw new IllegalArgumentException("[depth_limit] must be positive, got [" + v + "]");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,19 +95,24 @@ public static class Builder extends FieldMapper.Builder {
},
m -> toType(m).wrapper).setSerializer((b, n, v) -> b.field(n, v.name), v -> "wrapper_" + v.name);
final Parameter<Integer> intValue = Parameter.intParam("int_value", true, m -> toType(m).intValue, 5)
.setValidator(n -> {
.addValidator(n -> {
if (n > 50) {
throw new IllegalArgumentException("Value of [n] cannot be greater than 50");
}
})
.addValidator(n -> {
if (n < 0) {
throw new IllegalArgumentException("Value of [n] cannot be less than 0");
}
})
.setMergeValidator((o, n, c) -> n >= o);
final Parameter<NamedAnalyzer> analyzer
= Parameter.analyzerParam("analyzer", false, 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 -> {
.addValidator(value -> {
if (value == null) {
throw new IllegalArgumentException("field [required] must be specified");
}
Expand Down Expand Up @@ -370,6 +375,10 @@ public void testParameterValidation() {
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
() -> fromMapping("{\"type\":\"test_mapper\",\"int_value\":60,\"required\":\"value\"}"));
assertEquals("Value of [n] cannot be greater than 50", e.getMessage());

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

// test deprecations
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ public static class Builder extends FieldMapper.Builder {
}
}
return parsedMetrics;
}, m -> toType(m).metrics).setValidator(v -> {
}, m -> toType(m).metrics).addValidator(v -> {
if (v == null || v.isEmpty()) {
throw new IllegalArgumentException("Property [" + Names.METRICS + "] is required for field [" + name() + "].");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public static class Builder extends FieldMapper.Builder {

Parameter<Integer> dims
= new Parameter<>("dims", false, () -> null, (n, c, o) -> XContentMapValues.nodeIntegerValue(o), m -> toType(m).dims)
.setValidator(dims -> {
.addValidator(dims -> {
if (dims == null) {
throw new MapperParsingException("Missing required parameter [dims] for field [" + name + "]");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ public static class Builder extends FieldMapper.Builder {

final Parameter<Integer> ignoreAbove
= Parameter.intParam("ignore_above", true, m -> toType(m).ignoreAbove, Defaults.IGNORE_ABOVE)
.setValidator(v -> {
.addValidator(v -> {
if (v < 0) {
throw new IllegalArgumentException("[ignore_above] must be positive, got [" + v + "]");
}
Expand Down Expand Up @@ -333,7 +333,7 @@ public Query wildcardQuery(String wildcardPattern, RewriteMethod method, boolean
if (clauseCount > 0) {
// We can accelerate execution with the ngram query
BooleanQuery approxQuery = rewritten.build();
return new BinaryDvConfirmedAutomatonQuery(approxQuery, name(), wildcardPattern, automaton);
return new BinaryDvConfirmedAutomatonQuery(approxQuery, name(), wildcardPattern, automaton);
} else if (numWildcardChars == 0 || numWildcardStrings > 0) {
// We have no concrete characters and we're not a pure length query e.g. ???
return new DocValuesFieldExistsQuery(name());
Expand Down Expand Up @@ -365,11 +365,11 @@ public Query regexpQuery(String value, int syntaxFlags, int matchFlags, int maxD
// MatchAllButRequireVerificationQuery is a special case meaning the regex is reduced to a single
// clause which we can't accelerate at all and needs verification. Example would be ".."
if (approxNgramQuery instanceof MatchAllButRequireVerificationQuery) {
return new BinaryDvConfirmedAutomatonQuery(new MatchAllDocsQuery(), name(), value, automaton);
return new BinaryDvConfirmedAutomatonQuery(new MatchAllDocsQuery(), name(), value, automaton);
}

// We can accelerate execution with the ngram query
return new BinaryDvConfirmedAutomatonQuery(approxNgramQuery, name(), value, automaton);
return new BinaryDvConfirmedAutomatonQuery(approxNgramQuery, name(), value, automaton);
}

// Convert a regular expression to a simplified query consisting of BooleanQuery and TermQuery objects
Expand Down Expand Up @@ -740,9 +740,9 @@ public Query rangeQuery(

if (accelerationQuery == null) {
return new BinaryDvConfirmedAutomatonQuery(new MatchAllDocsQuery(),
name(), lower + "-" + upper, automaton);
name(), lower + "-" + upper, automaton);
}
return new BinaryDvConfirmedAutomatonQuery(accelerationQuery, name(), lower + "-" + upper, automaton);
return new BinaryDvConfirmedAutomatonQuery(accelerationQuery, name(), lower + "-" + upper, automaton);
}

@Override
Expand Down Expand Up @@ -822,10 +822,10 @@ public Query fuzzyQuery(
);
if (ngramQ.clauses().size() == 0) {
return new BinaryDvConfirmedAutomatonQuery(new MatchAllDocsQuery(),
name(), searchTerm, fq.getAutomata().automaton);
name(), searchTerm, fq.getAutomata().automaton);
}

return new BinaryDvConfirmedAutomatonQuery(ngramQ, name(), searchTerm, fq.getAutomata().automaton);
return new BinaryDvConfirmedAutomatonQuery(ngramQ, name(), searchTerm, fq.getAutomata().automaton);
} catch (IOException ioe) {
throw new ElasticsearchParseException("Error parsing wildcard field fuzzy string [" + searchTerm + "]");
}
Expand Down

0 comments on commit 6a88d84

Please sign in to comment.