SONAR-5407 - Fixed filtering logic for effective debt including NONE and null

2 files changed, 38 insertions, 10 deletions

diff --git a/sonar-server/src/main/java/org/sonar/server/rule/index/RuleIndex.java b/sonar-server/src/main/java/org/sonar/server/rule/index/RuleIndex.java
index 6e6364ccf3d..c40326a63ea 100644
--- a/sonar-server/src/main/java/org/sonar/server/rule/index/RuleIndex.java
+++ b/sonar-server/src/main/java/org/sonar/server/rule/index/RuleIndex.java
@@ -42,6 +42,7 @@ import org.elasticsearch.search.sort.SortBuilders;
 import org.elasticsearch.search.sort.SortOrder;
 import org.sonar.api.rule.RuleKey;
 import org.sonar.api.rule.RuleStatus;
+import org.sonar.api.server.debt.DebtCharacteristic;
 import org.sonar.core.cluster.WorkQueue;
 import org.sonar.core.profiling.Profiling;
 import org.sonar.core.profiling.StopWatch;
@@ -219,15 +220,36 @@ public class RuleIndex extends BaseIndex<Rule, RuleDto, RuleKey> {
       .termFilter(RuleNormalizer.RuleField.STATUS.field(),
         RuleStatus.REMOVED.toString()));
 
-    this.addMultiFieldTermFilter(fb, query.getDebtCharacteristics(),
-      RuleNormalizer.RuleField.SUB_CHARACTERISTIC.field(),
-      RuleNormalizer.RuleField.CHARACTERISTIC.field());
     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());
 
+    // Construct the debt filter on effective char and subChar
+    if (query.getDebtCharacteristics() != null && !query.getDebtCharacteristics().isEmpty()) {
+      fb.must(
+        FilterBuilders.orFilter(
+          // Match only when NOT NONE overriden
+          FilterBuilders.andFilter(
+            FilterBuilders.notFilter(
+              FilterBuilders.termsFilter(RuleNormalizer.RuleField.SUB_CHARACTERISTIC.field(), DebtCharacteristic.NONE)),
+            FilterBuilders.orFilter(
+              FilterBuilders.termsFilter(RuleNormalizer.RuleField.SUB_CHARACTERISTIC.field(), query.getDebtCharacteristics()),
+              FilterBuilders.termsFilter(RuleNormalizer.RuleField.CHARACTERISTIC.field(), query.getDebtCharacteristics()))
+          ),
+
+          // Match only when NOT NONE overriden
+          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(), query.getDebtCharacteristics()),
+              FilterBuilders.termsFilter(RuleNormalizer.RuleField.DEFAULT_CHARACTERISTIC.field(), query.getDebtCharacteristics())))
+        )
+      );
+    }
 
     //Debt char exist filter
     if (query.getHasDebtCharacteristic() != null && query.getHasDebtCharacteristic()) {
diff --git a/sonar-server/src/test/java/org/sonar/server/rule/index/RuleIndexMediumTest.java b/sonar-server/src/test/java/org/sonar/server/rule/index/RuleIndexMediumTest.java
index e43f4813674..dbc8525868a 100644
--- a/sonar-server/src/test/java/org/sonar/server/rule/index/RuleIndexMediumTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/rule/index/RuleIndexMediumTest.java
@@ -24,7 +24,10 @@ import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Sets;
 import org.apache.commons.lang.time.DateUtils;
-import org.junit.*;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.ClassRule;
+import org.junit.Test;
 import org.sonar.api.rule.RuleKey;
 import org.sonar.api.rule.RuleStatus;
 import org.sonar.api.rule.Severity;
@@ -47,8 +50,12 @@ import org.sonar.server.search.Result;
 import org.sonar.server.tester.ServerTester;
 
 import javax.annotation.Nullable;
-
-import java.util.*;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Date;
+import java.util.Iterator;
+import java.util.List;
 
 import static com.google.common.collect.Lists.newArrayList;
 import static org.fest.assertions.Assertions.assertThat;
@@ -412,7 +419,6 @@ public class RuleIndexMediumTest {
   }
 
   @Test
-  @Ignore("To be fixed")
   public void search_by_characteristics_with_default_and_overridden_char() throws InterruptedException {
     CharacteristicDto char1 = DebtTesting.newCharacteristicDto("RELIABILITY");
     db.debtCharacteristicDao().insert(char1, dbSession);
@@ -462,21 +468,21 @@ public class RuleIndexMediumTest {
 
     // filter by subChar
     query = new RuleQuery().setDebtCharacteristics(ImmutableSet.of(char11.getKey()));
-    assertThat(ruleKeys(index.search(query, new QueryOptions()).getHits())).containsOnly("S001", "S002", "S004"); // FIXME S002 is missing
+    assertThat(ruleKeys(index.search(query, new QueryOptions()).getHits())).containsOnly("S001", "S002", "S004");
 
     query = new RuleQuery().setDebtCharacteristics(ImmutableSet.of(char21.getKey()));
     assertThat(ruleKeys(index.search(query, new QueryOptions()).getHits())).containsOnly("S003");
 
     // filter by Char
     query = new RuleQuery().setDebtCharacteristics(ImmutableSet.of(char1.getKey()));
-    assertThat(ruleKeys(index.search(query, new QueryOptions()).getHits())).containsOnly("S001", "S002", "S004"); // FIXME S002 is missing
+    assertThat(ruleKeys(index.search(query, new QueryOptions()).getHits())).containsOnly("S001", "S002", "S004");
 
     query = new RuleQuery().setDebtCharacteristics(ImmutableSet.of(char2.getKey()));
     assertThat(ruleKeys(index.search(query, new QueryOptions()).getHits())).containsOnly("S003");
 
     // filter by Char and SubChar
     query = new RuleQuery().setDebtCharacteristics(ImmutableSet.of(char11.getKey(), char1.getKey(), char2.getKey(), char21.getKey()));
-    assertThat(ruleKeys(index.search(query, new QueryOptions()).getHits())).containsOnly("S001", "S002", "S003", "S004"); // FIXME S002 is missing
+    assertThat(ruleKeys(index.search(query, new QueryOptions()).getHits())).containsOnly("S001", "S002", "S003", "S004");
   }
 
   @Test