Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(147)

Issue 5482047: Changes to support 5450097 (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 3 months ago by rmistry
Modified:
12 years, 1 month ago
Reviewers:
yanivi
Base URL:
https://google-http-java-client.googlecode.com/hg/
Visibility:
Public.

Description

* Added a closeInputStream getter/setter for AbstractInputStreamContent.writeTo. * Added a closeInputStream parameter to AbstractInputStreamContent.copy * Made AbstractInputStreamContent.getInputStream public. * Flushing the output stream at the end of HttpContent.writeTo implementations. * Added allowEmptyContent to HttpRequest (defaulted to true) with a unit test.

Patch Set 1 #

Total comments: 10

Patch Set 2 : Addressing code review comments #

Total comments: 19

Patch Set 3 : Addressing code review comments #

Total comments: 9

Patch Set 4 : Addressing code review comments #

Patch Set 5 : Minor fixes #

Total comments: 8

Patch Set 6 : Addressing code review comments #

Patch Set 7 : Modifying InputStreamContent #

Patch Set 8 : Minor fixes #

Total comments: 13

Patch Set 9 : Merging in 5572060 #

Patch Set 10 : Addressing code review comments #

Patch Set 11 : Minor fixes #

Patch Set 12 : Merging #

Patch Set 13 : Minor fixes #

Total comments: 11

Patch Set 14 : Addressing code review comments #

Patch Set 15 : Minor fixes #

Patch Set 16 : getInputStream should be public with an upgrade warning #

Patch Set 17 : Added TODOs #

Total comments: 18

Patch Set 18 : Addressing code review comments #

Total comments: 6

Patch Set 19 : Addressing code review comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+252 lines, -56 lines) Patch
M google-http-client/src/main/java/com/google/api/client/http/AbstractHttpContent.java View 1 2 3 4 5 6 7 8 1 chunk +5 lines, -1 line 0 comments Download
M google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 8 chunks +78 lines, -7 lines 0 comments Download
M google-http-client/src/main/java/com/google/api/client/http/ByteArrayContent.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 2 chunks +6 lines, -1 line 0 comments Download
M google-http-client/src/main/java/com/google/api/client/http/FileContent.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 2 chunks +6 lines, -1 line 0 comments Download
M google-http-client/src/main/java/com/google/api/client/http/GZipContent.java View 1 2 3 4 5 6 7 8 1 chunk +11 lines, -1 line 0 comments Download
M google-http-client/src/main/java/com/google/api/client/http/HttpContent.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +9 lines, -1 line 0 comments Download
M google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 8 chunks +43 lines, -13 lines 0 comments Download
M google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +6 lines, -2 lines 0 comments Download
M google-http-client/src/main/java/com/google/api/client/http/InputStreamContent.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 4 chunks +26 lines, -2 lines 0 comments Download
M google-http-client/src/main/java/com/google/api/client/http/LogContent.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +10 lines, -5 lines 0 comments Download
M google-http-client/src/main/java/com/google/api/client/http/MultipartRelatedContent.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +6 lines, -0 lines 0 comments Download
M google-http-client/src/main/java/com/google/api/client/http/javanet/NetHttpRequest.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +5 lines, -1 line 0 comments Download
M google-http-client/src/main/java/com/google/api/client/http/protobuf/ProtoHttpContent.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +1 line, -0 lines 0 comments Download
M google-http-client/src/main/java/com/google/api/client/json/jackson/JacksonFactory.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +1 line, -0 lines 0 comments Download
M google-http-client/src/main/java/com/google/api/client/testing/http/MockHttpContent.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +1 line, -0 lines 0 comments Download
M google-http-client/src/test/java/com/google/api/client/http/HttpRequestTest.java View 1 2 3 9 chunks +38 lines, -21 lines 0 comments Download

Messages

