From: Julien Lancelot Date: Fri, 13 Jun 2014 16:32:19 +0000 (+0200) Subject: SONAR-5380 Ignore parameters when activa X-Git-Tag: 4.4-RC1~433 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=d34e7dad15a5fda93b92af1259d5a457469c3bc6;p=sonarqube.git SONAR-5380 Ignore parameters when activa --- diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java index c0d31a0198e..535ef221b8d 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java @@ -27,11 +27,7 @@ import org.sonar.api.server.rule.RuleParamType; import org.sonar.core.log.Log; import org.sonar.core.persistence.DbSession; import org.sonar.core.preview.PreviewCache; -import org.sonar.core.qualityprofile.db.ActiveRuleDto; -import org.sonar.core.qualityprofile.db.ActiveRuleKey; -import org.sonar.core.qualityprofile.db.ActiveRuleParamDto; -import org.sonar.core.qualityprofile.db.QualityProfileDto; -import org.sonar.core.qualityprofile.db.QualityProfileKey; +import org.sonar.core.qualityprofile.db.*; import org.sonar.core.rule.RuleDto; import org.sonar.core.rule.RuleParamDto; import org.sonar.server.db.DbClient; @@ -47,6 +43,7 @@ import org.sonar.server.search.QueryOptions; import org.sonar.server.util.TypeValidations; import javax.annotation.Nullable; + import java.util.Iterator; import java.util.List; import java.util.Map; @@ -66,8 +63,8 @@ public class RuleActivator implements ServerComponent { private final LogService log; public RuleActivator(DbClient db, IndexClient index, - RuleActivatorContextFactory contextFactory, TypeValidations typeValidations, - PreviewCache previewCache, LogService log) { + RuleActivatorContextFactory contextFactory, TypeValidations typeValidations, + PreviewCache previewCache, LogService log) { this.db = db; this.index = index; this.contextFactory = contextFactory; @@ -76,7 +73,6 @@ public class RuleActivator implements ServerComponent { this.log = log; } - /** * Activate a rule on a Quality profile. Update configuration (severity/parameters) if the rule is already * activated. @@ -151,15 +147,19 @@ public class RuleActivator implements ServerComponent { * 1. defined by end-user * 2. else inherited from parent profile * 3. else defined by rule defaults + * + * On custom rules, it's always rule parameters that are used */ private void applySeverityAndParamToChange(RuleActivation activation, RuleActivatorContext context, ActiveRuleChange change) { change.setSeverity(StringUtils.defaultIfEmpty(activation.getSeverity(), context.defaultSeverity())); - verifyParametersAreNotSetOnCustomRule(context, activation, change); for (RuleParamDto ruleParamDto : context.ruleParams()) { - String value = StringUtils.defaultIfEmpty( - activation.getParameters().get(ruleParamDto.getName()), - context.defaultParam(ruleParamDto.getName())); - verifyParam(ruleParamDto, value); + String value = null; + if (context.rule().getTemplateId() == null) { + value = StringUtils.defaultIfEmpty( + activation.getParameters().get(ruleParamDto.getName()), + context.defaultParam(ruleParamDto.getName())); + verifyParam(ruleParamDto, value); + } change.setParameter(ruleParamDto.getName(), StringUtils.defaultIfEmpty(value, ruleParamDto.getDefaultValue())); } } @@ -304,7 +304,6 @@ public class RuleActivator implements ServerComponent { return changes; } - private void verifyParam(RuleParamDto ruleParam, @Nullable String value) { if (value != null) { RuleParamType ruleParamType = RuleParamType.parse(ruleParam.getType()); diff --git a/sonar-server/src/test/java/org/sonar/server/qualityprofile/RuleActivatorMediumTest.java b/sonar-server/src/test/java/org/sonar/server/qualityprofile/RuleActivatorMediumTest.java index 74a3d835be7..0fcdf7c3930 100644 --- a/sonar-server/src/test/java/org/sonar/server/qualityprofile/RuleActivatorMediumTest.java +++ b/sonar-server/src/test/java/org/sonar/server/qualityprofile/RuleActivatorMediumTest.java @@ -29,11 +29,7 @@ import org.sonar.api.rule.RuleStatus; import org.sonar.api.rule.Severity; import org.sonar.api.server.rule.RuleParamType; import org.sonar.core.persistence.DbSession; -import org.sonar.core.qualityprofile.db.ActiveRuleDto; -import org.sonar.core.qualityprofile.db.ActiveRuleKey; -import org.sonar.core.qualityprofile.db.ActiveRuleParamDto; -import org.sonar.core.qualityprofile.db.QualityProfileDto; -import org.sonar.core.qualityprofile.db.QualityProfileKey; +import org.sonar.core.qualityprofile.db.*; import org.sonar.core.rule.RuleDto; import org.sonar.core.rule.RuleParamDto; import org.sonar.server.db.DbClient; @@ -46,6 +42,7 @@ import org.sonar.server.search.QueryOptions; import org.sonar.server.tester.ServerTester; import javax.annotation.Nullable; + import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -101,7 +98,7 @@ public class RuleActivatorMediumTest { RuleDto xooCustomRule1 = RuleTesting.newCustomRule(xooTemplateRule1).setRuleKey(CUSTOM_RULE_KEY.rule()) .setSeverity("MINOR").setLanguage("xoo"); db.ruleDao().insert(dbSession, xooCustomRule1); - db.ruleDao().addRuleParam(dbSession, xooTemplateRule1, RuleParamDto.createFor(xooTemplateRule1) + db.ruleDao().addRuleParam(dbSession, xooCustomRule1, RuleParamDto.createFor(xooTemplateRule1) .setName("format").setDefaultValue("txt").setType(RuleParamType.STRING.type())); // create pre-defined profile @@ -178,7 +175,6 @@ public class RuleActivatorMediumTest { activation.setSeverity(Severity.BLOCKER); ruleActivator.activate(activation); - assertThat(db.activeRuleDao().getParamByKeyAndName(activeRuleKey, "max", dbSession)).isNotNull(); db.activeRuleDao().removeParamByKeyAndName(dbSession, activeRuleKey, "max"); dbSession.commit(); @@ -213,6 +209,22 @@ public class RuleActivatorMediumTest { verifyHasActiveRule(activeRuleKey, Severity.MINOR, null, ImmutableMap.of("max", "10")); } + @Test + public void ignore_parameters_when_activating_custom_rule() throws Exception { + // initial activation + ActiveRuleKey activeRuleKey = ActiveRuleKey.of(XOO_PROFILE_KEY, CUSTOM_RULE_KEY); + RuleActivation activation = new RuleActivation(activeRuleKey); + ruleActivator.activate(activation); + + // update + RuleActivation update = new RuleActivation(activeRuleKey) + .setParameter("format", "xls"); + ruleActivator.activate(update); + + assertThat(countActiveRules(XOO_PROFILE_KEY)).isEqualTo(1); + verifyHasActiveRule(activeRuleKey, Severity.MINOR, null, ImmutableMap.of("format", "txt")); + } + @Test public void fail_to_activate_if_template() throws Exception { RuleActivation activation = new RuleActivation(ActiveRuleKey.of(XOO_PROFILE_KEY, TEMPLATE_RULE_KEY)); @@ -313,20 +325,6 @@ public class RuleActivatorMediumTest { } } - @Test - public void fail_to_activate_if_custom_rule_template_and_parameters_are_set() throws Exception { - RuleActivation activation = new RuleActivation(ActiveRuleKey.of(XOO_PROFILE_KEY, CUSTOM_RULE_KEY)) - .setParameter("format", "xls"); - - try { - ruleActivator.activate(activation); - fail(); - } catch (Exception e) { - assertThat(e).isInstanceOf(IllegalStateException.class).hasMessage("Parameters cannot be set when activating the custom rule 'xoo:custom1'"); - verifyZeroActiveRules(XOO_PROFILE_KEY); - } - } - @Test public void deactivate() throws Exception { // activation @@ -535,7 +533,6 @@ public class RuleActivatorMediumTest { verifyOneActiveRule(XOO_GRAND_CHILD_PROFILE_KEY, XOO_RULE_1, Severity.INFO, ActiveRuleDto.INHERITED, ImmutableMap.of("max", "7")); } - @Test public void do_not_override_on_child_if_same_values() throws Exception { createChildProfiles(); @@ -831,18 +828,18 @@ public class RuleActivatorMediumTest { } private void verifyOneActiveRule(QualityProfileKey profileKey, RuleKey ruleKey, String expectedSeverity, - @Nullable String expectedInheritance, Map expectedParams) { + @Nullable String expectedInheritance, Map expectedParams) { assertThat(countActiveRules(profileKey)).isEqualTo(1); verifyHasActiveRule(profileKey, ruleKey, expectedSeverity, expectedInheritance, expectedParams); } private void verifyHasActiveRule(QualityProfileKey profileKey, RuleKey ruleKey, String expectedSeverity, - @Nullable String expectedInheritance, Map expectedParams) { + @Nullable String expectedInheritance, Map expectedParams) { verifyHasActiveRule(ActiveRuleKey.of(profileKey, ruleKey), expectedSeverity, expectedInheritance, expectedParams); } private void verifyHasActiveRule(ActiveRuleKey activeRuleKey, String expectedSeverity, - @Nullable String expectedInheritance, Map expectedParams) { + @Nullable String expectedInheritance, Map expectedParams) { // verify db boolean found = false; List activeRuleDtos = db.activeRuleDao().findByProfileKey(dbSession, activeRuleKey.qProfile());