From: Julien Lancelot Date: Wed, 19 Jun 2013 14:33:40 +0000 (+0200) Subject: SONAR-3879 Template rules are now updated when there parent are updated X-Git-Tag: 3.6~5^2~5 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=71d0574eac2ef16092d8ca27cbbe36a44e4a9105;p=sonarqube.git 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 7bc4e18671c..45bafeecf3b 100644 --- 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 d48208da65e..2cd9f57b4fb 100644 --- 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 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 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 registerRules(RulesByRepository existingRules, TimeProfiler profiler, DatabaseSession session) { List 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 registerRepository(RuleRepository repository, RulesByRepository existingRules, DatabaseSession session) { + private List registerRepositoryRules(RuleRepository repository, RulesByRepository existingRules, DatabaseSession session) { List registeredRules = newArrayList(); Map 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 registerTemplateRules(RuleRepository repository, RulesByRepository existingRules, List registeredRules, DatabaseSession session) { + List 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 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 39dfc81fcdf..ff1b6f4f10b 100644 --- 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 5260453c80d..3d583d9b472 100644 --- 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 @@ + @@ -7,4 +8,12 @@ + + + + + + \ 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 index 00000000000..a90042bd505 --- /dev/null +++ b/sonar-server/src/test/resources/org/sonar/server/startup/RegisterRulesTest/should_reactivate_disabled_template_rules.xml @@ -0,0 +1,9 @@ + + + + + + + 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 index 00000000000..a5f4cb7e72f --- /dev/null +++ b/sonar-server/src/test/resources/org/sonar/server/startup/RegisterRulesTest/should_update_template_rule_language.xml @@ -0,0 +1,17 @@ + + + + + + + + + + + + + \ No newline at end of file