SONAR-5841 Empty parameter on integer, boolean and float types should be authorized...

diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivation.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivation.java
index e89c21d1e3257b3188c3ffdaf51d747991e2c7c6..2df174d362509908f57d3326ca0300240c17d0d4 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivation.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivation.java
@@ -93,7 +93,9 @@ public class RuleActivation {
 
   public RuleActivation setParameters(Map<String, String> m) {
     parameters.clear();
-    parameters.putAll(m);
+    for (Map.Entry<String, String> entry : m.entrySet()) {
+      setParameter(entry.getKey(), entry.getValue());
+    }
     return this;
   }
 

diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java
index a456b77eea904a27e147ffd50601fd2444767b18..e0203e4e90cf1b4dbd7645e66d0612e19590f5a3 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java
@@ -68,8 +68,8 @@ public class RuleActivator implements ServerComponent {
   private final ActivityService log;
 
   public RuleActivator(DbClient db, IndexClient index,
-    RuleActivatorContextFactory contextFactory, TypeValidations typeValidations,
-    PreviewCache previewCache, ActivityService log) {
+                       RuleActivatorContextFactory contextFactory, TypeValidations typeValidations,
+                       PreviewCache previewCache, ActivityService log) {
     this.db = db;
     this.index = index;
     this.contextFactory = contextFactory;
@@ -201,10 +201,11 @@ public class RuleActivator implements ServerComponent {
         context.defaultSeverity()));
       for (RuleParamDto ruleParamDto : context.ruleParams()) {
         String paramKey = ruleParamDto.getName();
-        change.setParameter(paramKey, validateParam(ruleParamDto, firstNonNull(
-          context.requestParamValue(request, paramKey),
-          context.parentParamValue(paramKey),
-          context.defaultParamValue(paramKey))));
+        change.setParameter(paramKey, validateParam(ruleParamDto,
+          firstNonNull(
+            context.requestParamValue(request, paramKey),
+            context.parentParamValue(paramKey),
+            context.defaultParamValue(paramKey))));
       }
     }
   }
