Share XContent rendering code in terms aggs (#23680)

javanna · javanna · commit 9617f426f21d · 2017-03-22T14:26:20.000+01:00
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.
diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/DoubleTerms.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/DoubleTerms.java
@@ -83,18 +83,11 @@ Bucket newBucket(long docCount, InternalAggregations aggs, long docCountError) {
         }
 
         @Override
-        public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
-            builder.startObject();
+        protected final XContentBuilder keyToXContent(XContentBuilder builder) throws IOException {
             builder.field(CommonFields.KEY, term);
             if (format != DocValueFormat.RAW) {
                 builder.field(CommonFields.KEY_AS_STRING, format.format(term));
             }
-            builder.field(CommonFields.DOC_COUNT, getDocCount());
-            if (showDocCountError) {
-                builder.field(InternalTerms.DOC_COUNT_ERROR_UPPER_BOUND_FIELD_NAME, getDocCountError());
-            }
-            aggregations.toXContentInternal(builder, params);
-            builder.endObject();
             return builder;
         }
     }
@@ -136,18 +129,6 @@ protected DoubleTerms create(String name, List<Bucket> buckets, long docCountErr
                 shardSize, showTermDocCountError, otherDocCount, buckets, docCountError);
     }
 
-    @Override
-    public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
-        builder.field(InternalTerms.DOC_COUNT_ERROR_UPPER_BOUND_FIELD_NAME, docCountError);
-        builder.field(SUM_OF_OTHER_DOC_COUNTS, otherDocCount);
-        builder.startArray(CommonFields.BUCKETS);
-        for (Bucket bucket : buckets) {
-            bucket.toXContent(builder, params);
-        }
-        builder.endArray();
-        return builder;
-    }
-
     @Override
     protected Bucket[] createBucketsArray(int size) {
         return new Bucket[size];
diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/GlobalOrdinalsStringTermsAggregator.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/GlobalOrdinalsStringTermsAggregator.java
@@ -246,7 +246,7 @@ protected void writeTermTo(StreamOutput out) throws IOException {
         }
 
         @Override
-        public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
+        protected final XContentBuilder keyToXContent(XContentBuilder builder) throws IOException {
             throw new UnsupportedOperationException();
         }
     }
diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/InternalMappedTerms.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/InternalMappedTerms.java
@@ -21,6 +21,7 @@
 
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
+import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.search.DocValueFormat;
 import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator;
 
@@ -110,4 +111,9 @@ public B getBucketByKey(String term) {
         }
         return bucketMap.get(term);
     }
+
+    @Override
+    public final XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
+        return doXContentCommon(builder, params, docCountError, otherDocCount, buckets);
+    }
 }
diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/InternalTerms.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/InternalTerms.java
@@ -21,6 +21,7 @@
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.xcontent.ToXContent;
+import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.search.DocValueFormat;
 import org.elasticsearch.search.aggregations.AggregationExecutionException;
 import org.elasticsearch.search.aggregations.Aggregations;
@@ -139,6 +140,21 @@ public B reduce(List<B> buckets, ReduceContext context) {
             InternalAggregations aggs = InternalAggregations.reduce(aggregationsList, context);
             return newBucket(docCount, aggs, docCountError);
         }
+
+        @Override
+        public final XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
+            builder.startObject();
+            keyToXContent(builder);
+            builder.field(CommonFields.DOC_COUNT, getDocCount());
+            if (showDocCountError) {
+                builder.field(InternalTerms.DOC_COUNT_ERROR_UPPER_BOUND_FIELD_NAME, getDocCountError());
+            }
+            aggregations.toXContentInternal(builder, params);
+            builder.endObject();
+            return builder;
+        }
+
+        protected abstract XContentBuilder keyToXContent(XContentBuilder builder) throws IOException;
     }
 
     protected final Terms.Order order;
@@ -286,4 +302,16 @@ public InternalAggregation doReduce(List<InternalAggregation> aggregations, Redu
      * Create an array to hold some buckets. Used in collecting the results.
      */
     protected abstract B[] createBucketsArray(int size);
+
+    protected static XContentBuilder doXContentCommon(XContentBuilder builder, Params params,
+                                               long docCountError, long otherDocCount, List<? extends Bucket> buckets) throws IOException {
+        builder.field(DOC_COUNT_ERROR_UPPER_BOUND_FIELD_NAME, docCountError);
+        builder.field(SUM_OF_OTHER_DOC_COUNTS, otherDocCount);
+        builder.startArray(CommonFields.BUCKETS);
+        for (Bucket bucket : buckets) {
+            bucket.toXContent(builder, params);
+        }
+        builder.endArray();
+        return builder;
+    }
 }
diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/LongTerms.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/LongTerms.java
@@ -83,18 +83,11 @@ Bucket newBucket(long docCount, InternalAggregations aggs, long docCountError) {
         }
 
         @Override
