honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Aggregations: Better JSON output scoping 

Before this change each aggregation had to output an object field with its name and write its JSON inside that object.  This allowed for badly behaved aggregations which could write JSON content in the root of the 'aggs' object.  this change move the writing of the aggregation name to a level above the aggregation itself, ensuring that aggregations can only write within there own scope in the JSON output.

Closes #7004
This commit is contained in:
Colin Goodheart-Smithe 2014-07-23 15:14:46 +01:00
parent d8cd755445
commit fdf2bb9371
23 changed files with 44 additions and 70 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
src/main/java/org/elasticsearch/search/aggregations/InternalAggregation.java
View File

@ -25,6 +25,7 @@ import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.io.stream.Streamable;
import org.elasticsearch.common.util.BigArrays;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentBuilderString;
import java.io.IOException;
@ -150,6 +151,16 @@ public abstract class InternalAggregation implements Aggregation, ToXContent, St
}
out.writeVInt(size);
}
@Override
public final XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject(name);
doXContentBody(builder, params);
builder.endObject();
return builder;
}
public abstract XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException;
/**
* Common xcontent fields that are shared among addAggregation

5
src/main/java/org/elasticsearch/search/aggregations/bucket/InternalSingleBucketAggregation.java
View File

@ -95,10 +95,9 @@ public abstract class InternalSingleBucketAggregation extends InternalAggregatio
}
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject(name);
public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
builder.field(CommonFields.DOC_COUNT, docCount);
aggregations.toXContentInternal(builder, params);
return builder.endObject();
return builder;
}
}

4
src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/InternalGeoHashGrid.java
View File

@ -228,8 +228,7 @@ public class InternalGeoHashGrid extends InternalAggregation implements GeoHashG
}
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject(name);
public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
builder.startArray(CommonFields.BUCKETS);
for (Bucket bucket : buckets) {
builder.startObject();
@ -239,7 +238,6 @@ public class InternalGeoHashGrid extends InternalAggregation implements GeoHashG
builder.endObject();
}
builder.endArray();
builder.endObject();
return builder;
}

5
src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/InternalHistogram.java
View File

@ -391,8 +391,7 @@ public class InternalHistogram<B extends InternalHistogram.Bucket> extends Inter
}
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject(name);
public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
if (keyed) {
builder.startObject(CommonFields.BUCKETS);
} else {
@ -406,7 +405,7 @@ public class InternalHistogram<B extends InternalHistogram.Bucket> extends Inter
} else {
builder.endArray();
}
return builder.endObject();
return builder;
}
}

5
src/main/java/org/elasticsearch/search/aggregations/bucket/range/InternalRange.java
View File

@ -263,8 +263,7 @@ public class InternalRange<B extends InternalRange.Bucket> extends InternalAggre
}
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject(name);
public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
if (keyed) {
builder.startObject(CommonFields.BUCKETS);
} else {
@ -278,7 +277,7 @@ public class InternalRange<B extends InternalRange.Bucket> extends InternalAggre
} else {
builder.endArray();
}
return builder.endObject();
return builder;
}
}

7
src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantLongTerms.java
View File

@ -27,7 +27,8 @@ import org.elasticsearch.common.text.Text;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.search.aggregations.AggregationStreams;
import org.elasticsearch.search.aggregations.InternalAggregations;
import org.elasticsearch.search.aggregations.bucket.significant.heuristics.*;
import org.elasticsearch.search.aggregations.bucket.significant.heuristics.SignificanceHeuristic;
import org.elasticsearch.search.aggregations.bucket.significant.heuristics.SignificanceHeuristicStreams;
import org.elasticsearch.search.aggregations.support.format.ValueFormatter;
import org.elasticsearch.search.aggregations.support.format.ValueFormatterStreams;
@ -160,8 +161,7 @@ public class SignificantLongTerms extends InternalSignificantTerms {
}
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject(name);
public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
builder.field("doc_count", subsetSize);
builder.startArray(CommonFields.BUCKETS);
for (InternalSignificantTerms.Bucket bucket : buckets) {
@ -177,7 +177,6 @@ public class SignificantLongTerms extends InternalSignificantTerms {
builder.endObject();
}
builder.endArray();
builder.endObject();
return builder;
}

7
src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantStringTerms.java
View File

@ -29,7 +29,8 @@ import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.search.aggregations.AggregationStreams;
import org.elasticsearch.search.aggregations.InternalAggregation;
import org.elasticsearch.search.aggregations.InternalAggregations;
import org.elasticsearch.search.aggregations.bucket.significant.heuristics.*;
import org.elasticsearch.search.aggregations.bucket.significant.heuristics.SignificanceHeuristic;
import org.elasticsearch.search.aggregations.bucket.significant.heuristics.SignificanceHeuristicStreams;
import java.io.IOException;
import java.util.ArrayList;
@ -153,8 +154,7 @@ public class SignificantStringTerms extends InternalSignificantTerms {
}
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject(name);
public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
builder.field("doc_count", subsetSize);
builder.startArray(CommonFields.BUCKETS);
for (InternalSignificantTerms.Bucket bucket : buckets) {
@ -171,7 +171,6 @@ public class SignificantStringTerms extends InternalSignificantTerms {
}
}
builder.endArray();
builder.endObject();
return builder;
}

4
src/main/java/org/elasticsearch/search/aggregations/bucket/significant/UnmappedSignificantTerms.java
View File

@ -99,10 +99,8 @@ public class UnmappedSignificantTerms extends InternalSignificantTerms {
}
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject(name);
public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
builder.startArray(CommonFields.BUCKETS).endArray();
builder.endObject();
return builder;
}

4
src/main/java/org/elasticsearch/search/aggregations/bucket/terms/DoubleTerms.java
View File

@ -145,8 +145,7 @@ public class DoubleTerms extends InternalTerms {
}
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject(name);
public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
builder.startArray(CommonFields.BUCKETS);
for (InternalTerms.Bucket bucket : buckets) {
builder.startObject();
@ -159,7 +158,6 @@ public class DoubleTerms extends InternalTerms {
builder.endObject();
}
builder.endArray();
builder.endObject();
return builder;
}

4
src/main/java/org/elasticsearch/search/aggregations/bucket/terms/LongTerms.java
View File

@ -146,8 +146,7 @@ public class LongTerms extends InternalTerms {
}
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject(name);
public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
builder.startArray(CommonFields.BUCKETS);
for (InternalTerms.Bucket bucket : buckets) {
builder.startObject();
@ -160,7 +159,6 @@ public class LongTerms extends InternalTerms {
builder.endObject();
}
builder.endArray();
builder.endObject();
return builder;
}

4
src/main/java/org/elasticsearch/search/aggregations/bucket/terms/StringTerms.java
View File

@ -142,8 +142,7 @@ public class StringTerms extends InternalTerms {
}
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject(name);
public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
builder.startArray(CommonFields.BUCKETS);
for (InternalTerms.Bucket bucket : buckets) {
builder.startObject();
@ -153,7 +152,6 @@ public class StringTerms extends InternalTerms {
builder.endObject();
}
builder.endArray();
builder.endObject();
return builder;
}

4
src/main/java/org/elasticsearch/search/aggregations/bucket/terms/UnmappedTerms.java
View File

@ -98,10 +98,8 @@ public class UnmappedTerms extends InternalTerms {
}
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject(name);
public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
builder.startArray(CommonFields.BUCKETS).endArray();
builder.endObject();
return builder;
}

4
src/main/java/org/elasticsearch/search/aggregations/metrics/avg/InternalAvg.java
View File

@ -101,13 +101,11 @@ public class InternalAvg extends InternalNumericMetricsAggregation.SingleValue i
}
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject(name);
public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
builder.field(CommonFields.VALUE, count != 0 ? getValue() : null);
if (count != 0 && valueFormatter != null) {
builder.field(CommonFields.VALUE_AS_STRING, valueFormatter.format(getValue()));
}
builder.endObject();
return builder;
}

4
src/main/java/org/elasticsearch/search/aggregations/metrics/cardinality/InternalCardinality.java
View File

@ -123,14 +123,12 @@ public final class InternalCardinality extends InternalNumericMetricsAggregation
}
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject(name);
public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
final long cardinality = getValue();
builder.field(CommonFields.VALUE, cardinality);
if (valueFormatter != null) {
builder.field(CommonFields.VALUE_AS_STRING, valueFormatter.format(cardinality));
}
builder.endObject();
return builder;
}

5
src/main/java/org/elasticsearch/search/aggregations/metrics/geobounds/InternalGeoBounds.java
View File

@ -104,7 +104,7 @@ public class InternalGeoBounds extends InternalMetricsAggregation implements Geo
}
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
GeoPoint topLeft = topLeft();
GeoPoint bottomRight = bottomRight();
if (topLeft != null) {
@ -117,8 +117,9 @@ public class InternalGeoBounds extends InternalMetricsAggregation implements Geo
builder.field("lat", bottomRight.lat());
builder.field("lon", bottomRight.lon());
builder.endObject();
builder.endObject();
}
return builder.endObject();
return builder;
}
@Override

4
src/main/java/org/elasticsearch/search/aggregations/metrics/max/InternalMax.java
View File

@ -95,14 +95,12 @@ public class InternalMax extends InternalNumericMetricsAggregation.SingleValue i
}
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject(name);
public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
boolean hasValue = !Double.isInfinite(max);
builder.field(CommonFields.VALUE, hasValue ? max : null);
if (hasValue && valueFormatter != null) {
builder.field(CommonFields.VALUE_AS_STRING, valueFormatter.format(max));
}
builder.endObject();
return builder;
}
}

4
src/main/java/org/elasticsearch/search/aggregations/metrics/min/InternalMin.java
View File

@ -96,14 +96,12 @@ public class InternalMin extends InternalNumericMetricsAggregation.SingleValue i
}
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject(name);
public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
boolean hasValue = !Double.isInfinite(min);
builder.field(CommonFields.VALUE, hasValue ? min : null);
if (hasValue && valueFormatter != null) {
builder.field(CommonFields.VALUE_AS_STRING, valueFormatter.format(min));
}
builder.endObject();
return builder;
}

4
src/main/java/org/elasticsearch/search/aggregations/metrics/percentiles/AbstractInternalPercentiles.java
View File

@ -104,8 +104,7 @@ abstract class AbstractInternalPercentiles extends InternalNumericMetricsAggrega
}
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject(name);
public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
if (keyed) {
builder.startObject(CommonFields.VALUES);
for(int i = 0; i < keys.length; ++i) {
@ -131,7 +130,6 @@ abstract class AbstractInternalPercentiles extends InternalNumericMetricsAggrega
}
builder.endArray();
}
builder.endObject();
return builder;
}
}

4
src/main/java/org/elasticsearch/search/aggregations/metrics/stats/InternalStats.java
View File

@ -174,8 +174,7 @@ public class InternalStats extends InternalNumericMetricsAggregation.MultiValue
}
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject(name);
public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
builder.field(Fields.COUNT, count);
builder.field(Fields.MIN, count != 0 ? min : null);
builder.field(Fields.MAX, count != 0 ? max : null);
@ -188,7 +187,6 @@ public class InternalStats extends InternalNumericMetricsAggregation.MultiValue
builder.field(Fields.SUM_AS_STRING, valueFormatter.format(sum));
}
otherStatsToXCotent(builder, params);
builder.endObject();
return builder;
}

4
src/main/java/org/elasticsearch/search/aggregations/metrics/sum/InternalSum.java
View File

@ -95,13 +95,11 @@ public class InternalSum extends InternalNumericMetricsAggregation.SingleValue i
}
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject(name);
public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
builder.field(CommonFields.VALUE, sum);
if (valueFormatter != null) {
builder.field(CommonFields.VALUE_AS_STRING, valueFormatter.format(sum));
}
builder.endObject();
return builder;
}

4
src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/InternalTopHits.java
View File

@ -142,10 +142,8 @@ public class InternalTopHits extends InternalMetricsAggregation implements TopHi
}
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject(name);
public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
searchHits.toXContent(builder, params);
builder.endObject();
return builder;
}
}

6
src/main/java/org/elasticsearch/search/aggregations/metrics/valuecount/InternalValueCount.java
View File

@ -88,10 +88,8 @@ public class InternalValueCount extends InternalNumericMetricsAggregation implem
}
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
return builder.startObject(name)
.field(CommonFields.VALUE, value)
.endObject();
public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
return builder.field(CommonFields.VALUE, value);
}
@Override

7
src/test/java/org/elasticsearch/search/aggregations/bucket/SignificantTermsSignificanceScoreTests.java
View File

@ -20,9 +20,7 @@
package org.elasticsearch.search.aggregations.bucket;
import org.elasticsearch.action.index.IndexRequestBuilder;
import org.elasticsearch.action.search.SearchPhaseExecutionException;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.settings.Settings;
@ -33,7 +31,6 @@ import org.elasticsearch.index.query.FilterBuilders;
import org.elasticsearch.index.query.QueryParsingException;
import org.elasticsearch.plugins.AbstractPlugin;
import org.elasticsearch.search.aggregations.Aggregation;
import org.elasticsearch.search.aggregations.Aggregations;
import org.elasticsearch.search.aggregations.bucket.filter.FilterAggregationBuilder;
import org.elasticsearch.search.aggregations.bucket.filter.InternalFilter;
import org.elasticsearch.search.aggregations.bucket.significant.SignificantTerms;
@ -44,6 +41,8 @@ import org.elasticsearch.search.aggregations.bucket.terms.StringTerms;
import org.elasticsearch.search.aggregations.bucket.terms.Terms;
import org.elasticsearch.search.aggregations.bucket.terms.TermsBuilder;
import org.elasticsearch.test.ElasticsearchIntegrationTest;
import org.elasticsearch.test.ElasticsearchIntegrationTest.ClusterScope;
import org.elasticsearch.test.ElasticsearchIntegrationTest.Scope;
import org.junit.Test;
import java.io.IOException;
@ -53,8 +52,6 @@ import java.util.concurrent.ExecutionException;
import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_REPLICAS;
import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_SHARDS;
import static org.elasticsearch.common.settings.ImmutableSettings.settingsBuilder;
import static org.elasticsearch.test.ElasticsearchIntegrationTest.ClusterScope;
import static org.elasticsearch.test.ElasticsearchIntegrationTest.Scope;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse;
import static org.hamcrest.Matchers.*;