(TK-103) Reworked sync single request APOIs to create and close client

For both the Clojure and Java single request APIs, the internal
implementation now creates an http client, makes a request on it, and
calls close on the client when done.  The close at the end avoids
resource leaks.

In support of this, the `JavaClient` class now requires the caller to
create an instance rather than just make a call to the static
`request` method on the `JavaClient` class.

The `coerceBodyType` method on the `JavaClient` class now closes the
body `InputStream` after coercing the stream content into a `String`.
This eliminates a possible resource leak and makes the Java-layer
coerce implementation consistent with the Clojure-layer coerce in
that the latter was already, via the use of the Clojure `slurp`
function, closing the stream content after coercing to `String`.
This commit is contained in:
Jeremy Barlow 2014-11-12 14:11:16 -08:00
parent c1abea3e37
commit e5f55affd3
8 changed files with 117 additions and 121 deletions

View file

@ -257,12 +257,6 @@
opts
(HttpClientException. "Request cancelled"))))))
(schema/defn extract-client-opts :- common/ClientOptions
[opts :- common/UserRequestOptions]
(select-keys opts [:ssl-context :ssl-ca-cert :ssl-cert :ssl-key
:ssl-protocols :cipher-suites
:force-redirects :follow-redirects]))
(schema/defn extract-ssl-opts :- common/SslOptions
[opts :- common/ClientOptions]
(select-keys opts [:ssl-context :ssl-ca-cert :ssl-cert :ssl-key]))
@ -320,9 +314,7 @@
:decompress-body true
:as :stream}
opts (merge defaults opts)
client-opts (extract-client-opts opts)
request-opts (extract-request-opts opts)
client (or client (create-default-client client-opts))
{:keys [method url body] :as coerced-opts} (coerce-opts opts)
request (construct-request method url)
result (promise)]
@ -342,48 +334,6 @@
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;;; Public
(schema/defn ^:always-validate request :- common/ResponsePromise
"Issues an async HTTP request and returns a promise object to which the value
of `(callback {:opts _ :status _ :headers _ :body _})` or
`(callback {:opts _ :error _})` will be delivered.
When unspecified, `callback` is the identity function.
Request options:
* :url
* :method - the HTTP method (:get, :head, :post, :put, :delete, :options, :patch
* :headers - a map of headers
* :body - the body; may be a String or any type supported by clojure's reader
* :decompress-body - if `true`, an 'accept-encoding' header with a value of
'gzip, deflate' will be added to the request, and the response will be
automatically decompressed if it contains a recognized 'content-encoding'
header. defaults to `true`.
* :as - used to control the data type of the response body. Supported values are
`:text` and `:stream`, which will return a `String` or an `InputStream`,
respectively. Defaults to `:stream`.
* :query-params - used to set the query parameters of an http request
* :force-redirects - used to set whether or not the client should follow
redirects on POST or PUT requests. Defaults to false.
* :follow-redirects - used to set whether or not the client should follow
redirects in general. Defaults to true. If set to false, will override
the :force-redirects setting.
SSL options:
* :ssl-context - an instance of SSLContext
OR
* :ssl-cert - path to a PEM file containing the client cert
* :ssl-key - path to a PEM file containing the client private key
* :ssl-ca-cert - path to a PEM file containing the CA cert"
([opts :- common/RawUserRequestClientOptions]
(request opts nil))
([opts :- common/RawUserRequestClientOptions
callback :- common/ResponseCallbackFn]
(request-with-client opts callback nil)))
(schema/defn create-client :- common/HTTPClient
[opts :- common/ClientOptions]
(let [client (create-default-client opts)]

View file

@ -12,6 +12,12 @@
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;;; Private utility functions
(schema/defn extract-client-opts :- common/ClientOptions
[opts :- common/RawUserRequestClientOptions]
(select-keys opts [:ssl-context :ssl-ca-cert :ssl-cert :ssl-key
:ssl-protocols :cipher-suites
:force-redirects :follow-redirects]))
(defn request-with-client
[req client]
(let [{:keys [error] :as resp} @(async/request-with-client req nil client)]
@ -19,16 +25,19 @@
(throw error)
resp)))
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;;; Public
(defn request
[req]
(let [{:keys [error] :as resp} @(async/request req nil)]
(if error
(throw error)
resp)))
(let [client (async/create-default-client (extract-client-opts req))]
(try
(let [{:keys [error] :as resp} @(async/request-with-client req nil client)]
(if error
(throw error)
resp))
(finally
(.close client)))))
(schema/defn create-client :- common/HTTPClient
[opts :- common/ClientOptions]

View file

@ -2,12 +2,11 @@ package com.puppetlabs.http.client;
import com.puppetlabs.http.client.impl.Promise;
import java.io.Closeable;
import java.net.URI;
import java.net.URISyntaxException;
public interface AsyncHttpClient {
public void close();
public interface AsyncHttpClient extends Closeable{
public Promise<Response> get(String url) throws URISyntaxException;
public Promise<Response> get(URI uri);
public Promise<Response> get(RequestOptions requestOptions);

View file

@ -1,15 +1,17 @@
package com.puppetlabs.http.client;
import com.puppetlabs.http.client.impl.SslUtils;
import com.puppetlabs.http.client.impl.Promise;
import com.puppetlabs.http.client.impl.JavaClient;
import com.puppetlabs.http.client.impl.PersistentSyncHttpClient;
import com.puppetlabs.http.client.impl.CoercedClientOptions;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import javax.net.ssl.SSLContext;
import java.io.IOException;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.Map;
public class Sync {
private static final Logger LOGGER = LoggerFactory.getLogger(Sync.class);
@ -20,32 +22,63 @@ public class Sync {
throw new HttpClientException(msg, t);
}
private static Response request(SimpleRequestOptions requestOptions, HttpMethod method) {
private static RequestOptions extractRequestOptions(SimpleRequestOptions simpleOptions) {
URI uri = simpleOptions.getUri();
Map<String, String> headers = simpleOptions.getHeaders();
Object body = simpleOptions.getBody();
boolean decompressBody = simpleOptions.getDecompressBody();
ResponseBodyType as = simpleOptions.getAs();
return new RequestOptions(uri, headers, body, decompressBody, as);
}
private static ClientOptions extractClientOptions(SimpleRequestOptions simpleOptions) {
SSLContext sslContext = simpleOptions.getSslContext();
String sslCert = simpleOptions.getSslCert();
String sslKey = simpleOptions.getSslKey();
String sslCaCert = simpleOptions.getSslCaCert();
String[] sslProtocols = simpleOptions.getSslProtocols();
String[] sslCipherSuites = simpleOptions.getSslCipherSuites();
boolean insecure = simpleOptions.getInsecure();
boolean forceRedirects = simpleOptions.getForceRedirects();
boolean followRedirects = simpleOptions.getFollowRedirects();
return new ClientOptions(sslContext, sslCert, sslKey, sslCaCert,
sslProtocols, sslCipherSuites, insecure,
forceRedirects, followRedirects);
}
private static Response request(SimpleRequestOptions simpleRequestOptions,
HttpMethod method) {
// TODO: if we end up implementing an async version of the java API,
// we should refactor this implementation so that it is based on the
// async one, as Patrick has done in the clojure API.
Promise<Response> promise = JavaClient.request(requestOptions, method, null);
Response response = null;
final SyncHttpClient client = createClient(
extractClientOptions(simpleRequestOptions));
try {
response = promise.deref();
} catch (InterruptedException e) {
logAndRethrow("Error while waiting for http response", e);
response = client.request(
extractRequestOptions(simpleRequestOptions),
method);
}
if (response.getError() != null) {
logAndRethrow("Error executing http request", response.getError());
finally {
try {
client.close();
}
catch (IOException e) {
logAndRethrow("Error closing client", e);
}
}
return response;
}
public static SyncHttpClient createClient(ClientOptions clientOptions) {
clientOptions = SslUtils.configureSsl(clientOptions);
CoercedClientOptions coercedClientOptions = JavaClient.coerceClientOptions(clientOptions);
return new PersistentSyncHttpClient(JavaClient.createClient(coercedClientOptions));
CoercedClientOptions coercedClientOptions =
JavaClient.coerceClientOptions(
SslUtils.configureSsl(clientOptions));
return new PersistentSyncHttpClient(
JavaClient.createClient(coercedClientOptions));
}
public static Response get(String url) throws URISyntaxException {
return get(new URI(url));
}

View file

@ -1,10 +1,11 @@
package com.puppetlabs.http.client;
import java.io.Closeable;
import java.net.URI;
import java.net.URISyntaxException;
public interface SyncHttpClient {
public void close();
public interface SyncHttpClient extends Closeable {
public Response request(RequestOptions requestOptions, HttpMethod method);
public Response get(String url) throws URISyntaxException;
public Response get(URI uri);

View file

@ -1,11 +1,32 @@
package com.puppetlabs.http.client.impl;
import com.puppetlabs.http.client.*;
import com.puppetlabs.http.client.ClientOptions;
import com.puppetlabs.http.client.HttpClientException;
import com.puppetlabs.http.client.HttpMethod;
import com.puppetlabs.http.client.RequestOptions;
import com.puppetlabs.http.client.Response;
import com.puppetlabs.http.client.ResponseBodyType;
import org.apache.commons.io.IOUtils;
import org.apache.http.*;
import org.apache.http.Consts;
import org.apache.http.Header;
import org.apache.http.HttpEntity;
import org.apache.http.HttpRequest;
import org.apache.http.HttpResponse;
import org.apache.http.ParseException;
import org.apache.http.ProtocolException;
import org.apache.http.client.RedirectStrategy;
import org.apache.http.client.methods.*;
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.concurrent.FutureCallback;
import org.apache.http.entity.ContentType;
import org.apache.http.entity.InputStreamEntity;
@ -15,17 +36,18 @@ import org.apache.http.impl.nio.client.CloseableHttpAsyncClient;
import org.apache.http.impl.nio.client.HttpAsyncClientBuilder;
import org.apache.http.impl.nio.client.HttpAsyncClients;
import org.apache.http.message.BasicHeader;
import org.apache.http.nio.client.HttpAsyncClient;
import org.apache.http.nio.conn.ssl.SSLIOSessionStrategy;
import org.apache.http.nio.entity.NStringEntity;
import org.apache.http.protocol.HttpContext;
import javax.net.ssl.*;
import javax.net.ssl.SSLContext;
import javax.net.ssl.TrustManager;
import javax.net.ssl.X509TrustManager;
import java.io.IOException;
import java.io.InputStream;
import java.io.UnsupportedEncodingException;
import java.net.URI;
import java.nio.charset.Charset;
import java.nio.charset.UnsupportedCharsetException;
import java.security.KeyManagementException;
import java.security.NoSuchAlgorithmException;
@ -186,16 +208,6 @@ public class JavaClient {
return context;
}
public static Promise<Response> request(final SimpleRequestOptions simpleRequestOptions, final HttpMethod method, final IResponseCallback callback) {
RequestOptions requestOptions = extractRequestOptions(simpleRequestOptions);
ClientOptions clientOptions = SslUtils.configureSsl(extractClientOptions(simpleRequestOptions));
CoercedClientOptions coercedClientOptions = coerceClientOptions(clientOptions);
final CloseableHttpAsyncClient client = createClient(coercedClientOptions);
return requestWithClient(requestOptions, method, callback, client);
}
public static Promise<Response> requestWithClient(final RequestOptions requestOptions,
final HttpMethod method,
final IResponseCallback callback,
@ -363,6 +375,8 @@ public class JavaClient {
public static Object coerceBodyType(InputStream body, ResponseBodyType as,
ContentType contentType) {
Object response = null;
switch (as) {
case TEXT:
String charset = "UTF-8";
@ -370,38 +384,21 @@ public class JavaClient {
charset = contentType.getCharset().name();
}
try {
return IOUtils.toString(body, charset);
response = IOUtils.toString(body, charset);
} catch (IOException e) {
throw new HttpClientException("Unable to read body as string", e);
}
try {
body.close();
} catch (IOException e) {
throw new HttpClientException(
"Unable to close response stream", e);
}
break;
default:
throw new HttpClientException("Unsupported body type: " + as);
}
return response;
}
private static RequestOptions extractRequestOptions(SimpleRequestOptions simpleOptions) {
URI uri = simpleOptions.getUri();
Map<String, String> headers = simpleOptions.getHeaders();
Object body = simpleOptions.getBody();
boolean decompressBody = simpleOptions.getDecompressBody();
ResponseBodyType as = simpleOptions.getAs();
return new RequestOptions(uri, headers, body, decompressBody, as);
}
private static ClientOptions extractClientOptions(SimpleRequestOptions simpleOptions) {
SSLContext sslContext = simpleOptions.getSslContext();
String sslCert = simpleOptions.getSslCert();
String sslKey = simpleOptions.getSslKey();
String sslCaCert = simpleOptions.getSslCaCert();
String[] sslProtocols = simpleOptions.getSslProtocols();
String[] sslCipherSuites = simpleOptions.getSslCipherSuites();
boolean insecure = simpleOptions.getInsecure();
boolean forceRedirects = simpleOptions.getForceRedirects();
boolean followRedirects = simpleOptions.getFollowRedirects();
return new ClientOptions(sslContext, sslCert, sslKey, sslCaCert,
sslProtocols, sslCipherSuites, insecure,
forceRedirects, followRedirects);
}
}

View file

@ -6,6 +6,7 @@ import com.puppetlabs.http.client.HttpMethod;
import com.puppetlabs.http.client.AsyncHttpClient;
import org.apache.http.impl.nio.client.CloseableHttpAsyncClient;
import java.io.IOException;
import java.net.URI;
import java.net.URISyntaxException;
@ -16,7 +17,9 @@ public class PersistentAsyncHttpClient implements AsyncHttpClient {
this.client = client;
}
public void close() {}
public void close() throws IOException {
client.close();
}
private Promise<Response> request(RequestOptions requestOptions, HttpMethod method) {
return JavaClient.requestWithClient(requestOptions, method, null, client);

View file

@ -9,6 +9,7 @@ import org.apache.http.impl.nio.client.CloseableHttpAsyncClient;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.net.URI;
import java.net.URISyntaxException;
@ -25,8 +26,9 @@ public class PersistentSyncHttpClient implements SyncHttpClient {
throw new HttpClientException(msg, t);
}
private Response request(RequestOptions requestOptions, HttpMethod method) {
Promise<Response> promise = JavaClient.requestWithClient(requestOptions, method, null, client);
public Response request(RequestOptions requestOptions, HttpMethod method) {
Promise<Response> promise =
JavaClient.requestWithClient(requestOptions, method, null, client);
Response response = null;
try {
@ -40,7 +42,9 @@ public class PersistentSyncHttpClient implements SyncHttpClient {
return response;
}
public void close() {}
public void close() throws IOException {
client.close();
}
public Response get(String url) throws URISyntaxException {
return get(new URI(url));