SONAR-4923 Fix rule parameter validation on single select list type -> multiple values can be set

5 files changed, 40 insertions, 11 deletions

diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileActiveRuleOperations.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileActiveRuleOperations.java
index 7c2a42d1adb..9a586ab17f6 100644
--- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileActiveRuleOperations.java
+++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileActiveRuleOperations.java
@@ -21,6 +21,7 @@
 package org.sonar.server.qualityprofile;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Splitter;
 import com.google.common.base.Strings;
 import org.apache.ibatis.session.SqlSession;
 import org.elasticsearch.common.base.Predicate;
@@ -44,7 +45,6 @@ import org.sonar.server.util.TypeValidations;
 
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
-
 import java.util.Date;
 import java.util.List;
 
@@ -172,7 +172,7 @@ public class QProfileActiveRuleOperations implements ServerComponent {
     return false;
   }
 
-  public int deactivateRules(int profileId, List<Integer> activeRuleIdsToDeactivate, UserSession userSession) {
+  public int deactivateRules(List<Integer> activeRuleIdsToDeactivate, UserSession userSession) {
     validatePermission(userSession);
 
     SqlSession session = myBatis.openSession();
@@ -421,7 +421,12 @@ public class QProfileActiveRuleOperations implements ServerComponent {
 
   private void validateParam(RuleParamDto ruleParam, String value) {
     RuleParamType ruleParamType = RuleParamType.parse(ruleParam.getType());
-    typeValidations.validate(value, ruleParamType.type(), ruleParamType.options());
+    if (ruleParamType.multiple()) {
+      List<String> values = newArrayList(Splitter.on(",").trimResults().split(value));
+      typeValidations.validate(values, ruleParamType.type(), ruleParamType.values());
+    } else {
+      typeValidations.validate(value, ruleParamType.type(), ruleParamType.values());
+    }
   }
 
   private String getLoggedName(UserSession userSession) {
diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfiles.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfiles.java
index 6db47467ae3..360986b44e4 100644
--- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfiles.java
+++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfiles.java
@@ -20,9 +20,8 @@
 
 package org.sonar.server.qualityprofile;
 
-import com.google.common.collect.ImmutableList;
-
 import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableList;
 import org.sonar.api.ServerComponent;
 import org.sonar.api.component.Component;
 import org.sonar.api.profiles.ProfileExporter;
@@ -46,7 +45,6 @@ import org.sonar.server.util.Validation;
 
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
-
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
@@ -291,7 +289,7 @@ public class QProfiles implements ServerComponent {
 
   public int bulkDeactivateRule(ProfileRuleQuery query) {
     List<Integer> activeRuleIdsToDeactivate = rules.searchProfileRuleIds(query);
-    return activeRuleOperations.deactivateRules(query.profileId(), activeRuleIdsToDeactivate, UserSession.get());
+    return activeRuleOperations.deactivateRules(activeRuleIdsToDeactivate, UserSession.get());
   }
 
   public void updateActiveRuleParam(int activeRuleId, String key, @Nullable String value) {
diff --git a/sonar-server/src/main/java/org/sonar/server/util/TypeValidations.java b/sonar-server/src/main/java/org/sonar/server/util/TypeValidations.java
index e3b9e84b080..da7441778c0 100644
--- a/sonar-server/src/main/java/org/sonar/server/util/TypeValidations.java
+++ b/sonar-server/src/main/java/org/sonar/server/util/TypeValidations.java
@@ -35,6 +35,13 @@ public class TypeValidations implements ServerComponent {
     this.typeValidationList = typeValidationList;
   }
 
+  public void validate(List<String> values, String type, List<String> options) {
+    TypeValidation typeValidation = findByKey(type);
+    for (String value : values) {
+      typeValidation.validate(value, options);
+    }
+  }
+
   public void validate(String value, String type, List<String> options) {
     TypeValidation typeValidation = findByKey(type);
     typeValidation.validate(value, options);
diff --git a/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileActiveRuleOperationsTest.java b/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileActiveRuleOperationsTest.java
index e28d1341687..15d5ee4e4d4 100644
--- a/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileActiveRuleOperationsTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileActiveRuleOperationsTest.java
@@ -32,6 +32,7 @@ import org.mockito.stubbing.Answer;
 import org.sonar.api.PropertyType;
 import org.sonar.api.rule.Severity;
 import org.sonar.api.rules.RulePriority;
+import org.sonar.api.server.rule.RuleParamType;
 import org.sonar.api.utils.DateUtils;
 import org.sonar.api.utils.System2;
 import org.sonar.core.permission.GlobalPermissions;
@@ -264,13 +265,12 @@ public class QProfileActiveRuleOperationsTest {
 
   @Test
   public void deactivate_rules() throws Exception {
-    when(profileDao.selectById(1, session)).thenReturn(new QualityProfileDto().setId(1).setName("Default").setLanguage("java"));
     when(ruleDao.selectById(10, session)).thenReturn(new RuleDto().setId(10));
     ActiveRuleDto activeRule = new ActiveRuleDto().setId(5).setProfileId(1).setRuleId(10).setSeverity(1);
     when(activeRuleDao.selectById(5, session)).thenReturn(activeRule);
     when(profilesManager.deactivated(eq(1), anyInt(), eq("Nicolas"))).thenReturn(new ProfilesManager.RuleInheritanceActions());
 
-    int result = operations.deactivateRules(1, newArrayList(5), authorizedUserSession);
+    int result = operations.deactivateRules(newArrayList(5), authorizedUserSession);
 
     assertThat(result).isEqualTo(1);
     verify(activeRuleDao).delete(eq(5), eq(session));
@@ -346,6 +346,21 @@ public class QProfileActiveRuleOperationsTest {
   }
 
   @Test
+  public void update_active_rule_param_with_single_select_list_type() throws Exception {
+    ActiveRuleDto activeRule = new ActiveRuleDto().setId(5).setProfileId(1).setRuleId(10).setSeverity(1);
+    when(activeRuleDao.selectById(5, session)).thenReturn(activeRule);
+    RuleParamDto ruleParam = new RuleParamDto().setRuleId(10).setName("max").setDefaultValue("20").setType(RuleParamType.multipleListOfValues("30", "31", "32", "33").toString());
+    when(ruleDao.selectParamByRuleAndKey(10, "max", session)).thenReturn(ruleParam);
+    ActiveRuleParamDto activeRuleParam = new ActiveRuleParamDto().setId(100).setActiveRuleId(5).setKey("max").setValue("20");
+    when(activeRuleDao.selectParamByActiveRuleAndKey(5, "max", session)).thenReturn(activeRuleParam);
+    when(profilesManager.ruleParamChanged(eq(1), eq(5), eq("max"), eq("20"), eq("30, 31, 32"), eq("Nicolas"))).thenReturn(new ProfilesManager.RuleInheritanceActions());
+
+    operations.updateActiveRuleParam(5, "max", "30, 31, 32", authorizedUserSession);
+
+    verify(typeValidations).validate(eq(newArrayList("30", "31", "32")), eq("SINGLE_SELECT_LIST"), anyList());
+  }
+
+  @Test
   public void remove_active_rule_param() throws Exception {
     ActiveRuleDto activeRule = new ActiveRuleDto().setId(5).setProfileId(1).setRuleId(10).setSeverity(1);
     when(activeRuleDao.selectById(5, session)).thenReturn(activeRule);
diff --git a/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfilesTest.java b/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfilesTest.java
index 12a43f93b50..8e62e3812c0 100644
--- a/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfilesTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfilesTest.java
@@ -52,7 +52,11 @@ import static com.google.common.collect.Lists.newArrayList;
 import static com.google.common.collect.Maps.newHashMap;
 import static org.fest.assertions.Assertions.assertThat;
 import static org.fest.assertions.Fail.fail;
-import static org.mockito.Matchers.*;
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyInt;
+import static org.mockito.Matchers.anyString;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Matchers.isA;
 import static org.mockito.Mockito.*;
 
 @RunWith(MockitoJUnitRunner.class)
@@ -447,7 +451,7 @@ public class QProfilesTest {
     ProfileRuleQuery query = ProfileRuleQuery.create(1);
     when(rules.searchProfileRuleIds(query)).thenReturn(newArrayList(10));
     qProfiles.bulkDeactivateRule(query);
-    verify(activeRuleOperations).deactivateRules(eq(1), eq(newArrayList(10)), any(UserSession.class));
+    verify(activeRuleOperations).deactivateRules(eq(newArrayList(10)), any(UserSession.class));
   }
 
   @Test