Adressing review comments

Author: cbuescher

core/src/main/java/org/elasticsearch/search/aggregations/metrics/ParsedSingleValueNumericMetricsAggregation.java

@@ -63,7 +63,7 @@ protected static double parseValue(XContentParser parser, double defaultNullValu
             }
         }
 
-    protected static void declareSingeValueFields(ObjectParser<? extends ParsedSingleValueNumericMetricsAggregation, Void> objectParser,
+    protected static void declareSingleValueFields(ObjectParser<? extends ParsedSingleValueNumericMetricsAggregation, Void> objectParser,
             double defaultNullValue) {
         declareAggregationFields(objectParser);
         objectParser.declareField(ParsedSingleValueNumericMetricsAggregation::setValue,

core/src/main/java/org/elasticsearch/search/aggregations/metrics/avg/ParsedAvg.java

@@ -53,7 +53,7 @@ protected XContentBuilder doXContentBody(XContentBuilder builder, Params params)
     private static final ObjectParser<ParsedAvg, Void> PARSER = new ObjectParser<>(ParsedAvg.class.getSimpleName(), true, ParsedAvg::new);
 
     static {
-        declareSingeValueFields(PARSER, Double.POSITIVE_INFINITY);
+        declareSingleValueFields(PARSER, Double.POSITIVE_INFINITY);
     }
 
     public static ParsedAvg fromXContent(XContentParser parser, final String name) {

core/src/main/java/org/elasticsearch/search/aggregations/metrics/max/ParsedMax.java

@@ -51,7 +51,7 @@ protected XContentBuilder doXContentBody(XContentBuilder builder, Params params)
     private static final ObjectParser<ParsedMax, Void> PARSER = new ObjectParser<>(ParsedMax.class.getSimpleName(), true, ParsedMax::new);
 
     static {
-        declareSingeValueFields(PARSER, Double.NEGATIVE_INFINITY);
+        declareSingleValueFields(PARSER, Double.NEGATIVE_INFINITY);
     }
 
     public static ParsedMax fromXContent(XContentParser parser, final String name) {

core/src/main/java/org/elasticsearch/search/aggregations/metrics/min/ParsedMin.java

@@ -51,7 +51,7 @@ protected XContentBuilder doXContentBody(XContentBuilder builder, Params params)
     private static final ObjectParser<ParsedMin, Void> PARSER = new ObjectParser<>(ParsedMin.class.getSimpleName(), true, ParsedMin::new);
 
     static {
-        declareSingeValueFields(PARSER, Double.POSITIVE_INFINITY);
+        declareSingleValueFields(PARSER, Double.POSITIVE_INFINITY);
     }
 
     public static ParsedMin fromXContent(XContentParser parser, final String name) {

core/src/main/java/org/elasticsearch/search/aggregations/metrics/sum/ParsedSum.java

@@ -50,7 +50,7 @@ protected XContentBuilder doXContentBody(XContentBuilder builder, Params params)
     private static final ObjectParser<ParsedSum, Void> PARSER = new ObjectParser<>(ParsedSum.class.getSimpleName(), true, ParsedSum::new);
 
     static {
-        declareSingeValueFields(PARSER, Double.NEGATIVE_INFINITY);
+        declareSingleValueFields(PARSER, Double.NEGATIVE_INFINITY);
     }
 
     public static ParsedSum fromXContent(XContentParser parser, final String name) {

core/src/main/java/org/elasticsearch/search/aggregations/pipeline/ParsedSimpleValue.java

@@ -37,18 +37,18 @@ protected String getType() {
             ParsedSimpleValue::new);
 
     static {
-        declareSingeValueFields(PARSER, Double.NaN);
+        declareSingleValueFields(PARSER, Double.NaN);
     }
 
     public static ParsedSimpleValue fromXContent(XContentParser parser, final String name) {
-        ParsedSimpleValue min = PARSER.apply(parser, null);
-        min.setName(name);
-        return min;
+        ParsedSimpleValue simpleValue = PARSER.apply(parser, null);
+        simpleValue.setName(name);
+        return simpleValue;
     }
 
     @Override
     protected XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
-        boolean hasValue = !(Double.isInfinite(value) || Double.isNaN(value));
+        boolean hasValue = Double.isNaN(value) == false;
         builder.field(CommonFields.VALUE.getPreferredName(), hasValue ? value : null);
         if (hasValue && valueAsString != null) {
             builder.field(CommonFields.VALUE_AS_STRING.getPreferredName(), valueAsString);

core/src/main/java/org/elasticsearch/search/aggregations/pipeline/derivative/ParsedDerivative.java

@@ -32,7 +32,7 @@ public class ParsedDerivative extends ParsedSimpleValue implements Derivative {
 
     private double normalizedValue;
     private String normalizedAsString;
-    private boolean valuePresent;
+    private boolean hasNormalizationFactor;
     private static final ParseField NORMALIZED_AS_STRING = new ParseField("normalized_value_as_string");
     private static final ParseField NORMALIZED = new ParseField("normalized_value");
 
@@ -50,29 +50,25 @@ protected String getType() {
             ParsedDerivative::new);
 
     static {
-        declareSingeValueFields(PARSER, Double.NaN);
+        declareSingleValueFields(PARSER, Double.NaN);
         PARSER.declareField((agg, normalized) -> {
             agg.normalizedValue = normalized;
-            agg.valuePresent = true;
-        }, (parser, context) -> parseValue(parser, Double.NaN), NORMALIZED,
-                ValueType.DOUBLE_OR_NULL);
-        PARSER.declareString((agg, normalAsString) -> {
-            agg.normalizedAsString = normalAsString;
-            agg.valuePresent = true;
-        }, NORMALIZED_AS_STRING);
+            agg.hasNormalizationFactor = true;
+        }, (parser, context) -> parseValue(parser, Double.NaN), NORMALIZED, ValueType.DOUBLE_OR_NULL);
+        PARSER.declareString((agg, normalAsString) -> agg.normalizedAsString = normalAsString, NORMALIZED_AS_STRING);
     }
 
     public static ParsedDerivative fromXContent(XContentParser parser, final String name) {
-        ParsedDerivative min = PARSER.apply(parser, null);
-        min.setName(name);
-        return min;
+        ParsedDerivative derivative = PARSER.apply(parser, null);
+        derivative.setName(name);
+        return derivative;
     }
 
     @Override
     protected XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
         super.doXContentBody(builder, params);
-        if (valuePresent) {
-            boolean hasValue = !(Double.isInfinite(normalizedValue) || Double.isNaN(normalizedValue));
+        if (hasNormalizationFactor) {
+            boolean hasValue = Double.isNaN(normalizedValue) == false;
             builder.field(NORMALIZED.getPreferredName(), hasValue ? normalizedValue : null);
             if (hasValue && normalizedAsString != null) {
                 builder.field(NORMALIZED_AS_STRING.getPreferredName(), normalizedAsString);

core/src/test/java/org/elasticsearch/search/aggregations/pipeline/InternalSimpleValueTests.java

@@ -31,8 +31,8 @@
 public class InternalSimpleValueTests extends InternalAggregationTestCase<InternalSimpleValue>{
 
     @Override
-    protected InternalSimpleValue createTestInstance(String name,
-            List<PipelineAggregator> pipelineAggregators, Map<String, Object> metaData) {
+    protected InternalSimpleValue createTestInstance(String name, List<PipelineAggregator> pipelineAggregators,
+            Map<String, Object> metaData) {
         DocValueFormat formatter = randomNumericDocValueFormat();
         double value = frequently() ? randomDoubleBetween(0, 100000, true)
                 : randomFrom(new Double[] { Double.NEGATIVE_INFINITY, Double.POSITIVE_INFINITY, Double.NaN });
@@ -59,12 +59,12 @@ protected Reader<InternalSimpleValue> instanceReader() {
     protected void assertFromXContent(InternalSimpleValue simpleValue, ParsedAggregation parsedAggregation) {
         ParsedSimpleValue parsed = ((ParsedSimpleValue) parsedAggregation);
         if (Double.isInfinite(simpleValue.getValue()) == false && Double.isNaN(simpleValue.getValue()) == false) {
-            assertEquals(simpleValue.getValue(), parsed.value(), Double.MIN_VALUE);
+            assertEquals(simpleValue.getValue(), parsed.value(), 0);
             assertEquals(simpleValue.getValueAsString(), parsed.getValueAsString());
         } else {
             // we write Double.NEGATIVE_INFINITY, Double.POSITIVE amd Double.NAN to xContent as 'null', so we
             // cannot differentiate between them. Also we cannot recreate the exact String representation
-            assertEquals(parsed.value(), Double.NaN, Double.MIN_VALUE);
+            assertEquals(parsed.value(), Double.NaN, 0);
         }
     }
 }