SONAR-10313 fix error when displaying rule facet

server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchAction.java

@@ -20,7 +20,6 @@
 package org.sonar.server.issue.ws;
 
 import com.google.common.base.Joiner;
-import com.google.common.collect.Collections2;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
 import java.util.Arrays;
@@ -69,6 +68,7 @@ import static java.lang.String.format;
 import static java.util.Collections.emptyList;
 import static java.util.Collections.singletonList;
 import static org.sonar.api.utils.Paging.forPageIndex;
+import static org.sonar.core.util.stream.MoreCollectors.toSet;
 import static org.sonar.server.es.SearchOptions.MAX_LIMIT;
 import static org.sonar.server.ws.KeyExamples.KEY_BRANCH_EXAMPLE_001;
 import static org.sonar.server.ws.KeyExamples.KEY_PROJECT_EXAMPLE_001;
@@ -360,7 +360,7 @@ public class SearchAction implements IssuesWsAction {
 
       Set<String> facetsRequiringProjectOrOrganizationParameter = facets.getNames().stream()
         .filter(FACETS_REQUIRING_PROJECT_OR_ORGANIZATION::contains)
-        .collect(MoreCollectors.toSet());
+        .collect(toSet());
       checkArgument(facetsRequiringProjectOrOrganizationParameter.isEmpty() ||
         (!query.projectUuids().isEmpty()) || query.organizationUuid() != null, "Facet(s) '%s' require to also filter by project or organization",
         COMA_JOINER.join(facetsRequiringProjectOrOrganizationParameter));
@@ -384,35 +384,59 @@ public class SearchAction implements IssuesWsAction {
   }
 
   private void replaceRuleIdsByRuleKeys(@Nullable Facets facets, List<RuleDefinitionDto> alreadyLoadedRules) {
-    if (facets == null || facets.get(PARAM_RULES) == null) {
+    if (facets == null) {
+      return;
+    }
+    LinkedHashMap<String, Long> rulesFacet = facets.get(PARAM_RULES);
+    if (rulesFacet == null) {
       return;
     }
 
     // The facet for PARAM_RULES contains the id of the rule as the key
     // We need to update the key to be a RuleKey
-    LinkedHashMap<String, Long> rulesFacet = facets.get(PARAM_RULES);
-
     try (DbSession dbSession = dbClient.openSession(false)) {
-      Set<String> ruleKeysToLoad = new HashSet<>(rulesFacet.keySet());
-      ruleKeysToLoad.removeAll(
+      Set<Integer> ruleIdsToLoad = new HashSet<>();
+      rulesFacet.keySet().forEach(s -> {
+        try {
+          ruleIdsToLoad.add(Integer.parseInt(s));
+        } catch (NumberFormatException e) {
+          // ignore, this is already a key
+        }
+      });
+      ruleIdsToLoad.removeAll(
         alreadyLoadedRules
           .stream()
-          .map(r -> r.getKey().toString())
+          .map(RuleDefinitionDto::getId)
           .collect(Collectors.toList()));
 
-      Map<Integer, RuleKey> idToRuleKey = Stream.concat(
+      List<RuleDefinitionDto> ruleDefinitions = Stream.concat(
         alreadyLoadedRules.stream(),
-        dbClient.ruleDao().selectDefinitionByIds(dbSession, Collections2.transform(ruleKeysToLoad, Integer::parseInt)).stream())
+        dbClient.ruleDao().selectDefinitionByIds(dbSession, ruleIdsToLoad).stream())
+        .collect(MoreCollectors.toList());
+      Map<Integer, RuleKey> ruleKeyById = ruleDefinitions.stream()
         .collect(Collectors.toMap(RuleDefinitionDto::getId, RuleDefinitionDto::getKey));
+      Map<String, Integer> idByRuleKeyAsString = ruleDefinitions.stream()
+        .collect(Collectors.toMap(s -> s.getKey().toString(), RuleDefinitionDto::getId));
 
       LinkedHashMap<String, Long> newRulesFacet = new LinkedHashMap<>();
       rulesFacet.forEach((k, v) -> {
-        RuleKey ruleKey = idToRuleKey.get(Integer.parseInt(k));
-        if (ruleKey != null) {
-          newRulesFacet.put(ruleKey.toString(), v);
-        } else {
-          // RuleKey not found ES/DB incorrect?
-          LOGGER.error("Rule with id {} is not available in database", k);
+        try {
+          int ruleId = Integer.parseInt(k);
+          RuleKey ruleKey = ruleKeyById.get(ruleId);
+          if (ruleKey != null) {
+            newRulesFacet.put(ruleKey.toString(), v);
+          } else {
+            // RuleKey not found ES/DB incorrect?
+            LOGGER.error("Rule with id {} is not available in database", k);
+          }
+        } catch (NumberFormatException e) {
+          // RuleKey are added into the facet from the HTTP request, there may be a result for this rule from the
+          // ES search (with the ruleId as a key). If so, do not add this entry again (anyway, value for ruleKey is
+          // always 0 since it is added SearchAction#completeFacets).
+          String ruleId = String.valueOf(idByRuleKeyAsString.get(k));
+          if (!rulesFacet.containsKey(ruleId)) {
+            newRulesFacet.put(k, v);
+          }
         }
       });
       rulesFacet.clear();
@@ -502,7 +526,7 @@ public class SearchAction implements IssuesWsAction {
   private static void collectFacets(SearchResponseLoader.Collector collector, Facets facets) {
     Set<String> facetRules = facets.getBucketKeys(PARAM_RULES);
     if (facetRules != null) {
-      collector.addAll(SearchAdditionalField.RULES, facetRules);
+      collector.addAll(SearchAdditionalField.RULE_IDS_AND_KEYS, facetRules);
     }
     collector.addProjectUuids(facets.getBucketKeys(PARAM_PROJECT_UUIDS));
     collector.addComponentUuids(facets.getBucketKeys(PARAM_COMPONENT_UUIDS));

server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchAdditionalField.java

@@ -38,7 +38,8 @@ public enum SearchAdditionalField {
   DEPRECATED_ACTION_PLANS("actionPlans"),
   COMMENTS("comments"),
   LANGUAGES("languages"),
-  RULES("rules"),
+  // may contain strings representing rule ids (from ES facets) or string representing ruleKeys (from HTTP request)
+  RULE_IDS_AND_KEYS("rules"),
   TRANSITIONS("transitions"),
   USERS("users");
 

server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchResponseFormat.java

@@ -89,7 +89,7 @@ public class SearchResponseFormat {
     if (facets != null) {
       formatFacets(facets, response);
     }
-    if (fields.contains(SearchAdditionalField.RULES)) {
+    if (fields.contains(SearchAdditionalField.RULE_IDS_AND_KEYS)) {
       response.setRules(formatRules(data));
     }
     if (fields.contains(SearchAdditionalField.USERS)) {

server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchResponseLoader.java

@@ -32,7 +32,9 @@ import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 import javax.annotation.Nullable;
+import org.sonar.api.rule.RuleKey;
 import org.sonar.core.issue.DefaultIssue;
 import org.sonar.core.util.stream.MoreCollectors;
 import org.sonar.db.DbClient;
@@ -62,7 +64,7 @@ import static org.sonar.server.issue.SetSeverityAction.SET_SEVERITY_KEY;
 import static org.sonar.server.issue.SetTypeAction.SET_TYPE_KEY;
 import static org.sonar.server.issue.ws.SearchAdditionalField.ACTIONS;
 import static org.sonar.server.issue.ws.SearchAdditionalField.COMMENTS;
-import static org.sonar.server.issue.ws.SearchAdditionalField.RULES;
+import static org.sonar.server.issue.ws.SearchAdditionalField.RULE_IDS_AND_KEYS;
 import static org.sonar.server.issue.ws.SearchAdditionalField.TRANSITIONS;
 import static org.sonar.server.issue.ws.SearchAdditionalField.USERS;
 
@@ -174,16 +176,32 @@ public class SearchResponseLoader {
   }
 
   private void loadRules(SearchResponseData preloadedResponseData, Collector collector, DbSession dbSession, SearchResponseData result) {
-    if (collector.contains(RULES)) {
+    if (collector.contains(RULE_IDS_AND_KEYS)) {
       List<RuleDefinitionDto> preloadedRules = firstNonNull(preloadedResponseData.getRules(), emptyList());
-      Set<Integer> preloaedRuleKeys = preloadedRules.stream().map(RuleDefinitionDto::getId).collect(MoreCollectors.toSet());
-      Set<Integer> ruleKeysToLoad = copyOf(difference(collector.get(RULES), preloaedRuleKeys));
-      if (ruleKeysToLoad.isEmpty()) {
-        result.setRules(preloadedResponseData.getRules());
-      } else {
-        List<RuleDefinitionDto> loadedRules = dbClient.ruleDao().selectDefinitionByIds(dbSession, ruleKeysToLoad);
-        result.setRules(concat(preloadedRules.stream(), loadedRules.stream()).collect(toList(preloadedRules.size() + loadedRules.size())));
-      }
+      Set<Integer> ruleIdsToLoad = new HashSet<>();
+      Set<RuleKey> ruleKeysToLoad = new HashSet<>();
+      collector.get(RULE_IDS_AND_KEYS).forEach(o -> {
+        if (o instanceof String) {
+          try {
+            ruleIdsToLoad.add(Integer.parseInt((String) o));
+          } catch (NumberFormatException e) {
+            ruleKeysToLoad.add(RuleKey.parse((String) o));
+          }
+        } else {
+          throw new IllegalArgumentException("Unsupported object " + o + " of type " + o.getClass().getSimpleName() + " in additional field " + RULE_IDS_AND_KEYS);
+        }
+      });
+      ruleIdsToLoad.removeAll(preloadedRules.stream().map(RuleDefinitionDto::getId).collect(toList(preloadedRules.size())));
+      ruleKeysToLoad.removeAll(preloadedRules.stream().map(RuleDefinitionDto::getKey).collect(toList(preloadedRules.size())));
+
+      result.setRules(
+        Stream.of(
+          preloadedRules.stream(),
+          dbClient.ruleDao().selectDefinitionByIds(dbSession, ruleIdsToLoad).stream(),
+          dbClient.ruleDao().selectDefinitionByKeys(dbSession, ruleKeysToLoad).stream())
+          .flatMap(s -> s)
+          .distinct()
+          .collect(Collectors.toList()));
     }
   }
 
@@ -214,11 +232,10 @@ public class SearchResponseLoader {
 
   private void loadActionsAndTransitions(Collector collector, SearchResponseData result) {
     if (collector.contains(ACTIONS) || collector.contains(TRANSITIONS)) {
-      Map<String, ComponentDto> componentsByProjectUuid =
-        result.getComponents()
-          .stream()
-          .filter(ComponentDto::isRootProject)
-          .collect(MoreCollectors.uniqueIndex(ComponentDto::projectUuid));
+      Map<String, ComponentDto> componentsByProjectUuid = result.getComponents()
+        .stream()
+        .filter(ComponentDto::isRootProject)
+        .collect(MoreCollectors.uniqueIndex(ComponentDto::projectUuid));
       for (IssueDto dto : result.getIssues()) {
         // so that IssueDto can be used.
         if (collector.contains(ACTIONS)) {
@@ -271,6 +288,8 @@ public class SearchResponseLoader {
     private final Set<String> componentUuids = new HashSet<>();
     private final Set<String> projectUuids = new HashSet<>();
     private final List<String> issueKeys;
+    private final Set<Integer> ruleIds = new HashSet<>();
+    private final Set<String> ruleKeys = new HashSet<>();
 
     public Collector(Set<SearchAdditionalField> fields, List<String> issueKeys) {
       this.fields = fields;
@@ -281,7 +300,7 @@ public class SearchResponseLoader {
       for (IssueDto issue : issues) {
         componentUuids.add(issue.getComponentUuid());
         projectUuids.add(issue.getProjectUuid());
-        add(RULES, issue.getRuleId());
+        ruleIds.add(issue.getRuleId());
         add(USERS, issue.getAssignee());
         collectComponentsFromIssueLocations(issue);
       }
@@ -349,6 +368,14 @@ public class SearchResponseLoader {
     public Set<String> getProjectUuids() {
       return projectUuids;
     }
+
+    public Set<Integer> getRuleIds() {
+      return ruleIds;
+    }
+
+    public Set<String> getRuleKeys() {
+      return ruleKeys;
+    }
   }
 
   private static class KeyToIssueFunction implements Function<String, IssueDto> {