SONAR-7330 When searching active rules by profile, ignore active rule on removed rules

server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileExporters.java

@@ -19,6 +19,9 @@
  */
 package org.sonar.server.qualityprofile;
 
+import com.google.common.collect.FluentIterable;
+import com.google.common.collect.ListMultimap;
+import com.google.common.collect.Lists;
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.io.Reader;
@@ -42,6 +45,8 @@ import org.sonar.api.utils.ValidationMessages;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
 import org.sonar.db.qualityprofile.ActiveRuleDto;
+import org.sonar.db.qualityprofile.ActiveRuleDtoFunctions.ActiveRuleDtoToId;
+import org.sonar.db.qualityprofile.ActiveRuleDtoFunctions.ActiveRuleParamDtoToActiveRuleId;
 import org.sonar.db.qualityprofile.ActiveRuleParamDto;
 import org.sonar.db.qualityprofile.QualityProfileDto;
 import org.sonar.server.exceptions.BadRequestException;
@@ -124,10 +129,15 @@ public class QProfileExporters {
     DbSession dbSession = dbClient.openSession(false);
     RulesProfile target = new RulesProfile(profile.getName(), profile.getLanguage());
     try {
-      for (ActiveRuleDto activeRule : dbClient.activeRuleDao().selectByProfileKey(dbSession, profile.getKey())) {
+      List<ActiveRuleDto> activeRuleDtos = dbClient.activeRuleDao().selectByProfileKey(dbSession, profile.getKey());
+      List<ActiveRuleParamDto> activeRuleParamDtos = dbClient.activeRuleDao().selectParamsByActiveRuleIds(dbSession, Lists.transform(activeRuleDtos, ActiveRuleDtoToId.INSTANCE));
+      ListMultimap<Integer, ActiveRuleParamDto> activeRuleParamsByActiveRuleId = FluentIterable.from(activeRuleParamDtos).index(ActiveRuleParamDtoToActiveRuleId.INSTANCE);
+
+      for (ActiveRuleDto activeRule : activeRuleDtos) {
+        // TODO all rules should be loaded by using one query with all active rule keys as parameter
         Rule rule = ruleFinder.findByKey(activeRule.getKey().ruleKey());
         org.sonar.api.rules.ActiveRule wrappedActiveRule = target.activateRule(rule, RulePriority.valueOf(activeRule.getSeverityString()));
-        List<ActiveRuleParamDto> paramDtos = dbClient.activeRuleDao().selectParamsByActiveRuleId(dbSession, activeRule.getId());
+        List<ActiveRuleParamDto> paramDtos = activeRuleParamsByActiveRuleId.get(activeRule.getId());
         for (ActiveRuleParamDto activeRuleParamDto : paramDtos) {
           wrappedActiveRule.setParameter(activeRuleParamDto.getKey(), activeRuleParamDto.getValue());
         }

server/sonar-server/src/main/java/org/sonar/server/rule/RegisterRules.java

@@ -53,7 +53,9 @@ import org.sonar.db.qualityprofile.ActiveRuleParamDto;
 import org.sonar.db.rule.RuleDto;
 import org.sonar.db.rule.RuleDto.Format;
 import org.sonar.db.rule.RuleParamDto;
+import org.sonar.server.qualityprofile.ActiveRuleChange;
 import org.sonar.server.qualityprofile.RuleActivator;
+import org.sonar.server.qualityprofile.index.ActiveRuleIndexer;
 import org.sonar.server.rule.index.RuleIndexer;
 
 import static com.google.common.base.Preconditions.checkNotNull;
@@ -70,15 +72,17 @@ public class RegisterRules implements Startable {
   private final RuleActivator ruleActivator;
   private final DbClient dbClient;
   private final RuleIndexer ruleIndexer;
+  private final ActiveRuleIndexer activeRuleIndexer;
   private final Languages languages;
   private final System2 system2;
 
   public RegisterRules(RuleDefinitionsLoader defLoader, RuleActivator ruleActivator, DbClient dbClient, RuleIndexer ruleIndexer,
-                       Languages languages, System2 system2) {
+                       ActiveRuleIndexer activeRuleIndexer, Languages languages, System2 system2) {
     this.defLoader = defLoader;
     this.ruleActivator = ruleActivator;
     this.dbClient = dbClient;
     this.ruleIndexer = ruleIndexer;
+    this.activeRuleIndexer = activeRuleIndexer;
     this.languages = languages;
     this.system2 = system2;
   }
@@ -100,9 +104,11 @@ public class RegisterRules implements Startable {
         }
       }
       List<RuleDto> activeRules = processRemainingDbRules(allRules.values(), session);
-      removeActiveRulesOnStillExistingRepositories(session, activeRules, context);
+      List<ActiveRuleChange> changes = removeActiveRulesOnStillExistingRepositories(session, activeRules, context);
       session.commit();
       ruleIndexer.setEnabled(true).index();
+      activeRuleIndexer.setEnabled(true);
+      activeRuleIndexer.index(changes);
       profiler.stopDebug();
     } finally {
       session.close();
@@ -309,7 +315,7 @@ public class RegisterRules implements Startable {
           .setType(param.type().toString());
         dbClient.ruleDao().insertRuleParam(session, rule, paramDto);
         if (!StringUtils.isEmpty(param.defaultValue())) {
-          // Propagate the default value to existing active rules
+          // Propagate the default value to existing active rule parameters
           for (ActiveRuleDto activeRule : dbClient.activeRuleDao().selectByRule(session, rule)) {
             ActiveRuleParamDto activeParam = ActiveRuleParamDto.createFor(paramDto).setValue(param.defaultValue());
             dbClient.activeRuleDao().insertParam(session, activeRule, activeParam);
@@ -442,7 +448,7 @@ public class RegisterRules implements Startable {
    * The side effect of this approach is that extended repositories will not be managed the same way.
    * If an extended repository do not exists anymore, then related active rules will be removed.
    */
-  private void removeActiveRulesOnStillExistingRepositories(DbSession session, Collection<RuleDto> removedRules, RulesDefinition.Context context) {
+  private List<ActiveRuleChange> removeActiveRulesOnStillExistingRepositories(DbSession session, Collection<RuleDto> removedRules, RulesDefinition.Context context) {
     List<String> repositoryKeys = newArrayList(Iterables.transform(context.repositories(), new Function<RulesDefinition.Repository, String>() {
       @Override
       public String apply(@Nonnull RulesDefinition.Repository input) {
@@ -451,15 +457,17 @@ public class RegisterRules implements Startable {
     }
       ));
 
+    List<ActiveRuleChange> changes = new ArrayList<>();
     for (RuleDto rule : removedRules) {
       // SONAR-4642 Remove active rules only when repository still exists
       if (repositoryKeys.contains(rule.getRepositoryKey())) {
-        ruleActivator.deactivate(session, rule);
+        changes.addAll(ruleActivator.deactivate(session, rule));
       }
     }
+    return changes;
   }
 
-  private void update(DbSession session, RuleDto rule){
+  private void update(DbSession session, RuleDto rule) {
     rule.setUpdatedAt(system2.now());
     dbClient.ruleDao().update(session, rule);
   }

server/sonar-server/src/main/java/org/sonar/server/rule/ws/ActiveRuleCompleter.java

@@ -39,6 +39,7 @@ import org.sonar.api.utils.log.Loggers;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
 import org.sonar.db.qualityprofile.ActiveRuleDto;
+import org.sonar.db.qualityprofile.ActiveRuleDtoFunctions.ActiveRuleDtoToId;
 import org.sonar.db.qualityprofile.ActiveRuleKey;
 import org.sonar.db.qualityprofile.ActiveRuleParamDto;
 import org.sonar.db.qualityprofile.QualityProfileDto;
@@ -236,13 +237,4 @@ public class ActiveRuleCompleter {
     }
   }
 
-  private enum ActiveRuleDtoToId implements Function<ActiveRuleDto, Integer> {
-    INSTANCE;
-
-    @Override
-    public Integer apply(@Nonnull ActiveRuleDto input) {
-      return input.getId();
-    }
-  }
-
 }

server/sonar-server/src/test/java/org/sonar/server/rule/RegisterRulesMediumTest.java

@@ -51,6 +51,7 @@ import org.sonar.server.platform.Platform;
 import org.sonar.server.qualityprofile.QProfileService;
 import org.sonar.server.qualityprofile.QProfileTesting;
 import org.sonar.server.qualityprofile.RuleActivation;
+import org.sonar.server.qualityprofile.index.ActiveRuleIndex;
 import org.sonar.server.rule.index.RuleIndex;
 import org.sonar.server.rule.index.RuleQuery;
 import org.sonar.server.tester.ServerTester;
@@ -79,6 +80,8 @@ public class RegisterRulesMediumTest {
   RuleIndex ruleIndex = TESTER.get(RuleIndex.class);
   RuleDao ruleDao = db.ruleDao();
 
+  ActiveRuleIndex activeRuleIndex = TESTER.get(ActiveRuleIndex.class);
+
   @Before
   public void before() {
     TESTER.clearDbAndIndexes();
@@ -130,6 +133,8 @@ public class RegisterRulesMediumTest {
     });
     assertThat(ruleIndex.search(new RuleQuery().setKey(RuleTesting.XOO_X1.toString()), new SearchOptions()).getTotal()).isEqualTo(0);
     assertThat(ruleIndex.search(new RuleQuery().setKey(RuleTesting.XOO_X2.toString()), new SearchOptions()).getTotal()).isEqualTo(1);
+    assertThat(ruleIndex.search(new RuleQuery().setActivation(true), new SearchOptions()).getIds()).isEmpty();
+    assertThat(activeRuleIndex.countAllByQualityProfileKey()).isEmpty();
     assertThat(db.activeRuleDao().selectByProfileKey(dbSession, QProfileTesting.XOO_P1_KEY)).isEmpty();
   }
 
@@ -157,7 +162,7 @@ public class RegisterRulesMediumTest {
     dbSession.clearCache();
 
     assertThat(ruleIndex.search(new RuleQuery().setKey(RuleTesting.XOO_X1.toString()), new SearchOptions()).getTotal()).isEqualTo(0);
-    assertThat(db.activeRuleDao().selectByProfileKey(dbSession, QProfileTesting.XOO_P1_KEY)).hasSize(1);
+    assertThat(db.activeRuleDao().selectByProfileKey(dbSession, QProfileTesting.XOO_P1_KEY)).isEmpty();
 
     // Re-install
     register(rules);

server/sonar-server/src/test/java/org/sonar/server/rule/RegisterRulesTest.java

@@ -42,6 +42,7 @@ import org.sonar.db.rule.RuleParamDto;
 import org.sonar.server.es.EsTester;
 import org.sonar.server.es.SearchOptions;
 import org.sonar.server.qualityprofile.RuleActivator;
+import org.sonar.server.qualityprofile.index.ActiveRuleIndexer;
 import org.sonar.server.rule.index.RuleIndex;
 import org.sonar.server.rule.index.RuleIndexDefinition;
 import org.sonar.server.rule.index.RuleIndexer;
@@ -76,6 +77,7 @@ public class RegisterRulesTest {
   DbClient dbClient = dbTester.getDbClient();
 
   RuleIndexer ruleIndexer;
+  ActiveRuleIndexer activeRuleIndexer;
 
   RuleIndex ruleIndex;
 
@@ -86,6 +88,8 @@ public class RegisterRulesTest {
     ruleIndexer = new RuleIndexer(dbClient, esTester.client());
     ruleIndexer.setEnabled(true);
     ruleIndex = new RuleIndex(esTester.client());
+    activeRuleIndexer = new ActiveRuleIndexer(dbClient, esTester.client());
+    activeRuleIndexer.setEnabled(true);
   }
 
   @Test
@@ -334,7 +338,7 @@ public class RegisterRulesTest {
     Languages languages = mock(Languages.class);
     when(languages.get("java")).thenReturn(mock(Language.class));
 
-    RegisterRules task = new RegisterRules(loader, ruleActivator, dbClient, ruleIndexer, languages, system);
+    RegisterRules task = new RegisterRules(loader, ruleActivator, dbClient, ruleIndexer, activeRuleIndexer, languages, system);
     task.start();
     // Execute a commit to refresh session state as the task is using its own session
     dbTester.getSession().commit();

sonar-db/src/main/java/org/sonar/db/qualityprofile/ActiveRuleDao.java

@@ -73,6 +73,9 @@ public class ActiveRuleDao implements Dao {
     return mapper(dbSession).selectAll();
   }
 
+  /**
+   * Active rule on removed rule are NOT returned
+   */
   public List<ActiveRuleDto> selectByProfileKey(DbSession session, String profileKey) {
     return mapper(session).selectByProfileKey(profileKey);
   }

sonar-db/src/main/java/org/sonar/db/qualityprofile/ActiveRuleDtoFunctions.java

@@ -0,0 +1,49 @@
+/*
+ * 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.qualityprofile;
+
+import com.google.common.base.Function;
+import javax.annotation.Nonnull;
+
+public class ActiveRuleDtoFunctions {
+
+  private ActiveRuleDtoFunctions() {
+    // Only static methods
+  }
+
+  public enum ActiveRuleDtoToId implements Function<ActiveRuleDto, Integer> {
+    INSTANCE;
+
+    @Override
+    public Integer apply(@Nonnull ActiveRuleDto input) {
+      return input.getId();
+    }
+  }
+
+  public enum ActiveRuleParamDtoToActiveRuleId implements Function<ActiveRuleParamDto, Integer> {
+    INSTANCE;
+
+    @Override
+    public Integer apply(@Nonnull ActiveRuleParamDto input) {
+      return input.getActiveRuleId();
+    }
+  }
+}

sonar-db/src/main/java/org/sonar/db/qualityprofile/ActiveRuleMapper.java

@@ -31,9 +31,6 @@ public interface ActiveRuleMapper {
 
   void delete(int activeRuleId);
 
-  @CheckForNull
-  ActiveRuleDto selectById(Integer id);
-
   ActiveRuleDto selectByKey(@Param("profileKey") String profileKey, @Param("repository") String repository, @Param("rule") String rule);
 
   List<ActiveRuleDto> selectByKeys(@Param("keys") List<ActiveRuleKey> keys);

sonar-db/src/main/resources/org/sonar/db/qualityprofile/ActiveRuleMapper.xml

@@ -21,23 +21,6 @@
     INNER JOIN rules r ON r.id = a.rule_id
   </sql>
 
-  <!-- Should be removed when ActiveRuleDao v2 will be removed -->
-  <sql id="activeRuleColumns">
-    a.id,
-    a.profile_id as profileId,
-    a.rule_id as ruleId,
-    a.failure_level as severity,
-    a.inheritance as inheritance,
-    a.created_at as "createdAt",
-    a.updated_at as "updatedAt"
-  </sql>
-
-  <!-- Should be removed when ActiveRuleDao v2 will be removed -->
-  <sql id="activeRuleJoin">
-    INNER JOIN rules_profiles qp ON qp.id=a.profile_id
-    LEFT JOIN rules_profiles profile_parent ON profile_parent.kee=qp.parent_kee
-  </sql>
-
   <insert id="insert" parameterType="ActiveRule" keyColumn="id" useGeneratedKeys="true" keyProperty="id">
     INSERT INTO active_rules (profile_id, rule_id, failure_level, inheritance, created_at, updated_at)
     VALUES (#{profileId}, #{ruleId}, #{severity}, #{inheritance}, #{createdAt}, #{updatedAt})
@@ -57,14 +40,6 @@
     DELETE FROM active_rules WHERE id=#{id}
   </update>
 
-  <select id="selectById" parameterType="int" resultType="ActiveRule">
-    SELECT
-    <include refid="activeRuleColumns"/>
-    FROM active_rules a
-    <include refid="activeRuleJoin"/>
-    WHERE a.id=#{id}
-  </select>
-
   <select id="selectByKey" parameterType="map" resultType="ActiveRule">
     SELECT
     <include refid="activeRuleKeyColumns"/>
@@ -94,7 +69,8 @@
     SELECT
     <include refid="activeRuleKeyColumns"/>
     FROM active_rules a
-    <include refid="activeRuleKeyJoin"/>
+    INNER JOIN rules_profiles qp ON qp.id=a.profile_id
+    INNER JOIN rules r ON r.id = a.rule_id AND r.status != 'REMOVED'
     where qp.kee=#{id}
   </select>
 
@@ -120,9 +96,9 @@
 
   <select id="selectAll" parameterType="map" resultType="ActiveRule">
     select
-    <include refid="activeRuleColumns"/>
+    <include refid="activeRuleKeyColumns"/>
     from active_rules a
-    <include refid="activeRuleJoin"/>
+    <include refid="activeRuleKeyJoin"/>
   </select>
 
   <!-- Parameters -->
@@ -173,7 +149,7 @@
     <where>
       <foreach collection="ids" item="id" open="(" separator=" or " close=")">
         p.active_rule_id=#{id}
-    </foreach>
+      </foreach>
     </where>
   </select>
 

sonar-db/src/test/java/org/sonar/db/qualityprofile/ActiveRuleDaoTest.java

@@ -23,6 +23,8 @@ import java.util.Collections;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
+import org.sonar.api.rule.RuleKey;
+import org.sonar.api.rule.RuleStatus;
 import org.sonar.api.rule.Severity;
 import org.sonar.api.utils.System2;
 import org.sonar.db.DbClient;
@@ -83,6 +85,29 @@ public class ActiveRuleDaoTest {
     assertThat(underTest.selectByKeys(dbSession, asList(ActiveRuleKey.of(QPROFILE_2.getKey(), RULE_1.getKey())))).isEmpty();
   }
 
+  @Test
+  public void select_by_profile() throws Exception {
+    ActiveRuleDto activeRule1 = ActiveRuleDto.createFor(QPROFILE_1, RULE_1).setSeverity(Severity.BLOCKER);
+    ActiveRuleDto activeRule2 = ActiveRuleDto.createFor(QPROFILE_1, RULE_2).setSeverity(Severity.BLOCKER);
+    underTest.insert(dbTester.getSession(), activeRule1);
+    underTest.insert(dbTester.getSession(), activeRule2);
+    dbSession.commit();
+
+    assertThat(underTest.selectByProfileKey(dbSession, QPROFILE_1.getKey())).hasSize(2);
+    assertThat(underTest.selectByProfileKey(dbSession, QPROFILE_2.getKey())).isEmpty();
+  }
+
+  @Test
+  public void select_by_profile_ignore_removed_rules() throws Exception {
+    RuleDto removedRule = RuleTesting.newDto(RuleKey.of("removed", "rule")).setStatus(RuleStatus.REMOVED);
+    dbClient.ruleDao().insert(dbTester.getSession(), removedRule);
+    ActiveRuleDto activeRule = ActiveRuleDto.createFor(QPROFILE_1, removedRule).setSeverity(Severity.BLOCKER);
+    underTest.insert(dbTester.getSession(), activeRule);
+    dbSession.commit();
+
+    assertThat(underTest.selectByProfileKey(dbSession, QPROFILE_1.getKey())).isEmpty();
+  }
+
   @Test
   public void select_by_rule_ids() {
     ActiveRuleDto activeRule1 = ActiveRuleDto.createFor(QPROFILE_1, RULE_1).setSeverity(Severity.BLOCKER);

sonar-plugin-api/src/main/java/org/sonar/api/profiles/RulesProfile.java

@@ -21,13 +21,10 @@ package org.sonar.api.profiles;
 
 import com.google.common.base.Predicate;
 import com.google.common.collect.Iterables;
-
 import java.util.ArrayList;
 import java.util.List;
-
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
-
 import org.apache.commons.collections.CollectionUtils;
 import org.apache.commons.collections.Transformer;
 import org.apache.commons.lang.StringUtils;