SONAR-5264 Force documentation of web services to be up-to-date

3 files changed, 228 insertions, 95 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 eacbb069512..792fa6837da 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
@@ -22,16 +22,27 @@ package org.sonar.api.server.ws;
 import com.google.common.base.Splitter;
 import com.google.common.collect.Lists;
 import org.apache.commons.lang.StringUtils;
+import org.slf4j.LoggerFactory;
 
 import javax.annotation.CheckForNull;
+import javax.annotation.Nullable;
 import java.util.List;
+import java.util.Set;
 
 /**
  * @since 4.2
  */
 public abstract class Request {
 
-  public abstract WebService.Action action();
+  private WebService.Action action;
+
+  protected void setAction(WebService.Action action) {
+    this.action = action;
+  }
+
+  public WebService.Action action() {
+    return action;
+  }
 
   /**
    * Returns the name of the HTTP method with which this request was made. Possible
@@ -40,20 +51,20 @@ public abstract class Request {
   public abstract String method();
 
   /**
-   * Returns value of a mandatory parameter
+   * Returns a non-null value. To be used when parameter is required or has a default value.
    *
    * @throws java.lang.IllegalArgumentException is value is null or blank
    */
   public String mandatoryParam(String key) {
     String value = param(key);
-    if (StringUtils.isBlank(value)) {
+    if (value == null) {
       throw new IllegalArgumentException(String.format("Parameter '%s' is missing", key));
     }
     return value;
   }
 
   /**
-   * Returns value of a mandatory parameter
+   * Returns a boolean value. To be used when parameter is required or has a default value.
    *
    * @throws java.lang.IllegalArgumentException is value is null or blank
    */
@@ -63,7 +74,7 @@ public abstract class Request {
   }
 
   /**
-   * Returns value of a mandatory parameter
+   * Returns an int value. To be used when parameter is required or has a default value.
    *
    * @throws java.lang.IllegalArgumentException is value is null or blank
    */
@@ -73,7 +84,7 @@ public abstract class Request {
   }
 
   /**
-   * Returns value of a mandatory parameter
+   * Returns a long value. To be used when parameter is required or has a default value.
    *
    * @throws java.lang.IllegalArgumentException is value is null or blank
    */
@@ -82,53 +93,123 @@ public abstract class Request {
     return Long.parseLong(s);
   }
 
+  public List<String> mandatoryParamAsStrings(String key) {
+    List<String> values = paramAsStrings(key);
+    if (values == null) {
+      throw new IllegalArgumentException(String.format("Parameter '%s' is missing", key));
+    }
+    return values;
+  }
+
   @CheckForNull
-  public abstract String param(String key);
+  public String param(String key) {
+    return param(key, true);
+  }
 
   @CheckForNull
-  public String param(String key, @CheckForNull String defaultValue) {
-    return StringUtils.defaultString(param(key), defaultValue);
+  String param(String key, boolean validateValue) {
+    WebService.Param definition = action.param(key);
+    String value = readParamOrDefaultValue(key, definition);
+    if (value != null && validateValue) {
+      validate(value, definition);
+    }
+    return value;
   }
 
   @CheckForNull
   public List<String> paramAsStrings(String key) {
-    String s = param(key);
-    if (s == null) {
+    WebService.Param definition = action.param(key);
+    String value = readParamOrDefaultValue(key, definition);
+    if (value == null) {
       return null;
     }
-    return Lists.newArrayList(Splitter.on(',').omitEmptyStrings().trimResults().split(s));
+    List<String> values = Lists.newArrayList(Splitter.on(',').omitEmptyStrings().trimResults().split(value));
+    for (String s : values) {
+      validate(s, definition);
+    }
+    return values;
   }
 
   @CheckForNull
-  public Integer paramAsInt(String key) {
-    String s = param(key);
-    return s == null ? null : Integer.parseInt(s);
+  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());
+      LoggerFactory.getLogger(getClass()).error(message);
+      throw new IllegalArgumentException(message);
+    }
+    String value = StringUtils.defaultString(readParam(key), definition.defaultValue());
+    if (value == null) {
+      return null;
+    }
+    return value;
   }
 
-  public int paramAsInt(String key, int defaultValue) {
-    String s = param(key);
-    return s == null ? defaultValue : Integer.parseInt(s);
+  @CheckForNull
+  protected abstract String readParam(String key);
+
+  private void validate(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));
+    }
   }
 
+  /**
+   * @deprecated to be dropped in 4.4. Default values are declared in ws metadata
+   */
   @CheckForNull
