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

)

We have a couple of existing enum mappings parameters that specify they enum type in lowercase letters.
That is convenient to avoid having to convert enum names back and to uppercase or lowercase depending
on what's needed, yet it does not follow coding conventions in that constants should be in capital letters.
More importantly, moving the `on_script_error` mapping parameter to a streamlined enum mapping parameter is
not possible with lowercase type names because one of its values is `continue` which is a java reserved keyword.
It becomes a requirement that the actual value for an enum based mapping parameter can potentially differ from
the enum name. In general, the type name will be in capital letters, while the parameter value will be lowercase.

With this commit we refactor the enum mappings parameter to provide their types in capital case, while
users will keep on providing the corresponding values in lowercase. This only affects how the enum
types are represented internally. We can leverage toString for the enum types to do the lowercasing when needed.

modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/ScaledFloatFieldMapper.java

@@ -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(
@@ -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()) {

server/src/internalClusterTest/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java

@@ -125,7 +125,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")
@@ -168,7 +168,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()
@@ -393,7 +393,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();

server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilities.java

@@ -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,

server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java

@@ -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;

server/src/main/java/org/elasticsearch/index/mapper/AbstractPointGeometryFieldMapper.java

@@ -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;

server/src/main/java/org/elasticsearch/index/mapper/AbstractScriptFieldType.java

@@ -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);
@@ -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());
         }
 

server/src/main/java/org/elasticsearch/index/mapper/BooleanFieldMapper.java

@@ -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();
 

server/src/main/java/org/elasticsearch/index/mapper/CompositeRuntimeField.java

@@ -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",
@@ -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,

server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java

@@ -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;

server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java

@@ -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
@@ -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
@@ -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);
@@ -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
+                    );
                 }
             });
         }
@@ -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);
         }
     }
 

server/src/main/java/org/elasticsearch/index/mapper/GeoPointFieldMapper.java

@@ -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;

server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java

@@ -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;

server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java

@@ -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;

server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java

@@ -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.
@@ -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"
@@ -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();
 

server/src/main/java/org/elasticsearch/index/mapper/OnScriptError.java

@@ -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,
@@ -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);
     }
 }