Skip to content

Commit 319ca82

Browse files
colings86colings86
committed
Improving parsing of sigma param for Extended Stats Bucket Aggregatio
Improving parsing of sigma param for Extended Stats Bucket Aggregation
2 parents 557fc8b + 93d1b38 commit 319ca82

File tree

10 files changed

+121
-75
lines changed

10 files changed

+121
-75
lines changed

‎core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/BucketMetricsParser.java

+23-22
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator;
2828

2929
import java.io.IOException;
30-
import java.text.ParseException;
3130
import java.util.ArrayList;
3231
import java.util.HashMap;
3332
import java.util.List;
@@ -53,7 +52,7 @@ public final BucketMetricsPipelineAggregatorBuilder<?> parse(String pipelineAggr
5352
String[] bucketsPaths = null;
5453
String format = null;
5554
GapPolicy gapPolicy = null;
56-
Map<String, Object> leftover = new HashMap<>(5);
55+
Map<String, Object> params = new HashMap<>(5);
5756

5857
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
5958
if (token == XContentParser.Token.FIELD_NAME) {
@@ -66,7 +65,7 @@ public final BucketMetricsPipelineAggregatorBuilder<?> parse(String pipelineAggr
6665
} else if (context.getParseFieldMatcher().match(currentFieldName, GAP_POLICY)) {
6766
gapPolicy = GapPolicy.parse(context, parser.text(), parser.getTokenLocation());
6867
} else {
69-
leftover.put(currentFieldName, parser.text());
68+
parseToken(pipelineAggregatorName, parser, context, currentFieldName, token, params);
7069
}
7170
} else if (token == XContentParser.Token.START_ARRAY) {
7271
if (context.getParseFieldMatcher().match(currentFieldName, BUCKETS_PATH)) {
@@ -77,10 +76,10 @@ public final BucketMetricsPipelineAggregatorBuilder<?> parse(String pipelineAggr
7776
}
7877
bucketsPaths = paths.toArray(new String[paths.size()]);
7978
} else {
80-
leftover.put(currentFieldName, parser.list());
79+
parseToken(pipelineAggregatorName, parser, context, currentFieldName, token, params);
8180
}
8281
} else {
83-
leftover.put(currentFieldName, parser.objectText());
82+
parseToken(pipelineAggregatorName, parser, context, currentFieldName, token, params);
8483
}
8584
}
8685

@@ -89,30 +88,32 @@ public final BucketMetricsPipelineAggregatorBuilder<?> parse(String pipelineAggr
8988
"Missing required field [" + BUCKETS_PATH.getPreferredName() + "] for aggregation [" + pipelineAggregatorName + "]");
9089
}
9190

92-
BucketMetricsPipelineAggregatorBuilder<?> factory = null;
93-
try {
94-
factory = buildFactory(pipelineAggregatorName, bucketsPaths[0], leftover);
95-
if (format != null) {
96-
factory.format(format);
97-
}
98-
if (gapPolicy != null) {
99-
factory.gapPolicy(gapPolicy);
100-
}
101-
} catch (ParseException exception) {
102-
throw new ParsingException(parser.getTokenLocation(),
103-
"Could not parse settings for aggregation [" + pipelineAggregatorName + "].", exception);
91+
BucketMetricsPipelineAggregatorBuilder<?> factory = buildFactory(pipelineAggregatorName, bucketsPaths[0], params);
92+
if (format != null) {
93+
factory.format(format);
10494
}
105-
106-
if (leftover.size() > 0) {
107-
throw new ParsingException(parser.getTokenLocation(),
108-
"Unexpected tokens " + leftover.keySet() + " in [" + pipelineAggregatorName + "].");
95+
if (gapPolicy != null) {
96+
factory.gapPolicy(gapPolicy);
10997
}
98+
11099
assert(factory != null);
111100

112101
return factory;
113102
}
114103

115104
protected abstract BucketMetricsPipelineAggregatorBuilder<?> buildFactory(String pipelineAggregatorName, String bucketsPaths,
116-
Map<String, Object> unparsedParams) throws ParseException;
105+
Map<String, Object> params);
117106

