From de7cb7d6d054a31c0bc7aafe75956d9d7a4c2dcc Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Tue, 4 Feb 2014 13:39:00 +0100 Subject: [PATCH] SONAR-4642 Profile emptied when server restarted without the related plugin --- .../sonar/server/rule/RuleRegistration.java | 100 ++++++++++++------ .../server/rule/RuleRegistrationTest.java | 23 +++- ...ules_when_repository_is_still_existing.xml | 6 ++ 3 files changed, 89 insertions(+), 40 deletions(-) create mode 100644 sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistrationTest/notify_for_removed_rules_when_repository_is_still_existing.xml 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 26549264a4d..7b6915e86a5 100644 --- 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.base.Function; import com.google.common.collect.*; import org.apache.commons.lang.ObjectUtils; import org.apache.commons.lang.StringUtils; @@ -42,8 +43,11 @@ import javax.annotation.CheckForNull; import java.util.*; +import static com.google.common.collect.Lists.newArrayList; + /** * Register rules at server startup + * * @since 4.2 */ public class RuleRegistration implements Startable { @@ -79,10 +83,12 @@ public class RuleRegistration implements Startable { TimeProfiler profiler = new TimeProfiler().start("Register rules"); SqlSession sqlSession = myBatis.openSession(); try { + RuleDefinitions.Context context = defLoader.load(); Buffer buffer = new Buffer(system.now()); selectRulesFromDb(buffer, sqlSession); - enableRuleDefinitions(buffer, sqlSession); - processRemainingDbRules(buffer, sqlSession); + enableRuleDefinitions(context, buffer, sqlSession); + List removedRules = processRemainingDbRules(buffer, sqlSession); + removeActiveRulesOnStillExistingRepositories(removedRules, context); index(buffer); ruleTagOperations.deleteUnusedTags(sqlSession); sqlSession.commit(); @@ -114,13 +120,12 @@ public class RuleRegistration implements Startable { } } - private void enableRuleDefinitions(Buffer buffer, SqlSession sqlSession) { - RuleDefinitions.Context context = defLoader.load(); + private void enableRuleDefinitions(RuleDefinitions.Context context, Buffer buffer, SqlSession sqlSession) { for (RuleDefinitions.Repository repoDef : context.repositories()) { enableRepository(buffer, sqlSession, repoDef); } for (RuleDefinitions.ExtendedRepository extendedRepoDef : context.extendedRepositories()) { - if (context.repository(extendedRepoDef.key())==null) { + if (context.repository(extendedRepoDef.key()) == null) { LOG.warn(String.format("Extension is ignored, repository %s does not exist", extendedRepoDef.key())); } else { enableRepository(buffer, sqlSession, extendedRepoDef); @@ -148,27 +153,27 @@ public class RuleRegistration implements Startable { private RuleDto enableAndInsert(Buffer buffer, SqlSession sqlSession, RuleDefinitions.Rule ruleDef) { RuleDto ruleDto = new RuleDto() - .setCardinality(ruleDef.template() ? Cardinality.MULTIPLE : Cardinality.SINGLE) - .setConfigKey(ruleDef.engineKey()) - .setDescription(ruleDef.htmlDescription()) - .setLanguage(ruleDef.repository().language()) - .setName(ruleDef.name()) - .setRepositoryKey(ruleDef.repository().key()) - .setRuleKey(ruleDef.key()) - .setSeverity(RulePriority.valueOf(ruleDef.severity()).name()) - .setCreatedAt(buffer.now()) - .setUpdatedAt(buffer.now()) - .setStatus(ruleDef.status().name()); + .setCardinality(ruleDef.template() ? Cardinality.MULTIPLE : Cardinality.SINGLE) + .setConfigKey(ruleDef.engineKey()) + .setDescription(ruleDef.htmlDescription()) + .setLanguage(ruleDef.repository().language()) + .setName(ruleDef.name()) + .setRepositoryKey(ruleDef.repository().key()) + .setRuleKey(ruleDef.key()) + .setSeverity(RulePriority.valueOf(ruleDef.severity()).name()) + .setCreatedAt(buffer.now()) + .setUpdatedAt(buffer.now()) + .setStatus(ruleDef.status().name()); ruleDao.insert(ruleDto, sqlSession); buffer.add(ruleDto); for (RuleDefinitions.Param param : ruleDef.params()) { RuleParamDto paramDto = new RuleParamDto() - .setRuleId(ruleDto.getId()) - .setDefaultValue(param.defaultValue()) - .setDescription(param.description()) - .setName(param.name()) - .setType(param.type().toString()); + .setRuleId(ruleDto.getId()) + .setDefaultValue(param.defaultValue()) + .setDescription(param.description()) + .setName(param.name()) + .setType(param.type().toString()); ruleDao.insert(paramDto, sqlSession); buffer.add(paramDto); } @@ -244,11 +249,11 @@ public class RuleRegistration implements Startable { for (RuleDefinitions.Param param : ruleDef.params()) { if (!persistedParamKeys.contains(param.key())) { RuleParamDto paramDto = new RuleParamDto() - .setRuleId(dto.getId()) - .setName(param.key()) - .setDescription(param.description()) - .setDefaultValue(param.defaultValue()) - .setType(param.type().toString()); + .setRuleId(dto.getId()) + .setName(param.key()) + .setDescription(param.description()) + .setDefaultValue(param.defaultValue()) + .setType(param.type().toString()); ruleDao.insert(paramDto, sqlSession); buffer.add(paramDto); } @@ -328,8 +333,8 @@ public class RuleRegistration implements Startable { return tagId; } - private void processRemainingDbRules(Buffer buffer, SqlSession sqlSession) { - List removedIds = Lists.newArrayList(); + private List processRemainingDbRules(Buffer buffer, SqlSession sqlSession) { + List removedRules = newArrayList(); for (Integer unprocessedRuleId : buffer.unprocessedRuleIds) { RuleDto ruleDto = buffer.rulesById.get(unprocessedRuleId); boolean toBeRemoved = true; @@ -349,21 +354,46 @@ public class RuleRegistration implements Startable { LOG.info("Disable rule " + ruleDto.getRuleKey()); ruleDto.setStatus(Rule.STATUS_REMOVED); ruleDto.setUpdatedAt(buffer.now()); - for (RuleRuleTagDto removed: buffer.tagsByRuleId.removeAll(ruleDto.getId())) { + for (RuleRuleTagDto removed : buffer.tagsByRuleId.removeAll(ruleDto.getId())) { ruleDao.deleteTag(removed, sqlSession); } ruleDao.update(ruleDto, sqlSession); - removedIds.add(ruleDto.getId()); - if (removedIds.size() % 100 == 0) { + removedRules.add(ruleDto); + if (removedRules.size() % 100 == 0) { sqlSession.commit(); } } } sqlSession.commit(); - // call to ProfileManager requires session to be committed - for (Integer removedId : removedIds) { - profilesManager.removeActivatedRules(removedId); + return removedRules; + } + + /** + * SONAR-4642 + * + * Remove active rules on repositories that still exists. + * + * For instance, if the javascript repository do not provide anymore some rules, active rules related to this rules will be removed. + * But if the javascript repository do not exists anymore, then related active rules will not be removed. + * + * 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(List removedRules, RuleDefinitions.Context context) { + List repositoryKeys = newArrayList(Iterables.transform(context.repositories(), new Function() { + @Override + public String apply(RuleDefinitions.Repository input) { + return input.key(); + } + } + )); + + for (RuleDto rule : removedRules) { + // SONAR-4642 Remove active rules only when repository still exists + if (repositoryKeys.contains(rule.getRepositoryKey())) { + profilesManager.removeActivatedRules(rule.getId()); + } } } @@ -374,7 +404,7 @@ public class RuleRegistration implements Startable { static class Buffer { private Date now; - private List unprocessedRuleIds = Lists.newArrayList(); + private List unprocessedRuleIds = newArrayList(); private Map rulesByKey = Maps.newHashMap(); private Map rulesById = Maps.newHashMap(); private Multimap paramsByRuleId = ArrayListMultimap.create(); 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 a1603ac6345..b73c177d94e 100644 --- a/sonar-server/src/test/java/org/sonar/server/rule/RuleRegistrationTest.java +++ b/sonar-server/src/test/java/org/sonar/server/rule/RuleRegistrationTest.java @@ -22,8 +22,8 @@ package org.sonar.server.rule; import org.junit.Before; import org.junit.Test; -import org.sonar.api.rule.Severity; import org.sonar.api.rule.RuleStatus; +import org.sonar.api.rule.Severity; import org.sonar.api.server.rule.RuleDefinitions; import org.sonar.core.persistence.AbstractDaoTestCase; import org.sonar.core.persistence.MyBatis; @@ -33,8 +33,7 @@ import org.sonar.core.rule.RuleTagDao; import org.sonar.server.qualityprofile.ProfilesManager; import static org.fest.assertions.Assertions.assertThat; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.*; public class RuleRegistrationTest extends AbstractDaoTestCase { @@ -78,14 +77,28 @@ public class RuleRegistrationTest extends AbstractDaoTestCase { checkTables("should_update_template_rule_language", EXCLUDED_COLUMN_NAMES, "rules"); } + /** + * SONAR-4642 + */ @Test - public void should_notify_for_removed_rules() { - setupData("shared"); + public void notify_for_removed_rules_when_repository_is_still_existing() { + setupData("notify_for_removed_rules_when_repository_is_still_existing"); task.start(); verify(profilesManager).removeActivatedRules(1); } + /** + * SONAR-4642 + */ + @Test + public void not_notify_for_removed_rules_when_repository_do_not_exists_anymore() { + setupData("shared"); + task.start(); + + verifyZeroInteractions(profilesManager); + } + @Test public void should_reactivate_disabled_rules() { setupData("should_reactivate_disabled_rules"); diff --git a/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistrationTest/notify_for_removed_rules_when_repository_is_still_existing.xml b/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistrationTest/notify_for_removed_rules_when_repository_is_still_existing.xml new file mode 100644 index 00000000000..1d08110ddd6 --- /dev/null +++ b/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistrationTest/notify_for_removed_rules_when_repository_is_still_existing.xml @@ -0,0 +1,6 @@ + + + + + -- 2.39.5