-  public Long paramAsLong(String key) {
+  @Deprecated
+  public String param(String key, @CheckForNull String defaultValue) {
+    return StringUtils.defaultString(param(key), defaultValue);
+  }
+
+  /**
+   * @deprecated to be dropped in 4.4. Default values must be declared in {@link org.sonar.api.server.ws.WebService} then
+   * this method can be replaced by {@link #mandatoryParamAsBoolean(String)}.
+   */
+  @Deprecated
+  public boolean paramAsBoolean(String key, boolean defaultValue) {
     String s = param(key);
-    return s == null ? null : Long.parseLong(s);
+    return s == null ? defaultValue : Boolean.parseBoolean(s);
+  }
+
+  /**
+   * @deprecated to be dropped in 4.4. Default values must be declared in {@link org.sonar.api.server.ws.WebService} then
+   * this method can be replaced by {@link #mandatoryParamAsInt(String)}.
+   */
+  @Deprecated
+  public int paramAsInt(String key, int defaultValue) {
+    String s = param(key);
+    return s == null ? defaultValue : Integer.parseInt(s);
   }
 
+  /**
+   * @deprecated to be dropped in 4.4. Default values must be declared in {@link org.sonar.api.server.ws.WebService} then
+   * this method can be replaced by {@link #mandatoryParamAsLong(String)}.
+   */
+  @Deprecated
   public long paramAsLong(String key, long defaultValue) {
     String s = param(key);
     return s == null ? defaultValue : Long.parseLong(s);
   }
 
+
   @CheckForNull
   public Boolean paramAsBoolean(String key) {
     String s = param(key);
     return s == null ? null : Boolean.parseBoolean(s);
   }
 
-  public boolean paramAsBoolean(String key, boolean defaultValue) {
+  @CheckForNull
+  public Integer paramAsInt(String key) {
     String s = param(key);
-    return s == null ? defaultValue : Boolean.parseBoolean(s);
+    return s == null ? null : Integer.parseInt(s);
+  }
+
+  @CheckForNull
+  public Long paramAsLong(String key) {
+    String s = param(key);
+    return s == null ? null : Long.parseLong(s);
   }
 }
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 cc2803e1ba3..a5ac15ea8d1 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
@@ -23,6 +23,7 @@ import com.google.common.base.Charsets;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Maps;
+import com.google.common.collect.Sets;
 import org.apache.commons.io.FilenameUtils;
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang.StringUtils;
@@ -37,6 +38,7 @@ import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 /**
  * Defines a web service. Note that contrary to the deprecated {@link org.sonar.api.web.Webservice}
@@ -434,7 +436,7 @@ public interface WebService extends ServerExtension {
   class NewParam {
     private String key, description, exampleValue, defaultValue;
     private boolean required = false;
-    private Collection<Object> possibleValues = null;
+    private Set<String> possibleValues = null;
 
     private NewParam(String key) {
       this.key = key;
@@ -469,9 +471,8 @@ public interface WebService extends ServerExtension {
      *
      * @since 4.4
      */
-    public NewParam setPossibleValues(@Nullable Object... s) {
-      this.possibleValues = (s == null ? null : Arrays.asList(s));
-      return this;
+    public NewParam setPossibleValues(@Nullable Object... values) {
+      return setPossibleValues(values == null ? (Collection) null : Arrays.asList(values));
     }
 
     /**
@@ -480,8 +481,15 @@ public interface WebService extends ServerExtension {
      *
      * @since 4.4
      */
-    public NewParam setPossibleValues(@Nullable Collection c) {
-      this.possibleValues = c;
+    public NewParam setPossibleValues(@Nullable Collection values) {
+      if (values == null) {
+        this.possibleValues = null;
+      } else {
+        this.possibleValues = Sets.newLinkedHashSet();
+        for (Object value : values) {
+          this.possibleValues.add(value.toString());
+        }
+      }
       return this;
     }
 
@@ -503,7 +511,7 @@ public interface WebService extends ServerExtension {
   class Param {
     private final String key, description, exampleValue, defaultValue;
     private final boolean required;
-    private final List<String> possibleValues;
+    private final Set<String> possibleValues;
 
     public Param(NewParam newParam) {
       this.key = newParam.key;
@@ -511,15 +519,7 @@ public interface WebService extends ServerExtension {
       this.exampleValue = newParam.exampleValue;
       this.defaultValue = newParam.defaultValue;
       this.required = newParam.required;
-      if (newParam.possibleValues == null) {
-        this.possibleValues = null;
-      } else {
-        ImmutableList.Builder<String> builder = ImmutableList.builder();
-        for (Object possibleValue : newParam.possibleValues) {
-          builder.add(possibleValue.toString());
-        }
-        this.possibleValues = builder.build();
-      }
+      this.possibleValues = newParam.possibleValues;
     }
 
     public String key() {
@@ -552,7 +552,7 @@ public interface WebService extends ServerExtension {
      * @since 4.4
      */
     @CheckForNull
-    public List<String> possibleValues() {
+    public Set<String> possibleValues() {
       return 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 ef28b62c2c8..ac46d611fc9 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
@@ -19,10 +19,11 @@
  */
 package org.sonar.api.server.ws;
 
+import com.google.common.collect.Maps;
+import org.junit.Before;
 import org.junit.Test;
 
-import javax.annotation.CheckForNull;
-import java.util.HashMap;
+import javax.annotation.Nullable;
 import java.util.Map;
 
 import static org.fest.assertions.Assertions.assertThat;
@@ -31,26 +32,16 @@ import static org.mockito.Mockito.mock;
 
 public class RequestTest {
 
-  static class SimpleRequest extends Request {
+  private static class SimpleRequest extends Request {
 
-    private final Map<String, String> params = new HashMap<String, String>();
-    private final WebService.Action action;
-
-    private SimpleRequest(WebService.Action action) {
-      this.action = action;
-    }
-
-    @Override
-    public WebService.Action action() {
-      return action;
-    }
+    private final Map<String, String> params = Maps.newHashMap();
 
     @Override
     public String method() {
       return "GET";
     }
 
-    public SimpleRequest setParam(String key, @CheckForNull String value) {
+    public SimpleRequest setParam(String key, @Nullable String value) {
       if (value != null) {
         params.put(key, value);
       }
@@ -58,84 +49,145 @@ public class RequestTest {
     }
 
     @Override
-    @CheckForNull
-    public String param(String key) {
+    protected String readParam(String key) {
       return params.get(key);
     }
   }
 
+  private static class SimpleWebService implements WebService {
+
+    @Override
+    public void define(Context context) {
+      NewController controller = context.createController("my_controller");
+      NewAction action = controller.createAction("my_action");
+      action.setHandler(mock(RequestHandler.class));
+      action
+        .createParam("required_param")
+        .setRequired(true);
+
+      action.createParam("a_string");
+      action.createParam("a_boolean");
+      action.createParam("a_number");
+
+      action.createParam("a_required_string").setRequired(true);
+      action.createParam("a_required_boolean").setRequired(true);
+      action.createParam("a_required_number").setRequired(true);
+
+      action.createParam("has_default_string").setDefaultValue("the_default_string");
+      action.createParam("has_default_number").setDefaultValue("10");
+      action.createParam("has_default_boolean").setDefaultValue("true");
+
+      action.createParam("has_possible_values").setPossibleValues("foo", "bar");
+
+      controller.done();
+    }
+  }
+
+  SimpleRequest request = new SimpleRequest();
 
-  SimpleRequest request = new SimpleRequest(mock(WebService.Action.class));
+  @Before
+  public void before() throws Exception {
+    WebService.Context context = new WebService.Context();
+    new SimpleWebService().define(context);
+    request.setAction(context.controller("my_controller").action("my_action"));
+  }
 
   @Test
-  public void mandatory_param_is_missing() throws Exception {
+  public void required_param_is_missing() throws Exception {
     try {
-      request.mandatoryParam("foo");
+      request.mandatoryParam("required_param");
       fail();
     } catch (IllegalArgumentException e) {
-      assertThat(e).hasMessage("Parameter 'foo' is missing");
+      assertThat(e).hasMessage("Parameter 'required_param' is missing");
     }
   }
 
   @Test
-  public void mandatory_param_is_set() throws Exception {
-    request.setParam("a_string", "foo");
-    request.setParam("a_long", "42");
-    request.setParam("a_int", "42");
-    request.setParam("a_boolean", "true");
-
-    assertThat(request.mandatoryParam("a_string")).isEqualTo("foo");
-    assertThat(request.mandatoryParamAsBoolean("a_boolean")).isTrue();
-    assertThat(request.mandatoryParamAsInt("a_int")).isEqualTo(42);
-    assertThat(request.mandatoryParamAsLong("a_long")).isEqualTo(42L);
+  public void required_param() throws Exception {
+    request.setParam("a_required_string", "foo");
+    request.setParam("a_required_number", "42");
+    request.setParam("a_required_boolean", "true");
+
+    assertThat(request.mandatoryParam("a_required_string")).isEqualTo("foo");
+    assertThat(request.mandatoryParamAsBoolean("a_required_boolean")).isTrue();
+    assertThat(request.mandatoryParamAsInt("a_required_number")).isEqualTo(42);
+    assertThat(request.mandatoryParamAsLong("a_required_number")).isEqualTo(42L);
+  }
+
+  @Test
+  public void required_param_as_strings() throws Exception {
+    try {
+      request.mandatoryParamAsStrings("a_required_string");
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessage("Parameter 'a_required_string' is missing");
+    }
+
+    request.setParam("a_required_string", "foo,bar");
+    assertThat(request.mandatoryParamAsStrings("a_required_string")).containsExactly("foo", "bar");
   }
 
   @Test
   public void default_value_of_optional_param() throws Exception {
-    String value = request.param("foo", "bar");
-    assertThat(value).isEqualTo("bar");
+    assertThat(request.param("has_default_string")).isEqualTo("the_default_string");
   }
 
   @Test
   public void param_as_string() throws Exception {
-    String value = request.setParam("foo", "bar").param("foo", "default");
-    assertThat(value).isEqualTo("bar");
+    assertThat(request.setParam("a_string", "foo").param("a_string")).isEqualTo("foo");
+  }
+
+  @Test
+  public void null_param() throws Exception {
+    assertThat(request.param("a_string")).isNull();
+    assertThat(request.paramAsBoolean("a_boolean")).isNull();
+    assertThat(request.paramAsInt("a_number")).isNull();
+    assertThat(request.paramAsLong("a_number")).isNull();
   }
 
   @Test
   public void param_as_int() throws Exception {
-    assertThat(request.setParam("foo", "123").paramAsInt("foo")).isEqualTo(123);
-    assertThat(request.setParam("foo", "123").paramAsInt("xxx")).isNull();
-    assertThat(request.setParam("foo", "123").paramAsInt("foo", 456)).isEqualTo(123);
-    assertThat(request.setParam("foo", "123").paramAsInt("xxx", 456)).isEqualTo(456);
+    assertThat(request.setParam("a_number", "123").paramAsInt("a_number")).isEqualTo(123);
   }
 
   @Test
   public void param_as_long() throws Exception {
-    assertThat(request.setParam("foo", "123").paramAsLong("foo")).isEqualTo(123L);
-    assertThat(request.setParam("foo", "123").paramAsLong("xxx")).isNull();
-    assertThat(request.setParam("foo", "123").paramAsLong("foo", 456L)).isEqualTo(123L);
-    assertThat(request.setParam("foo", "123").paramAsLong("xxx", 456L)).isEqualTo(456L);
+    assertThat(request.setParam("a_number", "123").paramAsLong("a_number")).isEqualTo(123L);
   }
 
   @Test
   public void param_as_boolean() throws Exception {
-    assertThat(request.setParam("foo", "true").paramAsBoolean("foo")).isTrue();
-    assertThat(request.setParam("foo", "false").paramAsBoolean("foo")).isFalse();
-    assertThat(request.setParam("foo", "true").paramAsBoolean("xxx")).isNull();
-
-    assertThat(request.setParam("foo", "true").paramAsBoolean("foo", true)).isTrue();
-    assertThat(request.setParam("foo", "true").paramAsBoolean("foo", false)).isTrue();
-    assertThat(request.setParam("foo", "true").paramAsBoolean("xxx", true)).isTrue();
-    assertThat(request.setParam("foo", "true").paramAsBoolean("xxx", false)).isFalse();
+    assertThat(request.setParam("a_boolean", "true").paramAsBoolean("a_boolean")).isTrue();
+    assertThat(request.setParam("a_boolean", "false").paramAsBoolean("a_boolean")).isFalse();
   }
 
   @Test
   public void param_as_strings() throws Exception {
-    assertThat(request.paramAsStrings("foo")).isNull();
-    assertThat(request.setParam("foo", "").paramAsStrings("foo")).isEmpty();
-    assertThat(request.setParam("foo", "bar").paramAsStrings("foo")).containsExactly("bar");
-    assertThat(request.setParam("foo", "bar,baz").paramAsStrings("foo")).containsExactly("bar", "baz");
-    assertThat(request.setParam("foo", "bar , baz").paramAsStrings("foo")).containsExactly("bar", "baz");
+    assertThat(request.paramAsStrings("a_string")).isNull();
+    assertThat(request.setParam("a_string", "").paramAsStrings("a_string")).isEmpty();
+    assertThat(request.setParam("a_string", "bar").paramAsStrings("a_string")).containsExactly("bar");
+    assertThat(request.setParam("a_string", "bar,baz").paramAsStrings("a_string")).containsExactly("bar", "baz");
+    assertThat(request.setParam("a_string", "bar , baz").paramAsStrings("a_string")).containsExactly("bar", "baz");
+  }
+
+  @Test
+  public void fail_if_param_is_not_defined() throws Exception {
+    try {
+      request.param("unknown");
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessage("BUG - parameter 'unknown' is undefined for action 'my_action'");
+    }
+  }
+
+  @Test
+  public void verify_possible_values() throws Exception {
+    request.setParam("has_possible_values", "foo");
+    assertThat(request.param("has_possible_values")).isEqualTo("foo");
+
+    try {
+      request.setParam("has_possible_values", "not_possible");
+      request.param("has_possible_values");
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessage("Value of parameter 'has_possible_values' (not_possible) must be one of: [foo, bar]");
+    }
   }
 }