107+
protected boolean token(XContentParser parser, QueryParseContext context, String field,
108+
XContentParser.Token token, Map<String, Object> params) throws IOException {
109+
return false;
110+
}
111+
112+
private void parseToken(String aggregationName, XContentParser parser, QueryParseContext context, String currentFieldName,
113+
XContentParser.Token currentToken, Map<String, Object> params) throws IOException {
114+
if (token(parser, context, currentFieldName, currentToken, params) == false) {
115+
throw new ParsingException(parser.getTokenLocation(),
116+
"Unexpected token " + currentToken + " [" + currentFieldName + "] in [" + aggregationName + "]");
117+
}
118+
}
118119
}

‎core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/avg/AvgBucketPipelineAggregatorBuilder.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ protected XContentBuilder doXContentBody(XContentBuilder builder, Params params)
7575
public static final PipelineAggregator.Parser PARSER = new BucketMetricsParser() {
7676
@Override
7777
protected AvgBucketPipelineAggregatorBuilder buildFactory(String pipelineAggregatorName,
78-
String bucketsPath, Map<String, Object> unparsedParams) {
78+
String bucketsPath, Map<String, Object> params) {
7979
return new AvgBucketPipelineAggregatorBuilder(pipelineAggregatorName, bucketsPath);
8080
}
8181
};
@@ -94,4 +94,4 @@ protected boolean innerEquals(BucketMetricsPipelineAggregatorBuilder<AvgBucketPi
9494
public String getWriteableName() {
9595
return NAME;
9696
}
97-
}
97+
}

‎core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/max/MaxBucketPipelineAggregatorBuilder.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ protected XContentBuilder doXContentBody(XContentBuilder builder, Params params)
7575
public static final PipelineAggregator.Parser PARSER = new BucketMetricsParser() {
7676
@Override
7777
protected MaxBucketPipelineAggregatorBuilder buildFactory(String pipelineAggregatorName,
78-
String bucketsPath, Map<String, Object> unparsedParams) {
78+
String bucketsPath, Map<String, Object> params) {
7979
return new MaxBucketPipelineAggregatorBuilder(pipelineAggregatorName, bucketsPath);
8080
}
8181
};
@@ -94,4 +94,4 @@ protected boolean innerEquals(BucketMetricsPipelineAggregatorBuilder<MaxBucketPi
9494
public String getWriteableName() {
9595
return NAME;
9696
}
97-
}
97+
}

‎core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/min/MinBucketPipelineAggregatorBuilder.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ protected XContentBuilder doXContentBody(XContentBuilder builder, Params params)
7575
public static final PipelineAggregator.Parser PARSER = new BucketMetricsParser() {
7676
@Override
7777
protected MinBucketPipelineAggregatorBuilder buildFactory(String pipelineAggregatorName,
78-
String bucketsPath, Map<String, Object> unparsedParams) {
78+
String bucketsPath, Map<String, Object> params) {
7979
return new MinBucketPipelineAggregatorBuilder(pipelineAggregatorName, bucketsPath);
8080
}
8181
};
@@ -94,4 +94,4 @@ protected boolean innerEquals(BucketMetricsPipelineAggregatorBuilder<MinBucketPi
9494
public String getWriteableName() {
9595
return NAME;
9696
}
97-
}
97+
}

‎core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/percentile/PercentilesBucketPipelineAggregatorBuilder.java

+25-27
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,13 @@
1919

2020
package org.elasticsearch.search.aggregations.pipeline.bucketmetrics.percentile;
2121

