Request does not log errors. It's WebServiceEngine responsability

2 files changed, 13 insertions, 15 deletions

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 389c6e62d3a..ea6bb59ee80 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
@@ -32,8 +32,8 @@ 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;
-import org.sonar.api.utils.log.Loggers;
 
+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;
@@ -137,26 +137,16 @@ public abstract class ValidatingRequest extends Request {
 
   @CheckForNull
   private String readParamOrDefaultValue(String key, @Nullable WebService.Param definition) {
-    if (definition == null) {
-      String message = String.format("BUG - parameter '%s' is undefined for action '%s'", key, action.key());
-      Loggers.get(getClass()).error(message);
-      throw new IllegalArgumentException(message);
-    }
+    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());
-    if (value == null) {
-      return null;
-    }
-    return value;
+    return value == null ? null : value;
   }
 
   private List<String> readMultiParamOrDefaultValue(String key, @Nullable WebService.Param definition) {
-    if (definition == null) {
-      String message = String.format("BUG - parameter '%s' is undefined for action '%s'", key, action.key());
-      Loggers.get(getClass()).error(message);
-      throw new IllegalArgumentException(message);
-    }
+    checkArgument(definition != null, "BUG - parameter '%s' is undefined for action '%s'", key, action.key());
 
     List<String> keyValues = readMultiParam(key);
     if (!keyValues.isEmpty()) {
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 d5580b1681a..fa3605d0935 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
@@ -259,6 +259,14 @@ public class RequestTest {
   }
 
   @Test
+  public void fail_if_multi_param_is_not_defined() {
+    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage("BUG - parameter 'unknown' is undefined for action 'my_action'");
+
+    underTest.multiParam("unknown");
+  }
+
+  @Test
   public void verify_possible_values() {
     underTest.setParam("has_possible_values", "foo");
     assertThat(underTest.param("has_possible_values")).isEqualTo("foo");