SONAR-8950 add selected tags to api/rules/search tags facet

server/sonar-server/src/main/java/org/sonar/server/es/StickyFacetBuilder.java

@@ -92,7 +92,7 @@ public class StickyFacetBuilder {
     Object... selected) {
     BoolQueryBuilder facetFilter = getStickyFacetFilter(fieldName);
     FilterAggregationBuilder facetTopAggregation = buildTopFacetAggregation(fieldName, facetName, facetFilter, size, additionalAggregationFilter);
-    facetTopAggregation = addSelectedItemsToFacet(fieldName, facetName, facetTopAggregation, selected);
+    facetTopAggregation = addSelectedItemsToFacet(fieldName, facetName, facetTopAggregation, additionalAggregationFilter, selected);
 
     return AggregationBuilders
       .global(facetName)
@@ -116,11 +116,11 @@ public class StickyFacetBuilder {
   private FilterAggregationBuilder buildTopFacetAggregation(String fieldName, String facetName, BoolQueryBuilder facetFilter, int size,
     Function<TermsBuilder, AggregationBuilder<?>> additionalAggregationFilter) {
     TermsBuilder termsAggregation = buildTermsFacetAggregation(fieldName, facetName, size);
-    AggregationBuilder<?> innerAggregation = additionalAggregationFilter.apply(termsAggregation);
+    AggregationBuilder<?> improvedAggregation = additionalAggregationFilter.apply(termsAggregation);
     return AggregationBuilders
       .filter(facetName + "_filter")
       .filter(facetFilter)
-      .subAggregation(innerAggregation);
+      .subAggregation(improvedAggregation);
   }
 
   private TermsBuilder buildTermsFacetAggregation(String fieldName, String facetName, int size) {
@@ -135,7 +135,8 @@ public class StickyFacetBuilder {
     return termsAggregation;
   }
 
-  public FilterAggregationBuilder addSelectedItemsToFacet(String fieldName, String facetName, FilterAggregationBuilder facetTopAggregation, Object... selected) {
+  public FilterAggregationBuilder addSelectedItemsToFacet(String fieldName, String facetName, FilterAggregationBuilder facetTopAggregation,
+    Function<TermsBuilder, AggregationBuilder<?>> additionalAggregationFilter, Object... selected) {
     if (selected.length <= 0) {
       return facetTopAggregation;
     }
@@ -150,7 +151,9 @@ public class StickyFacetBuilder {
     if (subAggregation != null) {
       selectedTerms = selectedTerms.subAggregation(subAggregation);
     }
-    facetTopAggregation.subAggregation(selectedTerms);
+
+    AggregationBuilder<?> improvedAggregation = additionalAggregationFilter.apply(selectedTerms);
+    facetTopAggregation.subAggregation(improvedAggregation);
     return facetTopAggregation;
   }
 

server/sonar-server/src/main/java/org/sonar/server/issue/index/IssueIndex.java

@@ -510,7 +510,7 @@ public class IssueIndex {
 
     Collection<String> assigneesEscaped = escapeValuesForFacetInclusion(query.assignees());
     if (!assigneesEscaped.isEmpty()) {
-      facetTopAggregation = assigneeFacetBuilder.addSelectedItemsToFacet(fieldName, facetName, facetTopAggregation, assigneesEscaped.toArray());
+      facetTopAggregation = assigneeFacetBuilder.addSelectedItemsToFacet(fieldName, facetName, facetTopAggregation, t -> t, assigneesEscaped.toArray());
     }
 
     // Add missing facet for unassigned issues
@@ -568,7 +568,7 @@ public class IssueIndex {
     StickyFacetBuilder assigneeFacetBuilder = newStickyFacetBuilder(query, resolutionFilters, esQuery);
     BoolQueryBuilder facetFilter = assigneeFacetBuilder.getStickyFacetFilter(fieldName);
     FilterAggregationBuilder facetTopAggregation = assigneeFacetBuilder.buildTopFacetAggregation(fieldName, facetName, facetFilter, DEFAULT_FACET_SIZE);
-    facetTopAggregation = assigneeFacetBuilder.addSelectedItemsToFacet(fieldName, facetName, facetTopAggregation);
+    facetTopAggregation = assigneeFacetBuilder.addSelectedItemsToFacet(fieldName, facetName, facetTopAggregation, t -> t);
 
     // Add missing facet for unresolved issues
     facetTopAggregation.subAggregation(

server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleIndex.java

@@ -379,7 +379,7 @@ public class RuleIndex {
             RuleExtensionScope.organization(organizationUuid).getScope()))
           .subAggregation(termsAggregation);
 
-        return AggregationBuilders.children("children_for_" + FACET_TAGS)
+        return AggregationBuilders.children("children_for_" + termsAggregation.getName())
           .childType(INDEX_TYPE_RULE_EXTENSION.getType())
           .subAggregation(scopeAggregation);
       };

server/sonar-server/src/test/java/org/sonar/server/rule/index/RuleIndexTest.java

@@ -49,7 +49,7 @@ import static java.util.Collections.emptyList;
 import static java.util.Collections.emptySet;
 import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.data.MapEntry.entry;
+import static org.assertj.guava.api.Assertions.entry;
 import static org.junit.Assert.fail;
 import static org.sonar.api.rule.Severity.BLOCKER;
 import static org.sonar.api.rule.Severity.CRITICAL;
@@ -569,7 +569,7 @@ public class RuleIndexTest {
   }
 
   @Test
-  public void all_tags() {
+  public void listTags_should_return_both_system_tags_and_organization_specific_tags() {
     OrganizationDto organization = dbTester.organizations().insert();
 
     RuleDefinitionDto rule1 = createRule(setSystemTags("sys1", "sys2"));
@@ -586,7 +586,7 @@ public class RuleIndexTest {
   }
 
   @Test
-  public void all_tags_minds_the_oranization() {
+  public void listTags_must_not_return_tags_of_other_organizations() {
     OrganizationDto organization1 = dbTester.organizations().insert();
     RuleDefinitionDto rule1 = createRule(setSystemTags("sys1"));
     createRuleMetadata(rule1, organization1,
@@ -599,8 +599,11 @@ public class RuleIndexTest {
       setOrganizationUuid(organization2.getUuid()),
       setTags("tag2"));
 
+    OrganizationDto organization3 = dbTester.organizations().insert();
+
     assertThat(index.listTags(organization1, null, 10)).containsOnly("tag1", "sys1", "sys2");
     assertThat(index.listTags(organization2, null, 10)).containsOnly("tag2", "sys1", "sys2");
+    assertThat(index.listTags(organization3, null, 10)).containsOnly("sys1", "sys2");
   }
 
   @Test
@@ -722,8 +725,7 @@ public class RuleIndexTest {
     OrganizationDto organization = dbTester.organizations().insert();
 
     RuleDefinitionDto rule = insertRuleDefinition(setSystemTags());
-    dbTester.rules().insertOrUpdateMetadata(rule, organization, setTags("bla"));
-    ruleIndexer.indexRuleExtension(organization, rule.getKey());
+    insertRuleMetaData(organization, rule, setTags("bla"));
 
     RuleQuery query = new RuleQuery()
       .setOrganizationUuid(organization.getUuid());
@@ -733,20 +735,61 @@ public class RuleIndexTest {
     assertThat(result.getFacets().get(FACET_TAGS)).contains(entry("bla", 1L));
   }
 
+  @Test
+  public void tags_facet_should_return_top_10_items() {
+    // default number of items returned in facets = 10
+    RuleDefinitionDto rule = insertRuleDefinition(setSystemTags("tag1", "tag2", "tag3", "tag4", "tag5", "tag6", "tag7", "tag8", "tag9", "tagA", "tagB"));
+
+    RuleQuery query = new RuleQuery()
+      .setOrganizationUuid(dbTester.getDefaultOrganization().getUuid());
+    SearchOptions options = new SearchOptions().addFacets(singletonList(FACET_TAGS));
+    SearchIdResult result = index.search(query, options);
+    assertThat(result.getFacets().get(FACET_TAGS)).containsExactly(entry("tag1", 1L), entry("tag2", 1L), entry("tag3", 1L), entry("tag4", 1L), entry("tag5", 1L),
+      entry("tag6", 1L), entry("tag7", 1L), entry("tag8", 1L), entry("tag9", 1L), entry("tagA", 1L));
+  }
+
+  @Test
+  public void tags_facet_should_include_matching_selected_items() {
+    // default number of items returned in facets = 10
+    RuleDefinitionDto rule = insertRuleDefinition(setSystemTags("tag1", "tag2", "tag3", "tag4", "tag5", "tag6", "tag7", "tag8", "tag9", "tagA", "tagB"));
+
+    RuleQuery query = new RuleQuery()
+      .setOrganizationUuid(dbTester.getDefaultOrganization().getUuid())
+      .setTags(singletonList("tagB"));
+    SearchOptions options = new SearchOptions().addFacets(singletonList(FACET_TAGS));
+    SearchIdResult result = index.search(query, options);
+    assertThat(result.getFacets().get(FACET_TAGS).entrySet()).extracting(e -> entry(e.getKey(), e.getValue())).containsExactly(
+
+      // check that selected item is added, although there are 10 other items
+      entry("tagB", 1L),
+
+      entry("tag1", 1L), entry("tag2", 1L), entry("tag3", 1L), entry("tag4", 1L), entry("tag5", 1L), entry("tag6", 1L), entry("tag7", 1L), entry("tag8", 1L), entry("tag9", 1L),
+      entry("tagA", 1L));
+  }
+
   @Test
   public void tags_facet_should_not_find_tags_of_any_other_organization() {
     OrganizationDto organization1 = dbTester.organizations().insert();
     OrganizationDto organization2 = dbTester.organizations().insert();
 
     RuleDefinitionDto rule = insertRuleDefinition(setSystemTags());
-    dbTester.rules().insertOrUpdateMetadata(rule, organization1, setTags("bla"));
+    insertRuleMetaData(organization1, rule, setTags("bla1"));
+    insertRuleMetaData(organization2, rule, setTags("bla2"));
 
     RuleQuery query = new RuleQuery()
       .setOrganizationUuid(organization2.getUuid());
     SearchOptions options = new SearchOptions().addFacets(singletonList(FACET_TAGS));
 
     SearchIdResult result = index.search(query, options);
-    assertThat(result.getFacets().get(FACET_TAGS)).contains();
+    assertThat(result.getFacets().get(FACET_TAGS).entrySet()).extracting(e -> entry(e.getKey(), e.getValue())).containsExactly(
+      entry("bla2", 1L)
+    );
+  }
+
+  @SafeVarargs
+  private final void insertRuleMetaData(OrganizationDto organization, RuleDefinitionDto rule, Consumer<RuleMetadataDto>... consumers) {
+    dbTester.rules().insertOrUpdateMetadata(rule, organization, consumers);
+    ruleIndexer.indexRuleExtension(organization, rule.getKey());
   }
 
   @Test
@@ -809,7 +852,7 @@ public class RuleIndexTest {
       .setTypes(asList(BUG, CODE_SMELL));
 
     SearchIdResult result = index.search(query, new SearchOptions().addFacets(asList(FACET_LANGUAGES, FACET_REPOSITORIES, FACET_TAGS,
-        FACET_TYPES)));
+      FACET_TYPES)));
     assertThat(result.getIds()).hasSize(2);
     assertThat(result.getFacets().getAll()).hasSize(4);
     assertThat(result.getFacets().get(FACET_LANGUAGES).keySet()).containsOnly("cpp", "java");
@@ -922,16 +965,56 @@ public class RuleIndexTest {
     assertThat(tester.countDocuments(INDEX_TYPE_ACTIVE_RULE)).isEqualTo(3);
 
     // 1. get all active rules.
-    assertThat(index.searchAll(new RuleQuery().setActivation(true))).containsOnly(rule1Key, rule2Key);
+    assertThat(index.searchAll(new RuleQuery().setActivation(true)))
+      .containsOnly(rule1Key, rule2Key);
+
+    // 2. get all inactive rules.
+    assertThat(index.searchAll(new RuleQuery().setActivation(false)))
+      .containsOnly(rule3Key);
+
+    // 3. get all rules not active on profile
+    assertThat(index.searchAll(new RuleQuery().setActivation(false).setQProfileKey(QUALITY_PROFILE_KEY2)))
+      .containsOnly(rule2Key, rule3Key);
+
+    // 4. get all active rules on profile
+    assertThat(index.searchAll(new RuleQuery().setActivation(true).setQProfileKey(QUALITY_PROFILE_KEY2)))
+      .containsOnly(rule1Key);
+  }
+
+  @Test
+  public void search_all_keys_by_profile_in_specific_organization() {
+    RuleDefinitionDto rule1 = insertRuleDefinition();
+    RuleDefinitionDto rule2 = insertRuleDefinition();
+    RuleDefinitionDto rule3 = insertRuleDefinition();
+
+    RuleKey rule1Key = rule1.getKey();
+    RuleKey rule2Key = rule2.getKey();
+    RuleKey rule3Key = rule3.getKey();
+
+    OrganizationDto organization = dbTester.organizations().insert();
+
+    indexActiveRules(
+      ActiveRuleDocTesting.newDoc(ActiveRuleKey.of(QUALITY_PROFILE_KEY1, rule1Key)).setOrganizationUuid(organization.getUuid()),
+      ActiveRuleDocTesting.newDoc(ActiveRuleKey.of(QUALITY_PROFILE_KEY2, rule1Key)).setOrganizationUuid(organization.getUuid()),
+      ActiveRuleDocTesting.newDoc(ActiveRuleKey.of(QUALITY_PROFILE_KEY1, rule2Key)).setOrganizationUuid(organization.getUuid()));
+
+    assertThat(tester.countDocuments(INDEX_TYPE_ACTIVE_RULE)).isEqualTo(3);
+
+    // 1. get all active rules.
+    assertThat(index.searchAll(new RuleQuery().setActivation(true).setOrganizationUuid(organization.getUuid())))
+      .containsOnly(rule1Key, rule2Key);
 
     // 2. get all inactive rules.
-    assertThat(index.searchAll(new RuleQuery().setActivation(false))).containsOnly(rule3Key);
+    assertThat(index.searchAll(new RuleQuery().setActivation(false).setOrganizationUuid(organization.getUuid())))
+      .containsOnly(rule3Key);
 
     // 3. get all rules not active on profile
-    assertThat(index.searchAll(new RuleQuery().setActivation(false).setQProfileKey(QUALITY_PROFILE_KEY2))).containsOnly(rule2Key, rule3Key);
+    assertThat(index.searchAll(new RuleQuery().setActivation(false).setOrganizationUuid(organization.getUuid()).setQProfileKey(QUALITY_PROFILE_KEY2)))
+      .containsOnly(rule2Key, rule3Key);
 
     // 4. get all active rules on profile
-    assertThat(index.searchAll(new RuleQuery().setActivation(true).setQProfileKey(QUALITY_PROFILE_KEY2))).containsOnly(rule1Key);
+    assertThat(index.searchAll(new RuleQuery().setActivation(true).setOrganizationUuid(organization.getUuid()).setQProfileKey(QUALITY_PROFILE_KEY2)))
+      .containsOnly(rule1Key);
   }
 
   @SafeVarargs

server/sonar-server/src/test/java/org/sonar/server/rule/ws/SearchActionTest.java

@@ -25,6 +25,7 @@ import java.io.InputStream;
 import java.util.function.Consumer;
 import java.util.stream.Collectors;
 import org.assertj.core.api.iterable.Extractor;
+import org.junit.Before;
 import org.junit.Test;
 import org.sonar.api.config.MapSettings;
 import org.sonar.api.resources.Languages;
@@ -48,9 +49,11 @@ import org.sonarqube.ws.Rules.Rule;
 import org.sonarqube.ws.Rules.SearchResponse;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.guava.api.Assertions.entry;
 import static org.mockito.Matchers.anyString;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
+import static org.sonar.db.rule.RuleTesting.setSystemTags;
 import static org.sonar.db.rule.RuleTesting.setTags;
 import static org.sonarqube.ws.MediaTypes.PROTOBUF;
 
@@ -77,9 +80,14 @@ public class SearchActionTest {
 
   private RuleIndexer ruleIndexer = new RuleIndexer(esClient, dbClient);
 
+  @Before
+  public void before() {
+    doReturn("interpreted").when(macroInterpreter).interpret(anyString());
+  }
+
   @Test
   public void should_find_rule() throws IOException {
-    RuleDefinitionDto rule = insertRule();
+    RuleDefinitionDto rule = insertRuleDefinition();
 
     doReturn("interpreted").when(macroInterpreter).interpret(anyString());
 
@@ -94,9 +102,9 @@ public class SearchActionTest {
   @Test
   public void should_filter_on_organization_specific_tags() throws IOException {
     OrganizationDto organization = dbTester.organizations().insert();
-    RuleDefinitionDto rule1 = insertRule();
+    RuleDefinitionDto rule1 = insertRuleDefinition();
     RuleMetadataDto metadata1 = insertMetadata(organization, rule1, setTags("tag1", "tag2"));
-    RuleDefinitionDto rule2 = insertRule();
+    RuleDefinitionDto rule2 = insertRuleDefinition();
     RuleMetadataDto metadata2 = insertMetadata(organization, rule2);
 
     InputStream inputStream = actionTester.newRequest()
@@ -106,19 +114,60 @@ public class SearchActionTest {
       .setParam("organization", organization.getKey())
       .execute()
       .getInputStream();
-    SearchResponse result;
-    try {
-      result = SearchResponse.parseFrom(inputStream);
-    } catch (IOException e) {
-      throw new RuntimeException("Cannot parse search response", e);
-    }
+    SearchResponse result = SearchResponse.parseFrom(inputStream);
     assertThat(result.getRulesList()).extracting(Rule::getKey).containsExactly(rule1.getKey().toString());
   }
 
+  @Test
+  public void should_list_tags_in_tags_facet() throws IOException {
+    OrganizationDto organization = dbTester.organizations().insert();
+    RuleDefinitionDto rule = insertRuleDefinition(setSystemTags("tag1", "tag3", "tag5", "tag7", "tag9", "x"));
+    RuleMetadataDto metadata = insertMetadata(organization, rule, setTags("tag2", "tag4", "tag6", "tag8", "tagA"));
+
+    InputStream inputStream = actionTester.newRequest()
+      .setMediaType(PROTOBUF)
+      .setParam("facets", "tags")
+      .setParam("organization", organization.getKey())
+      .execute()
+      .getInputStream();
+    SearchResponse result = SearchResponse.parseFrom(inputStream);
+    assertThat(result.getFacets().getFacets(0).getValuesList()).extracting(v -> entry(v.getVal(), v.getCount()))
+      .containsExactly(entry("tag1", 1L), entry("tag2", 1L), entry("tag3", 1L), entry("tag4", 1L), entry("tag5", 1L), entry("tag6", 1L), entry("tag7", 1L), entry("tag8", 1L),
+        entry("tag9", 1L), entry("tagA", 1L));
+  }
+
+  @Test
+  public void should_include_selected_matching_tag_in_facet() throws IOException {
+    insertRuleDefinition(setSystemTags("tag1", "tag2", "tag3", "tag4", "tag5", "tag6", "tag7", "tag8", "tag9", "tagA", "x"));
+
+    InputStream inputStream = actionTester.newRequest()
+      .setMediaType(PROTOBUF)
+      .setParam("facets", "tags")
+      .setParam("tags", "x")
+      .execute()
+      .getInputStream();
+    SearchResponse result = SearchResponse.parseFrom(inputStream);
+    assertThat(result.getFacets().getFacets(0).getValuesList()).extracting(v -> entry(v.getVal(), v.getCount())).contains(entry("x", 1L));
+  }
+
+  @Test
+  public void should_included_selected_non_matching_tag_in_facet() throws IOException {
+    insertRuleDefinition(setSystemTags("tag1", "tag2", "tag3", "tag4", "tag5", "tag6", "tag7", "tag8", "tag9", "tagA"));
+
+    InputStream inputStream = actionTester.newRequest()
+      .setMediaType(PROTOBUF)
+      .setParam("facets", "tags")
+      .setParam("tags", "x")
+      .execute()
+      .getInputStream();
+    SearchResponse result = SearchResponse.parseFrom(inputStream);
+    assertThat(result.getFacets().getFacets(0).getValuesList()).extracting(v -> entry(v.getVal(), v.getCount())).contains(entry("x", 0L));
+  }
+
   @Test
   public void should_return_organization_specific_tags() throws IOException {
     OrganizationDto organization = dbTester.organizations().insert();
-    RuleDefinitionDto rule = insertRule();
+    RuleDefinitionDto rule = insertRuleDefinition();
     RuleMetadataDto metadata = insertMetadata(organization, rule, setTags("tag1", "tag2"));
 
     InputStream inputStream = actionTester.newRequest()
@@ -127,12 +176,7 @@ public class SearchActionTest {
       .setParam("organization", organization.getKey())
       .execute()
       .getInputStream();
-    SearchResponse result;
-    try {
-      result = SearchResponse.parseFrom(inputStream);
-    } catch (IOException e) {
-      throw new RuntimeException("Cannot parse search response", e);
-    }
+    SearchResponse result = SearchResponse.parseFrom(inputStream);
     assertThat(result.getRulesList()).extracting(Rule::getKey).containsExactly(rule.getKey().toString());
     assertThat(result.getRulesList())
       .extracting(Rule::getTags).flatExtracting(Rules.Tags::getTagsList)
@@ -141,7 +185,7 @@ public class SearchActionTest {
 
   @Test
   public void should_return_specified_fields() throws IOException {
-    RuleDefinitionDto rule = insertRule();
+    RuleDefinitionDto rule = insertRuleDefinition();
 
     checkField(rule, "repo", Rule::getRepo, rule.getRepositoryKey());
     checkField(rule, "name", Rule::getName, rule.getName());
@@ -159,26 +203,22 @@ public class SearchActionTest {
   }
 
   @SafeVarargs
-  private final <T> void checkField(RuleDefinitionDto rule, String fieldName, Extractor<Rule, T> responseExtractor, T... expected) {
+  private final <T> void checkField(RuleDefinitionDto rule, String fieldName, Extractor<Rule, T> responseExtractor, T... expected) throws IOException {
     InputStream inputStream = actionTester.newRequest()
       .setMediaType(PROTOBUF)
       .setParam("f", fieldName)
       .execute()
       .getInputStream();
-    SearchResponse result;
-    try {
-      result = SearchResponse.parseFrom(inputStream);
-    } catch (IOException e) {
-      throw new RuntimeException("Cannot parse search response", e);
-    }
+    SearchResponse result = SearchResponse.parseFrom(inputStream);
     assertThat(result.getRulesList()).extracting(Rule::getKey).containsExactly(rule.getKey().toString());
     assertThat(result.getRulesList()).extracting(responseExtractor).containsExactly(expected);
   }
 
-  private RuleDefinitionDto insertRule() {
-    RuleDefinitionDto rule = dbTester.rules().insert();
-    ruleIndexer.indexRuleDefinition(rule.getKey());
-    return rule;
+  @SafeVarargs
+  private final RuleDefinitionDto insertRuleDefinition(Consumer<RuleDefinitionDto>... populaters) {
+    RuleDefinitionDto ruleDefinitionDto = dbTester.rules().insert(populaters);
+    ruleIndexer.indexRuleDefinition(ruleDefinitionDto.getKey());
+    return ruleDefinitionDto;
   }
 
   @SafeVarargs