Make SignificantTerms.Bucket an interface rather than an abstract class (#24670)

This commit changes SignificantTerms.Bucket so that it is not an
abstract class anymore but an interface. It will be easier for the Java
High Level Rest Client to provide its own implementation of
SignificantTerms and SignificantTerms.Bucket. Also, it is now more
coherent with the others aggregations.

6 files changed, 45 insertions, 62 deletions

diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/InternalMappedSignificantTerms.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/InternalMappedSignificantTerms.java
index 34fada101f..e8d856ebf2 100644
--- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/InternalMappedSignificantTerms.java
+++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/InternalMappedSignificantTerms.java
@@ -26,6 +26,7 @@ import org.elasticsearch.search.aggregations.bucket.significant.heuristics.Signi
 import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator;
 
 import java.io.IOException;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -74,7 +75,12 @@ public abstract class InternalMappedSignificantTerms<
     }
 
     @Override
-    protected List<B> getBucketsInternal() {
+    public Iterator<SignificantTerms.Bucket> iterator() {
+        return buckets.stream().map(bucket -> (SignificantTerms.Bucket) bucket).collect(Collectors.toList()).iterator();
+    }
+
+    @Override
+    public List<B> getBuckets() {
         return buckets;
     }
 
diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/InternalSignificantTerms.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/InternalSignificantTerms.java
index 6414a5eb82..82ba2826dc 100644
--- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/InternalSignificantTerms.java
+++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/InternalSignificantTerms.java
@@ -33,20 +33,19 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 
-import static java.util.Collections.unmodifiableList;
-
 /**
  * Result of the significant terms aggregation.
  */
 public abstract class InternalSignificantTerms<A extends InternalSignificantTerms<A, B>, B extends InternalSignificantTerms.Bucket<B>>
         extends InternalMultiBucketAggregation<A, B> implements SignificantTerms, ToXContent {
+
     @SuppressWarnings("PMD.ConstructorCallsOverridableMethod")
-    public abstract static class Bucket<B extends Bucket<B>> extends SignificantTerms.Bucket {
+    public abstract static class Bucket<B extends Bucket<B>> extends InternalMultiBucketAggregation.InternalBucket
+            implements SignificantTerms.Bucket {
         /**
          * Reads a bucket. Should be a constructor reference.
          */
@@ -55,14 +54,21 @@ public abstract class InternalSignificantTerms<A extends InternalSignificantTerm
             B read(StreamInput in, long subsetSize, long supersetSize, DocValueFormat format) throws IOException;
         }
 
+        long subsetDf;
+        long subsetSize;
+        long supersetDf;
+        long supersetSize;
         long bucketOrd;
-        protected InternalAggregations aggregations;
         double score;
+        protected InternalAggregations aggregations;
         final transient DocValueFormat format;
 
         protected Bucket(long subsetDf, long subsetSize, long supersetDf, long supersetSize,
                 InternalAggregations aggregations, DocValueFormat format) {
-            super(subsetDf, subsetSize, supersetDf, supersetSize);
+            this.subsetSize = subsetSize;
+            this.supersetSize = supersetSize;
+            this.subsetDf = subsetDf;
+            this.supersetDf = supersetDf;
             this.aggregations = aggregations;
             this.format = format;
         }
@@ -71,7 +77,8 @@ public abstract class InternalSignificantTerms<A extends InternalSignificantTerm
          * Read from a stream.
          */
         protected Bucket(StreamInput in, long subsetSize, long supersetSize, DocValueFormat format) {
-            super(in, subsetSize, supersetSize);
+            this.subsetSize = subsetSize;
+            this.supersetSize = supersetSize;
             this.format = format;
         }
 
@@ -95,7 +102,7 @@ public abstract class InternalSignificantTerms<A extends InternalSignificantTerm
             return subsetSize;
         }
 
-        public void updateScore(SignificanceHeuristic significanceHeuristic) {
+        void updateScore(SignificanceHeuristic significanceHeuristic) {
             score = significanceHeuristic.getScore(subsetDf, subsetSize, supersetDf, supersetSize);
         }
 
@@ -179,16 +186,7 @@ public abstract class InternalSignificantTerms<A extends InternalSignificantTerm
     protected abstract void writeTermTypeInfoTo(StreamOutput out) throws IOException;
 
     @Override
-    public Iterator<SignificantTerms.Bucket> iterator() {
-        return getBuckets().iterator();
-    }
-
-    @Override
-    public List<SignificantTerms.Bucket> getBuckets() {
-        return unmodifiableList(getBucketsInternal());
-    }
-
-    protected abstract List<B> getBucketsInternal();
+    public abstract List<B> getBuckets();
 
     @Override
     public InternalAggregation doReduce(List<InternalAggregation> aggregations, ReduceContext reduceContext) {
@@ -206,7 +204,7 @@ public abstract class InternalSignificantTerms<A extends InternalSignificantTerm
         for (InternalAggregation aggregation : aggregations) {
             @SuppressWarnings("unchecked")
             InternalSignificantTerms<A, B> terms = (InternalSignificantTerms<A, B>) aggregation;
-            for (B bucket : terms.getBucketsInternal()) {
+            for (B bucket : terms.getBuckets()) {
                 List<B> existingBuckets = buckets.get(bucket.getKeyAsString());
                 if (existingBuckets == null) {
                     existingBuckets = new ArrayList<>(aggregations.size());
diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantLongTerms.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantLongTerms.java
index 6a714a6b03..c1556609f1 100644
--- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantLongTerms.java
+++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantLongTerms.java
@@ -77,7 +77,7 @@ public class SignificantLongTerms extends InternalMappedSignificantTerms<Signifi
         }
 
         @Override
-        int compareTerm(SignificantTerms.Bucket other) {
+        public int compareTerm(SignificantTerms.Bucket other) {
             return Long.compare(term, ((Number) other.getKey()).longValue());
         }
 
diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantStringTerms.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantStringTerms.java
index d62213bc47..84832325ce 100644
--- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantStringTerms.java
+++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantStringTerms.java
@@ -28,6 +28,7 @@ import org.elasticsearch.search.aggregations.bucket.significant.heuristics.Signi
 import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator;
 
 import java.io.IOException;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -82,7 +83,7 @@ public class SignificantStringTerms extends InternalMappedSignificantTerms<Signi
         }
 
         @Override
-        int compareTerm(SignificantTerms.Bucket other) {
+        public int compareTerm(SignificantTerms.Bucket other) {
             return termBytes.compareTo(((Bucket) other).termBytes);
         }
 
diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTerms.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTerms.java
index aa0a25ac45..ae09feef93 100644
--- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTerms.java
+++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantTerms.java
@@ -18,8 +18,6 @@
  */
 package org.elasticsearch.search.aggregations.bucket.significant;
 
-import org.elasticsearch.common.io.stream.StreamInput;
-import org.elasticsearch.search.aggregations.InternalMultiBucketAggregation;
 import org.elasticsearch.search.aggregations.bucket.MultiBucketsAggregation;
 
 import java.util.List;
@@ -28,54 +26,26 @@ import java.util.List;
  * An aggregation that collects significant terms in comparison to a background set.
  */
 public interface SignificantTerms extends MultiBucketsAggregation, Iterable<SignificantTerms.Bucket> {
-    abstract class Bucket extends InternalMultiBucketAggregation.InternalBucket {
 
-        long subsetDf;
-        long subsetSize;
-        long supersetDf;
-        long supersetSize;
+    interface Bucket extends MultiBucketsAggregation.Bucket {
 
-        Bucket(long subsetDf, long subsetSize, long supersetDf, long supersetSize) {
-            this.subsetSize = subsetSize;
-            this.supersetSize = supersetSize;
-            this.subsetDf = subsetDf;
-            this.supersetDf = supersetDf;
-        }
+        double getSignificanceScore();
 
-        /**
-         * Read from a stream.
-         */
-        protected Bucket(StreamInput in, long subsetSize, long supersetSize) {
-            this.subsetSize = subsetSize;
-            this.supersetSize = supersetSize;
-        }
+        Number getKeyAsNumber();
 
-        abstract int compareTerm(SignificantTerms.Bucket other);
+        long getSubsetDf();
 
-        public abstract double getSignificanceScore();
+        long getSupersetDf();
 
-        abstract Number getKeyAsNumber();
+        long getSupersetSize();
 
-        public long getSubsetDf() {
-            return subsetDf;
-        }
-
-        public long getSupersetDf() {
-            return supersetDf;
-        }
-
-        public long getSupersetSize() {
-            return supersetSize;
-        }
-
-        public long getSubsetSize() {
-            return subsetSize;
-        }
+        long getSubsetSize();
 
+        int compareTerm(SignificantTerms.Bucket other);
     }
 
     @Override
-    List<Bucket> getBuckets();
+    List<? extends Bucket> getBuckets();
 
     /**
      * Get the bucket for the given term, or null if there is no such bucket.
diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/UnmappedSignificantTerms.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/UnmappedSignificantTerms.java
index f9d8375af8..7252909bf1 100644
--- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/UnmappedSignificantTerms.java
+++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/UnmappedSignificantTerms.java
@@ -31,15 +31,18 @@ import org.elasticsearch.search.aggregations.bucket.terms.UnmappedTerms;
 import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator;
 
 import java.io.IOException;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 
+import static java.util.Collections.emptyIterator;
 import static java.util.Collections.emptyList;
 
 /**
  * Result of the running the significant terms aggregation on an unmapped field.
  */
 public class UnmappedSignificantTerms extends InternalSignificantTerms<UnmappedSignificantTerms, UnmappedSignificantTerms.Bucket> {
+
     public static final String NAME = "umsigterms";
 
     /**
@@ -117,7 +120,12 @@ public class UnmappedSignificantTerms extends InternalSignificantTerms<UnmappedS
     }
 
     @Override
-    protected List<Bucket> getBucketsInternal() {
+    public Iterator<SignificantTerms.Bucket> iterator() {
+        return emptyIterator();
+    }
+
+    @Override
+    public List<Bucket> getBuckets() {
         return emptyList();
     }