SONAR-3879 Template rules are now updated when there parent are updated

diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/rules/Rule.java b/sonar-plugin-api/src/main/java/org/sonar/api/rules/Rule.java
index 7bc4e18671cd20a8fdfd0a544133b6a73710c6cf..45bafeecf3bb4cfce8f65b2d4caa85de87cbfdb2 100644 (file)
--- a/sonar-plugin-api/src/main/java/org/sonar/api/rules/Rule.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/rules/Rule.java
@@ -495,6 +495,7 @@ public final class Rule {
       .append("cardinality", cardinality)
       .append("status", status)
       .append("language", language)
+      .append("parent", parent)
       .toString();
   }
 

diff --git a/sonar-server/src/main/java/org/sonar/server/startup/RegisterRules.java b/sonar-server/src/main/java/org/sonar/server/startup/RegisterRules.java
index d48208da65e064c2465c21fff5fb82ccedff971d..2cd9f57b4fb686fdfa75082b772eaa689552c55f 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/startup/RegisterRules.java
+++ b/sonar-server/src/main/java/org/sonar/server/startup/RegisterRules.java
@@ -20,11 +20,11 @@
 
 package org.sonar.server.startup;
 
+import com.google.common.base.Objects;
 import com.google.common.base.Predicate;
 import com.google.common.base.Strings;
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.Iterables;
-import com.google.common.collect.Lists;
 import com.google.common.collect.Multimap;
 import org.apache.commons.lang.StringUtils;
 import org.slf4j.Logger;
@@ -40,12 +40,7 @@ import org.sonar.core.i18n.RuleI18nManager;
 import org.sonar.jpa.session.DatabaseSessionFactory;
 import org.sonar.server.configuration.ProfilesManager;
 
-import java.util.Collection;
-import java.util.Date;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Locale;
-import java.util.Map;
+import java.util.*;
 
 import static com.google.common.collect.Lists.newArrayList;
 import static com.google.common.collect.Maps.newHashMap;
