From ba9bc2ab649be7b6112e9b7efbd7589ff560beb7 Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Wed, 11 Jan 2023 23:28:54 +0100 Subject: [PATCH] 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. --- .../mapper/extras/ScaledFloatFieldMapper.java | 6 +- .../search/fieldcaps/FieldCapabilitiesIT.java | 6 +- .../action/fieldcaps/FieldCapabilities.java | 2 +- .../mapper/AbstractGeometryFieldMapper.java | 2 +- .../AbstractPointGeometryFieldMapper.java | 2 +- .../index/mapper/AbstractScriptFieldType.java | 14 ++-- .../index/mapper/BooleanFieldMapper.java | 2 +- .../index/mapper/CompositeRuntimeField.java | 20 +++--- .../index/mapper/DateFieldMapper.java | 2 +- .../index/mapper/FieldMapper.java | 70 ++++++++----------- .../index/mapper/GeoPointFieldMapper.java | 2 +- .../index/mapper/IpFieldMapper.java | 2 +- .../index/mapper/KeywordFieldMapper.java | 2 +- .../index/mapper/NumberFieldMapper.java | 6 +- .../index/mapper/OnScriptError.java | 14 +--- .../index/mapper/TimeSeriesParams.java | 19 ++++- .../vectors/DenseVectorFieldMapper.java | 26 ++++--- .../fieldcaps/FieldCapabilitiesTests.java | 18 ++--- .../MergedFieldCapabilitiesResponseTests.java | 2 +- .../index/mapper/NumberFieldMapperTests.java | 6 +- .../index/mapper/ParametrizedMapperTests.java | 27 ++++--- .../vectors/DenseVectorFieldMapperTests.java | 14 ++-- .../vectors/DenseVectorFieldTypeTests.java | 14 ++-- .../index/mapper/MapperTestCase.java | 2 +- .../rate/TimeSeriesRateAggregatorTests.java | 2 +- .../AggregateDoubleMetricFieldMapper.java | 2 +- .../unsignedlong/UnsignedLongFieldMapper.java | 4 +- .../xpack/downsample/FieldValueFetcher.java | 4 +- .../downsample/TimeseriesFieldTypeHelper.java | 2 +- .../downsample/TransportRollupAction.java | 4 +- .../DownsampleActionSingleNodeTests.java | 6 +- 31 files changed, 153 insertions(+), 151 deletions(-) diff --git a/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/ScaledFloatFieldMapper.java b/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/ScaledFloatFieldMapper.java index ae088d5fed02e..46c3a478a33a5 100644 --- a/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/ScaledFloatFieldMapper.java +++ b/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()) { diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java index 70f76c3de7a53..7646778bd8188 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java +++ b/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(); diff --git a/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilities.java b/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilities.java index eec23999dedca..9888240021c37 100644 --- a/server/src/main/java/org/elasticsearch/action/fieldcaps/FieldCapabilities.java +++ b/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, diff --git a/server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java index a6db9824c756f..c481efeb3a23b 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/AbstractGeometryFieldMapper.java @@ -163,7 +163,7 @@ protected AbstractGeometryFieldMapper( MultiFields multiFields, CopyTo copyTo, Parser parser, - String onScriptError + OnScriptError onScriptError ) { super(simpleName, mappedFieldType, multiFields, copyTo, true, onScriptError); this.ignoreMalformed = Explicit.EXPLICIT_FALSE; diff --git a/server/src/main/java/org/elasticsearch/index/mapper/AbstractPointGeometryFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/AbstractPointGeometryFieldMapper.java index 571f0d59a9f20..14db646d86849 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/AbstractPointGeometryFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/AbstractPointGeometryFieldMapper.java @@ -54,7 +54,7 @@ protected AbstractPointGeometryFieldMapper( MultiFields multiFields, CopyTo copyTo, Parser parser, - String onScriptError + OnScriptError onScriptError ) { super(simpleName, mappedFieldType, multiFields, copyTo, parser, onScriptError); this.nullValue = null; diff --git a/server/src/main/java/org/elasticsearch/index/mapper/AbstractScriptFieldType.java b/server/src/main/java/org/elasticsearch/index/mapper/AbstractScriptFieldType.java index 684adb9ac124a..b84294168397c 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/AbstractScriptFieldType.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/AbstractScriptFieldType.java @@ -225,7 +225,10 @@ abstract static class Builder extends RuntimeField.Builder { Objects::toString ).setSerializerCheck((id, ic, v) -> ic); - private final FieldMapper.Parameter onScriptError = FieldMapper.Parameter.onScriptErrorParam(m -> m.onScriptError, script); + private final FieldMapper.Parameter onScriptError = FieldMapper.Parameter.onScriptErrorParam( + m -> m.onScriptError, + script + ); Builder(String name, ScriptContext 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()); } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/BooleanFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/BooleanFieldMapper.java index b69d698f0f38f..63d9fe5c6b2b2 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/BooleanFieldMapper.java +++ b/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