22+
import com.carrotsearch.hppc.DoubleArrayList;
2223
import org.elasticsearch.common.ParseField;
2324
import org.elasticsearch.common.io.stream.StreamInput;
2425
import org.elasticsearch.common.io.stream.StreamOutput;
2526
import org.elasticsearch.common.xcontent.XContentBuilder;
27+
import org.elasticsearch.common.xcontent.XContentParser;
28+
import org.elasticsearch.index.query.QueryParseContext;
2629
import org.elasticsearch.search.aggregations.AggregatorFactory;
2730
import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator;
2831
import org.elasticsearch.search.aggregations.pipeline.PipelineAggregatorBuilder;
@@ -117,41 +120,36 @@ protected XContentBuilder doXContentBody(XContentBuilder builder, Params params)
117120
}
118121

119122
public static final PipelineAggregator.Parser PARSER = new BucketMetricsParser() {
123+
120124
@Override
121125
protected PercentilesBucketPipelineAggregatorBuilder buildFactory(String pipelineAggregatorName,
122-
String bucketsPath, Map<String, Object> unparsedParams) throws ParseException {
123-
double[] percents = null;
124-
int counter = 0;
125-
Object percentParam = unparsedParams.get(PERCENTS_FIELD.getPreferredName());
126-
127-
if (percentParam != null) {
128-
if (percentParam instanceof List) {
129-
percents = new double[((List<?>) percentParam).size()];
130-
for (Object p : (List<?>) percentParam) {
131-
if (p instanceof Double) {
132-
percents[counter] = (Double) p;
133-
counter += 1;
134-
} else {
135-
throw new ParseException(
136-
"Parameter [" + PERCENTS_FIELD.getPreferredName() + "] must be an array of doubles, type `"
137-
+ percentParam.getClass().getSimpleName() + "` provided instead",
138-
0);
139-
}
140-
}
141-
unparsedParams.remove(PERCENTS_FIELD.getPreferredName());
142-
} else {
143-
throw new ParseException("Parameter [" + PERCENTS_FIELD.getPreferredName() + "] must be an array of doubles, type `"
144-
+ percentParam.getClass().getSimpleName() + "` provided instead", 0);
145-
}
146-
}
126+
String bucketsPath, Map<String, Object> params) {
147127

148128
PercentilesBucketPipelineAggregatorBuilder factory = new
149-
PercentilesBucketPipelineAggregatorBuilder(pipelineAggregatorName, bucketsPath);
129+
PercentilesBucketPipelineAggregatorBuilder(pipelineAggregatorName, bucketsPath);
130+
131+
double[] percents = (double[]) params.get(PERCENTS_FIELD.getPreferredName());
150132
if (percents != null) {
151133
factory.percents(percents);
152134
}
135+
153136
return factory;
154137
}
138+
139+
@Override
140+
protected boolean token(XContentParser parser, QueryParseContext context, String field,
141+
XContentParser.Token token, Map<String, Object> params) throws IOException {
142+
if (context.getParseFieldMatcher().match(field, PERCENTS_FIELD) && token == XContentParser.Token.START_ARRAY) {
143+
DoubleArrayList percents = new DoubleArrayList(10);
144+
while (parser.nextToken() != XContentParser.Token.END_ARRAY) {
145+
percents.add(parser.doubleValue());
146+
}
147+
params.put(PERCENTS_FIELD.getPreferredName(), percents.toArray());
148+
return true;
149+
}
150+
return false;
151+
}
152+
155153
};
156154

157155
@Override
@@ -169,4 +167,4 @@ protected boolean innerEquals(BucketMetricsPipelineAggregatorBuilder<Percentiles
169167
public String getWriteableName() {
170168
return NAME;
171169
}
172-
}
170+
}

‎core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/stats/StatsBucketPipelineAggregatorBuilder.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ protected XContentBuilder doXContentBody(XContentBuilder builder, Params params)
7777
public static final PipelineAggregator.Parser PARSER = new BucketMetricsParser() {
7878
@Override
7979
protected StatsBucketPipelineAggregatorBuilder buildFactory(String pipelineAggregatorName,
80-
String bucketsPath, Map<String, Object> unparsedParams) {
80+
String bucketsPath, Map<String, Object> params) {
8181
return new StatsBucketPipelineAggregatorBuilder(pipelineAggregatorName, bucketsPath);
8282
}
8383
};
@@ -96,4 +96,4 @@ protected boolean innerEquals(BucketMetricsPipelineAggregatorBuilder<StatsBucket
9696
public String getWriteableName() {
9797
return NAME;
9898
}
99-
}
99+
}

