SONAR-5383 improve SonarClient#get() and #post()

* add helpful get(String relativeUrl, Object... params) and post(String relativeUrl, Object... params)
* support relativeUrl that does not start with slash /

5 files changed, 146 insertions, 9 deletions

diff --git a/server/sonar-ws-client/src/main/java/org/sonar/wsclient/SonarClient.java b/server/sonar-ws-client/src/main/java/org/sonar/wsclient/SonarClient.java
index 06ae374c2d7..bd97afb9ad4 100644
--- a/server/sonar-ws-client/src/main/java/org/sonar/wsclient/SonarClient.java
+++ b/server/sonar-ws-client/src/main/java/org/sonar/wsclient/SonarClient.java
@@ -39,6 +39,8 @@ import org.sonar.wsclient.user.internal.DefaultUserClient;
 
 import javax.annotation.Nullable;
 
+import java.util.Arrays;
+import java.util.HashMap;
 import java.util.Map;
 
 /**
@@ -63,7 +65,7 @@ public class SonarClient {
   final HttpRequestFactory requestFactory;
 
   private SonarClient(Builder builder) {
-    requestFactory = new HttpRequestFactory(builder.url)
+    this(new HttpRequestFactory(builder.url)
       .setLogin(builder.login)
       .setPassword(builder.password)
       .setProxyHost(builder.proxyHost)
@@ -71,7 +73,14 @@ public class SonarClient {
       .setProxyLogin(builder.proxyLogin)
       .setProxyPassword(builder.proxyPassword)
       .setConnectTimeoutInMilliseconds(builder.connectTimeoutMs)
-      .setReadTimeoutInMilliseconds(builder.readTimeoutMs);
+      .setReadTimeoutInMilliseconds(builder.readTimeoutMs));
+  }
+
+  /**
+   * Visible for testing
+   */
+  SonarClient(HttpRequestFactory requestFactory) {
+    this.requestFactory = requestFactory;
   }
 
   /**
@@ -117,7 +126,7 @@ public class SonarClient {
   }
 
   /**
-   * New client to interact with web services related to quality profilesgates
+   * New client to interact with web services related to quality profiles
    */
   public QProfileClient qProfileClient() {
     return new DefaultQProfileClient(requestFactory);
@@ -143,7 +152,15 @@ public class SonarClient {
   }
 
   /**
-   * Send a POST request on the given relativeUrl, with provided parameters (can be empty)
+   * Send a POST request on the given relativeUrl, with provided parameters (can be empty).
+   * The beginning slash (/) of relativeUrl is supported but not mandatory.
+   * <p/>
+   * Example:
+   * <pre>  {@code
+   *   Map<String,Object> params = new HashMap<>();
+   *   params.put("name", "My Quality Gate");
+   *   client.post("api/qualitygates/create", params);
+   * }</pre>
    * @since 4.5
    * @return the response body
    */
@@ -152,7 +169,16 @@ public class SonarClient {
   }
 
   /**
-   * Send a GET request on the given relativeUrl, with provided parameters (can be empty)
+   * Same as {@link #post(String, java.util.Map)} but parameters are defined as an array
+   * of even number of elements (key1, value1, key, value2, ...). Keys must not be null.
+   */
+  public String post(String relativeUrl, Object... params) {
+    return post(relativeUrl, paramsAsMap(params));
+  }
+
+  /**
+   * Send a GET request on the given relativeUrl, with provided parameters (can be empty).
+   * The beginning slash (/) of relativeUrl is supported but not mandatory.
    * @since 4.5
    * @return the response body
    */
@@ -160,6 +186,31 @@ public class SonarClient {
     return requestFactory.get(relativeUrl, params);
   }
 
+  /**
+   * Same as {@link #get(String, java.util.Map)} but parameters are defined as an array
+   * of even number of elements (key1, value1, key, value2, ...). Keys must not be null.
+   */
+  public String get(String relativeUrl, Object... params) {
+    return get(relativeUrl, paramsAsMap(params));
+  }
+
+  private Map<String, Object> paramsAsMap(Object[] params) {
+    if (params.length % 2 == 1) {
+      throw new IllegalArgumentException(String.format(
+        "Expecting even number of elements. Got %s", Arrays.toString(params)));
+    }
+    Map<String, Object> map = new HashMap<String, Object>();
+    for (int index = 0; index < params.length; index++) {
+      if (params[index] == null) {
+        throw new IllegalArgumentException(String.format(
+          "Parameter key can't be null at index %d of %s", index, Arrays.toString(params)));
+      }
+      map.put(params[index].toString(), params[index + 1]);
+      index++;
+    }
+    return map;
+  }
+
   public static class Builder {
     private String login, password, url, proxyHost, proxyLogin, proxyPassword;
     private int proxyPort = 0;
diff --git a/server/sonar-ws-client/src/main/java/org/sonar/wsclient/internal/HttpRequestFactory.java b/server/sonar-ws-client/src/main/java/org/sonar/wsclient/internal/HttpRequestFactory.java
index a6f43be762e..d88cf76c32f 100644
--- a/server/sonar-ws-client/src/main/java/org/sonar/wsclient/internal/HttpRequestFactory.java
+++ b/server/sonar-ws-client/src/main/java/org/sonar/wsclient/internal/HttpRequestFactory.java
@@ -23,10 +23,13 @@ import com.github.kevinsawicki.http.HttpRequest;
 import org.sonar.wsclient.base.HttpException;
 
 import javax.annotation.Nullable;
+
 import java.util.Arrays;
 import java.util.Map;
 
-import static java.net.HttpURLConnection.*;
+import static java.net.HttpURLConnection.HTTP_CREATED;
+import static java.net.HttpURLConnection.HTTP_NO_CONTENT;
+import static java.net.HttpURLConnection.HTTP_OK;
 
 /**
  * Not an API. Please do not use this class, except maybe for unit tests.
@@ -122,15 +125,25 @@ public class HttpRequestFactory {
   }
 
   public String get(String wsUrl, Map<String, Object> queryParams) {
-    HttpRequest request = prepare(HttpRequest.get(baseUrl + wsUrl, queryParams, true));
+    HttpRequest request = prepare(HttpRequest.get(buildUrl(wsUrl), queryParams, true));
     return execute(request);
   }
 
   public String post(String wsUrl, Map<String, Object> queryParams) {
-    HttpRequest request = prepare(HttpRequest.post(baseUrl + wsUrl, true)).form(queryParams, HttpRequest.CHARSET_UTF8);
+    HttpRequest request = prepare(HttpRequest.post(buildUrl(wsUrl), true)).form(queryParams, HttpRequest.CHARSET_UTF8);
     return execute(request);
   }
 
+  private String buildUrl(String part) {
+    StringBuilder url = new StringBuilder();
+    url.append(baseUrl);
+    if (!part.startsWith("/")) {
+      url.append('/');
+    }
+    url.append(part);
+    return url.toString();
+  }
+
   private String execute(HttpRequest request) {
     try {
       if (isSuccess(request)) {
diff --git a/server/sonar-ws-client/src/main/java/org/sonar/wsclient/project/internal/DefaultProjectClient.java b/server/sonar-ws-client/src/main/java/org/sonar/wsclient/project/internal/DefaultProjectClient.java
index 7419b687f17..23cc942a8cf 100644
--- a/server/sonar-ws-client/src/main/java/org/sonar/wsclient/project/internal/DefaultProjectClient.java
+++ b/server/sonar-ws-client/src/main/java/org/sonar/wsclient/project/internal/DefaultProjectClient.java
@@ -50,6 +50,6 @@ public class DefaultProjectClient implements ProjectClient {
   @SuppressWarnings({"rawtypes", "unchecked"})
   private Project jsonToProject(String json) {
     Map jsonRoot = (Map) JSONValue.parse(json);
-    return new DefaultProject((Map) jsonRoot);
+    return new DefaultProject(jsonRoot);
   }
 }
diff --git a/server/sonar-ws-client/src/test/java/org/sonar/wsclient/SonarClientTest.java b/server/sonar-ws-client/src/test/java/org/sonar/wsclient/SonarClientTest.java
index e6e82045b96..fc23e4d7c9e 100644
--- a/server/sonar-ws-client/src/test/java/org/sonar/wsclient/SonarClientTest.java
+++ b/server/sonar-ws-client/src/test/java/org/sonar/wsclient/SonarClientTest.java
@@ -19,7 +19,10 @@
  */
 package org.sonar.wsclient;
 
+import org.fest.assertions.MapAssert;
 import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+import org.sonar.wsclient.internal.HttpRequestFactory;
 import org.sonar.wsclient.issue.internal.DefaultActionPlanClient;
 import org.sonar.wsclient.issue.internal.DefaultIssueClient;
 import org.sonar.wsclient.permissions.internal.DefaultPermissionClient;
@@ -29,8 +32,13 @@ import org.sonar.wsclient.qualitygate.internal.DefaultQualityGateClient;
 import org.sonar.wsclient.system.internal.DefaultSystemClient;
 import org.sonar.wsclient.user.internal.DefaultUserClient;
 
+import java.util.Map;
+
 import static org.fest.assertions.Assertions.assertThat;
 import static org.fest.assertions.Fail.fail;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
 
 public class SonarClientTest {
   @Test
@@ -102,4 +110,59 @@ public class SonarClientTest {
     assertThat(client.requestFactory.getProxyLogin()).isEqualTo("proxyLogin");
     assertThat(client.requestFactory.getProxyPassword()).isEqualTo("proxyPass");
   }
+
+  @Test
+  public void get() throws Exception {
+    HttpRequestFactory requestFactory = mock(HttpRequestFactory.class);
+    SonarClient client = new SonarClient(requestFactory);
+
+    client.get("api/foo", "key", "the_key", "max", 10);
+
+    ArgumentCaptor<Map> paramsCapture = ArgumentCaptor.forClass(Map.class);
+    verify(requestFactory).get(eq("api/foo"), paramsCapture.capture());
+    Map params = paramsCapture.getValue();
+    assertThat(params).hasSize(2);
+    assertThat(params).includes(MapAssert.entry("key", "the_key"));
+    assertThat(params).includes(MapAssert.entry("max", 10));
+  }
+
+  @Test
+  public void post() throws Exception {
+    HttpRequestFactory requestFactory = mock(HttpRequestFactory.class);
+    SonarClient client = new SonarClient(requestFactory);
+
+    client.post("api/foo", "max", 10);
+
+    ArgumentCaptor<Map> paramsCapture = ArgumentCaptor.forClass(Map.class);
+    verify(requestFactory).post(eq("api/foo"), paramsCapture.capture());
+    Map params = paramsCapture.getValue();
+    assertThat(params).hasSize(1);
+    assertThat(params).includes(MapAssert.entry("max", 10));
+  }
+
+  @Test
+  public void fail_if_odd_number_arguments() throws Exception {
+    HttpRequestFactory requestFactory = mock(HttpRequestFactory.class);
+    SonarClient client = new SonarClient(requestFactory);
+
+    try {
+      client.post("api/foo", "max", 10, "min");
+      fail();
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessage("Expecting even number of elements. Got [max, 10, min]");
+    }
+  }
+
+  @Test
+  public void fail_if_null_property_key() throws Exception {
+    HttpRequestFactory requestFactory = mock(HttpRequestFactory.class);
+    SonarClient client = new SonarClient(requestFactory);
+
+    try {
+      client.post("api/foo", "max", 10, null, 5);
+      fail();
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessage("Parameter key can't be null at index 2 of [max, 10, null, 5]");
+    }
+  }
 }
diff --git a/server/sonar-ws-client/src/test/java/org/sonar/wsclient/internal/HttpRequestFactoryTest.java b/server/sonar-ws-client/src/test/java/org/sonar/wsclient/internal/HttpRequestFactoryTest.java
index 82d5e63f601..a4ff84c215a 100644
--- a/server/sonar-ws-client/src/test/java/org/sonar/wsclient/internal/HttpRequestFactoryTest.java
+++ b/server/sonar-ws-client/src/test/java/org/sonar/wsclient/internal/HttpRequestFactoryTest.java
@@ -123,6 +123,16 @@ public class HttpRequestFactoryTest {
   }
 
   @Test
+  public void beginning_slash_is_optional() throws Exception {
+    HttpRequestFactory factory = new HttpRequestFactory(httpServer.url());
+    factory.get("api/foo", Collections.<String, Object>emptyMap());
+    assertThat(httpServer.requestedPath()).isEqualTo("/api/foo");
+
+    factory.get("/api/bar", Collections.<String, Object>emptyMap());
+    assertThat(httpServer.requestedPath()).isEqualTo("/api/bar");
+  }
+
+  @Test
   public void should_encode_characters() {
     HttpRequestFactory requestFactory = new HttpRequestFactory(httpServer.url());
     httpServer.stubResponseBody("{\"issues\": [{\"key\": \"ABCDE\"}]}");