From 7bd2abe48af5a651b54bcd5bcb41c88e29390be0 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Fri, 5 May 2017 20:00:39 +0200 Subject: Change Terms.Bucket to an interface (#24492) This commit changes the Terms.Bucket abstract class to an interface, so that it's easier for the Java High Level Rest Client to provide its own implementation. In its current state, the Terms.Bucket abstract class inherits from InternalMultiBucketAggregation.InternalBucket which forces subclasses to implement Writeable and exposes a public getProperty() method that relies on InternalAggregation. This two points make it difficult for the Java High Level Rest Client to implement the Terms and Terms.Bucket correctly. This is also different from other MultiBucketsAggregation like Range which are pure interfaces. Changing Terms.Bucket to an interface causes a method clashes for the `getBuckets()` method in InternalTerms. This is because: - InternalTerms implements Terms which declared a `List getBuckets()` method - InternalTerms extends InternalMultiBucketAggregation which declares a `List getBuckets()` method - both overrides the MultiBucketsAggregation `List getBuckets()` method There was no clashes before this change because Terms.Bucket extends InternalBucket and conformed to both declaration. With Terms.Bucket now an interface, the getBuckets() method in the Terms interface is changed to avoid method clash. This is a breaking change in the Java API but it's a straightforward change and the Terms multi bucket aggregation interface is also more coherent with the other Range, Histogram, Filters, AdjacencyMatrix etc that all return a `List`. --- .../aggregations/bucket/DiversifiedSamplerIT.java | 11 ++++---- .../search/aggregations/bucket/DoubleTermsIT.java | 4 +-- .../search/aggregations/bucket/LongTermsIT.java | 4 +-- .../search/aggregations/bucket/MinDocCountIT.java | 4 +-- .../search/aggregations/bucket/SamplerIT.java | 5 ++-- .../aggregations/bucket/ShardSizeTermsIT.java | 32 +++++++++++----------- .../SignificantTermsSignificanceScoreIT.java | 2 +- .../search/aggregations/bucket/StringTermsIT.java | 8 +++--- .../aggregations/bucket/TermsDocCountErrorIT.java | 10 +++---- .../bucket/terms/TermsAggregatorTests.java | 2 +- 10 files changed, 42 insertions(+), 40 deletions(-) (limited to 'core/src/test/java/org/elasticsearch/search/aggregations/bucket') diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/DiversifiedSamplerIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/DiversifiedSamplerIT.java index 9af28806fe..c8803b7e79 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/DiversifiedSamplerIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/DiversifiedSamplerIT.java @@ -32,6 +32,7 @@ import org.elasticsearch.search.aggregations.metrics.max.Max; import org.elasticsearch.test.ESIntegTestCase; import java.util.Collection; +import java.util.List; import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_REPLICAS; import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_SHARDS; @@ -108,7 +109,7 @@ public class DiversifiedSamplerIT extends ESIntegTestCase { ).execute().actionGet(); assertSearchResponse(response); Terms genres = response.getAggregations().get("genres"); - Collection genreBuckets = genres.getBuckets(); + Collection genreBuckets = genres.getBuckets(); // For this test to be useful we need >1 genre bucket to compare assertThat(genreBuckets.size(), greaterThan(1)); double lastMaxPrice = asc ? Double.MIN_VALUE : Double.MAX_VALUE; @@ -141,7 +142,7 @@ public class DiversifiedSamplerIT extends ESIntegTestCase { assertSearchResponse(response); Sampler sample = response.getAggregations().get("sample"); Terms authors = sample.getAggregations().get("authors"); - Collection testBuckets = authors.getBuckets(); + List testBuckets = authors.getBuckets(); for (Terms.Bucket testBucket : testBuckets) { assertThat(testBucket.getDocCount(), lessThanOrEqualTo((long) NUM_SHARDS * MAX_DOCS_PER_AUTHOR)); @@ -162,11 +163,11 @@ public class DiversifiedSamplerIT extends ESIntegTestCase { .addAggregation(rootTerms).execute().actionGet(); assertSearchResponse(response); Terms genres = response.getAggregations().get("genres"); - Collection genreBuckets = genres.getBuckets(); + List genreBuckets = genres.getBuckets(); for (Terms.Bucket genreBucket : genreBuckets) { Sampler sample = genreBucket.getAggregations().get("sample"); Terms authors = sample.getAggregations().get("authors"); - Collection testBuckets = authors.getBuckets(); + List testBuckets = authors.getBuckets(); for (Terms.Bucket testBucket : testBuckets) { assertThat(testBucket.getDocCount(), lessThanOrEqualTo((long) NUM_SHARDS * MAX_DOCS_PER_AUTHOR)); @@ -195,7 +196,7 @@ public class DiversifiedSamplerIT extends ESIntegTestCase { Sampler sample = genreSample.getAggregations().get("sample"); Terms genres = sample.getAggregations().get("genres"); - Collection testBuckets = genres.getBuckets(); + List testBuckets = genres.getBuckets(); for (Terms.Bucket testBucket : testBuckets) { assertThat(testBucket.getDocCount(), lessThanOrEqualTo((long) NUM_SHARDS * MAX_DOCS_PER_GENRE)); } diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/DoubleTermsIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/DoubleTermsIT.java index cf28541121..ca106721fc 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/DoubleTermsIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/DoubleTermsIT.java @@ -842,7 +842,7 @@ public class DoubleTermsIT extends AbstractTermsTestCase { assertThat(tags.getName(), equalTo("num_tags")); assertThat(tags.getBuckets().size(), equalTo(2)); - Iterator iters = tags.getBuckets().iterator(); + Iterator iters = tags.getBuckets().iterator(); Terms.Bucket tag = iters.next(); assertThat(tag, notNullValue()); @@ -883,7 +883,7 @@ public class DoubleTermsIT extends AbstractTermsTestCase { assertThat(tags.getName(), equalTo("tags")); assertThat(tags.getBuckets().size(), equalTo(2)); - Iterator iters = tags.getBuckets().iterator(); + Iterator iters = tags.getBuckets().iterator(); // the max for "1" is 2 // the max for "0" is 4 diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/LongTermsIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/LongTermsIT.java index 6793768a91..a54dc3e2f5 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/LongTermsIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/LongTermsIT.java @@ -854,7 +854,7 @@ public class LongTermsIT extends AbstractTermsTestCase { assertThat(tags.getName(), equalTo("num_tags")); assertThat(tags.getBuckets().size(), equalTo(2)); - Iterator iters = tags.getBuckets().iterator(); + Iterator iters = tags.getBuckets().iterator(); Terms.Bucket tag = iters.next(); assertThat(tag, notNullValue()); @@ -893,7 +893,7 @@ public class LongTermsIT extends AbstractTermsTestCase { assertThat(tags.getName(), equalTo("tags")); assertThat(tags.getBuckets().size(), equalTo(2)); - Iterator iters = tags.getBuckets().iterator(); + Iterator iters = tags.getBuckets().iterator(); // the max for "1" is 2 // the max for "0" is 4 diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/MinDocCountIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/MinDocCountIT.java index 7307b756e3..e1e8f1ba66 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/MinDocCountIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/MinDocCountIT.java @@ -161,8 +161,8 @@ public class MinDocCountIT extends AbstractTermsTestCase { // check that terms2 is a subset of terms1 private void assertSubset(Terms terms1, Terms terms2, long minDocCount, int size, String include) { final Matcher matcher = include == null ? null : Pattern.compile(include).matcher("");; - final Iterator it1 = terms1.getBuckets().iterator(); - final Iterator it2 = terms2.getBuckets().iterator(); + final Iterator it1 = terms1.getBuckets().iterator(); + final Iterator it2 = terms2.getBuckets().iterator(); int size2 = 0; while (it1.hasNext()) { final Terms.Bucket bucket1 = it1.next(); diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/SamplerIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/SamplerIT.java index f87b98bc87..ebd078de67 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/SamplerIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/SamplerIT.java @@ -31,6 +31,7 @@ import org.elasticsearch.search.aggregations.metrics.max.Max; import org.elasticsearch.test.ESIntegTestCase; import java.util.Collection; +import java.util.List; import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_REPLICAS; import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_SHARDS; @@ -104,7 +105,7 @@ public class SamplerIT extends ESIntegTestCase { ).execute().actionGet(); assertSearchResponse(response); Terms genres = response.getAggregations().get("genres"); - Collection genreBuckets = genres.getBuckets(); + List genreBuckets = genres.getBuckets(); // For this test to be useful we need >1 genre bucket to compare assertThat(genreBuckets.size(), greaterThan(1)); double lastMaxPrice = asc ? Double.MIN_VALUE : Double.MAX_VALUE; @@ -130,7 +131,7 @@ public class SamplerIT extends ESIntegTestCase { assertSearchResponse(response); Sampler sample = response.getAggregations().get("sample"); Terms authors = sample.getAggregations().get("authors"); - Collection testBuckets = authors.getBuckets(); + List testBuckets = authors.getBuckets(); long maxBooksPerAuthor = 0; for (Terms.Bucket testBucket : testBuckets) { diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/ShardSizeTermsIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/ShardSizeTermsIT.java index ecc16b85f1..748c5f886f 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/ShardSizeTermsIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/ShardSizeTermsIT.java @@ -22,8 +22,8 @@ import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.search.aggregations.Aggregator.SubAggCollectionMode; import org.elasticsearch.search.aggregations.bucket.terms.Terms; -import java.util.Collection; import java.util.HashMap; +import java.util.List; import java.util.Map; import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; @@ -43,7 +43,7 @@ public class ShardSizeTermsIT extends ShardSizeTestCase { .execute().actionGet(); Terms terms = response.getAggregations().get("keys"); - Collection buckets = terms.getBuckets(); + List buckets = terms.getBuckets(); assertThat(buckets.size(), equalTo(3)); Map expected = new HashMap<>(); expected.put("1", 8L); @@ -66,7 +66,7 @@ public class ShardSizeTermsIT extends ShardSizeTestCase { .execute().actionGet(); Terms terms = response.getAggregations().get("keys"); - Collection buckets = terms.getBuckets(); + List buckets = terms.getBuckets(); assertThat(buckets.size(), equalTo(3)); Map expected = new HashMap<>(); expected.put("1", 8L); @@ -90,7 +90,7 @@ public class ShardSizeTermsIT extends ShardSizeTestCase { .execute().actionGet(); Terms terms = response.getAggregations().get("keys"); - Collection buckets = terms.getBuckets(); + List buckets = terms.getBuckets(); assertThat(buckets.size(), equalTo(3)); // we still only return 3 entries (based on the 'size' param) Map expected = new HashMap<>(); expected.put("1", 8L); @@ -114,7 +114,7 @@ public class ShardSizeTermsIT extends ShardSizeTestCase { .execute().actionGet(); Terms terms = response.getAggregations().get("keys"); - Collection buckets = terms.getBuckets(); + List buckets = terms.getBuckets(); assertThat(buckets.size(), equalTo(3)); // we still only return 3 entries (based on the 'size' param) Map expected = new HashMap<>(); expected.put("1", 5L); @@ -137,7 +137,7 @@ public class ShardSizeTermsIT extends ShardSizeTestCase { .execute().actionGet(); Terms terms = response.getAggregations().get("keys"); - Collection buckets = terms.getBuckets(); + List buckets = terms.getBuckets(); assertThat(buckets.size(), equalTo(3)); Map expected = new HashMap<>(); expected.put("1", 8L); @@ -160,7 +160,7 @@ public class ShardSizeTermsIT extends ShardSizeTestCase { .execute().actionGet(); Terms terms = response.getAggregations().get("keys"); - Collection buckets = terms.getBuckets(); + List buckets = terms.getBuckets(); assertThat(buckets.size(), equalTo(3)); Map expected = new HashMap<>(); expected.put(1, 8L); @@ -183,7 +183,7 @@ public class ShardSizeTermsIT extends ShardSizeTestCase { .execute().actionGet(); Terms terms = response.getAggregations().get("keys"); - Collection buckets = terms.getBuckets(); + List buckets = terms.getBuckets(); assertThat(buckets.size(), equalTo(3)); Map expected = new HashMap<>(); expected.put(1, 8L); @@ -206,7 +206,7 @@ public class ShardSizeTermsIT extends ShardSizeTestCase { .execute().actionGet(); Terms terms = response.getAggregations().get("keys"); - Collection buckets = terms.getBuckets(); + List buckets = terms.getBuckets(); assertThat(buckets.size(), equalTo(3)); // we still only return 3 entries (based on the 'size' param) Map expected = new HashMap<>(); expected.put(1, 8L); @@ -230,7 +230,7 @@ public class ShardSizeTermsIT extends ShardSizeTestCase { .execute().actionGet(); Terms terms = response.getAggregations().get("keys"); - Collection buckets = terms.getBuckets(); + List buckets = terms.getBuckets(); assertThat(buckets.size(), equalTo(3)); // we still only return 3 entries (based on the 'size' param) Map expected = new HashMap<>(); expected.put(1, 5L); @@ -253,7 +253,7 @@ public class ShardSizeTermsIT extends ShardSizeTestCase { .execute().actionGet(); Terms terms = response.getAggregations().get("keys"); - Collection buckets = terms.getBuckets(); + List buckets = terms.getBuckets(); assertThat(buckets.size(), equalTo(3)); Map expected = new HashMap<>(); expected.put(1, 8L); @@ -276,7 +276,7 @@ public class ShardSizeTermsIT extends ShardSizeTestCase { .execute().actionGet(); Terms terms = response.getAggregations().get("keys"); - Collection buckets = terms.getBuckets(); + List buckets = terms.getBuckets(); assertThat(buckets.size(), equalTo(3)); Map expected = new HashMap<>(); expected.put(1, 8L); @@ -299,7 +299,7 @@ public class ShardSizeTermsIT extends ShardSizeTestCase { .execute().actionGet(); Terms terms = response.getAggregations().get("keys"); - Collection buckets = terms.getBuckets(); + List buckets = terms.getBuckets(); assertThat(buckets.size(), equalTo(3)); Map expected = new HashMap<>(); expected.put(1, 8L); @@ -322,7 +322,7 @@ public class ShardSizeTermsIT extends ShardSizeTestCase { .execute().actionGet(); Terms terms = response.getAggregations().get("keys"); - Collection buckets = terms.getBuckets(); + List buckets = terms.getBuckets(); assertThat(buckets.size(), equalTo(3)); Map expected = new HashMap<>(); expected.put(1, 8L); @@ -345,7 +345,7 @@ public class ShardSizeTermsIT extends ShardSizeTestCase { .execute().actionGet(); Terms terms = response.getAggregations().get("keys"); - Collection buckets = terms.getBuckets(); + List buckets = terms.getBuckets(); assertThat(buckets.size(), equalTo(3)); Map expected = new HashMap<>(); expected.put(1, 5L); @@ -368,7 +368,7 @@ public class ShardSizeTermsIT extends ShardSizeTestCase { .execute().actionGet(); Terms terms = response.getAggregations().get("keys"); - Collection buckets = terms.getBuckets(); + List buckets = terms.getBuckets(); assertThat(buckets.size(), equalTo(3)); Map expected = new HashMap<>(); expected.put(1, 8L); diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/SignificantTermsSignificanceScoreIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/SignificantTermsSignificanceScoreIT.java index 7fc20908e8..09cec9a458 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/SignificantTermsSignificanceScoreIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/SignificantTermsSignificanceScoreIT.java @@ -438,7 +438,7 @@ public class SignificantTermsSignificanceScoreIT extends ESIntegTestCase { assertSearchResponse(response); StringTerms classes = response.getAggregations().get("class"); assertThat(classes.getBuckets().size(), equalTo(2)); - Iterator classBuckets = classes.getBuckets().iterator(); + Iterator classBuckets = classes.getBuckets().iterator(); Aggregations aggregations = classBuckets.next().getAggregations(); SignificantTerms sigTerms = aggregations.get("mySignificantTerms"); diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/StringTermsIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/StringTermsIT.java index b5dbcd9085..df69cfcfa9 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/StringTermsIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/StringTermsIT.java @@ -1012,7 +1012,7 @@ public class StringTermsIT extends AbstractTermsTestCase { assertThat(tags.getName(), equalTo("tags")); assertThat(tags.getBuckets().size(), equalTo(2)); - Iterator iters = tags.getBuckets().iterator(); + Iterator iters = tags.getBuckets().iterator(); Terms.Bucket tag = iters.next(); assertThat(tag, notNullValue()); @@ -1054,7 +1054,7 @@ public class StringTermsIT extends AbstractTermsTestCase { assertThat(tags.getName(), equalTo("tags")); assertThat(tags.getBuckets().size(), equalTo(2)); - Iterator iters = tags.getBuckets().iterator(); + Iterator iters = tags.getBuckets().iterator(); // the max for "more" is 2 // the max for "less" is 4 @@ -1117,7 +1117,7 @@ public class StringTermsIT extends AbstractTermsTestCase { assertThat(tags.getName(), equalTo("tags")); assertThat(tags.getBuckets().size(), equalTo(2)); - Iterator iters = tags.getBuckets().iterator(); + Iterator iters = tags.getBuckets().iterator(); // the max for "more" is 2 // the max for "less" is 4 @@ -1180,7 +1180,7 @@ public class StringTermsIT extends AbstractTermsTestCase { assertThat(tags.getName(), equalTo("tags")); assertThat(tags.getBuckets().size(), equalTo(2)); - Iterator iters = tags.getBuckets().iterator(); + Iterator iters = tags.getBuckets().iterator(); // the max for "more" is 2 // the max for "less" is 4 diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/TermsDocCountErrorIT.java b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/TermsDocCountErrorIT.java index 9d5ca3afc5..9ed32ca2e7 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/TermsDocCountErrorIT.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/TermsDocCountErrorIT.java @@ -177,7 +177,7 @@ public class TermsDocCountErrorIT extends ESIntegTestCase { assertThat(testTerms, notNullValue()); assertThat(testTerms.getName(), equalTo("terms")); assertThat(testTerms.getDocCountError(), greaterThanOrEqualTo(0L)); - Collection testBuckets = testTerms.getBuckets(); + List testBuckets = testTerms.getBuckets(); assertThat(testBuckets.size(), lessThanOrEqualTo(size)); assertThat(accurateTerms.getBuckets().size(), greaterThanOrEqualTo(testBuckets.size())); @@ -211,7 +211,7 @@ public class TermsDocCountErrorIT extends ESIntegTestCase { assertThat(testTerms, notNullValue()); assertThat(testTerms.getName(), equalTo("terms")); assertThat(testTerms.getDocCountError(), equalTo(0L)); - Collection testBuckets = testTerms.getBuckets(); + List testBuckets = testTerms.getBuckets(); assertThat(testBuckets.size(), lessThanOrEqualTo(size)); assertThat(accurateTerms.getBuckets().size(), greaterThanOrEqualTo(testBuckets.size())); @@ -229,7 +229,7 @@ public class TermsDocCountErrorIT extends ESIntegTestCase { assertThat(testTerms, notNullValue()); assertThat(testTerms.getName(), equalTo("terms")); assertThat(testTerms.getDocCountError(), equalTo(0L)); - Collection testBuckets = testTerms.getBuckets(); + List testBuckets = testTerms.getBuckets(); assertThat(testBuckets.size(), lessThanOrEqualTo(size)); for (Terms.Bucket testBucket : testBuckets) { @@ -248,7 +248,7 @@ public class TermsDocCountErrorIT extends ESIntegTestCase { assertThat(testTerms, notNullValue()); assertThat(testTerms.getName(), equalTo("terms")); assertThat(testTerms.getDocCountError(),anyOf(equalTo(-1L), equalTo(0L))); - Collection testBuckets = testTerms.getBuckets(); + List testBuckets = testTerms.getBuckets(); assertThat(testBuckets.size(), lessThanOrEqualTo(size)); assertThat(accurateTerms.getBuckets().size(), greaterThanOrEqualTo(testBuckets.size())); @@ -988,7 +988,7 @@ public class TermsDocCountErrorIT extends ESIntegTestCase { Terms terms = response.getAggregations().get("terms"); assertThat(terms, notNullValue()); assertThat(terms.getDocCountError(), equalTo(46L)); - List buckets = terms.getBuckets(); + List buckets = terms.getBuckets(); assertThat(buckets, notNullValue()); assertThat(buckets.size(), equalTo(5)); diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregatorTests.java b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregatorTests.java index f2977fd769..1648d8ede9 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregatorTests.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregatorTests.java @@ -136,7 +136,7 @@ public class TermsAggregatorTests extends AggregatorTestCase { InternalAggregation mergedAggs = internalAgg.doReduce(aggs, ctx); assertTrue(mergedAggs instanceof DoubleTerms); long expected = numLongs + numDoubles; - List buckets = ((DoubleTerms) mergedAggs).getBuckets(); + List buckets = ((DoubleTerms) mergedAggs).getBuckets(); assertEquals(4, buckets.size()); assertEquals("1.0", buckets.get(0).getKeyAsString()); assertEquals(expected, buckets.get(0).getDocCount()); -- cgit v1.2.3