‎core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/stats/extended/ExtendedStatsBucketParser.java

+17-16
Original file line numberDiff line numberDiff line change
@@ -20,35 +20,36 @@
2020
package org.elasticsearch.search.aggregations.pipeline.bucketmetrics.stats.extended;
2121

2222
import org.elasticsearch.common.ParseField;
23+
import org.elasticsearch.common.xcontent.XContentParser;
24+
import org.elasticsearch.index.query.QueryParseContext;
2325
import org.elasticsearch.search.aggregations.pipeline.bucketmetrics.BucketMetricsParser;
2426

25-
import java.text.ParseException;
27+
import java.io.IOException;
2628
import java.util.Map;
2729

2830
public class ExtendedStatsBucketParser extends BucketMetricsParser {
2931
static final ParseField SIGMA = new ParseField("sigma");
3032

3133
@Override
3234
protected ExtendedStatsBucketPipelineAggregatorBuilder buildFactory(String pipelineAggregatorName,
33-
String bucketsPath, Map<String, Object> unparsedParams) throws ParseException {
34-
35-
Double sigma = null;
36-
Object param = unparsedParams.get(SIGMA.getPreferredName());
37-
38-
if (param != null) {
39-
if (param instanceof Double) {
40-
sigma = (Double) param;
41-
unparsedParams.remove(SIGMA.getPreferredName());
42-
} else {
43-
throw new ParseException("Parameter [" + SIGMA.getPreferredName() + "] must be a Double, type `"
44-
+ param.getClass().getSimpleName() + "` provided instead", 0);
45-
}
46-
}
35+
String bucketsPath, Map<String, Object> params) {
4736
ExtendedStatsBucketPipelineAggregatorBuilder factory =
48-
new ExtendedStatsBucketPipelineAggregatorBuilder(pipelineAggregatorName, bucketsPath);
37+
new ExtendedStatsBucketPipelineAggregatorBuilder(pipelineAggregatorName, bucketsPath);
38+
Double sigma = (Double) params.get(SIGMA.getPreferredName());
4939
if (sigma != null) {
5040
factory.sigma(sigma);
5141
}
42+
5243
return factory;
5344
}
45+
46+
@Override
47+
protected boolean token(XContentParser parser, QueryParseContext context, String field,
48+
XContentParser.Token token, Map<String, Object> params) throws IOException {
49+
if (context.getParseFieldMatcher().match(field, SIGMA) && token == XContentParser.Token.VALUE_NUMBER) {
50+
params.put(SIGMA.getPreferredName(), parser.doubleValue());
51+
return true;
52+
}
53+
return false;
54+
}
5455
}

‎core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/sum/SumBucketPipelineAggregatorBuilder.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ protected XContentBuilder doXContentBody(XContentBuilder builder, Params params)
7575
public static final PipelineAggregator.Parser PARSER = new BucketMetricsParser() {
7676
@Override
7777
protected SumBucketPipelineAggregatorBuilder buildFactory(String pipelineAggregatorName,
78-
String bucketsPath, Map<String, Object> unparsedParams) {
78+
String bucketsPath, Map<String, Object> params) {
7979
return new SumBucketPipelineAggregatorBuilder(pipelineAggregatorName, bucketsPath);
8080
}
8181
};
@@ -94,4 +94,4 @@ protected boolean innerEquals(BucketMetricsPipelineAggregatorBuilder<SumBucketPi
9494
public String getWriteableName() {
9595
return NAME;
9696
}
97-
}
97+
}

‎core/src/test/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/ExtendedStatsBucketTests.java

+23
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,14 @@
1919

2020
package org.elasticsearch.search.aggregations.pipeline.bucketmetrics;
2121

