From 5601d7aa1d3f865cf739bf929cd3732f25ff0d99 Mon Sep 17 00:00:00 2001 From: Teryk Bellahsene Date: Mon, 29 Feb 2016 16:04:25 +0100 Subject: [PATCH] SONAR-7330 Improve active rules loading in rules WS --- .../server/rule/ws/ActiveRuleCompleter.java | 74 ++++++++++--------- .../db/qualityprofile/ActiveRuleDao.java | 17 +++++ .../db/qualityprofile/ActiveRuleDto.java | 7 +- .../db/qualityprofile/ActiveRuleMapper.java | 2 + .../org/sonar/db/rule/RuleDtoFunctions.java | 43 +++++++++++ .../db/qualityprofile/ActiveRuleMapper.xml | 12 +++ .../db/qualityprofile/ActiveRuleDaoTest.java | 28 +++++-- 7 files changed, 139 insertions(+), 44 deletions(-) create mode 100644 sonar-db/src/main/java/org/sonar/db/rule/RuleDtoFunctions.java diff --git a/server/sonar-server/src/main/java/org/sonar/server/rule/ws/ActiveRuleCompleter.java b/server/sonar-server/src/main/java/org/sonar/server/rule/ws/ActiveRuleCompleter.java index 68fe9103c9b..15577b82ccb 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/rule/ws/ActiveRuleCompleter.java +++ b/server/sonar-server/src/main/java/org/sonar/server/rule/ws/ActiveRuleCompleter.java @@ -20,7 +20,6 @@ package org.sonar.server.rule.ws; import com.google.common.base.Function; -import com.google.common.base.Optional; import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ListMultimap; import com.google.common.collect.Lists; @@ -44,6 +43,7 @@ import org.sonar.db.qualityprofile.ActiveRuleKey; import org.sonar.db.qualityprofile.ActiveRuleParamDto; import org.sonar.db.qualityprofile.QualityProfileDto; import org.sonar.db.rule.RuleDto; +import org.sonar.db.rule.RuleDtoFunctions; import org.sonar.server.qualityprofile.ActiveRule; import org.sonar.server.qualityprofile.QProfileLoader; import org.sonar.server.rule.index.RuleQuery; @@ -54,7 +54,6 @@ import org.sonarqube.ws.Rules.ShowResponse; import static com.google.common.base.Strings.nullToEmpty; import static com.google.common.collect.Sets.newHashSet; import static java.util.Collections.singletonList; -import static org.sonar.server.ws.WsUtils.checkFoundWithOptional; /** * Add details about active rules to api/rules/search and api/rules/show @@ -80,41 +79,29 @@ public class ActiveRuleCompleter { searchResponse.setQProfiles(buildQProfiles(harvestedProfileKeys)); } - private Collection writeActiveRules(DbSession dbSession, SearchResponse.Builder response, RuleQuery query, Collection rules) { + private Collection writeActiveRules(DbSession dbSession, SearchResponse.Builder response, RuleQuery query, List rules) { Collection qProfileKeys = newHashSet(); Rules.Actives.Builder activesBuilder = response.getActivesBuilder(); String profileKey = query.getQProfileKey(); if (profileKey != null) { // Load details of active rules on the selected profile + List activeRuleDtos = dbClient.activeRuleDao().selectByProfileKey(dbSession, profileKey); + ListMultimap activeRuleParamsByActiveRuleKey = activeRuleDtosToActiveRuleParamDtos(dbSession, activeRuleDtos); + for (RuleDto rule : rules) { ActiveRule activeRule = loader.getActiveRule(ActiveRuleKey.of(profileKey, rule.getKey())); if (activeRule != null) { - Optional activeRuleDto = dbClient.activeRuleDao().selectByKey(dbSession, activeRule.key()); - checkFoundWithOptional(activeRuleDto, "Active rule with key '%s' not found", activeRule.key().toString()); - List activeRuleParamDtos = dbClient.activeRuleDao().selectParamsByActiveRuleId(dbSession, activeRuleDto.get().getId()); - ListMultimap activeRuleParamByActiveRuleKey = ArrayListMultimap.create(1, activeRuleParamDtos.size()); - activeRuleParamByActiveRuleKey.putAll(activeRule.key(), activeRuleParamDtos); - qProfileKeys = writeActiveRules(rule.getKey(), singletonList(activeRule), activeRuleParamByActiveRuleKey, activesBuilder); + qProfileKeys = writeActiveRules(rule.getKey(), singletonList(activeRule), activeRuleParamsByActiveRuleKey, activesBuilder); } } } else { // Load details of all active rules + List activeRuleDtos = dbClient.activeRuleDao().selectByRuleIds(dbSession, Lists.transform(rules, RuleDtoFunctions.toId())); + ListMultimap activeRuleParamsByActiveRuleKey = activeRuleDtosToActiveRuleParamDtos(dbSession, activeRuleDtos); + for (RuleDto rule : rules) { List activeRules = loader.findActiveRulesByRule(rule.getKey()); - List activeRuleDtos = dbClient.activeRuleDao().selectByKeys(dbSession, Lists.transform(activeRules, ActiveRuleToKey.INSTANCE)); - Map activeRuleIdsByKey = new HashMap<>(); - for (ActiveRuleDto activeRuleDto : activeRuleDtos) { - activeRuleIdsByKey.put(activeRuleDto.getId(), activeRuleDto.getKey()); - } - - List activeRuleParamDtos = dbClient.activeRuleDao().selectParamsByActiveRuleIds(dbSession, Lists.transform(activeRuleDtos, ActiveRuleDtoToId.INSTANCE)); - ListMultimap activeRuleParamsByActiveRuleKey = ArrayListMultimap.create(activeRules.size(), 10); - for (ActiveRuleParamDto activeRuleParamDto : activeRuleParamDtos) { - ActiveRuleKey activeRuleKey = activeRuleIdsByKey.get(activeRuleParamDto.getId()); - activeRuleParamsByActiveRuleKey.put(activeRuleKey, activeRuleParamDto); - } - qProfileKeys = writeActiveRules(rule.getKey(), activeRules, activeRuleParamsByActiveRuleKey, activesBuilder); } } @@ -123,6 +110,35 @@ public class ActiveRuleCompleter { return qProfileKeys; } + private static Collection writeActiveRules(RuleKey ruleKey, Collection activeRules, + ListMultimap activeRuleParamsByActiveRuleKey, Rules.Actives.Builder activesBuilder) { + Collection qProfileKeys = newHashSet(); + Rules.ActiveList.Builder activeRulesListResponse = Rules.ActiveList.newBuilder(); + for (ActiveRule activeRule : activeRules) { + activeRulesListResponse.addActiveList(buildActiveRuleResponse(activeRule, activeRuleParamsByActiveRuleKey.get(activeRule.key()))); + qProfileKeys.add(activeRule.key().qProfile()); + } + activesBuilder + .getMutableActives() + .put(ruleKey.toString(), activeRulesListResponse.build()); + return qProfileKeys; + } + + private ListMultimap activeRuleDtosToActiveRuleParamDtos(DbSession dbSession, List activeRuleDtos) { + Map activeRuleIdsByKey = new HashMap<>(); + for (ActiveRuleDto activeRuleDto : activeRuleDtos) { + activeRuleIdsByKey.put(activeRuleDto.getId(), activeRuleDto.getKey()); + } + List activeRuleParamDtos = dbClient.activeRuleDao().selectParamsByActiveRuleIds(dbSession, Lists.transform(activeRuleDtos, ActiveRuleDtoToId.INSTANCE)); + ListMultimap activeRuleParamsByActiveRuleKey = ArrayListMultimap.create(activeRuleDtos.size(), 10); + for (ActiveRuleParamDto activeRuleParamDto : activeRuleParamDtos) { + ActiveRuleKey activeRuleKey = activeRuleIdsByKey.get(activeRuleParamDto.getActiveRuleId()); + activeRuleParamsByActiveRuleKey.put(activeRuleKey, activeRuleParamDto); + } + + return activeRuleParamsByActiveRuleKey; + } + void completeShow(DbSession dbSession, RuleDto rule, ShowResponse.Builder response) { List activeRules = loader.findActiveRulesByRule(rule.getKey()); List activeRuleDtos = dbClient.activeRuleDao().selectByKeys(dbSession, Lists.transform(activeRules, ActiveRuleToKey.INSTANCE)); @@ -143,20 +159,6 @@ public class ActiveRuleCompleter { } } - private static Collection writeActiveRules(RuleKey ruleKey, Collection activeRules, - ListMultimap activeRuleParamsByActiveRuleKey, Rules.Actives.Builder activesBuilder) { - Collection qProfileKeys = newHashSet(); - Rules.ActiveList.Builder activeRulesListResponse = Rules.ActiveList.newBuilder(); - for (ActiveRule activeRule : activeRules) { - activeRulesListResponse.addActiveList(buildActiveRuleResponse(activeRule, activeRuleParamsByActiveRuleKey.get(activeRule.key()))); - qProfileKeys.add(activeRule.key().qProfile()); - } - activesBuilder - .getMutableActives() - .put(ruleKey.toString(), activeRulesListResponse.build()); - return qProfileKeys; - } - private static Rules.Active buildActiveRuleResponse(ActiveRule activeRule, List parameters) { Rules.Active.Builder activeRuleResponse = Rules.Active.newBuilder(); activeRuleResponse.setQProfile(activeRule.key().qProfile()); diff --git a/sonar-db/src/main/java/org/sonar/db/qualityprofile/ActiveRuleDao.java b/sonar-db/src/main/java/org/sonar/db/qualityprofile/ActiveRuleDao.java index d81288a728c..dd668ed118b 100644 --- a/sonar-db/src/main/java/org/sonar/db/qualityprofile/ActiveRuleDao.java +++ b/sonar-db/src/main/java/org/sonar/db/qualityprofile/ActiveRuleDao.java @@ -64,6 +64,10 @@ public class ActiveRuleDao implements Dao { return mapper(dbSession).selectByRuleId(rule.getId()); } + public List selectByRuleIds(DbSession dbSession, List ids) { + return DatabaseUtils.executeLargeInputs(ids, new RuleIdsToDtos(mapper(dbSession))); + } + // TODO As it's only used by MediumTest, it should be replaced by DbTester.countRowsOfTable() public List selectAll(DbSession dbSession) { return mapper(dbSession).selectAll(); @@ -206,4 +210,17 @@ public class ActiveRuleDao implements Dao { return mapper.selectParamsByActiveRuleIds(partitionOfIds); } } + + private static class RuleIdsToDtos implements Function, List> { + private final ActiveRuleMapper mapper; + + private RuleIdsToDtos(ActiveRuleMapper mapper) { + this.mapper = mapper; + } + + @Override + public List apply(@Nonnull List partitionOfRuleIds) { + return mapper.selectByRuleIds(partitionOfRuleIds); + } + } } diff --git a/sonar-db/src/main/java/org/sonar/db/qualityprofile/ActiveRuleDto.java b/sonar-db/src/main/java/org/sonar/db/qualityprofile/ActiveRuleDto.java index 53d14c1957d..6f0a77466e3 100644 --- a/sonar-db/src/main/java/org/sonar/db/qualityprofile/ActiveRuleDto.java +++ b/sonar-db/src/main/java/org/sonar/db/qualityprofile/ActiveRuleDto.java @@ -19,7 +19,6 @@ */ package org.sonar.db.qualityprofile; -import com.google.common.base.Preconditions; import javax.annotation.CheckForNull; import javax.annotation.Nullable; import org.apache.commons.lang.StringUtils; @@ -30,6 +29,8 @@ import org.sonar.api.rules.ActiveRule; import org.sonar.db.rule.RuleDto; import org.sonar.db.rule.SeverityUtil; +import static java.util.Objects.requireNonNull; + public class ActiveRuleDto { public static final String INHERITED = ActiveRule.INHERITED; @@ -142,8 +143,8 @@ public class ActiveRuleDto { } public static ActiveRuleDto createFor(QualityProfileDto profileDto, RuleDto ruleDto) { - Preconditions.checkNotNull(profileDto.getId(), "Profile is not persisted"); - Preconditions.checkNotNull(ruleDto.getId(), "Rule is not persisted"); + requireNonNull(profileDto.getId(), "Profile is not persisted"); + requireNonNull(ruleDto.getId(), "Rule is not persisted"); ActiveRuleDto dto = new ActiveRuleDto(); dto.setProfileId(profileDto.getId()); dto.setRuleId(ruleDto.getId()); diff --git a/sonar-db/src/main/java/org/sonar/db/qualityprofile/ActiveRuleMapper.java b/sonar-db/src/main/java/org/sonar/db/qualityprofile/ActiveRuleMapper.java index d3c67a8abe1..0c63834f8c7 100644 --- a/sonar-db/src/main/java/org/sonar/db/qualityprofile/ActiveRuleMapper.java +++ b/sonar-db/src/main/java/org/sonar/db/qualityprofile/ActiveRuleMapper.java @@ -42,6 +42,8 @@ public interface ActiveRuleMapper { List selectByRuleId(int ruleId); + List selectByRuleIds(@Param("ruleIds") List partitionOfRuleIds); + List selectByProfileKey(String key); List selectAll(); diff --git a/sonar-db/src/main/java/org/sonar/db/rule/RuleDtoFunctions.java b/sonar-db/src/main/java/org/sonar/db/rule/RuleDtoFunctions.java new file mode 100644 index 00000000000..85ec0d6f1c3 --- /dev/null +++ b/sonar-db/src/main/java/org/sonar/db/rule/RuleDtoFunctions.java @@ -0,0 +1,43 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +package org.sonar.db.rule; + +import com.google.common.base.Function; +import javax.annotation.Nonnull; + +public class RuleDtoFunctions { + private RuleDtoFunctions() { + // prevent instantiation + } + + public static Function toId() { + return ToId.INSTANCE; + } + + private enum ToId implements Function { + INSTANCE; + + @Override + public Integer apply(@Nonnull RuleDto rule) { + return rule.getId(); + } + } +} diff --git a/sonar-db/src/main/resources/org/sonar/db/qualityprofile/ActiveRuleMapper.xml b/sonar-db/src/main/resources/org/sonar/db/qualityprofile/ActiveRuleMapper.xml index 6951207cdc1..e2b43b51649 100644 --- a/sonar-db/src/main/resources/org/sonar/db/qualityprofile/ActiveRuleMapper.xml +++ b/sonar-db/src/main/resources/org/sonar/db/qualityprofile/ActiveRuleMapper.xml @@ -119,6 +119,18 @@ WHERE a.rule_id=#{ruleId} + +