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.

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;
     }
 

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() {
+    public abstract List<B> getBuckets();
-        return getBuckets().iterator();
-    }
-
-    @Override
-    public List<SignificantTerms.Bucket> getBuckets() {
-        return unmodifiableList(getBucketsInternal());
-    }
-
-    protected abstract List<B> getBucketsInternal();
 
     @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());

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());
         }
 

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);
         }
 

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;
+    interface Bucket extends MultiBucketsAggregation.Bucket {
-        long subsetSize;
-        long supersetDf;
-        long supersetSize;
 
-        Bucket(long subsetDf, long subsetSize, long supersetDf, long supersetSize) {
+        double getSignificanceScore();
-            this.subsetSize = subsetSize;
-            this.supersetSize = supersetSize;
-            this.subsetDf = subsetDf;
-            this.supersetDf = supersetDf;
-        }
 
-        /**
+        Number getKeyAsNumber();
-         * Read from a stream.
-         */
-        protected Bucket(StreamInput in, long subsetSize, long supersetSize) {
-            this.subsetSize = subsetSize;
-            this.supersetSize = supersetSize;
-        }
 
-        abstract int compareTerm(SignificantTerms.Bucket other);
+        long getSubsetDf();
 
-        public abstract double getSignificanceScore();
+        long getSupersetDf();
 
-        abstract Number getKeyAsNumber();
+        long getSupersetSize();
 
-        public long getSubsetDf() {
+        long getSubsetSize();
-            return subsetDf;
-        }
-
-        public long getSupersetDf() {
-            return supersetDf;
-        }
-
-        public long getSupersetSize() {
-            return supersetSize;
-        }
-
-        public long getSubsetSize() {
-            return subsetSize;
-        }
 
+        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.

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();
     }
 

core/src/test/java/org/elasticsearch/search/aggregations/bucket/SignificantTermsSignificanceScoreIT.java

@@ -443,8 +443,8 @@ public class SignificantTermsSignificanceScoreIT extends ESIntegTestCase {
         Aggregations aggregations = classBuckets.next().getAggregations();
         SignificantTerms sigTerms = aggregations.get("mySignificantTerms");
 
-        Collection<SignificantTerms.Bucket> classA = sigTerms.getBuckets();
+        List<? extends SignificantTerms.Bucket> classA = sigTerms.getBuckets();
-        Iterator<SignificantTerms.Bucket> classBBucketIterator = sigTerms.getBuckets().iterator();
+        Iterator<SignificantTerms.Bucket> classBBucketIterator = sigTerms.iterator();
         assertThat(classA.size(), greaterThan(0));
         for (SignificantTerms.Bucket classABucket : classA) {
             SignificantTerms.Bucket classBBucket = classBBucketIterator.next();