SONAR-9051 Ability to set the maximum number of values on a WS parameter

6 files changed, 118 insertions, 57 deletions

diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/Request.java b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/Request.java
index 5b071f7cfc7..d8a6b9ce39c 100644
--- a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/Request.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/Request.java
@@ -21,7 +21,6 @@ package org.sonar.api.server.ws;
 
 import com.google.common.annotations.Beta;
 import com.google.common.base.Splitter;
-import com.google.common.collect.Lists;
 import java.io.InputStream;
 import java.util.ArrayList;
 import java.util.Date;
@@ -36,6 +35,7 @@ import org.sonar.api.utils.DateUtils;
 import org.sonar.api.utils.SonarException;
 
 import static com.google.common.base.Preconditions.checkArgument;
+import static java.lang.String.format;
 import static java.util.Objects.requireNonNull;
 import static org.sonar.api.utils.DateUtils.parseDateQuietly;
 import static org.sonar.api.utils.DateUtils.parseDateTimeQuietly;
@@ -71,7 +71,7 @@ public abstract class Request {
    */
   public String mandatoryParam(String key) {
     String value = param(key);
-    checkArgument(value != null, String.format(MSG_PARAMETER_MISSING, key));
+    checkArgument(value != null, format(MSG_PARAMETER_MISSING, key));
     return value;
   }
 
@@ -111,27 +111,18 @@ public abstract class Request {
 
   public List<String> mandatoryParamAsStrings(String key) {
     List<String> values = paramAsStrings(key);
-    if (values == null) {
-      throw new IllegalArgumentException(String.format(MSG_PARAMETER_MISSING, key));
-    }
+    checkArgument(values != null, format(MSG_PARAMETER_MISSING, key));
     return values;
   }
 
   public List<String> mandatoryMultiParam(String key) {
     List<String> values = multiParam(key);
     checkArgument(!values.isEmpty(), MSG_PARAMETER_MISSING, key);
-
     return values;
   }
 
   @CheckForNull
-  public List<String> paramAsStrings(String key) {
-    String value = param(key);
-    if (value == null) {
-      return null;
-    }
-    return Lists.newArrayList(Splitter.on(',').omitEmptyStrings().trimResults().split(value));
-  }
+  public abstract List<String> paramAsStrings(String key);
 
   @CheckForNull
   public abstract String param(String key);
@@ -267,14 +258,14 @@ public abstract class Request {
     if ("false".equals(value) || "no".equals(value)) {
       return false;
     }
-    throw new IllegalArgumentException(String.format("Property %s is not a boolean value: %s", key, value));
+    throw new IllegalArgumentException(format("Property %s is not a boolean value: %s", key, value));
   }
 
   private static int parseInt(String key, String value) {
     try {
       return Integer.parseInt(value);
     } catch (NumberFormatException expection) {
-      throw new IllegalArgumentException(String.format("The '%s' parameter cannot be parsed as an integer value: %s", key, value));
+      throw new IllegalArgumentException(format("The '%s' parameter cannot be parsed as an integer value: %s", key, value));
     }
   }
 
@@ -282,7 +273,7 @@ public abstract class Request {
     try {
       return Long.parseLong(value);
     } catch (NumberFormatException exception) {
-      throw new IllegalArgumentException(String.format("The '%s' parameter cannot be parsed as a long value: %s", key, value));
+      throw new IllegalArgumentException(format("The '%s' parameter cannot be parsed as a long value: %s", key, value));
     }
   }
 
diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/WebService.java b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/WebService.java
index 61a6bfe04a6..d9184442ac1 100644
--- a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/WebService.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/WebService.java
@@ -636,6 +636,7 @@ public interface WebService extends Definable<WebService.Context> {
     private boolean required = false;
     private boolean internal = false;
     private Set<String> possibleValues = null;
+    private Integer maxValuesAllowed = null;
 
     private NewParam(String key) {
       this.key = key;
@@ -765,6 +766,14 @@ public interface WebService extends Definable<WebService.Context> {
       return this;
     }
 
+    /**
+     * @since 6.4
+     */
+    public NewParam setMaxValuesAllowed(@Nullable Integer maxValuesAllowed) {
+      this.maxValuesAllowed = maxValuesAllowed;
+      return this;
+    }
+
     @Override
     public String toString() {
       return key;
@@ -818,6 +827,7 @@ public interface WebService extends Definable<WebService.Context> {
     private final boolean required;
     private final boolean internal;
     private final Set<String> possibleValues;
+    private final Integer maxValuesAllowed;
 
     protected Param(Action action, NewParam newParam) {
       this.key = newParam.key;
@@ -831,9 +841,8 @@ public interface WebService extends Definable<WebService.Context> {
       this.required = newParam.required;
       this.internal = newParam.internal;
       this.possibleValues = newParam.possibleValues;
-      if (required && defaultValue != null) {
-        throw new IllegalArgumentException(format("Default value must not be set on parameter '%s?%s' as it's marked as required", action, key));
-      }
+      this.maxValuesAllowed = newParam.maxValuesAllowed;
+      checkArgument(!required || defaultValue == null, "Default value must not be set on parameter '%s?%s' as it's marked as required", action, key);
     }
 
     public String key() {
@@ -920,6 +929,15 @@ public interface WebService extends Definable<WebService.Context> {
       return defaultValue;
     }
 
+    /**
+     * Specify the maximum number of values allowed when using this a parameter
+     *
+     * @since 6.4
+     */
+    public Integer maxValuesAllowed() {
+      return maxValuesAllowed;
+    }
+
     @Override
     public String toString() {
       return key;
diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/internal/SimpleGetRequest.java b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/internal/SimpleGetRequest.java
index f35960dd1a2..ac3b607fe31 100644
--- a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/internal/SimpleGetRequest.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/internal/SimpleGetRequest.java
@@ -19,10 +19,13 @@
  */
 package org.sonar.api.server.ws.internal;
 
+import com.google.common.base.Splitter;
+import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import java.io.InputStream;
 import java.util.List;
 import java.util.Map;
+import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
 import org.apache.commons.io.IOUtils;
 import org.sonar.api.server.ws.LocalConnector;
@@ -78,6 +81,16 @@ public class SimpleGetRequest extends Request {
   }
 
   @Override
+  @CheckForNull
+  public List<String> paramAsStrings(String key) {
+    String value = param(key);
+    if (value == null) {
+      return null;
+    }
+    return Lists.newArrayList(Splitter.on(',').omitEmptyStrings().trimResults().split(value));
+  }
+
+  @Override
   public InputStream paramAsInputStream(String key) {
     return IOUtils.toInputStream(param(key), UTF_8);
   }
diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/internal/ValidatingRequest.java b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/internal/ValidatingRequest.java
index b172724830a..440bf232ca2 100644
--- a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/internal/ValidatingRequest.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/internal/ValidatingRequest.java
@@ -21,14 +21,12 @@ package org.sonar.api.server.ws.internal;
 
 import com.google.common.base.CharMatcher;
 import com.google.common.base.Splitter;
-import com.google.common.collect.Lists;
 import java.io.InputStream;
-import java.util.ArrayList;
 import java.util.List;
 import java.util.Set;
+import java.util.stream.Collectors;
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
-import org.apache.commons.lang.StringUtils;
 import org.sonar.api.server.ws.LocalConnector;
 import org.sonar.api.server.ws.Request;
 import org.sonar.api.server.ws.WebService;
@@ -37,12 +35,14 @@ import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static java.util.Collections.emptyList;
 import static java.util.Collections.singletonList;
+import static org.apache.commons.lang.StringUtils.defaultString;
 
 /**
  * @since 4.2
  */
 public abstract class ValidatingRequest extends Request {
 
+  private static final Splitter COMMA_SPLITTER = Splitter.on(',').omitEmptyStrings().trimResults();
   private WebService.Action action;
   private LocalConnector localConnector;
 
@@ -74,10 +74,7 @@ public abstract class ValidatingRequest extends Request {
   public List<String> multiParam(String key) {
     WebService.Param definition = action.param(key);
     List<String> values = readMultiParamOrDefaultValue(key, definition);
-
-    values.forEach(value -> validate(value, definition));
-
-    return values;
+    return validateValues(values, definition);
   }
 
   @Override
@@ -96,9 +93,9 @@ public abstract class ValidatingRequest extends Request {
   private String param(String key, boolean validateValue) {
     WebService.Param definition = action.param(key);
     String value = readParamOrDefaultValue(key, definition);
-    String trimmedValue = value == null ? value : CharMatcher.WHITESPACE.trimFrom(value);
+    String trimmedValue = value == null ? null : CharMatcher.WHITESPACE.trimFrom(value);
     if (trimmedValue != null && validateValue) {
-      validate(trimmedValue, definition);
+      validateValue(trimmedValue, definition);
     }
     return trimmedValue;
   }
@@ -111,28 +108,20 @@ public abstract class ValidatingRequest extends Request {
     if (value == null) {
       return null;
     }
-    List<String> values = Lists.newArrayList(Splitter.on(',').omitEmptyStrings().trimResults().split(value));
-    for (String s : values) {
-      validate(s, definition);
-    }
-    return values;
+    List<String> values = COMMA_SPLITTER.splitToList(value);
+    return validateValues(values, definition);
   }
 
   @CheckForNull
   @Override
   public <E extends Enum<E>> List<E> paramAsEnums(String key, Class<E> enumClass) {
-    WebService.Param definition = action.param(key);
-    String value = readParamOrDefaultValue(key, definition);
-    if (value == null) {
+    List<String> values = paramAsStrings(key);
+    if (values == null) {
       return null;
     }
-    Iterable<String> values = Splitter.on(',').omitEmptyStrings().trimResults().split(value);
-    List<E> result = new ArrayList<>();
-    for (String s : values) {
-      validate(s, definition);
-      result.add(Enum.valueOf(enumClass, s));
-    }
-    return result;
+    return values.stream()
+      .map(value -> Enum.valueOf(enumClass, value))
+      .collect(Collectors.toList());
   }
 
   @CheckForNull
@@ -140,9 +129,8 @@ public abstract class ValidatingRequest extends Request {
     checkArgument(definition != null, "BUG - parameter '%s' is undefined for action '%s'", key, action.key());
 
     String deprecatedKey = definition.deprecatedKey();
-    String value = deprecatedKey != null ? StringUtils.defaultString(readParam(deprecatedKey), readParam(key)) : readParam(key);
-    value = StringUtils.defaultString(value, definition.defaultValue());
-    return value == null ? null : value;
+    String value = deprecatedKey != null ? defaultString(readParam(deprecatedKey), readParam(key)) : readParam(key);
+    return defaultString(value, definition.defaultValue());
   }
 
   private List<String> readMultiParamOrDefaultValue(String key, @Nullable WebService.Param definition) {
@@ -174,11 +162,15 @@ public abstract class ValidatingRequest extends Request {
   @CheckForNull
   protected abstract Part readPart(String key);
 
-  private static void validate(String value, WebService.Param definition) {
+  private static List<String> validateValues(List<String> values, WebService.Param definition) {
+    Integer maximumValues = definition.maxValuesAllowed();
+    checkArgument(maximumValues == null || values.size() <= maximumValues, "'%s' can contains only %s values, got %s", definition.key(), maximumValues, values.size());
+    values.forEach(value -> validateValue(value, definition));
+    return values;
+  }
+
+  private static void validateValue(String value, WebService.Param definition) {
     Set<String> possibleValues = definition.possibleValues();
-    if (possibleValues != null && !possibleValues.contains(value)) {
-      throw new IllegalArgumentException(String.format(
-        "Value of parameter '%s' (%s) must be one of: %s", definition.key(), value, possibleValues));
-    }
+    checkArgument(possibleValues == null || possibleValues.contains(value), "Value of parameter '%s' (%s) must be one of: %s", definition.key(), value, possibleValues);
   }
 }
diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/RequestTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/RequestTest.java
index 4c299ae37ae..b940b953d08 100644
--- a/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/RequestTest.java
+++ b/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/RequestTest.java
@@ -111,6 +111,24 @@ public class RequestTest {
   }
 
   @Test
+  public void multi_param() {
+    assertThat(underTest.multiParam("a_required_multi_param")).isEmpty();
+
+    underTest.setMultiParam("a_required_multi_param", newArrayList("firstValue", "secondValue", "thirdValue"));
+    assertThat(underTest.multiParam("a_required_multi_param")).containsExactly("firstValue", "secondValue", "thirdValue");
+  }
+
+  @Test
+  public void fail_when_multi_param_has_more_values_than_maximum_values() {
+    underTest.setMultiParam("has_maximum_values", newArrayList("firstValue", "secondValue", "thirdValue"));
+
+    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage("'has_maximum_values' can contains only 2 values, got 3");
+
+    underTest.multiParam("has_maximum_values");
+  }
+
+  @Test
   public void mandatory_multi_param() {
     underTest.setMultiParam("a_required_multi_param", newArrayList("firstValue", "secondValue", "thirdValue"));
 
@@ -215,8 +233,21 @@ public class RequestTest {
 
   @Test
   public void param_as_enums() {
-    assertThat(underTest.setParam("a_enum", "BETA,READY").paramAsEnums("a_enum", RuleStatus.class)).containsOnly(
-      RuleStatus.BETA, RuleStatus.READY);
+    assertThat(underTest.setParam("a_enum", "BETA,READY").paramAsEnums("a_enum", RuleStatus.class)).containsOnly(RuleStatus.BETA, RuleStatus.READY);
+    assertThat(underTest.setParam("a_enum", "").paramAsEnums("a_enum", RuleStatus.class)).isEmpty();
+  }
+
+  @Test
+  public void param_as_enums_returns_null_when_no_value() {
+    assertThat(underTest.paramAsEnums("a_enum", RuleStatus.class)).isNull();
+  }
+
+  @Test
+  public void fail_when_param_as_enums_has_more_values_than_maximum_values() {
+    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage("'has_maximum_values' can contains only 2 values, got 3");
+
+    underTest.setParam("has_maximum_values", "BETA,READY,REMOVED").paramAsEnums("has_maximum_values", RuleStatus.class);
   }
 
   @Test
@@ -346,7 +377,9 @@ public class RequestTest {
     IllegalArgumentException expected = new IllegalArgumentException("Faking validation of parameter value failed");
 
     try {
-      underTest.getParam("a_string", (str) -> {throw expected; });
+      underTest.getParam("a_string", (str) -> {
+        throw expected;
+      });
       fail("an IllegalStateException should have been raised");
     } catch (IllegalArgumentException e) {
       assertThat(e).isSameAs(expected);
@@ -407,7 +440,9 @@ public class RequestTest {
     RuntimeException expected = new RuntimeException("Faking BiConsumer throwing unchecked exception");
 
     try {
-      underTest.getParam("a_string", (rqt, key) -> { throw expected; });
+      underTest.getParam("a_string", (rqt, key) -> {
+        throw expected;
+      });
       fail("an RuntimeException should have been raised");
     } catch (RuntimeException e) {
       assertThat(e).isSameAs(expected);
@@ -473,6 +508,14 @@ public class RequestTest {
   }
 
   @Test
+  public void fail_when_param_as_strings_has_more_values_than_maximum_values() {
+    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage("'has_maximum_values' can contains only 2 values, got 3");
+
+    underTest.setParam("has_maximum_values", "foo,bar,baz").paramAsStrings("has_maximum_values");
+  }
+
+  @Test
   public void deprecated_key() {
     assertThat(underTest.setParam("deprecated_param", "bar").param("new_param")).isEqualTo("bar");
   }
@@ -636,6 +679,8 @@ public class RequestTest {
 
       action.createParam("has_possible_values").setPossibleValues("foo", "bar");
 
+      action.createParam("has_maximum_values").setMaxValuesAllowed(2);
+
       action.createParam("new_param").setDeprecatedKey("deprecated_param", "6.3");
       action.createParam("new_param_with_default_value").setDeprecatedKey("deprecated_new_param_with_default_value", "6.2").setDefaultValue("the_default_string");
 
diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/WebServiceTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/WebServiceTest.java
index 6942526c3d5..b73ce7ef5e4 100644
--- a/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/WebServiceTest.java
+++ b/sonar-plugin-api/src/test/java/org/sonar/api/server/ws/WebServiceTest.java
@@ -45,7 +45,7 @@ public class WebServiceTest {
   @Rule
   public LogTester logTester = new LogTester();
 
-  WebService.Context context = new WebService.Context();
+  private WebService.Context context = new WebService.Context();
 
   @Test
   public void no_web_services_by_default() {
@@ -195,7 +195,8 @@ public class WebServiceTest {
         .setSince("4.4")
         .setDeprecatedSince("5.3")
         .setDeprecatedKey("old-severity", "4.5")
-        .setPossibleValues("INFO", "MAJOR", "BLOCKER");
+        .setPossibleValues("INFO", "MAJOR", "BLOCKER")
+        .setMaxValuesAllowed(10);
       newAction.createParam("internal")
         .setInternal(true);
       newAction.addPagingParams(20);
@@ -223,6 +224,7 @@ public class WebServiceTest {
     assertThat(severityParam.deprecatedKeySince()).isEqualTo("4.5");
     assertThat(severityParam.defaultValue()).isEqualTo("MAJOR");
     assertThat(severityParam.possibleValues()).containsOnly("INFO", "MAJOR", "BLOCKER");
+    assertThat(severityParam.maxValuesAllowed()).isEqualTo(10);
 
     WebService.Param internalParam = action.param("internal");
     assertThat(internalParam.isInternal()).isTrue();