Adding HttpResponse.download(outputstream) and JsonHttpRequest.download(outputstream)

[yanivi, 2012-04-09 18:54:43]

http://codereview.appspot.com/5985069/diff/1/google-http-client/src/main/java...
File
google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java
(right):

http://codereview.appspot.com/5985069/diff/1/google-http-client/src/main/java...
google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java:431:
response.download(outputStream);
should an example of closing the outputstream in a finally block.  see example
code in AbstractInputStreamContent.copy.

http://codereview.appspot.com/5985069/diff/1/google-http-client/src/main/java...
google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java:436:
* This method closes the content of the HTTP response from {@link
#getContent()}.
specify that it does not close the output stream

http://codereview.appspot.com/5985069/diff/1/google-http-client/src/main/java...
google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java:445:
AbstractInputStreamContent.copy(inputStream, outputStream, true);
call the AbstractInputStreamContent.copy method without the last param since it
calls this one with true for the last param

[rmistry, 2012-04-10 12:12:58]

http://codereview.appspot.com/5985069/diff/1/google-http-client/src/main/java...
File
google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java
(right):

http://codereview.appspot.com/5985069/diff/1/google-http-client/src/main/java...
google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java:431:
response.download(outputStream);
On 2012/04/09 18:54:43, yanivi wrote:
> should an example of closing the outputstream in a finally block.  see example
> code in AbstractInputStreamContent.copy.

Done.

http://codereview.appspot.com/5985069/diff/1/google-http-client/src/main/java...
google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java:436:
* This method closes the content of the HTTP response from {@link
#getContent()}.
On 2012/04/09 18:54:43, yanivi wrote:
> specify that it does not close the output stream

Done.

http://codereview.appspot.com/5985069/diff/1/google-http-client/src/main/java...
google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java:445:
AbstractInputStreamContent.copy(inputStream, outputStream, true);
On 2012/04/09 18:54:43, yanivi wrote:
> call the AbstractInputStreamContent.copy method without the last param since
it
> calls this one with true for the last param

Done.

[yanivi, 2012-04-11 01:18:59]

http://codereview.appspot.com/5985069/diff/3001/google-http-client/src/main/j...
File
google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java
(right):

http://codereview.appspot.com/5985069/diff/3001/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java:430:
HttpResponse response = request.execute();
this line needs to be inside try block

http://codereview.appspot.com/5985069/diff/3001/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java:444:
* This method does not close the returned output stream.
"returned" -> "given"?

http://codereview.appspot.com/5985069/diff/3001/google-http-client/src/main/j...
File
google-http-client/src/main/java/com/google/api/client/http/json/JsonHttpRequest.java
(right):

http://codereview.appspot.com/5985069/diff/3001/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/json/JsonHttpRequest.java:145:
public void download(OutputStream outputStream) throws IOException {
[optional] do you want to also add JsonHttpClient.download?

[rmistry, 2012-04-11 09:54:03]

http://codereview.appspot.com/5985069/diff/3001/google-http-client/src/main/j...
File
google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java
(right):

http://codereview.appspot.com/5985069/diff/3001/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java:430:
HttpResponse response = request.execute();
On 2012/04/11 01:18:59, yanivi wrote:
> this line needs to be inside try block

Done.

http://codereview.appspot.com/5985069/diff/3001/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java:444:
* This method does not close the returned output stream.
On 2012/04/11 01:18:59, yanivi wrote:
> "returned" -> "given"?

Done.

http://codereview.appspot.com/5985069/diff/3001/google-http-client/src/main/j...
File
google-http-client/src/main/java/com/google/api/client/http/json/JsonHttpRequest.java
(right):

http://codereview.appspot.com/5985069/diff/3001/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/json/JsonHttpRequest.java:145:
public void download(OutputStream outputStream) throws IOException {
On 2012/04/11 01:18:59, yanivi wrote:
> [optional] do you want to also add JsonHttpClient.download?

I do not think we need it in JsonHttpClient, having it in JsonHttpRequest should
be sufficient.

[yanivi, 2012-04-11 11:03:17]

LGTM

[rmistry, 2012-04-11 14:56:10]

Submitting..