Bläddra i källkod

Add a min/max validation to `aggregate_metric_double` fields (#90381)

When parsing min/max values for an `aggregate_metric_double` fields, we must always check that max value is greater than min value.
weizijun 3 år sedan
förälder
incheckning
aa95070ea3

+ 5 - 0
docs/changelog/90381.yaml

@@ -0,0 +1,5 @@
+pr: 90381
+summary: aggregate metric double add a max min validation
+area: Mapping
+type: enhancement
+issues: []

+ 11 - 0
x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapper.java

@@ -572,6 +572,7 @@ public class AggregateDoubleMetricFieldMapper extends FieldMapper {
         XContentParser.Token token;
         XContentSubParser subParser = null;
         EnumSet<Metric> metricsParsed = EnumSet.noneOf(Metric.class);
+        Number max = null, min = null;
         try {
             token = context.parser().currentToken();
             if (token == XContentParser.Token.VALUE_NULL) {
@@ -629,10 +630,20 @@ public class AggregateDoubleMetricFieldMapper extends FieldMapper {
                             "Aggregate metric [" + metric.name() + "] of field [" + mappedFieldType.name() + "] cannot be a negative number"
                         );
                     }
+                } else if (Metric.max == metric) {
+                    max = context.doc().getByKey(delegateFieldMapper.name()).numericValue();
+                } else if (Metric.min == metric) {
+                    min = context.doc().getByKey(delegateFieldMapper.name()).numericValue();
                 }
                 metricsParsed.add(metric);
                 token = subParser.nextToken();
             }
+            // check max value must bigger then min value
+            if (max != null && min != null && max.doubleValue() < min.doubleValue()) {
+                throw new IllegalArgumentException(
+                    "Aggregate metric field [" + mappedFieldType.name() + "] max value cannot be smaller than min value"
+                );
+            }
 
             // Check if all required metrics have been parsed.
             if (metricsParsed.containsAll(metrics) == false) {

+ 43 - 0
x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapperTests.java

@@ -209,6 +209,45 @@ public class AggregateDoubleMetricFieldMapperTests extends MapperTestCase {
         assertEquals(77, doc.rootDoc().getField("field.value_count").numericValue().longValue());
     }
 
+    /**
+     * Test parsing a value_count metric written as double with some decimal digits
+     */
+    public void testInvalidDoubleValueCount() throws Exception {
+        DocumentMapper mapper = createDocumentMapper(fieldMapping(this::minimalMapping));
+        Exception e = expectThrows(
+            MapperParsingException.class,
+            () -> mapper.parse(
+                source(b -> b.startObject("field").field("min", 10.0).field("max", 50.0).field("value_count", 77.33).endObject())
+            )
+        );
+        assertThat(
+            e.getCause().getMessage(),
+            containsString("failed to parse field [field.value_count] of type [integer] in document with id '1'.")
+        );
+    }
+
+    /**
+     * Test parsing a metric and check the min max value
+     */
+    public void testCheckMinMaxValue() throws Exception {
+        DocumentMapper mapper = createDocumentMapper(fieldMapping(this::minimalMapping));
+
+        // min > max
+        Exception e = expectThrows(
+            MapperParsingException.class,
+            () -> mapper.parse(
+                source(b -> b.startObject("field").field("min", 50.0).field("max", 10.0).field("value_count", 14).endObject())
+            )
+        );
+        assertThat(e.getCause().getMessage(), containsString("Aggregate metric field [field] max value cannot be smaller than min value"));
+
+        // min == max
+        mapper.parse(source(b -> b.startObject("field").field("min", 50.0).field("max", 50.0).field("value_count", 14).endObject()));
+
+        // min < max
+        mapper.parse(source(b -> b.startObject("field").field("min", 10.0).field("max", 50.0).field("value_count", 14).endObject()));
+    }
+
     private void randomMapping(XContentBuilder b, int randomNumber) throws IOException {
         b.field("type", CONTENT_TYPE);
         switch (randomNumber) {
@@ -483,6 +522,10 @@ public class AggregateDoubleMetricFieldMapperTests extends MapperTestCase {
             for (Metric m : storedMetrics) {
                 if (Metric.value_count == m) {
                     value.put(m.name(), randomLongBetween(1, 1_000_000));
+                } else if (Metric.max == m) {
+                    value.put(m.name(), randomDoubleBetween(100d, 1_000_000d, false));
+                } else if (Metric.min == m) {
+                    value.put(m.name(), randomDoubleBetween(-1_000_000d, 99d, false));
                 } else {
                     value.put(m.name(), randomDouble());
                 }

+ 4 - 4
x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/downsample/DownsampleActionSingleNodeTests.java

@@ -251,7 +251,7 @@ public class DownsampleActionSingleNodeTests extends ESSingleNodeTestCase {
                 .field(FIELD_NUMERIC_1, randomInt())
                 .field(FIELD_NUMERIC_2, DATE_FORMATTER.parseMillis(ts))
                 .startObject(FIELD_AGG_METRIC)
-                .field("min", randomDoubleBetween(-1000, 1000, true))
+                .field("min", randomDoubleBetween(-2000, -1001, true))
                 .field("max", randomDoubleBetween(-1000, 1000, true))
                 .field("sum", randomIntBetween(100, 10000))
                 .field("value_count", randomIntBetween(100, 1000))
@@ -269,7 +269,7 @@ public class DownsampleActionSingleNodeTests extends ESSingleNodeTestCase {
                 .field(FIELD_LABEL_KEYWORD_ARRAY, keywordArray)
                 .field(FIELD_LABEL_DOUBLE_ARRAY, doubleArray)
                 .startObject(FIELD_LABEL_AGG_METRIC)
-                .field("min", randomDoubleBetween(-1000, 1000, true))
+                .field("min", randomDoubleBetween(-2000, -1001, true))
                 .field("max", randomDoubleBetween(-1000, 1000, true))
                 .field("sum", Double.valueOf(randomIntBetween(100, 10000)))
                 .field("value_count", randomIntBetween(100, 1000))
@@ -296,7 +296,7 @@ public class DownsampleActionSingleNodeTests extends ESSingleNodeTestCase {
                 .field(FIELD_NUMERIC_1, randomInt())
                 .field(FIELD_NUMERIC_2, DATE_FORMATTER.parseMillis(ts))
                 .startObject(FIELD_AGG_METRIC)
-                .field("min", randomDoubleBetween(-1000, 1000, true))
+                .field("min", randomDoubleBetween(-2000, -1001, true))
                 .field("max", randomDoubleBetween(-1000, 1000, true))
                 .field("sum", randomIntBetween(100, 10000))
                 .field("value_count", randomIntBetween(100, 1000))
@@ -304,7 +304,7 @@ public class DownsampleActionSingleNodeTests extends ESSingleNodeTestCase {
                 .field(FIELD_LABEL_DOUBLE, labelDoubleValue)
                 .field(FIELD_METRIC_LABEL_DOUBLE, labelDoubleValue)
                 .startObject(FIELD_LABEL_AGG_METRIC)
-                .field("min", randomDoubleBetween(-1000, 1000, true))
+                .field("min", randomDoubleBetween(-2000, -1001, true))
                 .field("max", randomDoubleBetween(-1000, 1000, true))
                 .field("sum", Double.valueOf(randomIntBetween(100, 10000)))
                 .field("value_count", randomIntBetween(100, 1000))