From 718a90d0e954cb4f1cbb1c1392bc5726a7857e1a Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Tue, 13 Feb 2018 12:29:11 +0100 Subject: [PATCH] SONAR-10313 fix error when displaying rule facet --- .../sonar/server/issue/ws/SearchAction.java | 58 ++++++++++++------ .../issue/ws/SearchAdditionalField.java | 3 +- .../server/issue/ws/SearchResponseFormat.java | 2 +- .../server/issue/ws/SearchResponseLoader.java | 59 ++++++++++++++----- 4 files changed, 87 insertions(+), 35 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchAction.java b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchAction.java index 0ec420128dd..a76a989689e 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchAction.java +++ b/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 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 alreadyLoadedRules) { - if (facets == null || facets.get(PARAM_RULES) == null) { + if (facets == null) { + return; + } + LinkedHashMap 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 rulesFacet = facets.get(PARAM_RULES); - try (DbSession dbSession = dbClient.openSession(false)) { - Set ruleKeysToLoad = new HashSet<>(rulesFacet.keySet()); - ruleKeysToLoad.removeAll( + Set 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 idToRuleKey = Stream.concat( + List 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 ruleKeyById = ruleDefinitions.stream() .collect(Collectors.toMap(RuleDefinitionDto::getId, RuleDefinitionDto::getKey)); + Map idByRuleKeyAsString = ruleDefinitions.stream() + .collect(Collectors.toMap(s -> s.getKey().toString(), RuleDefinitionDto::getId)); LinkedHashMap 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 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)); diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchAdditionalField.java b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchAdditionalField.java index 4fa8217c1cd..dd26aff6bd4 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchAdditionalField.java +++ b/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"); diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchResponseFormat.java b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchResponseFormat.java index 9f5ddbb07f3..e41a988935c 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchResponseFormat.java +++ b/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)) { diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchResponseLoader.java b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchResponseLoader.java index 9dc3608d4d5..9c793b935de 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchResponseLoader.java +++ b/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 preloadedRules = firstNonNull(preloadedResponseData.getRules(), emptyList()); - Set preloaedRuleKeys = preloadedRules.stream().map(RuleDefinitionDto::getId).collect(MoreCollectors.toSet()); - Set ruleKeysToLoad = copyOf(difference(collector.get(RULES), preloaedRuleKeys)); - if (ruleKeysToLoad.isEmpty()) { - result.setRules(preloadedResponseData.getRules()); - } else { - List loadedRules = dbClient.ruleDao().selectDefinitionByIds(dbSession, ruleKeysToLoad); - result.setRules(concat(preloadedRules.stream(), loadedRules.stream()).collect(toList(preloadedRules.size() + loadedRules.size()))); - } + Set ruleIdsToLoad = new HashSet<>(); + Set 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 componentsByProjectUuid = - result.getComponents() - .stream() - .filter(ComponentDto::isRootProject) - .collect(MoreCollectors.uniqueIndex(ComponentDto::projectUuid)); + Map 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 componentUuids = new HashSet<>(); private final Set projectUuids = new HashSet<>(); private final List issueKeys; + private final Set ruleIds = new HashSet<>(); + private final Set ruleKeys = new HashSet<>(); public Collector(Set fields, List 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 getProjectUuids() { return projectUuids; } + + public Set getRuleIds() { + return ruleIds; + } + + public Set getRuleKeys() { + return ruleKeys; + } } private static class KeyToIssueFunction implements Function { -- 2.39.5