From a285bfc4f2e51bb532c4c57cb636013ba01161f3 Mon Sep 17 00:00:00 2001
From: Christos Soulios <csoulios@gmail.com>
Date: Thu, 29 Jul 2021 19:19:51 +0300
Subject: [PATCH 01/11] Added metric mapping parameter for Number fields

---
 .../index/mapper/NumberFieldMapper.java       | 68 +++++++++++++++++--
 .../fielddata/IndexFieldDataServiceTests.java |  4 +-
 .../index/mapper/NumberFieldMapperTests.java  | 31 +++++++++
 .../index/mapper/NumberFieldTypeTests.java    |  2 +-
 .../aggregations/AggregatorBaseTests.java     |  3 +-
 .../bucket/filter/FiltersAggregatorTests.java |  6 +-
 .../bucket/range/RangeAggregatorTests.java    |  9 ++-
 .../search/collapse/CollapseBuilderTests.java |  4 +-
 8 files changed, 110 insertions(+), 17 deletions(-)

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java
index e66f829c9cfc9..d0ce555729b12 100644
--- a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java
+++ b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java
@@ -63,6 +63,18 @@
 /** A {@link FieldMapper} for numeric types: byte, short, int, long, float and double. */
 public class NumberFieldMapper extends FieldMapper {
 
+    public enum MetricType {
+        gauge,
+        counter;
+
+        public static MetricType fromString(String name) {
+            return Arrays.stream(values())
+                .filter(v -> v.name().equalsIgnoreCase(name))
+                .findFirst()
+                .orElse(null);
+        }
+    }
+
     public static final Setting<Boolean> COERCE_SETTING =
             Setting.boolSetting("index.mapping.coerce", true, Property.IndexScope);
 
@@ -84,7 +96,7 @@ public static class Builder extends FieldMapper.Builder {
         private final Parameter<Script> script = Parameter.scriptParam(m -> toType(m).builder.script.get());
         private final Parameter<String> onScriptError = Parameter.onScriptErrorParam(m -> toType(m).onScriptError, script);
         private final Parameter<Boolean> dimension;
-
+        private final Parameter<String> metric;
 
         private final Parameter<Map<String, String>> meta = Parameter.metaParam();
 
@@ -125,6 +137,18 @@ public Builder(String name, NumberType type, ScriptCompiler compiler, boolean ig
                         );
                     }
                 });
+            this.metric = Parameter.restrictedStringParam(
+                "metric",
+                false,
+                m -> toType(m).metricType != null ? toType(m).metricType.name() : null,
+                null,
+                MetricType.gauge.name(),
+                MetricType.counter.name()
+            ).acceptsNull().setValidator(v -> {
+                if (v != null && v.isEmpty() == false && hasDocValues.getValue() == false) {
+                    throw new IllegalArgumentException("Field [metric] requires that [" + hasDocValues.name + "] is true");
+                }
+            });
 
             this.script.precludesParameters(ignoreMalformed, coerce, nullValue);
             addScriptValidation(script, indexed, hasDocValues);
@@ -152,9 +176,26 @@ public Builder dimension(boolean dimension) {
             return this;
         }
 
+        public Builder metric(String metric) {
+            this.metric.setValue(metric);
+            return this;
+        }
+
         @Override
         protected List<Parameter<?>> getParameters() {
-            return List.of(indexed, hasDocValues, stored, ignoreMalformed, coerce, nullValue, script, onScriptError, meta, dimension);
+            return List.of(
+                indexed,
+                hasDocValues,
+                stored,
+                ignoreMalformed,
+                coerce,
+                nullValue,
+                script,
+                onScriptError,
+                meta,
+                dimension,
+                metric
+            );
         }
 
         @Override
@@ -967,26 +1008,29 @@ public static class NumberFieldType extends SimpleMappedFieldType {
         private final Number nullValue;
         private final FieldValues<Number> scriptValues;
         private final boolean isDimension;
+        private final MetricType metricType;
 
         public NumberFieldType(String name, NumberType type, boolean isSearchable, boolean isStored,
                                boolean hasDocValues, boolean coerce, Number nullValue, Map<String, String> meta,
-                               FieldValues<Number> script, boolean isDimension) {
+                               FieldValues<Number> script, boolean isDimension, MetricType metricType) {
             super(name, isSearchable, isStored, hasDocValues, TextSearchInfo.SIMPLE_MATCH_WITHOUT_TERMS, meta);
             this.type = Objects.requireNonNull(type);
             this.coerce = coerce;
             this.nullValue = nullValue;
             this.scriptValues = script;
             this.isDimension = isDimension;
+            this.metricType = metricType;
         }
 
         NumberFieldType(String name, Builder builder) {
             this(name, builder.type, builder.indexed.getValue(), builder.stored.getValue(), builder.hasDocValues.getValue(),
                 builder.coerce.getValue().value(), builder.nullValue.getValue(), builder.meta.getValue(),
-                builder.scriptValues(), builder.dimension.getValue());
+                builder.scriptValues(), builder.dimension.getValue(),
+                MetricType.fromString(builder.metric.getValue()));
         }
 
         public NumberFieldType(String name, NumberType type) {
-            this(name, type, true, false, true, true, null, Collections.emptyMap(), null, false);
+            this(name, type, true, false, true, true, null, Collections.emptyMap(), null, false, null);
         }
 
         @Override
@@ -1086,6 +1130,14 @@ public CollapseType collapseType() {
         public boolean isDimension() {
             return isDimension;
         }
+
+        /**
+         * If field is a time series metric field, returns its metric type
+         * @return the metric type or null
+         */
+        public MetricType getMetricType() {
+            return metricType;
+        }
     }
 
     private final Builder builder;
@@ -1101,6 +1153,7 @@ public boolean isDimension() {
     private final boolean ignoreMalformedByDefault;
     private final boolean coerceByDefault;
     private final boolean dimension;
+    private final MetricType metricType;
 
     private NumberFieldMapper(
             String simpleName,
@@ -1120,6 +1173,7 @@ private NumberFieldMapper(
         this.coerceByDefault = builder.coerce.getDefaultValue().value();
         this.scriptValues = builder.scriptValues();
         this.dimension = builder.dimension.getValue();
+        this.metricType = MetricType.fromString(builder.metric.getValue());
         this.builder = builder;
     }
 
@@ -1208,6 +1262,8 @@ protected void indexScriptValues(SearchLookup searchLookup, LeafReaderContext re
     @Override
     public FieldMapper.Builder getMergeBuilder() {
         return new Builder(simpleName(), type, builder.scriptCompiler, ignoreMalformedByDefault, coerceByDefault)
-            .dimension(dimension).init(this);
+            .dimension(dimension)
+            .metric(metricType != null ? metricType.name() : null)
+            .init(this);
     }
 }
diff --git a/server/src/test/java/org/elasticsearch/index/fielddata/IndexFieldDataServiceTests.java b/server/src/test/java/org/elasticsearch/index/fielddata/IndexFieldDataServiceTests.java
index f855712ca240d..68f1e6fad2da3 100644
--- a/server/src/test/java/org/elasticsearch/index/fielddata/IndexFieldDataServiceTests.java
+++ b/server/src/test/java/org/elasticsearch/index/fielddata/IndexFieldDataServiceTests.java
@@ -311,13 +311,13 @@ private void doTestRequireDocValues(MappedFieldType ft) {
     public void testRequireDocValuesOnLongs() {
         doTestRequireDocValues(new NumberFieldMapper.NumberFieldType("field", LONG));
         doTestRequireDocValues(new NumberFieldMapper.NumberFieldType("field", LONG,
-            true, false, false, false, null, Collections.emptyMap(), null, false));
+            true, false, false, false, null, Collections.emptyMap(), null, false, null));
     }
 
     public void testRequireDocValuesOnDoubles() {
         doTestRequireDocValues(new NumberFieldMapper.NumberFieldType("field", NumberType.DOUBLE));
         doTestRequireDocValues(new NumberFieldMapper.NumberFieldType("field", NumberType.DOUBLE,
-            true, false, false, false, null, Collections.emptyMap(), null, false));
+            true, false, false, false, null, Collections.emptyMap(), null, false, null));
     }
 
     public void testRequireDocValuesOnBools() {
diff --git a/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java
index 01ee8995e004c..c7fdfae1a1d29 100644
--- a/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java
+++ b/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java
@@ -21,6 +21,7 @@
 
 import java.io.IOException;
 import java.util.List;
+import java.util.function.Function;
 
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
@@ -260,6 +261,36 @@ public void testDimension() throws IOException {
         assertThat(e.getCause().getMessage(), containsString("Parameter [dimension] cannot be set"));
     }
 
+    protected <T> void assertMetricType(String metricType, Function<T, Enum> checker)
+        throws IOException {
+        MapperService mapperService = createMapperService(fieldMapping(b -> {
+            minimalMapping(b);
+            b.field("metric", metricType);
+        }));
+
+        @SuppressWarnings("unchecked") // Syntactic sugar in tests
+        T fieldType = (T) mapperService.fieldType("field");
+        assertThat(checker.apply(fieldType).name(), equalTo(metricType));
+    }
+
+    public void testMetricType() throws IOException {
+        // Test default setting
+        MapperService mapperService = createMapperService(fieldMapping(b -> minimalMapping(b)));
+        NumberFieldMapper.NumberFieldType ft = (NumberFieldMapper.NumberFieldType) mapperService.fieldType("field");
+        assertNull(ft.getMetricType());
+
+        assertMetricType("gauge", NumberFieldMapper.NumberFieldType::getMetricType);
+        assertMetricType("counter", NumberFieldMapper.NumberFieldType::getMetricType);
+    }
+
+    public void testMetricAndDocvalues() {
+        Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> {
+            minimalMapping(b);
+            b.field("metric", "counter").field("doc_values", false);
+        })));
+        assertThat(e.getCause().getMessage(), containsString("Field [metric] requires that [doc_values] is true"));
+    }
+
     @Override
     protected final Object generateRandomInputValue(MappedFieldType ft) {
         Number n = randomNumber();
diff --git a/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java b/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java
index f4ce2f36bdfb7..c0607eb9a45d2 100644
--- a/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java
+++ b/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldTypeTests.java
@@ -122,7 +122,7 @@ public void testLongTermQueryWithDecimalPart() {
     }
 
     private static MappedFieldType unsearchable() {
-        return new NumberFieldType("field", NumberType.LONG, false, false, true, true, null, Collections.emptyMap(), null, false);
+        return new NumberFieldType("field", NumberType.LONG, false, false, true, true, null, Collections.emptyMap(), null, false, null);
     }
 
     public void testTermQuery() {
diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/AggregatorBaseTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/AggregatorBaseTests.java
index 297e702cde6d2..5e6112d4dae34 100644
--- a/server/src/test/java/org/elasticsearch/search/aggregations/AggregatorBaseTests.java
+++ b/server/src/test/java/org/elasticsearch/search/aggregations/AggregatorBaseTests.java
@@ -89,7 +89,8 @@ private ValuesSourceConfig getVSConfig(
             null,
             Collections.emptyMap(),
             null,
-            false
+            false,
+            null
         );
         return ValuesSourceConfig.resolveFieldOnly(ft, context);
     }
diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/filter/FiltersAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/filter/FiltersAggregatorTests.java
index 075e177fab3fe..6e92c6bfd1a8f 100644
--- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/filter/FiltersAggregatorTests.java
+++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/filter/FiltersAggregatorTests.java
@@ -1062,7 +1062,8 @@ public void testDocValuesFieldExistsForNumber() throws IOException {
             null,
             Map.of(),
             null,
-            false
+            false,
+            null
         );
         docValuesFieldExistsTestCase(new ExistsQueryBuilder("f"), ft, true, i -> {
             return numberType.createFields("f", i, true, true, false);
@@ -1080,7 +1081,8 @@ public void testDocValuesFieldExistsForNumberWithoutData() throws IOException {
             null,
             Map.of(),
             null,
-            false
+            false,
+            null
         ));
     }
 
diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/range/RangeAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/range/RangeAggregatorTests.java
index 4e037402583a1..d91f66dc0c1d1 100644
--- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/range/RangeAggregatorTests.java
+++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/range/RangeAggregatorTests.java
@@ -146,7 +146,8 @@ public void testUnboundedRanges() throws IOException {
                 null,
                 Collections.emptyMap(),
                 null,
-                false
+                false,
+                null
             )
         );
     }
@@ -242,7 +243,8 @@ public void testNotFitIntoDouble() throws IOException {
             null,
             Collections.emptyMap(),
             null,
-            false
+            false,
+            null
         );
 
         long start = 2L << 54; // Double stores 53 bits of mantissa, so we aggregate a bunch of bigger values
@@ -515,7 +517,8 @@ private void testCase(
             null,
             Collections.emptyMap(),
             null,
-            false
+            false,
+            null
         );
         RangeAggregationBuilder aggregationBuilder = new RangeAggregationBuilder("test_range_agg");
         aggregationBuilder.field(NUMBER_FIELD_NAME);
diff --git a/server/src/test/java/org/elasticsearch/search/collapse/CollapseBuilderTests.java b/server/src/test/java/org/elasticsearch/search/collapse/CollapseBuilderTests.java
index 811fad16fb2d4..cff84c07019fb 100644
--- a/server/src/test/java/org/elasticsearch/search/collapse/CollapseBuilderTests.java
+++ b/server/src/test/java/org/elasticsearch/search/collapse/CollapseBuilderTests.java
@@ -145,14 +145,14 @@ public void testBuild() throws IOException {
 
             numberFieldType =
                 new NumberFieldMapper.NumberFieldType("field", NumberFieldMapper.NumberType.LONG, true, false,
-                    false, false, null, Collections.emptyMap(), null, false);
+                    false, false, null, Collections.emptyMap(), null, false, null);
             when(searchExecutionContext.getFieldType("field")).thenReturn(numberFieldType);
             IllegalArgumentException exc = expectThrows(IllegalArgumentException.class, () -> builder.build(searchExecutionContext));
             assertEquals(exc.getMessage(), "cannot collapse on field `field` without `doc_values`");
 
             numberFieldType =
                 new NumberFieldMapper.NumberFieldType("field", NumberFieldMapper.NumberType.LONG, false, false,
-                    true, false, null, Collections.emptyMap(), null, false);
+                    true, false, null, Collections.emptyMap(), null, false, null);
             when(searchExecutionContext.getFieldType("field")).thenReturn(numberFieldType);
             builder.setInnerHits(new InnerHitBuilder());
             exc = expectThrows(IllegalArgumentException.class, () -> builder.build(searchExecutionContext));