@@ -212,7 +213,7 @@ public class RuleActivator implements ServerComponent {
   @CheckForNull
   String firstNonNull(String... strings) {
     for (String s : strings) {
-      if (s!=null) {
+      if (s != null) {
         return s;
       }
     }

diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/RuleActivatorMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/RuleActivatorMediumTest.java
index a9f7c6f8c528083308714327307551be17e5ef62..a7da04a503931b354a22ac654b5d889516149adb 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/RuleActivatorMediumTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/RuleActivatorMediumTest.java
@@ -157,19 +157,49 @@ public class RuleActivatorMediumTest {
       ImmutableMap.of("max", "10"));
   }
 
+  /**
+   * SONAR-5841
+   */
   @Test
-  public void activate_rule_with_negative_integer_value_on_parameter_without_default_value() throws Exception {
-    RuleKey ruleKey = RuleKey.of("negative", "value");
-    RuleDto rule = RuleTesting.newDto(ruleKey).setLanguage("xoo").setSeverity(Severity.MINOR);
-    db.ruleDao().insert(dbSession, rule);
-    db.ruleDao().addRuleParam(dbSession, rule, RuleParamDto.createFor(rule)
-      .setName("max").setType(RuleParamType.INTEGER.type()));
+  public void activate_with_empty_parameter_having_no_default_value() throws Exception {
+    activate(new RuleActivation(RuleTesting.XOO_X1)
+        .setParameter("min", ""),
+      XOO_P1_KEY);
 
-    activate(new RuleActivation(ruleKey).setParameter("max", "-10"), XOO_P1_KEY);
+    assertThat(countActiveRules(XOO_P1_KEY)).isEqualTo(1);
+    verifyHasActiveRule(ActiveRuleKey.of(XOO_P1_KEY, RuleTesting.XOO_X1), Severity.MINOR, null,
+      // Max should be set to default value, min has not value it should be ignored
+      ImmutableMap.of("max", "10"));
+  }
+
+  /**
+   * SONAR-5841
+   */
+  @Test
+  public void activate_with_empty_parameters() throws Exception {
+    activate(new RuleActivation(RuleTesting.XOO_X1)
+        .setParameters(ImmutableMap.of("max", "", "min", "")),
+      XOO_P1_KEY);
+
+    assertThat(countActiveRules(XOO_P1_KEY)).isEqualTo(1);
+    // Max should be set to default value, min has not value it should be ignored
+    verifyHasActiveRule(ActiveRuleKey.of(XOO_P1_KEY, RuleTesting.XOO_X1), Severity.MINOR, null,
+      ImmutableMap.of("max", "10"));
+  }
+
+  /**
+   * SONAR-5840
+   */
+  @Test
+  public void activate_rule_with_negative_integer_value_on_parameter_having_no_default_value() throws Exception {
+    activate(new RuleActivation(RuleTesting.XOO_X1)
+        .setParameter("min", "-10"),
+      XOO_P1_KEY);
 
     assertThat(countActiveRules(XOO_P1_KEY)).isEqualTo(1);
-    verifyHasActiveRule(ActiveRuleKey.of(XOO_P1_KEY, ruleKey), Severity.MINOR, null,
-      ImmutableMap.of("max", "-10"));
+    // Max should be set to default value, min should be set to -10
+    verifyHasActiveRule(ActiveRuleKey.of(XOO_P1_KEY, RuleTesting.XOO_X1), Severity.MINOR, null,
+      ImmutableMap.of("max", "10", "min", "-10"));
   }
 
   @Test
@@ -1007,18 +1037,18 @@ public class RuleActivatorMediumTest {
   }
 
   private void verifyOneActiveRule(String 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(String 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());

diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/server/rule/RulesDefinition.java b/sonar-plugin-api/src/main/java/org/sonar/api/server/rule/RulesDefinition.java
index ac9e7c34fa329ae8743f11c5817a27097fb0ceba..75a5a2bf077a2fdb918dd60b378a2baf0ccba93d 100644 (file)
--- a/sonar-plugin-api/src/main/java/org/sonar/api/server/rule/RulesDefinition.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/server/rule/RulesDefinition.java
@@ -881,8 +881,11 @@ public interface RulesDefinition extends ServerExtension {
       return this;
     }
 
+    /**
+     * Empty default value will be converted to null.
+     */
     public NewParam setDefaultValue(@Nullable String s) {
-      this.defaultValue = s;
+      this.defaultValue = Strings.emptyToNull(s);
       return this;
     }
   }

diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/server/rule/RulesDefinitionTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/server/rule/RulesDefinitionTest.java
index aa4c87cffdedfa7b83eb7d8c399be51dfe0ddb54..56bf1b1bf6aef4b60ae029451a96e6c7100b0acf 100644 (file)
--- a/sonar-plugin-api/src/test/java/org/sonar/api/server/rule/RulesDefinitionTest.java
+++ b/sonar-plugin-api/src/test/java/org/sonar/api/server/rule/RulesDefinitionTest.java
@@ -172,6 +172,25 @@ public class RulesDefinitionTest {
     assertThat(level.hashCode()).isEqualTo(level.hashCode());
   }
 
+  @Test
+  public void define_rule_parameter_with_empty_default_value() {
+    RulesDefinition.NewRepository newFindbugs = context.createRepository("findbugs", "java");
+    RulesDefinition.NewRule newNpe = newFindbugs.createRule("NPE").setName("NPE").setHtmlDescription("NPE");
+    newNpe.createParam("level").setDefaultValue("").setName("Level").setDescription("The level").setType(RuleParamType.INTEGER);
+    newFindbugs.done();
+
+    RulesDefinition.Rule rule = context.repository("findbugs").rule("NPE");
+    assertThat(rule.params()).hasSize(1);
+
+    RulesDefinition.Param level = rule.param("level");
+    assertThat(level.key()).isEqualTo("level");
+    assertThat(level.name()).isEqualTo("Level");
+    assertThat(level.description()).isEqualTo("The level");
+    // Empty value is converted in null value
+    assertThat(level.defaultValue()).isNull();
+    assertThat(level.type()).isEqualTo(RuleParamType.INTEGER);
+  }
+
   @Test
   public void sanitize_rule_name() {
     RulesDefinition.NewRepository newFindbugs = context.createRepository("findbugs", "java");