From: Stephane Gamard Date: Fri, 3 Oct 2014 07:42:31 +0000 (+0200) Subject: SONAR-5634 - Implemented facets as sticky (multi-select) facets for RuleIndex X-Git-Tag: 4.5.1-RC1~118 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=928ef1264282f09bd869191f7f0d83b5f4235f0d;p=sonarqube.git SONAR-5634 - Implemented facets as sticky (multi-select) facets for RuleIndex Conflicts: server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleIndex.java server/sonar-server/src/main/java/org/sonar/server/search/Result.java --- diff --git a/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleIndex.java b/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleIndex.java index 53b6ca26d24..3207f4b5f9a 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleIndex.java +++ b/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleIndex.java @@ -20,6 +20,8 @@ package org.sonar.server.rule.index; import com.google.common.base.Preconditions; +import org.apache.commons.collections.CollectionUtils; +import org.apache.commons.lang.StringUtils; import org.elasticsearch.action.search.SearchRequestBuilder; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.action.search.SearchScrollRequestBuilder; @@ -36,6 +38,7 @@ import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.index.query.SimpleQueryStringBuilder; import org.elasticsearch.search.SearchHit; +import org.elasticsearch.search.aggregations.AggregationBuilder; import org.elasticsearch.search.aggregations.AggregationBuilders; import org.elasticsearch.search.aggregations.bucket.terms.Terms; import org.elasticsearch.search.sort.FieldSortBuilder; @@ -68,6 +71,10 @@ import static com.google.common.collect.Lists.newArrayList; public class RuleIndex extends BaseIndex { + public static final String FACET_LANGUAGES = "languages"; + public static final String FACET_TAGS = "tags"; + public static final String FACET_REPOSITORIES = "repositories"; + public RuleIndex(RuleNormalizer normalizer, SearchClient client) { super(IndexDefinition.RULE, normalizer, client); } @@ -120,13 +127,6 @@ public class RuleIndex extends BaseIndex { esSearch.setFetchSource(fields.toArray(new String[fields.size()]), null); } - private void setFacets(QueryOptions options, SearchRequestBuilder esSearch) { - /* Integrate Facets */ - if (options.isFacet()) { - this.setFacets(esSearch); - } - } - private void setSorting(RuleQuery query, SearchRequestBuilder esSearch) { /* integrate Query Sort */ String queryText = query.getQueryText(); @@ -202,62 +202,92 @@ public class RuleIndex extends BaseIndex { } /* Build main filter (match based) */ - protected FilterBuilder getFilter(RuleQuery query, QueryOptions options) { + protected HashMap getFilters(RuleQuery query, QueryOptions options) { - BoolFilterBuilder fb = FilterBuilders.boolFilter(); + HashMap filters = new HashMap(); /* Add enforced filter on rules that are REMOVED */ - fb.mustNot(FilterBuilders - .termFilter(RuleNormalizer.RuleField.STATUS.field(), - RuleStatus.REMOVED.toString())); - - this.addTermFilter(fb, RuleNormalizer.RuleField.INTERNAL_KEY.field(), query.getInternalKey()); - this.addTermFilter(fb, RuleNormalizer.RuleField.RULE_KEY.field(), query.getRuleKey()); - this.addTermFilter(fb, RuleNormalizer.RuleField.LANGUAGE.field(), query.getLanguages()); - this.addTermFilter(fb, RuleNormalizer.RuleField.REPOSITORY.field(), query.getRepositories()); - this.addTermFilter(fb, RuleNormalizer.RuleField.SEVERITY.field(), query.getSeverities()); - this.addTermFilter(fb, RuleNormalizer.RuleField.KEY.field(), query.getKey()); - this.addTermFilter(fb, RuleNormalizer.RuleField._TAGS.field(), query.getTags()); + filters.put(RuleNormalizer.RuleField.STATUS.field(), + FilterBuilders.boolFilter().mustNot( + FilterBuilders.termFilter(RuleNormalizer.RuleField.STATUS.field(), + RuleStatus.REMOVED.toString()))); + + if (!StringUtils.isEmpty(query.getInternalKey())) { + filters.put(RuleNormalizer.RuleField.INTERNAL_KEY.field(), + FilterBuilders.termFilter(RuleNormalizer.RuleField.INTERNAL_KEY.field(), query.getInternalKey())); + } + + if (!StringUtils.isEmpty(query.getRuleKey())) { + filters.put(RuleNormalizer.RuleField.RULE_KEY.field(), + FilterBuilders.termFilter(RuleNormalizer.RuleField.RULE_KEY.field(), query.getRuleKey())); + } + + if (!CollectionUtils.isEmpty(query.getLanguages())) { + filters.put(RuleNormalizer.RuleField.LANGUAGE.field(), + FilterBuilders.termsFilter(RuleNormalizer.RuleField.LANGUAGE.field(), query.getLanguages())); + } + + if (!CollectionUtils.isEmpty(query.getRepositories())) { + filters.put(RuleNormalizer.RuleField.REPOSITORY.field(), + FilterBuilders.termsFilter(RuleNormalizer.RuleField.REPOSITORY.field(), query.getRepositories())); + } + + if (!CollectionUtils.isEmpty(query.getSeverities())) { + filters.put(RuleNormalizer.RuleField.SEVERITY.field(), + FilterBuilders.termsFilter(RuleNormalizer.RuleField.SEVERITY.field(), query.getSeverities())); + } + + if (!StringUtils.isEmpty(query.getKey())) { + filters.put(RuleNormalizer.RuleField.KEY.field(), + FilterBuilders.termFilter(RuleNormalizer.RuleField.KEY.field(), query.getKey())); + } + + if (!CollectionUtils.isEmpty(query.getTags())) { + filters.put(RuleNormalizer.RuleField._TAGS.field(), + FilterBuilders.termsFilter(RuleNormalizer.RuleField._TAGS.field(), query.getTags())); + } // Construct the debt filter on effective char and subChar Collection debtCharacteristics = query.getDebtCharacteristics(); if (debtCharacteristics != null && !debtCharacteristics.isEmpty()) { - fb.must( - FilterBuilders.orFilter( - // Match only when NONE (overridden) - FilterBuilders.andFilter( - FilterBuilders.notFilter( - FilterBuilders.termsFilter(RuleNormalizer.RuleField.SUB_CHARACTERISTIC.field(), DebtCharacteristic.NONE)), - FilterBuilders.orFilter( - FilterBuilders.termsFilter(RuleNormalizer.RuleField.SUB_CHARACTERISTIC.field(), debtCharacteristics), - FilterBuilders.termsFilter(RuleNormalizer.RuleField.CHARACTERISTIC.field(), debtCharacteristics)) - ), - - // Match only when NOT NONE (not overridden) - FilterBuilders.andFilter( - FilterBuilders.orFilter( - FilterBuilders.termsFilter(RuleNormalizer.RuleField.SUB_CHARACTERISTIC.field(), ""), - FilterBuilders.notFilter(FilterBuilders.existsFilter(RuleNormalizer.RuleField.SUB_CHARACTERISTIC.field()))), - FilterBuilders.orFilter( - FilterBuilders.termsFilter(RuleNormalizer.RuleField.DEFAULT_SUB_CHARACTERISTIC.field(), debtCharacteristics), - FilterBuilders.termsFilter(RuleNormalizer.RuleField.DEFAULT_CHARACTERISTIC.field(), debtCharacteristics))) - ) - ); + filters.put("debtCharacteristics", + FilterBuilders.boolFilter().must( + FilterBuilders.orFilter( + // Match only when NONE (overridden) + FilterBuilders.andFilter( + FilterBuilders.notFilter( + FilterBuilders.termsFilter(RuleNormalizer.RuleField.SUB_CHARACTERISTIC.field(), DebtCharacteristic.NONE)), + FilterBuilders.orFilter( + FilterBuilders.termsFilter(RuleNormalizer.RuleField.SUB_CHARACTERISTIC.field(), debtCharacteristics), + FilterBuilders.termsFilter(RuleNormalizer.RuleField.CHARACTERISTIC.field(), debtCharacteristics)) + ), + + // Match only when NOT NONE (not overridden) + FilterBuilders.andFilter( + FilterBuilders.orFilter( + FilterBuilders.termsFilter(RuleNormalizer.RuleField.SUB_CHARACTERISTIC.field(), ""), + FilterBuilders.notFilter(FilterBuilders.existsFilter(RuleNormalizer.RuleField.SUB_CHARACTERISTIC.field()))), + FilterBuilders.orFilter( + FilterBuilders.termsFilter(RuleNormalizer.RuleField.DEFAULT_SUB_CHARACTERISTIC.field(), debtCharacteristics), + FilterBuilders.termsFilter(RuleNormalizer.RuleField.DEFAULT_CHARACTERISTIC.field(), debtCharacteristics))) + ) + )); } // Debt char exist filter Boolean hasDebtCharacteristic = query.getHasDebtCharacteristic(); if (hasDebtCharacteristic != null && hasDebtCharacteristic) { - fb.mustNot( - FilterBuilders.termsFilter(RuleNormalizer.RuleField.SUB_CHARACTERISTIC.field(), DebtCharacteristic.NONE)) - .should( - FilterBuilders.existsFilter(RuleNormalizer.RuleField.SUB_CHARACTERISTIC.field())) - .should( - FilterBuilders.existsFilter(RuleNormalizer.RuleField.DEFAULT_SUB_CHARACTERISTIC.field())); + filters.put("hasDebtCharacteristic", + FilterBuilders.boolFilter().mustNot( + FilterBuilders.termsFilter(RuleNormalizer.RuleField.SUB_CHARACTERISTIC.field(), DebtCharacteristic.NONE)) + .should( + FilterBuilders.existsFilter(RuleNormalizer.RuleField.SUB_CHARACTERISTIC.field())) + .should( + FilterBuilders.existsFilter(RuleNormalizer.RuleField.DEFAULT_SUB_CHARACTERISTIC.field()))); } if (query.getAvailableSince() != null) { - fb.must(FilterBuilders.rangeFilter(RuleNormalizer.RuleField.CREATED_AT.field()) + filters.put("availableSince", FilterBuilders.rangeFilter(RuleNormalizer.RuleField.CREATED_AT.field()) .gte(query.getAvailableSince())); } @@ -267,17 +297,20 @@ public class RuleIndex extends BaseIndex { for (RuleStatus status : statusValues) { stringStatus.add(status.name()); } - this.addTermFilter(fb, RuleNormalizer.RuleField.STATUS.field(), stringStatus); + filters.put(RuleNormalizer.RuleField.STATUS.field(), + FilterBuilders.termsFilter(RuleNormalizer.RuleField.STATUS.field(), stringStatus)); } Boolean isTemplate = query.isTemplate(); if (isTemplate != null) { - this.addTermFilter(fb, RuleNormalizer.RuleField.IS_TEMPLATE.field(), Boolean.toString(isTemplate)); + filters.put(RuleNormalizer.RuleField.IS_TEMPLATE.field(), + FilterBuilders.termFilter(RuleNormalizer.RuleField.IS_TEMPLATE.field(), Boolean.toString(isTemplate))); } String template = query.templateKey(); if (template != null) { - this.addTermFilter(fb, RuleNormalizer.RuleField.TEMPLATE_KEY.field(), template); + filters.put(RuleNormalizer.RuleField.TEMPLATE_KEY.field(), + FilterBuilders.termFilter(RuleNormalizer.RuleField.TEMPLATE_KEY.field(), template)); } // ActiveRule Filter (profile and inheritance) @@ -296,41 +329,86 @@ public class RuleIndex extends BaseIndex { /** Implementation of activation query */ if (Boolean.TRUE.equals(query.getActivation())) { - fb.must(FilterBuilders.hasChildFilter(IndexDefinition.ACTIVE_RULE.getIndexType(), - childQuery)); + filters.put("activation", + FilterBuilders.hasChildFilter(IndexDefinition.ACTIVE_RULE.getIndexType(), + childQuery)); } else if (Boolean.FALSE.equals(query.getActivation())) { - fb.mustNot(FilterBuilders.hasChildFilter(IndexDefinition.ACTIVE_RULE.getIndexType(), - childQuery)); + filters.put("activation", + FilterBuilders.boolFilter().mustNot( + FilterBuilders.hasChildFilter(IndexDefinition.ACTIVE_RULE.getIndexType(), + childQuery))); } - return fb; + return filters; } - protected void setFacets(SearchRequestBuilder query) { + protected Map getFacets(QueryBuilder query, HashMap filters) { + Map aggregations = new HashMap(); + BoolFilterBuilder langFacetFilter = FilterBuilders.boolFilter();// .must(FilterBuilders.queryFilter(query)); + for (Map.Entry filter : filters.entrySet()) { + if (filter.getKey() != RuleNormalizer.RuleField.LANGUAGE.field()) { + langFacetFilter.must(filter.getValue()); + } + } /* the Lang facet */ - query.addAggregation(AggregationBuilders - .terms("languages") - .field(RuleNormalizer.RuleField.LANGUAGE.field()) - .order(Terms.Order.count(false)) - .size(10) - .minDocCount(1)); - + aggregations.put(FACET_LANGUAGES + "global", + AggregationBuilders + .global(FACET_LANGUAGES) + .subAggregation( + AggregationBuilders + .filter(FACET_LANGUAGES + "_filter") + .filter(langFacetFilter) + .subAggregation( + AggregationBuilders.terms(FACET_LANGUAGES) + .field(RuleNormalizer.RuleField.LANGUAGE.field()) + .order(Terms.Order.count(false)) + .size(10) + .minDocCount(1)))); + + BoolFilterBuilder tagsFacetFilter = FilterBuilders.boolFilter();// .must(FilterBuilders.queryFilter(query)); + for (Map.Entry filter : filters.entrySet()) { + if (filter.getKey() != RuleNormalizer.RuleField._TAGS.field()) { + tagsFacetFilter.must(filter.getValue()); + } + } /* the Tag facet */ - query.addAggregation(AggregationBuilders - .terms("tags") - .field(RuleNormalizer.RuleField._TAGS.field()) - .order(Terms.Order.count(false)) - .size(10) - .minDocCount(1)); - + aggregations.put(FACET_TAGS + "global", + AggregationBuilders + .global(FACET_TAGS) + .subAggregation( + AggregationBuilders + .filter(FACET_TAGS + "_filter") + .filter(tagsFacetFilter) + .subAggregation( + AggregationBuilders.terms(FACET_TAGS) + .field(RuleNormalizer.RuleField._TAGS.field()) + .order(Terms.Order.count(false)) + .size(10) + .minDocCount(1)))); + + BoolFilterBuilder repositoriesFacetFilter = FilterBuilders.boolFilter();// .must(FilterBuilders.queryFilter(query)); + for (Map.Entry filter : filters.entrySet()) { + if (filter.getKey() != RuleNormalizer.RuleField.REPOSITORY.field()) { + repositoriesFacetFilter.must(filter.getValue()); + } + } /* the Repo facet */ - query.addAggregation(AggregationBuilders - .terms("repositories") - .field(RuleNormalizer.RuleField.REPOSITORY.field()) - .order(Terms.Order.count(false)) - .size(10) - .minDocCount(1)); + aggregations.put(FACET_REPOSITORIES + "global", + AggregationBuilders + .global(FACET_REPOSITORIES) + .subAggregation( + AggregationBuilders + .filter(FACET_REPOSITORIES + "_filter") + .filter(repositoriesFacetFilter) + .subAggregation( + AggregationBuilders.terms(FACET_REPOSITORIES) + .field(RuleNormalizer.RuleField.REPOSITORY.field()) + .order(Terms.Order.count(false)) + .size(10) + .minDocCount(1)))); + + return aggregations; } @@ -345,15 +423,25 @@ public class RuleIndex extends BaseIndex { esSearch.setScroll(TimeValue.timeValueMinutes(3)); } - setFacets(options, esSearch); + QueryBuilder qb = this.getQuery(query, options); + HashMap filters = this.getFilters(query, options); + + if (options.isFacet()) { + for (AggregationBuilder aggregation : getFacets(qb, filters).values()) { + esSearch.addAggregation(aggregation); + } + } + setSorting(query, esSearch); setPagination(options, esSearch); setFields(options, esSearch); - FilterBuilder fb = this.getFilter(query, options); - QueryBuilder qb = this.getQuery(query, options); - esSearch.setQuery(QueryBuilders.filteredQuery(qb, fb)); + BoolFilterBuilder fb = FilterBuilders.boolFilter(); + for (FilterBuilder ffb : filters.values()) { + fb.must(ffb); + } + esSearch.setQuery(QueryBuilders.filteredQuery(qb, fb)); SearchResponse esResult = getClient().execute(esSearch); return new Result(this, esResult); diff --git a/server/sonar-server/src/main/java/org/sonar/server/search/Result.java b/server/sonar-server/src/main/java/org/sonar/server/search/Result.java index 85a32c26cf4..6c39b47bf1d 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/search/Result.java +++ b/server/sonar-server/src/main/java/org/sonar/server/search/Result.java @@ -26,12 +26,16 @@ import org.apache.commons.lang.builder.ReflectionToStringBuilder; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.search.SearchHit; import org.elasticsearch.search.aggregations.Aggregation; +import org.elasticsearch.search.aggregations.HasAggregations; import org.elasticsearch.search.aggregations.bucket.terms.Terms; import javax.annotation.CheckForNull; import javax.annotation.Nullable; - -import java.util.*; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Iterator; +import java.util.List; +import java.util.Map; public class Result { @@ -60,12 +64,25 @@ public class Result { } if (response.getAggregations() != null) { for (Map.Entry facet : response.getAggregations().asMap().entrySet()) { - Terms aggregation = (Terms) facet.getValue(); - for (Terms.Bucket value : aggregation.getBuckets()) { - this.facets.put(facet.getKey(), new FacetValue(value.getKey(), (int) value.getDocCount())); - } + this.processAggregation(facet.getValue()); + } + } + } + + private void processAggregation(Aggregation aggregation) { + if (Terms.class.isAssignableFrom(aggregation.getClass())) { + Terms termAggregation = (Terms) aggregation; + for (Terms.Bucket value : termAggregation.getBuckets()) { + this.facets.put(aggregation.getName(), new FacetValue(value.getKey(), (int) value.getDocCount())); } + } else if (HasAggregations.class.isAssignableFrom(aggregation.getClass())) { + HasAggregations hasAggregations = (HasAggregations) aggregation; + for (Aggregation internalAggregation : ((HasAggregations) aggregation).getAggregations()) + this.processAggregation(internalAggregation); + } else { + ; // Ignore } + } public Iterator scroll() { diff --git a/server/sonar-server/src/test/java/org/sonar/server/rule/index/RuleIndexMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/rule/index/RuleIndexMediumTest.java index 67a74ecb90e..d385273a4ea 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/rule/index/RuleIndexMediumTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/rule/index/RuleIndexMediumTest.java @@ -20,6 +20,7 @@ package org.sonar.server.rule.index; import com.google.common.base.Function; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.Sets; @@ -45,6 +46,7 @@ import org.sonar.server.search.QueryOptions; import org.sonar.server.search.Result; import javax.annotation.Nullable; + import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -52,6 +54,7 @@ import java.util.Date; import java.util.HashSet; import java.util.Iterator; import java.util.List; +import java.util.Map; import java.util.Set; import static com.google.common.collect.Lists.newArrayList; @@ -941,6 +944,71 @@ public class RuleIndexMediumTest extends SearchMediumTest { assertThat(results.get(0).key()).isEqualTo(RuleTesting.XOO_X1); } + @Test + public void sticky_facets() { + + Integer numberOfSystemTags = 2; + dao.insert(dbSession, + RuleTesting.newDto(RuleKey.of("xoo", "S001")).setLanguage("java").setTags(ImmutableSet.of()), + RuleTesting.newDto(RuleKey.of("xoo", "S002")).setLanguage("java").setTags(ImmutableSet.of()), + RuleTesting.newDto(RuleKey.of("xoo", "S003")).setLanguage("java").setTags(ImmutableSet.of("T1", "T2")), + RuleTesting.newDto(RuleKey.of("xoo", "S011")).setLanguage("cobol").setTags(ImmutableSet.of()), + RuleTesting.newDto(RuleKey.of("xoo", "S012")).setLanguage("cobol").setTags(ImmutableSet.of()), + RuleTesting.newDto(RuleKey.of("foo", "S013")).setLanguage("cobol").setTags(ImmutableSet.of("T3", "T4")), + RuleTesting.newDto(RuleKey.of("foo", "S111")).setLanguage("cpp").setTags(ImmutableSet.of()), + RuleTesting.newDto(RuleKey.of("foo", "S112")).setLanguage("cpp").setTags(ImmutableSet.of()), + RuleTesting.newDto(RuleKey.of("foo", "S113")).setLanguage("cpp").setTags(ImmutableSet.of("T2", "T3"))); + dbSession.commit(); + + // 0 assert Base + assertThat(index.countAll()).isEqualTo(9); + assertThat(index.search(new RuleQuery(), new QueryOptions()).getHits()).hasSize(9); + + // 1 Facet with no filters at all + Map> facets = index.search(new RuleQuery(), new QueryOptions().setFacet(true)).getFacets(); + assertThat(facets.keySet()).hasSize(3); + assertThat(facets.get(RuleIndex.FACET_LANGUAGES)).hasSize(3); + assertThat(facets.get(RuleIndex.FACET_REPOSITORIES)).hasSize(2); + assertThat(facets.get(RuleIndex.FACET_TAGS)).hasSize(4 + numberOfSystemTags); + + // 2 Facet with a language filter + // -- lang facet should still have all language + Result result = index.search(new RuleQuery() + .setLanguages(ImmutableList.of("cpp")) + , new QueryOptions().setFacet(true)); + assertThat(result.getHits()).hasSize(3); + assertThat(result.getFacets()).hasSize(3); + assertThat(result.getFacets().get(RuleIndex.FACET_LANGUAGES)).hasSize(3); + + // 3 facet with 2 filters + // -- lang facet for tag T2 + // -- tag facet for lang cpp + // -- repository for cpp & T2 + result = index.search(new RuleQuery() + .setLanguages(ImmutableList.of("cpp")) + .setTags(ImmutableList.of("T2")) + , new QueryOptions().setFacet(true)); + assertThat(result.getHits()).hasSize(1); + assertThat(result.getFacets().keySet()).hasSize(3); + assertThat(result.getFacets().get(RuleIndex.FACET_LANGUAGES)).hasSize(2); // java & cpp + assertThat(result.getFacets().get(RuleIndex.FACET_REPOSITORIES)).hasSize(1); // foo + assertThat(result.getFacets().get(RuleIndex.FACET_TAGS)).hasSize(2 + numberOfSystemTags); // T2 & T3 + SystemTags + + // 4 facet with 2 filters + // -- lang facet for tag T2 + // -- tag facet for lang cpp & java + // -- repository for (cpp || java) & T2 + result = index.search(new RuleQuery() + .setLanguages(ImmutableList.of("cpp", "java")) + .setTags(ImmutableList.of("T2")) + , new QueryOptions().setFacet(true)); + assertThat(result.getHits()).hasSize(2); + assertThat(result.getFacets().keySet()).hasSize(3); + assertThat(result.getFacets().get(RuleIndex.FACET_LANGUAGES)).hasSize(2); // java & cpp + assertThat(result.getFacets().get(RuleIndex.FACET_REPOSITORIES)).hasSize(2); // foo & xoo + assertThat(result.getFacets().get(RuleIndex.FACET_TAGS)).hasSize(3 + numberOfSystemTags); // T1 & T2 & T3 + SystemTags + } + private static List ruleKeys(List rules) { return newArrayList(Iterables.transform(rules, new Function() { @Override