From: Jacek Date: Thu, 19 Mar 2020 14:20:28 +0000 (+0100) Subject: SONAR-13204 drop TermTopAggregationDef X-Git-Tag: 8.3.0.34182~69 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=b79678fc8390a9e0777c176d7bc3305ffbd63c94;p=sonarqube.git SONAR-13204 drop TermTopAggregationDef --- diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/es/searchrequest/SubAggregationHelper.java b/server/sonar-server-common/src/main/java/org/sonar/server/es/searchrequest/SubAggregationHelper.java index fb83c136caf..2641a373a1a 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/es/searchrequest/SubAggregationHelper.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/es/searchrequest/SubAggregationHelper.java @@ -62,12 +62,15 @@ public class SubAggregationHelper { this.order = order == null ? ORDER_BY_BUCKET_SIZE_DESC : order; } - public TermsAggregationBuilder buildTermsAggregation(String name, TermTopAggregationDef topAggregation) { + public TermsAggregationBuilder buildTermsAggregation(String name, + TopAggregationDefinition topAggregation, @Nullable Integer numberOfTerms) { TermsAggregationBuilder termsAggregation = AggregationBuilders.terms(name) .field(topAggregation.getFieldName()) .order(order) .minDocCount(TERM_AGGREGATION_MIN_DOC_COUNT); - topAggregation.getMaxTerms().ifPresent(termsAggregation::size); + if (numberOfTerms != null) { + termsAggregation.size(numberOfTerms); + } if (subAggregation != null) { termsAggregation = termsAggregation.subAggregation(subAggregation); } diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/es/searchrequest/TermTopAggregationDef.java b/server/sonar-server-common/src/main/java/org/sonar/server/es/searchrequest/TermTopAggregationDef.java deleted file mode 100644 index 10fc0d4401d..00000000000 --- a/server/sonar-server-common/src/main/java/org/sonar/server/es/searchrequest/TermTopAggregationDef.java +++ /dev/null @@ -1,55 +0,0 @@ -/* - * SonarQube - * Copyright (C) 2009-2020 SonarSource SA - * mailto:info AT sonarsource DOT com - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 3 of the License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - */ -package org.sonar.server.es.searchrequest; - -import java.util.OptionalInt; -import javax.annotation.Nullable; -import javax.annotation.concurrent.Immutable; - -import static com.google.common.base.Preconditions.checkArgument; - -/** - * A top-aggregation which defines (at least) one sub-aggregation on the terms of the fields of the top-aggregation. - */ -@Immutable -public class TermTopAggregationDef implements TopAggregationDefinition { - private final TopAggregationDef delegate; - private final Integer maxTerms; - - public TermTopAggregationDef(String fieldName, boolean sticky, @Nullable Integer maxTerms) { - this.delegate = new TopAggregationDef(fieldName, sticky); - checkArgument(maxTerms == null || maxTerms >= 0, "maxTerms can't be < 0"); - this.maxTerms = maxTerms; - } - - @Override - public String getFieldName() { - return delegate.getFieldName(); - } - - @Override - public boolean isSticky() { - return delegate.isSticky(); - } - - public OptionalInt getMaxTerms() { - return maxTerms == null ? OptionalInt.empty() : OptionalInt.of(maxTerms); - } -} diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/es/searchrequest/TopAggregationHelper.java b/server/sonar-server-common/src/main/java/org/sonar/server/es/searchrequest/TopAggregationHelper.java index 035ea059977..9202eae33c5 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/es/searchrequest/TopAggregationHelper.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/es/searchrequest/TopAggregationHelper.java @@ -20,6 +20,7 @@ package org.sonar.server.es.searchrequest; import java.util.function.Consumer; +import javax.annotation.Nullable; import org.elasticsearch.index.query.BoolQueryBuilder; import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.search.aggregations.AggregationBuilders; @@ -78,12 +79,13 @@ public class TopAggregationHelper { /** * Same as {@link #buildTopAggregation(String, TopAggregationDefinition, Consumer, Consumer)} with built-in addition of a - * top-term sub aggregation based field defined by {@link TermTopAggregationDef#getFieldName()}. + * top-term sub aggregation based field defined by {@link TopAggregationDefinition#getFieldName()}. */ - public FilterAggregationBuilder buildTermTopAggregation(String topAggregationName, TermTopAggregationDef topAggregation, + public FilterAggregationBuilder buildTermTopAggregation(String topAggregationName, + TopAggregationDefinition topAggregation, @Nullable Integer numberOfTerms, Consumer extraFilters, Consumer otherSubAggregations) { Consumer subAggregations = t -> { - t.subAggregation(subAggregationHelper.buildTermsAggregation(topAggregationName, topAggregation)); + t.subAggregation(subAggregationHelper.buildTermsAggregation(topAggregationName, topAggregation, numberOfTerms)); otherSubAggregations.accept(t); }; return buildTopAggregation(topAggregationName, topAggregation, extraFilters, subAggregations); diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/es/searchrequest/SubAggregationHelperTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/es/searchrequest/SubAggregationHelperTest.java index 3644c7b3d1f..201dc972635 100644 --- a/server/sonar-server-common/src/test/java/org/sonar/server/es/searchrequest/SubAggregationHelperTest.java +++ b/server/sonar-server-common/src/test/java/org/sonar/server/es/searchrequest/SubAggregationHelperTest.java @@ -45,13 +45,13 @@ public class SubAggregationHelperTest { @Test public void buildTermsAggregation_adds_term_subaggregation_with_minDoc_1_and_default_sort() { String aggName = randomAlphabetic(10); - TermTopAggregationDef topAggregation = new TermTopAggregationDef("bar", false, null); + TopAggregationDef topAggregation = new TopAggregationDef("bar", false); Stream.of( underTest, underTestWithCustomSubAgg) .forEach(t -> { - TermsAggregationBuilder agg = t.buildTermsAggregation(aggName, topAggregation); + TermsAggregationBuilder agg = t.buildTermsAggregation(aggName, topAggregation, null); assertThat(agg.getName()).isEqualTo(aggName); assertThat(agg.field()).isEqualTo(topAggregation.getFieldName()); @@ -64,9 +64,9 @@ public class SubAggregationHelperTest { @Test public void buildTermsAggregation_adds_custom_order_from_constructor() { String aggName = randomAlphabetic(10); - TermTopAggregationDef topAggregation = new TermTopAggregationDef("bar", false, null); + TopAggregationDef topAggregation = new TopAggregationDef("bar", false); - TermsAggregationBuilder agg = underTestWithCustomsSubAggAndOrder.buildTermsAggregation(aggName, topAggregation); + TermsAggregationBuilder agg = underTestWithCustomsSubAggAndOrder.buildTermsAggregation(aggName, topAggregation, null); assertThat(agg.getName()).isEqualTo(aggName); assertThat(agg.field()).isEqualTo(topAggregation.getFieldName()); @@ -76,13 +76,13 @@ public class SubAggregationHelperTest { @Test public void buildTermsAggregation_adds_custom_sub_agg_from_constructor() { String aggName = randomAlphabetic(10); - TermTopAggregationDef topAggregation = new TermTopAggregationDef("bar", false, null); + TopAggregationDef topAggregation = new TopAggregationDef("bar", false); Stream.of( underTestWithCustomSubAgg, underTestWithCustomsSubAggAndOrder) .forEach(t -> { - TermsAggregationBuilder agg = t.buildTermsAggregation(aggName, topAggregation); + TermsAggregationBuilder agg = t.buildTermsAggregation(aggName, topAggregation, null); assertThat(agg.getName()).isEqualTo(aggName); assertThat(agg.field()).isEqualTo(topAggregation.getFieldName()); @@ -95,14 +95,14 @@ public class SubAggregationHelperTest { public void buildTermsAggregation_adds_custom_size_if_TermTopAggregation_specifies_one() { String aggName = randomAlphabetic(10); int customSize = 1 + new Random().nextInt(400); - TermTopAggregationDef topAggregation = new TermTopAggregationDef("bar", false, customSize); + TopAggregationDef topAggregation = new TopAggregationDef("bar", false); Stream.of( underTest, underTestWithCustomSubAgg, underTestWithCustomsSubAggAndOrder) .forEach(t -> { - TermsAggregationBuilder agg = t.buildTermsAggregation(aggName, topAggregation); + TermsAggregationBuilder agg = t.buildTermsAggregation(aggName, topAggregation, customSize); assertThat(agg.getName()).isEqualTo(aggName); assertThat(agg.field()).isEqualTo(topAggregation.getFieldName()); @@ -113,7 +113,7 @@ public class SubAggregationHelperTest { @Test public void buildSelectedItemsAggregation_returns_empty_if_no_selected_item() { String aggName = randomAlphabetic(10); - TermTopAggregationDef topAggregation = new TermTopAggregationDef("bar", false, null); + TopAggregationDef topAggregation = new TopAggregationDef("bar", false); Stream.of( underTest, @@ -125,7 +125,7 @@ public class SubAggregationHelperTest { @Test public void buildSelectedItemsAggregation_does_not_add_custom_order_from_constructor() { String aggName = randomAlphabetic(10); - TermTopAggregationDef topAggregation = new TermTopAggregationDef("bar", false, null); + TopAggregationDef topAggregation = new TopAggregationDef("bar", false); String[] selected = randomNonEmptySelected(); TermsAggregationBuilder agg = underTestWithCustomsSubAggAndOrder.buildSelectedItemsAggregation(aggName, topAggregation, selected) @@ -139,7 +139,7 @@ public class SubAggregationHelperTest { @Test public void buildSelectedItemsAggregation_adds_custom_sub_agg_from_constructor() { String aggName = randomAlphabetic(10); - TermTopAggregationDef topAggregation = new TermTopAggregationDef("bar", false, null); + TopAggregationDef topAggregation = new TopAggregationDef("bar", false); String[] selected = randomNonEmptySelected(); Stream.of( diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/es/searchrequest/TopAggregationHelperTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/es/searchrequest/TopAggregationHelperTest.java index b020b01d3f2..ba62eee7296 100644 --- a/server/sonar-server-common/src/test/java/org/sonar/server/es/searchrequest/TopAggregationHelperTest.java +++ b/server/sonar-server-common/src/test/java/org/sonar/server/es/searchrequest/TopAggregationHelperTest.java @@ -145,11 +145,12 @@ public class TopAggregationHelperTest { @Test public void buildTermTopAggregation_adds_term_subaggregation_from_subAggregationHelper() { String topAggregationName = randomAlphabetic(10); - TermTopAggregationDef topAggregation = new TermTopAggregationDef("bar", false, null); + TopAggregationDef topAggregation = new TopAggregationDef("bar", false); TermsAggregationBuilder termSubAgg = AggregationBuilders.terms("foo"); - when(subAggregationHelper.buildTermsAggregation(topAggregationName, topAggregation)).thenReturn(termSubAgg); + when(subAggregationHelper.buildTermsAggregation(topAggregationName, topAggregation, null)).thenReturn(termSubAgg); - FilterAggregationBuilder aggregationBuilder = underTest.buildTermTopAggregation(topAggregationName, topAggregation, + FilterAggregationBuilder aggregationBuilder = underTest.buildTermTopAggregation( + topAggregationName, topAggregation, null, NO_EXTRA_FILTER, NO_OTHER_SUBAGGREGATION); assertThat(aggregationBuilder.getName()).isEqualTo(topAggregationName); @@ -159,16 +160,17 @@ public class TopAggregationHelperTest { @Test public void buildTermTopAggregation_adds_subAggregation_from_lambda_parameter() { - TermTopAggregationDef topAggregation = new TermTopAggregationDef("bar", false, null); + TopAggregationDef topAggregation = new TopAggregationDef("bar", false); AggregationBuilder[] subAggs = IntStream.range(0, 1 + new Random().nextInt(12)) .mapToObj(i -> AggregationBuilders.min("subAgg_" + i)) .toArray(AggregationBuilder[]::new); String topAggregationName = randomAlphabetic(10); TermsAggregationBuilder termSubAgg = AggregationBuilders.terms("foo"); - when(subAggregationHelper.buildTermsAggregation(topAggregationName, topAggregation)).thenReturn(termSubAgg); + when(subAggregationHelper.buildTermsAggregation(topAggregationName, topAggregation, null)).thenReturn(termSubAgg); AggregationBuilder[] allSubAggs = Stream.concat(Arrays.stream(subAggs), Stream.of(termSubAgg)).toArray(AggregationBuilder[]::new); - AggregationBuilder aggregationBuilder = underTest.buildTermTopAggregation(topAggregationName, topAggregation, + AggregationBuilder aggregationBuilder = underTest.buildTermTopAggregation( + topAggregationName, topAggregation, null, NO_EXTRA_FILTER, t -> Arrays.stream(subAggs).forEach(t::subAggregation)); assertThat(aggregationBuilder.getName()).isEqualTo(topAggregationName); @@ -178,17 +180,18 @@ public class TopAggregationHelperTest { @Test public void buildTermTopAggregation_adds_filter_from_FiltersComputer_for_TopAggregation() { - TermTopAggregationDef topAggregation = new TermTopAggregationDef("bar", false, null); - TermTopAggregationDef otherTopAggregation = new TermTopAggregationDef("acme", false, null); + TopAggregationDef topAggregation = new TopAggregationDef("bar", false); + TopAggregationDef otherTopAggregation = new TopAggregationDef("acme", false); BoolQueryBuilder computerFilter = boolQuery(); BoolQueryBuilder otherFilter = boolQuery(); when(filtersComputer.getTopAggregationFilter(topAggregation)).thenReturn(Optional.of(computerFilter)); when(filtersComputer.getTopAggregationFilter(otherTopAggregation)).thenReturn(Optional.of(otherFilter)); String topAggregationName = randomAlphabetic(10); TermsAggregationBuilder termSubAgg = AggregationBuilders.terms("foo"); - when(subAggregationHelper.buildTermsAggregation(topAggregationName, topAggregation)).thenReturn(termSubAgg); + when(subAggregationHelper.buildTermsAggregation(topAggregationName, topAggregation, null)).thenReturn(termSubAgg); - FilterAggregationBuilder aggregationBuilder = underTest.buildTermTopAggregation(topAggregationName, topAggregation, + FilterAggregationBuilder aggregationBuilder = underTest.buildTermTopAggregation( + topAggregationName, topAggregation, null, NO_EXTRA_FILTER, NO_OTHER_SUBAGGREGATION); assertThat(aggregationBuilder.getName()).isEqualTo(topAggregationName); @@ -197,16 +200,17 @@ public class TopAggregationHelperTest { @Test public void buildTermTopAggregation_has_empty_filter_when_FiltersComputer_returns_empty_for_TopAggregation() { - TermTopAggregationDef topAggregation = new TermTopAggregationDef("bar", false, null); - TermTopAggregationDef otherTopAggregation = new TermTopAggregationDef("acme", false, null); + TopAggregationDef topAggregation = new TopAggregationDef("bar", false); + TopAggregationDef otherTopAggregation = new TopAggregationDef("acme", false); BoolQueryBuilder otherFilter = boolQuery(); when(filtersComputer.getTopAggregationFilter(topAggregation)).thenReturn(Optional.empty()); when(filtersComputer.getTopAggregationFilter(otherTopAggregation)).thenReturn(Optional.of(otherFilter)); String topAggregationName = randomAlphabetic(10); TermsAggregationBuilder termSubAgg = AggregationBuilders.terms("foo"); - when(subAggregationHelper.buildTermsAggregation(topAggregationName, topAggregation)).thenReturn(termSubAgg); + when(subAggregationHelper.buildTermsAggregation(topAggregationName, topAggregation, null)).thenReturn(termSubAgg); - FilterAggregationBuilder aggregationBuilder = underTest.buildTermTopAggregation(topAggregationName, topAggregation, + FilterAggregationBuilder aggregationBuilder = underTest.buildTermTopAggregation( + topAggregationName, topAggregation, null, NO_EXTRA_FILTER, NO_OTHER_SUBAGGREGATION); assertThat(aggregationBuilder.getName()).isEqualTo(topAggregationName); @@ -216,17 +220,18 @@ public class TopAggregationHelperTest { @Test public void buildTermTopAggregation_adds_filter_from_FiltersComputer_for_TopAggregation_and_extra_one() { String topAggregationName = randomAlphabetic(10); - TermTopAggregationDef topAggregation = new TermTopAggregationDef("bar", false, null); - TermTopAggregationDef otherTopAggregation = new TermTopAggregationDef("acme", false, null); + TopAggregationDef topAggregation = new TopAggregationDef("bar", false); + TopAggregationDef otherTopAggregation = new TopAggregationDef("acme", false); BoolQueryBuilder computerFilter = boolQuery(); BoolQueryBuilder otherFilter = boolQuery(); BoolQueryBuilder extraFilter = boolQuery(); when(filtersComputer.getTopAggregationFilter(topAggregation)).thenReturn(Optional.of(computerFilter)); when(filtersComputer.getTopAggregationFilter(otherTopAggregation)).thenReturn(Optional.of(otherFilter)); TermsAggregationBuilder termSubAgg = AggregationBuilders.terms("foo"); - when(subAggregationHelper.buildTermsAggregation(topAggregationName, topAggregation)).thenReturn(termSubAgg); + when(subAggregationHelper.buildTermsAggregation(topAggregationName, topAggregation, null)).thenReturn(termSubAgg); - FilterAggregationBuilder aggregationBuilder = underTest.buildTermTopAggregation(topAggregationName, topAggregation, + FilterAggregationBuilder aggregationBuilder = underTest.buildTermTopAggregation( + topAggregationName, topAggregation, null, t -> t.must(extraFilter), NO_OTHER_SUBAGGREGATION); assertThat(aggregationBuilder.getName()).isEqualTo(topAggregationName); diff --git a/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueIndex.java b/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueIndex.java index 3713c2a3642..37dca8421b9 100644 --- a/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueIndex.java +++ b/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueIndex.java @@ -81,7 +81,6 @@ import org.sonar.server.es.Sorting; import org.sonar.server.es.searchrequest.RequestFiltersComputer; import org.sonar.server.es.searchrequest.RequestFiltersComputer.AllFilters; import org.sonar.server.es.searchrequest.SubAggregationHelper; -import org.sonar.server.es.searchrequest.TermTopAggregationDef; import org.sonar.server.es.searchrequest.TopAggregationDef; import org.sonar.server.es.searchrequest.TopAggregationDefinition; import org.sonar.server.es.searchrequest.TopAggregationHelper; @@ -248,15 +247,18 @@ public class IssueIndex { private final String name; private final TopAggregationDefinition topAggregation; + private final Integer numberOfTerms; - Facet(String name, String fieldName, boolean sticky, int size) { + Facet(String name, String fieldName, boolean sticky, int numberOfTerms) { this.name = name; - this.topAggregation = new TermTopAggregationDef(fieldName, sticky, size); + this.topAggregation = new TopAggregationDef(fieldName, sticky); + this.numberOfTerms = numberOfTerms; } Facet(String name, String fieldName, boolean sticky) { this.name = name; this.topAggregation = new TopAggregationDef(fieldName, sticky); + this.numberOfTerms = null; } public String getName() { @@ -271,8 +273,10 @@ public class IssueIndex { return topAggregation; } - public TermTopAggregationDef getTermTopAggregationDef() { - return (TermTopAggregationDef) topAggregation; + public int getNumberOfTerms() { + checkState(numberOfTerms != null, "numberOfTerms should have been provided in constructor"); + + return numberOfTerms; } } @@ -656,9 +660,9 @@ public class IssueIndex { } FilterAggregationBuilder topAggregation = aggregationHelper.buildTermTopAggregation( - facet.getName(), facet.getTermTopAggregationDef(), + facet.getName(), facet.getTopAggregationDef(), facet.getNumberOfTerms(), NO_EXTRA_FILTER, - t -> aggregationHelper.getSubAggregationHelper().buildSelectedItemsAggregation(facet.getName(), facet.getTermTopAggregationDef(), selectedValues) + t -> aggregationHelper.getSubAggregationHelper().buildSelectedItemsAggregation(facet.getName(), facet.getTopAggregationDef(), selectedValues) .ifPresent(t::subAggregation)); esRequest.addAggregation(topAggregation); } @@ -669,7 +673,7 @@ public class IssueIndex { } AggregationBuilder aggregation = aggregationHelper.buildTermTopAggregation( - SEVERITIES.getName(), SEVERITIES.getTermTopAggregationDef(), + SEVERITIES.getName(), SEVERITIES.getTopAggregationDef(), SEVERITIES.getNumberOfTerms(), // Ignore severity of Security HotSpots filter -> filter.mustNot(termQuery(FIELD_ISSUE_TYPE, SECURITY_HOTSPOT.name())), NO_OTHER_SUBAGGREGATION); @@ -682,7 +686,7 @@ public class IssueIndex { } AggregationBuilder aggregation = aggregationHelper.buildTermTopAggregation( - RESOLUTIONS.getName(), RESOLUTIONS.getTermTopAggregationDef(), + RESOLUTIONS.getName(), RESOLUTIONS.getTopAggregationDef(), RESOLUTIONS.getNumberOfTerms(), NO_EXTRA_FILTER, t -> { // add aggregation of type "missing" to return count of unresolved issues in the facet @@ -712,7 +716,8 @@ public class IssueIndex { }; AggregationBuilder aggregation = aggregationHelper.buildTermTopAggregation( - ASSIGNEES.getName(), ASSIGNEES.getTermTopAggregationDef(), NO_EXTRA_FILTER, assigneeAggregations); + ASSIGNEES.getName(), ASSIGNEES.getTopAggregationDef(), ASSIGNEES.getNumberOfTerms(), + NO_EXTRA_FILTER, assigneeAggregations); esRequest.addAggregation(aggregation); } @@ -802,11 +807,13 @@ public class IssueIndex { if (options.getFacets().contains(ASSIGNED_TO_ME.getName()) && !StringUtils.isEmpty(uuid)) { AggregationBuilder aggregation = aggregationHelper.buildTermTopAggregation( ASSIGNED_TO_ME.getName(), - ASSIGNED_TO_ME.getTermTopAggregationDef(), + ASSIGNED_TO_ME.getTopAggregationDef(), + ASSIGNED_TO_ME.getNumberOfTerms(), NO_EXTRA_FILTER, t -> { // add sub-aggregation to return issue count for current user - aggregationHelper.getSubAggregationHelper().buildSelectedItemsAggregation(ASSIGNED_TO_ME.getName(), ASSIGNED_TO_ME.getTermTopAggregationDef(), new String[] {uuid}) + aggregationHelper.getSubAggregationHelper() + .buildSelectedItemsAggregation(ASSIGNED_TO_ME.getName(), ASSIGNED_TO_ME.getTopAggregationDef(), new String[] {uuid}) .ifPresent(t::subAggregation); }); esRequest.addAggregation(aggregation);