22+
import org.elasticsearch.common.xcontent.XContentFactory;
23+
import org.elasticsearch.common.xcontent.XContentParser;
24+
import org.elasticsearch.index.query.QueryParseContext;
25+
import org.elasticsearch.search.aggregations.pipeline.bucketmetrics.stats.extended.ExtendedStatsBucketPipelineAggregator;
2226
import org.elasticsearch.search.aggregations.pipeline.bucketmetrics.stats.extended.ExtendedStatsBucketPipelineAggregatorBuilder;
2327

28+
import static org.hamcrest.Matchers.equalTo;
29+
2430
public class ExtendedStatsBucketTests extends AbstractBucketMetricsTestCase<ExtendedStatsBucketPipelineAggregatorBuilder> {
2531

2632
@Override
@@ -32,5 +38,22 @@ protected ExtendedStatsBucketPipelineAggregatorBuilder doCreateTestAggregatorFac
3238
return factory;
3339
}
3440

41+
public void testSigmaFromInt() throws Exception {
42+
String content = XContentFactory.jsonBuilder()
43+
.startObject()
44+
.field("sigma", 5)
45+
.field("buckets_path", "test")
46+
.endObject()
47+
.string();
48+
49+
XContentParser parser = XContentFactory.xContent(content).createParser(content);
50+
QueryParseContext parseContext = new QueryParseContext(queriesRegistry, parser, parseFieldMatcher);
51+
parser.nextToken(); // skip object start
3552

53+
ExtendedStatsBucketPipelineAggregatorBuilder builder = (ExtendedStatsBucketPipelineAggregatorBuilder) aggParsers
54+
.pipelineParser(ExtendedStatsBucketPipelineAggregator.TYPE.name(), parseFieldMatcher)
55+
.parse("test", parseContext);
56+
57+
assertThat(builder.sigma(), equalTo(5.0));
58+
}
3659
}

‎core/src/test/java/org/elasticsearch/search/aggregations/pipeline/bucketmetrics/PercentilesBucketTests.java

+23
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,14 @@
1919

2020
package org.elasticsearch.search.aggregations.pipeline.bucketmetrics;
2121

22+
import org.elasticsearch.common.xcontent.XContentFactory;
23+
import org.elasticsearch.common.xcontent.XContentParser;
24+
import org.elasticsearch.index.query.QueryParseContext;
25+
import org.elasticsearch.search.aggregations.pipeline.bucketmetrics.percentile.PercentilesBucketPipelineAggregator;
2226
import org.elasticsearch.search.aggregations.pipeline.bucketmetrics.percentile.PercentilesBucketPipelineAggregatorBuilder;
2327

28+
import static org.hamcrest.Matchers.equalTo;
29+
2430
public class PercentilesBucketTests extends AbstractBucketMetricsTestCase<PercentilesBucketPipelineAggregatorBuilder> {
2531

2632
@Override
@@ -37,5 +43,22 @@ protected PercentilesBucketPipelineAggregatorBuilder doCreateTestAggregatorFacto
3743
return factory;
3844
}
3945

46+
public void testPercentsFromMixedArray() throws Exception {
47+
String content = XContentFactory.jsonBuilder()
48+
.startObject()
49+
.field("buckets_path", "test")
50+
.array("percents", 0, 20.0, 50, 75.99)
51+
.endObject()
52+
.string();
53+
54+
XContentParser parser = XContentFactory.xContent(content).createParser(content);
55+
QueryParseContext parseContext = new QueryParseContext(queriesRegistry, parser, parseFieldMatcher);
56+
parser.nextToken(); // skip object start
4057

58+
PercentilesBucketPipelineAggregatorBuilder builder = (PercentilesBucketPipelineAggregatorBuilder) aggParsers
59+
.pipelineParser(PercentilesBucketPipelineAggregator.TYPE.name(), parseFieldMatcher)
60+
.parse("test", parseContext);
61+
62+
assertThat(builder.percents(), equalTo(new double[]{0.0, 20.0, 50.0, 75.99}));
63+
}
4164
}

0 commit comments

Comments
 (0)