From: Simon Brandhof <simon.brandhof@sonarsource.com>
Date: Mon, 7 Jul 2014 08:43:12 +0000 (+0200)
Subject: SONAR-5007 fix update of param activation when no default value
X-Git-Tag: 4.4-RC2~7
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=40a4c471d55c31c4f261be99599769f0d7d4e220;p=sonarqube.git

SONAR-5007 fix update of param activation when no default value
---

diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContext.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContext.java
index 81c470690ba..e779be00911 100644
--- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContext.java
+++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContext.java
@@ -214,7 +214,10 @@ class RuleActivatorContext {
     }
     for (Map.Entry<String, String> changeParam : change.getParameters().entrySet()) {
       ActiveRuleParamDto param = activeRuleParams.get(changeParam.getKey());
-      if (param != null && !StringUtils.equals(changeParam.getValue(), param.getValue())) {
+      if (changeParam.getValue()==null && param != null && param.getValue()!=null) {
+        return false;
+      }
+      if (changeParam.getValue()!=null && (param == null || !StringUtils.equals(changeParam.getValue(), param.getValue()))) {
         return false;
       }
     }
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 3e1f5ae6c50..5d55a161de7 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
@@ -112,12 +112,14 @@ public class RuleActivatorMediumTest {
     RuleActivation activation = new RuleActivation(RuleTesting.XOO_X1);
     activation.setSeverity(Severity.BLOCKER);
     activation.setParameter("max", "7");
+    activation.setParameter("min", "3");
     List<ActiveRuleChange> changes = ruleActivator.activate(dbSession, activation, XOO_P1_KEY);
     dbSession.commit();
     dbSession.clearCache();
 
     assertThat(countActiveRules(XOO_P1_KEY)).isEqualTo(1);
-    verifyHasActiveRule(ActiveRuleKey.of(XOO_P1_KEY, RuleTesting.XOO_X1), Severity.BLOCKER, null, ImmutableMap.of("max", "7"));
+    verifyHasActiveRule(ActiveRuleKey.of(XOO_P1_KEY, RuleTesting.XOO_X1), Severity.BLOCKER, null,
+      ImmutableMap.of("max", "7", "min", "3"));
     assertThat(changes).hasSize(1);
     assertThat(changes.get(0).getType()).isEqualTo(ActiveRuleChange.Type.ACTIVATED);
   }
@@ -127,7 +129,8 @@ public class RuleActivatorMediumTest {
     activate(new RuleActivation(RuleTesting.XOO_X1), XOO_P1_KEY);
 
     assertThat(countActiveRules(XOO_P1_KEY)).isEqualTo(1);
-    verifyHasActiveRule(ActiveRuleKey.of(XOO_P1_KEY, RuleTesting.XOO_X1), Severity.MINOR, null, ImmutableMap.of("max", "10"));
+    verifyHasActiveRule(ActiveRuleKey.of(XOO_P1_KEY, RuleTesting.XOO_X1), Severity.MINOR, null,
+      ImmutableMap.of("max", "10"));
   }
 
   @Test
@@ -160,21 +163,21 @@ public class RuleActivatorMediumTest {
   }
 
   @Test
-  @Ignore
   public void update_activation_with_parameter_without_default_value() throws Exception {
-    // initial activation
+    // initial activation -> param "max" has a default value
     RuleActivation activation = new RuleActivation(RuleTesting.XOO_X1);
     activation.setSeverity(Severity.BLOCKER);
     activate(activation, XOO_P1_KEY);
+    verifyHasActiveRule(ActiveRuleKey.of(XOO_P1_KEY, RuleTesting.XOO_X1), Severity.BLOCKER, null,
+      ImmutableMap.of("max", "10"));
 
-    // update
+    // update param "min", which has no default value
     RuleActivation update = new RuleActivation(RuleTesting.XOO_X1);
-    // parameter min has no default value
-    update.setParameter("min", "42");
+    update.setParameter("min", "3");
     List<ActiveRuleChange> changes = activate(update, XOO_P1_KEY);
-
     assertThat(countActiveRules(XOO_P1_KEY)).isEqualTo(1);
-    verifyHasActiveRule(ActiveRuleKey.of(XOO_P1_KEY, RuleTesting.XOO_X1), Severity.BLOCKER, null, ImmutableMap.of("min", "42"));
+    verifyHasActiveRule(ActiveRuleKey.of(XOO_P1_KEY, RuleTesting.XOO_X1), Severity.BLOCKER, null,
+      ImmutableMap.of("min", "3", "max", "10"));
     assertThat(changes).hasSize(1);
     assertThat(changes.get(0).getType()).isEqualTo(ActiveRuleChange.Type.UPDATED);
   }