SONAR-7330 Improve active rules loading in rules WS

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 68fe9103c9ba267484af7648425bdb021325207a..15577b82ccba47d62c6d2633785cc988e810578f 100644 (file)
--- 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<String> writeActiveRules(DbSession dbSession, SearchResponse.Builder response, RuleQuery query, Collection<RuleDto> rules) {
+  private Collection<String> writeActiveRules(DbSession dbSession, SearchResponse.Builder response, RuleQuery query, List<RuleDto> rules) {
     Collection<String> 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<ActiveRuleDto> activeRuleDtos = dbClient.activeRuleDao().selectByProfileKey(dbSession, profileKey);
+      ListMultimap<ActiveRuleKey, ActiveRuleParamDto> activeRuleParamsByActiveRuleKey = activeRuleDtosToActiveRuleParamDtos(dbSession, activeRuleDtos);
+
       for (RuleDto rule : rules) {
         ActiveRule activeRule = loader.getActiveRule(ActiveRuleKey.of(profileKey, rule.getKey()));
         if (activeRule != null) {
-          Optional<ActiveRuleDto> activeRuleDto = dbClient.activeRuleDao().selectByKey(dbSession, activeRule.key());
-          checkFoundWithOptional(activeRuleDto, "Active rule with key '%s' not found", activeRule.key().toString());
-          List<ActiveRuleParamDto> activeRuleParamDtos = dbClient.activeRuleDao().selectParamsByActiveRuleId(dbSession, activeRuleDto.get().getId());
-          ListMultimap<ActiveRuleKey, ActiveRuleParamDto> 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<ActiveRuleDto> activeRuleDtos = dbClient.activeRuleDao().selectByRuleIds(dbSession, Lists.transform(rules, RuleDtoFunctions.toId()));
+      ListMultimap<ActiveRuleKey, ActiveRuleParamDto> activeRuleParamsByActiveRuleKey = activeRuleDtosToActiveRuleParamDtos(dbSession, activeRuleDtos);
+
       for (RuleDto rule : rules) {
         List<ActiveRule> activeRules = loader.findActiveRulesByRule(rule.getKey());
-        List<ActiveRuleDto> activeRuleDtos = dbClient.activeRuleDao().selectByKeys(dbSession, Lists.transform(activeRules, ActiveRuleToKey.INSTANCE));
-        Map<Integer, ActiveRuleKey> activeRuleIdsByKey = new HashMap<>();
-        for (ActiveRuleDto activeRuleDto : activeRuleDtos) {
-          activeRuleIdsByKey.put(activeRuleDto.getId(), activeRuleDto.getKey());
-        }
-
-        List<ActiveRuleParamDto> activeRuleParamDtos = dbClient.activeRuleDao().selectParamsByActiveRuleIds(dbSession, Lists.transform(activeRuleDtos, ActiveRuleDtoToId.INSTANCE));
-        ListMultimap<ActiveRuleKey, ActiveRuleParamDto> 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<String> writeActiveRules(RuleKey ruleKey, Collection<ActiveRule> activeRules,
+    ListMultimap<ActiveRuleKey, ActiveRuleParamDto> activeRuleParamsByActiveRuleKey, Rules.Actives.Builder activesBuilder) {
+    Collection<String> 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<ActiveRuleKey, ActiveRuleParamDto> activeRuleDtosToActiveRuleParamDtos(DbSession dbSession, List<ActiveRuleDto> activeRuleDtos) {
+    Map<Integer, ActiveRuleKey> activeRuleIdsByKey = new HashMap<>();
+    for (ActiveRuleDto activeRuleDto : activeRuleDtos) {
+      activeRuleIdsByKey.put(activeRuleDto.getId(), activeRuleDto.getKey());
+    }
+    List<ActiveRuleParamDto> activeRuleParamDtos = dbClient.activeRuleDao().selectParamsByActiveRuleIds(dbSession, Lists.transform(activeRuleDtos, ActiveRuleDtoToId.INSTANCE));
+    ListMultimap<ActiveRuleKey, ActiveRuleParamDto> 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<ActiveRule> activeRules = loader.findActiveRulesByRule(rule.getKey());
     List<ActiveRuleDto> activeRuleDtos = dbClient.activeRuleDao().selectByKeys(dbSession, Lists.transform(activeRules, ActiveRuleToKey.INSTANCE));
@@ -143,20 +159,6 @@ public class ActiveRuleCompleter {
     }
   }
 
-  private static Collection<String> writeActiveRules(RuleKey ruleKey, Collection<ActiveRule> activeRules,
-    ListMultimap<ActiveRuleKey, ActiveRuleParamDto> activeRuleParamsByActiveRuleKey, Rules.Actives.Builder activesBuilder) {
-    Collection<String> 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<ActiveRuleParamDto> 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 d81288a728c73ca37d22dadab0e4b1ed8dfec488..dd668ed118b63546af4573123779e14d356d54bc 100644 (file)
--- 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<ActiveRuleDto> selectByRuleIds(DbSession dbSession, List<Integer> 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<ActiveRuleDto> 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<Integer>, List<ActiveRuleDto>> {
+    private final ActiveRuleMapper mapper;
+
+    private RuleIdsToDtos(ActiveRuleMapper mapper) {
+      this.mapper = mapper;
+    }
+
+    @Override
+    public List<ActiveRuleDto> apply(@Nonnull List<Integer> 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 53d14c1957d9bc7304be35cd3eb66327a75700ee..6f0a77466e3785bffc71096dc08b75e9d7426787 100644 (file)
--- 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 d3c67a8abe1ae4bd62e82d93afabe617d024051b..0c63834f8c796b330771bca426e074ea86c941a4 100644 (file)
--- 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<ActiveRuleDto> selectByRuleId(int ruleId);
 
+  List<ActiveRuleDto> selectByRuleIds(@Param("ruleIds") List<Integer> partitionOfRuleIds);
+
   List<ActiveRuleDto> selectByProfileKey(String key);
 
   List<ActiveRuleDto> 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 (file)
index 0000000..85ec0d6
--- /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<RuleDto, Integer> toId() {
+    return ToId.INSTANCE;
+  }
+
+  private enum ToId implements Function<RuleDto, Integer> {
+    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 6951207cdc16052d27003f1f5e5c2b6364e6cf2e..e2b43b516497a213ec38a100710b4d85dc8112ce 100644 (file)
--- 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}
   </select>
 
+  <select id="selectByRuleIds" parameterType="List" resultType="ActiveRule">
+    SELECT
+    <include refid="activeRuleKeyColumns"/>
+    FROM active_rules a
+    <include refid="activeRuleKeyJoin"/>
+    WHERE
+    a.rule_id in
+    <foreach collection="ruleIds" item="ruleId" separator="," open="(" close=")">
+      #{ruleId}
+    </foreach>
+  </select>
+
   <select id="selectAll" parameterType="map" resultType="ActiveRule">
     select
     <include refid="activeRuleColumns"/>

diff --git a/sonar-db/src/test/java/org/sonar/db/qualityprofile/ActiveRuleDaoTest.java b/sonar-db/src/test/java/org/sonar/db/qualityprofile/ActiveRuleDaoTest.java
index 2286653cbfa3b1d7652f90d3a7d890777979f7f3..bf20c99f5b0ba26493a1cac306b1b16b487eacbd 100644 (file)
--- a/sonar-db/src/test/java/org/sonar/db/qualityprofile/ActiveRuleDaoTest.java
+++ b/sonar-db/src/test/java/org/sonar/db/qualityprofile/ActiveRuleDaoTest.java
@@ -19,6 +19,7 @@
  */
 package org.sonar.db.qualityprofile;
 
+import java.util.Collections;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
@@ -30,6 +31,7 @@ import org.sonar.db.DbTester;
 import org.sonar.db.rule.RuleDto;
 import org.sonar.db.rule.RuleTesting;
 
+import static com.google.common.collect.Lists.newArrayList;
 import static java.util.Arrays.asList;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.mock;
@@ -41,11 +43,11 @@ public class ActiveRuleDaoTest {
 
   static final long NOW = 10000000L;
 
-  static final QualityProfileDto QPROFILE_1 = QualityProfileDto.createFor("qp1").setName("QProile1");
-  static final QualityProfileDto QPROFILE_2 = QualityProfileDto.createFor("qp2").setName("QProile2");
+  QualityProfileDto QPROFILE_1 = QualityProfileDto.createFor("qp1").setName("QProile1");
+  QualityProfileDto QPROFILE_2 = QualityProfileDto.createFor("qp2").setName("QProile2");
 
-  static final RuleDto RULE_1 = RuleTesting.newDto(RuleTesting.XOO_X1);
-  static final RuleDto RULE_2 = RuleTesting.newDto(RuleTesting.XOO_X2);
+  RuleDto RULE_1 = RuleTesting.newDto(RuleTesting.XOO_X1);
+  RuleDto RULE_2 = RuleTesting.newDto(RuleTesting.XOO_X2);
 
   System2 system = mock(System2.class);
 
@@ -58,7 +60,7 @@ public class ActiveRuleDaoTest {
   ActiveRuleDao underTest = dbTester.getDbClient().activeRuleDao();
 
   @Before
-  public void createDao() {
+  public void setUp() {
     when(system.now()).thenReturn(NOW);
 
     dbClient.qualityProfileDao().insert(dbTester.getSession(), QPROFILE_1);
@@ -80,4 +82,20 @@ public class ActiveRuleDaoTest {
     assertThat(underTest.selectByKeys(dbSession, asList(activeRule1.getKey()))).hasSize(1);
     assertThat(underTest.selectByKeys(dbSession, asList(ActiveRuleKey.of(QPROFILE_2.getKey(), RULE_1.getKey())))).isEmpty();
   }
+
+  @Test
+  public void select_by_rule_ids() {
+    ActiveRuleDto activeRule1 = ActiveRuleDto.createFor(QPROFILE_1, RULE_1).setSeverity(Severity.BLOCKER);
+    ActiveRuleDto activeRule2 = ActiveRuleDto.createFor(QPROFILE_1, RULE_2).setSeverity(Severity.BLOCKER);
+    ActiveRuleDto activeRule3 = ActiveRuleDto.createFor(QPROFILE_2, RULE_1).setSeverity(Severity.BLOCKER);
+    underTest.insert(dbSession, activeRule1);
+    underTest.insert(dbSession, activeRule2);
+    underTest.insert(dbSession, activeRule3);
+    dbSession.commit();
+
+    assertThat(underTest.selectByRuleIds(dbSession, Collections.singletonList(RULE_1.getId())))
+      .extracting("key").containsOnly(activeRule1.getKey(), activeRule3.getKey());
+    assertThat(underTest.selectByRuleIds(dbSession, newArrayList(RULE_1.getId(), RULE_2.getId())))
+      .extracting("key").containsOnly(activeRule1.getKey(), activeRule2.getKey(), activeRule3.getKey());
+  }
 }