Skip to content

Commit c6b881b

Browse files
javannajavanna
authored
Share XContent rendering code in terms aggs (#23680)
The output of the different implementations of terms aggs is always very similar. The toXContent methods for each of those classes though was duplicating almost the same code multiple times. This commit centralizes the code for rendering XContent to a single place, which can be reused from the different terms aggs implementations.
1 parent 5cf13f2 commit c6b881b

File tree

7 files changed

+43
-69
lines changed

7 files changed

+43
-69
lines changed

‎core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/DoubleTerms.java‎

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -86,18 +86,11 @@ Bucket newBucket(long docCount, InternalAggregations aggs, long docCountError) {
8686
}
8787

8888
@Override
89-
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
90-
builder.startObject();
89+
protected final XContentBuilder keyToXContent(XContentBuilder builder) throws IOException {
9190
builder.field(CommonFields.KEY, term);
9291
if (format != DocValueFormat.RAW) {
9392
builder.field(CommonFields.KEY_AS_STRING, format.format(term));
9493
}
95-
builder.field(CommonFields.DOC_COUNT, getDocCount());
96-
if (showDocCountError) {
97-
builder.field(InternalTerms.DOC_COUNT_ERROR_UPPER_BOUND_FIELD_NAME, getDocCountError());
98-
}
99-
aggregations.toXContentInternal(builder, params);
100-
builder.endObject();
10194
return builder;
10295
}
10396

@@ -149,18 +142,6 @@ protected DoubleTerms create(String name, List<Bucket> buckets, long docCountErr
149142
shardSize, showTermDocCountError, otherDocCount, buckets, docCountError);
150143
}
151144