-        public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
-            builder.startObject();
+        protected final XContentBuilder keyToXContent(XContentBuilder builder) throws IOException {
             builder.field(CommonFields.KEY, term);
             if (format != DocValueFormat.RAW) {
                 builder.field(CommonFields.KEY_AS_STRING, format.format(term));
             }
-            builder.field(CommonFields.DOC_COUNT, getDocCount());
-            if (showDocCountError) {
-                builder.field(InternalTerms.DOC_COUNT_ERROR_UPPER_BOUND_FIELD_NAME, getDocCountError());
-            }
-            aggregations.toXContentInternal(builder, params);
-            builder.endObject();
             return builder;
         }
     }
@@ -136,18 +129,6 @@ protected LongTerms create(String name, List<Bucket> buckets, long docCountError
                 showTermDocCountError, otherDocCount, buckets, docCountError);
     }
 
-    @Override
-    public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
-        builder.field(InternalTerms.DOC_COUNT_ERROR_UPPER_BOUND_FIELD_NAME, docCountError);
-        builder.field(SUM_OF_OTHER_DOC_COUNTS, otherDocCount);
-        builder.startArray(CommonFields.BUCKETS);
-        for (Bucket bucket : buckets) {
-            bucket.toXContent(builder, params);
-        }
-        builder.endArray();
-        return builder;
-    }
-
     @Override
     protected Bucket[] createBucketsArray(int size) {
         return new Bucket[size];
diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/StringTerms.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/StringTerms.java
@@ -84,16 +84,8 @@ Bucket newBucket(long docCount, InternalAggregations aggs, long docCountError) {
         }
 
         @Override
-        public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
-            builder.startObject();
-            builder.field(CommonFields.KEY, getKeyAsString());
-            builder.field(CommonFields.DOC_COUNT, getDocCount());
-            if (showDocCountError) {
-                builder.field(InternalTerms.DOC_COUNT_ERROR_UPPER_BOUND_FIELD_NAME, getDocCountError());
-            }
-            aggregations.toXContentInternal(builder, params);
-            builder.endObject();
-            return builder;
+        protected final XContentBuilder keyToXContent(XContentBuilder builder) throws IOException {
+            return builder.field(CommonFields.KEY, getKeyAsString());
         }
     }
 
@@ -134,18 +126,6 @@ protected StringTerms create(String name, List<Bucket> buckets, long docCountErr
                 showTermDocCountError, otherDocCount, buckets, docCountError);
     }
 
-    @Override
-    public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
-        builder.field(InternalTerms.DOC_COUNT_ERROR_UPPER_BOUND_FIELD_NAME, docCountError);
-        builder.field(SUM_OF_OTHER_DOC_COUNTS, otherDocCount);
-        builder.startArray(CommonFields.BUCKETS);
-        for (Bucket bucket : buckets) {
-            bucket.toXContent(builder, params);
-        }
-        builder.endArray();
-        return builder;
-    }
-
     @Override
     protected Bucket[] createBucketsArray(int size) {
         return new Bucket[size];
diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/UnmappedTerms.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/UnmappedTerms.java
@@ -27,6 +27,7 @@
 import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator;
 
 import java.io.IOException;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 
@@ -102,11 +103,8 @@ public InternalAggregation doReduce(List<InternalAggregation> aggregations, Redu
     }
 
     @Override
-    public XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
-        builder.field(InternalTerms.DOC_COUNT_ERROR_UPPER_BOUND_FIELD_NAME, 0);
-        builder.field(SUM_OF_OTHER_DOC_COUNTS, 0);
-        builder.startArray(CommonFields.BUCKETS).endArray();
-        return builder;
+    public final XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
+        return doXContentCommon(builder, params, 0, 0, Collections.emptyList());
     }
 
     @Override

Original file line number	Diff line number	Diff line change
`@@ -246,7 +246,7 @@ protected void writeTermTo(StreamOutput out) throws IOException {`
`246`	`246`	`}`
`247`	`247`
`248`	`248`	`@Override`
`249`		`- public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {`
	`249`	`+ protected final XContentBuilder keyToXContent(XContentBuilder builder) throws IOException {`
`250`	`250`	`throw new UnsupportedOperationException();`
`251`	`251`	`}`
`252`	`252`	`}`
Original file line number	Diff line number	Diff line change
`@@ -21,6 +21,7 @@`
`21`	`21`
`22`	`22`	`import org.elasticsearch.common.io.stream.StreamInput;`
`23`	`23`	`import org.elasticsearch.common.io.stream.StreamOutput;`
	`24`	`+import org.elasticsearch.common.xcontent.XContentBuilder;`
`24`	`25`	`import org.elasticsearch.search.DocValueFormat;`
`25`	`26`	`import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator;`
`26`	`27`
`@@ -110,4 +111,9 @@ public B getBucketByKey(String term) {`
`110`	`111`	`}`
`111`	`112`	`return bucketMap.get(term);`
`112`	`113`	`}`
	`114`	`+`
	`115`	`+ @Override`
	`116`	`+ public final XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {`
	`117`	`+ return doXContentCommon(builder, params, docCountError, otherDocCount, buckets);`
	`118`	`+ }`
`113`	`119`	`}`