SONAR-6233 Fix behavior of facet on uncharacterized rules

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 81df870ca2a2895dddab75ba7c37140905b94abd..483d6b2c55eb5d002559a004eb6b737bd88c6739 100644 (file)
--- 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
@@ -385,24 +385,46 @@ public class RuleIndex extends BaseIndex<Rule, RuleDto, RuleKey> {
   }
 
   private void addCharacteristicsFacetIfNeeded(RuleQuery query, QueryContext options, Map<String, AggregationBuilder> aggregations, StickyFacetBuilder stickyFacetBuilder) {
+
     if (options.facets().contains(FACET_DEBT_CHARACTERISTICS)) {
+      /*
+       * Since this facet concerns 2 fields, we're using an aggregation structure like this:
+       * global aggregation
+       *  |- sub-aggregation on characteristics: filter
+       *  |  |- classic sub-aggregation with top-n terms, excluding NONE
+       *  |  |- classic sub-aggregation with selected terms
+       *  |  |- terms aggregation on "NONE"
+       *  |  |- missing aggregation
+       *  |- sub-aggregation on sub-characteristics: filter, excluding NONE
+       *     |- classic sub-aggregation with top-n terms
+       *     |- classic sub-aggregation with selected terms
+       */
       int characsSize = 10;
       int subCharacsSize = 300;
       Collection<String> characsFromQuery = query.getDebtCharacteristics();
       Object[] selectedChars = characsFromQuery == null ? new Object[0] : characsFromQuery.toArray();
+      BoolFilterBuilder stickyFacetFilter = stickyFacetBuilder.getStickyFacetFilter(FILTER_DEBT_CHARACTERISTICS, FILTER_HAS_DEBT_CHARACTERISTICS);
       AggregationBuilder debtChar = AggregationBuilders.filter(FACET_DEBT_CHARACTERISTICS + "__chars")
-        .filter(stickyFacetBuilder.getStickyFacetFilter(FILTER_DEBT_CHARACTERISTICS))
+        .filter(stickyFacetFilter)
         .subAggregation(
           AggregationBuilders.terms(FACET_DEBT_CHARACTERISTICS + "__chars_top").field(RuleNormalizer.RuleField.CHARACTERISTIC.field())
+            .exclude(DebtCharacteristic.NONE)
             .size(characsSize))
         .subAggregation(
           AggregationBuilders.terms(FACET_DEBT_CHARACTERISTICS + "__chars_selected").field(RuleNormalizer.RuleField.CHARACTERISTIC.field())
             .include(Joiner.on('|').join(selectedChars))
-            .size(characsSize));
+            .size(characsSize))
+        .subAggregation(
+          AggregationBuilders.terms(FACET_DEBT_CHARACTERISTICS + "__chars_none").field(RuleNormalizer.RuleField.CHARACTERISTIC.field())
+              .include(DebtCharacteristic.NONE)
+              .size(characsSize))
+        .subAggregation(
+          AggregationBuilders.missing(FACET_DEBT_CHARACTERISTICS + "__chars_missing").field(RuleNormalizer.RuleField.CHARACTERISTIC.field()));
       AggregationBuilder debtSubChar = AggregationBuilders.filter(FACET_DEBT_CHARACTERISTICS + "__subchars")
-        .filter(stickyFacetBuilder.getStickyFacetFilter(FILTER_DEBT_CHARACTERISTICS))
+        .filter(stickyFacetFilter)
         .subAggregation(
           AggregationBuilders.terms(FACET_DEBT_CHARACTERISTICS + "__subchars_top").field(RuleNormalizer.RuleField.SUB_CHARACTERISTIC.field())
+            .exclude(DebtCharacteristic.NONE)
             .size(subCharacsSize))
         .subAggregation(
           AggregationBuilders.terms(FACET_DEBT_CHARACTERISTICS + "__chars_selected").field(RuleNormalizer.RuleField.SUB_CHARACTERISTIC.field())

diff --git a/server/sonar-server/src/main/java/org/sonar/server/rule/ws/SearchAction.java b/server/sonar-server/src/main/java/org/sonar/server/rule/ws/SearchAction.java
index d2c00afe29b0ae4bc2b01ddd45d6ff80be694320..66bbea4f7478fc58854b19fc5dd4c4bab70b3290 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/rule/ws/SearchAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/rule/ws/SearchAction.java
@@ -25,6 +25,7 @@ import com.google.common.collect.ImmutableSet;
 import com.google.common.io.Resources;
 import org.sonar.api.rule.RuleStatus;
 import org.sonar.api.rule.Severity;
+import org.sonar.api.server.debt.DebtCharacteristic;
 import org.sonar.api.server.ws.Request;
 import org.sonar.api.server.ws.WebService;
 import org.sonar.api.utils.text.JsonWriter;
@@ -35,6 +36,7 @@ import org.sonar.server.rule.RuleService;
 import org.sonar.server.rule.index.RuleIndex;
 import org.sonar.server.rule.index.RuleNormalizer;
 import org.sonar.server.rule.index.RuleQuery;
+import org.sonar.server.search.FacetValue;
 import org.sonar.server.search.QueryContext;
 import org.sonar.server.search.Result;
 import org.sonar.server.search.ws.SearchOptions;
@@ -44,6 +46,7 @@ import javax.annotation.CheckForNull;
 
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Iterator;
 
 /**
  * @since 4.4
@@ -314,6 +317,31 @@ public class SearchAction extends SearchRequestHandler<RuleQuery, Rule> implemen
     addMandatoryFacetValues(results, RuleIndex.FACET_STATUSES, RuleIndex.ALL_STATUSES_EXCEPT_REMOVED);
     addMandatoryFacetValues(results, RuleIndex.FACET_SEVERITIES, Severity.ALL);
     addMandatoryFacetValues(results, RuleIndex.FACET_TAGS, request.paramAsStrings(PARAM_TAGS));
+
+    mergeNoneAndEmptyBucketOnCharacteristics(results);
+
     super.writeFacets(request, context, results, json);
   }
+
+  protected void mergeNoneAndEmptyBucketOnCharacteristics(Result<?> results) {
+    if (results.getFacets().containsKey(RuleIndex.FACET_DEBT_CHARACTERISTICS)) {
+      Collection<FacetValue> characValues = results.getFacetValues(RuleIndex.FACET_DEBT_CHARACTERISTICS);
+      if (characValues == null) {
+        return;
+      }
+
+      long mergedCount = 0L;
+      Iterator<FacetValue> characIterator = characValues.iterator();
+      while (characIterator.hasNext()) {
+        FacetValue characValue = characIterator.next();
+        if ("".equals(characValue.getKey()) || DebtCharacteristic.NONE.equals(characValue.getKey())) {
+          mergedCount += characValue.getValue();
+          characIterator.remove();
+        }
+      }
+
+      FacetValue mergedNoneValue = new FacetValue(DebtCharacteristic.NONE, mergedCount);
+      characValues.add(mergedNoneValue);
+    }
+  }
 }

diff --git a/server/sonar-server/src/main/java/org/sonar/server/search/Facets.java b/server/sonar-server/src/main/java/org/sonar/server/search/Facets.java
index 58f6b140bc4f31d7984cf3c07d4678fcc9d1706c..2b23c003331aaf1bbb7fb9891e5b3ee91d83d0be 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/search/Facets.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/search/Facets.java
@@ -71,7 +71,11 @@ class Facets {
     Missing missing = (Missing) aggregation;
     long docCount = missing.getDocCount();
     if (docCount > 0L) {
-      this.facetValues.put(aggregation.getName().replace("_missing", ""), new FacetValue("", docCount));
+      String facetName = aggregation.getName();
+      if (facetName.contains("__") && !facetName.startsWith("__")) {
+        facetName = facetName.substring(0, facetName.indexOf("__"));
+      }
+      this.facetValues.put(facetName.replace("_missing", ""), new FacetValue("", docCount));
     }
   }
 

diff --git a/server/sonar-server/src/main/java/org/sonar/server/search/StickyFacetBuilder.java b/server/sonar-server/src/main/java/org/sonar/server/search/StickyFacetBuilder.java
index 53b36c093babe8310fc6cb2fe816536cfb3cc4c8..589536f14844988faa4ce992ee31275f3a358aa7 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/search/StickyFacetBuilder.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/search/StickyFacetBuilder.java
@@ -20,7 +20,7 @@
 package org.sonar.server.search;
 
 import com.google.common.base.Joiner;
-import org.apache.commons.lang.StringUtils;
+import org.apache.commons.lang.ArrayUtils;
 import org.elasticsearch.index.query.BoolFilterBuilder;
 import org.elasticsearch.index.query.FilterBuilder;
 import org.elasticsearch.index.query.FilterBuilders;
@@ -67,10 +67,10 @@ public class StickyFacetBuilder {
       .subAggregation(facetTopAggregation);
   }
 
-  public BoolFilterBuilder getStickyFacetFilter(String fieldName) {
+  public BoolFilterBuilder getStickyFacetFilter(String... fieldNames) {
     BoolFilterBuilder facetFilter = FilterBuilders.boolFilter().must(FilterBuilders.queryFilter(query));
     for (Map.Entry<String, FilterBuilder> filter : filters.entrySet()) {
-      if (filter.getValue() != null && !StringUtils.equals(filter.getKey(), fieldName)) {
+      if (filter.getValue() != null && !ArrayUtils.contains(fieldNames, filter.getKey())) {
         facetFilter.must(filter.getValue());
       }
     }

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 97b360332089ea37c67fb66a76d64e2c46825658..10f7dfba5da794a04b0ad2b79db74957c4e60cec 100644 (file)
--- 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
@@ -340,6 +340,97 @@ public class RuleIndexMediumTest {
     assertThat(index.search(new RuleQuery().setHasDebtCharacteristic(false), new QueryContext()).getTotal()).isEqualTo(2);
   }
 
+  @Test
+  public void facet_on_debt_characteristic() throws InterruptedException {
+    CharacteristicDto char1 = DebtTesting.newCharacteristicDto("c1")
+      .setEnabled(true)
+      .setName("char1");
+    db.debtCharacteristicDao().insert(char1, dbSession);
+    dbSession.commit();
+
+    CharacteristicDto char11 = DebtTesting.newCharacteristicDto("c11")
+      .setEnabled(true)
+      .setName("char11")
+      .setParentId(char1.getId());
+    db.debtCharacteristicDao().insert(char11, dbSession);
+
+    // Rule with default characteristic
+    dao.insert(dbSession, RuleTesting.newDto(RuleKey.of("findbugs", "S001"))
+      .setSubCharacteristicId(null)
+      .setRemediationFunction(null)
+      .setDefaultSubCharacteristicId(char11.getId())
+      .setDefaultRemediationFunction("LINEAR").setDefaultRemediationCoefficient("2h"));
+    // Rule with overridden characteristic
+    dao.insert(dbSession, RuleTesting.newDto(RuleKey.of("pmd", "S002"))
+      .setSubCharacteristicId(char11.getId())
+      .setRemediationFunction("LINEAR").setRemediationCoefficient("2h")
+      .setDefaultSubCharacteristicId(null)
+      .setDefaultRemediationFunction(null));
+    // Rule without debt characteristic
+    dao.insert(dbSession, RuleTesting.newDto(RuleKey.of("xoo", "S001"))
+      .setSubCharacteristicId(null)
+      .setRemediationFunction(null).setRemediationCoefficient(null)
+      .setDefaultSubCharacteristicId(null)
+      .setDefaultRemediationFunction(null).setDefaultRemediationCoefficient(null));
+    // Rule with disabled debt characteristic
+    dao.insert(dbSession, RuleTesting.newDto(RuleKey.of("xoo", "S002"))
+      .setSubCharacteristicId(-1)
+      .setRemediationFunction(null).setRemediationCoefficient(null)
+      .setDefaultSubCharacteristicId(null)
+      .setDefaultRemediationFunction(null).setDefaultRemediationCoefficient(null));
+    dbSession.commit();
+
+    QueryContext withDebtCharFacet = new QueryContext().addFacets(Arrays.asList(RuleIndex.FACET_DEBT_CHARACTERISTICS));
+
+    // Facet show results on characs, subcharacs and uncharacterized rules 
+    Result<Rule> result1 = index.search(new RuleQuery(), withDebtCharFacet);
+    assertThat(result1.getFacetValues(RuleIndex.FACET_DEBT_CHARACTERISTICS)).containsOnly(
+      new FacetValue("c1", 2L),
+      new FacetValue("c11", 2L),
+      new FacetValue("", 1L),
+      new FacetValue("NONE", 1L)
+    );
+
+    // Facet is sticky when using a charac filter
+    Result<Rule> result2 = index.search(new RuleQuery().setDebtCharacteristics(Arrays.asList("c1")), withDebtCharFacet);
+    assertThat(result2.getFacetValues(RuleIndex.FACET_DEBT_CHARACTERISTICS)).containsOnly(
+      new FacetValue("c1", 2L),
+      new FacetValue("c11", 2L),
+      new FacetValue("", 1L),
+      new FacetValue("NONE", 1L)
+    );
+
+    // Facet is sticky when using a sub-charac filter
+    Result<Rule> result3 = index.search(new RuleQuery().setDebtCharacteristics(Arrays.asList("c11")), withDebtCharFacet);
+    assertThat(result3.getFacetValues(RuleIndex.FACET_DEBT_CHARACTERISTICS)).containsOnly(
+      new FacetValue("c1", 2L),
+      new FacetValue("c11", 2L),
+      new FacetValue("", 1L),
+      new FacetValue("NONE", 1L)
+    );
+
+    // Facet is sticky when using hasCharac filter
+    Result<Rule> result4 = index.search(new RuleQuery().setHasDebtCharacteristic(false), withDebtCharFacet);
+    assertThat(result4.getFacetValues(RuleIndex.FACET_DEBT_CHARACTERISTICS)).containsOnly(
+      new FacetValue("c1", 2L),
+      new FacetValue("c11", 2L),
+      new FacetValue("", 1L),
+      new FacetValue("NONE", 1L)
+    );
+
+    // Facet applies other filters
+    Result<Rule> result5 = index.search(new RuleQuery().setRepositories(Arrays.asList("xoo")), withDebtCharFacet);
+    assertThat(result5.getFacetValues(RuleIndex.FACET_DEBT_CHARACTERISTICS)).containsOnly(
+      new FacetValue("", 1L),
+      new FacetValue("NONE", 1L)
+    );
+    Result<Rule> result6 = index.search(new RuleQuery().setRepositories(Arrays.asList("findbugs")), withDebtCharFacet);
+    assertThat(result6.getFacetValues(RuleIndex.FACET_DEBT_CHARACTERISTICS)).containsOnly(
+      new FacetValue("c1", 1L),
+      new FacetValue("c11", 1L)
+    );
+  }
+
   @Test
   public void search_by_any_of_repositories() {
     dao.insert(dbSession, RuleTesting.newDto(RuleKey.of("findbugs", "S001")));

diff --git a/server/sonar-server/src/test/java/org/sonar/server/rule/ws/RulesWebServiceMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/rule/ws/RulesWebServiceMediumTest.java
index eca79f58f1c76cb955f99575099465f6422074cd..4a97bf50e68de5a622896c4670bc3049ae3021f1 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/rule/ws/RulesWebServiceMediumTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/rule/ws/RulesWebServiceMediumTest.java
@@ -19,8 +19,6 @@
  */
 package org.sonar.server.rule.ws;
 
-import org.sonar.core.rule.RuleDto.Format;
-
 import com.google.common.collect.ImmutableSet;
 import org.junit.After;
 import org.junit.Before;
@@ -38,6 +36,7 @@ import org.sonar.core.qualityprofile.db.ActiveRuleParamDto;
 import org.sonar.core.qualityprofile.db.QualityProfileDao;
 import org.sonar.core.qualityprofile.db.QualityProfileDto;
 import org.sonar.core.rule.RuleDto;
+import org.sonar.core.rule.RuleDto.Format;
 import org.sonar.core.rule.RuleParamDto;
 import org.sonar.core.technicaldebt.db.CharacteristicDto;
 import org.sonar.server.db.DbClient;
@@ -316,14 +315,36 @@ public class RulesWebServiceMediumTest {
       .setRemediationCoefficient("30min")
       .setRemediationOffset("5min")
       );
+    ruleDao.insert(session, RuleTesting.newXooX3()
+      .setDefaultSubCharacteristicId(null)
+      .setDefaultRemediationFunction(DebtRemediationFunction.Type.LINEAR_OFFSET.name())
+      .setDefaultRemediationCoefficient("2min")
+      .setDefaultRemediationOffset("1min")
+
+      .setSubCharacteristicId(null)
+      .setRemediationFunction(null)
+      .setRemediationCoefficient(null)
+      .setRemediationOffset(null)
+      );
+    ruleDao.insert(session, RuleTesting.newDto(RuleKey.of("xoo", "x4")).setLanguage("xoo")
+      .setDefaultSubCharacteristicId(softReliabilityId)
+      .setDefaultRemediationFunction(DebtRemediationFunction.Type.LINEAR_OFFSET.name())
+      .setDefaultRemediationCoefficient("2min")
+      .setDefaultRemediationOffset("1min")
+
+      .setSubCharacteristicId(-1)
+      .setRemediationFunction(null)
+      .setRemediationCoefficient(null)
+      .setRemediationOffset(null)
+      );
     session.commit();
 
     MockUserSession.set();
-    WsTester.TestRequest request = tester.wsTester().newGetRequest(API_ENDPOINT, API_SEARCH_METHOD);
-    request.setParam(SearchOptions.PARAM_FIELDS, "debtChar,debtCharName,debtSubChar,debtSubCharName,debtRemFn,debtOverloaded,defaultDebtChar,defaultDebtSubChar,defaultDebtRemFn");
-    request.setParam("debt_characteristics", "SOFT_RELIABILITY");
-    request.setParam(SearchAction.PARAM_FACETS, "debt_characteristics");
-    WsTester.Result result = request.execute();
+    WsTester.Result result = tester.wsTester().newGetRequest(API_ENDPOINT, API_SEARCH_METHOD)
+      .setParam(SearchOptions.PARAM_FIELDS, "debtChar,debtCharName,debtSubChar,debtSubCharName,debtRemFn,debtOverloaded,defaultDebtChar,defaultDebtSubChar,defaultDebtRemFn")
+      .setParam("debt_characteristics", "SOFT_RELIABILITY")
+      .setParam(SearchAction.PARAM_FACETS, "debt_characteristics")
+      .execute();
     result.assertJson(this.getClass(), "search_debt_rules_sticky.json", false);
   }
 

diff --git a/server/sonar-server/src/test/resources/org/sonar/server/rule/ws/RulesWebServiceMediumTest/search_debt_rule.json b/server/sonar-server/src/test/resources/org/sonar/server/rule/ws/RulesWebServiceMediumTest/search_debt_rule.json
index cd2187655448a99fefb1ac63583f0df1c89d06cb..0a2211290ec743912b3fd5d25e8dcfa468587896 100644 (file)
--- a/server/sonar-server/src/test/resources/org/sonar/server/rule/ws/RulesWebServiceMediumTest/search_debt_rule.json
+++ b/server/sonar-server/src/test/resources/org/sonar/server/rule/ws/RulesWebServiceMediumTest/search_debt_rule.json
@@ -27,6 +27,10 @@
       {
         "val": "HARD_RELIABILITY",
         "count": 1
+      },
+      {
+        "val": "NONE",
+        "count": 0
       }
     ]
   }

diff --git a/server/sonar-server/src/test/resources/org/sonar/server/rule/ws/RulesWebServiceMediumTest/search_debt_rules_sticky.json b/server/sonar-server/src/test/resources/org/sonar/server/rule/ws/RulesWebServiceMediumTest/search_debt_rules_sticky.json
index fdd626cfb0e884add6cbe22e839824ac81c938f7..a7cba30b88e457eaed38bb87552ef821ba632e6e 100644 (file)
--- a/server/sonar-server/src/test/resources/org/sonar/server/rule/ws/RulesWebServiceMediumTest/search_debt_rules_sticky.json
+++ b/server/sonar-server/src/test/resources/org/sonar/server/rule/ws/RulesWebServiceMediumTest/search_debt_rules_sticky.json
@@ -31,6 +31,10 @@
       {
         "val": "HARD_RELIABILITY",
         "count": 1
+      },
+      {
+        "val": "NONE",
+        "count": 2
       }
     ]
   }