From d21ac75fe48328a14022bb5f06a6624f05a14920 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Mon, 25 Aug 2014 19:51:23 +0200 Subject: [PATCH] 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 / --- .../java/org/sonar/wsclient/SonarClient.java | 61 ++++++++++++++++-- .../wsclient/internal/HttpRequestFactory.java | 19 +++++- .../internal/DefaultProjectClient.java | 2 +- .../org/sonar/wsclient/SonarClientTest.java | 63 +++++++++++++++++++ .../internal/HttpRequestFactoryTest.java | 10 +++ 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. + *

+ * Example: + * 

  {@code
+   *   Map params = new HashMap<>();
+   *   params.put("name", "My Quality Gate");
+   *   client.post("api/qualitygates/create", params);
+   * }
* @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 paramsAsMap(Object[] params) { + if (params.length % 2 == 1) { + throw new IllegalArgumentException(String.format( + "Expecting even number of elements. Got %s", Arrays.toString(params))); + } + Map map = new HashMap(); + 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 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 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 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 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 @@ -122,6 +122,16 @@ public class HttpRequestFactoryTest { } } + @Test + public void beginning_slash_is_optional() throws Exception { + HttpRequestFactory factory = new HttpRequestFactory(httpServer.url()); + factory.get("api/foo", Collections.emptyMap()); + assertThat(httpServer.requestedPath()).isEqualTo("/api/foo"); + + factory.get("/api/bar", Collections.emptyMap()); + assertThat(httpServer.requestedPath()).isEqualTo("/api/bar"); + } + @Test public void should_encode_characters() { HttpRequestFactory requestFactory = new HttpRequestFactory(httpServer.url()); -- 2.39.5