Total messages: 29
rmistry
12 years, 3 months ago (2011-12-12 15:29:20 UTC) #1
rmistry
On 2011/12/12 15:29:20, rmistry wrote: Ping.
12 years, 3 months ago (2011-12-16 04:19:14 UTC) #2
yanivi
http://codereview.appspot.com/5482047/diff/1/google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java File google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java (right): http://codereview.appspot.com/5482047/diff/1/google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java#newcode60 google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java:60: public abstract InputStream getInputStream() throws IOException; while we are ...
12 years, 2 months ago (2012-01-13 16:27:31 UTC) #3
yanivi
http://codereview.appspot.com/5482047/diff/1/google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java File google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java (right): http://codereview.appspot.com/5482047/diff/1/google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java#newcode60 google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java:60: public abstract InputStream getInputStream() throws IOException; By the way, ...
12 years, 2 months ago (2012-01-13 16:40:30 UTC) #4
rmistry
http://codereview.appspot.com/5482047/diff/1/google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java File google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java (right): http://codereview.appspot.com/5482047/diff/1/google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java#newcode60 google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java:60: public abstract InputStream getInputStream() throws IOException; On 2012/01/13 16:27:31, ...
12 years, 2 months ago (2012-01-17 18:27:13 UTC) #5
yanivi
http://codereview.appspot.com/5482047/diff/7001/google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java File google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java (right): http://codereview.appspot.com/5482047/diff/7001/google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java#newcode3 google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java:3: * [optional] I would be nice if you could ...
12 years, 2 months ago (2012-01-20 20:32:17 UTC) #6
yanivi
http://codereview.appspot.com/5482047/diff/7001/google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java File google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java (right): http://codereview.appspot.com/5482047/diff/7001/google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java#newcode86 google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java:86: * empty contents. Defaults to {@code false}. Design issue: ...
12 years, 2 months ago (2012-01-20 21:45:38 UTC) #7
rmistry
http://codereview.appspot.com/5482047/diff/7001/google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java File google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java (right): http://codereview.appspot.com/5482047/diff/7001/google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java#newcode3 google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java:3: * On 2012/01/20 20:32:17, yanivi wrote: > [optional] I ...
12 years, 2 months ago (2012-01-23 16:11:54 UTC) #8
yanivi
http://codereview.appspot.com/5482047/diff/7001/google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java File google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java (right): http://codereview.appspot.com/5482047/diff/7001/google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java#newcode62 google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java:62: @Deprecated On 2012/01/23 16:11:54, rmistry wrote: > On 2012/01/20 ...
12 years, 2 months ago (2012-01-23 18:30:56 UTC) #9
rmistry
http://codereview.appspot.com/5482047/diff/12001/google-http-client/src/main/java/com/google/api/client/http/ByteArrayContent.java File google-http-client/src/main/java/com/google/api/client/http/ByteArrayContent.java (right): http://codereview.appspot.com/5482047/diff/12001/google-http-client/src/main/java/com/google/api/client/http/ByteArrayContent.java#newcode119 google-http-client/src/main/java/com/google/api/client/http/ByteArrayContent.java:119: protected InputStream getInputStream() { On 2012/01/23 18:30:56, yanivi wrote: ...
12 years, 2 months ago (2012-01-23 22:09:36 UTC) #10
rmistry
Ping.
12 years, 2 months ago (2012-01-26 16:03:22 UTC) #11
yanivi
LGTM http://codereview.appspot.com/5482047/diff/7001/google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java File google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java (right): http://codereview.appspot.com/5482047/diff/7001/google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java#newcode69 google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java:69: * {@link InputStream} from the source data whenever ...
12 years, 2 months ago (2012-01-26 20:53:16 UTC) #12
yanivi
LGTM http://codereview.appspot.com/5482047/diff/12001/google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java File google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java (right): http://codereview.appspot.com/5482047/diff/12001/google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java#newcode726 google-http-client/src/main/java/com/google/api/client/http/HttpRequest.java:726: if (!isAllowEmptyContent() On 2012/01/26 20:53:17, yanivi wrote: > ...
12 years, 2 months ago (2012-01-26 20:54:26 UTC) #13
rmistry
I will not submit this till http://codereview.appspot.com/5450097/ is ready to go as well. http://codereview.appspot.com/5482047/diff/7001/google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java File ...
12 years, 2 months ago (2012-01-26 23:32:27 UTC) #14
yanivi
It would really help to merge http://codereview.appspot.com/5572060/ into this one. http://codereview.appspot.com/5482047/diff/17002/google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java File google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java (right): http://codereview.appspot.com/5482047/diff/17002/google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java#newcode101 ...
12 years, 2 months ago (2012-01-27 16:56:37 UTC) #15
yanivi
http://codereview.appspot.com/5482047/diff/21009/google-http-client/src/main/java/com/google/api/client/http/InputStreamContent.java File google-http-client/src/main/java/com/google/api/client/http/InputStreamContent.java (right): http://codereview.appspot.com/5482047/diff/21009/google-http-client/src/main/java/com/google/api/client/http/InputStreamContent.java#newcode117 google-http-client/src/main/java/com/google/api/client/http/InputStreamContent.java:117: public void setLength(long length) { Please change the return ...
12 years, 2 months ago (2012-01-27 17:57:24 UTC) #16
rmistry
http://codereview.appspot.com/5482047/diff/21009/google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java File google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java (right): http://codereview.appspot.com/5482047/diff/21009/google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java#newcode25 google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java:25: * The {@link #type} field is required. Subclasses should ...
12 years, 2 months ago (2012-01-27 19:43:06 UTC) #17
rmistry
Ping.
12 years, 1 month ago (2012-02-06 13:47:13 UTC) #18
rmistry
Synced with head.
12 years, 1 month ago (2012-02-08 16:25:21 UTC) #19
yanivi
http://codereview.appspot.com/5482047/diff/17002/google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java File google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java (right): http://codereview.appspot.com/5482047/diff/17002/google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java#newcode101 google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java:101: inputStream.close(); On 2012/01/27 16:56:37, yanivi wrote: > Perhaps nicer ...
12 years, 1 month ago (2012-02-08 18:15:17 UTC) #20
rmistry
http://codereview.appspot.com/5482047/diff/17002/google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java File google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java (right): http://codereview.appspot.com/5482047/diff/17002/google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java#newcode101 google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java:101: inputStream.close(); On 2012/02/08 18:15:17, yanivi wrote: > On 2012/01/27 ...
12 years, 1 month ago (2012-02-09 14:44:44 UTC) #21
yanivi
waiting for 5450097 before reviewing http://codereview.appspot.com/5482047/diff/37004/google-http-client/src/main/java/com/google/api/client/http/HttpContent.java File google-http-client/src/main/java/com/google/api/client/http/HttpContent.java (left): http://codereview.appspot.com/5482047/diff/37004/google-http-client/src/main/java/com/google/api/client/http/HttpContent.java#oldcode44 google-http-client/src/main/java/com/google/api/client/http/HttpContent.java:44: void writeTo(OutputStream out) throws ...
12 years, 1 month ago (2012-02-09 15:24:23 UTC) #22
rmistry
http://codereview.appspot.com/5482047/diff/37004/google-http-client/src/main/java/com/google/api/client/http/HttpContent.java File google-http-client/src/main/java/com/google/api/client/http/HttpContent.java (left): http://codereview.appspot.com/5482047/diff/37004/google-http-client/src/main/java/com/google/api/client/http/HttpContent.java#oldcode44 google-http-client/src/main/java/com/google/api/client/http/HttpContent.java:44: void writeTo(OutputStream out) throws IOException; On 2012/02/09 15:24:23, yanivi ...
12 years, 1 month ago (2012-02-09 16:18:28 UTC) #23
rmistry
http://codereview.appspot.com/5482047/diff/17002/google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java File google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java (right): http://codereview.appspot.com/5482047/diff/17002/google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java#newcode101 google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java:101: inputStream.close(); On 2012/02/09 14:44:45, rmistry wrote: > On 2012/02/08 ...
12 years, 1 month ago (2012-02-09 17:03:07 UTC) #24
yanivi
http://codereview.appspot.com/5482047/diff/17002/google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java File google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java (right): http://codereview.appspot.com/5482047/diff/17002/google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java#newcode101 google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java:101: inputStream.close(); On 2012/02/09 17:03:07, rmistry wrote: > On 2012/02/09 ...
12 years, 1 month ago (2012-02-09 21:50:45 UTC) #25
rmistry
http://codereview.appspot.com/5482047/diff/17002/google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java File google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java (right): http://codereview.appspot.com/5482047/diff/17002/google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java#newcode101 google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java:101: inputStream.close(); On 2012/02/09 21:50:45, yanivi wrote: > On 2012/02/09 ...
12 years, 1 month ago (2012-02-10 14:42:51 UTC) #26
yanivi
LGTM http://codereview.appspot.com/5482047/diff/48008/google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java File google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java (right): http://codereview.appspot.com/5482047/diff/48008/google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java#newcode109 google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java:109: public boolean getCloseInputStream() { missing JavaDoc http://codereview.appspot.com/5482047/diff/48008/google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java#newcode109 google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java:109: ...
12 years, 1 month ago (2012-02-10 15:46:58 UTC) #27
rmistry
http://codereview.appspot.com/5482047/diff/48008/google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java File google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java (right): http://codereview.appspot.com/5482047/diff/48008/google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java#newcode109 google-http-client/src/main/java/com/google/api/client/http/AbstractInputStreamContent.java:109: public boolean getCloseInputStream() { On 2012/02/10 15:46:58, yanivi wrote: ...
12 years, 1 month ago (2012-02-10 16:06:50 UTC) #28
rmistry
12 years, 1 month ago (2012-02-10 16:17:40 UTC) #29
Submitting..
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b