SONAR-8436 Fix Quality flaws related to org.sonar.server.rule

2 files changed, 37 insertions, 31 deletions

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 ae5c60bd87d..b5410cda01b 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
@@ -471,31 +471,29 @@ public class RuleIndex extends BaseIndex {
   }
 
   public Set<String> terms(String fields, @Nullable String query, int size) {
-    Set<String> tags = new HashSet<>();
-    String key = "_ref";
-
-    TermsBuilder terms = AggregationBuilders.terms(key)
+    String termsAggregationKey = "_ref";
+    TermsBuilder termsAggregation = AggregationBuilders.terms(termsAggregationKey)
       .field(fields)
       .size(size)
       .minDocCount(1);
     if (query != null) {
-      terms.include(".*" + query + ".*");
+      termsAggregation.include(".*" + query + ".*");
     }
     SearchRequestBuilder request = getClient()
       .prepareSearch(INDEX)
       .setQuery(QueryBuilders.matchAllQuery())
-      .addAggregation(terms);
+      .addAggregation(termsAggregation);
 
     SearchResponse esResponse = request.get();
 
-    Terms aggregation = esResponse.getAggregations().get(key);
-
+    Set<String> terms = new HashSet<>();
+    Terms aggregation = esResponse.getAggregations().get(termsAggregationKey);
     if (aggregation != null) {
       for (Terms.Bucket value : aggregation.getBuckets()) {
-        tags.add(value.getKey());
+        terms.add(value.getKey());
       }
     }
-    return tags;
+    return terms;
   }
 
   private enum ToRuleKey implements Function<String, RuleKey> {
diff --git a/server/sonar-server/src/test/java/org/sonar/server/rule/RuleServiceMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/rule/RuleServiceMediumTest.java
index 93839de1ae7..e6fc5e610fa 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/rule/RuleServiceMediumTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/rule/RuleServiceMediumTest.java
@@ -19,8 +19,6 @@
  */
 package org.sonar.server.rule;
 
-import com.google.common.collect.Sets;
-import java.util.Collections;
 import java.util.Set;
 import org.junit.After;
 import org.junit.Before;
@@ -33,12 +31,11 @@ import org.sonar.db.DbSession;
 import org.sonar.db.rule.RuleDao;
 import org.sonar.db.rule.RuleTesting;
 import org.sonar.server.exceptions.UnauthorizedException;
-import org.sonar.server.rule.index.RuleIndex;
-import org.sonar.server.rule.index.RuleIndexDefinition;
 import org.sonar.server.rule.index.RuleIndexer;
 import org.sonar.server.tester.ServerTester;
 import org.sonar.server.tester.UserSessionRule;
 
+import static com.google.common.collect.Sets.newHashSet;
 import static org.assertj.core.api.Assertions.assertThat;
 
 public class RuleServiceMediumTest {
@@ -49,11 +46,10 @@ public class RuleServiceMediumTest {
   @org.junit.Rule
   public UserSessionRule userSessionRule = UserSessionRule.forServerTester(tester);
 
-  RuleDao dao = tester.get(RuleDao.class);
-  RuleIndex index = tester.get(RuleIndex.class);
-  RuleService service = tester.get(RuleService.class);
-  DbSession dbSession;
-  RuleIndexer ruleIndexer;
+  private RuleDao dao = tester.get(RuleDao.class);
+  private RuleService service = tester.get(RuleService.class);
+  private DbSession dbSession;
+  private RuleIndexer ruleIndexer;
 
   @Before
   public void before() {
@@ -69,24 +65,29 @@ public class RuleServiceMediumTest {
   }
 
   @Test
-  public void list_tags() {
+  public void listTags_returns_all_tags() {
     // insert db
-    RuleKey key1 = RuleKey.of("javascript", "S001");
-    RuleKey key2 = RuleKey.of("java", "S001");
-    dao.insert(dbSession,
-      RuleTesting.newDto(key1).setTags(Sets.newHashSet("tag1")).setSystemTags(Sets.newHashSet("sys1", "sys2")));
-    dao.insert(dbSession,
-      RuleTesting.newDto(key2).setTags(Sets.newHashSet("tag2")).setSystemTags(Collections.<String>emptySet()));
-    dbSession.commit();
-    ruleIndexer.index();
+    insertRule(RuleKey.of("javascript", "S001"), newHashSet("tag1"), newHashSet("sys1", "sys2"));
+    insertRule(RuleKey.of("java", "S001"), newHashSet("tag2"), newHashSet());
 
     // all tags, including system
     Set<String> tags = service.listTags();
     assertThat(tags).containsOnly("tag1", "tag2", "sys1", "sys2");
+  }
 
-    // verify in es
-    tags = index.terms(RuleIndexDefinition.FIELD_RULE_ALL_TAGS);
-    assertThat(tags).containsOnly("tag1", "tag2", "sys1", "sys2");
+  @Test
+  public void listTags_returns_tags_filtered_by_name() {
+    insertRule(RuleKey.of("javascript", "S001"), newHashSet("tag1"), newHashSet("sys1", "sys2"));
+    insertRule(RuleKey.of("java", "S001"), newHashSet("tag2"), newHashSet());
+
+    assertThat(service.listTags("missing", 10)).isEmpty();
+    assertThat(service.listTags("tag", 10)).containsOnly("tag1", "tag2");
+    assertThat(service.listTags("sys", 10)).containsOnly("sys1", "sys2");
+
+    // LIMITATION: case sensitive
+    assertThat(service.listTags("TAG", 10)).isEmpty();
+    assertThat(service.listTags("TAg", 10)).isEmpty();
+    assertThat(service.listTags("MISSing", 10)).isEmpty();
   }
 
   @Test(expected = UnauthorizedException.class)
@@ -95,4 +96,11 @@ public class RuleServiceMediumTest {
 
     service.delete(RuleKey.of("java", "S001"));
   }
+
+  private void insertRule(RuleKey key, Set<String> tags, Set<String> systemTags) {
+    dao.insert(dbSession,
+      RuleTesting.newDto(key).setTags(tags).setSystemTags(systemTags));
+    dbSession.commit();
+    ruleIndexer.index();
+  }
 }