From 9d0fc76f544d954968158b4f66ce509cdb18477c Mon Sep 17 00:00:00 2001 From: uboness Date: Thu, 27 Feb 2014 07:58:28 -0800 Subject: [PATCH] Added support for sorting buckets based on sub aggregations Supports sorting on sub-aggs down the current hierarchy. This is supported as long as the aggregation in the specified order path are of a single-bucket type, where the last aggregation in the path points to either a single-bucket aggregation or a metrics one. If it's a single-bucket aggregation, the sort will be applied on the document count in the bucket (i.e. doc_count), and if it is a metrics type, the sort will be applied on the pointed out metric (in case of a single-metric aggregations, such as avg, the sort will be applied on the single metric value) NOTE: this commit adds a constraint on what should be considered a valid aggregation name. Aggregations names must be alpha-numeric and may contain '-' and '_'. Closes #5253 --- .../bucket/histogram-aggregation.asciidoc | 42 +++ .../bucket/terms-aggregation.asciidoc | 40 +++ .../common/util/AbstractHash.java | 2 +- .../search/aggregations/Aggregator.java | 14 + .../aggregations/AggregatorParsers.java | 9 + .../search/aggregations/HasAggregations.java | 29 ++ .../bucket/BucketsAggregator.java | 2 +- .../bucket/MultiBucketsAggregation.java | 56 +-- .../bucket/SingleBucketAggregation.java | 3 +- .../bucket/histogram/DateHistogramParser.java | 6 +- .../bucket/histogram/Histogram.java | 9 +- .../bucket/histogram/HistogramAggregator.java | 2 +- .../bucket/histogram/HistogramParser.java | 6 +- .../bucket/histogram/InternalOrder.java | 4 - .../bucket/terms/InternalOrder.java | 133 +++----- .../aggregations/bucket/terms/Terms.java | 8 +- .../bucket/terms/TermsParser.java | 6 +- .../aggregations/support/OrderPath.java | 319 ++++++++++++++++++ .../search/aggregations/ParsingTests.java | 36 ++ .../aggregations/bucket/DoubleTermsTests.java | 103 +++++- .../aggregations/bucket/HistogramTests.java | 41 +++ .../aggregations/bucket/LongTermsTests.java | 101 +++++- .../aggregations/bucket/StringTermsTests.java | 105 +++++- .../aggregations/support/PathTests.java | 109 ++++++ 24 files changed, 1012 insertions(+), 173 deletions(-) create mode 100644 src/main/java/org/elasticsearch/search/aggregations/HasAggregations.java create mode 100644 src/main/java/org/elasticsearch/search/aggregations/support/OrderPath.java create mode 100644 src/test/java/org/elasticsearch/search/aggregations/support/PathTests.java diff --git a/docs/reference/search/aggregations/bucket/histogram-aggregation.asciidoc b/docs/reference/search/aggregations/bucket/histogram-aggregation.asciidoc index 469ea23db31..38b3c982301 100644 --- a/docs/reference/search/aggregations/bucket/histogram-aggregation.asciidoc +++ b/docs/reference/search/aggregations/bucket/histogram-aggregation.asciidoc @@ -175,6 +175,48 @@ If the histogram aggregation has a direct metrics sub-aggregation, the latter ca <2> There is no need to configure the `price` field for the `price_stats` aggregation as it will inherit it by default from its parent histogram aggregation. +It is also possible to order the buckets based on a "deeper" aggregation in the hierarchy. This is supported as long +as the aggregations path are of a single-bucket type, where the last aggregation in the path may either by a single-bucket +one or a metrics one. If it's a single-bucket type, the order will be defined by the number of docs in the bucket (i.e. `doc_count`), +in case it's a metrics one, the same rules as above apply (where the path must indicate the metric name to sort by in case of +a multi-value metrics aggregation, and in case of a single-value metrics aggregation the sort will be applied on that value). + +The path must be defined in the following form: + +-------------------------------------------------- +AGG_SEPARATOR := '>' +METRIC_SEPARATOR := '.' +AGG_NAME := +METRIC := +PATH := []*[] +-------------------------------------------------- + +[source,js] +-------------------------------------------------- +{ + "aggs" : { + "prices" : { + "histogram" : { + "field" : "price", + "interval" : 50, + "order" : { "promoted_products>rating_stats.avg" : "desc" } <1> + }, + "aggs" : { + "promoted_products" : { + "filter" : { "term" : { "promoted" : true }}, + "aggs" : { + "rating_stats" : { "stats" : { "field" : "rating" }} + } + } + } + } + } +} +-------------------------------------------------- + +The above will sort the buckets based on the avg rating among the promoted products + + ==== Minimum document count It is possible to only return buckets that have a document count that is greater than or equal to a configured diff --git a/docs/reference/search/aggregations/bucket/terms-aggregation.asciidoc b/docs/reference/search/aggregations/bucket/terms-aggregation.asciidoc index 2d386c2e8f9..09583e290c1 100644 --- a/docs/reference/search/aggregations/bucket/terms-aggregation.asciidoc +++ b/docs/reference/search/aggregations/bucket/terms-aggregation.asciidoc @@ -144,6 +144,46 @@ Ordering the buckets by multi value metrics sub-aggregation (identified by the a } -------------------------------------------------- +It is also possible to order the buckets based on a "deeper" aggregation in the hierarchy. This is supported as long +as the aggregations path are of a single-bucket type, where the last aggregation in the path may either by a single-bucket +one or a metrics one. If it's a single-bucket type, the order will be defined by the number of docs in the bucket (i.e. `doc_count`), +in case it's a metrics one, the same rules as above apply (where the path must indicate the metric name to sort by in case of +a multi-value metrics aggregation, and in case of a single-value metrics aggregation the sort will be applied on that value). + +The path must be defined in the following form: + +-------------------------------------------------- +AGG_SEPARATOR := '>' +METRIC_SEPARATOR := '.' +AGG_NAME := +METRIC := +PATH := []*[] +-------------------------------------------------- + +[source,js] +-------------------------------------------------- +{ + "aggs" : { + "countries" : { + "terms" : { + "field" : "address.country", + "order" : { "females>height_stats.avg" : "desc" } + }, + "aggs" : { + "females" : { + "filter" : { "term" : { "gender" : { "female" }}}, + "aggs" : { + "height_stats" : { "stats" : { "field" : "height" }} + } + } + } + } + } +} +-------------------------------------------------- + +The above will sort the countries buckets based on the average height among the female population. + ==== Minimum document count It is possible to only return terms that match more than a configured number of hits using the `min_doc_count` option: diff --git a/src/main/java/org/elasticsearch/common/util/AbstractHash.java b/src/main/java/org/elasticsearch/common/util/AbstractHash.java index 8b08b6c14f3..8dcb4b1ed4d 100644 --- a/src/main/java/org/elasticsearch/common/util/AbstractHash.java +++ b/src/main/java/org/elasticsearch/common/util/AbstractHash.java @@ -77,7 +77,7 @@ abstract class AbstractHash implements Releasable { } /** - * Get the id associated with key at 0 <e; index <e; capacity() or -1 if this slot is unused. + * Get the id associated with key at 0 <= index <= capacity() or -1 if this slot is unused. */ public long id(long index) { return ids.get(index) - 1; diff --git a/src/main/java/org/elasticsearch/search/aggregations/Aggregator.java b/src/main/java/org/elasticsearch/search/aggregations/Aggregator.java index d5cf64d88d8..5065b634396 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/Aggregator.java +++ b/src/main/java/org/elasticsearch/search/aggregations/Aggregator.java @@ -28,7 +28,9 @@ import org.elasticsearch.search.internal.SearchContext; import java.io.IOException; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; public abstract class Aggregator implements Releasable, ReaderContextAware { @@ -59,6 +61,8 @@ public abstract class Aggregator implements Releasable, ReaderContextAware { protected final AggregatorFactories factories; protected final Aggregator[] subAggregators; + private Map subAggregatorbyName; + /** * Constructs a new Aggregator. * @@ -113,6 +117,16 @@ public abstract class Aggregator implements Releasable, ReaderContextAware { return subAggregators; } + public Aggregator subAggregator(String aggName) { + if (subAggregatorbyName == null) { + subAggregatorbyName = new HashMap(subAggregators.length); + for (int i = 0; i < subAggregators.length; i++) { + subAggregatorbyName.put(subAggregators[i].name, subAggregators[i]); + } + } + return subAggregatorbyName.get(aggName); + } + /** * @return The current aggregation context. */ diff --git a/src/main/java/org/elasticsearch/search/aggregations/AggregatorParsers.java b/src/main/java/org/elasticsearch/search/aggregations/AggregatorParsers.java index 372e1195ad9..27de8dfbcdb 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/AggregatorParsers.java +++ b/src/main/java/org/elasticsearch/search/aggregations/AggregatorParsers.java @@ -27,14 +27,18 @@ import org.elasticsearch.search.internal.SearchContext; import java.io.IOException; import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; /** * A registry for all the aggregator parser, also servers as the main parser for the aggregations module */ public class AggregatorParsers { + public static final Pattern VALID_AGG_NAME = Pattern.compile("[a-zA-Z0-9\\-_]+"); private final ImmutableMap parsers; + /** * Constructs the AggregatorParsers out of all the given parsers * @@ -78,6 +82,8 @@ public class AggregatorParsers { XContentParser.Token token = null; String currentFieldName = null; + Matcher validAggMatcher = VALID_AGG_NAME.matcher(""); + AggregatorFactories.Builder factories = new AggregatorFactories.Builder(); String aggregationName = null; @@ -102,6 +108,9 @@ public class AggregatorParsers { if (aggregatorParser == null) { throw new SearchParseException(context, "Could not find aggregator type [" + currentFieldName + "]"); } + if (!validAggMatcher.reset(aggregationName).matches()) { + throw new SearchParseException(context, "Invalid aggregation name [" + aggregationName + "]. Aggregation names must be alpha-numeric and can only contain '_' and '-'"); + } factory = aggregatorParser.parse(aggregationName, parser, context); } } diff --git a/src/main/java/org/elasticsearch/search/aggregations/HasAggregations.java b/src/main/java/org/elasticsearch/search/aggregations/HasAggregations.java new file mode 100644 index 00000000000..46020b050bc --- /dev/null +++ b/src/main/java/org/elasticsearch/search/aggregations/HasAggregations.java @@ -0,0 +1,29 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.search.aggregations; + +/** + * + */ +public interface HasAggregations { + + Aggregations getAggregations(); + +} diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/BucketsAggregator.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/BucketsAggregator.java index 8c1558077aa..28438601e42 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/BucketsAggregator.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/BucketsAggregator.java @@ -84,7 +84,7 @@ public abstract class BucketsAggregator extends Aggregator { /** * Utility method to return the number of documents that fell in the given bucket (identified by the bucket ordinal) */ - protected final long bucketDocCount(long bucketOrd) { + public final long bucketDocCount(long bucketOrd) { if (bucketOrd >= docCounts.size()) { // This may happen eg. if no document in the highest buckets is accepted by a sub aggregator. // For example, if there is a long terms agg on 3 terms 1,2,3 with a sub filter aggregator and if no document with 3 as a value diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/MultiBucketsAggregation.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/MultiBucketsAggregation.java index 4c7408cc5eb..d1eebcfcb79 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/MultiBucketsAggregation.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/MultiBucketsAggregation.java @@ -19,12 +19,12 @@ package org.elasticsearch.search.aggregations.bucket; -import org.elasticsearch.ElasticsearchIllegalArgumentException; import org.elasticsearch.common.text.Text; import org.elasticsearch.common.util.Comparators; import org.elasticsearch.search.aggregations.Aggregation; import org.elasticsearch.search.aggregations.Aggregations; -import org.elasticsearch.search.aggregations.metrics.MetricsAggregation; +import org.elasticsearch.search.aggregations.HasAggregations; +import org.elasticsearch.search.aggregations.support.OrderPath; import java.util.Collection; @@ -38,7 +38,7 @@ public interface MultiBucketsAggregation extends Aggregation { * A bucket represents a criteria to which all documents that fall in it adhere to. It is also uniquely identified * by a key, and can potentially hold sub-aggregations computed over all documents in it. */ - public interface Bucket { + public interface Bucket extends HasAggregations { /** * @return The key associated with the bucket as a string @@ -62,66 +62,28 @@ public interface MultiBucketsAggregation extends Aggregation { static class SubAggregationComparator implements java.util.Comparator { - private final String aggName; - private final String valueName; + private final OrderPath path; private final boolean asc; public SubAggregationComparator(String expression, boolean asc) { this.asc = asc; - int i = expression.indexOf('.'); - if (i < 0) { - this.aggName = expression; - this.valueName = null; - } else { - this.aggName = expression.substring(0, i); - this.valueName = expression.substring(i+1); - } - } - - public SubAggregationComparator(String aggName, String valueName, boolean asc) { - this.aggName = aggName; - this.valueName = valueName; - this.asc = asc; + this.path = OrderPath.parse(expression); } public boolean asc() { return asc; } - public String aggName() { - return aggName; - } - - public String valueName() { - return valueName; + public OrderPath path() { + return path; } @Override public int compare(B b1, B b2) { - double v1 = value(b1); - double v2 = value(b2); + double v1 = path.resolveValue(b1); + double v2 = path.resolveValue(b2); return Comparators.compareDiscardNaN(v1, v2, asc); } - - private double value(B bucket) { - MetricsAggregation aggregation = bucket.getAggregations().get(aggName); - if (aggregation == null) { - throw new ElasticsearchIllegalArgumentException("Unknown aggregation named [" + aggName + "]"); - } - if (aggregation instanceof MetricsAggregation.SingleValue) { - //TODO should we throw an exception if the value name is specified? - return ((MetricsAggregation.SingleValue) aggregation).value(); - } - if (aggregation instanceof MetricsAggregation.MultiValue) { - if (valueName == null) { - throw new ElasticsearchIllegalArgumentException("Cannot sort on multi valued aggregation [" + aggName + "]. A value name is required"); - } - return ((MetricsAggregation.MultiValue) aggregation).value(valueName); - } - - throw new ElasticsearchIllegalArgumentException("A mal attempt to sort terms by aggregation [" + aggregation.getName() + - "]. Terms can only be ordered by either standard order or direct calc aggregators of the terms"); - } } } diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/SingleBucketAggregation.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/SingleBucketAggregation.java index de02387eb6f..a437960cf7e 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/SingleBucketAggregation.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/SingleBucketAggregation.java @@ -21,11 +21,12 @@ package org.elasticsearch.search.aggregations.bucket; import org.elasticsearch.search.aggregations.Aggregation; import org.elasticsearch.search.aggregations.Aggregations; +import org.elasticsearch.search.aggregations.HasAggregations; /** * A single bucket aggregation */ -public interface SingleBucketAggregation extends Aggregation { +public interface SingleBucketAggregation extends Aggregation, HasAggregations { /** * @return The number of documents in this bucket diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramParser.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramParser.java index 85d615e22b9..eb3f42ce9e9 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramParser.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramParser.java @@ -238,11 +238,7 @@ public class DateHistogramParser implements Aggregator.Parser { if ("_count".equals(key)) { return (InternalOrder) (asc ? InternalOrder.COUNT_ASC : InternalOrder.COUNT_DESC); } - int i = key.indexOf('.'); - if (i < 0) { - return new InternalOrder.Aggregation(key, null, asc); - } - return new InternalOrder.Aggregation(key.substring(0, i), key.substring(i + 1), asc); + return new InternalOrder.Aggregation(key, asc); } private long parseOffset(String offset) throws IOException { diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/Histogram.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/Histogram.java index 229e8077c83..9ebf2284732 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/Histogram.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/Histogram.java @@ -20,7 +20,6 @@ package org.elasticsearch.search.aggregations.bucket.histogram; import com.google.common.primitives.Longs; import org.elasticsearch.common.xcontent.ToXContent; -import org.elasticsearch.search.aggregations.Aggregation; import org.elasticsearch.search.aggregations.bucket.MultiBucketsAggregation; import java.util.Collection; @@ -110,11 +109,11 @@ public interface Histogram extends MultiBucketsAggregation { /** * Creates a bucket ordering strategy that sorts buckets based on a single-valued calc sug-aggregation * - * @param aggregationName the name of the aggregation + * @param path the name of the aggregation * @param asc The direction of the order (ascending or descending) */ - public static Order aggregation(String aggregationName, boolean asc) { - return new InternalOrder.Aggregation(aggregationName, null, asc); + public static Order aggregation(String path, boolean asc) { + return new InternalOrder.Aggregation(path, asc); } /** @@ -125,7 +124,7 @@ public interface Histogram extends MultiBucketsAggregation { * @param asc The direction of the order (ascending or descending) */ public static Order aggregation(String aggregationName, String valueName, boolean asc) { - return new InternalOrder.Aggregation(aggregationName, valueName, asc); + return new InternalOrder.Aggregation(aggregationName + "." + valueName, asc); } /** diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/HistogramAggregator.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/HistogramAggregator.java index 5544c21c673..8156586fa3a 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/HistogramAggregator.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/HistogramAggregator.java @@ -23,12 +23,12 @@ import org.apache.lucene.util.CollectionUtil; import org.elasticsearch.common.inject.internal.Nullable; import org.elasticsearch.common.lease.Releasables; import org.elasticsearch.common.rounding.Rounding; +import org.elasticsearch.common.util.LongHash; import org.elasticsearch.index.fielddata.LongValues; import org.elasticsearch.search.aggregations.Aggregator; import org.elasticsearch.search.aggregations.AggregatorFactories; import org.elasticsearch.search.aggregations.InternalAggregation; import org.elasticsearch.search.aggregations.bucket.BucketsAggregator; -import org.elasticsearch.common.util.LongHash; import org.elasticsearch.search.aggregations.support.AggregationContext; import org.elasticsearch.search.aggregations.support.ValueSourceAggregatorFactory; import org.elasticsearch.search.aggregations.support.ValuesSourceConfig; diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/HistogramParser.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/HistogramParser.java index 7005db5e475..aba5708e7c6 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/HistogramParser.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/HistogramParser.java @@ -159,10 +159,6 @@ public class HistogramParser implements Aggregator.Parser { if ("_count".equals(key)) { return (InternalOrder) (asc ? InternalOrder.COUNT_ASC : InternalOrder.COUNT_DESC); } - int i = key.indexOf('.'); - if (i < 0) { - return new InternalOrder.Aggregation(key, null, asc); - } - return new InternalOrder.Aggregation(key.substring(0, i), key.substring(i + 1), asc); + return new InternalOrder.Aggregation(key, asc); } } diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/InternalOrder.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/InternalOrder.java index e427499e93e..9d503a8e90b 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/InternalOrder.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/InternalOrder.java @@ -73,10 +73,6 @@ class InternalOrder extends Histogram.Order { super(ID, key, asc, new MultiBucketsAggregation.Bucket.SubAggregationComparator(key, asc)); } - Aggregation(String aggName, String valueName, boolean asc) { - super(ID, key(aggName, valueName), asc, new MultiBucketsAggregation.Bucket.SubAggregationComparator(aggName, valueName, asc)); - } - private static String key(String aggName, String valueName) { return (valueName == null) ? aggName : aggName + "." + valueName; } diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/InternalOrder.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/InternalOrder.java index 73adfe91b26..a315b1fe982 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/InternalOrder.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/InternalOrder.java @@ -19,14 +19,17 @@ package org.elasticsearch.search.aggregations.bucket.terms; import com.google.common.primitives.Longs; +import org.elasticsearch.Version; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.util.Comparators; import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.search.aggregations.AggregationExecutionException; import org.elasticsearch.search.aggregations.Aggregator; +import org.elasticsearch.search.aggregations.bucket.BucketsAggregator; import org.elasticsearch.search.aggregations.bucket.MultiBucketsAggregation; +import org.elasticsearch.search.aggregations.bucket.SingleBucketAggregator; import org.elasticsearch.search.aggregations.metrics.MetricsAggregator; +import org.elasticsearch.search.aggregations.support.OrderPath; import java.io.IOException; import java.util.Comparator; @@ -104,14 +107,6 @@ class InternalOrder extends Terms.Order { return id; } - String key() { - return key; - } - - boolean asc() { - return asc; - } - @Override protected Comparator comparator(Aggregator aggregator) { return comparator; @@ -126,39 +121,9 @@ class InternalOrder extends Terms.Order { if (!(order instanceof Aggregation)) { return order; } - String aggName = ((Aggregation) order).aggName(); - Aggregator[] subAggregators = termsAggregator.subAggregators(); - for (int i = 0; i < subAggregators.length; i++) { - Aggregator aggregator = subAggregators[i]; - if (aggregator.name().equals(aggName)) { - - // we can only apply order on metrics sub-aggregators - if (!(aggregator instanceof MetricsAggregator)) { - throw new AggregationExecutionException("terms aggregation [" + termsAggregator.name() + "] is configured to order by sub-aggregation [" - + aggName + "] which is is not a metrics aggregation. Terms aggregation order can only refer to metrics aggregations"); - } - - if (aggregator instanceof MetricsAggregator.MultiValue) { - String valueName = ((Aggregation) order).metricName(); - if (valueName == null) { - throw new AggregationExecutionException("terms aggregation [" + termsAggregator.name() + "] is configured with a sub-aggregation order [" - + aggName + "] which is a multi-valued aggregation, yet no metric name was specified"); - } - if (!((MetricsAggregator.MultiValue) aggregator).hasMetric(valueName)) { - throw new AggregationExecutionException("terms aggregation [" + termsAggregator.name() + "] is configured with a sub-aggregation order [" - + aggName + "] and value [" + valueName + "] yet the referred sub aggregator holds no metric that goes by this name"); - } - return order; - } - - // aggregator must be of a single value type - // todo we can also choose to be really strict and verify that the user didn't specify a value name and if so fail? - return order; - } - } - - throw new AggregationExecutionException("terms aggregation [" + termsAggregator.name() + "] is configured with a sub-aggregation order [" - + aggName + "] but no sub aggregation with this name is configured"); + OrderPath path = ((Aggregation) order).path(); + path.validate(termsAggregator); + return order; } static class Aggregation extends InternalOrder { @@ -169,22 +134,8 @@ class InternalOrder extends Terms.Order { super(ID, key, asc, new MultiBucketsAggregation.Bucket.SubAggregationComparator(key, asc)); } - Aggregation(String aggName, String metricName, boolean asc) { - super(ID, key(aggName, metricName), asc, new MultiBucketsAggregation.Bucket.SubAggregationComparator(aggName, metricName, asc)); - } - - String aggName() { - int index = key.indexOf('.'); - return index < 0 ? key : key.substring(0, index); - } - - String metricName() { - int index = key.indexOf('.'); - return index < 0 ? null : key.substring(index + 1, key.length()); - } - - private static String key(String aggName, String valueName) { - return (valueName == null) ? aggName : aggName + "." + valueName; + OrderPath path() { + return ((MultiBucketsAggregation.Bucket.SubAggregationComparator) comparator).path(); } @Override @@ -201,16 +152,32 @@ class InternalOrder extends Terms.Order { // sub aggregation values directly from the sub aggregators bypassing bucket creation. Note that the comparator // attached to the order will still be used in the reduce phase of the Aggregation. - final Aggregator aggregator = subAggregator(aggName(), termsAggregator); - assert aggregator != null && aggregator instanceof MetricsAggregator : "this should be picked up before the aggregation is executed"; - if (aggregator instanceof MetricsAggregator.MultiValue) { - final String valueName = metricName(); - assert valueName != null : "this should be picked up before the aggregation is executed"; + OrderPath path = path(); + final Aggregator aggregator = path.resolveAggregator(termsAggregator, false); + final String key = path.tokens[path.tokens.length - 1].key; + + if (aggregator instanceof SingleBucketAggregator) { + assert key == null : "this should be picked up before the aggregation is executed - on validate"; return new Comparator() { @Override public int compare(Terms.Bucket o1, Terms.Bucket o2) { - double v1 = ((MetricsAggregator.MultiValue) aggregator).metric(valueName, ((InternalTerms.Bucket) o1).bucketOrd); - double v2 = ((MetricsAggregator.MultiValue) aggregator).metric(valueName, ((InternalTerms.Bucket) o2).bucketOrd); + long v1 = ((SingleBucketAggregator) aggregator).bucketDocCount(((InternalTerms.Bucket) o1).bucketOrd); + long v2 = ((SingleBucketAggregator) aggregator).bucketDocCount(((InternalTerms.Bucket) o2).bucketOrd); + return asc ? Longs.compare(v1, v2) : Longs.compare(v2, v1); + } + }; + } + + // with only support single-bucket aggregators + assert !(aggregator instanceof BucketsAggregator) : "this should be picked up before the aggregation is executed - on validate"; + + if (aggregator instanceof MetricsAggregator.MultiValue) { + assert key != null : "this should be picked up before the aggregation is executed - on validate"; + return new Comparator() { + @Override + public int compare(Terms.Bucket o1, Terms.Bucket o2) { + double v1 = ((MetricsAggregator.MultiValue) aggregator).metric(key, ((InternalTerms.Bucket) o1).bucketOrd); + double v2 = ((MetricsAggregator.MultiValue) aggregator).metric(key, ((InternalTerms.Bucket) o2).bucketOrd); // some metrics may return NaN (eg. avg, variance, etc...) in which case we'd like to push all of those to // the bottom return Comparators.compareDiscardNaN(v1, v2, asc); @@ -218,6 +185,7 @@ class InternalOrder extends Terms.Order { }; } + // single-value metrics agg return new Comparator() { @Override public int compare(Terms.Bucket o1, Terms.Bucket o2) { @@ -229,16 +197,6 @@ class InternalOrder extends Terms.Order { } }; } - - private Aggregator subAggregator(String aggName, Aggregator termsAggregator) { - Aggregator[] subAggregators = termsAggregator.subAggregators(); - for (int i = 0; i < subAggregators.length; i++) { - if (subAggregators[i].name().equals(aggName)) { - return subAggregators[i]; - } - } - return null; - } } public static class Streams { @@ -247,11 +205,18 @@ class InternalOrder extends Terms.Order { out.writeByte(order.id()); if (order instanceof Aggregation) { out.writeBoolean(((MultiBucketsAggregation.Bucket.SubAggregationComparator) order.comparator).asc()); - out.writeString(((MultiBucketsAggregation.Bucket.SubAggregationComparator) order.comparator).aggName()); - boolean hasValueName = ((MultiBucketsAggregation.Bucket.SubAggregationComparator) order.comparator).valueName() != null; - out.writeBoolean(hasValueName); - if (hasValueName) { - out.writeString(((MultiBucketsAggregation.Bucket.SubAggregationComparator) order.comparator).valueName()); + OrderPath path = ((Aggregation) order).path(); + if (out.getVersion().onOrAfter(Version.V_1_1_0)) { + out.writeString(path.toString()); + } else { + // prev versions only supported sorting on a single level -> a single token; + OrderPath.Token token = path.lastToken(); + out.writeString(token.name); + boolean hasValueName = token.key != null; + out.writeBoolean(hasValueName); + if (hasValueName) { + out.writeString(token.key); + } } } } @@ -266,8 +231,12 @@ class InternalOrder extends Terms.Order { case 0: boolean asc = in.readBoolean(); String key = in.readString(); - if (in.readBoolean()) { - return new InternalOrder.Aggregation(key, in.readString(), asc); + if (in.getVersion().onOrAfter(Version.V_1_1_0)) { + return new InternalOrder.Aggregation(key, asc); + } + boolean hasValueNmae = in.readBoolean(); + if (hasValueNmae) { + return new InternalOrder.Aggregation(key + "." + in.readString(), asc); } return new InternalOrder.Aggregation(key, asc); default: diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/Terms.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/Terms.java index 55478766c1e..3c7b9b27364 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/Terms.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/Terms.java @@ -95,11 +95,11 @@ public interface Terms extends MultiBucketsAggregation { /** * Creates a bucket ordering strategy which sorts buckets based on a single-valued calc get * - * @param aggregationName the name of the get + * @param path the name of the get * @param asc The direction of the order (ascending or descending) */ - public static Order aggregation(String aggregationName, boolean asc) { - return new InternalOrder.Aggregation(aggregationName, null, asc); + public static Order aggregation(String path, boolean asc) { + return new InternalOrder.Aggregation(path, asc); } /** @@ -110,7 +110,7 @@ public interface Terms extends MultiBucketsAggregation { * @param asc The direction of the order (ascending or descending) */ public static Order aggregation(String aggregationName, String metricName, boolean asc) { - return new InternalOrder.Aggregation(aggregationName, metricName, asc); + return new InternalOrder.Aggregation(aggregationName + "." + metricName, asc); } /** diff --git a/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsParser.java b/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsParser.java index 69ab83e7770..7d585d1f16c 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsParser.java +++ b/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/TermsParser.java @@ -276,11 +276,7 @@ public class TermsParser implements Aggregator.Parser { if ("_count".equals(key)) { return asc ? InternalOrder.COUNT_ASC : InternalOrder.COUNT_DESC; } - int i = key.indexOf('.'); - if (i < 0) { - return new InternalOrder.Aggregation(key, asc); - } - return new InternalOrder.Aggregation(key.substring(0, i), key.substring(i+1), asc); + return new InternalOrder.Aggregation(key, asc); } } diff --git a/src/main/java/org/elasticsearch/search/aggregations/support/OrderPath.java b/src/main/java/org/elasticsearch/search/aggregations/support/OrderPath.java new file mode 100644 index 00000000000..ce1140e0618 --- /dev/null +++ b/src/main/java/org/elasticsearch/search/aggregations/support/OrderPath.java @@ -0,0 +1,319 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.search.aggregations.support; + +import org.elasticsearch.ElasticsearchIllegalArgumentException; +import org.elasticsearch.common.Strings; +import org.elasticsearch.search.aggregations.Aggregation; +import org.elasticsearch.search.aggregations.AggregationExecutionException; +import org.elasticsearch.search.aggregations.Aggregator; +import org.elasticsearch.search.aggregations.HasAggregations; +import org.elasticsearch.search.aggregations.bucket.SingleBucketAggregation; +import org.elasticsearch.search.aggregations.bucket.SingleBucketAggregator; +import org.elasticsearch.search.aggregations.metrics.MetricsAggregation; +import org.elasticsearch.search.aggregations.metrics.MetricsAggregator; + +/** + * A path that can be used to sort/order buckets (in some multi-bucket aggregations, eg terms & histogram) based on + * sub-aggregations. The path may point to either a single-bucket aggregation or a metrics aggregation. If the path + * points to a single-bucket aggregation, the sort will be applied based on the {@code doc_count} of the bucket. If this + * path points to a metrics aggregation, if it's a single-value metrics (eg. avg, max, min, etc..) the sort will be + * applied on that single value. If it points to a multi-value metrics, the path should point out what metric should be + * the sort-by value. + *

+ * The path has the following form: + *

+ *

{@code ['>'*]['.']}
+ *

+ *

+ * Examples: + * + *

    + *
  • + * {@code agg1>agg2>agg3} - where agg1, agg2 and agg3 are all single-bucket aggs (eg filter, nested, missing, etc..). In + * this case, the order will be based on the number of documents under {@code agg3}. + *
    • + *
  • + * {@code agg1>agg2>agg3} - where agg1 and agg2 are both single-bucket aggs and agg3 is a single-value metrics agg (eg avg, max, min, etc..). + * In this case, the order will be based on the value of {@code agg3}. + *
    • + *
  • + * {@code agg1>agg2>agg3.avg} - where agg1 and agg2 are both single-bucket aggs and agg3 is a multi-value metrics agg (eg stats, extended_stats, etc...). + * In this case, the order will be based on the avg value of {@code agg3}. + *
    • + *
+ * + */ +public class OrderPath { + + private final static String AGG_DELIM = ">"; + + public static OrderPath parse(String path) { + String[] elements = Strings.tokenizeToStringArray(path, AGG_DELIM); + Token[] tokens = new Token[elements.length]; + String[] tuple = new String[2]; + for (int i = 0; i < elements.length; i++) { + String element = elements[i]; + int index = element.lastIndexOf('.'); + if (index >= 0) { + if (index == 0 || index > element.length() - 2) { + throw new AggregationExecutionException("Invalid path element [" + element + "] in path [" + path + "]"); + } + tuple = split(element, index, tuple); + tokens[i] = new Token(element, tuple[0], tuple[1]); + continue; + } + index = element.lastIndexOf('['); + if (index < 0) { + tokens[i] = new Token(element, element, null); + continue; + } + if (index == 0 || index > element.length() - 3) { + throw new AggregationExecutionException("Invalid path element [" + element + "] in path [" + path + "]"); + } + if (element.charAt(element.length() - 1) != ']') { + throw new AggregationExecutionException("Invalid path element [" + element + "] in path [" + path + "]"); + } + tokens[i] = new Token(element, element.substring(0, index), element.substring(index + 1, element.length() - 1)); + } + return new OrderPath(tokens); + } + + public static class Token { + + private final String fullName; + public final String name; + public final String key; + + public Token(String fullName, String name, String key) { + this.fullName = fullName; + this.name = name; + this.key = key; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + + Token token = (Token) o; + + if (key != null ? !key.equals(token.key) : token.key != null) return false; + if (!name.equals(token.name)) return false; + + return true; + } + + @Override + public int hashCode() { + int result = name.hashCode(); + result = 31 * result + (key != null ? key.hashCode() : 0); + return result; + } + + @Override + public String toString() { + return fullName; + } + } + + public final Token[] tokens; + + public OrderPath(Token[] tokens) { + this.tokens = tokens; + if (tokens == null || tokens.length == 0) { + throw new ElasticsearchIllegalArgumentException("Invalid path [" + this + "]"); + } + } + + @Override + public String toString() { + return Strings.arrayToDelimitedString(tokens, AGG_DELIM); + } + + public Token lastToken() { + return tokens[tokens.length - 1]; + } + + public OrderPath subPath(int offset, int length) { + Token[] subTokens = new Token[length]; + System.arraycopy(tokens, offset, subTokens, 0, length); + return new OrderPath(tokens); + } + + /** + * Resolves the value pointed by this path given an aggregations root + * + * @param root The root that serves as a point of reference for this path + * @return The resolved value + */ + public double resolveValue(HasAggregations root) { + HasAggregations parent = root; + double value = Double.NaN; + for (int i = 0; i < tokens.length; i++) { + OrderPath.Token token = tokens[i]; + Aggregation agg = parent.getAggregations().get(token.name); + + if (agg == null) { + throw new ElasticsearchIllegalArgumentException("Invalid order path [" + this + + "]. Cannot find aggregation named [" + token.name + "]"); + } + + if (agg instanceof SingleBucketAggregation) { + if (token.key != null && !token.key.equals("doc_count")) { + throw new ElasticsearchIllegalArgumentException("Invalid order path [" + this + + "]. Unknown value key [" + token.key + "] for single-bucket aggregation [" + token.name + + "]. Either use [doc_count] as key or drop the key all together"); + } + parent = (SingleBucketAggregation) agg; + value = ((SingleBucketAggregation) agg).getDocCount(); + continue; + } + + // the agg can only be a metrics agg, and a metrics agg must be at the end of the path + if (i != tokens.length - 1) { + throw new ElasticsearchIllegalArgumentException("Invalid order path [" + this + + "]. Metrics aggregations cannot have sub-aggregations (at [" + token + ">" + tokens[i+1] + "]"); + } + + if (agg instanceof MetricsAggregation.SingleValue) { + if (token.key != null && !token.key.equals("value")) { + throw new ElasticsearchIllegalArgumentException("Invalid order path [" + this + + "]. Unknown value key [" + token.key + "] for single-value metric aggregation [" + token.name + + "]. Either use [value] as key or drop the key all together"); + } + parent = null; + value = ((MetricsAggregation.SingleValue) agg).value(); + continue; + } + + // we're left with a multi-value metric agg + if (token.key == null) { + throw new ElasticsearchIllegalArgumentException("Invalid order path [" + this + + "]. Missing value key in [" + token + "] which refers to a multi-value metric aggregation"); + } + parent = null; + value = ((MetricsAggregation.MultiValue) agg).value(token.key); + } + + return value; + } + + /** + * Resolves the aggregator pointed by this path using the given root as a point of reference. + * + * @param root The point of reference of this path + * @param validate Indicates whether the path should be validated first over the given root aggregator + * @return The aggregator pointed by this path starting from the given aggregator as a point of reference + */ + public Aggregator resolveAggregator(Aggregator root, boolean validate) { + if (validate) { + validate(root); + } + Aggregator aggregator = root; + for (int i = 0; i < tokens.length; i++) { + OrderPath.Token token = tokens[i]; + aggregator = aggregator.subAggregator(token.name); + assert (aggregator instanceof SingleBucketAggregator && i <= tokens.length - 1) || + (aggregator instanceof MetricsAggregator && i == tokens.length - 1) : + "this should be picked up before aggregation execution - on validate"; + } + return aggregator; + } + + /** + * Validates this path over the given aggregator as a point of reference. + * + * @param root The point of reference of this path + */ + public void validate(Aggregator root) { + Aggregator aggregator = root; + for (int i = 0; i < tokens.length; i++) { + aggregator = aggregator.subAggregator(tokens[i].name); + if (aggregator == null) { + throw new AggregationExecutionException("Invalid term-aggregator order path [" + this + "]. Unknown aggregation [" + tokens[i].name + "]"); + } + if (i < tokens.length - 1) { + + // we're in the middle of the path, so the aggregator can only be a single-bucket aggregator + + if (!(aggregator instanceof SingleBucketAggregator)) { + throw new AggregationExecutionException("Invalid terms aggregation order path [" + this + + "]. Terms buckets can only be sorted on a sub-aggregator path " + + "that is built out of zero or more single-bucket aggregations within the path and a final " + + "single-bucket or a metrics aggregation at the path end. Sub-path [" + + subPath(0, i + 1) + "] points to non single-bucket aggregation"); + } + + if (tokens[i].key != null) { + throw new AggregationExecutionException("Invalid terms aggregation order path [" + this + + "]. Terms buckets can only be sorted on a sub-aggregator path " + + "that is built out of zero or more single-bucket aggregations within the path and a " + + "final single-bucket or a metrics aggregation at the path end. Sub-path [" + + subPath(0, i + 1) + "] points to non single-bucket aggregation"); + } + } + } + boolean singleBucket = aggregator instanceof SingleBucketAggregator; + if (!singleBucket && !(aggregator instanceof MetricsAggregator)) { + throw new AggregationExecutionException("Invalid terms aggregation order path [" + this + + "]. Terms buckets can only be sorted on a sub-aggregator path " + + "that is built out of zero or more single-bucket aggregations within the path and a final " + + "single-bucket or a metrics aggregation at the path end."); + } + + OrderPath.Token lastToken = lastToken(); + + if (singleBucket) { + if (lastToken.key != null && !"doc_count".equals(lastToken.key)) { + throw new AggregationExecutionException("Invalid terms aggregation order path [" + this + + "]. Ordering on a single-bucket aggregation can only be done on its doc_count. " + + "Either drop the key (a la \"" + lastToken.name + "\") or change it to \"doc_count\" (a la \"" + lastToken.name + ".doc_count\")"); + } + return; // perfectly valid to sort on single-bucket aggregation (will be sored on its doc_count) + } + + if (aggregator instanceof MetricsAggregator.SingleValue) { + if (lastToken.key != null && !"value".equals(lastToken.key)) { + throw new AggregationExecutionException("Invalid terms aggregation order path [" + this + + "]. Ordering on a single-value metrics aggregation can only be done on its value. " + + "Either drop the key (a la \"" + lastToken.name + "\") or change it to \"value\" (a la \"" + lastToken.name + ".value\")"); + } + return; // perfectly valid to sort on single metric aggregation (will be sorted on its associated value) + } + + // the aggregator must be of a multi-value metrics type + if (lastToken.key == null) { + throw new AggregationExecutionException("Invalid terms aggregation order path [" + this + + "]. When ordering on a multi-value metrics aggregation a metric name must be specified"); + } + + if (!((MetricsAggregator.MultiValue) aggregator).hasMetric(lastToken.key)) { + throw new AggregationExecutionException("Invalid terms aggregation order path [" + this + + "]. Unknown metric name [" + lastToken.key + "] on multi-value metrics aggregation [" + lastToken.name + "]"); + } + } + + private static String[] split(String toSplit, int index, String[] result) { + result[0] = toSplit.substring(0, index); + result[1] = toSplit.substring(index + 1); + return result; + } +} diff --git a/src/test/java/org/elasticsearch/search/aggregations/ParsingTests.java b/src/test/java/org/elasticsearch/search/aggregations/ParsingTests.java index 9ae230a7e1f..9be451fca97 100644 --- a/src/test/java/org/elasticsearch/search/aggregations/ParsingTests.java +++ b/src/test/java/org/elasticsearch/search/aggregations/ParsingTests.java @@ -20,10 +20,15 @@ package org.elasticsearch.search.aggregations; import org.elasticsearch.action.search.SearchPhaseExecutionException; +import org.elasticsearch.common.Strings; import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.test.ElasticsearchIntegrationTest; import org.junit.Test; +import java.security.SecureRandom; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + public class ParsingTests extends ElasticsearchIntegrationTest { @Test(expected=SearchPhaseExecutionException.class) @@ -46,4 +51,35 @@ public class ParsingTests extends ElasticsearchIntegrationTest { .endObject()).execute().actionGet(); } + @Test(expected=SearchPhaseExecutionException.class) + public void testInvalidAggregationName() throws Exception { + + Matcher matcher = Pattern.compile("[a-zA-Z0-9\\-_]+").matcher(""); + String name; + SecureRandom rand = new SecureRandom(); + int len = randomIntBetween(1, 5); + char[] word = new char[len]; + while(true) { + for (int i = 0; i < word.length; i++) { + word[i] = (char) rand.nextInt(127); + } + name = String.valueOf(word); + if (!matcher.reset(name).matches()) { + break; + } + } + + createIndex("idx"); + client().prepareSearch("idx").setAggregations(JsonXContent.contentBuilder() + .startObject() + .startObject(name) + .startObject("filter") + .startObject("range") + .startObject("stock") + .field("gt", 0) + .endObject() + .endObject() + .endObject() + .endObject()).execute().actionGet(); + } } diff --git a/src/test/java/org/elasticsearch/search/aggregations/bucket/DoubleTermsTests.java b/src/test/java/org/elasticsearch/search/aggregations/bucket/DoubleTermsTests.java index 8c682442562..df18e96e6b6 100644 --- a/src/test/java/org/elasticsearch/search/aggregations/bucket/DoubleTermsTests.java +++ b/src/test/java/org/elasticsearch/search/aggregations/bucket/DoubleTermsTests.java @@ -25,9 +25,11 @@ import org.elasticsearch.common.settings.ImmutableSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.query.FilterBuilders; import org.elasticsearch.index.query.functionscore.ScoreFunctionBuilders; +import org.elasticsearch.search.aggregations.bucket.filter.Filter; import org.elasticsearch.search.aggregations.bucket.histogram.Histogram; import org.elasticsearch.search.aggregations.bucket.terms.Terms; import org.elasticsearch.search.aggregations.metrics.avg.Avg; +import org.elasticsearch.search.aggregations.metrics.max.Max; import org.elasticsearch.search.aggregations.metrics.stats.Stats; import org.elasticsearch.search.aggregations.metrics.stats.extended.ExtendedStats; import org.elasticsearch.search.aggregations.metrics.sum.Sum; @@ -38,6 +40,7 @@ import org.junit.Before; import org.junit.Test; import java.util.ArrayList; +import java.util.Iterator; import java.util.List; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; @@ -75,7 +78,8 @@ public class DoubleTermsTests extends ElasticsearchIntegrationTest { lowcardBuilders[i] = client().prepareIndex("idx", "type").setSource(jsonBuilder() .startObject() .field(SINGLE_VALUED_FIELD_NAME, (double) i) - .startArray(MULTI_VALUED_FIELD_NAME).value((double)i).value(i + 1d).endArray() + .field("num_tag", i < lowcardBuilders.length/2 + 1 ? 1 : 0) // used to test order by single-bucket sub agg + .startArray(MULTI_VALUED_FIELD_NAME).value((double) i).value(i + 1d).endArray() .endObject()); } @@ -673,6 +677,97 @@ public class DoubleTermsTests extends ElasticsearchIntegrationTest { } } + @Test + public void singleValuedField_OrderedBySingleBucketSubAggregationAsc() throws Exception { + boolean asc = randomBoolean(); + SearchResponse response = client().prepareSearch("idx").setTypes("type") + .addAggregation(terms("num_tags") + .field("num_tag") + .order(Terms.Order.aggregation("filter", asc)) + .subAggregation(filter("filter").filter(FilterBuilders.matchAllFilter())) + ).execute().actionGet(); + + + assertSearchResponse(response); + + Terms tags = response.getAggregations().get("num_tags"); + assertThat(tags, notNullValue()); + assertThat(tags.getName(), equalTo("num_tags")); + assertThat(tags.getBuckets().size(), equalTo(2)); + + Iterator iters = tags.getBuckets().iterator(); + + Terms.Bucket tag = iters.next(); + assertThat(tag, notNullValue()); + assertThat(key(tag), equalTo(asc ? "0" : "1")); + assertThat(tag.getDocCount(), equalTo(asc ? 2l : 3l)); + Filter filter = tag.getAggregations().get("filter"); + assertThat(filter, notNullValue()); + assertThat(filter.getDocCount(), equalTo(asc ? 2l : 3l)); + + tag = iters.next(); + assertThat(tag, notNullValue()); + assertThat(key(tag), equalTo(asc ? "1" : "0")); + assertThat(tag.getDocCount(), equalTo(asc ? 3l : 2l)); + filter = tag.getAggregations().get("filter"); + assertThat(filter, notNullValue()); + assertThat(filter.getDocCount(), equalTo(asc ? 3l : 2l)); + } + + @Test + public void singleValuedField_OrderedBySubAggregationAsc_MultiHierarchyLevels() throws Exception { + boolean asc = randomBoolean(); + SearchResponse response = client().prepareSearch("idx").setTypes("type") + .addAggregation(terms("tags") + .field("num_tag") + .order(Terms.Order.aggregation("filter1>filter2>max", asc)) + .subAggregation(filter("filter1").filter(FilterBuilders.matchAllFilter()) + .subAggregation(filter("filter2").filter(FilterBuilders.matchAllFilter()) + .subAggregation(max("max").field(SINGLE_VALUED_FIELD_NAME)))) + ).execute().actionGet(); + + + assertSearchResponse(response); + + Terms tags = response.getAggregations().get("tags"); + assertThat(tags, notNullValue()); + assertThat(tags.getName(), equalTo("tags")); + assertThat(tags.getBuckets().size(), equalTo(2)); + + Iterator iters = tags.getBuckets().iterator(); + + // the max for "1" is 2 + // the max for "0" is 4 + + Terms.Bucket tag = iters.next(); + assertThat(tag, notNullValue()); + assertThat(key(tag), equalTo(asc ? "1" : "0")); + assertThat(tag.getDocCount(), equalTo(asc ? 3l : 2l)); + Filter filter1 = tag.getAggregations().get("filter1"); + assertThat(filter1, notNullValue()); + assertThat(filter1.getDocCount(), equalTo(asc ? 3l : 2l)); + Filter filter2 = filter1.getAggregations().get("filter2"); + assertThat(filter2, notNullValue()); + assertThat(filter2.getDocCount(), equalTo(asc ? 3l : 2l)); + Max max = filter2.getAggregations().get("max"); + assertThat(max, notNullValue()); + assertThat(max.getValue(), equalTo(asc ? 2.0 : 4.0)); + + tag = iters.next(); + assertThat(tag, notNullValue()); + assertThat(key(tag), equalTo(asc ? "0" : "1")); + assertThat(tag.getDocCount(), equalTo(asc ? 2l : 3l)); + filter1 = tag.getAggregations().get("filter1"); + assertThat(filter1, notNullValue()); + assertThat(filter1.getDocCount(), equalTo(asc ? 2l : 3l)); + filter2 = filter1.getAggregations().get("filter2"); + assertThat(filter2, notNullValue()); + assertThat(filter2.getDocCount(), equalTo(asc ? 2l : 3l)); + max = filter2.getAggregations().get("max"); + assertThat(max, notNullValue()); + assertThat(max.getValue(), equalTo(asc ? 4.0 : 2.0)); + } + @Test public void singleValuedField_OrderedByMissingSubAggregation() throws Exception { @@ -693,7 +788,7 @@ public class DoubleTermsTests extends ElasticsearchIntegrationTest { } @Test - public void singleValuedField_OrderedByNonMetricsSubAggregation() throws Exception { + public void singleValuedField_OrderedByNonMetricsOrMultiBucketSubAggregation() throws Exception { MockBigArrays.discardNextCheck(); try { @@ -701,8 +796,8 @@ public class DoubleTermsTests extends ElasticsearchIntegrationTest { client().prepareSearch("idx").setTypes("type") .addAggregation(terms("terms") .field(SINGLE_VALUED_FIELD_NAME) - .order(Terms.Order.aggregation("filter", true)) - .subAggregation(filter("filter").filter(FilterBuilders.termFilter("foo", "bar"))) + .order(Terms.Order.aggregation("num_tags", true)) + .subAggregation(terms("num_tags").field("num_tags")) ).execute().actionGet(); fail("Expected search to fail when trying to sort terms aggregation by sug-aggregation which is not of a metrics type"); diff --git a/src/test/java/org/elasticsearch/search/aggregations/bucket/HistogramTests.java b/src/test/java/org/elasticsearch/search/aggregations/bucket/HistogramTests.java index b9f38d3a410..f04b1a9c7b0 100644 --- a/src/test/java/org/elasticsearch/search/aggregations/bucket/HistogramTests.java +++ b/src/test/java/org/elasticsearch/search/aggregations/bucket/HistogramTests.java @@ -23,8 +23,10 @@ import org.elasticsearch.action.index.IndexRequestBuilder; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.common.settings.ImmutableSettings; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.search.aggregations.bucket.filter.Filter; import org.elasticsearch.search.aggregations.bucket.histogram.Histogram; import org.elasticsearch.search.aggregations.bucket.terms.Terms; +import org.elasticsearch.search.aggregations.metrics.max.Max; import org.elasticsearch.search.aggregations.metrics.stats.Stats; import org.elasticsearch.search.aggregations.metrics.sum.Sum; import org.elasticsearch.test.ElasticsearchIntegrationTest; @@ -37,6 +39,7 @@ import java.util.Iterator; import java.util.List; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; +import static org.elasticsearch.index.query.FilterBuilders.matchAllFilter; import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; import static org.elasticsearch.search.aggregations.AggregationBuilders.*; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse; @@ -458,6 +461,44 @@ public class HistogramTests extends ElasticsearchIntegrationTest { } } + @Test + public void singleValuedField_OrderedBySubAggregationDesc_DeepOrderPath() throws Exception { + boolean asc = randomBoolean(); + SearchResponse response = client().prepareSearch("idx") + .addAggregation(histogram("histo").field(SINGLE_VALUED_FIELD_NAME).interval(interval).order(Histogram.Order.aggregation("filter>max", asc)) + .subAggregation(filter("filter").filter(matchAllFilter()) + .subAggregation(max("max").field(SINGLE_VALUED_FIELD_NAME)))) + .execute().actionGet(); + + assertSearchResponse(response); + + + Histogram histo = response.getAggregations().get("histo"); + assertThat(histo, notNullValue()); + assertThat(histo.getName(), equalTo("histo")); + assertThat(histo.getBuckets().size(), equalTo(numValueBuckets)); + + LongOpenHashSet visited = new LongOpenHashSet(); + double prevMax = asc? Double.NEGATIVE_INFINITY : Double.POSITIVE_INFINITY; + List buckets = new ArrayList(histo.getBuckets()); + for (int i = 0; i < numValueBuckets; ++i) { + Histogram.Bucket bucket = buckets.get(i); + assertThat(bucket, notNullValue()); + long key = bucket.getKeyAsNumber().longValue(); + assertTrue(visited.add(key)); + int b = (int) (key / interval); + assertThat(bucket.getDocCount(), equalTo(valueCounts[b])); + assertThat(bucket.getAggregations().asList().isEmpty(), is(false)); + Filter filter = bucket.getAggregations().get("filter"); + assertThat(filter, notNullValue()); + assertThat(bucket.getDocCount(), equalTo(filter.getDocCount())); + Max max = filter.getAggregations().get("max"); + assertThat(max, Matchers.notNullValue()); + assertThat(max.getValue(), asc ? greaterThanOrEqualTo(prevMax) : lessThanOrEqualTo(prevMax)); + prevMax = max.getValue(); + } + } + @Test public void singleValuedField_WithValueScript() throws Exception { SearchResponse response = client().prepareSearch("idx") diff --git a/src/test/java/org/elasticsearch/search/aggregations/bucket/LongTermsTests.java b/src/test/java/org/elasticsearch/search/aggregations/bucket/LongTermsTests.java index 7b66f64db46..89f857097c0 100644 --- a/src/test/java/org/elasticsearch/search/aggregations/bucket/LongTermsTests.java +++ b/src/test/java/org/elasticsearch/search/aggregations/bucket/LongTermsTests.java @@ -24,9 +24,11 @@ import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.common.settings.ImmutableSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.query.FilterBuilders; +import org.elasticsearch.search.aggregations.bucket.filter.Filter; import org.elasticsearch.search.aggregations.bucket.histogram.Histogram; import org.elasticsearch.search.aggregations.bucket.terms.Terms; import org.elasticsearch.search.aggregations.metrics.avg.Avg; +import org.elasticsearch.search.aggregations.metrics.max.Max; import org.elasticsearch.search.aggregations.metrics.stats.Stats; import org.elasticsearch.search.aggregations.metrics.stats.extended.ExtendedStats; import org.elasticsearch.search.aggregations.metrics.sum.Sum; @@ -37,6 +39,7 @@ import org.junit.Before; import org.junit.Test; import java.util.ArrayList; +import java.util.Iterator; import java.util.List; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; @@ -73,6 +76,7 @@ public class LongTermsTests extends ElasticsearchIntegrationTest { .startObject() .field(SINGLE_VALUED_FIELD_NAME, i) .startArray(MULTI_VALUED_FIELD_NAME).value(i).value(i + 1).endArray() + .field("num_tag", i < lowCardBuilders.length / 2 + 1 ? 1 : 0) // used to test order by single-bucket sub agg .endObject()); } indexRandom(randomBoolean(), lowCardBuilders); @@ -665,6 +669,97 @@ public class LongTermsTests extends ElasticsearchIntegrationTest { } } + @Test + public void singleValuedField_OrderedBySingleBucketSubAggregationAsc() throws Exception { + boolean asc = randomBoolean(); + SearchResponse response = client().prepareSearch("idx").setTypes("type") + .addAggregation(terms("num_tags") + .field("num_tag") + .order(Terms.Order.aggregation("filter", asc)) + .subAggregation(filter("filter").filter(FilterBuilders.matchAllFilter())) + ).execute().actionGet(); + + + assertSearchResponse(response); + + Terms tags = response.getAggregations().get("num_tags"); + assertThat(tags, notNullValue()); + assertThat(tags.getName(), equalTo("num_tags")); + assertThat(tags.getBuckets().size(), equalTo(2)); + + Iterator iters = tags.getBuckets().iterator(); + + Terms.Bucket tag = iters.next(); + assertThat(tag, notNullValue()); + assertThat(key(tag), equalTo(asc ? "0" : "1")); + assertThat(tag.getDocCount(), equalTo(asc ? 2l : 3l)); + Filter filter = tag.getAggregations().get("filter"); + assertThat(filter, notNullValue()); + assertThat(filter.getDocCount(), equalTo(asc ? 2l : 3l)); + + tag = iters.next(); + assertThat(tag, notNullValue()); + assertThat(key(tag), equalTo(asc ? "1" : "0")); + assertThat(tag.getDocCount(), equalTo(asc ? 3l : 2l)); + filter = tag.getAggregations().get("filter"); + assertThat(filter, notNullValue()); + assertThat(filter.getDocCount(), equalTo(asc ? 3l : 2l)); + } + + @Test + public void singleValuedField_OrderedBySubAggregationAsc_MultiHierarchyLevels() throws Exception { + boolean asc = randomBoolean(); + SearchResponse response = client().prepareSearch("idx").setTypes("type") + .addAggregation(terms("tags") + .field("num_tag") + .order(Terms.Order.aggregation("filter1>filter2>max", asc)) + .subAggregation(filter("filter1").filter(FilterBuilders.matchAllFilter()) + .subAggregation(filter("filter2").filter(FilterBuilders.matchAllFilter()) + .subAggregation(max("max").field(SINGLE_VALUED_FIELD_NAME)))) + ).execute().actionGet(); + + + assertSearchResponse(response); + + Terms tags = response.getAggregations().get("tags"); + assertThat(tags, notNullValue()); + assertThat(tags.getName(), equalTo("tags")); + assertThat(tags.getBuckets().size(), equalTo(2)); + + Iterator iters = tags.getBuckets().iterator(); + + // the max for "1" is 2 + // the max for "0" is 4 + + Terms.Bucket tag = iters.next(); + assertThat(tag, notNullValue()); + assertThat(key(tag), equalTo(asc ? "1" : "0")); + assertThat(tag.getDocCount(), equalTo(asc ? 3l : 2l)); + Filter filter1 = tag.getAggregations().get("filter1"); + assertThat(filter1, notNullValue()); + assertThat(filter1.getDocCount(), equalTo(asc ? 3l : 2l)); + Filter filter2 = filter1.getAggregations().get("filter2"); + assertThat(filter2, notNullValue()); + assertThat(filter2.getDocCount(), equalTo(asc ? 3l : 2l)); + Max max = filter2.getAggregations().get("max"); + assertThat(max, notNullValue()); + assertThat(max.getValue(), equalTo(asc ? 2.0 : 4.0)); + + tag = iters.next(); + assertThat(tag, notNullValue()); + assertThat(key(tag), equalTo(asc ? "0" : "1")); + assertThat(tag.getDocCount(), equalTo(asc ? 2l : 3l)); + filter1 = tag.getAggregations().get("filter1"); + assertThat(filter1, notNullValue()); + assertThat(filter1.getDocCount(), equalTo(asc ? 2l : 3l)); + filter2 = filter1.getAggregations().get("filter2"); + assertThat(filter2, notNullValue()); + assertThat(filter2.getDocCount(), equalTo(asc ? 2l : 3l)); + max = filter2.getAggregations().get("max"); + assertThat(max, notNullValue()); + assertThat(max.getValue(), equalTo(asc ? 4.0 : 2.0)); + } + @Test public void singleValuedField_OrderedByMissingSubAggregation() throws Exception { @@ -685,7 +780,7 @@ public class LongTermsTests extends ElasticsearchIntegrationTest { } @Test - public void singleValuedField_OrderedByNonMetricsSubAggregation() throws Exception { + public void singleValuedField_OrderedByNonMetricsOrMultiBucketSubAggregation() throws Exception { MockBigArrays.discardNextCheck(); try { @@ -693,8 +788,8 @@ public class LongTermsTests extends ElasticsearchIntegrationTest { client().prepareSearch("idx").setTypes("type") .addAggregation(terms("terms") .field(SINGLE_VALUED_FIELD_NAME) - .order(Terms.Order.aggregation("filter", true)) - .subAggregation(filter("filter").filter(FilterBuilders.termFilter("foo", "bar"))) + .order(Terms.Order.aggregation("num_tags", true)) + .subAggregation(terms("num_tags").field("num_tags")) ).execute().actionGet(); fail("Expected search to fail when trying to sort terms aggregation by sug-aggregation which is not of a metrics type"); diff --git a/src/test/java/org/elasticsearch/search/aggregations/bucket/StringTermsTests.java b/src/test/java/org/elasticsearch/search/aggregations/bucket/StringTermsTests.java index e7b87c97906..c2e202a543e 100644 --- a/src/test/java/org/elasticsearch/search/aggregations/bucket/StringTermsTests.java +++ b/src/test/java/org/elasticsearch/search/aggregations/bucket/StringTermsTests.java @@ -41,6 +41,7 @@ import org.junit.Test; import java.util.ArrayList; import java.util.Arrays; +import java.util.Iterator; import java.util.List; import java.util.regex.Pattern; @@ -82,6 +83,7 @@ public class StringTermsTests extends ElasticsearchIntegrationTest { .startObject() .field(SINGLE_VALUED_FIELD_NAME, "val" + i) .field("i", i) + .field("tag", i < lowCardBuilders.length/2 + 1 ? "more" : "less") .startArray(MULTI_VALUED_FIELD_NAME).value("val" + i).value("val" + (i + 1)).endArray() .endObject()); } @@ -833,6 +835,100 @@ public class StringTermsTests extends ElasticsearchIntegrationTest { } } + @Test + public void singleValuedField_OrderedBySingleBucketSubAggregationAsc() throws Exception { + boolean asc = randomBoolean(); + SearchResponse response = client().prepareSearch("idx").setTypes("type") + .addAggregation(terms("tags") + .executionHint(randomExecutionHint()) + .field("tag") + .order(Terms.Order.aggregation("filter", asc)) + .subAggregation(filter("filter").filter(FilterBuilders.matchAllFilter())) + ).execute().actionGet(); + + + assertSearchResponse(response); + + Terms tags = response.getAggregations().get("tags"); + assertThat(tags, notNullValue()); + assertThat(tags.getName(), equalTo("tags")); + assertThat(tags.getBuckets().size(), equalTo(2)); + + Iterator iters = tags.getBuckets().iterator(); + + Terms.Bucket tag = iters.next(); + assertThat(tag, notNullValue()); + assertThat(key(tag), equalTo(asc ? "less" : "more")); + assertThat(tag.getDocCount(), equalTo(asc ? 2l : 3l)); + Filter filter = tag.getAggregations().get("filter"); + assertThat(filter, notNullValue()); + assertThat(filter.getDocCount(), equalTo(asc ? 2l : 3l)); + + tag = iters.next(); + assertThat(tag, notNullValue()); + assertThat(key(tag), equalTo(asc ? "more" : "less")); + assertThat(tag.getDocCount(), equalTo(asc ? 3l : 2l)); + filter = tag.getAggregations().get("filter"); + assertThat(filter, notNullValue()); + assertThat(filter.getDocCount(), equalTo(asc ? 3l : 2l)); + } + + @Test + public void singleValuedField_OrderedBySubAggregationAsc_MultiHierarchyLevels() throws Exception { + boolean asc = randomBoolean(); + SearchResponse response = client().prepareSearch("idx").setTypes("type") + .addAggregation(terms("tags") + .executionHint(randomExecutionHint()) + .field("tag") + .order(Terms.Order.aggregation("filter1>filter2>stats.max", asc)) + .subAggregation(filter("filter1").filter(FilterBuilders.matchAllFilter()) + .subAggregation(filter("filter2").filter(FilterBuilders.matchAllFilter()) + .subAggregation(stats("stats").field("i")))) + ).execute().actionGet(); + + + assertSearchResponse(response); + + Terms tags = response.getAggregations().get("tags"); + assertThat(tags, notNullValue()); + assertThat(tags.getName(), equalTo("tags")); + assertThat(tags.getBuckets().size(), equalTo(2)); + + Iterator iters = tags.getBuckets().iterator(); + + // the max for "more" is 2 + // the max for "less" is 4 + + Terms.Bucket tag = iters.next(); + assertThat(tag, notNullValue()); + assertThat(key(tag), equalTo(asc ? "more" : "less")); + assertThat(tag.getDocCount(), equalTo(asc ? 3l : 2l)); + Filter filter1 = tag.getAggregations().get("filter1"); + assertThat(filter1, notNullValue()); + assertThat(filter1.getDocCount(), equalTo(asc ? 3l : 2l)); + Filter filter2 = filter1.getAggregations().get("filter2"); + assertThat(filter2, notNullValue()); + assertThat(filter2.getDocCount(), equalTo(asc ? 3l : 2l)); + Stats stats = filter2.getAggregations().get("stats"); + assertThat(stats, notNullValue()); + assertThat(stats.getMax(), equalTo(asc ? 2.0 : 4.0)); + + tag = iters.next(); + assertThat(tag, notNullValue()); + assertThat(key(tag), equalTo(asc ? "less" : "more")); + assertThat(tag.getDocCount(), equalTo(asc ? 2l : 3l)); + filter1 = tag.getAggregations().get("filter1"); + assertThat(filter1, notNullValue()); + assertThat(filter1.getDocCount(), equalTo(asc ? 2l : 3l)); + filter2 = filter1.getAggregations().get("filter2"); + assertThat(filter2, notNullValue()); + assertThat(filter2.getDocCount(), equalTo(asc ? 2l : 3l)); + stats = filter2.getAggregations().get("stats"); + assertThat(stats, notNullValue()); + assertThat(stats.getMax(), equalTo(asc ? 4.0 : 2.0)); + } + + @Test public void singleValuedField_OrderedByMissingSubAggregation() throws Exception { @@ -854,7 +950,7 @@ public class StringTermsTests extends ElasticsearchIntegrationTest { } @Test - public void singleValuedField_OrderedByNonMetricsSubAggregation() throws Exception { + public void singleValuedField_OrderedByNonMetricsOrMultiBucketSubAggregation() throws Exception { MockBigArrays.discardNextCheck(); try { @@ -863,11 +959,11 @@ public class StringTermsTests extends ElasticsearchIntegrationTest { .addAggregation(terms("terms") .executionHint(randomExecutionHint()) .field(SINGLE_VALUED_FIELD_NAME) - .order(Terms.Order.aggregation("filter", true)) - .subAggregation(filter("filter").filter(FilterBuilders.termFilter("foo", "bar"))) + .order(Terms.Order.aggregation("values", true)) + .subAggregation(terms("values").field("i")) ).execute().actionGet(); - fail("Expected search to fail when trying to sort terms aggregation by sug-aggregation which is not of a metrics type"); + fail("Expected search to fail when trying to sort terms aggregation by sug-aggregation which is not of a metrics or single-bucket type"); } catch (ElasticsearchException e) { // expected @@ -879,7 +975,6 @@ public class StringTermsTests extends ElasticsearchIntegrationTest { MockBigArrays.discardNextCheck(); try { - client().prepareSearch("idx").setTypes("type") .addAggregation(terms("terms") .executionHint(randomExecutionHint()) diff --git a/src/test/java/org/elasticsearch/search/aggregations/support/PathTests.java b/src/test/java/org/elasticsearch/search/aggregations/support/PathTests.java new file mode 100644 index 00000000000..d65ae279823 --- /dev/null +++ b/src/test/java/org/elasticsearch/search/aggregations/support/PathTests.java @@ -0,0 +1,109 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.search.aggregations.support; + +import org.elasticsearch.search.aggregations.AggregationExecutionException; +import org.junit.Test; + +import java.util.ArrayList; +import java.util.List; + +import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.fail; + +/** + * + */ +public class PathTests { + + @Test + public void testInvalidPaths() throws Exception { + assertInvalidPath("[foo]", "brackets at the beginning of the token expression"); + assertInvalidPath("foo[bar", "open brackets without closing at the token expression"); + assertInvalidPath("foo[", "open bracket at the end of the token expression"); + assertInvalidPath("foo[]", "empty brackets in the token expression"); + assertInvalidPath("foo[bar]baz", "brackets not enclosing at the end of the token expression"); + assertInvalidPath(".foo", "dot separator at the beginning of the token expression"); + assertInvalidPath("foo.", "dot separator at the end of the token expression"); + } + + @Test + public void testValidPaths() throws Exception { + assertValidPath("foo>bar", tokens().add("foo").add("bar")); + assertValidPath("foo.bar", tokens().add("foo", "bar")); + assertValidPath("foo[bar]", tokens().add("foo", "bar")); + assertValidPath("foo[bar]>baz", tokens().add("foo", "bar").add("baz")); + assertValidPath("foo[bar]>baz[qux]", tokens().add("foo", "bar").add("baz", "qux")); + assertValidPath("foo[bar]>baz.qux", tokens().add("foo", "bar").add("baz", "qux")); + assertValidPath("foo.bar>baz.qux", tokens().add("foo", "bar").add("baz", "qux")); + assertValidPath("foo.bar>baz[qux]", tokens().add("foo", "bar").add("baz", "qux")); + } + + private void assertInvalidPath(String path, String reason) { + try { + OrderPath.parse(path); + fail("Expected parsing path [" + path + "] to fail - " + reason); + } catch (AggregationExecutionException aee) { + // expected + } + } + + private void assertValidPath(String path, Tokens tokenz) { + OrderPath.Token[] tokens = tokenz.toArray(); + OrderPath p = OrderPath.parse(path); + assertThat(p.tokens.length, equalTo(tokens.length)); + for (int i = 0; i < p.tokens.length; i++) { + OrderPath.Token t1 = p.tokens[i]; + OrderPath.Token t2 = tokens[i]; + assertThat(t1, equalTo(t2)); + } + } + + private static Tokens tokens() { + return new Tokens(); + } + + private static class Tokens { + + private List tokens = new ArrayList(); + + Tokens add(String name) { + tokens.add(new OrderPath.Token(name, name, null)); + return this; + } + + Tokens add(String name, String key) { + if (Math.random() > 0.5) { + tokens.add(new OrderPath.Token(name + "." + key, name, key)); + } else { + tokens.add(new OrderPath.Token(name + "[" + key + "]", name, key)); + } + return this; + } + + OrderPath.Token[] toArray() { + return tokens.toArray(new OrderPath.Token[tokens.size()]); + } + + + } + +}