Index: google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java =================================================================== --- a/google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java +++ b/google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java @@ -711,6 +711,20 @@ * Almost all details of the request and response are logged if {@link Level#CONFIG} is loggable. * The only exception is the value of the {@code Authorization} header which is only logged if * {@link Level#ALL} is loggable. + *
+ * Callers should call {@link HttpResponse#disconnect} when the returned HTTP response object is + * no longer needed. However, {@link HttpResponse#disconnect} does not have to be called if the + * response stream is properly closed. Example usage: + *
+ * + *+ HttpResponse response = request.execute(); + try { + // process the HTTP response object + } finally { + response.disconnect(); + } + ** * @return HTTP response for an HTTP success response (or HTTP error response if * {@link #getThrowExceptionOnExecuteError()} is {@code false}) @@ -842,7 +856,17 @@ // execute lowLevelHttpRequest.setTimeout(connectTimeout, readTimeout); try { - response = new HttpResponse(this, lowLevelHttpRequest.execute()); + LowLevelHttpResponse lowLevelHttpResponse = lowLevelHttpRequest.execute(); + // Flag used to indicate if an exception is thrown before the response is constructed. + boolean responseConstructed = false; + try { + response = new HttpResponse(this, lowLevelHttpResponse); + responseConstructed = true; + } finally { + if (!responseConstructed) { + lowLevelHttpResponse.getContent().close(); + } + } } catch (IOException e) { if (!retryOnExecuteIOException) { throw e; @@ -852,46 +876,60 @@ logger.log(Level.WARNING, e.getMessage(), e); } - if (response != null && !response.isSuccessStatusCode()) { - boolean errorHandled = false; - boolean redirectRequest = false; - boolean backOffRetry = false; - if (unsuccessfulResponseHandler != null) { - // Even if we don't have the potential to retry, we might want to run the - // handler to fix conditions (like expired tokens) that might cause us - // trouble on our next request - errorHandled = unsuccessfulResponseHandler.handleResponse(this, response, retrySupported); - } - if (!errorHandled) { - if (getFollowRedirects() && isRedirected(response)) { - // The unsuccessful request's error could not be handled and it is a redirect request. - handleRedirect(response); - redirectRequest = true; - } else if (retrySupported && backOffPolicy != null - && backOffPolicy.isBackOffRequired(response.getStatusCode())) { - // The unsuccessful request's error could not be handled and should be backed off before - // retrying. - long backOffTime = backOffPolicy.getNextBackOffMillis(); - if (backOffTime != BackOffPolicy.STOP) { - sleep(backOffTime); - backOffRetry = true; + // Flag used to indicate if an exception is thrown before the response has completed + // processing. + boolean responseProcessed = false; + try { + if (response != null && !response.isSuccessStatusCode()) { + boolean errorHandled = false; + boolean redirectRequest = false; + boolean backOffRetry = false; + if (unsuccessfulResponseHandler != null) { + // Even if we don't have the potential to retry, we might want to run the + // handler to fix conditions (like expired tokens) that might cause us + // trouble on our next request + errorHandled = + unsuccessfulResponseHandler.handleResponse(this, response, retrySupported); + } + if (!errorHandled) { + if (getFollowRedirects() && isRedirected(response)) { + // The unsuccessful request's error could not be handled and it is a redirect request. + handleRedirect(response); + redirectRequest = true; + } else if (retrySupported && backOffPolicy != null + && backOffPolicy.isBackOffRequired(response.getStatusCode())) { + // The unsuccessful request's error could not be handled and should be backed off + // before + // retrying. + long backOffTime = backOffPolicy.getNextBackOffMillis(); + if (backOffTime != BackOffPolicy.STOP) { + sleep(backOffTime); + backOffRetry = true; + } } } + // A retry is required if the error was successfully handled or if it is a redirect + // request + // or if the back off policy determined a retry is necessary. + retrySupported &= (errorHandled || redirectRequest || backOffRetry); + // need to close the response stream before retrying a request + if (retrySupported) { + response.ignore(); + } + } else { + // Retry is not required for a successful status code unless the response is null. + retrySupported &= (response == null); } - // A retry is required if the error was successfully handled or if it is a redirect request - // or if the back off policy determined a retry is necessary. - retrySupported &= (errorHandled || redirectRequest || backOffRetry); - // need to close the response stream before retrying a request - if (retrySupported) { - response.ignore(); + // Once there are no more retries remaining, this will be -1 + // Count redirects as retries, we want a finite limit of redirects. + retriesRemaining--; + + responseProcessed = true; + } finally { + if (response != null && !responseProcessed) { + response.disconnect(); } - } else { - // Retry is not required for a successful status code unless the response is null. - retrySupported &= (response == null); } - // Once there are no more retries remaining, this will be -1 - // Count redirects as retries, we want a finite limit of redirects. - retriesRemaining--; } while (retrySupported); if (response == null) { @@ -900,7 +938,11 @@ } if (throwExceptionOnExecuteError && !response.isSuccessStatusCode()) { - throw new HttpResponseException(response); + try { + throw new HttpResponseException(response); + } finally { + response.disconnect(); + } } return response; } Index: google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java =================================================================== --- a/google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java +++ b/google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java @@ -32,6 +32,21 @@ * HTTP response. * *
+ * Callers should call {@link #disconnect} when the HTTP response object is no longer needed. + * However, {@link #disconnect} does not have to be called if the response stream is properly + * closed. Example usage: + *
+ * + *+ HttpResponse response = request.execute(); + try { + // process the HTTP response object + } finally { + response.disconnect(); + } + *+ * + *
* Implementation is not thread-safe. *
* @@ -57,7 +72,7 @@ private final HttpHeaders headers; /** Low-level HTTP response. */ - private LowLevelHttpResponse response; + LowLevelHttpResponse response; /** Status code. */ private final int statusCode; @@ -102,6 +117,9 @@ */ private boolean loggingEnabled; + /** Signals whether the content has been read from the input stream. */ + private boolean contentRead; + HttpResponse(HttpRequest request, LowLevelHttpResponse response) { this.request = request; transport = request.getTransport(); @@ -311,29 +329,52 @@ * Returns the content of the HTTP response. ** The result is cached, so subsequent calls will be fast. + *
+ * Callers should call {@link InputStream#close} after the returned {@link InputStream} is no + * longer needed. Example usage: + * + *
+ InputStream is = response.getContent(); + try { + // Process the input stream.. + } finally { + is.close(); + } + *+ *
+ * {@link HttpResponse#disconnect} does not have to be called if the content is closed. * * @return input stream content of the HTTP response or {@code null} for none * @throws IOException I/O exception */ public InputStream getContent() throws IOException { - LowLevelHttpResponse response = this.response; - if (response == null) { - return content; - } - InputStream content = this.response.getContent(); - this.response = null; - if (content != null) { - // gzip encoding (wrap content with GZipInputStream) - String contentEncoding = this.contentEncoding; - if (contentEncoding != null && contentEncoding.contains("gzip")) { - content = new GZIPInputStream(content); + if (!contentRead) { + InputStream lowLevelResponseContent = this.response.getContent(); + if (lowLevelResponseContent != null) { + // Flag used to indicate if an exception is thrown before the content is successfully + // processed. + boolean contentProcessed = false; + try { + // gzip encoding (wrap content with GZipInputStream) + String contentEncoding = this.contentEncoding; + if (contentEncoding != null && contentEncoding.contains("gzip")) { + lowLevelResponseContent = new GZIPInputStream(lowLevelResponseContent); + } + // logging (wrap content with LoggingInputStream) + Logger logger = HttpTransport.LOGGER; + if (loggingEnabled && logger.isLoggable(Level.CONFIG)) { + lowLevelResponseContent = new LoggingInputStream( + lowLevelResponseContent, logger, Level.CONFIG, contentLoggingLimit); + } + content = lowLevelResponseContent; + contentProcessed = true; + } finally { + if (!contentProcessed) { + lowLevelResponseContent.close(); + } + } } - // logging (wrap content with LoggingInputStream) - Logger logger = HttpTransport.LOGGER; - if (loggingEnabled && logger.isLoggable(Level.CONFIG)) { - content = new LoggingInputStream(content, logger, Level.CONFIG, contentLoggingLimit); - } - this.content = content; + contentRead = true; } return content; } @@ -385,11 +426,17 @@ } /** - * Disconnect using {@link LowLevelHttpResponse#disconnect()}. + * Close the HTTP response content and disconnect using {@link LowLevelHttpResponse#disconnect()}. + * + *
+ * Upgrade warning: since version 1.10 {@link #disconnect} now closes the HTTP response content + * input stream. This was not done by this method prior to version 1.10. + *
* * @since 1.4 */ public void disconnect() throws IOException { + ignore(); response.disconnect(); } Index: google-http-client/src/main/java/com/google/api/client/http/HttpResponseException.java =================================================================== --- a/google-http-client/src/main/java/com/google/api/client/http/HttpResponseException.java +++ b/google-http-client/src/main/java/com/google/api/client/http/HttpResponseException.java @@ -37,25 +37,23 @@ /** HTTP headers. */ private final transient HttpHeaders headers; - /** HTTP response. */ - @Deprecated - private final transient HttpResponse response; - - /** - * Returns the HTTP response. - * - * @since 1.5 - * @deprecated (scheduled to be removed in 1.10) - */ - @Deprecated - public final HttpResponse getResponse() { - return response; - } - /** * Constructor that constructs a detail message from the given HTTP response that includes the * status code, status message and HTTP response content. * + *+ * Callers of this constructor should call {@link HttpResponse#disconnect} after + * {@link HttpResponseException} is instantiated. Example usage: + *
+ * + *+ try { + throw new HttpResponseException(response); + } finally { + response.disconnect(); + } + *+ * * @param response HTTP response */ public HttpResponseException(HttpResponse response) { @@ -65,6 +63,19 @@ /** * Constructor that allows an alternative detail message to be used. * + *
+ * Callers of this constructor should call {@link HttpResponse#disconnect} after + * {@link HttpResponseException} is instantiated. Example usage: + *
+ * + *+ try { + throw new HttpResponseException(response, message); + } finally { + response.disconnect(); + } + *+ * * @param response HTTP response * @param message detail message to use or {@code null} for none * @since 1.6 @@ -74,7 +85,6 @@ statusCode = response.getStatusCode(); statusMessage = response.getStatusMessage(); headers = response.getHeaders(); - this.response = response; } /** @@ -118,7 +128,7 @@ /** * Returns the HTTP response headers. - * + * * @since 1.7 */ public HttpHeaders getHeaders() { Index: google-http-client/src/main/java/com/google/api/client/http/javanet/NetHttpRequest.java =================================================================== --- a/google-http-client/src/main/java/com/google/api/client/http/javanet/NetHttpRequest.java +++ b/google-http-client/src/main/java/com/google/api/client/http/javanet/NetHttpRequest.java @@ -85,8 +85,17 @@ } } // connect - connection.connect(); - return new NetHttpResponse(connection); + boolean successfulConnection = false; + try { + connection.connect(); + NetHttpResponse response = new NetHttpResponse(connection); + successfulConnection = true; + return response; + } finally { + if (!successfulConnection) { + connection.disconnect(); + } + } } @Override Index: google-http-client/src/main/java/com/google/api/client/http/javanet/NetHttpTransport.java =================================================================== --- a/google-http-client/src/main/java/com/google/api/client/http/javanet/NetHttpTransport.java +++ b/google-http-client/src/main/java/com/google/api/client/http/javanet/NetHttpTransport.java @@ -22,6 +22,13 @@ * Thread-safe HTTP low-level transport based on the {@code java.net} package. * *
+ * Users should consider modifying the keep alive property on {@link NetHttpTransport} to control + * whether the socket should be returned to a pool of connected sockets. More information is + * available here. + *
+ * + ** Implementation is thread-safe. For maximum efficiency, applications should use a single * globally-shared instance of the HTTP transport. *
Index: google-http-client/src/main/java/com/google/api/client/http/json/JsonHttpClient.java =================================================================== --- a/google-http-client/src/main/java/com/google/api/client/http/json/JsonHttpClient.java +++ b/google-http-client/src/main/java/com/google/api/client/http/json/JsonHttpClient.java @@ -361,10 +361,19 @@ * required, for example if a sequence of requests need to be built instead of a single request. * *- * Callers are responsible for closing the response's content input stream by calling - * {@link HttpResponse#ignore}. + * Callers are responsible for disconnecting the HTTP response by calling + * {@link HttpResponse#disconnect}. Example usage: *
* + *+ HttpResponse response = client.executeUnparsed(method, url, body); + try { + // process response.. + } finally { + response.disconnect(); + } + *+ * * @param method HTTP Method type * @param url The complete URL of the service where requests should be sent * @param body A POJO that can be serialized into JSON or {@code null} for none @@ -383,10 +392,19 @@ * required, for example if a custom error is required to be thrown. * *
- * Callers are responsible for closing the response's content input stream by calling - * {@link HttpResponse#ignore}. + * Callers are responsible for disconnecting the HTTP response by calling + * {@link HttpResponse#disconnect}. Example usage: *
* + *+ HttpResponse response = client.executeUnparsed(request); + try { + // process response.. + } finally { + response.disconnect(); + } + *+ * * @param request HTTP Request * @return {@link HttpResponse} type * @throws IOException if the request fails @@ -401,9 +419,18 @@ * {@link HttpResponse}. Subclasses may override if specific behavior is required. * *
- * Callers are responsible for closing the input stream. + * Callers are responsible for closing the input stream after it is processed. Example usage: *
* + *+ InputStream is = client.executeAsInputStream(); + try { + // Process input stream.. + } finally { + is.close(); + } + *+ * * @param method HTTP Method type * @param url The complete URL of the service where requests should be sent * @param body A POJO that can be serialized into JSON or {@code null} for none Index: google-http-client/src/main/java/com/google/api/client/http/json/JsonHttpRequest.java =================================================================== --- a/google-http-client/src/main/java/com/google/api/client/http/json/JsonHttpRequest.java +++ b/google-http-client/src/main/java/com/google/api/client/http/json/JsonHttpRequest.java @@ -158,10 +158,19 @@ * override if specific behavior is required. * *
- * Callers are responsible for closing the response's content input stream by calling - * {@link HttpResponse#ignore}. + * Callers are responsible for disconnecting the HTTP response by calling + * {@link HttpResponse#disconnect}. Example usage: *
* + *+ HttpResponse response = jsonHttpRequest.executeUnparsed(); + try { + // process response.. + } finally { + response.disconnect(); + } + *+ * * @return the {@link HttpResponse} * @throws IOException if the request fails */ @@ -177,9 +186,18 @@ * Subclasses may override if specific behavior is required. * *
- * Callers are responsible for closing the input stream. + * Callers are responsible for closing the input stream after it is processed. Example sample: *
* + *+ InputStream is = request.executeAsInputStream(); + try { + // Process input stream.. + } finally { + is.close(); + } + *+ * * @return input stream of the response content * @throws IOException if the request fails * @since 1.8 Index: google-http-client/src/test/java/com/google/api/client/http/HttpResponseExceptionTest.java =================================================================== --- a/google-http-client/src/test/java/com/google/api/client/http/HttpResponseExceptionTest.java +++ b/google-http-client/src/test/java/com/google/api/client/http/HttpResponseExceptionTest.java @@ -172,7 +172,6 @@ } } - @SuppressWarnings("deprecation") public void testSerialization() throws Exception { HttpTransport transport = new MockHttpTransport(); HttpRequest request = @@ -187,7 +186,6 @@ HttpResponseException e2 = (HttpResponseException) objectInput.readObject(); assertEquals(e.getMessage(), e2.getMessage()); assertEquals(e.getStatusCode(), e2.getStatusCode()); - assertNull(e2.getResponse()); assertNull(e2.getHeaders()); } } Index: google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java =================================================================== --- a/google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java +++ b/google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java @@ -20,11 +20,14 @@ import com.google.api.client.testing.http.MockLowLevelHttpRequest; import com.google.api.client.testing.http.MockLowLevelHttpResponse; import com.google.api.client.util.Key; +import com.google.api.client.util.StringUtils; import junit.framework.TestCase; +import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.InputStream; import java.util.Arrays; import java.util.logging.Level; @@ -207,6 +210,93 @@ assertEquals(SAMPLE, outputStream.toString("UTF-8")); } + class DisconnectLowLevelHttpResponse extends MockLowLevelHttpResponse { + boolean disconnectCalled; + DisconnectByteArrayInputStream content; + + @Override + public MockLowLevelHttpResponse setContent(String stringContent) { + content = stringContent == null ? null : new DisconnectByteArrayInputStream(StringUtils + .getBytesUtf8(stringContent)); + return this; + } + + @Override + public InputStream getContent() throws IOException { + return content; + } + + @Override + public void disconnect() { + disconnectCalled = true; + } + } + + class DisconnectByteArrayInputStream extends ByteArrayInputStream { + boolean closeCalled; + + public DisconnectByteArrayInputStream(byte[] buf) { + super(buf); + } + + @Override + public void close() throws IOException { + closeCalled = true; + } + } + + public void testDisconnectWithContent() throws IOException { + final DisconnectLowLevelHttpResponse lowLevelHttpResponse = + new DisconnectLowLevelHttpResponse(); + + HttpTransport transport = new MockHttpTransport() { + @Override + public LowLevelHttpRequest buildGetRequest(String url) throws IOException { + return new MockLowLevelHttpRequest() { + @Override + public LowLevelHttpResponse execute() throws IOException { + lowLevelHttpResponse.setContentType("application/json; charset=UTF-8"); + lowLevelHttpResponse.setContent(SAMPLE); + return lowLevelHttpResponse; + } + }; + } + }; + HttpRequest request = + transport.createRequestFactory().buildGetRequest(HttpTesting.SIMPLE_GENERIC_URL); + HttpResponse response = request.execute(); + + assertFalse(lowLevelHttpResponse.disconnectCalled); + assertFalse(lowLevelHttpResponse.content.closeCalled); + response.disconnect(); + assertTrue(lowLevelHttpResponse.disconnectCalled); + assertTrue(lowLevelHttpResponse.content.closeCalled); + } + + public void testDisconnectWithNoContent() throws IOException { + final DisconnectLowLevelHttpResponse lowLevelHttpResponse = + new DisconnectLowLevelHttpResponse(); + + HttpTransport transport = new MockHttpTransport() { + @Override + public LowLevelHttpRequest buildGetRequest(String url) throws IOException { + return new MockLowLevelHttpRequest() { + @Override + public LowLevelHttpResponse execute() throws IOException { + return lowLevelHttpResponse; + } + }; + } + }; + HttpRequest request = + transport.createRequestFactory().buildGetRequest(HttpTesting.SIMPLE_GENERIC_URL); + HttpResponse response = request.execute(); + + assertFalse(lowLevelHttpResponse.disconnectCalled); + response.disconnect(); + assertTrue(lowLevelHttpResponse.disconnectCalled); + } + public void testContentLoggingLimitWithLoggingEnabledAndDisabled() throws IOException { subtestContentLoggingLimit("", 2, false); subtestContentLoggingLimit("A", 2, false);