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

Refactor enum mappings parameter to allow for capital case types #92548

Merged
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 @@ -136,8 +136,8 @@ public Builder(String name, boolean ignoreMalformedByDefault, boolean coerceByDe

this.metric = TimeSeriesParams.metricParam(
m -> toType(m).metricType,
TimeSeriesParams.MetricType.gauge,
TimeSeriesParams.MetricType.counter
TimeSeriesParams.MetricType.GAUGE,
TimeSeriesParams.MetricType.COUNTER
).addValidator(v -> {
if (v != null && hasDocValues.getValue() == false) {
throw new IllegalArgumentException(
Expand Down Expand Up @@ -287,7 +287,7 @@ public IndexFieldData.Builder fielddataBuilder(FieldDataContext fieldDataContext
failIfNoDocValues();
}

ValuesSourceType valuesSourceType = metricType == TimeSeriesParams.MetricType.counter
ValuesSourceType valuesSourceType = metricType == TimeSeriesParams.MetricType.COUNTER
? TimeSeriesValuesSourceType.COUNTER
: IndexNumericFieldData.NumericType.LONG.getValuesSourceType();
if ((operation == FielddataOperation.SEARCH || operation == FielddataOperation.SCRIPT) && hasDocValues()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ public void setUp() throws Exception {
.endObject()
.startObject("some_metric")
.field("type", "long")
.field("time_series_metric", TimeSeriesParams.MetricType.counter)
.field("time_series_metric", TimeSeriesParams.MetricType.COUNTER)
.endObject()
.startObject("secret_soundtrack")
.field("type", "alias")
Expand Down Expand Up @@ -153,7 +153,7 @@ public void setUp() throws Exception {
.endObject()
.startObject("some_metric")
.field("type", "long")
.field("time_series_metric", TimeSeriesParams.MetricType.gauge)
.field("time_series_metric", TimeSeriesParams.MetricType.GAUGE)
.endObject()
.endObject()
.endObject()
Expand Down Expand Up @@ -368,7 +368,7 @@ public void testFieldMetricsAndDimensions() {
assertTrue(response.get().get("some_dimension").get("keyword").isDimension());
assertNull(response.get().get("some_dimension").get("keyword").nonDimensionIndices());
assertTrue(response.get().containsKey("some_metric"));
assertEquals(TimeSeriesParams.MetricType.counter, response.get().get("some_metric").get("long").getMetricType());
assertEquals(TimeSeriesParams.MetricType.COUNTER, response.get().get("some_metric").get("long").getMetricType());
assertNull(response.get().get("some_metric").get("long").metricConflictsIndices());

response = client().prepareFieldCaps("old_index", "new_index").setFields("some_dimension", "some_metric").get();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ public FieldCapabilities(
isSearchable,
isAggregatable,
isDimension == null ? false : isDimension,
metricType != null ? Enum.valueOf(TimeSeriesParams.MetricType.class, metricType) : null,
metricType != null ? TimeSeriesParams.MetricType.fromString(metricType) : null,
indices != null ? indices.toArray(new String[0]) : null,
nonSearchableIndices != null ? nonSearchableIndices.toArray(new String[0]) : null,
nonAggregatableIndices != null ? nonAggregatableIndices.toArray(new String[0]) : null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ protected AbstractGeometryFieldMapper(
MultiFields multiFields,
CopyTo copyTo,
Parser<T> parser,
String onScriptError
OnScriptError onScriptError
) {
super(simpleName, mappedFieldType, multiFields, copyTo, true, onScriptError);
this.ignoreMalformed = Explicit.EXPLICIT_FALSE;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ protected AbstractPointGeometryFieldMapper(
MultiFields multiFields,
CopyTo copyTo,
Parser<T> parser,
String onScriptError
OnScriptError onScriptError
) {
super(simpleName, mappedFieldType, multiFields, copyTo, parser, onScriptError);
this.nullValue = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,10 @@ abstract static class Builder<Factory> extends RuntimeField.Builder {
Objects::toString
).setSerializerCheck((id, ic, v) -> ic);

private final FieldMapper.Parameter<String> onScriptError = FieldMapper.Parameter.onScriptErrorParam(m -> m.onScriptError, script);
private final FieldMapper.Parameter<OnScriptError> onScriptError = FieldMapper.Parameter.onScriptErrorParam(
m -> m.onScriptError,
script
);

Builder(String name, ScriptContext<Factory> scriptContext) {
super(name);
Expand Down Expand Up @@ -270,14 +273,7 @@ final RuntimeField createRuntimeField(Factory scriptFactory) {
}

final RuntimeField createRuntimeField(Factory scriptFactory, Version indexVersion) {
var fieldType = createFieldType(
name,
scriptFactory,
getScript(),
meta(),
indexVersion,
OnScriptError.fromString(onScriptError.get())
);
var fieldType = createFieldType(name, scriptFactory, getScript(), meta(), indexVersion, onScriptError.get());
return new LeafRuntimeField(name, fieldType, getParameters());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public static class Builder extends FieldMapper.Builder {
).acceptsNull();

private final Parameter<Script> script = Parameter.scriptParam(m -> toType(m).script);
private final Parameter<String> onScriptError = Parameter.onScriptErrorParam(m -> toType(m).onScriptError, script);
private final Parameter<OnScriptError> onScriptError = Parameter.onScriptErrorParam(m -> toType(m).onScriptError, script);

private final Parameter<Map<String, String>> meta = Parameter.metaParam();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,10 @@ public class CompositeRuntimeField implements RuntimeField {
}
});

private final FieldMapper.Parameter<String> onScriptError = FieldMapper.Parameter.onScriptErrorParam(m -> m.onScriptError, script);
private final FieldMapper.Parameter<OnScriptError> onScriptError = FieldMapper.Parameter.onScriptErrorParam(
m -> m.onScriptError,
script
);

private final FieldMapper.Parameter<Map<String, Object>> fields = new FieldMapper.Parameter<Map<String, Object>>(
"fields",
Expand Down Expand Up @@ -85,15 +88,12 @@ protected RuntimeField createChildRuntimeField(
@Override
protected RuntimeField createRuntimeField(MappingParserContext parserContext) {
CompositeFieldScript.Factory factory = parserContext.scriptCompiler().compile(script.get(), CompositeFieldScript.CONTEXT);
Function<RuntimeField.Builder, RuntimeField> builder = b -> {
OnScriptError onScriptError = OnScriptError.fromString(this.onScriptError.get());
return b.createChildRuntimeField(
parserContext,
name,
lookup -> factory.newFactory(name, script.get().getParams(), lookup, onScriptError),
onScriptError
);
};
Function<RuntimeField.Builder, RuntimeField> builder = b -> b.createChildRuntimeField(
parserContext,
name,
lookup -> factory.newFactory(name, script.get().getParams(), lookup, onScriptError.get()),
onScriptError.get()
);
Map<String, RuntimeField> runtimeFields = RuntimeField.parseRuntimeFields(
new HashMap<>(fields.getValue()),
parserContext,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ public static class Builder extends FieldMapper.Builder {
private final Parameter<Boolean> ignoreMalformed;

private final Parameter<Script> script = Parameter.scriptParam(m -> toType(m).script);
private final Parameter<String> onScriptError = Parameter.onScriptErrorParam(m -> toType(m).onScriptError, script);
private final Parameter<OnScriptError> onScriptError = Parameter.onScriptErrorParam(m -> toType(m).onScriptError, script);

private final Resolution resolution;
private final Version indexCreatedVersion;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public abstract class FieldMapper extends Mapper implements Cloneable {
protected final MultiFields multiFields;
protected final CopyTo copyTo;
protected final boolean hasScript;
protected final String onScriptError;
protected final OnScriptError onScriptError;

/**
* @param simpleName the leaf name of the mapper
Expand All @@ -96,7 +96,7 @@ protected FieldMapper(
MultiFields multiFields,
CopyTo copyTo,
boolean hasScript,
String onScriptError
OnScriptError onScriptError
) {
super(simpleName);
// could be blank but not empty on indices created < 8.6.0
Expand Down Expand Up @@ -247,7 +247,7 @@ public final void executeScript(
try {
indexScriptValues(searchLookup, readerContext, doc, documentParserContext);
} catch (Exception e) {
if ("continue".equals(onScriptError)) {
if (onScriptError == OnScriptError.CONTINUE) {
documentParserContext.addIgnoredField(name());
} else {
throw new MapperParsingException("Error executing script on field [" + name() + "]", e);
Expand Down Expand Up @@ -938,36 +938,41 @@ public static <T extends Enum<T>> Parameter<T> enumParam(
/**
* Defines a parameter that takes one of a restricted set of values from an enumeration.
*
* @param name the parameter name
* @param updateable whether the parameter can be changed by a mapping update
* @param initializer a function that reads the parameter value from an existing mapper
* @param defaultValue the default value, to be used if the parameter is undefined in a mapping
* @param enumClass the enumeration class the parameter takes values from
* @param values the set of values that the parameter can take
* @param name the parameter name
* @param updateable whether the parameter can be changed by a mapping update
* @param initializer a function that reads the parameter value from an existing mapper
* @param defaultValue the default value, to be used if the parameter is undefined in a mapping
* @param enumClass the enumeration class the parameter takes values from
* @param acceptedValues the set of values that the parameter can take
*/
public static <T extends Enum<T>> Parameter<T> restrictedEnumParam(
String name,
boolean updateable,
Function<FieldMapper, T> initializer,
T defaultValue,
Class<T> enumClass,
Set<T> values
Set<T> acceptedValues
) {
assert values.size() > 0;
return new Parameter<T>(name, updateable, () -> defaultValue, (n, c, o) -> {
assert acceptedValues.size() > 0;
return new Parameter<>(name, updateable, () -> defaultValue, (n, c, o) -> {
if (o == null) {
return defaultValue;
}
try {
@SuppressWarnings("unchecked")
T enumValue = Enum.valueOf(enumClass, (String) o);
return enumValue;
} catch (IllegalArgumentException e) {
throw new MapperParsingException("Unknown value [" + o + "] for field [" + name + "] - accepted values are " + values);
EnumSet<T> enumSet = EnumSet.allOf(enumClass);
for (T t : enumSet) {
// the string representation may differ from the actual name of the enum type (e.g. lowercase vs uppercase)
if (t.toString().equals(o.toString())) {
return t;
}
}
throw new MapperParsingException(
"Unknown value [" + o + "] for field [" + name + "] - accepted values are " + acceptedValues
);
}, initializer, XContentBuilder::field, Objects::toString).addValidator(v -> {
if (v != null && values.contains(v) == false) {
throw new MapperParsingException("Unknown value [" + v + "] for field [" + name + "] - accepted values are " + values);
if (v != null && acceptedValues.contains(v) == false) {
throw new MapperParsingException(
"Unknown value [" + v + "] for field [" + name + "] - accepted values are " + acceptedValues
);
}
});
}
Expand Down Expand Up @@ -1069,29 +1074,12 @@ public static Parameter<Script> scriptParam(Function<FieldMapper, Script> initia
* @param dependentScriptParam the corresponding required script parameter
* @return a new on_error_script parameter
*/
public static Parameter<String> onScriptErrorParam(
Function<FieldMapper, String> initializer,
public static Parameter<OnScriptError> onScriptErrorParam(
Function<FieldMapper, OnScriptError> initializer,
Parameter<Script> dependentScriptParam
) {
return new Parameter<>(
"on_script_error",
true,
() -> "fail",
(n, c, o) -> XContentMapValues.nodeStringValue(o),
initializer,
XContentBuilder::field,
Function.identity()
).addValidator(v -> {
switch (v) {
case "fail":
case "continue":
return;
default:
throw new MapperParsingException(
"Unknown value [" + v + "] for field [on_script_error] - accepted values are [fail, continue]"
);
}
}).requiresParameter(dependentScriptParam);
return Parameter.enumParam("on_script_error", true, initializer, OnScriptError.FAIL, OnScriptError.class)
.requiresParameter(dependentScriptParam);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public static class Builder extends FieldMapper.Builder {
final Parameter<Boolean> hasDocValues = Parameter.docValuesParam(m -> builder(m).hasDocValues.get(), true);
final Parameter<Boolean> stored = Parameter.storeParam(m -> builder(m).stored.get(), false);
private final Parameter<Script> script = Parameter.scriptParam(m -> builder(m).script.get());
private final Parameter<String> onScriptError = Parameter.onScriptErrorParam(m -> builder(m).onScriptError.get(), script);
private final Parameter<OnScriptError> onScriptError = Parameter.onScriptErrorParam(m -> builder(m).onScriptError.get(), script);
final Parameter<Map<String, String>> meta = Parameter.metaParam();

private final ScriptCompiler scriptCompiler;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public static class Builder extends FieldMapper.Builder {
.acceptsNull();

private final Parameter<Script> script = Parameter.scriptParam(m -> toType(m).script);
private final Parameter<String> onScriptError = Parameter.onScriptErrorParam(m -> toType(m).onScriptError, script);
private final Parameter<OnScriptError> onScriptError = Parameter.onScriptErrorParam(m -> toType(m).onScriptError, script);

private final Parameter<Map<String, String>> meta = Parameter.metaParam();
private final Parameter<Boolean> dimension;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ public static class Builder extends FieldMapper.Builder {
private final Parameter<Map<String, String>> meta = Parameter.metaParam();

private final Parameter<Script> script = Parameter.scriptParam(m -> toType(m).script);
private final Parameter<String> onScriptError = Parameter.onScriptErrorParam(m -> toType(m).onScriptError, script);
private final Parameter<OnScriptError> onScriptError = Parameter.onScriptErrorParam(m -> toType(m).onScriptError, script);
private final Parameter<Boolean> dimension;

private final IndexAnalyzers indexAnalyzers;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public static class Builder extends FieldMapper.Builder {
private final Parameter<Number> nullValue;

private final Parameter<Script> script = Parameter.scriptParam(m -> toType(m).script);
private final Parameter<String> onScriptError = Parameter.onScriptErrorParam(m -> toType(m).onScriptError, script);
private final Parameter<OnScriptError> onScriptError = Parameter.onScriptErrorParam(m -> toType(m).onScriptError, script);

/**
* Parameter that marks this field as a time series dimension.
Expand Down Expand Up @@ -178,7 +178,7 @@ public Builder(
}
});

this.metric = TimeSeriesParams.metricParam(m -> toType(m).metricType, MetricType.gauge, MetricType.counter).addValidator(v -> {
this.metric = TimeSeriesParams.metricParam(m -> toType(m).metricType, MetricType.GAUGE, MetricType.COUNTER).addValidator(v -> {
if (v != null && hasDocValues.getValue() == false) {
throw new IllegalArgumentException(
"Field [" + TimeSeriesParams.TIME_SERIES_METRIC_PARAM + "] requires that [" + hasDocValues.name + "] is true"
Expand Down Expand Up @@ -1551,7 +1551,7 @@ public IndexFieldData.Builder fielddataBuilder(FieldDataContext fieldDataContext
failIfNoDocValues();
}

ValuesSourceType valuesSourceType = metricType == TimeSeriesParams.MetricType.counter
ValuesSourceType valuesSourceType = metricType == TimeSeriesParams.MetricType.COUNTER
? TimeSeriesValuesSourceType.COUNTER
: type.numericType.getValuesSourceType();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
package org.elasticsearch.index.mapper;

import java.util.Locale;
import java.util.Objects;

/**
* Represents the behaviour when a runtime field or an index-time script fails: either fail and raise the error,
Expand All @@ -19,15 +18,8 @@ public enum OnScriptError {
FAIL,
CONTINUE;

/**
* Parses the on_script_error parameter from a string into its corresponding enum instance
*/
public static OnScriptError fromString(final String str) {
Objects.requireNonNull(str, "input string is null");
return switch (str.toLowerCase(Locale.ROOT)) {
case "fail" -> FAIL;
case "continue" -> CONTINUE;
default -> throw new IllegalArgumentException("Unknown onScriptError [" + str + "]");
};
@Override
public final String toString() {
return name().toLowerCase(Locale.ROOT);
}
}
Loading