AbstractParsedPercentiles should use Percentile class (#24160)

Now the Percentile interface has been merged with the InternalPercentile
class in core (#24154) the AbstractParsedPercentiles should use it.

This commit also changes InternalPercentilesRanksTestCase so that it now
tests the iterator obtained from parsed percentiles ranks aggregations.

Adding this new test raised an issue in the iterators where key and
value are "swapped" in internal implementations when building the
iterators (see InternalTDigestPercentileRanks.Iter constructor that
accepts the `keys` as the first parameter named `values`, each key
being mapped to the `value` field of Percentile class). This is because
 percentiles ranks aggs inverts percentiles/values compared to the
 percentiles aggs.

* Add assume in InternalAggregationTestCase

* Update after Luca review

Author: tlrx

core/src/main/java/org/elasticsearch/search/aggregations/metrics/percentiles/AbstractParsedPercentiles.java

@@ -81,7 +81,7 @@ public boolean hasNext() {
             @Override
             public Percentile next() {
                 Map.Entry<Double, Double> next = iterator.next();
-                return new InternalPercentile(next.getKey(), next.getValue());
+                return new Percentile(next.getKey(), next.getValue());
             }
         };
     }

core/src/main/java/org/elasticsearch/search/aggregations/metrics/percentiles/hdr/ParsedHDRPercentileRanks.java

@@ -23,8 +23,10 @@
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.search.aggregations.metrics.percentiles.AbstractParsedPercentiles;
 import org.elasticsearch.search.aggregations.metrics.percentiles.ParsedPercentileRanks;
+import org.elasticsearch.search.aggregations.metrics.percentiles.Percentile;
 
 import java.io.IOException;
+import java.util.Iterator;
 
 public class ParsedHDRPercentileRanks extends ParsedPercentileRanks {
 
@@ -33,6 +35,23 @@ protected String getType() {
         return InternalHDRPercentileRanks.NAME;
     }
 
+    @Override
+    public Iterator<Percentile> iterator() {
+        final Iterator<Percentile> iterator = super.iterator();
+        return new Iterator<Percentile>() {
+            @Override
+            public boolean hasNext() {
+                return iterator.hasNext();
+            }
+
+            @Override
+            public Percentile next() {
+                Percentile percentile = iterator.next();
+                return new Percentile(percentile.getValue(), percentile.getPercent());
+            }
+        };
+    }
+
     private static ObjectParser<ParsedHDRPercentileRanks, Void> PARSER =
             new ObjectParser<>(ParsedHDRPercentileRanks.class.getSimpleName(), true, ParsedHDRPercentileRanks::new);
     static {

core/src/main/java/org/elasticsearch/search/aggregations/metrics/percentiles/tdigest/ParsedTDigestPercentileRanks.java

@@ -23,8 +23,10 @@
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.search.aggregations.metrics.percentiles.AbstractParsedPercentiles;
 import org.elasticsearch.search.aggregations.metrics.percentiles.ParsedPercentileRanks;
+import org.elasticsearch.search.aggregations.metrics.percentiles.Percentile;
 
 import java.io.IOException;
+import java.util.Iterator;
 
 public class ParsedTDigestPercentileRanks extends ParsedPercentileRanks {
 
@@ -33,6 +35,23 @@ protected String getType() {
         return InternalTDigestPercentileRanks.NAME;
     }
 
+    @Override
+    public Iterator<Percentile> iterator() {
+        final Iterator<Percentile> iterator = super.iterator();
+        return new Iterator<Percentile>() {
+            @Override
+            public boolean hasNext() {
+                return iterator.hasNext();
+            }
+
+            @Override
+            public Percentile next() {
+                Percentile percentile = iterator.next();
+                return new Percentile(percentile.getValue(), percentile.getPercent());
+            }
+        };
+    }
+
     private static ObjectParser<ParsedTDigestPercentileRanks, Void> PARSER =
             new ObjectParser<>(ParsedTDigestPercentileRanks.class.getSimpleName(), true, ParsedTDigestPercentileRanks::new);
     static {

core/src/test/java/org/elasticsearch/search/aggregations/InternalAggregationTestCase.java

@@ -54,7 +54,6 @@
 import static java.util.Collections.singletonMap;
 import static org.elasticsearch.common.xcontent.XContentHelper.toXContent;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertToXContentEquivalent;
-import static org.hamcrest.Matchers.containsString;
 
 public abstract class InternalAggregationTestCase<T extends InternalAggregation> extends AbstractWireSerializingTestCase<T> {
 
@@ -169,6 +168,10 @@ public final void testFromXContent() throws IOException {
         final NamedXContentRegistry xContentRegistry = xContentRegistry();
         final T aggregation = createTestInstance();
 
+        //norelease Remove this assumption when all aggregations can be parsed back.
+        assumeTrue("This test does not support the aggregation type yet",
+                getNamedXContents().stream().filter(entry -> entry.name.match(aggregation.getType())).count() > 0);
+
         final ToXContent.Params params = new ToXContent.MapParams(singletonMap(RestSearchAction.TYPED_KEYS_PARAM, "true"));
         final boolean humanReadable = randomBoolean();
         final XContentType xContentType = randomFrom(XContentType.values());
@@ -199,10 +202,6 @@ public final void testFromXContent() throws IOException {
             final BytesReference parsedBytes = toXContent((ToXContent) parsedAggregation, xContentType, params, humanReadable);
             assertToXContentEquivalent(originalBytes, parsedBytes, xContentType);
             assertFromXContent(aggregation, (ParsedAggregation) parsedAggregation);
-
-        } catch (NamedXContentRegistry.UnknownNamedObjectException e) {
-            //norelease Remove this catch block when all aggregations can be parsed back.
-            assertThat(e.getMessage(), containsString("Unknown Aggregation"));
         }
     }
 

core/src/test/java/org/elasticsearch/search/aggregations/metrics/percentiles/InternalPercentilesRanksTestCase.java

@@ -19,15 +19,26 @@
 
 package org.elasticsearch.search.aggregations.metrics.percentiles;
 
+import org.elasticsearch.common.bytes.BytesReference;
+import org.elasticsearch.common.xcontent.ToXContent;
+import org.elasticsearch.common.xcontent.XContentParser;
+import org.elasticsearch.common.xcontent.XContentType;
+import org.elasticsearch.rest.action.search.RestSearchAction;
 import org.elasticsearch.search.DocValueFormat;
+import org.elasticsearch.search.aggregations.Aggregation;
 import org.elasticsearch.search.aggregations.InternalAggregation;
 import org.elasticsearch.search.aggregations.InternalAggregationTestCase;
 import org.elasticsearch.search.aggregations.ParsedAggregation;
 import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator;
 
+import java.io.IOException;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 
+import static java.util.Collections.singletonMap;
+import static org.elasticsearch.common.xcontent.XContentHelper.toXContent;
+
 public abstract class InternalPercentilesRanksTestCase<T extends InternalAggregation> extends InternalAggregationTestCase<T> {
 
     @Override
@@ -64,5 +75,36 @@ protected final void assertFromXContent(T aggregation, ParsedAggregation parsedA
         assertTrue(parsedClass.isInstance(parsedAggregation));
     }
 
+    public void testPercentilesRanksIterators() throws IOException {
+        final T aggregation = createTestInstance();
+
+        final ToXContent.Params params = new ToXContent.MapParams(singletonMap(RestSearchAction.TYPED_KEYS_PARAM, "true"));
+        final XContentType xContentType = randomFrom(XContentType.values());
+        final BytesReference originalBytes = toXContent(aggregation, xContentType, params, randomBoolean());
+
+        Aggregation parsedAggregation;
+        try (XContentParser parser = xContentType.xContent().createParser(xContentRegistry(), originalBytes)) {
+            assertEquals(XContentParser.Token.START_OBJECT, parser.nextToken());
+            assertEquals(XContentParser.Token.FIELD_NAME, parser.nextToken());
+
+            String currentName = parser.currentName();
+            int i = currentName.indexOf(InternalAggregation.TYPED_KEYS_DELIMITER);
+            String aggType = currentName.substring(0, i);
+            String aggName = currentName.substring(i + 1);
+
+            parsedAggregation = parser.namedObject(Aggregation.class, aggType, aggName);
+
+            assertEquals(XContentParser.Token.END_OBJECT, parser.currentToken());
+            assertEquals(XContentParser.Token.END_OBJECT, parser.nextToken());
+            assertNull(parser.nextToken());
+        }
+
+        final Iterator<Percentile> it = ((PercentileRanks) aggregation).iterator();
+        final Iterator<Percentile> parsedIt = ((PercentileRanks) parsedAggregation).iterator();
+        while (it.hasNext()) {
+            assertEquals(it.next(), parsedIt.next());
+        }
+    }
+
     protected abstract Class<? extends ParsedPercentileRanks> parsedParsedPercentileRanksClass();
 }