From 1b0fa952d6f0715ceef794484c3cfd03c8c756ef Mon Sep 17 00:00:00 2001 From: Israel Colomer Date: Fri, 13 May 2016 12:20:17 +0100 Subject: [PATCH] Allow datamill client to perform PATCH requests Reimplemented client request core method to rely on httpclient rather than JDK UrlConnection, as the latter doesn't allow certain Http methods (i.e. PATCH). Adapted tests to new implementation. --- core/pom.xml | 5 + .../org/chodavarapu/datamill/http/Client.java | 205 ++++++++++++------ .../org/chodavarapu/datamill/http/Method.java | 2 +- .../chodavarapu/datamill/http/ClientTest.java | 105 ++++++--- .../http/impl/RequestBuilderImplTest.java | 2 - 5 files changed, 212 insertions(+), 107 deletions(-) diff --git a/core/pom.xml b/core/pom.xml index bc76850..5a76b48 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -120,6 +120,11 @@ system-rules 1.16.0 + + org.apache.httpcomponents + httpclient + 4.5.2 + com.fasterxml.jackson.core jackson-core diff --git a/core/src/main/java/org/chodavarapu/datamill/http/Client.java b/core/src/main/java/org/chodavarapu/datamill/http/Client.java index 97b7e42..4be6820 100644 --- a/core/src/main/java/org/chodavarapu/datamill/http/Client.java +++ b/core/src/main/java/org/chodavarapu/datamill/http/Client.java @@ -1,9 +1,30 @@ package org.chodavarapu.datamill.http; -import com.google.common.base.Joiner; import com.google.common.collect.Multimap; import com.google.common.collect.Multimaps; -import org.chodavarapu.datamill.http.impl.*; +import org.apache.http.Header; +import org.apache.http.client.config.RequestConfig; +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.client.methods.HttpDelete; +import org.apache.http.client.methods.HttpEntityEnclosingRequestBase; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.client.methods.HttpHead; +import org.apache.http.client.methods.HttpOptions; +import org.apache.http.client.methods.HttpPatch; +import org.apache.http.client.methods.HttpPost; +import org.apache.http.client.methods.HttpPut; +import org.apache.http.client.methods.HttpRequestBase; +import org.apache.http.client.methods.HttpTrace; +import org.apache.http.client.methods.HttpUriRequest; +import org.apache.http.client.utils.URIBuilder; +import org.apache.http.entity.BasicHttpEntity; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClients; +import org.chodavarapu.datamill.http.impl.InputStreamEntity; +import org.chodavarapu.datamill.http.impl.RequestBuilderImpl; +import org.chodavarapu.datamill.http.impl.ResponseImpl; +import org.chodavarapu.datamill.http.impl.TemplateBasedUriBuilder; +import org.chodavarapu.datamill.http.impl.ValueEntity; import org.chodavarapu.datamill.values.Value; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -12,16 +33,13 @@ import rx.util.async.Async; import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; +import java.io.PipedInputStream; +import java.io.PipedOutputStream; import java.io.UnsupportedEncodingException; -import java.net.HttpURLConnection; -import java.net.URL; -import java.net.URLConnection; +import java.net.URI; +import java.net.URISyntaxException; import java.net.URLEncoder; import java.util.HashMap; -import java.util.Iterator; -import java.util.List; import java.util.Map; import java.util.function.Function; @@ -42,10 +60,6 @@ public Observable request(Method method, Map headers, return request(method, headers, uri, new ValueEntity(entity)); } - protected URLConnection createConnection(String uri) throws IOException { - return new URL(uri).openConnection(); - } - public Observable request(Method method, Map headers, String uri, Entity entity) { return request(method, headers != null ? Multimaps.forMap(headers) : null, uri, null, null, null, entity); } @@ -61,100 +75,147 @@ public Observable request( if (uriParameters != null && uriParameters.size() > 0) { uri = uriBuilder.build(uri, uriParameters); } + URIBuilder uriBuilder = new URIBuilder(); + + + int firstSlash = uri.indexOf('/'); + uriBuilder.setScheme(uri.substring(0, firstSlash - 1)); + + String uriNoScheme = uri.substring(firstSlash + 1, uri.length()); + uriBuilder.setHost(uriNoScheme.substring(1)); - uri = appendQueryParameters(uri, queryParameters); + URI anURI = null; + try { + anURI = appendQueryParameters(uriBuilder, queryParameters); + } catch (URISyntaxException e) { + e.printStackTrace(); + } + final URI theURI = anURI; final String composedUri = uri; return Async.fromCallable(() -> { - URLConnection urlConnection = createConnection(composedUri); - HttpURLConnection httpConnection = (HttpURLConnection) urlConnection; + PipedOutputStream pipedOutputStream = null; + PipedInputStream pipedInputStream = null; + + try { + + + CloseableHttpClient httpclient = HttpClients.createDefault(); - httpConnection.setRequestMethod(method.toString()); + HttpUriRequest request = buildHttpRequest(method, theURI); - if (options != null && options.size() > 0) { - Object connectTimeout = options.get(Request.OPTION_CONNECT_TIMEOUT); - if (connectTimeout instanceof Integer) { - httpConnection.setConnectTimeout((int) connectTimeout); + if (options != null && options.size() > 0) { + Object connectTimeout = options.get(Request.OPTION_CONNECT_TIMEOUT); + if (connectTimeout instanceof Integer) { + RequestConfig requestConfig = RequestConfig.custom() + .setConnectTimeout((int) connectTimeout) + .build(); + ((HttpRequestBase) request).setConfig(requestConfig); + } } - } - if (headers != null) { - for (Map.Entry header : headers.entries()) { - httpConnection.addRequestProperty(header.getKey(), header.getValue()); + if (headers != null) { + for (Map.Entry header : headers.entries()) { + request.addHeader(header.getKey(), header.getValue()); + } } - } - if (entity != null) { - writeEntityOutOverConnection(entity, httpConnection); - } + if (entity != null) { + if (!(request instanceof HttpEntityEnclosingRequestBase)) { + throw new IllegalArgumentException("Expecting to write an entity for a request type that does not support it!"); + } + + pipedOutputStream = buildPipedOutputStream(); + pipedInputStream = buildPipedInputStream(); + pipedInputStream.connect(pipedOutputStream); + + BasicHttpEntity httpEntity = new BasicHttpEntity(); + httpEntity.setContent(pipedInputStream); + ((HttpEntityEnclosingRequestBase) request).setEntity(httpEntity); - logger.debug("Making HTTP request {} {}", method.name(), composedUri); - if (headers != null && logger.isDebugEnabled()) { - logger.debug(" HTTP request headers:"); - for (Map.Entry header : headers.entries()) { - logger.debug(" {}: {}", header.getKey(), header.getValue()); + writeEntityOutOverConnection(entity, pipedOutputStream); } - } - int responseCode = httpConnection.getResponseCode(); - InputStream inputStream = httpConnection.getInputStream(); + logger.debug("Making HTTP request {} {}", method.name(), composedUri); + if (headers != null && logger.isDebugEnabled()) { + logger.debug(" HTTP request headers:"); + for (Map.Entry header : headers.entries()) { + logger.debug(" {}: {}", header.getKey(), header.getValue()); + } + } + + CloseableHttpResponse httpResponse = httpclient.execute(request); + + int responseCode = httpResponse.getStatusLine().getStatusCode(); + Map combinedHeaders = new HashMap<>(); - Map> responseHeaders = httpConnection.getHeaderFields(); - Map combinedHeaders = new HashMap<>(); - for (Map.Entry> header : responseHeaders.entrySet()) { - if (header.getValue().size() > 1) { - combinedHeaders.put(header.getKey(), Joiner.on(',').join(header.getValue())); - } else { - combinedHeaders.put(header.getKey(), header.getValue().get(0)); + for (Header header : httpResponse.getAllHeaders()) { + combinedHeaders.put(header.getName(), header.getValue()); } - } - return new ResponseImpl(Status.valueOf(responseCode), combinedHeaders, new InputStreamEntity(inputStream)); + return new ResponseImpl(Status.valueOf(responseCode), combinedHeaders, new InputStreamEntity(httpResponse.getEntity().getContent())); + } finally { + if (pipedInputStream != null) { + try { + pipedInputStream.close(); + } catch (IOException e) {} + } + if (pipedOutputStream != null) { + try { + pipedOutputStream.close(); + } catch (IOException e) {} + } + } }, Schedulers.io()); } - private String appendQueryParameters(String uri, Multimap queryParameters) { - if (queryParameters != null && queryParameters.size() > 0) { - try { - StringBuilder queryBuilder = new StringBuilder("?"); - Iterator> iterator = queryParameters.entries().iterator(); - while (iterator.hasNext()) { - Map.Entry parameter = iterator.next(); + protected PipedOutputStream buildPipedOutputStream() { + return new PipedOutputStream(); + } - queryBuilder.append(URLEncoder.encode(parameter.getKey(), "UTF-8")); - queryBuilder.append('='); + protected PipedInputStream buildPipedInputStream() { + return new PipedInputStream(); + } - if (parameter.getValue() != null) { - queryBuilder.append(URLEncoder.encode(parameter.getValue())); - } + protected HttpUriRequest buildHttpRequest(Method method, URI uri) { + switch (method) { + case OPTIONS: return new HttpOptions(uri); + case GET: return new HttpGet(uri); + case HEAD: return new HttpHead(uri); + case POST: return new HttpPost(uri); + case PUT: return new HttpPut(uri); + case DELETE:return new HttpDelete(uri); + case TRACE: return new HttpTrace(uri); + case PATCH: return new HttpPatch(uri); + default: throw new IllegalArgumentException("Method " + method + " is not implemented!"); + } + } - if (iterator.hasNext()) { - queryBuilder.append('&'); - } + private URI appendQueryParameters(URIBuilder uriBuilder, Multimap queryParameters) throws URISyntaxException { + if (queryParameters != null && queryParameters.size() > 0) { + queryParameters.entries().stream().forEach(entry -> { + try { + uriBuilder.setParameter(URLEncoder.encode(entry.getKey(), "UTF-8"), entry.getValue()); } - - uri = uri + queryBuilder.toString(); - } catch (UnsupportedEncodingException e) { - } + catch (UnsupportedEncodingException e) {} + }); } - return uri; + return uriBuilder.build(); } - private void writeEntityOutOverConnection(Entity entity, HttpURLConnection httpConnection) throws IOException { - httpConnection.setDoOutput(true); - OutputStream outputStream = httpConnection.getOutputStream(); + private void writeEntityOutOverConnection(Entity entity, PipedOutputStream pipedOutputStream) throws IOException { entity.asChunks().observeOn(Schedulers.io()) .doOnNext(bytes -> { try { - outputStream.write(bytes); + pipedOutputStream.write(bytes); } catch (IOException e) { throw new HttpException("Error writing entity!", e); } }) .doOnCompleted(() -> { try { - outputStream.close(); + pipedOutputStream.close(); onEntitySendingCompletion(entity); } catch (IOException e) { throw new HttpException("Error while closing stream!", e); @@ -162,7 +223,7 @@ private void writeEntityOutOverConnection(Entity entity, HttpURLConnection httpC }) .doOnError(e -> { try { - outputStream.close(); + pipedOutputStream.close(); onErrorSendingEntity(entity); } catch (IOException closing) { onErrorSendingEntity(entity); diff --git a/core/src/main/java/org/chodavarapu/datamill/http/Method.java b/core/src/main/java/org/chodavarapu/datamill/http/Method.java index 5078997..87ebc23 100644 --- a/core/src/main/java/org/chodavarapu/datamill/http/Method.java +++ b/core/src/main/java/org/chodavarapu/datamill/http/Method.java @@ -7,7 +7,7 @@ * @author Ravi Chodavarapu (rchodava@gmail.com) */ public enum Method { - OPTIONS, GET, HEAD, POST, PUT, DELETE, TRACE, CONNECT, PATCH, UNKNOWN; + OPTIONS, GET, HEAD, POST, PUT, DELETE, TRACE, PATCH, UNKNOWN; private static final Set methods = EnumSet.allOf(Method.class); diff --git a/core/src/test/java/org/chodavarapu/datamill/http/ClientTest.java b/core/src/test/java/org/chodavarapu/datamill/http/ClientTest.java index c96e8b9..7ab469e 100644 --- a/core/src/test/java/org/chodavarapu/datamill/http/ClientTest.java +++ b/core/src/test/java/org/chodavarapu/datamill/http/ClientTest.java @@ -1,22 +1,32 @@ package org.chodavarapu.datamill.http; import com.google.common.collect.ImmutableMap; +import org.apache.http.client.methods.HttpDelete; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.client.methods.HttpHead; +import org.apache.http.client.methods.HttpOptions; +import org.apache.http.client.methods.HttpPatch; +import org.apache.http.client.methods.HttpPost; +import org.apache.http.client.methods.HttpPut; +import org.apache.http.client.methods.HttpTrace; +import org.apache.http.client.methods.HttpUriRequest; import org.chodavarapu.datamill.http.impl.ValueEntity; import org.chodavarapu.datamill.values.StringValue; import org.junit.Test; import rx.Observable; import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.net.HttpURLConnection; -import java.net.URLConnection; +import java.io.PipedOutputStream; +import java.net.URI; import java.util.Map; import java.util.concurrent.Semaphore; import java.util.function.Function; -import static org.junit.Assert.assertEquals; -import static org.mockito.Matchers.anyString; -import static org.mockito.Mockito.*; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; /** * @author Ravi Chodavarapu (rchodava@gmail.com) @@ -24,19 +34,20 @@ public class ClientTest { private void verifyConnectionSetup(TestClient client, Method method, String uri, Map headers, String entity) throws Exception { - assertEquals(uri, client.getLastUri()); - verify(client.getMockConnection(), times(1)).setRequestMethod(method.name()); + HttpUriRequest request = client.getRequest(); + + assertThat(request.getURI().toString(), equalTo(uri)); + assertThat(request.getMethod(), equalTo(method.name())); if (headers != null) { for (Map.Entry header : headers.entrySet()) { - verify(client.getMockConnection(), times(1)).addRequestProperty(header.getKey(), header.getValue()); + verify(request, times(1)).addHeader(header.getKey(), header.getValue()); } - } else { - verify(client.getMockConnection(), times(0)).addRequestProperty(anyString(), anyString()); } if (entity != null) { - assertEquals(entity, client.getLastOutputValue()); + byte[] testAsBytes = entity.getBytes(); + verify(client.getSpiedPipedOutputStream()).write(testAsBytes, 0, testAsBytes.length); } } @@ -201,31 +212,24 @@ public void putRequests() throws Exception { private class TestClient extends Client { private final ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); - private String uri; - private HttpURLConnection mockConnection; private Semaphore entitySent = new Semaphore(0); + private PipedOutputStream spiedPipedOutputStream; + + private HttpUriRequest request; + public TestClient() { - mockConnection = mock(HttpURLConnection.class); - try { - when(mockConnection.getOutputStream()).thenReturn(outputStream); - } catch (IOException e) { - throw new RuntimeException(e); - } } - @Override - protected URLConnection createConnection(String uri) throws IOException { - this.uri = uri; - return mockConnection; - } - public String getLastUri() { - return uri; + PipedOutputStream getSpiedPipedOutputStream() { + return spiedPipedOutputStream; } - public HttpURLConnection getMockConnection() { - return mockConnection; + @Override + protected PipedOutputStream buildPipedOutputStream() { + this.spiedPipedOutputStream = spy(PipedOutputStream.class); + return this.spiedPipedOutputStream; } @Override @@ -233,9 +237,46 @@ protected void onEntitySendingCompletion(Entity entity) { entitySent.release(); } - public String getLastOutputValue() { - entitySent.acquireUninterruptibly(); - return new String(outputStream.toByteArray()); + public HttpUriRequest getRequest() { + return request; + } + + protected HttpUriRequest buildHttpRequest(Method method, URI uri) { + switch (method) { + case OPTIONS: + HttpOptions httpOptions = new HttpOptions(uri); + this.request = spy(httpOptions); + return request; + case GET: + HttpGet httpGet = new HttpGet(uri); + this.request = spy(httpGet); + return request; + case HEAD: + HttpHead httpHead = new HttpHead(uri); + this.request = spy(httpHead); + return request; + case POST: + HttpPost httpPost = new HttpPost(uri); + this.request = spy(httpPost); + return request; + case PUT: + HttpPut httpPut = new HttpPut(uri); + this.request = spy(httpPut); + return request; + case DELETE: + HttpDelete httpDelete = new HttpDelete(uri); + this.request = spy(httpDelete); + return request; + case TRACE: + HttpTrace httpTrace = new HttpTrace(uri); + this.request = spy(httpTrace); + return request; + case PATCH: + HttpPatch httpPatch = new HttpPatch(uri); + this.request = spy(httpPatch); + return request; + default: throw new IllegalArgumentException("Method " + method + " is not implemented!"); + } } } } diff --git a/core/src/test/java/org/chodavarapu/datamill/http/impl/RequestBuilderImplTest.java b/core/src/test/java/org/chodavarapu/datamill/http/impl/RequestBuilderImplTest.java index d27bb53..869dc65 100644 --- a/core/src/test/java/org/chodavarapu/datamill/http/impl/RequestBuilderImplTest.java +++ b/core/src/test/java/org/chodavarapu/datamill/http/impl/RequestBuilderImplTest.java @@ -55,14 +55,12 @@ public void entity() { @Test public void methods() { - assertEquals(Method.CONNECT, new RequestBuilderImpl().method("CONNECT").build().method()); assertEquals(Method.GET, new RequestBuilderImpl().method("GET").build().method()); assertEquals(Method.HEAD, new RequestBuilderImpl().method("HEAD").build().method()); assertEquals(Method.POST, new RequestBuilderImpl().method("POST").build().method()); assertEquals(Method.PUT, new RequestBuilderImpl().method("PUT").build().method()); assertEquals(Method.DELETE, new RequestBuilderImpl().method("DELETE").build().method()); assertEquals(Method.TRACE, new RequestBuilderImpl().method("TRACE").build().method()); - assertEquals(Method.CONNECT, new RequestBuilderImpl().method("CONNECT").build().method()); assertEquals(Method.PATCH, new RequestBuilderImpl().method("PATCH").build().method()); } }