10000 Changes to support http://codereview.appspot.com/5450097/ · sdecima/google-http-java-client@139ee3d · GitHub
[go: up one dir, main page]
Skip to content

Commit 139ee3d

Browse files
rmistryrmistry
committed
1 parent aa46c31 commit 139ee3d

File tree

16 files changed

+252
-56
lines changed

16 files changed

+252
-56
lines changed

‎google-http-client/src/main/java/com/google/api/client/http/AbstractHttpContent.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,11 @@ protected long computeLength() throws IOException {
6363
return -1;
6464
}
6565
ByteCountingOutputStream countingStream = new ByteCountingOutputStream();
66-
writeTo(countingStream);
66+
try {
67+
writeTo(countingStream);
68+
} finally {
69+
countingStream.close();
70+
}
6771
return countingStream.count;
6872
}
6973

‎google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java

Lines changed: 78 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,12 @@ public abstract class AbstractInputStreamContent implements HttpContent {
4343
/** Content encoding (for example {@code "gzip"}) or {@code null} for none. */
4444
private String encoding;
4545

46+
/**
47+
* Whether the input stream should be closed at the end of {@link #writeTo}. Default is
48+
* {@code true}.
49+
*/
50+
private boolean closeInputStream = true;
51+
4652
/**
4753
* @param type Content type or {@code null} for none
4854
* @since 1.5
@@ -56,14 +62,20 @@ public AbstractInputStreamContent(String type) {
5662
* {@link AbstractInputStreamContent}. If the specific implementation will return {@code true} for
5763
* {@link #retrySupported()} this should be a factory function which will create a new
5864
* {@link InputStream} from the source data whenever invoked.
65+
*
66+
* <p>
67+
* Upgrade warning: in prior version 1.6 {@link #getInputStream} was protected, it is now public.
68+
* </p>
69+
*
70+
* @since 1.7
5971
*/
60-
protected abstract InputStream getInputStream() throws IOException;
72+
public abstract InputStream getInputStream() throws IOException;
6173

6274
public void writeTo(OutputStream out) throws IOException {
6375
InputStream inputStream = getInputStream();
6476
long contentLength = getLength();
6577
if (contentLength < 0) {
66-
copy(inputStream, out);
78+
copy(inputStream, out, closeInputStream);
6779
} else {
6880
byte[] buffer = new byte[BUFFER_SIZE];
6981
try {
@@ -78,9 +90,12 @@ public void writeTo(OutputStream out) throws IOException {
7890
remaining -= read;
7991
}
8092
} finally {
81-
inputStream.close();
93+
if (closeInputStream) {
94+
inputStream.close();
95+
}
8296
}
8397
}
98+
out.flush();
8499
}
85100

86101
public String getEncoding() {
@@ -92,7 +107,18 @@ public String getType() {
92107
}
93108

94109
/**
95-
* Sets the content encoding (for example {@code "gzip"}) or {@code null} for none.
110+
* Returns whether the input stream should be closed at the end of {@link #writeTo}. Default is
111+
* {@code true}.
112+
*
113+
* @since 1.7
114+
*/
115+
public final boolean getCloseInputStream() {
116+
return closeInputStream;
117+
}
118+
119+
/**
120+
* Sets the content encoding (for example {@code "gzip"}) or {@code null} for none. Subclasses
121+
* should override by calling super.
96122
*
97123
* @since 1.5
98124
*/
@@ -102,7 +128,7 @@ public AbstractInputStreamContent setEncoding(String encoding) {
102128
}
103129

104130
/**
105-
* Sets the content type or {@code null} for none.
131+
* Sets the content type or {@code null} for none. Subclasses should override by calling super.
106132
*
107133
* @since 1.5
108134
*/
@@ -111,12 +137,25 @@ public AbstractInputStreamContent setType(String type) {
111137
return this;
112138
}
113139

140+
/**
141+
* Sets whether the input stream should be closed at the end of {@link #writeTo}. Default is
142+
* {@code true}. Subclasses should override by calling super.
143+
*
144+
* @since 1.7
145+
*/
146+
public AbstractInputStreamContent setCloseInputStream(boolean closeInputStream) {
147+
this.closeInputStream = closeInputStream;
148+
return this;
149+
}
150+
114151
/**
115152
* Writes the content provided by the given source input stream into the given destination output
116153
* stream.
154+
*
117155
* <p>
118-
* The input stream is guaranteed to be closed at the end of the method.
156+
* The input stream is guaranteed to be closed at the end of this method.
119157
* </p>
158+
*
120159
* <p>
121160
* Sample use:
122161
*
@@ -137,14 +176,46 @@ static void downloadMedia(HttpResponse response, File file)
137176
* @param outputStream destination output stream
138177
*/
139178
public static void copy(InputStream inputStream, OutputStream outputStream) throws IOException {
179+
copy(inputStream, outputStream, true);
180+
}
181+
182+
/**
183+
* Writes the content provided by the given source input stream into the given destination output
184+
* stream.
185+
*
186+
* <p>
187+
* Sample use:
188+
*
189+
* <pre><code>
190+
static void downloadMedia(HttpResponse response, File file)
191+
throws IOException {
192+
FileOutputStream out = new FileOutputStream(file);
193+
try {
194+
AbstractInputStreamContent.copy(response.getContent(), out, true);
195+
} finally {
196+
out.close();
197+
}
198+
}
199+
* </code></pre>
200+
* </p>
201+
*
202+
* @param inputStream source input stream
203+
* @param outputStream destination output stream
204+
* @param closeInputStream whether the input stream should be closed at the end of this method
205+
* @since 1.7
206+
*/
207+
public static void copy(InputStream inputStream, OutputStream outputStream,
208+
boolean closeInputStream) throws IOException {
140209
try {
141210
byte[] tmp = new byte[BUFFER_SIZE];
142211
int bytesRead;
143212
while ((bytesRead = inputStream.read(tmp)) != -1) {
144213
outputStream.write(tmp, 0, bytesRead);
145214
}
146215
} finally {
147-
inputStream.close();
216+
if (closeInputStream) {
217+
inputStream.close();
218+
}
148219
}
149220
}
150221
}

‎google-http-client/src/main/java/com/google/api/client/http/ByteArrayContent.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ public boolean retrySupported() {
115115
}
116116

117117
@Override
118-
protected InputStream getInputStream() {
118+
public InputStream getInputStream() {
119119
return new ByteArrayInputStream(byteArray, offset, length);
120120
}
121121

@@ -128,4 +128,9 @@ public ByteArrayContent setEncoding(String encoding) {
128128
public ByteArrayContent setType(String type) {
129129
return (ByteArrayContent) super.setType(type);
130130
}
131+
132+
@Override
133+
public ByteArrayContent setCloseInputStream(boolean closeInputStream) {
134+
return (ByteArrayContent) super.setCloseInputStream(closeInputStream);
135+
}
131136
}

‎google-http-client/src/main/java/com/google/api/client/http/FileContent.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ public boolean retrySupported() {
6767
}
6868

6969
@Override
70-
protected InputStream getInputStream() throws FileNotFoundException {
70+
public InputStream getInputStream() throws FileNotFoundException {
7171
return new FileInputStream(file);
7272
}
7373

@@ -89,4 +89,9 @@ public FileContent setEncoding(String encoding) {
8989
public FileContent setType(String type) {
9090
return (FileContent) super.setType(type);
9191
}
92+
93+
@Override
94+
public FileContent setCloseInputStream(boolean closeInputStream) {
95+
return (FileContent) super.setCloseInputStream(closeInputStream);
96+
}
9297
}

‎google-http-client/src/main/java/com/google/api/client/http/GZipContent.java

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,20 @@ final class GZipContent extends AbstractHttpContent {
4141
this.contentType = contentType;
4242
}
4343

44+
/**
45+
* Writes the content to the given output stream and closes the output stream.
46+
*
47+
* <p>
48+
* The output stream is closed by calling {@link GZIPOutputStream#close} to avoid a resource leak
49+
* caused due to an instance of {@link java.util.zip.Deflater} being left open. See Bug <a
50+
* href='http://code.google.com/p/google-http-java-client/issues/detail?id=61'>61</a> for more
51+
* information.
52+
* </p>
53+
*/
4454
public void writeTo(OutputStream out) throws IOException {
4555
GZIPOutputStream zipper = new GZIPOutputStream(out);
4656
httpContent.writeTo(zipper);
47-
zipper.finish();
57+
zipper.close();
4858
}
4959

5060
@Override

‎google-http-client/src/main/java/com/google/api/client/http/HttpContent.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,15 @@ public interface HttpContent {
4040
/** Returns the content type or {@code null} for none. */
4141
String getType();
4242

43-
/** Writes the content to the given output stream. */
43+
/**
44+
* Writes the content to the given output stream.
45+
*
46+
* <p>
47+
* The recommendation for implementations is that they should not close the output stream. Callers
48+
* should not assume whether or not the output stream has been closed. Implementations that do not
49+
* close the output stream should flush it at the end of the method.
50+
* </p>
51+
*/
4452
void writeTo(OutputStream out) throws IOException;
4553

4654
/**

‎google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java

Lines changed: 43 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,8 @@ public final class HttpRequest {
4949
*
5050
* @since 1.4
5151
*/
52-
public static final String USER_AGENT_SUFFIX =
53-
"Google-HTTP-Java-Client/" + Strings.VERSION + " (gzip)";
52+
public static final String USER_AGENT_SUFFIX = "Google-HTTP-Java-Client/" + Strings.VERSION
53+
+ " (gzip)";
5454

5555
/**
5656
* HTTP request execute interceptor to intercept the start of {@link #execute()} (before executing
@@ -80,6 +80,13 @@ static String executeAndGetValueOfSomeCustomHeader(HttpRequest request) {
8080
*/
8181
private HttpHeaders responseHeaders = new HttpHeaders();
8282

83+
/**
84+
* Some servers will fail to process a POST/PUT/PATCH unless Content-Length header >= 1. If this
85+
* value is set to {@code false} then " " is set as the content with Content-Length {@code 1} for
86+
* empty contents. Defaults to {@code true}.
87+
*/
88+
private boolean allowEmptyContent = true;
89+
8390
/**
8491
* Set the number of retries that will be allowed to execute as the result of an
8592
* {@link HttpUnsuccessfulResponseHandler} before being terminated or {@code 0} to not retry
@@ -521,6 +528,29 @@ public HttpRequest setUnsuccessfulResponseHandler(
521528
return this;
522529
}
523530

531+
/**
532+
* Some servers will fail to process a POST/PUT/PATCH unless Content-Length header >= 1. If this
533+
* value is set to {@code false} then " " is set as the content with Content-Length {@code 1} for
534+
* empty contents. Defaults to {@code true}.
535+
*
536+
* @since 1.7
537+
*/
538+
public HttpRequest setAllowEmptyContent(boolean allowEmptyContent) {
539+
this.allowEmptyContent = allowEmptyContent;
540+
return this;
541+
}
542+
543+
/**
544+
* Some servers will fail to process a POST/PUT/PATCH unless Content-Length header >= 1. If this
545+
* value is set to {@code false} then " " is set as the content with Content-Length {@code 1} for
546+
* empty contents. Defaults to {@code true}.
547+
*
548+
* @since 1.7
549+
*/
550+
public boolean isAllowEmptyContent() {
551+
return allowEmptyContent;
552+
}
553+
524554
/**
525555
* Returns the number of retries that will be allowed to execute as the result of an
526556
* {@link HttpUnsuccessfulResponseHandler} before being terminated or {@code 0} to not retry
@@ -610,7 +640,7 @@ public boolean getThrowExceptionOnExecuteError() {
610640
}
611641

612642
/**
613-
* Sets whether to throw an exception at the end of {@link #execute()} on an HTTP error code
643+
* Sets whether to throw an exception at the end of {@link #execute()} on a HTTP error code
614644
* (non-2XX) after all retries and response handlers have been exhausted.
615645
*
616646
* <p>
@@ -674,13 +704,13 @@ public HttpResponse execute() throws IOException {
674704
lowLevelHttpRequest = transport.buildGetRequest(urlString);
675705
break;
676706
case HEAD:
677-
Preconditions.checkArgument(
678-
transport.supportsHead(), "HTTP transport doesn't support HEAD");
707+
Preconditions.checkArgument(transport.supportsHead(),
708+
"HTTP transport doesn't support HEAD");
679709
lowLevelHttpRequest = transport.buildHeadRequest(urlString);
680710
break;
681711
case PATCH:
682-
Preconditions.checkArgument(
683-
transport.supportsPatch(), "HTTP transport doesn't support PATCH");
712+
Preconditions.checkArgument(transport.supportsPatch(),
713+
"HTTP transport doesn't support PATCH");
684714
lowLevelHttpRequest = transport.buildPatchRequest(urlString);
685715
break;
686716
case POST:
@@ -726,8 +756,8 @@ public HttpResponse execute() throws IOException {
726756
}
727757
// content
728758
HttpContent content = this.content;
729-
// Hack: some servers will fail to process a POST/PUT/PATCH unless Content-Length header >= 1
730-
if ((method == HttpMethod.PUT || method == HttpMethod.POST || method == HttpMethod.PATCH)
759+
if (!isAllowEmptyContent()
760+
&& (method == HttpMethod.PUT || method == HttpMethod.POST || method == HttpMethod.PATCH)
731761
&& (content == null || content.getLength() == 0)) {
732762
content = ByteArrayContent.fromString(null, " ");
733763
}
@@ -742,8 +772,9 @@ public HttpResponse execute() throws IOException {
742772
&& LogContent.isTextBasedContentType(contentType)
743773
&& (loggable && !disableContentLogging || logger.isLoggable(Level.ALL))) {
744774
if (contentLength <= contentLoggingLimit && contentLoggingLimit != 0) {
745-
content = new LogContent(content, contentType, contentEncoding, contentLength,
746-
contentLoggingLimit);
775+
content =
776+
new LogContent(content, contentType, contentEncoding, contentLength,
777+
contentLoggingLimit);
747778
}
748779
}
749780
// gzip
@@ -795,8 +826,7 @@ public HttpResponse execute() throws IOException {
795826
// The unsuccessful request's error could not be handled and it is a redirect request.
796827
handleRedirect(response);
797828
redirectRequest = true;
798-
} else if (
799-
retrySupported && backOffPolicy != null
829+
} else if (retrySupported && backOffPolicy != null
800830
&& backOffPolicy.isBackOffRequired(response.getStatusCode())) {
801831
// The unsuccessful request's error could not be handled and should be backed off before
802832
// retrying.

‎google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -378,8 +378,12 @@ public InputStream getContent() throws IOException {
378378
!disableContentLogging && logger.isLoggable(Level.CONFIG) || logger.isLoggable(Level.ALL);
379379
if (loggable) {
380380
ByteArrayOutputStream debugStream = new ByteArrayOutputStream();
381-
AbstractInputStreamContent.copy(content, debugStream);
382-
debugContentByteArray = debugStream.toByteArray();
381+
try {
382+
AbstractInputStreamContent.copy(content, debugStream);
383+
debugContentByteArray = debugStream.toByteArray();
384+
} finally {
385+
debugStream.close();
386+
}
383387
content = new ByteArrayInputStream(debugContentByteArray);
384388
logger.config("Response size: " + debugContentByteArray.length + " bytes");
385389
}

0 commit comments

Comments
 (0)
0