152-
@Override
153-
public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
154-
builder.field(InternalTerms.DOC_COUNT_ERROR_UPPER_BOUND_FIELD_NAME, docCountError);
155-
builder.field(SUM_OF_OTHER_DOC_COUNTS, otherDocCount);
156-
builder.startArray(CommonFields.BUCKETS);
157-
for (Bucket bucket : buckets) {
158-
bucket.toXContent(builder, params);
159-
}
160-
builder.endArray();
161-
return builder;
162-
}
163-
164145
@Override
165146
protected Bucket[] createBucketsArray(int size) {
166147
return new Bucket[size];
@@ -171,7 +152,7 @@ public InternalAggregation doReduce(List<InternalAggregation> aggregations, Redu
171152
boolean promoteToDouble = false;
172153
for (InternalAggregation agg : aggregations) {
173154
if (agg instanceof LongTerms && ((LongTerms) agg).format == DocValueFormat.RAW) {
174-
/**
155+
/*
175156
* this terms agg mixes longs and doubles, we must promote longs to doubles to make the internal aggs
176157
* compatible
177158
*/

‎core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/GlobalOrdinalsStringTermsAggregator.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ protected void writeTermTo(StreamOutput out) throws IOException {
246246
}
247247

248248
@Override
249-
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
249+
protected final XContentBuilder keyToXContent(XContentBuilder builder) throws IOException {
250250
throw new UnsupportedOperationException();
251251
}
252252
}

‎core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/InternalMappedTerms.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
import org.elasticsearch.common.io.stream.StreamInput;
2323
import org.elasticsearch.common.io.stream.StreamOutput;
24+
import org.elasticsearch.common.xcontent.XContentBuilder;
2425
import org.elasticsearch.search.DocValueFormat;
2526
import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator;
2627

@@ -127,4 +128,9 @@ protected boolean doEquals(Object obj) {
127128
protected int doHashCode() {
128129
return Objects.hash(super.doHashCode(), buckets, format, otherDocCount, showTermDocCountError, shardSize);
129130
}
131+
132+
@Override
133+
public final XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
134+
return doXContentCommon(builder, params, docCountError, otherDocCount, buckets);
135+
}
130136
}

‎core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/InternalTerms.java‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import org.elasticsearch.common.io.stream.StreamInput;
2222
import org.elasticsearch.common.io.stream.StreamOutput;
2323
import org.elasticsearch.common.xcontent.ToXContent;
24+
import org.elasticsearch.common.xcontent.XContentBuilder;
2425
import org.elasticsearch.search.DocValueFormat;
2526
import org.elasticsearch.search.aggregations.AggregationExecutionException;
2627
import org.elasticsearch.search.aggregations.Aggregations;
@@ -141,6 +142,21 @@ public B reduce(List<B> buckets, ReduceContext context) {
141142
return newBucket(docCount, aggs, docCountError);
142143
}
143144

145+
@Override
146+
public final XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
147+
builder.startObject();
148+
keyToXContent(builder);
149+
builder.field(CommonFields.DOC_COUNT, getDocCount());
150+
if (showDocCountError) {
151+
builder.field(InternalTerms.DOC_COUNT_ERROR_UPPER_BOUND_FIELD_NAME, getDocCountError());
152+
}
153+
aggregations.toXContentInternal(builder, params);
154+
builder.endObject();
155+
return builder;
156+
}
157+
158+
protected abstract XContentBuilder keyToXContent(XContentBuilder builder) throws IOException;
159+
144160
@Override
145161
public boolean equals(Object obj) {
146162
if (obj == null || getClass() != obj.getClass()) {
@@ -319,4 +335,16 @@ protected boolean doEquals(Object obj) {
319335
protected int doHashCode() {
320336
return Objects.hash(minDocCount, order, requiredSize);
321337
}
338+
339+
protected static XContentBuilder doXContentCommon(XContentBuilder builder, Params params,
340+
long docCountError, long otherDocCount, List<? extends Bucket> buckets) throws IOException {
341+
builder.field(DOC_COUNT_ERROR_UPPER_BOUND_FIELD_NAME, docCountError);
342+
builder.field(SUM_OF_OTHER_DOC_COUNTS, otherDocCount);
343+
builder.startArray(CommonFields.BUCKETS);
344+
for (Bucket bucket : buckets) {
345+
bucket.toXContent(builder, params);
346+
}
347+
builder.endArray();
348+
return builder;
349+
}
322350
}

‎core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/LongTerms.java‎

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -86,18 +86,11 @@ Bucket newBucket(long docCount, InternalAggregations aggs, long docCountError) {
8686
}
8787

8888
@Override
89-
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
90-
builder.startObject();
89+
protected final XContentBuilder keyToXContent(XContentBuilder builder) throws IOException {
9190
builder.field(CommonFields.KEY, term);
9291
if (format != DocValueFormat.RAW) {
9392
builder.field(CommonFields.KEY_AS_STRING, format.format(term));
9493
}
95-
builder.field(CommonFields.DOC_COUNT, getDocCount());
96-
if (showDocCountError) {
97-
builder.field(InternalTerms.DOC_COUNT_ERROR_UPPER_BOUND_FIELD_NAME, getDocCountError());
98-
}
99-
aggregations.toXContentInternal(builder, params);
100-
builder.endObject();
10194
return builder;
10295
}
10396

@@ -149,18 +142,6 @@ protected LongTerms create(String name, List<Bucket> buckets, long docCountError
149142
showTermDocCountError, otherDocCount, buckets, docCountError);
150143
}
151144

152-
@Override
153-
public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
154-
builder.field(InternalTerms.DOC_COUNT_ERROR_UPPER_BOUND_FIELD_NAME, docCountError);
155-
builder.field(SUM_OF_OTHER_DOC_COUNTS, otherDocCount);
156-
builder.startArray(CommonFields.BUCKETS);
157-
for (Bucket bucket : buckets) {
158-
bucket.toXContent(builder, params);
159-
}
160-
builder.endArray();
161-
return builder;
162-
}
163-
164145
@Override
165146
protected Bucket[] createBucketsArray(int size) {
166147
return new Bucket[size];

‎core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/StringTerms.java‎

Lines changed: 2 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -85,16 +85,8 @@ Bucket newBucket(long docCount, InternalAggregations aggs, long docCountError) {
8585
}
8686

8787
@Override
88-
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
89-
builder.startObject();
90-
builder.field(CommonFields.KEY, getKeyAsString());
91-
builder.field(CommonFields.DOC_COUNT, getDocCount());
92-
if (showDocCountError) {
93-
builder.field(InternalTerms.DOC_COUNT_ERROR_UPPER_BOUND_FIELD_NAME, getDocCountError());
94-
}
95-
aggregations.toXContentInternal(builder, params);
96-
builder.endObject();
97-
return builder;
88+
protected final XContentBuilder keyToXContent(XContentBuilder builder) throws IOException {
89+
return builder.field(CommonFields.KEY, getKeyAsString());
9890
}
9991

10092
@Override
@@ -145,18 +137,6 @@ protected StringTerms create(String name, List<Bucket> buckets, long docCountErr
145137
showTermDocCountError, otherDocCount, buckets, docCountError);
146138
}
147139

148-
@Override
149-
public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
150-
builder.field(InternalTerms.DOC_COUNT_ERROR_UPPER_BOUND_FIELD_NAME, docCountError);
151-
builder.field(SUM_OF_OTHER_DOC_COUNTS, otherDocCount);
152-
builder.startArray(CommonFields.BUCKETS);
153-
for (Bucket bucket : buckets) {
154-
bucket.toXContent(builder, params);
155-
}
156-
builder.endArray();
157-
return builder;
158-
}
159-
160140
@Override
161141
protected Bucket[] createBucketsArray(int size) {
162142
return new Bucket[size];

‎core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/UnmappedTerms.java‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator;
2828

2929
import java.io.IOException;
30+
import java.util.Collections;
3031
import java.util.List;
3132
import java.util.Map;
3233

@@ -102,11 +103,8 @@ public InternalAggregation doReduce(List<InternalAggregation> aggregations, Redu
102103
}
103104

104105
@Override
105-
public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
106-
builder.field(InternalTerms.DOC_COUNT_ERROR_UPPER_BOUND_FIELD_NAME, 0);
107-
builder.field(SUM_OF_OTHER_DOC_COUNTS, 0);
108-
builder.startArray(CommonFields.BUCKETS).endArray();
109-
return builder;
106+
public final XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
107+
return doXContentCommon(builder, params, 0, 0, Collections.emptyList());
110108
}
111109

112110
@Override

0 commit comments

Comments
 (0)