@@ -77,7 +72,6 @@ public final class RegisterRules {
     List<Rule> registeredRules = registerRules(existingRules, profiler, session);
     disableDeprecatedRules(existingRules, registeredRules, session);
     disableDeprecatedRepositories(existingRules, session);
-    disableDeprecatedUserRules(profiler, existingRules, session);
 
     session.commit();
 
@@ -86,29 +80,27 @@ public final class RegisterRules {
 
   private List<Rule> findAllRules(DatabaseSession session) {
     // the hardcoded repository "manual" is used for manual violations
-    return session.createQuery("from " + Rule.class.getSimpleName() + " r WHERE r.parent IS NULL AND r.pluginName<>:repository")
-        .setParameter("repository", "manual")
-        .getResultList();
+    return session.createQuery("from " + Rule.class.getSimpleName() + " r WHERE r.pluginName<>:repository")
+      .setParameter("repository", "manual")
+      .getResultList();
   }
 
   private List<Rule> registerRules(RulesByRepository existingRules, TimeProfiler profiler, DatabaseSession session) {
     List<Rule> registeredRules = newArrayList();
     for (RuleRepository repository : repositories) {
       profiler.start("Register rules [" + repository.getKey() + "/" + StringUtils.defaultString(repository.getLanguage(), "-") + "]");
-
-      registeredRules.addAll(registerRepository(repository, existingRules, session));
+      registeredRules.addAll(registerRepositoryRules(repository, existingRules, session));
+      registeredRules.addAll(registerTemplateRules(repository, existingRules, registeredRules, session));
       profiler.stop();
     }
     return registeredRules;
   }
 
-  private List<Rule> registerRepository(RuleRepository repository, RulesByRepository existingRules, DatabaseSession session) {
+  private List<Rule> registerRepositoryRules(RuleRepository repository, RulesByRepository existingRules, DatabaseSession session) {
     List<Rule> registeredRules = newArrayList();
     Map<String, Rule> ruleByKey = newHashMap();
     for (Rule rule : repository.createRules()) {
-      rule.setRepositoryKey(repository.getKey());
-      rule.setLanguage(repository.getLanguage());
-      rule.setStatus(!Strings.isNullOrEmpty(rule.getStatus()) ? rule.getStatus() : Rule.STATUS_READY);
+      updateRuleFromRepositoryInfo(rule, repository);
       validateRule(rule, repository.getKey());
       ruleByKey.put(rule.getKey(), rule);
       registeredRules.add(rule);
@@ -118,7 +110,7 @@ public final class RegisterRules {
     for (Rule persistedRule : existingRules.get(repository.getKey())) {
       Rule rule = ruleByKey.get(persistedRule.getKey());
       if (rule != null) {
-        updateRule(persistedRule, rule, session);
+        updateExistingRule(persistedRule, rule, session);
         session.saveWithoutFlush(persistedRule);
         ruleByKey.remove(rule.getKey());
       }
@@ -127,6 +119,32 @@ public final class RegisterRules {
     return registeredRules;
   }
 
+  /**
+   * Template rules do not exists in the rule repository, only in database, but have to be updated.
+   */
+  private List<Rule> registerTemplateRules(RuleRepository repository, RulesByRepository existingRules, List<Rule> registeredRules, DatabaseSession session) {
+    List<Rule> templateRules = newArrayList();
+    for (Rule persistedRule : existingRules.get(repository.getKey())) {
+      Rule parent = persistedRule.getParent();
+      if (parent != null && registeredRules.contains(parent)) {
+        persistedRule.setRepositoryKey(parent.getRepositoryKey());
+        persistedRule.setLanguage(parent.getLanguage());
+        persistedRule.setStatus(parent.getStatus());
+        persistedRule.setUpdatedAt(new Date());
+
+        session.saveWithoutFlush(persistedRule);
+        templateRules.add(persistedRule);
+      }
+    }
+    return templateRules;
+  }
+
+  private void updateRuleFromRepositoryInfo(Rule rule, RuleRepository repository) {
+    rule.setRepositoryKey(repository.getKey());
+    rule.setLanguage(repository.getLanguage());
+    rule.setStatus(Objects.firstNonNull(rule.getStatus(), Rule.STATUS_READY));
+  }
+
   private void validateRule(Rule rule, String repositoryKey) {
     validateRuleRepositoryName(rule, repositoryKey);
     validateRuleDescription(rule, repositoryKey);
@@ -143,15 +161,15 @@ public final class RegisterRules {
       if (!Strings.isNullOrEmpty(rule.getName()) && Strings.isNullOrEmpty(ruleI18nManager.getName(repositoryKey, rule.getKey(), Locale.ENGLISH))) {
         // specific case
         throw new SonarException("No description found for the rule '" + rule.getName() + "' (repository: " + repositoryKey + ") because the entry 'rule."
-            + repositoryKey + "." + rule.getKey() + ".name' is missing from the bundle.");
+          + repositoryKey + "." + rule.getKey() + ".name' is missing from the bundle.");
       } else {
         throw new SonarException("The following rule (repository: " + repositoryKey + ") must have a description: " + rule);
       }
     }
   }
 
-  private void updateRule(Rule persistedRule, Rule rule, DatabaseSession session) {
-    LOG.debug("Update rule " + rule);
+  private void updateExistingRule(Rule persistedRule, Rule rule, DatabaseSession session) {
+    LOG.debug("Update existing rule " + rule);
 
     persistedRule.setName(rule.getName());
     persistedRule.setConfigKey(rule.getConfigKey());
@@ -190,9 +208,9 @@ public final class RegisterRules {
         if (rule.getParam(persistedParam.getKey()) == null) {
           it.remove();
           session
-              .createQuery("delete from " + ActiveRuleParam.class.getSimpleName() + " where ruleParam=:param")
-              .setParameter("param", persistedParam)
-              .executeUpdate();
+            .createQuery("delete from " + ActiveRuleParam.class.getSimpleName() + " where ruleParam=:param")
+            .setParameter("param", persistedParam)
+            .executeUpdate();
         }
       }
     }
@@ -228,26 +246,6 @@ public final class RegisterRules {
     }
   }
 
-  private void disableDeprecatedUserRules(TimeProfiler profiler, RulesByRepository existingRules, DatabaseSession session) {
-    profiler.start("Disable deprecated user rules");
-    List<Integer> deprecatedUserRuleIds = Lists.newLinkedList();
-    deprecatedUserRuleIds.addAll(session.createQuery(
-        "SELECT r.id FROM " + Rule.class.getSimpleName() +
-            " r WHERE r.parent IS NOT NULL AND NOT EXISTS(FROM " + Rule.class.getSimpleName() + " p WHERE r.parent=p)").getResultList());
-
-    deprecatedUserRuleIds.addAll(session.createQuery(
-        "SELECT r.id FROM " + Rule.class.getSimpleName() +
-            " r WHERE r.parent IS NOT NULL AND EXISTS(FROM " + Rule.class.getSimpleName() + " p WHERE r.parent=p and p.status=:status)")
-        .setParameter("status", Rule.STATUS_REMOVED)
-        .getResultList());
-
-    for (Integer deprecatedUserRuleId : deprecatedUserRuleIds) {
-      Rule rule = session.getSingleResult(Rule.class, "id", deprecatedUserRuleId);
-      disable(rule, existingRules, session);
-    }
-    profiler.stop();
-  }
-
   private void disable(Rule rule, RulesByRepository existingRules, DatabaseSession session) {
     if (!rule.getStatus().equals(Rule.STATUS_REMOVED)) {
       LOG.debug("Disable rule " + rule);

diff --git a/sonar-server/src/test/java/org/sonar/server/startup/RegisterRulesTest.java b/sonar-server/src/test/java/org/sonar/server/startup/RegisterRulesTest.java
index 39dfc81fcdff6894d4f6f1dab724fb6a98215bf8..ff1b6f4f10b28432dff148232634f5ec60b9102f 100644 (file)
--- a/sonar-server/src/test/java/org/sonar/server/startup/RegisterRulesTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/startup/RegisterRulesTest.java
@@ -22,12 +22,7 @@ package org.sonar.server.startup;
 
 import org.junit.Before;
 import org.junit.Test;
-import org.sonar.api.rules.ActiveRule;
-import org.sonar.api.rules.ActiveRuleParam;
-import org.sonar.api.rules.Rule;
-import org.sonar.api.rules.RuleParam;
-import org.sonar.api.rules.RulePriority;
-import org.sonar.api.rules.RuleRepository;
+import org.sonar.api.rules.*;
 import org.sonar.api.utils.SonarException;
 import org.sonar.core.i18n.RuleI18nManager;
 import org.sonar.jpa.test.AbstractDbUnitTestCase;
@@ -38,21 +33,13 @@ import java.util.Arrays;
 import java.util.List;
 import java.util.Locale;
 
-import static org.hamcrest.Matchers.containsString;
-import static org.hamcrest.Matchers.is;
-import static org.hamcrest.Matchers.notNullValue;
-import static org.hamcrest.Matchers.nullValue;
+import static org.hamcrest.Matchers.*;
 import static org.hamcrest.number.OrderingComparisons.greaterThan;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.fail;
+import static org.junit.Assert.*;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyInt;
 import static org.mockito.Matchers.anyString;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.*;
 
 public class RegisterRulesTest extends AbstractDbUnitTestCase {
 
@@ -83,6 +70,23 @@ public class RegisterRulesTest extends AbstractDbUnitTestCase {
     assertThat(first.getParams().size(), is(2));
   }
 
+  @Test
+  public void should_update_template_rule() {
+    setupData("should_update_template_rule_language");
+    task.start();
+
+    Rule rule = getSession().getSingleResult(Rule.class, "id", 2);
+    assertThat(rule.getRepositoryKey(), is("fake"));
+    assertThat(rule.getLanguage(), is("java"));
+    assertThat(rule.getStatus(), is(Rule.STATUS_READY));
+
+    rule = getSession().getSingleResult(Rule.class, "id", 4);
+    assertThat(rule.getRepositoryKey(), is("fake"));
+    assertThat(rule.getLanguage(), is("java"));
+    // Status was READY in db, but parent status is now DEPRECATED
+    assertThat(rule.getStatus(), is(Rule.STATUS_DEPRECATED));
+  }
+
   @Test
   public void should_disable_deprecated_repositories() {
     setupData("shared");
@@ -116,6 +120,16 @@ public class RegisterRulesTest extends AbstractDbUnitTestCase {
     assertThat(rule.getUpdatedAt(), notNullValue());
   }
 
+  @Test
+  public void should_reactivate_disabled_template_rules() {
+    setupData("should_reactivate_disabled_template_rules");
+    task.start();
+
+    Rule rule = getSession().getSingleResult(Rule.class, "id", 2);
+    assertThat(rule.getStatus(), is(Rule.STATUS_READY));
+    assertThat(rule.getUpdatedAt(), notNullValue());
+  }
+
   @Test
   public void should_not_update_already_disabled_rules() {
     setupData("notUpdateAlreadyDisabledRule");
@@ -209,7 +223,7 @@ public class RegisterRulesTest extends AbstractDbUnitTestCase {
   }
 
   @Test
-  public void should_not_disable_user_rules_if_parent_is_enabled() {
+  public void should_not_disable_template_rules_if_parent_is_enabled() {
     setupData("doNotDisableUserRulesIfParentIsEnabled");
     task.start();
 
@@ -218,13 +232,15 @@ public class RegisterRulesTest extends AbstractDbUnitTestCase {
   }
 
   @Test
-  public void should_disable_user_rules_if_parent_is_disabled() {
+  public void should_disable_template_rules_if_parent_is_disabled() {
     setupData("disableUserRulesIfParentIsDisabled");
     task.start();
 
     Rule rule = getSession().getSingleResult(Rule.class, "id", 2);
     assertThat(rule.isEnabled(), is(false));
     assertThat(rule.getUpdatedAt(), notNullValue());
+
+    assertThat(getSession().getSingleResult(Rule.class, "id", 4).isEnabled(), is(false));
   }
 
   @Test

diff --git a/sonar-server/src/test/resources/org/sonar/server/startup/RegisterRulesTest/disableUserRulesIfParentIsDisabled.xml b/sonar-server/src/test/resources/org/sonar/server/startup/RegisterRulesTest/disableUserRulesIfParentIsDisabled.xml
index 5260453c80dad43b2958d3ab095db2c0b4eaf171..3d583d9b47270dea0af8265af30a57edf562fcf3 100644 (file)
--- a/sonar-server/src/test/resources/org/sonar/server/startup/RegisterRulesTest/disableUserRulesIfParentIsDisabled.xml
+++ b/sonar-server/src/test/resources/org/sonar/server/startup/RegisterRulesTest/disableUserRulesIfParentIsDisabled.xml
@@ -1,5 +1,6 @@
 <dataset>
 
+  <!-- Rule as been removed -->
   <rules id="1" plugin_rule_key="disabled_rule" plugin_name="fake" plugin_config_key="[null]" name="Disabled rule" description="[null]"
          status="REMOVED" priority="4" cardinality="MULTIPLE" parent_id="[null]"/>
 
@@ -7,4 +8,12 @@
   <rules id="2" plugin_rule_key="user_rule" plugin_name="fake" plugin_config_key="[null]" name="User rule" description="[null]"
          status="READY" priority="4" cardinality="SINGLE" parent_id="1"/>
 
+  <!-- This rule will be removed... -->
+  <rules id="3" plugin_rule_key="disabled_rule2" plugin_name="fake" plugin_config_key="old_config_key2" name="old name2" description="old description2"
+         status="READY" priority="1" cardinality="SINGLE" parent_id="[null]" />
+
+  <!-- ...so this template will be removed too -->
+  <rules id="4" plugin_rule_key="template_rule2" plugin_name="fake" plugin_config_key="[null]" name="User rule" description="[null]"
+         status="READY" priority="4" cardinality="SINGLE" parent_id="3"/>
+
 </dataset>
\ No newline at end of file

diff --git a/sonar-server/src/test/resources/org/sonar/server/startup/RegisterRulesTest/should_reactivate_disabled_template_rules.xml b/sonar-server/src/test/resources/org/sonar/server/startup/RegisterRulesTest/should_reactivate_disabled_template_rules.xml
new file mode 100644 (file)
index 0000000..a90042b
--- /dev/null
+++ b/sonar-server/src/test/resources/org/sonar/server/startup/RegisterRulesTest/should_reactivate_disabled_template_rules.xml
@@ -0,0 +1,9 @@
+<dataset>
+
+  <rules id="1" plugin_rule_key="rule1" plugin_name="fake" plugin_config_key="config1" name="One" description="Description of One"
+         status="REMOVED" priority="4" cardinality="SINGLE" parent_id="[null]"/>
+
+  <rules id="2" plugin_rule_key="user_rule" plugin_name="fake" plugin_config_key="[null]" name="User rule" description="[null]"
+         status="REMOVED" priority="4" cardinality="SINGLE" parent_id="1"/>
+
+</dataset>

diff --git a/sonar-server/src/test/resources/org/sonar/server/startup/RegisterRulesTest/should_update_template_rule_language.xml b/sonar-server/src/test/resources/org/sonar/server/startup/RegisterRulesTest/should_update_template_rule_language.xml
new file mode 100644 (file)
index 0000000..a5f4cb7
--- /dev/null
+++ b/sonar-server/src/test/resources/org/sonar/server/startup/RegisterRulesTest/should_update_template_rule_language.xml
@@ -0,0 +1,17 @@
+<dataset>
+
+  <rules id="1" plugin_rule_key="rule1" plugin_name="fake" plugin_config_key="[null]" name="Rule one" description="[null]"
+         status="READY" priority="4" cardinality="MULTIPLE" parent_id="[null]"/>
+
+  <!-- Template of rule 1 -->
+  <rules id="2" plugin_rule_key="template_rule1" plugin_name="fake" plugin_config_key="[null]" name="User rule" description="[null]"
+         status="[null]" priority="4" cardinality="SINGLE" parent_id="1"/>
+
+  <rules id="3" plugin_rule_key="rule2" plugin_name="fake" plugin_config_key="old_config_key2" name="old name2" description="old description2"
+         status="DEPRECATED" priority="1" cardinality="SINGLE" parent_id="[null]" />
+
+  <!-- Template of rule 3 -->
+  <rules id="4" plugin_rule_key="template_rule2" plugin_name="fake" plugin_config_key="[null]" name="User rule" description="[null]"
+         status="READY" priority="4" cardinality="SINGLE" parent_id="3"/>
+
+</dataset>
\ No newline at end of file