Fix quality flaws

4 files changed, 44 insertions, 95 deletions

diff --git a/sonar-ws/src/main/java/org/sonarqube/ws/client/HttpRequestFactory.java b/sonar-ws/src/main/java/org/sonarqube/ws/client/HttpRequestFactory.java
index 4ae2ce1f912..3b10c9d0a48 100644
--- a/sonar-ws/src/main/java/org/sonarqube/ws/client/HttpRequestFactory.java
+++ b/sonar-ws/src/main/java/org/sonarqube/ws/client/HttpRequestFactory.java
@@ -26,7 +26,6 @@ import com.google.protobuf.InvalidProtocolBufferException;
 import com.google.protobuf.Message;
 import com.google.protobuf.Parser;
 import java.util.Arrays;
-import java.util.Map;
 import javax.annotation.Nullable;
 
 import static java.net.HttpURLConnection.HTTP_CREATED;
@@ -41,7 +40,11 @@ public class HttpRequestFactory {
   private static final int[] RESPONSE_SUCCESS = {HTTP_OK, HTTP_CREATED, HTTP_NO_CONTENT};
 
   private final String baseUrl;
-  private String login, password, proxyHost, proxyLogin, proxyPassword;
+  private String login;
+  private String password;
+  private String proxyHost;
+  private String proxyLogin;
+  private String proxyPassword;
   private int proxyPort;
   private int connectTimeoutInMilliseconds;
   private int readTimeoutInMilliseconds;
@@ -126,16 +129,6 @@ public class HttpRequestFactory {
     return readTimeoutInMilliseconds;
   }
 
-  public String get(String wsUrl, Map<String, Object> queryParams) {
-    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(buildUrl(wsUrl), true)).form(queryParams, HttpRequest.CHARSET_UTF8);
-    return execute(request);
-  }
-
   public String execute(WsRequest wsRequest) {
     HttpRequest httpRequest = wsRequestToHttpRequest(wsRequest);
     return execute(httpRequest);
@@ -148,7 +141,7 @@ public class HttpRequestFactory {
     } catch (InvalidProtocolBufferException e) {
       Throwables.propagate(e);
     }
-    
+
     throw new IllegalStateException("Uncatched exception when parsing protobuf response");
   }
 
@@ -161,9 +154,13 @@ public class HttpRequestFactory {
       case PROTOBUF:
         httpRequest.accept(MediaType.PROTOBUF.toString());
         break;
-      default:
+      case JSON:
         httpRequest.accept(MediaType.JSON_UTF_8.toString());
         break;
+      case TEXT:
+      default:
+        httpRequest.accept(MediaType.PLAIN_TEXT_UTF_8.toString());
+        break;
     }
 
     return httpRequest;
@@ -179,21 +176,20 @@ public class HttpRequestFactory {
     return url.toString();
   }
 