From a117873f18734e09ce88616d3f5732f82597bca6 Mon Sep 17 00:00:00 2001
From: Christos Soulios <csoulios@gmail.com>
Date: Wed, 18 Aug 2021 17:40:53 +0300
Subject: [PATCH 02/11] Moved metric param definition to a static method

So that it's reusable.
Added more tests
---
 .../index/mapper/TokenCountFieldMapper.java   | 24 +++++++-
 .../index/mapper/FieldMapper.java             |  4 ++
 .../index/mapper/NumberFieldMapper.java       | 35 +++--------
 .../index/mapper/TimeSeriesParams.java        | 61 +++++++++++++++++++
 .../index/mapper/NumberFieldMapperTests.java  | 10 +++
 5 files changed, 105 insertions(+), 29 deletions(-)
 create mode 100644 server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesParams.java

diff --git a/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/TokenCountFieldMapper.java b/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/TokenCountFieldMapper.java
index 8a1ee05873e18..58c6c466b5a22 100644
--- a/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/TokenCountFieldMapper.java
+++ b/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/TokenCountFieldMapper.java
@@ -75,9 +75,27 @@ public TokenCountFieldMapper build(ContentPath contentPath) {
 
     static class TokenCountFieldType extends NumberFieldMapper.NumberFieldType {
 
-        TokenCountFieldType(String name, boolean isSearchable, boolean isStored,
-                            boolean hasDocValues, Number nullValue, Map<String, String> meta) {
-            super(name, NumberFieldMapper.NumberType.INTEGER, isSearchable, isStored, hasDocValues, false, nullValue, meta, null, false);
+        TokenCountFieldType(
+            String name,
+            boolean isSearchable,
+            boolean isStored,
+            boolean hasDocValues,
+            Number nullValue,
+            Map<String, String> meta
+        ) {
+            super(
+                name,
+                NumberFieldMapper.NumberType.INTEGER,
+                isSearchable,
+                isStored,
+                hasDocValues,
+                false,
+                nullValue,
+                meta,
+                null,
+                false,
+                null
+            );
         }
 
         @Override
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java
index 57d42c43d0c70..06d344b6488be 100644
--- a/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java
+++ b/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java
@@ -688,6 +688,10 @@ public Parameter<T> setValidator(Consumer<T> validator) {
             return this;
         }
 
+        public Consumer<T> getValidator() {
+            return this.validator;
+        }
+
         /**
          * Configure a custom serializer for this parameter
          */
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java
index d0ce555729b12..25cac85bb8289 100644
--- a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java
+++ b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java
@@ -37,6 +37,7 @@
 import org.elasticsearch.index.fielddata.IndexFieldData;
 import org.elasticsearch.index.fielddata.IndexNumericFieldData.NumericType;
 import org.elasticsearch.index.fielddata.plain.SortedNumericIndexFieldData;
+import org.elasticsearch.index.mapper.TimeSeriesParams.MetricType;
 import org.elasticsearch.index.query.SearchExecutionContext;
 import org.elasticsearch.script.DoubleFieldScript;
 import org.elasticsearch.script.LongFieldScript;
@@ -63,18 +64,6 @@
 /** A {@link FieldMapper} for numeric types: byte, short, int, long, float and double. */
 public class NumberFieldMapper extends FieldMapper {
 
-    public enum MetricType {
-        gauge,
-        counter;
-
-        public static MetricType fromString(String name) {
-            return Arrays.stream(values())
-                .filter(v -> v.name().equalsIgnoreCase(name))
-                .findFirst()
-                .orElse(null);
-        }
-    }
-
     public static final Setting<Boolean> COERCE_SETTING =
             Setting.boolSetting("index.mapping.coerce", true, Property.IndexScope);
 
@@ -96,7 +85,13 @@ public static class Builder extends FieldMapper.Builder {
         private final Parameter<Script> script = Parameter.scriptParam(m -> toType(m).builder.script.get());
         private final Parameter<String> onScriptError = Parameter.onScriptErrorParam(m -> toType(m).onScriptError, script);
         private final Parameter<Boolean> dimension;
-        private final Parameter<String> metric;
+        private final Parameter<String> metric = TimeSeriesParams.metricParam(
+            m -> toType(m).metricType != null ? toType(m).metricType.name() : null,
+            hasDocValues,
+            null,
+            MetricType.gauge.name(),
+            MetricType.counter.name()
+        );
 
         private final Parameter<Map<String, String>> meta = Parameter.metaParam();
 
@@ -137,18 +132,6 @@ public Builder(String name, NumberType type, ScriptCompiler compiler, boolean ig
                         );
                     }
                 });
-            this.metric = Parameter.restrictedStringParam(
-                "metric",
-                false,
-                m -> toType(m).metricType != null ? toType(m).metricType.name() : null,
-                null,
-                MetricType.gauge.name(),
-                MetricType.counter.name()
-            ).acceptsNull().setValidator(v -> {
-                if (v != null && v.isEmpty() == false && hasDocValues.getValue() == false) {
-                    throw new IllegalArgumentException("Field [metric] requires that [" + hasDocValues.name + "] is true");
-                }
-            });
 
             this.script.precludesParameters(ignoreMalformed, coerce, nullValue);
             addScriptValidation(script, indexed, hasDocValues);
@@ -1153,7 +1136,7 @@ public MetricType getMetricType() {
     private final boolean ignoreMalformedByDefault;
     private final boolean coerceByDefault;
     private final boolean dimension;
-    private final MetricType metricType;
+    private final TimeSeriesParams.MetricType metricType;
 
     private NumberFieldMapper(
             String simpleName,
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesParams.java b/server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesParams.java
new file mode 100644
index 0000000000000..ec545011d952f
--- /dev/null
+++ b/server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesParams.java
@@ -0,0 +1,61 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+package org.elasticsearch.index.mapper;
+
+import java.util.Arrays;
+import java.util.function.Consumer;
+import java.util.function.Function;
+
+/**
+ * Utility functions for time series related mapper parameters
+ */
+public final class TimeSeriesParams {
+
+    private TimeSeriesParams() {
+    }
+
+    public enum MetricType {
+        gauge,
+        counter,
+        histogram,
+        summary;
+
+        /**
+         * Convert string to MetricType value returning null for null or invalid string values
+         */
+        public static MetricType fromString(String name) {
+            return name != null ? Arrays.stream(values()).filter(v -> v.name().equalsIgnoreCase(name)).findFirst().orElse(null) : null;
+        }
+    }
+
+    public static FieldMapper.Parameter<String> metricParam(
+        Function<FieldMapper, String> initializer,
+        FieldMapper.Parameter<Boolean> hasDocValues,
+        String... values
+    ) {
+        FieldMapper.Parameter<String> param = FieldMapper.Parameter.restrictedStringParam("metric", false, initializer, values)
+            .acceptsNull();
+        // We are overriding the default validator that checks for acceptable values.
+        // So we must call the default validator explicitly from inside our new validator.
+        // TODO: Make parameters support multiple validators
+        final Consumer<String> defaultValidator = param.getValidator();
+        param.setValidator(v -> {
+            // Call the default validator first
+            defaultValidator.accept(v);
+
+            if (v != null && v.isEmpty() == false && hasDocValues != null && hasDocValues.getValue() == false) {
+                throw new IllegalArgumentException("Field [metric] requires that [" + hasDocValues.name + "] is true");
+            }
+        });
+
+        return param;
+    }
+
+
+}
diff --git a/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java
index c7fdfae1a1d29..f2f3f9ecd3996 100644
--- a/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java
+++ b/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java
@@ -281,6 +281,16 @@ public void testMetricType() throws IOException {
 
         assertMetricType("gauge", NumberFieldMapper.NumberFieldType::getMetricType);
         assertMetricType("counter", NumberFieldMapper.NumberFieldType::getMetricType);
+
+        // Test invalid metric type for this field type
+        Exception e = expectThrows(MapperParsingException.class, () -> createMapperService(fieldMapping(b -> {
+            minimalMapping(b);
+            b.field("metric", "histogram");
+        })));
+        assertThat(
+            e.getCause().getMessage(),
+            containsString("Unknown value [histogram] for field [metric] - accepted values are [null, gauge, counter]")
+        );
     }
 
     public void testMetricAndDocvalues() {

From 421a64b8260d324a2fa01b09ebaef9242fd6cb34 Mon Sep 17 00:00:00 2001
From: Christos Soulios <csoulios@gmail.com>
Date: Thu, 19 Aug 2021 17:37:00 +0300
Subject: [PATCH 03/11] Add metric param to aggregate_metric_double fields

---
 .../index/mapper/NumberFieldMapper.java       | 10 ++++
 .../index/mapper/NumberFieldMapperTests.java  | 31 +++++++----
 .../AggregateDoubleMetricFieldMapper.java     | 51 +++++++++++++++++--
 ...AggregateDoubleMetricFieldMapperTests.java | 48 +++++++++++++++++
 .../AggregateDoubleMetricFieldTypeTests.java  |  2 +-
 5 files changed, 127 insertions(+), 15 deletions(-)

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java
index 25cac85bb8289..f78bf5869175a 100644
--- a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java
+++ b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java
@@ -84,7 +84,17 @@ public static class Builder extends FieldMapper.Builder {
 
         private final Parameter<Script> script = Parameter.scriptParam(m -> toType(m).builder.script.get());
         private final Parameter<String> onScriptError = Parameter.onScriptErrorParam(m -> toType(m).onScriptError, script);
+
+        /**
+         * Parameter that marks this field as a time series dimension.
+         */
         private final Parameter<Boolean> dimension;
+
+        /**
+         * Parameter that marks this field as a time series metric defining its time series metric type.
+         * For the numeric fields gauge and counter metric types are
+         * supported
+         */
         private final Parameter<String> metric = TimeSeriesParams.metricParam(
             m -> toType(m).metricType != null ? toType(m).metricType.name() : null,
             hasDocValues,
diff --git a/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java
index f2f3f9ecd3996..62639d73c7a49 100644
--- a/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java
+++ b/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java
@@ -282,15 +282,28 @@ public void testMetricType() throws IOException {
         assertMetricType("gauge", NumberFieldMapper.NumberFieldType::getMetricType);
         assertMetricType("counter", NumberFieldMapper.NumberFieldType::getMetricType);
 
-        // Test invalid metric type for this field type
-        Exception e = expectThrows(MapperParsingException.class, () -> createMapperService(fieldMapping(b -> {
-            minimalMapping(b);
-            b.field("metric", "histogram");
-        })));
-        assertThat(
-            e.getCause().getMessage(),
-            containsString("Unknown value [histogram] for field [metric] - accepted values are [null, gauge, counter]")
-        );
+        {
+            // Test invalid metric type for this field type
+            Exception e = expectThrows(MapperParsingException.class, () -> createMapperService(fieldMapping(b -> {
+                minimalMapping(b);
+                b.field("metric", "histogram");
+            })));
+            assertThat(
+                e.getCause().getMessage(),
+                containsString("Unknown value [histogram] for field [metric] - accepted values are [null, gauge, counter]")
+            );
+        }
+        {
+            // Test invalid metric type for this field type
+            Exception e = expectThrows(MapperParsingException.class, () -> createMapperService(fieldMapping(b -> {
+                minimalMapping(b);
+                b.field("metric", "unknown");
+            })));
+            assertThat(
+                e.getCause().getMessage(),
+                containsString("Unknown value [unknown] for field [metric] - accepted values are [null, gauge, counter]")
+            );
+        }
     }
 
     public void testMetricAndDocvalues() {
diff --git a/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapper.java b/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapper.java
index 8c428794d5d41..34c7956e85360 100644
--- a/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapper.java
+++ b/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapper.java
@@ -32,6 +32,8 @@
 import org.elasticsearch.index.mapper.SimpleMappedFieldType;
 import org.elasticsearch.index.mapper.SourceValueFetcher;
 import org.elasticsearch.index.mapper.TextSearchInfo;
+import org.elasticsearch.index.mapper.TimeSeriesParams;
+import org.elasticsearch.index.mapper.TimeSeriesParams.MetricType;
 import org.elasticsearch.index.mapper.ValueFetcher;
 import org.elasticsearch.index.query.QueryRewriteContext;
 import org.elasticsearch.index.query.SearchExecutionContext;
@@ -130,6 +132,20 @@ public static class Builder extends FieldMapper.Builder {
             }
         });
 
+        /**
+         * Parameter that marks this field as a time series metric defining its time series metric type.
+         * For {@link AggregateDoubleMetricFieldMapper} fields gauge, counter and summary metric types are
+         * supported.
+         */
+        private final Parameter<String> metric = TimeSeriesParams.metricParam(
+            m -> toType(m).metricType != null ? toType(m).metricType.name() : null,
+            null, // This field type always stores its subfields as doc_values
+            null,
+            MetricType.gauge.name(),
+            MetricType.counter.name(),
+            MetricType.summary.name()
+        );
+
         /**
          * Set the default metric so that query operations are delegated to it.
          */
@@ -153,7 +169,12 @@ public Builder(String name, Boolean ignoreMalformedByDefault) {
 
         @Override
         protected List<Parameter<?>> getParameters() {
-            return List.of(ignoreMalformed, metrics, defaultMetric, meta);
+            return List.of(ignoreMalformed, metrics, defaultMetric, meta, metric);
+        }
+
+        public Builder metric(String metric) {
+            this.metric.setValue(metric);
+            return this;
         }
 
         @Override
@@ -216,7 +237,11 @@ public AggregateDoubleMetricFieldMapper build(ContentPath context) {
                     )
                 );
 
-            AggregateDoubleMetricFieldType metricFieldType = new AggregateDoubleMetricFieldType(buildFullName(context), meta.getValue());
+            AggregateDoubleMetricFieldType metricFieldType = new AggregateDoubleMetricFieldType(
+                buildFullName(context),
+                meta.getValue(),
+                metric.getValue() != null ? MetricType.valueOf(metric.getValue()) : null
+            );
             metricFieldType.setMetricFields(metricFields);
             metricFieldType.setDefaultMetric(defaultMetric.getValue());
 
@@ -235,12 +260,15 @@ public static final class AggregateDoubleMetricFieldType extends SimpleMappedFie
 
         private Metric defaultMetric;
 
+        private final MetricType metricType;
+
         public AggregateDoubleMetricFieldType(String name) {
-            this(name, Collections.emptyMap());
+            this(name, Collections.emptyMap(), null);
         }
 
-        public AggregateDoubleMetricFieldType(String name, Map<String, String> meta) {
+        public AggregateDoubleMetricFieldType(String name, Map<String, String> meta, MetricType metricType) {
             super(name, true, false, false, TextSearchInfo.SIMPLE_MATCH_WITHOUT_TERMS, meta);
+            this.metricType = metricType;
         }
 
         /**
@@ -459,6 +487,14 @@ protected Object parseSourceValue(Object value) {
                 }
             };
         }
+
+        /**
+         * If field is a time series metric field, returns its metric type
+         * @return the metric type or null
+         */
+        public MetricType getMetricType() {
+            return metricType;
+        }
     }
 
     private final EnumMap<Metric, NumberFieldMapper> metricFieldMappers;
@@ -473,6 +509,8 @@ protected Object parseSourceValue(Object value) {
     /** The default metric to be when querying this field type */
     protected Metric defaultMetric;
 
+    private final TimeSeriesParams.MetricType metricType;
+
     private AggregateDoubleMetricFieldMapper(
         String simpleName,
         MappedFieldType mappedFieldType,
@@ -485,6 +523,7 @@ private AggregateDoubleMetricFieldMapper(
         this.metrics = builder.metrics.getValue();
         this.defaultMetric = builder.defaultMetric.getValue();
         this.metricFieldMappers = metricFieldMappers;
+        this.metricType = TimeSeriesParams.MetricType.fromString(builder.metric.getValue());
     }
 
     boolean ignoreMalformed() {
@@ -611,6 +650,8 @@ protected void parseCreateField(DocumentParserContext context) throws IOExceptio
 
     @Override
     public FieldMapper.Builder getMergeBuilder() {
-        return new Builder(simpleName(), ignoreMalformedByDefault).init(this);
+        return new Builder(simpleName(), ignoreMalformedByDefault)
+            .metric(metricType != null ? metricType.name() : null)
+            .init(this);
     }
 }
diff --git a/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapperTests.java b/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapperTests.java
index 4a6a165a109fe..de4ab0db73239 100644
--- a/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapperTests.java
+++ b/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapperTests.java
@@ -27,6 +27,7 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.function.Function;
 
 import static org.elasticsearch.xpack.aggregatemetric.mapper.AggregateDoubleMetricFieldMapper.Names.IGNORE_MALFORMED;
 import static org.elasticsearch.xpack.aggregatemetric.mapper.AggregateDoubleMetricFieldMapper.Names.METRICS;
@@ -551,4 +552,51 @@ public void testCannotBeUsedInMultifields() {
         })));
         assertThat(e.getMessage(), containsString("Field [metric] of type [aggregate_metric_double] can't be used in multifields"));
     }
+
+    protected <T> void assertMetricType(String metricType, Function<T, Enum> checker)
+        throws IOException {
+        MapperService mapperService = createMapperService(fieldMapping(b -> {
+            minimalMapping(b);
+            b.field("metric", metricType);
+        }));
+
+        @SuppressWarnings("unchecked") // Syntactic sugar in tests
+        T fieldType = (T) mapperService.fieldType("field");
+        assertThat(checker.apply(fieldType).name(), equalTo(metricType));
+    }
+
+    public void testMetricType() throws IOException {
+        // Test default setting
+        MapperService mapperService = createMapperService(fieldMapping(b -> minimalMapping(b)));
+        AggregateDoubleMetricFieldMapper.AggregateDoubleMetricFieldType ft =
+            (AggregateDoubleMetricFieldMapper.AggregateDoubleMetricFieldType) mapperService.fieldType("field");
+        assertNull(ft.getMetricType());
+
+        assertMetricType("gauge", AggregateDoubleMetricFieldMapper.AggregateDoubleMetricFieldType::getMetricType);
+        assertMetricType("counter", AggregateDoubleMetricFieldMapper.AggregateDoubleMetricFieldType::getMetricType);
+        assertMetricType("summary", AggregateDoubleMetricFieldMapper.AggregateDoubleMetricFieldType::getMetricType);
+
+        {
+            // Test invalid metric type for this field type
+            Exception e = expectThrows(MapperParsingException.class, () -> createMapperService(fieldMapping(b -> {
+                minimalMapping(b);
+                b.field("metric", "histogram");
+            })));
+            assertThat(
+                e.getCause().getMessage(),
+                containsString("Unknown value [histogram] for field [metric] - accepted values are [null, gauge, counter, summary]")
+            );
+        }
+        {
+            // Test invalid metric type for this field type
+            Exception e = expectThrows(MapperParsingException.class, () -> createMapperService(fieldMapping(b -> {
+                minimalMapping(b);
+                b.field("metric", "unknown");
+            })));
+            assertThat(
+                e.getCause().getMessage(),
+                containsString("Unknown value [unknown] for field [metric] - accepted values are [null, gauge, counter, summary]")
+            );
+        }
+    }
 }
diff --git a/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldTypeTests.java b/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldTypeTests.java
index 4fa8c24afcc26..9cfc619ee9cc3 100644
--- a/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldTypeTests.java
+++ b/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldTypeTests.java
@@ -45,7 +45,7 @@
 public class AggregateDoubleMetricFieldTypeTests extends FieldTypeTestCase {
 
     protected AggregateDoubleMetricFieldType createDefaultFieldType(String name, Map<String, String> meta, Metric defaultMetric) {
-        AggregateDoubleMetricFieldType fieldType = new AggregateDoubleMetricFieldType(name, meta);
+        AggregateDoubleMetricFieldType fieldType = new AggregateDoubleMetricFieldType(name, meta, null);
         for (AggregateDoubleMetricFieldMapper.Metric m : List.of(
             AggregateDoubleMetricFieldMapper.Metric.min,
             AggregateDoubleMetricFieldMapper.Metric.max

From e6b36f88c1a11f54729ae506d1523403fe7964a7 Mon Sep 17 00:00:00 2001
From: Christos Soulios <csoulios@gmail.com>
Date: Fri, 20 Aug 2021 16:07:03 +0300
Subject: [PATCH 04/11] Add metric param to histogram fields

---
 .../index/mapper/TimeSeriesParams.java        |  6 +--
 .../mapper/HistogramFieldMapper.java          | 44 +++++++++++++++--
 .../HistoBackedHistogramAggregatorTests.java  |  2 +-
 .../HistoBackedRangeAggregatorTests.java      |  2 +-
 ...regatedPercentileRanksAggregatorTests.java |  2 +-
 ...eAggregatedPercentilesAggregatorTests.java |  2 +-
 .../HistoBackedAvgAggregatorTests.java        |  2 +-
 .../HistoBackedMaxAggregatorTests.java        |  2 +-
 .../HistoBackedMinAggregatorTests.java        |  2 +-
 .../HistoBackedSumAggregatorTests.java        |  2 +-
 .../HistoBackedValueCountAggregatorTests.java |  2 +-
 ...regatedPercentileRanksAggregatorTests.java |  2 +-
 ...eAggregatedPercentilesAggregatorTests.java |  2 +-
 .../mapper/HistogramFieldMapperTests.java     | 48 ++++++++++++++++++-
 .../analytics/rate/RateAggregatorTests.java   | 10 ++--
 .../AggregateDoubleMetricFieldMapper.java     |  3 +-
 16 files changed, 107 insertions(+), 26 deletions(-)

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesParams.java b/server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesParams.java
index ec545011d952f..517bb2e64f7c7 100644
--- a/server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesParams.java
+++ b/server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesParams.java
@@ -8,7 +8,6 @@
 
 package org.elasticsearch.index.mapper;
 
-import java.util.Arrays;
 import java.util.function.Consumer;
 import java.util.function.Function;
 
@@ -27,10 +26,10 @@ public enum MetricType {
         summary;
 
         /**
-         * Convert string to MetricType value returning null for null or invalid string values
+         * Convert string to MetricType value returning null for null values
          */
         public static MetricType fromString(String name) {
-            return name != null ? Arrays.stream(values()).filter(v -> v.name().equalsIgnoreCase(name)).findFirst().orElse(null) : null;
+            return name != null ? valueOf(name) : null;
         }
     }
 
@@ -57,5 +56,4 @@ public static FieldMapper.Parameter<String> metricParam(
         return param;
     }
 
-
 }
diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java
index 8224881ddad06..6dfbceba20a4c 100644
--- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java
+++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java
@@ -40,6 +40,7 @@
 import org.elasticsearch.index.mapper.MapperParsingException;
 import org.elasticsearch.index.mapper.SourceValueFetcher;
 import org.elasticsearch.index.mapper.TextSearchInfo;
+import org.elasticsearch.index.mapper.TimeSeriesParams;
 import org.elasticsearch.index.mapper.ValueFetcher;
 import org.elasticsearch.index.query.SearchExecutionContext;
 import org.elasticsearch.search.DocValueFormat;
@@ -74,6 +75,17 @@ public static class Builder extends FieldMapper.Builder {
         private final Parameter<Map<String, String>> meta = Parameter.metaParam();
         private final Parameter<Explicit<Boolean>> ignoreMalformed;
 
+        /**
+         * Parameter that marks this field as a time series metric defining its time series metric type.
+         * For {@link HistogramFieldMapper} fields only the histogram metric type is supported.
+         */
+        private final Parameter<String> metric = TimeSeriesParams.metricParam(
+            m -> toType(m).metricType != null ? toType(m).metricType.name() : null,
+            null, // This field type always stores its subfields as doc_values
+            null,
+            TimeSeriesParams.MetricType.histogram.name()
+        );
+
         public Builder(String name, boolean ignoreMalformedByDefault) {
             super(name);
             this.ignoreMalformed
@@ -82,13 +94,22 @@ public Builder(String name, boolean ignoreMalformedByDefault) {
 
         @Override
         protected List<Parameter<?>> getParameters() {
-            return List.of(ignoreMalformed, meta);
+            return List.of(ignoreMalformed, meta, metric);
         }
 
         @Override
         public HistogramFieldMapper build(ContentPath contentPath) {
-            return new HistogramFieldMapper(name, new HistogramFieldType(buildFullName(contentPath), meta.getValue()),
-                multiFieldsBuilder.build(this, contentPath), copyTo.build(), this);
+            return new HistogramFieldMapper(
+                name,
+                new HistogramFieldType(
+                    buildFullName(contentPath),
+                    meta.getValue(),
+                    TimeSeriesParams.MetricType.fromString(metric.getValue())
+                ),
+                multiFieldsBuilder.build(this, contentPath),
+                copyTo.build(),
+                this
+            );
         }
     }
 
@@ -98,11 +119,15 @@ public HistogramFieldMapper build(ContentPath contentPath) {
     private final Explicit<Boolean> ignoreMalformed;
     private final boolean ignoreMalformedByDefault;
 
+    /** The metric type (gauge, counter, summary) if  field is a time series metric */
+    private final TimeSeriesParams.MetricType metricType;
+
     public HistogramFieldMapper(String simpleName, MappedFieldType mappedFieldType,
                                 MultiFields multiFields, CopyTo copyTo, Builder builder) {
         super(simpleName, mappedFieldType, multiFields, copyTo);
         this.ignoreMalformed = builder.ignoreMalformed.getValue();
         this.ignoreMalformedByDefault = builder.ignoreMalformed.getDefaultValue().value();
+        this.metricType = TimeSeriesParams.MetricType.fromString(builder.metric.getValue());
     }
 
     boolean ignoreMalformed() {
@@ -126,8 +151,11 @@ protected void parseCreateField(DocumentParserContext context) {
 
     public static class HistogramFieldType extends MappedFieldType {
 
-        public HistogramFieldType(String name, Map<String, String> meta) {
+        private final TimeSeriesParams.MetricType metricType;
+
+        public HistogramFieldType(String name, Map<String, String> meta, TimeSeriesParams.MetricType metricType) {
             super(name, false, false, true, TextSearchInfo.NONE, meta);
+            this.metricType = metricType;
         }
 
         @Override
@@ -223,6 +251,14 @@ public Query termQuery(Object value, SearchExecutionContext context) {
             throw new IllegalArgumentException("[" + CONTENT_TYPE + "] field do not support searching, " +
                 "use dedicated aggregations instead: [" + name() + "]");
         }
+
+        /**
+         * If field is a time series metric field, returns its metric type
+         * @return the metric type or null
+         */
+        public TimeSeriesParams.MetricType getMetricType() {
+            return metricType;
+        }
     }
 
     @Override
diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/bucket/histogram/HistoBackedHistogramAggregatorTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/bucket/histogram/HistoBackedHistogramAggregatorTests.java
index 03e70af6f7c87..b797d1dc45cdb 100644
--- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/bucket/histogram/HistoBackedHistogramAggregatorTests.java
+++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/bucket/histogram/HistoBackedHistogramAggregatorTests.java
@@ -247,7 +247,7 @@ protected AggregationBuilder createAggBuilderForTypeTest(MappedFieldType fieldTy
     }
 
     private MappedFieldType defaultFieldType(String fieldName) {
-        return new HistogramFieldMapper.HistogramFieldType(fieldName, Collections.emptyMap());
+        return new HistogramFieldMapper.HistogramFieldType(fieldName, Collections.emptyMap(), null);
     }
 
 }
diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/bucket/range/HistoBackedRangeAggregatorTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/bucket/range/HistoBackedRangeAggregatorTests.java
index 52d6bf72b9c83..825f58a27f6e8 100644
--- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/bucket/range/HistoBackedRangeAggregatorTests.java
+++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/bucket/range/HistoBackedRangeAggregatorTests.java
@@ -374,7 +374,7 @@ protected AggregationBuilder createAggBuilderForTypeTest(MappedFieldType fieldTy
 
     private MappedFieldType defaultFieldType(String fieldName) {
         if (fieldName.equals(HISTO_FIELD_NAME)) {
-            return new HistogramFieldMapper.HistogramFieldType(fieldName, Collections.emptyMap());
+            return new HistogramFieldMapper.HistogramFieldType(fieldName, Collections.emptyMap(), null);
         } else {
             return new NumberFieldMapper.NumberFieldType(fieldName, NumberFieldMapper.NumberType.DOUBLE);
         }
diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HDRPreAggregatedPercentileRanksAggregatorTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HDRPreAggregatedPercentileRanksAggregatorTests.java
index 96c41f6dd5429..2e21a2d58304b 100644
--- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HDRPreAggregatedPercentileRanksAggregatorTests.java
+++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HDRPreAggregatedPercentileRanksAggregatorTests.java
@@ -90,7 +90,7 @@ public void testSimple() throws IOException {
             PercentileRanksAggregationBuilder aggBuilder = new PercentileRanksAggregationBuilder("my_agg", new double[]{0.1, 0.5, 12})
                     .field("field")
                     .method(PercentilesMethod.HDR);
-            MappedFieldType fieldType = new HistogramFieldMapper.HistogramFieldType("field", Collections.emptyMap());
+            MappedFieldType fieldType = new HistogramFieldMapper.HistogramFieldType("field", Collections.emptyMap(), null);
             try (IndexReader reader = w.getReader()) {
                 IndexSearcher searcher = new IndexSearcher(reader);
                 PercentileRanks ranks = searchAndReduce(searcher, new MatchAllDocsQuery(), aggBuilder, fieldType);
diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HDRPreAggregatedPercentilesAggregatorTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HDRPreAggregatedPercentilesAggregatorTests.java
index 527578abca8fb..3d984befcfb32 100644
--- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HDRPreAggregatedPercentilesAggregatorTests.java
+++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HDRPreAggregatedPercentilesAggregatorTests.java
@@ -149,7 +149,7 @@ private void testCase(Query query, CheckedConsumer<RandomIndexWriter, IOExceptio
                 PercentilesAggregationBuilder builder =
                         new PercentilesAggregationBuilder("test").field("number").method(PercentilesMethod.HDR);
 
-                MappedFieldType fieldType = new HistogramFieldMapper.HistogramFieldType("number", Collections.emptyMap());
+                MappedFieldType fieldType = new HistogramFieldMapper.HistogramFieldType("number", Collections.emptyMap(), null);
                 Aggregator aggregator = createAggregator(builder, indexSearcher, fieldType);
                 aggregator.preCollection();
                 indexSearcher.search(query, aggregator);
diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedAvgAggregatorTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedAvgAggregatorTests.java
index 37e74c5326944..0f1ee9d4c5009 100644
--- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedAvgAggregatorTests.java
+++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedAvgAggregatorTests.java
@@ -127,6 +127,6 @@ protected AggregationBuilder createAggBuilderForTypeTest(MappedFieldType fieldTy
     }
 
     private MappedFieldType defaultFieldType() {
-        return new HistogramFieldMapper.HistogramFieldType(HistoBackedAvgAggregatorTests.FIELD_NAME, Collections.emptyMap());
+        return new HistogramFieldMapper.HistogramFieldType(HistoBackedAvgAggregatorTests.FIELD_NAME, Collections.emptyMap(), null);
     }
 }
diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedMaxAggregatorTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedMaxAggregatorTests.java
index a9e019480dc3f..f90bd70647833 100644
--- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedMaxAggregatorTests.java
+++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedMaxAggregatorTests.java
@@ -127,6 +127,6 @@ protected AggregationBuilder createAggBuilderForTypeTest(MappedFieldType fieldTy
     }
 
     private MappedFieldType defaultFieldType() {
-        return new HistogramFieldMapper.HistogramFieldType(HistoBackedMaxAggregatorTests.FIELD_NAME, Collections.emptyMap());
+        return new HistogramFieldMapper.HistogramFieldType(HistoBackedMaxAggregatorTests.FIELD_NAME, Collections.emptyMap(), null);
     }
 }
diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedMinAggregatorTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedMinAggregatorTests.java
index 1122761fcfc97..7bae37ffef03b 100644
--- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedMinAggregatorTests.java
+++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedMinAggregatorTests.java
@@ -127,6 +127,6 @@ protected AggregationBuilder createAggBuilderForTypeTest(MappedFieldType fieldTy
     }
 
     private MappedFieldType defaultFieldType() {
-        return new HistogramFieldMapper.HistogramFieldType(HistoBackedMinAggregatorTests.FIELD_NAME, Collections.emptyMap());
+        return new HistogramFieldMapper.HistogramFieldType(HistoBackedMinAggregatorTests.FIELD_NAME, Collections.emptyMap(), null);
     }
 }
diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedSumAggregatorTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedSumAggregatorTests.java
index 8d93d307981b7..4742bad2986be 100644
--- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedSumAggregatorTests.java
+++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedSumAggregatorTests.java
@@ -127,6 +127,6 @@ protected AggregationBuilder createAggBuilderForTypeTest(MappedFieldType fieldTy
     }
 
     private MappedFieldType defaultFieldType() {
-        return new HistogramFieldMapper.HistogramFieldType(HistoBackedSumAggregatorTests.FIELD_NAME, Collections.emptyMap());
+        return new HistogramFieldMapper.HistogramFieldType(HistoBackedSumAggregatorTests.FIELD_NAME, Collections.emptyMap(), null);
     }
 }
diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedValueCountAggregatorTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedValueCountAggregatorTests.java
index 332fac6396f20..62269c23b166b 100644
--- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedValueCountAggregatorTests.java
+++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/HistoBackedValueCountAggregatorTests.java
@@ -132,6 +132,6 @@ protected AggregationBuilder createAggBuilderForTypeTest(MappedFieldType fieldTy
     }
 
     private MappedFieldType defaultFieldType() {
-        return new HistogramFieldMapper.HistogramFieldType("field", Collections.emptyMap());
+        return new HistogramFieldMapper.HistogramFieldType("field", Collections.emptyMap(), null);
     }
 }
diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/TDigestPreAggregatedPercentileRanksAggregatorTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/TDigestPreAggregatedPercentileRanksAggregatorTests.java
index 8f90693eaec7e..e4c3835384cf2 100644
--- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/TDigestPreAggregatedPercentileRanksAggregatorTests.java
+++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/TDigestPreAggregatedPercentileRanksAggregatorTests.java
@@ -70,7 +70,7 @@ public void testSimple() throws IOException {
             PercentileRanksAggregationBuilder aggBuilder = new PercentileRanksAggregationBuilder("my_agg", new double[] {0.1, 0.5, 12})
                     .field("field")
                     .method(PercentilesMethod.TDIGEST);
-            MappedFieldType fieldType = new HistogramFieldMapper.HistogramFieldType("field", Collections.emptyMap());
+            MappedFieldType fieldType = new HistogramFieldMapper.HistogramFieldType("field", Collections.emptyMap(), null);
             try (IndexReader reader = w.getReader()) {
                 IndexSearcher searcher = new IndexSearcher(reader);
                 PercentileRanks ranks = searchAndReduce(searcher, new MatchAllDocsQuery(), aggBuilder, fieldType);
diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/TDigestPreAggregatedPercentilesAggregatorTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/TDigestPreAggregatedPercentilesAggregatorTests.java
index c3ea1ae515a34..f12e895b39f5a 100644
--- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/TDigestPreAggregatedPercentilesAggregatorTests.java
+++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/aggregations/metrics/TDigestPreAggregatedPercentilesAggregatorTests.java
@@ -123,7 +123,7 @@ private void testCase(Query query, CheckedConsumer<RandomIndexWriter, IOExceptio
                 PercentilesAggregationBuilder builder =
                         new PercentilesAggregationBuilder("test").field("number").method(PercentilesMethod.TDIGEST);
 
-                MappedFieldType fieldType = new HistogramFieldMapper.HistogramFieldType("number", Collections.emptyMap());
+                MappedFieldType fieldType = new HistogramFieldMapper.HistogramFieldType("number", Collections.emptyMap(), null);
                 Aggregator aggregator = createAggregator(builder, indexSearcher, fieldType);
                 aggregator.preCollection();
                 indexSearcher.search(query, aggregator);
diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapperTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapperTests.java
index b5809dfc099dc..1e17c1fb9ffe5 100644
--- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapperTests.java
+++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapperTests.java
@@ -10,6 +10,7 @@
 import org.elasticsearch.index.mapper.DocumentMapper;
 import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.index.mapper.MapperParsingException;
+import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.index.mapper.MapperTestCase;
 import org.elasticsearch.index.mapper.ParsedDocument;
 import org.elasticsearch.index.mapper.SourceToParse;
@@ -20,12 +21,13 @@
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
+import java.util.function.Function;
 
 import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.notNullValue;
 import static org.hamcrest.Matchers.nullValue;
 
-
 public class HistogramFieldMapperTests extends MapperTestCase {
 
     @Override
@@ -322,4 +324,48 @@ public void testCannotBeUsedInMultifields() {
         })));
         assertThat(e.getMessage(), containsString("Field [hist] of type [histogram] can't be used in multifields"));
     }
+
+    protected <T> void assertMetricType(String metricType, Function<T, Enum> checker)
+        throws IOException {
+        MapperService mapperService = createMapperService(fieldMapping(b -> {
+            minimalMapping(b);
+            b.field("metric", metricType);
+        }));
+
+        @SuppressWarnings("unchecked") // Syntactic sugar in tests
+        T fieldType = (T) mapperService.fieldType("field");
+        assertThat(checker.apply(fieldType).name(), equalTo(metricType));
+    }
+
+    public void testMetricType() throws IOException {
+        // Test default setting
+        MapperService mapperService = createMapperService(fieldMapping(b -> minimalMapping(b)));
+        HistogramFieldMapper.HistogramFieldType ft = (HistogramFieldMapper.HistogramFieldType) mapperService.fieldType("field");
+        assertNull(ft.getMetricType());
+        assertMetricType("histogram", HistogramFieldMapper.HistogramFieldType::getMetricType);
+
+        {
+            // Test invalid metric type for this field type
+            Exception e = expectThrows(MapperParsingException.class, () -> createMapperService(fieldMapping(b -> {
+                minimalMapping(b);
+                b.field("metric", "gauge");
+            })));
+            assertThat(
+                e.getCause().getMessage(),
+                containsString("Unknown value [gauge] for field [metric] - accepted values are [null, histogram]")
+            );
+        }
+        {
+            // Test invalid metric type for this field type
+            Exception e = expectThrows(MapperParsingException.class, () -> createMapperService(fieldMapping(b -> {
+                minimalMapping(b);
+                b.field("metric", "unknown");
+            })));
+            assertThat(
+                e.getCause().getMessage(),
+                containsString("Unknown value [unknown] for field [metric] - accepted values are [null, histogram]")
+            );
+        }
+    }
+
 }
diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/rate/RateAggregatorTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/rate/RateAggregatorTests.java
index deb35fb17e9bc..5aba69269b847 100644
--- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/rate/RateAggregatorTests.java
+++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/rate/RateAggregatorTests.java
@@ -595,7 +595,7 @@ public void testFormatter() throws IOException {
     }
 
     public void testHistogramFieldMonthToMonth() throws IOException {
-        MappedFieldType histType = new HistogramFieldMapper.HistogramFieldType("val", Collections.emptyMap());
+        MappedFieldType histType = new HistogramFieldMapper.HistogramFieldType("val", Collections.emptyMap(), null);
         MappedFieldType dateType = dateFieldType(DATE_FIELD);
         RateAggregationBuilder rateAggregationBuilder = new RateAggregationBuilder("my_rate").rateUnit("month").field("val");
         if (randomBoolean()) {
@@ -617,7 +617,7 @@ public void testHistogramFieldMonthToMonth() throws IOException {
     }
 
     public void testHistogramFieldMonthToYear() throws IOException {
-        MappedFieldType histType = new HistogramFieldMapper.HistogramFieldType("val", Collections.emptyMap());
+        MappedFieldType histType = new HistogramFieldMapper.HistogramFieldType("val", Collections.emptyMap(), null);
         MappedFieldType dateType = dateFieldType(DATE_FIELD);
         RateAggregationBuilder rateAggregationBuilder = new RateAggregationBuilder("my_rate").rateUnit("month").field("val");
         if (randomBoolean()) {
@@ -638,7 +638,7 @@ public void testHistogramFieldMonthToYear() throws IOException {
     }
 
     public void testHistogramFieldMonthToMonthValueCount() throws IOException {
-        MappedFieldType histType = new HistogramFieldMapper.HistogramFieldType("val", Collections.emptyMap());
+        MappedFieldType histType = new HistogramFieldMapper.HistogramFieldType("val", Collections.emptyMap(), null);
         MappedFieldType dateType = dateFieldType(DATE_FIELD);
         RateAggregationBuilder rateAggregationBuilder = new RateAggregationBuilder("my_rate").rateUnit("month")
             .rateMode("value_count")
@@ -659,7 +659,7 @@ public void testHistogramFieldMonthToMonthValueCount() throws IOException {
     }
 
     public void testHistogramFieldMonthToYearValueCount() throws IOException {
-        MappedFieldType histType = new HistogramFieldMapper.HistogramFieldType("val", Collections.emptyMap());
+        MappedFieldType histType = new HistogramFieldMapper.HistogramFieldType("val", Collections.emptyMap(), null);
         MappedFieldType dateType = dateFieldType(DATE_FIELD);
         RateAggregationBuilder rateAggregationBuilder = new RateAggregationBuilder("my_rate").rateUnit("month")
             .rateMode("value_count")
@@ -679,7 +679,7 @@ public void testHistogramFieldMonthToYearValueCount() throws IOException {
     }
 
     public void testFilterWithHistogramField() throws IOException {
-        MappedFieldType histType = new HistogramFieldMapper.HistogramFieldType("val", Collections.emptyMap());
+        MappedFieldType histType = new HistogramFieldMapper.HistogramFieldType("val", Collections.emptyMap(), null);
         MappedFieldType dateType = dateFieldType(DATE_FIELD);
         MappedFieldType keywordType = new KeywordFieldMapper.KeywordFieldType("term");
         RateAggregationBuilder rateAggregationBuilder = new RateAggregationBuilder("my_rate").rateUnit("month").field("val");
diff --git a/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapper.java b/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapper.java
index 34c7956e85360..b1d15beef3615 100644
--- a/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapper.java
+++ b/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapper.java
@@ -240,7 +240,7 @@ public AggregateDoubleMetricFieldMapper build(ContentPath context) {
             AggregateDoubleMetricFieldType metricFieldType = new AggregateDoubleMetricFieldType(
                 buildFullName(context),
                 meta.getValue(),
-                metric.getValue() != null ? MetricType.valueOf(metric.getValue()) : null
+                MetricType.fromString(metric.getValue())
             );
             metricFieldType.setMetricFields(metricFields);
             metricFieldType.setDefaultMetric(defaultMetric.getValue());
@@ -509,6 +509,7 @@ public MetricType getMetricType() {
     /** The default metric to be when querying this field type */
     protected Metric defaultMetric;
 
+    /** The metric type (gauge, counter, summary) if  field is a time series metric */
     private final TimeSeriesParams.MetricType metricType;
 
     private AggregateDoubleMetricFieldMapper(

From b98ecaeb0044043852b58b7bad58de2dd9601048 Mon Sep 17 00:00:00 2001
From: Christos Soulios <csoulios@gmail.com>
Date: Fri, 20 Aug 2021 16:44:00 +0300
Subject: [PATCH 05/11] Renamed metric param to time_series_metric

---
 .../index/mapper/NumberFieldMapper.java       | 28 ++++++++++++++-----
 .../index/mapper/TimeSeriesParams.java        | 25 +++--------------
 .../index/mapper/NumberFieldMapperTests.java  | 14 +++++-----
 .../mapper/HistogramFieldMapper.java          | 13 +++++----
 .../mapper/HistogramFieldMapperTests.java     | 10 +++----
 .../AggregateDoubleMetricFieldMapper.java     | 25 +++++++++--------
 ...AggregateDoubleMetricFieldMapperTests.java | 14 ++++++----
 7 files changed, 66 insertions(+), 63 deletions(-)

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java
index f78bf5869175a..54b763ce03c1a 100644
--- a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java
+++ b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java
@@ -58,6 +58,7 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.function.BiFunction;
+import java.util.function.Consumer;
 import java.util.function.Function;
 import java.util.function.Supplier;
 
@@ -95,13 +96,7 @@ public static class Builder extends FieldMapper.Builder {
          * For the numeric fields gauge and counter metric types are
          * supported
          */
-        private final Parameter<String> metric = TimeSeriesParams.metricParam(
-            m -> toType(m).metricType != null ? toType(m).metricType.name() : null,
-            hasDocValues,
-            null,
-            MetricType.gauge.name(),
-            MetricType.counter.name()
-        );
+        private final Parameter<String> metric;
 
         private final Parameter<Map<String, String>> meta = Parameter.metaParam();
 
@@ -143,6 +138,25 @@ public Builder(String name, NumberType type, ScriptCompiler compiler, boolean ig
                     }
                 });
 
+            this.metric = TimeSeriesParams.metricParam(
+                m -> toType(m).metricType != null ? toType(m).metricType.name() : null,
+                null,
+                MetricType.gauge.name(),
+                MetricType.counter.name()
+            );
+            // We are overriding the default validator that checks for acceptable values.
+            // Therefore, we must call the default validator explicitly from inside our new validator.
+            // TODO: Make parameters support multiple validators
+            final Consumer<String> defaultValidator = metric.getValidator();
+            metric.setValidator(v -> {
+                // Call the default validator first
+                defaultValidator.accept(v);
+
+                if (v != null && v.isEmpty() == false && hasDocValues.getValue() == false) {
+                    throw new IllegalArgumentException("Field [" + metric.name + "] requires that [" + hasDocValues.name + "] is true");
+                }
+            });
+
             this.script.precludesParameters(ignoreMalformed, coerce, nullValue);
             addScriptValidation(script, indexed, hasDocValues);
         }
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesParams.java b/server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesParams.java
index 517bb2e64f7c7..b4136f9996897 100644
--- a/server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesParams.java
+++ b/server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesParams.java
@@ -16,6 +16,8 @@
  */
 public final class TimeSeriesParams {
 
+    public static final String TIME_SERIES_METRIC_PARAM = "time_series_metric";
+
     private TimeSeriesParams() {
     }
 
@@ -33,27 +35,8 @@ public static MetricType fromString(String name) {
         }
     }
 
-    public static FieldMapper.Parameter<String> metricParam(
-        Function<FieldMapper, String> initializer,
-        FieldMapper.Parameter<Boolean> hasDocValues,
-        String... values
-    ) {
-        FieldMapper.Parameter<String> param = FieldMapper.Parameter.restrictedStringParam("metric", false, initializer, values)
-            .acceptsNull();
-        // We are overriding the default validator that checks for acceptable values.
-        // So we must call the default validator explicitly from inside our new validator.
-        // TODO: Make parameters support multiple validators
-        final Consumer<String> defaultValidator = param.getValidator();
-        param.setValidator(v -> {
-            // Call the default validator first
-            defaultValidator.accept(v);
-
-            if (v != null && v.isEmpty() == false && hasDocValues != null && hasDocValues.getValue() == false) {
-                throw new IllegalArgumentException("Field [metric] requires that [" + hasDocValues.name + "] is true");
-            }
-        });
-
-        return param;
+    public static FieldMapper.Parameter<String> metricParam(Function<FieldMapper, String> initializer, String... values) {
+        return FieldMapper.Parameter.restrictedStringParam(TIME_SERIES_METRIC_PARAM, false, initializer, values).acceptsNull();
     }
 
 }
diff --git a/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java
index 62639d73c7a49..eeb7d2b14afd1 100644
--- a/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java
+++ b/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java
@@ -265,7 +265,7 @@ protected <T> void assertMetricType(String metricType, Function<T, Enum> checker
         throws IOException {
         MapperService mapperService = createMapperService(fieldMapping(b -> {
             minimalMapping(b);
-            b.field("metric", metricType);
+            b.field("time_series_metric", metricType);
         }));
 
         @SuppressWarnings("unchecked") // Syntactic sugar in tests
@@ -286,22 +286,22 @@ public void testMetricType() throws IOException {
             // Test invalid metric type for this field type
             Exception e = expectThrows(MapperParsingException.class, () -> createMapperService(fieldMapping(b -> {
                 minimalMapping(b);
-                b.field("metric", "histogram");
+                b.field("time_series_metric", "histogram");
             })));
             assertThat(
                 e.getCause().getMessage(),
-                containsString("Unknown value [histogram] for field [metric] - accepted values are [null, gauge, counter]")
+                containsString("Unknown value [histogram] for field [time_series_metric] - accepted values are [null, gauge, counter]")
             );
         }
         {
             // Test invalid metric type for this field type
             Exception e = expectThrows(MapperParsingException.class, () -> createMapperService(fieldMapping(b -> {
                 minimalMapping(b);
-                b.field("metric", "unknown");
+                b.field("time_series_metric", "unknown");
             })));
             assertThat(
                 e.getCause().getMessage(),
-                containsString("Unknown value [unknown] for field [metric] - accepted values are [null, gauge, counter]")
+                containsString("Unknown value [unknown] for field [time_series_metric] - accepted values are [null, gauge, counter]")
             );
         }
     }
@@ -309,9 +309,9 @@ public void testMetricType() throws IOException {
     public void testMetricAndDocvalues() {
         Exception e = expectThrows(MapperParsingException.class, () -> createDocumentMapper(fieldMapping(b -> {
             minimalMapping(b);
-            b.field("metric", "counter").field("doc_values", false);
+            b.field("time_series_metric", "counter").field("doc_values", false);
         })));
-        assertThat(e.getCause().getMessage(), containsString("Field [metric] requires that [doc_values] is true"));
+        assertThat(e.getCause().getMessage(), containsString("Field [time_series_metric] requires that [doc_values] is true"));
     }
 
     @Override
diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java
index 6dfbceba20a4c..0113cbdbc5242 100644
--- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java
+++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java
@@ -79,17 +79,18 @@ public static class Builder extends FieldMapper.Builder {
          * Parameter that marks this field as a time series metric defining its time series metric type.
          * For {@link HistogramFieldMapper} fields only the histogram metric type is supported.
          */
-        private final Parameter<String> metric = TimeSeriesParams.metricParam(
-            m -> toType(m).metricType != null ? toType(m).metricType.name() : null,
-            null, // This field type always stores its subfields as doc_values
-            null,
-            TimeSeriesParams.MetricType.histogram.name()
-        );
+        private final Parameter<String> metric;
 
         public Builder(String name, boolean ignoreMalformedByDefault) {
             super(name);
             this.ignoreMalformed
                 = Parameter.explicitBoolParam("ignore_malformed", true, m -> toType(m).ignoreMalformed, ignoreMalformedByDefault);
+
+            this.metric = TimeSeriesParams.metricParam(
+                m -> toType(m).metricType != null ? toType(m).metricType.name() : null,
+                null,
+                TimeSeriesParams.MetricType.histogram.name()
+            );
         }
 
         @Override
diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapperTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapperTests.java
index 1e17c1fb9ffe5..d21b82d912182 100644
--- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapperTests.java
+++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapperTests.java
@@ -329,7 +329,7 @@ protected <T> void assertMetricType(String metricType, Function<T, Enum> checker
         throws IOException {
         MapperService mapperService = createMapperService(fieldMapping(b -> {
             minimalMapping(b);
-            b.field("metric", metricType);
+            b.field("time_series_metric", metricType);
         }));
 
         @SuppressWarnings("unchecked") // Syntactic sugar in tests
@@ -348,22 +348,22 @@ public void testMetricType() throws IOException {
             // Test invalid metric type for this field type
             Exception e = expectThrows(MapperParsingException.class, () -> createMapperService(fieldMapping(b -> {
                 minimalMapping(b);
-                b.field("metric", "gauge");
+                b.field("time_series_metric", "gauge");
             })));
             assertThat(
                 e.getCause().getMessage(),
-                containsString("Unknown value [gauge] for field [metric] - accepted values are [null, histogram]")
+                containsString("Unknown value [gauge] for field [time_series_metric] - accepted values are [null, histogram]")
             );
         }
         {
             // Test invalid metric type for this field type
             Exception e = expectThrows(MapperParsingException.class, () -> createMapperService(fieldMapping(b -> {
                 minimalMapping(b);
-                b.field("metric", "unknown");
+                b.field("time_series_metric", "unknown");
             })));
             assertThat(
                 e.getCause().getMessage(),
-                containsString("Unknown value [unknown] for field [metric] - accepted values are [null, histogram]")
+                containsString("Unknown value [unknown] for field [time_series_metric] - accepted values are [null, histogram]")
             );
         }
     }
diff --git a/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapper.java b/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapper.java
index b1d15beef3615..6455635493eb6 100644
--- a/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapper.java
+++ b/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapper.java
@@ -137,14 +137,7 @@ public static class Builder extends FieldMapper.Builder {
          * For {@link AggregateDoubleMetricFieldMapper} fields gauge, counter and summary metric types are
          * supported.
          */
-        private final Parameter<String> metric = TimeSeriesParams.metricParam(
-            m -> toType(m).metricType != null ? toType(m).metricType.name() : null,
-            null, // This field type always stores its subfields as doc_values
-            null,
-            MetricType.gauge.name(),
-            MetricType.counter.name(),
-            MetricType.summary.name()
-        );
+        private final Parameter<String> timeSeriesMetric;
 
         /**
          * Set the default metric so that query operations are delegated to it.
@@ -165,15 +158,23 @@ public Builder(String name, Boolean ignoreMalformedByDefault) {
                 m -> toType(m).ignoreMalformed,
                 ignoreMalformedByDefault
             );
+
+            this.timeSeriesMetric = TimeSeriesParams.metricParam(
+                m -> toType(m).metricType != null ? toType(m).metricType.name() : null,
+                null,
+                MetricType.gauge.name(),
+                MetricType.counter.name(),
+                MetricType.summary.name()
+            );
         }
 
         @Override
         protected List<Parameter<?>> getParameters() {
-            return List.of(ignoreMalformed, metrics, defaultMetric, meta, metric);
+            return List.of(ignoreMalformed, metrics, defaultMetric, meta, timeSeriesMetric);
         }
 
         public Builder metric(String metric) {
-            this.metric.setValue(metric);
+            this.timeSeriesMetric.setValue(metric);
             return this;
         }
 
@@ -240,7 +241,7 @@ public AggregateDoubleMetricFieldMapper build(ContentPath context) {
             AggregateDoubleMetricFieldType metricFieldType = new AggregateDoubleMetricFieldType(
                 buildFullName(context),
                 meta.getValue(),
-                MetricType.fromString(metric.getValue())
+                MetricType.fromString(timeSeriesMetric.getValue())
             );
             metricFieldType.setMetricFields(metricFields);
             metricFieldType.setDefaultMetric(defaultMetric.getValue());
@@ -524,7 +525,7 @@ private AggregateDoubleMetricFieldMapper(
         this.metrics = builder.metrics.getValue();
         this.defaultMetric = builder.defaultMetric.getValue();
         this.metricFieldMappers = metricFieldMappers;
-        this.metricType = TimeSeriesParams.MetricType.fromString(builder.metric.getValue());
+        this.metricType = TimeSeriesParams.MetricType.fromString(builder.timeSeriesMetric.getValue());
     }
 
     boolean ignoreMalformed() {
diff --git a/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapperTests.java b/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapperTests.java
index de4ab0db73239..32ed29f71715b 100644
--- a/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapperTests.java
+++ b/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapperTests.java
@@ -557,7 +557,7 @@ protected <T> void assertMetricType(String metricType, Function<T, Enum> checker
         throws IOException {
         MapperService mapperService = createMapperService(fieldMapping(b -> {
             minimalMapping(b);
-            b.field("metric", metricType);
+            b.field("time_series_metric", metricType);
         }));
 
         @SuppressWarnings("unchecked") // Syntactic sugar in tests
@@ -580,22 +580,26 @@ public void testMetricType() throws IOException {
             // Test invalid metric type for this field type
             Exception e = expectThrows(MapperParsingException.class, () -> createMapperService(fieldMapping(b -> {
                 minimalMapping(b);
-                b.field("metric", "histogram");
+                b.field("time_series_metric", "histogram");
             })));
             assertThat(
                 e.getCause().getMessage(),
-                containsString("Unknown value [histogram] for field [metric] - accepted values are [null, gauge, counter, summary]")
+                containsString(
+                    "Unknown value [histogram] for field [time_series_metric] - accepted values are [null, gauge, counter, summary]"
+                )
             );
         }
         {
             // Test invalid metric type for this field type
             Exception e = expectThrows(MapperParsingException.class, () -> createMapperService(fieldMapping(b -> {
                 minimalMapping(b);
-                b.field("metric", "unknown");
+                b.field("time_series_metric", "unknown");
             })));
             assertThat(
                 e.getCause().getMessage(),
-                containsString("Unknown value [unknown] for field [metric] - accepted values are [null, gauge, counter, summary]")
+                containsString(
+                    "Unknown value [unknown] for field [time_series_metric] - accepted values are [null, gauge, counter, summary]"
+                )
             );
         }
     }

From efe0b67f5437efe3f3b11ca90e86ca6b0014cf58 Mon Sep 17 00:00:00 2001
From: Christos Soulios <csoulios@gmail.com>
Date: Fri, 20 Aug 2021 17:28:56 +0300
Subject: [PATCH 06/11] Checkstyle

---
 .../elasticsearch/benchmark/script/ScriptScoreBenchmark.java  | 2 +-
 .../java/org/elasticsearch/index/mapper/TimeSeriesParams.java | 1 -
 .../elasticsearch/index/mapper/NumberFieldMapperTests.java    | 2 +-
 .../xpack/analytics/mapper/HistogramFieldMapperTests.java     | 3 ++-
 .../mapper/AggregateDoubleMetricFieldMapper.java              | 4 +---
 .../mapper/AggregateDoubleMetricFieldMapperTests.java         | 4 ++--
 6 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/benchmarks/src/main/java/org/elasticsearch/benchmark/script/ScriptScoreBenchmark.java b/benchmarks/src/main/java/org/elasticsearch/benchmark/script/ScriptScoreBenchmark.java
index 19829074d96f1..05ac262c533e5 100644
--- a/benchmarks/src/main/java/org/elasticsearch/benchmark/script/ScriptScoreBenchmark.java
+++ b/benchmarks/src/main/java/org/elasticsearch/benchmark/script/ScriptScoreBenchmark.java
@@ -80,7 +80,7 @@ public class ScriptScoreBenchmark {
     private final ScriptModule scriptModule = new ScriptModule(Settings.EMPTY, pluginsService.filterPlugins(ScriptPlugin.class));
 
     private final Map<String, MappedFieldType> fieldTypes = Map.ofEntries(
-        Map.entry("n", new NumberFieldType("n", NumberType.LONG, false, false, true, true, null, Map.of(), null, false))
+        Map.entry("n", new NumberFieldType("n", NumberType.LONG, false, false, true, true, null, Map.of(), null, false, null))
     );
     private final IndexFieldDataCache fieldDataCache = new IndexFieldDataCache.None();
     private final CircuitBreakerService breakerService = new NoneCircuitBreakerService();
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesParams.java b/server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesParams.java
index b4136f9996897..0dc200e5ec0aa 100644
--- a/server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesParams.java
+++ b/server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesParams.java
@@ -8,7 +8,6 @@
 
 package org.elasticsearch.index.mapper;
 
-import java.util.function.Consumer;
 import java.util.function.Function;
 
 /**
diff --git a/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java
index eeb7d2b14afd1..b3a019b75e29e 100644
--- a/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java
+++ b/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java
@@ -261,7 +261,7 @@ public void testDimension() throws IOException {
         assertThat(e.getCause().getMessage(), containsString("Parameter [dimension] cannot be set"));
     }
 
-    protected <T> void assertMetricType(String metricType, Function<T, Enum> checker)
+    protected <T> void assertMetricType(String metricType, Function<T, Enum<TimeSeriesParams.MetricType>> checker)
         throws IOException {
         MapperService mapperService = createMapperService(fieldMapping(b -> {
             minimalMapping(b);
diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapperTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapperTests.java
index d21b82d912182..61c18c08a9e1b 100644
--- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapperTests.java
+++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapperTests.java
@@ -14,6 +14,7 @@
 import org.elasticsearch.index.mapper.MapperTestCase;
 import org.elasticsearch.index.mapper.ParsedDocument;
 import org.elasticsearch.index.mapper.SourceToParse;
+import org.elasticsearch.index.mapper.TimeSeriesParams;
 import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.xpack.analytics.AnalyticsPlugin;
 
@@ -325,7 +326,7 @@ public void testCannotBeUsedInMultifields() {
         assertThat(e.getMessage(), containsString("Field [hist] of type [histogram] can't be used in multifields"));
     }
 
-    protected <T> void assertMetricType(String metricType, Function<T, Enum> checker)
+    protected <T> void assertMetricType(String metricType, Function<T, Enum<TimeSeriesParams.MetricType>> checker)
         throws IOException {
         MapperService mapperService = createMapperService(fieldMapping(b -> {
             minimalMapping(b);
diff --git a/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapper.java b/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapper.java
index 6455635493eb6..24d5bdc7e9b13 100644
--- a/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapper.java
+++ b/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapper.java
@@ -652,8 +652,6 @@ protected void parseCreateField(DocumentParserContext context) throws IOExceptio
 
     @Override
     public FieldMapper.Builder getMergeBuilder() {
-        return new Builder(simpleName(), ignoreMalformedByDefault)
-            .metric(metricType != null ? metricType.name() : null)
-            .init(this);
+        return new Builder(simpleName(), ignoreMalformedByDefault).metric(metricType != null ? metricType.name() : null).init(this);
     }
 }
diff --git a/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapperTests.java b/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapperTests.java
index 32ed29f71715b..9eab1d09b7d02 100644
--- a/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapperTests.java
+++ b/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapperTests.java
@@ -18,6 +18,7 @@
 import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.index.mapper.MapperTestCase;
 import org.elasticsearch.index.mapper.ParsedDocument;
+import org.elasticsearch.index.mapper.TimeSeriesParams;
 import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.xpack.aggregatemetric.AggregateMetricMapperPlugin;
 import org.hamcrest.Matchers;
@@ -553,8 +554,7 @@ public void testCannotBeUsedInMultifields() {
         assertThat(e.getMessage(), containsString("Field [metric] of type [aggregate_metric_double] can't be used in multifields"));
     }
 
-    protected <T> void assertMetricType(String metricType, Function<T, Enum> checker)
-        throws IOException {
+    protected <T> void assertMetricType(String metricType, Function<T, Enum<TimeSeriesParams.MetricType>> checker) throws IOException {
         MapperService mapperService = createMapperService(fieldMapping(b -> {
             minimalMapping(b);
             b.field("time_series_metric", metricType);

From 2dbe9ba91eeb6bc19287449ec6d0b774b9060444 Mon Sep 17 00:00:00 2001
From: Christos Soulios <csoulios@gmail.com>
Date: Tue, 31 Aug 2021 23:49:19 +0300
Subject: [PATCH 07/11] Add new validator to t_s_metric for numeric fields

Now that multiple validators are supported (#77073) we can add
a new validator instead of replacing the default param validator.
---
 .../elasticsearch/index/mapper/FieldMapper.java   |  4 ----
 .../index/mapper/NumberFieldMapper.java           | 15 ++++-----------
 .../mapper/HistogramFieldMapperTests.java         |  3 +--
 3 files changed, 5 insertions(+), 17 deletions(-)

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java
index 4ddcda8f3489d..86dc9ec80e099 100644
--- a/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java
+++ b/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java
@@ -691,10 +691,6 @@ public Parameter<T> addValidator(Consumer<T> validator) {
             return this;
         }
 
-        public Consumer<T> getValidator() {
-            return this.validator;
-        }
-
         /**
          * Configure a custom serializer for this parameter
          */
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java
index 4d5e74de828b0..cdd6e79f3f678 100644
--- a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java
+++ b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java
@@ -58,7 +58,6 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.function.BiFunction;
-import java.util.function.Consumer;
 import java.util.function.Function;
 import java.util.function.Supplier;
 
@@ -143,17 +142,11 @@ public Builder(String name, NumberType type, ScriptCompiler compiler, boolean ig
                 null,
                 MetricType.gauge.name(),
                 MetricType.counter.name()
-            );
-            // We are overriding the default validator that checks for acceptable values.
-            // Therefore, we must call the default validator explicitly from inside our new validator.
-            // TODO: Make parameters support multiple validators
-            final Consumer<String> defaultValidator = metric.getValidator();
-            metric.setValidator(v -> {
-                // Call the default validator first
-                defaultValidator.accept(v);
-
+            ).addValidator(v -> {
                 if (v != null && v.isEmpty() == false && hasDocValues.getValue() == false) {
-                    throw new IllegalArgumentException("Field [" + metric.name + "] requires that [" + hasDocValues.name + "] is true");
+                    throw new IllegalArgumentException(
+                        "Field [" + TimeSeriesParams.TIME_SERIES_METRIC_PARAM + "] requires that [" + hasDocValues.name + "] is true"
+                    );
                 }
             });
 
diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapperTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapperTests.java
index 9e32364ae3f70..714be6729da93 100644
--- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapperTests.java
+++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapperTests.java
@@ -317,8 +317,7 @@ public void testCannotBeUsedInMultifields() {
         assertThat(e.getMessage(), containsString("Field [hist] of type [histogram] can't be used in multifields"));
     }
 
-    protected <T> void assertMetricType(String metricType, Function<T, Enum<TimeSeriesParams.MetricType>> checker)
-        throws IOException {
+    protected <T> void assertMetricType(String metricType, Function<T, Enum<TimeSeriesParams.MetricType>> checker) throws IOException {
         MapperService mapperService = createMapperService(fieldMapping(b -> {
             minimalMapping(b);
             b.field("time_series_metric", metricType);

From 87594430397acd3c957c217c0acbe3fab6bb477a Mon Sep 17 00:00:00 2001
From: Christos Soulios <csoulios@gmail.com>
Date: Fri, 3 Sep 2021 17:45:02 +0300
Subject: [PATCH 08/11] Changed metric params from String to enum

---
 .../index/mapper/FieldMapper.java             | 63 +++++++++++++++++++
 .../index/mapper/NumberFieldMapper.java       | 30 ++++-----
 .../index/mapper/TimeSeriesParams.java        | 28 +++++----
 .../index/mapper/NumberFieldMapperTests.java  |  4 +-
 .../mapper/HistogramFieldMapper.java          | 16 ++---
 .../mapper/HistogramFieldMapperTests.java     |  4 +-
 .../AggregateDoubleMetricFieldMapper.java     | 19 +++---
 ...AggregateDoubleMetricFieldMapperTests.java |  8 +--
 8 files changed, 111 insertions(+), 61 deletions(-)

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java
index 86dc9ec80e099..5e392369c1d33 100644
--- a/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java
+++ b/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java
@@ -32,6 +32,7 @@
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.Comparator;
+import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.LinkedHashSet;
@@ -906,6 +907,68 @@ public static Parameter<String> restrictedStringParam(String name, boolean updat
                 });
         }
 
+        /**
+         * Defines a parameter that takes any of the values of 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
+         */
+        public static <T extends Enum<T>> Parameter<T> enumParam(
+            String name,
+            boolean updateable,
+            Function<FieldMapper, T> initializer,
+            T defaultValue,
+            Class<T> enumClass
+        ) {
+            Set<T> acceptedValues = EnumSet.noneOf(enumClass);
+            acceptedValues.addAll(Arrays.asList(enumClass.getEnumConstants()));
+            return restrictedEnumParam(name, updateable, initializer, defaultValue, enumClass, acceptedValues);
+        }
+
+        /**
+         * 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
+         */
+        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
+        ) {
+            assert values.size() > 0;
+            return new Parameter<T>(name, updateable, () -> defaultValue, (n, c, o) -> {
+                if (o == null) {
+                    return defaultValue;
+                }
+                try {
+                    @SuppressWarnings("unchecked")
+                    T enumValue = Enum.valueOf(enumClass, (String) o);
+                    return enumValue;
+                } catch (Exception e) {
+                    throw new MapperParsingException(
+                        "Unknown value [" + o + "] for field [" + name + "] - accepted values are " + values
+                    );
+                }
+            }, initializer).addValidator(v -> {
+                if (v != null && values.contains(v) == false) {
+                    throw new MapperParsingException(
+                        "Unknown value [" + v + "] for field [" + name + "] - accepted values are " + values
+                    );
+                }
+            });
+        }
+
         /**
          * Defines a parameter that takes an analyzer name
          * @param name              the parameter name
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java
index cdd6e79f3f678..a8051c6e4b488 100644
--- a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java
+++ b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java
@@ -95,7 +95,7 @@ public static class Builder extends FieldMapper.Builder {
          * For the numeric fields gauge and counter metric types are
          * supported
          */
-        private final Parameter<String> metric;
+        private final Parameter<MetricType> metric;
 
         private final Parameter<Map<String, String>> meta = Parameter.metaParam();
 
@@ -137,18 +137,14 @@ public Builder(String name, NumberType type, ScriptCompiler compiler, boolean ig
                     }
                 });
 
-            this.metric = TimeSeriesParams.metricParam(
-                m -> toType(m).metricType != null ? toType(m).metricType.name() : null,
-                null,
-                MetricType.gauge.name(),
-                MetricType.counter.name()
-            ).addValidator(v -> {
-                if (v != null && v.isEmpty() == false && hasDocValues.getValue() == false) {
-                    throw new IllegalArgumentException(
-                        "Field [" + TimeSeriesParams.TIME_SERIES_METRIC_PARAM + "] requires that [" + hasDocValues.name + "] is true"
-                    );
-                }
-            });
+            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"
+                        );
+                    }
+                });
 
             this.script.precludesParameters(ignoreMalformed, coerce, nullValue);
             addScriptValidation(script, indexed, hasDocValues);
@@ -176,7 +172,7 @@ public Builder dimension(boolean dimension) {
             return this;
         }
 
-        public Builder metric(String metric) {
+        public Builder metric(MetricType metric) {
             this.metric.setValue(metric);
             return this;
         }
@@ -1026,7 +1022,7 @@ public NumberFieldType(String name, NumberType type, boolean isSearchable, boole
             this(name, builder.type, builder.indexed.getValue(), builder.stored.getValue(), builder.hasDocValues.getValue(),
                 builder.coerce.getValue().value(), builder.nullValue.getValue(), builder.meta.getValue(),
                 builder.scriptValues(), builder.dimension.getValue(),
-                MetricType.fromString(builder.metric.getValue()));
+                builder.metric.getValue());
         }
 
         public NumberFieldType(String name, NumberType type) {
@@ -1173,7 +1169,7 @@ private NumberFieldMapper(
         this.coerceByDefault = builder.coerce.getDefaultValue().value();
         this.scriptValues = builder.scriptValues();
         this.dimension = builder.dimension.getValue();
-        this.metricType = MetricType.fromString(builder.metric.getValue());
+        this.metricType = builder.metric.getValue();
         this.builder = builder;
     }
 
@@ -1263,7 +1259,7 @@ protected void indexScriptValues(SearchLookup searchLookup, LeafReaderContext re
     public FieldMapper.Builder getMergeBuilder() {
         return new Builder(simpleName(), type, builder.scriptCompiler, ignoreMalformedByDefault, coerceByDefault)
             .dimension(dimension)
-            .metric(metricType != null ? metricType.name() : null)
+            .metric(metricType)
             .init(this);
     }
 }
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesParams.java b/server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesParams.java
index 0dc200e5ec0aa..026c2c933e8cb 100644
--- a/server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesParams.java
+++ b/server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesParams.java
@@ -8,6 +8,8 @@
 
 package org.elasticsearch.index.mapper;
 
+import java.util.Arrays;
+import java.util.EnumSet;
 import java.util.function.Function;
 
 /**
@@ -17,25 +19,27 @@ public final class TimeSeriesParams {
 
     public static final String TIME_SERIES_METRIC_PARAM = "time_series_metric";
 
-    private TimeSeriesParams() {
-    }
+    private TimeSeriesParams() {}
 
     public enum MetricType {
         gauge,
         counter,
         histogram,
-        summary;
-
-        /**
-         * Convert string to MetricType value returning null for null values
-         */
-        public static MetricType fromString(String name) {
-            return name != null ? valueOf(name) : null;
-        }
+        summary
     }
 
-    public static FieldMapper.Parameter<String> metricParam(Function<FieldMapper, String> initializer, String... values) {
-        return FieldMapper.Parameter.restrictedStringParam(TIME_SERIES_METRIC_PARAM, false, initializer, values).acceptsNull();
+    public static FieldMapper.Parameter<MetricType> metricParam(Function<FieldMapper, MetricType> initializer, MetricType... values) {
+        assert values.length > 0;
+        EnumSet<MetricType> acceptedValues = EnumSet.noneOf(MetricType.class);
+        acceptedValues.addAll(Arrays.asList(values));
+        return FieldMapper.Parameter.restrictedEnumParam(
+            TIME_SERIES_METRIC_PARAM,
+            false,
+            initializer,
+            null,
+            MetricType.class,
+            acceptedValues
+        ).acceptsNull();
     }
 
 }
diff --git a/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java
index b3a019b75e29e..5470063d57f16 100644
--- a/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java
+++ b/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java
@@ -290,7 +290,7 @@ public void testMetricType() throws IOException {
             })));
             assertThat(
                 e.getCause().getMessage(),
-                containsString("Unknown value [histogram] for field [time_series_metric] - accepted values are [null, gauge, counter]")
+                containsString("Unknown value [histogram] for field [time_series_metric] - accepted values are [gauge, counter]")
             );
         }
         {
@@ -301,7 +301,7 @@ public void testMetricType() throws IOException {
             })));
             assertThat(
                 e.getCause().getMessage(),
-                containsString("Unknown value [unknown] for field [time_series_metric] - accepted values are [null, gauge, counter]")
+                containsString("Unknown value [unknown] for field [time_series_metric] - accepted values are [gauge, counter]")
             );
         }
     }
diff --git a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java
index 9b2e1433fc45c..77de306cb37bb 100644
--- a/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java
+++ b/x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapper.java
@@ -78,7 +78,7 @@ public static class Builder extends FieldMapper.Builder {
          * Parameter that marks this field as a time series metric defining its time series metric type.
          * For {@link HistogramFieldMapper} fields only the histogram metric type is supported.
          */
-        private final Parameter<String> metric;
+        private final Parameter<TimeSeriesParams.MetricType> metric;
 
         public Builder(String name, boolean ignoreMalformedByDefault) {
             super(name);
@@ -89,11 +89,7 @@ public Builder(String name, boolean ignoreMalformedByDefault) {
                 ignoreMalformedByDefault
             );
 
-            this.metric = TimeSeriesParams.metricParam(
-                m -> toType(m).metricType != null ? toType(m).metricType.name() : null,
-                null,
-                TimeSeriesParams.MetricType.histogram.name()
-            );
+            this.metric = TimeSeriesParams.metricParam(m -> toType(m).metricType, TimeSeriesParams.MetricType.histogram);
         }
 
         @Override
@@ -105,11 +101,7 @@ protected List<Parameter<?>> getParameters() {
         public HistogramFieldMapper build(ContentPath contentPath) {
             return new HistogramFieldMapper(
                 name,
-                new HistogramFieldType(
-                    buildFullName(contentPath),
-                    meta.getValue(),
-                    TimeSeriesParams.MetricType.fromString(metric.getValue())
-                ),
+                new HistogramFieldType(buildFullName(contentPath), meta.getValue(), metric.getValue()),
                 multiFieldsBuilder.build(this, contentPath),
                 copyTo.build(),
                 this
@@ -138,7 +130,7 @@ public HistogramFieldMapper(
         super(simpleName, mappedFieldType, multiFields, copyTo);
         this.ignoreMalformed = builder.ignoreMalformed.getValue();
         this.ignoreMalformedByDefault = builder.ignoreMalformed.getDefaultValue().value();
-        this.metricType = TimeSeriesParams.MetricType.fromString(builder.metric.getValue());
+        this.metricType = builder.metric.getValue();
     }
 
     boolean ignoreMalformed() {
diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapperTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapperTests.java
index 714be6729da93..3c26586c2c941 100644
--- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapperTests.java
+++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapperTests.java
@@ -343,7 +343,7 @@ public void testMetricType() throws IOException {
             })));
             assertThat(
                 e.getCause().getMessage(),
-                containsString("Unknown value [gauge] for field [time_series_metric] - accepted values are [null, histogram]")
+                containsString("Unknown value [gauge] for field [time_series_metric] - accepted values are [histogram]")
             );
         }
         {
@@ -354,7 +354,7 @@ public void testMetricType() throws IOException {
             })));
             assertThat(
                 e.getCause().getMessage(),
-                containsString("Unknown value [unknown] for field [time_series_metric] - accepted values are [null, histogram]")
+                containsString("Unknown value [unknown] for field [time_series_metric] - accepted values are [histogram]")
             );
         }
     }
diff --git a/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapper.java b/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapper.java
index ea051466c309a..b3bceee6d2686 100644
--- a/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapper.java
+++ b/x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapper.java
@@ -137,7 +137,7 @@ public static class Builder extends FieldMapper.Builder {
          * For {@link AggregateDoubleMetricFieldMapper} fields gauge, counter and summary metric types are
          * supported.
          */
-        private final Parameter<String> timeSeriesMetric;
+        private final Parameter<MetricType> timeSeriesMetric;
 
         /**
          * Set the default metric so that query operations are delegated to it.
@@ -160,11 +160,10 @@ public Builder(String name, Boolean ignoreMalformedByDefault) {
             );
 
             this.timeSeriesMetric = TimeSeriesParams.metricParam(
-                m -> toType(m).metricType != null ? toType(m).metricType.name() : null,
-                null,
-                MetricType.gauge.name(),
-                MetricType.counter.name(),
-                MetricType.summary.name()
+                m -> toType(m).metricType,
+                MetricType.gauge,
+                MetricType.counter,
+                MetricType.summary
             );
         }
 
@@ -173,7 +172,7 @@ protected List<Parameter<?>> getParameters() {
             return List.of(ignoreMalformed, metrics, defaultMetric, meta, timeSeriesMetric);
         }
 
-        public Builder metric(String metric) {
+        public Builder metric(MetricType metric) {
             this.timeSeriesMetric.setValue(metric);
             return this;
         }
@@ -241,7 +240,7 @@ public AggregateDoubleMetricFieldMapper build(ContentPath context) {
             AggregateDoubleMetricFieldType metricFieldType = new AggregateDoubleMetricFieldType(
                 buildFullName(context),
                 meta.getValue(),
-                MetricType.fromString(timeSeriesMetric.getValue())
+                timeSeriesMetric.getValue()
             );
             metricFieldType.setMetricFields(metricFields);
             metricFieldType.setDefaultMetric(defaultMetric.getValue());
@@ -525,7 +524,7 @@ private AggregateDoubleMetricFieldMapper(
         this.metrics = builder.metrics.getValue();
         this.defaultMetric = builder.defaultMetric.getValue();
         this.metricFieldMappers = metricFieldMappers;
-        this.metricType = TimeSeriesParams.MetricType.fromString(builder.timeSeriesMetric.getValue());
+        this.metricType = builder.timeSeriesMetric.getValue();
     }
 
     boolean ignoreMalformed() {
@@ -652,6 +651,6 @@ protected void parseCreateField(DocumentParserContext context) throws IOExceptio
 
     @Override
     public FieldMapper.Builder getMergeBuilder() {
-        return new Builder(simpleName(), ignoreMalformedByDefault).metric(metricType != null ? metricType.name() : null).init(this);
+        return new Builder(simpleName(), ignoreMalformedByDefault).metric(metricType).init(this);
     }
 }
diff --git a/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapperTests.java b/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapperTests.java
index 9eab1d09b7d02..40419b4385b41 100644
--- a/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapperTests.java
+++ b/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapperTests.java
@@ -584,9 +584,7 @@ public void testMetricType() throws IOException {
             })));
             assertThat(
                 e.getCause().getMessage(),
-                containsString(
-                    "Unknown value [histogram] for field [time_series_metric] - accepted values are [null, gauge, counter, summary]"
-                )
+                containsString("Unknown value [histogram] for field [time_series_metric] - accepted values are [gauge, counter, summary]")
             );
         }
         {
@@ -597,9 +595,7 @@ public void testMetricType() throws IOException {
             })));
             assertThat(
                 e.getCause().getMessage(),
-                containsString(
-                    "Unknown value [unknown] for field [time_series_metric] - accepted values are [null, gauge, counter, summary]"
-                )
+                containsString("Unknown value [unknown] for field [time_series_metric] - accepted values are [gauge, counter, summary]")
             );
         }
     }

From 4d7bcd4ee048e07c5f11fb01cfe87d157c4e9457 Mon Sep 17 00:00:00 2001
From: Christos Soulios <csoulios@gmail.com>
Date: Tue, 14 Sep 2021 14:45:38 +0300
Subject: [PATCH 09/11] Addressed reviewer comments

---
 .../java/org/elasticsearch/index/mapper/FieldMapper.java     | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java
index 32eb4f2e2159c..6018f410be6a3 100644
--- a/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java
+++ b/server/src/main/java/org/elasticsearch/index/mapper/FieldMapper.java
@@ -922,8 +922,7 @@ public static <T extends Enum<T>> Parameter<T> enumParam(
             T defaultValue,
             Class<T> enumClass
         ) {
-            Set<T> acceptedValues = EnumSet.noneOf(enumClass);
-            acceptedValues.addAll(Arrays.asList(enumClass.getEnumConstants()));
+            Set<T> acceptedValues = EnumSet.allOf(enumClass);
             return restrictedEnumParam(name, updateable, initializer, defaultValue, enumClass, acceptedValues);
         }
 
@@ -954,7 +953,7 @@ public static <T extends Enum<T>> Parameter<T> restrictedEnumParam(
                     @SuppressWarnings("unchecked")
                     T enumValue = Enum.valueOf(enumClass, (String) o);
                     return enumValue;
-                } catch (Exception e) {
+                } catch (IllegalArgumentException e) {
                     throw new MapperParsingException(
                         "Unknown value [" + o + "] for field [" + name + "] - accepted values are " + values
                     );

From 6ccfb0c28751e85d49c11ead71575260017c436b Mon Sep 17 00:00:00 2001
From: Christos Soulios <csoulios@gmail.com>
Date: Tue, 14 Sep 2021 20:23:31 +0300
Subject: [PATCH 10/11] Added tests

---
 .../index/mapper/ParametrizedMapperTests.java | 90 ++++++++++++++++++-
 1 file changed, 87 insertions(+), 3 deletions(-)

diff --git a/server/src/test/java/org/elasticsearch/index/mapper/ParametrizedMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/ParametrizedMapperTests.java
index 3ec6d50b34fda..e2d7eddfed211 100644
--- a/server/src/test/java/org/elasticsearch/index/mapper/ParametrizedMapperTests.java
+++ b/server/src/test/java/org/elasticsearch/index/mapper/ParametrizedMapperTests.java
@@ -29,6 +29,7 @@
 import java.io.IOException;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.EnumSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -40,6 +41,12 @@
 
 public class ParametrizedMapperTests extends MapperServiceTestCase {
 
+    public enum DummyEnumType {
+        name1,
+        name2,
+        name3
+    }
+
     public static class TestPlugin extends Plugin implements MapperPlugin {
         @Override
         public Map<String, Mapper.TypeParser> getMappers() {
@@ -119,6 +126,23 @@ public static class Builder extends FieldMapper.Builder {
         final Parameter<String> restricted
             = Parameter.restrictedStringParam("restricted", true, m -> toType(m).restricted, "foo", "bar");
 
+        final Parameter<DummyEnumType> enumField = Parameter.enumParam(
+            "enum_field",
+            true,
+            m -> toType(m).enumField,
+            DummyEnumType.name1,
+            DummyEnumType.class
+        );
+
+        final Parameter<DummyEnumType> restrictedEnumField = Parameter.restrictedEnumParam(
+            "restricted_enum_field",
+            true,
+            m -> toType(m).restrictedEnumField,
+            DummyEnumType.name1,
+            DummyEnumType.class,
+            EnumSet.of(DummyEnumType.name1, DummyEnumType.name2)
+        );
+
         protected Builder(String name) {
             super(name);
             // only output search analyzer if different to analyzer
@@ -128,7 +152,20 @@ protected Builder(String name) {
 
         @Override
         protected List<Parameter<?>> getParameters() {
-            return List.of(fixed, fixed2, variable, index, wrapper, intValue, analyzer, searchAnalyzer, required, restricted);
+            return List.of(
+                fixed,
+                fixed2,
+                variable,
+                index,
+                wrapper,
+                intValue,
+                analyzer,
+                searchAnalyzer,
+                required,
+                restricted,
+                enumField,
+                restrictedEnumField
+            );
         }
 
         @Override
@@ -162,6 +199,8 @@ public static class TestMapper extends FieldMapper {
         private final boolean index;
         private final String required;
         private final String restricted;
+        private final DummyEnumType enumField;
+        private final DummyEnumType restrictedEnumField;
 
         protected TestMapper(String simpleName, String fullName, MultiFields multiFields, CopyTo copyTo,
                              ParametrizedMapperTests.Builder builder) {
@@ -176,6 +215,8 @@ protected TestMapper(String simpleName, String fullName, MultiFields multiFields
             this.index = builder.index.getValue();
             this.required = builder.required.getValue();
             this.restricted = builder.restricted.getValue();
+            this.enumField = builder.enumField.getValue();
+            this.restrictedEnumField = builder.restrictedEnumField.getValue();
         }
 
         @Override
@@ -253,7 +294,8 @@ public void testDefaults() throws IOException {
         assertEquals("{\"field\":{\"type\":\"test_mapper\",\"fixed\":true," +
                 "\"fixed2\":false,\"variable\":\"default\",\"index\":true," +
                 "\"wrapper\":\"default\",\"int_value\":5,\"analyzer\":\"_keyword\"," +
-                "\"required\":\"value\",\"restricted\":\"foo\"}}",
+                "\"required\":\"value\",\"restricted\":\"foo\",\"enum_field\":\"name1\"," +
+                "\"restricted_enum_field\":\"name1\"}}",
             toStringWithDefaults(mapper));
     }
 
@@ -476,7 +518,8 @@ public void testLinkedAnalyzers() throws IOException {
         assertEquals("{\"field\":{\"type\":\"test_mapper\",\"fixed\":true," +
                 "\"fixed2\":false,\"variable\":\"default\",\"index\":true," +
                 "\"wrapper\":\"default\",\"int_value\":5,\"analyzer\":\"default\"," +
-                "\"required\":\"value\",\"restricted\":\"foo\"}}",
+                "\"required\":\"value\",\"restricted\":\"foo\",\"enum_field\":\"name1\"," +
+                "\"restricted_enum_field\":\"name1\"}}",
             toStringWithDefaults(mapper));
     }
 
@@ -511,6 +554,47 @@ public void testRestrictedField() {
         }
     }
 
+    public void testEnumField() {
+        {
+            String mapping = "{\"type\":\"test_mapper\",\"required\":\"a\",\"enum_field\":\"baz\"}";
+            MapperParsingException e = expectThrows(MapperParsingException.class, () -> fromMapping(mapping));
+            assertEquals("Unknown value [baz] for field [enum_field] - accepted values are [name1, name2, name3]", e.getMessage());
+        }
+        {
+            String mapping = "{\"type\":\"test_mapper\",\"required\":\"a\",\"enum_field\":\"name3\"}";
+            TestMapper mapper = fromMapping(mapping);
+            assertEquals(DummyEnumType.name3, mapper.enumField);
+        }
+        {
+            String mapping = "{\"type\":\"test_mapper\",\"required\":\"a\"}";
+            TestMapper mapper = fromMapping(mapping);
+            assertEquals(DummyEnumType.name1, mapper.enumField);
+        }
+    }
+
+    public void testRestrictedEnumField() {
+        {
+            String mapping = "{\"type\":\"test_mapper\",\"required\":\"a\",\"restricted_enum_field\":\"baz\"}";
+            MapperParsingException e = expectThrows(MapperParsingException.class, () -> fromMapping(mapping));
+            assertEquals("Unknown value [baz] for field [restricted_enum_field] - accepted values are [name1, name2]", e.getMessage());
+        }
+        {
+            String mapping = "{\"type\":\"test_mapper\",\"required\":\"a\",\"restricted_enum_field\":\"name3\"}";
+            MapperParsingException e = expectThrows(MapperParsingException.class, () -> fromMapping(mapping));
+            assertEquals("Unknown value [name3] for field [restricted_enum_field] - accepted values are [name1, name2]", e.getMessage());
+        }
+        {
+            String mapping = "{\"type\":\"test_mapper\",\"required\":\"a\",\"restricted_enum_field\":\"name2\"}";
+            TestMapper mapper = fromMapping(mapping);
+            assertEquals(DummyEnumType.name2, mapper.restrictedEnumField);
+        }
+        {
+            String mapping = "{\"type\":\"test_mapper\",\"required\":\"a\"}";
+            TestMapper mapper = fromMapping(mapping);
+            assertEquals(DummyEnumType.name1, mapper.restrictedEnumField);
+        }
+    }
+
     public void testCustomMergeValidation() throws IOException {
         MapperService mapperService = createMapperService(fieldMapping(b -> {
             b.field("type", "test_mapper");

From 49e157b731348373e27452963467c8b9f990b4b0 Mon Sep 17 00:00:00 2001
From: Christos Soulios <csoulios@gmail.com>
Date: Wed, 15 Sep 2021 15:24:10 +0300
Subject: [PATCH 11/11] Moved assertMetricType() to parent class

---
 .../index/mapper/NumberFieldMapperTests.java       | 13 -------------
 .../elasticsearch/index/mapper/MapperTestCase.java | 12 ++++++++++++
 .../mapper/HistogramFieldMapperTests.java          | 14 --------------
 .../AggregateDoubleMetricFieldMapperTests.java     | 13 -------------
 4 files changed, 12 insertions(+), 40 deletions(-)

diff --git a/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java
index 5470063d57f16..3b3482e2171c7 100644
--- a/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java
+++ b/server/src/test/java/org/elasticsearch/index/mapper/NumberFieldMapperTests.java
@@ -21,7 +21,6 @@
 
 import java.io.IOException;
 import java.util.List;
-import java.util.function.Function;
 
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
@@ -261,18 +260,6 @@ public void testDimension() throws IOException {
         assertThat(e.getCause().getMessage(), containsString("Parameter [dimension] cannot be set"));
     }
 
-    protected <T> void assertMetricType(String metricType, Function<T, Enum<TimeSeriesParams.MetricType>> checker)
-        throws IOException {
-        MapperService mapperService = createMapperService(fieldMapping(b -> {
-            minimalMapping(b);
-            b.field("time_series_metric", metricType);
-        }));
-
-        @SuppressWarnings("unchecked") // Syntactic sugar in tests
-        T fieldType = (T) mapperService.fieldType("field");
-        assertThat(checker.apply(fieldType).name(), equalTo(metricType));
-    }
-
     public void testMetricType() throws IOException {
         // Test default setting
         MapperService mapperService = createMapperService(fieldMapping(b -> minimalMapping(b)));
diff --git a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java
index afc9132c2ebb5..1953fb79a0aba 100644
--- a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java
+++ b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java
@@ -183,6 +183,18 @@ protected <T> void assertDimension(boolean isDimension, Function<T, Boolean> che
         assertThat(checker.apply(fieldType), equalTo(isDimension));
     }
 
+    protected <T> void assertMetricType(String metricType, Function<T, Enum<TimeSeriesParams.MetricType>> checker)
+        throws IOException {
+        MapperService mapperService = createMapperService(fieldMapping(b -> {
+            minimalMapping(b);
+            b.field("time_series_metric", metricType);
+        }));
+
+        @SuppressWarnings("unchecked") // Syntactic sugar in tests
+        T fieldType = (T) mapperService.fieldType("field");
+        assertThat(checker.apply(fieldType).name(), equalTo(metricType));
+    }
+
     public final void testEmptyName() {
         MapperParsingException e = expectThrows(MapperParsingException.class, () -> createMapperService(mapping(b -> {
             b.startObject("");
diff --git a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapperTests.java b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapperTests.java
index 3c26586c2c941..32d4942e65af2 100644
--- a/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapperTests.java
+++ b/x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/mapper/HistogramFieldMapperTests.java
@@ -14,7 +14,6 @@
 import org.elasticsearch.index.mapper.MapperTestCase;
 import org.elasticsearch.index.mapper.ParsedDocument;
 import org.elasticsearch.index.mapper.SourceToParse;
-import org.elasticsearch.index.mapper.TimeSeriesParams;
 import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.xpack.analytics.AnalyticsPlugin;
 
@@ -22,10 +21,8 @@
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
-import java.util.function.Function;
 
 import static org.hamcrest.Matchers.containsString;
-import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.notNullValue;
 import static org.hamcrest.Matchers.nullValue;
 
@@ -317,17 +314,6 @@ public void testCannotBeUsedInMultifields() {
         assertThat(e.getMessage(), containsString("Field [hist] of type [histogram] can't be used in multifields"));
     }
 
-    protected <T> void assertMetricType(String metricType, Function<T, Enum<TimeSeriesParams.MetricType>> checker) throws IOException {
-        MapperService mapperService = createMapperService(fieldMapping(b -> {
-            minimalMapping(b);
-            b.field("time_series_metric", metricType);
-        }));
-
-        @SuppressWarnings("unchecked") // Syntactic sugar in tests
-        T fieldType = (T) mapperService.fieldType("field");
-        assertThat(checker.apply(fieldType).name(), equalTo(metricType));
-    }
-
     public void testMetricType() throws IOException {
         // Test default setting
         MapperService mapperService = createMapperService(fieldMapping(b -> minimalMapping(b)));
diff --git a/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapperTests.java b/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapperTests.java
index 40419b4385b41..d51ba6f638ddf 100644
--- a/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapperTests.java
+++ b/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapperTests.java
@@ -18,7 +18,6 @@
 import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.index.mapper.MapperTestCase;
 import org.elasticsearch.index.mapper.ParsedDocument;
-import org.elasticsearch.index.mapper.TimeSeriesParams;
 import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.xpack.aggregatemetric.AggregateMetricMapperPlugin;
 import org.hamcrest.Matchers;
@@ -28,7 +27,6 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
-import java.util.function.Function;
 
 import static org.elasticsearch.xpack.aggregatemetric.mapper.AggregateDoubleMetricFieldMapper.Names.IGNORE_MALFORMED;
 import static org.elasticsearch.xpack.aggregatemetric.mapper.AggregateDoubleMetricFieldMapper.Names.METRICS;
@@ -554,17 +552,6 @@ public void testCannotBeUsedInMultifields() {
         assertThat(e.getMessage(), containsString("Field [metric] of type [aggregate_metric_double] can't be used in multifields"));
     }
 
-    protected <T> void assertMetricType(String metricType, Function<T, Enum<TimeSeriesParams.MetricType>> checker) throws IOException {
-        MapperService mapperService = createMapperService(fieldMapping(b -> {
-            minimalMapping(b);
-            b.field("time_series_metric", metricType);
-        }));
-
-        @SuppressWarnings("unchecked") // Syntactic sugar in tests
-        T fieldType = (T) mapperService.fieldType("field");
-        assertThat(checker.apply(fieldType).name(), equalTo(metricType));
-    }
-
     public void testMetricType() throws IOException {
         // Test default setting
         MapperService mapperService = createMapperService(fieldMapping(b -> minimalMapping(b)));