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
This commit is contained in:
Tanguy Leroux 2017-04-18 16:54:17 +02:00 committed by GitHub
parent 67a9696e55
commit c1ba6997ff
5 changed files with 85 additions and 6 deletions

View File

@ -81,7 +81,7 @@ public abstract class AbstractParsedPercentiles extends ParsedAggregation implem
@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());
}
};
}

View File

@ -23,8 +23,10 @@ import org.elasticsearch.common.xcontent.ObjectParser;
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 @@ public class ParsedHDRPercentileRanks extends ParsedPercentileRanks {
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 {

View File

@ -23,8 +23,10 @@ import org.elasticsearch.common.xcontent.ObjectParser;
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 @@ public class ParsedTDigestPercentileRanks extends ParsedPercentileRanks {
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 {

View File

@ -54,7 +54,6 @@ import static java.util.Collections.emptyList;
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 abstract class InternalAggregationTestCase<T extends InternalAggregation>
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 abstract class InternalAggregationTestCase<T extends InternalAggregation>
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"));
}
}

View File

@ -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 @@ public abstract class InternalPercentilesRanksTestCase<T extends InternalAggrega
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();
}