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 c0d31a0198e15e16605fdc4541cd78c9e417e297..535ef221b8d10546b5b37fdbe3e6e2ade8dfdfe0 100644 (file)
--- 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 74a3d835be72acc799710d670c5ce5daa61edae5..0fcdf7c3930504e8e90be64ac9c75309c82bb4cd 100644 (file)
--- 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<String, String> expectedParams) {
+    @Nullable String expectedInheritance, Map<String, String> 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<String, String> expectedParams) {
+    @Nullable String expectedInheritance, Map<String, String> expectedParams) {
     verifyHasActiveRule(ActiveRuleKey.of(profileKey, ruleKey), expectedSeverity, expectedInheritance, expectedParams);
   }
 
   private void verifyHasActiveRule(ActiveRuleKey activeRuleKey, String expectedSeverity,
-                                   @Nullable String expectedInheritance, Map<String, String> expectedParams) {
+    @Nullable String expectedInheritance, Map<String, String> expectedParams) {
     // verify db
     boolean found = false;
     List<ActiveRuleDto> activeRuleDtos = db.activeRuleDao().findByProfileKey(dbSession, activeRuleKey.qProfile());