Fix quality flaws

This commit is contained in:
Teryk Bellahsene 2015-10-15 09:43:57 +02:00
parent 77a9aece7d
commit 62dd54d41a
4 changed files with 44 additions and 95 deletions

View File

@ -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) {

View File

@ -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;

View File

@ -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() {
expectedException.expect(IllegalStateException.class);
expectedException.expectMessage("Fail to request http://localhost:1/api/issues");
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("{}");
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");
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);
}
}
}

View File

@ -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)