SONAR-4326 Do not disable already disabled rules at server startup

diff --git a/sonar-server/src/main/java/org/sonar/server/rule/RuleRegistration.java b/sonar-server/src/main/java/org/sonar/server/rule/RuleRegistration.java
index 9eb42a8ee1bacd6da7deb6501f59ac2bf2328f01..d62ea4e72bd856cc0395dab2ed92b93294909332 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/rule/RuleRegistration.java
+++ b/sonar-server/src/main/java/org/sonar/server/rule/RuleRegistration.java
@@ -19,6 +19,7 @@
  */
 package org.sonar.server.rule;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Function;
 import com.google.common.base.Predicate;
 import com.google.common.collect.*;
@@ -69,15 +70,22 @@ public class RuleRegistration implements Startable {
   private final RuleTagOperations ruleTagOperations;
   private final ActiveRuleDao activeRuleDao;
   private final CharacteristicDao characteristicDao;
-  private final System2 system = System2.INSTANCE;
+  private final System2 system;
 
   /**
-   * @param registerTechnicalDebtModel used only to be started after init of the technical debt model
+   * @param registerDebtModel used only to be started after init of the technical debt model
    */
   public RuleRegistration(RuleDefinitionsLoader defLoader, ProfilesManager profilesManager,
                           RuleRegistry ruleRegistry, ESRuleTags esRuleTags, RuleTagOperations ruleTagOperations,
                           MyBatis myBatis, RuleDao ruleDao, RuleTagDao ruleTagDao, ActiveRuleDao activeRuleDao, CharacteristicDao characteristicDao,
-                          RegisterDebtModel registerTechnicalDebtModel) {
+                          RegisterDebtModel registerDebtModel) {
+    this(defLoader, profilesManager, ruleRegistry, esRuleTags, ruleTagOperations, myBatis, ruleDao, ruleTagDao, activeRuleDao, characteristicDao, System2.INSTANCE);
+  }
+
+  @VisibleForTesting
+  RuleRegistration(RuleDefinitionsLoader defLoader, ProfilesManager profilesManager,
+                   RuleRegistry ruleRegistry, ESRuleTags esRuleTags, RuleTagOperations ruleTagOperations,
+                   MyBatis myBatis, RuleDao ruleDao, RuleTagDao ruleTagDao, ActiveRuleDao activeRuleDao, CharacteristicDao characteristicDao, System2 system) {
     this.defLoader = defLoader;
     this.profilesManager = profilesManager;
     this.ruleRegistry = ruleRegistry;
@@ -88,6 +96,7 @@ public class RuleRegistration implements Startable {
     this.ruleTagDao = ruleTagDao;
     this.activeRuleDao = activeRuleDao;
     this.characteristicDao = characteristicDao;
+    this.system = system;
   }
 
   @Override
@@ -402,7 +411,7 @@ public class RuleRegistration implements Startable {
       // Update copy of template rules from template
       if (ruleDto.getParentId() != null) {
         RuleDto parent = buffer.rulesById.get(ruleDto.getParentId());
-        if (parent != null && !parent.getStatus().equals(Rule.STATUS_REMOVED)) {
+        if (parent != null && !Rule.STATUS_REMOVED.equals(parent.getStatus())) {
           // TODO merge params and tags ?
           ruleDto.setLanguage(parent.getLanguage());
           ruleDto.setStatus(parent.getStatus());
@@ -416,7 +425,7 @@ public class RuleRegistration implements Startable {
           toBeRemoved = false;
         }
       }
-      if (toBeRemoved) {
+      if (toBeRemoved && !Rule.STATUS_REMOVED.equals(ruleDto.getStatus())) {
         // TODO log repository key
         LOG.info("Disable rule " + ruleDto.getRuleKey());
         ruleDto.setStatus(Rule.STATUS_REMOVED);

diff --git a/sonar-server/src/test/java/org/sonar/server/rule/RuleRegistrationTest.java b/sonar-server/src/test/java/org/sonar/server/rule/RuleRegistrationTest.java
index 17ddb6753a73bedb7d6b279733b6330be4f0a48d..e8bd583c621e498b47115ed80875bcb49981a416 100644 (file)
--- a/sonar-server/src/test/java/org/sonar/server/rule/RuleRegistrationTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/rule/RuleRegistrationTest.java
@@ -26,7 +26,9 @@ import org.sonar.api.rule.RuleStatus;
 import org.sonar.api.rule.Severity;
 import org.sonar.api.server.rule.DebtRemediationFunction;
 import org.sonar.api.server.rule.RulesDefinition;
+import org.sonar.api.utils.DateUtils;
 import org.sonar.api.utils.MessageException;
+import org.sonar.api.utils.System2;
 import org.sonar.core.persistence.AbstractDaoTestCase;
 import org.sonar.core.persistence.MyBatis;
 import org.sonar.core.qualityprofile.db.ActiveRuleDao;
@@ -36,6 +38,8 @@ import org.sonar.core.technicaldebt.db.CharacteristicDao;
 import org.sonar.server.qualityprofile.ProfilesManager;
 import org.sonar.server.startup.RegisterDebtModel;
 
+import java.util.Date;
+
 import static org.fest.assertions.Assertions.assertThat;
 import static org.fest.assertions.Fail.fail;
 import static org.mockito.Mockito.*;
@@ -58,9 +62,13 @@ public class RuleRegistrationTest extends AbstractDaoTestCase {
   RuleTagDao ruleTagDao;
   ActiveRuleDao activeRuleDao;
   CharacteristicDao characteristicDao;
+  System2 system;
+  Date date = DateUtils.parseDateTime("2014-03-17T19:10:03+0100");
 
   @Before
   public void before() {
+    system = mock(System2.class);
+    when(system.now()).thenReturn(date.getTime());
     myBatis = getMyBatis();
     ruleDao = new RuleDao(myBatis);
     ruleTagDao = new RuleTagDao(myBatis);
@@ -68,7 +76,7 @@ public class RuleRegistrationTest extends AbstractDaoTestCase {
     ruleTagOperations = new RuleTagOperations(ruleTagDao, esRuleTags);
     characteristicDao = new CharacteristicDao(myBatis);
     task = new RuleRegistration(new RuleDefinitionsLoader(mock(RuleRepositories.class), new RulesDefinition[]{new FakeRepository()}),
-      profilesManager, ruleRegistry, esRuleTags, ruleTagOperations, myBatis, ruleDao, ruleTagDao, activeRuleDao, characteristicDao, mock(RegisterDebtModel.class));
+      profilesManager, ruleRegistry, esRuleTags, ruleTagOperations, myBatis, ruleDao, ruleTagDao, activeRuleDao, characteristicDao, system);
   }
 
   @Test
@@ -151,6 +159,14 @@ public class RuleRegistrationTest extends AbstractDaoTestCase {
     checkTables("disable_deprecated_rules", EXCLUDED_COLUMN_NAMES_INCLUDING_DEBT, "rules", "rules_parameters", "rules_rule_tags", "rule_tags");
   }
 
+  @Test
+  public void not_disable_already_disabled_rules() {
+    setupData("not_disable_already_disabled_rules");
+    task.start();
+
+    checkTables("not_disable_already_disabled_rules", new String[]{"created_at", "note_data", "note_user_login", "note_created_at", "note_updated_at"}, "rules");
+  }
+
   @Test
   public void update_rule_fields() {
     setupData("update_rule_fields");

diff --git a/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistrationTest/not_disable_already_disabled_rules-result.xml b/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistrationTest/not_disable_already_disabled_rules-result.xml
new file mode 100644 (file)
index 0000000..a5faaa7
--- /dev/null
+++ b/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistrationTest/not_disable_already_disabled_rules-result.xml
@@ -0,0 +1,40 @@
+<dataset>
+
+  <rules id="1" plugin_rule_key="deprecated-key" plugin_name="deprecated-repo" plugin_config_key="[null]" name="Deprecated" description="[null]"
+         status="REMOVED" priority="4" cardinality="SINGLE" parent_id="[null]" language="[null]"
+         characteristic_id="[null]" default_characteristic_id="[null]"
+         remediation_function="[null]" default_remediation_function="[null]"
+         remediation_factor="[null]" default_remediation_factor="[null]"
+         remediation_offset="[null]" default_remediation_offset="[null]"
+         effort_to_fix_l10n_key="[null]"
+         updated_at="2014-03-16"/>
+
+  <rules id="2" plugin_rule_key="deprecated" plugin_name="fake" plugin_config_key="[null]" name="Deprecated fake" description="[null]"
+         status="REMOVED" priority="4" cardinality="SINGLE" parent_id="[null]" language="[null]"
+         characteristic_id="[null]" default_characteristic_id="[null]"
+         remediation_function="[null]" default_remediation_function="[null]"
+         remediation_factor="[null]" default_remediation_factor="[null]"
+         remediation_offset="[null]" default_remediation_offset="[null]"
+         effort_to_fix_l10n_key="[null]"
+         updated_at="2014-03-16"/>
+
+ <!-- New rules -->
+  <rules id="3" plugin_rule_key="rule1" plugin_name="fake" plugin_config_key="config1" name="One" description="Description of One"
+         status="READY" priority="4" cardinality="SINGLE" parent_id="[null]" language="java"
+         characteristic_id="[null]" default_characteristic_id="2"
+         remediation_function="[null]" default_remediation_function="LINEAR_OFFSET"
+         remediation_factor="[null]" default_remediation_factor="5d"
+         remediation_offset="[null]" default_remediation_offset="10h"
+         effort_to_fix_l10n_key="squid.S115.effortToFix"
+         updated_at="2014-03-17 19:10:03.0"/>
+
+  <rules id="4" plugin_rule_key="rule2" plugin_name="fake" plugin_config_key="[null]" name="Two" description="Description of Two"
+         status="DEPRECATED" priority="0" cardinality="SINGLE" parent_id="[null]" language="java"
+         characteristic_id="[null]" default_characteristic_id="[null]"
+         remediation_function="[null]" default_remediation_function="[null]"
+         remediation_factor="[null]" default_remediation_factor="[null]"
+         remediation_offset="[null]" default_remediation_offset="[null]"
+         effort_to_fix_l10n_key="[null]"
+         updated_at="2014-03-17 19:10:03.0"/>
+
+</dataset>

diff --git a/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistrationTest/not_disable_already_disabled_rules.xml b/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistrationTest/not_disable_already_disabled_rules.xml
new file mode 100644 (file)
index 0000000..8b0604e
--- /dev/null
+++ b/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistrationTest/not_disable_already_disabled_rules.xml
@@ -0,0 +1,23 @@
+<dataset>
+
+  <characteristics id="2" kee="MEMORY_EFFICIENCY" name="Memory Efficiency" parent_id="1" characteristic_order="1" enabled="[true]"/>
+
+  <rules id="1" plugin_rule_key="deprecated-key" plugin_name="deprecated-repo" plugin_config_key="[null]" name="Deprecated" description="[null]"
+         status="REMOVED" priority="4" cardinality="SINGLE" parent_id="[null]"
+         characteristic_id="[null]" default_characteristic_id="[null]"
+         remediation_function="[null]" default_remediation_function="[null]"
+         remediation_factor="[null]" default_remediation_factor="[null]"
+         remediation_offset="[null]" default_remediation_offset="[null]"
+         effort_to_fix_l10n_key="[null]"
+         updated_at="2014-03-16"/>
+
+  <rules id="2" plugin_rule_key="deprecated" plugin_name="fake" plugin_config_key="[null]" name="Deprecated fake" description="[null]"
+         status="REMOVED" priority="4" cardinality="SINGLE" parent_id="[null]"
+         characteristic_id="[null]" default_characteristic_id="[null]"
+         remediation_function="[null]" default_remediation_function="[null]"
+         remediation_factor="[null]" default_remediation_factor="[null]"
+         remediation_offset="[null]" default_remediation_offset="[null]"
+         effort_to_fix_l10n_key="[null]"
+         updated_at="2014-03-16"/>
+
+</dataset>