Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add multiple validators to FieldMapper.Parameter #77073

Merged
merged 1 commit into from
Aug 31, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,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 @@ -213,7 +213,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 @@ -133,7 +133,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 @@ -586,7 +586,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 @@ -681,10 +681,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 @@ -741,8 +742,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 @@ -893,7 +895,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 @@ -1077,7 +1079,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 @@ -90,7 +90,7 @@ public Builder(String name, ScriptCompiler scriptCompiler, boolean ignoreMalform
this.script.precludesParameters(nullValue, ignoreMalformed);
addScriptValidation(script, indexed, hasDocValues);
this.dimension = Parameter.boolParam("dimension", false, m -> toType(m).dimension, false)
.setValidator(v -> {
.addValidator(v -> {
if (v && (indexed.getValue() == false || hasDocValues.getValue() == false)) {
throw new IllegalArgumentException(
"Field [dimension] requires that [" + indexed.name + "] and [" + hasDocValues.name + "] are true"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ public Builder(String name, IndexAnalyzers indexAnalyzers, ScriptCompiler script
this.script.precludesParameters(nullValue);
addScriptValidation(script, indexed, hasDocValues);

this.dimension = Parameter.boolParam("dimension", false, m -> toType(m).dimension, false).setValidator(v -> {
this.dimension = Parameter.boolParam("dimension", false, m -> toType(m).dimension, false).addValidator(v -> {
if (v && (indexed.getValue() == false || hasDocValues.getValue() == false)) {
throw new IllegalArgumentException(
"Field [dimension] requires that [" + indexed.name + "] and [" + hasDocValues.name + "] are true"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,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 @@ -115,7 +115,7 @@ public Builder(String name, NumberType type, ScriptCompiler compiler, boolean ig
(n, c, o) -> o == null ? null : type.parse(o, false), m -> toType(m).nullValue).acceptsNull();

this.dimension = Parameter.boolParam("dimension", false, m -> toType(m).dimension, false)
.setValidator(v -> {
.addValidator(v -> {
if (v && EnumSet.of(NumberType.INTEGER, NumberType.LONG, NumberType.BYTE, NumberType.SHORT).contains(type) == false) {
throw new IllegalArgumentException("Parameter [dimension] cannot be set to numeric type [" + type.name + "]");
}
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 @@ -113,7 +113,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 @@ -94,19 +94,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 @@ -369,6 +374,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 @@ -55,7 +55,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