-  private String execute(HttpRequest request) {
+  private static String execute(HttpRequest request) {
     try {
-      if (isSuccess(request)) {
-        return request.body(HttpRequest.CHARSET_UTF8);
-      }
-      // TODO better handle error messages
-      throw new HttpException(request.url().toString(), request.code(), request.body());
-
+      checkSuccess(request);
+      return request.body(HttpRequest.CHARSET_UTF8);
     } catch (HttpRequest.HttpRequestException e) {
-      throw new IllegalStateException("Fail to request " + request.url(), e.getCause());
+      throw new IllegalStateException("Fail to request " + request.url(), e);
     }
   }
 
-  private boolean isSuccess(HttpRequest request) {
-    return Arrays.binarySearch(RESPONSE_SUCCESS, request.code()) >= 0;
+  private static void checkSuccess(HttpRequest request) {
+    boolean isSuccess = Arrays.binarySearch(RESPONSE_SUCCESS, request.code()) >= 0;
+    if (!isSuccess) {
+      throw new HttpException(request.url().toString(), request.code(), request.body());
+    }
   }
 
   private HttpRequest prepare(HttpRequest request) {
diff --git a/sonar-ws/src/main/java/org/sonarqube/ws/client/WsClient.java b/sonar-ws/src/main/java/org/sonarqube/ws/client/WsClient.java
index e6e6c827bd6..641ac51d8a1 100644
--- a/sonar-ws/src/main/java/org/sonarqube/ws/client/WsClient.java
+++ b/sonar-ws/src/main/java/org/sonarqube/ws/client/WsClient.java
@@ -87,7 +87,12 @@ public class WsClient {
   }
 
   public static class Builder {
-    private String login, password, url, proxyHost, proxyLogin, proxyPassword;
+    private String login;
+    private String password;
+    private String url;
+    private String proxyHost;
+    private String proxyLogin;
+    private String proxyPassword;
     private int proxyPort = 0;
     private int connectTimeoutMs = DEFAULT_CONNECT_TIMEOUT_MILLISECONDS, readTimeoutMs = DEFAULT_READ_TIMEOUT_MILLISECONDS;
 
diff --git a/sonar-ws/src/test/java/org/sonarqube/ws/client/HttpRequestFactoryTest.java b/sonar-ws/src/test/java/org/sonarqube/ws/client/HttpRequestFactoryTest.java
index f14ad893b54..5007e4d827a 100644
--- a/sonar-ws/src/test/java/org/sonarqube/ws/client/HttpRequestFactoryTest.java
+++ b/sonar-ws/src/test/java/org/sonarqube/ws/client/HttpRequestFactoryTest.java
@@ -19,27 +19,24 @@
  */
 package org.sonarqube.ws.client;
 
-import java.net.ConnectException;
-import java.text.ParseException;
-import java.text.SimpleDateFormat;
-import java.util.Collections;
-import java.util.Date;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.junit.Assert.fail;
 
 public class HttpRequestFactoryTest {
   @Rule
   public MockHttpServerInterceptor httpServer = new MockHttpServerInterceptor();
+  @Rule
+  public ExpectedException expectedException = ExpectedException.none();
 
   @Test
   public void test_get() {
     httpServer.stubStatusCode(200).stubResponseBody("{'issues': []}");
 
     HttpRequestFactory factory = new HttpRequestFactory(httpServer.url());
-    String json = factory.get("/api/issues", Collections.<String, Object>emptyMap());
+    String json = factory.execute(new WsRequest("/api/issues"));
 
     assertThat(json).isEqualTo("{'issues': []}");
     assertThat(httpServer.requestedPath()).isEqualTo("/api/issues");
@@ -47,48 +44,11 @@ public class HttpRequestFactoryTest {
 
   @Test
   public void should_throw_illegal_state_exc_if_connect_exception() {
-    HttpRequestFactory factory = new HttpRequestFactory("http://localhost:1");
-    try {
-      factory.get("/api/issues", Collections.<String, Object>emptyMap());
-      fail();
-    } catch (Exception e) {
-      assertThat(e).isInstanceOf(IllegalStateException.class);
-      assertThat(e).hasMessage("Fail to request http://localhost:1/api/issues");
-      assertThat(e.getCause().getMessage()).matches(".*(Connection refused|Connexion refusée).*");
-    }
-  }
-
-  @Test
-  public void test_post() {
-    httpServer.stubStatusCode(200).stubResponseBody("{}");
-
-    HttpRequestFactory factory = new HttpRequestFactory(httpServer.url());
-    String json = factory.post("/api/issues/change", Collections.<String, Object>emptyMap());
-
-    assertThat(json).isEqualTo("{}");
-    assertThat(httpServer.requestedPath()).isEqualTo("/api/issues/change");
-  }
-
-  @Test
-  public void post_successful_if_created() {
-    httpServer.stubStatusCode(201).stubResponseBody("{}");
+    expectedException.expect(IllegalStateException.class);
+    expectedException.expectMessage("Fail to request http://localhost:1/api/issues");
 
-    HttpRequestFactory factory = new HttpRequestFactory(httpServer.url());
-    String json = factory.post("/api/issues/change", Collections.<String, Object>emptyMap());
-
-    assertThat(json).isEqualTo("{}");
-    assertThat(httpServer.requestedPath()).isEqualTo("/api/issues/change");
-  }
-
-  @Test
-  public void post_successful_if_no_content() {
-    httpServer.stubStatusCode(204).stubResponseBody("");
-
-    HttpRequestFactory factory = new HttpRequestFactory(httpServer.url());
-    String json = factory.post("/api/issues/change", Collections.<String, Object>emptyMap());
-
-    assertThat(json).isEqualTo("");
-    assertThat(httpServer.requestedPath()).isEqualTo("/api/issues/change");
+    HttpRequestFactory factory = new HttpRequestFactory("http://localhost:1");
+    factory.execute(new WsRequest("/api/issues"));
   }
 
   @Test
@@ -96,7 +56,7 @@ public class HttpRequestFactoryTest {
     httpServer.stubStatusCode(200).stubResponseBody("{}");
 
     HttpRequestFactory factory = new HttpRequestFactory(httpServer.url()).setLogin("karadoc").setPassword("legrascestlavie");
-    String json = factory.get("/api/issues", Collections.<String, Object>emptyMap());
+    String json = factory.execute(new WsRequest("/api/issues"));
 
     assertThat(json).isEqualTo("{}");
     assertThat(httpServer.requestedPath()).isEqualTo("/api/issues");
@@ -105,34 +65,21 @@ public class HttpRequestFactoryTest {
 
   @Test
   public void test_proxy() throws Exception {
+    expectedException.expect(IllegalStateException.class);
+
     HttpRequestFactory factory = new HttpRequestFactory(httpServer.url())
       .setProxyHost("localhost").setProxyPort(1)
       .setProxyLogin("john").setProxyPassword("smith");
-    try {
-      factory.get("/api/issues", Collections.<String, Object>emptyMap());
-      fail();
-    } catch (IllegalStateException e) {
-      // it's not possible to check that the proxy is correctly configured
-      assertThat(e.getCause()).isInstanceOf(ConnectException.class);
-    }
+    factory.execute(new WsRequest("/api/issues"));
   }
 
   @Test
   public void beginning_slash_is_optional() throws Exception {
     HttpRequestFactory factory = new HttpRequestFactory(httpServer.url());
-    factory.get("api/foo", Collections.<String, Object>emptyMap());
+    factory.execute(new WsRequest("api/foo"));
     assertThat(httpServer.requestedPath()).isEqualTo("/api/foo");
 
-    factory.get("/api/bar", Collections.<String, Object>emptyMap());
+    factory.execute(new WsRequest("/api/bar"));
     assertThat(httpServer.requestedPath()).isEqualTo("/api/bar");
   }
-
-  protected static Date toDate(String sDate) {
-    try {
-      SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd");
-      return sdf.parse(sDate);
-    } catch (ParseException e) {
-      throw new RuntimeException(e);
-    }
-  }
 }
diff --git a/sonar-ws/src/test/java/org/sonarqube/ws/client/WsClientTest.java b/sonar-ws/src/test/java/org/sonarqube/ws/client/WsClientTest.java
index 6a62da72401..715d8a4cc76 100644
--- a/sonar-ws/src/test/java/org/sonarqube/ws/client/WsClientTest.java
+++ b/sonar-ws/src/test/java/org/sonarqube/ws/client/WsClientTest.java
@@ -92,7 +92,7 @@ public class WsClientTest {
     expectedException.expect(IllegalStateException.class);
     expectedException.expectMessage("Server URL must be set");
 
-    underTest.builder().build();
+    WsClient.builder().build();
   }
 
   @Test
@@ -100,7 +100,7 @@ public class WsClientTest {
     expectedException.expect(IllegalStateException.class);
     expectedException.expectMessage("Server URL must be set");
 
-    underTest.create("");
+    WsClient.create("");
   }
 
   @Test
@@ -119,7 +119,8 @@ public class WsClientTest {
 
   @Test
   public void test_custom_configuration() throws Exception {
-    underTest = WsClient.builder().url("http://localhost:9000")
+    underTest = WsClient.builder()
+      .url("http://localhost:9000")
       .login("eric")
       .password("pass")
       .connectTimeoutMilliseconds(12345)